feat(host-store): Postgres operational store — migrate JSON/in-process state off the filesystem

[jmlago]

What

A single transactional Postgres store (host_store.py, psycopg3) for the
host's operational state — operator knob overrides, the provider overlay,
dashboard-issued consumer keys, and the call/usage ledger — migrated off the
scattered JSON/JSONL files + in-process dicts.

Why Postgres and not SQLite: the deployment runs the router and the ingress as
SEPARATE containers (router read-only on secrets, ingress read-write) sharing
state. A SQLite file can't bridge that asymmetry (mount RO/RW, WAL-reader-needs-RW,
multi-process fragility) — it was a real deploy-time blocker. A network DB both
containers reach over TCP solves it at the root, and is the end goal anyway.
Prod = RDS; dev = a compose postgres service (DATABASE_URL).

History note: the early commits on this branch built a SQLite version table by
table; the deployment topology (two containers) forced the jump to Postgres. The
commits tell that evolution; the end state is Postgres.

Landed (suite green against real Postgres throughout)

• Backend: psycopg3 + ConnectionPool, schema created idempotently under a
  pg_advisory_lock (race-safe across both containers), each write its own
  transaction (atomic commit/rollback per with pool.connection() — the
  orphan-transaction class is impossible by construction).
• Tables, fully migrated off JSON: settings_overrides (overrides.json),
  provider_overlays (providers.local.json), consumer_keys
  (issued-consumer-keys.json), and the calls fact ledger (alongside
  usage-history; GET /x/calls). One-shot idempotent legacy-JSON backfill at
  startup (router + ingress, guard-on-empty + advisory lock) so existing
  deployments migrate with no data loss.
• Durable-write contract: set_* return a success bool; callers surface a
  persistence failure (a silently-failed key revoke would be a security hole) —
  config save, provider add, and the three key endpoints (update/revoke/rotate)
  now report 500 on failure.
• Review fixes folded in (doctrine review + CodeRabbit): retention env
  validated; the ledger writer is a BOUNDED queue + dict(row) snapshot off the
  request latency path; get_consumer_keys fails CLOSED on an unreadable row;
  _ID_RE 2-40 off-by-one.
• Infra: compose postgres:16 service + DATABASE_URL to both containers;
  psycopg[binary,pool] pinned; shell.nix updated.

Verify

Full suite 406 passed, 2 skipped, 0 failed against a real Postgres 16 under
nix-shell (tests run against the DB, no mocks; pure-logic tests need no DB). The
e2e (behave, features/) run against the live compose stack — the path that
exercises the two-container topology end to end.

Proposal for independent adjudication — not to self-merge.

Still ahead (separate PRs)

buyer_status ← market/status JSON, peer_offers ← market.json (+ the antseed
sidecar), and deriving route_* / retiring _stats + usage-history readers as
queries over calls — now unblocked because both containers reach the shared DB.

Summary by CodeRabbit

• New Features

  • Added a new operator-facing view of recent call activity, including total request counts and capped history.
  • Provider and configuration changes now persist across restarts using shared app storage.

• Bug Fixes

  • Improved reliability of dashboard updates by surfacing save failures instead of silently succeeding.
  • Added automatic migration of older saved data so existing setups continue to work after upgrade.
  • Configuration and provider changes now reload more consistently during startup and after updates.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a Postgres-backed host store for calls and operator state, migrates settings/provider/consumer-key persistence to it, adds startup backfill, exposes recent calls through an API, and updates runtime wiring and tests to use the new store.

Changes

Host store operational state and call ledger

Layer / File(s)	Summary
host_store: schema, queue, persistence host_store.py	Defines the Postgres schema, connection pool, ledger writes and pruning, query helpers, durable operator-state tables, legacy migration, and test reset utilities.
settings, provider overlay, and auth proxy persistence auth_proxy.py, provider_overlay.py, settings.py, serve.py	Moves settings overrides, provider overlays, and issued consumer keys onto host-store persistence, adds startup backfill, and makes dashboard writes fail when persistence fails.
compose, dependencies, and calls endpoint compose.yml, requirements.txt, shell.nix, shim.py	Adds Postgres service wiring, updates container environment and health checks, adds the psycopg dependency, extends the dev shell, and exposes recent call history through GET /x/calls.
DB-backed test isolation and rewired persistence tests tests/conftest.py, tests/test_auth_proxy_dashboard_full.py, tests/test_host_store.py, tests/test_provider_overlay.py, tests/test_settings.py, tests/test_sources.py	Replaces file-based fixtures with host-store-backed test isolation and updates dashboard, overlay, settings, and source tests to read and write through the new store.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 I hopped from files to Postgres bright,
With calls in a ledger and keys tucked tight.
Overlays, settings, all found their new den,
The burrow reloaded and bloomed again.
🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: moving operational state from filesystem/JSON storage to a Postgres host store.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch host-store

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[jmlago]

Re-review @ b719301 — follow-up to the prior doctrine review

The three new commits map 1:1 to the prior review's two blocking findings plus
the cosmetic cleanup: b719301 (offload) ↔ Tier 2 #1; 25769a9 (rollback) ↔
Tier 2 #2; 054cc30 (docs) ↔ the body's pending comment-text cleanup. Verified
against the tree, not the commit messages.

Verdict: APPROVE WITH COMMENTS

Both blocking Tier 2 findings are resolved and verified:

Tier 2 #1 — blocking IO + serialization on the async path → RESOLVED.
_append_usage_history now calls insert_call_async, which submits insert_call
to a single-worker ThreadPoolExecutor. The SQLite write and the process-wide
_lock move off the event loop; one worker preserves FIFO order and serializes
safely with set_*/reads under the same lock (check_same_thread=False already
present). The comment honestly records that the usage-history file append below
is still pre-existing synchronous blocking IO — the right scoping.

Tier 2 #2 — orphaned transaction poisons the shared connection → RESOLVED.
Every set_*, insert_call and _prune_locked now wraps its mutation in
with c: (commit on success, rollback on exception). Covered by a precise
new regression test, test_failed_write_rolls_back_and_does_not_poison_next_commit:
it forces an OperationalError mid-executemany after the DELETE and asserts
(a) the credentials stay intact and (b) the next insert_call commit does not
flush a phantom DELETE. Exactly the reported failure mode.

Comments (non-blocking)

• [Axis 7 — unbounded writer queue] New, introduced by the offload. The
  ThreadPoolExecutor(max_workers=1) has an unbounded work queue. If insert
  rate ever outran the single worker's drain (e.g. the _lock held by a long
  set_*/prune during a burst), the Future queue would grow without bound — the
  same failure class the store exists to prevent (the OOMKilled lesson), in a
  different organ. In practice unreachable: local SQLite drain (µs–ms) outpaces
  LLM-call completion (~seconds/call) by orders of magnitude, so queue depth stays
  ~0. Flagged only because a maxsize with a drop-oldest policy is cheap insurance
  consistent with the "bound everything" discipline. Not blocking.
• [ontology] Still standing: the ledger route_key (provider|family|served)
  diverges from the canonical route_reliability.route_key (provider|family|peer).
  Reconcile and enrich peer_id before deriving route_* from calls.
  Acknowledged in the description; tracked roadmap precondition, unchanged here.
• reset() (test hook) does not touch _writer; harmless because tests use the
  synchronous insert_call, never _async. Minor.
• Atomicity (Axis 5): the prior advisory to split the consumer_keys migration
  into its own revertable PR still stands as advisory, not blocking.

Verification

• tests/test_host_store.py → 11 passed (10 → 11, +the rollback regression)
  under nix-shell + timeout @ b719301.
• Layer (Axis 2): host_store still imports stdlib only (+concurrent.futures)
  → leaf, no cycle. Dependency (Axis 4): zero new deps (stdlib).

Path to merge

Mergeable. The only actionable item is the unbounded-queue comment, and it is
optional. Before merge: re-run the full suite locally (CI is notify-only, so the
"400 passed" figure is author testimony until reproduced) and, to keep it fresh,
regenerate the review object to record host_store in the ontology/layers/prior-art
and the new debt (still-synchronous file append, unbounded writer queue) in §8.

[coderabbitai]

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

auth_proxy.py (1)

316-323: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not report key mutations as saved when persistence failed.

host_store.set_consumer_keys() catches all write exceptions and returns None, so rotations, revocations, or consumer settings can return success without durable metadata. A failed revoke can reappear after restart. Make the setter raise/return a status and abort the dashboard operation on failure.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@auth_proxy.py` around lines 316 - 323, The persistence path in
_write_issued_consumer_records currently assumes host_store.set_consumer_keys
succeeds even though it swallows write failures, so mutations can be reported as
saved when they are not durable. Update host_store.set_consumer_keys to either
raise on persistence errors or return an explicit success status, then have
_write_issued_consumer_records and the dashboard mutation flow check that result
and abort/report failure for rotations, revocations, or consumer setting updates
when persistence does not complete.

🧹 Nitpick comments (1)

tests/test_provider_overlay.py (1)

111-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Restore malformed-overlay coverage on the SQLite path.

This roundtrip test no longer checks the old “bad persisted data loads as empty overlay” contract. Since po.load_overlay() now relies on host_store.get_provider_overlays() for that fail-soft behavior, please add the DB-backed equivalent here so the migration keeps that safety net covered.

Possible follow-up test

 def test_overlay_load_save_roundtrip(db):
     assert po.load_overlay() == {"providers": {}}
     overlay = {"providers": {"groq": {"base_url": "https://x", "auth_env": "G_KEY",
                                       "served_models": [{"family": "f"}]}}}
     po.save_overlay(overlay)
     assert po.load_overlay() == overlay
     po.save_overlay({"providers": {}})        # save empty -> overlay cleared
     assert po.load_overlay() == {"providers": {}}
+
+
+def test_overlay_load_ignores_malformed_rows(db):
+    with host_store._lock:
+        c = host_store._connect()
+        c.execute(
+            "INSERT INTO provider_overlays(provider_id, entry, added_at) VALUES (?,?,?)",
+            ("groq", "not json{", 0),
+        )
+        c.commit()
+    assert po.load_overlay() == {"providers": {}}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_provider_overlay.py` around lines 111 - 118, The roundtrip test in
test_overlay_load_save_roundtrip no longer covers the fail-soft behavior for
malformed persisted data on the SQLite path. Add a DB-backed assertion around
po.load_overlay() that seeds an invalid provider overlay through
host_store.get_provider_overlays() (or the underlying SQLite-backed store) and
verifies it still returns {"providers": {}}. Keep the existing save/load
roundtrip, and make sure the new coverage targets the load_overlay path that now
depends on host_store.get_provider_overlays().

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@auth_proxy.py`:
- Around line 213-215: Preserve fail-closed behavior in the consumer key loading
path: `host_store.get_consumer_keys()` currently skips JSON-decode failures but
still reports success, which leaves `_issued_keys_load_failed` false and can let
`_consumer_meta()` fall back to default metadata. Update `get_consumer_keys()`
so any unreadable row makes `ok` false, or return an inactive tombstone for that
consumer, and keep the existing `_issued_keys_load_failed = not ok` handling in
`auth_proxy.py` so corrupted metadata cannot be silently ignored.

In `@host_store.py`:
- Around line 236-250: The durable write helpers are swallowing persistence
failures instead of surfacing them to callers. Update `set_overrides()` and the
related operator-state writers `set_provider_overlays()` and
`set_consumer_keys()` in `host_store.py` so they either re-raise on failure or
return an explicit success indicator, and then make the `auth_proxy.py` callers
fail the request when the write does not persist. Preserve the atomic
transaction behavior in these helpers, but do not leave the error only in
`_log.warning`; callers must be able to detect the failure.
- Line 41: _VALIDATE_`ROUTER_DB_RETENTION_DAYS`_ before assigning
`_RETENTION_DAYS` in `host_store.py` so module import cannot fail on non-integer
values, and reject or clamp negative values so the retention cutoff used by the
ledger pruning logic around the cutoff calculation does not end up in the
future. Update the `_RETENTION_DAYS` initialization and the related
retention/cutoff path to use a safe default or explicit validation with a clear
fallback.
- Around line 173-181: The async writer in insert_call_async should not rely on
an unbounded ThreadPoolExecutor backlog, and it must avoid writing a mutable row
object that callers can change after submission. Update the host-store writer
path around insert_call_async and _writer so it snapshots each row with
dict(row) before enqueueing, and add a bounded queue/semaphore around submit to
limit pending inserts when SQLite is slow.

In `@provider_overlay.py`:
- Around line 41-44: The save_overlay path is hiding persistence failures
because host_store.set_provider_overlays() does not surface errors back to
callers. Update save_overlay() to use a store contract that returns success or
raises on write failure, and make dashboard_add_provider() handle that result so
provider creation is not reported as successful when the overlay write fails.
Keep the fix focused on save_overlay() and the
host_store.set_provider_overlays() call site so failures propagate clearly.
- Line 31: The provider ID regex in _ID_RE currently allows 41 characters
because the first character is matched separately from the quantified range.
Update the pattern so the total length matches the intended 2-40 character
validation, and keep the regex and any related validation message in sync in
provider_overlay.py.

In `@settings.py`:
- Around line 188-190: The settings update path in the host override flow is
swallowing persistence failures and still returning success. Update the code
around host_store.set_overrides() and reload() so the setter exposes write
failure explicitly (by raising or returning a status) and handle that failure
here by returning an error response instead of new, []. Keep the fix localized
to the override save/reload logic so the caller can distinguish a successful
write from a DB write that failed and left state unchanged.

In `@tests/test_auth_proxy_dashboard_full.py`:
- Around line 26-30: _reset_db setup in _use_db() only clears the cached
host_store connection before the test, so later tests can keep using the old tmp
SQLite handle after ROUTER_DB_PATH is restored. Convert _use_db into a yield
fixture or add a teardown cleanup that calls host_store.reset() again after the
test, ensuring the host_store cache is cleared both before and after each use.

In `@tests/test_settings.py`:
- Around line 20-27: The `store` fixture only resets `host_store`, but
`settings.reload()` leaves module-global overrides in memory, so teardown can
leak state into later tests. Update the `store` fixture teardown to clear the
`settings` cache/state as well as calling `host_store.reset()`, using the
existing `settings.reload()`/`settings.get()` flow and the `host_store` and
`settings` symbols to ensure each test ends with default empty settings.

In `@tests/test_sources.py`:
- Around line 669-673: The test is leaving the process-wide host_store SQLite
connection open, so later tests can keep using the cached tmp database after
ROUTER_DB_PATH is unset. Wrap the host_store usage in a try/finally and ensure
the shared connection is closed or reset in the cleanup path after
host_store.reset() and settings.reload(). Use the host_store and settings
symbols in this test to place the isolation fix where the connection is opened.

---

Outside diff comments:
In `@auth_proxy.py`:
- Around line 316-323: The persistence path in _write_issued_consumer_records
currently assumes host_store.set_consumer_keys succeeds even though it swallows
write failures, so mutations can be reported as saved when they are not durable.
Update host_store.set_consumer_keys to either raise on persistence errors or
return an explicit success status, then have _write_issued_consumer_records and
the dashboard mutation flow check that result and abort/report failure for
rotations, revocations, or consumer setting updates when persistence does not
complete.

---

Nitpick comments:
In `@tests/test_provider_overlay.py`:
- Around line 111-118: The roundtrip test in test_overlay_load_save_roundtrip no
longer covers the fail-soft behavior for malformed persisted data on the SQLite
path. Add a DB-backed assertion around po.load_overlay() that seeds an invalid
provider overlay through host_store.get_provider_overlays() (or the underlying
SQLite-backed store) and verifies it still returns {"providers": {}}. Keep the
existing save/load roundtrip, and make sure the new coverage targets the
load_overlay path that now depends on host_store.get_provider_overlays().

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5885fabc-bb65-4e95-805c-4070408b77a5

📥 Commits

Reviewing files that changed from the base of the PR and between 3c0718f and b719301.

📒 Files selected for processing (11)

• auth_proxy.py
• host_store.py
• provider_overlay.py
• serve.py
• settings.py
• shim.py
• tests/test_auth_proxy_dashboard_full.py
• tests/test_host_store.py
• tests/test_provider_overlay.py
• tests/test_settings.py
• tests/test_sources.py

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Preserve fail-closed behavior for unreadable key metadata.

host_store.get_consumer_keys() skips rows whose JSON cannot be decoded while still returning ok=True. That keeps _issued_keys_load_failed false, so a corrupted metadata row can disappear and _consumer_meta() falls back to default metadata for a statically known caller. Treat any row decode failure as ok=False, or return an inactive tombstone for that consumer.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@auth_proxy.py` around lines 213 - 215, Preserve fail-closed behavior in the
consumer key loading path: `host_store.get_consumer_keys()` currently skips
JSON-decode failures but still reports success, which leaves
`_issued_keys_load_failed` false and can let `_consumer_meta()` fall back to
default metadata. Update `get_consumer_keys()` so any unreadable row makes `ok`
false, or return an inactive tombstone for that consumer, and keep the existing
`_issued_keys_load_failed = not ok` handling in `auth_proxy.py` so corrupted
metadata cannot be silently ignored.

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Clear the settings cache in teardown as well.

host_store.reset() only closes the DB connection. The module-global overrides loaded by settings.reload() remain live, so a test that leaves an override set can affect later tests that read settings.get() without reloading. Teardown should restore empty/default settings, not just close SQLite.

One way to make teardown fully isolate state

 `@pytest.fixture`
 def store(tmp_path, monkeypatch):
     monkeypatch.setenv("ROUTER_DB_PATH", str(tmp_path / "host-store.db"))
     host_store.reset()
     settings.reload()
     yield
+    host_store.set_overrides({})
+    settings.reload()
     host_store.reset()

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	@pytest.fixture
	def store(tmp_path, monkeypatch):
	monkeypatch.setenv("ROUTER_DB_PATH", str(tmp_path / "host-store.db"))
	host_store.reset()
	settings.reload()
	yield
	host_store.reset()
	`@pytest.fixture`
	def store(tmp_path, monkeypatch):
	monkeypatch.setenv("ROUTER_DB_PATH", str(tmp_path / "host-store.db"))
	host_store.reset()
	settings.reload()
	yield
	host_store.set_overrides({})
	settings.reload()
	host_store.reset()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_settings.py` around lines 20 - 27, The `store` fixture only resets
`host_store`, but `settings.reload()` leaves module-global overrides in memory,
so teardown can leak state into later tests. Update the `store` fixture teardown
to clear the `settings` cache/state as well as calling `host_store.reset()`,
using the existing `settings.reload()`/`settings.get()` flow and the
`host_store` and `settings` symbols to ensure each test ends with default empty
settings.

[coderabbitai]

Actionable comments posted: 6

🧹 Nitpick comments (1)

host_store.py (1)

379-379: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Avoid dynamic table SQL in the seed helper.

Current callers pass constants, but this f-string still trips S608/OpenGrep and makes future misuse easy. Use a fixed SQL map instead.

Proposed fix

+_SEED_COUNT_SQL = {
+    "settings_overrides": "SELECT count(*) FROM settings_overrides",
+    "provider_overlays": "SELECT count(*) FROM provider_overlays",
+    "consumer_keys": "SELECT count(*) FROM consumer_keys",
+}
+
 def _seed_if_empty(table: str, legacy_path: str, to_rows) -> None:
@@
-            n = conn.execute(f"SELECT count(*) FROM {table}").fetchone()[0]
+            count_sql = _SEED_COUNT_SQL.get(table)
+            if count_sql is None:
+                raise ValueError(f"unsupported seed table {table!r}")
+            n = conn.execute(count_sql).fetchone()[0]

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@host_store.py` at line 379, The seed helper is building SQL with a dynamic
table name in the count query, which triggers S608/OpenGrep and leaves room for
unsafe future use. Update the seeding logic in host_store.py to avoid f-string
SQL in this path by using a fixed SQL mapping for the known tables, and have the
helper look up the appropriate constant query before calling conn.execute in the
seed/count code.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@auth_proxy.py`:
- Around line 316-326: The handlers that use _issued_consumer_records() must
fail closed when _issued_keys_load_failed is true instead of continuing into
state changes. Add an early guard in dashboard_update_consumer,
dashboard_revoke_key, and dashboard_create_key immediately after loading the
records so they return 500 before any env mutation, in-memory hash updates, or
calls that would persist a replacement set. Keep the check near the
_issued_consumer_records() flow so it is applied before any record mutation
logic.

In `@compose.yml`:
- Line 29: The Postgres password is hardcoded in the default DATABASE_URL/DSN
values, so make it configurable while preserving the dev default. Update the
compose/env defaults and any matching DSN definitions at the other referenced
locations so they all read from the same password variable or fallback value
consistently. Use the existing DATABASE_URL-related entries in the compose
configuration to keep the service env and defaults in sync.

In `@host_store.py`:
- Around line 386-388: The legacy backfill path in _seed_if_empty() is logging
success even when persistence fails because to_rows() can return False. Update
the seeding flow for set_overrides(), set_provider_overlays(), and
set_consumer_keys() so the return value from the setter is checked before
emitting the “seeded” log, and only log success when the destination was
actually persisted; otherwise treat it as a failed seed and avoid the success
message.
- Around line 428-432: The test helper truncate_all_for_tests currently
truncates tables without waiting for queued ledger writes, so stale async
inserts can repopulate the store afterward. Update truncate_all_for_tests to
drain or flush any pending ledger/background write queue before running the
TRUNCATE on calls, settings_overrides, provider_overlays, and consumer_keys,
using the relevant pool/ledger helper methods already available in host_store.py
to ensure the store is truly clean for the next test.
- Around line 123-125: The provisional ConnectionPool created in the host store
initialization path can leak if _init_schema(p) throws before _pool is assigned.
Update the initialization logic around _init_schema and _pool in host_store.py
to ensure the newly opened pool is always closed on schema init failure, using a
try/finally or equivalent cleanup path, while keeping _pool set only after
successful initialization.

In `@tests/conftest.py`:
- Around line 34-38: The blanket exception handling in host_store setup is
masking real regressions by turning all failures from host_store.reset() and
host_store.truncate_all_for_tests() into pytest.skip. Narrow the exception
handling in this conftest fixture so only genuine Postgres
connectivity/bootstrap errors are skipped, and let unexpected exceptions from
host_store.truncate_all_for_tests() propagate and fail the test instead.

---

Nitpick comments:
In `@host_store.py`:
- Line 379: The seed helper is building SQL with a dynamic table name in the
count query, which triggers S608/OpenGrep and leaves room for unsafe future use.
Update the seeding logic in host_store.py to avoid f-string SQL in this path by
using a fixed SQL mapping for the known tables, and have the helper look up the
appropriate constant query before calling conn.execute in the seed/count code.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8843c9a6-3902-47da-82d1-23e2eae6ae65

📥 Commits

Reviewing files that changed from the base of the PR and between b719301 and 06c3bdd.

📒 Files selected for processing (15)

• auth_proxy.py
• compose.yml
• host_store.py
• provider_overlay.py
• requirements.txt
• serve.py
• settings.py
• shell.nix
• shim.py
• tests/conftest.py
• tests/test_auth_proxy_dashboard_full.py
• tests/test_host_store.py
• tests/test_provider_overlay.py
• tests/test_settings.py
• tests/test_sources.py

✅ Files skipped from review due to trivial changes (1)

• requirements.txt

🚧 Files skipped from review as they are similar to previous changes (7)

• tests/test_settings.py
• serve.py
• provider_overlay.py
• shim.py
• settings.py
• tests/test_auth_proxy_dashboard_full.py
• tests/test_host_store.py

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail closed before mutating key state when the store load failed.

These handlers build records from _issued_consumer_records(), but if _issued_keys_load_failed is true, continuing can turn an unreadable/failed load into a full-table replacement; revoke/create can also mutate env/in-memory hashes before returning 500. Abort immediately after loading records.

Proposed fix pattern

     records = _issued_consumer_records()
+    if _issued_keys_load_failed:
+        return JSONResponse(status_code=503, content={"ok": False, "error": "consumer key store unavailable"})
     meta = records.get(consumer, _normalize_consumer_record(consumer, {}))

Apply the guard in dashboard_update_consumer, dashboard_revoke_key, and dashboard_create_key before any record/env mutation.

Also applies to: 2005-2006, 2056-2057, 2512-2513

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@auth_proxy.py` around lines 316 - 326, The handlers that use
_issued_consumer_records() must fail closed when _issued_keys_load_failed is
true instead of continuing into state changes. Add an early guard in
dashboard_update_consumer, dashboard_revoke_key, and dashboard_create_key
immediately after loading the records so they return 500 before any env
mutation, in-memory hash updates, or calls that would persist a replacement set.
Keep the check near the _issued_consumer_records() flow so it is applied before
any record mutation logic.

[coderabbitai]

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Make the dev Postgres password configurable.

The password is baked into both the service env and default DSNs. Keep the dev default if needed, but allow overriding it consistently.

Proposed fix

-      DATABASE_URL: ${DATABASE_URL:-postgresql://hoststore:hoststore@postgres:5432/hoststore}
+      DATABASE_URL: ${DATABASE_URL:-postgresql://hoststore:${POSTGRES_PASSWORD:-hoststore}`@postgres`:5432/hoststore}
@@
-      DATABASE_URL: ${DATABASE_URL:-postgresql://hoststore:hoststore@postgres:5432/hoststore}
+      DATABASE_URL: ${DATABASE_URL:-postgresql://hoststore:${POSTGRES_PASSWORD:-hoststore}`@postgres`:5432/hoststore}
@@
-      POSTGRES_USER: hoststore
-      POSTGRES_PASSWORD: hoststore
-      POSTGRES_DB: hoststore
+      POSTGRES_USER: ${POSTGRES_USER:-hoststore}
+      POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-hoststore}
+      POSTGRES_DB: ${POSTGRES_DB:-hoststore}

Also applies to: 85-85, 183-185

🧰 Tools

🪛 Checkov (3.3.1)

[medium] 29-30: Basic Auth Credentials

(CKV_SECRET_4)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@compose.yml` at line 29, The Postgres password is hardcoded in the default
DATABASE_URL/DSN values, so make it configurable while preserving the dev
default. Update the compose/env defaults and any matching DSN definitions at the
other referenced locations so they all read from the same password variable or
fallback value consistently. Use the existing DATABASE_URL-related entries in
the compose configuration to keep the service env and defaults in sync.

Source: Linters/SAST tools

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the file and inspect the relevant section with line numbers.
git ls-files | rg '^host_store\.py$|/host_store\.py$'
sed -n '1,220p' host_store.py | cat -n

# Find related pool/schema initialization and shutdown paths.
rg -n "_init_schema|ConnectionPool|close\(|_pool\s*=" host_store.py

Repository: genlayerlabs/unhardcoded

Length of output: 10955

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the shutdown path around the pool cleanup and any other pool lifecycle handling.
sed -n '390,450p' host_store.py | cat -n

# Quick read-only probe for psycopg_pool docs installed in the environment, if available.
python3 - <<'PY'
import inspect
try:
    from psycopg_pool import ConnectionPool
    print("ConnectionPool found")
    print("init signature:", inspect.signature(ConnectionPool))
    print("close signature:", inspect.signature(ConnectionPool.close))
except Exception as e:
    print("Unable to introspect psycopg_pool:", e)
PY

Repository: genlayerlabs/unhardcoded

Length of output: 2353

---

Close the provisional pool if schema initialization fails. host_store.py:123-125 If _init_schema(p) raises, _pool stays None and the newly opened ConnectionPool is never closed, so repeated retries can leak pool resources.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@host_store.py` around lines 123 - 125, The provisional ConnectionPool created
in the host store initialization path can leak if _init_schema(p) throws before
_pool is assigned. Update the initialization logic around _init_schema and _pool
in host_store.py to ensure the newly opened pool is always closed on schema init
failure, using a try/finally or equivalent cleanup path, while keeping _pool set
only after successful initialization.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Check legacy backfill setter failures before logging success.

set_overrides(), set_provider_overlays(), and set_consumer_keys() return False on persistence failure, but _seed_if_empty() ignores that and logs seeded even when the destination stayed empty.

Proposed fix

-            to_rows(data)
+            if not to_rows(data):
+                raise RuntimeError(f"{table} setter returned false")
             _log.info("host_store: seeded %s from %s", table, legacy_path)

Also applies to: 399-413

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@host_store.py` around lines 386 - 388, The legacy backfill path in
_seed_if_empty() is logging success even when persistence fails because
to_rows() can return False. Update the seeding flow for set_overrides(),
set_provider_overlays(), and set_consumer_keys() so the return value from the
setter is checked before emitting the “seeded” log, and only log success when
the destination was actually persisted; otherwise treat it as a failed seed and
avoid the success message.

[coderabbitai]

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Drain pending ledger writes before truncating test tables.

Queued async inserts from a prior test can land after TRUNCATE, contaminating the next test’s supposedly clean store.

Proposed fix

 def truncate_all_for_tests() -> None:
     """Test helper: wipe every table for isolation against a shared Postgres."""
+    _write_q.join()
     with _get_pool().connection() as conn:
         conn.execute("TRUNCATE calls, settings_overrides, provider_overlays,"
                      " consumer_keys")

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def truncate_all_for_tests() -> None:
	"""Test helper: wipe every table for isolation against a shared Postgres."""
	with _get_pool().connection() as conn:
	conn.execute("TRUNCATE calls, settings_overrides, provider_overlays,"
	" consumer_keys")
	def truncate_all_for_tests() -> None:
	"""Test helper: wipe every table for isolation against a shared Postgres."""
	_write_q.join()
	with _get_pool().connection() as conn:
	conn.execute("TRUNCATE calls, settings_overrides, provider_overlays,"
	" consumer_keys")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@host_store.py` around lines 428 - 432, The test helper truncate_all_for_tests
currently truncates tables without waiting for queued ledger writes, so stale
async inserts can repopulate the store afterward. Update truncate_all_for_tests
to drain or flush any pending ledger/background write queue before running the
TRUNCATE on calls, settings_overrides, provider_overlays, and consumer_keys,
using the relevant pool/ledger helper methods already available in host_store.py
to ensure the store is truly clean for the next test.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Don't turn host-store regressions into skipped tests.

Lines 34-38 catch every exception, so schema or migration bugs from host_store.truncate_all_for_tests() get reported as “Postgres unavailable” and the suite can go green. Only skip on actual connectivity/bootstrap failures; let all other exceptions fail the test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/conftest.py` around lines 34 - 38, The blanket exception handling in
host_store setup is masking real regressions by turning all failures from
host_store.reset() and host_store.truncate_all_for_tests() into pytest.skip.
Narrow the exception handling in this conftest fixture so only genuine Postgres
connectivity/bootstrap errors are skipped, and let unexpected exceptions from
host_store.truncate_all_for_tests() propagate and fail the test instead.

[jmlago]

Remaining work to make the migration complete

This PR landed the Postgres store + migrated the operator-config tables
(settings_overrides, provider_overlays, consumer_keys) and the calls
ledger, with the legacy-JSON backfill — validated against a real Postgres and the
live two-container compose stack (auth + revoke + ledger end-to-end, 28+ behave
scenarios green).

What's left for zero JSON / zero in-process operational state, in two buckets:

Bucket 1 — the remaining JSON files (clean kills; coherent with this PR's scope)

• peer_offers ← market.json (antseed marketplace).
  • The Node sidecar (antseed/merge-market.js) writes market.json today; it
    must write Postgres instead (add a pg client to Dockerfile.antseed; the
    15-min rolling window becomes INSERT ... ts + WHERE last_seen > now()-interval
    instead of the hand-rolled union).
  • sources/antseed.py._load_market reads market.json → read peer_offers.
  • Suggested table: peer_offers(peer_id, provider_id, service, price_in, price_cache_read, price_out, reputation, max_concurrency, first_seen, last_seen, fetched_at).
• buyer_status ← status-{pid}.json (antseed buyer pin/escrow status).
  • Same owner (sidecar writes it from buyer status --json); sources/antseed.py
    reads it (_pinned_peer). Small table buyer_status(pid, status_json, fetched_at)
    or per-field.

Bucket 2 — analytics + ranking state (a different form — recommend its own PR/PRs)

This is the big, higher-risk piece (hot-path ranking + the dashboard analytics
rewrite). Now unblocked because both containers reach the shared Postgres (the
router can finally persist its own state; the §8 per-pod debt goes away).

• Enrich calls with peer_id (+ cache-token breakdown, ttft, error_type)
  — prerequisite for the two below. The usage row doesn't carry peer_id today,
  so per-peer marketplace route identity is lost; thread it through _record_request.
• route_* folds → Postgres (reliability / latency / economics / cache /
  session-meter / tool-capability). They fold at the call outcome in
  llm_router_host (the router). Either persist the fold state to a
  route_state table or derive from calls by query. Touches the offer-stamp /
  ranking hot path → validate carefully (and keep folding/scoring in the policy,
  not host-side — store raw, expose raw).
• _stats + usage-history.jsonl → queries over calls. The dashboard
  analytics (_stats by_caller/provider/route/key…) and the timeframe stats read
  usage-history. Rewrite those readers to aggregate calls, then drop the
  duplicate usage-history write (_append_usage_history currently dual-writes) and
  delete the file path. Largest mechanical piece.

Cross-cutting cleanup (after the above)

• Delete the legacy JSON files post-migration (the backfill is guard-on-empty)
  and then remove migrate_legacy_json + its env paths.
• codex_accounts stays files (credentials) and .env.secrets stays (secrets) —
  not part of "no JSON"; only encrypted-at-rest would move them.
• Decide retention/indexing on calls as volume grows; consider a materialized
  route_stats/sessions if the live queries get hot.

Design notes (the law, the per-provider price map, the table sketch) live in the
working design doc the author kept; the principle throughout: store raw, derive by
query; folding/scoring is the policy's job, not the host's.