fix(cli): fail closed on a malformed sandbox registry file

[laitingsheng]

Summary

A sandboxes.json file that was present but held invalid JSON was read as an empty registry, so the next registry mutation atomically rewrote the file and dropped every recorded sandbox without reporting an error. Reading a malformed configuration file now fails with the file path and recovery commands, and the file on disk is left exactly as it was found. A missing file still returns the first-run fallback.

Related Issue

Fixes #8420

Changes

• readConfigFile in src/lib/state/config-io.ts now separates the read from the parse. Permission failures still raise ConfigPermissionError, ENOENT alone returns the caller's fallback, any other read failure propagates, and a parse failure raises the new ConfigCorruptError (ECONFIGCORRUPT).
• ConfigCorruptError carries the file path and a remediation block that copies the file aside and removes it. It drops the JSON.parse error instead of keeping it as cause, because Node can quote the offending input and configuration files hold state that must stay out of diagnostics.
• Nothing is renamed, moved, or backed up while reading. The malformed bytes stay in place, so the failure repeats identically for every later read and every other process, and there is no read-to-rename window that could move a concurrent writer's valid state.
• registry.load() is the only caller of readConfigFile, so registry mutations such as registerSandbox now fail inside the registry lock before save() runs.
• test/registry.test.ts covers the reported reproduction: the read reports the damage, registerSandbox refuses and leaves the file byte-identical with no lock directory or temp file behind, four separate reader processes all exit with ECONFIGCORRUPT while the file stays unchanged, and a repaired file registers normally again. The existing handles corrupt registry file gracefully test encoded the old behavior and was replaced.
• src/lib/state/config-io.test.ts covers the parse-failure contract directly: repeated reads keep failing with the file and its directory untouched, the error names the path and the recovery commands while a planted secret value never reaches the serialized error, a malformed file behind a symlinked final component fails without touching link or target, and a present path that is not a readable file throws.
• docs/reference/host-files-and-state.mdx adds Malformed Registry File with the recovery steps and the non-default gateway-port path; docs/reference/troubleshooting.mdx adds the matching symptom entry.

Some read-only display paths keep their own catch around the registry, such as safeListRegistryEntries in src/lib/status-command-deps.ts and the sandbox-name probe in src/lib/diagnostics/debug.ts. Those still degrade to an empty list or a gateway probe. They persist nothing, so they cannot cause the reported data loss, and this change leaves them as they are.

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:
• 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: Maintainer review passed all nine security categories for the current branch revision; no findings.
• 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: docs-updated
• Evidence: docs/reference/host-files-and-state.mdx, docs/reference/troubleshooting.mdx. The review ran against the completed diff and the writing rules in docs/AGENTS.md, WRITING.md, and the controlled word list. Its findings were applied: the claim about affected commands was narrowed to the registry readers and writers that actually stop, the unsupported rebuild-backups/ recovery route was removed, the data-loss warning was moved ahead of the rm command, the shell fence language was corrected, one term (malformed) is now used for the concept, the frontmatter routing fields and the sandboxes.json table row were updated, a troubleshooting symptom entry was added, and the error and remediation strings were reworded. The independent current-branch review also corrected the optional messaging health fallback wording in both pages. A post-refresh review remained PASS after the conflict-free main merge.
• 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 validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/state 526/526 pass; npx vitest run --project cli src/lib/state/config-io.test.ts 22/22 pass after the rebase; npx vitest run --project integration test/registry.test.ts test/registry-default-selection-revision.test.ts 84/84 pass, and test/registry.test.ts 74/74 pass again after the rebase; after the main refresh, src/lib/state/config-io.test.ts passed 22/22 and test/registry.test.ts passed 77/77.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs passes with 0 errors; 2 existing warnings remain (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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Invalid registry files now produce a clear invalid-JSON error instead of being treated as empty.
  • Malformed files are preserved without being overwritten, replaced, or leaving temporary artifacts.
  • Registry-dependent commands fail safely, while optional health checks continue without registry details.
  • Commands can recover normally after the registry file is repaired.

• Documentation

  • Added troubleshooting and reference guidance for detecting and recovering from malformed registry files.

[coderabbitai]

📝 Walkthrough

Walkthrough

The configuration reader now reports malformed JSON instead of using a fallback. Registry operations preserve corrupted files and fail explicitly. Tests cover corruption, concurrency, recovery, symlinks, and path handling. Documentation describes backup, removal, retry, and re-onboarding steps.

Changes

Malformed registry handling

Layer / File(s)	Summary
Config corruption contract src/lib/state/config-io.ts, src/lib/state/config-io.test.ts	ConfigCorruptError reports sanitized path and remediation metadata. Malformed JSON throws after successful file I/O. Tests verify unchanged files, symlinks, repeated failures, and unreadable directories.
Registry fail-closed validation test/registry.test.ts	Registry reads and mutations fail on invalid JSON. Tests verify preserved bytes, no lock or temporary-file residue, consistent concurrent ECONFIGCORRUPT errors, and recovery after repair.
Recovery documentation docs/reference/host-files-and-state.mdx, docs/reference/troubleshooting.mdx	Documentation describes malformed registry errors, backup and removal steps, retry commands, and re-onboarding guidance.

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

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the fail-closed behavior, preserve corrupted files, prevent unsafe mutations, add recovery guidance, and provide regression coverage for issue #8420.
Out of Scope Changes check	✅ Passed	The implementation, tests, and documentation changes directly support the requirements in issue #8420; no unrelated changes are identified.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: malformed sandbox registry files now fail closed.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/registry-fail-closed-on-malformed-json

---

_{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 in commit 02a8035 in the fix/registry-fail-cl... branch remains at 96%, unchanged from commit e323de1 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 02a8035 in the fix/registry-fail-cl... branch remains at 81%, unchanged from commit e323de1 in the main branch.

Show a code coverage summary of the most impacted files.

File	main e323de1	fix/registry-fail-cl... 02a8035	+/-
src/lib/domain/.../connect-env.ts	97%	89%	-8%
src/lib/actions...air-approval.ts	90%	89%	-1%
src/lib/inferen...ompatibility.ts	94%	94%	0%
src/lib/messagi...an-authority.ts	100%	100%	0%
src/lib/sandbox...rce-identity.ts	87%	87%	0%
src/lib/state/registry.ts	90%	90%	0%
src/lib/state/config-io.ts	93%	95%	+2%
src/lib/inferen...a/model-size.ts	83%	96%	+13%

---

_{Updated August 06, 2026 14:39 UTC}

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-8443.docs.buildwithfern.com/nemoclaw

[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 terminology decisions differ; normalized E2E selections match; severity counts match.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

2 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• established — malformed registry file at docs/reference/host-files-and-state.mdx:55: Use “malformed registry file” consistently for a present sandbox registry file that fails JSON parsing.
• justified — registry-derived information at docs/reference/host-files-and-state.mdx:60: Keep the modifier because it identifies the optional output omitted when the registry cannot be read.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

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.

[apurvvkumaria]

Addressed PRA-1 on the current branch revision (342fc07).

• Narrowed both malformed-registry pages to operations that require complete sandbox records, such as nemoclaw list and nemoclaw onboard.
• Documented the intentional fallback precisely: optional messaging health checks omit registry-derived information when they cannot read the registry.
• npm run docs passes with 0 errors; 2 existing warnings remain. Commit hooks, secret scanning, commit lint, and push hooks passed.
• Independent documentation writer review: PASS after the wording refinement above.
• Maintainer security review: PASS across secrets, input handling, authentication, dependencies, data exposure, cryptography, authorization boundaries, test coverage, and fail-closed behavior. No findings.

Fresh CI and automated review are now running. The PR still needs the required independent approval before merge.

[apurvvkumaria]

Post-review refresh completed.

• Merged current main conflict-free while preserving the contributor commits; the PR-relative diff remains limited to the same five intended files.
• The signed merge commit is GitHub Verified.
• Post-refresh validation passed: npm run docs (0 errors; 2 existing warnings), config I/O tests (22/22), and registry tests (77/77).
• The independent documentation writer repeated the review after the refresh and returned PASS. The documentation and security receipt metadata now matches the current branch revision.

Fresh required checks are queued or running. Independent approval is still required before merge.

[coderabbitai]

🧹 Nitpick comments (1)

test/registry.test.ts (1)

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

Use an ESM import for child_process.

Line 1428 uses CommonJS require() in a root-level test. Import spawnSync from node:child_process at module scope and remove this local require().

As per coding guidelines, root-level tests must use ESM imports.

🤖 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/registry.test.ts` at line 1428, Replace the local CommonJS require in
the root-level test with a module-scope ESM import of spawnSync from
node:child_process, and remove the require declaration from the test body.

Source: Coding guidelines

🤖 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/registry.test.ts`:
- Line 1428: Replace the local CommonJS require in the root-level test with a
module-scope ESM import of spawnSync from node:child_process, and remove the
require declaration from the test body.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 0f2fbb34-acae-4875-93be-b3b2bb68982b

📥 Commits

Reviewing files that changed from the base of the PR and between 342fc07 and 02a8035.

📒 Files selected for processing (1)

• test/registry.test.ts