feat: raise long-turn defaults and give the liveness oracle a macOS backend - #8949
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Root cause fixed in the right order — evidence widened before timeouts, darwin probes live in Watch
[DESIGN-REVIEWED] 5169f79 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All counts check out. Writing the review. First-Principles-Verdict: PASS Every raised number traces to a shipped budget you can grep, and the window raise ships with the oracle fix that keeps it from sheltering real hangs. What this change shipsIntent: let a long unattended turn (a 90-min test run, a 2h subagent wave) finish on any platform without being falsely cancelled — a FIX, whose safe half is a new macOS liveness backend.
Watch
[FIRST-PRINCIPLES-REVIEWED] 5169f79 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review details
FINDING — src/kiro_crew/platform_compat.py:1173 — [OPUS-REVIEWED] 5169f79 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
88cbcbe to
e8bf797
Compare
|
First Principles CONCERNS on Watch (stale_window_secs help text is darwin-shaped): accurate as written, fixed in wording rather than value. The help text now states the platform-neutral trade: an attested think (established backend socket) already takes the model-silent window, so Subtraction ( |
e8bf797 to
19cb1d3
Compare
…ackend Long unattended turns were bounded by defaults sized for a 2h session and, on macOS, by a watchdog that could never see a tool working. Two changes, shipped together because each alone leaves such a turn wrong: Defaults. agent.chat_turn_timeout_secs 7200 -> 14400 (with the transport's _DEFAULT_PROMPT_TIMEOUT and constants.CHAT_TURN_TIMEOUT moving in step, so a config-less context still behaves like a default config); watchdog stale_window_secs 300 -> 600, model_silent_probe_secs 900 -> 1800, tool_stall_suspect_secs 3600 -> 5400, tool_stall_hard_cap_secs 3600 -> 7200. Four hours is the longest single turn the shipped budgets can legitimately produce (a 90-minute test command plus a fix and a re-run; a blocking subagent wave at its 2h wait cap plus synthesis). The UNKNOWN windows clear every budget a single tool call can spend silent while staying inside the ceiling so recovery stays reachable. check_after_secs is unchanged: DEAD and STUCK_INPUT verdicts act at that mark regardless of the windows, so a genuinely stuck backend is still reported in about a minute. macOS oracle. liveness.py read only Linux /proc, so on macOS every shell and MCP tool call was UNKNOWN for its whole life and tool_stall_suspect_secs acted as a de-facto bash timeout. An in-process libproc backend (proc_listchildpids, PROC_PIDTBSDINFO, proc_pidpath, KERN_PROCARGS2, PROC_PIDTASKINFO) now lets the oracle match, track and detect the exit of the running command and sum subtree CPU for MCP tools by the same rules as Linux. Evidence only /proc carries (established sockets, blocked-read stuck-input, wchan) is never invented; those cases keep the plain UNKNOWN. The backend is selected once per oracle when the platform is darwin and procfs is absent, and is injectable so the fake-backend tests need no libproc. Tests that pinned the old literals now derive from the dataclass defaults; the approval-window clamp tests lower the default ceiling under the static approval max so the cross-field clamp stays observable.
19cb1d3 to
5169f79
Compare
|
Tech Lead review note (posted as a comment, not a review, since GitHub blocks self-review): confirmed a real resource-protection regression, not approving as-is.
CI does not catch this: all 20 new darwin unit tests inject a Fix: |
Problem / Motivation
Two things stop a long unattended turn from working on a default install:
agent.chat_turn_timeout_secsis 7200,and the watchdog's UNKNOWN-verdict windows (
tool_stall_suspect_secs3600,model_silent_probe_secs900,stale_window_secs300) cancel or probe workthat is merely quiet for longer than that. A 90-minute test command plus a fix
and a re-run already does not fit the ceiling.
acp/liveness.py) reads only Linux/proc, soevery shell and MCP tool call is UNKNOWN for its whole life. There the
tool_stall_suspect_secswindow is not a backstop behind an oracle; it IS theonly detector, and acts as a de-facto bash timeout. A 40-minute
pytestthatis genuinely running and a wedged one look identical.
Why it matters
Raising the windows alone would fix (1) and make (2) worse: a genuinely hung tool
on a Mac would hold the session slot for the whole widened window with the user's
next message queued behind it. Fixing the oracle alone would leave the ceiling
too short for the work the product already ships budgets for. Shipped together,
a long build reads WORKING on every platform (never cancelled at any duration),
a truly stuck backend is still reported at
check_after_secs(60s, unchanged,because DEAD/STUCK_INPUT act there regardless of the windows), and the
windows only govern what the oracle genuinely cannot attest.
What changed (motivation → approach → change)
Defaults. Three independent analyses (workload inventory, code-constraint
audit, devil's advocate) converged on the same numbers; the longest single turn
the shipped budgets can legitimately produce is ~2h (the task runner's
TEST_TIMEOUT=5400plus fix and re-run; a blockingspawn_sub_agentswave atits 7200s wait cap plus synthesis), so 4h is the ceiling and 90 min the UNKNOWN
tool window. 24h was considered and rejected: a marathon turn survives none of a
gateway restart, laptop sleep or in-prompt compaction, which the monitor/goal
loops are built for, and the design comment on the constant says as much.
agent.chat_turn_timeout_secswatchdog.check_after_secswatchdog.stale_window_secswatchdog.model_silent_probe_secswatchdog.tool_stall_suspect_secswatchdog.tool_stall_hard_cap_secsconstants.CHAT_TURN_TIMEOUTand the transport's_DEFAULT_PROMPT_TIMEOUTmoveto 14400 in step, so a config-less context behaves exactly like a default config
(an existing test pins that equality). Consequence worth naming: the transport
wait for callers that pass no ceiling of their own (subagent, review and cron
turns) also becomes 4h. The loader's fallback literals,
WatchdogSettings,config-baseline.json, the design comment onCHAT_TURN_TIMEOUT_MIN/MAX, thefield help text and the specs all move together.
macOS oracle.
liveness.pygains an injectable darwin backend(
DarwinProcessBackend/LibprocBackend,select_darwin_backend), chosen onceper oracle when the platform is darwin and
proc_rootis absent. It isin-process libproc via ctypes, no
ps:proc_listchildpidsfor descendants,PROC_PIDTBSDINFOfor ppid/zombie/start time,proc_pidpathandsysctl KERN_PROCARGS2for the executable and argv,PROC_PIDTASKINFOfor CPU.The shell-child matching logic is factored into
_scan_for_child, shared by the/procwalk and the darwin path, so a shell command is WORKING / DEAD /shell_child_absenton macOS by exactly the Linux rules;_tree_movementsumssubtree CPU (evidence labelled
darwin cpu-only, since IO bytes are not readablethere) so an active MCP subtree reads WORKING. Evidence only
/proccarries(the
established_flatsocket tag,blocked_read_fdstuck-input,wchan) isnever invented; those cases keep the plain UNKNOWN. Dispatch stamps on darwin
are wall-clock, the clock libproc dates processes on. Zero new conditionals on
the Linux path beyond the one backend-selection probe.
Darwin start-time attribution is guarded against wall-clock steps
The darwin dispatch stamp is the wall clock (what libproc dates processes on),
and a wall clock can step: a backward NTP or VM-resume correction landing
between the stamp and the runtime's fork dates a live child before its own
dispatch. For a command the matchers cannot recognise (fully redacted input)
nothing would veto the absence claim, and the caller would narrow to the stale
window and cancel live work. So the stamp is paired with a steady one
(
steady_now(), darwinCLOCK_MONOTONIC, which counts sleep); when wallelapsed and steady elapsed disagree by more than the attribution tolerance the
oracle declines to attribute by start time at all — a matched child stays
WORKING and no
shell_child_absentclaim is made. A missing steady stamp getsthe same fail-open answer. Three tests pin it (backward step + redacted command
is not tagged absent; backward step + matched command stays WORKING; agreeing
clocks still date an old descendant as old).
Known trade: Windows
Windows gets the widened windows and no new detector. The oracle there reads
only the runtime's own CPU time (
proc_cpu_nanos_for_pidviaGetProcessTimes,root pid only), so every shell and MCP tool call stays UNKNOWN and the 90-minute
suspect window is the effective tool timeout on that platform — a genuinely hung
tool holds its slot for 90 min instead of 60. Accepted rather than sized around,
because sizing the global default to the platform with the weakest oracle would
re-impose the macOS false-cancellation problem on the two platforms that can now
attest. The follow-up is a Toolhelp-based descendant walk (the Windows twin of
this PR's libproc backend). A second follow-up: macOS long silent thinks land on
the 600s stale probe rather than the 1800s
established_flatextension, becausesocket evidence is not read on darwin yet;
PROC_PIDLISTFDSfdinfo could restoreit. When that lands, re-derive
stale_window_secs: the 600s default exists forthinks the oracle cannot attest, and once darwin can attest them the window's
remaining job is wedge-recovery latency, which argues for narrowing it back.
A third follow-up:
process_matchesandprocess_argv_matches_exactstill forkps -o command=on darwin;darwin_process_argvis the in-process replacement.Tests
test/test_acp_liveness_darwin.py(new, 20 tests, fake backend): matched livechild → WORKING and tracked; tracked child gone → UNKNOWN in grace, DEAD after
CHILD_EXIT_GRACE_SECS; observable tree with nothing started since dispatch →shell_child_absent; backend cannot enumerate → plain UNKNOWN with no absenceclaim; MCP subtree CPU delta → WORKING with the cpu-only label;
established_flatis not tagged on darwin; backend selection,
fresh()carry-over, clock choice.One real-libproc smoke test,
skipif != darwin, inspects onlyos.getpid()'sown subtree. Negative control: disabling the
_check_shell_childdispatch fails7 of them, disabling the
_tree_movementdispatch fails 4; both restored.test/test_acp_liveness.pypasses unchanged (75 across theliveness/watchdog files).
(
AgentConfig(),WatchdogConfig()), so the next default change landswithout a literal to chase.
test_default_preserves_existing_behaviournowloads a config that omits the key from an isolated home, proving the loader's
fallback literal tracks the dataclass. The approval-window clamp tests lower
_DEFAULT_CHAT_TURN_TIMEOUT_SECSunder the staticTOOL_APPROVAL_TIMEOUT_MAXso the cross-field clamp stays observable now that the default ceiling sits
above it.
window, platform_compat, stale/stuck recovery: 75 files): 3773 passed. The 3
failures are
test_acp_runtime.py::test_runtime_spawn_passes_installed_path_through_exact_wrappersand two in
test_acp_spawn_offload.py, which fail identically on a cleandetached
origin/mainworktree on this macOS host.Manual verification
N/A — unit coverage sufficient: the defaults are pure data with loader/baseline
parity tests, and the oracle branch is fully exercised through the injected fake
backend plus a real-libproc smoke test on darwin.
Related Issues
Follow-up to the watchdog work in #8559 and the macOS lost-turn fix in #8520.
Pattern harvest
Rule candidate: review-prompt
Pattern: a timeout is being widened to stop false positives from a detector that
cannot see the platform it runs on — widen the detector's evidence first, then
size the timeout to what the detector genuinely cannot attest.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)acp-client.md,learn-cron-dashboard.md,config.md,overview.md,resource-protection.md