Skip to content

Fix hosted remote tool-call timeout#64

Merged
rodaddy merged 2 commits into
mainfrom
fix/remote-timeout-gitingest
Jun 30, 2026
Merged

Fix hosted remote tool-call timeout#64
rodaddy merged 2 commits into
mainfrom
fix/remote-timeout-gitingest

Conversation

@rodaddy

@rodaddy rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace the hardcoded 10s hosted remote request timeout with the normal 60s daemon request timeout
  • add MCP2CLI_REMOTE_REQUEST_TIMEOUT_MS as an override for remote tool calls
  • add a regression test that verifies the override times out and then allows the same delayed remote call

Validation

  • PASS: bun test tests/process/client-remote.test.ts -t 'honors remote request timeout override for hosted tool calls'
  • PASS: bun run typecheck
  • PASS: bun run build
  • PASS: git diff --check
  • LIVE PASS: installed built 0.3.13 locally and mcp2cli gitingest ingest_repo succeeded against octocat/Hello-World via CT216

Known unrelated baseline

  • tests/process/client-remote.test.ts currently has two pre-existing failures on origin/main around source-resolution assertions; they reproduce when run alone and are not introduced by this patch.

@rodaddy

rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Review-swarm: PASS
Pinned diff: 8aca0e4..54f677d
Fresh-context lanes: 5 lanes via Codex subagents: correctness, antagonist/regression, security/auth, runtime/deployment gotcha, quality/maintainability.

Findings:

  • HIGH/MEDIUM: src/process/client.ts remote timeout default changed from 10s to 60s per attempt for all remote calls. With REMOTE_RETRIES=3, remote-local fallback can now wait about 181.5s before falling back to local, a regression from the prior roughly 31.5s path.
  • MEDIUM: tests/process/client-remote.test.ts only proves an override path with a 50ms stub; it does not prove default hosted behavior or remote-local fallback timing/attempt behavior.
  • MEDIUM: MCP2CLI_REMOTE_REQUEST_TIMEOUT_MS is not documented in README.md or deploy/env.example.
  • LOW: timeout test only asserts success=false for the short-timeout branch, not that the failure is specifically a remote timeout/connection failure.
  • Security/auth lane: no findings; token/header behavior and 401/403 no-fallback branch were unchanged.

Disposition: merge is blocked until the timeout behavior is split so explicit remote services can run longer without slowing remote-local fallback, docs are updated, tests are tightened, and a fix-verification swarm reports zero unresolved issues.

@rodaddy rodaddy force-pushed the fix/remote-timeout-gitingest branch from 54f677d to 6dece2a Compare June 30, 2026 02:19
@rodaddy

rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Fixes pushed for review-swarm findings.

Changes:

  • Split remote timeout budgets by route source: explicit remote calls use MCP2CLI_REMOTE_REQUEST_TIMEOUT_MS with a 60000ms default; remote-local probes use MCP2CLI_REMOTE_FALLBACK_TIMEOUT_MS with a 10000ms default before local fallback.
  • Documented both env vars in README.md and deploy/env.example.
  • Tightened the short-timeout test to assert CONNECTION_ERROR and timeout text.
  • Added a remote-local regression test proving the fallback path uses the short fallback timeout even when the remote-only request timeout is longer.
  • Rebuilt and installed the local binary, then re-ran the live CT216 gitingest public repo call successfully.

Validation run locally:

  • PASS: bun test tests/process/client-remote.test.ts
  • PASS: bun run typecheck
  • PASS: bun run build
  • PASS: git diff --check
  • LIVE PASS: mcp2cli gitingest ingest_repo for octocat/Hello-World via CT216 returned README digest.

@rodaddy rodaddy force-pushed the fix/remote-timeout-gitingest branch from 6dece2a to 37ca330 Compare June 30, 2026 02:28
@rodaddy

rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Fix-verification correction pushed.

The antagonist lane correctly found that the previous remote-local test used requiresCredentials=true, so it proved the short remote-local timeout budget but did not prove actual fallback. Updated head now changes that test to:

  • isolate a local daemon on a temp Unix socket;
  • make the service fallback-eligible;
  • force the remote mock to exceed MCP2CLI_REMOTE_FALLBACK_TIMEOUT_MS;
  • assert the final error comes from the local daemon () and not from the remote daemon.

Validation after correction:

  • PASS: bun test tests/process/client-remote.test.ts -t 'uses short fallback timeout for remote-local services'
  • PASS: bun test tests/process/client-remote.test.ts
  • PASS: bun run typecheck
  • PASS: bun run build
  • PASS: git diff --check
  • LIVE PASS: rebuilt/installed local binary and mcp2cli gitingest ingest_repo for octocat/Hello-World via CT216 returned README digest.

@rodaddy rodaddy force-pushed the fix/remote-timeout-gitingest branch from 37ca330 to 7e2c977 Compare June 30, 2026 02:34
@rodaddy

rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Final fallback correction pushed.

The previous fix still allowed remote-local to spend retry/backoff time before local fallback. Head now changes remote-local to one short remote probe by default:

  • explicit remote calls keep MCP2CLI_REMOTE_REQUEST_TIMEOUT_MS and normal MCP2CLI_REMOTE_RETRIES behavior;
  • remote-local calls use MCP2CLI_REMOTE_FALLBACK_TIMEOUT_MS and MCP2CLI_REMOTE_FALLBACK_RETRIES, defaulting to one probe;
  • docs now list MCP2CLI_REMOTE_FALLBACK_RETRIES;
  • the fallback regression test starts an isolated local Unix-socket daemon, forces the remote mock to exceed the short fallback timeout, asserts the local daemon error is returned, and asserts the remote mock was called exactly once.

Validation after final correction:

  • PASS: bun test tests/process/client-remote.test.ts
  • PASS: bun run typecheck
  • PASS: bun run build
  • PASS: git diff --check
  • LIVE PASS: rebuilt/installed local binary and mcp2cli gitingest ingest_repo for octocat/Hello-World via CT216 returned README digest.

@rodaddy

rodaddy commented Jun 30, 2026

Copy link
Copy Markdown
Owner Author

Review-swarm: PASS
Pinned diff: 8aca0e4..04f65f3
Fresh-context lanes: 5 initial lanes plus 2 fix-verification lanes plus 1 final delta lane via Codex subagents: correctness, antagonist/regression, security/auth, runtime/deployment gotcha, quality/maintainability, SME fix verification, antagonist fix verification, final delta verification.
Findings: initial remote timeout, remote-local fallback, documentation, and timeout assertion findings were fixed. Follow-up fix-verification found a documentation and explicit-remote retry test gap; final delta verification found no findings.

Fix-verification: PASS
Zero known unresolved issues: yes
Validation: PASS bun test tests/process/client-remote.test.ts; PASS bun run typecheck; PASS bun run build; PASS git diff --check; PASS GitHub CI check on 04f65f3; PASS GitGuardian on 04f65f3; LIVE PASS rebuilt local binary earlier and mcp2cli gitingest ingest_repo for octocat/Hello-World via CT216 returned README digest.

Final behavior: explicit remote calls use MCP2CLI_REMOTE_REQUEST_TIMEOUT_MS default 60000 and MCP2CLI_REMOTE_RETRIES default 3; remote-local fallback uses MCP2CLI_REMOTE_FALLBACK_TIMEOUT_MS default 10000 and MCP2CLI_REMOTE_FALLBACK_RETRIES default 1.

@rodaddy rodaddy merged commit a09f6e4 into main Jun 30, 2026
4 checks passed
@rodaddy rodaddy deleted the fix/remote-timeout-gitingest branch June 30, 2026 02:45
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