fix: make apply blocked-state guidance profile-agnostic#1096
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR fixes a bug where the core profile's apply-change workflow incorrectly suggested ChangesApply-change blocked guidance fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/templates/workflows/apply-change.ts (1)
210-210:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUpdate line 210 to use profile-agnostic guidance consistent with line 50.
Line 50 in
getApplyChangeSkillTemplate()was updated to "suggest continuing work on the missing artifacts before re-running apply", but line 210 ingetOpsxApplyCommandTemplate()still suggests using/opsx:continue. Since both templates are generated for the core profile (which includes the 'apply' workflow), and/opsx:continueis not available in core profile, this creates a user-facing inconsistency where the command template recommends a command that doesn't exist. Update line 210 to match the profile-agnostic guidance.🤖 Prompt for 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. In `@src/core/templates/workflows/apply-change.ts` at line 210, The message in getOpsxApplyCommandTemplate() still tells users to run `/opsx:continue`, which isn't available in the core profile; change the text to the profile-agnostic guidance used in getApplyChangeSkillTemplate() — i.e., replace the `/opsx:continue` suggestion with a recommendation to continue working on the missing artifacts and re-run apply (phrase it like "suggest continuing work on the missing artifacts before re-running apply") so both templates are consistent.
🧹 Nitpick comments (1)
test/core/templates/skill-templates-parity.test.ts (1)
173-178: ⚡ Quick winConsider: Test coverage should include command template consistency.
The new test verifies that
getApplyChangeSkillTemplate()uses profile-agnostic guidance, but doesn't verifygetOpsxApplyCommandTemplate(). If both templates should be profile-agnostic (as suggested by the PR objective to "make apply blocked-state guidance profile-agnostic"), consider adding a similar assertion for the command template.🧪 Proposed test extension
it('keeps apply blocked-state guidance profile-agnostic', () => { const content = generateSkillContent(getApplyChangeSkillTemplate(), 'PARITY-BASELINE'); expect(content).toContain('suggest continuing work on the missing artifacts before re-running apply'); expect(content).not.toContain('openspec-continue-change'); + + // Also verify command template consistency + const commandContent = getOpsxApplyCommandTemplate().content; + expect(commandContent).not.toContain('openspec-continue-change'); });🤖 Prompt for 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. In `@test/core/templates/skill-templates-parity.test.ts` around lines 173 - 178, The test currently checks that getApplyChangeSkillTemplate() is profile-agnostic but misses getOpsxApplyCommandTemplate(); add a parallel assertion in skill-templates-parity.test.ts that generates content from getOpsxApplyCommandTemplate() with the same profile ('PARITY-BASELINE') and asserts it contains the same blocked-state guidance text ('suggest continuing work on the missing artifacts before re-running apply') and does not contain the profile-specific marker ('openspec-continue-change'), using the same helper generateSkillContent() to produce the content for the assertions.
🤖 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.
Outside diff comments:
In `@src/core/templates/workflows/apply-change.ts`:
- Line 210: The message in getOpsxApplyCommandTemplate() still tells users to
run `/opsx:continue`, which isn't available in the core profile; change the text
to the profile-agnostic guidance used in getApplyChangeSkillTemplate() — i.e.,
replace the `/opsx:continue` suggestion with a recommendation to continue
working on the missing artifacts and re-run apply (phrase it like "suggest
continuing work on the missing artifacts before re-running apply") so both
templates are consistent.
---
Nitpick comments:
In `@test/core/templates/skill-templates-parity.test.ts`:
- Around line 173-178: The test currently checks that
getApplyChangeSkillTemplate() is profile-agnostic but misses
getOpsxApplyCommandTemplate(); add a parallel assertion in
skill-templates-parity.test.ts that generates content from
getOpsxApplyCommandTemplate() with the same profile ('PARITY-BASELINE') and
asserts it contains the same blocked-state guidance text ('suggest continuing
work on the missing artifacts before re-running apply') and does not contain the
profile-specific marker ('openspec-continue-change'), using the same helper
generateSkillContent() to produce the content for the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2b2cee5-e820-42ba-a99a-b3bd776c745d
📒 Files selected for processing (3)
.changeset/fix-apply-blocked-guidance.mdsrc/core/templates/workflows/apply-change.tstest/core/templates/skill-templates-parity.test.ts
… template Line 50 in getApplyChangeSkillTemplate was updated to profile-agnostic phrasing; line 210 in getOpsxApplyCommandTemplate had the same /opsx:continue suggestion. Update for consistency and refresh the parity-test hash.
|
Addressed in 8f99bd8:
|
1 participant
Summary
The blocked-state branch of
getApplyChangeSkillTemplatesuggested runningopenspec-continue-change, which is only available on the full profile. Users on the core profile (no continue-change skill) saw a dead suggestion when their change was blocked by missing artifacts. Change the guidance to ask the user to continue the missing artifacts before re-running apply, which works on every profile.Why this matters
#963 reports the dead suggestion on the core profile. The skill template is the same source for both profiles, so any profile-specific suggestion creates this asymmetry. The replacement text is plain language and works on full profile too (since the suggestion is "continue the missing artifacts," which is what
openspec-continue-changedoes anyway). Updated parity test enforces the new text and asserts the core-only skill name is gone.Fixes #963
AI was used for assistance.
Summary by CodeRabbit
Bug Fixes
Tests
Chores