Hold discovery in the registry middleware called getRecord on the
repository owner's PDS for every request under /v2/: every HEAD, POST,
PATCH, PUT and GET. A 10-layer push was 40 or more PDS round trips, and
it was the last per-request network call on the push path that had
nothing to do with moving bytes. Only two profile fields are used
there: the default hold and the auto-remove-untagged flag.
The users row already caches the default hold, written by the Jetstream
processor on every profile event and prefilled by the backfill, and the
auth gate already reads it from there. This makes the row a faithful
copy of what the registry needs and switches the middleware to it.
The auto-remove flag gets a nullable users column. NULL means the value
has never been learned; the processor writes 0 or 1 on every profile
event and never NULL. On a request whose row is missing or still NULL,
the middleware does one live fetch, uses it, and writes both fields
back, including a 0 for a user with no profile at all, so the fallback
runs at most once per user. A failed fetch writes nothing and uses the
appview default for that request, so a network error is never cached.
That single mechanism covers the minutes after a deploy while the
startup backfill fills the column, a brand-new user, and a user the
backfill has not reached.
The processor also stops returning early on an empty default hold,
which left a user who removed their custom hold pushing to it forever.
Empty is now written through and means the appview default, matching
what the auth gate already reads.
Tests count PDS requests with a test server: a populated row makes
none, a NULL row makes exactly one and then none, a missing profile is
cached as known, and a failed fetch degrades without writing. The
migration was applied to a fresh database and to one built from the
previous schema.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Yf1ZVA7sXYhQNb9tCo1m5
Completes the swap 0033 set up. layers and manifest_references move onto
manifest_key and manifests.id is gone, which removes the last node-allocated
identifier in the AppView schema.
Statement order in 0034 is load-bearing. With foreign keys on, DROP TABLE
performs an implicit DELETE FROM, so dropping manifests while layers still holds
an ON DELETE CASCADE reference deletes every layer row. Migration 0009 did
exactly that; it went unnoticed because the Jetstream backfill rebuilds layers
from PDS records, so the damage healed itself. PRAGMA foreign_keys is no help:
it is a no-op inside a transaction and migrations run in one. So the new
children are built pointing at manifests_new, the old children are dropped
first, and only then is the old manifests table dropped, by which point nothing
references it. Verified both behaviors before relying on them.
manifest_key is declared NOT NULL as well as PRIMARY KEY, because in SQLite a
PRIMARY KEY column still accepts NULL unless it is INTEGER PRIMARY KEY. That
constraint immediately caught four test helpers inserting manifests without one.
Five queries used MAX(id) as "the newest manifest in this repo", which I had
previously reported as absent after grepping only for ORDER BY. A derived key
has no ordering, so recency now comes from created_at with manifest_key as a
deterministic tiebreak. This is a real behavior change, and a fix: the two
disagree whenever a manifest is indexed out of order, which the backfill does
routinely, and created_at is the push time these queries always wanted. Both
directions are tested, including that ties resolve the same way every run.
InsertManifest and BatchInsertManifests no longer read anything back. The key is
derived from (did, repository, digest), so the writer knows it before the
statement runs: the select-back, its per-DID IN list, and the "manifest missing
id after batch insert" branch all go away, along with the UNIQUE-conflict
fallback that existed only to recover a rowid.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0033 added the column and left the fill to the Jetstream backfill. That is fine
for a running system and wrong for replay: a database several releases behind
runs every pending migration back-to-back at boot, long before any worker
starts. Anything built on top of manifest_key would, on that path, silently
operate on NULLs while working perfectly on a system that had been up for a
while. Establishing a migration's data precondition out-of-band means replay
cannot see it.
The runner now supports a Go step per migration version, running inside the same
transaction as that migration's SQL, after the DDL it depends on and before the
version is recorded. A version is never recorded without its Go half.
0033's step fills manifest_key for every row lacking one. It has to be Go: the
value is a truncated sha256 and SQLite has no hash builtin. It pages through the
table and writes one UPDATE ... CASE per 500 rows, because a statement per row
would be correct and unusably slow against a remote primary.
Verified by unregistering the hook: replay then leaves 3 of 3 seeded manifests
with NULL keys. With it, all three are filled, from a snapshot of the pre-0009
schema forward.
The upserts keep their "OR manifests.manifest_key IS NULL" clause. It is a
self-healing net for rows that somehow arrive without a key rather than the
mechanism anything depends on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
First half of replacing manifests.id with a node-independent identity. Nothing
depends on the column yet: id is still the primary key, and layers and
manifest_references still reference it. Getting the column in place and filled
first means the eventual swap operates on data that is already complete and
already proven unique, instead of doing the fill and three table rebuilds in one
step.
The value cannot be computed by the migration. It is a truncated sha256, SQLite
has no hash builtin, and go-libsql exposes no way to register one. So the fill
uses machinery that already exists: the Jetstream backfill re-upserts every
manifest across the protocol on startup, and both upsert paths now write
manifest_key.
That only works because of one extra clause. Both upserts guard their DO UPDATE
with a WHERE that skips rows where nothing changed, which on a backfill re-run is
nearly every row, so they would have skipped the very manifests that need
filling. Adding "OR manifests.manifest_key IS NULL" is what makes an otherwise
no-op pass populate the column. Verified by removing it: the backfill then fills
zero of three manifests instead of three of three.
The index is UNIQUE even though the column is nullable. SQLite treats NULLs as
distinct, so unfilled rows coexist while every filled row is checked. That makes
production data verify the 16-byte truncation rather than us assuming it: if two
manifests ever derived the same key, it fails loudly at insert instead of
silently attaching one manifest's layers to another after the swap.
AppView logs the unfilled count at startup, since there is no single moment at
which this becomes complete and the follow-up migration is only safe at zero.
ManifestKey replaces the old fat "did|repo|digest" map key rather than sitting
beside it; they were always the same question, answered without asking the
database.
The jetstream tests hand-maintained their own CREATE TABLE statements, which is
the drift problem moved into a test: the copy had already fallen behind (it still
had tags.id) and only failed once a query touched the difference. They use
db.InitDB now, with foreign keys switched off to preserve the behavior the
hand-rolled schema had.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing joined on it. It was selected into a struct field no caller read, and
used only by DeleteTagsNotInList, which fetched surrogate ids, filtered them in
Go with a nested loop over the keep list, and issued one DELETE per row. The
natural key was already enforced by UNIQUE(did, repository, tag), so that
becomes the primary key and the column goes.
An AUTOINCREMENT rowid is allocated by whichever node performs the insert. That
is fine while every write funnels through one writer and stops being a stable
identity the moment they do not, so removing an identifier nobody used is the
cheapest way to shrink that surface before local-write replicas.
DeleteTagsNotInList now diffs against a set and deletes in batches. It still
reads the current tags first rather than issuing one NOT IN over the keep list:
that would need two placeholders per kept tag and would break past the driver's
parameter ceiling for a user with enough tags, and it cannot be chunked, because
each chunk would delete the tags every other chunk meant to keep. An explicit
delete list chunks safely.
idx_tags_did_repo is dropped rather than recreated: the new primary key indexes
(did, repository) as a prefix. It existed only because the primary key used to be
the surrogate id.
The rebuild names its columns explicitly. Column order is not guaranteed to
match between a fresh install and a migrated one, so INSERT ... SELECT * here
could write values into the wrong columns. TestMigration0032PreservesTagRows runs
the migration body against a table in the old shape and checks the contents
survive, which the schema drift test cannot: it compares shape, not data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refresh tokens rotate on use, and DoWithSession serializes refreshes per DID with
an in-process mutex. That is the right mechanism and it protects nothing once
there are two instances: both can refresh the same account at the same time, the
slower one presents a refresh token the auth server has already superseded, gets
invalid_grant, and isAuthError deletes the session. The user is signed out
mid-push, and the session another instance had just legitimately refreshed is
destroyed along with it.
oauth_sessions gains a rev that increments on every write. A store that has read
a session writes with a compare-and-swap against the revision it read and gets
ErrSessionRevConflict if anyone wrote first, so a stale writer can no longer
replace rotated tokens with invalidated ones. The persist callback treats that
conflict as an ordinary outcome rather than an error, since leaving the newer
state alone is exactly right.
The delete path is now guarded by the same signal. An auth error on a session
whose revision has moved since we read it means "someone else refreshed this",
not "this session is dead", so it retries once against the newer tokens instead
of deleting. Exactly once: a second failure means staleness was not the problem,
and looping would hold the per-DID lock while getting the same answer.
The guard is deliberately conservative. A store without revisions, no recorded
revision, a failed lookup, a session that is simply gone: all answer "not
advanced" and keep the previous delete-on-error behavior. Wrongly claiming a
concurrent refresh would keep a genuinely dead session alive with no way out but
waiting; wrongly missing one costs a re-login.
The sentinel lives in pkg/auth/oauth rather than next to the SQLite store,
because pkg/appview/db already imports pkg/auth/oauth and the other direction
would be an import cycle. The db package re-exports it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groundwork for running more than one AppView instance. Nothing is wired to this
yet; the next commit moves the background workers onto it.
Several workers must run on exactly one instance. The Jetstream consumer is the
sharpest case: StatsCache is per-process in-memory state, and the aggregate it
produces is written to repository_stats as an absolute value rather than an
increment, so two consumers would each hold a partial view of the holds and each
write its partial sum as the whole truth, overwriting one another indefinitely.
The webhook dispatcher hangs off the same processor, so a second consumer also
means every webhook fires twice.
Instances contend for a named lease; only the holder runs the worker. Acquire is
a single INSERT ... ON CONFLICT ... WHERE, so two instances racing for the same
expired lease cannot both win: the loser's update matches no rows. The fence
token increments on every change of custody, so a process that stalled past its
TTL discovers on its next renewal that it was superseded, rather than continuing
to act as the holder.
A renewal blackout is treated as a loss. If the database has been unreachable
for longer than the TTL, another instance is entitled to steal the lease and we
must assume it has, even though we cannot ask. Continuing to work in that state
is the one outcome the lease exists to prevent.
Clean shutdown expires the lease in place rather than deleting the row, so a
replacement starts in seconds instead of waiting out the TTL, while the fence
token survives to keep a stalled former holder from matching again.
Timestamps are Unix milliseconds, not TIMESTAMP text. libSQL normalizes
date-like TEXT on the way in, and Go's driver and CURRENT_TIMESTAMP disagree on
format, so a stored expiry and a literal would compare as strings that sort
differently. That comparison is the whole safety property, so it does not get to
be subtle. The cost is a dependency on roughly-synced clocks, the same
assumption Kubernetes leases make; keep the TTL well above any plausible skew.
The lease tests are file-backed rather than :memory:. go-libsql gives every
connection to an in-memory DSN its own private database, so with MaxOpenConns of
8 a second goroutine lands on a connection where the schema was never applied
("no such table"). Every existing test in the package is sequential and reuses
one pooled connection, which is why this has stayed invisible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migrations can be recorded without being executed. That is not hypothetical:
migration 0009 exists to clean up after 0004, which production recorded but
never applied, leaving eleven columns behind that fresh installs never had.
Nothing reported it at the time; it surfaced later as confusing behavior.
InitDB now compares an existing database against schema.sql after migrations run
and logs one warning per difference. Fresh databases skip the check, since they
were just built from schema.sql and agree by construction.
The comparison works by applying schema.sql to a throwaway in-memory database
and introspecting that, rather than parsing the DDL. SQLite's own resolution of
types, defaults and implicit indexes is exactly what we want to compare against,
and a hand-rolled parser would drift from the engine. The introspection is
shared with TestSchemaMatchesMigrations, so the test exercises the same code
that runs at boot.
Warn-only, never fatal. A database merely ahead of or behind schema.sql is
almost always still able to serve traffic, so refusing to boot would turn a diff
that wants a corrective migration into an outage, during a deploy, which is the
worst possible moment to have one.
The README claimed new tables go in schema.sql only. That is wrong in the
direction that hurts: InitDB skips schema.sql entirely once schema_migrations
has rows, so such a table appears on fresh installs, passes every test, and is
silently absent in production. Documented the real rule along with two others
the test cannot enforce: migrations must not return rows (go-libsql rejects them
with "Execute returned rows"), and rebuild migrations must name columns
explicitly, since column order legitimately differs between fresh and upgraded
databases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two migration files shipped as version 28: 0028_add_device_secret_lookup
(08121f3) and 0028_create_stripe_processed_events (12c55ed).
runMigrations keys applied migrations by the integer parsed from the
filename and skips any version already present in schema_migrations, so
the first file to load records 28 and the second is skipped in silence —
no error, no log. loadMigrations enumerates via fs.Glob, which sorts
lexically, so add_device_secret_lookup won and stripe_processed_events
never ran.
On an existing database that leaves stripe_processed_events missing, and
StripeEventSeen then fails closed: the error wraps into ErrWebhookProcessing,
the webhook returns 500, and Stripe redelivers into the same missing table
forever. No subscription, tier, or dispute event is ever applied — defeating
the exact idempotency 12c55ed was written to add.
Fresh installs were unaffected, which is why no test caught it: they take
the applySchema path where schema.sql already has the table and both
version-28 rows are merely recorded as applied.
Renumbered to 0029 rather than renumbering the device migration, so a
database that already ran this build (28 recorded, devices.secret_lookup
present, stripe table missing) picks the migration up on next boot instead
of staying broken. Renumbering the other file would have left that database
with the table still missing and re-run its ALTER on a column that exists.
Adds two guards: one asserting migration versions are distinct, and one
exercising the upgrade path where a duplicate manifests as a missing
schema_migrations row. Both fail on a planted duplicate.
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>
ValidateDeviceSecret ran bcrypt.CompareHashAndPassword against every row in
the devices table until one matched — no WHERE clause. At bcrypt cost 10
(~65ms on the single-core production host) and 244 registered devices, a
device near the end of the scan cost ~15.8s of pure CPU per /auth/token,
which is past Docker's client deadline. Measured on production: 15.7-16.0s
steady state with the appview pinned at 100% CPU for the duration, while
anonymous requests on the same box served in 20ms.
The cost grew linearly with every device registered, and the scan ran in
rowid order, so the newest devices — the ones most likely to be in active
use — paid the most. This is the timeout users were reporting.
Devices now carry secret_lookup = hex(sha256(secret)), indexed, and
authentication fetches the single matching row.
SHA-256 is the verifier here, not merely an index. Device secrets are 32
bytes from crypto/rand, so presenting a value that hashes to a stored digest
requires a preimage or a 2^256 search. bcrypt's work factor only helps when
the input space is small enough to enumerate, which does not apply to a
random 256-bit token, and a database leak exposes no more than before.
The plaintext is not recoverable from a bcrypt hash, so existing rows cannot
be backfilled directly. They are migrated lazily on their next successful
authentication, which any push, pull or login triggers, and the legacy scan
is filtered to un-migrated rows so its cost decays as devices migrate. The
backfill runs after the cursor is closed: issuing it inside the rows loop
deadlocks, because the open cursor holds the connection the write needs.
bcrypt now exists solely to carry legacy rows across and can be deleted once
the table is fully migrated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
1. Removing distribution/distribution from the Hold Service (biggest change)
The hold service previously used distribution's StorageDriver interface for all blob operations. This replaces it with direct AWS SDK v2 calls through ATCR's own pkg/s3.S3Service:
- New S3Service methods: Stat(), PutBytes(), Move(), Delete(), WalkBlobs(), ListPrefix() added to pkg/s3/types.go
- Pull zone fix: Presigned URLs are now generated against the real S3 endpoint, then the host is swapped to the CDN URL post-signing (previously the CDN URL was set as the endpoint, which
broke SigV4 signatures)
- All hold subsystems migrated: GC, OCI uploads, XRPC handlers, profile uploads, scan broadcaster, manifest posts — all now use *s3.S3Service instead of storagedriver.StorageDriver
- Config simplified: Removed configuration.Storage type and buildStorageConfigFromFields(); replaced with a simple S3Params() method
- Mock expanded: MockS3Client gains an in-memory object store + 5 new methods, replacing duplicate mockStorageDriver implementations in tests (~160 lines deleted from each test file)
2. Vulnerability Scan UI in AppView (new feature)
Displays scan results from the hold's PDS on the repository page:
- New lexicon: io/atcr/hold/scan.json with vulnReportBlob field for storing full Grype reports
- Two new HTMX endpoints: /api/scan-result (badge) and /api/vuln-details (modal with CVE table)
- New templates: vuln-badge.html (severity count chips) and vuln-details.html (full CVE table with NVD/GHSA links)
- Repository page: Lazy-loads scan badges per manifest via HTMX
- Tests: ~590 lines of test coverage for both handlers
3. S3 Diagnostic Tool
New cmd/s3-test/main.go (418 lines) — tests S3 connectivity with both SDK v1 and v2, including presigned URL generation, pull zone host swapping, and verbose signing debug output.
4. Deployment Tooling
- New syncServiceUnit() for comparing/updating systemd units on servers
- Update command now syncs config keys (adds missing keys from template) and service units with daemon-reload
5. DB Migration
0011_fix_captain_successor_column.yaml — rebuilds hold_captain_records to add the successor column that was missed in a previous migration.
6. Documentation
- APPVIEW-UI-FUTURE.md rewritten as a status-tracked feature inventory
- DISTRIBUTION.md renamed to CREDENTIAL_HELPER.md
- New REMOVING_DISTRIBUTION.md — 480-line analysis of fully removing distribution from the appview side
7. go.mod
aws-sdk-go v1 moved from indirect to direct (needed by cmd/s3-test).
Evan JarrettandClaude Fable 5.1