Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughChangesThe update and apply workflows no longer reference optional workflow handoffs. Missing artifacts and tracking files now use direct CLI recovery guidance. Tests cover generated references, profile transitions, CLI recovery, and updated template hashes. Core workflow handoffs
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Update and apply guidance now uses available CLI recovery commands instead of unavailable core-profile workflow handoffs. The covered recovery and profile-generation paths indicate no remaining merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant Workflow
participant OpenSpecCLI
participant Schema
Workflow->>OpenSpecCLI: Request status for the change
OpenSpecCLI->>Schema: Resolve artifact rules and tracking configuration
Schema-->>OpenSpecCLI: Return instructions and template
OpenSpecCLI-->>Workflow: Return recovery guidance
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
▶ View full results and scan again 🔎 11 requirements drifted — 7 pointing at code, 4 at stale specs.
On 🔴 Apply Instructions Command — code is wrong · highExpected —
Observed —
Next → fix the code at Agent prompt
🔴 Archive Command Argument Support — code is wrong · highExpected —
Observed —
Next → fix the code at Agent prompt
🔴 Slash Command Updates — code is wrong · highExpected —
Observed —
Next → fix the code at Agent prompt
8 more findings are in the full check. View results · Click Refresh, then Scan again in the check. Or comment |
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The core-profile handoff fix is consistent across runtime output, distributed skills, and generated templates. Missing planning artifacts and missing or empty tracking files now get distinct, actionable recovery without assuming optional workflows, while store scoping and planning-only boundaries are preserved. CI is green.
Regenerates the skill/command parity hashes and the shipped skills/ mirror after the merge; main moved explore, ff-change, and propose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying openspec-docs with
|
| Latest commit: |
194116c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6c2b308c.openspec-docs.pages.dev |
| Branch Preview URL: | https://codex-fix-core-workflow-hand.openspec-docs.pages.dev |
#1735 fixes the same issue (#1734) by removing the optional handoffs outright. This PR resolves them at generation time instead, which is strictly better for the template layer: an install that has `continue` still gets told about it. So the mechanism here wins and #1735's content is folded in, rather than the two competing for the same lines. What #1735 had that this did not: - src/commands/workflow/instructions.ts. The CLI's own runtime strings named the openspec-continue-change skill. Those are chosen at run time, so optionalWorkflow() cannot reach them; taken from #1735 verbatim. - The blocked-state fallback. It was a one-line pointer; it now carries #1735's full CLI recovery (select the next `ready` artifact, not `skipped` or `blocked`, read its rules with `openspec instructions`, keep the selected `--store` on both commands) plus the tracking-file repair path and the `missingArtifacts` field it branches on. The installed branch still names `/opsx:continue`, so neither audience loses. #1735's update-change.ts rewrite is not carried over: this PR already covers all six of those sites conditionally, which is the better answer. Both of #1735's test suites come across, and they are worth more here than there. test/core/templates/profile-handoffs.test.ts asserts that no generated file names an uninstalled workflow across every tool and all three delivery modes, which is the property this PR's mechanism exists to provide, and it passes against it. test/commands/profile-handoffs.test.ts covers the runtime CLI strings. The two guards are complementary: that one is broad on tools and deliveries, this PR's own profile-workflow-references.test.ts is broad on workflow subsets. #1735's command-references.test.ts assertions could not be carried as written, since they assume the reference is gone unconditionally. Replaced with a case that resolves the template against a set without `continue` and asserts the fallback carries the whole recovery. Verified it fails when the fallback is shortened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing as folded into #1775 ( #1775 keeps the mechanism, because it is the better one: this PR removed the optional handoffs unconditionally, which fixes the core profile by taking Everything this PR had that #1775 lacked is carried over, not dropped:
The one thing not carried over is the |
…its (Fission-AI#1775) * fix(templates): stop generated skills naming workflows the profile omits The `core` profile installs six of the twelve workflows, but the update and apply templates named `/opsx:continue` (6 times) and `/opsx:new` (twice) regardless. On a default install those became `/openspec-continue-change` and `/openspec-new-change` — skills that were never written — so `update-change` refused to create a missing artifact and handed off to a dead end. The only guard was a sentence asking the model to check availability at runtime, 70 lines above the two places it hits the wall. `command-references.ts` decides how a reference is spelled; nothing decided whether it should be emitted at all. Add that: templates author both wordings with `optionalWorkflow()`, and `getSkillTemplates()` / `getCommandTemplates()` — the one place every generation path already funnels the resolved workflow set through — pick a branch before the reference transformers run. A profile that omits a workflow now gets a concrete `openspec status` / `openspec instructions` fallback instead of a reference to a skill that does not exist. Closes Fission-AI#1734 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(changeset): describe profile-aware workflow references Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(init): assert the core profile names no uninstalled workflow The end-to-end init test pinned the runtime availability hedging that Fission-AI#1734 is about, and asserted `/opsx:continue` appears in the default profile's generated update workflow — the bug itself. Assert the fixed behavior instead: neither `/opsx:continue` nor `/opsx:new` appears, and the CLI fallback is stated outright, for both the update and apply surfaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(templates): resolve every cross-workflow reference, not just core's The first commit fixed the two templates the default profile broke on. Every other cross-workflow handoff had the same shape, and arbitrary subsets are reachable: a `custom` profile is whatever the user picked, and `openspec update` re-derives a workflow set from what it finds on disk (legacy tool overrides, inferred Codex workflows) without passing it through getProfileWorkflows. So resolve all of them: - `apply` -> archive; `continue` -> apply, archive; `ff` -> apply; `new` -> continue; `propose` -> apply; `update` -> apply, archive; `archive` and `bulk-archive` -> sync. - `onboard`'s two command-reference tables are built from the installed set rather than printed in full with an "only if installed" caveat, and its explore, resume and next-step prompts are resolved the same way. Two supporting changes: - `onlyWithWorkflow()` plus a whole-line rule in the resolver: a conditional that owns its line takes the line with it when it resolves to empty, so a dropped table row cannot leave a blank line that ends the table in markdown. - `generateSkillContent()` and `generateCommand()` now throw on an unresolved marker. A generation path that skips the choke point fails loudly instead of writing `[[opsx:...]]` into a user's SKILL.md. The propose and ff surfaces keep their deliberate wording difference (Fission-AI#258): the command surface never invites "ask me to implement", so its missing-`apply` fallback names the CLI rather than a conversation. The guard test now runs the property over every subset that could expose a reference — each workflow alone, everything but one, the empty set, and the two shipped profiles — for skills and commands, in both spellings. Twenty-plus of those cases fail against the previous commit. Only `openspec-onboard` changes in the skills/ mirror: with every workflow installed, all other templates render byte for byte as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(changeset): cover the full cross-workflow reference fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(templates): validate conditionals before choosing a branch Resolution discarded the unselected branch and only then checked for residual markers, so a truncated block inside the *missing* branch was accepted for a profile that installs the workflow and rejected for one that does not. Profile-dependent authoring errors are exactly what this module exists to remove. Validate the authored text up front instead: every marker must be one of the three recognized forms, and they must appear as a flat sequence of if / else / end. A malformed block now throws identically for every profile. The post-resolution check stays as a backstop. Caught by CodeRabbit on Fission-AI#1775. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state the profile-aware handoff rule in the skills reference alfred-openspec on Fission-AI#1775: docs-lab/reference/skills.md described several handoffs as unconditional while this change deliberately emits a CLI or conversational fallback when the profile omits the target. Stated once, above the entries, rather than as a caveat on each of the eleven affected Response and Creates rows: the page's recipe is one fact per row, and repeating the same conditional eleven times would bury the contracts it exists to state. The rows keep naming the skill that owns the next step, which is the fact a reader looks up; the rule above them says what happens when that skill is not installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(workflows): fold Fission-AI#1735 into the profile-aware references Fission-AI#1735 fixes the same issue (Fission-AI#1734) by removing the optional handoffs outright. This PR resolves them at generation time instead, which is strictly better for the template layer: an install that has `continue` still gets told about it. So the mechanism here wins and Fission-AI#1735's content is folded in, rather than the two competing for the same lines. What Fission-AI#1735 had that this did not: - src/commands/workflow/instructions.ts. The CLI's own runtime strings named the openspec-continue-change skill. Those are chosen at run time, so optionalWorkflow() cannot reach them; taken from Fission-AI#1735 verbatim. - The blocked-state fallback. It was a one-line pointer; it now carries Fission-AI#1735's full CLI recovery (select the next `ready` artifact, not `skipped` or `blocked`, read its rules with `openspec instructions`, keep the selected `--store` on both commands) plus the tracking-file repair path and the `missingArtifacts` field it branches on. The installed branch still names `/opsx:continue`, so neither audience loses. Fission-AI#1735's update-change.ts rewrite is not carried over: this PR already covers all six of those sites conditionally, which is the better answer. Both of Fission-AI#1735's test suites come across, and they are worth more here than there. test/core/templates/profile-handoffs.test.ts asserts that no generated file names an uninstalled workflow across every tool and all three delivery modes, which is the property this PR's mechanism exists to provide, and it passes against it. test/commands/profile-handoffs.test.ts covers the runtime CLI strings. The two guards are complementary: that one is broad on tools and deliveries, this PR's own profile-workflow-references.test.ts is broad on workflow subsets. Fission-AI#1735's command-references.test.ts assertions could not be carried as written, since they assume the reference is gone unconditionally. Replaced with a case that resolves the template against a set without `continue` and asserts the fallback carries the whole recovery. Verified it fails when the fallback is shortened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: name the core-profile fallback on the two rows that hit it The rule above the entries covers every profile, but apply-change and update-change are Core skills whose rows name openspec-continue-change, which the core profile never installs. On the default install those rows now say what the generated skill points to instead: openspec status and openspec instructions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(changeset): drop em dashes from the release note Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2 participants
Status
LGTM after hardening at
03631d8d5. All CI checks pass, including Linux, macOS, Windows, security, and CodeRabbit. Maintainer review remains a merge gate; this PR is not merged.What was wrong
Core installs neither
continuenornew, but update/apply handoffs still named them. The CLI's runtime apply instructions also recommended the missing continue skill, including when planning was complete but its tracking file was missing or empty.Fixes #1734. Follows up on #963 and #919's default-profile acceptance criteria. Preserves the existing #913 archive/sync dependency and #1409 profile-aware console guidance.
How it was fixed
status/instructionsfor missing artifacts,new changefor distinct intent.This follows PR #726's decision 9. No new transformer, CLI command, dependency, core workflow, schema field, or artifact-graph behavior. Task parsing, readiness, progress, and completion calculations are unchanged.
Replication / proof
Notes / nits
Summary by CodeRabbit
Workflow Improvements
Bug Fixes