docs: correct call-timeout default (60s, not 30s)#230
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 7:25 AM ET / 11:25 UTC. Summary Reproducibility: not applicable. This PR corrects documentation rather than reporting a runtime failure; source inspection is sufficient to verify the timeout contract. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the README change and rewrite the CLI-reference entry as “defaults to 60 s; override with Do we have a high-confidence way to reproduce the issue? Not applicable: this PR corrects documentation rather than reporting a runtime failure; source inspection is sufficient to verify the timeout contract. Is this the best way to solve the issue? No, not quite: distinguishing 30-second listings from 60-second calls is correct, but the CLI-reference wording should identify Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 710840fb7dd0. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: docs: correct call-timeout default (60s, not 30s) This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
What Problem This Solves
Two docs claim the call timeout defaults to 30 s, but the code defaults it to 60 s. Only the list timeout is 30 s.
In
src/cli/timeouts.ts:resolveCallTimeoutis whatmcporter calluses (src/cli/call-command.ts:74), and the command's own timeout error message names the same env var: "OverrideMCPORTER_CALL_TIMEOUTor pass--timeoutto adjust." (src/cli/call-command.ts:533).Two doc sites are wrong:
README.md— "Timeouts default to 30 s; override withMCPORTER_LIST_TIMEOUTorMCPORTER_CALL_TIMEOUT…" lumps both env vars under a single 30 s figure, so a reader expectsmcporter callto abort at 30 s when it actually runs to 60 s.docs/cli-reference.md— "--timeout <ms>– override call timeout (defaults toCALL_TIMEOUT_MS)" references a symbol that does not exist in the codebase; the real knob is theMCPORTER_CALL_TIMEOUTenv var (default 60 s).Why This Change Was Made
Docs-correctness only. The diff touches two Markdown lines and no production code, config, or defaults:
MCPORTER_CALL_TIMEOUT) and the real 60 s default instead of the non-existentCALL_TIMEOUT_MS.User Impact
No runtime change. Readers relying on the docs now get the correct call-timeout default (60 s) and the correct env-var name to override it, so timeout expectations for
mcporter callmatch observed behavior.Evidence
Claims verified against current
main(710840f):src/cli/timeouts.ts:1-2—DEFAULT_LIST_TIMEOUT_MS = 30_000,DEFAULT_CALL_TIMEOUT_MS = 60_000.src/cli/timeouts.ts:23— call default falls back toMCPORTER_CALL_TIMEOUT/DEFAULT_CALL_TIMEOUT_MS(60 s).src/cli/call-command.ts:74—mcporter callresolves its timeout viaresolveCallTimeout.src/cli/call-command.ts:533— user-facing timeout error namesMCPORTER_CALL_TIMEOUT, matching the corrected docs.git diff --checkis clean; the change is limited toREADME.mdanddocs/cli-reference.md.Generated by Claude Code
Generated by Claude Code