From 44203f3fa7423d476dbdea6c14a80c424237f0ca Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Sun, 2 Aug 2026 21:23:27 +0800 Subject: [PATCH] fix(code-runtime-python): resolve worker-exit on sync spawn failure; aggregate stray output by line Wrap spawn and the fd-3 narrowing so a synchronous throw (ENAMETOOLONG on an over-PATH_MAX pythonBin, EMFILE) removes the run's staging directory and resolves the same worker-exit class as the async error event, instead of rejecting run() and leaking the directory. Aggregate native stdout/stderr by real newline rather than by Node data chunk: logs entries are joined with "\n" downstream, so a newline-free write larger than one pipe read no longer reads back with spurious breaks. The ledger still bounds a newline-free flood. Track a running scan offset in both frame readers so a large frame accumulated across chunks is scanned once, not re-scanned from 0 per chunk. Reword the deadline hard-bound v8-ignore to state its real environment dependence (PID-1-doesn't-reap container, zombie survivor) and cross-ref the note's rejected signal-0 alternative; fix settle comments that quoted the pre-qualification teardown contract; document the capMessage vs _cap_message billing split on both sides; guard the dispose-after-resolve heartbeat assertion against a vacuous 0===0 pass; reuse _TRUNCATION_MARKER_BYTES; note the abandoned-call pending-entry bound. Update the Agent Note Decision/Testing/Alternatives/Consequences for the above and record the confirmed-empty finalize as a second honest fail-before exception; sync the zh pair. --- ...-runtime-python-settlement-fixes.i18n.yaml | 4 +- ...31-code-runtime-python-settlement-fixes.md | 30 ++- ...code-runtime-python-settlement-fixes.zh.md | 30 ++- .../code-runtime-python/py/bootstrap.py | 26 ++- .../code-runtime-python/src/index.ts | 196 ++++++++++++------ .../tests/boot-write-failure.spec.ts | 24 +++ .../code-runtime-python/tests/runtime.spec.ts | 33 +++ 7 files changed, 262 insertions(+), 81 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml index 4225a668cf..45ccc1425f 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md -2026-07-31-code-runtime-python-settlement-fixes.md: ba2f227d606c1c6d3efdd22372a4ea29ef50d068 -2026-07-31-code-runtime-python-settlement-fixes.zh.md: fa72c434e81ef5354f6112f834f85377666d8b3b +2026-07-31-code-runtime-python-settlement-fixes.md: 06df6f2c882f47c03ea45a4ec5085ef6c66a7013 +2026-07-31-code-runtime-python-settlement-fixes.zh.md: 8aa0e61f8818af3fd47fa589e32bf40336825268 diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md index ba2f227d60..06df6f2c88 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md @@ -6,11 +6,11 @@ English | [中文](2026-07-31-code-runtime-python-settlement-fixes.zh.md) ## Problem -The CPython subprocess backend for Code Mode, built on the [fd-3 frame protocol](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md), resolves every program outcome as a `CodeRunResult`, rejects `run()` only for seam misuse, and disposes to quiescence so no subprocess that stays in the child's process group outlives the fiber (a descendant that escapes the group with `setsid()` is the documented exception — see the package README's Known Limitations). A sequence of review passes surfaced defects that broke those contracts in ways unit coverage did not catch — each hid behind a `/* v8 ignore */`, a captured-callable that read as a fix but was not, a memory effect invisible through the seam, a load-time bound that double-counted, a process-group escalation that a survivor could outlast, or a cross-event-loop completion that silently deadlocked. Each behavioral fix ships with a test that fails without it; the one exception is a syscall-count improvement (chunked frame reading) with no cross-platform-deterministic failure to assert. +The CPython subprocess backend for Code Mode, built on the [fd-3 frame protocol](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md), resolves every program outcome as a `CodeRunResult`, rejects `run()` only for seam misuse, and disposes to quiescence so no subprocess that stays in the child's process group outlives the fiber (a descendant that escapes the group with `setsid()` is the documented exception — see the package README's Known Limitations). A sequence of review passes surfaced defects that broke those contracts in ways unit coverage did not catch — each hid behind a `/* v8 ignore */`, a captured-callable that read as a fix but was not, a memory effect invisible through the seam, a load-time bound that double-counted, a process-group escalation that a survivor could outlast, a cross-event-loop completion that silently deadlocked, a synchronous throw outside the settlement path, or a transport boundary rendered as a log boundary. Most behavioral fixes ship with a test that fails without them; two do not, and are called out as such — the chunked frame read (a syscall-count improvement with no cross-platform-deterministic failure) and the confirmed-empty finalize (its only seam-observable effect, a frozen heartbeat, freezes the instant SIGKILL is delivered, which the pre-fix finalize-on-delivery code also produced, and the discriminating probe is the signal-0 check the Alternatives reject as cross-environment-unreliable). ## Decision -Eight independent corrections, each in the package that owns the defect. +Independent corrections, each in the package that owns the defect. ### Boot-write failure no longer rejects run() @@ -26,7 +26,7 @@ Also in `src/index.ts`, after the newline loop over a `Buffer.concat` of the pen ### Output-cap load bound is ceiling minus envelope, not divided by six -The load-time check that rejects a `maxLogBytes`/`maxValueBytes` larger than one fd-3 frame can carry divided the frame ceiling by six for worst-case escape expansion. But both budgets are metered in ALREADY-ESCAPED serialized bytes — the host log ledger charges `Buffer.byteLength(JSON.stringify(text))` and `checkDoneValue` measures the escaped form — so a payload admitted under the cap occupies at most `cap + envelope` on the wire; escaping is inside the charge and must not be multiplied in again. The bound is now `FRAME_CEILING_BYTES - FRAME_ENVELOPE_BYTES`, and the unused `MAX_JSON_ESCAPE_EXPANSION` constant is gone. The old bound was not unsafe — it under-admitted — but it silently forbade legitimate large caps. +The load-time check that rejects a `maxLogBytes`/`maxValueBytes` larger than one fd-3 frame can carry divided the frame ceiling by six for worst-case escape expansion. But both budgets are metered in ALREADY-ESCAPED serialized bytes — the host log ledger charges `Buffer.byteLength(JSON.stringify(text))`, `checkDoneValue` measures the escaped form, and the producing-side `_cap_message` also caps by serialized cost — so a payload admitted under the cap occupies at most `cap + envelope` on the wire; escaping is inside the charge and must not be multiplied in again. The bound is now `FRAME_CEILING_BYTES - FRAME_ENVELOPE_BYTES`, and the unused `MAX_JSON_ESCAPE_EXPANSION` constant is gone. The old bound was not unsafe — it under-admitted — but it silently forbade legitimate large caps. The same load check also rejects a NON-INTEGER `maxLogBytes`/`maxValueBytes`: the child reads each budget through `int(...)`, which floors a float, so `maxLogBytes: 3.5` would truncate at 3 bytes child-side while the host meters the fraction — the two sides enforcing different public config. Rejecting the float at load keeps them in step, matching the worker backend. ### Same-group survivors are reaped before the fiber goes quiescent @@ -34,9 +34,11 @@ A model program can leave a descendant in the child's OWN process group (no `set Settlement also CANCELS the SIGKILL timer the moment the group is confirmed empty (the normal path, and when the poll sees the survivor gone). Leaving it armed would expose a PID-reuse hazard: a `kill(-pid)` left pending for up to `graceMs` after the leader was reaped could hit a RECYCLED pgid once the kernel reused the leader's pid, SIGKILLing an unrelated group (`killGroup` swallowing ESRCH does not help — the danger is precisely the kill that SUCCEEDS against a reused group). Clearing it on the empty probe bounds the reuse window to only the genuine-survivor case, where the group cannot be empty to reuse. +The reap poll also handles a host event loop BLOCKED past both timers. If a synchronous computation holds the loop from before the poll was scheduled until after its deadline, both the poll timer and the grace-window SIGKILL timer are overdue when the loop resumes, and Node runs the earlier-scheduled poll first — so the grace SIGKILL may never have fired. The deadline branch therefore sends SIGKILL ITSELF (idempotent if the timer already ran) rather than cancelling the unfired escalation, then grants ONE more `CLOSE_REAP_MARGIN_MS` and keeps polling until the group is confirmed empty, because finalizing on mere signal delivery would declare quiescence while the group is still dying. The outer bound on the wait is therefore `graceMs + 2 * CLOSE_REAP_MARGIN_MS`. A final hard bound finalizes if that extra margin elapses with the group still non-empty; that branch carries a `/* v8 ignore */` because it is reachable only where a SIGKILL'd survivor lingers as a zombie and is never `wait()`'d — a container whose PID 1 does not reap orphans — which cannot be built deterministically across CI platforms. The ignore's reason states that environment dependence rather than claiming the branch cannot run, cross-referencing the Alternatives entry that rejected the signal-0 reap assertion for the same reason. + ### RLIMIT clamps against the inherited soft limit, not only the hard -In [`py/bootstrap.py`](../../../../packages/code-runtime/code-runtime-python/py/bootstrap.py) `_clamped` bounded a requested `(soft, hard)` rlimit pair by the inherited HARD limit alone. A deployment that inherited a soft limit below the requested one — say inherited `(100, 200)`, requested `(150, 160)` — got back `(150, 160)`, RAISING the effective soft from 100 to 150: for `RLIMIT_AS` that loosens the memory ceiling, for `RLIMIT_CPU` it defers SIGXCPU, both violating "strictest of configured and inherited". `_clamped` now clamps each side against its own inherited counterpart (`RLIM_INFINITY` imposing no ceiling), then pins soft under hard so `setrlimit` never sees an inverted pair. The settlement-time CPU recheck (`die_if_cpu_exhausted`) follows the same rule: it compares spent CPU against the EFFECTIVE clamped `cpu_soft`, not the configured `cpuSeconds`, so a program that traps SIGXCPU and burns past a stricter inherited soft before returning is reported as a timeout rather than a false success. +In [`py/bootstrap.py`](../../../../packages/code-runtime/code-runtime-python/py/bootstrap.py) `_clamped` bounded a requested `(soft, hard)` rlimit pair by the inherited HARD limit alone. A deployment that inherited a soft limit below the requested one — say inherited `(100, 200)`, requested `(150, 160)` — got back `(150, 160)`, RAISING the effective soft from 100 to 150: for `RLIMIT_AS` that loosens the memory ceiling, for `RLIMIT_CPU` it defers SIGXCPU, both violating "strictest of configured and inherited". `_clamped` now clamps each side against its own inherited counterpart (`RLIM_INFINITY` imposing no ceiling), then pins soft under hard so `setrlimit` never sees an inverted pair. The settlement-time CPU recheck (`die_if_cpu_exhausted`) follows the same rule: it compares spent CPU against the EFFECTIVE clamped `cpu_soft`, not the configured `cpuSeconds`, so a program that traps SIGXCPU and burns past a stricter inherited soft before returning is reported as a timeout rather than a false success. The SIGXCPU diagnostic no longer names the configured `cpuSeconds` as the effective budget — under a stricter inherited soft that number is wrong — and instead reports that CPU time was exhausted at "at most the configured N seconds", which holds whichever limit fired. ### Binding replies complete on the calling loop's thread @@ -44,13 +46,21 @@ Also in `py/bootstrap.py`, a binding reply Future is created on the loop that ra ### The blocking frame reader reads in chunks, not byte by byte -`ProtocolChannel.read_frame` — used for the `boot` and `run` handshake frames — read through `FileIO.readline()` on the unbuffered (`buffering=0`) fd, which issues one `os.read(1)` per byte. The `run` frame arrives AFTER `RLIMIT_CPU` is in force, so a legitimate multi-megabyte program burned seconds of CPU in millions of single-byte syscalls before `ast.parse` ran — potentially exhausting the budget on the read alone. It now reads in `_READ_CHUNK_BYTES` chunks into the same `_pending` residual buffer the async reader already uses (the wrapping `os.fdopen` object is gone; both readers call `os.read(self._fd, ...)` directly), so the read cost is trivial and read-ahead past a newline is preserved for the next frame. +`ProtocolChannel.read_frame` — used for the `boot` and `run` handshake frames — read through `FileIO.readline()` on the unbuffered (`buffering=0`) fd, which issues one `os.read(1)` per byte. The `run` frame arrives AFTER `RLIMIT_CPU` is in force, so a legitimate multi-megabyte program burned seconds of CPU in millions of single-byte syscalls before `ast.parse` ran — potentially exhausting the budget on the read alone. It now reads in `_READ_CHUNK_BYTES` chunks into the same `_pending` residual buffer the async reader already uses (the wrapping `os.fdopen` object is gone; both readers call `os.read(self._fd, ...)` directly), so the read cost is trivial and read-ahead past a newline is preserved for the next frame. Both readers track a running scan offset (`find(b"\n", scanned)`) so a large frame accumulated across many chunks is scanned once, not re-scanned from index 0 per chunk — a chunked rescan would have replaced the byte-at-a-time cost with an O(N²) memchr cost on the same large-frame path. + +### Synchronous spawn failure resolves worker-exit, not reject + +Also in `src/index.ts`, `spawn` is called before the settlement Promise executor exists. Node defers only a fixed set of spawn errnos (EACCES, EAGAIN, EMFILE, ENFILE, ENOENT) to an asynchronous `error` event, which the settlement path already turns into a `worker-exit`; every other errno throws SYNCHRONOUSLY from `spawn`. A `pythonBin` longer than the platform PATH_MAX passes the load-time validation (non-empty, no NUL) but makes `spawn` throw `ENAMETOOLONG` here — outside the executor — so `run()` REJECTED instead of resolving, violating resolve-don't-reject, and left this run's just-materialized staging directory on disk since only `settle()` removes it. The `spawn` call and the fd-3 narrowing are now wrapped: a synchronous throw removes the staging directory and resolves the same `worker-exit` class (`python spawn error: …`) the async `error` event produces. + +### Stray pipe output is aggregated by line, not by transport chunk + +Also in `src/index.ts`, native stdout/stderr bytes (C-extension writes, `os.write` past the pipe buffer) were pushed to `logs` one entry per Node `data` chunk. `logs` entries are joined with `\n` downstream (Code Mode), so a single newline-free write larger than one pipe read — arriving as several `data` chunks — read back with model-visible newlines inserted at arbitrary transport boundaries. Capture now holds a per-stream residual, admits an entry only on a real `\n`, and flushes the trailing partial once on the pipe's `end`, matching the child's own line-granular `log` frames. The residual stays bounded by the ledger: when it would cross the budget with no newline in sight it is admitted (and truncated) immediately, and once the ledger has truncated, buffering stops so a newline-free flood cannot retain host memory for output that can never be admitted. ## Testing -- `tests/boot-write-failure.spec.ts` mocks `spawn` so the fd-3 pipe throws on the boot write — the one path a real subprocess cannot be coerced into — and asserts `run()` resolves a `worker-exit` rather than rejecting. Isolated in its own spec so the real-subprocess suite is untouched. +- `tests/boot-write-failure.spec.ts` mocks `spawn` so the fd-3 pipe throws on the boot write — the one path a real subprocess cannot be coerced into — and asserts `run()` resolves a `worker-exit` rather than rejecting. A sibling case makes the mocked `spawn` throw SYNCHRONOUSLY and asserts `run()` still resolves a `worker-exit` and leaves no `dsh-code-runtime-python-*` directory behind in `tmpdir` (before/after diff). Both are isolated in this spec so the real-subprocess suite is untouched. - `tests/residual-detach.spec.ts` unit-tests `detachResidual`: the carried copy equals the residual, owns a backing store sized to its own length (fixture kept above Node's Buffer pool threshold), and does not share the source frame's `ArrayBuffer`. -- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. A dispose-after-resolve case asserts `dispose()` of a completed run with a same-group survivor returns only after the survivor stops executing (proving the run stays in `live` until its group is reaped). A deadline case busy-blocks the event loop past both timers and asserts the survivor's heartbeat freezes (proving the poll's deadline arm sends SIGKILL itself rather than cancelling the unfired escalation). The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out; a companion case abandons a thread's call so its loop closes, then answers it before a later binding — asserting the pump survives the closed-loop `call_soon_threadsafe` (host-gated ordering makes it deterministic, fail-before hangs the later binding to the wall clock). The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`); a companion inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. A control-heavy-diagnostic case raises a NUL-flood exception under a small `maxValueBytes` and asserts the serialized frame fits (proving the diagnostic is metered by serialized cost). A non-integer-budget case asserts a fractional `maxLogBytes`/`maxValueBytes` rejects at load. +- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. A native-write case writes 200 KiB with no newline via `os.write` under a raised `maxLogBytes` and asserts it reads back as EXACTLY one log entry (proving stray output is aggregated by line, not split at pipe-chunk boundaries); a companion writes `b"one\ntwo\nthree"` and asserts three entries (proving real newlines still delimit). The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. A dispose-after-resolve case asserts `dispose()` of a completed run with a same-group survivor returns only after the survivor stops executing (proving the run stays in `live` until its group is reaped), with an `expect(afterDispose).toBeGreaterThan(0)` guard so the frozen-heartbeat assertion cannot pass vacuously when the file was never written. A deadline case busy-blocks the event loop past both timers and asserts the survivor's heartbeat freezes (proving the poll's deadline arm sends SIGKILL itself rather than cancelling the unfired escalation). The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out; a companion case abandons a thread's call so its loop closes, then answers it before a later binding — asserting the pump survives the closed-loop `call_soon_threadsafe` (host-gated ordering makes it deterministic, fail-before hangs the later binding to the wall clock). The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`); a companion inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. A control-heavy-diagnostic case raises a NUL-flood exception under a small `maxValueBytes` and asserts the serialized frame fits (proving the diagnostic is metered by serialized cost). A non-integer-budget case asserts a fractional `maxLogBytes`/`maxValueBytes` rejects at load. ## Alternatives considered @@ -72,6 +82,10 @@ Also in `py/bootstrap.py`, a binding reply Future is created on the loop that ra **Clamp rlimits by the inherited hard limit only.** Rejected: that silently RAISES an inherited soft limit stricter than the request, loosening the very containment the clamp exists to preserve. Clamping each side against its own inherited bound (then pinning soft under hard) keeps the strictest of configured and inherited on both. +**Bill the host-side `capMessage` backstop by serialized cost, matching the child's `_cap_message`.** Rejected: the two caps guard different things. `_cap_message`'s output re-crosses fd 3 as a JSON string, so its escaped width is what the frame ceiling bounds — serialized billing is required there. `capMessage`'s output goes straight into `CodeRunResult.error.message` and never re-crosses a frame-bounded channel, so the honest measure of what it retains is the raw byte length of the model-visible string. An honest child has already capped by serialized cost and raw length ≤ serialized cost, so a well-formed message passes unchanged; a forged control-heavy message could serialize to ~6× its raw length, but since it travels no capped channel, billing it by that inflated wire width would truncate a legitimately-sized diagnostic for no containment gain. Each side's JSDoc documents the split and points at the other. + +**Push stray pipe output one entry per `data` chunk.** Rejected: `logs` entries are joined with `\n` downstream, so a transport chunk boundary would become a model-visible newline — a single native write split across pipe reads would read back with spurious line breaks. Aggregating by real newline (residual + flush on `end`) matches the child's line-granular `log` frames; the ledger still bounds a newline-free flood by admitting-and-truncating the residual when it would cross the budget. + ## Consequences -The seam's resolve-don't-reject contract holds on the boot-write path with measured coverage. Log capture is thread-safe at the cost of one re-entrant lock acquisition per write and flush. Fd-3 residual memory is bounded by the actual retained bytes. The output caps admit every value a frame can carry. Disposal is genuinely quiescent against a same-group survivor — bounded by the existing grace budget, zero-cost when the group is already empty, with the SIGKILL timer cleared once the group empties so a stale kill cannot strike a recycled pgid — RLIMIT enforcement keeps the strictest of configured and inherited on both soft and hard, bindings called from model-created threads complete instead of timing out, and the handshake frame reader no longer burns the CPU budget on a large program. Each fix carries a test that fails without it (except the chunked frame read, a syscall-count improvement with no cross-platform-deterministic failure to assert), so a future regression on the rest goes red. +The seam's resolve-don't-reject contract holds on the boot-write path and the synchronous-spawn-failure path, both with measured coverage, and neither strands a staging directory. Log capture is thread-safe at the cost of one re-entrant lock acquisition per write and flush, and stray native output is delimited by its own newlines rather than by transport chunks. Fd-3 residual memory is bounded by the actual retained bytes, and both frame readers scan an accumulating frame once rather than quadratically. The output caps admit every value a frame can carry and reject a non-integer budget at load. Disposal is genuinely quiescent against a same-group survivor — bounded by `graceMs + 2 * CLOSE_REAP_MARGIN_MS`, zero-cost when the group is already empty, with the SIGKILL timer cleared once the group empties so a stale kill cannot strike a recycled pgid — RLIMIT enforcement keeps the strictest of configured and inherited on both soft and hard (and the SIGXCPU diagnostic no longer names a budget the host cannot guarantee), bindings called from model-created threads complete instead of timing out, and the handshake frame reader no longer burns the CPU budget on a large program. Every behavioral fix carries a test that fails without it, except the two called out in the Problem section — the chunked frame read (a syscall-count improvement) and the confirmed-empty finalize (whose only seam-observable effect freezes at signal delivery, which the pre-fix code also produced) — so a future regression on the rest goes red. diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md index fa72c434e8..8aa0e61f88 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md @@ -6,11 +6,11 @@ Status: implemented ## Problem -用于 Code Mode 的 CPython 子进程后端建立在 [fd-3 帧协议](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md)之上,把每个程序结果都 resolve 成一个 `CodeRunResult`,仅在 seam 被误用时才 reject `run()`,并且会 dispose 到完全停稳,从而没有任何留在子进程自己进程组内的子进程存活得比 fiber 更久(一个用 `setsid()` 逃出该进程组的后代是有文档记载的例外——见该包 README 的 Known Limitations)。一连串审查暴露出一些缺陷,它们以单元测试覆盖率无法捕获的方式破坏了这些契约:每一个都藏在一处 `/* v8 ignore */` 之后、一个读起来像修复但实际并非修复的捕获可调用对象之后、一处透过 seam 不可见的内存效应之后、一处重复计数的加载期上界之后、一处存活者能够熬过的进程组升级之后,或者一处静默死锁的跨事件循环完成之后。每处行为修复都附带一个在缺少它时会失败的测试;唯一的例外是一处系统调用次数的改进(分块读取帧),它没有可跨平台确定性断言的失败可供断言。 +用于 Code Mode 的 CPython 子进程后端建立在 [fd-3 帧协议](../architecture/2026-07-31-code-runtime-python-fd3-protocol.md)之上,把每个程序结果都 resolve 成一个 `CodeRunResult`,仅在 seam 被误用时才 reject `run()`,并且会 dispose 到完全停稳,从而没有任何留在子进程自己进程组内的子进程存活得比 fiber 更久(一个用 `setsid()` 逃出该进程组的后代是有文档记载的例外——见该包 README 的 Known Limitations)。一连串审查暴露出一些缺陷,它们以单元测试覆盖率无法捕获的方式破坏了这些契约:每一个都藏在一处 `/* v8 ignore */` 之后、一个读起来像修复但实际并非修复的捕获可调用对象之后、一处透过 seam 不可见的内存效应之后、一处重复计数的加载期上界之后、一处存活者能够熬过的进程组升级之后、一处静默死锁的跨事件循环完成之后、一处位于结算路径之外的同步抛出之后,或者一处被当作日志边界处理的传输边界之后。大多数行为修复都附带一个在缺少它时会失败的测试;有两处没有,并被如此标注——分块读取帧(一处系统调用次数的改进,没有可跨平台确定性断言的失败),以及确认为空后的收尾(它唯一透过 seam 可观测的效应,即一个冻结的心跳,会在 SIGKILL 被投递的瞬间冻结,而修复前"投递即收尾"的代码也会产生同样的结果,用于区分的探测手段是 Alternatives 以跨环境不可靠为由否决的 signal-0 检查)。 ## Decision -八处相互独立的修正,各自位于拥有对应缺陷的包中。 +若干处相互独立的修正,各自位于拥有对应缺陷的包中。 ### Boot-write failure no longer rejects run() @@ -26,7 +26,7 @@ Status: implemented ### Output-cap load bound is ceiling minus envelope, not divided by six -那处在加载期拒绝比单个 fd-3 帧所能承载更大的 `maxLogBytes`/`maxValueBytes` 的检查,会把帧上限除以六以应对最坏情况下的转义膨胀。但这两项预算都是以已转义的序列化字节来计量的:宿主日志账本按 `Buffer.byteLength(JSON.stringify(text))` 计费,而 `checkDoneValue` 度量的是转义后的形式,因此一个在上限之内被放行的载荷在传输时最多占用 `cap + envelope`;转义已经包含在计费之内,不能再被乘一次。现在该上界为 `FRAME_CEILING_BYTES - FRAME_ENVELOPE_BYTES`,未使用的 `MAX_JSON_ESCAPE_EXPANSION` 常量已被删除。旧的上界并非不安全(它是放行不足),但它静默地禁止了合法的大上限。 +那处在加载期拒绝比单个 fd-3 帧所能承载更大的 `maxLogBytes`/`maxValueBytes` 的检查,会把帧上限除以六以应对最坏情况下的转义膨胀。但这两项预算都是以已转义的序列化字节来计量的:宿主日志账本按 `Buffer.byteLength(JSON.stringify(text))` 计费,`checkDoneValue` 度量的是转义后的形式,而生产侧的 `_cap_message` 同样按序列化开销设上限,因此一个在上限之内被放行的载荷在传输时最多占用 `cap + envelope`;转义已经包含在计费之内,不能再被乘一次。现在该上界为 `FRAME_CEILING_BYTES - FRAME_ENVELOPE_BYTES`,未使用的 `MAX_JSON_ESCAPE_EXPANSION` 常量已被删除。旧的上界并非不安全(它是放行不足),但它静默地禁止了合法的大上限。这同一处加载检查还会拒绝一个非整数的 `maxLogBytes`/`maxValueBytes`:子进程通过 `int(...)` 读取每一项预算,而 `int(...)` 会对浮点数向下取整,因此 `maxLogBytes: 3.5` 会在子进程侧截断在 3 字节,而宿主却把小数部分也计入——两侧因此强制着不同的公开配置。在加载期拒绝该浮点数使两侧保持一致,与 worker 后端相符。 ### Same-group survivors are reaped before the fiber goes quiescent @@ -34,9 +34,11 @@ Status: implemented 结算还会在进程组被确认为空的那一刻取消 SIGKILL 定时器(正常路径,以及轮询看到存活者已消失时)。让它继续处于装设状态会暴露一个 PID 复用隐患:一个在 leader 被回收后仍挂起长达 `graceMs` 的 `kill(-pid)`,可能在内核复用了 leader 的 pid 之后击中一个被回收(recycled)的 pgid,从而 SIGKILL 掉一个无关的进程组(`killGroup` 吞掉 ESRCH 并无帮助——危险恰恰是那次针对被复用进程组成功执行的 kill)。在空进程组探测时清除它,把复用窗口收窄到只剩真正存在存活者的情形,此时进程组不可能为空以供复用。 +回收轮询还会处理宿主事件循环被阻塞、越过两个定时器的情形。如果一次同步计算从轮询被调度之前一直占住事件循环、直到越过它的截止时间,那么当事件循环恢复时,轮询定时器和宽限窗口的 SIGKILL 定时器都已逾期,而 Node 会先运行更早调度的轮询——因此宽限窗口的 SIGKILL 可能从未触发。为此截止时间分支会自己发送 SIGKILL(若定时器已运行则该操作幂等),而不是取消尚未触发的升级,随后再额外给予一个 `CLOSE_REAP_MARGIN_MS`,并持续轮询直到进程组被确认为空,因为仅凭信号投递就收尾会在进程组仍在消亡时宣告完全停稳。因此等待的外层上界为 `graceMs + 2 * CLOSE_REAP_MARGIN_MS`。若这段额外余量耗尽而进程组仍非空,一个最终的硬性上界会收尾;该分支带有一处 `/* v8 ignore */`,因为它仅在一个被 SIGKILL 的存活者作为僵尸进程滞留且从未被 `wait()`——一个 PID 1 不回收孤儿进程的容器——时才可达,而这无法在各 CI 平台上确定性地构造出来。该 ignore 的理由陈述的是这种环境依赖性,而不是声称该分支不可能运行,并交叉引用 Alternatives 中以同样理由否决 signal-0 回收断言的那一条。 + ### RLIMIT clamps against the inherited soft limit, not only the hard -在 [`py/bootstrap.py`](../../../../packages/code-runtime/code-runtime-python/py/bootstrap.py) 中,`_clamped` 仅用继承而来的 HARD 限制来约束一个请求的 `(soft, hard)` rlimit 对。一个继承了低于请求值的软限制的部署——比如继承 `(100, 200)`、请求 `(150, 160)`——会拿回 `(150, 160)`,把有效软限制从 100 抬高到 150:对 `RLIMIT_AS` 而言这放松了内存上限,对 `RLIMIT_CPU` 而言它推迟了 SIGXCPU,两者都违反了"取配置值与继承值中最严格者"。现在 `_clamped` 用每一侧各自继承而来的对应值来约束该侧(`RLIM_INFINITY` 不施加任何上限),随后把 soft 钉在 hard 之下,因此 `setrlimit` 绝不会看到一个倒置的对。结算时的 CPU 复查(`die_if_cpu_exhausted`)遵循同一规则:它把已消耗的 CPU 与实际生效的、被夹紧的 `cpu_soft` 比较,而不是与配置的 `cpuSeconds` 比较,因此一个捕获 SIGXCPU、在返回前消耗超过更严格的继承软限制的程序会被报告为 timeout,而非误判为成功。 +在 [`py/bootstrap.py`](../../../../packages/code-runtime/code-runtime-python/py/bootstrap.py) 中,`_clamped` 仅用继承而来的 HARD 限制来约束一个请求的 `(soft, hard)` rlimit 对。一个继承了低于请求值的软限制的部署——比如继承 `(100, 200)`、请求 `(150, 160)`——会拿回 `(150, 160)`,把有效软限制从 100 抬高到 150:对 `RLIMIT_AS` 而言这放松了内存上限,对 `RLIMIT_CPU` 而言它推迟了 SIGXCPU,两者都违反了"取配置值与继承值中最严格者"。现在 `_clamped` 用每一侧各自继承而来的对应值来约束该侧(`RLIM_INFINITY` 不施加任何上限),随后把 soft 钉在 hard 之下,因此 `setrlimit` 绝不会看到一个倒置的对。结算时的 CPU 复查(`die_if_cpu_exhausted`)遵循同一规则:它把已消耗的 CPU 与实际生效的、被夹紧的 `cpu_soft` 比较,而不是与配置的 `cpuSeconds` 比较,因此一个捕获 SIGXCPU、在返回前消耗超过更严格的继承软限制的程序会被报告为 timeout,而非误判为成功。SIGXCPU 诊断不再把配置的 `cpuSeconds` 说成实际生效的预算——在一个更严格的继承软限制之下那个数字是错的——而是报告 CPU 时间是在"至多配置的 N 秒"处被耗尽,这一表述无论哪个限制先触发都成立。 ### Binding replies complete on the calling loop's thread @@ -44,13 +46,21 @@ Status: implemented ### The blocking frame reader reads in chunks, not byte by byte -`ProtocolChannel.read_frame`(用于 `boot` 和 `run` 握手帧)过去通过在无缓冲(`buffering=0`)fd 上的 `FileIO.readline()` 读取,这会为每个字节发起一次 `os.read(1)`。`run` 帧在 `RLIMIT_CPU` 生效之后才到达,因此一个合法的数兆字节程序会在 `ast.parse` 运行之前,在数以百万计的单字节系统调用中烧掉数秒 CPU——有可能仅在读取这一步就耗尽预算。现在它以 `_READ_CHUNK_BYTES` 为单位分块读取,写入异步读取器已经使用的那同一个 `_pending` 残余缓冲区(包裹用的 `os.fdopen` 对象已被移除;两个读取器都直接调用 `os.read(self._fd, ...)`),因此读取开销微不足道,并且越过换行符的预读也为下一帧保留了下来。 +`ProtocolChannel.read_frame`(用于 `boot` 和 `run` 握手帧)过去通过在无缓冲(`buffering=0`)fd 上的 `FileIO.readline()` 读取,这会为每个字节发起一次 `os.read(1)`。`run` 帧在 `RLIMIT_CPU` 生效之后才到达,因此一个合法的数兆字节程序会在 `ast.parse` 运行之前,在数以百万计的单字节系统调用中烧掉数秒 CPU——有可能仅在读取这一步就耗尽预算。现在它以 `_READ_CHUNK_BYTES` 为单位分块读取,写入异步读取器已经使用的那同一个 `_pending` 残余缓冲区(包裹用的 `os.fdopen` 对象已被移除;两个读取器都直接调用 `os.read(self._fd, ...)`),因此读取开销微不足道,并且越过换行符的预读也为下一帧保留了下来。两个读取器都跟踪一个持续推进的扫描偏移(`find(b"\n", scanned)`),使一个跨多个分块累积起来的大帧只被扫描一次,而不是每来一个分块就从索引 0 重新扫描——分块式重扫会把逐字节的开销换成同一大帧路径上 O(N²) 的 memchr 开销。 + +### Synchronous spawn failure resolves worker-exit, not reject + +同样在 `src/index.ts` 中,`spawn` 是在结算 Promise 的 executor 存在之前被调用的。Node 只把一组固定的 spawn errno(EACCES、EAGAIN、EMFILE、ENFILE、ENOENT)推迟为一个异步的 `error` 事件,而结算路径已经把它转成一个 `worker-exit`;其余每一个 errno 都会从 `spawn` 同步抛出。一个长度超过平台 PATH_MAX 的 `pythonBin` 能通过加载期校验(非空、无 NUL),却会让 `spawn` 在此处抛出 `ENAMETOOLONG`——在 executor 之外——因此 `run()` 会 reject 而不是 resolve,违反了"只 resolve、不 reject",并且由于只有 `settle()` 才会移除本次运行刚物化出来的暂存目录,它会把该目录留在磁盘上。现在 `spawn` 调用和 fd-3 收窄被包裹起来:一次同步抛出会移除暂存目录,并 resolve 与异步 `error` 事件所产生的同一类 `worker-exit`(`python spawn error: …`)。 + +### Stray pipe output is aggregated by line, not by transport chunk + +同样在 `src/index.ts` 中,原生 stdout/stderr 字节(C 扩展写入、越过管道缓冲区的 `os.write`)过去每来一个 Node `data` 分片就被推入 `logs` 一条条目。`logs` 条目在下游(Code Mode)会用 `\n` 拼接,因此一次大于单次管道读取、且不含换行符的写入——它以若干个 `data` 分片到达——回读时会在任意传输边界处被插入模型可见的换行符。现在捕获会为每个流持有一份残余数据,仅在遇到真正的 `\n` 时才准入一条条目,并在管道 `end` 时一次性冲刷尾部的不完整部分,与子进程自己的按行粒度的 `log` 帧相符。该残余数据仍受账本约束:当它在看不到换行符的情况下将要越过预算时,会被立即准入(并截断);而一旦账本已经截断,缓冲便停止,从而一场不含换行符的洪泛无法为永远无法被准入的输出保留宿主内存。 ## Testing -- `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock,使 fd-3 管道在引导写入时抛出异常(这是真实子进程无法被迫进入的唯一路径),并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。它被隔离在自己的 spec 中,因此真实子进程测试套件不受影响。 +- `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock,使 fd-3 管道在引导写入时抛出异常(这是真实子进程无法被迫进入的唯一路径),并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。一个同级用例让被 mock 的 `spawn` 同步抛出,并断言 `run()` 仍然 resolve 出一个 `worker-exit`,且不会在 `tmpdir` 中留下任何 `dsh-code-runtime-python-*` 目录(前后差分)。两者都被隔离在这个 spec 中,因此真实子进程测试套件不受影响。 - `tests/residual-detach.spec.ts` 对 `detachResidual` 做单元测试:向前传递的副本与残余数据相等、拥有一个大小与其自身长度一致的底层存储(fixture 保持在 Node 的 Buffer 池阈值之上),并且不与源帧的 `ArrayBuffer` 共享。 -- `tests/runtime.spec.ts`:output-cap 用例断言 `ceiling - envelope` 上界(268435392)及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代,它释放管道并递增一个心跳文件;该测试断言在宽限窗口的 SIGKILL 之后心跳停止:无论被杀死的后代是被回收还是作为僵尸进程滞留,这个断言都成立,因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。一个 dispose-after-resolve 用例断言,对一个已完成、且存在同进程组存活者的运行调用 `dispose()`,只有在该存活者停止执行之后才返回(证明该运行会一直留在 `live` 中,直到它的进程组被回收)。一个 deadline 用例忙阻塞事件循环越过两个定时器,断言该存活者的心跳冻结(证明轮询的截止时间分支自身发送 SIGKILL,而不是取消尚未触发的升级)。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时,从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定,断言该回复完成往返而不是超时;一个配套用例放弃某个线程的调用,使其事件循环关闭,随后在一个后续绑定之前回答它——断言 pump 在关闭事件循环上的 `call_soon_threadsafe` 之后仍然存活(由宿主门控的顺序使其具有确定性,未修复时会把后续绑定拖到墙钟上挂起)。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器,将 CPU 软限制设为低于 `cpuSeconds`,并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值,而不是配置的值(用 CPU 而非地址空间,因为 macOS 忽略 `ulimit -v`)。一个配套用例继承 1 秒的 CPU 软限制,让程序捕获 SIGXCPU 并忙循环越过它,断言结算复查报告 timeout——证明复查用的是实际生效的软限制,而不是配置的 `cpuSeconds`。一个 control-heavy-diagnostic 用例在一个较小的 `maxValueBytes` 之下抛出一个 NUL 洪泛异常,断言序列化后的帧能放得下(证明该诊断是按序列化开销计量的)。一个 non-integer-budget 用例断言一个小数的 `maxLogBytes`/`maxValueBytes` 在加载期被拒绝。 +- `tests/runtime.spec.ts`:output-cap 用例断言 `ceiling - envelope` 上界(268435392)及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。一个 native-write 用例在抬高后的 `maxLogBytes` 之下,通过 `os.write` 写入 200 KiB 且不含换行符,断言它回读时恰好是一条日志条目(证明散逸输出是按行聚合的,而不是在管道分片边界处被切开);一个配套用例写入 `b"one\ntwo\nthree"`,断言得到三条条目(证明真正的换行符仍然起分隔作用)。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代,它释放管道并递增一个心跳文件;该测试断言在宽限窗口的 SIGKILL 之后心跳停止:无论被杀死的后代是被回收还是作为僵尸进程滞留,这个断言都成立,因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。一个 dispose-after-resolve 用例断言,对一个已完成、且存在同进程组存活者的运行调用 `dispose()`,只有在该存活者停止执行之后才返回(证明该运行会一直留在 `live` 中,直到它的进程组被回收),并带有一个 `expect(afterDispose).toBeGreaterThan(0)` 守卫,使得当心跳文件从未被写入时,冻结心跳的断言不会被空洞地通过。一个 deadline 用例忙阻塞事件循环越过两个定时器,断言该存活者的心跳冻结(证明轮询的截止时间分支自身发送 SIGKILL,而不是取消尚未触发的升级)。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时,从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定,断言该回复完成往返而不是超时;一个配套用例放弃某个线程的调用,使其事件循环关闭,随后在一个后续绑定之前回答它——断言 pump 在关闭事件循环上的 `call_soon_threadsafe` 之后仍然存活(由宿主门控的顺序使其具有确定性,未修复时会把后续绑定拖到墙钟上挂起)。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器,将 CPU 软限制设为低于 `cpuSeconds`,并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值,而不是配置的值(用 CPU 而非地址空间,因为 macOS 忽略 `ulimit -v`)。一个配套用例继承 1 秒的 CPU 软限制,让程序捕获 SIGXCPU 并忙循环越过它,断言结算复查报告 timeout——证明复查用的是实际生效的软限制,而不是配置的 `cpuSeconds`。一个 control-heavy-diagnostic 用例在一个较小的 `maxValueBytes` 之下抛出一个 NUL 洪泛异常,断言序列化后的帧能放得下(证明该诊断是按序列化开销计量的)。一个 non-integer-budget 用例断言一个小数的 `maxLogBytes`/`maxValueBytes` 在加载期被拒绝。 ## Alternatives considered @@ -72,6 +82,10 @@ Status: implemented **只用继承而来的硬限制来约束 rlimit。** 已否决:那会静默地抬高一个比请求更严格的继承软限制,放松了该约束本应保持的那种收束。用每一侧各自继承而来的界来约束该侧(随后把 soft 钉在 hard 之下),在 soft 和 hard 两者上都保持配置值与继承值中的最严格者。 +**按序列化开销对宿主侧的 `capMessage` 兜底做计费,与子进程的 `_cap_message` 相符。** 已否决:这两处上限守护的是不同的东西。`_cap_message` 的输出会作为一个 JSON 字符串再次穿过 fd 3,因此帧上限约束的是它转义后的宽度——那里必须按序列化计费。`capMessage` 的输出直接进入 `CodeRunResult.error.message`,绝不会再次穿过一个受帧上限约束的通道,因此对它所保留内容的诚实度量是模型可见字符串的原始字节长度。一个诚实的子进程已经按序列化开销设过上限,而原始长度 ≤ 序列化开销,因此一条格式良好的消息会原样通过;一条伪造的、控制字符密集的消息可能序列化到其原始长度约 6 倍,但由于它不经过任何受上限约束的通道,按那个被抬高的传输宽度对它计费只会截断一条尺寸合法的诊断,而换不来任何收束上的收益。每一侧的 JSDoc 都记录了这一区分,并指向另一侧。 + +**每来一个 `data` 分片就把散逸的管道输出推入一条条目。** 已否决:`logs` 条目在下游会用 `\n` 拼接,因此一个传输分片边界会变成一个模型可见的换行符——一次被拆散在多次管道读取中的原生写入会带着无端的换行回读。按真正的换行符聚合(残余数据 + 在 `end` 时冲刷)与子进程的按行粒度的 `log` 帧相符;账本仍然通过在残余数据将要越过预算时把它准入并截断,来约束一场不含换行符的洪泛。 + ## Consequences -seam 的"只 resolve、不 reject"契约在引导写入路径上得以成立,且覆盖率是被度量的。日志捕获是线程安全的,代价是每次写入和 flush 都要获取一次可重入锁。fd-3 残余数据的内存受实际保留的字节数约束。输出上限放行一个帧所能承载的每一个值。dispose 面对同进程组存活者是真正完全停稳的(以既有的宽限预算为界,在进程组已为空时代价为零,并且一旦进程组清空就清除 SIGKILL 定时器,从而一次滞留的 kill 无法击中一个被回收的 pgid),RLIMIT 强制在 soft 和 hard 两者上都保持配置值与继承值中的最严格者,并且从模型创建的线程调用的绑定会完成而不是超时,而且握手帧读取器不再在一个大程序上烧掉 CPU 预算。每处修复都附带一个在缺少它时会失败的测试(分块读取帧除外,它是一处系统调用次数的改进,没有可跨平台确定性断言的失败),因此其余各处未来若发生回归都会变红。 +seam 的"只 resolve、不 reject"契约在引导写入路径和同步 spawn 失败路径上都得以成立,两者的覆盖率都是被度量的,且两者都不会遗留一个暂存目录。日志捕获是线程安全的,代价是每次写入和 flush 都要获取一次可重入锁,并且散逸的原生输出由它自己的换行符来分隔,而不是由传输分片来分隔。fd-3 残余数据的内存受实际保留的字节数约束,并且两个帧读取器都以一次而非平方级的方式扫描一个不断累积的帧。输出上限放行一个帧所能承载的每一个值,并在加载期拒绝一个非整数的预算。dispose 面对同进程组存活者是真正完全停稳的(以 `graceMs + 2 * CLOSE_REAP_MARGIN_MS` 为界,在进程组已为空时代价为零,并且一旦进程组清空就清除 SIGKILL 定时器,从而一次滞留的 kill 无法击中一个被回收的 pgid),RLIMIT 强制在 soft 和 hard 两者上都保持配置值与继承值中的最严格者(并且 SIGXCPU 诊断不再把一个宿主无法保证的预算说出来),并且从模型创建的线程调用的绑定会完成而不是超时,而且握手帧读取器不再在一个大程序上烧掉 CPU 预算。每处行为修复都附带一个在缺少它时会失败的测试,除了 Problem 一节点出的那两处——分块读取帧(一处系统调用次数的改进),以及确认为空后的收尾(它唯一透过 seam 可观测的效应会在信号投递时冻结,而修复前的代码也会产生同样的结果)——因此其余各处未来若发生回归都会变红。 diff --git a/packages/code-runtime/code-runtime-python/py/bootstrap.py b/packages/code-runtime/code-runtime-python/py/bootstrap.py index 3b850f995d..579da24800 100644 --- a/packages/code-runtime/code-runtime-python/py/bootstrap.py +++ b/packages/code-runtime/code-runtime-python/py/bootstrap.py @@ -384,12 +384,17 @@ class ProtocolChannel: past a newline for the next frame. """ + # Scan only the bytes not yet examined: `find` from a running offset so a + # frame arriving in N chunks costs one linear pass total, not one rescan + # of the whole buffer per chunk (which is quadratic in the frame size). + scanned = 0 while True: - newline = self._pending.find(b"\n") + newline = self._pending.find(b"\n", scanned) if newline >= 0: line = bytes(self._pending[:newline]) del self._pending[: newline + 1] return _decode_json_plain(line.decode("utf-8")) + scanned = len(self._pending) chunk = os.read(self._fd, _READ_CHUNK_BYTES) if not chunk: # EOF before a newline: drop the partial line, as the host drops @@ -421,12 +426,17 @@ class ProtocolChannel: """ loop = asyncio.get_event_loop() + # Scan only the not-yet-examined bytes (running offset), so a frame + # arriving across many reads costs one linear pass, not a quadratic + # rescan of the whole buffer per read. + scanned = 0 while True: - newline = self._pending.find(b"\n") + newline = self._pending.find(b"\n", scanned) if newline >= 0: line = bytes(self._pending[:newline]) del self._pending[: newline + 1] return _decode_json_plain(line.decode("utf-8")) + scanned = len(self._pending) ready = loop.create_future() # `add_reader` only reports readability; the read itself happens here, # and `os.read` returns whatever is buffered without waiting for more. @@ -912,7 +922,10 @@ async def _pump_replies( # Future and the reply is moot. Drop it; scheduling onto a closed # loop raises RuntimeError, and letting that escape would kill the # pump and strand every later reply — the exact failure class this - # cross-loop delivery exists to prevent. + # cross-loop delivery exists to prevent. An abandoned call's pending + # entry is not leaked: it is popped here when its reply arrives + # (dispatch's cancellation does not remove it), so stranded entries + # are bounded by the number of calls THIS run itself issued. continue @@ -1603,6 +1616,11 @@ def _cap_message(message: str, max_bytes: int) -> str: serialized cost comes OUT of ``max_bytes``, so the returned string's own frame form honors the cap; the host meters the same field again on arrival. A ``max_bytes`` below the marker's cost yields the marker alone. + + This is the PRODUCING-side cap. The host's receive-side ``capMessage`` + (``src/index.ts``) bills the same field by RAW bytes instead, because its + output goes into ``CodeRunResult.error.message`` and never re-crosses a + frame-bounded channel — see that function's JSDoc for the split. """ raw = message.encode("utf-8", errors="replace") @@ -1616,7 +1634,7 @@ def _cap_message(message: str, max_bytes: int) -> str: # at most a budget's worth of bytes, allocating nothing (unlike building the # escaped form). `max(0, ...)` handles a `max_bytes` below the marker's own # cost, yielding the marker alone. - content_budget = max(0, max_bytes - 2 - len(_TRUNCATION_MARKER.encode("utf-8"))) + content_budget = max(0, max_bytes - 2 - _TRUNCATION_MARKER_BYTES) cost = 0 end = 0 for end in range(len(raw)): diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index 50ac14f6cc..ebee63e9c4 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -12,7 +12,7 @@ * @module @deepseek-ai/dsh-code-runtime-python */ -import { spawn } from 'node:child_process' +import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { StringDecoder } from 'node:string_decoder' import { accessSync, copyFileSync, constants as fsConstants, mkdtempSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -315,10 +315,24 @@ const TRUNCATION_MARKER_BYTES = Buffer.byteLength(TRUNCATION_MARKER, 'utf8') /** * Cap a done-frame `error.message` to `maxValueBytes` host-side: a forged done - * frame can carry an arbitrarily long message, so truncate by byte length and - * append the shared marker on overflow. Completion VALUES are never truncated - * — the seam forbids substitution, so an oversized value fails the run as - * `output-limit` instead (see the done case in `execute`). + * frame can carry an arbitrarily long message, so truncate by RAW UTF-8 byte + * length and append the shared marker on overflow. Completion VALUES are never + * truncated — the seam forbids substitution, so an oversized value fails the run + * as `output-limit` instead (see the done case in `execute`). + * + * This is the RECEIVE-side backstop, and it bills by raw bytes on purpose, + * unlike the producing-side `_cap_message` in `py/bootstrap.py`, which bills by + * SERIALIZED (JSON-escaped) cost. The split is deliberate: `_cap_message`'s + * output has to cross fd 3 as a JSON string, so its escaped width is what the + * frame ceiling bounds; this function's output goes straight into + * `CodeRunResult.error.message` and never re-crosses a frame-bounded channel, so + * the honest measure of what it retains is the raw length. An honest child has + * already capped the diagnostic by serialized cost, and raw length ≤ serialized + * cost, so a well-formed message passes through unchanged. A forged message with + * control characters could serialize to roughly six times its raw length, but it + * is not travelling any capped channel, so the raw-byte bound is the right one: + * the value it protects is the model-visible size of `error.message`, not a wire + * width. * * The marker's bytes are RESERVED from the budget, not added on top: the whole * returned string, marker included, is at most `maxValueBytes` bytes. Appending @@ -503,10 +517,16 @@ export class PythonCodeRuntime extends CodeRuntime { // arrives as an over-ceiling frame and fails the run as `worker-exit` // instead of the `output-limit` the cap describes — a silent inversion, so // it fails at load. Both budgets are metered in SERIALIZED (JSON-escaped) - // bytes — the host log ledger charges `Buffer.byteLength(JSON.stringify(text))` - // and `checkDoneValue` measures the escaped form — so a payload admitted - // under the cap occupies at most `cap + envelope` bytes on the wire; escaping - // is already inside the charge and must not be multiplied in again. The + // bytes — the host log ledger charges `Buffer.byteLength(JSON.stringify(text))`, + // `checkDoneValue` measures the escaped form, and the producing-side + // `_cap_message` in the child also caps by serialized cost (which is why a + // capped diagnostic still fits its frame) — so a payload admitted under the + // cap occupies at most `cap + envelope` bytes on the wire; escaping is + // already inside the charge and must not be multiplied in again. The + // receive-side `capMessage` backstop is the one exception to this argument: + // it bills a forged `done.error.message` by RAW bytes, but that output goes + // into `CodeRunResult.error.message` and never re-crosses a frame-bounded + // channel, so it is not part of the wire-width bound (see its JSDoc). The // admissible cap is therefore `ceiling - envelope`. for (const key of ['maxLogBytes', 'maxValueBytes'] as const) { // Require an integer: the child reads these budgets through `int(...)`, @@ -639,20 +659,38 @@ export class PythonCodeRuntime extends CodeRuntime { // Explicit pipe count of 4 puts the framed-JSON channel at fd 3 in the child. // Resolve the interpreter against the current PATH first: the child's empty // env would otherwise strip PATH and miss a basename python3 (see resolvePythonBin). - const child = spawn(resolvePythonBin(this.config.pythonBin), ['-I', bootstrapPath], { - env: {}, - detached: true, // Own process group — kill(-pid, sig) reaches subprocesses the model program spawns. - stdio: ['pipe', 'pipe', 'pipe', 'pipe'], - }) - - // Fd 3 is a duplex pipe carrying protocol frames. Node types extra stdio - // entries as `Stream | null`; the runtime shape with `'pipe'` is a duplex, - // so we narrow at the boundary rather than smearing casts below. Stdout - // and stderr are guaranteed non-null under `'pipe'` and typed as such. - const proto = child.stdio[3] as Duplex | null - /* v8 ignore next 3 -- `'pipe'` stdio always populates fd 3; guarding Node's `Stream | null` typing widening. */ - if (proto === null) { - throw new Error('dsh-code-runtime-python: python subprocess spawned without a fd-3 pipe') + // `spawn` can throw SYNCHRONOUSLY — a descriptor-exhausted host (EMFILE) or a + // libuv-level failure surfaces here, before the Promise executor and its + // settlement path exist. Left uncaught it would REJECT run() (the seam + // permits rejection only for misuse) and strand this run's staging directory, + // which only settle() removes. Catch it, unlink the directory, and resolve a + // `worker-exit` — the same class as the async ENOENT `error` event below. + let child: ChildProcessWithoutNullStreams + let proto: Duplex | null + try { + child = spawn(resolvePythonBin(this.config.pythonBin), ['-I', bootstrapPath], { + env: {}, + detached: true, // Own process group — kill(-pid, sig) reaches subprocesses the model program spawns. + stdio: ['pipe', 'pipe', 'pipe', 'pipe'], + }) + // Fd 3 is a duplex pipe carrying protocol frames. Node types extra stdio + // entries as `Stream | null`; the runtime shape with `'pipe'` is a duplex, + // so we narrow at the boundary rather than smearing casts below. Stdout + // and stderr are guaranteed non-null under `'pipe'` and typed as such. + proto = child.stdio[3] as Duplex | null + /* v8 ignore next 3 -- `'pipe'` stdio always populates fd 3; guarding Node's `Stream | null` typing widening. */ + if (proto === null) { + throw new Error('dsh-code-runtime-python: python subprocess spawned without a fd-3 pipe') + } + } catch (error: unknown) { + try { + rmSync(bootstrapDir, { recursive: true, force: true }) + } catch { + // Same swallow as settle()'s removal: `force` already absorbs a missing + // directory, so only a filesystem-level refusal reaches here, and the + // staging copy holds nothing but two checked-in scripts. + } + return Promise.resolve({ logs: [], error: { kind: 'worker-exit' as const, message: `python spawn error: ${messageOf(error)}` } }) } return new Promise((resolve) => { @@ -711,25 +749,52 @@ export class PythonCodeRuntime extends CodeRuntime { // replacement characters. StringDecoder holds the partial sequence // until its continuation bytes arrive; the pipes are separate byte // streams, so they cannot share one decoder. - const strayOut = new StringDecoder('utf8') - const strayErr = new StringDecoder('utf8') - const captureStray = (decoder: StringDecoder, chunk: Buffer): void => { - const text = decoder.write(chunk) - // Empty only when the chunk is nothing but a partial multibyte - // sequence — needs a pipe boundary INSIDE one character, which cannot - // be forced deterministically from the child side. - /* v8 ignore next */ - if (text.length > 0) admit(text) + // + // Output is admitted per LINE, not per transport chunk. `logs` entries + // are joined with `\n` downstream (Code Mode), so each entry must be one + // line: pushing a raw `data` chunk would turn every arbitrary pipe-read + // boundary into a model-visible newline, so a single 200 KiB native write + // split across pipe reads would read back with spurious line breaks. The + // child's own `log` frames are already line-granular; stray capture + // matches them by holding a per-stream residual and admitting only on a + // real `\n`. A run of bytes carrying no newline accumulates in the + // residual; the ledger bounds it — `admit` charges each completed line, so + // a newline-free flood is capped when the pending residual would cross the + // budget, and the trailing partial is flushed once on `end`. + const strayOut = { decoder: new StringDecoder('utf8'), residual: '' } + const strayErr = { decoder: new StringDecoder('utf8'), residual: '' } + const captureStray = (stray: { decoder: StringDecoder; residual: string }, chunk: Buffer): void => { + // Once the ledger has truncated, stop buffering: admit() is a no-op past + // that point, so continuing to grow the residual would retain host + // memory for output that can never be admitted. + if (logsTruncated) return + stray.residual += stray.decoder.write(chunk) + let newline = stray.residual.indexOf('\n') + while (newline >= 0) { + admit(stray.residual.slice(0, newline)) + stray.residual = stray.residual.slice(newline + 1) + newline = stray.residual.indexOf('\n') + } + // Newline-free residual is bounded by the ledger, not left to grow with + // the stream: an `os.write(1, b"A"*N)` flood carrying no newline would + // otherwise accumulate N bytes in host memory before `end`. When the + // pending residual would cross the budget, admit it now — admit() + // truncates and marks the ledger, and the truncation short-circuit above + // stops further buffering on the next chunk. + if (stray.residual.length + 3 > logBudget) { + admit(stray.residual) + stray.residual = '' + } } child.stdout.on('data', (chunk: Buffer) => { captureStray(strayOut, chunk) }) child.stderr.on('data', (chunk: Buffer) => { captureStray(strayErr, chunk) }) - // Flush each decoder when its pipe ends: output that STOPS mid-sequence - // (native code killed between bytes) leaves the partial character in - // the decoder, and end() renders it as U+FFFD rather than dropping the - // evidence. `end` fires before `close` settles the run, so the flush is - // admitted into `logs`. - const flushStray = (decoder: StringDecoder): void => { - const tail = decoder.end() + // Flush each pipe's residual and decoder when it ends: a final line with + // no trailing newline, plus output that STOPS mid-sequence (native code + // killed between bytes) leaves a partial character in the decoder, which + // end() renders as U+FFFD rather than dropping the evidence. `end` fires + // before `close` settles the run, so the flush is admitted into `logs`. + const flushStray = (stray: { decoder: StringDecoder; residual: string }): void => { + const tail = stray.residual + stray.decoder.end() if (tail.length > 0) admit(tail) } child.stdout.on('end', () => { flushStray(strayOut) }) @@ -1069,26 +1134,30 @@ export class PythonCodeRuntime extends CodeRuntime { // actually empty — dropping from `live` before then would let a // `dispose()` that races a just-resolved run() snapshot an empty `live` // and return while a same-group survivor is still alive, making teardown's - // "no subprocess outlives the fiber" false for that window. Keeping the - // run in `live` until the group is reaped is exactly what makes a - // concurrent teardown await it. + // "no SAME-GROUP subprocess outlives the fiber" guarantee false for that + // window (a setsid escapee is the documented exception — see teardown's + // JSDoc). Keeping the run in `live` until the group is reaped is exactly + // what makes a concurrent teardown await it. const finalize = (): void => { this.live.delete(live) finishResolve() } - // `finished` is what teardown awaits to honor "no subprocess outlives the - // fiber". When no escalation ran (normal completion, no kill) or the - // group is already empty, cancel the pending SIGKILL and finalize now. - // Clearing it is what bounds the PID-reuse hazard: an armed `kill(-pid)` - // left to fire up to graceMs later could hit a RECYCLED pgid once the - // kernel reused the leader's pid, SIGKILLing an unrelated group. So the - // timer stays armed only while a real survivor exists — a same-group - // descendant that ignored SIGTERM but released the pipes, still alive - // here because its `close` is what got us to settle. In that case - // withhold finalize and poll the group on REF'd timers (a short-lived - // host would otherwise exit before the unref'd SIGKILL fired, reparenting - // the survivor to init), clearing the timer the moment the group empties; - // the wait is bounded by the same graceMs + margin the escalation uses. + // `finished` is what teardown awaits to honor "no same-group subprocess + // outlives the fiber". When no escalation ran (normal completion, no + // kill) or the group is already empty, cancel the pending SIGKILL and + // finalize now. Clearing it is what bounds the PID-reuse hazard: an armed + // `kill(-pid)` left to fire up to graceMs later could hit a RECYCLED pgid + // once the kernel reused the leader's pid, SIGKILLing an unrelated group. + // So the timer stays armed only while a real survivor exists — a + // same-group descendant that ignored SIGTERM but released the pipes, + // still alive here because its `close` is what got us to settle. In that + // case withhold finalize and poll the group on REF'd timers (a + // short-lived host would otherwise exit before the unref'd SIGKILL fired, + // reparenting the survivor to init), clearing the timer the moment the + // group empties. The wait is bounded by `graceMs + CLOSE_REAP_MARGIN_MS` + // in the normal case; if the host event loop was blocked past both timers + // the deadline branch below sends SIGKILL itself and grants ONE more reap + // margin, so the outer bound is `graceMs + 2 * CLOSE_REAP_MARGIN_MS`. if (!killing || groupEmpty()) { if (graceTimer !== undefined) clearTimeout(graceTimer) finalize() @@ -1122,10 +1191,19 @@ export class PythonCodeRuntime extends CodeRuntime { clearTimeout(graceTimer) hardDeadline = Date.now() + CLOSE_REAP_MARGIN_MS } - // Hard bound: only reached if the self-sent SIGKILL never empties the - // reachable group (a kernel that never reports ESRCH), which does not - // happen in practice — hence the ignore on the branch below. - /* v8 ignore next 4 -- SIGKILL empties the reachable group within the reap margin. */ + // Hard bound: the self-sent SIGKILL delivered but `groupEmpty()` still + // reports the group non-empty for a full extra reap margin. This is + // reachable, not a kernel quirk: a SIGKILL'd same-group survivor + // lingers as a ZOMBIE until its parent `wait()`s it, and in a + // container whose PID 1 does not reap orphans the survivor is + // reparented to init and never waited, so the signal-0 probe keeps + // succeeding — the same environment dependence the Agent Note's + // rejected "assert the reap with process.kill(pid, 0)" alternative + // documents. The ignore stays because that container cannot be built + // deterministically across CI platforms, not because the branch is + // unreachable; finalizing here bounds the wait so such a deployment + // still goes quiescent within `graceMs + 2 * CLOSE_REAP_MARGIN_MS`. + /* v8 ignore next 4 -- reachable only in a PID-1-doesn't-reap container (zombie survivor); not deterministically buildable. */ if (hardDeadline !== 0 && Date.now() >= hardDeadline) { finalize() return diff --git a/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts b/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts index 2a36fffa17..c488ea2f37 100644 --- a/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts @@ -1,5 +1,7 @@ import { EventEmitter } from 'node:events' +import { readdirSync } from 'node:fs' import { PassThrough } from 'node:stream' +import { tmpdir } from 'node:os' import { afterEach, describe, expect, it, vi } from 'vitest' import { Context } from 'cordis' @@ -64,4 +66,26 @@ describe('PythonCodeRuntime — boot-write failure', () => { expect(result.error?.message).toContain('failed to boot python subprocess') await fiber.dispose() }) + + it('resolves a worker-exit and removes the staging dir when spawn throws synchronously', async () => { + // `spawn` can throw same-tick — EMFILE on a descriptor-exhausted host, or a + // libuv-level failure — before the Promise executor and its settlement path + // exist. Left uncaught it rejected run() (the seam permits rejection only for + // misuse) and stranded the staging directory materializePyScripts had just + // written, which only settle() removes. The fix catches it, unlinks the + // directory, and resolves the same `worker-exit` class as an async ENOENT. + const before = readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-')) + spawnMock.mockImplementation(() => { throw Object.assign(new Error('EMFILE: too many open files'), { code: 'EMFILE' }) }) + const ctx = new Context() + const fiber = await ctx.plugin(PythonCodeRuntime) + const runtime = ctx.codeRuntime as InstanceType + + const result = await runtime.run({ program: 'return 1', bindings: [] }) + + expect(result.error?.kind).toBe('worker-exit') + expect(result.error?.message).toContain('python spawn error') + const after = readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-')) + expect(after).toEqual(before) + await fiber.dispose() + }) }) diff --git a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts index 9dc6dfe382..2c9c0988e8 100644 --- a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts @@ -682,6 +682,35 @@ describe('PythonCodeRuntime — programs and bindings', () => { expect(result.logs).toEqual(['partial']) }) + it('aggregates a large newline-free native write into one log entry, not one per pipe chunk', async () => { + // A single `os.write` larger than one pipe read arrives as several Node + // `data` chunks. `logs` entries are joined with `\n` downstream, so pushing + // one entry per transport chunk would insert model-visible newlines at + // arbitrary pipe boundaries inside one native write. Stray capture holds a + // per-stream residual and admits only on a real `\n`, so a 200 KiB blast + // with no newline reads back as exactly one entry with no interior breaks. + const { runtime } = await setup({ maxLogBytes: 300_000 }) + const size = 200_000 + const result = await runtime.run({ + program: ['import os', `os.write(1, b"A" * ${size})`, 'return None'].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(result.logs).toEqual(['A'.repeat(size)]) + }) + + it('splits native output on its own newlines, one entry per line', async () => { + // The complement of the aggregation case: real newlines in a native write + // still delimit entries, matching the child's line-granular `log` frames. + const { runtime } = await setup() + const result = await runtime.run({ + program: ['import os', 'os.write(1, b"one\\ntwo\\nthree")', 'return None'].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(result.logs).toEqual(['one', 'two', 'three']) + }) + it('fails a completion dict with a non-string key as invalid-output (no key coercion)', async () => { // json.dumps would coerce {1: "a", "1": "b"} to a single "1" key, silently // dropping data. The shape validator rejects it before encoding. @@ -2116,6 +2145,10 @@ describe('PythonCodeRuntime — budgets, termination, disposal', () => { await fiber.dispose() const mtime = (): number => { try { return statSync(heartbeat).mtimeMs } catch { return 0 } } const afterDispose = mtime() + // Pin the assertion to a heartbeat that actually ran: mtime() returns 0 when + // the file never existed, so without this the `toBe` below would pass + // vacuously (0 === 0) if the survivor never wrote a heartbeat at all. + expect(afterDispose).toBeGreaterThan(0) await new Promise(resolve => setTimeout(resolve, 500)) expect(mtime()).toBe(afterDispose) }, 20_000)