diff --git a/agents/plan-sync.md b/agents/plan-sync.md index 63c243c5..110568bf 100644 --- a/agents/plan-sync.md +++ b/agents/plan-sync.md @@ -81,6 +81,7 @@ Look for references to: - Names/APIs from completed task spec (now stale) - Assumptions about data structures - Integration points that changed +- File paths in `## Investigation targets` sections — if the completed task renamed or moved files that are listed as Required/Optional targets in downstream tasks, those paths are now stale Flag tasks that need updates. @@ -130,6 +131,7 @@ Changes should: - Update variable/function names to match actual - Correct API signatures - Fix data structure assumptions +- Update stale file paths in `## Investigation targets` (e.g., if `src/old.ts` was moved to `src/new.ts`) - Add note: `` **DO NOT:** diff --git a/agents/worker.md b/agents/worker.md index f0ed5b88..1f1928af 100644 --- a/agents/worker.md +++ b/agents/worker.md @@ -219,6 +219,40 @@ git diff --stat HEAD 2>/dev/null || true Save `GIT_BASELINE_REV` — you'll use it in Phase 5 to generate workspace change evidence. + +## Phase 1.5: Pre-implementation Investigation + +**If the task spec contains `## Investigation targets` with content, execute this phase. Otherwise skip to Phase 2a/2.** + +1. **Read every Required file** listed before writing any code. Note: + - Patterns to follow (function signatures, naming conventions, structure) + - Constraints discovered (validation rules, type contracts, env requirements) + - Anything surprising that might affect your approach + +2. **Similar functionality search** — before writing new code: + ```bash + # Search for functions/modules that do similar things + # Use terms from the task description + acceptance criteria + grep -r "" --include="*.rs" --include="*.ts" --include="*.py" -l src/ + ``` + If similar functionality exists, pick one: + - **Reuse**: Use the existing code directly + - **Extend**: Modify existing code to support the new case + - **New**: Create new code (justify why existing isn't suitable) + + Report what you found: + ``` + Investigation results: + - Found: `existingHelper()` in src/utils.ts:23 — reusing + - Found: `src/routes/api.ts:45` — following this pattern + - No existing implementation found — creating new + ``` + +3. Read **Optional** files as needed based on Step 1 findings. + +4. Continue to Phase 2a/2 only after investigation is complete. + + ## Phase 2a: TDD Red-Green (if TDD_MODE=true) diff --git a/flowctl/crates/flowctl-cli/src/commands/task/mod.rs b/flowctl/crates/flowctl-cli/src/commands/task/mod.rs index 38d2801e..18f1d27b 100644 --- a/flowctl/crates/flowctl-cli/src/commands/task/mod.rs +++ b/flowctl/crates/flowctl-cli/src/commands/task/mod.rs @@ -57,6 +57,9 @@ pub enum TaskCmd { /// Acceptance section file. #[arg(long, alias = "acceptance")] accept: Option, + /// Investigation targets section file. + #[arg(long)] + investigation: Option, }, /// Reset task to todo. Reset { @@ -171,7 +174,7 @@ fn parse_domain(s: &str) -> Domain { fn create_task_spec(id: &str, title: &str, acceptance: Option<&str>) -> String { let acceptance_content = acceptance.unwrap_or("- [ ] TBD"); format!( - "# {} {}\n\n## Description\nTBD\n\n## Acceptance\n{}\n\n## Done summary\nTBD\n\n## Evidence\n- Commits:\n- Tests:\n- PRs:\n", + "# {} {}\n\n## Description\nTBD\n\n## Investigation targets\n\n## Acceptance\n{}\n\n## Done summary\nTBD\n\n## Evidence\n- Commits:\n- Tests:\n- PRs:\n", id, title, acceptance_content ) } @@ -338,7 +341,8 @@ pub fn dispatch(cmd: &TaskCmd, json: bool) { file, desc, accept, - } => query::cmd_task_set_spec(json, id, file.as_deref(), desc.as_deref(), accept.as_deref()), + investigation, + } => query::cmd_task_set_spec(json, id, file.as_deref(), desc.as_deref(), accept.as_deref(), investigation.as_deref()), TaskCmd::Reset { task_id, cascade } => mutate::cmd_task_reset(json, task_id, *cascade), TaskCmd::Skip { task_id, reason } => mutate::cmd_task_skip(json, task_id, reason.as_deref()), TaskCmd::Split { diff --git a/flowctl/crates/flowctl-cli/src/commands/task/query.rs b/flowctl/crates/flowctl-cli/src/commands/task/query.rs index 75ee9269..16225697 100644 --- a/flowctl/crates/flowctl-cli/src/commands/task/query.rs +++ b/flowctl/crates/flowctl-cli/src/commands/task/query.rs @@ -18,6 +18,7 @@ pub(super) fn cmd_task_set_spec( file: Option<&str>, description: Option<&str>, acceptance: Option<&str>, + investigation: Option<&str>, ) { let flow_dir = ensure_flow_exists(); @@ -28,8 +29,8 @@ pub(super) fn cmd_task_set_spec( )); } - if file.is_none() && description.is_none() && acceptance.is_none() { - error_exit("Requires --file, --description, or --acceptance"); + if file.is_none() && description.is_none() && acceptance.is_none() && investigation.is_none() { + error_exit("Requires --file, --description, --acceptance, or --investigation"); } let mut doc = load_task_doc(&flow_dir, task_id); @@ -67,6 +68,12 @@ pub(super) fn cmd_task_set_spec( sections_updated.push("## Acceptance"); } + if let Some(inv_file) = investigation { + let inv_content = read_file_or_stdin(inv_file); + doc.body = patch_body_section(&doc.body, "## Investigation targets", &inv_content); + sections_updated.push("## Investigation targets"); + } + doc.frontmatter.updated_at = Utc::now(); write_task_doc(&flow_dir, task_id, &doc); diff --git a/flowctl/crates/flowctl-core/src/types.rs b/flowctl/crates/flowctl-core/src/types.rs index d516f38a..3ad8f35d 100644 --- a/flowctl/crates/flowctl-core/src/types.rs +++ b/flowctl/crates/flowctl-core/src/types.rs @@ -305,6 +305,7 @@ impl std::fmt::Display for PhaseStatus { pub const PHASE_DEFS: &[(&str, &str, &str)] = &[ ("0", "Verify Configuration", "OWNED_FILES verified and configuration validated"), ("1", "Re-anchor", "Run flowctl show and verify spec was read"), + ("1.5", "Investigation", "Required investigation target files read and patterns noted"), ("2a", "TDD Red-Green", "Failing tests written and confirmed to fail"), ("2", "Implement", "Feature implemented and code compiles"), ("2.5", "Verify & Fix", "flowctl guard passes and diff reviewed"), @@ -319,9 +320,9 @@ pub const PHASE_DEFS: &[(&str, &str, &str)] = &[ /// Phase sequences by mode (from Python constants.py). /// Phase `5c` (outputs_dump) is NOT in these static sequences — it is added /// dynamically by `worker-phase next` based on the `outputs.enabled` config. -pub const PHASE_SEQ_DEFAULT: &[&str] = &["0", "1", "2", "2.5", "3", "5", "5b", "6"]; -pub const PHASE_SEQ_TDD: &[&str] = &["0", "1", "2a", "2", "2.5", "3", "5", "5b", "6"]; -pub const PHASE_SEQ_REVIEW: &[&str] = &["0", "1", "2", "2.5", "3", "4", "5", "5b", "6"]; +pub const PHASE_SEQ_DEFAULT: &[&str] = &["0", "1", "1.5", "2", "2.5", "3", "5", "5b", "6"]; +pub const PHASE_SEQ_TDD: &[&str] = &["0", "1", "1.5", "2a", "2", "2.5", "3", "5", "5b", "6"]; +pub const PHASE_SEQ_REVIEW: &[&str] = &["0", "1", "1.5", "2", "2.5", "3", "4", "5", "5b", "6"]; // ── Evidence ───────────────────────────────────────────────────────── @@ -551,7 +552,7 @@ mod tests { #[test] fn test_phase_defs_complete() { - assert_eq!(PHASE_DEFS.len(), 11); + assert_eq!(PHASE_DEFS.len(), 12); // Verify all phase sequences reference valid phase IDs let valid_ids: Vec<&str> = PHASE_DEFS.iter().map(|(id, _, _)| *id).collect(); for seq_id in PHASE_SEQ_DEFAULT { diff --git a/skills/flow-code-plan/examples.md b/skills/flow-code-plan/examples.md index b7c08436..bf9d85e4 100644 --- a/skills/flow-code-plan/examples.md +++ b/skills/flow-code-plan/examples.md @@ -332,6 +332,43 @@ flowchart LR --- +## Good vs Bad: Investigation Targets + +### ✅ GOOD: Specific paths with purpose + +```markdown +## Investigation targets +**Required** (read before coding): +- `flowctl/crates/flowctl-cli/src/commands/task/mod.rs:170-177` — existing task spec template to extend +- `flowctl/crates/flowctl-cli/src/commands/task/query.rs:15-82` — `--desc`/`--accept` pattern to follow + +**Optional** (reference as needed): +- `flowctl/crates/flowctl-core/src/types.rs:305-325` — phase registry structure +``` + +**Why it works**: Exact paths with line ranges. Clear Required vs Optional. Brief purpose descriptions. Worker reads these before coding, grounding implementation in real patterns. + +### ❌ BAD: Vague or overloaded targets + +```markdown +## Investigation targets +- `src/` — look at the source code +- `tests/` — check the tests +- `lib/utils.ts` — might be useful +- `src/auth/oauth.ts` +- `src/auth/session.ts` +- `src/auth/middleware.ts` +- `src/auth/types.ts` +- `src/auth/index.ts` +- `src/auth/helpers.ts` +- `src/auth/validators.ts` +- `src/auth/errors.ts` +``` + +**Why it's bad**: No Required/Optional labels. Vague descriptions ("might be useful"). Directory-level paths waste context. Too many targets (11) — worker reads everything and remembers nothing. Max 5-7 targets per task. + +--- + ## Summary | Include in specs | Don't include | @@ -342,3 +379,4 @@ flowchart LR | Recent/surprising APIs | Obvious patterns | | Non-obvious gotchas | Every function body | | Acceptance criteria | Redundant details | +| Investigation targets (Required/Optional) | Vague directory paths | diff --git a/skills/flow-code-plan/steps.md b/skills/flow-code-plan/steps.md index 0188fbeb..81672c96 100644 --- a/skills/flow-code-plan/steps.md +++ b/skills/flow-code-plan/steps.md @@ -282,6 +282,14 @@ Default to standard unless complexity demands more or less. - Follow pattern at `src/example.ts:42` - Reuse `existingHelper()` from `lib/utils.ts` + ## Investigation targets + **Required** (read before coding): + - `src/auth/oauth.ts` — existing OAuth flow to extend + - `src/middleware/session.ts:23-45` — session validation pattern + + **Optional** (reference as needed): + - `src/auth/*.test.ts` — existing test patterns + ## Key context [Only for recent API changes, surprising patterns, or non-obvious gotchas] [If stack config exists, include relevant framework conventions here] @@ -291,6 +299,13 @@ Default to standard unless complexity demands more or less. - [ ] Criterion 2 ``` + **Investigation targets rules:** + - Max 5-7 targets per task — enough to ground the worker, not so many it wastes context + - Use exact file paths with optional line ranges (e.g., `src/auth.ts:23-45`) + - **Required** = must read before implementing. **Optional** = helpful reference + - Auto-populated from repo-scout/context-scout findings in Step 1 research + - If no relevant files found by scouts, leave the section empty (worker skips Phase 1.5) + **Layer field**: If stack config is set, tag each task with its primary layer. This helps the worker select the right guard commands (e.g., `pytest` for backend, `pnpm test` for frontend). Full-stack tasks run all guards. 7. Add task dependencies (if not already set via `--deps`):