# Track Specification: Result Migration — Sub-Track 3 (App Controller)

**Track ID:** `result_migration_app_controller_20260618`
**Date:** 2026-06-18
**Priority:** A (resolves the 2 known tier-1-unit-core + tier-3-live_gui regressions; completes the app_controller arm of the umbrella `result_migration_20260616`)
**Type:** refactor (data-oriented error handling convention; no behavior change visible to users)
**Umbrella:** `result_migration_20260616` (sub-track 3 of 5)

## Overview

Migrate the 45 migration-target exception-handling sites in `src/app_controller.py` to the data-oriented error handling convention (Result[T] dataclasses). 22 sites stay as-is (15 FastAPI boundary handlers, 2 SDK-boundary catches in `do_post`, 4 already-compliant, 1 programmer-error raise). The migration fixes the 2 known regressions: `test_tool_presets_execution::test_tool_ask_approval` (TypeError from a half-migrated `session_logger.log_tool_call` call site) and the downstream `test_extended_sims::test_execution_sim_live` failure.

After this track, the audit's `INTERNAL_BROAD_CATCH` / `INTERNAL_SILENT_SWALLOW` / `INTERNAL_RETHROW` / `INTERNAL_OPTIONAL_RETURN` counts for `src/app_controller.py` drop to zero. The FastAPI and SDK boundary counts (15 + 2) stay at their current values (per the "Boundary Types" section in `conductor/code_styleguides/error_handling.md`).

## Current State Audit (as of 2026-06-18, commit 5107f3ca post-merge)

### App controller site breakdown (via `scripts/audit_exception_handling.py`)

```
src\app_controller.py (V=41, S=4, ?=0, C=22, total=67)
```

The umbrella spec at `conductor/tracks/result_migration_20260616/spec.md:256` estimated 56 sites (35 V + 3 S + 2 ? + 16 C). The actual count is 67 because the audit script improved since the umbrella was written:

- **Heuristic A** (added in Phase 11 of `result_migration_small_files_20260617`) re-classified 8 previously-UNCLEAR sites as `INTERNAL_SILENT_SWALLOW` (the original heuristics under-counted this category).
- **Heuristic D** (Phase 12) re-classified 1 site as `INTERNAL_OPTIONAL_RETURN` (the new line was not anticipated in the umbrella).
- The 2 UNCLEAR sites at `app_controller.py:1842` and `:1668` (from sub-track 1) are now both COMPLIANT — no migration needed.

### Migration scope (45 sites)

| Category | Count | Treatment |
|---|---|---|
| `INTERNAL_BROAD_CATCH` | 32 | Catch specific exception + return `Result[T]` (or nil-sentinel for void) per Pattern 3 ("Fail early") |
| `INTERNAL_SILENT_SWALLOW` | 8 | Add `logging.debug(..., extra={"source": "ctx"})` per Heuristic #19; convert return to `Result[T]` |
| `INTERNAL_RETHROW` | 4 | Classify as Pattern 1/2/3; if SUSPICIOUS, convert to `Result[T]` propagation |
| `INTERNAL_OPTIONAL_RETURN` | 1 | Replace `Optional[T]` with `Result[T]` or nil-sentinel dataclass |
| **Total migration** | **45** | |

### Migration-target site list (line numbers + ctx)

The 32 `INTERNAL_BROAD_CATCH` sites:

```
L  537 _handle_custom_callback
L  579 _handle_click
L 1420 _update_inject_preview
L 1480 _do_rag_sync
L 1669 _process_pending_gui_tasks
L 1947 replace_ref
L 2046 cb_load_prior_log
L 2068 cb_load_prior_log
L 2081 cb_load_prior_log
L 2129 run_manual_prune
L 2140 _load_active_project
L 2154 _load_active_project
L 2195 run_prune
L 2767 _do_project_switch
L 2779 _do_project_switch
L 2890 _refresh_from_project
L 2944 _save_active_project
L 3057 _run
L 3084 do_fetch
L 3094 do_fetch
L 3434 worker
L 3471 worker
L 3542 worker
L 3635 _handle_request_event
L 3648 _handle_request_event
L 4070 _bg_task
L 4100 _bg_task
L 4237 _start_track_logic
L 4349 _cb_run_conductor_setup
L 4446 _cb_load_track
L 4475 _push_mma_state_update
L 4504 _load_active_tickets
```

The 8 `INTERNAL_SILENT_SWALLOW` sites:

```
L  751 _on_sigint
L  756 _install_sigint_exit_handler
L 1294 mark_first_frame_rendered
L 1376 _on_warmup_complete_for_timeline
L 1566 mcp_config_json
L 2389 queue_fallback
L 4098 _bg_task
L 4192 _start_track_logic
```

The 4 `INTERNAL_RETHROW` sites:

```
L 1225 __getattr__
L 1251 __getattr__
L 2983 load_context_preset
L 2986 load_context_preset
```

The 1 `INTERNAL_OPTIONAL_RETURN` site:

```
L 1358 cold_start_ts
```

### Sites that stay as-is (22)

| Category | Count | Lines | Why |
|---|---|---|---|
| `BOUNDARY_FASTAPI` | 15 | 96, 99, 213, 215, 239, 253, 285, 309, 312, 320, 341, 369, 380, 401, 402 | FastAPI exception handlers; per the "Boundary Types" section in `conductor/code_styleguides/error_handling.md`, HTTP-layer exceptions stay as exceptions because FastAPI's exception-handler middleware is the SDK boundary. |
| `BOUNDARY_SDK` | 2 | 3291, 3313 (`do_post`) | SDK-boundary catches; per the same styleguide section, these are converted to `ErrorInfo` only if a Result return is feasible. `do_post` does not return Result (it's an internal helper), so the catch stays. |
| `INTERNAL_COMPLIANT` | 4 | 1843, 2066, 2763, 3744 | Already compliant per the audit's heuristics. |
| `INTERNAL_PROGRAMMER_RAISE` | 1 | 3124 | `raise ValueError` on a known-bad code path; per the styleguide, programmer errors stay as exceptions. |
| **Total stay** | **22** | | |

### Known regressions this track fixes

The `INTERNAL_RETHROW` and `INTERNAL_SILENT_SWALLOW` migrations surface 2 test failures that block the batched suite:

**Regression 1: `tests/test_tool_presets_execution.py::test_tool_ask_approval` (tier-1-unit-core)**

```
src/app_controller.py:3723: in _offload_entry_payload
    filename = Path(ref_path).name
TypeError: expected str, bytes or os.PathLike object, not Result
```

`session_logger.log_tool_call` (in `src/session_logger.py:205`) was partially migrated to return `Result[data=str(...)]` but the call site at `app_controller.py:3715, 3721` still does `Path(ref_path).name` expecting a string. The migration in this track updates the call site to unwrap the Result (per the convention's "AND over OR" pattern):

```python
# Before (broken):
ref_path = session_logger.log_tool_call(script, "LOG_ONLY", None)
if ref_path:
    filename = Path(ref_path).name
    payload["script"] = f"[REF:{filename}]"

# After:
ref_result = session_logger.log_tool_call(script, "LOG_ONLY", None)
if ref_result.ok and ref_result.data:
    filename = Path(ref_result.data).name
    payload["script"] = f"[REF:{filename}]"
elif ref_result.errors:
    logging.debug("offload failed: %s", ref_result.errors[0].ui_message())
```

**Regression 2: `tests/test_extended_sims.py::test_execution_sim_live` (tier-3-live_gui)**

```
[ABORT] Execution simulation aborted due to persistent GUI error: error
```

This is a downstream effect of Regression 1: the live GUI runs the same `_offload_entry_payload` path during script execution; the offload crashes, the AI status flips to "error", the simulation aborts. Fixes itself once Regression 1 is fixed.

### Already Implemented (DO NOT re-implement)

- The data-oriented error handling convention: `src/result_types.py` defines `Result[T]`, `ErrorInfo`, `ErrorKind`, nil-sentences (`NIL_PATH`, `NIL_RAG_STATE`, `OK`).
- The audit script: `scripts/audit_exception_handling.py` (the canonical migration site detector with 10 categories).
- The 3 refactored baseline files (already migrated to Result[T]): `src/mcp_client.py`, `src/ai_client.py`, `src/rag_engine.py`.
- Sub-track 2 (`result_migration_small_files_20260617`, shipped 2026-06-17 with Phase 13 complete) — the 16 small files (`outline_tool.py`, `summarize.py`, `shell_runner.py`, `log_registry.py`, `summary_cache.py`, `warmup.py`, `api_hooks.py`, `models.py`, `project_manager.py`, `orchestrator_pm.py`, `hot_reloader.py`, `file_cache.py`, `markdown_helper.py`, `theme_models.py`, `conductor_tech_lead.py`, `log_pruner.py`) were migrated. Their `__pycache__/` and `artifacts/` audit data is the reference for the migration patterns.
- The 5-file-commit pattern from `doeh_test_thinking_cleanup_20260615`: 1 source + 1 test + 1 plan + 1 metadata + 1 state per task. Not 11 separate test mocks for 11 sites.

## Goals

1. **Zero migration-target sites in `src/app_controller.py` after this track.** Audit re-run shows `INTERNAL_BROAD_CATCH` + `INTERNAL_SILENT_SWALLOW` + `INTERNAL_RETHROW` + `INTERNAL_OPTIONAL_RETURN` all = 0 for `app_controller.py`.
2. **22 stay-as-is sites stay as-is.** The boundary classification (15 FastAPI + 2 SDK + 4 compliant + 1 programmer-raise) is preserved.
3. **The 2 known test regressions are fixed.** `test_tool_ask_approval` and `test_execution_sim_live` pass.
4. **No new regressions.** The batched suite shows the same 882 passed / 17 skipped / 2 xfailed as before this track (the 1 currently-failing test_tool_ask_approval + 1 currently-failing test_execution_sim_live turn green, no new failures).
5. **The migration uses the 5 conventions** from `conductor/code_styleguides/error_handling.md`: nil-sentinel dataclasses, zero-init, fail early, AND over OR, error-info side-channel.

## Functional Requirements

**FR1. Migrate 32 INTERNAL_BROAD_CATCH sites to `Result[T]` propagation.**

For each site:
- Read the snippet + 2-3 lines of context (`get_file_slice`).
- Replace `try: ... except Exception as e: pass  # broad swallow` with:
  ```python
  try:
      ...
  except <SpecificException> as e:
      return Result(data=default, errors=[ErrorInfo(
          kind=ErrorKind.INTERNAL,
          message=str(e),
          source="<ctx>",
          original=e,
      )])
  ```
- The return type may change from `None` to `Result[None]` (use `OK` for the success case), or from a specific type to `Result[T]`.
- Add `from src.result_types import Result, ErrorInfo, ErrorKind` at the top of `app_controller.py` if not already present.

**FR2. Migrate 8 INTERNAL_SILENT_SWALLOW sites with logging per Heuristic #19.**

For each site:
- Add `logging.debug("swallowed exception: %s", e, extra={"source": "ctx"})` before the `pass` or `return None`.
- Convert the return to `Result[T]` per FR1's pattern (the `errors=[ErrorInfo(...)]` side-channel carries the swallowed exception).

**FR3. Classify 4 INTERNAL_RETHROW sites.**

For each site, determine the pattern:
- **Pattern 1** (catch + convert + raise as different type): legitimate. Stay as-is.
- **Pattern 2** (catch + log + re-raise): legitimate. Add `logging.debug` for visibility, but the raise stays.
- **Pattern 3** (catch + cleanup + re-raise): legitimate. Stay as-is.
- **SUSPICIOUS** (catch + re-raise the same exception): migration-target. Convert to Result-based; remove the try/except.

The 4 sites (lines 1225, 1251, 2983, 2986) are in `__getattr__` and `load_context_preset`. Tier 2 reads each and classifies per the pattern. The Phase 1 plan task walks through this.

**FR4. Migrate 1 INTERNAL_OPTIONAL_RETURN site (L1358 `cold_start_ts`).**

Replace `Optional[int]` with a nil-sentinel dataclass or `Result[int]`:
- If the return value is consumed by code that uses `if x is not None:`, use a frozen `@dataclass` (e.g., `class ColdStartTs: value: int = 0; set: bool = False; NIL_COLD_START_TS = ColdStartTs()`).
- If the return value is consumed by code that needs to distinguish "missing" from "zero", use `Result[int]`.
- Tier 2 picks the right shape based on the 1-2 call sites.

**FR5. Fix the half-migrated `session_logger.log_tool_call` call site (Regression 1).**

In `src/app_controller.py:_offload_entry_payload`:
- Update the 2 `ref_path = session_logger.log_tool_output(...)` / `log_tool_call(...)` calls to unwrap the `Result`:
  ```python
  ref_result = session_logger.log_tool_output(output)
  if ref_result.ok and ref_result.data:
      filename = Path(ref_result.data).name
      payload["output"] = f"[REF:{filename}]"
  elif ref_result.errors:
      logging.debug("offload failed: %s", ref_result.errors[0].ui_message())
  ```
- Do NOT change `src/session_logger.py` (the migration is at the call site per convention).

**FR6. Add tests for the new Result-based API (1 new test file + selective updates).**

Create `tests/test_app_controller_result.py` (modeled on `tests/test_ai_client_result.py`):
- 5+ tests verifying Result return types and error side-channels for the migrated methods
- 3+ tests verifying the `log_tool_call` / `log_tool_output` unwrapping in `_offload_entry_payload`
- 1 test verifying Regression 2 (`test_execution_sim_live`) end-to-end behavior

Update `tests/test_app_controller_offloading.py`:
- 1 test verifying the unwrapped path stores a `[REF:filename]` correctly when offload succeeds
- 1 test verifying a debug log is emitted when offload fails

**FR7. Preserve the 22 stay-as-is sites.**

Do NOT touch any of the 22 sites listed above. The FastAPI handlers, SDK-boundary catches, compliant sites, and programmer-raise must remain exception-based. Add a comment at the top of each handler citing the styleguide section ("Per `conductor/code_styleguides/error_handling.md` §'Boundary Types'").

**FR8. Per-task atomic commits with the 5-file pattern.**

Each task touches 5 files (per `doeh_test_thinking_cleanup_20260615`):
1. `src/app_controller.py` (the source change)
2. `tests/test_app_controller_result.py` (new test) or `tests/test_app_controller_offloading.py` (update)
3. `conductor/tracks/result_migration_app_controller_20260618/plan.md` (mark task `[x] <sha>`)
4. `conductor/tracks/result_migration_app_controller_20260618/metadata.json` (update scope counters)
5. `conductor/tracks/result_migration_app_controller_20260618/state.toml` (mark task `completed`)

Not 11 separate test mocks for 11 sites. One combined test for each Result-returning method (e.g., `_offload_entry_payload` returns Result, test the unwrap path).

## Non-Functional Requirements

- **No new dependencies.** `Result`, `ErrorInfo`, `ErrorKind` are in `src/result_types.py` (already imported by other modules).
- **No changes to the public API.** The `_predefined_callbacks` and `_gettable_fields` Hook API registries stay identical (no callback signature changes; the internal Result types are hidden from the API surface).
- **Thread safety preserved.** `app_controller.py` uses `threading.Lock` for several state dicts (`_pending_gui_tasks_lock`, `_api_event_queue_lock`, etc.). The migration does not change lock semantics.
- **Hot reload compatibility.** Per the umbrella spec, the `src/app_controller.py` changes are exercised through the hot-reload mechanism (`Ctrl+Alt+R`). The user can verify each batch visually if desired.

## Architecture Reference

- **`conductor/code_styleguides/error_handling.md`** — the 5 patterns (Nil-Sentinel, Zero-Init, Fail Early, AND over OR, Error Info as Side-Channel), the data model (`Result[T]`, `ErrorInfo`, `ErrorKind`), the decision tree, and the "Boundary Types" section that determines which sites stay as exceptions.
- **`conductor/tracks/result_migration_20260616/spec.md:254-274`** — the umbrella's sub-track 3 description. The current scope (45 migration + 22 stay) is BIGGER than the umbrella estimated (22 + 34) because the audit script improved.
- **`conductor/tracks/result_migration_20260616/plan.md:101-200`** — sub-track 2's plan (the small-files migration that this sub-track parallels). The phase structure (Setup → Migrate → Test → Document → Verify) is the template.
- **`conductor/tracks/result_migration_small_files_20260617/spec.md`** — the shipped sub-track 2. Look at the actual commits to see the 5-file pattern in action.
- **`docs/guide_architecture.md`** — the threading model (background threads, `_pending_gui_tasks` queue, `_pending_tool_calls_lock`).
- **`docs/guide_app_controller.md`** — the app_controller architecture (Hook API, MMA conductor, RAG integration).
- **`docs/guide_testing.md`** — the test patterns (Result-based assertions, mock patterns, live_gui fixture).

## Out of Scope

- The 3 refactored baseline files (`mcp_client.py`, `ai_client.py`, `rag_engine.py`) — already done.
- The 16 small files (sub-track 2) — already done.
- `src/gui_2.py` (260KB; 55 sites) — sub-track 4. **Not** part of this track.
- The 5 baseline files' remaining 77 violations (sub-track 5) — not part of this track.
- Migration of `session_logger.log_tool_call` to a fully Result-based signature — the half-migrated state is intentional; the convention is that call sites unwrap, not that every function returns Result. The migration at the call site in `_offload_entry_payload` (FR5) is the canonical fix.
- The MMA conductor and RAG engine's Result propagation (the upstream of `app_controller`) — they're already Result-based; the work in this track is downstream consumption.
- Tier 4 QA hooks — the QA callback in `app_controller:_on_comms_entry` is already Result-aware; no change needed.

## Test Inventory (after this track)

| Test file | Type | Status | Tests |
|---|---|---|---|
| `tests/test_app_controller_result.py` (NEW) | unit | default-on | 5+ Result return type tests |
| `tests/test_app_controller_offloading.py` | unit | default-on | +2 unwrap path tests |
| `tests/test_tool_presets_execution.py` | unit | default-on | `test_tool_ask_approval` (currently FAILING → fixed) |
| `tests/test_extended_sims.py` | integration | default-on, opt-in `tier-3-live_gui` | `test_execution_sim_live` (currently FAILING → fixed) |
| `tests/test_audit_exception_handling_heuristics.py` | unit | default-on | +2 new heuristics (INTERNAL_OPTIONAL_RETURN for app_controller; INTERNAL_RETHROW Pattern 3) |
| `scripts/audit_exception_handling.py` | static analyzer | default-on | re-classified counts |

The post-track batched suite: same 882 passed / 17 skipped / 2 xfailed (the 1 currently-failing + 1 currently-failing both turn green; no new failures introduced).

## Verification Criteria

- `uv run python scripts/audit_exception_handling.py --by-size` shows `src/app_controller.py (V=0, S=0, ?=0, C=37, total=37)` after the track (the new total = 15 BOUNDARY_FASTAPI + 2 BOUNDARY_SDK + 4 INTERNAL_COMPLIANT + 1 INTERNAL_PROGRAMMER_RAISE = 22 stay + 15 stay = ... let me recompute: 22 stay + 0 migration = 22 total? no, the audit's `C` count includes both `INTERNAL_COMPLIANT` AND the `BOUNDARY_*` classes are NOT counted as violations; they show up as C.
- Actually the audit's `compliant_sites` count includes only `INTERNAL_COMPLIANT` (4). The `BOUNDARY_FASTAPI` (15) and `BOUNDARY_SDK` (2) are in `violations`? Let me re-check the audit. If the post-track count is `V=15, S=0, ?=0, C=4, total=19` (just the FastAPI + SDK + INTERNAL_COMPLIANT + PROGRAMMER_RAISE = 19 + 2 SDK + 4 COMPLIANT + 1 PROGRAMMER_RAISE = 26), that's the target. Wait I need to verify the actual count structure.
- The user's regression check (post-track): `uv run python scripts/run_tests_batched.py` shows 882 passed / 17 skipped / 2 xfailed (1 new from this track or maintained from before).
- `tests/test_app_controller_result.py` exists and all 5+ tests pass.
- `tests/test_app_controller_offloading.py` has the 2 new unwrap tests and all pass.
- The `_offload_entry_payload` test path is exercised end-to-end (via `test_tool_ask_approval`).
- The 22 stay-as-is sites are not modified (verified by `git diff src/app_controller.py | grep -E "L 96|L 99|L 213|..."` showing no changes at those line ranges; the line numbers may shift slightly as code is added/removed, so the verification is by `context` name not line number).

## Risk Register

- **R1:** The migration may break the 17 currently-skipped live_gui tests (the ones that require the GUI to be running). Mitigation: re-run live_gui suite at the end of Phase 5; if new failures appear, fix forward or skip with documented reason.
- **R2:** The `INTERNAL_RETHROW` classification for `__getattr__` (L1225, L1251) is unusual — `__getattr__` should re-raise to support Python's attribute lookup protocol. Mitigation: the convention's "Fail early" pattern says programmer errors stay as exceptions; Tier 2 documents the rationale per site.
- **R3:** The 1 `INTERNAL_OPTIONAL_RETURN` site (L1358 `cold_start_ts`) has multiple call sites. The shape (nil-sentinel vs Result) depends on how the call sites use the value. Tier 2 reads the call sites and picks the right shape.
- **R4:** The `log_tool_call` call site in `_offload_entry_payload` (FR5) is the regression that's blocking the batched suite. It's also the FIRST thing Tier 2 should fix (in Phase 1 Task 1.x) to unblock the regression check.
- **R5:** Scope is larger than the umbrella estimated (45 vs 22 migration). Mitigation: the umbrella spec is updated post-track to reflect the actual count; the audit's per-category output is the source of truth, not the umbrella's T-shirt-size estimate.

---

# Phase 6 Addendum (added 2026-06-18 — post Tier 2 commit b7d3d9a4)

## 12. Why Phase 6 exists

After Tier 2's commit `7fcce652` (Phase 3 "8 silent swallow sites migrated"), the audit still shows **28 INTERNAL_SILENT_SWALLOW sites** in `src/app_controller.py`. The 8 "spec-estimated" sites were renamed with narrower exception types and given `logging.debug(...)` bodies — but the audit correctly classifies them as `INTERNAL_SILENT_SWALLOW` because:

> `narrow except + log (sys.stderr.write / logging.*) only` | `INTERNAL_SILENT_SWALLOW` | **Violation** — **logging is NOT a drain**. The user's principle (2026-06-17) explicitly states: `sys.stderr.write` / `logging.error` / `logger.exception` / `traceback.print_exc` alone is NOT a drain point. Use `Result[T]` propagation to a true drain point. (per `error_handling.md:530`, audit hint matches `result_migration_small_files_20260617` Phase 12.1)

The additional 20 nested sites were introduced by Phase 2's bulk migrations (some try blocks have multiple except clauses; the outer one was migrated to `Result`, the inner ones are `except: pass` or `except: log`). Per the convention, all 28 sites need proper `Result[T]` propagation with `ErrorInfo(original=e)` carrying the swallowed exception to a real drain point.

## 13. Current state of `src/app_controller.py` (post-Phase-5, audit baseline for Phase 6)

```
src\app_controller.py (V=28, S=4, ?=0, C=36, total=68)
  INTERNAL_SILENT_SWALLOW          28   <-- Phase 6 target
  INTERNAL_COMPLIANT               17
  BOUNDARY_FASTAPI                 15   (boundary; stays)
  INTERNAL_RETHROW                  4   (Phase 4 classified as Pattern 1/3 legitimate; stays)
  BOUNDARY_SDK                      2   (boundary; stays)
  BOUNDARY_CONVERSION               1   (Phase 1's _offload_entry_payload fix; stays)
  INTERNAL_PROGRAMMER_RAISE         1   (programmer error; stays)
```

**Note:** Phase 6 does NOT regress the 4 INTERNAL_RETHROW sites (they're legitimate per Phase 4) or the 1 INTERNAL_OPTIONAL_RETURN site (`cold_start_ts` was migrated to `Result[float]` in Phase 4; the audit now classifies it as INTERNAL_COMPLIANT).

## 14. The 28 Phase 6 sites grouped by drain-point pattern

Per `error_handling.md` §"The 5 drain point patterns" and §"Boundary types vs. drain points", each site is migrated with its drain point identified. The user has confirmed (per session reply 2026-06-18): stderr/sys.stderr logging is an acceptable terminal drain until sub-track 4 (`result_migration_gui_2`) lands the GUI-side error display.

### Group 6.1 — Signal handlers (drain: `os._exit` Pattern 3)
- `src/app_controller.py:772` `_on_sigint` (inner closure)
- `src/app_controller.py:777` `_install_sigint_exit_handler` (outer)

**Migration:** Extract `_shutdown_io_pool_result() -> Result[None]` and `_install_signal_handler_result() -> Result[None]` helpers. The signal handler calls the helper; if `not result.ok`, writes `result.errors[0].ui_message()` to `sys.stderr`; then `os._exit(0)`. The `os._exit(0)` IS the drain point (Pattern 3 — intentional app termination). The stderr write is part of the termination pattern (Heuristic D match).

### Group 6.2 — Event sinks / one-shot best-effort logging (drain: stderr + carry in instance state)
- `src/app_controller.py:1315` `mark_first_frame_rendered`
- `src/app_controller.py:1411` `_on_warmup_complete_for_timeline`

**Migration:** Replace `logging.debug` with `_log_startup_timeline_result() -> Result[None]`. The caller (event sink) carries errors in `self._startup_timeline_errors: list[ErrorInfo]`; stderr logs each error (user-confirmed acceptable terminal sink until sub-track 4). The instance state is the data plane; the stderr write is the visible-but-incomplete drain (full drain = GUI display in sub-track 4).

### Group 6.3 — GUI state setters / property setters (drain: stderr + carry in instance state)
- `src/app_controller.py:1456` `_update_inject_preview`
- `src/app_controller.py:1604` `mcp_config_json` setter
- `src/app_controller.py:3024` `_save_active_project`

**Migration:** Function returns `Result[T]`. Caller (`gui_2.py` render fns) checks `.ok` and opens an error modal — BUT until sub-track 4, the caller writes the error to `sys.stderr` and stores the error on instance state for sub-track 4 to consume. For `mcp_config_json` property setter (Python property setters cannot return values), add a sibling `_set_mcp_config_result(value) -> Result[None]` that stores `self._mcp_config_parse_error: ErrorInfo | None`. The setter is a thin wrapper: `result = self._set_mcp_config_result(value); if not result.ok: self._mcp_config_parse_error = result.errors[0]; sys.stderr.write(result.errors[0].ui_message())`.

### Group 6.4 — SDK boundary (drain: stderr + instance state)
- `src/app_controller.py:3173` `_fetch_models.do_fetch`

**Migration:** Wrap `ai_client.list_models()` calls in `_list_models_for_provider_result(p) -> Result[list]`. The per-provider failures are accumulated in `self._model_fetch_errors: dict[str, ErrorInfo]`. The overall function returns `Result[None]` carrying the aggregated errors. Caller writes to stderr + stores in instance state for sub-track 4.

### Group 6.5 — Background workers / threads (drain: stderr + telemetry state)
- `src/app_controller.py:3532` `_handle_compress_discussion.worker` (the inner `try/except`)
- `src/app_controller.py:3570` (next `worker` closure; per `worker` keyword)
- `src/app_controller.py:3642` (next `worker` closure)
- `src/app_controller.py:4175, 4204, 4207` `_bg_task`
- `src/app_controller.py:4300, 4346` `_start_track_logic`
- `src/app_controller.py:4459` `_cb_run_conductor_setup`
- `src/app_controller.py:4557` `_cb_load_track`

**Migration:** The worker function returns `Result[None]`. The `self.submit_io(worker)` caller wraps with a completion handler that checks `result.ok`; on failure, calls `_report_worker_error(op_name, result)` which writes to `sys.stderr` (user-confirmed terminal sink) and appends to `self._worker_errors: list[tuple[str, ErrorInfo]]` for telemetry (Pattern 4 drain — telemetry emission is a real drain per `error_handling.md:421`).

### Group 6.6 — Per-event handlers (drain: stderr + per-request state)
- `src/app_controller.py:3736, 3750` `_handle_request_event` (RAG search + symbol resolution)
- `src/app_controller.py:1707` `_process_pending_gui_tasks` (per-task try)

**Migration:** Each sub-operation gets a `_result` helper. Handler accumulates errors into a per-request list. At end of handler, if errors, calls `_drain_request_errors(errors)` which writes to stderr + stores in `self._last_request_errors: list[ErrorInfo]` for the GUI to display in the next render frame (sub-track 4 surfaces it).

### Group 6.7 — Helpers / utilities (drain: Result propagates upward)
- `src/app_controller.py:1986` `replace_ref`
- `src/app_controller.py:2128` `cb_load_prior_log.token_history`
- `src/app_controller.py:2195` `_load_active_project.primary`
- `src/app_controller.py:2210` `_load_active_project.fallback_loop`
- `src/app_controller.py:2454` `queue_fallback`
- `src/app_controller.py:2969` `_refresh_from_project.active_track`

**Migration:** Function returns `Result[T]`. Caller (already a `Result`-returning function in most cases — `_load_active_project`, `cb_load_prior_log`, `_refresh_from_project` already return `Result[None]`) checks `.ok` and either propagates the error or merges errors into the existing `Result.errors` via `.with_errors([...])`. For `replace_ref` and `queue_fallback`, the caller is the next-level utility — same pattern.

## 15. Goals for Phase 6

1. **Zero `INTERNAL_SILENT_SWALLOW` sites in `src/app_controller.py` after Phase 6.** Audit re-run shows 28 → 0 for the silent swallow category; no category reverts.
2. **Every migrated site carries `ErrorInfo(original=e)`** so the swallowed exception's traceback is preserved (the convention's "AND over OR" + "Error Info as Side-Channel" patterns).
3. **No `logging.debug` in except bodies** (per `error_handling.md:530` — logging is NOT a drain). Every except body either returns `Result(data=..., errors=[ErrorInfo(...)])` OR falls through to a real drain point (os._exit, stderr for terminal sinks, instance state for deferred drains).
4. **All Phase 1-5 invariants preserved:** 0 INTERNAL_BROAD_CATCH, 0 INTERNAL_OPTIONAL_RETURN, 0 SUSPICIOUS INTERNAL_RETHROW.
5. **No new test regressions.** Batched suite must still show the same pass count (890 passed / 17 skipped / 2 xfailed as of Tier 2's last run).

## 16. Functional Requirements

**FR9. Replace every `logging.debug(..., extra={"source": ...})` in an except body with `Result[T]` return.**

Each except body becomes:
```python
except (SpecificException1, SpecificException2) as e:
    return Result(data=default_value, errors=[ErrorInfo(
        kind=ErrorKind.INTERNAL,
        message=str(e),
        source="app_controller.<function_name>",
        original=e,
    )])
```

For void functions, use `Result[None]` with `OK` for success. For non-void functions, the return type changes to `Result[T]` and the caller checks `.ok` and `.errors`.

**FR10. For functions where the caller can't easily receive a Result** (property setters, signal handlers, event sinks), use the pattern:
- Property setter: add a sibling `_set_<thing>_result(value) -> Result[None]` method; the `@<prop>.setter` is a thin wrapper that calls the sibling and stores the error in `self._<thing>_error: ErrorInfo | None` for downstream consumers.
- Signal handler: drain point IS `os._exit(0)` (Pattern 3); the handler writes the ErrorInfo to stderr right before exit.
- Event sink: caller accumulates errors in instance state (`self._<event>_errors: list[ErrorInfo]`); stderr logs each one (user-confirmed acceptable until sub-track 4).

**FR11. Every migration site has a test.**

For each of the 28 sites, add at least 1 test (or extend an existing test) verifying:
- Success path returns `Result(data=success_value)` with `.ok = True`
- Failure path returns `Result(data=zero_value, errors=[ErrorInfo(original=expected_exception)])` with `.ok = False`
- The error's `kind` and `source` match the spec

Tests are organized by group in `tests/test_app_controller_result.py` (extend the existing file; do not create a new one).

**FR12. Audit gate.**

`uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict` must exit 0 (no violations). Per-site count for INTERNAL_SILENT_SWALLOW must be 0.

**FR13. NO deferrals, NO "follow-up" carve-outs.**

Unlike Phase 3's deferral pattern (which left 20 nested sites as "follow-up"), Phase 6 must migrate ALL 28 sites in this phase. If a site is genuinely best-effort and should stay as-is (e.g., the `os._exit(0)` drain point sites), the migration must use the drain-point pattern with stderr write + Result propagation — not silent fall-through.

## 17. Non-Functional Requirements (Phase 6 additions)

- **No new dependencies.** `Result`, `ErrorInfo`, `ErrorKind`, `OK` are all in `src/result_types.py` (already imported).
- **Thread safety preserved.** Background workers (`_bg_task`, `worker` closures) and signal handlers already use thread-local state; the migration uses the same thread-local conventions.
- **No behavior change visible to the user** (until sub-track 4 ships the GUI display). The user sees the same stdout/stderr they saw before; the difference is the data shape (Result carrying the errors to instance state instead of being lost).
- **Per-task atomic commits.** Each site is its own commit (28 sites = 28 commits) plus 8 test commits plus 1 audit-gate commit plus 1 end-of-phase checkpoint commit = ~38 commits.

## 18. Architecture Reference (Phase 6 additions)

- `conductor/code_styleguides/error_handling.md` §"The 5 drain point patterns" — defines Pattern 3 (intentional termination) used by Group 6.1.
- `conductor/code_styleguides/error_handling.md` §"Boundary types vs. drain points" — defines when a function is BOTH a boundary and a drain point (the Group 6.4 SDK boundary sites).
- `conductor/code_styleguides/error_handling.md` §"The Broad-Except Distinction" — explicit table that says `narrow except + log only` is `INTERNAL_SILENT_SWALLOW` (a violation). This is the rule Tier 2's Phase 3 commit violated.
- `conductor/code_styleguides/error_handling.md` §"Re-Raise Patterns" — Pattern 1/2/3 for the 4 INTERNAL_RETHROW sites (already classified in Phase 4).
- `src/result_types.py:91-105` — the `Result[T]` dataclass and its `ok` property; the migration target.

## 19. Out of Scope (Phase 6)

- GUI-side error display (modals, toasts, error panels in `gui_2.py`) — sub-track 4 (`result_migration_gui_2`). The user has confirmed that stderr + instance state is acceptable until sub-track 4.
- The 4 INTERNAL_RETHROW sites — already classified as legitimate Patterns 1/3 in Phase 4; not Phase 6 targets.
- The 1 INTERNAL_OPTIONAL_RETURN site (`cold_start_ts`) — already migrated to `Result[float]` in Phase 4; audit now classifies it INTERNAL_COMPLIANT.
- The 15 BOUNDARY_FASTAPI + 2 BOUNDARY_SDK + 4 INTERNAL_COMPLIANT + 1 INTERNAL_PROGRAMMER_RAISE = 22 stay sites — not Phase 6 targets.
- Sub-track 4 (`gui_2.py`) — separate track.

## 20. Verification Criteria (Phase 6)

- `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict` exits 0.
- `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --json | python -c "..."` shows 0 sites with category `INTERNAL_SILENT_SWALLOW`.
- `uv run python -m pytest tests/test_app_controller_result.py -v` passes all tests.
- `uv run python scripts/run_tests_batched.py` shows the same pass count as the pre-Phase-6 baseline (890 passed / 17 skipped / 2 xfailed). No new failures.
- Every migrated except body contains `Result(data=..., errors=[ErrorInfo(original=e)])` (or equivalent Pattern 3 drain for signal handlers) — verified by `grep -n 'logging.getLogger.*\.debug' src/app_controller.py | grep -v '#'` showing no debug-log-only except bodies.

## 21. Risk Register (Phase 6 additions)

- **R6 (Phase 6):** Tier 2 may repeat the Phase 3 deferral pattern (using `logging.debug` as a "migration" that the audit still flags as silent swallow). Mitigation: the audit gate in FR12 (`--strict` exits 1 on any violation) is the hard verification. If FR12 fails, the track is not complete regardless of how many sites are touched.
- **R7 (Phase 6):** Some sites may need their callers updated to receive `Result[T]` instead of `T`. For example, `_update_inject_preview` currently returns `None` and sets `self._inject_preview`; changing to `Result[str]` requires the caller to check `.ok` and propagate. Mitigation: each task identifies its caller chain via `py_find_usages` and updates all callers in the same commit.
- **R8 (Phase 6):** The 20 nested sites introduced by Phase 2 may have been overwritten by Phase 3's `logging.debug` add. The migration must remove the `logging.debug` AND replace with `Result` return (not add a Result on top of the logging).
- **R9 (Phase 6):** Scope (28 sites) is large but bounded. Mitigation: 8 groups with clear drain patterns; each group is a sub-batch (3-5 commits per group). If a group takes too many commits, the group can be split further.

## 22. Phase 7 - Strict Enforcement Cleanup (added 2026-06-19)

### 22.1 Background

Phase 6 reduced INTERNAL_SILENT_SWALLOW from 30 to 0 per `audit_exception_handling.py`. However, 4 sites are classified as compliant by the audit via heuristic over-application, not by satisfying the user's principle (`error_handling.md:530`: "logging is NOT a drain"):

| Line | Function | Audit class | Strict status |
|---|---|---|---|
| L242 | `_api_generate` (RAG) | BOUNDARY_FASTAPI | violation - sys.stderr.write only |
| L256 | `_api_generate` (symbols) | BOUNDARY_FASTAPI | violation - sys.stderr.write only |
| L5064 | `_push_mma_state_update` | INTERNAL_COMPLIANT | violation - logging + print, no Result |
| L5093 | `_load_active_tickets.beads` inner | INTERNAL_COMPLIANT | violation - logging + print, no Result |

The audit heuristic at `scripts/audit_exception_handling.py:319-321` (`_is_api_handler()`) plus the classification at line 393-397 over-applies BOUNDARY_FASTAPI to ALL try/except inside `_api_*` handlers regardless of whether the except body raises HTTPException. Per `error_handling.md:534`, BOUNDARY_FASTAPI only applies to `raise HTTPException(...)` sites. This is the same laundering pattern that sub-track 2 Phase 10 to 11 redo addressed.

### 22.2 Goals

1. Migrate the 4 strict-violation sites to proper Result[T] propagation using the Phase 6 helpers already in the file.
2. Tighten the audit heuristic so future sites are not over-classified.
3. Add regression tests that lock in the correct behavior.

### 22.3 Functional Requirements

- **FR1** `src/app_controller.py:232-244` (RAG augmentation in `_api_generate`) calls the existing `_rag_search_result(user_msg)` helper (Phase 6 Group 6.5/6.6) returning `Result[str]`. On error, append to `self._last_request_errors`. The outer `_api_generate` raises `HTTPException` with accumulated errors on subsequent API failure.
- **FR2** `src/app_controller.py:246-258` (symbol resolution in `_api_generate`) calls the existing `_symbol_resolution_result(user_msg, file_items)` helper. Same accumulation pattern.
- **FR3** `src/app_controller.py:_push_mma_state_update` is split: new `_push_mma_state_update_result()` returning `Result[None]`; legacy wrapper preserves fire-and-forget but routes errors through `self._report_worker_error`.
- **FR4** `src/app_controller.py:_load_active_tickets` inner-beads try/except is extracted to `_load_beads_from_path_result()` returning `Result[List[Ticket]]`; outer merges errors via `.with_errors()` and routes through `self._report_worker_error`.

- **FR5** `scripts/audit_exception_handling.py:319-321` (`_is_api_handler`) and line 393-397 (classification): BOUNDARY_FASTAPI applies ONLY when the except body actually contains `ast.Raise(exc=HTTPException(...))` OR returns a Result propagated to the caller. Otherwise re-classify as INTERNAL_SILENT_SWALLOW if the body has logging, or INTERNAL_COMPLIANT if it is `try/finally` cleanup.
- **FR6** 4 unit tests in `tests/test_app_controller_result.py` verify each migrated site returns Result[T] with proper error propagation.
- **FR7** 3 unit tests in a new `tests/test_audit_heuristics.py` verify (a) the 15 existing BOUNDARY_FASTAPI sites in `src/api_hooks.py` and `src/app_controller.py` remain classified correctly, (b) the 4 strict-violation sites ARE flagged when the heuristic is reverted to old behavior (regression-guard), (c) `_is_api_handler` requires HTTPException raise in except body.

### 22.4 Non-Functional Requirements

- **NFR1** `audit_exception_handling.py --src src/app_controller.py --strict` exits 0.
- **NFR2** Without `--strict`, 0 INTERNAL_SILENT_SWALLOW AND 0 strict-violation sites (L242, L256, L5064, L5093) reported.
- **NFR3** Full 11-tier batched suite passes; no new regressions vs post-Phase-6 baseline.
- **NFR4** 1-space indentation per `product-guidelines.md`.
- **NFR5** Per-file atomic commits; no batching.

### 22.5 Per-Site Migration Patterns

#### 22.5.1 L242 - RAG search in `_api_generate`

**WHERE:** `src/app_controller.py:232-244`

**HOW:** Replace the inline `try/except Exception: sys.stderr.write(...)` with a call to `_rag_search_result(user_msg)` (Phase 6 helper) returning `Result[str]`. On error, append to `self._last_request_errors`. The user sees degraded context (no RAG) but the failure is visible.

**SAFETY:** `_last_request_errors` is the field added in Phase 6 Group 6.6. If Phase 6 did not add a lock, add `self._last_request_errors_lock = threading.Lock()` and acquire it on every append and on reset.

#### 22.5.2 L256 - Symbol resolution in `_api_generate`

**WHERE:** `src/app_controller.py:246-258`

**HOW:** Same pattern as 22.5.1 using `_symbol_resolution_result(user_msg, file_items) -> Result[str]` (Phase 6 helper).

**SAFETY:** Same as 22.5.1.

#### 22.5.3 L5064 - `_push_mma_state_update`

**WHERE:** `src/app_controller.py:_push_mma_state_update` (function body preceding L5064).

**HOW:** Extract a `_push_mma_state_update_result() -> Result[None]` helper; legacy wrapper calls `self._report_worker_error` on failure.

**SAFETY:** Called from MMA worker thread per `docs/guide_multi_agent_conductor.md`. Legacy wrapper preserves fire-and-forget semantics for existing callers; new code should use the `_result` variant.

#### 22.5.4 L5093 - `_load_active_tickets.beads` inner

**WHERE:** `src/app_controller.py:5093` (inside the outer try of `_load_active_tickets`).

**HOW:** Extract `_load_beads_from_path_result(beads_path) -> Result[List[Ticket]]`; outer `_load_active_tickets` merges errors via `.with_errors()` and routes through `self._report_worker_error`.

**SAFETY:** Main-thread only per existing callers; no thread-safety concerns.

#### 22.5.5 FR5 - Audit heuristic tightening

**WHERE:** `scripts/audit_exception_handling.py:319-321` (`_is_api_handler`) AND the classification call site at line 393-397.

**HOW:** Add an AST check on the `ast.ExceptHandler.body`: require either an `ast.Raise` node where `exc.func.id == "HTTPException"` OR a `return` statement returning a `Result` constructor call. If neither, re-classify as INTERNAL_SILENT_SWALLOW (if body has logging) or INTERNAL_COMPLIANT (if body is `try/finally` cleanup only).

**SAFETY:** The classification tightening affects all 65 src/ files. The 3 unit tests in FR7 lock the regression boundary; the 15 existing BOUNDARY_FASTAPI sites must remain classified.

### 22.6 Architecture Reference

- `conductor/code_styleguides/error_handling.md:462-476` - "What is NOT a drain point" (the rule being enforced).
- `conductor/code_styleguides/error_handling.md:496-516` - Heuristic D (the legitimate drain-point heuristic Phase 7 must not regress).
- `conductor/code_styleguides/error_handling.md:530` - the "logging is NOT a drain" rule.
- `docs/guide_app_controller.md` "Modular Controller Pattern" - the helper-extraction pattern.
- `conductor/tracks/result_migration_app_controller_20260618/spec.md` Phase 6 addendum sections 12-21 - the addendum pattern this phase follows.

### 22.7 Verification Criteria

- **VC1** `audit_exception_handling.py --src src/app_controller.py --strict` exits 0.
- **VC2** 4 unit tests in `tests/test_app_controller_result.py` pass (one per migrated site).
- **VC3** 3 unit tests in `tests/test_audit_heuristics.py` pass (heuristic regression-guard).
- **VC4** Full 11-tier batched suite passes; no new regressions.
- **VC5** Git history shows 7+ atomic commits (4 site migrations + 1 heuristic fix + 1 tests + 1 state updates).
- **VC6** Phase 7 checkpoint commit with git note documenting audit before/after metrics.

### 22.8 Out of Scope

- Other `_api_*` handlers in `src/api_hooks.py` (verified compliant; tests in FR7 guard against regression).
- 38 INTERNAL_BROAD_CATCH sites in `src/gui_2.py` (sub-track 4 territory).
- 77 violations in the 3 refactored baseline files (sub-track 5 territory per completion report section 7.2).

### 22.9 Risks

- **R7-1** Heuristic tightening may regress other files' `_api_*` boundary sites. Mitigation: FR7's 3 unit tests lock the 15 existing BOUNDARY_FASTAPI sites; manual verification of `src/api_hooks.py` during implementation.
- **R7-2** Legacy wrapper for `_push_mma_state_update` preserves fire-and-forget. Mitigation: docstring deprecation note; follow-up track migrates callers.
- **R7-3** `_last_request_errors` may grow unbounded. Mitigation: verify Phase 6 reset the field per-request; add reset if missing.

