From 5169f793773a3e731f37a2c15c4002db8aaa7913 Mon Sep 17 00:00:00 2001 From: Bolin Chen Date: Sun, 6 Sep 2026 01:20:13 -0700 Subject: [PATCH] feat: raise long-turn defaults and give the liveness oracle a macOS backend 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. --- config-baseline.json | 30 +- docs/architecture/overview.md | 8 +- docs/architecture/resource-protection.md | 2 +- docs/system-specs/modules/acp-client.md | 4 +- docs/system-specs/modules/config.md | 2 +- .../modules/learn-cron-dashboard.md | 2 +- src/kiro_crew/acp/client.py | 8 +- src/kiro_crew/acp/liveness.py | 483 +++++++++++++--- src/kiro_crew/acp/session_handle.py | 18 +- src/kiro_crew/config/loader.py | 12 +- src/kiro_crew/config/sections.py | 76 ++- src/kiro_crew/constants.py | 8 +- src/kiro_crew/dashboard/state.py | 8 +- src/kiro_crew/dashboard/turn_dispatch.py | 2 +- src/kiro_crew/platform_compat.py | 213 ++++++- src/kiro_crew/slack/gateway.py | 2 +- test/test_acp_liveness_darwin.py | 528 ++++++++++++++++++ test/test_acp_stale_recovery.py | 25 +- test/test_acp_watchdog_windows.py | 3 +- test/test_chat_turn_timeout_consistency.py | 16 +- test/test_config_loader.py | 3 +- test/test_dashboard_approval_window.py | 39 +- test/test_loop_stall_surfacing.py | 20 +- 23 files changed, 1323 insertions(+), 189 deletions(-) create mode 100644 test/test_acp_liveness_darwin.py diff --git a/config-baseline.json b/config-baseline.json index fcea36d8d83..02a54024c01 100644 --- a/config-baseline.json +++ b/config-baseline.json @@ -53,7 +53,7 @@ "admission_gate": true, "workflow_run_timeout_secs": 3600, "subagent_mem_buffer_pct": 20, - "chat_turn_timeout_secs": 7200, + "chat_turn_timeout_secs": 14400, "session_start_timeout_secs": 90, "tool_approval_timeout_secs": 600, "session_control": true, @@ -730,10 +730,10 @@ "sensitive": false, "tags": [], "label": "Chat Turn Timeout (secs)", - "help": "Wall-clock ceiling for one chat turn. This is a runaway backstop, so it is clamped to 300s..86400s (24h) and can never be disabled. Raise it above the 2h default for long unattended turns (full test suites, long builds); the ACP transport's prompt wait follows it. Hitting the ceiling is visible: the turn ends with a card naming the limit. For work spanning days, prefer monitor/goal loops — they end the turn between cycles and survive restarts.", + "help": "Wall-clock ceiling for one chat turn. This is a runaway backstop, so it is clamped to 300s..86400s (24h) and can never be disabled. The 4h default covers the longest single turn the shipped budgets produce (a 90-minute test command plus a fix and a re-run, or a blocking subagent wave at its 2h wait cap); the ACP transport's prompt wait follows it. Hitting the ceiling is visible: the turn ends with a card naming the limit. For work spanning many hours or days, prefer monitor/goal loops — they end the turn between cycles and survive restarts, which a single marathon turn cannot.", "hasChildren": false, "enumValues": null, - "defaultValue": 7200 + "defaultValue": 14400 }, { "path": "agent.session_start_timeout_secs", @@ -3338,10 +3338,10 @@ "enumValues": null, "defaultValue": { "check_after_secs": 60.0, - "stale_window_secs": 300.0, - "tool_stall_suspect_secs": 3600.0, - "tool_stall_hard_cap_secs": 3600.0, - "model_silent_probe_secs": 900.0, + "stale_window_secs": 600.0, + "tool_stall_suspect_secs": 5400.0, + "tool_stall_hard_cap_secs": 7200.0, + "model_silent_probe_secs": 1800.0, "wellness_sample_secs": 3.0 } }, @@ -3368,10 +3368,10 @@ "sensitive": false, "tags": [], "label": "Stale probe window (s)", - "help": "Idle seconds before an UNKNOWN-verdict model-wait turn is safe-probed via session/cancel. Probes are non-lethal: a live turn auto-recovers.", + "help": "Idle seconds before an UNKNOWN-verdict model-wait turn is safe-probed via session/cancel. Probes are non-lethal, but a probe of a LIVE think cancels and regenerates it, so the window must clear an ordinary silent think. Default 10 min. A think the oracle can attest (an established backend socket) gets the longer model-silent window instead, so this one governs only thinks with no such evidence: a host without procfs, or a backend connection that is momentarily down. Its cost is wedge-recovery latency on a runtime that is already dead, never lost work.", "hasChildren": false, "enumValues": null, - "defaultValue": 300.0 + "defaultValue": 600.0 }, { "path": "watchdog.tool_stall_suspect_secs", @@ -3382,10 +3382,10 @@ "sensitive": false, "tags": [], "label": "Tool stall suspect (s)", - "help": "Idle seconds before an UNKNOWN-verdict in-flight tool is cancelled and the turn routed to tool-stall recovery (continue-nudge, no re-run of the original message). WORKING tools (e.g. a matched live build child) are never cancelled regardless of duration. Default 1h: generous enough for long builds and MCP tools on macOS, where the liveness oracle degrades (no /proc) and cannot distinguish a live build from a stall, while still landing inside the turn's own ceiling (agent.chat_turn_timeout_secs) so recovery is reachable. Enforcement is at handle construction, not config load: a window past the headroom fraction of the transport's per-prompt timeout is clamped with a warning, while one that merely exceeds agent.chat_turn_timeout_secs is warned about but left as set, because the same handle also serves callers that pass a larger prompt timeout (review and cron turns).", + "help": "Idle seconds before an UNKNOWN-verdict in-flight tool is cancelled and the turn routed to tool-stall recovery (continue-nudge, no re-run of the original message). WORKING tools (a matched live build child, an MCP subtree with CPU movement) are never cancelled regardless of duration, so this window governs only what the liveness oracle cannot attest. Default 90 min: it clears every shipped budget a single tool call can legitimately spend silent (the task runner's 90-minute test command, a full test suite with one retry in one shell call) while still landing inside the turn's own ceiling (agent.chat_turn_timeout_secs) so recovery is reachable. Enforcement is at handle construction, not config load: a window past the headroom fraction of the transport's per-prompt timeout is clamped with a warning, while one that merely exceeds agent.chat_turn_timeout_secs is warned about but left as set, because the same handle also serves callers that pass a larger prompt timeout (review and cron turns).", "hasChildren": false, "enumValues": null, - "defaultValue": 3600.0 + "defaultValue": 5400.0 }, { "path": "watchdog.tool_stall_hard_cap_secs", @@ -3396,10 +3396,10 @@ "sensitive": false, "tags": [], "label": "Hard cap (s)", - "help": "Absolute ceiling for UNKNOWN-verdict forbearance (e.g. the extended probably-thinking window). Applies ONLY to UNKNOWN verdicts — never to a WORKING session, which is deferred before this cap is consulted and is therefore bounded only by the turn's own ceiling. Default 1h, clamped against the transport's per-prompt timeout like the suspect window.", + "help": "Absolute ceiling for UNKNOWN-verdict forbearance (e.g. the extended probably-thinking window) and for any per-agent watchdog_tool_stall_* override. Applies ONLY to UNKNOWN verdicts — never to a WORKING session, which is deferred before this cap is consulted and is therefore bounded only by the turn's own ceiling. Default 2h, clamped against the transport's per-prompt timeout like the suspect window.", "hasChildren": false, "enumValues": null, - "defaultValue": 3600.0 + "defaultValue": 7200.0 }, { "path": "watchdog.model_silent_probe_secs", @@ -3410,10 +3410,10 @@ "sensitive": false, "tags": [], "label": "Silent-think probe window (s)", - "help": "Extended probe window for a model-wait with an established backend connection but flat counters (non-streamed server-side reasoning, e.g. long xhigh thinks). Probing a live think cancels and regenerates it, so this window is deliberately generous.", + "help": "Extended probe window for a model-wait with an established backend connection but flat counters (non-streamed server-side reasoning, e.g. long xhigh thinks). Probing a live think cancels and regenerates it, so this window is deliberately generous: 30 min clears the long end of an extended-effort think.", "hasChildren": false, "enumValues": null, - "defaultValue": 900.0 + "defaultValue": 1800.0 }, { "path": "watchdog.wellness_sample_secs", diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 807d33d1a97..d8f23812f61 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -281,11 +281,11 @@ graph TB older than `session.pool_ttl_secs` (default 1800s) are discarded at claim time. - **Idle timeout** reclaims a session after `session.timeout_secs`, default **3600s**. -- **Turn ceiling**: `agent.chat_turn_timeout_secs` defaults to **7200s** (2h), - clamped to 300s..86400s (`CHAT_TURN_TIMEOUT_MAX`, deliberately decoupled from the 7200s default) and never disable-able. It is a runaway backstop, so a +- **Turn ceiling**: `agent.chat_turn_timeout_secs` defaults to **14400s** (4h), + clamped to 300s..86400s (`CHAT_TURN_TIMEOUT_MAX`, deliberately decoupled from the 14400s default) and never disable-able. It is a runaway backstop, so a turn that hits it ends with a card naming the limit rather than failing - silently. The ACP transport carries its own prompt timeout of the same - magnitude and bounds the turn first. + silently. The ACP transport's prompt timeout follows the configured ceiling + (plus a margin) so the dashboard's card always fires first. - **Tool-approval window**: `agent.tool_approval_timeout_secs` defaults to **600s** (10 min). It must expire *inside* the turn that opened it — otherwise an unanswered prompt is reported as a turn timeout and the real cause is lost — diff --git a/docs/architecture/resource-protection.md b/docs/architecture/resource-protection.md index 5135388c120..7fceab14826 100644 --- a/docs/architecture/resource-protection.md +++ b/docs/architecture/resource-protection.md @@ -46,7 +46,7 @@ anything that survived a gateway crash. No single mechanism is a single point of | Context compaction | `session.py` | Chat sessions | `session.autocompact_pct` | No | Sends `/compact` to kiro-cli to free context window | | Background session recycle | `session.py` | Background sessions (cron, subagent) | 70% context usage (`_BG_RECYCLE_PCT`) | No | Recycles the session before context overflow | | Watchdog process liveness | `taskrunner.py` | Task runner steps | 2 consecutive dead checks (`_DEAD_THRESHOLD`) at 30s intervals | Yes, part of the watchdog loop | Resets the session to trigger crash recovery | -| Config bound clamp | `config/loader.py` | Subagent count, turns, timeouts and pool size at load time | `subagent_auto_max` and `max_subagents` to 64 (`SUBAGENT_AUTO_MAX_CEILING`), `subagent_max_turns` 1..200, `chat_turn_timeout_secs` 300..86400 (`CHAT_TURN_TIMEOUT_MAX`; 7200 is the default, not the ceiling), `session_start_timeout_secs`, `tool_approval_timeout_secs` 30..7200 and cross-field to 60s under the turn ceiling (`APPROVAL_TURN_MARGIN_SECS`), `loop_stall_exit_after_secs` 10..300, `pool_size` 0..10 (`_SECURITY_BOUNDED_FIELDS`) | No | `_clamp_security_bounds` clamps out-of-range ints, logs a WARNING, emits SEL `config_bounds_clamped` (`outcome=clamped`) | +| Config bound clamp | `config/loader.py` | Subagent count, turns, timeouts and pool size at load time | `subagent_auto_max` and `max_subagents` to 64 (`SUBAGENT_AUTO_MAX_CEILING`), `subagent_max_turns` 1..200, `chat_turn_timeout_secs` 300..86400 (`CHAT_TURN_TIMEOUT_MAX`; 14400 is the default, not the ceiling), `session_start_timeout_secs`, `tool_approval_timeout_secs` 30..7200 and cross-field to 60s under the turn ceiling (`APPROVAL_TURN_MARGIN_SECS`), `loop_stall_exit_after_secs` 10..300, `pool_size` 0..10 (`_SECURITY_BOUNDED_FIELDS`) | No | `_clamp_security_bounds` clamps out-of-range ints, logs a WARNING, emits SEL `config_bounds_clamped` (`outcome=clamped`) | ## Per-workflow coverage matrix diff --git a/docs/system-specs/modules/acp-client.md b/docs/system-specs/modules/acp-client.md index 6ea601ca63d..bf2dbcb3d51 100644 --- a/docs/system-specs/modules/acp-client.md +++ b/docs/system-specs/modules/acp-client.md @@ -417,7 +417,7 @@ The submitted future is tracked on the client, and polls while it is unfinished Both boundaries that drop a movement baseline — turn start in `_prompt_loop()` and `_reset_state()` — **retire** the liveness state through `_retire_liveness_state()`, which releases the tracked consult future AND swaps in a fresh oracle via `LivenessOracle.fresh()` (`fresh()` rather than a default construction, so an injected `/proc` root or sampling interval survives the swap). The two must retire together: replacing only the oracle would leave a walk wedged during the previous turn answering every later poll with `"prior consult still in flight"`, so the new turn would never sample its own process and the 90s cutoff would complete it early. Clearing in place is not sufficient either — a consult detached by a timeout keeps a bound reference to the instance it was submitted with, and samples are keyed without a PID, so a late write would repopulate the live baseline after that baseline was taken; since any nonzero delta counts as movement, that reads `WORKING` for a flat turn and defers its reap. Retiring confines a late writer to an instance nobody reads, which is what makes the `"sampling"` behaviour above hold. Retirement sits inside `_prompt_loop()` immediately after `_turn_lock` is acquired, which is load-bearing twice over: it is the single point every prompt path funnels through (`send_message` via `_read_prompt_response`, `send_message_stream`, and `_dispatch_events`), so no public prompt API is left carrying the previous turn's walk; and doing it under the lock stops a queued turn from clearing the *active* turn's tracked consult and thereby allowing a second walk while the first is still pending. -A retired walk that fails afterwards has its exception consumed via a done-callback attached at submission, so an ordinary probe failure is not reported as an unhandled-asyncio crash. Past the cutoff, **only `VERDICT_WORKING`** (moving CPU/IO in the backend subprocess subtree) defers the turn (loop continues); every other verdict (`DEAD`/`UNKNOWN`/`STUCK_INPUT`) preserves the prior end-the-turn behavior, so hang recovery is never weakened — a genuinely dead turn still ends, bounded by the resolved prompt timeout (`_DEFAULT_PROMPT_TIMEOUT`, 2h — raised alongside `agent.chat_turn_timeout_secs` via `resolve_prompt_timeout`) and the tool-stall watchdog below. Unlike the runtime path's `session/cancel` probe, the `AcpClient` reap is a plain `return` (process-per-session: the turn simply completes; no shared runtime to protect). +A retired walk that fails afterwards has its exception consumed via a done-callback attached at submission, so an ordinary probe failure is not reported as an unhandled-asyncio crash. Past the cutoff, **only `VERDICT_WORKING`** (moving CPU/IO in the backend subprocess subtree) defers the turn (loop continues); every other verdict (`DEAD`/`UNKNOWN`/`STUCK_INPUT`) preserves the prior end-the-turn behavior, so hang recovery is never weakened — a genuinely dead turn still ends, bounded by the resolved prompt timeout (`_DEFAULT_PROMPT_TIMEOUT`, 4h — raised alongside `agent.chat_turn_timeout_secs` via `resolve_prompt_timeout`) and the tool-stall watchdog below. Unlike the runtime path's `session/cancel` probe, the `AcpClient` reap is a plain `return` (process-per-session: the turn simply completes; no shared runtime to protect). The compatibility reap emits `EVENT_COMPLETE` with `stop_reason=end_turn` so existing consumers finalize normally, but also sets `synthetic_completion=true`. @@ -429,7 +429,7 @@ accounting consumers must reject the synthetic form. While a turn is dispatching, both ACP transports run a watchdog over a turn gone silent after a tool was dispatched — and both **recover** rather than just `return` on a dead turn (`AcpClient` keeps the blanket `_TOOL_STALL_TIMEOUT` window; the session handle is verdict-driven, below): - **`AcpClient`** (process-per-session, `_TOOL_STALL_TIMEOUT = 600s`): the stall clock is measured against `_tool_last_seen = max(last_data_ts, self._last_activity)`, so tools that keepalive-ping without emitting stdout frames (`wait`, `spawn_sub_agents`) don't trip a false stall (`_last_activity` is refreshed out of band by the stderr drain / keepalive). On a real stall it `_kill_process(force=True)` and raises `AcpProcessDied`, routing through the existing pipe-death recovery (dashboard resets the session + re-queues, bounded by `_acp_pipe_death_retries`; cron/other callers get a clean error instead of a wedged slot). `_kill_process` only touches the subprocess/pipes (never `_turn_lock`), and blast radius is one session — each `AcpClient` owns exactly one process. -- **`runtime.py` / `AcpSessionHandle`**: watchdogs are **verdict-driven, not timeout-driven** — the prior design used timeouts as death detectors and killed healthy-but-slow work (a silent 30-min redirected build `long-build > build.log 2>&1` at exactly the blanket window; healthy long non-streamed reasoning at 90s, where the destructive `session/cancel` probe was acked by the LIVE turn and surfaced as "Turn cancelled by user"). Once a turn is idle past `watchdog.check_after_secs` (60s), the per-session `LivenessOracle` (`acp/liveness.py`) returns a verdict with evidence: **WORKING** (a live cmdline-matched shell child, a `wait` tool inside its declared duration + slack, moving CPU/IO counters, backend socket bytes flowing) is never acted on at any elapsed time (logged at most once per 10 min — at INFO below the escalation mark, which is the lower of 30 min and a quarter of this turn's deadline, and at WARNING past it so a deferral able to hold the turn to its ceiling is visible at the default `agent.log_level`); **DEAD** (tracked shell child exited without a result frame past a 15s grace; model-wait with flat counters and NO established backend socket — the done-but-lost-frame wedge signature) acts immediately, so recovery lands seconds after actual death instead of at a blanket window; **STUCK_INPUT** (matched subtree flat across samples with a process blocked reading a tty/stdin pipe) acts immediately with a cause the recovery nudge names; **UNKNOWN** is the only timeout-governed class — stale probe at `watchdog.stale_window_secs` (300s; extended to `watchdog.model_silent_probe_secs` = 900s when the evidence is `established_flat`, i.e. probably a non-streamed server-side think), tool cancel at `watchdog.tool_stall_suspect_secs` (3600s / 1h — generous enough that a long build or MCP tool on macOS, where the liveness oracle degrades without `/proc`, is not falsely cancelled), hard-capped at `watchdog.tool_stall_hard_cap_secs` (3600s / 1h, UNKNOWN only). Three refinements keep the build-scale tool forbearance from sheltering an **LLM-shaped** stall (a model turn riding inside a tool, e.g. kiro-cli `use_subagent`, whose longest legitimate silent gap is minutes) or an **already-finished** one: (1) the oracle tags an UNKNOWN tool verdict with `established_flat` when the subtree's counters are genuinely flat (a real two-sample delta, not the baseline tick) AND the **runtime process itself** holds an established backend socket — deliberately narrower than the model-wait branch's whole-tree socket scan, so an MCP server blocked on *its own* remote call keeps the full tool windows — and the tool branch then uses `min(model_silent_probe_secs, tool_stall_suspect_secs)` as the effective suspect window; plain flat-subtree evidence keeps the full window, and under the OS sandbox (pid = launcher parent, no sockets on it) the tag never fires, failing toward the long build-safe window. (2) the never-matched SHELL fork is split instead of uniformly forgiven: `no matching shell child` conflated a command that already exited — a sub-second `ls | grep | wc` whose result frame was lost is never observed alive, so the DEAD branch's 15s exit grace can never fire for it — with one running unrecognized, and the two got the same 1h. The oracle now tags the first case `shell_child_absent` when the runtime's descendant tree is OBSERVABLE (a readable `/proc//task//children`, empty or not) and holds no live descendant attributable to this dispatch, and the tool branch then uses `min(stale_window_secs, tool_stall_suspect_secs)` — the ordinary silence budget — instead of the build-scale one. Attribution compares a descendant's `starttime` against a `CLOCK_BOOTTIME` stamp taken at the tool_call frame and widened by the turn's banked consumer parking (`_parked_total`), because `/proc` dates processes on a clock that counts suspended time while `time.monotonic()` does not, and the stamp is taken when the frame is PROCESSED rather than when the runtime spawned (a frame queued behind an approval is stamped that late). Four states each keep the full window, so every unattributable one fails toward build-scale patience: a descendant young enough to be this dispatch's, one whose cmdline matches while predating the stamp (indistinguishable from a coincidental lookalike), an unreadable child list, and a missing stamp or tick rate (no `os.sysconf` off Linux). The verdict stays UNKNOWN, never DEAD — absence is inferred, so it only shortens the non-lethal cancel. (3) An agent definition can override the windows per agent (`agents..watchdog_tool_stall_suspect_secs` / `watchdog_tool_stall_hard_cap_secs`, 0 = inherit the global — the same empty-inherits convention as the agent's `model`), applied in the `WatchdogSettings` snapshot at handle construction (`_load_watchdog_settings(crew_agent)` — a direct lookup on the CANONICAL crew name, resolved by the surface that owns the identity: the dashboard passes the slot member explicitly through `get_or_create(crew_agent=...)`, and crew-name-passing surfaces (Slack threads, cron, spawned agents) are covered by the provider factory's crew-namespace membership fallback; the identity is plumbed provider → runtime → handle, and a warm-pool claim rebinds the live handle via `rebind_watchdog()` so it travels with the SESSION, not the pool key — a name that is not a crew key simply inherits the global) so a pure-LLM agent like a PR reviewer can declare minutes-scale windows without touching the global build budget; an override is bounded by the same load-time ceiling clamp as the global windows, so it cannot smuggle a window past the prompt timeout. Every idle window is bounded at load by the resolved prompt timeout (`resolve_prompt_timeout` — the one deadline every caller shares; 7200s default, following a raised `agent.chat_turn_timeout_secs`) minus 10% headroom for the cancel + ack grace, and an over-ceiling on-disk value is clamped with a warning: a window at or past the deadline makes the UNKNOWN class unreachable, because the turn's timeout fires first and the user gets the generic turn-limit card instead of the tool-stall recovery below. A window above the DASHBOARD ceiling (`agent.chat_turn_timeout_secs`) is reported but **not** clamped — the same handle serves callers that pass their own larger prompt timeout, and shrinking their windows would cancel live work. **Every watchdog action is non-lethal:** a stale probe's cancel-ack is reclassified in the turn-complete branch (`_stale_probe` + `stopReason==cancelled` → `STOP_REASON_STALE_RECOVER`; the flag is single-shot — consumed on reclassification and superseded by a genuine `cancel()`, so a user cancel arriving after a probe is never misattributed to auto-recovery) so the dashboard auto-recovers instead of logging a user cancellation — an oracle mistake costs a regeneration, never a session. A tool stall ends the turn with `STOP_REASON_TOOL_STALL` (`"error: tool stall"`, in the `error:` family so branch-less callers degrade to generic handling) carrying the tool title / redacted command / evidence on the terminal `AcpEvent`; chat_runner's dedicated branch queues a **continue-nudge** (`build_tool_stall_recovery_prompt` — check partial results, tail any `> file` redirect target, re-run non-interactively on STUCK_INPUT) instead of the legacy verbatim re-queue of the original user message (which restarted the whole task and re-ran the very command that stalled), charged against a separate `slot._tool_stall_retries` budget (3) so a stall never burns the pipe-death reconnect budget. The runtime is **shared** (multiple sessions multiplexed on one process), so recovery is always `session/cancel` for **this `sessionId` only** (bounded by `asyncio.wait_for(..., 5s)`); siblings keep running. `watchdog.*` config is snapshotted at handle construction (`WatchdogSettings`); the dispatch loop never reads config. +- **`runtime.py` / `AcpSessionHandle`**: watchdogs are **verdict-driven, not timeout-driven** — the prior design used timeouts as death detectors and killed healthy-but-slow work (a silent 30-min redirected build `long-build > build.log 2>&1` at exactly the blanket window; healthy long non-streamed reasoning at 90s, where the destructive `session/cancel` probe was acked by the LIVE turn and surfaced as "Turn cancelled by user"). Once a turn is idle past `watchdog.check_after_secs` (60s), the per-session `LivenessOracle` (`acp/liveness.py`) returns a verdict with evidence: **WORKING** (a live cmdline-matched shell child, a `wait` tool inside its declared duration + slack, moving CPU/IO counters, backend socket bytes flowing) is never acted on at any elapsed time (logged at most once per 10 min — at INFO below the escalation mark, which is the lower of 30 min and a quarter of this turn's deadline, and at WARNING past it so a deferral able to hold the turn to its ceiling is visible at the default `agent.log_level`); **DEAD** (tracked shell child exited without a result frame past a 15s grace; model-wait with flat counters and NO established backend socket — the done-but-lost-frame wedge signature) acts immediately, so recovery lands seconds after actual death instead of at a blanket window; **STUCK_INPUT** (matched subtree flat across samples with a process blocked reading a tty/stdin pipe) acts immediately with a cause the recovery nudge names; **UNKNOWN** is the only timeout-governed class — stale probe at `watchdog.stale_window_secs` (600s; extended to `watchdog.model_silent_probe_secs` = 1800s when the evidence is `established_flat`, i.e. probably a non-streamed server-side think), tool cancel at `watchdog.tool_stall_suspect_secs` (5400s / 90 min — clears every shipped budget a single tool call can legitimately spend silent, such as the task runner's 90-minute test command), hard-capped at `watchdog.tool_stall_hard_cap_secs` (7200s / 2h, UNKNOWN only; also bounds the per-agent overrides). The oracle's evidence is Linux `/proc` where it exists; on macOS (no procfs) it selects an in-process **libproc backend** once per oracle instance (`select_darwin_backend`, injectable for tests): `proc_listchildpids` enumerates the runtime's descendants, `PROC_PIDTBSDINFO` supplies ppid / zombie state / start time, `proc_pidpath` and `sysctl KERN_PROCARGS2` supply the executable and argv for the same cmdline match the `/proc` walk performs, and `PROC_PIDTASKINFO` supplies per-process CPU time summed over the subtree (evidence labelled `darwin cpu-only`, since IO bytes are not readable there). So a shell command is WORKING/DEAD/`shell_child_absent` on macOS by the same rules as Linux, and an active MCP subtree reads WORKING instead of running out the suspect window; evidence only `/proc` carries — the `established_flat` socket tag, the `blocked_read_fd` STUCK_INPUT check, `wchan` — is never invented on macOS, so those cases keep the plain UNKNOWN. Dispatch stamps on darwin are wall-clock (`time.time()`), the clock libproc dates processes on, and a wall clock can step: a backward NTP or VM-resume correction between the dispatch stamp and the runtime's fork dates a live child before its own dispatch. The stamp is therefore paired with a steady one (`steady_now()`, darwin `CLOCK_MONOTONIC`, which counts sleep), and when wall elapsed and steady elapsed disagree by more than the attribution tolerance the oracle declines to attribute by start time at all — every row reads as possibly this tool's, so a matched child stays WORKING and no `shell_child_absent` claim is made. A missing steady stamp gets the same fail-open answer. **Windows has no tree backend yet**: 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 that long. The trade is accepted rather than sized around: a Toolhelp-based descendant walk is the follow-up that closes it, the same way the darwin backend did for macOS. Three refinements keep the build-scale tool forbearance from sheltering an **LLM-shaped** stall (a model turn riding inside a tool, e.g. kiro-cli `use_subagent`, whose longest legitimate silent gap is minutes) or an **already-finished** one: (1) the oracle tags an UNKNOWN tool verdict with `established_flat` when the subtree's counters are genuinely flat (a real two-sample delta, not the baseline tick) AND the **runtime process itself** holds an established backend socket — deliberately narrower than the model-wait branch's whole-tree socket scan, so an MCP server blocked on *its own* remote call keeps the full tool windows — and the tool branch then uses `min(model_silent_probe_secs, tool_stall_suspect_secs)` as the effective suspect window; plain flat-subtree evidence keeps the full window, and under the OS sandbox (pid = launcher parent, no sockets on it) the tag never fires, failing toward the long build-safe window. (2) the never-matched SHELL fork is split instead of uniformly forgiven: `no matching shell child` conflated a command that already exited — a sub-second `ls | grep | wc` whose result frame was lost is never observed alive, so the DEAD branch's 15s exit grace can never fire for it — with one running unrecognized, and the two got the same 1h. The oracle now tags the first case `shell_child_absent` when the runtime's descendant tree is OBSERVABLE (a readable `/proc//task//children`, empty or not) and holds no live descendant attributable to this dispatch, and the tool branch then uses `min(stale_window_secs, tool_stall_suspect_secs)` — the ordinary silence budget — instead of the build-scale one. Attribution compares a descendant's `starttime` against a `CLOCK_BOOTTIME` stamp taken at the tool_call frame and widened by the turn's banked consumer parking (`_parked_total`), because `/proc` dates processes on a clock that counts suspended time while `time.monotonic()` does not, and the stamp is taken when the frame is PROCESSED rather than when the runtime spawned (a frame queued behind an approval is stamped that late). Four states each keep the full window, so every unattributable one fails toward build-scale patience: a descendant young enough to be this dispatch's, one whose cmdline matches while predating the stamp (indistinguishable from a coincidental lookalike), an unreadable child list, and a missing stamp or tick rate (no `os.sysconf` off Linux). The verdict stays UNKNOWN, never DEAD — absence is inferred, so it only shortens the non-lethal cancel. (3) An agent definition can override the windows per agent (`agents..watchdog_tool_stall_suspect_secs` / `watchdog_tool_stall_hard_cap_secs`, 0 = inherit the global — the same empty-inherits convention as the agent's `model`), applied in the `WatchdogSettings` snapshot at handle construction (`_load_watchdog_settings(crew_agent)` — a direct lookup on the CANONICAL crew name, resolved by the surface that owns the identity: the dashboard passes the slot member explicitly through `get_or_create(crew_agent=...)`, and crew-name-passing surfaces (Slack threads, cron, spawned agents) are covered by the provider factory's crew-namespace membership fallback; the identity is plumbed provider → runtime → handle, and a warm-pool claim rebinds the live handle via `rebind_watchdog()` so it travels with the SESSION, not the pool key — a name that is not a crew key simply inherits the global) so a pure-LLM agent like a PR reviewer can declare minutes-scale windows without touching the global build budget; an override is bounded by the same load-time ceiling clamp as the global windows, so it cannot smuggle a window past the prompt timeout. Every idle window is bounded at load by the resolved prompt timeout (`resolve_prompt_timeout` — the one deadline every caller shares; 14400s default, following a raised `agent.chat_turn_timeout_secs`) minus 10% headroom for the cancel + ack grace, and an over-ceiling on-disk value is clamped with a warning: a window at or past the deadline makes the UNKNOWN class unreachable, because the turn's timeout fires first and the user gets the generic turn-limit card instead of the tool-stall recovery below. A window above the DASHBOARD ceiling (`agent.chat_turn_timeout_secs`) is reported but **not** clamped — the same handle serves callers that pass their own larger prompt timeout, and shrinking their windows would cancel live work. **Every watchdog action is non-lethal:** a stale probe's cancel-ack is reclassified in the turn-complete branch (`_stale_probe` + `stopReason==cancelled` → `STOP_REASON_STALE_RECOVER`; the flag is single-shot — consumed on reclassification and superseded by a genuine `cancel()`, so a user cancel arriving after a probe is never misattributed to auto-recovery) so the dashboard auto-recovers instead of logging a user cancellation — an oracle mistake costs a regeneration, never a session. A tool stall ends the turn with `STOP_REASON_TOOL_STALL` (`"error: tool stall"`, in the `error:` family so branch-less callers degrade to generic handling) carrying the tool title / redacted command / evidence on the terminal `AcpEvent`; chat_runner's dedicated branch queues a **continue-nudge** (`build_tool_stall_recovery_prompt` — check partial results, tail any `> file` redirect target, re-run non-interactively on STUCK_INPUT) instead of the legacy verbatim re-queue of the original user message (which restarted the whole task and re-ran the very command that stalled), charged against a separate `slot._tool_stall_retries` budget (3) so a stall never burns the pipe-death reconnect budget. The runtime is **shared** (multiple sessions multiplexed on one process), so recovery is always `session/cancel` for **this `sessionId` only** (bounded by `asyncio.wait_for(..., 5s)`); siblings keep running. `watchdog.*` config is snapshotted at handle construction (`WatchdogSettings`); the dispatch loop never reads config. **Both idle clocks measure BACKEND silence, so consumer time is subtracted from them.** `_dispatch_events` is an async generator: it is suspended at its `yield` for the whole of a consumer-side await (a tool approval, an IM send, a hook), and `last_data_ts` does not advance while suspended. Charging that interval to the runtime lets the arm cancel a turn moments *after* a human approves a tool — and at that instant the tool has not started, so the oracle draws `UNKNOWN` or `DEAD`, and `DEAD` acts immediately regardless of the window. `prompt()` therefore times each park around its single re-yield (`_parked_since` → `_parked_total`, cleared in a `finally` so an abandoned generator does not read as parked forever), and the timeout arm subtracts the park accumulated since `last_data_ts` was taken. The tool clock is exact; the stale clock can key off the newer stderr/keepalive activity, in which case part of the correction predates its reference point and is subtracted twice — which only makes that branch more patient, never quicker to probe. diff --git a/docs/system-specs/modules/config.md b/docs/system-specs/modules/config.md index f612187a945..65c1b932fb0 100644 --- a/docs/system-specs/modules/config.md +++ b/docs/system-specs/modules/config.md @@ -803,7 +803,7 @@ class AgentConfig: subagent_auto_max: int = 16 # ceiling on the auto-sized cap (max_subagents=0 only). Load-time clamped to [3, 64] subagent_max_turns: int = 100 # default per-subagent tool-call budget. Load-time clamped to [1, 200] subagent_result_ttl_secs: int = 3600 # seconds a delivered subagent's result.txt is retained before the reaper prunes it - chat_turn_timeout_secs: int = 7200 # wall-clock ceiling for one chat turn. Load-time clamped to [300, 86400]; the ACP prompt wait follows it (resolve_prompt_timeout) + chat_turn_timeout_secs: int = 14400 # wall-clock ceiling for one chat turn. Load-time clamped to [300, 86400]; the ACP prompt wait follows it (resolve_prompt_timeout) tool_approval_timeout_secs: int = 600 # how long a chat turn waits for a human to answer a tool-approval prompt. Load-time clamped to [30, 7200] AND to 60s below chat_turn_timeout_secs @dataclass diff --git a/docs/system-specs/modules/learn-cron-dashboard.md b/docs/system-specs/modules/learn-cron-dashboard.md index d4f859470c0..b1f23bbd20a 100644 --- a/docs/system-specs/modules/learn-cron-dashboard.md +++ b/docs/system-specs/modules/learn-cron-dashboard.md @@ -1001,7 +1001,7 @@ Cross-tab context: **removed** (budget redistributed to other caps). Previously **Context budget** (`context.py`): total cap 165,000 chars (~55k tokens). Priority order: critical rules → memory (preferences 4,250, projects 6,400, history 26,600) → skills (on-demand, few always-on) → lessons (37,250) → conversation history (8k budget, 8,000 chars/message cap, most-recent-first fill) → provenance. Individual messages exceeding 8,000 chars are truncated with `…[truncated]`. If total exceeds 165,000, hard-truncated at nearest newline. -**Per-turn timeout** (`constants.py:CHAT_TURN_TIMEOUT`): every `_run_chat` invocation is wrapped with `asyncio.wait_for(timeout=CHAT_TURN_TIMEOUT)` regardless of dispatch site. This applies uniformly to: primary user-typed turn (`chat_handlers.py`), queue-drain (`chat_runner.py` finally block), cron injection (`handlers/messaging.py`), Slack/dashboard nudge (`slack/gateway.py` autonudge path), subagent injection (`slack/gateway.py` two paths), and the post-fan-out synthesis turn (`chat_runner.py` drain/idle branch — fires one consolidated synthesis after the last sub-agent of a fan-out completes). The structured dashboard-monitor path runs `_run_chat` inside an authorization coroutine passed to `spawn_guarded_turn`; authorization is rechecked after the background permit and the helper still owns the same ceiling. The cap (7200s, 2 hours) is sized to match the inner ACP `_DEFAULT_PROMPT_TIMEOUT` so the dashboard layer does not bound below the transport. The `_STALE_TURN_TIMEOUT` (90s, in `acp/client.py`) is the real wedged-session guard — it fires when streaming has gone silent. `CHAT_TURN_TIMEOUT` is the upper safety ceiling for genuinely runaway work, not a "this turn took too long" guard. +**Per-turn timeout** (`constants.py:CHAT_TURN_TIMEOUT`): every `_run_chat` invocation is wrapped with `asyncio.wait_for(timeout=CHAT_TURN_TIMEOUT)` regardless of dispatch site. This applies uniformly to: primary user-typed turn (`chat_handlers.py`), queue-drain (`chat_runner.py` finally block), cron injection (`handlers/messaging.py`), Slack/dashboard nudge (`slack/gateway.py` autonudge path), subagent injection (`slack/gateway.py` two paths), and the post-fan-out synthesis turn (`chat_runner.py` drain/idle branch — fires one consolidated synthesis after the last sub-agent of a fan-out completes). The structured dashboard-monitor path runs `_run_chat` inside an authorization coroutine passed to `spawn_guarded_turn`; authorization is rechecked after the background permit and the helper still owns the same ceiling. The cap (14400s, 4 hours) is sized to match the inner ACP `_DEFAULT_PROMPT_TIMEOUT` so the dashboard layer does not bound below the transport; four hours is the longest single turn the shipped budgets can legitimately produce (the task runner's 90-minute test command plus a fix and a re-run, or a blocking subagent wave at its 2h wait cap), and anything longer belongs to the loop mechanisms, which end the turn between cycles. The `_STALE_TURN_TIMEOUT` (90s, in `acp/client.py`) is the real wedged-session guard — it fires when streaming has gone silent. `CHAT_TURN_TIMEOUT` is the upper safety ceiling for genuinely runaway work, not a "this turn took too long" guard. **Custom agent context**: When a dashboard slot uses a non-kirocrew agent, `build_message()` and `build_session_context()` skip only skills and workspace identity (custom agents load their own via kiro-cli). All other context is injected for all agents: critical rules (diff rendering, OPTIONS buttons), memory (preferences, projects, history, semantic, episodic), lessons, hooks, and OPTIONS reminder. This ensures custom agents, cron jobs, and task runners all benefit from the user's learned preferences and project context. diff --git a/src/kiro_crew/acp/client.py b/src/kiro_crew/acp/client.py index f4939350e8d..b7c41e2a860 100644 --- a/src/kiro_crew/acp/client.py +++ b/src/kiro_crew/acp/client.py @@ -943,7 +943,7 @@ def _mentions_skill_file(raw_params: dict | None, command: str | None) -> bool: # security filter cancels every tool use in an assistant turn (e.g. shell commands # containing "credentials"). After this text kiro-cli returns to an idle state waiting # for the next user prompt and NEVER sends a ``complete`` response for the in-flight -# ``session/prompt`` — so without special handling KiroCrew waits the full 2h timeout. +# ``session/prompt`` — so without special handling Kiro Crew waits the full prompt timeout. # Treating this chunk as end-of-turn unblocks the caller; the text itself is still # yielded so the user/agent sees what happened. We use an exact (stripped) match so # the detection does not fire if the model merely quotes the marker string in prose. @@ -1015,7 +1015,7 @@ def parse_slash_command(command: str) -> tuple[str, dict]: # active server. Must stay strictly below _DRAIN_DURATION, otherwise the hard cap # fires first and the idle path becomes dead code. _DRAIN_IDLE_EXIT = 0.5 -_DEFAULT_PROMPT_TIMEOUT = 7200.0 # 2 hours — allow very long tool execution +_DEFAULT_PROMPT_TIMEOUT = 14400.0 # 4 hours — mirrors constants.CHAT_TURN_TIMEOUT # Slack the transport leaves ABOVE the configured turn ceiling. The dashboard's # own deadline (turn_dispatch._bounded_turn) must always fire first so the user # sees the "turn hit the N-hour limit" card; a transport cut at the same instant @@ -6411,7 +6411,7 @@ async def _consult_liveness_model_wait(self) -> tuple[str, str]: strict improvement on the gateway's Linux deploy target. - Subtree-aggregate movement. A busy *unrelated* descendant (e.g. an MCP child polling) can read WORKING even if the model turn itself is - wedged with a lost completion frame, extending that turn to the 2h + wedged with a lost completion frame, extending that turn to the ``_DEFAULT_PROMPT_TIMEOUT`` backstop rather than reaping at 90s. This is an inherent property of the shared ``LivenessOracle`` (the kiro path has it too); tighter per-branch attribution belongs in @@ -6653,7 +6653,7 @@ async def _dispatch_events( if not is_thinking and _is_tool_interrupted_marker(chunk): # kiro-cli's built-in security filter cancelled the turn's tools. # It will not send a ``complete`` response — synthesize one so the - # caller exits instead of waiting 2 hours for the prompt timeout. + # caller exits instead of waiting out the prompt timeout. # (_emit_tool_interrupted_sel logs + audits the cancellation.) self._emit_tool_interrupted_sel("_dispatch_events") got_complete = True diff --git a/src/kiro_crew/acp/liveness.py b/src/kiro_crew/acp/liveness.py index 7f6f2eab203..a38aeac21a6 100644 --- a/src/kiro_crew/acp/liveness.py +++ b/src/kiro_crew/acp/liveness.py @@ -18,7 +18,8 @@ ("re-run non-interactively"). - ``UNKNOWN`` -> the only timeout-governed class, with non-lethal actions. -Evidence sources (all Linux ``/proc`` based, no new dependencies): +Evidence sources (Linux ``/proc`` first-class, a ``libproc`` backend on +macOS, no new dependencies): - **Shell tool in flight**: scan the runtime's descendant tree for a non-zombie child whose cmdline matches the session's cached command. A live match is @@ -44,6 +45,21 @@ :data:`EVIDENCE_ESTABLISHED_FLAT` tag so the caller can extend the probe window for probably-thinking (non-streamed reasoning) turns. +On macOS there is no procfs, so the tree is read in-process through ``libproc`` +(:class:`LibprocBackend`, selected once per oracle when the platform is darwin +and ``proc_root`` does not exist): ``proc_listchildpids`` enumerates the +runtime's descendants, ``PROC_PIDTBSDINFO`` dates each one and tells a zombie +apart, ``KERN_PROCARGS2`` (falling back to ``proc_pidpath``) supplies the +cmdline the SAME matching rules run against, and ``PROC_PIDTASKINFO`` sums the +subtree's CPU time. That gives macOS the shell-child match / exit detection / +absence narrowing and the MCP-subtree movement probe, with movement being +CPU-only (the evidence string says so — there is no per-process IO counter to +read). What has no libproc equivalent stays exactly as absent: no socket +evidence (so a flat model wait is UNKNOWN, never DEAD, and never tagged +``established_flat``), no wchan / blocked-fd evidence (so STUCK_INPUT is never +claimed). The backend is injectable (``darwin_backend`` ctor arg), and the +Linux path is untouched by its presence. + Every probe is wrapped: any error degrades the verdict to UNKNOWN — never to a kill. Each check is cheap (<10ms of file reads); two-sample deltas are computed across successive calls (the dispatch loop's queue-timeout ticks) rather than @@ -63,10 +79,11 @@ import logging import os import re +import sys import time from concurrent.futures import Executor from dataclasses import dataclass -from typing import Any, Callable, Protocol +from typing import Any, Callable, Iterable, Protocol from kiro_crew import platform_compat @@ -336,20 +353,51 @@ def established_inodes(proc_root: str, pid: int) -> set[str]: def boottime_now() -> float | None: - """Seconds since boot on the clock ``/proc`` dates processes against. + """Now, on the clock this host dates process starts against. + + Linux: ``CLOCK_BOOTTIME`` counts time spent suspended, exactly as + ``/proc/uptime`` and the ``starttime`` field of ``/proc//stat`` do. + ``time.monotonic()`` (``CLOCK_MONOTONIC``) does not, so the two MUST NOT be + mixed in one comparison: after a suspend of S seconds, a boot-clock age minus + a monotonic stamp places a process S seconds EARLIER than it really started, + which is how a live shell child comes to look like it predates its own + dispatch. + + macOS: ``libproc`` reports a process's start as an absolute wall-clock + instant (``pbi_start_tvsec``), so the stamp is ``time.time()`` — the same + clock, suspend included. That clock can STEP (NTP correction after a VM + resume, an admin reset), and a backward step between the stamp and the + runtime's fork dates a live child before its own dispatch. The oracle pairs + this stamp with :func:`steady_now` and refuses to attribute by start time + once the two disagree (see :meth:`LivenessOracle._started_after_dispatch`); + the stamp alone cannot tell a step from a slow spawn. + + Returns None where no such clock is available, which every caller must read + as "cannot attribute" rather than as a time. + """ + try: + return time.clock_gettime(time.CLOCK_BOOTTIME) + except (AttributeError, OSError): # pragma: no cover - platform dependent + if sys.platform == "darwin": + return time.time() + return None - ``CLOCK_BOOTTIME`` counts time spent suspended, exactly as ``/proc/uptime`` - and the ``starttime`` field of ``/proc//stat`` do. ``time.monotonic()`` - (``CLOCK_MONOTONIC``) does not, so the two MUST NOT be mixed in one - comparison: after a suspend of S seconds, a boot-clock age minus a monotonic - stamp places a process S seconds EARLIER than it really started, which is how - a live shell child comes to look like it predates its own dispatch. - Returns None where the clock is unavailable (no ``CLOCK_BOOTTIME``), which - every caller must read as "cannot attribute" rather than as a time. +def steady_now() -> float | None: + """A step-immune reading paired with :func:`boottime_now` on darwin. + + ``CLOCK_MONOTONIC`` there keeps counting through sleep (unlike + ``time.monotonic()``, which is ``mach_absolute_time`` and stops), so the pair + (wall stamp, steady stamp) taken at dispatch lets a later probe check whether + the wall clock has since moved by anything other than elapsed time. None on + a platform whose process-start clock cannot step (Linux dates processes on + ``CLOCK_BOOTTIME``), and wherever the clock is unavailable — a consumer + reads None as "cannot validate the wall clock", never as a time. """ + if sys.platform != "darwin": + return None try: - return time.clock_gettime(time.CLOCK_BOOTTIME) + return time.clock_gettime(time.CLOCK_MONOTONIC) except (AttributeError, OSError): # pragma: no cover - platform dependent return None @@ -371,6 +419,112 @@ def process_start_boot_secs(starttime_ticks: float) -> float | None: return starttime_ticks / hz +# ── Darwin process backend (a host without procfs) ── + +# Sample key for the darwin subtree CPU probe. Distinct from the ``/proc`` +# walk's jiffies keys and from the portable root-only nanosecond key: three +# counters in three units, and a host must never diff one against another. +_DARWIN_CPU_KEY = "darwin_cpu" +# Evidence label naming what the darwin movement probe can and cannot see: CPU +# time across the subtree, no IO — libproc has no per-process byte counter. +_DARWIN_CPU_LABEL = "darwin cpu-only" + + +@dataclass(frozen=True) +class ProcessRow: + """One live, non-zombie process in the shape the shell-child matcher reads. + + ``started`` is the process's start on the :func:`boottime_now` clock (None + when it cannot be dated, which the matcher reads as possibly-this-tool's). + ``cmdline`` is argv joined by single spaces — the executable path alone when + argv is unreadable, "" when nothing is — so the fragment match and the + program-basename match run unchanged against it. + """ + + pid: int + started: float | None + cmdline: str + + +class DarwinProcessBackend(Protocol): + """What the oracle asks of a host that has no ``/proc``. + + ``descendants`` answers the whole live subtree under ``root_pid`` (the root + itself excluded, any order, zombies may be included) or None when the tree + cannot be enumerated. The two are kept apart because only an enumerated + tree supports the absent-shell-child claim. ``row`` describes one live + process, or answers None for one that is gone, a zombie, or unreadable — + the shapes the ``/proc`` walk skips as ``stat is None or state == "Z"``. + ``cpu_nanos`` is the process's total CPU time, None when unreadable. + """ + + def descendants(self, root_pid: int) -> list[int] | None: ... + + def row(self, pid: int) -> ProcessRow | None: ... + + def cpu_nanos(self, pid: int) -> int | None: ... + + +class LibprocBackend: + """The production darwin backend, over ``platform_compat``'s libproc probes. + + Every call is an in-process syscall on a same-uid process: no fork, no + ``ps``, so it holds up on the oracle's cadence and inside the offloaded + consult's timeout. Matching fidelity is argv when ``KERN_PROCARGS2`` reads + (the normal case for the runtime's own children) and the executable path + otherwise, where only the program-basename rule can match. + """ + + def descendants(self, root_pid: int) -> list[int] | None: + # A root that cannot be read has no observable tree: proc_listchildpids + # answers 0 for an unknown pid exactly as for a childless one, so the + # root's own facts are what separate "empty" from "gone". + if platform_compat.darwin_process_facts(root_pid) is None: + return None + order: list[int] = [] + visited = {root_pid} + stack = [root_pid] + while stack: + parent = stack.pop() + children = platform_compat.darwin_child_pids(parent) + if children is None: + # One unreadable branch makes the whole tree unobservable — a + # partial list would ground an absence claim on missing data. + return None + for child in children: + if child not in visited: + visited.add(child) + order.append(child) + stack.append(child) + return order + + def row(self, pid: int) -> ProcessRow | None: + facts = platform_compat.darwin_process_facts(pid) + if facts is None or facts.zombie: + return None + argv = platform_compat.darwin_process_argv(pid) + if argv: + cmdline = " ".join(argv) + else: + cmdline = platform_compat.darwin_process_path(pid) or "" + return ProcessRow(pid=pid, started=facts.start_secs, cmdline=cmdline) + + def cpu_nanos(self, pid: int) -> int | None: + return platform_compat.proc_cpu_nanos_for_pid(pid) + + +def select_darwin_backend(proc_root: str) -> DarwinProcessBackend | None: + """The backend an oracle uses for *proc_root*, or None to walk ``/proc``. + + Selected on the platform AND on ``proc_root`` being absent, so a fake + ``/proc`` tree handed to the oracle under test keeps the ``/proc`` walk on + any host, and a Linux host with procfs never sees libproc. + """ + if sys.platform == "darwin" and not os.path.exists(proc_root): + return LibprocBackend() + return None + + # ── Command matching ── @@ -445,6 +599,13 @@ class ToolCallState: # when the clock is unavailable; every consumer must then decline to attribute # a process to this dispatch rather than guess. dispatch_boot_ts: float | None = None + # ``steady_now()`` taken alongside ``dispatch_boot_ts``. On darwin that stamp + # is the wall clock, which can step; this one cannot, so the pair lets the + # oracle detect a step since dispatch before it grounds an absence claim on + # start-time attribution. None where the platform needs no check (Linux) or + # the caller took no such stamp — on darwin the oracle then declines to + # attribute at all rather than trust an unvalidated wall clock. + dispatch_steady_ts: float | None = None # Consumer parking banked in THIS TURN before the stamp above was taken # (``AcpSessionHandle._parked_total``, which is per-turn and includes the whole # of a human approval wait). The stamp is taken when the tool_call frame is @@ -463,7 +624,7 @@ class ToolCallState: class LivenessOracle: - """Per-session liveness verdicts from /proc evidence. + """Per-session liveness verdicts from /proc evidence (libproc on macOS). One instance per :class:`~kiro_crew.acp.session_handle.AcpSessionHandle`. Stateful across checks: it tracks the matched shell child (so exit @@ -484,10 +645,24 @@ def __init__( *, now=time.monotonic, sample_min_secs: float = 3.0, + darwin_backend: DarwinProcessBackend | None = None, + wall_now=time.time, + steady_now_fn=steady_now, ) -> None: self._proc = str(proc_root) self._now = now + # The darwin start-time ruler and its step detector (see + # ``_started_after_dispatch``); injectable so a test can step one without + # the other. Unused on a ``/proc`` host. + self._wall_now = wall_now + self._steady_now = steady_now_fn self._sample_min_secs = sample_min_secs + # The one platform probe: decided at construction so every check on this + # instance reads the same source. A caller passes its own backend to + # drive the darwin paths without libproc (tests) or to pin the choice. + self._darwin = ( + darwin_backend if darwin_backend is not None else select_darwin_backend(self._proc) + ) self._tracked_child: int | None = None self._child_gone_ts: float | None = None # sample key -> (ts, counter). Keys: "io", "cpu". @@ -521,7 +696,12 @@ def fresh(self) -> "LivenessOracle": movement. """ return LivenessOracle( - self._proc, now=self._now, sample_min_secs=self._sample_min_secs + self._proc, + now=self._now, + sample_min_secs=self._sample_min_secs, + darwin_backend=self._darwin, + wall_now=self._wall_now, + steady_now_fn=self._steady_now, ) # ── Public checks ── @@ -606,56 +786,114 @@ def _check_tool(self, runtime_pid: int | None, tool: ToolCallState) -> tuple[str return VERDICT_UNKNOWN, f"mcp subtree flat ({evidence})" def _check_shell_child(self, runtime_pid: int, tool: ToolCallState) -> tuple[str, str]: + if self._darwin is not None: + return self._check_shell_child_darwin(self._darwin, runtime_pid, tool) descendants = iter_descendants(self._proc, runtime_pid) # Exact exit detection once a child was matched. if self._tracked_child is not None: stat = read_pid_stat(self._proc, self._tracked_child) alive = stat is not None and stat[0] != "Z" and self._tracked_child in descendants - if alive: - self._child_gone_ts = None - stuck = self._stuck_input_check(self._tracked_child) - if stuck: - return VERDICT_STUCK_INPUT, stuck - return VERDICT_WORKING, f"shell child {self._tracked_child} alive" - if self._child_gone_ts is None: - self._child_gone_ts = self._now() - gone_for = self._now() - self._child_gone_ts - if gone_for > CHILD_EXIT_GRACE_SECS: - return ( - VERDICT_DEAD, - f"shell child {self._tracked_child} exited {gone_for:.0f}s ago, no result frame", - ) - return VERDICT_UNKNOWN, f"shell child exited {gone_for:.0f}s ago (grace)" + return self._tracked_child_verdict(self._tracked_child, alive) # Not matched yet: scan for a live non-zombie descendant whose cmdline # matches this session's cached command. The same pass answers a second, # cmdline-INDEPENDENT question — is any live descendant young enough to # have been started by this dispatch — which is what separates "the # command already exited" from "the command is running unrecognized". - fragment = match_fragment(tool.command) - program = first_program(tool.command) - live_descendants = 0 - started_since_dispatch = False - matched_but_older = False + matched, live_descendants, possibly_running = self._scan_for_child( + self._proc_rows(runtime_pid, descendants), tool + ) + if matched is not None: + return self._track_child(matched) + if possibly_running: + return VERDICT_UNKNOWN, "no matching shell child" + if not live_descendants and not children_interface_readable(self._proc, runtime_pid): + # No live descendant AND no readable child list: the tree is not + # observable (no procfs, no CONFIG_PROC_CHILDREN, a sandbox hiding + # the subtree), which is indistinguishable from an empty one. Absence + # is not assertable, so nothing narrows. + return VERDICT_UNKNOWN, "no matching shell child" + return self._absent_child_verdict(live_descendants) + + def _check_shell_child_darwin( + self, backend: DarwinProcessBackend, runtime_pid: int, tool: ToolCallState + ) -> tuple[str, str]: + """The ``/proc`` shell-child check, fed by *backend* instead of procfs. + + Same tracked-child exit detection, same matching pass, same three-way + unmatched verdict. The observability question the ``/proc`` walk answers + with ``children_interface_readable`` is answered here by enumeration + itself: a tree that could not be read is None, and only a read tree + grounds an absence claim. + """ + descendants = backend.descendants(runtime_pid) + + if self._tracked_child is not None: + row = backend.row(self._tracked_child) + # Membership is tested only against a tree that could be read: a + # transient enumeration failure must not age a live child through the + # grace into DEAD, the one verdict this branch can be wrong about. + alive = row is not None and (descendants is None or self._tracked_child in descendants) + return self._tracked_child_verdict(self._tracked_child, alive) + + if descendants is None: + return VERDICT_UNKNOWN, "no matching shell child" + rows = (row for row in map(backend.row, descendants) if row is not None) + matched, live_descendants, possibly_running = self._scan_for_child(rows, tool) + if matched is not None: + return self._track_child(matched) + if possibly_running: + return VERDICT_UNKNOWN, "no matching shell child" + return self._absent_child_verdict(live_descendants) + + def _proc_rows(self, runtime_pid: int, descendants: list[int]) -> Iterable[ProcessRow]: + """The live, non-zombie descendants in *descendants*, read from ``/proc``. + + The start is dated before the cmdline is read, on purpose: a live + descendant whose cmdline is momentarily unreadable (mid-exec, + mid-teardown) still counts as possibly-this-tool's in the scan. + """ for pid in descendants: if pid == runtime_pid: continue stat = read_pid_stat(self._proc, pid) if stat is None or stat[0] == "Z": continue + yield ProcessRow( + pid=pid, + started=process_start_boot_secs(stat[1]), + cmdline=read_cmdline(self._proc, pid), + ) + + def _scan_for_child( + self, rows: Iterable[ProcessRow], tool: ToolCallState + ) -> tuple[int | None, int, bool]: + """``(matched_pid, live_descendants, possibly_running)`` over *rows*. + + ``matched_pid`` is the first live descendant whose cmdline matches the + cached command AND that is young enough to be this dispatch's own. + ``possibly_running`` is True when something that could be this command + is running unrecognized: a descendant started since the dispatch (the + match heuristic may have missed a live command — a shell that exec'd + away, a cached input redacted past any usable fragment), or one whose + cmdline matches while predating a late-taken stamp. Either vetoes the + absence claim; the caller keeps the plain evidence and the full window. + """ + fragment = match_fragment(tool.command) + program = first_program(tool.command) + live_descendants = 0 + started_since_dispatch = False + matched_but_older = False + for row in rows: live_descendants += 1 - # Evaluated before the cmdline gate on purpose: a live descendant - # whose cmdline is momentarily unreadable (mid-exec, mid-teardown) - # still counts as possibly-this-tool's. - fresh = self._started_after_dispatch(stat[1], tool) + fresh = self._started_after_dispatch(row.started, tool) started_since_dispatch = started_since_dispatch or fresh - cmdline = read_cmdline(self._proc, pid) - if not cmdline: + if not row.cmdline: continue - matched = bool(fragment) and fragment in cmdline + matched = bool(fragment) and fragment in row.cmdline if not matched and program: - base_tokens = {t.rsplit("/", 1)[-1] for t in cmdline.split()} + base_tokens = {t.rsplit("/", 1)[-1] for t in row.cmdline.split()} matched = program in base_tokens if not matched: continue @@ -667,33 +905,42 @@ def _check_shell_child(self, runtime_pid: int, tool: ToolCallState) -> tuple[str # stamp is taken when that frame is PROCESSED, and the consumer # can park for minutes on an approval, an IM send or a hook # (see ``_parked`` in the dispatch loop) while kiro-cli has - # already spawned. So refuse the match as before, but let it - # veto the absence claim below: a live process that looks like - # this command is not evidence that nothing is running. + # already spawned. So refuse the match, but let it veto the + # absence claim: a live process that looks like this command is + # not evidence that nothing is running. matched_but_older = True continue - self._tracked_child = pid + return row.pid, live_descendants, True + return None, live_descendants, started_since_dispatch or matched_but_older + + def _track_child(self, pid: int) -> tuple[str, str]: + self._tracked_child = pid + self._child_gone_ts = None + # Prime the stuck-detection movement baseline now so the NEXT check + # can already compare deltas (otherwise stuck detection needs three + # ticks: match, baseline, compare). + self._tree_movement(pid, key_prefix="stuck") + return VERDICT_WORKING, f"shell child {pid} matched command" + + def _tracked_child_verdict(self, pid: int, alive: bool) -> tuple[str, str]: + """Verdict for the tracked shell child *pid* given whether it is still alive.""" + if alive: self._child_gone_ts = None - # Prime the stuck-detection movement baseline now so the NEXT check - # can already compare deltas (otherwise stuck detection needs three - # ticks: match, baseline, compare). - self._tree_movement(pid, key_prefix="stuck") - return VERDICT_WORKING, f"shell child {pid} matched command" - if started_since_dispatch or matched_but_older: - # Something that could be this command is running: either a - # descendant young enough to have been started by this dispatch (the - # match heuristic may have missed a live command — a shell that - # exec'd away, a cached input redacted past any usable fragment), or - # one whose cmdline matches while predating a late-taken stamp. That - # is what build-scale forbearance exists for, so keep the plain - # evidence and the full suspect window. - return VERDICT_UNKNOWN, "no matching shell child" - if not live_descendants and not children_interface_readable(self._proc, runtime_pid): - # No live descendant AND no readable child list: the tree is not - # observable (no procfs, no CONFIG_PROC_CHILDREN, a sandbox hiding - # the subtree), which is indistinguishable from an empty one. Absence - # is not assertable, so nothing narrows. - return VERDICT_UNKNOWN, "no matching shell child" + stuck = self._stuck_input_check(pid) + if stuck: + return VERDICT_STUCK_INPUT, stuck + return VERDICT_WORKING, f"shell child {pid} alive" + if self._child_gone_ts is None: + self._child_gone_ts = self._now() + gone_for = self._now() - self._child_gone_ts + if gone_for > CHILD_EXIT_GRACE_SECS: + return ( + VERDICT_DEAD, + f"shell child {pid} exited {gone_for:.0f}s ago, no result frame", + ) + return VERDICT_UNKNOWN, f"shell child exited {gone_for:.0f}s ago (grace)" + + def _absent_child_verdict(self, live_descendants: int) -> tuple[str, str]: # The tree is observable and nothing in it was started for this tool, so # the command is not running: the same physical state the DEAD branch # above reports as "exited, no result frame" — the oracle just never got @@ -707,13 +954,13 @@ def _check_shell_child(self, runtime_pid: int, tool: ToolCallState) -> tuple[str f"({live_descendants} live descendants, none started since dispatch)", ) - def _started_after_dispatch(self, starttime_ticks: float, tool: ToolCallState) -> bool: - """Whether a process is young enough to be *tool*'s own child. + def _started_after_dispatch(self, started: float | None, tool: ToolCallState) -> bool: + """Whether a process started at *started* is young enough to be *tool*'s. - Both sides are read on the boot clock — the process's ``starttime`` and - the stamp ``boottime_now()`` took at EVENT_TOOL_CALL — so a host suspend - between dispatch and this probe moves neither. Deriving the start from an - age instead (a boot-clock age subtracted from a monotonic stamp) placed a + Both sides are read on the same clock — the process's start and the stamp + ``boottime_now()`` took at EVENT_TOOL_CALL — so a host suspend between + dispatch and this probe moves neither. Deriving the start from an age + instead (a boot-clock age subtracted from a monotonic stamp) placed a live child a full suspend EARLIER than it started, so a laptop resumed mid-command read as "this child predates its own dispatch". @@ -724,18 +971,46 @@ def _started_after_dispatch(self, starttime_ticks: float, tool: ToolCallState) - than its own dispatch. The dispatch loop already measures exactly that park, so the bound is measured rather than guessed. - Fail-open by design: a missing boot stamp or an unreadable tick rate - answers True, so an unattributable process reads as possibly-this-tool's. - That keeps a live command matched and keeps an absence claim from resting - on evidence the oracle does not have. + Fail-open by design: a missing stamp or an undatable process (no tick + rate on Linux, no readable start elsewhere) answers True, so an + unattributable process reads as possibly-this-tool's. That keeps a live + command matched and keeps an absence claim from resting on evidence the + oracle does not have. """ if not tool.dispatch_boot_ts: return True - started_boot = process_start_boot_secs(starttime_ticks) - if started_boot is None: + if started is None: + return True + if self._darwin is not None and self._wall_clock_stepped_since(tool): return True tolerance = _DISPATCH_START_TOLERANCE_SECS + max(0.0, tool.dispatch_parked_secs) - return started_boot >= tool.dispatch_boot_ts - tolerance + return started >= tool.dispatch_boot_ts - tolerance + + def _wall_clock_stepped_since(self, tool: ToolCallState) -> bool: + """Whether the darwin start-time ruler has moved since *tool* was stamped. + + On darwin both sides of the start-time comparison are wall clock, and a + wall clock steps. A backward step landing between the dispatch stamp and + the runtime's fork dates a live child BEFORE its dispatch; a command the + matchers cannot recognise (fully redacted input) then has nothing to veto + the absence claim, and the caller narrows to the stale window and cancels + live work. So the stamp is paired with a steady clock: when wall elapsed + and steady elapsed disagree by more than the attribution tolerance, the + wall clock is not a ruler this dispatch can be measured with, and every + row reads as possibly-this-tool's (fail-open, like an undatable process). + A missing steady stamp is the same answer: an absence claim needs a + validated ruler, and there is none to validate. + + A suspend does not trip this: ``CLOCK_MONOTONIC`` on darwin counts sleep, + as the wall clock does. If a host's steady clock did stop across a sleep, + the disagreement would still fail toward the WIDER window, never toward + a cancel. + """ + if tool.dispatch_boot_ts is None or tool.dispatch_steady_ts is None: + return True + wall_elapsed = self._wall_now() - tool.dispatch_boot_ts + steady_elapsed = self._steady_now() - tool.dispatch_steady_ts + return abs(wall_elapsed - steady_elapsed) > _DISPATCH_START_TOLERANCE_SECS def _stuck_input_check(self, pid: int) -> str: """STUCK_INPUT evidence for a live child subtree, or "" when not stuck. @@ -797,6 +1072,13 @@ def _check_model_wait(self, runtime_pid: int | None) -> tuple[str, str]: if evidence == EVIDENCE_SAMPLING: # No baseline yet — cannot attest either way. return VERDICT_UNKNOWN, evidence + if self._darwin is not None: + # A genuinely flat subtree, but the split below rests on socket + # evidence that only ``/proc`` carries. Without it a flat wait is + # neither the lost-frame wedge nor a probable think: UNKNOWN with + # plain evidence, so the caller's timeout policy governs and + # nothing is invented in either direction. + return VERDICT_UNKNOWN, f"backend subtree flat ({evidence})" # Flat counters: distinguish the done-but-lost-frame wedge (no backend # connection at all) from a probably-thinking server-side silence # (established socket, nothing flowing yet). @@ -845,6 +1127,8 @@ def _tree_movement(self, root_pid: int, key_prefix: str = "") -> tuple[bool, str ``(False, "sampling")``. Counters can only shrink when processes exit, which itself is movement — negative deltas count as moved. """ + if self._darwin is not None: + return self._darwin_tree_movement(self._darwin, root_pid, key_prefix) io_total = 0 cpu_total = 0 io_seen = False @@ -879,6 +1163,43 @@ def _tree_movement(self, root_pid: int, key_prefix: str = "") -> tuple[bool, str self._samples[cpu_key] = (now, cpu_total) return (io_delta != 0 or cpu_delta != 0), f"io {io_delta:+d}B cpu {cpu_delta:+d}t" + def _darwin_tree_movement( + self, backend: DarwinProcessBackend, root_pid: int, key_prefix: str + ) -> tuple[bool, str]: + """(moved, evidence) for the CPU delta of *root_pid*'s subtree via *backend*. + + The ``/proc`` walk's two-sample contract, CPU-only: libproc exposes no + per-process byte counter, so an IO-bound but CPU-idle subtree (a long + ``recv`` on a remote call) reads flat here where Linux would see bytes. + The evidence names that limit. A tree that cannot be enumerated reads as + "no readable counters" — absent evidence, same as an unreadable procfs. + """ + descendants = backend.descendants(root_pid) + if descendants is None: + return False, "no readable counters" + cpu_total = 0 + cpu_seen = False + for pid in (root_pid, *descendants): + cpu = backend.cpu_nanos(pid) + if cpu is not None: + cpu_total += cpu + cpu_seen = True + if not cpu_seen: + return False, "no readable counters" + now = self._now() + key = f"{key_prefix}:{_DARWIN_CPU_KEY}" if key_prefix else _DARWIN_CPU_KEY + prev = self._samples.get(key) + if prev is None: + self._samples[key] = (now, cpu_total) + return False, EVIDENCE_SAMPLING + cpu_delta = cpu_total - prev[1] + if now - prev[0] < self._sample_min_secs: + # Too soon for a fresh delta — report against the stored baseline + # without advancing it. + return cpu_delta != 0, f"cpu {cpu_delta:+d}ns ({_DARWIN_CPU_LABEL}, early)" + self._samples[key] = (now, cpu_total) + return cpu_delta != 0, f"cpu {cpu_delta:+d}ns ({_DARWIN_CPU_LABEL})" + def _portable_movement(self, root_pid: int) -> tuple[bool, str]: """(moved, evidence) for *root_pid*'s CPU delta, without ``/proc``. @@ -892,7 +1213,9 @@ def _portable_movement(self, root_pid: int) -> tuple[bool, str]: Root pid only, so a busy DESCENDANT under an idle root reads flat here. That is the conservative direction (it never invents movement), and the case where the work lives in a child — an open tool call — is governed by - the caller's tool-stall policy rather than by this probe. + the caller's tool-stall policy rather than by this probe. On macOS the + darwin backend's subtree probe answers first; this one is reached there + only when that backend could not enumerate the tree. """ cpu = platform_compat.proc_cpu_nanos_for_pid(root_pid) if cpu is None: diff --git a/src/kiro_crew/acp/session_handle.py b/src/kiro_crew/acp/session_handle.py index 3606283bee7..b0cd5a96929 100644 --- a/src/kiro_crew/acp/session_handle.py +++ b/src/kiro_crew/acp/session_handle.py @@ -69,6 +69,7 @@ _consume_future_exception, boottime_now, consult_offloaded, + steady_now, ) from kiro_crew.acp.mcp_session_report import McpSessionReport from kiro_crew.acp.prompt_blocks import build_prompt_blocks, summarize_prompt_structure @@ -137,10 +138,10 @@ class WatchdogSettings: :data:`_TURN_CEILING_WINDOW_FRACTION` for the enforced headroom.""" check_after_secs: float = 60.0 - stale_window_secs: float = 300.0 - tool_stall_suspect_secs: float = 3600.0 - tool_stall_hard_cap_secs: float = 3600.0 - model_silent_probe_secs: float = 900.0 + stale_window_secs: float = 600.0 + tool_stall_suspect_secs: float = 5400.0 + tool_stall_hard_cap_secs: float = 7200.0 + model_silent_probe_secs: float = 1800.0 wellness_sample_secs: float = 3.0 # Whether a per-agent watchdog_tool_stall_* override was applied to this # snapshot. Telemetry-only (the kirocrew.watchdog.action attr): a BOOLEAN, @@ -2240,7 +2241,7 @@ async def _dispatch_events( self._log_working_deferral(_tool_idle, evidence, timeout) continue # UNKNOWN acts at the suspect window. The suspect - # default (1h) is BUILD-scale forbearance — an LLM-shaped + # default (90 min) is BUILD-scale forbearance — an LLM-shaped # stall (flat subtree whose only live evidence is an # established backend socket: a model turn riding inside # a tool, e.g. kiro-cli use_subagent) narrows to the @@ -2363,9 +2364,9 @@ async def _dispatch_events( # user"), and an unacked cancel confirms the wedge via the # unresponsive-cancel branch at the loop top. # ``window`` = "extended" when the established_flat - # model-wait probe window (model_silent_probe_secs, 900s) + # model-wait probe window (model_silent_probe_secs, 1800s) # governed the decision instead of the ordinary stale - # window (stale_window_secs, 300s). The established_flat + # window (stale_window_secs, 600s). The established_flat # case is an EXTENSION for model-wait (silence of a # non-streamed think), not a narrowing as on the tool # branch — emitting "extended" lets dashboards distinguish @@ -2864,7 +2865,7 @@ def _emit_watchdog_metric( "standard" (default), "narrowed" (a tool-branch tag reduces the build-scale suspect window — established_flat to the model-silent budget, shell_child_absent to the ordinary silence window), or "extended" - (model-wait established_flat extends the 300s stale window to the + (model-wait established_flat extends the 600s stale window to the model-silent probe window for a non-streamed server-side think). ``agent_override`` is the per-agent-override BOOLEAN from the settings snapshot — deliberately NOT the agent name (per-agent joins happen via @@ -3696,6 +3697,7 @@ def _handle_update(self, msg: JsonRpcMessage) -> list[AcpEvent]: command=ev.tool_input, dispatch_ts=time.monotonic(), dispatch_boot_ts=boottime_now(), + dispatch_steady_ts=steady_now(), dispatch_parked_secs=self._parked_total, is_shell=ev.is_shell, tool_name=ev.tool_name, diff --git a/src/kiro_crew/config/loader.py b/src/kiro_crew/config/loader.py index 70aabb07ab4..86bfdeff918 100644 --- a/src/kiro_crew/config/loader.py +++ b/src/kiro_crew/config/loader.py @@ -2689,8 +2689,8 @@ def _load_resolved(cls) -> tuple[KiroCrewConfig, int]: agent_data.get("subagent_mem_buffer_pct", 20), 20 ), chat_turn_timeout_secs=_safe_int( - agent_data.get("chat_turn_timeout_secs", 7200), - 7200, + agent_data.get("chat_turn_timeout_secs", 14400), + 14400, CHAT_TURN_TIMEOUT_MIN, CHAT_TURN_TIMEOUT_MAX, ), @@ -2848,15 +2848,15 @@ def _load_resolved(cls) -> tuple[KiroCrewConfig, int]: ), watchdog=WatchdogConfig( check_after_secs=_safe_float(watchdog_data.get("check_after_secs", 60.0), 60.0), - stale_window_secs=_safe_float(watchdog_data.get("stale_window_secs", 300.0), 300.0), + stale_window_secs=_safe_float(watchdog_data.get("stale_window_secs", 600.0), 600.0), tool_stall_suspect_secs=_safe_float( - watchdog_data.get("tool_stall_suspect_secs", 3600.0), 3600.0 + watchdog_data.get("tool_stall_suspect_secs", 5400.0), 5400.0 ), tool_stall_hard_cap_secs=_safe_float( - watchdog_data.get("tool_stall_hard_cap_secs", 3600.0), 3600.0 + watchdog_data.get("tool_stall_hard_cap_secs", 7200.0), 7200.0 ), model_silent_probe_secs=_safe_float( - watchdog_data.get("model_silent_probe_secs", 900.0), 900.0 + watchdog_data.get("model_silent_probe_secs", 1800.0), 1800.0 ), wellness_sample_secs=_safe_float( watchdog_data.get("wellness_sample_secs", 3.0), 3.0 diff --git a/src/kiro_crew/config/sections.py b/src/kiro_crew/config/sections.py index b62d1cfb963..abc201c6fa5 100644 --- a/src/kiro_crew/config/sections.py +++ b/src/kiro_crew/config/sections.py @@ -1039,16 +1039,18 @@ class AgentConfig: ), ) chat_turn_timeout_secs: int = field( - default=7200, + default=14400, metadata=_meta( "Chat Turn Timeout (secs)", "Wall-clock ceiling for one chat turn. This is a runaway backstop, " "so it is clamped to 300s..86400s (24h) and can never be disabled. " - "Raise it above the 2h default for long unattended turns (full test " - "suites, long builds); the ACP transport's prompt wait follows it. " - "Hitting the ceiling is visible: the turn ends with a card naming " - "the limit. For work spanning days, prefer monitor/goal loops — " - "they end the turn between cycles and survive restarts.", + "The 4h default covers the longest single turn the shipped budgets " + "produce (a 90-minute test command plus a fix and a re-run, or a " + "blocking subagent wave at its 2h wait cap); the ACP transport's " + "prompt wait follows it. Hitting the ceiling is visible: the turn ends " + "with a card naming the limit. For work spanning many hours or days, " + "prefer monitor/goal loops — they end the turn between cycles and " + "survive restarts, which a single marathon turn cannot.", ), ) session_start_timeout_secs: int = field( @@ -3456,13 +3458,18 @@ def __post_init__(self) -> None: # agent.chat_turn_timeout_secs — wall-clock ceiling for one chat turn. The ACP # transport's per-prompt wait follows this value (acp/client.py # ``resolve_prompt_timeout``, which adds a margin so the dashboard's visible -# card fires before the transport cut), so the max is no longer pinned to the -# transport's 2h default. It is bounded at 24h because the ceiling is a runaway -# backstop, not a scheduler: a single prompt→response turn longer than a day is -# pathological, and multi-day unattended operation belongs to the loop -# mechanisms (monitor/goal loops, crons), which end the turn between cycles and -# survive restarts — a marathon turn does not. The floor keeps the backstop -# from being set so low it cuts ordinary work. +# card fires before the transport cut), so the max is not pinned to the +# transport's default. The default is 4h: the longest single turn the shipped +# budgets can legitimately produce is a 90-minute test command plus a fix and a +# re-run, or a blocking subagent wave at its 2h wait cap plus synthesis, and +# the watchdog's UNKNOWN-verdict windows must sit well inside the ceiling so its +# non-lethal recovery is reachable before the turn is cut. It is bounded at 24h +# because the ceiling is a runaway backstop, not a scheduler: a single +# prompt→response turn longer than a day is pathological, and multi-day +# unattended operation belongs to the loop mechanisms (monitor/goal loops, +# crons), which end the turn between cycles and survive restarts — a marathon +# turn does not. The floor keeps the backstop from being set so low it cuts +# ordinary work. CHAT_TURN_TIMEOUT_MIN = 300 CHAT_TURN_TIMEOUT_MAX = 86400 @@ -4692,24 +4699,33 @@ class WatchdogConfig: ), ) stale_window_secs: float = field( - default=300.0, + default=600.0, metadata=_meta( "Stale probe window (s)", "Idle seconds before an UNKNOWN-verdict model-wait turn is safe-probed " - "via session/cancel. Probes are non-lethal: a live turn auto-recovers.", + "via session/cancel. Probes are non-lethal, but a probe of a LIVE think " + "cancels and regenerates it, so the window must clear an ordinary " + "silent think. Default 10 min. A think the oracle can attest (an " + "established backend socket) gets the longer model-silent window " + "instead, so this one governs only thinks with no such evidence: a " + "host without procfs, or a backend connection that is momentarily " + "down. Its cost is wedge-recovery latency on a runtime that is " + "already dead, never lost work.", ), ) tool_stall_suspect_secs: float = field( - default=3600.0, + default=5400.0, metadata=_meta( "Tool stall suspect (s)", "Idle seconds before an UNKNOWN-verdict in-flight tool is cancelled and " "the turn routed to tool-stall recovery (continue-nudge, no re-run of " - "the original message). WORKING tools (e.g. a matched live build child) " - "are never cancelled regardless of duration. Default 1h: generous enough " - "for long builds and MCP tools on macOS, where the liveness oracle " - "degrades (no /proc) and cannot distinguish a live build from a stall, " - "while still landing inside the turn's own ceiling " + "the original message). WORKING tools (a matched live build child, an " + "MCP subtree with CPU movement) are never cancelled regardless of " + "duration, so this window governs only what the liveness oracle cannot " + "attest. Default 90 min: it clears every shipped budget a single tool " + "call can legitimately spend silent (the task runner's 90-minute test " + "command, a full test suite with one retry in one shell call) while " + "still landing inside the turn's own ceiling " "(agent.chat_turn_timeout_secs) so recovery is reachable. Enforcement is " "at handle construction, not config load: a window past the headroom " "fraction of the transport's per-prompt timeout is clamped with a " @@ -4719,25 +4735,27 @@ class WatchdogConfig: ), ) tool_stall_hard_cap_secs: float = field( - default=3600.0, + default=7200.0, metadata=_meta( "Hard cap (s)", "Absolute ceiling for UNKNOWN-verdict forbearance (e.g. the extended " - "probably-thinking window). Applies ONLY to UNKNOWN verdicts — never " - "to a WORKING session, which is deferred before this cap is consulted " - "and is therefore bounded only by the turn's own ceiling. Default 1h, " - "clamped against the transport's per-prompt timeout like the suspect " - "window.", + "probably-thinking window) and for any per-agent " + "watchdog_tool_stall_* override. Applies ONLY to UNKNOWN verdicts — " + "never to a WORKING session, which is deferred before this cap is " + "consulted and is therefore bounded only by the turn's own ceiling. " + "Default 2h, clamped against the transport's per-prompt timeout like " + "the suspect window.", ), ) model_silent_probe_secs: float = field( - default=900.0, + default=1800.0, metadata=_meta( "Silent-think probe window (s)", "Extended probe window for a model-wait with an established backend " "connection but flat counters (non-streamed server-side reasoning, " "e.g. long xhigh thinks). Probing a live think cancels and regenerates " - "it, so this window is deliberately generous.", + "it, so this window is deliberately generous: 30 min clears the long " + "end of an extended-effort think.", ), ) wellness_sample_secs: float = field( diff --git a/src/kiro_crew/constants.py b/src/kiro_crew/constants.py index f5d830f7673..a526a53f013 100644 --- a/src/kiro_crew/constants.py +++ b/src/kiro_crew/constants.py @@ -46,12 +46,16 @@ def env_flag_enabled(name: str) -> bool: # Outer wall-clock cap on a single ``_run_chat`` invocation (any dispatch site: # primary user turn, queue-drain, cron injection, subagent injection, Slack first -# turn). Sized to match the inner ACP ``_DEFAULT_PROMPT_TIMEOUT`` (7200s) in +# turn). Sized to match the inner ACP ``_DEFAULT_PROMPT_TIMEOUT`` (14400s) in # ``acp/client.py`` so the dashboard layer doesn't bound below the transport. +# Four hours is the longest single turn the shipped budgets can legitimately +# produce (the task runner's 90-minute test command plus a fix and a re-run, or a +# blocking subagent wave at its 2h wait cap plus synthesis); work that outlives +# it belongs to the loop mechanisms, which end the turn between cycles. # Wedged-session detection is handled by ``_STALE_TURN_TIMEOUT`` (90s, also in # ``acp/client.py``); this cap is the upper safety ceiling for genuinely runaway # work, not a "this turn took too long" guard. -CHAT_TURN_TIMEOUT = 7200.0 +CHAT_TURN_TIMEOUT = 14400.0 # How long the dashboard chat path parks a turn waiting for a human to answer a # tool-approval prompt, when config is unavailable (tests, early bootstrap). diff --git a/src/kiro_crew/dashboard/state.py b/src/kiro_crew/dashboard/state.py index 2d9f193d239..0160e8ab48f 100644 --- a/src/kiro_crew/dashboard/state.py +++ b/src/kiro_crew/dashboard/state.py @@ -5818,11 +5818,11 @@ def status_snapshot( MAX_BACKGROUND_TURNS_CEIL = 16 # hard ceiling — config can raise up to here # Longest a queued turn may sit waiting for a permit. Needed because the # queue wait happens INSIDE the coroutine ``spawn_guarded_turn`` already - # bounds at ``CHAT_TURN_TIMEOUT`` (7200s), so an unbounded wait would let a + # bounds at ``CHAT_TURN_TIMEOUT`` (14400s), so an unbounded wait would let a # fully-saturated cap consume a turn's whole ceiling and then kill it with - # "turn exceeded the 7200s ceiling" — a true statement that names the wrong - # cause. 1800s never trips under ordinary throttling and leaves 90 minutes - # of the ceiling for the turn itself; on expiry the turn fails with a + # "turn exceeded the 14400s ceiling" — a true statement that names the wrong + # cause. 1800s never trips under ordinary throttling and leaves three and a half + # hours of the ceiling for the turn itself; on expiry the turn fails with a # message that says what actually happened. _BACKGROUND_QUEUE_WAIT_SECS = 1800 diff --git a/src/kiro_crew/dashboard/turn_dispatch.py b/src/kiro_crew/dashboard/turn_dispatch.py index d790d2c2432..79f6c5d893e 100644 --- a/src/kiro_crew/dashboard/turn_dispatch.py +++ b/src/kiro_crew/dashboard/turn_dispatch.py @@ -69,7 +69,7 @@ def _acp_prompt_ceiling() -> float: """The transport's own per-prompt timeout. Resolves through :func:`~kiro_crew.acp.client.resolve_prompt_timeout`, which - follows a configured ceiling ABOVE the 2h default (plus a margin so this + follows a configured ceiling ABOVE the 4h default (plus a margin so this module's card always fires before the transport cut). The clamp in :func:`chat_turn_timeout_secs` therefore no longer fires in normal operation; it stays as the fail-safe for a resolver that could not read config. diff --git a/src/kiro_crew/platform_compat.py b/src/kiro_crew/platform_compat.py index 680f94d45a2..be9baae72c9 100644 --- a/src/kiro_crew/platform_compat.py +++ b/src/kiro_crew/platform_compat.py @@ -1088,6 +1088,210 @@ def _darwin_process_cpu_nanos(pid: int) -> int | None: return None +# Further ``proc_bsdinfo`` fields the liveness backend reads: ``pbi_status`` is +# the second uint32 (offset 4). ``SZOMB`` is the BSD process-state code for +# a zombie; in practice the kernel refuses ``PROC_PIDTBSDINFO`` for a zombie +# outright, so the code is a second line of defence, not the primary test. +_DARWIN_PBI_STATUS_OFFSET = 4 +_DARWIN_SZOMB = 5 + +# ``proc_pidpath`` fails unless handed ``PROC_PIDPATHINFO_MAXSIZE`` bytes, which +# is four times ``MAXPATHLEN``. +_DARWIN_PIDPATH_MAXSIZE = 4 * _DARWIN_MAXPATHLEN + +# ``sysctl(CTL_KERN, KERN_PROCARGS2, pid)`` returns an ``int`` argc, the exec +# path, NUL padding, then the argv strings and finally the environment. The +# kernel truncates silently to the buffer handed in (no error), so a bounded +# buffer costs at most the tail of an unusually long argv — never a failure. +_DARWIN_CTL_KERN = 1 +_DARWIN_KERN_PROCARGS2 = 49 +_DARWIN_PROCARGS_BUFSIZE = 64 * 1024 + +# ``proc_listchildpids`` writes ``pid_t`` values and returns HOW MANY it wrote. +# A childless parent and a pid that does not exist both answer 0, so a caller +# that needs to tell them apart reads the parent's own facts first. +_DARWIN_CHILD_LIST_INITIAL = 1024 + + +class DarwinProcessFacts(NamedTuple): + """One process as ``PROC_PIDTBSDINFO`` describes it. + + ``start_secs`` is the absolute wall-clock start instant (``pbi_start_tvsec`` + + ``pbi_start_tvusec``), i.e. the same clock ``time.time()`` reads — the pair + a caller compares to date a process against an event it stamped itself. + """ + + zombie: bool + start_secs: float + + +_darwin_libproc_tree_bound = False + + +def _darwin_libproc_tree_handle() -> Any: + """The cached ``libproc`` handle with the tree-walk entry points declared. + + Declared lazily and separately from :func:`_darwin_libproc_handle` so a + libproc missing either symbol still serves the cwd / start-time / CPU probes + that need only ``proc_pidinfo``. Returns None when the extra entry points + cannot be bound. + """ + global _darwin_libproc_tree_bound + lib = _darwin_libproc_handle() + if lib is None: + return None + if _darwin_libproc_tree_bound: + return lib + try: + lib.proc_listchildpids.argtypes = [ctypes.c_int, ctypes.c_void_p, ctypes.c_int] + lib.proc_listchildpids.restype = ctypes.c_int + lib.proc_pidpath.argtypes = [ctypes.c_int, ctypes.c_void_p, ctypes.c_uint32] + lib.proc_pidpath.restype = ctypes.c_int + except Exception: + return None + _darwin_libproc_tree_bound = True + return lib + + +def darwin_child_pids(ppid: int) -> list[int] | None: + """Direct children of *ppid* via ``proc_listchildpids``; None when unreadable. + + In-process and unprivileged: pid enumeration needs no entitlement for any + process. The buffer grows until the kernel's answer fits, so a parent with + more children than the initial capacity is enumerated completely rather than + truncated. An empty list is a genuine answer only for a parent that exists — + the syscall answers 0 for an unknown pid too — so callers that assert + absence pair this with :func:`darwin_process_facts` on the parent. + """ + lib = _darwin_libproc_tree_handle() + if lib is None: + return None + capacity = _DARWIN_CHILD_LIST_INITIAL + try: + while True: + buf = ctypes.create_string_buffer(capacity * 4) + count = lib.proc_listchildpids(ppid, buf, capacity * 4) + if count < 0: + return None + if count < capacity: + return list(struct.unpack_from(f"<{count}i", buf.raw, 0)) + capacity *= 2 + except Exception: + return None + + +def darwin_process_facts(pid: int) -> DarwinProcessFacts | None: + """``PROC_PIDTBSDINFO`` of *pid*, or None when the process cannot be read. + + None covers "gone", "zombie" (the kernel refuses the query for one) and + "not ours to inspect" (another user's process) alike: each is a process the + caller cannot attribute work to. A same-uid live process always answers. + """ + lib = _darwin_libproc_handle() + if lib is None: + return None + try: + buf = ctypes.create_string_buffer(_DARWIN_PROC_BSDINFO_SIZE) + filled = lib.proc_pidinfo(pid, _DARWIN_PROC_PIDTBSDINFO, 0, buf, _DARWIN_PROC_BSDINFO_SIZE) + if filled != _DARWIN_PROC_BSDINFO_SIZE: + return None + status = struct.unpack_from(" str | None: + """Executable path of *pid* via ``proc_pidpath``, or None when unreadable.""" + lib = _darwin_libproc_tree_handle() + if lib is None: + return None + try: + buf = ctypes.create_string_buffer(_DARWIN_PIDPATH_MAXSIZE) + length = lib.proc_pidpath(pid, buf, _DARWIN_PIDPATH_MAXSIZE) + if length <= 0: + return None + return buf.raw[:length].decode("utf-8", errors="replace") or None + except Exception: + return None + + +_darwin_libc_sysctl: Any = None +_darwin_libc_sysctl_loaded = False + + +def _darwin_sysctl_handle() -> Any: + """Cached ``libc`` handle with ``sysctl`` declared, or None when unavailable. + + Cached for the same reason as the libproc handle: the argv probe runs per + descendant on the liveness oracle's cadence, and a fresh ``CDLL`` per call + would dlopen every time. + """ + global _darwin_libc_sysctl, _darwin_libc_sysctl_loaded + if _darwin_libc_sysctl_loaded: + return _darwin_libc_sysctl + _darwin_libc_sysctl_loaded = True + try: + path = ctypes.util.find_library("c") + if path is None: + return None + libc = ctypes.CDLL(path) + libc.sysctl.argtypes = [ + ctypes.POINTER(ctypes.c_int), + ctypes.c_uint, + ctypes.c_void_p, + ctypes.POINTER(ctypes.c_size_t), + ctypes.c_void_p, + ctypes.c_size_t, + ] + libc.sysctl.restype = ctypes.c_int + _darwin_libc_sysctl = libc + except Exception: + _darwin_libc_sysctl = None + return _darwin_libc_sysctl + + +def darwin_process_argv(pid: int) -> list[str] | None: + """argv of *pid* via ``sysctl KERN_PROCARGS2``, or None when unreadable. + + Same-uid processes only (the kernel answers EPERM for another user's), and + bounded to :data:`_DARWIN_PROCARGS_BUFSIZE` — the kernel truncates rather + than fails, so a very long argv comes back with its tail cut, which still + carries the program and the head of its arguments. Returns None rather than + an empty list when the record cannot be parsed, so a caller can fall back to + the executable path instead of treating "unreadable" as "no arguments". + """ + libc = _darwin_sysctl_handle() + if libc is None: + return None + try: + mib = (ctypes.c_int * 3)(_DARWIN_CTL_KERN, _DARWIN_KERN_PROCARGS2, pid) + buf = ctypes.create_string_buffer(_DARWIN_PROCARGS_BUFSIZE) + size = ctypes.c_size_t(_DARWIN_PROCARGS_BUFSIZE) + if libc.sysctl(mib, 3, buf, ctypes.byref(size), None, 0) != 0: + return None + raw = buf.raw[: size.value] + if len(raw) < 4: + return None + argc = struct.unpack_from(" str | None: """Current working directory of *pid*, or None when no source can answer. @@ -4874,10 +5078,11 @@ def proc_cpu_nanos_for_pid(pid: int) -> int | None: handle. - Any other platform: None. - Root pid ONLY — deliberately no subtree walk, because neither macOS nor - Windows has a child enumeration cheap enough for a probe on a read loop's - cadence. A caller that needs the subtree total and knows it is on Linux uses - :func:`proc_subtree_sample` instead. + Root pid ONLY — deliberately no subtree walk here: this probe runs on a read + loop's cadence and Windows has no child enumeration cheap enough for that. A + caller that needs the subtree total uses :func:`proc_subtree_sample` on Linux, + or walks :func:`darwin_child_pids` and sums this per pid on macOS (the + liveness oracle's darwin backend does exactly that). """ if type(pid) is not int or pid <= 0: return None diff --git a/src/kiro_crew/slack/gateway.py b/src/kiro_crew/slack/gateway.py index 3daa6e3d8fe..f4f9745b816 100644 --- a/src/kiro_crew/slack/gateway.py +++ b/src/kiro_crew/slack/gateway.py @@ -7750,7 +7750,7 @@ async def _inject_with_retry( _slot_name, ) # Bounded by the configured turn ceiling - # (chat_turn_timeout_secs, 7200s default): + # (chat_turn_timeout_secs, 14400s default): # _run_chat's finally block drains slot._queue # on any exit path. # Carry the structured completion facts so the diff --git a/test/test_acp_liveness_darwin.py b/test/test_acp_liveness_darwin.py new file mode 100644 index 00000000000..7915d660099 --- /dev/null +++ b/test/test_acp_liveness_darwin.py @@ -0,0 +1,528 @@ +"""Unit tests for the liveness oracle's darwin backend (``acp/liveness.py``). + +On a host without procfs the oracle reads the runtime's tree through a +``DarwinProcessBackend``. These tests drive that path with a fake backend — a +hand-built descendant table, no libproc — so they are deterministic on every +platform. The ``/proc`` walk keeps its own tests in ``test_acp_liveness.py``; +this file pins that the darwin path reaches the same verdicts from the same +matching rules, and that the evidence procfs alone carries is never invented. +""" + +from __future__ import annotations + +import os +import sys + +import pytest + +from kiro_crew.acp import liveness +from kiro_crew.acp.liveness import ( + CHILD_EXIT_GRACE_SECS, + EVIDENCE_ESTABLISHED_FLAT, + EVIDENCE_SAMPLING, + EVIDENCE_SHELL_CHILD_ABSENT, + VERDICT_DEAD, + VERDICT_UNKNOWN, + VERDICT_WORKING, + LibprocBackend, + LivenessOracle, + ProcessRow, + ToolCallState, +) + +RUNTIME = 100 + + +class _Clock: + def __init__(self, t: float = 1000.0) -> None: + self.t = t + + def __call__(self) -> float: + return self.t + + def advance(self, secs: float) -> None: + self.t += secs + + +class FakeBackend: + """A descendant table standing in for libproc. + + ``add(pid, cmdline, started=..., cpu=...)`` registers a live process under + the runtime; ``remove`` makes it gone; ``enumerable=False`` answers None from + ``descendants`` the way an unreadable tree does. + """ + + def __init__(self, *, enumerable: bool = True) -> None: + self.enumerable = enumerable + self.rows: dict[int, ProcessRow] = {} + self.cpu: dict[int, int] = {} + + def add( + self, pid: int, cmdline: str, *, started: float | None = 10_000_000.0, cpu: int = 0 + ) -> None: + self.rows[pid] = ProcessRow(pid=pid, started=started, cmdline=cmdline) + self.cpu[pid] = cpu + + def remove(self, pid: int) -> None: + self.rows.pop(pid, None) + self.cpu.pop(pid, None) + + # -- DarwinProcessBackend -- + + def descendants(self, root_pid: int) -> list[int] | None: + if not self.enumerable: + return None + return [pid for pid in self.rows if pid != root_pid] + + def row(self, pid: int) -> ProcessRow | None: + return self.rows.get(pid) + + def cpu_nanos(self, pid: int) -> int | None: + if pid == RUNTIME: + return 1_000_000 + return self.cpu.get(pid) + + +def _oracle( + backend: FakeBackend, + clock: _Clock, + tmp_path, + sample_min: float = 3.0, + wall: _Clock | None = None, +): + # A proc_root that does not exist: the oracle must not read a fake tree here. + # The wall clock (what libproc dates processes on) defaults to the same fake + # clock as the steady one, so the two agree unless a test steps ``wall``. + return LivenessOracle( + str(tmp_path / "nonexistent"), + now=clock, + sample_min_secs=sample_min, + darwin_backend=backend, + wall_now=wall if wall is not None else clock, + steady_now_fn=clock, + ) + + +def _shell_tool(command: str, clock: _Clock, wall: _Clock | None = None) -> ToolCallState: + return ToolCallState( + title="bash", + command=command, + dispatch_ts=clock.t, + dispatch_boot_ts=(wall if wall is not None else clock).t, + dispatch_steady_ts=clock.t, + is_shell=True, + ) + + +def _mcp_tool(clock: _Clock, tool_name: str = "use_subagent") -> ToolCallState: + return ToolCallState( + title="kirocrew-core___use_subagent", + command='{"task": "x"}', + dispatch_ts=clock.t, + dispatch_boot_ts=clock.t, + dispatch_steady_ts=clock.t, + is_shell=False, + tool_name=tool_name, + ) + + +# ── (a) match ──────────────────────────────────────────────────────────────── + + +def test_matched_live_child_is_working_and_tracked(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(200, "/bin/bash -c long-build release > build.log 2>&1") + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("long-build release > build.log 2>&1", clock) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_WORKING + assert evidence == "shell child 200 matched command" + assert oracle._tracked_child == 200 + + clock.advance(4.0) + verdict, evidence = oracle.check_tool(RUNTIME, tool) + assert verdict == VERDICT_WORKING + assert evidence == "shell child 200 alive" + + +def test_program_basename_matches_when_only_the_path_is_readable(tmp_path): + """argv unreadable → the row carries the executable path, and the + program-name rule still recognizes it (reduced fidelity, same verdict).""" + clock = _Clock() + backend = FakeBackend() + backend.add(200, "/opt/homebrew/bin/brazil-build") + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("brazil-build release", clock) + + assert oracle.check_tool(RUNTIME, tool) == (VERDICT_WORKING, "shell child 200 matched command") + + +# ── (b) exit detection ─────────────────────────────────────────────────────── + + +def test_tracked_child_gone_is_unknown_in_grace_then_dead(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(200, "/bin/bash -c long-build release > build.log 2>&1") + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("long-build release > build.log 2>&1", clock) + assert oracle.check_tool(RUNTIME, tool)[0] == VERDICT_WORKING + + backend.remove(200) + clock.advance(1.0) + verdict, evidence = oracle.check_tool(RUNTIME, tool) + assert verdict == VERDICT_UNKNOWN + assert "grace" in evidence + + clock.advance(CHILD_EXIT_GRACE_SECS + 1.0) + verdict, evidence = oracle.check_tool(RUNTIME, tool) + assert verdict == VERDICT_DEAD + assert evidence.startswith("shell child 200 exited") + + +def test_enumeration_failure_does_not_age_a_live_tracked_child_into_dead(tmp_path): + """The child still reads (alive) while the tree cannot be enumerated: that + is not an exit, and DEAD is the one verdict this branch must not guess.""" + clock = _Clock() + backend = FakeBackend() + backend.add(200, "/bin/bash -c long-build release > build.log 2>&1") + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("long-build release > build.log 2>&1", clock) + assert oracle.check_tool(RUNTIME, tool)[0] == VERDICT_WORKING + + backend.enumerable = False + clock.advance(CHILD_EXIT_GRACE_SECS + 5.0) + assert oracle.check_tool(RUNTIME, tool)[0] == VERDICT_WORKING + + +# ── (c) observable tree, nothing started since dispatch ───────────────────── + + +def test_observable_tree_with_only_old_descendants_is_tagged_absent(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(201, "python -m kiro_crew.mcp_gateway.stub --server github", started=0.0) + backend.add(202, "python -m kiro_crew.mcp_gateway.stub --server slack", started=0.0) + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("ls /some/dir | grep needle | wc -l", clock) + clock.advance(61.0) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert evidence.startswith(EVIDENCE_SHELL_CHILD_ABSENT), evidence + assert "2 live descendants" in evidence + + +def test_empty_but_enumerable_tree_is_an_absent_child(tmp_path): + clock = _Clock() + oracle = _oracle(FakeBackend(), clock, tmp_path) + tool = _shell_tool("ls /some/dir | grep needle | wc -l", clock) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert evidence.startswith(EVIDENCE_SHELL_CHILD_ABSENT), evidence + + +def test_young_unmatched_descendant_vetoes_the_absence_claim(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(201, "python -m kiro_crew.mcp_gateway.stub --server github", started=0.0) + backend.add(300, "/opt/vendor/bin/opaque-worker --serve") # young + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("[REDACTED-CREDENTIAL] x", clock) + + assert oracle.check_tool(RUNTIME, tool) == (VERDICT_UNKNOWN, "no matching shell child") + + +def test_backward_wall_step_never_grounds_an_absence_claim(tmp_path): + """The darwin ruler is the wall clock, and it can step backwards between the + dispatch stamp and the runtime's fork. A live child then predates its own + dispatch; with a command the matchers cannot recognise there is nothing to + veto the absence claim, and the caller would narrow to the stale window and + cancel live work. The steady stamp exposes the step, and attribution fails + open instead.""" + clock = _Clock() + wall = _Clock(1_000_000.0) + backend = FakeBackend() + oracle = _oracle(backend, clock, tmp_path, wall=wall) + tool = _shell_tool("[REDACTED-CREDENTIAL] x", clock, wall=wall) + # NTP steps the wall clock back two minutes, then kiro-cli forks the command: + # libproc dates the child at the post-step instant, 120s before its dispatch. + wall.t -= 120.0 + backend.add(300, "/opt/vendor/bin/opaque-worker --serve", started=wall.t) + clock.advance(61.0) + wall.advance(61.0) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert not evidence.startswith(EVIDENCE_SHELL_CHILD_ABSENT), evidence + + +def test_backward_wall_step_keeps_a_matched_child_working(tmp_path): + clock = _Clock() + wall = _Clock(1_000_000.0) + backend = FakeBackend() + oracle = _oracle(backend, clock, tmp_path, wall=wall) + tool = _shell_tool("make -j8 all && ./run-tests.sh --verbose", clock, wall=wall) + wall.t -= 120.0 + backend.add(300, "/bin/sh -c make -j8 all && ./run-tests.sh --verbose", started=wall.t) + clock.advance(61.0) + wall.advance(61.0) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_WORKING, evidence + assert "300" in evidence + + +def test_agreeing_clocks_still_date_an_old_descendant_as_old(tmp_path): + """Negative control for the step guard: with wall and steady clocks in + agreement the ruler is trusted, so a genuinely old descendant is still what + grounds the absence claim (the same setup as the tagged-absent test, with an + explicit wall clock).""" + clock = _Clock() + wall = _Clock(1_000_000.0) + backend = FakeBackend() + backend.add(201, "python -m kiro_crew.mcp_gateway.stub --server github", started=0.0) + oracle = _oracle(backend, clock, tmp_path, wall=wall) + tool = _shell_tool("[REDACTED-CREDENTIAL] x", clock, wall=wall) + clock.advance(61.0) + wall.advance(61.0) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert evidence.startswith(EVIDENCE_SHELL_CHILD_ABSENT), evidence + + +def test_missing_steady_stamp_declines_to_attribute_on_darwin(tmp_path): + """No steady stamp means the wall clock cannot be validated, and an absence + claim on darwin rests on nothing else — so none is made.""" + clock = _Clock() + backend = FakeBackend() + backend.add(201, "python -m kiro_crew.mcp_gateway.stub --server github", started=0.0) + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("[REDACTED-CREDENTIAL] x", clock) + tool.dispatch_steady_ts = None + clock.advance(61.0) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert not evidence.startswith(EVIDENCE_SHELL_CHILD_ABSENT), evidence + + +def test_undatable_descendant_keeps_the_full_window(tmp_path): + """A row without a start reads as possibly-this-tool's: fail-open.""" + clock = _Clock() + backend = FakeBackend() + backend.add(201, "python -m kiro_crew.mcp_gateway.stub --server github", started=None) + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("ls /some/dir | grep needle | wc -l", clock) + + assert oracle.check_tool(RUNTIME, tool) == (VERDICT_UNKNOWN, "no matching shell child") + + +# ── (d) tree cannot be enumerated ──────────────────────────────────────────── + + +def test_unenumerable_tree_is_plain_unknown_without_an_absence_claim(tmp_path): + clock = _Clock() + backend = FakeBackend(enumerable=False) + backend.add(200, "/bin/bash -c long-build release") + oracle = _oracle(backend, clock, tmp_path) + tool = _shell_tool("long-build release", clock) + + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert (verdict, evidence) == (VERDICT_UNKNOWN, "no matching shell child") + assert oracle._tracked_child is None + + +# ── (e) MCP subtree movement ───────────────────────────────────────────────── + + +def test_mcp_subtree_cpu_delta_is_working_with_the_cpu_only_label(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(210, "python -m kiro_crew.mcp_gateway.stub --server github", cpu=5_000_000_000) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + tool = _mcp_tool(clock, tool_name="ReadInternalWebsites") + + assert oracle.check_tool(RUNTIME, tool) == ( + VERDICT_UNKNOWN, + f"mcp subtree flat ({EVIDENCE_SAMPLING})", + ) + + backend.cpu[210] += 250_000_000 + clock.advance(2.0) + verdict, evidence = oracle.check_tool(RUNTIME, tool) + assert verdict == VERDICT_WORKING + assert evidence == "mcp subtree active (cpu +250000000ns (darwin cpu-only))" + + +def test_mcp_subtree_without_any_cpu_counter_is_plain_unknown(tmp_path): + clock = _Clock() + backend = FakeBackend(enumerable=False) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + + verdict, evidence = oracle.check_tool(RUNTIME, _mcp_tool(clock)) + assert (verdict, evidence) == (VERDICT_UNKNOWN, "mcp subtree flat (no readable counters)") + + +def test_model_wait_subtree_cpu_delta_is_working(tmp_path): + """The busy descendant under an idle root that the root-only portable probe + could not see now reads WORKING through the subtree sum.""" + clock = _Clock() + backend = FakeBackend() + backend.add(210, "python -m kiro_crew.mcp_gateway.stub --server github", cpu=5_000_000_000) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + + assert oracle.check_model_wait(RUNTIME) == (VERDICT_UNKNOWN, EVIDENCE_SAMPLING) + backend.cpu[210] += 1_000_000 + clock.advance(2.0) + verdict, evidence = oracle.check_model_wait(RUNTIME) + assert verdict == VERDICT_WORKING + assert evidence == "backend activity (cpu +1000000ns (darwin cpu-only))" + + +# ── (f) /proc-only evidence is never invented ──────────────────────────────── + + +def test_flat_model_wrapping_tool_is_not_tagged_established_flat_on_darwin(tmp_path): + clock = _Clock() + backend = FakeBackend() + backend.add(210, "python -m kiro_crew.mcp_gateway.stub --server github", cpu=5_000_000_000) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + tool = _mcp_tool(clock, tool_name="use_subagent") + + oracle.check_tool(RUNTIME, tool) # baseline + clock.advance(2.0) + verdict, evidence = oracle.check_tool(RUNTIME, tool) + + assert verdict == VERDICT_UNKNOWN + assert not evidence.startswith(EVIDENCE_ESTABLISHED_FLAT), evidence + assert evidence == "mcp subtree flat (cpu +0ns (darwin cpu-only))" + + +def test_flat_model_wait_is_unknown_never_dead_on_darwin(tmp_path): + """No socket evidence → the lost-frame wedge cannot be claimed.""" + clock = _Clock() + backend = FakeBackend() + backend.add(210, "python -m kiro_crew.mcp_gateway.stub --server github", cpu=5_000_000_000) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + + oracle.check_model_wait(RUNTIME) # baseline + clock.advance(2.0) + verdict, evidence = oracle.check_model_wait(RUNTIME) + + assert verdict == VERDICT_UNKNOWN + assert not evidence.startswith(EVIDENCE_ESTABLISHED_FLAT), evidence + assert evidence == "backend subtree flat (cpu +0ns (darwin cpu-only))" + + +def test_live_tracked_child_never_reads_stuck_input_on_darwin(tmp_path): + """wchan / blocked-fd evidence is /proc-only: a flat live child is WORKING.""" + clock = _Clock() + backend = FakeBackend() + backend.add(200, "/bin/bash -c long-build release", cpu=1_000) + oracle = _oracle(backend, clock, tmp_path, sample_min=1.0) + tool = _shell_tool("long-build release", clock) + + assert oracle.check_tool(RUNTIME, tool)[0] == VERDICT_WORKING + clock.advance(2.0) + assert oracle.check_tool(RUNTIME, tool) == (VERDICT_WORKING, "shell child 200 alive") + clock.advance(2.0) + assert oracle.check_tool(RUNTIME, tool) == (VERDICT_WORKING, "shell child 200 alive") + + +# ── Backend selection & lifecycle ──────────────────────────────────────────── + + +def test_fake_proc_tree_never_selects_the_darwin_backend(tmp_path): + """A readable proc_root pins the /proc walk on every host.""" + proc = tmp_path / "proc" + proc.mkdir() + assert LivenessOracle(str(proc))._darwin is None + assert liveness.select_darwin_backend(str(proc)) is None + + +def test_absent_proc_root_selects_libproc_only_on_darwin(tmp_path, monkeypatch): + missing = str(tmp_path / "nonexistent") + monkeypatch.setattr(sys, "platform", "linux") + assert liveness.select_darwin_backend(missing) is None + monkeypatch.setattr(sys, "platform", "darwin") + assert isinstance(liveness.select_darwin_backend(missing), LibprocBackend) + + +def test_fresh_carries_the_backend(tmp_path): + backend = FakeBackend() + oracle = _oracle(backend, _Clock(), tmp_path) + assert oracle.fresh()._darwin is backend + + +def test_darwin_dispatch_stamp_is_wall_clock(monkeypatch): + """Where CLOCK_BOOTTIME is absent, darwin dates processes on the wall clock, + so the dispatch stamp must come from the same clock.""" + monkeypatch.delattr(liveness.time, "CLOCK_BOOTTIME", raising=False) + monkeypatch.setattr(sys, "platform", "darwin") + before = liveness.time.time() + stamp = liveness.boottime_now() + assert stamp is not None + assert before <= stamp <= liveness.time.time() + monkeypatch.setattr(sys, "platform", "win32") + assert liveness.boottime_now() is None + + +def test_steady_stamp_is_taken_only_where_the_start_clock_can_step(monkeypatch): + """The gate is the platform, not the host: a faked clock makes the test prove + the gate on every CI OS, including one with no ``clock_gettime`` at all.""" + monkeypatch.setattr(liveness.time, "clock_gettime", lambda clk: 12_345.0, raising=False) + monkeypatch.setattr(liveness.time, "CLOCK_MONOTONIC", 6, raising=False) + monkeypatch.setattr(sys, "platform", "darwin") + assert liveness.steady_now() == 12_345.0 + monkeypatch.setattr(sys, "platform", "linux") + assert liveness.steady_now() is None + + +@pytest.mark.skipif(sys.platform != "darwin", reason="reads the real darwin clock") +def test_steady_stamp_reads_a_monotonic_clock_on_darwin(): + first = liveness.steady_now() + assert first is not None + assert first <= liveness.steady_now() + + +# ── Real libproc smoke (own subtree only) ──────────────────────────────────── + + +@pytest.mark.skipif(sys.platform != "darwin", reason="libproc is macOS only") +def test_libproc_backend_reads_own_subtree(): + import subprocess + + backend = LibprocBackend() + child = subprocess.Popen(["/bin/sleep", "30"]) + try: + descendants = backend.descendants(os.getpid()) + assert descendants is not None + assert child.pid in descendants + row = backend.row(child.pid) + assert row is not None + assert row.started is not None + assert "sleep" in row.cmdline + assert row.started <= liveness.boottime_now() + 1.0 + assert backend.cpu_nanos(os.getpid()) is not None + finally: + child.kill() + child.wait() + # A reaped child is gone: no row, and it drops out of the tree. + assert backend.row(child.pid) is None + gone = backend.descendants(os.getpid()) + assert gone is not None and child.pid not in gone + # An unknown pid has no observable tree. + assert backend.descendants(2**22 - 1) is None diff --git a/test/test_acp_stale_recovery.py b/test/test_acp_stale_recovery.py index ec81ecd7dde..0da51bd62e4 100644 --- a/test/test_acp_stale_recovery.py +++ b/test/test_acp_stale_recovery.py @@ -39,6 +39,7 @@ STOP_REASON_TOOL_STALL, JsonRpcMessage, ) +from kiro_crew.config.loader import WatchdogConfig from kiro_crew.dashboard.state import ( STALE_RECOVERY_PREFIX, TOOL_STALL_RECOVERY_PREFIX, @@ -86,6 +87,10 @@ def _last_activity(self) -> float: return time.monotonic() +# The shipped global windows, so a default change lands here without a literal to chase. +_GLOBAL = WatchdogConfig() + + def _make_handle( last_activity: float | None = None, watchdog: WatchdogSettings = _FAST_WD, @@ -584,8 +589,8 @@ def test_per_agent_override_narrows_watchdog_snapshot(monkeypatch): assert wd.tool_stall_suspect_secs == 900.0 assert wd.tool_stall_hard_cap_secs == 1800.0 # Non-overridden windows inherit the globals untouched. - assert wd.model_silent_probe_secs == 900.0 - assert wd.stale_window_secs == 300.0 + assert wd.model_silent_probe_secs == _GLOBAL.model_silent_probe_secs + assert wd.stale_window_secs == _GLOBAL.stale_window_secs def test_per_agent_override_zero_inherits_global(monkeypatch): @@ -599,8 +604,8 @@ def test_per_agent_override_zero_inherits_global(monkeypatch): }) wd = _load_watchdog_settings("builder") - assert wd.tool_stall_suspect_secs == 3600.0 - assert wd.tool_stall_hard_cap_secs == 3600.0 + assert wd.tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs + assert wd.tool_stall_hard_cap_secs == _GLOBAL.tool_stall_hard_cap_secs def test_kiro_binding_name_is_not_resolved(monkeypatch): @@ -619,7 +624,7 @@ def test_kiro_binding_name_is_not_resolved(monkeypatch): ), }) - assert _load_watchdog_settings("pr-reviewer-kiro").tool_stall_suspect_secs == 3600.0 + assert _load_watchdog_settings("pr-reviewer-kiro").tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs def test_shared_binding_cannot_collide_canonical_names(monkeypatch): @@ -657,7 +662,7 @@ def test_handle_snapshots_crew_agent_overrides(monkeypatch): assert handle._watchdog.tool_stall_suspect_secs == 450.0 assert handle._watchdog.agent_override is True bare = AcpSessionHandle("s2", asyncio.Queue(), rt) - assert bare._watchdog.tool_stall_suspect_secs == 3600.0 + assert bare._watchdog.tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs assert bare._watchdog.agent_override is False @@ -676,7 +681,7 @@ def test_rebind_watchdog_follows_warm_pool_rekey(monkeypatch): rt = MagicMock() rt.pid = None handle = AcpSessionHandle("s1", asyncio.Queue(), rt) # pool spawn: no crew - assert handle._watchdog.tool_stall_suspect_secs == 3600.0 + assert handle._watchdog.tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs handle.rebind_watchdog("claimer") assert handle._crew_agent == "claimer" @@ -684,7 +689,7 @@ def test_rebind_watchdog_follows_warm_pool_rekey(monkeypatch): assert handle._watchdog.agent_override is True handle.rebind_watchdog("") - assert handle._watchdog.tool_stall_suspect_secs == 3600.0 + assert handle._watchdog.tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs assert handle._watchdog.agent_override is False @@ -695,8 +700,8 @@ def test_unknown_agent_inherits_global(monkeypatch): _cfg_with_agent_overrides(monkeypatch, {}) - assert _load_watchdog_settings("nope").tool_stall_suspect_secs == 3600.0 - assert _load_watchdog_settings("").tool_stall_suspect_secs == 3600.0 + assert _load_watchdog_settings("nope").tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs + assert _load_watchdog_settings("").tool_stall_suspect_secs == _GLOBAL.tool_stall_suspect_secs @pytest.mark.asyncio diff --git a/test/test_acp_watchdog_windows.py b/test/test_acp_watchdog_windows.py index a1e38eb14bc..4da716ac024 100644 --- a/test/test_acp_watchdog_windows.py +++ b/test/test_acp_watchdog_windows.py @@ -106,7 +106,8 @@ def test_loader_defaults_match_the_handle_snapshot_defaults() -> None: def test_over_ceiling_window_is_clamped_with_a_warning( monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture ) -> None: - cfg = _fake_config(tool_stall_suspect_secs=10800.0, tool_stall_hard_cap_secs=10800.0) + over = _WINDOW_BUDGET * 2 + cfg = _fake_config(tool_stall_suspect_secs=over, tool_stall_hard_cap_secs=over) with caplog.at_level(logging.WARNING, logger=_LOGGER_NAME): wd = _load_with(monkeypatch, cfg) diff --git a/test/test_chat_turn_timeout_consistency.py b/test/test_chat_turn_timeout_consistency.py index f9fa00794f4..05b8faf6b3f 100644 --- a/test/test_chat_turn_timeout_consistency.py +++ b/test/test_chat_turn_timeout_consistency.py @@ -7,7 +7,7 @@ cron injection path (handlers/messaging.py), the Slack/dashboard nudge path (slack/gateway.py:_handle_nudge), and the cron-script delivery path (slack/gateway.py:_deliver_script_result) remained unwrapped — depending on -the inner ACP _DEFAULT_PROMPT_TIMEOUT (7200s) instead. +the inner ACP _DEFAULT_PROMPT_TIMEOUT (14400s) instead. This module verifies the cap value is correct AND that all helper-visible dispatch sites in the source tree are wrapped with ``asyncio.wait_for(..., @@ -62,16 +62,22 @@ def test_cap_matches_inner_acp_prompt_timeout() -> None: ) -def test_cap_value_is_seven_thousand_two_hundred() -> None: +def test_cap_value_is_four_hours() -> None: """Regression guard against silently changing the value back to 600s. The 600s value was sized for a recovery-path budget, not the master cap. - 7200s aligns with the ACP layer underneath. If you intend to change this, - update docs/system-specs/modules/learn-cron-dashboard.md too. + 14400s covers the longest single turn the shipped budgets produce (a + 90-minute test command plus a fix and a re-run) and aligns with the ACP + layer underneath. If you intend to change this, update + docs/system-specs/modules/learn-cron-dashboard.md too, and keep the config + default (``AgentConfig.chat_turn_timeout_secs``) in step: a config-less + context must behave exactly like a default config. """ + from kiro_crew.config.loader import AgentConfig from kiro_crew.constants import CHAT_TURN_TIMEOUT - assert CHAT_TURN_TIMEOUT == 7200.0 + assert CHAT_TURN_TIMEOUT == 14400.0 + assert AgentConfig().chat_turn_timeout_secs == CHAT_TURN_TIMEOUT def _find_create_task_dispatches(path: Path) -> list[tuple[int, str]]: diff --git a/test/test_config_loader.py b/test/test_config_loader.py index f9e4a7cf38a..896e1775a00 100644 --- a/test/test_config_loader.py +++ b/test/test_config_loader.py @@ -37,6 +37,7 @@ SessionConfig, SlackConfig, SttConfig, + WatchdogConfig, WorkspaceConfig, _migrate_workspaces, _validated_stt_model, @@ -4195,7 +4196,7 @@ def test_invalid_import_onboarded_inherits_onboarded_without_jsonschema( def test_absent_sections_use_defaults(self) -> None: cfg = _load_from_dict({}) assert cfg.orchestrator.stage_timeout_seconds == 1800 - assert cfg.watchdog.tool_stall_hard_cap_secs == 3600.0 + assert cfg.watchdog.tool_stall_hard_cap_secs == WatchdogConfig().tool_stall_hard_cap_secs assert cfg.dashboard.theme_mode == "" assert cfg.dashboard.onboarded is False assert cfg.dashboard.import_onboarded is False diff --git a/test/test_dashboard_approval_window.py b/test/test_dashboard_approval_window.py index 90eb56737c1..efc3f18d8a5 100644 --- a/test/test_dashboard_approval_window.py +++ b/test/test_dashboard_approval_window.py @@ -324,11 +324,19 @@ def test_default_window_survives_the_default_ceiling(self) -> None: _clamp_security_bounds(data) assert data["agent"]["tool_approval_timeout_secs"] == 600 - def test_absent_ceiling_uses_the_field_default(self) -> None: - """Omitting the ceiling must not disable the cross-field clamp.""" + def test_absent_ceiling_uses_the_field_default(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Omitting the ceiling must not disable the cross-field clamp. + + With the shipped default ceiling the static ``TOOL_APPROVAL_TIMEOUT_MAX`` + binds first, so the cross-field clamp can only be SEEN to consult the + field default by lowering that default below the static max. + """ + from kiro_crew.config import loader as loader_mod + + monkeypatch.setattr(loader_mod, "_DEFAULT_CHAT_TURN_TIMEOUT_SECS", 1200) data = {"agent": {"tool_approval_timeout_secs": 7200}} _clamp_security_bounds(data) - assert data["agent"]["tool_approval_timeout_secs"] == 7200 - APPROVAL_TURN_MARGIN_SECS + assert data["agent"]["tool_approval_timeout_secs"] == 1200 - APPROVAL_TURN_MARGIN_SECS def test_static_bounds_applied_first(self) -> None: """The generic range clamp still runs on this field.""" @@ -338,8 +346,18 @@ def test_static_bounds_applied_first(self) -> None: data = {"agent": {"tool_approval_timeout_secs": TOOL_APPROVAL_TIMEOUT_MAX * 10}} _clamp_security_bounds(data) - # Static ceiling first, then the cross-field margin under the default - # turn ceiling (both 7200, so the margin binds). + # Static ceiling first. The default turn ceiling sits above the static + # max by more than the margin, so the cross-field clamp leaves the + # statically-clamped value alone. + assert data["agent"]["tool_approval_timeout_secs"] == TOOL_APPROVAL_TIMEOUT_MAX + # With a ceiling INSIDE the static max the cross-field margin binds after it. + data = { + "agent": { + "tool_approval_timeout_secs": TOOL_APPROVAL_TIMEOUT_MAX * 10, + "chat_turn_timeout_secs": TOOL_APPROVAL_TIMEOUT_MAX, + } + } + _clamp_security_bounds(data) assert ( data["agent"]["tool_approval_timeout_secs"] == TOOL_APPROVAL_TIMEOUT_MAX - APPROVAL_TURN_MARGIN_SECS @@ -371,10 +389,17 @@ def test_bool_window_is_left_to_dataclass_coercion(self) -> None: _clamp_security_bounds(data) assert data["agent"]["tool_approval_timeout_secs"] is True - def test_non_int_ceiling_falls_back_to_the_default(self) -> None: + def test_non_int_ceiling_falls_back_to_the_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from kiro_crew.config import loader as loader_mod + + # Lowered below the static max so the fallback is observable (see + # test_absent_ceiling_uses_the_field_default). + monkeypatch.setattr(loader_mod, "_DEFAULT_CHAT_TURN_TIMEOUT_SECS", 1200) data = {"agent": {"tool_approval_timeout_secs": 7200, "chat_turn_timeout_secs": "lots"}} _clamp_security_bounds(data) - assert data["agent"]["tool_approval_timeout_secs"] == 7200 - APPROVAL_TURN_MARGIN_SECS + assert data["agent"]["tool_approval_timeout_secs"] == 1200 - APPROVAL_TURN_MARGIN_SECS class TestArmTimeBudget: diff --git a/test/test_loop_stall_surfacing.py b/test/test_loop_stall_surfacing.py index 6b98d708480..85f0cb8477b 100644 --- a/test/test_loop_stall_surfacing.py +++ b/test/test_loop_stall_surfacing.py @@ -226,8 +226,24 @@ def test_out_of_range_values_are_clamped(self) -> None: class TestChatTurnCeilingConfig: - def test_default_preserves_existing_behaviour(self) -> None: - assert KiroCrewConfig().agent.chat_turn_timeout_secs == 7200 + def test_default_preserves_existing_behaviour(self, tmp_path, monkeypatch) -> None: + """A config that omits the key must load the dataclass default. + + The loader carries its own fallback literal for the key; a drift between + it and ``AgentConfig`` makes an on-disk config with the key omitted + behave differently from ``KiroCrewConfig()``. + """ + from kiro_crew.config.loader import AgentConfig, _invalidate_config_cache + + (tmp_path / "config.json").write_text(json.dumps({"agent": {}}), encoding="utf-8") + monkeypatch.setattr("kiro_crew.config.loader.config_dir", lambda: tmp_path) + _invalidate_config_cache() + try: + loaded = KiroCrewConfig.load().agent.chat_turn_timeout_secs + finally: + _invalidate_config_cache() + assert loaded == AgentConfig().chat_turn_timeout_secs + assert KiroCrewConfig().agent.chat_turn_timeout_secs == AgentConfig().chat_turn_timeout_secs def test_out_of_range_values_are_clamped(self) -> None: from kiro_crew.config.loader import CHAT_TURN_TIMEOUT_MAX, CHAT_TURN_TIMEOUT_MIN