fix(archive): report retirement cleanup failures accurately

[Marzx13]

When a retirement backup disappears or changes after a change is archived, the cleanup error can say that every backup was retained. JSON also reduces this recovery state to the generic archive_error, making it difficult for callers to distinguish it from other failures.

This PR reports archive_retirement_cleanup_failed and corrects the message to state that archiving occurred but retirement cleanup did not complete. Recovery guidance covers all reported paths, including a staged source left by fallback-copy cleanup, without promising that those paths still exist or contain their original bytes. The agent contract documents the diagnostic and explains that archive: null does not guarantee unchanged files.

Exit 1, the failure envelope, underlying error details, and transaction behavior are preserved. Consumers matching only archive_error for this failure should account for the more precise code. No automatic cleanup or retry is added. This is separate from the archive-claim lock release addressed by #1769.

Validation

• CI on 76a0e75 passes the full test suite on Linux, macOS, and Windows, plus build, lint, type checks, and release tracking. Security checks also pass.
• Seven focused regression cases pass, covering human/JSON output, pre-mutation failure, edited/replaced/disappeared backups, and combined source/backup cleanup failures.
• An aliased Windows temporary path reproduced the five initial CI failures. Canonicalizing the fixture made all five pass, with assertions confirming each fault injection fired. The full archive file also passes locally: 197 tests, 24 existing Windows skips.
• Sixteen built-CLI fault-injection scenarios pass across local roots and selected stores; twelve retry checks preserve surviving file contents.
• Before submission, the local Windows full suite had 4,346 passing tests, 79 skips, and one unchanged Git-source packaging test blocked by npm 12.0.1 with EALLOWGIT (allow-git: none). That packaging test and npm restriction were not changed; the hosted Windows suite now passes.

Generated and reviewed with Codex using gpt-6-astra with xhigh reasoning.

Summary by CodeRabbit

• Bug Fixes
  • Archive cleanup failures now have a specific diagnostic and recovery guidance. Reports clarify when changes were archived despite a cleanup failure.
• Documentation
  • Archive guidance now explains that a null result does not guarantee files are unchanged, and recommends inspecting affected paths before cleanup or retrying.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

test/AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: Fission-AI/OpenSpec/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ef3afb21-3eaf-43af-9587-8163d6099340

📥 Commits

Reviewing files that changed from the base of the PR and between 3a34ea3 and c82d703.

📒 Files selected for processing (3)

• docs/agent-contract.md
• src/core/archive.ts
• test/core/archive.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

Archive retirement cleanup failures now use a dedicated error and JSON diagnostic. Documentation describes the diagnostic and recovery considerations. Tests cover claim failures, retained or changed backups, and combined cleanup failures.

Changes

Archive cleanup failures

Layer / File(s)	Summary
Cleanup error and diagnostic contract src/core/archive.ts, docs/agent-contract.md, test/core/archive.test.ts	Retirement cleanup errors map to archive_retirement_cleanup_failed. Documentation explains that a null archive result does not confirm that files are unchanged and advises inspecting affected paths. Tests check human and JSON output, retained backups, and claim failures before mutation.
Retirement cleanup failure handling src/core/archive.ts, test/core/archive.test.ts	Finalization failures report that the change was archived but cleanup did not complete. Tests cover fallback retirement, concurrent backup edits or removal, and combined source and backup cleanup failures.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c82d7

The change distinguishes completed archiving from incomplete retirement cleanup and provides recovery guidance without adding automatic cleanup. No merge-blocking issue is established; consumers matching only archive_error should accommodate the new diagnostic.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to c82d7

The dedicated failure code and recovery guidance do not change filesystem operations, access controls, or rollback behavior. No introduced or worsened security issue was identified in the reviewed paths.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The reviewed change affects archive callers and their interpretation of recovery state within the selected project or store root. The inspected base/head implementation diff introduces no additional filesystem operation, caller capability, or path-selection authority.

Trust Boundaries and Controls

• observed — The cleanup error rename does not bypass archive-claim ownership controls. Claim release remains in the run-level finally path and verifies claim identity and contents before removal. This covers handled failures and normal unwinding, not abrupt process termination.

Resilience and Maintainability Implications

• observed — The new diagnostic directs callers to inspect all recovery paths and preserve needed content rather than treating a failed command as an unchanged filesystem or assuming every backup is intact. It clarifies recovery ownership without adding destructive automated recovery.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 …	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 and concisely describes the main change: accurate reporting of archive retirement cleanup failures.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[alfred-openspec]

Reviewed the archive failure classification, recovery messaging, transaction paths, and fault-injection coverage. The new diagnostic accurately distinguishes committed archives with incomplete retirement cleanup, and CI is green.