fix(archive): refuse delta files the merge path never reads

[dwin-gharibi]

Closes #1869.

Why

validate and archive read a change's deltas only from
specs/<capability-path>/spec.md (discoverSpecFiles). The spec-driven
schema declares the specs artifact as generates: "specs/**/*.md", and both
artifact completion (status) and the #1783 no-specs warning in
instructions apply resolve that glob. The two readers disagree about any
other markdown file under specs/.

A delta written as specs/user-auth.md, or as a second file beside a
capability's spec.md, therefore went through every surface without a stop:

• status: specs done, isPlanningComplete: true
• instructions apply: all_done, no warning (the glob matches the file,
  so the fix(apply): warn when a change is ready to implement with no specs #1783 warning does not fire)
• validate: exit 1, but only the generic "Change must have at least one delta"
• archive: exit 0. Nothing is merged into openspec/specs/, and the
  output never mentions specs.

archive.ts runs delta validation only when it finds delta content it can read,
or a root specs/spec.md (#1385). Otherwise it takes the zero-delta path its
own comment acknowledges ("An UNMARKED zero-delta change still archives with
only non-blocking proposal warnings"). The authored requirement is dropped, and
the change is filed as done.

Verified end to end in a project created by openspec init +
openspec new change, on the published 1.13.0 package and on main @ 9d4e597.

What Changes

• findUnreadDeltaFiles (src/utils/spec-discovery.ts) lists markdown
  files under a change's specs/ that carry delta sections but are not a
  capability's spec.md, with the spec.md their requirements belong in:

  • specs/user-auth.md → specs/user-auth/spec.md
  • specs/user-auth/more.md → specs/user-auth/spec.md

  "Carries delta sections" is decided by parseDeltaSpec, the reader archive
  merges with, so headers inside a code fence do not count. The walk follows
  discoverSpecFiles: dot entries skipped, symlinked directories not followed,
  dangling links skipped, other read errors thrown. A root specs/spec.md is
  left to its existing validate accepts a delta spec.md directly under specs/ that archive silently drops #1385 check.

• validate reports each file as an ERROR naming the file and where its
  requirements belong. The generic "No deltas found" is suppressed when this
  error already explains the missing deltas, the same way the validate accepts a delta spec.md directly under specs/ that archive silently drops #1385 root error
  does.

• archive treats such a file as delta content, the way it treats a root
  specs/spec.md, so it runs delta validation and refuses the change instead of
  archiving it unmerged. --no-validate keeps its existing meaning.

• instructions apply lists each file in warnings, so apply stops being
  the one surface that green-lights the change.

• docs-lab/reference/schemas/spec-driven/index.md: one sentence, in the Delta specs section, on where a change's delta must live.

Unchanged:

• A change with no spec files still archives (the zero-delta leniency is kept
  for changes that authored no delta).
• Notes under specs/ with no delta section (a README, review notes) are not
  reported.
• The flat and nested layouts that already worked behave as before.

Why not change the schema glob instead. Tightening
schemas/spec-driven/schema.yaml:51 to specs/*/spec.md would make status
agree, but it would also break the supported nested layout
(specs/<area>/<capability>/spec.md, #1353). A recursive specs/**/spec.md
would change what status and the instruction loader show, and what completes
the artifact, for projects in mid-flight. Custom schemas keep their own globs
either way. Reporting the unread file where it matters (validate, archive,
apply) closes the gap without changing any schema.

Testing

test/core/misplaced-delta-files.test.ts: 27 tests, written first and run
before the fix: 20 failed / 7 passed (the 7 are controls). With the fix:
27 passed (27).

Before the fix, the helper tests failed because the helper did not exist, and
every behavioural test failed on its assertion:

• validate found no ERROR for the file;
• archive set no exit code and archived the change;
• apply returned no warnings.

Edge cases covered:

• helper: flat file; another name in a capability folder; a stray file beside a
  valid spec.md; nested area folder; SPEC.md (case); flat and nested valid
  layouts (not reported); root spec.md (left to validate accepts a delta spec.md directly under specs/ that archive silently drops #1385); notes without delta
  sections; delta headers only inside a code fence; dot entries and non-.md
  files; sorted output; missing specs/; dangling symlink (skipped on Windows)
• validate: the flat file is an ERROR naming both paths, with no "No deltas
  found"; a stray file is rejected even beside a valid spec.md; controls for
  the nested layout and for notes beside a valid delta
• archive: refuses the flat file and the stray file (exit 1, not archived, main
  spec untouched); --no-validate still archives; controls for a change with no
  spec files and for the valid layout (archived and merged)
• apply: warns about the flat file exactly when the validator rejects it; names
  a stray file; control stays quiet
• end to end through the built CLI (init, new change, apply, validate,
  archive) for the flat file and the valid layout

CI parity, in a clean checkout of 9d4e597 with this patch, on Node 20.19.0:

• pnpm run build: success
• pnpm exec tsc --noEmit: no errors
• pnpm lint: no errors
• pnpm test (full suite, VITEST_MAX_WORKERS=4), run in a sandbox shared
  with several other suites:
  • The first run caught one real failure, in
    validation.archive-preflight.test.ts (see Notes for reviewers). It is
    fixed. That file, this branch's test file, validation, archive and
    apply-instructions-warnings pass together: 369 passed.
  • The rerun after the fix: 4560 passed, 30 failed (159 files). Every failure
    is a 10 s test timeout or spawnSync npm ETIMEDOUT.
    • 25 are in store-references, store-remote, store-root-selection,
      store, workset and package-install-scripts. Each of those files
      fails the same way on unpatched 9d4e597 under the same load.
    • The other 5 each hit one timeout in cli-e2e/basic,
      cli-e2e/store-lifecycle, cli-e2e/validate-international,
      artifact-workflow and declared-store-fallback. Each passes run alone,
      both on this branch and on 9d4e597.

Changeset

.changeset/reject-unread-delta-files.md (patch). For release notes: a change
whose delta sits outside specs/<capability-path>/spec.md used to archive
without merging it. It now fails validation until the file is moved.

Notes for reviewers

• status still reports the specs artifact as done for such a file, because
  the schema glob is unchanged (see above). instructions apply now carries the
  warning.
• In validateChangeDeltaSpecs the unread-file check runs after the advisory
  merge-conflict discovery (validate: MODIFIED/REMOVED/RENAMED-from headers that don't exist in base spec aren't caught until archive (proposal: opt-in cross-change MODIFIED) #1112), not inside it. An error from the advisory
  pass then stays an INFO note while strict validation stays valid, which is
  what validation.archive-preflight.test.ts pins. The first full-suite run
  caught this ordering.
• This touches validator.ts and archive.ts in small, separate hunks. Other
  open fixes that edit those files may need a trivial rebase.

Summary by CodeRabbit

• Bug Fixes

  • Validation now reports delta content stored in unsupported files, including the affected file and expected location.
  • Archive refuses changes containing misplaced delta files instead of archiving them without merging.
  • Apply instructions now warn about each misplaced delta file.
  • Changes without delta files continue to archive normally, and --no-validate behavior is unchanged.

• Documentation

  • Clarified that capability deltas must be written in the capability folder’s spec.md file.

[Copilot]

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8d7d7a2b-51ba-400b-b709-7b8b64104d08

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4e597 and 66c16a6.

📒 Files selected for processing (7)

• .changeset/reject-unread-delta-files.md
• docs/writing-specs.md
• src/commands/workflow/instructions.ts
• src/core/archive.ts
• src/core/validation/validator.ts
• src/utils/spec-discovery.ts
• test/core/misplaced-delta-files.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The change detects delta sections outside capability spec.md files. Validation reports each file, archive refuses invalid changes, and apply instructions provide move warnings. Tests cover discovery, validation, archive, CLI flows, symlinks, sorting, and valid controls.

Changes

Unread delta file handling

Layer / File(s)	Summary
Delta discovery and traversal src/utils/spec-discovery.ts, test/core/misplaced-delta-files.test.ts	Adds findUnreadDeltaFiles to detect misplaced delta sections. The helper handles nested paths, ignored files, symlinks, missing paths, and sorting.
Validation, archive, and apply behavior src/core/validation/validator.ts, src/core/archive.ts, src/commands/workflow/instructions.ts, test/core/misplaced-delta-files.test.ts	Validation reports misplaced files as errors. Archive refuses them unless --no-validate is used. Apply instructions reports the expected capability spec.md path.
Documentation and release metadata docs/writing-specs.md, .changeset/reject-unread-delta-files.md	Documents the required delta file location and records the patch behavior.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant ApplyInstructions
  participant Validator
  participant Archive
  participant findUnreadDeltaFiles
  ApplyInstructions->>findUnreadDeltaFiles: inspect change specs
  findUnreadDeltaFiles-->>ApplyInstructions: unread delta paths
  ApplyInstructions-->>ApplyInstructions: create move warnings
  Validator->>findUnreadDeltaFiles: inspect change specs
  findUnreadDeltaFiles-->>Validator: unread delta paths
  Validator-->>Validator: emit file-specific errors
  Archive->>Validator: validate change
  Validator-->>Archive: validation failure
  Archive-->>Archive: refuse archive

Loading

Suggested reviewers: clay-good

Merge Risk: ⚪ Minimal · up to 66c16

The validation, archive, and apply-warning paths consistently handle misplaced delta files and are covered by targeted tests. The PR is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Issue #1869 requires no workflow surface to mark an unread delta as complete. The PR adds detection to validate, archive, and instructions apply, with tests and destination paths. However, `stat…	Update status or its artifact-completion logic to identify delta sections in files that the merge path does not read. Do not report the specs artifact or planning state as complete for those files. Add status tests for flat and nested mispl…
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The changed source files implement detection, validation errors, archive protection, and apply warnings for the misplaced delta behavior in issue #1869. The tests cover these workflow surfaces and edg…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main archive behavior change: refusing delta files that the merge path cannot read. It accurately reflects the primary objective, although the pull reques…

Full details: Linked Issues check

Explanation

Issue #1869 requires no workflow surface to mark an unread delta as complete. The PR adds detection to validate, archive, and instructions apply, with tests and destination paths. However, status is unchanged. The spec-driven schema still uses specs/**/*.md, and status derives completion from the artifact graph, so specs/<capability>.md can still report specs: done and isPlanningComplete: true. This does not meet the issue's no-green-light requirement.

Resolution

Update status or its artifact-completion logic to identify delta sections in files that the merge path does not read. Do not report the specs artifact or planning state as complete for those files. Add status tests for flat and nested misplaced delta files, while preserving valid spec.md layouts and zero-delta changes.

Full details: Docstring Coverage

Explanation

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

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[clay-good]

Hardening pass: pushed b699b7e, docs(schemas): move delta file placement note to docs-lab. The sentence on where a change's delta must live now sits in docs-lab/reference/schemas/spec-driven/index.md (Delta specs section) instead of the frozen docs/writing-specs.md, which is restored to main. No code changes. @TabishB for the docs-lab wording.

Verified on the built CLI: the #1869 repro now fails validate and archive (exit 1) and apply warns. Nested layouts, a README without delta headers beside spec.md, and delta headers inside a fence still validate and archive. New tests fail without the fix (19 of 27; the rest are controls or run against the built CLI).

[alfred-openspec]

Misplaced delta files are detected consistently by apply guidance, validation, and archive before anything is silently dropped. The canonical docs describe the enforced layout, and focused tests pass.