Skip to content

feat: raise long-turn defaults and give the liveness oracle a macOS backend - #8949

Merged
iamwhatever merged 1 commit into
mainfrom
feat/long-turn-defaults-and-macos-liveness
Sep 6, 2026
Merged

feat: raise long-turn defaults and give the liveness oracle a macOS backend#8949
iamwhatever merged 1 commit into
mainfrom
feat/long-turn-defaults-and-macos-liveness

Conversation

@bolichen97

@bolichen97 bolichen97 commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Problem / Motivation

Two things stop a long unattended turn from working on a default install:

  1. The defaults are sized for a 2h session. agent.chat_turn_timeout_secs is 7200,
    and the watchdog's UNKNOWN-verdict windows (tool_stall_suspect_secs 3600,
    model_silent_probe_secs 900, stale_window_secs 300) cancel or probe work
    that 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.
  2. On macOS the liveness oracle (acp/liveness.py) reads only Linux /proc, so
    every shell and MCP tool call is UNKNOWN for its whole life. There the
    tool_stall_suspect_secs window is not a backstop behind an oracle; it IS the
    only detector, and acts as a de-facto bash timeout. A 40-minute pytest that
    is 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=5400 plus fix and re-run; a blocking spawn_sub_agents wave at
its 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.

key before after
agent.chat_turn_timeout_secs 7200 14400
watchdog.check_after_secs 60 60 (unchanged)
watchdog.stale_window_secs 300 600
watchdog.model_silent_probe_secs 900 1800
watchdog.tool_stall_suspect_secs 3600 5400
watchdog.tool_stall_hard_cap_secs 3600 7200

constants.CHAT_TURN_TIMEOUT and the transport's _DEFAULT_PROMPT_TIMEOUT move
to 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 on CHAT_TURN_TIMEOUT_MIN/MAX, the
field help text and the specs all move together.

macOS oracle. liveness.py gains an injectable darwin backend
(DarwinProcessBackend / LibprocBackend, select_darwin_backend), chosen once
per oracle when the platform is darwin and proc_root is absent. It is
in-process libproc via ctypes, no ps: proc_listchildpids for descendants,
PROC_PIDTBSDINFO for ppid/zombie/start time, proc_pidpath and
sysctl KERN_PROCARGS2 for the executable and argv, PROC_PIDTASKINFO for CPU.
The shell-child matching logic is factored into _scan_for_child, shared by the
/proc walk and the darwin path, so a shell command is WORKING / DEAD /
shell_child_absent on macOS by exactly the Linux rules; _tree_movement sums
subtree CPU (evidence labelled darwin cpu-only, since IO bytes are not readable
there) so an active MCP subtree reads WORKING. Evidence only /proc carries
(the established_flat socket tag, blocked_read_fd stuck-input, wchan) is
never 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(), darwin CLOCK_MONOTONIC, which counts sleep); when wall
elapsed 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_absent claim is made. A missing steady stamp gets
the 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_pid via GetProcessTimes,
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_flat extension, because
socket evidence is not read on darwin yet; PROC_PIDLISTFDS fdinfo could restore
it. When that lands, re-derive stale_window_secs: the 600s default exists for
thinks 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_matches and process_argv_matches_exact still fork
ps -o command= on darwin; darwin_process_argv is the in-process replacement.

Tests

  • test/test_acp_liveness_darwin.py (new, 20 tests, fake backend): matched live
    child → 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 absence
    claim; MCP subtree CPU delta → WORKING with the cpu-only label; established_flat
    is not tagged on darwin; backend selection, fresh() carry-over, clock choice.
    One real-libproc smoke test, skipif != darwin, inspects only os.getpid()'s
    own subtree. Negative control: disabling the _check_shell_child dispatch fails
    7 of them, disabling the _tree_movement dispatch fails 4; both restored.
  • Every test in test/test_acp_liveness.py passes unchanged (75 across the
    liveness/watchdog files).
  • Tests that pinned the old literals now derive from the dataclass defaults
    (AgentConfig(), WatchdogConfig()), so the next default change lands
    without a literal to chase. test_default_preserves_existing_behaviour now
    loads 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_SECS under the static TOOL_APPROVAL_TIMEOUT_MAX
    so the cross-field clamp stays observable now that the default ceiling sits
    above it.
  • Related suites (acp, watchdog, liveness, config, turn_dispatch, approval
    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_wrappers
    and two in test_acp_spawn_offload.py, which fail identically on a clean
    detached origin/main worktree 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

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — acp-client.md, learn-cron-dashboard.md, config.md, overview.md, resource-protection.md
  • No secrets, credentials, or internal references in the diff

@bolichen97
bolichen97 requested a review from a team as a code owner September 6, 2026 08:23
@bolichen97
bolichen97 requested a review from pepmach September 6, 2026 08:23
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 5169f793773a3e731f37a2c15c4002db8aaa7913 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Root cause fixed in the right order — evidence widened before timeouts, darwin probes live in platform_compat, all defaults reversible config data with fail-open fallbacks.

Watch

  • Windows now inherits the widened windows with no new detector, so a genuinely hung tool holds its session slot 90 min instead of 60 until the promised Toolhelp follow-up lands ("Windows gets the widened windows and no new detector"). Documented and bounded, but the follow-up should be tracked, not just named in prose.

[DESIGN-REVIEWED] 5169f79

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 5169f793773a3e731f37a2c15c4002db8aaa7913 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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 ships

Intent: 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.

  1. A chat turn may now run 4h (was 2h) before the ceiling card, in config, config-less contexts and the transport alike — justified (TEST_TIMEOUT = 5400 in task_models.py:15 and the 7200s spawn wait cap in mcp_tools/spawn.py:1038 both verified).
  2. A quiet unattested tool is cancelled at 90 min instead of 1h; hard cap 2h — justified, same derivation.
  3. Silent model waits probed at 10/30 min instead of 5/15 — justified; cost named in the help text; author schedules re-narrowing.
  4. macOS shell/MCP calls now get real WORKING/DEAD/absent verdicts instead of lifelong UNKNOWN — justified, cause-level (removes the evidence gap instead of widening timeouts around it).
  5. A wall-clock step on macOS can no longer date a live child before its own dispatch (paired steady stamp, fail-open) — justified, pinned by tests.
  6. On Windows a genuinely hung tool now holds its slot 90 min instead of 60 — declared trade, cause deferred.
  7. Four new in-process darwin probes in platform_compat — justified (documented platform-compat routing invariant; 1 real consumer each).
  8. Doc/spec/help-text updates — mandated same-commit by AGENTS.md.
  9. Tests derive from dataclass defaults instead of literals — rides along in direct support of the raise.

Watch

  • Sibling count on the "fork ps to read darwin process info" root cause: 6 trusted_system_bin("ps") sites remain in platform_compat.py (1830, 2290, 2387, 2657, 2713, 2898); at least 4 are replaceable by the new probes (_posix_process_parent_mapdarwin_child_pids; process_matches, process_argv_matches_exact, process_command_linedarwin_process_argv), but the description declares only 2 as follow-up. Accepted-and-deferred; track all 4, or the two spellings of "read a darwin cmdline" will diverge.

[FIRST-PRINCIPLES-REVIEWED] 5169f79

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5169f793773a3e731f37a2c15c4002db8aaa7913 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 5169f79

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5169f793773a3e731f37a2c15c4002db8aaa7913: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5169f793773a3e731f37a2c15c4002db8aaa7913 — this comment is updated in place on each push.

Review details

darwin_child_pids mis-reads the libproc byte-count as a pid count, corrupting the macOS descendant tree — advisory, does not block.

FINDING — src/kiro_crew/platform_compat.py:1173 — count = lib.proc_listchildpids(ppid, buf, capacity * 4) treats the return as a pid count, but proc_listchildpids returns BYTES written (n * sizeof(pid_t), same contract as proc_listpids, which proc_listallpids divides by sizeof(int)); for N children struct.unpack_from(f"<{count}i", ...) unpacks 4N int32s → [pid_1…pid_N, 0×3N], so a spurious 0 enters LibprocBackend.descendants, which recurses darwin_child_pids(0) and balloons the runtime's "subtree" into the whole system process tree → _darwin_tree_movement sums unrelated CPU and reads WORKING, so the macOS tool-stall / model-wait watchdog effectively never fires (a hung tool holds its slot to the 4h turn ceiling); test_libproc_backend_reads_own_subtree misses it because it only asserts child.pid in descendants, which padding survives → Fix: read the return as bytes — n = count // 4, grow while count >= capacity * 4, unpack n ints, and drop non-positive pids.

[OPUS-REVIEWED] 5169f79

Verdict parsed from the review's SHA-scoped output markers for commit 5169f793773a3e731f37a2c15c4002db8aaa7913.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5169f793773a3e731f37a2c15c4002db8aaa7913: <one-sentence reason>

@bolichen97
bolichen97 force-pushed the feat/long-turn-defaults-and-macos-liveness branch 2 times, most recently from 88cbcbe to e8bf797 Compare September 6, 2026 08:46
@bolichen97

Copy link
Copy Markdown
Collaborator Author

First Principles CONCERNS on 88cbcbe22 — disposition

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 stale_window_secs governs only thinks with no evidence — a host without procfs, or a backend connection momentarily down — and its cost is recovery latency on a runtime that is already wedged, never lost work. That distinguishes it from the Windows trade I declined to size around: there a live tool holds a slot for the widened window; here the probe is non-lethal and a live Linux think is not on this path at all. The value stays 600s; the PR body's follow-up list now records that landing darwin socket evidence (PROC_PIDLISTFDS) is the trigger to re-derive it, so the narrowing is prompted rather than forgotten.

Subtraction (process_matches / process_argv_matches_exact ps-fork on darwin): agreed, deferred as the reviewer allows. Those two are the PID-reuse guards used from spawn and orphan-recovery paths; swapping their evidence source belongs in its own change with its own tests, not folded into a defaults PR. Recorded in the PR body follow-ups.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@bolichen97
bolichen97 force-pushed the feat/long-turn-defaults-and-macos-liveness branch from e8bf797 to 19cb1d3 Compare September 6, 2026 09:27
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Sep 6, 2026
…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.
@bolichen97
bolichen97 force-pushed the feat/long-turn-defaults-and-macos-liveness branch from 19cb1d3 to 5169f79 Compare September 6, 2026 10:04
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@iamwhatever
iamwhatever merged commit f08d3f6 into main Sep 6, 2026
65 checks passed
@iamwhatever
iamwhatever deleted the feat/long-turn-defaults-and-macos-liveness branch September 6, 2026 15:51
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 6, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator Author

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.

darwin_child_pids at platform_compat.py:1173 reads proc_listchildpids's return as a pid COUNT, but the syscall returns BYTES WRITTEN (confirmed by the sibling proc_listallpids, which divides the identical return by sizeof(int)). For N real children this unpacks 4N int32s and appends 3N zero pids.

LibprocBackend.descendants then recurses with no filter on non-positive pids, no depth cap, no size cap — the "subtree" silently expands to the entire machine (pid 0's children are launchd and the kernel threads). _darwin_tree_movement sums always-advancing system-wide CPU, so the macOS liveness oracle returns WORKING unconditionally — inverting this PR's own purpose: a genuinely hung tool now holds its slot to the newly-widened 4h ceiling instead of being caught at 90 minutes.

CI does not catch this: all 20 new darwin unit tests inject a FakeBackend with its own descendants(), and the one real-libproc test is skipif != darwin and asserts only child.pid in descendants, which the zero-padding bug still satisfies.

Fix: n = count // 4 (not count), grow the buffer while count >= capacity * 4, drop unpacked pids <= 0, add a test pinning the exact bounded descendant set. Needs a second human's review either way since I'm the author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants