# MACP Phase 1 — Review Fixes

**Branch:** `feat/macp-phase1` (amend/fix commits on top of existing)
**Repo worktree:** `~/src/mosaic-bootstrap-worktrees/macp-phase1`

These are 5 findings from code review and security review. Fix all of them.

---

## Fix 1 (BLOCKER): Legacy task reclassification in tasks_md_sync.py

**File:** `tools/orchestrator-matrix/controller/tasks_md_sync.py`
**Problem:** `build_task()` now always sets `task["dispatch"]` from `macp_defaults` even when the `docs/TASKS.md` table has no `dispatch` column. The controller's `is_macp_task()` check is simply `"dispatch" in task`, so ALL synced tasks get reclassified as MACP tasks and routed through the dispatcher instead of the legacy `run_shell()` path. This breaks backward compatibility.

**Fix:** Only populate `dispatch`, `type`, `runtime`, and `branch` from MACP defaults when the markdown table row **explicitly contains that column**. If the column is absent from the table header, do NOT inject MACP fields. Check whether the parsed headers include these column names before adding them.

---

## Fix 2 (BLOCKER): ACP dispatch is a no-op that falsely reports success

**File:** `tools/orchestrator-matrix/dispatcher/macp_dispatcher.py`
**Problem:** For `dispatch == "acp"`, `build_dispatch_command()` returns a `python3 -c ...` command that only prints JSON but doesn't actually spawn an ACP session. The controller then marks the task completed even though no work ran.

**Fix:** For `acp` dispatch, do NOT generate a command that runs and exits 0. Instead, set the task status to `"escalated"` with `escalation_reason = "ACP dispatch requires OpenClaw integration (Phase 2)"` and return exit code 1. This makes it fail-safe — ACP tasks won't silently succeed. The controller should emit a `task.escalated` event for these.

---

## Fix 3 (SHOULD-FIX): Premature result write before quality gates

**File:** `tools/orchestrator-matrix/dispatcher/macp_dispatcher.py`
**Problem:** `dispatch_task()` calls `collect_result(task, exit_code, [], orch_dir)` immediately after the worker exits, before quality gates run. If the controller crashes between this write and the final overwrite, a false "completed" result with empty gate_results sits on disk.

**Fix:** Remove the `collect_result()` call from `dispatch_task()`. Instead, have `dispatch_task()` return `(exit_code, output)` only. The controller in `mosaic_orchestrator.py` should call `collect_result()` AFTER quality gates have run, when the final status is known. Make sure the controller passes gate_results to collect_result.

---

## Fix 4 (SECURITY MEDIUM): Brief contents exposed in process arguments

**File:** `tools/orchestrator-matrix/dispatcher/macp_dispatcher.py`
**Problem:** For `yolo` dispatch, `build_dispatch_command()` puts the full task brief text into the shell command as an argument: `mosaic yolo codex "<entire brief>"`. This is visible in `ps` output, shell logs, and crash reports.

**Fix:** Write the brief contents to a temporary file with restrictive permissions (0600), and pass the file path to the worker instead. The command should read: `mosaic yolo <runtime> "$(cat /path/to/brief-TASKID.tmp)"` or better, restructure so the worker script reads from a file path argument. Use the existing `orchestrator-worker.sh` pattern which already reads from a task file. Clean up the temp file after dispatch completes.

---

## Fix 5 (SECURITY MEDIUM): Unrestricted worktree cleanup path

**File:** `tools/orchestrator-matrix/dispatcher/macp_dispatcher.py`  
**Problem:** `cleanup_worktree()` trusts `task['worktree']` without validating it belongs to the expected worktree base directory. A tampered task could cause deletion of unrelated worktrees.

**Fix:** Before running `git worktree remove`, validate that the resolved worktree path starts with the configured `worktree_base` (from `config.macp.worktree_base`, with `{repo}` expanded). If the path doesn't match the expected base, log a warning and refuse to clean up. Add a helper `_is_safe_worktree_path(worktree_path, config)` for this validation.

---

## Verification

After fixes:
1. `python3 -c "from tools.orchestrator_matrix.dispatcher import macp_dispatcher"` should not error (fix import path if needed, or just verify `python3 tools/orchestrator-matrix/dispatcher/macp_dispatcher.py` has no syntax errors)
2. Legacy tasks.json with no `dispatch` field must still work with the controller's `run_shell()` path
3. ACP dispatch should NOT mark tasks completed — should escalate
4. No brief text should appear in generated shell commands for yolo dispatch
5. `cleanup_worktree()` should refuse paths outside the configured base

Commit with: `fix: address review findings — backward compat, ACP safety, result timing, security`