[jilio]

Readers previously had to use New, which may create a missing database, migrate its schema and change its journal mode. OpenReadOnly opens existing file-backed stores with SQLite-enforced read-only connections, validates their schema, and returns ErrReadOnly for mutations. Reads observe later WAL commits; shared-memory sidecars may still change.

Tests cover reads and mutators, engine-level write denial, missing/corrupt/incompatible databases without repair, literal URI-sensitive filenames, pooled connections and concurrent WAL commits. All standalone minimum-Go builds, vet, race and coverage jobs and the full workspace CI passed on the merged release commit. A separate consumer downloaded all four modules through the normal Go proxy with checksum verification, selected v0.20.0 uniformly, and passed a read/write-denial test.

Included in the unified ebu v0.20.0 release, with v0.20.0 tags for core, SQLite, Durable Streams and OpenTelemetry. Existing APIs remain compatible.

[claude]

Review: Add read-only opening for existing SQLite stores

This is a well-scoped, carefully engineered addition. Overall assessment: solid, ready to merge after considering the minor points below.

Strengths

• Security: OpenReadOnly builds the SQLite DSN via net/url (url.URL + url.Values.Encode()), so the path is properly percent-escaped. This avoids the URI-parameter-injection class of bug that New() currently has to defend against with an outright block-list on ?/# in the path (store.go). TestOpenReadOnlyRejectsWriteOptionsAndEscapesLiteralFilenames specifically exercises a filename containing %3f, ?, =, and # and confirms it round-trips correctly — nice catch, this is exactly the kind of thing that's easy to get wrong.
• Defense in depth: mode=ro is applied via the DSN (so it's enforced on every pooled connection, not just the first one handed out — the package already has a comment elsewhere explaining why this matters for _pragma params), and query_only(1) is layered on top. TestOpenReadOnlyReadsWithoutDataSchemaOrJournalModeChanges goes further and manually flips query_only=OFF on a pooled connection to prove mode=ro alone still blocks writes — a good regression guard against a future internal call accidentally bypassing the public API's ErrReadOnly checks.
• Schema validation without repair: checking schema_version for an exact version match (not <=), then running bounded (LIMIT 0) probes against events, subscription_positions, and snapshots is a nice way to catch a mid-migration/corrupted database (missing table, dropped column) that a naive version-number check would miss, without scanning actual data. The missing-event-column, missing-events, etc. test cases confirm this works.
• Test coverage is thorough: journal-mode variations (WAL/DELETE), concurrent-writer WAL visibility, pooled-connection enforcement, byte-for-byte file immutability checks across the full open/read/reject-mutation/close cycle, and a good spread of invalid-database shapes (missing/empty/garbage/truncated/unrelated/future-schema/older-schema/missing-table/missing-column). This is exactly the kind of test suite you want for a read path that must never silently create or repair a file.
• Mutation guards on Append/SaveOffset/SaveSnapshot/TruncateBefore are placed before any side effects, and Append/SaveOffset correctly still notify the metrics hook on rejection (matching how they report other errors).
• Docs addition in PERSISTENCE.md clearly calls out the mode=ro vs immutable=1 distinction and the WAL-sidecar caveat, which is important for callers who assume "read-only" means "immutable snapshot."

Minor suggestions (non-blocking)

1. Read-side pragmas dropped: buildDSN (used by New) sets synchronous(NORMAL), cache_size(-64000), temp_store(MEMORY), and mmap_size(268435456) for every writer connection. OpenReadOnly's DSN only carries busy_timeout and query_only(1), so read-only connections fall back to SQLite's small default page cache and no mmap. For a large database this could make OpenReadOnly reads noticeably slower than reads through a New-opened store. Worth considering whether cache_size/mmap_size (at least) should carry over — synchronous is moot for a read-only connection.
2. Duplicated pragma-string formatting: both buildDSN and OpenReadOnly independently do fmt.Sprintf("busy_timeout(%d)", cfg.busyTimeout.Milliseconds()). Minor, but a shared helper would prevent the two from drifting if the format ever needs to change.
3. Pre-existing asymmetry now more visible: New() rejects any path containing ?/# outright, while OpenReadOnly handles the same characters correctly via proper URI escaping. Not a bug introduced here, but now that the escaping approach exists and is proven by tests, it might be worth a follow-up to apply the same fix to New() instead of the current block-list.
4. Consider whether a CHANGELOG.md entry is expected for this repo's release process, given the PR description calls out a specific target release (stores/sqlite/v0.20.0).

Note on verification

I was not able to execute go build/go test/go fmt in this review environment (sandbox blocked those commands), so I could not independently confirm the coverage numbers cited in the PR description (100% core, 97.6% sqlite) or run race detection. The code and tests read as correct and consistent with the rest of the package's conventions on manual review, but please make sure CI is green (per this repo's CLAUDE.md: go fmt ./..., go test ./..., and the coverage check) before merging.

No correctness bugs or security issues found.