fix(resolution): converge validate, view, and archive onto canonical resolution (#1182, #1202, #1156) - #1280
Conversation
…ion-AI#1182, Fission-AI#1202, Fission-AI#1156) Planning artifacts only (proposal/design/spec deltas/tasks) for a focused bug-fix bundle. Three read/validate paths silently diverge from the canonical logic a sibling command already gets right: - Fission-AI#1182 validate ignores workspace planning homes that status/instructions resolve - Fission-AI#1202 view counts only changes/<name>/tasks.md, ignoring the schema tasks glob - Fission-AI#1156 the SHALL/MUST body-keyword hint fires for deltas but not main specs Fix converges each divergent path onto the canonical one; parity is asserted by test. No new surface, no behavior change to the already-correct paths. Validates --strict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a change bundle with specs, implementation, and tests for validate resolution parity, tracked-task progress resolution, archive gating, and main-spec SHALL/MUST hinting. ChangesResolution Parity Bug-Fix Bundle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openspec/changes/fix-validate-view-resolution-parity/tasks.md (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the validate resolution path.
status.tsusestoPlanningHome(root)+getChangeDir(...), whileresolveCurrentPlanningHomeSync(...)is used ininstructions.ts; update 1.2 to name the exact patternvalidateshould follow so the implementation target is unambiguous.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/fix-validate-view-resolution-parity/tasks.md` at line 6, Clarify the validate resolution flow by specifying the exact planning-home lookup pattern it should use: in `src/commands/validate.ts`, resolve the named item via `resolveCurrentPlanningHomeSync` and `getChangeDir` from `src/core/planning-home.ts` before any repo-discovery fallback, matching the shared resolution approach used by `workflow/status.ts` and aligning the wording with `instructions.ts` so the implementation target is unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openspec/changes/fix-validate-view-resolution-parity/tasks.md`:
- Line 6: Clarify the validate resolution flow by specifying the exact
planning-home lookup pattern it should use: in `src/commands/validate.ts`,
resolve the named item via `resolveCurrentPlanningHomeSync` and `getChangeDir`
from `src/core/planning-home.ts` before any repo-discovery fallback, matching
the shared resolution approach used by `workflow/status.ts` and aligning the
wording with `instructions.ts` so the implementation target is unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 409ffc07-1111-49e4-933d-df06e388608b
📒 Files selected for processing (6)
openspec/changes/fix-validate-view-resolution-parity/.openspec.yamlopenspec/changes/fix-validate-view-resolution-parity/design.mdopenspec/changes/fix-validate-view-resolution-parity/proposal.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-validate/spec.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-view/spec.mdopenspec/changes/fix-validate-view-resolution-parity/tasks.md
…e review Hardened all three bugs after tracing each path to source with parallel verification agents. Material corrections: - Fission-AI#1182: reframed from "workspace planning home resolution" (planning homes are repo-only; the feature is now 'stores', and validate already accepts --store) to the real, reproducible-at-HEAD mechanism: validate's proposal.md membership gate (getActiveChangeIds) vs status/instructions' directory-existence rule (validateChangeExists). Pulled nested specs/<area>/<cap> delta discovery and bulk --all into scope; noted show.ts sibling. - Fission-AI#1202: widened from view-only to the shared helper's real blast radius — also the archive incomplete-task gate (silently archives unfinished glob-tasks changes: data safety) and a 2nd hardcoded copy in change.ts. Pinned apply.tracks as the source, change-dir scope containment, and the no-schema fallback. Added cli-archive delta for the gate. - Fission-AI#1156: the main-spec parser discards the requirement header before Zod runs, so the hint can't be "lifted" — fix needs header recovery (reuse requirement-blocks) + Zod de-dup, and the main-spec message can't be byte-identical to the delta's (no ADDED prefix). Pinned the actionable sentence + single-emission + regression scenarios across all main-spec surfaces. 4 deltas (cli-validate x2, cli-view, cli-archive). Validates --strict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
openspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.md (1)
21-26: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding a scenario for aggregated task counts across multiple glob matches.
The PR summary mentions "aggregate matches," but the spec only implicitly covers this via the glob-tasks scenario. A dedicated scenario would clarify behavior when both a top-level
tasks.mdand nested**/tasks.mdfiles exist, ensuring the gate counts all tasks rather than stopping at the first match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.md` around lines 21 - 26, Add a dedicated scenario in the cli-archive spec to cover aggregated task counts when both a top-level changes/<name>/tasks.md and nested **/tasks.md files exist. Update the archive behavior description around the incomplete-task gate so it clearly states that the gate uses all matched task files rather than stopping at the first match, and reference the existing glob-tasks scenario plus the single top-level tasks file scenario for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@openspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.md`:
- Around line 21-26: Add a dedicated scenario in the cli-archive spec to cover
aggregated task counts when both a top-level changes/<name>/tasks.md and nested
**/tasks.md files exist. Update the archive behavior description around the
incomplete-task gate so it clearly states that the gate uses all matched task
files rather than stopping at the first match, and reference the existing
glob-tasks scenario plus the single top-level tasks file scenario for
consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5850442c-96e9-4809-ac01-ea6e3e647fbe
📒 Files selected for processing (6)
openspec/changes/fix-validate-view-resolution-parity/design.mdopenspec/changes/fix-validate-view-resolution-parity/proposal.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-validate/spec.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-view/spec.mdopenspec/changes/fix-validate-view-resolution-parity/tasks.md
✅ Files skipped from review due to trivial changes (2)
- openspec/changes/fix-validate-view-resolution-parity/tasks.md
- openspec/changes/fix-validate-view-resolution-parity/specs/cli-view/spec.md
🚧 Files skipped from review as they are similar to previous changes (1)
- openspec/changes/fix-validate-view-resolution-parity/proposal.md
…tion Round 2 of bulletproofing: 3 parallel agents reproduced every bug against the built (pre-fix) CLI and traced fix sites. This pass corrected two substantive errors in my own prior spec and closed several gaps. Fission-AI#1202 (two corrections to the prior draft): - apply.tracks is a FILENAME that selects the tracked artifact, NOT a glob; the glob is that artifact's `generates`. status resolves via resolveArtifactOutputs(changeDir, artifact.generates). Fixed all wording. - "view/archive counts equal status" is FALSE: status checks file EXISTENCE, not checkboxes (proven: status calls a 3/5 change isComplete:true). Deleted the two count-parity scenarios; reframed as resolution-mechanism parity (same files). - Added schema-resolution-failure fallback (resolveSchema throws; helper must catch or view/list/archive crash). Added projectRoot param + 6-site wiring. - Empirically PROVEN data-safety bug: archive moved a 3/5 unfinished change into changes/archive/. Fission-AI#1182: - Found a THIRD getActiveChangeIds site (interactive selector, validate.ts:97). - Proven: --all with a lone proposal-less change exits 0 silently. Added exit-code scenarios. Trimmed over-scope: getSpecIds spec-side is NOT a bug; no store-specific scenario needed; noun-form scoped out. Fission-AI#1156: - Refine-relaxation regression resolved: deltas don't use the Zod refine (validate imperatively), so REMOVE it (not relax) once applySpecRules owns both header-only and no-keyword cases. Added RENAMED (out-of-scope), lowercase, and the new no-body-line-valid-today scenarios; pinned exact message + prefix. Still 4 deltas; validates --strict; empirical evidence section added to design. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
openspec/changes/fix-validate-view-resolution-parity/design.md (1)
15-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify
#1182brelationship to#1182in the root causes table.The table lists
#1182and#1182bas separate rows, but the PR objectives and commit messages describe three issues total (#1182,#1202,#1156). The#1182bsuffix appears to denote a secondary symptom (nested delta discovery) of the same#1182validation resolution failure. Consider adding a parenthetical or footnote clarifying that#1182bis a sub-symptom of#1182(nested-layout delta discovery failure), not a fourth independent issue, to prevent readers from expecting a fourth work item in tasks.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/fix-validate-view-resolution-parity/design.md` around lines 15 - 20, Clarify the root-causes table entry for `#1182b` so it is explicitly presented as a sub-symptom of `#1182` rather than a separate independent issue. Update the wording around the `#1182` and `#1182b` rows in design.md to note that the nested-layout delta discovery failure belongs under the broader validate resolution parity problem, using the existing anchors like src/core/validation/validator.ts and src/commands/validate.ts to keep the relationship obvious.openspec/changes/fix-validate-view-resolution-parity/tasks.md (1)
18-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the
apply.tracksvsgeneratesdistinction in task 3.2.The task item correctly captures Decision 4's mechanism, but the inline parenthetical "(NOTE:
apply.tracksis a filename that selects the artifact, NOT a glob — the glob is the artifact'sgenerates)" is critical and easy to miss in a long bullet. Consider pulling this into a separate sub-bullet or callout, as misunderstanding this distinction was explicitly called out in design lines 50-53 as a "correction to an earlier draft" that is "load-bearing."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openspec/changes/fix-validate-view-resolution-parity/tasks.md` around lines 18 - 27, Task 3.2 currently buries a load-bearing distinction between apply.tracks and generates, which can be missed and lead to the wrong matching logic. Rewrite the task text in openspec/changes/fix-validate-view-resolution-parity/tasks.md so the helper in src/utils/task-progress.ts clearly resolves the tracked artifact by schema apply.tracks (or tasks fallback) and then counts checkboxes from artifact.generates, with the distinction pulled into its own sub-bullet or callout for visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@openspec/changes/fix-validate-view-resolution-parity/design.md`:
- Around line 15-20: Clarify the root-causes table entry for `#1182b` so it is
explicitly presented as a sub-symptom of `#1182` rather than a separate
independent issue. Update the wording around the `#1182` and `#1182b` rows in
design.md to note that the nested-layout delta discovery failure belongs under
the broader validate resolution parity problem, using the existing anchors like
src/core/validation/validator.ts and src/commands/validate.ts to keep the
relationship obvious.
In `@openspec/changes/fix-validate-view-resolution-parity/tasks.md`:
- Around line 18-27: Task 3.2 currently buries a load-bearing distinction
between apply.tracks and generates, which can be missed and lead to the wrong
matching logic. Rewrite the task text in
openspec/changes/fix-validate-view-resolution-parity/tasks.md so the helper in
src/utils/task-progress.ts clearly resolves the tracked artifact by schema
apply.tracks (or tasks fallback) and then counts checkboxes from
artifact.generates, with the distinction pulled into its own sub-bullet or
callout for visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d618687-638e-4dfa-bd8c-65c7c7239ce1
📒 Files selected for processing (6)
openspec/changes/fix-validate-view-resolution-parity/design.mdopenspec/changes/fix-validate-view-resolution-parity/proposal.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-validate/spec.mdopenspec/changes/fix-validate-view-resolution-parity/specs/cli-view/spec.mdopenspec/changes/fix-validate-view-resolution-parity/tasks.md
✅ Files skipped from review due to trivial changes (2)
- openspec/changes/fix-validate-view-resolution-parity/proposal.md
- openspec/changes/fix-validate-view-resolution-parity/specs/cli-view/spec.md
🚧 Files skipped from review as they are similar to previous changes (2)
- openspec/changes/fix-validate-view-resolution-parity/specs/cli-archive/spec.md
- openspec/changes/fix-validate-view-resolution-parity/specs/cli-validate/spec.md
… upgrade Fission-AI#312, tier the risk Second adversarial bulletproofing pass (reproduced everything against main): - Add Fission-AI#418 (metadata-before-description): live on the spec path (req.text = "**ID**: ...") but ALREADY fixed on the delta path. The asymmetry is direct evidence for unifying the two extractors. - Upgrade Fission-AI#312 from "regression hazard" to LIVE bug: a fenced code block before the prose line makes req.text = "```bash" on both paths today (distinct from the already-fixed section-count manifestation). - Tier the fixes by risk after auditing the existing test contract (markdown-parser.test.ts, 15 tests green on main): Tier 1 (false-negative fixes Fission-AI#361/Fission-AI#418/Fission-AI#312): only widens what is read; updates one test (:331, which asserts the first-line bug). Fence tests (:106/:139) preserved because skip-and-join keeps SHALL-first bodies. Tier 2 (recognition tightening Fission-AI#498): canonical ### Requirement: only; a deliberate behavior change that updates bare-header tests (:258/:310) and needs a migration note. Flagged for maintainer decision, with a conservative opt-in-lint alternative documented. - Surface the four-column extractor divergence table (capture / metadata / recognition / predicate) and an explicit "Behavior changes and test impact" section with exact test line refs. Proposal passes `openspec validate --strict`. Does not claim Fission-AI#1156 (PR Fission-AI#1280). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed the proposal against the current validate/view/archive paths and codebase notes. This is scoped correctly: it fixes the real parity splits in change discovery, schema-backed task resolution, archive task gating, and main-spec hint emission without turning into the broader sync/check work. Approved, with the key implementation bar being parity tests for direct/bulk/interactive validate, store roots, nested task globs, archive blocking, and single-emission hints.
…n-AI#1182, Fission-AI#1202, Fission-AI#1156) Implements the resolution/validation parity bug bundle planned in openspec/changes/fix-validate-view-resolution-parity. Each fix points a divergent read/validate path at the canonical implementation a sibling command already gets right, with parity tests guarding against re-forking. Fission-AI#1182 — validate resolves changes like status. validate now resolves a change by directory existence (shared getAvailableChanges) instead of requiring proposal.md, at all three sites (targeted, bulk, interactive selector). A scaffolded/still-authoring change is validated rather than reported Unknown item; a resolved-but-invalid change exits non-zero. show.ts and the deprecated noun-form change validate are scoped out. #1182b — validateChangeDeltaSpecs recurses the nested multi-area layout (specs/<area>/<capability>/spec.md) via a new findDeltaSpecFiles walker, so a resolved multi-area change validates its deltas instead of reporting "No delta sections found". Fission-AI#1202 — getTaskProgressForChange resolves task progress through the tracked-tasks artifact's generates glob (the same resolveArtifactOutputs status uses), aggregating checkboxes across every matched tasks.md scoped to the change dir, with a never-throw fallback to a single top-level tasks.md. Updates all four callers (view/list/archive x2) for the new projectRoot arg and folds the second copy in change.ts onto the helper. Fixes view's Draft misclassification and the archive incomplete-task gate that let an unfinished glob-tasks change archive (data safety). Fission-AI#1156 — the SHALL/MUST body-keyword hint applies to main specs. applySpecRules recovers the requirement header via extractRequirementsSection and emits the targeted hint (header-only) or generic message (no keyword), exactly once; the Zod refine is removed (deltas never used it). The actionable sentence is byte-identical to the change-delta path. Adds parity/regression tests (Decision 7): validate<->status resolution incl. exit code, view/archive resolve the same files as status, and the main-spec<->delta actionable-sentence parity. Full suite green (1791 passed; only the pre-existing, environment-specific zsh-installer failures remain). Change validates --strict; all 36 repo specs pass --specs --strict with no new false positives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/core/validation.test.ts (1)
682-695: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider asserting the issue
pathfield uses forward slashes for the nested layout.This test exercises new path-construction logic (
findDeltaSpecFiles+path.relative/toPosixPathinvalidator.ts) for nestedspecs/<area>/<capability>discovery, but only checksreport.valid/absence of message substrings — it never asserts the actualissue.pathvalue. Adding an assertion that the reported path uses/(e.g. via a deliberately broken-requirement fixture so an issue with apathis produced) would guard against a future regression leaking OS-specific separators.As per path instructions: "When touching path behavior, add coverage that would fail on Windows path separators."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/validation.test.ts` around lines 682 - 695, The nested delta discovery test currently verifies validity and message text only, but it does not cover the reported issue path formatting. Update the `Validator.validateChangeDeltaSpecs` / `findDeltaSpecFiles` coverage in this test to produce an issue that includes a `path`, then assert that the `issue.path` uses forward slashes for the nested `specs/<area>/<capability>` layout. This will exercise the `path.relative` and `toPosixPath` behavior in `validator.ts` and protect against OS-specific separator regressions.Source: Path instructions
test/core/archive.test.ts (1)
108-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood regression coverage for the nested-glob
tasks.mdgate; consider also asserting the archive completed.The test confirms the warning message reflects nested-glob task counts, but doesn't assert that
archiveCommand.execute(...)with--yesactually archived the change (i.e., that the gate didn't block it). Adding that assertion would close the loop on verifying--yesbypass behavior alongside correct counting.✅ Suggested addition
expect(console.log).toHaveBeenCalledWith( expect.stringContaining('2 incomplete task(s) found') ); + + const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); + const archives = await fs.readdir(archiveDir); + expect(archives.some(a => a.includes(changeName))).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/archive.test.ts` around lines 108 - 150, The regression test in archiveCommand.execute only checks the nested-glob task warning and does not verify that the archive actually completed when called with { yes: true }. Extend the existing test to assert the change was archived after execution, using the archiveCommand.execute call and the resulting change location/state so the test covers both correct task counting and that --yes does not block archiving.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/commands/validate.test.ts`:
- Around line 170-185: The test fixture in validate.test.ts is creating its
scratch directory inside the repository tree, which can leave behind
test-validate-iso-tmp after interruptions. Update the isolated setup in the
validate --all test to build isoRoot under os.tmpdir() instead of process.cwd(),
and keep using the existing isoRoot, isoChanges, and runCLI flow so cleanup
remains confined to a temp location.
---
Nitpick comments:
In `@test/core/archive.test.ts`:
- Around line 108-150: The regression test in archiveCommand.execute only checks
the nested-glob task warning and does not verify that the archive actually
completed when called with { yes: true }. Extend the existing test to assert the
change was archived after execution, using the archiveCommand.execute call and
the resulting change location/state so the test covers both correct task
counting and that --yes does not block archiving.
In `@test/core/validation.test.ts`:
- Around line 682-695: The nested delta discovery test currently verifies
validity and message text only, but it does not cover the reported issue path
formatting. Update the `Validator.validateChangeDeltaSpecs` /
`findDeltaSpecFiles` coverage in this test to produce an issue that includes a
`path`, then assert that the `issue.path` uses forward slashes for the nested
`specs/<area>/<capability>` layout. This will exercise the `path.relative` and
`toPosixPath` behavior in `validator.ts` and protect against OS-specific
separator regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fe761afc-1e21-4564-9494-f6cc83d479c5
📒 Files selected for processing (15)
.changeset/fix-validate-view-resolution-parity.mdopenspec/changes/fix-validate-view-resolution-parity/tasks.mdsrc/commands/change.tssrc/commands/validate.tssrc/core/archive.tssrc/core/list.tssrc/core/schemas/base.schema.tssrc/core/validation/validator.tssrc/core/view.tssrc/utils/task-progress.tstest/commands/validate.test.tstest/core/archive.test.tstest/core/validation.test.tstest/core/view.test.tstest/utils/task-progress.test.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/fix-validate-view-resolution-parity.md
- openspec/changes/fix-validate-view-resolution-parity/tasks.md
| it('includes a sole proposal-less change in --all (not "No items found") (#1182)', async () => { | ||
| const isoRoot = path.join(projectRoot, 'test-validate-iso-tmp'); | ||
| const isoChanges = path.join(isoRoot, 'openspec', 'changes'); | ||
| const deltaDir = path.join(isoChanges, 'only', 'specs', 'alpha'); | ||
| await fs.mkdir(deltaDir, { recursive: true }); | ||
| try { | ||
| await fs.writeFile(path.join(isoChanges, 'only', '.openspec.yaml'), 'schema: spec-driven\n', 'utf-8'); | ||
| await fs.writeFile(path.join(deltaDir, 'spec.md'), validDelta, 'utf-8'); | ||
|
|
||
| const result = await runCLI(['validate', '--all'], { cwd: isoRoot }); | ||
| expect(result.stdout + result.stderr).not.toContain('No items found to validate'); | ||
| expect(result.exitCode).toBe(0); | ||
| } finally { | ||
| await fs.rm(isoRoot, { recursive: true, force: true }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '1,40p' test/commands/validate.test.ts | rg -n "projectRoot"
rg -n "export const projectRoot|projectRoot =" test/helpers/run-cli.ts test/commands/validate.test.tsRepository: Fission-AI/OpenSpec
Length of output: 422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== validate.test.ts context =="
sed -n '1,220p' test/commands/validate.test.ts | cat -n
echo
echo "== temp-dir patterns in tests =="
rg -n "os\.tmpdir\(\)|test-validate-iso-tmp|test-validate-command-tmp|path\.join\(projectRoot" test -g '*.test.ts'
echo
echo "== run-cli helper context =="
sed -n '1,120p' test/helpers/run-cli.ts | cat -nRepository: Fission-AI/OpenSpec
Length of output: 27352
Move this scratch directory under os.tmpdir()
isoRoot currently lives under process.cwd(), so an interrupted test run can leave test-validate-iso-tmp behind in the repo tree. Use a temp directory outside the checkout for this fixture.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 175-175: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(path.join(isoChanges, 'only', '.openspec.yaml'), 'schema: spec-driven\n', 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 176-176: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(path.join(deltaDir, 'spec.md'), validDelta, 'utf-8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/commands/validate.test.ts` around lines 170 - 185, The test fixture in
validate.test.ts is creating its scratch directory inside the repository tree,
which can leave behind test-validate-iso-tmp after interruptions. Update the
isolated setup in the validate --all test to build isoRoot under os.tmpdir()
instead of process.cwd(), and keep using the existing isoRoot, isoChanges, and
runCLI flow so cleanup remains confined to a temp location.
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-reviewed Clay's latest push at 6a5579c against the validate/view/archive parity paths and ran the focused local regression set: validate, task-progress, archive, and validation all pass. The remaining CodeRabbit notes are useful test/documentation polish, but not blockers for this fix.
|
A quick unrelated note after going through a couple of these PR reviews: I think we actually need a step, possibly before the proposal reviews, where we align on whether something is an issue and deserves a proposal to be made. The proposals are quite heavy in themselves. I think a lot of this stuff can be caught early. I'm finding that I'm trying to figure out the "why" behind some of this, and it is not as transparent as it could be. There is a lot of Markdown to go through, read, and understand. I think this can hopefully be figured out at an even earlier step. I am just leaving this as a note for us to discuss later. |
…n-AI#1281) * docs(openspec): propose spec parser reading fidelity (fixes Fission-AI#361, Fission-AI#498, Fission-AI#312) The requirement-parsing layer silently misreads valid Markdown: - Fission-AI#361: requirement-body extraction returns only the first non-blank line, so a SHALL/MUST that wraps onto line 2 fails `validate --strict`. - Fission-AI#498: `validate` (delta-block parser) and `archive` (full-spec parser) recognize requirements by different rules, so a stray `###` header passes validate but becomes a phantom requirement that blocks archive. - Fission-AI#312 (residual): the requirement-body loop breaks on any `#` line without consulting the code-fence mask, truncating bodies that contain fenced code with `#` comments. Proposal: one shared, multi-line, fence-aware requirement-body extractor used by both the validator and the markdown parser; recognize only `### Requirement:`-prefixed level-3 headers; guarantee validate/archive parity. Adds regression + parity tests. Fission-AI#559 investigated and deferred (ambiguous root cause — see design.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(openspec): bulletproof parser-fidelity proposal with empirical evidence Hardened the proposal after reproducing every claim against main with the bundled CLI and correcting two inaccuracies: - Fission-AI#498 reframed: archive does NOT hard-fail. validate passes; archive emits NON-BLOCKING phantom "Proposal warnings in proposal.md" because validateChange/parseRequirements counts every level-3 header as a requirement, while the delta-block parser (validate) and specs-apply (rebuild) only recognize canonical `### Requirement:`. It is a consistency bug, not data loss. Verified the rebuilt spec is clean. - Fission-AI#312 reframed: the original repro is already fixed by codeFenceLineMask (requirement count verified correct). The residual is a regression hazard: the body loop is fence-unaware, harmless only while first-line-only, so the multi-line fix must be fence-aware from the start. Also: unify recognition on the canonical REQUIREMENT_HEADER_REGEX (/^###\s*Requirement:\s*(.+)$/i, case-insensitive); surfaced a third latent inconsistency (Zod substring includes('SHALL') vs delta word-boundary \b(SHALL|MUST)\b) and added a single-predicate requirement; verified zero non-Requirement level-3 headers in repo specs (CI-safe); added edge-case scenarios (multi-line spec+delta paths, fenced scenario-looking lines, REMOVED/RENAMED unaffected, display vs detection); replaced broken relative links with plain paths. Proposal passes `openspec validate --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(openspec): deepen parser-fidelity proposal — add Fission-AI#418, upgrade Fission-AI#312, tier the risk Second adversarial bulletproofing pass (reproduced everything against main): - Add Fission-AI#418 (metadata-before-description): live on the spec path (req.text = "**ID**: ...") but ALREADY fixed on the delta path. The asymmetry is direct evidence for unifying the two extractors. - Upgrade Fission-AI#312 from "regression hazard" to LIVE bug: a fenced code block before the prose line makes req.text = "```bash" on both paths today (distinct from the already-fixed section-count manifestation). - Tier the fixes by risk after auditing the existing test contract (markdown-parser.test.ts, 15 tests green on main): Tier 1 (false-negative fixes Fission-AI#361/Fission-AI#418/Fission-AI#312): only widens what is read; updates one test (:331, which asserts the first-line bug). Fence tests (:106/:139) preserved because skip-and-join keeps SHALL-first bodies. Tier 2 (recognition tightening Fission-AI#498): canonical ### Requirement: only; a deliberate behavior change that updates bare-header tests (:258/:310) and needs a migration note. Flagged for maintainer decision, with a conservative opt-in-lint alternative documented. - Surface the four-column extractor divergence table (capture / metadata / recognition / predicate) and an explicit "Behavior changes and test impact" section with exact test line refs. Proposal passes `openspec validate --strict`. Does not claim Fission-AI#1156 (PR Fission-AI#1280). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(openspec): third pass — reject recognition tightening, add fenced-scenario bug, Fission-AI#498→safe INFO Third deep pass found the prior Tier 2 (recognition tightening to `### Requirement:`) was the WRONG fix and over-scoped: - Bare `### <statement>` headers are a SUPPORTED, tested requirement format: test/core/validation.test.ts asserts a bare-header spec is valid, and bare headers appear across json-converter/archive/spec tests and tmp-init fixtures. Tightening would break a large test surface and silently drop requirements from real specs. REJECTED, with evidence documented. - Replace the Fission-AI#498 fix with a SAFE INFO note in validate <change> that surfaces non-`### Requirement:` headers in delta sections. INFO never fails validation (strict: valid = no errors && no warnings), so nothing newly fails. - New bug found and folded in: countScenarios is fence-unaware, so a `#### Scenario:` inside a fenced block is counted as real — a malformed delta passes validate <change> while validate <spec> correctly fails. Same fence family. - Proved the archive WRITE path is independent of the reader: specs-apply rebuilds from raw `### Requirement:` blocks (extractRequirementsSection + RequirementBlock.raw), never parseSpec/req.text → Part A cannot change archived content. Net effect: recognition is unchanged, so the proposal now updates exactly ONE existing test (:331, the first-line assertion) instead of breaking bare-header tests. Consolidated to a single cli-validate delta (dropped cli-archive and openspec-conventions deltas). Dropped the no-space-header hypothesis (no divergence). Passes `openspec validate --strict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(parser): unify the requirement reader, fence/metadata/multi-line aware (Fission-AI#361, Fission-AI#418, Fission-AI#312); surface Fission-AI#498 The requirement reader was implemented twice — MarkdownParser.parseRequirements (validate <spec>/archive) and Validator.extractRequirementText/countScenarios (validate <change>) — and the two had drifted. Both now delegate to one shared, fence-/metadata-/multi-line-aware extraction in parsers/requirement-text.ts so they cannot diverge again. Part A — unify the reader: - Capture the full requirement body up to the first non-fenced `#### Scenario:`, skipping blank, `**metadata**:`, and fenced-code lines; run SHALL/MUST detection over the whole body. Fixes a wrapped keyword being dropped (Fission-AI#361), metadata before the description failing validate <spec> (Fission-AI#418), and a fenced block before the prose line becoming the requirement text (Fission-AI#312). - Count only non-fenced `#### ` headers, so a `#### Scenario:` inside a fenced example no longer counts as a real scenario in validate <change> (parity with validate <spec>). - One whole-word `\b(SHALL|MUST)\b` predicate (containsShallOrMust) shared by the validator and base.schema, replacing the substring/word-boundary split. - Extract buildCodeFenceMask into the shared module; MarkdownParser and ChangeParser import it (single fence implementation). Part B — surface Fission-AI#498 safely: - validate <change> emits an INFO note when an ADDED/MODIFIED Requirements section contains a non-`### Requirement:` level-3 header (one the delta reader silently skips). INFO never changes the valid result, including under --strict, so nothing newly fails. Recognition is unchanged: bare `### <statement>` headers remain a supported requirement format. Write path is unaffected: specs-apply rebuilds from raw `### Requirement:` blocks, never req.text, so archived content cannot change. Displayed text in JSON output and delta descriptions now reflects the full body. Tests: markdown-parser.test.ts:331 updated to expect the full body; regression tests added for Fission-AI#361/Fission-AI#418/Fission-AI#312, the fenced scenario, the Fission-AI#498 INFO note, a single-line guard, and CRLF. Changeset added (patch). tasks.md completed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(parser): add cross-reader predicate + metadata-only guards (design edge cases) Exhaustive verification of the unified reader surfaced two design "edge cases for tests" not yet covered by committed unit tests: - Cross-reader predicate agreement: a SHALL substring inside a word ("MARSHALL") is rejected identically by validate <change> and validate <spec> — proving the one shared whole-word predicate, and guarding against a regression to the old substring check. - Metadata-only body still fails validation (no requirement text) on the delta path. Behavior unchanged; tests only. Full end-to-end parity across all four spec requirements confirmed against the real Validator; no spurious INFO note fires on any existing repo change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(parser): address review — metadata-only bodies, header-bounded extraction, reader-derived INFO - Skip **metadata**: lines only when other body text remains; a body written entirely as metadata (e.g. `**Constraint**: The system MUST ...`) is kept as the requirement text instead of being emptied (was a regression vs main). - Move the empty-body rule into the shared reader: both paths fall back to the header title, so the same block cannot pass one path and fail the other. - End body extraction at any non-fenced markdown header, restoring old-reader parity: a stray `### Background` divider's notes no longer satisfy the SHALL/MUST check. - Replace the standalone fence-aware INFO scanner with skipped-header collection inside parseDeltaSpec, so the note reflects exactly what the reader skipped (same section boundaries, no whole-file fence mask). - Special-case the nameless `### Requirement:` INFO message; document that the any-#### scenario match is deliberate spec-path parity; un-export REQUIREMENT_HEADER_REGEX; move the import up top. - Soften the changeset claim and list the known remaining divergences in design.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(openspec): record the no-space ###Requirement: divergence as a known leftover Jun's edge (reproduced): the delta/write reader's REQUIREMENT_HEADER_REGEX accepts `###Requirement:` with no space, but MarkdownParser.parseSections requires whitespace (per GFM) — so a no-space requirement validates as a change with zero INFO, syncs as-is, then fails validate <spec>. Pre-existing on main and out of scope here (tightening the shared regex would change write-path recognition); documented under known remaining divergences with the follow-up options, folded together with the bullet from the merge resolution. Corrects c63913b's 'no divergence' note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: TabishB <tabishbidiwale@gmail.com>
* fix(resolution): converge validate, view, and archive onto canonical resolution (#1182, #1202, #1156) (#1280)
* docs(openspec): propose resolution/validation parity bug bundle (#1182, #1202, #1156)
Planning artifacts only (proposal/design/spec deltas/tasks) for a focused
bug-fix bundle. Three read/validate paths silently diverge from the canonical
logic a sibling command already gets right:
- #1182 validate ignores workspace planning homes that status/instructions resolve
- #1202 view counts only changes/<name>/tasks.md, ignoring the schema tasks glob
- #1156 the SHALL/MUST body-keyword hint fires for deltas but not main specs
Fix converges each divergent path onto the canonical one; parity is asserted by
test. No new surface, no behavior change to the already-correct paths. Validates
--strict.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): bulletproof the parity bundle after adversarial source review
Hardened all three bugs after tracing each path to source with parallel
verification agents. Material corrections:
- #1182: reframed from "workspace planning home resolution" (planning homes are
repo-only; the feature is now 'stores', and validate already accepts --store)
to the real, reproducible-at-HEAD mechanism: validate's proposal.md membership
gate (getActiveChangeIds) vs status/instructions' directory-existence rule
(validateChangeExists). Pulled nested specs/<area>/<cap> delta discovery and
bulk --all into scope; noted show.ts sibling.
- #1202: widened from view-only to the shared helper's real blast radius — also
the archive incomplete-task gate (silently archives unfinished glob-tasks
changes: data safety) and a 2nd hardcoded copy in change.ts. Pinned apply.tracks
as the source, change-dir scope containment, and the no-schema fallback. Added
cli-archive delta for the gate.
- #1156: the main-spec parser discards the requirement header before Zod runs, so
the hint can't be "lifted" — fix needs header recovery (reuse requirement-blocks)
+ Zod de-dup, and the main-spec message can't be byte-identical to the delta's
(no ADDED prefix). Pinned the actionable sentence + single-emission + regression
scenarios across all main-spec surfaces.
4 deltas (cli-validate x2, cli-view, cli-archive). Validates --strict.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): deep-harden the parity bundle with empirical reproduction
Round 2 of bulletproofing: 3 parallel agents reproduced every bug against the
built (pre-fix) CLI and traced fix sites. This pass corrected two substantive
errors in my own prior spec and closed several gaps.
#1202 (two corrections to the prior draft):
- apply.tracks is a FILENAME that selects the tracked artifact, NOT a glob; the
glob is that artifact's `generates`. status resolves via
resolveArtifactOutputs(changeDir, artifact.generates). Fixed all wording.
- "view/archive counts equal status" is FALSE: status checks file EXISTENCE, not
checkboxes (proven: status calls a 3/5 change isComplete:true). Deleted the two
count-parity scenarios; reframed as resolution-mechanism parity (same files).
- Added schema-resolution-failure fallback (resolveSchema throws; helper must
catch or view/list/archive crash). Added projectRoot param + 6-site wiring.
- Empirically PROVEN data-safety bug: archive moved a 3/5 unfinished change into
changes/archive/.
#1182:
- Found a THIRD getActiveChangeIds site (interactive selector, validate.ts:97).
- Proven: --all with a lone proposal-less change exits 0 silently. Added
exit-code scenarios. Trimmed over-scope: getSpecIds spec-side is NOT a bug;
no store-specific scenario needed; noun-form scoped out.
#1156:
- Refine-relaxation regression resolved: deltas don't use the Zod refine
(validate imperatively), so REMOVE it (not relax) once applySpecRules owns both
header-only and no-keyword cases. Added RENAMED (out-of-scope), lowercase, and
the new no-body-line-valid-today scenarios; pinned exact message + prefix.
Still 4 deltas; validates --strict; empirical evidence section added to design.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: converge validate/view/archive onto canonical resolution (#1182, #1202, #1156)
Implements the resolution/validation parity bug bundle planned in
openspec/changes/fix-validate-view-resolution-parity. Each fix points a
divergent read/validate path at the canonical implementation a sibling
command already gets right, with parity tests guarding against re-forking.
#1182 — validate resolves changes like status. validate now resolves a
change by directory existence (shared getAvailableChanges) instead of
requiring proposal.md, at all three sites (targeted, bulk, interactive
selector). A scaffolded/still-authoring change is validated rather than
reported Unknown item; a resolved-but-invalid change exits non-zero.
show.ts and the deprecated noun-form change validate are scoped out.
#1182b — validateChangeDeltaSpecs recurses the nested multi-area layout
(specs/<area>/<capability>/spec.md) via a new findDeltaSpecFiles walker,
so a resolved multi-area change validates its deltas instead of reporting
"No delta sections found".
#1202 — getTaskProgressForChange resolves task progress through the
tracked-tasks artifact's generates glob (the same resolveArtifactOutputs
status uses), aggregating checkboxes across every matched tasks.md scoped
to the change dir, with a never-throw fallback to a single top-level
tasks.md. Updates all four callers (view/list/archive x2) for the new
projectRoot arg and folds the second copy in change.ts onto the helper.
Fixes view's Draft misclassification and the archive incomplete-task
gate that let an unfinished glob-tasks change archive (data safety).
#1156 — the SHALL/MUST body-keyword hint applies to main specs.
applySpecRules recovers the requirement header via extractRequirementsSection
and emits the targeted hint (header-only) or generic message (no keyword),
exactly once; the Zod refine is removed (deltas never used it). The
actionable sentence is byte-identical to the change-delta path.
Adds parity/regression tests (Decision 7): validate<->status resolution
incl. exit code, view/archive resolve the same files as status, and the
main-spec<->delta actionable-sentence parity. Full suite green (1791
passed; only the pre-existing, environment-specific zsh-installer
failures remain). Change validates --strict; all 36 repo specs pass
--specs --strict with no new false positives.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: add cloudflare documentation deployment website (#1285)
* docs(website): add Fumadocs documentation site for Cloudflare Pages
Add a self-contained marketing + documentation site under website/, built
with Fumadocs (Next.js) and configured as a static export so it deploys
directly to Cloudflare Pages with no server runtime.
What's included:
- A marketing landing page (hero, the two-folder model, the four core
ideas, the explore→propose→apply→archive loop, and the "why").
- 13 documentation pages rewritten for clarity and delight: introduction,
installation, getting started, how commands work, core concepts, the
workflow, explore first, existing projects, editing a change,
customization, FAQ, and a reference section (slash commands, CLI,
supported tools).
- Static client-side search (Orama), per-page Open Graph images, and
llms.txt / llms-full.txt routes — fitting for an AI-native tool.
- website/README.md with one-table Cloudflare Pages deploy settings
(root: website, build: npm run build, output: out).
Content is faithful to the docs/ overhaul from #1237, restated in a
simpler, friendlier voice. Verified with a clean `next build` (48 static
pages, no warnings).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): sharpen the sell, add a Stores guide
Completes the documentation work begun in #1237 by tightening the
Fumadocs site toward the quality bar of the stores user-guide:
- Intro now opens problem-first ("the requirements lived only in chat"),
adds an honest "How it compares" table (Spec Kit / Kiro / nothing), and
frames the tradeoff in a "When the ceremony isn't worth it" callout.
- New Stores guide (beta) distilled from docs/stores-beta/user-guide.md:
the problem, the annotated shape, a five-minute walkthrough with real
command output, a role-based story, the root-resolution order, and an
honest-limitations section. Linked from Existing Projects.
Verified with a clean `next build` (51 static pages, no warnings).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): make the value tangible — landing sections + Examples page
Continue the #1237 docs completion with a stronger product story:
- Landing page now reads like a real product site:
- "Works with the tools you already use" strip (15 named assistants + more)
- "What a change actually looks like" — three real artifacts
(proposal.md, a spec delta, tasks.md) so the workflow is concrete
- "The honest middle" comparison block (Spec Kit / Kiro / no specs)
- Robust hero gradient via color-mix instead of v3 theme() syntax
- New Examples & Recipes page: seven copy-pasteable, narrated walkthroughs
(small feature, bug fix, explore-first, parallel changes, no-behavior
refactor with --skip-specs, step-by-step, onboard). Linked from the intro
and getting-started.
Verified: clean `next build` (54 static pages, no warnings); Tailwind
opacity/color-mix utilities confirmed in the generated CSS; all internal
links resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): add favicon, sitemap, and robots for a complete public site
- Branded SVG favicon (app/icon.svg) in the OpenSpec indigo.
- Static sitemap.xml covering the home page and every doc, built from the
content source and NEXT_PUBLIC_SITE_URL.
- robots.txt allowing all and pointing at the sitemap.
All three are emitted by the static export. Clean `next build`, 57 pages.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: lead with stores as "why teams adopt OpenSpec"; complete docs coverage
Final pass completing the #1237 documentation work.
Reposition stores (beta) as the team adoption story, consistently:
- README.md gains a prominent "Why teams adopt OpenSpec" section right
after the demo (cross-repo features, shared requirements, plan before
code), leading with stores.
- Landing page gains a matching "Why teams adopt OpenSpec" section.
- Docs intro gains a teams card + callout pointing at stores.
- Stores page expanded with full References and Worksets technical
examples (the cross-team requirements story, workset create/open).
Incorporate the remaining source-doc knowledge so the site is complete:
- New pages: Glossary, Troubleshooting, Multi-Language, and an
Agents & Automation reference (the machine-readable --json surfaces and
workflow primitives that make OpenSpec AI-native).
- The Workflow page now covers ff-vs-continue, a three-dimension verify
example, and the update-vs-start-fresh decision guide.
- Nav restructured with a Help section; reference section gains Agents.
Build hardening: `build` now runs `fumadocs-mdx && next build` so the
content source is always regenerated. Clean build: 69 static pages, no
warnings; all internal links verified.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): fix docs GitHub source links + address review nits
- page.tsx: prefix ViewOptionsPopover githubUrl with website/ so the
"view/edit source" links resolve to website/content/docs/... instead
of 404-ing on every deployed docs page (Alfred blocker).
- installation.mdx: note that `yarn global add` is Classic Yarn only and
point Yarn Berry users at `yarn dlx` / npm / pnpm.
- index.mdx: label the comparison table's first column ("Option").
- (home)/page.tsx: use the shared docsRoute constant for all /docs links
instead of hardcoded paths.
Verified with `npm run build` in website/ — 69 static pages, and the
built getting-started page links to blob/main/website/content/docs/...
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): mirror docs/*.md into the site + auto-deploy on a cadence
Make the repository's docs/*.md the single source of truth for the docs
site instead of maintaining a parallel set of hand-written MDX pages that
silently drift.
- scripts/sync-docs.mjs mirrors ../docs into content/docs/ on every build:
derives title/description, injects Fumadocs frontmatter (+ githubSource),
rewrites internal *.md links to /docs routes, and emits meta.json. Pages
are written as .md so <placeholders>/{braces} in the docs stay literal
and never break the MDX build.
- docs.sync.config.mjs is the one manifest deciding which docs publish and
their slug/section/icon. content/docs/ is now generated + git-ignored;
the curated .mdx pages are removed. The marketing landing page stays
hand-authored.
- build/dev/types:check run sync:docs first, so the site is always current.
- .github/workflows/deploy-docs.yml rebuilds and deploys to Cloudflare
Pages via Wrangler on push to docs/**|website/**, daily on a schedule,
on demand, and as a build-only check on PRs. Needs CLOUDFLARE_API_TOKEN
+ CLOUDFLARE_ACCOUNT_ID secrets and the DOCS_SITE_URL variable.
- source.config.ts carries githubSource so "edit this page" opens the real
docs/*.md; website/README.md documents the pipeline.
Verified: clean build, 23 pages generated, 78 static pages, no warnings;
all internal doc links resolve; MDX-hazard docs (cli, customization) build.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(website): fall back to default site URL when NEXT_PUBLIC_SITE_URL is empty
The deploy workflow passes NEXT_PUBLIC_SITE_URL from the DOCS_SITE_URL repo
variable, which resolves to an empty string when unset. `?? fallback` does
not catch '' (only null/undefined), so `metadataBase: new URL('')` crashed
`next build` with ERR_INVALID_URL while collecting page data. Use `||` so an
empty value also falls back. Verified: `NEXT_PUBLIC_SITE_URL='' npm run build`
now generates all 78 static pages.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: add reviewing, writing-specs, and team-workflow guides
Fill the biggest gaps a new user hits, in the plain-language voice of the
stores user guide:
- reviewing-changes.md: the two-minute human review of an AI-drafted plan
before /opsx:apply — what to open, in what order, and the red flags per
artifact — plus the /opsx:verify pass after code.
- writing-specs.md: what a strong requirement and scenario are made of,
choosing ADDED/MODIFIED/REMOVED, and right-sizing a change.
- team-workflow.md: how a change maps onto a branch and a pull request,
reviewing spec deltas in a PR, when to archive, and parallel changes —
framed as convention, since OpenSpec never touches git.
Wire them into the docs map (README), the site nav (docs.sync.config.mjs),
and light "next steps" cross-links from getting-started, editing-changes,
and workflows. Verified: site builds clean, 26 pages, all internal links
resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(website): add one-time deploy setup checklist + landing-page note
Spell out the three maintainer steps that activate auto-deploy (create the
openspec-docs Pages project, add CLOUDFLARE_API_TOKEN/ACCOUNT_ID secrets,
merge to main), and note that the pipeline mirrors docs on build regardless.
Also flag that openspec.dev is a separate Astro landing page and whether to
keep/port this Fumadocs landing page is a maintainer decision.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(website): address review feedback on docs-site PR
Maintainer review (TabishB) + Alfred blocker:
- deploy-docs.yml: guard the Cloudflare deploy on `github.ref ==
refs/heads/main`. A `workflow_dispatch` on a feature branch previously
passed the guard and, since wrangler hardcodes `--branch=main` (a
production deploy), would overwrite the live docs site. Non-main
dispatches are now build-only. Also resolves Alfred's deploy-path blocker.
- package.json: drop the direct `cnfast` dependency and delete the dead
`lib/cn.ts` (nothing imports it; a class-merge helper isn't used).
- package.json: declare `zod` (^4.4.3) — it was a phantom dep only
resolving via fumadocs-mdx's hoisted copy. Refresh the lockfile.
- docs page: omit the on-page <DocsDescription>. The frontmatter
description is derived from the first body paragraph, so it rendered
the intro twice on every page. Kept in generateMetadata for SEO/OG.
- team-workflow.md: `openspec store create` does an initial commit, so
scope "never commits" to the user's project and reframe the store
clause as "never clones or syncs on its own."
- README.md: bump stale "20+ AI assistants" to "30+" to match the site.
Verified: npm run types:check + npm run build pass, 26 docs synced,
intro paragraph now renders once per page.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(website): use pnpm to match the rest of the repo
Per maintainer review (TabishB): the root repo is pnpm (ci.yml runs
`pnpm install --frozen-lockfile` against a v9 `pnpm-lock.yaml`), but
`website/` had introduced npm + a `package-lock.json`. Standardize on
one package manager:
- Replace website/package-lock.json with website/pnpm-lock.yaml
(lockfileVersion 9.0, generated with pnpm v9 to match root).
- deploy-docs.yml: add pnpm/action-setup@v4 (version 9, before
setup-node, as in ci.yml), switch setup-node to `cache: pnpm` /
`cache-dependency-path: website/pnpm-lock.yaml`, and
`npm ci` → `pnpm install --frozen-lockfile`, `npm run build` →
`pnpm run build`.
- package.json scripts + README: `npm run ...` → `pnpm run ...`.
website/ stays a standalone package (no pnpm-workspace.yaml), as before.
Verified: `pnpm install --frozen-lockfile`, `pnpm run build`, and
`pnpm run types:check` all pass — 26 docs synced, 87/87 static pages.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: temporarily disable docs deploy
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
* Fix `archive` exit code on validation failure (#1311)
* Fix archive exit code on validation failure
In human (non-JSON) mode, openspec archive returned exit code 0 when
validation failed and nothing was archived. The three blocking paths
in ArchiveCommand.run() printed an error message but returned null
silently, leaving process.exitCode at 0. Scripts and CI could not
distinguish a blocked archive from a successful one.
The --json path was already correct (it throws ArchiveBlockedError,
caught by printJsonFailure which sets exitCode = 1). This was an
asymmetry between the two modes for the same failure.
Set process.exitCode = 1 at the three human-mode abort points before
returning null:
- delta-spec validation failure
- spec rebuild failure
- rebuilt-spec validation failure
Legitimate user cancellations (selecting no change, declining a
confirmation prompt) remain exit 0 by design.
Aligns archive with the same exit-code guarantee already approved for
apply instructions in #1250. References #498.
* Add regression test for rebuilt-spec validation exit code
Cover the third archive blocking path (spot 3): buildUpdatedSpec
succeeds but Validator.validateSpecContent rejects the rebuilt
content. Spy on validateSpecContent (same pattern as the existing
--no-validate test) to force the rebuilt spec invalid while the rest
of the flow runs for real, since this branch is otherwise defensive
and nearly unreachable — spot 1 already enforces the same
SHALL/MUST/scenario rules on the delta.
Asserts process.exitCode === 1, the failure is logged, the main spec
is left unchanged, and no archive is created.
* feat(skills): propose /opsx:update planning-artifact update skill (#1278)
* docs(openspec): propose add-update-workflow — graph-driven /opsx:update + cohesive audit
Dogfooded OpenSpec proposal for the missing first-class "update" action:
a /opsx:update workflow that propagates an edit to one artifact across its
downstream dependents (targeted mode) or audits a whole change for stale/
incoherent artifacts (audit mode) — driven by the schema's artifact graph,
never hardcoded filenames, editing planning artifacts only (never code).
- artifact-graph: expose reverse-dependency queries (getDependents/getDownstream)
+ a requires-edge mtime staleness signal (the engine already builds the
dependents map at graph.ts:98 and discards it).
- cli-artifact-workflow: surface requires/dependents/stale on `openspec status
--json` and add a `--impact <artifact>` downstream-revisit-order selector.
- opsx-update-skill: the user-facing /opsx:update command (targeted + audit).
Supersedes the proposal-only stub add-artifact-regeneration-support. Addresses
the cluster #1188/#705/#673/#247 (closes), #694/#684/#618 (answers), and is
graph-driven to avoid the #777/#666 hardcoded-artifact-pattern bug class.
Validates clean under `openspec validate --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): make add-update-workflow deterministic & grounded (Tabish review)
Reframe per the steer "more deterministic and grounded in reality":
- Deterministic spine: the CLI computes the impact set (which downstream
artifacts to revisit, in build order, with paths) as a pure function of
schema edges + filesystem. The agent only rewrites prose. Grounded in real
APIs already present: getUnlockedArtifacts (direct dependents), getBuildOrder
(order), resolveArtifactOutputs (paths); reverse map built at graph.ts:82-87.
- Replace fragile mtime staleness with a newline-normalized SHA-256 content
digest (reproducible cross-platform). Drift = upstream digest vs recorded
baseline; no baseline => "unknown", never a false positive. mtime and pure-git
rejected with rationale; digest ledger is a separable, optional layer.
- Explicit determinism boundary decision (CLI decides files/order/drift; agent
rewrites). Skill MUST source the file list/order from `openspec status
--impact`, never compute it.
- Corrected all code citations to verified lines (graph.ts:82-87,
instruction-loader.ts:366/429, status.ts); noted #1277's coverage helpers are
not in this branch's base (coordinate, don't reuse).
- Specs updated: artifact-graph Content Digest requirement; cli status digest +
deterministic impact ordering; skill determinism + baseline-aware audit.
tasks add digest/determinism/cross-platform tests + optional ledger section.
Still validates clean under `openspec validate add-update-workflow --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): harden add-update-workflow determinism; drop direct name refs
- Digest ledger tracks DIRECT upstream digests; document that transitive drift
emerges hop-by-hop as downstream is reconciled (no transitive bookkeeping).
- Ground audit's no-baseline structural facts on signals available in this
branch (missing/empty output, blocked/incomplete); capability-coverage is an
add-on only when #1277's validateChangeCapabilityCoverage is present.
- Add the "update revises only existing downstream; defer not-yet-created ones
to /opsx:continue" rule across proposal/design/specs/tasks; impact entries now
carry existence/status.
- Note artifact-level (not file-level) granularity and that getDownstream
terminates by the schema's acyclic guarantee.
- Remove direct personal references from the docs.
Validates clean under `openspec validate add-update-workflow --strict`; 10 deltas.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): full issue/PR/discussion coverage + command-family design
After a comprehensive sweep of open issues, PRs, and discussions, grounded the
proposal in the complete adjacent landscape and answered the open design
questions the cluster raises:
- #783 (Cross-artifact quality review before apply) is now a primary Closes:
it IS audit mode. Answer its open "new skill vs. extend validate" question via
the determinism split — deterministic checks (drift/completeness/coverage) are
CLI/validate-shaped; the semantic cross-artifact review is the skill. Added a
skill spec scenario for the #783 patterns (scope contradiction, spec gap,
duplication).
- Discussion #1206 ("refine proposal now?") + prior-art PR #372: official answer
is /opsx:update.
- New design Decision 8 (command family): delineate /opsx:update from
/opsx:clarify (#702, within-artifact), /opsx:review (#1251, plan-vs-code), and
verify; /opsx:update consolidates update+regen+refine into one action,
addressing skill-sprawl (#1263, #783).
- Reuse, don't reinvent: audit's empty/incomplete check reuses #1098's
artifactOutputComplete (same outputs.ts the digest helper lives in); capability
coverage reuses #1277's validateChangeCapabilityCoverage.
- New open questions: surface deterministic coherence in `validate` for a CI gate
(#783-B, #829); naming reconciliation with #783's /opsx:refine.
- Confirmed add-update-command* branches are the `openspec update` tool-file
refresh (not artifact update) — no collision.
Validates clean under --strict; 10 deltas; all relative links resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): resolve open questions to committed decisions; drift in scope
Per review steer, every open question is now a committed happy-path decision so
build-out has no dangling forks, and the deterministic drift baseline is pulled
into scope (it is what makes audit-mode drift deterministic vs. agent-guessed):
- Digest ledger IN SCOPE (design Decision 3): per-artifact DIRECT upstream
digests in ChangeMetadataSchema, written by a deterministic `openspec status
--record`; pre-existing changes (no baseline) degrade to drift `unknown` +
structural checks. Generating-flow auto-recording stays optional (graceful).
- cli-artifact-workflow spec: folded drift into the digest requirement (record
baseline / drift vs baseline / unknown-without-baseline) — stays at 10 deltas.
- opsx-update-skill spec: skill records baseline via `--record` after each
confirmed edit, so audits clear once reconciled.
- Replaced "## Open Questions" with "## Decisions resolved": ledger in scope;
targeted entry baseline-aware; apply stays standalone (points to update on
drift); cross-change (#247), continue/ff de-hardcoding (#777), and validate
CI-gate (#783-B/#829) are named follow-ups, not deferrals of the core feature;
/opsx:update kept as the umbrella name.
- Migration Plan + Capabilities + Impact + tasks updated; status JSON gains
`drift`, CLI gains `--record`. Re-synced with upstream main (0 behind).
Validates clean under --strict; 10 deltas; all links resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): harden add-update-workflow — close cross-OS, read-only, edge gaps
Stress-tested every claim against live source and fixed the soft spots:
- Cross-OS digest determinism (real bug): resolveArtifactOutputs (outputs.ts:34)
sorts ABSOLUTE paths via .sort(), which differs by OS — so a multi-file glob
artifact (specs/**/*.md) would hash differently on Windows vs POSIX. Digest now
specified to order files by change-relative forward-slash path and hash
relpath+content. Added spec scenarios (cross-platform glob stability; rename
changes digest) and a cross-OS test task.
- Read-only status invariant: moved baseline recording OFF `openspec status`
(a read command silently mutating the drift reference is a footgun) to a
dedicated `openspec reconcile` write verb. Updated spec, skill, design, impact,
capabilities, tasks; reconciled the "no new verb" claims.
- Edge case: missing upstream at record time is stored as an explicit `absent`
marker so later creating it registers as drift (spec scenario added).
- Edge case: coherent change yields no edits (clean-path scenario).
- Grounding fixes: continue-change hardcoded block is duplicated (skill 103-112 +
command 225-234) — both must be fixed in the #777 follow-up; verified no
content-hash util exists.
- Fixed two stale claims the layered edits left: the Impact digest bullet
(concatenation→relative-path) and the naming-boundary line.
Validates clean under --strict; 10 deltas (4+3+3), 44 scenarios; all links
resolve; re-synced with upstream main (0 behind); issue/PR/discussion sweep
re-run, no new items.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): pin data contracts + digest forward-compat; delineate #880
Grounded the surface so an implementer builds it without guessing, and added
proportionate forward-compatibility:
- New design "Data contracts" section with exact shapes: extended ArtifactStatus
(requires/dependents/digest/drift/driftFrom — additive to the real interface at
instruction-loader.ts:120), the --impact response, and the `.openspec.yaml`
baselines ledger. All additive; nothing existing changes type.
- Digest scheme tag (`sha256-relpath-v1:`) + forward-compat: drift compares only
same-scheme digests; an unrecognized/older scheme reports `unknown` rather than
silently mis-comparing — re-reconcile restores it. Added a cli spec scenario
and tasks for it.
- Grounded the ledger write: there is no central change-metadata writer today
(change-metadata/index.ts only re-exports schema), so reconcile does a safe
read-modify-write of .openspec.yaml mirroring the store's
parse/serialize/writeStoreMetadataState pattern (foundation.ts).
- Coverage: re-swept; folded #880 (/opsx:validate code-vs-living-specs) into the
plan-vs-code delineation alongside #1251/#1073. Main unchanged (546224e); all
citations still valid.
Validates clean under --strict; 10 deltas; links resolve.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): simplify add-update-workflow to a thin /opsx:update skill
Rework per @TabishB review (PR #1278): the proposal over-built. Drop the
deterministic-spine machinery and lean on the existing status command.
- Cut the reverse-dependency graph API (getDependents/getDownstream),
SHA-256 content digests, the .openspec.yaml baseline ledger, the
`openspec reconcile` write op, the drift report, and `status --impact`.
Removes the artifact-graph and cli-artifact-workflow spec deltas.
- Reframe propagation as bidirectional coherence (editing design can
require revising proposal), not downstream-only.
- Center the feature on one thin skill over the existing
`openspec status` / `openspec list`; design now sketches the actual
minimal skill instruction body ("written by hand").
- v1 adds no new CLI/graph/schema code: just update-change.ts + wiring.
Validates clean: `openspec validate add-update-workflow --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(update-workflow): pin the status path contract to existingOutputPaths
Address @alfred-openspec's review: the skill's write target was described
loosely as "resolved paths." Make it precise across proposal/design/spec/tasks:
- `openspec status --json` already returns everything the skill needs, in the
top-level `artifactPaths` map — `resolvedOutputPath` and `existingOutputPaths`
per artifact. No new CLI field is required.
- The skill edits `existingOutputPaths` (the concrete, glob-expanded files) and
never writes to `resolvedOutputPath`, which for a glob artifact like
`specs/**/*.md` remains the glob pattern rather than a real file.
- Add spec scenarios for editing a glob artifact's concrete files and for
deferring a brand-new file under a glob artifact to `/opsx:continue`.
- Tighten the cross-platform scenario and add a template test (3.4) asserting
the write target is `existingOutputPaths`, not a glob `resolvedOutputPath`.
Validates clean under `openspec validate add-update-workflow --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(update-workflow): address review — default profile, next-step guidance, change-scoped naming
- Register /opsx:update in the default core profile, not expanded-only
(maintainer call on the PR)
- Add next-step guidance: after updating, recommend /opsx:continue,
/opsx:apply (esp. when the change was already implemented), or
/opsx:archive — guidance only, never acted on
- Pin naming scope: skill openspec-update-change, change proposals only;
generalizing update to other graph types is an explicit non-goal
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(skills): implement the /opsx:update skill (openspec-update-change)
Implements the approved add-update-workflow change: one thin skill over
the existing status/list commands, in the default core profile.
- new update-change.ts template (skill + command), registered across
init, profiles, skill-generation, tool-detection, profile-sync-drift
- update joins CORE_WORKFLOWS and ALL_WORKFLOWS
- docs: opsx.md command row + usage note, commands.md reference section,
supported-tools.md skill list
- retire the superseded add-artifact-regeneration-support stub
- template tests pin the guardrails (schema-driven ids, planning-only,
existingOutputPaths write contract, next-step guidance); parity hashes
regenerated; profile/init/update/config tests cover the new core set
- tasks.md checked off; validate --strict passes
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: unify requirement reader and surface #498 (#1281)
* docs(openspec): propose spec parser reading fidelity (fixes #361, #498, #312)
The requirement-parsing layer silently misreads valid Markdown:
- #361: requirement-body extraction returns only the first non-blank line,
so a SHALL/MUST that wraps onto line 2 fails `validate --strict`.
- #498: `validate` (delta-block parser) and `archive` (full-spec parser)
recognize requirements by different rules, so a stray `###` header passes
validate but becomes a phantom requirement that blocks archive.
- #312 (residual): the requirement-body loop breaks on any `#` line without
consulting the code-fence mask, truncating bodies that contain fenced
code with `#` comments.
Proposal: one shared, multi-line, fence-aware requirement-body extractor used
by both the validator and the markdown parser; recognize only
`### Requirement:`-prefixed level-3 headers; guarantee validate/archive parity.
Adds regression + parity tests. #559 investigated and deferred (ambiguous root
cause — see design.md).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): bulletproof parser-fidelity proposal with empirical evidence
Hardened the proposal after reproducing every claim against main with the
bundled CLI and correcting two inaccuracies:
- #498 reframed: archive does NOT hard-fail. validate passes; archive emits
NON-BLOCKING phantom "Proposal warnings in proposal.md" because
validateChange/parseRequirements counts every level-3 header as a
requirement, while the delta-block parser (validate) and specs-apply
(rebuild) only recognize canonical `### Requirement:`. It is a consistency
bug, not data loss. Verified the rebuilt spec is clean.
- #312 reframed: the original repro is already fixed by codeFenceLineMask
(requirement count verified correct). The residual is a regression hazard:
the body loop is fence-unaware, harmless only while first-line-only, so the
multi-line fix must be fence-aware from the start.
Also: unify recognition on the canonical REQUIREMENT_HEADER_REGEX
(/^###\s*Requirement:\s*(.+)$/i, case-insensitive); surfaced a third latent
inconsistency (Zod substring includes('SHALL') vs delta word-boundary
\b(SHALL|MUST)\b) and added a single-predicate requirement; verified zero
non-Requirement level-3 headers in repo specs (CI-safe); added edge-case
scenarios (multi-line spec+delta paths, fenced scenario-looking lines,
REMOVED/RENAMED unaffected, display vs detection); replaced broken relative
links with plain paths. Proposal passes `openspec validate --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): deepen parser-fidelity proposal — add #418, upgrade #312, tier the risk
Second adversarial bulletproofing pass (reproduced everything against main):
- Add #418 (metadata-before-description): live on the spec path
(req.text = "**ID**: ...") but ALREADY fixed on the delta path. The
asymmetry is direct evidence for unifying the two extractors.
- Upgrade #312 from "regression hazard" to LIVE bug: a fenced code block
before the prose line makes req.text = "```bash" on both paths today
(distinct from the already-fixed section-count manifestation).
- Tier the fixes by risk after auditing the existing test contract
(markdown-parser.test.ts, 15 tests green on main):
Tier 1 (false-negative fixes #361/#418/#312): only widens what is read;
updates one test (:331, which asserts the first-line bug). Fence tests
(:106/:139) preserved because skip-and-join keeps SHALL-first bodies.
Tier 2 (recognition tightening #498): canonical ### Requirement: only;
a deliberate behavior change that updates bare-header tests (:258/:310)
and needs a migration note. Flagged for maintainer decision, with a
conservative opt-in-lint alternative documented.
- Surface the four-column extractor divergence table (capture / metadata /
recognition / predicate) and an explicit "Behavior changes and test impact"
section with exact test line refs.
Proposal passes `openspec validate --strict`. Does not claim #1156 (PR #1280).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(openspec): third pass — reject recognition tightening, add fenced-scenario bug, #498→safe INFO
Third deep pass found the prior Tier 2 (recognition tightening to
`### Requirement:`) was the WRONG fix and over-scoped:
- Bare `### <statement>` headers are a SUPPORTED, tested requirement format:
test/core/validation.test.ts asserts a bare-header spec is valid, and bare
headers appear across json-converter/archive/spec tests and tmp-init
fixtures. Tightening would break a large test surface and silently drop
requirements from real specs. REJECTED, with evidence documented.
- Replace the #498 fix with a SAFE INFO note in validate <change> that surfaces
non-`### Requirement:` headers in delta sections. INFO never fails validation
(strict: valid = no errors && no warnings), so nothing newly fails.
- New bug found and folded in: countScenarios is fence-unaware, so a `####
Scenario:` inside a fenced block is counted as real — a malformed delta passes
validate <change> while validate <spec> correctly fails. Same fence family.
- Proved the archive WRITE path is independent of the reader: specs-apply
rebuilds from raw `### Requirement:` blocks (extractRequirementsSection +
RequirementBlock.raw), never parseSpec/req.text → Part A cannot change
archived content.
Net effect: recognition is unchanged, so the proposal now updates exactly ONE
existing test (:331, the first-line assertion) instead of breaking bare-header
tests. Consolidated to a single cli-validate delta (dropped cli-archive and
openspec-conventions deltas). Dropped the no-space-header hypothesis (no
divergence). Passes `openspec validate --strict`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(parser): unify the requirement reader, fence/metadata/multi-line aware (#361, #418, #312); surface #498
The requirement reader was implemented twice — MarkdownParser.parseRequirements
(validate <spec>/archive) and Validator.extractRequirementText/countScenarios
(validate <change>) — and the two had drifted. Both now delegate to one shared,
fence-/metadata-/multi-line-aware extraction in parsers/requirement-text.ts so
they cannot diverge again.
Part A — unify the reader:
- Capture the full requirement body up to the first non-fenced `#### Scenario:`,
skipping blank, `**metadata**:`, and fenced-code lines; run SHALL/MUST
detection over the whole body. Fixes a wrapped keyword being dropped (#361),
metadata before the description failing validate <spec> (#418), and a fenced
block before the prose line becoming the requirement text (#312).
- Count only non-fenced `#### ` headers, so a `#### Scenario:` inside a fenced
example no longer counts as a real scenario in validate <change> (parity with
validate <spec>).
- One whole-word `\b(SHALL|MUST)\b` predicate (containsShallOrMust) shared by the
validator and base.schema, replacing the substring/word-boundary split.
- Extract buildCodeFenceMask into the shared module; MarkdownParser and
ChangeParser import it (single fence implementation).
Part B — surface #498 safely:
- validate <change> emits an INFO note when an ADDED/MODIFIED Requirements
section contains a non-`### Requirement:` level-3 header (one the delta reader
silently skips). INFO never changes the valid result, including under --strict,
so nothing newly fails. Recognition is unchanged: bare `### <statement>`
headers remain a supported requirement format.
Write path is unaffected: specs-apply rebuilds from raw `### Requirement:`
blocks, never req.text, so archived content cannot change. Displayed text in JSON
output and delta descriptions now reflects the full body.
Tests: markdown-parser.test.ts:331 updated to expect the full body; regression
tests added for #361/#418/#312, the fenced scenario, the #498 INFO note, a
single-line guard, and CRLF. Changeset added (patch). tasks.md completed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(parser): add cross-reader predicate + metadata-only guards (design edge cases)
Exhaustive verification of the unified reader surfaced two design "edge cases
for tests" not yet covered by committed unit tests:
- Cross-reader predicate agreement: a SHALL substring inside a word ("MARSHALL")
is rejected identically by validate <change> and validate <spec> — proving the
one shared whole-word predicate, and guarding against a regression to the old
substring check.
- Metadata-only body still fails validation (no requirement text) on the delta
path.
Behavior unchanged; tests only. Full end-to-end parity across all four spec
requirements confirmed against the real Validator; no spurious INFO note fires on
any existing repo change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(parser): address review — metadata-only bodies, header-bounded extraction, reader-derived INFO
- Skip **metadata**: lines only when other body text remains; a body written
entirely as metadata (e.g. `**Constraint**: The system MUST ...`) is kept as
the requirement text instead of being emptied (was a regression vs main).
- Move the empty-body rule into the shared reader: both paths fall back to the
header title, so the same block cannot pass one path and fail the other.
- End body extraction at any non-fenced markdown header, restoring old-reader
parity: a stray `### Background` divider's notes no longer satisfy the
SHALL/MUST check.
- Replace the standalone fence-aware INFO scanner with skipped-header
collection inside parseDeltaSpec, so the note reflects exactly what the
reader skipped (same section boundaries, no whole-file fence mask).
- Special-case the nameless `### Requirement:` INFO message; document that the
any-#### scenario match is deliberate spec-path parity; un-export
REQUIREMENT_HEADER_REGEX; move the import up top.
- Soften the changeset claim and list the known remaining divergences in
design.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(openspec): record the no-space ###Requirement: divergence as a known leftover
Jun's edge (reproduced): the delta/write reader's REQUIREMENT_HEADER_REGEX
accepts `###Requirement:` with no space, but MarkdownParser.parseSections
requires whitespace (per GFM) — so a no-space requirement validates as a
change with zero INFO, syncs as-is, then fails validate <spec>. Pre-existing
on main and out of scope here (tightening the shared regex would change
write-path recognition); documented under known remaining divergences with
the follow-up options, folded together with the bullet from the merge
resolution. Corrects c63913b's 'no divergence' note.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
* feat(skills): auto-approve the openspec CLI in generated skills and commands (#1300)
* feat(skills): auto-approve the openspec CLI in generated skills
Emit `allowed-tools: Bash(openspec:*)` in every generated SKILL.md so
agents that honor the Agent Skills standard run `openspec` commands
without prompting on each call. Scope is limited to the CLI; per the
standard `allowed-tools` pre-approves rather than restricts, so every
other tool a skill uses stays available under the user's normal
permission settings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(commands): auto-approve the openspec CLI in Claude slash commands
Extend the allowed-tools pre-approval to the second surface: Claude Code
/opsx:* slash commands share the skill frontmatter contract, so the
Claude command adapter now emits `allowed-tools: Bash(openspec:*)` too.
The value is single-sourced in `src/core/shared/allowed-tools.ts` (a
leaf module both surfaces import). Other command adapters are unchanged
— no other tool's slash-command format defines a per-command
pre-approval field; on the skills side every tool already gets the
standard field via generateSkillContent and non-implementing tools
ignore the unknown key.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix archive scenario drift for #1246 (#1252)
* fix archive scenario drift for #1246
* fix archive scenario drift for #1246
* remove local openspec change docs
* docs: clarify change name format (#1261)
* chore: remove stale npm lockfile (#1319)
* chore: remove stale npm lockfile
* ci: use package manager metadata for pnpm setup
* chore: scope npm lockfile ignore to root
* feat: add Trae command adapter (#1090)
* feat(tools): add Trae command adapter
- Added Trae command adapter for generating `.trae/commands/opsx-<id>.md` files
- Complete unit tests (9 test cases) and integration tests
- Updated documentation and .gitignore
- Fixed YAML escaping for carriage returns (\r)
Co-Authored-By: Claude Code <noreply@anthropic.com>
* fix: handle empty string in YAML escaping
- Add explicit check for empty string in escapeYamlValue
- Return quoted empty string '""' instead of unquoted empty scalar
- Update test to verify empty string is properly quoted
Co-Authored-By: Claude Code <noreply@anthropic.com>
* fix: address PR review feedback for Trae adapter
- Update docs/commands.md Trae entry to reflect generated opsx-* commands
- Export traeAdapter from adapters/index.ts
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs: align Trae command adapter docs
---------
Co-authored-by: jjxyxsjr <jjxyxsjr@users.noreply.github.com>
Co-authored-by: Claude Code <noreply@anthropic.com>
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
* feat: add Oh My Pi (OMP) tool support (#1276)
* feat: add Oh My Pi (OMP) tool support
Add ToolCommandAdapter for Oh My Pi terminal AI coding agent.
- New adapter: src/core/command-generation/adapters/oh-my-pi.ts
- Commands: .omp/commands/opsx-<id>.md with description frontmatter
- Hyphen transform: /opsx: -> /opsx- (filename = command name)
- Argument injection: **Provided arguments**: $@ after **Input**: heading
- escapeYamlValue applied to description field
- Register in CommandAdapterRegistry and adapters/index.ts
- Add oh-my-pi to AI_TOOLS with skillsDir: '.omp'
- Add to hyphen command transformer whitelist in init.ts and update.ts
- Full test coverage (10 cases) in adapters.test.ts
- Update docs/supported-tools.md with directory reference and tool ID
Closes #713
* fix: address CodeRabbit nitpicks
- Move ohMyPiAdapter import before opencodeAdapter (alphabetical order)
- Break long SHALL sentence and remove redundant 'follows after' in spec
* docs: polish Oh My Pi support
* docs: address Oh My Pi review nits
---------
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
* chore: remove scheduled docs workflow (#1324)
* Fix Windows CI flake hardening (#1325)
* fix windows ci test flake hardening
* restore required test check status
* ci(release): add beta prerelease workflow (#1327)
* ci(release): add beta prerelease workflow
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: harden beta release workflow
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
* fix empty store registration (#1328)
* i18n: 汉化 spec-structure 结构诊断与 SHALL/MUST 缺失提示(含测试断言)
把本次合并「先走英文」的部分块补回中文:
- spec-structure.ts:delta 标题 / 需求在 ## Requirements 之外 两条诊断
- validator:buildMissingShallOrMustMessage 提示文案
- 同步 validation.test / archive.test 断言与 shallIssues 过滤器回中文
全量 1887 测试通过;tsc / eslint 通过。archive.ts 与 v1.6.0 新功能英文待后续批次。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* i18n: 汉化 archive 与 specs-apply(命令输出 + 错误消息 + 测试断言)
- archive.ts:归档流程全部用户可见文案(校验/任务/spec 更新/确认/结果等 ~38 处)
- specs-apply.ts:delta 应用的结构化错误消息(重复/冲突/重命名/结构无效等 ~16 处)
- 同步 archive.test / store-root-selection 断言到中文
- 保留 ### Requirement: / ADDED·MODIFIED 等格式字面量与骨架 spec 英文
全量 1887 测试通过;tsc / eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* i18n: 汉化 legacy-cleanup / update / 多选提示等提示文案(含测试断言)
- legacy-cleanup.ts:清理结果、project.md 迁移提示等
- update.ts:工具最新提示、--force 提示
- searchable-multi-select.ts:搜索/导航/无匹配 UI 文案
- 同步 legacy-cleanup.test / update.test 断言
全量 1887 通过;tsc / eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* i18n: 汉化 view 仪表盘文案(含测试断言)
OpenSpec 仪表盘标题、草稿/活跃/已完成变更、规范、任务进度、需求计数等;同步 view.test。
全量 1887 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* i18n: 汉化 instructions 模板中的 agent 说明与错误提示
<project_context>/<rules>/<template> 等模板注释、tracks 文件缺失/无任务的错误提示。
全量 1887 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* i18n: 汉化剩余零散提示(校验/元数据/多选/标记/工具选择等)
- validator: skipped-header INFO、delta 章节未解析提示
- change-metadata schema、specs-apply ADDED 已存在、init 技能计数、
file-system 标记状态、workset 工具选择、legacy 迁移提示补漏
- 同步各测试断言(含正则)
全量 1887 通过;tsc / eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: 移除上游 changeset(引用 @fission-ai/openspec,不属于本 fork)
上游 v1.6.0-beta.1 带进来的 5 个 changeset 引用上游包名,本仓库不用 changeset
发布(走 tag + pnpm publish),导致 CI 的 Validate Release Tracking 失败。删除之。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(review): 双语化 spec-structure 结构诊断正则(review F1 major)
review-loop 双评审员一致发现:spec-structure.ts 只汉化了消息、未双语化检测正则,
导致 (A) `## 需求` 段内英文 `### Requirement:` 头被误报为「在 ## Requirements 之外」
并使 archive 抛错,(B) 错位的中文 `### 需求:` 需求静默丢失(#498 诊断对中文作者失效)。
- spec-structure.ts:REQUIREMENTS_SECTION_HEADER / DELTA_HEADER / REQUIREMENT_HEADER 双语化
- validator.ts:151:无名头正则 /^requirement:?$/i → /^(?:requirement|需求)[::]?$/i(F2)
- chinese-bilingual.test:把「验证 RequirementSchema 接受 必须/禁止」的空断言(base.schema
不再强制关键字,任意非空文本都过)改为经 Validator 的真实识别测试(正+负)(F4);
新增 findMainSpecStructureIssues 双语回归(混合段 / 错位中文 / 中文 delta 头)
已在编译产物上实证:Case A 无误报、Case B 正确标记。全量 1890 通过;tsc / eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(review): 双语化归档场景漂移保护 + 对齐无空格头正则(review round 2)
Codex 跨模型评审发现两处双语盲区 + 一处测试加强:
- C1 (major, 数据丢失): specs-apply.ts parseScenarioBlocks 只认 ASCII `#### Scenario:`,
中文 `#### 场景:` 场景计为 0,使 #1246「陈旧 MODIFIED 丢场景」保护对中文失效——
MODIFIED 可静默删除已有中文场景。改为 `(?:Scenario|场景)[::]` 双语;
新增归档回归测试(中文场景被丢弃 → 正确中止、老场景保留)。
- C2 (minor): spec-structure REQUIREMENT_HEADER `###\s+` → `###\s*`,与共享读取器
requirement-blocks(`###\s*`,且测试固定支持无空格头)对齐。
- C3 (minor): 加强 over-match 测试为断言 issues 全空(可捕获误判为 delta-header);
新增无名中文头(全/半角冒号)不被误标的用例。
已实证:parseScenarioBlocks 现能解析中文场景。全量 1892 通过;tsc / eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(review): 场景漂移保护改为围栏感知 + 收紧名称捕获(review round 3)
Codex 第三轮在 parseScenarioBlocks 上又发现两处边界(均已实证):
- Codex#1 (major): 未做围栏屏蔽——需求正文中围栏代码块内的 `#### 场景:` 示例
被计为真实场景,MODIFIED 保留真实场景时会误判「丢了示例场景」→ 误中止归档。
改用 buildCodeFenceMask,头匹配与边界扫描都跳过围栏行;新增归档回归(围栏内示例
不计、归档成功)。
- Codex#2 (minor): `#### 场景:<空白>` 会以空名匹配,违反无名不匹配。名称捕获
`(.+)` → `(\S.*?)`,要求至少一个非空白字符。
- 抽出 SCENARIO_HEADER_RE 常量,消除双处重复字面量。
实证:围栏内示例不计、纯空白名不计、EN+ZH 混合仍正确。全量 1893 通过;tsc/eslint 通过。
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Clay Good <hi@claygood.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
Co-authored-by: TabishB <tabishbidiwale@gmail.com>
Co-authored-by: Danilo <danilopopeye@users.noreply.github.com>
Co-authored-by: zhangsan582 <1553977725@qq.com>
Co-authored-by: Ercan Erdoğan <ercanerdogan@gmail.com>
Co-authored-by: shin <112563017+jjxyxsjr@users.noreply.github.com>
Co-authored-by: jjxyxsjr <jjxyxsjr@users.noreply.github.com>
Co-authored-by: xianzheTM <ylxianzhe@outlook.com>
Status
LGTM — ready to review. Planning artifacts (proposal / design / spec deltas / tasks) and the implementation + tests are now all in this PR. The change validates
--strict; the full suite is green (1791 passed; only the pre-existing, environment-specific zsh-installer failures remain); all 36 repo specs pass--specs --strictwith no new false positives.What was wrong
Three read/validate commands each silently forked their own resolution or validation logic instead of reusing the canonical implementation a sibling command already gets right. The fork stays invisible until the two paths disagree — then the tool reports a confident falsehood about the spec source of truth. All three were reproduced against the built pre-fix CLI (base
546224e).validategates onproposal.mdwhilestatus/instructionsgate on directory existence. A scaffolded/still-authoring change thatstatus --change fooresolves (exit 0) makesvalidate fooreturnUnknown item(exit 1). Worse,validate --allover a lone proposal-less change printsNo items found to validateand exits 0 — a silent CI pass. Theproposal.mdmembership gate (getActiveChangeIds) diverged from the directory-existence rule (validateChangeExists) at three call sites (targeted, bulk, interactive selector). Nested multi-area deltas (specs/<area>/<capability>/spec.md) were also not discovered, so a resolved multi-area change reportedNo delta sections found.view/list/archivehardcodechanges/<name>/tasks.mdwhilestatusresolves the schema's tracked-tasks glob. With tasks split acrossbackend/tasks.md(2/2) +frontend/tasks.md(1/3) and no top-level file,viewmislabeled the change Draft andlistshowed No tasks. The same helper bug letarchivemove a 3/5 unfinished change intochanges/archive/— the incomplete-task gate was wholly bypassed (data-safety bug), proven empirically.How it was fixed
Each divergent path is pointed at the canonical implementation, and the agreement is asserted by test (design Decision 7). No new command, flag, schema field, or output format; the already-correct paths are unchanged.
status/instructionsresolve by directory existencevalidatenow resolves via sharedgetAvailableChangesat all 3 sites; newfindDeltaSpecFileswalker recurses the nested multi-area layoutstatusresolves the tracked-tasks artifact'sgeneratesglob (resolveArtifactOutputs)getTaskProgressForChangenow resolves through that glob, aggregating checkboxes scoped to the change dir, with a never-throw fallback to a single top-leveltasks.md; all 4 callers rewired and the duplicate copy inchange.tsfolded onto the helperapplySpecRulesrecovers the header viaextractRequirementsSectionand emits the hint (header-only) or generic message (no keyword) exactly once; the now-redundant Zod refine is removed (deltas never used it)The main-spec actionable sentence is byte-identical to the change-delta path.
Proof it works
design.md→ "Empirical evidence" (including thearchivemoving a 3/5 change intochanges/archive/, andvalidate --allexiting 0 on a lone proposal-less change).test/commands/validate.test.ts), nested multi-area delta discovery and header-only main-spec hint parity (test/core/validation.test.ts), view resolves the same files as status (test/core/view.test.ts), the archive incomplete-task gate (test/core/archive.test.ts), and nested-glob task aggregation (test/utils/task-progress.test.ts, new).--strict; 36 repo specs pass--specs --strict.Corrections captured during verification
validatealready accepts--store.show.tsand the deprecated noun-formchange validateare scoped out.apply.tracksis a filename, not a glob — it selects the tracked artifact; the glob is that artifact'sgenerates. Parity is resolution-mechanism (same files), not count:statuschecks file existence and never counts checkboxes (it called a 3/5 changeisComplete:true), soview/archivecount checkboxes themselves.applySpecRulesowns both the header-only and no-keyword cases, the refine would double-emit. RENAMED carries no body and is out of scope.Scope boundaries
sync --checkin the sync/unarchive proposal).Review surface
proposal.md,design.md(root-cause table, empirical evidence, 8 decisions, parity test strategy),specs/cli-validate|cli-view|cli-archive/spec.md,tasks.md.src/commands/validate.ts,src/core/validation/validator.ts,src/utils/task-progress.ts,src/commands/change.ts,src/core/{view,list,archive}.ts,src/core/schemas/base.schema.ts.test/commands/validate.test.ts,test/core/{validation,view,archive}.test.ts,test/utils/task-progress.test.ts.🤖 Generated with Claude Code