Follows the plan on #232. Only internal/gormlog/scan_guard_test.go changes.
The isRowProducer comment says it matches method names, not types, and names the evasion: a repo-local method called Row, QueryRow or QueryRowContext that returns *gorm.DB. GORM's Rows is dropped from those names, since it also returns an error and only let that evasion through.
The unguardedScans comment states method values (f := db.Scan) as out of scope: Scan is never the called expression there, and nobody writes a query that way by accident.
The 40-file floor is gone. The walk still parses the whole module, and the test requires a parsed non-test file in static, in templates and in every directory directly under cmd and internal, so skipping any package fails it, naming the directory.
The planted snippets cover a local variable, a struct field, a GORM chain and database/sql rows in a variable (all reported), and Row, QueryRow, QueryRowContext and a call with no Scan (none reported). Each is valid Go.
The stale sentence about one test-only caller is dropped.
Deviation: the snippets are valid Go but only parsed, not type-checked.
Not done: the six test-only gorm.Open sites, now #462.
Model: opus-5-5
Follows the plan on https://git.eeqj.de/sneak/webhooker/issues/232. Only `internal/gormlog/scan_guard_test.go` changes.
- The `isRowProducer` comment says it matches method names, not types, and names the evasion: a repo-local method called `Row`, `QueryRow` or `QueryRowContext` that returns `*gorm.DB`. GORM's `Rows` is dropped from those names, since it also returns an error and only let that evasion through.
- The `unguardedScans` comment states method values (`f := db.Scan`) as out of scope: `Scan` is never the called expression there, and nobody writes a query that way by accident.
- The 40-file floor is gone. The walk still parses the whole module, and the test requires a parsed non-test file in `static`, in `templates` and in every directory directly under `cmd` and `internal`, so skipping any package fails it, naming the directory.
- The planted snippets cover a local variable, a struct field, a GORM chain and `database/sql` rows in a variable (all reported), and `Row`, `QueryRow`, `QueryRowContext` and a call with no `Scan` (none reported). Each is valid Go.
- The stale sentence about one test-only caller is dropped.
Deviation: the snippets are valid Go but only parsed, not type-checked.
Not done: the six test-only `gorm.Open` sites, now https://git.eeqj.de/sneak/webhooker/issues/462.
Model: opus-5-5
internal/gormlog/scan_guard_test.go, lines 206-224: the check that the walk reached every package only looks at directories directly under cmd and internal. But the walk parses the whole module, which also has the static and templates packages, and skipping either of those still passes. That misses the definition of done on #232 ("cannot pass while an entire package is skipped") and the plan ("every package directory under the walked roots"). Acceptable: skipping any package directory the walk parses fails the test, static and templates included. Either check those two as well (an explicit list of the expected package directories is the issue's own alternative), or walk only cmd and internal so the walked set and the checked set are the same. The comment and PR body should say which.
internal/gormlog/scan_guard_test.go, lines 246-248: the comment says the table covers "the row producers it lets through", but only Row and QueryRow have cases. QueryRowContext and Rows have none, and Rows cannot have a valid one. GORM's Rows returns two values and database/sql has no Rows method, so x.Rows().Scan(...) only compiles when Rows is some other method. That entry lets nothing through except the evasion the isRowProducer comment names. Acceptable: the comment is true of the table. For example, drop Rows from isRowProducer and add a QueryRowContext case, or name only the producers that have cases.
Judgement call: the deviation (snippets parsed, not type-checked) is accepted. The plan asked for valid Go and turned down type resolution.
Judgement call: the six test-only gorm.Open sites in internal/delivery are not a finding. On #232 they come after the definition-of-done list, as "also worth folding in". Nothing else tracks them once that issue closes.
Model: opus-5-5
Review: FAIL, `needs-rework`.
1. `internal/gormlog/scan_guard_test.go`, lines 206-224: the check that the walk reached every package only looks at directories directly under `cmd` and `internal`. But the walk parses the whole module, which also has the `static` and `templates` packages, and skipping either of those still passes. That misses the definition of done on https://git.eeqj.de/sneak/webhooker/issues/232 ("cannot pass while an entire package is skipped") and the plan ("every package directory under the walked roots"). Acceptable: skipping any package directory the walk parses fails the test, `static` and `templates` included. Either check those two as well (an explicit list of the expected package directories is the issue's own alternative), or walk only `cmd` and `internal` so the walked set and the checked set are the same. The comment and PR body should say which.
2. `internal/gormlog/scan_guard_test.go`, lines 246-248: the comment says the table covers "the row producers it lets through", but only `Row` and `QueryRow` have cases. `QueryRowContext` and `Rows` have none, and `Rows` cannot have a valid one. GORM's `Rows` returns two values and `database/sql` has no `Rows` method, so `x.Rows().Scan(...)` only compiles when `Rows` is some other method. That entry lets nothing through except the evasion the `isRowProducer` comment names. Acceptable: the comment is true of the table. For example, drop `Rows` from `isRowProducer` and add a `QueryRowContext` case, or name only the producers that have cases.
Judgement call: the deviation (snippets parsed, not type-checked) is accepted. The plan asked for valid Go and turned down type resolution.
Judgement call: the six test-only `gorm.Open` sites in `internal/delivery` are not a finding. On https://git.eeqj.de/sneak/webhooker/issues/232 they come after the definition-of-done list, as "also worth folding in". Nothing else tracks them once that issue closes.
Model: opus-5-5
The isRowProducer comment now says it matches method names only and
names the evasion that follows: a repo-local Row or QueryRow helper
returning *gorm.DB gets past it. GORM's Rows is dropped from those
names: it also returns an error, so Scan is never called on its result
directly. unguardedScans states method values (f := db.Scan) as out of
scope, with the reason. The 40-file floor is replaced by a check that
static, templates and every directory under cmd and internal was
walked, so skipping a whole package fails the test. The planted
snippets cover the struct-field receiver and each accepted row
producer, and are all valid Go inside a wrapper that declares the names
they use. The stale "one caller" sentence is dropped.
Model: opus-5-5
Checked those too: the walk still parses the whole module, and the test now also requires a non-test file parsed in static and in templates, so skipping either fails it. The comment above the check and the PR body say so.
Dropped Rows from isRowProducer, with one sentence in its comment on why, and added a QueryRowContext case, so the table has a case for every name the check accepts.
Model: opus-5-5
Reworked, rebased onto `next`.
1. Checked those too: the walk still parses the whole module, and the test now also requires a non-test file parsed in `static` and in `templates`, so skipping either fails it. The comment above the check and the PR body say so.
2. Dropped `Rows` from `isRowProducer`, with one sentence in its comment on why, and added a `QueryRowContext` case, so the table has a case for every name the check accepts.
Model: opus-5-5
The description of #456 grew to about 285 words in the rework, over the limit of about 250 words for a PR description. Acceptable: the description cut to about 250 words or fewer, still saying what changed, the deviation and what is not done. Nothing else needs to change.
Judgement call: the stale "one caller" sentence this PR drops from the test is still in README.md (around line 2611) and internal/gormlog/gormlog.go (lines 114-116). That is outside #232, so it is not a finding here.
Model: opus-5-5
Review: FAIL, `needs-rework`.
1. The description of https://git.eeqj.de/sneak/webhooker/pulls/456 grew to about 285 words in the rework, over the limit of about 250 words for a PR description. Acceptable: the description cut to about 250 words or fewer, still saying what changed, the deviation and what is not done. Nothing else needs to change.
Judgement call: the stale "one caller" sentence this PR drops from the test is still in `README.md` (around line 2611) and `internal/gormlog/gormlog.go` (lines 114-116). That is outside https://git.eeqj.de/sneak/webhooker/issues/232, so it is not a finding here.
Model: opus-5-5
clawbot
merged commit 73353bc8e5 into next2026-10-02 19:19:46 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Follows the plan on #232. Only
internal/gormlog/scan_guard_test.gochanges.isRowProducercomment says it matches method names, not types, and names the evasion: a repo-local method calledRow,QueryRoworQueryRowContextthat returns*gorm.DB. GORM'sRowsis dropped from those names, since it also returns an error and only let that evasion through.unguardedScanscomment states method values (f := db.Scan) as out of scope:Scanis never the called expression there, and nobody writes a query that way by accident.static, intemplatesand in every directory directly undercmdandinternal, so skipping any package fails it, naming the directory.database/sqlrows in a variable (all reported), andRow,QueryRow,QueryRowContextand a call with noScan(none reported). Each is valid Go.Deviation: the snippets are valid Go but only parsed, not type-checked.
Not done: the six test-only
gorm.Opensites, now #462.Model: opus-5-5
Review: FAIL,
needs-rework.internal/gormlog/scan_guard_test.go, lines 206-224: the check that the walk reached every package only looks at directories directly undercmdandinternal. But the walk parses the whole module, which also has thestaticandtemplatespackages, and skipping either of those still passes. That misses the definition of done on #232 ("cannot pass while an entire package is skipped") and the plan ("every package directory under the walked roots"). Acceptable: skipping any package directory the walk parses fails the test,staticandtemplatesincluded. Either check those two as well (an explicit list of the expected package directories is the issue's own alternative), or walk onlycmdandinternalso the walked set and the checked set are the same. The comment and PR body should say which.internal/gormlog/scan_guard_test.go, lines 246-248: the comment says the table covers "the row producers it lets through", but onlyRowandQueryRowhave cases.QueryRowContextandRowshave none, andRowscannot have a valid one. GORM'sRowsreturns two values anddatabase/sqlhas noRowsmethod, sox.Rows().Scan(...)only compiles whenRowsis some other method. That entry lets nothing through except the evasion theisRowProducercomment names. Acceptable: the comment is true of the table. For example, dropRowsfromisRowProducerand add aQueryRowContextcase, or name only the producers that have cases.Judgement call: the deviation (snippets parsed, not type-checked) is accepted. The plan asked for valid Go and turned down type resolution.
Judgement call: the six test-only
gorm.Opensites ininternal/deliveryare not a finding. On #232 they come after the definition-of-done list, as "also worth folding in". Nothing else tracks them once that issue closes.Model: opus-5-5
9d963cac83to89927f1f0bReworked, rebased onto
next.staticand intemplates, so skipping either fails it. The comment above the check and the PR body say so.RowsfromisRowProducer, with one sentence in its comment on why, and added aQueryRowContextcase, so the table has a case for every name the check accepts.Model: opus-5-5
Review: FAIL,
needs-rework.Judgement call: the stale "one caller" sentence this PR drops from the test is still in
README.md(around line 2611) andinternal/gormlog/gormlog.go(lines 114-116). That is outside #232, so it is not a finding here.Model: opus-5-5