db: keep startup fail-closed on a missing SQLite file

[rain-marvin]

On a fresh disk the issuance bot exits at startup with (code: 14) unable to open database file. This PR keeps that behavior, on purpose, and pins it with tests. A missing file means a bad path or an unmounted volume. If the bot created an empty database there, it would serve with no event history, no enabled assets and no receipt inventory. docs/nixos-provisioning.md already documents the fail-closed contract and the one-time install … /mnt/data/issuance.db step.

Live effect: none. Startup behavior does not change; this adds a code comment, a runbook sentence and tests. · Risk: low · Ships: next release

Decisions

• Do not set create_if_missing on either server pool. An earlier version of this PR set it to true, and review showed that this removed the documented guard (thread).
• Keep ?mode=rwc in DATABASE_URL as the only opt-in, instead of adding an env flag. sqlx already parses it, and the test suites already use it. The runbook now says never to set it in staging or production.
• Do not explicitly set create_if_missing(false), because that would override mode=rwc and remove the opt-in.

Proof

• tests/fresh_database.rs::startup_fails_without_creating_a_missing_database_file: initialize_rocket against a missing file with no mode=rwc fails with "unable to open database file", and the file still does not exist.
• tests/fresh_database.rs::startup_creates_a_missing_database_file_with_mode_rwc: the same start with ?mode=rwc creates the file.
• cargo fmt, cargo check --workspace --tests and cargo clippy --workspace --all-targets --all-features -D warnings pass locally.

🤖 Generated with Claude Code

[rain-marvin]

• db: keep startup fail-closed on a missing SQLite file #447 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add the label add-to-gt-merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[kaisbaccour]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (1)

AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Team
• Run ID: 81fd8893-13a0-4efb-857c-28dd85a3f04a

📥 Commits

Reviewing files that changed from the base of the PR and between 0834d51 and 081d333.

📒 Files selected for processing (3)

• docs/nixos-provisioning.md
• src/lib.rs
• tests/fresh_database.rs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

Walkthrough

The pool documentation states that neither pool enables create_if_missing and that mode=rwc opts into creating a missing SQLite file. The provisioning documentation describes the same option. Two integration tests check startup behavior with and without mode=rwc.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 081d3

The documented startup behavior matches the pool configuration; no issue identified here needs to be resolved before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preserving fail-closed startup behavior when the SQLite file is missing.
Description check	✅ Passed	The description explains the missing-file behavior, the opt-in creation mode, the tests, and the documentation changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[rain-marvin]

@kaisbaccour #447, now at 8fe17584: I restacked #447 onto main's current head (3e90a8f) without conflicts and force-pushed it. The new head is 8fe1758. There were no review threads, the restack changed no code so I didn't run any tests, and cargo fmt --check, cargo check --workspace --tests and clippy with warnings denied all pass.

To stop me driving this stack, say “pause #447”.

[rain-marvin]

🔎 Reviewing 8fe1758, started by @kaisbaccour. The review will appear here when it's done.

[rain-marvin]

This PR sets create_if_missing(true) on both SQLite pools (the event store pool and the apalis job pool), so the bot creates issuance.db and runs migrations when the file is missing, instead of exiting with SQLite error 14. It adds tests/fresh_database.rs to prove that startup now creates the file.

The code change does what the description says. But the old behavior was a deliberate safeguard, not a bug. docs/nixos-provisioning.md (Database section) says the service opens SQLite with create_if_missing = false on purpose, because a missing file means a wrong path or an unmounted volume. It also says the error 14 crash loop on a fresh host is "the fail-fast working as intended", and that the operator creates the file once by hand. src/burn_excess/cli.rs keeps create_if_missing(false) for the same reason. This PR removes that guard, and it does not update the runbook. The fix for a fresh disk is the documented one-time install ... /dev/null /mnt/data/issuance.db step. If the team wants automatic creation, it should be an explicit opt-in, with a mount check on the service. One blocking finding.

Verdict: Request changes. I opened this PR, and GitHub does not let an author approve or request changes, so the verdict is in the rain-marvin/reviewed check.

claude-opus-5-5 · high · 8 min

[rain-marvin]

@kaisbaccour #447, now at 081d3335: The review was right: setting create_if_missing(true) removed the fail-closed guard that the runbook says is there on purpose. So #447 no longer makes the bot create a missing database file; both pools are back to how main opens them, and a missing file stops startup again. ?mode=rwc in DATABASE_URL is now documented in the code comment and runbook as the only way to opt in, and tests/fresh_database.rs checks both cases: without it, startup fails and leaves no file; with it, the file is created. I replied in the thread, resolved it, and updated the PR title and description to match. The new head is 081d333; the two fresh_database tests, fmt, cargo check --workspace --tests and clippy all pass.

To stop me driving this stack, say “pause #447”.

[rain-marvin]

@coderabbitai review

[rain-marvin]

🔎 Verifying that my findings from 081d333 are addressed at 081d333, started by @kaisbaccour. The review will appear here when it's done.

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[rain-marvin]

This PR no longer changes runtime behavior. The server keeps opening SQLite without create_if_missing, so a missing database file still stops startup with error 14. The PR adds a code comment in create_pool, a runbook sentence saying ?mode=rwc in DATABASE_URL is the only way to opt in to creating the file (never in staging or production), and two tests in tests/fresh_database.rs that pin both sides of that rule.

Earlier findings:

• Resolved: removing the fail-closed guard on a missing SQLite file (src/lib.rs). At 081d333, neither create_pool (sqlx 0.9) nor create_apalis_pool (apalis/sqlx 0.8) sets create_if_missing, so both use the sqlx default (false) unless the URL carries mode=rwc. The runbook keeps the manual install ... /mnt/data/issuance.db step. startup_fails_without_creating_a_missing_database_file runs the real initialize_rocket and checks that it fails with "unable to open database file" and leaves no file. startup_creates_a_missing_database_file_with_mode_rwc checks the opt-in. I ran cargo test --test fresh_database at 081d333 and both tests pass. Not setting create_if_missing(false) explicitly is the right call, because that would override the mode=rwc opt-in the whole test suite relies on.

Threads resolved by hand: none. The only thread was resolved by the bot after the fix above, and the code backs it.

No commits since my last review (head is still 081d333). I found no new problems in the full PR diff. CI (static, test) was still running when I checked.

Verdict: Approve. I opened this PR, and GitHub does not let an author approve or request changes, so the verdict is in the rain-marvin/reviewed check.

claude-opus-5-5 · high · 2 min

[rain-marvin]

@kaisbaccour #447 is ready for human review: both bot reviews pass on 081d3335, CI is green and no thread is open.

To stop me driving this stack, say “pause #447”.

[rain-marvin]

@kaisbaccour 🔔 Follow-up: a day after my review

@kaisbaccour, #447 hasn't changed since I marked it ready yesterday. It's still on 081d3335. CI is green, both bot reviews pass and it can merge with no conflicts. All it needs is a human approval, so human/reviewed is the only failing check. You, @JuaniRios or @ueco-jb can approve it, then add add-to-gt-merge-queue to merge it.

claude-opus-5-5 · high · 15 s

[ueco-jb]

@rain-marvin what's the point of this change? You modified two comments and added a test?

[rain-marvin]

@ueco-jb You're right, and there's not much left in it now. The PR started as a fix for (code: 14) unable to open database file on a fresh disk, by setting create_if_missing(true) on the pools. Review showed that broke the fail-closed rule in docs/nixos-provisioning.md, so I took it out. A missing file means a wrong path or an unmounted volume, and an empty database would mean serving with no events, no enabled assets and no receipt inventory. After that, nothing in runtime behaviour changes. What's left:

1. tests/fresh_database.rs::startup_fails_without_creating_a_missing_database_file is the only part that does anything. Fail-closed comes from sqlx's default (create_if_missing is false), not from anything this repo sets, so nothing stops someone from "fixing" the code-14 crash the way I first did. This test would fail in that case and point to the runbook.
2. The src/lib.rs comment and the runbook sentence say the same thing in words. They also say ?mode=rwc in DATABASE_URL is the only opt-in, and that staging and production must never set it.
3. startup_creates_a_missing_database_file_with_mode_rwc mostly tests sqlx's URL parsing. It's the weakest part.

So it's a regression guard for a mistake that already happened once, and that's all. If you think that's worth an anvil-backed integration test, merge it. I'd drop the mode=rwc test first. If the runbook is enough, close it; it won't change anything that's running. I haven't changed the PR. Say "drop the rwc test" or "close it" and I'll do that.

claude-opus-5-5 · high · 25 s

[rain-marvin]

@kaisbaccour #447, now at 458aff9b: I restacked #447 onto main at d818965, 12 commits newer than its old base. There were no conflicts and no review threads, and I force-pushed it. The new head is 458aff9. In the Nix dev shell, cargo fmt --check, cargo check --workspace --tests and clippy with warnings denied all pass; the code didn't change, so I didn't run the tests.

To stop me driving this stack, say “pause #447”.

[rain-marvin]

@kaisbaccour #447 is ready for human review: both bot reviews pass on 458aff9b, CI is green and no thread is open.

To stop me driving this stack, say “pause #447”.