mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-09-14 20:16:53 +08:00
main
121 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Clay Good
|
8ba4ac1b16 |
fix(apply): warn when a change is ready to implement with no specs (#1783)
* fix(apply): warn when a change is ready to implement with no specs Apply gates on the schema's `apply.requires` (tasks) alone, so a change whose tasks file was written ahead of its specs read as ready even though it had no delta specs at all — the state `openspec validate` rejects. Apply was the one surface that green-lit a change every other surface flags, which is how agents end up implementing before the specs exist. Report it as a warning, in the text output and in `--json`, naming both ways out: write the specs, or declare `skip_specs: true`. Blocking would be a policy change; naming the gap is not. Changes that have specs, declare `skip_specs`, or are still blocked on their own required artifacts are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(apply): name the metadata file from its shared constant Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(apply): cover custom schemas in the no-specs warning A schema with no spec-producing artifact must stay quiet, and one whose spec artifact is not called `specs` must still warn - the rule keys off the output path, not the artifact id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(apply): stop asserting an absolute temp path on Windows os.tmpdir() hands back the short form (C:\Users\RUNNER~1) while the CLI resolves the long one, so the assertion pinned a path that never matched on windows-pwsh. Assert the change-relative tail instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(apply): name the whole chain a blocked change still needs Apply blocks on the schema's `apply.requires` alone, so its message stopped at the first hop: a change holding only a proposal was told "Missing artifacts: tasks" while the specs `tasks` depends on were missing too. Taken literally that is an instruction to write the tracking file straight from the proposal and skip everything between — the failure reported in #834 and #869. Walk `requires` and report the whole set, in build order, as `missingPrerequisites` (text and `--json`). What apply blocks on is unchanged, and the wording leaves conditional artifacts to the schema rather than demanding them. The remedies these messages give are now CLI commands rather than the `openspec-continue-change` skill: `continue` is not in CORE_WORKFLOWS, so on the default profile the old advice named a skill that is never installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(apply): name the schema's own spec artifact in the warning alfred-openspec on #1783: collectApplyWarnings() discovers spec-producing artifacts by output path, so it correctly fires for a schema whose artifact id is `contracts`, but the remediation text then hardcoded `openspec instructions specs`. That names an artifact such a schema does not declare, so the advertised custom-schema support dead-ended at the exact step meant to resolve the warning. The command now derives its target from specArtifacts: the artifact's own id when the schema declares one spec-producing artifact, and `<artifact-id>` as a placeholder when it declares several, since there is no single right answer there and a guess would read as an instruction. The renamed-artifact test now asserts the command names `contracts` and rejects the hardcoded `specs` spelling, and a new test pins the two-artifact placeholder. Verified both fail against the hardcoded string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(changeset): bump apply warnings to minor This adds `missingPrerequisites` and `warnings` to the documented `instructions apply --json` contract in docs/agent-contract.md. New fields are backward compatible, but they are new capability an agent can consume, which is a minor under semver rather than a patch. Taking the conservative direction deliberately: shipping new API surface as a patch is the violation, since a consumer pinned to a patch range would receive it without opting in. A minor costs nothing if the fields turn out to be uninteresting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay Good
|
954d4796a4 |
docs(community): add a community showcase (#1739)
* docs(readme): list the independent openspec ui project * docs(community): move the showcase out of the readme |
||
|
Howard
|
2fd175c8b0 |
docs(cli): document managed PowerShell completion setup (#1070)
* docs(cli): add Windows PowerShell completion example The shell completion documentation only showed Unix/bash examples, making it unusable for Windows users. Added platform-specific examples for both Unix/macOS (bash) and Windows (PowerShell). Changes: - Add Unix/macOS (bash) example with ~/.bash_completion.d path - Add Windows (PowerShell) example with C:\Users\y00031947\Documents\WindowsPowerShell\Microsoft.PowerShell_profile.ps1 path - Improve clarity with platform labels Fixes: Windows users cannot use shell completion manual installation * fix(cli): use append operator for PowerShell profile to avoid data loss Critical fix: Using '>' operator would overwrite the user's PowerShell profile, deleting existing configurations. Changed to '>>' to append instead of overwrite, preserving user's existing settings. * docs(cli): harden PowerShell completion setup --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Александр Мелентьев
|
44a39eb24b |
feat(core): add codeassistant support (#1171)
* feat(core): add codeassistant support Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: change format file and add test Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: add tests Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * fix: escaped description yaml values Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: add test Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: change adapter Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: handle \r in description Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore: use common escapeYamlValue helper Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> * chore(release): track sourcecraft support --------- Signed-off-by: Александр Мелентьев <aleksandr4842@ya.ru> Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay Good
|
d0071d7326 | docs(archive): show how to retire capabilities (#1751) | ||
|
Tanner
|
a7353aea9a |
feat(status): add --all for batch status of every active change (#1301)
* feat(status): add --all for batch status of every active change
`openspec status --all --json` reports every active change in one
process instead of one CLI spawn (~500ms module-load) per change,
mirroring the existing `validate --all`. Emits a single
`{ changes: [ChangeStatus, ...], root }` envelope sorted by change
name; a change that fails to load contributes a per-change error entry
instead of failing the sweep. `--all` and `--change` are mutually
exclusive, honoring the --json null-shape on failure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): harden --all per adversarial review findings
- Validate --schema before the no-changes early return so a bogus
schema fails consistently whether or not any change exists.
- Text mode now exits 1 when any change fails to load (mirrors
validate --all); JSON mode still exits 0 with per-change diagnostics.
- Add tests for the --all --schema interaction (unknown schema
null-shape, override propagation, broken-metadata precedence) and
text-mode failure rendering.
- Changeset heading to "### New Features" per repo convention; add
status --all to the agent quick-reference table in docs/cli.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): thread batch null-shape through root resolution, align sort with validate
Code-review findings on --all:
- Pass failurePayload: { changes: [] } to resolveRootForCommand so a
root-selection failure under --all --json still emits the documented
batch null-shape (siblings like list/doctor/context already do this).
- Sort with localeCompare to match validate --all's ordering for
mixed-case change names.
- Extract a shared loadStatus helper so the batch and single-change
payloads cannot drift apart.
- Changeset no longer claims exact validate --all parity (JSON exit
semantics deliberately differ).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(status): give the --all failure null-shape a single owner
Simplify pass on the --all diff: hoist the { changes: [] } batch
null-shape into an exported BATCH_STATUS_FAILURE_PAYLOAD constant so
the root-resolution and CLI-wrapper failure paths cannot drift, replace
the conditional spread with the plain ternary the sibling call site
already uses, drop a redundant array copy before sort, and narrow the
text-mode failure counter to the boolean it actually is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): point the missing-target error at --all, correct the docs
`openspec status` with neither --change nor --all listed the available
changes and named only --change, so the batch path was discoverable
only from --help. The error now offers both.
Also corrects two stale claims in the status section of docs/cli.md
that the new row sits next to: the command never prompts for a change
(it errors), and bare `openspec status` is not an interactive check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(status): assert --all carries store context like the single-change path
The batch sweep resolves the root once and threads the store id into
every entry. Nothing pinned that: a regression would have shown up only
as a wrong path inside an agent's JSON. Assert the sweep's envelope root
and per-change payload match `status --change` in a registered store.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(status): fail incomplete batch reports
* docs(status): clarify empty and batch output
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Clay Good <hi@claygood.com>
|
||
|
Clay Good
|
7276c6c268 |
fix(packaging): print the completions tip from the CLI, not a postinstall script (#1704)
* fix(packaging): print the completions tip from the CLI, not a postinstall script The package's only install script existed to print one line suggesting `openspec completion install`. Shipping it made every `npm install -g` emit an npm allow-scripts warning, and `npm approve-scripts` then failed with ENOMATCH because it looks in the local project, not a global install — so the warning looked like a packaging fault with no way to clear it. The tip now prints once on the CLI's first run, recorded via a `completionTipSeen` flag in the existing global config alongside the telemetry notice's `noticeSeen`. It writes to stderr so it can never contaminate piped stdout, and is suppressed under CI, OPENSPEC_NO_COMPLETIONS=1, `--json` runs, and `openspec completion` itself. The published package now ships no lifecycle scripts at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): stop the first-run tip from corrupting global config Adversarial review of the previous commit found it wrote a defaults-merged config: `saveGlobalConfig({ ...getGlobalConfig(), completionTipSeen: true })` stamped `profile: "core"` into every user's config.json on first run. `migrateIfNeeded` treats a raw `profile` as "already migrated", so the one-time profile migration would never run again — and `openspec update` then deleted the user's installed workflow skills. Reproduced: 2 skill directories removed where main reports "Migrated: custom profile with 8 workflows". The same write also overwrote an unparsable config with defaults and made `openspec config list` report defaults as explicit. The tip now reads and writes the raw config file and touches only its own key, leaving an unreadable config strictly alone. Other hardening from the same review: - Suppress the tip for the hidden `__complete` resolver. Generated completion scripts call it on every Tab press with stderr discarded, so the one-shot tip was consumed where nobody could see it. - Defer, never consume, when stderr is not a terminal. Agents and pipes drive this CLI far more often than humans do and would otherwise spend the tip into a log nobody opens. - Skip the tip when completions are already installed. Previously the CLI advertised `completion install` to users who had run it — including on the very next command after installing. Adds `isInstalled()` to the bash/fish/powershell installers, mirroring the zsh one. - Use the repo's `isCiEnvironment()` instead of a `CI === 'true'` string check, so `CI=yes`/`True`/`on` are as quiet as telemetry is. - Move the call to `postAction` so the tip trails the command's output instead of pushing errors and `init`'s setup summary down the screen. - Record before printing, so an unwritable config dir means silence rather than nagging on every run. Tests: assert the message literal (mutation testing showed the message text was the one unguarded behavior), the raw-write shape, corrupt-config safety, the already-installed path, the defer policy, and an e2e case pinning the non-TTY contract. Docs: SECURITY.md no longer claims zero lifecycle scripts — `prepare` is still declared and runs for git/directory installs; the registry-install claim is the accurate one. `OPENSPEC_NO_COMPLETIONS` is now documented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(completions): make the unwritable-config case portable to Windows fs.chmodSync(dir, 0o555) does not stop a write on Windows, so this test's unwritable condition never existed there: markTipSeen succeeded, the tip printed, and windows-pwsh was the only failing job. Occupy the config directory's path with a file instead. mkdirSync with recursive: true tolerates an existing directory but throws on an existing file on every platform, so the persist fails where a real permission error would - before anything is printed. Also asserts the path is still a file, so a partial write through the failure would be caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): retire the first-run tip instead of advising a dead end Second adversarial pass over the tip, covering the hardening commit itself. - An undetected or unsupported shell now retires the tip quietly. It used to print, but `openspec completion install` exits 1 for exactly those users ("Shell 'tcsh' is not supported yet" / "Could not auto-detect shell"), so the one message they would ever get about completions sent them to a command that fails. - `markTipSeen` re-reads the config immediately before writing and swaps the file in by rename. Deciding whether to show the tip costs a `ps` spawn plus a stat, and a sibling process writing config in that window got clobbered — on a first run that is exactly when telemetry mints `anonymousId`. Concurrent-process loss drops from 15/40 to ~2/40, and what now usually loses is the tip's own flag (it simply shows once more) rather than telemetry identity. The residual is the non-atomic read-modify-write shape shared with telemetry's own writer. - `isInstalled()` uses stat().isFile(), so a directory at the install path no longer counts as an installed completion script. - Documented what `isInstalled()` actually promises: the script file, not the profile sourcing line that bash and PowerShell also need. Callers deciding whether to *advertise* completions want the loose reading — a user whose profile config failed has already met the installer. - Corrected a comment claiming the probe costs "one stat": detectShell() forks `ps` to read the parent process on every non-Windows run. Tests: mutation testing found four surviving mutants — dropping isCompletionRun from the defer policy, reverting isCiEnvironment to a CI==='true' string check, failing closed on an undetected shell, and neutering the non-object config guard (which lets a JSON array config be rewritten as {"0":...}). All four now fail a test. Adds direct coverage for the three new isInstalled() implementations, which had none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(validate): stop `change validate` exiting past commander's postAction `change validate` on a failing change called process.exit(exitCode). That tears down before commander's postAction hook, which is the same trap the `update` command documents 165 lines earlier: "exiting here would skip commander's postAction hook, killing the telemetry flush mid-request". A change that fails validation is a routine outcome, not an error, so this silently dropped the telemetry flush and — since the completions tip moved to postAction — the first-run tip for anyone whose first command was a failing validate. Verified under a pty: before, the tip never printed and completionTipSeen was never recorded; after, both happen and the exit code is still 1 (validate() already sets process.exitCode, which Node honours at natural exit — top-level `validate --all` has always relied on exactly that). The existing e2e in validate-scenario-loss.test.ts pins the exit code. Also wraps the postAction tip in try/finally so the telemetry flush runs even if the hint throws: program.parse() is synchronous, so a rejection there has no catch above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.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
|
f3aa167d6e |
feat(tools): add Zed Agent support (#1659)
* feat(tools): add Zed Agent support * fix(tools): detect Zed projects |
||
|
Clay Good
|
98c79324ac | docs(workflows): fix sequence diagram rendering (#1654) | ||
|
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 |
||
|
Patodo
|
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 Good
|
1a10dd5820 |
docs(opsx): clarify /opsx:sync description and add usage section (#1606)
* docs(opsx): fix /opsx:sync description and add detailed documentation - Changed description from 'Sync delta specs to main' to 'Merge delta specs into main specs' - Added detailed Usage section for /opsx:sync command - Now consistent with commands.md and migration-guide.md - Improves documentation completeness and clarity * docs(opsx): harden Sync delta specs section for accuracy and house style Fold the /opsx:sync usage entry into a single prose paragraph to match the six sibling Usage sections (heading -> fence -> paragraph), and fix two accuracy issues found against src/core/templates/workflows/sync-specs.ts: - Drop the invented "changes see each other's specs" and "test integration" use cases (no cross-change propagation or test step exists). - State that sync applies the whole delta -- a REMOVED requirement is deleted from the main spec and a RENAMED one retitled -- so the section no longer reads as additive-only. - Use the file's spaced em-dash convention. Docs-site build verified: sync-docs + fumadocs next build compile and render /docs/opsx end-to-end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Howard <yhwelcome1981@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
137404b423 |
fix(cli): reject missing roots for list and validate (#1612)
* fix(cli): reject missing roots for list and validate * test(cli): cover legacy list root fallback |
||
|
Clay Good
|
07dea6ed2f |
fix(update): don't hijack the agents target on legacy Codex upgrade (#1522)
* fix(update): don't hijack the agents target on legacy Codex upgrade Codex and the vendor-neutral `agents` target share `.agents/skills`. In upgradeLegacyTools, a Codex install inferred only from global ~/.codex/prompts wrote Codex skills into `.agents` and flipped the ownership marker agents -> codex, silently rewriting an existing agents-owned tree. The main generation path reconciles shared-target ownership first; this legacy-upgrade path did not. Add sharedSkillRootOwnedByOther() and skip generation when a different tool already owns the shared root (marker or existing tree), while still allowing a genuine first-time Codex upgrade with no `.agents` yet. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(update): cover the hijack guard end-to-end; name the owner on skip Harden the agents-target ownership fix after a multi-agent review: - Add an integration test that runs the real update flow for the bug scenario (agents-owned .agents + a legacy global Codex prompt) and asserts the marker stays `agents` and skills keep generic `/openspec-` syntax. A unit test of the predicate can't catch a future refactor that stops calling it; this can. - Name the owning tool in the skip message ("...managed by another tool (Shared .agents skills)") via a new sharedSkillRootOwner() helper that sharedSkillRootOwnedByOther now delegates to. - Add a unit case for the ambiguous-tree branch (existing skills, no marker, no inferable syntax) and one asserting sharedSkillRootOwner names agents. - Document the known, harmless re-offer tradeoff (a skipped tool isn't recorded as configured, so a persistent legacy prompt re-offers it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(update): preserve skipped tool's legacy files; add upgrade-path tests Adversarial review of the shared-root ownership guard surfaced one real integration defect and the review asks from alfred/CodeRabbit. Defect: when the guard skips a legacy Codex upgrade because the `.agents` root is owned by another tool, the caller's immediate legacy cleanup still deleted Codex's repo-local `.codex/prompts/openspec-*.md`. That violates the cleanup contract (remove X only because replacement Y was written): no replacement is written for a skipped tool, so its legacy files must stay. `upgradeLegacyTools` now reports `skippedSharedSkillTools`, and `performImmediateLegacyCleanup` exempts those tools' repo-local artifacts via a new `omitToolLegacyArtifacts` helper. Refactored the per-artifact tool matching out of `getToolsFromLegacyArtifacts` so both share one matcher. Tests (addressing the review + the defect): - update.test.ts: hijack test now asserts Codex is absent from the persisted configured-tool set and that the skip names the established owner. - update.test.ts: inverse no-root case proves a first-time Codex upgrade still writes the `codex` marker via the real UpdateCommand path. - update.test.ts: a skipped tool's repo-local `.codex/prompts` is preserved. - legacy-cleanup.test.ts: unit coverage for omitToolLegacyArtifacts. - shared-skill-target.test.ts: assert sharedSkillRootOwner resolves 'agents'. Docs + changeset updated to describe the preserve-on-skip behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(update): lock in legacy-prompt preservation on skipped Codex upgrade Address the outstanding CodeRabbit review notes on #1522. The fix itself is confirmed correct by three independent adversarial reviews — these are test-only hardening that locks in the guarantees the fix promises: - Assert the global ~/.codex/prompts survives (byte-for-byte) in the hijack scenario. Previously the test set the prompt up but never checked it was preserved; on unfixed code Codex would be generated, its 'explore' workflow would read as installed, and the deferred global cleanup would delete the prompt — so this assertion fails without the fix. - Assert the repo-local .codex/prompts is preserved by content, not mere existence (distinguishes 'left untouched' from 'deleted+rewritten'). - Restore the stdout/stderr spies in a finally so a throw can't swallow output for the rest of the suite. - Cover backslash-delimited (Windows) paths in omitToolLegacyArtifacts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
9ae75c86ef |
fix(archive): don't write ANSI escape codes to a redirected (non-TTY) stdout (#1603)
* fix(archive): stop non-TTY confirm prompts from writing ANSI escapes to stdout `openspec archive` asks up to three yes/no questions through @inquirer's `confirm`, which renders by writing ANSI cursor-movement escape sequences — and emits them even when stdout is not a TTY. When archive runs with its output captured to a file or pipe (an agent's background task, CI), those escapes are noise, and in some non-TTY hosts the render loop never settles and repeats `ESC[NNG` moves until the disk fills (reporter hit 19.8 GB). Add `confirmPrompt` in interactive.ts: a real terminal (stdin AND stdout TTY) still gets @inquirer's rich prompt; every other case reads one plain line via node:readline with `terminal:false`, emitting no escapes. Parsing mirrors @inquirer/confirm exactly (prefix match on y/yes and n/no, else the default), and an unreadable stdin rejects with an ExitPromptError-shaped error so the existing #1479 "rerun with --yes" guidance is unchanged. archive's confirmOrBlock now calls confirmPrompt. Closes #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(interactive): cover Windows CRLF and drained-stdin paths; doc note Adds two regression tests surfaced by adversarial review of the #1526 fix: - Windows CRLF piped input (`y\r\n`) parses as a clean yes with no ANSI — the reporter's platform, previously untested (all inputs used `\n`). - A second prompt after stdin was already drained blocks with an ExitPromptError instead of hanging, exercising the readableEnded guard. Also documents in troubleshooting.md that a redirected/agent archive run that pipes an answer no longer writes terminal escape codes into the capture. Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(interactive): align non-interactive classification and handle readline errors Addresses two review findings on the #1526 confirm-prompt fix: - confirmPrompt drops to the plain reader whenever either stream is not a TTY, but isNonInteractivePromptError only checked stdin. A stdin-TTY / stdout-redirected run that hit EOF leaked the raw ExitPromptError instead of the #1479 "rerun with --yes" guidance. Classification now also counts a redirected stdout, matching how the prompt mode is chosen. (isInteractive, used broadly elsewhere, is left untouched.) - readYesNo never listened for the readline/input 'error' event, so a stdin error would hang the promise (and go unhandled). It now settles with the underlying fault, guarded so the promise resolves or rejects exactly once. Tests: TTY-stdin/redirected-stdout EOF is classified non-interactive; an erroring input stream rejects instead of hanging; the archive usable-terminal test now models a full terminal (both streams TTY). Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(archive): gate the change picker on a TTY and tidy the reader Follow-ups from a second review round: - selectChange (the no-argument change picker) called @inquirer's `select` unconditionally. `select` writes ANSI escapes to stdout even when redirected — the same #1526 mechanism the confirm prompts were fixed for — so `openspec archive > log.txt` with no change name still spewed cursor moves into the capture before blocking. Refuse before rendering when either stream is not a TTY, with the same "pass a change name / --yes" guidance the caught ExitPromptError already gives. A new test asserts the picker is never reached in a non-terminal run. - readYesNo now removes its input-stream 'error' listener on every settle path (it lives on the long-lived process.stdin) and closes the readline interface on error too, so nothing accumulates across archive's sequential prompts. - troubleshooting.md now notes the picker also stays clean. Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(changeset): add patch changeset for the archive non-TTY fix (#1526) User-facing patch note for the archive ANSI/disk-fill fix. Also drops an unnecessary optional-chain on the non-nullable readline handle in readYesNo (the listener is only attached after the interface exists). Refs #1526 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
83be9d113e |
feat(validate): add --archived to lint task completion of archived changes (#1604)
* feat(validate): add --archived to lint task completion of archived changes `openspec validate --archived` scans every change under changes/archive/ and fails (exit 1) if any has unchecked tasks in tasks.md. This catches changes archived with unfinished work — which the normal validate flow never sees, since it only looks at active changes — and is meant for a pre-commit or CI hook. It is a standalone, opt-in scope: it returns before any existing bulk path, so no current `validate` invocation changes behavior, and it does not re-validate already-applied spec deltas. Reuses getTaskProgressForChange (the same counter status/list/archive use) so task counting never forks, and reads root.archiveDir so it is store-aware. Closes #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(validate): fail loudly on archive read errors and unreadable task files Address adversarial + CodeRabbit review of `validate --archived`: - listArchivedChangeIds now returns [] only for ENOENT (missing archive dir) and rethrows permission/I/O/ENOTDIR errors, so a real archive-read failure exits 1 instead of silently reading as "no archived changes". - Add getTaskProgressDetailForChange, which reports task files that exist but cannot be read; --archived turns those into an ERROR (naming the file) rather than silently counting them as zero tasks. The shared getTaskProgressForChange now wraps it and drops the detail, so status/list/archive totals are byte-identical. - Start the spinner after listing so a thrown listing error never leaves a spinner running. Adds regression tests (archive path is a file; archived tasks.md is unreadable) and unit tests for the new detail variant. Docs: align the --archived table verb and add a troubleshooting one-liner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(validate): address CodeRabbit nits on archived-task tests - Build the unreadable-fixture path from separate path.join components instead of a hard-coded Unix-separator string. - Assert the reported unreadable path (canonicalized with realpathSync.native), not just the count, so a wrong path can't pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(validate): address round-3 review of --archived From three fresh adversarial reviews (scale/perf, flag/output-shape, filesystem/security): - perf: memoize schema→glob resolution across archived changes via a run-scoped SchemaGlobCache, so the same schema.yaml isn't re-parsed once per change (the archive is append-only and can hold thousands). Threaded as an optional arg; existing callers are unchanged. Loop stays sequential by design (per-change work is synchronous) — now documented. - output shape: issue `path` now follows validate's convention — 'tasks.md' for incomplete tasks, and the POSIX root-relative file path for an unreadable file (one issue per file) instead of the bare 'tasks'. - plain output: print `change/<id>` (matching the JSON `type` and bulk validation) instead of `archived/<id>`. - docs: correct the "Never throws" docstrings (glob resolution can throw on a malformed/unsafe schema; the caller guards it) and note the load-bearing projectRoot override for the archive path depth. Store-mode resolution confirmed correct by review. Tests updated + a memo regression test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
59c16a4461 |
feat(tools): add Command Code command adapter for /opsx-* commands (#1622)
* feat(tools): add Command Code command adapter for /opsx-* commands Command Code documents custom slash commands under `.commandcode/commands/`, where the command name is the markdown filename without its `.md` extension (see https://commandcode.ai/docs/reference/slash-commands). That is the same flat naming Cursor and OpenCode use, so a standard flat adapter writing `.commandcode/commands/opsx-<id>.md` registers `/opsx-<id>`. Registering the adapter flips Command Code from `none` to `adapter-backed`, so with the default `both` delivery `openspec init` now generates OpenSpec commands alongside the skills it already installs under `.commandcode/skills/`. Builds on #1613, which registered Command Code as a skills-only tool. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(tools): preserve Command Code command arguments * test(command-code): cover commands-only delivery and openspec update Addresses review: prove the Command Code adapter survives both the commands-only init path and the update path, not just default delivery. - init: delivery=commands generates .commandcode/commands/opsx-explore.md and installs no skills. - update: a detected .commandcode install regenerates the flat opsx-<id>.md command (plain Markdown, $ARGUMENTS injected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ángel Peña
|
42d7f673bc |
feat(tools): add Command Code support as a skills-only tool (#1613)
* feat(tools): add Command Code support as a skills-only tool Register Command Code in AI_TOOLS with skillsDir `.commandcode`, so `openspec init`/`update` install the OpenSpec skills where Command Code discovers them (.commandcode/skills/<name>/SKILL.md) and reference them with the `/openspec-*` invocations its skill surface registers. No command adapter: Command Code has no slash-command files, so the skills-only target behaves like other adapterless tools and reports "Commands skipped for: command-code ^(no adapter^)". Co-authored-by: CommandCodeBot <noreply@commandcode.ai> * docs(supported-tools): add Command Code to skills-only invocation table Keeps the How-To-Invoke table consistent with the Tool Directory Reference row added for Command Code. Co-authored-by: CommandCodeBot <noreply@commandcode.ai> --------- Co-authored-by: CommandCodeBot <noreply@commandcode.ai> |
||
|
Clay Good
|
73207a6f2c |
feat(copilot): make cloud coding-agent files opt-in (#1517)
* feat(copilot): make cloud coding-agent files opt-in Selecting the `github-copilot` tool auto-generated a GitHub Actions workflow (.github/workflows/copilot-setup-steps.yml) plus an agent file. Writing into a user's CI on init/update is invasive, benefits only the narrow set of Copilot *cloud* coding-agent users, and couples us to GitHub's externally-owned custom-agent format. Cloud files are now opt-in: - `openspec init` prompts before generating them (default No) and records the choice in openspec/config.yaml (`githubCopilot.cloudAgent`). - `--copilot-cloud` / `--no-copilot-cloud` decide non-interactively. - `openspec update` never prompts; it only refreshes files for projects that opted in, or that already have generated cloud files (so existing setups keep working — the migration path). The pre-existing content-matching guarantees are unchanged and now proven by regression tests: a user-customized cloud file is never overwritten or deleted. Opt-in state is persisted via the YAML document model so the user's hand-authored config comments and formatting survive untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(copilot): polish the cloud opt-in — safety, UX, and docs Follow-up hardening driven by a five-agent review swarm over the opt-in. Correctness: - persistCopilotCloudOptIn no longer throws on a scalar/`null` config file (reproduced crash); it starts a fresh map while preserving comment-only and empty files. - Explicit opt-out (`--no-copilot-cloud` / `cloudAgent: false`) now removes OpenSpec-managed cloud files on both init and update, instead of orphaning them. Customized files are still never touched. - `--copilot-cloud` / `--no-copilot-cloud` warns when github-copilot isn't among the selected tools, instead of silently no-opping. UX / discoverability: - init prints whether cloud files were written or, when skipped for want of a signal, how to enable them (`--copilot-cloud`). - When the user opts in but already has their own copilot-setup-steps.yml or agent file, init/update say it was left untouched and that the OpenSpec install step must be added by hand — the direct answer to "will this affect my existing Copilot cloud agent?". - Clearer interactive prompt (names both files; distinguishes the GitHub-hosted cloud agent from Copilot in the editor); a dim, interactive-only, decision- gated hint on `openspec update`; tightened flag help text. Docs (the feature was undocumented): new "GitHub Copilot cloud coding agent" section in supported-tools.md; init flags in cli.md; the githubCopilot.cloudAgent key in customization.md. Tests: interactive prompt (accept/decline), opt-out removal + customized-file preservation, config.yml variant, scalar-config regression, collision reporting, flag-ignored warning, re-init honoring persisted opt-in, and the config parse/warn branches. 2763 tests pass; the only failures are pre-existing and unrelated (completion mocks, adapters loader, one config-profile PATH case, one experimental-alias case), verified identical on clean main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(copilot): make init cloud-file output honest; harden config guard Final hardening pass (adversarial review of the opt-in polish). - init's success line listed both cloud-file paths from the *decision* to write, not from what was written — so it claimed files that a write skipped (user already owns them) or that the alternate-agent path removed. It now lists only OpenSpec-managed files that actually exist after the write (listManagedCloudFiles), keeps the "left untouched" caveat for user-owned files, and reports opt-out removals in the normal output block. - persistCopilotCloudOptIn's non-map guard used isCollection, which is also true for sequences, so a YAML list at the config root still made setIn throw. Gate on isMap so scalars AND sequences fall back to a fresh document; empty/comment-only files still round-trip with comments intact. - Fixed a misleading catch comment on the opt-out removal path. Tests: success-line accuracy over a user-owned file, sequence-root config regression, and listManagedCloudFiles coverage. 318 tests pass across the touched suites; build + lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(copilot): replace a non-map githubCopilot node before setIn Addresses alfred review on #1517. The prior guard only fixed a non-map config *root*; a valid top-level map whose `githubCopilot` value is itself a scalar/null/sequence (`githubCopilot: false`, `null`, or a list) still made `setIn(['githubCopilot','cloudAgent'], ...)` throw, which init swallowed — so the explicit opt-in/out was never saved. Now the intermediate node is replaced with an empty map before descending, keeping the rest of the config and its comments intact. Regression covers all three reproduced cases (false/null/sequence). Full suite: 2770 pass; only the pre-existing unrelated failures remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(copilot): never throw persisting into an unparseable config Deeper pass on persistCopilotCloudOptIn (the function alfred flagged), driven by an exhaustive input-shape check. Two malformed inputs still threw at toString(): a multi-document YAML stream and a tab-indented (syntactically invalid) file. Such a file can't be edited without corrupting it, so persist now detects parse errors and leaves it untouched (no throw, no clobber) — it is already invalid, so readProjectConfig ignores it regardless. With this the function is throw-free across every shape exercised: empty, comment-only, scalar/sequence root, a non-map githubCopilot value, anchors, CRLF, BOM, and the two malformed cases (now skipped byte-identical). Regression added for the multi-document case. Touched suites: 314 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
13e213e00f |
feat(tools): add Atlassian Rovo Dev CLI as a first-class tool (#1516)
* feat(tools): add Atlassian Rovo Dev CLI as a first-class tool Rovo Dev CLI loads project Agent Skills from `.rovodev/skills/<name>/SKILL.md` (Atlassian docs), the same SKILL.md format OpenSpec generates. It was usable only via the generic "Shared .agents skills" fallback; this makes it a named, selectable target in `openspec init`. Rovo has no slash-command surface, so it is registered as an adapterless skills-only tool (like CodeArts/ForgeCode/Hermes) — no command adapter. Closes #212 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(tools): reference Rovo skills by natural language, not dead slash commands Rovo Dev CLI has no slash-command surface — it matches skills automatically or by prompt, and `/skills` only manages them. The generated skills and the getting-started hint still advertised `/openspec-*` slash commands (18 references across the skill bodies plus the "Start your first change" hint), so every one was a dead command. Adds a natural-language skill-reference path for no-slash tools: `/opsx:<id>` now renders as "the openspec-<skill> skill" for rovodev, in both skill bodies and the init hint. Other tools are unchanged. - src/utils/command-references.ts: NATURAL_LANGUAGE_SKILL_TOOLS + usesNaturalLanguageSkillReferences(); getSkillReferenceTransformer returns the prose transformer for rovodev. - src/core/init.ts: phrase the skills-only hint as an instruction for no-slash tools ("ask Rovo Dev CLI to use the openspec-propose skill…"). - docs/supported-tools.md: correct the Rovo row (was "use skill-based /openspec-* invocations"). - tests: assert generated Rovo skills contain no /openspec-* or /opsx slash tokens, the hint advertises no dead command, and the transformer emits prose. Addresses alfred-openspec review on #1516. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
aliouswe
|
d9bcc18582 |
docs(stores): add multi-repo implementation flow (#1491)
* docs(stores): add multi-repo implementation flow * docs(stores): qualify project pointer precedence --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
FasterPHP
|
622c509a13 |
fix(telemetry): honor telemetry.enabled in global config (#1513)
* fix(telemetry): honor telemetry.enabled in global config Honor the documented global config opt-out while preserving environment and CI overrides. Keep runtime-managed telemetry identity fields intact and apply the same privacy setting to update checks. AI: agentic * docs(telemetry): address automated review feedback Document the full opt-out behavior in the changeset and describe the new test helper so automated documentation coverage meets the project threshold. AI: agentic --------- Co-authored-by: Marcus Don <marcus.don@team.blue> |
||
|
Clay Good
|
59bfb27a76 |
fix(codex): install skills in canonical agents directory (#1511)
* fix(codex): install skills in canonical agents directory * fix(codex): preserve shared agents compatibility * fix(codex): harden shared skill migration * fix(codex): preserve customized legacy skills * fix(codex): reject malformed generated versions |
||
|
Wei Yunfay
|
161f9454a3 |
feat: add MiniMax Code skills support (#1214)
* feat: add MiniMax Code skills support to OpenSpec * fix: separate init skill and command output summaries * feat(minimax): add global skills support --------- Co-authored-by: showms <showms@users.noreply.github.com> Co-authored-by: Clay Good <hi@claygood.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
|
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 Good
|
4e4c9e1ffd |
docs(workflows): visualize the OpenSpec lifecycle (#1507)
* docs(workflows): add lifecycle diagrams * docs(workflows): clarify optional archive paths * docs(workflows): correct lifecycle diagrams * docs(website): render Mermaid diagrams * fix(website): preserve Mermaid label text |
||
|
Arda Kılıçdağı
|
1da6dfa8d7 |
Docs: add deno install instructions (#1079)
* docs: add deno install instructions * chore(docs): address pr feedback * chore(docs): address note feedback. --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay Good
|
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 Good
|
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> |
||
|
solanab
|
1aa0f2abfc |
feat(init): add shared agents skills target (#1303)
Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Suhaib Aslam
|
1014c59ed1 |
docs: catalog intent-driven community schema (#1487)
Co-authored-by: SuhaibAslam <SuhaibAslam@users.noreply.github.com> |
||
|
Mehdi Shahdoost
|
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 Good
|
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> |
||
|
Henry Su
|
10fa39b1c3 |
fix(update): refresh command files for tools configured without skills (#1442)
* fix(update): mark command-configured tools as needing update when skill version is missing * fix(update): compare command content fingerprint for commands-only tools when skill version is missing * test(update): isolate config homes in regressions * fix(update): keep skill drift detectable behind the command fingerprint Review follow-ups on the commands-only update fix: - Only fall back to the command-content fingerprint when a tool has no skill files at all. Gating on `generatedByVersion === null` also swallowed the case where a SKILL.md exists but its version is unreadable, so a truncated or hand-edited skill file could never be repaired by `openspec update` again. - Drop the command `generatedBy` scan: command adapters emit no version stamp, so the loop was unreachable and made the fingerprint fallback read as a secondary path rather than the only one. - Compute version status with the same workflow set the generation loop writes (`legacyWorkflowOverrides[toolId] ?? desiredWorkflows`), so a legacy-upgraded tool is not fingerprinted against commands it was never given. - Remove the unread `delivery` option from the three tool-detection signatures, the leftover `getCommandConfiguredTools` / `COMMAND_IDS` imports, and the unused `toolHasAnyConfiguredCommand` re-export. - runCLI: never let temp-dir cleanup replace the CLI result or a real failure, and treat an explicitly-empty XDG_CONFIG_HOME as an override. Adds regressions for the unreadable-skill case and for a deselected workflow leaving a command file behind, and documents how "up to date" is decided. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): ignore CRLF and BOM when fingerprinting command files Round-two review follow-ups: - Command files are committed project files. A Windows clone with `core.autocrlf` re-materializes them with CRLF endings, which the byte-exact comparison read as drift: every fresh checkout spent one `openspec update` rewriting identical content and announcing a bogus "unknown → <version>". Normalize CRLF and a leading BOM on both sides before comparing. - Collapse `getCommandConfiguredTools`, which the widened `getConfiguredTools` made a strict subset of itself, into the single remaining caller. - Correct the new `openspec update` doc paragraph: content drift is only detected for commands-only installs, so it must not promise that hand edits are always overwritten. - Add the changeset this repo requires per fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(update): drop dead exports and correct stale status doc comments `ToolVersionStatus.configured` and `.generatedByVersion` are now fed by command files too, so their comments no longer say "skills". Removes the barrel exports and the `options` parameter this change added but nothing consumes, and the import left dangling by the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): make the CRLF fixture idempotent on a CRLF checkout Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): cover non-claude adapters and a custom profile The fingerprint regressions all ran against claude and the core profile, so two things were covered by reasoning rather than by an executed test: - Command paths differ in shape per adapter. Added a parametrized case over gemini (nested dir, TOML), cursor (flat opsx-* file), and cline, whose commands live in .clinerules/workflows — not in its skillsDir (.cline) at all, so a commands-only install leaves that directory absent. Each asserts detection, a clean fingerprint, and drift. Reverting the getConfiguredTools widening fails all three. - A custom profile must be fingerprinted against its own workflow subset. The new case inits with ['explore', 'apply'] and asserts the same tree reads as drifted when compared against the wider core set. Making the fingerprint ignore the caller's workflows and fall back to global config fails it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Clay Good
|
6295515d4d |
feat(update): offer to upgrade a stale CLI during openspec update (#1470)
* fix(update): flag a stale global CLI during openspec update Instruction files are generated by the installed CLI, so running `openspec update` against an outdated global install printed "All 1 tool(s) up to date (v1.6.0)" while the workflows newer releases ship were never written. Users read that as success and reported the missing workflows as bugs. `openspec update` now checks the npm registry alongside the update and, when the installed CLI is behind, prints the upgrade command instead of leaving the up-to-date line to speak for itself. The check never gets in the way: it runs concurrently with the update, times out after 1.5s, caches the answer for 24h, returns null on any failure, and is skipped in CI, under tests, and whenever OPENSPEC_NO_UPDATE_CHECK is set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): name the right install in the stale-CLI hint The hint assumed a global install. A project-local dependency is now pointed at that dependency instead of `npm install -g`, and every hint prints the directory the running CLI was loaded from, so anyone who upgraded but still runs an old pnpm/volta/npx shim can see which copy answered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): drop the temp-file cache and fix prerelease ordering CodeQL flagged the version-check cache twice: a predictable path in the shared OS temp dir (js/insecure-temporary-file, high) and registry data written to that file (js/http-to-file-access, medium). `openspec update` is a rare, human-run command, so the cache bought little — removing it resolves both alerts outright and deletes the code that needed them. Also from review: CI=1 now opts out alongside CI=true, and prerelease tags compare per SemVer (dot-separated identifiers, numeric compared numerically) so 1.7.0-beta.10 outranks 1.7.0-beta.2. Build metadata is ignored per spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): make the version check actually reach the registry Adversarial review found the check could never fire: the request sent `accept: application/vnd.npm.install-v1+json`, which npm serves only on the full packument — on `/<pkg>/latest` it answers 406, so every real run returned null. Every test mocked fetch, so nothing caught it. The header is gone, and a new suite exercises the real fetch path against a local HTTP server, including an assertion that we never send that Accept type. Also from review: - Validate the published version against a strict SemVer pattern before printing it. It lands in the terminal beside an install command, so an unvalidated string could smuggle ANSI cursor controls and repaint the surrounding lines. - Honor DO_NOT_TRACK=1 and OPENSPEC_TELEMETRY=0, the opt-outs telemetry already respects, and update SECURITY.md, which promised telemetry was the only network egress. - Anchor project-local detection on the path being updated and its ancestors instead of process.cwd(), so `openspec update <path>` and workspace sub-packages with a hoisted root node_modules are no longer told to install globally. It can no longer throw when the working directory has been deleted. - Send npx/dlx users `npx @fission-ai/openspec@latest update` rather than advice that would create the global install they avoided. - Query npm_config_registry when set, so private mirrors get an answer their own install command can deliver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): make version-check fixtures portable on Windows The new fixtures mixed unresolved POSIX literals with path.join output. On Windows path.resolve adds a drive letter and path.join does not, so the prefix match could never succeed and two assertions failed there. Fixtures now derive from resolved roots. Real installs were unaffected — both sides come from resolved absolute paths — but case and drive-letter casing can still differ between require.resolve and path.resolve on Windows, so the comparison is now case-insensitive on win32. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): stop a blackholed registry from holding the CLI open Verification found the 1.5s timeout did not bound the command. Aborting a fetch still completing its TCP handshake — a firewall dropping packets, a captive portal — leaves the connect handle ref'd, so `openspec update` sat for ~10s after printing everything. Measured against an unroutable address: resolved at 1523ms, process exited at 10558ms. The request now uses node:http(s), whose socket the timeout can actually destroy: same probe resolves at 1547ms and exits at 1550ms. Because the client is no longer fetch, the mocked tests would have gone inert and silently reached the real registry. The whole suite now drives the real code path against a local server, which is also the only way to prove an opt-out sent nothing. Added a child-process guard for the teardown itself (no in-process assertion can see it), a case for a non-JSON body — the captive-portal login page — and order-independence fixes: the mock leak between describes made the 406 regression guard the first casualty under --sequence.shuffle. Also: bound the version pattern and the response body so neither can be absurdly long, and narrow the ephemeral-runner match so a user directory named "dlx" is no longer mistaken for a pnpm cache. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(update): offer to run the upgrade instead of only printing it Being told to run a command, then run the update again, is two steps the CLI can take for you. `openspec update` now asks: A newer OpenSpec CLI is available (v1.6.0 -> v1.7.0). Running from: /usr/local/lib/node_modules/@fission-ai/openspec ? Upgrade to v1.7.0 now? (Y/n) Yes runs `npm install -g` with stdio inherited — so any auth or sudo prompt reaches the user directly — then re-runs the update with the new CLI, because this process still holds the old templates and cannot write the new workflows itself. No prints the command and updates with the CLI you have. It asks rather than acting: a CLI that mutates a global install without consent is the wrong default. Guards: - Interactive terminals only, via the repo's isInteractive() (no TTY, or CI set, means the note prints exactly as before). - Global npm installs only. A project dependency belongs to that project's package manager, and an npx/dlx cache has nothing to upgrade; both get the command instead. - The re-run carries OPENSPEC_NO_UPDATE_CHECK=1, so a PATH that still resolves to the old binary cannot loop. - A failed upgrade, a missing openspec on PATH, and Ctrl-C at the prompt each fall back to the printed command rather than an error. The check now runs before the update rather than alongside it, so an accepted upgrade regenerates files with the new templates in one pass. Verified end to end against a stubbed npm and openspec on PATH, both answers, plus the unchanged non-interactive path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): offer the upgrade only where npm install -g would help Review found `canSelfUpgrade` treated "not a project dependency and not an npx cache" as proof of a global npm install. It is not: a pnpm, bun, yarn or volta global, and a plain git clone, all qualified. Reproduced by running the CLI from this repo — it offered to npm install -g over the checkout, which would have shadowed it with a second copy. The offer now requires npm to own the install, derived from the running node's global root (and APPDATA/npm_config_prefix) rather than by shelling out to `npm prefix -g`. Everything else gets the command that matches how it was installed — `pnpm add -g`, `bun add -g`, `yarn global add`, `volta install` — a project dependency is pointed at its own package manager with no npm command at all, and a source checkout gets no note, since its version is whatever the branch says. Docs corrected where they had drifted from the code: - The check runs before the update, not alongside it; it can delay the update by up to 1.5s. docs/cli.md and the changeset said otherwise. - npm_config_registry is only honored when npm exports it; an .npmrc setting alone is invisible to us. Docs and JSDoc claimed more. - SECURITY.md gains an "Installing software" row: running a package manager on the user's behalf is the most security-relevant behavior here and the table did not mention it. The "Running other programs" row now covers the re-run's path argument and cross-spawn's Windows shim escaping, and the network row lists every opt-out precisely. - troubleshooting.md's "Commands don't show up" — the exact symptom this PR exists to fix — now explains that instruction files come from the installed CLI, and installation.md's Updating section links onward. - The env-var table notes the CI and NODE_ENV skips, and that npm_config_registry must be an http(s) URL. - "the new workflows land in the same command" no longer overpromises: when the upgraded openspec is not on PATH, the CLI now says the files were not regenerated instead of printing a dim aside. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): make the upgrade offer tell the truth about what happened Adversarial review found the flow could claim success it had not earned, and could strand a non-interactive caller. All verified by running the CLI, all fixed: - `npm install -g` exits 0 even when it installs nothing, so "✓ Upgraded to vX" was an assertion, not a fact. The version is now read back from the installed binary; when another install earlier on PATH still answers with the old one — the exact silent staleness this feature exists to fix — it says so instead of claiming the upgrade landed. - The prompt hung forever under `openspec update > log.txt`: the question went to the file while the user watched a blank terminal. The offer now requires stdout to be a terminal too. - Ctrl-C at the prompt read as "no thanks" and carried on into the next prompt. It now stops the command with 130. - `--force` never reached the re-run, so `openspec update --force` could regenerate nothing and exit 0. Flags are forwarded, with `--` before the path so a flag-shaped path stays a path. - A signal-killed re-run, and a re-run with no CLI to hand off to, both reported 0. Both now report failure. - `process.exit()` skipped commander's postAction hook, killing the telemetry flush mid-request. The action sets process.exitCode and returns instead. - The check read only npm_config_registry, which npm exports only under `npm run` — so an enterprise user with a mirror in .npmrc got an unannounced call to public npm. It now reads .npmrc too. - Two different CI predicates: `CI=yes` suppressed the prompt but not the request. One predicate now, and it treats any value except an explicit off-value as CI. - A project-local install was offered a global one when updating a different directory; both anchors are checked now. Tests: the re-run had no coverage at all and now has four cases. Mutation testing over nine mutations (406 header, DO_NOT_TRACK, version validation, prerelease ordering, canSelfUpgrade, the anti-loop env guard, the cwd-vs-target anchor, the timeout) — one survived, the anti-loop guard, so it has a test now and the mutation dies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): compare re-run arguments as tokens, not as a raw line cmd.exe echoes `%*` with every argument quoted, so the Windows job saw `"update" "--force" "--" "--weird-path"` and the substring assertion for `-- --weird-path` failed. The forwarding itself was correct on both platforms; the assertion now splits and unquotes before checking that the separator immediately precedes the path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): read only the user's .npmrc for the registry CodeQL flagged file data reaching an outbound request, and it has a point: the project `.npmrc` travels with the repository, so honoring it let a cloned repo choose where the version check sends its request. Only `~/.npmrc` is read now — which is where a mirror is configured anyway, since `npm config set registry` writes there — and a test pins that a project `.npmrc` cannot redirect the request. Docs and changeset say so explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): detect the install from its own layout, not from node's path Adversarial review found the offer never appeared on Homebrew — a mainstream macOS install — and I reproduced it on this machine: npm root -g: /opt/homebrew/lib/node_modules derived roots: /opt/homebrew/Cellar/node/25.8.1_1/lib/node_modules process.execPath is realpath'd through Homebrew's symlink into the Cellar, so a root derived from the node binary never matches the prefix npm installs into. The same mismatch hits Debian-style layouts. The install's own shape is now the primary signal: <prefix>/lib/node_modules/<pkg> (POSIX) or <prefix>/node_modules/<pkg> (Windows), confirmed by the bin directory npm would have written the shim into. The node-derived roots stay as a fast path. Also from the same review, each reproduced first: - volta nests a whole node install, so its packages sit in exactly npm's layout: we called it npm-owned, ran `npm install -g`, and on failure told the user to run volta. Ownership is now decided before location. - upgradedBinPath returned the first prefix that merely had an openspec in it, preferring a stale one over the prefix npm just wrote to. It now derives from the running install first. - readCliVersion took the first version-shaped token anywhere in stdout, so a wrapper banner ("Node.js v25.8.1 | OpenSpec") was read as the answer — turning a real upgrade into a false "still reports vX", or worse, claiming success for a version nobody installed. It now takes the line that is only a version. - The probe child could outlive its 5s timeout indefinitely: SIGTERM with no escalation and no unref, so a signal-trapping wrapper held the CLI open for as long as it ran. - "Another install earlier on your PATH is answering first" was a misdiagnosis whenever we had asked a known binary directly. - A `registry=${VAR}` or `@scope:registry=` line in .npmrc — both npm's documented syntax, the latter being how a scoped package is normally routed to a mirror — silently fell back to the public registry. - A 3xx from the registry disabled the check permanently and silently. Redirects are followed, bounded, under one timeout budget. - An incidental directory named "pnpm" or "yarn" was read as a global install of one, printing the wrong upgrade command. Plus the earlier docs-audit round: the npx branch no longer tells users to run an update they were just handed, the check no longer fires for a source checkout whose answer is discarded, the offer gate moved into a tested pure function, and the declined command now prints below the update output instead of scrolling away above it. Docs: install-flavor table, CI off-values, empty-value opt-out, the "no cache" fact in SECURITY.md, and a changeset trimmed to a summary that points at the CLI reference. The changeset is now `minor` — this adds a prompt, an env var, an outbound request, and the ability to install software; that is not a patch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): stop reading .npmrc for the registry CodeQL flagged file data reaching an outbound request (js/file-access-to-http), and it is right that a file choosing where a request goes is a flow worth avoiding. The convenience did not earn it: reading ~/.npmrc needed three follow-up fixes in one review round (project-vs-user precedence, ${VAR} expansion, scoped registry keys), and none of it is necessary — anyone on a private mirror can export npm_config_registry, which is still honored, or turn the check off. Removes the .npmrc read and its two helpers; a test pins that a registry= line in a .npmrc cannot steer the request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Jikku Joyce
|
ec6cbb4b0b |
docs: add anvil to Community Schemas table (#1469)
* docs: add anvil to Community Schemas table Adds a row to the Community Schemas catalog in docs/customization.md for the anvil schema (jikkujoyce/openspec-schemas), a spec-driven workflow with TDD discipline and an adversarial review gate. Documentation only; the schema itself lives in its own repository. Generated with Cursor using Claude Opus 5. * docs(customization): describe anvil's review verdict as advisory The row said the VERDICT: line "gates test-plan, tasks, and apply", which reads as enforcement. OpenSpec's artifact graph only checks that artifact files exist, and the anvil bundle ships no CI or hook — its own schema.yaml and README say the gate is honored by the agent, not mechanically enforced. Reword to match, and backtick artifact names consistently across the cell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(customization): trim the anvil row to sibling length The cell ran nearly twice as long as any other row in the table. Drop the verdict-staleness rule and the 1:1 mapping detail — both are README material — and keep the flow, the adversarial review gate, its advisory caveat, and the test-plan ledger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Clay Good
|
9a61f3f30d |
docs(installation): add an AI-assistant setup prompt (#1466)
* docs(installation): add an AI-assistant setup prompt
Adds a provider-neutral "Install with your AI assistant" section to
docs/installation.md with one copyable prompt that detects the runtime and
package manager, installs the CLI, runs `openspec init --tools <id>`, and
verifies the result. Surfaced from the README Quick Start and the docs map.
The manual package-manager instructions stay the source of truth.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(installation): harden the AI-assistant prompt and link it from the install paths
Adversarial review found the first draft's verify step false-failing on healthy
installs and its guardrails unenforceable. The prompt now reports what init
actually printed instead of asserting config.yaml and command files (config.yml
is equally valid; six tools and delivery=skills correctly generate zero
commands), warns that --tools auto-cleans legacy files including opsx-*.md
prompts under $HOME, picks the package manager by what's on PATH rather than by
lockfile, scopes yarn to 1.x, and stops cleanly on EACCES, a missing pnpm global
bin dir, or a version-manager shim.
Also links the flow from getting-started, the docs map, troubleshooting, and the
website CTA; notes Berry dropped `yarn global`; replaces `npm bin -g` (removed in
npm 9) with `npm prefix -g`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(installation): close the gaps two trial runs found in the setup prompt
Two assistants (different models) ran the prompt end to end in sandboxes, one
on Cursor and one on Codex with deliberately messy legacy files. Both finished
with a working, verified setup. Their findings:
- Cursor's commands are `/opsx-propose`, not `/opsx:propose`. The prompt named
the colon form and init's summary agrees with it, so the assistant would have
handed back a command the tool doesn't match. It now takes the spelling from
the files init created.
- "List whatever you find and wait for my go-ahead" was undefined when the list
is empty, i.e. on every fresh project. It now says to carry on.
- `openspec --version` succeeding doesn't prove it's the copy just installed;
an older one earlier on PATH shadows it. Step 3 now compares the two.
- The request asked for confirmation before privileged/global changes; the
prompt only stopped reactively on failure. It now shows the global install
command and waits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: correct the core profile to six workflows and the tool count to 30+
Two long-standing inaccuracies, found while verifying the install docs.
`CORE_WORKFLOWS` (src/core/profiles.ts:14) is six — propose, explore, apply,
update, sync, archive — and a real `openspec init` generates six skills and six
commands. Eleven pages listed five, omitting `update`; migration-guide listed
four and filed `sync` under the expanded set. supported-tools also dropped
`update` from the full workflow-ID list. docs/commands.md was already right and
is untouched, as are flow diagrams that show a typical path rather than a
profile roster.
The tool count was written as both "25+" and "30+" against 34 supported tools.
Now consistently "30+".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: address CodeRabbit review on the AI-assisted install flow
- Windows puts global npm binaries directly in the prefix directory, not in a
`bin/` subdirectory; the troubleshooting fix I added said otherwise.
- Tell the assistant to stop rather than improvise when none of npm/pnpm/yarn/bun
is available, and point Nix users at the Nix section.
- Drop the blockquote on the getting-started pointer so it isn't a second `>`
block adjacent to the explore callout (markdownlint MD028).
Two other comments were already fixed in
|
||
|
Clay Good
|
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>
|
||
|
Clay Good
|
ebf66c7ee1 |
fix(init): skip the welcome animation for reduced-motion users (#1462)
* fix(init): skip the welcome animation for reduced-motion users The openspec init welcome animation had no off switch: it repainted eight frames on a 120ms loop with ANSI cursor-clearing, which is a seizure and nausea trigger for motion-sensitive users (#722). canAnimate() now also yields the existing static welcome screen when: - the OS reduced-motion preference is on (macOS Reduce Motion, GNOME animations disabled), detected best-effort with a 500ms timeout and animation kept on any lookup failure - OPENSPEC_NO_ANIMATION is set - the new init --no-animation flag is passed Closes #722 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(init): honor an empty OPENSPEC_NO_ANIMATION value Presence is what counts, like NO_COLOR: OPENSPEC_NO_ANIMATION= (set but empty) now also disables the welcome animation, matching the documented 'when set' behavior. CodeRabbit review follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(init): state animation-skip env semantics precisely Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Wei Yunfay
|
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> |
||
|
Clay Good
|
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 Good
|
6a5171e186 |
fix(validate): allow numeric-prefixed change names (#1435)
* fix(validate): allow numeric-prefixed change names `validateChangeName` required a leading letter, so `openspec new change 100-add-feature` or `00001-add-auth` failed with "Change name must start with a letter". This contradicted the rest of OpenSpec: the shared kebab-id grammar in src/core/id.ts (store ids, workset names, change metadata ids) already allows a leading digit, and archive explicitly supports `YYYY-MM-DD-` prefixed change names as a convention (#1309). Reuse the canonical `isKebabId` grammar for change names so numeric prefixes work, keeping the tailored error messages for the other failure cases. Fully backward-compatible: every previously valid name still validates (`[a-z]` ⊂ `[a-z0-9]`), and consecutive/leading/trailing hyphens, uppercase, spaces, underscores and other characters are still rejected. Closes #850 Closes #1169 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs, changeset, tests for numeric-prefixed change names Address review of the numeric-prefix change: - add the required changeset (patch) - fix docs/cli.md which still said names "cannot start with a number" and advised prefixing ticket IDs with a word (website copy regenerates from this file at build time) - pin the all-numeric case (`100`) so accepting it is a conscious decision Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: name the tiered-prefix test for what it covers CodeRabbit noted the 101-01-fix-auth fixture contains letters, so the old title 'all digits and hyphens' was inaccurate. Rename it to describe the tiered numeric-prefix case (#850); the dedicated all-numeric case is the separate '100' test. 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 Good
|
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 |
||
|
T
|
81d5109b86 |
docs: switch Roo Code references to Zoo Code (#1428)
* docs: switch Roo Code references to Zoo Code * no-mistakes(review): Remove unrelated AGENTS.md changes * no-mistakes(document): Update Roo Code references to Zoo Code; fix unused eslint directive --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay Good
|
27b22ab4cb |
feat(validate): accept zero-delta changes that declare skip_specs (#1399)
Squashed for rebase; see PR #1399 for the full commit history. |
||
|
Clay Good
|
1dc670deea |
fix(templates): stop propose from skipping the specs artifact (#1412)
Squashed for rebase; see PR #1412 for the full commit history. |