From 4eb4e789b569177c22a219491f9190dba986914e Mon Sep 17 00:00:00 2001 From: Ryan de Melo Date: Wed, 23 Sep 2026 21:46:14 +0800 Subject: [PATCH 1/4] fix(verify): stop reporting removed requirements as missing Verify walked every "### Requirement:" in the delta specs and looked for an implementation of each one, whatever section it sat under. A REMOVED requirement that the change had removed correctly came back as CRITICAL "Requirement not found", with a recommendation to implement it. An agent that follows the report puts back the behavior the change just deleted. Verify now notes the delta section of each requirement first. ADDED and MODIFIED keep the existing checks. REMOVED is checked the other way round: finding nothing is the expected result, and it is only critical while the behavior is still in the code. RENAMED only changes a name, so the old name is not reported as missing. Scenario coverage skips removed requirements, since there is nothing left to cover. Archive and sync already handle each section on its own terms; verify was the one step in the loop that did not. Closes #1959 --- .changeset/verify-removed-requirements.md | 5 ++ openspec/specs/opsx-verify-skill/spec.md | 13 +++- skills/openspec-verify-change/SKILL.md | 19 +++-- src/core/templates/workflows/verify-change.ts | 38 +++++++--- .../templates/skill-templates-parity.test.ts | 6 +- .../verify-change-delta-operations.test.ts | 69 +++++++++++++++++++ 6 files changed, 131 insertions(+), 19 deletions(-) create mode 100644 .changeset/verify-removed-requirements.md create mode 100644 test/core/templates/verify-change-delta-operations.test.ts diff --git a/.changeset/verify-removed-requirements.md b/.changeset/verify-removed-requirements.md new file mode 100644 index 0000000000..bd40bbbc93 --- /dev/null +++ b/.changeset/verify-removed-requirements.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Stop `/opsx:verify` from reporting a correctly removed requirement as missing. Verify now reads which delta section each requirement sits under: ADDED and MODIFIED requirements are checked for an implementation as before, a REMOVED requirement passes once its behavior is gone and is flagged only while it is still present, and the old name of a RENAMED requirement is no longer reported as missing. diff --git a/openspec/specs/opsx-verify-skill/spec.md b/openspec/specs/opsx-verify-skill/spec.md index 91562c0e55..9909b5854a 100644 --- a/openspec/specs/opsx-verify-skill/spec.md +++ b/openspec/specs/opsx-verify-skill/spec.md @@ -80,10 +80,21 @@ The agent SHALL verify that implementation matches the specifications. - **AND** suggest: either update implementation or update spec to match reality #### Scenario: Missing implementation -- **WHEN** no implementation found for a requirement +- **WHEN** no implementation found for an ADDED or MODIFIED requirement - **THEN** report as CRITICAL issue - **AND** suggest: "Implement requirement X" with guidance on what's needed +#### Scenario: Removed requirement +- **WHEN** a requirement sits under `## REMOVED Requirements` in a delta spec +- **THEN** the agent treats the absence of its implementation as the expected result +- **AND** does not report it as missing or suggest implementing it +- **AND** reports it as CRITICAL only if the removed behavior is still present in the codebase +- **AND** skips scenario coverage for it + +#### Scenario: Renamed requirement +- **WHEN** a requirement is listed under `## RENAMED Requirements` in a delta spec +- **THEN** the agent does not report its FROM name as missing + ### Requirement: Coherence Verification The agent SHALL verify that implementation is sensible and follows design decisions. diff --git a/skills/openspec-verify-change/SKILL.md b/skills/openspec-verify-change/SKILL.md index 355febc9b2..9e7a4ecf11 100644 --- a/skills/openspec-verify-change/SKILL.md +++ b/skills/openspec-verify-change/SKILL.md @@ -81,18 +81,25 @@ In both branches, never create the root as a side effect: do not run `openspec i **Spec Coverage**: - If delta specs exist in `contextFiles.specs`: - - Extract all requirements (marked with "### Requirement:") - - For each requirement: + - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: `## ADDED`, `## MODIFIED`, `## REMOVED`, or `## RENAMED Requirements`. The section decides what the check looks for. + - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists - - If requirements appear unimplemented: + - If ADDED or MODIFIED requirements appear unimplemented: - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " + - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: + - Search codebase for the removed behavior + - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. + - If the behavior is still present: + - Add CRITICAL issue: "Removed requirement still implemented: " + - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" + - A RENAMED entry (`FROM:`/`TO:`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. 6. **Verify Correctness** **Requirement Implementation Mapping**: - - For each requirement from delta specs: + - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -101,12 +108,13 @@ In both branches, never create the root as a side effect: do not run `openspec i - Recommendation: "Review : against requirement X" **Scenario Coverage**: - - For each scenario in delta specs (marked with "#### Scenario:"): + - For each scenario under an ADDED or MODIFIED requirement in delta specs (marked with "#### Scenario:"): - Check if conditions are handled in code - Check if tests exist covering the scenario - If scenario appears uncovered: - Add WARNING: "Scenario not covered: " - Recommendation: "Add test or implementation for scenario: " + - Skip scenarios under a REMOVED requirement; that behavior is meant to be gone. 7. **Verify Coherence** @@ -145,6 +153,7 @@ In both branches, never create the root as a side effect: do not run `openspec i 1. **CRITICAL** (Must fix before archive): - Incomplete tasks - Missing requirement implementations + - Removed requirements still implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): diff --git a/src/core/templates/workflows/verify-change.ts b/src/core/templates/workflows/verify-change.ts index 8bc85edfb6..633043108b 100644 --- a/src/core/templates/workflows/verify-change.ts +++ b/src/core/templates/workflows/verify-change.ts @@ -75,18 +75,25 @@ ${PROJECT_ROOT_GUARD} **Spec Coverage**: - If delta specs exist in \`contextFiles.specs\`: - - Extract all requirements (marked with "### Requirement:") - - For each requirement: + - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. + - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists - - If requirements appear unimplemented: + - If ADDED or MODIFIED requirements appear unimplemented: - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " + - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: + - Search codebase for the removed behavior + - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. + - If the behavior is still present: + - Add CRITICAL issue: "Removed requirement still implemented: " + - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" + - A RENAMED entry (\`FROM:\`/\`TO:\`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. 6. **Verify Correctness** **Requirement Implementation Mapping**: - - For each requirement from delta specs: + - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -95,12 +102,13 @@ ${PROJECT_ROOT_GUARD} - Recommendation: "Review : against requirement X" **Scenario Coverage**: - - For each scenario in delta specs (marked with "#### Scenario:"): + - For each scenario under an ADDED or MODIFIED requirement in delta specs (marked with "#### Scenario:"): - Check if conditions are handled in code - Check if tests exist covering the scenario - If scenario appears uncovered: - Add WARNING: "Scenario not covered: " - Recommendation: "Add test or implementation for scenario: " + - Skip scenarios under a REMOVED requirement; that behavior is meant to be gone. 7. **Verify Coherence** @@ -139,6 +147,7 @@ ${PROJECT_ROOT_GUARD} 1. **CRITICAL** (Must fix before archive): - Incomplete tasks - Missing requirement implementations + - Removed requirements still implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): @@ -254,18 +263,25 @@ ${PROJECT_ROOT_GUARD} **Spec Coverage**: - If delta specs exist in \`contextFiles.specs\`: - - Extract all requirements (marked with "### Requirement:") - - For each requirement: + - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. + - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists - - If requirements appear unimplemented: + - If ADDED or MODIFIED requirements appear unimplemented: - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " + - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: + - Search codebase for the removed behavior + - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. + - If the behavior is still present: + - Add CRITICAL issue: "Removed requirement still implemented: " + - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" + - A RENAMED entry (\`FROM:\`/\`TO:\`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. 6. **Verify Correctness** **Requirement Implementation Mapping**: - - For each requirement from delta specs: + - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -274,12 +290,13 @@ ${PROJECT_ROOT_GUARD} - Recommendation: "Review : against requirement X" **Scenario Coverage**: - - For each scenario in delta specs (marked with "#### Scenario:"): + - For each scenario under an ADDED or MODIFIED requirement in delta specs (marked with "#### Scenario:"): - Check if conditions are handled in code - Check if tests exist covering the scenario - If scenario appears uncovered: - Add WARNING: "Scenario not covered: " - Recommendation: "Add test or implementation for scenario: " + - Skip scenarios under a REMOVED requirement; that behavior is meant to be gone. 7. **Verify Coherence** @@ -318,6 +335,7 @@ ${PROJECT_ROOT_GUARD} 1. **CRITICAL** (Must fix before archive): - Incomplete tasks - Missing requirement implementations + - Removed requirements still implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 4ab2bd8cb2..6bc04a0a44 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -91,11 +91,11 @@ const EXPECTED_FUNCTION_HASHES: Record = { getArchiveChangeSkillTemplate: '8447a2489240bf0c27f863065d61453dd0264842d1dabafe27b577d6bff96eb3', getBulkArchiveChangeSkillTemplate: 'f17399959921ff98c7798e4591c8888825b7c9a83b0a90f09d98c7e0984ab793', getOpsxSyncCommandTemplate: '60550b7bb9829421656d6324a9e4c951bc912f48f88882d1a07ce7f78397a5e7', - getVerifyChangeSkillTemplate: '2e069a277dac23818b13bb50b66e806ab405bc3b7f535400e1ebf81b84153699', + getVerifyChangeSkillTemplate: '3e6eec9bb151aca4af6868d8a9f0fa51f9bac64b3a82d1bae039a7a667341079', getOpsxArchiveCommandTemplate: '980109e5f8362610872c70fe0a0f1d48d3d2692275b2b17e2f4c91c3de89c2fd', getOpsxOnboardCommandTemplate: '35332b79e943daefd4118513f03dc48926267af60348f781b8d92bbe5086a986', getOpsxBulkArchiveCommandTemplate: '3db03eadb764abd74c8c180656c3f64a8b9a4971056c91624d38df3209d7b446', - getOpsxVerifyCommandTemplate: '938f52f20fb9a3b811ea47314baac1034cd550e8ab363ae878ccba4b6329348f', + getOpsxVerifyCommandTemplate: '3f8c4248c807ad3b74ae2d5289aa6bb26359acf3ccce262b5dd0031af893cf8e', getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8', getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', @@ -112,7 +112,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-sync-specs': '3909936a236a21a9a6d5bf495f90b396b3b68fc9220d7b2c1894668653beb2e4', 'openspec-archive-change': '305a21a9c76a925055f3bdbaac504f208660ef6948d78f73928de166250609bf', 'openspec-bulk-archive-change': '4bd638a50111d2ee3a667752a2355ed513f770695b137b93fc28848ca7bf60d2', - 'openspec-verify-change': 'ad8a3098bd27d852721687c47a12db7107ed8b8dfc7f071406bb19961652e7ee', + 'openspec-verify-change': '729cddd950b1ff4ded4e048315ac4d641d94b862bc6bf95a26e60fd478f64783', 'openspec-onboard': '526bb7f9b8ceb8670b600ce33b0a62fe268393b7e778e120c796316da3a3cc3d', 'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b', 'openspec-update-change': '19163b8c1b40ccdc0840019aa8005877a90a3a1cd9f7aadb87f76ccce1342f19', diff --git a/test/core/templates/verify-change-delta-operations.test.ts b/test/core/templates/verify-change-delta-operations.test.ts new file mode 100644 index 0000000000..c75eb31a6d --- /dev/null +++ b/test/core/templates/verify-change-delta-operations.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from 'vitest'; + +import { + getVerifyChangeSkillTemplate, + getOpsxVerifyCommandTemplate, +} from '../../../src/core/templates/skill-templates.js'; + +// #1959: verify treated every "### Requirement:" in a change's delta specs as +// behavior that must exist, whichever section it sat under. A REMOVED +// requirement that was removed correctly came back as CRITICAL "Requirement not +// found" with the recommendation to implement it, so an agent following the +// report restored what the change had just deleted. +const bodies: Array<[string, string]> = [ + ['skill', getVerifyChangeSkillTemplate().instructions], + ['command', getOpsxVerifyCommandTemplate().content], +]; + +function section(body: string, start: string, end: string, label: string): string { + const from = body.indexOf(start); + const to = body.indexOf(end, from + start.length); + expect(from, `${label}: "${start}" not found`).toBeGreaterThanOrEqual(0); + expect(to, `${label}: "${end}" not found after "${start}"`).toBeGreaterThan(from); + return body.slice(from, to); +} + +describe('verify checks each requirement by its delta operation', () => { + it.each(bodies)('%s: classifies requirements by delta section before checking them', (label, body) => { + const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + + for (const header of ['## ADDED', '## MODIFIED', '## REMOVED', '## RENAMED Requirements']) { + expect(coverage, label).toContain(header); + } + // The unscoped loop is what produced the bug. + expect(coverage, label).not.toMatch(/^\s*- For each requirement:$/m); + }); + + it.each(bodies)('%s: reports a missing requirement only for ADDED or MODIFIED', (label, body) => { + const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + const addedOrModified = section(coverage, '- For each ADDED or MODIFIED requirement', '- For each REMOVED requirement', label); + + expect(addedOrModified, label).toContain('Add CRITICAL issue: "Requirement not found: "'); + }); + + it.each(bodies)('%s: inverts the check for a REMOVED requirement', (label, body) => { + const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + const removed = section(coverage, '- For each REMOVED requirement', '- A RENAMED entry', label); + + expect(removed, label).toContain('Finding no implementation is the expected result.'); + expect(removed, label).toContain('Never report a REMOVED requirement as "Requirement not found"'); + expect(removed, label).toContain('Add CRITICAL issue: "Removed requirement still implemented: "'); + expect(removed, label).not.toContain('Recommendation: "Implement'); + }); + + it.each(bodies)('%s: does not report the old name of a RENAMED requirement as missing', (label, body) => { + const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + + expect(coverage, label).toContain('Do not report the FROM name as missing.'); + }); + + it.each(bodies)('%s: maps implementation and scenarios only for ADDED or MODIFIED requirements', (label, body) => { + const correctness = section(body, '6. **Verify Correctness**', '7. **Verify Coherence**', label); + + expect(correctness, label).toContain('- For each ADDED or MODIFIED requirement from delta specs'); + expect(correctness, label).toContain('- For each scenario under an ADDED or MODIFIED requirement in delta specs'); + expect(correctness, label).toContain('Skip scenarios under a REMOVED requirement'); + expect(correctness, label).not.toContain('- For each requirement from delta specs:'); + expect(correctness, label).not.toContain('- For each scenario in delta specs'); + }); +}); From 0fa39af7eaa301a278962bd29be4aa4ae25dcdb9 Mon Sep 17 00:00:00 2001 From: Ryan de Melo Date: Wed, 23 Sep 2026 21:53:32 +0800 Subject: [PATCH 2/4] docs(specs): scope the general verify scenarios to ADDED and MODIFIED The Spec coverage, Requirement implementation mapping and Scenario coverage scenarios still told the verifier to check every requirement in the delta specs, which contradicts the Removed requirement scenario added in the previous commit. A verifier following them would repeat the #1959 failure. --- openspec/specs/opsx-verify-skill/spec.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/openspec/specs/opsx-verify-skill/spec.md b/openspec/specs/opsx-verify-skill/spec.md index 9909b5854a..96014d4517 100644 --- a/openspec/specs/opsx-verify-skill/spec.md +++ b/openspec/specs/opsx-verify-skill/spec.md @@ -35,9 +35,10 @@ The agent SHALL verify that all required work has been completed. #### Scenario: Spec coverage check - **WHEN** verifying completeness - **AND** delta specs exist in `openspec/changes//specs/` -- **THEN** the agent extracts all requirements from delta specs -- **AND** searches codebase for implementation of each requirement -- **AND** reports which requirements appear to have implementation vs which are missing +- **THEN** the agent extracts all requirements from delta specs, noting the delta section each one sits under +- **AND** searches codebase for implementation of each ADDED or MODIFIED requirement +- **AND** reports which ADDED or MODIFIED requirements appear to have implementation vs which are missing +- **AND** checks REMOVED and RENAMED requirements as described in the Removed requirement and Renamed requirement scenarios #### Scenario: All tasks complete - **WHEN** all tasks are marked complete @@ -56,14 +57,14 @@ The agent SHALL verify that implementation matches the specifications. #### Scenario: Requirement implementation mapping - **WHEN** verifying correctness -- **THEN** for each requirement in delta specs: +- **THEN** for each ADDED or MODIFIED requirement in delta specs: - Search codebase for implementation - Identify relevant files and line numbers - Assess whether implementation satisfies the requirement #### Scenario: Scenario coverage check - **WHEN** verifying correctness -- **THEN** for each scenario in delta specs: +- **THEN** for each scenario under an ADDED or MODIFIED requirement in delta specs: - Check if the scenario's conditions are handled in code - Check if tests exist that cover the scenario - Report coverage status From bd4e28d9c5d744be1d4c58a2517bd60dcb0d67b0 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 23 Sep 2026 13:40:23 -0500 Subject: [PATCH 3/4] fix(verify): report a removal-only change as ready when nothing remains With #1732 merged, a change whose delta specs only remove or rename requirements left Requirement Implementation Mapping and Scenario Coverage with nothing to check. The "no usable requirements" rule then marked them not verified, so verify never reported the change ready, the exact case #1959 describes. Those two checks are now not applicable when the readable delta specs hold REMOVED or RENAMED requirements and no ADDED or MODIFIED ones. An empty or unparseable delta still marks them not verified. Keyword matches in openspec/ artifacts, docs, or code that serves only the Migration note or an ADDED requirement are no longer evidence by themselves that a removed requirement is still implemented; a code path that still delivers the removed behavior is reported even when shared. The summary counts removals separately from covered requirements. Co-Authored-By: Claude Opus 5.5 --- openspec/specs/opsx-verify-skill/spec.md | 8 ++++++ skills/openspec-verify-change/SKILL.md | 8 +++--- src/core/templates/workflows/verify-change.ts | 16 +++++++----- .../templates/skill-templates-parity.test.ts | 6 ++--- .../verify-change-delta-operations.test.ts | 25 +++++++++++++++++++ 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/openspec/specs/opsx-verify-skill/spec.md b/openspec/specs/opsx-verify-skill/spec.md index 64e03dde3a..781a29014b 100644 --- a/openspec/specs/opsx-verify-skill/spec.md +++ b/openspec/specs/opsx-verify-skill/spec.md @@ -107,11 +107,19 @@ The agent SHALL verify that implementation matches the specifications. - **AND** does not report it as missing or suggest implementing it - **AND** reports it as CRITICAL only if the removed behavior is still present in the codebase - **AND** skips scenario coverage for it +- **AND** does not treat matches in OpenSpec artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, as evidence by themselves +- **AND** still reports a code path that delivers the removed behavior, even when it is shared with an ADDED requirement #### Scenario: Renamed requirement - **WHEN** a requirement is listed under `## RENAMED Requirements` in a delta spec - **THEN** the agent does not report its FROM name as missing +#### Scenario: Change that only removes or renames requirements +- **WHEN** the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements +- **THEN** the agent reports requirement implementation mapping and scenario coverage as not applicable +- **AND** does not mark them as not verified or withhold readiness because of them +- **AND** a delta spec with no parseable requirements still marks them as not verified + ### Requirement: Coherence Verification The agent SHALL verify that implementation is sensible and follows design decisions. diff --git a/skills/openspec-verify-change/SKILL.md b/skills/openspec-verify-change/SKILL.md index 6e7f8a64ad..5e7828fbac 100644 --- a/skills/openspec-verify-change/SKILL.md +++ b/skills/openspec-verify-change/SKILL.md @@ -71,7 +71,7 @@ In both branches, never create the root as a side effect: do not run `openspec i Verification is advisory. Respect intentional omissions such as `skip_specs: true`, optional design documents, and schemas without task tracking. Do not require or invent optional or intentionally omitted artifacts to obtain a clean report. `Not verified` describes a limit of this report, not a new archive prerequisite. Archive retains its own checks and user-confirmation behavior. - Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. + Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. The correctness checks of a change whose readable delta specs contain REMOVED or RENAMED requirements but no ADDED or MODIFIED requirements are also **Not applicable** (see step 6). Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. If only task evidence is available for applicable checks, verify task completion only and mark the remaining applicable checks, including **Code Pattern Consistency**, as not verified with the reason "Only task evidence available". @@ -101,7 +101,7 @@ In both branches, never create the root as a side effect: do not run `openspec i - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: - - Search codebase for the removed behavior + - Search codebase for the removed behavior. Matches in `openspec/` artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, are not evidence by themselves. Report any code path that still delivers the removed behavior, including one shared with an ADDED requirement. - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " @@ -110,6 +110,8 @@ In both branches, never create the root as a side effect: do not run `openspec i 6. **Verify Correctness** + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + **Requirement Implementation Mapping**: - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence @@ -162,7 +164,7 @@ In both branches, never create the root as a side effect: do not run `openspec i | Coherence | Followed/Issues | ``` - In each Status cell, report the results of checks that ran and `Not verified ()` for every skipped check. If all checks in a dimension were skipped, start the cell with `Not verified`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. + In each Status cell, report the results of checks that ran and `Not verified ()` for every skipped check. If all checks in a dimension were skipped, start the cell with `Not verified`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads `Not applicable (no ADDED or MODIFIED requirements)`. **Issues by Priority**: diff --git a/src/core/templates/workflows/verify-change.ts b/src/core/templates/workflows/verify-change.ts index af49b904c7..028e2d0397 100644 --- a/src/core/templates/workflows/verify-change.ts +++ b/src/core/templates/workflows/verify-change.ts @@ -65,7 +65,7 @@ ${PROJECT_ROOT_GUARD} Verification is advisory. Respect intentional omissions such as \`skip_specs: true\`, optional design documents, and schemas without task tracking. Do not require or invent optional or intentionally omitted artifacts to obtain a clean report. \`Not verified\` describes a limit of this report, not a new archive prerequisite. Archive retains its own checks and user-confirmation behavior. - Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. + Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. The correctness checks of a change whose readable delta specs contain REMOVED or RENAMED requirements but no ADDED or MODIFIED requirements are also **Not applicable** (see step 6). Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. If only task evidence is available for applicable checks, verify task completion only and mark the remaining applicable checks, including **Code Pattern Consistency**, as not verified with the reason "Only task evidence available". @@ -95,7 +95,7 @@ ${PROJECT_ROOT_GUARD} - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: - - Search codebase for the removed behavior + - Search codebase for the removed behavior. Matches in \`openspec/\` artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, are not evidence by themselves. Report any code path that still delivers the removed behavior, including one shared with an ADDED requirement. - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " @@ -104,6 +104,8 @@ ${PROJECT_ROOT_GUARD} 6. **Verify Correctness** + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + **Requirement Implementation Mapping**: - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence @@ -156,7 +158,7 @@ ${PROJECT_ROOT_GUARD} | Coherence | Followed/Issues | \`\`\` - In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. + In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. **Issues by Priority**: @@ -264,7 +266,7 @@ ${PROJECT_ROOT_GUARD} Each dimension can have CRITICAL, WARNING, or SUGGESTION issues. Verification is advisory. Respect intentional omissions such as \`skip_specs: true\`, optional design documents, and schemas without task tracking. Do not require or invent optional or intentionally omitted artifacts to obtain a clean report. \`Not verified\` describes a limit of this report, not a new archive prerequisite. Archive retains its own checks and user-confirmation behavior. - Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. + Mark checks the schema does not define, or artifacts the status reports as intentionally skipped, as **Not applicable**. The correctness checks of a change whose readable delta specs contain REMOVED or RENAMED requirements but no ADDED or MODIFIED requirements are also **Not applicable** (see step 6). Exclude them from skipped-check counts and the archive-readiness assessment. Reserve **Not verified** for applicable checks whose evidence is missing or unusable. If only task evidence is available for applicable checks, verify task completion only and mark the remaining applicable checks, including **Code Pattern Consistency**, as not verified with the reason "Only task evidence available". @@ -294,7 +296,7 @@ ${PROJECT_ROOT_GUARD} - Add CRITICAL issue: "Requirement not found: " - Recommendation: "Implement requirement X: " - For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check: - - Search codebase for the removed behavior + - Search codebase for the removed behavior. Matches in \`openspec/\` artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, are not evidence by themselves. Report any code path that still delivers the removed behavior, including one shared with an ADDED requirement. - Finding no implementation is the expected result. Never report a REMOVED requirement as "Requirement not found" or recommend implementing it. - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " @@ -303,6 +305,8 @@ ${PROJECT_ROOT_GUARD} 6. **Verify Correctness** + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + **Requirement Implementation Mapping**: - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): - Search codebase for implementation evidence @@ -355,7 +359,7 @@ ${PROJECT_ROOT_GUARD} | Coherence | Followed/Issues | \`\`\` - In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. + In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. **Issues by Priority**: diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index ebde34697a..79297fa3e2 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -91,11 +91,11 @@ const EXPECTED_FUNCTION_HASHES: Record = { getArchiveChangeSkillTemplate: '71715f9d5899498942af03e182e6d1ac2c95952dde967950c2a9161084a53a8b', getBulkArchiveChangeSkillTemplate: '2a6ec08fea0f942158b4abe9c8d1af9622038e0c4dc684e7c73dad2fb8379a54', getOpsxSyncCommandTemplate: '60550b7bb9829421656d6324a9e4c951bc912f48f88882d1a07ce7f78397a5e7', - getVerifyChangeSkillTemplate: '245702eab4cc96ff66c4f50c210785b4d7fa2dc51d6dc1921cdb6b0e4f66bd15', + getVerifyChangeSkillTemplate: '3860f97217bfba502531443da7498bb25b97260101fe0d97b56dfbec53a5eb4b', getOpsxArchiveCommandTemplate: '3d2a330b46043fbb9f220831aa42ebbb62f411e9597b2bb491ad1ac1fa2d0873', getOpsxOnboardCommandTemplate: '0cf66e164c0e14c916c6d1ebb5d80ded07d7fb8e55d4eb34eba43e8ca9c28558', getOpsxBulkArchiveCommandTemplate: '4e2e39c4d634074f4a1ed67f076d5c4d0ead8b998f4d75218c33cdc6173719be', - getOpsxVerifyCommandTemplate: '8afe7e8f3a1c0124258297311d7deedad557e32988ffb69cd90d572bfce9157d', + getOpsxVerifyCommandTemplate: 'dcb2f32b721a0a2e786fca9ddd85c21a6ced5399cb43e40e25d314bc41e2f055', getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8', getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', @@ -112,7 +112,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-sync-specs': '3909936a236a21a9a6d5bf495f90b396b3b68fc9220d7b2c1894668653beb2e4', 'openspec-archive-change': 'd01d9eeb06223ee89708b7963e82c5ebc11719c5b2dc62d4abb268ee016fcb7b', 'openspec-bulk-archive-change': '10f050ad5ef77084dc55a202427988b23903ee122f00985238a4eb9354a5dc3c', - 'openspec-verify-change': '99a1b35116f4bba2939db74f8c2e7fe010d79d617405c3451f01aa798110136d', + 'openspec-verify-change': '5d4584b3a7adb64bd22bc2b4e509a58d31972865a9f3f0f625395084e55b4601', 'openspec-onboard': '6993eff867d97d485e080078f9dfb80e968e242f3b17a924eeb077715fd548fa', 'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b', 'openspec-update-change': '5f4ea19aa732b33d87a2120ec393ee34578e70678d97e8c3bb10f988c00cb4d3', diff --git a/test/core/templates/verify-change-delta-operations.test.ts b/test/core/templates/verify-change-delta-operations.test.ts index c75eb31a6d..d14e742353 100644 --- a/test/core/templates/verify-change-delta-operations.test.ts +++ b/test/core/templates/verify-change-delta-operations.test.ts @@ -66,4 +66,29 @@ describe('verify checks each requirement by its delta operation', () => { expect(correctness, label).not.toContain('- For each requirement from delta specs:'); expect(correctness, label).not.toContain('- For each scenario in delta specs'); }); + // With #1732's "Not verified" rule, a change with nothing to add or modify + // left both correctness checks empty, which read as unverified and withheld + // readiness. That is the exact case #1959 reports. + it.each(bodies)('%s: treats the correctness checks of a removal-only change as not applicable', (label, body) => { + const correctness = section(body, '6. **Verify Correctness**', '**Requirement Implementation Mapping**:', label); + + expect(correctness, label).toContain('If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements'); + // An empty or unparseable delta must not pass as a removal-only change. + expect(correctness, label).toContain('A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified.'); + expect(correctness, label).toContain('report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**'); + expect(correctness, label).toContain('do not mark these two checks as not verified'); + expect(body, label).toContain('The correctness checks of a change whose readable delta specs contain REMOVED or RENAMED requirements but no ADDED or MODIFIED requirements are also **Not applicable** (see step 6).'); + }); + + it.each(bodies)('%s: does not treat artifacts or replacement code as the removed behavior', (label, body) => { + const removed = section(body, '- For each REMOVED requirement', '- A RENAMED entry', label); + + expect(removed, label).toContain('Matches in `openspec/` artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, are not evidence by themselves.'); + expect(removed, label).toContain('Report any code path that still delivers the removed behavior, including one shared with an ADDED requirement.'); + }); + + it.each(bodies)('%s: counts removals separately from covered requirements', (label, body) => { + expect(body, label).toContain('Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately'); + expect(body, label).toContain('the Correctness cell reads `Not applicable (no ADDED or MODIFIED requirements)`'); + }); }); From 06b32048e63b27934a725ecd857cff86a9ea367c Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 23 Sep 2026 14:08:30 -0500 Subject: [PATCH 4/4] fix(verify): check a renamed requirement's behavior against its baseline A RENAMED entry only told verify not to report the FROM name as missing, and a rename-only change marked the correctness checks not applicable. Nothing checked that the renamed requirement's behavior was still implemented, so verify could report readiness unchecked. Spec Coverage now reads the baseline requirement from the main spec (under the FROM name, or the TO name once synced) and checks that its behavior is still implemented, without requiring code symbols to be renamed. A missing behavior is CRITICAL "Renamed requirement not found"; an unreadable baseline marks the entry not verified. A TO name that also appears under MODIFIED is still checked there. Regression tests cover the skill template, the command template, and the committed skills/ mirror. Co-Authored-By: Claude Opus 5.5 --- openspec/specs/opsx-verify-skill/spec.md | 4 ++ skills/openspec-verify-change/SKILL.md | 19 +++++--- src/core/templates/workflows/verify-change.ts | 38 +++++++++++----- .../templates/skill-templates-parity.test.ts | 6 +-- .../verify-change-delta-operations.test.ts | 45 ++++++++++++++++--- 5 files changed, 89 insertions(+), 23 deletions(-) diff --git a/openspec/specs/opsx-verify-skill/spec.md b/openspec/specs/opsx-verify-skill/spec.md index 781a29014b..8e69096ffd 100644 --- a/openspec/specs/opsx-verify-skill/spec.md +++ b/openspec/specs/opsx-verify-skill/spec.md @@ -113,6 +113,10 @@ The agent SHALL verify that implementation matches the specifications. #### Scenario: Renamed requirement - **WHEN** a requirement is listed under `## RENAMED Requirements` in a delta spec - **THEN** the agent does not report its FROM name as missing +- **AND** does not require code symbols or file names to be renamed +- **AND** unless the TO name also appears under MODIFIED, verifies that the behavior of the baseline requirement (its body and scenarios in the main spec, under the FROM name, or under the TO name only when the main spec is already synced) is still implemented +- **AND** reports CRITICAL "Renamed requirement not found" when that behavior is missing +- **AND** marks spec coverage as not verified for the entry when the baseline requirement cannot be found or read #### Scenario: Change that only removes or renames requirements - **WHEN** the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements diff --git a/skills/openspec-verify-change/SKILL.md b/skills/openspec-verify-change/SKILL.md index 5e7828fbac..41be312432 100644 --- a/skills/openspec-verify-change/SKILL.md +++ b/skills/openspec-verify-change/SKILL.md @@ -93,7 +93,7 @@ In both branches, never create the root as a side effect: do not run `openspec i - If status marks the spec artifact skipped by `skip_specs: true`, or the schema defines no spec artifact, report the spec-dependent checks as not applicable. - Otherwise, `contextFiles` is keyed by artifact id, and artifact ids come from the active schema. If `contextFiles.specs` is absent or empty, mark **Spec Coverage**, **Requirement Implementation Mapping**, and **Scenario Coverage** as not verified; do not treat any of them as clean. - If delta specs exist in `contextFiles.specs`: - - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: `## ADDED`, `## MODIFIED`, `## REMOVED`, or `## RENAMED Requirements`. The section decides what the check looks for. + - Extract all requirements (marked with "### Requirement:", or listed as `FROM:`/`TO:` pairs under `## RENAMED Requirements`) and note the delta section each one sits under: `## ADDED`, `## MODIFIED`, `## REMOVED`, or `## RENAMED Requirements`. The section decides what the check looks for. - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists @@ -106,14 +106,22 @@ In both branches, never create the root as a side effect: do not run `openspec i - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" - - A RENAMED entry (`FROM:`/`TO:`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. + - For each RENAMED entry (`FROM:`/`TO:`), the name changes but the behavior stays, so check the TO requirement for that unchanged behavior: + - Do not report the FROM name as missing, and do not require code symbols, identifiers, or file names to be renamed. + - If the TO name also appears under MODIFIED, its behavior is checked there against the MODIFIED text; skip it here. + - Otherwise, read the baseline requirement in the main spec at `/openspec/specs//spec.md`, using the same capability path as the delta spec: the requirement under the FROM name, or under the TO name only when the FROM name is absent because the main spec is already synced. Its body and scenarios are the evidence for the behavior the TO requirement keeps. + - Search codebase for that behavior and assess if it is still implemented. + - If it appears unimplemented: + - Add CRITICAL issue: "Renamed requirement not found: " + - Recommendation: "Restore the behavior of (renamed from ); a rename must not change behavior" + - If the baseline requirement cannot be found or read, mark **Spec Coverage** as not verified for that entry with the reason. Never count an unchecked rename as passing. 6. **Verify Correctness** - If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change (each RENAMED entry is checked there against its baseline behavior), so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. **Requirement Implementation Mapping**: - - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): + - For each ADDED or MODIFIED requirement from delta specs (REMOVED entries, and RENAMED entries without a MODIFIED block, were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -164,7 +172,7 @@ In both branches, never create the root as a side effect: do not run `openspec i | Coherence | Followed/Issues | ``` - In each Status cell, report the results of checks that ran and `Not verified ()` for every skipped check. If all checks in a dimension were skipped, start the cell with `Not verified`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads `Not applicable (no ADDED or MODIFIED requirements)`. + In each Status cell, report the results of checks that ran and `Not verified ()` for every skipped check. If all checks in a dimension were skipped, start the cell with `Not verified`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED and RENAMED requirements separately (for example, "1 removal confirmed, 1 rename verified"). For a change that only removes or renames requirements, the Correctness cell reads `Not applicable (no ADDED or MODIFIED requirements)`. **Issues by Priority**: @@ -172,6 +180,7 @@ In both branches, never create the root as a side effect: do not run `openspec i - Incomplete tasks - Missing requirement implementations - Removed requirements still implemented + - Renamed requirements whose behavior is no longer implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): diff --git a/src/core/templates/workflows/verify-change.ts b/src/core/templates/workflows/verify-change.ts index 028e2d0397..544e1da608 100644 --- a/src/core/templates/workflows/verify-change.ts +++ b/src/core/templates/workflows/verify-change.ts @@ -87,7 +87,7 @@ ${PROJECT_ROOT_GUARD} - If status marks the spec artifact skipped by \`skip_specs: true\`, or the schema defines no spec artifact, report the spec-dependent checks as not applicable. - Otherwise, \`contextFiles\` is keyed by artifact id, and artifact ids come from the active schema. If \`contextFiles.specs\` is absent or empty, mark **Spec Coverage**, **Requirement Implementation Mapping**, and **Scenario Coverage** as not verified; do not treat any of them as clean. - If delta specs exist in \`contextFiles.specs\`: - - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. + - Extract all requirements (marked with "### Requirement:", or listed as \`FROM:\`/\`TO:\` pairs under \`## RENAMED Requirements\`) and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists @@ -100,14 +100,22 @@ ${PROJECT_ROOT_GUARD} - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" - - A RENAMED entry (\`FROM:\`/\`TO:\`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. + - For each RENAMED entry (\`FROM:\`/\`TO:\`), the name changes but the behavior stays, so check the TO requirement for that unchanged behavior: + - Do not report the FROM name as missing, and do not require code symbols, identifiers, or file names to be renamed. + - If the TO name also appears under MODIFIED, its behavior is checked there against the MODIFIED text; skip it here. + - Otherwise, read the baseline requirement in the main spec at \`/openspec/specs//spec.md\`, using the same capability path as the delta spec: the requirement under the FROM name, or under the TO name only when the FROM name is absent because the main spec is already synced. Its body and scenarios are the evidence for the behavior the TO requirement keeps. + - Search codebase for that behavior and assess if it is still implemented. + - If it appears unimplemented: + - Add CRITICAL issue: "Renamed requirement not found: " + - Recommendation: "Restore the behavior of (renamed from ); a rename must not change behavior" + - If the baseline requirement cannot be found or read, mark **Spec Coverage** as not verified for that entry with the reason. Never count an unchecked rename as passing. 6. **Verify Correctness** - If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change (each RENAMED entry is checked there against its baseline behavior), so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. **Requirement Implementation Mapping**: - - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): + - For each ADDED or MODIFIED requirement from delta specs (REMOVED entries, and RENAMED entries without a MODIFIED block, were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -158,7 +166,7 @@ ${PROJECT_ROOT_GUARD} | Coherence | Followed/Issues | \`\`\` - In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. + In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED and RENAMED requirements separately (for example, "1 removal confirmed, 1 rename verified"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. **Issues by Priority**: @@ -166,6 +174,7 @@ ${PROJECT_ROOT_GUARD} - Incomplete tasks - Missing requirement implementations - Removed requirements still implemented + - Renamed requirements whose behavior is no longer implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): @@ -288,7 +297,7 @@ ${PROJECT_ROOT_GUARD} - If status marks the spec artifact skipped by \`skip_specs: true\`, or the schema defines no spec artifact, report the spec-dependent checks as not applicable. - Otherwise, \`contextFiles\` is keyed by artifact id, and artifact ids come from the active schema. If \`contextFiles.specs\` is absent or empty, mark **Spec Coverage**, **Requirement Implementation Mapping**, and **Scenario Coverage** as not verified; do not treat any of them as clean. - If delta specs exist in \`contextFiles.specs\`: - - Extract all requirements (marked with "### Requirement:") and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. + - Extract all requirements (marked with "### Requirement:", or listed as \`FROM:\`/\`TO:\` pairs under \`## RENAMED Requirements\`) and note the delta section each one sits under: \`## ADDED\`, \`## MODIFIED\`, \`## REMOVED\`, or \`## RENAMED Requirements\`. The section decides what the check looks for. - For each ADDED or MODIFIED requirement (for MODIFIED, check the text in the delta, not the old wording): - Search codebase for keywords related to the requirement - Assess if implementation likely exists @@ -301,14 +310,22 @@ ${PROJECT_ROOT_GUARD} - If the behavior is still present: - Add CRITICAL issue: "Removed requirement still implemented: " - Recommendation: "Remove the remaining implementation at :, following the requirement's Migration note if it has one" - - A RENAMED entry (\`FROM:\`/\`TO:\`) changes only a name. Do not report the FROM name as missing. If the renamed requirement's behavior also changes, it appears under MODIFIED with its TO name and is checked there. + - For each RENAMED entry (\`FROM:\`/\`TO:\`), the name changes but the behavior stays, so check the TO requirement for that unchanged behavior: + - Do not report the FROM name as missing, and do not require code symbols, identifiers, or file names to be renamed. + - If the TO name also appears under MODIFIED, its behavior is checked there against the MODIFIED text; skip it here. + - Otherwise, read the baseline requirement in the main spec at \`/openspec/specs//spec.md\`, using the same capability path as the delta spec: the requirement under the FROM name, or under the TO name only when the FROM name is absent because the main spec is already synced. Its body and scenarios are the evidence for the behavior the TO requirement keeps. + - Search codebase for that behavior and assess if it is still implemented. + - If it appears unimplemented: + - Add CRITICAL issue: "Renamed requirement not found: " + - Recommendation: "Restore the behavior of (renamed from ); a rename must not change behavior" + - If the baseline requirement cannot be found or read, mark **Spec Coverage** as not verified for that entry with the reason. Never count an unchecked rename as passing. 6. **Verify Correctness** - If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change, so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. + If the delta specs are readable and contain at least one REMOVED or RENAMED requirement but no ADDED or MODIFIED requirements (the change only removes or renames requirements), report **Requirement Implementation Mapping** and **Scenario Coverage** as **Not applicable**. The REMOVED and RENAMED checks under Spec Coverage are the evidence for such a change (each RENAMED entry is checked there against its baseline behavior), so do not mark these two checks as not verified. A delta spec with no parseable requirements at all is unusable evidence, not a removal-only change: mark these checks as not verified. **Requirement Implementation Mapping**: - - For each ADDED or MODIFIED requirement from delta specs (REMOVED and RENAMED entries were settled under Spec Coverage): + - For each ADDED or MODIFIED requirement from delta specs (REMOVED entries, and RENAMED entries without a MODIFIED block, were settled under Spec Coverage): - Search codebase for implementation evidence - If found, note file paths and line ranges - Assess if implementation matches requirement intent @@ -359,7 +376,7 @@ ${PROJECT_ROOT_GUARD} | Coherence | Followed/Issues | \`\`\` - In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately (for example, "1 removal confirmed"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. + In each Status cell, report the results of checks that ran and \`Not verified ()\` for every skipped check. If all checks in a dimension were skipped, start the cell with \`Not verified\`. Never score a skipped check as passing. Treat every not verified or partially verified check as skipped in the final assessment. Count only ADDED and MODIFIED requirements in N, and report REMOVED and RENAMED requirements separately (for example, "1 removal confirmed, 1 rename verified"). For a change that only removes or renames requirements, the Correctness cell reads \`Not applicable (no ADDED or MODIFIED requirements)\`. **Issues by Priority**: @@ -367,6 +384,7 @@ ${PROJECT_ROOT_GUARD} - Incomplete tasks - Missing requirement implementations - Removed requirements still implemented + - Renamed requirements whose behavior is no longer implemented - Each with specific, actionable recommendation 2. **WARNING** (Should fix): diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index 79297fa3e2..80d2670195 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -91,11 +91,11 @@ const EXPECTED_FUNCTION_HASHES: Record = { getArchiveChangeSkillTemplate: '71715f9d5899498942af03e182e6d1ac2c95952dde967950c2a9161084a53a8b', getBulkArchiveChangeSkillTemplate: '2a6ec08fea0f942158b4abe9c8d1af9622038e0c4dc684e7c73dad2fb8379a54', getOpsxSyncCommandTemplate: '60550b7bb9829421656d6324a9e4c951bc912f48f88882d1a07ce7f78397a5e7', - getVerifyChangeSkillTemplate: '3860f97217bfba502531443da7498bb25b97260101fe0d97b56dfbec53a5eb4b', + getVerifyChangeSkillTemplate: 'eecb063792075191b613978dec45f9f2fee247d2ff3003f2ebf17d632e54352e', getOpsxArchiveCommandTemplate: '3d2a330b46043fbb9f220831aa42ebbb62f411e9597b2bb491ad1ac1fa2d0873', getOpsxOnboardCommandTemplate: '0cf66e164c0e14c916c6d1ebb5d80ded07d7fb8e55d4eb34eba43e8ca9c28558', getOpsxBulkArchiveCommandTemplate: '4e2e39c4d634074f4a1ed67f076d5c4d0ead8b998f4d75218c33cdc6173719be', - getOpsxVerifyCommandTemplate: 'dcb2f32b721a0a2e786fca9ddd85c21a6ced5399cb43e40e25d314bc41e2f055', + getOpsxVerifyCommandTemplate: 'f47bc0c30cfa8e93b5e42026e9417636c5f15bd8505fb9138872e34af8906abb', getOpsxProposeSkillTemplate: '1aa2f2eb9c8cbc4dcab9d777bf8832b92ca04f9ef91d0494f1224a566aefdfe8', getOpsxProposeCommandTemplate: '3b7090ce5e79e879ab9b5bdaf4ff2b52e3c02211f71188838772d36ac337f96c', getFeedbackSkillTemplate: 'dabeb5e825b9349abc8156c3e7b8608f27987912a6d9bf47ef29addde6138133', @@ -112,7 +112,7 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-sync-specs': '3909936a236a21a9a6d5bf495f90b396b3b68fc9220d7b2c1894668653beb2e4', 'openspec-archive-change': 'd01d9eeb06223ee89708b7963e82c5ebc11719c5b2dc62d4abb268ee016fcb7b', 'openspec-bulk-archive-change': '10f050ad5ef77084dc55a202427988b23903ee122f00985238a4eb9354a5dc3c', - 'openspec-verify-change': '5d4584b3a7adb64bd22bc2b4e509a58d31972865a9f3f0f625395084e55b4601', + 'openspec-verify-change': '62c2d471a1ebc4be38df0d06393eb94d3d8b803719b6349b8a1d8e9231448275', 'openspec-onboard': '6993eff867d97d485e080078f9dfb80e968e242f3b17a924eeb077715fd548fa', 'openspec-propose': '66e3395adf9f2d93a09e8ef1d20e4efb010e5e8d4811f2d42a9316e4d1ca5a8b', 'openspec-update-change': '5f4ea19aa732b33d87a2120ec393ee34578e70678d97e8c3bb10f988c00cb4d3', diff --git a/test/core/templates/verify-change-delta-operations.test.ts b/test/core/templates/verify-change-delta-operations.test.ts index d14e742353..b0a1a19fe2 100644 --- a/test/core/templates/verify-change-delta-operations.test.ts +++ b/test/core/templates/verify-change-delta-operations.test.ts @@ -1,3 +1,5 @@ +import { readFileSync } from 'node:fs'; + import { describe, expect, it } from 'vitest'; import { @@ -13,6 +15,11 @@ import { const bodies: Array<[string, string]> = [ ['skill', getVerifyChangeSkillTemplate().instructions], ['command', getOpsxVerifyCommandTemplate().content], + // The committed skills.sh mirror is what `npx skills add` installs. + [ + 'committed skill file', + readFileSync(new URL('../../../skills/openspec-verify-change/SKILL.md', import.meta.url), 'utf8'), + ], ]; function section(body: string, start: string, end: string, label: string): string { @@ -27,6 +34,9 @@ describe('verify checks each requirement by its delta operation', () => { it.each(bodies)('%s: classifies requirements by delta section before checking them', (label, body) => { const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + // RENAMED entries carry no "### Requirement:" heading, so a rename-only + // delta must not read as empty. + expect(coverage, label).toContain('or listed as `FROM:`/`TO:` pairs under `## RENAMED Requirements`'); for (const header of ['## ADDED', '## MODIFIED', '## REMOVED', '## RENAMED Requirements']) { expect(coverage, label).toContain(header); } @@ -43,7 +53,7 @@ describe('verify checks each requirement by its delta operation', () => { it.each(bodies)('%s: inverts the check for a REMOVED requirement', (label, body) => { const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); - const removed = section(coverage, '- For each REMOVED requirement', '- A RENAMED entry', label); + const removed = section(coverage, '- For each REMOVED requirement', '- For each RENAMED entry', label); expect(removed, label).toContain('Finding no implementation is the expected result.'); expect(removed, label).toContain('Never report a REMOVED requirement as "Requirement not found"'); @@ -52,9 +62,34 @@ describe('verify checks each requirement by its delta operation', () => { }); it.each(bodies)('%s: does not report the old name of a RENAMED requirement as missing', (label, body) => { - const coverage = section(body, '**Spec Coverage**', '6. **Verify Correctness**', label); + const renamed = section(body, '- For each RENAMED entry', '6. **Verify Correctness**', label); + + expect(renamed, label).toContain('Do not report the FROM name as missing'); + expect(renamed, label).toContain('do not require code symbols, identifiers, or file names to be renamed'); + }); + + // A rename keeps behavior, so a rename-only change must still prove the + // behavior exists before verify can call it ready. Its evidence is the + // baseline requirement in the main spec, not the RENAMED entry itself. + it.each(bodies)('%s: verifies the unchanged behavior of a RENAMED requirement against its baseline', (label, body) => { + const renamed = section(body, '- For each RENAMED entry', '6. **Verify Correctness**', label); + + expect(renamed, label).toContain('check the TO requirement for that unchanged behavior'); + expect(renamed, label).toContain('`/openspec/specs//spec.md`'); + expect(renamed, label).toContain('the requirement under the FROM name, or under the TO name only when the FROM name is absent because the main spec is already synced.'); + expect(renamed, label).toContain('Its body and scenarios are the evidence for the behavior the TO requirement keeps.'); + expect(renamed, label).toContain('Search codebase for that behavior and assess if it is still implemented.'); + expect(renamed, label).toContain('Add CRITICAL issue: "Renamed requirement not found: "'); + expect(body, label).toContain('- Renamed requirements whose behavior is no longer implemented'); + expect(renamed, label).toContain('If the TO name also appears under MODIFIED, its behavior is checked there'); + }); + + it.each(bodies)('%s: never counts an unchecked rename as passing', (label, body) => { + const renamed = section(body, '- For each RENAMED entry', '6. **Verify Correctness**', label); - expect(coverage, label).toContain('Do not report the FROM name as missing.'); + expect(renamed, label).toContain('If the baseline requirement cannot be found or read, mark **Spec Coverage** as not verified for that entry'); + expect(renamed, label).toContain('Never count an unchecked rename as passing.'); + expect(body, label).toContain('(each RENAMED entry is checked there against its baseline behavior)'); }); it.each(bodies)('%s: maps implementation and scenarios only for ADDED or MODIFIED requirements', (label, body) => { @@ -81,14 +116,14 @@ describe('verify checks each requirement by its delta operation', () => { }); it.each(bodies)('%s: does not treat artifacts or replacement code as the removed behavior', (label, body) => { - const removed = section(body, '- For each REMOVED requirement', '- A RENAMED entry', label); + const removed = section(body, '- For each REMOVED requirement', '- For each RENAMED entry', label); expect(removed, label).toContain('Matches in `openspec/` artifacts or docs, or in code that serves only the Migration note or an ADDED requirement, are not evidence by themselves.'); expect(removed, label).toContain('Report any code path that still delivers the removed behavior, including one shared with an ADDED requirement.'); }); it.each(bodies)('%s: counts removals separately from covered requirements', (label, body) => { - expect(body, label).toContain('Count only ADDED and MODIFIED requirements in N, and report REMOVED requirements separately'); + expect(body, label).toContain('Count only ADDED and MODIFIED requirements in N, and report REMOVED and RENAMED requirements separately'); expect(body, label).toContain('the Correctness cell reads `Not applicable (no ADDED or MODIFIED requirements)`'); }); });