## The defect `container_images.last_downloaded_at` and `container_manifests.last_downloaded_at` are declared in the schema and have no production writer anywhere in the tree. Every row of both tables holds `NULL`, and it stays `NULL` for the life of the row. Both columns are read. The management API projects them and serializes them, and the OpenAPI contract marks the field required on both resources. So the columns are a permanent `null` on a field the contract promises. This is a different defect from the one issue [#632](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/632) tracks. That issue is about a lost write on a column that does have a writer. Here there is no write to lose. ## What the schema declares Both columns are `timestamptz`, nullable, with no `DEFAULT`. - `internal/datastore/migrations/sql/20260526120100_oci_container_images.sql:17` declares `last_downloaded_at timestamptz,`. - `internal/datastore/migrations/sql/20260526120300_oci_container_manifests.sql:26` declares the same. Both tables also carry a retention index on the column, and both are total rather than partial: ```sql CREATE INDEX index_container_images_on_ns_id_cr_id_last_downloaded_at ON ONLY public.container_images USING btree (namespace_id, container_repository_id, last_downloaded_at NULLS FIRST); CREATE INDEX index_container_manifests_on_ns_id_ci_id_last_downloaded_at ON ONLY public.container_manifests USING btree (namespace_id, container_image_id, last_downloaded_at NULLS FIRST); ``` ## No production writer, and the controls that make the negative real A negative claim needs its controls, so here are the ones this rests on. 1. **The single-column jet census.** `git grep -nE "UPDATE\([^)]*LastDownloadedAt" -- internal cmd` returns twelve sites across ten distinct tables, every one in `internal/datastore` and none of them on these two tables. The same invocation matches all ten tables that do have a writer, which is the positive control. 1. **A positive control on `container_images` itself.** `table.ContainerImages.SoftDeletedAt` is an `UPDATE` target at `internal/datastore/container_image.go:696`. So the pattern finds an `UPDATE` on `container_images` where one exists. What it does not find is one on `last_downloaded_at`. 1. **The insert paths enumerate their columns and omit it.** `upsertContainerImageInsert` inserts `ID`, `NamespaceID`, `ContainerRepositoryID` and `Name`, then ends `DO_NOTHING()`. `createContainerManifestStmt` inserts fourteen named columns and ends `DO_NOTHING()` as well. Neither is an `ON CONFLICT DO UPDATE`, and neither names the column. 1. **A migration DML census.** The census over the migration SQL returns seven real `UPDATE` statements, and two of them are `size_bytes` backfills. So backfills of a buffered column exist in this repository, and the pattern finds them. None of the seven targets a `last_downloaded_at`. 1. **Triggers.** `structure.sql` carries four triggers, and none of them is on either table. ## One honest weakening A test-only writer exists for `container_images`. `internal/datastore/container_image_integration_test.go:595` runs `UPDATE container_images SET last_downloaded_at = $3 WHERE namespace_id = $1 AND id = $2` as a fixture helper. The file is a `_test.go` file, so it is not compiled into the binary and it cannot run in production. A future reader who greps for a writer will find that line, which is why it is recorded here. `container_manifests.last_downloaded_at` has no writer in any file at all. ## Both columns are read The management API serializes the field on both resources: - `internal/managementapi/container_resources.go:61` for the hosted image. - `internal/managementapi/container_resources.go:113` for the hosted manifest. - Lines 122 and 136 do the same for the two remote tables, which do have writers. `api/openapi/v1.yaml` lists `last_downloaded_at` under `required` on `ContainerImage` and on `ContainerManifest`. An integration test already pins the outcome. `internal/managementapi/container_read_handlers_integration_test.go:268` asserts that "a freshly upserted row's DB-default NULL serializes as null". ## Why it matters ADR-007 line 497 states what the `container_images` index is for, verbatim: > index on `(namespace_id, container_repository_id, last_downloaded_at NULLS FIRST) WHERE soft_deleted_at IS NULL` — support `keep_last_downloaded_at` lifecycle rule evaluation; returns only aged-out images via a bounded range scan rather than scanning every image in the repository and filtering row-by-row. `NULLS FIRST` groups never-downloaded images with the oldest rows so both are returned by the same range scan. Every hosted image is permanently `NULL`. So every hosted image lands in the oldest group of a scan built to return aged-out images. Two qualifications keep that claim inside what the sources say. First, no rule evaluator reads the column on `main` today, so nothing acts on the `NULL` yet. While no `keep_last_downloaded_at` evaluator exists, the exposure is latent. Once one lands, it starts on a table where every row is in the oldest group. Second, ADR-007's "`NULL` is treated as the oldest possible download time" sentence sits at line 866 and is written about `maven_packages` alone. It does not generalise by its own text, and this issue does not lean on it. Line 497 is the citation that is about `container_images`, and it is enough. There is an asymmetry worth recording. ADR-007's `container_manifests` bullet at line 499 lists no index of that shape, although the shipped schema creates one. A second difference sits inside line 497 itself: the ADR describes the image index with `WHERE soft_deleted_at IS NULL`, and the shipped index is total. ADR-010 line 29 sets what a wrong eviction costs, verbatim: > Closed beta ships permanent delete only: every delete permanently removes its target with no recovery path, and deleting a repository deletes its contents. The same paragraph names two guards, a typed repository-name confirmation in the UI and an explicit destructive intent on the API. Both guards are about a deliberate delete that a person asks for. Neither one reaches a retention rule that evicts a row on its own, which is the case here. ## The counter-argument, answered The first objection to this issue is that the absence is deliberate, and that objection is correct as far as it goes. `docs/specs/S12-container-oci-hosted.md`, in "Counter and Timestamp Updates", says that `repositories.downloads_count` and the two columns "are owned by S18, which will be specced separately and has no spec yet". The same section says "The S12 handlers emit no counter events and update none of these columns", then enumerates the exact writes a Blob GET or HEAD and a Manifest GET or HEAD owe. `api/openapi/v1.yaml:3474-3481` documents the absence to API consumers as well. S18 has no spec file under `docs/specs/`. Its two work items, [#292](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/292) and [#293](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/293), are unassigned stubs. So the absence is a recorded deferral, not an oversight. The argument for acting now is that the deferral was recorded before anything gave `NULL` a destructive meaning. ADR-007 line 497 and ADR-010 line 29 now give it one. ## A drift note on the contract text The OpenAPI description of the field is stale on one half. `api/openapi/v1.yaml:3474-3481` says: > When the artifact was last downloaded, or `null` when nothing is recorded. Only npm downloads record it today, so a container or Maven artifact reads `null` even after it has been pulled, until the writer for its format ships. The Maven half is now false. `MavenPackageStore.BumpAccessTimestamps` at `internal/datastore/maven_packages.go:249` executes both `UPDATE` statements through `instrumentExec` on `main`. Container is the only half of that sentence still true. ## Precedent for the fix shape This is an option rather than a demand, and the shape is already in the tree one format over. Issue [#971](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/971), "Maven hosted: last_downloaded_at is never written (no-op stub)", is the same defect for Maven. Its fix shipped an interim direct `UPDATE`, dispatched off the request path through `bufferedUpdate`, and kept a `TODO(s18-buffered-counters)` swap marker. It did not wait for S18. That shape is one option here. Whoever picks up this issue should weigh it against a wait for the S18 spec. The two options differ in what they commit the S18 design to. ## Related work - [#632](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/632) — the parent question this issue was split from, and a different defect: a lost write on a column that does have a writer. - [#971](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/971) — the same defect one format over, with a shipped interim fix. - [#551](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/551) — closed. Its "Out of scope" names both columns and says "Neither has a writer today", then the issue closed without tracking the fix. - [#680](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/680) — its surface-3 bullet says both tables "carry the index with no bump method yet" and that "Whatever adds the hosted download path inherits this". - [#292](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/292) and [#293](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/293) — the S18 spec and implementation stubs, both unassigned. - [#1026](https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/1026) — adjacent, on `repositories.downloads_count`. ## Measurement Code claims: AR `origin/main` `61ee043a2735bc21e359dc9b9fdf5a7d723f3552`. The census and its controls were re-run at that sha immediately before filing: `git grep -nE "UPDATE\([^)]*LastDownloadedAt" -- internal cmd` returns the same twelve sites across the same ten tables, and the positive control `UPDATE(table.ContainerImages.SoftDeletedAt)` at `internal/datastore/container_image.go:696` still matches, so the pattern still finds an `UPDATE` on `container_images` where one exists. `origin/main` moved 26 commits between `516caf08ce8604bebedd6c6dd1da418c7b86870a` and `61ee043a2`, and none of the 21 paths those commits changed is a path cited above, so every file:line coordinate here is unmoved. The earlier passes were at `516caf08c` and, for the census and controls, at `9431b68aeefcebcf712c0adf7e7ac4874bb9d318`. ADR text: handbook `35bc8de8c37f519abe554ab5be1b4c032307229a`, and the `docs/adr/` copy of ADR-007 is byte-identical to it. _This is a bot message 🤖 — /smurfit_