Skip to content

Complete native PostgreSQL and SQLite repositories - #129

Merged
dborovcanin merged 2 commits into
mainfrom
codex/complete-db-repositories
Oct 13, 2026
Merged

dborovcanin merged 2 commits into
mainfrom
codex/complete-db-repositories

Conversation

@arvindh123

@arvindh123 arvindh123 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Closes #126. Completes the PostgreSQL/SQLite repository migration started in #119 in this single follow-up PR.

All production database operations now use typed domain operations and private native PostgreSQL/SQLite adapters. Business validation, authorization evaluation, transaction orchestration, cache barriers, and audit/outbox behavior stay shared. The runtime SQL translator and generic query compatibility API are removed; test fixtures supply explicit SQL for each backend.

  • Port identity, credentials/session/email/OAuth/signing-key storage, tenants, authorization/guardrails/listings, PKI, bootstrap, audit/outbox, cleanup, custom endpoints, and runtime queries.

  • Reuse one canonical grant expansion and scope/ceiling implementation per backend. Preserve caller-owned transactions, tenant/entity lock ordering, SQLite writer coordination, and PKI retry savepoints.

  • Fix nullable resource sorting in both directions with deterministic pagination. Add regressions for actual resource/outbox rollback, nested savepoint recovery, JSON semantic parity, and resource/entity revisions.

  • Preserve PostgreSQL refresh replay revocation under concurrent exchange and post-commit audit writes: use an entity guard compatible with audit foreign-key locks, with a deterministic regression.

  • Enforce the native storage boundary and migration pairing/schema parity. Preserve applied migrations and public API contracts. Update developer guidance and the persistent, single-process SQLite quick start.

The environment example includes a PostgreSQL host URL and port, SQLite URL alternatives, pool options, and admin/encryption guidance, with the current Make/Compose limitations explained. Both URL examples parse with Atom's dotenvy parser; Docker Compose configuration validation also passes.

Review follow-up fixes share credential-ceiling interpretation and identical certificate-profile/signing-key SQL while keeping backend-specific execution private. Test fixtures can share genuinely identical SQL without bringing back runtime translation. The latest main-branch OAuth changes are preserved on both native adapters, including browser-bound exchange codes and verified-email responses. API documentation and regression tests now cover Apple's POST callback and nullable OAuth email responses.

Security dependency updates align both package manifests and workspace overrides on sharp 0.35.5, with regenerated frozen lockfiles. UI and docs production audits pass under their existing policies; no audit exceptions were added. The docs audit still reports pre-existing advisories covered by its existing exception policy, so a passing audit is not a claim of zero advisories.

The follow-up review fixes compare SQLite tenant grant conditions as JSON rather than text and add a zero fallback to authority expiry metrics. The new tenant regression failed on the original { } comparison and passes after the fix, while actual conditional grants remain excluded. The expiry fallback is defensive: existing completed-material constraints already prevent active/retiring authorities from having NULL expiry dates.

Validation on final signed head a921ed8354614b4c8094d6f6c11f36b4bb832392:

  • Rust CI passed formatting, Clippy, database-boundary/schema-parity enforcement, full included unit/integration suites on fresh PostgreSQL databases and SQLite files, PKCS#11 backup recovery, and the real PKI smoke test.

  • API/contract CI passed OpenAPI validation, protobuf lint/generated docs, runtime-backed HTTP contracts, GraphQL schema comparison, frozen contracts/migrations, and contract-gate regressions.

  • Frontend CI passed UI and docs lint, tests, production builds, and audits.

  • Local follow-up verification passed all 13 SQLite tenant-listing tests, Rust formatting/Clippy, and the database-boundary/schema-parity check. The new regression exercises compact and whitespace-formatted empty JSON, plus a nonempty condition that must continue to fail closed. The API CI job passed after retrying a compiler-download failure caused by a GitHub API rate limit; no code change was needed for that retry.

Additional live API smoke validation (2026-10-13) on a921ed8: started real servers against isolated PostgreSQL 18.4 and persistent SQLite databases; verified admin/user logins, tenant/user/resource creation, role assignment enabling resource reads, immediate revocation using the same user token, and preservation of an unrelated grant. Restarted both servers and verified persisted data, role state, pre-restart sessions, unchanged signing keys, fresh logins, and new writes. SQLite also rejected a second process opening the same database. For PostgreSQL upgrade coverage, created representative data with main 775a326, copied it with pg_dump/pg_restore, then started and restarted the PR server against the restored copy: all six migration checksums, existing sessions, signing keys, records, and grant/revocation decisions were preserved. These were live HTTP/GraphQL tests with generated data, not browser UI testing or a production database snapshot.

The repository requires verified signatures and linear history. The baseline consolidation and this follow-up commit are GitHub-signed. The original development history is preserved on codex/pr129-before-signing. No repository protection rules were changed.

Coverage limits are explicit: the two live AMQP suites (m27_live_amqp_delivery, m28_amqp_mtls_local_principal) compile but were not executed because their external broker/mTLS setup is not provisioned, matching existing CI exclusions. PostgreSQL lock-observation cases (three object-coordination cases, two entity-authorization cases, and the audit foreign-key lock regression), the process-replacement test, and synthetic PKI serial-collision injection remain PostgreSQL-specific. SQLite is covered by its writer/ownership, concurrent mutation, restart, and shared savepoint regressions; these PostgreSQL-specific skips are not claimed as SQLite passes. SoftHSM backup recovery is verified on PostgreSQL; the provider path runs on both backends.

SQLite-to-PostgreSQL data transfer is outside this issue: changing DATABASE_URL selects a backend and does not move existing data. No applied migrations are changed.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
atom-docs 5258e58 Commit Preview URL

Branch Preview URL
Oct 12 2026, 07:48 PM

@arvindh123
arvindh123 marked this pull request as ready for review September 30, 2026 22:50
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T22:55:22.941695Z 69ad0ce Draft marked ready
ℹ️ 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.

@arvindh123
arvindh123 force-pushed the codex/complete-db-repositories branch from 772d541 to e000a17 Compare October 6, 2026 14:00
Completes #126, including shared SQL cleanup, current-main OAuth integration, security dependency updates, and API contract parity.

Signed consolidation of PR #129 at d709c25. File contents are unchanged from that CI-validated tree. Original development history is preserved on codex/pr129-before-signing.
@arvindh123
arvindh123 force-pushed the codex/complete-db-repositories branch from d709c25 to 5258e58 Compare October 12, 2026 19:45
@felixgateru

felixgateru commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review and test summary

Review

  • Checked whether the Postgres side carried over faithfully by pulling all 750 SQL statements out of the pre-refactor code and matching them against the new adapters. 733 are unchanged. The other 17 are reformatting, moved code, or deliberate changes (for example, the entity guard now uses FOR NO KEY UPDATE to avoid the audit-insert deadlock).
  • Compared every Postgres/SQLite adapter pair. Timestamps are bound in the fixed-width …Z format throughout, UUID lists and JSON are encoded consistently, and the removed row and advisory locks rely on BEGIN IMMEDIATE as intended.
  • The auth and authorization paths match across both backends: session and API-key loading, the canonical grant expansion, authorized listings, the scoped-token limit on listings, and the audit tenant filter.
  • Nothing blocking. Two small SQLite-only issues, neither reachable with current data:
    • Tenant listing compares conditions as raw text (src/tenants/repo/storage/sqlite.rs:1257): it uses g.conditions = '{}' where every other reader uses atom_json_eq. A grant stored as { } would hide the tenant. It fails closed, but it's inconsistent with the other readers.
    • Authority expiry metric can fail on SQLite (src/certs/lifecycle/repo/sqlite.rs:193): max(MIN(...), 0.0) returns NULL when all not_after values in a group are NULL. Postgres GREATEST returns 0 there. seconds is a plain f64, so the decode error would abort the lifecycle sweep. The column is nullable on SQLite only, which is an existing schema gap. COALESCE(..., 0.0) fixes it.
  • Minor notes:
    • SQLite reveal_shared_key takes a write lock to read.
    • SQLite LIKE ignores case only for ASCII, unlike Postgres ILIKE.
    • search_pattern doesn't escape %/_. This is not new in this PR.

Tests run locally

  • cargo fmt --check, cargo clippy --locked -- -D warnings, cargo test --no-run --locked, and the DB boundary/schema parity check all pass.
  • Full suite on PostgreSQL 18.4 with a fresh database per test binary: library tests (428) and 70 integration test binaries pass.
  • Full suite on SQLite with a fresh file per test binary: library tests (428) and 71 integration test binaries pass.
  • m41_pki_est passes on both backends with the pinned GlobalSign EST client (v1.0.7).
  • The UI and docs both build with the bumped dependencies.

felixgateru

This comment was marked as duplicate.

Compare grant conditions structurally so whitespace in an empty JSON object cannot hide an authorized tenant. Add a regression that also keeps conditional grants excluded. Use a zero fallback for SQLite authority expiry metrics to match PostgreSQL null handling.
@dborovcanin
dborovcanin merged commit 526edd1 into main Oct 13, 2026
8 of 9 checks passed
@dborovcanin
dborovcanin deleted the codex/complete-db-repositories branch October 13, 2026 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complete PostgreSQL/SQLite repository migration and review fixes in one PR

3 participants