diff --git a/.agents/skills/map-efficient/SKILL.md b/.agents/skills/map-efficient/SKILL.md index 22ca5426..7ab5e4e4 100644 --- a/.agents/skills/map-efficient/SKILL.md +++ b/.agents/skills/map-efficient/SKILL.md @@ -13,7 +13,11 @@ final verifier unless an explicit subagent dispatch is available and useful. Use [efficient-reference.md](efficient-reference.md) for wave details, retry recipes, TDD mode, commit policy, and troubleshooting. Read only the referenced -section when the workflow below points to it. +section when the workflow below points to it. Under `isolation_active` (Slice 5a), +the wave-loop creates per-member worktrees, dispatches actor subagents +**sequentially** (one per turn), verifies via `concurrency_ready`, then accepts +atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b +(`dispatch_mode==concurrent`). ## Mutation Boundary Constraints diff --git a/.agents/skills/map-efficient/efficient-reference.md b/.agents/skills/map-efficient/efficient-reference.md index 460d9be7..fb3597db 100644 --- a/.agents/skills/map-efficient/efficient-reference.md +++ b/.agents/skills/map-efficient/efficient-reference.md @@ -113,7 +113,9 @@ wave-loop on every run. The wave-loop engages **only when ALL THREE hold** **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+). +wave-loop engages, dispatch remains **sequential** in Slice 5a (`isolation_active=True`, +`dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out +is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -136,10 +138,16 @@ 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 -merge advances HEAD and the next trips `BASE_DIVERGED`): +When `worktree.isolation` is enabled and a wave has ≥2 disjoint subtasks +(`isolation_active=True`), execute the **Slice 5a sequential worktree flow**: + +1. **Create** a worktree per wave member via `create_subtask_worktree`. +2. **Dispatch actor subagents sequentially** — one per turn (`HC-3`), each + pinned to its own worktree path. Do NOT dispatch all in one turn (that is + Slice 5b / `dispatch_mode==concurrent`). +3. **Verify** all member worktrees with `concurrency_ready` before merge. +4. **Accept atomically** — never merge one at a time (the first merge advances + HEAD and the next trips `BASE_DIVERGED`): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" @@ -150,23 +158,24 @@ 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 +### Concurrent actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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**: +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one actor subagent per turn, each +> pinned to its own worktree. The example below is reference material for when +> Slice 5b ships; do NOT treat it as an active instruction now. Use your runtime's +> own parallel actor-subagent dispatch mechanism — this is the provider-neutral +> shape, not a literal API call. + +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' 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) @@ -180,12 +189,14 @@ dispatch actor subagent -> ST-004 (pinned to its own worktree) **`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are pre-split into sequential batches before dispatch. -### Anti-patterns +### Anti-patterns — Slice 5b concurrent dispatch only + +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one actor dispatch per turn **is** the correct behavior. -- 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. +- One actor dispatch per turn across N turns — serial loop, no concurrency. (Slice 5b only — expected, correct behavior in 5a.) +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. (Slice 5b only.) +- Waiting for one actor result before dispatching the next. (Correct for 5a, wrong for 5b.) +- Mixing `get_next_step` and `get_wave_step` for the same wave. (Applies to both 5a and 5b.) ## TDD Mode diff --git a/.claude/skills/map-efficient/SKILL.md b/.claude/skills/map-efficient/SKILL.md index 7e1da6f5..efb7b7fc 100644 --- a/.claude/skills/map-efficient/SKILL.md +++ b/.claude/skills/map-efficient/SKILL.md @@ -207,7 +207,7 @@ else fi ``` -**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. +**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. Under `isolation_active` (Slice 5a), the wave-loop creates per-member worktrees, dispatches Actors **sequentially** (one per turn, `HC-3`), verifies via `concurrency_ready`, then accepts atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`). 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 diff --git a/.claude/skills/map-efficient/efficient-reference.md b/.claude/skills/map-efficient/efficient-reference.md index 76b9554a..f855be02 100644 --- a/.claude/skills/map-efficient/efficient-reference.md +++ b/.claude/skills/map-efficient/efficient-reference.md @@ -136,7 +136,7 @@ including clean passes — must carry concrete evidence references. | `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`). +**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 remains **sequential** in Slice 5a (`isolation_active=True`, `dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -146,22 +146,23 @@ Use `get_next_step` for all sequential (default) execution. One phase at a time, 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. +When the wave-loop engages AND `isolation_active` is true (`worktree.isolation` ∈ {`auto`, `required`}), the Slice 5a flow applies: (a) create a worktree per wave member via `create_subtask_worktree`; (b) dispatch the member Actors **sequentially** — one per turn, each pinned to its own worktree path (`HC-3`); (c) call `concurrency_ready` (ST-003) to verify all member worktrees before merge; (d) accept the whole wave atomically via `merge_wave_worktrees` — never one-at-a-time, with whole-wave rollback on any failure. See [Parallel waves](#worktree-isolation) under Worktree isolation for the full protocol. Concurrent fan-out (dispatching all Actors in one message) is Slice 5b (`dispatch_mode==concurrent`) and is not yet active. -### Concurrent Actor dispatch — GATED EXAMPLE +### Concurrent Actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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. +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one Actor per turn, each pinned to +> its own worktree. The example below is reference material for when Slice 5b 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: +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' only) — N Task calls in one message: Task( subagent_type="actor", description="Implement ST-003", @@ -184,12 +185,14 @@ Task( **`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) +### Anti-patterns — Slice 5b concurrent dispatch only -- **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. +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one Task per turn **is** the correct behavior — the first three below are NOT anti-patterns there. + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. (Slice 5b only — this is the expected, correct behavior in 5a.) +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. (Slice 5b only.) +- **Waiting for one actor result before dispatching the next** — correct for sequential dispatch (5a), wrong for concurrent waves (5b). +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. (Applies to both 5a and 5b.) ### Actor-boundary prompt template (worktree-isolated subtasks) @@ -653,12 +656,20 @@ with `worktree_isolation_status`. ### Parallel waves (≥2 worktree-isolated subtasks) — #284 Phase 2 When `get_wave_step` returns `mode:"parallel"` (a wave with ≥2 disjoint-file -subtasks) AND isolation is enabled, give EACH subtask its own worktree and -dispatch the Actors concurrently (separate Task agents, each pinned to its own -`$WT_PATH`). Do NOT merge them one at a time: every worktree was cut off the same -HEAD, so the first `merge_subtask_worktree` advances the working branch and the -next trips `BASE_DIVERGED`. Accept the whole wave atomically instead — only after -EVERY subtask in the wave has passed Monitor (+ Evaluator): +subtasks) AND `isolation_active` is true, execute the **Slice 5a sequential +worktree flow**: + +1. **Create** a worktree per wave member: `create_subtask_worktree` for each. +2. **Dispatch Actors sequentially** — one per turn (`HC-3`), each pinned to its + own `$WT_PATH`. Do NOT dispatch all in one message (that is Slice 5b). +3. **Verify** all member worktrees with `concurrency_ready` (ST-003) before merge. +4. **Accept atomically** via `merge_wave_worktrees` after every subtask passes + Monitor (+ Evaluator) — never merge one at a time. + +Do NOT merge them one at a time: every worktree was cut off the same HEAD, so +the first `merge_subtask_worktree` advances the working branch and the next trips +`BASE_DIVERGED`. Accept the whole wave atomically instead — only after EVERY +subtask in the wave has passed Monitor (+ Evaluator): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" "$ST_C" diff --git a/.map/scripts/map_orchestrator.py b/.map/scripts/map_orchestrator.py index 7d02035b..e2dad2c4 100755 --- a/.map/scripts/map_orchestrator.py +++ b/.map/scripts/map_orchestrator.py @@ -251,6 +251,11 @@ def _extract_subtask_ids_from_plan_artifacts( # concurrent Task dispatch is actually implemented and safe to enable. WAVE_CONCURRENCY_ENABLED = False +# Stable reason codes for get_wave_step return sites (ST-002). +WAVE_REASON_NO_WAVES = "no_waves" +WAVE_REASON_WAVE_COMPLETE = "wave_complete" +WAVE_REASON_DISPATCH_SEQUENTIAL = "dispatch_sequential_5a" + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2298,13 +2303,26 @@ def get_wave_step(branch: str) -> dict: state_file = Path(f".map/{branch}/step_state.json") state = StepState.load(state_file) + # Compute structured dispatch signal fields (ST-002). + dispatch_mode = "concurrent" if WAVE_CONCURRENCY_ENABLED else "sequential" + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _worktree_isolation_mode, + ) + isolation_active = _worktree_isolation_mode(Path(".")) != "off" + except ImportError: + isolation_active = False + if not state.execution_waves: return { "mode": "sequential", "wave_index": 0, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_NO_WAVES, "message": "No execution waves configured. Use sequential mode.", } @@ -2314,7 +2332,10 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_WAVE_COMPLETE, } wave = state.execution_waves[state.current_wave_index] @@ -2353,7 +2374,10 @@ def get_wave_step(branch: str) -> dict: "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, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_DISPATCH_SEQUENTIAL, } @@ -2507,12 +2531,18 @@ def select_execution_strategy( reason = "no color-group with width>=2 → sequential (all width-1 waves)" strategy = "sequential" + concurrency_allowed = ( + strategy == "wave_loop" + and isolation_mode in {"auto", "required"} + and has_parallel_groups + ) return { "strategy": strategy, "wave_mode": wave_mode, "worktree_isolation": isolation_mode, "has_parallel_groups": has_parallel_groups, "reason": reason, + "concurrency_allowed": concurrency_allowed, } @@ -4418,6 +4448,7 @@ def main(): "get_wave_step", "validate_wave_step", "advance_wave", + "select_execution_strategy", "resume_single_subtask", "get_plan_progress", "monitor_failed", @@ -4804,6 +4835,10 @@ def main(): result = get_wave_step(branch) print(json.dumps(result, indent=2)) + elif args.command == "select_execution_strategy": + result = select_execution_strategy(branch) + print(json.dumps(result, indent=2)) + elif args.command == "validate_wave_step": if not args.task_or_step: print( diff --git a/.map/scripts/map_step_runner.py b/.map/scripts/map_step_runner.py index 4188c256..a343f61a 100755 --- a/.map/scripts/map_step_runner.py +++ b/.map/scripts/map_step_runner.py @@ -15342,6 +15342,12 @@ def _wt_force_remove(path: Path, branch_ref: str) -> None: _WT_REASON_UNSUPPORTED: str = "worktree_unsupported" _WT_REASON_CREATE_FAILED: str = "worktree_create_failed" _WT_REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" +# concurrency_ready reason codes +_WT_REASON_NO_RECORD: str = "no_record" +_WT_REASON_PATH_MISSING: str = "path_missing" +_WT_REASON_NOT_REGISTERED: str = "not_registered" +_WT_REASON_HEAD_MISMATCH: str = "head_mismatch" +_WT_REASON_DIRTY: str = "dirty" _WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) @@ -16752,6 +16758,154 @@ def worktree_isolation_status(branch: Optional[str] = None) -> dict[str, object] } +def concurrency_ready( + subtask_ids: list[str], + branch: Optional[str] = None, +) -> dict[str, object]: + """Read-only wave-worktree readiness check (coordinator-owned, council Q1). + + For each supplied subtask verifies: + 1. A worktree record exists in the branch-scoped sidecar. + 2. The recorded path exists on disk AND is git-registered (appears in + ``git worktree list --porcelain``). + 3. HEAD in the worktree == the recorded base_sha (frozen-SHA invariant #284). + 4. The worktree tree is clean, ignoring MAP runtime-state paths. + + Returns:: + + { + "ready": bool, + "per_subtask": {sid: {"ok": bool, "reason": code | None, ...}}, + "reason": | None, + } + + This function NEVER creates, merges, removes, or commits anything. + When not inside a git repo or no worktrees are recorded the function + returns a structured non-error result (ready=False) rather than raising. + """ + if not _wt_is_git_repo(): + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NOT_GIT_REPO, + } + + branch_name = branch or get_branch_name() + state = _read_worktree_state(branch_name) + worktrees = state.get("worktrees", {}) + if not isinstance(worktrees, dict) or not worktrees: + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NO_RECORD, + } + + # Build a set of paths registered with git for O(1) membership checks. + registered_paths: set[str] = set() + 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 "): + try: + registered_paths.add( + str(Path(line[len("worktree "):].strip()).resolve()) + ) + except OSError: + pass + + per_subtask: dict[str, object] = {} + first_failure: Optional[str] = None + + ids = sorted({str(s) for s in subtask_ids if str(s).strip()}) + for sid in ids: + slug = _wt_slug(sid) + if slug is None: + reason = "invalid_subtask_id" + per_subtask[sid] = {"ok": False, "reason": reason} + if first_failure is None: + first_failure = reason + continue + + record = worktrees.get(slug) + if not isinstance(record, dict): + per_subtask[sid] = {"ok": False, "reason": _WT_REASON_NO_RECORD} + if first_failure is None: + first_failure = _WT_REASON_NO_RECORD + continue + + raw_path = str(record.get("path", "")) + base_sha = str(record.get("base_sha", "")) + wt_path = Path(raw_path) if raw_path else None + + # (1) Path exists on disk + if wt_path is None or not wt_path.is_dir(): + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_PATH_MISSING, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_PATH_MISSING + continue + + # (2) git-registered + try: + resolved = str(wt_path.resolve()) + except OSError: + resolved = raw_path + if resolved not in registered_paths: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_NOT_REGISTERED, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_NOT_REGISTERED + continue + + # (3) HEAD == base_sha + head = _wt_head_sha(wt_path) + if head != base_sha: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_HEAD_MISMATCH, + "expected": base_sha[:8] if base_sha else None, + "actual": head[:8] if head else None, + } + if first_failure is None: + first_failure = _WT_REASON_HEAD_MISMATCH + continue + + # (4) Clean tree (ignoring MAP runtime-state paths) + st = _wt_git(["status", "--porcelain"], cwd=wt_path) + dirty_lines = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty_lines: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_DIRTY, + "dirty": dirty_lines[:10], + } + if first_failure is None: + first_failure = _WT_REASON_DIRTY + continue + + per_subtask[sid] = {"ok": True, "reason": None} + + ready = bool(ids) and all( + isinstance(v, dict) and v.get("ok") for v in per_subtask.values() + ) + return { + "ready": ready, + "per_subtask": per_subtask, + "reason": first_failure, + } + + if __name__ == "__main__": # Simple CLI interface for testing import sys @@ -18219,6 +18373,20 @@ def _flag_val(name: str) -> Optional[str]: if _wt_r.get("status") == "error": sys.exit(1) + elif func_name == "concurrency_ready": + # CLI: concurrency_ready [ ...] [--branch B] + # Coordinator-owned read-only wave-worktree readiness check (council Q1). + # Returns JSON; exits 0 even when ready=False (a structural result, not an + # error); exits 1 only on argument/parse errors. + import argparse as _ap + + _p = _ap.ArgumentParser(prog="map_step_runner.py concurrency_ready") + _p.add_argument("subtask_ids", nargs="+") + _p.add_argument("--branch", default=None) + _a = _p.parse_args(sys.argv[2:]) + _wt_r = concurrency_ready(_a.subtask_ids, _a.branch) + print(json.dumps(_wt_r, indent=2)) + elif func_name == "discard_subtask_worktree": # CLI: discard_subtask_worktree [--attempt N] [--branch B] # [--save-patch] diff --git a/src/mapify_cli/config/project_config.py b/src/mapify_cli/config/project_config.py index 6f43b80f..7c2d9de9 100644 --- a/src/mapify_cli/config/project_config.py +++ b/src/mapify_cli/config/project_config.py @@ -194,6 +194,35 @@ class MapConfig: # bare `off`/`on` as booleans — load_map_config migrates them to strings. execution_wave_mode: str = "auto" + # Maximum number of concurrent Actor workers in a parallel wave dispatch (#303 + # Slice 5b). Range 1–8; out-of-range values are clamped by clamp_max_actors(). + # Non-int / bool values fall back to the default 4 (see clamp_max_actors). + # Dotted YAML key: `execution.max_actors`. + # DORMANT in Slice 5a — parsed and validated but no execution path reads it yet. + max_actors: int = 4 + + # When True, a single worker that crashes with a transient error in a parallel + # wave will be retried once before the wave is aborted. Pairs with max_actors + # in Slice 5b concurrent dispatch. Dotted YAML key: `execution.retry_degraded_once`. + # DORMANT in Slice 5a — parsed and validated but no execution path reads it yet. + retry_degraded_once: bool = False + + +def clamp_max_actors(n: object) -> int: + """Clamp max_actors to the valid range [1, 8], or return the default 4. + + Non-int values (including bool, str, None) return the default 4. + int values are clamped: below 1 → 1, above 8 → 8. + + Note: bool is explicitly excluded (isinstance(True, int) is True in Python) + because a YAML boolean arriving here is a misconfiguration, not an int. + The floor is 1, NOT the default 4 — a valid-but-low int (e.g. 0) is clamped + to 1 (minimum legal value), while a non-int/bool/None falls back to 4. + """ + if isinstance(n, bool) or not isinstance(n, int): + return 4 + return max(1, min(8, n)) + def load_map_config(project_path: Path) -> MapConfig: """Load MAP config from .map/config.yaml with fallback to defaults. @@ -264,6 +293,8 @@ def load_map_config(project_path: Path) -> MapConfig: ("worktree.isolation", "worktree_isolation"), ("worktree.max_deletions", "worktree_max_deletions"), ("execution.wave_mode", "execution_wave_mode"), + ("execution.max_actors", "max_actors"), + ("execution.retry_degraded_once", "retry_degraded_once"), ): if dotted in data and field_name not in data: data[field_name] = data.pop(dotted) @@ -404,6 +435,10 @@ def load_map_config(project_path: Path) -> MapConfig: ) cfg.execution_wave_mode = "auto" + # Clamp max_actors to [1, 8]; non-int/bool → default 4. + # retry_degraded_once is a plain bool handled by the generic type-check loop. + cfg.max_actors = clamp_max_actors(cfg.max_actors) + return cfg except yaml.YAMLError as e: @@ -568,6 +603,16 @@ def generate_default_config(include_comments: bool = True) -> str: # on — always attempt parallel (reserved; same as auto until dispatch lands). # execution.wave_mode: auto +# Concurrent Actor limit for parallel wave dispatch (#303 Slice 5b). +# Valid range 1–8; values outside the range are clamped (0→1, 9→8). +# Non-int / bool values fall back to the default 4. +# DORMANT until Slice 5b — setting this in Slice 5a has no effect. +# execution.max_actors: 4 + +# Retry a crashed worker once before aborting the wave (Slice 5b). +# DORMANT until Slice 5b — setting this in Slice 5a has no effect. +# execution.retry_degraded_once: false + # Strip MAP-internal workflow IDs (ST-/AC-/VC-/INV-/HC-) from the code a run # changed, at workflow completion (Stop hook). On by default; uncomment and set # to false to keep the IDs the framework wrote into comments/strings/test names. diff --git a/src/mapify_cli/templates/codex/skills/map-efficient/SKILL.md b/src/mapify_cli/templates/codex/skills/map-efficient/SKILL.md index 22ca5426..7ab5e4e4 100644 --- a/src/mapify_cli/templates/codex/skills/map-efficient/SKILL.md +++ b/src/mapify_cli/templates/codex/skills/map-efficient/SKILL.md @@ -13,7 +13,11 @@ final verifier unless an explicit subagent dispatch is available and useful. Use [efficient-reference.md](efficient-reference.md) for wave details, retry recipes, TDD mode, commit policy, and troubleshooting. Read only the referenced -section when the workflow below points to it. +section when the workflow below points to it. Under `isolation_active` (Slice 5a), +the wave-loop creates per-member worktrees, dispatches actor subagents +**sequentially** (one per turn), verifies via `concurrency_ready`, then accepts +atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b +(`dispatch_mode==concurrent`). ## Mutation Boundary Constraints 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 460d9be7..fb3597db 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 @@ -113,7 +113,9 @@ wave-loop on every run. The wave-loop engages **only when ALL THREE hold** **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+). +wave-loop engages, dispatch remains **sequential** in Slice 5a (`isolation_active=True`, +`dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out +is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -136,10 +138,16 @@ 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 -merge advances HEAD and the next trips `BASE_DIVERGED`): +When `worktree.isolation` is enabled and a wave has ≥2 disjoint subtasks +(`isolation_active=True`), execute the **Slice 5a sequential worktree flow**: + +1. **Create** a worktree per wave member via `create_subtask_worktree`. +2. **Dispatch actor subagents sequentially** — one per turn (`HC-3`), each + pinned to its own worktree path. Do NOT dispatch all in one turn (that is + Slice 5b / `dispatch_mode==concurrent`). +3. **Verify** all member worktrees with `concurrency_ready` before merge. +4. **Accept atomically** — never merge one at a time (the first merge advances + HEAD and the next trips `BASE_DIVERGED`): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" @@ -150,23 +158,24 @@ 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 +### Concurrent actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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**: +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one actor subagent per turn, each +> pinned to its own worktree. The example below is reference material for when +> Slice 5b ships; do NOT treat it as an active instruction now. Use your runtime's +> own parallel actor-subagent dispatch mechanism — this is the provider-neutral +> shape, not a literal API call. + +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' 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) @@ -180,12 +189,14 @@ dispatch actor subagent -> ST-004 (pinned to its own worktree) **`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are pre-split into sequential batches before dispatch. -### Anti-patterns +### Anti-patterns — Slice 5b concurrent dispatch only + +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one actor dispatch per turn **is** the correct behavior. -- 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. +- One actor dispatch per turn across N turns — serial loop, no concurrency. (Slice 5b only — expected, correct behavior in 5a.) +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. (Slice 5b only.) +- Waiting for one actor result before dispatching the next. (Correct for 5a, wrong for 5b.) +- Mixing `get_next_step` and `get_wave_step` for the same wave. (Applies to both 5a and 5b.) ## TDD Mode diff --git a/src/mapify_cli/templates/map/scripts/map_orchestrator.py b/src/mapify_cli/templates/map/scripts/map_orchestrator.py index 7d02035b..e2dad2c4 100755 --- a/src/mapify_cli/templates/map/scripts/map_orchestrator.py +++ b/src/mapify_cli/templates/map/scripts/map_orchestrator.py @@ -251,6 +251,11 @@ def _extract_subtask_ids_from_plan_artifacts( # concurrent Task dispatch is actually implemented and safe to enable. WAVE_CONCURRENCY_ENABLED = False +# Stable reason codes for get_wave_step return sites (ST-002). +WAVE_REASON_NO_WAVES = "no_waves" +WAVE_REASON_WAVE_COMPLETE = "wave_complete" +WAVE_REASON_DISPATCH_SEQUENTIAL = "dispatch_sequential_5a" + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2298,13 +2303,26 @@ def get_wave_step(branch: str) -> dict: state_file = Path(f".map/{branch}/step_state.json") state = StepState.load(state_file) + # Compute structured dispatch signal fields (ST-002). + dispatch_mode = "concurrent" if WAVE_CONCURRENCY_ENABLED else "sequential" + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _worktree_isolation_mode, + ) + isolation_active = _worktree_isolation_mode(Path(".")) != "off" + except ImportError: + isolation_active = False + if not state.execution_waves: return { "mode": "sequential", "wave_index": 0, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_NO_WAVES, "message": "No execution waves configured. Use sequential mode.", } @@ -2314,7 +2332,10 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_WAVE_COMPLETE, } wave = state.execution_waves[state.current_wave_index] @@ -2353,7 +2374,10 @@ def get_wave_step(branch: str) -> dict: "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, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_DISPATCH_SEQUENTIAL, } @@ -2507,12 +2531,18 @@ def select_execution_strategy( reason = "no color-group with width>=2 → sequential (all width-1 waves)" strategy = "sequential" + concurrency_allowed = ( + strategy == "wave_loop" + and isolation_mode in {"auto", "required"} + and has_parallel_groups + ) return { "strategy": strategy, "wave_mode": wave_mode, "worktree_isolation": isolation_mode, "has_parallel_groups": has_parallel_groups, "reason": reason, + "concurrency_allowed": concurrency_allowed, } @@ -4418,6 +4448,7 @@ def main(): "get_wave_step", "validate_wave_step", "advance_wave", + "select_execution_strategy", "resume_single_subtask", "get_plan_progress", "monitor_failed", @@ -4804,6 +4835,10 @@ def main(): result = get_wave_step(branch) print(json.dumps(result, indent=2)) + elif args.command == "select_execution_strategy": + result = select_execution_strategy(branch) + print(json.dumps(result, indent=2)) + elif args.command == "validate_wave_step": if not args.task_or_step: print( 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 4188c256..a343f61a 100755 --- a/src/mapify_cli/templates/map/scripts/map_step_runner.py +++ b/src/mapify_cli/templates/map/scripts/map_step_runner.py @@ -15342,6 +15342,12 @@ def _wt_force_remove(path: Path, branch_ref: str) -> None: _WT_REASON_UNSUPPORTED: str = "worktree_unsupported" _WT_REASON_CREATE_FAILED: str = "worktree_create_failed" _WT_REASON_DIRTY_MERGE_TARGET: str = "dirty_merge_target" +# concurrency_ready reason codes +_WT_REASON_NO_RECORD: str = "no_record" +_WT_REASON_PATH_MISSING: str = "path_missing" +_WT_REASON_NOT_REGISTERED: str = "not_registered" +_WT_REASON_HEAD_MISMATCH: str = "head_mismatch" +_WT_REASON_DIRTY: str = "dirty" _WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) @@ -16752,6 +16758,154 @@ def worktree_isolation_status(branch: Optional[str] = None) -> dict[str, object] } +def concurrency_ready( + subtask_ids: list[str], + branch: Optional[str] = None, +) -> dict[str, object]: + """Read-only wave-worktree readiness check (coordinator-owned, council Q1). + + For each supplied subtask verifies: + 1. A worktree record exists in the branch-scoped sidecar. + 2. The recorded path exists on disk AND is git-registered (appears in + ``git worktree list --porcelain``). + 3. HEAD in the worktree == the recorded base_sha (frozen-SHA invariant #284). + 4. The worktree tree is clean, ignoring MAP runtime-state paths. + + Returns:: + + { + "ready": bool, + "per_subtask": {sid: {"ok": bool, "reason": code | None, ...}}, + "reason": | None, + } + + This function NEVER creates, merges, removes, or commits anything. + When not inside a git repo or no worktrees are recorded the function + returns a structured non-error result (ready=False) rather than raising. + """ + if not _wt_is_git_repo(): + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NOT_GIT_REPO, + } + + branch_name = branch or get_branch_name() + state = _read_worktree_state(branch_name) + worktrees = state.get("worktrees", {}) + if not isinstance(worktrees, dict) or not worktrees: + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NO_RECORD, + } + + # Build a set of paths registered with git for O(1) membership checks. + registered_paths: set[str] = set() + 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 "): + try: + registered_paths.add( + str(Path(line[len("worktree "):].strip()).resolve()) + ) + except OSError: + pass + + per_subtask: dict[str, object] = {} + first_failure: Optional[str] = None + + ids = sorted({str(s) for s in subtask_ids if str(s).strip()}) + for sid in ids: + slug = _wt_slug(sid) + if slug is None: + reason = "invalid_subtask_id" + per_subtask[sid] = {"ok": False, "reason": reason} + if first_failure is None: + first_failure = reason + continue + + record = worktrees.get(slug) + if not isinstance(record, dict): + per_subtask[sid] = {"ok": False, "reason": _WT_REASON_NO_RECORD} + if first_failure is None: + first_failure = _WT_REASON_NO_RECORD + continue + + raw_path = str(record.get("path", "")) + base_sha = str(record.get("base_sha", "")) + wt_path = Path(raw_path) if raw_path else None + + # (1) Path exists on disk + if wt_path is None or not wt_path.is_dir(): + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_PATH_MISSING, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_PATH_MISSING + continue + + # (2) git-registered + try: + resolved = str(wt_path.resolve()) + except OSError: + resolved = raw_path + if resolved not in registered_paths: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_NOT_REGISTERED, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_NOT_REGISTERED + continue + + # (3) HEAD == base_sha + head = _wt_head_sha(wt_path) + if head != base_sha: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_HEAD_MISMATCH, + "expected": base_sha[:8] if base_sha else None, + "actual": head[:8] if head else None, + } + if first_failure is None: + first_failure = _WT_REASON_HEAD_MISMATCH + continue + + # (4) Clean tree (ignoring MAP runtime-state paths) + st = _wt_git(["status", "--porcelain"], cwd=wt_path) + dirty_lines = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty_lines: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_DIRTY, + "dirty": dirty_lines[:10], + } + if first_failure is None: + first_failure = _WT_REASON_DIRTY + continue + + per_subtask[sid] = {"ok": True, "reason": None} + + ready = bool(ids) and all( + isinstance(v, dict) and v.get("ok") for v in per_subtask.values() + ) + return { + "ready": ready, + "per_subtask": per_subtask, + "reason": first_failure, + } + + if __name__ == "__main__": # Simple CLI interface for testing import sys @@ -18219,6 +18373,20 @@ def _flag_val(name: str) -> Optional[str]: if _wt_r.get("status") == "error": sys.exit(1) + elif func_name == "concurrency_ready": + # CLI: concurrency_ready [ ...] [--branch B] + # Coordinator-owned read-only wave-worktree readiness check (council Q1). + # Returns JSON; exits 0 even when ready=False (a structural result, not an + # error); exits 1 only on argument/parse errors. + import argparse as _ap + + _p = _ap.ArgumentParser(prog="map_step_runner.py concurrency_ready") + _p.add_argument("subtask_ids", nargs="+") + _p.add_argument("--branch", default=None) + _a = _p.parse_args(sys.argv[2:]) + _wt_r = concurrency_ready(_a.subtask_ids, _a.branch) + print(json.dumps(_wt_r, indent=2)) + elif func_name == "discard_subtask_worktree": # CLI: discard_subtask_worktree [--attempt N] [--branch B] # [--save-patch] diff --git a/src/mapify_cli/templates/skills/map-efficient/SKILL.md b/src/mapify_cli/templates/skills/map-efficient/SKILL.md index 7e1da6f5..efb7b7fc 100644 --- a/src/mapify_cli/templates/skills/map-efficient/SKILL.md +++ b/src/mapify_cli/templates/skills/map-efficient/SKILL.md @@ -207,7 +207,7 @@ else fi ``` -**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. +**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. Under `isolation_active` (Slice 5a), the wave-loop creates per-member worktrees, dispatches Actors **sequentially** (one per turn, `HC-3`), verifies via `concurrency_ready`, then accepts atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`). 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 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 76b9554a..f855be02 100644 --- a/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md +++ b/src/mapify_cli/templates/skills/map-efficient/efficient-reference.md @@ -136,7 +136,7 @@ including clean passes — must carry concrete evidence references. | `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`). +**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 remains **sequential** in Slice 5a (`isolation_active=True`, `dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -146,22 +146,23 @@ Use `get_next_step` for all sequential (default) execution. One phase at a time, 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. +When the wave-loop engages AND `isolation_active` is true (`worktree.isolation` ∈ {`auto`, `required`}), the Slice 5a flow applies: (a) create a worktree per wave member via `create_subtask_worktree`; (b) dispatch the member Actors **sequentially** — one per turn, each pinned to its own worktree path (`HC-3`); (c) call `concurrency_ready` (ST-003) to verify all member worktrees before merge; (d) accept the whole wave atomically via `merge_wave_worktrees` — never one-at-a-time, with whole-wave rollback on any failure. See [Parallel waves](#worktree-isolation) under Worktree isolation for the full protocol. Concurrent fan-out (dispatching all Actors in one message) is Slice 5b (`dispatch_mode==concurrent`) and is not yet active. -### Concurrent Actor dispatch — GATED EXAMPLE +### Concurrent Actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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. +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one Actor per turn, each pinned to +> its own worktree. The example below is reference material for when Slice 5b 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: +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' only) — N Task calls in one message: Task( subagent_type="actor", description="Implement ST-003", @@ -184,12 +185,14 @@ Task( **`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) +### Anti-patterns — Slice 5b concurrent dispatch only -- **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. +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one Task per turn **is** the correct behavior — the first three below are NOT anti-patterns there. + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. (Slice 5b only — this is the expected, correct behavior in 5a.) +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. (Slice 5b only.) +- **Waiting for one actor result before dispatching the next** — correct for sequential dispatch (5a), wrong for concurrent waves (5b). +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. (Applies to both 5a and 5b.) ### Actor-boundary prompt template (worktree-isolated subtasks) @@ -653,12 +656,20 @@ with `worktree_isolation_status`. ### Parallel waves (≥2 worktree-isolated subtasks) — #284 Phase 2 When `get_wave_step` returns `mode:"parallel"` (a wave with ≥2 disjoint-file -subtasks) AND isolation is enabled, give EACH subtask its own worktree and -dispatch the Actors concurrently (separate Task agents, each pinned to its own -`$WT_PATH`). Do NOT merge them one at a time: every worktree was cut off the same -HEAD, so the first `merge_subtask_worktree` advances the working branch and the -next trips `BASE_DIVERGED`. Accept the whole wave atomically instead — only after -EVERY subtask in the wave has passed Monitor (+ Evaluator): +subtasks) AND `isolation_active` is true, execute the **Slice 5a sequential +worktree flow**: + +1. **Create** a worktree per wave member: `create_subtask_worktree` for each. +2. **Dispatch Actors sequentially** — one per turn (`HC-3`), each pinned to its + own `$WT_PATH`. Do NOT dispatch all in one message (that is Slice 5b). +3. **Verify** all member worktrees with `concurrency_ready` (ST-003) before merge. +4. **Accept atomically** via `merge_wave_worktrees` after every subtask passes + Monitor (+ Evaluator) — never merge one at a time. + +Do NOT merge them one at a time: every worktree was cut off the same HEAD, so +the first `merge_subtask_worktree` advances the working branch and the next trips +`BASE_DIVERGED`. Accept the whole wave atomically instead — only after EVERY +subtask in the wave has passed Monitor (+ Evaluator): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" "$ST_C" diff --git a/src/mapify_cli/templates_src/codex/skills/map-efficient/SKILL.md.jinja b/src/mapify_cli/templates_src/codex/skills/map-efficient/SKILL.md.jinja index 22ca5426..7ab5e4e4 100644 --- a/src/mapify_cli/templates_src/codex/skills/map-efficient/SKILL.md.jinja +++ b/src/mapify_cli/templates_src/codex/skills/map-efficient/SKILL.md.jinja @@ -13,7 +13,11 @@ final verifier unless an explicit subagent dispatch is available and useful. Use [efficient-reference.md](efficient-reference.md) for wave details, retry recipes, TDD mode, commit policy, and troubleshooting. Read only the referenced -section when the workflow below points to it. +section when the workflow below points to it. Under `isolation_active` (Slice 5a), +the wave-loop creates per-member worktrees, dispatches actor subagents +**sequentially** (one per turn), verifies via `concurrency_ready`, then accepts +atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b +(`dispatch_mode==concurrent`). ## Mutation Boundary Constraints 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 460d9be7..fb3597db 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 @@ -113,7 +113,9 @@ wave-loop on every run. The wave-loop engages **only when ALL THREE hold** **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+). +wave-loop engages, dispatch remains **sequential** in Slice 5a (`isolation_active=True`, +`dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out +is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -136,10 +138,16 @@ 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 -merge advances HEAD and the next trips `BASE_DIVERGED`): +When `worktree.isolation` is enabled and a wave has ≥2 disjoint subtasks +(`isolation_active=True`), execute the **Slice 5a sequential worktree flow**: + +1. **Create** a worktree per wave member via `create_subtask_worktree`. +2. **Dispatch actor subagents sequentially** — one per turn (`HC-3`), each + pinned to its own worktree path. Do NOT dispatch all in one turn (that is + Slice 5b / `dispatch_mode==concurrent`). +3. **Verify** all member worktrees with `concurrency_ready` before merge. +4. **Accept atomically** — never merge one at a time (the first merge advances + HEAD and the next trips `BASE_DIVERGED`): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" @@ -150,23 +158,24 @@ 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 +### Concurrent actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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**: +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one actor subagent per turn, each +> pinned to its own worktree. The example below is reference material for when +> Slice 5b ships; do NOT treat it as an active instruction now. Use your runtime's +> own parallel actor-subagent dispatch mechanism — this is the provider-neutral +> shape, not a literal API call. + +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' 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) @@ -180,12 +189,14 @@ dispatch actor subagent -> ST-004 (pinned to its own worktree) **`max_actors` cap:** Default 4–8 per wave. Groups larger than `max_actors` are pre-split into sequential batches before dispatch. -### Anti-patterns +### Anti-patterns — Slice 5b concurrent dispatch only + +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one actor dispatch per turn **is** the correct behavior. -- 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. +- One actor dispatch per turn across N turns — serial loop, no concurrency. (Slice 5b only — expected, correct behavior in 5a.) +- Writing between dispatches (TodoWrite, etc.) — serializes the batch. (Slice 5b only.) +- Waiting for one actor result before dispatching the next. (Correct for 5a, wrong for 5b.) +- Mixing `get_next_step` and `get_wave_step` for the same wave. (Applies to both 5a and 5b.) ## TDD Mode 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 7d02035b..e2dad2c4 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 @@ -251,6 +251,11 @@ AGGRESSIVE_COMPRESSION_MULTIPLIER = 0.4 # concurrent Task dispatch is actually implemented and safe to enable. WAVE_CONCURRENCY_ENABLED = False +# Stable reason codes for get_wave_step return sites (ST-002). +WAVE_REASON_NO_WAVES = "no_waves" +WAVE_REASON_WAVE_COMPLETE = "wave_complete" +WAVE_REASON_DISPATCH_SEQUENTIAL = "dispatch_sequential_5a" + def _read_map_config_scalars(project_dir: Path) -> dict[str, str]: """Read top-level scalar values from .map/config.yaml without dependencies.""" @@ -2298,13 +2303,26 @@ def get_wave_step(branch: str) -> dict: state_file = Path(f".map/{branch}/step_state.json") state = StepState.load(state_file) + # Compute structured dispatch signal fields (ST-002). + dispatch_mode = "concurrent" if WAVE_CONCURRENCY_ENABLED else "sequential" + try: + from map_step_runner import ( # pyright: ignore[reportMissingImports] + _worktree_isolation_mode, + ) + isolation_active = _worktree_isolation_mode(Path(".")) != "off" + except ImportError: + isolation_active = False + if not state.execution_waves: return { "mode": "sequential", "wave_index": 0, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_NO_WAVES, "message": "No execution waves configured. Use sequential mode.", } @@ -2314,7 +2332,10 @@ def get_wave_step(branch: str) -> dict: "wave_index": state.current_wave_index, "subtasks": [], "is_complete": True, - "concurrency_enabled": WAVE_CONCURRENCY_ENABLED, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_WAVE_COMPLETE, } wave = state.execution_waves[state.current_wave_index] @@ -2353,7 +2374,10 @@ def get_wave_step(branch: str) -> dict: "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, + "concurrency_enabled": dispatch_mode == "concurrent", + "dispatch_mode": dispatch_mode, + "isolation_active": isolation_active, + "reason": WAVE_REASON_DISPATCH_SEQUENTIAL, } @@ -2507,12 +2531,18 @@ def select_execution_strategy( reason = "no color-group with width>=2 → sequential (all width-1 waves)" strategy = "sequential" + concurrency_allowed = ( + strategy == "wave_loop" + and isolation_mode in {"auto", "required"} + and has_parallel_groups + ) return { "strategy": strategy, "wave_mode": wave_mode, "worktree_isolation": isolation_mode, "has_parallel_groups": has_parallel_groups, "reason": reason, + "concurrency_allowed": concurrency_allowed, } @@ -4418,6 +4448,7 @@ def main(): "get_wave_step", "validate_wave_step", "advance_wave", + "select_execution_strategy", "resume_single_subtask", "get_plan_progress", "monitor_failed", @@ -4804,6 +4835,10 @@ def main(): result = get_wave_step(branch) print(json.dumps(result, indent=2)) + elif args.command == "select_execution_strategy": + result = select_execution_strategy(branch) + print(json.dumps(result, indent=2)) + elif args.command == "validate_wave_step": if not args.task_or_step: print( 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 4188c256..a343f61a 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 @@ -15342,6 +15342,12 @@ _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" +# concurrency_ready reason codes +_WT_REASON_NO_RECORD: str = "no_record" +_WT_REASON_PATH_MISSING: str = "path_missing" +_WT_REASON_NOT_REGISTERED: str = "not_registered" +_WT_REASON_HEAD_MISMATCH: str = "head_mismatch" +_WT_REASON_DIRTY: str = "dirty" _WT_ISOLATION_VALID = frozenset({"off", "auto", "required"}) @@ -16752,6 +16758,154 @@ def worktree_isolation_status(branch: Optional[str] = None) -> dict[str, object] } +def concurrency_ready( + subtask_ids: list[str], + branch: Optional[str] = None, +) -> dict[str, object]: + """Read-only wave-worktree readiness check (coordinator-owned, council Q1). + + For each supplied subtask verifies: + 1. A worktree record exists in the branch-scoped sidecar. + 2. The recorded path exists on disk AND is git-registered (appears in + ``git worktree list --porcelain``). + 3. HEAD in the worktree == the recorded base_sha (frozen-SHA invariant #284). + 4. The worktree tree is clean, ignoring MAP runtime-state paths. + + Returns:: + + { + "ready": bool, + "per_subtask": {sid: {"ok": bool, "reason": code | None, ...}}, + "reason": | None, + } + + This function NEVER creates, merges, removes, or commits anything. + When not inside a git repo or no worktrees are recorded the function + returns a structured non-error result (ready=False) rather than raising. + """ + if not _wt_is_git_repo(): + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NOT_GIT_REPO, + } + + branch_name = branch or get_branch_name() + state = _read_worktree_state(branch_name) + worktrees = state.get("worktrees", {}) + if not isinstance(worktrees, dict) or not worktrees: + return { + "ready": False, + "per_subtask": {}, + "reason": _WT_REASON_NO_RECORD, + } + + # Build a set of paths registered with git for O(1) membership checks. + registered_paths: set[str] = set() + 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 "): + try: + registered_paths.add( + str(Path(line[len("worktree "):].strip()).resolve()) + ) + except OSError: + pass + + per_subtask: dict[str, object] = {} + first_failure: Optional[str] = None + + ids = sorted({str(s) for s in subtask_ids if str(s).strip()}) + for sid in ids: + slug = _wt_slug(sid) + if slug is None: + reason = "invalid_subtask_id" + per_subtask[sid] = {"ok": False, "reason": reason} + if first_failure is None: + first_failure = reason + continue + + record = worktrees.get(slug) + if not isinstance(record, dict): + per_subtask[sid] = {"ok": False, "reason": _WT_REASON_NO_RECORD} + if first_failure is None: + first_failure = _WT_REASON_NO_RECORD + continue + + raw_path = str(record.get("path", "")) + base_sha = str(record.get("base_sha", "")) + wt_path = Path(raw_path) if raw_path else None + + # (1) Path exists on disk + if wt_path is None or not wt_path.is_dir(): + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_PATH_MISSING, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_PATH_MISSING + continue + + # (2) git-registered + try: + resolved = str(wt_path.resolve()) + except OSError: + resolved = raw_path + if resolved not in registered_paths: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_NOT_REGISTERED, + "path": raw_path, + } + if first_failure is None: + first_failure = _WT_REASON_NOT_REGISTERED + continue + + # (3) HEAD == base_sha + head = _wt_head_sha(wt_path) + if head != base_sha: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_HEAD_MISMATCH, + "expected": base_sha[:8] if base_sha else None, + "actual": head[:8] if head else None, + } + if first_failure is None: + first_failure = _WT_REASON_HEAD_MISMATCH + continue + + # (4) Clean tree (ignoring MAP runtime-state paths) + st = _wt_git(["status", "--porcelain"], cwd=wt_path) + dirty_lines = [ + ln + for ln in st.stdout.splitlines() + if ln.strip() and not _wt_is_runtime_state_path(_wt_porcelain_path(ln)) + ] + if dirty_lines: + per_subtask[sid] = { + "ok": False, + "reason": _WT_REASON_DIRTY, + "dirty": dirty_lines[:10], + } + if first_failure is None: + first_failure = _WT_REASON_DIRTY + continue + + per_subtask[sid] = {"ok": True, "reason": None} + + ready = bool(ids) and all( + isinstance(v, dict) and v.get("ok") for v in per_subtask.values() + ) + return { + "ready": ready, + "per_subtask": per_subtask, + "reason": first_failure, + } + + if __name__ == "__main__": # Simple CLI interface for testing import sys @@ -18219,6 +18373,20 @@ if __name__ == "__main__": if _wt_r.get("status") == "error": sys.exit(1) + elif func_name == "concurrency_ready": + # CLI: concurrency_ready [ ...] [--branch B] + # Coordinator-owned read-only wave-worktree readiness check (council Q1). + # Returns JSON; exits 0 even when ready=False (a structural result, not an + # error); exits 1 only on argument/parse errors. + import argparse as _ap + + _p = _ap.ArgumentParser(prog="map_step_runner.py concurrency_ready") + _p.add_argument("subtask_ids", nargs="+") + _p.add_argument("--branch", default=None) + _a = _p.parse_args(sys.argv[2:]) + _wt_r = concurrency_ready(_a.subtask_ids, _a.branch) + print(json.dumps(_wt_r, indent=2)) + elif func_name == "discard_subtask_worktree": # CLI: discard_subtask_worktree [--attempt N] [--branch B] # [--save-patch] 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 7e1da6f5..efb7b7fc 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 @@ -207,7 +207,7 @@ else fi ``` -**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. +**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. Under `isolation_active` (Slice 5a), the wave-loop creates per-member worktrees, dispatches Actors **sequentially** (one per turn, `HC-3`), verifies via `concurrency_ready`, then accepts atomically via `merge_wave_worktrees`; concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`). 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 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 76b9554a..f855be02 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 @@ -136,7 +136,7 @@ including clean passes — must carry concrete evidence references. | `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`). +**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 remains **sequential** in Slice 5a (`isolation_active=True`, `dispatch_mode` from `get_wave_step` keyed to `sequential`); concurrent fan-out is Slice 5b (`dispatch_mode==concurrent`, `concurrency_enabled=True`, not yet shipped). ### Sequential walker @@ -146,22 +146,23 @@ Use `get_next_step` for all sequential (default) execution. One phase at a time, 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. +When the wave-loop engages AND `isolation_active` is true (`worktree.isolation` ∈ {`auto`, `required`}), the Slice 5a flow applies: (a) create a worktree per wave member via `create_subtask_worktree`; (b) dispatch the member Actors **sequentially** — one per turn, each pinned to its own worktree path (`HC-3`); (c) call `concurrency_ready` (ST-003) to verify all member worktrees before merge; (d) accept the whole wave atomically via `merge_wave_worktrees` — never one-at-a-time, with whole-wave rollback on any failure. See [Parallel waves](#worktree-isolation) under Worktree isolation for the full protocol. Concurrent fan-out (dispatching all Actors in one message) is Slice 5b (`dispatch_mode==concurrent`) and is not yet active. -### Concurrent Actor dispatch — GATED EXAMPLE +### Concurrent Actor dispatch — **Slice 5b only** (`dispatch_mode == 'concurrent'`) — 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. +> **Slice 5b** and is enabled **only when `concurrency_enabled: true` / +> `parallel_ready` flag set / `dispatch_mode == 'concurrent'`**. In the **current +> framework** (`concurrency_enabled=False`, Slice 5a), dispatch stays **SEQUENTIAL +> even when a wave has `mode=="parallel"`** — one Actor per turn, each pinned to +> its own worktree. The example below is reference material for when Slice 5b 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: +When Slice 5b concurrency is enabled, 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: +# CORRECT (Slice 5b / concurrency_enabled=True / dispatch_mode=='concurrent' only) — N Task calls in one message: Task( subagent_type="actor", description="Implement ST-003", @@ -184,12 +185,14 @@ Task( **`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) +### Anti-patterns — Slice 5b concurrent dispatch only -- **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. +> These apply **only** under Slice 5b concurrent dispatch (`dispatch_mode == 'concurrent'`). In Slice 5a and the default sequential walker, one Task per turn **is** the correct behavior — the first three below are NOT anti-patterns there. + +- **One Task per turn across N turns** — serial actor loop that happens to use wave state; does not achieve concurrency. (Slice 5b only — this is the expected, correct behavior in 5a.) +- **TodoWrite between actor dispatches** — a TodoWrite call between Task calls serializes the batch; emit all Task calls in one message. (Slice 5b only.) +- **Waiting for one actor result before dispatching the next** — correct for sequential dispatch (5a), wrong for concurrent waves (5b). +- **Mixing `get_next_step` and `get_wave_step` for the same wave** — corrupts the state-machine cursor. (Applies to both 5a and 5b.) ### Actor-boundary prompt template (worktree-isolated subtasks) @@ -653,12 +656,20 @@ with `worktree_isolation_status`. ### Parallel waves (≥2 worktree-isolated subtasks) — #284 Phase 2 When `get_wave_step` returns `mode:"parallel"` (a wave with ≥2 disjoint-file -subtasks) AND isolation is enabled, give EACH subtask its own worktree and -dispatch the Actors concurrently (separate Task agents, each pinned to its own -`$WT_PATH`). Do NOT merge them one at a time: every worktree was cut off the same -HEAD, so the first `merge_subtask_worktree` advances the working branch and the -next trips `BASE_DIVERGED`. Accept the whole wave atomically instead — only after -EVERY subtask in the wave has passed Monitor (+ Evaluator): +subtasks) AND `isolation_active` is true, execute the **Slice 5a sequential +worktree flow**: + +1. **Create** a worktree per wave member: `create_subtask_worktree` for each. +2. **Dispatch Actors sequentially** — one per turn (`HC-3`), each pinned to its + own `$WT_PATH`. Do NOT dispatch all in one message (that is Slice 5b). +3. **Verify** all member worktrees with `concurrency_ready` (ST-003) before merge. +4. **Accept atomically** via `merge_wave_worktrees` after every subtask passes + Monitor (+ Evaluator) — never merge one at a time. + +Do NOT merge them one at a time: every worktree was cut off the same HEAD, so +the first `merge_subtask_worktree` advances the working branch and the next trips +`BASE_DIVERGED`. Accept the whole wave atomically instead — only after EVERY +subtask in the wave has passed Monitor (+ Evaluator): ```bash python3 .map/scripts/map_step_runner.py merge_wave_worktrees "$ST_A" "$ST_B" "$ST_C" diff --git a/tests/test_map_orchestrator.py b/tests/test_map_orchestrator.py index c9ea7f04..36b8f916 100644 --- a/tests/test_map_orchestrator.py +++ b/tests/test_map_orchestrator.py @@ -4757,5 +4757,508 @@ def test_advance_wave_atomic_reset( ) +def test_vc1_concurrency_allowed_gate( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC1 [AC-1]: concurrency_allowed truth table for select_execution_strategy. + + True only when strategy==wave_loop AND isolation in {auto,required} AND has_parallel_groups. + Each condition is toggled off in turn to verify the AND semantics. + """ + import sys as _sys + import types + + monkeypatch.chdir(tmp_path) + branch = "test-vc1-concurrency-allowed" + # Seed a step_state with a width>=2 execution_waves group. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + def _fake_runner(wave_mode: str, isolation: str) -> "types.ModuleType": + 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 + + _orig_msr = _sys.modules.get("map_step_runner") + try: + # True: all three conditions satisfied (wave_loop + auto isolation + parallel groups). + _sys.modules["map_step_runner"] = _fake_runner("on", "auto") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["concurrency_allowed"] is True, ( + f"on+auto+width>=2 must give concurrency_allowed=True: {result}" + ) + assert result["strategy"] == "wave_loop" + + # True: isolation=required also qualifies. + _sys.modules["map_step_runner"] = _fake_runner("on", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["concurrency_allowed"] is True, ( + f"on+required+width>=2 must give concurrency_allowed=True: {result}" + ) + + # True: wave_mode=auto also qualifies. + _sys.modules["map_step_runner"] = _fake_runner("auto", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["concurrency_allowed"] is True, ( + f"auto+required+width>=2 must give concurrency_allowed=True: {result}" + ) + + # False: condition 1 off — wave_mode=off → strategy=sequential. + _sys.modules["map_step_runner"] = _fake_runner("off", "required") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["concurrency_allowed"] is False, ( + f"wave_mode=off must give concurrency_allowed=False: {result}" + ) + assert result["strategy"] == "sequential" + + # False: condition 2 off — isolation=off → strategy=sequential. + _sys.modules["map_step_runner"] = _fake_runner("on", "off") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + assert result["concurrency_allowed"] is False, ( + f"isolation=off must give concurrency_allowed=False: {result}" + ) + + # False: condition 3 off — all waves width-1 → has_parallel_groups=False. + _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["concurrency_allowed"] is False, ( + f"all width-1 waves must give concurrency_allowed=False: {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_vc2_default_config_sequential( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC2 [AC-10]: default config (no worktree.isolation key) gives strategy=='sequential' + AND concurrency_allowed==False AND pre-existing reason string is unchanged. + """ + import sys as _sys + import types + + monkeypatch.chdir(tmp_path) + branch = "test-vc2-default-sequential" + # Seed a width>=2 wave so has_parallel_groups=True — isolation gate must block wave_loop. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + def _fake_runner_default(wave_mode: str, isolation: str) -> "types.ModuleType": + 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 + + _orig_msr = _sys.modules.get("map_step_runner") + try: + # Default: wave_mode=auto (MapConfig default), isolation=off (MapConfig default). + _sys.modules["map_step_runner"] = _fake_runner_default("auto", "off") + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + + assert result["strategy"] == "sequential", ( + f"default config must give strategy='sequential': {result}" + ) + assert result["concurrency_allowed"] is False, ( + f"default config must give concurrency_allowed=False: {result}" + ) + # Pre-existing reason string for the isolation-off path must be unchanged. + assert "worktree.isolation='off'" in result["reason"], ( + f"reason string for isolation=off path must contain worktree.isolation='off': {result['reason']!r}" + ) + assert "legacy sequential" in result["reason"], ( + f"reason string for isolation=off path must contain 'legacy sequential': {result['reason']!r}" + ) + finally: + if _orig_msr is not None: + _sys.modules["map_step_runner"] = _orig_msr + else: + _sys.modules.pop("map_step_runner", None) + + +def test_vc3_strategy_cli_handler( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC3 [render-parity]: CLI 'select_execution_strategy ' prints JSON with concurrency_allowed. + + Invokes the generated .map/scripts/map_orchestrator.py via subprocess to prove + the CLI handler is wired in the rendered tree after make render-templates. + """ + import shutil + + monkeypatch.chdir(tmp_path) + branch = "test-vc3-cli-handler" + + # Copy the rendered scripts into /.map/scripts/ (same pattern as other CLI tests). + scripts_dir = tmp_path / ".map" / "scripts" + scripts_dir.mkdir(parents=True) + for py_file in ORCHESTRATOR_PATH.glob("*.py"): + shutil.copy(py_file, scripts_dir / py_file.name) + + script = scripts_dir / "map_orchestrator.py" + + # Seed branch state: sequential (no config.yaml → isolation defaults to off). + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + proc = subprocess.run( + [sys.executable, str(script), "select_execution_strategy", "--branch", branch], + cwd=str(tmp_path), + capture_output=True, + text=True, + timeout=30, + ) + assert proc.returncode == 0, ( + f"CLI exited {proc.returncode}\nstdout={proc.stdout!r}\nstderr={proc.stderr!r}" + ) + data = json.loads(proc.stdout) + assert "concurrency_allowed" in data, ( + f"CLI output missing 'concurrency_allowed' key: {data}" + ) + assert isinstance(data["concurrency_allowed"], bool), ( + f"concurrency_allowed must be bool, got {type(data['concurrency_allowed'])}: {data}" + ) + assert "strategy" in data + assert "worktree_isolation" in data + assert "has_parallel_groups" in data + + +# --------------------------------------------------------------------------- +# ST-002: structured dispatch signal in get_wave_step +# --------------------------------------------------------------------------- + + +def test_vc1_get_wave_step_dispatch_mode_sequential( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC1: dispatch_mode=='sequential' and isolation_active/reason non-empty for both + width-1 and width>=2 active waves while WAVE_CONCURRENCY_ENABLED is False.""" + import types + + monkeypatch.chdir(tmp_path) + + def _fake_runner(isolation: str) -> "types.ModuleType": + mod = types.ModuleType("map_step_runner") + + def _iso(_project_dir: object) -> str: + del _project_dir + return isolation + + mod._worktree_isolation_mode = _iso # type: ignore[attr-defined] + return mod + + import sys as _sys + + _orig_msr = _sys.modules.get("map_step_runner") + try: + # width-1 wave, isolation=required → isolation_active=True + _sys.modules["map_step_runner"] = _fake_runner("required") + branch1 = "test-st002-vc1-width1" + _write_step_state(branch1, tmp_path, execution_waves=[["ST-001"]]) + result1 = map_orchestrator.get_wave_step(branch1) + + assert result1["dispatch_mode"] == "sequential", ( + f"width-1 wave: expected dispatch_mode='sequential', got: {result1}" + ) + assert result1["isolation_active"] is True, ( + f"width-1 wave: isolation='required' → isolation_active must be True: {result1}" + ) + assert result1["reason"], f"width-1 wave: reason must be non-empty: {result1}" + assert result1["is_complete"] is False + + # width-2 wave, isolation=off → isolation_active=False + _sys.modules["map_step_runner"] = _fake_runner("off") + branch2 = "test-st002-vc1-width2" + _write_step_state(branch2, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + result2 = map_orchestrator.get_wave_step(branch2) + + assert result2["dispatch_mode"] == "sequential", ( + f"width-2 wave: expected dispatch_mode='sequential', got: {result2}" + ) + assert result2["isolation_active"] is False, ( + f"width-2 wave: isolation='off' → isolation_active must be False: {result2}" + ) + assert result2["reason"], f"width-2 wave: reason must be non-empty: {result2}" + assert result2["is_complete"] is False + finally: + if _orig_msr is None: + _sys.modules.pop("map_step_runner", None) + else: + _sys.modules["map_step_runner"] = _orig_msr + + +def test_vc2_dispatch_mode_never_concurrent_in_5a( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC2: dispatch_mode is 'sequential' (never 'concurrent') on every return path + while WAVE_CONCURRENCY_ENABLED is False.""" + monkeypatch.chdir(tmp_path) + branch = "test-st002-vc2" + + # Path 1: no waves → no_waves early return + _write_step_state(branch, tmp_path, execution_waves=[]) + r_no_waves = map_orchestrator.get_wave_step(branch) + assert r_no_waves["dispatch_mode"] == "sequential", ( + f"no-waves path: dispatch_mode must be 'sequential': {r_no_waves}" + ) + assert r_no_waves["concurrency_enabled"] is False, ( + f"no-waves path: concurrency_enabled alias must be False: {r_no_waves}" + ) + + # Path 2: wave exhausted → wave_complete early return + _write_step_state( + branch, + tmp_path, + execution_waves=[["ST-001"]], + extra={"current_wave_index": 99}, + ) + r_complete = map_orchestrator.get_wave_step(branch) + assert r_complete["dispatch_mode"] == "sequential", ( + f"wave-complete path: dispatch_mode must be 'sequential': {r_complete}" + ) + assert r_complete["concurrency_enabled"] is False, ( + f"wave-complete path: concurrency_enabled alias must be False: {r_complete}" + ) + + # Path 3: active wave (width>=2) → main return + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + r_active = map_orchestrator.get_wave_step(branch) + assert r_active["dispatch_mode"] == "sequential", ( + f"active-wave path: dispatch_mode must be 'sequential': {r_active}" + ) + assert r_active["concurrency_enabled"] is False, ( + f"active-wave path: concurrency_enabled alias must be False: {r_active}" + ) + + +def test_vc3_get_wave_step_reason_codes( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """VC3: each of the three return paths emits its distinct stable reason code.""" + monkeypatch.chdir(tmp_path) + branch = "test-st002-vc3" + + # Path 1: no waves + _write_step_state(branch, tmp_path, execution_waves=[]) + r_no_waves = map_orchestrator.get_wave_step(branch) + assert r_no_waves["reason"] == map_orchestrator.WAVE_REASON_NO_WAVES, ( + f"no-waves path: expected reason={map_orchestrator.WAVE_REASON_NO_WAVES!r}: {r_no_waves}" + ) + + # Path 2: wave index exhausted + _write_step_state( + branch, + tmp_path, + execution_waves=[["ST-001"]], + extra={"current_wave_index": 99}, + ) + r_complete = map_orchestrator.get_wave_step(branch) + assert r_complete["reason"] == map_orchestrator.WAVE_REASON_WAVE_COMPLETE, ( + f"wave-complete path: expected reason={map_orchestrator.WAVE_REASON_WAVE_COMPLETE!r}: " + f"{r_complete}" + ) + + # Path 3: active wave + _write_step_state(branch, tmp_path, execution_waves=[["ST-001"]]) + r_active = map_orchestrator.get_wave_step(branch) + assert r_active["reason"] == map_orchestrator.WAVE_REASON_DISPATCH_SEQUENTIAL, ( + f"active-wave path: expected reason={map_orchestrator.WAVE_REASON_DISPATCH_SEQUENTIAL!r}: " + f"{r_active}" + ) + + # All three codes must be distinct stable strings + codes = { + map_orchestrator.WAVE_REASON_NO_WAVES, + map_orchestrator.WAVE_REASON_WAVE_COMPLETE, + map_orchestrator.WAVE_REASON_DISPATCH_SEQUENTIAL, + } + assert len(codes) == 3, f"reason codes must all be distinct: {codes}" + + +# --------------------------------------------------------------------------- +# ST-007: HC-1 default-config behavior-neutrality PROOF tests +# --------------------------------------------------------------------------- +# These tests use the REAL map_step_runner (no sys.modules mock) against an +# actual filesystem project_dir with no .map/config.yaml (or an empty one), +# proving that the real MapConfig defaults keep every dispatch path sequential. +# The genuine proof: each test seeds a width>=2 wave (a parallelizable plan), +# which WOULD trigger wave_loop if the config asked for it — but the isolation +# gate (defaults to 'off') keeps everything on the legacy sequential path. + + +def test_vc1_default_config_neutral_strategy( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """HC-1 proof: under default config (no .map/config.yaml), select_execution_strategy + returns strategy=='sequential', concurrency_allowed==False, and + worktree_isolation=='off' — even when a parallelizable (width>=2) wave exists. + + This is non-tautological: the width>=2 wave makes has_parallel_groups=True, so + the isolation gate is the only thing keeping the result sequential. The test + exercises the real _execution_wave_mode + _worktree_isolation_mode functions + against a real (empty) project dir — no mocks. + """ + import sys as _sys + + monkeypatch.chdir(tmp_path) + branch = "test-st007-vc1-default-neutral" + + # Seed a width>=2 execution wave — has_parallel_groups will be True. + # Without the isolation gate, this WOULD select strategy='wave_loop'. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + # No .map/config.yaml — real defaults apply: wave_mode='auto', isolation='off'. + # Evict any cached map_step_runner so the function does a fresh import with + # the real module reading from tmp_path (monkeypatched cwd). + _orig_msr = _sys.modules.pop("map_step_runner", None) + try: + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + finally: + if _orig_msr is not None: + _sys.modules["map_step_runner"] = _orig_msr + + assert result["strategy"] == "sequential", ( + f"default config (no config.yaml) must give strategy='sequential': {result}" + ) + assert result["concurrency_allowed"] is False, ( + f"default config must give concurrency_allowed=False: {result}" + ) + assert result["worktree_isolation"] == "off", ( + f"default config must give worktree_isolation='off': {result}" + ) + # Confirm the isolation gate is what blocked wave_loop — not missing waves. + assert result["has_parallel_groups"] is True, ( + "has_parallel_groups must be True (width>=2 wave seeded) to prove the " + "isolation gate is what keeps the strategy sequential, not absence of a " + "parallelizable plan" + ) + + +def test_vc2_default_config_neutral_dispatch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """HC-1 proof: under default config, get_wave_step returns dispatch_mode=='sequential' + and isolation_active==False, and create_subtask_worktree returns status=='disabled' + (no worktree created). + + Width>=2 wave is seeded to prove the dispatch stays sequential regardless. + """ + import sys as _sys + + monkeypatch.chdir(tmp_path) + branch = "test-st007-vc2-default-neutral" + + # Seed a width>=2 wave — concurrently dispatchable IF isolation were on. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + # No config.yaml — real isolation defaults to 'off'. + _orig_msr = _sys.modules.pop("map_step_runner", None) + try: + result = map_orchestrator.get_wave_step(branch) + finally: + if _orig_msr is not None: + _sys.modules["map_step_runner"] = _orig_msr + + assert result["dispatch_mode"] == "sequential", ( + f"default config: dispatch_mode must be 'sequential': {result}" + ) + assert result["isolation_active"] is False, ( + f"default config: isolation_active must be False (isolation='off'): {result}" + ) + assert result["concurrency_enabled"] is False, ( + f"default config: concurrency_enabled alias must be False: {result}" + ) + + # create_subtask_worktree must no-op (status='disabled') under default config. + # The real _wt_isolation_enabled reads from cwd/.map/config.yaml (absent here). + _orig_msr2 = _sys.modules.pop("map_step_runner", None) + try: + import map_step_runner as _msr # noqa: E402 # pyright: ignore[reportMissingImports] + + wt_result = _msr.create_subtask_worktree("ST-001") + finally: + if _orig_msr2 is not None: + _sys.modules["map_step_runner"] = _orig_msr2 + + assert wt_result["status"] == "disabled", ( + f"default config: create_subtask_worktree must return status='disabled': {wt_result}" + ) + assert wt_result["ok"] is False, ( + f"default config: create_subtask_worktree must return ok=False: {wt_result}" + ) + + +def test_vc3_dormant_keys_do_not_flip_strategy( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """HC-1 proof: setting ONLY the dormant Slice-5a keys (execution.max_actors and + execution.retry_degraded_once) in config does NOT change strategy away from + sequential — strategy remains 'sequential' and concurrency_allowed remains False. + + Proves that the dormant fields parsed by ST-005 have no effect on the execution + path in Slice 5a: they are parsed and validated, but no code reads them yet. + Width>=2 wave is seeded so the isolation gate (not missing parallelizable work) + is the only thing keeping the dispatch sequential. + """ + import sys as _sys + + monkeypatch.chdir(tmp_path) + branch = "test-st007-vc3-dormant-keys" + + # Seed a width>=2 wave — parallelizable plan present. + _write_step_state(branch, tmp_path, execution_waves=[["ST-001", "ST-002"]]) + + # Write config with ONLY the dormant ST-005 keys — no wave_mode, no worktree.isolation. + # Real MapConfig defaults for the missing keys: wave_mode='auto', isolation='off'. + map_dir = tmp_path / ".map" + map_dir.mkdir(parents=True, exist_ok=True) + (map_dir / "config.yaml").write_text( + "execution.max_actors: 3\n" + "execution.retry_degraded_once: true\n", + encoding="utf-8", + ) + + _orig_msr = _sys.modules.pop("map_step_runner", None) + try: + result = map_orchestrator.select_execution_strategy(branch, tmp_path) + finally: + if _orig_msr is not None: + _sys.modules["map_step_runner"] = _orig_msr + + assert result["strategy"] == "sequential", ( + f"dormant-keys-only config must not flip strategy away from 'sequential': {result}" + ) + assert result["concurrency_allowed"] is False, ( + f"dormant-keys-only config must not flip concurrency_allowed to True: {result}" + ) + assert result["worktree_isolation"] == "off", ( + f"worktree_isolation must remain 'off' when worktree.isolation key is absent: {result}" + ) + assert result["has_parallel_groups"] is True, ( + "has_parallel_groups must be True so the isolation gate — not absent waves — " + "is what keeps the strategy sequential" + ) + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/tests/test_project_config.py b/tests/test_project_config.py new file mode 100644 index 00000000..224ae2b2 --- /dev/null +++ b/tests/test_project_config.py @@ -0,0 +1,183 @@ +"""ST-005: MapConfig dormant fields max_actors + retry_degraded_once, and clamp helper. + +Covers: + VC1 — dotted-key aliasing and field parsing from YAML + VC2 — clamp_max_actors truth table + VC3 — no runner/orchestrator .jinja in templates_src reads these fields +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +from mapify_cli.config.project_config import ( + MapConfig, + clamp_max_actors, + load_map_config, +) + +_TEMPLATES_SRC = Path(__file__).parent.parent / "src" / "mapify_cli" / "templates_src" + + +def _write_config(tmp_path: Path, body: str) -> None: + (tmp_path / ".map").mkdir(parents=True, exist_ok=True) + (tmp_path / ".map" / "config.yaml").write_text(body, encoding="utf-8") + + +class TestVc1MaxActorsAndRetryDegraded: + """VC1: dotted-key aliases are parsed; clamp and type fallback apply.""" + + def test_vc1_defaults(self): + cfg = MapConfig() + assert cfg.max_actors == 4 + assert cfg.retry_degraded_once is False + + def test_vc1_absent_config_uses_defaults(self, tmp_path: Path): + cfg = load_map_config(tmp_path) + assert cfg.max_actors == 4 + assert cfg.retry_degraded_once is False + + def test_vc1_max_actors_above_range_clamped_to_8(self, tmp_path: Path): + _write_config(tmp_path, "execution.max_actors: 12\n") + cfg = load_map_config(tmp_path) + assert cfg.max_actors == 8 + + def test_vc1_max_actors_zero_clamped_to_1(self, tmp_path: Path): + # 0 is a valid int (below floor 1) → clamped to 1, NOT the default 4. + # Only non-int/bool/None values fall back to the default 4. + _write_config(tmp_path, "execution.max_actors: 0\n") + cfg = load_map_config(tmp_path) + assert cfg.max_actors == 1 + + def test_vc1_max_actors_in_range_preserved(self, tmp_path: Path): + _write_config(tmp_path, "execution.max_actors: 3\n") + cfg = load_map_config(tmp_path) + assert cfg.max_actors == 3 + + def test_vc1_retry_degraded_once_true(self, tmp_path: Path): + _write_config(tmp_path, "execution.retry_degraded_once: true\n") + cfg = load_map_config(tmp_path) + assert cfg.retry_degraded_once is True + + def test_vc1_retry_degraded_once_default_false(self, tmp_path: Path): + _write_config(tmp_path, "profile: full\n") + cfg = load_map_config(tmp_path) + assert cfg.retry_degraded_once is False + + def test_vc1_max_actors_non_int_falls_back_to_default(self, tmp_path: Path): + _write_config(tmp_path, "execution.max_actors: \"notanint\"\n") + cfg = load_map_config(tmp_path) + # Generic type-check loop rejects non-int; clamp_max_actors on the + # default 4 returns 4. + assert cfg.max_actors == 4 + + +class TestVc2ClampMaxActors: + """VC2: clamp_max_actors truth table.""" + + def test_vc2_in_range_values_preserved(self): + for n in range(1, 9): + assert clamp_max_actors(n) == n, f"expected {n} for input {n}" + + def test_vc2_below_floor_clamped_to_1(self): + # int values below the floor (1) are clamped to 1, not the default 4. + assert clamp_max_actors(0) == 1 + assert clamp_max_actors(-5) == 1 + + def test_vc2_above_ceiling_clamped_to_8(self): + assert clamp_max_actors(9) == 8 + assert clamp_max_actors(100) == 8 + + def test_vc2_none_returns_default_4(self): + assert clamp_max_actors(None) == 4 + + def test_vc2_string_returns_default_4(self): + assert clamp_max_actors("4") == 4 + assert clamp_max_actors("bad") == 4 + + def test_vc2_bool_returns_default_4(self): + # bool is a subclass of int in Python; clamp_max_actors explicitly + # excludes bools so True/False return 4, not 1/0 clamped. + assert clamp_max_actors(True) == 4 + assert clamp_max_actors(False) == 4 + + def test_vc2_float_returns_default_4(self): + assert clamp_max_actors(4.0) == 4 + + def test_vc2_boundary_values(self): + assert clamp_max_actors(1) == 1 + assert clamp_max_actors(8) == 8 + + +class TestVc3DormantKeysUnused: + """VC3: no execution path in templates_src runner/orchestrator reads the fields.""" + + def _grep_templates_src(self, field_name: str) -> list[str]: + """Return lines from runner/orchestrator .jinja files that reference field_name.""" + if not _TEMPLATES_SRC.exists(): + return [] + matches = [] + for jinja_file in _TEMPLATES_SRC.rglob("*.jinja"): + # Only check runner and orchestrator files — those are the execution paths. + if not any( + tag in jinja_file.name + for tag in ("runner", "orchestrator", "step_runner", "wave") + ): + continue + content = jinja_file.read_text(encoding="utf-8") + for lineno, line in enumerate(content.splitlines(), 1): + if field_name in line: + matches.append(f"{jinja_file}:{lineno}: {line.rstrip()}") + return matches + + def test_vc3_max_actors_not_consumed_in_runner_orchestrator(self): + hits = self._grep_templates_src("max_actors") + assert hits == [], ( + "max_actors is DORMANT in Slice 5a — no runner/orchestrator .jinja " + "should reference it yet.\nFound:\n" + "\n".join(hits) + ) + + def test_vc3_retry_degraded_once_not_consumed_in_runner_orchestrator(self): + hits = self._grep_templates_src("retry_degraded_once") + assert hits == [], ( + "retry_degraded_once is DORMANT in Slice 5a — no runner/orchestrator " + ".jinja should reference it yet.\nFound:\n" + "\n".join(hits) + ) + + def test_vc3_field_defined_in_project_config(self): + """Positive proof: the fields exist on MapConfig.""" + cfg = MapConfig() + assert hasattr(cfg, "max_actors") + assert hasattr(cfg, "retry_degraded_once") + + def test_vc3_grep_subprocess_confirms_no_runner_orchestrator_consumer(self): + """Subprocess grep across runner/orchestrator/step_runner Python sources. + + VC3 dormant means: no execution dispatch path reads the fields. + Documentation files (.md, .md.jinja) and observability modules are + permitted to mention max_actors by name; only the runner/orchestrator + execution paths are forbidden in Slice 5a. + """ + src_root = Path(__file__).parent.parent / "src" / "mapify_cli" + result = subprocess.run( + ["grep", "-rl", "max_actors", str(src_root)], + capture_output=True, + text=True, + ) + files_with_max_actors = [ + line for line in result.stdout.splitlines() if line.strip() + ] + # Runner/orchestrator Python source files are forbidden in Slice 5a. + # Documentation (.md, .jinja) and observability modules are allowed. + _FORBIDDEN_STEMS = ("runner", "orchestrator", "step_runner", "wave_coordinator") + forbidden = [ + f for f in files_with_max_actors + if any(stem in Path(f).stem for stem in _FORBIDDEN_STEMS) + and f.endswith(".py") + ] + assert forbidden == [], ( + "max_actors found in runner/orchestrator Python sources in Slice 5a " + "(DORMANT violation — field must not be consumed until Slice 5b):\n" + + "\n".join(forbidden) + ) diff --git a/tests/test_wave_parallel_harness.py b/tests/test_wave_parallel_harness.py new file mode 100644 index 00000000..6a77b094 --- /dev/null +++ b/tests/test_wave_parallel_harness.py @@ -0,0 +1,348 @@ +"""Parallel-aware test harness for merge_wave_worktrees Q6 landmines (#303 Slice 5a). + +Drives the REAL merge_wave_worktrees function against real tmp git repos and real +worktrees. Distinct from the sequential tests in test_worktree_isolation.py. + +Coverage (five scenarios mapped to VCs): +- VC1: no orphan / unmanaged worktrees after partial failure +- VC2: generated-file conflict rolls back the whole wave (no subset merged) +- VC3: post-wave gate failure rolls back inside the transaction (HC-4) +- VC4a: no-op subtask classified as no_changes, excluded from merge +- VC4b: deterministic merge order by sorted subtask id +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import pytest + +# Mirror the import pattern used by test_worktree_isolation.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 as m # noqa: E402 # type: ignore[import-not-found] + + +# --------------------------------------------------------------------------- +# Shared helpers (mirrors test_worktree_isolation.py conventions) +# --------------------------------------------------------------------------- + + +def _git(args: list[str], cwd: Path) -> subprocess.CompletedProcess[str]: + return subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True) + + +def _make_repo(tmp_path: Path, branch: str = "feat/x") -> Path: + repo = tmp_path / "repo" + repo.mkdir() + _git(["init", "-q", "-b", branch], repo) + _git(["config", "user.email", "t@example.com"], repo) + _git(["config", "user.name", "Tester"], repo) + (repo / "a.txt").write_text("line1\nline2\nline3\n") + _git(["add", "-A"], repo) + _git(["commit", "-q", "-m", "init"], repo) + return repo + + +def _enable(repo: Path) -> None: + (repo / ".map").mkdir(exist_ok=True) + (repo / ".map" / "config.yaml").write_text( + "worktree.isolation: true\nworktree.max_deletions: 50\n", + encoding="utf-8", + ) + + +@pytest.fixture +def repo(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """Real tmp git repo with worktree isolation enabled; cwd monkeypatched.""" + r = _make_repo(tmp_path) + _enable(r) + monkeypatch.chdir(r) + return r + + +def _wt_with_files(sid: str, files: dict[str, str]) -> Path: + """Create a subtask worktree and write files (path -> content) into it.""" + created = m.create_subtask_worktree(sid) + assert created["status"] == "success", created + wt = Path(str(created["worktree_path"])) + for rel, content in files.items(): + (wt / rel).write_text(content, encoding="utf-8") + return wt + + +def _live_worktree_paths(repo: Path) -> list[str]: + """Return all paths reported by 'git worktree list' (includes the main checkout).""" + result = _git(["worktree", "list", "--porcelain"], repo) + paths = [] + for line in result.stdout.splitlines(): + if line.startswith("worktree "): + paths.append(line[len("worktree "):].strip()) + return paths + + +def _managed_worktree_paths() -> set[str]: + """Return paths of worktrees recorded in the MAP sidecar (active_worktrees). + + Reads via ``m.worktree_isolation_status()``, which resolves the sidecar from + the current working directory (the ``repo`` fixture has already chdir'd into + the tmp repo), so no repo path argument is needed. + """ + status = m.worktree_isolation_status() + active = status.get("active_worktrees", []) + assert isinstance(active, list) + return {str(w["path"]) for w in active if isinstance(w, dict) and "path" in w} + + +# --------------------------------------------------------------------------- +# VC1 — no orphan/unmanaged worktrees after partial failure +# --------------------------------------------------------------------------- + + +def test_vc1_no_orphan_worktrees_on_partial_failure(repo: Path) -> None: + """After a mid-wave conflict, git worktree list must show NO paths outside the + main checkout that are not recorded in the MAP sidecar (no stray/unmanaged + worktrees). The conflicting worktrees are intentionally kept for retry — that + is correct. HC-4: working branch is rolled back to wave base. + """ + base = _git(["rev-parse", "HEAD"], repo).stdout.strip() + + # Two subtasks that conflict on a.txt -> mid-wave failure after ST-001 merges + _wt_with_files("ST-001", {"a.txt": "conflict-from-001\n"}) + _wt_with_files("ST-002", {"a.txt": "conflict-from-002\n"}) + + result = m.merge_wave_worktrees( + ["ST-001", "ST-002"], verify_cmds=[], post_wave_cmds=[] + ) + + # Function returned an error + assert result["status"] == "error", result + assert result["kind"] == "WAVE_MERGE_CONFLICT" + + # HC-4: working branch rolled back to wave base + head_after = _git(["rev-parse", "HEAD"], repo).stdout.strip() + assert head_after == base, ( + f"HC-4 violated: working branch head {head_after!r} != base {base!r}" + ) + + # No orphan / unmanaged worktrees: every live worktree (except main checkout) + # must be registered in the MAP sidecar. + live_paths = _live_worktree_paths(repo) + managed_paths = _managed_worktree_paths() + main_checkout = str(repo.resolve()) + + stray = [ + p for p in live_paths + if p != main_checkout and p not in managed_paths + ] + assert stray == [], ( + f"VC1 violated: stray unmanaged worktrees after failure: {stray}. " + f"Live: {live_paths}. Managed: {managed_paths}" + ) + + # Worktrees kept for retry (both subtasks still registered) + status = m.worktree_isolation_status() + active_ids = {w["subtask_id"] for w in status["active_worktrees"]} + assert "ST-001" in active_ids or "ST-002" in active_ids, ( + "Expected at least one worktree kept for retry, got none" + ) + + +# --------------------------------------------------------------------------- +# VC2 — generated-file conflict rolls back the WHOLE wave +# --------------------------------------------------------------------------- + + +def test_vc2_generated_file_conflict_rolls_back_whole_wave(repo: Path) -> None: + """Two subtasks with disjoint declared affected_files both write conflicting + content to the same file -> merge conflict -> the function rolls the WHOLE + wave back (no subset merged) and returns kind=WAVE_MERGE_CONFLICT with + attribution naming the culprit subtasks. + """ + base = _git(["rev-parse", "HEAD"], repo).stdout.strip() + + # Both subtasks rewrite the same file with conflicting content. + # ST-001 goes first (sorted order), ST-002 creates the conflict. + _wt_with_files("ST-001", {"a.txt": "written-by-001\n"}) + _wt_with_files("ST-002", {"a.txt": "written-by-002\n"}) + + result = m.merge_wave_worktrees( + ["ST-002", "ST-001"], verify_cmds=[], post_wave_cmds=[] + ) + + # Real kind/reason from _wt_error for a merge conflict: + assert result["status"] == "error" + assert result["kind"] == "WAVE_MERGE_CONFLICT" + assert "conflict_files" in result + assert isinstance(result["conflict_files"], list) + assert "a.txt" in result["conflict_files"], ( + f"Expected a.txt in conflict_files, got: {result['conflict_files']}" + ) + + # HC-4: NO subset merged — branch back at wave base, no new commits + head_after = _git(["rev-parse", "HEAD"], repo).stdout.strip() + assert head_after == base, ( + f"HC-4 violated: subset was merged; head {head_after!r} != base {base!r}" + ) + + # a.txt must not hold either subtask's conflicting content (clean rollback) + a_content = (repo / "a.txt").read_text() + assert "written-by-001" not in a_content + assert "written-by-002" not in a_content + + # Attribution (AC-8): the conflict must be attributed to BOTH culprit + # subtasks. Both wrote a.txt and both pass PHASE-1 preflight, so both land + # in `prepared` with a.txt in changed_files -> _wt_attribute_conflict names + # both. Asserted unconditionally (no `if` guard) so a regression that drops + # attribution fails the test rather than silently skipping it. + attribution = result.get("attribution") + assert isinstance(attribution, list) and attribution, ( + f"Expected non-empty attribution list, got: {attribution!r}" + ) + attributed_ids = { + a["subtask_id"] for a in attribution if isinstance(a, dict) and "subtask_id" in a + } + assert {"ST-001", "ST-002"} <= attributed_ids, ( + f"AC-8: attribution must name both culprit subtasks; got {attributed_ids}" + ) + + # Worktrees intact for retry + status = m.worktree_isolation_status() + slugs = {w["subtask_id"] for w in status["active_worktrees"]} + assert {"ST-001", "ST-002"} <= slugs, ( + f"Expected both worktrees kept for retry, got: {slugs}" + ) + + +# --------------------------------------------------------------------------- +# VC3 — post-wave gate failure rolls back inside the transaction (HC-4) +# --------------------------------------------------------------------------- + + +def test_vc3_post_wave_gate_failure_rolls_back(repo: Path) -> None: + """Members pass per-worktree verify, the post-wave gate (false) exits non-zero. + The function rolls the WHOLE wave back to base (HC-4: post-wave gate runs + INSIDE the transaction, no partial merge survives). + """ + base = _git(["rev-parse", "HEAD"], repo).stdout.strip() + + _wt_with_files("ST-001", {"b.txt": "from-001\n"}) + _wt_with_files("ST-002", {"c.txt": "from-002\n"}) + + # verify_cmds=[] -> per-worktree verify passes; post_wave_cmds=["false"] -> fails + result = m.merge_wave_worktrees( + ["ST-001", "ST-002"], + verify_cmds=[], + post_wave_cmds=["false"], + ) + + # Real kind from _wt_error for post-wave gate failure: + assert result["status"] == "error" + assert result["kind"] == "WAVE_VERIFY_FAILED" + + # HC-4: working branch at wave base — no partial merge survived + head_after = _git(["rev-parse", "HEAD"], repo).stdout.strip() + assert head_after == base, ( + f"HC-4 violated: partial merge survived post-wave gate failure; " + f"head {head_after!r} != base {base!r}" + ) + + # No squash-committed files on the working branch + assert not (repo / "b.txt").exists(), "b.txt must not exist after rollback" + assert not (repo / "c.txt").exists(), "c.txt must not exist after rollback" + + # Worktrees intact for retry + status = m.worktree_isolation_status() + assert len(status["active_worktrees"]) == 2, ( + f"Expected 2 worktrees kept for retry, got: {status['active_worktrees']}" + ) + + +# --------------------------------------------------------------------------- +# VC4a — no-op subtask classified as no_changes, excluded from merge +# --------------------------------------------------------------------------- + + +def test_vc4_noop_classified(repo: Path) -> None: + """A wave member that produces no commit (no changes in its worktree) is + classified as no_changes and excluded from the merge, while remaining + members still merge successfully. + """ + # ST-001 worktree left empty (no files written) -> no_changes + m.create_subtask_worktree("ST-001") + # ST-002 has real changes + _wt_with_files("ST-002", {"d.txt": "from-002\n"}) + + result = m.merge_wave_worktrees( + ["ST-001", "ST-002"], verify_cmds=[], post_wave_cmds=[] + ) + + assert result["status"] == "success", result + + # Real field names from merge_wave_worktrees return dict: + # "merged" -> list[str] of merged subtask ids + # "no_changes" -> list[str] of no-op subtask ids + merged_ids = result["merged"] + no_change_ids = result["no_changes"] + + assert isinstance(merged_ids, list) + assert isinstance(no_change_ids, list) + + assert "ST-002" in merged_ids, f"ST-002 must be in merged: {merged_ids}" + assert "ST-001" not in merged_ids, f"ST-001 must NOT be in merged (no-op): {merged_ids}" + assert "ST-001" in no_change_ids, f"ST-001 must be in no_changes: {no_change_ids}" + + # d.txt landed on the working branch (ST-002 merged successfully) + assert (repo / "d.txt").exists(), "d.txt must be present after ST-002 merged" + + # Only one squash commit (ST-002); the no-op added none. + commit_count = _git(["rev-list", "--count", "HEAD"], repo).stdout.strip() + assert commit_count == "2", ( + f"Expected 2 commits (init + ST-002), got {commit_count}" + ) + + +# --------------------------------------------------------------------------- +# VC4b — deterministic squash-merge order by sorted subtask id +# --------------------------------------------------------------------------- + + +def test_vc4_deterministic_merge_order(repo: Path) -> None: + """The squash-merge order is by SORTED subtask id regardless of the input list + order. Confirm by checking commit subjects and the "merged" field ordering. + """ + _wt_with_files("ST-003", {"e.txt": "from-003\n"}) + _wt_with_files("ST-001", {"f.txt": "from-001\n"}) + _wt_with_files("ST-002", {"g.txt": "from-002\n"}) + + # Supply ids in reverse order; function must sort before merging. + result = m.merge_wave_worktrees( + ["ST-003", "ST-001", "ST-002"], verify_cmds=[], post_wave_cmds=[] + ) + assert result["status"] == "success", result + + # "merged" field must be in sorted order + merged_ids = result["merged"] + assert isinstance(merged_ids, list) + assert merged_ids == sorted(merged_ids), ( + f"VC4b violated: merged ids are not sorted: {merged_ids}" + ) + assert merged_ids == ["ST-001", "ST-002", "ST-003"], ( + f"Expected sorted ids, got: {merged_ids}" + ) + + # Commit subjects: newest commit = ST-003 (last merged), oldest new commit = ST-001 + subjects = _git(["log", "-3", "--format=%s"], repo).stdout.strip().splitlines() + assert subjects[0].startswith("ST-003:"), f"Newest commit should be ST-003, got: {subjects[0]}" + assert subjects[1].startswith("ST-002:"), f"Second commit should be ST-002, got: {subjects[1]}" + assert subjects[2].startswith("ST-001:"), f"Third commit should be ST-001, got: {subjects[2]}" diff --git a/tests/test_worktree_isolation.py b/tests/test_worktree_isolation.py index 0b659eb6..a2d4a528 100644 --- a/tests/test_worktree_isolation.py +++ b/tests/test_worktree_isolation.py @@ -613,3 +613,71 @@ def test_cli_wave_merge_unknown_subtask_exits_nonzero(self, repo: Path) -> None: assert proc.returncode == 1 out = json.loads(proc.stdout) assert out["kind"] == "NO_WORKTREE" + + +# --------------------------------------------------------------------------- # +# concurrency_ready — coordinator-owned read-only readiness check (ST-003/AC-3) +# --------------------------------------------------------------------------- # +class TestConcurrencyReady: + def test_vc1_concurrency_ready_all_clean(self, repo: Path) -> None: + """VC1: returns ready=True when all worktrees exist, registered, HEAD==base, clean.""" + del repo + _wt_with_files("ST-010", {}) + _wt_with_files("ST-011", {}) + result = m.concurrency_ready(["ST-010", "ST-011"]) + assert result["ready"] is True + assert result["reason"] is None + per = result["per_subtask"] + assert isinstance(per, dict) + assert per["ST-010"]["ok"] is True + assert per["ST-011"]["ok"] is True + + def test_vc1_concurrency_ready_dirty_member(self, repo: Path) -> None: + """VC1: returns ready=False with dirty reason when one worktree has uncommitted changes.""" + del repo + created = m.create_subtask_worktree("ST-020") + assert created["status"] == "success" + wt = Path(str(created["worktree_path"])) + # Write a real (non-runtime-state) file to make the worktree dirty. + (wt / "dirty.txt").write_text("uncommitted\n", encoding="utf-8") + + m.create_subtask_worktree("ST-021") + + result = m.concurrency_ready(["ST-020", "ST-021"]) + assert result["ready"] is False + per = result["per_subtask"] + assert per["ST-020"]["ok"] is False + assert per["ST-020"]["reason"] == "dirty" + assert per["ST-021"]["ok"] is True + assert result["reason"] == "dirty" + + def test_vc2_concurrency_ready_readonly(self, repo: Path) -> None: + """VC2: calling concurrency_ready does NOT create/merge/remove worktrees or move HEAD.""" + _wt_with_files("ST-030", {}) + _wt_with_files("ST-031", {}) + + wl_before = _git(["worktree", "list", "--porcelain"], repo).stdout + head_before = _git(["rev-parse", "HEAD"], repo).stdout.strip() + + result = m.concurrency_ready(["ST-030", "ST-031"]) + assert result["ready"] is True + + wl_after = _git(["worktree", "list", "--porcelain"], repo).stdout + head_after = _git(["rev-parse", "HEAD"], repo).stdout.strip() + + assert wl_before == wl_after, "concurrency_ready must not add/remove worktrees" + assert head_before == head_after, "concurrency_ready must not move HEAD" + + def test_vc3_concurrency_ready_disabled( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """VC3: when isolation is off / no worktrees recorded, returns structured result, never raises.""" + r = _make_repo(tmp_path) + # No worktree.isolation config -> isolation is off, no worktrees recorded. + monkeypatch.chdir(r) + result = m.concurrency_ready(["ST-040"]) + # Must not raise; must return a structured dict with ready=False. + assert isinstance(result, dict) + assert result["ready"] is False + assert "reason" in result + assert "per_subtask" in result