fix: composition defects across codegen, replay, daemon elicitation, and public types - #262
Conversation
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix: composition defects across codegen, replay, daemon elicitation, and public types 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b01dca397
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const snapshot = await loadConfigSnapshot(options); | ||
| const elicitation = createNonInteractiveElicitationResponder(); | ||
| const elicitation = createNonInteractiveElicitationResponder({ | ||
| onDecline: () => daemonRequestNotices.getStore()?.add(NON_INTERACTIVE_ELICITATION_HINT), |
There was a problem hiding this comment.
When a keep-alive stdio server is already connected before the call that prompts (for example after listTools/pre-warm or an earlier non-prompting call), the MCP client's stream callbacks run under the async context that existed when the transport was created, not under the current processRequestWithNotices() call. This getStore() can therefore be undefined or point at an old Set, so the daemon response omits notices and the caller still gets a silent decline despite the new warning path; store the notice collector on the in-flight daemon operation rather than relying on AsyncLocalStorage attached to long-lived transports.
Useful? React with 👍 / 👎.
1 participant
Fixes composition failures that survived the green suite — cases where two individually-correct pieces of the MCP 2.0 work broke at their boundary. Found by adversarial review; each fix ships with a regression test that fails before and passes after.
generate-clisilently dropped pinned protocol versions.normalizeDefinition()didn't copyprotocolVersion, so a server pinned to2026-07-28orlegacyreverted to auto-negotiation in generated CLIs and inline-JSON targets. Auditing that function against the config schema turned up four more silently-dropped aliases:token_cache_dir,client_name,oauth_redirect_url,oauth_scope.initialize/server/discovermatching drift-tolerant, but every other modern request still compared per-request client identity_metabyte-for-byte — so a recording made by 0.12.4 failed attools/listunder 0.12.5. The advertised tolerance was passing vacuously because the test stopped after discovery. Matching now normalizes only the three reservedio.modelcontextprotocol/*identity keys; user-supplied_metais still compared strictly.lifecycle: "keep-alive"servers route through the daemon, which always installed the non-interactive responder — so a tool needing input declined instantly with no prompt and no hint. Forwarding prompts across the daemon socket needs protocol multiplexing beyond this scope, so the daemon now returns the actionable hint to the calling CLI, and the limitation plus itslifecycle: "ephemeral"workaround is documented rather than hidden.ConnectionInfoand other publicly-reachable types weren't exported, soimport type { ConnectionInfo } from 'mcporter'failed TS2305.Promise.allstartup left the successfully-started sibling unreferenced by cleanup.subscriptions/listen+toggle_tooland modern cache metadata (ttlMs/cacheScope) — previously implemented in the fixtures but never asserted in CI.Proof:
pnpm checkclean; 970 passed / 13 skipped (the 10 new skips are the opt-in live suite from #261); autoreview clean (0.98).🤖 Generated with Claude Code