diff --git a/.agents/specs/2026-09-24-mcp-initialize-timeout-design.md b/.agents/specs/2026-09-24-mcp-initialize-timeout-design.md new file mode 100644 index 0000000..45bbc86 --- /dev/null +++ b/.agents/specs/2026-09-24-mcp-initialize-timeout-design.md @@ -0,0 +1,114 @@ +# Dedicated Timeout for the MCP Initialize Handshake + +**Issue:** [#101](https://github.com/Svagtlys/Octave/issues/101) — bug(mcp): dedicated timeout for initialize handshake +**Branch:** `fix/mcp-initialize-timeout` · **Draft PR:** [#102](https://github.com/Svagtlys/Octave/pull/102) +**Date:** 2026-09-24 + +## Problem + +`McpClient._await_initialize()` ([client.py:250](../../backend/src/octave/mcp/client.py)) runs the +MCP initialize handshake under `anyio.fail_after(self._settings.request_timeout_seconds)` — the +same knob that governs every steady-state request in `_run()`. The two budgets are semantically +different but coupled by accident: + +- **Start-up budget** — how long the client waits for a server process to boot and complete the + handshake (spawn + initialize). Slow on stdio servers (interpreter cold-start). +- **Steady-state budget** — how long a single JSON-RPC call may take. Callers who tighten this + (e.g. `request_timeout_seconds=0.5` to fail fast on wedged tool calls) unintentionally starve + the handshake: a server that takes >0.5 s to boot now fails `connect()` with + `McpTimeoutError`, even though nothing is wedged. + +The docs table already documents the smell: `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` is described as +"Per-request timeout for client calls *(and the initialize handshake)*". + +## Decision + +| # | Question | Decision | +|---|---|---| +| 1 | Where does the new budget live? | **New `McpSettings.initialize_timeout_seconds: float = 30.0`** (env `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS`). Same two-layer config pattern as every other MCP knob; env var comes for free via `env_prefix="OCTAVE_MCP_"`. | +| 2 | Default value | **`30.0`** — identical to the current effective handshake budget (`request_timeout_seconds`'s default), so defaults are zero-impact: no user on default settings sees any behavior change. | +| 3 | Fallback semantics | **Independent value, not `None`-falls-back-to-request-timeout.** An explicit knob with a concrete default is simpler to reason about, matches every sibling field, and the issue specifies `30.0`. Users who *want* coupled budgets set both vars. | +| 4 | Validation | **None.** `McpSettings` has no field validators today; adding a `gt=0` guard for one field is inconsistent style. Negative/zero values behave like `anyio.fail_after` does elsewhere in the codebase (immediate/simultaneous timeout) — same posture as `request_timeout_seconds`. | +| 5 | Test harness plumbing | **None.** New client tests are standalone stub-peer tests in `test_client.py` (the established pattern — see `test_death_during_initialize_raises_connection_error`), constructing `McpSettings` directly. `conftest.py` and `_stub_harness` stay untouched. | +| 6 | Existing tight-budget tests | **Untouched.** `test_client.py:147` (`request_timeout=0.5`), `test_stdio_integration.py:143` (`0.5`), and `FAST` in `test_manager.py` (`0.05`, `FakeClient` overrides `connect()` — no real handshake) are correct *by construction* after the fix: their handshakes get the 30 s default, their request budgets stay tight. | + +## Design + +### 1. `octave/mcp/config.py` + +Add one field to `McpSettings`, placed directly below `request_timeout_seconds` (start-up knob +next to the steady-state knob it replaces for the handshake): + +```python + request_timeout_seconds: float = 30.0 + + initialize_timeout_seconds: float = 30.0 + """Budget for the initialize handshake during connect()/restart(). + Separate from request_timeout_seconds: a slow-to-boot server must not be + penalized by callers who tighten the steady-state per-request budget.""" +``` + +No other config changes. `McpServerManager` needs no change — it passes the same `McpSettings` +instance to every client via `_default_client_factory`, so the new knob propagates automatically. + +### 2. `octave/mcp/client.py` + +Two call sites, both already existing: + +- `_await_initialize()` — `anyio.fail_after(self._settings.initialize_timeout_seconds)`. + The death-cancel scope wrapper stays exactly as-is: a subprocess that dies mid-handshake + still surfaces as `McpConnectionError` fast, never waits for the new budget. +- `connect()`'s `except TimeoutError` branch — the `McpTimeoutError` message cites + `initialize_timeout_seconds` instead of `request_timeout_seconds`, so the reported number + matches the budget that actually fired. + +`_run()` keeps using `request_timeout_seconds` — the steady-state path is untouched. + +### 3. Error behavior (unchanged taxonomy) + +| Scenario | Before | After | +|---|---|---| +| Handshake exceeds budget | `McpTimeoutError` citing `request_timeout_seconds` | `McpTimeoutError` citing `initialize_timeout_seconds` | +| Server dies mid-handshake | `McpConnectionError` (death scope cancels first) | unchanged | +| Steady-state request exceeds budget | `McpTimeoutError` citing `request_timeout_seconds` | unchanged | +| Defaults everywhere | 30 s handshake budget | 30 s handshake budget (zero-impact) | + +### 4. Tests (new only) + +`tests/mcp/test_config.py`: + +- `test_settings_default_initialize_timeout` — default is `30.0` (delenv-guarded like the + existing default test). +- `test_settings_env_override_initialize_timeout` — `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS=8` + → `8.0`. + +`tests/mcp/test_client.py` (standalone stub-peer tests, mirroring the existing +`test_death_during_initialize_raises_connection_error` pattern — no conftest changes): + +- `test_initialize_timeout_uses_dedicated_budget` — peer that reads its inbound stream but + never answers `initialize`; settings `initialize_timeout_seconds=0.05`, + `request_timeout_seconds=1.0`. Assert `McpTimeoutError` raised, and its message cites + `0.05` (the new knob) and not `1.0` — proving the handshake no longer reads the request + budget. +- `test_slow_initialize_survives_tight_request_timeout` — peer that sleeps 0.2 s before + answering `initialize`; settings `request_timeout_seconds=0.05` (tighter than boot time), + `initialize_timeout_seconds` left at default. Assert `connect()` succeeds and + `is_connected` is True — the regression this issue is about: tight request budgets must not + strangle the handshake. + +Existing tests: **no edits** (decision 6). + +### 5. Docs + +`docs/DEVELOPMENT.md` env-var table: + +- New row: `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` | `30.0` | Budget for the initialize handshake during connect/restart +- Amend the `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` row: drop the "(and the initialize handshake)" + parenthetical → "Per-request timeout for client calls". + +## Out of scope + +- Per-server handshake budgets (would ride on `ServerConfig`, roadmap #7 config persistence). +- Validation of timeout values (`gt=0` etc.) — decision 4. +- Any change to `_run()`, the manager supervisor, the probe-on-timeout path, or the conftest + harnesses. diff --git a/.agents/specs/2026-09-24-mcp-initialize-timeout.md b/.agents/specs/2026-09-24-mcp-initialize-timeout.md new file mode 100644 index 0000000..a69a3d4 --- /dev/null +++ b/.agents/specs/2026-09-24-mcp-initialize-timeout.md @@ -0,0 +1,332 @@ +# Dedicated Initialize Handshake Timeout — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give the MCP initialize handshake its own timeout budget (`McpSettings.initialize_timeout_seconds`, default `30.0`, env `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS`) so tightening the steady-state `request_timeout_seconds` no longer starves slow-to-boot servers. + +**Architecture:** One new field on the existing pydantic-settings class `McpSettings`; two call-site swaps in `McpClient` (`_await_initialize()`'s `fail_after` and `connect()`'s timeout error message). `_run()` and the steady-state budget are untouched. Defaults keep the effective handshake budget at 30 s — zero-impact for default settings. + +**Tech Stack:** Python 3.12, `anyio` timeouts, `pydantic-settings`, `mcp` SDK 1.x (quarantined behind the façade), pytest (asyncio auto-mode), `uv`. + +**Spec:** [`.agents/specs/2026-09-24-mcp-initialize-timeout-design.md`](./2026-09-24-mcp-initialize-timeout-design.md) · **Branch:** `fix/mcp-initialize-timeout` · **Draft PR:** [#102](https://github.com/Svagtlys/Octave/pull/102) · **Issue:** [#101](https://github.com/Svagtlys/Octave/issues/101) + +**Conventions:** run all commands from `backend/` with `uv run`. Ruff line-length 88, mypy strict. Do **not** touch `tests/mcp/conftest.py`, `_run()`, `McpServerManager`, or any existing test body — the plan adds new tests only. + +--- + +### Task 1: `McpSettings.initialize_timeout_seconds` field + +**Files:** +- Modify: `backend/src/octave/mcp/config.py` (in `McpSettings`, after `request_timeout_seconds`, ~line 64) +- Test: `backend/tests/mcp/test_config.py` (append after `test_settings_env_override`, ~line 47) + +- [ ] **Step 1: Write the failing tests** + +Append to `backend/tests/mcp/test_config.py` (after `test_settings_env_override`, before `class TestSupervisionSettings`): + +```python +def test_settings_default_initialize_timeout(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS", raising=False) + assert McpSettings().initialize_timeout_seconds == 30.0 + + +def test_settings_env_override_initialize_timeout(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS", "8") + assert McpSettings().initialize_timeout_seconds == 8.0 +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && uv run pytest tests/mcp/test_config.py -v` +Expected: the two new tests FAIL with `AttributeError: 'McpSettings' object has no attribute 'initialize_timeout_seconds'`; all pre-existing tests in the file PASS. + +- [ ] **Step 3: Implement the field** + +In `backend/src/octave/mcp/config.py`, replace: + +```python + request_timeout_seconds: float = 30.0 +``` + +with: + +```python + request_timeout_seconds: float = 30.0 + + initialize_timeout_seconds: float = 30.0 + """Budget for the initialize handshake during connect()/restart(). + Separate from request_timeout_seconds: a slow-to-boot server must not be + penalized by callers who tighten the steady-state per-request budget.""" +``` + +The env var `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` works automatically via +`env_prefix="OCTAVE_MCP_"` — no other wiring needed. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cd backend && uv run pytest tests/mcp/test_config.py -v` +Expected: all PASS. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/octave/mcp/config.py backend/tests/mcp/test_config.py +git commit -m "feat(mcp): add initialize_timeout_seconds setting" +``` + +--- + +### Task 2: Run the handshake under the dedicated budget + +**Files:** +- Modify: `backend/src/octave/mcp/client.py` (`_await_initialize()` ~line 261; `connect()` timeout message ~lines 192–195) +- Test: `backend/tests/mcp/test_client.py` (append two tests at end of file) + +- [ ] **Step 1: Write the failing tests** + +Append to `backend/tests/mcp/test_client.py` (end of file; every import these tests +need — `anyio`, `asynccontextmanager`, `AsyncIterator`, `create_client_server_memory_streams`, +`SessionMessage`, `JSONRPCMessage`/`JSONRPCRequest`/`JSONRPCResponse`, `McpClient`, +`McpSettings`, `ServerConfig`, `StdioConfig`, `McpTimeoutError`, `TransportStreams`, +`_INIT_RESULT` — already exists at module level): + +```python +async def test_initialize_timeout_uses_dedicated_budget() -> None: + """The handshake budget is initialize_timeout_seconds, not the request budget. + + Silent-but-alive peer (never answers initialize); initialize_timeout=0.05, + request_timeout=1.0. The raised McpTimeoutError must cite 0.05 — proving + the handshake no longer reads the steady-state knob. + """ + + @asynccontextmanager + async def _silent_factory( + _config: ServerConfig, + ) -> AsyncIterator[TransportStreams]: + # Memory streams buffer infinitely: the initialize request simply + # sits unanswered; no peer task needed. + async with create_client_server_memory_streams() as ( + (cread, cwrite), + (_sread, _swrite), + ): + yield TransportStreams(read=cread, write=cwrite) + + client = McpClient( + transport_factory=_silent_factory, + settings=McpSettings( + initialize_timeout_seconds=0.05, request_timeout_seconds=1.0 + ), + ) + with pytest.raises(McpTimeoutError) as excinfo: + await client.connect(StdioConfig(command="silent-peer")) + assert "0.05" in str(excinfo.value) # the dedicated budget fired + assert "1.0" not in str(excinfo.value) # not the request budget + assert client.is_connected is False + + +async def test_slow_initialize_survives_tight_request_timeout() -> None: + """Regression (#101): a tight request_timeout_seconds must not strangle the + handshake. Peer boots slowly (answers initialize after 0.2 s) while the + steady-state budget is 0.05 s; connect() must still succeed on the default + 30 s initialize budget.""" + + @asynccontextmanager + async def _slow_peer_factory( + _config: ServerConfig, + ) -> AsyncIterator[TransportStreams]: + async with create_client_server_memory_streams() as ( + (cread, cwrite), + (sread, swrite), + ): + + async def _answer_after_a_moment() -> None: + message = await sread.receive() + root = message.message.root + if isinstance(root, JSONRPCRequest) and root.method == "initialize": + await anyio.sleep(0.2) # slow boot: outlasts request_timeout + await swrite.send( + SessionMessage( + JSONRPCMessage( + root=JSONRPCResponse( + jsonrpc="2.0", id=root.id, result=_INIT_RESULT + ) + ) + ) + ) + async for _message in sread: + pass # consume notifications/initialized and everything else + + async with anyio.create_task_group() as tg: + tg.start_soon(_answer_after_a_moment) + try: + yield TransportStreams(read=cread, write=cwrite) + finally: + tg.cancel_scope.cancel() + + client = McpClient( + transport_factory=_slow_peer_factory, + settings=McpSettings(request_timeout_seconds=0.05), + ) + try: + await client.connect(StdioConfig(command="slow-boot-peer")) + assert client.is_connected is True + finally: + await client.aclose() +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cd backend && uv run pytest tests/mcp/test_client.py -k "initialize_timeout_uses_dedicated_budget or slow_initialize_survives" -v` +Expected: BOTH FAIL — +- `test_initialize_timeout_uses_dedicated_budget`: `McpTimeoutError` message cites `1.0` (old code used the request budget), so `assert "0.05" in str(...)` fails. +- `test_slow_initialize_survives_tight_request_timeout`: `connect()` raises `McpTimeoutError` after 0.05 s (old code applied the request budget to the handshake). + +- [ ] **Step 3: Implement the swap** + +In `backend/src/octave/mcp/client.py`: + +3a. In `_await_initialize()`, replace: + +```python + scope = anyio.CancelScope() + self._request_scopes.add(scope) + try: + with scope: + with anyio.fail_after(self._settings.request_timeout_seconds): + init = await session.initialize() +``` + +with: + +```python + scope = anyio.CancelScope() + self._request_scopes.add(scope) + try: + with scope: + with anyio.fail_after(self._settings.initialize_timeout_seconds): + init = await session.initialize() +``` + +Also in that method's docstring, replace the phrase +``McpTimeoutError`` after the full request timeout. +with +``McpTimeoutError`` after the full initialize timeout. + +3b. In `connect()`, replace: + +```python + raise McpTimeoutError( + f"initialize handshake timed out after " + f"{self._settings.request_timeout_seconds}s" + ) from exc +``` + +with: + +```python + raise McpTimeoutError( + f"initialize handshake timed out after " + f"{self._settings.initialize_timeout_seconds}s" + ) from exc +``` + +Do **not** touch `_run()` — the steady-state path keeps `request_timeout_seconds`. +Do **not** touch the death-cancel scope logic — a subprocess dying mid-handshake +must keep surfacing `McpConnectionError` fast (existing +`test_death_during_initialize_raises_connection_error` guards this). + +- [ ] **Step 4: Run the new tests, then the whole client suite** + +Run: `cd backend && uv run pytest tests/mcp/test_client.py -v` +Expected: all PASS — the two new tests, plus `test_death_during_initialize_raises_connection_error` (death still beats timeout) and every pre-existing lifecycle test. + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/octave/mcp/client.py backend/tests/mcp/test_client.py +git commit -m "fix(mcp): run initialize handshake under its own timeout budget" +``` + +--- + +### Task 3: Env-var documentation + +**Files:** +- Modify: `docs/DEVELOPMENT.md` (MCP env-var table, ~lines 206–213) + +- [ ] **Step 1: Edit the table** + +In `docs/DEVELOPMENT.md`, replace the row: + +```markdown +| `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` | `30.0` | Per-request timeout for client calls (and the initialize handshake) | +``` + +with: + +```markdown +| `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` | `30.0` | Per-request timeout for client calls | +| `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` | `30.0` | Budget for the initialize handshake during connect/restart | +``` + +- [ ] **Step 2: Verify** + +Run: `grep -n "OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS" docs/DEVELOPMENT.md` +Expected: exactly one match — the new table row. Also confirm the request-timeout row +no longer contains "(and the initialize handshake)". + +- [ ] **Step 3: Commit** + +```bash +git add docs/DEVELOPMENT.md +git commit -m "docs(mcp): document OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS" +``` + +--- + +### Task 4: Full gate + design/plan docs commit + +**Files:** +- Commit: `.agents/specs/2026-09-24-mcp-initialize-timeout-design.md`, `.agents/specs/2026-09-24-mcp-initialize-timeout.md` + +- [ ] **Step 1: Full test suite** + +Run: `cd backend && uv run pytest` +Expected: all tests PASS. If the sandbox forbids spawning subprocesses, the +`tests/mcp/test_stdio_integration.py` file can be skipped with +`OCTAVE_MCP_SKIP_SUBPROCESS_TESTS=1 uv run pytest` — note the skip in the PR. + +- [ ] **Step 2: Lint and type gate** + +Run: `cd backend && uv run ruff check . && uv run mypy src` +Expected: `All checks passed!` and no mypy errors. + +- [ ] **Step 3: Commit the planning docs** + +```bash +git add .agents/specs/2026-09-24-mcp-initialize-timeout-design.md .agents/specs/2026-09-24-mcp-initialize-timeout.md +git commit -m "docs(specs): design and plan for dedicated initialize handshake timeout" +``` + +- [ ] **Step 4: Push and mark the PR ready for review** + +```bash +git push +``` + +Then update PR #102's description to reference issue #101 (`Closes #101`), summarize the +change (new `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` knob, zero-impact default), and list +the test evidence from Steps 1–2. + +--- + +## Self-review notes + +- **Spec coverage:** config field + env var (Task 1), client budget + error message (Task 2), + 4 new tests — config default, config env override, handshake-cites-new-knob, + slow-initialize regression (Tasks 1–2), docs table (Task 3). Out-of-scope items + (`_run()`, conftest, manager, existing tests) have no tasks by design. +- **Placeholders:** none — every code step is complete, every command has expected output. +- **Type consistency:** field name `initialize_timeout_seconds` identical in config, client, + tests, docs; env var `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` identical in tests and docs + (derived via `env_prefix="OCTAVE_MCP_"`). diff --git a/backend/src/octave/mcp/client.py b/backend/src/octave/mcp/client.py index 1950797..117fc27 100644 --- a/backend/src/octave/mcp/client.py +++ b/backend/src/octave/mcp/client.py @@ -191,7 +191,7 @@ async def connect(self, config: ServerConfig) -> None: await stack.aclose() raise McpTimeoutError( f"initialize handshake timed out after " - f"{self._settings.request_timeout_seconds}s" + f"{self._settings.initialize_timeout_seconds}s" ) from exc except (FileNotFoundError, PermissionError) as exc: await stack.aclose() @@ -252,13 +252,13 @@ async def _await_initialize(self, session: ClientSession) -> InitializeResult: If the subprocess dies mid-handshake the monitor cancels the scope; the death surfaces as ``McpConnectionError`` instead of a misleading - ``McpTimeoutError`` after the full request timeout. + ``McpTimeoutError`` after the full initialize timeout. """ scope = anyio.CancelScope() self._request_scopes.add(scope) try: with scope: - with anyio.fail_after(self._settings.request_timeout_seconds): + with anyio.fail_after(self._settings.initialize_timeout_seconds): init = await session.initialize() finally: self._request_scopes.discard(scope) diff --git a/backend/src/octave/mcp/config.py b/backend/src/octave/mcp/config.py index 6778a62..8f115a1 100644 --- a/backend/src/octave/mcp/config.py +++ b/backend/src/octave/mcp/config.py @@ -63,6 +63,11 @@ class McpSettings(BaseSettings): request_timeout_seconds: float = 30.0 + initialize_timeout_seconds: float = 30.0 + """Budget for the initialize handshake during connect()/restart(). + Separate from request_timeout_seconds: a slow-to-boot server must not be + penalized by callers who tighten the steady-state per-request budget.""" + restart_base_delay_seconds: float = 1.0 """First auto-restart delay; exponential backoff multiplies by 2 per attempt.""" diff --git a/backend/tests/mcp/test_client.py b/backend/tests/mcp/test_client.py index 57551aa..15bc976 100644 --- a/backend/tests/mcp/test_client.py +++ b/backend/tests/mcp/test_client.py @@ -545,3 +545,86 @@ def boom(_reason: str) -> None: async with harness(request_timeout=0.1, on_lost=boom) as (client, _peer): with pytest.raises(McpTimeoutError): # the Octave error, not RuntimeError await client.call_tool("slow") + + +async def test_initialize_timeout_uses_dedicated_budget() -> None: + """The handshake budget is initialize_timeout_seconds, not the request budget. + + Silent-but-alive peer (never answers initialize); initialize_timeout=0.05, + request_timeout=1.0. The raised McpTimeoutError must cite 0.05 — proving + the handshake no longer reads the steady-state knob. + """ + + @asynccontextmanager + async def _silent_factory( + _config: ServerConfig, + ) -> AsyncIterator[TransportStreams]: + # Memory streams buffer infinitely: the initialize request simply + # sits unanswered; no peer task needed. + async with create_client_server_memory_streams() as ( + (cread, cwrite), + (_sread, _swrite), + ): + yield TransportStreams(read=cread, write=cwrite) + + client = McpClient( + transport_factory=_silent_factory, + settings=McpSettings( + initialize_timeout_seconds=0.05, request_timeout_seconds=1.0 + ), + ) + with pytest.raises(McpTimeoutError) as excinfo: + await client.connect(StdioConfig(command="silent-peer")) + assert "0.05" in str(excinfo.value) # the dedicated budget fired + assert "1.0" not in str(excinfo.value) # not the request budget + assert client.is_connected is False + + +async def test_slow_initialize_survives_tight_request_timeout() -> None: + """Regression (#101): a tight request_timeout_seconds must not strangle the + handshake. Peer boots slowly (answers initialize after 0.2 s) while the + steady-state budget is 0.05 s; connect() must still succeed on the default + 30 s initialize budget.""" + + @asynccontextmanager + async def _slow_peer_factory( + _config: ServerConfig, + ) -> AsyncIterator[TransportStreams]: + async with create_client_server_memory_streams() as ( + (cread, cwrite), + (sread, swrite), + ): + + async def _answer_after_a_moment() -> None: + message = await sread.receive() + root = message.message.root + if isinstance(root, JSONRPCRequest) and root.method == "initialize": + await anyio.sleep(0.2) # slow boot: outlasts request_timeout + await swrite.send( + SessionMessage( + JSONRPCMessage( + root=JSONRPCResponse( + jsonrpc="2.0", id=root.id, result=_INIT_RESULT + ) + ) + ) + ) + async for _message in sread: + pass # consume notifications/initialized and everything else + + async with anyio.create_task_group() as tg: + tg.start_soon(_answer_after_a_moment) + try: + yield TransportStreams(read=cread, write=cwrite) + finally: + tg.cancel_scope.cancel() + + client = McpClient( + transport_factory=_slow_peer_factory, + settings=McpSettings(request_timeout_seconds=0.05), + ) + try: + await client.connect(StdioConfig(command="slow-boot-peer")) + assert client.is_connected is True + finally: + await client.aclose() diff --git a/backend/tests/mcp/test_config.py b/backend/tests/mcp/test_config.py index 13c085f..31d90ad 100644 --- a/backend/tests/mcp/test_config.py +++ b/backend/tests/mcp/test_config.py @@ -46,6 +46,18 @@ def test_settings_env_override(monkeypatch: pytest.MonkeyPatch) -> None: assert McpSettings().request_timeout_seconds == 5.0 +def test_settings_default_initialize_timeout(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS", raising=False) + assert McpSettings().initialize_timeout_seconds == 30.0 + + +def test_settings_env_override_initialize_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS", "8") + assert McpSettings().initialize_timeout_seconds == 8.0 + + class TestSupervisionSettings: """Lifecycle-manager knobs (spec: restart policy table).""" diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index c7cd5d3..6301cc6 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -204,7 +204,8 @@ The MCP layer (`octave.mcp`) reads `OCTAVE_MCP_*` variables via | Variable | Default | Description | |---|---|---| -| `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` | `30.0` | Per-request timeout for client calls (and the initialize handshake) | +| `OCTAVE_MCP_REQUEST_TIMEOUT_SECONDS` | `30.0` | Per-request timeout for client calls | +| `OCTAVE_MCP_INITIALIZE_TIMEOUT_SECONDS` | `30.0` | Budget for the initialize handshake during connect/restart | | `OCTAVE_MCP_RESTART_BASE_DELAY_SECONDS` | `1.0` | First auto-restart delay; exponential backoff multiplies by 2 per attempt | | `OCTAVE_MCP_RESTART_MAX_DELAY_SECONDS` | `60.0` | Cap on the backoff delay | | `OCTAVE_MCP_RESTART_MAX_ATTEMPTS` | `5` | Consecutive failed restart cycles before a server enters `crashed` |