[P0] #404 — Execute the 15 canonical stages as individual tasks (not 8 mission bundles) - #413
Conversation
…8 bundles sdlc_plan now emits one task per canonical SDLC stage (15) instead of grouping them into 8 missions. Each stage becomes its own execution unit: its own builder contract, its own validator profile (resolveValidatorProfile receives a single stage, so architecture/compliance are no longer shadowed by the security profile they used to share a bundle with), its own approval gate, checkpoint, and memory episode. A single validator verdict can no longer mark three stages PASS at once. - lifecycleStages is generated from CANONICAL_SDLC_STAGES; MISSION_META carries the per-mission intent, inherited by each stage. - Missions are retained as display-only grouping metadata on each task (mission_id/mission_title) so the dashboard can still swimlane by mission. - The 8 mission spec documents (product-brief.md, architecture.md, ...) stay decoupled via missionSpecStages, so sdlc_spec / approvals / the release gate are unchanged. - requiredApprovalsForTask rekeyed off canonical stage ORDER (the old lexicographic mission-id thresholds no longer bucket correctly), with a legacy fallback for non-canonical ids. - budgetEnvelopeForTask fallback now divides the run budget across 15 stages. Cross-harness by construction: this lives in the shared bridge implementation (bin/rstack-bridge.ts imports it), so Pi, Claude Code, Tau, Operator, and Hermes all get it. No tool names or param schemas changed — bridge conformance holds. Tests updated (tests/helpers/claim.js added to target mid-pipeline stages). No regressions vs baseline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughSDLC planning now creates 15 canonical stage tasks, retains 8 mission-level specification briefs, aligns approval and budget calculations with canonical stages, and updates integration tests to claim and validate canonical task identifiers. ChangesCanonical stage execution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Planner
participant CanonicalStages
participant Tasks
participant Validator
Planner->>CanonicalStages: derive 15 execution stages
CanonicalStages->>Tasks: create one task per stage
Tasks->>Validator: claim canonical task
Validator-->>Tasks: record stage validation and attribution
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (3)
src/integrations/pi/rstack-sdlc.ts (1)
762-780: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist and guard
codeIdx/archIdxagainst silent drift.
codeIdx/archIdxare recomputed viafindIndexon every call and keyed off the literal strings"07-code"/"06-architecture". If those canonical ids are ever renamed, both lookups silently return-1, andidx >= -1becomes always true — every canonical stage would require the fullplan+requirements+architectureapproval set with no error raised, unlike the fail-fast checks a few lines above for mission ownership (Lines 358-360). Consider hoisting these to module scope alongside athrowif either id is missing, for consistency with the existing fail-fast pattern.♻️ Suggested refactor
+const CODE_STAGE_IDX = CANONICAL_SDLC_STAGES.findIndex((stage) => stage.id === "07-code"); +const ARCHITECTURE_STAGE_IDX = CANONICAL_SDLC_STAGES.findIndex((stage) => stage.id === "06-architecture"); +if (CODE_STAGE_IDX === -1 || ARCHITECTURE_STAGE_IDX === -1) { + throw new Error("requiredApprovalsForTask: expected canonical stages 07-code/06-architecture not found"); +} + function requiredApprovalsForTask(taskId: string): string[] { const idx = CANONICAL_SDLC_STAGES.findIndex((stage) => stage.id === taskId); if (idx === -1) { if (taskId >= "004-implementation") return ["plan.md", "requirements.json", "architecture.md"]; if (taskId >= "003-architecture") return ["plan.md", "requirements.json"]; return ["plan.md"]; } - const codeIdx = CANONICAL_SDLC_STAGES.findIndex((stage) => stage.id === "07-code"); - const archIdx = CANONICAL_SDLC_STAGES.findIndex((stage) => stage.id === "06-architecture"); - if (idx >= codeIdx) return ["plan.md", "requirements.json", "architecture.md"]; - if (idx >= archIdx) return ["plan.md", "requirements.json"]; + if (idx >= CODE_STAGE_IDX) return ["plan.md", "requirements.json", "architecture.md"]; + if (idx >= ARCHITECTURE_STAGE_IDX) return ["plan.md", "requirements.json"]; return ["plan.md"]; }🤖 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/integrations/pi/rstack-sdlc.ts` around lines 762 - 780, Hoist the canonical stage indexes used by requiredApprovalsForTask into module scope alongside the existing stage definitions, and fail fast if either the "07-code" or "06-architecture" stage is missing instead of allowing -1 indexes. Update requiredApprovalsForTask to reuse these validated constants while preserving its approval thresholds and legacy-ID fallback behavior.src/core/profiles.js (1)
159-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the stage count instead of hardcoding
15.The fallback divides by a literal
15, duplicating the canonical-stage count thatCANONICAL_SDLC_STAGES.lengthalready represents elsewhere (e.g.lifecycleStagesinrstack-sdlc.ts, and the test assertion intests/extension-stage-attribution.test.js). This reintroduces exactly the kind of count-drift risk#404was meant to eliminate — if a canonical stage is added/removed, this fallback silently goes stale.♻️ Suggested refactor
- // `#404`: the run budget is now spread across the 15 canonical stage tasks - // (previously 8 bundled missions) when no per-stage budget is configured. - const fallback = Number(budgetPolicy.run_budget_usd || 0) / 15; + // `#404`: the run budget is spread across the canonical stage tasks + // (previously 8 bundled missions) when no per-stage budget is configured. + const fallback = Number(budgetPolicy.run_budget_usd || 0) / CANONICAL_SDLC_STAGES.length;As per coding guidelines, "Keep shared classification and governance decisions centralized rather than duplicating logic across builders, validators, adapters, or UI paths."
🤖 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/profiles.js` around lines 159 - 161, Update the fallback calculation in the relevant profile budget logic to divide by the existing canonical stage count symbol, such as CANONICAL_SDLC_STAGES.length, instead of the literal 15. Reuse the established shared stage definition and preserve the current zero-budget behavior.Source: Coding guidelines
tests/extension-validator-profile.test.js (1)
49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant directory creation.
The
claimTaskForTesthelper already ensures that the task's output directory is created recursively. ThemkdirSynccall here is redundant and can be safely removed.♻️ Proposed refactor
const outputDir = join(projectRoot, archTask.output_dir); - mkdirSync(outputDir, { recursive: true });🤖 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 `@tests/extension-validator-profile.test.js` around lines 49 - 50, Remove the redundant mkdirSync call in the test setup around outputDir, relying on claimTaskForTest to create the task output directory recursively. Keep the outputDir path calculation and subsequent test behavior unchanged.
🤖 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.
Nitpick comments:
In `@src/core/profiles.js`:
- Around line 159-161: Update the fallback calculation in the relevant profile
budget logic to divide by the existing canonical stage count symbol, such as
CANONICAL_SDLC_STAGES.length, instead of the literal 15. Reuse the established
shared stage definition and preserve the current zero-budget behavior.
In `@src/integrations/pi/rstack-sdlc.ts`:
- Around line 762-780: Hoist the canonical stage indexes used by
requiredApprovalsForTask into module scope alongside the existing stage
definitions, and fail fast if either the "07-code" or "06-architecture" stage is
missing instead of allowing -1 indexes. Update requiredApprovalsForTask to reuse
these validated constants while preserving its approval thresholds and legacy-ID
fallback behavior.
In `@tests/extension-validator-profile.test.js`:
- Around line 49-50: Remove the redundant mkdirSync call in the test setup
around outputDir, relying on claimTaskForTest to create the task output
directory recursively. Keep the outputDir path calculation and subsequent test
behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ae4c98e-6aa0-4932-b0cb-b57998f7f0ca
📒 Files selected for processing (13)
src/core/profiles.jssrc/integrations/pi/rstack-sdlc.tstests/environment-report.test.jstests/extension-checkpoints.test.jstests/extension-goal-gate.test.jstests/extension-memory.test.jstests/extension-retry-policy.test.jstests/extension-stage-attribution.test.jstests/extension-validator-profile.test.jstests/harness.test.jstests/helpers/claim.jstests/people-layer-approvals.test.jstests/profiles.test.js
2 participants
Closes #404.
What
sdlc_plannow emits one task per canonical SDLC stage (15) instead of grouping them into 8 missions. Each stage becomes a first-class execution unit: its own builder contract, its own validator profile, its own approval gate, checkpoint, and memory episode. A single validator verdict can no longer mark three bundled stages PASS at once.Why it matters
resolveValidatorProfilenow receives a single stage, so06-architectureand13-compliance-checkerget their own validators instead of losing to the security profile they used to share a mission bundle with.How
lifecycleStagesis generated fromCANONICAL_SDLC_STAGES;MISSION_METAholds the per-mission intent each stage inherits.mission_id/mission_title) so the dashboard can still swimlane by mission.product-brief.md,architecture.md, …) stay decoupled viamissionSpecStages, sosdlc_spec/ approvals / the release gate are unchanged.requiredApprovalsForTaskrekeyed off canonical stage order (the old lexicographic mission-id thresholds no longer bucket correctly), with a legacy fallback for non-canonical ids.budgetEnvelopeForTaskfallback divides the run budget across 15 stages.Harness-agnostic by construction
This lives in the shared bridge implementation (
bin/rstack-bridge.tsimportssrc/integrations/pi/rstack-sdlc.tsverbatim), so Pi, Claude Code, Tau, Operator, and Hermes all get it — a future adapter shells the same bridge. No tool names or param schemas changed, sobridge-conformanceholds.Tests
Fixtures updated to the 15-stage model;
tests/helpers/claim.jsadded to target mid-pipeline stages. Full suite shows no regressions vsmainbaseline (the only failures are pre-existing Windowsspawn npx ENOENT/EBUSYenvironmental issues that also fail onmain).Base:
main. First in the Wave 1 stack (#404 → #405, #407).Summary by CodeRabbit
New Features
Improvements