chore(managementapi): add bulk-delete job args and the worker seam

Why

The bulk-delete routes answer 202 and apply their entries in a job, so before any route or worker can exist, three things have to be settled: what a bulk-delete job's payload looks like, how a batch that does not fit the 64 KiB payload cap is handled, and where a one-shot worker registers. This lands those three and nothing else. The composition root had no worker-registration seam at all, only the periodic one.

Two payload constraints drive the design, and both are asserted rather than argued:

  • A manifests batch must never split. Manifest deletes apply in dependency order (indexes before the manifests they index) and River orders nothing between jobs. 1,000 canonical sha256:<hex> strings marshal to about 74 KB, over the cap. Carried as raw 32-byte values the same batch is 47,249 bytes, 18,287 under the cap, so it stays in one job.
  • A tag-name batch may split, so it does. Tag deletes are order-free and idempotent per entry. 1,000 names at the 255-character stored bound is about 257 KB, so ChunkTagNameJobs splits by byte budget: 4 jobs, largest 65,520 bytes, 16 bytes of headroom. The whole set enqueues in the one transaction that answers the request, so the 202 stays all-or-nothing.

Plan: docs/plans/2026-08-10-s17-phase4-artifact-writes.md, ### Step 29: Bulk worker plumbing. Spec: docs/specs/S17-rest-management-api.md, ### Bulk delete (Phase 4).

What is worth a reviewer's attention

Reading order. This is 2,608 reviewable LOC against the project's 500 ceiling, which docs/dev/development-model.md says to split or justify. The plan defines Step 29 as one step spanning all three file groups, so splitting would deviate from what was reviewed. It reads in two independent passes: internal/managementapi/bulk_args.go plus its test (2,002 LOC, the payload types, digest encoding, and the chunker), then cmd/artifact-registry/* plus docs/dev/background-jobs.md (606 LOC, the registration seam). 1,410 lines are the one test file.

The chunker budgets against a worst-case acceptance timestamp, not the one in hand. The acceptance instant is read from the database clock inside the enqueue transaction, which is after the chunks are built. A zero time.Time encodes to 20 characters and a real RFC 3339 nanosecond stamp with a zone offset to 35, so a chunk measured unstamped can cross the cap the moment the stamp lands. worstCaseAcceptedAt is the widest value time.Time.MarshalJSON can emit, and TestChunkTagNameJobs_LargestChunkSurvivesTheStampedTimestamp is the regression that catches a chunker measuring the zero value.

Validate runs after that stamp, not before it. It is the enqueue path's last check before the insert and a worker's first check on decode. A pre-stamp call rejects every delete_all payload, because a delete_all scope is a created_at <= accepted_at predicate and an unstamped one either applies to nothing while reporting success, or reads the absent bound as no bound.

managementBulkWorkerRegistrations returns an empty set on purpose. It is the named append site the three family-worker steps fill, one region per family, so their diffs stay disjoint. Its doc comment records the two sequencing facts a family will hit: wireJobs takes the collected slice by value, so a later append is invisible, and neither the call site nor the wiring holds a database handle, so a worker needing database-backed collaborators has to be handed them.

Identifiers are uuid.UUID, settled now rather than later. The payload is a durable wire format in river_job.args that six later steps consume. The package already types every identifier this way, including in JSON-tagged response structs, and the repo's only other jobs.Args type carrying ids (maven.ReconcileArgs) uses the same spelling with the same tags. A malformed value now fails at River's args decode instead of reaching a query as a cast error the job retries for about 20 days. The fixed encoded width also makes an over-large job envelope unreachable, so errBulkChunkEnvelopeTooLarge is now defensive only and is fired directly by test.

Correction to the test(managementapi) commit body. It records PackageID on the files collection as "neither required nor forbidden". That is wrong: the code forbids it, uniformly with every other surplus parent scope, because the files route is scoped by version and carries no package segment. bulk_args_internal_test.go's "files with a surplus package scope" case pins the rejection. That body's payload figures (47,234 bytes, 18,302 of headroom) are also 15 bytes stale, because the digest assertion now measures the stamped worst case: the current numbers are 47,249 and 18,287.

Spec coverage

Scoped to ### Bulk delete (Phase 4) and the criteria the plan's Step 29 names. The other criteria belong to the plan's decode, handler, and worker steps, each carrying its own table.

Criterion Covered by
AC #47 (each selector takes its artifact's identifier; non-canonical digest rejected) TestBulkDeleteContainerArgs_Validate, TestBulkDeletePackageFamilyArgs_Validate, TestEncodeManifestDigests_Rejects. Non-canonical UUID and tag grammar belong to the selector decode
AC #55 (closed) (a batch of exactly the cap is accepted; empty rejected) TestBulkDeleteContainerArgs_FullDigestBatchFitsOneJob, TestChunkTagNameJobs_FullMaxLengthBatchChunksUnderCap, TestChunkTagNameJobs_Rejects
AC #56 (closed) (repeated entries applied as a set) The decode collapses duplicates. ChunkTagNameJobs deliberately preserves them, pinned by TestChunkTagNameJobs_ReassemblesEverySize and TestChunkTagNameJobs_ReassemblyProperty
AC #59 (closed) (both selectors, neither, or delete_all: false rejected) Both Validate tables, plus the three zero-value tests
AC #60, #61, #62 (delete_all scope and the survivor boundary) Payload side only: AcceptedAt on all three types, its new rejection arm, and TestChunkTagNameJobs_LargestChunkSurvivesTheStampedTimestamp for the budget the stamp has to fit. The predicate itself is the workers'
AC #64 (no failure response echoes a submitted entry) Nothing here writes a response. Every wrapped error carries an index and byte counts, never a tag name or digest
Plan: a 1,000-digest batch fits one job TestBulkDeleteContainerArgs_FullDigestBatchFitsOneJob, with the canonical-string contrast measured in the same test
Plan: the largest chunked tag-name job stays under the cap TestChunkTagNameJobs_FullMaxLengthBatchChunksUnderCap, TestChunkTagNameJobs_LargestChunkSurvivesTheStampedTimestamp
Plan: digest round trip through the raw encoding TestEncodeManifestDigests_RoundTrip, TestDecodeManifestDigests_ProducesLowercaseHex, and the rejection tables both directions
Plan: a worker registered through the seam receives a committed enqueue TestIntegration_WorkerRegistrationsReachTheRiverClient, including its rolled-back-is-never-delivered arm
Plan: registration-set assertions cover the new seam TestWiring_CollectsManagementBulkWorkerRegistrations, TestManagementBulkWorkerRegistrations_EveryEntryIsWellFormed, TestApplyJobRegistrations_*
Plan: kind-string uniqueness pin TestBulkDeleteKinds_AreDistinctBoundedAndInvariant, TestDuplicateWorkerKind_DetectsARepeatedKind

Test plan

go build ./...
go vet ./internal/managementapi/ ./cmd/artifact-registry/
go vet -tags=integration ./cmd/artifact-registry/
go test -race -count=1 ./internal/managementapi/
go test -count=1 ./cmd/artifact-registry/
go-lint-ci ./internal/managementapi/ ./cmd/artifact-registry/

The seam's integration floor needs PostgreSQL, from *_TEST_DSN or testcontainers:

go test -tags=integration -count=1 -run TestIntegration_WorkerRegistrationsReachTheRiverClient ./cmd/artifact-registry/

TestWireStorage_CloudCDNPresent fails locally without Google application-default credentials, identically on main.

Context for LLM agents

Design rationale, with the alternatives that were rejected

  • Three payload types rather than one with a family discriminator. jobsriver.RegisterWorker keys kind to Go type, so one type cannot carry three kinds. One kind per family also keeps registration additive across the three worker MRs and stops a poison batch in one family's queue entries from blocking another family's retries. The Maven and npm types have identical field sets and validate through one shared unexported view (packageFamilyView), so the logic is single-sourced while the types stay distinct. An exported embedded struct was rejected: it makes composite literals unusable from outside the package.
  • [][]byte per digest rather than one flat []byte. A single concatenated buffer is about 4 KB smaller, but it loses per-entry framing and makes a truncated payload decode as a valid but differently-sized batch. DecodeManifestDigests rejects any entry that is not exactly 32 bytes, which the flat form cannot express.
  • Uppercase hex is rejected, not lowercased. A digest is a delete key. A caller sending a spelling the surface refuses should learn it from the refusal rather than have its input silently rewritten.
  • The chunker verifies by marshaling every chunk, not only by arithmetic. verifyChunkedPayloads uses the same > comparison against jobs.MaxPayloadBytes that jobsriver.Client.prepare uses, so the local check and the enqueue check cannot disagree at the boundary. Its sentinel is fired directly by test, because an unfired guard is indistinguishable from a broken one.
  • Duplicate tag names are preserved by the chunker. Set collapse belongs to the selector decode. A deduping chunker would break the reassembly contract the enqueue relies on.
  • The boot line is river: applied job registrations, not jobs:. The helper takes a *jobsriver.Client and registers River work only, so a jobs:-scoped message would read as spanning both backends. Its two keys are file-scope constants because operators grep them.

Non-goals a reviewer might reasonably raise

  • No recover was added to riverComponent.run. A panic from a contributed register closure (which jobsriver.RegisterWorker raises on a duplicate kind for a different Go type) bypasses the component's fatal abort path and kills the process. That is fail-closed, it is pre-existing surface the periodic seam already carries identically, and it is outside this step's lines. Worth its own change if wanted.
  • The 1,000-entry selector cap is not referenced from this code. The cap lands in the selector-decode step's file, and forward-referencing a later step's symbol is forbidden by the repo guardrails. The tests carry it as a local bulkSubsetCap literal, which should be repointed at the real constant when that step lands. Until then, raising the cap there would turn a large manifests batch into jobs.ErrPayloadTooLarge at enqueue, which is neither the documented 400 nor the documented 503.
  • The boot line reports contributed kinds, not registered kinds. A caller holding the concrete client can register without passing through the seam. Making the stronger claim true needs a Kinds() accessor on jobsriver.Client, whose kinds map is unexported.
  • The plan's Status table is untouched. Nine steps of this plan are on parallel branches at once, so every branch would edit the same rows. The whole table is filled once for the wave.
  • cmd/artifact-registry/wire_jobs.go and wire.go are also touched by the bulk-enqueue-seam step, which adds a river-client accessor published from the same build closure. The two regions are disjoint by design (this one is the workerRegistrations seam beside periodicRegistrations), so the branches merge in either order.

Related to #313 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading