feat(sdd-triage): defer sub-issue creation until /approve (#79)#92
Merged
Conversation
Phase B now posts the proposed plan as a single comment on the tracking issue and creates no sub-issues. Phase C, on /approve, materializes the plan by creating both Unit and task sub-issues together (ADR 0010). The cycle and requirement-coverage checks run before any create-issue fires, so a hand-off never leaves an orphan tree. Adds ADR 0010, which also records the post-approve /revise reconciliation rule (close items dropped from the revised plan, create items added, leave intersections alone; refuse while any task is sdd:in-progress or has an open implementation PR). Updates the spec (R5.4 / R5.5 and the triage acceptance tests), shared/sdd-interaction.md, docs/sdd/index.md (diagram and step table), and the bug/feature issue templates to match.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughImplements ADR 0010: Phase B posts one sentinel plan comment (no sub-issues); Phase C ( ChangesPlan-comment-before-tree gate restructuring
Sequence Diagram (high-level)sequenceDiagram
participant ArchPR as Architecture PR
participant Triage as sdd-triage Agent
participant Tracking as Tracking Issue
participant Human as Human Reviewer
ArchPR->>Triage: merge (arch/<slug>)
Triage->>Tracking: post plan comment <!-- sdd-triage:plan -->
Human->>Tracking: /revise or /approve
alt revise before approve
Triage->>Tracking: post new plan, hide prior as OUTDATED
else approve
Triage->>Triage: validate cycle & coverage
Triage->>Tracking: create-issue Unit(s)
Triage->>Tracking: create-issue Task(s) (parent=Unit)
Triage->>Tracking: label sdd:ready (where unblocked)
end
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues:
Possibly related PRs:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 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 @.github/workflows/sdd-triage.lock.yml:
- Around line 884-885: The compiled prompt is losing the sentinel literal `<!--
sdd-triage:plan -->` (leaving empty backticks) during lock-file generation;
update the lock-file generation/compilation routine (e.g., functions like
compilePrompt or generateCompiledPrompt / generate_lockfile) to preserve or
explicitly inject the sentinel into the compiled prompt code-fence so the exact
string `<!-- sdd-triage:plan -->` appears (not empty backticks), and add a
unit/integration check that the compiled output contains that literal to prevent
regressions.
In @.github/workflows/sdd-triage.md:
- Around line 42-43: The workflow sets hide-comment.max = 5 which conflicts with
the steps that must "hide every prior plan comment" after repeated /revise runs;
update the hide-comment configuration (the hide-comment and hide-comment.max
setting) to remove the fixed cap (or set it to an unlimited value such as 0 or
"all") so it can hide every prior plan comment as required by the plan-comment
hiding logic referenced by the /revise flows and the
hide-every-prior-plan-comment steps.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 80c31ac6-65c1-41bb-9f37-0d74b75774f8
📒 Files selected for processing (8)
.github/workflows/sdd-triage.lock.yml.github/workflows/sdd-triage.mddecisions/0010-plan-comment-before-tree.mddocs/sdd/index.mddocs/specs/01-spec-issue-native-sdd/01-spec-issue-native-sdd.mdshared/sdd-interaction.mdtemplates/.github/ISSUE_TEMPLATE/bug.mdtemplates/.github/ISSUE_TEMPLATE/feature.md
…se runs Phase B (lines 295-297) and the post-approve /revise reconcile (lines 449-451) both require hiding **every** prior plan comment as OUTDATED. The previous cap of 5 meant that after enough /revise iterations the agent could no longer satisfy that invariant in one run, leaving stale active plan comments alongside the latest one. Match the close-issue / create-issue cap of 30, which is the practical ceiling on tree size and therefore on revisable-plan history per tracking issue.
gh aw's markdown→YAML compiler strips HTML comments even inside inline code spans, so the literal `<!-- sdd-triage:plan -->` was dropping out of the compiled prompt, leaving empty backticks at every reference site. The agent prompt no longer named the exact string it must write into plan comments or match against on `/revise` and `/approve`. Backslash-escape the bang (`<\!-- sdd-triage:plan -->`) at every prompt site so the compiler preserves the literal, and add a short parenthetical at each site clarifying that the backslash is a prompt-escape only — the actual sentinel posted to GitHub is the un-escaped HTML comment.
8 tasks
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
4 tasks
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/sdd-triage.md (1)
219-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPhase A PR-body guidance still describes old Phase B behavior.
This line says phase B creates Unit sub-issues, which contradicts ADR 0010 behavior defined elsewhere in this same workflow (phase B is plan-comment-only).
Suggested fix
- request advances the tracking issue to phase B, where one Unit sub-issue per - demoable unit is created. + request advances the tracking issue to phase B, where one sentinel plan + comment is posted and no sub-issues are created.🤖 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 @.github/workflows/sdd-triage.md around lines 219 - 220, Update the Phase A PR-body guidance text that currently states "request advances the tracking issue to phase B, where one Unit sub-issue per demoable unit is created" to match ADR 0010 and the workflow's actual behavior: change the wording to say that advancing to Phase B only posts the plan comment (plan-comment-only) and does not create Unit sub-issues, and ensure any mention of "Phase B" in the same file (and related rules in this workflow) uses the same language so the guidance is consistent with ADR 0010.
🧹 Nitpick comments (1)
.github/workflows/sdd-triage.lock.yml (1)
1372-1374: 💤 Low valueStatic analysis:
${{ false }}could be simplified.The zizmor tool flags
"draft":"${{ false }}"as obfuscated since it could be replaced with the literalfalse. However, this is auto-generated code fromgh aw compile, and the pattern may be intentional for template consistency. Consider updating the compiler if this pattern is undesirable.🤖 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 @.github/workflows/sdd-triage.lock.yml around lines 1372 - 1374, The safe outputs JSON contains the stringified template expression `"draft":"${{ false }}"` under the create_pull_request object; replace that with a real JSON boolean (`"draft": false`) so the value is not an obfuscated string. Fix this in the generator that emits the here-doc (the GH_AW_SAFE_OUTPUTS_CONFIG... block from gh aw compile) to output a boolean for create_pull_request.draft (or add a post-process step that replaces the exact token `"draft":"${{ false }}"` with `"draft": false`).
🤖 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 `@decisions/0010-plan-comment-before-tree.md`:
- Around line 166-168: The ADR currently states the `hide-comment` safe-output
is capped at 5 but the workflow and `/revise` flow expect and use a cap of 30;
update the document text so the `hide-comment` cap matches the implemented
contract (change "capped at 5 per run" to "capped at 30 per run") and, if
relevant, add a short note referencing the workflow behavior that hides all
prior plan comments during repeated `/revise` runs; ensure you update the line
describing `hide-comment` (the sentence that currently reads "`sdd-triage`
declares two additional safe-outputs: `hide-comment` (capped at 5 per run)...")
to reflect the new cap.
---
Outside diff comments:
In @.github/workflows/sdd-triage.md:
- Around line 219-220: Update the Phase A PR-body guidance text that currently
states "request advances the tracking issue to phase B, where one Unit sub-issue
per demoable unit is created" to match ADR 0010 and the workflow's actual
behavior: change the wording to say that advancing to Phase B only posts the
plan comment (plan-comment-only) and does not create Unit sub-issues, and ensure
any mention of "Phase B" in the same file (and related rules in this workflow)
uses the same language so the guidance is consistent with ADR 0010.
---
Nitpick comments:
In @.github/workflows/sdd-triage.lock.yml:
- Around line 1372-1374: The safe outputs JSON contains the stringified template
expression `"draft":"${{ false }}"` under the create_pull_request object;
replace that with a real JSON boolean (`"draft": false`) so the value is not an
obfuscated string. Fix this in the generator that emits the here-doc (the
GH_AW_SAFE_OUTPUTS_CONFIG... block from gh aw compile) to output a boolean for
create_pull_request.draft (or add a post-process step that replaces the exact
token `"draft":"${{ false }}"` with `"draft": false`).
🪄 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: CHILL
Plan: Pro
Run ID: ede348b2-d51f-41f9-986f-9f9c9d5a2af6
📒 Files selected for processing (8)
.github/workflows/sdd-triage.lock.yml.github/workflows/sdd-triage.mddecisions/0010-plan-comment-before-tree.mddocs/sdd/index.mddocs/specs/01-spec-issue-native-sdd/01-spec-issue-native-sdd.mdshared/sdd-interaction.mdtemplates/.github/ISSUE_TEMPLATE/bug.mdtemplates/.github/ISSUE_TEMPLATE/feature.md
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
This was referenced May 21, 2026
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
1 participant
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.
Summary
<!-- sdd-triage:plan -->sentinel) and creates no sub-issues./approveis now the single gate at which structure is committed to the tree./approve, creates the Unit sub-issues and the implementation task sub-issues together, materializing exactly what the plan comment shows. Cycle and spec-requirement-coverage checks run before anycreate-issueis emitted, so a hand-off never leaves an orphan tree./reviserules (set-difference reconciliation; refuse while any task is in flight).Closes #79.
Changes
.github/workflows/sdd-triage.md— rewrite §5 (phase B is now plan-comment only), rewrite §6 (phase C creates Units and tasks; checks run before anycreate-issue), update §7 (no extra cycle gate), add §9 (post-approve/revisereconcile + in-flight refusal). New safe-outputs:hide-comment(max 5) for collapsing prior plan comments asOUTDATED,close-issuetarget: '*'(max 30) for reconciliation closes.create-issuecap raised from 20 to 30 to cover Units alongside sub-tasks in one phase-C run..github/workflows/sdd-triage.lock.yml— regenerated bygh aw compile.decisions/0010-plan-comment-before-tree.md— new ADR.docs/specs/01-spec-issue-native-sdd/01-spec-issue-native-sdd.md— rewrites R5.4 / R5.5; updates the triage acceptance tests at L500–L502; updates the design-notes paragraph framing the three phases; small wording fix in the introduction and Unit 5 purpose.shared/sdd-interaction.md— restates the/approverow, the "merging the architecture PR…" sentence, and the phase-coverage line.docs/sdd/index.md— Mermaid nodesa_unitsanda_tasksretitled; step table rows 5 and 6 retitled; adds a one-paragraph rationale under the table.templates/.github/ISSUE_TEMPLATE/feature.md,templates/.github/ISSUE_TEMPLATE/bug.md— restate/approveand the "merging the architecture PR" sentence so the user-facing copy matches.Acceptance
Quoting the non-obvious rules verbatim from the issue and ADR 0010:
Post-approve
/revisereconciliation (no task in flight):In-flight refusal:
Verifiable behaviors (see ADR 0010 § Verification and
sdd-triage.md§ Verification):/revise <note>between merge and/approveposts a new plan comment and hides the prior one asOUTDATED. No sub-issues created./approvefrom a write-access author creates Units (parent = tracking issue) and tasks (parent = Unit) in one phase. Tracking issue movessdd:triage→sdd:ready; unblocked tasks carrysdd:readyat creation.needs-humancomment and zerocreate-issuesafe-outputs./revisewith no task in flight reconciles the tree set-difference-wise; a re-run with the same note emits no tree safe-outputs (idempotent)./revisewhile any task carriessdd:in-progressor has an open linked implementation PR posts one refusal comment and emitsnoop; the plan and tree are not changed;needs-humanis not applied.References
decisions/0010-plan-comment-before-tree.md), new in this PR.sdd:readyat creation) — both unchanged; ADR 0010 layers on top of them.parentis set in the samecreate-issuecall still holds; only the phase at which Units are created moves (phase B → phase C).decisions/0010-*.mdis also claimed by issue sdd-dispatch: add /dispatch on the tracking issue to cascade task execution with bounded parallelism; remove daily cron #81 (dispatch cascade); sdd-dispatch: add /dispatch on the tracking issue to cascade task execution with bounded parallelism; remove daily cron #81's PR will renumber if it lands second.Open implementation note
gh-aw has no
update-commentsafe-output today, so "edit the plan comment in place" is rendered as:add-commentposts the revised plan +hide-commentcollapses the prior plan comment asOUTDATED. A reader sees one active plan and GitHub's edit log preserves history. Ifupdate-commentis added later, the agent can switch to literal in-place edits without changing the gate semantics. This is called out in ADR 0010's Consequences.