mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-20 06:19:21 +08:00
@fission-ai/openspec@1.13.2
307
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
336414665f |
fix(verify): stop reporting removed requirements as missing (#1962)
* fix(verify): stop reporting removed requirements as missing Verify walked every "### Requirement:" in the delta specs and looked for an implementation of each one, whatever section it sat under. A REMOVED requirement that the change had removed correctly came back as CRITICAL "Requirement not found", with a recommendation to implement it. An agent that follows the report puts back the behavior the change just deleted. Verify now notes the delta section of each requirement first. ADDED and MODIFIED keep the existing checks. REMOVED is checked the other way round: finding nothing is the expected result, and it is only critical while the behavior is still in the code. RENAMED only changes a name, so the old name is not reported as missing. Scenario coverage skips removed requirements, since there is nothing left to cover. Archive and sync already handle each section on its own terms; verify was the one step in the loop that did not. Closes #1959 * docs(specs): scope the general verify scenarios to ADDED and MODIFIED The Spec coverage, Requirement implementation mapping and Scenario coverage scenarios still told the verifier to check every requirement in the delta specs, which contradicts the Removed requirement scenario added in the previous commit. A verifier following them would repeat the #1959 failure. * fix(verify): report a removal-only change as ready when nothing remains With #1732 merged, a change whose delta specs only remove or rename requirements left Requirement Implementation Mapping and Scenario Coverage with nothing to check. The "no usable requirements" rule then marked them not verified, so verify never reported the change ready, the exact case #1959 describes. Those two checks are now not applicable when the readable delta specs hold REMOVED or RENAMED requirements and no ADDED or MODIFIED ones. An empty or unparseable delta still marks them not verified. Keyword matches in openspec/ artifacts, docs, or code that serves only the Migration note or an ADDED requirement are no longer evidence by themselves that a removed requirement is still implemented; a code path that still delivers the removed behavior is reported even when shared. The summary counts removals separately from covered requirements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(verify): check a renamed requirement's behavior against its baseline A RENAMED entry only told verify not to report the FROM name as missing, and a rename-only change marked the correctness checks not applicable. Nothing checked that the renamed requirement's behavior was still implemented, so verify could report readiness unchecked. Spec Coverage now reads the baseline requirement from the main spec (under the FROM name, or the TO name once synced) and checks that its behavior is still implemented, without requiring code symbols to be renamed. A missing behavior is CRITICAL "Renamed requirement not found"; an unreadable baseline marks the entry not verified. A TO name that also appears under MODIFIED is still checked there. Regression tests cover the skill template, the command template, and the committed skills/ mirror. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
072de6bc39 |
fix(verify): do not report unverified dimensions as passing (#1732)
* fix(verify): do not report unverified dimensions as passing Step 5 gates task and spec coverage on `contextFiles.tasks` and `contextFiles.specs`. `contextFiles` is an artifact-id map and artifact ids come from the active schema, so on a schema that defines neither, both branches are no-ops: nothing is checked, no issues are raised, and step 8 concludes "All checks passed. Ready for archive." The Graceful Degradation guardrail already asks the agent to note skipped checks, but nothing stopped the all-clear verdict. Mark an unchecked dimension `Not verified` in the scorecard and require the final assessment to name it. * fix(verify): map skipped checks to report outcomes * fix(verify): retain no-task and skipped-check context * fix(verify): harden evidence gaps and final assessments * fix(verify): preserve optional workflows and task artifact fallback * fix(apply): resolve tracked task globs by schema path * fix(verify): preserve unavailable task evidence * fix(verify): distinguish untracked tasks from missing evidence * docs(apply): document tracked globs and JSON evidence * test(parity): regenerate hashes after merging #1940 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): restore the #1837 regression tests dropped in the merge The earlier conflict resolution took our whole side of the parity file, which discarded the two threshold tests main gained in #1940. Take main's file verbatim and regenerate the hashes instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): regenerate hashes after merging #1955 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): regenerate hashes after merging #1795 and #1926 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): regenerate hashes after merging #1731 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
fb1b87613b |
fix(archive): use schema-aware task progress in workflows (#1795)
* fix(archive): use schema-aware task progress in workflows * test(archive): verify task lookup follows the selected store * fix(archive): reject invalid task progress in workflows * test(parity): regenerate hashes after merging #1940 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): restore the #1837 regression tests dropped in the merge The earlier conflict resolution took our whole side of the parity file, which discarded the two threshold tests main gained in #1940. Take main's file verbatim and regenerate the hashes instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): regenerate hashes after merging #1955 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
72fbe4c904 |
fix(update): close the dead end for partially populated glob artifacts (#1733)
* fix(update): close the dead end for partially populated glob artifacts `artifactOutputExists` returns true as soon as a single file matches a glob `generates`, so a `specs/**/*.md` artifact is `done` after the first delta spec. `/opsx:continue` selects only `ready` artifacts and never revisits it. The update templates forbid creating new files under a glob artifact and point the user to `/opsx:continue` instead, which cannot act on it. A capability spec the coherence review finds missing therefore has no supported way to be created. Allow update to write that file: concrete path only, rules fetched from `openspec instructions`, and the same confirm-before-write rule as every other revision. Creating an artifact that has no files at all stays out of scope - that one is genuinely `/opsx:continue`'s job. * fix(update): tighten glob gap write guardrails * fix(update): narrow continue handoff to empty artifacts * fix(update): harden glob gap creation guidance * fix(update): preserve creation safeguards for glob companions * fix(update): integrate glob guidance with current workflows * docs(skills): note update's glob companion-file exception docs-lab/reference/skills.md said update creates nothing new, which this change makes false for glob artifacts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(parity): restore the #1837 regression tests dropped in the merge The earlier conflict resolution took our whole side of the parity file, which discarded the two threshold tests main gained in #1940. Take main's file verbatim and regenerate the hashes instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(explore-docs): allow the update guidance line that says "no files yet" main's #1833 guard flags any docs line containing "no files". The new /opsx:update guidance describes which artifacts update leaves to continue, not what explore writes, so allow that exact line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
fd56e12c9e |
fix(artifact-graph): support brace expansion and extglob output patterns (#1885)
* fix(artifact-graph): support brace expansion and extglob output patterns (#1854) * fix(artifact-graph): preserve literal output filenames * fix(artifact-graph): confine expanded brace patterns * test(artifact-graph): cover later brace ranges and specify glob contract * test(artifact-graph): resolve brace globs with Windows separators --------- Co-authored-by: 胥寅 <xuuyin@dingtalk.com> Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay Good
|
fe429a13dc |
fix(kilocode): generate commands in canonical directory (#1938)
* fix(kilocode): generate commands in canonical directory * fix(kilocode): preserve unrelated legacy workflows |
||
|
|
d3d770736f |
fix: release archive lock on Windows (#1769)
* fix: release archive lock on Windows * test(archive): make the Windows claim-release regression actually fail The new test compared the lstat target against the temp-dir claim path verbatim. The command stats the resolved real path, so on macOS (/var -> /private/var) the comparison never matched, `dev: 0n` was never injected, and the test only asserted that an ordinary archive releases its claim — which already passed before the fix. Verified: it passed with the source change reverted. Match the claim by file name instead, and count the interceptions so the test fails loudly if the mock ever goes inert again rather than silently passing. With the source change reverted the test now fails as intended. Also add the missing changeset for the user-visible fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(archive): keep replaced claims with absent device ids * test(archive): retain claim when zero-device path identity changes --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 5
|
92fb72d1dc |
fix(workflows): create the main spec when a capability is new (#1701)
* fix(workflows): create the main spec when a capability is new The agent-driven archive workflow told agents to "compare each delta spec with its corresponding main spec" and said nothing about the case where that main spec does not exist yet. Comparing against nothing reads as "already synced", so the agent took the archive branch and the new capability's main spec was never written — the change landed in changes/archive/ with openspec/specs/ still empty. `openspec archive` already handles this: buildUpdatedSpec creates the spec from the delta's ADDED requirements, rejects MODIFIED/RENAMED with "only ADDED requirements are allowed for new specs", and warns past REMOVED. The guidance now says the same thing, so the agent path and the CLI path agree: - archive-change: a missing main spec counts as changes needed and is named in the summary as a spec the sync will create — never as already synced. - sync-specs: MODIFIED and RENAMED have no requirement to act on when the main spec is absent, so the sync stops and reports rather than inventing one; REMOVED is skipped with a warning. Guidance text only — no CLI, parser, or archive behavior changes. Closes #1222 Closes #1264 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(sync): never create a main spec with nothing to put in it CodeRabbit caught a gap in the previous commit: step 4b now tells the agent a REMOVED-only delta has nothing to remove, but step 4d still read as "create the main spec if the capability doesn't exist yet" unconditionally. Following both would write a spec whose `## Requirements` section is empty. Verified against the CLI on a REMOVED-only delta targeting a capability with no main spec: Specs to update: parking: create ⚠️ Warning: parking - 1 REMOVED requirement(s) ignored for new spec. Validation errors in rebuilt spec for parking (will not write changes): ✗ Spec must have at least one requirement Aborted. No files were changed. So step 4d is now gated on the delta having ADDED requirements to seed the spec with, and says what the CLI reports when it does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: define "main spec" and close the no-ADDED archive loop Hardening pass over the two fixes in this branch. Guidance: the archive step's verification pass re-runs the same comparison the fix touched, so a delta that can create nothing — no ADDED requirements, no main spec to merge into — would have been reported as "still needs sync" after a sync that correctly created nothing, and an agent could loop on it. That case now short-circuits with the reason, matching `openspec archive`, which refuses it with "Spec must have at least one requirement". Docs: the glossary defined "delta spec" but never "main spec", which is half of #1647's terminology complaint. It now defines the term and says that for a new capability the main spec is created by the archive rather than written up front; concepts.md says the same in the delta-section table and the archive process. The docs site generates from docs/ at build time, so no website files change. Changeset rewritten in the house style (prose, no commit header; the changelog-github action supplies attribution) and renamed descriptively. All three CLI branches this guidance describes were verified end to end: ADDED against a greenfield repo creates the spec and carries its Purpose; MODIFIED reports "target spec does not exist; only ADDED requirements are allowed for new specs"; REMOVED-only aborts with "Spec must have at least one requirement" and writes nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(glossary): a standalone sync creates the main spec too The "Main spec" entry said the spec is created "by the archive", but the "Sync" entry two sections down says /opsx:sync creates it as well, without archiving — and specs-sync-skill's "New capability spec" scenario is the sync's own behavior. Names both paths so the two entries agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(workflows): clarify removed deltas for missing specs * fix(workflows): preserve explicitly retired missing specs * fix(archive): preserve explicit skip-sync choice for blocked deltas --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 5
|
8b99c07bd0 |
fix(status): name the command that resumes a change (#1786)
* fix(status): name the command that resumes a change `openspec status` printed the artifact checklist and stopped. The command that moves the change forward was computed already and shipped in the JSON `nextSteps` sentence, but the text surface never rendered it, so anyone resuming a change had to know the next command by heart. Extract `resolveNextStep` so the command and the published sentence come from one place, and print it as a `Next:` line — matching the idiom `openspec new change` already uses to hand off to `openspec status`. The completion case matters most: "All planning artifacts complete!" reads as "you are done" even while implementation tasks remain, and it is now followed by the `openspec instructions apply` command that resumes the work. Closes #906 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style(status): keep the loadStatus comment on loadStatus The store-flag note landed between the "single definition" comment and the declaration it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(status): cover the next-step line, and record it in the spec The behavior change shipped without the OpenSpec change this repo requires of user-facing work, and without coverage for the paths a reviewer would reasonably ask about. Adds `add-status-next-step`, whose delta modifies `Next Artifact Discovery` in `cli-artifact-workflow` - the requirement that already says status is how you learn what comes next. It now also says status names the command. Coverage added: - Unit tests for `resolveNextStep`, pinning the published `nextSteps` sentences verbatim. Confirmed byte-identical to main's output, so the split into command + sentence provably did not reword the contract. - Custom-schema case: the line is built from the resolved artifact id, so a project with neither proposal/specs/design/tasks still gets a usable command. - Skipped artifacts are never named - they satisfy dependents but must not be created. - `--json` stays parseable and carries no `Next:` line. - `--all` gives every change its own line, and a failed entry none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(status): assert the next-step line closes the output The spec delta says the text output ends with the `Next:` line, but the assertions used toContain, which a later line would still satisfy. Compare the last non-empty line instead, across all four ready/complete cases and the skipped one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(status): read the closing line CRLF-safely Split on /\r?\n/ so a CRLF stream cannot leave a carriage return attached to the line under comparison, and route the parity check through the same helper instead of its own scan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: move the status Next: line to the canonical CLI page docs-lab/README.md makes docs-lab/ canonical and the old docs/ tree legacy, so the entry now lives under 'openspec status' in docs-lab/reference/cli.md and docs/cli.md is back to its state on main. Both documented outputs were captured from real runs rather than written by hand: the blocked case in a change with proposal and specs, and the complete case with all four artifacts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(status): drop em dashes from the changeset and proposal Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Marzx13andClay Good
|
db03c6c4b0 |
feat(validate): add findings-only bulk reports (#1713)
* feat(validate): propose findings report * docs(validate): clarify findings report contract * feat(validate): implement and harden bulk findings reports * test(validate): canonicalize store paths natively --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
openspec-cloud[bot]andClay Good
|
cdd06a0594 |
docs(specs): align four requirements with current behavior (#1707)
* docs(openspec): correct 4 requirements that the code has outgrown - ai-tool-paths/path-configuration-for-supported-tools#2: Changed the windsurf scenario's required skillsDir from `.windsurf` to `.devin`. - cli-update/slash-command-updates#6: Require $ARGUMENTS to be placed in the file body (not frontmatter) for OpenCode archive commands. - rules-injection/validate-artifact-ids-during-instruction-loading#6: Updated the expected warning text to use double quotes and to state it matches no artifact in any available schema, listing known artifact IDs. - specs-sync-skill/skill-output#3: Changed the expected no-changes message to 'Specs already in sync; no files changed.' to match the code. None of these reduce what a requirement demands. Scanned at |
||
|
|
2fa679f180 |
fix(schema): make schema init --default actually set the default (#1709)
* fix(schema): make schema init --default actually set the default --default wrote defaultSchema to openspec/config.yaml, but the config loader only reads schema, so new changes kept using spec-driven while the command reported success. Write the key that is read, and drop the dead one a previous run may have left behind. Fixes #1708 * fix(schema): preserve supported configs when setting default * docs(schema): specify default config file handling * docs(schema): protect default config migration * fix(schema): make default initialization atomic * fix(schema): hide init staging and backup dirs from discovery `schema init` stages into `.init-staging-<rand>` and moves an existing schema aside to `<name>.init-backup-<pid>-<ts>`, both inside the schemas dir. `schema fork` already did this and the resolver filters its temp names out of discovery; the init names were never added, so `listSchemas` and `listSchemasWithInfo` surfaced them as real schemas -- in shell completions, "available schemas" error lists, and change-metadata validation. The backup is the durable case: cleanup failure is deliberately tolerated with a warning, so a blocked cleanup (or a crash mid-transaction) leaves a permanent phantom schema behind. Generalize the fork-only filter to cover both commands' staging and backup names. Real schema names are kebab-case, so excluding these dot-bearing names can never hide a legitimate schema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
dd7cea3ffe |
feat(show): diff delta requirements against the main specs (#980)
* Proposed feature: openspec show --diff to see changed requirements more clearly * Implementation of the --diff feature, which led to some spec changes during implementation. * Update the proposal to be clearer. Thanks coderabbit * Fix a bug identified by coderabbit with excessive trimming, and add a testcase for it. * fix(show): harden --diff for review feedback Keeps `openspec show <change>` without `--diff` a raw proposal passthrough, reports when a change has no delta specs instead of returning silently, preserves the Reason/Migration body of a REMOVED requirement, and resolves main specs through the command's root so `--store <id>` diffs against that store. Text mode and JSON mode now render from one shared collection pass, the CLI tests drive argv arrays from a mkdtemp project instead of interpolated shell strings, `--diff` is registered for shell completions, and the stray package-lock.json is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * build(deps): declare the diff dependency and refresh the flake hash Adds the `diff` runtime dependency that requirement-diff.ts imports, updates pnpm-lock.yaml, and regenerates the flake's pnpmDeps hash so `nix build` matches the new lockfile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(show): diff nested capabilities too collectSpecDiffs enumerated only top-level directories under the change's specs/, so a nested capability (specs/<area>/<id>/spec.md) was skipped: text mode printed nothing for it and its MODIFIED deltas came back from --json with no diff. It now uses the same discoverSpecFiles() helper ChangeParser uses, so the capability ids match the `spec` field of the JSON deltas. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(show): report header mismatches instead of hiding them Two cases where --diff quietly showed something misleading: A MODIFIED requirement whose capability has no main spec was rendered as all-additions, which reads like a new capability. It is an authoring error archive will reject, so it now prints the raw text with a warning naming the missing spec. A header that differs from the main spec only in case or interior spacing found no match at all under exact lookup, or matched under a lowercase-only comparison that let a real mismatch through silently. Lookup is now exact first, then the shared foldRequirementName fallback, and a folded match prints the diff the author meant alongside a warning that archive matches names exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(show): name the main spec as such in collectSpecDiffs Comment and locals still called the main spec the "base" spec, and the no-main-spec comment described the old all-additions behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(deps-dev): bump the development-dependencies group with 2 updates Bumps the development-dependencies group with 2 updates: [smol-toml](https://github.com/squirrelchat/smol-toml) and [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint). Updates `smol-toml` from 1.7.1 to 1.8.0 - [Release notes](https://github.com/squirrelchat/smol-toml/releases) - [Commits](https://github.com/squirrelchat/smol-toml/compare/v1.7.1...v1.8.0) Updates `typescript-eslint` from 8.66.0 to 8.67.0 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.67.0/packages/typescript-eslint) --- updated-dependencies: - dependency-name: smol-toml dependency-version: 1.8.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: development-dependencies - dependency-name: typescript-eslint dependency-version: 8.67.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: development-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> * chore(nix): invalidate pnpm dependency hash * fix(nix): update pnpm dependency hash * fix(show): harden requirement diff output * build(nix): pin combined dependency hash * test(show): assert proposal precedes diffs * docs(show): clarify JSON diff diagnostics * fix(show): retain unified diff hunk headers --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
126c5d6c59 |
fix(validate): report a Purpose left as the archive placeholder (#1671)
* docs(openspec): propose warn-on-purpose-placeholder
When a delta introduces a capability with no usable `## Purpose`, archive
writes `TBD - created by archiving change <name>. Update Purpose after
archive.` into the new main spec. Three places already tell authors to
replace it -- the `specs` instruction ("including a leftover `TBD`
placeholder"), the sync-specs summary step ("so it gets written now rather
than lingering"), and the cli-archive contract -- but nothing reports that
it is still there.
`--strict` cannot reach it. The check meant to catch a Purpose nobody wrote
is a 50-character floor and the placeholder is 91 characters, so the one
rule that exists to catch a thin Purpose is satisfied by the exact text
meaning nobody wrote one: a Purpose reading "Does stuff." fails --strict
today, while one saying nothing at all passes.
Proposes reporting it as a warning against the spec's Purpose -- silent by
default, failing under --strict, so a project already carrying placeholders
keeps validating until it opts into the stricter gate. Detection is narrow:
the generated sentence wherever it appears, and otherwise only a `TBD`
opening the Purpose, so prose raising an open question is left alone.
Planning artifacts only; no source changes.
Refs #369
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(validate): report a Purpose left as the archive placeholder
When a delta introduces a capability with no usable `## Purpose`, archive
writes a placeholder into the new main spec. Nothing read it afterwards, so
the capability kept a to-do in it while every command reported success.
`--strict` could not reach it. The check that exists to catch a Purpose
nobody wrote is a 50-character floor, and the placeholder clears it: a spec
whose Purpose read "Does stuff." failed --strict, while a spec whose Purpose
said nothing at all passed. #369 reported agents leaving the placeholder
behind and stayed open seven months; every remedy since has been an
instruction, which is the mechanism that report described as unreliable.
validate now reports it as a warning against the Purpose, naming the line
and saying to edit the main spec directly -- a delta's `## Purpose` is read
only when the capability is created, so it cannot replace an existing one.
Warning rather than error, because strict mode already means "warnings
fail": a project carrying placeholders keeps validating by default and only
--strict fails. Archive is untouched -- it validates rebuilt specs without
--strict, so a spec archive writes still passes the validation it would have
passed before, and the text archive writes is byte-identical.
The placeholder is recognised through the same constants the writer composes
it from, so the check cannot drift from the sentence it looks for -- the
failure mode of a second, hand-copied spelling being a check that matches
nothing and looks exactly like a check that found nothing. The one case that
cannot be a lookup is an agent-written placeholder, kept to a `TBD` opening
the Purpose: "the retry budget is TBD pending benchmarks" is authored prose
and is left alone.
Verified: 209 archive tests pass unchanged (the placeholder text is
asserted literally, so the output is provably identical); full suite 138
files / 3993 tests; 36/36 strict spec validations; build, lint and typecheck
clean. Against a project carrying four real placeholders, default mode still
exits 0 and --strict fails exactly those four.
Cross-platform CI is not yet confirmed -- it needs a pushed branch. Line
endings are covered by tests asserting a CRLF spec and an LF spec produce
identical findings, and the module does no path handling.
Refs #369
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(validate): make every placeholder guard load-bearing
A mutation pass over the seven guards -- revert one, see which tests die --
found two that no test held.
The prefix/suffix test did not exercise the guard it named. Its Purpose read
"Explains what happens when archiving change my-change runs twice", which
contains neither half of the generated sentence, so it passed whether or not
the suffix was required. Matching on the prefix alone killed nothing. The
Purpose now embeds the real prefix constant and asserts the suffix is absent,
so the case is the one the name claims; the mutation kills it.
The empty-Purpose early return was genuinely dead. Neither rule matches empty
text, so removing the branch changed no behaviour and failed no test. Rather
than keep a guard nothing can hold, the branch is gone and the comment says
why an empty Purpose still yields null. The tests asserting that behaviour
are unchanged and still pass.
Every guard now dies under mutation:
whole check removed from applySpecRules ......... 6 tests
brevity no longer suppressed (else -> if) ....... 1
word boundary dropped from the TBD marker ....... 1
generated placeholder matched on prefix alone ... 1
line-ending normalisation removed ............... 2
section-boundary guard removed from locator ..... 1
Full suite 138 files / 3993 tests, lint and typecheck clean.
Refs #1670
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(openspec): record the mutation pass in the task list
The mutation work changed the implementation -- a test rewritten and a dead
branch removed -- but no task covered it, so the plan claimed less work than
was done. Added as group 6, marked complete, with why it was not planned.
5.4 now says what blocks it. It needs a pushed branch for the cross-platform
matrix, and the note records that line endings are covered locally by tests
asserting a CRLF spec and an LF spec produce identical findings, so a reader
can tell the difference between unverified and unverifiable-from-here.
The specs, proposal and design are unchanged and were checked: the delta's
empty-Purpose clause constrains behaviour, not structure, and that behaviour
is the same -- the redundant branch went, the rule did not.
26 of 27 tasks complete; the change still validates --strict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(openspec): close 5.4 on a green cross-platform matrix
CI dispatched on the fork against this branch: lint & typecheck, and the test
suite on linux-bash, macos-bash and windows-pwsh -- all green. The Windows job
installed, built and ran the suite rather than short-circuiting, which is the
part 5.4 existed to check, since the placeholder locator counts lines in files
that may carry either ending.
Recorded as a workflow_dispatch run on the fork, not the upstream pull-request
run, because those are not the same gate and the note should not let a reader
assume otherwise. Nix Flake Validation and Validate Release Tracking skipped:
this branch touches neither the flake nor release tracking.
27 of 27 tasks complete.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(validate): name the placeholder's line, not the prose above it
The warning tells you which line to fix, and named the wrong one when the
generated sentence did not open the Purpose:
3 ## Purpose
4 Handles widget retries. <- warning pointed here
5
6 TBD - created by archiving ... <- placeholder is here
The locator asked "what is the first non-blank line after ## Purpose?"
rather than "where is the placeholder?". Those are the same line in five of
the six shapes a placeholder can take -- a leading TBD marker is the first
non-blank line by definition, and archive writes the generated sentence as
the section's only content -- so the two questions only diverge when a human
types prose above a leftover placeholder.
Pointing at that prose is worse than pointing nowhere: the reader sees a
sentence that is plainly fine and concludes the check is broken. design.md
already said a wrong line number is worse than none, and the delta already
required naming the line the placeholder is on, so this is the
implementation meeting a contract that was already written, not a change of
contract.
The locator is now told which rule matched. A leading marker keeps the
first-non-blank behaviour, because that is where it sits; the generated
sentence is located by its own text. When both match the leading marker
wins, being the earlier of the two.
Found by CodeRabbit on #1671. The finding was real despite its own
"Addressed" marker, which only tracked the file changing in a later commit.
Two test gaps let it through. The case that covered this input asserted
only that something was reported, never which line -- so it now asserts the
line, and a table pins every position a placeholder can occupy, each case
first checking that the line it expects really carries the placeholder. The
mutation pass could not have caught it either: mutation proves a test dies
when a guard is broken, and cannot invent an assertion nobody wrote.
Reverting the branch fails exactly the three new expectations. Full suite
4000 tests / 138 files, lint and typecheck clean.
Refs #1670
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(validate): set the placeholder line unconditionally
ValidationIssue.line is optional and the project does not enable
exactOptionalPropertyTypes, so a plain assignment typechecks and
JSON output is unchanged - JSON.stringify drops undefined values.
findPurposePlaceholderIssue already returns the key unconditionally,
and the neighbouring push sites assign line plainly, so the
conditional spread was the odd one out.
* fix(validate): widen the placeholder check to TODO and read fences as quoted
#1670 left two questions open. Both are answered here, against how OpenSpec
already reads a spec.
A `TODO` opening the Purpose now reports as the same finding as a `TBD`.
Nothing OpenSpec writes produces one, but the marker an author leaves behind is
whichever word they reached for, and a Purpose reading `TODO: fill this in` is
as unwritten as one reading `TBD`. Only the opening position counts, as before,
so `TODOs are tracked in the linked issue` is still authored prose.
Fenced code inside a Purpose is now read as quoted material rather than as the
Purpose speaking, through the `buildCodeFenceMask` the requirement and structure
parsers already share. Without it a spec documenting the sentence archive writes
is reported as carrying it, which is the check failing the one document that
explains it - and a warning that fires on the docs teaches people to ignore the
warning. Fenced lines are skipped when locating the placeholder too, so a
`## Purpose` or `## Requirements` quoted in a fence can neither be mistaken for
the section header nor end the section early.
The message now names both what archive writes and a marker left in its place,
since one message covers both. Severity is unchanged: still a warning, so a
project carrying placeholders keeps validating and only --strict fails.
Every new guard is mutation-checked: dropping `TODO` kills 3 tests, unmasking
detection kills 2, unmasking the line locator kills 3, unmasking the header
search kills 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(validate): read the marker boundary in any script, not just ASCII
Review found `\b` reading `TODOé` and `TBD١` as a marker followed by
punctuation, because `\b` only knows ASCII word characters. A Purpose is prose
and prose is not always Latin script, so the rule that a longer word beginning
with those letters is not a marker has to hold in any script.
The lookahead rejects letters, digits, combining marks and `_`, and nothing
else, so `TODO:`, `TBD -` and `TODO(owner):` are still the marker they look
like. Held in both directions: loosening it back to `\b` kills 1 test,
tightening it to reject punctuation kills 4.
Also reworded a task line that opened with `#1670`, which markdownlint reads as
a heading missing its space.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(validate): locate the matched purpose placeholder
* docs(validate): remove trailing task whitespace
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Clay Good <hi@claygood.com>
|
||
|
Clay Good
|
18688c8b27 |
fix(archive): never dead-end a capability retirement (#1699)
* fix(archive): never dead-end a capability retirement A change whose delta removes the last requirement a capability has rebuilds the main spec empty, which can never validate. Archive already knows retiring is the fix and names the `retire_capabilities: true` marker that authorises deleting the spec - but only when the marker is the single thing missing. If the spec also holds a line the merge cannot account for (a `## Notes` section, a comment under a requirement - both ordinary), that hint was suppressed, and the hint that names such lines only spoke to authors who had already set the marker. Neither fired, so the archive aborted on "Spec must have at least one requirement" with no guidance at all: the exact dead end the marker exists to close. Archive now names the blocking content in that case. It deliberately does not name the marker there - adding it would not have let this run through, and the marker is only ever named when it really is the one thing missing. Once the content is resolved, the rerun names the marker. Closes #1696 * fix(archive): harden the blocked-retirement abort Three follow-ups to the same message. The blocking lines are authored spec content printed verbatim to a terminal, so they now get the treatment `describeChangeName` already gives a change directory name: control characters replaced, since a raw CR could forge a line of its own and an ESC could redraw the screen. Each line is bounded too - one very long line would push the way out of the abort off the reader's screen - and the cut counts code points so it can never leave half a surrogate pair. Both the declared and undeclared branches share the helper, so the marker-declared abort that shipped with #1484 is hardened with it. The wording no longer claims retiring is "the way through". It is not, in the one case this fires on that has a live requirement hiding in a second `## Requirements` section: merging the sections fixes that spec without deleting anything. `openspec/specs/cli-archive/spec.md` records the behavior change - the blocking lines are named whether or not the marker was declared, and the marker is still named only when adding it would let the archive through. * refactor(archive): drop a helper the revised wording made single-use The marker sentence is said in one place again, so it goes back inline rather than through a function that now has one caller. Also corrects the comment above `emptiedByThisRun`: retiring is not the only fix in every case it covers, which is exactly why the message stopped saying so. * docs(openspec): record the change as a delta, not a direct spec edit Both conventions exist in this repo's history, but the two most recent behavior fixes (#1609, #1616) carry an `openspec/changes/` delta rather than editing the main spec in place, which is also the workflow this project asks of everyone else. The delta reproduces the whole Capability Retirement requirement, so archiving it drops no scenario. Verified by archiving into a scratch copy of `openspec/`: the merged main spec differs from today's by exactly the three added bullets. * fix(archive): report an unhonorable marker alongside the blocking content An author who set `retire_capabilities: yes-please` believes they have authorised the deletion. Clearing the blocking content first, only to then learn the marker was never read, is two aborts for one mistake. The abort still never invites the marker to be added while content blocks the retirement - it only reports the one already there. The spec delta records that distinction, which the old bullet ("say nothing about the marker") did not draw. * style(archive): use one sentence for an unhonorable marker in both aborts * fix(metadata): strip control characters from an unhonorable marker reason Every reason a boolean change-metadata marker gives quotes something the author wrote - a schema name, a parser message carrying one, a filesystem error carrying a path - and two commands print it straight to a terminal. A schema name carrying a raw ESC, with the marker set, put that ESC on screen through `openspec archive`; `openspec validate` prints the same reason. Fixed at the source in `readBooleanMarker` rather than at either call site, so no consumer has to remember. The reason still quotes the name recognisably; only control characters are replaced. Reported by CodeRabbit on #1699. Pre-existing on main, and this PR would have added a second place it reaches the terminal. * test(archive): fix a comment left behind by the reworded abort |
||
|
Clay Good
|
c747ed1f34 |
feat(init): add language option (#1685)
* feat(init): add language option * fix(init): harden language configuration * fix(init): fail when language config cannot be written |
||
|
Clay Good
|
fc0fec1250 |
fix(feedback): keep full reports in issue bodies (#1653)
* fix(feedback): keep full reports in issue bodies * fix(feedback): preserve report formatting |
||
|
|
8364428661 |
fix(schemas): honor canonical root selection (#1616)
* docs(openspec): propose schemas root selection fix * fix(schemas): honor canonical root selection * test(schemas): assert complete JSON schema shape * docs(stores): drop view from the cwd-only, no --store list view already accepts --store <id> (registered in src/cli/index.ts), so listing it among the commands that act on the current directory only was incorrect. Remove it; templates and the deprecated noun forms remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: regenerate skills and parity hashes after rebase onto main Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
804427b6ff |
fix(telemetry): suppress first-run notice in --json mode (#1609)
* fix(telemetry): suppress first-run notice in --json mode The first-run telemetry disclosure notice was written to stdout from the global preAction hook. On a user's first-ever command with --json this polluted stdout and could break JSON parsers. Read the executing command's --json flag (actionCommand.opts().json) and, when set, skip the notice and leave noticeSeen unset so the disclosure is deferred to the first later non-JSON run rather than lost. Spinner suppression, new-change --json output, and structured JSON errors already landed on main (#960, #1190); this closes the one remaining stdout writer in --json mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden: detect --json from argv to cover all invocation forms The preAction guard read actionCommand.opts().json, which only sees a declared leaf option. That missed two supported --json forms that emit a single JSON document to stdout: - openspec store --json (permissive group reads --json from residual args; never declares the option, so opts().json is undefined) - openspec workset --json <sub> (--json on the parent group, consumed before the leaf; leaf opts().json is undefined) Both would still print the first-run telemetry notice ahead of their JSON. Detect --json from process.argv instead: it covers leaf, parent, and residual-arg forms uniformly. Suppressing is always safe (the disclosure defers to the next non-JSON run, never lost), so a broad argv check is the correct, conservative signal. Also add a direct assertion that noticeSeen stays unset after a silent run, and note the pre-existing raw-stdout commands (completion generate, config get/path, __complete) as out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: derive --json from parsed command state + regression test Replace the process.argv check with isJsonRun(command), an exported pure helper that reads Commander's parsed state: optsWithGlobals().json (leaf and parent-group forms) OR command.args (residual --json on permissive bare groups like store). This is tied to the actually-parsed command rather than raw args, and — unlike process.argv — is unit-testable in-process. Add test/core/cli-is-json-run.test.ts: a synthetic program reproducing all three registration patterns proves isJsonRun returns true for status --json, store --json, workset --json list, and workset list --json, and false otherwise. This locks in the store/workset coverage against future regressions (an e2e test can't: telemetry is disabled under CI, so the notice never fires there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): qualify first-command notice scenario as non-JSON The generic 'First command execution' scenario asserted the notice displays on every first command, contradicting the JSON scenario that says it does not. Qualify it as 'without --json' so the required behavior is unambiguous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
3d0701f871 |
fix(workflows): preserve nested spec paths (#1508)
* fix(workflows): preserve nested spec paths * fix(workflows): key conflicts by capability path * fix(workflows): preserve full paths in examples * fix(workflows): clarify nested path inputs * test(workflows): align parity hashes after rebase |
||
|
Clay Good
|
afea111cd4 |
fix(status): clarify planning completion (#1505)
* fix(status): clarify planning completion * test(status): cover skipped planning artifacts * fix(workflows): gate archive guidance on implementation * fix(status): clarify human completion message * fix(status): make completion guidance stage-neutral * test(status): align parity hashes after rebase |
||
|
Clay Good
|
ece8660d44 |
fix(validate): allow non-English requirements (#1502)
* fix(validate): allow non-English requirements * test(validate): cover non-English change deltas * test(validate): distinguish missing bodies from guidance |
||
|
Clay GoodandClaude Opus 5
|
521ee33e6e |
feat(archive): let a change retire a capability it empties (#1484)
* fix(archive): retire a capability when a change removes its last requirement
A delta whose REMOVED entries cover every requirement rebuilt the main spec
empty, and an empty spec fails validation ("Spec must have at least one
requirement"), so the archive aborted with no way forward. Pre-deleting the
main spec did not help: the delta was then treated as a create and landed on
the same empty spec.
Archive now treats an emptied capability as retired. It deletes the
capability's spec.md and any directory the deletion leaves empty, stopping
short of the specs root, and reports the removals in the totals. Nothing is
deleted unless this run actually removed a requirement, so a re-applied or
already-synced delta still leaves the file alone.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): decide retirement from the validator and contain the deletion
Adversarial review found the original rule unsound. It retired whenever no
canonical `### Requirement:` blocks were left, but the validator counts
requirements differently: MarkdownParser accepts any `###` heading under
`## Requirements`, while the delta block parser indexes only canonical headers
and sweeps the rest into the preamble, which survives into the rebuilt spec. A
strict-valid spec could therefore be deleted on an archive that previously
succeeded. Retirement is now decided by putting the rebuilt spec to the
validator and retiring only when its sole error is that it has no requirements,
which makes "this spec could not have been written anyway" true by construction.
Also fixed:
- The directory prune walked string prefixes, but path.resolve does not resolve
symlinks and readdir/rmdir both follow them, so a symlinked capability
directory let it delete directories outside the repository. Pruning is now
bounded by real paths and refuses to descend through a symlink.
- A spec that was already requirement-less and lost nothing this run is no
longer skipped past validation; it aborts exactly as it did before.
- Deletions are deferred until every spec write has succeeded, so a later
failure cannot leave a spec already deleted.
- Retirement is recorded in `warnings`, naming any other sections the deleted
file held, so JSON consumers and humans can both see what went.
- Totals carry every applied operation; a rename applied on the way to the
removal was being dropped.
- bulk-archive guidance, the sync/archive skill specs, and the docs that
described archive as never deleting a spec.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): close the retirement gaps a second review round found
Five adversarial reviews, mutation testing and CodeRabbit went at the reworked
retirement. The findings, all verified by repro before fixing:
- The archive-name collision check ran AFTER the spec merge, so archiving twice
in one day deleted the capability's spec and then failed, leaving the change
unarchived and the file gone. The destination depends only on the change name,
so it is now settled before any spec is written or deleted - which also closes
the same, older window for ordinary writes.
- `--no-validate` retired too, but the whole safety argument is the validator's
verdict, and that path produces none. It now writes the spec exactly as it did
before this feature existed, leaving no exception to the claim that nothing
previously working changes.
- The validator can be talked out of seeing a requirement: a stray
`### Requirements` under Purpose captures its section lookup, so a spec still
holding a real requirement reported "no requirements" and was deleted. Any
`###` heading left under `## Requirements` now vetoes retirement outright - a
reader is not fooled by the stray heading even when the parser is.
- A dangling symlink made `update.exists` false (`fs.access` follows links,
`unlink` does not), skipping the "removed something this run" guard: a run that
removed nothing deleted an entry and reported a removal. The no-target case is
now an explicit branch that never deletes, instead of an ENOENT probe.
- `findOtherSections` reported `## ` headings that were inside HTML comments and
listed duplicates; it now masks comments like every other structural scan here
and dedupes. The warning also names the `## Purpose`, which the deletion always
takes, and the resolved path when a symlink puts the file outside the repo.
- A failed `unlink` surfaced a bare errno; it now says what was being attempted
and what to do.
Tests grew from 19 to 33, killing every surviving mutant the review found:
deferral proven against a failing write (not just a failing validation), the
warnings payload, the already-gone path's output, multi-level pruning, the
`+ path.sep` boundary, a symlinked specs root, two retirements in one archive,
and `isRetirableSpec` unit-tested directly - including the two-error shape that
proves `every` rather than `some`.
Agent guidance, the three living specs and the docs now state the same
conditions the CLI applies, so a sync agent cannot delete a spec archive keeps.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): make the write-failure test platform-neutral and the path note meaningful
Windows CI and CodeRabbit each caught one:
- `chmod 0o555` is not a write barrier on Windows, so the test that proves
deletions are deferred until every write succeeds never failed a write there:
the archive completed, the spec was retired, and the assertion blew up. It now
puts a directory where the second spec's file belongs, which fails the write on
every platform. Verified it still kills the reordering mutant.
- The "resolved to" note compared a canonicalized path against a merely resolved
one, so any symlinked ancestor - the platform's own /var -> /private/var is
enough - decorated an ordinary retirement with a path that says nothing. It now
fires only when the spec really lived outside the specs tree, which is the fact
the nominal path hides. Both directions are pinned by tests.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): make the residual-heading veto position-independent
A third review round, scoped to the code the earlier rounds never saw.
The veto that is supposed to stop a retirement deleting hand-written content
only worked when that content sat ABOVE the first requirement. `parts.preamble`
is by definition the text before the first `### Requirement:` header; anything
after the last one belongs to that block's raw and is discarded with it, so the
rebuilt-body scan never saw it. Identical content, different position: one
aborted, the other was deleted silently. The veto now reads the original
Requirements section - preamble plus every block - so position does not matter.
Also:
- `realpath` follows a symlinked `spec.md` but `unlink` removes the link, so the
warning declared it had deleted a file outside the repo that was still there.
The note is now skipped when the target is itself a symlink.
- `findHeadings` masked HTML comments before code fences, so an unterminated
`<!--` inside a fenced example blanked the rest of the document and truncated
the very list of sections the deletion was reporting. Fence first, then
comments.
- Moving the collision check before the merge widened the window between it and
the move, where a claimed destination surfaced as a raw ENOTEMPTY and degraded
to `archive_error`. `moveDirectory` now reports that as `archive_target_exists`,
the same diagnostic the pre-flight check gives.
And a simplification the review asked for: the overlapping `retirable` /
`deletes` / `retired` booleans are now one `decideSpecOutcome()` returning
'write' | 'delete' | 'skip'. Behavior is identical - same clauses, same order -
but the fourth state that existed only as a comment is now a visible return.
Both guards were kept: the review constructed inputs where each is the sole
thing preventing a data-losing delete.
Two tests the review found wanting are gone or rewritten: one killed no unique
mutant, and one assertion straddled two editable message fragments and could
have gone vacuously true.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(archive): canonicalize both negative path assertions
CodeRabbit caught that `expect(warnings).not.toContain(shared)` passed
vacuously: on macOS the temp root lives under /var, whose realpath is
/private/var, so the warning would print a form the assertion never compared
against. The sibling assertion on `tempDir` had the same flaw.
Both now canonicalize first, and both were confirmed to fail against a mutant -
dropping the lstat guard, and forcing the resolved-path note on - which neither
did before.
Closes #1302
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): move a retired capability's spec into the archive instead of deleting it
Retiring a capability was the first case where archiving deleted a file
under `openspec/specs/`. Nothing in the repo had ever removed spec content
before, so the blast radius of a wrong verdict was a lost file with only
the reflog to recover it.
The spec now moves instead. It is staged into the change directory, which
the archive step renames onto the archive path moments later, so it comes
to rest at `<archive>/retired-specs/<capability>/spec.md` beside the
proposal and tasks that retired it. `git` records a rename, and bringing a
capability back is a `git mv` from the archive.
Staged into the change rather than written to the archive path after the
move, because the archive path must not exist yet and the ordering is
safer: if a later step fails, the spec sits in a change that is still
active and a rerun carries it through, versus stranding the live specs
tree without a spec it still needs.
A symlinked `spec.md` is copied by content and its link removed, rather
than moved: relocating the link itself would archive a relative path that
no longer resolves from where it landed. A spec already staged by an
earlier aborted run is never overwritten - it is the only copy once the
live one moves.
The retirement verdict, its guards, and the deferral until every write has
succeeded are all unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): clean up staging directories when a retirement move fails
The staging directories are created before the move, so any failure left an
empty `retired-specs/<capability>/` behind. That folder then rode into the
archive with the change, where it reads as a retirement that never happened -
a spec was supposedly retired here, and there is nothing to show for it.
The failure path now prunes back up to the change directory. Only empty
directories go, so a capability the same run already staged next to the
failing one is untouched, and the guard that refuses to overwrite a staged
spec still stops at a non-empty destination.
Both cases are covered by tests that fail without the prune: a dangling
symlink is the reproducible post-staging failure, since lstat sees a file and
the copy then follows the link and finds nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(archive): say "moved" where the retirement path still said "deleted"
Three leftovers from the deletion version: the `residualRequirementHeadings`
comment, `pruneEmptyDirs`'s `mainSpecsDir` parameter - now a boundary that is
the change directory on the cleanup path, not the specs root - and a sentence
in writing-specs.md that used "deleted" for the requirement and then again for
the file, two lines apart.
No behavior change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): roll back a staged copy when the live spec cannot be removed
Both non-atomic retirement routes - a symlinked main spec, and the
EXDEV/EPERM rename fallback - copy the spec into staging first and remove
the original second. A copy that landed before an `unlink` that failed left
the spec in TWO places, and the staged one then tripped the "already
staged" guard on every rerun. The error told the caller to rerun the
archive, and the rerun could never work.
Reproduced at the previous head with a symlinked `spec.md` in a read-only
capability directory: `copyFile` succeeded, `unlink` returned EACCES, and
both copies remained.
The failure path now deletes the destination this attempt created, so the
capability is left exactly as the attempt found it and the rerun works. The
rollback is gated on a flag set only after the destination is proven free,
so a spec staged by an EARLIER run is never the thing removed - the
overwrite guard still fires ahead of it and rolls nothing back. A partially
written copy is cleaned by the same call.
The message no longer promises more than it delivers: it reports that the
spec is still in place, or names the leftover copy when the rollback itself
failed.
Regression tests cover both routes and assert the rerun succeeds, not just
that the copy is gone. Both fail without the rollback. The cross-device
route injects EXDEV, which cannot be provoked inside one temp directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(archive): run the rename-fallback rollback case on Windows too
The two post-copy rollback cases shared one `skipIf(win32)`, inherited from
the symlink case, which needs privileges Windows does not grant by default.
The rename-fallback case uses regular files and spies only, and the sibling
errno it stands in for - EPERM - is the Windows case, so skipping it there
left that route untested on the platform that produces it.
Skipping is now per-case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): claim the retirement destination atomically
`fs.access` followed by a write is not an ownership claim. Two concurrent
retirements both saw the destination free and both set `destIsOurs`; one
moved the spec into staging, and the other - equally convinced the file was
its own - rolled it back out. The source and the staged copy both ended up
gone. Reproduced at the previous head in 36 of 40 iterations.
The claim and the content now arrive in one syscall: `copyFile` with
`COPYFILE_EXCL` fails with EEXIST rather than overwriting, so exactly one
caller can ever own the path. That is also the check that refuses to
clobber a spec an earlier aborted run staged, now decided atomically rather
than by a separate look beforehand.
The losing caller fails two ways, and both used to destroy the winner's
file. EEXIST is the obvious one. ENOENT is not: `copyFile` opens the source
first, so a loser that arrives after the winner removed the source fails
before creating anything - and treating that as "a partial copy of mine"
unlinked the winner's file. Neither errno now claims ownership. Fixing only
EEXIST left 4 of 40 iterations still losing both copies.
Copying rather than renaming is what makes the claim possible: `rename`
overwrites silently on every platform, so it cannot tell "I created this"
from "I destroyed someone else's". It also crosses filesystems, which
retires the EXDEV/EPERM fallback, and reads a symlink's content rather than
moving the link - so the two routes collapse into one shape.
Regression asserts the invariant over 25 rounds: exactly one caller
retires, the spec survives once and intact, and the source is gone. It
fails against the old access-then-write shape.
Not crash-safe, which is a weaker promise and now documented: a process
killed between the copy and the unlink leaves the spec in both places, and
the next run refuses rather than guessing which to keep.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): take retirement ownership from an exclusive create, not an errno
Claiming the destination with `copyFile(..., COPYFILE_EXCL)` closed the
concurrent race but kept reading ownership out of a failure code, and that
cannot be made correct however the errnos are partitioned. An errno says
what went wrong, not what was created: a source-side EACCES is
indistinguishable from a partial copy of our own, so the cleanup deleted a
recovery copy an earlier run had staged - the last remaining copy of a spec
whose live file could not even be read.
Reproduced at the previous head with an unreadable `spec.md` and a
pre-existing `retired-specs/legacy/spec.md`: the staged file was destroyed.
Ownership now comes from `open(dest, 'wx')`. O_CREAT|O_EXCL returns a
handle exactly when it created the file, so the question is answered by the
syscall instead of inferred afterwards, and every failure path leaves the
flag false. EEXIST remains the refusal that protects an earlier run's copy,
now decided by the same operation. Content is written through the claimed
handle, as bytes, and the handle is closed before any rollback so Windows
can unlink it.
The regression uses real mode bits, skipped on Windows and under root: the
defect was a source-side errno being read as proof about the destination,
and stubbing a JS-level read cannot reproduce it, because the copy it has
to fool never went through one. Verified it fails against the errno-
inference version.
All three findings on this path now hold together: the pre-existing copy
survives, 0 of 120 racing iterations lose a spec, and a post-copy unlink
failure still rolls back and reruns cleanly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): keep the staged copy when the source is already gone
The rollback exists for a copy that landed while the source survived - the
two-places state that blocks every rerun. It must not fire once the source
is gone: at that point the staged copy holds the only remaining content, and
the end state the retirement was reaching for is already reached.
An external delete landing between the read and the unlink produced exactly
that, and the rollback destroyed the spec outright - `retired: false`, no
live file, no staged copy, content gone.
`unlink` returning ENOENT is now a success rather than a failure to roll
back. Every other errno still throws: the source is still sitting there, and
leaving the staged copy beside it is the state that blocks a rerun.
Found reviewing the finished path rather than reported - the same class as
the three review findings before it, all of them the rollback reaching a
copy it should not have. Regression verified against the unconditional
unlink.
Also corrects a doc line that still credited the copy with claiming the
destination; the claim is the exclusive create.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(archive): gate retirement on a declared marker, drop retired-specs/
Reworks #1302 to follow the design that already exists instead of adding one.
The move-into-the-archive approach introduced two things OpenSpec did not
have: capability retirement as a lifecycle state, and `retired-specs/` as an
on-disk convention no schema declares - which a future unarchive command
would have to know about. Its whole justification was preserving content that
two existing mechanisms already preserve: the archived change carries the
delta naming every REMOVED requirement with its Reason and Migration, and git
carries the file. The approach even conceded the point by advertising `git mv`
as the recovery path.
The issue itself proposed neither. It asked for a delete, or an explicit
retirement marker. This does both: archive deletes the emptied spec, and only
when the change declares `retire_capabilities: true` in its `.openspec.yaml`.
`skip_specs` is the precedent. The marker reader is the same function,
parameterised by key, so the two can never drift apart on what counts as
honorable metadata - a marker in unparseable YAML, or one whose schema does
not load, is not a marker in either case. An explicit `false` is not an
unhonorable marker, it is simply undeclared.
Without the marker nothing changes: the unwritable spec aborts the archive
exactly as before, except the abort now names the marker as the way out - and
says nothing about it when retiring would not have made the spec writable
anyway, so it never sends an author after the wrong fix. Applying REMOVED
already deletes requirement content from a main spec, so deleting the spec
once nothing is left is that same operation carried to its end.
Every guard survives: the validator's verdict, the residual-heading veto,
something-removed-this-run, and never under --no-validate. What goes is the
exclusive claim, the rollback, the staging directories, and the four
data-loss windows they created across four review rounds. Net 307 lines
smaller than the move.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: regenerate parity hashes over the merged sync-specs template
#1482 and this branch both edit the sync-specs template, so the merged
template needs its own hash - neither side's committed value describes it.
* docs(archive): correct claims the redesign left false, and bump to minor
Review findings, all verified before fixing:
- `pruneEmptyDirs`'s doc claimed "two callers, two boundaries", naming the
change directory as the second. That was the staging walk from the move
design; there is one caller. The boundary stays a parameter, and the comment
now says why.
- Three comments still described the retirement as moving the file somewhere.
It deletes it.
- The sync skill told agents the retirement condition includes "no other
`###` headings or prose" and then claimed "openspec archive draws exactly
these lines". It does not draw the prose line: a main spec with loose prose
under `## Requirements` retires and is deleted, and the prose is not named
in the warning, which reports `## ` sections only. Verified against the
built CLI. The condition now states what the CLI enforces, and the template
tells the agent to read that prose back to the user, since the CLI cannot
see it for the agent.
- `docs/concepts.md`'s `.openspec.yaml` field list omitted the new marker -
the one place a user goes to learn what that file may hold.
- `docs/cli.md`'s `--no-validate` row did not mention that it disables
retirement, though the row two lines down documents retirement.
- Bumped patch -> minor. `skip_specs`, the marker this one mirrors, shipped as
a minor change in 1.7.0 (#1399); this adds a metadata field and an archive
outcome on the same footing.
No behavior change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): refuse to retire a spec with a second Requirements section
Four review agents ran against this branch. Two data-loss findings, both
reproduced before fixing.
1. A spec with a SECOND `## Requirements` section was deleted even though it
passed `validate --strict` with zero issues, and the report named only
`Purpose`.
`extractRequirementsSection` binds to the FIRST `## Requirements`, so
everything after it rides through the merge untouched: the residual-heading
veto never sees it, `findOtherSections` filters it out by title, and the
validator's own section lookup stops there too - which is why a second
section holding a `SHALL` with a scenario reads as valid and then died with
the file. The earlier round made that veto position-independent WITHIN the
section; this is the same evasion one level up.
Retirement is now refused outright for such a spec, so the archive aborts as
it did before #1302. The abort's marker hint takes the same conjunct, so it
never advises a marker that would not have helped.
2. The recovery line promised `git checkout HEAD -- <path>` unconditionally,
and the path was wrong twice over. Verified failures: an UNTRACKED spec -
the ordinary case, since an earlier `openspec archive` creates the main spec
and nobody has committed it yet - is deleted and the printed command errors,
so the file is gone for good; under a store-selected root the nominal
`openspec/specs/...` path does not exist in the caller's repo; and a
symlinked capability directory puts the file somewhere else entirely.
The line now names the path the file actually lived at, and is phrased as
the condition it really is rather than a promise archive cannot keep.
Regressions for both, plus the three fail-closed branches on the deletion
authorisation path that no test observed: a marker in unparseable YAML, and a
failing unlink. Each verified against a mutation - removing the veto, restoring
the unconditional promise, swallowing the unlink error, and honouring a marker
in broken YAML each fail their test.
Also pins the sync skill's retirement guidance by content rather than by golden
hash, since a hash proves only that it matches its source.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(archive): note that retiring a capability strands an in-flight MODIFIED
A capability's main spec is the base #1482's scenario-loss check compares a
MODIFIED block against. Retire the capability and that check goes silent by
design (a missing main spec is the sister-change-in-flight case), so a change
that modifies the retired capability keeps validating clean and then refuses to
archive with "target spec does not exist". Nothing is lost - there are no
scenarios left to drop - but nothing connects the refusal back to the
retirement either, so the changeset says it up front.
Found by testing this PR against the three that merged into main today.
* fix(archive): veto retirement on any heading past the merged section
A sixth data-loss defect, from a second round of review agents. Reproduced
before fixing: a `validate --strict`-clean spec was deleted with a live SHALL
requirement in it, and the report named only "Purpose".
The cause is a mask disagreement. `extractRequirementsSection` - the function
that decides where the Requirements section ENDS - masks fenced blocks only.
`findHeadings`, which both retirement vetoes were built on, masks HTML comments
as well. So a multi-line comment holding a `## ` line terminates the section for
the merge while being invisible to the scan that had to notice it: everything
below became a tail no guard could see. The round-five guard counted `##
Requirements` headings, which the same trick skins straight past.
The veto is now asked of the tail itself - does anything `###`-shaped sit past
the boundary the merge actually chose - read with the fence-only mask, so it
answers the question whatever produced that boundary. That subsumes the
multiple-Requirements-sections case it replaces and every comment variant.
Also from this round:
- The recovery command is derived from the path that was unlinked, not rebuilt
from the capability id. On a case-insensitive filesystem the id and the real
directory differ in case, git is case-sensitive, and the printed command was
one git rejects.
- An absolute recovery path now says which checkout to run it in - for a
selected store, the file is not under the directory archive was run from.
- A declared marker refused by the tail veto says why, instead of dropping the
author who did what the docs asked back into the bare #1302 abort.
- Corrected "draws exactly these four lines" in the sync skill, a claim added
two commits ago that was false when written: the CLI checks two more.
Both regressions are mutation-verified. Reverting the veto to the narrow
multi-section count fails the comment-boundary test.
One reported finding was NOT actioned, because its premise does not hold: a
residual `###` heading INSIDE the section still counts as a requirement to the
validator, so that spec is valid and simply gets written - there is no silent
dead end there to explain.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(archive): say the marker needs the schema key beside it
`.openspec.yaml` requires `schema:`, so a file holding only
`retire_capabilities: true` is not honorable metadata and the marker does
nothing. The docs and the abort hint both described adding one line, which sends
anyone creating that file from scratch into a dead end. The message did explain
itself once you were there ("schema: Invalid input: expected string, received
undefined"), but it should not need to.
Pre-existing shared behavior - `skip_specs` has the same requirement - so this
is wording, not a behavior change.
* chore: merge main (#1483) and keep both archive test suites
#1483 landed while this branch was in review. Three conflicts:
- `archive.ts`: one import line, both sides' imports kept.
- `skill-templates-parity.test.ts`: hash constants, resolved by key-union and
then regenerated from the merged source, which is the only authority once two
branches have edited the same template.
- `archive.test.ts`: the trap this repo documents. Both branches appended a
DIFFERENT describe block at the same place - `capability retirement (#1302)`
here, `non-interactive prompts (#1479)` on main - so taking either side would
have dropped 16 or 133 tests with a green suite. Both are kept.
The conflict boundary also cut the retirement describe's last two closing
braces, which `tsc --noEmit` accepted and only esbuild caught as "Unexpected
end of file". Restored by brace-balance against both parents.
Verified after: every one of main's 91 archive titles and 19 parity titles is
present, #1483's describe still holds its 16 tests, and its own non-interactive
repro still behaves as it does on main.
* fix(archive): only print a recovery command that would actually run
Both blockers from the last review.
The recovery line offered `git checkout HEAD -- <path>` for every retirement,
including ones where the file never lived under the directory archive was run
from: a selected store, or a symlinked capability directory. Git rejects an
absolute path from a different worktree however it is quoted, and an unquoted
path containing a space splits when pasted - a real store path reproduced both.
Those cases now say where the file was and leave recovery to the reader, rather
than handing them a command that cannot work. The ordinary case still gets the
command, quoted when the path needs it, via the portable quoting #1483 already
established for change names.
And `openspec/specs/specs-sync-skill/spec.md` still authorised deletion from the
four original conditions, with no mention of the tail-heading veto the CLI
gained - so the living spec permitted something the code refuses. It now carries
that condition, and a parity test pins it in the generated guidance so the two
cannot drift apart again.
Both fixes are mutation-verified: restoring the unconditional command fails the
escaped-path regression, and rewording the veto out of the template fails the
guidance test.
* fix(archive): retire only what the merge can account for
Replaces the tail-heading veto with a rule that does not read Markdown at all.
Six review rounds each found a different way to dress content so a heading scan
would miss it: a second `## Requirements` section, a `##` inside an HTML comment
ending the section early, a three-space indent, a setext underline. Every fix
was another regex approximating a parser, and every round found the next skin.
`extractRequirementsSection` has already split the file into the parts this
merge understands. So instead of asking "does anything here look like a
requirement" - a question a regex and a renderer answer differently - the guard
now asks where content ended up: anything non-blank between the `## Requirements`
header and the first requirement, or after the section ends, is content the merge
carried through without understanding, and a retirement that would delete the
file is refused. There is no second opinion to disagree with the first, because
there is no second parse.
The in-block heading guard stays, and its comment now says why: a `###` heading
that is not a requirement header is absorbed into the block above it, so it
never reaches the preamble or the tail. Folding that into the rule above needs a
parser that ends a block at any `###` heading, which belongs in the parser.
This narrows the feature: a spec carrying an authored section beyond Purpose can
no longer be retired automatically. That is deliberate. The abort names the
lines that stood in the way, and deleting a file whose contents this merge
cannot enumerate is exactly the case a person should decide.
Depends on #1490 for indented requirement headers, which are swallowed by the
block parser before any of this runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): account for the whole spec, not two slices of it
Defect eight, same class as the seven before it. The guard asked where content
landed, which was the right question, but it only read two of the five slices
`extractRequirementsSection` produces: the preamble and the tail. Content simply
moved somewhere nobody looked.
Reproduced: a hand-written migration runbook and a table written below a
requirement's scenarios live inside that requirement's `raw` - the block runs to
the next header the parser RECOGNISES - so removing the requirement deleted them,
and the report said "Its section(s) went with it: Purpose". Not silence: a false
statement the reader can act on. The same hole covered anything written above
the `## Requirements` section. And because the abort hint is gated on the same
checks, an unmarked run RECOMMENDED adding the marker that destroys it.
The audit now covers the whole file. Expected: the title, the `## Purpose`
section, the `## Requirements` header, and inside each block a requirement's own
parts - its header, its statement, its scenarios' bullets. Every other non-blank
line is reported and refuses the retirement. That folds in the `###`-heading
guard, which was a patch on this same leak using the technique the rewrite was
meant to abandon.
One reported shape is deliberately not a case: prose between `## Purpose` and
`## Requirements` IS the Purpose body, since the section runs to the next `##`,
and the warning already names Purpose as going with the file. The test says so.
Both regressions fail against the two-slice version.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(specs): keep content absorbed into a removed requirement
A requirement block's `raw` runs to the next header the parser RECOGNISES, so a
heading it does not - one indented by the 0-3 spaces CommonMark allows, or a
plain `### Notes` - is absorbed into the requirement above it. Removing that
requirement deleted the absorbed content with it. Silently: nothing counted it,
so nothing warned, and the spec left behind still validated.
Reproducible on main with no marker and no capability retirement involved.
Anything from the first `#`/`##`/`###` heading after a removed block's own
header is now kept in place. `####` is excluded deliberately - a requirement's
`#### Scenario:` headings are its own and go with it.
This replaces an earlier attempt on this branch that widened every heading
pattern in both parsers to accept indentation. That was wrong twice over. It
reclassified content, so a spec that was valid became invalid - commented-out
and indented examples started parsing as real requirements, taking `list` from
1 requirement to 3. And it did not even fix the bug: moving the line out of the
block only meant the reconstruction dropped it at a different step, since
`rebuilt` is assembled from `before + header + kept blocks + after` and anything
skipped is simply gone.
So nothing is reclassified now. An indented heading is still not a requirement,
exactly as before; it just survives its neighbour's removal, which is all this
ever needed to do. The repo's own corpus produces byte-identical `list`,
`validate --specs --strict` and `validate --changes --strict` output.
Four regressions, each mutation-verified: removing the salvage fails the three
absorbed-content cases, and counting `####` as a boundary fails the scenario
case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(specs): keep notes absorbed into a modified or removed requirement
A slow audit of the previous commit found the fix covered one of three paths.
A requirement block absorbs anything below it that the parser does not read as
a new header - a note indented by the 0-3 spaces CommonMark allows, say - so
that content rides inside the block. The previous commit salvaged it when the
requirement was REMOVED and missed MODIFIED entirely: that path rebuilds the
block from the delta, which never carried the note, so it was dropped exactly as
before. Verified against the real CLI: main loses it on both paths.
RENAMED was the opposite trap. It rewrites the original block's header line in
place, so the note is already there - but it also deletes the original key from
the block map, which made the requirement look REMOVED to the salvage and
produced a duplicate. Tracking which operation applied is therefore not reliable
at this point in the merge, so the salvage now asks the assembled result
instead: re-insert a note only when nothing else in the rebuilt section already
carries it. That is correct for all three paths by construction.
Salvaged content also keeps its position now, next to the requirement it was
written beside, rather than being appended at the end of the section.
Six regressions, three of them mutation-verified against this logic: never
re-inserting fails four, always re-inserting duplicates on rename, and appending
at the end loses the position.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(specs): decide salvage by identity, not by matching text
Another audit pass, another defect in my own fix.
Deciding whether a note survived by searching the rebuilt section for its text
is wrong when two requirements carry the same note: the first copy is found,
and the second is dropped. Reproduced - two removed requirements each followed
by an identical `### Notes`, one note destroyed.
Survival is a question about the block, not about text. An untouched block is
the same object the parser produced and still carries its note; a replaced one
is a different object and does not. The RENAMED path previously blurred that by
copying the whole raw, so it now carries only the requirement's own lines and
the salvage puts the note back like every other path. With every replacement
uniformly lacking the tail, `replacement !== block` decides it exactly, and no
text is compared at all.
Four properties, each mutation-verified: matching text instead of identity
loses the duplicate note, always re-inserting doubles an untouched block's note,
letting RENAMED keep the tail doubles it on rename, and counting `####` as a
boundary severs a requirement from its scenarios.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(specs): warn when a note absorbed into a requirement will be deleted
An adversarial review found the previous approach was worse than the bug.
Salvaging the "foreign tail" out of a requirement block relied on a positional
rule: everything after the first heading-shaped line is not the requirement's.
That is not true. A `# comment` inside a scenario bullet, or a markdown example,
matches the same shape - and on MODIFIED the old text was then spliced back in
after the new, so the spec asserted both. The validator called the result valid,
and re-applying the same delta grew the file every time. Reproduced end to end.
It also turned a working archive into a hard abort: preserving an unindented
`### Notes` made the rebuilt spec fail validation as a scenario-less
requirement, so changes that archived cleanly on main stopped archiving, with an
error that never mentioned the note.
Measured before choosing: 3 of 742 requirement blocks in this repo contain a
heading-shaped line, and the repro shows those are false positives. Trading a
rare silent deletion for silent corruption on the most common operation is a bad
trade.
So the merge is left exactly as it was - byte-identical output, verified against
main - and the loss is reported instead. That fixes the part of the bug that
actually hurt: it was silent. A wrong warning costs a line of output; acting on
a wrong answer rewrites the spec.
Eight tests. Dropping the warning fails three; ignoring the fence mask fails
one - the fence case the previous version left unpinned.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): scope a scenario's bullets, and stop refusing ordinary prose
Defect nine, plus the over-refusal it exposed.
Every bullet counted as a scenario's own, anywhere in the block. So an
operational note bulleted below the last scenario - "IMPORTANT: escrow keys
live in the legacy vault" - was deleted with the file, on a spec that passes
`validate --strict`, and the report named only "Purpose". A scenario's bullets
run unbroken beneath its header; a blank line after them ends the run, and
bullets past that point are the author's own note.
Measuring the guard against this repo's 36 specs then showed the opposite
failure was already there: 7 of them could never be retired, almost entirely
because every fenced line inside a requirement was treated as foreign. A code
example inside a scenario is that requirement's own content - a
`### Requirement:` inside a fence is not a heading to any reader - so fenced
lines are now accounted for, as are numbered lists and a statement that opens
with inline code.
One ambiguity is left deliberately unresolved: a scenario whose bullets are
split by a blank line reads exactly like a note bulleted below it, and no
line-based rule separates them. Those specs are REFUSED, never deleted. The
abort quotes the lines, and the author moves them or removes the file by hand.
Refusing costs a message; the alternative costs the file.
Two regressions: the bulleted note must refuse, and a requirement using a
numbered list, a fenced example and an inline-code statement must still retire.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): a section is not only an ATX heading
Defect nine, from a deep adversarial pass, and it is the same species as the
eight before it: the guard decided what a section IS by one syntax while a
reader recognises three.
Once `## Purpose` was seen, every later line in the pre-requirements slice was
accepted as its body until the next ATX `##`. But a setext underline turns the
line above it into a heading, and raw HTML says so outright - a reader sees a
sibling of `## Purpose`, not more of it. So a whole authored section could sit
between Purpose and Requirements, pass `validate --specs --strict`, and be
deleted with the file while the report said only "Purpose". On main the same
archive aborts and loses nothing.
Reproduced with a `Data Migration Notes` section underlined with dashes: the
capability retired, the notes gone, unnamed. Now refused, with the lines quoted.
Two path defects from the same review, one fix: the reported path was rebuilt
from the capability id, so on a case-insensitive filesystem it differed in case
from the file actually unlinked and git rejected the printed command; and a
capability directory symlinked to a sibling deleted one spec while naming
another. `retireSpec` now always returns the path it unlinked, and archive
reports that. Whether to print a command at all is decided against the REAL
repo root, so a symlink that stays inside the repo still gets a working command
and only a path that genuinely leaves it falls back to prose.
Also pins `!skipValidation` in isolation. The existing --no-validate test passed
for the wrong reason - its fixture was blocked by the content guard - so the
conjunct itself was unpinned.
Four regressions, all mutation-verified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(archive): close remaining capability retirement gaps
* fix(archive): close final transaction safety gaps
* fix(archive): close retirement race windows
* fix(archive): preserve retirement authorization
* fix(archive): verify complete fallback copies
* fix(archive): preserve transactional safety
Reject structurally ambiguous or symlinked inputs before mutation, serialize archive claims safely, and preserve permissions during verified fallback moves.
Keep retired specs as inode-preserving backups until the archive commits, restore them on rollback, and retain any backup changed concurrently instead of deleting user data.
* fix(archive): preserve replaced claims on Windows
Add a per-claim nonce and verify stable claim contents before unlinking because Windows file IDs may not distinguish a replacement lock entry.
* test(archive): respect Windows deferred deletion
Skip the POSIX unlink-and-recreate claim simulation on Windows, where deletion of an open file remains pending until the original handle closes.
* test(archive): align symlink fixtures with path boundaries
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Clay GoodandClaude Opus 5
|
2b3d368539 |
fix(archive): tell the caller which flag to pass when archive can't ask its questions (#1483)
* fix(archive): name the flag when a prompt has no terminal to answer it An AI agent runs the CLI with stdin closed, so every confirmation `openspec archive` asks rejects with @inquirer's "User force closed the prompt with 0 null" - true, and useless: it names neither the question nor the flag that answers it, so agents abort and guess (#1479). Each confirmation now reports the same guidance JSON mode has always given for that decision point, with a pasteable command. The change picker got the opposite treatment: it swallowed the same failure, printed "No change selected. Aborting." and exited 0, reporting success for a run that archived nothing. It now exits 1 asking for a change name, matching `openspec show` and `openspec validate`. The detection is reactive - a prompt that already failed, at a stdin that is not a terminal - so piped answers, --yes, --json and Ctrl-C at a real terminal are untouched. Closes #1479 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(archive): carry the caller's flags into the suggested rerun, and honor every non-interactive signal Adversarial review of the first commit found four defects in it: - The suggested rerun dropped the flags the caller had passed. For `archive x --skip-specs` it suggested a bare `--yes` rerun, and following it merged deltas into the main specs - the exact thing --skip-specs was passed to prevent. - The change name went into that command unquoted, so a change named `my change` produced an unrunnable paste and one named `a;touch x` produced a paste that runs a second command. - The predicate keyed on stdin.isTTY alone, so a CI runner that allocates a pty still got the raw @inquirer failure - #1479 unfixed under the very signals `isInteractive()` already treats as authoritative. - A genuine Ctrl-C reaches a process whose stdin is a pipe, and that was reported as "this terminal is not interactive", telling a user who deliberately quit to rerun with --yes. The signal is now `!isInteractive()` with SIGINT excluded, so the terminal proves capability and the signal proves intent. Messages say what happened ("no answer could be read from stdin") rather than asserting a property of the terminal, which was false under MinTTY. Mutation testing found four more gaps in the tests: an unconditional `throw blocked()`, a stripped `withStoreFlag`, and either half of the predicate's `||` all left the suite green. Each now has a test, along with the flag carry-forward, the quoting, the pty-CI case, and the two prompts that had no end-to-end coverage. docs/cli.md documents the behavior without a terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(archive): close two gaps CodeRabbit found in the new guards The template guard required a space after `openspec archive`, so a regression to a bare `openspec archive` line - which blocks agents exactly as #1479 describes - would have passed it. Verified by mutation: the widened pattern fails on that edit. Expected filesystem paths in the new e2e assertions are built from path segments, per the repo's testing guideline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(archive): make the suggested rerun runnable for dashed names, stores, and Windows shells A second adversarial pass, scoped to the previous two commits, found three defects in the fix itself: - A change named `--force` was emitted bare, and commander reads it as an option however it is quoted, so the suggested command failed with `unknown option`. Such changes do archive, so the case is reachable: the name now goes behind a `--`, with the store flag kept in front of it where it is still read as an option. - The change-name-required path was the one blocked site left hard-coded, so `archive --skip-specs` with nothing to answer the picker suggested a rerun without `--skip-specs` - the same merge the previous commit set out to prevent. - Quoting was POSIX-only: cmd.exe does not treat `'` as quoting at all, and PowerShell escapes an embedded quote by doubling it, so the emitted command was wrong on Windows. Names now use double quotes, which bash, zsh, PowerShell and cmd.exe all read the same way, and a name containing something with no portable spelling (a quote, backslash, `$`, backtick, newline) names the placeholder rather than emitting a command that could expand. Two tests were pinning less than they claimed. The real-terminal cancellation test had become a duplicate of the piped one, since the SIGINT check short-circuits before the terminal is consulted; it now covers the terminal leg with a non-SIGINT failure, which is the leg nothing else guarded. The template guard iterated two identical strings and could not see an indented invocation; it now sweeps every rendered skill and command template, and both mutations were confirmed to fail it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(changeset): name the quoting form the fix actually emits Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(archive): stop quoting change names cmd.exe would expand anyway `%USERNAME%` is a legal change directory name, and cmd.exe expands it inside double quotes, so the suggested rerun `openspec archive "%USERNAME%" --yes` targets a different change than the one that was blocked. `!` has the same problem under cmd.exe's delayed expansion and bash's interactive history expansion. Both characters now fall back to the `<change-name>` placeholder, the same path a `$`/backtick name already took: a rerun the reader has to fill in beats one that silently archives something else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(archive): stop a change directory from forging its own Fix line Four adversarial reviews of this branch turned up one real defect and two guards that were not actually pinned. The human-mode message for an unanswerable incomplete-task confirmation interpolated the change name raw, and archive resolves a change by stat-ing its directory, so the name is attacker-influenceable. A newline in it added a second, forged `Fix:` line - and because `quoteChangeName` degrades the real fix to `<change-name>` for exactly those names, the forged line was the only pasteable command on screen. Control characters are now collapsed. Also pinned two mutations that passed the whole suite green: dropping `withStoreFlag` from only the dash-leading branch of `rerunCommand`, and dropping the `validate === false` leg of the `--no-validate` test - the one leg Commander actually produces. The --yes parity guard only saw invocations that opened a line, so a `$ ` prompt, a list marker or `openspec --store x archive` slipped past it. It now matches those and names the onboarding floor instead of trusting `total > 0`. Docs and spec catch up: a troubleshooting entry under the message people actually search for, and cli-archive scenarios for the unanswerable-prompt paths, including that Ctrl-C stays a cancellation. * test(archive): tokenise the --yes guard instead of pattern-matching it Accepting a global flag between `openspec` and `archive` needed nested quantifiers, and CodeQL was right to call that a ReDoS shape (js/redos, high) even in a test over our own templates. Splitting the line into tokens decides the same question in linear time - a 20k-flag line now costs ~2ms - and reads more plainly than the pattern did. Same classifications as before, plus it correctly ignores `openspec list archive`, where `archive` is an argument rather than the subcommand. * test(archive): skip the forged-Fix-line case on Windows Windows rejects control characters in a filename, so the change directory the test needs cannot be created there - which is also why the hole it covers is POSIX-only. Matches the existing `it.skipIf(process.platform === 'win32')` idiom in the suite. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 5
|
84ebc57cb3 |
fix(validate): report scenarios a MODIFIED requirement would drop (#1482)
* fix(validate): report scenarios a MODIFIED requirement would drop `openspec validate <change>` accepted a MODIFIED requirement that omits a scenario the main spec still has, even with --strict. Archive refuses to apply that block (a MODIFIED replaces the whole requirement, so the omitted scenario would be lost), so the change could pass validation, be implemented and reviewed, and fail only days later at archive time (#1477). Validate now runs the same non-mutating check against the main specs and reports each omitted scenario, naming the delta file. The comparison itself moved to the parser module so archive and validate share one implementation and cannot drift. The check is silent when the main spec file or the requirement header is absent — a MODIFIED written against a sister change still in flight is a separate condition archive gates — so validate can only report what archive already refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(validate): tighten the scenario-loss check after review Keep the moved scenario parser module-private, derive change validate's main specs root from the changes root it already resolved, replace the rename re-keying with a lookup fallback, and say at archive's call site why it does not opt in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(validate): follow rename chains when checking for dropped scenarios A delta that renames A to B and then B to C leaves C holding A's block at archive time. Walk the rename map instead of looking it up once, so the chained case reports the same loss archive refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(validate): close the gaps five adversarial reviews found Code: - An unreadable main spec was swallowed, so a change archive aborts on validated clean. Only ENOENT/ENOTDIR mean "no main spec" now; anything else is reported. - A MODIFIED naming a header the same delta renames away no longer names scenarios from the block it would not land on. That contradiction is already reported on its own, and the scenario list pointed at the wrong requirement. Guidance: the sync-specs skill told agents a MODIFIED block may carry only the changed scenario, and its format reference showed one. Both validate and archive reject that shape, so the template, the generated skill, and the golden hashes are updated to match the schema's own rule. Tests: the CLI wiring had no coverage at all — removing the argument that turns the check on broke nothing. Adds end-to-end coverage of every entry point and exit code, plus the non-strict default, a fenced scenario in the delta, an unreadable main spec, the rename-away case, and a rename cycle. Loose assertions now pin the scenario-loss issue itself. Docs: a troubleshooting entry for the new message, and the changeset says that a stale change will newly fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(validate): never turn a transient read error into a verdict The unreadable-main-spec report added in the last commit fired for any errno that was not ENOENT/ENOTDIR, which includes resource errors like EMFILE that say nothing about the file. `validate --all` reads six changes at once, so a busy process could have failed a change that is fine. Reported now only for the codes that mean the file itself is unusable and will be just as unusable when archive reads it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(troubleshooting): label the example fence (MD040) Every other fence in the file names its language. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
solanabandClay Good
|
1aa0f2abfc |
feat(init): add shared agents skills target (#1303)
Co-authored-by: Clay Good <hi@claygood.com> |
||
|
|
1637856c42 |
feat(adapters): follow the Windsurf rename to Devin Desktop (#1167)
* proposal: add devin desktop support
* feat(adapters): add devin desktop command adapter
- Create new Devin Desktop adapter for .devin/workflows/opsx-<id>.md
- Register adapter in CommandAdapterRegistry
- Export adapter from adapters index
- Update docs/supported-tools.md with Devin Desktop entry
- Add 'devin' to available tool IDs list
Devin Desktop uses the same Cascade workflow system as Windsurf,
making it a natural migration path for existing users.
* fix(config): add devin desktop to AI_TOOLS
Add Devin Desktop entry to AI_TOOLS configuration so that:
- getToolsWithSkillsDir() includes 'devin' as a valid tool ID
- getWorkspaceSkillToolIds() returns 'devin' in the list
- parseWorkspaceSkillToolsValue() accepts 'devin' as valid input
- openspec init --tools devin works correctly
This fixes validation failures where 'devin' was documented in
docs/supported-tools.md but not recognized by validation functions
that derive valid IDs from AI_TOOLS.
* fix(devin-adapter): escape implicit YAML scalars in frontmatter
Update escapeYamlValue to detect and quote implicit YAML scalars that
would be coerced by parsers:
- Booleans: true, false, yes, no, on, off
- Null variants: null, ~
- Numbers: integers, floats, exponentials, hex (0x), octal (0o)
- Edge cases: standalone dash (-) and dot (.)
This ensures values like 'true', '123', 'null' remain strings in YAML
frontmatter instead of being interpreted as booleans, numbers, or nulls.
Preserves existing escaping logic for special characters and newlines.
* test(devin-adapter): add comprehensive tests for Devin Desktop adapter
Add test coverage for the Devin Desktop adapter including:
- Command reference transformation from colon to hyphen syntax
- YAML frontmatter escaping for special characters and implicit scalars
- File path generation for workflows
- Integration with available tools detection
- Init and update command workflows
* Add cross-platform testcase.
* fix(devin): refresh deltas against canonical specs and point skills at skills
Addresses the two release blockers on this PR.
Archive: the change's MODIFIED blocks were written against an older
canonical `cli-init`, so `openspec archive add-devin-desktop-support`
aborted rather than merging. The deltas are regenerated from the current
canonical specs (cli-init `Skill Generation` + `Slash Command
Generation`, cli-update `Slash Command Updates`, and a new
`ai-tool-paths` delta for the `.devin` skillsDir), each restating every
existing scenario so archive is purely additive.
Invocation syntax: only Devin Desktop reads `.devin/workflows/`, so a
`/opsx-*` workflow reference is dead text on Devin Local, which supports
skills only. Devin now takes the skill-reference transformer, so skill
bodies and the getting-started hint say `/openspec-*`. Workflow bodies
keep hyphen references, applied by devinAdapter itself.
The adapter also drops its private copy of escapeYamlValue /
formatTagsArray in favor of the shared helpers main centralized in
#1447, which quote unconditionally and escape control characters.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): correct commands-only hint, fill doc gaps, cover both surfaces
Follow-up from adversarial review of the previous commit.
The devin special case in getTransformerForTool was unconditional, so
under commands-only delivery — where `.devin/skills/` is deleted — the
getting-started hint named `/openspec-propose`, a skill that is not on
disk. Devin now takes the skill transformer only when skills are
generated, and the hyphen form otherwise. The cli-init delta records the
fallback, and a unit test pins all three delivery modes.
Docs: `devin` was missing from the `--tools` list in docs/cli.md (which
mirrors the list supported-tools.md already had) and from the
command-syntax tables in docs/commands.md and docs/how-commands-work.md.
The supported-tools row gains a footnote citing Cognition's docs for the
`.windsurf/` -> `.devin/` move and the Devin Local workflow gap.
Tests: init and update now assert both surfaces — workflows carry
`/opsx-*`, skills carry `/openspec-*`, neither carries `/opsx:` — and
update checks the seeded skill was actually refreshed. Adds the negative
detection case. Drops three devin-only YAML assertions that duplicated,
less rigorously, the registry-derived escaping matrix that now enrolls
devin automatically.
Also reverts an unrelated zcode export and lingma reorder that a merge
resolution had pulled into adapters/index.ts. zcodeAdapter is registered
but missing from that barrel on main; that is a pre-existing gap and
belongs in its own change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): name the right command in the profile migration notice
The profile-migration notice printed by both `init` and `update` hardcoded
`/opsx:propose` for every adapter-backed tool. Devin registers no such
command on any surface — its workflows answer to `/opsx-propose` and its
skills to `/openspec-propose` — so an upgrading Devin user was told to run
something that does not exist:
Migrated: custom profile with 6 workflows
New in this version: /opsx:propose.
The reference now goes through getTransformerForTool, the same call
init.ts already makes for the getting-started hint. Devin prints
`/openspec-propose`; opencode and the other filename-invoked tools are
corrected to `/opsx-propose` as a side effect; claude is unchanged.
Also corrects two inherited false claims in the cli-update delta — Devin
workflows carry no OpenSpec markers, and update writes every profile
workflow rather than only refreshing files that already exist, which the
PR's own test demonstrates. Qualifies the supported-tools footnote for
commands-only delivery, and strips trailing whitespace.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): keep the cli-update delta in step with the canonical spec
The delta restates the whole 'Slash Command Updates' requirement, and its
copy of the OpenCode scenario predated #1471 — archiving it would have
quietly reverted the spec to calling the hyphen rewrite an OpenCode special
case, the hand-maintained framing #1471 removed. Archive on a scratch copy
is now purely additive.
Also point tasks.md at the generator rather than the deleted
transformToHyphenCommands, and enroll devin in the pure-formatter tripwire —
it is the one adapter whose private body transform was just removed, so it
is the likeliest to have it re-added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat(adapters): follow the Windsurf rename to Devin Desktop, with migration
Windsurf was rebranded to Devin Desktop on 2026-06-02 and its config
directory moved: `.devin/` is the preferred read+write location, `.windsurf/`
a legacy read-only fallback. Devin Local does not read `.windsurf/` at all,
so an existing Windsurf user's OpenSpec files are invisible to it.
Carrying `devin` as a second tool id alongside `windsurf` would list one
product twice and leave upgraders with two parallel installs — `openspec
update` even told them to create the second one ("Detected new tool: Devin
Desktop"). This follows the rename instead, as the repo already did for
Kimi CLI -> Kimi Code:
- `windsurf` is retired as a tool id; `devin` takes its place, with
`detectionPaths: ['.devin', '.windsurf']` so pre-rebrand projects are
still recognized. The Windsurf adapter is replaced, not duplicated.
- `TOOL_ID_ALIASES` keeps `--tools windsurf` resolving, so existing setup
scripts and CI keep working; they now configure `.devin/`.
- OpenSpec-managed skills (`openspec-*`) and command files (`opsx-*`) under
`.windsurf/` move to `.devin/`. The kimi migration handled skills only;
command files now move too, deriving the legacy path from the adapter's
own getFilePath rather than hard-coding a layout.
- The move is offered, not taken: nothing on disk distinguishes a user who
took the rebrand from one still on a pre-rebrand Windsurf build that reads
only `.windsurf/`. `openspec update` explains the rename and asks; --force
and non-interactive runs migrate; declining leaves every file untouched and
says what that costs. Files the user wrote are never moved.
Also gives Devin its own row in the authoritative invocation table — the
catch-all row claimed `/opsx-<id>` for both agents, which is wrong for Devin
Local.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): stop the migration from deleting anything it does not own
An adversarial pass found two ways the move destroyed files.
Symlinked roots wiped the install. `ln -s .devin .windsurf` is a realistic
way to straddle the rebrand, and it makes source and destination the same
file — so the "destination exists, drop the legacy copy" branch deleted the
only copy. Twelve generated files, gone, and not regenerated: the wipe
happens before tool detection, so update then reported no configured tools.
Both roots are now realpath'd and a self-move is skipped.
User content inside an OpenSpec-managed path was deleted. The same branch
rm -rf'd the whole legacy skill directory, taking a hand-written
reference.md beside SKILL.md with it, and deleted a legacy command file even
when the user had edited it. Now only SKILL.md is removed from a skill
directory, and a command file is removed only when byte-identical to the
one that survives — an edit is left where it is.
Also: declining the move stranded the user. `update` then printed "No
configured tools found. Run openspec init", which is wrong — the project is
configured, just in the directory OpenSpec no longer writes. It now says so
and how to resume. A closed stdin during the prompt aborted the whole
update; it is treated as a decline.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(devin): add a changeset for the Windsurf rename and migration
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): move only SKILL.md, never the skill directory around it
alfred caught a data-loss path the earlier fix missed. When the destination
did not yet exist, migration renamed the whole legacy skill directory into
`.devin/` — carrying any file the user kept beside `SKILL.md` with it. That
destination is a directory OpenSpec owns and removes on its own: under
commands-only delivery, or for a workflow outside the active profile. So the
move handed the user's file to a later rm and it vanished.
Reproduced on `d94af8b`: with `delivery: commands`, a `reference.md` beside a
legacy `SKILL.md` was gone after `openspec update`.
Only `SKILL.md` crosses now, in both branches; anything else stays under the
legacy root, and the legacy directory is still removed when the move leaves
it empty. Regression tests cover the commands-only and deselected-workflow
cases and both fail against the previous code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(devin): treat an edited skill the way an edited command is already treated
A final adversarial pass found the two paths disagreeing. When both roots
held the same file with different content, the command path compared bytes
and kept the user's version; the skill path deleted it with no comparison —
so one `openspec update` destroyed an edited SKILL.md while preserving an
edited opsx-*.md in the same project.
Both now share one `classifyManagedFile` rule: move when the destination is
empty, drop the legacy copy only when byte-identical, otherwise leave it.
Anything left behind is reported, so a user who customized a file knows two
copies exist rather than discovering it later.
Note on the other finding from that pass: OpenSpec regenerating or pruning
the files it owns is long-standing behavior, not something this PR
introduces. Verified against main — an edited SKILL.md under a deselected
workflow, and an edited selected skill and command, are all destroyed by
`openspec update` on
|
||
|
Clay GoodandClaude Opus 5
|
9a937cb9b3 |
fix(adapters): reference slash commands by the names each tool registers (#1471)
* fix(adapters): reference slash commands by the names each tool registers Generated command bodies, skills and the post-setup hints all advertised /opsx:<id>, but only 7 of 28 adapter-backed tools register that name. The other 21 write .../opsx-<id>.md, where the filename is the command, so their users were told to type a command their palette never had. Codex, which registers no slash commands at all, was told to type them too. The invocation style is now derived from the command file each adapter writes rather than a hand-maintained tool list, so every tool-specific surface - command bodies, SKILL.md cross-references, and the init, update and migration hints - names the command that tool answers to. Closes #1307 Closes #727 Closes #1379 Closes #1110 Refs #1129 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: name the per-tool invocation exceptions in the table itself Review follow-up: the "every other adapter-backed tool" row swept Amazon Q, Cline and Kilo Code into the plain /opsx-<id> form. Each is now its own row with the wrapper it actually uses, and the command-references tests pass the now-required invocation style explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: make every invocation reference match what OpenSpec generates Review follow-up across the docs, the living specs and two hardcoded strings: - supported-tools: the How To Invoke section no longer splits the "How It Works" profile paragraph from its heading, keys rows on the file shape rather than a `.md` extension the Gemini/Continue/Copilot/Kiro adapters do not use, and drops the Cline/Kilo Code/Amazon Q rows. Kilo Code's docs say the current format drops the `.md` suffix, and the Cline and Amazon Q forms could not be confirmed - a wrong exception row is worse than none, so the caveat now describes the shape without asserting a spelling OpenSpec does not generate. - commands, how-commands-work: the two partial nine-row tables that drifted into #727/#1307 now key on the same file shape and defer to the authoritative table; both note that skill rows carry skill names, which are not command ids. - faq, troubleshooting, installation, README: stop telling skills-only users they have no slash command, stop offering "/opsx autocompletes" as a health check on tools where it never will, and name Hermes with the other adapterless tools. - specs: cli-init no longer claims every tool gets `commands/opsx/`, cli-update no longer frames the hyphen rewrite as OpenCode-specific, and command-generation describes the classifier the code implements. - the legacy-cleanup summary and the pre-selection welcome banner no longer print `/opsx:*` at users whose tool never registers it. - adds the missing changeset; it supersedes the Codex sentence in the pending adapterless-skill-references note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: cover the update and migration paths a mutation run found unguarded Mutation testing showed five ways to delete parts of this change without failing a single test. All five now fail: - `openspec update` had no flat-tool coverage at all, so the headline upgrade path - an existing Cursor project still carrying `/opsx:` references - was asserted nowhere. Two tests now cover it: one heals a project seeded with stale references, one runs claude+qwen together and pins each to its own form. - the legacy-upgrade getting-started menu is covered for a newly configured Cursor project, so passing the wrong invocation style there is caught. - migration.ts had no flat-tool case: reverting it to a hard-coded `/opsx:propose` passed the whole suite. A qwen-only migration and a claude+qwen disagreement now pin the message. - the unknown-command-id guard in `transformToHyphenCommands` was new behaviour with no test; removing it was invisible. Also tightened assertions the same run showed were weak: the `resolveCommandInvocationStyle` loop compared the implementation against itself, the per-id consistency check asserted only that a style was uniform rather than which one, and the init test's `/opsx-` assertion was satisfied by frontmatter rather than a body reference. The adapter tests that moved to `generateCommand` are renamed after their real subject, and a new case pins the contract those five adapters now rely on: they stay pure formatters and do not rewrite the body themselves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: assert the rewritten form, not just the absence of the old one Review follow-up: the refreshed-skill checks were negative-only, so a regression that dropped every command reference rather than rewriting it would have passed. Each now pins the invocation its tool registers, the stale fixture asserts it really seeded a colon reference into the skill, and the claude+qwen case pins Claude's namespaced skill alongside Qwen's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(adapters): spell Amazon Q's prompts with @, not as slash commands The invocation model derived the whole command name from the file an adapter writes, which covers `/opsx:<id>` versus `/opsx-<id>` but not the wrapper around it. Amazon Q loads `.amazonq/prompts/opsx-<id>.md` into its prompt library, invoked as `@opsx-propose`; it registers no slash command, so its command bodies, skills, and the "Getting started" hint all named something the tool never answers to. The name still comes from the file path. The prefix is now adapter metadata (`invocationPrefix`, defaulting to `/`), so it cannot be guessed wrong and a new adapter has to declare it deliberately — invocation.test.ts fails if one appears undeclared. Also fixes three copy issues: - The FAQ told users to run `openspec update` when command files are missing; update only refreshes files for already-configured tools, so a tool that was never initialized needs `openspec init`. - The installation prompt omitted Kimi Code's `/skill:openspec-propose`. - The welcome screen promised "opsx slash commands" before tool selection, which is wrong for skills-only tools that correctly get no command files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(init): stop naming slash commands where none are registered Two spots still promised a slash command to users who get none: - The welcome screen's quick start shows canonical names (/opsx:propose), but renders one prompt before tools are picked — an Amazon Q user types @opsx-propose and a Codex user $openspec-propose. It now says the spelling varies by tool, so the canonical form stops reading as the literal thing to type. "Getting started" still prints the real form. - The post-setup restart line said "slash commands to take effect" whenever commands were generated. Amazon Q's generated files are prompt library entries, not slash commands, so it now says "the new commands". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: name Amazon Q's @ form where the other exceptions are listed The README's one-line exception list and the troubleshooting checklist both enumerated the per-tool spellings and skipped Amazon Q. The troubleshooting entry was actively misleading: it explains that /opsx never autocompletes for tools without command files, and Amazon Q is not one of those — it has command files, they just land in the prompt library. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(migration): cover the legacy-upgrade hint for amazon-q The migration hint resolves its propose reference through the same transformer as init and update, but no case exercised a non-slash prefix there. The second test is the one that matters: @opsx-propose and /opsx-propose are both "flat", so a style-only model would treat Amazon Q and Qwen as agreeing and advertise one form to both. Reverting the prefix to a constant '/' fails 5 tests, so neither assertion is a tautology. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Clay GoodandClaude Fable 5
|
fc886af7f9 |
fix(templates): auto-select the only active change instead of always prompting (#1468)
* fix(templates): auto-select the only active change instead of always prompting
The continue, update, verify, sync, and archive workflows told agents
'Do NOT guess or auto-select a change. Always let the user choose', which
contradicted their own Input line ('check if it can be inferred from
conversation context') and stalled every invocation on a question with a
single possible answer when only one change was active. Align them with
the selection pattern /opsx:apply has used since #513: use the provided
name, infer from context, auto-select a sole active change, prompt only
when ambiguous, and always announce the selection with how to override.
Bulk archive keeps its always-prompt behavior.
Closes #679
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(openspec): add the announce clause to the update workflow's selection contract
CodeRabbit noted the add-update-workflow delta spec and design sketch
adopted auto-selection without the 'Using change: <name>' announcement
the other selection contracts require. Add the same announce-and-override
clause so the update skill's contract matches the template it describes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Alfred
|
d32d49f066 | chore(openspec): archive schema init force validation change (#1467) | ||
|
Clay GoodandClaude Opus 5
|
f917b8be5e |
fix(status): order artifacts by the schema, not the alphabet (#1465)
* fix(status): order artifacts by the schema, not the alphabet
Artifacts that become ready at the same time were sorted alphabetically,
so spec-driven's `specs` and `design` - both requiring only `proposal` -
came back as design first. `openspec status` listed design above specs
and `nextSteps` pointed at design, sending agents to write design.md
before any spec existed. That contradicts the schema's own description
(proposal -> specs -> design -> tasks), the design instruction ("reference
the specs for requirements"), the workflow docs, and the schema `openspec
schema init` scaffolds (where design requires specs).
Break ties by the order the schema declares its artifacts instead. The
dependency edges are untouched, so nothing newly blocks and no artifact
becomes mandatory - only the order of equally-ready artifacts changes, and
it now follows the sequence the schema author wrote, for custom schemas
too.
Closes #692
Closes #695
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(status): re-sort the whole ready queue, not just new arrivals
CodeRabbit caught it: sorting only the newly ready artifacts left an
already-queued artifact ahead of one declared earlier. For [root, child,
laterRoot] where child requires root, the build order came out root ->
laterRoot -> child even though child is declared first and both are ready
after root. Sort the full queue after each push.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(instructions): order unlocks like status, and document the guarantee
Adversarial review found `unlocks` was left alphabetical while build order,
ready lists and blocked lists moved to declaration order, so `openspec
instructions proposal` said "enables: design, specs" while `openspec status`
listed specs first - the one field whose job is naming what comes next
disagreed with everything else. getAllArtifacts() already yields declaration
order, so the stray sort is simply dropped.
Also make compareByDeclarationOrder a method rather than an arrow-valued
field: the field added an own enumerable function property that made
ArtifactGraph fail structuredClone.
Docs and specs updated for the new guarantee:
- openspec/specs/{artifact-graph,cli-artifact-workflow,instruction-loader}
- docs/agent-contract.md: status --json and instructions --json ordering
- docs/opsx.md: the status sample's missingDeps was missing design
- changeset
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(commands): correct the continue transcript's blocked and unlocked lines
The sample said tasks was blocked by specs alone and that creating specs
made tasks available; tasks needs design too. Same class of inaccuracy as
the status samples this branch already corrected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: state the ordering guarantee as dependency-order-then-declaration
CodeRabbit was right that "artifacts appear in the order the schema
declares them" over-claims: dependency order still wins, and declaration
order only breaks ties. Proved with a schema that declares tasks, specs,
proposal - status renders proposal, specs, tasks, not the declared order.
Corrected in the cli-artifact-workflow spec, agent-contract.md, cli.md and
the changeset.
Also restores "status": "blocked" in the opsx.md status sample (split across
two lines so the ASCII box still aligns) and uses "recommends writing next"
in the artifact-graph spec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
eac2973819 |
feat(instructions): add runtime context and operation guidance (#1062)
* docs(openspec): define runtime guidance for apply and archive - add typed apply and archive operation guidance - extend runtime instruction inputs for apply and archive - preserve existing archive execution and spec sync behavior * docs(openspec): refine apply and archive guidance design - carry artifact rules into archive-driven spec sync - reuse one config snapshot per instruction command - clarify that operation guidance is advisory - classify bulk archive skill as a new capability * docs(openspec): clarify artifact rule handling for archive and sync - define owning artifact resolution for mixed schemas - apply artifact rules in archive and standalone sync flows - align archive and bulk guidance conflict semantics - clarify existing apply pause-on-blocker behavior * docs(openspec): tighten archive and spec sync contracts - scope delta discovery and artifact rules to the specs artifact - fail closed on invalid archive and specs instruction responses - clarify no-write and no-move behavior for single and bulk archive * feat(workflow): extend config injection to apply and archive - expose project context and operation guidance in apply/archive instructions - apply context and guidance across apply, archive, bulk archive, and spec sync - preserve workflow state, artifact-rule boundaries, and fail-closed behavior - update generated skills, documentation, tests, and parity hashes * fix(skills): make the archive-inputs lookup fail open `openspec instructions archive` is introduced by this PR, so no released CLI has it. The archive and bulk-archive skills required a zero exit status from that lookup and told the agent to stop when it failed. `skills/` is installed standalone via `npx skills add Fission-AI/OpenSpec` and drives whatever CLI the user already has, so between merging this and publishing the next release every skills.sh consumer would have had archiving blocked outright — verified against @fission-ai/openspec@1.6.0, which exits 1 on that command. The lookup only supplies optional prompt inputs, so it now degrades: on a non-zero exit or invalid JSON the workflow continues with no context and no operation guidance. The `openspec instructions specs` lookup is an existing command and stays fail-closed, since a missing rule set there would silently change what gets written to main specs. Parity assertions updated to encode fail-open for archive inputs and fail-closed for specs rules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: showms <showms@users.noreply.github.com> Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
5348da930c |
fix(schema): validate artifacts before forced init (#1446)
* fix(schema): validate artifacts before forced init * test(schema): assert successful forced init exit status --------- Co-authored-by: showms <showms@users.noreply.github.com> Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay GoodandClaude Fable 5
|
19d41714c8 |
fix(archive): treat early-synced REMOVED deltas as no-ops, plus audit follow-ups (#1437)
* fix(archive): treat early-synced REMOVED deltas as no-ops, plus audit follow-ups Follow-ups from the post-v1.6.0 full-branch audit: - archive: a REMOVED delta whose requirement is already gone from the main spec (early-sync pattern) now warns and continues instead of aborting, matching the ADDED (#1376) and RENAMED (#1386) escapes; spec-update totals now count applied removals only - archive: the has-delta-specs gate matches section headers case-insensitively like the parser, so lowercase headers get the same delta validation errors validate reports - discovery: a symlinked specs/<cap>/spec.md is resolved instead of being invisible (hasAnyFileUnder and the artifact graph already counted it); dangling links are skipped - show: a plain `openspec show <change>` no longer warns about the never-passed `scenarios` flag (commander defaults --no-scenarios to true) - parsers: buildCodeFenceMask now has a single implementation in code-fence.ts; requirement-text.ts re-exports it - templates: apply/update/onboard no longer dead-end core-profile users on /opsx:continue and /opsx:new - they name the CLI fallback (openspec status/instructions) for profiles that do not install those workflows - qwen/bob: command bodies and skills reference commands by the hyphen names their files actually answer to (/opsx-<id>), matching opencode/pi/oh-my-pi - specs-apply: remove the dead applySpecs export (no callers, bypassed store-aware roots) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(archive): reject RENAMED+REMOVED conflicts, surface JSON warnings, skip no-op writes Adversarial-review round for #1437: - a delta that both RENAMEs and REMOVEs the same requirement is rejected explicitly by both validate and archive - the warn-and-continue REMOVED path would otherwise have masked the contradiction that previously failed incidentally at apply time - buildUpdatedSpec collects its warnings and archive --json carries them in a new optional `warnings` array, so agent flows see the same skipped-REMOVED signal humans get on stdout - archive skips rewriting a spec whose operations were all already synced, instead of churning normalization differences into the file (and no longer materializes an empty skeleton for a REMOVED-only new spec) - init's getting-started hint uses each tool's real invocation form (/opsx-propose for qwen/bob/opencode/pi/oh-my-pi) - onboard's pause guidance names the CLI fallback when /opsx:continue is not installed (CodeRabbit) - openspec-conventions spec updated to state the idempotent archive semantics; changeset added Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(archive): abort on near-miss REMOVED typos, honest specsUpdated for no-op archives Round-2 adversarial review for #1437: - a REMOVED header that differs only in case or interior whitespace from an existing requirement is a typo, not an early sync - it stays a hard abort naming the near-miss, instead of degrading to warn-and-continue - specsUpdated is true only when a spec file was actually written; a fully-already-synced change prints "Specs already in sync; no files changed." and reports specsUpdated: false in JSON (CodeRabbit) - agent-contract documents the archive warnings field and specsUpdated semantics; changeset wording fixed (CodeRabbit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(archive): compare the RENAMED+REMOVED conflict case- and whitespace-insensitively Addresses alfred's review on #1437: `RENAMED FROM: Old Name` plus `REMOVED: old name` slipped past the exact-match cross-section guard, so validate passed, archive renamed the requirement, reported the removal as already synced, and archived the change. Both the validator and the apply-side guard now compare the two spellings with the shared foldRequirementName (lowercase, collapsed whitespace), and the error names the variant spelling when it differs. Focused regressions cover both paths; requirement matching everywhere else stays case-sensitive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
6a4f0d7f33 |
fix(archive): keep the delta spec's Purpose in a new main spec (#1431)
* fix(archive): keep the delta spec's Purpose in a new main spec Archiving a change that creates a brand-new capability always overwrote the delta's authored `## Purpose` with the TBD placeholder, so the Purpose had to be re-typed by hand after every archive. buildSpecSkeleton now takes the delta's Purpose when there is one. The placeholder still appears when the delta has no Purpose or an empty one, and an existing main spec's Purpose is never touched. The spec-driven schema now tells agents to open a new capability's delta with a `## Purpose` (and not to add one to a delta for an existing capability), so the default workflow stops producing placeholders. Closes #1413 Closes #369 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(archive): create the temp dir with fs.mkdtemp Matches the mkdtemp pattern the rest of the suite already uses and clears the CodeQL insecure-temp-file alerts on this file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(archive): pin fenced-Purpose behavior and align the spec wording Review flagged that the spec scenario read as "only non-fenced content counts", which the code does not do. Masking fenced lines out of the Purpose body would truncate a legitimate Purpose that includes an example block, so the code is right and the wording was wrong. - Reword the cli-archive scenarios: the fence check is on the `## Purpose` header, and the section body is copied verbatim. - Add regressions: fenced code inside a real Purpose survives, a Purpose header that only appears inside a fence falls back to TBD, and an empty Purpose section falls back to TBD. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(archive): never let a carried Purpose abort the archive Self-review found a regression introduced by the carry-over: a delta whose `## Purpose` body contains a `### Requirement:` header put that header outside `## Requirements` in the new main spec, so the structure guard rejected it and archive exited 1. The same delta archived fine before this branch. Fall back to the placeholder and warn when the carried Purpose would make the new spec structurally invalid, so archive completes as it did before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(archive): make the Purpose carry-over safe and consistent Three adversarial reviews of the carry-over found the guard added in |
||
|
Clay Good
|
1dc670deea |
fix(templates): stop propose from skipping the specs artifact (#1412)
Squashed for rebase; see PR #1412 for the full commit history. |
||
|
Clay GoodandClaude Fable 5
|
0da5f98e14 |
fix(templates): show the main spec format in the sync-specs skill (#1402)
The sync-specs skill's only markdown example was the delta format, so agents (Junie in #1120) copied delta files into openspec/specs/ as-is, leaving ## MODIFIED Requirements headers that the spec parser rejects — openspec view reported 0 requirements. Add a Main Spec Format Reference, point step 4d at it, and add a guardrail against wholesale delta copies. Fixes #1120 Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Jun
|
9b5d2cdd0c |
fix(templates): stop instructing a second date prefix on dated archive names (#1388)
* fix(templates): stop instructing a second date prefix on dated archive names The archive-change and bulk-archive-change workflow templates told agents to unconditionally build the archive target as YYYY-MM-DD-<name>, so a change already named with the common YYYY-MM-DD- convention came out double-dated — the template-side twin of the CLI bug fixed in #1316, which a CLI fix cannot reach because the behavior is baked into instruction text. The generate-target-name step and the bulk guardrail now mirror the CLI rule: use the change name as-is when it already starts with a YYYY-MM-DD- prefix, otherwise prepend the current date. The literal mv commands move to <target-name> so an agent copying them verbatim cannot stack dates, and the onboarding walkthrough's archived-path example carries the same caveat. Regenerated skills/ and updated the pinned parity hashes; a new parity test guards the caveat and rejects the raw stacked mv target. * fix(templates): report the derived archive name in success summaries The success and failure summaries still printed archive/YYYY-MM-DD-<name>, so an agent copying them would report a stacked date for a change whose name already carries a YYYY-MM-DD- prefix. Point those examples at <target-name> instead, and widen the regression guard from the mv target to any date used as a path segment, which leaves the rule statements that must keep explaining the derivation untouched. The opsx-archive-skill spec still specified the unconditional current-date rule the previous commit removed from the template, so bring it in line with the wording cli-archive already carries. * fix(specs): name the derived target in the archive scenario The successful-archive scenario still spelled the destination as archive/YYYY-MM-DD-<name>/, the same literal form this PR removed from the templates, so it contradicted the keep-as-is rule the behavior requirements now carry. |
||
|
Clay GoodandClaude Opus 4.8
|
c439a4ee48 |
fix(parser): stop delta section dividers from becoming phantom requirements (#1411)
* fix(archive): stop reporting phantom proposal warnings from delta specs `openspec validate --strict` reported a change as valid while `openspec archive` printed "Proposal warnings in proposal.md" for the same change, blaming requirements that do not exist. Archive validates the proposal with `validateChange`, which parses the change together with its delta specs. Requirement-level issues from those deltas were printed in the proposal block even though they are not proposal issues. Two problems followed: - The change parser records every requirement under both `requirement` and `requirements`, so each defect was printed twice, then a third time by the delta report. - A heading inside a delta section that is not a `### Requirement:` heading was parsed as a requirement, producing a scenario warning against a requirement that does not exist. The delta reader already handles this correctly and reports it as an informational note. Proposal warnings now report proposal-level issues only. Delta spec issues keep being reported once, by the delta report, with the capability file path and requirement name. Exit codes are unchanged: this block was already non-blocking, and blocking delta validation is untouched. Refs #498 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(archive): correct proposal-warning claims and pin bracket-path rules Review follow-ups, no behavior change: - The delta report prints only the issue message, never `issue.path`, so it does not name the capability file. Drop that claim from the spec scenario and the code comment; two capabilities with the same defect print two identical lines. - Only the missing-scenario class was reported three times. Say that precisely instead of generalizing to every delta error. - Widen the spec scenario: the filter applies to every archive, not only to changes carrying a stray heading. - Add a test pinning that applyChangeRules bracket paths (`deltas[<n>].description`) survive the dot-anchored filter, so a future path normalization cannot silently widen it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(parser): ignore delta headers that are not "### Requirement:" Fixes the cause of #498 rather than one of its symptoms. A header inside a delta section that is not a `### Requirement:` header — a divider such as `### Documentation Requirements` — was read as a requirement with no scenario. That invented a delta that does not exist: `openspec archive` warned about a missing scenario, and `openspec show <change> --json` and `openspec change list` counted it. ChangeParser now filters those headers before reading requirements, matching REQUIREMENT_HEADER_REGEX, which the delta reader already uses. The override lives in ChangeParser, so main spec parsing — view, list, spec --json, spec validation — is untouched. The archive filter stays: it covers the half the parser cannot. The change parser records every requirement under both `requirement` and `requirements`, so each delta defect was printed twice, and REMOVED requirements are names-only by design yet were reported as missing a scenario on every correct removal. Also from review: - Soften the spec scenario; delta spec validation does not always run (the hasDeltaSpecs gate is case-sensitive), so it cannot be promised as the reporter. - Assert VALIDATION_MESSAGES constants instead of message literals. - Add parser-level and REMOVED-only regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
60f720c43a |
fix(feedback): submit feedback when the repo has no feedback label (#1396)
`openspec feedback` passed `--label feedback` unconditionally, but the repository does not define that label. gh resolves label names before creating the issue, so it failed with "could not add label: labels not found: feedback" on every invocation and the command exited non-zero, discarding the feedback the user had just composed. Retry once without the label when — and only when — gh's stderr reports that it could not add the label, and tell the user the label was not applied. Every other failure keeps its existing behavior: print gh's error and exit with gh's exit code, with no retry. Only stderr is matched, because the error message also embeds the command line, which carries the user's own feedback text. The cli-feedback spec gains a scenario for the unlabeled path, and its gh-failure scenario is narrowed to exclude it. The fallback scenarios are unchanged. Refs #1091 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
b474f81cb4 |
fix(templates): don't archive a change before its spec sync finishes (#1394)
* fix(templates): wait for the spec sync before archiving a change The generated openspec-archive-change skill dispatched the spec sync to a subagent via the Task tool and then moved changeRoot in the very next step, with nothing requiring it to wait. Where subagents run asynchronously, the archive relocates the delta specs out from under the running sync, so the change is archived while openspec/specs/ is never updated — and the success summary still reports "Specs: ✓ Synced". Step 4 now requires waiting for the dispatched sync to return, verifying the synced requirements are present in the main spec, and stopping without archiving if either check fails. Adds a matching guardrail bullet and a parity assertion so the gate cannot silently disappear again. Fixes #1393 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(templates): run the spec sync inline and verify it before archiving Addresses review on #1394. The first pass asked the agent to "wait" for a dispatched subagent, but subagents run in the background by default and the wait is not reliably expressible in prose — the race survived. It also gated the archive on the synced requirements being *present*, which a correct REMOVED-only or RENAMED-only sync does not satisfy, turning a successful sync into a hard block. The sync now runs inline via the Skill tool, with a synchronous-subagent fallback for harnesses that need one. Verification follows delta semantics: ADDED/MODIFIED present, REMOVED gone, RENAMED under the new name, checked across every capability the sync touched. Also resolves the opsx command variant's contradiction with its own guardrail, stops the summary reporting a checkmark that step 4 never verified, and updates openspec/specs/opsx-archive-skill/spec.md, which still said the skill proceeds with the archive regardless of the sync choice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(specs): encode delta verification semantics in the archive skill spec The scenario said the agent verifies each capability "matches its delta", which is ambiguous about what a match means — and a REMOVED-only sync correctly leaves requirements absent. Spell out the predicate the template implements, and separate an explicit "Archive without syncing" choice from a requested sync that failed or could not be verified: only the former may skip verification, the latter must stop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(templates): close verification holes and drop Claude-only tool names Second review pass on #1394. The gate was weaker than it looked. "MODIFIED requirements present" is vacuous — a MODIFIED requirement exists in the main spec before the sync runs, so a no-op sync passed the check for the most common delta shape, which is the exact symptom #1393 reports. "RENAMED under their new name" passed a sync that copied rather than renamed, leaving both names behind. And scoping the re-check to "every capability it touched" derived the verification set from the artifact being verified, so a silently skipped capability escaped it. Verification is now bound to the delta specs in artifactPaths.specs, covers the changes each MODIFIED delta names, and requires RENAMED requirements to be gone from the old name. Separately, the previous pass named the Claude Code "Skill tool" and run_in_background in a template that is also the slash-command source for ~28 other tools, where skills are removed entirely for commands-only delivery. Both variants now use the runtime-neutral phrasing bulk-archive-change already uses. Also: route the prompt options explicitly instead of defaulting unknown answers to archive, tell the user a stopped archive is recoverable, and mark the summary line as a conditional rather than literal text to copy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(templates): verify the sync by re-running step 4's own comparison The verification predicate restated delta semantics in its own words, which could drift from what openspec-sync-specs actually does. Anchor it instead to the comparison step 4 already performs before prompting: a successful sync leaves nothing to apply, so every capability must read as already synced. The explicit ADDED/MODIFIED/REMOVED/RENAMED bullets stay as the definition of what "nothing left to apply" means. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
HowardandClay Good
|
d2082d1f91 |
docs: align cli-update OpenCode spec with commands/ and opsx-* paths (#1170)
Update OpenCode cli-update spec to .opencode/commands/opsx-*.md and document legacy path cleanup via init. Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Jun
|
9b70481df7 |
fix(archive): keep an existing date prefix instead of stacking a new one (#1316)
Archiving unconditionally prepended today's date to the change name, so a change already named with the common YYYY-MM-DD- convention came out double-dated (2026-07-07-2026-07-04-voice-copilot-v1) — and when archived on a later day, the folder sorted under a day on which the change did not happen. Detect a full YYYY-MM-DD- prefix and archive the change under its own name. Names without one (including partial dates like 2026-07-feature) keep the current behavior. This also makes the naming idempotent. Nothing in src/ parses the date back out of archive folder names — the prefix only drives human chronological sorting — so keeping the original date is the minimal, non-breaking choice. The cli-archive spec wording is updated to match. Fixes #1309 |
||
|
|
9acddcda07 |
fix: use local dates for CLI date-only values (#1361)
Co-authored-by: showms <showms@users.noreply.github.com> Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com> |
||
|
showmsandshowms
|
79f1dac668 |
feat(codex): make Codex skills-only and retire managed custom prompts (#1283)
* feat: make Codex skills-only * test: cover codex legacy prompt paths * fix: address review feedback for Codex skills-only migration * fix(codex): revalidate managed global prompt paths before cleanup * resolve conflicts with upstream/main * fix(codex): refine legacy prompt migration --------- Co-authored-by: showms <showms@users.noreply.github.com> |
||
|
|
4a0f15d3b2 |
feat: add Hermes Agent support (#1292)
* feat: add Hermes Agent support * feat(init): surface Hermes external_dirs setup note during init and update Hermes only loads skills from ~/.hermes/skills unless the project .hermes/skills directory is added to skills.external_dirs in ~/.hermes/config.yaml, so init could report success for skills Hermes ignores. Add a setupNote field to AIToolOption, print it after init and update (including the up-to-date path), and cover the adapterless init path, the adapter registry, and both update paths with tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
e60ff53644 |
update Kimi CLI to Kimi Code (#1208)
* update Kimi CLI to Kimi Code * feat(migration): migrate OpenSpec skills from legacy .kimi to .kimi-code Renaming the Kimi skillsDir stranded OpenSpec-managed skills under .kimi/skills: update and cleanup only inspect current AI_TOOLS paths, so old installs would never be detected or refreshed again. Add a legacy skillsDir migration (run by init and update before tool detection) that moves openspec-* skill directories to .kimi-code/skills, preserves user files, and removes the legacy directories only when empty. Keep .kimi as a detection path and cover the migration with focused init and update tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(specs): update cli-init Kimi scenario to .kimi-code Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
8886e3ae22 |
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> |
||
|
|
3f0ca3f6ce |
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> |
||
|
Clay GoodandClaude Opus 4.8
|
a5bfedafc8 |
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> |