Skip to content

test(db): reapply DSN pragmas after OpenMemory deserializes its template - #3042

Merged
vavallee merged 1 commit into
vavallee:mainfrom
francisrath:fix/openmemory-schema-pragmas
Oct 6, 2026
Merged

vavallee merged 1 commit into
vavallee:mainfrom
francisrath:fix/openmemory-schema-pragmas

Conversation

@francisrath

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2983, from @vavallee's review: synchronous and cache_size are per-schema, so Deserialize reset them on every OpenMemory copy (to 2 and -2000), while the DSN asks for 1 and -16000. OpenMemory now runs the DSN's pragmas again after loading the template, so a copy matches what a plain open gives.

Refs #2982

Implementation notes

  • connectionPragmas() reads the pragma list out of connectionPragmaDSN, so the values live in one place and a pragma added to the DSN later is reapplied (and tested) without anyone remembering to.
  • The test now checks every DSN pragma on a copy rather than only foreign_keys, and its comment no longer claims they all survive the copy.
  • Production Open is unchanged.

How it was verified

The updated test fails on main for the reason in the review:

--- FAIL: TestOpenMemory_CopiesAreIndependent
    openmemory_test.go:43: PRAGMA synchronous = 2, want 1
    openmemory_test.go:43: PRAGMA cache_size = -2000, want -16000

and passes here. The extra pragmas cost nothing measurable: go test -race -run '^TestDownloadClient' ./internal/api is 4.0s.

Checklist

  • Every commit carries a Signed-off-by that matches its author (git commit -s)
  • Changelog fragment: N/A, test-only change
  • Tests added or updated
  • docs/DEPLOYMENT.md: N/A
  • Wiki pages: N/A
  • No new dependency

Test plan

  • go test ./internal/db
  • make test
  • make test-race (all six shards green, no races reported)

🤖 Generated with Claude Code

synchronous and cache_size are per-schema, so Deserialize reset them to
SQLite's defaults (2 and -2000) on every OpenMemory copy, while the
DSN asks for 1 and -16000. OpenMemory now runs the DSN's pragmas again
after loading the template, and the test checks every DSN pragma on the
copy instead of only foreign_keys, so a pragma added to the DSN later
is covered too.

Refs vavallee#2982

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Francis Rath <fr@ncis.no>
@github-actions github-actions Bot added the bindery-notified Discord notification already sent for this PR label Oct 6, 2026
@codecov

codecov Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/db/db.go 57.14% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@vavallee vavallee left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's exactly the fix I had in mind, and reading the list out of the DSN means nobody has to remember to keep two lists in sync. OpenMemory's pool is a single connection so the reapplied pragmas land on the only connection there is. Merged current main into it locally (including the sync.Once driver fix from #3037) and go test ./internal/db -count=2 is green. The codecov patch miss is just the two error branches, fine by me. Merging.

@vavallee
vavallee merged commit 6e8d77c into vavallee:main Oct 6, 2026
41 of 42 checks passed
vavallee added a commit that referenced this pull request Oct 8, 2026
Fold the 41 changelog.d fragments into a v1.41.0 section with upgrade
notes for proxy auth, NZB content failure blocklisting, the Fix match
default and the queued library scan, credit contributors the fragments
did not name (#2615, #3042, #3053, #3057, #2995, #3098), and
clear changelog.d.


Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9

Signed-off-by: vavallee <vavallee@protonmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindery-notified Discord notification already sent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants