Skip to content

fix(plugins): preserve outcomes when SDK cleanup fails - #228

Merged
yxflc11 merged 3 commits into
mainfrom
codex/plugin-cleanup-results
Oct 10, 2026
Merged

yxflc11 merged 3 commits into
mainfrom
codex/plugin-cleanup-results

Conversation

@yxflc11

@yxflc11 yxflc11 commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

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

  1. Read a tool or resource result from the existing official MCP SDK loopback fixture, then inject local SDK close failure, including signal expiry during cleanup.
  2. Retain the exact response, perform one tool/resource request and one remote session deletion, with no resend.
  3. Inject an operation failure or pre-dispatch caller cancellation plus close failure; retain the original error rather than returning a result.

Open-source research

  • Evidence: docs/OPEN_SOURCE_REUSE.md (MCP lifecycle and shared plugin contracts), docs/research/plugin-flow-refactor.md, and docs/research/typescript-control-plane-p0.md (reviewed MCP advisory update).
  • Reuse: existing MCP SDK 1.32.1 and its Client lifecycle, the current bounded PluginTransport and the existing contractPluginFixture. Reviewed endpoint, authority and persistence assumptions remain unchanged.
  • Source copied or substantially adapted: no.

Verification

  • Regression before fix: 6 new cases failed, 3 existing cases passed.
  • Focused plugin transport suite after fix: 9/9 passed, using a real loopback SDK peer with injected close errors/signal expiry. This is transport/outcome evidence, not a new SQL/Temporal recovery qualification.
  • 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.
  • Cold-output reproduction: removed the generated Server output, then ran the corrected affected-check runner on the original immutable two-file range (50837bea → e2f81696): workspace lane passed, with build before typecheck/test. The original hosted runner failed on missing compiled public-work-web imports. Existing CI scope/guard positive and negative tests passed in the complete check.
  • Node 24.21.0 on macOS arm64; synthetic fixture data, no installed application/profile or paid model calls. Hosted CI is restarting for the runner repair; the earlier failed run is not reported as passing.

Security impact

  • No new privilege or external side effect
  • No credentials or private user data included

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

  • Tools/models used: Codex, repository checks, existing MCP fixture and browser acceptance.
  • Human-reviewed areas and evidence: Owner requested checking the TypeScript replacement for fix(plugins): retain observed results after cleanup failures #211 and opening a small repair PR if needed. Review and merge are pending; no merge or installation is authorized.

@yxflc11
yxflc11 merged commit a21882b into main Oct 10, 2026
17 checks passed
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