feat(store): migrate messages + topics + delivery to sqlc (T4) - #795
Merged
Conversation
|
Compass engineering docs preview: https://compass-repo-rig-3034-sqlc-t-qu7x.compass-eng-docs.pages.dev Deployed from |
rigel-mintaka
force-pushed
the
compass-repo/rig-3034-sqlc-t4-messages
branch
from
August 31, 2026 16:52
e49ae3f to
8c39f8b
Compare
Base automatically changed from
compass-repo/rig-3034-sqlc-t3-channels
to
main
August 31, 2026 17:25
Migrate the message, topic, and delivery-consumer store domains from inline pgx SQL to sqlc-generated queries (RIG-3034 T4, design record §T4). The hand-written Store methods keep their exact exported signatures and wrap the generated calls; error wrapping, domain-type mapping, tx orchestration, the D9 not-found/forbidden merge, and the D2 seed self-guard all stay hand-written. Files migrated (queries/*.sql compiled into internal/store/db/): - messages.go (16 sites): post-policy read, insert (:one, ON CONFLICT dedup signalled via errMessageInsertConflict), topic get-or-create, list/search (:many), ask-containment find (FOR UPDATE), authored update (:one RETURNING), block update (:execrows), dedup read. - topics.go (9 sites): list, resolve-for-update, rename/merge resolution loop, archive, get. - delivery_cursors.go (15 sites): seed (per-agent + per-channel), owed-mention record/read/clear (:execrows)/count, mention-routed mark + recovery scan, AckDelivery resolve/load(FOR UPDATE)/self-authored/advance, undelivered sweep, in-sweep-set. - delivery_reads.go (7 sites): subscribed agents, channel agent members, is-agent, message-by-id, message-channel, topic/channel names, sweep channels. - presence_reads.go (2 sites): open-ask JSONB path probe, shared-channel EXISTS. Const-hoisted, never in the gate allowlist — no entry to remove. Each migrated statement is semantically identical to the inline SQL it replaces; RowsAffected-branching sites use :execrows. scanMessages/scanTopics/ scanMessagesByChannel and the now-dead execer interface are removed in favor of generated-row mappers (messageFromParts, topicFromRow); the fail-loud decode error on a malformed block set is preserved. applyAskAnswer takes a nil-guard on its *Message parameter: messageFromParts returns a value Message flowing in via &msg, and the module's nilaway gate (now gating, not advisory) cannot prove the pointer non-nil across the call, so it reports a potential nil-panic at the msg.Blocks deref. The guard is the clean suppression (nilaway ignores //nolint); the branch is a defensive invariant — the sole caller passes the address of a freshly-built local — so the gating nilaway job is its regression guard. inline-sql-gate allowlist: 21 -> 17 (drop messages.go, topics.go, delivery_cursors.go, delivery_reads.go). Verified: go build ./... clean; go test ./internal/store/ ok; full pgtest lane (podman postgres:16) ok; golangci-lint 0 issues; sqlc generate drift-clean; GATE_ROOT=. bun tools/inline-sql-gate/index.ts exits 0 (allowlist 17). Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-repo/rig-3034-sqlc-t4-messages
branch
from
August 31, 2026 19:30
8c39f8b to
c84f42a
Compare
This was referenced Aug 31, 2026
mattwilkinsonn
approved these changes
Aug 31, 2026
rigel-mintaka
added a commit
that referenced
this pull request
Aug 31, 2026
…te identifier-passed SQL (RIG-3034) Collapses the remaining sqlc-adoption slices (T5, T6, T7) into one change atop the merged T4 (#795). Every store domain now uses sqlc-generated typed queries, the inline-sql-gate allowlist is down to its two permanent escape hatches, `sqlc vet` (db-prepare) runs against a live schema in CI, and the gate now catches identifier-passed SQL, not just string literals. Method signatures are unchanged throughout; only bodies now call generated queries via s.q (pool) or s.q.WithTx(tx) / db.New(tx). Error wrapping, domain-type mapping, RowsAffected branching, uint64<->int64 narrowing, flush orchestration, and tx isolation stay hand-written. ## Agent domains (was T5) Five agent-domain files migrated (queries/<domain>.sql): - agent_sessions.go → 3 queries - agent_transcripts.go → 13 distinct queries backing 16 call sites (SessionTranscript + SafetyValveSegments each serve a pool method and the resume-snapshot tx; InsertTranscriptEntry is :execrows for ON CONFLICT dedup). - agent_activity.go → 2 queries - agent_config.go → 3 queries - agent_placements.go → 5 queries COALESCE(MAX/SUM(...),0) reads carry an explicit ::BIGINT cast so sqlc types them int64 (mirrors messages.sql MessagesHeadSeq). Bare-column references visible in both outer and MAX-subquery scope are table-aliased (te/cp/e) — pure disambiguation, semantically identical. Removed dead scanTranscriptRows/ scanSegmentRows helpers and the remarkSafetyValveSQL const. ## Remaining domains (was T6) Every remaining domain file migrated, leaving the allowlist at only the two permanent entries (store.go migration runner, pgshare.go test harness): - authz.go: 3 new queries; dead querier interface removed from store.go; helpers take db.DBTX (pool and pgx.Tx both satisfy it). - tokens.go: 4 (RevokeToken :execrows; revoked_at IS NOT NULL cast ::boolean). - secrets.go: 3 (DeleteSecret :execrows). - issues.go: 4 (UpsertIssueForgeFields CTE, SetIssueState :execrows); scanIssue/ scanRow → issueFromColumns. - forge_authored.go: 4. - forge_cursors.go: 7 (StoreForgeRepoWatermark, SetForgeRepoSubscriptionEnabled :execrows). - forge_subscriptions.go: 9 (AdvanceForgeDeliveredRevision :execrows). derefString removed. - tenant.go: 2. - linear_sessions.go: 2 (UpsertLinearAgentSession :execrows). scanLinearAgentSession removed. - dm.go: residue file (added post-record); 8 queries covering get-or-create, the channel resolution/insert loop, the advisory lock, the R3 verify-reconcile belt, and the reserved-group discriminator. Non-obvious mappings: interface{} columns forced concrete via casts (tokens revoked ::boolean; forge_subscriptions coord_number ::BIGINT, has_cursor ::boolean; SubscribersForArtifact openedEvent ::boolean); GC subquery columns table-qualified to resolve an ambiguous-column analyzer failure; constant enum→int16 conversions carry no nolint (compile-time checked), only variable narrowings do. ## Vet in CI + gate promotion (was T7) sqlc.yaml: add `database.uri: ${SQLC_DATABASE_URL}` + `rules: [sqlc/db-prepare]` so `sqlc vet` PREPAREs every generated query against a live Postgres. Set `analyzer.database: false` so `sqlc generate` and the DB-free sqlc-drift gate keep working without a database — only db-prepare (vet) uses database.uri. Without this, the database stanza would force generate to require a live DB and break the '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; 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. NOT added to the moon `ci` deps: that battery realizes no service. ci.yml: run `moon run compass-go:sqlc-vet --force` 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 (Matt's Option-B ruling on the CI fork: the spec assumed the ci-aggregate job had the service, but the service lives only on the pgtest job). A phase-two nixpkgs-tools step puts psql + sqlc on PATH. `--force` because the pgtest job checks out shallow (no fetch-depth: 0); a bare `moon run` resolves the PR base branch for affected/cache-hash and dies git-128 in a shallow clone, and sqlc-vet must always run here regardless of what the diff touched. inline-sql-gate: promote identifier-passed SQL from advisory to GATED. Beyond string-literal SQL at any receiver, a bare-identifier 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; a call/composite/concat expression 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). Fail-closed stale-entry ratchet intact; +7 detection tests. RIG-3034 Co-authored-by: Matt Wilkinson <matt@rigel.build>
mattwilkinsonn
added a commit
that referenced
this pull request
Aug 31, 2026
…te identifier-passed SQL (RIG-3034) (#812) Collapses the remaining sqlc-adoption slices (T5, T6, T7) into one change atop the merged T4 (#795). Every store domain now uses sqlc-generated typed queries, the inline-sql-gate allowlist is down to its two permanent escape hatches, `sqlc vet` (db-prepare) runs against a live schema in CI, and the gate now catches identifier-passed SQL, not just string literals. Method signatures are unchanged throughout; only bodies now call generated queries via s.q (pool) or s.q.WithTx(tx) / db.New(tx). Error wrapping, domain-type mapping, RowsAffected branching, uint64<->int64 narrowing, flush orchestration, and tx isolation stay hand-written. ## Agent domains (was T5) Five agent-domain files migrated (queries/<domain>.sql): - agent_sessions.go → 3 queries - agent_transcripts.go → 13 distinct queries backing 16 call sites (SessionTranscript + SafetyValveSegments each serve a pool method and the resume-snapshot tx; InsertTranscriptEntry is :execrows for ON CONFLICT dedup). - agent_activity.go → 2 queries - agent_config.go → 3 queries - agent_placements.go → 5 queries COALESCE(MAX/SUM(...),0) reads carry an explicit ::BIGINT cast so sqlc types them int64 (mirrors messages.sql MessagesHeadSeq). Bare-column references visible in both outer and MAX-subquery scope are table-aliased (te/cp/e) — pure disambiguation, semantically identical. Removed dead scanTranscriptRows/ scanSegmentRows helpers and the remarkSafetyValveSQL const. ## Remaining domains (was T6) Every remaining domain file migrated, leaving the allowlist at only the two permanent entries (store.go migration runner, pgshare.go test harness): - authz.go: 3 new queries; dead querier interface removed from store.go; helpers take db.DBTX (pool and pgx.Tx both satisfy it). - tokens.go: 4 (RevokeToken :execrows; revoked_at IS NOT NULL cast ::boolean). - secrets.go: 3 (DeleteSecret :execrows). - issues.go: 4 (UpsertIssueForgeFields CTE, SetIssueState :execrows); scanIssue/ scanRow → issueFromColumns. - forge_authored.go: 4. - forge_cursors.go: 7 (StoreForgeRepoWatermark, SetForgeRepoSubscriptionEnabled :execrows). - forge_subscriptions.go: 9 (AdvanceForgeDeliveredRevision :execrows). derefString removed. - tenant.go: 2. - linear_sessions.go: 2 (UpsertLinearAgentSession :execrows). scanLinearAgentSession removed. - dm.go: residue file (added post-record); 8 queries covering get-or-create, the channel resolution/insert loop, the advisory lock, the R3 verify-reconcile belt, and the reserved-group discriminator. Non-obvious mappings: interface{} columns forced concrete via casts (tokens revoked ::boolean; forge_subscriptions coord_number ::BIGINT, has_cursor ::boolean; SubscribersForArtifact openedEvent ::boolean); GC subquery columns table-qualified to resolve an ambiguous-column analyzer failure; constant enum→int16 conversions carry no nolint (compile-time checked), only variable narrowings do. ## Vet in CI + gate promotion (was T7) sqlc.yaml: add `database.uri: ${SQLC_DATABASE_URL}` + `rules: [sqlc/db-prepare]` so `sqlc vet` PREPAREs every generated query against a live Postgres. Set `analyzer.database: false` so `sqlc generate` and the DB-free sqlc-drift gate keep working without a database — only db-prepare (vet) uses database.uri. Without this, the database stanza would force generate to require a live DB and break the '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; 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. NOT added to the moon `ci` deps: that battery realizes no service. ci.yml: run `moon run compass-go:sqlc-vet --force` 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 (Matt's Option-B ruling on the CI fork: the spec assumed the ci-aggregate job had the service, but the service lives only on the pgtest job). A phase-two nixpkgs-tools step puts psql + sqlc on PATH. `--force` because the pgtest job checks out shallow (no fetch-depth: 0); a bare `moon run` resolves the PR base branch for affected/cache-hash and dies git-128 in a shallow clone, and sqlc-vet must always run here regardless of what the diff touched. inline-sql-gate: promote identifier-passed SQL from advisory to GATED. Beyond string-literal SQL at any receiver, a bare-identifier 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; a call/composite/concat expression 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). Fail-closed stale-entry ratchet intact; +7 detection tests. RIG-3034 Co-authored-by: Matt Wilkinson <matt@rigel.build>
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 4 PRs:
mainMigrate the message, topic, and delivery-consumer store domains from inline
pgx SQL to sqlc-generated queries (RIG-3034 T4, design record §T4). The
hand-written Store methods keep their exact exported signatures and wrap the
generated calls; error wrapping, domain-type mapping, tx orchestration, the
D9 not-found/forbidden merge, and the D2 seed self-guard all stay
hand-written.
Files migrated (queries/*.sql compiled into internal/store/db/):
signalled via errMessageInsertConflict), topic get-or-create, list/search
(:many), ask-containment find (FOR UPDATE), authored update (:one RETURNING),
block update (:execrows), dedup read.
archive, get.
record/read/clear (:execrows)/count, mention-routed mark + recovery scan,
AckDelivery resolve/load(FOR UPDATE)/self-authored/advance, undelivered
sweep, in-sweep-set.
is-agent, message-by-id, message-channel, topic/channel names, sweep
channels.
EXISTS. Const-hoisted, never in the gate allowlist — no entry to remove.
Each migrated statement is semantically identical to the inline SQL it
replaces; RowsAffected-branching sites use :execrows. scanMessages/scanTopics/
scanMessagesByChannel and the now-dead execer interface are removed in favor of
generated-row mappers (messageFromParts, topicFromRow); the fail-loud decode
error on a malformed block set is preserved.
inline-sql-gate allowlist: 21 -> 17 (drop messages.go, topics.go,
delivery_cursors.go, delivery_reads.go).
Verified: go build ./... clean; go test ./internal/store/ ok; full pgtest lane
(podman postgres:16) ok; golangci-lint 0 issues; sqlc generate drift-clean;
GATE_ROOT=. bun tools/inline-sql-gate/index.ts exits 0 (allowlist 17).
Co-authored-by: Matt Wilkinson matt@rigel.build