chore(managementapi): bulk-delete enqueue seam and River adapter

Why

Bulk delete answers 202 and applies the batch in a job, so a handler has to enqueue. managementapi holds no database handle and no job-backend client on purpose: its Deps is narrow interfaces, and the started *jobsriver.Client lives unexported in the composition root. A bulk request that arrives while River is still building has to answer 503 service_unavailable rather than read as a server defect.

This is Step 28 of the S17 Phase 4 plan, against the bulk-delete Outcome and Error Cases sections of S17. The seam ships inert: the bulk routes that consume it are later steps of the same plan, so nothing wires the adapter into managementapi.Deps yet.

What is worth attention

  • The seam takes a batch builder, not built jobs. Only the enqueuer can read the acceptance time off the database clock (SELECT now(), inside the enqueue transaction), and only the caller knows the payload types a later step introduces. BulkDeleteBatchFunc receives that instant and returns the jobs, which keeps the seam free of any concrete payload type and lets a chunked selector enqueue atomically.
  • Which failures carry the unavailable sentinel. An unpublished client, an absent pool, a transaction that will not start, an unreadable clock, and a refused insert all record nothing, so they answer 503 and the caller resubmits the whole batch. A failed Commit withholds the sentinel, because the batch's fate is then unknown, and a batch this service built wrong (empty, unregistered kind, over the payload cap) travels as itself, because a retry rebuilds it.
  • The 503 arm stamps 499 on a dead request. A disconnect finalizes the transaction, and the insert that follows is classified as a backend outage, so the cancellation is gone from the error by the time the helper sees it. The request context is the predicate instead. Left unstamped, the instrumentation writer records the request as 200, and a backend slow enough to time clients out would read as a burst of successes.
  • wire_jobs.go gains 5 lines, and the accessor rides bootDeps. Publishing before Start is safe because jobsriver refuses a pre-Start enqueue with ErrBackendUnavailable, which this seam maps to the 503. Using bootDeps rather than a new wireJobs parameter keeps the insertion disjoint from Step 29, which edits the same closure's registration loop, so the two branches merge in either order.
  • Size: 1458 lines, 285 of them production. The test floor is the rest, and it was authored before the code it constrains, so splitting the two would undo that.

Test plan

go test ./internal/managementapi/... ./cmd/artifact-registry/..., plus the enqueue floor against real PostgreSQL: go test -tags=integration -run 'BulkEnqueuer|RiverClientHolder' ./cmd/artifact-registry/.... That floor asserts a two-job batch lands both rows on one database-clock acceptance time, a batch whose second job fails records neither, an empty batch records nothing, an unstarted client answers unavailable, and the River build closure publishes the client. Lint is clean in both tag sets, integration tags included.

Spec coverage

Scoped to the bulk-delete enqueue path, which is the spec surface this step ships. Every other S17 row belongs to another step of the plan and travels with that step's MR.

Acceptance criteria

# Criterion Tests
AC-15 Every error path returns the S01 envelope with the correlation request_id TestEnqueueBulkDelete_ResponseMatrix (the 503 and 500 arms decode the envelope). request_id is stamped by transport.WriteError and covered in internal/transport.
AC-39 No bulk request returns 409 or 422 on any route TestEnqueueBulkDelete_ResponseMatrix (the helper's whole status vocabulary is 202, 503, 500)
AC-44 No bulk response body reports per-entry outcomes TestEnqueueBulkDelete_ResponseMatrix (the accepted row asserts an empty body)
AC-51 Bulk delete applies its entries in a job on the S27 foundation TestIntegration_BulkEnqueuer_EnqueueFloor/a_two-job_batch_lands_both_rows_on_one_database-clock_acceptance_time. The enqueue half only, since job application is the worker steps.
AC-55 A batch of exactly the cap returns 202 Partial: TestEnqueueBulkDelete_ResponseMatrix (the 202) and TestEnqueueBulkDelete_ChunkedBatch_IsOneEnqueue (a multi-job batch is one enqueue). The cap itself is Step 27.
AC-62 An artifact newer than a delete_all acceptance time survives it TestIntegration_BulkEnqueuer_EnqueueFloor/a_two-job_batch_lands_both_rows_on_one_database-clock_acceptance_time (the time's provenance and its single-instant property). The predicate is the worker steps.

Error cases

Condition Status Code Tests
Bulk delete: job backend unavailable at enqueue 503 service_unavailable TestEnqueueBulkDelete_ResponseMatrix (bare and wrapped sentinel), TestBulkEnqueuer_UnpublishedClient_IsUnavailable, TestBulkEnqueuer_NilHolder_IsUnavailable, TestBulkEnqueuer_NilDatabaseHandle_IsUnavailable, TestIntegration_BulkEnqueuer_EnqueueFloor/a_built_but_unstarted_client_is_unavailable
Bulk delete: no job recorded when the enqueue fails 503 service_unavailable TestIntegration_BulkEnqueuer_EnqueueFloor/a_built_but_unstarted_client_is_unavailable, .../a_batch_whose_second_job_fails_records_neither
All: unexpected server failure 500 internal_server_error TestEnqueueBulkDelete_ResponseMatrix (unrelated-error row), TestEnqueueBulkDelete_CancelledRequest_WritesNothing, TestEmptyBulkDeleteBatch_IsNotTheUnavailableSentinel, TestIntegration_BulkEnqueuer_EnqueueFloor/an_empty_batch_is_an_error_and_records_nothing
Bulk delete: the 400 selector and body rows, 413 size cap - - Owned by Step 27 (selector decode). This helper runs after validation, so no arm of it can answer them.
Artifact writes: the 404 and 405 route rows - - Owned by Step 8 (write-route scaffold). The seam holds no route, no slug, and no path value.

Security considerations

Concern Tests
Echoed input: no failure response repeats a submitted identifier TestEnqueueBulkDelete_ResponseMatrix (the 503 and 500 envelopes carry static messages, and the seam never receives a selector)
Bounded blast radius: the batch cap Owned by Step 27 (selector decode), which checks the cap before the namespace lookup.
Authentication, authorization, existence hiding, tenant isolation Not reached by this seam: it holds no namespace, no selector, and no query. Covered by the route and handler steps.
Context for LLM agents

Rejected alternatives

  • A []jobs.Args parameter with the acceptance time stamped by the caller. The caller cannot read the database clock, and a time taken from an application host is what the delete_all boundary rules out ("read from the database clock so the boundary does not depend on an application host's").
  • A second seam method that returns the acceptance time. The clock read would then land in a different transaction from the inserts, so the boundary a worker resolves against and the batch it resolves could disagree.
  • A new wireJobs parameter for the accessor, mirroring periodicRegistrations. Correct in isolation, but Step 29 adds its own parameter to the same signature and the same call line, so both branches would conflict there. bootDeps already reaches wireJobs and already carries a non-factory hook (fatal), and it doubles as the override seam the composition test uses to observe what the build closure published.
  • Widening the riverClient interface the River component holds. jobsriver.EnqueueTx is a generic free function over the concrete client, so no interface method can carry it.
  • A retry inside the adapter. The spec makes the caller resubmit the whole batch, and a server-side retry would hold a connection through an outage for a request that has already lost its client.

Non-goals

  • Wiring the adapter into managementapi.Deps or any route. Steps 32, 34, and 37 own those, and the plan orders workers before routes because a live enqueue route whose worker kind is unregistered strands accepted jobs.
  • The bulk job payload types and the byte-budget chunker (Step 29), the batch cap and selector validation (Step 27), and the background-jobs documentation (Step 29).
  • The plan's Status table. Nine step branches are in flight from this one plan, so the rows land together in a single follow-up commit rather than nine conflicting edits.
  • An upper bound on batch size inside the seam. The cap lives at decode and in Step 29's chunker, so a second one here would duplicate it.

Carried forward for the route steps

  • Nothing yet ties a consumer to the bootDeps holder, so a route step that constructs newBulkEnqueuer(db, &riverClientHolder{}) would read nil forever and every bulk delete would answer 503 while the boot-window comment made that look expected. Add a boot-level composition test that enqueues through the wired managementapi.Deps after Start and asserts a job row lands. Tracked in Boot-level composition test for the bulk enqueu... (#603) • Hayley Swimelar.
  • internal/managementapi/contract_test.go's emittable list omits service_unavailable, and api/openapi/v1.yaml on main ends its Error code enum at internal_server_error, so the code this surface answers with is today neither documented nor asserted. Its open diff adds both halves, so the gap closes with feat(managementapi): declare the artifact delet... (!1445 - merged) • Hayley Swimelar • 19.3.
  • On a done request context this surface's shared 500 path (logAndWriteInternalError) still returns without stamping a status, so those requests are recorded as 200. The 503 arm added here stamps transport.StatusClientClosedRequest. Aligning the 500 path is its own change, in resolve.go.

Related to #313 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading