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 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 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](https://gitlab.com/gitlab-org/ops/artifact-registry/-/merge_requests/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](https://gitlab.com/gitlab-org/ops/artifact-registry/-/merge_requests/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; 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. ## 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 gitlab-org/ops/artifact-registry#551+s (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 gitlab-org/ops/artifact-registry#264+s. **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.