Propose: prevent silent spec drop — deterministic, CLI-enforced, end-to-end#1277
Propose: prevent silent spec drop — deterministic, CLI-enforced, end-to-end#1277clay-good wants to merge 8 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a ChangesPrevent Silent Spec Drop
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
db7167a to
23f3c10
Compare
clay-good
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.md (1)
1-51: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDocument that
--yesdoes not bypass the archive validation guard.The PR objectives explicitly state that
--yesdoes not bypass the guard, but this spec does not mention--yesbehavior. Since archive has confirmation flows for--no-validate, spec updates, and incomplete tasks, users might reasonably expect--yesto force through validation failures. Clarify that--yesonly affects confirmation prompts, not the validation gate itself.Consider adding a scenario such as:
Scenario:
--yesdoes not bypass validation
- WHEN executing
openspec archive change-name --yes- AND the change has no delta specs and neither
--skip-specsnor--no-validateis provided- THEN the archive is still blocked with
CHANGE_NO_DELTAS- AND the change is not archived
🤖 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 `@openspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.md` around lines 1 - 51, Clarify the archive spec’s validation behavior by explicitly stating that `--yes` only skips confirmation prompts and does not bypass the validation gate. Update the Archive Validation section in spec.md to add a scenario for `openspec archive change-name --yes` showing that, without `--skip-specs` or `--no-validate`, a spec-less change still fails with `CHANGE_NO_DELTAS` and is not archived. Refer to the existing “Archive Validation” and “Skip Specs Option” requirements so the new scenario aligns with the current `openspec archive` behavior.
🧹 Nitpick comments (4)
openspec/changes/prevent-silent-spec-drop/tasks.md (1)
21-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit test coverage for
--yesnot bypassing the validation guard.The PR objectives state that
--yesdoes not bypass the guard, but the test checklist does not explicitly verify this. Item 5.3 tests--no-validatewith--yes, but there is no test for--yesalone on a no-delta change. Consider adding:
- 5.X Archive: change with no delta specs and
--yes(without--skip-specsor--no-validate) → still blocked withCHANGE_NO_DELTAS, non-zero exit🤖 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 `@openspec/changes/prevent-silent-spec-drop/tasks.md` around lines 21 - 31, Add explicit archive test coverage to verify that `--yes` alone does not bypass the no-delta-specs guard. In the test checklist for the archive flow, add a case alongside the existing archive scenarios in tasks.md that exercises a change with no `specs/` using `--yes` without `--skip-specs` or `--no-validate`, and assert it still fails with `CHANGE_NO_DELTAS` and a non-zero exit. Keep this aligned with the existing archive coverage entries so the behavior is validated by the same archive test suite.openspec/changes/prevent-silent-spec-drop/design.md (2)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The code block contains TypeScript-style comments. Add
typescriptto satisfy markdownlint and improve syntax highlighting.+```typescript
// before: if (hasDeltaSpecs) { run validateChangeDeltaSpecs }
// after: if (!options.skipSpecs) { run validateChangeDeltaSpecs }🤖 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 `@openspec/changes/prevent-silent-spec-drop/design.md` around lines 45 - 48, The fenced code block in the design document is missing a language specifier, which triggers markdownlint and reduces syntax highlighting. Update the markdown fence in the relevant snippet to use a TypeScript language tag, and keep the existing comment lines unchanged so the example remains readable and correctly highlighted.
9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The code block contains TypeScript-style pseudocode. Add
typescriptto satisfy markdownlint and improve syntax highlighting.+```typescript
if (totalDeltas === 0) {
issues.push({ level: 'ERROR', path: 'file', message: CHANGE_NO_DELTAS });
}🤖 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 `@openspec/changes/prevent-silent-spec-drop/design.md` around lines 9 - 13, Add the missing language tag to the fenced code block in the design document so the pseudocode is marked as TypeScript. Update the markdown fence around the snippet containing totalDeltas and issues.push to use a typescript code fence, keeping the existing code unchanged, so markdownlint passes and syntax highlighting works.openspec/changes/prevent-silent-spec-drop/proposal.md (1)
18-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
This block shows a shell session. Add
shellorbashto satisfy markdownlint and enable syntax highlighting.+```shell
$ openspec validate silent-drop
...
$ openspec archive silent-drop --yes
...exit 0 — archived; no openspec/specs/ directory created
🤖 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 `@openspec/changes/prevent-silent-spec-drop/proposal.md` around lines 18 - 29, The fenced shell-session example in proposal.md is missing a language tag, so update the code fence around the openspec commands to use a shell-friendly specifier such as shell or bash. Keep the existing content unchanged and ensure the block is still clearly identified as a terminal session for markdownlint and syntax highlighting.
🤖 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.
Outside diff comments:
In `@openspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.md`:
- Around line 1-51: Clarify the archive spec’s validation behavior by explicitly
stating that `--yes` only skips confirmation prompts and does not bypass the
validation gate. Update the Archive Validation section in spec.md to add a
scenario for `openspec archive change-name --yes` showing that, without
`--skip-specs` or `--no-validate`, a spec-less change still fails with
`CHANGE_NO_DELTAS` and is not archived. Refer to the existing “Archive
Validation” and “Skip Specs Option” requirements so the new scenario aligns with
the current `openspec archive` behavior.
---
Nitpick comments:
In `@openspec/changes/prevent-silent-spec-drop/design.md`:
- Around line 45-48: The fenced code block in the design document is missing a
language specifier, which triggers markdownlint and reduces syntax highlighting.
Update the markdown fence in the relevant snippet to use a TypeScript language
tag, and keep the existing comment lines unchanged so the example remains
readable and correctly highlighted.
- Around line 9-13: Add the missing language tag to the fenced code block in the
design document so the pseudocode is marked as TypeScript. Update the markdown
fence around the snippet containing totalDeltas and issues.push to use a
typescript code fence, keeping the existing code unchanged, so markdownlint
passes and syntax highlighting works.
In `@openspec/changes/prevent-silent-spec-drop/proposal.md`:
- Around line 18-29: The fenced shell-session example in proposal.md is missing
a language tag, so update the code fence around the openspec commands to use a
shell-friendly specifier such as shell or bash. Keep the existing content
unchanged and ensure the block is still clearly identified as a terminal session
for markdownlint and syntax highlighting.
In `@openspec/changes/prevent-silent-spec-drop/tasks.md`:
- Around line 21-31: Add explicit archive test coverage to verify that `--yes`
alone does not bypass the no-delta-specs guard. In the test checklist for the
archive flow, add a case alongside the existing archive scenarios in tasks.md
that exercises a change with no `specs/` using `--yes` without `--skip-specs` or
`--no-validate`, and assert it still fails with `CHANGE_NO_DELTAS` and a
non-zero exit. Keep this aligned with the existing archive coverage entries so
the behavior is validated by the same archive test suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fcff2276-c41b-4632-83a5-963d10eca0db
📒 Files selected for processing (6)
openspec/changes/prevent-silent-spec-drop/.openspec.yamlopenspec/changes/prevent-silent-spec-drop/design.mdopenspec/changes/prevent-silent-spec-drop/proposal.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.mdopenspec/changes/prevent-silent-spec-drop/tasks.md
✅ Files skipped from review due to trivial changes (1)
- openspec/changes/prevent-silent-spec-drop/.openspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.md
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
23f3c10 to
48ea8ae
Compare
clay-good
changed the title
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
48ea8ae to
af48253
Compare
clay-good
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (4)
openspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.md (1)
14-20: 📐 Maintainability & Code Quality | 🔵 TrivialClarify how "partial" specs are detected without referencing layer 2 directly.
The scenario references "partial" delta specs, but this file doesn't define what constitutes partial. The capability-coverage rule (from the layer 2 dependency) defines this. Consider adding a brief note or reference so readers of this standalone spec understand what "partial" means, e.g., "where declared capabilities lack corresponding delta spec files per the capability-coverage rule."
🤖 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 `@openspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.md` around lines 14 - 20, Clarify the meaning of “partial” delta specs in the scenario so the standalone spec is understandable: update the wording in the spec’s scenario around openspec archive to define partial as declared capabilities that do not have corresponding delta spec files, using the capability-coverage rule as the reference. Keep the existing scenario structure and make the detection criteria explicit without mentioning layer 2 directly.openspec/changes/prevent-silent-spec-drop/proposal.md (1)
22-29: 📐 Maintainability & Code Quality | 🔵 TrivialAdd shell language tag to the fenced code block.
The reproduction example is missing a language identifier (e.g.,
bashorshell). Adding one improves rendering and copy-paste ergonomics.🤖 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 `@openspec/changes/prevent-silent-spec-drop/proposal.md` around lines 22 - 29, The fenced reproduction block in the proposal is missing a shell language tag, so update the code block in the document to use a shell/bash identifier. Locate the example block describing the validate/archive commands and add the language tag to that fenced snippet so it renders and copies correctly.openspec/changes/prevent-silent-spec-drop/tasks.md (1)
9-9: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test scenario for unparseable-proposal fail-open behavior.
Task 1.4 specifies "fail-open on unparseable proposal," but no test in section 6 explicitly covers this. Add a test ensuring that a malformed proposal (e.g., unclosed code fence, invalid frontmatter) does not trigger coverage errors and does not crash the validator.
🤖 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 `@openspec/changes/prevent-silent-spec-drop/tasks.md` at line 9, Add a test case for the fail-open path in validateChangeCapabilityCoverage(changeDir) within src/core/validation/validator.ts. Cover a malformed proposal (such as invalid frontmatter or an unclosed code fence) and assert the validator does not throw and does not emit capability coverage errors when the proposal cannot be parsed. Place the test alongside the existing validator coverage tests so the unparseable-proposal behavior is explicitly verified.openspec/changes/prevent-silent-spec-drop/design.md (1)
49-52: 📐 Maintainability & Code Quality | 🔵 TrivialClarify how the
specsartifact is resolved.Decision 3 gives two ways to find the artifact (
graph.getArtifact('specs')vs. the artifact whosegeneratescoversspecs/**). These diverge when the artifact has a different name. Pick one authoritative method and document when the other applies, or state that both must agree.🤖 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 `@openspec/changes/prevent-silent-spec-drop/design.md` around lines 49 - 52, Clarify the schema-aware lookup for the `specs` artifact by choosing one authoritative resolution path in the design and using it consistently in the validation flow: either rely on `graph.getArtifact('specs')` as the primary check or define the fallback based on the artifact whose `generates` includes `specs/**`, but do not leave both as interchangeable. Update the `validateChangeDeltaSpecs`/schema-graph logic to state exactly when the alternate lookup is used, and make sure the documented rule matches the actual behavior in the resolved schema path so the `hasValidationErrors → ArchiveBlockedError` / `return null` handling applies only when a `specs` artifact is truly present.
🤖 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 `@openspec/changes/prevent-silent-spec-drop/design.md`:
- Around line 49-52: Clarify the schema-aware lookup for the `specs` artifact by
choosing one authoritative resolution path in the design and using it
consistently in the validation flow: either rely on `graph.getArtifact('specs')`
as the primary check or define the fallback based on the artifact whose
`generates` includes `specs/**`, but do not leave both as interchangeable.
Update the `validateChangeDeltaSpecs`/schema-graph logic to state exactly when
the alternate lookup is used, and make sure the documented rule matches the
actual behavior in the resolved schema path so the `hasValidationErrors →
ArchiveBlockedError` / `return null` handling applies only when a `specs`
artifact is truly present.
In `@openspec/changes/prevent-silent-spec-drop/proposal.md`:
- Around line 22-29: The fenced reproduction block in the proposal is missing a
shell language tag, so update the code block in the document to use a shell/bash
identifier. Locate the example block describing the validate/archive commands
and add the language tag to that fenced snippet so it renders and copies
correctly.
In `@openspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.md`:
- Around line 14-20: Clarify the meaning of “partial” delta specs in the
scenario so the standalone spec is understandable: update the wording in the
spec’s scenario around openspec archive to define partial as declared
capabilities that do not have corresponding delta spec files, using the
capability-coverage rule as the reference. Keep the existing scenario structure
and make the detection criteria explicit without mentioning layer 2 directly.
In `@openspec/changes/prevent-silent-spec-drop/tasks.md`:
- Line 9: Add a test case for the fail-open path in
validateChangeCapabilityCoverage(changeDir) within
src/core/validation/validator.ts. Cover a malformed proposal (such as invalid
frontmatter or an unclosed code fence) and assert the validator does not throw
and does not emit capability coverage errors when the proposal cannot be parsed.
Place the test alongside the existing validator coverage tests so the
unparseable-proposal behavior is explicitly verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f10cf86-9ff2-41c5-b71d-d22d37a7d4e3
📒 Files selected for processing (8)
openspec/changes/prevent-silent-spec-drop/.openspec.yamlopenspec/changes/prevent-silent-spec-drop/design.mdopenspec/changes/prevent-silent-spec-drop/proposal.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-validate/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.mdopenspec/changes/prevent-silent-spec-drop/tasks.md
✅ Files skipped from review due to trivial changes (2)
- openspec/changes/prevent-silent-spec-drop/.openspec.yaml
- openspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.md
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
af48253 to
3e1e8fb
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openspec/changes/prevent-silent-spec-drop/proposal.md (1)
22-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifier to fenced code block.
The shell commands should specify
shellorbashfor proper syntax highlighting and accessibility.-``` +# ```shell $ openspec validate silent-drop # exit 1: CHANGE_NO_DELTAS $ openspec archive silent-drop --yes # exit 0: "archived"; no openspec/specs/ created🤖 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 `@openspec/changes/prevent-silent-spec-drop/proposal.md` around lines 22 - 29, Add a shell language specifier to the fenced command block in the proposal so the openspec validate/archive examples are rendered as shell/bash code, updating the markdown fence where the commands are shown and keeping the rest of the example text unchanged.
🤖 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 `@openspec/changes/prevent-silent-spec-drop/proposal.md`:
- Around line 22-29: Add a shell language specifier to the fenced command block
in the proposal so the openspec validate/archive examples are rendered as
shell/bash code, updating the markdown fence where the commands are shown and
keeping the rest of the example text unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd5d7ac5-97a7-4932-8db2-4d12744f7721
📒 Files selected for processing (8)
openspec/changes/prevent-silent-spec-drop/.openspec.yamlopenspec/changes/prevent-silent-spec-drop/design.mdopenspec/changes/prevent-silent-spec-drop/proposal.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/cli-validate/spec.mdopenspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.mdopenspec/changes/prevent-silent-spec-drop/tasks.md
✅ Files skipped from review due to trivial changes (3)
- openspec/changes/prevent-silent-spec-drop/.openspec.yaml
- openspec/changes/prevent-silent-spec-drop/specs/cli-artifact-workflow/spec.md
- openspec/changes/prevent-silent-spec-drop/specs/cli-archive/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/prevent-silent-spec-drop/specs/opsx-archive-skill/spec.md
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
This is the right consolidation direction, but one contract gap needs fixing before implementation: validate also needs the same schema-aware delta gate as archive. On current main, src/commands/validate.ts calls validator.validateChangeDeltaSpecs(changeDir) unconditionally in single and bulk validation, and validateChangeDeltaSpecs emits CHANGE_NO_DELTAS when there is no specs/ directory. So a proposal-only schema with no specs artifact would still fail openspec validate, even though the design says #997 is exempt and archive/validate should agree. Please add tasks and tests so validate resolves the change schema and only runs the existing delta-spec validation when schemaProducesDeltaSpecs(...) is true. Coverage should include proposal-only + no specs passes validate, while spec-driven + no specs still fails with CHANGE_NO_DELTAS.
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
3e1e8fb to
04a386d
Compare
|
Thanks @alfred-openspec and CodeRabbit — addressed all of it in the proposal/design/specs/tasks (no implementation yet; this is still the change proposal). @alfred-openspec — the validate contract gap (the important one). You're right: making only
Captured in CodeRabbit:
The proposal continues to pass its own new rules ( |
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
2 times, most recently
from
6d3298d to
9ae0781
Compare
|
Ran a deep self-review (three independent passes: consistency, spec-delta correctness, and adversarial engineering) and folded the findings into the proposal/design/specs/tasks. Still proposal-only; Substantive corrections:
Re #1252 / #1246 (flagged as a possible conflict): verified complementary, not conflicting. #1246 is the |
|
Prerequisite: land #1252 before this PR. #1252 (APPROVED) is the tactical fix for the distinct |
Deterministic, CLI-enforced prevention of silent spec drop in the spec-driven workflow, addressing one root fault (correctness decisions in agent prompts) across four layers: schema-aware delta gate shared by validate+archive (Fission-AI#997), the non-transitive apply-gate loop fix (incorporates Fission-AI#1250), the keystone (all archive templates call `openspec archive` — Fission-AI#656/Fission-AI#863), and a specced archived-drift audit. Hardened across three review rounds (alfred-openspec, CodeRabbit), an internal adversarial pass, and an open-PR coordination scan: - reconciles Fission-AI#977 (allow-specless) via the schema-aware gate + --skip-specs, deliberately not allowing silent specless archives under spec-driven; - coordinates with Fission-AI#902 (propose/ff spec discovery), the unify-template-generation-pipeline manifest, add-change-stacking-awareness (provides/touches markers + overlap warnings), and add-artifact- regeneration-support (complementary staleness); - rebases onto approved Fission-AI#1186/Fission-AI#1151/Fission-AI#1153/Fission-AI#1252; Fission-AI#1252 is a prerequisite. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#913. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses Fission-AI#997 Fission-AI#977 Fission-AI#902 Fission-AI#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252(prereq) Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
9ae0781 to
dd24830
Compare
|
Ran an open-PR/issue coordination scan over the 13 source files this change will touch, to make sure it composes with in-flight work. Folded the results into the proposal/design (no scope change — sequencing + reconciliation only). Highlights: Design reconciliations (called out explicitly):
Mechanical sequencing (verified states 2026-06-29):
Architecture coherence: with No competing PR implements the deterministic CLI gate — this is not a duplicate of any open PR. |
… (silent-spec-drop §1) The deterministic CLI gate — the guarantee layer of prevent-silent-spec-drop. - extractDeclaredCapabilities: pure parser of a proposal's ## Capabilities (heading + bold-label forms; non-kebab → warning, not dropped). - schemaProducesDeltaSpecs(schema): true iff an artifact generates under specs/. - Validator.validateChangeCapabilityCoverage: one ERROR per declared capability with no specs/<id>/spec.md (presence-only; delta-format left to delta check). - makeDeltaRequirementResolver: memoized, schema-aware predicate shared by validate and archive; fail-safe to requiring deltas when resolution throws. - validate (single + bulk) and archive both gate delta validation + coverage on the shared predicate, so they agree by construction and proposal-only schemas pass both (Fission-AI#997). archive's self-defeating `if (hasDeltaSpecs)` gate is gone; it now blocks total, partial, and format (non-delta) drops. - instructions apply exits non-zero when blocked; spec-driven apply.requires is [specs, tasks] (the non-transitive apply gate now names specs). Updated archive/artifact-workflow tests for the new behavior; added the c1 apply fixture and declared-capabilities unit tests. Full suite green (the pre-existing zsh-installer env failures are unrelated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…`; harden propose/ff loop (§2–3) Keystone: the archive skill/command and bulk-archive skill/command no longer assess sync by agent judgment or move the change with raw `mkdir`+`mv`. All four templates now run `openspec archive --json --yes`, which deterministically validates, syncs delta specs, and moves the change — so the §1 CLI guarantees are finally reachable from the agent path (Fission-AI#656, Fission-AI#863). On a block they surface the diagnostic and re-run; they never bypass with `--skip-specs`/`--no-validate` and never self-certify. Loop: propose/ff guardrails state the change is not apply-ready until every `apply.requires` artifact is `done` (for spec-driven that includes `specs`) — backing the schema gate from §1. Updated the template parity baselines for the 8 changed templates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ity (§4–5) - `openspec validate --archived`: detection-only audit that flags archived changes whose declared capabilities never reached openspec/specs/. Forward-only (pre-contract archives reported as not-auditable); exits 1 on drift. Does not regenerate content. Registered in CLI + completion registry. - spec-driven schema: the specs artifact instruction now states a change spec is a DELTA spec (## ADDED/MODIFIED/REMOVED/RENAMED), never a full ## Purpose/## Requirements spec, and that archive blocks otherwise; the proposal Capabilities instruction states ids must match the spec folder and that validate/archive enforce coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- capability-coverage.test.ts: validateChangeCapabilityCoverage + schemaProducesDeltaSpecs. - archive-spec-gate.test.ts: total/partial/format drop blocked; --yes does not bypass; --skip-specs bypasses; valid change archives; proposal-only archives. - validate-schema-aware.test.ts: proposal-only passes, spec-driven fails (single + bulk); --archived audit detects drift and clears after sync. - archive-uses-cli.test.ts: all four archive surfaces invoke `openspec archive`, no raw mv, no agent-driven sync delegation. - artifact-workflow: design-less change with specs+tasks stays apply-ready. Full suite: 1782 passing (pre-existing zsh-installer env failures excluded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the changeset (minor) and mark all implementation tasks complete. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implemented (this PR now contains the full change, not just the proposal)Built against current §1 — Deterministic CLI gate (the guarantee)
§2 — Loop: propose/ff guardrails state the change isn't apply-ready until every §3 — Keystone: all four archive surfaces ( §4 — Recovery: §5 — Clarity: the §6 — Tests: Heads-up: the new audit reveals pre-existing drift in 6 other active changesRunning Prerequisite reminder: land #1252 first (complementary |
…d-to-end Add test/cli-e2e/issue-1212-spec-drop.test.ts, a regression that drives the real built CLI against a spec-driven project reproducing Fission-AI#1212: - PR Fission-AI#1250: `instructions apply --json` on a change with no delta specs exits 1 with state=blocked, missingArtifacts includes "specs", and the spec-driven "Delta specs must exist…" hint. (This PR contains all of Fission-AI#1250 plus the archive guard its reviewer deferred.) - Fission-AI#1212 safeguard: `archive --json --yes` refuses — the change is not moved and no main spec is silently created. - Fission-AI#1212 happy path: once the delta spec exists, apply is unblocked and archive syncs the main spec. - Fission-AI#1212 recovery: `validate --archived` detects an already-archived change whose declared capability never reached openspec/specs/. Docs: proposal now records the e2e proof for Fission-AI#1212/Fission-AI#1250. Full e2e suite green (32), full suite 1786 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#1212 and #1250 are now fully addressed — with an end-to-end regressionAdded PR #1250 — fully superseded. This PR contains all of #1250's changes and the archive guard its reviewer explicitly deferred:
Issue #1212 — both requested safeguards + recovery, proven:
Verified e2e green (32 tests incl. the new 4); full suite 1786 passing; lint clean. |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for pushing this through implementation. The overall direction still looks right, and the new archive/apply/keystone coverage is much stronger, but I found one blocking regression in the validate path.
openspec validate <change> now skips proposal validation entirely. In ValidateCommand.validateChange(), the report is built only from validateChangeDeltaSpecs() and validateChangeCapabilityCoverage() when the schema produces specs, and returns a clean empty report when it does not. That drops the pre-existing validator.validateChange(proposal.md) checks for required proposal sections and change-shape warnings/errors.
I reproduced this on the PR head with a change whose proposal.md is missing ## Why but has a valid delta spec. node bin/openspec.js validate bad --json exits 0 with no issues. Proposal-only schemas would now pass even with malformed or incomplete proposals too.
Please preserve the old proposal validation in both single and bulk validate, then layer the schema-aware delta gate on top. Add coverage for: invalid proposal + valid delta still fails validate, and proposal-only schema + invalid proposal still fails validate while proposal-only + valid proposal + no specs passes.
@alfred-openspec — blocking regression fixed (c6ad446)You're right that
Two honest notes on the fix:
Surfacing this also flagged three store-test fixtures with sub-50-char Where this PR standsImplementation is complete and green: 1789 tests pass, full e2e suite green, Not "merge-button" ready — two reasons, both external/decision-level: 1. Sequencing (PRs to land first):
2. A judgment call that's yours: the new audit ( Also worth a glance for reviewers: this is a deliberate behavior change (archive hard-blocks by default, |
…e (review) alfred-openspec found that `openspec validate <change>` skipped proposal validation: the report was built only from delta-spec + coverage checks (and was empty for proposal-only schemas), so a malformed proposal (e.g. missing ## Why) with a valid delta passed. Fix: add Validator.validateChangeProposal(changeDir) — full validateChange (required sections, Why length, What Changes) minus the delta-quantity rules, which are schema-gated/advisory — and run it ALWAYS in both single and bulk validate, layering the schema-aware delta gate + coverage on top. Result (covered by new tests): - invalid proposal + valid delta -> FAILS - proposal-only schema + invalid proposal -> FAILS - proposal-only + valid proposal + no specs -> PASSES (Fission-AI#997 preserved) Also: trim this change's own Why under the 1000-char limit (detail is in design.md) and lengthen three store-test proposal fixtures that had sub-50-char Why sections (now surfaced by the restored validation). Full suite 1789 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
clay-good
force-pushed
the
prevent-silent-spec-drop
branch
from
c6ad446 to
6b1c915
Compare
…bish review) Reframe per the steer "more deterministic and grounded in reality": - Deterministic spine: the CLI computes the impact set (which downstream artifacts to revisit, in build order, with paths) as a pure function of schema edges + filesystem. The agent only rewrites prose. Grounded in real APIs already present: getUnlockedArtifacts (direct dependents), getBuildOrder (order), resolveArtifactOutputs (paths); reverse map built at graph.ts:82-87. - Replace fragile mtime staleness with a newline-normalized SHA-256 content digest (reproducible cross-platform). Drift = upstream digest vs recorded baseline; no baseline => "unknown", never a false positive. mtime and pure-git rejected with rationale; digest ledger is a separable, optional layer. - Explicit determinism boundary decision (CLI decides files/order/drift; agent rewrites). Skill MUST source the file list/order from `openspec status --impact`, never compute it. - Corrected all code citations to verified lines (graph.ts:82-87, instruction-loader.ts:366/429, status.ts); noted Fission-AI#1277's coverage helpers are not in this branch's base (coordinate, don't reuse). - Specs updated: artifact-graph Content Digest requirement; cli status digest + deterministic impact ordering; skill determinism + baseline-aware audit. tasks add digest/determinism/cross-platform tests + optional ledger section. Still validates clean under `openspec validate add-update-workflow --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ame refs - Digest ledger tracks DIRECT upstream digests; document that transitive drift emerges hop-by-hop as downstream is reconciled (no transitive bookkeeping). - Ground audit's no-baseline structural facts on signals available in this branch (missing/empty output, blocked/incomplete); capability-coverage is an add-on only when Fission-AI#1277's validateChangeCapabilityCoverage is present. - Add the "update revises only existing downstream; defer not-yet-created ones to /opsx:continue" rule across proposal/design/specs/tasks; impact entries now carry existence/status. - Note artifact-level (not file-level) granularity and that getDownstream terminates by the schema's acyclic guarantee. - Remove direct personal references from the docs. Validates clean under `openspec validate add-update-workflow --strict`; 10 deltas. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sign After a comprehensive sweep of open issues, PRs, and discussions, grounded the proposal in the complete adjacent landscape and answered the open design questions the cluster raises: - Fission-AI#783 (Cross-artifact quality review before apply) is now a primary Closes: it IS audit mode. Answer its open "new skill vs. extend validate" question via the determinism split — deterministic checks (drift/completeness/coverage) are CLI/validate-shaped; the semantic cross-artifact review is the skill. Added a skill spec scenario for the Fission-AI#783 patterns (scope contradiction, spec gap, duplication). - Discussion Fission-AI#1206 ("refine proposal now?") + prior-art PR Fission-AI#372: official answer is /opsx:update. - New design Decision 8 (command family): delineate /opsx:update from /opsx:clarify (Fission-AI#702, within-artifact), /opsx:review (Fission-AI#1251, plan-vs-code), and verify; /opsx:update consolidates update+regen+refine into one action, addressing skill-sprawl (Fission-AI#1263, Fission-AI#783). - Reuse, don't reinvent: audit's empty/incomplete check reuses Fission-AI#1098's artifactOutputComplete (same outputs.ts the digest helper lives in); capability coverage reuses Fission-AI#1277's validateChangeCapabilityCoverage. - New open questions: surface deterministic coherence in `validate` for a CI gate (Fission-AI#783-B, Fission-AI#829); naming reconciliation with Fission-AI#783's /opsx:refine. - Confirmed add-update-command* branches are the `openspec update` tool-file refresh (not artifact update) — no collision. Validates clean under --strict; 10 deltas; all relative links resolve. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
This fixes the validate contract regressions I called out on the previous heads. I re-ran the focused validate/archive/capability coverage tests plus typecheck, and the schema-aware gate now preserves proposal validation while letting proposal-only schemas pass without delta specs.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for the quick fixes. One blocker remains: validate/archive still disagree for proposal-shape errors. In , proposal validation is still non-blocking in text mode and skipped entirely for JSON mode, while \ now fails malformed proposals via . I reproduced this on this branch with a change missing \ but containing a valid delta: \ exits 1, then \ exits 0 and archives/syncs it, printing only a non-blocking proposal warning. Since this PR’s contract is that archive blocks what validate rejects, archive needs to run the same proposal validation and block on errors before syncing/moving, including \ mode.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for the quick fixes. One blocker remains: validate/archive still disagree for proposal-shape errors.
In src/core/archive.ts, proposal validation is still non-blocking in text mode and skipped entirely for JSON mode, while openspec validate <change> now fails malformed proposals via validateChangeProposal().
I reproduced this on this branch with a change missing ## Why but containing a valid delta: openspec validate bad-prop exits 1, then openspec archive bad-prop --yes exits 0 and archives/syncs it, printing only a non-blocking proposal warning. Since this PR’s contract is that archive blocks what validate rejects, archive needs to run the same proposal validation and block on errors before syncing/moving, including --json mode.
2 participants
Summary
A spec-driven change can finish the whole workflow — propose, implement, archive — while silently leaving
openspec/specs/stale, with no error and no recovery. A forensic read of the cluster (#1212, #1250, #656, #863, #164, #194, #997, #1260, #1222, #1264, #799, plus live Discord reports) shows it's not one bug but one architectural fault at four layers:This is a dogfooded OpenSpec change proposal (
openspec/changes/prevent-silent-spec-drop/) that fixes each layer at its most deterministic point.validate --strictpasses; the proposal passes its own new coverage rule (declares 4 capabilities, ships a delta for each).The four layers (all filed as issues)
applyRequiresartifacts are done" (ff-change.ts:72);spec-drivenapplyRequires=[tasks]excludesspecs, soopenspec instructions specsis never called. Non-deterministic — fix(apply): fail with exit 1 when apply instructions are blocked #1250's author couldn't reproduce it. (spec-driven fast-path workflow silently produces stale specs with no recovery path #1212, No Specs generated after /opsx:propose #1260)openspec archive: it judges sync state (archive-change.ts:60), optionally runs a separate agent sync, thenmkdir+mv(archive-change.ts:86). All CLI validation is bypassed for agents. (question: why achive skill or command relies on llm to manually merge specs why it does not use openspec archive cli command? #656, AI commands and skills for/opsx:archivedo not invokeopenspec archiveCLI, unlike official docs and tutorials #863)if (hasDeltaSpecs)(archive.ts:282), hiding three drop modes: total (no specs), partial (declaresa,b,c, shipsa), format/mode G (a full spec where a delta belongs — the Discord case; archive.ts:274's regex misses it). (spec-driven fast-path workflow silently produces stale specs with no recovery path #1212, Error: Change 'my-task' has issues #164, Model tends to cheat around "no deltas found error" #194)Reproduced on current
main:openspec validateexits 1 (CHANGE_NO_DELTAS) whileopenspec archive --yesexits 0 and archives with noopenspec/specs/written; andgrep "openspec archive" archive-change.ts→ no match.Fix (ordered by leverage)
validate(drop thehasDeltaSpecsgate; gate on!--skip-specs), running delta validation + a new schema-aware declared-capability coverage rule. Blocks total (CHANGE_NO_DELTAS), partial (coverage), and format (existing "No delta sections found"). Applies only when the schema graph includes aspecsartifact, so proposal-only schemas are unaffected ([Proposal] Route change validation by workflow schema (delta vs proposal-only) #997).spec-drivenapply.requires: [specs, tasks], and propose/ff walk the full schema build order instead of stopping atapplyRequires. Incorporates fix(apply): fail with exit 1 when apply instructions are blocked #1250 — whose reviewer explicitly deferred this archive guard as the needed "last line of defense."openspec archive --json(never--skip-specs), handling the structured result /ArchiveBlockedErrorand guiding the user to fix a blocked delta spec rather than bypassing. This makes (1) reachable by agents (question: why achive skill or command relies on llm to manually merge specs why it does not use openspec archive cli command? #656, AI commands and skills for/opsx:archivedo not invokeopenspec archiveCLI, unlike official docs and tutorials #863).Deterministic CLI checks are the guarantee; schema/skill/docs changes remove agent judgment from the critical paths. Design, parsing contract, the corpus validation (28 declaring proposals, zero false extractions, 7 real inconsistencies found), and the #997 schema-aware handling are in
design.md. Implementation is sequenced intasks.md(CLI gate → loop → keystone → audit/docs).Closes
#1212 (canonical), #1260, #1222, #1264, #799 (silent drop), #656, #863 (skill bypasses the CLI), #913 (archive's "Sync now" needed an uninstalled skill — eliminated now that archive syncs via the CLI).
Supersedes (open PRs)
#1250 (apply-only; its reviewer deferred the archive guard — this is it), #1271, #1241, #1233 (prompt-only guidance). Prior attempt #1268 was already closed unmerged.
Addresses in part
#997 (schema-aware delta requirement), #164 / #426 / #911 (delta-vs-full-spec & sync-direction clarity). Prior art: #194 (closed).
Out of scope (referenced)
Distinct delta-merge bug / packaging / design requests: #1246 / #1112 / #1252 (cross-change MODIFIED scenario loss), #1120 (Junie packaging), #827 (main-spec evolution), #1265 (change naming / archived reads), and regenerating content for changes already archived without specs (the audit detects them).
Verified buildable (current main, rebuilt CLI)
The committed
binbundle was stale; rebuilt and re-checked: the silent drop reproduces;openspec archive --jsonreturns structured output andArchiveBlockedErroron block (the keystone interface exists); the validator already emits the total/format errors behind thehasDeltaSpecsgate; the dependency chain has nooptionalfield (loop fix well-defined); all issue/PR states re-verified 2026-06-29 (#1250 not merged — main isapply.requires: [tasks]).🤖 Generated with Claude Code
Summary by CodeRabbit
openspec archivevalidation to align withopenspec validate, including schema-aware coverage enforcement.instructions applyreturn a non-zero exit code in both standard and--jsonmodes.--skip-specs,--no-validate), plus recovery/audit guidance.