Skip to content

fix(v1): bound null harness MCP connect at 60s#1977

Merged
eligotts merged 4 commits into
mainfrom
fix/null-mcp-connect-timeout
Jul 14, 2026
Merged

fix(v1): bound null harness MCP connect at 60s#1977
eligotts merged 4 commits into
mainfrom
fix/null-mcp-connect-timeout

Conversation

@eligotts

@eligotts eligotts commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Wrap the null harness's connect_mcp in asyncio.wait_for(..., timeout=60) so a wedged streamable-HTTP handshake fails loudly instead of eating the whole rollout budget as a 0-turn harness_timeout.
  • Incidental: this file was already out of format vs current ruff on main; touching it applies that format (no behavior change beyond the timeout).

Test plan

  • Smoke a null-harness eval with MCP tools (happy path still connects)
  • Optional: force a stuck connect and confirm it surfaces as a program error within ~60s

Note

Low Risk
Isolated to the null eval harness startup path; improves failure visibility without touching auth, data, or shared production APIs.

Overview
MCP connect in the null harness is capped at 60 seconds so a stuck streamable-HTTP handshake fails with asyncio.TimeoutError instead of hanging until the rollout hits a 0-turn harness_timeout.

connect_mcp runs inside asyncio.timeout(60) when mcpServers is configured; the no-MCP path is unchanged. The implementation uses asyncio.timeout rather than wait_for so MCP/httpx cancel scopes registered on the shared AsyncExitStack are torn down in the same task.

The inline script metadata bumps requires-python to >=3.11, which is required for asyncio.timeout.

Reviewed by Cursor Bugbot for commit 250a5c8. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Bound null harness MCP connect with a 60s timeout

Wraps the connect_mcp call in program.py with asyncio.timeout(60) so MCP initialization fails with asyncio.TimeoutError after 60 seconds instead of waiting indefinitely. Also bumps the script's requires-python from >=3.10 to >=3.11, which is required for asyncio.timeout.

Changes since #1977 opened

  • Replaced multi-line comment with shorter comment in main function [250a5c8]

Macroscope summarized cf5fa73.

An unbounded streamable-HTTP handshake can wedge on an intermittent
mcp-SDK race and silently burn the rollout as a 0-turn harness_timeout.
Fail loud so the error is classified and retryable.

Also applies current ruff format to this file (already drifted on main).

Co-authored-by: Cursor <cursoragent@cursor.com>
@eligotts eligotts force-pushed the fix/null-mcp-connect-timeout branch from 42a8d50 to 595d321 Compare July 13, 2026 04:09
Comment thread verifiers/v1/harnesses/null/program.py Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 13, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved

This is a straightforward defensive timeout addition (60s bound) to prevent indefinite hangs on MCP connection. The Python version bump from 3.10 to 3.11 is necessary for asyncio.timeout. Small, self-contained bug fix with clear intent.

You can customize Macroscope's approvability policy. Learn more.

wait_for runs connect_mcp in a child task; MCP/httpx cancel scopes pushed
onto AsyncExitStack then fail on parent-task cleanup. timeout() keeps
enter/exit in the same task.

Co-authored-by: Cursor <cursoragent@cursor.com>
@eligotts

Copy link
Copy Markdown
Contributor Author

Addressed the cancel-scope review: switched from asyncio.wait_for to async with asyncio.timeout(60) so MCP contexts entered onto the exit stack are cleaned up in the same task.

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 13, 2026
Resolves the conflict in verifiers/v1/harnesses/null/program.py: this branch's
asyncio.timeout(60) wrap around connect_mcp() lands adjacent to main's
--initial-messages-file refactor (#1974). Kept both: the timeout wrap, and main's
file-based `initial` (dropping this branch's now-stale
`initial = json.loads(os.environ.get("INITIAL_MESSAGES", "[]"))`, which would have
silently clobbered the value main's refactor already computed earlier in main()).

Also re-ran ruff format: this branch's incidental single-line reformatting of
chat()/connect_mcp() no longer matches the pinned ruff (0.15.12) target — main's
original multi-line wrapping is what ruff format now produces, confirmed by running
ruff format --diff against main's own unmodified file (no changes). Restored it so
the diff is limited to the actual timeout fix + the requires-python bump.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Jul 14, 2026
xeophon
xeophon previously approved these changes Jul 14, 2026
The bounding rationale (bounded handshake, 0-turn harness_timeout) restates what
the diff and PR description already say. Keep only why asyncio.timeout instead of
wait_for: cancel scopes entered onto `stack` must exit in this task.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@eligotts eligotts dismissed stale reviews from xeophon and macroscopeapp[bot] via 250a5c8 July 14, 2026 19:43
@eligotts eligotts merged commit 13eed1f into main Jul 14, 2026
13 checks passed
@eligotts eligotts deleted the fix/null-mcp-connect-timeout branch July 14, 2026 23:44
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.

3 participants