docs(rfc): extension contract — identity in, resolve internally#214
Draft
behinddwalls wants to merge 2 commits into
Draft
docs(rfc): extension contract — identity in, resolve internally#214behinddwalls wants to merge 2 commits into
behinddwalls wants to merge 2 commits into
Conversation
This was referenced Jun 8, 2026
behinddwalls
force-pushed
the
preetam/rethink-extension-entity-contracts
branch
from
4a9a93c to
8620ecf
Compare
behinddwalls
force-pushed
the
preetam/request-log-ownership-doc
branch
from
4ab6db2 to
aca149d
Compare
behinddwalls
force-pushed
the
preetam/rethink-extension-entity-contracts
branch
from
8620ecf to
f13a693
Compare
behinddwalls
force-pushed
the
preetam/rethink-extension-entity-contracts
branch
from
f13a693 to
bf4ad19
Compare
behinddwalls
force-pushed
the
preetam/request-log-ownership-doc
branch
from
aca149d to
1a51248
Compare
behinddwalls
added a commit
that referenced
this pull request
Jun 8, 2026
## Summary ### Why? The request log had two persistence paths: the gateway wrote some entries directly, while the orchestrator ran the `log`-topic consumer that wrote all downstream entries to storage. Having the orchestrator persist the request log blurs ownership — the orchestrator is a pipeline that should only emit events, not own the request-log table. Concentrating all writes in the gateway gives a single owner for the request log and keeps the orchestrator free of request-log storage writes. ### What? The request-log persistence consumer moves from the orchestrator to the gateway: - Move `submitqueue/orchestrator/controller/log/` → `submitqueue/gateway/controller/log/` (importpath, doc comment, and default consumer group `orchestrator-log` → `gateway-log`). Logic is unchanged. - Orchestrator: `TopicKeyLog` becomes publish-only (subscription dropped), the log controller registration and import are removed, controller count 11 → 10. It still publishes via `submitqueue/core/request.PublishLog`. - Gateway: builds a consumer (generic + mysql classifiers), registers the moved log controller on `TopicKeyLog` with a subscription (group `gateway-log`), starts it, and drains it with `Stop(30000)` on shutdown — preserving the 128+SIGTERM graceful-exit contract. - Add `HOSTNAME=gateway-dev` to both gateway compose files for a stable subscriber name; update the workflow RFC and gateway README. - Tests: add a gateway integration test that publishes to the log topic (as the orchestrator does) and asserts the gateway consumer persists it, and an e2e test that lands a request and asserts Status advances to `started` — exercising the publish→consume→persist path across both services. The gateway keeps its two synchronous direct writes (`accepted` on Land, `cancelling` on Cancel) for read-your-write visibility at RPC return. Both are gateway writes, so the invariant holds: only the gateway persists the request log; the orchestrator only publishes. This works because gateway and orchestrator already share the same queue and app databases. ## Test Plan - ✅ `bazel build` of both servers + the moved package - ✅ `make test` — unit tests pass (incl. the moved log_test) - ✅ `make check-gazelle`, `make check-tidy`, `make lint` (fmt + license) - ✅ `make integration-test-submitqueue-gateway` — new `TestRequestLogConsumer` verifies the gateway consumer persists a log entry published to the log topic - ✅ `make e2e-test` — new `TestLandRequest_PersistsStartedLogViaGatewayConsumer` verifies an orchestrator-published `started` log is persisted by the gateway and readable via Status; both services still exit 128+SIGTERM on shutdown ## Issues ## Stack 1. @ #205 1. #213 1. #214 --------- Co-authored-by: Oz <oz-agent@warp.dev>
behinddwalls
added a commit
that referenced
this pull request
Jun 8, 2026
## Summary ### Why? Issue #211 (follow-up from PR #205) asks for a single place that records the submitqueue topology at a high level: which service owns its data and how the two services communicate. The workflow RFC already covers the cross-queue flow, so ownership belongs alongside it. ### What? Append an "Ownership by service" section to doc/rfc/submitqueue/workflow.md, described at a conceptual level rather than enumerating individual tables and topics: - Gateway — RPC entry point and owner of the request log; the only service that reads or writes that record. - Orchestrator — runs the pipeline and owns its working state (requests, batches, builds); the only service that writes it. - Messaging queue — the shared, pluggable infrastructure the two services communicate through, kept in its own database separate from application data. A closing "Request-log ownership invariant" section captures the rule: the orchestrator only emits log events, the gateway is the sole consumer and the only writer of the request log. Documentation only; no code, schema, or proto changes. ## Test Plan - ✅ `make lint` (clean tree) ## Issue Closes #211 ## Issues ## Stack 1. #205 1. @ #213 1. #214 --------- Co-authored-by: Oz <oz-agent@warp.dev>
## Summary ### Why? Extension input granularity is inconsistent across the orchestrator pipeline: `conflict.Analyzer` takes orchestrator identity (`entity.Batch`), while `scorer` / `mergechecker` / `changeprovider` / `buildrunner` / `pusher` take controller-resolved `entity.Change`. The split caps what an extension can do — a real `target_overlap` conflict analyzer and a diff-aware heuristic scorer both cannot be written today, because the data they need is neither in the contract nor resolvable by the extension. ### What? Adds `doc/rfc/submitqueue/extension-contract.md` proposing that decision/action extensions accept thin reference entities at their pipeline-stage granularity (`entity.Request` for request-stage, `entity.Batch` / `[]entity.Batch` for batch-stage) and resolve granular content themselves via narrowly-injected `Factory` dependencies, while `storage` / `changestore` / `queueconfig` stay key/value resolution targets. `conflict.Analyzer` is the baseline. The RFC revises the BuildRunner base/head contract (`build-runner.md`) to pass batches rather than change lists. Also encodes the rule in `CLAUDE.md` so new extensions and signature changes follow it, and links the RFC from the RFC index. Documentation only — no code changes.
behinddwalls
force-pushed
the
preetam/rethink-extension-entity-contracts
branch
from
71a4697 to
c615b22
Compare
Add an "Output" column to the verdict table and a principle stating that an output element self-identifies with its input unit (ChangeInfo by URI, Conflict by BatchID); a wrapper entity is added only to aggregate up to a coarser unit. Five of six return contracts are unchanged. pusher is the exception: its input becomes a list of batches, so its result regroups per batch (BatchID-tagged, per-change commit detail underneath). Atomicity stays all-or-nothing, so a per-batch status is intentionally omitted. Also note entity.BatchChanges is kept as the shared resolver's detailed output rather than a controller-assembled value.
This was referenced Jun 8, 2026
albertywu
approved these changes
Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
Extension input granularity is inconsistent across the orchestrator pipeline:
conflict.Analyzertakes orchestrator identity (entity.Batch), whilescorer/mergechecker/changeprovider/buildrunner/pushertake controller-resolvedentity.Change. The split caps what an extension can do — a realtarget_overlapconflict analyzer and a diff-aware heuristic scorer both cannot be written today, because the data they need is neither in the contract nor resolvable by the extension.What?
Adds
doc/rfc/submitqueue/extension-contract.mdproposing that decision/action extensions accept thin reference entities at their pipeline-stage granularity (entity.Requestfor request-stage,entity.Batch/[]entity.Batchfor batch-stage) and resolve granular content themselves via narrowly-injectedFactorydependencies, whilestorage/changestore/queueconfigstay key/value resolution targets.conflict.Analyzeris the baseline. The RFC revises the BuildRunner base/head contract (build-runner.md) to pass batches rather than change lists.Also encodes the rule in
CLAUDE.mdso new extensions and signature changes follow it, and links the RFC from the RFC index. Documentation only — no code changes.Test Plan
Issues
Stack