From f1b7d52a778ab68fd1bfd2da044c9fb6cf328fb1 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Sun, 26 Jul 2026 14:12:52 +0800
Subject: [PATCH] fix review findings: hostile code accessor + swallowed
 teardown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Both ds-review-bot findings were real:

- markLlmAdapterFailure's carried-facts cross-check read error.code
  directly; a foreign Error with a valid own failure payload but a
  throwing code accessor would replace the original adapter error with
  the accessor exception, breaking the error-identity guarantee. The
  read now goes through foreignErrorCode(), which contains the trap and
  falls back to the normalized snapshot (test: hostile code accessor
  beside a valid failure payload -> original identity kept, UNKNOWN
  facts).
- live-interactions' afterEach caught scaffold.close() into undefined,
  silently disabling ReplayHandle.assertConsumed() — the fixture-drift
  tripwire — and hiding cleanup defects. Teardown now runs every step,
  collects failures, and rethrows (AggregateError when several).
---
 apps/web/tests/live-interactions.e2e.ts | 13 ++++++++++---
 packages/llm/llm/src/adapter-failure.ts | 13 ++++++++++++-
 packages/llm/llm/tests/service.spec.ts  | 21 +++++++++++++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/apps/web/tests/live-interactions.e2e.ts b/apps/web/tests/live-interactions.e2e.ts
index 60dec690c7..46a03281f9 100644
--- a/apps/web/tests/live-interactions.e2e.ts
+++ b/apps/web/tests/live-interactions.e2e.ts
@@ -57,12 +57,19 @@ describe('web e2e: live-turn interactions (cancel / error / retry)', () => {
   let sidecarDir: string | undefined
 
   afterEach(async () => {
-    await browser?.close().catch(() => undefined)
+    // scaffold.close() failures MUST fail the scenario: assertConsumed() is
+    // the fixture-drift tripwire and cleanup problems are real defects. Run
+    // every teardown step regardless, then rethrow what failed.
+    const failures: unknown[] = []
+    await browser?.close().catch((error: unknown) => failures.push(error))
     browser = undefined
-    await scaffold?.close().catch(() => undefined)
+    const closing = scaffold
     scaffold = undefined
-    if (sidecarDir !== undefined) await rm(sidecarDir, { recursive: true, force: true }).catch(() => undefined)
+    await closing?.close().catch((error: unknown) => failures.push(error))
+    if (sidecarDir !== undefined) await rm(sidecarDir, { recursive: true, force: true }).catch((error: unknown) => failures.push(error))
     sidecarDir = undefined
+    if (failures.length === 1) throw failures[0]
+    if (failures.length > 1) throw new AggregateError(failures, 'live-interactions teardown failed')
   })
 
   /** Boot scaffold + page with an optional override doc materialized per run. */
diff --git a/packages/llm/llm/src/adapter-failure.ts b/packages/llm/llm/src/adapter-failure.ts
index 8da17807fa..2cf2dbe216 100644
--- a/packages/llm/llm/src/adapter-failure.ts
+++ b/packages/llm/llm/src/adapter-failure.ts
@@ -53,7 +53,7 @@ export function markLlmAdapterFailure(
   // exactly when class identity is lost (a second copy of this package in
   // the process, e.g. a source-plane test harness over a lib-plane boot).
   const carried = ownFailureSnapshot(error)
-  const failure = carried !== undefined && carried.code === error.code ? carried : Object.freeze({
+  const failure = carried !== undefined && carried.code === foreignErrorCode(error) ? carried : Object.freeze({
     message: errorMessage(error),
     code: harnessErrorCode(error),
   })
@@ -61,6 +61,17 @@ export function markLlmAdapterFailure(
   return error
 }
 
+/** Read a foreign error's `code` for the cross-check without letting an SDK accessor replace the primary failure. */
+function foreignErrorCode(error: Error & { code?: string }): unknown {
+  try {
+    return error.code
+  } catch (_sdkCodeGetter) {
+    // An unreadable code cannot confirm the carried facts describe this
+    // error; the caller falls back to the normalized snapshot.
+    return undefined
+  }
+}
+
 /** Snapshot an own data property without invoking an SDK-defined accessor. */
 function ownFailureSnapshot(error: Error): LlmFailure | undefined {
   try {
diff --git a/packages/llm/llm/tests/service.spec.ts b/packages/llm/llm/tests/service.spec.ts
index 90be1ffcb0..7c9f632a20 100644
--- a/packages/llm/llm/tests/service.spec.ts
+++ b/packages/llm/llm/tests/service.spec.ts
@@ -324,6 +324,27 @@ describe('LlmService', () => {
     expect(llmFailureOf(stream, original)).toEqual({ message: 'LLM adapter failed', code: 'UNKNOWN' })
   })
 
+  it('keeps an SDK Error exact when a valid failure payload rides a hostile code accessor', async () => {
+    // The carried-facts cross-check reads error.code; a throwing accessor
+    // there must fall back to the normalized snapshot instead of replacing
+    // the original adapter error with the accessor exception.
+    const original = Object.assign(new Error('busy'), {
+      failure: { message: 'busy', code: 'SERVER', status: 503 },
+    })
+    Object.defineProperty(original, 'code', {
+      get() { throw new Error('SDK code accessor must not escape') },
+    })
+    const ctx = new Context()
+    await ctx.plugin(LlmService)
+    ctx.llm.registerAdapter(['test-provider'], new ThrowingAdapter(original))
+    const stream = ctx.llm.stream({ provider: 'test-provider', model: 'test-model', messages: [] })
+
+    await expect((async () => {
+      for await (const _chunk of stream) { /* drain */ }
+    })()).rejects.toBe(original)
+    expect(llmFailureOf(stream, original)).toEqual({ message: 'busy', code: 'UNKNOWN' })
+  })
+
   it('falls back safely when SDK objects trap failure inspection or expose malformed facts', async () => {
     const propertyTrap = new Proxy(new HarnessError('descriptor trapped', 'SERVER'), {
       getOwnPropertyDescriptor(target, property) {