diff --git a/.agents/skills/map-efficient/efficient-reference.md b/.agents/skills/map-efficient/efficient-reference.md index 63eb173f..460d9be7 100644 --- a/.agents/skills/map-efficient/efficient-reference.md +++ b/.agents/skills/map-efficient/efficient-reference.md @@ -93,11 +93,34 @@ envelope/anti-gaming checks. ## Wave Execution -Sequential execution is the default. Use wave APIs only when the blueprint has -multiple ready subtasks whose writes are low-risk and disjoint, or when the user -explicitly requests parallel execution. +### Execution strategy decision table -Commands: +`select_execution_strategy` picks between the legacy sequential walker and the +wave-loop on every run. The wave-loop engages **only when ALL THREE hold** +(otherwise the legacy sequential walker `get_next_step` runs): + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, +`worktree.isolation=off`. The isolation gate fails by default, so a stock +`mapify init` config always runs the legacy sequential walker. Even when the +wave-loop engages, dispatch stays sequential until concurrency ships (Slice 5+). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. Do not mix wave +APIs with the sequential cursor for the same workflow. + +### Wave-loop commands ```bash python3 .map/scripts/map_orchestrator.py set_waves --blueprint ".map/${BRANCH}/blueprint.json" @@ -109,6 +132,10 @@ python3 .map/scripts/map_orchestrator.py advance_wave Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. +Use wave APIs only when the blueprint has multiple ready subtasks whose writes +are low-risk and disjoint, or when the user explicitly requests parallel +execution. + When `worktree.isolation` is enabled and a wave runs in parallel (≥2 disjoint subtasks), give each subtask its own worktree and accept the whole wave atomically after all pass Monitor — never merge them one at a time (the first @@ -123,6 +150,43 @@ to base on any conflict or gate failure (worktrees kept for retry). On a single subtask's Monitor failure, `discard_subtask_worktree` that subtask and retry it before calling `merge_wave_worktrees`. +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (dispatching multiple actor subagents in a single turn) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: +> true` / `parallel_ready` flag set**. In the **current framework** +> `concurrency_enabled` is **False**, so dispatch stays **SEQUENTIAL even when a +> wave has `mode=="parallel"`**. The example below is reference material for when +> that capability ships; do NOT treat it as an active instruction now. Use your +> Codex runtime's own parallel actor-subagent dispatch mechanism — this is the +> provider-neutral shape, not a literal API call. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks +dispatches all N actor subagents in **one turn**: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — one turn, N actor subagents: +dispatch actor subagent -> ST-003 (pinned to its own worktree) +dispatch actor subagent -> ST-004 (pinned to its own worktree) + +# INCORRECT — one actor per turn (serial, defeats the wave): +# Turn 1: actor -> ST-003 +# Turn 2: actor -> ST-004 +``` + +**Self-audit before dispatch:** "I will dispatch {n} actor subagents in one turn." + +**`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are +pre-split into sequential batches before dispatch. + +### Anti-patterns + +- One actor dispatch per turn across N turns — serial loop, no concurrency. +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. +- Waiting for one actor result before dispatching the next. +- Mixing `get_next_step` and `get_wave_step` for the same wave. + ## TDD Mode `--tdd` inserts `TEST_WRITER` and `TEST_FAIL_GATE` before `ACTOR`. diff --git a/.claude/rules/learned/architecture-patterns.md b/.claude/rules/learned/architecture-patterns.md index aa4df181..faf3c51a 100644 --- a/.claude/rules/learned/architecture-patterns.md +++ b/.claude/rules/learned/architecture-patterns.md @@ -249,3 +249,17 @@ git commit -m "ST-006: ..." python3 .map/scripts/map_step_runner.py refresh_blueprint_affected_files "$BRANCH" ST-006 ``` + +- **Reserve-Parameter Forward-Compat: Pin a Public Function Signature Early With Unused Params to Prevent Caller-Rewrite Cascades Across Subtasks** (2026-06-29): In a multi-subtask agentic workflow where subtask N defines a function and subtasks N+1..N+K extend it, pin the FINAL intended public signature in subtask N even if some parameters are unused until a later subtask. Suppress unused-parameter warnings with `del param` in the function body (valid in `def`, not `lambda` — see [[del-is-illegal-inside-a-python-lambda-body]]). Without early pinning, each subtask that adds a parameter forces callers and tests from all previous subtasks to be rewritten — a cascade that risks regressions in already-validated outputs and forces Monitor re-validation of untouched subtasks. The unused-param comment must name the subtask that will consume it so Monitor can verify the suppression is intentional; remove the `del` and comment when the parameter becomes live. [workflow: map-efficient] + ```python + # WRONG: add parameters one subtask at a time; each addition cascades to all callers + # ST-006: def lint_dependency_graph(graph): ... + # ST-007: def lint_dependency_graph(graph, affected_files_map, node_io, enforcement, auto_prune): ... # rewrites every ST-006 caller/test + + # CORRECT: pin the final signature in ST-006; del-suppress params consumed later + def lint_dependency_graph(graph, affected_files_map=None, node_io=None, + enforcement="warn", auto_prune=False) -> list[str]: + del affected_files_map, node_io, enforcement, auto_prune # consumed in ST-007 (Layer B) + return _lint_layer_a(graph) + # ST-006 callers/tests already call the full signature; ST-007 just removes the del. + ``` diff --git a/.claude/rules/learned/error-patterns.md b/.claude/rules/learned/error-patterns.md index b077b735..f796cdd4 100644 --- a/.claude/rules/learned/error-patterns.md +++ b/.claude/rules/learned/error-patterns.md @@ -145,3 +145,25 @@ python -c "import pytest; import truststore; import hypothesis" # ImportError here means venv-setup issue, not a code bug -> uv sync --all-extras --all-groups ``` + +- **Decomposer Existence Claims Are Assumptions — Grep Before Dispatching Any 'Create New X' Subtask** (2026-06-29): A task-decomposer or plan that asserts "X does not exist yet" or scopes a subtask to "add X" is making an assumption about the live codebase, not a verified fact. Decomposers work from context summaries and miss existing implementations — especially in mature multi-module codebases. Acting on a false existence claim makes the actor implement a DUPLICATE of an existing component (e.g. duplicating a state machine from the orchestrator into the runner). Protocol: before dispatching any actor on a "create new function/class/module" subtask, grep the relevant source trees for the symbol; if it exists, fix the subtask scope before dispatch. A wrong existence claim that goes unverified propagates into duplicated machinery across all dependent subtasks. [workflow: map-efficient] + ```bash + # Plan: 'ST-010: add get_wave_step/validate_wave_step/advance_wave to runner — do not exist yet.' + # WRONG: trust the claim -> actor creates a duplicate state machine + # CORRECT: grep before dispatch + grep -r 'get_wave_step\|validate_wave_step\|advance_wave' src/mapify_cli/templates_src/ + # -> map_orchestrator.py.jinja already defines them: claim is FALSE. + # Re-scope ST-010 to GATE the existing orchestrator functions, not recreate them. + # Any subtask verb 'add'/'create'/'implement new' requires a pre-dispatch grep. + ``` + +- **Targeted Per-File Checks During a Subtask Are Not a Substitute for the Full Aggregate Lint Gate** (2026-06-29): Running `pyright ` / `mypy ` during a subtask is faster but has two systematic blind spots vs the project's aggregate gate (`make check` / `ruff check src tests`): (1) SCOPE — targeted checks run on one file; ruff F401/F811/E501 run across all of `src/` and `tests/`, surfacing unused imports introduced in any file touched this subtask; (2) RULE SET — pyright enforces type correctness; ruff enforces lint rules pyright ignores (unused imports, line length). A file that passes targeted pyright can still fail ruff at the aggregate level (it did: `sys`/`Any` F401 surfaced only at the final `make check`). Run the project's aggregate gate as part of each code subtask's verification step, not only at the final gate; per-file targeted checks are for fast iteration, not the commit-before-monitor gate. [workflow: map-efficient] + ```bash + # WRONG: per-subtask verification uses only a targeted single-file type-check + pyright src/mapify_cli/parallelism_observability.py # passes; ruff F401 not checked + # CORRECT: run the aggregate gate the CI runs, at each code subtask: + ruff check src/ tests/ # ALL files, ALL rules incl. F401 + python -m pyright src/ # ALL source files + pytest tests/ -x -q + # If make check fails on F401 after per-file pyright passed -> scope mismatch. + ``` diff --git a/.claude/rules/learned/implementation-patterns.md b/.claude/rules/learned/implementation-patterns.md index 26ed4fc0..f18061ea 100644 --- a/.claude/rules/learned/implementation-patterns.md +++ b/.claude/rules/learned/implementation-patterns.md @@ -179,3 +179,30 @@ paths: from typing import Any def validate(bp) -> dict[str, Any]: ... # 34 downstream errors cleared by 1 change site ``` + +- **Behavior-Neutral Foundation: Gate Every New Path Behind a Flag That Defaults to Current Behavior, Plus Add a Default-Config Proof Test** (2026-06-29): When shipping a foundation subtask for a risky feature (parallel execution, new state machine, schema migration), every new code path must be predicate-gated behind a config key or compile-time constant that defaults to the existing behavior. A `WAVE_CONCURRENCY_ENABLED=False` constant (or equivalent) prevents any new dispatch path from being exercised by default. The proof is NOT "the tests still pass" — it is a dedicated test that loads the default (empty) config and asserts the old/sequential path is selected. Without this test, a subtle config-default bug can silently activate the new path on upgrade in every consumer repo. Ship the flag, ship the test, then the foundation can land without a soak period. [workflow: map-efficient] + ```python + # WRONG: new path always reachable; no behavioral gate + def select_wave_loop(config: MapConfig) -> WaveLoop: + if config.execution_wave_mode == 'parallel': + return ParallelWaveLoop(config) # reachable if user misconfigures + return SequentialWaveLoop(config) + + # CORRECT: compile-time kill-switch + proof test + WAVE_CONCURRENCY_ENABLED = False # PR-level constant; flip only when feature is ready + def select_wave_loop(config: MapConfig) -> WaveLoop: + if not WAVE_CONCURRENCY_ENABLED or config.execution_wave_mode != 'parallel': + return SequentialWaveLoop(config) # old path, guaranteed in this PR + return ParallelWaveLoop(config) + + def test_default_config_selects_sequential_loop(tmp_path): + cfg = load_map_config(tmp_path / '.map' / 'config.yaml') # no file -> defaults + assert isinstance(select_wave_loop(cfg), SequentialWaveLoop) + ``` + +- **Shared .jinja Templates Rendering Into Multiple Provider Trees Must Not Contain Provider-Specific API Tokens in the Codex Variant** (2026-06-29): When a `.jinja` template in `templates_src/` renders into BOTH a Claude-family tree (`.claude/skills/`) AND a Codex/agents-family tree (`.agents/skills/`, `templates/codex/`), provider-specific Claude API identifiers — `subagent_type=`, `Agent(`, `AskUserQuestion(`, `Task(` — must not appear in the codex-rendered output. A CI test (`test_ac10_no_claude_refs_anywhere`) enforces this and hard-fails `make check`. The leak is easy to introduce when editing doc examples in a shared `.jinja` (you write from the Claude perspective). Fix: after `make render-templates`, grep the codex output paths for forbidden tokens; if found, use provider-neutral prose or a jinja conditional. Do not rely on the CI gate as first-line detection — grep after rendering, before commit. [workflow: map-efficient] + ```bash + make render-templates + grep -r 'subagent_type=\|AskUserQuestion\|Agent(\|Task(' .agents/skills/ src/mapify_cli/templates/codex/ + # must be empty; otherwise use provider-neutral phrasing or {% if provider == 'claude' %}...{% else %}...{% endif %} + ``` diff --git a/.claude/skills/map-efficient/SKILL.md b/.claude/skills/map-efficient/SKILL.md index e4abc02f..7e1da6f5 100644 --- a/.claude/skills/map-efficient/SKILL.md +++ b/.claude/skills/map-efficient/SKILL.md @@ -20,11 +20,11 @@ Use [efficient-reference.md](efficient-reference.md) for wave examples, TDD deta ```yaml thinking_policy: medium/adaptive -parallel_tool_policy: guarded_wave_only +parallel_tool_policy: wave_mode_aware ``` - Use deeper reasoning only when a subtask is risky, blocked, under-specified, or repeatedly failing Monitor. -- Keep execution sequential by default. Parallel waves are allowed only under the existing wave rules: all dependencies satisfied, low risk, disjoint new-file writes, and the wave API is used. +- Execution strategy is determined by `select_execution_strategy`: the wave-loop engages only when `execution.wave_mode` is `on` or `auto` AND a color group has ≥2 members; otherwise the legacy sequential walker runs. With default config this is the sequential walker. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and dispatch mechanics. - Do not parallelize state transitions, Monitor retries for the same subtask, or writes to shared branch artifacts. ## Execution Rules @@ -32,7 +32,7 @@ parallel_tool_policy: guarded_wave_only 1. Execute the next state-machine step only; never skip phases. 2. Use the exact agent type for the current phase. 3. Max 5 retry iterations per subtask. -4. Batch mode is default. Sequential subtask execution is default. +4. Batch mode is default. Execution strategy is selected by `select_execution_strategy` (sequential walker by default; wave-loop only when wave_mode is enabled and a color group has ≥2 members). 5. After Monitor pass, record files changed in `step_state.json` for guard isolation. 6. Validate planning metadata before Actor starts: `expected_diff_size`, `concern_type`, `one_logical_step`, `split_rationale`, `concern_justification`, `coverage_map`, `hard_constraints`, `soft_constraints`, `validation_criteria`, `[AC-1]` bracket tags, and `tradeoff_rationale`. 7. Script routing: `map_orchestrator.py` owns state-machine transitions (`get_next_step`, `validate_step`, `monitor_failed`, `record_subtask_result`, `set_waves`, `resume_from_plan`, …); `map_step_runner.py` owns every `detect_*` / `build_*` / `save_*` / `load_*` / `refresh_*` / `log_*` helper plus baseline `record_*` and artifact writers. Full table + the `record_*` / `validate_*` disambiguation in [efficient-reference.md#script-routing-dispatcher-reference](efficient-reference.md#script-routing-dispatcher-reference). @@ -207,23 +207,13 @@ else fi ``` -Default to sequential execution. Use wave APIs only for low-risk disjoint new-file waves or explicit user-requested parallel execution. See [efficient-reference.md](efficient-reference.md#wave-execution) for the full wave loop. +**Execution strategy:** `select_execution_strategy` chooses between the legacy sequential walker and the wave-loop. The wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) engages only when `execution.wave_mode ∈ {on, auto}` AND a color group has ≥2 members; otherwise `get_next_step` (sequential walker) runs. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and full wave loop. **Note on resume:** `resume_from_plan` (Step 0) now auto-invokes `set_waves` when `blueprint.json` is present, so resumed workflows do not need a manual `set_waves` dispatch. The result is reported in the `waves_computed` field of the resume response (`"success"`, `"error"`, or `"skipped"` if no blueprint). -**Wave-loop vs sequential dispatcher:** `get_next_step` is the **sequential** -walker (one phase at a time, in `subtask_sequence` order). The **wave-loop** -(`get_wave_step` / `validate_wave_step` / `advance_wave`) honors -`execution_waves` and is the canonical path when waves contain >1 subtask. -For a single-Actor batch run with a fully-linear plan, `get_next_step` and -the wave-loop converge on the same order, so the skill defaults to the -sequential walker for simplicity. Switch to the wave-loop when (a) waves -have ≥2 subtasks AND (b) the subtasks in that wave touch disjoint files -(see `split_wave_by_file_conflicts`). - ### No-op subtask short-circuit (before RESEARCH) Some subtasks are already-done historically (rename/refactor landed in a prior PR), or truly do not need Actor/Monitor because the requested work is already satisfied by repo state. Skip them up-front to save tokens: diff --git a/.claude/skills/map-efficient/efficient-reference.md b/.claude/skills/map-efficient/efficient-reference.md index 49620993..76b9554a 100644 --- a/.claude/skills/map-efficient/efficient-reference.md +++ b/.claude/skills/map-efficient/efficient-reference.md @@ -121,7 +121,90 @@ including clean passes — must carry concrete evidence references. ## Wave Execution -Sequential is default. Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. Use `get_wave_step`, `validate_wave_step`, and `advance_wave`; do not mix wave APIs with the single-current-subtask API. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. +### Execution strategy decision table + +`select_execution_strategy` picks between the legacy sequential walker and the wave-loop on every run. The wave-loop engages **only when ALL THREE hold**; otherwise the legacy sequential walker (`get_next_step`) runs: + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, `worktree.isolation=off`. Because the isolation gate (#2) fails by default, a stock `mapify init` config always runs the legacy sequential walker — byte-identical to pre-Slice-3. Even when the wave-loop does engage, dispatch stays sequential until concurrency ships (Slice 5+, `concurrency_enabled=False`). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. One phase at a time, in `subtask_sequence` order. Do not mix wave APIs with the sequential cursor for the same workflow. + +### Wave-loop + +Use `get_wave_step`, `validate_wave_step`, and `advance_wave` when the wave-loop is active. Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. + +Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. + +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (emitting multiple `Task(actor)` calls in a single message) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: true` / +> `parallel_ready` flag set**. In the **current framework** `concurrency_enabled` is +> **False**, so dispatch stays **SEQUENTIAL even when a wave has `mode=="parallel"`**. +> The example below is reference material for when that capability ships; do NOT +> treat it as an active instruction now. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks dispatches all N Actors in **one message** with N `Task` calls — not one per turn: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — N Task calls in one message: +Task( + subagent_type="actor", + description="Implement ST-003", + prompt="..." +) +Task( + subagent_type="actor", + description="Implement ST-004", + prompt="..." +) +``` + +```text +# INCORRECT — one Task per turn (sequential, defeats the wave): +# Turn 1: Task(actor, ST-003) +# Turn 2: Task(actor, ST-004) ← serial, not concurrent +``` + +**Self-audit before dispatch:** "I will emit {n} Task(actor) calls in one message for this wave." Confirm n matches the color group size. + +**`max_actors` cap:** Default 4–8 concurrent actors per wave. Groups larger than `max_actors` are pre-split into sequential batches of `max_actors` before dispatch; do not emit more than `max_actors` Task calls in a single message. + +### Anti-patterns (wave execution) + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. +- **Waiting for one actor result before dispatching the next** — correct for sequential, wrong for concurrent waves. +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. + +### Actor-boundary prompt template (worktree-isolated subtasks) + +When a subtask runs in its own worktree, prefix the Actor prompt with: + +```text +You are working inside the isolated worktree for {SUBTASK_ID}. +Worktree path: {WT_PATH} +Frozen base SHA: {BASE_SHA} + +HARD CONSTRAINTS: +- Write ONLY inside {WT_PATH}. Never touch the main tree or sibling worktrees. +- Do not commit directly. Your output is merged by merge_subtask_worktree / merge_wave_worktrees. +- On completion, return JSON: {"subtask_id": "{SUBTASK_ID}", "files_changed": [...], "tests_run": [...], "validation_notes": "...", "blocker": null} +``` ## Predictor Recovery diff --git a/.map/scripts/map_orchestrator.py b/.map/scripts/map_orchestrator.py index d24ee45b..7d02035b 100755 --- a/.map/scripts/map_orchestrator.py +++ b/.map/scripts/map_orchestrator.py @@ -247,6 +247,10 @@ def _extract_subtask_ids_from_plan_artifacts( AGGRESSIVE_COMPRESSION_MULTIPLIER = 0.4 +# Slice 3: sequential-inside wave-loop. Slice 5 flips this to True when +# concurrent Task dispatch is actually implemented and safe to enable. +WAVE_CONCURRENCY_ENABLED = False + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2300,6 +2304,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": 0, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, "message": "No execution waves configured. Use sequential mode.", } @@ -2309,6 +2314,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } wave = state.execution_waves[state.current_wave_index] @@ -2345,6 +2351,9 @@ def get_wave_step(branch: str) -> dict: "wave_total": len(state.execution_waves), "subtasks": subtask_infos, "is_complete": False, + # concurrency_enabled=False: even when mode=="parallel" (width>=2 wave), + # dispatch is strictly sequential this slice. Slice 5 flips WAVE_CONCURRENCY_ENABLED. + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } @@ -2440,6 +2449,73 @@ def advance_wave(branch: str) -> dict: } +def select_execution_strategy( + branch: str, project_dir: Optional[Path] = None +) -> dict: + """Determine whether to use wave_loop or legacy sequential walker. + + Predicate: wave_loop IFF wave_mode in {on, auto} AND worktree.isolation != 'off' + AND any color-group has width >= 2. This mirrors the canonical MapConfig gating + (#305): execution_wave_mode defaults to 'auto' but the wave-loop stays dormant + because worktree.isolation defaults to 'off', so default config always returns + 'sequential' and the legacy get_next_step path is byte-identical (HC-1). + + Args: + branch: Git branch name (sanitized) + project_dir: Project root containing .map/config.yaml. + Defaults to Path('.') consistent with other helpers. + + Returns: + { + "strategy": "wave_loop" | "sequential", + "wave_mode": "off" | "auto" | "on", + "worktree_isolation": "off" | "auto" | "required", + "has_parallel_groups": bool, + "reason": str, + } + """ + if project_dir is None: + project_dir = Path(".") + + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _execution_wave_mode, + _worktree_isolation_mode, + ) + wave_mode = _execution_wave_mode(project_dir) + isolation_mode = _worktree_isolation_mode(project_dir) + except ImportError: + wave_mode = "off" + isolation_mode = "off" + + state_file = Path(f".map/{branch}/step_state.json") + state = StepState.load(state_file) + has_parallel_groups = any(len(g) >= 2 for g in state.execution_waves) + + if wave_mode in {"on", "auto"} and isolation_mode != "off" and has_parallel_groups: + strategy = "wave_loop" + reason = ( + f"wave_mode={wave_mode!r}, worktree.isolation={isolation_mode!r}, " + "and execution_waves has width>=2 group" + ) + else: + if wave_mode not in {"on", "auto"}: + reason = f"wave_mode={wave_mode!r} (not on/auto) → legacy sequential" + elif isolation_mode == "off": + reason = "worktree.isolation='off' → legacy sequential (no isolation, no parallel)" + else: + reason = "no color-group with width>=2 → sequential (all width-1 waves)" + strategy = "sequential" + + return { + "strategy": strategy, + "wave_mode": wave_mode, + "worktree_isolation": isolation_mode, + "has_parallel_groups": has_parallel_groups, + "reason": reason, + } + + def _write_feedback_file( branch: str, filename: str, header: str, feedback: str ) -> Optional[str]: diff --git a/.map/scripts/map_step_runner.py b/.map/scripts/map_step_runner.py index 2ba7e9d5..4188c256 100755 --- a/.map/scripts/map_step_runner.py +++ b/.map/scripts/map_step_runner.py @@ -146,6 +146,22 @@ def _map_config_int(project_dir: Path, key: str, default: int) -> int: return parsed if parsed > 0 else default +_WAVE_MODE_VALID = frozenset({"off", "auto", "on"}) + + +def _execution_wave_mode(project_dir: Path) -> str: + """Return the execution.wave_mode setting: 'off' | 'auto' | 'on'. + + Default + enum mirror the canonical MapConfig schema (config/project_config.py): + absent/unknown/garbage normalises to 'auto'. 'auto' is still behavior-neutral + by default because the wave-loop only engages when worktree.isolation != 'off' + (which itself defaults to 'off') AND a color group has >=2 members — see + select_execution_strategy. Never raises. + """ + raw = _map_config_str(project_dir, "execution.wave_mode", "auto") + return raw if raw in _WAVE_MODE_VALID else "auto" + + def _extract_transcript_usage(entry: dict) -> Optional[int]: message = entry.get("message") if not isinstance(message, dict): @@ -15321,6 +15337,34 @@ def _wt_force_remove(path: Path, branch_ref: str) -> None: _wt_git(["branch", "-D", branch_ref]) +# Stable reason codes shared by resolve_worktree_isolation and ST-011 observability. +_WT_REASON_NOT_GIT_REPO: str = "not_git_repo" +_WT_REASON_UNSUPPORTED: str = "worktree_unsupported" +_WT_REASON_CREATE_FAILED: str = "worktree_create_failed" +_WT_REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" + +_WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) + + +def _worktree_isolation_mode(project_dir: Path) -> str: + """Return the worktree.isolation setting: 'off' | 'auto' | 'required'. + + Accepts the new enum strings directly (case-insensitive). + Legacy boolean compat: boolish-truthy (true/1/yes) → 'required'; + boolish-false (false/0/no/'') → 'off'. + Absent key → 'off' (today's behavior). Any unknown/garbage → 'off'. + Never raises. + """ + raw = _map_config_str(project_dir, "worktree.isolation", "") + normalized = raw.strip().lower() + if normalized in _WT_ISOLATION_VALID: + return normalized + # Legacy boolean compat + if _parse_boolish(normalized): + return "required" + return "off" + + def _wt_isolation_enabled(project_dir: Path) -> bool: """Return True when worktree isolation is active for the current project. @@ -15332,6 +15376,11 @@ def _wt_isolation_enabled(project_dir: Path) -> bool: lands in Slice 5; the call site in create_subtask_worktree already degrades to sequential when this returns False) - ``required`` -> True (hard-fail on unavailability, same as old ``true``) + + Canonical enum vocabulary + default live in MapConfig + (config/project_config.py). `_worktree_isolation_mode` above mirrors the + same `worktree.isolation` key for probe/fallback paths that need the full + enum value (auto vs required), not just the enabled boolean. """ val = _map_config_str(project_dir, "worktree.isolation", "off").strip().lower() return val in {"required", "true", "yes", "y", "1", "on"} @@ -15346,6 +15395,41 @@ def _wt_max_deletions(project_dir: Path) -> int: return n if n >= 0 else 50 +_LINT_ENFORCEMENT_VALID = frozenset({"off", "warn", "repair_once", "strict"}) + + +def _lint_dependency_enforcement(project_dir: Path) -> str: + """Return the lint.dependency_enforcement setting. + + Accepted values: 'off' | 'warn' | 'repair_once' | 'strict'. + Absent key or unknown/garbage value → 'warn' (today's no-op default). + Never raises. + """ + raw = _map_config_str(project_dir, "lint.dependency_enforcement", "warn") + normalized = raw.strip().lower() + return normalized if normalized in _LINT_ENFORCEMENT_VALID else "warn" + + +def _lint_auto_prune(project_dir: Path) -> bool: + """Return the lint.auto_prune setting (default False). + + Absent key → False (no mutation today). Never raises. + """ + raw = _map_config_str(project_dir, "lint.auto_prune", "false") + return _parse_boolish(raw) + + +def _observability_parallelism_enabled(project_dir: Path) -> bool: + """Return the observability.parallelism setting (default False). + + This is the dormant no-op gate that ST-011's parallelism.json writer + will check. Absent key → False (no observability writes today). + Never raises. + """ + raw = _map_config_str(project_dir, "observability.parallelism", "false") + return _parse_boolish(raw) + + def _wt_config_verification_checks(project_dir: Path) -> list[str]: """Read the `verification_checks` LIST from .map/config.yaml (lazy yaml).""" config_path = project_dir / ".map" / "config.yaml" @@ -15412,6 +15496,322 @@ def _wt_parse_name_status(text: str) -> tuple[list[str], list[str], int]: return deleted, runtime, changed +# --------------------------------------------------------------------------- +# Worktree probe (Slice 2 / ST-008) +# --------------------------------------------------------------------------- +# Module-level cache: key = resolved toplevel path, value = probe result dict. +# Reset between test runs via _WORKTREE_PROBE_CACHE.clear(). +_WORKTREE_PROBE_CACHE: dict[str, dict[str, object]] = {} + + +def _worktree_probe(project_dir: Path) -> dict[str, object]: + """Safe detached worktree probe. Cached per session keyed on toplevel. + + HC-1 dormancy contract: when worktree.isolation is 'off' (the default), + this function returns immediately WITHOUT running any git command. + + When mode is 'auto' or 'required': + * Resolves the storage root via _wt_storage_root() (never .map/worktrees/). + * Adds a detached probe worktree at /.probe- to test + `git worktree add --detach` support. + * Always removes the probe (force + prune) in a try/finally so no probe + leaks even on exception. + * Verifies we are in the primary checkout by comparing _wt_toplevel() + against the first `worktree` line from `git worktree list --porcelain`. + * Returns {"status":"ok","ok":True,"supported":True,"is_primary":bool} on + success, or _wt_error("WORKTREE_PROBE_FAILED", ...) on any failure. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Check the session cache (keyed on resolved toplevel, falls back to str(project_dir)) + toplevel = _wt_toplevel() + cache_key = str(toplevel) if toplevel is not None else str(project_dir.resolve()) + if cache_key in _WORKTREE_PROBE_CACHE: + return _WORKTREE_PROBE_CACHE[cache_key] + + # Verify primary checkout + is_primary = False + if toplevel is not None: + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + primary_path = Path(line[len("worktree "):].strip()).resolve() + is_primary = primary_path == toplevel + break # first entry is always the main checkout + + storage = _wt_storage_root() + if storage is None: + # Not cached: only successful probes are memoized so a transient + # failure never becomes a permanent session verdict. + return _wt_error( + "WORKTREE_PROBE_FAILED", + "could not resolve git common dir for probe storage", + ) + + import os as _os # noqa: PLC0415 — local to keep module-level clean + + probe_path = storage / f".probe-{_os.getpid()}" + try: + storage.mkdir(parents=True, exist_ok=True) + add = _wt_git( + ["worktree", "add", "--detach", str(probe_path), "HEAD"], + timeout=30, + ) + if add.returncode != 0: + # Transient create failure — do NOT cache; let auto/required retry. + return _wt_error( + "WORKTREE_PROBE_FAILED", + add.stderr.strip() or "git worktree add --detach failed", + ) + result = { + "status": "ok", + "ok": True, + "supported": True, + "is_primary": is_primary, + } + except Exception as exc: + result = _wt_error("WORKTREE_PROBE_FAILED", str(exc)) + finally: + _wt_git(["worktree", "remove", "--force", str(probe_path)], timeout=30) + _wt_git(["worktree", "prune"], timeout=15) + + # Memoize ONLY successful probes; a transient failure must not stick for the + # session (auto can recover, required stops aborting once the repo is fixed). + if result.get("ok"): + _WORKTREE_PROBE_CACHE[cache_key] = result + return result + + +def _require_clean_merge_target(project_dir: Path) -> dict[str, object]: + """Check that the main checkout is clean enough for a worktree merge. + + Dormant when worktree.isolation is 'off' (default — HC-1). + When the check is active AND require_clean_merge_target config is True + (default True), runs `git status --porcelain` and returns: + * {"status":"ok","ok":True} when clean (excluding MAP runtime state) + * {"status":"dirty","ok":False,"kind":"DIRTY_MERGE_TARGET","dirty":[...]} + when uncommitted changes are present. + Never raises. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Read the require_clean_merge_target flag (default True) + raw_require = _map_config_str(project_dir, "require_clean_merge_target", "true") + if not _parse_boolish(raw_require): + return {"status": "ok", "ok": True, "skipped": True} + + st = _wt_git(["status", "--porcelain"], timeout=15) + if st.returncode != 0: + return _wt_error("CLEAN_CHECK_FAILED", st.stderr.strip() or "git status failed") + + dirty = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty: + return { + "status": "dirty", + "ok": False, + "kind": "DIRTY_MERGE_TARGET", + "reason": "main checkout has uncommitted changes", + "dirty": dirty[:20], + } + return {"status": "ok", "ok": True} + + +# --------------------------------------------------------------------------- +# Slice 2 / ST-009: fallback matrix + orphan cleanup +# --------------------------------------------------------------------------- + + +def resolve_worktree_isolation(project_dir: Path) -> dict[str, object]: + """Classify the current environment and decide the execution decision. + + HC-1 dormancy: when isolation is 'off' (the default), returns immediately + without running any git command. + + Return schema + ------------- + Dormant (off): + {"status": "dormant", "ok": False, "mode": "off", "decision": "sequential"} + Success (auto or required, all checks pass): + {"ok": True, "decision": "isolated", "degraded": False, "mode": } + Degraded (auto only, any fallback condition): + {"ok": True, "decision": "sequential", "degraded": True, + "reason": , "warning": } + Hard failure (required, any fallback condition): + {"status": "error", "ok": False, "kind": , ...} # from _wt_error + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "decision": "sequential", + } + + # --- determine fallback condition (if any) --- + + # 1. Not a git repo at all + if not _wt_is_git_repo(): + reason_code = _WT_REASON_NOT_GIT_REPO + reason_msg = "not inside a git work tree; worktree isolation requires git" + else: + # 2. Worktree support probe + probe = _worktree_probe(project_dir) + if not probe.get("ok"): + # Probe failed → classify as unsupported or create-failed + # WORKTREE_PROBE_FAILED maps to unsupported (the git worktree add step + # is the minimum bar; any probe failure means we cannot create worktrees) + reason_code = _WT_REASON_UNSUPPORTED + reason_msg = str(probe.get("message", "worktree probe failed")) + else: + # 3. Dirty merge target + clean = _require_clean_merge_target(project_dir) + if not clean.get("ok") and clean.get("status") != "dormant": + reason_code = _WT_REASON_DIRTY_MERGE_TARGET + reason_msg = ( + "main checkout has uncommitted changes; worktree isolation " + "requires a clean merge target. Commit or stash first." + ) + else: + reason_code = "" + reason_msg = "" + + if reason_code: + if mode == "auto": + # Degrade gracefully: warn and fall back to sequential + warning = ( + f"[MAP] WARNING: worktree isolation degraded to sequential — " + f"{reason_msg} (reason={reason_code})" + ) + return { + "ok": True, + "decision": "sequential", + "degraded": True, + "reason": reason_code, + "warning": warning, + } + else: # mode == "required" + return _wt_error( + reason_code.upper(), + reason_msg, + decision="abort", + ) + + return { + "ok": True, + "decision": "isolated", + "degraded": False, + "mode": mode, + } + + +def cleanup_orphan_worktrees(branch: str) -> dict[str, object]: + """Remove worktrees present in storage/git-list but NOT in the active registry. + + HC-1 dormancy: when isolation is 'off', returns immediately without + running any git command. + + Idempotent + crash-safe: a second call removes nothing and does not error. + NEVER removes a worktree recorded as active in _read_worktree_state(branch). + + Returns {"removed": [...paths], "kept_active": [...paths], "ok": True} + """ + # HC-1 dormancy: read isolation mode WITHOUT calling git. + # _map_config_str searches for .map/config.yaml starting from cwd upward, so + # we pass Path(".") to avoid _wt_project_dir() -> _wt_toplevel() -> _wt_git(). + project_dir = Path(".") + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "removed": [], + "kept_active": [], + } + + # Build the set of active (registered) worktree paths from state + state = _read_worktree_state(branch) + worktrees_dict = state.get("worktrees", {}) + if not isinstance(worktrees_dict, dict): + worktrees_dict = {} + active_paths: set[str] = { + str(Path(str(rec.get("path", ""))).resolve()) + for rec in worktrees_dict.values() + if isinstance(rec, dict) and rec.get("path") + } + + # Enumerate candidates from storage root + git worktree list + candidate_paths: set[Path] = set() + + storage = _wt_storage_root() + if storage is not None and storage.exists(): + try: + for entry in storage.iterdir(): + if entry.is_dir() and not entry.name.startswith(".probe-"): + # Recurse one level: storage// + for sub in entry.iterdir(): + if sub.is_dir(): + candidate_paths.add(sub.resolve()) + except OSError: + pass + + # Also enumerate from `git worktree list --porcelain` + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + wt_raw = line[len("worktree "):] + wt_path = Path(wt_raw.strip()) + try: + wt_path = wt_path.resolve() + except OSError: + pass + # Include only map-managed worktrees: real resolved-path ancestry + # under the storage root, NOT a substring match (a substring check + # can misclassify e.g. `/worktrees-backup/...` as managed and + # destroy an unrelated worktree). + if storage is not None: + try: + if wt_path.is_relative_to(storage.resolve()): + candidate_paths.add(wt_path) + except (OSError, ValueError): + pass + + removed: list[str] = [] + kept_active: list[str] = [] + + for candidate in sorted(candidate_paths): + candidate_str = str(candidate) + if candidate_str in active_paths: + kept_active.append(candidate_str) + continue + # Orphan — remove it. Derive the expected branch ref name from the + # directory name (best-effort; _wt_force_remove handles branch-not-found + # by ignoring the branch -D error). + orphan_branch_ref = f"{WORKTREE_BRANCH_PREFIX}{candidate.name}" + try: + _wt_force_remove(candidate, orphan_branch_ref) + removed.append(candidate_str) + except Exception: + # Non-fatal: log and continue. A failed removal is not an error + # for the caller — next call will retry. + pass + + return {"removed": removed, "kept_active": kept_active, "ok": True} + + def create_subtask_worktree( subtask_id: str, attempt: int = 0, diff --git a/src/mapify_cli/dependency_graph.py b/src/mapify_cli/dependency_graph.py index a4b17dc2..d80fef09 100644 --- a/src/mapify_cli/dependency_graph.py +++ b/src/mapify_cli/dependency_graph.py @@ -21,10 +21,30 @@ """ from dataclasses import dataclass, field -from typing import List, Dict, Set, Optional +from typing import Any, List, Dict, Set, Optional, Tuple from collections import deque +@dataclass(frozen=True) +class LintFinding: + """ + A single finding from lint_dependency_graph. + + Attributes: + severity: "error" | "warning" | "info" + code: Machine-readable finding code (e.g. "self_loop", "cycle") + message: Human-readable description + subtask_id: The subtask this finding relates to, if applicable + edge: The (from, to) edge this finding relates to, if applicable + """ + + severity: str + code: str + message: str + subtask_id: Optional[str] = None + edge: Optional[Tuple[str, str]] = None + + @dataclass class SubtaskNode: """ @@ -461,3 +481,297 @@ def size(self) -> int: Performance: O(1) """ return len(self.nodes) + + +_SOFT_PHRASES = ( + "logical ordering", + "do this first", + "natural order", + "for safety", +) + + +def soft_phrase_findings( + justifications: Dict[Tuple[str, str], str], +) -> List[LintFinding]: + """ + Flag edge justifications containing soft/vague ordering phrases. + + Args: + justifications: mapping of (from_id, to_id) -> justification text + + Returns: + LintFinding list with severity 'info' or 'warning' for flagged edges. + """ + results: List[LintFinding] = [] + for edge, text in justifications.items(): + lower = text.lower() + for phrase in _SOFT_PHRASES: + if phrase in lower: + results.append( + LintFinding( + severity="warning", + code="soft_phrase", + message=( + f"Edge {edge[0]!r} -> {edge[1]!r} justification contains " + f"vague ordering phrase {phrase!r}: {text!r}" + ), + edge=edge, + ) + ) + break # one finding per edge + return results + + +def _transitive_deps(graph: "DependencyGraph", node_id: str) -> Set[str]: + """Return set of all nodes reachable from node_id via dependency edges (BFS).""" + visited: Set[str] = set() + queue = list(graph.nodes.get(node_id, SubtaskNode(id=node_id)).dependencies) + while queue: + dep = queue.pop() + if dep in visited or dep not in graph.nodes: + continue + visited.add(dep) + queue.extend(graph.nodes[dep].dependencies) + return visited + + +def lint_dependency_graph( + graph: "DependencyGraph", + *, + affected_files_map: Dict[str, Set[str]] | None = None, + node_io: Dict[str, Dict[str, Any]] | None = None, + enforcement: str = "warn", + auto_prune: bool = False, +) -> List[LintFinding]: + """ + Lint a DependencyGraph and return structured findings. + + Layer A (always on, severity='error', regardless of enforcement): + - self_loop : a subtask lists its own id in dependencies + - cycle : a real cycle among >=2 distinct nodes + - unknown_dep : a dependency id not present in graph.nodes + - duplicate_edge : the same dependency id listed more than once + + Layer B (warn-only; skipped when enforcement=='off'): + - thin_edge : edge A->B with empty io-overlap AND empty file-overlap (warning) + - same_file_coloring : two subtasks in same wave share a file (info) + - fully_serialized : N>=4 nodes and max wave width==1 (warning) + - redundant_edge : A->B where A is transitively reachable via other deps of B (info) + - soft_phrase : justification text contains vague ordering phrases (warning) + + auto_prune performs NO mutation in this slice (warn-only). + + Args: + graph: DependencyGraph to lint + affected_files_map: (Layer B) subtask_id -> set of affected file paths + node_io: (Layer B) subtask_id -> {"inputs": set, "outputs": set, + optionally "dep_justifications": {dep_id: text}} + enforcement: "off" suppresses Layer B; "warn"/"repair_once"/"strict" emit Layer B + auto_prune: accepted but performs no mutation in this slice + + Returns: + List of LintFinding instances. + """ + # auto_prune is accepted but intentionally performs no mutation in this slice. + del auto_prune + + findings: List[LintFinding] = [] + + # --- Layer A: hard errors, always on --- + + # Pass 1: self-loops and duplicate edges (per-node, no graph-wide traversal needed) + self_loop_nodes: Set[str] = set() + for node_id, node in graph.nodes.items(): + seen: Set[str] = set() + for dep in node.dependencies: + if dep == node_id: + # Report self-loop once per node even if listed multiple times + if node_id not in self_loop_nodes: + self_loop_nodes.add(node_id) + findings.append( + LintFinding( + severity="error", + code="self_loop", + message=f"Subtask '{node_id}' lists itself as a dependency.", + subtask_id=node_id, + edge=(node_id, node_id), + ) + ) + elif dep in seen: + findings.append( + LintFinding( + severity="error", + code="duplicate_edge", + message=( + f"Subtask '{node_id}' lists dependency '{dep}' more than once." + ), + subtask_id=node_id, + edge=(node_id, dep), + ) + ) + else: + seen.add(dep) + + # unknown_dep check (applies to self-loop deps too, but self-loop is primary) + if dep != node_id and dep not in graph.nodes: + findings.append( + LintFinding( + severity="error", + code="unknown_dep", + message=( + f"Subtask '{node_id}' depends on '{dep}' which is not in the graph." + ), + subtask_id=node_id, + edge=(node_id, dep), + ) + ) + + # Pass 2: cycle detection on a self-loop-free view to avoid double-reporting. + # Build a temporary graph excluding self-loop edges, then check has_cycle(). + if not self_loop_nodes or any( + dep != node_id + for node_id, node in graph.nodes.items() + for dep in node.dependencies + if dep in graph.nodes + ): + # Build a view with self-loop edges stripped + clean_graph = DependencyGraph() + for node_id, node in graph.nodes.items(): + clean_deps = [d for d in node.dependencies if d != node_id] + clean_graph.add_node( + SubtaskNode( + id=node_id, + dependencies=clean_deps, + outputs=node.outputs, + status=node.status, + ) + ) + if clean_graph.has_cycle(): + findings.append( + LintFinding( + severity="error", + code="cycle", + message="Dependency graph contains a cycle among 2 or more distinct nodes.", + ) + ) + + # --- Layer B: warn-only metrics; skipped when enforcement is "off" --- + if enforcement == "off": + return findings + + afm = affected_files_map or {} + io = node_io or {} + + # thin_edge: edge A->B where A.outputs ∩ B.inputs == ∅ AND files(A) ∩ files(B) == ∅ + # Conservative: if data is absent for either node, do NOT flag (avoid false positives). + for node_id, node in graph.nodes.items(): + b_io = io.get(node_id, {}) + b_inputs: Set[str] = b_io.get("inputs", set()) or set() + b_files: Set[str] = afm.get(node_id, set()) or set() + for dep_id in node.dependencies: + if dep_id == node_id or dep_id not in graph.nodes: + continue # skip self-loops and unknown deps (Layer A already handles these) + a_io = io.get(dep_id, {}) + a_outputs: Set[str] = a_io.get("outputs", set()) or set() + a_files: Set[str] = afm.get(dep_id, set()) or set() + # Conservative: if any side lacks io AND files data, skip + if not (a_outputs or b_inputs) and not (a_files or b_files): + continue # both sides are empty — no data at all, skip + # Real data-flow check: non-empty intersection on either dimension → not thin + io_overlap = a_outputs & b_inputs + file_overlap = a_files & b_files + if io_overlap or file_overlap: + continue # real edge, not thin + # Both intersections empty → candidate thin edge. Be conservative: + # only flag when BOTH nodes have io data, otherwise an empty + # intersection is just unknown io on one side (false positive). + if not (a_io and b_io): + continue + findings.append( + LintFinding( + severity="warning", + code="thin_edge", + message=( + f"Edge '{dep_id}' -> '{node_id}' has no io-overlap and no " + f"file-overlap; dependency may be ordering-only." + ), + edge=(dep_id, node_id), + ) + ) + + # same_file_coloring: two subtasks in the same wave share an affected file → INFO + waves = graph.compute_waves() + if waves and afm: + for wave in waves: + if len(wave) < 2: + continue + for i in range(len(wave)): + for j in range(i + 1, len(wave)): + sid_a, sid_b = wave[i], wave[j] + fa = afm.get(sid_a, set()) or set() + fb = afm.get(sid_b, set()) or set() + if fa & fb: + findings.append( + LintFinding( + severity="info", + code="same_file_coloring", + message=( + f"Subtasks '{sid_a}' and '{sid_b}' in the same wave " + f"share affected files: {sorted(fa & fb)!r}. " + f"Consider coloring (split_wave_by_file_conflicts)." + ), + edge=(sid_a, sid_b), + ) + ) + + # fully_serialized: N>=4 AND every wave has width==1 → one warning + if waves is not None: + n = len(graph.nodes) + if n >= 4 and waves and max(len(w) for w in waves) == 1: + findings.append( + LintFinding( + severity="warning", + code="fully_serialized", + message=( + f"All {n} subtasks are fully serialized (every wave has width 1). " + f"Consider whether some can run in parallel." + ), + ) + ) + + # redundant_edge: A->B where A is also reachable transitively via other deps of B → INFO + for node_id, node in graph.nodes.items(): + for dep_id in node.dependencies: + if dep_id == node_id or dep_id not in graph.nodes: + continue + # Other deps of node_id (excluding dep_id itself) + other_deps = [d for d in node.dependencies if d != dep_id and d in graph.nodes] + # Check if dep_id is reachable transitively from any other dep + for other in other_deps: + if dep_id in _transitive_deps(graph, other): + findings.append( + LintFinding( + severity="info", + code="redundant_edge", + message=( + f"Edge '{dep_id}' -> '{node_id}' is redundant: " + f"'{dep_id}' is already reachable transitively via '{other}'." + ), + edge=(dep_id, node_id), + ) + ) + break # one finding per edge + + # soft_phrase: scan dep_justifications in node_io if present + justifications: Dict[Tuple[str, str], str] = {} + for node_id, node_data in io.items(): + dep_justs = node_data.get("dep_justifications", {}) + if dep_justs: + for dep_id, text in dep_justs.items(): + if isinstance(text, str): + justifications[(dep_id, node_id)] = text + if justifications: + findings.extend(soft_phrase_findings(justifications)) + + return findings diff --git a/src/mapify_cli/parallelism_observability.py b/src/mapify_cli/parallelism_observability.py new file mode 100644 index 00000000..1f85beeb --- /dev/null +++ b/src/mapify_cli/parallelism_observability.py @@ -0,0 +1,153 @@ +"""Dormant observability scaffolding for parallel-wave execution. + +This module defines: + - The canonical, stable reason-code constants for worktree fallback and + parallel-dispatch decisions (consumed by the runner in ST-009 and Slice 5). + - The ``ParallelismReport`` TypedDict schema for + ``.map/runs//parallelism.json``. + - A ``write_parallelism_report`` writer that is NO-OP by default (gated on + an explicit ``enabled=True`` argument or the ST-003 observability toggle). + +Slice 5 will add the runtime writer/detection logic and must resolve the +runner-vs-CLI runtime-locality concern: the runner (.jinja template installed +in user repos) cannot import this CLI-side module in installed repos. Slice 5 +should either (a) duplicate the write path inside the runner .jinja with this +module as the canonical schema reference, or (b) expose a subprocess-callable +entry point that the runner shells out to. Decide in the Slice 5 spike. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Optional + +# --------------------------------------------------------------------------- +# Canonical reason-code constants +# --------------------------------------------------------------------------- +# Worktree-fallback codes — values MUST match the runner's _WT_REASON_* in +# map_step_runner.py.jinja (a parity test in test_parallelism_observability.py +# enforces this invariant to prevent silent drift). + +REASON_NOT_GIT_REPO: str = "not_git_repo" +REASON_WORKTREE_UNSUPPORTED: str = "worktree_unsupported" +REASON_WORKTREE_CREATE_FAILED: str = "worktree_create_failed" +REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" + +# Dispatch / observability codes (Slice 5+ consumers) +REASON_DISPATCH_SERIAL: str = "dispatch_serial" +REASON_PARALLEL_CAPPED_BY_MAX_ACTORS: str = "parallel_capped_by_max_actors" +REASON_MONITOR_REJECTED_SUBTASK: str = "monitor_rejected_subtask" +REASON_MERGE_CONFLICT: str = "merge_conflict" +REASON_POST_WAVE_GATE_FAILED: str = "post_wave_gate_failed" + +# Validation set — all 9 canonical codes in one place. +ALL_REASON_CODES: frozenset[str] = frozenset( + { + REASON_NOT_GIT_REPO, + REASON_WORKTREE_UNSUPPORTED, + REASON_WORKTREE_CREATE_FAILED, + REASON_DIRTY_MERGE_TARGET, + REASON_DISPATCH_SERIAL, + REASON_PARALLEL_CAPPED_BY_MAX_ACTORS, + REASON_MONITOR_REJECTED_SUBTASK, + REASON_MERGE_CONFLICT, + REASON_POST_WAVE_GATE_FAILED, + } +) + +# --------------------------------------------------------------------------- +# Schema: ParallelismReport +# --------------------------------------------------------------------------- +# Defined once here (TypedDict per the contract-first learned rule). +# Slice 5 imports this type to populate and write the report. + +from typing import TypedDict # noqa: E402 — grouped after constants for readability + + +class ColorGroupDecision(TypedDict): + """Per color-group (wave sub-group) dispatch decision record.""" + + group_id: str + """Identifier for this color group within the wave.""" + + planned_mode: str + """Mode selected by the config predicate: 'sequential' | 'parallel'.""" + + actual_mode: str + """Mode actually executed after fallback resolution.""" + + worktree_status: str + """Worktree probe outcome: 'ok' | 'skipped' | reason_code.""" + + reason_code: Optional[str] + """Populated when actual_mode != planned_mode; one of ALL_REASON_CODES.""" + + dispatch_count: int + """Number of subtasks dispatched in this group.""" + + +class ParallelismReport(TypedDict): + """Schema for .map/runs//parallelism.json. + + Caller (Slice 5) supplies ``run_id`` and ``generated_at``; this module + never calls ``datetime.now()`` (clock-free per the learned rule). + """ + + schema_version: str + """Semver for this schema — bump when fields are added/removed.""" + + run_id: str + """Unique run identifier; matches the ``.map/runs//`` directory.""" + + generated_at: str + """ISO-8601 timestamp, supplied by the caller (not generated here).""" + + # Plan summary + total_subtasks: int + total_edges: int + total_waves: int + max_wave_width: int + """Width of the widest wave (max parallel color groups).""" + + color_group_breakdown: list[ColorGroupDecision] + """One entry per color group, in wave-then-group order.""" + + +# --------------------------------------------------------------------------- +# Dormant writer — NO-OP by default (, SC-1) +# --------------------------------------------------------------------------- + + +def write_parallelism_report( + report: ParallelismReport, + out_path: Path, + *, + enabled: bool = False, +) -> bool: + """Write ``report`` as JSON to ``out_path``. + + DORMANT by default (``enabled=False``): returns ``False`` without creating + or touching the file. Slice 5 activates this by passing ``enabled=True`` + (driven by the ST-003 ``observability.parallelism`` toggle). + + Clock-free: caller supplies ``out_path`` and ``report['generated_at']``. + Does NOT call ``datetime.now()`` internally. + + Args: + report: A ``ParallelismReport`` dict to serialize. + out_path: Destination path for ``parallelism.json``. + enabled: Gate flag. Default ``False`` keeps the writer dormant. + + Returns: + ``True`` if the file was written, ``False`` if dormant/disabled. + """ + if not enabled: + return False + + out_path.parent.mkdir(parents=True, exist_ok=True) + out_path.write_text( + json.dumps(report, indent=2, ensure_ascii=False) + "\n", + encoding="utf-8", + ) + return True diff --git a/src/mapify_cli/templates/codex/skills/map-efficient/efficient-reference.md b/src/mapify_cli/templates/codex/skills/map-efficient/efficient-reference.md index 63eb173f..460d9be7 100644 --- a/src/mapify_cli/templates/codex/skills/map-efficient/efficient-reference.md +++ b/src/mapify_cli/templates/codex/skills/map-efficient/efficient-reference.md @@ -93,11 +93,34 @@ envelope/anti-gaming checks. ## Wave Execution -Sequential execution is the default. Use wave APIs only when the blueprint has -multiple ready subtasks whose writes are low-risk and disjoint, or when the user -explicitly requests parallel execution. +### Execution strategy decision table -Commands: +`select_execution_strategy` picks between the legacy sequential walker and the +wave-loop on every run. The wave-loop engages **only when ALL THREE hold** +(otherwise the legacy sequential walker `get_next_step` runs): + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, +`worktree.isolation=off`. The isolation gate fails by default, so a stock +`mapify init` config always runs the legacy sequential walker. Even when the +wave-loop engages, dispatch stays sequential until concurrency ships (Slice 5+). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. Do not mix wave +APIs with the sequential cursor for the same workflow. + +### Wave-loop commands ```bash python3 .map/scripts/map_orchestrator.py set_waves --blueprint ".map/${BRANCH}/blueprint.json" @@ -109,6 +132,10 @@ python3 .map/scripts/map_orchestrator.py advance_wave Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. +Use wave APIs only when the blueprint has multiple ready subtasks whose writes +are low-risk and disjoint, or when the user explicitly requests parallel +execution. + When `worktree.isolation` is enabled and a wave runs in parallel (≥2 disjoint subtasks), give each subtask its own worktree and accept the whole wave atomically after all pass Monitor — never merge them one at a time (the first @@ -123,6 +150,43 @@ to base on any conflict or gate failure (worktrees kept for retry). On a single subtask's Monitor failure, `discard_subtask_worktree` that subtask and retry it before calling `merge_wave_worktrees`. +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (dispatching multiple actor subagents in a single turn) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: +> true` / `parallel_ready` flag set**. In the **current framework** +> `concurrency_enabled` is **False**, so dispatch stays **SEQUENTIAL even when a +> wave has `mode=="parallel"`**. The example below is reference material for when +> that capability ships; do NOT treat it as an active instruction now. Use your +> Codex runtime's own parallel actor-subagent dispatch mechanism — this is the +> provider-neutral shape, not a literal API call. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks +dispatches all N actor subagents in **one turn**: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — one turn, N actor subagents: +dispatch actor subagent -> ST-003 (pinned to its own worktree) +dispatch actor subagent -> ST-004 (pinned to its own worktree) + +# INCORRECT — one actor per turn (serial, defeats the wave): +# Turn 1: actor -> ST-003 +# Turn 2: actor -> ST-004 +``` + +**Self-audit before dispatch:** "I will dispatch {n} actor subagents in one turn." + +**`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are +pre-split into sequential batches before dispatch. + +### Anti-patterns + +- One actor dispatch per turn across N turns — serial loop, no concurrency. +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. +- Waiting for one actor result before dispatching the next. +- Mixing `get_next_step` and `get_wave_step` for the same wave. + ## TDD Mode `--tdd` inserts `TEST_WRITER` and `TEST_FAIL_GATE` before `ACTOR`. diff --git a/src/mapify_cli/templates/map/scripts/map_orchestrator.py b/src/mapify_cli/templates/map/scripts/map_orchestrator.py index d24ee45b..7d02035b 100755 --- a/src/mapify_cli/templates/map/scripts/map_orchestrator.py +++ b/src/mapify_cli/templates/map/scripts/map_orchestrator.py @@ -247,6 +247,10 @@ def _extract_subtask_ids_from_plan_artifacts( AGGRESSIVE_COMPRESSION_MULTIPLIER = 0.4 +# Slice 3: sequential-inside wave-loop. Slice 5 flips this to True when +# concurrent Task dispatch is actually implemented and safe to enable. +WAVE_CONCURRENCY_ENABLED = False + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2300,6 +2304,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": 0, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, "message": "No execution waves configured. Use sequential mode.", } @@ -2309,6 +2314,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } wave = state.execution_waves[state.current_wave_index] @@ -2345,6 +2351,9 @@ def get_wave_step(branch: str) -> dict: "wave_total": len(state.execution_waves), "subtasks": subtask_infos, "is_complete": False, + # concurrency_enabled=False: even when mode=="parallel" (width>=2 wave), + # dispatch is strictly sequential this slice. Slice 5 flips WAVE_CONCURRENCY_ENABLED. + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } @@ -2440,6 +2449,73 @@ def advance_wave(branch: str) -> dict: } +def select_execution_strategy( + branch: str, project_dir: Optional[Path] = None +) -> dict: + """Determine whether to use wave_loop or legacy sequential walker. + + Predicate: wave_loop IFF wave_mode in {on, auto} AND worktree.isolation != 'off' + AND any color-group has width >= 2. This mirrors the canonical MapConfig gating + (#305): execution_wave_mode defaults to 'auto' but the wave-loop stays dormant + because worktree.isolation defaults to 'off', so default config always returns + 'sequential' and the legacy get_next_step path is byte-identical (HC-1). + + Args: + branch: Git branch name (sanitized) + project_dir: Project root containing .map/config.yaml. + Defaults to Path('.') consistent with other helpers. + + Returns: + { + "strategy": "wave_loop" | "sequential", + "wave_mode": "off" | "auto" | "on", + "worktree_isolation": "off" | "auto" | "required", + "has_parallel_groups": bool, + "reason": str, + } + """ + if project_dir is None: + project_dir = Path(".") + + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _execution_wave_mode, + _worktree_isolation_mode, + ) + wave_mode = _execution_wave_mode(project_dir) + isolation_mode = _worktree_isolation_mode(project_dir) + except ImportError: + wave_mode = "off" + isolation_mode = "off" + + state_file = Path(f".map/{branch}/step_state.json") + state = StepState.load(state_file) + has_parallel_groups = any(len(g) >= 2 for g in state.execution_waves) + + if wave_mode in {"on", "auto"} and isolation_mode != "off" and has_parallel_groups: + strategy = "wave_loop" + reason = ( + f"wave_mode={wave_mode!r}, worktree.isolation={isolation_mode!r}, " + "and execution_waves has width>=2 group" + ) + else: + if wave_mode not in {"on", "auto"}: + reason = f"wave_mode={wave_mode!r} (not on/auto) → legacy sequential" + elif isolation_mode == "off": + reason = "worktree.isolation='off' → legacy sequential (no isolation, no parallel)" + else: + reason = "no color-group with width>=2 → sequential (all width-1 waves)" + strategy = "sequential" + + return { + "strategy": strategy, + "wave_mode": wave_mode, + "worktree_isolation": isolation_mode, + "has_parallel_groups": has_parallel_groups, + "reason": reason, + } + + def _write_feedback_file( branch: str, filename: str, header: str, feedback: str ) -> Optional[str]: diff --git a/src/mapify_cli/templates/map/scripts/map_step_runner.py b/src/mapify_cli/templates/map/scripts/map_step_runner.py index 2ba7e9d5..4188c256 100755 --- a/src/mapify_cli/templates/map/scripts/map_step_runner.py +++ b/src/mapify_cli/templates/map/scripts/map_step_runner.py @@ -146,6 +146,22 @@ def _map_config_int(project_dir: Path, key: str, default: int) -> int: return parsed if parsed > 0 else default +_WAVE_MODE_VALID = frozenset({"off", "auto", "on"}) + + +def _execution_wave_mode(project_dir: Path) -> str: + """Return the execution.wave_mode setting: 'off' | 'auto' | 'on'. + + Default + enum mirror the canonical MapConfig schema (config/project_config.py): + absent/unknown/garbage normalises to 'auto'. 'auto' is still behavior-neutral + by default because the wave-loop only engages when worktree.isolation != 'off' + (which itself defaults to 'off') AND a color group has >=2 members — see + select_execution_strategy. Never raises. + """ + raw = _map_config_str(project_dir, "execution.wave_mode", "auto") + return raw if raw in _WAVE_MODE_VALID else "auto" + + def _extract_transcript_usage(entry: dict) -> Optional[int]: message = entry.get("message") if not isinstance(message, dict): @@ -15321,6 +15337,34 @@ def _wt_force_remove(path: Path, branch_ref: str) -> None: _wt_git(["branch", "-D", branch_ref]) +# Stable reason codes shared by resolve_worktree_isolation and ST-011 observability. +_WT_REASON_NOT_GIT_REPO: str = "not_git_repo" +_WT_REASON_UNSUPPORTED: str = "worktree_unsupported" +_WT_REASON_CREATE_FAILED: str = "worktree_create_failed" +_WT_REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" + +_WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) + + +def _worktree_isolation_mode(project_dir: Path) -> str: + """Return the worktree.isolation setting: 'off' | 'auto' | 'required'. + + Accepts the new enum strings directly (case-insensitive). + Legacy boolean compat: boolish-truthy (true/1/yes) → 'required'; + boolish-false (false/0/no/'') → 'off'. + Absent key → 'off' (today's behavior). Any unknown/garbage → 'off'. + Never raises. + """ + raw = _map_config_str(project_dir, "worktree.isolation", "") + normalized = raw.strip().lower() + if normalized in _WT_ISOLATION_VALID: + return normalized + # Legacy boolean compat + if _parse_boolish(normalized): + return "required" + return "off" + + def _wt_isolation_enabled(project_dir: Path) -> bool: """Return True when worktree isolation is active for the current project. @@ -15332,6 +15376,11 @@ def _wt_isolation_enabled(project_dir: Path) -> bool: lands in Slice 5; the call site in create_subtask_worktree already degrades to sequential when this returns False) - ``required`` -> True (hard-fail on unavailability, same as old ``true``) + + Canonical enum vocabulary + default live in MapConfig + (config/project_config.py). `_worktree_isolation_mode` above mirrors the + same `worktree.isolation` key for probe/fallback paths that need the full + enum value (auto vs required), not just the enabled boolean. """ val = _map_config_str(project_dir, "worktree.isolation", "off").strip().lower() return val in {"required", "true", "yes", "y", "1", "on"} @@ -15346,6 +15395,41 @@ def _wt_max_deletions(project_dir: Path) -> int: return n if n >= 0 else 50 +_LINT_ENFORCEMENT_VALID = frozenset({"off", "warn", "repair_once", "strict"}) + + +def _lint_dependency_enforcement(project_dir: Path) -> str: + """Return the lint.dependency_enforcement setting. + + Accepted values: 'off' | 'warn' | 'repair_once' | 'strict'. + Absent key or unknown/garbage value → 'warn' (today's no-op default). + Never raises. + """ + raw = _map_config_str(project_dir, "lint.dependency_enforcement", "warn") + normalized = raw.strip().lower() + return normalized if normalized in _LINT_ENFORCEMENT_VALID else "warn" + + +def _lint_auto_prune(project_dir: Path) -> bool: + """Return the lint.auto_prune setting (default False). + + Absent key → False (no mutation today). Never raises. + """ + raw = _map_config_str(project_dir, "lint.auto_prune", "false") + return _parse_boolish(raw) + + +def _observability_parallelism_enabled(project_dir: Path) -> bool: + """Return the observability.parallelism setting (default False). + + This is the dormant no-op gate that ST-011's parallelism.json writer + will check. Absent key → False (no observability writes today). + Never raises. + """ + raw = _map_config_str(project_dir, "observability.parallelism", "false") + return _parse_boolish(raw) + + def _wt_config_verification_checks(project_dir: Path) -> list[str]: """Read the `verification_checks` LIST from .map/config.yaml (lazy yaml).""" config_path = project_dir / ".map" / "config.yaml" @@ -15412,6 +15496,322 @@ def _wt_parse_name_status(text: str) -> tuple[list[str], list[str], int]: return deleted, runtime, changed +# --------------------------------------------------------------------------- +# Worktree probe (Slice 2 / ST-008) +# --------------------------------------------------------------------------- +# Module-level cache: key = resolved toplevel path, value = probe result dict. +# Reset between test runs via _WORKTREE_PROBE_CACHE.clear(). +_WORKTREE_PROBE_CACHE: dict[str, dict[str, object]] = {} + + +def _worktree_probe(project_dir: Path) -> dict[str, object]: + """Safe detached worktree probe. Cached per session keyed on toplevel. + + HC-1 dormancy contract: when worktree.isolation is 'off' (the default), + this function returns immediately WITHOUT running any git command. + + When mode is 'auto' or 'required': + * Resolves the storage root via _wt_storage_root() (never .map/worktrees/). + * Adds a detached probe worktree at /.probe- to test + `git worktree add --detach` support. + * Always removes the probe (force + prune) in a try/finally so no probe + leaks even on exception. + * Verifies we are in the primary checkout by comparing _wt_toplevel() + against the first `worktree` line from `git worktree list --porcelain`. + * Returns {"status":"ok","ok":True,"supported":True,"is_primary":bool} on + success, or _wt_error("WORKTREE_PROBE_FAILED", ...) on any failure. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Check the session cache (keyed on resolved toplevel, falls back to str(project_dir)) + toplevel = _wt_toplevel() + cache_key = str(toplevel) if toplevel is not None else str(project_dir.resolve()) + if cache_key in _WORKTREE_PROBE_CACHE: + return _WORKTREE_PROBE_CACHE[cache_key] + + # Verify primary checkout + is_primary = False + if toplevel is not None: + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + primary_path = Path(line[len("worktree "):].strip()).resolve() + is_primary = primary_path == toplevel + break # first entry is always the main checkout + + storage = _wt_storage_root() + if storage is None: + # Not cached: only successful probes are memoized so a transient + # failure never becomes a permanent session verdict. + return _wt_error( + "WORKTREE_PROBE_FAILED", + "could not resolve git common dir for probe storage", + ) + + import os as _os # noqa: PLC0415 — local to keep module-level clean + + probe_path = storage / f".probe-{_os.getpid()}" + try: + storage.mkdir(parents=True, exist_ok=True) + add = _wt_git( + ["worktree", "add", "--detach", str(probe_path), "HEAD"], + timeout=30, + ) + if add.returncode != 0: + # Transient create failure — do NOT cache; let auto/required retry. + return _wt_error( + "WORKTREE_PROBE_FAILED", + add.stderr.strip() or "git worktree add --detach failed", + ) + result = { + "status": "ok", + "ok": True, + "supported": True, + "is_primary": is_primary, + } + except Exception as exc: + result = _wt_error("WORKTREE_PROBE_FAILED", str(exc)) + finally: + _wt_git(["worktree", "remove", "--force", str(probe_path)], timeout=30) + _wt_git(["worktree", "prune"], timeout=15) + + # Memoize ONLY successful probes; a transient failure must not stick for the + # session (auto can recover, required stops aborting once the repo is fixed). + if result.get("ok"): + _WORKTREE_PROBE_CACHE[cache_key] = result + return result + + +def _require_clean_merge_target(project_dir: Path) -> dict[str, object]: + """Check that the main checkout is clean enough for a worktree merge. + + Dormant when worktree.isolation is 'off' (default — HC-1). + When the check is active AND require_clean_merge_target config is True + (default True), runs `git status --porcelain` and returns: + * {"status":"ok","ok":True} when clean (excluding MAP runtime state) + * {"status":"dirty","ok":False,"kind":"DIRTY_MERGE_TARGET","dirty":[...]} + when uncommitted changes are present. + Never raises. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Read the require_clean_merge_target flag (default True) + raw_require = _map_config_str(project_dir, "require_clean_merge_target", "true") + if not _parse_boolish(raw_require): + return {"status": "ok", "ok": True, "skipped": True} + + st = _wt_git(["status", "--porcelain"], timeout=15) + if st.returncode != 0: + return _wt_error("CLEAN_CHECK_FAILED", st.stderr.strip() or "git status failed") + + dirty = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty: + return { + "status": "dirty", + "ok": False, + "kind": "DIRTY_MERGE_TARGET", + "reason": "main checkout has uncommitted changes", + "dirty": dirty[:20], + } + return {"status": "ok", "ok": True} + + +# --------------------------------------------------------------------------- +# Slice 2 / ST-009: fallback matrix + orphan cleanup +# --------------------------------------------------------------------------- + + +def resolve_worktree_isolation(project_dir: Path) -> dict[str, object]: + """Classify the current environment and decide the execution decision. + + HC-1 dormancy: when isolation is 'off' (the default), returns immediately + without running any git command. + + Return schema + ------------- + Dormant (off): + {"status": "dormant", "ok": False, "mode": "off", "decision": "sequential"} + Success (auto or required, all checks pass): + {"ok": True, "decision": "isolated", "degraded": False, "mode": } + Degraded (auto only, any fallback condition): + {"ok": True, "decision": "sequential", "degraded": True, + "reason": , "warning": } + Hard failure (required, any fallback condition): + {"status": "error", "ok": False, "kind": , ...} # from _wt_error + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "decision": "sequential", + } + + # --- determine fallback condition (if any) --- + + # 1. Not a git repo at all + if not _wt_is_git_repo(): + reason_code = _WT_REASON_NOT_GIT_REPO + reason_msg = "not inside a git work tree; worktree isolation requires git" + else: + # 2. Worktree support probe + probe = _worktree_probe(project_dir) + if not probe.get("ok"): + # Probe failed → classify as unsupported or create-failed + # WORKTREE_PROBE_FAILED maps to unsupported (the git worktree add step + # is the minimum bar; any probe failure means we cannot create worktrees) + reason_code = _WT_REASON_UNSUPPORTED + reason_msg = str(probe.get("message", "worktree probe failed")) + else: + # 3. Dirty merge target + clean = _require_clean_merge_target(project_dir) + if not clean.get("ok") and clean.get("status") != "dormant": + reason_code = _WT_REASON_DIRTY_MERGE_TARGET + reason_msg = ( + "main checkout has uncommitted changes; worktree isolation " + "requires a clean merge target. Commit or stash first." + ) + else: + reason_code = "" + reason_msg = "" + + if reason_code: + if mode == "auto": + # Degrade gracefully: warn and fall back to sequential + warning = ( + f"[MAP] WARNING: worktree isolation degraded to sequential — " + f"{reason_msg} (reason={reason_code})" + ) + return { + "ok": True, + "decision": "sequential", + "degraded": True, + "reason": reason_code, + "warning": warning, + } + else: # mode == "required" + return _wt_error( + reason_code.upper(), + reason_msg, + decision="abort", + ) + + return { + "ok": True, + "decision": "isolated", + "degraded": False, + "mode": mode, + } + + +def cleanup_orphan_worktrees(branch: str) -> dict[str, object]: + """Remove worktrees present in storage/git-list but NOT in the active registry. + + HC-1 dormancy: when isolation is 'off', returns immediately without + running any git command. + + Idempotent + crash-safe: a second call removes nothing and does not error. + NEVER removes a worktree recorded as active in _read_worktree_state(branch). + + Returns {"removed": [...paths], "kept_active": [...paths], "ok": True} + """ + # HC-1 dormancy: read isolation mode WITHOUT calling git. + # _map_config_str searches for .map/config.yaml starting from cwd upward, so + # we pass Path(".") to avoid _wt_project_dir() -> _wt_toplevel() -> _wt_git(). + project_dir = Path(".") + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "removed": [], + "kept_active": [], + } + + # Build the set of active (registered) worktree paths from state + state = _read_worktree_state(branch) + worktrees_dict = state.get("worktrees", {}) + if not isinstance(worktrees_dict, dict): + worktrees_dict = {} + active_paths: set[str] = { + str(Path(str(rec.get("path", ""))).resolve()) + for rec in worktrees_dict.values() + if isinstance(rec, dict) and rec.get("path") + } + + # Enumerate candidates from storage root + git worktree list + candidate_paths: set[Path] = set() + + storage = _wt_storage_root() + if storage is not None and storage.exists(): + try: + for entry in storage.iterdir(): + if entry.is_dir() and not entry.name.startswith(".probe-"): + # Recurse one level: storage// + for sub in entry.iterdir(): + if sub.is_dir(): + candidate_paths.add(sub.resolve()) + except OSError: + pass + + # Also enumerate from `git worktree list --porcelain` + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + wt_raw = line[len("worktree "):] + wt_path = Path(wt_raw.strip()) + try: + wt_path = wt_path.resolve() + except OSError: + pass + # Include only map-managed worktrees: real resolved-path ancestry + # under the storage root, NOT a substring match (a substring check + # can misclassify e.g. `/worktrees-backup/...` as managed and + # destroy an unrelated worktree). + if storage is not None: + try: + if wt_path.is_relative_to(storage.resolve()): + candidate_paths.add(wt_path) + except (OSError, ValueError): + pass + + removed: list[str] = [] + kept_active: list[str] = [] + + for candidate in sorted(candidate_paths): + candidate_str = str(candidate) + if candidate_str in active_paths: + kept_active.append(candidate_str) + continue + # Orphan — remove it. Derive the expected branch ref name from the + # directory name (best-effort; _wt_force_remove handles branch-not-found + # by ignoring the branch -D error). + orphan_branch_ref = f"{WORKTREE_BRANCH_PREFIX}{candidate.name}" + try: + _wt_force_remove(candidate, orphan_branch_ref) + removed.append(candidate_str) + except Exception: + # Non-fatal: log and continue. A failed removal is not an error + # for the caller — next call will retry. + pass + + return {"removed": removed, "kept_active": kept_active, "ok": True} + + def create_subtask_worktree( subtask_id: str, attempt: int = 0, diff --git a/src/mapify_cli/templates/skills/map-efficient/SKILL.md b/src/mapify_cli/templates/skills/map-efficient/SKILL.md index e4abc02f..7e1da6f5 100644 --- a/src/mapify_cli/templates/skills/map-efficient/SKILL.md +++ b/src/mapify_cli/templates/skills/map-efficient/SKILL.md @@ -20,11 +20,11 @@ Use [efficient-reference.md](efficient-reference.md) for wave examples, TDD deta ```yaml thinking_policy: medium/adaptive -parallel_tool_policy: guarded_wave_only +parallel_tool_policy: wave_mode_aware ``` - Use deeper reasoning only when a subtask is risky, blocked, under-specified, or repeatedly failing Monitor. -- Keep execution sequential by default. Parallel waves are allowed only under the existing wave rules: all dependencies satisfied, low risk, disjoint new-file writes, and the wave API is used. +- Execution strategy is determined by `select_execution_strategy`: the wave-loop engages only when `execution.wave_mode` is `on` or `auto` AND a color group has ≥2 members; otherwise the legacy sequential walker runs. With default config this is the sequential walker. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and dispatch mechanics. - Do not parallelize state transitions, Monitor retries for the same subtask, or writes to shared branch artifacts. ## Execution Rules @@ -32,7 +32,7 @@ parallel_tool_policy: guarded_wave_only 1. Execute the next state-machine step only; never skip phases. 2. Use the exact agent type for the current phase. 3. Max 5 retry iterations per subtask. -4. Batch mode is default. Sequential subtask execution is default. +4. Batch mode is default. Execution strategy is selected by `select_execution_strategy` (sequential walker by default; wave-loop only when wave_mode is enabled and a color group has ≥2 members). 5. After Monitor pass, record files changed in `step_state.json` for guard isolation. 6. Validate planning metadata before Actor starts: `expected_diff_size`, `concern_type`, `one_logical_step`, `split_rationale`, `concern_justification`, `coverage_map`, `hard_constraints`, `soft_constraints`, `validation_criteria`, `[AC-1]` bracket tags, and `tradeoff_rationale`. 7. Script routing: `map_orchestrator.py` owns state-machine transitions (`get_next_step`, `validate_step`, `monitor_failed`, `record_subtask_result`, `set_waves`, `resume_from_plan`, …); `map_step_runner.py` owns every `detect_*` / `build_*` / `save_*` / `load_*` / `refresh_*` / `log_*` helper plus baseline `record_*` and artifact writers. Full table + the `record_*` / `validate_*` disambiguation in [efficient-reference.md#script-routing-dispatcher-reference](efficient-reference.md#script-routing-dispatcher-reference). @@ -207,23 +207,13 @@ else fi ``` -Default to sequential execution. Use wave APIs only for low-risk disjoint new-file waves or explicit user-requested parallel execution. See [efficient-reference.md](efficient-reference.md#wave-execution) for the full wave loop. +**Execution strategy:** `select_execution_strategy` chooses between the legacy sequential walker and the wave-loop. The wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) engages only when `execution.wave_mode ∈ {on, auto}` AND a color group has ≥2 members; otherwise `get_next_step` (sequential walker) runs. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and full wave loop. **Note on resume:** `resume_from_plan` (Step 0) now auto-invokes `set_waves` when `blueprint.json` is present, so resumed workflows do not need a manual `set_waves` dispatch. The result is reported in the `waves_computed` field of the resume response (`"success"`, `"error"`, or `"skipped"` if no blueprint). -**Wave-loop vs sequential dispatcher:** `get_next_step` is the **sequential** -walker (one phase at a time, in `subtask_sequence` order). The **wave-loop** -(`get_wave_step` / `validate_wave_step` / `advance_wave`) honors -`execution_waves` and is the canonical path when waves contain >1 subtask. -For a single-Actor batch run with a fully-linear plan, `get_next_step` and -the wave-loop converge on the same order, so the skill defaults to the -sequential walker for simplicity. Switch to the wave-loop when (a) waves -have ≥2 subtasks AND (b) the subtasks in that wave touch disjoint files -(see `split_wave_by_file_conflicts`). - ### No-op subtask short-circuit (before RESEARCH) Some subtasks are already-done historically (rename/refactor landed in a prior PR), or truly do not need Actor/Monitor because the requested work is already satisfied by repo state. Skip them up-front to save tokens: diff --git a/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md b/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md index 49620993..76b9554a 100644 --- a/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md +++ b/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md @@ -121,7 +121,90 @@ including clean passes — must carry concrete evidence references. ## Wave Execution -Sequential is default. Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. Use `get_wave_step`, `validate_wave_step`, and `advance_wave`; do not mix wave APIs with the single-current-subtask API. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. +### Execution strategy decision table + +`select_execution_strategy` picks between the legacy sequential walker and the wave-loop on every run. The wave-loop engages **only when ALL THREE hold**; otherwise the legacy sequential walker (`get_next_step`) runs: + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, `worktree.isolation=off`. Because the isolation gate (#2) fails by default, a stock `mapify init` config always runs the legacy sequential walker — byte-identical to pre-Slice-3. Even when the wave-loop does engage, dispatch stays sequential until concurrency ships (Slice 5+, `concurrency_enabled=False`). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. One phase at a time, in `subtask_sequence` order. Do not mix wave APIs with the sequential cursor for the same workflow. + +### Wave-loop + +Use `get_wave_step`, `validate_wave_step`, and `advance_wave` when the wave-loop is active. Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. + +Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. + +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (emitting multiple `Task(actor)` calls in a single message) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: true` / +> `parallel_ready` flag set**. In the **current framework** `concurrency_enabled` is +> **False**, so dispatch stays **SEQUENTIAL even when a wave has `mode=="parallel"`**. +> The example below is reference material for when that capability ships; do NOT +> treat it as an active instruction now. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks dispatches all N Actors in **one message** with N `Task` calls — not one per turn: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — N Task calls in one message: +Task( + subagent_type="actor", + description="Implement ST-003", + prompt="..." +) +Task( + subagent_type="actor", + description="Implement ST-004", + prompt="..." +) +``` + +```text +# INCORRECT — one Task per turn (sequential, defeats the wave): +# Turn 1: Task(actor, ST-003) +# Turn 2: Task(actor, ST-004) ← serial, not concurrent +``` + +**Self-audit before dispatch:** "I will emit {n} Task(actor) calls in one message for this wave." Confirm n matches the color group size. + +**`max_actors` cap:** Default 4–8 concurrent actors per wave. Groups larger than `max_actors` are pre-split into sequential batches of `max_actors` before dispatch; do not emit more than `max_actors` Task calls in a single message. + +### Anti-patterns (wave execution) + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. +- **Waiting for one actor result before dispatching the next** — correct for sequential, wrong for concurrent waves. +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. + +### Actor-boundary prompt template (worktree-isolated subtasks) + +When a subtask runs in its own worktree, prefix the Actor prompt with: + +```text +You are working inside the isolated worktree for {SUBTASK_ID}. +Worktree path: {WT_PATH} +Frozen base SHA: {BASE_SHA} + +HARD CONSTRAINTS: +- Write ONLY inside {WT_PATH}. Never touch the main tree or sibling worktrees. +- Do not commit directly. Your output is merged by merge_subtask_worktree / merge_wave_worktrees. +- On completion, return JSON: {"subtask_id": "{SUBTASK_ID}", "files_changed": [...], "tests_run": [...], "validation_notes": "...", "blocker": null} +``` ## Predictor Recovery diff --git a/src/mapify_cli/templates_src/codex/skills/map-efficient/efficient-reference.md.jinja b/src/mapify_cli/templates_src/codex/skills/map-efficient/efficient-reference.md.jinja index 63eb173f..460d9be7 100644 --- a/src/mapify_cli/templates_src/codex/skills/map-efficient/efficient-reference.md.jinja +++ b/src/mapify_cli/templates_src/codex/skills/map-efficient/efficient-reference.md.jinja @@ -93,11 +93,34 @@ envelope/anti-gaming checks. ## Wave Execution -Sequential execution is the default. Use wave APIs only when the blueprint has -multiple ready subtasks whose writes are low-risk and disjoint, or when the user -explicitly requests parallel execution. +### Execution strategy decision table -Commands: +`select_execution_strategy` picks between the legacy sequential walker and the +wave-loop on every run. The wave-loop engages **only when ALL THREE hold** +(otherwise the legacy sequential walker `get_next_step` runs): + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, +`worktree.isolation=off`. The isolation gate fails by default, so a stock +`mapify init` config always runs the legacy sequential walker. Even when the +wave-loop engages, dispatch stays sequential until concurrency ships (Slice 5+). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. Do not mix wave +APIs with the sequential cursor for the same workflow. + +### Wave-loop commands ```bash python3 .map/scripts/map_orchestrator.py set_waves --blueprint ".map/${BRANCH}/blueprint.json" @@ -109,6 +132,10 @@ python3 .map/scripts/map_orchestrator.py advance_wave Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. +Use wave APIs only when the blueprint has multiple ready subtasks whose writes +are low-risk and disjoint, or when the user explicitly requests parallel +execution. + When `worktree.isolation` is enabled and a wave runs in parallel (≥2 disjoint subtasks), give each subtask its own worktree and accept the whole wave atomically after all pass Monitor — never merge them one at a time (the first @@ -123,6 +150,43 @@ to base on any conflict or gate failure (worktrees kept for retry). On a single subtask's Monitor failure, `discard_subtask_worktree` that subtask and retry it before calling `merge_wave_worktrees`. +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (dispatching multiple actor subagents in a single turn) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: +> true` / `parallel_ready` flag set**. In the **current framework** +> `concurrency_enabled` is **False**, so dispatch stays **SEQUENTIAL even when a +> wave has `mode=="parallel"`**. The example below is reference material for when +> that capability ships; do NOT treat it as an active instruction now. Use your +> Codex runtime's own parallel actor-subagent dispatch mechanism — this is the +> provider-neutral shape, not a literal API call. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks +dispatches all N actor subagents in **one turn**: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — one turn, N actor subagents: +dispatch actor subagent -> ST-003 (pinned to its own worktree) +dispatch actor subagent -> ST-004 (pinned to its own worktree) + +# INCORRECT — one actor per turn (serial, defeats the wave): +# Turn 1: actor -> ST-003 +# Turn 2: actor -> ST-004 +``` + +**Self-audit before dispatch:** "I will dispatch {n} actor subagents in one turn." + +**`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are +pre-split into sequential batches before dispatch. + +### Anti-patterns + +- One actor dispatch per turn across N turns — serial loop, no concurrency. +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. +- Waiting for one actor result before dispatching the next. +- Mixing `get_next_step` and `get_wave_step` for the same wave. + ## TDD Mode `--tdd` inserts `TEST_WRITER` and `TEST_FAIL_GATE` before `ACTOR`. diff --git a/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja b/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja index d24ee45b..7d02035b 100755 --- a/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja +++ b/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja @@ -247,6 +247,10 @@ def _extract_subtask_ids_from_plan_artifacts( AGGRESSIVE_COMPRESSION_MULTIPLIER = 0.4 +# Slice 3: sequential-inside wave-loop. Slice 5 flips this to True when +# concurrent Task dispatch is actually implemented and safe to enable. +WAVE_CONCURRENCY_ENABLED = False + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2300,6 +2304,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": 0, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, "message": "No execution waves configured. Use sequential mode.", } @@ -2309,6 +2314,7 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } wave = state.execution_waves[state.current_wave_index] @@ -2345,6 +2351,9 @@ def get_wave_step(branch: str) -> dict: "wave_total": len(state.execution_waves), "subtasks": subtask_infos, "is_complete": False, + # concurrency_enabled=False: even when mode=="parallel" (width>=2 wave), + # dispatch is strictly sequential this slice. Slice 5 flips WAVE_CONCURRENCY_ENABLED. + "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, } @@ -2440,6 +2449,73 @@ def advance_wave(branch: str) -> dict: } +def select_execution_strategy( + branch: str, project_dir: Optional[Path] = None +) -> dict: + """Determine whether to use wave_loop or legacy sequential walker. + + Predicate: wave_loop IFF wave_mode in {on, auto} AND worktree.isolation != 'off' + AND any color-group has width >= 2. This mirrors the canonical MapConfig gating + (#305): execution_wave_mode defaults to 'auto' but the wave-loop stays dormant + because worktree.isolation defaults to 'off', so default config always returns + 'sequential' and the legacy get_next_step path is byte-identical (HC-1). + + Args: + branch: Git branch name (sanitized) + project_dir: Project root containing .map/config.yaml. + Defaults to Path('.') consistent with other helpers. + + Returns: + { + "strategy": "wave_loop" | "sequential", + "wave_mode": "off" | "auto" | "on", + "worktree_isolation": "off" | "auto" | "required", + "has_parallel_groups": bool, + "reason": str, + } + """ + if project_dir is None: + project_dir = Path(".") + + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _execution_wave_mode, + _worktree_isolation_mode, + ) + wave_mode = _execution_wave_mode(project_dir) + isolation_mode = _worktree_isolation_mode(project_dir) + except ImportError: + wave_mode = "off" + isolation_mode = "off" + + state_file = Path(f".map/{branch}/step_state.json") + state = StepState.load(state_file) + has_parallel_groups = any(len(g) >= 2 for g in state.execution_waves) + + if wave_mode in {"on", "auto"} and isolation_mode != "off" and has_parallel_groups: + strategy = "wave_loop" + reason = ( + f"wave_mode={wave_mode!r}, worktree.isolation={isolation_mode!r}, " + "and execution_waves has width>=2 group" + ) + else: + if wave_mode not in {"on", "auto"}: + reason = f"wave_mode={wave_mode!r} (not on/auto) → legacy sequential" + elif isolation_mode == "off": + reason = "worktree.isolation='off' → legacy sequential (no isolation, no parallel)" + else: + reason = "no color-group with width>=2 → sequential (all width-1 waves)" + strategy = "sequential" + + return { + "strategy": strategy, + "wave_mode": wave_mode, + "worktree_isolation": isolation_mode, + "has_parallel_groups": has_parallel_groups, + "reason": reason, + } + + def _write_feedback_file( branch: str, filename: str, header: str, feedback: str ) -> Optional[str]: diff --git a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja index 2ba7e9d5..4188c256 100755 --- a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja +++ b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja @@ -146,6 +146,22 @@ def _map_config_int(project_dir: Path, key: str, default: int) -> int: return parsed if parsed > 0 else default +_WAVE_MODE_VALID = frozenset({"off", "auto", "on"}) + + +def _execution_wave_mode(project_dir: Path) -> str: + """Return the execution.wave_mode setting: 'off' | 'auto' | 'on'. + + Default + enum mirror the canonical MapConfig schema (config/project_config.py): + absent/unknown/garbage normalises to 'auto'. 'auto' is still behavior-neutral + by default because the wave-loop only engages when worktree.isolation != 'off' + (which itself defaults to 'off') AND a color group has >=2 members — see + select_execution_strategy. Never raises. + """ + raw = _map_config_str(project_dir, "execution.wave_mode", "auto") + return raw if raw in _WAVE_MODE_VALID else "auto" + + def _extract_transcript_usage(entry: dict) -> Optional[int]: message = entry.get("message") if not isinstance(message, dict): @@ -15321,6 +15337,34 @@ def _wt_force_remove(path: Path, branch_ref: str) -> None: _wt_git(["branch", "-D", branch_ref]) +# Stable reason codes shared by resolve_worktree_isolation and ST-011 observability. +_WT_REASON_NOT_GIT_REPO: str = "not_git_repo" +_WT_REASON_UNSUPPORTED: str = "worktree_unsupported" +_WT_REASON_CREATE_FAILED: str = "worktree_create_failed" +_WT_REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" + +_WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) + + +def _worktree_isolation_mode(project_dir: Path) -> str: + """Return the worktree.isolation setting: 'off' | 'auto' | 'required'. + + Accepts the new enum strings directly (case-insensitive). + Legacy boolean compat: boolish-truthy (true/1/yes) → 'required'; + boolish-false (false/0/no/'') → 'off'. + Absent key → 'off' (today's behavior). Any unknown/garbage → 'off'. + Never raises. + """ + raw = _map_config_str(project_dir, "worktree.isolation", "") + normalized = raw.strip().lower() + if normalized in _WT_ISOLATION_VALID: + return normalized + # Legacy boolean compat + if _parse_boolish(normalized): + return "required" + return "off" + + def _wt_isolation_enabled(project_dir: Path) -> bool: """Return True when worktree isolation is active for the current project. @@ -15332,6 +15376,11 @@ def _wt_isolation_enabled(project_dir: Path) -> bool: lands in Slice 5; the call site in create_subtask_worktree already degrades to sequential when this returns False) - ``required`` -> True (hard-fail on unavailability, same as old ``true``) + + Canonical enum vocabulary + default live in MapConfig + (config/project_config.py). `_worktree_isolation_mode` above mirrors the + same `worktree.isolation` key for probe/fallback paths that need the full + enum value (auto vs required), not just the enabled boolean. """ val = _map_config_str(project_dir, "worktree.isolation", "off").strip().lower() return val in {"required", "true", "yes", "y", "1", "on"} @@ -15346,6 +15395,41 @@ def _wt_max_deletions(project_dir: Path) -> int: return n if n >= 0 else 50 +_LINT_ENFORCEMENT_VALID = frozenset({"off", "warn", "repair_once", "strict"}) + + +def _lint_dependency_enforcement(project_dir: Path) -> str: + """Return the lint.dependency_enforcement setting. + + Accepted values: 'off' | 'warn' | 'repair_once' | 'strict'. + Absent key or unknown/garbage value → 'warn' (today's no-op default). + Never raises. + """ + raw = _map_config_str(project_dir, "lint.dependency_enforcement", "warn") + normalized = raw.strip().lower() + return normalized if normalized in _LINT_ENFORCEMENT_VALID else "warn" + + +def _lint_auto_prune(project_dir: Path) -> bool: + """Return the lint.auto_prune setting (default False). + + Absent key → False (no mutation today). Never raises. + """ + raw = _map_config_str(project_dir, "lint.auto_prune", "false") + return _parse_boolish(raw) + + +def _observability_parallelism_enabled(project_dir: Path) -> bool: + """Return the observability.parallelism setting (default False). + + This is the dormant no-op gate that ST-011's parallelism.json writer + will check. Absent key → False (no observability writes today). + Never raises. + """ + raw = _map_config_str(project_dir, "observability.parallelism", "false") + return _parse_boolish(raw) + + def _wt_config_verification_checks(project_dir: Path) -> list[str]: """Read the `verification_checks` LIST from .map/config.yaml (lazy yaml).""" config_path = project_dir / ".map" / "config.yaml" @@ -15412,6 +15496,322 @@ def _wt_parse_name_status(text: str) -> tuple[list[str], list[str], int]: return deleted, runtime, changed +# --------------------------------------------------------------------------- +# Worktree probe (Slice 2 / ST-008) +# --------------------------------------------------------------------------- +# Module-level cache: key = resolved toplevel path, value = probe result dict. +# Reset between test runs via _WORKTREE_PROBE_CACHE.clear(). +_WORKTREE_PROBE_CACHE: dict[str, dict[str, object]] = {} + + +def _worktree_probe(project_dir: Path) -> dict[str, object]: + """Safe detached worktree probe. Cached per session keyed on toplevel. + + HC-1 dormancy contract: when worktree.isolation is 'off' (the default), + this function returns immediately WITHOUT running any git command. + + When mode is 'auto' or 'required': + * Resolves the storage root via _wt_storage_root() (never .map/worktrees/). + * Adds a detached probe worktree at /.probe- to test + `git worktree add --detach` support. + * Always removes the probe (force + prune) in a try/finally so no probe + leaks even on exception. + * Verifies we are in the primary checkout by comparing _wt_toplevel() + against the first `worktree` line from `git worktree list --porcelain`. + * Returns {"status":"ok","ok":True,"supported":True,"is_primary":bool} on + success, or _wt_error("WORKTREE_PROBE_FAILED", ...) on any failure. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Check the session cache (keyed on resolved toplevel, falls back to str(project_dir)) + toplevel = _wt_toplevel() + cache_key = str(toplevel) if toplevel is not None else str(project_dir.resolve()) + if cache_key in _WORKTREE_PROBE_CACHE: + return _WORKTREE_PROBE_CACHE[cache_key] + + # Verify primary checkout + is_primary = False + if toplevel is not None: + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + primary_path = Path(line[len("worktree "):].strip()).resolve() + is_primary = primary_path == toplevel + break # first entry is always the main checkout + + storage = _wt_storage_root() + if storage is None: + # Not cached: only successful probes are memoized so a transient + # failure never becomes a permanent session verdict. + return _wt_error( + "WORKTREE_PROBE_FAILED", + "could not resolve git common dir for probe storage", + ) + + import os as _os # noqa: PLC0415 — local to keep module-level clean + + probe_path = storage / f".probe-{_os.getpid()}" + try: + storage.mkdir(parents=True, exist_ok=True) + add = _wt_git( + ["worktree", "add", "--detach", str(probe_path), "HEAD"], + timeout=30, + ) + if add.returncode != 0: + # Transient create failure — do NOT cache; let auto/required retry. + return _wt_error( + "WORKTREE_PROBE_FAILED", + add.stderr.strip() or "git worktree add --detach failed", + ) + result = { + "status": "ok", + "ok": True, + "supported": True, + "is_primary": is_primary, + } + except Exception as exc: + result = _wt_error("WORKTREE_PROBE_FAILED", str(exc)) + finally: + _wt_git(["worktree", "remove", "--force", str(probe_path)], timeout=30) + _wt_git(["worktree", "prune"], timeout=15) + + # Memoize ONLY successful probes; a transient failure must not stick for the + # session (auto can recover, required stops aborting once the repo is fixed). + if result.get("ok"): + _WORKTREE_PROBE_CACHE[cache_key] = result + return result + + +def _require_clean_merge_target(project_dir: Path) -> dict[str, object]: + """Check that the main checkout is clean enough for a worktree merge. + + Dormant when worktree.isolation is 'off' (default — HC-1). + When the check is active AND require_clean_merge_target config is True + (default True), runs `git status --porcelain` and returns: + * {"status":"ok","ok":True} when clean (excluding MAP runtime state) + * {"status":"dirty","ok":False,"kind":"DIRTY_MERGE_TARGET","dirty":[...]} + when uncommitted changes are present. + Never raises. + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return {"status": "dormant", "ok": False, "reason": "worktree.isolation is off"} + + # Read the require_clean_merge_target flag (default True) + raw_require = _map_config_str(project_dir, "require_clean_merge_target", "true") + if not _parse_boolish(raw_require): + return {"status": "ok", "ok": True, "skipped": True} + + st = _wt_git(["status", "--porcelain"], timeout=15) + if st.returncode != 0: + return _wt_error("CLEAN_CHECK_FAILED", st.stderr.strip() or "git status failed") + + dirty = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty: + return { + "status": "dirty", + "ok": False, + "kind": "DIRTY_MERGE_TARGET", + "reason": "main checkout has uncommitted changes", + "dirty": dirty[:20], + } + return {"status": "ok", "ok": True} + + +# --------------------------------------------------------------------------- +# Slice 2 / ST-009: fallback matrix + orphan cleanup +# --------------------------------------------------------------------------- + + +def resolve_worktree_isolation(project_dir: Path) -> dict[str, object]: + """Classify the current environment and decide the execution decision. + + HC-1 dormancy: when isolation is 'off' (the default), returns immediately + without running any git command. + + Return schema + ------------- + Dormant (off): + {"status": "dormant", "ok": False, "mode": "off", "decision": "sequential"} + Success (auto or required, all checks pass): + {"ok": True, "decision": "isolated", "degraded": False, "mode": } + Degraded (auto only, any fallback condition): + {"ok": True, "decision": "sequential", "degraded": True, + "reason": , "warning": } + Hard failure (required, any fallback condition): + {"status": "error", "ok": False, "kind": , ...} # from _wt_error + """ + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "decision": "sequential", + } + + # --- determine fallback condition (if any) --- + + # 1. Not a git repo at all + if not _wt_is_git_repo(): + reason_code = _WT_REASON_NOT_GIT_REPO + reason_msg = "not inside a git work tree; worktree isolation requires git" + else: + # 2. Worktree support probe + probe = _worktree_probe(project_dir) + if not probe.get("ok"): + # Probe failed → classify as unsupported or create-failed + # WORKTREE_PROBE_FAILED maps to unsupported (the git worktree add step + # is the minimum bar; any probe failure means we cannot create worktrees) + reason_code = _WT_REASON_UNSUPPORTED + reason_msg = str(probe.get("message", "worktree probe failed")) + else: + # 3. Dirty merge target + clean = _require_clean_merge_target(project_dir) + if not clean.get("ok") and clean.get("status") != "dormant": + reason_code = _WT_REASON_DIRTY_MERGE_TARGET + reason_msg = ( + "main checkout has uncommitted changes; worktree isolation " + "requires a clean merge target. Commit or stash first." + ) + else: + reason_code = "" + reason_msg = "" + + if reason_code: + if mode == "auto": + # Degrade gracefully: warn and fall back to sequential + warning = ( + f"[MAP] WARNING: worktree isolation degraded to sequential — " + f"{reason_msg} (reason={reason_code})" + ) + return { + "ok": True, + "decision": "sequential", + "degraded": True, + "reason": reason_code, + "warning": warning, + } + else: # mode == "required" + return _wt_error( + reason_code.upper(), + reason_msg, + decision="abort", + ) + + return { + "ok": True, + "decision": "isolated", + "degraded": False, + "mode": mode, + } + + +def cleanup_orphan_worktrees(branch: str) -> dict[str, object]: + """Remove worktrees present in storage/git-list but NOT in the active registry. + + HC-1 dormancy: when isolation is 'off', returns immediately without + running any git command. + + Idempotent + crash-safe: a second call removes nothing and does not error. + NEVER removes a worktree recorded as active in _read_worktree_state(branch). + + Returns {"removed": [...paths], "kept_active": [...paths], "ok": True} + """ + # HC-1 dormancy: read isolation mode WITHOUT calling git. + # _map_config_str searches for .map/config.yaml starting from cwd upward, so + # we pass Path(".") to avoid _wt_project_dir() -> _wt_toplevel() -> _wt_git(). + project_dir = Path(".") + mode = _worktree_isolation_mode(project_dir) + if mode == "off": + return { + "status": "dormant", + "ok": False, + "mode": "off", + "removed": [], + "kept_active": [], + } + + # Build the set of active (registered) worktree paths from state + state = _read_worktree_state(branch) + worktrees_dict = state.get("worktrees", {}) + if not isinstance(worktrees_dict, dict): + worktrees_dict = {} + active_paths: set[str] = { + str(Path(str(rec.get("path", ""))).resolve()) + for rec in worktrees_dict.values() + if isinstance(rec, dict) and rec.get("path") + } + + # Enumerate candidates from storage root + git worktree list + candidate_paths: set[Path] = set() + + storage = _wt_storage_root() + if storage is not None and storage.exists(): + try: + for entry in storage.iterdir(): + if entry.is_dir() and not entry.name.startswith(".probe-"): + # Recurse one level: storage// + for sub in entry.iterdir(): + if sub.is_dir(): + candidate_paths.add(sub.resolve()) + except OSError: + pass + + # Also enumerate from `git worktree list --porcelain` + wl = _wt_git(["worktree", "list", "--porcelain"], timeout=15) + if wl.returncode == 0: + for raw_line in wl.stdout.splitlines(): + line = raw_line.strip() + if line.startswith("worktree "): + wt_raw = line[len("worktree "):] + wt_path = Path(wt_raw.strip()) + try: + wt_path = wt_path.resolve() + except OSError: + pass + # Include only map-managed worktrees: real resolved-path ancestry + # under the storage root, NOT a substring match (a substring check + # can misclassify e.g. `/worktrees-backup/...` as managed and + # destroy an unrelated worktree). + if storage is not None: + try: + if wt_path.is_relative_to(storage.resolve()): + candidate_paths.add(wt_path) + except (OSError, ValueError): + pass + + removed: list[str] = [] + kept_active: list[str] = [] + + for candidate in sorted(candidate_paths): + candidate_str = str(candidate) + if candidate_str in active_paths: + kept_active.append(candidate_str) + continue + # Orphan — remove it. Derive the expected branch ref name from the + # directory name (best-effort; _wt_force_remove handles branch-not-found + # by ignoring the branch -D error). + orphan_branch_ref = f"{WORKTREE_BRANCH_PREFIX}{candidate.name}" + try: + _wt_force_remove(candidate, orphan_branch_ref) + removed.append(candidate_str) + except Exception: + # Non-fatal: log and continue. A failed removal is not an error + # for the caller — next call will retry. + pass + + return {"removed": removed, "kept_active": kept_active, "ok": True} + + def create_subtask_worktree( subtask_id: str, attempt: int = 0, diff --git a/src/mapify_cli/templates_src/skills/map-efficient/SKILL.md.jinja b/src/mapify_cli/templates_src/skills/map-efficient/SKILL.md.jinja index e4abc02f..7e1da6f5 100644 --- a/src/mapify_cli/templates_src/skills/map-efficient/SKILL.md.jinja +++ b/src/mapify_cli/templates_src/skills/map-efficient/SKILL.md.jinja @@ -20,11 +20,11 @@ Use [efficient-reference.md](efficient-reference.md) for wave examples, TDD deta ```yaml thinking_policy: medium/adaptive -parallel_tool_policy: guarded_wave_only +parallel_tool_policy: wave_mode_aware ``` - Use deeper reasoning only when a subtask is risky, blocked, under-specified, or repeatedly failing Monitor. -- Keep execution sequential by default. Parallel waves are allowed only under the existing wave rules: all dependencies satisfied, low risk, disjoint new-file writes, and the wave API is used. +- Execution strategy is determined by `select_execution_strategy`: the wave-loop engages only when `execution.wave_mode` is `on` or `auto` AND a color group has ≥2 members; otherwise the legacy sequential walker runs. With default config this is the sequential walker. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and dispatch mechanics. - Do not parallelize state transitions, Monitor retries for the same subtask, or writes to shared branch artifacts. ## Execution Rules @@ -32,7 +32,7 @@ parallel_tool_policy: guarded_wave_only 1. Execute the next state-machine step only; never skip phases. 2. Use the exact agent type for the current phase. 3. Max 5 retry iterations per subtask. -4. Batch mode is default. Sequential subtask execution is default. +4. Batch mode is default. Execution strategy is selected by `select_execution_strategy` (sequential walker by default; wave-loop only when wave_mode is enabled and a color group has ≥2 members). 5. After Monitor pass, record files changed in `step_state.json` for guard isolation. 6. Validate planning metadata before Actor starts: `expected_diff_size`, `concern_type`, `one_logical_step`, `split_rationale`, `concern_justification`, `coverage_map`, `hard_constraints`, `soft_constraints`, `validation_criteria`, `[AC-1]` bracket tags, and `tradeoff_rationale`. 7. Script routing: `map_orchestrator.py` owns state-machine transitions (`get_next_step`, `validate_step`, `monitor_failed`, `record_subtask_result`, `set_waves`, `resume_from_plan`, …); `map_step_runner.py` owns every `detect_*` / `build_*` / `save_*` / `load_*` / `refresh_*` / `log_*` helper plus baseline `record_*` and artifact writers. Full table + the `record_*` / `validate_*` disambiguation in [efficient-reference.md#script-routing-dispatcher-reference](efficient-reference.md#script-routing-dispatcher-reference). @@ -207,23 +207,13 @@ else fi ``` -Default to sequential execution. Use wave APIs only for low-risk disjoint new-file waves or explicit user-requested parallel execution. See [efficient-reference.md](efficient-reference.md#wave-execution) for the full wave loop. +**Execution strategy:** `select_execution_strategy` chooses between the legacy sequential walker and the wave-loop. The wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) engages only when `execution.wave_mode ∈ {on, auto}` AND a color group has ≥2 members; otherwise `get_next_step` (sequential walker) runs. See [efficient-reference.md](efficient-reference.md#wave-execution) for the decision table and full wave loop. **Note on resume:** `resume_from_plan` (Step 0) now auto-invokes `set_waves` when `blueprint.json` is present, so resumed workflows do not need a manual `set_waves` dispatch. The result is reported in the `waves_computed` field of the resume response (`"success"`, `"error"`, or `"skipped"` if no blueprint). -**Wave-loop vs sequential dispatcher:** `get_next_step` is the **sequential** -walker (one phase at a time, in `subtask_sequence` order). The **wave-loop** -(`get_wave_step` / `validate_wave_step` / `advance_wave`) honors -`execution_waves` and is the canonical path when waves contain >1 subtask. -For a single-Actor batch run with a fully-linear plan, `get_next_step` and -the wave-loop converge on the same order, so the skill defaults to the -sequential walker for simplicity. Switch to the wave-loop when (a) waves -have ≥2 subtasks AND (b) the subtasks in that wave touch disjoint files -(see `split_wave_by_file_conflicts`). - ### No-op subtask short-circuit (before RESEARCH) Some subtasks are already-done historically (rename/refactor landed in a prior PR), or truly do not need Actor/Monitor because the requested work is already satisfied by repo state. Skip them up-front to save tokens: diff --git a/src/mapify_cli/templates_src/skills/map-efficient/efficient-reference.md.jinja b/src/mapify_cli/templates_src/skills/map-efficient/efficient-reference.md.jinja index 49620993..76b9554a 100644 --- a/src/mapify_cli/templates_src/skills/map-efficient/efficient-reference.md.jinja +++ b/src/mapify_cli/templates_src/skills/map-efficient/efficient-reference.md.jinja @@ -121,7 +121,90 @@ including clean passes — must carry concrete evidence references. ## Wave Execution -Sequential is default. Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. Use `get_wave_step`, `validate_wave_step`, and `advance_wave`; do not mix wave APIs with the single-current-subtask API. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. +### Execution strategy decision table + +`select_execution_strategy` picks between the legacy sequential walker and the wave-loop on every run. The wave-loop engages **only when ALL THREE hold**; otherwise the legacy sequential walker (`get_next_step`) runs: + +1. `execution.wave_mode` ∈ {`auto`, `on`}, **AND** +2. `worktree.isolation` ≠ `off`, **AND** +3. at least one color group has ≥2 members. + +| `execution.wave_mode` | `worktree.isolation` | Color group ≥2? | Dispatcher selected | +|---|---|---|---| +| any | `off` (default) | any | Legacy sequential walker (`get_next_step`) | +| `off` | any | any | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | no (all groups size 1) | Legacy sequential walker (`get_next_step`) | +| `auto` / `on` | `auto` / `required` | yes | Wave-loop (`get_wave_step` / `validate_wave_step` / `advance_wave`) | + +**Defaults (canonical MapConfig):** `execution.wave_mode=auto`, `worktree.isolation=off`. Because the isolation gate (#2) fails by default, a stock `mapify init` config always runs the legacy sequential walker — byte-identical to pre-Slice-3. Even when the wave-loop does engage, dispatch stays sequential until concurrency ships (Slice 5+, `concurrency_enabled=False`). + +### Sequential walker + +Use `get_next_step` for all sequential (default) execution. One phase at a time, in `subtask_sequence` order. Do not mix wave APIs with the sequential cursor for the same workflow. + +### Wave-loop + +Use `get_wave_step`, `validate_wave_step`, and `advance_wave` when the wave-loop is active. Do not mix wave APIs with the sequential `get_next_step` cursor for the same wave unless the orchestrator response explicitly tells you to fall back. + +Parallel execution is allowed only when a wave has satisfied dependencies, low risk, and disjoint new-file writes, or when the user explicitly requests it. When `worktree.isolation` is on and a wave runs in parallel, each subtask gets its own worktree and the wave is accepted atomically via `merge_wave_worktrees` — see [Parallel waves](#worktree-isolation) under Worktree isolation. + +### Concurrent Actor dispatch — GATED EXAMPLE + +> **IMPORTANT — read before using this example.** +> Concurrent fan-out (emitting multiple `Task(actor)` calls in a single message) is +> enabled **only when concurrency is shipped: Slice 5+ / `concurrency_enabled: true` / +> `parallel_ready` flag set**. In the **current framework** `concurrency_enabled` is +> **False**, so dispatch stays **SEQUENTIAL even when a wave has `mode=="parallel"`**. +> The example below is reference material for when that capability ships; do NOT +> treat it as an active instruction now. + +When concurrency is enabled (Slice 5+ only), a parallel wave with N subtasks dispatches all N Actors in **one message** with N `Task` calls — not one per turn: + +```text +# CORRECT (Slice 5+ / concurrency_enabled=True only) — N Task calls in one message: +Task( + subagent_type="actor", + description="Implement ST-003", + prompt="..." +) +Task( + subagent_type="actor", + description="Implement ST-004", + prompt="..." +) +``` + +```text +# INCORRECT — one Task per turn (sequential, defeats the wave): +# Turn 1: Task(actor, ST-003) +# Turn 2: Task(actor, ST-004) ← serial, not concurrent +``` + +**Self-audit before dispatch:** "I will emit {n} Task(actor) calls in one message for this wave." Confirm n matches the color group size. + +**`max_actors` cap:** Default 4–8 concurrent actors per wave. Groups larger than `max_actors` are pre-split into sequential batches of `max_actors` before dispatch; do not emit more than `max_actors` Task calls in a single message. + +### Anti-patterns (wave execution) + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. +- **Waiting for one actor result before dispatching the next** — correct for sequential, wrong for concurrent waves. +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. + +### Actor-boundary prompt template (worktree-isolated subtasks) + +When a subtask runs in its own worktree, prefix the Actor prompt with: + +```text +You are working inside the isolated worktree for {SUBTASK_ID}. +Worktree path: {WT_PATH} +Frozen base SHA: {BASE_SHA} + +HARD CONSTRAINTS: +- Write ONLY inside {WT_PATH}. Never touch the main tree or sibling worktrees. +- Do not commit directly. Your output is merged by merge_subtask_worktree / merge_wave_worktrees. +- On completion, return JSON: {"subtask_id": "{SUBTASK_ID}", "files_changed": [...], "tests_run": [...], "validation_notes": "...", "blocker": null} +``` ## Predictor Recovery diff --git a/tests/fixtures/wave_blueprints.py b/tests/fixtures/wave_blueprints.py new file mode 100644 index 00000000..1f352b64 --- /dev/null +++ b/tests/fixtures/wave_blueprints.py @@ -0,0 +1,393 @@ +""" +Canned blueprint + repo-shape fixtures for the eval/regression harness. + +Blueprint-shape fixtures return a small typed dict carrying per-subtask: + id, dependencies, outputs, inputs, affected_files (set) +Plus: + affected_files_map: dict[str, set[str]] — id -> files, for split_wave_by_file_conflicts + build_graph() -> DependencyGraph — convenience constructor + +Git-shape fixtures build repo state under a caller-supplied tmp_path. +They use subprocess `git` calls; each function documents its git property and +skips gracefully when git is unavailable. + +Consumers (ST-005/006/007/009/010) import from this module by name. +""" + +from __future__ import annotations + +import shutil +import subprocess +from dataclasses import dataclass, field +from pathlib import Path + +import pytest + +from mapify_cli.dependency_graph import DependencyGraph, SubtaskNode + + +# --------------------------------------------------------------------------- +# Typed blueprint shape +# --------------------------------------------------------------------------- + + +@dataclass +class SubtaskSpec: + """Per-subtask specification consumed by lint and wave computations.""" + + id: str + dependencies: list[str] = field(default_factory=list) + outputs: list[str] = field(default_factory=list) + inputs: list[str] = field(default_factory=list) + affected_files: set[str] = field(default_factory=set) + + +@dataclass +class BlueprintFixture: + """ + Returned by every blueprint-shape factory. + + subtasks: ordered list of SubtaskSpec + affected_files_map: id -> set of file paths (for split_wave_by_file_conflicts) + """ + + subtasks: list[SubtaskSpec] + affected_files_map: dict[str, set[str]] + + def build_graph(self) -> DependencyGraph: + """Construct a DependencyGraph from the fixture's subtasks.""" + graph = DependencyGraph() + for spec in self.subtasks: + graph.add_node( + SubtaskNode( + id=spec.id, + dependencies=spec.dependencies, + outputs=spec.outputs, + status="pending", + ) + ) + return graph + + +# --------------------------------------------------------------------------- +# Blueprint-shape factories +# --------------------------------------------------------------------------- + + +def linear_chain() -> BlueprintFixture: + """ + 4-subtask linear chain: ST-001 -> ST-002 -> ST-003 -> ST-004. + + compute_waves() must return 4 waves each of width 1. + No file conflicts (each subtask has disjoint affected_files). + """ + subtasks = [ + SubtaskSpec( + id="ST-001", + dependencies=[], + outputs=["out_a.py"], + inputs=[], + affected_files={"src/module_a.py"}, + ), + SubtaskSpec( + id="ST-002", + dependencies=["ST-001"], + outputs=["out_b.py"], + inputs=["out_a.py"], + affected_files={"src/module_b.py"}, + ), + SubtaskSpec( + id="ST-003", + dependencies=["ST-002"], + outputs=["out_c.py"], + inputs=["out_b.py"], + affected_files={"src/module_c.py"}, + ), + SubtaskSpec( + id="ST-004", + dependencies=["ST-003"], + outputs=["out_d.py"], + inputs=["out_c.py"], + affected_files={"src/module_d.py"}, + ), + ] + affected_files_map: dict[str, set[str]] = {s.id: s.affected_files for s in subtasks} + return BlueprintFixture(subtasks=subtasks, affected_files_map=affected_files_map) + + +def two_wave_parallel() -> BlueprintFixture: + """ + Root node then 3 independent children with DISJOINT affected_files. + + Wave 0: [ST-001] + Wave 1: [ST-002, ST-003, ST-004] (all independent, no shared files) + + Used by wave-computation tests (ST-005) to verify parallel wave detection. + split_wave_by_file_conflicts must leave wave 1 unsplit. + """ + subtasks = [ + SubtaskSpec( + id="ST-001", + dependencies=[], + outputs=["config.yaml"], + inputs=[], + affected_files={"config.yaml"}, + ), + SubtaskSpec( + id="ST-002", + dependencies=["ST-001"], + outputs=["service_a.py"], + inputs=["config.yaml"], + affected_files={"src/service_a.py"}, + ), + SubtaskSpec( + id="ST-003", + dependencies=["ST-001"], + outputs=["service_b.py"], + inputs=["config.yaml"], + affected_files={"src/service_b.py"}, + ), + SubtaskSpec( + id="ST-004", + dependencies=["ST-001"], + outputs=["service_c.py"], + inputs=["config.yaml"], + affected_files={"src/service_c.py"}, + ), + ] + affected_files_map = {s.id: s.affected_files for s in subtasks} + return BlueprintFixture(subtasks=subtasks, affected_files_map=affected_files_map) + + +def conflict_split() -> BlueprintFixture: + """ + A wave of 3 subtasks where 2 share an affected file. + + Dependency structure: + ST-001 (root) -> ST-002, ST-003, ST-004 (all parallel) + + Wave 1: [ST-002, ST-003, ST-004] + File layout (conflict on 'src/shared.py'): + ST-002: {src/shared.py, src/module_x.py} + ST-003: {src/module_y.py} <- no conflict + ST-004: {src/shared.py, src/module_z.py} <- conflicts with ST-002 + + split_wave_by_file_conflicts([ST-002, ST-003, ST-004], map) must produce + at least 2 sub-waves, with ST-002 and ST-004 in different sub-waves. + """ + subtasks = [ + SubtaskSpec( + id="ST-001", + dependencies=[], + outputs=["base.py"], + inputs=[], + affected_files={"src/base.py"}, + ), + SubtaskSpec( + id="ST-002", + dependencies=["ST-001"], + outputs=["x.py"], + inputs=["base.py"], + affected_files={"src/shared.py", "src/module_x.py"}, + ), + SubtaskSpec( + id="ST-003", + dependencies=["ST-001"], + outputs=["y.py"], + inputs=["base.py"], + affected_files={"src/module_y.py"}, + ), + SubtaskSpec( + id="ST-004", + dependencies=["ST-001"], + outputs=["z.py"], + inputs=["base.py"], + affected_files={"src/shared.py", "src/module_z.py"}, + ), + ] + affected_files_map = {s.id: s.affected_files for s in subtasks} + return BlueprintFixture(subtasks=subtasks, affected_files_map=affected_files_map) + + +# --------------------------------------------------------------------------- +# Git-shape factories +# --------------------------------------------------------------------------- + + +def _git_available() -> bool: + """Return True if git is available on PATH.""" + return shutil.which("git") is not None + + +def _git( + *args: str, + cwd: Path | None = None, + check: bool = True, + env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run a git command, returning the CompletedProcess.""" + import os + + run_env = dict(os.environ) + if env: + run_env.update(env) + return subprocess.run( + ["git", *args], + cwd=cwd, + capture_output=True, + text=True, + check=check, + env=run_env, + ) + + +def _git_allow_file_protocol() -> dict[str, str]: + """ + Return env-var override that allows local file:// git transport. + + Modern git (2.38.1+) blocks local-path clones/submodule-add by default + (CVE-2022-39253 mitigation). Setting GIT_CONFIG_COUNT + the matching + key/value pair re-enables the file protocol for test-only local repos. + """ + return { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "protocol.file.allow", + "GIT_CONFIG_VALUE_0": "always", + } + + +def non_git_dir(tmp_path: Path) -> Path: + """ + Return a plain directory that is NOT a git repository. + + Property asserted by tests: `git rev-parse --git-dir` exits nonzero. + """ + repo = tmp_path / "non_git" + repo.mkdir() + (repo / "file.txt").write_text("hello\n") + return repo + + +def shallow_clone(tmp_path: Path) -> Path: + """ + Return a git repo that simulates shallow-clone semantics. + + Implementation: creates a real git repo, commits one file, then writes + `.git/shallow` (the file that git uses to mark a repo as shallow). + The worktree probe code (ST-008/009) can detect this via: + git rev-parse --is-shallow-repository (returns 'true') + or by checking for the existence of .git/shallow. + + This is an honest minimal implementation: a real git init with a real + commit and the shallow marker file — not a fake. + + Skip if git is unavailable. + """ + if not _git_available(): + pytest.skip("git not available") + + repo = tmp_path / "shallow_repo" + repo.mkdir() + _git("init", cwd=repo) + _git("config", "user.email", "test@example.com", cwd=repo) + _git("config", "user.name", "Test User", cwd=repo) + (repo / "README.md").write_text("shallow clone fixture\n") + _git("add", "README.md", cwd=repo) + _git("commit", "-m", "initial commit", cwd=repo) + + # Write the shallow marker — this is what makes git treat the repo as shallow + (repo / ".git" / "shallow").write_text("") + + return repo + + +def submodule_repo(tmp_path: Path) -> Path: + """ + Return a git repo containing a real submodule. + + Creates: + 1. A bare "upstream" repo to act as the submodule remote. + 2. A "main" repo that contains the submodule via `git submodule add`. + + Property asserted by tests: `.gitmodules` file exists and `git submodule + status` exits 0. + + Skip if git is unavailable. + """ + if not _git_available(): + pytest.skip("git not available") + + # Create a bare upstream repo that will be used as the submodule origin + upstream = tmp_path / "upstream.git" + upstream.mkdir() + _git("init", "--bare", str(upstream)) + + # Seed the upstream with one commit (bare repos start empty, submodule add needs HEAD) + seed = tmp_path / "seed" + seed.mkdir() + _git("init", cwd=seed) + _git("config", "user.email", "test@example.com", cwd=seed) + _git("config", "user.name", "Test User", cwd=seed) + (seed / "lib.py").write_text("# submodule lib\n") + _git("add", "lib.py", cwd=seed) + _git("commit", "-m", "initial", cwd=seed) + _git("remote", "add", "origin", str(upstream), cwd=seed) + _git("push", "origin", "HEAD:main", cwd=seed) + + # Create the main repo + main = tmp_path / "main_repo" + main.mkdir() + _git("init", cwd=main) + _git("config", "user.email", "test@example.com", cwd=main) + _git("config", "user.name", "Test User", cwd=main) + (main / "main.py").write_text("# main\n") + _git("add", "main.py", cwd=main) + _git("commit", "-m", "initial main", cwd=main) + + # Add the submodule — uses the local bare repo as the remote. + # Modern git (>=2.38.1) blocks file:// transport by default; allow it for + # this test-only local repo via GIT_CONFIG_COUNT env vars. + _file_env = _git_allow_file_protocol() + _git( + "submodule", "add", + "--branch", "main", + str(upstream), + "lib", + cwd=main, + env=_file_env, + ) + _git("commit", "-m", "add submodule", cwd=main) + + return main + + +def dirty_repo(tmp_path: Path) -> Path: + """ + Return a git repo with uncommitted changes (`git status --porcelain` non-empty). + + Creates a clean repo, commits one file, then modifies a tracked file + without staging — this makes the working tree dirty. + + Property asserted by tests: `git status --porcelain` produces non-empty output. + + Skip if git is unavailable. + """ + if not _git_available(): + pytest.skip("git not available") + + repo = tmp_path / "dirty_repo" + repo.mkdir() + _git("init", cwd=repo) + _git("config", "user.email", "test@example.com", cwd=repo) + _git("config", "user.name", "Test User", cwd=repo) + + # Commit a clean file first + tracked = repo / "tracked.py" + tracked.write_text("x = 1\n") + _git("add", "tracked.py", cwd=repo) + _git("commit", "-m", "initial", cwd=repo) + + # Modify the tracked file without staging — makes working tree dirty + tracked.write_text("x = 2 # modified but not staged\n") + + return repo diff --git a/tests/test_dependency_graph.py b/tests/test_dependency_graph.py index 5c3dddfb..921719f5 100644 --- a/tests/test_dependency_graph.py +++ b/tests/test_dependency_graph.py @@ -8,7 +8,13 @@ """ import pytest -from mapify_cli.dependency_graph import DependencyGraph, SubtaskNode +from mapify_cli.dependency_graph import ( + DependencyGraph, + LintFinding, + SubtaskNode, + lint_dependency_graph, +) +from tests.fixtures.wave_blueprints import conflict_split, linear_chain class TestGetDependentsImmediate: @@ -351,5 +357,209 @@ def test_empty_wave(self): assert graph.split_wave_by_file_conflicts([], {}) == [] +class TestLintDependencyGraphLayerA: + """ST-006: Layer A hard-error lint checks (always on).""" + + def test_layer_a_hard_errors(self): + """Layer A detects self_loop, cycle, unknown_dep, duplicate_edge; valid DAG → zero errors.""" + # --- self_loop --- + g = DependencyGraph() + g.add_node(SubtaskNode(id="ST-001", dependencies=["ST-001"])) + findings = lint_dependency_graph(g) + codes = [f.code for f in findings if f.severity == "error"] + assert "self_loop" in codes, f"Expected self_loop, got: {codes}" + + # --- cycle (2-node) --- + g2 = DependencyGraph() + g2.add_node(SubtaskNode(id="ST-001", dependencies=["ST-002"])) + g2.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + findings2 = lint_dependency_graph(g2) + codes2 = [f.code for f in findings2 if f.severity == "error"] + assert "cycle" in codes2, f"Expected cycle, got: {codes2}" + # self_loop must NOT be reported for a pure 2-node cycle + assert "self_loop" not in codes2, f"self_loop falsely reported for 2-node cycle: {codes2}" + + # --- unknown_dep --- + g3 = DependencyGraph() + g3.add_node(SubtaskNode(id="ST-001", dependencies=["ST-MISSING"])) + findings3 = lint_dependency_graph(g3) + codes3 = [f.code for f in findings3 if f.severity == "error"] + assert "unknown_dep" in codes3, f"Expected unknown_dep, got: {codes3}" + + # --- duplicate_edge --- + g4 = DependencyGraph() + g4.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g4.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001", "ST-001"])) + findings4 = lint_dependency_graph(g4) + codes4 = [f.code for f in findings4 if f.severity == "error"] + assert "duplicate_edge" in codes4, f"Expected duplicate_edge, got: {codes4}" + + # --- valid DAG → zero error-severity findings --- + g5 = DependencyGraph() + g5.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g5.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + g5.add_node(SubtaskNode(id="ST-003", dependencies=["ST-001", "ST-002"])) + findings5 = lint_dependency_graph(g5) + errors5 = [f for f in findings5 if f.severity == "error"] + assert errors5 == [], f"Valid DAG should produce no errors, got: {errors5}" + + def test_layer_a_always_on(self): + """Layer A errors are emitted for all enforcement values (always on).""" + # Graph with a self-loop — triggers a Layer A error regardless of enforcement + for enforcement in ("off", "warn", "repair_once", "strict"): + g = DependencyGraph() + g.add_node(SubtaskNode(id="ST-001", dependencies=["ST-001"])) + findings = lint_dependency_graph(g, enforcement=enforcement) + errors = [f for f in findings if f.severity == "error" and f.code == "self_loop"] + assert errors, ( + f"Layer A self_loop error must be emitted for enforcement={enforcement!r}, " + f"but got: {findings}" + ) + + +class TestLintDependencyGraphLayerB: + """ST-007: Layer B warn-only mechanical edge check + INFO metrics.""" + + def test_layer_b_thin_edge_and_samefile_info(self): + """ + VC1: thin edge (empty io∩ AND empty files∩) → warning; + real data-flow edge → no thin_edge warning; + same-file pair in one wave → INFO (not error). + + Uses two_wave_parallel (for thin/real edge checks) and + conflict_split (for same-file coloring in a wave). + """ + # ---- thin_edge: build a graph with a deliberate ordering-only edge ---- + # ST-A -> ST-B: no shared io, no shared files → thin + g = DependencyGraph() + g.add_node(SubtaskNode(id="ST-A", dependencies=[])) + g.add_node(SubtaskNode(id="ST-B", dependencies=["ST-A"])) + afm = { + "ST-A": {"src/alpha.py"}, + "ST-B": {"src/beta.py"}, # disjoint files + } + # io that also have no overlap + io = { + "ST-A": {"inputs": set(), "outputs": {"out_a.txt"}}, + "ST-B": {"inputs": {"in_b.txt"}, "outputs": {"out_b.txt"}}, # in_b ≠ out_a + } + findings = lint_dependency_graph(g, affected_files_map=afm, node_io=io) + assert all(isinstance(f, LintFinding) for f in findings) + thin = [f for f in findings if f.code == "thin_edge"] + assert thin, f"Expected thin_edge warning, got: {[f.code for f in findings]}" + assert thin[0].severity == "warning" + assert thin[0].edge == ("ST-A", "ST-B") + + # ---- real data-flow edge: shared output/input → NOT flagged as thin ---- + g2 = DependencyGraph() + g2.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g2.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + # outputs config.yaml; inputs config.yaml (real data-flow) + io2 = { + "ST-001": {"inputs": set(), "outputs": {"config.yaml"}}, + "ST-002": {"inputs": {"config.yaml"}, "outputs": {"service_a.py"}}, + } + afm2 = {"ST-001": {"config.yaml"}, "ST-002": {"src/service_a.py"}} + findings2 = lint_dependency_graph(g2, affected_files_map=afm2, node_io=io2) + thin2 = [f for f in findings2 if f.code == "thin_edge"] + assert not thin2, f"Real data-flow edge must NOT be flagged thin; got: {thin2}" + + # ---- same_file_coloring: two subtasks in same wave share a file → INFO ---- + fix2 = conflict_split() + g3 = fix2.build_graph() + # and share src/shared.py in wave 1 + findings3 = lint_dependency_graph(g3, affected_files_map=fix2.affected_files_map) + coloring = [f for f in findings3 if f.code == "same_file_coloring"] + assert coloring, ( + f"Expected same_file_coloring INFO for ST-002/ST-004 sharing src/shared.py; " + f"codes: {[f.code for f in findings3]}" + ) + assert all(f.severity == "info" for f in coloring), ( + f"same_file_coloring must be severity='info', got: {coloring}" + ) + # must NOT be severity='error' + errors = [f for f in findings3 if f.severity == "error"] + assert not errors, f"No errors expected for valid conflict_split DAG; got: {errors}" + + def test_fully_serialized_and_redundant_and_default_warn(self): + """ + VC2: linear_chain (N=4, all width-1) → fully_serialized warning; + redundant edge (A->C where A->B->C) → INFO; + enforcement='off' → Layer B silent (only Layer A); + auto_prune=True performs NO mutation (graph unchanged); + default enforcement='warn' emits Layer B findings. + """ + # ---- fully_serialized: linear_chain has N=4 and max wave width=1 ---- + fix = linear_chain() + g = fix.build_graph() + findings = lint_dependency_graph( + g, affected_files_map=fix.affected_files_map, enforcement="warn" + ) + assert all(isinstance(f, LintFinding) for f in findings) + serialized = [f for f in findings if f.code == "fully_serialized"] + assert serialized, ( + f"Expected fully_serialized warning for 4-node linear chain; " + f"codes: {[f.code for f in findings]}" + ) + assert serialized[0].severity == "warning" + + # ---- redundant_edge: A->B->C AND A->C → A->C is redundant ---- + g2 = DependencyGraph() + g2.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g2.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + # depends on directly AND via → -> is redundant + g2.add_node(SubtaskNode(id="ST-003", dependencies=["ST-002", "ST-001"])) + findings2 = lint_dependency_graph(g2) + redundant = [f for f in findings2 if f.code == "redundant_edge"] + assert redundant, ( + f"Expected redundant_edge INFO for ST-001->ST-003 (also reachable via ST-002); " + f"codes: {[f.code for f in findings2]}" + ) + assert all(f.severity == "info" for f in redundant) + + # ---- enforcement='off' → Layer B silent; Layer A still runs ---- + g3 = DependencyGraph() + g3.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g3.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + g3.add_node(SubtaskNode(id="ST-003", dependencies=["ST-002", "ST-001"])) + findings_off = lint_dependency_graph(g3, enforcement="off") + layer_b_off = [ + f for f in findings_off + if f.code in ("thin_edge", "same_file_coloring", "fully_serialized", "redundant_edge") + ] + assert not layer_b_off, ( + f"enforcement='off' must suppress ALL Layer B findings; got: {layer_b_off}" + ) + + # ---- auto_prune=True performs NO mutation: graph unchanged after lint ---- + g4 = DependencyGraph() + g4.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g4.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + g4.add_node(SubtaskNode(id="ST-003", dependencies=["ST-002", "ST-001"])) + node_ids_before = set(g4.nodes.keys()) + deps_before = {nid: list(n.dependencies) for nid, n in g4.nodes.items()} + lint_dependency_graph(g4, auto_prune=True) + assert set(g4.nodes.keys()) == node_ids_before, "auto_prune must not remove nodes" + for nid in node_ids_before: + assert g4.nodes[nid].dependencies == deps_before[nid], ( + f"auto_prune must not mutate dependencies of {nid}" + ) + + # ---- default enforcement='warn' emits Layer B (redundant_edge case) ---- + g5 = DependencyGraph() + g5.add_node(SubtaskNode(id="ST-001", dependencies=[])) + g5.add_node(SubtaskNode(id="ST-002", dependencies=["ST-001"])) + g5.add_node(SubtaskNode(id="ST-003", dependencies=["ST-002", "ST-001"])) + findings_default = lint_dependency_graph(g5) # default enforcement='warn' + layer_b_default = [ + f for f in findings_default + if f.code in ("thin_edge", "same_file_coloring", "fully_serialized", "redundant_edge") + ] + assert layer_b_default, ( + f"Default enforcement='warn' must emit Layer B findings; " + f"codes: {[f.code for f in findings_default]}" + ) + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/tests/test_map_orchestrator.py b/tests/test_map_orchestrator.py index b2c42d38..c9ea7f04 100644 --- a/tests/test_map_orchestrator.py +++ b/tests/test_map_orchestrator.py @@ -4526,5 +4526,236 @@ def test_vc2_validate_step_24_cli_nonzero_without_recommendation( ) +# --------------------------------------------------------------------------- +# Slice 3 — predicate-gated sequential wave-loop +# --------------------------------------------------------------------------- + + +def _write_config(tmp_path: Path, wave_mode: str) -> None: + """Write a minimal .map/config.yaml with execution.wave_mode set.""" + map_dir = tmp_path / ".map" + map_dir.mkdir(parents=True, exist_ok=True) + (map_dir / "config.yaml").write_text( + f"execution.wave_mode: {wave_mode}\n", encoding="utf-8" + ) + + +def _write_step_state( + branch: str, + tmp_path: Path, + execution_waves: list, + extra: dict | None = None, +) -> None: + """Write a minimal step_state.json with given execution_waves.""" + import json as _json + + state: dict = { + "workflow": "map-efficient", + "started_at": "2026-01-01T00:00:00", + "current_subtask_id": None, + "subtask_index": 0, + "subtask_sequence": [], + "current_step_id": "1.0", + "current_step_phase": "DECOMPOSE", + "completed_steps": [], + "pending_steps": [], + "retry_count": 0, + "max_retries": 5, + "plan_approved": False, + "execution_mode": "batch", + "tdd_mode": False, + "skipped_steps": [], + "execution_waves": execution_waves, + "current_wave_index": 0, + "subtask_phases": {}, + "subtask_retry_counts": {}, + "workflow_status": "INITIALIZED", + "subtask_files_changed": {}, + "guard_rework_counts": {}, + "constraints": None, + "subtask_results": {}, + "last_subtask_commit_sha": None, + "contract_ready_subtasks": {}, + "clean_retry_count": 0, + "contaminated_retry_count": 0, + "scope_feedback_subtasks": [], + "progress_feedback_subtasks": [], + "retry_isolation_status": {}, + "retry_quarantine_paths": {}, + "completed_at": None, + "subtask_completion_reasons": {}, + } + if extra: + state.update(extra) + branch_dir = tmp_path / ".map" / branch + branch_dir.mkdir(parents=True, exist_ok=True) + (branch_dir / "step_state.json").write_text( + _json.dumps(state), encoding="utf-8" + ) + + +def test_wave_loop_sequential_no_concurrency( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC1 [HC-2]: get_wave_step returns concurrency_enabled=False even for width>=2 wave.""" + monkeypatch.chdir(tmp_path) + branch = "test-st010-vc1" + # Two subtasks in one wave → mode="parallel" but concurrency_enabled must be False. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + result = map_orchestrator.get_wave_step(branch) + + assert result["is_complete"] is False + assert result["mode"] == "parallel", f"expected parallel mode for width-2 wave: {result}" + assert result["concurrency_enabled"] is False, ( + f"concurrency_enabled must be False in Slice 3 (no concurrent dispatch): {result}" + ) + # One-subtask-at-a-time dispatch is the contract: caller iterates subtasks[] + # sequentially; concurrency_enabled=False is the explicit signal. + assert len(result["subtasks"]) == 2 # both listed; dispatcher iterates one at a time + + +def test_wave_loop_predicate_gating_default_legacy( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC2 [AC-10]: select_execution_strategy gates on wave_mode AND has_parallel_groups.""" + import types + + monkeypatch.chdir(tmp_path) + branch = "test-st010-vc2" + # Build a state with a width>=2 wave. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + # 1. Default (wave_mode=off) → sequential regardless of wave width. + _write_config(tmp_path, "off") + monkeypatch.setattr( + map_orchestrator, + "select_execution_strategy", + map_orchestrator.select_execution_strategy, + ) + # Inject a mock _execution_wave_mode returning "off" via the import path. + # select_execution_strategy imports from map_step_runner inside the function; + # we patch by injecting a fake module into sys.modules for the call scope. + import sys as _sys + + def _fake_runner(wave_mode: str, isolation: str) -> "types.ModuleType": + """Build a fake map_step_runner exposing BOTH readers that + select_execution_strategy imports (wave_mode + worktree isolation).""" + mod = types.ModuleType("map_step_runner") + + def _wm(_project_dir: object) -> str: + del _project_dir + return wave_mode + + def _iso(_project_dir: object) -> str: + del _project_dir + return isolation + + mod._execution_wave_mode = _wm # type: ignore[attr-defined] + mod._worktree_isolation_mode = _iso # type: ignore[attr-defined] + return mod + + # Save any real map_step_runner so we restore (not erase) it afterwards — + # other tests in the session may rely on the genuine imported module. + _orig_msr = _sys.modules.get("map_step_runner") + try: + # 1. wave_mode=off → sequential even with isolation on and a parallel group. + _sys.modules["map_step_runner"] = _fake_runner("off", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["strategy"] == "sequential", f"off → sequential: {result}" + assert result["wave_mode"] == "off" + assert result["has_parallel_groups"] is True # waves exist, but mode is off + + # 2. wave_mode=on + isolation=required + parallel group → wave_loop. + _sys.modules["map_step_runner"] = _fake_runner("on", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["strategy"] == "wave_loop", f"on + iso=required + width>=2 → wave_loop: {result}" + assert result["wave_mode"] == "on" + assert result["worktree_isolation"] == "required" + + # 3. wave_mode=auto + isolation=auto + parallel group → wave_loop. + _sys.modules["map_step_runner"] = _fake_runner("auto", "auto") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["strategy"] == "wave_loop", f"auto + iso=auto + width>=2 → wave_loop: {result}" + + # 4. ISOLATION GATE: wave_mode=auto (default) but isolation=off → sequential. + # This is the behavior-neutral default (MapConfig: wave_mode=auto, isolation=off). + _sys.modules["map_step_runner"] = _fake_runner("auto", "off") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["strategy"] == "sequential", f"auto + iso=off → sequential (default): {result}" + assert result["worktree_isolation"] == "off" + + # 5. wave_mode=on + isolation=required but ALL waves width-1 → sequential. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001"], ["ST-002"]]) + _sys.modules["map_step_runner"] = _fake_runner("on", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["strategy"] == "sequential", f"on + all width-1 → sequential: {result}" + assert result["has_parallel_groups"] is False + finally: + if _orig_msr is not None: + _sys.modules["map_step_runner"] = _orig_msr + else: + _sys.modules.pop("map_step_runner", None) + + +def test_advance_wave_atomic_reset( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC3 [AC-10]: advance_wave atomically resets ALL per-wave sub-state.""" + monkeypatch.chdir(tmp_path) + branch = "test-st010-vc3" + # Seed stale sub-state that must be cleared on advance. + _write_step_state( + branch, + tmp_path, + execution_waves=[["ST-001"], ["ST-002"]], + extra={ + "current_wave_index": 0, + "subtask_phases": {"ST-001": "2.4"}, + "subtask_retry_counts": {"ST-001": 3}, + "pending_steps": ["2.3", "2.4"], + "completed_steps": ["1.0", "2.2"], + "skipped_steps": ["2.25"], + "current_step_id": "2.3", + "current_step_phase": "ACTOR", + "retry_count": 2, + "current_subtask_id": "ST-001", + }, + ) + + result = map_orchestrator.advance_wave(branch) + + assert result["status"] == "success", result + assert result["current_wave_index"] == 1 + + # Read back state and assert atomic reset of ALL per-wave sub-state. + import json as _json + state_path = tmp_path / ".map" / branch / "step_state.json" + state = _json.loads(state_path.read_text()) + + assert state["subtask_phases"] == {}, f"subtask_phases not reset: {state['subtask_phases']}" + assert state["subtask_retry_counts"] == {}, ( + f"subtask_retry_counts not reset: {state['subtask_retry_counts']}" + ) + assert state["completed_steps"] == [], f"completed_steps not reset: {state['completed_steps']}" + assert state["skipped_steps"] == [], f"skipped_steps not reset: {state['skipped_steps']}" + assert state["retry_count"] == 0, f"retry_count not reset: {state['retry_count']}" + # current_subtask_id and current_step_id must point at the new wave's first subtask. + assert state["current_subtask_id"] == "ST-002", ( + f"current_subtask_id not advanced: {state['current_subtask_id']}" + ) + assert state["current_step_id"] == "2.2", ( + f"current_step_id not reset to research start: {state['current_step_id']}" + ) + # pending_steps must be reset to the research-onward order (no stale entries). + assert state["pending_steps"], "pending_steps not repopulated for the new wave" + assert state["pending_steps"][0] == "2.2", ( + f"pending_steps not reset to research start: {state['pending_steps']}" + ) + assert "2.4" in state["pending_steps"], ( + f"pending_steps missing later phases after reset: {state['pending_steps']}" + ) + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/tests/test_map_step_runner.py b/tests/test_map_step_runner.py index 5e98f2ed..96c326db 100644 --- a/tests/test_map_step_runner.py +++ b/tests/test_map_step_runner.py @@ -12143,3 +12143,658 @@ def test_cli_run_cross_ai_review_disabled_exit_zero(tmp_path): payload = json.loads(completed.stdout) assert payload["status"] == "disabled" assert payload["config"]["enabled"] is False + + +# --------------------------------------------------------------------------- +# _execution_wave_mode tests +# --------------------------------------------------------------------------- + + +def test_wave_mode_absent_defaults_auto(tmp_path: Path) -> None: + """Config without execution.wave_mode key returns the canonical MapConfig + default 'auto' (still behavior-neutral: the wave-loop is gated on + worktree.isolation != 'off', which defaults to 'off').""" + (tmp_path / ".map").mkdir() + # Write a config without any wave_mode key at all. + (tmp_path / ".map" / "config.yaml").write_text( + "some.other.key: value\n", encoding="utf-8" + ) + assert map_step_runner._execution_wave_mode(tmp_path) == "auto" + + +def test_wave_mode_enum_and_unknown_fallback(tmp_path: Path) -> None: + """Known enum values parse; unknown or empty values fall back to 'off'.""" + map_dir = tmp_path / ".map" + map_dir.mkdir() + config = map_dir / "config.yaml" + + # Valid: "auto" + config.write_text("execution.wave_mode: auto\n", encoding="utf-8") + assert map_step_runner._execution_wave_mode(tmp_path) == "auto" + + # Valid: "on" + config.write_text("execution.wave_mode: on\n", encoding="utf-8") + assert map_step_runner._execution_wave_mode(tmp_path) == "on" + + # Valid: "off" explicit + config.write_text("execution.wave_mode: off\n", encoding="utf-8") + assert map_step_runner._execution_wave_mode(tmp_path) == "off" + + # Unknown value -> "auto" (canonical MapConfig default) + config.write_text("execution.wave_mode: parallel\n", encoding="utf-8") + assert map_step_runner._execution_wave_mode(tmp_path) == "auto" + + # Empty value -> "auto" + config.write_text("execution.wave_mode: \n", encoding="utf-8") + assert map_step_runner._execution_wave_mode(tmp_path) == "auto" + + +# --------------------------------------------------------------------------- +# _worktree_isolation_mode tests +# --------------------------------------------------------------------------- + + +def test_worktree_isolation_legacy_bool_mapping(tmp_path: Path) -> None: + """Legacy boolean literals map correctly; absent key defaults to 'off'.""" + map_dir = tmp_path / ".map" + map_dir.mkdir() + config = map_dir / "config.yaml" + + # Absent key -> "off" + config.write_text("some.other.key: value\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + # Legacy false -> "off" + config.write_text("worktree.isolation: false\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + # Legacy true -> "required" + config.write_text("worktree.isolation: true\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "required" + assert map_step_runner._wt_isolation_enabled(tmp_path) is True + + # Legacy "1" -> "required" + config.write_text("worktree.isolation: 1\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "required" + + # Legacy "0" -> "off" + config.write_text("worktree.isolation: 0\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + +def test_worktree_isolation_enum_and_disabled_check_parity(tmp_path: Path) -> None: + """New enum strings parse directly. Per the canonical MapConfig/#305 semantics, + _wt_isolation_enabled is True ONLY for 'required' (and legacy truthy): 'auto' + stays disabled in Slice 0 (parallel dispatch lands in Slice 5), 'off' disabled. + _worktree_isolation_mode still reports the full enum (off/auto/required) for the + probe/fallback paths.""" + map_dir = tmp_path / ".map" + map_dir.mkdir() + config = map_dir / "config.yaml" + + # Enum: "off" + config.write_text("worktree.isolation: off\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + # Enum: "auto" — mode is 'auto' but isolation NOT enabled until Slice 5. + config.write_text("worktree.isolation: auto\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "auto" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + # Enum: "required" — enabled (hard-fail on unavailability, same as legacy true). + config.write_text("worktree.isolation: required\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "required" + assert map_step_runner._wt_isolation_enabled(tmp_path) is True + + # Case-insensitive enum + config.write_text("worktree.isolation: AUTO\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "auto" + + # Unknown garbage -> "off" (disabled) + config.write_text("worktree.isolation: maybe\n", encoding="utf-8") + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off" + assert map_step_runner._wt_isolation_enabled(tmp_path) is False + + +# _lint_dependency_enforcement / _lint_auto_prune / _observability_parallelism_enabled + + +def test_lint_toggle_defaults(tmp_path: Path) -> None: + """lint.dependency_enforcement defaults to 'warn' when absent; parses all enum values; + unknown values fall back to 'warn'. lint.auto_prune defaults to False; 'true' -> True.""" + config = tmp_path / ".map" / "config.yaml" + config.parent.mkdir(parents=True, exist_ok=True) + + # absent key -> "warn" + config.write_text("", encoding="utf-8") + assert map_step_runner._lint_dependency_enforcement(tmp_path) == "warn" + + # valid enum values + for value in ("off", "warn", "repair_once", "strict"): + config.write_text(f"lint.dependency_enforcement: {value}\n", encoding="utf-8") + assert map_step_runner._lint_dependency_enforcement(tmp_path) == value + + # unknown/garbage -> "warn" + config.write_text("lint.dependency_enforcement: parallel\n", encoding="utf-8") + assert map_step_runner._lint_dependency_enforcement(tmp_path) == "warn" + + config.write_text("lint.dependency_enforcement: \n", encoding="utf-8") + assert map_step_runner._lint_dependency_enforcement(tmp_path) == "warn" + + # lint.auto_prune absent -> False + config.write_text("", encoding="utf-8") + assert map_step_runner._lint_auto_prune(tmp_path) is False + + # lint.auto_prune "true" -> True + config.write_text("lint.auto_prune: true\n", encoding="utf-8") + assert map_step_runner._lint_auto_prune(tmp_path) is True + + # lint.auto_prune "false" -> False + config.write_text("lint.auto_prune: false\n", encoding="utf-8") + assert map_step_runner._lint_auto_prune(tmp_path) is False + + +def test_observability_toggle_defaults(tmp_path: Path) -> None: + """observability.parallelism defaults to False when absent; 'true' -> True. + Default config => all toggles are no-op: enforcement=='warn', auto_prune is False, + observability is False.""" + config = tmp_path / ".map" / "config.yaml" + config.parent.mkdir(parents=True, exist_ok=True) + + # absent key -> False + config.write_text("", encoding="utf-8") + assert map_step_runner._observability_parallelism_enabled(tmp_path) is False + + # "true" -> True + config.write_text("observability.parallelism: true\n", encoding="utf-8") + assert map_step_runner._observability_parallelism_enabled(tmp_path) is True + + # "false" -> False + config.write_text("observability.parallelism: false\n", encoding="utf-8") + assert map_step_runner._observability_parallelism_enabled(tmp_path) is False + + # Assert default config (empty) => all no-op + config.write_text("", encoding="utf-8") + assert map_step_runner._lint_dependency_enforcement(tmp_path) == "warn" + assert map_step_runner._lint_auto_prune(tmp_path) is False + assert map_step_runner._observability_parallelism_enabled(tmp_path) is False + + +# _worktree_probe / _require_clean_merge_target + + +def test_probe_dormant_when_off(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """With isolation off (default), _worktree_probe returns dormant and runs NO git command. + + Proves zero-git behavior by monkeypatching _wt_git to record calls and raise + if invoked, then asserting that the call list is empty and status=="dormant". + """ + # Ensure isolation is off (default — config absent) + (tmp_path / ".map").mkdir() + (tmp_path / ".map" / "config.yaml").write_text("", encoding="utf-8") + + # Monkeypatch _wt_git to track calls; any call = test failure + calls: list[list[str]] = [] + + def _no_git(args: list[str], **_kw: object) -> object: + del _kw + calls.append(args) + raise AssertionError(f"_wt_git must not be called when isolation is off; got {args!r}") + + monkeypatch.setattr(map_step_runner, "_wt_git", _no_git) + # Also clear the probe cache so we don't hit a stale cached result + map_step_runner._WORKTREE_PROBE_CACHE.clear() + + result = map_step_runner._worktree_probe(tmp_path) + + assert result["status"] == "dormant", f"expected dormant, got {result}" + assert result["ok"] is False + assert "worktree.isolation is off" in str(result.get("reason", "")) + assert calls == [], "No git command should have been issued when isolation is off" + + # Verify _require_clean_merge_target is also dormant ( symmetry) + result_clean = map_step_runner._require_clean_merge_target(tmp_path) + assert result_clean["status"] == "dormant" + assert result_clean["ok"] is False + + +def test_probe_roundtrip_and_clean_target(tmp_path: Path) -> None: + """When isolation is 'auto': probe adds+removes the .probe-* worktree in try/finally, + and a dirty repo makes _require_clean_merge_target report not-clean. + + Uses a real git init tmp repo; verifies the .probe-* directory is gone after the + call and that git worktree list shows no leftover probe entries. + """ + import subprocess as _subprocess + + # Build a minimal real git repo with a commit so HEAD exists + repo = tmp_path / "repo" + repo.mkdir() + _subprocess.run(["git", "init", str(repo)], check=True, capture_output=True) + _subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=str(repo), check=True, capture_output=True, + ) + _subprocess.run( + ["git", "config", "user.name", "Test"], + cwd=str(repo), check=True, capture_output=True, + ) + (repo / "README.md").write_text("hello", encoding="utf-8") + _subprocess.run(["git", "add", "."], cwd=str(repo), check=True, capture_output=True) + _subprocess.run( + ["git", "commit", "-m", "init"], + cwd=str(repo), check=True, capture_output=True, + ) + + # Write config with isolation=auto + map_dir = repo / ".map" + map_dir.mkdir() + (map_dir / "config.yaml").write_text("worktree.isolation: auto\n", encoding="utf-8") + + # Change cwd into the repo so _wt_git commands resolve against it + import os as _os + + orig_cwd = Path.cwd() + try: + _os.chdir(repo) + + # Clear probe cache before each probe call so we get a fresh run + map_step_runner._WORKTREE_PROBE_CACHE.clear() + result = map_step_runner._worktree_probe(repo) + finally: + _os.chdir(orig_cwd) + + # The probe should succeed (git worktree add --detach supported in a normal repo) + assert result["status"] == "ok", f"probe expected ok, got {result}" + assert result["ok"] is True + assert result.get("supported") is True + + # Verify no .probe-* worktrees leaked: list the storage root directory + # Storage root = /map-framework/worktrees + git_common_dir_proc = _subprocess.run( + ["git", "rev-parse", "--git-common-dir"], + cwd=str(repo), capture_output=True, text=True, + ) + assert git_common_dir_proc.returncode == 0 + git_common_dir = Path(git_common_dir_proc.stdout.strip()).resolve() + storage_root = git_common_dir / "map-framework" / "worktrees" + + # Either the storage root does not exist, or it contains no .probe-* directories + probe_dirs = list(storage_root.glob(".probe-*")) if storage_root.exists() else [] + assert probe_dirs == [], ( + f"probe worktrees leaked after _worktree_probe: {probe_dirs}" + ) + + # Also confirm via `git worktree list` that no probe worktree remains registered + wl_proc = _subprocess.run( + ["git", "worktree", "list", "--porcelain"], + cwd=str(repo), capture_output=True, text=True, + ) + probe_entries = [ + ln for ln in wl_proc.stdout.splitlines() + if ln.startswith("worktree ") and ".probe-" in ln + ] + assert probe_entries == [], ( + f"probe worktrees still registered in git: {probe_entries}" + ) + + # Now test _require_clean_merge_target with a dirty working tree + try: + _os.chdir(repo) + # Make the repo dirty + (repo / "dirty.txt").write_text("uncommitted", encoding="utf-8") + _subprocess.run(["git", "add", "dirty.txt"], cwd=str(repo), capture_output=True) + # clear probe cache (isolation mode the same, not probe cache, but be safe) + map_step_runner._WORKTREE_PROBE_CACHE.clear() + dirty_result = map_step_runner._require_clean_merge_target(repo) + finally: + _os.chdir(orig_cwd) + + assert dirty_result["ok"] is False, f"expected not-clean, got {dirty_result}" + assert dirty_result["status"] == "dirty" + assert dirty_result.get("kind") == "DIRTY_MERGE_TARGET" + assert len(dirty_result.get("dirty", [])) >= 1 + + +# --------------------------------------------------------------------------- +# resolve_worktree_isolation / cleanup_orphan_worktrees +# --------------------------------------------------------------------------- + + +def _make_git_repo(path: Path) -> None: + """Create a minimal real git repo with one commit at *path*.""" + import subprocess as _sp + + _sp.run(["git", "init", str(path)], check=True, capture_output=True) + _sp.run( + ["git", "config", "user.email", "test@example.com"], + cwd=str(path), check=True, capture_output=True, + ) + _sp.run( + ["git", "config", "user.name", "Test"], + cwd=str(path), check=True, capture_output=True, + ) + (path / "README.md").write_text("hello", encoding="utf-8") + _sp.run(["git", "add", "."], cwd=str(path), check=True, capture_output=True) + _sp.run( + ["git", "commit", "-m", "init"], + cwd=str(path), check=True, capture_output=True, + ) + + +def _write_isolation_config(project_dir: Path, mode: str) -> None: + map_dir = project_dir / ".map" + map_dir.mkdir(exist_ok=True) + (map_dir / "config.yaml").write_text( + f"worktree.isolation: {mode}\n", encoding="utf-8" + ) + + +def test_fallback_auto_vs_required( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC1 [AC-9]: Under isolation=auto each fallback condition degrades gracefully; + under isolation=required the same conditions hard-fail. + + Conditions tested: + - not_git_repo (non_git_dir fixture equivalent: tmp_path with no .git) + - dirty_merge_target (monkeypatched _require_clean_merge_target) + - worktree_create_failed / worktree_unsupported (monkeypatched probe to fail) + """ + import os as _os + + # ----------------------------------------------------------------------- + # Helper: run resolve under a specific isolation mode with given probe/clean + # patches, asserting on expected outcome shape. + # ----------------------------------------------------------------------- + + def _assert_auto_degrades( + project_dir: Path, + expected_reason: str, + ) -> None: + result = map_step_runner.resolve_worktree_isolation(project_dir) + assert result.get("ok") is True, f"auto should ok=True; got {result}" + assert result.get("decision") == "sequential", f"auto must degrade to sequential; got {result}" + assert result.get("degraded") is True, f"auto must set degraded=True; got {result}" + assert result.get("reason") == expected_reason, ( + f"reason mismatch: expected {expected_reason!r}, got {result.get('reason')!r}" + ) + warning = str(result.get("warning", "")) + assert len(warning) > 0, "auto must emit a non-empty warning string" + + def _assert_required_hardfails(project_dir: Path) -> None: + result = map_step_runner.resolve_worktree_isolation(project_dir) + assert result.get("ok") is False, f"required must ok=False; got {result}" + assert result.get("status") == "error", ( + f"required must status=error; got {result.get('status')!r}" + ) + assert result.get("decision") == "abort", ( + f"required must decision=abort; got {result.get('decision')!r}" + ) + + # ----------------------------------------------------------------------- + # Condition 1: not_git_repo — directory without a .git + # ----------------------------------------------------------------------- + non_git = tmp_path / "non_git" + non_git.mkdir() + _write_isolation_config(non_git, "auto") + orig_cwd = Path.cwd() + try: + _os.chdir(non_git) + # _wt_is_git_repo() will return False here (no .git) + _assert_auto_degrades(non_git, map_step_runner._WT_REASON_NOT_GIT_REPO) + finally: + _os.chdir(orig_cwd) + + _write_isolation_config(non_git, "required") + try: + _os.chdir(non_git) + _assert_required_hardfails(non_git) + finally: + _os.chdir(orig_cwd) + + # ----------------------------------------------------------------------- + # Condition 2: dirty_merge_target — real git repo, probe ok, dirty status + # ----------------------------------------------------------------------- + dirty = tmp_path / "dirty_repo" + dirty.mkdir() + _make_git_repo(dirty) + + # Monkeypatch probe to succeed (avoids needing real worktree add support) + # and _require_clean_merge_target to report dirty. + def _probe_ok(_project_dir: object) -> dict[str, object]: + del _project_dir + return {"status": "ok", "ok": True, "supported": True, "is_primary": True} + + def _clean_dirty(_project_dir: object) -> dict[str, object]: + del _project_dir + return { + "status": "dirty", + "ok": False, + "kind": "DIRTY_MERGE_TARGET", + "reason": "main checkout has uncommitted changes", + "dirty": ["M README.md"], + } + + monkeypatch.setattr(map_step_runner, "_worktree_probe", _probe_ok) + monkeypatch.setattr(map_step_runner, "_require_clean_merge_target", _clean_dirty) + + _write_isolation_config(dirty, "auto") + try: + _os.chdir(dirty) + _assert_auto_degrades(dirty, map_step_runner._WT_REASON_DIRTY_MERGE_TARGET) + finally: + _os.chdir(orig_cwd) + + _write_isolation_config(dirty, "required") + try: + _os.chdir(dirty) + _assert_required_hardfails(dirty) + finally: + _os.chdir(orig_cwd) + + # ----------------------------------------------------------------------- + # Condition 3: worktree_create_failed / probe failure + # ----------------------------------------------------------------------- + probe_fail = tmp_path / "probe_fail_repo" + probe_fail.mkdir() + _make_git_repo(probe_fail) + + def _probe_fail(_project_dir: object) -> dict[str, object]: + del _project_dir + return map_step_runner._wt_error( + "WORKTREE_PROBE_FAILED", "git worktree add --detach failed" + ) + + # Restore original _require_clean_merge_target first, then patch probe + def _clean_ok(_project_dir: object) -> dict[str, object]: + del _project_dir + return {"status": "ok", "ok": True} + + monkeypatch.setattr(map_step_runner, "_require_clean_merge_target", _clean_ok) + monkeypatch.setattr(map_step_runner, "_worktree_probe", _probe_fail) + + _write_isolation_config(probe_fail, "auto") + try: + _os.chdir(probe_fail) + _assert_auto_degrades(probe_fail, map_step_runner._WT_REASON_UNSUPPORTED) + finally: + _os.chdir(orig_cwd) + + _write_isolation_config(probe_fail, "required") + try: + _os.chdir(probe_fail) + _assert_required_hardfails(probe_fail) + finally: + _os.chdir(orig_cwd) + + # ----------------------------------------------------------------------- + # Dormant path (isolation=off) — zero git regardless + # ----------------------------------------------------------------------- + dormant = tmp_path / "dormant" + dormant.mkdir() + _write_isolation_config(dormant, "off") + + calls: list[list[str]] = [] + + def _no_git_call(args: list[str], **_kw: object) -> object: + del _kw + calls.append(args) + raise AssertionError(f"_wt_git must not be called when isolation is off; got {args!r}") + + monkeypatch.setattr(map_step_runner, "_wt_git", _no_git_call) + result = map_step_runner.resolve_worktree_isolation(dormant) + assert result["status"] == "dormant" + assert result["decision"] == "sequential" + assert result["ok"] is False + assert calls == [], "No git must run when isolation is off" + + +def test_orphan_cleanup_idempotent( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC2 [AC-9]: cleanup_orphan_worktrees removes orphans, keeps active, is + idempotent (second call removes nothing), and is dormant when isolation=off. + """ + import os as _os + import subprocess as _sp + + # ----------------------------------------------------------------------- + # Build a real git repo so git worktree list works + # ----------------------------------------------------------------------- + repo = tmp_path / "repo" + repo.mkdir() + _make_git_repo(repo) + + _write_isolation_config(repo, "auto") + orig_cwd = Path.cwd() + + # ----------------------------------------------------------------------- + # Resolve storage root for this repo + # ----------------------------------------------------------------------- + try: + _os.chdir(repo) + git_common_dir_proc = _sp.run( + ["git", "rev-parse", "--git-common-dir"], + capture_output=True, text=True, + ) + assert git_common_dir_proc.returncode == 0 + git_common_dir = Path(git_common_dir_proc.stdout.strip()).resolve() + finally: + _os.chdir(orig_cwd) + + storage_root = git_common_dir / "map-framework" / "worktrees" + + # ----------------------------------------------------------------------- + # Create one REGISTERED active worktree entry (manually, no real wt needed) + # and one ORPHAN directory under the storage root. + # ----------------------------------------------------------------------- + branch = "test-cleanup-branch" + branch_slug = "test-cleanup-branch" + + # Active worktree dir (create the dir but NOT register it as orphan) + active_dir = storage_root / branch_slug / "active-ST-001-0" + active_dir.mkdir(parents=True, exist_ok=True) + + # Orphan dir (exists on disk but NOT in the worktree state) + orphan_dir = storage_root / branch_slug / "orphan-ST-999-0" + orphan_dir.mkdir(parents=True, exist_ok=True) + + # Write the worktree state registering ONLY the active worktree + branch_dir = repo / ".map" / branch + branch_dir.mkdir(parents=True, exist_ok=True) + import json as _json + state = { + "schema_version": "1.0", + "branch": branch, + "worktrees": { + "active-ST-001-0": { + "subtask_id": "ST-001", + "slug": "active-ST-001-0", + "attempt": 0, + "branch": "map-wt/active-ST-001-0", + "path": str(active_dir.resolve()), + "base_sha": "abc123", + "status": "created", + } + }, + } + (branch_dir / "worktrees.json").write_text( + _json.dumps(state), encoding="utf-8" + ) + + # ----------------------------------------------------------------------- + # Monkeypatch _wt_force_remove to avoid real git calls on non-git orphan dirs + # ----------------------------------------------------------------------- + removed_by_force: list[str] = [] + + def _fake_force_remove(path: Path, branch_ref: str) -> None: + del branch_ref + removed_by_force.append(str(path.resolve())) + # Simulate removal by deleting the directory + import shutil as _shutil + _shutil.rmtree(str(path), ignore_errors=True) + + monkeypatch.setattr(map_step_runner, "_wt_force_remove", _fake_force_remove) + + try: + _os.chdir(repo) + + # First call: should remove orphan, keep active + result1 = map_step_runner.cleanup_orphan_worktrees(branch) + assert result1["ok"] is True, f"expected ok, got {result1}" + removed1 = result1.get("removed", []) + kept1 = result1.get("kept_active", []) + + orphan_str = str(orphan_dir.resolve()) + active_str = str(active_dir.resolve()) + + assert orphan_str in removed1, ( + f"orphan dir should be in removed list; removed={removed1}" + ) + assert active_str in kept1, ( + f"active dir should be in kept_active list; kept={kept1}" + ) + assert active_str not in removed1, ( + f"active dir must NOT be removed; removed={removed1}" + ) + + # Verify orphan_dir was actually deleted + assert not orphan_dir.exists(), "orphan directory should have been removed" + + # Second call: idempotent — orphan no longer exists, nothing to remove + removed_by_force.clear() + result2 = map_step_runner.cleanup_orphan_worktrees(branch) + assert result2["ok"] is True, f"second call should be ok; got {result2}" + removed2 = result2.get("removed", []) + assert removed2 == [], ( + f"second call should remove nothing (idempotent); removed={removed2}" + ) + + finally: + _os.chdir(orig_cwd) + + # ----------------------------------------------------------------------- + # Dormant when isolation=off: zero git calls + # ----------------------------------------------------------------------- + _write_isolation_config(repo, "off") + + git_calls: list[list[str]] = [] + + def _no_git(args: list[str], **_kw: object) -> object: + del _kw + git_calls.append(args) + raise AssertionError(f"_wt_git must not be called when isolation is off; got {args!r}") + + monkeypatch.setattr(map_step_runner, "_wt_git", _no_git) + + dormant_result = map_step_runner.cleanup_orphan_worktrees(branch) + assert dormant_result["status"] == "dormant", ( + f"expected dormant when off; got {dormant_result}" + ) + assert dormant_result["ok"] is False + assert git_calls == [], "No git must run when isolation is off" diff --git a/tests/test_parallelism_observability.py b/tests/test_parallelism_observability.py new file mode 100644 index 00000000..297dc27f --- /dev/null +++ b/tests/test_parallelism_observability.py @@ -0,0 +1,243 @@ +"""Tests for src/mapify_cli/parallelism_observability.py (ST-011). + +Covers: + - VC1: writer is no-op by default (no file created, returns False) + - VC2: schema and reason-code constants are importable; ALL_REASON_CODES has + all 9 codes; a sample ParallelismReport dict conforms to the TypedDict. + - Parity: worktree reason-code constants match runner's _WT_REASON_* values. +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +# --------------------------------------------------------------------------- +# Suppress bytecode pollution in the generated runner tree (learned rule: +# Test-Induced Bytecode Cache Pollution in Generated Trees). +# --------------------------------------------------------------------------- +sys.dont_write_bytecode = True + +from mapify_cli.parallelism_observability import ( # noqa: E402 + ALL_REASON_CODES, + REASON_DIRTY_MERGE_TARGET, + REASON_DISPATCH_SERIAL, + REASON_MERGE_CONFLICT, + REASON_MONITOR_REJECTED_SUBTASK, + REASON_NOT_GIT_REPO, + REASON_PARALLEL_CAPPED_BY_MAX_ACTORS, + REASON_POST_WAVE_GATE_FAILED, + REASON_WORKTREE_CREATE_FAILED, + REASON_WORKTREE_UNSUPPORTED, + ColorGroupDecision, + ParallelismReport, + write_parallelism_report, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_RUNNER_SCRIPT = ( + Path(__file__).parent.parent + / "src/mapify_cli/templates/map/scripts/map_step_runner.py" +) + + +def _sample_report(run_id: str = "run-test-001") -> ParallelismReport: + """Return a minimal conformant ParallelismReport dict for shape tests.""" + group: ColorGroupDecision = { + "group_id": "wave-1-group-A", + "planned_mode": "sequential", + "actual_mode": "sequential", + "worktree_status": "skipped", + "reason_code": None, + "dispatch_count": 2, + } + report: ParallelismReport = { + "schema_version": "1.0.0", + "run_id": run_id, + "generated_at": "2026-06-29T23:40:00Z", + "total_subtasks": 4, + "total_edges": 3, + "total_waves": 2, + "max_wave_width": 1, + "color_group_breakdown": [group], + } + return report + + +# --------------------------------------------------------------------------- +# writer is no-op by default +# --------------------------------------------------------------------------- + + +def test_writer_noop_by_default(tmp_path: Path) -> None: + """Calling write_parallelism_report with default enabled=False must not + create the output file and must return False.""" + out_path = tmp_path / "parallelism.json" + result = write_parallelism_report(_sample_report(), out_path) + + assert result is False, "write_parallelism_report must return False when disabled" + assert not out_path.exists(), ( + "write_parallelism_report must NOT create the file when enabled=False" + ) + + +def test_writer_noop_explicit_false(tmp_path: Path) -> None: + """Explicit enabled=False also keeps writer dormant.""" + out_path = tmp_path / "runs" / "r1" / "parallelism.json" + result = write_parallelism_report(_sample_report(), out_path, enabled=False) + + assert result is False + assert not out_path.exists() + + +def test_writer_active_when_enabled(tmp_path: Path) -> None: + """Sanity: enabled=True actually writes the file (gates work both ways).""" + out_path = tmp_path / "runs" / "r2" / "parallelism.json" + result = write_parallelism_report(_sample_report("r2"), out_path, enabled=True) + + assert result is True + assert out_path.exists(), "File must be created when enabled=True" + + +# --------------------------------------------------------------------------- +# schema and reason-code constants importable; ALL_REASON_CODES complete +# --------------------------------------------------------------------------- + + +def test_schema_and_reason_codes_importable() -> None: + """ParallelismReport, ColorGroupDecision, and all reason-code constants + import cleanly; ALL_REASON_CODES contains exactly the 9 canonical codes; + a sample dict conforms to the TypedDict shape.""" + # ALL_REASON_CODES completeness + expected_codes = { + REASON_NOT_GIT_REPO, + REASON_WORKTREE_UNSUPPORTED, + REASON_WORKTREE_CREATE_FAILED, + REASON_DIRTY_MERGE_TARGET, + REASON_DISPATCH_SERIAL, + REASON_PARALLEL_CAPPED_BY_MAX_ACTORS, + REASON_MONITOR_REJECTED_SUBTASK, + REASON_MERGE_CONFLICT, + REASON_POST_WAVE_GATE_FAILED, + } + assert len(expected_codes) == 9, "Should have exactly 9 reason codes" + assert ALL_REASON_CODES == expected_codes, ( + f"ALL_REASON_CODES mismatch.\n" + f"Missing: {expected_codes - ALL_REASON_CODES}\n" + f"Extra: {ALL_REASON_CODES - expected_codes}" + ) + + # Sample dict conforms to ParallelismReport TypedDict shape + report = _sample_report() + required_fields = { + "schema_version", + "run_id", + "generated_at", + "total_subtasks", + "total_edges", + "total_waves", + "max_wave_width", + "color_group_breakdown", + } + missing = required_fields - set(report.keys()) + assert not missing, f"Sample report missing fields: {missing}" + + # ColorGroupDecision shape + group = report["color_group_breakdown"][0] + group_required = { + "group_id", + "planned_mode", + "actual_mode", + "worktree_status", + "reason_code", + "dispatch_count", + } + missing_group = group_required - set(group.keys()) + assert not missing_group, f"ColorGroupDecision missing fields: {missing_group}" + + +def test_detection_not_implemented() -> None: + """Detection-by-tool-call-count must NOT be present in the module.""" + import mapify_cli.parallelism_observability as mod + + for attr in dir(mod): + assert "tool_call" not in attr.lower(), ( + f"Unexpected tool-call-count detection attribute found: {attr!r}. " + "Detection is Slice 5 only." + ) + + +# --------------------------------------------------------------------------- +# Parity test: worktree reason codes match runner's _WT_REASON_* constants +# --------------------------------------------------------------------------- + + +def _load_runner_module() -> object: + """Import the rendered runner script as a module (bytecode-free). + + The runner does ``from map_utils import get_branch_name`` at module level, + which fails outside an installed MAP project. We inject a stub into + sys.modules before exec so the import resolves without a real map_utils. + The stub is cleaned up after exec to avoid polluting the test session. + """ + if not _RUNNER_SCRIPT.exists(): + pytest.skip(f"Runner script not found: {_RUNNER_SCRIPT}") + + import types + + # Stub out map_utils so the runner's top-level import doesn't abort. + stub = types.ModuleType("map_utils") + stub.get_branch_name = lambda *a, **kw: "stub" # type: ignore[attr-defined] + injected = "map_utils" not in sys.modules + if injected: + sys.modules["map_utils"] = stub + + try: + spec = importlib.util.spec_from_file_location( + "_map_step_runner_parity", _RUNNER_SCRIPT + ) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + # Suppress bytecode in the generated tree (learned rule) + if mod.__spec__ is not None: + mod.__spec__.cached = None + try: + spec.loader.exec_module(mod) # type: ignore[union-attr] + except SystemExit: + pass # runner may call sys.exit at module level in some guard paths + return mod + finally: + if injected: + sys.modules.pop("map_utils", None) + + +def test_worktree_reason_codes_match_runner() -> None: + """The observability module's worktree reason-code constants must equal the + runner's _WT_REASON_* constants to prevent silent drift (contract-first).""" + runner = _load_runner_module() + + parity_pairs = [ + (REASON_NOT_GIT_REPO, "_WT_REASON_NOT_GIT_REPO"), + (REASON_WORKTREE_UNSUPPORTED, "_WT_REASON_UNSUPPORTED"), + (REASON_WORKTREE_CREATE_FAILED, "_WT_REASON_CREATE_FAILED"), + (REASON_DIRTY_MERGE_TARGET, "_WT_REASON_DIRTY_MERGE_TARGET"), + ] + + for obs_value, runner_attr in parity_pairs: + runner_value = getattr(runner, runner_attr, None) + assert runner_value is not None, ( + f"Runner is missing constant {runner_attr!r} — " + "was ST-009 merged correctly?" + ) + assert obs_value == runner_value, ( + f"Reason-code drift detected!\n" + f" observability.{obs_value!r}\n" + f" runner.{runner_attr} = {runner_value!r}\n" + "Update the observability module or the runner to restore parity." + ) diff --git a/tests/test_wave_eval_harness.py b/tests/test_wave_eval_harness.py new file mode 100644 index 00000000..6ca53a17 --- /dev/null +++ b/tests/test_wave_eval_harness.py @@ -0,0 +1,205 @@ +""" +ST-005 eval/regression harness: + - VC1: compute_waves + split_wave_by_file_conflicts shape assertions for + linear_chain, two_wave_parallel, conflict_split fixtures. + - VC2: With a default (no-new-key) config, _execution_wave_mode == 'off' AND + _worktree_isolation_mode == 'off', proving HC-1 behavior-neutrality. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +# --------------------------------------------------------------------------- +# Runner import — identical pattern to tests/test_map_step_runner.py +# --------------------------------------------------------------------------- + +SCRIPTS_PATH = ( + Path(__file__).resolve().parents[1] + / "src" + / "mapify_cli" + / "templates" + / "map" + / "scripts" +) + +sys.path.insert(0, str(SCRIPTS_PATH)) + +import map_step_runner # noqa: E402 # type: ignore[import-not-found] + +# --------------------------------------------------------------------------- +# Fixtures import +# --------------------------------------------------------------------------- + +from tests.fixtures.wave_blueprints import ( # noqa: E402 + BlueprintFixture, + conflict_split, + linear_chain, + two_wave_parallel, +) + +# --------------------------------------------------------------------------- +# DependencyGraph import (same package as fixtures use) +# --------------------------------------------------------------------------- + +from mapify_cli.dependency_graph import DependencyGraph # noqa: E402 + + +# --------------------------------------------------------------------------- +# wave / color-group shape assertions +# --------------------------------------------------------------------------- + + +def test_wave_color_computation() -> None: + """ + VC1 [AC-5]: compute_waves + split_wave_by_file_conflicts produce the + expected wave/color-group shapes for the three blueprint fixtures. + """ + + # ---- linear_chain: 4 waves each of width 1 ---------------------------- + lc: BlueprintFixture = linear_chain() + graph: DependencyGraph = lc.build_graph() + waves = graph.compute_waves() + + assert waves is not None, "linear_chain must not have a cycle" + assert len(waves) == 4, f"expected 4 waves, got {len(waves)}: {waves}" + for i, wave in enumerate(waves): + assert len(wave) == 1, ( + f"linear_chain wave {i} must have width 1, got {len(wave)}: {wave}" + ) + + # ---- two_wave_parallel: wave 1 has width >= 2 with disjoint files ----- + tp: BlueprintFixture = two_wave_parallel() + tp_graph: DependencyGraph = tp.build_graph() + tp_waves = tp_graph.compute_waves() + + assert tp_waves is not None, "two_wave_parallel must not have a cycle" + assert len(tp_waves) == 2, f"expected 2 waves, got {len(tp_waves)}: {tp_waves}" + + parallel_wave = tp_waves[1] + assert len(parallel_wave) >= 2, ( + f"two_wave_parallel wave 1 must have width >= 2, got {len(parallel_wave)}: {parallel_wave}" + ) + + # Verify members have disjoint affected_files + seen_files: set[str] = set() + for subtask_id in parallel_wave: + subtask_files = tp.affected_files_map[subtask_id] + overlap = seen_files & subtask_files + assert not overlap, ( + f"two_wave_parallel wave 1 members share files: {subtask_id} overlaps {overlap}" + ) + seen_files |= subtask_files + + # Confirm split_wave_by_file_conflicts leaves wave 1 intact (no conflicts) + sub_waves = tp_graph.split_wave_by_file_conflicts(parallel_wave, tp.affected_files_map) + assert len(sub_waves) == 1, ( + f"disjoint files: split_wave_by_file_conflicts should produce 1 sub-wave, " + f"got {len(sub_waves)}: {sub_waves}" + ) + assert sorted(sub_waves[0]) == sorted(parallel_wave), ( + f"sub-wave members must equal original wave members: {sub_waves[0]} != {parallel_wave}" + ) + + # ---- conflict_split: shared-file pair ends up in different sub-waves -- + cs: BlueprintFixture = conflict_split() + cs_graph: DependencyGraph = cs.build_graph() + cs_waves = cs_graph.compute_waves() + + assert cs_waves is not None, "conflict_split must not have a cycle" + assert len(cs_waves) == 2, f"expected 2 waves, got {len(cs_waves)}: {cs_waves}" + + conflict_wave = cs_waves[1] + # are all in wave 1 + assert sorted(conflict_wave) == ["ST-002", "ST-003", "ST-004"], ( + f"conflict wave members mismatch: {conflict_wave}" + ) + + sub_waves_cs = cs_graph.split_wave_by_file_conflicts( + conflict_wave, cs.affected_files_map + ) + # Must produce at least 2 sub-waves because and share 'src/shared.py' + assert len(sub_waves_cs) >= 2, ( + f"conflict_split must produce >= 2 sub-waves, got {len(sub_waves_cs)}: {sub_waves_cs}" + ) + + # and must be in different sub-waves + def _sub_wave_index(subtask_id: str) -> int: + for idx, sw in enumerate(sub_waves_cs): + if subtask_id in sw: + return idx + raise AssertionError(f"{subtask_id!r} not found in any sub-wave: {sub_waves_cs}") + + assert _sub_wave_index("ST-002") != _sub_wave_index("ST-004"), ( + f"ST-002 and ST-004 share 'src/shared.py' and must be in different sub-waves; " + f"sub_waves={sub_waves_cs}" + ) + + +# --------------------------------------------------------------------------- +# default config selects the sequential / legacy path +# --------------------------------------------------------------------------- + + +def test_default_config_selects_sequential(tmp_path: Path) -> None: + """ + VC2 [AC-5] [SC-2]: With a default (no-new-key) config, the canonical + MapConfig defaults apply: _execution_wave_mode == 'auto' and + _worktree_isolation_mode == 'off'. Behavior stays neutral (HC-1) because + the wave-loop is gated on worktree.isolation != 'off' — which is 'off' by + default — so the legacy sequential path is selected regardless of wave_mode. + + Also verifies the color-group concurrency predicate: the condition that + WOULD contribute to wave-mode (any color group of width >= 2) exists in the + two_wave_parallel fixture, but the isolation gate keeps the legacy path. + """ + # Case A: no .map directory at all + assert map_step_runner._execution_wave_mode(tmp_path) == "auto", ( + "_execution_wave_mode must default to 'auto' (MapConfig) when .map dir is absent" + ) + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off", ( + "_worktree_isolation_mode must return 'off' when .map dir is absent" + ) + + # Case B: .map/config.yaml exists but contains NO new keys + map_dir = tmp_path / ".map" + map_dir.mkdir() + (map_dir / "config.yaml").write_text( + "minimality: off\n", # existing key only — no wave_mode or isolation keys + encoding="utf-8", + ) + + assert map_step_runner._execution_wave_mode(tmp_path) == "auto", ( + "_execution_wave_mode must default to 'auto' when execution.wave_mode key is absent" + ) + assert map_step_runner._worktree_isolation_mode(tmp_path) == "off", ( + "_worktree_isolation_mode must return 'off' when worktree.isolation key is absent" + ) + + # Show the concurrency predicate (color group width >= 2) WOULD be true + # for the two_wave_parallel fixture, but wave_mode='off' gates around it. + tp: BlueprintFixture = two_wave_parallel() + tp_graph: DependencyGraph = tp.build_graph() + tp_waves = tp_graph.compute_waves() + + assert tp_waves is not None + color_groups = [ + tp_graph.split_wave_by_file_conflicts(w, tp.affected_files_map) for w in tp_waves + ] + # At least one color group has width >= 2 (the parallel wave) + assert any(len(g) >= 2 for groups in color_groups for g in groups), ( + "two_wave_parallel must expose at least one color group of width >= 2 " + "(the concurrency predicate gate)" + ) + + # wave_mode defaults to 'auto' and a width>=2 group exists, but the isolation + # gate (worktree.isolation == 'off' by default) keeps the legacy sequential + # path — that is what makes the default behavior-neutral. + wave_mode = map_step_runner._execution_wave_mode(tmp_path) + isolation = map_step_runner._worktree_isolation_mode(tmp_path) + assert wave_mode == "auto" and isolation == "off", ( + f"default config: wave_mode='auto', isolation='off'; got {wave_mode!r}/{isolation!r}" + ) + # The wave-loop predicate requires isolation != 'off', so default => sequential. + assert isolation == "off", "isolation gate must hold the legacy path by default" diff --git a/tests/test_wave_fixtures.py b/tests/test_wave_fixtures.py new file mode 100644 index 00000000..0dd601f1 --- /dev/null +++ b/tests/test_wave_fixtures.py @@ -0,0 +1,214 @@ +""" +Validation tests for the wave_blueprints fixture module (ST-004). + +VC1: Every fixture constructs without error and exposes required fields. +VC2: Fixture module source contains no TODO/FIXME/... placeholders. +""" + +from __future__ import annotations + +import ast +import shutil +import subprocess +from pathlib import Path + +import pytest + +from tests.fixtures.wave_blueprints import ( + BlueprintFixture, + SubtaskSpec, + conflict_split, + dirty_repo, + linear_chain, + non_git_dir, + shallow_clone, + submodule_repo, + two_wave_parallel, +) + +_FIXTURE_SRC = Path(__file__).parent / "fixtures" / "wave_blueprints.py" + + +# --------------------------------------------------------------------------- +# — all fixtures loadable and expose documented fields +# --------------------------------------------------------------------------- + + +class TestAllFixturesLoadable: + """VC1: Each fixture constructs without error and exposes the documented fields.""" + + # --- blueprint-shape fixtures --- + + def _assert_blueprint_fields(self, fix: BlueprintFixture) -> None: + assert isinstance(fix, BlueprintFixture) + assert isinstance(fix.subtasks, list) + assert len(fix.subtasks) >= 1 + assert isinstance(fix.affected_files_map, dict) + for spec in fix.subtasks: + assert isinstance(spec, SubtaskSpec) + assert isinstance(spec.id, str) and spec.id + assert isinstance(spec.dependencies, list) + assert isinstance(spec.outputs, list) + assert isinstance(spec.inputs, list) + assert isinstance(spec.affected_files, set) + + def test_all_fixtures_loadable_linear_chain(self) -> None: + fix = linear_chain() + self._assert_blueprint_fields(fix) + assert len(fix.subtasks) == 4, "linear_chain must have 4 subtasks" + ids = [s.id for s in fix.subtasks] + assert ids == ["ST-001", "ST-002", "ST-003", "ST-004"] + # verify chain structure + assert fix.subtasks[0].dependencies == [] + assert fix.subtasks[1].dependencies == ["ST-001"] + assert fix.subtasks[2].dependencies == ["ST-002"] + assert fix.subtasks[3].dependencies == ["ST-003"] + + def test_all_fixtures_loadable_two_wave_parallel(self) -> None: + fix = two_wave_parallel() + self._assert_blueprint_fields(fix) + root = fix.subtasks[0] + children = fix.subtasks[1:] + assert root.dependencies == [] + for child in children: + assert root.id in child.dependencies + # verify disjoint affected_files among children + child_file_sets = [c.affected_files for c in children] + for i, a in enumerate(child_file_sets): + for j, b in enumerate(child_file_sets): + if i != j: + assert not (a & b), ( + f"two_wave_parallel: children {i} and {j} share files — " + "wave must be truly parallel" + ) + + def test_all_fixtures_loadable_conflict_split(self) -> None: + fix = conflict_split() + self._assert_blueprint_fields(fix) + # must have at least one shared file among the parallel subtasks + parallel_ids = [s.id for s in fix.subtasks if s.dependencies != []] + all_files: list[set[str]] = [fix.affected_files_map[sid] for sid in parallel_ids] + found_conflict = any( + bool(all_files[i] & all_files[j]) + for i in range(len(all_files)) + for j in range(i + 1, len(all_files)) + ) + assert found_conflict, "conflict_split must have at least one shared file" + + def test_all_fixtures_loadable_build_graph(self) -> None: + """build_graph() works for all three blueprint fixtures.""" + from mapify_cli.dependency_graph import DependencyGraph + + for factory in (linear_chain, two_wave_parallel, conflict_split): + fix = factory() + graph = fix.build_graph() + assert isinstance(graph, DependencyGraph) + assert graph.size() == len(fix.subtasks) + + # --- git-shape fixtures --- + + def test_all_fixtures_loadable_non_git_dir(self, tmp_path: Path) -> None: + if shutil.which("git") is None: + pytest.skip("git not available") + repo = non_git_dir(tmp_path) + assert repo.is_dir() + # must NOT be a git repo + result = subprocess.run( + ["git", "rev-parse", "--git-dir"], + cwd=repo, + capture_output=True, + text=True, + ) + assert result.returncode != 0, "non_git_dir must not be a git repository" + + def test_all_fixtures_loadable_shallow_clone(self, tmp_path: Path) -> None: + repo = shallow_clone(tmp_path) + assert repo.is_dir() + # must be a git repo + result = subprocess.run( + ["git", "rev-parse", "--git-dir"], + cwd=repo, + capture_output=True, + text=True, + ) + assert result.returncode == 0, "shallow_clone must be a valid git repo" + # shallow marker file must exist + shallow_file = repo / ".git" / "shallow" + assert shallow_file.exists(), ".git/shallow marker must exist for shallow_clone" + + def test_all_fixtures_loadable_submodule_repo(self, tmp_path: Path) -> None: + repo = submodule_repo(tmp_path) + assert repo.is_dir() + # .gitmodules must exist + assert (repo / ".gitmodules").exists(), "submodule_repo must have .gitmodules" + # git submodule status must succeed + result = subprocess.run( + ["git", "submodule", "status"], + cwd=repo, + capture_output=True, + text=True, + ) + assert result.returncode == 0, f"git submodule status failed: {result.stderr}" + + def test_all_fixtures_loadable_dirty_repo(self, tmp_path: Path) -> None: + repo = dirty_repo(tmp_path) + assert repo.is_dir() + result = subprocess.run( + ["git", "status", "--porcelain"], + cwd=repo, + capture_output=True, + text=True, + ) + assert result.returncode == 0 + assert result.stdout.strip(), ( + "dirty_repo must have uncommitted changes (git status --porcelain non-empty)" + ) + + +# --------------------------------------------------------------------------- +# — no placeholder TODO/FIXME/... values in the fixture module +# --------------------------------------------------------------------------- + + +class TestFixturesNoPlaceholders: + """VC2: Fixture source has no TODO/FIXME/ellipsis placeholders.""" + + def test_fixtures_no_placeholders(self) -> None: + source = _FIXTURE_SRC.read_text() + forbidden = ["TODO", "FIXME"] + for token in forbidden: + assert token not in source, ( + f"Placeholder '{token}' found in {_FIXTURE_SRC}; " + "fixtures must not contain placeholder values" + ) + + # Check for bare ellipsis used as a body placeholder (not in docstrings) + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant): + if node.value.value is ...: + raise AssertionError( + f"Bare ellipsis placeholder found at line {node.lineno} " + f"in {_FIXTURE_SRC}" + ) + + def test_fixtures_blueprint_affected_files_nonempty(self) -> None: + """Blueprint fixtures' affected_files are non-empty for each subtask.""" + for factory in (linear_chain, two_wave_parallel, conflict_split): + fix = factory() + for spec in fix.subtasks: + assert spec.affected_files, ( + f"{factory.__name__}: subtask {spec.id} has empty affected_files; " + "lint and wave computations require at least one file" + ) + + def test_fixtures_affected_files_map_matches_subtasks(self) -> None: + """affected_files_map keys match subtask ids for all blueprint fixtures.""" + for factory in (linear_chain, two_wave_parallel, conflict_split): + fix = factory() + spec_ids = {s.id for s in fix.subtasks} + map_ids = set(fix.affected_files_map.keys()) + assert spec_ids == map_ids, ( + f"{factory.__name__}: affected_files_map keys {map_ids} " + f"do not match subtask ids {spec_ids}" + )