fix(mcp): treat server timeout as infra, not a diagnostic (#346)#348
Merged
Conversation
cclsp returns its internal LSP-orchestrator timeout ("orchestrator timeout
after 3s") as a normal MCP text result with isError unset, so it flowed
through dispatch as a real result and lsp-diag counted it as a phantom +1
diagnostic — reading like the edit caused a regression.
Detect infra conditions in the shared _mcp_call_or_message (covers
diag/hover/rename) via two signals: the structural MCP isError flag and a
configurable per-server infra_patterns list (default orchestrator timeout /
timed out / timeout after). Matched results are returned prefixed `op: …`
so the adapter's existing op-prefix guard drops them. No count delta.
Tests: TestMcpInfraSkip (8 pure + 2 end-to-end). Docs + CHANGELOG.
Co-Authored-By: Max <noreply>
Drop bare "timed out"/"timeout after" from the default infra_patterns —
they match against the whole diag result, so one real diagnostic containing
the phrase would drop every diagnostic in that result. Default is now
("orchestrator timeout", "timed out after") — both require word-pairing.
The cclsp bug is caught by "orchestrator timeout"; the daemon variant is
already handled by the exception path.
Also: chr(10) -> "\\n" for file convention, add an end-to-end test for the
structural isError path, sync the default list in docs + CHANGELOG.
Co-Authored-By: Max <noreply>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After an edit,
lsp-diagreported a0 → 1count where the single "diag" was actuallyorchestrator timeout after 3s— an infrastructure condition, not a code finding (phpstan already covered the same file clean). It read like the edit introduced a regression.Root cause
cclsp swallows its internal LSP-orchestrator timeout and hands it back as a normal MCP text result with the
isErrorflag unset. So it flowed through the shared dispatch as a real result, and the lsp-diag adapter counted it as a+1advisory. This is not ourMCPTimeoutexception path (that was already prefix-guarded).Fix
Detection moved to the shared
_mcp_call_or_message— so every MCP-backed op (diag/hover/rename) benefits, not just lsp-diag — with two signals:isErrorflag (spec-standard, any server, zero config).infra_patternssubstring list (default["orchestrator timeout", "timed out", "timeout after"]) for servers that report timeouts as plain text.A matched result is returned prefixed
op: …— the same shape as supertool's own MCP errors — so the adapter's existingop:-prefix guard drops it instead of counting it. No count delta, no false regression. The timeout value stays per-server configurable viamcp.<name>.timeout(unchanged).Tests
TestMcpInfraSkip— 8 pure-helper tests + 2 end-to-end via_stub_server(proved red → green). Full suite: 3480 passed, 34 skipped.Docs
infra_patternsdocumented in both config-key lists indocs/mcp-integration.md;CHANGELOG.mdFixed entry.Closes #346.