Skip to content

fix(cli): block recovery after lifecycle probe failures - #10437

Merged
prekshivyas merged 1 commit into
mainfrom
codex/fail-closed-gateway-recovery
Aug 27, 2026
Merged

fix(cli): block recovery after lifecycle probe failures#10437
prekshivyas merged 1 commit into
mainfrom
codex/fail-closed-gateway-recovery

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Named-gateway recovery now rejects non-recoverable lifecycle probe failures before it selects or starts a gateway. The change closes the fail-closed test gap left by #10424.

Related Issue

Related to #10421. Follow-up to #10424.

Changes

  • Reuse the typed OpenShell CLI error classifier for gateway lifecycle probes.
  • Block recovery for authentication, schema, gateway identity, and invalid-request failures.
  • Stop after a failed post-selection probe without starting the gateway.
  • Apply the same guard to the direct named-gateway reconciliation path.
  • Add negative tests that assert gateway select and gateway start do not run.
  • Root cause: raw lifecycle failures were classified as recoverable topology states.
  • Detection gap: fix(cli): recover named gateway before sandbox inventory #10424 tested these errors only after its mocked recovery call returned success.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: The implementation review covered authentication, schema, gateway identity, invalid-request, and post-selection failure states. Negative tests assert that forbidden mutations do not run.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable; this PR does not change scripts/prepare-dgx-station-host.sh.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project cli --maxWorkers=1 src/lib/gateway-runtime-action.test.ts src/lib/adapters/openshell/sandbox-observer-cli.test.ts src/lib/actions/sandbox/gateway-state-drift.test.ts: 3 files and 47 tests passed. npm run test:titles:check passed.
  • Applicable broad gate passed — Not applicable; this change extends one lifecycle classifier and two current mutation guards.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Informational local run: npm run test:changed passed 5,485 tests and failed 16 tests across nine unchanged files under host concurrency. Eight files passed sequentially; one unchanged uninstall test retained a five-second timeout. GitHub CI remains the authoritative broad result.


Signed-off-by: Prekshi Vyas prekshiv@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved sandbox recovery handling when gateway lifecycle checks detect authentication, schema, identity, or request-validation errors.
    • Prevented recovery from selecting or starting a gateway when recovery is blocked.
    • Preserved the original missing-sandbox result and existing registry data in blocked recovery scenarios.
    • Improved recognition of missing administrator privileges during gateway command checks.
  • Tests
    • Added coverage for blocked recovery, lifecycle probe failures, and prevention of unintended gateway startup.

@prekshivyas prekshivyas self-assigned this Aug 27, 2026
@github-code-quality

github-code-quality Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 7ea519d in the codex/fail-closed-ga... branch remains at 96%, unchanged from commit 583331a in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 7ea519d in the codex/fail-closed-ga... branch remains at 84%, unchanged from commit d67d2aa in the main branch.

Show a line coverage summary of the most impacted files.
File main d67d2aa codex/fail-closed-ga... 7ea519d +/-
src/lib/shields/index.ts 85% 83% -2%
src/lib/actions...-add-restart.ts 17% 16% -1%
src/lib/shields...ate-mutation.ts 78% 77% -1%
src/lib/adapter...observer-cli.ts 100% 99% -1%
src/lib/onboard...uild-context.ts 74% 74% 0%
src/lib/actions...ateway-state.ts 79% 80% +1%
src/lib/onboard...ate-mutation.ts 71% 72% +1%
src/lib/actions...ter-openclaw.ts 85% 88% +3%
src/lib/actions...ter-teardown.ts 81% 88% +7%
src/lib/actions...dge-adapters.ts 81% 95% +14%

Updated August 27, 2026 00:48 UTC

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e8db5b2f-07b1-45b3-b205-7a7fed88e58a

📥 Commits

Reviewing files that changed from the base of the PR and between 583331a and 7ea519d.

📒 Files selected for processing (5)
  • src/lib/actions/sandbox/gateway-state-drift.test.ts
  • src/lib/actions/sandbox/gateway-state.ts
  • src/lib/adapters/openshell/sandbox-observer-cli.ts
  • src/lib/gateway-runtime-action.test.ts
  • src/lib/gateway-runtime-action.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Gateway recovery now classifies additional OpenShell CLI errors as recovery-blocking. Lifecycle results expose this state. Recovery and sandbox reconciliation stop without unsafe selection, startup, or registry removal.

Changes

Gateway recovery blocking

Layer / File(s) Summary
CLI error classification
src/lib/adapters/openshell/sandbox-observer-cli.ts
The classifier is exported, recognizes missing admin privileges, and is used by sandbox lookup and inventory listing.
Lifecycle recovery guards
src/lib/gateway-runtime-action.ts
Named gateway lifecycle results include recoveryBlocked. Recovery stops before startup when blocking errors occur before or after gateway selection.
Reconciliation and recovery validation
src/lib/actions/sandbox/gateway-state.ts, src/lib/actions/sandbox/gateway-state-drift.test.ts, src/lib/gateway-runtime-action.test.ts
Blocked recovery preserves the missing-sandbox result. Tests cover initial and post-selection lifecycle probe failures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 7ea51

The change prevents gateway selection or startup after non-recoverable lifecycle probe failures. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: cv, sandl99, laitingsheng

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: blocking CLI recovery after lifecycle probe failures.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fail-closed-gateway-recovery

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

PR review advisory complete for commit 7ea519d: read the full review. Read it before deciding whether to request changes, approve, or merge this PR.

All previous runs

@rsliter rsliter left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 7ea519d. The shared CLI classifier now blocks authentication, schema, gateway-identity, and invalid-request failures before gateway selection or startup. The post-selection and missing-sandbox guards preserve the fail-closed recovery boundary. The focused 47-test set passed locally, and all 59 current checks pass.

Non-blocking: The PR Review Advisor Operations specialist found that getReconciledSandboxGatewayState can replace the blocked lifecycle class with the original transport diagnostic. A focused follow-up PR should preserve the sanitized kind and reason and add boundary tests. This affects remediation guidance, not the recovery boundary.

@prekshivyas
prekshivyas merged commit 6317ea7 into main Aug 27, 2026
76 checks passed
@prekshivyas
prekshivyas deleted the codex/fail-closed-gateway-recovery branch August 27, 2026 03:45
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression labels Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants