fix(uploads): treat a missing storage object as absent metadata, not a failure

[waleedlatif1]

Summary

Root cause of the chronic FilesServeAPI / FileAuthorization ERROR noise — 1,000–2,000 lines/day for at least 14 days, peaking at 2,105 on 07-29.

A workspace file is rewritten under a new key on every content update, and the superseded object is deleted immediately (cleanupWorkspaceStorageObject(oldKey, 'version replacement'), from #5991). Any reader still holding the previous key therefore finds nothing — which is an ordinary outcome, not a failure.

getFileMetadata let that not-found propagate out of all three provider branches. verifyWorkspaceFileAccess caught it in its generic handler and logged ERROR, so the branch already written for exactly this case — logger.warn('Workspace file missing authorization metadata') at authorization.ts:264 — was unreachable.

Changes

• getFileMetadata returns {} when the object is absent — the same value it already returns when no provider is configured, so no contract or caller changes
• Genuine failures (permission, network, provider 5xx) still propagate and still log ERROR
• Collapsed the three divergent per-provider not-found predicates (S3 NotFound/NoSuchKey, Azure BlobNotFound, GCS numeric 404) onto one shared isObjectNotFoundError, rather than adding a fourth copy

Verified against the AWS SDK v3 source: HeadObjectCommand throws NotFound and GetObjectCommand throws NoSuchKey, both __BaseException with $fault: 'client' and $metadata.httpStatusCode: 404 — matching the production payload exactly.

Not in scope

This corrects how absence is modelled. The underlying design issue — URLs addressing a mutable storage key rather than the stable file id — is a separate change; a stale key legitimately 404s.

Type of Change

• Bug fix

Testing

8 new tests. reports an absent object as no metadata rather than throwing verified red before the fix and green after. Full lib/uploads suite: 374 passing across 31 files.

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

---

Audit of every changed line

A defect in this PR's own predicate was found and fixed (a5b3…). The first version read the label as typeof name === 'string' ? name : code, so a non-matching name short-circuited the code check entirely. Azure raises a RestError whose name carries the class and whose code carries the reason, so BlobNotFound was missed — narrower than the per-provider check it replaced. name and code are now tested independently, with a regression test verified red before / green after.

All three callers audited for fail-closed behavior. getFileMetadata has exactly 3 callers, all in authorization.ts:

Caller	With {}	Outcome
verifyWorkspaceFileAccess (243)	workspaceId undefined	falls to the existing 'missing authorization metadata' warn → deny
verifyPublicAssetWriteAccess (313)	userId undefined	hits the branch already commented "Fail closed when the owner cannot be established" → deny
verifyCopilotFileAccess (427)	userId undefined	falls through to the legacy path check

The third is a genuine behavior change and is called out deliberately: an absent copilot object previously threw and denied; it now takes the same path as an object that exists without metadata. That path grants only when cloudKey.split('/')[0] === userId — strictly the caller's own namespace — so no cross-user access is reachable. Downstream, a granted read of an absent object 404s at storage and a granted delete is a no-op. No exposure either way.

Other checks: no import cycle (core/errors.ts imports nothing; providers are loaded dynamically from core). Provider files changed by import position only — verified by diffing after biome --write --unsafe. Full lib/uploads suite 375 passing across 31 files. Typecheck clean; the sole error (heic-convert) reproduces on untouched origin/main.

Second audit round — container-level 404s

A high-effort review pass caught a defect the first audit missed: NoSuchBucket and ContainerNotFound also answer 404, so the status-only clause read them as an absent object. A deleted or misconfigured bucket would have degraded into silent "no metadata" — every file read failing closed, with the ERROR that used to alert on it now downgraded to the routine stale-key warn. A total storage outage would have been indistinguishable from ordinary noise.

Container-level labels are now excluded before the object-level check, verified red before / green after:

{ name: 'NoSuchBucket', $metadata: { httpStatusCode: 404 } }        → false
{ name: 'RestError', code: 'ContainerNotFound', statusCode: 404 }   → false

Known limitation, stated rather than implied solved: GCS reports both object- and bucket-level misses as a bare numeric code: 404 with no distinguishing label, so the two are not separable there. S3 is the production provider and does distinguish them.

app/api/tools/s3/head-object/route.ts:86 still hand-rolls its own check and was deliberately not migrated — it is a user-facing tool against user-supplied credentials that reports exists: false, and adopting the shared predicate would turn a nonexistent bucket into an error rather than a negative result. That is a user-visible semantics change and does not belong in this PR.

Suites after the fix: lib/uploads 376 passing / 31 files, app/api/files 238 passing / 16 files.

---

Third audit round — the abstraction itself

The earlier rounds kept finding defects in one place: the shared not-found predicate. That was the symptom. The cause was that getFileMetadata re-implemented the S3 and Blob HEAD calls inline, duplicating headS3Object and headBlobObject — including Azure's entire connection-string / shared-key branch, verbatim. Because it owned those calls, it had to interpret provider errors itself, and doing that safely across providers forced a second, stricter predicate.

Deleting the duplication removes the whole problem:

• getFileMetadata now calls headS3Object / headBlobObject, which already report absence as null, and maps that to {}
• its try/catch is gone — it no longer inspects errors at all
• the second predicate (hasObjectNotFoundLabel) is gone with it; one predicate again, used only by provider clients that know they just performed an object-level operation
• GCS still raises, exactly as before this PR

Each provider now owns its own not-found semantics and the dispatcher just maps null → {}.

Test coverage gap this exposed

Reverting the S3 not-found handling broke no test — storage-client.test.ts mocks headS3Object, so the seam hid whether the two layers agree. Three tests were added to the S3 client's own suite exercising the real headS3Object over a mocked SDK: absent object → null, NoSuchBucket → raises, AccessDenied → raises. Reverting the handling now fails reports an absent object as null rather than raising, as it should.

An earlier attempt at this used vi.resetModules() + vi.doMock() + await import(); that is banned by CLAUDE.md's testing rules and did not work regardless (the file-level mock shadows the real module). Replaced with vi.hoisted() + vi.mock() + static imports, matching the file it lives in.

Final: lib/uploads 380 passing / 31 files, app/api/files 238 passing / 16 files, biome clean on all 8 touched files, typecheck clean apart from the pre-existing heic-convert.

---

Completing the fix: the other two thirds

The change above removes one of the three ERROR lines a stale key produces. Production over 24h shows they fire in near-lockstep, because all three come from the same request:

Line	24h count
Error verifying workspace file access	274
Error downloading from cloud storage:	275
Error serving file:	274

Once authorization correctly denies, the route throws FileNotFoundError; the inner handler logs it at error and rethrows, and the outer handler logs it at error again. So an ordinary 404 was reported twice as a server fault — and route.ts made that plain:

logger.error('Error serving file:', error)   // logged as a failure first
if (error instanceof FileNotFoundError) {
  return createErrorResponse(error)          // then handled correctly as a 404
}

The precedent for the right behaviour sits a few lines above: DocCompileUserError is already logged at info with a comment noting it is "not a server fault". FileNotFoundError now gets the same treatment, via one logServeFailure helper shared by all five catch sites in the file — error is reserved for failures that really are the server's.

Nothing is silenced: the 404 is still recorded, still carries its reason, and every non-FileNotFoundError still logs at error. Two tests cover both directions, verified red before / green after.

With this, the module's ~823 ERROR lines/day drop to ~0 for the stale-key case, rather than the ~549 that would have remained.

Suites: app/api/files 240 passing / 16 files, lib/uploads 380 passing / 31 files.

Per-provider verification

Provider	Change	Result
S3	delegates to headS3Object; NoSuchBucket now raises rather than reading as absence	better
Blob	same delegation; ContainerNotFound now raises	better
GCS	predicate swap is behaviour-identical (a bare numeric 404 still matches); getFileMetadata still raises	unchanged — no fix, no regression
Local	getFileMetadata returns {} as before; gains the serve-route log level	better

All three cloud providers now assert absence and non-404 rethrow, each mutation-verified: removing the guard fails exactly one test in S3 and one in Blob. Blob had no not-found coverage at all before this, and it is the one provider that puts the reason in code rather than name, so the container-level exclusion was the least-verified path in the change.

headS3Object / headBlobObject are also reached through headObject, which serves the TikTok upload tool, workspace forking (2 sites), and workspace-file-manager (2 sites). Raising on a missing bucket brings those call sites into line with headObject's documented contract — "Returns … null when missing. Throws on errors other than 'not found'" — which returning null for a missing bucket had violated, sending callers into a doomed path instead of failing fast.

Two narrow behaviour changes, neither reachable from current callers: a containerName-only custom config is no longer honoured in the Blob branch (every call site passes undefined), and abortMultipartUpload now emits a warn on a missing container where it was previously silent.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 7, 2026 5:59pm

[cursor]

PR Summary

Medium Risk
Touches file authorization inputs and cloud storage error handling; genuine outages must still raise while stale keys downgrade to info—container-level 404 exclusion and GCS ambiguity are documented limits.

Overview
Cuts chronic FilesServeAPI ERROR noise when readers hit superseded workspace file keys (keys rewritten on every content update). Missing objects are modeled as absence instead of failures, and expected 404s are logged at info instead of error.

Storage metadata: getFileMetadata no longer performs inline S3/Blob HEAD calls or catches provider errors. It delegates to headS3Object / headBlobObject and maps null → {}, matching the “no metadata” contract so authorization can deny via existing warn paths instead of generic ERROR handlers.

Shared predicate: New isObjectNotFoundError unifies S3, Azure, and GCS “object missing” shapes while excluding bucket/container 404s (NoSuchBucket, ContainerNotFound) so misconfiguration still surfaces. S3, Blob, and GCS clients use it for HEAD and related cleanup paths.

Serve route: logServeFailure logs FileNotFoundError at info (with reason); real failures stay at error, applied across all serve catch sites—aligned with existing DocCompileUserError → 409 handling.

Tests cover the predicate, provider HEAD behavior, getFileMetadata absence vs real errors, and serve log levels.

^{Reviewed by Cursor Bugbot for commit fac2ed9. Configure here.}

[greptile-apps]

Greptile Summary

The PR changes missing-object handling so S3 and Azure metadata lookups report absence without swallowing provider or configuration failures, while GCS continues propagating ambiguous 404 responses. It also records expected file-not-found responses at informational severity while preserving error logging for other failures.

• Centralizes provider-level object-not-found classification.
• Delegates metadata HEAD requests to the S3 and Azure provider clients.
• Adds regression coverage for absent objects, missing containers or buckets, permission failures, and serve-route logging.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported GCS bucket-failure issue is fixed because ambiguous GCS 404 responses now propagate from metadata lookup rather than being converted to absent metadata.

Important Files Changed

Filename	Overview
apps/sim/lib/uploads/core/storage-client.ts	Delegates S3 and Azure metadata lookups to provider-owned HEAD helpers while preserving GCS failure propagation.
apps/sim/lib/uploads/core/errors.ts	Introduces shared object-not-found classification with explicit S3 bucket and Azure container exclusions.
apps/sim/lib/uploads/providers/s3/client.ts	Maps object-level HEAD 404 responses to null while propagating bucket, permission, and service failures.
apps/sim/lib/uploads/providers/blob/client.ts	Uses the shared classifier for Blob HEAD and multipart-cleanup absence handling.
apps/sim/lib/uploads/providers/gcs/client.ts	Reuses the shared predicate in the existing multipart completion lookup without changing metadata-dispatch behavior.
apps/sim/app/api/files/serve/[...path]/route.ts	Routes expected FileNotFoundError logging through an informational path while retaining error logging for other exceptions.

Sequence Diagram

sequenceDiagram
  participant Route as File Serve Route
  participant Auth as File Authorization
  participant Metadata as Metadata Dispatcher
  participant Provider as S3/Azure Provider
  Route->>Auth: verify file access
  Auth->>Metadata: getFileMetadata(key)
  Metadata->>Provider: HEAD object
  alt Object exists
    Provider-->>Metadata: metadata
    Metadata-->>Auth: metadata
  else Object is absent
    Provider-->>Metadata: null
    Metadata-->>Auth: "{}"
    Auth-->>Route: deny as not found
    Route-->>Route: log at info
  else Provider/configuration failure
    Provider--xMetadata: throw error
    Metadata--xAuth: propagate
  end

Loading

_{Reviews (7): Last reviewed commit: "test(uploads): cover the Blob not-found ..." | Re-trigger Greptile}

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit bcde675. Configure here.}

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit fb85639. Configure here.}

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit fac2ed9. Configure here.}