Skip to content

feat(stovepipe): add the record controller, advancing a queue's last-green bookmark on a successful build - #519

Merged
roychying merged 1 commit into
mainfrom
chenghan.ying/stovepipe-record-controller
Aug 7, 2026
Merged

feat(stovepipe): add the record controller, advancing a queue's last-green bookmark on a successful build#519
roychying merged 1 commit into
mainfrom
chenghan.ying/stovepipe-record-controller

Conversation

@roychying

Copy link
Copy Markdown
Contributor

Why?

buildsignal has been publishing to the record topic already, but nothing consumes it, so the topic accumulates and Queue.LastGreenURI is never set.

The visible consequence is cost: with the bookmark permanently empty, process.deriveBuildStrategy always returns BuildStrategyFull, so every validation is a full-repository build even when the delta is one commit.

What?

New stovepipe/controller/record. It consumes Record{id}, loads the request, and branches on the outcome buildsignal already stamped:

State Behavior
succeeded advance the bookmark
failed, cancelled ack; no queue write
superseded ack (unreachable — buildsignal never publishes it)
anything else non-retryable invariant error

The advance is a CAS retry loop mirroring buildsignal's releaseBuildSlot, guarded by CompareRequestID: a candidate that is not strictly newer is skipped without a write, which also makes redelivery of the same request a no-op.

Also wires the stage into the example server: the record topic previously had no Subscription, and the controller is now registered alongside the others.

One deliberate deviation: greenness is read from request.State rather than a persisted validation fact, because the fact store does not exist yet. This will be resolved in follow up PR.

Test Plan

Issue

@roychying
roychying requested review from a team, behinddwalls and sbalabanov as code owners August 5, 2026 22:15
@roychying
roychying force-pushed the chenghan.ying/stovepipe-record-controller branch from 72579db to 7f28a45 Compare August 5, 2026 23:08
@mnoah1
mnoah1 force-pushed the chenghan.ying/stovepipe-record-controller branch from 7f28a45 to 81357df Compare August 6, 2026 18:51
@roychying
roychying force-pushed the chenghan.ying/stovepipe-record-controller branch from 81357df to 8fd9e71 Compare August 6, 2026 22:28
@roychying
roychying force-pushed the chenghan.ying/stovepipe-record-controller branch from 8fd9e71 to 56f126f Compare August 7, 2026 01:16
@roychying
roychying force-pushed the chenghan.ying/stovepipe-record-controller branch from 56f126f to 9975b8d Compare August 7, 2026 01:19
Base automatically changed from chenghan.ying/stovepipe-queue-last-green-request-id to main August 7, 2026 01:26
@roychying
roychying force-pushed the chenghan.ying/stovepipe-record-controller branch from 9975b8d to e3b26b9 Compare August 7, 2026 01:26
@roychying
roychying added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit 94073b7 Aug 7, 2026
15 checks passed
@behinddwalls
behinddwalls deleted the chenghan.ying/stovepipe-record-controller branch August 7, 2026 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants