From 29af7d7d112d5a518430b8308917bca005cbd1b6 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:07:59 -0500 Subject: [PATCH 1/7] fix(apply): warn when a change is ready to implement with no specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/apply-warns-missing-specs.md | 5 + docs/agent-contract.md | 2 +- src/commands/workflow/instructions.ts | 63 +++++++- src/commands/workflow/shared.ts | 2 + .../apply-instructions-warnings.test.ts | 136 ++++++++++++++++++ 5 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 .changeset/apply-warns-missing-specs.md create mode 100644 test/commands/apply-instructions-warnings.test.ts diff --git a/.changeset/apply-warns-missing-specs.md b/.changeset/apply-warns-missing-specs.md new file mode 100644 index 0000000000..ee489b96eb --- /dev/null +++ b/.changeset/apply-warns-missing-specs.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +Apply now says when a change has no delta specs. Apply gates on the schema's `apply.requires` alone, so a change whose `tasks.md` was written ahead of its specs read as ready to implement even though it had no spec deltas at all — the state `openspec validate` rejects. `openspec instructions apply` now reports that gap as a warning (text and `--json`), naming both ways out: write the specs, or declare `skip_specs: true`. Changes that have specs, declare `skip_specs`, or are still blocked on their own required artifacts are unaffected. diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 30fcfacccf..4b04cb1ed8 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -66,7 +66,7 @@ Change: `{ "id", "title", "deltaCount", "deltas": [...], "root" }`. Spec: `{ "id `ReferenceIndexEntry`: `{ "store_id", "root"?, "specs"?: [{id,summary}], "fetch"?, "status": [] }` — resolved entries carry root/specs/fetch; unresolved carry store_id + warning status. Index capped at 50KB (`reference_index_truncated`). ### 4.6 `instructions apply --json` -`{ "changeName", "changeDir", "schemaName", "contextFiles": { "": ["/abs", ...] }, "progress": {total,complete,remaining}, "tasks": [{id,description,done}], "state": "blocked"|"all_done"|"ready", "missingArtifacts"?, "instruction", "references"?, "context"?, "operationGuidance"?, "root" }`. Both optional fields are read from the selected root on every invocation. `context` is a required prompt-level input whose relevant project facts, conventions, and constraints must be applied; `operationGuidance` is advisory input whose entries are followed only when applicable and compatible with the built-in workflow. Both remain separate from state, tasks, progress, context files, and the built-in instruction. +`{ "changeName", "changeDir", "schemaName", "contextFiles": { "": ["/abs", ...] }, "progress": {total,complete,remaining}, "tasks": [{id,description,done}], "state": "blocked"|"all_done"|"ready", "missingArtifacts"?, "warnings"?, "instruction", "references"?, "context"?, "operationGuidance"?, "root" }`. `warnings` lists non-blocking problems with the change itself - today, a change that is ready to implement with no delta specs and no `skip_specs: true`, the state `openspec validate` rejects. Both optional root fields (`context`, `operationGuidance`) are read from the selected root on every invocation. `context` is a required prompt-level input whose relevant project facts, conventions, and constraints must be applied; `operationGuidance` is advisory input whose entries are followed only when applicable and compatible with the built-in workflow. Both remain separate from state, tasks, progress, context files, and the built-in instruction. ### 4.7 `instructions archive --json` `{ "changeName", "context"?, "operationGuidance"?, "root" }`. Requires a valid `--change` in the resolved repo/store root and uses the same required-context/advisory-guidance semantics as apply. This is a read-only runtime-input surface: it does not return the static archive workflow, inspect or merge delta specs, write main specs, or move the change. diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index 1ae6fac7c0..e5da0c2df5 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -16,6 +16,7 @@ import { resolveArtifactOutputs, type ArtifactInstructions, } from '../../core/artifact-graph/index.js'; +import { isSpecsArtifactPath } from '../../core/artifact-graph/outputs.js'; import { getChangeDir, resolveCurrentPlanningHomeSync, @@ -350,6 +351,48 @@ function toTaskItems(parsed: ParsedTask[]): TaskItem[] { return tasks; } +/** + * Warnings apply reports alongside its instruction. + * + * Apply gates on the schema's `apply.requires` only, so a change whose tasks + * file was written ahead of its specs reads as ready even though no delta spec + * exists - the state `openspec validate` rejects. Blocking here would be a + * policy change; naming the gap is not, and it is what keeps apply from being + * the one surface that green-lights a change every other surface flags. + * + * Only reported once apply is past its own gate: for a change that has not + * reached tasks yet, the missing specs are the next step rather than a warning. + * Schemas that declare no spec-producing artifact carry `skip_specs` from + * creation, so this never fires on them. + */ +function collectApplyWarnings(input: { + state: ApplyInstructions['state']; + schema: { artifacts: { id: string; generates: string }[] }; + changeDir: string; + changeName: string; + skippedArtifacts?: Set; +}): string[] { + const { state, schema, changeDir, changeName, skippedArtifacts } = input; + if (state === 'blocked') return []; + + const specArtifacts = schema.artifacts.filter((artifact) => + isSpecsArtifactPath(artifact.generates) + ); + if (specArtifacts.length === 0) return []; + if (specArtifacts.some((artifact) => skippedArtifacts?.has(artifact.id))) return []; + const hasDeltas = specArtifacts.some( + (artifact) => resolveArtifactOutputs(changeDir, artifact.generates).length > 0 + ); + if (hasDeltas) return []; + + const metadataPath = path.join(changeDir, '.openspec.yaml'); + return [ + `This change has no delta specs and does not declare \`skip_specs: true\`, so \`openspec validate ${changeName}\` fails on it. ` + + `Write the delta specs before implementing (\`openspec instructions specs --change ${changeName}\`), ` + + `or add \`skip_specs: true\` to ${metadataPath} if this change really changes no specified behavior.`, + ]; +} + export interface GenerateApplyInstructionsOptions { planningHome?: PlanningHome; references?: ReferenceIndexEntry[]; @@ -461,6 +504,14 @@ export async function generateApplyInstructions( instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.'; } + const warnings = collectApplyWarnings({ + state, + schema, + changeDir, + changeName, + skippedArtifacts: context.skippedArtifacts, + }); + return { changeName, changeDir, @@ -470,6 +521,7 @@ export async function generateApplyInstructions( tasks, state, missingArtifacts: missingArtifacts.length > 0 ? missingArtifacts : undefined, + ...(warnings.length > 0 ? { warnings } : {}), instruction, ...(references !== undefined ? { references } : {}), ...operationInputs, @@ -524,7 +576,7 @@ export async function applyInstructionsCommand(options: ApplyInstructionsOptions } export function printApplyInstructionsText(instructions: ApplyInstructions): void { - const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions; + const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, warnings, instruction } = instructions; console.log(`## Apply: ${changeName}`); console.log(`Schema: ${schemaName}`); @@ -544,6 +596,15 @@ export function printApplyInstructionsText(instructions: ApplyInstructions): voi console.log(); } + if (warnings && warnings.length > 0) { + console.log('### ⚠️ Warnings'); + console.log(); + for (const warning of warnings) { + console.log(`- ${warning}`); + } + console.log(); + } + // Context files (dynamically from schema) const contextFileEntries = Object.entries(contextFiles); if (contextFileEntries.length > 0) { diff --git a/src/commands/workflow/shared.ts b/src/commands/workflow/shared.ts index 2840e004ed..e67d305970 100644 --- a/src/commands/workflow/shared.ts +++ b/src/commands/workflow/shared.ts @@ -43,6 +43,8 @@ export interface ApplyInstructions { tasks: TaskItem[]; state: 'blocked' | 'all_done' | 'ready'; missingArtifacts?: string[]; + /** Non-blocking problems with the change, reported alongside the instruction. */ + warnings?: string[]; instruction: string; /** Referenced-store index (read-only upstream context; omitted when none declared) */ references?: ReferenceIndexEntry[]; diff --git a/test/commands/apply-instructions-warnings.test.ts b/test/commands/apply-instructions-warnings.test.ts new file mode 100644 index 0000000000..435f87a9dc --- /dev/null +++ b/test/commands/apply-instructions-warnings.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + generateApplyInstructions, + printApplyInstructionsText, +} from '../../src/commands/workflow/instructions.js'; + +/** + * Apply gates on the schema's `apply.requires` (tasks) alone, so a change whose + * tasks file was written ahead of its specs reads as ready with no spec deltas + * at all - the state `openspec validate` rejects. Apply has to say so. + */ +describe('generateApplyInstructions warnings', () => { + let tempDir: string; + let changeDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-apply-warnings-')); + changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change'); + fs.mkdirSync(changeDir, { recursive: true }); + fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: spec-driven\n'); + fs.writeFileSync(path.join(changeDir, 'proposal.md'), '## Why\nx\n'); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + function writeTasks(): void { + fs.writeFileSync( + path.join(changeDir, 'tasks.md'), + '## 1. Implementation\n- [ ] 1.1 Write the code\n' + ); + } + + function writeSpecs(): void { + fs.mkdirSync(path.join(changeDir, 'specs', 'demo'), { recursive: true }); + fs.writeFileSync( + path.join(changeDir, 'specs', 'demo', 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Demo\nThe system SHALL demo.\n\n#### Scenario: Works\n- **WHEN** run\n- **THEN** works\n' + ); + } + + it('warns when a ready change has no delta specs and no skip_specs marker', async () => { + writeTasks(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.warnings).toHaveLength(1); + expect(instructions.warnings?.[0]).toContain('no delta specs'); + expect(instructions.warnings?.[0]).toContain('skip_specs: true'); + expect(instructions.warnings?.[0]).toContain('openspec validate my-change'); + expect(instructions.warnings?.[0]).toContain( + 'openspec instructions specs --change my-change' + ); + expect(instructions.warnings?.[0]).toContain(path.join(changeDir, '.openspec.yaml')); + }); + + it('stays quiet once the change has a delta spec', async () => { + writeTasks(); + writeSpecs(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.warnings).toBeUndefined(); + }); + + it('stays quiet for a change that declares skip_specs', async () => { + fs.writeFileSync( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nskip_specs: true\n' + ); + writeTasks(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.warnings).toBeUndefined(); + }); + + it('stays quiet while apply is still blocked on its own required artifacts', async () => { + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('blocked'); + expect(instructions.warnings).toBeUndefined(); + }); + + it('still warns once every task is done, so the gap surfaces before archive', async () => { + fs.writeFileSync( + path.join(changeDir, 'tasks.md'), + '## 1. Implementation\n- [x] 1.1 Write the code\n' + ); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('all_done'); + expect(instructions.warnings).toHaveLength(1); + }); + + it('prints the warnings section above the context files', async () => { + writeTasks(); + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + const lines: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + lines.push(args.join(' ')); + }); + printApplyInstructionsText(instructions); + vi.restoreAllMocks(); + const output = lines.join('\n'); + + expect(output).toContain('### ⚠️ Warnings'); + expect(output).toContain('no delta specs'); + expect(output.indexOf('### ⚠️ Warnings')).toBeLessThan(output.indexOf('### Context Files')); + }); + + it('prints no warnings section when there is nothing to warn about', async () => { + writeTasks(); + writeSpecs(); + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + const lines: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + lines.push(args.join(' ')); + }); + printApplyInstructionsText(instructions); + vi.restoreAllMocks(); + + expect(lines.join('\n')).not.toContain('Warnings'); + }); +}); From d868224a5fce24dbb3d8bd2e72c25a318ddc4e2d Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:10:16 -0500 Subject: [PATCH 2/7] refactor(apply): name the metadata file from its shared constant Co-Authored-By: Claude Opus 5 --- src/commands/workflow/instructions.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index e5da0c2df5..4d6f4bef7c 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -49,6 +49,7 @@ import { type ArchiveInstructions, } from './shared.js'; import { parseTaskLines, type ParsedTask } from '../../utils/task-progress.js'; +import { METADATA_FILENAME } from '../../utils/change-metadata.js'; // ----------------------------------------------------------------------------- // Types @@ -385,7 +386,7 @@ function collectApplyWarnings(input: { ); if (hasDeltas) return []; - const metadataPath = path.join(changeDir, '.openspec.yaml'); + const metadataPath = path.join(changeDir, METADATA_FILENAME); return [ `This change has no delta specs and does not declare \`skip_specs: true\`, so \`openspec validate ${changeName}\` fails on it. ` + `Write the delta specs before implementing (\`openspec instructions specs --change ${changeName}\`), ` + From 163e3bbde8328d533b42c010883f1e53986a7958 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:10:59 -0500 Subject: [PATCH 3/7] test(apply): cover custom schemas in the no-specs warning 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 --- .../apply-instructions-warnings.test.ts | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/test/commands/apply-instructions-warnings.test.ts b/test/commands/apply-instructions-warnings.test.ts index 435f87a9dc..b690478780 100644 --- a/test/commands/apply-instructions-warnings.test.ts +++ b/test/commands/apply-instructions-warnings.test.ts @@ -119,6 +119,77 @@ describe('generateApplyInstructions warnings', () => { expect(output.indexOf('### ⚠️ Warnings')).toBeLessThan(output.indexOf('### Context Files')); }); + it('stays quiet for a schema that produces no specs at all', async () => { + const schemaDir = path.join(tempDir, 'openspec', 'schemas', 'mini'); + fs.mkdirSync(schemaDir, { recursive: true }); + fs.writeFileSync( + path.join(schemaDir, 'schema.yaml'), + [ + 'name: mini', + 'version: 1', + 'artifacts:', + ' - id: proposal', + ' generates: proposal.md', + ' description: p', + ' template: proposal.md', + ' - id: tasks', + ' generates: tasks.md', + ' description: t', + ' template: tasks.md', + ' requires: [proposal]', + 'apply:', + ' requires: [tasks]', + ' tracks: tasks.md', + '', + ].join('\n') + ); + fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: mini\n'); + writeTasks(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.warnings).toBeUndefined(); + }); + + it('warns for a custom schema whose spec artifact is named something else', async () => { + const schemaDir = path.join(tempDir, 'openspec', 'schemas', 'renamed'); + fs.mkdirSync(schemaDir, { recursive: true }); + fs.writeFileSync( + path.join(schemaDir, 'schema.yaml'), + [ + 'name: renamed', + 'version: 1', + 'artifacts:', + ' - id: proposal', + ' generates: proposal.md', + ' description: p', + ' template: proposal.md', + ' - id: contracts', + ' generates: "specs/**/*.md"', + ' description: c', + ' template: spec.md', + ' requires: [proposal]', + ' - id: tasks', + ' generates: tasks.md', + ' description: t', + ' template: tasks.md', + ' requires: [proposal]', + 'apply:', + ' requires: [tasks]', + ' tracks: tasks.md', + '', + ].join('\n') + ); + fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: renamed\n'); + writeTasks(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.warnings).toHaveLength(1); + }); + it('prints no warnings section when there is nothing to warn about', async () => { writeTasks(); writeSpecs(); From 26e71eb94544a7cd6976ffa42937ac0f1686a39d Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 09:19:41 -0500 Subject: [PATCH 4/7] test(apply): stop asserting an absolute temp path on Windows 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 --- test/commands/apply-instructions-warnings.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/commands/apply-instructions-warnings.test.ts b/test/commands/apply-instructions-warnings.test.ts index b690478780..dce08ab28d 100644 --- a/test/commands/apply-instructions-warnings.test.ts +++ b/test/commands/apply-instructions-warnings.test.ts @@ -57,7 +57,11 @@ describe('generateApplyInstructions warnings', () => { expect(instructions.warnings?.[0]).toContain( 'openspec instructions specs --change my-change' ); - expect(instructions.warnings?.[0]).toContain(path.join(changeDir, '.openspec.yaml')); + // Not the absolute path: on Windows the CLI resolves `os.tmpdir()`'s short + // form (C:\Users\RUNNER~1) to its long one, so only the tail is stable. + expect(instructions.warnings?.[0]).toContain( + path.join('my-change', '.openspec.yaml') + ); }); it('stays quiet once the change has a delta spec', async () => { From 5cd26d11659b63a6d123f1a92f22699c6cf4455c Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 4 Sep 2026 10:07:16 -0500 Subject: [PATCH 5/7] fix(apply): name the whole chain a blocked change still needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .changeset/apply-warns-missing-specs.md | 2 + docs/agent-contract.md | 2 +- src/commands/workflow/instructions.ts | 112 ++++++++++++++++- src/commands/workflow/shared.ts | 6 + .../apply-instructions-blocked.test.ts | 119 ++++++++++++++++++ .../apply-instructions-warnings.test.ts | 23 ++++ 6 files changed, 259 insertions(+), 5 deletions(-) create mode 100644 test/commands/apply-instructions-blocked.test.ts diff --git a/.changeset/apply-warns-missing-specs.md b/.changeset/apply-warns-missing-specs.md index ee489b96eb..e9885a7af3 100644 --- a/.changeset/apply-warns-missing-specs.md +++ b/.changeset/apply-warns-missing-specs.md @@ -3,3 +3,5 @@ --- Apply now says when a change has no delta specs. Apply gates on the schema's `apply.requires` alone, so a change whose `tasks.md` was written ahead of its specs read as ready to implement even though it had no spec deltas at all — the state `openspec validate` rejects. `openspec instructions apply` now reports that gap as a warning (text and `--json`), naming both ways out: write the specs, or declare `skip_specs: true`. Changes that have specs, declare `skip_specs`, or are still blocked on their own required artifacts are unaffected. + +A blocked apply also names the whole chain now, not just the first hop: a change holding only a proposal reported `Missing artifacts: tasks` while the specs that `tasks` depends on were missing too, which reads as an instruction to write the tracking file straight from the proposal. The full build order is reported as `missingPrerequisites` in `--json`. The remedies these messages give are CLI commands (`openspec instructions --change `) rather than the `openspec-continue-change` skill, which the `core` profile never installs. diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 4b04cb1ed8..401a9694f6 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -66,7 +66,7 @@ Change: `{ "id", "title", "deltaCount", "deltas": [...], "root" }`. Spec: `{ "id `ReferenceIndexEntry`: `{ "store_id", "root"?, "specs"?: [{id,summary}], "fetch"?, "status": [] }` — resolved entries carry root/specs/fetch; unresolved carry store_id + warning status. Index capped at 50KB (`reference_index_truncated`). ### 4.6 `instructions apply --json` -`{ "changeName", "changeDir", "schemaName", "contextFiles": { "": ["/abs", ...] }, "progress": {total,complete,remaining}, "tasks": [{id,description,done}], "state": "blocked"|"all_done"|"ready", "missingArtifacts"?, "warnings"?, "instruction", "references"?, "context"?, "operationGuidance"?, "root" }`. `warnings` lists non-blocking problems with the change itself - today, a change that is ready to implement with no delta specs and no `skip_specs: true`, the state `openspec validate` rejects. Both optional root fields (`context`, `operationGuidance`) are read from the selected root on every invocation. `context` is a required prompt-level input whose relevant project facts, conventions, and constraints must be applied; `operationGuidance` is advisory input whose entries are followed only when applicable and compatible with the built-in workflow. Both remain separate from state, tasks, progress, context files, and the built-in instruction. +`{ "changeName", "changeDir", "schemaName", "contextFiles": { "": ["/abs", ...] }, "progress": {total,complete,remaining}, "tasks": [{id,description,done}], "state": "blocked"|"all_done"|"ready", "missingArtifacts"?, "missingPrerequisites"?, "warnings"?, "instruction", "references"?, "context"?, "operationGuidance"?, "root" }`. `missingArtifacts` is what apply blocks on (the schema's `apply.requires`); `missingPrerequisites` is everything still to build before apply can run, in build order - the transitive closure of those requires, so it can be the longer list. `warnings` lists non-blocking problems with the change itself - today, a change that is ready to implement with no delta specs and no `skip_specs: true`, the state `openspec validate` rejects. Both optional root fields (`context`, `operationGuidance`) are read from the selected root on every invocation. `context` is a required prompt-level input whose relevant project facts, conventions, and constraints must be applied; `operationGuidance` is advisory input whose entries are followed only when applicable and compatible with the built-in workflow. Both remain separate from state, tasks, progress, context files, and the built-in instruction. ### 4.7 `instructions archive --json` `{ "changeName", "context"?, "operationGuidance"?, "root" }`. Requires a valid `--change` in the resolved repo/store root and uses the same required-context/advisory-guidance semantics as apply. This is a read-only runtime-input surface: it does not return the static archive workflow, inspect or merge delta specs, write main specs, or move the change. diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index 4d6f4bef7c..f2e93c1a51 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -352,6 +352,77 @@ function toTaskItems(parsed: ParsedTask[]): TaskItem[] { return tasks; } +/** + * The command that builds one artifact. + * + * Every earlier remedy here named the `openspec-continue-change` skill, which + * the `core` profile never installs - the advice was a dead end for the default + * install. The CLI verb exists on every profile and is what the skill runs. + */ +function describeArtifactRemedy( + changeName: string, + artifactId?: string, + options: { many?: boolean } = {} +): string { + const target = artifactId ?? ''; + const verb = options.many ? 'Create each with' : 'Create it with'; + return ( + `${verb} \`openspec instructions ${target} --change ${changeName}\`` + + ` (\`openspec status --change ${changeName}\` shows what is left).` + ); +} + +/** + * Finds the artifact a schema path is generated by, so a remedy can name it. + */ +function findArtifactIdFor( + schema: { artifacts: { id: string; generates: string }[] }, + generates: string +): string | undefined { + return schema.artifacts.find((artifact) => artifact.generates === generates)?.id; +} + +/** + * Everything still to build before apply can run, in build order. + * + * Apply blocks on the schema's `apply.requires` alone, so its own list stops at + * the first hop: a change with only a proposal is told "Missing artifacts: + * tasks" while the specs `tasks` depends on are missing too. An agent that + * takes that literally writes the tracking file straight from the proposal and + * skips the artifacts in between - the failure reported in #834 and #869. + * Walking `requires` names the whole chain, the same set and order + * `openspec status` already prints, without changing what apply blocks on. + */ +function collectMissingPrerequisites(input: { + requiredArtifactIds: string[]; + schema: { artifacts: { id: string; requires: string[] }[] }; + buildOrder: string[]; + completed: Set; +}): string[] { + const { requiredArtifactIds, schema, buildOrder, completed } = input; + const byId = new Map(schema.artifacts.map((artifact) => [artifact.id, artifact])); + const missing = new Set(); + const queue = [...requiredArtifactIds]; + const seen = new Set(queue); + + while (queue.length > 0) { + const id = queue.shift() as string; + const artifact = byId.get(id); + if (!artifact) continue; + if (!completed.has(id)) missing.add(id); + for (const dependency of artifact.requires) { + if (seen.has(dependency)) continue; + seen.add(dependency); + queue.push(dependency); + } + } + + const order = new Map(buildOrder.map((id, index) => [id, index])); + return [...missing].sort( + (a, b) => (order.get(a) ?? 0) - (order.get(b) ?? 0) + ); +} + /** * Warnings apply reports alongside its instruction. * @@ -447,6 +518,14 @@ export async function generateApplyInstructions( } } + // Everything still to build, not just the first hop apply blocks on. + const missingPrerequisites = collectMissingPrerequisites({ + requiredArtifactIds: [...requiredArtifactIds], + schema, + buildOrder: context.graph.getBuildOrder(), + completed: context.completed, + }); + // Build context files from all existing artifacts in schema const contextFiles: Record = {}; for (const artifact of schema.artifacts) { @@ -481,18 +560,35 @@ export async function generateApplyInstructions( if (missingArtifacts.length > 0) { state = 'blocked'; - instruction = `Cannot apply this change yet. Missing artifacts: ${missingArtifacts.join(', ')}.\nUse the openspec-continue-change skill to create the missing artifacts first.`; + const chain = + missingPrerequisites.length > missingArtifacts.length + ? `\nNot created yet, in build order: ${missingPrerequisites.join(', ')}.` + + ` Build the ones this change needs before applying - the schema says which are conditional.` + : ''; + instruction = + `Cannot apply this change yet. Missing artifacts: ${missingArtifacts.join(', ')}.${chain}` + + `\n${describeArtifactRemedy( + changeName, + // Only name one when one is left: the first of several would be the + // schema's conditional artifact as often as not. + missingPrerequisites.length === 1 ? missingPrerequisites[0] : undefined, + { many: missingPrerequisites.length > 1 } + )}`; } else if (tracksFile && !tracksFileExists) { // Tracking file configured but doesn't exist yet const tracksFilename = path.basename(tracksFile); state = 'blocked'; - instruction = `The ${tracksFilename} file is missing and must be created.\nUse openspec-continue-change to generate the tracking file.`; + instruction = + `The ${tracksFilename} file is missing and must be created.` + + `\n${describeArtifactRemedy(changeName, findArtifactIdFor(schema, tracksFile))}`; } else if (tracksFile && tracksFileExists && tasks.length === 0) { // Tracking file exists but lists nothing an agent can work on: either no // checkboxes at all, or only checkboxes with no text after them. const tracksFilename = path.basename(tracksFile); state = 'blocked'; - instruction = `The ${tracksFilename} file exists but contains no tasks to work on.\nAdd tasks to ${tracksFilename} or regenerate it with openspec-continue-change.`; + instruction = + `The ${tracksFilename} file exists but contains no tasks to work on.` + + `\nAdd tasks to ${tracksFilename}, or rebuild it: ${describeArtifactRemedy(changeName, findArtifactIdFor(schema, tracksFile))}`; } else if (tracksFile && remaining === 0 && total > 0) { state = 'all_done'; instruction = 'All tasks are complete! This change is ready to be archived.\nConsider running tests and reviewing the changes before archiving.'; @@ -522,6 +618,7 @@ export async function generateApplyInstructions( tasks, state, missingArtifacts: missingArtifacts.length > 0 ? missingArtifacts : undefined, + ...(missingPrerequisites.length > 0 ? { missingPrerequisites } : {}), ...(warnings.length > 0 ? { warnings } : {}), instruction, ...(references !== undefined ? { references } : {}), @@ -593,7 +690,14 @@ export function printApplyInstructionsText(instructions: ApplyInstructions): voi console.log('### ⚠️ Blocked'); console.log(); console.log(`Missing artifacts: ${missingArtifacts.join(', ')}`); - console.log('Use the openspec-continue-change skill to create these first.'); + if ( + instructions.missingPrerequisites && + instructions.missingPrerequisites.length > missingArtifacts.length + ) { + console.log( + `Not created yet, in build order: ${instructions.missingPrerequisites.join(', ')}` + ); + } console.log(); } diff --git a/src/commands/workflow/shared.ts b/src/commands/workflow/shared.ts index e67d305970..a5975293ee 100644 --- a/src/commands/workflow/shared.ts +++ b/src/commands/workflow/shared.ts @@ -43,6 +43,12 @@ export interface ApplyInstructions { tasks: TaskItem[]; state: 'blocked' | 'all_done' | 'ready'; missingArtifacts?: string[]; + /** + * Everything still to build before apply can run, in build order - the + * transitive closure of the schema's `apply.requires`, so it can be longer + * than `missingArtifacts`, which stops at the first hop apply blocks on. + */ + missingPrerequisites?: string[]; /** Non-blocking problems with the change, reported alongside the instruction. */ warnings?: string[]; instruction: string; diff --git a/test/commands/apply-instructions-blocked.test.ts b/test/commands/apply-instructions-blocked.test.ts new file mode 100644 index 0000000000..798ed32ca7 --- /dev/null +++ b/test/commands/apply-instructions-blocked.test.ts @@ -0,0 +1,119 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + generateApplyInstructions, + printApplyInstructionsText, +} from '../../src/commands/workflow/instructions.js'; + +/** + * Apply blocks on the schema's `apply.requires` alone, so its own list stops at + * the first hop: "Missing artifacts: tasks" for a change that has nothing but a + * proposal. Taken literally that is an instruction to write the tracking file + * straight from the proposal, skipping the artifacts in between. + */ +describe('generateApplyInstructions blocked prerequisites', () => { + let tempDir: string; + let changeDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-apply-blocked-')); + changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change'); + fs.mkdirSync(changeDir, { recursive: true }); + fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: spec-driven\n'); + fs.writeFileSync(path.join(changeDir, 'proposal.md'), '## Why\nx\n'); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + function writeSpecs(): void { + fs.mkdirSync(path.join(changeDir, 'specs', 'demo'), { recursive: true }); + fs.writeFileSync( + path.join(changeDir, 'specs', 'demo', 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Demo\nThe system SHALL demo.\n\n#### Scenario: Works\n- **WHEN** run\n- **THEN** works\n' + ); + } + + it('names the whole chain, not just the artifact apply blocks on', async () => { + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('blocked'); + expect(instructions.missingArtifacts).toEqual(['tasks']); + expect(instructions.missingPrerequisites).toEqual(['specs', 'design', 'tasks']); + expect(instructions.instruction).toContain( + 'Not created yet, in build order: specs, design, tasks' + ); + }); + + it('leaves the conditional artifacts to the schema rather than demanding them', async () => { + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.instruction).toContain('the schema says which are conditional'); + // Naming the first of several would point at design as often as at specs. + expect(instructions.instruction).toContain('openspec instructions '); + }); + + it('drops the chain line once only the required artifact is left', async () => { + writeSpecs(); + fs.writeFileSync(path.join(changeDir, 'design.md'), '# Design\n'); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.missingPrerequisites).toEqual(['tasks']); + expect(instructions.instruction).not.toContain('Not created yet'); + expect(instructions.instruction).toContain( + 'openspec instructions tasks --change my-change' + ); + }); + + it('counts a skipped specs artifact as built', async () => { + fs.writeFileSync( + path.join(changeDir, '.openspec.yaml'), + 'schema: spec-driven\nskip_specs: true\n' + ); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.missingPrerequisites).toEqual(['design', 'tasks']); + }); + + it('points at a command every profile has, never at a skill it may not', async () => { + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + // `continue` is not in CORE_WORKFLOWS, so the default install never had the + // skill the old message named. + expect(instructions.instruction).not.toContain('openspec-continue-change'); + expect(instructions.instruction).toContain('openspec status --change my-change'); + }); + + it('reports no prerequisites once the change is ready to apply', async () => { + writeSpecs(); + fs.writeFileSync(path.join(changeDir, 'design.md'), '# Design\n'); + fs.writeFileSync(path.join(changeDir, 'tasks.md'), '## 1. W\n- [ ] 1.1 Do it\n'); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.state).toBe('ready'); + expect(instructions.missingPrerequisites).toBeUndefined(); + }); + + it('prints the chain under the blocked heading', async () => { + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + const lines: string[] = []; + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + lines.push(args.join(' ')); + }); + printApplyInstructionsText(instructions); + vi.restoreAllMocks(); + const output = lines.join('\n'); + + expect(output).toContain('Missing artifacts: tasks'); + expect(output).toContain('Not created yet, in build order: specs, design, tasks'); + expect(output).not.toContain('openspec-continue-change'); + }); +}); diff --git a/test/commands/apply-instructions-warnings.test.ts b/test/commands/apply-instructions-warnings.test.ts index dce08ab28d..dacc640b81 100644 --- a/test/commands/apply-instructions-warnings.test.ts +++ b/test/commands/apply-instructions-warnings.test.ts @@ -6,6 +6,7 @@ import { generateApplyInstructions, printApplyInstructionsText, } from '../../src/commands/workflow/instructions.js'; +import { Validator } from '../../src/core/validation/validator.js'; /** * Apply gates on the schema's `apply.requires` (tasks) alone, so a change whose @@ -194,6 +195,28 @@ describe('generateApplyInstructions warnings', () => { expect(instructions.warnings).toHaveLength(1); }); + // The warning tells the author `openspec validate` fails on this change. If + // that ever stops being true the warning is a lie, so pin it to the validator + // rather than to a copy of its rule. + it('warns about exactly the state the validator rejects', async () => { + writeTasks(); + const warned = await generateApplyInstructions(tempDir, 'my-change'); + const rejected = await new Validator().validateChangeDeltaSpecs(changeDir); + + expect(warned.warnings).toHaveLength(1); + expect(rejected.valid).toBe(false); + }); + + it('stays quiet about exactly the state the validator accepts', async () => { + writeTasks(); + writeSpecs(); + const quiet = await generateApplyInstructions(tempDir, 'my-change'); + const accepted = await new Validator().validateChangeDeltaSpecs(changeDir); + + expect(quiet.warnings).toBeUndefined(); + expect(accepted.valid).toBe(true); + }); + it('prints no warnings section when there is nothing to warn about', async () => { writeTasks(); writeSpecs(); From 5ef978114a8ab603f6d6f0dfc0a14fa63976f4d2 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 9 Sep 2026 07:30:57 -0500 Subject: [PATCH 6/7] fix(apply): name the schema's own spec artifact in the warning alfred-openspec on #1783: collectApplyWarnings() discovers spec-producing artifacts by output path, so it correctly fires for a schema whose artifact id is `contracts`, but the remediation text then hardcoded `openspec instructions specs`. That names an artifact such a schema does not declare, so the advertised custom-schema support dead-ended at the exact step meant to resolve the warning. The command now derives its target from specArtifacts: the artifact's own id when the schema declares one spec-producing artifact, and `` as a placeholder when it declares several, since there is no single right answer there and a guess would read as an instruction. The renamed-artifact test now asserts the command names `contracts` and rejects the hardcoded `specs` spelling, and a new test pins the two-artifact placeholder. Verified both fail against the hardcoded string. Co-Authored-By: Claude Opus 5 --- src/commands/workflow/instructions.ts | 9 ++- .../apply-instructions-warnings.test.ts | 59 +++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index f2e93c1a51..cc2b143bb2 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -458,9 +458,16 @@ function collectApplyWarnings(input: { if (hasDeltas) return []; const metadataPath = path.join(changeDir, METADATA_FILENAME); + // The command names the artifact this schema actually declares, never the + // literal `specs`. A schema whose spec-producing artifact is `contracts` was + // told to run `openspec instructions specs`, an artifact it does not have, + // so the warning dead-ended at the exact step meant to resolve it. With more + // than one such artifact there is no single right answer, so the id becomes + // a placeholder rather than a guess. + const specTarget = specArtifacts.length === 1 ? specArtifacts[0].id : ''; return [ `This change has no delta specs and does not declare \`skip_specs: true\`, so \`openspec validate ${changeName}\` fails on it. ` + - `Write the delta specs before implementing (\`openspec instructions specs --change ${changeName}\`), ` + + `Write the delta specs before implementing (\`openspec instructions ${specTarget} --change ${changeName}\`), ` + `or add \`skip_specs: true\` to ${metadataPath} if this change really changes no specified behavior.`, ]; } diff --git a/test/commands/apply-instructions-warnings.test.ts b/test/commands/apply-instructions-warnings.test.ts index dacc640b81..44f296e927 100644 --- a/test/commands/apply-instructions-warnings.test.ts +++ b/test/commands/apply-instructions-warnings.test.ts @@ -193,6 +193,65 @@ describe('generateApplyInstructions warnings', () => { expect(instructions.state).toBe('ready'); expect(instructions.warnings).toHaveLength(1); + // The remediation has to name this schema's own artifact. Hardcoding + // `specs` sent the agent to an artifact this schema does not declare, so + // the warning dead-ended at the step meant to resolve it. + expect(instructions.warnings?.[0]).toContain( + 'openspec instructions contracts --change my-change' + ); + expect(instructions.warnings?.[0]).not.toContain('openspec instructions specs'); + }); + + it('falls back to a placeholder when a schema declares two spec artifacts', async () => { + // No single right answer, so the command must not pick one and present it + // as the step to run. + const schemaDir = path.join(tempDir, 'openspec', 'schemas', 'twospec'); + fs.mkdirSync(schemaDir, { recursive: true }); + fs.writeFileSync( + path.join(schemaDir, 'schema.yaml'), + [ + 'name: twospec', + 'version: 1', + 'artifacts:', + ' - id: proposal', + ' generates: proposal.md', + ' description: p', + ' template: proposal.md', + ' - id: contracts', + ' generates: "specs/**/*.md"', + ' description: c', + ' template: spec.md', + ' requires: [proposal]', + ' - id: schemas', + ' generates: "specs/**/*.yaml"', + ' description: s', + ' template: spec.md', + ' requires: [proposal]', + ' - id: tasks', + ' generates: tasks.md', + ' description: t', + ' template: tasks.md', + ' requires: [proposal]', + 'apply:', + ' requires: [tasks]', + ' tracks: tasks.md', + '', + ].join('\n') + ); + fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: twospec\n'); + writeTasks(); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.warnings).toHaveLength(1); + expect(instructions.warnings?.[0]).toContain( + 'openspec instructions --change my-change' + ); + // Presence of the placeholder is not enough: naming either artifact as + // well would still be picking one, which is the thing there is no basis + // for here. + expect(instructions.warnings?.[0]).not.toContain('openspec instructions contracts'); + expect(instructions.warnings?.[0]).not.toContain('openspec instructions schemas'); }); // The warning tells the author `openspec validate` fails on this change. If From 13b0eb1d9a40cce3ab56eeec56e8df2952b420ff Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 9 Sep 2026 10:49:21 -0500 Subject: [PATCH 7/7] chore(changeset): bump apply warnings to minor This adds `missingPrerequisites` and `warnings` to the documented `instructions apply --json` contract in docs/agent-contract.md. New fields are backward compatible, but they are new capability an agent can consume, which is a minor under semver rather than a patch. Taking the conservative direction deliberately: shipping new API surface as a patch is the violation, since a consumer pinned to a patch range would receive it without opting in. A minor costs nothing if the fields turn out to be uninteresting. Co-Authored-By: Claude Opus 5 --- .changeset/apply-warns-missing-specs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/apply-warns-missing-specs.md b/.changeset/apply-warns-missing-specs.md index e9885a7af3..f95a0a90b6 100644 --- a/.changeset/apply-warns-missing-specs.md +++ b/.changeset/apply-warns-missing-specs.md @@ -1,5 +1,5 @@ --- -'@fission-ai/openspec': patch +'@fission-ai/openspec': minor --- Apply now says when a change has no delta specs. Apply gates on the schema's `apply.requires` alone, so a change whose `tasks.md` was written ahead of its specs read as ready to implement even though it had no spec deltas at all — the state `openspec validate` rejects. `openspec instructions apply` now reports that gap as a warning (text and `--json`), naming both ways out: write the specs, or declare `skip_specs: true`. Changes that have specs, declare `skip_specs`, or are still blocked on their own required artifacts are unaffected.