fix(archive): treat early-synced REMOVED deltas as no-ops, plus audit follow-ups - #1437
Merged
Conversation
… follow-ups Follow-ups from the post-v1.6.0 full-branch audit: - archive: a REMOVED delta whose requirement is already gone from the main spec (early-sync pattern) now warns and continues instead of aborting, matching the ADDED (#1376) and RENAMED (#1386) escapes; spec-update totals now count applied removals only - archive: the has-delta-specs gate matches section headers case-insensitively like the parser, so lowercase headers get the same delta validation errors validate reports - discovery: a symlinked specs/<cap>/spec.md is resolved instead of being invisible (hasAnyFileUnder and the artifact graph already counted it); dangling links are skipped - show: a plain `openspec show <change>` no longer warns about the never-passed `scenarios` flag (commander defaults --no-scenarios to true) - parsers: buildCodeFenceMask now has a single implementation in code-fence.ts; requirement-text.ts re-exports it - templates: apply/update/onboard no longer dead-end core-profile users on /opsx:continue and /opsx:new - they name the CLI fallback (openspec status/instructions) for profiles that do not install those workflows - qwen/bob: command bodies and skills reference commands by the hyphen names their files actually answer to (/opsx-<id>), matching opencode/pi/oh-my-pi - specs-apply: remove the dead applySpecs export (no callers, bypassed store-aware roots) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR refines archive delta validation and warning propagation, adds hyphenated command handling for additional tools, supports symlinked spec discovery, corrects explicit flag warnings, relocates code-fence masking, and updates installation-aware workflow guidance. ChangesCore behavior and workflow guidance
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/templates/workflows/apply-change.ts`:
- Line 53: Preserve selected store context in all blocked-state and update
fallback command examples by appending the conditional --store <id> flag only
when store selection exists, while leaving local-root examples unchanged. Apply
this consistently at src/core/templates/workflows/apply-change.ts lines 53 and
213, skills/openspec-apply-change/SKILL.md line 51,
src/core/templates/workflows/update-change.ts lines 87-88 and 174-175, and
skills/openspec-update-change/SKILL.md line 86.
In `@src/core/templates/workflows/onboard.ts`:
- Line 516: Add a CLI recovery fallback next to the /opsx:continue guidance in
src/core/templates/workflows/onboard.ts at lines 516-516, describing openspec
status --change "<name>" --json and openspec instructions <artifact-id> --change
"<name>" --json, then regenerate the committed skill content and parity hash.
Mirror the same fallback in skills/openspec-onboard/SKILL.md at lines 506-506;
both sites require the guidance update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cb877b6-4150-4e85-85d1-59a9b1af62cb
📒 Files selected for processing (19)
skills/openspec-apply-change/SKILL.mdskills/openspec-onboard/SKILL.mdskills/openspec-update-change/SKILL.mdsrc/commands/show.tssrc/core/archive.tssrc/core/command-generation/adapters/qwen.tssrc/core/parsers/requirement-text.tssrc/core/specs-apply.tssrc/core/templates/workflows/apply-change.tssrc/core/templates/workflows/onboard.tssrc/core/templates/workflows/update-change.tssrc/utils/command-references.tssrc/utils/spec-discovery.tstest/commands/show.test.tstest/core/archive.test.tstest/core/command-generation/adapters.test.tstest/core/templates/skill-templates-parity.test.tstest/utils/command-references.test.tstest/utils/spec-discovery.test.ts
…, skip no-op writes Adversarial-review round for #1437: - a delta that both RENAMEs and REMOVEs the same requirement is rejected explicitly by both validate and archive - the warn-and-continue REMOVED path would otherwise have masked the contradiction that previously failed incidentally at apply time - buildUpdatedSpec collects its warnings and archive --json carries them in a new optional `warnings` array, so agent flows see the same skipped-REMOVED signal humans get on stdout - archive skips rewriting a spec whose operations were all already synced, instead of churning normalization differences into the file (and no longer materializes an empty skeleton for a REMOVED-only new spec) - init's getting-started hint uses each tool's real invocation form (/opsx-propose for qwen/bob/opencode/pi/oh-my-pi) - onboard's pause guidance names the CLI fallback when /opsx:continue is not installed (CodeRabbit) - openspec-conventions spec updated to state the idempotent archive semantics; changeset added Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/archive-early-synced-removed.md:
- Line 5: Update the wording in the changeset description to use the grammatical
form “Qwen- and Bob-generated files” instead of “qwen and bob generated files,”
preserving the rest of the description unchanged.
In `@src/core/archive.ts`:
- Around line 523-537: The archive update flow around writeUpdatedSpec must
track whether at least one file was actually written. Set specsUpdated and emit
the “Specs updated successfully” output only when writeUpdatedSpec runs, while
preserving zero-count skips and accurate JSON results for already-synced
changes.
In `@test/core/archive.test.ts`:
- Line 483: Remove the duplicate block-scoped logCalls declarations, retaining
only one declaration in test/core/archive.test.ts at lines 483-483 and one in
test/core/init.test.ts at lines 1071-1071; preserve each test’s existing use of
the retained constant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a65afc04-76c1-42e6-b1e6-84b275edea03
📒 Files selected for processing (12)
.changeset/archive-early-synced-removed.mdopenspec/specs/openspec-conventions/spec.mdskills/openspec-onboard/SKILL.mdsrc/core/archive.tssrc/core/init.tssrc/core/specs-apply.tssrc/core/templates/workflows/onboard.tssrc/core/validation/validator.tstest/core/archive.test.tstest/core/init.test.tstest/core/templates/skill-templates-parity.test.tstest/core/validation.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/core/templates/workflows/onboard.ts
- test/core/templates/skill-templates-parity.test.ts
- src/core/specs-apply.ts
…or no-op archives Round-2 adversarial review for #1437: - a REMOVED header that differs only in case or interior whitespace from an existing requirement is a typo, not an early sync - it stays a hard abort naming the near-miss, instead of degrading to warn-and-continue - specsUpdated is true only when a spec file was actually written; a fully-already-synced change prints "Specs already in sync; no files changed." and reports specsUpdated: false in JSON (CodeRabbit) - agent-contract documents the archive warnings field and specsUpdated semantics; changeset wording fixed (CodeRabbit) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying openspec-docs with
|
| Latest commit: |
74a1026
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9ea2eb5f.openspec-docs.pages.dev |
| Branch Preview URL: | https://audit-followups.openspec-docs.pages.dev |
alfred-openspec
requested changes
Jul 23, 2026
alfred-openspec
left a comment
alfred-openspec
left a comment
Collaborator
There was a problem hiding this comment.
The new conflict guard still misses case/whitespace variants: RENAMED FROM: Old Name plus REMOVED: old name passes validate --strict --json, then archive exits 0, renames the requirement, reports the removal as already synced, and archives the change. Please use the same case/whitespace-folded identity for the RENAMED+REMOVED cross-section check in both validation and apply, with focused regressions for both paths.
…ce-insensitively Addresses alfred's review on #1437: `RENAMED FROM: Old Name` plus `REMOVED: old name` slipped past the exact-match cross-section guard, so validate passed, archive renamed the requirement, reported the removal as already synced, and archived the change. Both the validator and the apply-side guard now compare the two spellings with the shared foldRequirementName (lowercase, collapsed whitespace), and the error names the variant spelling when it differs. Focused regressions cover both paths; requirement matching everywhere else stays case-sensitive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
@alfred-openspec Fixed in 33ebba5. Both the validator and the apply-side guard now compare the RENAMED FROM header against REMOVED entries with a shared case- and whitespace-insensitive fold ( Your exact scenario, re-run against the built CLI:
Focused regressions for both paths: |
alfred-openspec
approved these changes
Jul 23, 2026
alfred-openspec
left a comment
alfred-openspec
left a comment
Collaborator
There was a problem hiding this comment.
The follow-up closes the case/whitespace conflict gap in both validation and apply. I reproduced the original failure against the built CLI, confirmed both paths now fail safely without mutation, and the full 2,253-test suite passes at head 33ebba5.
This was referenced Jul 27, 2026
Closed
clay-good
added a commit
that referenced
this pull request
Jul 28, 2026
Two asymmetries left over from the #1376/#1386/#1437 no-op work: - MODIFIED counted every delta as applied even when the block was byte-equal to the main spec, so a fully early-synced change rewrote the file (normalization churn), printed '~ N modified', and reported specsUpdated: true where its ADDED/REMOVED/RENAMED twins print 'Specs already in sync; no files changed.' Count only real replacements. - RENAMED's already-synced skip (source gone, target present) had no near-miss guard: a case/whitespace variant of the source still in the spec means a typo'd header, and REMOVED already hard-aborts on that signal. Apply the same guard, excluding the target itself so a case-only rename still no-ops. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
birhantprkc
pushed a commit
to birhantprkc/OpenSpec
that referenced
this pull request
Jul 29, 2026
…-audit follow-ups (Fission-AI#1475) * fix(archive): make the scenario-drift check fence-aware parseScenarioBlocks matched #### Scenario: headers on raw lines while the validator's countScenarios masks fenced code blocks (Fission-AI#1151). The drift check (Fission-AI#1391) inherited the raw scan, so a fenced scenario example in the current spec aborted an archive that validate had passed, and a fenced name in the MODIFIED block counted as keeping a scenario the block had actually dropped. Build the shared code-fence mask and skip masked lines in both the header scan and the block-end scan. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): tear down the redirected request when the budget expires The overall request budget was armed inside the first send() and its callback closed over that hop's request. After a redirect the timer destroyed the already-dead first request, so a redirect target that trickled bytes kept resetting its idle timeout and held the socket open until the body-size cap. Track the in-flight request and have the budget timer destroy whichever one is open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): add changesets for user-facing changes missing from the 1.7.0 notes 18 feat/fix commits merged since v1.6.0 without a changeset, so the pending Version Packages PR would have released them silently: five tool integrations (ZCode, Hermes, CodeArts, Kimi Code rename, Codex skills-only), skills.sh distribution, symlinked schema dirs, nested spec discovery, drift multiplicity, checkbox markers, Windows welcome input, npx avoidance, doctor store drift, local dates, missing-core-workflows warning, store-aware main specs, open-questions guidance, and spec content guidance. Plus changesets for this branch's two fixes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(adapters): escape TOML-active characters in Gemini command files The gemini adapter interpolated the description into a TOML basic string and the body into a multiline basic string with no escaping. Every current template value happens to be safe; the first description with a double quote or backslash would silently produce invalid TOML for all Gemini command files. Escape both contexts (Fission-AI#1447 fixed the same class for the YAML adapters but scoped itself to YAML). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): harden install detection and redirect handling Three follow-ups from the release audit: - A path segment literally named volta (a user or project directory) classified the install as volta-managed and swallowed the upgrade offer. The undotted spelling now requires volta's own tools/image layout, matching how pnpm and yarn already demand corroboration. - The Windows npm-ownership fallback checked that the npm prefix exists, which is true of any X\node_modules\pkg tree, hand-copied ones included. Corroborate with the openspec.cmd shim npm actually writes. - A https registry redirecting to plain http was followed; a MITM on that reply controls the newer-version answer. Refuse the downgrade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(cli): export zcodeAdapter from the barrel and sync a completion description zcode was registered but missing from the adapters barrel (its test imported the module directly), and the completion registry still carried the pre-Fission-AI#1062 description for the instructions command. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(parser): strip a UTF-8 BOM before parsing specs and deltas A BOM-prefixed delta spec (Windows editors, PowerShell Out-File) failed validate and archive with 'No delta sections found' because the first line never matched '## ADDED Requirements'. Strip the BOM in both normalizers, the same way tool detection already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(cli): reject over-long change names with a validation message A 300-character change name surfaced two raw ENAMETOOLONG errno dumps from stat and mkdir. Bound the name at 200 characters in validateChangeName so the failure is a normal validation error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(archive): finish the early-sync no-op rules for MODIFIED and RENAMED Two asymmetries left over from the Fission-AI#1376/Fission-AI#1386/Fission-AI#1437 no-op work: - MODIFIED counted every delta as applied even when the block was byte-equal to the main spec, so a fully early-synced change rewrote the file (normalization churn), printed '~ N modified', and reported specsUpdated: true where its ADDED/REMOVED/RENAMED twins print 'Specs already in sync; no files changed.' Count only real replacements. - RENAMED's already-synced skip (source gone, target present) had no near-miss guard: a case/whitespace variant of the source still in the spec means a typo'd header, and REMOVED already hard-aborts on that signal. Apply the same guard, excluding the target itself so a case-only rename still no-ops. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(validate): stop reporting an unreadable specs dir as 'no deltas' The delta-validation loop swallowed every error as 'if no specs dir, treat as no deltas', so an EACCES capability folder produced the misleading 'Change must have at least one delta' while archive let the same error propagate. Tolerate only ENOENT and ENOTDIR (a stray specs file); anything else stays loud, matching discoverSpecFiles' documented fail-loud contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): say when commands-only delivery leaves a tool with nothing Under delivery: commands, update removed the skills of adapterless skills-only tools (Hermes, Kimi Code, Vibe, CodeArts, ForgeCode) without a word — leaving zero OpenSpec artifacts while the tool's detection dir kept re-suggesting an init that would also generate nothing. Print the same per-tool configuration correction init already prints, pointing at 'openspec config set delivery both'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(completion): honor $ZSH and $ZSH_CUSTOM for Oh My Zsh installs The installer used a set $ZSH only as an is-installed signal and then wrote to ~/.oh-my-zsh regardless, so a custom OMZ location got a freshly created ~/.oh-my-zsh tree that no shell ever loads — and isInstalled/uninstall looked in the same wrong place. Route every path through the $ZSH/$ZSH_CUSTOM-aware helpers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(init): make the static welcome screen wait for the Enter it asks for The static branch printed 'Press Enter to select tools...' and returned immediately, so the Enter landed in the tool picker and submitted the pre-selected set sight-unseen. Fission-AI#1462 routed reduced-motion, OPENSPEC_NO_ANIMATION, --no-animation, NO_COLOR, and narrow-terminal users onto this path. Wait in a TTY; drop the prompt line when there is no TTY to wait on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(feedback): keep the manual fallback on every gh failure Only missing-gh and unauthenticated flows showed the formatted feedback and pre-filled submission URL; issues-disabled, network, or rate-limit failures printed gh's stderr and discarded the path to submit what the user had already typed. Route those through the same manual fallback, preserving gh's exit code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(update): model the npm shim in the Windows prefix fixture The ownership corroboration now checks for the openspec.cmd shim npm writes beside node_modules; the Homebrew-prefix fixture built the layout without it, so the test failed on windows-pwsh. Write the shim in the fixture and pin the inverse: the same shape with nothing npm wrote (a hand-copied portable tree) is not an npm install. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(update): require volta's full tools/image layout for the undotted spelling The corroboration used has('tools', 'image'), which is some() — volta AND (tools OR image) — so /srv/volta/tools/apps/... still classified as a Volta install and swallowed the upgrade offer. Require both segments, matching the real %LOCALAPPDATA%\Volta\tools\image layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(adapters): escape control characters in Gemini multiline prompts escapeTomlMultilineBasicString handled backslashes and quote-triples but not the C0 controls that are as invalid in a multiline basic string as in a single-line one. Reuse TOML_CONTROL_CHARS, applied last so the escapes it introduces are not re-doubled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(completion): finish the $ZSH_CUSTOM support and isolate it in tests The fpath verification advice still grepped the literal custom/completions, which a relocated $ZSH_CUSTOM need never contain — grep the actual directory instead. The installer tests cleared only $ZSH, so on a machine exporting $ZSH_CUSTOM they would have written into (and deleted from) the developer's real OMZ custom dir — the same leakage class Fission-AI#1400 fixed for $ZSH. Clear/restore both, and pin the custom-location paths with two new tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(release): correct the hermes and zcode changeset wording Hermes is skills-only (no command adapter), and zcode's namespaced commands register /opsx:<id>, not /opsx-* — the release notes must not reintroduce the invocation-spelling confusion Fission-AI#1471 removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(feedback): pin the manual fallback on a non-label gh failure The new reportGhFailure output (formatted feedback + pre-filled URL) had no coverage; the network-failure test now asserts it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(completion): match fpath entries as literal strings in the OMZ guidance The verification advice interpolated the completions dir into grep "<dir>" where regex metacharacters make the check unreliable and quotes could break the displayed command. Print one fpath entry per line and match with grep -F on a shell-quoted literal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(adapters): never emit a bare carriage return in Gemini TOML prompts A lone CR is illegal in a multiline basic string — Python 3.13 tomllib rejects the file — and the control-char pass deliberately skipped it on the assumption it only appears as CRLF. Normalize CRLF to LF and escape any remaining CR as \r. The escaping guarantee is now parser-backed: smol-toml (new devDependency) round-trips every hostile body in the regression matrix (lone CR, CRLF, CR before a quote run, NUL/VT/FF, trailing backslash, four- and five-quote runs), and the same outputs were verified against Python tomllib. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * build(nix): update the pnpm deps hash for the smol-toml devDependency The lockfile changed, so the fixed-output derivation hash moved; value taken from the CI mismatch report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status: LGTM — full suite green (112 files / 2,251 tests, 14 new), lint clean, all 36 in-repo specs validate, every fix verified end-to-end against the built CLI. Hardened by two rounds of three independent adversarial reviews each, plus CodeRabbit; all confirmed findings addressed (details below).
What was wrong: a full audit of v1.6.0..main found no regressions, but a handful of pre-existing gaps — one workflow-blocking, the rest paper cuts:
REMOVED failed ... not found), while ADDED (#1376) and RENAMED (#1386) got already-synced escapes. The sync skill steers users straight into this.- 2 removednext to "2 ignored").## ADDED Requirementscase-sensitively while the parser is case-insensitive, so lowercase headers skipped delta validation and failed later with a worse error.specs/<cap>/spec.mdwas invisible todiscoverSpecFilesbut counted as content byhasAnyFileUnderand the artifact graph — archive would silently drop the delta.openspec show <change>printedWarning: Ignoring flags not applicable to change: scenarios(commander defaults--no-scenariostoscenarios: true).buildCodeFenceMaskexisted twice (#1151 added a second copy); both claimed to be the single source of truth./opsx:continueand/opsx:new, which core never installs (#1410 fixed only the init hints)./opsx:*, but their command files answer to/opsx-<id>— every cross-reference pointed at a nonexistent command.applySpecsexport with zero callers that bypassed store-aware roots.How it was fixed:
treating it as already removed) — analogous to the ADDED/RENAMED escapes, but with a warning since, unlike RENAMED, there is no signal separating early-sync from a typo. Totals count applied removals only. A missing MODIFIED target still aborts./i, so lowercase headers get the samevalidateChangeDeltaSpecserrors validate shows.spec.mdentries (dangling links skipped, other errors still thrown); symlinked directories remain unfollowed, as before.showcountsscenariosas user-provided only when it isfalse.buildCodeFenceMasklives incode-fence.ts;requirement-text.tsre-exports it (no import-site churn)./opsx:continue//opsx:newsuggestions but name the CLI fallback (openspec status --change/openspec instructions) for profiles that don't install them; onboard's extra-command tables say "only if installed."init --tools qwen,boboutput has zero/opsx:references left.applySpecsdeleted (not reachable from the public entry point).Adversarial-review round (second commit): three independent reviews (core logic, generation/templates, tests/compat) plus CodeRabbit produced these additional fixes, each with tests:
validateandarchive— previously it failed incidentally at apply time, and the new warn-and-continue path would have masked it.archive --jsoncarries spec-merge warnings in a new optionalwarningsarray, so agent flows see the skipped-REMOVED signal (previously the silent path swallowed it entirely — including for typos)./opsx-proposefor qwen/bob/opencode/pi/oh-my-pi,/skill:openspec-proposefor Kimi)./opsx:continueisn't installed (CodeRabbit).openspec-conventions/spec.mdupdated to state the idempotent archive semantics (direct spec edit, same precedent as fix(archive): keep the delta spec's Purpose in a new main spec #1431 — this also fixes the clause that fix(archive): stop failing on specs that were already synced before archiving #1376's idempotent-ADDED already contradicted), and a changeset was added.Round-2 review (third commit): a second set of three adversarial reviews on the updated diff (second-round-delta, fresh-eyes with a mandatory attack set, and consumers/contract) all returned merge-safe, and produced two more hardening fixes: a REMOVED header that differs only in case or interior whitespace from an existing requirement now stays a hard abort naming the near-miss (a fold-insensitive match present in the spec means typo, not early-sync), and
specsUpdatedis true only when a spec file was actually written — a fully-already-synced change printsSpecs already in sync; no files changed.and reportsspecsUpdated: falsewith its skips inwarnings(CodeRabbit). The reviews also verified: no template or skill parses archive output, the deleted exports were never publicly reachable (exportsmap blocks deep imports, so the patch bump is right), CRLF deltas work through the new paths, error paths never leak partial warnings arrays, and all parity hashes were independently recomputed.Replication / proof: repro before the fix — sync a REMOVED delta to the main spec, then
openspec archive <change>→REMOVED failed for header ... - not found, exit 1. After: archives with the warning, honest+0/−0totals, and the main spec byte-untouched (verified live and pinned by test). New tests cover each fix: early-synced REMOVED (human + JSON), applied-only counts, RENAMED+REMOVED conflict (validate + archive), lowercase-header gate parity, symlinked/danglingspec.md, default vs explicit--no-scenarios, qwen hyphen transform, transformer selection, and the per-tool init hint. Parity hashes andskills/regenerated via the documented recipe.Notes / nits:
/opsx:cross-reference pattern; each tool's invocation convention needs individual verification, so this PR only fixes the two with a documentedopsx-<id>surface (and matching in-repo precedent). Codex reference rewriting stays parked, per the existing comment.🤖 Generated with Claude Code
Summary by CodeRabbit
spec.mdfiles and ignores dangling symlinks.openspec show --jsonscenario warnings now respect explicit--no-scenarios.openspec archivenow preserves non-fatal warnings in output, avoids unnecessary rewrites, and treats early-synced removals as warnings.--jsoncontract to reflect optional command availability and warning output.