Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/verify-removed-requirements.md
Original file line number Diff line number Diff line change
@@ -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.
36 changes: 30 additions & 6 deletions openspec/specs/opsx-verify-skill/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,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/<name>/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
Expand All @@ -72,14 +73,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
Expand All @@ -96,10 +97,33 @@ 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
- **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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- **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
- **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.

Expand Down
34 changes: 27 additions & 7 deletions skills/openspec-verify-change/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -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".

Expand All @@ -93,18 +93,35 @@ 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:")
- For each requirement:
- 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
- If requirements appear unimplemented:
- If ADDED or MODIFIED requirements appear unimplemented:
- Add CRITICAL issue: "Requirement not found: <requirement name>"
- Recommendation: "Implement requirement X: <description>"
- For each REMOVED requirement, the change asks for the behavior to be gone, so invert the check:
- 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: <requirement name>"
- Recommendation: "Remove the remaining implementation at <file>:<lines>, following the requirement's Migration note if it has one"
- 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 `<planningHome.root>/openspec/specs/<capability-path>/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: <TO name>"
- Recommendation: "Restore the behavior of <TO name> (renamed from <FROM name>); 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 (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 requirement from delta specs:
- 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
Expand All @@ -113,12 +130,13 @@ In both branches, never create the root as a side effect: do not run `openspec i
- Recommendation: "Review <file>:<lines> 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: <scenario name>"
- Recommendation: "Add test or implementation for scenario: <description>"
- Skip scenarios under a REMOVED requirement; that behavior is meant to be gone.

7. **Verify Coherence**

Expand Down Expand Up @@ -154,13 +172,15 @@ 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 (<reason>)` 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 (<reason>)` 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**:

1. **CRITICAL** (Must fix before archive):
- 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):
Expand Down
Loading
Loading