fix(rebuild): journal same-name sandbox replacement - #7788
Merged
Conversation
`rebuild` deleted the sandbox and only then handed off to resumed onboarding, so nothing durable proved the source, target, or completed phase. A termination in that window left a later command with no replacement intent to reconcile, and the ordinary path had already dropped the registry row. Open the canonical recreate transaction before the destroy phase, bound to the recorded gateway, the source registry fingerprint and live identity, and a target fingerprint built from validated non-secret rebuild inputs. Record `deleting` before the delete command and `deleted` only after the journal re-proves absence on that exact gateway; a probe that shows neither a live sandbox nor explicit absence stops the command with the backup, MCP state, and registry recovery data preserved. Keep the journaled source row across the delete for every rebuild, not just the MCP-bearing and baseline-exclusion cases, so a restart between deletion and replacement registration still has a source contract to reconcile. Prepared recovery therefore no longer reverses a default-sandbox transition, because none happens. Carry the journal across the session reset the recreate phase performs, rebinding only the sandbox identity, gateway authority, and transaction onto a checkpoint derived from the new session, and hand the journaled target fingerprint to the inner run so it adopts the open transaction instead of starting its own. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
📝 WalkthroughWalkthroughRebuild and non-resumed onboarding now use durable, gateway-bound sandbox replacement journals. Deletion is journaled and confirmed before recreation. Checkpoint state and target fingerprints cross onboarding boundaries. Recovery tests cover failure, restart, identity drift, registry preservation, and multi-gateway behavior. ChangesSandbox recreate journaling
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 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 |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 3e6c3d0 in the TypeScript / code-coverage/cliThe overall coverage in commit 3e6c3d0 in the Show a code coverage summary of the most impacted files.
Updated |
laitingsheng
added
area: sandbox
Contributor
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
6 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 2 optional E2E recommendations
Blockers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/actions/sandbox/rebuild-destroy-phase.ts`:
- Line 359: Wrap the recreateJournal.markDeleting() call in the rebuild/destroy
phase with failure handling that invokes reattachMcpAfterDeleteFailure for the
prepared MCP entries, relocks the shields, and exits before any OpenShell
invocation. Preserve propagation or reporting of the original durable-state
error while ensuring the still-running sandbox is restored when markDeleting
throws.
In `@src/lib/actions/sandbox/rebuild-recreate-observability.test.ts`:
- Around line 221-235: Update the test around rebuildOnboardDependencies.onboard
to retain the spy returned by vi.spyOn and restore it in a finally block
surrounding the execution and assertions. Preserve the existing observations and
expectations while ensuring restoration occurs even when an assertion fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9827a9f4-ebe9-46fe-bf23-f3e6bcd56219
📒 Files selected for processing (21)
ci/source-architecture-budget.jsonsrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-gpu-opt-out.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-prepared-recovery.test.tssrc/lib/actions/sandbox/rebuild-recreate-journal.test.tssrc/lib/actions/sandbox/rebuild-recreate-journal.tssrc/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/actions/sandbox/rebuild-recreate-phase.tssrc/lib/actions/sandbox/rebuild-shields-finally.test.tssrc/lib/onboard.tssrc/lib/onboard/machine/core-flow-phases.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-recreate-transaction.tssrc/lib/onboard/types.tstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-flow-lifecycle-cases.tstest/helpers/rebuild-flow-recovery-cases.tstest/helpers/rebuild-flow-test-support.tstest/mcp-destroy-lifecycle.test.ts
`createSandbox` re-derives its own recreation triggers, so a plain onboard run could delete and replace a live same-name sandbox with no durable replacement intent. The sandbox handler only opened the canonical journal when resuming, and without one the lifecycle runtime was an inert stub: the delete boundary, the post-delete absence proof, and the registry-row reservation all became no-ops. Open the canonical recreate transaction inside `createSandbox`, after every required confirmation and non-mutating check and before provider cleanup, the OpenShell delete, or registry removal. Bind it to the sandbox name, the resolved gateway name and port, the source registry fingerprint and generation, the hashed source OpenShell identity, and a target fingerprint built from validated non-secret create inputs. An active journal is reconciled through `planSandboxRecreateRecovery` before any mutation, so a later invocation resumes the replacement without `--resume`. Fail closed instead of guessing: a missing source registry row, a live sandbox with no stable OpenShell Id, a changed source identity, a changed target fingerprint, and a probe that reports neither a live sandbox nor explicit absence all stop the command before the next mutation. Name the gateway explicitly on the sandbox get, list, and delete commands that drive replacement, so a host running several gateways cannot answer or mutate for a sibling. Extract the shared strict-absence classifier and gateway-scoped observation so the rebuild and onboard journals prove absence the same way. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard.ts`:
- Around line 2566-2567: The recovery journal must observe the recorded gateway
rather than resolving the ambient gateway at runtime. Update the `observe`
callback passed to `openOnboardRecreateJournal` and the
`getSandboxRecreateObservation` API to propagate `probeTarget.gatewayName`
alongside `sandboxName`, ensuring resumed recovery inspects the journal-scoped
gateway; add coverage for resuming on a non-default gateway.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d483639f-382f-4a76-a303-d7f48f175fd1
📒 Files selected for processing (18)
ci/source-architecture-budget.jsonsrc/lib/actions/sandbox/gateway-state.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-recreate-journal.tssrc/lib/onboard.tssrc/lib/onboard/onboard-recreate-journal.test.tssrc/lib/onboard/onboard-recreate-journal.tssrc/lib/onboard/sandbox-recreate-probe.tssrc/lib/onboard/sandbox-reuse.tstest/onboard-custom-dockerfile.test.tstest/onboard-extra-provider-reconciliation.test.tstest/onboard-installer-restore-intent.test.tstest/onboard-messaging.test.tstest/onboard-reservation-recreate.test.tstest/onboard-sandbox-build.test.tstest/onboard-sandbox-recreation.test.tstest/onboard.test.tstest/shellquote-sandbox.test.ts
💤 Files with no reviewable changes (1)
- src/lib/actions/sandbox/gateway-state.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/rebuild-recreate-journal.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.ts
The journal opened inside createSandbox has no outer owner, so it now advances to completed and clears itself once the replacement registry row commits. Without that a later replacement met a stale active transaction and refused to start. Read the committed journal back from the session the write returned rather than through a store that may not observe it yet, keep src/lib/onboard.ts net-neutral by moving the managed-MCP refusal text into the journal module, and model post-delete absence in the onboard stubs the guard now requires. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
23 tasks
A restart that observed a registered, ready sandbox carrying the journaled target generation and identity still entered the destroy phase, so the rebuild could delete the replacement it had already proved. Surface that accepted target from the journal, retire the transaction, and end the rebuild before deletion. Reattach the prepared MCP entries when the delete boundary cannot be journaled, observe a resumed replacement on the gateway its journal records rather than the ambient one, and stop the Docker GPU rollback tests from probing DNS through a live container. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/actions/sandbox/rebuild-destroy-phase.test.ts (2)
932-971: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueSolid coverage of the journal-failure boundary.
Correctly verifies MCP reattachment, shield relock, rejection, and that the destructive OpenShell delete never fires when
markDeleting()throws — matching the try/catch inrunRebuildDestroyPhase.One small nit: Line 966 uses
not.toHaveBeenCalledWith(specificArgs), which would still pass ifrunOpenshellwere called with different arguments.not.toHaveBeenCalled()would be a strictly tighter guarantee that no OpenShell invocation occurs at all in this failure path.🔧 Tighten the negative assertion
- expect(mocks.runOpenshell).not.toHaveBeenCalledWith( - ["sandbox", "delete", "-g", "nemoclaw", "alpha"], - expect.anything(), - ); + expect(mocks.runOpenshell).not.toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/rebuild-destroy-phase.test.ts` around lines 932 - 971, In the test “reattaches MCP providers when the delete boundary cannot be journaled,” replace the argument-specific negative assertion on mocks.runOpenshell with an assertion that it was never called at all, preserving the existing failure-path coverage.
76-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
stubRecreateJournal()into a shared test helper.The identical helper is duplicated verbatim in
test/mcp-destroy-lifecycle.test.ts; this PR had to update both copies with the same three new fields. Centralizing it (e.g., intest/helpers/) would avoid future drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/rebuild-destroy-phase.test.ts` around lines 76 - 86, Extract the duplicated stubRecreateJournal helper into a shared test utility, then import and reuse it in both rebuild-destroy-phase.test.ts and mcp-destroy-lifecycle.test.ts. Preserve the current returned RebuildRecreateJournal shape, including all three mock methods and target fields, while removing the local duplicate definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/actions/sandbox/rebuild-destroy-phase.test.ts`:
- Around line 932-971: In the test “reattaches MCP providers when the delete
boundary cannot be journaled,” replace the argument-specific negative assertion
on mocks.runOpenshell with an assertion that it was never called at all,
preserving the existing failure-path coverage.
- Around line 76-86: Extract the duplicated stubRecreateJournal helper into a
shared test utility, then import and reuse it in both
rebuild-destroy-phase.test.ts and mcp-destroy-lifecycle.test.ts. Preserve the
current returned RebuildRecreateJournal shape, including all three mock methods
and target fields, while removing the local duplicate definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 37030ec8-d631-4fa5-8192-06c59eca3332
📒 Files selected for processing (13)
src/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-destroy-phase.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-recreate-journal.test.tssrc/lib/actions/sandbox/rebuild-recreate-journal.tssrc/lib/actions/sandbox/rebuild-recreate-observability.test.tssrc/lib/onboard.tssrc/lib/onboard/docker-gpu-patch-rollback.test.tssrc/lib/onboard/sandbox-reuse.test.tssrc/lib/onboard/sandbox-reuse.tstest/helpers/rebuild-flow-recovery-cases.tstest/mcp-destroy-lifecycle.test.tstest/onboard-messaging.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/lib/actions/sandbox/rebuild-pipeline.ts
- src/lib/actions/sandbox/rebuild-recreate-observability.test.ts
- test/mcp-destroy-lifecycle.test.ts
- src/lib/onboard/sandbox-reuse.ts
- test/onboard-messaging.test.ts
- src/lib/actions/sandbox/rebuild-recreate-journal.test.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.ts
- src/lib/onboard.ts
Rebuild the proven-replacement helper without a guard branch so changed test files add no conditionals, and tighten the shell-quote fan-in budget to the count the current tree reports. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/rebuild-recreate-journal.test.ts (1)
346-353: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConstrain the registry mock to the expected lookup.
mockReturnValueignores everygetSandboxargument, so a regression that queries the wrong sandbox name or gateway still receives thealphafixture and may pass. Use a narrowmockImplementationthat validates the requested identity and gateway. As per path instructions, broad mocks that bypass the behavior under test should be flagged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/rebuild-recreate-journal.test.ts` around lines 346 - 353, Replace the broad mockReturnValue on registry.getSandbox with a mockImplementation that checks the requested sandbox identity and gateway against the expected alpha and gateway values, returning the fixture only for that lookup. Preserve the existing fixture fields and make unexpected arguments fail rather than silently returning alpha.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/lib/actions/sandbox/rebuild-recreate-journal.test.ts`:
- Around line 346-353: Replace the broad mockReturnValue on registry.getSandbox
with a mockImplementation that checks the requested sandbox identity and gateway
against the expected alpha and gateway values, returning the fixture only for
that lookup. Preserve the existing fixture fields and make unexpected arguments
fail rather than silently returning alpha.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cc1c11eb-b89e-4928-8653-c21aeb369ac0
📒 Files selected for processing (2)
ci/source-architecture-budget.jsonsrc/lib/actions/sandbox/rebuild-recreate-journal.test.ts
Drive the rebuild pipeline from persisted creating, created, registry_committing, and completed journals. Assert a restart accepts only a ready replacement whose identity and generation match the journal, and that a foreign same-name sandbox stops the rebuild before any destructive mutation. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
23 tasks
…journal Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Collaborator
|
Babysitting status for exact head e8e8a28 (plain comment; no Changes Requested review from me): the current deterministic gate reports failed E2E / PR Gate and coordination, and the branch is based on 3aea38c rather than current main da1b103. Maintainer edits are disabled. This large state/rebuild patch needs a refreshed quiet head and clean exact-head protected evidence before approval; I will re-gate it when available. |
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…dy proved Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Collaborator
|
Correction to my prior handoff: conflict-free base refreshes are explicitly waived. Please do not merge main solely for base currency; preserving exact-head evidence is preferred unless GitHub reports a real conflict or reviewed behavior requires a change. The substantive blocker or missing evidence described in the earlier handoff remains, but base age by itself is not a blocker. This is a plain coordination comment, not Changes Requested. |
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
22 tasks
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7788.docs.buildwithfern.com/nemoclaw |
Collaborator
|
Security review for exact head This is an agent review by Codex Desktop, not independent human sensitive-path approval. Result: PASS — no actionable security finding in the complete 48-file diff.
Product scope is approved independently of |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/helpers/rebuild-flow-harness.ts (1)
338-349: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract one shared
resolveGatewayTeardownAuthoritytest fixture. All three sites hardcode the identical fixedGatewayOwnerresult (mode: "nemoclaw-managed",source: "standalone", nullendpoint/stateDir/supervisor, emptyrequiredCapabilities). The shared root cause is the lack of one canonical test fixture for this authority contract, so a future field addition or rename needs three synchronized edits.
test/helpers/rebuild-flow-harness.ts#L338-L349: replace the inlinevi.spyOn(...).mockImplementation(...)object with a call to a new shared fixture helper (for example exported from a common test-helpers module) that returns this standalone-authority shape.src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts#L95-L106: use the same shared fixture helper instead of repeating the literal.test/helpers/onboard-script-mocks.cjs#L162-L176: keepmockStandaloneGatewayTeardownAuthorityas the CommonJS entry point, but have it source its returned shape from the same canonical fixture definition (or vice versa) so all three stay in sync.♻️ Example shared fixture
// test/helpers/gateway-teardown-authority-fixture.ts export function standaloneGatewayTeardownAuthority({ gatewayName, gatewayPort, }: { gatewayName: string; gatewayPort: number; }) { return { gatewayName, gatewayPort, mode: "nemoclaw-managed" as const, source: "standalone" as const, endpoint: null, stateDir: null, supervisor: null, requiredCapabilities: [], }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/helpers/rebuild-flow-harness.ts` around lines 338 - 349, Extract one canonical standalone gateway teardown authority fixture and reuse it at all three sites: update test/helpers/rebuild-flow-harness.ts lines 338-349 and src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts lines 95-106 to call the shared helper, and update test/helpers/onboard-script-mocks.cjs lines 162-176 so mockStandaloneGatewayTeardownAuthority sources the same definition while remaining the CommonJS entry point. Preserve the existing gateway name/port inputs and authority fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/helpers/rebuild-flow-harness.ts`:
- Around line 338-349: Extract one canonical standalone gateway teardown
authority fixture and reuse it at all three sites: update
test/helpers/rebuild-flow-harness.ts lines 338-349 and
src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts lines 95-106 to call the
shared helper, and update test/helpers/onboard-script-mocks.cjs lines 162-176 so
mockStandaloneGatewayTeardownAuthority sources the same definition while
remaining the CommonJS entry point. Preserve the existing gateway name/port
inputs and authority fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 38d70202-5657-4c81-b0e7-c7b2321e0059
📒 Files selected for processing (14)
docs/manage-sandboxes/recover-rebuild-sandboxes.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/rebuild-destroy-phase.test.tssrc/lib/actions/sandbox/rebuild-recreate-phase.tssrc/lib/actions/sandbox/rebuild-recreate-reasoning.test.tssrc/lib/actions/sandbox/rebuild-resume-snapshot.test.tstest/helpers/onboard-script-mocks.cjstest/helpers/rebuild-flow-harness.tstest/helpers/rebuild-flow-lifecycle-cases.tstest/helpers/rebuild-flow-test-harness.tstest/onboard-installer-restore-intent.test.tstest/onboard-reservation-recreate.test.tstest/onboard-sandbox-build.test.tstest/onboard-sandbox-recreation.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- test/helpers/rebuild-flow-lifecycle-cases.ts
- src/lib/actions/sandbox/rebuild-recreate-phase.ts
- test/onboard-sandbox-build.test.ts
- test/onboard-reservation-recreate.test.ts
- test/helpers/rebuild-flow-test-harness.ts
- test/onboard-installer-restore-intent.test.ts
- test/onboard-sandbox-recreation.test.ts
- src/lib/actions/sandbox/rebuild-destroy-phase.test.ts
…h-09810f1f # Conflicts: # ci/source-architecture-budget.json
Collaborator
|
Exact-head maintainer preparation receipt
Security specialist review: PASS across all nine repository categories, with no actionable findings. The exact-head security-focused run passed 114/114 tests across four files. The review covered the complete 48-file diff, including exact-gateway authority, strict presence/absence classification, source identity and target fingerprint binding, monotonic journal recovery, delete-edge revalidation, redacted errors, and adversarial fail-closed tests. This was an independent Codex Desktop agent review; it is not human or sensitive-path human approval. Product scope approval is recorded independently of GitHub Automated-review dispositions carried onto this exact tree:
All previously published review threads were resolved. Fresh automated advisors and exact-pair CI/E2E remain merge gates for this head/base pair. |
senthilr-nv
approved these changes
Jul 31, 2026
Collaborator
|
Exact-head maintainer preparation receipt
Security specialist review: PASS across all nine repository categories, with no actionable findings. The exact-head security-focused run passed 114/114 tests across four files. The complete exact-head focused run passed 405/405 tests across 27 files. The review covered the complete 48-file diff, including exact-gateway authority, strict presence/absence classification, source identity and target fingerprint binding, monotonic journal recovery, delete-edge revalidation, redacted errors, and adversarial fail-closed tests. This was an independent Codex Desktop agent review; it is not human or sensitive-path human approval. Product scope approval is recorded independently of GitHub Automated-review dispositions carried onto this exact tree:
All previously published review threads were resolved. Fresh automated advisors and exact-pair CI/E2E remain merge gates for this head/base pair. |
23 tasks
senthilr-nv
added a commit
that referenced
this pull request
Aug 1, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated changelog entry for `v0.0.100` so the maintainer release plan can verify the pre-tag documentation prerequisite. The entry summarizes the user-facing changes merged since `v0.0.99` and links to the relevant guides. ## Changes - Add `docs/changelog/2026-07-31.mdx` with the exact `## v0.0.100` heading. - Cover restored OpenClaw pairing, transactional replacement, Deep Agents Code, onboarding recovery, lifecycle cleanup, Hermes builds, host provenance, documentation, and trusted E2E evidence. - Distinguish active Docker and Kubernetes runtime-bundle enforcement from the still-inactive managed shared-state transaction foundation. ## Source Coverage The release entry maps the doc-impacting merged PRs in the `v0.0.99..main` release range to `docs/changelog/2026-07-31.mdx`: #8021, #8024, #7973, #8028, #7947, #7788, #7884, #8023, #7969, #8020, #7989, #8000, #7907, #7942, #7567, #8013, #7955, #8017, #8014, #8015, #7629, #7644, #7821, #7971, and #7991. PR #7974 was reviewed after the final rebase and excluded because it changes internal maintainer-skill attribution policy and tests only; it does not change a user-facing product or documentation surface. ## 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: the changelog contract test validates the dated entry, version heading, SPDX form, and route constraints. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs 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: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-31.mdx`; exact-head review passed for `6093f44f`; writing rules and documentation style reviewed; `npx vitest run test/changelog-docs.test.ts` passed 6/6; `npm run docs` passed with zero Fern errors and two generic Fern upgrade notices. - Agent: Codex Desktop <!-- docs-review-head-sha: 6093f44 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; no DGX Station host script changed. - 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 — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6/6 at `6093f44f`. - [ ] 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 a dated prose-only release entry. - [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) — validation passed with zero errors; Fern emitted two generic upgrade notices. - [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 changelog entry has the required parser-safe MDX SPDX header; dated changelog entries intentionally do not use page frontmatter. --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.100. * Documented improvements to restore pairing, sandbox replacement, onboarding recovery, lifecycle cleanup, runtime handling, build support, host readiness, and end-to-end validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
23 tasks
senthilr-nv
added a commit
that referenced
this pull request
Aug 1, 2026
## Summary Rebuild now removes the obsolete owned sandbox image only after the same-name replacement is registered and its exact journaled generation and live identity are proven. Interrupted resume preserves shared-image ownership state, so cleanup retains shared images. This restores the documented cleanup behavior without weakening the recreate journal added by #7788. ## Changes - Capture the source registry entry before journaled same-name replacement. - Persist the minimal secret-free source-workload cleanup receipt so interrupted replacements resume cleanup against the original image. - Preserve the source workload's sharing state through checkpoint validation and resume cleanup. - Reload the same recreate transaction after replacement creation and bind cleanup to its exact recorded live identity. - Record post-create transaction and cleanup failures as repair failures while preserving the original error. - Remove only a provider-owned obsolete image after the replacement generation and live identity are proven. - Retain shared images, reused images, foreign generations, and workloads whose ownership cannot be proven. - Use the active launcher name in cleanup guidance, including `nemohermes gc`. - Add lifecycle-boundary regressions for removal, reuse, sharing, missing identity, foreign generation, and journal ordering. Product scope: this restores the existing supported and documented `rebuild` image-cleanup contract; it does not add a new integration, configuration, or product surface. ## Type of Change - [x] 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 - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Existing `docs/reference/commands.mdx` already documents automatic rebuild image cleanup and the `gc` recovery path; this change restores that contract. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Final exact-head nine-category review PASS: #8039 (comment) - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Existing `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx:172-195` already requires matching live identity and registry generation and fails closed on identity mismatch; `docs/reference/commands.mdx:284-287` routes interrupted replacements to that contract; exact-head writer review found no command, configuration, output, or workflow change; focused tests passed 98/98, and `git diff --check` passed. - Agent: Codex Desktop <!-- docs-review-head-sha: 7ab0927 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - 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 — command/result or justification: Focused tests passed 98/98 at the exact final tree; regressions cover shared-state loss, same-generation replacement identity mismatch, and post-create cleanup failure recording; independent exact-head writer review 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 a broad runtime or harness change; `npm run validate:pr` passed repository checks, CLI type-checking, commitlint, secret scanning, and all applicable hooks. - [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) - [ ] 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) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
22 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
rebuildand non-resumedonboardpreviously deleted a same-name sandbox before durably recording its replacement. They now open the canonical recreation transaction before the first destructive mutation, bind it to the exact source, target, and gateway, and recover or fail closed after interruption instead of guessing.Related Issue
Fixes #7734
Fixes #7735
Product Scope Approval
Product scope is approved independently of GitHub
mergeStateStatus: accepted maintainer-authored issues #7734 and #7735 define the same-name replacement recovery contract and ownership. Maintainer direction in #6492 also establishes lifecycle recovery as supported NemoClaw behavior. This PR does not add a new third-party integration, custom image, or solution recipe.Changes
CheckpointSandboxRecreateTransactionbefore rebuild or onboard replacement mutates sandbox state.deletingbefore deletion anddeletedonly after exact-gateway absence is proved; ambiguous observations fail closed and preserve recovery evidence.docs/manage-sandboxes/recover-rebuild-sandboxes.mdxanddocs/reference/commands.mdx.Type of Change
Quality Gates
3e6c3d0391f9fd7e58ce9775871807b9b3c2ff8eis PASS; this is agent evidence, not human review.Documentation Writer Review
docs-updateddocs/manage-sandboxes/recover-rebuild-sandboxes.mdx;docs/reference/commands.mdx. Exact head3e6c3d0391f9fd7e58ce9775871807b9b3c2ff8e, base299050fc0563db0cd3a803298a6fb986dcfbb745; PASS with no blocking writing findings.npm run docscompleted with 0 errors and two unchanged advisory warnings.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
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 unavailable3e6c3d0391f9fd7e58ce9775871807b9b3c2ff8e; 405 focused CLI and integration tests passed across 27 files;npm run typecheck:clipassed; source architecture passed with 1,425 files, 4,201 edges, one allowed cycle, max fan-in 103, and max fan-out 225.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: localnpm run checkdid not provide valid evidence on this unsupported host (Node 23.7.0 instead of the repository's Node 22.19+, missing Pythonyaml, and missing the official OpenShell Homebrew formula caused unrelated failures and timeouts). Fresh supported GitHub CI for this exact head/base pair is required before merge.npm run docsbuilds without warnings (doc changes only) — 0 errors; two unchanged advisory warnings remain (Fern redirect-auth availability and an existing light-mode accent-contrast warning).Signed-off-by: Tinson Lai tinsonl@nvidia.com