refactor(rebuild): collapse recovery planning layers

[jyaunches]

Summary

This stacked draft consolidates the durable rebuild recovery boundary after the behavior-proving rollout in #6574. It preserves recovery behavior while removing two pass-through production modules, their duplicate plan object, and split transaction-generation routing. The full PR is exactly line-neutral at +258/-258; production code is +120/-218 (net -98) versus #6574, while tests are +138/-40 (net +98).

Related Issue

Closes #6580.
Part of #6433.

Changes

• Keep decideRebuildRecovery as the independently tested pure phase/observation decision boundary.
• Move live-evidence collection, decision evaluation, refusal presentation, and journal-owned registry restoration into one cohesive recovery-preflight operation beside that decision boundary.
• Remove the pass-through rebuild-recovery-plan.ts and rebuild-recovery-preflight.ts production modules.
• Replace the duplicate recovery-plan object with its behaviorally meaningful action: adopt, create, recreate, or resume.
• Derive replacement presence from that action at the surviving preflight/orchestrator boundary rather than carrying another copied boolean.
• Make RebuildTransactionCoordinator.startOrResume the single typed owner of active/completed generation selection, prepare/refresh routing, and observed-deletion reconciliation; the recovery orchestrator now delegates instead of repeating those phase decisions.
• Stabilize the boundary as: pure exported recovery decision plus its evidence collector in rebuild-recovery.ts, transaction generation/phase routing in the coordinator, and replacement revalidation/receipt publication in the recovery orchestrator. The decision remains independently testable; another pass-through decision file would recreate the layer this PR removes.
• Preserve fail-closed replacement adoption, registry restoration ownership, transaction-store contracts, receipt validation, redaction, and all established recovery outcomes.
• Shrink rebuild-pipeline.ts by one line, leave the durable transaction-store implementation untouched, and keep rebuild-preflight-phase.ts net neutral.
• Add a focused start/resume phase-routing matrix, prove adoption receipt ordering, and exhaustively exercise all 120 phase/live/registry/session decision combinations while retaining focused refusal-code coverage.
• Preserve the durable journal's source-of-truth/removal rationale at the surviving missing-registry repair and prove the typed start/resume input is forwarded intact before adoption receipt publication.
• Keep the PR at production +120/-218 (net -98), tests +138/-40 (net +98), and total +258/-258 (net 0) versus test(rebuild): prove live recovery boundaries #6574.
• Reduce the cumulative epic stack to production net +1957; tests rise to net +3494 and test-only config remains net +8, leaving the cumulative total unchanged at net +5459 while increasing durable proof.
• Treat refactor(rebuild): resume transaction-owned replacements #6522 as an intentional ancestor, not a concurrent competing change: the stack order is refactor(rebuild): resume transaction-owned replacements #6522 → test(rebuild): prove live recovery boundaries #6574 → this PR, and this PR's base is the exact test(rebuild): prove live recovery boundaries #6574 head.
• Do not manufacture impossible action/registry combinations for equivalence testing: adopt and resume require present correlated registry evidence, while registry repair is reachable only for create with a missing row. The 120-case decision matrix, focused missing-row restoration/refusal tests, replacement-boundary tests, and direct action-derived presence mapping cover the reachable legacy semantics.
• Keep resolveLiveState null propagation unchanged: every production null path already prints/logs the specific failure and invokes bail inside resolveRebuildLiveState; adding a second generic log or bail in the transparent collector would duplicate operator output and introduce behavior into this refactor.
• Model adoption prepare forwarding with a complete top-level StartOrResumeRebuildTransactionInput fixture and assert exact object identity plus receipt ordering. Nested target/recreate values are intentionally opaque at this forwarding-only boundary; the coordinator matrix owns their routing behavior.
• Preserve the intentional prepared/live refresh contract: while the source sandbox still exists, changed intent or backup evidence refreshes the prepared record after a fresh preflight/backup; it must not throw as the Nemotron follow-up proposed. After deletion, target override drift is rejected before startOrResume, and loadRebuildRecovery rejects changed backup/registry fingerprints. Focused boundary tests cover fresh prepared refresh, post-delete intent drift, and changed recovery evidence.
• Keep missing-row restoration race-safe through the existing boundaries: the per-sandbox onboard lock is held before recovery preflight, and restoreRebuildRegistryRecoveryIfMissing independently takes the registry lock, reloads current state, and refuses an existing name before its atomic save. The apparent read/restore TOCTOU does not bypass that locked compare-and-add.
• Keep registryRestored local to the effect site: no downstream branch depends on it; the only observable consequence is the success log emitted immediately when the locked restore returns true. Reintroducing a carried boolean would recreate the duplicate plan state this PR removes.

Type of Change

• 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

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: this is an internal orchestration consolidation with no public interface, configuration, or recovery-behavior change.
• 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: pending automated and human review on this draft.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npm run typecheck:cli; 77 focused CLI cases covering the exhaustive decision table, recovery preflight, start/resume owner, orchestrator, transaction/replacement boundaries, and installer recovery (the stateful replacement-boundary file is green in its required isolated run); 6 process-death integration tests; npm run test:projects:check; npm run test:titles:check.
• Applicable broad gate passed — exact-head pull-request CI passed build/typecheck, static checks, installer integration, plugin/ollama checks, all five CLI test shards, and aggregate coverage/checks; the surgical refactor does not change broad runtime/test-harness infrastructure.
• E2E-advisor-required exact-head targets passed — onboard-repair, onboard-resume, state-backup-restore, and upgrade-stale-sandbox.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ae3efae4-1922-424c-96a6-8531ab8f21b6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/6580-rebuild-recovery-consolidation

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the test/6437-rebuild-li... branch.

TypeScript / code-coverage/cli

The overall coverage in the refactor/6580-rebuil... branch is 76%. The coverage in the test/6437-rebuild-li... branch is 77%.

Show a code coverage summary of the most impacted files.

File	test/6437-rebuild-li... 4970809	refactor/6580-rebuil... 48aff7c	+/-
src/lib/inference/vllm.ts	76%	41%	-35%
src/lib/actions...ess-recovery.ts	79%	64%	-15%
src/lib/inference/nim.ts	71%	64%	-7%
src/lib/sandbox/config.ts	53%	61%	+8%
src/lib/inferen...board-probes.ts	86%	95%	+9%
src/lib/actions...-coordinator.ts	0%	93%	+93%
src/lib/state/r...-transaction.ts	0%	96%	+96%
src/lib/actions...ild-recovery.ts	0%	100%	+100%
src/lib/rebuild-correlation.ts	0%	100%	+100%
src/lib/actions...test-support.ts	0%	100%	+100%

---

_{Updated July 09, 2026 16:47 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

E2E Advisor Recommendation

Required E2E: onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox, sandbox-rebuild
Optional E2E: rebuild-openclaw, sandbox-operations

Dispatch hint: onboard-repair,onboard-resume,state-backup-restore,upgrade-stale-sandbox,sandbox-rebuild

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/test/6437-rebuild-live-recovery
Head: HEAD
Confidence: high

Required E2E

• onboard-repair (high): Required by the deterministic lifecycle-state risk plan. Rebuild recovery changes can affect repair convergence between registry, session, gateway, and live sandbox state after interrupted lifecycle operations.
• onboard-resume (high): Required by the deterministic lifecycle-state risk plan. Resume-style lifecycle state convergence is adjacent to the changed durable transaction recovery and session correlation behavior and must be validated against a real OpenShell sandbox.
• state-backup-restore (high): Required by the deterministic upgrade-rebuild risk plan. The changed rebuild recovery and transaction coordinator paths rely on validated backups and must preserve/restored user state across mutation and recovery.
• upgrade-stale-sandbox (high): Required by the deterministic upgrade-rebuild risk plan. The PR changes stale recovery routing and transaction phase handling, so stale sandbox upgrade/rebuild must prove retryability and state preservation on a live sandbox.
• sandbox-rebuild (high): The PR directly changes sandbox rebuild recovery, including old_deleted, replacement_created, adopt, recreate, and start-or-resume behavior. The live sandbox-rebuild E2E exercises real destructive rebuild interruptions and public-command recovery through those durable boundaries.

Optional E2E

• rebuild-openclaw (high): Useful adjacent confidence for OpenClaw-specific rebuild behavior and version/state preservation after rebuild, but sandbox-rebuild plus the deterministic floor are the merge-blocking coverage for this PR.
• sandbox-operations (medium): Optional broad lifecycle smoke for sandbox commands after lifecycle-state changes; not as targeted as the required rebuild, resume, repair, stale-upgrade, and backup/restore jobs.

New E2E recommendations

• sandbox rebuild recovery refusal (medium): Existing live rebuild coverage exercises successful old_deleted recovery and replacement adoption, while the changed decision table also contains refusal paths for mismatched registry/session/live evidence. A targeted live test that seeds mismatched recovery evidence and proves rebuild refuses without adoption or destructive cleanup would close that boundary gap.
  • Suggested test: rebuild-recovery-refusal-live

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: onboard-repair,onboard-resume,state-backup-restore,upgrade-stale-sandbox,sandbox-rebuild

[github-actions]

E2E Target Recommendation

Required E2E targets: onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox
Optional E2E targets: None

Dispatch required E2E targets:

• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume
• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=state-backup-restore
• gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=upgrade-stale-sandbox

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/test/6437-rebuild-live-recovery
Head: HEAD
Confidence: high

Required E2E targets

• onboard-repair: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-repair
• onboard-resume: Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=onboard-resume
• state-backup-restore: Upgrade, rebuild, snapshot, and restore operations must preserve user state while replacing stale runtime state.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=state-backup-restore
• upgrade-stale-sandbox: Upgrade, rebuild, snapshot, and restore operations must preserve user state while replacing stale runtime state.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field jobs=upgrade-stale-sandbox

Optional E2E targets

• None.

Relevant changed files

• src/lib/actions/sandbox/rebuild-pipeline.ts
• src/lib/actions/sandbox/rebuild-preflight-phase.ts
• src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts
• src/lib/actions/sandbox/rebuild-recovery-plan.ts
• src/lib/actions/sandbox/rebuild-recovery-preflight.ts
• src/lib/actions/sandbox/rebuild-recovery.ts
• src/lib/actions/sandbox/rebuild-transaction-coordinator.ts

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 7 test follow-ups

Action checklist

• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Runtime validation
• PRA-T5 Add or justify test follow-up: Runtime validation
• PRA-T6 Add or justify test follow-up: Acceptance clause
• PRA-T7 Add or justify test follow-up: Acceptance clause

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Run the `onboard-repair` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Changed files match deterministic `lifecycle-state` and `upgrade-rebuild` risk families. Static tests cover pure decisions, recovery preflight, transaction routing, receipt ordering, and identity-drift refusals, but live OpenShell/gateway/version/state-preservation behavior requires runtime validation.
• PRA-T2 Runtime validation — Run the `onboard-repair` E2E job to validate lifecycle-state convergence after repair paths: persisted metadata, reported status, and live sandbox/gateway state agree after recovery.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Changed files match deterministic `lifecycle-state` and `upgrade-rebuild` risk families. Static tests cover pure decisions, recovery preflight, transaction routing, receipt ordering, and identity-drift refusals, but live OpenShell/gateway/version/state-preservation behavior requires runtime validation.
• PRA-T3 Runtime validation — Run the `onboard-resume` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Changed files match deterministic `lifecycle-state` and `upgrade-rebuild` risk families. Static tests cover pure decisions, recovery preflight, transaction routing, receipt ordering, and identity-drift refusals, but live OpenShell/gateway/version/state-preservation behavior requires runtime validation.
• PRA-T4 Runtime validation — Run the `onboard-resume` E2E job to validate lifecycle-state retry convergence: partial failure and retry do not leave ghost resources or stale ports.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Changed files match deterministic `lifecycle-state` and `upgrade-rebuild` risk families. Static tests cover pure decisions, recovery preflight, transaction routing, receipt ordering, and identity-drift refusals, but live OpenShell/gateway/version/state-preservation behavior requires runtime validation.
• PRA-T5 Runtime validation — Run the `state-backup-restore` E2E job for Upgrade, rebuild, snapshot, and restore operations must preserve user state while replacing stale runtime state. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Changed files match deterministic `lifecycle-state` and `upgrade-rebuild` risk families. Static tests cover pure decisions, recovery preflight, transaction routing, receipt ordering, and identity-drift refusals, but live OpenShell/gateway/version/state-preservation behavior requires runtime validation.
• PRA-T6 Acceptance clause — Existing unit, integration, installer, and six-process-death suites remain green. — add test evidence or identify existing coverage. This advisor review does not evaluate live CI or external job status. Static evidence shows changed unit tests for the recovery decision, preflight, orchestrator, and coordinator boundaries; runtime/integration outcomes are handled by other PR surfaces.
• PRA-T7 Acceptance clause — E2E-advisor-required exact-head jobs are green. — add test evidence or identify existing coverage. This advisor review identifies the required runtime validation floor but does not report or evaluate external E2E job outcomes.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-1: Transaction startOrResume atomic operation replaces two-phase prepare+reconcile; then add or justify PRA-T1.
Open items: 1 required · 2 warnings · 2 suggestions · 6 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 5 new items found

Action checklist

• PRA-1 Fix: Transaction startOrResume atomic operation replaces two-phase prepare+reconcile in src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:155
• PRA-2 Resolve or justify: Registry restoration workaround retains mutable-fallback TOCTOU window in src/lib/actions/sandbox/rebuild-recovery.ts:155
• PRA-3 Resolve or justify: High overlap with 6 concurrent PRs on core rebuild transaction files in src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:1
• PRA-T1 Add or justify test follow-up: Runtime validation
• PRA-T2 Add or justify test follow-up: Runtime validation
• PRA-T3 Add or justify test follow-up: Runtime validation
• PRA-T4 Add or justify test follow-up: Runtime validation
• PRA-T5 Add or justify test follow-up: Exhaustive 120-combination test validates decision table completeness
• PRA-T6 Add or justify test follow-up: Registry restoration workaround in prepareRebuildRecoveryPreflight (rebuild-recovery.ts:145-160)
• PRA-4 In-scope improvement: Recovery plan type simplification loses explicit registryRestored tracking in src/lib/actions/sandbox/rebuild-recovery.ts:25
• PRA-5 In-scope improvement: Exhaustive 120-combination test validates decision table completeness in src/lib/actions/sandbox/rebuild-recovery.test.ts:12

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Required	correctness	src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:155	Verify the 5 test cases in rebuild-transaction-coordinator.test.ts cover all phase/stale/operation combinations from the risk plan invariants. Add explicit test for intentChanged/backupChanged rejection paths under startOrResume.
PRA-2	Resolve/justify	security	src/lib/actions/sandbox/rebuild-recovery.ts:155	Document the TOCTOU window explicitly. Consider making restoreRegistry atomic with a compare-and-swap on registry version, or gate restoration behind a lock. Track #6433 resolution.
PRA-3	Resolve/justify	architecture	src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:1	Coordinate merge order with PR authors. Ensure this PR rebases cleanly on latest main before merge. Run full rebuild E2E suite after merge to catch integration issues.
PRA-4	Improvement	correctness	src/lib/actions/sandbox/rebuild-recovery.ts:25	Verify all call sites correctly handle the implicit registryRestored. Consider adding a helper function or restoring explicit field if inference is error-prone.
PRA-5	Improvement	tests	src/lib/actions/sandbox/rebuild-recovery.test.ts:12	Keep this test as regression floor. Ensure it runs in CI on every rebuild-related change.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-1 Required — Transaction startOrResume atomic operation replaces two-phase prepare+reconcile

• Location: src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:155
• Category: correctness
• Problem: The new startOrResume() method consolidates transaction creation/refresh and stale-recovery deletion marking into one call. The old prepare() + reconcileObservedDeletion() sequence allowed intermediate validation; the new atomic path must preserve the same phase transition guarantees (prepared→old_deleted only on staleRecovery, no double-creation, intent/backup change detection still works).
• Impact: Incorrect phase routing could leave transactions in wrong state, causing duplicate destructive effects or stalled recoveries.
• Required action: Verify the 5 test cases in rebuild-transaction-coordinator.test.ts cover all phase/stale/operation combinations from the risk plan invariants. Add explicit test for intentChanged/backupChanged rejection paths under startOrResume.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read rebuild-transaction-coordinator.test.ts lines 14-35 and verify each case maps to risk plan invariant: 'partial failure and retry converge without ghost resources or stale ports'.
• Missing regression test: Add test case: startOrResume with recovered transaction at 'prepared' phase, staleRecovery=false, intentChanged=true should throw 'intent changed; refusing another destructive effect'.
• Done when: The required change is committed and verification passes: Read rebuild-transaction-coordinator.test.ts lines 14-35 and verify each case maps to risk plan invariant: 'partial failure and retry converge without ghost resources or stale ports'.
• Evidence: rebuild-transaction-coordinator.ts:155-195 (startOrResume implementation) rebuild-transaction-coordinator.test.ts:14-35 (5 routing cases) riskPlan.invariants[0]: 'partial failure and retry converge without ghost resources or stale ports'

Review findings by urgency: 1 required fix, 2 items to resolve/justify, 2 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-2 Resolve/justify — Registry restoration workaround retains mutable-fallback TOCTOU window

• Location: src/lib/actions/sandbox/rebuild-recovery.ts:155
• Category: security
• Problem: In prepareRebuildRecoveryPreflight, when decision.action === 'create' && !registryEntry, the code calls restoreRegistry(transaction.intent.source.registryRecovery). The comment acknowledges this is a repair to be removed when journal owns complete snapshot (Epic: make sandbox rebuild resumable across host-process interruption #6433). Between readRegistryEntry() returning null and restoreRegistry() writing, another process could create a conflicting registry entry.
• Impact: Race condition could cause registry corruption or recovery adopting wrong sandbox identity.
• Recommended action: Document the TOCTOU window explicitly. Consider making restoreRegistry atomic with a compare-and-swap on registry version, or gate restoration behind a lock. Track Epic: make sandbox rebuild resumable across host-process interruption #6433 resolution.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check rebuild-recovery.ts:145-160 for the restoreRegistry call and the Epic: make sandbox rebuild resumable across host-process interruption #6433 comment. Verify restoreRegistry implementation in registry.ts uses atomic write.
• Missing regression test: Add concurrent test: two processes calling prepareRebuildRecoveryPreflight simultaneously for same sandbox with missing registry — only one should succeed in restoration.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check rebuild-recovery.ts:145-160 for the restoreRegistry call and the Epic: make sandbox rebuild resumable across host-process interruption #6433 comment. Verify restoreRegistry implementation in registry.ts uses atomic write.
• Evidence: rebuild-recovery.ts:145-160 (prepareRebuildRecoveryPreflight registry restoration) rebuild-recovery-plan.ts (deleted) had same pattern with comment 'Source of truth: the durable journal; the registry is a mutable fallback'

PRA-3 Resolve/justify — High overlap with 6 concurrent PRs on core rebuild transaction files

• Location: src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:1
• Category: architecture
• Problem: Open PRs refactor(rebuild): resume transaction-owned replacements #6522, test(rebuild): prove live recovery boundaries #6574, refactor(rebuild): journal destructive recovery #6470, refactor(rebuild): unify post-restore verification and completion gating #6491, feat(state): add key-level restore ownership to agent manifests #6445, feat(agents): introduce Firecrawl web search support #6273 all modify rebuild-pipeline.ts, rebuild-preflight-phase.ts, rebuild-transaction-coordinator.ts, and recovery files. This PR's centralization of startOrResume and collapse of recovery-plan layer may conflict with peers' changes to transaction phases, recovery evidence, or completion gating.
• Impact: Merge conflicts or semantic conflicts could reintroduce bugs fixed in this PR or lose fixes from peer PRs.
• Recommended action: Coordinate merge order with PR authors. Ensure this PR rebases cleanly on latest main before merge. Run full rebuild E2E suite after merge to catch integration issues.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check gh pr view for each overlapping PR number to see current status and target branches. Verify git merge-base against main includes all peer fixes.
• Missing regression test: No automated test can catch cross-PR semantic conflicts. Requires manual integration verification after merge.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check gh pr view for each overlapping PR number to see current status and target branches. Verify git merge-base against main includes all peer fixes.
• Evidence: riskPlan.openPrOverlaps: 6 PRs with sameFiles covering all 11 changed files diffStat shows 11 files changed, all appear in at least one overlapping PR

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-4 Improvement — Recovery plan type simplification loses explicit registryRestored tracking

• Location: src/lib/actions/sandbox/rebuild-recovery.ts:25
• Category: correctness
• Problem: Old RebuildRecoveryPlan had explicit registryRestored boolean. New RebuildRecoveryAction string union derives replacementAlreadyPresent from action but registryRestored is now implicit (only true for 'create' when registry was missing). Callers in rebuild-preflight-phase.ts and rebuild-recovery-orchestrator.ts must infer correctly.
• Impact: Callers may incorrectly assume registryRestored state, affecting logging or downstream decisions.
• Suggested action: Verify all call sites correctly handle the implicit registryRestored. Consider adding a helper function or restoring explicit field if inference is error-prone.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check rebuild-preflight-phase.ts:110 (replacementAlreadyPresent = ['adopt', 'resume'].includes(recoveryPlan)) and rebuild-recovery-orchestrator.ts:38 (replacementAlreadyPresent getter).
• Missing regression test: Add test asserting registryRestored log message only emitted for 'create' action with missing registry entry.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rebuild-recovery.ts:25 (RebuildRecoveryAction type) rebuild-preflight-phase.ts:110 (replacementAlreadyPresent derivation) rebuild-recovery-orchestrator.ts:38 (replacementAlreadyPresent getter)

PRA-5 Improvement — Exhaustive 120-combination test validates decision table completeness

• Location: src/lib/actions/sandbox/rebuild-recovery.test.ts:12
• Category: tests
• Problem: New test iterates 2 phases × 4 live states × 5 registry observations × 3 session observations = 120 cases, asserting expected action for each. This provides high confidence the pure recovery decision logic matches the intended state machine.
• Impact: Positive: reduces risk of missing edge cases in recovery decision logic.
• Suggested action: Keep this test as regression floor. Ensure it runs in CI on every rebuild-related change.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Run vitest on rebuild-recovery.test.ts and verify 120 cases pass. Check cases constant equals 120.
• Missing regression test: None needed — test already exhaustive for pure function. Consider adding property-based test for decideRebuildRecovery to catch future refactors.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rebuild-recovery.test.ts:12-35 (exhaustive 120-case loop) riskPlan.invariants: 'status agrees with independently probed gateway and sandbox state'

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Run the `onboard-repair` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Deterministic regression risks require live validation: lifecycle-state and upgrade-rebuild Tier-2 families with 4 required E2E jobs (onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox). Unit tests strengthened but cannot fully validate cross-process recovery boundaries.
• PRA-T2 Runtime validation — Run the `onboard-resume` E2E job for Onboarding and sandbox state must converge across persisted metadata, reported status, and the live runtime. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Deterministic regression risks require live validation: lifecycle-state and upgrade-rebuild Tier-2 families with 4 required E2E jobs (onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox). Unit tests strengthened but cannot fully validate cross-process recovery boundaries.
• PRA-T3 Runtime validation — Run the `state-backup-restore` E2E job for Upgrade, rebuild, snapshot, and restore operations must preserve user state while replacing stale runtime state. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Deterministic regression risks require live validation: lifecycle-state and upgrade-rebuild Tier-2 families with 4 required E2E jobs (onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox). Unit tests strengthened but cannot fully validate cross-process recovery boundaries.
• PRA-T4 Runtime validation — Run the `upgrade-stale-sandbox` E2E job for Upgrade, rebuild, snapshot, and restore operations must preserve user state while replacing stale runtime state. Matched files: `src/lib/actions/sandbox/rebuild-pipeline.ts`, `src/lib/actions/sandbox/rebuild-preflight-phase.ts`, `src/lib/actions/sandbox/rebuild-recovery-orchestrator.ts`, `src/lib/actions/sandbox/rebuild-recovery-plan.ts`, `src/lib/actions/sandbox/rebuild-recovery-preflight.ts`.. Deterministic regression risks require live validation: lifecycle-state, upgrade-rebuild. Deterministic regression risks require live validation: lifecycle-state and upgrade-rebuild Tier-2 families with 4 required E2E jobs (onboard-repair, onboard-resume, state-backup-restore, upgrade-stale-sandbox). Unit tests strengthened but cannot fully validate cross-process recovery boundaries.
• PRA-T5 Exhaustive 120-combination test validates decision table completeness — Keep this test as regression floor. Ensure it runs in CI on every rebuild-related change.
• PRA-T6 Registry restoration workaround in prepareRebuildRecoveryPreflight (rebuild-recovery.ts:145-160) — rebuild-recovery-plan.test.ts 'restores source metadata only for an absent old-deleted replacement' (now in rebuild-recovery.ts test coverage); missing concurrent test for TOCTOU. rebuild-recovery.ts:157-165 comment 'Source of truth: the durable journal; the registry is a mutable fallback. Remove this repair when the journal owns the complete recovery snapshot or registry updates become atomic with transaction publication (Epic: make sandbox rebuild resumable across host-process interruption #6433).'

Since last review details

Current findings, using the urgency labels above:

PRA-1 Required — Transaction startOrResume atomic operation replaces two-phase prepare+reconcile

• Location: src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:155
• Category: correctness
• Problem: The new startOrResume() method consolidates transaction creation/refresh and stale-recovery deletion marking into one call. The old prepare() + reconcileObservedDeletion() sequence allowed intermediate validation; the new atomic path must preserve the same phase transition guarantees (prepared→old_deleted only on staleRecovery, no double-creation, intent/backup change detection still works).
• Impact: Incorrect phase routing could leave transactions in wrong state, causing duplicate destructive effects or stalled recoveries.
• Required action: Verify the 5 test cases in rebuild-transaction-coordinator.test.ts cover all phase/stale/operation combinations from the risk plan invariants. Add explicit test for intentChanged/backupChanged rejection paths under startOrResume.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read rebuild-transaction-coordinator.test.ts lines 14-35 and verify each case maps to risk plan invariant: 'partial failure and retry converge without ghost resources or stale ports'.
• Missing regression test: Add test case: startOrResume with recovered transaction at 'prepared' phase, staleRecovery=false, intentChanged=true should throw 'intent changed; refusing another destructive effect'.
• Done when: The required change is committed and verification passes: Read rebuild-transaction-coordinator.test.ts lines 14-35 and verify each case maps to risk plan invariant: 'partial failure and retry converge without ghost resources or stale ports'.
• Evidence: rebuild-transaction-coordinator.ts:155-195 (startOrResume implementation) rebuild-transaction-coordinator.test.ts:14-35 (5 routing cases) riskPlan.invariants[0]: 'partial failure and retry converge without ghost resources or stale ports'

PRA-2 Resolve/justify — Registry restoration workaround retains mutable-fallback TOCTOU window

• Location: src/lib/actions/sandbox/rebuild-recovery.ts:155
• Category: security
• Problem: In prepareRebuildRecoveryPreflight, when decision.action === 'create' && !registryEntry, the code calls restoreRegistry(transaction.intent.source.registryRecovery). The comment acknowledges this is a repair to be removed when journal owns complete snapshot (Epic: make sandbox rebuild resumable across host-process interruption #6433). Between readRegistryEntry() returning null and restoreRegistry() writing, another process could create a conflicting registry entry.
• Impact: Race condition could cause registry corruption or recovery adopting wrong sandbox identity.
• Recommended action: Document the TOCTOU window explicitly. Consider making restoreRegistry atomic with a compare-and-swap on registry version, or gate restoration behind a lock. Track Epic: make sandbox rebuild resumable across host-process interruption #6433 resolution.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check rebuild-recovery.ts:145-160 for the restoreRegistry call and the Epic: make sandbox rebuild resumable across host-process interruption #6433 comment. Verify restoreRegistry implementation in registry.ts uses atomic write.
• Missing regression test: Add concurrent test: two processes calling prepareRebuildRecoveryPreflight simultaneously for same sandbox with missing registry — only one should succeed in restoration.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check rebuild-recovery.ts:145-160 for the restoreRegistry call and the Epic: make sandbox rebuild resumable across host-process interruption #6433 comment. Verify restoreRegistry implementation in registry.ts uses atomic write.
• Evidence: rebuild-recovery.ts:145-160 (prepareRebuildRecoveryPreflight registry restoration) rebuild-recovery-plan.ts (deleted) had same pattern with comment 'Source of truth: the durable journal; the registry is a mutable fallback'

PRA-3 Resolve/justify — High overlap with 6 concurrent PRs on core rebuild transaction files

• Location: src/lib/actions/sandbox/rebuild-transaction-coordinator.ts:1
• Category: architecture
• Problem: Open PRs refactor(rebuild): resume transaction-owned replacements #6522, test(rebuild): prove live recovery boundaries #6574, refactor(rebuild): journal destructive recovery #6470, refactor(rebuild): unify post-restore verification and completion gating #6491, feat(state): add key-level restore ownership to agent manifests #6445, feat(agents): introduce Firecrawl web search support #6273 all modify rebuild-pipeline.ts, rebuild-preflight-phase.ts, rebuild-transaction-coordinator.ts, and recovery files. This PR's centralization of startOrResume and collapse of recovery-plan layer may conflict with peers' changes to transaction phases, recovery evidence, or completion gating.
• Impact: Merge conflicts or semantic conflicts could reintroduce bugs fixed in this PR or lose fixes from peer PRs.
• Recommended action: Coordinate merge order with PR authors. Ensure this PR rebases cleanly on latest main before merge. Run full rebuild E2E suite after merge to catch integration issues.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check gh pr view for each overlapping PR number to see current status and target branches. Verify git merge-base against main includes all peer fixes.
• Missing regression test: No automated test can catch cross-PR semantic conflicts. Requires manual integration verification after merge.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check gh pr view for each overlapping PR number to see current status and target branches. Verify git merge-base against main includes all peer fixes.
• Evidence: riskPlan.openPrOverlaps: 6 PRs with sameFiles covering all 11 changed files diffStat shows 11 files changed, all appear in at least one overlapping PR

PRA-4 Improvement — Recovery plan type simplification loses explicit registryRestored tracking

• Location: src/lib/actions/sandbox/rebuild-recovery.ts:25
• Category: correctness
• Problem: Old RebuildRecoveryPlan had explicit registryRestored boolean. New RebuildRecoveryAction string union derives replacementAlreadyPresent from action but registryRestored is now implicit (only true for 'create' when registry was missing). Callers in rebuild-preflight-phase.ts and rebuild-recovery-orchestrator.ts must infer correctly.
• Impact: Callers may incorrectly assume registryRestored state, affecting logging or downstream decisions.
• Suggested action: Verify all call sites correctly handle the implicit registryRestored. Consider adding a helper function or restoring explicit field if inference is error-prone.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check rebuild-preflight-phase.ts:110 (replacementAlreadyPresent = ['adopt', 'resume'].includes(recoveryPlan)) and rebuild-recovery-orchestrator.ts:38 (replacementAlreadyPresent getter).
• Missing regression test: Add test asserting registryRestored log message only emitted for 'create' action with missing registry entry.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rebuild-recovery.ts:25 (RebuildRecoveryAction type) rebuild-preflight-phase.ts:110 (replacementAlreadyPresent derivation) rebuild-recovery-orchestrator.ts:38 (replacementAlreadyPresent getter)

PRA-5 Improvement — Exhaustive 120-combination test validates decision table completeness

• Location: src/lib/actions/sandbox/rebuild-recovery.test.ts:12
• Category: tests
• Problem: New test iterates 2 phases × 4 live states × 5 registry observations × 3 session observations = 120 cases, asserting expected action for each. This provides high confidence the pure recovery decision logic matches the intended state machine.
• Impact: Positive: reduces risk of missing edge cases in recovery decision logic.
• Suggested action: Keep this test as regression floor. Ensure it runs in CI on every rebuild-related change.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Run vitest on rebuild-recovery.test.ts and verify 120 cases pass. Check cases constant equals 120.
• Missing regression test: None needed — test already exhaustive for pure function. Consider adding property-based test for decideRebuildRecovery to catch future refactors.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: rebuild-recovery.test.ts:12-35 (exhaustive 120-case loop) riskPlan.invariants: 'status agrees with independently probed gateway and sandbox state'

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29033782610
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: onboard-repair
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
onboard-repair	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29033782708
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: onboard-resume
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
onboard-resume	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29033782690
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: state-backup-restore
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
state-backup-restore	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29033782685
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: upgrade-stale-sandbox
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
upgrade-stale-sandbox	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29034110518
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: onboard-resume
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
onboard-resume	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29034110604
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: onboard-repair
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
onboard-repair	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29034110505
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: state-backup-restore
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
state-backup-restore	✅ success

[github-actions]

E2E Target Results — ✅ All requested jobs passed

Run: 29034110486
Workflow ref: refactor/6580-rebuild-recovery-consolidation
Requested targets: (default — all supported)
Requested jobs: upgrade-stale-sandbox
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job	Result
upgrade-stale-sandbox	✅ success

[jyaunches]

Closing this draft stack as requested while the broader human-and-agent lifecycle architecture is reconsidered.