Repository navigation
Store hosted keys and sessions in Supabase for Render - #46
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4a8f113b7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif pool is not None: | ||
| store = PostgresSessionStore(pool) |
There was a problem hiding this comment.
When DEEP_AGENTIC_CORE_MCP_DATABASE_URL is set without Redis—the exact configuration introduced in render.yaml—this path serves HTTP workflow state from PostgreSQL, while the repository's hosted deployment contract explicitly requires Redis-backed user-scoped workflow state. Either retain Redis for this path or update the governing contract as part of the change; otherwise the default Render deployment violates a documented storage invariant.
AGENTS.md reference: AGENTS.md:L125-L128
Useful? React with 👍 / 👎.
| if any(count >= limit for count, (_, limit, _) in zip(counts, buckets, strict=True)): | ||
| return False |
There was a problem hiding this comment.
Once the global counter reaches 100, a request from a previously unseen peer has already inserted its ip: bucket before this return, and exiting the connection context commits that row. Because no code deletes expired signup counters, requests from distinct client addresses—including globally denied requests—make mcp.signup_counters grow permanently and can eventually exhaust the hosted database; avoid inserting the client bucket after global denial or periodically delete expired buckets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings remain across documentation, error handling, pool lifecycle and capacity, counter retention, and CI coverage.
3 open issues
What changed in this PR
Adds Supabase/PostgreSQL persistence for hosted keys, sessions, and signup limits, with updated Render deployment support.
Changes:
- Adds PostgreSQL stores, schema, dependencies, and integration tests.
- Updates transport, signup handling, and key conflict behavior.
- Updates Render configuration, documentation, and test fixtures.
| File | Summary |
|---|---|
uv.lock |
Locks PostgreSQL dependencies. |
tests/test_transport_http.py |
Updates transport fixture isolation. |
tests/test_signup.py |
Updates signup and conflict tests. |
tests/test_postgres_store.py |
Adds PostgreSQL integration coverage. |
src/deep_agentic_core_mcp/transport_http.py |
Selects database backends and configures hosts. |
src/deep_agentic_core_mcp/signup.py |
Generalizes signup storage handling. |
src/deep_agentic_core_mcp/services/postgres_store.py |
Implements PostgreSQL backends. |
src/deep_agentic_core_mcp/services/api_keys.py |
Adds backend-neutral key conflict handling. |
render.yaml |
Configures Supabase-backed deployment. |
pyproject.toml |
Adds PostgreSQL dependencies. |
docs/remote-hosting.md |
Documents remote hosting configuration. |
deploy/supabase/schema.sql |
Defines the private schema and permissions. |
CHANGELOG.md |
Records the changes. |
Review effort: Lite
| API keys (SHA-256 digests only), per-user workflow sessions and signup limits | ||
| are stored in Supabase PostgreSQL when `DEEP_AGENTIC_CORE_MCP_DATABASE_URL` is | ||
| set. Redis is used for sessions instead only if `DEEP_AGENTIC_CORE_MCP_REDIS_URL` | ||
| is also set. |
There was a problem hiding this comment.
Fixed in c51e9c5: the table now lists DEEP_AGENTIC_CORE_MCP_DATABASE_URL, marks Redis and the database as alternatives, and the intro and expiry text cover both backends.
| return ConnectionPool( | ||
| url, | ||
| min_size=1, | ||
| max_size=max_size, | ||
| timeout=10, |
There was a problem hiding this comment.
Fixed in e10d533: the pool's max_size is now max_concurrent_total + 4, so every admitted request gets a connection with headroom for auth/signup queries.
| database_url = os.environ.get("DEEP_AGENTIC_CORE_MCP_DATABASE_URL") | ||
| pool: Any = None | ||
| if database_url and (store is None or (key_store is None and api_keys is None)): | ||
| pool = connect(database_url) |
There was a problem hiding this comment.
Fixed in e10d533: the lifespan closes the pool in a finally block (including failed startup), and create_app closes it if construction fails after opening it. Covered by new tests.
…hutdown A session transaction holds a pool connection for a whole tool call, so the pool now allows max_concurrent_total plus headroom for auth and signup queries. The app owns the pool: lifespan closes it, and so does a failed create_app.
Summary
DEEP_AGENTIC_CORE_MCP_DATABASE_URL.deploy/supabase/schema.sqlcreates a privatemcpschema (not exposed through the Data API), RLS, and a least-privilegemcp_servicerole.KeyConflictError.main).Testing
pytest: 251 passed against PostgreSQL 16 in Docker (DEEP_AGENTIC_CORE_MCP_TEST_DATABASE_URL); 245 passed, 6 skipped without it.Dockerfile.renderand ran it against PostgreSQL asmcp_service:/healthzok, unauthenticated/mcp401, signup 201, authenticated tool call 200.