test(datastore): pin container manifest size immutability on re-push

Why

ADR-007 rests its "no reconciliation is needed" claim on container_manifests.size never being rewritten after push, and nothing observed the stored size on a re-push: both re-push suites passed identical sizes on the second call, so a rewrite that preserves the other re-push signals passed everything, whether an explicit UPDATE on the conflict path before the fallback SELECT or a persistExisting that consumes params.Size. A true re-push recomputes an identical value, so the extended tests inject a differing Size as a probe and read the stored row back. A new doc comment on datastore.ContainerManifest.Size anchors the immutability and double-counting rules at the value's source.

Decisions

The issue's two "Decisions required" items, recorded here as its deliverable.

Decision 1: container_remote_manifests.size is payload-only for the MVP (S16 stands). The MVP writes a manifest's own payload size at cache fill and never updates it. An index or manifest list stays at its own payload, zero relationship rows are written, and convergence waits on relationship population, which is deferred and tracked in Container remote and virtual: deferred follow-u... (#264) • Unassigned. S16 already states and pins this with an acceptance criterion, so it needs no edit. ADR-007 states convergence as present behavior in two sentences, and ADRs are handbook-synced, so the correction routes to the standing ADR-007 amendment item ADR-007 follow-up: amendments from S12 OCI loca... (#30) • Unassigned. Until that coordinated handbook MR merges and syncs, ADR-007 keeps the stale text.

Decision 2: no per-image aggregate columns now. Per-image manifests_count and image-level size are served by read-time aggregation when their consumers land. Today there are none: the images list serializes id, name, and last_downloaded_at only, and the statistics endpoints are unimplemented. Either revisit trigger reopens the decision: the container-redesign patch confirms an image-list sort or filter by size or count, or profiling of the page-bounded batched fetch fails. Two warnings bind any future implementer. First, an image, repository, or namespace footprint is never SUM(container_manifests.size): a shared blob counts once per referencing manifest and a child manifest counts once for itself and once under each parent index, so a correct image size deduplicates within the image. Second, any future manifests_count must settle whether referrers count before landing: S17 describes the counter as including referrers while the default manifests list excludes them, so the naive count would visibly disagree with the list beside it. No follow-up issue, since no columns are added.

Coverage

Issue "Done when" Where it lands
A test asserts a re-push does not rewrite container_manifests.size TestCreateContainerManifest_IdempotentRePush gains a changeSize arm asserting first.Size, second.Size, and the read-back read.Size all keep the original. The persister re-push subtest passes a differing params.Size and reads the stored row before and after. A conflict-path rewrite (DO UPDATE SET size = EXCLUDED.size) fails second.Size and read.Size.
Remote size semantics settled in one document, contradicted in none Decision 1. S16 is the settled document, and the ADR-007 correction is filed on the standing amendment item.
Per-image schema decision recorded Decision 2.
Double-counting guard at the value's Go source Doc comment on datastore.ContainerManifest.Size. The OpenAPI contract already carries the consumer-facing guard.

Test plan

Both suites against the integration database, plus the integration-tag lint gate at the CI-pinned golangci-lint 2.12 (CI lint does not compile //go:build integration files):

go test -tags=integration -count=1 \
  -run 'TestCreateContainerManifest_IdempotentRePush|TestContainerManifestPersister_PersistManifest' \
  ./internal/datastore/
golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 ./internal/datastore/

Test-only change, so no e2e scenario is added or affected.

Related to #551 (closed)

Context for LLM agents

Rationale

  • A DO UPDATE SET size = EXCLUDED.size upsert as the modeled threat. Rejected as the framing: DO UPDATE ... RETURNING returns a row on conflict, so the pre-existing inserted=false assertion already fails it. The uncaught rewrites were an explicit UPDATE on the conflict path before the fallback SELECT and a persistExisting consuming params.Size, both of which preserve every previously asserted signal. The new size assertions catch exactly those.
  • A handler-level re-push test. Rejected: a true re-push recomputes an identical tree size from the content-addressed payload and immutable child sizes, so it cannot distinguish "never rewrites" from "rewrote the same value". The probe injects the differing size at the datastore seam instead.

Non-goals

  • Per-image manifests_count or size columns. Decision 2 declines them until a named revisit trigger fires.
  • container_remote_manifests code or schema. Decision 1 settles the size semantics without code, and the table's migration is in flight in another author's open MR.
  • The wider integration-tag lint backlog. This MR lint-gates only internal/datastore/, and the backlog is tracked in Sweep the identifier-bearing error wraps in int... (#544) • Hayley Swimelar.

Merge request reports

Loading
Loading