Browse Source

fix(subprocess): complete runner resource release

pku-xht 1 tháng trước cách đây
mục cha
commit
e70405847c

+ 14 - 11
packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts

@@ -1505,10 +1505,12 @@ describe('run publication, cancellation, and settlement', () => {
     expect(cancelledFailedSpawnClose).toHaveBeenCalledOnce()
 
     const failedSpawnCloseError = new Error('query close failed')
+    const failedSpawnWaitError = new Error('managed range wait failed')
     const failedSpawnClose = vi.fn(() => { throw failedSpawnCloseError })
     const failedSpawnWithCloseFailure = fakeChild({
       pid: -1,
       doneError: spawnError,
+      waitForExitError: failedSpawnWaitError,
     })
     queryMock.mockImplementationOnce(({ options }) => {
       options.spawnClaudeCodeProcess!(sdkSpawnOptions())
@@ -1518,17 +1520,18 @@ describe('run publication, cancellation, and settlement', () => {
       ...unused.spec,
       spawn: () => failedSpawnWithCloseFailure.handle,
     })
-    await expect(failedWithCloseFailure)
-      .rejects.toThrow(expectedFailureDiagnostic('query-start', 'unknown'))
-    await expect(failedWithCloseFailure)
-      .rejects.not.toThrow('spawn /sdk/claude EACCES')
-    await expect(failedWithCloseFailure).rejects.toMatchObject({
-      message: `subagent-claude-code: ${expectedFailureDiagnostic('query-start', 'unknown')}; subagent-claude-code: ${expectedFailureDiagnostic('teardown', 'unknown')}`,
-      errors: [
-        expect.objectContaining({ cause: spawnError }),
-        expect.objectContaining({ cause: failedSpawnCloseError }),
-      ],
-    })
+    const failedWithCloseError = await failedWithCloseFailure.catch((error: unknown) => error)
+    expect(failedWithCloseError).toBeInstanceOf(AggregateError)
+    expect(String(failedWithCloseError)).toContain(expectedFailureDiagnostic('query-start', 'unknown'))
+    expect(String(failedWithCloseError)).not.toContain('spawn /sdk/claude EACCES')
+    const failures = (failedWithCloseError as AggregateError).errors as unknown[]
+    expect(failures[0]).toMatchObject({ cause: spawnError })
+    const cleanupCause = errorCause(failures[1])
+    expect(cleanupCause).toBeInstanceOf(AggregateError)
+    expect((cleanupCause as AggregateError).errors).toEqual([
+      failedSpawnCloseError,
+      failedSpawnWaitError,
+    ])
 
     const cleanupError = new Error('live child cleanup failed')
     const constructionError = new Error(

+ 4 - 6
packages/subprocess/subprocess-local/src/spawn-runner.ts

@@ -101,15 +101,13 @@ interface MaterializedStdioStream {
   readonly _handle?: { close(): void } | null
 }
 
-function closeMaterializedStdio(stream: NodeJS.WriteStream): void {
-  (stream as unknown as MaterializedStdioStream)._handle?.close()
-}
-
 /** Release the runner's copies after the Windows target inherits its standard handles. */
 function releaseRunnerStdio(): void {
   const stdin = process.stdin
   const stdout = process.stdout
   const stderr = process.stderr
+  const stdoutHandle = (stdout as unknown as MaterializedStdioStream)._handle
+  const stderrHandle = (stderr as unknown as MaterializedStdioStream)._handle
   stdin.destroy()
   for (const fd of [0, 1, 2]) {
     try {
@@ -121,8 +119,8 @@ function releaseRunnerStdio(): void {
   // Node deliberately keeps stdout/stderr alive when destroy() is called. A
   // loader may already have materialized their libuv handles, so close those
   // runner-owned references explicitly; the target keeps its inherited copies.
-  closeMaterializedStdio(stdout)
-  closeMaterializedStdio(stderr)
+  stdoutHandle?.close()
+  stderrHandle?.close()
 }
 
 async function runWin32(request: RunnerRequest, eventsPath: string): Promise<void> {

+ 13 - 1
packages/subprocess/subprocess-local/tests/spawn-runner.spec.ts

@@ -1,6 +1,6 @@
 import { spawn, spawnSync } from 'node:child_process'
 import type { ChildProcess } from 'node:child_process'
-import { existsSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
+import { existsSync, mkdirSync, mkdtempSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { fileURLToPath } from 'node:url'
@@ -127,6 +127,18 @@ describe('spawn runner transport', () => {
     }
   })
 
+  it('contains an unexpected owned-path cleanup failure', () => {
+    const files = createRunnerFiles({ argv: ['node'], cwd: '.', env: {} })
+    rmSync(files.requestPath, { force: true })
+    mkdirSync(files.requestPath)
+    try {
+      expect(() => { cleanupRunnerFiles(files) }).not.toThrow()
+      expect(existsSync(files.directory)).toBe(true)
+    } finally {
+      rmSync(files.directory, { recursive: true, force: true })
+    }
+  })
+
   it.each([
     ['non-object request', null, 'no executable'],
     ['non-array argv', { argv: 'node', cwd: '.', env: {} }, 'no executable'],