Lau 20 document deletion storage fix - #337
Open
t1ffnyw wants to merge 20 commits into
Open
Conversation
…act_edges, storage_deletion_requests/items, storage_deletion_tombstones
…sionDeletion, purgeDocumentRelational) + manual test script
…emove database-adapter delete (owned by Dev A/A3), mark those items BLOCKED
…ard-fail rollback on Inngest failure
…UploadThing refs, and version-delete coordinator
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Merge e84b7fa was committed with conflict markers still in the files and pushed to origin. packages/core/src/storage/index.ts had markers inside an export statement - a syntax error - so nothing importing @launchstack/core/storage would load, and the whole deletion lifecycle plus every test script was unbuildable. Also fixes damage git merged silently, with no markers to spot it by: ObjectRef/DeleteResult declared twice in types.ts, deleteRef/deleteMany defined twice in port.ts, duplicate ref keys in uploadthing/core.ts and upload-local/route.ts, and a duplicate ObjectRef import in AddSourceModal.tsx. Upload path standardised on storageRef (not documentRef): processDocumentUpload already takes storageRef, and uploadDocument/route.ts already called resolveUploadRef outside the conflicted region. HEAD's canonical-ref check (Decision 5) is preserved, rewritten against the storageRef naming. legacy-promote.ts takes the HEAD side because the other one references an undeclared dbPathMatch and does not compile. The version-delete route is resolved to A6b (the coordinator cutover) and marked PROVISIONAL in a comment - design doc A6b says to swap this route onto requestVersionDeletion once B1 shipped and delete the A6a path, and the code below the conflict already assumes the A6b result shape. Dev A owns A6; confirm before this is pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5Ag5CzaNtNtTrq2Hvp4qQ
…+ file tenant auth B3 item 2: the worker no longer switches per adapter and deletes one key at a time. Items are grouped by ref identity and go out through Dev A's deleteManyByRef, which batches by (adapter, storageLocationId) - one S3 DeleteObjects call per group, per-ref outcomes so a partial batch retries only what failed (A7). Fixes a real bug that grouping exposed: s3, vercel-blob and uploadthing were called with the bare key, so the Decision 4 stale-location guard never ran for three of four adapters. A ref minted against a retired bucket or Blob store would have been deleted out of whatever store is configured now. It is reported blocked instead. B3 item 3: rejected mapped to blocked, contradicting Decision 2 and the shipped mapDeleteOutcomeToItemState. Removed the local duplicate; the frozen mapping is now the only one, so rejected reaches QUARANTINED and dominates manual_review. B3 item 10: adds scripts/test-storage-failure-matrix.ts - transient retry, budget exhaustion, stale location, unknown adapter, missing-object idempotency, partial batch, quarantine dominance, plus B8. Updates the B3 worker test, which was silently relying on the database adapter being unimplemented to produce BLOCKED items; that stopped being true when A3 shipped. It now blocks deliberately via a stale location, and Part D runs the real delete instead of faking one. B8: adds api-response-schemas.ts (zod shapes for B7's status payload and the B4/B5 bodies, validated outside production only and never able to throw), and file-ownership.ts, which derives a company for a file_uploads row - that table records only an uploader, never a tenant - from manifest, document, version, then uploader, reporting which source answered since they are not equal evidence. Wires that into /api/files/[id], which had no authentication at all and is enumerable. Defaults to log-only via STORAGE_FILE_TENANT_AUTH_MODE: it reports what it would refuse and refuses nothing, because the ingestion path fetches these files server-to-server with no session and enforcing blind would surface as failed document processing. Collapses three flag readers into one. Adds both deletion flags plus the new mode to .env.example. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5Ag5CzaNtNtTrq2Hvp4qQ
… 0022 0022 drops the ON DELETE SET NULL foreign key on storage_deletion_tombstones.request_id, because storage_deletion_requests.document_id is ON DELETE CASCADE - completing a deletion destroys the request row, and the SET NULL then wipes the tombstone's only pointer back to it, exactly when someone holding a request id wants to ask what happened (B7 finding 2). base.ts still declared that FK. db:push regenerates the schema from base.ts, so any database built the documented dev way got the constraint back and the fix silently reverted. Caught on a real run: the worker inserts requestId one line before the purge, and the tombstone came back with requestId null, making getDeletionStatusByRequestId's tombstone fallback permanently unmatchable. Part K of the failure-matrix script (added in the previous commit) purges a document and asserts both that the tombstone keeps its request id and that status-by-request-id still answers, so this cannot regress silently again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related
Issue #313
Lau 20
Checklist
pnpm checkpasses (lint + typecheck)pnpm --filter @launchstack/web testpassespackages/core/changed —pnpm changeset).env.exampleandapps/web/src/env.tsTesting
Notes for reviewers