`POST /api/assets` treated "same bytes, same display name" as "you already have
this" and handed back the existing record with `created_new=false`/HTTP 200.
That silently discarded everything the second request supplied - its tags, its
`user_metadata`, its `preview_id` - and made record identity depend on a
display name, which rulings 5 and 8 say is a label and may legitimately repeat.
An upload is a delivery event, so it now always mints its own record, exactly
as the executed-output path already does when a cached rerun reuses content.
Content dedup is untouched and is the only dedup left: identical bytes still
share one `AssetContent` row, in both hashing modes. `lookup_for_upload_dedup`
existed solely to find the same-name record and, with that arm gone, reduced to
`lookup_for_view`, so ingest calls `lookup_for_view` directly and the function
is deleted.
USER RULING: this makes a retried upload non-idempotent - a client that resends
after a timeout gets a second record. That is accepted deliberately, not
overlooked. No idempotency key or retry protection is introduced here; making
retry safe is a separate decision about a client-supplied identity, and the
multipart parser still rejects a client-provided `id`.
Concurrency safety - the content-reuse arm is a compare-and-swap, not a
read-then-write:
The old content-reuse arm selected a content row in one session, deleted the
uploaded bytes, then opened a fresh session to attach a record to the
now-detached id. A scanner pass or competing writer retiring that row in
between produced a record pointing at content already gone, with no copy of
the bytes left to fall back on. Removing the exact-record arm routes every
same-name upload through this arm instead of only cross-name ones, so the fix
ships in the same commit.
A first version of this fix replaced the two-session gap with a same-session
SELECT re-check (`content_still_qualified`) between the lookup and the insert.
An adversarial review reproduced two concurrency holes that survived it: (1)
the re-check never compared the row's hash, so a concurrent hash correction
(e.g. `detect_content_change`) went undetected; (2) a plain SELECT does not
hold SQLite's write lock, so a competing writer could still commit a
retirement in the window between the re-check passing and this session's own
commit - because pysqlite only opens an implicit transaction (acquiring the
write lock) before a DML statement, never before a SELECT.
`claim_qualified_content` (`lookup.py`) replaces that re-check with a
conditional UPDATE: `UPDATE asset_contents SET is_missing = is_missing WHERE
id = :id AND hash = :hash AND is_missing = 0`, a no-op write whose only job is
to take the write lock and prove the row's state atomically. `rowcount == 1`
means both facts were true at the instant of the write; the row's true hash is
now part of the check, and because it is a write (not a read), SQLite holds
this connection's lock continuously from that statement through this
session's own commit. That lock is database-file-wide, not row-scoped - this
app's default rollback-journal SQLite locking has no row-level granularity -
so it briefly serializes every writer in the app, not only writers to this
row, for the length of this short critical section (claim, filesystem
re-check, record creation, commit). That breadth is what makes the guarantee
hold, not an accident to narrow down. `refresh_qualified_content` then reruns
the unchanged filesystem-level predicate (`_qualifies`: exists,
stat-consistent, non-temp) against a forced re-read, since a DB transaction
cannot make the filesystem itself atomic against an external process. Either
check failing rolls back and falls through to the ordinary new-content path
with the upload still on disk - unchanged from before.
Proven with two tests using two independent file-backed SQLite connections
(not `:memory:`, which is a single shared connection): one where a second
connection commits a hash change between the lookup and the claim (must
reject and fall back), one where a second connection's retirement attempt
during the claim's held lock must itself fail (`sqlite3`/`OperationalError`
naming lock contention specifically), not merely lose a race. Both were
confirmed to reproduce against the prior (first-draft) mechanism via a
standalone repro before being written as this shape.
`openapi.yaml`: the multipart operation loses its HTTP 200 outcome (now
unreachable) and its `id` field (dead spec - the parser rejects `id` with
UNSUPPORTED_FIELD), and `created_new` is described as what it now is.
Rollback: reverting restores the exact-record dedup arm (and with it the
original race).
`Asset.updated_at` carried `onupdate=get_utc_now`, so every write to the row
moved it — including writes the user never asked for. Serving a download or a
hash bumped it via `update_record_access_time`, and background enrichment
bumped it by assigning `system_metadata`/`mime_type`. Sorting the catalog by
`updated_at` therefore reordered on reads and on scanner passes. Master never
did this; the coupling is this branch's regression.
`onupdate` is gone and each explicit user/API mutation sets the column itself:
the `user_metadata`, `mime_type` and `preview_id` updates fold it into their
existing `.values()`, `rename_record` already did it, and manual tag writes now
bump it through `bump_record_updated_at` — once per call, and only when a link
was actually added or removed, so a no-op tag call stays inert.
Deliberately not full master parity: master's background fills also bumped
`updated_at`, and keeping that would let a future hashing-enabled enrichment
sweep silently reorder the whole catalog. Access-time bookkeeping, enrichment,
the automatic missing/recovered tag projection, content split/retire, and the
preview-target `SET NULL` cascade all leave the column alone.
`apply_tags`/`remove_tags` take no new parameter: their only production callers
are the two user-facing tag routes, so the bump is unconditional at the point a
link changes and no system caller can reach it.
The top-level `file_path` served the global-namespace-root path
("models/checkpoints/flux.safetensors") to every API client, duplicating what
`display_name` and `loader_path` already carry in the forms clients actually
consume. Nothing in the response contract needs the namespace-rooted form, so
it is surface no caller has to be given.
Dropped from schemas_out.Asset and from both response builders, and from the
Asset and AssetUpdated schemas in openapi.yaml. AssetCreated inherits Asset, so
list, detail, create/upload, from-hash and update all lose it together.
Every internal path stays: ReferenceData.file_path, AssetContent.path, the
preview URL computation, and system_metadata["file_path"] are untouched, as is
the persisted loader_path this file's tests guard.
Kept as a standalone top commit so the policy can be reverted on its own.
register_file_in_place writes bytes at the locator BEFORE registering, then ran a
global lookup_for_upload_dedup. Both match branches could point at a different file
that merely shares bytes: the Asset branch returned that file's record, the
AssetContent branch created a record against that file's path. Either way the
just-written locator stayed untracked and the caller got back an asset describing
someone else's path.
Ruling (2026-08-27): /upload/image accepts weaker content-dedup than the multipart
endpoint. Drop the global lookup and fall through to create_content unconditionally.
_reconcile_live_content_at_path already leaves at most one live row at the locator,
so unchanged bytes reuse that content row and changed bytes retire it; a distinct
path holding equal bytes now gets its own content row and record, and the hash still
finds both. Re-registering an unchanged file writes a new delivery record against the
reused content row, which is what a repeat save through /upload/image is.
upload_from_temp_path keeps its global dedup-before-write unchanged.
drain_transition_queue branched on null-hash and changed-hash with no else, so the
success case — a recomputed digest equal to the stored one, which is proof the bytes are
unchanged — fell through and left size_bytes/mtime_ns untouched. A row whose stored stat
was stale then failed lookup._stat_consistent and stayed unservable by hash until the next
full scan, even though the drain had just verified it.
Adopt the observed stat on the equal-hash arm, matching what drain_pending_verifications
already does on its matching-hash path.
snapshot_hash returns the stat it verified brackets the bytes it read, and
enrich_asset threw it away. Metadata came from the stat taken on entry, the
digest from whatever the file was during the read, and nothing checked the two
described the same file. A writer landing between them welded old metadata to a
new-bytes hash — permanently, because a row with non-NULL system_metadata is
never an enrichment candidate again.
Keep the verified stat and reject the whole result when it disagrees with the
metadata's observation. The row stays NULL on both fields, so the next pass
retries it once change detection reconciles the stored stat.
The live-path uniqueness race is reported differently by each backend: SQLite
names the column in the message, Postgres exposes the index on diag. Two named
predicates carry that at the call site instead of a comment.
_reconcile_live_content_at_path adopted a freshly-computed hash onto an
existing hash=None row whenever the recorded size_bytes matched the file.
Equal size is not equal bytes: via /upload/image (which writes the
replacement BEFORE calling register_file_in_place) a same-sized overwrite
left the original record and content live while silently adopting the NEW
digest, so records describing the old bytes claimed the new ones.
Rather than guess, take the signal from the caller that has it. server.py
already computes image_is_duplicate - true exactly when compare_image_hash
proved the bytes were already on disk and the write was skipped - so
register_file_in_place now takes content_written and reconciles on fact:
written -> retire, not written -> adopt as before. The no-write path is
untouched, so genuinely unchanged unhashed files are still never retired.
upload_from_temp_path has no caller signal, but it has something better:
the incumbent bytes still exist when it runs. It now settles the
destination row against that file's OWN hash before both the dedup lookup
and the move, so the post-move reconciliation always compares known
hashes and never has to read size as identity.
Second defect: a matching hash returned without refreshing the stored
stat, so a same-bytes mtime-only re-registration left the row failing
lookup._stat_consistent - unservable by hash, and routed into creating a
DUPLICATE record. A matching hash IS proof of byte equality, so the
observed stat is now written back.
Tests: the existing unhashed-changed-file test used different-SIZED bytes
and passed under the old buggy logic too; it now uses same-sized bytes so
it actually discriminates. Adds stale-stat and same-size-different-bytes
coverage for both register_file_in_place and upload_from_temp_path.
