deriveThumbnails checked only that the parent ATTACHMENT row was live and
then copied parent.ItemID verbatim into every derived row. After TASK-2401's
read gate that is a waste with a cost: a variant of an archived item's
attachment is quota-counted storage that the blob path (DR-13) refuses to
serve, so the bytes are written, charged, and unreadable until the item is
restored. The same holds for a malformed item_id — the column has no FK and
no same-workspace constraint, so a row can name a foreign-workspace item or
no item at all.
Derivation now resolves the parent item at entry, before the blob is even
opened, and skips when it is soft-deleted, unresolvable, or in another
workspace. GetItem, not GetItemIncludeDeleted, so "live" means the same
thing here as on the read path. Orphan rows (item_id NULL) have no item to
check and still derive. This is internal background work with no HTTP
response, so there is no 404 shape to match: it skips and logs a WARN
alongside the existing decode/resize/persist skip logs, with the malformed
cases carrying distinct messages so they are greppable ahead of PLAN-2397's
repair.
The post-check window is DELIBERATELY ACCEPTED, and the comment on
thumbnailParentItemLive says so at length so the next reader does not file
it as a bug. The check is point-in-time — item deletion commits in its own
transaction and the read/decode/resize/encode/Put in between is unbounded
work — so an item archived mid-flight can still get a variant. Transform
(TASK-2402) closes its equivalent window with store.CreateAttachmentForLiveItem;
derivation deliberately does NOT, and makes the opposite trade: transform is
user-initiated and low-volume, whereas derivation is a background worker
fanning out from every image upload, so an item lock here is disproportionate
to the harm. What leaks through is a thumbnail — small, unreadable for as
long as its item stays archived, and tombstoned by the delete cascade with
its parent attachment.
Tests cover the sequential cases only: already-archived (with a sanity check
that DeleteItem really is a soft delete), unresolvable item_id, and a
foreign-workspace item_id that resolution alone would accept. The raced case
is deliberately not asserted — it is permitted behaviour, and pinning it
either way would constrain what the design leaves free. Two controls keep
the skips honest: a live parent and an orphan row must both still derive
from the same fixture and the same bytes, so a fixture that stopped
deriving at all would fail loudly rather than pass the skip assertions
vacuously. All three skip tests were mutation-verified against a
short-circuited guard, and the file passes -race -count=3 and make test-pg.
Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC
xarmian
90eb871da3