fix: byte-exact value/error caps + write-callback contract (agent review)

Two [P1] review findings on the worker runtime:

- maxValueBytes gated and sliced the rendered fallback by UTF-16 code
  units, so a multibyte string ("€€€€" under a 4-byte cap) crossed whole
  and a truncated multibyte rendering could still run ~3x over budget.
  New truncateUtf8Bytes cuts at code-point boundaries under a real byte
  budget; prepareValue's fallback and the host's forged-error-text bound
  both use it, and the VALUE_RENDER_SLACK comment drops its now-obsolete
  "sliced by characters" wrinkle.

- The patched stream write dropped Node's optional encoding/callback
  arguments, so a program awaiting flush completion
  (write(chunk, resolve)) hung to the wall ceiling and misreported as a
  timeout. The shim now fires the callback asynchronously once the chunk
  is admitted — including for writes the exhausted budget drops.
This commit is contained in:
Tianyi Cui
2026-07-08 21:56:28 +08:00
parent 030eebb634
commit 90547f283b
4 changed files with 132 additions and 11 deletions

View File

@@ -1,6 +1,6 @@
import { describe, expect, it } from 'vitest'
import { EventEmitter } from 'node:events'
import { LogBuffer, makeConsoleShim, makeNamespaces, captureStreamWrites, prepareValue, runWorkerMain, wireReplies } from '@deepseek-ai/dsh-code-runtime-worker/src/bootstrap.ts'
import { LogBuffer, makeConsoleShim, makeNamespaces, captureStreamWrites, prepareValue, runWorkerMain, truncateUtf8Bytes, wireReplies } from '@deepseek-ai/dsh-code-runtime-worker/src/bootstrap.ts'
import type { BootstrapPort, PatchableStream, PendingCall } from '@deepseek-ai/dsh-code-runtime-worker/src/bootstrap.ts'
import type { ReplyMessage, WorkerToHost } from '@deepseek-ai/dsh-code-runtime-worker/src/protocol.ts'
import type { CodeLogEntry } from '@deepseek-ai/dsh-code-runtime'
@@ -90,6 +90,27 @@ describe('captureStreamWrites', () => {
expect(seen[0]).toMatchObject({ source: 'stdout' })
expect(underlying).toBe('after')
})
it('invokes the write callback asynchronously, in both optional-encoding shapes', async () => {
const buffer = new LogBuffer(1_000, () => {})
const stream: PatchableStream = { write: () => true }
captureStreamWrites(buffer, stream, 'stdout')
const calls: (Error | null | undefined)[] = []
stream.write('two-arg', (error?: Error | null) => calls.push(error))
stream.write('three-arg', 'utf8', (error?: Error | null) => calls.push(error))
// Node's contract: the callback fires after the write call returns.
expect(calls).toEqual([])
await new Promise<void>(resolve => stream.write('awaited flush', resolve))
expect(calls).toEqual([null, null])
})
it('still fires the callback for a write the exhausted budget drops', async () => {
const buffer = new LogBuffer(4, () => {})
const stream: PatchableStream = { write: () => true }
captureStreamWrites(buffer, stream, 'stdout')
stream.write('this write overflows the budget and is dropped')
await new Promise<void>(resolve => stream.write('also dropped', resolve))
})
})
describe('prepareValue', () => {
@@ -118,6 +139,34 @@ describe('prepareValue', () => {
expect(typeof value).toBe('string')
expect(value).toContain('more items')
})
it('caps a multibyte string by UTF-8 bytes, not UTF-16 length', () => {
// 4 code units but 12 UTF-8 bytes: a length-counting cap would pass the
// full string through untruncated.
expect(prepareValue('€€€€', 4)).toEqual({ value: '€… [truncated]' })
})
it('caps a multibyte rendering by UTF-8 bytes too', () => {
// Wire size (24-byte string inside an array) exceeds the cap, so the
// value crosses as its rendering — whose truncation must also be
// byte-exact: "[ '" (3 bytes) + two € (6 bytes) = 9; a third € would
// overflow the 10-byte budget.
expect(prepareValue(['€€€€€€€€'], 10)).toEqual({ value: "[ '€€… [truncated]" })
})
})
describe('truncateUtf8Bytes', () => {
it('returns a fitting string whole', () => {
expect(truncateUtf8Bytes('fits', 4)).toBe('fits')
})
it('cuts at a code-point boundary, never mid-surrogate-pair', () => {
// Each 😀 is one code point, two code units, four UTF-8 bytes: a 5-byte
// budget fits exactly one — and never leaves a lone surrogate behind.
const cut = truncateUtf8Bytes('😀😀', 5)
expect(cut).toBe('😀')
expect(Buffer.byteLength(truncateUtf8Bytes('😀😀', 3), 'utf8')).toBe(0)
})
})
describe('makeNamespaces', () => {

View File

@@ -221,6 +221,29 @@ describe('WorkerCodeRuntime — budgets and containment (real workers)', () => {
expect(result.value).toBe(`${'y'.repeat(64)}… [truncated]`)
})
it('caps a multibyte return value by UTF-8 bytes, not string length', async () => {
// 4 code units, 12 UTF-8 bytes: a length-counting cap would let the full
// string cross. The worker's byte-exact capped rendering then passes the
// host re-cap unchanged (cap + marker is exactly the granted slack).
const { runtime } = await setup({ maxValueBytes: 4 })
const result = await runtime.run({ program: 'return "€€€€"', bindings: [] })
expect(result.value).toBe('€… [truncated]')
})
it('completes a program that awaits its write callback, capturing the chunk', async () => {
// Node's write(chunk[, encoding][, callback]) contract: dropping the
// callback would leave this promise pending until the wall ceiling and
// misreport a completed program as a timeout.
const { runtime } = await setup({ maxWallMs: 2_000 })
const result = await runtime.run({
program: 'await new Promise(resolve => process.stdout.write("flushed", resolve)); return "done"',
bindings: [],
})
expect(result.error).toBeUndefined()
expect(result.value).toBe('done')
expect(result.logs).toContainEqual({ source: 'stdout', text: 'flushed' })
})
it('caps a huge container whose bounded rendering is small (wire size, not rendering, is what counts)', async () => {
const { runtime } = await setup()
const result = await runtime.run({ program: 'return new Array(50_000).fill(7)', bindings: [] })
@@ -340,6 +363,21 @@ describe('WorkerCodeRuntime — hostile programs (real workers)', () => {
expect(result.error).toEqual({ kind: 'exception', message: 'fake failure' })
})
it('byte-bounds forged multibyte error text at the host', async () => {
// Forged error text bypasses the worker entirely; the host bound is a
// BYTE bound (two € = 6 bytes fit an 8-byte cap, a third would not).
const { runtime } = await setup({ maxValueBytes: 8 })
const result = await runtime.run({
program: `
const { parentPort } = await import('node:worker_threads');
parentPort.postMessage({ type: 'done', error: { message: '€'.repeat(1000) } });
for (;;) {}
`,
bindings: [],
})
expect(result.error).toEqual({ kind: 'exception', message: '€€' })
})
it('answers a binding whose resolution cannot be cloned with a failure reply', async () => {
const { runtime } = await setup()
const result = await runtime.run({