REMOTE-2111 (3/3): Wire the periodic checkpoint coordinator into AgentDriver

[joeywangzr]

Part 3 of 3 in a stack splitting the periodic workspace-handoff checkpoint client work into reviewable chunks.

Stack

1. REMOTE-2111 (1/3): Add checkpoint upload/commit pipeline mechanics #14588 — checkpoint upload/commit mechanics (base: master)
2. REMOTE-2111 (2/3): Add periodic checkpoint coordinator state machine #14573 — periodic checkpoint coordinator state machine (base: REMOTE-2111 (1/3): Add checkpoint upload/commit pipeline mechanics #14588's branch)
3. This PR — wire the coordinator into AgentDriver (base: REMOTE-2111 (2/3): Add periodic checkpoint coordinator state machine #14573's branch)

Stacked on #14573; only the diff vs. that branch is new in this PR. Please review against #14573's branch, not master. This PR's tree, once the full stack is applied, is byte-identical to the original combined implementation for driver.rs, mod.rs, terminal/view.rs, and warp_features/lib.rs.

What this does

Instantiates and drives CheckpointCoordinatorHandle from AgentDriver's own lifecycle, completing REMOTE-2111 Phase 3:

• AgentDriverOptions gains checkpoint_interval (override for the default 5-minute-plus-jitter cadence; primarily for tests).
• AgentDriver spawns a coordinator when a cloud task_id is present, snapshot uploads aren't disabled, and both FeatureFlag::OzHandoff and the new FeatureFlag::PeriodicHandoffCheckpoints are enabled.
• Driver finalization routes through the coordinator's finalize() (bounded by the existing snapshot upload timeout) when a coordinator is active, instead of the legacy one-shot end-of-run snapshot path.
• warp_features: add PeriodicHandoffCheckpoints, off by default while the coordinator rolls out.

Validation

cargo clippy --all-targets --tests -- -D warnings, cargo fmt --check, and the full checkpoint + coordinator test suites (19 + 13 tests respectively) all pass at the top of the stack. All #[allow(dead_code)] annotations added in #14588/#14573 are now provably unnecessary (clippy passes clean without any new warnings appearing once this PR's wiring makes everything reachable), confirming the split didn't drop any functionality.

Co-Authored-By: Oz oz-agent@warp.dev

[joeywangzr]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• REMOTE-2111 (3/3): Wire the periodic checkpoint coordinator into AgentDriver #14589 👈 (View in Graphite)
• REMOTE-2111 (2/3): Add periodic checkpoint coordinator state machine #14573
• REMOTE-2111 (1/3): Add checkpoint upload/commit pipeline mechanics #14588
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[oz-for-oss]

@joeywangzr

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[oz-for-oss]

Overview

This PR wires the periodic handoff checkpoint coordinator into AgentDriver behind the existing Oz handoff gate, the new off-by-default PeriodicHandoffCheckpoints flag, cloud task presence, and snapshot enablement. When active, end-of-run snapshot handling delegates to the coordinator's bounded finalize() path; otherwise the legacy one-shot upload path remains unchanged.

Concerns

• No blocking correctness, security, or spec-alignment concerns found in the reviewed diff.
• spec_context.md reports no approved or repository spec context for this PR, so there were no concrete spec commitments to enforce.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

[joeywangzr]

Pushed a follow-up commit to this branch from a review pass (1461247), and rebased onto the updated coordinator branch. Summary:

Blocking — finalize(upload_timeout) could never run the final attempt.
The coordinator's floor is script_timeout + upload_timeout, so remaining > floor was false for every possible configuration (defaults: 120s > 60s + 120s). Because this path returns past the legacy one-shot upload, enabling PeriodicHandoffCheckpoints meant no end-of-run snapshot at all: runs shorter than one 5-minute interval uploaded nothing, and longer runs lost everything since the last periodic tick. That's a strict regression against today's behavior, on the rollout flag's happy path.

Now uses checkpoint_coordinator::finalize_budget(script_timeout, upload_timeout), added in #14573, which owns the floor so the two can't drift apart. Covered by a regression test that also asserts the old pairing provably can't clear the floor.

Blocking — the coordinator never got the declarations writer.
run_snapshot_upload flushes queued driver-side file appends before running the declarations script, precisely so nothing is in flight when the bash script starts appending to the same JSONL (see the comment on DeclarationsWriterHandle::flush). The coordinator branch returns before that block, so both periodic and final checkpoints could miss the agent's most recent edits. Now passes snapshot_file_writer.clone() through — the coordinator is gated on a superset of the writer's own conditions, so the handle is always present when the coordinator exists.

Removed the allow(dead_code) scaffolding.
Both #14588 and #14573 say their allows are "removable once that PR is merged on top of this one" — this is that PR, and it wasn't doing the removal. Dropped the module-wide #![allow(dead_code)] in checkpoint_coordinator (which would otherwise permanently blind the lint over new async/networking code) plus the per-item allows in snapshot and harness_support, along with the now-stale explanatory comments. Verified with the exact gate presubmit uses: cargo clippy -p warp --all-targets --tests -- -D warnings.

Note on CI: this PR and #14573 have only ever run the OWNERS/CLA checks, because their bases aren't master — the format/clippy/test matrix has never run against the coordinator or this wiring. Worth confirming green before merge. I ran cargo check, clippy -D warnings, cargo fmt --check and the 65 snapshot + coordinator unit tests locally on the stack tip; I could not run the full app test suite here because linking the test binary exceeds this sandbox's memory.

Two smaller things I left for you: checkpoint_interval is added to AgentDriverOptions but hard-wired to None at the only production construction site, so it's currently unreachable configuration — either plumb a CLI/env flag or drop the field. And the coordinator is spawned in AgentDriver::new, i.e. before environment setup, so the first tick can land mid-setup.