feat(coder): SQLite stores per §15.1 DDL

[kovtcharov]

Summary

Implements the ten persistent stores called out in §15.1 of docs/plans/coder-agent.mdx — the durable-state layer every Phase 1 / Phase 3 gaia-coder feature depends on (task queue, EM inbox, feedback queue, spend ledger, audit log, CI history, memory, paused-task snapshots, self-edit JSONL, learnings plain-text log). No business logic — just DDL, Pydantic row models, typed CRUD, and tests.

• Seven SQLite stores — em_inbox.db, tasks.db, feedback.db, spend.db, audit.log.db, ci_history.db, memory.db. Each ships the §15.1 DDL verbatim (grep-verifiable), a create_tables() / open_store() pair with canonical PRAGMAs (WAL, foreign_keys=ON, busy_timeout=5000), and insert_row / get_row / update_row / list_rows helpers keyed off a per-store Pydantic v2 model.
• Three append-only stores — paused_tasks (JSON-per-file), self_edits_log (JSONL), learnings_log (plain text with a strict single-line grammar). All three use Pydantic for schema validation on the write side and fail loudly on read errors.
• stores/__init__.py::create_all_stores(root) — idempotent convenience that materialises every store under one directory and returns a {store_name: Path} map.
• 30 tests in tests/coder/test_stores.py — create / round-trip / CHECK-constraint for every SQLite store, round-trip / schema-rejection for the append-only logs, smoke test for create_all_stores.
• tests/coder/conftest.py — defensive sys.modules stubs for gaia.coder.base / gaia.coder.loop so these tests stay isolated from sibling scaffold churn; becomes inert once the sibling branch stabilises.

FAISS index wiring for the memory store is intentionally out of scope — that's Phase 10.

Dependencies

Branched off feature/gaia-coder-scaffold (sibling task, already merged into this branch via rebase). The three scaffold commits (daefa1d, a160148, da051cd) at the tip are required for from gaia.coder import CoderAgent to resolve, but nothing in this PR's 12 commits depends on the scaffold beyond defensive test stubs.

Commits

One commit per store module (ten) plus one for the infra (_common.py + test conftest) and one for the package __init__ with create_all_stores:

1. feat(coder): stores — SQLite connection helpers + test harness
2. feat(coder): em_inbox store + CRUD + tests
3. feat(coder): tasks store + CRUD + tests
4. feat(coder): feedback store + CRUD + tests
5. feat(coder): spend store + CRUD + tests
6. feat(coder): audit store + CRUD + tests
7. feat(coder): ci_history store + CRUD + tests
8. feat(coder): memory store + CRUD + tests (SQL side only)
9. feat(coder): paused_tasks snapshot store + tests
10. feat(coder): self_edits_log JSONL append-only store + tests
11. feat(coder): learnings_log plain-text append-only store + tests
12. feat(coder): stores/__init__ with create_all_stores + smoke test

Test plan

• python -m pytest tests/coder/test_stores.py -xvs — 30 passed locally.
• python util/lint.py --black --isort — clean.
• Grep-verify that every §15.1 DDL string appears verbatim under src/gaia/coder/stores/:

  for t in em_inbox tasks feedback spend audit ci_history memory; do
    grep -l "CREATE TABLE $t" src/gaia/coder/stores/*.py || echo "MISSING: $t"
  done

Do not merge

Draft — blocked on the sibling feature/gaia-coder-scaffold PR landing first so the git history stays clean once both land on feature/coding-agent.

[github-actions]

Summary

Implements the ten persistent stores called out in §15.1 of docs/plans/coder-agent.mdx as pure data-layer infrastructure: seven SQLite stores (each with verbatim DDL, Pydantic row models, and typed CRUD on top of a shared _common.py helper) plus three append-only stores (JSON-per-file, JSONL, plain-text). The shape is uniform across the seven SQLite modules, tests exercise every CHECK constraint, and create_all_stores() is a nice self-contained entry point with a smoke test. Code quality is high; my main concern is a latent SQL-injection surface in the shared helper and a handful of smaller issues that are easier to fix now than after more callers wire up.

Issues Found

🟡 SQL injection surface: filter_ keys and order_by are string-interpolated (src/gaia/coder/stores/_common.py:51, 247)

_compile_where builds f"{key} IS NULL" / f"{key} = ?" directly from dict keys, and fetch_all interpolates order_by as f" ORDER BY {order_by}". Values go through ? parameters (good), but column names and ORDER BY do not. The in-repo callers today are all store modules passing constants, so it's safe right now — but every store re-exports list_rows(..., filter: Mapping[str, Any] | None) on its public surface with no documentation saying the keys must be trusted. Future callers (EM handles from em_inbox, context URLs from feedback, tool filters forwarded from CLI flags) could easily plumb untrusted input into filter={...} and quietly turn it into injection.

Two reasonable hardenings — either would be fine:

_IDENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")


def _check_identifier(name: str) -> None:
    if not _IDENT_RE.match(name):
        raise ValueError(f"invalid SQL identifier: {name!r}")


def _compile_where(filter_: Mapping[str, Any] | None) -> tuple[str, list[Any]]:
    """Build a ``WHERE`` clause and parameter list from an equality filter.

    Empty/``None`` filter returns ``("", [])`` so callers can concatenate
    unconditionally. Values are passed as positional parameters — no string
    interpolation — so this is safe against injection. Column names are
    validated against ``_IDENT_RE`` so a tainted ``filter_`` key cannot
    inject SQL.
    """
    if not filter_:
        return "", []
    parts = []
    params: list[Any] = []
    for key, value in filter_.items():
        _check_identifier(key)
        if value is None:
            parts.append(f"{key} IS NULL")
        else:
            parts.append(f"{key} = ?")
            params.append(value)
    return " WHERE " + " AND ".join(parts), params

Apply the same _check_identifier to every column in update's patch and to fetch_all's order_by (split on commas first, then validate each token / direction). Cheap, and turns a latent landmine into a loud ValueError.

🟡 Test conftest swallows any import error (tests/coder/conftest.py:26)

def _try_real_import() -> bool:
    try:
        import gaia.coder  # noqa: F401
    except Exception:
        return False
    return True

This is exactly the silent-fallback pattern CLAUDE.md prohibits: a broken gaia.coder.stores module (e.g., a typo in em_inbox.py) will raise, _try_real_import returns False, stubs get installed, and the tests run against those stubs — masking the regression. Now that the sibling scaffold has landed (per your PR description), the rationale for the fallback is gone. Prefer one of:

• Delete the stub machinery entirely; let import gaia.coder fail loudly.
• Or narrow the exception to the specific missing-name ImportError you observed during the overlap window, and add a visible warnings.warn(...) when stubs are installed so a real failure is never silent.

def _try_real_import() -> bool:
    """Attempt the real import; return True on success.

    Narrow catch: we only want to paper over the sibling-branch window where
    ``gaia.coder.base`` / ``gaia.coder.loop`` are referenced before they
    exist. Any other import failure is a real regression and must surface.
    """
    try:
        import gaia.coder  # noqa: F401
    except ImportError as e:
        msg = str(e)
        if "gaia.coder.base" in msg or "gaia.coder.loop" in msg:
            return False
        raise
    return True

🟢 learnings_log canonical format doesn't escape embedded " (src/gaia/coder/stores/learnings_log.py:68)

format_entry writes f'"{entry.observation}"' unchanged, and _LINE_RE parses with "(?P<observation>.*)". Greedy matching usually recovers, but an observation ending in a literal " (or containing " → exactly) is a round-trip footgun that no test covers. Either reject " in the observation (same spirit as the newline rule) or escape it:

    @field_validator("observation")
    @classmethod
    def _single_line_no_quote(cls, v: str) -> str:
        if "\n" in v or "\r" in v:
            raise ValueError("observation must be a single line")
        if '"' in v:
            raise ValueError("observation must not contain double quotes")
        return v

Plus a small test covering 'he said "hi"' to lock the behavior.

🟢 self_edits_log concurrency comment overstates safety (src/gaia/coder/stores/self_edits_log.py:43)

concurrent appends on POSIX are safe for records shorter than PIPE_BUF (4096 bytes on Linux/macOS).

Real self-edit records easily exceed 4KB once files: list[str] and the seven review-pass verdicts are filled in. O_APPEND write atomicity applies to regular files with write(2), but Python's buffered file objects make this non-trivial. I'd either (a) add fh.flush() + os.fsync() and rely on O_APPEND-per-write with a single write() of the full line, or (b) soften the comment to say concurrent use is not supported and callers must serialize. Either is fine; the current docstring promises something it doesn't deliver.

🟢 memory.py DDL comment is misleading (src/gaia/coder/stores/memory.py:42)

-- One table per topic for simple cases; in production these are views over a single `memory` table
-- keyed by topic for FAISS co-location.

The DDL ships exactly one table called memory — there are no per-topic tables and no views. The comment reads like it was copy-pasted from a design doc. Trim to the one-line "single memory table keyed by topic; one FAISS index per topic in Phase 10" that matches the actual schema.

🟢 __init__.py mixes Dict and dict (src/gaia/coder/stores/init.py:41, 74, 91)

from typing import Dict plus Dict[str, Path] in the return annotation, alongside PEP-585 dict[str, str] in _SQLITE_LAYOUT. Pick one — every other new module in this PR already uses lowercase dict:

from __future__ import annotations

from pathlib import Path

…then change -> Dict[str, Path] → -> dict[str, Path] and drop the Dict import.

Strengths

• Uniform shape across seven SQLite modules. Every store exposes DDL, create_tables, open_store, insert_row, get_row, update_row, list_rows with an identical signature. A reader who understands em_inbox.py understands tasks.py, feedback.py, spend.py, etc. for free — exactly the right move for a data layer that future phases will grow into.
• Test coverage is thoughtful, not just wide. The CHECK-constraint tests (test_em_inbox_check_constraints, test_feedback_check_constraints, test_memory_check_constraints) prove the DDL's CHECK clauses actually reject invalid enums — catching the common bug where a constraint is declared but never validated. The AUTOINCREMENT-id strict-monotonicity test in test_audit_check_constraints is a nice touch.
• Path-traversal defence in paused_tasks._validate_task_id + atomic .tmp-then-rename in write_snapshot are both exactly right for a directory-of-files store — and there's a test for the traversal rejection.

Verdict

Approve with suggestions. The PR is already merged; nothing here is a blocker. The SQL-identifier hardening in _common.py and the narrowed-catch in tests/coder/conftest.py are the two items worth a quick follow-up — both are small, both close real latent failure modes rather than hypothetical ones. The others (learnings_log quote escaping, self_edits_log docstring, memory DDL comment, Dict/dict mix) are low-cost cleanups to fold into the next coder-stores commit.