fix(onboard): continue journaled rebuild recreation - #8339
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds journal-aware sandbox recreation resume handling. It preserves active recreation state after source deletion and updates risk-plan routing so affected handlers select focused E2E coverage. ChangesSandbox recreate handoff
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxFlow
participant decideSandboxResume
participant SandboxRegistry
participant RecreateJournal
SandboxFlow->>decideSandboxResume: provide recreate request and journal handoff
decideSandboxResume->>SandboxRegistry: preserve active recreation entry
decideSandboxResume-->>SandboxFlow: return recreate continuation
SandboxFlow->>RecreateJournal: complete journal after replacement creation
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 834c533 in the TypeScript / code-coverage/cliThe overall coverage in commit 834c533 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
5 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/README.md (1)
687-692: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTie the documented file list to the executable risk plan.
test/e2e/README.mdrepeats the two paths stored intools/advisors/risk-plan.mtsLines 33-36. If one list changes without the other, the documented E2E contract and actual routing can diverge. Generate this mapping from the Set or add a consistency check that fails on drift.As per path instructions, derive inventories from a canonical source where possible and flag duplicated lists that can silently drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/README.md` around lines 687 - 692, Update the E2E documentation and risk-plan handling so the repeated handler path list in test/e2e/README.md is derived from, or validated against, the canonical Set in tools/advisors/risk-plan.mts. Add a consistency check that fails when the documented paths for openshell-gateway-upgrade and ubuntu-repo-cloud-langchain-deepagents-code diverge from the executable mapping.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 `@test/e2e/README.md`:
- Around line 687-692: Update the E2E documentation and risk-plan handling so
the repeated handler path list in test/e2e/README.md is derived from, or
validated against, the canonical Set in tools/advisors/risk-plan.mts. Add a
consistency check that fails when the documented paths for
openshell-gateway-upgrade and ubuntu-repo-cloud-langchain-deepagents-code
diverge from the executable mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89442b8e-8f38-4f82-9705-ddad75267677
📒 Files selected for processing (4)
test/e2e/README.mdtest/e2e/support/e2e-cross-runtime-compatibility.test.tstest/pr-risk-plan.test.tstools/advisors/risk-plan.mts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@test/e2e/e2e-cloud-experimental/checks/12-deepagents-code-thread-auto-approval.sh`:
- Around line 112-113: Update the status check around the CLI invocation to stop
passing raw status_json to fail; use a content-free failure message or an
approved redacted artifact reference, while preserving the existing failure path
for the expected_mode check.
🪄 Autofix
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: b902ed77-7877-43c5-b50d-85e85f40df5b
📒 Files selected for processing (3)
test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.shtest/e2e/e2e-cloud-experimental/checks/12-deepagents-code-thread-auto-approval.shtest/e2e/live/openshell-gateway-upgrade.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
3 participants
Summary
NemoClaw now continues a journal-owned sandbox recreation when an outer rebuild has already deleted the source sandbox or left it not ready. Before this change, resumed onboarding treated that expected unavailable state as a new repair. That behavior interrupted gateway upgrades and repeated named Deep Agents Code rebuilds.
Changes
missingornot_ready, including when the outer rebuild starts a replacement session whose sandbox step is incomplete.Type of Change
Quality Gates
test/e2e/README.mddocuments the internal E2E selection contract.37d19e2b0088a380135a9ee338ae379aa2e30abdagainst all nine categories and reported PASS with no findings. Continuation requires resume mode, state exactlymissingornot_ready, explicit recreation, and the internal journal fingerprint. Missing-source backup selection additionally requires deletion proven by that journal. Unknown states fail closed, and same-name replacement remains bound to the sandbox, gateway, transaction, generation, target fingerprint, registry identity, and live identity recorded by the transaction.Documentation Writer Review
docs-updatedtest/e2e/README.mdwith the deterministic E2E selection contract for shared journaled sandbox recreation handlers. Reviewed the complete 13-file PR diff, including journal-owned continuation, deletion proof before backup restoration, fail-closed diagnostics, test titles, lifecycle comments, resume output, and E2E activation text. Existing user documentation already covers interrupted rebuild and installer recovery behavior, so nodocs/source-page update is needed. Focused tests and applicable repository validation pass; one existing 5-second changed-test timeout passed when rerun alone./root/docs_reviewDGX 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 run docsbuilds without warnings (doc changes only)Additional regression evidence:
31008970541failed four gateway-version jobs after the source sandbox was destroyed; the fifth version passed.nemoclaw statusunhealthy.33128995eand failed in the first inspected main suite that included the commit.31022626003confirmed that both named Deep Agents Code rebuilds completed. Its final JSON assertion failure came from a temporary stderr-merging diagnostic that is not in the current PR commit.31026765445confirmed the gateway failure observed the deleted source asmissing, which now has direct decision and transaction coverage.31029439056confirmed the Deep Agents Code target passes both named rebuilds. Its gateway jobs reached journal continuation and then exposed the separate post-delete backup-proof failure fixed at the current head.openshell-gateway-upgradeandubuntu-repo-cloud-langchain-deepagents-codefor commit37d19e2b0088a380135a9ee338ae379aa2e30abd.Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests