fix(mcpls-core): correct error classification and unsubscribe leak - #501
Merged
Merged
Conversation
…ectly, fix unsubscribe leak on deleted files Route SubscriptionError::LimitReached through the mcp_error_kind classifier instead of hand-mapping it to InvalidParams, matching DocumentLimitExceeded's treatment (aggregate tracker state, not a per-request bad parameter). Reclassify LspServerError as InvalidParams only for the recognized position-out-of-range message shape that sanitize_lsp_server_message already rewrites for display; every other LspServerError message stays Internal. Consolidate the three bespoke JSON-RPC error codes into error.rs. unsubscribe previously fell back to the raw request URI whenever canonicalization failed, which is exactly the case where a deleted file's raw URI no longer matches the canonical key subscribe recorded it under -- permanently leaking the subscription slot. subscribe now records a raw-to-canonical alias (bounded to one alias per canonical, pruned on either removal path) so a stale unsubscribe still resolves to the right entry; a failed removal is now also logged at debug level. Closes #496 Closes #499
github-actions
Bot
added
documentation
bug-ops
enabled auto-merge (squash)
September 21, 2026 13:48
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.
Summary
SubscriptionError::LimitReachedthrough the existingmcp_error_kindclassifier instead of hand-mapping it toInvalidParams, matchingDocumentLimitExceeded's treatment: it fires on aggregate per-session tracker state, not this request's parameters, so it stays a retryable server-side condition rather than a caller-fault one.LspServerErrorasInvalidParamsonly for the recognized "position out of range" message shape thatsanitize_lsp_server_messagealready rewrites for display (a stale line/character from the caller). Every otherLspServerErrormessage staysInternal, since that variant legitimately carries genuine server faults too.WORKSPACE_INDEXING_ERROR_CODE,SERVER_INITIALIZING_ERROR_CODE,STATELESS_SUBSCRIPTION_ERROR_CODE) intoerror.rs, so allocating the next code no longer requires checking two files.unsubscribeleaking a capped subscription slot when canonicalization fails (e.g. the file was deleted since subscribing): it previously fell back to the raw request URI as a lookup key, which almost never matches the canonical URIsubscriberecorded it under.subscribenow also records a raw-to-canonical alias (bounded to one alias per canonical URI, pruned on either removal path), so a stale unsubscribe resolves to the right entry. A failed removal is now logged at debug level.Test plan
cargo +nightly fmt --all -- --checkcargo clippy --all-targets --all-features --workspace -- -D warningscargo nextest run --workspace --all-features --lib --bins(1016 passed)cargo test --doc --workspace --all-featuresRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featuresCloses #496
Closes #499