Skip to content

Commit 1e98b54

Browse files
joeywangzroz-agent
andcommitted
Add checkpoint upload/commit pipeline mechanics
Adds the storage mechanics for one periodic-handoff checkpoint attempt (REMOTE-2111), as a self-contained module the (forthcoming, stacked) periodic checkpoint coordinator will drive: - harness_support.rs: SnapshotUploadMode (legacy|checkpoint) and generation on SnapshotUploadRequest; CheckpointGeneration (a validated, client-minted generation identifier); CommitSnapshotRequest/ Response; HarnessSupportClient::commit_snapshot. - snapshot.rs: CheckpointResult (Committed/Skipped/Failed outcome of one attempt); PipelineMode selects legacy vs. checkpoint-mode uploads; mint_generation (per-attempt, collision-free generation IDs); storage_name (generation-prefixed object naming); and the run_checkpoint_from_declarations_file / run_checkpoint_pipeline entry points: gather, upload every blob plus the manifest in checkpoint mode, then commit the exact object set that landed. Commit is withheld entirely (never partial) if the manifest or any required blob fails to upload, or if upload-target allocation fails. The whole pipeline has no production caller yet -- the coordinator that drives it periodically lands in a follow-up, stacked PR -- so the new items are marked #[allow(dead_code)] with a comment explaining why; each annotation is removable once that PR merges on top of this one. Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent 620e8f3 commit 1e98b54

3 files changed

Lines changed: 806 additions & 11 deletions

File tree

app/src/ai/agent_sdk/driver/snapshot.rs

Lines changed: 214 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ use crate::server::server_api::ai::{
4848
UploadLocalHandoffSnapshotRequest,
4949
};
5050
use crate::server::server_api::harness_support::{
51-
HarnessSupportClient, SnapshotFileInfo, SnapshotUploadRequest, UploadTarget, upload_to_target,
51+
CheckpointGeneration, CommitSnapshotRequest, HarnessSupportClient, SnapshotFileInfo,
52+
SnapshotUploadRequest, UploadTarget, upload_to_target,
5253
};
5354

5455
/// Default path of the declarations file when neither the env var override nor a task ID
@@ -210,8 +211,9 @@ pub(super) async fn run_declarations_script(
210211
///
211212
/// Reads `$OZ_SNAPSHOT_DECLARATIONS_FILE` for the operator/test override, then delegates to
212213
/// [`resolve_declarations_path_with_override`] so tests can exercise the pure logic without
213-
/// racing on the shared env var.
214-
fn resolve_declarations_path(task_id: Option<&AmbientAgentTaskId>) -> PathBuf {
214+
/// racing on the shared env var. `pub(super)` so `checkpoint_coordinator` can resolve the same
215+
/// path used by the declarations writer and by [`run_declarations_script`].
216+
pub(super) fn resolve_declarations_path(task_id: Option<&AmbientAgentTaskId>) -> PathBuf {
215217
resolve_declarations_path_with_override(task_id, std::env::var_os(DECLARATIONS_PATH_ENV_VAR))
216218
}
217219

@@ -651,6 +653,87 @@ struct SnapshotOutcome {
651653
manifest_uploaded: bool,
652654
}
653655

656+
/// Outcome of one checkpoint attempt, as opposed to [`SnapshotOutcome`] which only tracks
657+
/// per-entry upload results within a single attempt.
658+
// The whole checkpoint pipeline below (through `run_checkpoint_pipeline`) has no
659+
// production caller yet -- the periodic checkpoint coordinator that drives it lands
660+
// in a follow-up, stacked PR. `#[allow(dead_code)]` is temporary and should be
661+
// removable once that PR is merged on top of this one.
662+
#[allow(dead_code)]
663+
#[derive(Debug)]
664+
pub(super) enum CheckpointResult {
665+
/// Every required object (blobs plus manifest) for `generation` uploaded successfully
666+
/// and the exact-set commit call succeeded; `generation` is now the server's selected
667+
/// checkpoint.
668+
Committed { generation: CheckpointGeneration },
669+
/// There were no usable declarations to checkpoint (declarations file missing, empty,
670+
/// or containing no valid entries). No generation was minted and no network calls
671+
/// beyond reading local state were made.
672+
Skipped,
673+
/// A required upload (a non-cap-skipped blob, or the manifest), the upload-target
674+
/// allocation, or the commit call itself failed. `generation` is `None` only when the
675+
/// attempt was cut off before a generation was even minted (e.g. an external timeout
676+
/// wrapping the whole attempt). Any minted generation's objects (if uploaded) are left
677+
/// as uncommitted debris in storage; the server's existing marker (if any) is untouched.
678+
Failed {
679+
generation: Option<CheckpointGeneration>,
680+
reason: String,
681+
},
682+
}
683+
684+
/// Selects which upload-accounting path the shared gather/upload pipeline uses for a given
685+
/// attempt. See `SnapshotUploadMode` (`crate::server::server_api::harness_support`) for the
686+
/// server-side semantics.
687+
enum PipelineMode {
688+
/// One-shot end-of-run upload: unprefixed object names, counted against the
689+
/// execution's cumulative lifetime attachment quota.
690+
Legacy,
691+
/// Periodic or finalization checkpoint attempt: the server stores each requested file
692+
/// as `checkpoint_<generation>__<filename>` and does not charge the cumulative quota.
693+
// Not constructed until the coordinator PR (see the allow(dead_code) note above).
694+
#[allow(dead_code)]
695+
Checkpoint(CheckpointGeneration),
696+
}
697+
698+
/// Monotonic disambiguator for [`mint_generation`] so two attempts minted within the same
699+
/// millisecond (e.g. in tests, or on a very fast retry) never collide.
700+
#[allow(dead_code)]
701+
static GENERATION_COUNTER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
702+
703+
/// Mint a new checkpoint generation identifier.
704+
///
705+
/// Must be called exactly once per checkpoint *attempt*, and only after that attempt's
706+
/// payload has been gathered ("frozen") — retrying the same already-gathered payload (e.g.
707+
/// after a transient upload failure) must reuse the previously minted generation rather than
708+
/// calling this again; any newly gathered payload always mints a fresh one. Enforcing that
709+
/// distinction is the caller's responsibility (see the coordinator in
710+
/// `checkpoint_coordinator.rs`).
711+
///
712+
/// Format: `<millis-since-epoch>-<counter>`. This satisfies the server's
713+
/// `[A-Za-z0-9._-]{1,128}` charset and never contains the reserved `__` separator.
714+
#[allow(dead_code)]
715+
pub(super) fn mint_generation() -> CheckpointGeneration {
716+
let millis = std::time::SystemTime::now()
717+
.duration_since(std::time::UNIX_EPOCH)
718+
.unwrap_or_default()
719+
.as_millis();
720+
let counter = GENERATION_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
721+
CheckpointGeneration::from_validated(format!("{millis}-{counter}"))
722+
}
723+
724+
/// Compute the generation-prefixed storage object name for a logical filename (a blob or the
725+
/// manifest), matching the server's `checkpoint_<generation>__<logical_name>` convention.
726+
///
727+
/// Used only when assembling the exact-set [`CommitSnapshotRequest`] after upload — the
728+
/// *logical* name (produced by [`unique_filename`]) is what flows through gather, manifest
729+
/// building, and the upload-targets request; the server itself derives the storage name for
730+
/// each presigned upload target from that logical filename plus the request's `generation`
731+
/// field, so no client-side renaming is needed before that point.
732+
#[allow(dead_code)]
733+
fn storage_name(generation: &CheckpointGeneration, logical: &str) -> String {
734+
format!("checkpoint_{}__{logical}", generation.as_str())
735+
}
736+
654737
// --- Manifest schema ---
655738

656739
#[derive(serde::Serialize)]
@@ -887,6 +970,7 @@ async fn run_pipeline(
887970

888971
upload_gathered_snapshot(
889972
client,
973+
&PipelineMode::Legacy,
890974
manifest_filename,
891975
upload_files,
892976
repos,
@@ -896,6 +980,125 @@ async fn run_pipeline(
896980
.await
897981
}
898982

983+
/// Run one checkpoint attempt from the declarations file at `path`: read declarations, gather
984+
/// the payload, mint a generation for it, upload every blob plus the manifest in checkpoint
985+
/// mode, and commit the exact set that landed. Never panics; all failure modes are reported
986+
/// via the returned [`CheckpointResult`] (and, for unexpected failures, `report_error!`).
987+
///
988+
/// Unlike [`upload_snapshot_from_declarations_file`], a missing/empty/unusable declarations
989+
/// file is reported as [`CheckpointResult::Skipped`] rather than `None`, since the coordinator
990+
/// needs to distinguish "nothing to do" from "tried and failed" to drive its state machine.
991+
#[allow(dead_code)]
992+
pub(super) async fn run_checkpoint_from_declarations_file(
993+
path: &Path,
994+
client: Arc<dyn HarnessSupportClient>,
995+
) -> CheckpointResult {
996+
log::info!("Checkpoint attempt starting from {}", path.display());
997+
let Some(declarations) = read_and_parse_declarations(path) else {
998+
return CheckpointResult::Skipped;
999+
};
1000+
let declarations = drop_files_covered_by_repos(declarations);
1001+
if declarations.is_empty() {
1002+
log::info!("Checkpoint declarations empty after de-duplication; skipping attempt");
1003+
return CheckpointResult::Skipped;
1004+
}
1005+
let gathered = gather_snapshot_entries(declarations).await;
1006+
// The generation is minted here, once the gathered payload (blob contents, manifest
1007+
// stubs) is frozen for this attempt — see `mint_generation`'s contract.
1008+
let generation = mint_generation();
1009+
run_checkpoint_pipeline(client, generation, gathered).await
1010+
}
1011+
1012+
/// Upload and commit an already-gathered payload under `generation`. Split out from
1013+
/// [`run_checkpoint_from_declarations_file`] so a caller retrying the exact same attempt (as
1014+
/// opposed to gathering fresh) can reuse both the payload and the generation.
1015+
#[allow(dead_code)]
1016+
async fn run_checkpoint_pipeline(
1017+
client: Arc<dyn HarnessSupportClient>,
1018+
generation: CheckpointGeneration,
1019+
gathered: GatheredSnapshot,
1020+
) -> CheckpointResult {
1021+
let GatheredSnapshot {
1022+
manifest_filename,
1023+
upload_files,
1024+
repos,
1025+
files,
1026+
pre_upload_entries,
1027+
} = gathered;
1028+
1029+
let outcome = upload_gathered_snapshot(
1030+
client.clone(),
1031+
&PipelineMode::Checkpoint(generation.clone()),
1032+
manifest_filename.clone(),
1033+
upload_files,
1034+
repos,
1035+
files,
1036+
pre_upload_entries,
1037+
)
1038+
.await;
1039+
log::info!(
1040+
"Checkpoint attempt generation={generation} pending commit",
1041+
generation = generation.as_str()
1042+
);
1043+
let Some(outcome) = outcome else {
1044+
return CheckpointResult::Failed {
1045+
generation: Some(generation),
1046+
reason: "failed to allocate upload targets or serialize manifest".to_string(),
1047+
};
1048+
};
1049+
log_snapshot_outcome(&outcome);
1050+
1051+
if !outcome.manifest_uploaded {
1052+
return CheckpointResult::Failed {
1053+
generation: Some(generation),
1054+
reason: "manifest failed to upload".to_string(),
1055+
};
1056+
}
1057+
if outcome
1058+
.entries
1059+
.iter()
1060+
.any(|e| e.status == EntryStatus::Failed)
1061+
{
1062+
return CheckpointResult::Failed {
1063+
generation: Some(generation),
1064+
reason: "one or more required blobs failed to upload".to_string(),
1065+
};
1066+
}
1067+
1068+
// Exact-set commit: the manifest object plus every blob whose own upload actually
1069+
// succeeded (cap-skipped, gather-failed, and read-failed entries are never included,
1070+
// matching the server's exact-set contract).
1071+
let manifest_object = storage_name(&generation, &manifest_filename);
1072+
let mut objects: Vec<String> = outcome
1073+
.entries
1074+
.iter()
1075+
.filter(|e| e.status == EntryStatus::Uploaded && e.label != manifest_filename)
1076+
.map(|e| storage_name(&generation, &e.label))
1077+
.collect();
1078+
objects.push(manifest_object.clone());
1079+
1080+
let commit_request = CommitSnapshotRequest {
1081+
generation: generation.as_str().to_string(),
1082+
manifest_object,
1083+
objects,
1084+
};
1085+
match client.commit_snapshot(&commit_request).await {
1086+
Ok(response) => {
1087+
log::info!("Checkpoint committed: generation={}", response.generation);
1088+
CheckpointResult::Committed { generation }
1089+
}
1090+
Err(e) => {
1091+
let e = e.context("Failed to commit checkpoint snapshot");
1092+
let reason = format!("{e:#}");
1093+
report_error!(e);
1094+
CheckpointResult::Failed {
1095+
generation: Some(generation),
1096+
reason,
1097+
}
1098+
}
1099+
}
1100+
}
1101+
8991102
struct GatheredSnapshot {
9001103
manifest_filename: String,
9011104
upload_files: Vec<SnapshotUploadFile>,
@@ -954,6 +1157,7 @@ async fn gather_snapshot_entries(declarations: Vec<DeclarationEntry>) -> Gathere
9541157

9551158
async fn upload_gathered_snapshot(
9561159
client: Arc<dyn HarnessSupportClient>,
1160+
mode: &PipelineMode,
9571161
manifest_filename: String,
9581162
mut upload_files: Vec<SnapshotUploadFile>,
9591163
mut repos: Vec<RepoManifestEntry>,
@@ -990,12 +1194,13 @@ async fn upload_gathered_snapshot(
9901194

9911195
let mut target_map: HashMap<String, UploadTarget> = HashMap::new();
9921196
for chunk in file_infos.chunks(UPLOAD_BATCH_SIZE) {
993-
let targets = match client
994-
.get_snapshot_upload_targets(&SnapshotUploadRequest {
995-
files: chunk.to_vec(),
996-
})
997-
.await
998-
{
1197+
let request = match mode {
1198+
PipelineMode::Legacy => SnapshotUploadRequest::legacy(chunk.to_vec()),
1199+
PipelineMode::Checkpoint(generation) => {
1200+
SnapshotUploadRequest::checkpoint(generation.clone(), chunk.to_vec())
1201+
}
1202+
};
1203+
let targets = match client.get_snapshot_upload_targets(&request).await {
9991204
Ok(t) => t,
10001205
Err(e) => {
10011206
// Pipeline-abort: route through report_error! so Sentry captures the structured

0 commit comments

Comments
 (0)