fix(tools): gate the unix-only process test helpers for windows builds - #77
Merged
Merged
Conversation
The bounded-child regression test in tools/process.rs is #[cfg(unix)] (it drives a real `sh` and a backgrounded grandchild holding the pipes), but the module-level `use super::*` and the `shell_command` helper it feeds on stayed ungated. Under cfg(test) on Windows the test is compiled out and the leftovers trip `-D warnings` (unused import + dead code), which is how pinjou-agent#595's windows-rust-test caught it after the 2026-09-22/23 batch landed (#63 introduced the module). Gate both items with #[cfg(unix)] so the module is empty rather than broken on Windows; no behavior change on unix targets. Signed-off-by: asto18089 <asto18089@126.com>
Collaborator
Author
|
@JensenChen28 批次合入后在父仓 Windows 全量测试暴露的编译回归（#63 引入，2 行门控修复），阻塞 r3 收口（Pinvou/pinvou-agent#595），麻烦批准，谢谢。 |
asto18089
added a commit
that referenced
this pull request
Sep 27, 2026
A durable task whose turn hit an approval or user-input prompt was still
bounded by the 30-minute task wall_time, which interrupted the turn at
expiry and denied the pending decision on the human's behalf. The window
is now excluded from the wall clock (like the engine-side turn budget),
the idle clock pauses and restarts fresh when the window closes, and the
exclusion is bounded by a fail-safe cap. Three design holes close the
gaps the first landing shipped with:
- Answerability: the pause applies only where a host can actually
deliver the decision. The runtime API's task threads are served by
HTTP decide_approval/submit_user_input and pause; the TUI's private
task runtime has no such channel, so a prompt there never arms the
window and the wall clock releases the worker at its own deadline
(before, it parked the worker for the 24h cap).
- Aggregate bound: the cap now bounds the windows summed across the
whole task, not only each open window — a task chaining short prompts
can no longer sum its way past the cap once per prompt.
- Park-poll backoff: while a window is parked the journal poll backs
off from the 200ms catch-up cadence, doubling up to 2s; an arriving
decision broadcasts on the subscription and wakes the loop early, so
the backoff only removes the empty polls.
Pinned by red-green tests: an unanswerable surface keeps the wall clock
running (mutation check fails without the gate), chained short waits
sum to the cap and terminalize between windows (fails without the
hoisted aggregate check), and every wait-writer surface that arms the
window still completes on a late answer.
Revert "revert(tasks): move the human-wait clock pause to its own PR"
This reverts commit ce5a01fe93fde5a17e94f44b9127d3c00d0782c2.
feat(app-server): resume a dropped event stream before interrupting
A dropped SSE connection is not evidence the turn died — the runtime's
lifecycle task keeps it running — yet the bridge interrupted the turn on
first stream failure. That killed healthy turns: an idle stall on the
child's event stream (bounded at 60s) or any transient transport error
ended the turn even though the model was fine.
stream_turn_events now reports how the connection ended via StreamDrop:
the last consumed seq and whether the failure was the streaming writer
itself. message_thread reconnects with since_seq=last_seq — the runtime
treats since_seq as exclusive, so the replay picks up exactly the events
that never arrived, and no delta is ever replayed to the client — for up
to MAX_STREAM_RESUMES (2) further connections. A writer failure (the
peer is gone; no resume can ever deliver an event to it) or an
exhausted resume budget falls through to the existing best-effort
interrupt, which remains the only cure for a genuinely wedged child.
Tests pin all three endings against a scripted-drop mock runtime: a
mid-stream drop resumes and completes with no duplicate deltas and no
interrupt, exhausted resumes interrupt the orphan exactly once, and a
writer that dies after response_start skips the resume entirely.
fix(tools): kill the child's process group on timeout, not just the child
The observed leak: a timed-out run killed only the direct child, so a
command the shell had forked (`... & sleep 60`) outlived the call — every
timed-out gate run left a live orphan, and so did js_execution, plugin,
and code_execution through the shared runner.
- run_bounded_child now spawns each child with process_group(0) (the
same shape the shell tool uses) and its timeout path SIGKILLs the
whole group via kill(-pgid), then reaps. The group is only signalled
on the timeout path; a successful run never touches it, so an
intentionally-detached `&` process survives — pinned by test.
- The core of the runner is now run_bounded_child_observed, which also
harvests the pipes after the kill: the partial output the interpreter
produced before the budget is returned with the timed-out result
instead of being discarded. run_bounded_child stays a thin wrapper
reporting ToolError::Timeout, so the existing callers keep their
semantics.
- The gate task had its own runner — timeout(cmd.output()) +
kill_on_drop, which could not take out a forked command. It now runs
on the shared bounded runner: timed-out gates die as a whole group and
carry the output printed before the kill, a Unix group-killed child
reports the conventional 137 exit code, and spawn_error is surfaced in
the result metadata so a gate that cannot even spawn remains
structured evidence instead of a raised tool error.
Regression tests assert the grandchild itself is dead after the timeout
(in the runner and in the gate), that the pre-kill output survives the
group kill, and that a detached success run leaves its background
process alive.
fix(runtime): close the approval no-resolution windows
Three windows could strand a decided approval without a published
resolution, or let the settler race a late-delivered decision:
- deliver_external_approval dropped the entry even when the waiter's
receiver was already closed, so a decision posted a moment after the
turn settled would be reported delivered while the settler had
already emitted its own deny+interrupted. The entry now stays for the
settler to reclaim; delivery to a closed receiver reports false and
the count stays put.
- The settler ran the in-loop rescue concurrently with delivery; a
decision sent between close() and try_recv() could be observed by
neither side. Rescue now happens at a single point after the loop,
ordered after any concurrent delivery, and the entry is cleaned up
only when its sender is closed (cancel_closed_pending_approval), so a
live approval reusing the id can never be removed by a stale waiter.
- The Decision(Err(_)) and Interrupted arms folded into one: both mean
"no decision reached", and both emit exactly one deny+interrupted
resolution.
Covers the contract with red-green pins: a dead-receiver delivery
never counts, the pre-close decision is honored by the rescue, the
post-settle send on a reused id never removes the live approval, and
settlement publishes exactly one resolution. RUNTIME_API.md now states
the delivery contract outright.
fix(snapshot): heal every half-written side repo a killed init leaves
The HEAD-based readiness predicate still missed two states an
interrupted `git init` produces (reproduced with git 2.54):
- HEAD written but `objects/` not yet created: the predicate called the
repo ready, so no re-init ran and every snapshot failed with "not a git
repository". Readiness now also requires `objects/` and `refs/`; a
re-init fills in whatever is missing.
- A `HEAD.lock` left by a kill during the HEAD write: `git init` then
fails with "cannot lock ref 'HEAD'" on every attempt. It is swept on
the same staleness rule as `config.lock`.
`list()` also mapped every failed `git log` to "no restore points", so a
broken side repo read as an empty history in `/undo`, `/restore`, export
and the runtime API. Only an unborn HEAD (`rev-parse --verify` exit 1) is
empty now; anything else is an error.
The timed-out-init hint no longer promises a re-init on the very next
attempt: a fresh lock left by the killed init blocks it until the
stale-lock sweep may clear it.
docs: complete the 1800s family roster
The anchor comment claimed the full roster but missed three members with
the same 1800s value: the background-task wall clock
`TaskExecutionLimits::wall_time`, the sub-agent `default_wall_time_secs`,
and the fleet `builder` role preset. Both wall clocks preempt the tools
in the family, so a family-wide bump that skipped them would change
nothing. Also note that `STREAM_MAX_DURATION_SECS` is the one member user
config can override.
test(client): hold the env lock on the Anthropic endpoint tests
The Anthropic dialect now resolves its non-streaming budget through the
process-global `non_streaming_request_envelope()`, which
`NonStreamingEnvelopeGuard` overrides (a neighbouring test injects 1s).
Two tests on the same path did not take `lock_test_env`, so a parallel
injection could cut their request off mid-flight.
fix(tools): abort the interpreter drains on the cancel path too
`abort_drains` promised that every exit that stops waiting on the child
calls it, but the most common non-success exit cannot: on a turn
interrupt the whole tool future is dropped, and dropping a `JoinHandle`
detaches its task instead of aborting it. The drain tasks then kept the
pipe read ends (and their growing buffers) alive for as long as a
pipe-inheriting grandchild held the write ends — the exact leak the
module doc describes.
Own the three handles in a `DrainTasks` guard that aborts in `Drop`, so
the cancel path behaves like every other exit; the grace-expiry exit
still aborts explicitly so the abort is ordered before the buffer
snapshots.
fix(snapshot): stop a failed re-init from disabling snapshots silently
A killed `git init` writes `core.repositoryformatversion` under
`.git/config.lock` before it creates HEAD, so the interruption that
leaves a HEAD-less side repo can also leave that lock. Git never ages
lockfiles out, so every later `git init` failed with "could not lock
config file" and the HEAD-based re-init could never succeed. Sweep a
stale `config.lock` before the init, on the same age rule as
`index.lock`.
That failure carried `ErrorKind::Other`, and the degraded-snapshot notice
only passes size-gated and `TimedOut` errors, so a permanent loss of undo
history reached only the tracing log. Give init failures their own notice
arm and hint, and build the hint router's needles from marker constants
exported by `snapshot::repo`, so the hint tests really pin the producer
text instead of hand-copied literals.
Run the first-init size walk only when `.git` does not exist at all:
while a repair kept failing, `needs_init` stayed true and every snapshot
attempt re-paid a walk of up to 200k entries.
`run_bounded_fs` now spawns with `Builder::spawn` (it abandons a thread
per expiry, so thread exhaustion is the failure it is most likely to meet
and `thread::spawn` would panic), reports a panicking probe as such rather
than as a wedged mount, and renders sub-second bounds as milliseconds.
`run_bounded_git`'s doc comment, which the `git_stderr_tail` insertion had
attached to the wrong function, moves back. The reader-leak test samples
a settled baseline so transient readers from parallel tests cannot mask a
real leak.
fix(app-server): run the orphan-turn interrupt on every turn surface
The post-stream-failure interrupt only ran when `registered_turn` was
Some, which is built only for interruptible (stdio `thread/message`)
turns — the one surface that already has a manual `thread/interrupt`.
HTTP `/thread` Message, `/prompt` and stdio `prompt/*` turns have no
client-reachable interrupt at all, so after a dropped stream their
thread rejected every later message with "already has an active turn"
until the orphan finished on its own.
Everything the interrupt needs is in scope on every surface, so build it
unconditionally; only the registry insert stays stdio-only, because only
that surface can ask for a cancel by thread id.
revert(tasks): move the human-wait clock pause to its own PR
The task wall/idle clock pause (9e667d1fc) is a task-lifecycle behavior
change rather than a review fix: it adds a terminal reason, two public
execution events and a public limits field, and it changes what
`wall_time` bounds. As landed it also regresses the TUI: TUI tasks run on
a private runtime-thread manager that no client can deliver an approval
or user-input answer to, so an unanswered prompt now holds a worker for
the 24h cap instead of releasing it at the 30-minute wall.
Take it out of this follow-up wave so it can be reviewed and fixed on its
own. The heartbeat short-circuit (1feed0794) stays, with its comment
corrected: a spurious dirty mark only arms a redundant flush. The
SUBAGENTS docs paragraph now states the real bounds on a long tool: the
child's own wall time and, inside a task, the task wall clock, which runs
while a prompt waits on a human.
fix(snapshot): bound the safety classifier's re-canonicalization
The open path bounded its workspace probe but then ran
unsafe_workspace_snapshot_reason inline, which re-canonicalizes the
workspace and HOME (twice) with plain syscalls — another unbounded
pre-git window on the same mount, exactly where the workspace probe
just bounded. Any post-probe wedge in that window parked the open
forever, so the 30s boundary the PR establishes never actually
closed.
Put the whole classifier behind one run_bounded_fs bound via
snapshot_safety_reason_bounded; the classification semantics are
untouched. A regression test pins that the bounded classifier agrees
with the inline one on every anchored classification (HOME, Desktop
collection, unrelated dir); the timeout leg is the pass-through
already pinned by bounded_fs_probe_times_out…, since a genuinely
wedged mount cannot be induced in a unit test.
docs: list the model stream cap in the 1800s family roster
STREAM_MAX_DURATION_SECS shares the family's value and generation but
was absent from the roster the family's comments keep in sync, so a
family-wide bump could silently leave it behind. List it at the anchor
and cross-reference the roster from the constant.
test(tools): widen the gate-kill deadline for login-shell startup
The 800ms deadline raced the login shell's profile sourcing on slow
runners: the child could be killed before echo landed its pid and the
expect would panic. Five seconds still sits far below the sleep-60
payload the deadline exists to cut off.
fix(snapshot): key the wedge hints to their producer text
The degraded-snapshots notice selected its remedy hint inline; a
timed-out git init (which leaves no index.lock and now heals by
re-initing on the next attempt) was advised to wait out a stale lock
for about an hour. Extract the selection into snapshot_failure_hint,
give the init timeout its own re-init hint, and pin every branch
against the producer message text with unit tests.
fix(snapshot): honor cap=0 and keep the init-timeout kind
- The first-init size walk ran even when cap_bytes was 0, the
documented max_workspace_gb=0 opt-out, so a workspace too large to
walk within SIZE_WALK_TIMEOUT failed init with a misleading
wedged-filesystem error and re-paid the failing walk on every later
attempt. Skip the walk, timeout included, when the gate is disabled.
- A timed-out git init was re-wrapped with io::Error::other, dropping
the TimedOut kind the turn pipeline's degraded-snapshots notice
gates on — the exact swallow the size-walk re-wrap eight lines up
documents as the contract. Preserve the kind.
- Extract the stderr-tail construction into git_stderr_tail and pin
the truncation semantics (last N characters, ellipsis, multibyte
safety) with unit tests.
fix(runtime): make the approval exits race-free by construction
- The interrupt exit's last-chance rescue dropped the oneshot receiver
after a final empty check, but a send racing that window could still
land on the alive receiver, report delivered to the HTTP layer, and
then be discarded by the waiter in favor of the interrupted deny.
Close the receiver before the final try_recv: a racing send now fails
immediately (reported not-delivered) while a value that landed before
the close stays readable, so a decision is either rescued or never
promised.
- settle_claimed_turn_failure scanned the pending-approval ids under
the lock and removed them one by one afterwards; a concurrent
delivery could steal an entry between the two steps, fail its own
send on the monitor-dead receiver, and leave the approval with no
resolution event at all. Collect and remove in one lock hold, and
pin with a test that the publish fires even when the deny cannot
deliver.
docs: align the timeout family and fix claims
- The four 1800s-family comments each listed a different subset and the
anchor itself was missing two members. Anchor the full roster at the
core dispatch backstop's comment and point the other three at it.
- The heartbeat short-circuit's second claimed benefit was not real:
the run loop rebuilds the debounce timer on every event, so real
mutations wait out the same starvation with or without the spurious
dirty marks. Say what the short-circuit actually buys (no manager
lock per tick, no redundant flushes) and state the limitation.
- Document the merged-window premise of the human-wait guard: windows
only ever serialize today because the engine blocks inside one
prompt at a time; nested prompts need paired windows first.
- Correct the sandbox transport comment: local foreground Bash clamps
at the same 600s, the contract bash timeout can run far longer, and
the external-backend path never consumes the tool timeout at all.
refactor(tools): extract the shared drain aborts
The five-line abort block was copied across all three exits of
run_bounded_child, and the wait-error exit is exactly where a copy was
missed before this PR fixed it. Fold the aborts into one helper so the
next exit path cannot forget them, and replace the module doc's pointer
at the PR-level Windows disclosure with the substance inline — the
lingering blocking-pool read ends when the grandchild exits, which
bounds the leak by the grandchild's own lifetime.
test: pin the timeout follow-up fixes
- client: the envelope-exceeded exit must fire the /models recovery
probe once the connection has degraded — two stalled requests then
exactly one probe, verified end to end (the probe used to leave
health bookkeeping untouched).
- runtime_threads: monitor-death settlement must publish the stranded
approval as deny+interrupted, clear the pending map, and resolve the
decision channel.
- snapshot: HEAD-less side repos re-init and snapshot cleanly (see the
previous commit).
- rlm: the sub-query budget setter floors 0 at one second.
- vision: pin the ToolError::Timeout variant, not just the message —
the pre-fix execution_failed form shares the "timed out after"
substring, so the old assertion survived a classification regression.
- web: bind the fetch_url schema text's "max 300,000" to
HARD_MAX_TIMEOUT; the constant alone never caught schema drift.
- vision: reword the envelope comment — retries do fire for slow
failures, they just rarely fit before the shared budget expires.
fix(snapshot): report wedge failures as errors
Three wedge paths were quieter than the follow-up claimed:
- open_existing swallowed a timed-out workspace probe into Ok(None),
which both read-only consumers render as "no restore points" — the
exact silent omission their Unreadable arm exists to avoid. Surface
the probe failure as Err so users see "could not be opened" plus
the reason.
- The first-init size-walk timeout was re-wrapped with
io::Error::other, dropping the TimedOut kind the turn pipeline's
degraded-snapshots notice gates on; the user-visible stderr warning
was lost to tracing. Preserve the kind.
- That notice's hint always blamed a stale index.lock, which no pre-git
probe ever created. Pick the hint by failure stage.
- The wait-error exit of the bounded git core now kills the child too
(std handles have no kill_on_drop).
- Pin the HEAD-less side-repo recovery with a regression test: a .git
without HEAD must re-init and snapshot cleanly again.
fix(runtime): close the approval delivery race
The interrupted-approval exit checked the oneshot one last time and
then walked two real await points before dropping the receiver, so a
decision delivered in that window still reported delivered=true while
the monitor published the interrupted deny over it. Drop the receiver
as soon as the final check comes up empty: the arms below never read
it again, and a send that lands afterwards now fails on the dropped
receiver and reports not-delivered, making the documented invariant
literally true.
Also point the dynamic-tool wait comment at the anchored 1800s family
roster instead of re-listing a stale subset.
docs: disclose the per-leg MCP budget and the task wall clock
- MCP.md (en+zh): execute_timeout applies per leg (send, then read),
so a server that drains its input barely within the budget can push
the total toward twice the configured value.
- SUBAGENTS.md (en+zh): inside a durable task the heartbeat-floor
guarantee is additionally bounded by the task wall_time — the wall
clock pauses for pending human prompts (24h fail-safe per prompt)
but otherwise wins over an in-flight sub-agent, so raising the tool
or execute budget above the remaining task wall has no effect.
test: harden the timeout regression pins
- client: widen the two sub-3x timing margins (the stream-open probe
answers at 4s against the 2s injected envelope; the pinned-total
probe answers at 4s against a raised 12s pinned budget), so a slow
runner cannot blur red-state detection.
- rlm: give the hanging-child budget test an outer 5s deadline so a
regression back to the 120s const fails in seconds instead of
hanging the suite.
- tasks: regression test that the gate child is actually killed and
reaped at the gate deadline (the kill_on_drop claim previously
rested on static reasoning); removing the flag fails the test.
- mcp: pin the engine-side stdio proxy's budget wiring (tools/call at
1800s, separate from the 120s generic request budget) — the proxy
has no behavioral timeout tests, so the constants are the seam.
fix(client): route the Anthropic budget through the test seam
The Anthropic dialect's non-streaming request read the raw
NON_STREAMING_REQUEST_ENVELOPE constant, so no test could shrink the
budget and the cutoff was pinned by nothing (the PR body's earlier
round implied a weaker test existed where none did). It now goes
through the same injectable accessor as every other non-streaming
completion, with a wiremock test pinning that a provider answering
past the envelope is cut off and that the failure reads as a timeout.
fix(mcp): state the send budget instead of the elapsed display
The send-timeout error embedded tokio's Elapsed display ("deadline
has elapsed") next to the already-stated budget; drop it and keep the
budget figure, which is the actionable part.
fix(runtime-api): forward the approval posture to compat streams
The auto-review fail-closed producer emits posture: auto_review, but
the compat projection dropped it, so those clients could not tell an
execution-policy denial from a plain auto-deny (the raw event stream
always carried it). Forward it and pin it in the projection test.
fix(runtime): publish the channel-closed approval exit
The Decision(Err) arm — the decision channel closing mid-delivery —
resolved the wait by denying the engine without emitting
approval.decided, the one exit that made the documented
every-resolution-published contract false. It now publishes like any
other forced resolution (deny + interrupted), and the doc sentence
softened for it in the previous round is restored to the universal
claim, which now holds.
fix(snapshot): bound the pre-git probes and carry the wedge evidence
The bounded git core never ran before two plain blocking calls had
already parked the turn pipeline: workspace canonicalize (twice —
once in open_or_init, again inside snapshot_dir_with_home) and the
first-init size walk, each unbounded, on the very wedged NFS/FUSE
mount the bounds exist for. Both probes now run on a helper thread
under hard bounds (30s path resolution, 120s size walk); a wedge
errors the open into the usual degraded-snapshots notice, the
availability surface reads it as "no snapshots" instead of hanging,
a fast canonicalize failure still falls back to the raw path exactly
as before, and the module claim now covers the whole open path.
Two diagnosability gaps in the bounded core close alongside: the
TimedOut error now carries the last 500 characters of what the child
managed to say (hook and clean-filter diagnostics are exactly what
explains a timeout), and the restore commit-message label slices the
id through str::get instead of a bare byte range — defensive only,
ids are git-produced hex today.
fix(tasks): pause the wall and idle clocks for human waits
A durable task whose turn hit an approval or user-input prompt was
still bounded by the 30-minute task wall_time, which interrupted the
turn at expiry and denied the pending decision on the human's behalf
— the exact failure the timeout audit set out to remove, and the
headline guarantee silently not holding on the background path.
The turn loop now recognizes the human-wait windows from the journal
(approval.required / user_input.required through their paired closing
events) and excludes them from the wall clock exactly like the
engine-side turn budget does, publishing the window as
HumanWaitStarted/Ended so the worker supervisor's guard follows in
lockstep. Silence during a human decision is expected, so the idle
clock pauses too and restarts fresh when the window closes. The
exclusion is bounded by a per-prompt fail-safe cap (24h default,
configurable with the other execution limits) with a dedicated
HumanWaitTimeout terminal reason, so a prompt nobody will ever answer
still releases the worker.
Tests: the wall clock survives a 2x-budget wait and completes on the
late answer; the fail-safe cap releases an unanswered prompt; the
supervisor guard completes a task whose executor waited past the wall
budget.
fix(tests): pin the fetch hard cap against schema drift
Dropping the strict-equality assertion left no test binding the
fetch_url schema's "max 300,000" to HARD_MAX_TIMEOUT; pin the
absolute value beside the coherence invariant.
(cherry picked from commit 7a03db628674e88ba38e216587c40543149f7be8)
docs: correct timeout claims and cross-reference the 1800s family
Comment and contract-accuracy fixes from a fresh review pass:
- The snapshot stale-lock doc no longer claims a lock older than the
command bound cannot belong to a live writer (a git wedged in
uninterruptible I/O can), and names why removal stays benign; the
bounded-git comment names the real sandbox-CLI gap instead of
claiming to mirror it.
- The OpenSandbox comment matches its own rationale (the Bash
foreground cap is effectively unbounded), the fetch-envelope
comment discloses the pre-envelope SSRF pre-flight lookup, the
self-update comment states the real ~58 MiB at 100 KiB/s, and the
MCP execute-timeout comment discloses the per-leg worst case.
- The 1800s definitions in core, the sub-agent limits, the MCP
defaults, and the dynamic-tool wait now cross-reference their
family, and subagent_limits documents that a task wall_time
preempts the tool timeout in background tasks.
- RUNTIME_API.md: the restore id must match any snapshot the side
repo knows (not just the listed window) and the guarantee is that
a non-matching id never reaches a git command; the example id is a
real 40-hex SHA; the common-events list marks approval.timeout as
legacy; and the every-resolution claim is scoped to forced
resolutions.
(cherry picked from commit bc447c1f06f9a57efdfb09dafccd285deb8fcf3d)
fix(rlm): floor the sub-query budget and share the 120s default
with_sub_query_timeout_secs accepted 0, which builds a tokio timeout
that fires immediately and kills every child query; floor at one
second. The compatibility session's 120s default now references
CHILD_TIMEOUT_SECS instead of duplicating the literal, and the inline
round comment no longer implies a single minutes-long sub-query
survives on a path where the bridge still bounds each one.
(cherry picked from commit 83dc99763a4fdcfc5e8ee75a031acd252f10ebc6)
fix(tools): abort the drains on the wait-error exit too
The child.wait() error path propagated before aborting the pipe-drain
tasks and the stdin writer, letting pipes outlive the call on the one
exit that was neither a timeout nor a clean exit. The module doc also
states the EPIPE contract is Unix-only now.
(cherry picked from commit 04bd7968ac86d7d46cc25661f8f5f48aa3b474e1)
fix(app-server): interrupt orphaned turns and bound the health probe
Three adjacencies the new bounds made reachable or left behind:
- A stream that dies mid-turn (idle/header timeout, transport error)
left the runtime turn running unattended; a retried message was
rejected with "already has an active turn" until the orphan
completed. Leave a best-effort interrupt behind, reusing the same
bounded POST as the interrupt handler.
- The /health boot probe only checked its deadline after send()
resolved, so a child that accepts but never writes headers hung
the probe forever and respawned a child per message. Bound each
attempt by the remaining deadline.
- The proxy handler held the config read guard across the upstream
round trip (now up to 30 minutes), blocking config writes and
every later resolve. Drop it once the endpoint is resolved.
(cherry picked from commit a0f109364dd0e73ca148c54658fad0e84c627226)
fix(client): probe recovery after the request envelope is exceeded
The envelope-exceeded branch marked the failure and returned without
maybe_probe_recovery, so connection health stayed degraded until the
next real request succeeded — exactly the wedged-provider case where
the /models health probe matters.
(cherry picked from commit cf10b3cbcc30270708858e7826cf02bf9f5fd1a3)
fix(vision): classify the envelope timeout and bound the permit wait
The envelope expiry surfaced as execution_failed instead of
ToolError::Timeout, breaking the error taxonomy every other bounded
tool follows; receipts and severity now classify it as a timeout.
Acquire the runtime-chat inference permit inside the envelope: the
ownership window is part of the bounded call, so a contested permit
can no longer park the tool outside every total bound.
(cherry picked from commit 54e1f88e4672dace81c2f2fd1da3f19b09ded41b)
fix(tasks): short-circuit liveness heartbeats before the state lock
ToolHeartbeat never mutates the record, but each tick still took the
manager-wide state lock for a no-op and set dirty=true, which deferred
the persistence of real mutations past the debounce window for the
whole silent tool. Return before the lock; the supervisor guard is
already refreshed by the progress classification.
(cherry picked from commit a689b703dd3c381f49430661c06cc1293e595ca1)
fix(runtime): settle stranded approvals and the delivery race
Two exit-hardening fixes to the external approval wait:
- When the monitor task itself dies, nothing resolved the pending
approval: the map entry leaked and clients never got the promised
approval.decided. settle_claimed_turn_failure now settles this
turn's approvals the same way the interrupt exit does.
- deliver_external_approval removes the map entry before sending, so
a user decision can land in the oneshot after both rescue checks
saw Empty and then be reported as an interrupted deny. The
interrupted exit now makes one final try_recv and routes a found
decision through the decision path; a send landing after that
finds a dropped receiver and reports not-delivered.
(cherry picked from commit 484c1dc6f1239efd98e5b70d69fa625e0c2d4b06)
fix(snapshot): re-init after a timed-out partial git init
A killed git init can leave the side-repo .git directory created
without a HEAD, and needs_init keyed on directory existence alone
skipped init forever, fast-failing every later snapshot with "not
a git repository". Use the same readiness predicate as open_existing
git init is idempotent, so re-initializing the partial directory is
safe.
(cherry picked from commit 7b82f3dcc075ab0e7d12cdbe0ecb362866d038f5)
fix(exec): resolve user-input requests in headless runs
The engine's request_user_input wait is unbounded now that it is
recognized as human-paced, but the headless exec event loop only
auto-resolves approvals and never touches user-input requests: a
run whose model asks a clarifying question parked forever where the
old 300s cap used to resume it. Cancel the request instead — the
tool returns a cancelled error and the turn continues — and say so
once on stderr.
(cherry picked from commit e4c47e237835c25b8b250b70c2c448b5ba2f311f)
fix(tui): recognize restore carriers by provenance (replaces #74) (#76)
* fix: recognize restore carriers by provenance
* fix: correct restore residual docs and pin the anchor order
Review follow-ups on the restore-by-provenance change:
- The loose fallback fires on the structural absence of a stamped
carrier, not on session age: it also covers histories that never
compacted, where checkpoint is None and a pasted header is dropped
outright. State that in the doc comment instead of equating the
branch with pre-provenance saves.
- Point the history_recognition doc at the live system-prompt scanners
(extract_compaction_summary, strip_summary_text);
is_compaction_summary_text has no callers.
- Add the mirror-order regression test: a pasted full summary BEFORE
the real carrier is the anchor-theft order, so the test pins the
authoritative summary landing at the real carrier's index, with
distinct carrier/checkpoint texts so leaving the old carrier in
place cannot pass.
---------
Co-authored-by: asto18089 <asto18089@users.noreply.github.com>
fix(tools): gate the unix-only process test helpers for windows builds (#77)
The bounded-child regression test in tools/process.rs is #[cfg(unix)]
(it drives a real `sh` and a backgrounded grandchild holding the pipes),
but the module-level `use super::*` and the `shell_command` helper it
feeds on stayed ungated. Under cfg(test) on Windows the test is compiled
out and the leftovers trip `-D warnings` (unused import + dead code),
which is how pinjou-agent#595's windows-rust-test caught it after the
2026-09-22/23 batch landed (#63 introduced the module).
Gate both items with #[cfg(unix)] so the module is empty rather than
broken on Windows; no behavior change on unix targets.
refactor(tui): extract checkpoint recognition to leaf module (#73)
* refactor: extract checkpoint recognition to leaf module
* docs(tui): point the moved recognition docs at their consumers
The leaf has no consumers of its own, so "every consumer here" was
vacuous; name the compaction/runtime_handoff consumers instead. The
module doc's no-reverse-edge claim gains its production qualifier and
names the disclosed test-module residue.
---------
fix(computer-use): bind the region origin in screenshot rasters (#72)
* fix: bind region origin in screenshot rasters
* test: pin screenshot raster-origin binding
* fix(computer-use): aim screenshot regions in global screen points and bind measured pixels
Review of the #554 first cut surfaced three gaps this closes:
- darwin derived scale from _spdisplays_resolution, which is the POINT
resolution on Retina panels — every region shot bound scale 1 while the
PNG carries 2x, so clicks still landed 2x off (verified on real
hardware). The binding now measures the actual PNG (sips, as zoom
already does) and the metadata fallback uses the real pixel field
_spdisplays_pixels.
- darwin treated -R as display-space with -D; the weight of evidence says
screencapture crops -R in GLOBAL screen points. A region now pins the
crop itself (no -D) and binds the region verbatim — which also makes
region shots work on displays whose origin macOS will not report — and
clamps only against a complete display union, since clamping on a
partial union could pull another display's region into the main one.
- linux rejected negative regions although grim/scrot/import crop in
global layout points that go negative on exactly the layouts the
virtual-screen binding exists for; and it hardcoded scale 1 while grim
renders at the highest output scale, so virtualScreen now carries the
contributing scale. An unbound shot no longer advances the backend's
last raster — the crop rides along as 'path' so the previous binding
stays whole and a follow-up zoom cannot slip past the foreign-source
guard with a stale frame.
The server stops guessing: a screenshot with neither points nor origin
keeps the previous binding (or none — coordinate targets then fail closed
with no_raster) instead of binding (0,0), and over ssh the backend's own
note is joined with the scp hint instead of overwritten. The screenshot
schema now describes the global frame and the clip.
* test: pin global region frames, measured pixels, and the unbound-raster contract
Six mutation checks revert each binding fix and fail the suite: the raw
-R passthrough, resolution-only metadata scale, the linux n >= 0 gate,
the linux scale-1 hardcode, the server (0,0) fallback, and the darwin
file-swap under a stale frame. The Retina fixtures use the real
system_profiler field pair (resolution = points, pixels = backing).
* fix(computer-use): aim win32 screenshot regions in global points
Review round on #72 surfaced the last cross-platform coordinate
mismatch: the shared schema, darwin, and linux all define the
screenshot region in GLOBAL screen points — the frame list_displays
and cursor_position report, negative on multi-monitor layouts — but
win32 required non-negative x/y and consumed them as offsets from the
VirtualScreen top-left. On layouts whose VirtualScreen origin is not
(0,0) the same tool arguments cropped a different position, and a
region on the left/upper display (negative global coordinates, e.g.
[-1500,100,400,300] with VirtualScreen x=-1920) was rejected outright.
The win32 backend now consumes global points like the other backends:
the script derives the VirtualScreen-relative crop offset itself
(rx = region.x - bounds.X; node cannot know the bounds before the
script echoes them), clamps it against the virtual screen, and copies
at bounds + offset as before. Node recomputes the same clip with
clampRegion and restores global points, so the bound points remain the
crop actually taken. Tests pin the contract from global-coordinate
inputs — including a negative region aiming at the left display and a
region clamping past the left/top edge — and the malformed-region gate
now covers junk, arity, and degenerate sizes without the negativity
clause.
---------
fix: relativize context source labels to repo root (#70)
Chain-segment ("<!-- scoped instructions: ... -->") and
<project_rule source=...> labels carried absolute paths into the
pinned system prompt, so a checkout move or recase changed the prompt
prefix and could emit a spurious <context_update> history append.
Render both labels repo-relative (git-root-relative, forward
slashes), falling back to workspace-relative for rules outside any
checkout and to the absolute spelling only for paths outside the
label root (unreachable by construction).
Pins Pinvou/pinvou-agent#514.
fix(tui): close the fresh-review residuals (rlm, registry pin, echo) (#71)
* fix: add activation hints to rlm handle_read text
* fix: add the fourth side to the registry cap pin
* test: cover every submission-echo self-start path
* fix(rlm): teach the activation path on the stdout preview and truncation marker
The two same-class residuals registered by the first round of this PR
are now fixed instead of tracked: the handle-route preview line ("N
chars; retrieve via handle_read", emitted on every eval output past the
handle threshold) and the preview_output truncation marker both embed
the shared HANDLE_READ_ACTIVATION_HINT, each with a runtime pin.
* test(registry): pin the registry_sync description wording and sync the side list
The enumeration comment now names all four text sides and both
compile-time pins, and the registry_sync tool description gains the
missing runtime "eight" pin; the mcp-discovery skill markdown remains
the one side reached only by the tripwire message.
* test(forkguard): make the composer-shell accidental-dispatch defense real
The composer shell test pre-advances the model-client call counter so
every provider request blocks, making the documented bounded-timeout
defense true (the first request used to be absorbed by the canned
completion); the goal continuation test documents why its host-status
sync stays even though the constructor already armed the objective.
---------
fix(mcp): stream session boot and make tool_search boot-aware (#69)
* fix(mcp): stream session boot, make tool_search boot-aware
The session-boot connect pass returned only after every pending server
settled, and only then stored results into the pool. Two slow remote
401s could therefore hold the local command servers' tools off the
shelf for the whole boot window (~88s in the field), while tool_search
consulted a turn catalog built from the empty pool — and a bare empty
result is indistinguishable from "no server configured", so a
forced-capability request issued shortly after launch collapsed into a
definitive "capability unavailable" verdict even though the capability
came up moments later.
Three coordinated seams:
- boot: `spawn_pending_connects` returns the unfinished JoinSet and the
session-boot task applies each result under a short pool lock the
moment it settles — first-settled-first-available — sending Progress
per settle; `connect_all` keeps the drained batch wrapper.
- turn loop: while boot is in flight, each tool batch folds servers
that became ready since the turn started into the live catalog
(append-only, deferral mirrored from the turn build), so an in-turn
tool_search retry can find them. The boot update channel is left to
the idle seam, which owns the failure briefing — it must never be
appended mid-tool-loop.
- tool_search: while boot is in flight the result carries an `mcp_boot`
block naming the servers still connecting, and the tool description
states the retry contract, so a miss reads as "not yet", never as
"absent".
A settled boot keeps every existing contract byte-compatible: the
payload carries no booting block, the refresh is a no-op, and the
drained batch shape remains for `connect_all`.
Tests use a real stdio fixture server: fast-server tools become
snapshot-able while a hung server is mid-timeout (mutation-checked
against the batch-return shape); the refresh append/deferral/
no-duplicate contracts; the connecting-status truthfulness boundaries;
and the settled-boot payload shape.
* fix(mcp): narrow boot appends and trust pool diagnosis
Review of 79853e30e surfaced four MAJOR seams the first cut left open;
this closes them without changing the streaming shape.
- The boot-window catalog refresh now applies the same narrowing the
turn build does: the MCP feature and tool-security gates, the turn's
ToolSurfacePolicy allow/deny predicates, and apply_mcp_tool_deferral
itself for the deferral treatment. A tool the build stripped can no
longer re-enter mid-turn, where a search hit would defeat the
denied-name concealment and a declared activation would serialize an
operator-excluded schema into the request.
- An always_load append now resolves eager and joins the active set,
with the caller declaring the growth through the existing
tool_surface re-pin; a turn starting while boot is in flight stamps
"mcp-session-boot" up front, so the next turn's snapshot picking up a
newly settled tool is a declared re-pin instead of undeclared drift
tripping the C5 guard (the stamp is inert when nothing changed).
- The connecting status reads the pool's live truth: catalog_authorized
servers count as answered (wider than is_ready, matching all_tools)
and connect_backoff entries count as diagnosed, so a server that
already settled with a failure is never reported "still connecting"
before the queued boot update is drained; collect_pending_connects
drops the backoff entry when it re-pends a server, preserving
"backoff entry => no attempt in flight".
- The booting note and comments now describe what the code does, the
JoinError attribution lives in one helper, and the misspelled test
name is fixed.
New tests drive the turn loop end to end (search finds the refreshed
tool and carries the boot note; the boot-window turn start declares its
surface change), plus policy/security, always-load, pool-side-diagnosis
and drain-lag regression tests.
* fix(mcp): keep the retry ladder and close gate gaps
Round-2 review of the branch surfaced one consumer regression and two
boot-window gate gaps; this closes them without changing the streaming
shape.
- collect_pending_connects marked a re-pended server by dropping its
backoff entry, which also reset the failure ladder: every automatic
reconnect cycle restarted a dead server at the 30s base instead of
climbing toward the 600s cap, and mcp_tools' per-turn connect_all
paid a full spawn+handshake for each reset. The entry now survives
the re-pend and a connects_in_flight set marks the attempt, so
settled_failure_names still splits "diagnosed, cooling down" from
"connecting right now" — and the ladder climbs.
- The tool_search seam re-folds the live pool before searching: a
server that settled while earlier tools of the batch were executing
used to produce a bare miss with no note at all, because the status
correctly stopped naming it while the batch-start fold had not
picked it up yet.
- mcp_boot_search_status now applies the same MCP feature and
tool-security gates as the refresh: a tool-security turn's catalog
has no MCP surface, so its search must not carry a connecting note
for servers that turn can never reach.
- The boot re-pin declaration is first-set wins across all three boot
stamps (turn start, Finished, drain): an already-pending `resume` or
`tool_surface` reason keeps ownership instead of being washed into
`mcp-session-boot`.
- allows_tool is promoted out of #[cfg(test)] so the refresh and the
build spell the narrowing in one place, the batch wrapper's doc
describes its actual caller, and two overreaching comments (the
backoff entry's "exactly while no attempt is in flight" claim and
drop_all_connections' settle guarantee) are rewritten around what
the code actually does.
Four new tests pin the behavior fixes and were mutation-checked:
restoring the entry drop, deleting the search-time re-fold, removing
the status gate, and overwriting an already-declared reason each turn
the matching test red.
---------
fix: 评估 #67 与上游重叠面——采纳上游更合理的四处做法 (#68)
* fix(tui): 恢复历史隐藏内部运行时 handoff，保留 MCP system cell 例外
吸收上游 6362e1e84 的通用过滤：history_cells_from_message 在
restored checkpoint 分支之后接入 is_internal_runtime_handoff，
background shell completion 等运行时 handoff 恢复后不再渲染为
User cell（fork 同样存在该 bug）。过滤仅作用于显示层，持久化与
模型侧 api_messages 逐字节不变。MCP briefing/recovery 在更早分支
已按 fork 决策返回 System cell，不会落入新过滤器，行为保持。
测试按上游语义改写并新增：
- apply_loaded_session_never_restores_background_shell_event_as_composer_draft
覆盖 current/legacy 两种 provenance、保存与会话消息字节不变、
恢复后 cell 序列精确为 [User, Tool, Assistant]；
- 新增 apply_loaded_session_keeps_user_authored_shell_event_lookalikes：
无 provenance 的用户手打 envelope 形似物仍可见；
- 新增 restored_background_shell_completions_yield_no_cells_but_user_text_survives：
单元级钉住两种 provenance 隐藏、普通用户文本幸存；
- backtrack 回归测试追加 handoff 不计为用户发言的断言。
红→绿：改写后的 resume 回归、单元级隐藏断言与 backtrack 断言在
改动前均失败，改动后通过；forkguard MCP system cell 测试全程通过。
验证：cargo fmt -p codewhale-tui；cargo clippy -p codewhale-tui
--lib -- -D warnings 通过；history::/ui::/runtime_handoff::/
session_manager::/session_peek::/compaction:: 共 924 项通过；全量
lib 11849 项通过（4 个 remote_control/runtime_threads 用例因共享
runtime 目录在并行下环境性失败，单独运行通过，与本次改动无关）。
* fix(tui): read 结果按预算自限后不再被上下文压缩器二次截断
移植上游 e7f7c71e2 的 context 压缩层豁免守卫。read 工具按字节预算自我
截断并以页脚声明"从 offset 续读"，但压缩器硬限 12,000 字符会把打满预算
的 read 结果再做一次 head/tail 压缩，内容被切、续读契约丢失。
- compact_tool_result_for_route：metadata 带 read_budget_bytes 且 raw
字节长度不超过预算时原样放行（位置与上游一致：evidence_available
早退之后、subagent 摘要器之前）；超出自身预算仍走常规压缩。
- read 工具两条物理路径在受预算约束的成功结果上写入 read_budget_bytes：
execute_contract_read（read 原语，50KiB 预算）与 render_line_window
（read_file 遗留阅读器，16KiB 预算）。值取最终输出实际字节长度：
窗口预算约束的是内容，续读页脚与 <file> 包装叠加其上，只有按最终
payload 声明，守卫条件才对打满预算的结果成立。
- 测试：engine 层验证带 metadata 原样放行、无 metadata 与超预算仍压缩；
file 层两条路径各验证打满预算的结果携带 metadata 且过压缩器后页脚
完整存活（端到端）。
验证：cargo fmt；cargo clippy -p codewhale-tui --lib -- -D warnings 通过；
cargo test -p codewhale-tui --lib -- core::engine::tests tools::file
531 通过 0 失败；contract_read read_budget budgeted_read 过滤 7 通过。
改动前 3 个新测试均红（被二次压缩 / 缺 metadata）。
* fix(tui): 行摘要器输出有界的 child 路由回执
上游 0c03b5a81 在 emitter 侧让每行 compact status 携带有界化的 typed
child_route，fork 的 subagent 行摘要器此前丢弃该字段，unscoped fleet
监视面上 child 实际使用的 provider/model 路由到不了模型。
- summarize_subagent_snapshot 每行在 receipt 可打印时输出 route 行：
provider/model，附 route_source 与 resolved_profile_id（连接身份）；
字段缺失或形状不符时静默跳过，不打印空占位。
- 有界：各字符串字段 preview 截 64 字符，整行硬性守卫 200 字符，不
依赖 emitter 侧 compact_child_route 的 ≤1024 字节保证。
- 兼容三形状：对象 receipt、裸 "provider/model" 字符串、snapshot
wrapper 包裹（wrapped 行优先，envelope 兜底，与 transcript_handle
的 fallback 规则一致）。
验证：cargo fmt -p codewhale-tui；cargo clippy -p codewhale-tui --lib
-- -D warnings 通过；RUST_MIN_STACK=16777216 cargo test -p codewhale-tui
--lib -- core::engine::tests 381 通过（新增
forkguard_fleet_summary_rows_carry_bounded_child_route 与
forkguard_subagent_projection_child_route_reaches_summary_row，
先红后绿）。
* style: 自检微调——守卫注释改 fall-through 表述，route 行缺失段不再打占位
---------
feat(subagent): list host-presented profiles in agent roster action (T7) (#65)
* feat(subagent): list host-presented profiles in agent roster action (T7)
The agent tool's action=roster only ever returned the 8 builtin Fleet
roles, while the base contract tells hosts to present exact prompt-only
profile ids themselves — leaving embedder-injected profiles (Pinvou's
expert cards) undiscoverable by the model. The roster payload now adds
host_profiles (sorted by member id, capped at 48 with
host_profiles_truncated and host_profile_count), listing exactly the
members profile=<member_id> can resolve: the acceptance contract
(config origin, prompt-only, no built-in role-token shadow, bounded
selector) is single-sourced in is_spawnable_host_profile and shared by
the listing and spawn resolution, so the roster never advertises an id
spawn would refuse. Builtin members keys are unchanged and the payload
stays stable when no host profiles exist.
Tests: forkguard_agent_roster_lists_only_spawnable_host_profiles pins
that the listing mirrors spawn acceptance (mixed origins, route pins,
role-token shadows, oversized selectors);
agent_roster_lists_host_presented_profiles_spawn_resolves;
agent_roster_truncates_host_profiles_at_the_listing_cap; the existing
roster action test pins the new empty-shape keys.
* feat(subagent): let roster profile_query filter host profiles (T7)
The roster host-profile listing caps at 48 rows and the agent schema has
no pagination, so embedding hosts presenting more spawnable profiles than
the cap left their tail permanently undiscoverable: the model could see
host_profiles_truncated=true but had no way to reach the dropped ids, and
profile=<member_id> requires knowing the exact id. Host-review follow-up
on the swarm phase 2 parent PR.
Add an advertised, roster-scoped profile_query keyword: a bounded,
case-insensitive substring match over member ids and the same description
the listing displays (matched before output bounding). The filter narrows
the spawnable pool before the cap; host_profile_count and
host_profiles_truncated describe the filtered set, so a zero-match answer
stays honest. Blank or non-string values filter nothing. The advertised
surface grows 12 -> 13 fields; budgets, routing and workspace knobs stay
off (pins updated).
Tests: the 13-field schema pin, the roster dependentSchemas branch
advertising profile_query, and a discovery regression that builds 50
spawnable profiles, proves exp-host-049 is cut from the default listing,
found by description and id keywords, honest on zero matches, ignored on
blank input, and resolvable at spawn via profile=.
* test(subagent): close review-pinned coverage gaps (T7)
Fresh-review follow-ups on the roster/profile_query wave: the forkguard
shadow mirror now resolves against a roster that actually contains the
renamed role-token member, so FleetRole::from_str-over-roster precedence
is load-bearing (a roster without the id takes the role path under any
ordering); the discovery regression pins partial-id substring matching
so an exact-equality regression on the id leg fails; the ROSTER_PROFILE_
QUERY_MAX_CHARS doc now states the truncate-not-refuse convention it
actually implements; docs/FLEET.md cross-reference follows the advertised
surface to 13 fields.
* fix(fleet): dedupe host ids that get() conflates
[fleet.profiles] is a byte-order BTreeMap, so case-variant and
whitespace-variant keys (Expert / expert / ' expert') are distinct legal
entries while FleetRoster::get resolves all of them to the first member
in order. The host roster built by from_host_config listed every key
verbatim, so the roster action could advertise an id whose selection
silently spawned a different profile's prompt — every other profile
layer (directory loaders, ambient merge) already fails closed or shadows
on exactly this collision.
Collapse each duplicate to the member get actually resolves to, and pin
the listing/spawn invariant with a case/whitespace collision regression:
every advertised member_id must resolve back to itself.
* fix(fleet): collapse conflatable config ids at load too
The roster listing and the spawn path build the [fleet.profiles]
layer through two independent constructions. The listing reads the
engine-installed config.fleet_roster, built by FleetRoster::load,
whose merge_member folds each config key into the merged roster
last-wins (byte-order-last key replaces its conflatable siblings in
place). The spawn path re-resolves profile=<id> against a roster
rebuilt per spawn by refresh_spawn_route_sources via
FleetRoster::from_host_config, which 0576deb2c teaches to collapse
ids get() conflates (trimmed, ASCII case-insensitive) FIRST-wins.
For case-variant config keys such as 'Expert' and 'expert' — both
legal, distinct TOML keys — load advertised the last key's member
while get() on the spawn roster resolved every spelling to the
first one: the roster could list a profile whose prompt silently
differs from the one a selection actually spawns.
The invariant was previously established only on the from_host_config
side; both regressions (agent_roster_never_lists_ids_that_spawn_a_
different_profile, from_host_config_collapses_ids_that_get_resolves_
together) constructed their rosters through from_host_config
directly, so the listing/spawn pairing gap could not surface.
Extract the first-wins collapse (and the config-entry-to-member
mapping) into shared helpers and apply the same collapse to the
config-extracted profile list in load before merge_member, so the
last-wins layer merge never sees two conflatable config keys.
Cross-source precedence is preserved exactly: config entries still
override built-ins and are still overridden by plugin, personal, and
workspace layers; only intra-config duplicates are removed, in the
same direction from_host_config already pins.
Regressions:
- agent_roster_listing_and_spawn_agree_on_conflatable_config_ids
builds the listing side the way the engine does (FleetRoster::load)
and drives the spawn side through the real refresh_spawn_route_sources,
asserting the listed row is the member spawn resolves, with the same
id, description, and prompt overlay. Verified to fail on the
pre-fix code.
- load_collapses_conflatable_config_ids_like_from_host_config pins
the load-side direction at the roster unit level.
- roster_host_profile_listing_cap_is_pinned_at_48 literal-pins
ROSTER_HOST_PROFILE_LIMIT to the 48-row cap the app-side roster
contract teaches.
---------
fix: stop finite timeouts from killing legitimate work (#63)
* fix: stop finite timeouts from killing legitimate work
A timeout-behaviour audit found several finite wall-clock caps sitting on
paths whose work legitimately runs minutes to hours, auto-expiring human
decisions, and local waits with no bound at all. Fix the high-impact set:
- External tool approvals no longer auto-deny after 300s: the decision
wait is unbounded (human-paced) and ends on turn interrupt or runtime
shutdown instead, emitting approval.decided{interrupted} so clients
clear pending UI. The engine's own approval wait was already unbounded;
request_user_input now waits unboundedly too instead of answering the
model with a ToolError::Timeout after 300s.
- Background-task idle detection now treats a running tool as progress:
the journal only records tool calls at start/completion, so the 2-minute
idle deadline fired during any silent build, test suite, or MCP call.
The wall-time budget remains the backstop.
- MCP tool calls: the TUI pool default execute timeout 60s -> 1800s and
the response-read wait derives from max(read, execute) so raising the
documented knob actually governs; the engine-side stdio proxy gets a
dedicated 1800s tools/call budget instead of the 120s generic request
cap; the headless runtime dispatch backstop 300s -> 1800s; MCP OAuth
browser-callback wait 300s -> 900s.
- js_execution and code_execution: 120s -> 600s and the child is killed
on timeout instead of being left running orphaned.
- image_analyze vision calls: replace the 120s total deadline with
connect + per-read idle timeouts; a multi-MB upload plus a long
non-streaming generation legitimately exceeds two minutes.
- RLM child completions now honor the advertised sub_query_timeout_secs
(it was stored but never read; the bridge always used its 120s const),
and inline repl rounds 180s -> 900s to match the model-thinking
reality the RLM path already accommodates.
- web fetch hard cap 60s -> 300s and the SSRF pre-flight DNS lookup is
bounded at 10s so a hung resolver cannot stall the fetch envelope.
- Skill/plugin installs get connect + total timeouts (previously none:
a stalled connection hung installs forever); snapshot git subprocesses
are bounded at 300s with degrade-on-timeout instead of blocking the
turn pipeline on a wedged git.
- Non-streaming model requests get a 30-minute total envelope around the
retry loop (all attempts + honored Retry-After included). The shared
client intentionally has no client-level total timeout, so compaction,
translate, and similar non-streaming callers previously had no bound
against a provider that accepts and stalls, nor against a gateway
answering 429 + Retry-After: 3600 forever.
- Sub-agent tool timeout default 300s -> 1800s (single source of truth;
the heartbeat floor derives from it automatically).
Verified: cargo check --tests on tui/mcp/core; targeted suites for
runtime_threads (157), task_manager (46), web (192), mcp (76), core (82),
vision, js_execution, repl, snapshot, skills install, client all pass.
* fix(tasks): keep the idle watchdog honest about in-flight tools
The timeout-audit fix for background-task idle detection tracked running
tools with mismatched keys and only refreshed progress on event arrival:
- item.started carries the engine tool-use id under "tool", but
item.completed/item.failed only carry the journal item id under
"item", so the tracking set was never drained: after the first tool
call the watchdog was disarmed for the rest of the turn (only the
wall-time budget remained).
- note_progress was evaluated per event, but the very scenario being
fixed is a tool whose execution window produces no journal events at
all, so the 2-minute idle deadline still fired during silent builds,
test suites, and MCP calls.
Track both lifecycle edges by the journal item id, and refresh progress
from the steady-state loop (which wakes at least every catch-up poll)
for as long as any tool item is in flight. The wall-time budget stays
the backstop for a tool that never completes.
Pin both edges with tests: a silent tool survives well past the idle
deadline and the deadline resumes after completion, and a hung tool
still hits the wall budget.
* fix(vision): bound image_analyze with a real total envelope
The audit change replaced the 120s total deadline with connect(10s) +
read_timeout(120s), but reqwest's read_timeout is not a per-read idle
bound for the request phase: its timer starts at send() and is never
reset until the response headers arrive, so it silently re-introduced a
~120s total deadline on the multi-MB upload plus the full non-streaming
generation - the exact healthy work the change meant to protect (the
per-read idle semantics only apply once the response body is streaming).
The accompanying comment described the opposite of the actual behavior.
Bound only the connect handshake on the client and put a 30-minute
envelope (matching the non-streaming model request budget) around the
retry loop and response body consumption, reporting a clear timeout
when it fires. Wiremock tests pin both directions: a stalled provider
is cut off by the envelope with a timeout error, and a prompt answer
arriving in time still flows through.
* fix(snapshot): drain git pipes while the child runs
run_git waited for the child to exit before reading stdout/stderr, so
any invocation producing more than the 64 KiB OS pipe buffer deadlocked
on its own output and died at the 300s command timeout. That is not a
corner case: ls-tree -r -z --name-only on the restore path lists one
path per workspace file, and diff --name-only after a large refactor
grows the same way, so medium-size workspaces would have seen /undo and
snapshot listing stall 300s and then fail - the exact turn-pipeline
blocking the audit meant to remove (the old .output() call drained
concurrently and had no such cliff; the removed 'quiet-output' comment
was wrong about the tree-listing and diff commands).
Spawn two reader threads (mirroring the sandbox exec plumbing in
crate::run_sandboxed_exec) so the pipes are drained while the child
runs, join them on both the success and the kill path, and use a
tighter test-only command timeout so the large-output regression test
fails in seconds rather than hanging.
* fix(mcp): widen the read wait per request instead of clamping the knob
The audit fix set the connection-level read timeout to
max(read, execute) so a raised execute_timeout actually governs a
tools/call. That traded one bug for two more: the read_timeout knob
documented in docs/MCP.md was silently disabled (a user-configured read
budget of 30s or 180s was raised to the 1800s execute default, so fast
requests on a wedged server waited out 30 minutes before failing), and
every non tools/call request on the connection - resources/read,
prompts/get, notifications - inherited the same 30-minute liveness
regression for detecting a dead server.
Restore the connection read timeout to the user's knob and widen the
read wait per request instead: call_method now reads for at least its
own outer budget (so a silent server is not declared dead mid tool
call), while everything else keeps the configured read budget. The HTTP
transport's client-level total stays a separate ceiling computed as
max(read, execute) because it must cover the longest request the
transport carries; both decisions are extracted into named pure
functions and pinned with tests.
* fix(runtime): resolve pending approvals when the engine dies or the runtime shuts down
The unbounded external-approval wait only ended on turn interrupt or
runtime-shutdown cancellation, but two of its documented exits were
not actually reachable. A crashed e…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
修复 2026-09-22/23 批次合入后在父仓 windows-rust-test 暴露的 Windows 编译错误（阻塞 Pinvou/pinvou-agent#595 r3 收口）。
Root cause
#63（d349f2537）新增的
tools/process.rsbounded-child 回归测试是#[cfg(unix)]（依赖真实sh与持有管道的后台 grandchild），但模块级use super::*与shell_commandhelper 未一起门控。Windows 的 cfg(test) 编译把测试裁掉后，残余项在-D warnings下报 unused import + dead code（crates/tui/src/tools/process.rs:202/204），CodeWhale Windows PowerShell regressions步骤 exit 101。fork 自身 CI 在 Ubuntu 上 unix 测试正常编译，故未在 fork 侧暴露。Fix
给两项加
#[cfg(unix)]，Windows 下模块为空而非编译失败；unix 目标行为不变，diff 共 2 行。Verification
could not compile codewhale-tui (lib test) due to 2 previous errors）。No-Issue: fixes the windows-rust-test regression blocking Pinvou/pinvou-agent#595; no issue in this repository.