feat(sdk/io): add PostgreSQLFileStore for persistent EventLog

[StatPan]

Closes #2903

Summary

• Adds PostgreSQLFileStore — a FileStore backed by PostgreSQL for environments where the local filesystem is unavailable or ephemeral (Cloud Run, containers without persistent volumes)
• Adds file_store: FileStore | None = None parameter to ConversationState.create() and LocalConversation.__init__() so callers can inject any FileStore implementation

Motivation

In ephemeral environments, LocalFileStore requires a filesystem path that doesn't persist across requests. The SDK falls back to InMemoryFileStore, losing all event history. Without a persistent EventLog, full multi-turn resume — including tool_call / tool_result events — is impossible.

Changes

File	Change
openhands/sdk/io/postgresql.py	New: PostgreSQLFileStore implementation
openhands/sdk/io/__init__.py	Conditional re-export (no hard asyncpg dep)
openhands/sdk/conversation/state.py	file_store param on ConversationState.create()
openhands/sdk/conversation/impl/local_conversation.py	file_store param on LocalConversation.__init__()
pyproject.toml	Optional extra [postgresql] = asyncpg>=0.29.0
tests/sdk/io/test_postgresql_filestore.py	10 unit tests (mocked asyncpg)

All changes are backward-compatible: file_store=None preserves existing LocalFileStore/InMemoryFileStore selection logic.

Design notes

• Background event-loop thread + asyncpg pool: bridges the sync FileStore interface to async asyncpg without creating a new loop per call and without interfering with the host app's event loop
• pg_advisory_lock-based locking: safe for concurrent writers across multiple processes and instances (Cloud Run, autoscaled deployments). Lock key = SHA-256(namespace + "\x00" + path)[:8] as signed int64. Uses pg_try_advisory_lock with 50 ms polling and a dedicated connection per lock() call to keep the session-level advisory lock lifecycle self-contained.
• Optional import: asyncpg is not a hard dependency. PostgreSQLFileStore is re-exported from openhands.sdk.io only when asyncpg is installed; __init__.py uses try/except ImportError

Usage

from openhands.sdk.io.postgresql import PostgreSQLFileStore

store = PostgreSQLFileStore(
    dsn="postgresql://user:pass@host:5432/db",
    namespace=str(conversation_id),  # isolates events per conversation
)
# New conversation:
conv = LocalConversation(agent=agent, workspace=workspace, file_store=store)

# Resume across requests (same namespace → SDK reads full event history from DB):
conv = LocalConversation(agent=agent, workspace=workspace, file_store=store)

Docs

OpenHands/docs#465

🤖 Generated with Claude Code

[xingyaoww]

@OpenHands /codereview /github-pr-review

---

🤖 This comment was automatically posted by the pr-triage skill (OpenHands) on behalf of a maintainer. See PR #2906 for skill details.

[openhands-ai]

I'm on it! xingyaoww can track my progress at all-hands.dev

[xingyaoww]

🟡 Acceptable — useful feature, backward-compatible plumbing, but has a real SQL injection vector and testing gaps that should be addressed before merging.

This review was generated by an AI assistant (OpenHands) on behalf of the maintainers.

[openhands-ai]

Summary

I performed a code review on PR #2902 ("feat(sdk/io): add PostgreSQLFileStore for persistent EventLog") as requested by the /codereview /github-pr-review comment.

What was done:

• ✅ Retrieved the PR diff against the base branch (main) to understand all changes
• ✅ Reviewed all 8 changed files thoroughly (postgresql.py, init.py, state.py, local_conversation.py, both pyproject.toml files, test file, uv.lock)
• ✅ Checked asyncpg dependency against the 7-day freshness supply-chain guardrail (uploaded 2025-11-24, well past the window)
• ✅ Assessed whether the PR falls in the eval-risk category (it does not — no agent behavior changes)
• ✅ Posted a single COMMENT review with 7 inline comments following the repo's code review guidelines and the github-pr-review skill format
• ✅ Used priority labels (🔴🟠🟡) on every comment
• ✅ Included AI disclosure in the review body
• ✅ No code changes were made (review-only task)

Key findings posted:

1. 🔴 Critical: SQL injection via unvalidated table parameter in f-string SQL queries
2. 🟠 Important: Resource leak (thread + event loop) if constructor's _setup() fails
3. 🟠 Important: DDL (CREATE TABLE) runs on every instantiation, causing lock contention at scale
4. 🟠 Important: Tests are 100% mocked — they verify the mock's Python logic, not actual SQL correctness
5. 🟡 Suggestion: Connection-per-lock is expensive (bypasses pool)
6. 🟡 Suggestion: Missing __enter__/__exit__ context manager support
7. 🟡 Suggestion: Undocumented file_store + persistence_dir interaction

The review was posted as a COMMENT (not APPROVE) because there are issues at "important" level or higher that should be addressed before merging.

[StatPan]

Thanks for the thorough and speedy review! Didn't expect feedback this fast — almost like you had a robot reading it for you. Oh wait. 😄

All points are well taken. Here's my plan:

Will fix in next push:

1. SQL injection on table — Yep, that's a real one. Adding identifier regex validation in the constructor. Considered asyncpg's quote_ident but a strict ^[a-zA-Z_][a-zA-Z0-9_]*$ check is simpler and doesn't need a live connection.

2. Resource leak on _setup() failure — Good catch. Will wrap future.result() in try/except to stop the loop and join the thread on failure.

3. Context manager support — Easy win, adding __enter__/__exit__.

4. DDL in data path — Will extract to a ensure_schema() classmethod. Constructor will skip DDL by default; callers opt in or run it once during deployment.

Will address with a note / follow-up:

5. Connection-per-lock — This was intentional: pool connections release advisory locks when returned to the pool, which breaks the lock lifecycle. I'll add a docstring explaining the trade-off. Open to revisiting if there's a pattern I'm missing.

6. Tests — Fair point. Will relabel existing tests as contract tests and add an integration test gated behind @pytest.mark.integration + POSTGRES_DSN env var.

7. file_store + persistence_dir interaction — Will add a docstring note clarifying the behavior when both/neither are provided.

Working on the fixes now — should have a push up today. Thanks again for the detailed feedback!

[StatPan]

Hi @xingyaoww, all 7 review items have been addressed in cfe7473:

1. SQL injection — _VALID_IDENT_RE regex validation on table name
2. Resource leak on setup failure — try/except around future.result(), cleans up loop + thread
3. Connection-per-lock — docstring explaining why pool isn't used (advisory lock lifecycle)
4. Context manager — __enter__/__exit__ added
5. DDL in data path — extracted to ensure_schema() classmethod + auto_create_schema flag
6. Mock-only tests — relabeled as contract tests, added @pytest.mark.integration suite with real round-trip coverage
7. file_store + persistence_dir docs — docstring on create() clarifying the interaction

Would appreciate a re-review when you have a moment. Thank you!

[StatPan]

@mamoodi @cbagwell Could you please review this PR when you get a chance? 🥺\n\nAll checks are now passing (including pre-commit), and the branch is merge-ready. Thank you! 🙏

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @StatPan, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

[StatPan]

Resolved the merge conflicts by merging the latest upstream/main into feat/postgresql-filestore and pushed the update in 2570149.
Local verification:

• uv lock --check
• uv run pytest tests/sdk/io/test_postgresql_filestore.py (12 passed, 6 skipped)
• uv run pre-commit run --files ... for the PostgreSQLFileStore-related files