ADR-007 follow-up: amendments from S12 OCI local implementation

Tracking follow-up from docs/dev/oci-spec-gaps.md. Several deviations from ADR-007 surfaced during S12 OCI local implementation that should be reconciled with a coordinated amendment.

ADR-007 lives in the handbook repo and is synced into docs/adr/ daily. Each amendment below should land via an MR against the handbook repo.

Scope note. This issue is the collection point for ADR-007 amendments from the container workstream, not only the ones S12's own implementation surfaced. Divergences recorded by the container remote spec (S16) are tracked here too — items 6 onward below — so a single coordinated handbook MR carries them all rather than racing several MRs against the same ADR file.

1. Shorten artifacts_registry_ prefix to ar_

Type: adr-amendment

Gap or concern: ADR-007 mandates the artifacts_registry_ prefix on all production tables. Combined with the 64-partition fan-out, the prefix overflows the 63-char identifier cap on indexes and constraints.

Decision or deferral: Shorten the prefix to ar_. This keeps a namespace marker without overflowing the 63-char cap. The initial choice (recorded 2026-05-04 via !141 (merged) review) was to omit the prefix entirely. Reviewer feedback steered to ar_ to preserve the marker for shared-database visibility and grep. Resolve via a post-merge ADR-007 amendment that adopts ar_ as the canonical prefix.

Pointer: migrations currently land without a prefix in bef7c4f (database branch). Update pending in !125 (closed) to apply the ar_ rename.

2. Drop sha256 recovery column from upload_sessions

Type: adr-amendment

Gap or concern: ADR-007 specifies sha256 BYTEA NULL on upload_sessions, set after the blob is moved to its final content-addressable path so the crash recovery path can detect "blob already moved, finish cleanup and delete row." The migration omits the column. S12 §Upload Sessions Contract supersedes ADR-007 here: it specifies a binary existence model where row presence alone tracks lifecycle, and successful completion deletes the session row in the same transaction that creates the blob_storage_blobs and blob_storage_attachments records.

Decision or deferral: Omit the column. Crash recovery probes the CAS for the expected content-addressed path rather than reading a flag from the session row. Resolve via a post-merge ADR-007 amendment that aligns with the S12 binary existence model.

Pointer: commit bef7c4f, upload_sessions definition in 20260428120100_oci_local_schema_container.sql.

3. Clarify day-one workspaces requirement vs. deferral

Type: adr-amendment

Gap or concern: ADR-007 §Workspaces and §Workspace repositories require both tables "from day one," with every namespace getting a default workspace and every repository linked through workspace_repositories. The OCI conformance harness does not exercise workspaces (OCI clients have no workspace concept), so the milestone implementation skips them and routes every container repository directly off repositories.id.

Decision or deferral: Defer both tables. A follow-up migration will add them and backfill a default workspace per existing namespace plus a workspace link per existing repository. The namespace-creation path will need to atomically create the default workspace at that point. Resolve via a post-merge ADR-007 amendment that either keeps the day-one requirement (and documents the deferral as a one-time migration step) or relaxes it.

Pointer: n/a (apply via a future migration plus namespace-creation refactor).

4. Drop object_storage_key and content_type from blob_storage_blobs (resolved in code)

Type: adr-amendment

Gap or concern: S06 (MR !108) drops object_storage_key (the path is a pure function of (namespace_id, sha256) via PathResolver) and content_type (uploads cannot know the manifest context, and the manifest pull reads Content-Type from container_manifests.media_type).

Decision or deferral: Resolved 2026-04-30. The Step 1 migration no longer creates either column; internal/datastore/blobstorage.go, internal/cas/blob.go (StoreBlob), and internal/cas/session.go (persistCommit) no longer write them; the blobUploadContentType constant is gone. The ADR-007 amendment to align the upstream ADR with the migration shape still needs to land.

Pointer: migration at internal/datastore/migrations/sql/20260428120000_oci_local_schema_root.sql, internal/datastore/blobstorage.go, internal/cas/blob.go, internal/cas/session.go.

5. Record non-composite (expires_at) index on upload_sessions (resolved in code)

Type: adr-amendment

Gap or concern: S06 (MR !108) specifies the index as non-composite (expires_at) because the upload purger (ADR-011) scans globally with WHERE expires_at < $now LIMIT N rather than per-namespace. The Step 1 migration originally shipped a composite (namespace_id, expires_at) index, inherited from ADR-007.

Decision or deferral: Resolved 2026-04-30. The migration now creates index_upload_sessions_on_expires_at ON upload_sessions (expires_at). The composite index covered the same access pattern with worse efficiency for the global purger; the simpler shape is a superset of nothing currently in the codebase, and S20 (purger spec) will rely on the cheaper plan when it lands. The ADR-007 amendment to record the index shape still needs to land.

Pointer: migration at internal/datastore/migrations/sql/20260428120100_oci_local_schema_container.sql.

6. container_remote_blobs: drop media_type, add created_at

Type: adr-amendment

Gap or concern: ADR-007 gives container_remote_blobs a media_type column (text, NOT NULL, limit 255) and no created_at. S16 inverts both.

Decision or deferral: Drop media_type; add created_at timestamptz NOT NULL DEFAULT NOW(). The same physical blob can be referenced by manifests carrying different descriptor media types, so the manifest payload stays the authoritative source for a blob's type — which is why the hosted container_blobs table carries no media_type either, and the remote cache should match it. created_at is already on container_remote_manifests and is needed on the blob table for the same chronological provenance and GC-ordering scans.

Pointer: docs/specs/S16-container-remote.md, Data Model → container_remote_blobs. No migration exists yet; S16 owns the migration when it lands.

7. container_remote_*: FK coverage and keyset tiebreakers

Type: adr-amendment

Gap or concern: Nothing in ADR-007's index list covers the parent FK on any of the three container_remote_* child tables, and two of its indexes lack a tiebreaker.

  • container_remote_images lists only the partial unique index; nothing covers the parent FK (namespace_id, container_remote_repository_id).
  • container_remote_blobs and container_remote_manifests list the partial unique index plus (namespace_id, blob_storage_attachment_id) and (namespace_id, blob_sha256), but neither covers the parent FK (namespace_id, container_remote_image_id).
  • container_remote_manifests' two scan indexes — (namespace_id, soft_deleted_at DESC) and (namespace_id, created_at DESC) — carry no id tiebreaker.

Decision or deferral: Add one full (non-partial) FK-coverage index — (namespace_id, container_remote_image_id) on container_remote_blobs — and add id as a trailing tiebreaker to both container_remote_manifests scan indexes.

container_remote_images and container_remote_manifests get no index of their own. Item 9's last_downloaded_at range-scan indexes are non-partial and lead with those tables' two FK columns, so the referential-integrity check on a parent delete descends one of them on its leading keys and never reads the third. A separate (namespace_id, <parent FK>) index would be a strict key-column prefix of it: no access path of its own, and a set of entries to maintain on every last_downloaded_at bump, a write that cannot be HOT precisely because that column is indexed. container_remote_blobs keeps its own because it has no retention index for the check to descend instead.

Measured on PostgreSQL 17 with 20,000 container_remote_images rows in one partition and a childless parent, the check is an index scan on the range-scan index at Buffers: shared hit=2 in 0.020 ms. With that index dropped the same check is a sequential scan of the partition, ~1,200 buffers.

The partial unique indexes cannot serve the check on any of the three tables, since they exclude soft-deleted rows, and Postgres does not auto-index FK columns. The tiebreaker gives keyset pagination a stable cursor when sort values tie, which bulk cache-fills produce routinely.

Note for whoever writes the handbook MR: items 7 and 9 have to land together, and this item must not be published as "three FK-coverage indexes". Doing so leaves ADR-007 carrying an index on container_remote_images and container_remote_manifests that no migration creates, and the next spec transcribing ADR-007 reintroduces the redundant pair. The shipped container_remote_images migration builds exactly two indexes: the partial unique and the range-scan.

Pointer: docs/specs/S16-container-remote.md, Data Model → the index table under each of the three tables, and the index-refinements list above them.

8. container_remote_*: seven missing CHECK constraints

Type: adr-amendment

Gap or concern: S16 specifies seven CHECK constraints ADR-007 has no equivalent for, in two groups. Three are semantic: CHECK (cache_validity_hours >= 0) on the remote-repository tables (which S13 mandates), CHECK (size >= 0) on container_remote_manifests (matching hosted), and the control-character check on container_remote_tags.upstream_etag (CHECK (upstream_etag IS NULL OR (position(chr(13) IN upstream_etag) = 0 AND position(chr(10) IN upstream_etag) = 0 AND upstream_etag !~ '[\x01-\x1f\x7f]'))). Four are minimum-length bounds on text columns ADR-007 publishes with an upper bound only. This item does not cover auth_status’s range CHECK, which item 10 carries.

  • container_remote_repositories.cache_validity_hours — ADR-007 gives NOT NULL, DEFAULT 24 with no CHECK. S13 requires the value be validated >= 0. Without the constraint a negative value makes every tag lookup permanently stale, so it revalidates on every request.
  • container_remote_manifests.size — ADR-007 gives NOT NULL, updated as children are cached with no CHECK. The hosted container_manifests.size carries CHECK (size >= 0); the remote column should match.
  • container_remote_tags.upstream_etag — ADR-007 gives nullable, limit 255 with no CHECK. S16 forbids every C0 control and DEL in the value, because the stored ETag is replayed into the outbound If-None-Match header during stale-tag revalidation, so a hostile upstream could otherwise inject headers into a later request. The rejected set is wider than CR and LF, and the difference matters: net/http runs httpguts.ValidHeaderFieldValue over an outbound header value, refusing every byte below 0x20 except tab, plus 0x7f. A CR/LF-only predicate admits a value carrying 0x01, 0x1f, or 0x7f that then fails that check instead — the request never leaves the process, upstream_checked_at is never bumped, and every later revalidation of the tag retries the same failure, so an upstream controlling its own ETag can pin a cached tag into permanent revalidation failure. RFC 7232 admits only %x21 / %x23-7E / obs-text in an ETag, so no control character is ever legitimate here. The shipped constraint is check_container_remote_tags_upstream_etag_no_ctl, whose predicate and name both match check_maven_remote_files_upstream_etag_no_ctl. Tab is refused too, one byte stricter than the outbound check rather than equal to it — RFC 7232 excludes tab from an ETag, so refusing it costs no legitimate value. Do not "align" the predicate to ValidHeaderFieldValue exactly by re-admitting tab. NUL needs no arm: PostgreSQL text cannot hold one, so such an INSERT fails in the protocol layer before any CHECK runs.
  • container_remote_manifests.media_type — ADR-007 gives NOT NULL, limit 255. S16 specifies minimum length 1, and the shipped migration carries CHECK (char_length(media_type) >= 1 AND char_length(media_type) <= 255) as a single constraint. The value is server-recorded rather than client-supplied, so it carries none of the name columns' lookup-key exposure, but an empty value would cache a manifest no client can be served correctly (an empty Content-Type), and the partial unique digest index keys on digest, not media_type, so a poisoned empty-media_type row would hold that digest's only cache slot until explicitly evicted.
  • container_remote_images.name — ADR-007 gives NOT NULL, limit 255. S16 specifies minimum length 1, and the shipped migration carries CHECK (char_length(name) >= 1 AND char_length(name) <= 255) as a single constraint. An upper bound alone admits the empty string, which passes char_length(name) <= 255 and inserts cleanly as a row nothing can ever resolve: the cache lookup that would find it is keyed on the name the request carried, and no request carries none. It would also hold the partial unique index's live slot for the empty name in that repository, against nothing.
  • container_remote_tags.name — ADR-007 gives NOT NULL, limit 255. Same shape and same reasoning as container_remote_images.name; the spec specifies minimum length 1. Not yet shipped — container_remote_tags is a later step's table.
  • container_remote_repositories.auth_url — ADR-007 gives nullable, limit 1024. The shipped migration carries check_container_remote_repositories_auth_url_min_length CHECK (char_length(auth_url) >= 1) alongside the upper bound, so an empty string cannot stand in for a real URL while CHECK ((auth_status = 2) = (auth_url IS NOT NULL)) reports that one was supplied.

Decision or deferral: Record all seven in ADR-007.

Note on the minimum-length group: scoped to these four container columns on purpose. ADR-007 publishes the same bare limit 255 on other formats' name columns, but those agree with their shipped schemas — npm_remote_packages.name bounds above only in code as well — so they are not divergences and should not be swept into this amendment.

Note for whoever writes the handbook MR: the upstream_etag gap is not container-only. ADR-007 declares upstream_etag as nullable, limit 255 with no CHECK on maven_remote_files, npm_remote_metadata_files, and npm_remote_files as well. Worth fixing all four in one pass — but publish the control-character predicate, not the CR/LF one, because the four do not currently agree and two of them already ship the wider shape:

Table Shipped constraint Coverage Spec
container_remote_tags check_container_remote_tags_upstream_etag_no_ctl C0 + DEL S16
maven_remote_files check_maven_remote_files_upstream_etag_no_ctl C0 + DEL S14
npm_remote_metadata_files check_npm_remote_metadata_files_upstream_etag_no_crlf CR/LF only S15
npm_remote_files check_npm_remote_files_upstream_etag_no_crlf CR/LF only S15

Correction (2026-08-13): this note previously read "S14 and S15 both specify the CR/LF constraint on their own upstream_etag columns for the same reason." That is wrong about S14. S14 §maven_remote_files specifies the control-character predicate, and maven_remote_files has shipped _no_ctl since 20260805120000_create_maven_remote_files.sql. Only S15 specifies CR/LF-only, on both npm tables. Publishing "the CR/LF constraint" for all four would have made ADR-007 narrower than both Maven's spec and its shipped schema, and the next spec transcribing the ADR would reintroduce the header-injection gap the widening closes — the same trap item 7's note warns about.

The remaining divergence is npm's two tables, and it is a code change in the npm slice rather than only an ADR row: both the S15 spec text and the two migrations need widening. Their shipped predicate also differs in spelling — upstream_etag NOT LIKE '%' || chr(13) || '%' AND ..., with no IS NULL disjunct, so the null case falls out of SQL's three-valued logic instead of being stated — where container and Maven both spell the disjunct out. Whoever widens npm should adopt the container/Maven spelling rather than adding an arm to the NOT LIKE form.

Pointer: docs/specs/S16-container-remote.md, Data Model → the schema tables for container_remote_repositories, container_remote_images, container_remote_manifests, and container_remote_tags.

9. container_remote_*: download-signal support for the remote cache

Type: adr-amendment

Gap or concern: ADR-007 declares container_remote_images.last_downloaded_at and says it is "used by keep_last_downloaded_at lifecycle rules and cache retention evaluation (ADR-010)", but the schema it publishes cannot serve that access pattern efficiently, and the manifest-granularity equivalent is missing entirely.

  • No index supports the retention scan. ADR-007's index list for container_remote_images carries only the partial unique index, so an eviction or retention sweep aging out cold cached content does a full per-namespace scan. The hosted container_images table already carries (namespace_id, container_repository_id, last_downloaded_at NULLS FIRST) for exactly this.
  • container_remote_manifests has no last_downloaded_at column at all, so a cached manifest read has nowhere to record its timestamp. The hosted container_manifests table carries one, itself recorded as an ADR-007 amendment by S12.

Decision or deferral: Add last_downloaded_at timestamptz (nullable) to container_remote_manifests, mirroring hosted's column but not hosted's buffered annotation — see item 13. Add bounded range-scan indexes (namespace_id, container_remote_repository_id, last_downloaded_at NULLS FIRST) on container_remote_images and (namespace_id, container_remote_image_id, last_downloaded_at NULLS FIRST) on container_remote_manifests, mirroring the hosted equivalents. The cache is the primary consumer of this access pattern — more so than the hosted tables — since eviction is what a proxy cache exists to support.

The column is written directly by the serving read, not through a buffered or async path. An earlier revision of this item said "(nullable, buffered)", which would have published a mechanism no format implements the moment the amendment landed. Item 13 corrects the same claim where ADR-007 already makes it about container_remote_images.last_downloaded_at; the two have to move together, or the ADR ends up describing one of the pair as buffered and the other as direct.

Pointer: docs/specs/S16-container-remote.md, Data Model → container_remote_manifests columns and the index tables for both tables; design section → Download signals.

10. container_remote_repositories: add auth_status and its two CHECKs

Type: adr-amendment

Gap or concern: ADR-007 publishes auth_url (text, nullable, limit 1024) as the only auth column on container_remote_repositories. A nullable auth_url carries two meanings at once — "challenge discovery has not run against this upstream" and "discovery ran and the upstream needs no token" — with nothing to tell them apart. S16's challenge-discovery flow persists nothing on the no-token outcome, so against an upstream that never answers 401 every token-cache miss, revalidation, tag listing and referrers call re-runs the unauthenticated discovery HEAD, for the life of the remote. S16's criterion that a remote issues at most one discovery HEAD is therefore satisfiable only for upstreams that do answer 401.

Decision or deferral: Add auth_status smallint NOT NULL DEFAULT 0, values 0=unknown, 1=none, 2=bearer, plus two CHECKs: CHECK (auth_status IN (0, 1, 2)) bounds the column to its defined values, and CHECK ((auth_status = 2) = (auth_url IS NOT NULL)) ties the verdict to the endpoint in both directions. The two do not imply each other — the pair CHECK leaves the domain open, since any value other than 2 satisfies it whenever auth_url is null, so 3 or -1 would store cleanly and then fall through the read side's switch on the verdict. The column memoizes the discovery verdict so it is learned once and read from persisted state thereafter, and the CHECK ties the pair so neither is written without the other. Reset it to 0 wherever auth_url is cleared — the url-change clear and the 404-on-token-exchange retry — because a surviving bearer verdict leaves the row failing its own CHECK, and a surviving none verdict lets a new upstream that does require a token be pulled anonymously. A three-state smallint follows last_health_status on this same table (0=unknown, 1=healthy, 2=unhealthy) rather than a distinguished auth_url sentinel, which would put a non-URL value in a URL-typed column and leave every read site special-casing it. Container-only: neither Maven nor npm negotiates upstream auth, so neither has a discovery verdict to memoize.

Pointer: docs/specs/S16-container-remote.md, Data Model → container_remote_repositories, and Design → Auth-challenge and token-exchange; migration internal/datastore/migrations/sql/20260804120000_create_container_remote_repositories.sql.

11. container_remote_repositories: record CHECK (last_health_status IN (0, 1, 2))

Type: adr-amendment

Gap or concern: ADR-007 declares last_health_status (smallint, NOT NULL, DEFAULT 0) on all three remote-repository tables with no CHECK. S13 owns the status taxonomy and enumerates exactly three values — 0=unknown, 1=healthy, 2=unhealthy — but neither document bounds the column, so a value outside the set stores cleanly and then falls through the read side's switch on the status.

Decision or deferral: Record CHECK (last_health_status IN (0, 1, 2)) on the container block only. The probe writes the value, which makes it service-written on the same terms as auth_status in item 10, and nothing else in this schema leaves an enum-style column unbounded. The npm and Maven declarations stay as published because those two tables are not backfilled alongside it — both are empty pre-production, so the change is one ADD CONSTRAINT ... NOT VALID plus a VALIDATE that scans nothing, but the tables belong to the npm and Maven slices. Tracked in #486 (closed); until it lands the three tables disagree.

Pointer: docs/specs/S16-container-remote.md, Data Model → container_remote_repositories; migration internal/datastore/migrations/sql/20260804120000_create_container_remote_repositories.sql. The matching S13 spec-text amendment, naming the bounded shape as the one a later remote slice copies, is tracked in #320 (closed).

12. container_remote_manifests.size: reword convergence as future behavior

Type: adr-amendment

Gap or concern: ADR-007 states convergence as present behavior in two places. The container_remote_manifests bullet says the value is "the manifest's own payload at cache time plus each child's size as children arrive" and "converges to the full tree footprint progressively as children are fetched", and the artifact-level storage accounting section repeats it ("The remote cache mirrors this with container_remote_manifests.size, which has progressive semantics"). No relationship rows or parent-update writer exist in the MVP: a cached index or manifest list stays at its own payload size, per the S16 acceptance criterion. Decided in Artifact-level accounting — Container/OCI: lock... (#551 - closed) • Hayley Swimelar (payload-only for the MVP, S16 stands unedited).

Decision or deferral: Reword both sentences so the value starts at the manifest's own payload and stays there until relationship population lands, at which point it converges progressively. Relationship population is deferred and tracked in Container remote and virtual: deferred follow-u... (#264) • Unassigned.

Pointer: docs/adr/007_database_schema.md, the container_remote_manifests bullet and the artifact-level storage accounting cross-reference; docs/specs/S16-container-remote.md, size semantics and the payload-only acceptance criterion.

13. last_downloaded_at: the buffered/async mechanism ADR-007 names does not exist

Type: adr-amendment

Gap or concern: ADR-007 flags container_remote_images.last_downloaded_at as buffered in its ERD and says it is "maintained via buffered/async writes (same pattern as repositories.downloads_count) to avoid hot-row contention". The mechanism that phrase names is a fast intermediate store merged back by a background process, and nothing implements it — for this column or for any other, in any format.

  • All three remote slices write the column inline on the serving read. Maven and npm remote already ship that way, and S16 mandates it for container: cache eviction reads the column, so a remote cache that waits for a buffered pipeline runs with its retention signal permanently NULL and a sweep evicts the hottest content first.
  • The direct write is the contention the ADR's sentence says the mechanism avoids, and the blob route takes it once per layer against a single image row.
  • The code is not the thing to change. The write pattern is deliberate and matches both siblings; it is the ADR's text that describes a pipeline no format has.
  • repositories.downloads_count, the column the ADR compares against, has no executing writer in any format either — it is gated on S18 — so the comparison points at a second unimplemented mechanism rather than a working precedent.

Decision or deferral: Reword the ERD flag and the accompanying sentence so the column is described as written directly on each served read, with hot-row contention named as a known cost rather than something avoided. Do not describe the pattern as shared with repositories.downloads_count. Applies to container_remote_images.last_downloaded_at and to the container_remote_manifests.last_downloaded_at column item 9 adds — both, in the same MR, since a split leaves one of the pair published as buffered and the other as direct.

Pointer: docs/adr/007_database_schema.md, the container_remote_images ERD flag and the buffered/async sentence; docs/specs/S16-container-remote.md, Follow-ups → buffered-counter pipeline, which records this divergence and states that the mechanism correction belongs to this amendment beside the schema items.

Related to #19 (closed).

Edited by Radamanthus Batnag