fix(state): prevent partial rebuild recovery manifests

[HOYALIM]

Summary

Rebuild snapshots currently write their recovery manifest directly to the final path, so an interrupted write can expose truncated state as if publication had completed. Write the complete private manifest to a same-directory temporary file and atomically rename it into place, while preserving the original publish error if cleanup also fails.

Changes

• publish rebuild-manifest.json only after a complete 0600 temporary write
• use exclusive temporary-file creation and same-directory rename semantics
• remove unpublished temporary state after write or rename failures without masking the primary failure
• cover successful publication, failed commit cleanup, and cleanup-error precedence

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: internal snapshot publication semantics only; CLI commands, output, configuration, and documented recovery semantics are unchanged.
• 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: Nine-category exact-head security review passed; future exact head f6d6d2a84 revalidated after the current-main merge and test-title clarification; the atomic publication implementation is unchanged.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: The exact-head diff only hardens internal rebuild-manifest publication and cleanup; the current-main merge has no overlap with either changed file. It does not change commands, output, configuration, or documented recovery semantics. Changed comments and test titles follow WRITING.md. Targeted tests passed 3/3, and CLI type-checking passed.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line 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 — npx vitest run --project cli src/lib/state/sandbox-manifest-publish.test.ts src/lib/actions/sandbox/rebuild-backup-phase.test.ts src/lib/actions/sandbox/rebuild-resume-snapshot.test.ts src/lib/state/sandbox-state-file-restore-contract.test.ts (23 passed)
• Applicable broad gate passed — npm run typecheck:cli -- --incremental
• 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: Ho Lim subhoya@gmail.com

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

📝 Walkthrough

Walkthrough

Manifest publication now writes through a private temporary file, atomically renames it into place, cleans up failed temporary files, and preserves primary errors. Vitest coverage validates contents, permissions, visible files, cleanup, and failure semantics.

Changes

Manifest publication

Layer / File(s)	Summary
Atomic manifest publish flow src/lib/state/sandbox.ts	writeManifest writes a PID-scoped private temporary file with exclusive-create semantics, renames it to rebuild-manifest.json, removes failed temporary files, and exposes the function for tests.
Publication behavior tests src/lib/state/sandbox-manifest-publish.test.ts	Tests validate manifest contents, 0600 permissions, absence of temporary files after success, cleanup after rename failure, and preservation of the original write failure.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: copilot, brandonpelfrey, apurvvkumaria

Sequence Diagram(s)

sequenceDiagram
  participant writeManifest
  participant ManifestPublishOps
  participant Filesystem
  writeManifest->>ManifestPublishOps: write temporary manifest
  ManifestPublishOps->>Filesystem: create private temp file
  writeManifest->>ManifestPublishOps: rename temporary file
  ManifestPublishOps->>Filesystem: publish rebuild-manifest.json
  writeManifest->>ManifestPublishOps: remove temporary file on failure

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing rebuild manifest publication to prevent partial or truncated recovery manifests.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[prekshivyas]

Security review: PASS at head 331f804

1. Secrets and credentials — PASS. The serialized manifest contract is unchanged and the file is created with mode 0600.
2. Input and path validation — PASS. Publication remains confined to the existing validated backup directory; no user-controlled destination is added.
3. Authentication and authorization — PASS. No authentication or authorization behavior changes.
4. Dependencies and supply chain — PASS. No dependencies or generated artifacts are added.
5. Error handling and logging — PASS. Failed writes or renames remove unpublished temporary state, and a cleanup failure cannot replace the primary publish error.
6. Cryptography — PASS / not applicable. No cryptographic behavior changes.
7. Configuration and environment — PASS. No configuration or environment behavior changes.
8. Testing — PASS. The focused tests pass 3/3 and CLI typechecking passes after the current-main merge. Coverage checks complete private publication, absence of visible temporary files, commit-failure cleanup, and primary-error precedence.
9. System security — PASS. Exclusive temporary-file creation prevents clobbering, same-directory atomic rename prevents readers from accepting a partial recovery manifest, and the final file remains private.

No blocking security finding remains. Final approval should wait for required CI and automated review checks on this exact head.

[prekshivyas]

Exact-head ordinary CI is green, but credentialed E2E is blocked by the repository's fork-approval environment configuration.

• Head: 331f8046fedba53293f1ef7ef0ce1264f36344e4
• Base: eeab81cc5542902538c97db63c132c0fdbd4341c
• Controller: https://github.com/NVIDIA/NemoClaw/actions/runs/30422131215
• Selected plan: onboard-repair, onboard-resume

The controller refused to start the plan because approve-credentialed-e2e-for-fork-pr has no required-reviewer protection rules, so GitHub recorded no protected approval. No selected E2E ran and no repository credential was exposed. A repository admin must configure required reviewers; the workflow then requires a new PR head and fresh CI. I have not weakened or changed the environment.

[cjagwani]

Exact-head security receipt for 649322858 (diff fingerprint 8f75549dd0688db63314f23138bd854bfb77413970ff69b9a3bd881a5defb91d): PASS. The only delta since the full nine-category review at 331f8046f is the verified merge of current main; it does not touch src/lib/state/sandbox.ts or src/lib/state/sandbox-manifest-publish.test.ts. The private 0600 exclusive temporary write, same-directory atomic rename, cleanup ordering, and primary-error preservation remain unchanged. No new findings across credentials, path validation, authorization, dependencies, error handling, data protection, configuration, testing, or system security.

[cjagwani]

Approved exact head 6493228584d890d2272d2b1635332896182a6ada after the maintainer gate. The reviewed state-recovery patch has no remaining security blocker; all commits are validly signed, the primary advisor is green, no unresolved current review threads remain, and the trusted protected E2E plan completed successfully (including onboard repair/resume coverage). Superseded cancelled runs were not treated as current evidence.

[cjagwani]

Re-approved exact head fc153405194841238d08bbcb150460da438572d9 after the one-hour quiet check. The only post-approval code delta is a verified one-line behavior-test title correction (commit fails → rename fails); production behavior and the prior nine-category security receipt are unchanged. Current normal CI, protected E2E, both advisors, signatures, documentation receipt, and all review threads are clean.

[cjagwani]

Exact-head approval after refresh onto current main. Deterministic maintainer gate passes with all 40 current checks green, clean merge state, verified history, and no unresolved major findings. Protected onboard-repair and onboard-resume both passed on this head.

[prekshivyas]

Approved exact head f6d6d2a84dfc23dbebd7865cc5fb1729e59b7240. This head adds only a signed/Verified empty gate-refresh over signed/Verified merge 0cd15f5b2; the reviewed two-file atomic-manifest product patch is unchanged. All 40 exact-head checks are green; trusted onboard-repair and onboard-resume E2E passed against reserved base 376beb50b6d184675283bdbc4f2eca18d7200a86; all 7 commits are Verified; exact-head documentation writer result is no-docs-needed; security review passes; no unresolved threads; GitHub reports CLEAN/MERGEABLE. Maintainer base-freshness waiver applied per direction: current main advanced only by docs release entry da1b10312, has zero path overlap, and git merge-tree is clean, so no refresh is needed.