conductor(plan): revise Phase 2 to Path C scope (additive _result variants)

conductor/tracks/data_oriented_error_handling_20260606/plan.md

@@ -645,22 +645,47 @@ git commit -m "conductor(plan): mark Phase 1 complete in data_oriented_error_han
 
 ---
 
-# Phase 2: `src/mcp_client.py` Refactor
+# Phase 2: `src/mcp_client.py` Refactor (Path C — conservative)
 
-> Goal: `(p, err)` tuple returns become `Result[Path]`. Tool functions return `Result[str]`. The 30+ `assert p is not None` chain (lines 304-794) is removed. Existing tests pass unchanged.
+> **Path C scope (per user decision 2026-06-12):** Add `*_result` variants of the existing mcp_client functions as NEW public functions, alongside the existing `(p, err)` tuple and `str` return API. Do NOT modify or remove the existing functions.
+>
+> **Why Path C:** The plan's original Phase 2 (full refactor of `(p, err)` tuples → `Result[Path]`, change `read_file`/`list_directory`/`search_files` returns) has huge blast radius:
+> - 50+ call sites in `src/mcp_client.py` itself
+> - 6+ test files (`test_mcp_ts_integration.py`, `test_ts_cpp_tools.py`, `test_ts_c_tools.py`, etc.) that monkey-patch `_resolve_and_check` to return `(Path(path), None)` — they would all break
+> - 2+ callers in `src/gui_2.py:3283, 3991, 4012` that expect `str` from `read_file()`
+> - `tests/test_mcp_client.py` (the main test file the plan assumes) does not exist
+>
+> Path C establishes the data-oriented convention in `src/mcp_client.py` incrementally without breaking anything. The old API is kept for backwards compatibility; the new `*_result` variants are available for new code and will be adopted by the dispatch layer in a follow-up track. The plan's spec §11 "Out of Scope" is satisfied: "established incrementally".
+>
+> **What Path C delivers:**
+> - `_resolve_and_check_result(raw_path) -> Result[Path]` — new function; uses the same resolve+allowlist logic
+> - `read_file_result(path) -> Result[str]` — new function; uses `_resolve_and_check_result`
+> - `list_directory_result(path) -> Result[str]` — new function
+> - `search_files_result(path, pattern) -> Result[str]` — new function
+> - Existing `_resolve_and_check`, `read_file`, `list_directory`, `search_files` unchanged
+> - `tests/test_mcp_client_paths.py` — minimal new tests for the new functions (not the comprehensive `test_mcp_client.py` from Path A)
+>
+> **Out of Path C (deferred to follow-up track):**
+> - The 30+ `assert p is not None` chain in the other tool functions
+> - Refactor of the remaining 30+ tool functions to return `Result[str]`
+> - Deprecation of the old `(p, err)` / `str` return API
+> - Update of `gui_2.py` callers to use `*_result` variants
+> - Update of 6+ test files that monkey-patch `_resolve_and_check`
 
 ---
 
-## Task 2.1: Baseline: verify existing mcp_client tests pass before refactor
+## Task 2.1: Baseline: verify the 4 existing mcp test files pass (Path C scope)
 
 **Files:** none (verification only)
 
-- [ ] **Step 1: Run tests/test_mcp_client.py and record pass/fail counts**
+- [x] **Step 1: Confirm `tests/test_mcp_client.py` does NOT exist (Path A's pre-condition is not needed for Path C)** (DONE 2026-06-12: 4 mcp test files exist: test_mcp_client_beads, test_mcp_config, test_mcp_perf_tool, test_mcp_ts_integration; no test_mcp_client.py)
 
-Run: `uv run pytest tests/test_mcp_client.py -v 2>&1 | tail -20`
-Expected: all existing tests pass.
+Run: `ls tests/test_mcp*.py` (or `Get-ChildItem tests -Filter "test_mcp*.py"`)
 
-- [ ] **Step 2: Record pass count in state.toml under `[mcp_client_refactor_stats]`**
+- [ ] **Step 2: Run the 4 existing mcp test files to confirm they all pass before adding new _result variants**
+
+Run: `uv run pytest tests/test_mcp_client_beads.py tests/test_mcp_config.py tests/test_mcp_perf_tool.py tests/test_mcp_ts_integration.py -v 2>&1 | tail -30`
+Expected: all tests pass (Path C is purely additive; the existing functions and their callers are untouched).
 
 - [ ] **Step 3: Commit nothing (baseline)**