Skip to content

Commit 8b983b3

Browse files
authored
fix(core): self-heal a stale drive_file_id when an update hits a definitive 404 (#168)
An UPDATE against a drive_file_id whose Drive object is permanently gone returned a 404 that classified as DriveErrorClassification::Other -> the generic drive.unreachable: the op failed, the Activity view said "Drive unavailable", the dead id was KEPT, and every later cycle re-planned the same doomed update forever. Verified against a real incident today: 4 files looping drive.unreachable every cycle while neighboring uploads succeeded; their recorded ids return hard 404 from the Drive API. Changes: - driven-drive: export error_is_not_found (walks the error chain for the drive HTTP 404 marker) instead of duplicating string matching downstream. - executor: an update op whose failure is a definitive not-found now clears the stale drive_file_id (mirroring reconcile's R3-P1-2 path), drops the op, and surfaces a WARN drive.remote_file_missing outcome instead of an error - the next scan re-plans a fresh CREATE and the file self-heals. Create-path 404s (dest-folder) and the trash 404-is-ok rule are untouched; transient 5xx behavior unchanged (regression-tested). - types: new stable ErrorCode DriveRemoteFileMissing <-> drive.remote_file_missing (+ round-trip test). - orchestrator: records the warn-level activity row for the new outcome; does not fail the cycle (handled skip). - ui: en-US.json errors entry ("Remote copy missing") + activity label test. - fake store: update-not-found fault injection + executor tests proving id cleared, op dropped, warn row written, next cycle re-creates. Gates: cargo fmt/clippy -D warnings clean; cargo test -p driven-core -p driven-drive all green; vitest 489 passed; prettier clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JLB3E2Jm7knNJd37fVpH8X
1 parent 292e221 commit 8b983b3

11 files changed

Lines changed: 660 additions & 10 deletions

File tree

crates/driven-core/src/executor.rs

Lines changed: 432 additions & 3 deletions
Large diffs are not rendered by default.

crates/driven-core/src/orchestrator.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4981,6 +4981,47 @@ mod tests {
49814981
);
49824982
}
49834983

4984+
/// A self-healed stale `drive_file_id` must read truthfully in the activity
4985+
/// log: a WARN row keyed `drive.remote_file_missing` naming the file - NOT
4986+
/// an Error row, and NOT the `drive.unreachable` the 404 used to be reported
4987+
/// as. It must also NOT count as a failed op, since holding the source's
4988+
/// scan timestamps back would punish every other file for a condition the
4989+
/// executor already fixed.
4990+
#[test]
4991+
fn a_healed_missing_remote_copy_is_a_warn_row_and_not_a_failure() {
4992+
let src_id = crate::types::SourceId::new_v4();
4993+
let outcome = OpOutcome::Skipped {
4994+
relative_path: RelativePath::try_from("docs/report.pdf".to_string()).unwrap(),
4995+
reason: crate::executor::SkipReason::RemoteFileMissing,
4996+
};
4997+
4998+
let row = SyncOrchestrator::outcome_activity_row(src_id, 1_700_000_000_000, &outcome);
4999+
assert_eq!(
5000+
row.level,
5001+
ActivityLevel::Warn,
5002+
"a handled self-heal is a warning, not an error"
5003+
);
5004+
assert_eq!(row.event_type, "drive.remote_file_missing");
5005+
assert_eq!(row.message.as_deref(), Some("docs/report.pdf"));
5006+
assert_eq!(row.source_id, Some(src_id));
5007+
5008+
// Not a failed op: the cycle's timestamp-advance gate and the closing
5009+
// progress snapshot both leave it out.
5010+
assert!(!matches!(outcome, OpOutcome::Failed { .. }));
5011+
assert_eq!(
5012+
exec_progress_from(
5013+
&crate::types::PlanSummary {
5014+
uploads: 1,
5015+
..Default::default()
5016+
},
5017+
std::slice::from_ref(&outcome),
5018+
)
5019+
.errors,
5020+
0,
5021+
"a healed missing remote copy must not be counted as a cycle error"
5022+
);
5023+
}
5024+
49845025
#[tokio::test]
49855026
async fn per_op_activity_survives_a_mid_plan_stop() {
49865027
// R2-P2-1: activity is streamed PER OP (persisted immediately after each

crates/driven-core/src/types.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,15 @@ pub enum ErrorCode {
10271027
/// `drive.unreachable` - Drive API down, unreachable, or 5xx
10281028
/// circuit-open.
10291029
DriveUnreachable,
1030+
/// `drive.remote_file_missing` - the backed-up copy this file's recorded
1031+
/// `drive_file_id` points at no longer exists on Drive (the user deleted
1032+
/// and purged it out-of-band), so the planned UPDATE returned a definitive
1033+
/// 404. NOT an error the user must act on: the executor clears the stale id
1034+
/// and the next scan re-uploads the file as a fresh create, so this is a
1035+
/// warn-level, self-healing outcome rather than a failure. Distinct from
1036+
/// [`Self::DriveUnreachable`], which a 404 used to be reported as - and
1037+
/// which is retried forever against an id that can never come back.
1038+
DriveRemoteFileMissing,
10301039
/// `drive.resumable_session_invalid` - 4xx during resumable upload;
10311040
/// caller must restart the session.
10321041
DriveResumableSessionInvalid,
@@ -1148,6 +1157,7 @@ impl ErrorCode {
11481157
ErrorCode::DriveUploadSizeLimit => "drive.upload_size_limit",
11491158
ErrorCode::DriveChecksumMismatch => "drive.checksum_mismatch",
11501159
ErrorCode::DriveUnreachable => "drive.unreachable",
1160+
ErrorCode::DriveRemoteFileMissing => "drive.remote_file_missing",
11511161
ErrorCode::DriveResumableSessionInvalid => "drive.resumable_session_invalid",
11521162
ErrorCode::DriveDestFolderMissing => "drive.dest_folder_missing",
11531163
ErrorCode::DriveDestFolderPermissionDenied => "drive.dest_folder_permission_denied",
@@ -1202,6 +1212,7 @@ impl ErrorCode {
12021212
"drive.upload_size_limit" => ErrorCode::DriveUploadSizeLimit,
12031213
"drive.checksum_mismatch" => ErrorCode::DriveChecksumMismatch,
12041214
"drive.unreachable" => ErrorCode::DriveUnreachable,
1215+
"drive.remote_file_missing" => ErrorCode::DriveRemoteFileMissing,
12051216
"drive.resumable_session_invalid" => ErrorCode::DriveResumableSessionInvalid,
12061217
"drive.dest_folder_missing" => ErrorCode::DriveDestFolderMissing,
12071218
"drive.dest_folder_permission_denied" => ErrorCode::DriveDestFolderPermissionDenied,
@@ -1374,6 +1385,35 @@ mod tests {
13741385
assert_eq!(rp.as_str(), "a/b.txt");
13751386
}
13761387

1388+
// --- ErrorCode (SPEC s24 stable codes) ----------------------------------
1389+
1390+
/// `drive.remote_file_missing` (the self-healing stale-`drive_file_id`
1391+
/// outcome) must round-trip BOTH ways: `code()` is the persisted
1392+
/// `activity_log.event_type` and the UI's i18n key, and `from_code()` is
1393+
/// how a stored row is read back. A mapping added in only one direction
1394+
/// would silently degrade an existing row to "unknown code".
1395+
#[test]
1396+
fn drive_remote_file_missing_round_trips() {
1397+
assert_eq!(
1398+
ErrorCode::DriveRemoteFileMissing.code(),
1399+
"drive.remote_file_missing"
1400+
);
1401+
assert_eq!(
1402+
ErrorCode::from_code("drive.remote_file_missing"),
1403+
Some(ErrorCode::DriveRemoteFileMissing)
1404+
);
1405+
// ...and it is its OWN code, not folded into drive.unreachable (the
1406+
// bug this code exists to fix reported a definitive 404 as unreachable).
1407+
assert_ne!(
1408+
ErrorCode::DriveRemoteFileMissing.code(),
1409+
ErrorCode::DriveUnreachable.code()
1410+
);
1411+
assert_eq!(
1412+
ErrorCode::DriveRemoteFileMissing.to_string(),
1413+
"drive.remote_file_missing"
1414+
);
1415+
}
1416+
13771417
// --- ScheduleConfig (V2 schedule windows) -------------------------------
13781418

13791419
/// Monday 2024-01-01 00:00:00 UTC, in epoch ms. The dow formula resolves

crates/driven-drive/src/fake/fault_injection.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
//! - Transient faults (rate-limit, 5xx, network-drop) reset to "never
2323
//! trip" once they fire; the next request after the trip succeeds.
2424
//! - "Stay-broken" faults (auth.invalid_grant, dest-folder missing /
25-
//! readonly, trashed-visible-in-find) latch on first trigger and
26-
//! remain set for the lifetime of the store.
25+
//! readonly, update-target-not-found, trashed-visible-in-find) latch on
26+
//! first trigger and remain set for the lifetime of the store.
2727
//! - `md5_mismatch_after` latches **on the affected entry**: when it
2828
//! trips during a write, the entry's wrong md5 is stamped onto the
2929
//! entry so every subsequent read of that entry (metadata,
@@ -175,6 +175,24 @@ impl InMemoryRemoteStore {
175175
self
176176
}
177177

178+
/// Latches "the object an UPDATE targets is gone": every `update()` and
179+
/// every resumable session opened as [`crate::remote_store::ResumableKind::Update`]
180+
/// fails with a real-shaped classified Drive 404, modelling a
181+
/// `drive_file_id` whose object the user deleted (and purged) out-of-band.
182+
///
183+
/// CREATES keep working, which is the point: the executor must clear the
184+
/// stale id and re-upload the same path as a fresh create on the next
185+
/// cycle (`drive.remote_file_missing`) instead of retrying a doomed update
186+
/// forever. Read-only calls are unaffected.
187+
///
188+
/// Latches for the lifetime of the store (a deleted Drive object does not
189+
/// come back), so a test that wants the re-upload to succeed simply lets
190+
/// the next cycle take the create path.
191+
pub fn with_update_not_found(self) -> Self {
192+
self.faults.update_not_found.store(true, Ordering::Release);
193+
self
194+
}
195+
178196
/// Latches the destination-folder-readonly state. Every subsequent
179197
/// write-target request returns `drive.dest_folder_permission_denied`
180198
/// (SPEC s24). Read-only calls keep working - mirrors the user

crates/driven-drive/src/fake/mod.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,14 @@ pub(crate) struct Faults {
524524
pub(crate) response_delay_nanos: AtomicU64,
525525
/// Latched once tripped (auth.invalid_grant is "stay-broken").
526526
pub(crate) invalid_grant_latched: std::sync::atomic::AtomicBool,
527+
/// When true, every `update()` (and every resumable session opened with
528+
/// [`ResumableKind::Update`]) fails with a REAL-SHAPED Drive 404 - the
529+
/// classified error a `GoogleDriveStore` produces when the recorded
530+
/// `drive_file_id` no longer exists (the user deleted or purged the object
531+
/// out-of-band). Creates are unaffected, so a self-healing executor can
532+
/// re-upload the same path on the next cycle. Latches for the lifetime of
533+
/// the store; set by [`fault_injection::with_update_not_found`].
534+
pub(crate) update_not_found: std::sync::atomic::AtomicBool,
527535
/// Dest-folder states are latched on construction by the builder.
528536
pub(crate) dest_folder_missing: std::sync::atomic::AtomicBool,
529537
pub(crate) dest_folder_readonly: std::sync::atomic::AtomicBool,
@@ -569,6 +577,7 @@ impl Default for Faults {
569577
daily_quota_latched: AtomicBool::new(false),
570578
response_delay_nanos: AtomicU64::new(0),
571579
invalid_grant_latched: AtomicBool::new(false),
580+
update_not_found: AtomicBool::new(false),
572581
dest_folder_missing: AtomicBool::new(false),
573582
dest_folder_readonly: AtomicBool::new(false),
574583
trashed_visible_in_find: AtomicBool::new(false),
@@ -840,6 +849,25 @@ enum RequestKind {
840849
WriteTarget,
841850
}
842851

852+
/// The error a real `GoogleDriveStore` produces when a request names a Drive
853+
/// object that no longer exists: a CLASSIFIED [`crate::google::DriveError`]
854+
/// built from a genuine Drive 404 body.
855+
///
856+
/// Built through [`crate::google::DriveError::from_response`] (not a plain
857+
/// `anyhow::anyhow!` string) precisely so the fake reproduces the real shape
858+
/// end-to-end: the executor's typed downcast classifies it
859+
/// [`crate::remote_store::DriveErrorClassification::Other`] AND
860+
/// [`crate::google::error_is_not_found`] finds `drive HTTP 404` in the source
861+
/// chain. A stringly fake message would satisfy neither.
862+
fn remote_file_missing_error(file_id: &str) -> anyhow::Error {
863+
let body = format!(r#"{{"error":{{"code":404,"message":"File not found: {file_id}."}}}}"#);
864+
anyhow::Error::new(crate::google::DriveError::from_response(
865+
404,
866+
body.as_bytes(),
867+
None,
868+
))
869+
}
870+
843871
/// Atomically decrement `counter` if it is non-zero and not `u64::MAX`.
844872
/// Returns `true` iff the decrement crossed from 1 to 0 (the "trip"
845873
/// edge). `u64::MAX` means "never trip" and is left alone.
@@ -1022,6 +1050,9 @@ impl RemoteStore for InMemoryRemoteStore {
10221050
app_properties_patch: HashMap<String, String>,
10231051
) -> anyhow::Result<RemoteEntry> {
10241052
self.check_request_faults(RequestKind::WriteTarget).await?;
1053+
if self.faults.update_not_found.load(Ordering::Acquire) {
1054+
return Err(remote_file_missing_error(file_id));
1055+
}
10251056
let body = collect_body(body, self.oracle_on()).await?;
10261057

10271058
let new_len = body.len();
@@ -1076,6 +1107,12 @@ impl RemoteStore for InMemoryRemoteStore {
10761107
guard.ensure_folder_parent(parent_id)?;
10771108
}
10781109
ResumableKind::Update { file_id } => {
1110+
// The update-target-missing fault applies here too: the real
1111+
// Drive validates the file id when the session is opened, so a
1112+
// gone object 404s before a single byte is pushed.
1113+
if self.faults.update_not_found.load(Ordering::Acquire) {
1114+
return Err(remote_file_missing_error(file_id));
1115+
}
10791116
guard.ensure_object(file_id)?;
10801117
}
10811118
}

crates/driven-drive/src/google/mod.rs

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ impl std::error::Error for DriveError {
240240
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
241241
match self {
242242
// The `anyhow::Error` source is surfaced as the error chain so the
243-
// `is_not_found` helper (and any caller) can walk to the
243+
// `error_is_not_found` helper (and any caller) can walk to the
244244
// `drive HTTP <status>` cause. `anyhow::Error` impls
245245
// `AsRef<dyn Error + Send + Sync>`; coerce off the auto traits to
246246
// the `source()` return type.
@@ -1494,7 +1494,7 @@ impl RemoteStore for GoogleDriveStore {
14941494
Ok(_) => Ok(()),
14951495
Err(e) => {
14961496
// 404 -> already gone, treated as success (SPEC s3 `trash`).
1497-
if is_not_found(&e) {
1497+
if error_is_not_found(&e) {
14981498
Ok(())
14991499
} else {
15001500
Err(e)
@@ -1522,7 +1522,7 @@ impl RemoteStore for GoogleDriveStore {
15221522
match result {
15231523
Ok(_) => Ok(()),
15241524
Err(e) => {
1525-
if is_not_found(&e) {
1525+
if error_is_not_found(&e) {
15261526
Ok(())
15271527
} else {
15281528
Err(e)
@@ -1758,8 +1758,26 @@ pub(crate) fn parse_retry_after(resp: &reqwest::Response) -> Option<u64> {
17581758
.map(|secs| secs.saturating_mul(1000))
17591759
}
17601760

1761-
/// Whether an error is a Drive 404 (used to make `trash` idempotent).
1762-
fn is_not_found(err: &anyhow::Error) -> bool {
1761+
/// Whether `err` is a DEFINITIVE Drive "not found" (HTTP 404): the object the
1762+
/// request named does not exist (deleted out-of-band, purged from the trash, or
1763+
/// never ours).
1764+
///
1765+
/// The generic [`DriveErrorClassification`] of a 404 is
1766+
/// [`DriveErrorClassification::Other`], which renders as
1767+
/// `drive.unreachable: unclassified Drive error` - indistinguishable from any
1768+
/// other unclassified failure. Callers that must react to a MISSING object
1769+
/// specifically (the executor self-healing a stale `drive_file_id`, `trash`
1770+
/// treating a 404 as an idempotent no-op) use this instead of re-deriving the
1771+
/// classification: [`DriveError::from_response`] embeds the literal
1772+
/// `drive HTTP 404` in the source chain, and [`DriveError`]'s
1773+
/// `Error::source` exposes that chain, so walking it is the one reliable
1774+
/// signal available across both the typed and stringly paths.
1775+
///
1776+
/// Deliberately narrow: only a real 404 matches. A 403 / 5xx / transport
1777+
/// failure returns `false`, so a transient fault is never mistaken for a
1778+
/// permanently-gone object.
1779+
#[must_use]
1780+
pub fn error_is_not_found(err: &anyhow::Error) -> bool {
17631781
// `DriveError::from_response` embeds `drive HTTP 404` in the source chain.
17641782
err.chain()
17651783
.any(|c| c.to_string().contains("drive HTTP 404"))
@@ -1964,6 +1982,59 @@ pub(crate) fn clone_kind(kind: &ResumableKind) -> ResumableKind {
19641982
mod tests {
19651983
use super::*;
19661984

1985+
/// A 404 arriving as a CLASSIFIED Drive error (the shape every real
1986+
/// `GoogleDriveStore` call produces) must be detected by
1987+
/// [`error_is_not_found`], even though its classification is the generic
1988+
/// [`DriveErrorClassification::Other`] whose `Display` says
1989+
/// `drive.unreachable`. Callers (the executor's stale-`drive_file_id`
1990+
/// self-heal, `trash`'s idempotent no-op) depend on this distinction.
1991+
#[test]
1992+
fn error_is_not_found_detects_a_classified_404() {
1993+
let err = anyhow::Error::new(DriveError::from_response(
1994+
404,
1995+
br#"{"error":{"code":404,"message":"File not found: abc123."}}"#,
1996+
None,
1997+
));
1998+
assert!(
1999+
error_is_not_found(&err),
2000+
"a classified Drive 404 must read as not-found, got: {err:?}"
2001+
);
2002+
// The generic classification stays `Other` (this helper is the decision
2003+
// point, not the classification) - guard against a future rework
2004+
// silently changing that assumption.
2005+
assert_eq!(
2006+
classification_of(&err),
2007+
Some(DriveErrorClassification::Other),
2008+
"a 404 classifies as Other; only `error_is_not_found` distinguishes it"
2009+
);
2010+
}
2011+
2012+
/// Nothing else may masquerade as not-found: a transient 5xx, a 403, a
2013+
/// plain `anyhow` message, and a wrapped error whose chain has no
2014+
/// `drive HTTP 404` all return `false`, so a temporary fault is never
2015+
/// mistaken for a permanently-gone object.
2016+
#[test]
2017+
fn error_is_not_found_rejects_non_404_errors() {
2018+
let five_xx = anyhow::Error::new(DriveError::from_response(503, b"unavailable", None));
2019+
assert!(
2020+
!error_is_not_found(&five_xx),
2021+
"a transient 5xx must NOT read as not-found"
2022+
);
2023+
let forbidden = anyhow::Error::new(DriveError::from_response(403, b"forbidden", None));
2024+
assert!(
2025+
!error_is_not_found(&forbidden),
2026+
"a 403 must NOT read as not-found"
2027+
);
2028+
assert!(
2029+
!error_is_not_found(&anyhow::anyhow!("drive: something else went wrong")),
2030+
"an untyped Drive error must NOT read as not-found"
2031+
);
2032+
assert!(
2033+
!error_is_not_found(&anyhow::anyhow!("boom").context("while updating")),
2034+
"a plain wrapped error must NOT read as not-found"
2035+
);
2036+
}
2037+
19672038
#[test]
19682039
fn google_store_new_builds_a_stream_client_with_the_ca() {
19692040
// Issue #34: `GoogleDriveStore::new` derives its streaming client via

design/SPEC.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1773,6 +1773,7 @@ Stable codes (V1):
17731773
| `drive.upload_size_limit` | File exceeds Drive's per-file size limit |
17741774
| `drive.checksum_mismatch` | Verification failed after upload |
17751775
| `drive.unreachable` | Drive API down / unreachable / 5xx-circuit-open |
1776+
| `drive.remote_file_missing` | The backed-up copy a recorded `drive_file_id` points at is gone (definitive 404 on an update); the stale id is cleared and the file re-uploads on the next scan (warn, self-healing) |
17761777
| `drive.resumable_session_invalid` | 4xx during resumable upload — caller must restart session |
17771778
| `local.file_locked` | Couldn't open even with `FILE_SHARE_DELETE` (V1: locked file; VSS path failed too — see `local.vss_unavailable`) |
17781779
| `local.vss_unavailable` | Driven needs elevation to use VSS but isn't elevated |

design/STRESS_HARNESS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,7 @@ impl InMemoryRemoteStore {
420420
pub fn with_md5_mismatch_after(self, n: u64) -> Self;
421421
pub fn with_dest_folder_missing(self) -> Self;
422422
pub fn with_dest_folder_readonly(self) -> Self;
423+
pub fn with_update_not_found(self) -> Self;
423424
pub fn with_fileid_recycle(self) -> Self;
424425
}
425426
```

src-tauri/src/tray.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,10 @@ fn error_code_is_network(code: ErrorCode) -> bool {
527527
| ErrorCode::DriveResumableSessionInvalid
528528
| ErrorCode::DriveDestFolderMissing
529529
| ErrorCode::DriveDestFolderPermissionDenied
530+
// A missing remote copy is a self-healed, per-file warning (the next
531+
// scan re-uploads it), and it says nothing about reachability - Drive
532+
// answered, the object was simply gone. Non-network either way.
533+
| ErrorCode::DriveRemoteFileMissing
530534
// Local filesystem / VSS errors -> red error. (The transient
531535
// vss_helper_pending skip is not an error condition, but it is not a
532536
// network one either, so it classifies here as non-network.)

ui/src/__tests__/activity-event-label.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ describe("activityEventLabel (R1-P2-3)", () => {
4545
// those are localized via the shared error labels.
4646
expect(label("drive.checksum_mismatch")).toBe("Verification failed");
4747
expect(label("local.file_locked")).toBe("File in use");
48+
// The self-healing stale-drive_file_id skip: a warn row whose event type is
49+
// this code, so the Activity table must have a label for it rather than
50+
// showing the raw dotted string.
51+
expect(label("drive.remote_file_missing")).toBe("Remote copy missing");
4852
});
4953

5054
it("safely falls back to the raw code for an unknown event type", () => {

0 commit comments

Comments
 (0)