feat(images): package and publish all-agent managed images - #8047
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>
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 exact reviewed PR3.5 patch while inheriting the canonical restacked PR3.4b base. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Trigger exact-head CI after the canonical GitHub bot restack without changing the reviewed tree. Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Preserve the exact reviewed PR3.5 tree while inheriting the signed exact-head validation trigger. 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 This change adds an operation-scoped Podman command adapter, rootless host preflight, and basic CPU start and stop lifecycle through an injected runtime-provider bundle. The bundle remains absent from the production registry, so this PR does not enable or advertise Podman support. ## Related Issue Part of #7744. ## Changes - Add an immutable `ContainerEngine` command boundary that binds one executable, endpoint, operation, engine identity, and opaque endpoint-authority identity. Runtime providers need separate host-doctor and sandbox-lifecycle authority, so a process-global engine selector is insufficient. - Add a Podman adapter that pins and revalidates one private rootless Unix socket and its directory chain before and after each command. The adapter rejects group/world-writable socket authority, detects endpoint rotation, and requires independently constructed provider surfaces to carry the same authority identity. - Add Linux amd64/arm64 rootless Podman preflight for Podman 5 or newer, cgroup v2, service architecture, and subordinate UID/GID maps. Preflight qualifies both the local client and the selected remote service endpoint and rejects mixed or stale versions. - Add fail-closed lookup plus CPU start, unpause, and stop behavior for strictly labeled OpenShell-compatible containers. Exact-ID mutation and post-mutation inspection make retries converge without falling back to a mutable container name. - Add a dormant provider bundle and exercise start and stop through the shared runtime-provider lifecycle for OpenClaw, Hermes, and DCode. Workload support and production registration remain disabled. - Route the existing host proxy helper directly to its leaf no-proxy implementation so the adapter remains within the repository's exact dependency budget. ## 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: The Podman bundle is not registered in `CURRENT_RUNTIME_PROVIDER_BUNDLES`; tests inject it explicitly, and existing docs must continue to describe Podman as unsupported until the activation slice. - [x] 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: `no-docs-needed` - Evidence: `AGENTS.md` and `WRITING.md` were reviewed at exact base `99c4ef2b9` and exact head `3231d9f71`. The 14-file diff adds only an injected, dormant Podman command/preflight/CPU-lifecycle bundle and tests; `CURRENT_RUNTIME_PROVIDER_BUNDLES` still exposes only Docker and Kubernetes. Current docs explicitly classify Podman as unsupported, so changing user documentation would advertise behavior this slice does not activate. Changed comments and test titles use consistent repository terms and introduce no blocking ambiguity. The append-only parent refresh to `8bfea3af9` preserves the exact reviewed slice diff and changes no reviewed documentation. - Agent: Codex Desktop - Qualification carry-forward: Exact head fda94da on base 1f4683f preserves the byte-identical previously reviewed slice diff (stable patch ID 0a6ac0c1566a2bf1c08c70efbec8305084603b29; binary diff SHA-256 9871d548aaf8a5ad09a190818393ee836aaed49d8aa24d29735733f631a74848), and its source and documentation tree is unchanged from reviewed head beb5731. <!-- docs-review-head-sha: fda94da --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - Qualification-only exact head/base: fda94da / 1f4683f; signed-DCO append-only cascade, stable-patch proof, byte-identical slice diff, final-tree equality, and normal pre-push hooks passed. - [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: exact-head `npx vitest run --project cli src/lib/adapters/container-engine.test.ts src/lib/adapters/podman/index.test.ts src/lib/adapters/podman/socket-authority.test.ts src/lib/onboard/runtime-provider/podman-lifecycle.test.ts src/lib/onboard/runtime-provider/podman-preflight.test.ts src/lib/onboard/runtime-provider/podman.test.ts` passed 6 files / 40 tests, and the 2-test runtime source-shape guard passed after inventorying the three new provider paths. - [x] Applicable broad gate passed — `npm run typecheck:cli` and `npm run checks:repository` passed at exact head `3231d9f71`; the append-only parent refresh preserved stable patch ID `0a6ac0c1566a2bf1c08c70efbec8305084603b29` from base `99c4ef2b9`. Exact-head CI run [30696718770](https://github.com/NVIDIA/NemoClaw/actions/runs/30696718770), both advisor lanes [30696717820](https://github.com/NVIDIA/NemoClaw/actions/runs/30696717820), CodeRabbit, protected E2E [30696810814](https://github.com/NVIDIA/NemoClaw/actions/runs/30696810814), and coordination all passed with no unresolved review threads. - [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: 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>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (17)
test/managed-startup-image-entrypoint.test.ts (2)
136-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace every absolute-path occurrence, not only the first.
String.prototype.replacewith a string pattern substitutes only the first match. Todayscripts/managed-startup-hold.shnames/usr/local/bin/nodeand/usr/local/bin/nemoclaw-startonce each, so the test works. If a later change adds a second reference, the substitution leaves the real absolute path in the fixture. Absolute paths bypass the$TEST_PATHredirect, so the test would run the real system binary and fail in a confusing way. UsereplaceAllto keep the fixture hermetic.♻️ Proposed hardening
- .replace("/usr/local/bin/node", path.join(directory, "node")) - .replace("/usr/local/bin/nemoclaw-start", path.join(directory, "nemoclaw-start")); + .replaceAll("/usr/local/bin/node", path.join(directory, "node")) + .replaceAll("/usr/local/bin/nemoclaw-start", path.join(directory, "nemoclaw-start"));🤖 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/managed-startup-image-entrypoint.test.ts` around lines 136 - 137, Update the fixture substitutions in the test setup to use replaceAll for both /usr/local/bin/node and /usr/local/bin/nemoclaw-start, ensuring every occurrence is redirected to the temporary directory and the fixture remains hermetic.
68-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider dropping the source-text assertions that behavior already proves.
Lines 67, 74, and 80-82 pin properties that process behavior cannot show, so source-text checks fit there. Lines 68-70 and 72 differ. The trace assertion at lines 174-177 already proves the identity forwarding and the delegation target. The negative case at lines 236-247 already proves the delimiter requirement. These four assertions lock in argument-index style and quoting, so a behavior-preserving refactor of
scripts/managed-startup-hold.shbreaks the test.Keep the shebang check, the single-
execcheck, and the import checks. Remove the four assertions that duplicate covered behavior.♻️ Proposed reduction
expect(source.startsWith("#!/bin/bash -p\n")).toBe(true); - expect(source).toContain('[ "$5" = "--bootstrap-identity" ]'); - expect(source).toContain('[ "$7" = "--" ]'); - expect(source).toContain('--bootstrap-identity "$_nemoclaw_bootstrap_identity"'); - expect(source).toContain( - 'exec "${_nemoclaw_scrubbed_env[@]}" /usr/local/bin/nemoclaw-start "$@"', - ); expect(source.match(/^exec /gmu)).toHaveLength(1);As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 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/managed-startup-image-entrypoint.test.ts` around lines 68 - 72, In the managed startup entrypoint test, remove the four source-text assertions for positional argument indices, delimiter syntax, bootstrap identity forwarding, and the delegation command. Keep the shebang, single-exec, and import assertions, relying on the existing trace and negative-case behavior tests to cover these outcomes.Source: Path instructions
test/helpers/managed-image-publication-barrier.ts (2)
39-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the digest stride from
publicationPlatforms.length.The literal
2at line 40 must equal the number of platforms. TodaypublicationPlatformshas two entries, so every(agent, platform)pair maps to a unique value, and the offsets of 20 keep the six families disjoint. If a third platform joinspublicationPlatformsat line 10,agentIndex * 2 + platformIndexcollides across agents, and unrelated candidates share a digest. The resulting test failures would be hard to attribute.Bind the stride to the array length so the helper stays correct when the platform list grows.
♻️ Proposed fix
function digestFor(agentIndex: number, platformIndex: number, offset: number): string { - return `sha256:${(offset + agentIndex * 2 + platformIndex).toString(16).padStart(64, "0")}`; + const stride = publicationPlatforms.length; + return `sha256:${(offset + agentIndex * stride + platformIndex).toString(16).padStart(64, "0")}`; }Note that the offset spacing of 20 also bounds the platform count. With more platforms, widen the offsets in
candidates()as well.🤖 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/managed-image-publication-barrier.ts` around lines 39 - 41, Update digestFor to use publicationPlatforms.length as the agent stride instead of the literal 2, ensuring each agent/platform pair remains unique as platforms are added. Also widen the offset spacing in candidates() to remain greater than the expanded per-agent platform range.
292-292: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe pointer script masks the promotion script exit status.
bash -cruns${script}and${pointerScript}as separate statements with noset -e.result.statusat line 330 therefore reports the exit status of the last command. When a caller suppliespointerScript, a non-zero exit from${script}is discarded.Current callers do not hit this.
test/managed-image-publication-workflow.test.tspasses a pointer only for the accepted promotion and passes no pointer for the failed-agent case. A future test that pairs a pointer with a failing promotion would assert a passing status and hide the failure.Chain the two scripts so the pointer runs only after the promotion succeeds and any failure propagates.
♻️ Proposed fix
- const result = spawnSync("bash", ["-c", `${script}\n${pointerScript}`], { + const command = pointerScript ? `${script}\n) && (\n${pointerScript}` : script; + const result = spawnSync("bash", ["-c", pointerScript ? `(\n${command}\n)` : command], {A simpler form also works if the pointer must always run: keep the current concatenation and return both statuses separately instead of one
status.🤖 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/managed-image-publication-barrier.ts` at line 292, Update the bash command construction in the managed-image publication barrier around spawnSync so the promotion script’s non-zero exit status propagates when a pointer script is provided. Chain the scripts with the pointer execution conditional on successful completion of ${script}, while preserving the existing behavior when no pointer script is supplied.test/managed-image-capability-union.test.ts (2)
139-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrace argv boundaries instead of
$*.The
uvshim records"$*", which joins all arguments with a single space. The assertion then compares one flattened string. An argument that contains whitespace, or a split argument, produces the same trace. Record one line per argument with"$@"to prove the exact argv.♻️ Proposed argv-faithful trace
fs.writeFileSync( path.join(temporaryRoot, "uv"), - '#!/bin/sh\nprintf \'%s\\n\' "$*" >> "$UV_TRACE"\n', + '#!/bin/sh\nprintf \'%s\\n\' "$@" >> "$UV_TRACE"\n', { mode: 0o755 }, ); @@ - expect(fs.readFileSync(trace, "utf8").trim()).toBe( - "pip install --python /opt/hermes/.venv/bin/python --no-cache -- microsoft-teams-apps==2.0.13.4 aiohttp==3.14.1", - ); + expect(fs.readFileSync(trace, "utf8").trim().split("\n")).toEqual([ + "pip", + "install", + "--python", + "/opt/hermes/.venv/bin/python", + "--no-cache", + "--", + "microsoft-teams-apps==2.0.13.4", + "aiohttp==3.14.1", + ]);🤖 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/managed-image-capability-union.test.ts` around lines 139 - 153, Update the temporary uv shim in installManagedImageCapabilityUnion to record each received argument separately using argv-preserving "$@" handling rather than flattening with "$*"; adjust the trace assertion to verify the resulting per-argument lines and preserve exact argument boundaries.
69-74: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider membership comparison for the alignment assertions.
toEqualon arrays also asserts order. These two assertions verify that the pinned constants cover every supported manifest. Manifest declaration order is not part of that claim. A reordering ofBUILT_IN_CHANNEL_MANIFESTStherefore fails this test even when the union stays correct. If install order is a separate contract, keep the order assertion in the pinned-spec test at Lines 40-53 and compare sorted copies here.🤖 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/managed-image-capability-union.test.ts` around lines 69 - 74, Update the alignment assertions in the managed-image capability union test to compare order-independent membership by sorting copies before comparison or using an equivalent set-based assertion. Apply this to the relevant pinned capability/platform arrays while preserving the existing order assertion in the pinned-spec test.test/managed-image-failure-diagnostics.test.ts (1)
176-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the caps were reached, not only respected.
The fixture creates
maxBundles + 2bundles and four files per bundle, so truncation must occur.toBeLessThanOrEqualalso passes when the exporter emits far fewer bundles or files, so a regression that drops evidence stays undetected. Assert the exact cap values for this fixture.♻️ Proposed exact cap assertions
- expect(fs.readdirSync(outputRoot).length).toBeLessThanOrEqual( - MANAGED_IMAGE_DIAGNOSTIC_EXPORT_LIMITS.maxBundles, - ); + expect(fs.readdirSync(outputRoot)).toHaveLength( + MANAGED_IMAGE_DIAGNOSTIC_EXPORT_LIMITS.maxBundles, + ); expect(fs.readdirSync(outputRoot)).toHaveLength(result.bundles); - expect(outputFiles.length).toBeLessThanOrEqual(MANAGED_IMAGE_DIAGNOSTIC_EXPORT_LIMITS.maxFiles); + expect(outputFiles).toHaveLength(MANAGED_IMAGE_DIAGNOSTIC_EXPORT_LIMITS.maxFiles);If the exporter can legitimately stop below
maxFilesbecausemaxTotalOutputBytesbinds first, assert that relationship explicitly instead.🤖 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/managed-image-failure-diagnostics.test.ts` around lines 176 - 180, Update the assertions in the managed-image diagnostic export test to verify the fixture reaches the configured caps, using exact maxBundles and maxFiles expectations rather than only upper bounds. If maxTotalOutputBytes can legitimately limit files first, assert that documented relationship explicitly while still confirming the expected bundle cap.test/managed-image-publication-evidence.test.ts (2)
390-405: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the rejection reason for these cases.
These two tests assert only a non-zero exit status and null evidence. The first case collapses three distinct defects, omitted SLSA, omitted SPDX, and a duplicate SLSA layer, into one generic check. Any unrelated failure of the verifier, for example a
jqparse error or a missing argument, also produces a non-zero status and no contract, so the test passes without proving its claim. The neighbouring tests at Lines 372-388 and 407-413 already assert specific stderr text. Add the expected message per case.♻️ Proposed reason-specific assertions
- it("rejects missing or duplicate predicate layers", () => { - for (const options of [{ omitSlsa: true }, { omitSpdx: true }, { duplicateSlsaLayer: true }]) { - const fixture = runEvidence(options); - expect(fixture.result.status).not.toBe(0); - expect(fixture.evidence).toBeNull(); - } - }); + it.each([ + [{ omitSlsa: true }, /exactly one SLSA/], + [{ omitSpdx: true }, /exactly one SPDX/], + [{ duplicateSlsaLayer: true }, /exactly one SLSA/], + ])("rejects missing or duplicate predicate layers", (options, expected) => { + const fixture = runEvidence(options); + expect(fixture.result.status).not.toBe(0); + expect(fixture.result.stderr).toMatch(expected); + expect(fixture.evidence).toBeNull(); + }); it("rejects an attestation layer labeled with the wrong predicate type", () => { const fixture = runEvidence({ slsaLayerPredicateType: "https://slsa.dev/provenance/v0.2", }); expect(fixture.result.status).not.toBe(0); + expect(fixture.result.stderr).toMatch(/predicate/); expect(fixture.evidence).toBeNull(); });Replace the patterns with the exact text that
scripts/checks/verify-managed-image-publication-evidence.shemits.As per path instructions: flag "conditionals that make a test pass without exercising its claim."
🤖 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/managed-image-publication-evidence.test.ts` around lines 390 - 405, Update the tests around “rejects missing or duplicate predicate layers” and the wrong predicate-type case to assert the verifier’s exact stderr rejection message for each individual defect. Split the first loop into separate cases for omitted SLSA, omitted SPDX, and duplicate SLSA layers, and retain the existing non-zero status and null-evidence checks alongside reason-specific assertions from verify-managed-image-publication-evidence.sh.Source: Path instructions
271-320: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRemove the fixture root in a
finallyblock.
fs.rmSyncruns only when bothspawnSyncandJSON.parsesucceed.spawnSyncthrows ifbashcannot be spawned, andJSON.parsethrows if the verifier writes a partial contract. Each throw leaks a temporary directory. This file callsrunEvidencemore than twenty times, so leaks accumulate.♻️ Proposed cleanup guarantee
- const evidence = fs.existsSync(output) - ? (JSON.parse(fs.readFileSync(output, "utf8")) as Record<string, unknown>) - : null; - fs.rmSync(root, { recursive: true, force: true }); - return { - candidateDigest, - evidence, - result, - slsaDescriptor, - spdxDescriptor, - workloadDigest, - }; + try { + const evidence = fs.existsSync(output) + ? (JSON.parse(fs.readFileSync(output, "utf8")) as Record<string, unknown>) + : null; + return { + candidateDigest, + evidence, + result, + slsaDescriptor, + spdxDescriptor, + workloadDigest, + }; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + }Move the
spawnSynccall inside the sametryto cover spawn failures as well.🤖 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/managed-image-publication-evidence.test.ts` around lines 271 - 320, Update the runEvidence cleanup flow around spawnSync, evidence parsing, and the return value so fs.rmSync(root, { recursive: true, force: true }) executes in a finally block. Place both verifier execution and JSON parsing inside the protected try path, ensuring fixture cleanup occurs when spawning bash or parsing partial output throws while preserving the existing successful return data.test/hermes-neutral-platform-env-activation.test.ts (1)
79-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert byte-identical output to prove idempotence.
The second patcher run only asserts exit status 0. A second run that appends a duplicate guard block would still pass. Capture the patched source after the first run and compare it after the second run.
♻️ Proposed stronger idempotence assertion
const { configPath, result, temporaryRoot } = runPatcher(UPSTREAM_FIXTURE); try { expect(result.status, result.stderr).toBe(0); + const patchedOnce = fs.readFileSync(configPath, "utf8"); const second = spawnSync("python3", ["-I", PATCHER, configPath], { encoding: "utf8", timeout: 5000, }); expect(second.status, second.stderr).toBe(0); + expect(fs.readFileSync(configPath, "utf8")).toBe(patchedOnce);🤖 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/hermes-neutral-platform-env-activation.test.ts` around lines 79 - 86, Strengthen the idempotence test around runPatcher by reading and storing the patched source after the first successful run, then read the source after the second spawnSync invocation and assert the contents are byte-identical. Keep the existing status checks and use the same configPath when comparing both results.test/managed-image-publication-workflow.test.ts (3)
132-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse one name for the
build-and-validatejob.
managedBuilderis a plain alias ofmanagedPublisher. Two names now refer to the same job, and a reader must resolve the alias to see that "builder" and "publisher" are identical. CallmanagedPublisherdirectly, or rename the single accessor to match the job name.As per coding guidelines: "Use existing repository vocabulary and one name per concept".
🤖 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/managed-image-publication-workflow.test.ts` at line 132, Remove the managedBuilder alias and use the existing managedPublisher accessor directly throughout the build-and-validate job setup in the test. Keep one consistent name for this job, following the repository’s managedPublisher vocabulary.Source: Coding guidelines
185-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLoosen the source-text markers that lock implementation syntax.
Lines 185-186 assert the exact JavaScript optional-chaining expression and the exact Python dict comparison inside the validation script. A local variable rename or an equivalent Python rewrite breaks these assertions without any behavior change. The neighbouring markers for label names, package identities, and the script name already cover the same claim. Drop these two markers, or replace them with the observable identifiers they guard.
As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 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/managed-image-publication-workflow.test.ts` around lines 185 - 186, Remove the two source-text markers asserting the exact JavaScript optional-chaining expression and Python dictionary comparison from the validation-script assertions. Keep the existing observable markers for labels, package identities, and script name, without adding replacement assertions for private implementation syntax.Source: Path instructions
66-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the new pull-request trigger paths.
The workflow adds
on.pull_request.paths, and Lines 66-69 add the type for it. No test pins those paths. ThemanagedInputPathslist at Lines 80-101 is compared only against the base workflow push paths at Lines 202-204. If a path filter is dropped fromon.pull_request, the PR build-and-runtime gate stops running for that change and no test fails. Add an assertion thaton.pull_request.pathscovers the required inputs.💚 Proposed coverage for the pull-request trigger
it("builds and exercises every shipped agent from an exact PR image before merge (`#7744`)", () => { const workflow = readWorkflow("managed-images.yaml"); + const prPaths = workflow.on?.pull_request?.paths ?? []; + for (const input of managedInputPaths) { + expect(prPaths, `pull-request trigger is missing ${input}`).toContain(input); + } const prBuilder = managedPrBuilder(workflow);🤖 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/managed-image-publication-workflow.test.ts` around lines 66 - 101, Extend the workflow test to assert that on.pull_request.paths covers every entry in managedInputPaths, alongside the existing push-path coverage check. Reuse the parsed pull_request configuration and the same required-path comparison logic, ensuring a missing PR trigger path causes the test to fail.scripts/checks/verify-managed-image-publication-evidence.sh (2)
289-303: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd explicit timeouts to the registry calls.
The token request and the blob downloads set
--retry 3but no timeout. If GHCR stalls the connection, each call can hang until the job timeout expires. Add--connect-timeoutand--max-timeso the verifier fails fast and the retries stay bounded.♻️ Proposed timeout hardening
registry_token="$({ - curl --fail --silent --show-error --location --retry 3 \ + curl --fail --silent --show-error --location --retry 3 \ + --connect-timeout 10 --max-time 60 \ "https://${registry}/token?service=${registry}&scope=repository:${registry_repository}:pull" } | jq -er '.token | select(type == "string" and length > 0)')" @@ - curl --fail --silent --show-error --location --retry 3 \ + curl --fail --silent --show-error --location --retry 3 \ + --connect-timeout 10 --max-time 300 \ -H "Authorization: Bearer ${registry_token}" \ "https://${registry}/v2/${registry_repository}/blobs/${statement_digest}" \ --output "$destination"🤖 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 `@scripts/checks/verify-managed-image-publication-evidence.sh` around lines 289 - 303, Add explicit --connect-timeout and --max-time options to both registry curl invocations: the token request assigned to registry_token and the blob download inside fetch_statement. Use bounded values that allow retries while ensuring stalled GHCR connections fail promptly, preserving the existing retry and error-handling behavior.
101-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the agent inventory from the canonical list.
Line 101 duplicates
MANAGED_STARTUP_AGENTSand the workflow matrix. A new agent can make this verifier reject valid evidence. Derive the allowlist from the canonical source or add a deterministic consistency check.🤖 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 `@scripts/checks/verify-managed-image-publication-evidence.sh` at line 101, Update the agent validation condition in the verification script to derive its allowlist from the canonical MANAGED_STARTUP_AGENTS source instead of hardcoding agent names. Ensure newly added canonical agents are accepted automatically, or add a deterministic check that fails when the verifier list diverges from the canonical inventory.Source: Path instructions
.github/workflows/managed-images.yaml (2)
1293-1319: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPin the Buildx version used by cohort staging.
The setup action is pinned, but its
versioninput is omitted. Buildx therefore resolves to the latest available release.--filesupports repeated descriptor inputs, but its merge behavior does not guarantee byte-for-byte preservation. Pin a tested Buildx version before relying on the full-objectcmpat line 1352.🤖 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 @.github/workflows/managed-images.yaml around lines 1293 - 1319, Pin the Buildx release used by the cohort staging workflow by adding the tested version input to the existing Buildx setup action before the descriptor loop and imagetools create flow. Keep the repeated --file inputs unchanged, but ensure the later full-object cmp validation runs with a deterministic Buildx version.
576-584: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one source of truth for managed-image package validation.
The canonical union includes
@openclaw/googlechat@{{openclaw.version}}, but the workflow omitsgooglechatand hard-codes plugin versions.diagnostics-otelandbravecome fromDockerfile, not the messaging manifests. Generate validation identities from these sources, or compare the complete installed set and versions deterministically, includinggooglechat,microsoft-teams-apps, andaiohttp.🤖 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 @.github/workflows/managed-images.yaml around lines 576 - 584, Update the managed-image package validation around the packages object to use the canonical package sources rather than a hand-maintained, hard-coded list. Include all required identities and versions, including googlechat, microsoft-teams-apps, and aiohttp, while sourcing diagnostics-otel and brave from Dockerfile and preserving the canonical openclaw.version-based version. Ensure the installed package set and versions are compared deterministically against this complete source of truth.
🤖 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 `@scripts/checks/verify-managed-image-publication-evidence.sh`:
- Around line 316-318: Update base_dependency_uri to match BuildKit’s dependency
schema, using the image’s tag or digest in the package URI with platform and
validating the immutable digest separately. Change provenance validation to
compare predicate.runDetails.builder.id rather than the entire builder object,
allowing optional fields. Refresh the fixture to represent actual pinned
BuildKit output.
In `@test/generate-openclaw-config-plugin-entries.test.ts`:
- Around line 51-73: Update the test case around neutralCapabilities to define a
fixed expected inventory of managed { channelId, pluginId } pairs instead of
deriving it from MANAGED_IMAGE_OPENCLAW_MESSAGING_CAPABILITIES and
MANAGED_IMAGE_OPENCLAW_BUNDLED_INERT_CAPABILITIES. Assert both exported
capability unions and generated config.plugins.entries/config.channels match
that inventory, while preserving the existing inert-state expectations and
avoiding dependence on production constants for the expected values.
---
Nitpick comments:
In @.github/workflows/managed-images.yaml:
- Around line 1293-1319: Pin the Buildx release used by the cohort staging
workflow by adding the tested version input to the existing Buildx setup action
before the descriptor loop and imagetools create flow. Keep the repeated --file
inputs unchanged, but ensure the later full-object cmp validation runs with a
deterministic Buildx version.
- Around line 576-584: Update the managed-image package validation around the
packages object to use the canonical package sources rather than a
hand-maintained, hard-coded list. Include all required identities and versions,
including googlechat, microsoft-teams-apps, and aiohttp, while sourcing
diagnostics-otel and brave from Dockerfile and preserving the canonical
openclaw.version-based version. Ensure the installed package set and versions
are compared deterministically against this complete source of truth.
In `@scripts/checks/verify-managed-image-publication-evidence.sh`:
- Around line 289-303: Add explicit --connect-timeout and --max-time options to
both registry curl invocations: the token request assigned to registry_token and
the blob download inside fetch_statement. Use bounded values that allow retries
while ensuring stalled GHCR connections fail promptly, preserving the existing
retry and error-handling behavior.
- Line 101: Update the agent validation condition in the verification script to
derive its allowlist from the canonical MANAGED_STARTUP_AGENTS source instead of
hardcoding agent names. Ensure newly added canonical agents are accepted
automatically, or add a deterministic check that fails when the verifier list
diverges from the canonical inventory.
In `@test/helpers/managed-image-publication-barrier.ts`:
- Around line 39-41: Update digestFor to use publicationPlatforms.length as the
agent stride instead of the literal 2, ensuring each agent/platform pair remains
unique as platforms are added. Also widen the offset spacing in candidates() to
remain greater than the expanded per-agent platform range.
- Line 292: Update the bash command construction in the managed-image
publication barrier around spawnSync so the promotion script’s non-zero exit
status propagates when a pointer script is provided. Chain the scripts with the
pointer execution conditional on successful completion of ${script}, while
preserving the existing behavior when no pointer script is supplied.
In `@test/hermes-neutral-platform-env-activation.test.ts`:
- Around line 79-86: Strengthen the idempotence test around runPatcher by
reading and storing the patched source after the first successful run, then read
the source after the second spawnSync invocation and assert the contents are
byte-identical. Keep the existing status checks and use the same configPath when
comparing both results.
In `@test/managed-image-capability-union.test.ts`:
- Around line 139-153: Update the temporary uv shim in
installManagedImageCapabilityUnion to record each received argument separately
using argv-preserving "$@" handling rather than flattening with "$*"; adjust the
trace assertion to verify the resulting per-argument lines and preserve exact
argument boundaries.
- Around line 69-74: Update the alignment assertions in the managed-image
capability union test to compare order-independent membership by sorting copies
before comparison or using an equivalent set-based assertion. Apply this to the
relevant pinned capability/platform arrays while preserving the existing order
assertion in the pinned-spec test.
In `@test/managed-image-failure-diagnostics.test.ts`:
- Around line 176-180: Update the assertions in the managed-image diagnostic
export test to verify the fixture reaches the configured caps, using exact
maxBundles and maxFiles expectations rather than only upper bounds. If
maxTotalOutputBytes can legitimately limit files first, assert that documented
relationship explicitly while still confirming the expected bundle cap.
In `@test/managed-image-publication-evidence.test.ts`:
- Around line 390-405: Update the tests around “rejects missing or duplicate
predicate layers” and the wrong predicate-type case to assert the verifier’s
exact stderr rejection message for each individual defect. Split the first loop
into separate cases for omitted SLSA, omitted SPDX, and duplicate SLSA layers,
and retain the existing non-zero status and null-evidence checks alongside
reason-specific assertions from verify-managed-image-publication-evidence.sh.
- Around line 271-320: Update the runEvidence cleanup flow around spawnSync,
evidence parsing, and the return value so fs.rmSync(root, { recursive: true,
force: true }) executes in a finally block. Place both verifier execution and
JSON parsing inside the protected try path, ensuring fixture cleanup occurs when
spawning bash or parsing partial output throws while preserving the existing
successful return data.
In `@test/managed-image-publication-workflow.test.ts`:
- Line 132: Remove the managedBuilder alias and use the existing
managedPublisher accessor directly throughout the build-and-validate job setup
in the test. Keep one consistent name for this job, following the repository’s
managedPublisher vocabulary.
- Around line 185-186: Remove the two source-text markers asserting the exact
JavaScript optional-chaining expression and Python dictionary comparison from
the validation-script assertions. Keep the existing observable markers for
labels, package identities, and script name, without adding replacement
assertions for private implementation syntax.
- Around line 66-101: Extend the workflow test to assert that
on.pull_request.paths covers every entry in managedInputPaths, alongside the
existing push-path coverage check. Reuse the parsed pull_request configuration
and the same required-path comparison logic, ensuring a missing PR trigger path
causes the test to fail.
In `@test/managed-startup-image-entrypoint.test.ts`:
- Around line 136-137: Update the fixture substitutions in the test setup to use
replaceAll for both /usr/local/bin/node and /usr/local/bin/nemoclaw-start,
ensuring every occurrence is redirected to the temporary directory and the
fixture remains hermetic.
- Around line 68-72: In the managed startup entrypoint test, remove the four
source-text assertions for positional argument indices, delimiter syntax,
bootstrap identity forwarding, and the delegation command. Keep the shebang,
single-exec, and import assertions, relying on the existing trace and
negative-case behavior tests to cover these outcomes.
🪄 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: 7e769baa-f549-48b2-a8d5-1cf83167ea45
📒 Files selected for processing (44)
.github/workflows/base-image.yaml.github/workflows/managed-images.yamlDockerfileagents/hermes/Dockerfileagents/hermes/config/hermes-config.tsagents/hermes/image-build-probes.pyagents/hermes/patch-neutral-platform-env-activation.pyagents/langchain-deepagents-code/Dockerfileci/env-var-doc-allowlist.jsonci/source-architecture-budget.jsonci/source-shape-test-budget.jsonscripts/check-dcode-profile-import-gate.shscripts/checks/export-managed-image-failure-diagnostics.tsscripts/checks/run-managed-image-direct-e2e.tsscripts/checks/verify-managed-image-publication-evidence.shscripts/generate-openclaw-config.mtsscripts/managed-startup-hold.shsrc/lib/onboard/managed-bootstrap/README.mdsrc/lib/onboard/managed-bootstrap/adapter.test.tssrc/lib/onboard/managed-bootstrap/adapter.tssrc/lib/onboard/managed-bootstrap/docker-test-fixture.tssrc/lib/onboard/managed-bootstrap/image-runtime.tssrc/lib/onboard/managed-startup-bootstrap-image-runtime.test.tssrc/lib/onboard/managed-startup/hold.tssrc/lib/onboard/sandbox-create-launch.test.tssrc/lib/onboard/sandbox-create-launch.tssrc/lib/sandbox/build-context.tstest/dcode-base-image-workflow.test.tstest/dcode-sandbox-identity-integration.test.tstest/generate-hermes-config.test.tstest/generate-openclaw-config-plugin-entries.test.tstest/helpers/managed-image-publication-barrier.tstest/hermes-doctor-config-hash.test.tstest/hermes-final-image-layout.test.tstest/hermes-image-build-probes.test.tstest/hermes-neutral-platform-env-activation.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-profile-build-gate.test.tstest/managed-image-capability-union.test.tstest/managed-image-direct-e2e-inputs.test.tstest/managed-image-failure-diagnostics.test.tstest/managed-image-publication-evidence.test.tstest/managed-image-publication-workflow.test.tstest/managed-startup-image-entrypoint.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- ci/source-architecture-budget.json
- scripts/check-dcode-profile-import-gate.sh
- ci/env-var-doc-allowlist.json
- test/hermes-doctor-config-hash.test.ts
- test/langchain-deepagents-code-profile-build-gate.test.ts
- test/langchain-deepagents-code-image.test.ts
- test/hermes-final-image-layout.test.ts
- scripts/checks/export-managed-image-failure-diagnostics.ts
- src/lib/sandbox/build-context.ts
- test/generate-hermes-config.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/onboard/managed-startup/image-runtime.ts (1)
1694-1697: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a failure-path test for the composed bootstrap entrypoint.
The composed runtime delegates to
mainManagedStartupImageRuntime(argv)once and setsprocess.exitCode = 1on rejection. Add a process-level test that asserts this behavior for a failing startup action.🤖 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-startup/image-runtime.ts` around lines 1694 - 1697, Add a process-level failure-path test for the composed managed-bootstrap entrypoint, invoking a startup action that causes mainManagedStartupImageRuntime(argv) to reject and asserting it is called once with the expected arguments and sets process.exitCode to 1. Keep the test focused on the composed runtime’s delegation and rejection behavior.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.
Nitpick comments:
In `@src/lib/onboard/managed-startup/image-runtime.ts`:
- Around line 1694-1697: Add a process-level failure-path test for the composed
managed-bootstrap entrypoint, invoking a startup action that causes
mainManagedStartupImageRuntime(argv) to reject and asserting it is called once
with the expected arguments and sets process.exitCode to 1. Keep the test
focused on the composed runtime’s delegation and rejection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d54b9ecc-60a5-4ae2-b286-e5dab4e1f4aa
📒 Files selected for processing (7)
Dockerfilesrc/lib/onboard/dockerfile-remote-dashboard-bind-contract.tssrc/lib/onboard/managed-startup/image-runtime.tstest/helpers/managed-image-publication-barrier.tstest/managed-image-publication-workflow.test.tstest/openclaw-final-image-layout.test.tstest/runtime-provider-source-shape.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- test/openclaw-final-image-layout.test.ts
- src/lib/onboard/dockerfile-remote-dashboard-bind-contract.ts
- test/helpers/managed-image-publication-barrier.ts
- test/managed-image-publication-workflow.test.ts
- Dockerfile
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
Package the dormant managed-bootstrap boundary into the OpenClaw, Hermes, and LangChain Deep Agents Code images and qualify exact managed-image artifacts for amd64 and arm64. Production runtime selection remains unsupported, so this slice does not advertise or activate buildless or Podman support.
Related Issue
Part of #7744.
Changes
Type of Change
Quality Gates
PR review advisor (GPT-5.6 Terra)andPR review advisor (Nemotron 3 Ultra)are accepted as mechanical service failures on exact-head attempt 2: Terra rejected an invalid service-generatedmax_output_tokens=1, while Nemotron returned no required analysis. The exact-head publisher reports 0 blockers, 0 warnings, 0 suggestions, and no follow-up needed; the primary advisor passed on the immediately preceding implementation head in run 30883374986. No code follow-up issue is warranted.Documentation Writer Review
docs-updatedsrc/lib/onboard/managed-bootstrap/README.mddocuments the all-agent image-packaging boundary and explicitly keeps production onboarding inert. The exact-head qualification corrections change only private image construction and direct-E2E boundaries; publicdocs/remain unchanged because this slice does not activate a user-visible runtime surface.DGX Station Hardware Evidence
Verification
4af85d6f86a9db6ff201f75f31f94e75987a7410.6a1b8443ais preserved atbackup/podman-stack/pr8047-contaminated-6a1b8443; reviewed clean sourced656863eais preserved atbackup/podman-stack/pr8047-clean-d656863e.mainat453b5df0a0cd17e1f898c333a6c1583d96f51096; the exact-head tree is4a3b61ec6a731babdcf01f19e469be7e00aff551, and the independently reviewed pre-correction clean source remains preserved at its backup ref.Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Chores