Skip to content

Commit 077debb

Browse files
pmaxhoganclaude
andcommitted
fix(core): complete reconcile invalid_grant->needs_reauth + keep op on metadata error; fix(app): full quit-drain budget for run loop; fix(app): Backoff tray icon = NetworkAttention; docs(codex): M5 recheck cap reached
M5 recheck-2 (FINAL, cap=2) - all 3 findings fixed. R2-P1-1 (driven-core executor.rs/orchestrator.rs): recheck-1 only mapped invalid_grant -> ReconcileError::AuthInvalidGrant for the corrupt-trash retry. The other reconcile remote awaits (resumable resume, encrypted-parent ensure_folder, update metadata, create find_by_op_uuid, adopt) propagated plain anyhow Drive errors, so a revoked token during a NORMAL create/update reconcile was retried forever instead of marking needs_reauth. The update-metadata branch's catch-all `_ => delete_pending_op` also deleted the op on ANY metadata error, losing the reconcile handle for an op that may have committed. Added to_reconcile_err (classify_drive_error == InvalidGrant -> AuthInvalidGrant) wrapping every reconcile remote await; delete the pending op ONLY on a SUCCESSFUL metadata/lookup result proving the UUID absent/not-applicable - on error KEEP the op. Tests: reconcile_invalid_grant_on_{create_lookup,update_metadata}_maps_to_ needs_reauth + reconcile_metadata_transient_error_keeps_the_pending_op. R2-P2-1 (src-tauri app_state.rs/lib.rs): the run loop shared the 5s bridge drain budget, so a >5s in-flight upload was aborted on explicit Quit. Gave the run loop its own RUN_LOOP_DRAIN_TIMEOUT (full ~20s), drained first; the auxiliary tasks keep the short 5s budget; lib.rs outer guard derived from the run-loop budget + margin. Zero-orphan guarantee preserved. Test: run_loop_gets_full_drain_budget_not_the_short_bridge_timeout (tokio test-util). R2-P2-2 (src-tauri tray.rs): Backoff aggregated as network-attention (severity 3) but rendered the blue syncing icon + tooltip. Mapped Backoff -> TrayIcon::NetworkAttention + service-down tooltip (DESIGN s8.1 "Drive unreachable"). Test: backoff_is_network_attention. CODEX_NOTES: M5 recheck cap (2) reached, all recheck-2 findings fixed; accepted residuals = power-suspend/resume seam, flat-tile tray icons, elevation live-test gate-skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent a773668 commit 077debb

6 files changed

Lines changed: 563 additions & 48 deletions

File tree

crates/driven-core/src/executor.rs

Lines changed: 270 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,10 +2920,16 @@ impl Executor for DefaultExecutor {
29202920
// falls straight to adopt-or-requeue, which cleanly re-enqueues a
29212921
// fresh op the next cycle re-snapshots + re-uploads from scratch.
29222922
if let Some(resumable) = payload.resumable.clone() {
2923-
match self
2923+
// R2-P1-1: a revoked token during the resume's remote awaits
2924+
// must mark the account needs_reauth (NOT be retried forever as
2925+
// a transient reconcile failure). Map an invalid_grant-classified
2926+
// error to ReconcileError::AuthInvalidGrant so reconcile_once's
2927+
// enter_needs_reauth fires.
2928+
let resumed = self
29242929
.resume_persisted(source, &op, &payload, resumable, crypto)
2925-
.await?
2926-
{
2930+
.await
2931+
.map_err(to_reconcile_err)?;
2932+
match resumed {
29272933
Some((entry, resumed_blake3)) => {
29282934
// P1-2 trap: the streaming-crash payload carries no
29292935
// `uploaded_blake3_hex`, so adopt would otherwise hit
@@ -2935,8 +2941,11 @@ impl Executor for DefaultExecutor {
29352941
// the real hash, not a placeholder.
29362942
let mut adopted = payload.clone();
29372943
adopted.uploaded_blake3_hex = Some(hex::encode(resumed_blake3));
2944+
// adopt_reconciled re-derives the encrypted parent chain
2945+
// (remote ensure_folder) - map an invalid_grant there too.
29382946
self.adopt_reconciled(source, &op, &adopted, entry, crypto)
2939-
.await?;
2947+
.await
2948+
.map_err(to_reconcile_err)?;
29402949
continue;
29412950
}
29422951
None => {
@@ -2949,24 +2958,51 @@ impl Executor for DefaultExecutor {
29492958

29502959
if let Some(file_id) = payload.drive_file_id.clone() {
29512960
// Update path: compare the existing object's appProperties.
2961+
self.pacer.permit_request().await;
29522962
match self.remote.metadata(&file_id).await {
2963+
// R2-P1-1: a SUCCESSFUL metadata read decides the op's fate.
29532964
Ok(entry)
29542965
if entry
29552966
.app_properties
29562967
.get(CLIENT_OP_UUID_KEY)
29572968
.map(|v| v == &uuid)
29582969
.unwrap_or(false) =>
29592970
{
2971+
self.pacer.note_response(ResponseClass::Ok);
29602972
// Already committed remotely; re-hash + finish.
2973+
// adopt_reconciled re-derives the encrypted parent chain
2974+
// (remote ensure_folder) - map an invalid_grant there.
29612975
self.adopt_reconciled(source, &op, &payload, entry, crypto)
2962-
.await?;
2976+
.await
2977+
.map_err(to_reconcile_err)?;
29632978
}
2964-
_ => {
2965-
// Not committed; drop the stale op so the next scan
2966-
// re-enqueues it cleanly (the prior file_state row
2967-
// keeps the existing drive_file_id for the update).
2979+
Ok(_) => {
2980+
// R2-P1-1: the object EXISTS but does NOT carry this op's
2981+
// uuid - a SUCCESSFUL result that PROVES this update never
2982+
// committed. Only THEN drop the stale op so the next scan
2983+
// re-enqueues it cleanly (the prior file_state row keeps
2984+
// the existing drive_file_id for the update).
2985+
self.pacer.note_response(ResponseClass::Ok);
29682986
self.state.delete_pending_op(op.id).await?;
29692987
}
2988+
Err(e) => {
2989+
// R2-P1-1: a metadata ERROR proves NOTHING about whether
2990+
// the update committed - it may have (a transient/auth
2991+
// failure on the read). NEVER delete the op on an error
2992+
// (that would lose the reconcile handle for a possibly-
2993+
// committed op). KEEP the op and surface the error so the
2994+
// source retries next cycle; an invalid_grant maps to
2995+
// needs_reauth via reconcile_once.
2996+
let class = classify_drive_error(&e);
2997+
self.pacer.note_response(class.response_class());
2998+
warn!(
2999+
target: TARGET,
3000+
source = %source.id,
3001+
path = %op.relative_path,
3002+
"reconcile: metadata read failed; keeping the pending op for retry next cycle (R2-P1-1): {e}"
3003+
);
3004+
return Err(to_reconcile_err(e));
3005+
}
29703006
}
29713007
} else {
29723008
// Create path: find the orphaned object by op uuid under its
@@ -2976,15 +3012,42 @@ impl Executor for DefaultExecutor {
29763012
// duplicate on the next scan (P1-5 must not regress the
29773013
// Cluster-A no-duplicate contract). Re-derive the parent
29783014
// (idempotent `ensure_folder` for the encrypted dir chain).
3015+
//
3016+
// R2-P1-1: reconcile_parent_id + find_by_op_uuid + adopt each
3017+
// do remote awaits; map an invalid_grant to needs_reauth.
29793018
let parent_id = self
29803019
.reconcile_parent_id(source, &op.relative_path, crypto)
2981-
.await?;
2982-
match self.remote.find_by_op_uuid(&parent_id, &uuid).await? {
2983-
Some(entry) => {
2984-
self.adopt_reconciled(source, &op, &payload, entry, crypto)
2985-
.await?
3020+
.await
3021+
.map_err(to_reconcile_err)?;
3022+
self.pacer.permit_request().await;
3023+
let found = match self.remote.find_by_op_uuid(&parent_id, &uuid).await {
3024+
Ok(found) => {
3025+
self.pacer.note_response(ResponseClass::Ok);
3026+
found
3027+
}
3028+
Err(e) => {
3029+
// R2-P1-1: a lookup ERROR proves nothing about whether the
3030+
// create committed; KEEP the op (do not delete) and surface
3031+
// the error so it retries next cycle / maps to needs_reauth.
3032+
let class = classify_drive_error(&e);
3033+
self.pacer.note_response(class.response_class());
3034+
warn!(
3035+
target: TARGET,
3036+
source = %source.id,
3037+
path = %op.relative_path,
3038+
"reconcile: find_by_op_uuid failed; keeping the pending op for retry next cycle (R2-P1-1): {e}"
3039+
);
3040+
return Err(to_reconcile_err(e));
29863041
}
3042+
};
3043+
match found {
3044+
Some(entry) => self
3045+
.adopt_reconciled(source, &op, &payload, entry, crypto)
3046+
.await
3047+
.map_err(to_reconcile_err)?,
29873048
None => {
3049+
// R2-P1-1: a SUCCESSFUL lookup that found NO orphan proves
3050+
// the create never landed - only THEN drop the op.
29883051
self.state.delete_pending_op(op.id).await?;
29893052
}
29903053
}
@@ -4089,6 +4152,30 @@ pub fn reconcile_error_is_invalid_grant(err: &anyhow::Error) -> bool {
40894152
)
40904153
}
40914154

4155+
/// R2-P1-1: map a reconcile remote-await error so a revoked token surfaces as a
4156+
/// typed [`ReconcileError::AuthInvalidGrant`] the orchestrator acts on.
4157+
///
4158+
/// recheck-1 only converted `invalid_grant` for the corrupt-trash retry; EVERY
4159+
/// other reconcile remote await (resumable resume, encrypted-parent
4160+
/// `ensure_folder`, update `metadata`, create `find_by_op_uuid`, adopt) used to
4161+
/// propagate a plain `anyhow` Drive error, so a revoked token during a NORMAL
4162+
/// create/update reconcile was treated as a transient reconcile failure and
4163+
/// retried forever instead of marking the account `needs_reauth`. Wrapping each
4164+
/// such await with this helper routes `classify_drive_error(&err) ==
4165+
/// InvalidGrant` into `reconcile_once`'s `enter_needs_reauth` path; any other
4166+
/// error (transient Drive fault, DB hiccup) passes through unchanged and is
4167+
/// retried next cycle. An error that is ALREADY a `ReconcileError` (e.g. from a
4168+
/// nested helper) is passed through unchanged so the typed signal is preserved.
4169+
fn to_reconcile_err(err: anyhow::Error) -> anyhow::Error {
4170+
if err.downcast_ref::<ReconcileError>().is_some() {
4171+
return err;
4172+
}
4173+
if classify_drive_error(&err) == DriveError::InvalidGrant {
4174+
return ReconcileError::AuthInvalidGrant.into();
4175+
}
4176+
err
4177+
}
4178+
40924179
/// Extract the stranded corrupt-create file id from a Drive-side error, if it
40934180
/// carries one (codex C5-P1-4).
40944181
///
@@ -4750,6 +4837,175 @@ mod tests {
47504837
.is_none());
47514838
}
47524839

4840+
// --- R2-P1-1: invalid_grant during a NORMAL reconcile -> needs_reauth ----
4841+
4842+
/// R2-P1-1: a revoked token observed during the CREATE-path reconcile
4843+
/// lookup (`find_by_op_uuid`) - NOT the corrupt-trash retry - must surface
4844+
/// as a typed `ReconcileError::AuthInvalidGrant` so the orchestrator runs
4845+
/// the SAME needs-reauth transition (DESIGN s5.4). Before the fix this path
4846+
/// propagated a plain anyhow Drive error and was retried forever.
4847+
#[tokio::test]
4848+
async fn reconcile_invalid_grant_on_create_lookup_maps_to_needs_reauth() {
4849+
// First remote request (planting the orphan create) succeeds; the
4850+
// SECOND (reconcile's find_by_op_uuid) trips invalid_grant + latches.
4851+
let h = harness_with_remote(InMemoryRemoteStore::new().with_invalid_grant_after(1)).await;
4852+
let (rel, _size) = h.write_file("auth-create.txt", b"committed pre-revoke");
4853+
4854+
let op_uuid = uuid::Uuid::new_v4().to_string();
4855+
let mut app = HashMap::new();
4856+
app.insert(CLIENT_OP_UUID_KEY.to_string(), op_uuid.clone());
4857+
// Request #1: plant the orphaned create (token still valid).
4858+
h.remote
4859+
.create(
4860+
h.source.drive_folder_id.as_str(),
4861+
"auth-create.txt",
4862+
"application/octet-stream",
4863+
UploadBody::Bytes(Bytes::from_static(b"committed pre-revoke")),
4864+
app,
4865+
)
4866+
.await
4867+
.unwrap();
4868+
let now = h.clock.now_ms();
4869+
h.state
4870+
.enqueue_pending_op(NewPendingOp {
4871+
source_id: h.source.id,
4872+
op_type: OP_TYPE_UPLOAD.to_string(),
4873+
relative_path: rel.clone(),
4874+
payload_json: serde_json::json!({ "client_op_uuid": op_uuid, "drive_file_id": null }),
4875+
scheduled_for: now,
4876+
created_at: now,
4877+
})
4878+
.await
4879+
.unwrap();
4880+
4881+
let exec = h.executor();
4882+
let err = exec
4883+
.reconcile(&h.source)
4884+
.await
4885+
.expect_err("invalid_grant on the create lookup must fail the reconcile");
4886+
assert!(
4887+
reconcile_error_is_invalid_grant(&err),
4888+
"a revoked token during the normal create reconcile must surface ReconcileError::AuthInvalidGrant (drives needs_reauth + suspend), got: {err:?}"
4889+
);
4890+
// The op is KEPT (never dropped) so it retries once the user re-links.
4891+
assert_eq!(
4892+
h.state
4893+
.get_pending_ops_for_source(h.source.id)
4894+
.await
4895+
.unwrap()
4896+
.len(),
4897+
1,
4898+
"an auth failure must KEEP the pending op (no data-loss drop)"
4899+
);
4900+
}
4901+
4902+
/// R2-P1-1: a revoked token observed during the UPDATE-path reconcile
4903+
/// (`metadata`) must likewise map to `ReconcileError::AuthInvalidGrant`
4904+
/// (not be swallowed by the catch-all delete-op arm).
4905+
#[tokio::test]
4906+
async fn reconcile_invalid_grant_on_update_metadata_maps_to_needs_reauth() {
4907+
// The metadata read is the FIRST remote request on the update path, so
4908+
// trip invalid_grant on request #1.
4909+
let h = harness_with_remote(InMemoryRemoteStore::new().with_invalid_grant_after(0)).await;
4910+
let (rel, _size) = h.write_file("auth-update.txt", b"existing object");
4911+
4912+
let op_uuid = uuid::Uuid::new_v4().to_string();
4913+
let now = h.clock.now_ms();
4914+
h.state
4915+
.enqueue_pending_op(NewPendingOp {
4916+
source_id: h.source.id,
4917+
op_type: OP_TYPE_UPLOAD.to_string(),
4918+
relative_path: rel.clone(),
4919+
// drive_file_id present => the UPDATE reconcile path (metadata).
4920+
payload_json: serde_json::json!({
4921+
"client_op_uuid": op_uuid,
4922+
"drive_file_id": "some-existing-id",
4923+
}),
4924+
scheduled_for: now,
4925+
created_at: now,
4926+
})
4927+
.await
4928+
.unwrap();
4929+
4930+
let exec = h.executor();
4931+
let err = exec
4932+
.reconcile(&h.source)
4933+
.await
4934+
.expect_err("invalid_grant on the metadata read must fail the reconcile");
4935+
assert!(
4936+
reconcile_error_is_invalid_grant(&err),
4937+
"a revoked token during the normal update reconcile must surface ReconcileError::AuthInvalidGrant, got: {err:?}"
4938+
);
4939+
assert_eq!(
4940+
h.state
4941+
.get_pending_ops_for_source(h.source.id)
4942+
.await
4943+
.unwrap()
4944+
.len(),
4945+
1,
4946+
"an auth failure on metadata must KEEP the op, never delete it"
4947+
);
4948+
}
4949+
4950+
/// R2-P1-1 (data-safety): a NON-auth (transient) metadata ERROR proves
4951+
/// nothing about whether the update committed, so the reconcile must KEEP
4952+
/// the pending op (retry next cycle) rather than delete it via the old
4953+
/// catch-all `_ => delete_pending_op` arm - which would lose the reconcile
4954+
/// handle for an op that may have committed remotely.
4955+
#[tokio::test]
4956+
async fn reconcile_metadata_transient_error_keeps_the_pending_op() {
4957+
// First remote request (the metadata read) drops the network.
4958+
let h = harness_with_remote(InMemoryRemoteStore::new().with_network_drop_after(0)).await;
4959+
let (rel, _size) = h.write_file("keep-on-error.txt", b"maybe-committed");
4960+
4961+
let op_uuid = uuid::Uuid::new_v4().to_string();
4962+
let now = h.clock.now_ms();
4963+
h.state
4964+
.enqueue_pending_op(NewPendingOp {
4965+
source_id: h.source.id,
4966+
op_type: OP_TYPE_UPLOAD.to_string(),
4967+
relative_path: rel.clone(),
4968+
payload_json: serde_json::json!({
4969+
"client_op_uuid": op_uuid,
4970+
"drive_file_id": "maybe-committed-id",
4971+
}),
4972+
scheduled_for: now,
4973+
created_at: now,
4974+
})
4975+
.await
4976+
.unwrap();
4977+
4978+
let exec = h.executor();
4979+
let err = exec
4980+
.reconcile(&h.source)
4981+
.await
4982+
.expect_err("a transient metadata error must fail the reconcile (retry next cycle)");
4983+
// A plain transient error - NOT an auth one (so the orchestrator simply
4984+
// retries the source next cycle instead of marking needs_reauth).
4985+
assert!(
4986+
!reconcile_error_is_invalid_grant(&err),
4987+
"a transient metadata error must NOT be classified as invalid_grant, got: {err:?}"
4988+
);
4989+
// CRITICAL: the op survives - never dropped on a metadata error.
4990+
assert_eq!(
4991+
h.state
4992+
.get_pending_ops_for_source(h.source.id)
4993+
.await
4994+
.unwrap()
4995+
.len(),
4996+
1,
4997+
"a metadata ERROR must KEEP the pending op (an op that may have committed must not lose its reconcile handle)"
4998+
);
4999+
assert!(
5000+
h.state
5001+
.get_file_state(h.source.id, &rel)
5002+
.await
5003+
.unwrap()
5004+
.is_none(),
5005+
"no file_state row should be written on a failed metadata reconcile"
5006+
);
5007+
}
5008+
47535009
// --- P1-2: orphan whose local bytes CHANGED post-upload requeues --------
47545010

47555011
#[tokio::test]

0 commit comments

Comments
 (0)