From dfb9401dfbfb1396c79314d03f36e366c86d7df6 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Mon, 28 Sep 2026 10:15:30 -0700 Subject: [PATCH 1/3] docs: align guides and mock generation with the tree ## Summary ### Why? Guides, RFCs, and local-dev comments still described the layout from before service-scoped storage and core packages, and they still described an older orchestrator and Stovepipe pipeline. `make mocks` also skipped several packages that already had `//go:generate` directives, so `make check-mocks` could not refresh them. ### What? Repoints the architecture guide, domain READMEs, and storage docs at the live paths: shared SubmitQueue core is `changeset`, `messagequeue`, and `topickey`; request and batch helpers live under the gateway and orchestrator. Rewrites the orchestrator workflow around `pipeline.go`, including dependency analysis, a real speculate stage, the Runway land round-trip, and the hook stage. Marks shipped RFCs as implemented, and marks Stovepipe's analyze stage as designed and not built. Fixes relative links, compose build-target names, stop and status commands, and the MySQL queue RFC link. Extends `make mocks` to every `//go:generate` tree and commits the three mocks that regeneration updated. ## Test Plan - `make fmt`, `make tidy`, and `make gazelle` leave the tree clean. - License, message-id, queue-shard, and binary-file linters pass. `make lint` still reports this uncommitted diff from its format gate until the commit lands. - `make mocks` covers every `//go:generate` package, and `make test` passes 123 tests. - A local Markdown link scan finds no missing targets. Co-authored-by: Cursor --- AGENTS.md | 26 ++- Makefile | 35 +++- doc/howto/DEVELOPMENT.md | 25 ++- doc/howto/TESTING.md | 15 +- doc/rfc/hook-framework.md | 6 +- doc/rfc/index.md | 14 +- doc/rfc/messagequeue-contract.md | 12 +- doc/rfc/service-scoped-extensions.md | 34 ++-- doc/rfc/stovepipe/steps/build.md | 50 +++--- doc/rfc/stovepipe/steps/buildsignal.md | 30 ++-- doc/rfc/stovepipe/workflow.md | 112 +++++-------- doc/rfc/submitqueue/extension-contract.md | 10 +- doc/rfc/submitqueue/list-api.md | 2 + doc/rfc/submitqueue/modular-queue-wiring.md | 26 +-- doc/rfc/submitqueue/status-list-api.md | 4 +- doc/rfc/submitqueue/workflow.md | 156 +++++++++++------- platform/README.md | 5 + .../extension/messagequeue/mysql/README.md | 4 +- runway/README.md | 8 +- service/README.md | 9 +- service/runway/README.md | 1 - service/submitqueue/README.md | 7 +- service/submitqueue/docker-compose.yml | 2 +- service/submitqueue/gateway/server/Dockerfile | 2 +- .../gateway/server/docker-compose.yml | 2 +- .../orchestrator/server/Dockerfile | 2 +- .../orchestrator/server/docker-compose.yml | 2 +- .../queueconfig/mock/queueconfig_mock.go | 4 +- submitqueue/README.md | 9 +- .../core/changeset/mock/changeset_mock.go | 2 +- submitqueue/core/core.go | 13 +- submitqueue/extension/storage/README.md | 11 +- submitqueue/gateway/README.md | 2 +- .../extension/storage/mock/storage_mock.go | 6 +- .../extension/storage/mysql/schema/README.md | 2 +- tool/README.md | 17 ++ 36 files changed, 393 insertions(+), 274 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e87871100..8dc8c3be5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,42 +38,49 @@ request.Version = newVersion ``` submitqueue/ # repo root (Go module github.com/uber/submitqueue) ├── api/ # Published wire contracts (cross-domain/external) +│ ├── base/ # Shared protos (change/, hook/, mergestrategy/, messagequeue/) │ ├── submitqueue/{gateway,orchestrator}/{proto,protopb}/ # RPC (proto) │ ├── stovepipe/{proto,protopb}/ # single-service RPC (proto) — no service segment yet │ ├── runway/{proto,protopb}/ # RPC (proto) — single-service domain, no service segment │ └── runway/messagequeue/ # external queue contracts (proto + protojson) ├── platform/ # SHARED cross-domain packages — no domain deps -│ ├── errs/, metrics/, consumer/, http/ +│ ├── errs/, metrics/, consumer/, http/, publish/ +│ ├── git/, hook/, lifecycle/, pipeline/ │ ├── base/ # SHARED entities (change/, messagequeue/, …) │ └── extension/ # SHARED extension contracts + backends (counter/, messagequeue/, …) ├── submitqueue/ # SubmitQueue domain │ ├── gateway/ # Gateway service (port 8081) - entry point -│ │ └── extension/ # Aggregates/backends only the gateway resolves (storage/) +│ │ ├── core/request/ # Request-log materialization +│ │ └── extension/storage/ # Aggregate, MySQL implementation, and schema │ ├── orchestrator/ # Orchestrator service (port 8082) - coordinates jobs -│ │ └── extension/ # Aggregates/backends only the orchestrator resolves (storage/) +│ │ ├── core/ # request publish/terminate; batch helpers +│ │ └── extension/storage/ # Aggregate, MySQL implementation, and schema +│ ├── client/ # Gateway client for CLIs and the demo │ ├── entity/ # SubmitQueue-specific domain entities │ ├── extension/ # SubmitQueue-specific extension contracts and implementations -│ └── core/ # SubmitQueue-internal shared infra (changeset, messagequeue, topickey) +│ └── core/ # Shared infra both services use (changeset, messagequeue, topickey) ├── stovepipe/ # Stovepipe domain (single service) │ ├── controller/ # RPC and queue-stage business logic │ ├── entity/ # Stovepipe domain entities │ ├── extension/ # Stovepipe-specific extension contracts and implementations │ └── core/ # Stovepipe-internal queue contracts and shared infrastructure ├── runway/ # Runway domain (single service — the domain *is* the service) -│ └── controller/ # Runway service controllers (consumes the merge queues; no gateway/orchestrator split) +│ ├── controller/ # Merge-queue controllers (no gateway/orchestrator split) +│ └── extension/ # Runway extensions (merger/) ├── tool/ # Development and CI tooling ├── service/ # Runnable server/client wiring (entry points + Docker Compose) +│ ├── messagequeue/ # Shared queue MySQL pool and tenant configuration │ ├── submitqueue/ # Runnable SubmitQueue servers/clients + Docker Compose │ ├── stovepipe/ # Runnable Stovepipe server/client + Docker Compose │ └── runway/ # Runnable Runway server/client + Docker Compose ├── test/ -│ ├── e2e/submitqueue/ # End-to-end tests (full stack) +│ ├── e2e/{submitqueue,stovepipe,runway}/ # End-to-end tests │ ├── integration/ # Integration tests (platform/, submitqueue/, stovepipe/, …) │ └── testutil/ # Test utilities (ComposeStack, MySQL helpers) └── doc/ # Documentation ``` -The `platform/` tree holds code reused across domains (infrastructure, shared entities, shared extension contracts). A multi-service **domain** (e.g. `submitqueue/`) keeps the same internal layout (`gateway/`, `orchestrator/`, `entity/`, `extension/`, `core/`); a domain's own `core/` (e.g. `submitqueue/core/`) holds infra shared only between that domain's services. A **single-service domain** collapses that split — the domain *is* the service, so its controllers live directly under the domain root (e.g. `runway/controller/`, `stovepipe/controller/`) with no `gateway/`/`orchestrator/` segment, and its wire contract is service-segment-free (`api/{domain}/`). `runway` is a consumer-only merge execution service with no gateway. `stovepipe` exposes ingestion RPC behavior and runs its own process, build, build-signal, record, hook, and DLQ queue stages. +The `platform/` tree holds code reused across domains (infrastructure, shared entities, shared extension contracts). A multi-service **domain** (e.g. `submitqueue/`) keeps the same internal layout (`gateway/`, `orchestrator/`, `entity/`, `extension/`, `core/`); a domain's own `core/` (e.g. `submitqueue/core/`) holds infra shared by that domain's services — SubmitQueue's is `changeset`, `messagequeue`, and `topickey`. Helpers that serve one service live under that service: `submitqueue/gateway/core/request` materializes request logs, `submitqueue/orchestrator/core/request` publishes and terminates them, and `submitqueue/orchestrator/core/batch` moves batches and their queue membership records. A **single-service domain** collapses that split — the domain *is* the service, so its controllers live directly under the domain root (e.g. `runway/controller/`, `stovepipe/controller/`) with no `gateway/`/`orchestrator/` segment, and its wire contract is service-segment-free (`api/{domain}/`). `runway` is a consumer-only merge execution service with no gateway; it publishes merge results on the signal queues. `stovepipe` exposes ingestion RPC behavior and runs its own process, build, build-signal, record, hook, and DLQ queue stages. The `api/` tree holds **published** wire contracts — those depended on from outside the owning domain. RPC contracts live at `api/{domain}/{service}/` (`proto/` for `.proto` sources, `protopb/` for committed generated Go); for a single-service domain the service segment is dropped, so the contract lives directly at `api/{domain}/` (e.g. `api/runway/{proto,protopb}/`). A service package may hold multiple `.proto` files, all generating into the same `protopb/`. External message-queue contracts live at `api/{domain}/messagequeue/` (see Message Queue Contracts below). Internal queue contracts do **not** go here — they live under `{domain}/core/messagequeue/`. @@ -175,7 +182,8 @@ Paths follow the directory layout: shared packages live under `platform/` at the - Domain extensions: `github.com/uber/submitqueue/{domain}/extension/{ext}[/{impl}]` (e.g. `.../submitqueue/extension/storage`) - Service-scoped extensions: `github.com/uber/submitqueue/{domain}/{service}/extension/{ext}[/{impl}]` (e.g. `.../submitqueue/orchestrator/extension/storage/mysql`) - Cross-domain consumer framework: `github.com/uber/submitqueue/platform/consumer`; internal topic keys live with the owning domain contract (for example `submitqueue/core/messagequeue` and `stovepipe/core/messagequeue`); external queue topic keys live with their published contract (for example `api/runway/messagequeue`) -- Domain-internal infra: `github.com/uber/submitqueue/{domain}/core/{pkg}` (e.g. `.../submitqueue/core/request`) +- Domain-internal infra: `github.com/uber/submitqueue/{domain}/core/{pkg}` (e.g. `.../submitqueue/core/changeset`, `.../submitqueue/core/messagequeue`) +- Service-scoped infra: `github.com/uber/submitqueue/{domain}/{service}/core/{pkg}` (e.g. `.../submitqueue/gateway/core/request`, `.../submitqueue/orchestrator/core/request`, `.../submitqueue/orchestrator/core/batch`) - Shared entities: `github.com/uber/submitqueue/platform/base/{pkg}` (e.g. `.../platform/base/messagequeue`) - Shared extensions: `github.com/uber/submitqueue/platform/extension/{ext}[/{impl}]` (e.g. `.../platform/extension/messagequeue/mysql`) - Cross-domain infra: `github.com/uber/submitqueue/platform/{pkg}` (e.g. `.../platform/errs`, `.../platform/metrics`, `.../platform/http`) @@ -246,7 +254,7 @@ make local-submitqueue-start # Start full stack with Docker Compose make local-submitqueue-ps # Show running containers and ports make local-submitqueue-logs # View logs from all services make local-stop # Stop all services -make clean # Clean Bazel cache +make clean # Clean Bazel cache and bin/ ``` ### Common Workflows diff --git a/Makefile b/Makefile index a27572139..c3e4bb2c7 100644 --- a/Makefile +++ b/Makefile @@ -232,7 +232,7 @@ check-tidy: tidy ## Check that go.mod and MODULE.bazel are tidy $(call assert_clean,make tidy) @echo "Module files are up to date." -clean: ## Clean generated files and binaries +clean: ## Remove the Bazel cache and bin/ (generated proto: make clean-proto) @echo "Cleaning with Bazel..." @$(BAZEL) clean @rm -rf bin/ @@ -485,7 +485,7 @@ local-submitqueue-ps: ## Show running containers and their ports @echo " mysql -h127.0.0.1 -P$$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-mysql-app-1 3306 2>/dev/null | cut -d: -f2 || echo 'PORT') -uroot -proot submitqueue" @echo "" @echo " # Call Gateway gRPC" - @echo " grpcurl -plaintext -d '{\"message\":\"test\"}' localhost:$$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'PORT') submitqueue.SubmitQueueGateway/Ping" + @echo " grpcurl -plaintext -d '{\"message\":\"test\"}' localhost:$$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | cut -d: -f2 || echo 'PORT') uber.submitqueue.gateway.SubmitQueueGateway/Ping" @echo "" @echo " # View logs" @echo " make local-submitqueue-logs" @@ -541,12 +541,12 @@ local-submitqueue-stop: ## Stop the SubmitQueue stack (keeps PROVIDER=git's sand echo "Sandbox repository left at $(SQ_GIT_SANDBOX_DIR); remove it with 'make local-submitqueue-clean'."; \ fi -local-stop: ## Stop every local stack — SubmitQueue, Stovepipe, and Runway (keep data) +local-stop: ## Stop every local stack — SubmitQueue, Stovepipe, and Runway @echo "Stopping all services..." @$(COMPOSE) -f $(COMPOSE_FILE) -p $(SUBMITQUEUE_LOCAL_PROJECT) down @$(COMPOSE) -f $(STOVEPIPE_COMPOSE_FILE) -p $(STOVEPIPE_LOCAL_PROJECT) down @$(COMPOSE) -f $(RUNWAY_COMPOSE_FILE) -p $(RUNWAY_LOCAL_PROJECT) down - @echo "Services stopped. Data volumes preserved." + @echo "Services stopped. Anonymous database volumes are not reused on the next start." local-stovepipe-debug-start: build-stovepipe-linux-debug ## Start Stovepipe under delve in Docker (attach IDE to :2345) @echo "Starting Stovepipe service with compose (debug)..." @@ -581,9 +581,34 @@ local-stovepipe-stop: ## Stop the Stovepipe service @$(COMPOSE) -f $(STOVEPIPE_COMPOSE_FILE) -p $(STOVEPIPE_LOCAL_PROJECT) down @echo "Stovepipe service stopped." +# go generate does not walk the module; every tree with a //go:generate directive is listed here. +GO_GENERATE_PACKAGES := \ + ./platform/consumer/... \ + ./platform/extension/consumergate/... \ + ./platform/extension/counter/... \ + ./platform/extension/hook/... \ + ./platform/extension/messagequeue/... \ + ./runway/extension/merger/... \ + ./stovepipe/core/requestlog/... \ + ./stovepipe/extension/buildrunner/... \ + ./stovepipe/extension/projectresult/... \ + ./stovepipe/extension/queueconfig/... \ + ./stovepipe/extension/sourcecontrol/... \ + ./stovepipe/extension/storage/... \ + ./submitqueue/core/changeset/... \ + ./submitqueue/extension/buildrunner/... \ + ./submitqueue/extension/changeprovider/... \ + ./submitqueue/extension/conflict/... \ + ./submitqueue/extension/queueconfig/... \ + ./submitqueue/extension/speculation/... \ + ./submitqueue/extension/storage/... \ + ./submitqueue/extension/validator/... \ + ./submitqueue/gateway/extension/storage/... \ + ./submitqueue/orchestrator/extension/storage/... + mocks: ## Generate mock files using mockgen @echo "Generating mocks..." - @$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/gateway/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/hook/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./runway/extension/merger/... ./submitqueue/extension/conflict/... ./submitqueue/extension/speculation/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/core/requestlog/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/... ./stovepipe/extension/projectresult/... + @$(BAZEL) run @rules_go//go -- generate $(GO_GENERATE_PACKAGES) @echo "Mocks generated successfully!" proto: ## Generate protobuf files from .proto definitions diff --git a/doc/howto/DEVELOPMENT.md b/doc/howto/DEVELOPMENT.md index 8b4d11219..a37cca1df 100644 --- a/doc/howto/DEVELOPMENT.md +++ b/doc/howto/DEVELOPMENT.md @@ -90,20 +90,33 @@ brew install grpcurl ## Common Make Targets +`make help` lists every target. The table below is the day-to-day set. + +CI runs `make lint`, `make check-tidy`, and `make check-gazelle`. `make lint` includes the format check, license headers, the tracked-binary check, the message-ID check, and the queue-shard check. `make fmt`, `make tidy`, and `make gazelle` apply the corresponding fixes. `make mocks` regenerates the checked-in mockgen files; `make check-mocks` fails when that output differs from the tree. `make clean` removes the Bazel cache and `bin/`. Generated protobuf Go files stay in the source tree until `make clean-proto`; `make proto` writes them again. + | Target | Description | |--------|-------------| | `make build` | Build all services | | `make test` | Run unit tests | | `make integration-test` | Run all integration tests (Docker-based) | | `make e2e-test` | Run end-to-end tests | +| `make fmt` | Format Go and YAML | +| `make lint` | Run the linters CI runs | +| `make tidy` | Tidy `go.mod` and `MODULE.bazel` | +| `make check-tidy` | Fail if `go.mod` or `MODULE.bazel` is untidy | +| `make check-gazelle` | Fail if `BUILD.bazel` files are stale | +| `make gazelle` | Update `BUILD.bazel` files | +| `make mocks` | Regenerate mockgen files | +| `make check-mocks` | Fail if generated mocks are stale | | `make proto` | Regenerate protobuf files | -| `make gazelle` | Update BUILD.bazel files | +| `make clean` | Remove the Bazel cache and `bin/` | +| `make clean-proto` | Remove generated protobuf Go files | | `make local-submitqueue-start` | Start full workflow stack (Gateway + Orchestrator + Runway + two MySQL databases) | -| `make local-submitqueue-ps` | Show running containers and ports | -| `make local-submitqueue-logs` | View logs from all services | -| `make local-stop` | Stop all services | -| `make clean` | Clean generated files and binaries | -| `make help` | Show all available targets with descriptions | +| `make local-submitqueue-ps` | Show running SubmitQueue containers and ports | +| `make local-submitqueue-logs` | View logs from all SubmitQueue services | +| `make local-submitqueue-stop` | Stop the SubmitQueue stack | +| `make local-stop` | Stop SubmitQueue, Stovepipe, and Runway | +| `make help` | List every target | ## Running Specific Tests diff --git a/doc/howto/TESTING.md b/doc/howto/TESTING.md index f5401a798..8f0f33630 100644 --- a/doc/howto/TESTING.md +++ b/doc/howto/TESTING.md @@ -144,6 +144,8 @@ Shared (cross-domain) suites carry no domain segment — e.g. the shared queue e | SubmitQueue consumer (core) | `core-submitqueue-consumer` | `sq-test-core-submitqueue-consumer-…-mysql-1` | | SubmitQueue e2e (full stack) | `e2e-submitqueue` | `sq-test-e2e-submitqueue-def456-gateway-service-1` | +The same shape covers the other suites, including `e2e-stovepipe`, `e2e-runway`, `e2e-submitqueue-git`, and `ext-messagequeue-vitess`. + ### Parallel execution Each suite normally gets a distinct project name (`{context}-{shortid}`), where the short suffix is derived from the low 24 bits of the current nanosecond timestamp. It is useful for separating concurrent runs but is not a guaranteed unique identifier. Every compose service publishes **ephemeral host ports** (`- "3306"`, `- "8080"`), so suites can run **in parallel**. `make integration-test` runs suites concurrently via `--test_output=errors` (`--test_output=streamed` would force Bazel to serialize them). The domain-qualified context keeps container names understandable when many run at once. @@ -158,11 +160,11 @@ For additional manual inspection: # See what tests are currently running docker ps --format "table {{.Names}}\t{{.Status}}" | grep sq-test -# Find all containers from gateway test -docker ps | grep sq-test-gateway +# Find all containers from the SubmitQueue gateway integration test +docker ps | grep sq-test-svc-submitqueue-gateway # Inspect a specific test's MySQL -docker exec -it sq-test-ext-counter-2ce1d0-mysql-1 \ +docker exec -it sq-test-ext-counter-mysql-2ce1d0-mysql-1 \ mysql -uroot -proot submitqueue -e "SHOW TABLES;" ``` @@ -277,7 +279,8 @@ grpcurl -plaintext -import-path . -proto api/submitqueue/gateway/proto/gateway.p | `make local-submitqueue-ps` | Show running containers and ports | | `make local-submitqueue-logs` | Follow logs from all services | | `make local-submitqueue-restart` | Rebuild and restart all services | -| `make local-stop` | Stop all services (keep data) | +| `make local-submitqueue-stop` | Stop the SubmitQueue stack (MySQL data does not survive; a `PROVIDER=git` sandbox is left in place) | +| `make local-stop` | Stop SubmitQueue, Stovepipe, and Runway | | `make local-submitqueue-gateway-stop` | Stop Gateway service | | `make local-submitqueue-orchestrator-stop` | Stop Orchestrator service | | `make local-submitqueue-clean` | Stop and remove all services, volumes, and images | @@ -352,6 +355,10 @@ docker network ls | grep sq-test | awk '{print $1}' | xargs docker network rm ## Writing New Tests +Integration tests under `test/integration/` use the directory name as the package (`package gateway`, `package orchestrator`, `package stovepipe`, `package mysql`), not an external `*_test` package. + +End-to-end tests under `test/e2e/` are the exception: every suite there is `package e2e_test`. + ### Adding Unit Tests 1. Create `{file}_test.go` next to production code diff --git a/doc/rfc/hook-framework.md b/doc/rfc/hook-framework.md index e4a5ae601..186add8d2 100644 --- a/doc/rfc/hook-framework.md +++ b/doc/rfc/hook-framework.md @@ -2,9 +2,13 @@ Fire-and-forget side effects for pipeline lifecycle events: one shared event contract, a durable hook topic per domain, pluggable hooks. +## Status + +Implemented. The contract is `api/base/hook`, the dispatcher and DLQ reconciler are `platform/hook`, and the extension is `platform/extension/hook`. Stovepipe's `process` and `record` publish repository-scoped hook events, and `service/stovepipe/server` registers the stage. The SubmitQueue orchestrator pipeline registers a `submitqueue-hook` stage; no orchestrator controller publishes a hook event, and `service/submitqueue/orchestrator/server` resolves every event to noop. A deployment's real integrations are whatever its resolver returns. + ## Problem -The pipelines emit lifecycle transitions — a request lands or fails, a batch lands, a build finishes — but nothing can react outside pipeline state: no warehouse export, no PR comments or closes on land events, no notifications or audit trails. The log topic is not this seam: SubmitQueue request statuses only, consumed solely to build gateway read models. +The pipelines emit lifecycle transitions — a request lands or fails, a batch lands, a build finishes — and a side effect needs a place outside pipeline state: warehouse export, PR comments or closes on land events, notifications, audit trails. The log topic is not this seam: SubmitQueue request statuses only, consumed solely to build gateway read models. Two requirements: side effects must never stall or fail the pipeline, and "fire and forget" must not mean lossy — a land-failure comment that silently never posts is a support ticket. diff --git a/doc/rfc/index.md b/doc/rfc/index.md index 37ff46ec5..d36971904 100644 --- a/doc/rfc/index.md +++ b/doc/rfc/index.md @@ -6,16 +6,16 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting - [SQL-Based Distributed Queue](sql-queue-rfc.md) - MySQL-based distributed message queue with partition leasing and at-least-once delivery (used by SubmitQueue, Stovepipe, and other repo-local services) - [Message Queue Tenant Sharding](messagequeue-tenant-sharding.md) - Per-tenant shard key on the platform MySQL message queue; SubmitQueue maps `queueName` to `tenant` at wiring -- [Message Queue Contract](messagequeue-contract.md) - How queue payloads are defined (Protobuf, serialized as protobuf JSON), located by audience (external in `api/{domain}/messagequeue/`, internal in `{domain}/core/messagequeue/`), bound to topics (the `topics` proto option), and enforced by Bazel visibility +- [Message Queue Contract](messagequeue-contract.md) - How queue payloads are defined (Protobuf, serialized as protobuf JSON), located by audience (external in `api/{domain}/messagequeue/`, internal in `{domain}/core/messagequeue/`), bound to topic keys (the `topic_keys` proto option), and enforced by Bazel visibility - [Consumer Gate](consumer-gate.md) - Stopping and starting individual queue controllers at runtime via a consumer-side check: blocked deliveries are recorded as parked and postponed back to the queue (re-checked on redelivery), gate state as a separate extension with a file-based first implementation shared by tests and operators - [Consumer Hold](consumer-hold.md) - Fourth delivery outcome letting a controller postpone its delivery: the message becomes a partition barrier that pauses consumption for a chosen delay, redelivers in order, and does not count as a failure toward dead-lettering - [Change URIs](change-uri.md) - Identity of a code change: `scheme://{host[:port]}/{path}` per provider (GitHub PR, Phabricator Diff, git ref/commit) and canonical-form rules -- [Hooks Framework](hook-framework.md) - Fire-and-forget side effects off pipeline lifecycle events: one shared `HookEvent` contract (`api/base/hook/`) published to a durable per-domain hook topic, dispatched by a per-domain stage to a pluggable hook extension (`platform/extension/hook/`) for integrations like warehouse export and code-review notifications -- [Service-Scoped Extensions](service-scoped-extensions.md) - An extension whose aggregate serves one service of a multi-service domain moves that aggregate, its implementations, mocks and schema to `{domain}/{service}/extension/{ext}/` while the store contracts stay shared; SubmitQueue's storage aggregate splits into gateway and orchestrator halves first +- [Hooks Framework](hook-framework.md) - Implemented fire-and-forget side effects: one shared `HookEvent` contract (`api/base/hook/`) on a durable per-domain hook topic, dispatched by `platform/hook` to `platform/extension/hook`. Stovepipe `process` and `record` publish repository events; the SubmitQueue orchestrator registers the stage and does not publish events yet +- [Service-Scoped Extensions](service-scoped-extensions.md) - Implemented for SubmitQueue storage: gateway and orchestrator aggregates, schemas, and the core packages that serve one service have moved, while store contracts stay at `submitqueue/extension/storage`. Domain-level `buildrunner`, `conflict`, and `speculation` have not moved, and `changeset` still declares its own store slice ## SubmitQueue -- [Orchestrator Workflow](submitqueue/workflow.md) - Queue-driven controller pipeline from gateway entry through batching, scoring, build, land, and conclude +- [Orchestrator Workflow](submitqueue/workflow.md) - Queue-driven orchestrator pipeline: start, cancel, validate, Runway conflict check, batch, dependency analysis, speculate, build, Runway land, conclude, and a hook stage with no orchestrator publisher yet - [Gateway History APIs](submitqueue/history-api.md) - Request lifecycle history exposed through separate request ID and change ID endpoints - [Build Runner](submitqueue/build-runner.md) - Vendor-agnostic BuildRunner interface, provider-neutral BuildStatus lifecycle, and how the orchestrator wires it into the build stage - [Extension Contract](submitqueue/extension-contract.md) - When extensions take orchestrator identity (request/batch) and resolve granular content themselves vs. take controller-resolved data; revises the BuildRunner base/head contract @@ -23,15 +23,15 @@ Design documents and technical proposals, grouped by scope. Shared/cross-cutting - [Speculation](submitqueue/speculation.md) - Why SubmitQueue speculates, the path/tree model, and the two pluggable seams: speculation-tree enumeration and path selection - [Outcome Scorer](submitqueue/outcome-scorer.md) - How likely a batch is to reach Succeeded: `Score(ctx, batch, paths)` as a logit-linear model — a base content price plus YAML weights on path and batch evidence (`pathPassed`, `pathFailed`, `landing`, `cancelling`) - [Best-First Speculation Path Generation](submitqueue/speculation-generator-best-first.md) - The default Generator: per-head lazy streams of flip subsets merged best-first across heads, log-probability ranking, and the strict snapshot contract -- [Modular Queue Wiring](submitqueue/modular-queue-wiring.md) - Declare-don't-assemble engine (`pipeline.Construct`) that unifies topic registry, controller registration, DLQ pairing, and lifecycle ordering into one typed call; services self-declare via Deps struct + Stages slice, hosts own per-queue profiles and transport +- [Modular Queue Wiring](submitqueue/modular-queue-wiring.md) - `pipeline.Construct` for the SubmitQueue orchestrator (`Deps` + `Stages` in `submitqueue/orchestrator/pipeline.go`, host in `service/submitqueue/orchestrator/server`). Stovepipe, and the gateway and Runway servers, remain hand-wired ## Stovepipe -- [Stovepipe Workflow](stovepipe/workflow.md) - Post-land validation pipeline overview: ingest, process, build, record greenness, analyze projects, notify downstream +- [Stovepipe Workflow](stovepipe/workflow.md) - Implemented post-land pipeline: ingest, process, build, buildsignal, record, hook. `record` resolves project results inline. An analyze stage and topic are design only and are not built - [Process stage](stovepipe/steps/process.md) - Build-strategy decision, per-queue concurrency gate, backlog coalescing, entity model, platform prerequisites - [Build stage](stovepipe/steps/build.md) - Trigger-only stage and Stovepipe's URI-based BuildRunner contract - [Buildsignal stage](stovepipe/steps/buildsignal.md) - Build polling, terminal status persistence, and the handoff to record -- [Record stage](stovepipe/steps/record.md) - Immutable validation facts keyed by `(queue, uri, project)`, monotonic last-green bookmark advancement and ref promotion, and the deferred hook-event and analyze handoffs +- [Record stage](stovepipe/steps/record.md) - Immutable validation facts keyed by `(queue, uri, project)`, monotonic last-green bookmark advancement and ref promotion, and the repository hook event. The analyze-stage handoff in that doc is design only - [Request Log](stovepipe/request-log.md) - Append-only request lifecycle log, durable source context, idempotent storage, and reliable write and repair paths - [Request History API](stovepipe/request-history-api.md) - Queue-scoped request-ID and URI lookup, public projection, materialization decision, ordering, and retention - [GetProjectStatusByURI API](stovepipe/get-project-status-by-uri-api.md) - Queue-scoped current validation lookup for a commit, with repository and future project-level results diff --git a/doc/rfc/messagequeue-contract.md b/doc/rfc/messagequeue-contract.md index 7c0383650..f3a3ca776 100644 --- a/doc/rfc/messagequeue-contract.md +++ b/doc/rfc/messagequeue-contract.md @@ -2,13 +2,17 @@ How queue payloads are defined, located, and bound to topics across domains. +## Status + +Implemented. Queue payloads are proto3 messages serialized as protobuf JSON (`protojson`). External contracts live under `api/{domain}/messagequeue/`; internal contracts live under `{domain}/core/messagequeue/`. Each message binds the topic keys that carry it with the `topic_keys` option. The problem below is the pre-migration gap; the decisions are the rationale the migration followed and the contract the code uses now. + ## Problem -Queue payloads are Go structs serialized with `encoding/json` (`submitqueue/entity`, `runway/entity`), so the wire shape is defined only by Go source. Three gaps: +Before this contract, queue payloads were Go structs serialized with `encoding/json` (`submitqueue/entity`, `runway/entity`), so the wire shape was defined only by Go source. Three gaps: -- **No language-neutral contract.** Some payloads cross a domain boundary — a client written in another language has nothing to compile or validate against. -- **No topic-to-payload binding.** `consumer.TopicRegistry` maps a `TopicKey` to a backend, topic name, and subscription — but not to the payload schema. That knowledge lives implicitly in whichever controller (de)serializes. -- **No audience distinction.** Nothing separates private wiring between our own services from a published cross-domain contract. +- **No language-neutral contract.** Some payloads cross a domain boundary — a client written in another language had nothing to compile or validate against. +- **No topic-to-payload binding.** `consumer.TopicRegistry` maps a `TopicKey` to a backend, topic name, and subscription — but not to the payload schema. That knowledge lived implicitly in whichever controller (de)serialized. +- **No audience distinction.** Nothing separated private wiring between our own services from a published cross-domain contract. ## Decisions diff --git a/doc/rfc/service-scoped-extensions.md b/doc/rfc/service-scoped-extensions.md index 46eb70807..6aa4bf122 100644 --- a/doc/rfc/service-scoped-extensions.md +++ b/doc/rfc/service-scoped-extensions.md @@ -1,24 +1,30 @@ # Service-Scoped Extensions -Deciding which service of a multi-service domain an extension belongs to, and moving the parts that are that service's alone out of `{domain}/extension/` and into `{domain}/{service}/extension/`. SubmitQueue's storage extension goes first; the layout rule is meant to apply to every extension after it. +Deciding which service of a multi-service domain an extension belongs to, and moving the parts that are that service's alone out of `{domain}/extension/` and into `{domain}/{service}/extension/`. SubmitQueue's storage extension is the one this layout was applied to; the same rule is meant to apply to every extension after it. + +## Status + +Implemented for SubmitQueue storage and for the core packages that served one service. `submitqueue/gateway/extension/storage` and `submitqueue/orchestrator/extension/storage` each declare a `Factory`, a `Storage` aggregate, and a MySQL schema. The thirteen store contracts, their mocks, and the error vocabulary remain at `submitqueue/extension/storage`. There is no `submitqueue/extension/storage/mysql` package. `submitqueue/extension/storage/BUILD.bazel` filegroups the two service schemas for deployments that colocate them. + +`submitqueue/core/batch` and `submitqueue/core/request` are not on disk. Batch helpers live in `submitqueue/orchestrator/core/batch`. Request-log publish and terminate live in `submitqueue/orchestrator/core/request`. The gateway materializer lives in `submitqueue/gateway/core/request`. `submitqueue/core` holds `changeset`, `messagequeue`, and `topickey`. `changeset` still declares `Stores` and `Resolve`, because `buildrunner`, `conflict`, and `speculation` are still domain-level packages that depend on it. Stovepipe keeps `stovepipe/extension/storage/mysql` because that domain is the service; that package is not a leftover SubmitQueue backend. ## Problem -`submitqueue/extension/storage/` holds thirteen stores behind one `Storage` aggregate and one `Factory`, and both SubmitQueue services depend on the whole thing. They do not use the whole thing. The gateway reaches for four stores and the orchestrator for the other nine, with no overlap in either direction and none through the shared helpers in `submitqueue/core/`. +`submitqueue/extension/storage/` held thirteen stores behind one `Storage` aggregate and one `Factory`, and both SubmitQueue services depended on the whole thing. They do not use the whole thing. The gateway reaches for four stores and the orchestrator for the other nine, with no overlap in either direction. -The split is already the documented design. [Gateway Status and List APIs](submitqueue/status-list-api.md) states it outright: +The split is the documented design. [Gateway Status and List APIs](submitqueue/status-list-api.md) states it outright: > The gateway owns the append-only request log and three new logical read models. The orchestrator's request and change stores are pipeline working state with different retention semantics, so neither API reads them. -Nothing enforces it. A gateway controller can resolve `BatchStore` and nothing objects. +The aggregates enforce it. A gateway `Storage` has no `GetBatchStore`, so a gateway controller cannot obtain an orchestrator store. -Separating the services onto their own databases already works — the orchestrator publishes lifecycle events on the log topic and the gateway persists them, so the four gateway tables are not a store two services share. What does not work is provisioning: the schema is one filegroup, so each database gets all thirteen tables, including the nine or four that service never reads. +Separating the services onto their own databases already worked — the orchestrator publishes lifecycle events on the log topic and the gateway persists them, so the four gateway tables are not a store two services share. Each service's schema directory now contains the tables that service reads. The union filegroup at `submitqueue/extension/storage` is what a colocated deployment takes. ## Decisions 1. An extension whose **aggregate** is used by exactly one service of a multi-service domain puts that aggregate, its implementations, its mocks and its schema at `{domain}/{service}/extension/{ext}/`. The store contracts themselves stay at `{domain}/extension/{ext}/`: the aggregate is what decides reachability, so moving the contracts adds no enforcement while forcing domain-level callers to import a service package. 2. An extension shared between a domain's services stays wholly at `{domain}/extension/{ext}/` — including one used by a single service today but expected in both. `submitqueue/extension/queueconfig/` is gateway-only now and stays where it is on that basis. A single-service domain is unaffected: its domain root is its service root, so nothing moves. -3. This RFC moves storage only. `changeprovider`, `validator`, `conflict`, `buildrunner` and `speculation` are orchestrator-only today and would qualify under Decision 1, but none has a schema and none is the reason for this change; they stay at `{domain}/extension/` until a later RFC takes them. The trigger for moving now is a schema that a separate-database deployment must not over-provision. +3. Storage is the extension this split applied to. `changeprovider`, `validator`, `conflict`, `buildrunner` and `speculation` are orchestrator-only and would qualify under Decision 1, but none has a schema and none was the reason for this change; they stay at `{domain}/extension/` until a later RFC takes them. The trigger for moving storage was a schema that a separate-database deployment must not over-provision. 4. SubmitQueue's storage aggregate splits in two: `submitqueue/gateway/extension/storage/` declares a `Factory` and a four-accessor `Storage`, `submitqueue/orchestrator/extension/storage/` a `Factory` and a nine-accessor `Storage`. Neither can resolve the other's stores. 5. `submitqueue/extension/storage/` keeps the thirteen store contracts, their mocks, the error vocabulary, `Config`, and the package README, which documents the contracts. Its import path does not change, so callers that name only a contract or an error are untouched. 6. The contract package is not promoted to `platform/`. It is not cross-domain — `stovepipe/extension/storage/storage.go` already declares its own verbatim copy of the same symbols, so a per-domain error vocabulary is the existing pattern rather than something this RFC introduces. @@ -49,7 +55,7 @@ Every store, verified against actual usage rather than intent. `counter` is not in either list. It is its own shared extension and each service keeps its own counter table. -## Target layout +## Layout ``` submitqueue/ @@ -75,15 +81,15 @@ Naming a contract stays legal from anywhere; obtaining one does not. A gateway f ## What this means for `submitqueue/core/` -`core/` is infra shared between the domain's services, so it must not depend on one. Splitting the aggregate makes that a live question: a file there that resolves stores has to name some service's `Storage` or `Factory`. +`core/` is infra shared between the domain's services, so it must not depend on one. Splitting the aggregate made that a live question: a file there that resolves stores has to name some service's `Storage` or `Factory`. -Keeping the contracts at domain level settles it for files that name only a store type — `core/request/request.go` takes a `RequestLogStore` and needs nothing else. It does not settle it for the six files that take an aggregate, because aggregates are service-scoped by construction. +Keeping the contracts at domain level settles it for files that name only a store type. It does not settle it for files that take an aggregate, because aggregates are service-scoped by construction. -Those six are resolved two ways. +Those files were resolved two ways. -**Relocate the package when it serves one service.** `core/batch` is imported only by orchestrator controllers, and `core/request` divides cleanly by file — the orchestrator publishes lifecycle events on the log topic, the gateway persists them. So `core/batch` and `core/request/{log,terminate}.go` move under the orchestrator, `core/request/{materializer,request}.go` under the gateway. They keep taking an aggregate; a service naming its own extension is not an inversion. +**Relocate the package when it serves one service.** This is done. `submitqueue/orchestrator/core/batch` holds the batch helpers. `submitqueue/orchestrator/core/request` holds log publish and terminate. `submitqueue/gateway/core/request` holds the materializer. They keep taking an aggregate; a service naming its own extension is not an inversion. -**Declare the shape when it cannot.** `core/changeset` stays, because seven domain-level extension packages depend on it — `buildrunner` and its three implementations, `conflict/pathoverlap`, and the two `speculation/scorer` implementations — and those remain at domain level under Decision 3. Moving it now would trade one inversion for another. Until then it declares the slice of an aggregate it needs, and the wiring layer supplies the binding: +**Declare the shape when it cannot.** `submitqueue/core/changeset` stays, because domain-level extension packages depend on it — `buildrunner` and its implementations, `conflict/pathoverlap`, and the `speculation/scorer` implementations — and those remain at domain level under Decision 3. Moving it now would trade one inversion for another. It still declares the slice of an aggregate it needs, and the wiring layer supplies the binding: ```go type Stores interface { @@ -94,9 +100,9 @@ type Stores interface { type Resolve func(queue string) (Stores, error) ``` -Each service's aggregate satisfies `Stores` structurally, so `changeset` names no service package in the meantime. Once those seven extensions relocate under the orchestrator, `changeset`'s consumers are all orchestrator-side and it should relocate with them — at which point it can drop `Stores`/`Resolve` and take `orchstorage.Factory` directly, matching the other relocated packages rather than staying the one exception. +Each service's aggregate satisfies `Stores` structurally, so `changeset` names no service package. Once those extensions relocate under the orchestrator, `changeset`'s consumers are all orchestrator-side and it should relocate with them — at which point it can drop `Stores`/`Resolve` and take `orchstorage.Factory` directly, matching the other relocated packages rather than staying the one exception. -Afterwards `core/` holds `changeset`, `messagequeue` and `topickey`, and nothing in it imports a service package. +`submitqueue/core` now holds `changeset`, `messagequeue` and `topickey`, and nothing in it imports a service package. A third option exists and is not used here: narrowing a signature to take the individual contracts rather than the aggregate, for example `FindByRequestID(ctx, batches storage.BatchStore, links storage.RequestBatchStore, …)`. It suits a function whose store set is fixed, and is worth reaching for before relocating a package that genuinely is shared. diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index f630870b2..5f1db58be 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -1,16 +1,16 @@ # Build stage -`build` triggers the build-runner for the scope `process` already decided and hands the resulting build id to `buildsignal`. See [workflow.md](doc/rfc/stovepipe/workflow.md) for where it sits in the pipeline, and [process.md](doc/rfc/stovepipe/steps/process.md) for how the scope it reads (`BuildStrategy`, `BaseURI`) is chosen. +`build` triggers the build-runner for the scope `process` already decided and hands the resulting build id to `buildsignal`. See [workflow.md](../workflow.md) for where it sits in the pipeline, and [process.md](process.md) for how the scope it reads (`BuildStrategy`, `BaseURI`) is chosen. It handles only the trigger: it does not poll for completion, record greenness, or decide incremental-vs-full — those are `buildsignal`'s, `record`'s, and `process`'s jobs respectively. -`build` is structurally the same controller as `submitqueue/orchestrator/controller/build/build.go`, and [doc/rfc/submitqueue/build-runner.md](doc/rfc/submitqueue/build-runner.md) is the reference rationale for the trigger-then-poll shape this stage reuses. The `BuildRunner` contract itself — `Trigger`, `Status`, and `Cancel` alike — stays a separate `stovepipe/extension/buildrunner` interface rather than sharing SubmitQueue's; the two domains model different problems, so each keeps its own, following the [extension-contract.md](doc/rfc/submitqueue/extension-contract.md) "identity in, resolve internally" principle. What's shared is the underlying implementation, not the contract: a concrete backend (e.g. Buildkite) can satisfy both domains' interfaces off one client, so real code reuse happens at that layer instead of forcing a lowest-common-denominator interface (see [Why separate contracts](#why-separate-contracts)). +`build` is structurally the same controller as `submitqueue/orchestrator/controller/build/build.go`, and [doc/rfc/submitqueue/build-runner.md](../../submitqueue/build-runner.md) is the reference rationale for the trigger-then-poll shape this stage reuses. The `BuildRunner` contract itself — `Trigger`, `Status`, and `Cancel` alike — stays a separate `stovepipe/extension/buildrunner` interface rather than sharing SubmitQueue's; the two domains model different problems, so each keeps its own, following the [extension-contract.md](../../submitqueue/extension-contract.md) "identity in, resolve internally" principle. What's shared is the underlying implementation, not the contract: a concrete backend (e.g. Buildkite) can satisfy both domains' interfaces off one client, so real code reuse happens at that layer instead of forcing a lowest-common-denominator interface (see [Why separate contracts](#why-separate-contracts)). ## Input, partitioning, and the single-writer property -`build` consumes a request id, published by `process` in Phase 1 (see [workflow.md](doc/rfc/stovepipe/workflow.md#workflow)). Both phases drive the same `build` → `buildsignal` machinery against the same `Request` row. The `process`/`analyze` → `build` topic is partitioned by **request id**; see [Partitioning](#partitioning) for the full rationale, including why `build` → `buildsignal` partitions finer (by build id) than SubmitQueue's equivalent topic. +`build` consumes a request id, published by `process` in Phase 1 (see [workflow.md](../workflow.md#workflow)). Both phases drive the same `build` → `buildsignal` machinery against the same `Request` row. The `process`/`analyze` → `build` topic is partitioned by **request id**; see [Partitioning](#partitioning) for the full rationale, including why `build` → `buildsignal` partitions finer (by build id) than SubmitQueue's equivalent topic. -**`build` is not the sole writer of the rows it touches, and its own writes are narrow.** On `Request`, `build` never writes at all — it only reads the `BuildStrategy`/`BaseURI`/`URI` fields `process` set at admit, and it leaves `Request.State` untouched at `processing` throughout (`process`, `buildsignal`, and the DLQ reconciler are `Request.State`'s only writers). On `Build`, `build` is the sole creator — it calls `BuildStore.Create` exactly once, at step 6 — and never mutates the row again; `buildsignal` is the sole writer of `Build.Status`/`Build.Version` afterward (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#input-and-re-entrancy)). This division is why `build` never needs a CAS/version write of its own: `Create` is the only storage mutation in its algorithm. +**`build` is not the sole writer of the rows it touches, and its own writes are narrow.** On `Request`, `build` never writes at all — it only reads the `BuildStrategy`/`BaseURI`/`URI` fields `process` set at admit, and it leaves `Request.State` untouched at `processing` throughout (`process`, `buildsignal`, and the DLQ reconciler are `Request.State`'s only writers). On `Build`, `build` is the sole creator — it calls `BuildStore.Create` exactly once, at step 6 — and never mutates the row again; `buildsignal` is the sole writer of `Build.Status`/`Build.Version` afterward (see [buildsignal.md](buildsignal.md#input-and-re-entrancy)). This division is why `build` never needs a CAS/version write of its own: `Create` is the only storage mutation in its algorithm. `build` is phase-agnostic: it never asks "which phase is this?" It reads whatever scope is already persisted and immutable on the `Request` and acts on it. Phase 2's project-scoped invocation is expected to read project-scoped equivalents of the scope fields off the same `Request`; the exact shape of a project-scoped trigger is left to the `analyze` design, consistent with `workflow.md`'s "project mapping contract" open question — see [Project-scoped `Trigger`: reserved, not yet designed](#project-scoped-trigger-reserved-not-yet-designed) for the resulting gap in the `BuildRunner` contract itself. @@ -22,7 +22,7 @@ For a delivery carrying request id `R`: ``` 1. Load Request R from the request store. - - ErrNotFound -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](stovepipe/extension/storage/README.md)). + - ErrNotFound -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](../../../../stovepipe/extension/storage/README.md)). - other store error -> return raw; classifier decides. 2. If R.State is terminal (superseded / succeeded / failed / cancelled): ack and return. @@ -81,7 +81,7 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu - **Strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet. - **Request already terminal** (step 2) — ack, no build. A redelivery after `record` finished, or after `process` superseded the head, never starts a stale build. - **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `buildsignal`'s outcome write is first-writer-wins, so the second verdict cannot flip the Request's state or overwrite the create-only validation fact. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue. -- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, where the fail-closed posture is meant to drive the Request terminal (see [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). No reconciler consumes `build_dlq` yet, so that last step does not happen today — see [Fail-closed interaction](#fail-closed-interaction). +- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, where the fail-closed posture is meant to drive the Request terminal (see [workflow.md](../workflow.md#fail-closed-on-unprocessable-work)). No reconciler consumes `build_dlq` yet, so that last step does not happen today — see [Fail-closed interaction](#fail-closed-interaction). ## Edge cases @@ -90,28 +90,28 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu ## Fail-closed interaction -A build that never reaches step 8 — `Trigger` failing repeatedly, the publish to `buildsignal` never landing, `BuildStore.Create` down — must not wedge its `Request`'s Queue slot forever: `process`'s per-Queue concurrency gate holds `in_flight_count` open until the Request reaches a terminal state (see [process.md](doc/rfc/stovepipe/steps/process.md#concurrency-lifecycle)). `build` does not implement the forcing function itself. Per [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work), every non-retryable failure in the algorithm rejects to DLQ (see [Error classification](#error-classification)), and a Request stuck past `MaxAttempts` is driven to a conservative terminal `failed` by the DLQ reconciler, which decrements `in_flight_count` and frees the slot. This is the same posture `buildsignal` relies on for its own poll loop (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction)) — `build` and `buildsignal` are two links in the same fail-closed chain that keeps one bad Request from wedging its Queue. +A build that never reaches step 8 — `Trigger` failing repeatedly, the publish to `buildsignal` never landing, `BuildStore.Create` down — must not wedge its `Request`'s Queue slot forever: `process`'s per-Queue concurrency gate holds `in_flight_count` open until the Request reaches a terminal state (see [process.md](process.md#concurrency-lifecycle)). `build` does not implement the forcing function itself. Per [workflow.md](../workflow.md#fail-closed-on-unprocessable-work), every non-retryable failure in the algorithm rejects to DLQ (see [Error classification](#error-classification)), and a Request stuck past `MaxAttempts` is driven to a conservative terminal `failed` by the DLQ reconciler, which decrements `in_flight_count` and frees the slot. This is the same posture `buildsignal` relies on for its own poll loop (see [buildsignal.md](buildsignal.md#fail-closed-interaction)) — `build` and `buildsignal` are two links in the same fail-closed chain that keeps one bad Request from wedging its Queue. -**That chain is not closed at `build` yet.** The `build` subscription enables dead-lettering, but no controller consumes `build_dlq` — the wiring registers only `process_dlq` and `buildsignal_dlq` — so nothing forces the Request terminal and nothing frees the slot. How much that costs depends on how far the delivery got. If a `Build` row was persisted and its signal published, a poll chain survives the dead-letter and `buildsignal` still releases the slot when the build goes terminal. If the message dead-letters before that — `Trigger` failing every attempt, `BuildStore.Create` down, the publish never landing — the Request stays `processing` and its Queue loses a slot for good, which is exactly the failure [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction) describes for a deployment missing its own reconciler. +**That chain is not closed at `build` yet.** The `build` subscription enables dead-lettering, but no controller consumes `build_dlq` — the wiring registers only `process_dlq` and `buildsignal_dlq` — so nothing forces the Request terminal and nothing frees the slot. How much that costs depends on how far the delivery got. If a `Build` row was persisted and its signal published, a poll chain survives the dead-letter and `buildsignal` still releases the slot when the build goes terminal. If the message dead-letters before that — `Trigger` failing every attempt, `BuildStore.Create` down, the publish never landing — the Request stays `processing` and its Queue loses a slot for good, which is exactly the failure [buildsignal.md](buildsignal.md#fail-closed-interaction) describes for a deployment missing its own reconciler. Whoever wires that reconciler has to decide what it records, not just what it releases: forcing `failed` on a Request whose build may still be running is what produces the permanently-wrong-fact path in [record.md](record.md#what-fail-closed-actually-guarantees), so this gap and that open question belong to the same piece of work. -One boundary is worth stating explicitly: this path fires only when `build` (or a downstream stage) *errors*. A `Trigger` call that returns successfully but the backend never actually runs — or a `Build` row created for a build the runner silently drops — has no protocol-level failure to escalate at the `build` stage; nothing here retries or dead-letters, because nothing failed. That gap surfaces one hop later, when `buildsignal` polls: either the runner reports an error (handled by `buildsignal`'s own classification) or it reports a non-terminal status forever, which is `buildsignal`'s fail-closed boundary to close, not `build`'s (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#fail-closed-interaction)). `build`'s liveness responsibility ends at a successful publish to `buildsignal`. +One boundary is worth stating explicitly: this path fires only when `build` (or a downstream stage) *errors*. A `Trigger` call that returns successfully but the backend never actually runs — or a `Build` row created for a build the runner silently drops — has no protocol-level failure to escalate at the `build` stage; nothing here retries or dead-letters, because nothing failed. That gap surfaces one hop later, when `buildsignal` polls: either the runner reports an error (handled by `buildsignal`'s own classification) or it reports a non-terminal status forever, which is `buildsignal`'s fail-closed boundary to close, not `build`'s (see [buildsignal.md](buildsignal.md#fail-closed-interaction)). `build`'s liveness responsibility ends at a successful publish to `buildsignal`. ## Cancellation: defined, not yet called -`Cancel` is in the `BuildRunner` contract for parity with SubmitQueue, but no controller in this design calls it. SubmitQueue cancels from its `speculate`/cancel path when a batch is preempted mid-validation; stovepipe's `process` explicitly does **not** preempt an admitted `Request` ("a newer head does not preempt an in-flight validation" — [process.md](doc/rfc/stovepipe/steps/process.md#concurrency-lifecycle)). So there is currently no trigger for cancelling a running build. `Cancel` stays in the contract because a future change — the lease-based self-healing `in_flight_count`, or raising `max_concurrent` past 1, both floated in `process.md` — could need to abandon a build no longer worth finishing. Until then it is unused surface, not dead weight. +`Cancel` is in the `BuildRunner` contract for parity with SubmitQueue, but no controller in this design calls it. SubmitQueue cancels from its `speculate`/cancel path when a batch is preempted mid-validation; stovepipe's `process` explicitly does **not** preempt an admitted `Request` ("a newer head does not preempt an in-flight validation" — [process.md](process.md#concurrency-lifecycle)). So there is currently no trigger for cancelling a running build. `Cancel` stays in the contract because a future change — the lease-based self-healing `in_flight_count`, or raising `max_concurrent` past 1, both floated in `process.md` — could need to abandon a build no longer worth finishing. Until then it is unused surface, not dead weight. ## Error classification -Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative terminal `failed` so a Request never wedges its Queue's slot ([workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit: +Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](../../../../platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative terminal `failed` so a Request never wedges its Queue's slot ([workflow.md](../workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit: | Failure | Disposition | Why | |---|---|---| | `BuildStrategy` not yet visible (step 4) | retryable (`errs.NewRetryableError`) | The producing stage's write (`process`'s CAS) may not be visible on this reader yet; redelivery converges. | | `Trigger` | raw error; classifier decides | Deliberately left open rather than fixed either way — a runner timeout/connection is transient, a bad URI is permanent, and only a backend classifier can tell them apart. | -`Request` not found (`storage.ErrNotFound`) is **not** in this table: storage is required to be read-after-write consistent (see [storage README](stovepipe/extension/storage/README.md)), so a miss here is already the correct default (non-retryable, straight to DLQ) rather than a departure worth overriding. +`Request` not found (`storage.ErrNotFound`) is **not** in this table: storage is required to be read-after-write consistent (see [storage README](../../../../stovepipe/extension/storage/README.md)), so a miss here is already the correct default (non-retryable, straight to DLQ) rather than a departure worth overriding. Everything else — factory lookup, a malformed message, a build-store error other than `ErrAlreadyExists`, and the publish to `buildsignal` — is returned raw with no override, because the default is already correct: none of them are worth automatically replaying (a queue with no registered builder is a config error, a broken payload will never parse, and storage/queue and publish failures dead-letter and let DLQ reconciliation recover). @@ -142,16 +142,16 @@ There is no batch, no dependency list, and nothing to resolve — the URIs *are* ### Why separate contracts 1. **Conceptual mismatch — materialize vs. check out.** SubmitQueue's `Trigger` builds a state that does not exist yet: the runner resolves each batch's changes and applies their patches into composite base/head layers before running CI (see the Buildkite backend). Stovepipe's head already exists on the branch — trunk history is linear, and a landed commit *contains* every commit below it — so the runner checks out `headURI` and, for an incremental build, diffs against `baseURI`; no patch application, no composite commit. Squeezing stovepipe through the patch-list contract breaks in one of two ways for a history interval `a → b → c → d → e` (`a` = last green, `e` = head): modeling it as base `a` + patch `e` misses the intermediate commits `b..d` in the built state (**missing history**), while modeling it as heads `[b, c, d, e]` makes the runner cherry-pick four commits into a composite that is identical to just checking out `e` (**excessive work**). Forcing single-element batch lists is only the mild form of the same mismatch: abstraction with no benefit. -2. **Baseline semantics — and a mode batches can't express.** SubmitQueue's base batches are *stacked* into a dependency DAG validated together; stovepipe's baseline is a single reference point, and the build is either against it (incremental) or from scratch (full). Batches don't model this naturally — an empty base list means "no dependencies", not "ignore ancestry and build the whole repo", so the `full_monorepo` fallback `process` selects on a history rewrite ([process.md](doc/rfc/stovepipe/steps/process.md#build-strategy-decision)) has no faithful batch encoding at all. +2. **Baseline semantics — and a mode batches can't express.** SubmitQueue's base batches are *stacked* into a dependency DAG validated together; stovepipe's baseline is a single reference point, and the build is either against it (incremental) or from scratch (full). Batches don't model this naturally — an empty base list means "no dependencies", not "ignore ancestry and build the whole repo", so the `full_monorepo` fallback `process` selects on a history rewrite ([process.md](process.md#build-strategy-decision)) has no faithful batch encoding at all. 3. **URI ownership.** Stovepipe's head and baseline are owned by `SourceControl`. Passing URIs directly keeps that boundary clean; passing batch objects would leak batch semantics into a runner that shouldn't know they exist. -The linearity assumption in point 1 is load-bearing and already guarded upstream: stovepipe assumes a linear trunk by default, and when `SourceControl` reports that last-green is no longer an ancestor of the head (history rewrite), `process` falls back to a full build rather than trusting the interval ([process.md](doc/rfc/stovepipe/steps/process.md#build-strategy-decision)). The URI-pair contract is exactly as expressive as that model — a valid `base..head` range, or a full build with an empty baseline — and nothing more. +The linearity assumption in point 1 is load-bearing and already guarded upstream: stovepipe assumes a linear trunk by default, and when `SourceControl` reports that last-green is no longer an ancestor of the head (history rewrite), `process` falls back to a full build rather than trusting the interval ([process.md](process.md#build-strategy-decision)). The URI-pair contract is exactly as expressive as that model — a valid `base..head` range, or a full build with an empty baseline — and nothing more. So `build`'s `Trigger` gets its own shape under `stovepipe/extension/buildrunner`, still "identity in, resolve internally" — just with URI identity instead of batch identity. `Status` and `Cancel` don't have this mismatch — both domains poll and cancel by the same opaque, runner-minted id with the same async semantics — but that similarity is shaped-the-same, not shared code: they stay on `stovepipe/extension/buildrunner.BuildRunner` too, duplicated in shape from SubmitQueue's, with reuse pushed down to a shared backend implementation instead (see the contract sketch below and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract)). ### Stovepipe `BuildRunner` contract (design sketch) -Not implemented here. `BuildID`, `BuildStatus`, and `BuildMetadata` are defined locally in `stovepipe/entity`, shaped the same as SubmitQueue's equivalents in `submitqueue/entity` but not the same Go types — per the reviewer preference recorded in [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract), a shared `platform/base`/`platform/extension/buildrunner` contract was considered and set aside in favor of keeping each domain's interface separate and reusing at the implementation layer instead. `stovepipe/extension/buildrunner` holds `Trigger`, `Status`, `Cancel`, `Config`, and the `Factory` interface, per [AGENTS.md](AGENTS.md)'s extension rules. +Not implemented here. `BuildID`, `BuildStatus`, and `BuildMetadata` are defined locally in `stovepipe/entity`, shaped the same as SubmitQueue's equivalents in `submitqueue/entity` but not the same Go types — per the reviewer preference recorded in [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract), a shared `platform/base`/`platform/extension/buildrunner` contract was considered and set aside in favor of keeping each domain's interface separate and reusing at the implementation layer instead. `stovepipe/extension/buildrunner` holds `Trigger`, `Status`, `Cancel`, `Config`, and the `Factory` interface, per [AGENTS.md](../../../../AGENTS.md)'s extension rules. ```go // package buildrunner (stovepipe/extension/buildrunner) @@ -190,9 +190,9 @@ type Factory interface{ For(cfg Config) (BuildRunner, error) } #### Project-scoped `Trigger`: reserved, not yet designed -**TODO**, tracked pending the `analyze` design (see [workflow.md](doc/rfc/stovepipe/workflow.md#open-questions)'s "Project mapping contract" open question). The sketch above has only a whole-repo/incremental dimension (`headURI`, `baseURI`); it has no parameter for "build only this project," so as written it cannot express a Phase 2 invocation. `build` itself stays phase-agnostic (see [Input, partitioning, and the single-writer property](#input-partitioning-and-the-single-writer-property)) — it reads whatever scope is already decided and passes it through — but `Trigger` still needs a slot to read that scope from and forward to the runner. +**TODO**, tracked pending the `analyze` design (see [workflow.md](../workflow.md#open-questions)'s "Project mapping contract" open question). The sketch above has only a whole-repo/incremental dimension (`headURI`, `baseURI`); it has no parameter for "build only this project," so as written it cannot express a Phase 2 invocation. `build` itself stays phase-agnostic (see [Input, partitioning, and the single-writer property](#input-partitioning-and-the-single-writer-property)) — it reads whatever scope is already decided and passes it through — but `Trigger` still needs a slot to read that scope from and forward to the runner. -The shape isn't decided here because project semantics belong to `analyze`, not `build`: how a project maps to a buildable scope (a Bazel target pattern, a directory, a service name) is implementer-specific per [workflow.md](doc/rfc/stovepipe/workflow.md#project---greenness-at-a-finer-grain). The expectation is that this stays an opaque token — following the same "identity in, resolve internally" shape already used for `headURI`/`baseURI` (owned and interpreted by `SourceControl`) — that `build` reads off the `Request`/message and hands to the runner uninterpreted, rather than a structured type `build` would have to understand: +The shape isn't decided here because project semantics belong to `analyze`, not `build`: how a project maps to a buildable scope (a Bazel target pattern, a directory, a service name) is implementer-specific per [workflow.md](../workflow.md#project---greenness-at-a-finer-grain). The expectation is that this stays an opaque token — following the same "identity in, resolve internally" shape already used for `headURI`/`baseURI` (owned and interpreted by `SourceControl`) — that `build` reads off the `Request`/message and hands to the runner uninterpreted, rather than a structured type `build` would have to understand: ```go Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error) @@ -232,7 +232,7 @@ Several shapes for sharing the `BuildRunner` contract across domains were raised }) ``` - Trade-offs: this inverts the metadata contract — `BuildMetadata` is caller annotation the runner echoes but **must not depend on** ([build-runner.md](doc/rfc/submitqueue/build-runner.md#buildmetadata)). Routing the build's one load-bearing input through it would make the scope untyped, unvalidated, and invisible in the interface — a runner correctly honoring the "must not depend on metadata" rule would ignore `head_uri`/`base_uri` entirely and build the wrong scope. + Trade-offs: this inverts the metadata contract — `BuildMetadata` is caller annotation the runner echoes but **must not depend on** ([build-runner.md](../../submitqueue/build-runner.md#buildmetadata)). Routing the build's one load-bearing input through it would make the scope untyped, unvalidated, and invisible in the interface — a runner correctly honoring the "must not depend on metadata" rule would ignore `head_uri`/`base_uri` entirely and build the wrong scope. - **Shared `Status`/`Cancel` via a `platform/extension/buildrunner.StatusCanceller` sub-interface, with `BuildID`/`BuildStatus`/`BuildMetadata` promoted to `platform/base`.** An earlier draft of this doc adopted exactly this: since both domains poll and cancel by the same opaque, runner-minted id with the same async semantics, `Status`/`Cancel` moved to a shared interface embedded in each domain's `BuildRunner`, with the supporting types promoted to `platform/base` so both sides used the same Go types (a dual-implementing backend would then satisfy both interfaces through one embedded method set). Trade-offs: set aside on review — splitting one conceptual contract (`Trigger` + `Status` + `Cancel`) across two packages (`platform/extension/buildrunner` for two of the three methods, `{domain}/extension/buildrunner` for the third) fragments a single interface across an ownership boundary for a resemblance that isn't yet load-bearing: SubmitQueue is the only existing consumer of the "shared" half today, and the promotion cost — migrating SubmitQueue's already-shipped controllers, storage, and protobuf mappings onto the shared type — bought less than keeping each domain's `BuildRunner` whole and pushing reuse down to the implementation layer instead, per the option below. @@ -266,7 +266,7 @@ Key the `Build` by identity derived from the Request — `buildKey(R) = R.ID` fo | Pros | Cons | |---|---| | Redelivery dedup by direct get: checking `BuildStore.Get(buildKey(R))` before triggering means at-least-once delivery never starts a second build | A second id concept (`Build.ID` beside `Build.RunnerBuildID`) carried by every entity, signature, and reader forever | -| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](AGENTS.md) | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs | +| `Request` → `Build` navigation with no reverse index, per the KV key-derivation rule in [AGENTS.md](../../../../AGENTS.md) | No current reader needs to *derive* a build id — the id travels in every message hop, so each consumer already holds the key it needs | | Enforces (rather than assumes) the direct-navigation property SubmitQueue's speculate takes on faith | Diverges entity shape and controller flow from SubmitQueue, weakening the "structurally the same controller" claim and dual-implementing-backend symmetry | Trade-offs: the dedup guards a rare event at a permanent modeling cost. The duplicate it prevents arises only from a redelivery inside the trigger window — rare, and already harmless (identical scope; `buildsignal`'s superseded short-circuit and its first-writer-wins outcome CAS make the loser a no-op — see [Idempotency](#idempotency)). The prospective key-derivers — a future canceller, or `analyze` reaching back to the Phase-1 target graph — would need to be handed the id by their producing stage instead, if those designs land. @@ -289,7 +289,7 @@ Either could be adopted independently: the idempotency token, if a backend that ### Carries over vs. new -- **Shaped the same, not shared code**: the `BuildStatus` enum and `IsTerminal()` (nothing batch-specific — [build-runner.md](doc/rfc/submitqueue/build-runner.md#buildstatus)); `BuildMetadata` (caller-supplied, provider-echoed, controller-uninterpreted — [#buildmetadata](doc/rfc/submitqueue/build-runner.md#buildmetadata)); the async contract — `Trigger` returns a handle not an outcome, `Status` may round-trip, `Cancel` reaches the provider not the engine ([#async-vs-sync-contract](doc/rfc/submitqueue/build-runner.md#async-vs-sync-contract)); and the id model — no caller-supplied id, the runner mints the build's identity, and that one `entity.BuildID` is the store key, queue payload, and `Status`/`Cancel` parameter (see [Alternatives considered](#alternatives-considered-for-the-build-identity)). These are duplicated locally in `stovepipe/entity`/`stovepipe/extension/buildrunner` rather than promoted to `platform/base`/`platform/extension/buildrunner` — see the contract sketch above and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract). +- **Shaped the same, not shared code**: the `BuildStatus` enum and `IsTerminal()` (nothing batch-specific — [build-runner.md](../../submitqueue/build-runner.md#buildstatus)); `BuildMetadata` (caller-supplied, provider-echoed, controller-uninterpreted — [#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)); the async contract — `Trigger` returns a handle not an outcome, `Status` may round-trip, `Cancel` reaches the provider not the engine ([#async-vs-sync-contract](../../submitqueue/build-runner.md#async-vs-sync-contract)); and the id model — no caller-supplied id, the runner mints the build's identity, and that one `entity.BuildID` is the store key, queue payload, and `Status`/`Cancel` parameter (see [Alternatives considered](#alternatives-considered-for-the-build-identity)). These are duplicated locally in `stovepipe/entity`/`stovepipe/extension/buildrunner` rather than promoted to `platform/base`/`platform/extension/buildrunner` — see the contract sketch above and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract). - **Shared as implementation, not contract**: a concrete backend (e.g. Buildkite) can satisfy both domains' `BuildRunner` interfaces off one client, sharing HTTP/auth/poll-loop plumbing internally even though the two `BuildRunner` interfaces it implements are separate types — see the "Shared backend under `platform`, thin per-domain contracts" option above. - **New in stovepipe**: URI-based scope in `Trigger` instead of batch lists — the one part of the contract that was always domain-specific by necessity, per [Why separate contracts](#why-separate-contracts). Mapping targets to projects is still stovepipe-only; how `analyze` obtains a target graph is left to its own design, out of scope for this doc. @@ -318,7 +318,7 @@ The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrateg | `failed` | Build finished, at least one check failed | **yes** | | `cancelled` | Build stopped before finishing (see [Cancellation](#cancellation-defined-not-yet-called)) | **yes** | -`IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md#algorithm), step 6). +`IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](buildsignal.md#algorithm), step 6). Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed. @@ -326,9 +326,9 @@ Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only tra - `Create(ctx, build entity.Build) error` — `ErrAlreadyExists` if the id is taken. - `Get(ctx, id string) (entity.Build, error)` — `ErrNotFound` if absent. -- `Update(ctx, build entity.Build, oldVersion, newVersion int32) error` — pure conditional write; `ErrVersionMismatch` on a stale guard. The controller computes `newVersion = oldVersion + 1`, calls the store, and assigns `build.Version = newVersion` only on success (see [AGENTS.md](AGENTS.md) and the [storage README](submitqueue/extension/storage/README.md)). +- `Update(ctx, build entity.Build, oldVersion, newVersion int32) error` — pure conditional write; `ErrVersionMismatch` on a stale guard. The controller computes `newVersion = oldVersion + 1`, calls the store, and assigns `build.Version = newVersion` only on success (see [AGENTS.md](../../../../AGENTS.md) and the [storage README](../../../../submitqueue/extension/storage/README.md)). -Single-key reads/writes only — no list-by-request, no query-by-attribute — per the key/value-shaped extension rule in [AGENTS.md](AGENTS.md). +Single-key reads/writes only — no list-by-request, no query-by-attribute — per the key/value-shaped extension rule in [AGENTS.md](../../../../AGENTS.md). **`Request` additions** (extending the existing entity, which already has `ID/Queue/URI/State/Version`): @@ -341,10 +341,10 @@ Single-key reads/writes only — no list-by-request, no query-by-attribute — p ## Queue contract additions -Two topic keys in `stovepipe/core/messagequeue/topics.go` — `TopicKeyBuild` (`process`/`analyze` → `build`) and `TopicKeyBuildSignal` (`build` → `buildsignal`) — and one proto message per key, since the contract test binds **exactly one message to each topic key** (see [messagequeue-contract.md](doc/rfc/messagequeue-contract.md)). `ProcessRequest` is bound to `process` and cannot be reused; the new messages mirror its shape (one id field, its own `topic_keys` option): +Two topic keys in `stovepipe/core/messagequeue/topics.go` — `TopicKeyBuild` (`process`/`analyze` → `build`) and `TopicKeyBuildSignal` (`build` → `buildsignal`) — and one proto message per key, since the contract test binds **exactly one message to each topic key** (see [messagequeue-contract.md](../../messagequeue-contract.md)). `ProcessRequest` is bound to `process` and cannot be reused; the new messages mirror its shape (one id field, its own `topic_keys` option): - `BuildRequest{ id }` (request id) → `topic_keys "build"`, produced by `process`/`analyze`, consumed by `build`. Phase 2's per-project trigger must also identify its project; because each topic key binds exactly one message, that lands as an **additive optional field on this same message** (protojson discards unknown fields, so the evolution is backward-compatible), not a second message type — the field's shape is deferred to the `analyze` design with the rest of the project-scoped trigger. -- `BuildSignal{ id }` (build id) → `topic_keys "buildsignal"`, produced by `build` and re-produced by `buildsignal`, consumed by `buildsignal` (see [buildsignal.md](doc/rfc/stovepipe/steps/buildsignal.md)). +- `BuildSignal{ id }` (build id) → `topic_keys "buildsignal"`, produced by `build` and re-produced by `buildsignal`, consumed by `buildsignal` (see [buildsignal.md](buildsignal.md)). ### Partitioning diff --git a/doc/rfc/stovepipe/steps/buildsignal.md b/doc/rfc/stovepipe/steps/buildsignal.md index 7ce0dfcaa..b51ddbfc1 100644 --- a/doc/rfc/stovepipe/steps/buildsignal.md +++ b/doc/rfc/stovepipe/steps/buildsignal.md @@ -1,18 +1,18 @@ # Buildsignal stage -`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the **request id** to `record`. See [workflow.md](doc/rfc/stovepipe/workflow.md) for where it sits in the pipeline and [build.md](doc/rfc/stovepipe/steps/build.md) for how the build it polls was triggered. +`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the **request id** to `record`. See [workflow.md](../workflow.md) for where it sits in the pipeline and [build.md](build.md) for how the build it polls was triggered. It handles only the poll loop: it does not decide build strategy, write greenness, or map targets to projects — those are `process`'s, `record`'s, and `analyze`'s jobs. It reports the *fact* (this request's build ended succeeded, failed, or cancelled); `record` derives what that fact *means* for greenness. -`buildsignal` is structurally the same controller as `submitqueue/orchestrator/controller/buildsignal/buildsignal.go`, and the "Flow", "Status delivery", and "Polling primitive" sections of [doc/rfc/submitqueue/build-runner.md](doc/rfc/submitqueue/build-runner.md) are the reference rationale it reuses directly. The entity and storage shapes it depends on are defined in [build.md](doc/rfc/stovepipe/steps/build.md#entity-and-storage-additions-needed); this doc introduces none of its own. +`buildsignal` is structurally the same controller as `submitqueue/orchestrator/controller/buildsignal/buildsignal.go`, and the "Flow", "Status delivery", and "Polling primitive" sections of [doc/rfc/submitqueue/build-runner.md](../../submitqueue/build-runner.md) are the reference rationale it reuses directly. The entity and storage shapes it depends on are defined in [build.md](build.md#entity-and-storage-additions-needed); this doc introduces none of its own. ## Input and re-entrancy -`buildsignal` consumes a build id, published by `build` — once per phase, since Phase 1 (whole-repo) and Phase 2 (per-project) each run their own `build` → `buildsignal` cycle against builds tied to the same `Request` (see [workflow.md](doc/rfc/stovepipe/workflow.md#workflow)). +`buildsignal` consumes a build id, published by `build` — once per phase, since Phase 1 (whole-repo) and Phase 2 (per-project) each run their own `build` → `buildsignal` cycle against builds tied to the same `Request` (see [workflow.md](../workflow.md#workflow)). Its logic does not branch on phase: it loads the `Build`, polls it toward terminal, persists the result, and publishes the request id onward to `record`. What differs between phases is what `record` does with that publish (whole-repo vs. per-project greenness) — not anything `buildsignal` decides. -`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](doc/rfc/stovepipe/steps/build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once, at the terminal transition, to record the build's outcome — the one `Request.State` write outside `process` and the DLQ reconciler. +`buildsignal` is the sole writer of `Build.Status`/`Build.Version` after `build` creates the row (see [build.md](build.md#input-partitioning-and-the-single-writer-property)). It reads `Request` via `RequestStore.Get` (for `R.Queue`, to resolve the build-runner) and writes it exactly once, at the terminal transition, to record the build's outcome — the one `Request.State` write outside `process` and the DLQ reconciler. Its early-exit guard is deliberately narrower than `State.IsTerminal()`: it proceeds when the request is `processing` **or** already carries a build outcome. The second case matters because a redelivery after the outcome was stamped but before the `record` publish landed must re-publish rather than drop the signal; everything it re-runs is a no-op (the status is unchanged, the outcome is already recorded, the slot is not released twice) and the `record` publish is idempotent. @@ -24,7 +24,7 @@ For a delivery carrying build id `B`: ``` 1. Load Build B from the build store. - - ErrNotFound -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](stovepipe/extension/storage/README.md)). + - ErrNotFound -> return raw; non-retryable (storage is read-after-write consistent; see [storage README](../../../../stovepipe/extension/storage/README.md)). - other store error -> return raw; classifier decides. 2. Load Request R = store.Get(Build.RequestID) — needed for R.Queue to resolve the build-runner. @@ -82,17 +82,17 @@ For a delivery carrying build id `B`: exactly one poll chain; no message ids are minted and no new rows are written per tick. ``` -**Why the slot is released before the outcome write, and why a failed release aborts it**: `Queue` and `Request` are separate entities with no cross-entity transaction, so the ordering picks which crash failure mode we accept. Both rules serve one invariant — *the request must not go terminal while still holding a slot* — because a terminal request is skipped by redelivery and by the DLQ reconciler alike, so nothing would ever decrement it. Failing this way leaves the request non-terminal: redelivery re-runs both steps and decrements again, transiently over-admitting by one slot until the zero clamp reconverges. Over-admission is the failure mode this pipeline already prefers, for the same reason and in the same words as the DLQ reconciler (see [process.md](doc/rfc/stovepipe/steps/process.md#in_flight_count-integrity)). +**Why the slot is released before the outcome write, and why a failed release aborts it**: `Queue` and `Request` are separate entities with no cross-entity transaction, so the ordering picks which crash failure mode we accept. Both rules serve one invariant — *the request must not go terminal while still holding a slot* — because a terminal request is skipped by redelivery and by the DLQ reconciler alike, so nothing would ever decrement it. Failing this way leaves the request non-terminal: redelivery re-runs both steps and decrements again, transiently over-admitting by one slot until the zero clamp reconverges. Over-admission is the failure mode this pipeline already prefers, for the same reason and in the same words as the DLQ reconciler (see [process.md](process.md#in_flight_count-integrity)). **Why `buildsignal` releases the slot rather than `record`**: the gate `process` claims is a *build* slot — it exists to bound concurrent builds per Queue — and once the build is terminal the build is over. Releasing here also keeps the invariant *a terminal `Request` has already released its slot*, which is what makes the DLQ reconciler's early-return on terminal requests safe. **Why `record` hears only terminal signals**: `record` has no non-terminal work — by its own contract a non-terminal signal would be a pure no-op — and step 7 already branches on terminality to decide whether to keep polling, so gating the publish costs nothing and spares `record` a no-op delivery on every poll tick of every running build. Crash-safety is unaffected: a crash between the terminal `Update` and the publish redelivers the message; step 5 re-polls (the runner reports the same terminal status), step 6 no-ops, step 7 publishes. This is a deliberate divergence from SubmitQueue, whose buildsignal republishes to `speculate` on every tick — sound there because speculate is a state machine that may act on any signal; stovepipe has no such consumer. -**Why step 6 guards on status and makes terminal write-once**: an unchanged status skips the CAS write entirely, so a long build being polled every couple of seconds doesn't churn `Build.Version` on every tick — the version only advances on a real state transition. The write-once rule exists because CAS alone cannot provide it: optimistic locking defends against *concurrent* writers, but a later delivery that polls a flaky backend and sees a different terminal status would CAS cleanly against the current version and overwrite (see Edge cases). A given `Build` has a single poll partition (see [Partitioning](doc/rfc/stovepipe/steps/build.md#partitioning)), so the only writer racing the CAS is a redelivery of the same message (e.g. after a lapsed visibility lease); `ErrVersionMismatch` carries a retryable classification and converges on redelivery. +**Why step 6 guards on status and makes terminal write-once**: an unchanged status skips the CAS write entirely, so a long build being polled every couple of seconds doesn't churn `Build.Version` on every tick — the version only advances on a real state transition. The write-once rule exists because CAS alone cannot provide it: optimistic locking defends against *concurrent* writers, but a later delivery that polls a flaky backend and sees a different terminal status would CAS cleanly against the current version and overwrite (see Edge cases). A given `Build` has a single poll partition (see [Partitioning](build.md#partitioning)), so the only writer racing the CAS is a redelivery of the same message (e.g. after a lapsed visibility lease); `ErrVersionMismatch` carries a retryable classification and converges on redelivery. ## Status: shaped like SubmitQueue's, not shared code -`Status` is not a point of conceptual divergence from SubmitQueue: both domains' `BuildRunner`s poll by the same opaque, runner-minted id with the same `Status(ctx, buildID) (BuildStatus, BuildMetadata, error)` signature. That similarity stays a shape, not a shared `platform/extension/buildrunner` interface — each domain keeps its own `BuildRunner` (including its own local `Status`), with reuse pushed to a shared backend implementation instead (see [build.md](doc/rfc/stovepipe/steps/build.md#alternatives-considered-for-sharing-the-contract)). `BuildMetadata` is caller-supplied and provider-echoed — the runner must not depend on it, but nothing stops a consumer from reading it. `buildsignal`'s own poll loop (steps 6-8) doesn't need to interpret it to decide when to stop polling, same as SubmitQueue's, but that's a statement about what the poll loop happens to need, not a rule that the value is unused: `build-runner.md` describes its purpose as round-tripping to users ([#buildmetadata](doc/rfc/submitqueue/build-runner.md#buildmetadata)), and a future check in either domain's buildsignal is free to read it — e.g. to short-circuit some behavior — without changing the contract. +`Status` is not a point of conceptual divergence from SubmitQueue: both domains' `BuildRunner`s poll by the same opaque, runner-minted id with the same `Status(ctx, buildID) (BuildStatus, BuildMetadata, error)` signature. That similarity stays a shape, not a shared `platform/extension/buildrunner` interface — each domain keeps its own `BuildRunner` (including its own local `Status`), with reuse pushed to a shared backend implementation instead (see [build.md](build.md#alternatives-considered-for-sharing-the-contract)). `BuildMetadata` is caller-supplied and provider-echoed — the runner must not depend on it, but nothing stops a consumer from reading it. `buildsignal`'s own poll loop (steps 6-8) doesn't need to interpret it to decide when to stop polling, same as SubmitQueue's, but that's a statement about what the poll loop happens to need, not a rule that the value is unused: `build-runner.md` describes its purpose as round-tripping to users ([#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)), and a future check in either domain's buildsignal is free to read it — e.g. to short-circuit some behavior — without changing the contract. Returning `TargetGraph` from `Status` in place of `BuildMetadata`, with `buildsignal` persisting it for `analyze` to read later, was considered and set aside — how `analyze` obtains the target graph is left to its own design, not `buildsignal`'s poll loop. @@ -103,7 +103,7 @@ On non-terminal status, step 8 holds the delivery, never `Nack`s: - **`Nack`** requeues and increments `retry_count`; at `MaxAttempts` the message dead-letters. That is the primitive for "something failed; retry." - **Hold** postpones the same delivery for a delay; the redelivery restarts failure accounting. That is the primitive for "still working; check back later." -Polling is a scheduled heartbeat, neither failure nor retry, so a long-running build never burns `retry_count` toward the DLQ. A genuine `Status` failure (runner down, bad id) is a *different* path: it returns from step 5 to the classifier, which decides retryability, and a retryable verdict nacks normally. See [consumer-hold.md](doc/rfc/consumer-hold.md) for the primitive's full rationale. +Polling is a scheduled heartbeat, neither failure nor retry, so a long-running build never burns `retry_count` toward the DLQ. A genuine `Status` failure (runner down, bad id) is a *different* path: it returns from step 5 to the classifier, which decides retryability, and a retryable verdict nacks normally. See [consumer-hold.md](../../consumer-hold.md) for the primitive's full rationale. ## Poll delays @@ -118,14 +118,14 @@ Package-level `var`s (not `const`s) so tests can shorten them; the server always ## Error classification -Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative terminal `failed` so a Request never wedges its Queue's slot ([workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit: +Per `platform/errs`'s non-retryable-by-default rule (see [platform/errs/README.md](../../../../platform/errs/README.md)), a plain returned error is already non-retryable and rejects straight to DLQ, where the fail-closed path forces a conservative terminal `failed` so a Request never wedges its Queue's slot ([workflow.md](../workflow.md#fail-closed-on-unprocessable-work)). So this section documents only the departures from that default, not every failure the algorithm can hit: | Failure | Disposition | Why | |---|---|---| | `Status` call | raw error; classifier decides | Deliberately left open rather than fixed either way — runner timeout/connection is transient, "runner not deployed for this queue" is not, and only a backend classifier can tell them apart. **This means the `BuildRunner` backend has to classify**: an unclassified transport or HTTP error gets the non-retryable default, so one proxy blip ends the poll chain (see below). | | `Update` CAS conflict (`ErrVersionMismatch`) | declaration-level retryable | A concurrent (redelivered) writer moved the row; reload and re-check converges. | -`Build`/`Request` not found (`storage.ErrNotFound`) are **not** in this table: storage is required to be read-after-write consistent (see [storage README](stovepipe/extension/storage/README.md)), so a miss here is already the correct default (non-retryable, straight to DLQ) rather than a departure worth overriding. +`Build`/`Request` not found (`storage.ErrNotFound`) are **not** in this table: storage is required to be read-after-write consistent (see [storage README](../../../../stovepipe/extension/storage/README.md)), so a miss here is already the correct default (non-retryable, straight to DLQ) rather than a departure worth overriding. Everything else — factory lookup, an `Update` store error other than a CAS conflict, and the `record` publish — is returned raw with no override, because the default is already correct: a queue with no registered runner is a config error, and storage/queue failures dead-letter and let DLQ reconciliation recover. The poll loop itself no longer has a publish to fail: holding is a local outcome, and a failed postpone write in the framework lapses into a normal visibility-timeout redelivery, so the loop's liveness never rides on an enqueue succeeding. @@ -153,15 +153,15 @@ The window to guard is between persisting status (step 6) and ack (steps 7–8); ## Edge cases -- **Runner has no record of this build id** (runner restarted without persisting in-flight state; a foreign id leaked in). `Status` returns an error, not a status value — non-retryable by default, unless the classifier has a domain sentinel for "unknown build" it chooses to treat as retryable (a restart may self-resolve). Left to the classifier, not hardcoded, per [build-runner.md](doc/rfc/submitqueue/build-runner.md#error-classification). +- **Runner has no record of this build id** (runner restarted without persisting in-flight state; a foreign id leaked in). `Status` returns an error, not a status value — non-retryable by default, unless the classifier has a domain sentinel for "unknown build" it chooses to treat as retryable (a restart may self-resolve). Left to the classifier, not hardcoded, per [build-runner.md](../../submitqueue/build-runner.md#error-classification). - **A later `Status` returns a *different* terminal status than what's stored** (a flaky backend flipping `succeeded`→`failed` between polls). CAS is *not* the defense here: the earlier delivery already committed its write and acked, so a later delivery would CAS cleanly against the current version — optimistic locking guards concurrent writers, not sequential overwrites. The defense is step 6's write-once rule: a stored terminal status is never overwritten, the delivery proceeds with the stored value, and `record` only ever hears one verdict per build. First terminal wins by design; a backend that flip-flops terminal states is broken in a way consumer-side ordering cannot repair, so a deterministic verdict is the most the pipeline can offer. ## Fail-closed interaction -A build that never reaches terminal `Status` — runner outage, a build the runner lost — must not wedge its `Request` forever, since callers gate deployments on greenness reaching a recorded terminal state. `buildsignal` does not implement the forcing function: per [workflow.md](doc/rfc/stovepipe/workflow.md#fail-closed-on-unprocessable-work) and the `in_flight_count` slot lifecycle in [process.md](doc/rfc/stovepipe/steps/process.md#concurrency-lifecycle), a `Request` stuck at `buildsignal` past `MaxAttempts` dead-letters, and the DLQ reconciler forces a conservative terminal `failed` and releases the Queue's slot. This is the same posture SubmitQueue's build/buildsignal pair relies on: terminal status is what releases the slot and lets validation progress. +A build that never reaches terminal `Status` — runner outage, a build the runner lost — must not wedge its `Request` forever, since callers gate deployments on greenness reaching a recorded terminal state. `buildsignal` does not implement the forcing function: per [workflow.md](../workflow.md#fail-closed-on-unprocessable-work) and the `in_flight_count` slot lifecycle in [process.md](process.md#concurrency-lifecycle), a `Request` stuck at `buildsignal` past `MaxAttempts` dead-letters, and the DLQ reconciler forces a conservative terminal `failed` and releases the Queue's slot. This is the same posture SubmitQueue's build/buildsignal pair relies on: terminal status is what releases the slot and lets validation progress. -One boundary of that posture is worth stating: the `MaxAttempts` path fires only when polls *fail*. A runner that keeps answering a healthy non-terminal status forever — a hung build on a backend with no timeout of its own — never errors, so the hold chain (whose redeliveries deliberately do not count toward the retry limit) re-polls indefinitely and nothing dead-letters; SubmitQueue's poll loop shares this property. Bounding it requires a poll deadline — a `max_validation_ms` past which `buildsignal` treats the build as failed and lets the normal terminal path run — which pairs naturally with the lease idea [process.md](doc/rfc/stovepipe/steps/process.md#per-queue-concurrency-gate) floats for `in_flight_count`. Deferred with it; until then a too-old non-terminal `Build` is an operational alert, not a self-healing path. +One boundary of that posture is worth stating: the `MaxAttempts` path fires only when polls *fail*. A runner that keeps answering a healthy non-terminal status forever — a hung build on a backend with no timeout of its own — never errors, so the hold chain (whose redeliveries deliberately do not count toward the retry limit) re-polls indefinitely and nothing dead-letters; SubmitQueue's poll loop shares this property. Bounding it requires a poll deadline — a `max_validation_ms` past which `buildsignal` treats the build as failed and lets the normal terminal path run — which pairs naturally with the lease idea [process.md](process.md#per-queue-concurrency-gate) floats for `in_flight_count`. Deferred with it; until then a too-old non-terminal `Build` is an operational alert, not a self-healing path. ## Entity, storage, and queue additions -No additions beyond [build.md](doc/rfc/stovepipe/steps/build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes/re-produces the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. The message it publishes to `record`, and the `record` topic key itself, are owned by the `record` stage and land with `record.md`; `buildsignal` only needs that the **request id** reaches the record topic once the build is terminal, partitioned by request id. +No additions beyond [build.md](build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes/re-produces the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. The message it publishes to `record`, and the `record` topic key itself, are owned by the `record` stage and land with `record.md`; `buildsignal` only needs that the **request id** reaches the record topic once the build is terminal, partitioned by request id. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 00da86a97..036f9da07 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -2,16 +2,18 @@ Stovepipe answers one question for the rest of the company: **at which commit is this thing green?** It continuously polls a repository branch for its latest commit, validates that commit, works out which projects (if any) are broken at it, records the result, and notifies downstream systems so they can gate deployments on a known-good commit. It is a post-land service: code lands first, Stovepipe finds out whether it was good. -The pipeline is a queue-driven chain of small, single-purpose controllers, in the same style as SubmitQueue (SQ). Each controller consumes one topic, advances one entity, and publishes to the next topic. Most hops carry only an **ID** and the controller reloads the entity from storage; the entry hop carries the caller's input because there is no row to load yet. The high-level shape is: +The pipeline is a queue-driven chain of small, single-purpose controllers, in the same style as SubmitQueue (SQ). Each controller consumes one topic, advances one entity, and publishes to the next topic. Most hops carry only an **ID** and the controller reloads the entity from storage; the entry hop carries the caller's input because there is no row to load yet. What runs today is: -> **poll for a new head → ingest → process the build strategy → build → record greenness → analyze projects → record per-project greenness → notify downstream.** +> **poll for a new head → ingest → process the build strategy → build → buildsignal → record whole-repo greenness (and any project results the resolver returns) → hook.** + +There is no `analyze` controller and no `analyze` topic. A later project-analysis stage that would map a target graph onto another `build` pass is design only, described under [Designed, not built](#designed-not-built-project-analysis). `record` resolves project results inline on the same delivery that writes the repository fact. ## What Stovepipe is agnostic about Two deliberate abstractions keep Stovepipe from being a git tool or a Bazel tool: - **The VCS is behind a `SourceControl` extension.** Stovepipe never shells out to git. Every commit, ref, and branch head is an opaque **URI** that a `SourceControl` implementation produces and interprets. A ref is `git://remote/repo/ref/…`; a specific commit is `git://remote/repo/ref/…/`. The `git://` scheme is just the reference implementation — a Mercurial or Perforce backend would mint its own scheme behind the same contract. Nothing downstream of `SourceControl` parses a URI; it is a token you hand back to `SourceControl` to ask questions ("is A an ancestor of B?", "what is the head of this ref?"). -- **The build system is behind a build-runner extension** (see [build-runner.md](../submitqueue/build-runner.md)), which returns a pass/fail and a **target graph** that the project-analysis stage maps to projects. +- **The build system is behind a build-runner extension** (see [build-runner.md](../submitqueue/build-runner.md)). `Trigger` starts a build at a head URI, optionally against a baseline URI, and `Status` returns pass/fail plus metadata. A target graph that a separate analyze stage would map to projects is part of the unbuilt design, not this contract. Designing to these contracts — not to git and Bazel specifically — is the whole point: the same pipeline should validate any branch in any VCS built by any build system. @@ -41,7 +43,7 @@ Greenness is recorded as a **health degree** where **`0` means green** and **hig ### Project — greenness at a finer grain -A **project** is a caller-defined slice of the repository. Whole-repo greenness answers "is the branch green at this URI"; project greenness answers the question deployments actually need — **"is *this project* green at this URI"**, and its dual, "what is the latest URI at which this project is green". Projects are derived from the build's **target graph**: analysis sees which targets broke and maps them to projects. How targets map to projects is implementer-specific (directory ownership, build metadata, an external service) and lives behind the project-analysis stage, not in the core pipeline. +A **project** is a caller-defined slice of the repository. Whole-repo greenness answers "is the branch green at this URI"; project greenness answers the question deployments actually need — **"is *this project* green at this URI"**, and its dual, "what is the latest URI at which this project is green". The running pipeline does not derive projects from a target graph. On a succeeded or failed request, `record` calls `projectresult.Resolver` and writes one validation fact per result it returns. The example server wires the noop resolver, which returns no results. How a resolver chooses projects is implementer-specific. The separate analyze stage that would map a target graph to project-scoped builds is not built; see [Designed, not built](#designed-not-built-project-analysis). ### Promotion ref — the last green commit, by name @@ -55,14 +57,14 @@ The ref is a *cache* of the last-green URI, not a second record of greenness. It |---|---| | **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range; advance the Queue's **promotion ref** to a commit. The sole owner of URI semantics, including which refs a Queue name resolves to. | | **build-runner** | Build a scope at a URI (optionally relative to a baseline URI), returning pass/fail and the target graph. See [build-runner.md](../submitqueue/build-runner.md). | -| **Hooks** | Deliver Stovepipe's validation events to downstream systems — "validation of this URI has begun", "this URI / this project is now green (or not green)". Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. The shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | +| **Hooks** | Deliver Stovepipe's validation events to downstream systems. What is published today is repository-scoped: validation of this URI has begun, and this URI is green or not green. Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. The shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | | **Storage** | Persist Queues (incl. last-green URI), Requests, build records, and per-URI / per-project greenness. Key/value-shaped per the extension-design rules in [AGENTS.md](../../../AGENTS.md). | -Hooks are the notification boundary. When validation of a commit begins, and when a validation fact is recorded — whole-repo green/not-green, or later a project green/not-green — the event reaches deployment systems, dashboards, and developer tooling without any of them polling Stovepipe's store, and each environment can route it to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. The mechanism is the cross-domain hook framework rather than a call out of the pipeline stages: `process` and `record` publish a `HookEvent` to Stovepipe's `hook` topic, and a dispatcher stage consumes it and invokes the wired hooks, so a slow or failing downstream cannot add latency to the pipeline. Both halves exist; what a deployment supplies is the hooks themselves, since the example server resolves every event to `noop`. See [process.md](steps/process.md#hooks) for the start event and [record.md](steps/record.md#hooks) for the fact-to-event mapping. +Hooks are the notification boundary. When validation of a commit begins, and when a whole-repository validation fact is recorded, the event reaches deployment systems, dashboards, and developer tooling without any of them polling Stovepipe's store, and each environment can route it to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. The mechanism is the cross-domain hook framework rather than a call out of the pipeline stages: `process` and `record` publish a repository-scoped `HookEvent` to Stovepipe's `hook` topic, and a dispatcher stage consumes it and invokes the wired hooks, so a slow or failing downstream cannot add latency to the pipeline. Both halves exist; what a deployment supplies is the hooks themselves, since the example server resolves every event to `noop`. Per-project hook events are not published. See [process.md](steps/process.md#hooks) for the start event and [record.md](steps/record.md#hooks) for the fact-to-event mapping. ## Workflow -The pipeline runs in two phases against the same Request. **Phase 1** establishes whole-repo greenness. **Phase 2** refines it to per-project greenness. Both phases reuse the same `build` → `buildsignal` → `record` machinery; `record` is re-entrant and fans out, which is why it is not a terminal stage. +What runs is one pass per Request. It establishes whole-repository greenness, and on a succeeded or failed outcome `record` also writes whatever project facts `projectresult.Resolver` returns. That call is inline on the record delivery. It does not publish to another stage, and it does not start another build. ``` external poller ──(Queue name)──► ┌──────────────────────────────┐ @@ -82,68 +84,43 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe │ else (history rewrite) │ │ → full monorepo │ └───────────────┬──────────────┘ - ┌────────────────────────────────────┤ RequestID (+ strategy, baseline URI) - │ PHASE 1: whole-repo greenness ▼ - │ ┌──────────────────────────────┐ - │ │ build │ - │ │ Run build-runner for the │ - │ │ chosen scope; baseline = │ - │ │ last-green URI iff incremental│ - │ └───────────────┬──────────────┘ - │ │ BuildID - │ ▼ - │ ┌──────────────────────────────┐ - │ │ buildsignal │ - │ │ Await/record build status + │ - │ │ target graph │ - │ └───────────────┬──────────────┘ - │ │ BuildID - │ ▼ - │ ┌──────────────────────────────┐ Hooks - │ │ record │┄┄┄┄┄► "URI green / - │ │ Write whole-repo greenness │ not green" - │ │ for URI; if green advance │ - │ │ Queue's last-green URI; Hooks │ - │ └───────────────┬──────────────┘ - │ PHASE 2: project greenness │ RequestID - │ ▼ - │ ┌──────────────────────────────┐ - │ │ analyze │ - │ │ Map broken/at-risk targets │ - │ │ → projects (impl-specific); │ - │ │ decide project-scoped builds │ - │ └───────────────┬──────────────┘ - │ │ RequestID (+ project) - │ ▼ - │ ┌──────────────────────────────┐ - │ │ build → buildsignal │ - │ │ CI job runs; artifacts stored │ - │ │ in blob store; status read │ - │ └───────────────┬──────────────┘ - │ │ BuildID - │ ▼ - │ ┌──────────────────────────────┐ Hooks - └───────────────────►│ record │┄┄┄┄┄► "project P - │ Capture per-project greenness │ green / not - │ for the URI; hook event │ green at URI" + │ RequestID (+ strategy, baseline URI) + ▼ + ┌──────────────────────────────┐ + │ build │ + │ Run build-runner for the │ + │ chosen scope; baseline = │ + │ last-green URI iff incremental│ + └───────────────┬──────────────┘ + │ BuildID + ▼ + ┌──────────────────────────────┐ + │ buildsignal │ + │ Poll until terminal; record │ + │ status; release the slot; │ + │ project the outcome │ + └───────────────┬──────────────┘ + │ RequestID + ▼ + ┌──────────────────────────────┐ Hooks + │ record │┄┄┄┄┄► "URI green / + │ Write whole-repo greenness; │ not green" + │ on green advance last-green │ + │ and the promotion ref; │ + │ resolve project results │ + │ inline │ └──────────────────────────────┘ ``` -### Phase 1 — whole-repo greenness - 1. **ingest** — invoked by the external poller with a **Queue name**. It asks `SourceControl` for that Queue's current head URI, mints a Request namespaced by the Queue, persists it with no recorded greenness yet, and dedups on `(Queue, head URI)` so a re-reported head is processed once. It publishes the RequestID onward. 2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, publishes a **hook event** announcing that validation of the commit has begun, and publishes to `build`. See [process.md](steps/process.md). 3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID. -4. **buildsignal** — records the build's status and target graph when the build completes, then releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. -5. **record** — writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It publishes a **hook event** for the green/not-green transition, then fans out into Phase 2. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. - -### Phase 2 — project greenness +4. **buildsignal** — polls until the build is terminal, records that status, releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. +5. **record** — for a succeeded or failed Request, writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It then calls `projectresult.Resolver` and writes one validation fact per returned result. The example server uses the noop resolver, so that list is empty unless a deployment supplies another. It publishes a repository-scoped **hook event** for the green/not-green transition. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. A cancelled Request writes no fact. -6. **analyze** (project-analysis) — takes the build's target graph and maps the relevant targets to **projects**, using whatever implementer-specific mapping is configured. It decides which project-scoped builds / CI jobs are needed to attribute breakage to specific projects, and publishes those builds. -7. **build → buildsignal** — the project-scoped CI job runs; its artifacts are stored in a blob store (e.g. TerraBlob), and `buildsignal` reads back the status. This is the same machinery as Phase 1, reused at project granularity. -8. **record** — captures **per-project greenness for the URI** — for each project, green or not at this commit — and publishes one hook event per project. This is what lets a caller ask "is project P green at URI U?" and "what is the latest URI where project P is green?". +### Designed, not built: project analysis -`record` appearing twice is intentional: it is one re-entrant stage that records greenness at whatever granularity the current phase produced and notifies downstream. The Request is *complete* when every planned granularity has been recorded, not at a single terminal hop. +An `analyze` stage is not part of the pipeline. `stovepipe/core/messagequeue` has no analyze topic, and `stovepipe/controller` has no analyze package. The design, still only a design, is a stage that would take a build's target graph, map broken or at-risk targets to projects, and publish project-scoped builds back through `build` → `buildsignal` → `record`. That second pass, a per-project hook event, and intermediate greenness degrees are not what the controllers do. Open questions for that design stay at the bottom of this doc. ## Per-controller summary @@ -152,9 +129,9 @@ The pipeline runs in two phases against the same Request. **Phase 1** establishe | **ingest** | Queue name (from poller) | process | Resolve head URI via SourceControl, mint Request, persist (no greenness), dedup on `(Queue, head URI)` | | **process** | RequestID | build, hook topic | Build strategy, concurrency gate, backlog coalescing; announce validation start on admit → [process.md](steps/process.md) | | **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental | -| **buildsignal** | BuildID | record (P1), record (P2) | Record build status + target graph; release `in_flight_count`; project the outcome onto the Request; signal completion | -| **record** | RequestID | analyze (P1→P2), hook topic | Write greenness; on whole-repo green advance last-green URI and the promotion ref; publish the hook event | -| **analyze** | RequestID | build | Map broken/at-risk targets → projects; decide project-scoped builds | +| **buildsignal** | BuildID | record | Record terminal build status; release `in_flight_count`; project the outcome onto the Request; publish the request id | +| **record** | RequestID | hook topic | Write whole-repo greenness; resolve project results inline; on green advance last-green URI and the promotion ref; publish the repository hook event | +| **hook** | HookEvent | — | Invoke the hooks the resolver returns. The example server resolves every event to noop | ## Step RFCs @@ -162,8 +139,8 @@ Per-stage design detail lives under `steps/` so this doc stays a pipeline overvi - [process.md](steps/process.md) — build-strategy decision, concurrency gate, backlog coalescing, [concurrency lifecycle](steps/process.md#concurrency-lifecycle), entity changes, [waiting for a slot](steps/process.md#waiting-for-a-slot) - [build.md](steps/build.md) — trigger-only stage: reads the decided scope off the Request, triggers the build-runner, hands off to buildsignal; the stovepipe `BuildRunner` contract and why it differs from SubmitQueue's -- [buildsignal.md](steps/buildsignal.md) — the poll loop: hold-based re-poll cadence, target-graph return, per-build partitioning, and the fail-closed handoff to record -- [record.md](steps/record.md) — turning a terminal build outcome into an immutable validation fact, monotonic last-green advancement and ref promotion, the hook event announcing the outcome, and the deferred analyze handoff +- [buildsignal.md](steps/buildsignal.md) — the poll loop: hold-based re-poll cadence, per-build partitioning, and the fail-closed handoff to record +- [record.md](steps/record.md) — turning a terminal build outcome into an immutable validation fact, monotonic last-green advancement and ref promotion, and the hook event announcing the outcome. Its Phase 2 / analyze handoff is the unbuilt design, not a topic `record` publishes ## Dedup, idempotency, and history rewrites @@ -177,5 +154,4 @@ Callers gate deployments on greenness, so the dangerous failure is a Request tha - **Greenness degree semantics.** The endpoints (`0` green, `1` fully broken) are fixed; the meaning of intermediate values once projects exist (fraction of projects broken? weighted severity?) is deferred until project analysis is concrete. - **Poller vs. webhook ingestion.** Only the external poller is in scope now. The dedup key is designed so a webhook producer can be added later without changing identity, but that producer is out of scope for this RFC. -- **Project mapping contract.** The exact shape of the target-graph→project mapping behind `analyze` (and whether it is a Stovepipe extension or an external service) is left to the project-analysis design. - +- **Project mapping contract.** Not built. There is no analyze controller or topic. `record` already persists facts from `projectresult.Resolver` on the same delivery as the repository fact. The unbuilt analyze design — a target-graph mapping, project-scoped builds, and whether that mapping is an extension or an external service — is still open. diff --git a/doc/rfc/submitqueue/extension-contract.md b/doc/rfc/submitqueue/extension-contract.md index d45498a74..71022558b 100644 --- a/doc/rfc/submitqueue/extension-contract.md +++ b/doc/rfc/submitqueue/extension-contract.md @@ -1,6 +1,10 @@ # Extension Contract -Design notes for what SubmitQueue's pluggable extensions accept: orchestrator **identity** they resolve themselves, versus **controller-resolved data**. Decisions and rationale only; the code changes land after this RFC is reviewed. +Design notes for what SubmitQueue's pluggable extensions accept: orchestrator **identity** they resolve themselves, versus **controller-resolved data**. + +## Status + +Implemented for the extensions named here. `changeprovider.Get` takes `entity.Request`. `conflict.Analyzer.Analyze` takes the batch under analysis and the in-flight batches. `buildrunner.Trigger` takes base `[]entity.Batch` and a head `entity.Batch`. `scorer.Score` takes `entity.Batch` and `entity.SpeculationPathSet`. There is no separate score stage; speculation asks the scorer. `changeset.Resolver` is the shared batch-to-changes reader and still declares its own `Stores` slice rather than importing an orchestrator aggregate. Runway still performs the asynchronous conflict check and the land. In the verdict table, the proposed inputs are these signatures, except the scorer, which also receives the path set. ## Problem @@ -21,9 +25,9 @@ Both unblock with the shape `conflict` already uses: accept identity, resolve in | Stage | Loads | Resolves for the extension | Hands to the extension | |---|---|---|---| -| `validate` | `entity.Request` | nothing — `request.Change` is already in hand (the change-store reads here serve duplicate detection) | `request.Change` → `changeprovider` | +| `validate` | `entity.Request` | the provider reads `request.Change`; change-store reads here serve duplicate detection | `entity.Request` → `changeprovider` | | `dependency` | `entity.Batch` + active `[]entity.Batch` | **nothing** — the batch it analyzes is already persisted, with `Contains` set to `[requestID]` | `entity.Batch`, `[]entity.Batch` → `conflict` | -| `score` | `entity.Batch`, then each `entity.Request` | batch → requests | `request.Change` per request, then multiplies the scores → `scorer` | +| speculation | `entity.Batch` and its path set | the scorer does not receive a controller-resolved `Change` | `entity.Batch`, `entity.SpeculationPathSet` → `scorer` | | `build` | head `entity.Batch` + path base `[]entity.Batch` | **nothing** — the build runner resolves each batch through its injected `changeset.Resolver` | base `[]entity.Batch`, head `entity.Batch` → `buildrunner` | This grounds `conflict` as the baseline: it already resolves nothing because the controller passes the identity it needs. diff --git a/doc/rfc/submitqueue/list-api.md b/doc/rfc/submitqueue/list-api.md index 3fd6305f8..04429b63e 100644 --- a/doc/rfc/submitqueue/list-api.md +++ b/doc/rfc/submitqueue/list-api.md @@ -1,5 +1,7 @@ # Gateway List API +**Superseded.** This design was not implemented. The gateway list and request-summary APIs that shipped are specified in [status-list-api.md](status-list-api.md). Read that document for the current contract, including receipt-time windows rather than the lifecycle-overlap window below. This file is kept only as the earlier proposal. + Design notes for a gateway `List` API that powers a queue-scoped UX for observing SubmitQueue requests over a time window. diff --git a/doc/rfc/submitqueue/modular-queue-wiring.md b/doc/rfc/submitqueue/modular-queue-wiring.md index 6bcfa412a..7e95e29d4 100644 --- a/doc/rfc/submitqueue/modular-queue-wiring.md +++ b/doc/rfc/submitqueue/modular-queue-wiring.md @@ -1,18 +1,22 @@ # Modular Queue Wiring -Design notes for making the orchestrator's per-queue extension wiring and topic-registry setup modular, reusable, and importable by external deployers. Decisions and rationale only; the code changes land after this RFC is reviewed. +Design notes for making the orchestrator's per-queue extension wiring and topic-registry setup modular, reusable, and importable by external deployers. + +## Status + +Implemented for the SubmitQueue orchestrator. `submitqueue/orchestrator/pipeline.go` declares `Deps` and `Stages`. `service/submitqueue/orchestrator/server/main.go` fills `Deps` from `profiles.go` and calls `pipeline.Construct`. Stovepipe remains hand-wired in `service/stovepipe/server/main.go`. The gateway and Runway servers also register their consumers directly. This document is the design that orchestrator wiring follows. It is not a claim that every service in the repo uses the engine. ## Problem -The orchestrator's example `main.go` (`example/submitqueue/orchestrator/server/main.go`) is ~950 lines that mixes three distinct concerns: +Before `pipeline.Construct`, the orchestrator host (`service/submitqueue/orchestrator/server/main.go`, previously laid out under `example/submitqueue/orchestrator/server/main.go`) mixed three distinct concerns: 1. **Infrastructure bootstrap** — DB connections, logger, metrics, gRPC server, signal handling (~200 lines of generic boilerplate, much of it duplicated between gateway and orchestrator). 2. **Queue topology / topic registry** — `newTopicRegistry` is a static list of 12+ pipeline stages, each with a primary subscription and a mirrored DLQ subscription, plus publish-only topics. Adding or removing a pipeline stage requires editing this function in lockstep with controller registration. 3. **Per-queue extension wiring** — `queueRegistry`, `newQueueRegistry`, and four thin `*Factory` adapter types. The only way to configure which scorer / analyzer / change-provider / build-runner a queue uses is to edit Go code in this file, recompile, and redeploy. -Adding a new queue today requires changes in **three places**: YAML config (`queues.yaml`), Go code (`newQueueRegistry`), and a recompile. Adding a new pipeline stage requires **two coordinated edits** (topic list + controller registration). The topic → subscription → DLQ subscription → DLQ controller linkage is maintained by copy-paste across 12 stages, where forgetting any half creates a silent failure. +Adding a new queue required changes in **three places**: YAML config (`queues.yaml`), Go code (`newQueueRegistry`), and a recompile. Adding a new pipeline stage required **two coordinated edits** (topic list + controller registration). The topic → subscription → DLQ subscription → DLQ controller linkage was maintained by copy-paste across the stages, where forgetting any half creates a silent failure. -The queue-registry pattern is flagged as a candidate for promotion into the domain layer once a second consumer needs the same wiring, data-driven config, or lifecycle requirements. Today the orchestrator's `main.go` wires it inline. +For the orchestrator, that assembly now lives in `pipeline.Construct` and the stage table. Per-queue implementation choice lives in `service/submitqueue/orchestrator/server/profiles.go`. Stovepipe's server still registers each controller by hand. ## Vocabulary @@ -23,7 +27,7 @@ stage one pipeline step: a topic being consumed + the controller consuming it (+ optionally its dead-letter reconciler) engine pipeline.Construct — the ONE shared assembly routine profile the host's per-queue choice of seam impls -host the deployer binary: our own example/ mains, or an external repo's fx/plain-main app +host the deployer binary: our own service/ mains, or an external repo's fx/plain-main app ``` ## Principle @@ -68,7 +72,7 @@ type Component interface { func NewGroup(ordered ...Component) *Group ``` -The engine uses Group internally; hosts can also nest Groups (e.g. two services in one process). This replaces the ad-hoc `sync.WaitGroup` + `chan` + manual error-joining in today's main.go. +The engine uses Group internally; hosts can also nest Groups (e.g. two services in one process). This replaces the ad-hoc `sync.WaitGroup` + `chan` + manual error-joining the host used before `pipeline.Construct`. ### Step 2 · `platform/pipeline` — the engine @@ -267,15 +271,17 @@ row appears on topic "start", partition key "monorepo/exp" deps.BuildRunner.For(Config{QueueName: "monorepo/exp"}) └─▶ Step 4's adapter → profiles.For("monorepo/exp").BuildRunner → local runner (the SAME Deps field answers "monorepo/main" with buildkite - on the next delivery — that's the Factory-as-resolver contract, - identical to today's buildRunnerFactory{queues} in main.go) + on the next delivery — that's the Factory-as-resolver contract + the host's profiles satisfy) controller returns nil ⇒ ack controller returns err ⇒ Step 5's classifier decides: retry (nack) or not; after retry budget ⇒ row moves to "start_dlq" ⇒ Step 2's derived pairing guarantees the reconciler from Step 3's DLQ field is listening there ``` -## Generalizes across all four services +## Shape across services + +Only the orchestrator column is assembled by `pipeline.Construct`. The gateway, Stovepipe, and Runway columns are the intended seams; those servers still register controllers by hand. ``` gateway orchestrator stovepipe runway @@ -640,7 +646,7 @@ Profile selection (which scorer/conflict/build-runner a queue uses) deserves sep ### Current role of QueueConfig -`QueueConfig` today is a single-field entity (`Name string`). Its sole consumer is the gateway's `LandController`, which calls `queueconfig.Store.Get(ctx, queue)` to reject requests targeting unknown queues — a pure name-validation gate. The orchestrator does not import `queueconfig` at all; it maintains its own hardcoded `queueRegistry` with no programmatic link to the YAML config. The TODO on line 475 of the orchestrator example envisions bridging the two ("see also queueconfig.Store, which holds the per-queue data half"), but that bridge does not exist today. +`QueueConfig` is a single-field entity (`Name string`). The gateway's `Land` and `List` controllers call `queueconfig.Store.Get` to reject unknown queues — a pure name-validation gate. The orchestrator host does not read that store. `service/submitqueue/orchestrator/server/profiles.go` chooses each queue's implementations, and the server passes the resulting factories into `orchestrator.Deps`. ### Three options for per-queue extension selection diff --git a/doc/rfc/submitqueue/status-list-api.md b/doc/rfc/submitqueue/status-list-api.md index 36d82c408..d5568464d 100644 --- a/doc/rfc/submitqueue/status-list-api.md +++ b/doc/rfc/submitqueue/status-list-api.md @@ -2,9 +2,9 @@ ## RFC Status -Proposed. +Implemented. The gateway RPCs `GetRequestSummaryByID`, `GetRequestSummaryByChangeURI`, and `List` follow this contract (`submitqueue/gateway/controller/request_summary.go`, `submitqueue/gateway/controller/list.go`). -This RFC replaces the unimplemented design in [list-api.md](list-api.md). It defines the gateway-owned request context and materialized status model used by request-summary retrieval and `List`. +This RFC replaces the unimplemented design in [list-api.md](list-api.md). That file is superseded and kept only as the earlier lifecycle-overlap proposal. This document is the gateway-owned request context and materialized status model used by request-summary retrieval and `List`. ## Problem diff --git a/doc/rfc/submitqueue/workflow.md b/doc/rfc/submitqueue/workflow.md index 92e718f4c..bcbcd1da9 100644 --- a/doc/rfc/submitqueue/workflow.md +++ b/doc/rfc/submitqueue/workflow.md @@ -1,34 +1,43 @@ # Orchestrator Workflow -The orchestrator processes land requests through a queue-driven pipeline of small, single-purpose controllers. The gateway accepts a request over RPC and hands it off asynchronously; from there each controller consumes one topic, advances the request or batch, and publishes to the next topic. Most hops carry only an ID — the controller fetches the entity from storage — while a few entry points (`start`, `buildsignal`, `log`) carry the full payload because there is no row to fetch yet. Some stages cross a service boundary: they publish a full payload to the other service's queue and consume a full payload back, because neither service can read the other's storage. The `validate` and `land` stages adapt SubmitQueue's land work to Runway's `MergeRequest` contract and consume `MergeResult` on `landconflictsignal` / `landsignal`. See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). +The orchestrator processes land requests through the queue-driven pipeline declared in [`submitqueue/orchestrator/pipeline.go`](../../../submitqueue/orchestrator/pipeline.go). The gateway accepts a land over RPC and publishes the full request to `start`; cancel is a separate RPC that publishes to `cancel`. Each controller consumes one topic, reloads what it needs, and publishes the next hop. Inside the orchestrator most hops carry only an ID. The hops that cross into Runway do not: `validate` and `land` publish a full `MergeRequest`, and `landconflictsignal` and `landsignal` consume the `MergeResult` Runway publishes back, because neither service can read the other's storage. Request-log entries are full payloads published with `submitqueue/orchestrator/core/request.PublishLog`; the gateway consumes that topic and is the only writer of the request log. `log`, Runway's `merge-conflict-check`, and Runway's `merge` are publish-only on the orchestrator. See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). -The pipeline has two cycles: `speculate → build → buildsignal → speculate` (CI feedback loop) and `land → runway → landsignal → speculate` (land the batch out of process, then advance the next). `conclude` is the only stage that transitions a request to a terminal state; `log` is an append-only sink that any controller can publish to via `submitqueue/core/request.PublishLog`. +Two cycles re-enter speculation. `speculate` dispatches funded paths to `build`; `build` starts each pending path and publishes the build id to `buildsignal`; `buildsignal` polls with `delivery.Hold` and, when the status changes, publishes the batch id back to `speculate`. A batch that can land is published to `land`, which sends the merge request to Runway; `landsignal` correlates the result, marks the batch Succeeded or Failed, and fans out to `conclude` and `speculate`. `speculate` also publishes Failed and Cancelled batches straight to `conclude`. + +Terminal request states are not `conclude`'s alone. `cancel` completes a request that has not been enrolled in a batch. `landconflictsignal` fails a request Runway reports as conflicted. `conclude` maps a terminal batch onto its member requests. The DLQ reconcilers force a failed terminal state when a consumed stage cannot finish. + +The same `Stages` table registers a `submitqueue-hook` stage, outside the order of the flow above. It consumes hook events and invokes the wired hooks, and it publishes nothing onward. No orchestrator controller publishes a hook event; `service/submitqueue/orchestrator/server` resolves every event to noop. The comment on that row in `pipeline.go` is the contract: any stage can publish there. ## Diagram [View Diagram](https://gitdiagram.com/uber/submitqueue) - ```mermaid flowchart TD subgraph group_gateway["Gateway API"] - node_gateway["Gateway
[land.go]"] + node_gateway["Gateway Land
[land.go]"] + node_gatewaycancel["Gateway Cancel
[cancel.go]"] end subgraph group_orchestration["Queue Orchestration"] node_orchestrator["Orchestrator Service
[main.go]"] - node_pipeline["Pipeline Engine
[pipeline.go]"] - node_start["Start Requests
[start.go]"] - node_validate["Validate Changes
[validate.go]"] - node_runway["Runway Merge
[merge.go]"] - node_conflictsignal["Conflict Signal"] - node_batch["Batch Changes
[batch.go]"] - node_speculate["Speculate States
[speculate.go]"] - node_build["Run Builds
[buildsignal.go]"] - node_buildsignal["Build Results
[buildsignal.go]"] - node_land["Land Changes
[land.go]"] - node_conclude["Conclude Requests
[conclude.go]"] + node_pipeline["Pipeline Topology
[pipeline.go]"] + node_start["start
[start.go]"] + node_cancel["cancel
[cancel.go]"] + node_validate["validate
[validate.go]"] + node_runwaycheck["Runway conflict check
[merge.go]"] + node_conflictsignal["landconflictsignal
[landconflictsignal.go]"] + node_batch["batch
[batch.go]"] + node_dependency["dependency-analysis
[dependencyanalysis.go]"] + node_speculate["speculate
[speculate.go]"] + node_build["build
[build.go]"] + node_buildsignal["buildsignal
[buildsignal.go]"] + node_land["land
[land.go]"] + node_runwaymerge["Runway merge
[merge.go]"] + node_landsignal["landsignal
[landsignal.go]"] + node_conclude["conclude
[conclude.go]"] + node_hook["hook
[controller.go]"] end subgraph group_integrations["External Integrations"] @@ -42,55 +51,70 @@ end subgraph group_platform["Platform State"] node_storagecontract["Storage Contract
[storage.go]"] node_queuestorage[("Queue State
[storage.go]")] - node_requestlog["Request Log
[log.go]"] + node_requestlog["Request Log publish
[log.go]"] node_messagequeue["Message Queue
[queue.go]"] end node_submitter(("Submitter")) -node_submitter -->|"submits changes"| node_gateway -node_gateway -->|"dispatches requests"| node_orchestrator -node_orchestrator -->|"starts pipeline"| node_pipeline +node_submitter -->|"lands"| node_gateway +node_submitter -->|"cancels"| node_gatewaycancel +node_gateway -->|"publishes land request"| node_start +node_gatewaycancel -->|"publishes cancel"| node_cancel +node_orchestrator -->|"declares stages"| node_pipeline node_pipeline -->|"constructs consumers"| node_messagequeue node_pipeline -->|"injects storage"| node_storagecontract -node_start -->|"starts validation"| node_validate -node_validate -->|"checks merges"| node_runway -node_runway -->|"emits signal"| node_conflictsignal -node_conflictsignal -->|"signals batching"| node_batch -node_batch -->|"dispatches batches"| node_speculate -node_speculate -->|"dispatches builds"| node_build -node_build -->|"runs builds"| node_buildrunner +node_start -->|"request id"| node_validate +node_validate -->|"MergeRequest"| node_runwaycheck +node_validate -->|"fetches metadata"| node_changeproviders +node_runwaycheck -->|"MergeResult"| node_conflictsignal +node_conflictsignal -->|"request id"| node_batch +node_batch -->|"batch id"| node_dependency +node_dependency -->|"batch id"| node_speculate +node_cancel -->|"batch id when enrolled"| node_speculate +node_speculate -->|"batch id"| node_build +node_speculate -->|"batch id"| node_land +node_speculate -->|"failed or cancelled batch"| node_conclude +node_build -->|"starts builds"| node_buildrunner +node_build -->|"build id"| node_buildsignal node_buildrunner -.->|"runs CI"| node_buildkite node_buildrunner -.->|"runs workflows"| node_github -node_build -->|"reports results"| node_buildsignal -node_buildsignal -->|"polls outcomes"| node_speculate -node_speculate -->|"lands passing batch"| node_land -node_land -->|"lands changes"| node_changeproviders -node_changeproviders -.->|"updates branches"| node_gitrepository -node_changeproviders -.->|"updates pull requests"| node_github -node_land -->|"concludes landing"| node_conclude -node_conclude -->|"records history"| node_requestlog +node_buildsignal -->|"batch id"| node_speculate +node_land -->|"MergeRequest"| node_runwaymerge +node_runwaymerge -->|"MergeResult"| node_landsignal +node_landsignal -->|"batch id"| node_conclude +node_landsignal -->|"batch id"| node_speculate +node_start -.->|"PublishLog"| node_requestlog +node_conclude -.->|"PublishLog"| node_requestlog +node_changeproviders -.->|"reads changes"| node_gitrepository +node_changeproviders -.->|"reads pull requests"| node_github node_storagecontract -->|"persists state"| node_queuestorage node_orchestrator -->|"reads and writes"| node_queuestorage click node_gateway "https://github.com/uber/submitqueue/blob/main/submitqueue/gateway/controller/land.go" +click node_gatewaycancel "https://github.com/uber/submitqueue/blob/main/submitqueue/gateway/controller/cancel.go" click node_orchestrator "https://github.com/uber/submitqueue/blob/main/service/submitqueue/orchestrator/server/main.go" -click node_pipeline "https://github.com/uber/submitqueue/blob/main/platform/pipeline/pipeline.go" +click node_pipeline "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/pipeline.go" click node_start "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/start/start.go" +click node_cancel "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/cancel/cancel.go" click node_validate "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/validate/validate.go" -click node_runway "https://github.com/uber/submitqueue/blob/main/runway/controller/merge/merge.go" +click node_runwaycheck "https://github.com/uber/submitqueue/blob/main/runway/controller/mergeconflictcheck/mergeconflictcheck.go" click node_conflictsignal "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/landconflictsignal/landconflictsignal.go" click node_batch "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/batch/batch.go" +click node_dependency "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/dependencyanalysis/dependencyanalysis.go" click node_speculate "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/speculate/speculate.go" -click node_build "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/buildsignal/buildsignal.go" +click node_build "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/build/build.go" click node_buildsignal "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/buildsignal/buildsignal.go" click node_land "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/land/land.go" +click node_runwaymerge "https://github.com/uber/submitqueue/blob/main/runway/controller/merge/merge.go" +click node_landsignal "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/landsignal/landsignal.go" click node_conclude "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/controller/conclude/conclude.go" +click node_hook "https://github.com/uber/submitqueue/blob/main/platform/hook/controller.go" click node_changeproviders "https://github.com/uber/submitqueue/blob/main/submitqueue/extension/changeprovider/change_provider.go" click node_buildrunner "https://github.com/uber/submitqueue/blob/main/submitqueue/extension/buildrunner/build_runner.go" click node_storagecontract "https://github.com/uber/submitqueue/blob/main/submitqueue/extension/storage/storage.go" click node_queuestorage "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/extension/storage/mysql/storage.go" -click node_requestlog "https://github.com/uber/submitqueue/blob/main/submitqueue/core/request/log.go" +click node_requestlog "https://github.com/uber/submitqueue/blob/main/submitqueue/orchestrator/core/request/log.go" click node_messagequeue "https://github.com/uber/submitqueue/blob/main/platform/extension/messagequeue/queue.go" classDef toneNeutral fill:#f8fafc,stroke:#334155,stroke-width:1.5px,color:#0f172a @@ -100,58 +124,66 @@ classDef toneMint fill:#dcfce7,stroke:#16a34a,stroke-width:1.5px,color:#14532d classDef toneRose fill:#ffe4e6,stroke:#e11d48,stroke-width:1.5px,color:#881337 classDef toneIndigo fill:#e0e7ff,stroke:#4f46e5,stroke-width:1.5px,color:#312e81 classDef toneTeal fill:#ccfbf1,stroke:#0f766e,stroke-width:1.5px,color:#134e4a -class node_gateway toneBlue -class node_orchestrator,node_pipeline,node_start,node_validate,node_runway,node_conflictsignal,node_batch,node_speculate,node_build,node_buildsignal,node_land,node_conclude toneAmber +class node_gateway,node_gatewaycancel toneBlue +class node_orchestrator,node_pipeline,node_start,node_cancel,node_validate,node_runwaycheck,node_conflictsignal,node_batch,node_dependency,node_speculate,node_build,node_buildsignal,node_land,node_runwaymerge,node_landsignal,node_conclude,node_hook toneAmber class node_changeproviders,node_buildrunner,node_gitrepository,node_github,node_buildkite toneMint class node_storagecontract,node_queuestorage,node_requestlog,node_messagequeue toneRose class node_submitter toneIndigo ``` +`hook` is drawn with the other stages because the topology registers it. Nothing in the orchestrator publishes to it, so the diagram has no edge into that node. + ## Per-controller summary | Controller | In | Out | One-line role | |---|---|---|---| -| **gateway/Land** | RPC | start | Accept request, mint ID, log Accepted, hand off async | -| **start** | LandRequest | validate, log | Persist Request and emit Started log | -| **validate** | RequestID | merge-conflict-check (Runway) | Dedup, fetch change metadata, claim changes, then adapt and publish the full `MergeRequest` to Runway (keyed by the request id, the correlation id) | -| **landconflictsignal** | MergeResult | batch | Correlate Runway's result; advance if landable, fail if conflicted | -| **batch** | RequestID | speculate | Group request into a Batch with dependencies | -| **speculate** | BatchID | build, land | (stub) Decide whether to verify via CI or land | -| **build** | BatchID | buildsignal | Trigger CI build for the batch | -| **buildsignal** | Build | speculate | Feed CI result back into speculation | -| **land** | BatchID | runway-merge (Runway) | Build the full land request from the batch's member requests, adapt it to `MergeRequest`, and publish to Runway keyed by the batch id (the correlation id) | -| **landsignal** | MergeResult | conclude, speculate | Correlate Runway's result; mark the batch Succeeded/Failed and fan out | -| **conclude** | BatchID | — | Map terminal batch state to request state | -| **log** | RequestLog | — | Gateway-owned sink: persists request log events to storage | +| **gateway/Land** | RPC | start | Mint the request id, persist an accepting receipt, publish the full land request, then persist Accepted | +| **gateway/Cancel** | RPC | cancel | Publish a cancel request for an existing land | +| **start** | LandRequest | validate, log | Persist the Request and publish it to validate | +| **cancel** | CancelRequest | log, or speculate | Record Cancelling; finish a request that is not in a batch, or hand each cancellable batch attempt to speculate | +| **validate** | RequestID | merge-conflict-check (Runway), log | Dedup, fetch change metadata, claim changes, then publish the full `MergeRequest` to Runway keyed by the request id | +| **landconflictsignal** | MergeResult | batch, or log | Correlate Runway's check; advance a landable request to batch, or fail a conflicted request | +| **batch** | RequestID | dependency-analysis, log | Mint a Creating batch for the request and hand that batch id onward | +| **dependency-analysis** | BatchID | speculate, log | Enrol the request, resolve what the batch serializes behind, and promote it from Creating to Created | +| **speculate** | BatchID | build, land, conclude | Treat the message as a dirty signal for the queue: admit Created batches, commit outcomes from facts already known, ask the speculator which paths to fund, and dispatch builds, a land, or conclude | +| **build** | BatchID | buildsignal | Start a build for each pending path on the head and publish that build id | +| **buildsignal** | BuildID | speculate | Poll `BuildRunner.Status`, stop a build whose path no longer wants it, and wake speculate when the status changes. In-flight polls `Hold` the same delivery | +| **land** | BatchID | runway-merge (Runway), log | Build the full land request from the batch's member requests, adapt it to `MergeRequest`, and publish it to Runway keyed by the batch id | +| **landsignal** | MergeResult | conclude, speculate | Correlate Runway's merge result, mark the batch Succeeded or Failed, and fan out | +| **conclude** | BatchID | log | Map the batch's terminal state onto its member requests | +| **hook** | HookEvent | — | Invoke the hooks the resolver returns for the event. Publishes nothing onward | +| **log** | RequestLog | — | Gateway-owned sink: persists request log events. The orchestrator only publishes to this topic | + +`speculate` is the decision stage, not a stub. A run reloads the queue's in-flight batches and path sets, admits anything still in Created, finalizes outcomes, and only then asks the speculator for proposals. Path-set changes are persisted before the build dispatch. The package doc in `submitqueue/orchestrator/controller/speculate` is the model of paths, budgets, and bypass. ## DLQ reconciliation -Every *consumed* primary pipeline topic above is paired with a `{topic}_dlq` subscription consumed by a dedicated DLQ controller. The `log` topic is the exception: the orchestrator only publishes to it (the gateway is the sole consumer that persists the request log), so it has no orchestrator-side subscription and therefore no DLQ. The consumer framework moves a message to its DLQ once the primary controller returns a non-retryable error or exhausts retries on a retryable one; without the DLQ side the affected request would stay in a non-terminal state forever and the gateway would still report it as "in progress". +Every consumed stage in `pipeline.go` is paired with a `{topic}_dlq` subscription. `pipeline.Construct` registers that companion with `errs.AlwaysRetryableProcessor` and `DLQSubscriptionConfig`, which disables a second-level DLQ and sets `Retry.MaxAttempts` to 0 (unlimited). The consumer moves a message to its DLQ once the primary controller returns a non-retryable error or exhausts retries on a retryable one. The publish-only topics — `log`, Runway `merge-conflict-check`, and Runway `merge` — have no orchestrator subscription and therefore no orchestrator DLQ. The gateway consumes `log`. Runway consumes the two merge request topics. -The DLQ controllers do not re-attempt the failed work. They decode the payload to recover the affected request (`RequestID`) or batch (`BatchID`) and drive the entity to a terminal failed state — `RequestStateError` for requests, `BatchStateFailed` for batches, with fan-out to the member requests. A DLQ whose topic carries a full payload rather than a bare ID recovers the id from that payload instead — the `landconflictsignal` and `landsignal` DLQs read it from the Runway `MergeResult` the producer echoed back. State writes use the same optimistic-locking CAS as the primary pipeline, so a late primary-pipeline update wins cleanly and a version mismatch is asked back for redelivery. +Most DLQ controllers do not re-attempt the failed work. They decode the payload to a `RequestID` or `BatchID` and drive that entity to a terminal failed state — `RequestStateError` for requests, `BatchStateFailed` for batches, with fan-out to the member requests. Topics that carry a full payload recover the id from it: the `landconflictsignal` DLQ reads the request id from Runway's `MergeResult`, and the `landsignal` DLQ reads the batch id. The `buildsignal` DLQ decodes a build id, loads that build, and fails the batch that owns it. A missing build or an empty batch id is acked: there is no batch to fail from this signal. State writes use the same optimistic-locking CAS as the primary pipeline, so a late primary-pipeline update wins cleanly and a version mismatch is asked back for redelivery. -DLQ consumers are wired with `errs.AlwaysRetryableProcessor` and a very high `Retry.MaxAttempts`, with their own DLQ disabled. That combination makes reconciliation effectively non-droppable: any failure is forced retryable rather than escalating to a second-level dead-letter that nobody consumes. The trade-off is that a genuinely unprocessable DLQ message — typically a malformed payload — must be removed by an operator. +Two consumed stages do not follow that shape. The speculate DLQ fails the batches the failure attributes — often not the batch named on the message, because a run re-plans the whole queue — and then republishes to `speculate`, so one dead letter does not strand every other batch. The hook DLQ records the dropped event and acks. A hook never writes pipeline state, so there is no request or batch to fail; republishing the logged event is how an operator recovers the side effect. A genuinely unprocessable request or batch DLQ message, typically a malformed payload, must be removed by an operator, because those consumers retry until reconciliation succeeds. -See `submitqueue/orchestrator/controller/dlq/README.md` for the design constraints (simplest possible implementation, reconcile-only, no recovery) and the per-topic controller mapping. +See `submitqueue/orchestrator/controller/dlq/README.md` for the reconcile-only constraints. The speculate and hook controllers in that wiring are the exceptions to the generic request/batch mapping that README's table still summarizes. ## Ownership by service -Each service owns its own data; the gateway and orchestrator never touch each other's, and the only thing they share is the messaging queue. +Each service owns its own data. The gateway and orchestrator do not read each other's stores. They share the messaging queue, and the orchestrator shares Runway's merge topics with Runway. ### Gateway -The gateway is the RPC entry point and the owner of the request log. It accepts requests, hands them to the orchestrator over the queue, and owns the record of what happened to each request — the only service that reads or writes the request log. It writes that record both directly, as requests arrive, and by consuming the log events the orchestrator emits. +The gateway is the RPC entry point and the owner of the request log. It accepts land and cancel requests, hands them to the orchestrator over the queue, and owns the record of what happened to each request — the only service that reads or writes the request log. It writes that record both directly, as requests arrive, and by consuming the log events the orchestrator emits. ### Orchestrator -The orchestrator runs the pipeline that advances a request from acceptance to a terminal state. It owns the working state of that pipeline — requests, batches, builds, and their bookkeeping — and is the only service that writes it. It drives a request through a series of internal stages, re-entering speculation as CI results arrive and as batches advance. +The orchestrator runs the pipeline that advances a request from acceptance to a terminal state. It owns the working state of that pipeline — requests, batches, builds, speculation path sets, and their bookkeeping — and is the only service that writes it. It re-enters speculation as CI results arrive and as batches land or fail, and it asks Runway to check merge conflicts and to perform the land. ### Shared: the messaging queue -The two services communicate only through the messaging queue. It is pluggable infrastructure kept in its own database, separate from either service's application data: the gateway publishes incoming requests for the orchestrator to consume, and the orchestrator publishes log events for the gateway to consume. +The gateway and orchestrator communicate through the messaging queue. It is pluggable infrastructure kept in its own database, separate from either service's application data: the gateway publishes land and cancel requests for the orchestrator to consume, and the orchestrator publishes log events for the gateway to consume. Merge requests and their results use the same queue machinery on Runway's topics. ## Request-log ownership invariant -The request log has exactly one owner: the **gateway**. The orchestrator only emits log events onto the queue; it never persists them. The gateway is the sole consumer of those events and the only writer of the request log. +The request log has exactly one owner: the **gateway**. The orchestrator only emits log events onto the queue, via `submitqueue/orchestrator/core/request.PublishLog`; it never persists them. The gateway is the sole consumer of those events and the only writer of the request log. -This keeps all request-log writes in one service: the orchestrator stays a pure pipeline that emits events, and the gateway owns the request log end to end. +This keeps all request-log writes in one service: the orchestrator stays a pipeline that emits events, and the gateway owns the request log end to end. diff --git a/platform/README.md b/platform/README.md index 1257d5f93..9fdcdd0ca 100644 --- a/platform/README.md +++ b/platform/README.md @@ -8,6 +8,11 @@ Cross-domain packages shared by SubmitQueue, Stovepipe, and other services in th - **metrics/** — Tally helpers with error-aware tagging via `platform/errs`. - **consumer/** — Queue consumer framework (`consumer.Controller`, registry, DLQ wiring). - **http/** — Small HTTP client helpers (e.g. base-URL `RoundTripper`). Go import path: `github.com/uber/submitqueue/platform/http`; package name is `http`. Callers that also import `net/http` should import this package with an alias (for example `phttp "github.com/uber/submitqueue/platform/http"`) and use `phttp.NewClient`. +- **git/** — Git command environment (`exec`) and local bare-repo plumbing (`repo`) shared by change providers and mergers. +- **hook/** — Consumer-side hook dispatch: turn queued hook events into `hook.Hook` calls, and reconcile events that dead-letter. +- **lifecycle/** — `Component` start/stop interface and ordered `Group` composition for runnable subsystems. +- **pipeline/** — Typed engine that assembles a service's queue topology into one lifecycle component. +- **publish/** — Topic-key publish plumbing and the message-ID convention that controls deduplication. - **buildkite/**, **githubactions/** — Vendor CI clients over `platform/http`: the REST calls and provider-specific vocabulary (state strings, id encoding) shared by every domain's `BuildRunner` backend. They deliberately define no interface, so they are plumbing rather than extensions — each domain keeps its own `BuildRunner` contract under `{domain}/extension/buildrunner` and adapts these clients to it. - **base/** — Shared domain entities (`change`, `messagequeue`, and related subpackages). Root package `base` is documentation-only. - **extension/** — Shared extension interfaces and implementations reused across domains (`counter`, `messagequeue`, and backends such as `mysql`). A package belongs here only if it defines a behavioral interface with its `Config` and `Factory` interface; a vendor client with no interface belongs directly under `platform/`. diff --git a/platform/extension/messagequeue/mysql/README.md b/platform/extension/messagequeue/mysql/README.md index eafbcc53e..6a9d52d67 100644 --- a/platform/extension/messagequeue/mysql/README.md +++ b/platform/extension/messagequeue/mysql/README.md @@ -2,7 +2,7 @@ MySQL-based distributed queue with partition leasing, delivery state tracking, and at-least-once delivery. -For design rationale, guarantees, and trade-offs, see the [RFC](../../../doc/rfc/sql-queue-rfc.md). +For design rationale, guarantees, and trade-offs, see the [RFC](../../../../doc/rfc/sql-queue-rfc.md). ## Quick Start @@ -115,7 +115,7 @@ platform/extension/messagequeue/mysql/ `queue_delivery_state` has a `postponed BOOLEAN NOT NULL DEFAULT FALSE` column that supports `Delivery.Postpone`. `MarkPostponed` sets `invisible_until = now + delay`, resets `retry_count` to 0, and sets the flag. While the flag is set and the row is invisible, the poll loop treats the message as a **barrier** — it stops scanning the partition instead of skipping past it (nacked rows keep skip-and-continue semantics, so a failed message never halts its partition). On the next `MarkDelivered` the flag is consumed: the `retry_count` increment is skipped and the flag cleared, so a postponed redelivery restarts as attempt 1 and only consecutive real failures count toward `Retry.MaxAttempts`. Default FALSE keeps existing rows back-compatible. -See `schema/` for full SQL definitions. See the [RFC](../../../doc/rfc/sql-queue-rfc.md#database-schema) for field-level documentation. +See `schema/` for full SQL definitions. See the [RFC](../../../../doc/rfc/sql-queue-rfc.md#database-schema) for field-level documentation. ### Store Architecture diff --git a/runway/README.md b/runway/README.md index 68e1baa96..a45d66bad 100644 --- a/runway/README.md +++ b/runway/README.md @@ -1,12 +1,8 @@ # Runway -Runway owns the merge queues defined by the external contract in -[`api/runway/messagequeue`](../api/runway/messagequeue): it consumes merge-conflict-check and merge -requests, performs the work, and (eventually) publishes the result to the corresponding signal queue. -SubmitQueue is a client of these queues. +Runway owns the merge queues defined by the external contract in [`api/runway/messagequeue`](../api/runway/messagequeue): it consumes merge-conflict-check and merge requests, performs the work, and publishes the result to the corresponding signal queue. SubmitQueue is a client of these queues. -Runway is a single service (the domain *is* the service); its controllers live directly under -[`controller/`](controller). It consumes Runway's merge queues: +Runway is a single service (the domain *is* the service); its controllers live directly under [`controller/`](controller), and its merger extension lives under [`extension/`](extension). It consumes Runway's merge queues: - `merge-conflict-check` — dry-run check that an ordered sequence of merge steps applies cleanly, without committing. - `merge` — committing merge: apply and commit the ordered steps. diff --git a/service/README.md b/service/README.md index 0177846ba..e55cfc551 100644 --- a/service/README.md +++ b/service/README.md @@ -17,7 +17,7 @@ Each domain has its own subdirectory with a dedicated README: | **Stovepipe** | 8083 | `stovepipe` | `Ping`, `Ingest` (+ consumes process, build, buildsignal, record, stovepipe-hook, and paired DLQ topics) | MySQL storage + queue | | **Runway** | 8086 | `runway` | `Ping` (+ consumes merge-conflict-check & runway-merge topics) | MySQL queue | -Ports above are the `go run` defaults; under Docker Compose each server listens on `:8080` inside its container and is published on a random ephemeral host port (use `make local-*-ps` / `docker port` to discover it). +Ports above are the `go run` defaults; under Docker Compose each server listens on `:8080` inside its container and is published on a random ephemeral host port. `make local-submitqueue-ps` is the only status target, and it prints the SubmitQueue mappings. `make local-stovepipe-start` and `make local-runway-start` print their ports when those stacks come up. ## Directory Structure @@ -63,11 +63,14 @@ make local-runway-start make local-submitqueue-logs make local-submitqueue-ps -# Stop everything (SubmitQueue + Stovepipe + Runway) +# Stop one stack, or every stack +make local-submitqueue-stop +make local-stovepipe-stop +make local-runway-stop make local-stop ``` -`make local-stop` stops the SubmitQueue, Stovepipe, and Runway stacks; the per-domain `make local-stovepipe-stop` / `make local-runway-stop` targets stop just one. Each `build-*-linux` target copies a distinct Linux binary into `.docker-bin/` so the compose stacks don't clobber each other's artifacts. +`make local-submitqueue-stop`, `make local-stovepipe-stop`, and `make local-runway-stop` each stop one stack. `make local-stop` stops all three. MySQL data sits on anonymous volumes, so a stop leaves those databases empty on the next start. `make local-submitqueue-stop` also names the provider overlay and leaves a `PROVIDER=git` sandbox in place; `make local-submitqueue-clean` removes that sandbox along with leftover volumes and images. Each `build-*-linux` target copies a distinct Linux binary into `.docker-bin/` so the compose stacks don't clobber each other's artifacts. ### Bazel diff --git a/service/runway/README.md b/service/runway/README.md index d0f848d9d..dc2a46bd2 100644 --- a/service/runway/README.md +++ b/service/runway/README.md @@ -117,4 +117,3 @@ grpcurl -plaintext -d '{"message": "hello"}' localhost:8086 uber.runway.Runway/P ## Shutdown The server handles `SIGINT` / `SIGTERM` gracefully: it drains in-flight RPCs, then stops the queue consumers — primary and DLQ, 30s timeout each. It exits `0` on clean shutdown, `143` (128 + SIGTERM) when stopped by signal, and `1` on startup/runtime errors (details on stderr). Shutdown errors override the signal exit code. - diff --git a/service/submitqueue/README.md b/service/submitqueue/README.md index 9c94dee19..365a77947 100644 --- a/service/submitqueue/README.md +++ b/service/submitqueue/README.md @@ -49,12 +49,15 @@ Both services handle `SIGINT` (Ctrl+C) and `SIGTERM` gracefully: 2. The Gateway stops its request-log consumer, and the Orchestrator stops its complete queue pipeline (30-second limit for each). 3. The process exits with a code reflecting the outcome (see below). -To stop Docker Compose services: +To stop the SubmitQueue Compose stack: ```bash -make local-stop +make local-submitqueue-stop # this stack only +make local-stop # SubmitQueue, Stovepipe, and Runway ``` +`make local-submitqueue-stop` is the SubmitQueue stop. The databases do not survive it, and a `PROVIDER=git` sandbox is left in place until `make local-submitqueue-clean`. `make local-stop` is reserved for stopping every local stack. + ## Exit Codes | Code | Meaning | diff --git a/service/submitqueue/docker-compose.yml b/service/submitqueue/docker-compose.yml index ba301598d..3055854cd 100644 --- a/service/submitqueue/docker-compose.yml +++ b/service/submitqueue/docker-compose.yml @@ -1,7 +1,7 @@ # Docker Compose for Full Stack (Gateway + Orchestrator + MySQL) # # IMPORTANT: Before running docker-compose, build the Linux binaries: -# make build-gateway-linux build-orchestrator-linux +# make build-submitqueue-gateway-linux build-submitqueue-orchestrator-linux # OR # bazel build --platforms=@rules_go//go/toolchain:linux_amd64 //service/submitqueue/gateway/server //service/submitqueue/orchestrator/server # diff --git a/service/submitqueue/gateway/server/Dockerfile b/service/submitqueue/gateway/server/Dockerfile index b9486b4c4..d000aabee 100644 --- a/service/submitqueue/gateway/server/Dockerfile +++ b/service/submitqueue/gateway/server/Dockerfile @@ -5,7 +5,7 @@ RUN apt-get update && apt-get install -y ca-certificates && rm -rf /var/lib/apt/ WORKDIR /app # Copy pre-built Linux binary -# Built via: make build-gateway-linux +# Built via: make build-submitqueue-gateway-linux COPY --chmod=0555 .docker-bin/gateway ./gateway # Sample queue configuration; the gateway reads it on startup via diff --git a/service/submitqueue/gateway/server/docker-compose.yml b/service/submitqueue/gateway/server/docker-compose.yml index 07696753b..7cda0e689 100644 --- a/service/submitqueue/gateway/server/docker-compose.yml +++ b/service/submitqueue/gateway/server/docker-compose.yml @@ -1,7 +1,7 @@ # Docker Compose for Gateway Manual Testing # # IMPORTANT: Before running docker-compose, build the Linux binary: -# make build-gateway-linux +# make build-submitqueue-gateway-linux # OR # bazel build --platforms=@rules_go//go/toolchain:linux_amd64 //service/submitqueue/gateway/server # diff --git a/service/submitqueue/orchestrator/server/Dockerfile b/service/submitqueue/orchestrator/server/Dockerfile index fdd59363b..dbb9d596e 100644 --- a/service/submitqueue/orchestrator/server/Dockerfile +++ b/service/submitqueue/orchestrator/server/Dockerfile @@ -16,7 +16,7 @@ RUN apt-get update && apt-get install -y ca-certificates git && rm -rf /var/lib/ WORKDIR /app # Copy pre-built Linux binary -# Built via: make build-orchestrator-linux +# Built via: make build-submitqueue-orchestrator-linux COPY --chmod=0555 .docker-bin/orchestrator ./orchestrator EXPOSE 8080 diff --git a/service/submitqueue/orchestrator/server/docker-compose.yml b/service/submitqueue/orchestrator/server/docker-compose.yml index ddc3494aa..78384993a 100644 --- a/service/submitqueue/orchestrator/server/docker-compose.yml +++ b/service/submitqueue/orchestrator/server/docker-compose.yml @@ -1,7 +1,7 @@ # Docker Compose for Orchestrator Manual Testing # # IMPORTANT: Before running docker-compose, build the Linux binary: -# make build-orchestrator-linux +# make build-submitqueue-orchestrator-linux # OR # bazel build --platforms=@rules_go//go/toolchain:linux_amd64 //service/submitqueue/orchestrator/server # diff --git a/stovepipe/extension/queueconfig/mock/queueconfig_mock.go b/stovepipe/extension/queueconfig/mock/queueconfig_mock.go index c0d851c3b..98114bc91 100644 --- a/stovepipe/extension/queueconfig/mock/queueconfig_mock.go +++ b/stovepipe/extension/queueconfig/mock/queueconfig_mock.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: stovepipe/extension/queueconfig/queueconfig.go +// Source: queueconfig.go // // Generated by this command: // -// mockgen -source=stovepipe/extension/queueconfig/queueconfig.go -destination=stovepipe/extension/queueconfig/mock/queueconfig_mock.go -package=mock +// mockgen -source=queueconfig.go -destination=mock/queueconfig_mock.go -package=mock // // Package mock is a generated GoMock package. diff --git a/submitqueue/README.md b/submitqueue/README.md index d6cb7ca2f..b4c1f9552 100644 --- a/submitqueue/README.md +++ b/submitqueue/README.md @@ -2,10 +2,11 @@ SubmitQueue service layout: -- `gateway/` — Gateway service: entry point for `Ping`, `Land`, `Cancel`, request-summary, request-history, and queue-listing RPCs. It also consumes request-log events and maintains the public request projections. -- `orchestrator/` — Orchestrator service: coordinates validation, dependency analysis, speculation, builds, landing, cancellation, conclusion, hooks, and DLQ reconciliation. -- `extension/` — SubmitQueue-specific extension contracts and implementations, including storage, queue configuration, change providers, validation, conflict analysis, speculation, and build runners. +- `gateway/` — Gateway service: entry point for `Ping`, `Land`, `Cancel`, request-summary, request-history, and queue-listing RPCs. It also consumes request-log events and maintains the public request projections. `gateway/core/request` materializes those logs; `gateway/extension/storage` is the aggregate, MySQL implementation, and schema only the gateway resolves. +- `orchestrator/` — Orchestrator service: coordinates validation, dependency analysis, speculation, builds, landing, cancellation, conclusion, hooks, and DLQ reconciliation. `orchestrator/core/request` publishes and terminates requests, `orchestrator/core/batch` moves batches, and `orchestrator/extension/storage` is the aggregate, MySQL implementation, and schema only the orchestrator resolves. +- `extension/` — SubmitQueue-specific extension contracts and implementations, including the shared storage contracts, queue configuration, change providers, validation, conflict analysis, speculation, and build runners. Service aggregates and MySQL backends live under the service that resolves them. - `entity/` — SubmitQueue-specific domain entities. -- `core/` — Infrastructure shared across SubmitQueue's own services, including request and batch lifecycle helpers, change-set resolution, and internal topic keys. +- `client/` — Gateway client used by command-line tools and the demo. +- `core/` — Infrastructure shared by the gateway and the orchestrator: change-set resolution (`changeset`), internal queue contracts (`messagequeue`), and topic keys (`topickey`). Cross-domain building blocks live outside this directory: shared entities in `platform/base/`, shared extensions in `platform/extension/`, and cross-domain infrastructure such as the consumer framework in `platform/`. diff --git a/submitqueue/core/changeset/mock/changeset_mock.go b/submitqueue/core/changeset/mock/changeset_mock.go index 7e5d318f1..6796c4fcb 100644 --- a/submitqueue/core/changeset/mock/changeset_mock.go +++ b/submitqueue/core/changeset/mock/changeset_mock.go @@ -13,7 +13,7 @@ import ( context "context" reflect "reflect" - "github.com/uber/submitqueue/platform/base/change" + change "github.com/uber/submitqueue/platform/base/change" entity "github.com/uber/submitqueue/submitqueue/entity" gomock "go.uber.org/mock/gomock" ) diff --git a/submitqueue/core/core.go b/submitqueue/core/core.go index abb2e052b..a8ed79154 100644 --- a/submitqueue/core/core.go +++ b/submitqueue/core/core.go @@ -12,10 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package core groups infrastructure shared across SubmitQueue's own services -// (gateway and orchestrator) — the SubmitQueue-scoped analogue of the repo-level -// core/. Cross-domain infrastructure lives in the top-level core/; this package -// is for plumbing private to SubmitQueue. Subpackages: core/consumer (queue -// consumption framework) and core/request (request lifecycle shared by gateway -// and orchestrator). +// Package core groups infrastructure shared by SubmitQueue's gateway and +// orchestrator. Cross-domain infrastructure lives under platform/. The +// subpackages here are changeset (change-set resolution), messagequeue +// (internal pipeline contracts), and topickey (topic-key constants). Request +// materialization lives in gateway/core/request; publishing and termination +// live in orchestrator/core/request; batch helpers live in +// orchestrator/core/batch. package core diff --git a/submitqueue/extension/storage/README.md b/submitqueue/extension/storage/README.md index df1095f0e..026429d84 100644 --- a/submitqueue/extension/storage/README.md +++ b/submitqueue/extension/storage/README.md @@ -1,6 +1,13 @@ # Storage -Pluggable persistence interfaces for SubmitQueue entities (requests, batches, dependents, logs, etc.). Implementations live under `extension/storage//`. +Shared persistence contracts for SubmitQueue entities (requests, batches, dependents, logs, and the gateway read models): the store interfaces, the error vocabulary, and the queue config both services are written against. Which stores a service may reach is decided by its own aggregate. + +Canonical aggregates, MySQL implementations, and schemas live with the service that resolves them: + +- `submitqueue/gateway/extension/storage` — the request log and the public projections. +- `submitqueue/orchestrator/extension/storage` — pipeline working state. + +Colocated deployments (the end-to-end tests and the local compose stack) apply both schemas through the `//submitqueue/extension/storage:schema` filegroup, which unions the two service schema packages. A deployment that runs one service depends on that service's schema instead. ## Queue-scoped resolution @@ -54,7 +61,7 @@ Store interfaces are designed for the storage technology *space*, not for SQL (s **Domain state is often already the index.** Before adding any lookup, check whether an entity the caller already loads enumerates the children — an aggregate that references its parts by ID (e.g. a tree whose paths record their build identities) is the batch→children index, persisted and versioned as domain state. Duplicating that relationship as a database index adds a second source of truth for something the domain already owns. -**When neither applies, the reverse lookup is real — give it its own mapping store.** In the KV space there is no third mechanism: the only way to look up by an attribute is to make that attribute a primary key somewhere. So promote the relationship to a first-class mapping entity — keyed by the lookup attribute, written by the same flow that creates the source entity with idempotent puts, and rebuildable as a projection if it drifts. `ChangeRecord` is the in-repo example: it exists so "which requests claimed this change URI" is a by-key read on (queue, URI). `QueueBatchState` is the same pattern for a mutable attribute: "which batches of this queue are in this state" is a by-key read on (queue, state), maintained as advisory records that move buckets alongside the batch's own state CAS (the shared primitives in `submitqueue/core/batch` own that protocol) — it replaced `BatchStore.GetByQueueAndStates`, which was the contract's one query-by-attribute. Unlike a `KEY idx_*`, the relationship is visible in the contract and portable to any backend. +**When neither applies, the reverse lookup is real — give it its own mapping store.** In the KV space there is no third mechanism: the only way to look up by an attribute is to make that attribute a primary key somewhere. So promote the relationship to a first-class mapping entity — keyed by the lookup attribute, written by the same flow that creates the source entity with idempotent puts, and rebuildable as a projection if it drifts. `ChangeRecord` is the in-repo example: it exists so "which requests claimed this change URI" is a by-key read on (queue, URI). `QueueBatchState` is the same pattern for a mutable attribute: "which batches of this queue are in this state" is a by-key read on (queue, state), maintained as advisory records that move buckets alongside the batch's own state CAS (the shared primitives in `submitqueue/orchestrator/core/batch` own that protocol) — it replaced `BatchStore.GetByQueueAndStates`, which was the contract's one query-by-attribute. Unlike a `KEY idx_*`, the relationship is visible in the contract and portable to any backend. ### Decision path diff --git a/submitqueue/gateway/README.md b/submitqueue/gateway/README.md index 4e9ec8465..cd0695524 100644 --- a/submitqueue/gateway/README.md +++ b/submitqueue/gateway/README.md @@ -19,7 +19,7 @@ The materializer appends the log, chooses the winning current status, and activa The gateway owns the request log read model and is the only service that reads it. - `Land` publishes first and then attempts to materialize `accepted`; publication is its success boundary. `Cancel` materializes `cancelling` before publishing so the user's intent is visible when the RPC returns. -- For statuses produced downstream, the orchestrator publishes entries to the `log` topic through `submitqueue/core/request.PublishLog`. The gateway consumes that topic and persists each entry through the same materializer. +- For statuses produced downstream, the orchestrator publishes entries to the `log` topic through `submitqueue/orchestrator/core/request.PublishLog`. The gateway consumes that topic and persists each entry through the same materializer (`submitqueue/gateway/core/request`). - Orchestrator DLQ reconciliation transitions durable request state and publishes terminal log entries to the same `log` topic; the gateway remains the materializer. - `GetRequestHistoryByID` and `GetRequestHistoryByChangeURI` read retained request-log rows directly. diff --git a/submitqueue/orchestrator/extension/storage/mock/storage_mock.go b/submitqueue/orchestrator/extension/storage/mock/storage_mock.go index 40a2fe729..9169780e7 100644 --- a/submitqueue/orchestrator/extension/storage/mock/storage_mock.go +++ b/submitqueue/orchestrator/extension/storage/mock/storage_mock.go @@ -13,7 +13,7 @@ import ( reflect "reflect" storage "github.com/uber/submitqueue/submitqueue/extension/storage" - orchstorage "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" + storage0 "github.com/uber/submitqueue/submitqueue/orchestrator/extension/storage" gomock "go.uber.org/mock/gomock" ) @@ -42,10 +42,10 @@ func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { } // For mocks base method. -func (m *MockFactory) For(config orchstorage.Config) (orchstorage.Storage, error) { +func (m *MockFactory) For(config storage0.Config) (storage0.Storage, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "For", config) - ret0, _ := ret[0].(orchstorage.Storage) + ret0, _ := ret[0].(storage0.Storage) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/submitqueue/orchestrator/extension/storage/mysql/schema/README.md b/submitqueue/orchestrator/extension/storage/mysql/schema/README.md index 520ab2366..14c09d619 100644 --- a/submitqueue/orchestrator/extension/storage/mysql/schema/README.md +++ b/submitqueue/orchestrator/extension/storage/mysql/schema/README.md @@ -16,7 +16,7 @@ The `batch` table is keyed by `(queue, id)` and carries no secondary index. List ### Composite primary key: `(queue, state, batch_id)` -`queue_batch_state` holds the queue's advisory per-state membership records (see `entity.QueueBatchState`): one row per batch per state bucket, no payload and no version column. The key leads with `queue` so a state-bucket listing is a primary-key-prefix scan and the table is shardable by queue. Rows are moved between buckets by the shared transition protocol in `submitqueue/core/batch`; writes are idempotent (`INSERT IGNORE`, keyed `DELETE`). The `batch` row remains authoritative — readers hydrate each candidate and classify by the batch's own state. +`queue_batch_state` holds the queue's advisory per-state membership records (see `entity.QueueBatchState`): one row per batch per state bucket, no payload and no version column. The key leads with `queue` so a state-bucket listing is a primary-key-prefix scan and the table is shardable by queue. Rows are moved between buckets by the shared transition protocol in `submitqueue/orchestrator/core/batch`; writes are idempotent (`INSERT IGNORE`, keyed `DELETE`). The `batch` row remains authoritative — readers hydrate each candidate and classify by the batch's own state. #### Future: Prune job diff --git a/tool/README.md b/tool/README.md index 2b0910cc3..2a8f53256 100644 --- a/tool/README.md +++ b/tool/README.md @@ -23,6 +23,23 @@ bazel build //... The Bazel version is controlled by `.bazelversion` at the repository root. Update that file to change the Bazel version used by the wrapper. +## Git sandbox + +`tool/gitsandbox` creates the bare repository that `make local-submitqueue-start PROVIDER=git` merges into. It runs before the stack starts, because Runway clones that repository at boot and fails if the target does not already exist. The result is one seed commit on the target branch. Running it again leaves an existing repository unchanged, so a restart keeps commits that earlier runs landed. The Makefile invokes it; `bazel run //tool/gitsandbox -- -sandbox-dir ` is the direct form. + +## Proto generation + +`tool/proto` is the Bazel codegen for the committed protobuf Go stubs. `make proto` builds `//tool/proto:generated` and copies each package's output into its `protopb/` directory. The package list and per-source outputs live in `tool/proto/BUILD.bazel`. Change the `.proto` sources and regenerate; do not edit the generated files. `make clean-proto` removes those stubs, and `make proto` writes them again. + +## Linters + +`tool/linter` holds the checkers behind `make lint`. Each one is a small program with its own Makefile target: + +- `licenseheader` checks Apache license headers (`make lint-license`); `make license-fix` writes missing ones. +- `binaryfile` fails when a binary file is tracked (`make lint-binary`). +- `messageid` fails when a queue message is constructed outside `platform/publish` and the queue backends (`make lint-message-id`). +- `queueshard` fails when a schema's primary key does not lead with its shard column, or a secondary index reaches across shards (`make lint-queue-shard`). + ## Adding New Tools When adding new tools to this directory: From e88fb9b7e9f62c4acf3eebc6fdf8d085405caede Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Mon, 28 Sep 2026 10:20:15 -0700 Subject: [PATCH 2/3] docs: correct reviewed pipeline contract descriptions ## Summary ### Why? The documentation audit still left three claims that did not match the interfaces and cancellation behavior in code. ### What? Documents Stovepipe's actual build-runner return values, distinguishes the extension contract's historical problem from its implemented signatures, and records that cancellation leaves Landing and terminal batches for conclude. ## Test Plan - `git diff --check` - Local link validation for the three updated RFCs Co-authored-by: Cursor --- doc/rfc/stovepipe/workflow.md | 2 +- doc/rfc/submitqueue/extension-contract.md | 16 ++++++++-------- doc/rfc/submitqueue/workflow.md | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 036f9da07..549562b89 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -56,7 +56,7 @@ The ref is a *cache* of the last-green URI, not a second record of greenness. It | Extension | Responsibility | |---|---| | **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range; advance the Queue's **promotion ref** to a commit. The sole owner of URI semantics, including which refs a Queue name resolves to. | -| **build-runner** | Build a scope at a URI (optionally relative to a baseline URI), returning pass/fail and the target graph. See [build-runner.md](../submitqueue/build-runner.md). | +| **build-runner** | Build a scope at a URI, optionally relative to a baseline URI. `Trigger` returns a build id; `Status` returns pass/fail and the caller-supplied metadata it echoed. It does not return a target graph. See [build-runner.md](../submitqueue/build-runner.md). | | **Hooks** | Deliver Stovepipe's validation events to downstream systems. What is published today is repository-scoped: validation of this URI has begun, and this URI is green or not green. Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. The shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | | **Storage** | Persist Queues (incl. last-green URI), Requests, build records, and per-URI / per-project greenness. Key/value-shaped per the extension-design rules in [AGENTS.md](../../../AGENTS.md). | diff --git a/doc/rfc/submitqueue/extension-contract.md b/doc/rfc/submitqueue/extension-contract.md index 71022558b..781342614 100644 --- a/doc/rfc/submitqueue/extension-contract.md +++ b/doc/rfc/submitqueue/extension-contract.md @@ -4,16 +4,16 @@ Design notes for what SubmitQueue's pluggable extensions accept: orchestrator ** ## Status -Implemented for the extensions named here. `changeprovider.Get` takes `entity.Request`. `conflict.Analyzer.Analyze` takes the batch under analysis and the in-flight batches. `buildrunner.Trigger` takes base `[]entity.Batch` and a head `entity.Batch`. `scorer.Score` takes `entity.Batch` and `entity.SpeculationPathSet`. There is no separate score stage; speculation asks the scorer. `changeset.Resolver` is the shared batch-to-changes reader and still declares its own `Stores` slice rather than importing an orchestrator aggregate. Runway still performs the asynchronous conflict check and the land. In the verdict table, the proposed inputs are these signatures, except the scorer, which also receives the path set. +Implemented for the extensions named here. `changeprovider.Get` takes `entity.Request`. `conflict.Analyzer.Analyze` takes the batch under analysis and the in-flight batches. `buildrunner.Trigger` takes base `[]entity.Batch` and a head `entity.Batch`. `scorer.Score` takes `entity.Batch` and `entity.SpeculationPathSet`. There is no separate score stage; speculation asks the scorer. `changeset.Resolver` is the shared batch-to-changes reader and still declares its own `Stores` slice rather than importing an orchestrator aggregate. Runway still performs the asynchronous conflict check and the land. The verdict table's "Input now" column is these signatures. ## Problem -Extension input granularity is inconsistent across the pipeline stages (see [workflow.md](workflow.md)). `conflict.Analyzer` takes identity (`entity.Batch`); `scorer`, `changeprovider`, and `buildrunner` take controller-resolved `entity.Change`. The split caps what an extension can do: +Before this contract, extension input granularity was inconsistent across the pipeline stages (see [workflow.md](workflow.md)). `conflict.Analyzer` took identity (`entity.Batch`); `scorer`, `changeprovider`, and `buildrunner` took a controller-resolved `entity.Change`. That split capped what an extension could do: -- `ConflictType` already names `target_overlap`, but a real target-overlap analyzer **cannot be written** — the dependency-analysis stage hands it identity-level batches (no changed targets) and the contract has nowhere to put them. -- `scorer` gets a URIs-only `Change`, so a heuristic scorer **cannot see** lines-changed / file-count. +- `ConflictType` already named `target_overlap`, but a real target-overlap analyzer could not be written: the dependency-analysis stage handed it identity-level batches, with no changed targets, and the contract had nowhere to put them. +- `scorer` got a URIs-only `Change`, so a heuristic scorer could not see lines-changed or file-count. -Both unblock with the shape `conflict` already uses: accept identity, resolve internally. +Both unblock by taking the shape `conflict` already used: accept identity and resolve internally. The signatures in Status are that resolution. ## Principle @@ -34,15 +34,15 @@ This grounds `conflict` as the baseline: it already resolves nothing because the ## Verdict -| Extension | Stage | Input today | Proposed input | Output | Injected deps | +| Extension | Stage | Input before | Input now | Output | Injected deps | |---|---|---|---|---|---| | `conflict.Analyzer` | batch | identity (`Batch`, `[]Batch`) | unchanged — **the baseline** | conflicting in-flight batches (`[]Conflict`, `BatchID`-tagged) — unchanged | request store + change provider | -| `scorer.Scorer` | score | flat `Change`, per request | `entity.Batch` — resolve + reduce internally | one batch score (`float64`) — unchanged | request store + change provider | +| `scorer.Scorer` | speculation | flat `Change`, per request | `entity.Batch` and `entity.SpeculationPathSet` — resolve + reduce internally | one batch score (`float64`) — unchanged | request store + change provider | | `changeprovider.ChangeProvider` | validate | `Change` | `entity.Request` | per-URI change info (`[]ChangeInfo`, `URI`-tagged) — unchanged | none — it *is* the resolver | | `buildrunner.BuildRunner` | build | base/head `[]Change` | base `[]entity.Batch` + head `entity.Batch` | build id, then status/cancel (`BuildID`, `BuildStatus`) — unchanged | request store + change provider | | `storage`, `changestore`, `queueconfig` | — | keys + entities | unchanged — resolution targets | entities | — | -**Outputs are unchanged.** This RFC moves the *input* toward identity; the four live return contracts — conflicts, score, change info, build id/status — are exactly what they are today. No output shape changes. +**Outputs are unchanged.** This RFC moved the input to identity. The four return contracts — conflicts, score, change info, and build id/status — kept their shapes. The validate-time landability **check** and the **land** itself both run **asynchronously and out-of-process** in Runway rather than as in-process extensions. SubmitQueue adapts its land request to Runway's shared `MergeRequest`/`MergeResult` contract, where a conflict check is a dry run of a merge. `validate` hands off directly to Runway (→ `merge-conflict-check`, result back via `landconflictsignal`); `land` hands the batch to Runway (→ `runway-merge`, result back via `landsignal`). See [workflow.md](workflow.md). SubmitQueue retains no parallel in-process checking or pushing contract. diff --git a/doc/rfc/submitqueue/workflow.md b/doc/rfc/submitqueue/workflow.md index bcbcd1da9..dab069559 100644 --- a/doc/rfc/submitqueue/workflow.md +++ b/doc/rfc/submitqueue/workflow.md @@ -140,7 +140,7 @@ class node_submitter toneIndigo | **gateway/Land** | RPC | start | Mint the request id, persist an accepting receipt, publish the full land request, then persist Accepted | | **gateway/Cancel** | RPC | cancel | Publish a cancel request for an existing land | | **start** | LandRequest | validate, log | Persist the Request and publish it to validate | -| **cancel** | CancelRequest | log, or speculate | Record Cancelling; finish a request that is not in a batch, or hand each cancellable batch attempt to speculate | +| **cancel** | CancelRequest | log, or speculate | Record Cancelling. Finish a request that has no applicable batch. Hand each cancellable batch attempt to speculate. Leave a Landing or already-terminal batch for conclude | | **validate** | RequestID | merge-conflict-check (Runway), log | Dedup, fetch change metadata, claim changes, then publish the full `MergeRequest` to Runway keyed by the request id | | **landconflictsignal** | MergeResult | batch, or log | Correlate Runway's check; advance a landable request to batch, or fail a conflicted request | | **batch** | RequestID | dependency-analysis, log | Mint a Creating batch for the request and hand that batch id onward | From 71feef5af70d739a173a9e92903a912e03f21eca Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Mon, 28 Sep 2026 10:53:21 -0700 Subject: [PATCH 3/3] docs: correct remaining pipeline and storage claims ## Summary ### Why? Autoreview found RFC sentences that still described an older pipeline: a gateway aggregate satisfying changeset.Stores, an unwired build_dlq, buildsignal republishing to itself, a missing cancelled hook, an unimplemented BuildRunner, and orchestrator topic keys named merge and land. ### What? Corrects those claims against the live controllers. Only the orchestrator aggregate satisfies changeset.Stores. build_dlq is consumed by NewDLQBuildController. Stovepipe buildsignal holds its delivery. record publishes validation.repository.cancelled. BuildRunner is the live interface. Topic keys are runway-merge and submitqueue-land, conflict checking is mergeconflictcheck.go, cancel hands only cancellable batches to speculate, and the gateway persists cancelling before publishing. ## Test Plan - `git diff --check` on the five updated RFCs Co-authored-by: Cursor --- doc/rfc/service-scoped-extensions.md | 2 +- doc/rfc/stovepipe/steps/build.md | 26 ++++++++++++-------------- doc/rfc/stovepipe/steps/buildsignal.md | 2 +- doc/rfc/stovepipe/workflow.md | 14 +++++++------- doc/rfc/submitqueue/workflow.md | 14 +++++++------- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/doc/rfc/service-scoped-extensions.md b/doc/rfc/service-scoped-extensions.md index 6aa4bf122..14274be85 100644 --- a/doc/rfc/service-scoped-extensions.md +++ b/doc/rfc/service-scoped-extensions.md @@ -100,7 +100,7 @@ type Stores interface { type Resolve func(queue string) (Stores, error) ``` -Each service's aggregate satisfies `Stores` structurally, so `changeset` names no service package. Once those extensions relocate under the orchestrator, `changeset`'s consumers are all orchestrator-side and it should relocate with them — at which point it can drop `Stores`/`Resolve` and take `orchstorage.Factory` directly, matching the other relocated packages rather than staying the one exception. +The orchestrator aggregate exposes both accessors, so it satisfies `Stores` structurally and `changeset` names no service package. The gateway aggregate does not: it holds the request log and the public projections, and it has neither `GetRequestStore` nor `GetChangeStore`. Once those extensions relocate under the orchestrator, `changeset`'s consumers are all orchestrator-side and it should relocate with them — at which point it can drop `Stores`/`Resolve` and take `orchstorage.Factory` directly, matching the other relocated packages rather than staying the one exception. `submitqueue/core` now holds `changeset`, `messagequeue` and `topickey`, and nothing in it imports a service package. diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 5f1db58be..886fdba3f 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -42,7 +42,7 @@ For a delivery carrying request id `R`: 5. Trigger: buildID, err := buildRunner.Trigger(ctx, baseURI, R.URI, metadata) - Trigger takes no caller-supplied id; the runner mints the build's identity, and buildID becomes Build.ID — SubmitQueue's exact convention (see - "Alternatives considered" under the contract sketch). + "Alternatives considered" under the contract). - there is deliberately no "already triggered?" pre-check: with a runner-minted id there is no key to check by, so a redelivery re-triggers and downstream idempotency absorbs the duplicate (see Idempotency). @@ -81,7 +81,7 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu - **Strategy not yet visible** — retryable; the producing stage's write is not visible on this reader yet. - **Request already terminal** (step 2) — ack, no build. A redelivery after `record` finished, or after `process` superseded the head, never starts a stale build. - **Redelivery while the Request is still in flight** (crash or failure anywhere in steps 5–8) — the redelivery re-runs from step 1, `Trigger` mints a fresh id, `Create` persists a second `Build` row, and a second poll loop starts. Harmless, in three layers: both builds target the identical `(headURI, baseURI)` scope; each `Build` polls in its own partition and `buildsignal` short-circuits the moment the Request goes terminal (its step 3); and `buildsignal`'s outcome write is first-writer-wins, so the second verdict cannot flip the Request's state or overwrite the create-only validation fact. A build triggered but never persisted (crash between steps 5 and 6) is the same story minus the row: an orphan the runner finishes and nobody ever reads. Wasted CI compute, not a correctness risk — the same accepted trade as SubmitQueue. -- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to DLQ, where the fail-closed posture is meant to drive the Request terminal (see [workflow.md](../workflow.md#fail-closed-on-unprocessable-work)). No reconciler consumes `build_dlq` yet, so that last step does not happen today — see [Fail-closed interaction](#fail-closed-interaction). +- **Trigger / publish / other store failure** — nothing durable is left half-written that a redelivery can't reconcile; the error rejects to `build_dlq`, and `dlq.NewDLQBuildController` drives a still-`processing` Request to `failed` and releases its Queue slot (see [Fail-closed interaction](#fail-closed-interaction)). ## Edge cases @@ -92,9 +92,7 @@ Every branch is safe under at-least-once redelivery — with SubmitQueue's postu A build that never reaches step 8 — `Trigger` failing repeatedly, the publish to `buildsignal` never landing, `BuildStore.Create` down — must not wedge its `Request`'s Queue slot forever: `process`'s per-Queue concurrency gate holds `in_flight_count` open until the Request reaches a terminal state (see [process.md](process.md#concurrency-lifecycle)). `build` does not implement the forcing function itself. Per [workflow.md](../workflow.md#fail-closed-on-unprocessable-work), every non-retryable failure in the algorithm rejects to DLQ (see [Error classification](#error-classification)), and a Request stuck past `MaxAttempts` is driven to a conservative terminal `failed` by the DLQ reconciler, which decrements `in_flight_count` and frees the slot. This is the same posture `buildsignal` relies on for its own poll loop (see [buildsignal.md](buildsignal.md#fail-closed-interaction)) — `build` and `buildsignal` are two links in the same fail-closed chain that keeps one bad Request from wedging its Queue. -**That chain is not closed at `build` yet.** The `build` subscription enables dead-lettering, but no controller consumes `build_dlq` — the wiring registers only `process_dlq` and `buildsignal_dlq` — so nothing forces the Request terminal and nothing frees the slot. How much that costs depends on how far the delivery got. If a `Build` row was persisted and its signal published, a poll chain survives the dead-letter and `buildsignal` still releases the slot when the build goes terminal. If the message dead-letters before that — `Trigger` failing every attempt, `BuildStore.Create` down, the publish never landing — the Request stays `processing` and its Queue loses a slot for good, which is exactly the failure [buildsignal.md](buildsignal.md#fail-closed-interaction) describes for a deployment missing its own reconciler. - -Whoever wires that reconciler has to decide what it records, not just what it releases: forcing `failed` on a Request whose build may still be running is what produces the permanently-wrong-fact path in [record.md](record.md#what-fail-closed-actually-guarantees), so this gap and that open question belong to the same piece of work. +`service/stovepipe/server` registers `dlq.NewDLQBuildController` on `build_dlq`. The reconciler loads the request named by the dead-lettered `BuildRequest`, writes a non-terminal request to `failed`, and releases the Queue slot when that request is still `processing`. A request that is already terminal is left alone, except a request already `failed`, whose missing failure log is repaired. A poll `build` already published keeps running on its own delivery: `buildsignal` still releases the slot when that build goes terminal, and this reconciler does not overwrite that outcome. One boundary is worth stating explicitly: this path fires only when `build` (or a downstream stage) *errors*. A `Trigger` call that returns successfully but the backend never actually runs — or a `Build` row created for a build the runner silently drops — has no protocol-level failure to escalate at the `build` stage; nothing here retries or dead-letters, because nothing failed. That gap surfaces one hop later, when `buildsignal` polls: either the runner reports an error (handled by `buildsignal`'s own classification) or it reports a non-terminal status forever, which is `buildsignal`'s fail-closed boundary to close, not `build`'s (see [buildsignal.md](buildsignal.md#fail-closed-interaction)). `build`'s liveness responsibility ends at a successful publish to `buildsignal`. @@ -147,11 +145,11 @@ There is no batch, no dependency list, and nothing to resolve — the URIs *are* The linearity assumption in point 1 is load-bearing and already guarded upstream: stovepipe assumes a linear trunk by default, and when `SourceControl` reports that last-green is no longer an ancestor of the head (history rewrite), `process` falls back to a full build rather than trusting the interval ([process.md](process.md#build-strategy-decision)). The URI-pair contract is exactly as expressive as that model — a valid `base..head` range, or a full build with an empty baseline — and nothing more. -So `build`'s `Trigger` gets its own shape under `stovepipe/extension/buildrunner`, still "identity in, resolve internally" — just with URI identity instead of batch identity. `Status` and `Cancel` don't have this mismatch — both domains poll and cancel by the same opaque, runner-minted id with the same async semantics — but that similarity is shaped-the-same, not shared code: they stay on `stovepipe/extension/buildrunner.BuildRunner` too, duplicated in shape from SubmitQueue's, with reuse pushed down to a shared backend implementation instead (see the contract sketch below and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract)). +So `build`'s `Trigger` gets its own shape under `stovepipe/extension/buildrunner`, still "identity in, resolve internally" — just with URI identity instead of batch identity. `Status` and `Cancel` don't have this mismatch — both domains poll and cancel by the same opaque, runner-minted id with the same async semantics — but that similarity is shaped-the-same, not shared code: they stay on `stovepipe/extension/buildrunner.BuildRunner` too, duplicated in shape from SubmitQueue's, with reuse pushed down to a shared backend implementation instead (see the contract below and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract)). -### Stovepipe `BuildRunner` contract (design sketch) +### Stovepipe `BuildRunner` contract -Not implemented here. `BuildID`, `BuildStatus`, and `BuildMetadata` are defined locally in `stovepipe/entity`, shaped the same as SubmitQueue's equivalents in `submitqueue/entity` but not the same Go types — per the reviewer preference recorded in [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract), a shared `platform/base`/`platform/extension/buildrunner` contract was considered and set aside in favor of keeping each domain's interface separate and reusing at the implementation layer instead. `stovepipe/extension/buildrunner` holds `Trigger`, `Status`, `Cancel`, `Config`, and the `Factory` interface, per [AGENTS.md](../../../../AGENTS.md)'s extension rules. +This is the live interface in [`stovepipe/extension/buildrunner`](../../../../stovepipe/extension/buildrunner). `BuildID`, `BuildStatus`, and `BuildMetadata` are defined locally in `stovepipe/entity`, shaped the same as SubmitQueue's equivalents in `submitqueue/entity` but not the same Go types — per the reviewer preference recorded in [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract), a shared `platform/base`/`platform/extension/buildrunner` contract was considered and set aside in favor of keeping each domain's interface separate and reusing at the implementation layer instead. The package holds `Trigger`, `Status`, `Cancel`, `Config`, and the `Factory` interface, per [AGENTS.md](../../../../AGENTS.md)'s extension rules. ```go // package buildrunner (stovepipe/extension/buildrunner) @@ -190,7 +188,7 @@ type Factory interface{ For(cfg Config) (BuildRunner, error) } #### Project-scoped `Trigger`: reserved, not yet designed -**TODO**, tracked pending the `analyze` design (see [workflow.md](../workflow.md#open-questions)'s "Project mapping contract" open question). The sketch above has only a whole-repo/incremental dimension (`headURI`, `baseURI`); it has no parameter for "build only this project," so as written it cannot express a Phase 2 invocation. `build` itself stays phase-agnostic (see [Input, partitioning, and the single-writer property](#input-partitioning-and-the-single-writer-property)) — it reads whatever scope is already decided and passes it through — but `Trigger` still needs a slot to read that scope from and forward to the runner. +**TODO**, tracked pending the `analyze` design (see [workflow.md](../workflow.md#open-questions)'s "Project mapping contract" open question). The interface above has only a whole-repo/incremental dimension (`headURI`, `baseURI`); it has no parameter for "build only this project," so as written it cannot express a Phase 2 invocation. `build` itself stays phase-agnostic (see [Input, partitioning, and the single-writer property](#input-partitioning-and-the-single-writer-property)) — it reads whatever scope is already decided and passes it through — but `Trigger` still needs a slot to read that scope from and forward to the runner. The shape isn't decided here because project semantics belong to `analyze`, not `build`: how a project maps to a buildable scope (a Bazel target pattern, a directory, a service name) is implementer-specific per [workflow.md](../workflow.md#project---greenness-at-a-finer-grain). The expectation is that this stays an opaque token — following the same "identity in, resolve internally" shape already used for `headURI`/`baseURI` (owned and interpreted by `SourceControl`) — that `build` reads off the `Request`/message and hands to the runner uninterpreted, rather than a structured type `build` would have to understand: @@ -198,7 +196,7 @@ The shape isn't decided here because project semantics belong to `analyze`, not Trigger(ctx context.Context, baseURI, headURI string, projectScope entity.ProjectScope, metadata entity.BuildMetadata) (entity.BuildID, error) ``` -`ProjectScope` lives in `stovepipe/entity` alongside `BuildID`/`BuildStatus`/`BuildMetadata` — projects have no SubmitQueue equivalent at all, not even a shape to mirror. Its zero value covers Phase 1 (no project — whole-repo/incremental scope only, exactly today's sketch); `analyze` is what would populate a non-zero value for Phase 2. This mirrors the additive optional field already reserved on `BuildRequest` for the same purpose (see [Queue contract additions](#queue-contract-additions)) — the wire message and the extension contract need the same new dimension, and both are deferred to the same design. +`ProjectScope` lives in `stovepipe/entity` alongside `BuildID`/`BuildStatus`/`BuildMetadata` — projects have no SubmitQueue equivalent at all, not even a shape to mirror. Its zero value covers Phase 1 (no project — whole-repo/incremental scope only, exactly today's contract); `analyze` is what would populate a non-zero value for Phase 2. This mirrors the additive optional field already reserved on `BuildRequest` for the same purpose (see [Queue contract additions](#queue-contract-additions)) — the wire message and the extension contract need the same new dimension, and both are deferred to the same design. Both `Trigger` and `Status`/`Cancel` differ *in contract* between domains, even though `Status`/`Cancel` happen to be identical in shape: both domains poll and cancel by the same opaque, runner-minted id with the same async semantics. Rather than promoting that shape parity into a shared `platform/base`/`platform/extension/buildrunner` type and interface — which would force a one-time migration of SubmitQueue's already-shipped controllers, storage, and protobuf mappings onto the shared type — each domain keeps its own `BuildRunner` interface and its own local `BuildID`/`BuildStatus`/`BuildMetadata`, and real code reuse happens one layer down, in a shared backend implementation (e.g. a Buildkite client) that both domains' concrete runners wrap. [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract) below records the shapes weighed against this one, including the shared-interface alternative that was set aside. @@ -289,13 +287,13 @@ Either could be adopted independently: the idempotency token, if a backend that ### Carries over vs. new -- **Shaped the same, not shared code**: the `BuildStatus` enum and `IsTerminal()` (nothing batch-specific — [build-runner.md](../../submitqueue/build-runner.md#buildstatus)); `BuildMetadata` (caller-supplied, provider-echoed, controller-uninterpreted — [#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)); the async contract — `Trigger` returns a handle not an outcome, `Status` may round-trip, `Cancel` reaches the provider not the engine ([#async-vs-sync-contract](../../submitqueue/build-runner.md#async-vs-sync-contract)); and the id model — no caller-supplied id, the runner mints the build's identity, and that one `entity.BuildID` is the store key, queue payload, and `Status`/`Cancel` parameter (see [Alternatives considered](#alternatives-considered-for-the-build-identity)). These are duplicated locally in `stovepipe/entity`/`stovepipe/extension/buildrunner` rather than promoted to `platform/base`/`platform/extension/buildrunner` — see the contract sketch above and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract). +- **Shaped the same, not shared code**: the `BuildStatus` enum and `IsTerminal()` (nothing batch-specific — [build-runner.md](../../submitqueue/build-runner.md#buildstatus)); `BuildMetadata` (caller-supplied, provider-echoed, controller-uninterpreted — [#buildmetadata](../../submitqueue/build-runner.md#buildmetadata)); the async contract — `Trigger` returns a handle not an outcome, `Status` may round-trip, `Cancel` reaches the provider not the engine ([#async-vs-sync-contract](../../submitqueue/build-runner.md#async-vs-sync-contract)); and the id model — no caller-supplied id, the runner mints the build's identity, and that one `entity.BuildID` is the store key, queue payload, and `Status`/`Cancel` parameter (see [Alternatives considered](#alternatives-considered-for-the-build-identity)). These are duplicated locally in `stovepipe/entity`/`stovepipe/extension/buildrunner` rather than promoted to `platform/base`/`platform/extension/buildrunner` — see the contract above and [Alternatives considered for sharing the contract](#alternatives-considered-for-sharing-the-contract). - **Shared as implementation, not contract**: a concrete backend (e.g. Buildkite) can satisfy both domains' `BuildRunner` interfaces off one client, sharing HTTP/auth/poll-loop plumbing internally even though the two `BuildRunner` interfaces it implements are separate types — see the "Shared backend under `platform`, thin per-domain contracts" option above. - **New in stovepipe**: URI-based scope in `Trigger` instead of batch lists — the one part of the contract that was always domain-specific by necessity, per [Why separate contracts](#why-separate-contracts). Mapping targets to projects is still stovepipe-only; how `analyze` obtains a target graph is left to its own design, out of scope for this doc. ## Entity and storage additions needed -**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), while `RequestID` stays stovepipe-specific: +**`Build` entity** (`stovepipe/entity/build.go`), following the immutable-except-`Status`/`Version` shape of `entity.Request`; `ID` and `Status` use the stovepipe-local `BuildID`/`BuildStatus` types (see the [contract](#stovepipe-buildrunner-contract)), while `RequestID` stays stovepipe-specific: | Field | Role | Mutable? | @@ -320,7 +318,7 @@ The row deliberately carries no scope: `R.URI`, `R.BaseURI`, and `R.BuildStrateg `IsTerminal()` on `entity.BuildStatus` covers exactly the three terminal rows. Once `buildsignal` persists one of them, that status is **write-once** — a later poll reporting a different terminal value never overwrites it (see [buildsignal.md](buildsignal.md#algorithm), step 6). -Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract sketch](#stovepipe-buildrunner-contract-design-sketch)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed. +Plus the `BuildID{ID string}` wire type in `stovepipe/entity` (same "id only travels" convention as `RequestID`, shaped like SubmitQueue's own `entity.BuildID` but not the same Go type — see the [contract](#stovepipe-buildrunner-contract)), wrapping the one runner-assigned id everywhere it appears — `Trigger`'s return, the queue payload, `Status`/`Cancel`'s parameter. `buildsignal` reaches a build by the id carried in its message, and `record` reads the `Request` (whose state carries the build's outcome) rather than a `Build`, so no reverse index from `Request` to its builds is ever needed. **`BuildStore`** (new, added to the `Storage` aggregator via `GetBuildStore()`), matching stovepipe's existing `RequestStore` conventions — **generic `Update` with caller-owned version arithmetic**: @@ -344,7 +342,7 @@ Single-key reads/writes only — no list-by-request, no query-by-attribute — p Two topic keys in `stovepipe/core/messagequeue/topics.go` — `TopicKeyBuild` (`process`/`analyze` → `build`) and `TopicKeyBuildSignal` (`build` → `buildsignal`) — and one proto message per key, since the contract test binds **exactly one message to each topic key** (see [messagequeue-contract.md](../../messagequeue-contract.md)). `ProcessRequest` is bound to `process` and cannot be reused; the new messages mirror its shape (one id field, its own `topic_keys` option): - `BuildRequest{ id }` (request id) → `topic_keys "build"`, produced by `process`/`analyze`, consumed by `build`. Phase 2's per-project trigger must also identify its project; because each topic key binds exactly one message, that lands as an **additive optional field on this same message** (protojson discards unknown fields, so the evolution is backward-compatible), not a second message type — the field's shape is deferred to the `analyze` design with the rest of the project-scoped trigger. -- `BuildSignal{ id }` (build id) → `topic_keys "buildsignal"`, produced by `build` and re-produced by `buildsignal`, consumed by `buildsignal` (see [buildsignal.md](buildsignal.md)). +- `BuildSignal{ id }` (build id) → `topic_keys "buildsignal"`, produced by `build` and consumed by `buildsignal`. A non-terminal poll postpones that same delivery with `delivery.Hold`; `buildsignal` does not publish another `BuildSignal` (see [buildsignal.md](buildsignal.md)). ### Partitioning diff --git a/doc/rfc/stovepipe/steps/buildsignal.md b/doc/rfc/stovepipe/steps/buildsignal.md index b51ddbfc1..2d5d7f5b4 100644 --- a/doc/rfc/stovepipe/steps/buildsignal.md +++ b/doc/rfc/stovepipe/steps/buildsignal.md @@ -164,4 +164,4 @@ One boundary of that posture is worth stating: the `MaxAttempts` path fires only ## Entity, storage, and queue additions -No additions beyond [build.md](build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes/re-produces the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. The message it publishes to `record`, and the `record` topic key itself, are owned by the `record` stage and land with `record.md`; `buildsignal` only needs that the **request id** reaches the record topic once the build is terminal, partitioned by request id. +No additions beyond [build.md](build.md#entity-and-storage-additions-needed): `buildsignal` calls `BuildStore.Get`/`Update` and `RequestStore.Get`/`Update` against the `Build`/`Request` shapes defined there — `Build.ID` being the runner-assigned id it hands straight back to `Status` — plus `QueueStore.Get`/`Update` to release the build slot, and consumes the `BuildSignal` message on `TopicKeyBuildSignal` introduced there. A non-terminal poll calls `delivery.Hold` on that delivery rather than publishing another one. `Request.State` gains the three build outcomes (`succeeded`, `failed`, `cancelled`), all terminal. The message it publishes to `record`, and the `record` topic key itself, are owned by the `record` stage and land with `record.md`; `buildsignal` only needs that the **request id** reaches the record topic once the build is terminal, partitioned by request id. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 549562b89..a1dead3e5 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -57,10 +57,10 @@ The ref is a *cache* of the last-green URI, not a second record of greenness. It |---|---| | **SourceControl** | Resolve a Queue name to its current head URI; answer ancestry/comparison questions between two URIs (is the new head a fast-forward descendant of the last green, or was history rewritten?); enumerate commits in a range; advance the Queue's **promotion ref** to a commit. The sole owner of URI semantics, including which refs a Queue name resolves to. | | **build-runner** | Build a scope at a URI, optionally relative to a baseline URI. `Trigger` returns a build id; `Status` returns pass/fail and the caller-supplied metadata it echoed. It does not return a target graph. See [build-runner.md](../submitqueue/build-runner.md). | -| **Hooks** | Deliver Stovepipe's validation events to downstream systems. What is published today is repository-scoped: validation of this URI has begun, and this URI is green or not green. Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. The shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | +| **Hooks** | Deliver Stovepipe's validation events to downstream systems. What is published today is repository-scoped: validation of this URI has begun, this URI is green or not green, and a cancelled validation ended without a fact. Fire-and-forget notification, decoupled so Stovepipe does not know or care who consumes the event. The shared cross-domain hook seam rather than a Stovepipe-specific extension. See [hook-framework.md](../hook-framework.md). | | **Storage** | Persist Queues (incl. last-green URI), Requests, build records, and per-URI / per-project greenness. Key/value-shaped per the extension-design rules in [AGENTS.md](../../../AGENTS.md). | -Hooks are the notification boundary. When validation of a commit begins, and when a whole-repository validation fact is recorded, the event reaches deployment systems, dashboards, and developer tooling without any of them polling Stovepipe's store, and each environment can route it to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. The mechanism is the cross-domain hook framework rather than a call out of the pipeline stages: `process` and `record` publish a repository-scoped `HookEvent` to Stovepipe's `hook` topic, and a dispatcher stage consumes it and invokes the wired hooks, so a slow or failing downstream cannot add latency to the pipeline. Both halves exist; what a deployment supplies is the hooks themselves, since the example server resolves every event to `noop`. Per-project hook events are not published. See [process.md](steps/process.md#hooks) for the start event and [record.md](steps/record.md#hooks) for the fact-to-event mapping. +Hooks are the notification boundary. When validation of a commit begins, when a whole-repository validation fact is recorded, and when a cancelled validation ends without a fact, the event reaches deployment systems, dashboards, and developer tooling without any of them polling Stovepipe's store, and each environment can route it to its own downstream (a deploy gate, a Slack notifier, an event bus) without changing the pipeline. The mechanism is the cross-domain hook framework rather than a call out of the pipeline stages: `process` and `record` publish a repository-scoped `HookEvent` to Stovepipe's `hook` topic, and a dispatcher stage consumes it and invokes the wired hooks, so a slow or failing downstream cannot add latency to the pipeline. Both halves exist; what a deployment supplies is the hooks themselves, since the example server resolves every event to `noop`. Per-project hook events are not published. See [process.md](steps/process.md#hooks) for the start event and [record.md](steps/record.md#hooks) for the fact-to-event mapping. ## Workflow @@ -103,9 +103,9 @@ What runs is one pass per Request. It establishes whole-repository greenness, an │ RequestID ▼ ┌──────────────────────────────┐ Hooks - │ record │┄┄┄┄┄► "URI green / - │ Write whole-repo greenness; │ not green" - │ on green advance last-green │ + │ record │┄┄┄┄┄► "URI green, + │ Write whole-repo greenness; │ not green, or + │ on green advance last-green │ cancelled" │ and the promotion ref; │ │ resolve project results │ │ inline │ @@ -116,7 +116,7 @@ What runs is one pass per Request. It establishes whole-repository greenness, an 2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, publishes a **hook event** announcing that validation of the commit has begun, and publishes to `build`. See [process.md](steps/process.md). 3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID. 4. **buildsignal** — polls until the build is terminal, records that status, releases the Queue's `in_flight_count` slot, projects the terminal status onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. -5. **record** — for a succeeded or failed Request, writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It then calls `projectresult.Resolver` and writes one validation fact per returned result. The example server uses the noop resolver, so that list is empty unless a deployment supplies another. It publishes a repository-scoped **hook event** for the green/not-green transition. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. A cancelled Request writes no fact. +5. **record** — for a succeeded or failed Request, writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It then calls `projectresult.Resolver` and writes one validation fact per returned result. The example server uses the noop resolver, so that list is empty unless a deployment supplies another. It publishes `validation.repository.recorded` for that fact. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. A cancelled Request writes no fact and publishes `validation.repository.cancelled`, so a consumer can stop waiting on the commit. ### Designed, not built: project analysis @@ -130,7 +130,7 @@ An `analyze` stage is not part of the pipeline. `stovepipe/core/messagequeue` ha | **process** | RequestID | build, hook topic | Build strategy, concurrency gate, backlog coalescing; announce validation start on admit → [process.md](steps/process.md) | | **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental | | **buildsignal** | BuildID | record | Record terminal build status; release `in_flight_count`; project the outcome onto the Request; publish the request id | -| **record** | RequestID | hook topic | Write whole-repo greenness; resolve project results inline; on green advance last-green URI and the promotion ref; publish the repository hook event | +| **record** | RequestID | hook topic | Write whole-repo greenness; resolve project results inline; on green advance last-green URI and the promotion ref; publish the recorded or cancelled repository hook event | | **hook** | HookEvent | — | Invoke the hooks the resolver returns. The example server resolves every event to noop | ## Step RFCs diff --git a/doc/rfc/submitqueue/workflow.md b/doc/rfc/submitqueue/workflow.md index dab069559..0872e9a07 100644 --- a/doc/rfc/submitqueue/workflow.md +++ b/doc/rfc/submitqueue/workflow.md @@ -1,8 +1,8 @@ # Orchestrator Workflow -The orchestrator processes land requests through the queue-driven pipeline declared in [`submitqueue/orchestrator/pipeline.go`](../../../submitqueue/orchestrator/pipeline.go). The gateway accepts a land over RPC and publishes the full request to `start`; cancel is a separate RPC that publishes to `cancel`. Each controller consumes one topic, reloads what it needs, and publishes the next hop. Inside the orchestrator most hops carry only an ID. The hops that cross into Runway do not: `validate` and `land` publish a full `MergeRequest`, and `landconflictsignal` and `landsignal` consume the `MergeResult` Runway publishes back, because neither service can read the other's storage. Request-log entries are full payloads published with `submitqueue/orchestrator/core/request.PublishLog`; the gateway consumes that topic and is the only writer of the request log. `log`, Runway's `merge-conflict-check`, and Runway's `merge` are publish-only on the orchestrator. See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). +The orchestrator processes land requests through the queue-driven pipeline declared in [`submitqueue/orchestrator/pipeline.go`](../../../submitqueue/orchestrator/pipeline.go). The gateway accepts a land over RPC and publishes the full request to `start`; cancel is a separate RPC that persists `cancelling` and then publishes to `cancel`. Each controller consumes one topic, reloads what it needs, and publishes the next hop. Inside the orchestrator most hops carry only an ID. The hops that cross into Runway do not: `validate` and `land` publish a full `MergeRequest`, and `landconflictsignal` and `landsignal` consume the `MergeResult` Runway publishes back, because neither service can read the other's storage. Request-log entries are full payloads published with `submitqueue/orchestrator/core/request.PublishLog`; the gateway consumes that topic and is the only writer of the request log. `log`, Runway's `merge-conflict-check`, and Runway's `runway-merge` are publish-only on the orchestrator. See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). -Two cycles re-enter speculation. `speculate` dispatches funded paths to `build`; `build` starts each pending path and publishes the build id to `buildsignal`; `buildsignal` polls with `delivery.Hold` and, when the status changes, publishes the batch id back to `speculate`. A batch that can land is published to `land`, which sends the merge request to Runway; `landsignal` correlates the result, marks the batch Succeeded or Failed, and fans out to `conclude` and `speculate`. `speculate` also publishes Failed and Cancelled batches straight to `conclude`. +Two cycles re-enter speculation. `speculate` dispatches funded paths to `build`; `build` starts each pending path and publishes the build id to `buildsignal`; `buildsignal` polls with `delivery.Hold` and, when the status changes, publishes the batch id back to `speculate`. A batch that can land is published to `submitqueue-land`, which sends the merge request to Runway; `landsignal` correlates the result, marks the batch Succeeded or Failed, and fans out to `conclude` and `speculate`. `speculate` also publishes Failed and Cancelled batches straight to `conclude`. Terminal request states are not `conclude`'s alone. `cancel` completes a request that has not been enrolled in a batch. `landconflictsignal` fails a request Runway reports as conflicted. `conclude` maps a terminal batch onto its member requests. The DLQ reconcilers force a failed terminal state when a consumed stage cannot finish. @@ -26,7 +26,7 @@ subgraph group_orchestration["Queue Orchestration"] node_start["start
[start.go]"] node_cancel["cancel
[cancel.go]"] node_validate["validate
[validate.go]"] - node_runwaycheck["Runway conflict check
[merge.go]"] + node_runwaycheck["Runway conflict check
[mergeconflictcheck.go]"] node_conflictsignal["landconflictsignal
[landconflictsignal.go]"] node_batch["batch
[batch.go]"] node_dependency["dependency-analysis
[dependencyanalysis.go]"] @@ -71,7 +71,7 @@ node_runwaycheck -->|"MergeResult"| node_conflictsignal node_conflictsignal -->|"request id"| node_batch node_batch -->|"batch id"| node_dependency node_dependency -->|"batch id"| node_speculate -node_cancel -->|"batch id when enrolled"| node_speculate +node_cancel -->|"batch id when cancellable"| node_speculate node_speculate -->|"batch id"| node_build node_speculate -->|"batch id"| node_land node_speculate -->|"failed or cancelled batch"| node_conclude @@ -138,14 +138,14 @@ class node_submitter toneIndigo | Controller | In | Out | One-line role | |---|---|---|---| | **gateway/Land** | RPC | start | Mint the request id, persist an accepting receipt, publish the full land request, then persist Accepted | -| **gateway/Cancel** | RPC | cancel | Publish a cancel request for an existing land | +| **gateway/Cancel** | RPC | cancel | Persist `cancelling` for an existing land, then publish the cancel request | | **start** | LandRequest | validate, log | Persist the Request and publish it to validate | | **cancel** | CancelRequest | log, or speculate | Record Cancelling. Finish a request that has no applicable batch. Hand each cancellable batch attempt to speculate. Leave a Landing or already-terminal batch for conclude | | **validate** | RequestID | merge-conflict-check (Runway), log | Dedup, fetch change metadata, claim changes, then publish the full `MergeRequest` to Runway keyed by the request id | | **landconflictsignal** | MergeResult | batch, or log | Correlate Runway's check; advance a landable request to batch, or fail a conflicted request | | **batch** | RequestID | dependency-analysis, log | Mint a Creating batch for the request and hand that batch id onward | | **dependency-analysis** | BatchID | speculate, log | Enrol the request, resolve what the batch serializes behind, and promote it from Creating to Created | -| **speculate** | BatchID | build, land, conclude | Treat the message as a dirty signal for the queue: admit Created batches, commit outcomes from facts already known, ask the speculator which paths to fund, and dispatch builds, a land, or conclude | +| **speculate** | BatchID | build, submitqueue-land, conclude | Treat the message as a dirty signal for the queue: admit Created batches, commit outcomes from facts already known, ask the speculator which paths to fund, and dispatch builds, a land, or conclude | | **build** | BatchID | buildsignal | Start a build for each pending path on the head and publish that build id | | **buildsignal** | BuildID | speculate | Poll `BuildRunner.Status`, stop a build whose path no longer wants it, and wake speculate when the status changes. In-flight polls `Hold` the same delivery | | **land** | BatchID | runway-merge (Runway), log | Build the full land request from the batch's member requests, adapt it to `MergeRequest`, and publish it to Runway keyed by the batch id | @@ -158,7 +158,7 @@ class node_submitter toneIndigo ## DLQ reconciliation -Every consumed stage in `pipeline.go` is paired with a `{topic}_dlq` subscription. `pipeline.Construct` registers that companion with `errs.AlwaysRetryableProcessor` and `DLQSubscriptionConfig`, which disables a second-level DLQ and sets `Retry.MaxAttempts` to 0 (unlimited). The consumer moves a message to its DLQ once the primary controller returns a non-retryable error or exhausts retries on a retryable one. The publish-only topics — `log`, Runway `merge-conflict-check`, and Runway `merge` — have no orchestrator subscription and therefore no orchestrator DLQ. The gateway consumes `log`. Runway consumes the two merge request topics. +Every consumed stage in `pipeline.go` is paired with a `{topic}_dlq` subscription. `pipeline.Construct` registers that companion with `errs.AlwaysRetryableProcessor` and `DLQSubscriptionConfig`, which disables a second-level DLQ and sets `Retry.MaxAttempts` to 0 (unlimited). The consumer moves a message to its DLQ once the primary controller returns a non-retryable error or exhausts retries on a retryable one. The publish-only topics — `log`, Runway `merge-conflict-check`, and Runway `runway-merge` — have no orchestrator subscription and therefore no orchestrator DLQ. The gateway consumes `log`. Runway consumes the two merge request topics. Most DLQ controllers do not re-attempt the failed work. They decode the payload to a `RequestID` or `BatchID` and drive that entity to a terminal failed state — `RequestStateError` for requests, `BatchStateFailed` for batches, with fan-out to the member requests. Topics that carry a full payload recover the id from it: the `landconflictsignal` DLQ reads the request id from Runway's `MergeResult`, and the `landsignal` DLQ reads the batch id. The `buildsignal` DLQ decodes a build id, loads that build, and fails the batch that owns it. A missing build or an empty batch id is acked: there is no batch to fail from this signal. State writes use the same optimistic-locking CAS as the primary pipeline, so a late primary-pipeline update wins cleanly and a version mismatch is asked back for redelivery.