fix(apply): fail with exit 1 when apply instructions are blocked by lawsonoates · Pull Request #1250 · Fission-AI/OpenSpec

lawsonoates · 2026-06-24T12:27:16Z

Summary

Fix for #1212.

Hardens the apply path so missing required artifacts are treated as a hard failure instead of a soft warning the agent can ignore.

Changes

openspec instructions apply now sets exit code 1 when apply is blocked (missing required artifacts per the schema's apply.requires)
Adds a spec-driven hint when specs is among the missing artifacts, pointing authors to create delta specs under changes/<name>/specs/ before implementation
Adds regression tests for blocked apply (including tasks present but delta specs missing when specs is in apply.requires)
Updates tmp-init fixture with tasks.md so the apply JSON e2e test exercises a ready change, not a blocked one

Behavior

Before: Apply could report Blocked but still exit 0, making it easy for agents/scripts to proceed anyway.

After: Blocked apply exits 1 and includes clearer guidance when delta specs are missing.

Testing

pnpm exec vitest run test/commands/artifact-workflow.test.ts
pnpm test (full suite)

Notes

I was unable to reproduce the exact failure described in #1212. On a greenfield playground project, the propose → apply → archive path created delta specs during propose, and archive synced them into openspec/specs/ as expected.

A follow-up could add specs to the default apply.requires if others can reproduce the skip-delta-specs behaviour.

Summary by CodeRabbit

Bug Fixes
- The apply command now returns a non-zero exit status when apply instructions are blocked, improving CI/script reliability.
- For spec-driven workflows, missing required artifacts now include a clearer, targeted hint about creating the needed delta specs.
- Text and JSON output behavior has been aligned for blocked states to keep responses consistent.
Tests
- Expanded CLI coverage for spec-driven blocking scenarios, including new assertions for JSON state, missing artifacts, and exit codes.
Chores
- Updated a fixture to include an implementation checklist item.

Exit with code 1 when apply instructions are blocked, add a spec-driven hint when delta specs are absent, and extend tests plus the tmp-init fixture so blocked and ready apply paths are covered.

Expect only tasks in the missing-artifacts message and assert that missing delta specs do not block apply while the default schema still requires tasks alone.

coderabbitai · 2026-06-24T12:27:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4d74f12b-31c6-4cd7-a2fa-7804e8f685f9

📥 Commits

Reviewing files that changed from the base of the PR and between 9315bd1 and f2d72fb.

📒 Files selected for processing (2)

schemas/spec-driven/schema.yaml
test/commands/artifact-workflow.test.ts

✅ Files skipped from review due to trivial changes (1)

schemas/spec-driven/schema.yaml

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

test/commands/artifact-workflow.test.ts

📝 Walkthrough

Walkthrough

generateApplyInstructions adds a spec-driven-specific hint when specs are missing. applyInstructionsCommand now sets process.exitCode = 1 for blocked output. The spec-driven schema requires both specs and tasks, and tests plus a fixture reflect the new behavior.

Changes

Apply instructions blocked state and spec-driven hint

Layer / File(s)	Summary
Spec-driven apply requirements `schemas/spec-driven/schema.yaml`	The spec-driven schema’s apply requires list includes both `specs` and `tasks`.
Blocked apply instructions `src/commands/workflow/instructions.ts`	`generateApplyInstructions` adds a `specs`-specific hint when `spec-driven` apply output is blocked by missing artifacts. `applyInstructionsCommand` prints text output in an `else` branch and sets `process.exitCode = 1` when `instructions.state === 'blocked'`.
Apply workflow tests and fixture `test/commands/artifact-workflow.test.ts`, `test/fixtures/tmp-init/openspec/changes/c1/tasks.md`	Blocked-apply tests now expect non-zero exit codes and missing `specs`/`tasks` output, add coverage for missing delta `specs`, keep the spec-driven ready-state assertion, and update the tasks fixture with an implementation checklist item.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Fission-AI/OpenSpec#443: Changes schema-name resolution, which is used to detect spec-driven for the conditional apply hint.
Fission-AI/OpenSpec#444: Updates the spec-driven apply requirement and blocked-state flow that this PR extends.
Fission-AI/OpenSpec#967: Also touches generateApplyInstructions and applyInstructionsCommand behavior around required artifacts and blocked output.

Suggested reviewers

alfred-openspec

Poem

🐇 I sniffed the specs, then tapped my toe,
When blocks appear, the exit code says “no.”
A delta-specs hint helps paths align,
And tasks march in the checklist line.
Hoppity hop, the flow is fine! 🥕

🚥 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 accurately summarizes the main change: blocked apply instructions now cause exit code 1.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

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

alfred-openspec

PR #1250 Review: blocked apply exits non-zero

Reviewed: 2026-06-24 23:18 AEST
PR: #1250
Author: @lawsonoates
Head: 9315bd1cdb32447f15f9d735eed77a969eeec914
Issue: #1212, spec-driven fast-path workflow silently leaves specs stale

Verdict

Changes requested.

The exit-code hardening is good and should be kept, but the PR no longer fixes the root #1212 path. The final commit explicitly preserves the existing spec-driven apply contract of apply.requires: [tasks] and adds a regression test proving that a change with proposal.md, design.md, and tasks.md but no specs/ is still considered ready to apply.

That is exactly the unsafe state described in #1212: fast-path generation can stop at apply requirements, opsx:apply proceeds, and archive later has no delta specs to sync.

What changed

src/commands/workflow/instructions.ts
- applyInstructionsCommand() now sets process.exitCode = 1 when generated apply instructions are blocked.
- JSON and text output both still print before the non-zero exit is set.
- generateApplyInstructions() includes a spec-driven delta-spec hint only when specs is already in missingArtifacts.
test/commands/artifact-workflow.test.ts
- Blocked apply now expects exit code 1.
- New regression asserts missing delta specs do not block apply when tasks exist.
test/fixtures/tmp-init/openspec/changes/c1/tasks.md
- Added a simple tasks fixture so existing apply JSON path is ready.

Verification

Ran in /tmp/openspec-pr1250:

pnpm install --frozen-lockfile
pnpm exec vitest run test/commands/artifact-workflow.test.ts

Result: 1 file, 60 tests passed.

Review findings

Blocking: missing delta specs still pass apply for spec-driven changes

The final PR state leaves schemas/spec-driven/schema.yaml as:

apply:
  requires: [tasks]

generateApplyInstructions() derives missingArtifacts exclusively from that apply.requires list. Since specs is not required, this branch never runs for the #1212 failure state:

const specsHint =
  missingArtifacts.includes('specs') && context.schemaName === 'spec-driven'

The new test makes this explicit:

await createTestChange('missing-specs-apply', ['proposal', 'design', 'tasks']);
expect(result.exitCode).toBe(0);
expect(json.state).toBe('ready');
expect(json.missingArtifacts).toBeUndefined();

That means the unsafe fast path remains green. Exit-code hardening only helps when the command is already blocked, but #1212 is about the command failing to block.

Recommended fix

Minimum acceptable fix for this PR:

Add specs to schemas/spec-driven/schema.yaml apply.requires, likely requires: [specs, tasks].
Flip the missing-specs test to expect blocked state, exit code 1, missingArtifacts: ['specs'], and the delta-spec hint.
Keep the existing exit-code hardening.

Follow-up still worth tracking separately:

Archive should also guard against spec-driven changes with no delta specs before it cleanly reports “No delta specs”. That is the last line of defense if the apply fast path regresses again.

Notes

This is small and close. I would not merge as-is because it creates a false sense that #1212 is fixed while preserving the exact missing-specs apply path.

Add specs to apply.requires so fast-path workflows cannot stop at tasks alone. Update apply instruction tests to expect blocked state when delta specs are missing.

lawsonoates · 2026-06-24T14:22:54Z

@alfred-openspec implemented your recommendations

alfred-openspec

Thanks Lawson, this addresses the blocking issue from my last review. I re-tested the PR at f2d72fb: pnpm install --frozen-lockfile and pnpm exec vitest run test/commands/artifact-workflow.test.ts pass, and spec-driven apply now blocks with exit 1 when delta specs are missing.\n\nI still think we should add an archive-side guard as a follow-up last line of defense for #1212, but this PR is good to merge for the apply path.

Propose a deterministic CLI backstop so a spec-driven change cannot archive while silently dropping the specs it declared. Root cause (confirmed): `openspec validate` runs validateChangeDeltaSpecs unconditionally and rejects a change with no deltas, but `openspec archive` gates the same check behind `if (hasDeltaSpecs)`, so a change with no specs/ skips validation and archives clean — the proposal's no-delta error is downgraded to a non-blocking warning. Fix: - Archive runs delta-spec validation unless --skip-specs (parity with validate); a no-delta change is blocked with CHANGE_NO_DELTAS. - instructions apply exits non-zero when blocked. - spec-driven apply.requires includes specs. - Archive skill prompt aligned with the new blocking behavior. Closes Fission-AI#1212, Fission-AI#1260, Fission-AI#1222, Fission-AI#1264, Fission-AI#799. Supersedes PRs Fission-AI#1250, Fission-AI#1271, Fission-AI#1241, Fission-AI#1233. References (out of scope) Fission-AI#1246, Fission-AI#1112, Fission-AI#1252. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Propose one deterministic completeness check, enforced in the validator and shared by `validate`, CI, and `archive`, so a spec-driven change cannot archive while silently dropping the specs it declared. Two silent failure modes, both closed: - Total drop: archive gated delta-spec validation behind `if (hasDeltaSpecs)`, so a change with no specs/ skipped the CHANGE_NO_DELTAS check that `validate` already enforces. Fix: validate unless --skip-specs (validate/archive parity). - Partial drop: nothing checked the proposal's declared capabilities against delivered delta specs. Fix: deterministic validateChangeCapabilityCoverage — every capability under `## Capabilities` must have specs/<id>/spec.md with a delta section. Parsing contract validated against all 93 in-repo proposals (found 7 real proposal/spec inconsistencies; zero false extractions). Plus earlier guardrails (incorporates Fission-AI#1250): apply exits non-zero when blocked; spec-driven apply.requires includes specs. Closes Fission-AI#1212, Fission-AI#1260, Fission-AI#1222, Fission-AI#1264, Fission-AI#799. Supersedes PRs Fission-AI#1250, Fission-AI#1271, Fission-AI#1241, Fission-AI#1233. References (out of scope) Fission-AI#1246, Fission-AI#1112, Fission-AI#1252. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its real root: one architectural fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — expressed at four layers. Fix, ordered by leverage: - Deterministic CLI gate: archive validates like `validate` unless --skip-specs, schema-aware (only when the schema graph has a `specs` artifact, per Fission-AI#997). Blocks total drop (CHANGE_NO_DELTAS), partial drop (new schema-aware capability-coverage rule), and format/mode-G (present non-delta spec → "No delta sections found"). - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff walk the full schema build order (incorporates Fission-AI#1250; Fission-AI#1250's reviewer explicitly deferred this archive guard). - Keystone: the archive skill calls `openspec archive` CLI instead of agent-judged sync + raw mv, so the CLI guarantees are reachable by agents (Fission-AI#656, Fission-AI#863). - Recovery audit for already-archived drift; delta-vs-full-spec clarity. Closes Fission-AI#1212 Fission-AI#1260 Fission-AI#1222 Fission-AI#1264 Fission-AI#799 Fission-AI#656 Fission-AI#863 Fission-AI#164. Supersedes PRs Fission-AI#1250 Fission-AI#1271 Fission-AI#1241 Fission-AI#1233. Addresses-in-part Fission-AI#997 Fission-AI#194 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#913 Fission-AI#1120. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — expressed at four layers, fixed at the most deterministic point for each. - CLI gate (guarantee): archive validates like `validate` unless --skip-specs, schema-aware (only when the schema graph produces delta specs, per Fission-AI#997). Blocks total (CHANGE_NO_DELTAS), partial (new capability-coverage rule), and format/mode-G ("No delta sections"). - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff create every artifact transitively required by applyRequires (incorporates Fission-AI#1250; its reviewer deferred this archive guard). - Keystone: archive skill calls `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); also removes the sync-specs skill dependency (Fission-AI#913). - Recovery audit; delta-vs-full-spec clarity. Verified against current main (rebuilt CLI): bug reproduces; archive --json + ArchiveBlockedError exist; Fission-AI#1250 not merged; Fission-AI#194/Fission-AI#1268 closed. 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#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — fixed at the most deterministic point for each of four layers. - CLI gate (guarantee): ONE shared schema-aware predicate (schemaProducesDeltaSpecs) gates the delta requirement in BOTH `validate` and `archive`, so they agree by construction and proposal-only schemas pass both while spec-driven-with-no-specs fails both (Fission-AI#997). Blocks total (CHANGE_NO_DELTAS), partial (capability coverage), and format/mode-G ("No delta sections"). `--yes` never bypasses the gate. - Loop fix: spec-driven apply.requires [specs, tasks] + propose/ff create every artifact transitively required by applyRequires (incorporates Fission-AI#1250). - Keystone: archive skill calls `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); removes the sync-specs skill dependency (Fission-AI#913). - Recovery audit; delta-vs-full-spec clarity. Addresses review feedback (alfred-openspec: validate needs the same schema-aware gate as archive; coderabbit: document --yes does not bypass, fenced-block tags, clarify partial detection, fail-open/--yes tests). 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#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address the spec-driven silent-spec-drop family at its root: one fault — correctness-critical decisions live in agent prompts, not deterministic CLI logic — fixed at the most deterministic point for each of four layers. CLI gate (guarantee): one shared schema-aware predicate (schemaProducesDeltaSpecs, memoized per run) gates the delta requirement in BOTH validate and archive; blocks total/partial/format drops; proposal-only schemas pass both (Fission-AI#997). Loop: spec-driven apply.requires [specs, tasks] (apply gate is non-transitive — design stays optional). Keystone: ALL archive templates (skill + command + bulk) call `openspec archive` instead of agent-judged sync + raw mv (Fission-AI#656, Fission-AI#863); never --skip-specs/--no-validate. Recovery: specced archived-drift audit (forward-only). Clarity: delta-vs-full spec + stricter Capabilities-section contract. Incorporates three review rounds (alfred-openspec, CodeRabbit) and an internal adversarial pass: validate schema-aware parity, --yes doesn't bypass, all archive surfaces reworked, non-transitive apply gate, schema-resolution memoization, capability deletion/rename + bold-label/non-kebab handling, proposal-only fixture for Fission-AI#997 tests. 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#164 Fission-AI#426 Fission-AI#911. Out of scope Fission-AI#1246 Fission-AI#1112 Fission-AI#1252 (complementary; land first) Fission-AI#1120 Fission-AI#827 Fission-AI#1265. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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>

…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>

lawsonoates added 2 commits June 24, 2026 22:05

fix(apply): block instructions apply when required artifacts are missing

75db1ed

Exit with code 1 when apply instructions are blocked, add a spec-driven hint when delta specs are absent, and extend tests plus the tmp-init fixture so blocked and ready apply paths are covered.

test(apply): align blocked-apply tests with spec-driven apply.requires

9315bd1

Expect only tasks in the missing-artifacts message and assert that missing delta specs do not block apply while the default schema still requires tasks alone.

lawsonoates requested a review from TabishB as a code owner June 24, 2026 12:27

alfred-openspec requested changes Jun 24, 2026

View reviewed changes

fix(spec-driven): require delta specs before apply

f2d72fb

Add specs to apply.requires so fast-path workflows cannot stop at tasks alone. Update apply instruction tests to expect blocked state when delta specs are missing.

lawsonoates requested a review from alfred-openspec June 29, 2026 05:57

alfred-openspec approved these changes Jun 29, 2026

View reviewed changes

clay-good mentioned this pull request Jun 29, 2026

Propose: prevent silent spec drop — deterministic, CLI-enforced, end-to-end #1277

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(apply): fail with exit 1 when apply instructions are blocked#1250

fix(apply): fail with exit 1 when apply instructions are blocked#1250
lawsonoates wants to merge 3 commits into
Fission-AI:mainfrom
lawsonoates:fix/1212-require-specs-before-apply

lawsonoates commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

alfred-openspec left a comment

Uh oh!

lawsonoates commented Jun 24, 2026

Uh oh!

alfred-openspec left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

lawsonoates commented Jun 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Behavior

Testing

Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

alfred-openspec left a comment

Choose a reason for hiding this comment

PR #1250 Review: blocked apply exits non-zero

Verdict

What changed

Verification

Review findings

Blocking: missing delta specs still pass apply for spec-driven changes

Recommended fix

Notes

Uh oh!

lawsonoates commented Jun 24, 2026

Uh oh!

alfred-openspec left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lawsonoates commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading