The review's follow-ups: (1) the product-visible change (an unresolvable
basename pythonBin now fails at load instead of a run-time ENOENT worker-exit)
is registered in the settlement note, paired. (2) PYABS falls back to the bare
name when python3 is not resolvable, instead of interpolating the literal
'undefined' into the wrappers.
The review's follow-ups on the pythonBin change: (1) the JSDoc and the two
call-site comments still described the old fallback-to-bare-name contract;
they now state the load-rejection behavior. (2) the ExceptionGroup case's
version guard raised a skip message on Python < 3.11 but the assertion still
required the truncation marker unconditionally — the assertion now matches
either the truncation marker (3.11+) or the skip message (3.10). (3) the shell
wrappers quote the resolved interpreter path.
The review's two non-blocking items: (1) resolvePythonBin returned the bare
basename when PATH had no hit, and spawn (env:{}) would silently fall to
execvp's platform default PATH and could start a system interpreter the caller
never asked for. It now returns undefined for an unresolvable basename and the
load check rejects it (absolute paths pass through), so the failure is loud at
configuration time instead of silent at spawn; the case that expected a
run-time worker-exit now asserts the load rejection, consistent with the
empty/NUL pythonBin cases. (2) the over-cap exception-group case skipped on
Python < 3.11 (ExceptionGroup is a 3.11+ builtin), matching the TaskGroup
case's version guard.
The review's portability warning: the six shell wrappers exec'd a bare
'python3', which /bin/sh resolves against its compiled-in default PATH while
the runtime spawns with env:{} — in environments where python3 is reachable
only through the caller's PATH (Nix, pyenv) every wrapper run would fail as
worker-exit. The wrappers now bake the resolved absolute interpreter path
(module-level resolvePythonBin, which the product spawn already uses), and
resolvePythonBin is exported for the tests.
The reviewer's residual timing item: the inherited-SIGXCPU reset ran AFTER
setrlimit(RLIMIT_CPU) and the boot-namespace construction, so a huge namespace
under an inherited ignore/block could burn past the soft limit inside that
window and be misclassified as worker-exit. The reset now happens at the very
top of _run, before the resource-limit setup and namespace construction.
The review's two follow-ups on the inherited-SIGXCPU fix: (1) a discriminating
case — pythonBin points at a wrapper that ignores SIGXCPU before exec'ing
python3, so the child genuinely inherits the ignore; with cpuSeconds: 1 the
busy loop must end as timeout (the bootstrap reset restored SIG_DFL), and
reverting the reset leaves it running to the wall — verified red. (2) The zh
README's OUTER wire section now carries the truncation-exception sentence
(the previous commit had duplicated it in the inner section instead); the
duplicate is removed, and the settlement note registers the inherited-SIGXCPU
reset.
The reviewer's standing issue: the child inherits the host's SIGXCPU
disposition and signal mask — if the host ignores or blocks SIGXCPU, the soft
RLIMIT_CPU fires but cannot stop the child, and the hard limit's SIGKILL then
classifies a definite CPU overrun as worker-exit instead of a timeout. The
bootstrap now resets SIGXCPU to SIG_DFL and unblocks it before any model code
runs (the settle-time enforcer already restores SIG_DFL for a program that
traps or masks the signal mid-run; this closes the inherited-state gap). The
zh README's outer wire section also gains the truncation-exception sentence to
match the en side.
The review's warning: the READMEs (outer and inner wire sections, en + zh) and
the fd-3 protocol note still claimed the next log frame always merges into an
open entry, while truncateLogs commits the already-billed prefix as its own
entry before the marker. The one exception (truncation) is now stated in both
READMEs and the owning note, paired and re-recorded. The prefix-commit case's
parenthetical describing the pre-fix implementation is removed per the
comment-does-not-record-review-history rule.
The reviewer's standing issue: line.toString('utf8') silently replaces illegal
bytes with U+FFFD, so a forged frame could land a corrupted completion value
(the honest child's lossless encoder never emits non-UTF-8, so such a frame is
hostile traffic). The fd-3 frame decode now uses a fatal UTF-8 decoder: an
illegal byte throws and the frame is dropped, same treatment as the
unsafe-integer check. A forged illegal-UTF-8 done frame is verified to be
dropped (the run settles on the program's real return), and reverting to
toString makes the case fail.
The review's warning: a flushed unterminated line is billed and committed
(README wire contract says so), but every truncation arm — the child truncated
frame, an over-budget open frame, an over-budget closing frame, and admit's two
budget arms — pushed only the marker, dropping the held prefix: the ledger
charged for output that vanished. All arms now funnel through truncateLogs(),
which pushes the (already billed) held prefix before the marker and clears
openParts, so the prefix survives and only the marker stays last; the finish()
guard drops the now-dead !logsTruncated check (a truncated run has an empty
hold). A regression case asserts [prefix, marker]; the forged-flood and
closing-overflow cases now expect the committed prefix plus the marker.
The review's suggestion: an empty open continuation frame bills zero and holds
one host slot, so a forged empty-open flood grows the held fragment array
without touching logBudget. Accepted as a residual (per-frame host cost far
below its ~30-byte fd-3 wire cost, bounded by pipe throughput, model-code trust
level equal to bash) and now registered in the README's Known Limitations on
both sides, paired and re-recorded.
The review's revision: the buffered-chunks pre-check's open-aware overhead is
observationally inert — when the +3 form trips and the open-aware form does not
(pending + newline in [remaining - 2, remaining]), _push_bounded_prefix
re-slices the same newline-free line text and _push_locked admits it under the
same open-aware billing, byte for byte. The comment now states that the
open-aware form keeps _push_bounded_prefix's 'certain to reject' precondition
true, contrasting with the scan pre-check whose slice carries the newline and
therefore genuinely truncates.
The review's warning: the en README's inner 'Wire contract' section still
described only the truncated flag while the zh counterpart (and the outer 'The
wire' section) described open. The inner en section now matches. The exact-fit
closing-line case comment described the buffered-chunks pre-check recipe while
the program actually drives the scan pre-check; the comment now states the
actual arithmetic and path (and the buffered variant was dropped — its writes
coalesce into one call in the test environment, so it did not discriminate).
The review's warning: while an open entry accumulates, the newline pre-checks in
the write path still charged a NEW entry's +3 cheap-bound overhead (quotes +
separator), so an exact-fit merged TAIL was truncated (or the pre-check
over-rejected it and flushed a truncated prefix). Both pre-checks now charge
the overhead only when no open entry is in progress, matching _push_locked's
open-aware bound. A regression case (the review's recipe: flush an open
fragment, then write one exact-fit newline-terminated line) is verified to
truncate when the +3 is restored.
The zh README's wire-contract section now describes the open flag like the en
side (the fd-3 Agent Note holds the split-billing arithmetic; a cross-doc link
was omitted to keep the bilingual link sequence aligned).
The review's warning: the one-byte overflow case ran through the CHILD ledger
(print path), so the host's first-fragment cap (logBudget - 1) never executed,
and the sub-2-byte guard test does not discriminate logBudget from
logBudget - 1 (a reverted cap still trips the guard). The frame is now forged
on fd 3, so a reverted cap of logBudget admits it and flushes it at settlement
— verified to turn the test red.
The review's dedupe suggestion: the split-billing arithmetic was stated in both
notes; the settlement note's Decision paragraph now links to the fd-3 protocol
note's wire-contract section (one home per fact), paired and re-recorded.
The new jsonStringCostUpTo guard (returns undefined below a 2-byte cap) was
uncovered: forged open frames drive the host ledger down to one byte, and a new
open entry's first-fragment cap (logBudget - 1 = 0) trips the guard and
truncates to the marker, asserted as the merged entry plus the marker.
The review's arithmetic checks: the closing-frame walk used cap
logBudget - openCost, so a compliant merged entry (58-byte wire cost under a
64-byte budget) could see a negative cap and truncate; the first-fragment cap
used logBudget instead of the ledger's logBudget - 1, so an open frame costing
63 was admitted with a bill of 64, pushing the ledger negative and letting a
subsequent empty frame ride in one byte past the configured cap; and the child
billed a closing frame as a fresh entry (quotes+separator again) instead of the
merged tail, truncating an exact-fit 30+30 entry.
Fixes: first-fragment cap logBudget - 1 (matching admit), continuation and
closing-frame cap logBudget + 2 (billed without quotes), jsonStringCostUpTo
returns undefined below 2 bytes, and the child's split billing keys off
_open_started alone (a closing frame pays content only) with the cheaper bound
len(text) while a merge is open. Regression cases cover all three arithmetic
paths.
The review's critical: the open-merge branch re-joined and re-walked the whole
held text per frame, so k tiny open frames cost O(k * budget) (thousands of
1-byte frames against a near-64 MiB budget would re-traverse hundreds of GB and
block the host event loop). The host now holds a fragment ARRAY with an
incrementally billed cost — each fragment's jsonStringCostUpTo walks only its
own text — and the closing frame bills only its own content, so the merged
entry's wire cost is charged exactly once, split across the fragments. The
child bills symmetrically: the first open fragment pays quotes+separator, each
continuation pays only its content, matching the host ledger (the review's
warning: per-fragment full billing truncated a 16-char merged entry under
maxLogBytes: 64 that costs only 19 bytes as one entry).
Regression cases: 16 single-character flushes merge to one whole entry; a
closing frame that overflows the remaining budget truncates to the marker; a
closing frame after an open flood already truncated the ledger is a no-op; and
a forged open-frame flood stays bounded by the ledger. The closing-frame
post-truncation guard is an invariant-false branch (an open frame that would
trip the ledger resets openParts, so a non-empty hold implies no truncation)
and carries a v8 ignore with that reason.
The review's critical: the open-merge branch accumulated the held fragment
before any ledger check, so a forged open flood could grow host memory without
touching logBudget. The held fragment is now bounded by the exact-cost walk
(jsonStringCostUpTo against the remaining budget; the closing frame's admit()
still bills the merged entry once), and the open field is registered in the
README wire-contract section and the fd-3 protocol note (en + zh). A forged
open-flood case asserts truncation to the marker under a 64-byte budget.
The review's remaining warning: an explicit flush of an unterminated line
(print(..., end='', flush=True)) pushed a full log frame, so the following
print() landed in a second entry and logs.join('\n') rendered 'a\nb' for what
the program printed as one line — a model-visible output defect. The flush
frame now carries an flag (LogMessage gains the optional field on both
sides and in the mirror test), the host holds it and appends the next log frame
to the same entry, and finish() admits the residual if the run ends with it
still open. The settlement note registers the decimal-context fix from the
previous commit.
The review's critical: Decimal(repr(value)).normalize() read the process-global
decimal context, so a legitimate program setting getcontext().prec = 2 silently
rounded the completion value's digits and traps[Inexact] = True made the encode
raise, misclassifying a successful run as an exception. A fixed module-level
Context(prec=28) makes the spelling decision context-independent; a regression
case mutates both context knobs and asserts the float round-trips exactly.
The binding-reply README entry now states the fact (no seam-level cap;
maxValueBytes meters only the done frame; a wide reply is rebuilt and encoded
whole, bounded by process memory), matching the earlier reviewer wording.
The review showed the added dispose case was a placebo (dispose in the same
tick as run means SIGTERM hits the group before the program body runs; the
group-emptied arm is already deterministically covered by the same-group
survivor case, which this removes the v8 ignore for). The test is deleted; the
stale silently-discards comment in the boundary test now says rejects; the
README Known Limitations gains the late-log-frame-drop and host-side
binding-value-memory entries. Pairing re-recorded.
The review's premise that the group-emptied arm could not be pinned was
incorrect; the same-group reap case already exercises it. This adds the missing
seam-observable case: dispose() while a setsid orphan holds the pipes and the
run is unresolved — settle kills the child, the group empties (the orphan is in
its own session), and the poll finalizes promptly instead of waiting out the
60 s grace. The v8 ignore on that arm is removed.
The review's remaining coverage gap: the group-emptied arm of pollGroup depends
on the close-driven settle winning the race against the grace SIGKILL, a timing
interleaving no seam-observable test pins deterministically (the same-group
cases assert the settle and the reap, not this exact interleaving) — the arm
now carries a v8 ignore with that reason. The load-check comment and the
FRAME_ENVELOPE_BYTES JSDoc say rejects-as-worker-exit instead of drops.
The forged-second-boot-ack regression makes the re-entry guard covered, so its
v8 ignore is removed. Doc drift: the python README and run() JSDoc state the
resolve-with-value/resolve-with-error contract without inversion; the README
Known Limitations gains the setsid-escaped-orphan entry (the settlement note
referenced it); the settlement note drops the stale drops/discard phrasing and
the two 256 MiB references; the fd-3 protocol zh note no longer claims the
codec is undelivered; the code-runtime seam README (en + zh) says both
backends ship. Pairings re-recorded.
The review rejected the v8-ignore defense for the ack gate: a forged second
boot-ack is deterministically constructible (one os.write on fd 3) and the
run-write failure is deterministically constructible with the boot-write-failure
mock pattern. A program that forges an extra boot-ack asserts the run still
completes once (the gate does not re-send the run frame); a mocked child whose
fd-3 pipe accepts the boot frame but rejects the run write resolves a
worker-exit.
The resolvePythonBin directory branch now has a regression: a PATH whose first
entry is an executable DIRECTORY named python3 is skipped for a later real
interpreter (fail-before: without the isFile guard the directory would be
chosen and spawn would fail). The boot-ack gate's forged-second-ack re-entry
guard and its write-failure branch are covered by v8 ignore comments (the
honest child sends exactly one ack; the write failure needs the child to exit
between ack and write).
The review's two behavior items: the run frame was written back-to-back with
the boot frame (the seam contract puts run after boot-ack, which confirms the
namespaces were accepted); it now goes out from the boot-ack handler, so a
boot failure cannot race the run frame. resolvePythonBin now requires the
candidate to be a regular file — a directory passes X_OK and would otherwise
shadow a later real interpreter. Doc spots: the load-time overflow message
says worker-exit (not stranding to the wall clock), the run JSDoc spells out
the resolve-with-error contract, the PATH-stub test removes the stale v8
ignore, and the README's binding-value bullet names serialization cost.
The earlier merge had adopted master's protocol-only package.json (peer/dev
limited to invariants and cordis, no dependencies), but src/index.ts imports
@deepseek-ai/dsh-code-runtime, dsh-session, dsh-timeout, and schemastery at
runtime — a published lib/index.js could not resolve those bare specifiers.
The manifest now mirrors code-runtime-worker-thread (the five peers, the
schemastery dependency, and the matching dev set); the lockfile, module graph,
and third-party notices are regenerated, and the module-graph zh pair is
re-synced.
The earlier merge had kept the branch's older package.json while taking
master's lockfile, so a frozen install failed on mismatched specifiers for the
code-runtime-python package (master added dsh-code-runtime, dsh-session, and
dsh-timeout peers). The branch changes no dependencies, so it adopts master's
manifest verbatim.
The audited library registry still listed dsh-code-runtime-python as a plain
protocol library, but the shipped package's src/index.ts has a plugin default
export; the entry is removed from PACKAGE_LIBRARIES and both READMEs declare
kind: package-reference. The zh README heading is 概述 per the standard.
The merge pulled master's README rewrite (front-matter, Summary, TOC, section
anchors, details-folding); its content described the pre-delivery protocol-only
package, contradicting the shipped backend. The README (en + zh) now follows
that structure with the delivered facts: PythonCodeRuntime, the fd-3 wire, the
load-validated caps, the 64 MiB frame parse cap (worker-exit settlement), and
the known limitations. Pairing re-recorded.
The review's three stale-comment items in index.ts: the orphan JSDoc above
FRAME_PARSE_CAP_BYTES (left over from the deleted receive ceiling), the
pre-join comment's change narration and its reference to a no-longer-existing
higher ceiling, and the first-frame comment's mention of a per-line cap check
that no longer exists. Test comments for the pythonBin and sealing-threshold
cases are weakened to their observable claims (both orders reject an over-cap
frame; the pythonBin case pins the contract, not a worker-exit distinction).
The review's doc drift items: the orphan receive-ceiling JSDoc, the frame-ceiling
references in index.ts/bootstrap.py/tests, and the README's 'dropped, stranding
to the wall clock' phrasing (the run now settles as a worker-exit) are all
updated to the 64 MiB FRAME_PARSE_CAP_BYTES semantics; the README notes the
>64 MiB binding-argument residual as a worker-exit trip of the same cap. A
regression case resolves a basename pythonBin against a PATH whose first entry
is relative ('.') and asserts the absolute entry is used.
The review's remaining code items:
- resolvePythonBin now skips RELATIVE PATH segments (a bare 'bin' or '.'): the
returned candidate must be absolute, because spawn() resolves a relative
pythonBin against the host CWD, outside the seam contract.
- A deterministic-ish regression pins the sealing-threshold corner: 64 MiB of
4 KiB (<= PIPE_BUF, atomic) newline-free writes plus 12289 more A's before
the first newline make the first frame exceed FRAME_PARSE_CAP_BYTES; the
newline-bearing chunk reaches the first-frame check (sealing is the ELSE
half of the newline branch), so the run reports worker-exit with the
protocol-frame-exceeded message.
The review's sealing corner: the fragment-count seal ran before the newline
branch and did not exclude a newline-bearing chunk, so the 1024th chunk (the
first to carry a newline) was concatenated into a sealed block, pendingChunks
was emptied, sawNewline stayed false, and the first-frame check was skipped for
a join that then contained the newline. Sealing now runs as the ELSE half of
the newline branch, so a newline-bearing chunk always reaches the join and its
first-frame check, and the invariant 'sealed blocks hold newline-free prefixes
only' is true — which is what makes the removed per-line check genuinely dead.
The pre-join counter (single unframed line) and the first-frame check
(newline-bearing chunk) reject any frame past FRAME_PARSE_CAP_BYTES before the
join, so every line reaching this loop is within the cap by construction — the
per-line check was dead code and its continue branch could never fire, failing
the per-file 100% coverage gate.
The pre-join check charged the whole unframed buffer, which legitimately holds
several frames each within FRAME_PARSE_CAP_BYTES: a first frame of exactly the
cap followed by a second frame crossed the counter and was misreported as a
worker-exit. The pre-join rejection now fires only while the held bytes are a
single unframed line (this chunk carries no newline); once a newline arrives,
a FIRST-FRAME check measures the bytes up to the first newline across the held
chunks (including sealed blocks) and rejects only that frame before the join —
keeping the peak at one copy of its wire bytes — while later frames in the
same buffer are handled by the restored per-line check. Regression cases: a
72 MiB newline-free buffer is rejected pre-join (fail-before: joining would
have doubled it); two within-cap frames whose combined buffer crosses the cap
both survive (fail-before: the unconditional counter check turns it red).
The unframed-buffer counter guard runs before every join and guarantees each
line is within FRAME_PARSE_CAP_BYTES, so the line-loop cap check was dead code
(its continue branch could never fire, failing the per-file 100% coverage gate
on index.ts). Removed with a comment explaining the invariant.
The pendingBytes guard now trips at FRAME_PARSE_CAP_BYTES (64 MiB) instead of
the 256 MiB wire ceiling, so the three tests that flood/pin frames against the
guard assert the 67108864 message and write a 64 MiB-based workload.
The review's remaining critical: the fd-3 data handler checked the unframed
counter against the 256 MiB wire ceiling, so a single 64-256 MiB frame was
fully Buffer.concat-joined (a second copy) and only then dropped in the line
loop — the peak-memory doubling the pre-join check exists to prevent, for a
frame the parser is guaranteed to discard. The counter is now checked against
FRAME_PARSE_CAP_BYTES before the join; the regression case asserts a worker-exit
with 'protocol frame exceeded' (fail-before: reverting to the ceiling turns it
green, proving the join path). FRAME_CEILING_BYTES is removed.
The rejection-cap fix now has its regression: a completion value whose class
name is 70 MiB of Ns asserts invalid-output, not worker-exit (fail-before:
uncapping the diagnostic turns it red).
The settlement note (en + zh) updates the remaining stale bound text, and the
fd-3 protocol note (en + zh) no longer claims protocol-only exports or a
missing Python codec. Pairings re-recorded.
The review's remaining items:
- _done_with_value's rejection branch now caps the _check_done_value diagnostic
through _cap_message (a reason embedding a hostile class name could otherwise
push the done frame past the host's 64 MiB parse cap, misreporting an
invalid-output run as a worker-exit).
- The settlement note (en + zh) updates three stale facts (load bound is now
parse-cap minus envelope at 67108800; the sink goes directly through the
bound primitives); the fd-3 protocol note (en + zh) no longer claims the
package ships protocol without the runtime; FRAME_ENVELOPE_BYTES' JSDoc and
_cap_message's docstring follow the new bound.
Pairings re-recorded.
The review's remaining warning: the _run binding comment claimed the log sink
went 'through the bound send', contradicting the sink's actual direct use of the
bound encode+write primitives. The comment now states that; the settlement note
(en + zh) registers FRAME_PARSE_CAP_BYTES and the 65 MiB-frame regression case.
Pairing re-recorded.
The review found the 64 MiB parse cap contradicted the load-time budget bound:
maxLogBytes/maxValueBytes could be configured up to ceiling - envelope (~256 MiB),
but the receive path silently dropped any frame past the 64 MiB parser cap, so an
honest child's budget-internal done frame under such a config would be discarded
and the run stranded to the wall clock. The load bound is now parse-cap -
envelope, so a configured budget always fits through the parser; the boundary
test moves to 64 MiB - 64. The >64 MiB model-constructed binding-argument drop
is registered as an accepted residual in the README (en + zh).
Addresses the review's remaining two items:
- FRAME_PARSE_CAP_BYTES (64 MiB) drops an fd-3 frame whose raw length exceeds
it BEFORE toString/JSON.parse: the 256 MiB wire ceiling bounds the bytes, not
the decoded structure, and a compact wide frame near that ceiling could decode
to far more host memory. A regression test writes a 65 MiB log frame plus a
normal one and asserts the oversized frame is dropped while the trailing frame
still lands in logs (fail-before: without the cap the oversized text is parsed
and admitted, truncating the ledger so the trailing frame is dropped). The
forged-oversized lower-bound test's frame is reduced to stay under the cap
while still exercising the truncation path.
- The log sink writes through the def-time bound encode+write primitives (not
send_sync, whose body resolves _encode_json_plain and self.write_encoded at
call time), so a rebind cannot break a log frame.
The review's remaining functional item: send_sync's body resolves
_encode_json_plain (module global) and self.write_encoded (class attribute) at
call time, so a program rebinding either before the first binding call could
turn a legitimate call into an exception. dispatch now writes the call frame
through def-time bound write_encoded+_encode_json_plain, and the log sink goes
through the bound send; the dispatch rebind test also rebinds those two names
(verified fail-before by reverting to send_sync). The annotation test title
matches its assertion direction, and the note (en + zh) registers the
error-class constructor, dispatch primitives, and dont_inherit mechanisms.
Pairing re-recorded.
The review required regression cases for the two cfb35bef6 fixes:
- Rebinding __main__.Exception/__main__.setattr must not break the minted
error class: a host rejection still surfaces as ToolCallError with the member
property readable.
- Rebinding __main__._lossless_json_violation/__main__.asyncio/
__main__.ProtocolChannel.send_sync must not break dispatch: a legitimate
binding call still round-trips.
bootstrap.py imports from __future__ import annotations; compile(wrapped) was
inheriting that PEP 563 flag, stringifying the program's type annotations and
changing the semantics of a legal program that reads f.__annotations__ at
runtime. compile(..., dont_inherit=True) stops the leak; a regression test
defines an annotated function and asserts the annotation is the live int class,
verified fail-before by removing dont_inherit (the test turns red).
The review's remaining items:
- _make_error_class captures Exception and setattr as def-time defaults, so a
rebind of __main__.Exception/__main__.setattr cannot break the rejection
constructor.
- dispatch binds _lossless_json_violation, asyncio.get_event_loop, and the
channel's send method into _run locals before the program runs, so a rebind
cannot turn a legitimate binding call into an exception or a wall-clock
timeout.
- The note (en + zh) corrects the stdin coverage phrasing: d3f9f57f5's direct
EOF-observing case is the in-tree pin, not an approximation.
- Collapse two stray double blank lines in the test file.
Pairing re-recorded.
The review's three remaining items:
- A regression test rebinds __main__.ProtocolChannel.read_frame_async and asserts
a binding reply still round-trips (the pump's reader is a bound method
captured by _run before the program runs).
- The settlement note (en + zh) records that send_done's frame-shape check uses
_run's bound _str/_isinstance.
- The staging-removal comment no longer claims teardown retries tracked state:
teardown deliberately does not sweep staging, so a removal failure is the one
case the gone-by-settlement contract degrades on.
Pairing re-recorded.
Chinesezjc