diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 94352628c..03e00e1b7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ Thank you for your interest in contributing to SubmitQueue! Whether you are repo ## Getting Started 1. Read the [Development Setup](doc/howto/DEVELOPMENT.md) guide for prerequisites, building, and running tests. -2. Review the [Architecture Guide](CLAUDE.md) to understand project layout, conventions, and code style. +2. Review the [Architecture Guide](AGENTS.md) to understand project layout, conventions, and code style. 3. Check the [Testing Guide](doc/howto/TESTING.md) for testing patterns and requirements. ## Development Workflow @@ -34,7 +34,7 @@ Thank you for your interest in contributing to SubmitQueue! Whether you are repo - Include tests for new functionality. - Ensure all existing tests pass (`make test`). - Ensure CI passes before requesting review. -- Follow the existing code style and patterns described in the [Architecture Guide](CLAUDE.md). +- Follow the existing code style and patterns described in the [Architecture Guide](AGENTS.md). - Fill out the PR template with a description, motivation, and test plan. ## Code Review diff --git a/README.md b/README.md index d05784253..37791cca0 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Designed for large monorepos and fast-moving teams where concurrent changes can ## Repository layout -Cross-domain Go code (errors, metrics, consumer framework, HTTP helpers, shared entities, shared extension contracts) lives under [`platform/`](platform/README.md). Each product domain has its own tree (`submitqueue/`, `stovepipe/`, …) and grows into `gateway/`, `orchestrator/`, `entity/`, `extension/`, and domain-local `core/` — though a domain may start smaller (Stovepipe is currently a single Ping-only service with just `controller/`). See [CLAUDE.md](CLAUDE.md) for conventions and import paths. +Cross-domain Go code (errors, metrics, consumer framework, HTTP helpers, shared entities, shared extension contracts) lives under [`platform/`](platform/README.md). Each product domain has its own tree (`submitqueue/`, `stovepipe/`, …) and grows into `gateway/`, `orchestrator/`, `entity/`, `extension/`, and domain-local `core/` — though a domain may start smaller (Stovepipe is currently a single Ping-only service with just `controller/`). See [AGENTS.md](AGENTS.md) for conventions and import paths. ## Quick Start @@ -49,7 +49,7 @@ The queue's own logic is real in all three: validation, batching, conflict analy | [Development Setup](doc/howto/DEVELOPMENT.md) | Prerequisites, build, environment, IDE setup | | [Contributing](CONTRIBUTING.md) | How to contribute, workflow, guidelines | | [Testing Guide](doc/howto/TESTING.md) | Unit, integration, and E2E testing patterns | -| [Architecture Guide](CLAUDE.md) | Project layout, patterns, conventions | +| [Architecture Guide](AGENTS.md) | Project layout, patterns, conventions | | [Examples](service/README.md) | Running services, clients, API reference | | [RFCs](doc/rfc/index.md) | Design documents and proposals | diff --git a/doc/howto/TESTING.md b/doc/howto/TESTING.md index 08893b8fc..643c107d5 100644 --- a/doc/howto/TESTING.md +++ b/doc/howto/TESTING.md @@ -73,9 +73,9 @@ make build-all-linux # Build Linux binaries for the local docker- - Speed: Fast (< 1s typically) **2. Integration Tests** - Service in isolation with real dependencies -- Location: `test/integration/submitqueue/{service}/` -- Run: `make integration-test-{service}` -- Containers: MySQL + one service +- Location: `test/integration/submitqueue//` (e.g., `gateway/`, `orchestrator/`, `extension//`) +- Run: `make integration-test-submitqueue-gateway`, `make integration-test-submitqueue-orchestrator`, `make integration-test-submitqueue-consumer`, or `make integration-test-extensions` +- Containers: MySQL + one service or the extension's dependencies - Tests one service isolated from others **3. E2E Tests** - Complete workflows across all services @@ -194,14 +194,14 @@ make local-stop make local-submitqueue-gateway-start # Test Ping API (port shown by make local-submitqueue-ps) -grpcurl -plaintext -d '{"message": "hello"}' localhost: submitqueue.SubmitQueueGateway/Ping +grpcurl -plaintext -d '{"message": "hello"}' localhost: uber.submitqueue.gateway.SubmitQueueGateway/Ping # Test Land API grpcurl -plaintext -d '{ "queue": "test-queue", "change": {"source": "github", "ids": ["PR-123"]}, "strategy": "REBASE" -}' localhost: submitqueue.SubmitQueueGateway/Land +}' localhost: uber.submitqueue.gateway.SubmitQueueGateway/Land # Stop make local-submitqueue-gateway-stop @@ -213,7 +213,7 @@ make local-submitqueue-gateway-stop make local-submitqueue-orchestrator-start # Test Ping API (port shown by make local-submitqueue-ps) -grpcurl -plaintext -d '{"message": "hello"}' localhost: submitqueue.SubmitQueueOrchestrator/Ping +grpcurl -plaintext -d '{"message": "hello"}' localhost: uber.submitqueue.orchestrator.SubmitQueueOrchestrator/Ping # Stop make local-submitqueue-orchestrator-stop @@ -256,18 +256,18 @@ brew install grpcurl # macOS grpcurl -plaintext localhost: list # Describe a service -grpcurl -plaintext localhost: describe submitqueue.SubmitQueueGateway +grpcurl -plaintext localhost: describe uber.submitqueue.gateway.SubmitQueueGateway # Call Ping grpcurl -plaintext -d '{"message": "test"}' \ - localhost: submitqueue.SubmitQueueGateway/Ping + localhost: uber.submitqueue.gateway.SubmitQueueGateway/Ping # Call Land grpcurl -plaintext -d '{ "queue": "my-queue", "change": {"source": "github", "ids": ["PR-456"]}, "strategy": "REBASE" -}' localhost: submitqueue.SubmitQueueGateway/Land +}' localhost: uber.submitqueue.gateway.SubmitQueueGateway/Land ``` ### Available Commands @@ -363,9 +363,9 @@ docker network ls | grep sq-test | awk '{print $1}' | xargs docker network rm ### Adding Integration Tests -1. Add test to `test/integration/submitqueue/{service}/suite_test.go` -2. Use suite's resources (`s.client`, `s.db`) -3. Run: `make integration-test-{service}` +1. Add test to `test/integration/submitqueue//suite_test.go` (e.g., `test/integration/submitqueue/gateway/suite_test.go` for Gateway, `test/integration/submitqueue/orchestrator/suite_test.go` for Orchestrator, or a subdirectory under `test/integration/submitqueue/extension/` for extension tests). +2. Use suite's resources (`s.client`, `s.db`). +3. Run the matching Makefile target such as `make integration-test-submitqueue-gateway`. Example: ```go @@ -387,7 +387,7 @@ assert.Equal(s.T(), "expected", resp.Value) ## See Also -- [CLAUDE.md](../../CLAUDE.md) - Development guidelines and project structure +- [AGENTS.md](../../AGENTS.md) - Development guidelines and project structure - [service/submitqueue/docker-compose.yml](../../service/submitqueue/docker-compose.yml) - Full stack service definitions - [service/submitqueue/gateway/server/docker-compose.yml](../../service/submitqueue/gateway/server/docker-compose.yml) - Gateway isolation - [service/submitqueue/orchestrator/server/docker-compose.yml](../../service/submitqueue/orchestrator/server/docker-compose.yml) - Orchestrator isolation diff --git a/doc/rfc/runway/workflow.md b/doc/rfc/runway/workflow.md index 568e8d470..b6d1cf464 100644 --- a/doc/rfc/runway/workflow.md +++ b/doc/rfc/runway/workflow.md @@ -1,16 +1,16 @@ # Runway Workflow -Runway is the landing service: it owns VCS operations — mergeability checking and landing — on behalf of SubmitQueue. Runway is a single service (the domain *is* the service): it subscribes to two inbound topics (`merge-conflict-checker`, `merger`) and publishes results to two outbound topics (`merge-conflict-checker-signal`, `merger-signal`). It is a consumer-only service with no gateway; work arrives via topic queues and results leave via topic queues. +Runway is the landing service: it owns VCS operations — mergeability checking and landing — on behalf of SubmitQueue. Runway is a single service (the domain *is* the service): it subscribes to two inbound topics (`merge-conflict-check`, `runway-merge`) and publishes results to two outbound topics (`merge-conflict-check-signal`, `merge-signal`). It is a consumer-only service with no gateway; work arrives via topic queues and results leave via topic queues. ## Merge-conflict check and merge -The two queues operate at different granularities: +The two queues are the same shape but different commit semantics: -- **merge-conflict-check** is request-level. A merge request carries an ordered sequence of steps (changes + merge strategy). Runway performs a read-only trial merge and publishes per-step mergeability results back. +- **merge-conflict-check** is a dry run. A merge request carries an ordered sequence of steps (changes + merge strategy). Runway performs a read-only trial merge and publishes per-step mergeability results back. -- **merge** is batch-level. A merge request carries the same payload but Runway commits the result and reports the revisions it produced (per-step output IDs). +- **merge** is the committing version. A merge request carries the same payload but Runway commits the result and reports the revisions it produced (per-step output IDs). -A third operation — **promote** — pushes a commit to a ref as-is (`--ff-only`). The primary use case is forwarding a landed SHA from `main` to `verified/main` without creating a new merge commit. Promote reuses the merge queue with the `PROMOTE` merge strategy; Runway fast-forwards the target ref and reports the same SHA back as the output ID. +- **promote** is a special `PROMOTE` strategy on the `merge` queue: it pushes a commit to a ref as-is (`--ff-only`). The primary use case is forwarding a landed SHA from `main` to `verified/main` without creating a new merge commit. These are independent input-output flows. A merge-conflict check can run without a merge ever running, and a merge does not depend on a prior check. @@ -23,39 +23,39 @@ The outbound topics partition by SubmitQueue queue name, matching SubmitQueue's ## Workflow ``` - ┌─────────────────────────────────────────────────────┐ - │ submitqueue orchestrator │ - └──────────┬───────────────────────────┬──────────────┘ - │ │ - MergeRequest (dry run) MergeRequest (commit) - │ │ - ▼ ▼ - [merge-conflict-checker] [merger] - │ │ - merge-conflict-check ctrl merge ctrl - (read-only) (apply + commit) - │ │ - MergeResult MergeResult - │ │ - ▼ ▼ - [merge-conflict-checker-signal] [merger-signal] - │ │ - ▼ ▼ - ┌──────────┬───────────────────────────┬──────────────┐ - │ merge-conflict-check- merge-signal ctrl │ - │ signal ctrl (update batch state, │ - │ (update request fan out to conclude) │ - │ mergeability) │ - │ submitqueue orchestrator │ - └─────────────────────────────────────────────────────┘ + ┌─────────────────────────────────────────────────────┐ + │ submitqueue orchestrator │ + └──────────┬───────────────────────────┬──────────────┘ + │ │ + MergeRequest (dry run) MergeRequest (commit) + │ │ + ▼ ▼ + [merge-conflict-check] [runway-merge] + │ │ + merge-conflict-check ctrl merge ctrl + (read-only) (apply + commit) + │ │ + MergeResult MergeResult + │ │ + ▼ ▼ + [merge-conflict-check-signal] [merge-signal] + │ │ + ▼ ▼ + ┌──────────┬───────────────────────────┬──────────────┐ + │ merge-conflict-check- merge-signal ctrl │ + │ signal ctrl (update batch state, │ + │ (update request fan out to conclude) │ + │ mergeability) │ + │ submitqueue orchestrator │ + └─────────────────────────────────────────────────────┘ ``` ## Per-controller summary | Controller | In | Out | One-line role | |---|---|---|---| -| **merge-conflict-check** | MergeRequest | MergeResult -> merge-conflict-checker-signal | Dry-run merge: check mergeability of ordered steps against the target branch (read-only) | -| **merge** | MergeRequest | MergeResult -> merger-signal | Apply, commit, and report per-step output IDs | +| **merge-conflict-check** | MergeRequest | MergeResult -> merge-conflict-check-signal | Dry-run merge: check mergeability of ordered steps against the target branch (read-only) | +| **merge** | MergeRequest | MergeResult -> merge-signal | Apply, commit, and report per-step output IDs | The merge-conflict-check controller always publishes a result — even when all steps are mergeable — so SubmitQueue receives a definitive answer. On infrastructure error it nacks for retry. @@ -83,8 +83,8 @@ Runway has no persistent state — no request store, no job store, no database. ### Runway -Runway is a single service. It subscribes to two inbound topics (`merge-conflict-checker`, `merger`), performs VCS operations through a pluggable extension, and publishes results to two outbound topics (`merge-conflict-checker-signal`, `merger-signal`). It owns no persistent data. +Runway is a single service. It subscribes to two inbound topics (`merge-conflict-check`, `runway-merge`), performs VCS operations through a pluggable extension, and publishes results to two outbound topics (`merge-conflict-check-signal`, `merge-signal`). It owns no persistent data. ### Shared: the messaging queue -Runway communicates with SubmitQueue only through the messaging queue. The inbound topics are owned by runway; the outbound topics are owned by SubmitQueue. +Runway communicates with SubmitQueue only through the messaging queue. The contract is owned by Runway and published under `api/runway/messagequeue/`; both inbound and outbound topic keys live there. SubmitQueue publishes `MergeRequest` messages and consumes the `MergeResult` signals. diff --git a/doc/rfc/stovepipe/steps/build.md b/doc/rfc/stovepipe/steps/build.md index 0078ecd2d..0501c9900 100644 --- a/doc/rfc/stovepipe/steps/build.md +++ b/doc/rfc/stovepipe/steps/build.md @@ -151,7 +151,7 @@ So `build`'s `Trigger` gets its own shape under `stovepipe/extension/buildrunner ### 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 [CLAUDE.md](CLAUDE.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) @@ -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 CLAUDE.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. @@ -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 [CLAUDE.md](CLAUDE.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 [CLAUDE.md](CLAUDE.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`): diff --git a/doc/rfc/stovepipe/steps/process.md b/doc/rfc/stovepipe/steps/process.md index 7f48603ac..4bece0a8a 100644 --- a/doc/rfc/stovepipe/steps/process.md +++ b/doc/rfc/stovepipe/steps/process.md @@ -199,7 +199,7 @@ Per-queue knobs such as `max_concurrent` live outside this row — see [Per-Queu | *(owned by buildsignal)* succeeded / failed / cancelled | Phase 1 build outcome | **yes** | | *(later)* building, recording, … | Finer states as downstream stages need them | — | -Transitions use the repo's optimistic-locking pattern: compute `newVersion = oldVersion + 1`, call `RequestStore.Update(ctx, req, oldVersion, newVersion)`, assign `req.Version = newVersion` only on success (see [storage README](../../../../submitqueue/extension/storage/README.md) and [CLAUDE.md](../../../../CLAUDE.md)). +Transitions use the repo's optimistic-locking pattern: compute `newVersion = oldVersion + 1`, call `RequestStore.Update(ctx, req, oldVersion, newVersion)`, assign `req.Version = newVersion` only on success (see [storage README](../../../../submitqueue/extension/storage/README.md) and [AGENTS.md](../../../../AGENTS.md)). ## Storage contract additions diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index c7b62442c..73bba9045 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 | **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 greenness events to downstream systems — "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. Not implemented yet; it will be 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 [CLAUDE.md](../../../CLAUDE.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 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 recording stage: `record` publishes 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. Neither half exists yet; see [record.md](steps/record.md#hooks) for the fact-to-event mapping and its open questions. diff --git a/doc/rfc/submitqueue/extension-contract.md b/doc/rfc/submitqueue/extension-contract.md index 81e086e5b..80c241ff2 100644 --- a/doc/rfc/submitqueue/extension-contract.md +++ b/doc/rfc/submitqueue/extension-contract.md @@ -14,7 +14,7 @@ Both unblock with the shape `conflict` already uses: accept identity, resolve in ## Principle - **Decision/action extensions** take orchestrator identity at their stage granularity and resolve granular content through narrowly-injected dependencies. Request stage → `entity.Request`; batch stage → `entity.Batch` / `[]entity.Batch`. Both are thin reference entities (a `Request` carries URIs, not diffs; a `Batch` carries IDs, not changes). -- **Resolution targets** — `storage`, `changestore`, `queueconfig` — stay key/value-shaped. They are what the others resolve *through* (see [storage/README.md](../../../submitqueue/extension/storage/README.md) and CLAUDE.md). Refinement: the storage *aggregate* has since gained the same per-queue factory resolution every other seam has — the stores it hands back remain strictly key/value, bound to their queue, while the cross-queue read-model stores stay individually-injected singletons. +- **Resolution targets** — `storage`, `changestore`, `queueconfig` — stay key/value-shaped. They are what the others resolve *through* (see [storage/README.md](../../../submitqueue/extension/storage/README.md) and [AGENTS.md](../../../AGENTS.md)). Refinement: the storage *aggregate* has since gained the same per-queue factory resolution every other seam has — the stores it hands back remain strictly key/value, bound to their queue, while the cross-queue read-model stores stay individually-injected singletons. - **Output mirrors the input unit.** Each output element self-identifies with the input it corresponds to — `changeprovider`'s `ChangeInfo` carries its `URI`, `conflict`'s `Conflict` carries its `BatchID` — so a flat list suffices and the caller correlates results back to inputs without re-deriving boundaries. A *wrapper* entity (`entity.BatchChanges`) is introduced only to aggregate *up* to a coarser unit than the elements — the scorer needs batch-wide line/file totals, so the rollup earns its keep; no `RequestChanges` exists because nothing needs request-wide rollups. And when the input is a *collection* of independently-actioned units, the output groups by them: `pusher`, fed `[]entity.Batch`, returns outcomes grouped per batch, the same way `conflict` already tags each `Conflict` with its in-flight `BatchID`. ### What each stage resolves today diff --git a/doc/rfc/submitqueue/modular-queue-wiring.md b/doc/rfc/submitqueue/modular-queue-wiring.md index c63a59479..1acdddb89 100644 --- a/doc/rfc/submitqueue/modular-queue-wiring.md +++ b/doc/rfc/submitqueue/modular-queue-wiring.md @@ -12,7 +12,7 @@ The orchestrator's example `main.go` (`example/submitqueue/orchestrator/server/m 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. -The [TODO on line 475](../../../service/submitqueue/orchestrator/server/main.go) already flags the queue-registry pattern as a candidate for promotion into the domain layer, contingent on a trigger: a second consumer needing the same wiring, data-driven config, or lifecycle requirements. +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. ## Vocabulary diff --git a/doc/rfc/submitqueue/workflow.md b/doc/rfc/submitqueue/workflow.md index e34eb85e9..b50e3958c 100644 --- a/doc/rfc/submitqueue/workflow.md +++ b/doc/rfc/submitqueue/workflow.md @@ -1,6 +1,6 @@ # 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 `merge` stages both hand work to runway — a merge-conflict check and the merge itself — and consume its result on `mergeconflictsignal` / `mergesignal`.) See the queue-payload-boundary rule in [CLAUDE.md](../../../CLAUDE.md). +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 `merge` stages both hand work to runway — a merge-conflict check and the merge itself — and consume its result on `mergeconflictsignal` / `mergesignal`.) See the queue-payload-boundary rule in [AGENTS.md](../../../AGENTS.md). The pipeline has two cycles: `speculate → build → buildsignal → speculate` (CI feedback loop) and `merge → runway → mergesignal → 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`. diff --git a/service/README.md b/service/README.md index 2e49af737..939808e29 100644 --- a/service/README.md +++ b/service/README.md @@ -13,9 +13,9 @@ Each domain has its own subdirectory with a dedicated README: | Service | Port | Domain | RPCs | Backing stores | |---------|------|--------|------|----------------| | **SubmitQueue Gateway** | 8081 | `submitqueue` | `Ping`, `Land`, `Cancel`, `GetRequestSummaryByID`, `GetRequestSummaryByChangeURI`, `List`, `GetRequestHistoryByID`, `GetRequestHistoryByChangeURI` | MySQL app + queue | -| **SubmitQueue Orchestrator** | 8082 | `submitqueue` | `Ping` (+ consumes 9 pipeline topics) | MySQL app + queue | -| **Stovepipe** | 8083 | `stovepipe` | `Ping`, `Ingest` (+ consumes the process topic) | MySQL storage + queue | -| **Runway** | 8086 | `runway` | `Ping` (+ consumes merge-conflict-check & merge topics) | MySQL queue | +| **SubmitQueue Orchestrator** | 8082 | `submitqueue` | `Ping` (+ consumes pipeline topics: start, cancel, validate, batch, dependency-analysis, speculate, build, buildsignal, submitqueue-merge, conclude, log, plus DLQ topics, and the two Runway signal topics) | MySQL app + queue | +| **Stovepipe** | 8083 | `stovepipe` | `Ping`, `Ingest` (+ consumes the process, build, buildsignal, and record topics, plus 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). @@ -34,7 +34,8 @@ service/ │ └── client/ # Orchestrator ping client ├── stovepipe/ │ ├── docker-compose.yml # Stovepipe service + storage MySQL + queue MySQL -│ ├── server/ # Stovepipe gRPC server + Dockerfile +│ ├── docker-compose.debug.yml # Debug variant with delve +│ ├── server/ # Stovepipe gRPC server + Dockerfile + compose │ └── client/ # Stovepipe ping client └── runway/ ├── server/ # Runway gRPC server + Dockerfile + compose @@ -53,6 +54,7 @@ make local-submitqueue-orchestrator-start # orchestrator-only stack # Stovepipe service (gRPC service + storage MySQL + queue MySQL) make local-stovepipe-start +make local-stovepipe-logs # Runway service (consumer + queue MySQL) make local-runway-start diff --git a/service/stovepipe/README.md b/service/stovepipe/README.md index 4a378bfed..2e8253802 100644 --- a/service/stovepipe/README.md +++ b/service/stovepipe/README.md @@ -1,14 +1,18 @@ # Stovepipe Service -Runnable wiring for the **Stovepipe** domain — a single-service domain (the domain *is* the service). The server exposes two RPCs and runs one internal pipeline stage as a queue consumer: +Runnable wiring for the **Stovepipe** domain — a single-service domain (the domain *is* the service). The server exposes two RPCs and runs the internal pipeline stages as queue consumers: - **`Ping`** — health check. - **`Ingest`** — resolves a queue's head commit, persists a `Request` (and its head URI) to storage, and publishes the request to the **process** stage. - **process consumer** (`TopicKeyProcess`) — reloads the persisted `Request` from storage and runs the process stage (`stovepipe/controller/process`). +- **build consumer** (`TopicKeyBuild`) — reloads the persisted `Request` and triggers the build-runner, then publishes to `buildsignal`. +- **buildsignal consumer** (`TopicKeyBuildSignal`) — polls/records the build's terminal status and releases the queue's in-flight slot, then publishes to `record`. +- **record consumer** (`TopicKeyRecord`) — writes the whole-repo validation fact, advances the queue's last-green bookmark and promotion ref, and publishes hook events. +- **DLQ reconciler** — for each internal topic, a `_dlq` consumer that drives stuck requests to a conservative terminal state so the queue's slot is freed. -The ingest → process hop stays inside one service and one store, so only the request **ID** travels on the queue; the consumer reloads from storage (the source of truth), which keeps messages small and redelivery idempotent. The process topic key and its internal wire contract are owned by the domain under `stovepipe/core/messagequeue/`. +The ingest → process → build → buildsignal → record hop stays inside one service and one store, so the queue messages carry only request **IDs**; the consumers reload from storage (the source of truth), which keeps messages small and redelivery idempotent. The process, build, buildsignal, and record topic keys and their internal wire contract are owned by the domain under `stovepipe/core/messagequeue/`. -Stovepipe therefore needs two MySQL databases: a **storage** database (the `request` and `request_uri` tables) and a **queue** database (messaging infrastructure). +Stovepipe therefore needs two MySQL databases: a **storage** database (the `queue`, `request`, `request_uri`, and `build` tables) and a **queue** database (messaging infrastructure). ## Wiring notes @@ -22,14 +26,15 @@ Stovepipe therefore needs two MySQL databases: a **storage** database (the `requ ``` stovepipe/ ├── docker-compose.yml # Stovepipe service + storage MySQL + queue MySQL +├── docker-compose.debug.yml # Debug variant with delve ├── server/ -│ ├── main.go # gRPC server (Ping, Ingest) + process-stage consumer wiring +│ ├── main.go # gRPC server (Ping, Ingest) + pipeline consumer wiring │ └── Dockerfile └── client/ └── main.go # Ping client (default :8083) ``` -The Stovepipe controllers live under [`stovepipe/controller/`](../../stovepipe/controller) and its extensions under [`stovepipe/extension/`](../../stovepipe/extension); this directory only contains the runnable wiring and a Docker Compose stack for manual testing. +The Stovepipe controllers live under [`stovepipe/controller/`](../../stovepipe/controller) (subdirectories for `ingest`, `process`, `build`, `buildsignal`, `record`, and `dlq`) and its extensions under [`stovepipe/extension/`](../../stovepipe/extension); this directory only contains the runnable wiring and a Docker Compose stack for manual testing. ## Configuration diff --git a/stovepipe/README.md b/stovepipe/README.md index 475de8846..269e8388d 100644 --- a/stovepipe/README.md +++ b/stovepipe/README.md @@ -1,7 +1,7 @@ # Stovepipe -Stovepipe is currently a single Ping-only service. Its layout: +Stovepipe is a post-merge validation service. Its layout: -- `controller/` — business logic (transport-agnostic). Currently exposes the `Ping` RPC. +- `controller/` — business logic (transport-agnostic). Exposes the `Ping` and `Ingest` RPCs, and consumes the internal pipeline stages (`process`, `build`, `buildsignal`, `record`) plus a DLQ reconciler. -The wire contract lives under `api/stovepipe/` (`proto/` for the `.proto` source, `protopb/` for the committed generated stubs). Entities, extensions, and the orchestration pipeline will be added back as the service grows. +The wire contract lives under `api/stovepipe/` (`proto/` for the `.proto` source, `protopb/` for the committed generated stubs). The internal queue contract and topic keys live under `stovepipe/core/messagequeue/`. Storage, source-control, and build-runner extensions live under `stovepipe/extension/`. diff --git a/stovepipe/extension/storage/README.md b/stovepipe/extension/storage/README.md index 530b07877..8ced668dc 100644 --- a/stovepipe/extension/storage/README.md +++ b/stovepipe/extension/storage/README.md @@ -8,7 +8,7 @@ This is a separate contract from `submitqueue/extension/storage` — same shape ## Optimistic locking contract -Entities that support concurrent mutation (`Request`, `Build`) carry an `int32 Version` field. `Update` methods take both `oldVersion` (the where-clause guard) and `newVersion` (the value to write) — the store performs a pure conditional write and never computes `oldVersion + 1` itself. Version arithmetic is owned by the controller: it computes `newVersion`, calls `Update`, and only assigns `entity.Version = newVersion` after the call succeeds. See [CLAUDE.md](../../../CLAUDE.md) and the [submitqueue storage README](../../../submitqueue/extension/storage/README.md#optimistic-locking-contract) for the full rationale and the caller pattern — the convention is identical here. +Entities that support concurrent mutation (`Request`, `Build`) carry an `int32 Version` field. `Update` methods take both `oldVersion` (the where-clause guard) and `newVersion` (the value to write) — the store performs a pure conditional write and never computes `oldVersion + 1` itself. Version arithmetic is owned by the controller: it computes `newVersion`, calls `Update`, and only assigns `entity.Version = newVersion` after the call succeeds. See [AGENTS.md](../../../AGENTS.md) and the [submitqueue storage README](../../../submitqueue/extension/storage/README.md#optimistic-locking-contract) for the full rationale and the caller pattern — the convention is identical here. ## Read-after-write consistency diff --git a/submitqueue/README.md b/submitqueue/README.md index 6147d254a..11bcf3b38 100644 --- a/submitqueue/README.md +++ b/submitqueue/README.md @@ -6,6 +6,6 @@ SubmitQueue service layout: - `orchestrator/` — Orchestrator service: coordinates the land pipeline (batch, speculate, build, merge, conclude, ...). - `extension/` — SubmitQueue-specific extension implementations (storage, counter, changestore, mergechecker, pusher, scorer, conflict, queueconfig, buildrunner, ...). - `entity/` — SubmitQueue-specific domain entities. -- `core/` — Infrastructure shared across SubmitQueue's own services (gateway and orchestrator): the queue `consumer` framework and the `request` lifecycle. The SubmitQueue-scoped analogue of the repo-level `core/`. +- `core/` — Infrastructure shared across SubmitQueue's own services (gateway and orchestrator): the queue `consumer` framework, the `request` lifecycle, and topic keys. The SubmitQueue-scoped analogue of the repo-level `platform/`. -Cross-domain building blocks live outside this directory: shared entities in `entity/`, shared extensions in `extension/`, and cross-domain infrastructure in the top-level `core/`. +Cross-domain building blocks live outside this directory: shared entities in `platform/base/`, shared extensions in `platform/extension/`, and cross-domain infrastructure in `platform/`. diff --git a/submitqueue/extension/storage/README.md b/submitqueue/extension/storage/README.md index 21002ee37..2672851f8 100644 --- a/submitqueue/extension/storage/README.md +++ b/submitqueue/extension/storage/README.md @@ -48,7 +48,7 @@ A `Get` immediately following a successful write (`Create`/`Update`) — by the ## Key-value contract -Store interfaces are designed for the storage technology *space*, not for SQL (see the Extensions section of the repo `CLAUDE.md`): every method must be satisfiable by a plain key-value backend (DynamoDB, Bigtable, an in-memory map) as cheaply as by MySQL. Concretely, a store exposes only get/put/conditional-update **by primary key**. No lookups by other attributes, no listings filtered server-side, no joins. +Store interfaces are designed for the storage technology *space*, not for SQL (see the Extensions section of the repo [AGENTS.md](../../../../AGENTS.md)): every method must be satisfiable by a plain key-value backend (DynamoDB, Bigtable, an in-memory map) as cheaply as by MySQL. Concretely, a store exposes only get/put/conditional-update **by primary key**. No lookups by other attributes, no listings filtered server-side, no joins. **The smell test is the index.** If implementing a proposed store method in MySQL requires adding a secondary index (`KEY idx_*`) to the schema, the method is a query-by-attribute in disguise and the contract has left the key-value space — a KV backend would need a global secondary index or a hand-maintained index table to fake it. Treat a new `KEY` line in a schema diff as a design review flag, not a tuning detail. diff --git a/submitqueue/orchestrator/README.md b/submitqueue/orchestrator/README.md index 0fd2cf1ee..c2405ffc1 100644 --- a/submitqueue/orchestrator/README.md +++ b/submitqueue/orchestrator/README.md @@ -1 +1,22 @@ -SubmitQueue Orchestrator +# SubmitQueue Orchestrator + +The orchestrator runs the SubmitQueue land pipeline. It consumes the internal topics declared in `submitqueue/core/topickey/` and advances requests and batches through the stages that lead from `accepted` to a terminal state. + +## Pipeline stages + +The pipeline is queue-driven: each stage consumes one topic, advances one entity, and publishes to the next topic. + +- **start** — receives `LandRequest` from the gateway, persists the `Request` entity, and emits `Started`. +- **validate** — checks for duplicates, resolves change metadata, and publishes a `MergeRequest` to Runway's `merge-conflict-check` topic. +- **mergeconflictsignal** — correlates the dry-run result, fails the request on conflict, or forwards it to batching. +- **batch** — groups the request into a `Batch` with its dependencies. +- **speculate** — decides which speculative paths to validate (CI) versus land directly. +- **build** — triggers a CI build for a speculative path. +- **buildsignal** — records the CI result and loops back to `speculate`. +- **merge** — publishes a committing `MergeRequest` to Runway's `runway-merge` topic. +- **mergesignal** — correlates the merge result and fans out to `conclude` and back to `speculate`. +- **conclude** — maps the terminal batch state to the request states. +- **log** — persists gateway-owned request-log events published by the orchestrator. +- **DLQ reconcilers** — one per primary consumed topic, driving stuck requests/batches to a conservative terminal `failed` state. + +See [doc/rfc/submitqueue/workflow.md](../../doc/rfc/submitqueue/workflow.md) for the full pipeline diagram and ownership rules.