Browse Source

Merge Claude failure facts into Codex layer

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

+ 11 - 8
packages/subagent/subagent-claude-code/src/process.ts

@@ -14,6 +14,7 @@ import type {
 import {
   scrubbedParentEnv,
   type SubprocessHandle,
+  type SubprocessOutcome,
   type SubprocessSpawnSpec,
 } from '@deepseek-ai/dsh-subprocess'
 
@@ -81,8 +82,7 @@ export class ManagedClaudeCodeProcess implements SpawnedProcess {
   readonly stdin
   readonly stdout
   private readonly events = new EventEmitter()
-  private exitCodeValue: number | null = null
-  private signalCodeValue: NodeJS.Signals | null = null
+  private outcomeValue: SubprocessOutcome | undefined
   private killRequested = false
 
   /**
@@ -98,8 +98,7 @@ export class ManagedClaudeCodeProcess implements SpawnedProcess {
     this.events.on('error', () => {})
     void child.done.then(
       (outcome) => {
-        this.exitCodeValue = outcome.exitCode
-        this.signalCodeValue = outcome.signal
+        this.outcomeValue = outcome
         this.events.emit('exit', outcome.exitCode, outcome.signal)
       },
       (error: unknown) => {
@@ -115,12 +114,17 @@ export class ManagedClaudeCodeProcess implements SpawnedProcess {
 
   /** Direct-child exit code, or null while running or after signal exit. */
   get exitCode(): number | null {
-    return this.exitCodeValue
+    return this.outcomeValue?.exitCode ?? null
   }
 
   /** Direct-child terminating signal, if any. */
   get signalCode(): NodeJS.Signals | null {
-    return this.signalCodeValue
+    return this.outcomeValue?.signal ?? null
+  }
+
+  /** Exact managed-process outcome after exit, or undefined while running. */
+  get outcome(): SubprocessOutcome | undefined {
+    return this.outcomeValue
   }
 
   /**
@@ -131,8 +135,7 @@ export class ManagedClaudeCodeProcess implements SpawnedProcess {
   kill(_signal: NodeJS.Signals): boolean {
     if (
       this.killRequested
-      || this.exitCodeValue !== null
-      || this.signalCodeValue !== null
+      || this.outcomeValue !== undefined
     ) {
       return false
     }

+ 38 - 36
packages/subagent/subagent-claude-code/src/run.ts

@@ -168,10 +168,6 @@ function thrown(value: unknown): Error {
   /* v8 ignore next -- typed SDK and subprocess failures reject with Error. */
   return value instanceof Error ? value : new Error(String(value))
 }
-
-function isAborted(signal: AbortSignal): boolean {
-  return signal.aborted
-}
 /* jscpd:ignore-end */
 
 /**
@@ -308,14 +304,17 @@ export async function disposeClaudeCodeChild(
  * Build the fixed official SDK options for one one-shot provider run.
  * @param spec - Workspace, environment, process service, and disposal policy.
  * @param controller - per-run cancellation owner.
- * @param capture - receives the real managed child synchronously from the SDK hook.
+ * @param capture - receives the shared child and SDK-facing process synchronously.
  * @param captureDiagnostic - receives safe facts from unattended interaction callbacks.
  * @returns options that inherit native settings while disabling persistence and user questions.
  */
 export function claudeQueryOptions(
   spec: ClaudeCodeRunSpec,
   controller: AbortController,
-  capture: (child: SubprocessHandle) => void,
+  capture: (
+    child: SubprocessHandle,
+    process: ManagedClaudeCodeProcess,
+  ) => void,
   captureDiagnostic: (diagnostic: string) => void,
 ): Options {
   return {
@@ -365,8 +364,9 @@ export function claudeQueryOptions(
     supportedDialogKinds: SUPPORTED_UNATTENDED_DIALOG_KINDS,
     spawnClaudeCodeProcess: (options: SpawnOptions) => {
       const child = spec.spawn(claudeSpawnSpec(options, spec.disposeGraceMs))
-      capture(child)
-      return new ManagedClaudeCodeProcess(child)
+      const process = new ManagedClaudeCodeProcess(child)
+      capture(child, process)
+      return process
     },
   }
 }
@@ -397,21 +397,23 @@ export async function startClaudeCodeRun(
 
   let child: SubprocessHandle | undefined
   let query: Query | undefined
-  let processOutcome: SubprocessOutcome | undefined
-  let failureDetail: string | undefined
-  let permissionDetail: string | undefined
+  let managedProcess: ManagedClaudeCodeProcess | undefined
+  let diagnostic: string | undefined
   const capturePermissionDiagnostic = (value: string): void => {
-    permissionDetail = value
+    diagnostic = value
+  }
+  const prependFailureDiagnostic = (facts: ClaudeCodeFailureFacts): void => {
+    const failure = failureDiagnostic(facts)
+    diagnostic = diagnostic === undefined
+      ? failure
+      : `${failure}\n${diagnostic}`
   }
-  const collectDiagnostic = (): string => [failureDetail, permissionDetail]
-    .filter((value): value is string => value !== undefined)
-    .join('\n')
-  const captureChild = (captured: SubprocessHandle): void => {
+  const captureChild = (
+    captured: SubprocessHandle,
+    process: ManagedClaudeCodeProcess,
+  ): void => {
     child = captured
-    void captured.done.then(
-      (outcome: SubprocessOutcome) => { processOutcome = outcome },
-      () => undefined,
-    )
+    managedProcess = process
   }
   try {
     query = officialQuery({
@@ -433,9 +435,8 @@ export async function startClaudeCodeRun(
     }
   } catch (error: unknown) {
     request.signal.removeEventListener('abort', onAbort)
-    const cancelledBeforeCleanup = controller.signal.aborted
     await Promise.resolve()
-    const startupOutcome = processOutcome
+    const startupOutcome = managedProcess?.outcome
     const startupFacts = {
       stage: 'query-start',
       category: 'unknown',
@@ -451,9 +452,10 @@ export async function startClaudeCodeRun(
         await disposeClaudeCodeChild(query, child)
       } catch (disposeError: unknown) {
         const failure = startupFailure()
+        const cleanupFailure = thrown(disposeError)
         throw new AggregateError(
-          [failure, thrown(disposeError)],
-          `${failure.message}; startup cleanup also failed`,
+          [failure, cleanupFailure],
+          `${failure.message}; ${cleanupFailure.message}`,
         )
       }
     } else if (query !== undefined) {
@@ -461,19 +463,19 @@ export async function startClaudeCodeRun(
         query.close()
       } catch (disposeError: unknown) {
         const failure = startupFailure()
+        const cleanupFailure = new ClaudeCodeFailure({
+          stage: 'teardown',
+          category: 'unknown',
+        }, thrown(disposeError))
         throw new AggregateError(
-          [
-            failure,
-            new ClaudeCodeFailure({
-              stage: 'teardown',
-              category: 'unknown',
-            }, thrown(disposeError)),
-          ],
-          `${failure.message}; startup cleanup also failed`,
+          [failure, cleanupFailure],
+          `${failure.message}; ${cleanupFailure.message}`,
         )
       }
     }
-    if (cancelledBeforeCleanup || isAborted(request.signal)) {
+    try {
+      request.signal.throwIfAborted()
+    } catch {
       throw new Error('subagent-claude-code: request was aborted before SDK startup')
     }
     throw startupFailure()
@@ -493,7 +495,7 @@ export async function startClaudeCodeRun(
           ))
         })
       } catch (error: unknown) {
-        await Promise.resolve()
+        const processOutcome = managedProcess?.outcome
         const facts = error instanceof ClaudeCodeFailure
           ? { ...error.facts, outcome: processOutcome }
           : processOutcome === undefined
@@ -503,14 +505,14 @@ export async function startClaudeCodeRun(
               category: 'process-exit',
               outcome: processOutcome,
             } as const
-        failureDetail = failureDiagnostic(facts)
+        prependFailureDiagnostic(facts)
         throw error instanceof ClaudeCodeFailure
           ? error
           : new ClaudeCodeFailure(facts, thrown(error))
       }
     },
     collectOutput: () => [],
-    collectDiagnostic,
+    collectDiagnostic: () => diagnostic,
     cancelled: () => controller.signal.aborted,
     onError: spec.onError,
     signal: request.signal,

+ 26 - 13
packages/subagent/subagent-claude-code/tests/real-product.spec.ts

@@ -227,17 +227,29 @@ async function expectQuiescent(
   }
 }
 
-function expectedProcessFailure(outcome: SubprocessOutcome): string {
+function expectedFailure(
+  stage: 'query-run' | 'process',
+  category: 'error_during_execution' | 'process-exit',
+  outcome: SubprocessOutcome,
+): string {
   const fields = [
     'product: Claude Code',
-    'stage: process',
-    'category: process-exit',
+    `stage: ${stage}`,
+    `category: ${category}`,
   ]
   if (outcome.exitCode !== null) fields.push(`exit code: ${outcome.exitCode}`)
   if (outcome.signal !== null) fields.push(`signal: ${outcome.signal}`)
   return `Product subagent failure (${fields.join('; ')})`
 }
 
+function expectedObservedFailure(outcome: SubprocessOutcome): string {
+  return observedSdkMessages.some(message =>
+    message.type === 'result'
+    && message.subtype === 'error_during_execution')
+    ? expectedFailure('query-run', 'error_during_execution', outcome)
+    : expectedFailure('process', 'process-exit', outcome)
+}
+
 function startRequest(
   harness: RealHarness,
   prompt: string,
@@ -356,11 +368,10 @@ describe('real Claude Agent SDK 0.3.220 and its distributed Claude Code 2.1.220
     expect(harness.handles).toHaveLength(1)
     harness.handles[0]!.terminate()
     const outcome = await harness.handles[0]!.done
-    await expect(run.result).resolves.toEqual({
-      output: [],
-      diagnostic: expectedProcessFailure(outcome),
-      stopReason: 'error',
-    })
+    const result = await run.result
+    expect(result.output).toEqual([])
+    expect(result.stopReason).toBe('error')
+    expect(result.diagnostic).toBe(expectedObservedFailure(outcome))
     await run.dispose()
     expect(fixture.requests).toHaveLength(1)
     expect(fixture.requests[0]!.headers['x-api-key']).toBe(fakeKey)
@@ -389,11 +400,13 @@ describe('real Claude Agent SDK 0.3.220 and its distributed Claude Code 2.1.220
     harness.handles[0]!.terminate()
     const outcome = await harness.handles[0]!.done
     const result = await run.result
-    expect(result).toEqual({
-      output: [],
-      diagnostic: `${expectedProcessFailure(outcome)}\nClaude Code unattended decision (mode: dontAsk; request: tool permission; decision: denied): Claude Code denied the request before an interactive prompt`,
-      stopReason: 'error',
-    })
+    expect(result.output).toEqual([])
+    expect(result.stopReason).toBe('error')
+    const diagnosticLines = result.diagnostic?.split('\n') ?? []
+    expect(diagnosticLines[0]).toBe(expectedObservedFailure(outcome))
+    expect(diagnosticLines[1]).toBe(
+      'Claude Code unattended decision (mode: dontAsk; request: tool permission; decision: denied): Claude Code denied the request before an interactive prompt',
+    )
     expect(result.diagnostic).not.toContain(target)
     expect(result.diagnostic).not.toContain('SECRET_TOKEN')
     await run.dispose()

+ 30 - 0
packages/subagent/subagent-claude-code/tests/subagent-claude-code.spec.ts

@@ -553,6 +553,7 @@ describe('official spawn projection', () => {
     expect(process.killed).toBe(false)
     expect(process.exitCode).toBeNull()
     expect(process.signalCode).toBeNull()
+    expect(process.outcome).toBeUndefined()
 
     const exit = vi.fn()
     const once = vi.fn()
@@ -572,6 +573,7 @@ describe('official spawn projection', () => {
     expect(once).toHaveBeenCalledOnce()
     expect(removed).not.toHaveBeenCalled()
     expect(process.signalCode).toBe('SIGTERM')
+    expect(process.outcome).toEqual({ exitCode: null, signal: 'SIGTERM' })
     expect(process.kill('SIGTERM')).toBe(false)
   })
 
@@ -598,6 +600,7 @@ describe('official spawn projection', () => {
     await nextTask()
     expect(process.exitCode).toBe(7)
     expect(process.signalCode).toBeNull()
+    expect(process.outcome).toEqual({ exitCode: 7, signal: null })
     expect(process.kill('SIGTERM')).toBe(false)
   })
 })
@@ -1084,6 +1087,9 @@ describe('run publication, cancellation, and settlement', () => {
     })
     await expect(noChild)
       .rejects.toThrow(expectedFailureDiagnostic('query-start', 'unknown'))
+    await expect(noChild).rejects.toThrow(
+      `${expectedFailureDiagnostic('query-start', 'unknown')}; subagent-claude-code: ${expectedFailureDiagnostic('teardown', 'unknown')}`,
+    )
     await expect(noChild).rejects.toBeInstanceOf(AggregateError)
 
     const startupAbort = new AbortController()
@@ -1126,6 +1132,9 @@ describe('run publication, cancellation, and settlement', () => {
       .rejects.toBeInstanceOf(AggregateError)
     await expect(cancelledCleanupFailure)
       .rejects.toThrow(expectedFailureDiagnostic('query-start', 'unknown'))
+    await expect(cancelledCleanupFailure).rejects.toThrow(
+      `${expectedFailureDiagnostic('query-start', 'unknown')}; subagent-claude-code: ${expectedFailureDiagnostic('teardown', 'unknown', { exitCode: 0, signal: null })}`,
+    )
     await expect(cancelledCleanupFailure)
       .rejects.not.toThrow('SECRET_TOKEN')
 
@@ -1167,6 +1176,24 @@ describe('run publication, cancellation, and settlement', () => {
     expect(factoryController?.signal.aborted).toBe(true)
     expect(spawned.terminate).toHaveBeenCalledOnce()
 
+    const cleanupRaceAbort = new AbortController()
+    const cleanupRaceChild = fakeChild({ exitOnTerminate: false })
+    queryMock.mockImplementationOnce(({ options }) => {
+      options.spawnClaudeCodeProcess!(sdkSpawnOptions())
+      throw new Error('query failed before cleanup wait')
+    })
+    const cleanupRace = startClaudeCodeRun(
+      request(undefined, cleanupRaceAbort.signal),
+      {
+        ...unused.spec,
+        spawn: () => cleanupRaceChild.handle,
+      },
+    )
+    await nextTask()
+    cleanupRaceAbort.abort(new Error('cancelled during cleanup'))
+    cleanupRaceChild.settle()
+    await expect(cleanupRace).rejects.toThrow('aborted before SDK startup')
+
     const failedSpawn = fakeChild({
       pid: -1,
       doneError: new Error('spawn failed'),
@@ -1175,6 +1202,9 @@ describe('run publication, cancellation, and settlement', () => {
     const failedStartup = startClaudeCodeRun(request(), failed.spec)
     await expect(failedStartup)
       .rejects.toThrow(expectedFailureDiagnostic('query-start', 'unknown'))
+    await expect(failedStartup).rejects.toThrow(
+      `${expectedFailureDiagnostic('query-start', 'unknown')}; subagent-claude-code: ${expectedFailureDiagnostic('teardown', 'unknown')}`,
+    )
     await expect(failedStartup).rejects.toBeInstanceOf(AggregateError)
     expect(failed.close).toHaveBeenCalledOnce()
   })