mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-20 06:19:21 +08:00
@fission-ai/openspec@1.13.0
389
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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 GoodandClaude Opus 5
|
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 GoodandClaude Opus 5
|
8ba4ac1b16 |
fix(apply): warn when a change is ready to implement with no specs (#1783)
* fix(apply): warn when a change is ready to implement with no specs Apply gates on the schema's `apply.requires` (tasks) alone, so a change whose tasks file was written ahead of its specs read as ready even though it had no delta specs at all — the state `openspec validate` rejects. Apply was the one surface that green-lit a change every other surface flags, which is how agents end up implementing before the specs exist. Report it as a warning, in the text output and in `--json`, naming both ways out: write the specs, or declare `skip_specs: true`. Blocking would be a policy change; naming the gap is not. Changes that have specs, declare `skip_specs`, or are still blocked on their own required artifacts are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(apply): name the metadata file from its shared constant Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(apply): cover custom schemas in the no-specs warning A schema with no spec-producing artifact must stay quiet, and one whose spec artifact is not called `specs` must still warn - the rule keys off the output path, not the artifact id. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(apply): stop asserting an absolute temp path on Windows os.tmpdir() hands back the short form (C:\Users\RUNNER~1) while the CLI resolves the long one, so the assertion pinned a path that never matched on windows-pwsh. Assert the change-relative tail instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(apply): name the whole chain a blocked change still needs Apply blocks on the schema's `apply.requires` alone, so its message stopped at the first hop: a change holding only a proposal was told "Missing artifacts: tasks" while the specs `tasks` depends on were missing too. Taken literally that is an instruction to write the tracking file straight from the proposal and skip everything between — the failure reported in #834 and #869. Walk `requires` and report the whole set, in build order, as `missingPrerequisites` (text and `--json`). What apply blocks on is unchanged, and the wording leaves conditional artifacts to the schema rather than demanding them. The remedies these messages give are now CLI commands rather than the `openspec-continue-change` skill: `continue` is not in CORE_WORKFLOWS, so on the default profile the old advice named a skill that is never installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(apply): name the schema's own spec artifact in the warning alfred-openspec on #1783: collectApplyWarnings() discovers spec-producing artifacts by output path, so it correctly fires for a schema whose artifact id is `contracts`, but the remediation text then hardcoded `openspec instructions specs`. That names an artifact such a schema does not declare, so the advertised custom-schema support dead-ended at the exact step meant to resolve the warning. The command now derives its target from specArtifacts: the artifact's own id when the schema declares one spec-producing artifact, and `<artifact-id>` as a placeholder when it declares several, since there is no single right answer there and a guess would read as an instruction. The renamed-artifact test now asserts the command names `contracts` and rejects the hardcoded `specs` spelling, and a new test pins the two-artifact placeholder. Verified both fail against the hardcoded string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(changeset): bump apply warnings to minor This adds `missingPrerequisites` and `warnings` to the documented `instructions apply --json` contract in docs/agent-contract.md. New fields are backward compatible, but they are new capability an agent can consume, which is a minor under semver rather than a patch. Taking the conservative direction deliberately: shipping new API surface as a patch is the violation, since a consumer pinned to a patch range would receive it without opting in. A minor costs nothing if the fields turn out to be uninteresting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 5
|
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 GoodandClaude Opus 5
|
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> |
||
|
Marzx13andClay Good
|
db03c6c4b0 |
feat(validate): add findings-only bulk reports (#1713)
* feat(validate): propose findings report * docs(validate): clarify findings report contract * feat(validate): implement and harden bulk findings reports * test(validate): canonicalize store paths natively --------- Co-authored-by: Clay Good <hi@claygood.com> |
||
|
Ryan de MeloandClay Good
|
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> |
||
|
Br1anandClay Good
|
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 LeeandClay Good
|
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)andClay Good
|
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> |
||
|
Александр МелентьевandClay Good
|
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> |
||
|
|
2fa679f180 |
fix(schema): make schema init --default actually set the default (#1709)
* fix(schema): make schema init --default actually set the default --default wrote defaultSchema to openspec/config.yaml, but the config loader only reads schema, so new changes kept using spec-driven while the command reported success. Write the key that is read, and drop the dead one a previous run may have left behind. Fixes #1708 * fix(schema): preserve supported configs when setting default * docs(schema): specify default config file handling * docs(schema): protect default config migration * fix(schema): make default initialization atomic * fix(schema): hide init staging and backup dirs from discovery `schema init` stages into `.init-staging-<rand>` and moves an existing schema aside to `<name>.init-backup-<pid>-<ts>`, both inside the schemas dir. `schema fork` already did this and the resolver filters its temp names out of discovery; the init names were never added, so `listSchemas` and `listSchemasWithInfo` surfaced them as real schemas -- in shell completions, "available schemas" error lists, and change-metadata validation. The backup is the durable case: cleanup failure is deliberately tolerated with a warning, so a blocked cleanup (or a crash mid-transaction) leaves a permanent phantom schema behind. Generalize the fork-only filter to cover both commands' staging and backup names. Real schema names are kebab-case, so excluding these dot-bearing names can never hide a legitimate schema. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
Marzx13andClay Good
|
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> |
||
|
|
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>
|
||
|
|
a7353aea9a |
feat(status): add --all for batch status of every active change (#1301)
* feat(status): add --all for batch status of every active change
`openspec status --all --json` reports every active change in one
process instead of one CLI spawn (~500ms module-load) per change,
mirroring the existing `validate --all`. Emits a single
`{ changes: [ChangeStatus, ...], root }` envelope sorted by change
name; a change that fails to load contributes a per-change error entry
instead of failing the sweep. `--all` and `--change` are mutually
exclusive, honoring the --json null-shape on failure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): harden --all per adversarial review findings
- Validate --schema before the no-changes early return so a bogus
schema fails consistently whether or not any change exists.
- Text mode now exits 1 when any change fails to load (mirrors
validate --all); JSON mode still exits 0 with per-change diagnostics.
- Add tests for the --all --schema interaction (unknown schema
null-shape, override propagation, broken-metadata precedence) and
text-mode failure rendering.
- Changeset heading to "### New Features" per repo convention; add
status --all to the agent quick-reference table in docs/cli.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): thread batch null-shape through root resolution, align sort with validate
Code-review findings on --all:
- Pass failurePayload: { changes: [] } to resolveRootForCommand so a
root-selection failure under --all --json still emits the documented
batch null-shape (siblings like list/doctor/context already do this).
- Sort with localeCompare to match validate --all's ordering for
mixed-case change names.
- Extract a shared loadStatus helper so the batch and single-change
payloads cannot drift apart.
- Changeset no longer claims exact validate --all parity (JSON exit
semantics deliberately differ).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(status): give the --all failure null-shape a single owner
Simplify pass on the --all diff: hoist the { changes: [] } batch
null-shape into an exported BATCH_STATUS_FAILURE_PAYLOAD constant so
the root-resolution and CLI-wrapper failure paths cannot drift, replace
the conditional spread with the plain ternary the sibling call site
already uses, drop a redundant array copy before sort, and narrow the
text-mode failure counter to the boolean it actually is.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(status): point the missing-target error at --all, correct the docs
`openspec status` with neither --change nor --all listed the available
changes and named only --change, so the batch path was discoverable
only from --help. The error now offers both.
Also corrects two stale claims in the status section of docs/cli.md
that the new row sits next to: the command never prompts for a change
(it errors), and bare `openspec status` is not an interactive check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(status): assert --all carries store context like the single-change path
The batch sweep resolves the root once and threads the store id into
every entry. Nothing pinned that: a regression would have shown up only
as a wrong path inside an agent's JSON. Assert the sweep's envelope root
and per-change payload match `status --change` in a registered store.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(status): fail incomplete batch reports
* docs(status): clarify empty and batch output
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Clay Good <hi@claygood.com>
|
||
|
Ayman A.andClay Good
|
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 GoodandClaude Opus 5
|
7276c6c268 |
fix(packaging): print the completions tip from the CLI, not a postinstall script (#1704)
* fix(packaging): print the completions tip from the CLI, not a postinstall script The package's only install script existed to print one line suggesting `openspec completion install`. Shipping it made every `npm install -g` emit an npm allow-scripts warning, and `npm approve-scripts` then failed with ENOMATCH because it looks in the local project, not a global install — so the warning looked like a packaging fault with no way to clear it. The tip now prints once on the CLI's first run, recorded via a `completionTipSeen` flag in the existing global config alongside the telemetry notice's `noticeSeen`. It writes to stderr so it can never contaminate piped stdout, and is suppressed under CI, OPENSPEC_NO_COMPLETIONS=1, `--json` runs, and `openspec completion` itself. The published package now ships no lifecycle scripts at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): stop the first-run tip from corrupting global config Adversarial review of the previous commit found it wrote a defaults-merged config: `saveGlobalConfig({ ...getGlobalConfig(), completionTipSeen: true })` stamped `profile: "core"` into every user's config.json on first run. `migrateIfNeeded` treats a raw `profile` as "already migrated", so the one-time profile migration would never run again — and `openspec update` then deleted the user's installed workflow skills. Reproduced: 2 skill directories removed where main reports "Migrated: custom profile with 8 workflows". The same write also overwrote an unparsable config with defaults and made `openspec config list` report defaults as explicit. The tip now reads and writes the raw config file and touches only its own key, leaving an unreadable config strictly alone. Other hardening from the same review: - Suppress the tip for the hidden `__complete` resolver. Generated completion scripts call it on every Tab press with stderr discarded, so the one-shot tip was consumed where nobody could see it. - Defer, never consume, when stderr is not a terminal. Agents and pipes drive this CLI far more often than humans do and would otherwise spend the tip into a log nobody opens. - Skip the tip when completions are already installed. Previously the CLI advertised `completion install` to users who had run it — including on the very next command after installing. Adds `isInstalled()` to the bash/fish/powershell installers, mirroring the zsh one. - Use the repo's `isCiEnvironment()` instead of a `CI === 'true'` string check, so `CI=yes`/`True`/`on` are as quiet as telemetry is. - Move the call to `postAction` so the tip trails the command's output instead of pushing errors and `init`'s setup summary down the screen. - Record before printing, so an unwritable config dir means silence rather than nagging on every run. Tests: assert the message literal (mutation testing showed the message text was the one unguarded behavior), the raw-write shape, corrupt-config safety, the already-installed path, the defer policy, and an e2e case pinning the non-TTY contract. Docs: SECURITY.md no longer claims zero lifecycle scripts — `prepare` is still declared and runs for git/directory installs; the registry-install claim is the accurate one. `OPENSPEC_NO_COMPLETIONS` is now documented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(completions): make the unwritable-config case portable to Windows fs.chmodSync(dir, 0o555) does not stop a write on Windows, so this test's unwritable condition never existed there: markTipSeen succeeded, the tip printed, and windows-pwsh was the only failing job. Occupy the config directory's path with a file instead. mkdirSync with recursive: true tolerates an existing directory but throws on an existing file on every platform, so the persist fails where a real permission error would - before anything is printed. Also asserts the path is still a file, so a partial write through the failure would be caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(completions): retire the first-run tip instead of advising a dead end Second adversarial pass over the tip, covering the hardening commit itself. - An undetected or unsupported shell now retires the tip quietly. It used to print, but `openspec completion install` exits 1 for exactly those users ("Shell 'tcsh' is not supported yet" / "Could not auto-detect shell"), so the one message they would ever get about completions sent them to a command that fails. - `markTipSeen` re-reads the config immediately before writing and swaps the file in by rename. Deciding whether to show the tip costs a `ps` spawn plus a stat, and a sibling process writing config in that window got clobbered — on a first run that is exactly when telemetry mints `anonymousId`. Concurrent-process loss drops from 15/40 to ~2/40, and what now usually loses is the tip's own flag (it simply shows once more) rather than telemetry identity. The residual is the non-atomic read-modify-write shape shared with telemetry's own writer. - `isInstalled()` uses stat().isFile(), so a directory at the install path no longer counts as an installed completion script. - Documented what `isInstalled()` actually promises: the script file, not the profile sourcing line that bash and PowerShell also need. Callers deciding whether to *advertise* completions want the loose reading — a user whose profile config failed has already met the installer. - Corrected a comment claiming the probe costs "one stat": detectShell() forks `ps` to read the parent process on every non-Windows run. Tests: mutation testing found four surviving mutants — dropping isCompletionRun from the defer policy, reverting isCiEnvironment to a CI==='true' string check, failing closed on an undetected shell, and neutering the non-object config guard (which lets a JSON array config be rewritten as {"0":...}). All four now fail a test. Adds direct coverage for the three new isInstalled() implementations, which had none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(validate): stop `change validate` exiting past commander's postAction `change validate` on a failing change called process.exit(exitCode). That tears down before commander's postAction hook, which is the same trap the `update` command documents 165 lines earlier: "exiting here would skip commander's postAction hook, killing the telemetry flush mid-request". A change that fails validation is a routine outcome, not an error, so this silently dropped the telemetry flush and — since the completions tip moved to postAction — the first-run tip for anyone whose first command was a failing validate. Verified under a pty: before, the tip never printed and completionTipSeen was never recorded; after, both happen and the exit code is still 1 (validate() already sets process.exitCode, which Node honours at natural exit — top-level `validate --all` has always relied on exactly that). The existing e2e in validate-scenario-loss.test.ts pins the exit code. Also wraps the postAction tip in try/finally so the telemetry flush runs even if the hint throws: program.parse() is synchronous, so a rejection there has no catch above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Clay Good
|
18688c8b27 |
fix(archive): never dead-end a capability retirement (#1699)
* fix(archive): never dead-end a capability retirement A change whose delta removes the last requirement a capability has rebuilds the main spec empty, which can never validate. Archive already knows retiring is the fix and names the `retire_capabilities: true` marker that authorises deleting the spec - but only when the marker is the single thing missing. If the spec also holds a line the merge cannot account for (a `## Notes` section, a comment under a requirement - both ordinary), that hint was suppressed, and the hint that names such lines only spoke to authors who had already set the marker. Neither fired, so the archive aborted on "Spec must have at least one requirement" with no guidance at all: the exact dead end the marker exists to close. Archive now names the blocking content in that case. It deliberately does not name the marker there - adding it would not have let this run through, and the marker is only ever named when it really is the one thing missing. Once the content is resolved, the rerun names the marker. Closes #1696 * fix(archive): harden the blocked-retirement abort Three follow-ups to the same message. The blocking lines are authored spec content printed verbatim to a terminal, so they now get the treatment `describeChangeName` already gives a change directory name: control characters replaced, since a raw CR could forge a line of its own and an ESC could redraw the screen. Each line is bounded too - one very long line would push the way out of the abort off the reader's screen - and the cut counts code points so it can never leave half a surrogate pair. Both the declared and undeclared branches share the helper, so the marker-declared abort that shipped with #1484 is hardened with it. The wording no longer claims retiring is "the way through". It is not, in the one case this fires on that has a live requirement hiding in a second `## Requirements` section: merging the sections fixes that spec without deleting anything. `openspec/specs/cli-archive/spec.md` records the behavior change - the blocking lines are named whether or not the marker was declared, and the marker is still named only when adding it would let the archive through. * refactor(archive): drop a helper the revised wording made single-use The marker sentence is said in one place again, so it goes back inline rather than through a function that now has one caller. Also corrects the comment above `emptiedByThisRun`: retiring is not the only fix in every case it covers, which is exactly why the message stopped saying so. * docs(openspec): record the change as a delta, not a direct spec edit Both conventions exist in this repo's history, but the two most recent behavior fixes (#1609, #1616) carry an `openspec/changes/` delta rather than editing the main spec in place, which is also the workflow this project asks of everyone else. The delta reproduces the whole Capability Retirement requirement, so archiving it drops no scenario. Verified by archiving into a scratch copy of `openspec/`: the merged main spec differs from today's by exactly the three added bullets. * fix(archive): report an unhonorable marker alongside the blocking content An author who set `retire_capabilities: yes-please` believes they have authorised the deletion. Clearing the blocking content first, only to then learn the marker was never read, is two aborts for one mistake. The abort still never invites the marker to be added while content blocks the retirement - it only reports the one already there. The spec delta records that distinction, which the old bullet ("say nothing about the marker") did not draw. * style(archive): use one sentence for an unhonorable marker in both aborts * fix(metadata): strip control characters from an unhonorable marker reason Every reason a boolean change-metadata marker gives quotes something the author wrote - a schema name, a parser message carrying one, a filesystem error carrying a path - and two commands print it straight to a terminal. A schema name carrying a raw ESC, with the marker set, put that ESC on screen through `openspec archive`; `openspec validate` prints the same reason. Fixed at the source in `readBooleanMarker` rather than at either call site, so no consumer has to remember. The reason still quotes the name recognisably; only control characters are replaced. Reported by CodeRabbit on #1699. Pre-existing on main, and this PR would have added a second place it reaches the terminal. * test(archive): fix a comment left behind by the reworded abort |
||
|
Clay Good
|
c747ed1f34 |
feat(init): add language option (#1685)
* feat(init): add language option * fix(init): harden language configuration * fix(init): fail when language config cannot be written |
||
|
Clay Good
|
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 |
||
|
|
91813641cf |
chore(deps): migrate to @inquirer/prompts v8 + @inquirer/core v11 (#1667)
* chore(deps): migrate to @inquirer/prompts v8 + @inquirer/core v11 Bumps both packages together. The two Dependabot attempts each moved one half (#1450 prompts->8, #1422 core->11) and failed: prompts@8 pulls checkbox@5 -> core@^11, while package.json depends on core@^10 directly for two custom prompts, so a one-sided bump leaves two copies of @inquirer/core in the tree — custom prompts on v10 internals alongside bundled prompts on v11. Resolves the `instructions` removal in checkbox v5 by dropping the option: the built-in keys help tip now renders a superset of the hint that was being passed, so no theme override is needed. Closes #1458 * fix(nix): regenerate pnpmDeps hash for the inquirer v8 lockfile The pnpmDeps fixed-output hash is pinned to the contents of pnpm-lock.yaml, so the inquirer v8/core v11 migration invalidated it and Nix Flake Validation failed with 'pnpm failed to install dependencies'. Regenerated against this branch's lockfile and verified: nix build .#default completes (exit 0) through openspec-1.9.0.drv, not just past the fetch. 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> |
||
|
Ryan de Melo
|
db981f279d |
fix(telemetry): print first-run notice to stderr, not stdout (#1666)
console.log put the notice on stdout, so any non-JSON command (e.g. spec show, change show) had it prepended to raw/passthrough output on a fresh machine with no prior telemetry config — breaking pipes and consumers expecting exact file content. --json mode already avoided this by deferring the notice; stderr fixes it for every mode at the source instead of special-casing each one. |
||
|
tech_ren
|
0221ac3d46 | fix(archive): preserve blank lines around ## Requirements when syncing specs (#1637) | ||
|
|
8127c7b7cc |
fix(schema): preserve YAML formatting when forking a schema (#1607)
* fix(schema): preserve YAML formatting when forking a schema Rename a forked schema via yaml's Document API (parseDocument + doc.set) instead of round-tripping through parseSchema/stringifyYaml, so block scalars, comments, and key order in the source schema.yaml survive the fork. Keep the structural parseSchema validation before the document mutation so an invalid source is still rejected (addresses PR #1130 review). Adds fork-level regression coverage for both formatting preservation and invalid-source rejection. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): clean up partial fork when validation fails If the source schema is structurally invalid, parseSchema throws after copyDirRecursive has already created the destination directory, leaving a broken half-schema on disk that made the next fork report "already exists". Wrap the read/validate/rename in a try/catch that removes the just-created destination on any failure and rethrows so the original error still drives the JSON/exit-code reporting. The cleanup can only ever delete a directory this run created: the no-force existing-dest path returns before the copy, and the --force path removes the prior directory first. This also closes a mid-write truncation window for free. Adds regression coverage: cleanup + retryability on invalid source, the pre-existing-destination-is-never-touched invariant, and a lock-in that YAML-ambiguous names (true/false/null/off) round-trip as strings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): validate fork source up front; never mask fork errors Second hardening pass on the fork command, from an adversarial review of the previously-added cleanup. 1. Atomicity: validate the source's schema.yaml up front, immediately after assertSchemaTreeCanBeCopied and BEFORE the --force removal of an existing destination. Previously the source was validated only after the copy, so `fork --force <invalid-source> <existing-valid-dest>` destroyed the existing destination and then failed, leaving nothing. This matches `schema init`, which already validates before it overwrites. Behavior is unchanged for valid sources, and the redundant post-copy validation is dropped. 2. Never mask the real error: the failure-cleanup rmSync is now wrapped in its own try/catch. fs.rmSync's `force` only suppresses ENOENT, not EPERM/EBUSY/ENOTEMPTY (e.g. a locked file on Windows or a concurrent process), so a failed cleanup could previously replace the real "Invalid schema" diagnostic with a confusing filesystem error. The original error is now always rethrown. Adds regression coverage: --force with an invalid source leaves a valid destination intact; the pre-existing-destination test now uses a valid source so it exercises the no-force "already exists" guard directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): reject self-fork and stage fork before replacing destination Two data-loss defects in `schema fork --force` (per alfred-openspec review): 1. Self-fork: forking a schema onto itself removed the destination (which IS the source) before the copy, so the copy then read a directory it had just deleted — destroying the only copy. Now rejected up front by comparing the real (symlink-resolved) source and destination paths before any removal. 2. Non-atomic replacement: an existing destination was removed before the new fork was fully copied and name-updated, so a mid-copy failure left the user with nothing. The fork is now staged in a temporary sibling directory and only swapped into place once complete; any failure while staging leaves both the source and the existing destination untouched. Adds regressions: self-fork is rejected with the source intact; a forced fork whose copy fails leaves the existing destination byte-identical with no staging leftovers. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): back up destination before installing fork so a failed final move restores The stage-then-swap still removed the destination and then renamed staging into place; if that final rename failed (e.g. a Windows lock) the destination was gone with no restore. Now, when a destination exists, `fork --force` moves it to a sibling backup, installs the staged fork, and only then discards the backup. If the install rename throws, the backup is moved back so the original destination is never lost. Non-existing destinations keep the simple staging rename. Adds a regression: forcing the final staging->destination move to fail leaves the pre-existing destination byte-identical with no staging/backup leftovers. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): surface unrecoverable fork restore + hide fork temp dirs from discovery Two more edge cases from alfred's review: 1. A failed backup->destination restore was silently swallowed, so if the final install AND the restore both failed the user lost the destination with no clue the backup existed. Now that case throws an error naming the backup directory and how to move it back, with the original install error attached as cause. 2. The transient `.fork-staging-*` / `<name>.fork-backup-*` directories live inside the schemas dir, so a concurrent scan could surface them as real schemas. isSchemaDir (the single discovery chokepoint) now excludes them; real schema names are kebab-case (no dots) so this can never hide a schema. Adds regressions: an unrecoverable restore surfaces the backup path (and the rescued content is really there); fork temp dirs are excluded from listSchemas and listSchemasWithInfo. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): fingerprint fork destination to abort on concurrent edits A concurrent process could edit an existing fork destination between the moment --force authorized the overwrite and the moment the destructive swap ran, and those edits were silently destroyed (reproduced by alfred: mutate destination schema.yaml during copy; --force completed and deleted the newer content). Now, when overwriting an existing destination, the fork: - fingerprints the authorized destination (SHA-256 over every file's relative path and bytes) BEFORE staging; - re-fingerprints and compares immediately before moving the destination aside; on mismatch it ABORTS without touching the destination, preserving the concurrent changes and telling the user to re-run; - re-fingerprints the backup before discarding it on the success path; if it changed during the install window it is kept, not deleted, and its location is surfaced. All prior guarantees remain: self-fork rejection, stage-then-swap, backup/restore on failed install with the backup path surfaced, and the temp-dir discovery filter. Adds regressions: a destination edited concurrently during staging aborts the fork and preserves the edit; a backup modified during the install window is kept and its location surfaced. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(schema): avoid stat-then-read in fork fingerprint (CodeQL js/file-system-race) fingerprintDir called fs.lstatSync then fs.readFileSync on the same path, which CodeQL flags as a file-system race (the file may change between the check and the read). Use the Dirent type already returned by readdirSync ({ withFileTypes: true }) instead of a separate lstat, and read files directly, deriving the size from the bytes read. Behavior is unchanged (13/13 fork-fidelity tests, incl. the concurrent-edit race regressions, still pass); one fewer syscall per entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden(schema): validate the completed staged fork before any destructive step The up-front parseSchema only checks the SOURCE, but copyDirRecursive reads source files that can change mid-copy, so the staged result can be invalid even though the source was valid at the pre-check (reproduced by alfred: mutate source schema.yaml to invalid inside copyFileSync; --force installed the invalid fork and deleted the valid destination). Now, after copying and the Document-API name edit, the fork validates the COMPLETED staged schema.yaml (the exact bytes about to be installed) with parseSchema BEFORE any destination displacement. On failure it aborts, cleans up staging, and rethrows a clear error ("the staged fork of '<source>' is not a valid schema ...; aborted, '<dest>' was not modified") chaining the parse error. The up-front source parseSchema stays as a fail-fast; this is the authoritative gate. Order before the swap: validate staged -> fingerprint-revalidate dest -> rename dest->backup -> rename staging->dest -> revalidate+rm backup. Adds a regression: a source that becomes structurally invalid during staging aborts the fork and leaves the valid destination byte-identical, no leftovers. Co-Authored-By: JinzeLin <linjinze999@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: JinzeLin <linjinze999@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
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> |
||
|
|
8364428661 |
fix(schemas): honor canonical root selection (#1616)
* docs(openspec): propose schemas root selection fix * fix(schemas): honor canonical root selection * test(schemas): assert complete JSON schema shape * docs(stores): drop view from the cwd-only, no --store list view already accepts --store <id> (registered in src/cli/index.ts), so listing it among the commands that act on the current directory only was incorrect. Remove it; templates and the deprecated noun forms remain. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: regenerate skills and parity hashes after rebase onto main Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Clay Good <hi@claygood.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
804427b6ff |
fix(telemetry): suppress first-run notice in --json mode (#1609)
* fix(telemetry): suppress first-run notice in --json mode The first-run telemetry disclosure notice was written to stdout from the global preAction hook. On a user's first-ever command with --json this polluted stdout and could break JSON parsers. Read the executing command's --json flag (actionCommand.opts().json) and, when set, skip the notice and leave noticeSeen unset so the disclosure is deferred to the first later non-JSON run rather than lost. Spinner suppression, new-change --json output, and structured JSON errors already landed on main (#960, #1190); this closes the one remaining stdout writer in --json mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden: detect --json from argv to cover all invocation forms The preAction guard read actionCommand.opts().json, which only sees a declared leaf option. That missed two supported --json forms that emit a single JSON document to stdout: - openspec store --json (permissive group reads --json from residual args; never declares the option, so opts().json is undefined) - openspec workset --json <sub> (--json on the parent group, consumed before the leaf; leaf opts().json is undefined) Both would still print the first-run telemetry notice ahead of their JSON. Detect --json from process.argv instead: it covers leaf, parent, and residual-arg forms uniformly. Suppressing is always safe (the disclosure defers to the next non-JSON run, never lost), so a broad argv check is the correct, conservative signal. Also add a direct assertion that noticeSeen stays unset after a silent run, and note the pre-existing raw-stdout commands (completion generate, config get/path, __complete) as out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: derive --json from parsed command state + regression test Replace the process.argv check with isJsonRun(command), an exported pure helper that reads Commander's parsed state: optsWithGlobals().json (leaf and parent-group forms) OR command.args (residual --json on permissive bare groups like store). This is tied to the actually-parsed command rather than raw args, and — unlike process.argv — is unit-testable in-process. Add test/core/cli-is-json-run.test.ts: a synthetic program reproducing all three registration patterns proves isJsonRun returns true for status --json, store --json, workset --json list, and workset list --json, and false otherwise. This locks in the store/workset coverage against future regressions (an e2e test can't: telemetry is disabled under CI, so the notice never fires there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): qualify first-command notice scenario as non-JSON The generic 'First command execution' scenario asserted the notice displays on every first command, contradicting the JSON scenario that says it does not. Qualify it as 'without --json' so the required behavior is unambiguous. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Clay GoodandClaude Opus 4.8
|
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> |
||
|
Clay Good
|
137404b423 |
fix(cli): reject missing roots for list and validate (#1612)
* fix(cli): reject missing roots for list and validate * test(cli): cover legacy list root fallback |
||
|
|
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 GoodandClaude Opus 4.8
|
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 GoodandClaude Opus 4.8
|
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 GoodandClaude Opus 4.8
|
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 GoodandClaude Opus 4.8
|
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 GoodandClaude Opus 4.8
|
83be9d113e |
feat(validate): add --archived to lint task completion of archived changes (#1604)
* feat(validate): add --archived to lint task completion of archived changes `openspec validate --archived` scans every change under changes/archive/ and fails (exit 1) if any has unchecked tasks in tasks.md. This catches changes archived with unfinished work — which the normal validate flow never sees, since it only looks at active changes — and is meant for a pre-commit or CI hook. It is a standalone, opt-in scope: it returns before any existing bulk path, so no current `validate` invocation changes behavior, and it does not re-validate already-applied spec deltas. Reuses getTaskProgressForChange (the same counter status/list/archive use) so task counting never forks, and reads root.archiveDir so it is store-aware. Closes #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(validate): fail loudly on archive read errors and unreadable task files Address adversarial + CodeRabbit review of `validate --archived`: - listArchivedChangeIds now returns [] only for ENOENT (missing archive dir) and rethrows permission/I/O/ENOTDIR errors, so a real archive-read failure exits 1 instead of silently reading as "no archived changes". - Add getTaskProgressDetailForChange, which reports task files that exist but cannot be read; --archived turns those into an ERROR (naming the file) rather than silently counting them as zero tasks. The shared getTaskProgressForChange now wraps it and drops the detail, so status/list/archive totals are byte-identical. - Start the spinner after listing so a thrown listing error never leaves a spinner running. Adds regression tests (archive path is a file; archived tasks.md is unreadable) and unit tests for the new detail variant. Docs: align the --archived table verb and add a troubleshooting one-liner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(validate): address CodeRabbit nits on archived-task tests - Build the unreadable-fixture path from separate path.join components instead of a hard-coded Unix-separator string. - Assert the reported unreadable path (canonicalized with realpathSync.native), not just the count, so a wrong path can't pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(validate): address round-3 review of --archived From three fresh adversarial reviews (scale/perf, flag/output-shape, filesystem/security): - perf: memoize schema→glob resolution across archived changes via a run-scoped SchemaGlobCache, so the same schema.yaml isn't re-parsed once per change (the archive is append-only and can hold thousands). Threaded as an optional arg; existing callers are unchanged. Loop stays sequential by design (per-change work is synchronous) — now documented. - output shape: issue `path` now follows validate's convention — 'tasks.md' for incomplete tasks, and the POSIX root-relative file path for an unreadable file (one issue per file) instead of the bare 'tasks'. - plain output: print `change/<id>` (matching the JSON `type` and bulk validation) instead of `archived/<id>`. - docs: correct the "Never throws" docstrings (glob resolution can throw on a malformed/unsafe schema; the caller guards it) and note the load-bearing projectRoot override for the archive path depth. Store-mode resolution confirmed correct by review. Tests updated + a memo regression test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |