Repository navigation
fix(plugins): preserve outcomes when SDK cleanup fails - #228
Merged
Merged
Conversation
5 tasks done
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
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.
What changed
The current TypeScript MCP session wrapper could receive a tool/resource result, then reject with a later
Client.close()error. It could also replace the original operation or cancellation error with the cleanup error. Treat local SDK close as best-effort cleanup, while still attempting the existing bounded remote session termination.This is the current-Server replacement for the obsolete Python-only #211. Work already retains parsed observations for ordinary cleanup failures; the shared wrapper now also preserves its callback outcome if the operation signal expires during cleanup. Missing responses and pre-dispatch cancellation remain failures. There is no retry, authority, timeout, dependency, or receipt-schema change.
The first hosted validation also exposed an existing cold-checkout issue: affected checks typechecked the Server before producing its own compiled files, although retained tests import that compiled entry. Run the same selected packages in sequential build → typecheck → test phases; selection, required gates and authority are unchanged.
Acceptance journey
Open-source research
Verification
npm run check: passed; 30 test tasks (16 cached), 20 build tasks (17 cached). Server 152 passed, Web 698 passed, Desktop 574 passed / 3 skipped. Existing platform/environment skips are not qualification.npm run ui:acceptance -- --entry ts: PASS 12/12; 110 API responses, zero unexpected responses or page errors, with disposable PostgreSQL/mTLS Temporal and actual Chrome.50837bea→e2f81696): workspace lane passed, with build before typecheck/test. The original hosted runner failed on missing compiledpublic-work-webimports. Existing CI scope/guard positive and negative tests passed in the complete check.Security impact
Cleanup cannot authorize another call. Existing endpoint/DNS/TLS bounds, remote cleanup limits, operation admission and durable unknown-effect policy remain in place.
Documentation and compatibility
No public contract or user-facing copy changed. The fix is limited to the current TypeScript transport and its tests; retired Python files are not restored.
AI or automation assistance