Vulnerability scanning produced nothing across the whole deployment for
nine days, from 2026-08-25 01:20:48 until a scanner restart on 2026-09-03.
The scanner was connected and idle, the hold's discovery pass kept
reporting unscannedFound=15 every four hours, and no scan_jobs row was
created in that entire window.
hasActiveJobs counted pending, assigned and processing rows globally with
no age bound, and waitForCapacity spins while it is true. dispatchLoop
calls it before popping any candidate, so a single pending row that never
reached a terminal state reported "busy" forever: discovery kept pushing
candidates into unscannedQueue and nothing ever popped them. That is why
the symptom was an empty queue rather than a growing one.
Nothing papered over it because push-triggered enqueue only fires for
owner or a tier with scan_on_push, which in production means pro alone.
All 210 manifests pushed to this hold in that window came from free,
supporter, or accounts with no crew row, so the frozen proactive loop was
the only source of jobs.
Nor could it recover on its own. Only Enqueue and drainPendingJobs
dispatch a pending row, and drainPendingJobs runs only when a scanner
newly connects; reDispatchTimedOut considered assigned rows only. The
hold had been up since Aug 14 and the scanner since Aug 21 on the same
websocket, so the drain path had not run since the row appeared.
So bound the capacity gate to pending rows younger than pendingStaleAfter,
give reDispatchTimedOut a pending reclaim, and check RowsAffected on the
assign UPDATE now that two dispatchers can race for a row. waitForCapacity
warns and names the blocking jobs after ten minutes without capacity,
because the failure mode above was completely silent.
Two adjacent fixes for the same outage. The scanner never called
InitLogger, so log_level and log_shipper were dead config and an idle
scanner was mute, which is what made nine days invisible. And skipReason
now also skips a job whose layers contain nothing tar-shaped: the job that
wedged this queue was an in-toto attestation whose config mediaType is an
ordinary image config, so the existing config-type check missed it and
buildOCILayout would have handed Syft an empty image.
The regression tests were verified against the old logic first: three of
them fail on it and pass on the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DPWkeCKcbtGoyXyyeMhSps
Both paginated walks bailed on the first error, and the caller treats a
failed walk as "assume everything is referenced", so one blip skipped that
DID's storage for the whole run.
Measured before changing anything: every DID GC had classified unreachable
but healthy failed only one or two runs out of six, and replaying the exact
same listRecords calls afterwards returned 200 in 45-680 ms with no rate
limiting. Ordinary blips on small self-hosted PDSes, amplified into a
full-DID skip.
Share one listRecordsPage helper between fetchUserTags and
fetchUserManifestsFromEndpoint. The retry decision splits deliberately:
timeouts, connection reset, 5xx and 429 get another attempt, while DNS
failure, TLS failure, connection refused and any 4xx do not. Those are
stable facts about an endpoint, and retrying them would only slow the run
and keep a dead PDS looking alive longer. Unrecognised errors stay
permanent, so an unfamiliar failure degrades to today's behaviour rather
than hammering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UAqi2hS2dhZoatqcWoYZQk
The delete handler returned 204 No Content for htmx requests, on the
theory that an empty body plus hx-swap="outerHTML" would make the row
disappear. htmx's default responseHandling maps 204 to swap:false, so
it never swapped at all: the record was gone from the PDS but the row
stayed on screen until a manual refresh.
Return an empty 200, which htmx does swap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ai43R3s33cBGMybGp2gUcG
264d332 fixed the behaviour but encoded it badly. manifestBelongsToHold
returned (true, false) for an unreachable hold — "yes, but not really" — a
return value that contradicts itself, and isPredecessorHold both applied the
fail-open policy and handed back the raw material for that policy. The
behaviour was right and the shape was wrong.
The underlying problem is that ownership has three states and the return type
had two:
ours - this hold's manifest, or a confirmed predecessor's
not ours - the hold answered, and it is someone else's
unknown - the hold did not answer; don't delete, but do not adopt
For the first two, "is it ours" and "should its blobs stay referenced" have
the same answer, so one bool worked and the design was never stressed. They
diverge only on unknown. Every version so far has had to collapse unknown
onto one of the other two: before 95d4f7c onto "not ours", which deleted a
live predecessor's blobs, and after it onto "ours", which adopted foreign
manifests and put ten phantom missing layer records on hold01. Same shape
error, opposite sides. That conflation is original, not something 95d4f7c
introduced: manifestBelongsToHold has fed knownManifests since the function
was written.
So name the state. manifestClaim has three values, classifyManifest and
classifyPredecessorHold report what they found and apply no policy, and the
one decision that matters — an unknown claim is carried for blob protection
but never adopted — now sits in the open at the call site instead of two
functions deep, which is how it leaked into ownership to begin with.
No behaviour change from 264d332; the three outcomes and the blob protection
are identical. The regression test was re-verified against this shape: it
fails, reporting the adoption, when claimUnknown is allowed to adopt.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWoKzpgtBJ33sCyGxJGR7x
95d4f7c made the predecessor check fail open so a five-second blip against
a live predecessor could not drop its blobs out of the referenced set. That
was right, but the boolean it flipped does two jobs: manifestBelongsToHold
decides both "keep these blobs referenced" and "this manifest is ours", and
for an unreachable hold those want different answers.
The consequence showed up on hold01 the moment it started running this
code. Five stale dev manifests pointing at did:web:localhost%3A8080 and
did:web:172.28.0.3:8080 were adopted into knownManifests, and since hold01
had never stored them, every one of their ten layers was reported as a
missing layer record. Worse than the noise: reconcileMissingRecords acts on
exactly that list, so a Reconcile would have written io.atcr.hold.layer
records asserting hold01 stores blobs for a localhost hold.
These DIDs are loopback and RFC1918, so they can never resolve from a
server. This is not a transient outage that clears itself on the next run.
manifestBelongsToHold and isPredecessorHold now return (value, definitive),
matching the idiom checkPredecessor already uses. An indefinite answer still
carries the manifest so its blobs stay referenced, but marks it ProtectOnly,
and analyzeRecords protects its digests without adding it to knownManifests
— the same shape the in-grace takedown branch above it already uses.
Left alone deliberately: the legacy holdEndpoint path still treats a resolve
failure as a definitive "not ours". That predates 95d4f7c and fails closed
rather than open, so it is a different bug with a different blast radius.
The regression test was verified to fail without the fix, reporting the
adoption rather than a build error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWoKzpgtBJ33sCyGxJGR7x
Update every direct dependency across all five workspace modules to
latest. Notable jumps: syft v1.43.0 -> v1.51.1, grype v0.111.1 ->
v0.118.0, stereoscope v0.1.23 -> v0.3.1, indigo -> 2026-09-01,
aws-sdk-go-v2/service/s3 v1.99.1 -> v1.110.0, grpc v1.80.0 -> v1.83.2,
x/crypto v0.50.0 -> v0.55.0.
Three deps needed more than a version bump:
go-libipfs could not be updated at all. The repo was renamed to boxo, so
every tag past v0.7.0 declares `module github.com/ipfs/boxo` and cannot
be required under the old path. sqlite_store.go already imported
go-block-format alongside it and used the archived package exactly once,
inside a function already returning blockformat.Block, so it was relying
on structural interface satisfaction. Collapsing to the native type drops
the archived dependency entirely.
go-didplc moved its package from the repo root into a didplc/ subdir in
v0.2.2. Package name is unchanged and every symbol we use (RegularOp,
OpEnum, OpService, Client.DirectoryURL, Submit) is intact, so this is an
import path change only.
The go-diskfs replace in scanner/go.mod had inverted. It pinned v1.7.0
because syft v1.43 passed diskfs entries as os.FileInfo; syft v1.51.1
fixed that upstream and now requires v1.9.4, so the workaround had become
the thing breaking the build. Removed per its own "Remove when syft ships
a fix" note, closing anchore/syft#4796 for us.
The indigo bump needed no code changes: of the 21 packages we import only
5 changed, and the repo/MST/CAR-store core is byte-identical. It does
bring a util/ssrf fix blocking 6to4 addresses (2002::/16), which we
inherit through atproto/auth/oauth.
Go 1.26.7 across go.work, all five go.mod files, the four Dockerfiles,
the three tangled workflows, and the stale references in
docs/DEVELOPMENT.md. Verified golang:1.26.7-trixie resolves on
mirror.gcr.io, which is what the Dockerfiles actually pull from.
Makefile's TRIXIE_BUILDER_IMAGE stays on the floating golang:1-trixie.
make test, make lint, and make test-race all pass, as do the scanner
module's tests and the integration-tagged build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWoKzpgtBJ33sCyGxJGR7x
The hold end of the billing fan-out had no test at all -- only the
ErrCrewMemberNotFound sentinel was covered. Its answer decides whether the
Stripe webhook records an event as processed or retries it, so each status it
can return means something different upstream and is covered separately: the
applied path (asserting the stored crew record actually changed, not just the
response body), not-crew as a successful no-op, the 403 on a body userDid that
disagrees with the signed subject, an empty body userDid falling back to the
token subject, 401 unsigned, 400 with no tiers configured, and rank clamping.
Each was mutation-verified. One of them corrected the test's own comment:
removing the 403 guard does not let a body retarget a grant, because every step
after it keys off the token's sub claim and req.UserDID is read nowhere else.
The guard makes a disagreeing body loud rather than silently ignored, and the
stored-tier assertion is the regression guard for the day something reaches for
that unsigned field when it needs "which user".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB
7d9de7c moved handle resolution below the sort-and-truncate, so a hold with
~500 crew stopped making ~500 serial identity lookups to render ten rows. It
shipped without a test, and the regression is a two-line move.
The property pinned here is the LOOKUP COUNT, not the wall clock. Timing would
pass or fail on how fast the machine is; the count fails precisely when
resolution moves back above the truncation. Mutation-verified: restoring the
old shape produces 60 lookups for 10 rendered rows against a 50-user hold, and
the failure message names the cause.
The second test covers the 3s resolve deadline the same commit added — a
stalled lookup must degrade to a bare DID rather than consume the reverse proxy
budget, which is what left the client hanging up mid-render before.
resolveHandle becomes a package var, since counting lookups is the only way to
observe either property from outside.
Two things worth knowing for the next test in this package. AdminUI.pds is a
concrete *pds.HoldPDS, so this needed a real one: NewHoldPDS with a file-backed
path (":memory:" is per-connection in libsql and disables the records index
QuotasByDID reads), then Bootstrap, or the first record write fails with
"cannot serialize undefined cid". And BatchCreateLayerRecords writes only to the
CAR store — the records index is fed from the repo event stream, which is not
running in a test, so BackfillRecordsIndex has to be called explicitly or the
quota query returns nothing and the count assertion passes vacuously at zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VwxF2N3HuZ8xSkx6nkirgB
deleteOrphanedBlobs is the only part of GC that removes bytes and it had no
test. It could not have had one: object age decides whether a blob is
deletable, the in-process harness stamps every object with time.Now(), and
MockS3Client's ListObjectsV2 set no LastModified at all. Neither could express
"this blob is nine days old", so both halves of the grace rule went
unexercised — including the half that protects a push still in flight.
MockS3Client gains ObjectTimes, a per-key LastModified consulted by
ListObjectsV2. Keys with no entry list without a timestamp exactly as before,
so existing tests are unaffected. It also gains DeleteObjectError, matching
the error injection the other operations already had.
Four cases, three of which are reasons NOT to delete: an old unreferenced
blob goes; a young unreferenced blob stays; a referenced old blob stays; a
/link object is never treated as a blob. Plus a failure case pinning that one
undeletable object does not abort the walk, and that a blob which never left
storage is not counted as deleted or reported as reclaimed space.
Verified by mutation. Disabling the grace check deletes the young blob;
disabling the referenced check deletes the live one; both fail. Disabling the
/data suffix check changes nothing, because extractDigestFromPath anchors on
/data$ and rejects everything else — so that check is a redundant early-out
rather than a guard, and the test says so rather than implying otherwise.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fetchUserManifestsFromEndpoint, fetchUserTags and fetchUserProfile
interpolated the user DID straight into a query string. A did:web carrying a
port spells that port as a literal %3A, so the PDS received the parameter
decoded back to ":" — a different DID, matching no repo. listRecords then
answers 200 with an empty list and GC reads it as "this user has no
manifests": every blob they own drops out of the referenced set and is
deleted once past the seven-day blob grace.
There is no error and no status code to notice, which is the same soft
failure 95d4f7c fixed one function away in this file. getRecord for
manifests and checkPredecessorAt already escaped; three of the five call
sites did not.
Scope is local testing only. did:plc, which every production user has,
contains nothing that needs escaping, and did:web with a port is not really
valid in atproto — but it is what the dev stack runs on, so GC there sees a
referenced set of zero and considers every blob in the bucket collectable.
That made it impossible to validate the sweep end to end, which is how it
surfaced.
Found by test/integration/gc_test.go, added here: it pushes an image and
asserts GC accounts for every blob the push wrote. The blob grace period is
a package constant, so nothing pushed during a test can age past it and "no
orphans" is vacuous; the load-bearing assertion is referenced == total,
which grace does not touch. Against the unescaped code it reported
referenced=0 of 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2e55352 taught the scan broadcaster that a successor label is only
interesting when it points at us; the GC copy of the same check was left
accepting any non-empty successor. Confirmed still divergent at the head of
this stack: gc.go was a bare `if captain.Successor != ""` while
scan_broadcaster.go:1473 compares against sb.holdDID.
A hold that retired into some third hold is that hold's predecessor, not
ours, and its manifests are not a reason to keep blobs referenced here. GC
therefore now makes the same comparison the broadcaster does, against
gc.pds.DID().
This is the one change in the batch that makes GC delete more rather than
less, so it is deliberately its own commit and carries a floor. If this hold
cannot say who it is, ourHoldDID() returns "" and the old permissive answer
stands: we cannot conclude a successor is not us, and over-protecting merely
leaks blobs while guessing the other way destroys them. That branch has its
own test, because an empty DID silently turning every predecessor into a
stranger is exactly how this reconciliation would become the next blob-loss
bug.
The inconclusive-on-failure semantics 95d4f7c added are untouched: only the
answers from a hold that actually replied are affected.
Verified by mutation: forcing the comparison back to the permissive form
fails exactly one case, the successor naming a third hold, and leaves the
unknown-own-DID fallback passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
95d4f7c split the fetch-and-parse half of the predecessor probe into
checkPredecessorAt precisely so it could be tested against a local server,
but no test ever followed. The paths that had none are the ones that used to
delete another hold's blobs: a 500, a refused connection, a body that is not
JSON, and an envelope wrapping a garbage record all have to report
definitive=false, because only a definitive answer is allowed into the
process-lifetime predecessorCache.
Verified by mutation rather than by passing: flipping the six failure-path
returns in checkPredecessorAt to definitive=true, which is the pre-95d4f7c
semantic, fails all four cases.
Also pins the reset that the predecessorUnresolved field documents but
nothing enforced. Removing the clear at the top of analyzeRecords makes the
test fail, which is the point: without it one outage is permanent, every
later run short-circuits on the stale entry, and GC silently stops
reclaiming anything that hold's manifests touch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
newTestScanBroadcaster builds the struct with only a database, so nothing in
the suite ever reached handleWriter, handleReader, or the Unsubscribe teardown
they share — which is precisely what 05b856b changed. These give the subscriber
a real WebSocket so the teardown actually runs.
The invariant is the one Unsubscribe documents: a dropped scanner unwinds both
goroutines and each calls Unsubscribe, so everything past the `found` guard must
happen exactly once. Closing `done` twice panics and takes the hold down with
it, and re-running the requeue UPDATE would unassign jobs a replacement scanner
had already claimed.
Three cases: Unsubscribe called twice on the same subscriber, handleWriter
releasing and closing the connection once done is closed, and the real shape of
a scanner vanishing — client closed, both goroutines unwinding into the same
subscriber, plus a late duplicate Unsubscribe after the fact.
Passes -race -count=5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Webhook delivery was neither idempotent nor order-safe, and every failure
returned 400, which Stripe does not retry. A transient DB or hold error
therefore dropped a subscription change silently and permanently.
- New stripe_processed_events table: event_id as primary key dedups
redelivery, and event_created per customer drops stale out-of-order
deliveries.
- HandleWebhook distinguishes ErrWebhookSignature (400, no retry) from
ErrWebhookProcessing (500, Stripe redelivers). The event handlers
return errors instead of swallowing them. ErrBillingDisabled maps to
400: the route is mounted but billing is off, so redelivery can never
succeed and Stripe should stop rather than retry to exhaustion.
- Refuse to boot when billing is enabled with an empty
STRIPE_WEBHOOK_SECRET. Stripe HMACs with the empty key, so an
attacker can reproduce the signature and the endpoint is forgeable.
- UpdateCrewTierOnAllHolds retries each hold (3 attempts, linear
backoff, 5s per request) and returns a joined error so the webhook
can fail and let Stripe redeliver.
The fan-out contacts holds concurrently rather than in sequence. Serially,
one unreachable hold burns the caller's entire 10s budget on its own
retries (3 x 5s plus backoff) and the holds after it are never contacted;
because Stripe redelivers in the same order, a persistently-down first
hold means the rest are never updated at all.
On the hold, the signature-validated sub claim is now the source of truth
for updateCrewTier: a mismatched body userDid is rejected with 403 rather
than retargeting the grant to another DID. "Not crew on this hold" is a
200 no-op, since the appview fans updates out to every managed hold and a
subscriber is not crew everywhere.
That no-op has to be told apart from a storage failure. GetCrewMember
collapsed both into one generic error, so a CAR-store failure read as
"not a member", answered 200, and let the appview record the event as
processed — losing the tier grant permanently, which is exactly the
failure mode this commit exists to prevent. Missing records now carry an
ErrCrewMemberNotFound sentinel, and anything else returns 500 so Stripe
redelivers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The dashboard's top-users panel resolved a handle for every user with a
quota record, then sorted and truncated to ten. On a hold with ~500 crew
that is ~500 serial identity lookups to render ten rows.
Each lookup goes through the shared identity directory, whose HTTP client
allows 10s per request. One stalled lookup consumed the entire reverse
proxy budget, so the panel returned a partial body and the client hung up
mid-render:
admin/auth.go:161 "Failed to render template"
template=partials/top_users.html
error="write: broken pipe"
"GET /admin/api/top-users?limit=10" - 200 4096B in 10.005s
Sort and truncate first, then resolve only the surviving rows, so the
count is bounded by the limit rather than by hold size. Resolve those
concurrently under a 3s deadline: a slow lookup now degrades to a bare
DID instead of taking the whole request down with it.
The crew tab has the same underlying problem in a different shape, one
lazy-load request per row, and is not addressed here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
checkPredecessor returned a bare false on every failure path (DNS failure,
dial error, non-200, 5s timeout, unparseable body), indistinguishable from a
hold affirmatively answering "I have no successor". isPredecessorHold then
cached that false in predecessorCache, which lives for the life of the
process and is never reset, so one blip during a single GC run permanently
unreferenced that hold's manifests. Those blobs are long past the 7-day
grace period that protects recent content, so the next run deleted them
outright with nothing to fall back on.
checkPredecessor now reports whether its answer is definitive, and only
definitive answers are cached. An inconclusive check keeps the hold's
manifests referenced and is recorded in predecessorUnresolved, which bounds
the cost to one timeout per run rather than one per manifest and is cleared
at the start of every analysis so a hold that was down once is re-checked
next time instead of written off.
This matches the convention the rest of the package already follows: a user
whose PDS cannot be reached has their records treated as referenced, never
as garbage. An outage must not be the reason content becomes deletable.
Non-200 counts as inconclusive on the same reasoning. A reachable service
that cannot produce its own captain record is malfunctioning, not answering,
and over-protecting an unrelated hold merely leaves some blobs unreclaimed.
Splits the fetch-and-parse half into checkPredecessorAt so it can be tested
against a local server without depending on DNS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same class of bug as the firehose backfill, plus a second one found alongside
it. Both take down the whole hold process.
1. send on closed channel. Subscribe spawns drainPendingJobs in its own
goroutine, and it sends to sub.send without holding sb.mu. Unsubscribe
closed sub.send under the lock, so a scanner disconnecting during the drain
closed the channel out from under an in-flight send. The existing
`case <-sub.done` guard did not help: done meant "writer goroutine exited"
and was closed by handleWriter, which is a different event from
unsubscribing.
2. close of closed channel. Unsubscribe closed sub.send unconditionally, but
it is called from two places — handleWriter on write error, and
handleReader in its defer. A scanner dropping mid-write hits both, and the
slice-removal loop had no guard, so the second call fell straight through
to the close. The unassign UPDATE ran twice for the same reason, which
could return jobs a replacement scanner had already been handed.
sub.send is now never closed. done is repurposed to mean "this subscriber is
gone", closed only by Unsubscribe and guarded on whether the subscriber was
actually still registered. That makes drainPendingJobs' existing done case
correct, and handleWriter selects on done rather than ranging over send.
dispatchJob was already safe — it sends under sb.mu, which excludes
Unsubscribe.
hold01 is unaffected in practice (scanner disabled, no shared secret), but
seamark-hold runs the scanner continuously and is exposed on any scanner
restart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A relay that connected with a stale cursor and then dropped mid-backfill took
the whole hold process down:
panic: send on closed channel
pds.sendBackfillMsg events.go:869
pds.backfillFromDatabase events.go:809
pds.backfillSubscriber events.go:716
Subscribe spawns backfillSubscriber in its own goroutine, and that goroutine
writes to sub.send without holding b.mu. Unsubscribe closed sub.send under the
lock, so a disconnect during backfill closed the channel out from under an
in-flight send. select cannot guard that — a send on a closed channel panics
unconditionally.
sub.send is now never closed. Subscriber gains a done channel that Unsubscribe
closes instead, and every sender that runs unlocked selects on it. The map
check in Unsubscribe keeps the close single-shot, which matters because both
readPump and handleSubscriber call it on the way out. handleSubscriber selects
on done rather than ranging over send, since nothing closes send any more.
Broadcast and BroadcastIdentity were already safe (they send under b.mu, which
excludes Unsubscribe). backfillFromMemory was safe too via b.mu.RLock, but now
routes through the shared helper so a disconnect aborts immediately instead of
stalling up to 5s per event while holding the read lock and blocking every
broadcast.
The bug dates to 2025-10, so every build since is affected. It only fires when
a backfill goroutine exists, which Subscribe skips when cursor == currentSeq —
that is why caught-up relays never triggered it and a hold whose relays are all
behind is exposed on every reconnect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to f12d5e0. Extending the orphan sweep to io.atcr.hold.scan and
io.atcr.hold.image.config brought those collections under a delete path built
for layer records, and the two are not interchangeable. Layer rkeys are
generated per write, so a remembered rkey is a stable handle on one record.
Scan and image-config rkeys are atproto.ScanRecordKey(manifestDigest) — the bare
digest hex, with no DID — which is both reused across re-pushes and shared
between users who push identical images. "Delete rkey K" therefore stopped
meaning "delete the record I judged", and the sweep could destroy live data two
ways.
Both were confirmed by probe before being fixed, and each guard below has a
regression test that fails when that guard alone is reverted.
Shared records: two users pushing the same image collapse onto ONE record whose
body names only the last writer. When that user deleted their manifest the
record looked orphaned, and deleting it stripped the vulnerability scan and the
layer history/env/entrypoint from every co-owner's still-live image.
- knownDigests keeps a record while any successfully fetched user still holds
a manifest at that digest, not just the one the record names.
- knownDigests alone is not enough: it is built only from users we reached, so
a co-owner whose PDS was down this run is indistinguishable from one who
deleted their image. digestOwners closes that. It maps each digest to every
DID that pushed it here, derived from the hold's own layer records during
the walk analyzeRecords already performs, so it costs nothing extra and
needs no network. If any owner was unreachable, the record stays. This is
the sweep's existing principle — an unreachable PDS never causes a deletion —
extended from the record's named user to everyone the record serves.
Layer records are the right source because they are load-bearing for storage
accounting and billing, so they exist for anything a user is charged for.
Reused rkeys: a re-push upserts into the same digest-derived slot, so a record
marked orphaned by an earlier scan can be replaced by a LIVE one before the
delete runs. The admin delete button makes this wide — lastPreview is in-memory
for the life of the process, so an open tab keeps a stale scan actionable — and
doRun has the same race across its analyze-to-delete gap.
- Records now carry the CID they had when judged, and the delete re-reads the
slot to confirm it still holds that revision. Anything else (rewritten,
missing, unreadable, or a ref with no recorded CID) is skipped, not deleted.
- The CID check is record identity, not orphanhood, and identity alone is not
enough either. A re-push does not necessarily rewrite the SCAN record:
scan-on-push is tier-gated, and the broadcaster's discovery loop skips any
manifest that already has a scan record, so its CID survives a re-push
unchanged. manifestsWithLayerRecords covers that — a push writes layer
records to this hold, so their presence proves the manifest is back
regardless of what the scan concluded. Local, no PDS round trip. A manifest
whose layer records are themselves orphaned but not yet collected also lands
in the set, which only defers its aux record one cycle; one more round of a
leak beats deleting a live user's records.
- maxPreviewAgeForDelete (30m) refuses a stale preview outright. The
per-record checks are the real safety net; this stops the admin acting on a
picture that is hours old. Refusal surfaces through startBackground ->
lastError -> the polling progress fragment, so it is visible rather than a
silent no-op.
The four state maps auxRecordOrphaned consults are grouped into auxOrphanState.
Three are same-shaped maps that were trivial to transpose positionally, and
swapping knownDigests with fetchedUsers would have silently widened what the
sweep deletes.
Not addressed, all failure-to-collect rather than data loss: maxPreviewItems
caps the admin button's list while doRun uses the uncapped one, so a hold with
10000+ orphaned layer records surfaces no aux orphans in the preview;
DeleteManifestAuxRecord cannot distinguish "already gone" from "write failed";
and discoverUserDIDs never consults the aux collections, so a record whose owner
has no remaining layer records is kept indefinitely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user who deletes a manifest record directly on their PDS was invisible
to the hold. purgeManifest only fires on appview-driven deletes, and the
GC orphan sweep only walked io.atcr.hold.layer, so scan and image config
records survived forever and the user's storage number never dropped.
analyzeRecords now also walks io.atcr.hold.scan and
io.atcr.hold.image.config, applying the same orphan test the layer sweep
uses: a record dies only when its owning PDS was reachable and
demonstrably lacks the manifest. Unreachable PDS, unparseable URI,
unparseable timestamp, and immature records all keep.
Splitting the grace period was required to make this useful, not
cosmetic. Records only need to outlast a push (blobs and layer records
are written before the manifest reaches the user's PDS), so they age out
at 24h. Blobs stay on the 7 day window, but since records now age out
faster than the blobs they name, a record can no longer serve as its
blob's clock. Blob age comes from S3 LastModified instead, which
WalkBlobs already received from ListObjectsV2 and was discarding.
Net effect: pruning an old manifest frees the user's storage on the next
nightly run rather than never, while the bytes are still reclaimed on
the same best-effort schedule as before.
DeleteManifestAuxRecord is deliberately narrow, accepting only the scan
and image config collections, so a bug in the sweep can't reach captain,
crew, or layer records.
Not addressed: the SBOM and vuln report blobs those scan records point
at. They share the /repos/{holdDID}/blobs/ prefix with the hold's avatar
and OG images, so collecting them needs a referenced-CID set built from
the hold's own records rather than a prefix walk.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
checkPredecessor returned true for any hold with a non-empty successor field,
without verifying the successor was actually this hold.
This caused every hold running proactive scan discovery to queue manifests from
every other migrated hold on the network, producing 404 errors when the scanner
tried to fetch foreign blobs from its own S3.
Signed-off-by: Maarten Rijke <did:plc:fy4lwkc4hrd776vfkcrbzr5a>
- diff view gains a Packages tab with added/removed/changed/unchanged
package tables and purl-derived type/license/upstream links
- captain records verified against the DID's atcr_hold service before
caching (processor + batch backfill), preventing forged holds
- fix empty-handle updates clobbering cached handles and colliding on
the UNIQUE constraint
- move fillPrevCIDs into repo.go; DirectRepoOperator is now canonical,
repomgr kept as a test oracle
- surface read-only crew status in hold selector
- reconcile docs
1. Multiple registry domains + per-user domain preference
The biggest feature. The appview can serve several registry domains (e.g. buoy.cr, atcr.io),
and users can now pick which one shows up in their pull/push commands.
- Lexicon/record: adds registryDomain (and documents ociClient)
to the sailor profile (lexicons/.../profile.json, pkg/atproto/lexicon.go).
- DB: new registry_domain column on users (schema.sql + migration 0027),
with GetUserByDID/Handle reads, UpdateUserRegistryDomain writer,
and Jetstream caching it on profile updates (writes unconditionally so clearing propagates).
- UI/handlers: new UpdateRegistryDomainHandler + /api/profile/registry-domain route,
a <select> in the user settings panel (only shown when >1 domain configured), and resolveRegistryURL()
which falls back to the primary domain if the user's pref is stale/removed. Tests added for all of it.
2. default_hold_did removed → first managed_holds entry is the default
Consolidates two overlapping config fields into one. ServerConfig.DefaultHoldDID is gone;
PrimaryHoldDID() now returns managed_holds[0]. managed_holds is now REQUIRED.
Updated in config, validation, server wiring, test harness, example YAML, and the deploy template.
3. Admin long-running operations → generic background-job framework
New pkg/hold/admin/jobs.go introduces a reusable startJob/jobRegistry pattern
(a detached context.Background() job + a /admin/api/jobs/{key}/status polling endpoint).
This replaces the bespoke scan-backfill goroutine state machine, and now also wraps crew tier remap and crew import
all three previously looped synchronously on the request context and got 504'd/cancelled mid-run by the reverse proxy.
Forms switched from POST-redirect to htmx fragments (job_progress.html, job_result.html, crew_import_results.html)
the old crew_import_results.html page and scan_backfill_progress.html partial were deleted.
This is also captured as a new rule in CLAUDE.md.
4. Cascade-delete manifest on last-tag deletion
DeleteTagHandler now, after removing the last tag pointing to a digest, cascade-deletes the manifest itself
(PDS + DB + hold blob purge) — but only if it's not a child of a manifest list (multi-arch parent).
New GetTagDigest and ShouldCascadeDeleteManifest queries back it, plus cascade_delete_test.go.
Also switches tag rkey computation to the atproto.RepositoryTagToRKey helper.
5. Billing simplification
Drops the OwnerBadge config option (hold-owner supporter badge).
The user-profile template no longer special-cases an "owner" badge value (only "Captain").
Example tiers renamed to the nautical scheme (deckhand/bosun/quartermaster).
6. Build/deploy: go generate always runs via Make
make generate is now a phony target that always runs go generate ./... (regenerating cbor_gen, icon sprites, etc.),
and build-trixie depends on it. The deploy tooling (provision.go/update.go)
drops its own runGenerate calls since the Makefile handles it.
7. New cmd/firehose-tap tool (untracked)
A standalone CLI that subscribes to a com.atproto.sync.subscribeRepos endpoint and pretty-prints events,
with emphasis on Sync 1.1 compliance fields (per-op prev CIDs, commit prevData) and a --validate CI mode.
Fits with the recent "more sync1.1 compliant" commit.
Evan JarrettandClaude Opus 5
Maarten RijkeandTangled