fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704, slice 2) by cdeust · Pull Request #937 · cdeust/Cortex

cdeust · 2026-11-07T07:49:03Z

Replaces #865 with the same head commit (f27f1c09), opened for one exceptional extra review (owner decision 2026-11-07). #865 used its four ADR-1119 reviews; its last (review 4) blocked on B2, which 21fbc2f3 answers: groomer.py, cortex_ingest.py and dev_run_ingest.py call settle_for_command before main(), and tests_py/infrastructure/test_scripts_settle_backend.py now scans every scripts/*.py instead of a hand-kept list. Review history: #865.

Summary

Library residual of #704, named out of scope by #804 (docs/verification/draft-relevance-20261027.md § 770): MemorySettings.DATABASE_URL still defaulted to postgresql://127.0.0.1:5432/cortex. Every library path that reads the setting without a named URL could therefore reach a local cortex database nobody configured:

_construct_store with the cli runtime mapped auto to postgresql and tried the default URL;
python -m mcp_server.migrate connected to it (RED reproduced: connection to server at "127.0.0.1", port 5432 failed);
agent_briefing_query._connect passed it to raw pg_connect when no backend was decided.

Root cause: the default URL in memory_config.py. It is now empty, and the callers follow an explicit contract:

auto with no named URL → SQLite in every runtime, with no connection attempted (_resolve_backend_url follows the same rule, so the cache key stays consistent);
an explicit postgresql backend with no URL → RuntimeError (no silent SQLite store, no guessed server; the old allow_fallback = not url branch disappears);
pg_store_ddl._get_database_url refuses an empty conninfo (ValueError), because libpq would then connect to its local defaults; cortex-migrate reports it on its exit-1 contract;
agent briefing: with no URL, only the shared store decides; for postgresql without a URL, nothing is opened.

PRIVACY.md is corrected: it promised a "default postgresql://127.0.0.1:5432/cortex". The marker path keeps ADR-1104's legacy URL in settle_backend, and the text says so.

Refs #704 (not Closes: see "Breaking changes / remaining work").

Type of change

Bug fix
Breaking change (a library caller relying on the implicit local URL must name one)

Test plan

New tests_py/infrastructure/test_no_default_database_url.py, 10 tests: 9 fail on origin/main (1de07f9) and 10 pass with the fix. Every _try_pg_verbose / pg_connect call fails the test, so any connection attempt is caught.
test_sqlite_backend.py::test_reachable_postgresql_is_preferred_over_sqlite asserted exactly the behaviour A profile with no configuration connects to a local PostgreSQL it was never configured for #704 rejects (a default URL that answers wins). It now names its URL through CORTEX_MEMORY_DATABASE_URL.
Targeted run: test_no_default_database_url, test_sqlite_backend, test_backend_resolution, hooks/test_entry_fresh_backend, test_shared_backend_bootstrap, hooks/test_agent_briefing*, test_migrate → 122 passed, 8 skipped (PostgreSQL absent).
Full pytest tests_py: 10904 passed, 624 skipped, 4 failed, none caused by this PR: 3 test_embedding_engine (torch not installable here, its download host is blocked) and test_wiki_coverage_dashboard::test_unreadable_page_is_skipped_not_fatal, which also fails on origin/main (the session runs as root, so "unreadable" files can be read).
ruff check / ruff format --check (0.17.3, from uv.lock) pass on the touched files (main has 15 E501/E741 errors elsewhere). scripts/check_craftsmanship.py: OK.

Audit notes

No independent review in this pass.
Not verified: the PostgreSQL-gated tests (no PostgreSQL here), Windows/macOS.

Breaking changes

Before: with no URL and no marker, a library caller (CLI auto, migrate, briefing) connected to postgresql://127.0.0.1:5432/cortex.
After: SQLite (auto), or an explicit error (postgresql backend, migrate). Migration: set DATABASE_URL or CORTEX_MEMORY_DATABASE_URL.
Remaining A profile with no configuration connects to a local PostgreSQL it was never configured for #704 slice, needs an owner decision: scripts/install-plugin.sh (lines 20–21, 87) still selects PostgreSQL at install time when it finds a reachable local cortex database ("never downgrade"). Removing that choice changes upgrades for existing PostgreSQL installs, so it is a product decision, not part of this PR.

Reviewer checklist

CHANGELOG.md updated (Unreleased / Fixed).
Documentation updated (PRIVACY.md).
No secrets / credentials / PII in the diff.
CI passes on the latest commit: not yet observed.

Generated by Claude Code

…s SQLite (#704) MemorySettings.DATABASE_URL defaulted to postgresql://127.0.0.1:5432/cortex, so library callers with no URL and no marker (CLI-runtime auto, cortex-migrate, the agent-briefing raw connection) reached whatever local cortex database answered. The default is now empty; auto without a named URL is SQLite in every runtime; an explicit postgresql backend with no URL raises; _get_database_url refuses an empty conninfo and cortex-migrate reports it on its exit-1 contract. Refs #704 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) Review B1 (PR #865, comment 6658250021): portability, migrate and wiki_migrate run as standalone commands and never read the backend marker. With no default URL, a PostgreSQL marker and no URL in the shell gave an empty SQLite store, silently. They now call backend_resolution.settle_for_command first, as mcp_server/__main__.py does. Also: migrate and memory_store docstrings, and the PRIVACY/CHANGELOG wording on which backends use the legacy URL. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) Codex review on 749bf05 (comment 4480636738): the nine scripts/ maintenance commands that open the shared store (memory_staleness_revalidate, memory_reheat, memory_dedup_exact, and six more) now read the backend marker first through settle_for_command, like portability and migrate. Without it, a PostgreSQL marker with no URL gave them an empty SQLite store. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) unit failed on 25c27d2 (run 42031311038, job 127258608806): test_migrate_reports_a_missing_url_on_its_exit_contract. migrate now settles the backend first. The fixture left CORTEX_MEMORY_STORE_BACKEND=postgresql in place: scripts/ci_unit.sh exports it, and tests_py/conftest.py replaces it with sqlite only when PostgreSQL is unreachable, which is why the test passed locally. settle_backend then supplied the legacy URL. The fixture now removes the backend variable, points the marker at an empty temporary methodology directory, and restores the environment whole. Reproduced locally by re-exporting the variable after conftest; passes with the fix. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) Mutation gate (ADR-1122) on cc3804d, run 42034218395: kill the survivors in the functions this PR changes. Adds tests for settle_for_command, portability.main, migrate._run, _get_database_url and agent_briefing._connect. settle_for_command moves above the first function: when it was appended after resolve_auto_backend, the diff's trailing blank lines put that untouched function in scope. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) Mutation gate (ADR-1122) on cc3804d, run 42034218395: for memory_store.py the stats run died on tests_py/hooks/test_entry.py. The hook subprocess inherits MUTANT_UNDER_TEST and runs mutmut's trampoline in a temp cwd with no [tool.mutmut] config ("Could not figure out where the code to mutate is"). hermetic_hook_env now drops the variable, so the hook process runs the original code and the in-process tests judge the mutants. Any PR that touches a module these hook tests import hits the same failure; the change is also proposed to main separately. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…a URL is SQLite (#704) fixup! fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704) unit on f944b6f (run 42042293324, attempt 2): 35 failed, 204 errors, most of them 'no PostgreSQL URL configured' or KeyError 'DATABASE_URL' in later PostgreSQL tests. The no_named_url fixture pops DATABASE_URL, a test then monkeypatch.setenv's it, and monkeypatch's own teardown (which runs after the fixture's full restore) deletes the key it recorded as absent. CI's DATABASE_URL was gone for the rest of the session. Both fixtures now call monkeypatch.undo() before the restore. Reproduced with a probe test after these files (failed before, passes after); a full before/after environment comparison now differs only in PYTEST_CURRENT_TEST. The tree_sitter_language_pack DownloadError failures in the same run are GitHub fetches, not this PR. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…tabase-url

…(300-line cap) They test backend_resolution.settle_for_command, not a standalone command; the move brings test_standalone_commands_settle_backend.py from 301 to 263 lines.

…a URL is SQLite (#704) Check unit, run 42204224811 (32ad0ff): the ADR-1122 mutation gate left 122 survivors in the changed functions of memory_store.py (_construct_store 102, _resolve_backend_url 16, _database_url_is_explicit 4). No test pinned the selection's outcomes: which store, path and dimension, which URL is tried, and the exact operator-facing error and warning texts. test_construct_store_selection.py pins every branch with PostgreSQL and SQLite replaced by recorders. Two code changes remove mutants no test can kill: the strict-failure message no longer carries the dead `url or '<unset>'` (url is non-empty there since #704 raises first), and os.environ.get("DATABASE_URL") drops a "" default that `or` made equivalent to None. The two remaining CORTEX_ALLOW_SQLITE_FALLBACK default mutants are registered equivalents. Local gate (mutation_check.sh, main's gate scripts, forkserver, only this test file): 187 in-scope mutants, 185 killed, 2 registered equivalents. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtrWzv6PGpJnXKSYPs

…tabase-url # Conflicts: # memory/mutation-equivalents.json

…ore a handler opens the store (#704) PR #865 review 4 (B2): scripts/groomer.py, scripts/cortex_ingest.py and scripts/dev_run_ingest.py reach get_shared_store through handlers (get_grooming_health, curate_distill, ingest_codebase) and never called settle_for_command. With a PostgreSQL marker and no DATABASE_URL (the documented launchd groomer job), they used an empty SQLite store, silently. Each entry point now settles first and exits 1 when it cannot. The earlier sweep relied on a list kept by hand. A new test scans scripts/ and requires settle_for_command in every script that imports a handler or the store, with the two exemptions explained in the test. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYtyFK1iv4S9wZDjGoP

…tabase-url

chatgpt-codex-connector · 2026-11-07T07:49:18Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-07T08:45:34.703790Z	`83ae4e5`	New commits
🔒 Security Review	✅ Completed 2026-11-07T08:47:51.584656Z	`83ae4e5`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

claude · 2026-11-07T07:49:45Z

Claude finished @cdeust's task in 4m 13s —— View job

Review 1 of 3 (ADR-1119): complete

Gathered context: the PR diff and CLAUDE.md
Ran all five reviews (code quality, performance, test coverage, documentation accuracy, security)
Checked their findings against the code
Posted the review: top-level comment and one inline Blocker

Blocker: PRIVACY.md:60-64 says that with no URL and no marker, Cortex opens no PostgreSQL connection. That is false. On a HOME that already has Cortex state, the first-launch decision probes localhost:5432 and can select postgresql://localhost:5432/cortex (backend_resolution.py:116-173).

I did not run any tests. I made no code changes.

Verdict: block

claude · 2026-11-07T07:53:44Z

+  is no default URL in code: with neither `DATABASE_URL` nor a PostgreSQL
+  backend marker, Cortex uses SQLite and opens no PostgreSQL connection
+  (a PostgreSQL backend selected by the marker or by
+  `CORTEX_MEMORY_STORE_BACKEND=postgresql`, with no URL, uses
+  `postgresql://localhost:5432/cortex`, ADR-1104; issue #704). The opt-in setup script installs


Blocker: this statement about behaviour is false. "With neither DATABASE_URL nor a PostgreSQL backend marker, Cortex uses SQLite and opens no PostgreSQL connection" only holds on a fresh install.

Here is the case that breaks it: a HOME that already has Cortex state in ~/.claude/methodology but no marker.

settle_backend → persist_first_decision → decide (mcp_server/infrastructure/backend_resolution.py:116-128) runs legacy_store_answer, which opens TCP sockets to localhost:5432 (lines 86-113).

If anything answers, or the result is unclear (for example a timeout or a PG* variable), it writes a postgresql marker.

settle_backend then sets DATABASE_URL=postgresql://localhost:5432/cortex (line 173).

So a user with no URL and no marker can end up connected to a local cortex database they never configured. That is the case this paragraph says cannot happen. The parenthetical also leaves out CORTEX_BACKEND=postgres and CORTEX_MEMORY_DATABASE_URL, both of which select PostgreSQL.

Suggested fix: limit the claim to a fresh install. Then say that on a HOME with earlier Cortex state and no marker, the first launch probes localhost:5432 once and may record and use PostgreSQL there (ADR-1104).

claude · 2026-11-07T07:54:06Z

Review 1 of 3 (ADR-1119)

All five reviews ran: code quality, performance, test coverage, documentation accuracy and security.

Blockers

PRIVACY.md:60-64: the new privacy text says something false about behaviour (inline).
- The claim: "with neither DATABASE_URL nor a PostgreSQL backend marker, Cortex uses SQLite and opens no PostgreSQL connection".
- What actually happens: on a HOME that already has Cortex state but no marker, settle_backend → decide (mcp_server/infrastructure/backend_resolution.py:116-128) probes localhost:5432. It picks PostgreSQL if anything answers or the answer is unclear, then sets DATABASE_URL=postgresql://localhost:5432/cortex (line 173).
- Fix: limit the claim to a fresh install, and describe the one-time first-launch probe (ADR-1104).

Notes, no action before merge

cortex-migrate on a HOME with no marker writes the first-launch marker and prints a second line to stderr ([cortex] storage: …). That contradicts the "exactly one line to stderr" Post in migrate.py:30; the test checks with in, so it passes.
groomer_coordinator.py:57-60 was not updated: auto with no URL now hashes "" as the store identity, while the store it actually opens is SQLite at SQLITE_FALLBACK_PATH.
A whitespace-only URL still counts as a URL in memory_store.py:110,137 (it is not stripped there, unlike in _get_database_url), so _try_pg_verbose(" ") can hand libpq an empty conninfo.

Verdict: block

…a URL is SQLite (#704) Review 1 of #937, blocker: PRIVACY.md said that with neither DATABASE_URL nor a marker Cortex opens no PostgreSQL connection. That holds on a fresh install only: on a HOME with earlier Cortex state and no marker, settle_backend -> persist_first_decision -> decide probes localhost:5432 once and selects PostgreSQL when something answers or the answer is unclear, then writes the marker (ADR-1104). The text now says both.

claude · 2026-11-07T08:44:32Z

Claude finished @cdeust's task in 1m 39s —— View job

Review 2 of 3 (ADR-1119): complete

Checked the review 1 Blocker (PRIVACY.md:60-64): fixed. The no-connection claim now covers only a fresh install, and the text describes the one-time first-launch probe of localhost:5432 (backend_resolution.py:116-174).
Found the commits pushed after f27f1c09: one commit, 83ae4e5, which changes only PRIVACY.md
Ran all five reviews on that commit (code quality, performance, test coverage, documentation accuracy, security)
Checked their findings against the code and posted the review

Blockers: none. The new commit introduces no false security or behaviour statement. The review lists three wording notes that need no action before merge:

"If a PostgreSQL answers" is too narrow: any listener on port 5432 counts, and a PG* variable skips the probe.
The sentence "never connects to a database you did not configure", already on main, now visibly contradicts the probe text just above it.
"No default URL in code" is not exact: backend_resolution.py:173 still hard-codes the legacy URL.

I did not run any tests. I made no code changes.

Verdict: approve

claude · 2026-11-07T08:46:13Z

Review 2 of 3 (ADR-1119)

Scope: this review checks the review 1 Blocker, then reviews only the one commit pushed after f27f1c09: 83ae4e5, which changes only PRIVACY.md. All five reviews ran (code quality, performance, test coverage, documentation accuracy, security), and I checked their findings against the code.

Review 1 Blocker (PRIVACY.md:60-64): fixed.

The text no longer says Cortex makes no connection whenever there is no URL and no marker. It now limits that claim to a fresh install, which matches backend_resolution.decide: it returns sqlite before any socket call (mcp_server/infrastructure/backend_resolution.py:120-121).
It now describes the one-time first-launch probe on a HOME with earlier Cortex state: PostgreSQL when the probe connects or the answer is unclear, SQLite otherwise (:122-128). "Once" is accurate because the decision is written as the marker (:141, :160-164).
It states that a selected PostgreSQL backend with no URL uses postgresql://localhost:5432/cortex (:171-173).

Blockers

None. I found no false security or behaviour statement introduced by 83ae4e5.

Notes, no action before merge

PRIVACY.md:63-64: the probe is a plain TCP connect, so any listener on 5432 counts (backend_resolution.py:101-113), and a set PG* variable skips the probe and selects PostgreSQL directly (:83-85); "if a PostgreSQL answers" is narrower than both.
PRIVACY.md:69-70 (text already on main, not changed here) says Cortex "never provisions or connects to any database you did not configure"; the probe path described just above it now visibly contradicts that sentence.
PRIVACY.md:60 says "no default URL in code", while backend_resolution.py:173 hard-codes the legacy URL; "in the settings" would be exact, and line 67 runs past the 78-column wrap.

Verdict: approve

claude and others added 14 commits November 5, 2026 12:36

Merge remote-tracking branch 'origin/main' into fix/704-no-default-da…

15ffa4a

…tabase-url

test: move the settle_for_command tests next to backend_resolution's …

32ad0ff

…(300-line cap) They test backend_resolution.settle_for_command, not a standalone command; the move brings test_standalone_commands_settle_backend.py from 301 to 263 lines.

Merge remote-tracking branch 'origin/main' into fix/704-no-default-da…

2c10ecd

…tabase-url # Conflicts: # memory/mutation-equivalents.json

Merge remote-tracking branch 'origin/main' into fix/704-no-default-da…

d034c79

…tabase-url # Conflicts: # memory/mutation-equivalents.json

Merge remote-tracking branch 'origin/main' into fix/704-no-default-da…

f27f1c0

…tabase-url

cdeust mentioned this pull request Nov 7, 2026

fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704, slice 2) #865

Closed

5 of 6 tasks

claude Bot reviewed Nov 7, 2026

View reviewed changes

github-actions Bot merged commit e47a499 into main Nov 7, 2026
18 checks passed

This was referenced Nov 7, 2026

A profile with no configuration connects to a local PostgreSQL it was never configured for #704

Open

test(hooks): no unconfigured hook dials PostgreSQL through either front door (#704) #1022

Merged

Navigation Menu

Repository navigation

fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704, slice 2) - #937

fix(storage): no default PostgreSQL URL in code; auto without a URL is SQLite (#704, slice 2)#937
github-actions[bot] merged 15 commits into
mainfrom
fix/704-no-default-database-url-review

cdeust commented Nov 7, 2026

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot Nov 7, 2026

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

claude Bot commented Nov 7, 2026 •

edited

Loading

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cdeust commented Nov 7, 2026

Summary

Type of change

Test plan

Audit notes

Breaking changes

Reviewer checklist

Uh oh!

chatgpt-codex-connector Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review 1 of 3 (ADR-1119): complete

Uh oh!

claude Bot Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

claude Bot commented Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review 2 of 3 (ADR-1119): complete

Uh oh!

claude Bot commented Nov 7, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatgpt-codex-connector Bot commented Nov 7, 2026 •

edited

Loading

claude Bot commented Nov 7, 2026 •

edited

Loading

claude Bot commented Nov 7, 2026 •

edited

Loading