fix(apply): surface deferred scope instead of silently simplifying tasks - #1530
fix(apply): surface deferred scope instead of silently simplifying tasks#1530clay-good wants to merge 5 commits into
Conversation
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>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe apply workflow now requires agents to report unexpected scope, pause for guidance, avoid silent simplification or deferral, and mark tasks complete only after full implementation. Tests cover the skill and command templates, with updated parity hashes and a patch changeset. ChangesApply scope guardrails
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/core/templates/apply-defer-guardrail.test.ts`:
- Around line 14-17: Update the assertions in the apply-defer guardrail test to
require the exact phrase “surface the added scope and pause” for both rendered
instruction surfaces, while preserving the existing checks prohibiting silent
simplification or deferment. Align the test with the corresponding requirement
in the apply-change workflow template.
🪄 Autofix
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: d384c31a-06f7-4c61-97f6-bd92ed705c9e
📒 Files selected for processing (5)
.changeset/apply-surface-deferred-scope.mdskills/openspec-apply-change/SKILL.mdsrc/core/templates/workflows/apply-change.tstest/core/templates/apply-defer-guardrail.test.tstest/core/templates/skill-templates-parity.test.ts
| it('tells the agent to surface added scope rather than defer or simplify', () => { | ||
| expect(instructions).toContain('surface the added scope'); | ||
| expect(instructions).toMatch(/never silently simplify, defer/); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the pause requirement explicitly.
The test checks that the instructions surface added scope and prohibit silent simplification or deferment. It does not check that the agent pauses. The test could pass if the workflow reports the added scope and then continues without waiting for guidance.
Add surface the added scope and pause to the single-surface assertion and use the same phrase for both rendered surfaces. The production requirement is in src/core/templates/workflows/apply-change.ts, Lines 181-182.
Proposed assertion update
- expect(instructions).toContain('surface the added scope');
+ expect(instructions).toContain('surface the added scope and pause');
...
- const needle = 'surface the added scope';
+ const needle = 'surface the added scope and pause';This follows the PR objective that unexpected scope must pause the workflow.
Also applies to: 23-26
🤖 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/apply-defer-guardrail.test.ts` around lines 14 - 17,
Update the assertions in the apply-defer guardrail test to require the exact
phrase “surface the added scope and pause” for both rendered instruction
surfaces, while preserving the existing checks prohibiting silent simplification
or deferment. Align the test with the corresponding requirement in the
apply-change workflow template.
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>
…idance 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>
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>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
This correctly anchors the pause to scope changes rather than effort, keeps the approved artifacts authoritative, and requires full specified behavior before checking a task off. Exact-head build, lint, focused guidance/parity tests, and regeneration are clean.
2 participants
Status
LGTM — passed multiple rounds of parallel adversarial review (semantics/over-pause, docs consistency, regen determinism, merge-readiness, repo-wide drift, behavioral robustness) plus the full test suite.
Closes #1529.
What was wrong
The
/opsx:applyguidance told agents to "keep going through tasks until done or blocked" but never said what to do when a task needed work beyond what the spec described. So agents absorbed the extra scope silently — narrowing it, deferring it, or accepting partial work — and still marked the task- [x]. The change then looked done while important, specified work was never implemented (#1529).What it does
Adds one pause trigger and two guardrails to the shared apply instructions (
getApplyInstructions()), which both the skill and command surfaces render, so they can't drift. The trigger is deliberately anchored to a change in scope, not to effort (a hard-but-specified task should not trip it):- [x]when its specified behavior is fully implemented, not when it is partially done or deferred.Guidance text only — no behavioral code paths change. The static
skills/openspec-apply-change/SKILL.mdand the three parity-hash pins were regenerated with the repo's owngenerate:skills/regen:parity-hashesscripts (the only three hashes that change are the two apply surfaces and the apply skill content).Proof it works
test/core/templates/apply-defer-guardrail.test.tsasserts the guidance is present ingetApplyInstructions()and on both surfaces; a full revert removes every asserted string, so it fails correctly.npm run generate:skills+npm run regen:parity-hasheson the committed tree produce zero further diff — the generated artifacts match the source exactly (verified deterministic across repeated runs).tsc --noEmitandeslintclean.Notes
apply-change—ff-changeandcontinue-changeare planning/artifact-creation surfaces that hand off to/opsx:applyand don't implement tasks or touch checkboxes, so they don't share this gap.- [x]before the behavior is fully implemented. Both new rules bite.state.ts) are distinct problems with different fixes and are intentionally not claimed here.🤖 Generated with Claude Code
Summary by CodeRabbit
Workflow Improvements
Tests