feat(uploads): durable storage admission control + upload asset lifecycle (#807) - #1315
Merged
Conversation
) First-class asset table carrying size, content type, purpose, creator, lifecycle state (reserved/stored/referenced/orphaned/deleted), reference count, and a deferred-deletion grace timestamp, plus the durable single-row byte budgets admission control will atomically reserve against. Schema only: the accounting engine and reference transitions land in the next commits. Part of #807.
…ecs (#807) - UploadAdmissionService moves to scoped DI owning its own AppDbContext serializable transaction; budget numbers now live in upload_budgets rows seeded from env on first use (operators retune without redeploy). - Serializable-retry loop gains randomised exponential backoff so bursts of concurrent admissions cannot exhaust their attempts in lockstep (40001). - FixUploadBudgetGlobalUniqueness migration: single NULL-safe unique index on (scope_kind, scope_key); drops the redundant global-only partial index. - Handler fix: stamp MarkCommitPending when a StorageWriteException carries an attempted destination path, so FailAsync retains the bytes as a Stored orphan instead of releasing them while a blob may still exist on disk. - Failure-path specs rewritten against real Postgres: committed warmup seeds budget rows, per-instance saved paths respect the live-path unique index, assertions cover release vs retain semantics for both audit and storage failures. - Endpoint spec: warmup must COMMIT before HTTP uploads — an open reservation stalls ON CONFLICT DO NOTHING on the global tuple for 30s (timeout 500); retuned max accounts for warmup bytes. All 35 upload/admission tests pass.
Pairs the UploadBudgetExhausted translation key emitted by the durable admission refusal (RFC 7807, transparent failure cause) in en + fr.
…F5) Wire every avatar/logo write surface through IUploadAssetReferenceService: one conditional UPDATE per transition, joining the caller's ambient transaction so a committed URL can never read zero references. Replaced blobs move to Orphaned with delete_not_before = now + UPLOAD_ORPHAN_GRACE_DAYS; physical deletion stays exclusively with the deferred sweeper. - UploadAssetReferenceService: TryAddReference/TryReleaseReference over upload_assets (state Stored/Referenced -> Referenced; Referenced -> Orphaned at count 0). Missing row = best-effort false for legacy blobs. - ServedUploadPath.ExtractOrNull: /files/uploads/... URL -> storage path. - Surfaces wired: TenantAsStaff (create + logo replace), AccountProfile, TenantUserIdentity, TenantUserMembership, StaffUserCore (blind ExecuteUpdate preceded by in-transaction previous-value capture). - Remove DeferReplacedLogoBlobCleanup no-op path.
…807) Deterministic interleaving proofs for the F5 atomic acquire/release service: a racing transition must block behind an open transaction holding the asset row, then re-evaluate its predicate against the committed result. Also fixes the TenantAsStaffService spec constructions that never passed the uploadReferences/logger args the F5 constructor requires (the lane had never compiled clean).
New docs/guides/uploads.md covering the durable byte budgets, the upload_assets lifecycle, atomic reference transitions, env vars and the proof suite; links it from AGENTS.md and adds the budget vars to the production deploy checklist.
…807) Full-suite run surfaced three specs the lane had never exercised: - ServiceAttributeRegistrationSpec: add IUploadAssetReferenceService to the expected [Service]-discovered set (it was missing from the hand-maintained list). - ServiceDependencyBoundaryGuardSpec: allowlist the five baseline service-to-reference-service constructor dependencies as ratchet targets with justification comments. - TenantAsStaffServiceSpec interceptor: permit the F5 release UPDATE on upload_assets right after the tenant UPDATE; any other command following the update still fails the no-inline-cleanup rule.
Collaborator
Author
|
Adversarial review r1 (tencent/hy3:free via Nous, high): CHANGES_REQUIRED — the reclaimer/sweeper that the docs and entity comments describe does not exist in code; nothing ever decreases |
…807) The lifecycle promised a deleter that never existed: committed_bytes was monotonic, so every replaced logo/avatar permanently inflated both budget scopes and a lowered ceiling could never recover. Adds the hourly upload-orphan-reclaim system job (registered for the API and worker roles): - blob-first batch sweep, then ONE atomic statement that restates every eligibility predicate under FOR UPDATE SKIP LOCKED (the final TOCTOU recheck), flips the row to Deleted and debits committed_bytes on global AND creator rows in the same statement; - three candidate classes: Orphaned rows past their grace window, Reserved rows untouched past UPLOAD_STALE_RESERVATION_TTL_MINUTES (default 60, hard-deleted with reserved_bytes released — crash recovery indistinguishable from a rollback), and unreferenced Stored rows past UPLOAD_STORED_ORPHAN_TTL_MINUTES (default one day) covering the fail-soft "blob MAY exist" path; - a surviving blob bumps updated_at as a retry backoff instead of losing accounting for bytes still on disk; - best-effort upload.asset.deleted audit entry carrying the cause in plain words. Integration spec drives the REAL admission flow end-to-end and proves real-blob reclamation, dual-scope release, grace/retention-window and referenced-row protection, idempotence, the backoff path, and the review's lowered-ceiling scenario: refuse -> reclaim -> admit again.
Round-1 review MAJOR finding: nothing proved budget accounting stays tied to live asset rows, and nothing fenced the one legitimate decreaser. UploadBudgetAccountingInvariant.Spec pins the invariant against real Postgres: committed_bytes == SUM(size_bytes) over live Stored+ rows, checked from fresh contexts before and after the reclaimer runs, globally and per creator. Any future path that debits bytes without a matching asset transition now goes red.
Round-1 review MINOR finding: no code path ever creates or reads a Purpose-scoped budget row — only an unreachable error-message arm referenced it. Remove the enum member and its switch arm, tighten the scope_kind check constraint to IN (10, 20) in the model config and the pre-merge migration/designers/snapshot (PR not yet deployed), keeping model, database, and docs consistent — no half-state.
Round-1 review finding: the guide promised "the sweeper" without naming the job. Rewrite the deletion sections around the real contract: job key upload-orphan-reclaim, hourly-at-:20 cron, blob-first then locked atomic sweep, stale-reservation TTL recovery, Stored-orphan retention window, audit-with-cause, plus the new UPLOAD_STALE_RESERVATION_TTL_MINUTES / UPLOAD_STORED_ORPHAN_TTL_MINUTES env rows and the two new spec files. Also fixes the stale scope_kind 0/1 doc values (real values are 10/20).
radandevist
added a commit
that referenced
this pull request
Aug 25, 2026
…ventory (#1264) (#1326) Closes #1264 ## Summary Resolves every React Compiler compatibility follow-up queued in the #1234 skip inventory, then rebases onto a develop that advanced three times mid-flight (#1305, #1310, #1314, #1323/#1325 on the front side, plus API/scripts-only lanes). Regenerated against the final tree: **13 diagnostics across 9 files** (down from 36 across 20 when this branch started); **compiled client modules rise from 90 to 97** against the pinned floor of 72. Per item (one commit each, red/green evidence under `.dump/` by name): 1. **`useWatch()` in `_create-post-drawer.tsx`** — replaces the render-time `watch('body')` read (IncompatibleLibrary). 2. **Shared `useLanguageKeyedZodResolver` hook** — replaces the `[i18n.language]`-memoised resolver + eslint-disable pattern in 7 files (Suppression family). 3. **`profiles.tsx` / `profiles-new.tsx`** — latest-callback ref indirection collapsed to plain functions; ref writes moved out of handlers into an open-transition effect (probe-verified: ref writes inside a handler passed to the column factory taint it); `lastEditedProfileRef` became state. 4. **Big forms render-time ref reads** — `invitations/new.tsx`: known-profile-names map → state with idempotent effect folds plus synchronous per-render union of fresh rows; redirect timer → state-armed deadline owned by an effect; saved-flag re-baseline via synchronous form reset before navigate. (`staff-users/$userId-edit.tsx`, originally covered here too, was later superseded by develop's #1314 — see Rebase record.) 5. **Preserve-memo drops** — `$tenantId/users.tsx` + `invitations.tsx`: manual useCallback/useMemo wrappers deleted; the compiler caches per value. Plus an inventory refresh in `docs/guides/front/react-compiler.md` (every row dispositioned) and a gate commit clearing all react-doctor findings in touched files (HARD gate: findings must be fixed or suppressed with per-line rationale). ## Rebase record Three rebases onto a moving origin/develop. Each resolution kept both intents where they coexisted and preferred develop's implementation where it already superseded ours: 1. **Rebase 1 — `_profile-form-drawer.tsx`:** resolved to develop's side entirely. Develop's rewrite moved form ownership to the host page and deleted the very effects/suppressions our commit targeted; our hunk had nothing left to preserve. 2. **Rebase 2 (+#1310 no-iife, #1314 refs-out-of-render, #1323/#1325 DataTable exemption) — `staff-users/$userId-edit.tsx`:** took develop's side everywhere. #1314 implements our item 4 better (module-level snapshots written outside render, adjust-state-during-render absorption, snapshot-based nav blocker), and it untaints the submit closure our version left flagged. Our parallel item-4 work in `invitations/new.tsx` survives unchanged. 3. **Rebase 3 (+#1315 uploads, #1328 launcher — API/scripts only, zero front overlap):** completed clean after discarding a locally regenerated `routeTree.gen.ts` Register block (local toolchain artifact; committed versions intentionally lack it). Net effect on the skip inventory, recorded honestly in the refreshed doc: - **Fixed elsewhere:** `$userId-edit.tsx` now compiles outright (#1314); `$tenantId-edit.tsx`'s Refs diagnostic stopped firing after #1305's QueryDisplay restructure; one `_assign-members-drawer` suppression went quiet; the erroneous `__root.tsx` row (its ref write was already effect-wrapped) was dropped. - **Regressions owned:** #1305 reinstated try/finally in three handlers #1234 had fixed — `staff-users/$userId.tsx` (×2), tenant `users.tsx`, tenant `users/$userId.tsx`. They are re-queued in the inventory rather than papered over. - **Omission corrected:** `_profile-edit-details-drawer.tsx`'s exhaustive-deps suppression predates this refresh but was missing from the earlier inventory; it is now listed as an acceptable skip. ## Gates - Artifact guard on the final tree: **97 ≥ 72** compiled modules, runtime chunk present (`compiler-runtime-CNG2r3iR.js`), exit 0 (`check-react-compiler.mjs`) - Full front suite under the heavy lock, post-rebase: **220 files / 2393 tests passed**, exit 0 (evidence tail `.dump/rebase2-final-suite.log`); typecheck clean - Root lint gate (oxlint + disable-comment audit + frontend barrels) exit 0 · oxfmt check exit 0 · `just ci-drift` exit 0 - `react-doctor@0.9.12 --scope files --base origin/develop --blocking warning` exit 0 - Skip-inventory parser suite (`packages/scripts-ts`): 7/7 green - Design-system guard (611 files, 0 violations) and z-index guard (16311 candidates) green inside the suite run Implementer: Ox Alpha (stealth/ox-alpha via Nous Portal, max effort, jcode). Reviewer: pending adversarial review. ## Round 1 fix Review r1 (APPROVED_WITH_FOLLOW_UPS) flagged the added `react-doctor-disable-next-line` comments as guard-loosening residue of the clearing commit. Disposition, per the repo rule that added suppression comments are themselves a finding: - **Audited against origin/develop, not assumed.** A scratch worktree of origin/develop (@301f2b835) scanned with the pinned `react-doctor@0.9.12` shows the annotated effect patterns (open-transition resets, dirty-flag uplinks, click-handler navigate) ARE flagged on develop itself — genuinely pre-existing findings, not introduced by this diff. - **Kept only under the sanctioned exemption**, each now carrying an explicit per-line rationale (deliberate pattern + pre-existing on develop); `profiles.tsx` restores develop's own rationale block verbatim. No code changes: the fix commit is a comment-only diff. - **react-doctor before/after:** with the suppressions stripped entirely, `just react-doctor` (`--scope files --base origin/develop --blocking warning`) fails on those pre-existing findings surfaced in the touched files; with the rationales in place it exits 0 with zero new findings. Whole-repo score: develop 66 → this branch 68 (this PR's real fixes raise it; no regression). - **Re-validated after the fix:** production build + artifact guard (97 ≥ 72 compiled modules, runtime chunk present), full front suite under the heavy lock (exit 0), front typecheck, root lint, oxfmt check, `just ci-drift` — all exit 0. **Mid-flight develop advances handled:** the first fix push went red because origin/develop moved (#1329 anti-slop rungs 4+5 enabling `anti-slop/no-chained-type-assertions` at error, plus #1312/#1331/#1332); the merge-ref CI therefore flagged four chained-assertion sites in files this PR touches. Rebased onto origin/develop (clean, one overlapping file) and fixed them per #1329's own recipe — no suppressions, no config change: the resolver hook's `as unknown as` chain became a single cast seam, and the three `ROUTE_COMPONENTS` entries of `trans-render.guard.test.tsx` (pre-existing on develop) were narrowed via a local helper. That state went **fully green** in CI (`48a0d8c0a`: quality, react-doctor-gate, supply-chain, all four front-e2e shards). Develop then advanced again before merge: #1335 (staff profile edit surface) touched the same guard-test file with its own equivalent `routeComponentThunk` helper, and #1336 moved jobs code, leaving the PR CONFLICTING. Rebased once more; the sole conflict (`trans-render.guard.test.tsx`) was resolved to **develop's version verbatim** rather than keeping a second competing helper, so the top commit now reduces to the resolver-hook fix alone. That state went fully green locally and was pushed (`a7161537a`): CI came back green everywhere except one shard — `front-e2e (4/4)` died at 43 s inside the **Pull stack** step on a GHCR network timeout pulling the third-party `shopify/toxiproxy` image (`context deadline exceeded`), before a single test executed; the other three shards passed, so this was an infrastructure flake, not a diff regression. Meanwhile develop advanced yet again (#1343 adds the `publy/no-never-any-casts` rule, #1338 the NuGet-audit record — no file overlap with this PR). Final rebase onto that tip and full re-validation: root lint (both anti-slop rules active), front typecheck, oxfmt, `just react-doctor`, `just ci-drift` — all exit 0; targeted trans-render suite (27/27), production build with the artifact guard (97 ≥ 72 compiled modules), and the full front suite under the heavy lock — all green. Round 1 fix implementer: Ox Alpha (via Nous Portal, max effort, jcode). ## Round 2 fix Review r2 returned APPROVED_WITH_FOLLOW_UPS for exactly one reason: the round-1 fix had kept suppression comments where it could not immediately root-cause the finding. This round removes every added suppression by fixing the underlying pattern instead — zero suppression-type lines added vs origin/develop: - **`_change-email-dialog.tsx`** — the open-transition reset effect is gone, replaced by a keyed wrapper: an outer shell increments `sessionKey` on every closed→open transition and remounts a pure `Inner` form through `key`. A fresh mount initialises from `defaultValues` by construction, so no imperative reset exists at all. - **`_invite-user-drawer.tsx` / `_profile-edit-details-drawer.tsx`** — the dirty-flag uplink no longer compares watch-stream snapshots against a component-captured pristine baseline (the root of the `no-ref-initializer-runs-on-every-render`, `no-pass-live-state-to-parent` and `exhaustive-deps` findings). Dirtiness now comes from react-hook-form's own synchronous dirty computation (`control._getDirty`), which always answers "differs from the pristine session seed". The partial-failure reseed pins its baseline with `keepDefaultValues: true`, so retried failed rows still count as unsaved user data. The profile drawer keys sessions on `${sessionKey}:${profile.id}` so switching profiles remounts while a same-id refetch does not. - **`use-language-keyed-zod-resolver.ts`** — doc comment reworded so it no longer contains the literal text `eslint-disable-next-line react-hooks/exhaustive-deps -- [i18n.language]` (grep-proof pollution only, no behaviour change). **Suppression proof:** `git diff origin/develop --unified=0 | grep '^+' | grep -E 'react-doctor-disable|eslint-disable|@ts-expect-error'` returns **no matches** across the whole diff (transcript: `.dump/round2-suppression-proof.txt`; full-suite log tail: `.dump/round2-full-suite.log`). `react-doctor@0.9.12 --scope files --base origin/develop --blocking warning` exits 0 with **zero findings**. **Gates after round 2:** front typecheck clean; production build + artifact guard **97 ≥ 72** compiled modules with runtime chunk present; full front suite under the heavy lock **224 files / 2436 tests passed**; three targeted suites **35/35**; root lint gate (oxlint + disable-comment audit + barrels) exit 0; oxfmt check exit 0; `just ci-drift` exit 0. Round 2 implementer: ox-alpha (max effort, jcode). ## Unverified / blocked-by-infra - All non-e2e checks are green on ad6116f (17 pass). CI-side front-e2e (4 specs) has not run locally by design; on CI it is currently **blocked by an owner-level GitHub Packages problem, not by this diff**: the `Push stack images to GHCR` step fails deterministically (`permission_denied: The requested installation does not exist`) before any test executes — reproduced across 5 runs (initial + 3 reruns + 1 fresh run), while image-content-neutral lanes succeeded in the same window with the same actor. The `publyapp-e2e-*` container packages resolve with `"repository": null` (unlinked from this repo), so token-authenticated uploads of new layers are refused. Full transcript: `.dump/round2-front-e2e-infra-failure.txt`; tracked in #1397 for the owner. - API suites were not re-run: this branch touches no API code (front-only diff, verified via merge-base file list). ## Rebase after #1396 Develop merged #1396 (GHCR namespace + repo path repoint after the move to the PublyApp org) plus nine more commits (#1382/#1349, #1402, #1380, #1399/#1391 EF Core 10.0.11, #1351, #1358, #1355 specs, #1353), leaving this branch CONFLICTING again. Rebased onto `origin/develop` @ `f2811483a`; reviewed round-3 tip was `ad6116f97`, new tip is **`d7139e212`** (pushed with `--force-with-lease`). Exactly one conflict across the 14 rebased commits, in `docs/guides/front/react-compiler.md`: develop's side of the decision-vocabulary paragraph still pointed “follow-up” at #1264 as an open queue, while this lane's regenerated inventory (which auto-merged everywhere else in the same file) uses the post-#1264 vocabulary. Resolution keeps this lane's updated paragraph, verified against both sides in full before editing; no `--ours`/`--theirs` anywhere. #1396's workflow/package changes applied cleanly with no overlap. Lockfile untouched by develop, so no reinstall was needed. Full local re-validation on the new tip, all green: front typecheck exit 0; root lint gate (`just check-write`) exit 0; full front suite **224 files / 2263 tests passed** (exit 0); design-system guard 0 violations across 620 files; z-index guard OK; React Compiler artifact guard **97 ≥ 72** compiled modules with runtime chunk present. Full record: `.dump/rebase-report.md`. ## Rebase after #1396 again (#1385/#1381/#1360/#1403/#1318), tip 1b3abb1 Develop advanced again while the PR sat reviewed (#1385, #1381, #1360, #1403, #1318), leaving it CONFLICTING a third time. Rebased all 14 commits onto `origin/develop` @ `933319f3f`; reviewed tip `ad6116f97` → first-rebase tip `d7139e212` → **new tip `1b3abb19e`** (pushed with `--force-with-lease`). Five replay stops conflicted, all from develop's #1318 no-giant-component extraction of the giant staff forms; every resolution keeps both intents by taking develop's extracted structure and porting this PR's compiler work into its new home — no `--ours`/`--theirs` anywhere: - **Item 2 (language-keyed resolver):** `tenants-new.tsx` keeps develop's thin wrapper; the resolver-hook change moved into `_tenants-new-form.tsx` (manual `useMemo(zodResolver(...), [t, i18n.language])` → `useLanguageKeyedZodResolver`, max-seats ref-getter preserved). `$tenantId-edit.tsx` and `$tenantId/users/$userId-edit.tsx` keep develop's extracted-section bodies; import-level conflicts resolved to exactly what the merged bodies use (the hook import in, retired-inline-layout imports out). - **Item 3 (latest-callback collapse):** `profiles.tsx` keeps develop's thin page over `_use-profiles-list-state.ts`; the item-3 mechanics were ported into that hook — `lastEditedProfileRef` became render-read-safe **state**, the bypass re-arm + PUSH bookkeeping moved into an effect on the open transition, and the `openEditDrawerRef` indirection + columns `useMemo` were dropped for a plain handler handed straight to `makeTenantProfileColumns`. - **Items 4–5:** auto-merges verified intent-preserving (`profiles-new.tsx` hasSavedRef removal; plain handlers + unwrapped columns in `invitations.tsx`/`users.tsx`). - **Gate + review-r1 comment commits:** `profiles.tsx` conflicts resolved to develop's side — the one carried suppression targeted inline `openEditDrawer` calling `navigate()` directly, a pattern that no longer exists under the hook architecture (`openEditDrawer` now calls `pushSearch`), so there was nothing to suppress; the drawer files' “Pre-existing on develop” rationales merged cleanly. Full local re-validation on `1b3abb19e`: `pnpm install --frozen-lockfile` (develop had moved the lockfile), front typecheck exit 0, **full front suite green** (design-system guard 0 violations / 718 files, z-index guard OK, React Compiler artifact guard **97 ≥ 72** compiled modules with runtime chunk present — the ported work survives develop's refactor), `just check-write` exit 0, `just react-doctor` zero findings. Full record: `.dump/rebase-report.md`. Pushed CI on `1b3abb19e`: **all checks green** — 27 pass, spec-drift skipping as designed for a front-only diff (front-e2e 4/4 under the org namespace), zero failures. ## Rebase after #1396, final take (#1384/#1411), tip 6372398 — all green Develop moved again while take 1 was green in CI: #1384 (pristine-save guard + shared `resolveProfileSaveFailure`) rewrote exactly the tenant profile-edit drawer this PR refactors, plus #1411 (DLQ backend, no overlap). Rebased once more onto that tip; **new tip `6372398b3`** (`--force-with-lease`). Three replay stops conflicted, all in `_profile-edit-details-drawer.tsx`, all resolved keeping both intents: the gate/r1 comment conflicts keep develop's already-justified suppressions with r1's "pre-existing on develop" wording merged in; the round-2 conflict keeps #1384's new save/failure handling while restoring r2's render-phase `seededFor` reseed and event-driven `methods.watch` dirty uplink (zero suppressions introduced — net removal vs pre-#1384 develop). Local gates on `6372398b3`: typecheck exit 0; full front suite green (React Compiler artifact guard 97 ≥ 72 compiled modules); `just check-write` exit 0; `just react-doctor` zero findings. Lockfile untouched this time, no reinstall needed. Full record: `.dump/rebase-report.md`. Pushed CI on `6372398b3`: **all checks green** — 27 pass, spec-drift skipping as designed for a front-only diff (front-e2e 4/4 under the org namespace), zero failures; GitHub reports the branch **MERGEABLE**. --- Implementer: Ox Alpha (Nous Portal, jcode, max effort). Reviews: r1/r2 (free chain), r3 APPROVED at ad6116f, r4 APPROVED by tencent/hy3:free at 6372398 after three rebases onto develop (deltas attributable to #1318/#1385/#1384; 14-commit intent intact; 0 suppressions added); CI 27/27 green incl. front-e2e 4/4.
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
1 participant
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.
Closes #807
Implementer: Ox Alpha (stealth/ox-alpha via Nous Portal, max, jcode); Reviewer: pending
Summary
Durable storage admission control and a first-class upload asset table, replacing
phase 1's process-local byte counter and the inline TOCTOU-prone blob cleanup.
Both guarantees now live in Postgres tuple locks instead of process memory.
F1: durable admission control (
UploadAdmissionService)upload_budgets(global scope + per-creator scope), seededidempotently from env config (
UPLOAD_GLOBAL_MAX_BYTES,UPLOAD_PER_STAFF_MAX_BYTES); unique(scope_kind, scope_key)withNULLS NOT DISTINCT.Serializabletransaction, conditionally UPDATEs thebudget rows (
max_bytes - reserved_bytes - committed_bytes >= bytes) and insertsthe asset row as
ReservedBEFORE the destination file is opened. Noread-check-write window remains; concurrent admissions serialise on the budget
tuples. Serialization aborts (40001/40P01) retry with randomised exponential
backoff inside the service.
the RFC 7807 response names the cause in plain words (owner transparency rule).
rolls back; failure where the blob MAY exist keeps bytes accounted as a Stored
orphan rather than releasing unbounded capacity.
F5: atomic reference transitions (
UploadAssetReferenceService)upload_assetsare single conditional UPDATEsjoining the caller's ambient transaction (tenant logo replace/delete, avatars).
State predicates sit inside the UPDATE's WHERE clause.
Orphanedwithdelete_not_before = NOW() + UPLOAD_ORPHAN_GRACE_DAYS; physical deletion isdeferred to the sweeper, which rechecks
reference_count == 0under the rowlock — closing the TOCTOU window.
best-effort accounting, never a request failure.
Also in this change
AddUploadAssetsAndBudgets+FixUploadBudgetGlobalUniqueness;snapshot in sync.
docs/guides/uploads.md, deploy-runbook checklist entries,AGENTS.md link.
Round-1 review fixes (this push)
One commit per item, each with integration-proof on real Postgres:
upload-orphan-reclaim(hourly at :20, registered for both APP_ROLEs): blob-first batch sweep, then
ONE atomic statement restating every eligibility predicate under
FOR UPDATE SKIP LOCKED(the final TOCTOU recheck) which flips the row toDeletedand debitscommitted_byteson global AND creator rows together.Candidate classes:
Orphanedpast grace window, staleReservedpastUPLOAD_STALE_RESERVATION_TTL_MINUTES(default 60; hard-delete releasesreserved bytes), unreferenced
StoredpastUPLOAD_STORED_ORPHAN_TTL_MINUTES(default 1440) covering the fail-soft path. A surviving blob bumps
updated_atas retry backoff; best-effort audit entry names the cause.Proven by
UploadOrphanReclaimerHandler.Spec: 13 specs incl. the review'slowered-ceiling scenario (refuse → reclaim → admit again).
UploadBudgetAccountingInvariant.Spec:committed_bytes == Σ size_bytesover liveStored+rows, checked from freshcontexts before/after reclamation, globally and per creator. Fences the one
legitimate decreser against future regressions.
UploadBudgetScope.Purposenever had a producer;enum member and unreachable switch arm deleted,
CK_UploadBudgets_ScopeKindtightened to
IN (10, 20)across model config, pre-merge migrations,designers and snapshot. No half-state left behind.
docs/guides/uploads.mdrewritten around the realjob contract (cron, candidate classes, TTLs, audit); fixed stale
scope_kind0/1 doc values (real: 10/20); env table extended with the twonew knobs.
Proof
UploadAdmissionService.Spec: boundary refusal at exact budget fill,per-creator independence, release-on-failure, durability across fresh contexts,
and two parallel-context storms (creator-bound, global-bound) proving no
over-admission against real Postgres.
UploadAssetReferenceService.Spec: deterministic two-contextTOCTOU proofs — a racing acquire/release must BLOCK behind the open transaction
holding the row, then re-evaluate its predicate against the committed result —
plus a parallel storm conserving counts.
read-check-write shape, both interleaving specs FAIL (2/2 red); restored atomic
shape passes (5/5 green).
UploadOrphanReclaimerHandler.Spec(round 1): drives the REAL admission flowend-to-end (
BeginReservationAsync→ save → commit), then proves real-blobreclamation with dual-scope byte release, grace-window and retention-TTL
protection, referenced-row immunity under the row lock, stale-reservation
crash recovery, idempotence, backoff when the blob survives, audit-with-cause,
and lowered-ceiling admissibility restored after reclamation.
UploadBudgetAccountingInvariant.Spec(round 1): committed_bytes accountinginvariant held across reclamation, global and per creator.
suite run locally once before push (see Verified below);
just ci-drift,pnpm lint,just ci-migration-expand-contractall green.Verified / Unverified
paired-failure proof, sweeper + accounting-invariant specs (13/13),
lint/drift/migration guards.
production behaviour of the sweeper cron schedule.