فهرست منبع

refactor(subagent): simplify Claude cleanup ownership

pku-xht 3 هفته پیش
والد
کامیت
bf2e9e474c

+ 11 - 15
packages/subagent/subagent-claude-code/src/run.ts

@@ -262,7 +262,8 @@ export async function consumeClaudeQuery(
  * Close the official query, terminate the managed process tree, and wait for
  * the subprocess owner to prove it is gone.
  * @param query - official SDK query, when creation reached that point.
- * @param child - shared-service handle that owns the CLI process tree.
+ * @param child - live shared-service handle that owns the CLI process tree;
+ * spawn-failed handles settle at the startup boundary instead.
  */
 export async function disposeClaudeCodeChild(
   query: Pick<Query, 'close'> | undefined,
@@ -276,13 +277,11 @@ export async function disposeClaudeCodeChild(
     failures.push(thrown(error))
   }
 
-  if (child.pid > 0) {
-    child.terminate()
-    try {
-      await child.waitForExit()
-    } catch (error: unknown) {
-      failures.push(thrown(error))
-    }
+  child.terminate()
+  try {
+    await child.waitForExit()
+  } catch (error: unknown) {
+    failures.push(thrown(error))
   }
   try {
     outcome = await child.done
@@ -297,13 +296,10 @@ export async function disposeClaudeCodeChild(
       category: 'unknown',
       outcome,
     } as const
-    if (failures.length === 1) {
-      throw new ClaudeCodeFailure(facts, firstFailure)
-    }
-    throw new AggregateError(
-      failures.map(failure => new ClaudeCodeFailure(facts, failure)),
-      `subagent-claude-code: ${failureDiagnostic(facts)}`,
-    )
+    const cause = failures.length === 1
+      ? firstFailure
+      : new AggregateError(failures, 'Claude Code teardown failures')
+    throw new ClaudeCodeFailure(facts, cause)
   }
 }
 

+ 0 - 3
packages/subagent/subagent-claude-code/tests/real-product.spec.ts

@@ -322,9 +322,6 @@ describe('real Claude Agent SDK 0.3.220 and its distributed Claude Code 2.1.220
       .toBe(process.platform === 'win32'
         ? realpathSync(claudeBin).toLowerCase()
         : realpathSync(claudeBin))
-    expect(harness.spawnSpecs[0]?.env)
-      .not.toHaveProperty('DSH_CLAUDE_CODE_EXECUTABLE')
-
     expect(fixture.requests).toHaveLength(1)
     const recorded = fixture.requests[0]!
     expect(recorded.method).toBe('POST')

+ 19 - 18
packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts

@@ -451,8 +451,6 @@ describe('task admission and package contracts', () => {
     const safeChild = fakeChild()
     const bypassChild = fakeChild()
     const spawnSpecs: SubprocessSpawnSpec[] = []
-    vi.spyOn(ctx.subprocess, 'resolveExecutable')
-      .mockResolvedValue('/native/claude')
     vi.spyOn(ctx.subprocess, 'spawn').mockImplementation((spec) => {
       spawnSpecs.push(spec)
       return spec.env?.DSH_CLAUDE_INSTANCE === 'safe'
@@ -463,7 +461,6 @@ describe('task admission and package contracts', () => {
     queryMock.mockImplementation(({ options }) => {
       queryOptions.push(options)
       options.spawnClaudeCodeProcess!(sdkSpawnOptions({
-        command: options.pathToClaudeCodeExecutable!,
         cwd: options.cwd!,
         env: options.env!,
         signal: options.abortController!.signal,
@@ -770,7 +767,6 @@ describe('official spawn projection', () => {
     expect(spec.argv).toEqual([
       command, '--output-format', 'stream-json',
     ])
-    expect(spec.env).not.toHaveProperty('DSH_CLAUDE_CODE_EXECUTABLE')
   })
 
   it('projects streams, exit facts, listeners, and idempotent tree termination', async () => {
@@ -1629,28 +1625,33 @@ describe('query and process disposal', () => {
       'unknown',
       { exitCode: 0, signal: null },
     ))
-    await expect(waitAndClose).rejects.toBeInstanceOf(AggregateError)
+    const waitAndCloseError = await waitAndClose.then(
+      () => undefined,
+      (error: unknown) => error,
+    )
+    const waitAndCloseCause = errorCause(waitAndCloseError)
+    expect(waitAndCloseCause).toBeInstanceOf(AggregateError)
+    expect((waitAndCloseCause as AggregateError).errors).toEqual([
+      expect.objectContaining({ message: 'close boom' }),
+      expect.objectContaining({ message: 'wait boom' }),
+    ])
     expect(waitFailure.terminate).toHaveBeenCalledOnce()
 
     const doneFailure = fakeChild({
-      pid: -1,
       doneError: new Error('spawn boom'),
     })
-    await expect(disposeClaudeCodeChild(
+    const directChildFailure = disposeClaudeCodeChild(
       { close: vi.fn() },
       doneFailure.handle,
-    )).rejects.toThrow(expectedFailureDiagnostic('teardown', 'unknown'))
-
-    const both = fakeChild({
-      pid: -1,
-      doneError: new Error('spawn boom'),
-    })
-    const bothFailures = disposeClaudeCodeChild(
-      { close: () => { throw new Error('close boom') } },
-      both.handle,
     )
-    await expect(bothFailures)
+    await expect(directChildFailure)
       .rejects.toThrow(expectedFailureDiagnostic('teardown', 'unknown'))
-    await expect(bothFailures).rejects.toBeInstanceOf(AggregateError)
+    await expect(directChildFailure).rejects.not.toThrow('spawn boom')
+    const directChildError = await directChildFailure.then(
+      () => undefined,
+      (error: unknown) => error,
+    )
+    expect(errorCause(directChildError)?.message).toBe('spawn boom')
+    expect(doneFailure.terminate).toHaveBeenCalledOnce()
   })
 })