fix(backup): name the per-dir failure cause in backup failure messages

[Hokonoken]

Summary

backup-all and snapshot create collapse every per-directory backup failure into one opaque list — backup failed (identity, devices, credentials) — which cannot distinguish a permission problem (tar cannot read content the backup user does not own) from a directory that never materialized from the extraction. These are different operator problems: the first points at whatever wrote sandbox state with the wrong ownership; the second at the archive pipeline. This PR records a per-directory failure cause and names it in the failure message, as proposed in the #6455 triage.

Related Issue

Addresses #6455

Changes

• backupSandboxState (src/lib/state/sandbox.ts) records an optional failedDirReasons map alongside failedDirs, populated at the three attribution points: permission denied (tar reported Permission denied for the dir), tar read error (other tar read errors), absent after extraction (tar succeeded but the dir never materialized on the host). failedDirs keeps its shape for existing consumers.
• failedDirsFromTarStderr returns the per-dir cause instead of a bare set; matching logic unchanged.
• New exported formatFailedBackupItems renders identity (permission denied); items without an attributable cause render unchanged.
• backup-all (src/lib/actions/maintenance.ts) and snapshot create (src/lib/actions/sandbox/snapshot.ts) use it in their failure messages.

Before → after on the failing clone from #6455:

✗ clone-test: backup failed (identity, devices, credentials)
✗ clone-test: backup failed (identity (permission denied), devices (permission denied), credentials (permission denied))

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: error-message wording and an optional additive result field; no documented workflow or user-facing behavior contract changes.
• 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: requested — src/lib/state/sandbox.ts is on the sandbox path; the change is additive diagnostics (no change to what is backed up, audited, or extracted). Awaiting maintainer review.
• 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 — command/result: npx vitest run --project cli src/lib/actions/maintenance.test.ts → 19 passed (2 new: cause rendering via the real formatter, and unchanged rendering when no cause is recorded); npx vitest run --project integration test/snapshot.test.ts → 48 passed (2 existing partial-tar tests extended with failedDirReasons assertions covering both permission denied and absent after extraction).
• Applicable broad gate passed — justification: change is confined to backup failure attribution and message rendering; no runtime harness or repo-wide validation surface is altered.
• 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)

Evidence

Verified live against a Docker sandbox stack (Ubuntu 24.04/WSL2, OpenShell CLI 0.0.72, main), using the construction from the #6455 triage: a snapshot-restored clone (snapshot restore baseline --to clone-test) with chown -R root:root applied to identity, devices, and credentials:

$ nemoclaw backup-all          # exit measured with no pipe
  ✗ clone-test: backup failed (identity (permission denied), devices (permission denied), credentials (permission denied))
  Pre-upgrade backup: 1 backed up, 1 failed, 8 skipped
BACKUP-ALL EXIT=1

Restoring ownership (chown -R 998:998) returns the same stack to a clean pass — no false positive from the new attribution:

  ✓ clone-test: 14 dirs, 1 files → ~/.nemoclaw/rebuild-backups/clone-test/2026-07-10T08-59-40-080Z
  Pre-upgrade backup: 2 backed up, 0 failed, 8 skipped
BACKUP-ALL EXIT=0

The absent after extraction cause is pinned by the extended integration test (marks non-attributed directories failed when they are missing from partial extraction), which asserts { agents: "permission denied", workspace: "absent after extraction" } from one partial-tar run.

---

Signed-off-by: Hokonoken 41166525+Hokonoken@users.noreply.github.com

Summary by CodeRabbit

• Bug Fixes

  • Improved backup and snapshot failure reporting by showing each failed directory/file with a specific failure cause (e.g., permission denied, tar read error, absent after extraction).
  • Updated console output formatting to include per-directory reasons when they’re available.

• Tests

  • Extended backup and snapshot test coverage with mixed backup outcomes and new assertions for the populated failure-cause mappings and rendered messages.
  • Added domain-level test coverage for failure classification and formatting behavior.

[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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a5766ef5-e774-4020-a234-c341278fa62b

📥 Commits

Reviewing files that changed from the base of the PR and between 2166152 and 27ad9f2.

📒 Files selected for processing (6)

• src/lib/actions/maintenance.test.ts
• src/lib/actions/maintenance.ts
• src/lib/actions/sandbox/snapshot.test.ts
• src/lib/actions/sandbox/snapshot.ts
• src/lib/domain/backup-failure.test.ts
• src/lib/domain/backup-failure.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/lib/actions/sandbox/snapshot.ts
• src/lib/actions/maintenance.ts

---

📝 Walkthrough

Walkthrough

Backup failures now retain per-directory causes, including permission errors, tar read errors, and missing directories after extraction. Backup-all and snapshot creation format these causes in failure messages, with tests covering annotated and unannotated output.

Changes

Backup failure reporting

Layer / File(s)	Summary
Collect and format failure reasons src/lib/domain/backup-failure.ts, src/lib/state/sandbox.ts	BackupResult records per-directory failure causes, tar errors are categorized, and failed items can be rendered with their reasons.
Integrate formatted failure messages src/lib/actions/maintenance.ts, src/lib/actions/sandbox/snapshot.ts	Backup-all and snapshot creation use formatted failed-item output instead of comma-joined directory lists.
Validate failure causes and output src/lib/domain/backup-failure.test.ts, src/lib/actions/maintenance.test.ts, src/lib/actions/sandbox/snapshot.test.ts, test/snapshot.test.ts	Tests verify failure reasons and messages with or without recorded causes.

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

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding per-directory failure causes to backup failure messages.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/state/sandbox.ts (1)

774-786: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider placing formatFailedBackupItems in a domain module rather than the state layer.

This is a pure, I/O-free decision/formatting function shared by two action files (maintenance.ts, snapshot.ts), but it lives in src/lib/state/sandbox.ts. As per path instructions, src/lib/{actions,domain,adapters,state}/** should be reviewed against layer ownership where "domain modules make pure decisions" while "state modules own persisted files and state I/O." Moving this helper to a domain module (keeping BackupResult/failedDirReasons types in state) would better align with that boundary; not blocking given the pragmatic co-location with BackupResult.

🤖 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/state/sandbox.ts` around lines 774 - 786, The pure formatting helper
formatFailedBackupItems is placed in the state layer despite having no state or
I/O responsibilities. Move it to an appropriate domain module, update
maintenance.ts and snapshot.ts imports to use the new location, and leave
BackupResult and failedDirReasons state types in sandbox.ts.

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/state/sandbox.ts`:
- Around line 774-786: The pure formatting helper formatFailedBackupItems is
placed in the state layer despite having no state or I/O responsibilities. Move
it to an appropriate domain module, update maintenance.ts and snapshot.ts
imports to use the new location, and leave BackupResult and failedDirReasons
state types in sandbox.ts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1ede3c3b-158e-414f-bc79-16ad4766d48e

📥 Commits

Reviewing files that changed from the base of the PR and between 9dec1fb and a8ad1ee.

📒 Files selected for processing (5)

• src/lib/actions/maintenance.test.ts
• src/lib/actions/maintenance.ts
• src/lib/actions/sandbox/snapshot.ts
• src/lib/state/sandbox.ts
• test/snapshot.test.ts

[github-actions]

E2E Advisor Recommendation

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

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: target/main
Head: HEAD
Confidence: high

Required E2E

• onboard-repair: Required by deterministic risk plan for lifecycle-state: sandbox state convergence across persisted metadata, reported status, and live runtime may be affected by snapshot/state changes.
• onboard-resume: Required by deterministic risk plan for lifecycle-state: retry/resume paths must not leave ghost resources or stale sandbox state after state/snapshot mutations.
• state-backup-restore: Required by deterministic risk plan for upgrade-rebuild: backup failure classification changes what is preserved in manifests and restored into a sandbox.
• upgrade-stale-sandbox: Required by deterministic risk plan for upgrade-rebuild: pre-upgrade backup and failed mutation retry behavior must preserve user state while replacing stale runtime state.
• snapshot-commands: Direct snapshot CLI behavior changed via failed backup diagnostics and BackupResult.failedDirReasons; run the live snapshot command flow to validate create/restore integration.

Optional E2E

• None.

New E2E recommendations

• backup-failure-diagnostics (medium): Existing live backup/snapshot tests may not force tar partial-read and permission-denied stderr from a real sandbox to verify user-facing failedDirReasons diagnostics and restorable-manifest exclusion in E2E.
  • Suggested test: Add a live E2E case that creates an unreadable state directory in a sandbox, runs backup/snapshot, asserts the permission-denied diagnostic, and verifies restore skips only the failed directory.

Dispatch hint

• Workflow: E2E
• jobs input: onboard-repair,onboard-resume,state-backup-restore,upgrade-stale-sandbox,snapshot-commands

[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: target/main
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/maintenance.ts
• src/lib/actions/sandbox/snapshot.ts
• src/lib/domain/backup-failure.ts
• src/lib/state/sandbox.ts

[cv]

Maintainer follow-up at exact head d5f91e7:

• Corrected linkage from Fixes [Ubuntu 24.04][CLI&UX] nemoclaw backup-all exits 1 when a cloned sandbox fails the pre-backup identity audit #6455 to Addresses [Ubuntu 24.04][CLI&UX] nemoclaw backup-all exits 1 when a cloned sandbox fails the pre-backup identity audit #6455. This PR improves failure attribution but intentionally preserves the nonzero backup failure behavior, so [Ubuntu 24.04][CLI&UX] nemoclaw backup-all exits 1 when a cloned sandbox fails the pre-backup identity audit #6455 remains open for remediation/success-skip policy.
• Added the missing generic tar-read-error regression. The integration now proves permission-denied and generic read errors are separately classified, excluded from the restorable manifest, and not removed during restore; absent-after-extraction coverage remains intact.
• Synced current main with a signed, conflict-free merge.
• Merged-head verification: 48 integration tests, 65 CLI tests, CLI typecheck, and normal pre-commit/commit-msg/pre-push hooks passed. Both maintainer commits are GitHub Verified and DCO-signed-off.
• Documentation review found no update necessary because commands, failure semantics, and operator actions are unchanged.
• Required exact-head lifecycle E2E is running: https://github.com/NVIDIA/NemoClaw/actions/runs/29146138590
• Trusted GPT/Nemotron review is running: https://github.com/NVIDIA/NemoClaw/actions/runs/29146132420

No merge action taken.

[cv]

Approved on exact head 27ad9f2: required CI and trusted E2E are green, CodeRabbit has no unresolved threads, both trusted advisors recommend merge as-is, and DCO/signature verification passes.