fix(images): report managed runtime assertion failures - #8673
Conversation
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe managed startup runtime checks now report detailed diagnostics before exiting. The bootstrap image contract tests validate missing-file and symlink failures, metadata checks, and fail-closed output across the native and agent Dockerfiles. ChangesManaged runtime diagnostics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
2 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
2 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 1 warning · 0 suggestionsWarningsWarnings do not block.
|
## Summary [Brev image run 31352614414](https://github.com/brevdev/nemoclaw-image/actions/runs/31352614414) failed the shared managed-image `RUN` without emitting a concrete assertion name. This change adds named, fail-closed diagnostics for MCP discovery bundle execution, JSON-contract validation, discovery-tree traversal, and the first unsafe path while preserving every existing predicate, assertion, and successful behavior. Contract mismatches report bounded, credential-redacted actual fields beside the expected contract. ## Related Issue Related to #8665. PR #8702 addresses the image-metadata root cause and overlaps the same shared block; this PR does not close the issue. ## Changes - Report the bundle or `find` exit status before another command can overwrite it. - Keep the existing three-field discovery contract and acceptance of extra fields, but report its sanitized actual response and exact expected response on failure. - Mirror NemoClaw's current credential-pattern families in the bounded response diagnostic, including malformed JSON and private-key material. - Report the first unsafe path with dereferenced UID, GID, file type, and mode plus the path's symlink status. Dangling links retain link metadata. - Apply the same block to the OpenClaw, Hermes, and Deep Code Dockerfiles without changing the five managed-runtime assertions added by #8673. - Execute every new failure message and the unchanged success path through the shared contract helper for all three Dockerfiles. - Preserve the evidence boundary: the run log proves that no guarded assertion failed, while the previously silent discovery prefix did not identify which check stopped the build. - Do not dispatch another Brev image build; downstream image validation remains after merge. ## 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: Internal image-build failure diagnostics and contract coverage only; no public command, configuration, default, successful runtime behavior, or documented workflow changes. - [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: Codex Desktop independently reviewed head `f67df5af4` against the security rubric. The review found and verified fixes for canonical credential redaction and `find -L` referent metadata, then reported PASS with no remaining publication blocker. - [ ] 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: No documentation paths changed. The final diff contains internal managed-image build diagnostics and their contract tests only. - Agent: Codex Desktop <!-- docs-review-head-sha: f67df5a --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## 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 — `npm exec -- vitest run --project integration test/mcp-tool-discovery-image-contract.test.ts test/openclaw-final-image-layout.test.ts test/hermes-final-image-layout.test.ts test/langchain-deepagents-code-image.test.ts` — 4 files and 49 tests passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not applicable because the change is limited to one managed-image contract helper and its three focused consumers. - [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: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved managed image validation for symlink targets and filesystem safety. * MCP tool discovery now distinguishes execution and scanning failures and reports explicit command statuses. * Invalid or unexpected discovery data is handled safely with clearer contract validation. * Diagnostic output now redacts credential- and private-key-like values. * **Tests** * Expanded coverage for discovery failures, malformed data, unsafe permissions, symlinks, and successful validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
PR #8642 restored the managed startup runtime file invariants, but a failed Docker assertion reported only the shell exit code. This change reports the failed invariant and bounded file metadata before preserving the same nonzero build result.
Changes
root:root, and0444checks fail-closed.Type of Change
Quality Gates
b18ad31e7against all nine security categories. The applicable error-handling, security-testing, and system-security checks passed. The diagnostic emits only a fixed path and numeric file metadata, and every failed assertion still exits nonzero.Documentation Writer Review
no-docs-neededb18ad31e7changes only internal managed-image build diagnostics and their tests. It changes no command, configuration, default, runtime behavior, or documented workflow. The review found no terminology, structure, or changed-text findings.DGX Station Hardware Evidence
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 unavailablenpm exec -- vitest run --project integration test/openclaw-final-image-layout.test.ts test/hermes-final-image-layout.test.ts test/langchain-deepagents-code-image.test.tspassed 36 tests in 3 files.hadolintandnpm run source-shape:checkalso passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Julie Yaunches jyaunches@nvidia.com