Tracked as follow-up to #4319 (F1 acp-bridge package self-sufficiency) review thread by @wenshao. F1 declined the structural fix because it requires either a new shared package OR an integration-test harness — both outside mechanical-lift scope.
Context
The MCP-restart timeout exists in two independent locations:
- Daemon side (
packages/acp-bridge/src/bridge.ts): MCP_RESTART_TIMEOUT_MS = 300_000 — upper bound on a single MCP rediscovery.
- SDK side (
packages/sdk-typescript/src/daemon/DaemonClient.ts): MCP_RESTART_DEFAULT_TIMEOUT_MS = 330_000 — default client-side AbortSignal deadline.
The 30s gap (client > server) is intentional: gives the daemon time to serialize + transmit the structured success/error response before the client aborts. F1 fix-up commit b78de2719 added this headroom after Codex flagged the race when both were exactly 300s.
But the coupling is documentation-only:
- The two constants live in different packages.
- A future server-side bump (e.g., to 600s for a slow-MCP-tier) would not warn that the SDK default is now stale.
- A future SDK timeout drop (e.g., to 200s as a "fail fast" UX tweak) would re-introduce the race the headroom was added to prevent.
Proposal — pick one
Option A: Shared constant
- Define
MCP_RESTART_SERVER_DEADLINE_MS + MCP_RESTART_CLIENT_HEADROOM_MS in a new tiny shared package (or @qwen-code/acp-bridge since both sides already consume from there).
- SDK computes
defaultTimeout = serverDeadline + clientHeadroom.
- Pro: compile-time coupling; no test infra needed.
- Con: SDK now has a build-time dep on acp-bridge (currently it doesn't).
Option B: Cross-package integration test
- New test in
packages/sdk-typescript/test/integration/ that imports MCP_RESTART_TIMEOUT_MS from acp-bridge and asserts SDK_DEFAULT > SERVER_CEILING.
- Pro: no runtime coupling, lightweight.
- Con: requires sdk-typescript to depend on acp-bridge (similar to A).
Option C: SDK startup runtime assertion
- On
DaemonClient construction, optionally check /capabilities payload for advertised mcpRestartTimeoutMs; if SDK default < server ceiling, warn.
- Pro: catches mismatch in production.
- Con: adds network round-trip on first call; needs server-side capability publishing first.
Recommendation
Option A is cleanest. acp-bridge already exposes bridgeOptions + bridgeTypes to consumers; another constant fits the pattern.
Refs
🤖 Generated with Qwen Code
Tracked as follow-up to #4319 (F1 acp-bridge package self-sufficiency) review thread by @wenshao. F1 declined the structural fix because it requires either a new shared package OR an integration-test harness — both outside mechanical-lift scope.
Context
The MCP-restart timeout exists in two independent locations:
packages/acp-bridge/src/bridge.ts):MCP_RESTART_TIMEOUT_MS = 300_000— upper bound on a single MCP rediscovery.packages/sdk-typescript/src/daemon/DaemonClient.ts):MCP_RESTART_DEFAULT_TIMEOUT_MS = 330_000— default client-sideAbortSignaldeadline.The 30s gap (client > server) is intentional: gives the daemon time to serialize + transmit the structured success/error response before the client aborts. F1 fix-up commit
b78de2719added this headroom after Codex flagged the race when both were exactly 300s.But the coupling is documentation-only:
Proposal — pick one
Option A: Shared constant
MCP_RESTART_SERVER_DEADLINE_MS+MCP_RESTART_CLIENT_HEADROOM_MSin a new tiny shared package (or@qwen-code/acp-bridgesince both sides already consume from there).defaultTimeout = serverDeadline + clientHeadroom.Option B: Cross-package integration test
packages/sdk-typescript/test/integration/that importsMCP_RESTART_TIMEOUT_MSfrom acp-bridge and assertsSDK_DEFAULT > SERVER_CEILING.Option C: SDK startup runtime assertion
DaemonClientconstruction, optionally check/capabilitiespayload for advertisedmcpRestartTimeoutMs; if SDK default < server ceiling, warn.Recommendation
Option A is cleanest. acp-bridge already exposes
bridgeOptions+bridgeTypesto consumers; another constant fits the pattern.Refs
b78de2719(added the 30s headroom)🤖 Generated with Qwen Code