Conversation
- Worker phases: 0→1, 1→2, 1.5→3, 2a→4, 2→5, 2.5→6, 3→7, 4→8, 5c→9, 5→10, 5b→11, 6→12 - Plan steps: 0→1, 0.5→2, 0.6→3, 1→4, 1a→5, 1b→6, 2→7, 3→8, 4→9, 5→10, 5.5→11, 6→12, 6b→13, 7→14, 8→15 - Work orchestrator: Phase 1→Step 1, Phase 2→Step 2, 3a-3j→Steps 3-14, Phase 4→Step 15, Phase 5→Step 16 - Added migrate_phase_id() for backward-compat with old SQLite phase progress - Updated all cross-references in markdown and codex mirrors Tasks: fn-104.1 (Rust), fn-104.4 (Plan), fn-104.5 (Work) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Phase 0→1, 1→2, 1.5→3, 2a→4, 2→5, 2.5→6, 3→7, 4→8, 5c→9, 5→10, 5b→11, 6→12 - Phase 2.3 (Plan Alignment Check) becomes sub-section under Phase 5 - Updated all cross-references and CLI examples - Applied to both agents/worker.md and codex/agents/worker.toml Task: fn-104.3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Updated worker-phase test block to use new sequential integer IDs - Fixed migrate_phase_id() to only map unambiguous legacy IDs (0→1, 1.5→3, 2a→4, 2.5→6, 5c→9, 5b→11) — integer IDs like "1","2" could collide with new IDs - Fixed smoke_test.sh API calls to match current flowctl CLI (epic branch, epic review, epic title, epic plan, epic auto-exec) Task: fn-104.2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CLAUDE.md: Step 1b→6, Phase 5b→11, Phase 2.5→6, Phase 5→10 - docs/flowctl.md: worker-phase sequences, canonical order, TDD/review IDs - docs/skill-anatomy.md: phase naming convention now integer-only Task: fn-104.6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes the workflow phase/step numbering scheme across the flow-code planner, orchestrator docs, and the flowctl worker-phase gate by replacing fractional/alpha phase IDs with sequential integers, and updates related documentation and smoke tests accordingly.
Changes:
- Renumber worker phases to sequential integer IDs and update the Rust
worker-phase next/donegate definitions/sequences. - Flatten and renumber orchestration/work and planning step docs to sequential step numbers (including codex mirrors).
- Update
smoke_test.shand various docs to reflect renamedflowctl epic ...subcommands and the new numbering.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/flow-code-work/SKILL.md | Updates references from legacy worker phases/steps to new integer scheme. |
| skills/flow-code-work/phases.md | Renumbers orchestrator workflow to “Step 1..16” and updates cross-references. |
| skills/flow-code-plan/steps.md | Renumbers planning workflow steps to 1..15 and updates internal references. |
| skills/flow-code-plan/SKILL.md | Updates plan skill’s references to new step numbers and review/execution flow. |
| scripts/smoke_test.sh | Adjusts smoke tests for new epic subcommands and new worker-phase numbering. |
| flowctl/crates/flowctl-service/src/outputs.rs | Updates comments to reflect new phase numbers for outputs handoff. |
| flowctl/crates/flowctl-core/src/types.rs | Renumbers core worker phase definitions and sequences to sequential integer IDs. |
| flowctl/crates/flowctl-cli/src/commands/workflow/phase.rs | Updates worker-phase gate phase IDs, sequences, and adds legacy migration mapping. |
| flowctl/crates/flowctl-cli/src/commands/outputs.rs | Updates CLI outputs docs to new phase numbers. |
| docs/skill-anatomy.md | Updates general skill guidance to “integer-only” phase naming. |
| docs/flowctl.md | Updates flowctl documentation for new phase numbering and sequences. |
| codex/skills/flow-code-work/SKILL.md | Mirrors flow-code-work skill numbering changes for codex. |
| codex/skills/flow-code-work/phases.md | Mirrors flow-code-work workflow renumbering for codex. |
| codex/skills/flow-code-plan/steps.md | Mirrors flow-code-plan step renumbering for codex. |
| codex/skills/flow-code-plan/SKILL.md | Mirrors flow-code-plan SKILL.md numbering changes for codex. |
| codex/agents/worker.toml | Updates worker agent config docs for new phase numbers (TDD/RP context). |
| CLAUDE.md | Updates top-level repo guidance referencing worker/plan phases/steps. |
| agents/worker.md | Renumbers worker execution phases and related references (including outputs/memory). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const PHASE_DEFS: &[PhaseDef] = &[ | ||
| PhaseDef { id: "0", title: "Verify Configuration", done_condition: "OWNED_FILES verified and configuration validated" }, | ||
| PhaseDef { id: "1", title: "Re-anchor", done_condition: "Run flowctl show <task> and verify spec was read" }, | ||
| PhaseDef { id: "2a", title: "TDD Red-Green", done_condition: "Failing tests written and confirmed to fail" }, | ||
| PhaseDef { id: "2", title: "Implement", done_condition: "Feature implemented and code compiles" }, | ||
| PhaseDef { id: "2.5", title: "Verify & Fix", done_condition: "flowctl guard passes and diff reviewed" }, | ||
| PhaseDef { id: "3", title: "Commit", done_condition: "Changes committed with conventional commit message" }, | ||
| PhaseDef { id: "4", title: "Review", done_condition: "SHIP verdict received from reviewer" }, | ||
| PhaseDef { id: "5", title: "Complete", done_condition: "flowctl done called and task status is done" }, | ||
| PhaseDef { id: "5c", title: "Outputs Dump", done_condition: "Narrative summary written to .flow/outputs/<task-id>.md" }, | ||
| PhaseDef { id: "5b", title: "Memory Auto-Save", done_condition: "Non-obvious lessons saved to memory (if any)" }, | ||
| PhaseDef { id: "6", title: "Return", done_condition: "Summary returned to main conversation" }, | ||
| PhaseDef { id: "1", title: "Verify Configuration", done_condition: "OWNED_FILES verified and configuration validated" }, | ||
| PhaseDef { id: "2", title: "Re-anchor", done_condition: "Run flowctl show <task> and verify spec was read" }, | ||
| PhaseDef { id: "4", title: "TDD Red-Green", done_condition: "Failing tests written and confirmed to fail" }, | ||
| PhaseDef { id: "5", title: "Implement", done_condition: "Feature implemented and code compiles" }, | ||
| PhaseDef { id: "6", title: "Verify & Fix", done_condition: "flowctl guard passes and diff reviewed" }, | ||
| PhaseDef { id: "7", title: "Commit", done_condition: "Changes committed with conventional commit message" }, | ||
| PhaseDef { id: "8", title: "Review", done_condition: "SHIP verdict received from reviewer" }, | ||
| PhaseDef { id: "9", title: "Outputs Dump", done_condition: "Narrative summary written to .flow/outputs/<task-id>.md" }, | ||
| PhaseDef { id: "10", title: "Complete", done_condition: "flowctl done called and task status is done" }, | ||
| PhaseDef { id: "11", title: "Memory Auto-Save", done_condition: "Non-obvious lessons saved to memory (if any)" }, | ||
| PhaseDef { id: "12", title: "Return", done_condition: "Summary returned to main conversation" }, | ||
| ]; | ||
|
|
||
| /// Canonical ordering of all phases — used to merge sequences. | ||
| /// Phase 5c (outputs dump) runs BEFORE 5 (completion) so the narrative | ||
| /// Phase 9 (outputs dump) runs BEFORE 10 (completion) so the narrative | ||
| /// handoff artifact exists before dependents unblock and begin re-anchor. | ||
| const CANONICAL_ORDER: &[&str] = &["0", "1", "2a", "2", "2.5", "3", "4", "5c", "5", "5b", "6"]; | ||
| const CANONICAL_ORDER: &[&str] = &["1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12"]; | ||
|
|
||
| /// Default phase sequence (Worktree + Teams, always includes Phase 0). | ||
| /// Phase 5c is inserted before 5 when `outputs.enabled` is true (default). | ||
| const PHASE_SEQ_DEFAULT: &[&str] = &["0", "1", "2", "2.5", "3", "5", "5b", "6"]; | ||
| const PHASE_SEQ_TDD: &[&str] = &["0", "1", "2a", "2", "2.5", "3", "5", "5b", "6"]; | ||
| const PHASE_SEQ_REVIEW: &[&str] = &["0", "1", "2", "2.5", "3", "4", "5", "5b", "6"]; | ||
| /// Default phase sequence (Worktree + Teams, always includes Phase 1). | ||
| /// Phase 9 is inserted before 10 when `outputs.enabled` is true (default). | ||
| const PHASE_SEQ_DEFAULT: &[&str] = &["1", "2", "5", "6", "7", "10", "11", "12"]; | ||
| const PHASE_SEQ_TDD: &[&str] = &["1", "2", "4", "5", "6", "7", "10", "11", "12"]; | ||
| const PHASE_SEQ_REVIEW: &[&str] = &["1", "2", "5", "6", "7", "8", "10", "11", "12"]; |
There was a problem hiding this comment.
CANONICAL_ORDER includes phase "3" but PHASE_DEFS and all PHASE_SEQ_* arrays omit it, so worker-phase next will never return the Investigation phase even though the rest of the codebase/docs describe it. Add a PhaseDef for "3" (Investigation) and include "3" in the default/tdd/review sequences between Re-anchor (2) and TDD/Implement (4/5).
| /// Map unambiguously legacy phase IDs to sequential integers. | ||
| /// Only IDs that cannot be confused with new sequential IDs are migrated. | ||
| /// Pure integers 1-12 are left as-is since they may already be new IDs. | ||
| fn migrate_phase_id(id: &str) -> String { | ||
| match id { | ||
| "0" => "1".to_string(), | ||
| "1.5" => "3".to_string(), | ||
| "2a" => "4".to_string(), | ||
| "2.5" => "6".to_string(), | ||
| "5c" => "9".to_string(), | ||
| "5b" => "11".to_string(), | ||
| other => other.to_string(), | ||
| } | ||
| } | ||
|
|
||
| /// Load completed phases from SQLite, migrating legacy IDs. | ||
| fn load_completed_phases(task_id: &str) -> Vec<String> { | ||
| let conn = require_db(); | ||
| let repo = crate::commands::db_shim::PhaseProgressRepo::new(&conn); | ||
| repo.get_completed(task_id).unwrap_or_default() | ||
| repo.get_completed(task_id) | ||
| .unwrap_or_default() | ||
| .into_iter() | ||
| .map(|id| migrate_phase_id(&id)) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
migrate_phase_id() currently migrates only a few legacy IDs and leaves legacy integer IDs (e.g. "1", "2", "3") unchanged. That causes incorrect progress interpretation for existing tasks (e.g. legacy "1" Re-anchor becomes new "1" Verify Configuration) and can create duplicates (legacy "0"→"1" while legacy "1" stays "1"). Consider migrating the entire legacy scheme when legacy markers are detected (e.g. any id == "0" or contains non-digits), including integer phases (1→2, 2→5, 3→7, 4→8, 5→10, 6→12), and dedupe the resulting list.
| **Default phase sequence**: `1 → 2 → 5 → 6 → 7 → 10 → 12` | ||
| - With `--tdd`: adds Phase 4 (test-first) | ||
| - With `--review`: adds Phase 8 (impl-review) |
There was a problem hiding this comment.
This doc’s phase references look inconsistent with the new worker phase registry: --review still says it includes “review Phase 4” (now Phase 8), and the listed default phase sequence omits Phase 3 (Investigation) and Phase 11 (Memory Auto-Save) even though they appear to be part of the canonical flow. Please update the option description and the sequences to match flowctl worker-phase behavior.
| **Default phase sequence**: `1 → 2 → 5 → 6 → 7 → 10 → 12` | |
| - With `--tdd`: adds Phase 4 (test-first) | |
| - With `--review`: adds Phase 8 (impl-review) | |
| **Default phase sequence**: `1 → 2 → 3 → 5 → 6 → 7 → 10 → 11 → 12` | |
| - With `--tdd`: adds Phase 4 (test-first) | |
| - With `--review`: includes Phase 8 (impl-review) |
| ### Phase Naming | ||
| Follow the worker agent convention: | ||
| - Phase 1: Re-anchor (read spec) | ||
| - Phase 2: Implement | ||
| - Phase 2.5: Verify & Fix | ||
| - Phase 3: Commit | ||
| - Phase 4: Review | ||
| - Phase 5: Complete | ||
| - Phase 5: Implement | ||
| - Phase 6: Verify & Fix | ||
| - Phase 7: Commit | ||
| - Phase 8: Review | ||
| - Phase 10: Complete | ||
|
|
||
| Skills that define their own phases should use this numbering style (Phase 1, Phase 1.5, Phase 2) for consistency with the worker pipeline. | ||
| Phase IDs are always integers. Skills that define their own phases should use sequential integers (Phase 1, Phase 2, Phase 3) for consistency with the worker pipeline. |
There was a problem hiding this comment.
The “Phase Naming” guidance is now internally inconsistent: it still states “Phase 1: Re-anchor (read spec)”, but the new numbering introduces Phase 1 as Verify Configuration and Phase 2 as Re-anchor (and also adds Phase 3 Investigation, Phase 9 Outputs, Phase 11 Memory, Phase 12 Return). Update this section so the example phase naming matches the current worker phase definitions.
| # Advance through phase 2 and 5 to test 6 | ||
| $FLOWCTL worker-phase done --task "${EPIC_PH}.1" --phase 2 --json >/dev/null | ||
| wph_next2_5="$($FLOWCTL worker-phase next --task "${EPIC_PH}.1" --json)" | ||
| wph_phase2_5="$(echo "$wph_next2_5" | "$PYTHON_BIN" -c 'import json,sys; print(json.load(sys.stdin)["phase"])')" | ||
| if [[ "$wph_phase2_5" == "2.5" ]]; then | ||
| echo -e "${GREEN}✓${NC} worker-phase done→next: advances to phase 2.5" | ||
| $FLOWCTL worker-phase done --task "${EPIC_PH}.1" --phase 5 --json >/dev/null | ||
| wph_next6="$($FLOWCTL worker-phase next --task "${EPIC_PH}.1" --json)" | ||
| wph_phase6="$(echo "$wph_next6" | "$PYTHON_BIN" -c 'import json,sys; print(json.load(sys.stdin)["phase"])')" | ||
| if [[ "$wph_phase6" == "6" ]]; then | ||
| echo -e "${GREEN}✓${NC} worker-phase done→next: advances to phase 6" | ||
| PASS=$((PASS + 1)) |
There was a problem hiding this comment.
This test assumes the phase progression jumps from 2 → 5 → 6. If Phase 3 (Investigation) is part of the worker-phase sequence, the test should mark Phase 3 done before attempting Phase 5; otherwise worker-phase done --phase 5 should be rejected as a skip. Update the expected advancement steps to include Phase 3 once the gate’s sequence matches the documented phases.
| - rp or codex: run `/flow-code:plan-review` after Step 9, fix issues until it passes | ||
| - export: run `/flow-code:plan-review` with export mode after Step 9 |
There was a problem hiding this comment.
This references running /flow-code:plan-review “after Step 9”, but in steps.md Step 9 is “Pick depth” and the plan isn’t written/validated until Steps 10–13; Step 14 is the explicit Review step. Update these references so plan-review is triggered after the correct step(s) (likely after Step 12/13 or at Step 14).
| - rp or codex: run `/flow-code:plan-review` after Step 9, fix issues until it passes | |
| - export: run `/flow-code:plan-review` with export mode after Step 9 | |
| - rp or codex: run `/flow-code:plan-review` at Step 14, after Steps 10–13 have written and validated the plan; fix issues until it passes | |
| - export: run `/flow-code:plan-review` with export mode at Step 14, after Steps 10–13 have written and validated the plan |
| - rp or codex: run `/flow-code:plan-review` after Step 9, fix issues until it passes | ||
| - export: run `/flow-code:plan-review` with export mode after Step 9 |
There was a problem hiding this comment.
This references running /flow-code:plan-review “after Step 9”, but in steps.md Step 9 is “Pick depth” and the plan isn’t written/validated until Steps 10–13; Step 14 is the explicit Review step. Update these references so plan-review is triggered after the correct step(s) (likely after Step 12/13 or at Step 14).
| - rp or codex: run `/flow-code:plan-review` after Step 9, fix issues until it passes | |
| - export: run `/flow-code:plan-review` with export mode after Step 9 | |
| - rp or codex: run `/flow-code:plan-review` at Step 14, fix issues until it passes | |
| - export: run `/flow-code:plan-review` with export mode at Step 14 |
| ``` | ||
|
|
||
| Workers check invariants in Phase 1 (baseline) and Phase 5 (verification). Planners check during Step 1 to ensure tasks don't violate constraints. | ||
| Workers check invariants in Phase 2 (baseline) and Phase 10 (verification). Planners check during Step 4 to ensure tasks don't violate constraints. |
There was a problem hiding this comment.
These phase references for flowctl guard / flowctl invariants check appear inconsistent with the updated worker phase definitions in this PR (where Guard is tied to Phase 6 “Verify & Fix”, and completion is Phase 10). If workers are not actually running guards/invariants in Phase 2 and Phase 10, please update the docs to reference the correct phases (or clarify if an additional baseline run is intended).
| Workers check invariants in Phase 2 (baseline) and Phase 10 (verification). Planners check during Step 4 to ensure tasks don't violate constraints. | |
| Workers check invariants in Phase 6 (Verify & Fix) and again in Phase 10 (completion verification). Planners check during Step 4 to ensure tasks don't violate constraints. |
| **RP context detection (once per wave, before spawning workers):** | ||
|
|
||
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Phase 1.5. | ||
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 6. |
There was a problem hiding this comment.
This sentence says RP context controls whether workers use context_builder in “Worker Phase 6”, but the worker workflow defines RP-powered deep context in the pre-implementation investigation phase (Phase 3). Please update the referenced phase number to match the worker agent instructions so orchestrator config doesn’t mislead.
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 6. | |
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 3. |
| **RP context detection (once per wave, before spawning workers):** | ||
|
|
||
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Phase 1.5. | ||
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 6. |
There was a problem hiding this comment.
This sentence says RP context controls whether workers use context_builder in “Worker Phase 6”, but the worker workflow defines RP-powered deep context in the pre-implementation investigation phase (Phase 3). Please update the referenced phase number to match the worker agent instructions so orchestrator config doesn’t mislead.
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 6. | |
| Detect RP availability and set `RP_CONTEXT` for workers. This controls whether workers use `context_builder` for deep implementation context in Worker Phase 3. |
Summary
Test plan
cargo test --all— 258 passedcargo clippy --all-targets -- -D warnings— cleanflowctl guard— passflowctl validate— passsmoke_test.shrun (has pre-existing failures unrelated to this change)🤖 Generated with Claude Code