From 728b94d4146a205ad80f7372820a091dfa7c35e7 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 28 Aug 2026 11:14:50 -0500 Subject: [PATCH 1/2] fix(workflows): remove unavailable core-profile handoffs --- .changeset/fix-core-workflow-handoffs.md | 5 + skills/openspec-apply-change/SKILL.md | 2 +- skills/openspec-update-change/SKILL.md | 18 +-- src/core/templates/workflows/apply-change.ts | 2 +- src/core/templates/workflows/update-change.ts | 36 ++--- test/commands/profile-handoffs.test.ts | 137 ++++++++++++++++++ test/core/init.test.ts | 39 +---- test/core/templates/profile-handoffs.test.ts | 59 ++++++++ .../templates/skill-templates-parity.test.ts | 12 +- test/core/templates/update-change.test.ts | 53 +++---- test/utils/command-references.test.ts | 22 ++- 11 files changed, 273 insertions(+), 112 deletions(-) create mode 100644 .changeset/fix-core-workflow-handoffs.md create mode 100644 test/commands/profile-handoffs.test.ts create mode 100644 test/core/templates/profile-handoffs.test.ts diff --git a/.changeset/fix-core-workflow-handoffs.md b/.changeset/fix-core-workflow-handoffs.md new file mode 100644 index 0000000000..d93e46388b --- /dev/null +++ b/.changeset/fix-core-workflow-handoffs.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Remove optional continue/new workflow handoffs from update and apply guidance. Missing artifacts now point directly to the existing status/instructions CLI, and intent changes use new change without requiring an installed skill. Preserve update's planning-only scope, store selection, and the core workflow set. diff --git a/skills/openspec-apply-change/SKILL.md b/skills/openspec-apply-change/SKILL.md index 098f63fecb..2166025f60 100644 --- a/skills/openspec-apply-change/SKILL.md +++ b/skills/openspec-apply-change/SKILL.md @@ -50,7 +50,7 @@ Implement tasks from an OpenSpec change. - Optional `operationGuidance`: current advisory guidance for apply **Handle states:** - - If `state: "blocked"` (missing artifacts): show message, suggest using `/openspec-continue-change` (if it is not installed, run `openspec status --change "" --json` to see the next artifact and `openspec instructions --change "" --json` for how to create it) + - If `state: "blocked"` (missing artifacts): show the message and pause implementation. Suggest completing the missing artifacts: run `openspec status --change "" --json`, select the next `ready` artifact (not `skipped` or `blocked`), and use `openspec instructions "" --change "" --json` for its rules and template. Keep the selected `--store ` on both commands. - If `state: "all_done"`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/skills/openspec-update-change/SKILL.md b/skills/openspec-update-change/SKILL.md index 24c9f88367..488778bd4e 100644 --- a/skills/openspec-update-change/SKILL.md +++ b/skills/openspec-update-change/SKILL.md @@ -15,8 +15,6 @@ Revise a change's existing planning artifacts and keep them coherent. Never edit **Input**: Optionally specify a change name. If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. -`/openspec-continue-change` is an optional workflow and may not be installed. Before suggesting it anywhere below, verify that it is available. If it is unavailable, `openspec status --change "" --json` shows the next artifact and `openspec instructions "" --change "" --json` explains how to create it. - **Steps** 1. **Select the change** @@ -58,7 +56,7 @@ Revise a change's existing planning artifacts and keep them coherent. Never edit - Read the artifact(s) the request touches and the change's other existing artifacts. - Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised. - Note everything that is now inconsistent, missing, or contradictory. - - Revise only files that already exist (`existingOutputPaths`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to `/openspec-continue-change` to create them. + - Revise only files that already exist (`existingOutputPaths`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them for a separate artifact-creation step using `openspec instructions "" --change "" --json` for the schema's rules and template. - If the change is already coherent, say so and make no edits. 5. **Confirm and apply, one artifact at a time** @@ -70,21 +68,21 @@ Revise a change's existing planning artifacts and keep them coherent. Never edit ``` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> suggest `/openspec-continue-change` to create them. - - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest `/openspec-apply-change` to carry the delta into code. - - Everything done and implemented -> suggest `/openspec-archive-change`. + - Artifacts still missing -> suggest completing them in a separate artifact-creation step: run `openspec status --change "" --json`, select the next `ready` artifact (not `skipped` or `blocked`), and use `openspec instructions "" --change "" --json` for its rules and template. Keep the selected `--store ` on both commands. + - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest implementing the revised plan in a separate implementation step. + - Everything done and implemented -> suggest archiving the change. **Output** After each invocation, show: - Which artifacts were revised (and which proposed revisions were rejected) -- Anything deferred to `/openspec-continue-change` (not-yet-created artifacts or files) +- Anything deferred to separate artifact creation (not-yet-created artifacts or files) - Where the change stands and the recommended next command **Guardrails** -- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and point to `/openspec-apply-change`. +- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and suggest a separate implementation step. - Use the artifact ids and paths reported by `openspec status`; never branch on hardcoded artifact names. - Edit only the concrete files in `existingOutputPaths`; never write to a glob `resolvedOutputPath`. -- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is `/openspec-continue-change`'s job. +- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - defer them to the separate artifact-creation step described above. - Confirm every edit with the user before writing. -- If the request changes the change's *intent* rather than refining it, first verify whether the optional `/openspec-new-change` workflow is available. If it is, recommend starting fresh with `/openspec-new-change` (the "Update vs. Start Fresh" heuristic). If it is unavailable, ask for a distinct unused change name and recommend `openspec new change ""` instead. +- If the request changes the change's *intent* rather than refining it, recommend starting a separate change (the "Update vs. Start Fresh" heuristic). Ask for a distinct unused change name and recommend `openspec new change ""`; keep the current change unchanged. diff --git a/src/core/templates/workflows/apply-change.ts b/src/core/templates/workflows/apply-change.ts index d7ebe2f4eb..4a18cc29e9 100644 --- a/src/core/templates/workflows/apply-change.ts +++ b/src/core/templates/workflows/apply-change.ts @@ -58,7 +58,7 @@ ${STORE_SELECTION_GUIDANCE} - Optional \`operationGuidance\`: current advisory guidance for apply **Handle states:** - - If \`state: "blocked"\` (missing artifacts): show message, suggest using \`/opsx:continue\` (if it is not installed, run \`openspec status --change "" --json\` to see the next artifact and \`openspec instructions --change "" --json\` for how to create it) + - If \`state: "blocked"\` (missing artifacts): show the message and pause implementation. Suggest completing the missing artifacts: run \`openspec status --change "" --json\`, select the next \`ready\` artifact (not \`skipped\` or \`blocked\`), and use \`openspec instructions "" --change "" --json\` for its rules and template. Keep the selected \`--store \` on both commands. - If \`state: "all_done"\`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 7700cd8d7e..f491947ac3 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -17,8 +17,6 @@ ${STORE_SELECTION_GUIDANCE} **Input**: Optionally specify a change name. If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. -\`/opsx:continue\` is an optional workflow and may not be installed. Before suggesting it anywhere below, verify that it is available. If it is unavailable, \`openspec status --change "" --json\` shows the next artifact and \`openspec instructions "" --change "" --json\` explains how to create it. - **Steps** 1. **Select the change** @@ -60,7 +58,7 @@ ${STORE_SELECTION_GUIDANCE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised. - Note everything that is now inconsistent, missing, or contradictory. - - Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to \`/opsx:continue\` to create them. + - Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them for a separate artifact-creation step using \`openspec instructions "" --change "" --json\` for the schema's rules and template. - If the change is already coherent, say so and make no edits. 5. **Confirm and apply, one artifact at a time** @@ -72,24 +70,24 @@ ${STORE_SELECTION_GUIDANCE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> suggest \`/opsx:continue\` to create them. - - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest \`/opsx:apply\` to carry the delta into code. - - Everything done and implemented -> suggest \`/opsx:archive\`. + - Artifacts still missing -> suggest completing them in a separate artifact-creation step: run \`openspec status --change "" --json\`, select the next \`ready\` artifact (not \`skipped\` or \`blocked\`), and use \`openspec instructions "" --change "" --json\` for its rules and template. Keep the selected \`--store \` on both commands. + - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest implementing the revised plan in a separate implementation step. + - Everything done and implemented -> suggest archiving the change. **Output** After each invocation, show: - Which artifacts were revised (and which proposed revisions were rejected) -- Anything deferred to \`/opsx:continue\` (not-yet-created artifacts or files) +- Anything deferred to separate artifact creation (not-yet-created artifacts or files) - Where the change stands and the recommended next command **Guardrails** -- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and point to \`/opsx:apply\`. +- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and suggest a separate implementation step. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is \`/opsx:continue\`'s job. +- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - defer them to the separate artifact-creation step described above. - Confirm every edit with the user before writing. -- If the request changes the change's *intent* rather than refining it, first verify whether the optional \`/opsx:new\` workflow is available. If it is, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic). If it is unavailable, ask for a distinct unused change name and recommend \`openspec new change ""\` instead.`, +- If the request changes the change's *intent* rather than refining it, recommend starting a separate change (the "Update vs. Start Fresh" heuristic). Ask for a distinct unused change name and recommend \`openspec new change ""\`; keep the current change unchanged.`, license: 'MIT', compatibility: 'Requires openspec CLI.', metadata: { author: 'openspec', version: '1.0' }, @@ -108,8 +106,6 @@ ${STORE_SELECTION_GUIDANCE} **Input**: Optionally specify a change name after \`/opsx:update\` (e.g., \`/opsx:update add-auth\`). If omitted, check if it can be inferred from conversation context. If vague or ambiguous you MUST prompt for available changes. -\`/opsx:continue\` is an optional workflow and may not be installed. Before suggesting it anywhere below, verify that it is available. If it is unavailable, \`openspec status --change "" --json\` shows the next artifact and \`openspec instructions "" --change "" --json\` explains how to create it. - **Steps** 1. **Select the change** @@ -151,7 +147,7 @@ ${STORE_SELECTION_GUIDANCE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Apply the requested edit. Then check every other existing artifact against it - in ANY direction: an edit to a later artifact may require revising an earlier one, not only the other way around. Build order is a useful reading order, not a constraint on which artifacts may be revised. - Note everything that is now inconsistent, missing, or contradictory. - - Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them and point the user to \`/opsx:continue\` to create them. + - Revise only files that already exist (\`existingOutputPaths\`). Do NOT create artifacts that don't exist yet, and do NOT invent new files under a glob artifact - note them for a separate artifact-creation step using \`openspec instructions "" --change "" --json\` for the schema's rules and template. - If the change is already coherent, say so and make no edits. 5. **Confirm and apply, one artifact at a time** @@ -163,23 +159,23 @@ ${STORE_SELECTION_GUIDANCE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> suggest \`/opsx:continue\` to create them. - - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest \`/opsx:apply\` to carry the delta into code. - - Everything done and implemented -> suggest \`/opsx:archive\`. + - Artifacts still missing -> suggest completing them in a separate artifact-creation step: run \`openspec status --change "" --json\`, select the next \`ready\` artifact (not \`skipped\` or \`blocked\`), and use \`openspec instructions "" --change "" --json\` for its rules and template. Keep the selected \`--store \` on both commands. + - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; suggest implementing the revised plan in a separate implementation step. + - Everything done and implemented -> suggest archiving the change. **Output** After each invocation, show: - Which artifacts were revised (and which proposed revisions were rejected) -- Anything deferred to \`/opsx:continue\` (not-yet-created artifacts or files) +- Anything deferred to separate artifact creation (not-yet-created artifacts or files) - Where the change stands and the recommended next command **Guardrails** -- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and point to \`/opsx:apply\`. +- Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, stop and suggest a separate implementation step. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is \`/opsx:continue\`'s job. +- Do not advance the build frontier: no new artifacts, no new files under glob artifacts - defer them to the separate artifact-creation step described above. - Confirm every edit with the user before writing. -- If the request changes the change's *intent* rather than refining it, first verify whether the optional \`/opsx:new\` workflow is available. If it is, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic). If it is unavailable, ask for a distinct unused change name and recommend \`openspec new change ""\` instead.` +- If the request changes the change's *intent* rather than refining it, recommend starting a separate change (the "Update vs. Start Fresh" heuristic). Ask for a distinct unused change name and recommend \`openspec new change ""\`; keep the current change unchanged.` }; } diff --git a/test/commands/profile-handoffs.test.ts b/test/commands/profile-handoffs.test.ts new file mode 100644 index 0000000000..3df0f7b054 --- /dev/null +++ b/test/commands/profile-handoffs.test.ts @@ -0,0 +1,137 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { promises as fs } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { getGlobalDataDir, registerStore } from '../../src/core/index.js'; +import { ALL_WORKFLOWS, CORE_WORKFLOWS } from '../../src/core/profiles.js'; +import { getSkillTemplates } from '../../src/core/shared/skill-generation.js'; +import { createOpenSpecRoot } from '../helpers/openspec-fixtures.js'; +import { runCLI } from '../helpers/run-cli.js'; + +describe('profile handoff CLI regressions (#1734)', () => { + let tempDir: string; + let project: string; + let env: NodeJS.ProcessEnv; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-profile-handoffs-')); + project = path.join(tempDir, 'project'); + await fs.mkdir(project); + env = { + XDG_CONFIG_HOME: path.join(tempDir, 'config'), + XDG_DATA_HOME: path.join(tempDir, 'data'), + USERPROFILE: path.join(tempDir, 'user-profile'), // Isolate global skill discovery (MiniMax). + }; + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + async function cli(args: string[]): Promise { + const result = await runCLI(args, { cwd: project, env }); + expect(result.exitCode, result.stderr || result.stdout).toBe(0); + return result.stdout; + } + + it.each(['skills', 'commands', 'both'])('keeps fresh and refreshed core %s free of optional handoffs', async (delivery) => { + const configDir = path.join(env.XDG_CONFIG_HOME!, 'openspec'); + await fs.mkdir(configDir, { recursive: true }); + const configFile = path.join(configDir, 'config.json'); + await fs.writeFile(configFile, JSON.stringify({ profile: 'core', delivery })); + await cli(['init', '--tools', 'claude', '--profile', 'core', '--no-animation', '--force']); + + const files = [ + ...(delivery !== 'commands' ? getSkillTemplates(CORE_WORKFLOWS).map(({ dirName }) => + path.join(project, '.claude', 'skills', dirName, 'SKILL.md')) : []), + ...(delivery !== 'skills' ? CORE_WORKFLOWS.map(id => + path.join(project, '.claude', 'commands', 'opsx', `${id}.md`)) : []), + ]; + async function assertCore(): Promise { + if (delivery !== 'commands') { + expect((await fs.readdir(path.join(project, '.claude', 'skills'))).sort()) + .toEqual(getSkillTemplates(CORE_WORKFLOWS).map(entry => entry.dirName).sort()); + } + if (delivery !== 'skills') { + expect((await fs.readdir(path.join(project, '.claude', 'commands', 'opsx'))).sort()) + .toEqual(CORE_WORKFLOWS.map(id => `${id}.md`).sort()); + } + for (const file of files) { + const content = await fs.readFile(file, 'utf-8'); + expect(content, file).not.toMatch(/\/opsx:(continue|new)\b|openspec-(continue|new)-change/); + } + } + await assertCore(); + + // Simulate files generated by an older release, then use the normal refresh path. + for (const file of files) { + const old = (await fs.readFile(file, 'utf-8')).replace(/generatedBy: "[^"]+"/, 'generatedBy: "0.0.0"'); + await fs.writeFile(file, `${old}\nStale handoff: /opsx:continue\n`); + } + await cli(['update']); + await assertCore(); + + // Explicit opt-in still works; switching back removes the optional files. + await fs.writeFile(configFile, JSON.stringify({ profile: 'custom', delivery, workflows: ALL_WORKFLOWS })); + await cli(['update']); + const continueFile = delivery === 'commands' + ? path.join(project, '.claude', 'commands', 'opsx', 'continue.md') + : path.join(project, '.claude', 'skills', 'openspec-continue-change', 'SKILL.md'); + expect(await fs.readFile(continueFile, 'utf-8')).toContain('Continue working on a change'); + await fs.writeFile(configFile, JSON.stringify({ profile: 'core', delivery })); + await cli(['update']); + await assertCore(); + }, 30_000); + + it.each(['local', 'store'])('supports the CLI recovery with custom artifact ids in a %s root', async (scope) => { + const root = scope === 'store' ? path.join(tempDir, 'planning-store') : project; + createOpenSpecRoot(root); + const flags = scope === 'store' ? ['--store', 'planning'] : []; + if (scope === 'store') { + await registerStore({ id: 'planning', localPath: root, globalDataDir: getGlobalDataDir({ env }) }); + createOpenSpecRoot(project); // A local root must not steal the selected store's change. + } + const schemaDir = path.join(root, 'openspec', 'schemas', 'handoff-test'); + await fs.mkdir(path.join(schemaDir, 'templates'), { recursive: true }); + await fs.writeFile(path.join(schemaDir, 'schema.yaml'), `name: handoff-test +version: 1 +artifacts: + - id: brief + generates: brief.md + description: Planning brief + template: brief.md + instruction: Explain the intended behavior. + requires: [] +apply: + requires: [brief] + instruction: Implement the brief. +`); + await fs.writeFile(path.join(schemaDir, 'templates', 'brief.md'), '# Brief\n'); + await fs.writeFile(path.join(root, 'openspec', 'config.yaml'), 'schema: handoff-test\ncontext: Preserve existing behavior.\nrules:\n brief:\n - Include a verification plan.\n'); + await cli(['new', 'change', 'original', ...flags]); + const changeRoot = path.join(root, 'openspec', 'changes', 'original'); + const before = await fs.readdir(changeRoot); + const blocked = JSON.parse(await cli(['instructions', 'apply', '--change', 'original', '--json', ...flags])); + expect(blocked.state).toBe('blocked'); + const status = JSON.parse(await cli(['status', '--change', 'original', '--json', ...flags])); + const next = status.artifacts.find((artifact: { status: string }) => artifact.status === 'ready'); + expect(next.id).toBe('brief'); + const instructions = JSON.parse(await cli(['instructions', next.id, '--change', 'original', '--json', ...flags])); + expect(instructions.template).toBe('# Brief\n'); + expect(instructions.instruction).toBe('Explain the intended behavior.'); + expect(instructions.context).toBe('Preserve existing behavior.'); + expect(instructions.rules).toEqual(['Include a verification plan.']); + expect(instructions.resolvedOutputPath).toBe(path.join(await fs.realpath(changeRoot), 'brief.md')); + expect(await fs.readdir(changeRoot)).toEqual(before); // Status/instructions do not create artifacts. + + // A fixture write models the separate creation step, not an agent execution. + await fs.writeFile(instructions.resolvedOutputPath, '# Brief\n\nConfirmed planning artifact.\n'); + const ready = JSON.parse(await cli(['instructions', 'apply', '--change', 'original', '--json', ...flags])); + expect(ready.state).toBe('ready'); + await cli(['new', 'change', 'different-intent', ...flags]); + expect(await fs.readFile(instructions.resolvedOutputPath, 'utf-8')).toBe('# Brief\n\nConfirmed planning artifact.\n'); + if (scope === 'store') { + expect(await fs.readdir(path.join(project, 'openspec', 'changes'))).toEqual(['archive']); + } + }, 30_000); +}); diff --git a/test/core/init.test.ts b/test/core/init.test.ts index 3c20733671..95807053db 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -442,41 +442,14 @@ describe('InitCommand', () => { ); } - const updateVariants: Array<[string, string]> = [ - [ - await fs.readFile( - path.join( - testDir, - '.claude', - 'skills', - 'openspec-update-change', - 'SKILL.md' - ), - 'utf-8' - ), - '`/opsx:continue`', - ], - [ - await fs.readFile( - path.join(testDir, '.claude', 'commands', 'opsx', 'update.md'), - 'utf-8' - ), - '`/opsx:continue`', - ], + const updateFiles = [ + path.join(testDir, '.claude', 'skills', 'openspec-update-change', 'SKILL.md'), + path.join(testDir, '.claude', 'commands', 'opsx', 'update.md'), ]; - for (const [content, continueReference] of updateVariants) { - const availabilityGuidance = content.indexOf( - `${continueReference} is an optional workflow and may not be installed` - ); - const nextReference = content.indexOf( - continueReference, - availabilityGuidance + continueReference.length - ); - - expect(availabilityGuidance).toBeGreaterThanOrEqual(0); - expect(content.indexOf(continueReference)).toBe(availabilityGuidance); - expect(nextReference).toBeGreaterThan(availabilityGuidance); + for (const file of updateFiles) { + const content = await fs.readFile(file, 'utf-8'); + expect(content).not.toMatch(/\/opsx:(continue|new)\b/); expect(content).toContain('openspec status --change "" --json'); expect(content).toContain( 'openspec instructions "" --change "" --json' diff --git a/test/core/templates/profile-handoffs.test.ts b/test/core/templates/profile-handoffs.test.ts new file mode 100644 index 0000000000..1e20573841 --- /dev/null +++ b/test/core/templates/profile-handoffs.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { AI_TOOLS } from '../../../src/core/config.js'; +import { ALL_WORKFLOWS, getProfileWorkflows } from '../../../src/core/profiles.js'; +import { + resolveCommandInvocation, + resolveCommandSurfaceCapability, + shouldGenerateCommandsForTool, + shouldGenerateSkillsForTool, +} from '../../../src/core/command-surface.js'; +import { CommandAdapterRegistry, generateCommands } from '../../../src/core/command-generation/index.js'; +import { + generateSkillContent, + getCommandContents, + getSkillTemplates, +} from '../../../src/core/shared/skill-generation.js'; +import { getTransformerForTool } from '../../../src/utils/command-references.js'; + +const profiles = [ + { name: 'core', workflows: getProfileWorkflows('core') }, + { name: 'custom expanded', workflows: getProfileWorkflows('custom', [...ALL_WORKFLOWS]) }, + { name: 'custom update only', workflows: getProfileWorkflows('custom', ['update']) }, + { name: 'custom archive with sync dependency', workflows: getProfileWorkflows('custom', ['archive']) }, +]; +const skillWorkflows = new Map(getSkillTemplates().map(({ dirName, workflowId }) => [dirName, workflowId])); + +// Check both invocation spellings and bare skill names (archive's sync handoff). +// Unknown references also fail: a typo must not make the guard silently pass. +function expectInstalledReferences(content: string, workflows: readonly string[], label: string): void { + for (const match of content.matchAll(/[/@]opsx[:-]([a-z-]+)|\b(openspec-[a-z-]+)\b/g)) { + const workflow = match[1] ?? skillWorkflows.get(match[2]); + expect(workflows, `${label}: unavailable workflow reference ${match[0]}`).toContain(workflow); + } +} + +describe.each(profiles)('$name workflow handoffs', ({ workflows }) => { + for (const delivery of ['skills', 'commands', 'both'] as const) { + const tools = AI_TOOLS.filter(tool => tool.skillsDir && ( + shouldGenerateSkillsForTool(tool.value, delivery) || shouldGenerateCommandsForTool(tool.value, delivery) + )); + it.each(tools)(`only references installed workflows for $value (${delivery})`, (tool) => { + if (shouldGenerateSkillsForTool(tool.value, delivery)) { + const transformer = getTransformerForTool( + tool.value, delivery, + resolveCommandSurfaceCapability(tool.value), + resolveCommandInvocation(tool.value), + ); + for (const { template, workflowId } of getSkillTemplates(workflows)) { + expectInstalledReferences(generateSkillContent(template, 'TEST', transformer), workflows, workflowId); + } + } + if (shouldGenerateCommandsForTool(tool.value, delivery)) { + const adapter = CommandAdapterRegistry.get(tool.value)!; + for (const command of generateCommands(getCommandContents(workflows), adapter)) { + expectInstalledReferences(command.fileContent, workflows, command.path); + } + } + }); + } +}); diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index c7a8b44b5c..6b67a297f3 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -41,14 +41,14 @@ const EXPECTED_FUNCTION_HASHES: Record = { getExploreSkillTemplate: 'ecaa0bea4c1cd14eee9dbfcfe4b5808fff4ff808cba0a46789b37c1df3048d9a', getNewChangeSkillTemplate: 'eabd1e895c5881dcb17dcbaa3fb26098dd59e8eacb318e400820b4dc811ef781', getContinueChangeSkillTemplate: '012136f6411a99c8fa228e2f9444cb64b0a89e0f56fdeac2fe03b2f5bee0c5d7', - getApplyChangeSkillTemplate: 'd1e7d5ceb85193c0964057dbb88e9651526754bd33f84020e2440ff0621d5dbb', + getApplyChangeSkillTemplate: '4e35287f57312bf9f9ec30a5da7abc3e4ade89ef9cf1398d5f72668ae4e327f9', getFfChangeSkillTemplate: '5501740e7ec36ab23ab8c3a0d6dd0655a5e2f35433c7b90e82904fef5e7a326a', getSyncSpecsSkillTemplate: 'b099e2ff31859c9b10d928066e662524f9aad9ecf2be12fceacb732d718c4146', getOnboardSkillTemplate: '3a836faae463d88c289a1c129cb7ee556a563b7e53e1a52a4711ff152a3b51f7', getOpsxExploreCommandTemplate: '1460fcb4fbdf22244e9e76608102e611db598cd4cca8c5dbd001292854bcba6e', getOpsxNewCommandTemplate: 'f2d30e569798a4c92ba932859d6ba4e0ad10e18feccbade1cfee0957597b3463', getOpsxContinueCommandTemplate: 'e50e50266efa1b8e64ff9b6274ee8254f0a240d6adc1b862d126e2f1c9d3a559', - getOpsxApplyCommandTemplate: 'e3579ac78f2e2c75fa3d3a7ac7dc3e49c395e96f7323398f0f041d94f8de9bb0', + getOpsxApplyCommandTemplate: 'f3940c5f1798886a34d4d51f89f5a9a6f474b17f9029ffa239b3a18d5bb1cdef', getOpsxFfCommandTemplate: 'e603bc0996604e6c17a3140943ea642a32d0fc65565e25424bf956e124c55772', getArchiveChangeSkillTemplate: '56bfada1a5f35a127791b70de9d428a75b5aedd1584d6c9803a1ecb1fd1b4a23', getBulkArchiveChangeSkillTemplate: '93875998cade5322d95b43299fba794bc1da754e917dd63a770406386a6d295d', @@ -61,15 +61,15 @@ const EXPECTED_FUNCTION_HASHES: Record = { getOpsxProposeSkillTemplate: '24623c066f97e34b957d448d1f9a9e8b8a13da3dfce45d45671f6226a2534848', getOpsxProposeCommandTemplate: 'e67ba591efb0fecacb2229d06dfa84af18b825fab8a7b01377279e4f09a06ce4', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', - getUpdateChangeSkillTemplate: '7dc8abc6f64c58bf34d7581ed4ab095a3b7a53cb372349bee2d840db58622819', - getOpsxUpdateCommandTemplate: 'e2388521b22f92f74561df9a0c2f98e1fa4d265af93b5ba26f42fb47a6c5bfed', + getUpdateChangeSkillTemplate: '0434e0c17f697cb47ec32f99857a0aa921f90027a6ce942a564ffc226dcb899e', + getOpsxUpdateCommandTemplate: '6ae71add33ae12297171a8d20eafc29c0b6f78e37eb3c5daff7176da570af9a8', }; const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-explore': '886680e71f2900378bd12bb9ff25c888a41a8f851e0bb3ec056affcc18d07ca8', 'openspec-new-change': 'ec4529beef978e34634a6f7286fab55d68fad8fb374dceb45691d52caab33fbb', 'openspec-continue-change': 'bb6194a16c54891cdb253678e8f70ce53b2af86735243980f366ce551d37e42e', - 'openspec-apply-change': '81ea96d9fa6ec8536cd23c1fe561ed28e1cc1cad0a8ceb700588e08974cc0e49', + 'openspec-apply-change': 'bf68c07d1f60ac379c11246bc815ff6c513c3a20c219a4fa8ea9a9b972392fc5', 'openspec-ff-change': '217c78da2b6e8358f609ac57dcd02266aaec3354ce26dc6ec2fc9c2174673ab4', 'openspec-sync-specs': 'd933d8856584d6c1253de91e652e7aee9e85c77ad4d3531f6476f79d84e6e5e8', 'openspec-archive-change': '7c65053d674ba4e1e20e2bf73ba7e5a7f94baef2eaa9b33cee48d4cadea51b7a', @@ -77,7 +77,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-verify-change': 'af9be013dcbe8c6d8f6d9ab10c893fbd03f4c62933c384d82f63894dd0ceb84f', 'openspec-onboard': 'f6f59476acaf5e4d65dbb180da4cef62432612f3cecf207d471a951295e2003a', 'openspec-propose': '25d08ed4f031770cea219604167d76bca9f3e89fe0c2f545263674482c6f13f0', - 'openspec-update-change': '586547406aca94422dfeb3ffedce6c01049429b743f57ce829baa79ebc714d51', + 'openspec-update-change': '115606aaf05c0e3fc139cc949d7d3fe3b0470a2ae03e54ec8020a1f9c1859fd1', }; // Intentionally excludes getFeedbackSkillTemplate: this list only models templates diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index cf68f234f5..67b647c1a9 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -49,11 +49,11 @@ describe('update-change templates', () => { } }); - it('edits planning artifacts only, hands code off to /opsx:apply, never advances the frontier (3.3)', () => { + it('edits planning artifacts only, defers implementation, never advances the frontier (3.3)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('Never edit code'); expect(body, label).toContain('NEVER edit implementation code'); - expect(body, label).toContain('stop and point to `/opsx:apply`'); + expect(body, label).toContain('stop and suggest a separate implementation step'); expect(body, label).toContain('Do not advance the build frontier'); expect(body, label).toContain('Do NOT create artifacts that don\'t exist yet'); } @@ -70,52 +70,39 @@ describe('update-change templates', () => { it('ends with next-step guidance and never acts on it (3.5)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('guidance only - NEVER act on it'); - expect(body, label).toContain('suggest `/opsx:continue`'); - expect(body, label).toContain('suggest `/opsx:apply`'); - expect(body, label).toContain('suggest `/opsx:archive`'); + expect(body, label).toContain('suggest completing them in a separate artifact-creation step'); + expect(body, label).toContain('suggest implementing the revised plan in a separate implementation step'); + expect(body, label).toContain('suggest archiving the change'); expect(body, label).toContain('the code may no longer match the revised plan'); } }); - it('explains the optional continue workflow before suggesting it', () => { + it('puts CLI recovery at the missing-artifact handoffs without optional workflow references', () => { for (const [label, body] of bodies) { - const availabilityGuidance = body.indexOf( - '`/opsx:continue` is an optional workflow and may not be installed' - ); - const firstSuggestion = body.indexOf( - '`/opsx:continue`', - availabilityGuidance + '`/opsx:continue`'.length - ); - - expect(availabilityGuidance, label).toBeGreaterThanOrEqual(0); - expect(body.indexOf('`/opsx:continue`'), label).toBe(availabilityGuidance); - expect(firstSuggestion, label).toBeGreaterThan(availabilityGuidance); - expect(body, label).toContain( - 'If it is unavailable, `openspec status --change "" --json` shows the next artifact' - ); - expect(body, label).toContain( - '`openspec instructions "" --change "" --json` explains how to create it' - ); + expect(body, label).not.toMatch(/\/opsx:(continue|new|apply|archive)\b/); + const reconcile = body.split('\n').find(line => line.includes('Do NOT create artifacts'))!; + const nextStep = body.split('\n').find(line => line.includes('Artifacts still missing ->'))!; + for (const handoff of [reconcile, nextStep]) { + expect(handoff, label).toContain('separate artifact-creation step'); + expect(handoff, label).toContain('openspec instructions "" --change "" --json'); + } + expect(nextStep, label).toContain('openspec status --change "" --json'); + expect(nextStep, label).toContain('next `ready` artifact (not `skipped` or `blocked`)'); + expect(nextStep, label).toContain('Keep the selected `--store ` on both commands'); } }); - it('confirms every edit and redirects intent changes to /opsx:new', () => { + it('confirms every edit and redirects intent changes to a distinct new change via the CLI', () => { for (const [label, body] of bodies) { expect(body, label).toContain('Write only after the user confirms'); expect(body, label).toContain('If the user rejects a revision, do not write it'); - expect(body, label).toContain('recommend starting fresh with `/opsx:new`'); + expect(body, label).toContain('recommend starting a separate change'); expect(body, label).toContain('Update vs. Start Fresh'); - expect(body, label).toContain('ask for a distinct unused change name'); + expect(body, label).toContain('Ask for a distinct unused change name'); expect(body, label).toContain('openspec new change ""'); expect(body, label).not.toContain('openspec new change ""'); - const newAvailabilityCheck = body.indexOf( - 'first verify whether the optional `/opsx:new` workflow is available' - ); - const newRecommendation = body.indexOf('recommend starting fresh with `/opsx:new`'); - expect(newAvailabilityCheck, label).toBeGreaterThanOrEqual(0); - expect(body.slice(0, newAvailabilityCheck), label).not.toContain('`/opsx:new`'); - expect(newRecommendation, label).toBeGreaterThan(newAvailabilityCheck); + expect(body, label).toContain('keep the current change unchanged'); } }); }); diff --git a/test/utils/command-references.test.ts b/test/utils/command-references.test.ts index d5886f2dfe..383bdb7ff8 100644 --- a/test/utils/command-references.test.ts +++ b/test/utils/command-references.test.ts @@ -334,7 +334,7 @@ describe('getTransformerForTool', () => { }); // Regression for #1153/#1514: the apply skill template must author its -// continue/apply/archive references as canonical /opsx:* tokens so the +// apply/archive references as canonical /opsx:* tokens so the // generator can rewrite them per target. Bare "openspec-continue-change" // prose is invisible to the transformers, which left skills.sh, Codex, and // Kimi with dead text and no archive/input invocation after a naive revert. @@ -343,22 +343,28 @@ describe('apply skill template generates valid per-target invocations', () => { it('authors invocation references as transformable /opsx:* tokens', () => { expect(skill).toContain('/opsx:apply add-auth'); - expect(skill).toContain('suggest using `/opsx:continue`'); + expect(skill).not.toContain('/opsx:continue'); expect(skill).toContain('archive this change with `/opsx:archive`'); // No bare, non-transformable skill-name prose remains. expect(skill).not.toContain('suggest using openspec-continue-change'); }); const cases = [ - { tool: 'default (skills.sh)', transform: transformToSkillReferences, cont: '/openspec-continue-change', arch: '/openspec-archive-change', apply: '/openspec-apply-change' }, - { tool: 'codex', transform: getSkillReferenceTransformer('codex'), cont: '$openspec-continue-change', arch: '$openspec-archive-change', apply: '$openspec-apply-change' }, - { tool: 'kimi', transform: getSkillReferenceTransformer('kimi'), cont: '/skill:openspec-continue-change', arch: '/skill:openspec-archive-change', apply: '/skill:openspec-apply-change' }, + { tool: 'default (skills.sh)', transform: transformToSkillReferences, arch: '/openspec-archive-change', apply: '/openspec-apply-change' }, + { tool: 'codex', transform: getSkillReferenceTransformer('codex'), arch: '$openspec-archive-change', apply: '$openspec-apply-change' }, + { tool: 'kimi', transform: getSkillReferenceTransformer('kimi'), arch: '/skill:openspec-archive-change', apply: '/skill:openspec-apply-change' }, ]; - for (const { tool, transform, cont, arch, apply } of cases) { - it(`emits ${tool} skill invocations for continue, apply, and archive`, () => { + for (const { tool, transform, arch, apply } of cases) { + it(`emits ${tool} invocations for apply/archive and direct CLI recovery when blocked`, () => { const out = transform(skill); - expect(out).toContain(cont); + expect(out).not.toContain('openspec-continue-change'); + const blocked = out.split('\n').find(line => line.includes('If `state: "blocked"`'))!; + expect(blocked).toContain('pause implementation'); + expect(blocked).toContain('openspec status --change "" --json'); + expect(blocked).toContain('next `ready` artifact (not `skipped` or `blocked`)'); + expect(blocked).toContain('openspec instructions "" --change "" --json'); + expect(blocked).toContain('Keep the selected `--store ` on both commands'); expect(out).toContain(arch); expect(out).toContain(`${apply} add-auth`); // No canonical token survives the rewrite. From 03631d8d507c4bc202f6ccfff6908261c9d24806 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 28 Aug 2026 13:36:02 -0500 Subject: [PATCH 2/2] fix(workflows): harden blocked apply recovery guidance --- .changeset/fix-core-workflow-handoffs.md | 2 + skills/openspec-apply-change/SKILL.md | 5 +- src/commands/workflow/instructions.ts | 10 ++- src/core/templates/workflows/apply-change.ts | 5 +- test/commands/profile-handoffs.test.ts | 63 +++++++++++++++++++ test/core/templates/profile-handoffs.test.ts | 19 +++++- .../templates/skill-templates-parity.test.ts | 6 +- test/utils/command-references.test.ts | 5 +- 8 files changed, 101 insertions(+), 14 deletions(-) diff --git a/.changeset/fix-core-workflow-handoffs.md b/.changeset/fix-core-workflow-handoffs.md index d93e46388b..524eaec262 100644 --- a/.changeset/fix-core-workflow-handoffs.md +++ b/.changeset/fix-core-workflow-handoffs.md @@ -3,3 +3,5 @@ --- Remove optional continue/new workflow handoffs from update and apply guidance. Missing artifacts now point directly to the existing status/instructions CLI, and intent changes use new change without requiring an installed skill. Preserve update's planning-only scope, store selection, and the core workflow set. + +Remove the same unavailable workflow recommendations from runtime apply instructions, including text output. Distinguish missing planning artifacts from missing or empty tracking files so recovery does not require a ready artifact when planning is already complete. diff --git a/skills/openspec-apply-change/SKILL.md b/skills/openspec-apply-change/SKILL.md index 2166025f60..331e11c0e3 100644 --- a/skills/openspec-apply-change/SKILL.md +++ b/skills/openspec-apply-change/SKILL.md @@ -48,9 +48,12 @@ Implement tasks from an OpenSpec change. - Dynamic instruction based on current state - Optional `context`: current required project instruction input from the selected root - Optional `operationGuidance`: current advisory guidance for apply + - `missingArtifacts` (when present): required artifact ids with no output **Handle states:** - - If `state: "blocked"` (missing artifacts): show the message and pause implementation. Suggest completing the missing artifacts: run `openspec status --change "" --json`, select the next `ready` artifact (not `skipped` or `blocked`), and use `openspec instructions "" --change "" --json` for its rules and template. Keep the selected `--store ` on both commands. + - If `state: "blocked"`: show the message and pause implementation. + - If `missingArtifacts` is non-empty: suggest completing the missing artifacts. Run `openspec status --change "" --json`, select the next `ready` artifact (not `skipped` or `blocked`), and use `openspec instructions "" --change "" --json` for its rules and template. Keep the selected `--store ` on both commands. + - Otherwise, follow the CLI instruction to create or repair the schema-configured tracking file from existing planning artifacts. Do not assume another artifact is ready or start implementation while blocked. - If `state: "all_done"`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index 1ae6fac7c0..1059aadeda 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -437,18 +437,16 @@ 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.`; + instruction = `Cannot apply this change yet. Missing artifacts: ${missingArtifacts.join(', ')}.\nComplete the missing planning artifacts before starting implementation.`; } 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 ${tracksFile} tracking file is missing and must be created.\nCreate actionable task checkboxes from the existing planning artifacts before starting implementation.`; } 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 ${tracksFile} tracking file exists but contains no tasks to work on.\nAdd actionable task checkboxes from the existing planning artifacts before starting implementation.`; } 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.'; @@ -540,7 +538,7 @@ 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.'); + console.log('Complete the missing planning artifacts before starting implementation.'); console.log(); } diff --git a/src/core/templates/workflows/apply-change.ts b/src/core/templates/workflows/apply-change.ts index 4a18cc29e9..2d66c31b18 100644 --- a/src/core/templates/workflows/apply-change.ts +++ b/src/core/templates/workflows/apply-change.ts @@ -56,9 +56,12 @@ ${STORE_SELECTION_GUIDANCE} - Dynamic instruction based on current state - Optional \`context\`: current required project instruction input from the selected root - Optional \`operationGuidance\`: current advisory guidance for apply + - \`missingArtifacts\` (when present): required artifact ids with no output **Handle states:** - - If \`state: "blocked"\` (missing artifacts): show the message and pause implementation. Suggest completing the missing artifacts: run \`openspec status --change "" --json\`, select the next \`ready\` artifact (not \`skipped\` or \`blocked\`), and use \`openspec instructions "" --change "" --json\` for its rules and template. Keep the selected \`--store \` on both commands. + - If \`state: "blocked"\`: show the message and pause implementation. + - If \`missingArtifacts\` is non-empty: suggest completing the missing artifacts. Run \`openspec status --change "" --json\`, select the next \`ready\` artifact (not \`skipped\` or \`blocked\`), and use \`openspec instructions "" --change "" --json\` for its rules and template. Keep the selected \`--store \` on both commands. + - Otherwise, follow the CLI instruction to create or repair the schema-configured tracking file from existing planning artifacts. Do not assume another artifact is ready or start implementation while blocked. - If \`state: "all_done"\`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/test/commands/profile-handoffs.test.ts b/test/commands/profile-handoffs.test.ts index 3df0f7b054..94aab3b138 100644 --- a/test/commands/profile-handoffs.test.ts +++ b/test/commands/profile-handoffs.test.ts @@ -113,6 +113,10 @@ apply: const before = await fs.readdir(changeRoot); const blocked = JSON.parse(await cli(['instructions', 'apply', '--change', 'original', '--json', ...flags])); expect(blocked.state).toBe('blocked'); + expect(blocked.instruction).not.toContain('openspec-continue-change'); + expect(blocked.instruction).toContain('Complete the missing planning artifacts'); + const blockedText = await cli(['instructions', 'apply', '--change', 'original', ...flags]); + expect(blockedText).not.toContain('openspec-continue-change'); const status = JSON.parse(await cli(['status', '--change', 'original', '--json', ...flags])); const next = status.artifacts.find((artifact: { status: string }) => artifact.status === 'ready'); expect(next.id).toBe('brief'); @@ -134,4 +138,63 @@ apply: expect(await fs.readdir(path.join(project, 'openspec', 'changes'))).toEqual(['archive']); } }, 30_000); + + it.each([ + { name: 'missing', content: undefined, total: 0 }, + { name: 'empty', content: '# Work\n', total: 0 }, + { name: 'text-less checkboxes', content: '- [ ]\n', total: 1 }, + ])('repairs $name tracking without an optional workflow or a ready artifact', async ({ content, total }) => { + createOpenSpecRoot(project); + const schemaDir = path.join(project, 'openspec', 'schemas', 'tracking-test'); + await fs.mkdir(path.join(schemaDir, 'templates'), { recursive: true }); + await fs.writeFile(path.join(schemaDir, 'schema.yaml'), `name: tracking-test +version: 1 +artifacts: + - id: brief + generates: brief.md + description: Planning brief + template: brief.md + requires: [] +apply: + requires: [brief] + tracks: implementation/checklist.md +`); + await fs.writeFile(path.join(schemaDir, 'templates', 'brief.md'), '# Brief\n'); + await fs.writeFile(path.join(project, 'openspec', 'config.yaml'), 'schema: tracking-test\n'); + await cli(['new', 'change', 'tracking']); + const changeRoot = path.join(project, 'openspec', 'changes', 'tracking'); + const briefPath = path.join(changeRoot, 'brief.md'); + await fs.writeFile(briefPath, '# Brief\n\nKeep this plan unchanged.\n'); + const trackingPath = path.join(changeRoot, 'implementation', 'checklist.md'); + await fs.mkdir(path.dirname(trackingPath)); + if (content !== undefined) await fs.writeFile(trackingPath, content); + + const status = JSON.parse(await cli(['status', '--change', 'tracking', '--json'])); + expect(status.artifacts.map((artifact: { status: string }) => artifact.status)).toEqual(['done']); + const blocked = JSON.parse(await cli(['instructions', 'apply', '--change', 'tracking', '--json'])); + expect(blocked.state).toBe('blocked'); + expect(blocked.missingArtifacts).toBeUndefined(); + expect(blocked.progress).toEqual({ total, complete: 0, remaining: total }); + expect(blocked.tasks).toEqual([]); + expect(blocked.instruction).not.toContain('openspec-continue-change'); + expect(blocked.instruction).toContain('implementation/checklist.md'); + expect(blocked.instruction).toContain('existing planning artifacts'); + expect(await cli(['instructions', 'apply', '--change', 'tracking'])).not.toContain('openspec-continue-change'); + if (content === undefined) { + await expect(fs.stat(trackingPath)).rejects.toMatchObject({ code: 'ENOENT' }); + } else { + expect(await fs.readFile(trackingPath, 'utf-8')).toBe(content); + } + + // The fixture repairs tracking separately; CLI status/progress semantics stay unchanged. + await fs.writeFile(trackingPath, '- [ ] Implement the brief\n'); + const ready = JSON.parse(await cli(['instructions', 'apply', '--change', 'tracking', '--json'])); + expect(ready.state).toBe('ready'); + expect(ready.progress).toEqual({ total: 1, complete: 0, remaining: 1 }); + await fs.writeFile(trackingPath, '- [x] Implement the brief\n'); + const done = JSON.parse(await cli(['instructions', 'apply', '--change', 'tracking', '--json'])); + expect(done.state).toBe('all_done'); + expect(done.progress).toEqual({ total: 1, complete: 1, remaining: 0 }); + expect(await fs.readFile(briefPath, 'utf-8')).toBe('# Brief\n\nKeep this plan unchanged.\n'); + }, 30_000); }); diff --git a/test/core/templates/profile-handoffs.test.ts b/test/core/templates/profile-handoffs.test.ts index 1e20573841..72aedf3190 100644 --- a/test/core/templates/profile-handoffs.test.ts +++ b/test/core/templates/profile-handoffs.test.ts @@ -13,6 +13,7 @@ import { getCommandContents, getSkillTemplates, } from '../../../src/core/shared/skill-generation.js'; +import { toolSupportsSkills } from '../../../src/core/shared/skill-paths.js'; import { getTransformerForTool } from '../../../src/utils/command-references.js'; const profiles = [ @@ -26,15 +27,29 @@ const skillWorkflows = new Map(getSkillTemplates().map(({ dirName, workflowId }) // Check both invocation spellings and bare skill names (archive's sync handoff). // Unknown references also fail: a typo must not make the guard silently pass. function expectInstalledReferences(content: string, workflows: readonly string[], label: string): void { - for (const match of content.matchAll(/[/@]opsx[:-]([a-z-]+)|\b(openspec-[a-z-]+)\b/g)) { + for (const match of content.matchAll(/[/@]opsx[:-]([\w-]+)|\b(openspec-[\w-]+)/g)) { const workflow = match[1] ?? skillWorkflows.get(match[2]); expect(workflows, `${label}: unavailable workflow reference ${match[0]}`).toContain(workflow); } } +describe('workflow reference guard', () => { + it.each(['/opsx:continue', '@opsx-continue', '$openspec-continue-change', 'the openspec-sync-specs skill'])( + 'rejects an unavailable workflow in %s', (reference) => { + expect(() => expectInstalledReferences(reference, ['apply'], 'guard')).toThrow('unavailable workflow reference'); + }, + ); + + it.each(['/opsx:aply', '/opsx:apply2', '/opsx:apply_new', '/skill:openspec-aply-change'])( + 'does not accept a misspelled workflow in %s', (reference) => { + expect(() => expectInstalledReferences(reference, ['apply'], 'guard')).toThrow('unavailable workflow reference'); + }, + ); +}); + describe.each(profiles)('$name workflow handoffs', ({ workflows }) => { for (const delivery of ['skills', 'commands', 'both'] as const) { - const tools = AI_TOOLS.filter(tool => tool.skillsDir && ( + const tools = AI_TOOLS.filter(tool => toolSupportsSkills(tool) && ( shouldGenerateSkillsForTool(tool.value, delivery) || shouldGenerateCommandsForTool(tool.value, delivery) )); it.each(tools)(`only references installed workflows for $value (${delivery})`, (tool) => { diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 6b67a297f3..c8dd86793a 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -41,14 +41,14 @@ const EXPECTED_FUNCTION_HASHES: Record = { getExploreSkillTemplate: 'ecaa0bea4c1cd14eee9dbfcfe4b5808fff4ff808cba0a46789b37c1df3048d9a', getNewChangeSkillTemplate: 'eabd1e895c5881dcb17dcbaa3fb26098dd59e8eacb318e400820b4dc811ef781', getContinueChangeSkillTemplate: '012136f6411a99c8fa228e2f9444cb64b0a89e0f56fdeac2fe03b2f5bee0c5d7', - getApplyChangeSkillTemplate: '4e35287f57312bf9f9ec30a5da7abc3e4ade89ef9cf1398d5f72668ae4e327f9', + getApplyChangeSkillTemplate: 'a64d5b717fdb3986d588750e386dff0506c878ea3e1a3b30c48ddf03b683f283', getFfChangeSkillTemplate: '5501740e7ec36ab23ab8c3a0d6dd0655a5e2f35433c7b90e82904fef5e7a326a', getSyncSpecsSkillTemplate: 'b099e2ff31859c9b10d928066e662524f9aad9ecf2be12fceacb732d718c4146', getOnboardSkillTemplate: '3a836faae463d88c289a1c129cb7ee556a563b7e53e1a52a4711ff152a3b51f7', getOpsxExploreCommandTemplate: '1460fcb4fbdf22244e9e76608102e611db598cd4cca8c5dbd001292854bcba6e', getOpsxNewCommandTemplate: 'f2d30e569798a4c92ba932859d6ba4e0ad10e18feccbade1cfee0957597b3463', getOpsxContinueCommandTemplate: 'e50e50266efa1b8e64ff9b6274ee8254f0a240d6adc1b862d126e2f1c9d3a559', - getOpsxApplyCommandTemplate: 'f3940c5f1798886a34d4d51f89f5a9a6f474b17f9029ffa239b3a18d5bb1cdef', + getOpsxApplyCommandTemplate: '8dd857d972630df5529d386e5cc416f5d8e00822039278609328e836f155f334', getOpsxFfCommandTemplate: 'e603bc0996604e6c17a3140943ea642a32d0fc65565e25424bf956e124c55772', getArchiveChangeSkillTemplate: '56bfada1a5f35a127791b70de9d428a75b5aedd1584d6c9803a1ecb1fd1b4a23', getBulkArchiveChangeSkillTemplate: '93875998cade5322d95b43299fba794bc1da754e917dd63a770406386a6d295d', @@ -69,7 +69,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-explore': '886680e71f2900378bd12bb9ff25c888a41a8f851e0bb3ec056affcc18d07ca8', 'openspec-new-change': 'ec4529beef978e34634a6f7286fab55d68fad8fb374dceb45691d52caab33fbb', 'openspec-continue-change': 'bb6194a16c54891cdb253678e8f70ce53b2af86735243980f366ce551d37e42e', - 'openspec-apply-change': 'bf68c07d1f60ac379c11246bc815ff6c513c3a20c219a4fa8ea9a9b972392fc5', + 'openspec-apply-change': '19b8e5c2693103e5014e7083a8dea13ace88e7b8e566cd3a0485e0af28d2d039', 'openspec-ff-change': '217c78da2b6e8358f609ac57dcd02266aaec3354ce26dc6ec2fc9c2174673ab4', 'openspec-sync-specs': 'd933d8856584d6c1253de91e652e7aee9e85c77ad4d3531f6476f79d84e6e5e8', 'openspec-archive-change': '7c65053d674ba4e1e20e2bf73ba7e5a7f94baef2eaa9b33cee48d4cadea51b7a', diff --git a/test/utils/command-references.test.ts b/test/utils/command-references.test.ts index 383bdb7ff8..8538fe62bf 100644 --- a/test/utils/command-references.test.ts +++ b/test/utils/command-references.test.ts @@ -359,12 +359,15 @@ describe('apply skill template generates valid per-target invocations', () => { it(`emits ${tool} invocations for apply/archive and direct CLI recovery when blocked`, () => { const out = transform(skill); expect(out).not.toContain('openspec-continue-change'); - const blocked = out.split('\n').find(line => line.includes('If `state: "blocked"`'))!; + const blocked = out.slice(out.indexOf('If `state: "blocked"`'), out.indexOf('If `state: "all_done"`')); expect(blocked).toContain('pause implementation'); + expect(blocked).toContain('If `missingArtifacts` is non-empty'); expect(blocked).toContain('openspec status --change "" --json'); expect(blocked).toContain('next `ready` artifact (not `skipped` or `blocked`)'); expect(blocked).toContain('openspec instructions "" --change "" --json'); expect(blocked).toContain('Keep the selected `--store ` on both commands'); + expect(blocked).toContain('Otherwise, follow the CLI instruction to create or repair the schema-configured tracking file'); + expect(blocked).toContain('Do not assume another artifact is ready or start implementation while blocked'); expect(out).toContain(arch); expect(out).toContain(`${apply} add-auth`); // No canonical token survives the rewrite.