fix(onboard): reconcile managed bootstrap after restart - #8043
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the exact reviewed tree while moving the stacked base to merged PR3.3. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the exact reviewed tree while moving the stacked base to restacked PR3.4a. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the exact reviewed tree while moving the stacked base to restacked PR3.4b. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Carry the reviewed PR3.4b slice unchanged onto the CodeRabbit feedback fix for PR3.4a. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Carry the reviewed PR3.5 slice unchanged onto the CodeRabbit feedback restack through PR3.4b. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Carry the reviewed PR3.4b slice unchanged onto the serialized PR3.4a transaction contract. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Carry the reviewed PR3.5 slice unchanged onto the serialized PR3.4a transaction contract. The review patch remains unchanged through PR3.4b. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Restack PR3.4b without changing its review patch. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Restack PR3.5 without changing its review patch. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the current public PR source head and record the exact post-#8040 test-contract reconstruction as explicit ancestry. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Persist managed-bootstrap mutation authority and image-owned shared-state commit receipts so restart does not erase the facts needed to prove, resume, or clean up one exact attempt. This slice remains inert: it does not advertise or activate buildless support. ## Related Issue Part of #7744. ## Changes - Replace process-local Docker transaction and terminal state with private, versioned, atomically written journal records and exact commit/finalization receipts. - Record provider, sandbox, plan, profile, immutable image, original and replacement runtime identities, rollback target, preparation receipt, and completion receipt. - Keep durable journals identity-addressable in this slice; unfinished-record enumeration and production phase reconciliation are introduced together by the following recovery slice. - Atomically convert image-owned shared-state backups into an identity-bound durable commit receipt, reject post-commit rollback, and clear the receipt only after external cleanup is proven. - Preserve legacy null-identity commit and rollback CLI forms while adding explicit identity-bound status, commit, rollback, and receipt-cleanup forms. - Cover OpenClaw, Hermes, and DCode persistence, restart reconstruction, interrupted receipt compaction, exact cleanup, and ownership-reuse defenses. - Cover recreated-adapter rollback after committed terminal finalization and reject same-timestamp phase mutations, closing the two valid Nemotron coverage warnings. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: Exact-tree reconstruction review verified durable identity authority, image-owned receipt semantics, provider neutrality, production dormancy, and the two advisor-requested regression cases. Exact-head public advisors remain part of the qualification wave. - [x] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: `reviewed-npm-audit` is being handled on the separate #8156/#8157 security stream and is explicitly waived as a blocker for this stack. Its consequential same-repository `E2E / PR Gate` blockage is waived with it; the manual gate override is fork-only and cannot dispatch for this maintainer branch. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: The final reconstruction preserves the reviewed `src/lib/onboard/managed-bootstrap/README.md` semantics from prior exact head `a3571b2fb794a6bf082f05146fb24096d448548e`: provider-neutral/MXC dormancy, pre-/post-cutover distinctions, durable identity-addressed journals, finalization records, the image-owned commit receipt, and the following recovery boundary. The restack changes no documentation behavior. - Agent: Codex Desktop <!-- docs-review-head-sha: 3aa4ba6 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - Merged parent: #8040 produced main commit `35f59704af0b917e1453ea2ebdc7ce817a36fd1c`. - Exact public head: `3aa4ba69335c37ec36721cec48f7b58cea9b8e6d`. - Review scope: 11 paths, `+1,743/-120`. - Stable patch ID: `76f455f1e0d0a6d9977b5fa65dd61357742bff09`. - Binary diff SHA-256: `27b3d940a6af0a1b610eb5b676d0decc763f7dc9700c5d32853475c0bd7875a4`. - Advisor test anchor: signed+DCO commit `0623bd054e12fa149635c60efa38a3281efc2050`, direct child of the prior public head. - Clean reconstruction: signed+DCO commit `79fd30710d194b619fe579d3daa201fec10a38e5`, direct child of merged #8040 main with exact tree `7fd1887c4ec0051ae5250a66252ddf17ca7b9e66`. - Append-only graft: signed+DCO commit `3aa4ba69335c37ec36721cec48f7b58cea9b8e6d`, first parent the preserved public head `a3571b2fb794a6bf082f05146fb24096d448548e`, second parent the clean reconstruction, and the identical clean tree. No remote history was rewritten. - Durable refs: `backup/podman-stack/pr8041-tests-0623bd05`, `backup/podman-stack/pr8041-clean-79fd3071`, `backup/podman-stack/pr8041-graft-3aa4ba69`, and `hold/podman-stack/pr8043-base-3aa4ba693`. - Focused validation passed 7 files and 148 tests, including both advisor-requested cases. CLI build, source and CLI typechecks, and Biome passed. - Later main movement `d756d15d867cdf91a90cad81971a8a29febe3002` is the separate audit-transition PR #8157. Its four changed paths do not overlap this 11-path slice, so no restack or retest is warranted solely for that movement. - Every new commit is SSH-signed and carries Aaron's DCO trailer. - Production provider bootstrap remains unsupported; no user-visible buildless or Podman activation is added. ## Stack - Base: merged PR3.11 #8040. - This slice: PR3.12A #8041, branch `feat/managed-bootstrap-durable-transactions`, exact head `3aa4ba69335c37ec36721cec48f7b58cea9b8e6d`. - Next: PR3.12B #8043; its source remains protected and its new base is `hold/podman-stack/pr8043-base-3aa4ba693`. - Buildless and Podman support remain disabled until the complete all-agent, multiarch, GPU/local-inference, recovery, installer, and protected-E2E activation gates in #7744 pass. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Exact-head CodeRabbit follow-up on
Focused tests, CLI typecheck, Biome, commit hooks, and push hooks pass locally; exact-head CI/advisors are now running. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/lib/onboard/managed-bootstrap/docker-journal.test.ts (1)
520-530: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant try/catch or assert on a captured error.
Line 522 already asserts the throw. The
try/catchat lines 525-529 repeats the call. If the parser stopped throwing, thecatchbody would never run, so thenot.toBeInstanceOfexpectation would silently pass. Capture the error once and assert both properties on it.♻️ Proposed single-capture assertion
it("does not classify a malformed legacy journal as upgradeable authority", () => { const malformed = { ...legacyJournalV2(), agent: "hermes" }; - expect(() => parseDockerManagedBootstrapJournal(`${JSON.stringify(malformed)}\n`)).toThrow( - "legacy journal schema is invalid", - ); - try { - parseDockerManagedBootstrapJournal(`${JSON.stringify(malformed)}\n`); - } catch (error) { - expect(error).not.toBeInstanceOf(DockerManagedBootstrapLegacyRecordRequiresAgentError); - } + let failure: unknown; + try { + parseDockerManagedBootstrapJournal(`${JSON.stringify(malformed)}\n`); + } catch (error) { + failure = error; + } + expect(failure).toBeInstanceOf(Error); + expect(failure).not.toBeInstanceOf(DockerManagedBootstrapLegacyRecordRequiresAgentError); + expect((failure as Error).message).toContain("legacy journal schema is invalid"); });🤖 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/onboard/managed-bootstrap/docker-journal.test.ts` around lines 520 - 530, Update the test “does not classify a malformed legacy journal as upgradeable authority” to invoke parseDockerManagedBootstrapJournal once, capture the thrown error, and assert both its message and that it is not a DockerManagedBootstrapLegacyRecordRequiresAgentError. Remove the redundant try/catch and repeated parser call while preserving the malformed journal setup.Source: Path instructions
src/lib/onboard/managed-bootstrap/docker.test.ts (2)
150-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
expectEventBeforehere too.Lines 150-154 repeat the presence-and-index pattern that the new helper encapsulates. Replace them with one helper call for consistency inside the same test.
♻️ Proposed fix
- expect(fake.events).toContain("journal:shared-state-committed"); - expect(fake.events).toContain(`rm:${OLD_ID}`); - expect(fake.events.indexOf("journal:shared-state-committed")).toBeLessThan( - fake.events.indexOf(`rm:${OLD_ID}`), - ); + expectEventBefore(fake.events, "journal:shared-state-committed", `rm:${OLD_ID}`);🤖 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/onboard/managed-bootstrap/docker.test.ts` around lines 150 - 154, Replace the repeated event-presence and ordering assertions in the affected test with a single expectEventBefore call, passing "journal:shared-state-committed" and the OLD_ID removal event, while preserving the existing ordering requirement.
27-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
expectEventBeforehelper in two test files. Both files define the identical event-ordering helper. Both already import shared helpers fromsrc/lib/onboard/managed-bootstrap/docker-test-fixture.ts, so the helper belongs there and should be exported once.
src/lib/onboard/managed-bootstrap/docker.test.ts#L27-L31: delete the local definition and importexpectEventBeforefrom./docker-test-fixture.src/lib/onboard/managed-bootstrap/docker-recovery.test.ts#L40-L44: delete the local definition and import the same shared helper.🤖 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/onboard/managed-bootstrap/docker.test.ts` around lines 27 - 31, The event-ordering helper is duplicated across both test files. In src/lib/onboard/managed-bootstrap/docker.test.ts lines 27-31, remove the local expectEventBefore definition and import it from ./docker-test-fixture; make the same removal and import change in src/lib/onboard/managed-bootstrap/docker-recovery.test.ts lines 40-44, exporting the single shared helper from docker-test-fixture.src/lib/onboard/managed-bootstrap/adapter.ts (1)
584-593: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the ownership predicate once.
Lines 585 and 588 repeat the same condition. A future change to the ownership rule must be applied in both places. Extract one predicate and derive both the blocking list and the warning list from it.
♻️ Proposed extraction
- const blocking = report.failures.filter( - (failure) => failure.sandbox === null || failure.sandbox.sandboxName === sandboxName, - ); - for (const failure of report.failures) { - if (failure.sandbox === null || failure.sandbox.sandboxName === sandboxName) continue; + const ownsRequestedName = (failure: ManagedBootstrapRecoveryFailure) => + failure.sandbox === null || failure.sandbox.sandboxName === sandboxName; + const blocking = report.failures.filter(ownsRequestedName); + for (const failure of report.failures) { + if (ownsRequestedName(failure)) continue; warn( `Managed bootstrap recovery retained unrelated sandbox '${failure.sandbox.sandboxName}' ` + `(${failure.bootstrapIdentity}, ${failure.code}).`, ); }🤖 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/onboard/managed-bootstrap/adapter.ts` around lines 584 - 593, In the managed bootstrap recovery logic, extract the repeated sandbox ownership condition into a single predicate near the blocking calculation. Reuse that predicate both when filtering blocking failures and when selecting unrelated failures for warnings, preserving the current ownership behavior.src/lib/onboard/managed-bootstrap/docker-test-fixture.ts (2)
548-555: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unnecessary cast on line 549.
originalis declared asDockerContainerInspect | null. Assigningnullneeds no cast. Theas unknown as DockerContainerInspectcast suppresses type checking and can hide a real type error after a future refactor.♻️ Proposed fix
removeOriginalExternally() { - original = null as unknown as DockerContainerInspect; + original = null; events.push(`external-rm:${OLD_ID}`); },🤖 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/onboard/managed-bootstrap/docker-test-fixture.ts` around lines 548 - 555, Remove the unnecessary `as unknown as DockerContainerInspect` cast in `removeOriginalExternally()` and assign `null` directly to `original`, preserving the existing event behavior.
312-323: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe fake store ignores the finalization context, so provider-side context wiring is untested.
recordFinalizationandloadFinalizationaccept an optionalDockerManagedBootstrapFinalizationContextin the real store, and the real store fails when the record contradicts the context. This fake drops the parameter.docker.tspasses a context fromfinalizationContext(handle)and from the recovered journal. A wrong context there would still pass every Docker fixture test.Validate the supplied context in the fake so the durable contract is exercised at the boundary.
♻️ Proposed context assertion in the fake store
- recordFinalization(value) { + recordFinalization(value, context) { + if (context) assertFinalizationContext(value, context); if ( finalization && serializeDockerManagedBootstrapFinalizationRecord(finalization) !== serializeDockerManagedBootstrapFinalizationRecord(value) ) { throw new Error("finalization changed"); } finalization = structuredClone(value); events.push(`finalization:${value.phase}`); }, - loadFinalization: () => (finalization ? structuredClone(finalization) : null), + loadFinalization: (_identity, context) => { + if (!finalization) return null; + if (context) assertFinalizationContext(finalization, context); + return structuredClone(finalization); + },Add the helper next to
failFixture:function assertFinalizationContext( record: DockerManagedBootstrapFinalizationRecord, context: DockerManagedBootstrapFinalizationContext, ): void { if ( record.bootstrapIdentity !== context.bootstrapIdentity || record.providerId !== context.providerId || record.agent !== context.agent || record.sandbox.sandboxName !== context.sandbox.sandboxName || record.sandbox.sandboxId !== context.sandbox.sandboxId || record.sandbox.driverId !== context.sandbox.driverId || record.planFingerprint !== context.planFingerprint || record.profileFingerprint !== context.profileFingerprint || record.imageReference !== context.imageReference ) { failFixture("finalization record does not match supplied durable context"); } }🤖 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/onboard/managed-bootstrap/docker-test-fixture.ts` around lines 312 - 323, Update the fake store’s recordFinalization and loadFinalization methods to accept the optional DockerManagedBootstrapFinalizationContext and validate it against the finalization record before storing or returning it. Add an assertFinalizationContext helper near failFixture that compares all durable identity, provider, agent, sandbox, plan/profile fingerprint, and image-reference fields, calling failFixture on mismatch; preserve existing cloning and change-detection behavior.
🤖 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/managed-bootstrap/docker-journal.ts`:
- Around line 421-428: Update normalizeDockerManagedBootstrapJournal and the
journal store.load path so schema-1 and schema-2 journals are upgraded in place
using the durable context, matching the finalization reader’s legacy-record
handling, before returning the normalized journal. Preserve the
DockerManagedBootstrapLegacyRecordRequiresAgentError path only when the legacy
journal cannot be safely upgraded, so stale journals no longer cause a
permanently blocking null sandbox.
In `@src/lib/onboard/sandbox-gpu-create-run-attempt.ts`:
- Around line 147-152: Update the public runSandboxGpuCreateFlow boundary to
catch ManagedBootstrapRecoveryBlockedError from managed lifecycle recovery, then
print its actionable recovery guidance instead of allowing the typed error to
propagate. Preserve existing recovery handling and behavior for other errors.
---
Nitpick comments:
In `@src/lib/onboard/managed-bootstrap/adapter.ts`:
- Around line 584-593: In the managed bootstrap recovery logic, extract the
repeated sandbox ownership condition into a single predicate near the blocking
calculation. Reuse that predicate both when filtering blocking failures and when
selecting unrelated failures for warnings, preserving the current ownership
behavior.
In `@src/lib/onboard/managed-bootstrap/docker-journal.test.ts`:
- Around line 520-530: Update the test “does not classify a malformed legacy
journal as upgradeable authority” to invoke parseDockerManagedBootstrapJournal
once, capture the thrown error, and assert both its message and that it is not a
DockerManagedBootstrapLegacyRecordRequiresAgentError. Remove the redundant
try/catch and repeated parser call while preserving the malformed journal setup.
In `@src/lib/onboard/managed-bootstrap/docker-test-fixture.ts`:
- Around line 548-555: Remove the unnecessary `as unknown as
DockerContainerInspect` cast in `removeOriginalExternally()` and assign `null`
directly to `original`, preserving the existing event behavior.
- Around line 312-323: Update the fake store’s recordFinalization and
loadFinalization methods to accept the optional
DockerManagedBootstrapFinalizationContext and validate it against the
finalization record before storing or returning it. Add an
assertFinalizationContext helper near failFixture that compares all durable
identity, provider, agent, sandbox, plan/profile fingerprint, and
image-reference fields, calling failFixture on mismatch; preserve existing
cloning and change-detection behavior.
In `@src/lib/onboard/managed-bootstrap/docker.test.ts`:
- Around line 150-154: Replace the repeated event-presence and ordering
assertions in the affected test with a single expectEventBefore call, passing
"journal:shared-state-committed" and the OLD_ID removal event, while preserving
the existing ordering requirement.
- Around line 27-31: The event-ordering helper is duplicated across both test
files. In src/lib/onboard/managed-bootstrap/docker.test.ts lines 27-31, remove
the local expectEventBefore definition and import it from ./docker-test-fixture;
make the same removal and import change in
src/lib/onboard/managed-bootstrap/docker-recovery.test.ts lines 40-44, exporting
the single shared helper from docker-test-fixture.
🪄 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: 1f6d6547-2611-4bae-bd91-2bac2bddef4a
📒 Files selected for processing (15)
src/lib/onboard/managed-bootstrap/README.mdsrc/lib/onboard/managed-bootstrap/adapter.test.tssrc/lib/onboard/managed-bootstrap/adapter.tssrc/lib/onboard/managed-bootstrap/docker-journal.test.tssrc/lib/onboard/managed-bootstrap/docker-journal.tssrc/lib/onboard/managed-bootstrap/docker-recovery.test.tssrc/lib/onboard/managed-bootstrap/docker-runtime.tssrc/lib/onboard/managed-bootstrap/docker-test-fixture.tssrc/lib/onboard/managed-bootstrap/docker.test.tssrc/lib/onboard/managed-bootstrap/docker.tssrc/lib/onboard/managed-bootstrap/index.tssrc/lib/onboard/managed-bootstrap/runtime-create.tssrc/lib/onboard/runtime-provider/runtime-provider-contract.test.tssrc/lib/onboard/sandbox-gpu-create-flow.test.tssrc/lib/onboard/sandbox-gpu-create-run-attempt.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/lib/onboard/sandbox-gpu-create-flow.test.ts
- src/lib/onboard/managed-bootstrap/index.ts
- src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts
- src/lib/onboard/managed-bootstrap/README.md
- src/lib/onboard/managed-bootstrap/docker-runtime.ts
- src/lib/onboard/managed-bootstrap/docker.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard/sandbox-gpu-create-flow.ts (1)
225-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert recovery-detail redaction at the public boundary.
The existing case covers
ManagedBootstrapRecoveryBlockedErrorand proves that sandbox creation stops. Use a token-bearing recovery detail and assert thaterrorOutput()contains the redacted value and not the original value.🤖 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/onboard/sandbox-gpu-create-flow.ts` around lines 225 - 231, Update the test around the sandbox creation flow’s catch handling to use a recovery detail containing a sensitive token, then assert that errorOutput() includes the redacted value and excludes the original token when ManagedBootstrapRecoveryBlockedError reaches the public boundary. Preserve the existing assertion that sandbox creation stops.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.
Inline comments:
In `@src/lib/onboard/managed-bootstrap/docker-recovery.test.ts`:
- Around line 107-109: Update the legacy recovery test around the existing
fake.events and fake.journal assertions to verify that fake.events is empty
after recovery, ensuring no durable finalization record or other mutation is
written. Retain the fake.journal null assertion and use the existing event
fixture rather than adding new state.
---
Nitpick comments:
In `@src/lib/onboard/sandbox-gpu-create-flow.ts`:
- Around line 225-231: Update the test around the sandbox creation flow’s catch
handling to use a recovery detail containing a sensitive token, then assert that
errorOutput() includes the redacted value and excludes the original token when
ManagedBootstrapRecoveryBlockedError reaches the public boundary. Preserve the
existing assertion that sandbox creation stops.
🪄 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: 6116e2a6-1ff5-457e-8c1c-df52a3496d62
📒 Files selected for processing (7)
src/lib/onboard/managed-bootstrap/README.mdsrc/lib/onboard/managed-bootstrap/docker-journal.test.tssrc/lib/onboard/managed-bootstrap/docker-journal.tssrc/lib/onboard/managed-bootstrap/docker-recovery.test.tssrc/lib/onboard/managed-bootstrap/docker.tssrc/lib/onboard/sandbox-gpu-create-flow.test.tssrc/lib/onboard/sandbox-gpu-create-flow.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/onboard/sandbox-gpu-create-flow.test.ts
- src/lib/onboard/managed-bootstrap/docker-journal.test.ts
- src/lib/onboard/managed-bootstrap/docker-journal.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Add the dormant image-owned managed-bootstrap runtime for OpenClaw, Hermes, and LangChain Deep Agents Code. It authenticates the canonical envelope, atomically moves that exact inode into a root-private retry claim, applies the root request, publishes identity-bound completion, and consumes only the authenticated claim. A newer canonical request is preserved. This slice does not package, advertise, or activate buildless support. ## Related Issue Part of #7744. ## Changes - Add the composed bootstrap image runtime and its recovery, apply, and verification CLI modes without importing managed-bootstrap from the neutral managed-startup layer. - Require bounded root:root mode-0400 single-link envelopes, protected root-owned claim parents, and a root-private mode-0700 adjacent claim. - Bind the authenticated descriptor through same-device atomic rename, preserve a newly staged canonical request, and fail closed before application if the claim cannot be made. - Restore a protected displaced request after interruption immediately following rename, and reconcile the exact two-link state left by interruption during exclusive restoration without overwriting a later canonical request. - Keep the private claim as sole retry authority across application, completion-publication, and cleanup interruption; resume it idempotently. - Publish bootstrap completion only after the shared managed-startup application succeeds, then delegate shared completion and runtime-environment verification. - Sequence recovery, conditional apply, and exact completion verification from the native trampoline for every supported agent before supervisor resume. - Preserve provider neutrality and inert activation: production providers remain bootstrap-unsupported and current images do not package this entrypoint. - Split handoff and descriptor-integrity tests into a focused 521-line suite; keep branch-heavy fake-FS setup in a 429-line non-test helper so no legacy size or conditional-growth allowance is required. - Address all CodeRabbit feedback and record the independent race, recovery, documentation, and advisor dispositions. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] 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: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [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: two independent exact-tree audits cover atomic claim ownership, descriptor identity, replacement preservation, protected-parent validation, crash recovery, completion ordering, all-agent trampoline behavior, provider neutrality, and inert activation. - [x] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: the maintainer explicitly waived the known npm advisory and its consequential same-repository E2E blockage for this stack because that advisory is handled on a separate security stream. No other check is waived. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `src/lib/onboard/managed-bootstrap/README.md` was checked against the exact claim runtime, trampoline, tests, provider registry, production image packaging, and source-shape contract. It documents atomic private-claim retry authority, preservation of a newer canonical request, post-rename crash restoration, fail-closed rename, one-way managed-bootstrap-to-managed-startup composition, delegated shared completion verification, Node-owned state transitions, and inert activation. No published `docs/` change is needed while the capability remains unpackaged and provider-unsupported. - Agent: Codex Desktop documentation writer and independent exact-tree auditor <!-- docs-review-head-sha: 9c67a9a --> <!-- docs-review-agents-blob-sha: 789b0d9 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - Exact public head: `9c67a9a2a0f98c34508305360671a8a9c7a4bec5`. - Exact head tree: `c615ee677c9a8bab4eebcbda57f97224da69eb09`. - Exact PR base used by the current workflow dispatch: `878430aebebcc5e584be3bef6685997d039c213a`. - Direct base-to-head scope: 11 files, `+2675/-575`. - Stable patch ID: `2ca3949b0dd55275a70bd86fd3a8d9b2118b2bc5`. - Binary diff SHA-256: `68d73166b4ac0dd62e7d12c7bab09a1b3be775082c544c211d3ed929f010ed78`. - Focused five-file runtime/source-shape verification: 94 passed; 9 Linux-only tests skipped on the macOS host. The exact runtime suite passes 46 of 46. - The largest changed test file is 1,291 lines; branch-heavy fake-FS setup is in a 429-line non-test helper. The trusted base-to-head evaluators report zero size violations and `0 -> 0` conditional growth for all five changed test files. - CLI build and typecheck, repository architecture checks, source-shape and test-size guards, Biome, Markdown lint, ShellCheck, diff checks, commit hooks, and pre-push hooks pass locally. - The append-only history preserves every former public head. `9c67a9a2a` has sole parent `30b61ca5c`; that GitHub update commit retains `856a021b3` as its first parent and the exact workflow base as its second. No remote history was rewritten. - GitHub verifies the exact head signature and DCO. Exact-head CI, advisors, CodeRabbit, and protected E2E are running. - All seven findings in CodeRabbit review `4850036066` are addressed. The prior Nemotron size warning and Terra post-rename replacement race are fixed; dependency direction, delegated verification, Node-owned state transitions, and fail-closed recovery behavior are explicit in the README and independently audited. - [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 - [ ] Applicable broad gate passed — exact-head CI is running - [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) - [x] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## Stack - Base: merged PR3.12B #8043 plus the PR workflow base above. - This slice: PR3.13 #8045 at exact public head `9c67a9a2a`, targeting `main`. - Later dormant/additive slices are being consolidated into internally complete review batches to pay one qualification cycle per batch. Buildless and Podman support remain disabled until the complete all-agent, multiarch, GPU/local- inference, recovery, installer, and protected-E2E gates in #7744 pass. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Reconcile every durable managed-bootstrap transaction before a new sandbox create begins, using the provider lifecycle boundary so restart recovery remains driver-neutral and pluggable. This slice remains inert: it does not advertise or activate buildless support.
Related Issue
Part of #7744.
Changes
Type of Change
Quality Gates
E2E / PR Gateblockage are handled on a separate security stream and explicitly waived as blockers for this stack.Documentation Writer Review
docs-updatedsrc/lib/onboard/managed-bootstrap/README.mdpreserves the identity-addressed/finality and driver-neutral ownership model, and adds the exact legacy journal drain contract: immutable-ID inspection, no mutable-name deletion, no inferred agent, preserved journal/sidecars, scoped unrelated-sandbox progress, and [Epic] Support native Podman with buildless managed onboarding #7744 retirement gating before activation.DGX Station Hardware Evidence
Verification
03fbf5ebf1dfc09506c5fefb1debe4b8766fde57.aca6e3d1787b4b9d5952c86e290300fb48fea683; exact tree:9b727abe92e634f006e55798f2a3fc78f02f14fa.+2816/-172.49f44db7a412c74cc1e67ab03f41ea1bf28a5c9c.0fef658b79f052247f9dbb0d0ec4e9ff31eec8fedbcde1592b6d5659963cb4ba.f3d39891bab25bec0092de9a1f001fb62b1cdd6c, then adds signed+DCO review children2d1c54276ac5bba78fc950eddb86106edef35d99,fd260f2e0fd7da6aa667f149e9c9edfd4d6283a3,456fd86e3f45c030651a67ca0afe14be53ee4d7e, andaca6e3d1787b4b9d5952c86e290300fb48fea683; no remote history was rewritten.backup/podman-stack/pr8043-feedback-2d1c5427,backup/podman-stack/pr8043-feedback-fd260f2e,backup/podman-stack/pr8043-feedback-456fd86e,backup/podman-stack/pr8043-feedback-aca6e3d1, andhold/podman-stack/pr8045-base-aca6e3d1; all earlier source/reconstruction refs remain intact.Stack
03fbf5ebf1dfc09506c5fefb1debe4b8766fde57.aca6e3d1787b4b9d5952c86e290300fb48fea683, targetingmain.hold/podman-stack/pr8045-base-aca6e3d1while its exact patch is recascaded append-only after this merge.Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes