Skip to content

Commit 834ee0d

Browse files
committed
docs: align architecture guides with implementation
Summary: Intent: - Keep the repository's canonical architecture guidance consistent with the current service and pipeline boundaries. - Distinguish durable snapshots, request-log projections, and queue wire formats accurately. Changes: - Updated AGENTS.md and domain overviews for current controller, storage, queue, and Stovepipe behavior. - Corrected Gateway and Orchestrator lifecycle documentation and refreshed extension-specific guides. --- Generated by the pr-create skill in devexp-agent-marketplace.
1 parent 67829de commit 834ee0d

9 files changed

Lines changed: 109 additions & 166 deletions

File tree

AGENTS.md

Lines changed: 31 additions & 26 deletions
Large diffs are not rendered by default.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Designed for large monorepos and fast-moving teams where concurrent changes can
1111

1212
## Repository layout
1313

14-
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.
14+
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/`, …). Multi-service domains may split into `gateway/` and `orchestrator/`; single-service domains keep controllers directly under the domain root. Stovepipe currently exposes ingestion behavior and runs its own queue pipeline. See [AGENTS.md](AGENTS.md) for conventions and import paths.
1515

1616
## Quick Start
1717

submitqueue/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
SubmitQueue service layout:
44

5-
- `gateway/` — Gateway service: entry point for land requests (`Ping`, `Land`, `Cancel` RPCs).
6-
- `orchestrator/` — Orchestrator service: coordinates the land pipeline (batch, speculate, build, merge, conclude, ...).
7-
- `extension/` — SubmitQueue-specific extension implementations (storage, counter, changestore, mergechecker, pusher, scorer, conflict, queueconfig, buildrunner, ...).
5+
- `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.
6+
- `orchestrator/` — Orchestrator service: coordinates validation, dependency analysis, speculation, builds, landing, cancellation, conclusion, hooks, and DLQ reconciliation.
7+
- `extension/` — SubmitQueue-specific extension contracts and implementations, including storage, queue configuration, change providers, validation, conflict analysis, speculation, and build runners.
88
- `entity/` — SubmitQueue-specific domain entities.
9-
- `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/`.
9+
- `core/` — Infrastructure shared across SubmitQueue's own services, including request and batch lifecycle helpers, change-set resolution, and internal topic keys.
1010

11-
Cross-domain building blocks live outside this directory: shared entities in `platform/base/`, shared extensions in `platform/extension/`, and cross-domain infrastructure in `platform/`.
11+
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/`.

submitqueue/core/changeset/README.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
# changeset
22

3-
`changeset` resolves batch identity into the changes a batch contains. It is the single place the orchestrator walks batch → requests → changes, consolidating the resolution the build and merge controllers each performed privately.
3+
`changeset` resolves batch identity into the changes a batch contains. Current consumers include build-runner and scorer implementations and the path-overlap conflict analyzer. The merge controller loads member requests directly because its Runway payload preserves one ordered merge step per request.
44

55
## Why it exists
66

7-
A `Batch` is a thin reference entity: it carries the IDs of the requests it contains, not their changes. Decision and action extensions (the scorer, build runner, pusher, and future detail-aware conflict analyzers) are handed that identity and resolve the granular content themselves through an injected `Resolver`, rather than depending on a controller to pre-resolve and pass the data in. The resolver depends only on the two resolution-target stores — the request store (to walk a batch's contained requests) and the change store (to attach provider details) — and nothing else.
7+
A `Batch` is a thin reference entity: it carries the IDs of the requests it contains, not their changes. Decision and action extensions such as scorers, build runners, and conflict analyzers are handed that identity and resolve the granular content themselves through an injected `Resolver`, rather than depending on a controller to pre-resolve and pass the data in. The resolver uses the queue-scoped storage aggregate to reach the request store (to walk a batch's contained requests) and the change store (to attach provider details).
88

99
## Two fidelities
1010

11-
The resolver offers the same walk at two levels of detail, and both preserve batch boundaries — neither flattens across batches, so a caller that wants a flat list flattens the result itself:
11+
Both methods operate on one batch per call:
1212

13-
- The raw view returns each batch's contained changes as URIs only, one group per input batch, in input order. It performs no change-store read. The build stage uses it for base and head inputs; the merge stage uses it for the pusher.
14-
- The detailed view returns a single batch's normalized, batch-level changes: one entry per claimed URI, each carrying the provider details recorded in the change store, aggregated across every request in the batch. Because the change store returns rows for every request that ever claimed a URI, the resolver selects the row owned by the requesting request. The scorer uses it, as will any analyzer that needs changed-file or line-count facts.
13+
- The raw view returns a batch's contained changes as URIs only, in request order. It performs no change-store read. Build-runner implementations use it to construct base and head inputs.
14+
- The detailed view returns a single batch's normalized, batch-level changes: one entry per claimed URI, each carrying the provider details recorded in the change store, aggregated across every request in the batch. Because the change store returns rows for every request that ever claimed a URI, the resolver selects the row owned by the requesting request. Scorers and detail-aware conflict analyzers use this view.
1515

1616
## Testing
1717

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,25 @@
11
# Conflict
22

3-
Vendor-agnostic interface for detecting conflicts between a candidate batch
4-
and the batches already in flight.
3+
Vendor-agnostic interface for detecting conflicts between a candidate batch and the batches already in flight.
54

65
## Interface
76

8-
`Analyzer` exposes a single `Analyze` method that takes the candidate batch
9-
and the list of in-flight batches it might conflict with. It returns the
10-
subset of in-flight batches that conflict with the candidate, each paired
11-
with a `ConflictType` describing the kind of conflict. An empty result means
12-
the candidate is free to advance independently.
7+
`Analyzer` exposes a single `Analyze` method that takes the candidate batch and the list of in-flight batches it might conflict with. It returns the subset of in-flight batches that conflict with the candidate, each paired with a `ConflictType` describing the kind of conflict. An empty result means the candidate is free to advance independently.
138

14-
Callers are responsible for filtering out the candidate itself and any
15-
terminal batches from the in-flight list before invoking the analyzer. The
16-
analyzer itself stays free of lifecycle knowledge. A non-nil error reports
17-
an infrastructure failure of the analysis and should be treated as
18-
retryable by the caller.
9+
Callers are responsible for filtering out the candidate itself and any terminal batches from the in-flight list before invoking the analyzer. The analyzer itself stays free of lifecycle-transition knowledge. A non-nil error reports that analysis could not be completed; implementations return plain errors and the configured error classifiers decide retryability.
1910

20-
The analyzer is intentionally pure with respect to batch state: it does not
21-
mutate inputs, does not read storage, and may be called concurrently. Real
22-
implementations are expected to resolve the batch contents (e.g. changed
23-
build targets, modified files) via whichever upstream system they depend
24-
on, and to return as much classification detail as that system supports.
11+
The analyzer does not mutate batch inputs and may be called concurrently. Implementations resolve the batch contents they need through injected dependencies. For example, `pathoverlap` uses a `changeset.Resolver`, whose store-backed implementation reads queue-scoped request and change records.
2512

2613
## Implementations
2714

28-
- [`all/`](all/) — pessimistic stub: reports every in-flight batch as a
29-
`ConflictTypeConservative` conflict. Useful as a worst-case baseline and
30-
for wiring tests where speculation must serialize.
31-
- [`none/`](none/) — optimistic stub: reports no conflicts. Useful as a
32-
best-case baseline and for wiring tests where speculation should run all
33-
batches in parallel.
15+
- [`all/`](all/) — pessimistic stub: reports every in-flight batch as a `ConflictTypeConservative` conflict. Useful as a worst-case baseline and for wiring tests where speculation must serialize.
16+
- [`fake/`](fake/) — wraps another analyzer and optionally injects configured failures for tests and example wiring.
17+
- [`none/`](none/) — optimistic stub: reports no conflicts. Useful as a best-case baseline and for wiring tests where speculation should run all batches in parallel.
18+
- [`pathoverlap/`](pathoverlap/) — resolves changed files and reports overlap conflicts by whole file or parent directory.
3419

3520
## Adding a new backend
3621

3722
1. Create `extension/conflict/{backend}/` with an `Analyzer` implementation.
38-
2. Resolve each `entity.Batch` into whatever signal the backend needs
39-
(e.g. changed build targets, files touched, dependency graphs).
40-
3. Emit one `Conflict` per (in-flight batch, detected conflict type). Pick
41-
the most specific `ConflictType` your backend can determine; use
42-
`ConflictTypeConservative` only when the backend cannot prove the absence
43-
of a conflict and falls back to a pessimistic default. Introduce a new
44-
`ConflictType` constant when you can classify the conflict more precisely.
45-
4. Return a plain error for transient infrastructure failures so callers
46-
can classify and retry.
23+
2. Resolve each `entity.Batch` into whatever signal the backend needs (e.g. changed build targets, files touched, dependency graphs).
24+
3. Emit one `Conflict` per (in-flight batch, detected conflict type). Pick the most specific `ConflictType` your backend can determine; use `ConflictTypeConservative` only when the backend cannot prove the absence of a conflict and falls back to a pessimistic default. Introduce a new `ConflictType` constant when you can classify the conflict more precisely.
25+
4. Return plain errors and let the consumer's configured classifiers determine retry behavior.

submitqueue/extension/storage/README.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ Pluggable persistence interfaces for SubmitQueue entities (requests, batches, de
66

77
Storage follows the extension contract: the queue-scoped store aggregate is resolved per queue through a factory keyed by queue name, mirroring how every decision/action extension resolves its implementation. A resolved aggregate is bound to its queue — entity arguments whose queue disagrees with the binding are rejected, queue-keyed reads are implicitly scoped, and the host wiring decides which backend serves which queue (single shared backend by default).
88

9-
Three read-model stores are deliberately global rather than queue-scoped, because their lookups start from identifiers that arrive without queue context (a bare request ID or change URI at the status API): the request log, the request summary, and the change-URI mapping. They are injected individually as standalone seams, following the gateway's per-store injection. The queue registry (`queueconfig`) was never part of this aggregate and stays the registry the factory sits beside.
10-
11-
The classification rule: a store is queue-scoped when every read path authoritatively holds the queue before the first read, and global when any read path begins from an identifier that arrives without queue context. Entity IDs are opaque — no reader may derive the queue from an ID prefix; the queue travels explicitly on payloads and requests.
9+
Every entity and read-model store is a member of the queue-scoped `Storage` aggregate returned by `Factory.For`. Gateway read requests therefore carry the queue explicitly before resolving request summaries, logs, URI mappings, or queue-list projections. Entity IDs remain opaque — readers do not derive the queue from an ID prefix.
1210

1311
## Optimistic locking contract
1412

@@ -48,7 +46,7 @@ A `Get` immediately following a successful write (`Create`/`Update`) — by the
4846

4947
## Key-value contract
5048

51-
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.
49+
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. Contracts use point operations by complete primary key and deliberate bounded prefix/range reads where the lookup fields are leading components of the primary key. They do not require secondary-index lookups, joins, or arbitrary server-side predicates.
5250

5351
**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.
5452

submitqueue/gateway/README.md

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,23 @@ The gateway is the RPC entry point to SubmitQueue. It accepts `Land`, `Cancel`,
44

55
## Request receipts and current summaries
66

7-
`Land` creates gateway-owned receipt projections before publishing the request:
7+
`Land` first creates an internal request summary in `accepting` state. That receipt prevents a failed publish from exposing work that was never admitted. After the start message is published, the gateway records the `accepted` log through the request materializer.
88

9-
- An authoritative request summary keyed by sqid.
10-
- One exact change-URI mapping per submitted URI.
11-
- A queue-ordered receipt projection used by `List`.
9+
The materializer appends the log, chooses the winning current status, and activates or repairs the public projections:
1210

13-
`GetRequestSummaryByID` and `GetRequestSummaryByChangeURI` read authoritative summaries. `List` reads the queue projection and may briefly lag those summaries while eventual repair converges.
11+
- The authoritative request summary keyed by sqid.
12+
- One change-URI mapping per submitted URI.
13+
- The queue-ordered projection used by `List`.
14+
15+
`GetRequestSummaryByID` and `GetRequestSummaryByChangeURI` read authoritative summaries. `List` reads the queue projection and may briefly lag while later log materialization repairs a partial attempt. If recording `accepted` fails after publication, `Land` still succeeds because subsequent pipeline logs can advance the internal `accepting` summary and create the public projections.
1416

1517
## Request log ownership
1618

1719
The gateway owns the request log read model and is the only service that reads it.
1820

19-
- For statuses produced synchronously by the gateway, such as `accepted` on `Land` and `cancelling` on `Cancel`, the gateway persists the event through the shared request-log materializer before returning or publishing.
21+
- `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.
2022
- 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.
21-
- Orchestrator DLQ reconciliation materializes terminal repairs directly so the DLQ delivery remains unacknowledged until the log and public projections converge.
23+
- Orchestrator DLQ reconciliation transitions durable request state and publishes terminal log entries to the same `log` topic; the gateway remains the materializer.
2224
- `GetRequestHistoryByID` and `GetRequestHistoryByChangeURI` read retained request-log rows directly.
2325

2426
The materializer appends every audit event, selects the current authoritative winner, and repairs the queue projection. The normal orchestrator pipeline does not read or write the request-log store directly.

submitqueue/orchestrator/README.md

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,24 @@
11
# SubmitQueue Orchestrator
22

3-
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.
3+
The orchestrator runs the SubmitQueue land pipeline. Its complete consumed-stage and publish-only topology is declared in [`pipeline.go`](pipeline.go). It consumes SubmitQueue-owned internal topics, Runway-owned result topics, and the shared hook topic.
44

55
## Pipeline stages
66

77
The pipeline is queue-driven: each stage consumes one topic, advances one entity, and publishes to the next topic.
88

99
- **start** — receives `LandRequest` from the gateway, persists the `Request` entity, and emits `Started`.
10+
- **cancel** — records cancellation intent and hands affected batches to speculation for best-effort cancellation.
1011
- **validate** — checks for duplicates, resolves change metadata, and publishes a `MergeRequest` to Runway's `merge-conflict-check` topic.
11-
- **mergeconflictsignal** — correlates the dry-run result, fails the request on conflict, or forwards it to batching.
12-
- **batch** — groups the request into a `Batch` with its dependencies.
13-
- **speculate** — decides which speculative paths to validate (CI) versus land directly.
12+
- **merge-conflict-check-signal** — correlates the dry-run result, fails the request on conflict, or forwards it to batching.
13+
- **batch** — creates an inert batch attempt and hands it to dependency analysis.
14+
- **dependency-analysis** — enrols requests, computes dependencies, and promotes the selected batch attempt.
15+
- **speculate** — reconciles queue-wide path state, decides outcomes, and allocates speculative builds.
1416
- **build** — triggers a CI build for a speculative path.
15-
- **buildsignal**records the CI result and loops back to `speculate`.
17+
- **buildsignal**polls or receives CI state, records the result, wakes `speculate`, and holds non-terminal deliveries until the next poll.
1618
- **merge** — publishes a committing `MergeRequest` to Runway's `runway-merge` topic.
17-
- **mergesignal** — correlates the merge result and fans out to `conclude` and back to `speculate`.
19+
- **merge-signal** — correlates the merge result and fans out to `conclude` and back to `speculate`.
1820
- **conclude** — maps the terminal batch state to the request states.
19-
- **log**persists gateway-owned request-log events published by the orchestrator.
21+
- **submitqueue-hook**dispatches lifecycle hook events to configured integrations.
2022
- **DLQ reconcilers** — one per primary consumed topic, driving stuck requests/batches to a conservative terminal `failed` state.
2123

22-
See [doc/rfc/submitqueue/workflow.md](../../doc/rfc/submitqueue/workflow.md) for the full pipeline diagram and ownership rules.
24+
The orchestrator publishes request-log entries to `log`, but does not consume or persist them; the gateway owns that stage. It also publishes full cross-service requests to Runway's merge-conflict-check and merge topics.

0 commit comments

Comments
 (0)