bootstrap/docs/tasks/MACP-PHASE1-fixes.md

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