fix review findings: hostile code accessor + swallowed teardown
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).
This commit is contained in:
@@ -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. */
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user