fix(sdd-apply): move Executor Override before Orchestrator Gate#1105
fix(sdd-apply): move Executor Override before Orchestrator Gate#1105chrisssp wants to merge 1 commit into
Conversation
The Executor Override block was placed after the ORCHESTRATOR GATE, causing LLMs to process the delegation instruction first (due to autoregressive decoding) before reaching the override that tells them they ARE the executor. Moving the override above the gate ensures the sub-agent reads 'you are the executor — execute' before encountering the gate, preventing self-delegation loops. Closes Gentleman-Programming#721
📝 WalkthroughWalkthroughThe Changessdd-apply execution gating
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Reorders the sdd-apply skill’s front-matter instructions so the Executor Override is read before the ORCHESTRATOR GATE, preventing the executor sub-agent from incorrectly self-delegating due to instruction ordering.
Changes:
- Moved
## Executor Overrideabove theORCHESTRATOR GATEblock ininternal/assets/skills/sdd-apply/SKILL.md. - Updated wording from “gate above” to “gate below” to match the new ordering.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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 `@internal/assets/skills/sdd-apply/SKILL.md`:
- Line 14: Change the “Executor Override” heading in SKILL.md from ATX syntax to
the repository’s configured setext heading style, unless ATX headings are
intentionally supported; in that case, update the markdownlint MD003
configuration accordingly.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 12936d4e-bd07-4bb9-be52-9ee1756ffe1d
📒 Files selected for processing (1)
internal/assets/skills/sdd-apply/SKILL.md
| delegate_only: true | ||
| --- | ||
|
|
||
| ## Executor Override |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured Markdown heading style.
## Executor Override triggers markdownlint MD003 because this repository expects setext headings. Convert it to the configured style, or update the markdownlint configuration if ATX headings are intentional.
Suggested fix
-## Executor Override
+Executor Override
+-----------------📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Executor Override | |
| Executor Override | |
| ----------------- |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 14-14: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
🪛 SkillSpector (2.3.7)
[error] 1: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
[error] 276: [P2] Hidden Instructions: Hidden instructions were detected in comments or invisible text. These could contain malicious directives. Manual review is recommended.
Remediation: Audit all comments and invisible characters. Remove any instructions that direct the agent to perform unauthorized actions. Use plain, reviewable content.
(Prompt Injection (P2))
🤖 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 `@internal/assets/skills/sdd-apply/SKILL.md` at line 14, Change the “Executor
Override” heading in SKILL.md from ATX syntax to the repository’s configured
setext heading style, unless ATX headings are intentionally supported; in that
case, update the markdownlint MD003 configuration accordingly.
Source: Linters/SAST tools
🔗 Linked Issue
Closes #721
🏷️ PR Type
What kind of change does this PR introduce?
type:bug— Bug fix (non-breaking change that fixes an issue)type:feature— New feature (non-breaking change that adds functionality)type:docs— Documentation onlytype:refactor— Code refactoring (no functional changes)type:chore— Build, CI, or tooling changestype:breaking-change— Breaking change (fix or feature that changes existing behavior)📝 Summary
The
Executor Overrideblock inskills/sdd-apply/SKILL.mdwas placed after theORCHESTRATOR GATE. LLMs are autoregressive — they process instructions sequentially. When thesdd-applysub-agent reads the skill, the first imperative instruction it encounters is "STOP. Delegate to the dedicated sdd-apply sub-agent". By the time it reaches the Executor Override (which says the gate does NOT apply), it has already locked onto the delegate action.This causes the sub-agent to attempt self-delegation, resulting in loops or no action taken.
The fix moves the
Executor Overrideabove theORCHESTRATOR GATEso the sub-agent reads "you are the executor — execute" before encountering the gate instruction. The internal reference is also updated ("the gate above" → "the gate below").📂 Changes
internal/assets/skills/sdd-apply/SKILL.md## Executor Overrideblock aboveORCHESTRATOR GATE; updated "the gate above" → "the gate below"🧪 Test Plan
Unit Tests
go test ./...E2E Tests (Docker required)
go test ./...)cd e2e && ./docker-test.sh) — Ubuntu: 79 passed, 0 failed, 2 skipped🤖 Automated Checks
The following checks run automatically on this PR:
additions + deletions) or usesize:exceptionCloses/Fixes/Resolves #Nstatus:approvedtype:*Labeltype:*label must be appliedgo test ./...must passcd e2e && ./docker-test.shmust pass✅ Contributor Checklist
status:approvedsize:exceptionwith rationale documentedtype:*label to this PR (needs maintainer to add via UI)go test ./...)cd e2e && ./docker-test.sh)Co-Authored-Bytrailers💬 Notes for Reviewers
E2E tests ran successfully in Docker (Ubuntu: 79 passed, 0 failed, 2 skipped). The change is purely a Markdown block reordering inside an embedded skill asset — no Go code or behavioral logic was modified.