fix: stop tab-input crashes and other 0.34.0 regressions (#2479)

* fix: stop tab-input crashes and other 0.34.0 regressions Release blockers found while reviewing release PR #2288: - tui: pin wrap-ansi to 9.0.2. wrap-ansi 10 expands tabs, so Cursor's wrapped-line lookup threw "Failed to find wrapped line in text" and a tab typed or pasted into any text input other than the main prompt exited the session. Only the Node runtime loads the npm package (under Bun, src/ink/wrapAnsi.ts uses Bun.wrapAnsi), so tests never saw it. A new contract test checks the npm package against Bun.wrapAnsi, and CONTRIBUTING documents the pin. - release: run the publish job's unit tests with test:full, the 8-shard layout PR Checks validates, and restore the real secure-storage modules after codexCredentials.test.ts. In one process its plainTextStorage mock leaked into mcpNeedsAuth.test.ts and failed 2 tests. - docker: copy vendor/ before bun install. Bun 1.4.4 fails the frozen install when the node-domexception shim folder is missing. Regressions from this release cycle: - settings: a command hook with the unsupported "args" field is skipped with a warning instead of invalidating the whole settings file, which also dropped that file's permission deny rules (#2398). Covers settings files and command-based managed settings; settings edits still reject it. - mcp: raise the stdio message limit to 128 MiB for MCP servers and `mcp serve`. MCP SDK 1.32 added a 10 MiB default that closed the connection on one large tool result (#2371). - lsp: raise the lifecycle notification timeout from 2s to 30s, matching startup. A server busy indexing for a few seconds was stopped and restarted on every edit (#2106). Co-Authored-By: OpenClaude <openclaude@gitlawb.com> * fix: address review of the 0.34.0 regression fixes MCP stdio: read messages with a linear-time buffer. The SDK 1.32.1 ReadBuffer copies and rescans all pending bytes for every chunk, so a 100 MiB message blocked the event loop for about 10 seconds under Node. It also kept a parsed message's allocation alive through an empty subarray. The new McpStdioReadBuffer replaces it in the client and `mcp serve` transports, with the same limit, errors and parsing. Hooks: when skipping a command hook with `args` empties a matcher or event, remove those too. Otherwise a managed policy holding only a skipped hook counts as non-empty and outranks a lower-priority policy and its deny rules. Settings write-back keeps a skipped hook, so an unrelated settings update no longer deletes it. Settings parsing also copies the memoized JSON parse before filtering, so a re-parse of the same content still sees the raw file and reports the warnings again. The stdio transport test now fails fast if the server closes without sending a message. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: reset cached model strings before the fast-mode Opus check The first fast-mode test resolved "opus" through model strings that an earlier suite had cached for an OpenAI provider, so it got gpt-4o and failed. It failed in about half of the unit shard 3 runs, on this branch and on the previous commit; the unsharded run hid it. The release publish job now runs the sharded test:full, so reset the cache before the test instead of relying on the previous file's state. Co-Authored-By: OpenClaude <openclaude@gitlawb.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: OpenClaude <openclaude@gitlawb.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-06 12:50:54 -04:00 · 2026-11-02 14:20:42 +08:00
parent cd6655470a
commit 87cbb5cba1
20 changed files with 737 additions and 30 deletions
@@ -104,8 +104,11 @@ jobs:
      - name: Build
        run: bun run build

+      # Same layout PR Checks validates: eight unit shards plus the dedicated
+      # conversation-arc and context-efficiency passes. A single-process run
+      # here hit cross-file mock leaks that no PR run could surface.
      - name: Run unit tests
-        run: bun run test
+        run: bun run test:full

      - name: Smoke test
        run: bun run smoke
@@ -267,6 +267,14 @@ only. Structured diff rendering in `src/native-ts/color-diff/index.ts` loads
 `highlight.js` separately. Run the highlighting and Markdown regression tests
 when changing this dependency or its ANSI renderer.

+Keep `wrap-ansi` pinned to `9.0.2`. Version 10 expands tabs into spaces, and
+`Cursor` (`src/utils/Cursor.ts`) and Ink's style mapping expect the wrapped
+text to keep the original characters; with v10, a tab in a text input throws
+and exits the session. Only the Node runtime loads the npm package: under Bun,
+`src/ink/wrapAnsi.ts` uses `Bun.wrapAnsi`, so tests never exercise it.
+`src/ink/wrapAnsi.test.ts` asserts the npm package keeps tabs. Adapt the tab
+handling before upgrading past 9.x.
+
 Keep all direct dependencies and devDependencies pinned to exact versions in the
 root and web manifests. Regenerate the relevant Bun lockfile
 when changing a pin.
@@ -5,6 +5,8 @@ WORKDIR /app

 # Copy dependency manifests first for better layer caching
 COPY package.json bun.lock .bun-version ./
+# The node-domexception override in package.json resolves to this local shim
+COPY vendor/ vendor/

 # Install the Bun version tracked by the repo
 RUN set -eu; \
@@ -84,7 +84,7 @@
        "usehooks-ts": "3.1.1",
        "vscode-languageserver-protocol": "3.18.5",
        "web-tree-sitter": "0.25.10",
-        "wrap-ansi": "10.0.2",
+        "wrap-ansi": "9.0.2",
        "ws": "8.22.0",
        "xss": "1.0.16",
        "yaml": "2.9.1",
@@ -1144,7 +1144,7 @@

    "statuses": ["statuses@2.0.2", "", {}, "sha512-DvEy55V3DB7uknRo+4iOGT5fP1slR8wQohVdknigZPMpMstaKJQWhwiYBACJE3Ul2pTnATihhBYnRhZQHGBiRw=="],

-    "string-width": ["string-width@8.3.0", "", { "dependencies": { "get-east-asian-width": "^1.5.0", "strip-ansi": "^7.1.2" } }, "sha512-ZbmZM0JCihQN91dWnxoipT2KOEyHqEyfRXUyjuRhW8b/xnqPDoq4gWEVApTVa9db2wN8mmoikgFBbjh71+cGeQ=="],
+    "string-width": ["string-width@7.2.0", "", { "dependencies": { "emoji-regex": "^10.3.0", "get-east-asian-width": "^1.0.0", "strip-ansi": "^7.1.0" } }, "sha512-tsaTIkKW9b4N+AEj+SVA+WhJzV7/zMhcSu78mLKWSk7cXMOSHsBKFWUs0fWwq8QyK3MgJBQRX6Gbi4kYbdvGkQ=="],

    "strip-ansi": ["strip-ansi@7.2.0", "", { "dependencies": { "ansi-regex": "^6.2.2" } }, "sha512-yDPMNjp4WyfYBkHnjIRLfca1i6KMyGCtsVgoKe/z1+6vukgaENdgGBZt+ZmKPc4gavvEZ5OgHfHdrazhgNyG7w=="],

@@ -1218,7 +1218,7 @@

    "word-wrap": ["word-wrap@1.2.5", "", {}, "sha512-BN22B5eaMMI9UMtjrGd5g5eCYPpCPDUy0FJXbYsaT5zYxjFOckS53SQDE3pWkVoWpHXVb3BrYcEN4Twa55B5cA=="],

-    "wrap-ansi": ["wrap-ansi@10.0.2", "", { "dependencies": { "ansi-styles": "^6.2.3", "string-width": "^8.2.0" } }, "sha512-6OLRNZqRntVGBm26ghZ5eZhCuWWyOkRCSn8XsOIuq5stpCE/tjLbdozd93naILZig0lmD8ANfRwnhn0DLeXGlg=="],
+    "wrap-ansi": ["wrap-ansi@9.0.2", "", { "dependencies": { "ansi-styles": "^6.2.1", "string-width": "^7.0.0", "strip-ansi": "^7.1.0" } }, "sha512-42AtmgqjV+X1VpdOfyTGOYRi0/zsoLqtXQckTmqTeybT+BDIbM/Guxo7x3pE2vtpr1ok6xRqM9OpBe+Jyoqyww=="],

    "wrappy": ["wrappy@1.0.2", "", {}, "sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ=="],

@@ -1486,6 +1486,8 @@

    "qrcode/yargs": ["yargs@15.4.1", "", { "dependencies": { "cliui": "^6.0.0", "decamelize": "^1.2.0", "find-up": "^4.1.0", "get-caller-file": "^2.0.1", "require-directory": "^2.1.1", "require-main-filename": "^2.0.0", "set-blocking": "^2.0.0", "string-width": "^4.2.0", "which-module": "^2.0.0", "y18n": "^4.0.0", "yargs-parser": "^18.1.2" } }, "sha512-aePbxDmcYW++PaqBsJ+HYUFwCdv4LVvdnhBy78E57PIor8/OVvhMrADFFEDh8DHDFRv/O9i3lPhsENjO7QX0+A=="],

+    "string-width/emoji-regex": ["emoji-regex@10.6.0", "", {}, "sha512-toUI84YS5YmxW219erniWD0CIVOo46xGKColeNQRgOzDorgBi1v4D71/OFzgD9GO2UGKIv1C3Sp8DAn0+j5w7A=="],
+
    "type-is/content-type": ["content-type@2.0.0", "", {}, "sha512-j/O/d7GcZCyNl7/hwZAb606rzqkyvaDctLmckbxLzHvFBzTJHuGEdodATcP3yIRoDrLHkIATJuvzbFlp/ki2cQ=="],

    "xss/commander": ["commander@2.20.3", "", {}, "sha512-GpVkmM8vF2vQUkj2LvZmD35JxeJOLCwJ9cUkugyk2nuhbv3+mJvpLYYt+0+USMxE+oj+ey/lJEnhZw75x/OMcQ=="],
@@ -165,7 +165,7 @@
    "usehooks-ts": "3.1.1",
    "vscode-languageserver-protocol": "3.18.5",
    "web-tree-sitter": "0.25.10",
-    "wrap-ansi": "10.0.2",
+    "wrap-ansi": "9.0.2",
    "ws": "8.22.0",
    "xss": "1.0.16",
    "yaml": "2.9.1",
@@ -6,7 +6,6 @@
 process.env.CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS ??= 'true'

 import { Server } from '@modelcontextprotocol/sdk/server/index.js'
-import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
 import { ZodError } from 'zod/v4'
 import {
  CallToolRequestSchema,
@@ -19,6 +18,7 @@ import { getDefaultAppState } from 'src/state/AppStateStore.js'
 import review from '../commands/review.js'
 import type { Command } from '../commands.js'
 import { getMcpToolsCommandsAndResources } from '../services/mcp/client.js'
+import { createMcpStdioServerTransport } from '../services/mcp/stdioTransport.js'
 import type { MCPServerConnection } from '../services/mcp/types.js'
 import {
  findToolByName,
@@ -257,7 +257,7 @@ export async function startMCPServer(
  )

  async function runServer() {
-    const transport = new StdioServerTransport()
+    const transport = createMcpStdioServerTransport()
    await server.connect(transport)
  }

@@ -0,0 +1,35 @@
+import { describe, expect, test } from 'bun:test'
+import wrapAnsiNpm from 'wrap-ansi'
+
+// The shipped CLI runs on Node, where ./wrapAnsi.ts falls back to the npm
+// package; this suite runs on Bun, where it uses Bun.wrapAnsi. Exercise the
+// npm package directly so a release that changes its output fails here rather
+// than only in the shipped CLI. wrap-ansi 10 expands tabs into spaces, which
+// made Cursor throw "Failed to find wrapped line in text" for any tab typed
+// into a text input and exited the session.
+const OPTIONS = { hard: true, trim: false } as const
+
+const SAMPLES: ReadonlyArray<readonly [string, number]> = [
+  ['foo\tbar', 80],
+  ['\t\tindented', 80],
+  [`${'x'.repeat(78)}\tyy\tzz`, 80],
+  ['a\tb c\td e\tf g\th', 8],
+  ['\u001b[31mred\tword\u001b[39m tail', 6],
+  ['日本\t語の文章です', 6],
+  ['plain words wrap here', 7],
+]
+
+describe('npm wrap-ansi (the Node runtime path)', () => {
+  test('keeps tab characters instead of expanding them', () => {
+    expect(wrapAnsiNpm('foo\tbar', 80, OPTIONS)).toBe('foo\tbar')
+    expect(wrapAnsiNpm('\t\tindented', 80, OPTIONS)).toBe('\t\tindented')
+  })
+
+  test('matches Bun.wrapAnsi, which the Bun-run tests exercise', () => {
+    for (const [input, columns] of SAMPLES) {
+      expect(wrapAnsiNpm(input, columns, OPTIONS)).toBe(
+        Bun.wrapAnsi(input, columns, OPTIONS),
+      )
+    }
+  })
+})
@@ -10,7 +10,17 @@ import {
 } from './hooks.js'
 import { PluginHooksSchema } from '../utils/plugins/schemas.js'
 import { SettingsSchema } from '../utils/settings/types.js'
-import { parseSettingsFile } from '../utils/settings/settings.js'
+import {
+  clearMdmSettingsCache,
+  parseCommandOutputAsSettings,
+  setMdmSettingsCache,
+} from '../utils/settings/mdm/settings.js'
+import {
+  getSettingsForSource,
+  loadManagedFileSettings,
+  parseSettingsFile,
+} from '../utils/settings/settings.js'
+import { resetSettingsCache } from '../utils/settings/settingsCache.js'
 import { validateSettingsFileContent } from '../utils/settings/validation.js'

 const validCommandHook = {
@@ -120,7 +130,7 @@ describe('settings diagnostics for unsupported command hook args', () => {
    hooks: { UserPromptSubmit: [{ hooks: [hook] }] },
  })

-  test('settings loading preserves the explanation and rejects the file', () => {
+  test('settings loading skips the hook and keeps the explanation', () => {
    const fixtureDir = mkdtempSync(join(tmpdir(), 'hook-args-diagnostic-'))
    const settingsPath = join(fixtureDir, 'settings.json')
    try {
@@ -128,9 +138,10 @@ describe('settings diagnostics for unsupported command hook args', () => {
        type: 'command', command: 'sh', args: ['-c', 'cat'],
      }))
      const result = parseSettingsFile(settingsPath)
-      expect(result.settings).toBeNull()
+      expect(result.settings).toEqual({})
      expect(result.errors).toHaveLength(1)
      expect(result.errors[0]?.path).toBe('hooks.UserPromptSubmit.0.hooks.0.args')
+      expect(result.errors[0]?.message).toContain('Command hook was skipped')
      expect(result.errors[0]?.message).toContain('do not support an "args" field')
      expect(result.errors[0]?.message).toContain('complete invocation in "command"')
    } finally {
@@ -138,6 +149,112 @@ describe('settings diagnostics for unsupported command hook args', () => {
    }
  })

+  test('one hook with args does not discard the rest of the settings file', () => {
+    const fixtureDir = mkdtempSync(join(tmpdir(), 'hook-args-isolation-'))
+    const settingsPath = join(fixtureDir, 'settings.json')
+    try {
+      writeFileSync(settingsPath, JSON.stringify({
+        permissions: { deny: ['Bash(rm -rf:*)'] },
+        env: { OPENAI_MODEL: 'gpt-4o' },
+        hooks: {
+          PreToolUse: [{
+            matcher: 'Bash',
+            hooks: [
+              { type: 'command', command: '~/bin/notify.sh', args: [] },
+              validCommandHook,
+            ],
+          }],
+          UserPromptSubmit: [{ hooks: [validCommandHook] }],
+        },
+      }))
+      const result = parseSettingsFile(settingsPath)
+      expect(result.settings?.permissions?.deny).toEqual(['Bash(rm -rf:*)'])
+      expect(result.settings?.env).toEqual({ OPENAI_MODEL: 'gpt-4o' })
+      expect(result.settings?.hooks?.PreToolUse?.[0]?.hooks).toEqual([validCommandHook])
+      expect(result.settings?.hooks?.UserPromptSubmit?.[0]?.hooks).toEqual([validCommandHook])
+      expect(result.errors.map(error => error.path)).toEqual([
+        'hooks.PreToolUse.0.hooks.0.args',
+      ])
+    } finally {
+      rmSync(fixtureDir, { recursive: true, force: true })
+    }
+  })
+
+  test('managed settings from a command skip the hook and keep the rest', () => {
+    const { settings, errors } = parseCommandOutputAsSettings(
+      JSON.stringify({
+        permissions: { deny: ['WebFetch'] },
+        hooks: {
+          UserPromptSubmit: [{
+            hooks: [{ type: 'command', command: 'sh', args: ['-c', 'cat'] }],
+          }],
+        },
+      }),
+      'managed-settings',
+    )
+    expect(settings).toEqual({ permissions: { deny: ['WebFetch'] } })
+    expect(errors.map(error => error.path)).toEqual([
+      'hooks.UserPromptSubmit.0.hooks.0.args',
+    ])
+  })
+
+  test('skipping a hook removes only the structures it empties', () => {
+    const kept = { type: 'command' as const, command: 'echo kept' }
+    const skipped = { type: 'command', command: 'sh', args: ['-c', 'cat'] }
+    const { settings } = parseCommandOutputAsSettings(
+      JSON.stringify({
+        hooks: {
+          PreToolUse: [
+            { matcher: 'Bash', hooks: [skipped] },
+            { matcher: 'Edit', hooks: [skipped, kept] },
+            { matcher: 'Read', hooks: [] },
+          ],
+          Stop: [{ hooks: [skipped] }],
+        },
+      }),
+      'managed-settings',
+    )
+    expect(settings.hooks).toEqual({
+      PreToolUse: [
+        { matcher: 'Edit', hooks: [kept] },
+        { matcher: 'Read', hooks: [] },
+      ],
+    })
+  })
+
+  test('a policy holding only a skipped hook does not outrank a lower-priority policy', () => {
+    const mdm = parseCommandOutputAsSettings(
+      JSON.stringify({
+        hooks: {
+          PreToolUse: [{
+            matcher: 'Bash',
+            hooks: [{ type: 'command', command: 'sh', args: ['-c', 'cat'] }],
+          }],
+        },
+      }),
+      'mdm',
+    )
+    expect(mdm.settings).toEqual({})
+
+    const hkcu = parseCommandOutputAsSettings(
+      JSON.stringify({ permissions: { deny: ['WebFetch'] } }),
+      'hkcu',
+    )
+    try {
+      setMdmSettingsCache(mdm, hkcu)
+      resetSettingsCache()
+      // A managed settings file outranks HKCU, so expect it when this machine
+      // has one; either way the MDM source must not win.
+      const managedFile = loadManagedFileSettings().settings
+      expect(getSettingsForSource('policySettings')).toEqual(
+        managedFile ?? hkcu.settings,
+      )
+    } finally {
+      clearMdmSettingsCache()
+      resetSettingsCache()
+    }
+  })
+
  test('settings edit validation explains every supplied JSON args value', () => {
    for (const args of [null, [], ['--flag'], '', 'value', 0, false, {}]) {
      const result = validateSettingsFileContent(settingsContent({
@@ -26,6 +26,13 @@ const IfConditionSchema = lazySchema(() =>
    ),
 )

+/** Explanation shown wherever a command hook's unsupported `args` is reported. */
+export const COMMAND_HOOK_ARGS_UNSUPPORTED_MESSAGE =
+  'Command hooks do not support an "args" field. Put the complete ' +
+  'invocation in "command" (e.g. "command": "sh -c \'printf HANDLER_RAN; cat\'"); ' +
+  'a supplied "args" would otherwise be silently discarded and a ' +
+  'different command would run.'
+
 // Internal factory for individual hook schemas (shared between exported
 // discriminated union members and the HookCommandSchema factory)
 function buildHookSchemas() {
@@ -42,14 +49,11 @@ function buildHookSchemas() {
    // undefined, so `.optional()` only admits a programmatically-constructed
    // explicit `undefined`, which means "not supplied" — every JSON-parsed
    // value (null, array, string, …) is rejected.
+    // Settings files drop just the offending hook before validation
+    // (filterUnsupportedCommandHookArgs), so one bad hook can't discard the
+    // rest of the file, including its permission deny rules.
    args: z
-      .never({
-        error:
-          'Command hooks do not support an "args" field. Put the complete ' +
-          'invocation in "command" (e.g. "command": "sh -c \'printf HANDLER_RAN; cat\'"); ' +
-          'a supplied "args" would otherwise be silently discarded and a ' +
-          'different command would run.',
-      })
+      .never({ error: COMMAND_HOOK_ARGS_UNSUPPORTED_MESSAGE })
      .optional()
      .describe(
        'Not supported: put the complete invocation in "command" instead',
@@ -648,6 +648,28 @@ describe('LSP lifecycle notification failures', () => {
      await open.catch(() => {})
    }
  })
+
+  test('the default timeout keeps a server that is busy for a few seconds', async () => {
+    jest.useFakeTimers()
+    const { manager, controls } = await createManager()
+    const server = controls.get('typescript')!
+    const gate = server.blockNextNotification('textDocument/didOpen')
+    const open = manager.openFile('/repo/src/busy-open.ts', 'contents')
+    await gate.started
+
+    try {
+      jest.advanceTimersByTime(5_000)
+      gate.release()
+      await expect(open).resolves.toBeUndefined()
+      expect(server.stopCalls).not.toHaveBeenCalled()
+      expect(server.server.state).toBe('running')
+    } finally {
+      gate.release()
+      jest.runAllTimers()
+      jest.useRealTimers()
+      await open.catch(() => {})
+    }
+  })
 })

 describe('LSP generation resynchronization', () => {
@@ -116,7 +116,12 @@ export type LSPServerManagerDependencies = {
  lifecycleNotificationTimeoutMs: number
 }

-const DEFAULT_LSP_LIFECYCLE_NOTIFICATION_TIMEOUT_MS = 2_000
+// A timed-out didOpen/didChange/didClose stops the server so its document
+// versions resynchronize on restart. A healthy server can stop reading stdin
+// for several seconds while indexing, and a full-document write then waits on
+// the pipe, so only treat it as wedged after the same budget as startup
+// (LSPServerInstance's DEFAULT_LSP_STARTUP_TIMEOUT_MS).
+const DEFAULT_LSP_LIFECYCLE_NOTIFICATION_TIMEOUT_MS = 30_000

 const DEFAULT_DEPENDENCIES: LSPServerManagerDependencies = {
  loadServerConfigs: getAllLspServers,
@@ -113,6 +113,7 @@ import {
 import { buildMcpToolName } from './mcpStringUtils.js'
 import { normalizeNameForMCP } from './normalization.js'
 import { listAllMcpResources, paginateMcpList } from './pagination.js'
+import { createMcpStdioClientTransport } from './stdioTransport.js'
 import { getLoggingSafeMcpBaseUrl } from './utils.js'

 /* eslint-disable @typescript-eslint/no-require-imports */
@@ -1199,7 +1200,7 @@ export const connectToServer = memoize(
          serverRef.args ?? [],
          process.env.CLAUDE_CODE_SHELL_PREFIX,
        )
-        transport = new StdioClientTransport({
+        transport = createMcpStdioClientTransport({
          command: finalCommand,
          args: finalArgs,
          env: {
@@ -0,0 +1,223 @@
+import { describe, expect, test } from 'bun:test'
+import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'
+import { ReadBuffer } from '@modelcontextprotocol/sdk/shared/stdio.js'
+import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js'
+import {
+  MCP_STDIO_MAX_BUFFER_BYTES,
+  McpStdioReadBuffer,
+  createMcpStdioClientTransport,
+  createMcpStdioServerTransport,
+} from './stdioTransport.js'
+
+const PAYLOAD_BYTES = 12 * 1024 * 1024
+const PIPE_CHUNK_BYTES = 64 * 1024
+
+// A stdio "server" that writes one 12 MiB JSON-RPC notification (above the
+// SDK's 10 MiB default) and exits once it is flushed.
+const SERVER_SCRIPT = `
+const message = JSON.stringify({
+  jsonrpc: '2.0',
+  method: 'notifications/message',
+  params: { level: 'info', data: 'x'.repeat(${PAYLOAD_BYTES}) },
+}) + '\\n'
+process.stdout.write(message, () => process.exit(0))
+`
+
+type FirstEvent = { message?: JSONRPCMessage; error?: Error }
+
+async function firstEvent(transport: StdioClientTransport): Promise<FirstEvent> {
+  try {
+    return await new Promise<FirstEvent>((resolve, reject) => {
+      transport.onmessage = message => resolve({ message })
+      transport.onerror = error => resolve({ error })
+      transport.onclose = () =>
+        reject(new Error('stdio server closed before sending a message'))
+      transport.start().catch(reject)
+    })
+  } finally {
+    await transport.close()
+  }
+}
+
+const serverParams = {
+  command: process.execPath,
+  args: ['-e', SERVER_SCRIPT],
+  stderr: 'pipe' as const,
+}
+
+function notification(data: string): Buffer {
+  return Buffer.from(
+    `${JSON.stringify({
+      jsonrpc: '2.0',
+      method: 'notifications/message',
+      params: { level: 'info', data },
+    })}\n`,
+  )
+}
+
+function chunksOf(bytes: Buffer, size: number): Buffer[] {
+  const chunks: Buffer[] = []
+  for (let offset = 0; offset < bytes.length; offset += size) {
+    chunks.push(bytes.subarray(offset, offset + size))
+  }
+  return chunks
+}
+
+type ReadOutcome = { message: JSONRPCMessage } | { error: string }
+
+/** Feeds chunks the way the SDK transports do: append, then drain. */
+function readAll(
+  buffer: Pick<ReadBuffer, 'append' | 'readMessage'>,
+  chunks: Buffer[],
+): ReadOutcome[] {
+  const outcomes: ReadOutcome[] = []
+  for (const chunk of chunks) {
+    try {
+      buffer.append(chunk)
+    } catch (error) {
+      outcomes.push({ error: (error as Error).message })
+      continue
+    }
+    while (true) {
+      try {
+        const message = buffer.readMessage()
+        if (message === null) break
+        outcomes.push({ message })
+      } catch (error) {
+        outcomes.push({ error: (error as Error).message })
+      }
+    }
+  }
+  return outcomes
+}
+
+describe('MCP stdio client transport', () => {
+  test('the limit is above the 10 MiB SDK default', () => {
+    expect(MCP_STDIO_MAX_BUFFER_BYTES).toBeGreaterThan(PAYLOAD_BYTES)
+  })
+
+  test('both transports read through the linear-time buffer', () => {
+    const client = createMcpStdioClientTransport(serverParams)
+    const server = createMcpStdioServerTransport()
+    for (const transport of [client, server]) {
+      expect(
+        (transport as unknown as { _readBuffer: unknown })._readBuffer,
+      ).toBeInstanceOf(McpStdioReadBuffer)
+    }
+  })
+
+  test('the SDK default drops a 12 MiB message', async () => {
+    const { message, error } = await firstEvent(new StdioClientTransport(serverParams))
+    expect(message).toBeUndefined()
+    expect(error?.message).toContain('ReadBuffer exceeded maximum size')
+  })
+
+  test('delivers a 12 MiB message from a stdio server', async () => {
+    const { message, error } = await firstEvent(
+      createMcpStdioClientTransport(serverParams),
+    )
+    expect(error).toBeUndefined()
+    expect(message).toMatchObject({ method: 'notifications/message' })
+    const data = (message as { params?: { data?: string } } | undefined)
+      ?.params?.data
+    expect(data?.length).toBe(PAYLOAD_BYTES)
+  })
+})
+
+describe('McpStdioReadBuffer', () => {
+  test('matches the SDK buffer for split, joined, CRLF, UTF-8 and invalid lines', () => {
+    const stream = Buffer.concat([
+      notification('first'),
+      notification('日本語 ✓ split across chunks'),
+      Buffer.from('{"jsonrpc":"2.0","id":1,"result":{}}\r\n'),
+      Buffer.from('not json\n'),
+      notification('after the invalid line'),
+      Buffer.from('{"jsonrpc":"2.0","method":"partial"'),
+    ])
+    for (const size of [1, 2, 3, 5, 7, 64, stream.length]) {
+      const chunks = chunksOf(stream, size)
+      const ours = readAll(new McpStdioReadBuffer(), chunks)
+      expect(ours).toEqual(readAll(new ReadBuffer(), chunks))
+      expect(ours.filter(outcome => 'message' in outcome)).toHaveLength(4)
+    }
+  })
+
+  test('accepts a message at the limit and rejects one byte more, like the SDK', () => {
+    const message = notification('x'.repeat(1000))
+    const chunks = chunksOf(message, 100)
+
+    const atLimit = readAll(new McpStdioReadBuffer(message.length), chunks)
+    expect(atLimit).toEqual(
+      readAll(new ReadBuffer({ maxBufferSize: message.length }), chunks),
+    )
+    expect(atLimit).toEqual([{ message: expect.anything() }])
+
+    const overLimit = new McpStdioReadBuffer(message.length - 1)
+    const outcomes = readAll(overLimit, chunks)
+    expect(outcomes).toEqual(
+      readAll(new ReadBuffer({ maxBufferSize: message.length - 1 }), chunks),
+    )
+    expect(outcomes).toEqual([
+      { error: `ReadBuffer exceeded maximum size of ${message.length - 1} bytes` },
+    ])
+    expect(overLimit.bufferedBytes).toBe(0)
+  })
+
+  test('reads a large chunked message with linear copying and scanning', () => {
+    const message = notification('x'.repeat(32 * 1024 * 1024))
+    const chunks = chunksOf(message, PIPE_CHUNK_BYTES)
+
+    const originalConcat = Buffer.concat
+    const originalIndexOf = Buffer.prototype.indexOf
+    let copiedBytes = 0
+    let scannedBytes = 0
+    Buffer.concat = function (list, totalLength) {
+      const result = originalConcat.call(Buffer, list, totalLength)
+      copiedBytes += result.length
+      return result
+    } as typeof Buffer.concat
+    Buffer.prototype.indexOf = function (
+      this: Buffer,
+      value: Parameters<Buffer['indexOf']>[0],
+      byteOffset?: number,
+      encoding?: BufferEncoding,
+    ) {
+      const index = originalIndexOf.call(this, value, byteOffset, encoding)
+      const start = byteOffset ?? 0
+      scannedBytes += (index === -1 ? this.length : index + 1) - start
+      return index
+    } as Buffer['indexOf']
+
+    const buffer = new McpStdioReadBuffer()
+    let outcomes: ReadOutcome[]
+    try {
+      outcomes = readAll(buffer, chunks)
+    } finally {
+      Buffer.concat = originalConcat
+      Buffer.prototype.indexOf = originalIndexOf
+    }
+
+    expect(outcomes).toHaveLength(1)
+    expect(outcomes[0]).toHaveProperty('message.params.data.length', 32 * 1024 * 1024)
+    // Each byte is scanned once and the line is copied once. The SDK buffer
+    // copies and scans the pending bytes again for every chunk, about 256
+    // times the message size here.
+    expect(scannedBytes).toBe(message.length)
+    expect(copiedBytes).toBe(message.length - 1)
+    expect(buffer.bufferedBytes).toBe(0)
+  })
+
+  test('keeps only the unread remainder after a message', () => {
+    const buffer = new McpStdioReadBuffer()
+    const first = notification('first')
+    buffer.append(Buffer.concat([first, Buffer.from('{"jsonrpc"')]))
+    expect(buffer.readMessage()).toMatchObject({ params: { data: 'first' } })
+    expect(buffer.readMessage()).toBeNull()
+    expect(buffer.bufferedBytes).toBe('{"jsonrpc"'.length)
+
+    buffer.clear()
+    buffer.append(first)
+    expect(buffer.readMessage()).toMatchObject({ params: { data: 'first' } })
+    expect(buffer.bufferedBytes).toBe(0)
+  })
+})
@@ -0,0 +1,140 @@
+import {
+  StdioClientTransport,
+  type StdioServerParameters,
+} from '@modelcontextprotocol/sdk/client/stdio.js'
+import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
+import { deserializeMessage } from '@modelcontextprotocol/sdk/shared/stdio.js'
+import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js'
+
+/**
+ * Largest single stdio MCP message we buffer. MCP SDK 1.32 added a 10 MiB
+ * default and closes the connection when one message exceeds it, which kills
+ * servers that return a base64 screenshot, file or large query result in one
+ * tool call (SDK 1.29 had no limit). Oversized results are still truncated
+ * before they reach the model.
+ */
+export const MCP_STDIO_MAX_BUFFER_BYTES = 128 * 1024 * 1024
+
+const NEWLINE = 0x0a
+
+/**
+ * Splits a stdio byte stream into JSON-RPC messages in linear time.
+ *
+ * The SDK's `ReadBuffer` (1.32.1) concatenates the whole pending buffer on
+ * every chunk and searches it from the start, so an n-byte message costs
+ * O(n²): a 100 MiB message in 64 KiB pipe chunks blocks the event loop for
+ * about 10 seconds. It also keeps the remainder of a parsed message as a
+ * `subarray`, which holds the whole allocation alive even when it is empty.
+ *
+ * This buffer keeps the chunks in a list, looks at each byte once when
+ * searching for a newline, copies a message once when it is complete, and
+ * never keeps an empty remainder. Limit, error and parsing behaviour match the
+ * SDK's buffer.
+ */
+export class McpStdioReadBuffer {
+  private chunks: Buffer[] = []
+  private size = 0
+  // Where the newline search resumes. Bytes before it hold no newline.
+  private scanChunk = 0
+  private scanOffset = 0
+
+  constructor(private readonly maxBufferSize = MCP_STDIO_MAX_BUFFER_BYTES) {}
+
+  get bufferedBytes(): number {
+    return this.size
+  }
+
+  append(chunk: Buffer): void {
+    if (this.size + chunk.length > this.maxBufferSize) {
+      this.clear()
+      throw new Error(
+        `ReadBuffer exceeded maximum size of ${this.maxBufferSize} bytes`,
+      )
+    }
+    if (chunk.length === 0) return
+    this.chunks.push(chunk)
+    this.size += chunk.length
+  }
+
+  readMessage(): JSONRPCMessage | null {
+    while (this.scanChunk < this.chunks.length) {
+      const chunk = this.chunks[this.scanChunk]!
+      const newline = chunk.indexOf(NEWLINE, this.scanOffset)
+      if (newline === -1) {
+        this.scanChunk++
+        this.scanOffset = 0
+        continue
+      }
+      return deserializeMessage(this.takeLine(this.scanChunk, newline))
+    }
+    return null
+  }
+
+  clear(): void {
+    this.chunks = []
+    this.size = 0
+    this.scanChunk = 0
+    this.scanOffset = 0
+  }
+
+  /** Removes the bytes up to and including the newline and decodes them. */
+  private takeLine(lastChunk: number, newline: number): string {
+    const last = this.chunks[lastChunk]!
+    const parts = this.chunks.slice(0, lastChunk)
+    parts.push(last.subarray(0, newline))
+    const lineBytes = parts.length === 1 ? parts[0]! : Buffer.concat(parts)
+
+    const rest = last.subarray(newline + 1)
+    const remaining = this.chunks.slice(lastChunk + 1)
+    if (rest.length > 0) remaining.unshift(rest)
+    this.chunks = remaining
+    this.size -= lineBytes.length + 1
+    this.scanChunk = 0
+    this.scanOffset = 0
+
+    return lineBytes.toString('utf8').replace(/\r$/, '')
+  }
+}
+
+type ReadBufferHost = { _readBuffer?: unknown }
+
+function isReadBuffer(value: unknown): boolean {
+  if (!value || typeof value !== 'object') return false
+  const buffer = value as Record<string, unknown>
+  return (
+    typeof buffer.append === 'function' &&
+    typeof buffer.readMessage === 'function' &&
+    typeof buffer.clear === 'function'
+  )
+}
+
+/**
+ * Swaps the SDK transport's private `_readBuffer` for McpStdioReadBuffer.
+ * SDK 1.32.1 only calls `append`, `readMessage` and `clear` on it. If a later
+ * SDK (possible for SDK embedders, who bring their own) no longer has the
+ * field, the SDK's own buffer stays, still with the raised size limit.
+ */
+function useLinearReadBuffer(transport: object, maxBufferSize: number): void {
+  const host = transport as ReadBufferHost
+  if (!isReadBuffer(host._readBuffer)) return
+  host._readBuffer = new McpStdioReadBuffer(maxBufferSize)
+}
+
+/** Creates the stdio client transport for a configured MCP server. */
+export function createMcpStdioClientTransport(
+  params: StdioServerParameters,
+): StdioClientTransport {
+  const maxBufferSize = params.maxBufferSize ?? MCP_STDIO_MAX_BUFFER_BYTES
+  const transport = new StdioClientTransport({ ...params, maxBufferSize })
+  useLinearReadBuffer(transport, maxBufferSize)
+  return transport
+}
+
+/** Creates the stdio server transport for `openclaude mcp serve`. */
+export function createMcpStdioServerTransport(): StdioServerTransport {
+  const transport = new StdioServerTransport(undefined, undefined, {
+    maxBufferSize: MCP_STDIO_MAX_BUFFER_BYTES,
+  })
+  useLinearReadBuffer(transport, MCP_STDIO_MAX_BUFFER_BYTES)
+  return transport
+}
@@ -2,12 +2,28 @@
 * These tests avoid static imports so Bun can mock secureStorage before
 * codexCredentials is first loaded.
 */
-import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
+import { afterAll, afterEach, beforeEach, describe, expect, mock, test } from 'bun:test'
 import {
  acquireSharedMutationLock,
  releaseSharedMutationLock,
 } from '../test/sharedMutationLock.js'

+// mock.restore() does not undo mock.module(), so put the real storage modules
+// back once this file is done. Otherwise a later file in the same process
+// (e.g. src/entrypoints/sdk/mcpNeedsAuth.test.ts) reads the stubs below.
+const nonce = `${Date.now()}-${Math.random()}`
+const realPlainTextStorage = {
+  ...await import(`./secureStorage/plainTextStorage.js?real=${nonce}`),
+}
+const realSecureStorage = {
+  ...await import(`./secureStorage/index.js?real=${nonce}`),
+}
+
+afterAll(() => {
+  mock.module('./secureStorage/plainTextStorage.js', () => realPlainTextStorage)
+  mock.module('./secureStorage/index.js', () => realSecureStorage)
+})
+
 type CodexCredentialsModule = typeof import('./codexCredentials.js')

 function importFreshCodexCredentials(
@@ -208,7 +208,12 @@ async function importActualProviders(): Promise<ProvidersModule> {
 }

 async function prepareFastModeTestState(): Promise<void> {
-  const { setIsInteractive } = await import('../bootstrap/state.js')
+  const { resetModelStringsForTestingOnly, setIsInteractive } = await import(
+    '../bootstrap/state.js'
+  )
+  // Model strings are cached process-wide. An earlier suite may have cached
+  // them for another provider (OpenAI's map turns "opus" into gpt-4o).
+  resetModelStringsForTestingOnly()
  setIsInteractive(true)
  const { DEFAULT_GLOBAL_CONFIG, _setGlobalConfigCacheForTesting } =
    await import('./config.js')
@@ -24,6 +24,7 @@ import { logForDiagnosticsNoPII } from '../../diagLogs.js'
 import { readFileSync } from '../../fileRead.js'
 import { getFsImplementation } from '../../fsOperations.js'
 import { safeParseJSON } from '../../json.js'
+import { clone } from '../../slowOperations.js'
 import { profileCheckpoint } from '../../startupProfiler.js'
 import {
  getManagedFilePath,
@@ -32,6 +33,7 @@ import {
 import { type SettingsJson, SettingsSchema } from '../types.js'
 import {
  filterInvalidPermissionRules,
+  filterUnsupportedCommandHookArgs,
  formatZodError,
  type ValidationError,
 } from '../validation.js'
@@ -206,12 +208,17 @@ export function parseCommandOutputAsSettings(
  stdout: string,
  sourcePath: string,
 ): { settings: SettingsJson; errors: ValidationError[] } {
-  const data = safeParseJSON(stdout, false)
+  // Copy the memoized parse result: the filters below edit it in place, and
+  // the 30-minute refresh parses the same output again.
+  const data = clone(safeParseJSON(stdout, false))
  if (!data || typeof data !== 'object') {
    return { settings: {}, errors: [] }
  }

-  const ruleWarnings = filterInvalidPermissionRules(data, sourcePath)
+  const ruleWarnings = [
+    ...filterInvalidPermissionRules(data, sourcePath),
+    ...filterUnsupportedCommandHookArgs(data, sourcePath),
+  ]
  const parseResult = SettingsSchema().safeParse(data)
  if (!parseResult.success) {
    const errors = formatZodError(parseResult.error, sourcePath)
@@ -344,6 +344,38 @@ test('reads the merge base from disk after ownership, not from warm caches', asy
  })
 })

+test('an unrelated update keeps a command hook that runtime loading skips', async () => {
+  await withIsolatedUserSettings((_root, settingsPath) => {
+    const hooks = {
+      PreToolUse: [
+        {
+          matcher: 'Bash',
+          hooks: [
+            { type: 'command', command: 'sh', args: ['-c', 'printf HOOK'] },
+            { type: 'command', command: 'echo kept' },
+          ],
+        },
+      ],
+    }
+    writeFileSync(
+      settingsPath,
+      `${JSON.stringify({ env: { BEFORE: 'yes' }, hooks }, null, 2)}\n`,
+    )
+    expect(
+      getSettingsForSource('userSettings')?.hooks?.PreToolUse?.[0]?.hooks,
+    ).toEqual([{ type: 'command', command: 'echo kept' }])
+
+    expect(
+      updateSettingsForSource('userSettings', { env: { AFTER: 'yes' } }),
+    ).toEqual({ error: null })
+
+    expect(JSON.parse(readFileSync(settingsPath, 'utf8'))).toEqual({
+      env: { BEFORE: 'yes', AFTER: 'yes' },
+      hooks,
+    })
+  })
+})
+
 test('creates a missing settings file without weakening the synchronous result', async () => {
  await withIsolatedUserSettings((_root, settingsPath) => {
    expect(existsSync(settingsPath)).toBe(false)
@@ -50,6 +50,7 @@ import { type SettingsJson, SettingsSchema } from './types.js'
 import {
  filterInvalidModelPricing,
  filterInvalidPermissionRules,
+  filterUnsupportedCommandHookArgs,
  formatZodError,
  type SettingsWithErrors,
  type ValidationError,
@@ -201,7 +202,10 @@ export function parseSettingsFile(path: string): {
  }
 }

-function parseSettingsFileUncached(path: string): {
+function parseSettingsFileUncached(
+  path: string,
+  { keepUnsupportedHooks = false }: { keepUnsupportedHooks?: boolean } = {},
+): {
  settings: SettingsJson | null
  errors: ValidationError[]
 } {
@@ -213,11 +217,19 @@ function parseSettingsFileUncached(path: string): {
      return { settings: {}, errors: [] }
    }

-    const data = safeParseJSON(content, false)
+    // safeParseJSON memoizes by content and returns the shared object. The
+    // filters below edit it in place, so work on a copy. Otherwise a later
+    // parse of the same content would see an already filtered object,
+    // losing both the warnings and the write-back copy of a skipped hook.
+    const data = clone(safeParseJSON(content, false))

-    // Filter invalid permission rules before schema validation so one bad
-    // rule doesn't cause the entire settings file to be rejected.
+    // Filter invalid permission rules and unsupported hook fields before
+    // schema validation so one bad entry doesn't cause the entire settings
+    // file to be rejected.
    const ruleWarnings = filterInvalidPermissionRules(data, path)
+    const hookWarnings = keepUnsupportedHooks
+      ? []
+      : filterUnsupportedCommandHookArgs(data, path)
    const modelPricingWarnings = filterInvalidModelPricing(data, path)

    const result = SettingsSchema().safeParse(data)
@@ -226,13 +238,18 @@ function parseSettingsFileUncached(path: string): {
      const errors = formatZodError(result.error, path)
      return {
        settings: null,
-        errors: [...ruleWarnings, ...modelPricingWarnings, ...errors],
+        errors: [
+          ...ruleWarnings,
+          ...hookWarnings,
+          ...modelPricingWarnings,
+          ...errors,
+        ],
      }
    }

    return {
      settings: result.data,
-      errors: [...ruleWarnings, ...modelPricingWarnings],
+      errors: [...ruleWarnings, ...hookWarnings, ...modelPricingWarnings],
    }
  } catch (error) {
    handleFileSystemError(error, path)
@@ -446,7 +463,12 @@ export function updateSettingsForSource(
      targetPath => {
        // The transaction merge base must bypass both process-local settings
        // caches so a peer's completed update cannot be overwritten.
-        let existingSettings = parseSettingsFileUncached(targetPath).settings
+        // Skipping a command hook with `args` is a runtime decision. Keeping
+        // it here sends the file through the raw fallback below, so an
+        // unrelated update writes that hook back unchanged.
+        let existingSettings = parseSettingsFileUncached(targetPath, {
+          keepUnsupportedHooks: true,
+        }).settings

        // If validation failed, distinguish syntax errors from schema-invalid
        // objects whose unknown fields still need to survive the merge.
@@ -1,5 +1,6 @@
 import type { ConfigScope } from 'src/services/mcp/types.js'
 import { type ZodError, type ZodIssue, z } from 'zod/v4'
+import { COMMAND_HOOK_ARGS_UNSUPPORTED_MESSAGE } from '../../schemas/hooks.js'
 import { jsonParse } from '../slowOperations.js'
 import { plural } from '../stringUtils.js'
 import { validatePermissionRule } from './permissionValidation.js'
@@ -269,6 +270,68 @@ export function filterInvalidPermissionRules(
  return warnings
 }

+/**
+ * Removes command hooks that supply the unsupported `args` field from raw
+ * parsed JSON before schema validation. The hook is skipped (it never runs
+ * with a different command than intended), and the rest of the file, including
+ * its permission rules, still loads. Returns a warning for each removed hook.
+ *
+ * Matchers, events and the `hooks` object that end up empty because of this
+ * filter are removed too. Otherwise a managed source that held nothing but a
+ * skipped hook would still count as non-empty and win first-source-wins
+ * policy selection over a lower-priority source with real rules.
+ */
+export function filterUnsupportedCommandHookArgs(
+  data: unknown,
+  filePath: string,
+): ValidationError[] {
+  if (!data || typeof data !== 'object') return []
+  const root = data as Record<string, unknown>
+  const hooks = root.hooks
+  if (!hooks || typeof hooks !== 'object' || Array.isArray(hooks)) return []
+  const hooksByEvent = hooks as Record<string, unknown>
+
+  const warnings: ValidationError[] = []
+  for (const [event, matchers] of Object.entries(hooksByEvent)) {
+    if (!Array.isArray(matchers)) continue
+    const keptMatchers = matchers.filter((matcher: unknown, matcherIndex) => {
+      if (!matcher || typeof matcher !== 'object') return true
+      const config = matcher as Record<string, unknown>
+      if (!Array.isArray(config.hooks)) return true
+      const hookCount = config.hooks.length
+      const keptHooks = config.hooks.filter((hook: unknown, hookIndex) => {
+        if (
+          !hook ||
+          typeof hook !== 'object' ||
+          (hook as Record<string, unknown>).type !== 'command' ||
+          (hook as Record<string, unknown>).args === undefined
+        ) {
+          return true
+        }
+        warnings.push({
+          file: filePath,
+          path: `hooks.${event}.${matcherIndex}.hooks.${hookIndex}.args`,
+          message: `Command hook was skipped. ${COMMAND_HOOK_ARGS_UNSUPPORTED_MESSAGE}`,
+          invalidValue: (hook as Record<string, unknown>).args,
+        })
+        return false
+      })
+      config.hooks = keptHooks
+      return hookCount === 0 || keptHooks.length > 0
+    })
+    if (keptMatchers.length === matchers.length) continue
+    if (keptMatchers.length > 0) {
+      hooksByEvent[event] = keptMatchers
+    } else {
+      delete hooksByEvent[event]
+    }
+  }
+  if (warnings.length > 0 && Object.keys(hooksByEvent).length === 0) {
+    delete root.hooks
+  }
+  return warnings
+}
+
 export function filterInvalidModelPricing(
  data: unknown,
  filePath: string,