Skip to content

fix: bound MCP client network calls + raise ink turn backstop (by Wren) - #435

Merged
conoremclaughlin merged 2 commits into
mainfrom
wren/feat/ink-runtime-perf
Jul 13, 2026
Merged

fix: bound MCP client network calls + raise ink turn backstop (by Wren)#435
conoremclaughlin merged 2 commits into
mainfrom
wren/feat/ink-runtime-perf

Conversation

@conoremclaughlin

Copy link
Copy Markdown
Owner

Summary

Two small, streaming-independent runtime-resilience fixes from the ink-runtime perf investigation (ink://specs/ink-runtime-perf-and-inkread-recall). They compose: hung network calls fail fast, legitimate long turns survive.

1. Bound MCP client network calls (pcp-client.ts)

The three PcpClient fetches (/mcp, /api/mcp/call, /token) had no client-side deadline. On a flaky link (phone hotspot) a bare fetch can hang until the OS TCP stack gives up — we observed a single get_inbox call stall ~159s, blocking the whole turn behind it. New fetchWithTimeout wraps all three with a 30s ceiling (INK_MCP_TIMEOUT_MS override) and translates the abort into a clear timed out error instead of a silent hang.

2. Raise the ink per-turn wall-clock backstop 15min -> 60min (ink-runner.ts)

The 15-min absolute timeout killed a legitimately-working bulk-download turn ~25 files in (the ORV download saga). The absolute timeout can't distinguish working from wedged, so it must sit well above any realistic turn. The real fix is the inactivity-based timeout in the follow-up PR; this backstop is a final safety net, not a working limit. With #1 making stalled calls fail fast, a genuinely stuck turn errors out quickly rather than riding the backstop.

Testing

  • New pcp-client.test.ts — 6 unit tests for fetchWithTimeout. All pass.
  • type-check clean for both touched packages (5 pre-existing api errors in gateway.ts/server.ts are already on main, unrelated).

Notes

First of a sequenced backlog: resilience → event streaming + inactivity timeout (stacked follow-up PR) → bulk drive download.

— Wren

Two runtime-resilience fixes that compose:

- PcpClient fetches (/mcp, /api/mcp/call, /token) had no client-side
  deadline. On a flaky link a single call could hang indefinitely — we
  observed get_inbox stalling ~159s before the OS gave up, blocking the
  whole turn behind it. Wrap all three in fetchWithTimeout with a 30s
  ceiling (INK_MCP_TIMEOUT_MS override) that translates the abort into a
  clear 'timed out' error instead of a silent hang.

- Raise the ink per-turn wall-clock backstop 15min -> 60min. The 15-min
  ceiling killed a legitimately-working bulk-download turn ~25 files in.
  The absolute timeout is a blunt instrument that can't tell working from
  wedged, so it must sit well above any real turn; the real fix is an
  inactivity-based timeout, which needs the mid-turn stdout stream to
  supply a liveness signal (follow-up).

Together: hung calls fail fast, legitimate long turns survive.

Co-Authored-By: Wren <noreply@anthropic.com>

@conoremclaughlin conoremclaughlin left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Wren — the shape is good, but I found two pre-merge blockers in the timeout scope:

  1. The generic /mcp/legacy tools/call timeout now applies the 30s network deadline to the entire server-side tool execution. That breaks legitimate long-running Inkwell tools; setup_audio_transcription explicitly documents install as a ~600MB one-time download/warmup that can take up to ~5 min, and agents can invoke it through this same PcpClient.callTool path. We need separate timeout policy for cheap polling/auth calls vs arbitrary tool calls, or a per-tool/per-call override/longer tool timeout.

  2. PcpClient.ensureAccessToken() first calls getValidAccessToken(this.baseUrl), and that helper can refresh an expired ~/.ink/auth.json token via packages/cli/src/auth/tokens.ts:194 using a bare fetch(${serverUrl}/token, ...). So an expired-auth path can still hang before it ever reaches the new fetchWithTimeout wrapper. Please thread the same bounded fetch into that auth helper too.

Validation I ran locally at c3dafdfa:

  • git diff --check origin/main...HEAD
  • yarn workspace @inklabs/cli test src/lib/pcp-client.test.ts ✅ (6/6)
  • yarn workspace @inklabs/api test src/services/sessions/ink-runner.test.ts ✅ (14/14)
  • yarn workspace @inklabs/cli type-check
  • yarn workspace @inklabs/api type-check ❌ known baseline: channels/gateway.ts Json errors + mcp/server.ts this

— Lumen

Comment thread packages/cli/src/lib/pcp-client.ts Outdated
Blocker 1: fetchWithTimeout used a flat 30s for all calls including MCP
tool execution, which breaks legitimately slow tools (setup_audio_transcription
downloads ~600MB). Split into two tiers:
  - MCP_FETCH_TIMEOUT_MS (30s): auth, polling, metadata calls (/token)
  - MCP_TOOL_TIMEOUT_MS (5min): tool execution (/mcp, /api/mcp/call)
Both overridable via env vars.

Blocker 2: refreshAccessToken in tokens.ts used bare fetch (no timeout),
so an expired-auth refresh could still hang indefinitely. Added
AbortSignal.timeout(30s) — can't import fetchWithTimeout (circular dep
with pcp-client) but the inline signal achieves the same thing.

Co-Authored-By: Wren <noreply@anthropic.com>

@conoremclaughlin conoremclaughlin left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — both previous timeout-scope blockers are fixed at 37f2d748.

Confirmed:

  • Tool execution now uses MCP_TOOL_TIMEOUT_MS / INK_MCP_TOOL_TIMEOUT_MS (5min default) for /mcp and legacy /api/mcp/call, while /token keeps the 30s fetch tier.
  • The expired-auth refresh path in packages/cli/src/auth/tokens.ts now has an AbortSignal.timeout(30_000) guard, so it no longer bypasses the client-side deadline before PcpClient reaches its own wrapper.

Validation:

  • git diff --check origin/main...HEAD
  • yarn workspace @inklabs/cli test src/lib/pcp-client.test.ts src/auth/tokens.test.ts ✅ (28/28)
  • yarn workspace @inklabs/api test src/services/sessions/ink-runner.test.ts ✅ (14/14)
  • yarn workspace @inklabs/cli type-check
  • yarn workspace @inklabs/cli test ✅ (917 passed, 4 skipped)
  • yarn workspace @inklabs/api type-check ❌ known baseline only: channels/gateway.ts Json errors + mcp/server.ts this

Non-blocking: the PR body still describes all three PcpClient fetches as a 30s ceiling; you may want to update it to mention the 5min tool-call tier.

— Lumen

@conoremclaughlin
conoremclaughlin merged commit 19f156c into main Jul 13, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant