diff --git a/openspec/changes/spec-diffs/.openspec.yaml b/openspec/changes/spec-diffs/.openspec.yaml new file mode 100644 index 000000000..859e7bb98 --- /dev/null +++ b/openspec/changes/spec-diffs/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-15 diff --git a/openspec/changes/spec-diffs/design.md b/openspec/changes/spec-diffs/design.md new file mode 100644 index 000000000..247de94a3 --- /dev/null +++ b/openspec/changes/spec-diffs/design.md @@ -0,0 +1,89 @@ +## Context + +`openspec show ` currently displays the raw proposal markdown (text mode) or a parsed JSON with deltas extracted from the proposal's Capabilities section. Delta spec files under `openspec/changes//specs//spec.md` contain full requirement text including unchanged content copied from the base spec at `openspec/specs//spec.md`. + +Reviewers need to see *what changed* without manually diffing files. The `--diff` flag adds this capability to the existing show command. + +The project currently has no diff dependency. chalk is already available for colorized output. + +## Goals / Non-Goals + +**Goals:** +- Let users see per-requirement diffs of delta specs via `openspec show --diff` +- Support both text (colorized) and JSON output modes +- Keep the implementation minimal — no changes to storage format, validation, or the archive workflow + +**Non-Goals:** +- Changing the delta spec format to store diffs instead of full text (future work, approach 2 from proposal) +- Diffing non-spec artifacts (proposal, design, tasks) +- Providing interactive diff navigation or side-by-side views +- Git-aware diffing (this compares files on disk) + +## Decisions + +### 1. Per-requirement diffing, not whole-file + +**Choice:** Diff individual requirement blocks, not entire spec files. + +The delta spec format already categorizes requirements by operation (`## ADDED`, `## MODIFIED`, `## REMOVED`, `## RENAMED`). Only MODIFIED requirements need a diff — the others are self-explanatory: + +- **ADDED** — display the full requirement text (it's all new) +- **REMOVED** — display the removal notice (Reason/Migration already present) +- **RENAMED** — display the FROM:/TO: (already present) +- **MODIFIED** — match by requirement name (`### Requirement: `) against the base spec at `openspec/specs//spec.md`, extract both blocks, and compute a unified diff of those blocks + +**Rationale:** The existing `ChangeParser` already parses delta specs into individual requirements with operations. The `MarkdownParser` already parses base specs into requirement blocks. We match by the `### Requirement:` header text (the same matching the archive step uses). This gives focused, meaningful output without noise from unchanged requirements. + +**Alternative rejected:** Whole-file diff of base spec vs delta spec. This works but shows context from unchanged requirements that were copied verbatim into the delta file, which is exactly the noise the user wants to eliminate. + +### 2. Diff library: `diff` (npm) + +**Choice:** Use the `diff` npm package (MIT, ~40KB, zero dependencies, widely used) for the MODIFIED requirement case. + +**Alternatives considered:** +- **Implement from scratch** — Unified diff is well-specified but subtle (context lines, hunk headers). A library avoids bugs and maintenance burden. +- **Shell out to `diff` command** — Not cross-platform (Windows lacks `diff` by default). Violates the project's cross-platform requirements. + +The `diff` package provides `createPatch()` which generates standard unified diff output from two strings. + +### 3. Requirement block extraction + +**Choice:** Extract raw markdown text for a requirement block from a spec file by: +1. Finding the `### Requirement: ` header line +2. Collecting all lines until the next `###` header at the same or higher level (or EOF) +3. Including the header line itself in the extracted block + +This reuses the section-parsing logic already in `MarkdownParser` but needs a function that returns the raw markdown text (not the parsed `Requirement` object) so the diff is human-readable. + +**Matching:** Requirement names are matched case-insensitively and whitespace-insensitively, consistent with how the archive step matches requirements. When a MODIFIED requirement's name matches a RENAMED entry's TO name in the same spec, the base block is looked up using the RENAMED FROM name instead — mirroring the archive step's behavior where RENAMED is applied before MODIFIED. + +### 4. Integration point: `ChangeCommand.show()` + +**Choice:** Add diff logic to `ChangeCommand.show()` in `src/commands/change.ts`. When `--diff` is set: +- In text mode: iterate delta specs, group output by capability, show each requirement with its operation and (for MODIFIED) the colorized diff +- In JSON mode: add a `diff` field to each MODIFIED delta in the output object + +**Alternative:** A separate `openspec diff` command. Rejected because the diff is about *viewing* a change, which is what `show` does. Adding a flag is more discoverable and consistent. + +### 5. Diff output format + +**Text mode:** Per capability, per requirement: +- Header: capability name and operation +- ADDED/REMOVED/RENAMED: display the requirement text as-is (prefixed with operation label) +- MODIFIED: unified diff of the requirement block, colorized with chalk (green for `+`, red for `-`, dim for headers/context) + +**JSON mode:** `--json --diff` extends the existing `--json` output (same `{ id, title, deltaCount, deltas }` structure). For each MODIFIED delta, a `diff` string field is added containing the raw unified diff text. ADDED/REMOVED/RENAMED deltas are unchanged. This is backwards-compatible: consumers that don't look for `diff` see the same shape they always did. + +### 6. Flag registration + +Add `--diff` to: +- `openspec show` (top-level, passed through as a change-only flag) +- `openspec change show` (direct) + +Add `'diff'` to the `CHANGE_FLAG_KEYS` set in `src/commands/show.ts` so it triggers a warning when used with `--type spec`. + +## Risks / Trade-offs + +- **\[New dependency\]** Adding `diff` increases the dependency footprint slightly. → The package is small, zero-dep, and MIT. Acceptable for the functionality gained. +- **\[Requirement name mismatch\]** If a MODIFIED requirement's `### Requirement:` header doesn't match the base spec (after whitespace normalization), the diff can't find the base block. → Fall back to showing the full MODIFIED text with a warning. Note: `openspec validate` does not currently check this — the mismatch is only caught at archive time by `buildUpdatedSpec()`. The `--diff` warning becomes a useful early signal of the problem. +- **\[Path display on Windows\]** Capability names derived from directory names are platform-safe already. Paths used in diff headers should use forward slashes for readability. → Normalize display paths using `.replace(/\\/g, '/')` for display only; use `path.join()` for all filesystem operations. diff --git a/openspec/changes/spec-diffs/proposal.md b/openspec/changes/spec-diffs/proposal.md new file mode 100644 index 000000000..9113abac0 --- /dev/null +++ b/openspec/changes/spec-diffs/proposal.md @@ -0,0 +1,50 @@ +## Why + +When proposing a change, delta spec files under `openspec/changes//specs/` duplicate large portions of existing specs in `openspec/specs/`. A MODIFIED requirement must include the entire requirement block (all scenarios), making it hard to see what actually changed versus what was copied verbatim. This friction slows review and increases the risk of errors. + +## What Changes + +Add a `--diff` flag to `openspec show` (change type) that renders each delta spec as a unified diff against the corresponding base spec in `openspec/specs/`. This is the smallest viable improvement: it doesn't change the storage format or workflow, just adds a new way to view the deltas. + +### Approaches considered + +Three approaches were evaluated: + +1. **Stop storing delta specs; edit base specs on the branch directly.** This would eliminate duplication entirely but conflicts with the spec-driven workflow where changes are proposed, reviewed, and archived as discrete artifacts before the base specs are updated. Deferred — would require rethinking the change lifecycle. + +2. **Store deltas as diffs instead of full specs.** The `specs//spec.md` files inside a change would contain unified diffs (or a structured delta format) rather than full requirement text. This eliminates duplication at the source but complicates authoring (AI and humans must produce correct diffs), parsing, validation, and the archive/apply step that merges deltas into base specs. Promising for a future change, but high complexity. + +3. **Add `openspec show --diff` to render deltas against base specs.** (Chosen.) Leave the storage format unchanged. When displaying a change, compute the diff on the fly by comparing each delta spec file against its matching base spec. This gives reviewers the view they need with minimal code changes and zero workflow disruption. + +### What this change delivers + +- A `--diff` flag on `openspec show ` (and `openspec change show `) that outputs a human-readable unified diff per delta spec + +**JSON mode** (`--json --diff`): The existing JSON structure (`{ id, title, deltaCount, deltas }`) is preserved. Only deltas with `operation: "MODIFIED"` include a `"diff"` field containing unified-diff text. Deltas with `operation: "ADDED"`, `"REMOVED"`, or `"RENAMED"` do not include a `"diff"` field (it is absent from the object). When a base spec exists but no matching requirement block is found for a MODIFIED delta, that delta includes a `"warning"` field (string) instead of `"diff"`, describing the mismatch. + +**Text mode** (`--diff` without `--json`): The proposal markdown is printed first, followed by a "Specifications Changed (diffs)" section. MODIFIED deltas show colorized unified diffs (additions in green, removals in red); ADDED deltas show the full requirement text as all-additions in green; REMOVED deltas show the requirement name in red; RENAMED deltas show old and new names. When a MODIFIED delta has no base spec (new capability), the diff is rendered as all-additions. When a MODIFIED delta has a base spec but no matching requirement block, a warning is printed with the raw requirement text. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `cli-show`: Add `--diff` flag support for change display, computing unified diffs of delta specs against their base specs + +## Non-goals + +- Changing the delta spec storage format (approach 2 above — future work) +- Changing when or how base specs are updated (approach 1 above — future work) +- Diffing non-spec artifacts (proposal, design, tasks) +- Git-aware diffing (this compares files on disk, not git history) + +## Impact + +- `src/commands/show.ts` — pass `--diff` flag through to change display +- `src/commands/change.ts` — implement diff rendering in `show()` for text and JSON modes +- `src/cli/index.ts` — register `--diff` option on the show and change show commands +- New utility: diff computation between two markdown strings (can use a lightweight diff library or Node built-in) +- `src/commands/spec.ts` or `src/utils/` — helper to resolve base spec path for a given delta spec diff --git a/openspec/changes/spec-diffs/specs/cli-show/spec.md b/openspec/changes/spec-diffs/specs/cli-show/spec.md new file mode 100644 index 000000000..fb350bdee --- /dev/null +++ b/openspec/changes/spec-diffs/specs/cli-show/spec.md @@ -0,0 +1,96 @@ +## MODIFIED Requirements + +### Requirement: Output format options + +The show command SHALL support various output formats consistent with existing commands. + +#### Scenario: JSON output + +- **WHEN** executing `openspec show --json` +- **THEN** output the item in JSON format +- **AND** include parsed metadata and structure +- **AND** maintain format consistency with existing change/spec show commands + +#### Scenario: Flag scoping and delegation + +- **WHEN** showing a change or a spec via the top-level command +- **THEN** accept common flags such as `--json` +- **AND** pass through type-specific flags to the corresponding implementation + - Change-only flags: `--deltas-only` (alias `--requirements-only` deprecated), `--diff` + - Spec-only flags: `--requirements`, `--no-scenarios`, `-r/--requirement` +- **AND** ignore irrelevant flags for the detected type with a warning + +#### Scenario: Change-specific options + +- **WHEN** showing a change with `openspec show --deltas-only` +- **THEN** display only the deltas in JSON format +- **AND** maintain compatibility with existing change show options + +#### Scenario: Spec-specific options + +- **WHEN** showing a spec with `openspec show --requirements` +- **THEN** display only requirements in JSON format +- **AND** support other spec options (--no-scenarios, -r) +- **AND** maintain compatibility with existing spec show options + +#### Scenario: Text mode change display + +- **WHEN** executing `openspec show ` in text mode (no `--json`) +- **THEN** display the proposal markdown text +- **AND** if delta spec files exist under `openspec/changes//specs/`, display each delta spec's full content grouped by capability name + +#### Scenario: Diff output in text mode + +- **WHEN** executing `openspec show --diff` in text mode (no `--json`) +- **THEN** display the proposal markdown text +- **AND** for each delta spec file under `openspec/changes//specs//spec.md`, iterate parsed deltas grouped by capability +- **AND** for ADDED requirements, display the full requirement text with a green "ADDED" label +- **AND** for REMOVED requirements, display the removal notice (Reason/Migration) with a red "REMOVED" label +- **AND** for RENAMED requirements, display the FROM:/TO: with a cyan "RENAMED" label +- **AND** for MODIFIED requirements, extract the matching requirement block from the base spec at `openspec/specs//spec.md` by `### Requirement:` header name, compute a unified diff of the base block vs the delta block, and display it colorized (green for `+` lines, red for `-` lines, dim for context lines and diff headers) +- **AND** when a MODIFIED requirement's name matches a RENAMED entry's TO name in the same spec, the system SHALL look up the base block using the RENAMED entry's FROM name instead +- **AND** if a MODIFIED requirement has no matching base requirement (and no corresponding RENAMED entry), display the full text with a warning + +#### Scenario: Diff output in JSON mode + +- **WHEN** executing `openspec show --json --diff` +- **THEN** the output SHALL use the same JSON structure as `--json` alone (`{ id, title, deltaCount, deltas }`) +- **AND** for each MODIFIED delta, the delta object SHALL include an additional `diff` string field containing the unified diff of the base requirement block vs the delta requirement block +- **AND** when a MODIFIED requirement corresponds to a RENAMED entry, the base block SHALL be looked up using the RENAMED FROM name +- **AND** ADDED, REMOVED, and RENAMED deltas SHALL NOT have a `diff` field +- **AND** if a MODIFIED requirement has no matching base requirement, the delta object SHALL include a `warning` string field instead of `diff` + +#### Scenario: Diff with no delta specs + +- **WHEN** executing `openspec show --diff` and the change has no delta spec files +- **THEN** print a message indicating no delta specs exist for this change +- **AND** exit with code 0 + +#### Scenario: Diff flag on non-change item + +- **WHEN** executing `openspec show --diff` +- **THEN** ignore the `--diff` flag with a warning (flag is not applicable to specs) + +## ADDED Requirements + +### Requirement: Requirement block extraction for diffing + +The system SHALL extract raw markdown text for individual requirement blocks from spec files to support per-requirement diffing. + +#### Scenario: Extract requirement block by name + +- **WHEN** a requirement name is provided and a spec file contains a matching `### Requirement: ` header +- **THEN** the system SHALL return the raw markdown text from the `### Requirement:` header line through all content until the next `###` header at the same or higher level (or end of file) +- **AND** matching SHALL be case-insensitive and whitespace-insensitive + +#### Scenario: Requirement name not found in base spec + +- **WHEN** a MODIFIED delta requirement name does not match any `### Requirement:` header in the base spec +- **THEN** the system SHALL return null for the base block +- **AND** the caller SHALL display the full MODIFIED requirement text with a warning that no base was found + +#### Scenario: Cross-platform path resolution for base specs + +- **WHEN** resolving the base spec path for a given capability +- **THEN** the system SHALL use `path.join()` for filesystem operations to build `openspec/specs//spec.md` +- **AND** display paths SHALL use forward slashes regardless of platform diff --git a/openspec/changes/spec-diffs/tasks.md b/openspec/changes/spec-diffs/tasks.md new file mode 100644 index 000000000..7392648e2 --- /dev/null +++ b/openspec/changes/spec-diffs/tasks.md @@ -0,0 +1,43 @@ +## 1. Add diff dependency + +- [x] 1.1 Install the `diff` npm package: `pnpm add diff` and `pnpm add -D @types/diff` + +## 2. Requirement block extraction + +- [x] 2.1 Create `src/utils/requirement-block.ts` with a function `extractRequirementBlock(specContent: string, requirementName: string): string | null` that finds the `### Requirement: ` header (case/whitespace insensitive match) and returns the raw markdown from that header through all content until the next `###` header at the same or higher level (or EOF). Returns null if no match. +- [x] 2.2 Add unit tests for `extractRequirementBlock`: exact match, case-insensitive match, whitespace-insensitive match, no match returns null, last requirement in file (no following header), requirement inside code fence is not matched + +## 3. Per-requirement diff utility + +- [x] 3.1 In `src/utils/requirement-block.ts`, add function `diffRequirementBlock(baseBlock: string | null, deltaBlock: string, label: string): string` that uses `createPatch` from the `diff` package to produce a unified diff. When `baseBlock` is null, diff against empty string (all additions). +- [x] 3.2 Add unit tests: base exists (expect removals + additions), base is null (all additions), identical blocks (empty/minimal diff) +- [x] 3.3 Add function `buildRenameMap(renames: Array<{ from: string; to: string }>): Map` that returns a map from normalized TO name → normalized FROM name, for use when looking up base blocks for MODIFIED requirements that were also renamed +- [x] 3.4 Add unit tests for `buildRenameMap`: single rename, multiple renames, empty list + +## 4. CLI flag registration + +- [x] 4.1 In `src/cli/index.ts`, add `.option('--diff', 'Show per-requirement diffs for delta specs')` to the `show` command and the `change show` subcommand +- [x] 4.2 In `src/commands/show.ts`, add `'diff'` to the `CHANGE_FLAG_KEYS` set so it warns when used with `--type spec` + +## 5. Text mode diff display + +- [x] 5.1 In `src/commands/change.ts` `show()` method, when `options.diff` is set and `options.json` is not set: use `ChangeParser.parseDeltaSpecs()` to get deltas grouped by capability, then for each delta: display ADDED (green label + full text), REMOVED (red label + removal notice), RENAMED (cyan label + FROM:/TO:); for MODIFIED, read the base spec, extract the matching requirement block, compute the diff, and print colorized output (green for `+` lines, red for `-` lines, dim for headers/context lines) +- [x] 5.2 Build a rename map from the parsed RENAMED entries for the current spec. For MODIFIED requirements whose normalized name matches a RENAMED TO name, look up the base block using the RENAMED FROM name instead of the MODIFIED name +- [x] 5.3 Handle the no-delta-specs case: print a message like "No delta specs found for change ''" and return (exit code 0) +- [x] 5.4 Handle the MODIFIED-no-base-match case: print the full MODIFIED requirement text with a warning that no matching base requirement was found +- [x] 5.5 Add integration test: text mode diff with a change that has one MODIFIED and one ADDED requirement +- [x] 5.6 Add integration test: text mode RENAMED + MODIFIED on the same requirement — shows both the rename label and the body diff, with the base block looked up by the old name +- [x] 5.7 Add integration test: text mode MODIFIED with no matching base requirement — shows warning and full text + +## 6. JSON mode diff output + +- [x] 6.1 In `src/commands/change.ts` `show()` method, when `options.diff` and `options.json` are both set: for each MODIFIED delta, compute the diff (using rename map for base lookup) and add a `diff` string field to the delta object in the JSON output +- [x] 6.2 Add integration test: JSON mode diff output includes `diff` field on MODIFIED deltas only (not on ADDED/REMOVED/RENAMED) +- [x] 6.3 Add integration test: JSON mode RENAMED + MODIFIED — `diff` field on the MODIFIED delta shows changes relative to the old-name base block + +## 7. Cross-platform and CI verification + +- [x] 7.1 Ensure all path operations in new code use `path.join()` or `path.resolve()`; display paths normalize to forward slashes +- [x] 7.2 Ensure unit tests use `path.join()` for expected path values, not hardcoded slash strings +- [x] 7.3 Verify all existing tests pass (`pnpm test`) +- [ ] 7.4 Verify Windows CI passes (no path-separator issues in requirement matching or file discovery) diff --git a/package-lock.json b/package-lock.json index 03dad207d..10ac1c9e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@fission-ai/openspec", - "version": "1.2.0", + "version": "1.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@fission-ai/openspec", - "version": "1.2.0", + "version": "1.3.0", "hasInstallScript": true, "license": "MIT", "dependencies": { @@ -14,6 +14,7 @@ "@inquirer/prompts": "^7.8.0", "chalk": "^5.5.0", "commander": "^14.0.0", + "diff": "^9.0.0", "fast-glob": "^3.3.3", "ora": "^8.2.0", "posthog-node": "^5.20.0", @@ -1803,7 +1804,6 @@ "integrity": "sha512-oH72nZRfDv9lADUBSo104Aq7gPHpQZc4BTx38r9xf9pg5LfP6EzSyH2n7qFmmxRQXh7YlUXODcYsg6PuTDSxGg==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1853,7 +1853,6 @@ "integrity": "sha512-IgSWvLobTDOjnaxAfDTIHaECbkNlAlKv2j5SjpB2v7QHKv1FIfjwMy8FsDbVfDX/KjmCmYICcw7uGaXLhtsLNg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.0", "@typescript-eslint/types": "8.56.0", @@ -2184,7 +2183,6 @@ "integrity": "sha512-hGISOaP18plkzbWEcP/QvtRW1xDXF2+96HbEX6byqQhAUbiS5oH6/9JwW+QsQCIYON2bI6QZBF+2PvOmrRZ9wA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "fflate": "^0.8.2", @@ -2222,7 +2220,6 @@ "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -2578,6 +2575,15 @@ "node": ">=8" } }, + "node_modules/diff": { + "version": "9.0.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-9.0.0.tgz", + "integrity": "sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==", + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/dir-glob": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz", @@ -2689,7 +2695,6 @@ "integrity": "sha512-VmQ+sifHUbI/IcSopBCF/HO3YiHQx/AVd3UVyYL6weuwW+HvON9VYn5l6Zl1WZzPWXPNZrSQpxwkkZ/VuvJZzg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -4453,7 +4458,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -4552,7 +4556,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -4618,7 +4621,6 @@ "integrity": "sha512-w+N7Hifpc3gRjZ63vYBXA56dvvRlNWRczTdmCBBa+CotUzAPf5b7YMdMR/8CQoeYE5LX3W4wj6RYTgonm1b9DA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", @@ -4735,7 +4737,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -4749,7 +4750,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -4929,7 +4929,6 @@ "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.8.2.tgz", "integrity": "sha512-mplynKqc1C2hTVYxd0PU2xQAc22TI1vShAYGksCCfxbn/dFwnHTNi1bvYsBTkhdUNtGIf5xNOg938rrSSYvS9A==", "license": "ISC", - "peer": true, "bin": { "yaml": "bin.mjs" }, diff --git a/package.json b/package.json index 4ccf4305d..48b533eee 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "@inquirer/prompts": "^7.8.0", "chalk": "^5.5.0", "commander": "^14.0.0", + "diff": "^9.0.0", "fast-glob": "^3.3.3", "ora": "^8.2.0", "posthog-node": "^5.20.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a632f8113..d1217777f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -20,6 +20,9 @@ importers: commander: specifier: ^14.0.0 version: 14.0.0 + diff: + specifier: ^9.0.0 + version: 9.0.0 fast-glob: specifier: ^3.3.3 version: 3.3.3 @@ -861,6 +864,10 @@ packages: resolution: {integrity: sha512-reYkTUJAZb9gUuZ2RvVCNhVHdg62RHnJ7WJl8ftMi4diZ6NWlciOzQN88pUhSELEwflJht4oQDv0F0BMlwaYtA==} engines: {node: '>=8'} + diff@9.0.0: + resolution: {integrity: sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==} + engines: {node: '>=0.3.1'} + dir-glob@3.0.1: resolution: {integrity: sha512-WkrWp9GR4KXfKGYzOLmTuGVi1UWFfws377n9cc55/tb6DuqyF6pcQ5AbiHEshaDpY9v6oaSr2XCDidGmMwdzIA==} engines: {node: '>=8'} @@ -2440,6 +2447,8 @@ snapshots: detect-indent@6.1.0: {} + diff@9.0.0: {} + dir-glob@3.0.1: dependencies: path-type: 4.0.0 diff --git a/src/cli/index.ts b/src/cli/index.ts index 8947736f7..8b5cb8335 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -219,6 +219,7 @@ changeCmd .option('--json', 'Output as JSON') .option('--deltas-only', 'Show only deltas (JSON only)') .option('--requirements-only', 'Alias for --deltas-only (deprecated)') + .option('--diff', 'Show per-requirement diffs for delta specs') .option('--no-interactive', 'Disable interactive prompts') .action(async (changeName?: string, options?: { json?: boolean; requirementsOnly?: boolean; deltasOnly?: boolean; noInteractive?: boolean }) => { try { @@ -319,6 +320,7 @@ program // change-only flags .option('--deltas-only', 'Show only deltas (JSON only, change)') .option('--requirements-only', 'Alias for --deltas-only (deprecated, change)') + .option('--diff', 'Show per-requirement diffs for delta specs (change)') // spec-only flags .option('--requirements', 'JSON only: Show only requirements (exclude scenarios)') .option('--no-scenarios', 'JSON only: Exclude scenario content') diff --git a/src/commands/change.ts b/src/commands/change.ts index 051b4697c..3a50e71ec 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -1,17 +1,37 @@ import { promises as fs } from 'fs'; import path from 'path'; +import chalk from 'chalk'; import { JsonConverter } from '../core/converters/json-converter.js'; import { Validator } from '../core/validation/validator.js'; import { ChangeParser } from '../core/parsers/change-parser.js'; import { Change } from '../core/schemas/index.js'; import { isInteractive } from '../utils/interactive.js'; import { getActiveChangeIds } from '../utils/item-discovery.js'; +import { + parseDeltaSpec, + normalizeRequirementName, +} from '../core/parsers/requirement-blocks.js'; +import { + extractRequirementBlock, + diffRequirementBlock, + buildRenameMap, +} from '../utils/requirement-diff.js'; // Constants for better maintainability const ARCHIVE_DIR = 'archive'; const TASK_PATTERN = /^[-*]\s+\[[\sx]\]/i; const COMPLETED_TASK_PATTERN = /^[-*]\s+\[x\]/i; +interface DiffResult { + capability: string; + operation: string; + requirementName: string; + raw: string; + diff?: string; + rename?: { from: string; to: string }; + warning?: string; +} + export class ChangeCommand { private converter: JsonConverter; @@ -24,8 +44,9 @@ export class ChangeCommand { * - Text mode: raw markdown passthrough (no filters) * - JSON mode: minimal object with deltas; --deltas-only returns same object with filtered deltas * Note: --requirements-only is deprecated alias for --deltas-only + * - --diff mode: per-requirement diffs of delta specs against base specs */ - async show(changeName?: string, options?: { json?: boolean; requirementsOnly?: boolean; deltasOnly?: boolean; noInteractive?: boolean }): Promise { + async show(changeName?: string, options?: { json?: boolean; requirementsOnly?: boolean; deltasOnly?: boolean; diff?: boolean; noInteractive?: boolean }): Promise { const changesPath = path.join(process.cwd(), 'openspec', 'changes'); if (!changeName) { @@ -71,21 +92,262 @@ export class ChangeCommand { const id = parsed.name; const deltas = parsed.deltas || []; - if (options.requirementsOnly || options.deltasOnly) { - const output = { id, title, deltaCount: deltas.length, deltas }; - console.log(JSON.stringify(output, null, 2)); - } else { - const output = { - id, - title, - deltaCount: deltas.length, - deltas, - }; - console.log(JSON.stringify(output, null, 2)); + if (options.diff) { + await this.enrichDeltasWithDiffs(deltas, changeName, changesPath); } + + const output = options.requirementsOnly || options.deltasOnly + ? { id, title, deltaCount: deltas.length, deltas } + : { id, title, deltaCount: deltas.length, deltas }; + console.log(JSON.stringify(output, null, 2)); } else { + // Text mode: show proposal, then specs (full content or diffs) const content = await fs.readFile(proposalPath, 'utf-8'); console.log(content); + + if (options?.diff) { + await this.showSpecDiffs(changeName, changesPath); + } else { + await this.showSpecContent(changeName, changesPath); + } + } + } + + /** + * Add `diff` and `warning` fields to MODIFIED deltas by computing per-requirement + * diffs against base specs. Mutates the deltas array in place. + * + * The parsed Delta objects store requirement body text in `description`, not the + * header name. We re-parse the spec files with parseDeltaSpec to get the header + * names and raw blocks, then match by spec + operation order. + */ + private async enrichDeltasWithDiffs(deltas: any[], changeName: string, changesPath: string): Promise { + const changeDir = path.join(changesPath, changeName); + const specsDir = path.join(changeDir, 'specs'); + const mainSpecsDir = path.join(process.cwd(), 'openspec', 'specs'); + + // Group deltas by spec name so we can match them to parsed blocks + const deltasBySpec = new Map(); + for (const delta of deltas) { + if (!delta.spec || delta.operation !== 'MODIFIED') continue; + const list = deltasBySpec.get(delta.spec) ?? []; + list.push(delta); + deltasBySpec.set(delta.spec, list); + } + + for (const [capName, modifiedDeltas] of deltasBySpec) { + const deltaSpecPath = path.join(specsDir, capName, 'spec.md'); + let deltaContent: string; + try { + deltaContent = await fs.readFile(deltaSpecPath, 'utf-8'); + } catch { + continue; + } + + const baseSpecPath = path.join(mainSpecsDir, capName, 'spec.md'); + let baseContent: string | null = null; + try { + baseContent = await fs.readFile(baseSpecPath, 'utf-8'); + } catch { + // No base spec — new capability + } + + const plan = parseDeltaSpec(deltaContent); + const renameMap = buildRenameMap(plan.renamed); + + // Match parsed Delta objects to RequirementBlocks by position order — + // ChangeParser creates one Delta per MODIFIED block in source order + for (let i = 0; i < modifiedDeltas.length && i < plan.modified.length; i++) { + const delta = modifiedDeltas[i]; + const block = plan.modified[i]; + + const normalizedName = normalizeRequirementName(block.name).toLowerCase(); + const oldName = renameMap.get(normalizedName); + const lookupName = oldName ?? block.name; + + if (baseContent) { + const baseBlock = extractRequirementBlock(baseContent, lookupName); + if (baseBlock) { + delta.diff = diffRequirementBlock(baseBlock, block.raw, `${capName}/${block.name}`); + } else { + delta.warning = `No matching base requirement found for "${block.name}" in ${capName}`; + } + } else { + delta.diff = diffRequirementBlock(null, block.raw, `${capName}/${block.name}`); + } + } + } + } + + /** + * Text-mode: show full delta spec content, grouped by capability. + */ + private async showSpecContent(changeName: string, changesPath: string): Promise { + const changeDir = path.join(changesPath, changeName); + const specsDir = path.join(changeDir, 'specs'); + + let specDirs: string[]; + try { + const entries = await fs.readdir(specsDir, { withFileTypes: true }); + specDirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort(); + } catch { + return; + } + + if (specDirs.length === 0) return; + + console.log(); + console.log(chalk.bold('Specifications Changed')); + console.log(); + + for (const capName of specDirs) { + const deltaSpecPath = path.join(specsDir, capName, 'spec.md'); + try { + const content = await fs.readFile(deltaSpecPath, 'utf-8'); + console.log(chalk.bold.underline(capName)); + console.log(); + console.log(content.trimEnd()); + console.log(); + } catch { + continue; + } + } + } + + /** + * Text-mode: show per-requirement diffs of delta specs against base specs. + */ + private async showSpecDiffs(changeName: string, changesPath: string): Promise { + const changeDir = path.join(changesPath, changeName); + const specsDir = path.join(changeDir, 'specs'); + const mainSpecsDir = path.join(process.cwd(), 'openspec', 'specs'); + + let specDirs: string[]; + try { + const entries = await fs.readdir(specsDir, { withFileTypes: true }); + specDirs = entries.filter(e => e.isDirectory()).map(e => e.name).sort(); + } catch { + specDirs = []; + } + + if (specDirs.length === 0) { + return; + } + + console.log(); + console.log(chalk.bold('Specifications Changed (diffs)')); + console.log(); + + const results: DiffResult[] = []; + + for (const capName of specDirs) { + const deltaSpecPath = path.join(specsDir, capName, 'spec.md'); + let deltaContent: string; + try { + deltaContent = await fs.readFile(deltaSpecPath, 'utf-8'); + } catch { + continue; + } + + const baseSpecPath = path.join(mainSpecsDir, capName, 'spec.md'); + let baseContent: string | null = null; + try { + baseContent = await fs.readFile(baseSpecPath, 'utf-8'); + } catch { + // No base spec — new capability + } + + const plan = parseDeltaSpec(deltaContent); + const renameMap = buildRenameMap(plan.renamed); + + for (const block of plan.added) { + results.push({ capability: capName, operation: 'ADDED', requirementName: block.name, raw: block.raw }); + } + + for (const name of plan.removed) { + results.push({ capability: capName, operation: 'REMOVED', requirementName: name, raw: `### Requirement: ${name}` }); + } + + for (const r of plan.renamed) { + results.push({ capability: capName, operation: 'RENAMED', requirementName: r.to, raw: '', rename: r }); + } + + for (const block of plan.modified) { + const entry: DiffResult = { capability: capName, operation: 'MODIFIED', requirementName: block.name, raw: block.raw }; + + const normalizedName = normalizeRequirementName(block.name).toLowerCase(); + const oldName = renameMap.get(normalizedName); + const lookupName = oldName ?? block.name; + + if (baseContent) { + const baseBlock = extractRequirementBlock(baseContent, lookupName); + if (baseBlock) { + entry.diff = diffRequirementBlock(baseBlock, block.raw, `${capName}/${block.name}`); + } else { + entry.warning = `No matching base requirement found for "${block.name}" in ${capName}`; + } + } else { + entry.diff = diffRequirementBlock(null, block.raw, `${capName}/${block.name}`); + } + + results.push(entry); + } + } + + this.printDiffText(results); + } + + private printDiffText(results: DiffResult[]): void { + let currentCap = ''; + + for (const r of results) { + if (r.capability !== currentCap) { + if (currentCap) console.log(); + currentCap = r.capability; + console.log(chalk.bold.underline(currentCap)); + console.log(); + } + + switch (r.operation) { + case 'ADDED': + console.log(chalk.green.bold(` ADDED: ${r.requirementName}`)); + for (const line of r.raw.split('\n')) { + console.log(chalk.green(` ${line}`)); + } + console.log(); + break; + + case 'REMOVED': + console.log(chalk.red.bold(` REMOVED: ${r.requirementName}`)); + console.log(); + break; + + case 'RENAMED': + console.log(chalk.cyan.bold(` RENAMED: ${r.rename?.from} → ${r.rename?.to}`)); + console.log(); + break; + + case 'MODIFIED': + console.log(chalk.yellow.bold(` MODIFIED: ${r.requirementName}`)); + if (r.warning) { + console.log(chalk.yellow(` ⚠ ${r.warning}`)); + for (const line of r.raw.split('\n')) { + console.log(` ${line}`); + } + } else if (r.diff) { + for (const line of r.diff.split('\n')) { + if (line.startsWith('+')) { + console.log(chalk.green(` ${line}`)); + } else if (line.startsWith('-')) { + console.log(chalk.red(` ${line}`)); + } else { + console.log(` ${line}`); + } + } + } + console.log(); + break; + } } } diff --git a/src/commands/show.ts b/src/commands/show.ts index 6413b5951..2d5b9ec07 100644 --- a/src/commands/show.ts +++ b/src/commands/show.ts @@ -7,7 +7,7 @@ import { nearestMatches } from '../utils/match.js'; type ItemType = 'change' | 'spec'; -const CHANGE_FLAG_KEYS = new Set(['deltasOnly', 'requirementsOnly']); +const CHANGE_FLAG_KEYS = new Set(['deltasOnly', 'requirementsOnly', 'diff']); const SPEC_FLAG_KEYS = new Set(['requirements', 'scenarios', 'requirement']); export class ShowCommand { diff --git a/src/utils/requirement-diff.ts b/src/utils/requirement-diff.ts new file mode 100644 index 000000000..163c1ea80 --- /dev/null +++ b/src/utils/requirement-diff.ts @@ -0,0 +1,58 @@ +import { structuredPatch } from 'diff'; +import { + extractRequirementsSection, + normalizeRequirementName, +} from '../core/parsers/requirement-blocks.js'; + +/** + * Extract the raw markdown block for a requirement by name from a spec file. + * + * Delegates to extractRequirementsSection() which already handles code fences, + * section boundaries, and requirement header parsing. We just look up by name. + * + * Returns null if no matching requirement header is found. + */ +export function extractRequirementBlock(specContent: string, requirementName: string): string | null { + const parts = extractRequirementsSection(specContent); + const targetName = normalizeRequirementName(requirementName).toLowerCase(); + + for (const block of parts.bodyBlocks) { + if (normalizeRequirementName(block.name).toLowerCase() === targetName) { + return block.raw; + } + } + + return null; +} + +/** + * Compute a unified diff between a base requirement block and a delta requirement block. + * When baseBlock is null, diffs against empty string (all additions). + * + * Uses structuredPatch to get only the hunk content lines (context/add/remove) + * without any header matter, since the caller provides its own labeling. + */ +export function diffRequirementBlock(baseBlock: string | null, deltaBlock: string, label: string): string { + const base = ensureTrailingNewline(baseBlock ?? ''); + const delta = ensureTrailingNewline(deltaBlock); + const patch = structuredPatch(label, label, base, delta); + + return patch.hunks.flatMap(h => h.lines).join('\n').trimEnd(); +} + +function ensureTrailingNewline(s: string): string { + return s.endsWith('\n') ? s : s + '\n'; +} + +/** + * Build a map from normalized RENAMED TO name -> normalized RENAMED FROM name. + * Used to look up the base block under the old name when a requirement is both + * renamed and modified. + */ +export function buildRenameMap(renames: Array<{ from: string; to: string }>): Map { + const map = new Map(); + for (const r of renames) { + map.set(normalizeRequirementName(r.to).toLowerCase(), normalizeRequirementName(r.from)); + } + return map; +} diff --git a/test/commands/show-diff.test.ts b/test/commands/show-diff.test.ts new file mode 100644 index 000000000..492553762 --- /dev/null +++ b/test/commands/show-diff.test.ts @@ -0,0 +1,349 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { execSync } from 'child_process'; + +describe('openspec show --diff', () => { + const projectRoot = process.cwd(); + const testDir = path.join(projectRoot, 'test-show-diff-tmp'); + const changesDir = path.join(testDir, 'openspec', 'changes'); + const specsDir = path.join(testDir, 'openspec', 'specs'); + const openspecBin = path.join(projectRoot, 'bin', 'openspec.js'); + + beforeEach(async () => { + await fs.mkdir(changesDir, { recursive: true }); + await fs.mkdir(specsDir, { recursive: true }); + + // Base spec: auth capability with one requirement + const baseSpec = [ + '# auth Specification', + '', + '## Purpose', + 'Authentication spec.', + '', + '## Requirements', + '### Requirement: User login', + '', + 'The system SHALL allow users to log in with email and password.', + '', + '#### Scenario: Valid credentials', + '- **WHEN** user provides valid email and password', + '- **THEN** system authenticates the user', + '', + '### Requirement: Session management', + '', + 'The system SHALL manage user sessions.', + '', + '#### Scenario: Session timeout', + '- **WHEN** session is idle for 30 minutes', + '- **THEN** system expires the session', + '', + ].join('\n'); + + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'auth', 'spec.md'), baseSpec, 'utf-8'); + + // Change proposal + const proposal = [ + '## Why', + 'Improve auth.', + '', + '## What Changes', + '- **auth:** Modify login, add MFA', + '', + '## Capabilities', + '### Modified Capabilities', + '- `auth`: Change login requirement, add MFA', + '', + ].join('\n'); + + const changeDir = path.join(changesDir, 'auth-update'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'proposal.md'), proposal, 'utf-8'); + + // Delta spec: one MODIFIED, one ADDED + const deltaSpec = [ + '## MODIFIED Requirements', + '', + '### Requirement: User login', + '', + 'The system SHALL allow users to log in with email, password, or SSO.', + '', + '#### Scenario: Valid credentials', + '- **WHEN** user provides valid email and password', + '- **THEN** system authenticates the user', + '', + '#### Scenario: SSO login', + '- **WHEN** user clicks SSO provider', + '- **THEN** system redirects to SSO flow', + '', + '## ADDED Requirements', + '', + '### Requirement: Multi-factor authentication', + '', + 'The system SHALL support MFA via TOTP.', + '', + '#### Scenario: MFA setup', + '- **WHEN** user enables MFA', + '- **THEN** system generates TOTP secret', + '', + ].join('\n'); + + await fs.mkdir(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + await fs.writeFile(path.join(changeDir, 'specs', 'auth', 'spec.md'), deltaSpec, 'utf-8'); + }); + + afterEach(async () => { + await fs.rm(testDir, { recursive: true, force: true }); + }); + + function run(args: string): string { + const originalCwd = process.cwd(); + try { + process.chdir(testDir); + return execSync(`node ${openspecBin} ${args}`, { + encoding: 'utf-8', + env: { ...process.env, NO_COLOR: '1' }, + }); + } finally { + process.chdir(originalCwd); + } + } + + function runWithStderr(args: string): { stdout: string; stderr: string } { + const originalCwd = process.cwd(); + try { + process.chdir(testDir); + const stdout = execSync(`node ${openspecBin} ${args}`, { + encoding: 'utf-8', + env: { ...process.env, NO_COLOR: '1' }, + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { stdout, stderr: '' }; + } catch (e: any) { + return { stdout: e.stdout?.toString() ?? '', stderr: e.stderr?.toString() ?? '' }; + } finally { + process.chdir(originalCwd); + } + } + + // Task 5.5: text mode diff with MODIFIED and ADDED + it('text mode: shows proposal then MODIFIED diff and ADDED full text', () => { + const output = run('show auth-update --type change --diff'); + + // Proposal should appear first + expect(output).toContain('Improve auth.'); + + // MODIFIED should show unified diff with changes + expect(output).toContain('MODIFIED: User login'); + expect(output).toContain('-The system SHALL allow users to log in with email and password.'); + expect(output).toContain('+The system SHALL allow users to log in with email, password, or SSO.'); + expect(output).toContain('+#### Scenario: SSO login'); + + // ADDED should show full text + expect(output).toContain('ADDED: Multi-factor authentication'); + expect(output).toContain('The system SHALL support MFA via TOTP.'); + }); + + it('text mode: without --diff shows proposal then full spec content', () => { + const output = run('show auth-update --type change'); + + // Proposal + expect(output).toContain('Improve auth.'); + + // Full delta spec content (not diffed) + expect(output).toContain('MODIFIED Requirements'); + expect(output).toContain('ADDED Requirements'); + expect(output).toContain('The system SHALL allow users to log in with email, password, or SSO.'); + expect(output).toContain('The system SHALL support MFA via TOTP.'); + }); + + // Task 5.7: MODIFIED with no matching base + it('text mode: shows warning when MODIFIED has no matching base', async () => { + // Add a delta that references a nonexistent base requirement + const noMatchDelta = [ + '## MODIFIED Requirements', + '', + '### Requirement: Nonexistent base', + '', + 'The system SHALL do something new.', + '', + '#### Scenario: Works', + '- **WHEN** called', + '- **THEN** works', + '', + ].join('\n'); + + const changeDir = path.join(changesDir, 'no-match'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '## Why\nTest.\n\n## What Changes\n- **auth:** Modify\n', + 'utf-8', + ); + await fs.mkdir(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'specs', 'auth', 'spec.md'), + noMatchDelta, + 'utf-8', + ); + + const output = run('show no-match --type change --diff'); + expect(output).toContain('MODIFIED: Nonexistent base'); + expect(output).toContain('No matching base requirement found'); + }); + + // Task 5.3: no delta specs — proposal is still shown, no spec section + it('text mode: shows only proposal when change has no delta specs', async () => { + const changeDir = path.join(changesDir, 'empty-change'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '## Why\nTest reason.\n\n## What Changes\n- nothing\n', + 'utf-8', + ); + + const output = run('show empty-change --type change --diff'); + expect(output).toContain('Test reason.'); + expect(output).not.toContain('Specifications Changed'); + }); + + // Task 6.2: JSON mode includes diff on MODIFIED only + it('JSON mode: includes diff field on MODIFIED, not on ADDED', () => { + const output = run('show auth-update --type change --diff --json'); + const json = JSON.parse(output); + + // --json --diff uses the same top-level structure as --json alone + expect(json.id).toBe('auth-update'); + expect(json.title).toBeDefined(); + expect(json.deltaCount).toBeDefined(); + expect(Array.isArray(json.deltas)).toBe(true); + + const modified = json.deltas.find((d: any) => d.operation === 'MODIFIED'); + expect(modified).toBeDefined(); + expect(modified.diff).toBeDefined(); + expect(modified.diff).toContain('-The system SHALL allow users to log in with email and password.'); + expect(modified.diff).toContain('+The system SHALL allow users to log in with email, password, or SSO.'); + + const added = json.deltas.find((d: any) => d.operation === 'ADDED'); + expect(added).toBeDefined(); + expect(added.diff).toBeUndefined(); + }); + + it('JSON mode: --json --diff is backwards-compatible with --json', () => { + const jsonOnly = JSON.parse(run('show auth-update --type change --json')); + const jsonDiff = JSON.parse(run('show auth-update --type change --json --diff')); + + // Same top-level keys + expect(Object.keys(jsonDiff).sort()).toEqual(Object.keys(jsonOnly).sort()); + expect(jsonDiff.id).toBe(jsonOnly.id); + expect(jsonDiff.title).toBe(jsonOnly.title); + expect(jsonDiff.deltaCount).toBe(jsonOnly.deltaCount); + expect(jsonDiff.deltas.length).toBe(jsonOnly.deltas.length); + + // Each delta has the same base fields + for (let i = 0; i < jsonOnly.deltas.length; i++) { + expect(jsonDiff.deltas[i].spec).toBe(jsonOnly.deltas[i].spec); + expect(jsonDiff.deltas[i].operation).toBe(jsonOnly.deltas[i].operation); + } + }); + + // Task 5.6: RENAMED + MODIFIED with base lookup by old name + it('text mode: RENAMED + MODIFIED looks up base by old name', async () => { + const deltaSpec = [ + '## RENAMED Requirements', + '', + 'FROM: ### Requirement: Session management', + 'TO: ### Requirement: Session lifecycle', + '', + '## MODIFIED Requirements', + '', + '### Requirement: Session lifecycle', + '', + 'The system SHALL manage user sessions with configurable timeout.', + '', + '#### Scenario: Session timeout', + '- **WHEN** session is idle for configurable duration', + '- **THEN** system expires the session', + '', + ].join('\n'); + + const changeDir = path.join(changesDir, 'rename-change'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '## Why\nRename.\n\n## What Changes\n- **auth:** Rename and modify session\n', + 'utf-8', + ); + await fs.mkdir(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'specs', 'auth', 'spec.md'), + deltaSpec, + 'utf-8', + ); + + const output = run('show rename-change --type change --diff'); + + // Proposal shown first + expect(output).toContain('Rename.'); + + // RENAMED should appear + expect(output).toContain('RENAMED: Session management'); + expect(output).toContain('Session lifecycle'); + + // MODIFIED should show diff against the old "Session management" base + expect(output).toContain('MODIFIED: Session lifecycle'); + expect(output).toContain('+The system SHALL manage user sessions with configurable timeout.'); + expect(output).toContain('-The system SHALL manage user sessions.'); + }); + + // Task 6.3: JSON RENAMED + MODIFIED + it('JSON mode: RENAMED + MODIFIED has diff relative to old-name base', async () => { + const deltaSpec = [ + '## RENAMED Requirements', + '', + 'FROM: ### Requirement: Session management', + 'TO: ### Requirement: Session lifecycle', + '', + '## MODIFIED Requirements', + '', + '### Requirement: Session lifecycle', + '', + 'The system SHALL manage user sessions with configurable timeout.', + '', + '#### Scenario: Session timeout', + '- **WHEN** session is idle for configurable duration', + '- **THEN** system expires the session', + '', + ].join('\n'); + + const changeDir = path.join(changesDir, 'rename-json'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '## Why\nRename.\n\n## What Changes\n- **auth:** Rename and modify session\n', + 'utf-8', + ); + await fs.mkdir(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(changeDir, 'specs', 'auth', 'spec.md'), + deltaSpec, + 'utf-8', + ); + + const output = run('show rename-json --type change --diff --json'); + const json = JSON.parse(output); + + expect(json.id).toBe('rename-json'); + + const renamed = json.deltas.find((d: any) => d.operation === 'RENAMED'); + expect(renamed).toBeDefined(); + expect(renamed.diff).toBeUndefined(); + + const modified = json.deltas.find((d: any) => d.operation === 'MODIFIED'); + expect(modified).toBeDefined(); + expect(modified.diff).toBeDefined(); + expect(modified.diff).toContain('-The system SHALL manage user sessions.'); + expect(modified.diff).toContain('+The system SHALL manage user sessions with configurable timeout.'); + }); +}); diff --git a/test/utils/requirement-diff.test.ts b/test/utils/requirement-diff.test.ts new file mode 100644 index 000000000..80a85d5f6 --- /dev/null +++ b/test/utils/requirement-diff.test.ts @@ -0,0 +1,173 @@ +import { describe, it, expect } from 'vitest'; +import { + extractRequirementBlock, + diffRequirementBlock, + buildRenameMap, +} from '../../src/utils/requirement-diff.js'; + +const SAMPLE_SPEC = `# sample Specification + +## Purpose +A test spec. + +## Requirements +### Requirement: Load project config + +The system SHALL read config. + +#### Scenario: Valid config +- **WHEN** config exists +- **THEN** parse it + +### Requirement: Support .yml alias + +The system SHALL accept .yml. + +#### Scenario: yml extension +- **WHEN** config.yml exists +- **THEN** read it + +### Requirement: Enforce size limit + +The system SHALL reject large configs. + +#### Scenario: Over limit +- **WHEN** config > 50KB +- **THEN** reject +`; + +describe('extractRequirementBlock', () => { + it('returns the full block for an exact name match', () => { + const block = extractRequirementBlock(SAMPLE_SPEC, 'Load project config'); + expect(block).not.toBeNull(); + expect(block).toContain('### Requirement: Load project config'); + expect(block).toContain('The system SHALL read config.'); + expect(block).toContain('#### Scenario: Valid config'); + // Should NOT contain the next requirement + expect(block).not.toContain('Support .yml alias'); + }); + + it('matches case-insensitively', () => { + const block = extractRequirementBlock(SAMPLE_SPEC, 'load PROJECT config'); + expect(block).not.toBeNull(); + expect(block).toContain('### Requirement: Load project config'); + }); + + it('matches with extra whitespace', () => { + const block = extractRequirementBlock(SAMPLE_SPEC, ' Load project config '); + expect(block).not.toBeNull(); + expect(block).toContain('### Requirement: Load project config'); + }); + + it('returns null when name does not match', () => { + const block = extractRequirementBlock(SAMPLE_SPEC, 'Nonexistent requirement'); + expect(block).toBeNull(); + }); + + it('extracts the last requirement in a file (no following header)', () => { + const block = extractRequirementBlock(SAMPLE_SPEC, 'Enforce size limit'); + expect(block).not.toBeNull(); + expect(block).toContain('### Requirement: Enforce size limit'); + expect(block).toContain('#### Scenario: Over limit'); + }); + + it('does not match requirement headers outside the Requirements section', () => { + const specWithPreamble = `# Spec + +## Purpose +Test purpose. + +### Requirement: Preamble header + +This is not inside a Requirements section. + +## Requirements + +### Requirement: Real requirement + +The system SHALL do something. + +#### Scenario: Works +- **WHEN** called +- **THEN** works +`; + // The preamble header is outside ## Requirements, so it should not be found + const preambleBlock = extractRequirementBlock(specWithPreamble, 'Preamble header'); + expect(preambleBlock).toBeNull(); + + const realBlock = extractRequirementBlock(specWithPreamble, 'Real requirement'); + expect(realBlock).not.toBeNull(); + expect(realBlock).toContain('The system SHALL do something.'); + }); + + it('handles Windows-style line endings', () => { + const windowsSpec = SAMPLE_SPEC.replace(/\n/g, '\r\n'); + const block = extractRequirementBlock(windowsSpec, 'Load project config'); + expect(block).not.toBeNull(); + expect(block).toContain('### Requirement: Load project config'); + }); +}); + +describe('diffRequirementBlock', () => { + it('produces a diff when base and delta differ', () => { + const base = '### Requirement: Foo\n\nThe system SHALL do A.\n\n#### Scenario: A\n- **WHEN** called\n- **THEN** A'; + const delta = '### Requirement: Foo\n\nThe system SHALL do B.\n\n#### Scenario: A\n- **WHEN** called\n- **THEN** B'; + + const diff = diffRequirementBlock(base, delta, 'test'); + expect(diff).toContain('-The system SHALL do A.'); + expect(diff).toContain('+The system SHALL do B.'); + // Should not contain header lines + expect(diff).not.toContain('Index:'); + expect(diff).not.toContain('---'); + expect(diff).not.toContain('+++'); + expect(diff).not.toContain('@@'); + }); + + it('shows all additions when base is null', () => { + const delta = '### Requirement: New\n\nThe system SHALL exist.\n\n#### Scenario: Exists\n- **WHEN** checked\n- **THEN** exists'; + + const diff = diffRequirementBlock(null, delta, 'test'); + expect(diff).toContain('+### Requirement: New'); + expect(diff).toContain('+The system SHALL exist.'); + expect(diff).not.toContain('No newline'); + }); + + it('preserves leading space on context lines before first change', () => { + const base = '### Requirement: Foo\n\nLine one.\n\nLine two.\n\nLine three.'; + const delta = '### Requirement: Foo\n\nLine one.\n\nLine two changed.\n\nLine three.'; + + const diff = diffRequirementBlock(base, delta, 'test'); + const lines = diff.split('\n'); + const firstNonEmpty = lines.find(l => l.length > 0)!; + expect(firstNonEmpty.startsWith(' ')).toBe(true); + }); + + it('produces empty output for identical blocks', () => { + const content = '### Requirement: Same\n\nThe system SHALL stay.\n\n#### Scenario: Same\n- **WHEN** called\n- **THEN** same'; + + const diff = diffRequirementBlock(content, content, 'test'); + expect(diff).toBe(''); + }); +}); + +describe('buildRenameMap', () => { + it('builds map from single rename', () => { + const map = buildRenameMap([{ from: 'Old name', to: 'New name' }]); + expect(map.get('new name')).toBe('Old name'); + }); + + it('builds map from multiple renames', () => { + const map = buildRenameMap([ + { from: 'Alpha', to: 'Bravo' }, + { from: 'Charlie', to: 'Delta' }, + ]); + expect(map.size).toBe(2); + expect(map.get('bravo')).toBe('Alpha'); + expect(map.get('delta')).toBe('Charlie'); + }); + + it('returns empty map for empty list', () => { + const map = buildRenameMap([]); + expect(map.size).toBe(0); + }); +});