|
| 1 | +--- |
| 2 | +title: Multi-Agent Adapter — mcp_setup() Not Called in Session Start Flow |
| 3 | +category: integration-issues |
| 4 | +tags: [adapters, mcp, multi-agent, orchestration, integration-testing] |
| 5 | +module: cli/_study.py |
| 6 | +symptom: MCP config files absent from session directory; Gemini and OpenCode agents start without persona |
| 7 | +root_cause: _handle_start() called adapter.setup() but omitted adapter.mcp_setup(session_dir) |
| 8 | +--- |
| 9 | + |
| 10 | +# Multi-Agent Adapter: mcp_setup() Gap in Session Start Flow |
| 11 | + |
| 12 | +## Problem Summary |
| 13 | + |
| 14 | +The `AgentAdapter` dataclass carries four callables: `setup`, `launch_cmd`, `teardown`, and `mcp_setup`. |
| 15 | +The orchestrator (`_handle_start()` in `cli/_study.py`) wired `setup()` but never called `mcp_setup()`. |
| 16 | +Result: MCP config files were never written to the session directory, so agents started without persona context. |
| 17 | + |
| 18 | +## Symptom |
| 19 | + |
| 20 | +``` |
| 21 | +studyctl study --agent gemini # starts, but Gemini has no system prompt |
| 22 | +studyctl study --agent opencode # starts, but OpenCode ignores agent persona |
| 23 | +``` |
| 24 | + |
| 25 | +Expected files that were absent: |
| 26 | + |
| 27 | +| Agent | Expected file | |
| 28 | +|-----------|----------------------------------------| |
| 29 | +| Gemini | `<session_dir>/.gemini/settings.json` | |
| 30 | +| OpenCode | `<session_dir>/.opencode/opencode.json`| |
| 31 | + |
| 32 | +Unit tests: 45/45 passed. |
| 33 | +Integration tests: 5/7 passed — the two failures were persona-verification checks. |
| 34 | + |
| 35 | +## Agent Persona Injection Mechanisms |
| 36 | + |
| 37 | +Each tool uses a fundamentally different mechanism — no universal standard exists. |
| 38 | + |
| 39 | +| Agent | Mechanism | Written by | |
| 40 | +|-----------|----------------------------------------------------|--------------------| |
| 41 | +| Claude | `--system-prompt` CLI flag on launch | `launch_cmd` | |
| 42 | +| Gemini | `GEMINI.md` file in the working directory | `mcp_setup()` | |
| 43 | +| Kiro | Atomic JSON written to `~/.kiro/agents/<name>.json`| `mcp_setup()` | |
| 44 | +| OpenCode | `<session_dir>/.opencode/agents/<name>.md` with YAML frontmatter | `mcp_setup()` | |
| 45 | + |
| 46 | +Claude's persona is injected at launch time via a flag, so it worked even without `mcp_setup()`. |
| 47 | +Gemini, Kiro, and OpenCode all depend on files written by `mcp_setup()` — which was never called. |
| 48 | + |
| 49 | +## Root Cause |
| 50 | + |
| 51 | +```python |
| 52 | +# cli/_study.py — _handle_start() BEFORE fix |
| 53 | +def _handle_start(session_dir: Path, adapter: AgentAdapter, ...) -> None: |
| 54 | + adapter.setup(session_dir) # ✓ called |
| 55 | + # adapter.mcp_setup(session_dir) # ✗ forgotten — not called |
| 56 | + cmd = adapter.launch_cmd(session_dir) |
| 57 | + _launch_tmux(cmd, ...) |
| 58 | +``` |
| 59 | + |
| 60 | +The `AgentAdapter` dataclass is frozen; all callables are optional (`Callable | None`). |
| 61 | +`mcp_setup` was defined on the adapter objects but simply never invoked in the orchestrator. |
| 62 | + |
| 63 | +## Fix |
| 64 | + |
| 65 | +```python |
| 66 | +# cli/_study.py — _handle_start() AFTER fix |
| 67 | +def _handle_start(session_dir: Path, adapter: AgentAdapter, ...) -> None: |
| 68 | + adapter.setup(session_dir) |
| 69 | + if adapter.mcp_setup: # guard for adapters that don't need it |
| 70 | + adapter.mcp_setup(session_dir) # ← added |
| 71 | + cmd = adapter.launch_cmd(session_dir) |
| 72 | + _launch_tmux(cmd, ...) |
| 73 | +``` |
| 74 | + |
| 75 | +One line. The guard handles adapters (e.g. Claude) where `mcp_setup` is `None`. |
| 76 | + |
| 77 | +## Why Unit Tests Passed |
| 78 | + |
| 79 | +Unit tests exercised each adapter function in isolation: |
| 80 | + |
| 81 | +```python |
| 82 | +def test_gemini_mcp_setup_writes_settings(tmp_path): |
| 83 | + adapter = gemini_adapter() |
| 84 | + adapter.mcp_setup(tmp_path) # called directly |
| 85 | + assert (tmp_path / ".gemini/settings.json").exists() |
| 86 | +``` |
| 87 | + |
| 88 | +This confirmed `mcp_setup()` worked correctly. It did not test whether `_handle_start()` *called* it. |
| 89 | +The orchestration gap is invisible to unit tests — only integration tests that run a full session lifecycle can catch it. |
| 90 | + |
| 91 | +## Prevention Strategies |
| 92 | + |
| 93 | +1. **Checklist on adapter contracts** — when an adapter/strategy carries multiple callables, the orchestrator review checklist must include "all callables invoked". |
| 94 | + |
| 95 | +2. **Integration smoke test per adapter** — a test that calls `_handle_start()` end-to-end (not the adapter directly) and asserts all expected side-effects (files written, tmux pane alive). |
| 96 | + |
| 97 | +3. **Dataclass field ordering as a hint** — group "must-call" fields at the top of the dataclass so code review catches omissions by visual inspection. |
| 98 | + |
| 99 | +4. **Post-setup assertion in `_handle_start()`** — after setup, assert that required paths exist before launching: |
| 100 | + |
| 101 | + ```python |
| 102 | + missing = adapter.required_paths(session_dir) - {p for p in ... if p.exists()} |
| 103 | + if missing: |
| 104 | + raise SetupError(f"Adapter setup incomplete: {missing}") |
| 105 | + ``` |
| 106 | + |
| 107 | +## Related Files |
| 108 | + |
| 109 | +- `src/studyctl/adapters/` — one module per agent adapter |
| 110 | +- `src/studyctl/cli/_study.py` — `_handle_start()` orchestrator |
| 111 | +- `tests/unit/test_adapters.py` — per-function unit tests |
| 112 | +- `tests/integration/test_session_lifecycle.py` — end-to-end tests that caught this |
0 commit comments