feat(buildrunner): add vendor-agnostic BuildRunner interface + RFC#175
Merged
Conversation
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
2 times, most recently
from
975793b to
293db26
Compare
behinddwalls
changed the title
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
from
2b8dd80 to
293db26
Compare
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
from
293db26 to
9e9cf91
Compare
JamyDev
reviewed
Jun 2, 2026
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
2 times, most recently
from
dc4b4cb to
b6f1414
Compare
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
from
b6f1414 to
518ef64
Compare
Contributor
|
When would the orchestration engine call the synchronous build edit: nvm i see your 4th PR here: #177 |
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
3 times, most recently
from
7d6bf16 to
f612697
Compare
## Summary
### Why?
The build stage needs a vendor-agnostic abstraction for talking to a
Build Runner — one that fits the existing extension family (storage,
queue, conflict) and that does not have to change when the first real
backend lands. Design rationale lives in `doc/rfc/build-runner.md`.
### What?
Introduces the `BuildRunner` contract — three verbs, all keyed by an `entity.BuildID`: `Trigger` submits a build (ordered `base` + `head` change sets plus a free-form metadata map) and returns its ID; `Status` fetches the current `BuildStatus` and runner-defined metadata; `Cancel` requests cancellation. See `extension/buildrunner/build_runner.go` for the signatures.
Highlights:
- `Trigger` takes ordered `base` (assumed-good prefix from dependency
batches) and `head` (changes being verified) — a runner can cache or
short-circuit a prefix it has validated before, and attribute terminal
failures to base vs head via `BuildMetadata`.
- Build identifiers are the typed `entity.BuildID` (a lightweight
`{ID string}` value, also the queue payload envelope) rather than a
bare `string`, so the runner contract and the on-queue messages share
one identifier type and the compiler distinguishes a build ID from
other string IDs.
- `metadata` is a free-form `map[string]string` for caller-supplied
attributes (requester, ticket ID, trace ID) the runner MAY persist or
echo back via `Status`.
- `Trigger` and `Cancel` return promptly; `Status` MAY be lengthy.
- The `BuildStatus` enum is narrowed to `Accepted` / `Running` /
`Succeeded` / `Failed` / `Cancelled` so every backend can map its
native lifecycle without leaking runner-specific stages.
- `BuildMetadata` is added as a free-form map for round-tripping
runner-defined attributes (build URL, duration, SHA, etc.).
Naming: the interface is `BuildRunner` (not `BuildManager` — the
"Manager" suffix doesn't describe what it does, and "Build" alone would
collide cognitively with `entity.Build`). The package is
`extension/buildrunner` to follow the repo convention used by
`mergechecker`, `changeprovider`, `pusher`, `counter`, and `storage`.
Implementation (noop backend, queue `PublishAfter` primitive, controller
wiring, and the poll-driven `buildsignal` loop) lands in the follow-up
PRs stacked on top of this one.
## Test Plan
- ✅ `make test` — entity/build tests pass; mock-only consumers of the
interface compile and pass.
- ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean.
- ✅ `make build` — all targets compile.
behinddwalls
force-pushed
the
preetam/build-mgr-interface
branch
from
f612697 to
4f0d367
Compare
JamyDev
approved these changes
Jun 3, 2026
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary ### Why? Wiring tests for the orchestrator's build stage need a `BuildRunner` implementation, but real backends (BuildKite, Jenkins, etc.) won't land until later. A noop that immediately reports success is also useful as the local default in `make local-start` and as a best-case baseline where every build passes. ### What? Adds `extension/buildrunner/noop` — a `BuildRunner` that does no real work. Every `Trigger` returns a unique `noop-<n>` build ID with no error; every `Status` returns `BuildStatusSucceeded` with no metadata; `Cancel` is a no-op. The atomic counter keeps IDs unique under concurrent use. ## Test Plan - ✅ `make test` — 36 unit tests pass, including the new `extension/buildrunner/noop:noop_test` (Trigger uniqueness, Status, Cancel, interface satisfaction). - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Issues ## Stack 1. #175 1. @ #179 1. #176 1. #177
behinddwalls
added a commit
that referenced
this pull request
Jun 3, 2026
## Summary ### Why? The orchestrator's build poll loop needs to space `Status` calls out without consuming `retry_count` / DLQ retry slots. `Delivery.Nack` does schedule redelivery after a delay, but it overloads `retry_count` — which the operator relies on to flag real failures. The fix is a separate "postpone this work" primitive, distinct from "this delivery failed, try again", so both signals stay meaningful. ### What? Adds `Publisher.PublishAfter(ctx, topic, msg, delayMs)` to the queue extension: a fresh message inserted into the topic, made visible to subscribers only after `delayMs` from now. Distinct from `Delivery.Nack(requeueAfterMillis)`: - `Nack` is "this delivery failed, try again" — it bumps `retry_count` and eventually trips DLQ. - `PublishAfter` is "postpone this work" — `retry_count` resets to 0, DLQ stays available for true failures. SQL backing in `extension/queue/mysql`: - New `visible_after BIGINT UNSIGNED NOT NULL DEFAULT 0` column on `queue_messages`. Default 0 means immediately visible — back-compat for any existing rows. - `messageStore.InsertDelayed(ctx, topic, messages, visibleAfterMs)` is the underlying primitive. `Insert` is now a thin wrapper that passes `visibleAfterMs = 0`. - `FetchByOffset` gains a `nowMs` parameter and an `AND visible_after <= ?` predicate so subscribers skip rows whose delivery is still deferred. - `MoveToDLQ` writes `visible_after = 0` explicitly: any delay on the original message has already been consumed by the time it failed. The first consumer of `PublishAfter` is the orchestrator's poll-driven `buildsignal` loop, which lands in the stacked PR on top of this one. ## Test Plan - ✅ `bazel test //extension/queue/...` — all pass, including new coverage for `PublishAfter` (positive / zero / negative delay, closed-publisher) and for `FetchByOffset` skipping deferred rows via the new `visible_after` predicate. - ✅ `make fmt lint check-tidy check-gazelle check-mocks` — clean. - ✅ `make build` — all targets compile. ## Issues ## Stack 1. #175 1. #179 1. @ #176 1. #177
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
3 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?
The build stage needs a vendor-agnostic abstraction for talking to a
Build Runner — one that fits the existing extension family (storage,
queue, conflict) and that does not have to change when the first real
backend lands. Design rationale lives in
doc/rfc/build-runner.md.What?
Introduces the
BuildRunnercontract — three verbs, all keyed by anentity.BuildID:Triggersubmits a build (orderedbase+headchange sets plus a free-form metadata map) and returns its ID;Statusfetches the currentBuildStatusand runner-defined metadata;Cancelrequests cancellation. Seeextension/buildrunner/build_runner.gofor the signatures.Highlights:
Triggertakes orderedbase(assumed-good prefix from dependencybatches) and
head(changes being verified) — a runner can cache orshort-circuit a prefix it has validated before, and attribute terminal
failures to base vs head via
BuildMetadata.entity.BuildID(a lightweight{ID string}value, also the queue payload envelope) rather than abare
string, so the runner contract and the on-queue messages shareone identifier type and the compiler distinguishes a build ID from
other string IDs.
metadatais a free-formmap[string]stringfor caller-suppliedattributes (requester, ticket ID, trace ID) the runner MAY persist or
echo back via
Status.TriggerandCancelreturn promptly;StatusMAY be lengthy.BuildStatusenum is narrowed toAccepted/Running/Succeeded/Failed/Cancelledso every backend can map itsnative lifecycle without leaking runner-specific stages.
BuildMetadatais added as a free-form map for round-trippingrunner-defined attributes (build URL, duration, SHA, etc.).
Naming: the interface is
BuildRunner(notBuildManager— the"Manager" suffix doesn't describe what it does, and "Build" alone would
collide cognitively with
entity.Build). The package isextension/buildrunnerto follow the repo convention used bymergechecker,changeprovider,pusher,counter, andstorage.Implementation (noop backend, queue
PublishAfterprimitive, controllerwiring, and the poll-driven
buildsignalloop) lands in the follow-upPRs stacked on top of this one.
Test Plan
make test— entity/build tests pass; mock-only consumers of theinterface compile and pass.
make fmt lint check-tidy check-gazelle check-mocks— clean.make build— all targets compile.Stack