feat(store): sqlc vet in CI + promote inline-sql-gate to gate identifier-passed SQL (T7) - #809
Closed
rigel-mintaka wants to merge 1 commit into
Conversation
…ier-passed SQL (T7)
Final slice of the sqlc adoption (RIG-3034). Adds the strongest sqlc check —
db-prepare against a live schema — to CI, and closes the inline-SQL gate's
last known gap now that every store domain has migrated.
sqlc.yaml: add `database.uri: ${SQLC_DATABASE_URL}` + `rules: [sqlc/db-prepare]`
so `sqlc vet` PREPAREs every generated query against a live Postgres. Also set
`analyzer.database: false` so `sqlc generate` (and the DB-free sqlc-drift gate)
keep working without a database — db-prepare (vet) still uses database.uri.
Without this, the database stanza would force generate to require a live DB and
break the design's 'sqlc-drift is fully local, no DB' invariant.
moon.yml: add a `sqlc-vet` task (modeled on sqlc-drift) — DSN-gated skip when
neither COMPASS_TEST_DATABASE_DSN nor SQLC_DATABASE_URL is set (dev-box posture,
mirroring the pgtest DSN-gated skip); otherwise it provisions a uniquely-named
throwaway DB (sqlcvet_$$) on the service, applies internal/store/migrations/*.sql
via psql, points SQLC_DATABASE_URL at it, runs `sqlc vet`, and drops the DB on a
cleanup trap. go/cmd/compass-postgres is deliberately NOT reused (it is the
stack supervisor's process-managed instance; the CI service DB is simpler and
already provisioned). sqlc-vet is NOT added to the moon `ci` deps: the moon
battery realizes no service.
ci.yml: run `moon run compass-go:sqlc-vet` in the existing `pgtest` job, which
already carries the postgres:16-alpine service + COMPASS_TEST_DATABASE_DSN. This
is where db-prepare runs live — the same peel-into-a-service-job pattern as
pgtest/microvm/forge/gtk4 (ci.yml: 'the moon battery has no business realizing
an environment'). A phase-two nixpkgs-tools step is added to that job so psql +
sqlc are on PATH. This wiring (Option B) was Matt's ruling on the CI fork: the
spec assumed the ci-aggregate job had the service, but it does not — the service
lives only on the pgtest job.
inline-sql-gate: promote identifier-passed SQL from advisory to GATED. In
addition to string-literal SQL at any receiver, a bare-identifier / simple-
selector SQL slot (`q`, `ddl`, `m.sql`) is now flagged when the call's receiver
is a pgx pool/tx/conn handle (last segment in {pool, tx, conn, c}). The
receiver scope is the load-bearing guard against runtime/compute false positives
(r.runtime.Exec(ctx, id, spec) / g.client.Exec(ctx, req) are not queries). Only
the SQL slot (arg after ctx) is tested, never params; a call/composite/concat
expression is not a hoisted-SQL name and is left alone. Stays GREEN on the
migrated tree: the only remaining identifier-passed sites are conn.Exec(ctx,ddl)
/ tx.Exec(ctx,m.sql) in the permanently-allowlisted store.go (allowlist=2, no
stale entry). Header + ALLOWLIST notes updated; fail-closed stale-entry ratchet
intact.
Tests: add identifier-passed detection cases to index.test.ts — bare-identifier
at pool/conn/tx receivers IS flagged (incl. the store.go conn.Exec(ctx,ddl) and
tx.Exec(ctx,m.sql) shapes), non-pgx runtime.Exec is NOT flagged, a call-valued
SQL slot is NOT flagged, SQL-slot-only (params never double-flag), and the
existing string-literal detection still works. 28/28 pass.
Open item (travels with the code): db-prepare runs live in the pgtest CI job per
Matt's Option-B ruling; it is not in the moon ci battery because that job has no
Postgres service.
RIG-3034
Co-authored-by: Matt Wilkinson <matt@rigel.build>
This was referenced Aug 31, 2026
|
Compass engineering docs preview: https://compass-repo-rig-3034-sqlc-t-eizk.compass-eng-docs.pages.dev Deployed from Changed pages: |
Contributor
Author
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.
This PR is part of a stack containing 4 PRs:
mainFinal slice of the sqlc adoption (RIG-3034). Adds the strongest sqlc check —
db-prepare against a live schema — to CI, and closes the inline-SQL gate's
last known gap now that every store domain has migrated.
sqlc.yaml: add
database.uri: ${SQLC_DATABASE_URL}+rules: [sqlc/db-prepare]so
sqlc vetPREPAREs every generated query against a live Postgres. Also setanalyzer.database: falsesosqlc generate(and the DB-free sqlc-drift gate)keep working without a database — db-prepare (vet) still uses database.uri.
Without this, the database stanza would force generate to require a live DB and
break the design's 'sqlc-drift is fully local, no DB' invariant.
moon.yml: add a
sqlc-vettask (modeled on sqlc-drift) — DSN-gated skip whenneither COMPASS_TEST_DATABASE_DSN nor SQLC_DATABASE_URL is set (dev-box posture,
mirroring the pgtest DSN-gated skip); otherwise it provisions a uniquely-named
throwaway DB (sqlcvet_$$) on the service, applies internal/store/migrations/*.sql
via psql, points SQLC_DATABASE_URL at it, runs
sqlc vet, and drops the DB on acleanup trap. go/cmd/compass-postgres is deliberately NOT reused (it is the
stack supervisor's process-managed instance; the CI service DB is simpler and
already provisioned). sqlc-vet is NOT added to the moon
cideps: the moonbattery realizes no service.
ci.yml: run
moon run compass-go:sqlc-vetin the existingpgtestjob, whichalready carries the postgres:16-alpine service + COMPASS_TEST_DATABASE_DSN. This
is where db-prepare runs live — the same peel-into-a-service-job pattern as
pgtest/microvm/forge/gtk4 (ci.yml: 'the moon battery has no business realizing
an environment'). A phase-two nixpkgs-tools step is added to that job so psql +
sqlc are on PATH. This wiring (Option B) was Matt's ruling on the CI fork: the
spec assumed the ci-aggregate job had the service, but it does not — the service
lives only on the pgtest job.
inline-sql-gate: promote identifier-passed SQL from advisory to GATED. In
addition to string-literal SQL at any receiver, a bare-identifier / simple-
selector SQL slot (
q,ddl,m.sql) is now flagged when the call's receiveris a pgx pool/tx/conn handle (last segment in {pool, tx, conn, c}). The
receiver scope is the load-bearing guard against runtime/compute false positives
(r.runtime.Exec(ctx, id, spec) / g.client.Exec(ctx, req) are not queries). Only
the SQL slot (arg after ctx) is tested, never params; a call/composite/concat
expression is not a hoisted-SQL name and is left alone. Stays GREEN on the
migrated tree: the only remaining identifier-passed sites are conn.Exec(ctx,ddl)
/ tx.Exec(ctx,m.sql) in the permanently-allowlisted store.go (allowlist=2, no
stale entry). Header + ALLOWLIST notes updated; fail-closed stale-entry ratchet
intact.
Tests: add identifier-passed detection cases to index.test.ts — bare-identifier
at pool/conn/tx receivers IS flagged (incl. the store.go conn.Exec(ctx,ddl) and
tx.Exec(ctx,m.sql) shapes), non-pgx runtime.Exec is NOT flagged, a call-valued
SQL slot is NOT flagged, SQL-slot-only (params never double-flag), and the
existing string-literal detection still works. 28/28 pass.
Open item (travels with the code): db-prepare runs live in the pgtest CI job per
Matt's Option-B ruling; it is not in the moon ci battery because that job has no
Postgres service.
RIG-3034
Co-authored-by: Matt Wilkinson matt@rigel.build