The stdin destroy (child.stdin?.destroy() right after spawn) previously had no
in-tree coverage. A program that reads fd 0 now sees EOF immediately; without
the destroy it blocks and the run would hang to maxWallMs as a timeout —
verified fail-before by disabling the destroy (the test turns red at the wall
ceiling) and restoring it (green). The _str rebind regression was attempted but
is not viable: the success path's done-frame serialization reaches str
transitively through _encode_json_plain, which the README Known Limitations
already records as the accepted success-to-exception residual, so any rebind
test trips that documented residual before send_done's bound _str.
The review's completeness check found the def-time capture pattern was not yet
applied to every name the reply/settlement paths resolve at call time:
- _decode_json_plain now also captures isinstance/str/list.
- read_frame/read_frame_async capture len; read_frame_async captures
asyncio.get_event_loop.
- send_done uses _run's bound _str/_isinstance for its frame-shape check.
- The reply pump's frame reader is a bound method captured by _run BEFORE the
program runs and passed into _pump_replies, so a rebind of the class
attribute cannot redirect it.
The decode-rebind regression test still pins the _decode_json_plain rebind;
rebinding builtins (len/isinstance/list/str) in a test is not viable because
the Python runtime itself resolves them implicitly.
The boot-write-failure fake child carries no stdin at runtime, so the optional
call is the documented guard; the static type (ChildProcessWithoutNullStreams)
says stdin is non-null, which trips the no-unnecessary-condition lint.
The boot-write-failure path's fake child carries no stdin handle, so the
unconditional destroy threw inside the spawn error handler and mislabeled the
worker-exit. Use the optional-call form; the no-stdin branch is exercised by
that same test.
Addresses the review's two remaining items:
- The host closes the child's stdin write handle immediately after spawn. The
program is an async body that reads nothing from fd 0; a live pipe would hold
a host-side handle open past the run, so a setsid-escaped descendant
inheriting fd 0 could keep the host process from exiting even after the
closeDeadline forced settlement. The child (and any descendant) reads EOF on
fd 0 and no host handle survives.
- read_frame/read_frame_async bind their decode primitives (_decode_json_plain,
os.read, _READ_CHUNK_BYTES, bytes) as def-time default arguments, and
_decode_json_plain itself captures json.loads, its two regexes, and len the
same way, so a __main__ rebind cannot kill the reply pump and strand every
pending Future to the wall clock. _decode_json_plain and its regexes moved
before the ProtocolChannel class so the defaults resolve at class-definition
time. A regression test rebinds _decode_json_plain and asserts a binding reply
still round-trips.
Note (en + zh) registers both mechanisms; pairings re-recorded.
The review's remaining non-blocking suggestion: dispatch's call_failure(str(exc))
resolved the builtin str at call time, so a program rebinding __main__.str could
run a hostile callable when the binding-rejection message is formatted. Bind
_str into _run locals and use it in dispatch.
dispatch's call_failure and its except clause resolved the module globals at
call time, so a program rebinding __main__._BindingRejection = ValueError let
the internal marker type leak into model code. Bind _RuntimeError_cls and
_BindingRejection_cls into _run locals before the program runs (names distinct
from the module globals so the assignment RHS resolves the global, not an
unbound local); dispatch now uses the locals. A regression test rebinds
_BindingRejection and asserts a host rejection still surfaces as RuntimeError.
The sys.__stdout__ flush test now reconfigures the streams back to block
buffering (write_through=False) so the settlement drain path is what the case
pins — verified fail-before: binding the stream objects instead of their flush
methods turns the test red.
The settlement drain iterated the bound stream OBJECTS, which are not
callable — every _flush() raised TypeError and was swallowed by the loop's
except, so the drain never ran and only the -u flag carried the behavior.
Bind sys.__stdout__.flush/sys.__stderr__.flush (bound methods, capturing the
stream at binding time, immune to a later sys.__stdout__ rebind; None-guarded).
Verified by removing -u temporarily: the sys.__stdout__ regression test still
passes, so the drain is a genuine backstop, not a documented-but-dead layer.
Addresses the review's two carried warnings and the comment suggestion:
- Once the ledger truncates, every arm that marks it (admit()'s two ceilings and
the child-marker frame arm) now clears both stray pipes' buffered output
wholesale, so the end-path flushStray sees empty buffers instead of
concat+decoding doomed data near a 256 MiB maxLogBytes; captureStray's newline
loop re-checks the flag before re-retaining the residual.
- The child runs with -u (unbuffered), so sys.__stdout__/sys.__stderr__ writes
are visible to stray capture immediately; the settlement flush still drains
the original std streams before the done frame as a guard. A regression test
writes through sys.__stdout__/sys.__stderr__ without an explicit flush and
asserts both bytes land in logs. C-ext stdio remains an accepted residual,
recorded in the README Known Limitations (en + zh).
- The ledger-comment arithmetic now states the exact boundary (serializes to
exactly maxLogBytes; without the reserved byte it would be maxLogBytes + 1)
in both host and child.
Note (en + zh) registers the stray-clear and -u/settlement-drain mechanisms and
the new test; pairings re-recorded; corpus passes 1029.
The review flagged the change-narrative wording 'degrades to the pre-existing
behavior' (prohibited by docs/AGENTS.md) in four spots — README en/zh, the
readProcessStart JSDoc, and the test comment — and the incomplete :77 residual
sentence ('can still' with no verb complement). Reword the four to a direct
statement of current behavior (killGroup signals the pgid without the identity
re-check on macOS), complete the residual sentence with the actual consequence,
and re-record both pairings. Corpus-wide verify-translation-pairing passes 1029.
The master merge brought a stale generated config-catalog that omitted the
dsh-code-runtime-python config section and mislisted the package. Regenerate
docs/config-catalog.md (verify-config-catalog passes), translate the python
config section into zh, keep the ts config-catalog code blocks verbatim
(untranslated, per the pairing rule), and drop the stray zh Library-packages
line. Corpus-wide verify-translation-pairing passes 1029.
The review found the 62 floor off by two (the marker's fixed prefix is 51
characters counting both square brackets, so marker(62) serializes to 63) and
the constructor error over-claiming a bound the marker-as-envelope design does
not deliver. Fixes:
- MIN_LOG_BYTES is 64 (marker-only serialization fits with one byte of room);
the JSDoc arithmetic counts the brackets; the rejection test pins 63; the
forged-frame test uses 11 NULs (69 escaped) at 64.
- The constructor error now states the marker-only guarantee, and the README
Known Limitations (en + zh) records the real bound: a truncated run with
admitted entries serializes its logs to maxLogBytes + marker + envelope.
- The SIGXCPU-mask tests burn with time.process_time() instead of wall-clock
perf_counter, so a contended CI runner cannot under-burn the budget.
- The settlement note (en + zh) records the 64 floor and the marker envelope
bound, including the zh pre-encode section that the earlier pass missed.
- The README constructor-rejection list names the maxLogBytes floor.
Pairings re-recorded; corpus-wide verify-translation-pairing passes 1004.
The review flagged four mechanism changes shipped without note registration:
- Log ledgers start one byte below the budget (outer-array envelope reservation)
and the constructor floors maxLogBytes at 62 (the smallest budget that can
serialize its own truncation marker plus the envelope).
- die_if_cpu_exhausted restores SIG_DFL before unblocking a program-masked
SIGXCPU, so a trap+mask program cannot run a re-masking handler at the unblock.
- ast.parse passes filename="<model>" so parse-time syntax diagnostics share the
compile/runtime source label.
Decision and Testing (en + zh) now record all four with their fail-before cases
(exact-limit, budget rejection, syntax label, SIGXCPU-mask, trap+mask); pairing
re-recorded and consistent.
Addresses the review's two code warnings and one suggestion:
- die_if_cpu_exhausted now restores SIG_DFL BEFORE unblocking SIGXCPU: a program
that installed a custom handler AND masked the signal would otherwise have
that pending handler run at the unblock (in model code, re-masking or raising)
and escape the re-raise; with SIG_DFL first the pending signal kills inside
the kernel with no bytecode window. A trap+mask combined regression test pins
it (the mask-only case was already covered).
- The constructor rejects budgets too small to honor: maxLogBytes must fit the
truncation marker plus the serialized outer-array envelope (floor 64), and
maxValueBytes must at least represent the smallest JSON completion (floor 4,
matching the worker backend). The exact-limit test moves to the 64 floor and
a rejection test pins the floors.
- The pthread_sigmask None-guard comment cites the real rationale (defensive
against stripped CPython builds; win32 is refused at construction), not the
unreachable Windows path.
The residual bullets listed the encoder's transitive deps as an exhaustive set
but disagreed with each other and omitted io. Mark the list as a non-exhaustive
example (e.g. _dump_scalar/_dump_string/json/io) in the README (en + zh) and the
settlement note (en + zh); pairings re-recorded and consistent.
Addresses the review's two remaining code warnings and the three suggestions:
- Log ledgers (host and child) start one byte below the budget, reserving the
serialized outer-array envelope (two brackets and n-1 commas over n entries'
separators); the exact-zero test moves to maxLogBytes 104 and a new exact-limit
case pins that maxLogBytes 5 admits ['a'] (5 bytes) while 4 truncates to the
marker alone.
- die_if_cpu_exhausted unblocks SIGXCPU (pthread_sigmask SIG_UNBLOCK, captured at
import, None-guarded for Windows) before re-delivering it, so a program that
masks SIGXCPU, burns past the soft limit, and returns is still classified as a
timeout; a regression test pins the masked path.
- ast.parse passes filename="<model>" so parse-time syntax diagnostics carry the
same source label as compile and runtime tracebacks; the syntax-error test
asserts the label.
- The NUL-escape test comments use the true six-byte JSON escape \u0000 instead
of the caret notation; the README Known Limitations (en + zh) records that
PID-reuse protection is inert on macOS; a combined-rebind regression test pins
BaseException plus the traceback reporter rebinding together.
Addresses the review's registration-text accuracy findings:
- _run binds _done_with_value into a local (done_with_value_bound) before the
program runs, closing the __main__._done_with_value = boom success-rewrite
vector; a regression test rebinds it and returns a legitimate value, asserting
the success survives.
- README (en + zh): the CPU-recheck bullet now states the recheck runs
unconditionally after the program returns (a pre-return overrun dies there as
a timeout) and the false-success window is only a trap-SIGXCPU program that
passes the recheck and overruns during the settlement flush/encode; the
encoder-deps residual rationale is replaced with the actual one (bash-equivalent
trust, verdict still delivered via the send_done fallback frame) and names the
now-bound entry; the t.join() deadlock bullet fixes the subject/object (the
main coroutine joins the worker, blocking the pump's main event loop).
- The portable-identifier-seam architecture note no longer claims the Python
backend does not exist.
- Settlement note (en + zh) registers the entry-name binding and the new test.
- All pairings re-recorded; corpus-wide verify-translation-pairing passes 1004.
Master reverted PR 2573, deleting the case-insensitive-path-round-trips
note that carried the dead link, so take the deletion and drop the note
repair from this branch; the jsonl.spec.ts change merges cleanly.
Addresses the review's two registration-text accuracy findings:
- The cross-thread t.join() deadlock is a process-isolation-backend property (the
pump runs on the child's main event loop), so it is split out of the wide-binding
REPLY bullet into its own Known Limitations entry with the correct attribution
(fix belongs in this backend, not packages/core/session); the zh half-width
space is removed.
- The settlement note's _done_with_value def-time default-arg sentence is
qualified: it guards a rebind of _check_done_value/_encode_json_plain, while a
transitive encoder dep (_dump_scalar/io) rebind can still downgrade, which is
registered as an accepted residual in the package README.
Pairing re-recorded; corpus-wide verify-translation-pairing passes 1004.
Document the two remaining keep-current residuals in the python package README
Known Limitations (en + zh), per the review's accepted-resolution path:
- A trap-SIGXCPU program can exceed the soft CPU limit during settlement encoding
and still report success (containment holds via hard +1s and wall clock; only
the classification is degraded, because the recheck cannot meter mid-encode).
- The encoder's direct deps (_dump_scalar/_dump_string/json) resolve at call
time, so a __main__ rebind after a legit return can downgrade success to
exception; the value path's top-level deps are def-time bound, the transitive
ones are an accepted residual.
Pairing re-recorded and consistent.
Addresses the bot's keep-current findings:
- The settlement note distinguishes the BaseException (lost done frame) and
RuntimeError (pump killed -> replies stranded to the wall clock) consequences;
registers the _done_with_value def-time default-arg capture and the new
RuntimeError-rebind closed-loop test; zh:95 half-width space fixed.
- The python package README Known Limitations records the cross-thread binding +
sync t.join() deadlock (en + zh).
- The code-runtime Service Definition README no longer claims only the
worker-thread backend ships: the Python (process) backend is acknowledged,
with 'container' as future work (en + zh).
- All pairings re-recorded; corpus-wide verify-translation-pairing passes 1002.
A body-local X = X binding in _pump_replies is too late: _run reaches the
model's top-level statements (which run first, since there is no suspension
point between create_task and await __dsh_main__) before the pump's first step,
so a __main__.RuntimeError rebind there would be captured by the body local and
a closed-loop failure would escape the except, killing the pump. Bind
_RuntimeError, _BindingRejection, str, and bool as DEF-TIME default arguments of
_pump_replies (evaluated at import, before any model code runs). Add a regression
test that rebinds __main__.RuntimeError as the first program statement and drives
the closed-loop worker pattern, asserting the pump survives and delivers the
later binding. Update the settlement note (en + zh) to describe the default-arg
capture; pairing re-recorded and consistent.
- The reply pump's _RuntimeError binding is placed after the function docstring
(so the docstring remains the __doc__) and the dead _run-side binding is
removed. _done_with_value binds _check_done_value/_encode_json_plain as
default arguments so a __main__ rebind after model execution cannot rewrite a
success into an exception.
The _str/_bool/_BindingRejection pump bindings were attempted but break the
closed-loop pump test (the self-referential _BindingRejection local interferes
with the closure), so they are left unbound; rebinding those names (builtins and
one internal class) is outside the practical threat model.
The previous commit bound _RuntimeError in _run, but _pump_replies is a separate
module-level function, so its except _RuntimeError referenced an out-of-scope
local and raised NameError instead of catching the closed-loop failure — killing
the pump and timing out the run. Bind _RuntimeError at the top of _pump_replies
too. The closed-loop pump test now passes.
The reply pump's except RuntimeError resolved the module global at runtime, so a
__main__.RuntimeError rebind could make a closed-loop scheduling failure escape
the catch, killing the pump and stranding every later reply. Bind RuntimeError
into a _run local alongside BaseException and catch the local. The settlement
note Decision now records that the exception classes the settlement-path except
clauses catch are bound into locals / a closure cell before model code runs
(en + zh); pairing re-recorded and consistent.
The rebindable-BaseException vector the bot flagged existed in every except
clause of the settlement path, not just the _run outer catch: safe_model_traceback
(three guards) and the post-done flush swallow resolved the module-global
BaseException at runtime, so a __main__.BaseException rebind plus a throwing
__str__ could let a render-time exception escape and lose the done frame. Bind
BaseException into a _run local (at the top) and a closure cell in
_make_failure_reporter, and change every such except clause to catch the local
— immune to a one-line rebind.
The zh Consequences section counted ten but enumerated only nine; add the
log-fragment seal as the 10th no-fail-before item. Also unify the term to
'封存' (matching the Decision/Testing sections) instead of '封口'. Pairing
re-recorded and consistent.
The _run outer try/except used the module-global BaseException, which the
program (running as __main__) can rebind: __main__.BaseException = RuntimeError
made the except resolve to RuntimeError, so a subsequent ValueError escaped _run
with no done frame and misreported the run as worker-exit. Bind BaseException
into a _run local before the program runs so the catch is immune; a regression
test rebinds BaseException and raises, asserting an exception, not a worker-exit.
Also correct the NUL-escape comment text: the JSON escape-result side is \^@ (6
bytes, the valid JSON NUL escape), not \x00, so the 6x-budget arithmetic in the
comments is self-consistent. Register the BaseException-rebind case in the
settlement note Testing (en + zh) and re-record the pairing.
Addresses the bot's keep-current review findings:
- The module-level fallback comment now states the mechanism truthfully: the
module globals are RAW primitives bound into _run LOCALS before the program
runs (the immunity lives in the frame-local binding, not the module global);
and the fallback literal <unrenderable> is distinguished from the failure
reporter's _UNRENDERABLE_DIAGNOSTIC text.
- The settlement note's fallback mechanism wording, the transitive-name rebind
case (now listing the three fallback primitives), and the no-fail-before count
are aligned en/zh; the zh Problem paste damage is fixed and the Consequences
count is ten with the 10th item.
- Pairing re-recorded and consistent.
Addresses the keep-current review findings:
- The settlement note's Problem/Consequences count is nine -> ten, adding the
log-fragment seal to the no-fail-before enumeration (its 25 M-scale OOM is not
deterministically constructible in CI); the new Decision section title now
names all four mechanisms and the double blank line is removed.
- README Known Limitations (en + zh) documents the 1-second dual-limit
ulimit -t 1 CPU overrun being reported as worker-exit (the hard >= 2 guard
cannot lower a 1-second soft to 0); pairings re-recorded and consistent.
The done-frame fallback read _os_write/_memoryview/_FALLBACK_DONE_FRAME as module
globals at call time, so a single-line rebind of any of them reopened the
rebind hole the fallback exists to close. Bind them into _run locals before the
program runs, and use a bare except (which catches everything without naming
BaseException, so a rebind of that name cannot defeat the handler). The
transitive-name rebind test now also rebinds _os_write/_memoryview/
_FALLBACK_DONE_FRAME to pin the fallback's immunity.
Keep the settlement note current with the latest code-review fixes:
- New Decision section for the _LogStream fragment seal, the _clamped
RLIMIT_CPU soft-lowering (and its hard==1 blind spot), the send_done
fallback frame, and the reply-queue slot release.
- Testing registers the fragment-cap drip (no-fail-before), the dual-limit CPU
overrun, and the transitive-name rebind cases.
- zh mirrored; settlement-fixes.i18n.yaml re-recorded and consistent.
Addresses the bot's follow-up review findings on the settlement-path fixes:
- The _LogStream seal joined the WHOLE accumulated buffer past the fragment cap,
re-copying the growing block O(B^2/cap) times for a large drip. It now seals
only the current fragments into a _pending_blocks entry (character count
unchanged), so a 25 M single-character drip stays O(B); the newline/flush/
_push_bounded_prefix consumers join blocks + fragments once.
- The _clamped soft==hard lowering is scoped to RLIMIT_CPU: for RLIMIT_AS a
one-byte soft differential would only misalign the child's applied limit with
the host-side budget gate, with no signal to preserve. The hard == 1 blind
spot is documented.
- send_done's fallback captures memoryview at import (_memoryview) alongside
os.write, so a one-line rebind of the name cannot change the fallback write;
the comment now states the module-level-captured mechanism.
The dual-limit CPU test used ulimit -t 1 (hard == 1), which the _clamped
soft-lowering guard (hard >= 2) intentionally does not lower, and trapped
SIGXCPU (which defeats the fix). Use ulimit -t 2 (hard == 2, so the soft is
lowered to 1) and leave SIGXCPU unhandled; the run then classifies as a timeout.
The message is the CPU-time-exhausted diagnostic, not the literal 'SIGXCPU'.
Addresses the bot's v16 review on the settlement-path code:
- critical: _LogStream._pending now seals the fragment list past a chunk cap
(like the host captureStray seal), so a newline-free single-character drip no
longer accumulates one list slot per write and OOMs on its own accounting.
- _clamped lowers a soft==hard result by one unit (when hard >= 2) so a
dual-limit ulimit -t leaves SIGXCPU a window to fire and a definite CPU
overrun is reported as a timeout, not a worker-exit.
- send_done wraps its encode+write in a try and, on any throw from a rebound
transitive name (_dump_scalar/os), writes a fixed pre-encoded done frame via
the import-time captured os.write, so a settled exception verdict is never
downgraded to worker-exit.
- drainReplies clears the consumed replyQueue slot so a wide written payload is
released immediately, bounding host memory to the current backlog under
sustained fd-3 backpressure.
Tests added for each (fragment cap drip, dual-limit CPU overrun, transitive-name
rebind done frame).
The Testing sentence's subject attached the three rebinds to 'the fix' rather than
to the fixture that performs them; reword to 'pinned by a case that rebinds' and
mirror zh ('由一个…用例钉住'), re-recording the pairing.
The rebinds-every-name fixture previously only rebound ProtocolChannel.send_sync,
which a bound method object ignores and the shipped send_done no longer calls —
so it did not actually guard the call-time-lookup shape. Rebind write_encoded
and _encode_json_plain too (the names send_done would resolve late if it looked
them up at call time) and state that in the settlement note's Testing section
(en + zh), re-recording the pairing.
Keep the agent note current with the recently landed code-review fixes:
- six -> nine no-fail-before cases, adding the done-value TOCTOU pre-encoding,
the stray-UTF-8 budget-flush retention, and the late-rejection settled guard,
each with its reason for not carrying a fail-before test.
- New Decision sections for the pre-encode + send_done binding and the stray
flush retention; Testing lists the binding-all-names case as a tested fix.
- zh mirrored; settlement-fixes.i18n.yaml re-recorded and consistent.
Each shift() re-slices the remaining array, so draining a large gather of
wide bindings awaiting fd 3's drain was O(n^2). Reading by a head index into
the array keeps the drain linear; the finally still discards everything.
Addresses the follow-up review findings on the settlement-path fixes:
- send_done now routes both the pre-encoded VALUE frame and the dict ERROR
frame through a bound _encode_json_plain + bound write_encoded, never through
channel.send_sync (whose body re-resolves self.write_encoded and the module
_encode_json_plain at call time) — a program rebinding ProtocolChannel.
write_encoded or __main__._encode_json_plain no longer skips the done frame.
- flushStray retention re-accrues the withheld multibyte tail from a FRESH
utf8 state (previously metering the carried lead against the post-flush
expected>0 state charged it as an illegal continuation), and skips admitting
when the whole residual drained into the retained tail so no bogus empty
entry is pushed.
Corrections to the settlement-path review fixes:
- send_done was invoking channel.send_sync / channel.write_encoded via a late
method look-up, which a program running as __main__ could rebind through
__main__.ProtocolChannel.send_sync before the failure path ran — a rebound
send that raises then skipped the done frame and downgraded a settled
exception to worker-exit. Bind both channel methods into locals before the
program runs, mirroring the pre-existing binding of flush_out/flush_err/
safe_model_traceback.
- Restructure flushStray so the mid-sequence budget-flush retention arm is a
self-contained v8-ignored branch and the covered default path decodes the
full residual (not schedulable-through-the-seam boundary).
Pace-free completion framing, stray UTF-8 flush, and late-rejection guards:
- Pre-encode the completion value at its validation point so send_done never
re-walks a live value a mutating daemon thread could change (TOCTOU); a
mutation-induced encode throw is then classified as 'exception', not a
host-side worker-exit.
- Budget-triggered stray flush retains an incomplete multibyte UTF-8 tail
(<=3 bytes) as residual instead of decoding a legal, split character to
U+FFFD in an admitted entry; the end/closeDeadline paths still full-decode.
- Check 'settled' before formatting a late binding rejection's message, so a
hostile message getter cannot stall or exhaust a run that already settled.
- Document _check_done_value's first-to-trip ruling in its docstring.
- Rewrite ProtocolChannel.send_sync around a shared write_encoded that the
done frame's pre-encoded string path uses.
