From 08f613e88108e8fbb0ad08d0a4eb53ac61f20ad2 Mon Sep 17 00:00:00 2001 From: Alex <45095641+runsonmypc@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:06:10 -0400 Subject: [PATCH 1/9] fix(update): close the dead end for partially populated glob artifacts `artifactOutputExists` returns true as soon as a single file matches a glob `generates`, so a `specs/**/*.md` artifact is `done` after the first delta spec. `/opsx:continue` selects only `ready` artifacts and never revisits it. The update templates forbid creating new files under a glob artifact and point the user to `/opsx:continue` instead, which cannot act on it. A capability spec the coherence review finds missing therefore has no supported way to be created. Allow update to write that file: concrete path only, rules fetched from `openspec instructions`, and the same confirm-before-write rule as every other revision. Creating an artifact that has no files at all stays out of scope - that one is genuinely `/opsx:continue`'s job. --- .changeset/update-glob-artifact-gap.md | 5 +++++ src/core/templates/workflows/update-change.ts | 16 ++++++++++------ test/core/templates/update-change.test.ts | 13 ++++++++++++- 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 .changeset/update-glob-artifact-gap.md diff --git a/.changeset/update-glob-artifact-gap.md b/.changeset/update-glob-artifact-gap.md new file mode 100644 index 0000000000..a775ca6028 --- /dev/null +++ b/.changeset/update-glob-artifact-gap.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Let `/opsx:update` fill a missing file under an already-satisfied glob artifact. A glob artifact is complete once one file matches it, and `/opsx:continue` only picks up `ready` artifacts, so the previous "point the user to `/opsx:continue`" handoff was unreachable and the missing file could never be created through the documented flow. diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 6735d1dbe8..71a44b7f99 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -128,7 +128,8 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions only to 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 ${CONTINUE_CREATE_THEM}. + - Propose revisions to the files that already exist (\`existingOutputPaths\`). Do NOT create an artifact that has no files at all - note it and ${CONTINUE_CREATE_THEM}. + - A glob artifact (e.g. \`specs/**/*.md\`) counts as done as soon as ONE file matches it, so the continue workflow will never come back to it. If reconciliation finds a file missing under a glob artifact that already has at least one file, propose creating it here: choose a concrete path (never \`resolvedOutputPath\`), fetch that artifact's rules and template with \`openspec instructions "" --change "" --json\`, and write it only under step 5's confirmation rule. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -149,14 +150,15 @@ ${CONTINUE_SCOPE_NOTE} After each invocation, show: - Which artifacts were revised (and which proposed revisions were rejected) +- Any file created under a glob artifact that was already partially populated - ${CONTINUE_DEFERRED} - Where the change stands and the recommended next command **Guardrails** - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - 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 - ${CONTINUE_FRONTIER}. +- Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. +- Do not advance the build frontier: never create an artifact that has no files yet - ${CONTINUE_FRONTIER}. Filling a gap under a glob artifact that is already satisfied is in scope because the continue workflow cannot reach it. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.`, license: 'MIT', @@ -222,7 +224,8 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions only to 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 ${CONTINUE_CREATE_THEM}. + - Propose revisions to the files that already exist (\`existingOutputPaths\`). Do NOT create an artifact that has no files at all - note it and ${CONTINUE_CREATE_THEM}. + - A glob artifact (e.g. \`specs/**/*.md\`) counts as done as soon as ONE file matches it, so the continue workflow will never come back to it. If reconciliation finds a file missing under a glob artifact that already has at least one file, propose creating it here: choose a concrete path (never \`resolvedOutputPath\`), fetch that artifact's rules and template with \`openspec instructions "" --change "" --json\`, and write it only under step 5's confirmation rule. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -243,14 +246,15 @@ ${CONTINUE_SCOPE_NOTE} After each invocation, show: - Which artifacts were revised (and which proposed revisions were rejected) +- Any file created under a glob artifact that was already partially populated - ${CONTINUE_DEFERRED} - Where the change stands and the recommended next command **Guardrails** - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - 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 - ${CONTINUE_FRONTIER}. +- Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. +- Do not advance the build frontier: never create an artifact that has no files yet - ${CONTINUE_FRONTIER}. Filling a gap under a glob artifact that is already satisfied is in scope because the continue workflow cannot reach it. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.` }; diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index 1801f46c0c..2af2e5e4b3 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -241,7 +241,18 @@ describe('update-change templates', () => { expect(body, label).toContain('NEVER edit implementation code'); expect(body, label).toContain('stop and point to `/opsx:apply`'); expect(body, label).toContain('Do not advance the build frontier'); - expect(body, label).toContain('Do NOT create artifacts that don\'t exist yet'); + expect(body, label).toContain('never create an artifact that has no files yet'); + expect(body, label).toContain('Do NOT create an artifact that has no files at all'); + } + }); + + it('fills a gap under an already-satisfied glob artifact instead of deferring it (3.3a)', () => { + for (const [label, body] of bodies) { + // A glob artifact is complete once one file matches, and /opsx:continue only + // picks up `ready` artifacts, so deferring the missing file strands it. + expect(body, label).toContain('counts as done as soon as ONE file matches'); + expect(body, label).toContain('create it here rather than deferring'); + expect(body, label).toContain('cannot reach it'); } }); From 2361885852375690a5916e204674f0637fc0f963 Mon Sep 17 00:00:00 2001 From: Alex <45095641+runsonmypc@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:02:03 -0400 Subject: [PATCH 2/9] fix(update): tighten glob gap write guardrails --- src/core/templates/workflows/update-change.ts | 18 ++++++++---- test/core/templates/update-change.test.ts | 28 +++++++++++++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 71a44b7f99..41bd8961ff 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -128,8 +128,11 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions to the files that already exist (\`existingOutputPaths\`). Do NOT create an artifact that has no files at all - note it and ${CONTINUE_CREATE_THEM}. - - A glob artifact (e.g. \`specs/**/*.md\`) counts as done as soon as ONE file matches it, so the continue workflow will never come back to it. If reconciliation finds a file missing under a glob artifact that already has at least one file, propose creating it here: choose a concrete path (never \`resolvedOutputPath\`), fetch that artifact's rules and template with \`openspec instructions "" --change "" --json\`, and write it only under step 5's confirmation rule. + - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files, note it and ${CONTINUE_CREATE_THEM}; it is still \`ready\` or \`blocked\`. + - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\`, \`rules\`, and \`template\`. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\`. Never write to the glob \`resolvedOutputPath\`. + 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -158,7 +161,7 @@ After each invocation, show: - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: never create an artifact that has no files yet - ${CONTINUE_FRONTIER}. Filling a gap under a glob artifact that is already satisfied is in scope because the continue workflow cannot reach it. +- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\`, ${CONTINUE_FRONTIER}. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.`, license: 'MIT', @@ -224,8 +227,11 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions to the files that already exist (\`existingOutputPaths\`). Do NOT create an artifact that has no files at all - note it and ${CONTINUE_CREATE_THEM}. - - A glob artifact (e.g. \`specs/**/*.md\`) counts as done as soon as ONE file matches it, so the continue workflow will never come back to it. If reconciliation finds a file missing under a glob artifact that already has at least one file, propose creating it here: choose a concrete path (never \`resolvedOutputPath\`), fetch that artifact's rules and template with \`openspec instructions "" --change "" --json\`, and write it only under step 5's confirmation rule. + - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files, note it and ${CONTINUE_CREATE_THEM}; it is still \`ready\` or \`blocked\`. + - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\`, \`rules\`, and \`template\`. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\`. Never write to the glob \`resolvedOutputPath\`. + 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -254,7 +260,7 @@ After each invocation, show: - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: never create an artifact that has no files yet - ${CONTINUE_FRONTIER}. Filling a gap under a glob artifact that is already satisfied is in scope because the continue workflow cannot reach it. +- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\`, ${CONTINUE_FRONTIER}. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.` }; diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index 2af2e5e4b3..74b7ef2b43 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -241,26 +241,36 @@ describe('update-change templates', () => { expect(body, label).toContain('NEVER edit implementation code'); expect(body, label).toContain('stop and point to `/opsx:apply`'); expect(body, label).toContain('Do not advance the build frontier'); - expect(body, label).toContain('never create an artifact that has no files yet'); - expect(body, label).toContain('Do NOT create an artifact that has no files at all'); + expect(body, label).toContain( + 'Leave an artifact with no existing output files for `/opsx:continue`' + ); + expect(body, label).toContain( + 'leave artifacts with empty `existingOutputPaths` for `/opsx:continue`' + ); } }); it('fills a gap under an already-satisfied glob artifact instead of deferring it (3.3a)', () => { for (const [label, body] of bodies) { - // A glob artifact is complete once one file matches, and /opsx:continue only - // picks up `ready` artifacts, so deferring the missing file strands it. - expect(body, label).toContain('counts as done as soon as ONE file matches'); - expect(body, label).toContain('create it here rather than deferring'); - expect(body, label).toContain('cannot reach it'); + expect(body, label).toContain('is marked `done` after at least one file matches'); + expect(body, label).toContain('`/opsx:continue` only handles `ready` artifacts'); + expect(body, label).toContain('whose `existingOutputPaths` is non-empty'); + expect(body, label).toContain( + 'use its `instruction`, `rules`, and `template`' + ); + expect(body, label).toContain( + 'inside `changeRoot` that matches `artifactPaths..outputPath`' + ); + expect(body, label).toContain('create it only after the user confirms'); } }); it('writes to existingOutputPaths, never to a glob resolvedOutputPath (3.4)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('artifactPaths..existingOutputPaths'); - expect(body, label).toContain('Do NOT write to `resolvedOutputPath`'); - expect(body, label).toContain('still the glob pattern, not a real file'); + expect(body, label).toContain('`resolvedOutputPath` is still a pattern'); + expect(body, label).toContain('Never write to the glob `resolvedOutputPath`'); + expect(body, label).toContain('The only new-file exception'); } }); From 25c4e3321716aef02dc9563cb926d054561e6991 Mon Sep 17 00:00:00 2001 From: Alex <45095641+runsonmypc@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:26:50 -0400 Subject: [PATCH 3/9] fix(update): narrow continue handoff to empty artifacts --- skills/openspec-update-change/SKILL.md | 2 +- src/core/templates/workflows/update-change.ts | 4 ++-- test/core/templates/update-change.test.ts | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/skills/openspec-update-change/SKILL.md b/skills/openspec-update-change/SKILL.md index 9aae524ba0..950bab665c 100644 --- a/skills/openspec-update-change/SKILL.md +++ b/skills/openspec-update-change/SKILL.md @@ -82,7 +82,7 @@ This workflow revises artifacts that already exist; `/openspec-continue-change` ``` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> suggest `/openspec-continue-change` to create them. + - Artifacts with empty `existingOutputPaths` 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`. diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 41bd8961ff..188efb70c7 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -145,7 +145,7 @@ ${CONTINUE_SCOPE_NOTE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> ${CONTINUE_NEXT_STEP}. + - Artifacts with empty \`existingOutputPaths\` still missing -> ${CONTINUE_NEXT_STEP}. - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; ${APPLY_DELTA_HANDOFF}. - Everything done and implemented -> ${ARCHIVE_HANDOFF}. @@ -244,7 +244,7 @@ ${CONTINUE_SCOPE_NOTE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts still missing -> ${CONTINUE_NEXT_STEP}. + - Artifacts with empty \`existingOutputPaths\` still missing -> ${CONTINUE_NEXT_STEP}. - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; ${APPLY_DELTA_HANDOFF}. - Everything done and implemented -> ${ARCHIVE_HANDOFF}. diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index 74b7ef2b43..b36a486a1d 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -277,6 +277,9 @@ 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( + 'Artifacts with empty `existingOutputPaths` still missing -> suggest `/opsx:continue`' + ); expect(body, label).toContain('suggest `/opsx:continue`'); expect(body, label).toContain('suggest `/opsx:apply`'); expect(body, label).toContain('suggest `/opsx:archive`'); From f80323498954d3ecbe7714b6d43606310a9cc9e9 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 28 Aug 2026 07:45:05 -0500 Subject: [PATCH 4/9] fix(update): harden glob gap creation guidance --- src/core/templates/workflows/update-change.ts | 24 ++-- test/commands/artifact-workflow.test.ts | 105 ++++++++++++++++++ test/core/templates/update-change.test.ts | 17 ++- 3 files changed, 130 insertions(+), 16 deletions(-) diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 188efb70c7..c901952b32 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -35,8 +35,8 @@ const CONTINUE_NEXT_STEP = optionalWorkflow( const CONTINUE_DEFERRED = optionalWorkflow( 'continue', - 'Anything deferred to `/opsx:continue` (not-yet-created artifacts or files)', - 'Anything deferred because it does not exist yet (not-yet-created artifacts or files)' + 'Anything deferred to `/opsx:continue` (artifacts with no files yet and status `ready` or `blocked`, never `skipped` artifacts)', + 'Anything deferred because it does not exist yet (artifacts with no files and status `ready` or `blocked`, never `skipped` artifacts)' ); const CONTINUE_FRONTIER = optionalWorkflow( @@ -128,10 +128,10 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files, note it and ${CONTINUE_CREATE_THEM}; it is still \`ready\` or \`blocked\`. + - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\`, \`rules\`, and \`template\`. - 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\`. Never write to the glob \`resolvedOutputPath\`. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. - If the change is already coherent, say so and propose no revisions. @@ -145,7 +145,7 @@ ${CONTINUE_SCOPE_NOTE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts with empty \`existingOutputPaths\` still missing -> ${CONTINUE_NEXT_STEP}. + - Artifacts with empty \`existingOutputPaths\` and status \`ready\` or \`blocked\` -> ${CONTINUE_NEXT_STEP}. - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; ${APPLY_DELTA_HANDOFF}. - Everything done and implemented -> ${ARCHIVE_HANDOFF}. @@ -161,7 +161,7 @@ After each invocation, show: - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\`, ${CONTINUE_FRONTIER}. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. +- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\` and status \`ready\` or \`blocked\`, ${CONTINUE_FRONTIER}. Leave \`skipped\` artifacts untouched. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.`, license: 'MIT', @@ -227,10 +227,10 @@ ${CONTINUE_SCOPE_NOTE} - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files, note it and ${CONTINUE_CREATE_THEM}; it is still \`ready\` or \`blocked\`. + - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\`, \`rules\`, and \`template\`. - 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\`. Never write to the glob \`resolvedOutputPath\`. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. - If the change is already coherent, say so and propose no revisions. @@ -244,7 +244,7 @@ ${CONTINUE_SCOPE_NOTE} \`\`\` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts with empty \`existingOutputPaths\` still missing -> ${CONTINUE_NEXT_STEP}. + - Artifacts with empty \`existingOutputPaths\` and status \`ready\` or \`blocked\` -> ${CONTINUE_NEXT_STEP}. - Change already implemented (tasks checked off / already applied) -> the code may no longer match the revised plan; ${APPLY_DELTA_HANDOFF}. - Everything done and implemented -> ${ARCHIVE_HANDOFF}. @@ -260,7 +260,7 @@ After each invocation, show: - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. - Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. -- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\`, ${CONTINUE_FRONTIER}. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. +- Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\` and status \`ready\` or \`blocked\`, ${CONTINUE_FRONTIER}. Leave \`skipped\` artifacts untouched. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.` }; diff --git a/test/commands/artifact-workflow.test.ts b/test/commands/artifact-workflow.test.ts index 0658d2beb0..8b2732b30c 100644 --- a/test/commands/artifact-workflow.test.ts +++ b/test/commands/artifact-workflow.test.ts @@ -340,6 +340,17 @@ describe('artifact-workflow CLI commands', () => { expect(status.artifacts.find((artifact: any) => artifact.id === 'specs')?.status).toBe( 'skipped' ); + expect(status.artifactPaths.specs.existingOutputPaths).toEqual([]); + const instructionsResult = await runCLI( + ['instructions', 'specs', '--change', 'skip-specs-change', '--json'], + { cwd: tempDir } + ); + expect(instructionsResult.exitCode).toBe(0); + expect(JSON.parse(instructionsResult.stdout)).toMatchObject({ + skipped: true, + existingOutputPaths: [], + warning: expect.stringContaining('Do not create spec files'), + }); await expect(fs.stat(path.join(changeDir, 'specs'))).rejects.toMatchObject({ code: 'ENOENT' }); }); @@ -450,6 +461,100 @@ describe('artifact-workflow CLI commands', () => { }); describe('instructions command', () => { + it('keeps instructions available for missing companion outputs after a glob artifact is done', async () => { + const schemaName = 'companion-outputs'; + const schemaDir = path.join(tempDir, 'openspec', 'schemas', schemaName); + const outputPath = 'reviews/*/notes.md'; + const template = '# Review\n\n## Findings\n'; + await fs.mkdir(path.join(schemaDir, 'templates'), { recursive: true }); + await fs.writeFile( + path.join(schemaDir, 'schema.yaml'), + `name: ${schemaName} +version: 1 +artifacts: + - id: assessments + generates: ${outputPath} + description: Component assessments + template: review.md + instruction: Write an assessment for each affected component. + requires: [] + - id: signoff + generates: signoff.md + description: Review signoff + template: signoff.md + requires: [assessments] +` + ); + await fs.writeFile(path.join(schemaDir, 'templates', 'review.md'), template); + await fs.writeFile(path.join(schemaDir, 'templates', 'signoff.md'), '# Signoff\n'); + await fs.writeFile( + path.join(tempDir, 'openspec', 'config.yaml'), + `schema: ${schemaName} +context: Review both the API and UI components. +rules: + assessments: + - Preserve existing findings when adding a companion assessment. +` + ); + const changeName = 'companion-review'; + const changeDir = path.join(changesDir, changeName); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, '.openspec.yaml'), `schema: ${schemaName}\n`); + const apiPath = path.join(changeDir, 'reviews', 'api', 'notes.md'); + const uiPath = path.join(changeDir, 'reviews', 'ui', 'notes.md'); + + async function readJson(args: string[]) { + const result = await runCLI([...args, '--change', changeName, '--json'], { cwd: tempDir }); + expect(result.exitCode).toBe(0); + return JSON.parse(result.stdout); + } + + const empty = await readJson(['status']); + expect(empty.artifacts).toMatchObject([ + { id: 'assessments', status: 'ready' }, + { id: 'signoff', status: 'blocked', missingDeps: ['assessments'] }, + ]); + expect(empty.artifactPaths.assessments.existingOutputPaths).toEqual([]); + + // Fixture writes simulate authored outputs; the CLI only reports their state. + const existingContent = '# Review\n\n## Findings\nKeep this API finding.\n'; + await fs.mkdir(path.dirname(apiPath), { recursive: true }); + await fs.writeFile(apiPath, existingContent); + const partial = await readJson(['status']); + expect(partial.artifacts).toMatchObject([ + { id: 'assessments', status: 'done' }, + { id: 'signoff', status: 'ready' }, + ]); + expect(partial.artifactPaths.assessments.existingOutputPaths.map(canonical)).toEqual([ + canonical(apiPath), + ]); + const instructions = await readJson(['instructions', 'assessments']); + expect(instructions).toMatchObject({ + artifactId: 'assessments', + outputPath, + instruction: 'Write an assessment for each affected component.', + context: 'Review both the API and UI components.', + rules: ['Preserve existing findings when adding a companion assessment.'], + template, + }); + expect(canonical(instructions.changeDir)).toBe(canonical(changeDir)); + expect(instructions.resolvedOutputPath).toBe(path.join(instructions.changeDir, outputPath)); + expect(instructions.existingOutputPaths.map(canonical)).toEqual([canonical(apiPath)]); + expect(instructions.skipped).toBeUndefined(); + await expect(fs.stat(uiPath)).rejects.toMatchObject({ code: 'ENOENT' }); + + await fs.mkdir(path.dirname(uiPath), { recursive: true }); + await fs.writeFile(uiPath, '# Review\n\n## Findings\nNew UI finding.\n'); + const expanded = await readJson(['status']); + expect(expanded.artifacts).toEqual(partial.artifacts); + expect(expanded.nextSteps).toEqual(partial.nextSteps); + expect(expanded.artifactPaths.assessments.existingOutputPaths.map(canonical)).toEqual( + [apiPath, uiPath].map(canonical).sort() + ); + expect(await fs.readFile(apiPath, 'utf-8')).toBe(existingContent); + await expect(fs.stat(path.join(changeDir, 'signoff.md'))).rejects.toMatchObject({ code: 'ENOENT' }); + }); + it('shows instructions for proposal on scaffolded change', async () => { // Create empty change directory (no proposal.md) const changeDir = path.join(changesDir, 'scaffolded-change'); diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index b36a486a1d..e3a68befb0 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -242,11 +242,13 @@ describe('update-change templates', () => { expect(body, label).toContain('stop and point to `/opsx:apply`'); expect(body, label).toContain('Do not advance the build frontier'); expect(body, label).toContain( - 'Leave an artifact with no existing output files for `/opsx:continue`' + 'Leave an artifact with no existing output files and status `ready` or `blocked` for `/opsx:continue`' ); expect(body, label).toContain( - 'leave artifacts with empty `existingOutputPaths` for `/opsx:continue`' + 'leave artifacts with empty `existingOutputPaths` and status `ready` or `blocked` for `/opsx:continue`' ); + expect(body, label).toContain('Leave `skipped` artifacts untouched'); + expect(body, label).toContain('do not treat them as missing or send them to `/opsx:continue`'); } }); @@ -256,12 +258,16 @@ describe('update-change templates', () => { expect(body, label).toContain('`/opsx:continue` only handles `ready` artifacts'); expect(body, label).toContain('whose `existingOutputPaths` is non-empty'); expect(body, label).toContain( - 'use its `instruction`, `rules`, and `template`' + 'use its `instruction` and `template`' ); + expect(body, label).toContain('Apply `context` and `rules` as constraints; do not copy them into the file'); + expect(body, label).toContain('If instructions report `skipped: true`, do not create the file'); expect(body, label).toContain( 'inside `changeRoot` that matches `artifactPaths..outputPath`' ); expect(body, label).toContain('create it only after the user confirms'); + expect(body, label).toContain('does not already exist'); + expect(body, label).toContain('after resolving any symlinked parent directories'); } }); @@ -278,7 +284,7 @@ describe('update-change templates', () => { for (const [label, body] of bodies) { expect(body, label).toContain('guidance only - NEVER act on it'); expect(body, label).toContain( - 'Artifacts with empty `existingOutputPaths` still missing -> suggest `/opsx:continue`' + 'Artifacts with empty `existingOutputPaths` and status `ready` or `blocked` -> suggest `/opsx:continue`' ); expect(body, label).toContain('suggest `/opsx:continue`'); expect(body, label).toContain('suggest `/opsx:apply`'); @@ -320,6 +326,9 @@ describe('update-change templates', () => { it('confirms every edit and redirects intent changes to /opsx:new when installed', () => { for (const [label, body] of bodies) { + const reconciliation = body.slice(body.indexOf('4. **Read and reconcile**'), body.indexOf('5. **Confirm and apply')); + expect(reconciliation, label).toContain('Draft the requested edit without writing'); + expect(reconciliation, label).not.toContain('Apply the requested edit'); 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`'); From 462f3b395f32f682bd42252f6a4459296e61ea3a Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 28 Aug 2026 13:56:02 -0500 Subject: [PATCH 5/9] fix(update): preserve creation safeguards for glob companions --- docs/commands.md | 10 ++++-- docs/opsx.md | 4 ++- .../changes/add-update-workflow/design.md | 6 ++++ .../specs/opsx-update-skill/spec.md | 33 +++++++++++++++---- src/core/templates/workflows/update-change.ts | 8 +++-- test/commands/artifact-workflow.test.ts | 24 +++++++++++++- test/core/templates/update-change.test.ts | 21 ++++++++++++ 7 files changed, 93 insertions(+), 13 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index c7611faf8e..ac64cb20e3 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -352,7 +352,13 @@ Revise a change's existing planning artifacts and keep them coherent with one an - Applies your requested revision, or reviews the artifacts for contradictions if you didn't name one - Reconciles the other existing artifacts in any direction (a design edit may ripple back to the proposal) - Confirms every edit with you before writing, one artifact at a time -- Ends by recommending the next step: `/opsx:continue` (artifacts missing), `/opsx:apply` (carry a revised plan into code), or `/opsx:archive` (all done) +- Ends by recommending the next step: `/opsx:continue` (unstarted artifacts), `/opsx:apply` (carry a revised plan into code), or `/opsx:archive` (all done) + +**Missing files:** + +- For a glob artifact such as `specs/**/*.md` with at least one existing file, update can propose a missing companion file. It uses the schema's instructions and asks you to confirm the concrete path before creating it. +- Artifacts with no files yet remain with `/opsx:continue`. Intentionally skipped artifacts stay untouched. +- New files must stay inside the change directory. If a file appears at the confirmed path before creation, update stops instead of overwriting it. **Example:** @@ -373,7 +379,7 @@ AI: Reading add-dark-mode artifacts... **Tips:** -- It won't create missing artifacts - that's `/opsx:continue` +- It won't start an artifact with no existing files. Enable `/opsx:continue` for that, or use `openspec status` and `openspec instructions` if that optional workflow isn't installed. - If the change was already implemented, follow up with `/opsx:apply` so the code matches the revised plan - If your revision changes the *intent* of the change, start fresh with a new change instead (see [When to Update vs. Start Fresh](opsx.md#when-to-update-vs-start-fresh)) diff --git a/docs/opsx.md b/docs/opsx.md index c1b2dba37f..a046eb8a21 100644 --- a/docs/opsx.md +++ b/docs/opsx.md @@ -213,7 +213,9 @@ Works through tasks, checking them off as you go. If you're juggling multiple ch ``` /opsx:update add-dark-mode - we're storing the theme in a cookie now ``` -Revises the change's existing planning artifacts and keeps them coherent - in any direction (a design edit may ripple back to the proposal). Planning artifacts only: it never edits code, and it never creates missing artifacts (that's `/opsx:continue`). Every edit is confirmed with you first. If the change was already implemented, it recommends `/opsx:apply` so the code catches up with the revised plan. If your revision changes the change's *intent*, start fresh instead - see [When to Update vs. Start Fresh](#when-to-update-vs-start-fresh). +Revises the change's existing planning artifacts and keeps them coherent in any direction (a design edit may ripple back to the proposal). It never edits code. Every edit is confirmed with you first. See [the update reference](commands.md#opsxupdate) for how it handles missing files without starting a new artifact. + +If the change was already implemented, it recommends `/opsx:apply` so the code catches up with the revised plan. If your revision changes the change's *intent*, start fresh instead. See [When to Update vs. Start Fresh](#when-to-update-vs-start-fresh). ### Sync delta specs ```text diff --git a/openspec/changes/add-update-workflow/design.md b/openspec/changes/add-update-workflow/design.md index 3dd6ca53a3..f49b105551 100644 --- a/openspec/changes/add-update-workflow/design.md +++ b/openspec/changes/add-update-workflow/design.md @@ -105,6 +105,12 @@ Review feedback flagged that "update" alone is generic — could it apply to any ### 6. Next-step guidance, especially for already-implemented changes A change can be revised after it was built — tasks checked off, `/opsx:apply` already run. The update itself behaves identically (planning artifacts only), but stopping silently would strand the user: the code and the revised plan now disagree. So the skill ends by reporting where the change stands (from the status JSON and the tasks checklist) and recommending the next command — `/opsx:continue` if artifacts are missing, `/opsx:apply` to carry a revised plan into code, `/opsx:archive` when everything is done. Guidance only: the skill never implements, mirroring the "All artifacts created! You can now implement this change with `/opsx:apply`" hand-off that `continue-change.ts` already uses. +### 7. Companion-file correction (#1733) + +The original glob-file deferral was unreachable: one matching file marks an artifact `done`, while continue selects only `ready` artifacts. Update can therefore propose a missing companion file within an already populated glob. This corrects the unarchived spec's former blanket deferral without changing the graph's completion rule or starting another artifact. + +The exception uses existing status and instructions output, requires current dependency context and user confirmation, and preserves the change-only planning scope. Immediately before creation, it rechecks scope and the concrete path and uses an operation that refuses an existing target. Delegated creators must obey the same limits. No new CLI command, metadata, graph state, or automatic artifact writer is introduced. + ## Risks / Trade-offs - **No deterministic staleness signal.** With no digest/ledger, the skill relies on the agent reading the artifacts to spot incoherence. Trade-off accepted: an agent that rewrites prose must read it anyway, and a content-blind signal earns its cost only for use cases this change excludes (Decision 3). diff --git a/openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md b/openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md index 6dc4a4b704..212c6932c7 100644 --- a/openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md +++ b/openspec/changes/add-update-workflow/specs/opsx-update-skill/spec.md @@ -19,7 +19,7 @@ The system SHALL provide a `/opsx:update` workflow skill that revises a change's #### Scenario: Missing artifacts are deferred to continue -- **WHEN** keeping the change coherent would require an artifact that has not been created yet +- **WHEN** keeping the change coherent would require an artifact with no existing output files and status `ready` or `blocked` - **THEN** the skill revises only the artifacts that currently exist - **AND** it notes the not-yet-created artifacts and points the user to `/opsx:continue` to create them @@ -53,7 +53,7 @@ The `/opsx:update` skill SHALL learn which artifacts exist and where they live b #### Scenario: Resolve artifact paths cross-platform -- **WHEN** the skill reads or writes an artifact on macOS, Linux, or Windows +- **WHEN** the skill reads or revises an existing artifact file on macOS, Linux, or Windows - **THEN** it uses the `existingOutputPaths` provided by the CLI status output - **AND** it does not assume forward-slash separators @@ -63,11 +63,30 @@ The `/opsx:update` skill SHALL learn which artifacts exist and where they live b - **THEN** the skill edits the concrete files reported in that artifact's `existingOutputPaths` - **AND** it does not write to `resolvedOutputPath`, which for a glob artifact remains the glob pattern rather than a real file -#### Scenario: A new file under a glob artifact is deferred to continue +#### Scenario: A missing companion file under a populated glob artifact -- **WHEN** keeping the change coherent would require a new file under a glob artifact that does not exist yet (for example a spec for a not-yet-captured capability) -- **THEN** the skill revises only the files already present in `existingOutputPaths` -- **AND** it points the user to `/opsx:continue`/`/opsx:propose` to create the new file rather than inventing a path from the glob +- **WHEN** reconciliation identifies a missing companion file for a glob artifact with non-empty `existingOutputPaths` +- **THEN** the skill MAY propose creating that file using the artifact's instructions, template, project context, rules, and current dependency files +- **AND** it selects an unused concrete path matching the artifact's `outputPath` inside `changeRoot`, including after resolving linked parent directories +- **AND** it creates the file only after user confirmation, refreshing status, instructions, and path checks immediately before creation +- **AND** creation SHALL fail rather than overwrite a file that appeared in the meantime +- **AND** it SHALL NOT start another artifact, write main specs, or edit implementation code + +#### Scenario: Required inputs are no longer available + +- **WHEN** a populated glob artifact remains `done` but a required non-skipped dependency is missing +- **THEN** the skill SHALL stop new companion creation and ask the user to restore the dependency first + +#### Scenario: Schema delegates companion creation + +- **WHEN** the artifact instruction delegates creation to another skill or command +- **THEN** the skill SHALL invoke it only if it can honor the confirmed concrete path and the update guardrails +- **AND** otherwise it SHALL stop rather than invoke broader generation + +#### Scenario: Intentionally skipped artifact + +- **WHEN** status or instructions mark an artifact as skipped +- **THEN** the skill SHALL leave it untouched and SHALL NOT treat its empty outputs as missing or send it to continue ### Requirement: Bidirectional Coherence Review @@ -109,7 +128,7 @@ After applying confirmed revisions (or finding none needed), the `/opsx:update` #### Scenario: Next step when artifacts are incomplete -- **WHEN** the update finishes and the change still has not-yet-created artifacts +- **WHEN** the update finishes and the change still has artifacts with no outputs and status `ready` or `blocked` - **THEN** the skill recommends `/opsx:continue` to create them #### Scenario: Next step when the change is fully done diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index c901952b32..10c81ab529 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -130,9 +130,11 @@ ${CONTINUE_SCOPE_NOTE} - Note everything that is now inconsistent, missing, or contradictory. - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. + 4. After confirmation, immediately before creation, refresh status and instructions. Verify the artifact is still in scope, not skipped, and partially populated; repeat the concrete-path checks above. + 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user instead of overwriting or choosing a different path. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -229,9 +231,11 @@ ${CONTINUE_SCOPE_NOTE} - Note everything that is now inconsistent, missing, or contradictory. - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. + 4. After confirmation, immediately before creation, refresh status and instructions. Verify the artifact is still in scope, not skipped, and partially populated; repeat the concrete-path checks above. + 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user instead of overwriting or choosing a different path. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** diff --git a/test/commands/artifact-workflow.test.ts b/test/commands/artifact-workflow.test.ts index 8b2732b30c..94c0d828a9 100644 --- a/test/commands/artifact-workflow.test.ts +++ b/test/commands/artifact-workflow.test.ts @@ -472,12 +472,17 @@ describe('artifact-workflow CLI commands', () => { `name: ${schemaName} version: 1 artifacts: + - id: brief + generates: brief.md + description: Review brief + template: brief.md + requires: [] - id: assessments generates: ${outputPath} description: Component assessments template: review.md instruction: Write an assessment for each affected component. - requires: [] + requires: [brief] - id: signoff generates: signoff.md description: Review signoff @@ -485,6 +490,7 @@ artifacts: requires: [assessments] ` ); + await fs.writeFile(path.join(schemaDir, 'templates', 'brief.md'), '# Brief\n'); await fs.writeFile(path.join(schemaDir, 'templates', 'review.md'), template); await fs.writeFile(path.join(schemaDir, 'templates', 'signoff.md'), '# Signoff\n'); await fs.writeFile( @@ -500,6 +506,8 @@ rules: const changeDir = path.join(changesDir, changeName); await fs.mkdir(changeDir, { recursive: true }); await fs.writeFile(path.join(changeDir, '.openspec.yaml'), `schema: ${schemaName}\n`); + const briefPath = path.join(changeDir, 'brief.md'); + await fs.writeFile(briefPath, '# Brief\nReview the API and UI.\n'); const apiPath = path.join(changeDir, 'reviews', 'api', 'notes.md'); const uiPath = path.join(changeDir, 'reviews', 'ui', 'notes.md'); @@ -511,6 +519,7 @@ rules: const empty = await readJson(['status']); expect(empty.artifacts).toMatchObject([ + { id: 'brief', status: 'done' }, { id: 'assessments', status: 'ready' }, { id: 'signoff', status: 'blocked', missingDeps: ['assessments'] }, ]); @@ -522,6 +531,7 @@ rules: await fs.writeFile(apiPath, existingContent); const partial = await readJson(['status']); expect(partial.artifacts).toMatchObject([ + { id: 'brief', status: 'done' }, { id: 'assessments', status: 'done' }, { id: 'signoff', status: 'ready' }, ]); @@ -536,6 +546,7 @@ rules: context: 'Review both the API and UI components.', rules: ['Preserve existing findings when adding a companion assessment.'], template, + dependencies: [{ id: 'brief', done: true, path: 'brief.md' }], }); expect(canonical(instructions.changeDir)).toBe(canonical(changeDir)); expect(instructions.resolvedOutputPath).toBe(path.join(instructions.changeDir, outputPath)); @@ -553,6 +564,17 @@ rules: ); expect(await fs.readFile(apiPath, 'utf-8')).toBe(existingContent); await expect(fs.stat(path.join(changeDir, 'signoff.md'))).rejects.toMatchObject({ code: 'ENOENT' }); + + await fs.unlink(briefPath); + const missingInput = await readJson(['status']); + expect(missingInput.artifacts.find((artifact: any) => artifact.id === 'assessments')).toMatchObject({ + status: 'done', + requires: ['brief'], + }); + const missingInputInstructions = await readJson(['instructions', 'assessments']); + expect(missingInputInstructions.dependencies).toMatchObject([ + { id: 'brief', done: false, path: 'brief.md' }, + ]); }); it('shows instructions for proposal on scaffolded change', async () => { diff --git a/test/core/templates/update-change.test.ts b/test/core/templates/update-change.test.ts index e3a68befb0..ca4582cfae 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -262,6 +262,10 @@ describe('update-change templates', () => { ); expect(body, label).toContain('Apply `context` and `rules` as constraints; do not copy them into the file'); expect(body, label).toContain('If instructions report `skipped: true`, do not create the file'); + expect(body, label).toContain('Read current dependency files from disk'); + expect(body, label).toContain('if a required non-skipped dependency is missing, stop and ask the user to restore it first'); + expect(body, label).toContain('If `instruction` delegates creation to another skill or command'); + expect(body, label).toContain('only if it can honor the confirmed path and these guardrails; otherwise stop'); expect(body, label).toContain( 'inside `changeRoot` that matches `artifactPaths..outputPath`' ); @@ -271,6 +275,23 @@ describe('update-change templates', () => { } }); + it('rechecks new-file scope after confirmation and refuses concurrent overwrites', () => { + for (const [label, body] of bodies) { + const confirmation = body.indexOf('create it only after the user confirms'); + const recheck = body.indexOf('After confirmation, immediately before creation'); + const create = body.indexOf('Use a create operation that fails if the target already exists'); + + expect(confirmation, label).toBeGreaterThanOrEqual(0); + expect(recheck, label).toBeGreaterThan(confirmation); + expect(create, label).toBeGreaterThan(recheck); + const writeGuard = body.slice(recheck, create); + expect(writeGuard, label).toContain('refresh status and instructions'); + expect(writeGuard, label).toContain('still in scope, not skipped, and partially populated'); + expect(writeGuard, label).toContain('repeat the concrete-path checks above'); + expect(body, label).toContain('stop and reconcile with the user instead of overwriting or choosing a different path'); + } + }); + it('writes to existingOutputPaths, never to a glob resolvedOutputPath (3.4)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('artifactPaths..existingOutputPaths'); From f61bf11cbd3855e9b0cc5821971ae26e8f2e47e4 Mon Sep 17 00:00:00 2001 From: Alex <45095641+runsonmypc@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:18:58 +0200 Subject: [PATCH 6/9] fix(update): integrate glob guidance with current workflows --- skills/openspec-update-change/SKILL.md | 15 ++++++++++---- src/core/templates/workflows/update-change.ts | 16 +++++++-------- .../templates/skill-templates-parity.test.ts | 6 +++--- test/core/templates/update-change.test.ts | 20 +++++++++---------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/skills/openspec-update-change/SKILL.md b/skills/openspec-update-change/SKILL.md index 950bab665c..0504ccd5de 100644 --- a/skills/openspec-update-change/SKILL.md +++ b/skills/openspec-update-change/SKILL.md @@ -69,7 +69,13 @@ This workflow revises artifacts that already exist; `/openspec-continue-change` - Read the artifact(s) the request touches and the change's other existing artifacts. - Draft the requested edit in the conversation, not in files. Work out exactly what it changes; step 5 owns every write. Then check every other existing artifact against the drafted edit - 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. - - Propose revisions only to 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. + - Propose revisions to files that already exist (`existingOutputPaths`). If an artifact has no existing output files and status `ready` or `blocked`, note it and point the user to `/openspec-continue-change` to create them. Leave `skipped` artifacts untouched; do not treat them as missing or defer them to the continue workflow. + - A glob artifact (e.g. `specs/**/*.md`) is marked `done` after at least one file matches, and the continue workflow only handles `ready` artifacts. When reconciliation identifies a missing file for a glob artifact whose `existingOutputPaths` is non-empty: + 1. Run `openspec instructions "" --change "" --json` and use its `instruction` and `template`. Treat `context` and `rules` as constraints; do not copy them into the file. If instructions report `skipped: true`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. + 2. Choose a concrete path inside `changeRoot` that matches `artifactPaths..outputPath` and does not already exist. Verify it remains inside `changeRoot` after resolving any symlinked parent directories. The glob `resolvedOutputPath` is not a valid target. + 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. + 4. After confirmation, immediately before creation, refresh status and instructions. Verify the artifact is still in scope, not skipped, and partially populated; repeat the concrete-path checks above. + 5. Use a create operation that fails if the target already exists. If `instruction` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user rather than replacing existing content or choosing a different path. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -82,7 +88,7 @@ This workflow revises artifacts that already exist; `/openspec-continue-change` ``` 6. **Point to the next step (guidance only - NEVER act on it)** - - Artifacts with empty `existingOutputPaths` still missing -> suggest `/openspec-continue-change` to create them. + - Artifacts with empty `existingOutputPaths` and status `ready` or `blocked` -> 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`. @@ -90,13 +96,14 @@ This workflow revises artifacts that already exist; `/openspec-continue-change` 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) +- Any file created under a glob artifact that was already partially populated +- Anything deferred to `/openspec-continue-change` (artifacts with no files yet and status `ready` or `blocked`, never `skipped` artifacts) - 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`. - 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: if an artifact has empty `existingOutputPaths` and status `ready` or `blocked`, that is `/openspec-continue-change`'s job. Leave `skipped` artifacts untouched. The only new-file scope is a confirmed concrete path under a glob artifact whose `existingOutputPaths` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, recommend starting fresh with `/openspec-new-change` (the "Update vs. Start Fresh" heuristic). diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index 10c81ab529..b3992a7658 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -130,11 +130,11 @@ ${CONTINUE_SCOPE_NOTE} - Note everything that is now inconsistent, missing, or contradictory. - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. - 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Treat \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. The glob \`resolvedOutputPath\` is not a valid target. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. 4. After confirmation, immediately before creation, refresh status and instructions. Verify the artifact is still in scope, not skipped, and partially populated; repeat the concrete-path checks above. - 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user instead of overwriting or choosing a different path. + 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user rather than replacing existing content or choosing a different path. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -162,7 +162,7 @@ After each invocation, show: **Guardrails** - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. -- Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. +- Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. - Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\` and status \`ready\` or \`blocked\`, ${CONTINUE_FRONTIER}. Leave \`skipped\` artifacts untouched. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.`, @@ -231,11 +231,11 @@ ${CONTINUE_SCOPE_NOTE} - Note everything that is now inconsistent, missing, or contradictory. - Propose revisions to files that already exist (\`existingOutputPaths\`). If an artifact has no existing output files and status \`ready\` or \`blocked\`, note it and ${CONTINUE_CREATE_THEM}. Leave \`skipped\` artifacts untouched; do not treat them as missing or defer them to the continue workflow. - A glob artifact (e.g. \`specs/**/*.md\`) is marked \`done\` after at least one file matches, and the continue workflow only handles \`ready\` artifacts. When reconciliation identifies a missing file for a glob artifact whose \`existingOutputPaths\` is non-empty: - 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Apply \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. - 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. Never write to the glob \`resolvedOutputPath\`. + 1. Run \`openspec instructions "" --change "" --json\` and use its \`instruction\` and \`template\`. Treat \`context\` and \`rules\` as constraints; do not copy them into the file. If instructions report \`skipped: true\`, do not create the file. Read current dependency files from disk; if a required non-skipped dependency is missing, stop and ask the user to restore it first. + 2. Choose a concrete path inside \`changeRoot\` that matches \`artifactPaths..outputPath\` and does not already exist. Verify it remains inside \`changeRoot\` after resolving any symlinked parent directories. The glob \`resolvedOutputPath\` is not a valid target. 3. Include the new file in step 5's proposed revisions and create it only after the user confirms. 4. After confirmation, immediately before creation, refresh status and instructions. Verify the artifact is still in scope, not skipped, and partially populated; repeat the concrete-path checks above. - 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user instead of overwriting or choosing a different path. + 5. Use a create operation that fails if the target already exists. If \`instruction\` delegates creation to another skill or command, invoke it only if it can honor the confirmed path and these guardrails; otherwise stop. If any check fails or the confirmed draft is no longer valid, stop and reconcile with the user rather than replacing existing content or choosing a different path. - If the change is already coherent, say so and propose no revisions. 5. **Confirm and apply, one artifact at a time** @@ -263,7 +263,7 @@ After each invocation, show: **Guardrails** - Planning artifacts only - NEVER edit implementation code. If the revised plan implies code changes, ${APPLY_GUARDRAIL}. - Use the artifact ids and paths reported by \`openspec status\`; never branch on hardcoded artifact names. -- Write only concrete file paths; never write to a glob \`resolvedOutputPath\`. +- Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. - Do not advance the build frontier: if an artifact has empty \`existingOutputPaths\` and status \`ready\` or \`blocked\`, ${CONTINUE_FRONTIER}. Leave \`skipped\` artifacts untouched. The only new-file scope is a confirmed concrete path under a glob artifact whose \`existingOutputPaths\` is non-empty. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, ${INTENT_CHANGE_GUARDRAIL}.` diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 3cbe25b3a3..0b69ee05bd 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -99,8 +99,8 @@ const EXPECTED_FUNCTION_HASHES: Record = { getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8', getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', - getUpdateChangeSkillTemplate: 'f4c38adf3c82b3e0af7c460de97b72740d69a8966b5426b259f7c2cb6dc11d3d', - getOpsxUpdateCommandTemplate: 'a3156c2c3b4a429fed56545f315f66a7cc25bc9f8822c5fe30a60ccd87159a0f', + getUpdateChangeSkillTemplate: '90c49d56fbc4feb67983d18064aa6b60909a9f81d5b7f6ff192f877d6b7a6b6c', + getOpsxUpdateCommandTemplate: 'aa86facbb5354ba7316e0a708bae40d5ab87553be37ef2c220a832130cd73e0c', }; const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { @@ -115,7 +115,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-verify-change': 'ad8a3098bd27d852721687c47a12db7107ed8b8dfc7f071406bb19961652e7ee', 'openspec-onboard': 'd4c5f3e24c19c8e389950544ea0d1844027753def14748c9684210ae4c6cd5e5', 'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b', - 'openspec-update-change': '19163b8c1b40ccdc0840019aa8005877a90a3a1cd9f7aadb87f76ccce1342f19', + 'openspec-update-change': 'c53a8416550d1c2dd7b2131acddd3498b8eef5c10c86bc2bc21bb1cdecb891aa', }; // 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 ca4582cfae..993bb9c366 100644 --- a/test/core/templates/update-change.test.ts +++ b/test/core/templates/update-change.test.ts @@ -242,25 +242,25 @@ describe('update-change templates', () => { expect(body, label).toContain('stop and point to `/opsx:apply`'); expect(body, label).toContain('Do not advance the build frontier'); expect(body, label).toContain( - 'Leave an artifact with no existing output files and status `ready` or `blocked` for `/opsx:continue`' + 'no existing output files and status `ready` or `blocked`, note it and point the user to `/opsx:continue`' ); expect(body, label).toContain( - 'leave artifacts with empty `existingOutputPaths` and status `ready` or `blocked` for `/opsx:continue`' + 'empty `existingOutputPaths` and status `ready` or `blocked`, that is `/opsx:continue`\'s job' ); expect(body, label).toContain('Leave `skipped` artifacts untouched'); - expect(body, label).toContain('do not treat them as missing or send them to `/opsx:continue`'); + expect(body, label).toContain('do not treat them as missing or defer them to the continue workflow'); } }); it('fills a gap under an already-satisfied glob artifact instead of deferring it (3.3a)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('is marked `done` after at least one file matches'); - expect(body, label).toContain('`/opsx:continue` only handles `ready` artifacts'); + expect(body, label).toContain('the continue workflow only handles `ready` artifacts'); expect(body, label).toContain('whose `existingOutputPaths` is non-empty'); expect(body, label).toContain( 'use its `instruction` and `template`' ); - expect(body, label).toContain('Apply `context` and `rules` as constraints; do not copy them into the file'); + expect(body, label).toContain('Treat `context` and `rules` as constraints; do not copy them into the file'); expect(body, label).toContain('If instructions report `skipped: true`, do not create the file'); expect(body, label).toContain('Read current dependency files from disk'); expect(body, label).toContain('if a required non-skipped dependency is missing, stop and ask the user to restore it first'); @@ -288,16 +288,16 @@ describe('update-change templates', () => { expect(writeGuard, label).toContain('refresh status and instructions'); expect(writeGuard, label).toContain('still in scope, not skipped, and partially populated'); expect(writeGuard, label).toContain('repeat the concrete-path checks above'); - expect(body, label).toContain('stop and reconcile with the user instead of overwriting or choosing a different path'); + expect(body, label).toContain('stop and reconcile with the user rather than replacing existing content or choosing a different path'); } }); it('writes to existingOutputPaths, never to a glob resolvedOutputPath (3.4)', () => { for (const [label, body] of bodies) { expect(body, label).toContain('artifactPaths..existingOutputPaths'); - expect(body, label).toContain('`resolvedOutputPath` is still a pattern'); - expect(body, label).toContain('Never write to the glob `resolvedOutputPath`'); - expect(body, label).toContain('The only new-file exception'); + expect(body, label).toContain('it is still the glob pattern'); + expect(body, label).toContain('The glob `resolvedOutputPath` is not a valid target'); + expect(body, label).toContain('The only new-file scope'); } }); @@ -348,7 +348,7 @@ describe('update-change templates', () => { it('confirms every edit and redirects intent changes to /opsx:new when installed', () => { for (const [label, body] of bodies) { const reconciliation = body.slice(body.indexOf('4. **Read and reconcile**'), body.indexOf('5. **Confirm and apply')); - expect(reconciliation, label).toContain('Draft the requested edit without writing'); + expect(reconciliation, label).toContain('Draft the requested edit in the conversation, not in files'); expect(reconciliation, label).not.toContain('Apply the requested edit'); expect(body, label).toContain('Write only after the user confirms'); expect(body, label).toContain('If the user rejects a revision, do not write it'); From 0f7c41454a7ca928747b33a49e52260976b0b4d3 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 22 Sep 2026 13:38:09 -0500 Subject: [PATCH 7/9] docs(skills): note update's glob companion-file exception docs-lab/reference/skills.md said update creates nothing new, which this change makes false for glob artifacts. Co-Authored-By: Claude Opus 5 --- docs-lab/reference/skills.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-lab/reference/skills.md b/docs-lab/reference/skills.md index cc833427d0..838a501789 100644 --- a/docs-lab/reference/skills.md +++ b/docs-lab/reference/skills.md @@ -101,7 +101,7 @@ other. | Contract | Description | |---|---| | **Arguments** | A change proposal name, optional, plus the revision you want. With no revision stated it runs a coherence review: artifacts checked against each other for contradictions, gaps, and duplication. | -| **Creates** | Nothing new. Edits only artifact files that already exist. Missing artifacts are `openspec-continue-change`'s job. Without that skill (the core profile leaves it out), it points to `openspec status` and `openspec instructions` instead. Never code. | +| **Creates** | Edits artifact files that already exist. One exception: for an artifact written as a glob, such as `specs/**/*.md`, that already has at least one file, it can add a missing companion file once you confirm the path. An artifact with no files yet is `openspec-continue-change`'s job. Without that skill (the core profile leaves it out), it points to `openspec status` and `openspec instructions` instead. Never code. | | **Response** | Shows each proposed revision and writes it only after you confirm, one artifact at a time. Ends with what was revised and the next step; implementation waits for `openspec-apply-change`. | ## openspec-sync-specs From 23319a383779ab3969bfab3b7890c544a270ed08 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 22 Sep 2026 14:52:52 -0500 Subject: [PATCH 8/9] test(parity): restore the #1837 regression tests dropped in the merge The earlier conflict resolution took our whole side of the parity file, which discarded the two threshold tests main gained in #1940. Take main's file verbatim and regenerate the hashes instead. Co-Authored-By: Claude Opus 5 --- .../templates/skill-templates-parity.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 0b69ee05bd..25c576430e 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -156,6 +156,51 @@ function hash(value: string): string { } describe('skill templates split parity', () => { + it('uses one clarification threshold in fast-forward guidance (#1837)', () => { + const variants: Array<[string, string]> = [ + ['ff skill', getFfChangeSkillTemplate().instructions], + ['ff command', getOpsxFfCommandTemplate().content], + ]; + + for (const [variant, content] of variants) { + expect(content, variant).toContain( + '**If an artifact requires user input** (critically unclear context)' + ); + expect(content, variant).not.toContain( + '**If an artifact requires user input** (unclear context)' + ); + } + }); + + it('approves onboarding tasks before saving or offering implementation (#1837)', () => { + const variants: Array<[string, string]> = [ + ['onboard skill', getOnboardSkillTemplate().instructions], + ['onboard command', getOpsxOnboardCommandTemplate().content], + ]; + + for (const [variant, content] of variants) { + expect(content, variant).toContain('Does this task breakdown look right?'); + expect(content, variant).not.toContain( + 'Each checkbox becomes a unit of work in the apply phase. Ready to implement?' + ); + expect(content, variant).toContain( + '**PAUSE** - Wait for user approval/feedback.\n\n' + + 'After approval, save to the `resolvedOutputPath` from `openspec instructions tasks --change "" --json`.' + ); + expect(content, variant).toContain('> "Tasks are saved. Ready to implement?"'); + expect(content, variant).toContain( + '**PAUSE** - Wait for user to confirm before implementation.' + ); + + const saveAt = content.indexOf('After approval, save to the `resolvedOutputPath`'); + const implementationChoiceAt = content.indexOf('> "Tasks are saved. Ready to implement?"'); + const implementationAt = content.indexOf('## Phase 9: Apply (Implementation)'); + expect(saveAt, variant).toBeGreaterThanOrEqual(0); + expect(implementationChoiceAt, variant).toBeGreaterThan(saveAt); + expect(implementationAt, variant).toBeGreaterThan(implementationChoiceAt); + } + }); + it('preserves all template function payloads exactly', () => { const functionFactories: Record unknown> = { getExploreSkillTemplate, From cfa3048637b97f737d8331996b32b455651e23a5 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 22 Sep 2026 18:10:04 -0500 Subject: [PATCH 9/9] test(explore-docs): allow the update guidance line that says "no files yet" main's #1833 guard flags any docs line containing "no files". The new /opsx:update guidance describes which artifacts update leaves to continue, not what explore writes, so allow that exact line. Co-Authored-By: Claude Opus 5 --- test/explore-docs-claims.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/explore-docs-claims.test.ts b/test/explore-docs-claims.test.ts index 287a4b083b..76b283f5d5 100644 --- a/test/explore-docs-claims.test.ts +++ b/test/explore-docs-claims.test.ts @@ -68,6 +68,9 @@ const FORBIDDEN: readonly RegExp[] = [ const ALLOW = [ // docs/commands.md and docs/troubleshooting.md quote this CLI message. /"No artifacts ready"/, + // docs/commands.md describes which artifacts `/opsx:update` leaves to + // `/opsx:continue`. "no files yet" is about an update target, not explore. + /Artifacts with no files yet remain with/, ]; const TEST_FILE = 'test/explore-docs-claims.test.ts';