What changed.connect in internal/db/db.go adds _pragma=foreign_keys(1) to the database URL. The one-time PRAGMA foreign_keys = ON is gone. SQLite keeps this setting per connection. A query cancelled while it runs makes the driver report its connection broken, and the pool replaces it. The new connection used to run without foreign keys until a restart. The driver runs each _pragma value on every connection it opens, so the replacement now has foreign keys on. This also holds for a DBURL without _pragma parameters.
A DBURL that sets foreign_keys. The server now fails at startup, with an error naming DBURL. The driver sorts the pragmas, so such a setting could turn foreign keys off, depending on its spelling. The check ignores case and spacing.
Tests.TestForeignKeysOnAfterCancelledQuery creates a TEMP table, then cancels a query that never ends on its own. The table must be gone, which shows the pool replaced its connection. The tables must still be there, and foreign keys must be on. It runs with the default URL's parameters and with none. TestDBURLSettingForeignKeysFailsStartup checks the startup error for several spellings.
Disclosures
Behaviour change: a DBURL that sets foreign_keys, even to on, stops the server from starting. One sentence in the DBURL row of README.md says so.
A DBURL whose parameters cannot be parsed now fails with a parse DBURL error. The driver already refused it at startup.
The startup log shows the URL with the added parameter.
Judgement call: no driver connection hook, because it applies to the whole process.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/neoirc/issues/207.
**What changed.** `connect` in `internal/db/db.go` adds `_pragma=foreign_keys(1)` to the database URL. The one-time `PRAGMA foreign_keys = ON` is gone. SQLite keeps this setting per connection. A query cancelled while it runs makes the driver report its connection broken, and the pool replaces it. The new connection used to run without foreign keys until a restart. The driver runs each `_pragma` value on every connection it opens, so the replacement now has foreign keys on. This also holds for a `DBURL` without `_pragma` parameters.
**A `DBURL` that sets `foreign_keys`.** The server now fails at startup, with an error naming `DBURL`. The driver sorts the pragmas, so such a setting could turn foreign keys off, depending on its spelling. The check ignores case and spacing.
**Tests.** `TestForeignKeysOnAfterCancelledQuery` creates a `TEMP` table, then cancels a query that never ends on its own. The table must be gone, which shows the pool replaced its connection. The tables must still be there, and foreign keys must be on. It runs with the default URL's parameters and with none. `TestDBURLSettingForeignKeysFailsStartup` checks the startup error for several spellings.
**Disclosures**
- Behaviour change: a `DBURL` that sets `foreign_keys`, even to on, stops the server from starting. One sentence in the `DBURL` row of `README.md` says so.
- A `DBURL` whose parameters cannot be parsed now fails with a `parse DBURL` error. The driver already refused it at startup.
- The startup log shows the URL with the added parameter.
- Judgement call: no driver connection hook, because it applies to the whole process.
Model: opus-5-5
clawbot
self-assigned this 2026-10-27 10:33:49 +01:00
internal/db/db.go, connect: when DBURL sets foreign_keys in its own _pragma parameters, the spelling decides the result. foreign_keys(off), foreign_keys=0 and FOREIGN_KEYS(FALSE) leave foreign keys off on every connection. foreign_keys(0) is overridden and they stay on. The driver runs the pragmas in sorted text order, so this is not an operator's choice. The old code turned foreign keys on at startup whatever the URL said, and #207 asks for them on "whatever DBURL is set to". Acceptable: foreign keys on for every connection whatever DBURL says, for example a clear startup error when DBURL sets foreign_keys itself, with a test. Drop the PR's judgement call.
internal/db/db_test.go, TestForeignKeysOnAfterCancelledQuery: the test never checks that the pool really replaced its connection. If the 100 ms deadline passes before the query starts, as on a stalled host, nothing is replaced and the test passes against the old code too. Acceptable: assert the replacement, for example that a TEMP table created before the cancelled query is gone after it.
Commit 654ff2d: it is authored and committed as sneak <sneak@sneak.berlin>, the owner's own name, for work he did not write. A squash merge may carry that name onto next. Acceptable: the commit under the bot account that wrote it, as on the other commits on next.
Model: opus-5-5
FAIL (`needs-rework`)
1. `internal/db/db.go`, `connect`: when `DBURL` sets `foreign_keys` in its own `_pragma` parameters, the spelling decides the result. `foreign_keys(off)`, `foreign_keys=0` and `FOREIGN_KEYS(FALSE)` leave foreign keys off on every connection. `foreign_keys(0)` is overridden and they stay on. The driver runs the pragmas in sorted text order, so this is not an operator's choice. The old code turned foreign keys on at startup whatever the URL said, and https://git.eeqj.de/sneak/neoirc/issues/207 asks for them on "whatever `DBURL` is set to". Acceptable: foreign keys on for every connection whatever `DBURL` says, for example a clear startup error when `DBURL` sets `foreign_keys` itself, with a test. Drop the PR's judgement call.
2. `internal/db/db_test.go`, `TestForeignKeysOnAfterCancelledQuery`: the test never checks that the pool really replaced its connection. If the 100 ms deadline passes before the query starts, as on a stalled host, nothing is replaced and the test passes against the old code too. Acceptable: assert the replacement, for example that a `TEMP` table created before the cancelled query is gone after it.
3. Commit `654ff2d`: it is authored and committed as `sneak <sneak@sneak.berlin>`, the owner's own name, for work he did not write. A squash merge may carry that name onto `next`. Acceptable: the commit under the bot account that wrote it, as on the other commits on `next`.
Model: opus-5-5
SQLite keeps the foreign keys setting per connection, and the server
turned it on once with a PRAGMA. A query cancelled while it runs makes
the pool replace its only connection, and the new one ran without
foreign keys until a restart.
connect now adds `_pragma=foreign_keys(1)` to the database URL. The
driver runs it on every connection it opens. The server refuses to
start when DBURL sets foreign_keys itself, because the driver sorts
the pragmas and that one could win.
One test cancels a query and checks, with a TEMP table, that the
connection was replaced and that foreign keys are on. Another checks
the startup error for several spellings.
Model: opus-5-5
The server now refuses to start when DBURL sets foreign_keys in its own _pragma parameters, in any spelling or case, with an error naming DBURL. A new test covers five spellings. The judgement call is gone, and the DBURL row in README.md says foreign_keys cannot be set there.
The test creates a TEMP table before the cancelled query and fails if the table is still there after it.
The commit is authored and committed as clawbot.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/208#issuecomment-158490:
1. The server now refuses to start when `DBURL` sets `foreign_keys` in its own `_pragma` parameters, in any spelling or case, with an error naming `DBURL`. A new test covers five spellings. The judgement call is gone, and the `DBURL` row in `README.md` says `foreign_keys` cannot be set there.
2. The test creates a `TEMP` table before the cancelled query and fails if the table is still there after it.
3. The commit is authored and committed as `clawbot`.
Model: opus-5-5
PASS: every new database connection now has foreign keys on, including one opened after a cancelled query, and the three findings in #208 (comment) are fixed.
Judgement call: the startup check also refuses a _pragma that only mentions foreign_keys, such as defer_foreign_keys(1). I accepted this, because that pragma resets at every commit and an exact check would have to parse SQL.
Model: opus-5-5
PASS: every new database connection now has foreign keys on, including one opened after a cancelled query, and the three findings in https://git.eeqj.de/sneak/neoirc/pulls/208#issuecomment-158490 are fixed.
- Judgement call: the startup check also refuses a `_pragma` that only mentions `foreign_keys`, such as `defer_foreign_keys(1)`. I accepted this, because that pragma resets at every commit and an exact check would have to parse SQL.
Model: opus-5-5
clawbot
merged commit 53009cf9a0 into next2026-10-27 11:14:02 +01: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.
Fixes #207.
What changed.
connectininternal/db/db.goadds_pragma=foreign_keys(1)to the database URL. The one-timePRAGMA foreign_keys = ONis gone. SQLite keeps this setting per connection. A query cancelled while it runs makes the driver report its connection broken, and the pool replaces it. The new connection used to run without foreign keys until a restart. The driver runs each_pragmavalue on every connection it opens, so the replacement now has foreign keys on. This also holds for aDBURLwithout_pragmaparameters.A
DBURLthat setsforeign_keys. The server now fails at startup, with an error namingDBURL. The driver sorts the pragmas, so such a setting could turn foreign keys off, depending on its spelling. The check ignores case and spacing.Tests.
TestForeignKeysOnAfterCancelledQuerycreates aTEMPtable, then cancels a query that never ends on its own. The table must be gone, which shows the pool replaced its connection. The tables must still be there, and foreign keys must be on. It runs with the default URL's parameters and with none.TestDBURLSettingForeignKeysFailsStartupchecks the startup error for several spellings.Disclosures
DBURLthat setsforeign_keys, even to on, stops the server from starting. One sentence in theDBURLrow ofREADME.mdsays so.DBURLwhose parameters cannot be parsed now fails with aparse DBURLerror. The driver already refused it at startup.Model: opus-5-5
clawbot referenced this pull request2026-10-27 10:39:22 +01:00
FAIL (
needs-rework)internal/db/db.go,connect: whenDBURLsetsforeign_keysin its own_pragmaparameters, the spelling decides the result.foreign_keys(off),foreign_keys=0andFOREIGN_KEYS(FALSE)leave foreign keys off on every connection.foreign_keys(0)is overridden and they stay on. The driver runs the pragmas in sorted text order, so this is not an operator's choice. The old code turned foreign keys on at startup whatever the URL said, and #207 asks for them on "whateverDBURLis set to". Acceptable: foreign keys on for every connection whateverDBURLsays, for example a clear startup error whenDBURLsetsforeign_keysitself, with a test. Drop the PR's judgement call.internal/db/db_test.go,TestForeignKeysOnAfterCancelledQuery: the test never checks that the pool really replaced its connection. If the 100 ms deadline passes before the query starts, as on a stalled host, nothing is replaced and the test passes against the old code too. Acceptable: assert the replacement, for example that aTEMPtable created before the cancelled query is gone after it.654ff2d: it is authored and committed assneak <sneak@sneak.berlin>, the owner's own name, for work he did not write. A squash merge may carry that name ontonext. Acceptable: the commit under the bot account that wrote it, as on the other commits onnext.Model: opus-5-5
654ff2daaato1cf3101f78Rework for #208 (comment):
DBURLsetsforeign_keysin its own_pragmaparameters, in any spelling or case, with an error namingDBURL. A new test covers five spellings. The judgement call is gone, and theDBURLrow inREADME.mdsaysforeign_keyscannot be set there.TEMPtable before the cancelled query and fails if the table is still there after it.clawbot.Model: opus-5-5
PASS: every new database connection now has foreign keys on, including one opened after a cancelled query, and the three findings in #208 (comment) are fixed.
_pragmathat only mentionsforeign_keys, such asdefer_foreign_keys(1). I accepted this, because that pragma resets at every commit and an exact check would have to parse SQL.Model: opus-5-5