fix(connect): exit zero when probe succeeds without platform evidence - #9282
Conversation
`connect --probe-only` completed the gateway probe and any dashboard- forward recovery, then exited 1 on macOS because the launch-readiness evidence store requires a Linux per-user runtime authority. The permanent platform gap converted every successful probe into a failure, so a scripted health check could not distinguish a healthy sandbox from a real outage. After a successful probe and recovery, print a note that evidence is unavailable on this platform and exit 0. A publication failure on a platform that supports evidence still exits 1. Fence failures, validation failures, and unsafe epochs are unchanged. Closes #9278 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan includes up to 12 reviews per rolling hour; 11 remain after this review. 📝 WalkthroughWalkthrough
ChangesSandbox probe outcome
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR narrows probe-only failures on platforms without launch-readiness evidence so successful probes exit 0 while genuine Linux publication, validation, fence, and unsafe-epoch failures remain nonzero; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 8b5f742 in the TypeScript / code-coverage/cliThe overall coverage in commit 8b5f742 in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-9282.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: None Manual-only E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
prekshivyas
left a comment
There was a problem hiding this comment.
Reviewed exact head 8b5f742fea499015ca2e8461b93a7eb2350fb852 against #9278, the complete diff, current review threads, and the sensitive-path security rubric.
Code verdict: no blocking diff finding. The success conversion is limited to a fallback decision whose authorityUnsupported bit is produced by the OS runtime-authority boundary; Linux fence/publication, validation, changed-epoch, and unsafe-state failures still exit nonzero. The complete probe and recovery runs before the informational return. Focused validation passed locally: 4 files, 50/50 tests.
Security categories
- PASS — no secrets or credential flow changed.
- PASS — no new external input reaches execution or policy decisions.
- PASS — no authentication or authorization boundary changed.
- PASS — no dependencies or downloaded artifacts changed.
- PASS — only the proven unsupported-platform publication gap becomes informational; real validation/publication failures remain propagated.
- PASS — no cryptography or protected-data handling changed.
- PASS — Linux authority and restrictive launch-readiness defaults are preserved.
- PASS — unit and CLI coverage exercise macOS success and preserve Linux/non-platform failure behavior.
- PASS — mutation locks, epoch revalidation, complete preflight, and recovery ordering are unchanged.
I am not approving this head while the required checks gate is red. The failing job is the CLI credential-filter coverage ratchet (credential-filter.ts below its 80/80/65 per-file floor), outside this PR diff; update/re-run against the coverage fix before approval. CodeRabbit and the PR Review Advisor report no actionable diff findings.
… created (#9285) ## Summary On Ubuntu 24.04, `nemoclaw <name> connect --probe-only` exits 1 with *"no prior launch-readiness evidence can be accepted, but new launch-readiness authority could not be created"* **before running any preflight or recovery**, whenever the launch-readiness authority and receipt are both securely absent and new authority creation fails (e.g. no per-user OS runtime authority in the current session). That early exit contradicts the shipped contract in two places: - **Docs** (`docs/reference/commands.mdx`): "A securely absent runtime authority and receipt let ordinary `launch` run the complete preflight without optimization if new authority creation fails, but `connect --probe-only` still exits nonzero **because it could not publish launch-readiness evidence**" — i.e. the preflight runs and only the publication fails. - **`launchSandbox`** (`src/lib/actions/sandbox/launch.ts`): refuses only on `decision.recoveryBlocked` and runs the complete preflight for this same decision. ## Change - `connectSandbox` probe-only guard now exits early only when `recoveryBlocked` is set (a prior epoch might exist and could not be durably rotated). The securely-absent case (`fenceFailed` with `recoveryBlocked === false`) proceeds into the launch-readiness mutation gate. - Safety is preserved: the gate independently re-verifies via `checkLaunchReadinessMutationAuthority(null)` that **both stores are still missing** before admitting the null-epoch recovery; any other state returns `changed`/`unsafe` and recovery does not run. - After recovery, publication reports honestly: "Probe failed: complete probe and recovery succeeded, but final launch-readiness evidence could not be verified or published." (exit 1, per the documented Linux contract). Lifecycle callers with `requireLaunchReadinessPublication: false` now continue after recovery instead of failing before it. - Rewrote the test that pinned the early exit (`connect-flow.test.ts`) to assert recovery runs, publication is attempted, and the misleading message is gone. ## Why Refs, not Closes The issue's Expected Result asks for exit 0. The documented contract deliberately keeps `connect --probe-only` nonzero on Linux when evidence cannot be published, because a missing per-user runtime authority is a deployment signal infrastructure must repair. This PR fixes the defective part — recovery not running and the misleading refusal — and keeps the documented exit code. If maintainers want exit 0 for the securely-absent case (mirroring #9282's treatment of platform-unsupported evidence), that is a small follow-up on top of this change. With this fix, the reproduction now prints "Probe complete: … restored dashboard port forward." followed by the honest publication failure, instead of refusing to recover at all. Refs #9280 ## Testing - `npx vitest run src/lib/actions/sandbox/connect-flow.test.ts` (35 passed) - `npx vitest run test/cli/connect-recovery.test.ts` (5 passed) - sibling probe/readiness suites (93 passed), `npm run typecheck:cli`, `oxlint`, `npm run format:check` all clean Signed-off-by: Dongni Yang <dongniy@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved probe-only sandbox connection recovery when launch-readiness checks encounter fencing issues. * Recovery and sandbox setup now proceed in eligible cases, with failures reported as unavailable or unverifiable evidence. * Removed misleading authority-creation failure reporting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Dongni Yang <dongniy@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical dated changelog entry required before planning the v0.0.110 release. The entry summarizes user-facing changes merged since v0.0.109 and links each change to its published documentation route and source PR. ## Changes - Add `docs/changelog/2026-08-17.mdx` with the exact `## v0.0.110` release heading. - Cover managed local inference, endpoint validation, onboarding and recovery, explicit experimental Portable OpenClaw, messaging and policy cleanup, backup and security hardening, and release qualification. - Preserve the documentation skip list and the current supported-agent matrix; test-only refactors, dormant activation work, and Pi-only changes are intentionally excluded. ### Source-to-doc mapping - #8711 -> `docs/changelog/2026-08-17.mdx`: Add the Muse Glimmer llama.cpp profile. - #9099 -> `docs/changelog/2026-08-17.mdx`: Update the Muse Glimmer vLLM runtime. - #9319 -> `docs/changelog/2026-08-17.mdx`: Select the provider required by an explicit serving profile. - #9311 -> `docs/changelog/2026-08-17.mdx`: Report probe-image pull failures separately. - #9345 -> `docs/changelog/2026-08-17.mdx`: Reuse mirrored Windows Ollama. - #9284 -> `docs/changelog/2026-08-17.mdx`: Complete the required Ollama upgrade. - #9320 -> `docs/changelog/2026-08-17.mdx`: Reject unsafe custom endpoint URLs before mutation. - #9119 -> `docs/changelog/2026-08-17.mdx`: Reject unsupported custom endpoint URL components. - #9236 -> `docs/changelog/2026-08-17.mdx`: Require native Anthropic tool-use evidence. - #9347 -> `docs/changelog/2026-08-17.mdx`: Distinguish Gemini runtime 404 diagnostics. - #9307 -> `docs/changelog/2026-08-17.mdx`: Preserve the recorded API family when only the model drifts. - #9233 -> `docs/changelog/2026-08-17.mdx`: Fail incomplete Hermes route synchronization. - #9185 -> `docs/changelog/2026-08-17.mdx`: Serialize Model Router lifecycle work across gateways. - #9112 -> `docs/changelog/2026-08-17.mdx`: Stop Model Router after the last routed sandbox is destroyed. - #9229 -> `docs/changelog/2026-08-17.mdx`: Verify fresh sandbox execution readiness. - #9299 -> `docs/changelog/2026-08-17.mdx`: Verify a separate agent API host forward before reporting ready. - #9318 -> `docs/changelog/2026-08-17.mdx`: Honor explicit sandbox recreation. - #9325 -> `docs/changelog/2026-08-17.mdx`: Measure readiness reuse windows from collection completion. - #9352 -> `docs/changelog/2026-08-17.mdx`: Guide users away from the deprecated global start command. - #9370 -> `docs/changelog/2026-08-17.mdx`: Persist managed OpenClaw agent identity. - #9366 -> `docs/changelog/2026-08-17.mdx`: Pass messaging dependencies during reused onboarding. - #9321 -> `docs/changelog/2026-08-17.mdx`: Detect proxied connect sessions. - #9285 -> `docs/changelog/2026-08-17.mdx`: Run probe-only recovery when absent authority cannot be created. - #9282 -> `docs/changelog/2026-08-17.mdx`: Complete probe-only recovery without platform evidence. - #8920 -> `docs/changelog/2026-08-17.mdx`: Preserve legacy gateway identity. - #9198 -> `docs/changelog/2026-08-17.mdx`: Report sandbox config-read failures. - #9201 -> `docs/changelog/2026-08-17.mdx`: Remove only the exact Docker orphan on destroy. - #9176 -> `docs/changelog/2026-08-17.mdx`: Use rootless Podman for Portable lifecycle operations. - #9197 -> `docs/changelog/2026-08-17.mdx`: Preflight Portable CPU delegation. - #9289 -> `docs/changelog/2026-08-17.mdx`: Narrow Portable policy defaults. - #9270 -> `docs/changelog/2026-08-17.mdx`: Preserve Portable model intent. - #9339 -> `docs/changelog/2026-08-17.mdx`: Reconcile timed-out Portable stop state. - #9209 -> `docs/changelog/2026-08-17.mdx`: Clean receipt-owned Portable Podman resources. - #9186 -> `docs/changelog/2026-08-17.mdx`: Separate Podman activation readiness. - #9376 -> `docs/changelog/2026-08-17.mdx`: Settle Portable OpenClaw pairing before readiness. - #9296 -> `docs/changelog/2026-08-17.mdx`: Retire messaging channel presets the host no longer configures. - #9327 -> `docs/changelog/2026-08-17.mdx`: Drop retired channels from reused messaging selections. - #9306 -> `docs/changelog/2026-08-17.mdx`: Remove gateway-enforced presets without a local record. - #9248 -> `docs/changelog/2026-08-17.mdx`: Activate Google Chat pairing approval. - #9374 -> `docs/changelog/2026-08-17.mdx`: Accept schema-owned messaging plan fields. - #9317 -> `docs/changelog/2026-08-17.mdx`: Accept safe hard-linked package files during backup. - #9288 -> `docs/changelog/2026-08-17.mdx`: Remove managed CLI shims with destroyed user data. - #9239 -> `docs/changelog/2026-08-17.mdx`: Read voice credentials from fixed descriptors. - #9269 -> `docs/changelog/2026-08-17.mdx`: Accept bounded native OpenClaw device modes. - #9371 -> `docs/changelog/2026-08-17.mdx`: Isolate OpenClaw startup-guard output. - #9351 -> `docs/changelog/2026-08-17.mdx`: Restore staging Launchable validation. - #9350 -> `docs/changelog/2026-08-17.mdx`: Retry transient collaborator-permission reads. - #9353 -> `docs/changelog/2026-08-17.mdx`: Retry transient exact-artifact downloads. - #9226 -> `docs/changelog/2026-08-17.mdx`: Add bounded Brev readiness diagnostics. - #9237 -> `docs/changelog/2026-08-17.mdx`: Report same-commit E2E reliability. - #9232 -> `docs/changelog/2026-08-17.mdx`: Execute native-runtime qualification. - #9275 -> `docs/changelog/2026-08-17.mdx`: Define E2E selection and retry guidance. - #9234 -> `docs/changelog/2026-08-17.mdx`: Move documentation review after merge. - #9365 -> `docs/changelog/2026-08-17.mdx`: Mount documentation reviewer inputs before startup. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated release-entry contract. - [ ] 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: - [ ] 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; documentation-only change. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] 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 - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (7 passed) - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to one prose-only changelog page; `npm run docs` passed the repository's strict documentation gate. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — passed with 0 errors and the 2 existing Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — the SPDX header is present; dated changelog pages intentionally do not use frontmatter. --- Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.110. * Documented experimental managed llama.cpp and Portable OpenClaw profiles. * Covered inference validation, onboarding and recovery improvements, rootless lifecycle handling, messaging and policy updates, backups, credential handling, filesystem protections, and release qualification updates. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
On macOS,
nemoclaw <name> connect --probe-onlycompleted the gateway probe and any dashboard-forward recovery, then exited 1 because the launch-readiness evidence store requires a Linux per-user runtime authority (/run/user/<uid>). The permanent platform gap turned every successful probe into a failure, so a scripted health check could not tell a healthy sandbox from a real outage. After this change, a successful probe and recovery on such a platform prints a note that evidence is unavailable and exits 0.Related Issue
Closes #9278
Changes
src/lib/actions/sandbox/connect.ts: when publication reportsevidence-failedand the readiness decision carriesauthorityUnsupported(thrown only for non-Linux platforms insrc/lib/state/launch-readiness-lease.ts), printNote: launch-readiness evidence is unavailable on this platform; the next launch runs the complete preflight.and return with exit 0. A publication failure on a platform that supports evidence keepsProbe failed: ...and exit 1. Fence failures, validation failures, and unsafe-epoch exits are unchanged.src/lib/actions/sandbox/connect-flow.test.ts: the macOS-shaped case now asserts recovery completes, the note prints, and the command resolves with no exit call. The sibling cases for Linux publication failure and validation failure still assert exit 1.test/cli/connect-recovery.test.ts,test/cli/connect-terminal-agent.test.ts,test/sandbox-connect-inference/auto-pair-approval.test.ts: probe-only now expects exit 0 on every platform; the note substring still appears only on darwin.docs/reference/commands.mdx,docs/manage-sandboxes/recover-rebuild-sandboxes.mdx: state the new macOS behavior and scope the nonzero publication-failure exits to Linux.Design record
PR #8951 (#8942 launch-readiness leases) recorded the previous contract: macOS probe-only "completes recovery and probes, then returns nonzero because authoritative evidence is unavailable." This PR narrows that decision for the permanent platform gap only, per the QA expectation in #9278: the probe's product operation succeeded,
launchruns the complete preflight without evidence on these platforms, and no consumer relies on the macOS nonzero exit. Verified consumers: internal probe-only callers (start.ts,hermes-cron-restore-recovery.ts) passrequireLaunchReadinessPublication: falseand return before the changed branch; the E2E lease producer (test/e2e/live/launch-agent-turn.ts) requires exit 0; managed-cloud checks treat probe-only nonzero as failure. Linux infrastructure-producer strictness is untouched: a broken/run/user/<uid>classifies asmissing, notunsupported, and still exits nonzero.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/reference/commands.mdx,docs/manage-sandboxes/recover-rebuild-sandboxes.mdx; review verified the changed sentences againstsrc/lib/actions/sandbox/connect.tsand the controlled-word list, and its one accuracy suggestion (scopingcommands.mdx:1277to Linux) is applied in this commitVerification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run src/lib/actions/sandbox/connect-flow.test.ts35/35 passed;npx vitest run test/cli/connect-recovery.test.ts test/cli/connect-terminal-agent.test.ts6/6 passed;npx vitest run test/sandbox-connect-inference/auto-pair-approval.test.ts9/9 passed;npm run typecheck:clicleannpm run docsbuilds without warnings (doc changes only) — exits 0; the 2 remaining warnings (fern auth, theme contrast) exist onmainbefore this changePlatform coverage
The Linux CI lanes exercise the unchanged strict behavior. The changed branch is platform-independent (it keys on the
authorityUnsupportedreadiness-decision field, not onprocess.platform), and the unit test insrc/lib/actions/sandbox/connect-flow.test.tsdrives that exact decision shape end to end: recovery completes, the note prints, and no exit call is made. The three CLI-level tests assert exit 0 with the note on darwin and exit 0 without the note elsewhere.Signed-off-by: Dongni Yang dongniy@nvidia.com
Summary by CodeRabbit
Bug Fixes
connect --probe-onlynow completes recovery successfully when launch-readiness evidence is unavailable.0when core checks pass.Documentation