Separate 404 from other errors - #60

Fixes #41.

Problem

GetResult and ListResult (and the other storage result types) collapse every failure into ok: false plus a message string. A 404 (object genuinely absent) is indistinguishable from a 403, 500, or network error without parsing the message. The sync engine must tell "remote file deleted" apart from "transient failure", otherwise it will propagate deletions on a flaky connection.

Changes

• Add ResultStatus type ("ok" | "not_found" | "auth" | "server" | "network")
  to all five storage result types: ConnectionResult, PutResult, GetResult,
  DeleteResult, ListResult
• Map HTTP status codes and fetch exceptions to the appropriate status in every
  S3 function (testConnection, s3PutObject, s3GetObject, s3DeleteObject,
  s3ListObjects)
• Extract XML parsing helpers (fieldFrom, decodeXmlText, parseListObjectsXml)
  into src/utils/storage/xml.ts
• Extract error mapping helpers (messageFor, statusForHttp) into
  src/utils/storage/errors.ts
• Update unit and integration tests to assert on the new status field

Why

The sync engine needs structural error classification to make safe decisions. With status, a caller can switch on the outcome:

• "ok" — use the result
• "not_found" — safe to treat as remote deletion
• "auth" — stop, credentials or config need fixing
• "server" / "network" — retry with backoff

The utility extraction keeps storage.ts focused on the S3 client interface and implementation while giving XML parsing and error mapping their own single-concern modules.

Greptile Summary

This PR adds a structured ResultStatus type ("ok" | "not_found" | "auth" | "server" | "network") to all five storage result types and maps HTTP status codes and fetch exceptions to the appropriate status in every S3 function, letting the sync engine distinguish a genuine remote deletion from a transient failure without parsing message strings. It also extracts XML parsing and error-mapping helpers into dedicated single-concern modules under src/utils/storage/.

• ResultStatus is threaded through ConnectionResult, PutResult, GetResult, DeleteResult, and ListResult, with every return site updated consistently.
• statusForHttp in errors.ts covers 404 → "not_found", 401/403 → "auth", and 500+ → "server", but the if (code >= 500) guard and the final fallback both return "server", leaving the guard as dead code.
• Unit and integration tests are updated to assert the new status field, including the "not_found" case on a missing key.

Confidence Score: 5/5

Safe to merge — the change is additive, every return site is updated consistently, and tests cover the key status values including not_found.

The core classification logic is correct and all five storage operations are updated. The only finding is dead code in statusForHttp where the if (code >= 500) guard returns the same value as the fallback, which does not affect runtime behaviour.

src/utils/storage/errors.ts — the if (code >= 500) guard is dead code worth cleaning up before the function grows.

Important Files Changed

_{Reviews (2): Last reviewed commit: "Update src/utils/storage/errors.ts" | Re-trigger Greptile}