carlos / platform Public

objstore: what two reviews of the queue found

Two independent reviews of tasks 1-10 landed on the same four defects in the
drain, and both named the same root cause for two of them: a rule stated once
in the spec and implemented in only one of the two places it applies.

DELETION IS DRIVEN BY ABSENCE, NEVER BY ERROR (§7.5). queuePermitted honoured
that for the record read; the item read did not. One transient GET failure on
scan-queue/<id>.json deleted an accepted request outright — no scan, no
callback, and a status door answering `pending` until some sweep happened to
tag the object. Store errors now keep the item and the index entry; only
absence and bytes no build will ever parse are deletions.

DEDUPE THE WORK, NOT THE PROMISES. Two enqueues for one key with different
callback paths were answered with two 202s and delivered one callback: the
duplicate item was deleted before its path was read. The drain now groups a
page by object key, reads the object once, and carries every distinct path
into the outcome. Drained.CallbackPath becomes CallbackPaths.

A DENIED HeadObject IS NOT A RECORDED VERDICT. With no version and no size
there is nothing to scan and nothing was written; reporting recorded:true
would have put "denied" in a signed callback beside a claim the object carries
it, when the console never saw a tag and the enforcement deny matches one that
is not there.

ENQUEUE OWNS ITS CLOCK. The id is derived from At and the TTL is measured from
it, so a door that forgot to stamp it would mint an id sorting at year one and
an item the next drain discards as expired — a 202 that meant nothing. It is a
parameter now, not a field a caller fills in.

Also: a nil Scanner refuses in words rather than dereferencing (RunSweep's
posture); a permanent refusal deletes the app's scan-results with its queue
items, because a result nobody may answer from still blocks the name-claim
gate and resurrects the app; and the two collectors task 9 specified but never
got — DeleteScanResults and CollectScanResults, the latter degrading on a
store with no ModLister the way storagePass degrades without SizeLister.

The tests both reviews called vacuous are rewritten to fail under the mutation
each was supposed to catch, and each was checked by making it:

  denied head says recorded      -> TestDrainAppDeniedHeadIsNotRecorded
  delete on any read error       -> TestDrainAppKeepsItemOnTransientReadError
  no purge re-read guard         -> TestResultObjectNotWrittenUnderPurgedPrefix
  Observe after the listing      -> TestIndexEntrySurvivesEnqueueDuringDrain
  bump before the write          -> TestEnqueueWritesObjectBeforeBump
  dedupe drops later callbacks   -> TestDrainAppDedupesWorkNotCallbacks
  results survive a refusal      -> TestDrainAppDeletesResultsWhenNotPermitted

TestIndexEntrySurvivesEnqueueDuringDrain drove Observe/Bump/Release by hand,
so it proved the methods worked while leaving the ordering they exist to
protect free to be wrong; it goes through DrainApp now. The result key is
pinned to an exact string rather than compared with itself. Status is pinned
against the re-issue rule on MemScanner, which is the only implementation the
queue tests ever see. And the stale-tag test gains the case the version
binding actually exists for: same ETag, later timestamp — single-part ETag is
the content MD5 and chosen-prefix collisions are practical.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YJkJkxZZLgW8X8A5tk69Sq