refactor(proto): standardize the PR-number wire field on pr_number (RIG-3561) - #1037
Open
rigel-mintaka wants to merge 1 commit into
Open
refactor(proto): standardize the PR-number wire field on pr_number (RIG-3561)#1037rigel-mintaka wants to merge 1 commit into
rigel-mintaka wants to merge 1 commit into
Conversation
This was referenced Sep 8, 2026
rigel-mintaka
added this pull request to stack #1022
September 9, 2026 01:54
|
Compass engineering docs preview: https://compass-forge-rig-3561-pr-nu.compass-eng-docs.pages.dev Deployed from Changed pages: |
rigel-mintaka
force-pushed
the
compass-forge/rig-3561-pr-number
branch
from
September 9, 2026 03:35
4c886d0 to
02bfbc3
Compare
rigel-mintaka
marked this pull request as ready for review
September 9, 2026 03:59
…IG-3561) agent_gateway.proto spelled the PR-addressing field two ways: the three pre-existing PR requests used pull_number while RIG-3331's transition request used pr_number. Converge on pr_number, which names the concept the forge actually uses. The split had already leaked past the wire: the agent tool took a pull_number parameter and mapped it onto the prNumber wire field, so a reader held two names for one value. That mapping is gone. Field numbers are unchanged, so the encoding is identical and nothing has shipped on this wire. The ingest PullNumberResolver seam is a separate internal concept and keeps its name. RIG-3561 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-forge/rig-3561-pr-number
branch
from
September 9, 2026 05:32
02bfbc3 to
ecddb3e
Compare
mattwilkinsonn
approved these changes
Sep 9, 2026
|
❌ This stack could not start testing because there was a merge conflict. See more details here.
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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.
This PR is part of a stack containing 6 PRs:
mainagent_gateway.proto spelled the PR-addressing field two ways: the three
pre-existing PR requests used pull_number while RIG-3331's transition
request used pr_number. Converge on pr_number, which names the concept
the forge actually uses.
The split had already leaked past the wire: the agent tool took a
pull_number parameter and mapped it onto the prNumber wire field, so a
reader held two names for one value. That mapping is gone.
Field numbers are unchanged, so the encoding is identical and nothing
has shipped on this wire. The ingest PullNumberResolver seam is a
separate internal concept and keeps its name.
RIG-3561
Co-authored-by: Matt Wilkinson matt@rigel.build
Review round
One review agent, all lenses: high 0 / medium 1 / low 1. It verified the field numbers are preserved, proved the generated bindings byte-identical to a fresh regen of both lanes, confirmed all four PR-taking tools converged on one name, and found no weakened test and no code survivor of the old spelling.
The medium corrected a call I had made wrong. I had dismissed all three unamended design records as frozen history. Two are:
compass-forge-write-pathandcompass-server-ownership-layershow the field inside an illustrative "proto to add" block, with the authoritative.protoone hop away.compass-agent-forge-toolsis not — it isStatus: Activewith T1/T2 unchecked, and its T2 prescribes "the ten tools (exact schemas above)", where those schemas are the arktype keys an agent must literally emit. Nothing reconciles a reader of that record againstforge.ts: no compiler, no drift gate.Matt ruled: record it as a ledger row rather than rewrite a frozen record in place. DL-350 appended, gate reads 309 rows. Numbered clear of the DL-344..347 range that open PRs #984, #1001, #1029 and #1040 already claim: every open PR reads the same DL-343 max off
mainindependently, so the next id is contended, and #984 claimed DL-344 first.The low is a pre-existing Go coverage gap this diff surfaces but does not widen: no Go test builds a
GetPullRequestRequest, so that one renamed accessor has compile-time proof only. The TS side does cover the field on the wire. Not folded in.Verification
moon ciwith explicitMOON_BASE/MOON_HEAD— 70 actions completed, 0 failed, includingcompass-proto:drift, which is what proves the regenerated bindings match the.proto.go build -tags unix ./...andgo vet -tags 'pgtest unix'clean;go test -tags unix ./server/... ./internal/forge/...pass.design-ledger-gateOK (309 rows, 130 record headers valid); markdownlint 0 errors.pull_numberin prompts or any agent-facing tool description.Scope note
ingest.PullNumberResolver/PullNumberForSHA/CachedPullNumberResolverkeep their names. They resolve acheck_suitehead SHA to its PR number (RIG-2869) — a different concept from the PR-addressing wire field. That accounts for every survivingPullNumberin the tree.Ledger-impact: DL-350 (new row; no existing row flipped)