mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-09-14 20:16:53 +08:00
dependabot/github_actions/github-actions-03d71bef4d
276 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dwin Gharibi
|
aedf4d0c64 |
fix(archive): preserve blank lines inside code fences (#1798)
* fix(archive): preserve blank lines inside code fences * docs(test): document fence-preservation test helpers * chore(changeset): track the fenced blank-line fix The fix changes archive output for any spec documenting a fenced sample with consecutive blank lines, so it belongs in the changelog. Release tracking only validates changesets that exist; it never requires one, which is why CI stayed green without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dwin Gharibi
|
d9e1a28c38 |
fix(update): refresh generated files that drifted (#1808)
* fix(update): refresh generated files that drifted * test(update): isolate command drift from missing files and host config * chore(changeset): track the command-drift fix `openspec update` now reports and repairs tools it previously called up to date, so users will see a behavior change. That belongs in the changelog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dwin Gharibi
|
8251763ecd |
fix(parser): apply every delta section header (#1802)
* fix(parser): apply every delta section header * test(parser): assert section presence for a header after another section * chore(changeset): track the repeated-section fix Deltas that previously applied only part of what was authored now apply all of it, which changes archive output for affected changes. That belongs in the changelog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Dwin Gharibi
|
fadac3e1c9 |
fix(parser): accept all CommonMark list markers in deltas (#1800)
* fix(parser): accept all CommonMark list markers in deltas * docs(parser): document the REMOVED and RENAMED readers * chore(changeset): track the list-marker fix A removal or rename written with `*` or `+` now takes effect where it previously did nothing, so existing specs can change on the next archive. That is a user-visible behavior change and belongs in the changelog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay Good
|
3915db763a |
fix(guidance): teach the spec-inventory verb to generated guidance (#1700)
* fix(guidance): teach the spec-inventory verb to generated guidance `openspec list --specs` appeared in no generated skill, command, or artifact instruction, while `openspec list --json` — the in-flight CHANGE list — appeared throughout. An agent asked to read the existing specs first reached for the one enumeration verb it had been taught, got the change list, found it plausible, and reported the step complete against the wrong object. Explore now lists the spec inventory alongside the change list and says which is which. The spec-driven `proposal` and `specs` instructions name the command at the two points that need it: researching existing capabilities before filling in the Capabilities section, and confirming a delta's path matches an existing capability. Guidance text only — no CLI, parser, or archive behavior changes. Closes #1689 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(guidance): carry the store qualifier wherever the command is named A bare `openspec list --specs` reads the local inventory, so under a selected store it confirms a capability path against the wrong root. The proposal instruction carried the qualifier; the modified-capability instruction did not. All four sites now use the same wording, and the guard is scoped to the passage that names the command — every explore body already carries the qualifier in its unrelated capture steps, so a whole-body assertion would pass with it dropped here. Addresses CodeRabbit review on #1700. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(guidance): read a listed capability with the store-aware command The read step I added defeated the fix under a store. It told the agent to list the inventory with `--store "<id>"`, then read the result back from `openspec/specs/<capability-path>/spec.md` — a local path. Verified against a registered store: `list --specs --store mystore` returns `store-only-capability`, and the corresponding local read fails outright (or, when a local capability happens to share the name, silently returns a different one). That is the same wrong-object failure #1689 is about, reintroduced one line later. Capabilities are now read with `openspec show "<spec-id>" --type spec --json --no-scenarios`, which resolves against the same root the listing came from and returns purpose plus requirement texts without pulling whole spec files into context. `--type spec` is load-bearing: a change and a spec sharing a name is an ambiguous_item error, and change names routinely mirror capability names. Also documents `--store` on `list` and `show` in docs/cli.md. Both already accepted the flag — the prose at line 228 says so — but neither options table listed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: Use ASCII arrows instead of unicode This fixes the issue of ambiguous unicode character width when visualizing on terminals * fix: Update remaining docs within explore to use ASCII * fix(explore): finish the ASCII conversion and guard it Rebase onto main and close the gaps in the original fix: - Regenerate skills/openspec-explore/SKILL.md. The static skills/ mirror landed after this branch was cut, so the parity test would have failed with the template and the mirror out of sync. - Regenerate the three parity hashes through scripts/regen-parity-hashes.mjs. - Convert the ambiguous-width glyphs the first pass missed: the bullets in the CLI-storage example, and the check/cross marks in its comparison table, which sat in the column-aligned block the bug is about. - Tighten the ASCII guidance to two lines. It ships into every user project on both delivery surfaces, so the paragraph was pure overhead. - Add regression tests (#983): every fenced example in both the skill and the command body must be free of box-drawing, arrow, bullet, and check/cross glyphs, and the guidance must state the rule and the reason. - Add a patch changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(explore): cover every check/cross dingbat in the ASCII guard The matcher listed U+2713 and U+2717 only, so a fenced example could use ✕ (U+2715) or ✘ (U+2718) — same ambiguous width, same misalignment — and still pass. Widen to the U+2713-U+2718 run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(explore): require explicit confirmation before writing files * test(explore): harden write confirmation guardrail * fix(explore): scope write confirmation precisely * test(guidance): pin store-aware spec reads * test(templates): regenerate explore parity hashes The explore template now carries three independent guidance edits: the spec-inventory verb, the ASCII diagram conversion, and the write confirmation contract. Each pinned its own hash constants, so the pinned values no longer describe the combined template. Regenerate them from the merged source with `regen:parity-hashes` rather than hand-editing, and confirm the committed skills mirror still matches byte-for-byte. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(guidance): read complete specs before coverage decisions * docs: drop the redundant legacy docs/cli.md edit docs-lab/README.md makes docs-lab/ canonical and the old docs/ tree legacy. docs-lab/reference/cli.md already documents `--store <id>` for both `openspec list` and `openspec show`, so this branch's docs/cli.md rows added a third copy in the stale tree and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(changeset): drop the docs claim this PR no longer makes alfred-openspec on #1700: the release note still said docs/cli.md now documents --store on list and show, but that legacy-tree edit was removed from this head and the diff does not touch docs/cli.md. The canonical docs-lab/reference/cli.md already documented the flag on both commands, which is why the edit went. Removing the sentence rather than repointing it at docs-lab: nothing in docs-lab changed either, so there is no documentation change to announce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Shooks <justanormalme@gmail.com> Co-authored-by: Ayman D. <ayman.bacc@gmail.com> |
||
|
Clay Good
|
c170dc77ad |
fix(archive): read a wrapped scenario bullet as one bullet (#1782)
* fix(archive): read a wrapped scenario bullet as one bullet A repository that wraps its prose at a column limit writes most scenario bullets over two lines. The retirement guard read the continuation line as content the merge could not account for, so `retire_capabilities` refused every such spec - and because the hint that names the marker is gated on that same count, an unmarked author got the bare "must have at least one requirement" abort and never learned the retirement path exists. A line indented to the content column of the item above it, with no blank line between, is part of that item. It is accounted for when the item was and already reported when it was not, so nothing is deleted unmentioned either way. A blank line still ends the item, so a note written below the scenarios is still the author's own however it is indented. Closes #1780 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): keep an indented heading out of a bullet's continuation Continuation is for wrapped prose. A raw HTML heading indented under a scenario bullet was absorbed by it, so indenting a section one level would have smuggled it past the audit and deleted it with the file. ATX headings were already excluded; HTML ones now are too, matching how the pass above the requirements section reads them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): flag a setext heading indented under a bullet A setext underline turns the line above it into a heading, so indenting the pair one level under a scenario bullet let a whole section be absorbed as continuation and deleted with the file. Checked ahead of the continuation branch now, the same way the ATX and raw HTML forms already are. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): read an unindented wrapped bullet as one bullet too Not every wrap indents its continuation, and the indent-only rule left the reported bug fixed for one spelling and live for the other: a hand-wrapped scenario bullet still refused the retirement. Inside a scenario's unbroken bullet run a lazy continuation is now read as part of the bullet above it. This widens nothing - a sibling bullet written in that same position is already read as the scenario's own, and a lazy line is part of the bullet where a sibling is merely next to it. Past the blank line that ends the run the indent is still required, so a note bulleted below the scenarios and the line that wraps it stay the author's. Also covers CRLF specs, and asserts the refusal report names only the real leftover in a wrapped multi-requirement spec rather than burying it under continuations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): stop a lazy continuation at anything that opens a block CommonMark lets a blockquote, thematic break, table, list item or raw HTML interrupt a paragraph, so one written flush against a scenario bullet starts something new rather than continuing it. The lazy allowance absorbed all of them, which would have deleted an author's note with the file and named nothing. The bullet's paragraph is now tracked as its own state: opened by a bullet, closed by a blank line, a fence, a heading, or a line that opens a block - including one indented inside the item, whose own paragraph ends the bullet's. Lazy continuation applies only while it is open. Indented continuation is unaffected: a nested list or quote sitting inside the item is still the item's own content. Each of the six holes is pinned by a test proven to fail with the narrower rule removed. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): classify a line as the list item sees it A marker as wide as `100. ` puts the item's content past the three columns a Markdown construct is allowed at the file's left margin, so `## Retention` written inside such an item read as five spaces of nothing and was absorbed as continuation - a regression against the behavior before continuation existed, which named it. Every syntax test in the audit now reads the line with the item's indent removed, so a heading, a setext underline or a block start is recognized wherever the item sits. Found by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: drop test scratch directory committed by mistake `test-spec-command-tmp/` is a fixture a test run leaves behind, swept up by `git add -A` in the previous commit. It is not part of the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(archive): share one list-marker definition with the paragraph rule Folds in the marker coverage from the duplicate PR #1789, which fixes the same issue (#1780) with a shallower model. The audit named `-`, `*` and ordered items as list markers, while INTERRUPTS_PARAGRAPH, added in this same PR, already named `+` and capped an ordered marker at CommonMark's nine digits. The two disagreed, so a line one called a bullet and the other did not was read as both at once. Both now use one LIST_ITEM constant: - `+` is the behavior fix. A spec bulleted with `+` validates like any other, and every one of its scenario bullets was reported as unaccounted content, so that capability could not be retired at all. Regression added, verified to fail against the old marker set. - The nine-digit cap changes no verdict in this design, since a line the pattern rejects is weighed by the same rules either way. It is here for the consistency, and the comment says so rather than claiming a fix. The case is pinned so a later change cannot start deleting such a note. LIST_ITEM also no longer requires content after the marker, so an empty `- ` reads as the bullet it is instead of falling through to the leftovers, which is what the surrounding indent tracking already assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay Good
|
3c6d318b83 |
fix(init): name the workflows the profile left out (#1779)
* fix(init): name the workflows the profile left out Setup output listed the workflows it installed but never mentioned the ones it did not, so a user on the default core profile who typed /opsx:ff saw nothing and read it as a broken install. The docs explain profiles; nobody reads them before typing a command that should be there. init now closes with the missing workflows by name and the two commands that add them. The note is skipped when nothing was generated at all, where the existing delivery correction is the whole story, and when the profile already installs everything. Also adds a troubleshooting entry for the "only some /opsx: commands show up" symptom, which the existing list did not cover. Relates to #1076 (the optional-workflow discoverability half; the Windows command-discovery repro in that thread is not addressed here) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: drop a test scratch directory committed by mistake test-show-command-tmp/ is created by a test run and does not exist on main; it was picked up by a `git add -A`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(init): keep the workflow note off runs that generate nothing With no tools selected (or only tools that could not receive a surface), `openspec config profile` followed by `openspec update` writes nothing, so naming the missing workflows pointed at the wrong problem. Reported by CodeRabbit on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(init): drop the redundant update step from the workflow note `openspec config profile` offers to apply to the current project before it exits, and prints the `openspec update` guidance itself when the user declines, so naming a second command was one step too many. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(troubleshooting): match the profile steps to what the CLI does `openspec config profile` applies to the current project itself, so listing `openspec update` as a second required step was wrong; it is the fallback for declining the prompt or for other projects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(update): name the workflows the profile left out `openspec update` is what the troubleshooting checklist tells a user to run when a command they read about never appeared, and it is what people run after upgrading the CLI. Neither of its existing profile notes fires on the default `core` profile, so that user reached "All tools up to date" and still learned nothing about the six workflows they don't have. The note is the fallback pointer: silent when the extra-workflow or missing-core note already named `openspec config profile`, and when no configured tool can receive a workflow surface under the active delivery. Reading the two existing notes as one short-circuited `||` would have swallowed whichever ran second; they are evaluated separately. Relates to #1076 (the optional-workflow discoverability half; the Windows command-discovery repro in that thread is not addressed here) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(update): gather the profile notes behind one call The two call sites had grown identical six-line blocks. One displayProfileNotes() keeps the ordering and the single-pointer rule in one place, where the "evaluate every note, never chain them with ||" constraint can be stated once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: drop the legacy troubleshooting entry alfred-openspec on #1779: docs-lab/README.md says the old docs/ tree is legacy, is no longer used by the site, and must stay untouched. The canonical docs-lab/customize/profiles.md already lists the six optional workflows and the 'openspec config profile' command that adds them, and the root README already calls out the expanded set, so this entry was a third copy in a stale tree. The docs-lab troubleshooting page is a heading-only skeleton held back from the site, so there is nothing to move it to; this PR is now source and tests only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay Good
|
6d2dbe62d3 |
fix(propose): load project context before planning (#1657)
* fix(propose): load project context before planning * test(propose): assert project context is applied * fix(propose): honor project context limits * fix(propose): fail closed on unsafe context * fix(propose): skip config without a root * chore(parity): regenerate hashes after merging main * fix(propose): harden early context loading guidance * fix(propose): require initialization before planning in bare repos --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Marzx13
|
db03c6c4b0 |
feat(validate): add findings-only bulk reports (#1713)
* feat(validate): propose findings report * docs(validate): clarify findings report contract * feat(validate): implement and harden bulk findings reports * test(validate): canonicalize store paths natively --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Ryan de Melo
|
a4fcdbece6 |
feat(validate): report the deltas archive would refuse (#1710)
* feat(validate): report the deltas archive would refuse validate checked a change's deltas against themselves and, for MODIFIED blocks, against the main spec's scenarios. It never checked whether the main spec can supply the target a delta acts on, so a MODIFIED naming a requirement that is not there, a RENAMED whose source is gone, or an ADDED whose name already exists all validated clean and failed at archive instead - typically weeks later, after the implementing PR had shipped and the authoring session was gone. Run the merge archive runs and report what it refuses. buildUpdatedSpec returns the rebuilt content without writing it, so the preflight is the same function on the same inputs with the result discarded, and cannot disagree with the code that does the writing. That matters here: several of those preconditions deliberately read a missing target as already-synced rather than as a failure, and a second copy of the rules would be free to drift. Reported as INFO so no verdict changes in any mode. A MODIFIED whose target is missing is also what a change modifying a sibling's unarchived requirement looks like, and validate stays valid for that case today; telling the two apart needs the opt-in marker #1112 asks for. What is missing until then is the information, not the verdict. Refs #1112 * fix(validate): skip preflight for deltas whose errors come after the loop missingHeaderSpecs and emptySectionSpecs are collected inside the per-spec loop but only become issues after it, so a suppression set built from the issues raised so far could not see them. A headerless or empty-section delta has nothing for the merge to apply, so the preflight reported that as a blocker of its own, on top of the error that names the actual mistake. * fix(validate): harden archive preflight diagnostics * fix(validate): preserve reports when archive preflight cannot start --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Br1an
|
0296401b82 |
fix(init): add .gitkeep files to empty directories (#786)
* fix(init): add .gitkeep files to empty directories After running openspec init, the specs/, changes/, and changes/archive/ directories are empty. Since git does not track empty directories, these folders are lost when the repository is cloned, causing openspec list to recommend re-initialization. Added .gitkeep file creation to createDirectoryStructure() for both normal and extend modes, ensuring empty directories are preserved in version control. Fixes #269 * fix(init): preserve directory anchors without overwriting user files --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Aron Lee
|
cd724449ac |
refactor(core): share the IDE restart hint between init and update (#1725)
* refactor(core): share the IDE restart hint between init and update
init named the surface it generated ("the new commands" / "the new skills");
update printed a generic "changes" for the same event, decided by the same
rule. Extract that rule and its wording into shared/ide-restart.ts so both
commands say the same thing, and update now names the surface too.
No condition changed: the hint still requires one tool that is both
IDE-resident and actually received a generated surface under the active
delivery, so a CLI tool's commands can never speak for an IDE tool that got
nothing.
Verified by mutation: dropping the IDE-resident filter turns 9 tests red
across the helper, init and update; swapping the commands/skills precedence
turns 5 red.
* test(core): harden shared IDE restart guidance
* fix(core): describe restart guidance for removed workflows
---------
Co-authored-by: Clay Good <hi@claygood.com>
|
||
|
Clay Good
|
98bf53e59e | fix(workflows): ground proposals in relevant project code (#1737) | ||
|
Dan (Danilo) Rio (Ribeiro)
|
b976106d95 |
fix(explore): guide planning with focused discovery questions (#1017)
* feat: improve explore discovery questions * chore: add changeset for explore guidance --------- 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> |
||
|
Shooks
|
e5e350d04b |
fix(explore): use ASCII in diagram examples (#1010)
* fix: Use ASCII arrows instead of unicode This fixes the issue of ambiguous unicode character width when visualizing on terminals * fix: Update remaining docs within explore to use ASCII * fix(explore): finish the ASCII conversion and guard it Rebase onto main and close the gaps in the original fix: - Regenerate skills/openspec-explore/SKILL.md. The static skills/ mirror landed after this branch was cut, so the parity test would have failed with the template and the mirror out of sync. - Regenerate the three parity hashes through scripts/regen-parity-hashes.mjs. - Convert the ambiguous-width glyphs the first pass missed: the bullets in the CLI-storage example, and the check/cross marks in its comparison table, which sat in the column-aligned block the bug is about. - Tighten the ASCII guidance to two lines. It ships into every user project on both delivery surfaces, so the paragraph was pure overhead. - Add regression tests (#983): every fenced example in both the skill and the command body must be free of box-drawing, arrow, bullet, and check/cross glyphs, and the guidance must state the rule and the reason. - Add a patch changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(explore): cover every check/cross dingbat in the ASCII guard The matcher listed U+2713 and U+2717 only, so a fenced example could use ✕ (U+2715) or ✘ (U+2718) — same ambiguous width, same misalignment — and still pass. Widen to the U+2713-U+2718 run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(explore): require explicit confirmation before writing files * test(explore): harden write confirmation guardrail * fix(explore): scope write confirmation precisely --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Ayman D. <ayman.bacc@gmail.com> |
||
|
bsmedberg-xometry
|
dd7cea3ffe |
feat(show): diff delta requirements against the main specs (#980)
* Proposed feature: openspec show --diff to see changed requirements more clearly * Implementation of the --diff feature, which led to some spec changes during implementation. * Update the proposal to be clearer. Thanks coderabbit * Fix a bug identified by coderabbit with excessive trimming, and add a testcase for it. * fix(show): harden --diff for review feedback Keeps `openspec show <change>` without `--diff` a raw proposal passthrough, reports when a change has no delta specs instead of returning silently, preserves the Reason/Migration body of a REMOVED requirement, and resolves main specs through the command's root so `--store <id>` diffs against that store. Text mode and JSON mode now render from one shared collection pass, the CLI tests drive argv arrays from a mkdtemp project instead of interpolated shell strings, `--diff` is registered for shell completions, and the stray package-lock.json is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * build(deps): declare the diff dependency and refresh the flake hash Adds the `diff` runtime dependency that requirement-diff.ts imports, updates pnpm-lock.yaml, and regenerates the flake's pnpmDeps hash so `nix build` matches the new lockfile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(show): diff nested capabilities too collectSpecDiffs enumerated only top-level directories under the change's specs/, so a nested capability (specs/<area>/<id>/spec.md) was skipped: text mode printed nothing for it and its MODIFIED deltas came back from --json with no diff. It now uses the same discoverSpecFiles() helper ChangeParser uses, so the capability ids match the `spec` field of the JSON deltas. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(show): report header mismatches instead of hiding them Two cases where --diff quietly showed something misleading: A MODIFIED requirement whose capability has no main spec was rendered as all-additions, which reads like a new capability. It is an authoring error archive will reject, so it now prints the raw text with a warning naming the missing spec. A header that differs from the main spec only in case or interior spacing found no match at all under exact lookup, or matched under a lowercase-only comparison that let a real mismatch through silently. Lookup is now exact first, then the shared foldRequirementName fallback, and a folded match prints the diff the author meant alongside a warning that archive matches names exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(show): name the main spec as such in collectSpecDiffs Comment and locals still called the main spec the "base" spec, and the no-main-spec comment described the old all-additions behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(deps-dev): bump the development-dependencies group with 2 updates Bumps the development-dependencies group with 2 updates: [smol-toml](https://github.com/squirrelchat/smol-toml) and [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint). Updates `smol-toml` from 1.7.1 to 1.8.0 - [Release notes](https://github.com/squirrelchat/smol-toml/releases) - [Commits](https://github.com/squirrelchat/smol-toml/compare/v1.7.1...v1.8.0) Updates `typescript-eslint` from 8.66.0 to 8.67.0 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.67.0/packages/typescript-eslint) --- updated-dependencies: - dependency-name: smol-toml dependency-version: 1.8.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: development-dependencies - dependency-name: typescript-eslint dependency-version: 8.67.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: development-dependencies ... Signed-off-by: dependabot[bot] <support@github.com> * chore(nix): invalidate pnpm dependency hash * fix(nix): update pnpm dependency hash * fix(show): harden requirement diff output * build(nix): pin combined dependency hash * test(show): assert proposal precedes diffs * docs(show): clarify JSON diff diagnostics * fix(show): retain unified diff hunk headers --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Leonardo Rojas
|
ab81a4b43a |
fix(completions): stop Fish completions falling back to filenames (#1199)
* fix(completions): suppress filesystem fallback in Fish completions * fix(completions): force files back on for path positionals in Fish Fish never restores filesystem completion once a matching rule sets --no-files, so a path positional needs an explicit --force-files rule. Without it, `openspec store register <TAB>` lost file completion because the sibling subcommand rules in the same context now carry -f. Also drop retired "context store" vocabulary from the test fixtures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): preserve workset member paths in fish * fix(completions): force Fish path fallback * fix(completions): harden Fish option handling * fix(completions): scope Fish path fallback * fix(completions): match Fish command paths exactly * fix(completions): skip parent options in Fish paths --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Alfred
|
109f81f17d |
fix(antigravity): update skill and workflow paths from .agent to .agents (#830)
* fix(antigravity): update skill and workflow paths from .agent to .agents Antigravity has migrated from .agent (singular) to .agents (plural) for workspace skills and workflows. The old .agent path still works via backward compatibility, but the official docs now specify .agents. Changes: - config.ts: skillsDir '.agent' -> '.agents' - antigravity adapter: workflow path '.agent/workflows/' -> '.agents/workflows/' - legacy-cleanup: add patterns for old .agent/ artifacts cleanup - docs: update supported-tools.md table - tests: update expected path assertions Closes #0 (reported by BugsCreator and Minh Pham in Discord) * fix(antigravity): migrate an existing .agent install to .agents Pointing Antigravity at `.agents` leaves every existing `.agent/` install behind, so this registers the move instead of only changing the target: - `.agent` becomes Antigravity's legacy skills root and legacy tool root, so update relocates managed skills and commands after generating their replacement, keeping a file the user customized. - Detection keys off `.agent` and `.agents/workflows`. The bare `.agents` root is shared with Codex, Zed, and the vendor-neutral target, so it cannot stand in for "Antigravity is set up here". - Delivery inference reads a tool's legacy roots for command files too. Without it the first update after the move saw skills but no commands, wrote `delivery: skills`, and the next update deleted every slash command. - Legacy slash-command cleanup stays scoped to the pre-opsx `openspec-*` names under `.agent`; the migration owns the `opsx-*` files, and a shared root is never glob-swept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: record the Antigravity root move Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(antigravity): require a generated command before moving its legacy copy migrateCommandFiles relocated a legacy command file whether or not the current root held a replacement. Codex never reached that path — it has no command adapter — so Antigravity is the first after-generation move where it matters: under skills-only delivery, or for a deselected workflow, the move recreated a command OpenSpec had just decided not to install. Gate the move on an existing destination, the same way migrateSkillDirs already does for after-generation timing. Reported by CodeRabbit on #830. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(antigravity): arbitrate shared agent skills * test(antigravity): cover Windows migration paths * fix(antigravity): harden shared-root migration --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Marzx13
|
04b37ac1d5 |
fix(archive): preserve requirement order when renaming (#1712)
* fix(archive): preserve requirement order when renaming * chore(archive): add rename-order changeset --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
mark
|
126c5d6c59 |
fix(validate): report a Purpose left as the archive placeholder (#1671)
* docs(openspec): propose warn-on-purpose-placeholder
When a delta introduces a capability with no usable `## Purpose`, archive
writes `TBD - created by archiving change <name>. Update Purpose after
archive.` into the new main spec. Three places already tell authors to
replace it -- the `specs` instruction ("including a leftover `TBD`
placeholder"), the sync-specs summary step ("so it gets written now rather
than lingering"), and the cli-archive contract -- but nothing reports that
it is still there.
`--strict` cannot reach it. The check meant to catch a Purpose nobody wrote
is a 50-character floor and the placeholder is 91 characters, so the one
rule that exists to catch a thin Purpose is satisfied by the exact text
meaning nobody wrote one: a Purpose reading "Does stuff." fails --strict
today, while one saying nothing at all passes.
Proposes reporting it as a warning against the spec's Purpose -- silent by
default, failing under --strict, so a project already carrying placeholders
keeps validating until it opts into the stricter gate. Detection is narrow:
the generated sentence wherever it appears, and otherwise only a `TBD`
opening the Purpose, so prose raising an open question is left alone.
Planning artifacts only; no source changes.
Refs #369
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(validate): report a Purpose left as the archive placeholder
When a delta introduces a capability with no usable `## Purpose`, archive
writes a placeholder into the new main spec. Nothing read it afterwards, so
the capability kept a to-do in it while every command reported success.
`--strict` could not reach it. The check that exists to catch a Purpose
nobody wrote is a 50-character floor, and the placeholder clears it: a spec
whose Purpose read "Does stuff." failed --strict, while a spec whose Purpose
said nothing at all passed. #369 reported agents leaving the placeholder
behind and stayed open seven months; every remedy since has been an
instruction, which is the mechanism that report described as unreliable.
validate now reports it as a warning against the Purpose, naming the line
and saying to edit the main spec directly -- a delta's `## Purpose` is read
only when the capability is created, so it cannot replace an existing one.
Warning rather than error, because strict mode already means "warnings
fail": a project carrying placeholders keeps validating by default and only
--strict fails. Archive is untouched -- it validates rebuilt specs without
--strict, so a spec archive writes still passes the validation it would have
passed before, and the text archive writes is byte-identical.
The placeholder is recognised through the same constants the writer composes
it from, so the check cannot drift from the sentence it looks for -- the
failure mode of a second, hand-copied spelling being a check that matches
nothing and looks exactly like a check that found nothing. The one case that
cannot be a lookup is an agent-written placeholder, kept to a `TBD` opening
the Purpose: "the retry budget is TBD pending benchmarks" is authored prose
and is left alone.
Verified: 209 archive tests pass unchanged (the placeholder text is
asserted literally, so the output is provably identical); full suite 138
files / 3993 tests; 36/36 strict spec validations; build, lint and typecheck
clean. Against a project carrying four real placeholders, default mode still
exits 0 and --strict fails exactly those four.
Cross-platform CI is not yet confirmed -- it needs a pushed branch. Line
endings are covered by tests asserting a CRLF spec and an LF spec produce
identical findings, and the module does no path handling.
Refs #369
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(validate): make every placeholder guard load-bearing
A mutation pass over the seven guards -- revert one, see which tests die --
found two that no test held.
The prefix/suffix test did not exercise the guard it named. Its Purpose read
"Explains what happens when archiving change my-change runs twice", which
contains neither half of the generated sentence, so it passed whether or not
the suffix was required. Matching on the prefix alone killed nothing. The
Purpose now embeds the real prefix constant and asserts the suffix is absent,
so the case is the one the name claims; the mutation kills it.
The empty-Purpose early return was genuinely dead. Neither rule matches empty
text, so removing the branch changed no behaviour and failed no test. Rather
than keep a guard nothing can hold, the branch is gone and the comment says
why an empty Purpose still yields null. The tests asserting that behaviour
are unchanged and still pass.
Every guard now dies under mutation:
whole check removed from applySpecRules ......... 6 tests
brevity no longer suppressed (else -> if) ....... 1
word boundary dropped from the TBD marker ....... 1
generated placeholder matched on prefix alone ... 1
line-ending normalisation removed ............... 2
section-boundary guard removed from locator ..... 1
Full suite 138 files / 3993 tests, lint and typecheck clean.
Refs #1670
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(openspec): record the mutation pass in the task list
The mutation work changed the implementation -- a test rewritten and a dead
branch removed -- but no task covered it, so the plan claimed less work than
was done. Added as group 6, marked complete, with why it was not planned.
5.4 now says what blocks it. It needs a pushed branch for the cross-platform
matrix, and the note records that line endings are covered locally by tests
asserting a CRLF spec and an LF spec produce identical findings, so a reader
can tell the difference between unverified and unverifiable-from-here.
The specs, proposal and design are unchanged and were checked: the delta's
empty-Purpose clause constrains behaviour, not structure, and that behaviour
is the same -- the redundant branch went, the rule did not.
26 of 27 tasks complete; the change still validates --strict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(openspec): close 5.4 on a green cross-platform matrix
CI dispatched on the fork against this branch: lint & typecheck, and the test
suite on linux-bash, macos-bash and windows-pwsh -- all green. The Windows job
installed, built and ran the suite rather than short-circuiting, which is the
part 5.4 existed to check, since the placeholder locator counts lines in files
that may carry either ending.
Recorded as a workflow_dispatch run on the fork, not the upstream pull-request
run, because those are not the same gate and the note should not let a reader
assume otherwise. Nix Flake Validation and Validate Release Tracking skipped:
this branch touches neither the flake nor release tracking.
27 of 27 tasks complete.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(validate): name the placeholder's line, not the prose above it
The warning tells you which line to fix, and named the wrong one when the
generated sentence did not open the Purpose:
3 ## Purpose
4 Handles widget retries. <- warning pointed here
5
6 TBD - created by archiving ... <- placeholder is here
The locator asked "what is the first non-blank line after ## Purpose?"
rather than "where is the placeholder?". Those are the same line in five of
the six shapes a placeholder can take -- a leading TBD marker is the first
non-blank line by definition, and archive writes the generated sentence as
the section's only content -- so the two questions only diverge when a human
types prose above a leftover placeholder.
Pointing at that prose is worse than pointing nowhere: the reader sees a
sentence that is plainly fine and concludes the check is broken. design.md
already said a wrong line number is worse than none, and the delta already
required naming the line the placeholder is on, so this is the
implementation meeting a contract that was already written, not a change of
contract.
The locator is now told which rule matched. A leading marker keeps the
first-non-blank behaviour, because that is where it sits; the generated
sentence is located by its own text. When both match the leading marker
wins, being the earlier of the two.
Found by CodeRabbit on #1671. The finding was real despite its own
"Addressed" marker, which only tracked the file changing in a later commit.
Two test gaps let it through. The case that covered this input asserted
only that something was reported, never which line -- so it now asserts the
line, and a table pins every position a placeholder can occupy, each case
first checking that the line it expects really carries the placeholder. The
mutation pass could not have caught it either: mutation proves a test dies
when a guard is broken, and cannot invent an assertion nobody wrote.
Reverting the branch fails exactly the three new expectations. Full suite
4000 tests / 138 files, lint and typecheck clean.
Refs #1670
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* refactor(validate): set the placeholder line unconditionally
ValidationIssue.line is optional and the project does not enable
exactOptionalPropertyTypes, so a plain assignment typechecks and
JSON output is unchanged - JSON.stringify drops undefined values.
findPurposePlaceholderIssue already returns the key unconditionally,
and the neighbouring push sites assign line plainly, so the
conditional spread was the odd one out.
* fix(validate): widen the placeholder check to TODO and read fences as quoted
#1670 left two questions open. Both are answered here, against how OpenSpec
already reads a spec.
A `TODO` opening the Purpose now reports as the same finding as a `TBD`.
Nothing OpenSpec writes produces one, but the marker an author leaves behind is
whichever word they reached for, and a Purpose reading `TODO: fill this in` is
as unwritten as one reading `TBD`. Only the opening position counts, as before,
so `TODOs are tracked in the linked issue` is still authored prose.
Fenced code inside a Purpose is now read as quoted material rather than as the
Purpose speaking, through the `buildCodeFenceMask` the requirement and structure
parsers already share. Without it a spec documenting the sentence archive writes
is reported as carrying it, which is the check failing the one document that
explains it - and a warning that fires on the docs teaches people to ignore the
warning. Fenced lines are skipped when locating the placeholder too, so a
`## Purpose` or `## Requirements` quoted in a fence can neither be mistaken for
the section header nor end the section early.
The message now names both what archive writes and a marker left in its place,
since one message covers both. Severity is unchanged: still a warning, so a
project carrying placeholders keeps validating and only --strict fails.
Every new guard is mutation-checked: dropping `TODO` kills 3 tests, unmasking
detection kills 2, unmasking the line locator kills 3, unmasking the header
search kills 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(validate): read the marker boundary in any script, not just ASCII
Review found `\b` reading `TODOé` and `TBD١` as a marker followed by
punctuation, because `\b` only knows ASCII word characters. A Purpose is prose
and prose is not always Latin script, so the rule that a longer word beginning
with those letters is not a marker has to hold in any script.
The lookahead rejects letters, digits, combining marks and `_`, and nothing
else, so `TODO:`, `TBD -` and `TODO(owner):` are still the marker they look
like. Held in both directions: loosening it back to `\b` kills 1 test,
tightening it to reject punctuation kills 4.
Also reworded a task line that opened with `#1670`, which markdownlint reads as
a heading missing its space.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(validate): locate the matched purpose placeholder
* docs(validate): remove trailing task whitespace
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Clay Good <hi@claygood.com>
|
||
|
Ayman A.
|
7010e26890 |
fix(explore): require explicit confirmation before writing files (#1716)
* fix(explore): require explicit confirmation before writing files * test(explore): harden write confirmation guardrail * fix(explore): scope write confirmation precisely --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Clay Good
|
7da3f34fb6 |
fix(tasks): include verification in generated plans (#1660)
* fix(tasks): include verification in generated plans * test(tasks): enforce inline verification guidance * fix(tasks): harden verification guidance * test(tasks): verify every onboarding checkbox |
||
|
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
|
9643888a75 |
fix(schema): resolve main-spec reads against the store-aware root (#1703)
* fix(schema): resolve main-spec reads against the store-aware root
The spec-driven `specs` instruction named
`openspec/specs/<capability-path>/spec.md` — a cwd-relative path — for the
two operations that touch a capability's main spec: step 1 of the MODIFIED
workflow ("locate the existing requirement") and the edit that fixes a
leftover TBD Purpose.
When the change lives in a registered store, the main spec is under the
store root. Verified against one: `openspec instructions specs --store
mystore --json` returns `planningHome.root` pointing at the store while the
instruction sent the read to the working repo, where the capability does
not exist. Where a local capability happens to share the name it is worse
than a miss — the read succeeds against a different capability and step 2
copies the wrong requirement block into the delta, silently.
Both now use `<planningHome.root>/openspec/specs/...`, the root the same
JSON already returns, matching what sync-specs.ts and archive-change.ts
have said since they were written: use the store-aware root, not a
hardcoded repo path.
Guidance text only — no CLI, parser, or archive behavior changes. The two
remaining `openspec/specs/` mentions describe the shape of a capability
path rather than a file operation, and are left alone.
Closes #1702
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(schema): make the store-aware root unconditional, and prove it resolves
Two hardening findings.
The wording said the root "points at the store when a store is selected."
Verified across all four root configurations, that undersells it: a project
`store:` pointer (source `declared`) and a global default store (source
`global_default`) both resolve to the store with no `--store` flag passed.
An agent reading the old sentence could conclude the case did not apply to
it and fall back to a repo-relative path. It now says to always use the
field and not to reason about which case applies.
The test only pinned the placeholder text, which would still pass if
`planningHome.root` were renamed or the suffix were wrong. Added a guard
that substitutes the placeholder with a real resolved planning home and
asserts the composed path lands on an actual main spec. Mutation-tested:
inserting a path segment and renaming the field each fail it.
Verified end to end that the composed path exists under all three
store-selecting configurations, and under a plain local repo.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test: compose the main-spec path from segments, not string substitution
The guard substituted `planningHome.root` into a template spelled with
forward slashes. On Windows that yields a mixed-separator path, so the
assertion passed because Node accepts forward slashes there rather than
because the path was built correctly. Windows CI was green either way;
this makes the construction right instead of merely tolerated.
The suffix is now captured on its own and joined to the root with
path.join, so the assertion uses native separators everywhere. All three
mutations (cwd-relative path, extra segment, renamed field) still fail
the guard.
Addresses CodeRabbit review on #1703.
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
|
15e50d6889 |
fix(opencode): pass command arguments to workflows (#1664)
* fix(opencode): pass command arguments to workflows * test(opencode): recognize existing argument placeholders * test(opencode): harden argument generation * test(opencode): cover commands-only upgrades * test(opencode): verify repaired command content |
||
|
Clay Good
|
cf06d45f91 |
fix(profiles): include sync with archive workflows (#1663)
* fix(profiles): install sync with archive workflows * test(profiles): harden archive dependency coverage * fix(config): preserve custom profile ownership |
||
|
Clay Good
|
f3aa167d6e |
feat(tools): add Zed Agent support (#1659)
* feat(tools): add Zed Agent support * fix(tools): detect Zed projects |
||
|
Clay Good
|
a72a74de65 |
fix(update): only suggest IDE restarts when needed (#1656)
* fix(update): only suggest IDE restarts when needed * test(update): cover restart hint edge cases |
||
|
Clay Good
|
a2b965aa5e |
fix(workflow): keep no-spec schema changes valid (#1655)
* fix(workflow): scaffold valid no-spec changes * fix(workflow): normalize specs artifact paths |
||
|
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 |
||
|
tech_ren
|
0221ac3d46 | fix(archive): preserve blank lines around ## Requirements when syncing specs (#1637) | ||
|
Clay Good
|
207f3cc515 |
fix(config): label the update workflow in the picker; drop "expanded-profile" wording (#1632)
* fix(config): label the update workflow in the picker and drop "expanded-profile" wording The config workflow picker builds each row's label from WORKFLOW_PROMPT_META in src/commands/config.ts. The table had entries for 11 of the 12 workflows but not `update`, so `openspec config` rendered that row as the raw id `update` with a `Workflow: update` placeholder description. Since `update` is one of the six core workflows, every user who opens the picker saw it. Add the missing `update` entry so the row reads "Update change / Revise the planning artifacts of an existing change". Also reword the update-change workflow template, which called `/opsx:continue` and `/opsx:new` "expanded-profile" workflows. There is no "expanded" profile; the only profile values the product stores are `core` and `custom`. They are now described as "optional" workflows. Regenerated the committed skills.sh mirror and parity hashes accordingly. Harden with a regression test asserting every ALL_WORKFLOWS id has real picker metadata (no raw-id name, no "Workflow:" placeholder), so a future workflow addition can't silently reintroduce the fallback. Closes #1627 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: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
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
|
804427b6ff |
fix(telemetry): suppress first-run notice in --json mode (#1609)
* fix(telemetry): suppress first-run notice in --json mode The first-run telemetry disclosure notice was written to stdout from the global preAction hook. On a user's first-ever command with --json this polluted stdout and could break JSON parsers. Read the executing command's --json flag (actionCommand.opts().json) and, when set, skip the notice and leave noticeSeen unset so the disclosure is deferred to the first later non-JSON run rather than lost. Spinner suppression, new-change --json output, and structured JSON errors already landed on main (#960, #1190); this closes the one remaining stdout writer in --json mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden: detect --json from argv to cover all invocation forms The preAction guard read actionCommand.opts().json, which only sees a declared leaf option. That missed two supported --json forms that emit a single JSON document to stdout: - openspec store --json (permissive group reads --json from residual args; never declares the option, so opts().json is undefined) - openspec workset --json <sub> (--json on the parent group, consumed before the leaf; leaf opts().json is undefined) Both would still print the first-run telemetry notice ahead of their JSON. Detect --json from process.argv instead: it covers leaf, parent, and residual-arg forms uniformly. Suppressing is always safe (the disclosure defers to the next non-JSON run, never lost), so a broad argv check is the correct, conservative signal. Also add a direct assertion that noticeSeen stays unset after a silent run, and note the pre-existing raw-stdout commands (completion generate, config get/path, __complete) as out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: derive --json from parsed command state + regression test Replace the process.argv check with isJsonRun(command), an exported pure helper that reads Commander's parsed state: optsWithGlobals().json (leaf and parent-group forms) OR command.args (residual --json on permissive bare groups like store). This is tied to the actually-parsed command rather than raw args, and — unlike process.argv — is unit-testable in-process. Add test/core/cli-is-json-run.test.ts: a synthetic program reproducing all three registration patterns proves isJsonRun returns true for status --json, store --json, workset --json list, and workset list --json, and false otherwise. This locks in the store/workset coverage against future regressions (an e2e test can't: telemetry is disabled under CI, so the notice never fires there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): qualify first-command notice scenario as non-JSON The generic 'First command execution' scenario asserted the notice displays on every first command, contradicting the JSON scenario that says it does not. Qualify it as 'without --json' so the required behavior is unambiguous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay Good
|
17581c11ed |
fix(init): only show 'Restart your IDE' hint for IDE-embedded tools (#1610)
Reconstructed on current main (the original branch predated the Codex .agents rename, Command Code, Rovo Dev, Antigravity, Zoo Code, and the Kimi/Windsurf changes, so a direct rebase conflicted heavily in config.ts/init.ts/init.test.ts). Adds requiresIdeRestart to AIToolOption and gates the success-screen restart hint so it shows only when an IDE-resident tool actually received a surface. Wording follows that tool's own surface. Closes #1067. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Marzx13
|
942589741d |
fix(core): canonicalize rebuilt spec EOF (#1528)
* fix(core): canonicalize rebuilt spec EOF * chore(changeset): add patch changeset for spec EOF canonicalization 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
|
c751b3da52 |
fix(validate): count every level-4 header as a scenario in the loss guard (#1521)
* fix(validate): count every level-4 header as a scenario in the loss guard The scenario-loss guard (#1482) recognized only `#### Scenario:` headers, but the spec path (SCENARIO_HEADER / countScenarios) counts every `#### ` child of a requirement as a scenario. A MODIFIED block that dropped a differently-labeled level-4 child (e.g. `#### Edge case`) therefore passed validate and was silently deleted by archive. Align parseScenarioBlocks with the spec path so both agree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(validate): guard scenario-header parity; reuse SCENARIO_HEADER Harden the scenario-loss parity fix after a multi-agent review: - Export SCENARIO_HEADER from requirement-text.ts and reuse it in the delta path (scenarioHeaderAt/scenarioNameAt) so parity is guaranteed by construction, not two matching literals plus a comment. - Add boundary tests for the widened matcher: a level-5 (#####) header must not count, an unlabeled #### inside a fence must not count, an optional Scenario: label normalizes (relabel is not a loss), and unlabeled scenarios are counted by multiplicity. Plus an integration case: a dropped labeled scenario is caught even when an unlabeled sibling is kept (validate/archive parity, both directions). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(validate): harden scenario-name folding + incoming-fence parity Adversarial review of the scenario-loss guard surfaced one over-strict nit and one untested symmetry: - scenarioNameAt now also strips a CommonMark closing `#` run, so `#### Foo` and `#### Foo ####` fold to the same scenario name. Without this, relabeling a scenario's header on one side (ATX-open vs ATX-closed) read as a dropped scenario — a false-abort. Safe direction only: a genuine drop still lowers a folded name's count and is caught. - Add unit tests for the untested incoming-side fence mask (a fenced `####` in the MODIFIED block must not satisfy a real scenario), lowercase `scenario:` label normalization, and the ATX-closed header fold. Behavior for conventional `#### Scenario:` headers is unchanged; parser, validation, and archive suites stay green (269 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(validate): match CommonMark whitespace in ATX-close strip; changeset nit Second adversarial-review round follow-ups: - scenarioNameAt's ATX-closing-sequence strip now matches only a space/tab before the trailing `#` run (`[ \t]` not `\s`), exactly as CommonMark defines a closing sequence. A looser `\s` could strip a `#` run after an exotic space (e.g. NBSP) that CommonMark keeps rendered, folding two distinct scenario names into one and masking a real loss. Correct-direction hardening for a data-loss guard; no behavior change for real space/tab-authored headers. - Changeset: describe the header whitespace outside the code span to satisfy markdownlint MD038 (no trailing space inside `#### `). Resolves CodeRabbit. Parser/validation/archive suites green (243 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
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
|
bf5099e39f |
fix(apply): surface deferred scope instead of silently simplifying tasks (#1530)
* fix(apply): surface deferred scope instead of silently simplifying tasks The /opsx:apply guidance told agents to keep going through tasks but never told them what to do when a task turned out harder than the spec assumed. Agents absorbed the extra scope silently — narrowing, deferring, or declaring partial work done — and marked the task complete anyway (#1529). Add a pause trigger and two guardrails to the shared apply instructions (rendered identically by the skill and command surfaces): surface the added scope and ask rather than simplify to fit, and mark a task complete only when it is fully implemented as specified. Regenerate the static skill and parity-hash pins. Guidance text only — no behavioral code paths change. Fixes #1529 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(apply): anchor deferred-scope guidance to spec scope, not effort Adversarial review flagged that "more complex than the spec assumed" could be read as "takes more effort than I guessed," which would make an agent pause on nearly every task. Retie the pause trigger and guardrail to a change in scope — work beyond what the spec/tasks describe, or dropping / narrowing / deferring specified behavior — so normal implementation effort does not trip it. Regenerate the static skill and parity pins; update the regression test and changeset to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(apply): name the "accept exceptions" pattern in deferred-scope guidance Issue #1529's concrete example is an agent that found three exceptions to a "zero writes on the main thread" task, declared them "accepted," and moved on. Add "accept exceptions to" to the pause trigger's verb list so the guidance names that exact failure mode, not just drop/narrow/defer. Behavior is otherwise unchanged; regenerate the static skill and parity pins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(apply): assert the deferred-scope guidance requires pausing CodeRabbit noted the guardrail test checked that added scope is surfaced but not that the agent pauses, so it could pass if the workflow reported scope and kept going. Assert the exact "surface the added scope and pause" phrasing. 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
|
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> |
||
|
aliouswe
|
e50bd0983d |
fix(validate): warn on ambiguous task numbering (#1523)
* fix(validate): warn on ambiguous task numbering * fix(validate): honor task numbering review boundaries |
||
|
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> |
||
|
Clay Good
|
96a6548664 |
refactor(templates): share one apply instruction body across skill and command (#1515)
* refactor(templates): share one apply instruction body across skill and command The apply skill and command templates each carried a full ~150-line copy of the same instruction body, differing in exactly one line (the `contextFiles` note). Two near-identical copies invite silent drift. Author the body once in `getApplyInstructions(contextFilesNote)` and render it per surface, passing each surface's own note. The single intentional wording difference stays explicit as a named constant, and further per-surface parameters can be added here as the surfaces evolve — the skill and command remain distinct templates. Pure refactor: the generated skill and command output is byte-identical to before (SKILL.md and all parity hashes unchanged). Added a contract test that fails both if the shared body drifts between surfaces and if the intentional contextFiles difference is flattened away. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(templates): unify apply instruction body into one shared core Builds on the shared-core extraction: the apply skill and command still each carried a slightly different `contextFiles` note (skill spelled out example artifact sets, command said only "varies by schema"). That difference was long-standing accidental drift between the two copies, not an intentional surface distinction — the surfaces are meant to differ only in how they are invoked, which the generation transformers already handle downstream by rewriting `/opsx:<id>` tokens per surface. Resolve the drift by unifying on the more informative note, so both surfaces render one shared `getApplyInstructions()` body with no per-surface text. Skill output is unchanged; the command's contextFiles note gains the example artifact sets. Updated the contract test to assert both surfaces render the shared core (no silent template-level drift), and regenerated the command function hash accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.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> |