From a1967fbbdbc2c26197bf2a4441daff7acb69382a Mon Sep 17 00:00:00 2001 From: Wenyu Chiou <162016108+WenyuChiou@users.noreply.github.com> Date: Sat, 24 Oct 2026 13:16:26 -0400 Subject: [PATCH] fix(lldb): retain completed trace facts through cleanup failures (#7023) --- src/native/LldbCallTracer.ts | 71 +++-- src/native/NativeCallObservation.ts | 14 +- .../native/lldbCallTracerFailures.test.ts | 251 ++++++++++++++++++ .../native/nativeCallObservation.test.ts | 3 +- 4 files changed, 314 insertions(+), 25 deletions(-) diff --git a/src/native/LldbCallTracer.ts b/src/native/LldbCallTracer.ts index e7c7c68eca..85f6d774a7 100644 --- a/src/native/LldbCallTracer.ts +++ b/src/native/LldbCallTracer.ts @@ -691,22 +691,68 @@ export class LldbCallTracer implements NativeCallTracer { ? "cancelled" : exited.kind === "timeout" ? "timeout" - : exited.cleanupFailure !== undefined + : lifecycleCleanup !== undefined ? "cleanup-failure" : "tracer-failure"; - const getPartialObservation = ( + const readCompletedCaptures = async ( + run: z.infer, + ) => ({ + stdout: await capturedOutput( + paths.stdoutCapture, + run.target_output.stdout_bytes, + run.target_output.stdout_truncated, + run.target_output.stdout_complete, + retained.files, + ), + stderr: await capturedOutput( + paths.stderrCapture, + run.target_output.stderr_bytes, + run.target_output.stderr_truncated, + run.target_output.stderr_complete, + retained.files, + ), + }); + const getPartialObservation = async ( reason: NativeCallPartialObservation["coverage"]["reason"] = partialReason, - ): Promise => - partialNativeObservation({ + ): Promise => { + // Cleanup uncertainty does not invalidate a completed, validated result. + // Keep host termination proof separate from the bridge's reported facts. + if (output?.status === "traced") { + const { stdout, stderr } = await readCompletedCaptures(output); + return { + observation: projectPartialNativeCalls( + { + path: request.executable, + sha256: request.expectedSha256, + architecture: request.architecture, + }, + request.input, + { + debugger: { ...lldb, version: output.version }, + run: output, + stdout: stdout.ok ? stdout.value : (stdout.partial ?? null), + stderr: stderr.ok ? stderr.value : (stderr.partial ?? null), + terminated, + }, + reason, + ), + readFailures: [ + ...(stdout.ok ? [] : [stdout.error]), + ...(stderr.ok ? [] : [stderr.error]), + ], + }; + } + return partialNativeObservation({ request, files: retained.files, pidPath: paths.pid, stdoutPath: paths.stdoutCapture, stderrPath: paths.stderrCapture, observationPath: paths.observations, - version: output?.status === "traced" ? output.version : null, + version: null, reason, }); + }; const failureWithPartial = async ( createFailure: (observation: NativeCallPartialObservation) => Failure, reason?: NativeCallPartialObservation["coverage"]["reason"], @@ -818,20 +864,7 @@ export class LldbCallTracer implements NativeCallTracer { : { cleanup: lifecycleCleanup }), }), ); - const stdout = await capturedOutput( - paths.stdoutCapture, - output.target_output.stdout_bytes, - output.target_output.stdout_truncated, - output.target_output.stdout_complete, - retained.files, - ); - const stderr = await capturedOutput( - paths.stderrCapture, - output.target_output.stderr_bytes, - output.target_output.stderr_truncated, - output.target_output.stderr_complete, - retained.files, - ); + const { stdout, stderr } = await readCompletedCaptures(output); if (!stdout.ok || !stderr.ok) { return err( new ProviderAdapterError(PROVIDER, OPERATION, { diff --git a/src/native/NativeCallObservation.ts b/src/native/NativeCallObservation.ts index 12b9e72a90..494dab21fc 100644 --- a/src/native/NativeCallObservation.ts +++ b/src/native/NativeCallObservation.ts @@ -116,6 +116,10 @@ const nativeCallFacts = ( trace: NativeCallReadback, ) => { const { run } = trace; + const terminated = run.terminated && trace.terminated; + const terminationSummary = terminated + ? "the process was killed" + : "termination was requested but could not be confirmed"; const unresolved = run.breakpoints .filter(({ location_count: count }) => count === 0) .map(({ index }) => index); @@ -153,7 +157,7 @@ const nativeCallFacts = ( outcome: run.outcome, exit_status: run.exit_status, exit_description: run.exit_description, - terminated: run.terminated && trace.terminated, + terminated, elapsed_ms: run.elapsed_ms, stdout: trace.stdout, stderr: trace.stderr, @@ -183,17 +187,17 @@ const nativeCallFacts = ( "The selected pathname digest and LLDB module identity are checked around launch. No immutable executable snapshot is used; concurrent in-place mutation or replacement and restoration cannot be ruled out, so the loaded-image digest remains unknown.", ...(eventLimit ? [ - `Observation stopped at max_events (${input.max_events}); later calls were not recorded and the process was killed.`, + `Observation stopped at max_events (${input.max_events}); later calls were not recorded and ${terminationSummary}.`, ] : []), ...(resourceLimit ? [ - "Observation stopped before retaining the next complete event because the estimated 8388608-byte (8 MiB) trace-payload or 65536-frame aggregate budget was reached; the process was killed. This release has no runtime override for those budgets.", + `Observation stopped before retaining the next complete event because the estimated 8388608-byte (8 MiB) trace-payload or 65536-frame aggregate budget was reached; ${terminationSummary}. This release has no runtime override for those budgets.`, ] : []), ...(run.outcome === "duration-elapsed" ? [ - `The process was still running after duration_ms (${input.duration_ms}) and was killed.`, + `The process was still running after duration_ms (${input.duration_ms}); ${terminationSummary}.`, ] : []), ...(run.target_identity.stable @@ -203,7 +207,7 @@ const nativeCallFacts = ( ]), ...(run.outcome === "stop-limit" ? [ - "Observation stopped after repeated signal or exception stops; the process was killed.", + `Observation stopped after repeated signal or exception stops; ${terminationSummary}.`, ] : []), ...(unresolved.length === 0 diff --git a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts index 2ab65131ec..3ba872d901 100644 --- a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts +++ b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts @@ -8,6 +8,7 @@ import { stat, type FileHandle, } from "node:fs/promises"; +import { ChildProcess } from "node:child_process"; import { dirname } from "node:path"; import { join } from "node:path"; import { tmpdir } from "node:os"; @@ -20,6 +21,7 @@ import { AnalysisCapabilityUnavailableError, AnalysisTimeoutError, } from "../../../../src/domain/analysisErrorCore.js"; +import { ProviderCleanupError } from "../../../../src/domain/providerCleanupError.js"; import { EvidenceIntegrityError } from "../../../../src/domain/evidenceErrors.js"; import { ArtifactFileCleanupFailure } from "../../../../src/artifacts/ArtifactFileOperation.js"; import { resolvedAnalysisError } from "../../../../src/domain/analysisErrorBase.js"; @@ -278,6 +280,255 @@ describe("LLDB completed trace retention through finalization", () => { }); }); +const expectCompletedCleanupFacts = ( + partial: ReturnType, + completed: typeof successfulTracerOutput, +) => { + expect(partial.debugger).toEqual({ + ...fakeTool, + version: completed.version, + }); + expect(partial.target.launch_identity).toEqual(completed.target_identity); + expect(partial.events).toEqual(completed.events); + expect(partial.breakpoints).toEqual([ + { + ...completed.breakpoints[0], + request: { kind: "function", name: "main", module: null }, + }, + ]); + expect(partial.limitations).not.toContain( + "The LLDB run did not produce a complete native call observation result.", + ); + expect(partial.limitations).not.toContain( + "Captured output byte counts are observed lower bounds because LLDB did not report final stream counters.", + ); +}; + +describe("LLDB completed trace retention after process cleanup", () => { + it.each([ + "clean", + "group-cleanup", + "target-uncertain", + "target-uncertain-limit", + "group-capture-failure", + ] as const)("retains validated completed facts with %s", async (phase) => { + let runtime: string | undefined; + let launches = 0; + let releaseGroup = false; + const targetUncertain = phase.startsWith("target-"); + const groupFailure = phase.startsWith("group-"); + const captureFailure = phase === "group-capture-failure"; + // This handle is never spawned. The production supervisor owns only the + // synthetic launcher's injected cleanup receipt, not an OS process. + const child = new ChildProcess(); + const cleanup = vi.fn(async () => + releaseGroup + ? { cleaned: true as const, signaled: false } + : { cleaned: false as const, reason: "fixture group cleanup failed" }, + ); + const supervisor = new ProviderProcessSupervisor({ + process: child, + ownsProcessLifetime: true, + cleanup, + }); + child.emit("exit", 0, null); + child.emit("close", 0, null); + const completed = { + ...successfulTracerOutput, + ...(phase === "target-uncertain-limit" + ? { outcome: "duration-elapsed", terminated: false, killed: true } + : {}), + target_output: { + stdout_bytes: 4096, + stderr_bytes: 2, + stdout_truncated: true, + stderr_truncated: false, + stdout_complete: true, + stderr_complete: true, + }, + }; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + launches += 1; + const { config } = await configFromArguments(arguments_); + runtime = dirname(config.result_path); + // A shorter journal must not replace the validated completed result. + await writeFile(config.observation_path, ""); + if (!captureFailure) await writeFile(config.stdout_capture_path, "abc"); + await writeFile(config.stderr_capture_path, "ok"); + await writeFile(config.result_path, JSON.stringify(completed)); + return { + kind: "exited", + targetLaunched: targetUncertain, + exitCode: 0, + output: "", + targetIdentity: undefined, + cleanupFailure: groupFailure + ? "fixture group cleanup failed" + : undefined, + ...(groupFailure ? { cleanupOwner: supervisor } : {}), + }; + }, + async () => ok(fakeTool), + ); + try { + const result = await tracer.trace(request); + if (runtime === undefined) throw new Error("fixture did not launch"); + const stdout = { + text: "abc", + bytes: 4096, + truncated: true, + complete: false, + }; + const stderr = { + text: "ok", + bytes: 2, + truncated: false, + complete: true, + }; + if (phase === "clean") { + expect(result.ok).toBe(true); + if (!result.ok) throw result.error; + expect(result.value.run).toEqual(completed); + expect(result.value.stdout).toEqual(stdout); + expect(result.value.stderr).toEqual(stderr); + await expect(access(runtime)).rejects.toMatchObject({ + code: "ENOENT", + }); + return; + } + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected cleanup failure"); + expect(result.error).toBeInstanceOf(ProviderCleanupError); + expect(result.error.cleanupIncomplete).toBe(true); + expect(result.error.cleanupResources).toContain(runtime); + await access(runtime); + const retry = await tracer.trace(request); + expect(retry.ok).toBe(false); + expect(launches).toBe(1); + expect((await tracer.close()).ok).toBe(false); + const partial = nativeCallPartialObservationSchema.parse( + result.error.partialObservation, + ); + expect(partial.coverage.reason).toBe("cleanup-failure"); + expect(partial.process).toEqual({ + pid: completed.pid, + outcome: completed.outcome, + exit_status: completed.exit_status, + exit_description: completed.exit_description, + // Producer termination does not establish independent host cleanup. + terminated: !targetUncertain, + elapsed_ms: completed.elapsed_ms, + stdout: captureFailure ? null : stdout, + stderr, + other_stops: completed.other_stops, + }); + expectCompletedCleanupFacts(partial, completed); + if (phase === "target-uncertain-limit") { + expect(partial.limitations.join(" ")).not.toContain("was killed"); + expect(partial.limitations.join(" ")).toContain( + "termination was requested but could not be confirmed", + ); + } + if (captureFailure) { + expect(result.error.secondaryFailures).toHaveLength(1); + expect(result.error.secondaryFailures[0]?.cause).toMatchObject({ + code: "ENOENT", + }); + expect(partial.limitations).toContain( + "One or more target output captures were unavailable.", + ); + } + if (groupFailure) { + releaseGroup = true; + expect((await tracer.close()).ok).toBe(true); + expect(cleanup).toHaveBeenCalled(); + expect(result.error.cleanupIncomplete).toBe(false); + await expect(access(runtime)).rejects.toMatchObject({ + code: "ENOENT", + }); + } + } finally { + releaseGroup = true; + await tracer.close(); + supervisor.dispose(); + // No target was launched; only the synthetic unknown-target fixture + // needs test-owned removal after proving production retains ownership. + if (runtime !== undefined) + await rm(runtime, { recursive: true, force: true }); + } + }); +}); + +describe("LLDB cleanup without a validated completed trace", () => { + it.each(["missing", "invalid"] as const)( + "keeps journal-only fallback when the completed result is %s", + async (resultKind) => { + let runtime: string | undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + const { config } = await configFromArguments(arguments_); + runtime = dirname(config.result_path); + await writeFile( + config.observation_path, + `${JSON.stringify({ kind: "event", event: EVENT })}\n`, + ); + await writeFile(config.stdout_capture_path, "abc"); + await writeFile(config.stderr_capture_path, ""); + if (resultKind === "invalid") + await writeFile( + config.result_path, + JSON.stringify({ status: "traced", outcome: "exited" }), + ); + return { + kind: "exited", + targetLaunched: true, + exitCode: 0, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + try { + const result = await tracer.trace(request); + if (result.ok) throw new Error("expected cleanup failure"); + expect(result.error).toBeInstanceOf(ProviderCleanupError); + expect(result.error.cleanupIncomplete).toBe(true); + const partial = nativeCallPartialObservationSchema.parse( + result.error.partialObservation, + ); + expect(partial.coverage.reason).toBe("cleanup-failure"); + expect(partial.events).toEqual([EVENT]); + expect(partial.process.stdout).toEqual({ + text: "abc", + bytes: 3, + truncated: true, + complete: false, + }); + expect(partial.process.outcome).toBeUndefined(); + expect(partial.process.exit_status).toBeUndefined(); + expect(partial.process.terminated).toBeUndefined(); + expect(partial.process.elapsed_ms).toBeUndefined(); + expect(partial.target.launch_identity).toBeUndefined(); + expect(partial.breakpoints).toBeUndefined(); + expect(partial.debugger.version).toBeNull(); + expect(partial.limitations).toContain( + "Captured output byte counts are observed lower bounds because LLDB did not report final stream counters.", + ); + expect((await tracer.close()).ok).toBe(false); + } finally { + // The launcher never created a target; remove only its synthetic files. + if (runtime !== undefined) + await rm(runtime, { recursive: true, force: true }); + } + }, + ); +}); + describe("LLDB failure retention through the production tracer", () => { it("treats an invalid UTF-8 tracer result as unavailable and retains journal facts", async () => { let configPath: string | undefined; diff --git a/tests/boundary/providers/native/nativeCallObservation.test.ts b/tests/boundary/providers/native/nativeCallObservation.test.ts index aac1f4f8be..beea50d320 100644 --- a/tests/boundary/providers/native/nativeCallObservation.test.ts +++ b/tests/boundary/providers/native/nativeCallObservation.test.ts @@ -307,12 +307,13 @@ describe("observe_native_calls projection: reports partial observation coverage" unresolved_breakpoints: [1], }); expect(result.process.terminated).toBe(false); + expect(result.limitations.join(" ")).not.toContain("was killed"); expect(result.limitations).toContain( "Target output draining did not complete; reported byte counts are observed lower bounds, not stream totals.", ); expect(result.limitations).toEqual( expect.arrayContaining([ - "Observation stopped at max_events (1); later calls were not recorded and the process was killed.", + "Observation stopped at max_events (1); later calls were not recorded and termination was requested but could not be confirmed.", "Breakpoints 1 matched no code in any image loaded while observing; their calls could not be observed.", "Target output beyond 1048576 bytes (1 MiB) per stream is counted but not kept. This release has no runtime override.", "REA could not confirm that process 4242 exited; check for it before relying on host state.",