Conversation
…atched work agentflare work's single fixed --timeout (30 min) killed jobs that were still making real progress, with no way to tell a genuinely stuck agent apart from one on a long but productive run (observed live in #398: an opencode run pushed 4 commits, then hung -- the timeout comment gave no hint work had happened). run_captured/run_headless now take two durations: an idle_timeout that kills only when stdout+stderr produce no new bytes for that long (the primary liveness signal), and a hard_cap absolute backstop against a runaway process. agentflare work's --timeout becomes that hard cap (default bumped 30min -> 6h, since it's no longer the primary judge), and a new --idle-timeout (default 5 min) does the real work: a task producing steady output can now run for hours, while a stalled one is caught in 5 minutes instead of the old 30. Failure messages report which one fired. agentflare run --print and cli_run_headless pass the same value for both params, preserving their existing flat-deadline behavior exactly since they're one-off invocations, not dispatched work.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHeadless execution now uses separate hard-cap and idle-output timeouts. Output readers track activity incrementally. Captured results identify idle termination. CLI defaults, call sites, and supervisor limits use the new timeout model. ChangesHeadless timeout handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant run_headless
participant run_captured
participant child_process
participant stdout_stderr_readers
run_headless->>run_captured: Pass hard cap and idle timeout
run_captured->>child_process: Start and drain process
child_process->>stdout_stderr_readers: Produce output chunks
stdout_stderr_readers->>run_captured: Report activity
run_captured->>child_process: Kill after idle timeout or hard cap
run_captured-->>run_headless: Return captured output and termination metadata
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…ew hard cap WORK_JOB_TIMEOUT_SECS wraps every daemon-dispatched agentflare work subprocess in agentflare-jobs::Supervisor's own fixed wall-clock kill -- a second, independent timeout layer on top of work's own --timeout/ --idle-timeout. It was sized against work's old fixed 1800s timeout (2100s = budget + margin) and was never updated when this same change raised work's own hard cap to 21600s (6h), primary judgment moved to --idle-timeout. Left unaligned, every job actually dispatched by the daemon -- the real "dispatched work" this feature targets -- would still get killed at 35 minutes regardless of how much progress it was making, silently reintroducing the exact bug the idle-timeout change exists to fix, just one layer out. Bumped to 6h + margin so the outer timeout can no longer fire before work's own inner timeout would.
…rap() Unrelated to this branch's actual change -- fixing it here because it's been intermittently failing build (windows-latest), a required status check, blocking this PR (and #402, based on it) from reaching green. Confirmed pre-existing on master's own HEAD, not something either PR introduced: a different one of these tests failed on each of three consecutive Windows CI reruns, always at the same remove_dir_all(&dir) cleanup line, after the test's real assertions had already passed. Classic Windows CI flake: a file just written/renamed can stay transiently locked for a few ms (real-time AV scanning) after the code under test is done with it, so an immediate remove_dir_all can hit PermissionDenied through no fault of the test's own logic. temp_dir_for_test's own pre-cleanup already treats this as best-effort (`let _ = ...`); these end-of-test cleanups asserted success instead, so the flake surfaced as a spurious test failure with nothing left to actually assert by that point in the test.
…nter The previous fix (best-effort cleanup) treated a symptom, not the cause: cargo nextest runs each test in its own process by default, so temp_dir_for_test's static AtomicU64 counter restarts at 0 in every process -- concurrent test processes on Windows CI were colliding on the literal same path (agentflare-coaching-rule-test-0), causing both write/write and write/delete races, not just an AV-scanning hiccup on cleanup. Confirmed by a second Windows CI failure after the first fix, on a *different* line (an initial std::fs::write, not the removal). tempfile::tempdir() (already used elsewhere in this crate, e.g. cli/work.rs's tests) is genuinely unique across processes and cleans up on drop, so this also drops the now-unnecessary manual best-effort remove_dir_all calls the previous commit added.
Summary
agentflare work's single fixed--timeout(default 30 min) killed jobs still making real progress — no way to tell a genuinely stuck agent from one on a long but productive run.run_captured/run_headlessnow take two durations:idle_timeout(kill only when stdout+stderr produce no new bytes for that long — the primary liveness signal) andhard_cap(absolute backstop against a runaway process).--timeoutbecomes the hard cap (default bumped 30min → 6h); new--idle-timeout(default 5 min) is the real signal. A task producing steady output can now run for hours; a stalled one is caught in 5 min instead of the old 30. Failure messages report which one fired.agentflare run --print/cli_run_headlesspass the same value for both params, preserving their existing flat-deadline behavior exactly (one-off invocations, not dispatched work).Closes item #20 (agentflare tracker) — interim idle-timeout direction chosen over the larger loopx-style turn/delivery-metadata rearchitecture, which needs restructuring
agentflare workaround discrete inspectable turns and is a bigger, separate lift.Test plan
cargo build -p agentflare— cleancargo test -p agentflare --bin agentflare agent_launch::— 22/22 pass, incl. 2 new tests: output-keeps-arriving survives the idle window; stall-after-real-output is caught by idle, not the hard capcargo test -p agentflare --bin agentflare -- agents:: cli::work:: cli::run::— 33/33 passcargo clippy -p agentflare --bin agentflare— no new warningsSummary by CodeRabbit
New Features
Bug Fixes