Skip to content

fix(apply): warn when a change is ready to implement with no specs - #1783

Open
clay-good wants to merge 5 commits into
mainfrom
fix/apply-warns-missing-specs
Open

fix(apply): warn when a change is ready to implement with no specs#1783
clay-good wants to merge 5 commits into
mainfrom
fix/apply-warns-missing-specs

Conversation

@clay-good

@clay-good clay-good commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Status: Ready for review.

Closes #834. Related: #869 (same failure seen from Copilot).

What was wrong

Two ways openspec instructions apply told an agent to skip the artifacts a change is supposed to be built from.

1. It reported ready for a change with no specs at all. Apply gates on the schema's apply.requires — for spec-driven that is tasks alone. Nothing checks that tasks own prerequisites were built, so a change whose tasks.md was written ahead of its specs came back ready to implement:

$ openspec status --change demo-change
[x] proposal
[ ] specs          ← never written
[ ] design
[x] tasks

$ openspec instructions apply --change demo-change
### Instruction
Read context files, work through pending tasks, mark complete as you go.

status says the specs are missing, openspec validate demo-change fails the change ("Change must have at least one delta … set skip_specs: true"), and archive warns about it. Apply was the one surface saying "go ahead" — the surface an agent reads immediately before writing code.

2. When it did block, it named only the first hop. A change holding nothing but a proposal got:

Missing artifacts: tasks
Use the openspec-continue-change skill to create these first.

Read literally, that is an instruction to write tasks.md straight from the proposal — which lands you back in case 1. And the remedy was a dead end on the default install: continue is not in CORE_WORKFLOWS, so the core profile never installs the skill the message named.

How it was fixed

Ready-state warning. generateApplyInstructions now collects warnings about the change itself. One rule today: apply is past its own gate, the schema declares spec-producing artifacts, none produced a file, and the change does not declare skip_specs: true.

### ⚠️ Warnings

- This change has no delta specs and does not declare `skip_specs: true`, so `openspec validate my-change`
  fails on it. Write the delta specs before implementing (`openspec instructions specs --change my-change`),
  or add `skip_specs: true` to <changeDir>/.openspec.yaml if this change really changes no specified behavior.

Blocked-state chain. Walking requires from apply.requires gives everything still to build, in build order — reported as missingPrerequisites:

### ⚠️ Blocked

Missing artifacts: tasks
Not created yet, in build order: specs, design, tasks

Remedies that exist everywhere. Every message in this function now points at openspec instructions <artifact> --change <name> and openspec status --change <name> instead of the openspec-continue-change skill. The CLI verbs are what the skill runs, and they are there on every profile.

Deliberate boundaries:

  • Reports, does not block. The state machine is untouched — missingArtifacts still decides blocked, so no change that applied before applies any differently now. No deadlock risk and no new policy. Turning the no-specs warning into a hard block is a one-line follow-up, but it is a policy call and belongs to the founder, not to this PR.
  • Spec artifacts identified by output path, reusing the existing isSpecsArtifactPath helper that skip_specs already uses — no artifact id is hardcoded, so custom schemas keep working. A schema with no spec-producing artifact gets skip_specs at change creation, so this never fires on one.
  • design is never demanded. It is optional in practice (38 of the 83 archived changes in this repo have a design.md) and, unlike specs, has no opt-out marker. It appears in the build-order list because the schema declares it — the wording says "build the ones this change needs … the schema says which are conditional" — and when several artifacts are left the remedy stays <artifact> rather than naming the first one, which would point at design as often as at specs.

--json gains missingPrerequisites?: string[] and warnings?: string[]; both documented in docs/agent-contract.md §4.6.

Replication / proof

The reproduction above, against the built CLI on this branch:

$ openspec instructions apply --change c4 --json
  "state": "blocked",
  "missingArtifacts": ["tasks"],
  "missingPrerequisites": ["specs", "design", "tasks"],
  "instruction": "Cannot apply this change yet. Missing artifacts: tasks.\nNot created yet, in build order:
     specs, design, tasks. Build the ones this change needs before applying - the schema says which are
     conditional.\nCreate each with `openspec instructions <artifact> --change c4` …"

# tasks.md written, specs skipped:
### ⚠️ Warnings
- This change has no delta specs and does not declare `skip_specs: true`, so `openspec validate c4` fails on it. …

18 tests across two new files, run against unmodified src/ first to confirm they fail:

test/commands/apply-instructions-warnings.test.ts (11) — warns on a ready change with no specs; prints the section above the context files; still warns once every task is done; warns about exactly the state Validator.validateChangeDeltaSpecs rejects, and stays quiet about exactly the state it accepts (so the message cannot drift from the rule it cites); quiet with specs, quiet with skip_specs, quiet while blocked; quiet for a custom schema that produces no specs; warns for a custom schema whose spec artifact is called contracts. 3 failed before the change.

test/commands/apply-instructions-blocked.test.ts (7) — names the whole chain; leaves conditional artifacts to the schema; drops the chain line once only the required artifact is left; counts a skipped specs as built; never names openspec-continue-change; no prerequisites once ready; prints the chain under the blocked heading. 6 failed before the change.

Full local suite: 4419 passed, 19 failed — 16 vitest 10s timeouts plus one npm ETIMEDOUT in subprocess-heavy e2e files on a loaded machine, and the two failures that reproduce with src/ reverted to main (config-profile, artifact-workflow; sandbox path/permission errors under /var/folders). No assertion failure anywhere near this change. CI is the real check: linux, macOS and windows-pwsh all green.

Notes / nits

  • Claude Opus 4.6 will skip the opsx:ff and went ahead to implement #869 (Copilot implementing straight after /opsx:explore) is the same class of failure but starts before any apply call, in the workflow templates. This PR covers every CLI surface an agent hits on the way to implementing; it does not claim the template half.
  • Removing the skill name from these messages overlaps in spirit with fix(templates): stop generated skills naming workflows the profile omits #1775 (profile-aware workflow references), which covers generated templates only and does not touch this file — no conflict.
  • The known limitation stays: apply cannot judge whether the specs a change does have are complete, only whether any exist. Gating on artifact contents is out of scope by design.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • openspec instructions apply now warns when changes lack delta specs without declaring skip_specs: true.
    • Warnings appear in text and JSON output with recommended remedies and validation commands.
    • Blocked applies now report the complete missing prerequisite chain and provide relevant CLI guidance.
  • Documentation

    • Updated the JSON contract to document optional warnings and missing-prerequisite fields, distinguishing prerequisites from missing artifacts.

Apply gates on the schema's `apply.requires` (tasks) alone, so a change
whose tasks file was written ahead of its specs read as ready even though
it had no delta specs at all — the state `openspec validate` rejects.
Apply was the one surface that green-lit a change every other surface
flags, which is how agents end up implementing before the specs exist.

Report it as a warning, in the text output and in `--json`, naming both
ways out: write the specs, or declare `skip_specs: true`. Blocking would
be a policy change; naming the gap is not. Changes that have specs,
declare `skip_specs`, or are still blocked on their own required
artifacts are unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@clay-good
clay-good requested a review from a team as a code owner September 4, 2026 14:08
@clay-good
clay-good requested review from alfred-openspec and removed request for a team September 4, 2026 14:08
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 4, 2026

Copy link
Copy Markdown

Deploying openspec-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5cd26d1
Status: ✅  Deploy successful!
Preview URL: https://2ec30f23.openspec-docs.pages.dev
Branch Preview URL: https://fix-apply-warns-missing-spec.openspec-docs.pages.dev

View logs

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 265d2976-af35-4d63-ace1-a454d985c69a

📥 Commits

Reviewing files that changed from the base of the PR and between 26e71eb and 5cd26d1.

📒 Files selected for processing (6)
  • .changeset/apply-warns-missing-specs.md
  • docs/agent-contract.md
  • src/commands/workflow/instructions.ts
  • src/commands/workflow/shared.ts
  • test/commands/apply-instructions-blocked.test.ts
  • test/commands/apply-instructions-warnings.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • .changeset/apply-warns-missing-specs.md

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


📝 Walkthrough

Walkthrough

openspec instructions apply now reports missing delta specs as non-blocking warnings and reports the full missing prerequisite chain for blocked applies. Both text and JSON output include the new diagnostics and CLI remedies.

Changes

Apply instruction diagnostics

Layer / File(s) Summary
Warning contract and detection
src/commands/workflow/shared.ts, src/commands/workflow/instructions.ts
Adds optional warnings output and detects missing delta specs for applicable, unblocked changes.
Prerequisite reporting and output
src/commands/workflow/instructions.ts
Computes transitive missing prerequisites, updates artifact commands, and renders prerequisite and warning sections.
Validation and contract coverage
test/commands/apply-instructions-blocked.test.ts, test/commands/apply-instructions-warnings.test.ts, docs/agent-contract.md, .changeset/apply-warns-missing-specs.md
Tests blocked and warning states, documents the JSON contract, and records the patch release.

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

Merge Risk: ⚪ Minimal · up to 5cd26

Apply instructions now surface non-blocking missing-spec guidance and complete prerequisite chains while preserving existing apply gating. The updated behavior is covered across JSON, text, schema, and validation scenarios and is ready to merge.

Sequence Diagram(s)

sequenceDiagram
  participant Change
  participant generateApplyInstructions
  participant collectMissingPrerequisites
  participant collectApplyWarnings
  participant ApplyOutput
  Change->>generateApplyInstructions: request apply instructions
  generateApplyInstructions->>collectMissingPrerequisites: resolve required artifact chain
  collectMissingPrerequisites-->>generateApplyInstructions: return missing prerequisites
  generateApplyInstructions->>collectApplyWarnings: inspect state and spec outputs
  collectApplyWarnings-->>generateApplyInstructions: return warnings when delta specs are absent
  generateApplyInstructions->>ApplyOutput: include diagnostics in JSON or text output
Loading

Suggested reviewers: tabishb

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR addresses the apply-stage symptom from issue #834 by warning about missing specs. It does not enforce the required Proposal → Design → Spec → Tasks → Apply sequence, prevent premature apply sug… Implement the sequence enforcement required by issue #834, including preventing apply suggestions until required prior artifacts are complete. If this PR intentionally covers only the apply-stage warning, link it to a narrower issue or upda…
Docstring Coverage ⚠️ Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (2 skipped: … 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 warning logic, prerequisite-chain reporting, contract updates, and related tests all support apply workflow behavior. No unrelated code changes are evident.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: openspec instructions apply warns when a ready change has no specs.
Full details: Linked Issues check

Explanation

The PR addresses the apply-stage symptom from issue #834 by warning about missing specs. It does not enforce the required Proposal → Design → Spec → Tasks → Apply sequence, prevent premature apply suggestions, or update the skill files.

Resolution

Implement the sequence enforcement required by issue #834, including preventing apply suggestions until required prior artifacts are complete. If this PR intentionally covers only the apply-stage warning, link it to a narrower issue or update the issue scope explicitly.

Full details: Docstring Coverage

Explanation

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

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/apply-warns-missing-specs

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.

clay-good and others added 2 commits September 4, 2026 09:10
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A schema with no spec-producing artifact must stay quiet, and one whose
spec artifact is not called `specs` must still warn - the rule keys off
the output path, not the artifact id.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@openspec-cloud

openspec-cloud Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

No PR-relevant drift confirmed.

AI-generated · A citation proves the line exists, not that it makes the case — verify before acting.
No issue was confirmed at 29af7d7; 2 requirements could not be verified.
This is not a full-repository clean result; see the check for coverage and any broader findings.
View results · Click Refresh, then Scan again in the check. Or comment /openspec-cloud.

clay-good and others added 2 commits September 4, 2026 09:19
os.tmpdir() hands back the short form (C:\Users\RUNNER~1) while the CLI
resolves the long one, so the assertion pinned a path that never matched
on windows-pwsh. Assert the change-relative tail instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Apply blocks on the schema's `apply.requires` alone, so its message
stopped at the first hop: a change holding only a proposal was told
"Missing artifacts: tasks" while the specs `tasks` depends on were
missing too. Taken literally that is an instruction to write the
tracking file straight from the proposal and skip everything between —
the failure reported in #834 and #869.

Walk `requires` and report the whole set, in build order, as
`missingPrerequisites` (text and `--json`). What apply blocks on is
unchanged, and the wording leaves conditional artifacts to the schema
rather than demanding them.

The remedies these messages give are now CLI commands rather than the
`openspec-continue-change` skill: `continue` is not in CORE_WORKFLOWS,
so on the default profile the old advice named a skill that is never
installed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude Code should follow OpenSpec sequence without prompting apply before specs and tasks are complete

1 participant