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

[lawsonoates]

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.

[coderabbitai]

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

• X
• Mastodon
• Reddit
• LinkedIn

_{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:

1. Add specs to schemas/spec-driven/schema.yaml apply.requires, likely requires: [specs, tasks].
2. Flip the missing-specs test to expect blocked state, exit code 1, missingArtifacts: ['specs'], and the delta-spec hint.
3. 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.

[lawsonoates]

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