fix(ci): platform-portable mcp stdin + abort test; round-2 bug batch#9
Merged
Conversation
Two platform-specific CI failures merged to main while the local verify (macOS-only) stayed green — neither failure mode reproduces on macOS: - ubuntu: McpServerConnection never listened for 'error' on the child's stdin. Writing to a just-exited MCP server's stdin surfaces an async EPIPE on the writable stream; with no listener Node escalates it to an uncaught exception and kills the process (macOS silently buffers the write, so mcp-stdin-write-error.spec.mjs passed there). Add a stdin 'error' handler that fails in-flight requests and marks the connection closed, and guard notify()'s best-effort write against a sync throw. - windows: run-process-abort-cleanup.spec.mjs spawned '/bin/sh', which does not exist on Windows and ENOENTs before the abort timer fires, so the assertion saw a spawn error instead of an abort rejection. Spawn a long-running child via process.execPath + `node -e setTimeout` instead, exercising abort cleanup identically on all three platforms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A read-only parallel review surfaced a batch of real bugs; each fix ships with a red-before/green-after test. Flawed candidate findings were dropped when their test failed `npm run verify` (verify-as-arbiter). - cli/community-auth: a line-wrapped portal-token paste was joined with '+' (token.replace(/\s+/g, '+')), corrupting the JWT; strip whitespace instead. Export normalizePortalToken for the regression test. - cli/input/vim: count-prefix digit guard used /^[1-9]$/, so '0' could never be a non-leading count digit (10dd was unreachable); accept /^[0-9]$/ with the bare-'0' line-start motion preserved. Add default mode indicator/color. - cli/attachments: validate index bounds before use. - cli/print: safe JSON stringify for non-serializable values. - cli/tui: numeric picker accepts [1-9]. - provider/anthropic: tool_use input must be an object, not a bare value. - tools/browser-tools: bound the close() wait with a timeout. - tools/web-fetch: re-check the SSRF waiver after a redirect. - memory-manager: extractTerms built n-grams in O(2^n); cap the window. - skill-learner: extractToolCalls now tracks per-call failure for scoring. - contracts/async-task: consistent summary fallback. - contracts/host-adapter: reject tools whose side-effect surface is undeclared. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Why
CI on
mainis red (runs #7/#8) even though localverifyis green — because the local gate only runs on macOS, and neither failure reproduces on macOS. This is the real "检测不完备": macOS-only verification cannot catch Linux EPIPE semantics or the absence of/bin/shon Windows.Confirmed failures fixed (commit 1,
e7af47e)mcp-stdin-write-error.spec.mjscrashed the node process withwrite EPIPE.McpServerConnectionnever listened for'error'on the child's stdin; writing to a just-exited MCP server's stdin emits an async error event that, unhandled, escalates to an uncaught exception. macOS silently buffers the write, so the test passed there. Fix: add a stdin'error'handler (fail in-flight requests + mark closed) and guardnotify()'s best-effort write.run-process-abort-cleanup.spec.mjsspawned/bin/sh, which ENOENTs on Windows before the abort timer fires, so the assertion saw a spawn error, not an abort rejection. Fix: spawn a cross-platform sleeper viaprocess.execPath+node -e setTimeout.The test runner is fail-fast, so each platform died at its first failure and hid everything after it. I swept every test that actually spawns a process: only
run-process-abort-cleanupwas unguarded;tools-cancel-safety,ssh-end-to-end,background-execalready skip on win32, and the MCP tests usenode. New tests sorting after the crash point carry no/bin/sh/spawn hazards.Round-2 review bug batch (commit 2,
c30c74f)Verified bug fixes (each with a red-before/green-after test) across cli (community-auth token corruption, vim count
0, attachments bounds, print, tui), providers (anthropic tool_use object), tools (browser close timeout, web-fetch SSRF-after-redirect), memory (exponential n-grams), skills (tool-call failure tracking), and contracts (async-task summary, host-adapter undeclared-surface rejection).Verification
npm run verifygreen locally (macOS).mainonly after CI passes on ubuntu + macos + windows (matrixfail-fast: false, so all three report).