Skip to content

Commit 6735cd6

Browse files
joeywangzroz-agent
andcommitted
Make the coordinator's finalize reachable and its loop terminable
Review follow-ups on the periodic checkpoint coordinator. 1. Expose `finalize_budget` so the finalize floor is reachable. `finalize_with_new_attempt` gates on `remaining > script_timeout + upload_timeout`. The wiring PR passes `upload_timeout` alone as the budget, so that comparison is false for *every* configuration (defaults: 120s > 60s + 120s), and because the coordinator owns the whole end-of-run path once enabled, the result is no end-of-run snapshot at all. Add `finalize_budget(script_timeout, upload_timeout)` and document that callers must derive the budget from it, so `AgentDriver` cannot drift out of sync with the floor. Downgrade the skip log to WARN, since reaching it means the end-of-run checkpoint was dropped. Also give `finalize`'s outer ack timeout slack over the inner deadline. Bounding both on exactly `budget` meant the "defense-in-depth" bound would routinely preempt an attempt the coordinator was legitimately still finishing, rather than only catching a bug. 2. Stop the loop when the driver is gone. `is_safe_boundary` mapped `Err(ModelDropped)` to "safe", so a dropped `AgentDriver` left the coordinator gathering and uploading the whole workspace every interval forever. Nothing else stops the loop, and `run_snapshot_upload` returns without finalizing under exactly the same condition (spawner round trip fails, or `OzHandoff` is off). Replace the boolean predicate with a `Boundary` tri-state so `DriverGone` returns from `coordinator_loop` instead of being conflated with `Safe`. 3. Checkpoint blocked conversations, and cap boundary deferral. `Blocked` is backed by a pending action, so it fell through to `has_unfinished_actions_for_conversation` and reported busy forever: a run parked on user approval (often hours) polled every 2s and never checkpointed, though nothing was mutating the workspace. Treat `Blocked` and `is_done` as quiescent alongside `WaitingForEvents`. Add `MAX_BOUNDARY_DEFERRAL` (10m) so a long turn can no longer starve the feature entirely. A slightly-inconsistent checkpoint beats none, since the previous committed generation is only replaced on success. 4. Keep boundary polling responsive to finalize. The boundary check was awaited outside `select!`, so a stalled model task queue could wedge shutdown behind an unbounded await. `wait_for_safe_boundary` now races it against the finalize channel. 5. Flush the declarations writer before every attempt. `AgentDriver::run_snapshot_upload` flushes pending driver-side `file` appends before running the declarations script, precisely so no write is in flight when the bash script starts appending. The coordinator did not, so both periodic and final checkpoints could miss the agent's most recent edits and race the script on the shared append-only JSONL. Thread an optional `DeclarationsWriterHandle` through and flush it in `run_one_attempt`. 6. Correct `DEFAULT_CHECKPOINT_INTERVAL`'s doc: the cadence is measured from attempt completion, not attempt start. Tests: the derived budget clears the floor (and the old pairing provably cannot); finalize during boundary polling still runs an attempt; a gone driver stops the loop without attempting; the loop stops after finalize; and a failed attempt returns to Idle and retries on the next tick. Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent 0b6d126 commit 6735cd6

2 files changed

Lines changed: 467 additions & 54 deletions

File tree

0 commit comments

Comments
 (0)