fix(ci): generate sqlc drift check out-of-tree (RIG-3591) - #1063
Open
rigel-mintaka wants to merge 2 commits into
Open
fix(ci): generate sqlc drift check out-of-tree (RIG-3591)#1063rigel-mintaka wants to merge 2 commits into
rigel-mintaka wants to merge 2 commits into
Conversation
sqlc-drift ran `sqlc generate` in place, rewriting go/internal/store/db while moon ran build/nilaway/test against that same directory in parallel. A reader could catch a generated file mid-write and fail with `expected 'package', found 'EOF'` on a different random file each run, which aborts the whole push atomically and reads as the submitter's defect. Stage the sqlc inputs into a scratch dir and generate there, so the gate has no shared mutable subject. The drift comparison is unchanged. Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Merging to
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 |
|
Compass engineering docs preview: https://compass-forge-rig-3591-sqlc.compass-eng-docs.pages.dev Deployed from |
…RIG-3591) Review finding: `sqlc generate`'s exit code was discarded, so the gate failed closed only as a side effect — sqlc writing nothing made the comparison path missing, which errored. A broken query therefore reported `internal/store/db is stale` and pointed the operator at a regen that fails the same way. It also left fail-closed depending on sqlc validating every block before writing any, which is observed behaviour of the pinned version, not a contract. `set -e` aborts at the sqlc error instead. Errexit is suspended inside the `if` condition, so the drift comparison is unaffected, and the EXIT trap still fires. Also trims the task comment to the constraint a future editor needs. Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
marked this pull request as ready for review
September 10, 2026 04:13
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
1 participant
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.
Fixes RIG-3591.
Problem
compass-go:sqlc-driftis a dep ofcompass-go:ciand ransqlc generatein place, rewriting
go/internal/store/db/*.goin the working tree. moonschedules it in parallel with
compass-go:build,:nilawayand:test,which compile that same directory — no task declares a
depsedge against it.Readers caught a generated file mid-write:
A different random file each run, load-independent (seen at loadavg 13 through
96). A pre-push hook failure aborts the whole push atomically, so it presented
as the submitter's defect. Four consecutive submits of an unchanged tree failed
this way; the first submit after this fix passed.
Fix
Stage sqlc's inputs into a scratch dir and generate there.
sqlchas nooutput-redirect flag, so relocating the inputs is how the output relocates
(
out:ingo/sqlc.yamlresolves relative to the config's directory). Thetask now performs zero writes into the working tree — it touches
internal/store/dbonly as the read side ofgit diff --no-index, so theconcurrent set becomes readers-only and the race is structurally eliminated
rather than narrowed.
Also closes a pre-existing fail-open
Surfaced by review, and not a symptom of the race. On
mainthe gatesnapshotted the committed tree, regenerated in place, and diffed the two — so
when
sqlcfailed it wrote nothing, the snapshot matched the untouchedtree, and the gate went green. Verified A/B with a stub
sqlcthat exits 1:old script
rc=0, new scriptrc=1.The second commit hardens this properly.
sqlc generate's exit code was stilldiscarded, so the new form failed closed only as a side effect (missing output
dir →
git differrors), and reported a real sqlc error asinternal/store/db is stale, pointing the operator at a regen that fails thesame way.
set -eaborts at the sqlc error instead; errexit is suspended insidethe
ifcondition, so the drift comparison is unaffected and the EXIT trapstill fires.
Verification
internal/store/dbis left unmodified (the point).DriftProbeXyzquery makes the gate fail with areadable diff naming it; restored byte-identical.
SELECT * FROM no_such_table_xyz) → aborts at the real sqlcerror, and no longer prints the misleading
is staleline.moon ci(MOON_BASE=origin/main): 29 actions, 0 failed.Review: 0 high, 1 medium, 4 low. The medium (
set -e) is fixed in the secondcommit. Two lows declined as out of scope — pinning the gate on the
sqlcbinary version (pre-existing, worth its own issue) and cosmetic diff prefixes.