feat(gateway): per-server requestTimeoutMs for stdio live calls

[justinkeltner]

Closes #853. Companion to #852 (remote HTTP/SSE) — same field name, so one registry concept covers both transports.

What and why

Remote HTTP/SSE servers can carry a per-server requestTimeoutMs (#852), but stdio live calls stay pinned to the 30-second STDIO_READ_TIMEOUT constant, so a local server with legitimately long synchronous tools is killed at 30 s — and each such call counts as a health failure, tripping the circuit breaker after three occurrences (BREAKER_FAILURE_THRESHOLD).

This honors the same requestTimeoutMs field for stdio servers:

• ServerEntry::request_timeout_ms() reads the field out of the existing unknown_fields flatten map — no ServerEntry shape change, so the 55+ struct-literal sites are untouched and older binaries already preserve the key on re-save (the documented mixed-version contract). When a typed field lands, this getter is the only site to update.
• DownstreamServer gains call_timeout (defaults to STDIO_READ_TIMEOUT) with a set_call_timeout setter; the three catalog-refresh restore sites use it instead of the const.
• connect_one applies the configured value right after connect, clamped to >= 1 ms so a zero entry degrades to fast visible failure, not an inverted deadline. connect_one is also the reconnect factory, so re-spawned adapters re-inherit the value.

What deliberately does not change

Connect handshake (10 s), launcher cold-start budget (120 s), the server/discover probe (750 ms), and every batch health-probe path keep their current bounds — a hung server still fails fast and the grid still can't stall on one slow probe. Only the post-handshake read deadline widens, per server.

Testing

• cargo test --no-default-features --lib — 1283 passed / 0 failed
  (includes new server_entry_request_timeout_ms_round_trips_and_survives_resave)
• End-to-end probe against the built gateway: stdio tool sleeping 45 s with
  requestTimeoutMs: 90000 answered at 45.009 s; the same call without the
  field is killed at 30.007 s on v1.17.0
• Live smoke through the real gateway: identity tool returns normally with
  the field set on a 38-tool stdio server
• npm run test (frontend untouched by this change)
• Registry round-trip: field survives re-save; absent → default 30 s

Notes

• The unknown_fields read is deliberate for review-ability of the stdio/HTTP
  symmetry; if you'd rather have a typed request_timeout_ms field on
  ServerEntry (touching ~55 construction sites), say the word and I'll rework.
• Zero-value policy: clamped to 1 ms here; Make remote request timeout configurable #852 rejects zero for HTTP with a
  config error — easy to align either way.

---

Note

Apply per-server requestTimeoutMs to stdio live calls in gateway.connect_one

After the initial catalog load completes, the connection handler sets the downstream server's request timeout to the configured per-server requestTimeoutMs value. The value is clamped to a minimum of 1ms. Connection, handshake, probe, and initial catalog-load operations continue to use the default bounded read timeout.

• Risk: a misconfigured requestTimeoutMs that is too low could cause live calls to time out prematurely; reviewers should check the clamping logic in toolport-gateway.rs.

^{Macroscope summarized 0d29cd9.}

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🔴 Startup catalog reads inherit live-call timeout

When set_call_timeout runs before initial catalog loading, an unresponsive advertised catalog inherits the configured live-call timeout. Large values can stall startup for each catalog request.

Prompt for agents

In src-tauri/src/bin/toolport-gateway.rs, connect_one applies ServerEntry::request_timeout_ms through DownstreamServer::set_call_timeout immediately after the stdio handshake, but the common success path then calls load_resources_prompts. Those initial resources/list, resources/templates/list, and prompts/list requests use the transport's current timeout, so a large live-call setting also enlarges startup catalog waits. Defer applying the stdio live-call timeout until after load_resources_prompts completes, while preserving the configured timeout for the returned server and every reconnect. Add a transport-level or gateway test with an advertised but unresponsive catalog to verify startup retains its bounded/default budget and subsequent live calls use the configured value.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 83579929-9640-4285-b03b-80daa3ec7d50

📥 Commits

Reviewing files that changed from the base of the PR and between 8eaa654 and 02ab0f0.

📒 Files selected for processing (3)

• src-tauri/src/bin/toolport-gateway.rs
• src-tauri/src/downstream.rs
• src-tauri/src/registry.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The gateway reads each server’s requestTimeoutMs value and applies it to stdio live-call reads. DownstreamServer preserves this timeout across catalog refreshes, while connection, handshake, and probe timeouts remain unchanged.

Changes

Stdio call timeout

Layer / File(s)	Summary
Registry timeout access src-tauri/src/registry.rs	ServerEntry::request_timeout_ms() reads numeric requestTimeoutMs values from preserved unknown fields. Tests cover absent values, parsing, and re-serialization.
Downstream timeout state src-tauri/src/downstream.rs	DownstreamServer stores and applies a live-call timeout. Tool, resource, and prompt refreshes restore that timeout. Test fixtures initialize the default timeout.
Gateway timeout wiring src-tauri/src/bin/toolport-gateway.rs	The gateway applies the configured timeout after connection and clamps it to a minimum of one millisecond. Connection errors remain unchanged.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 02ab0

The change enables longer-running stdio tools while preserving existing connection and discovery limits. A very large configured timeout could delay failure detection for a hung server, so the PR is mergeable with explicit owner awareness or follow-up to define an upper bound.

Sequence Diagram(s)

sequenceDiagram
  participant ServerEntry
  participant ToolportGateway
  participant DownstreamServer
  participant Transport
  ServerEntry->>ToolportGateway: read requestTimeoutMs
  ToolportGateway->>DownstreamServer: connect
  ToolportGateway->>DownstreamServer: set_call_timeout
  DownstreamServer->>Transport: set_read_timeout
  DownstreamServer->>Transport: restore timeout after catalog refresh

Loading

Suggested reviewers: tsouth89, anaygarodia

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The implementation addresses issue #853 by applying requestTimeoutMs to stdio live-call reads, preserving other timeout limits, restoring the value after refreshes, and reapplying it on reconnect. How…	Verify the registry test fixtures and confirm that the complete test suite passes. Remove any literal '+' separators from the JSON fixtures if they are invalid JSON, then rerun the test suite.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: per-server requestTimeoutMs support for stdio live calls.
Description check	✅ Passed	The description is directly related to the changes. It explains the problem, implementation, scope, compatibility behavior, and testing.
Out of Scope Changes check	✅ Passed	The changes remain within scope. They modify stdio timeout configuration, registry compatibility handling, reconnect behavior, and catalog-refresh restoration. No unrelated code changes are identified…

Full details: Linked Issues check

Explanation

The implementation addresses issue #853 by applying requestTimeoutMs to stdio live-call reads, preserving other timeout limits, restoring the value after refreshes, and reapplying it on reconnect. However, the description also reports that the new registry test may use invalid JSON fixtures, which conflicts with the claimed passing test run.

Full details: Out of Scope Changes check

Explanation

The changes remain within scope. They modify stdio timeout configuration, registry compatibility handling, reconnect behavior, and catalog-refresh restoration. No unrelated code changes are identified.

Full details: Docstring Coverage

Explanation

Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files. (1 skipped: 1 too large.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

Warning

⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

[btsouth]

Thanks for this, the shape is right. Plan on my side: #852 merges first, then this needs a rebase. Two things once it's rebased:

1. Make remote request timeout configurable #852 adds request_timeout_ms as a typed field on ServerEntry, so the unknown_fields getter will read None from then on. Read the typed field and drop the getter.
2. The Devin finding is real. connect_one calls set_call_timeout before load_resources_prompts, so the initial resources/list, resources/templates/list and prompts/list reads run at the widened deadline. Either apply the timeout after that load, or have load_resources_prompts pin STDIO_READ_TIMEOUT for its own reads and restore call_timeout after.

One decision to make in the PR: #852 strips requestTimeoutMs from stdio entries on team export and shared import, since it treated the field as remote only. If stdio honors it, those two seams should keep it and the doc comment on the field needs updating. Fine either way, just say which you went with.

[justinkeltner]

Thanks for the review.. all three points make sense. Plan sounds good.

On the typed field: Agreed, once #852 lands I'll rebase and read server.request_timeout_ms directly, dropping the unknown_fields getter. Cleaner that way.

On the Devin finding: You're right, that's a real sequencing bug. I'll go with option A — apply the timeout after load_resources_prompts. Rationale: the initial catalog load is a connect-time handshake-adjacent operation, and it should stay bounded by STDIO_READ_TIMEOUT (30s) so a hung server still fails fast. The widened deadline is only for live tool calls after the connection is healthy. The reconnect path (re-spawned adapters via connect_one) will inherit the same behavior automatically since it goes through the same flow.

On the export/import seam: Since this PR makes stdio honor the field, the two seams should keep requestTimeoutMs for stdio entries — stripping it would silently drop a live setting on team export/shared import, which feels like a footgun. I'll update the doc comment on the field to say it applies to both transports (remote HTTP/SSE and stdio), and adjust the export/import logic + tests to preserve it for stdio.

One suggestion for a follow-up (not this PR): Now that requestTimeoutMs is user-facing for both transports, it'd be worth surfacing it in the Server dialog UI under a timeout/deadline section. Right now users have to hand-edit registry.json to discover or change it, which is opaque for a setting that can silently break things if too low (or hold connections open if too high). Happy to open a separate issue/PR for that if you're interested.

[btsouth]

#852 is merged (3b5f56b), so this is ready for the rebase whenever you are.

[justinkeltner]

Rebased on #852. All three items addressed: typed field is read directly, set_call_timeout moved after load_resources_prompts, and the export/import seams now preserve requestTimeoutMs for stdio. Happy to add the Server dialog UI for this in a follow-up PR if you'd like, or feel free to wire it up yourself — the field is already in the TypeScript types from #852.