Skip to content

feat(cpp): SQLite integration and gaia::Database - #2816

Open
kovtcharov wants to merge 1 commit into
mainfrom
cpp/sqlite-database
Open

feat(cpp): SQLite integration and gaia::Database#2816
kovtcharov wants to merge 1 commit into
mainfrom
cpp/sqlite-database

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

The C++ framework had no database. SessionStore and AllowedToolsStore persist as loose JSON files, so agent memory, scratchpad tables, and the email agent's state store had nothing to build on — and no C++ agent could do full-text search at all. This lands SQLite as a first-class part of gaia_core: #include <gaia/database.h> and you get an RAII connection, prepared statements, transactions, ordered schema migrations, and FTS5, with no package to install and no system SQLite to find. It ships with no consumer by design — it is the foundation Phase 5 and the email milestone build on.

The amalgamation is vendored rather than fetched, deliberately: a distro SQLite may lack FTS5 or carry different compile-time defaults, and that only surfaces at query time on one platform. Vendoring makes Windows, Linux, and macOS compile byte-identical sources with identical flags. Because a missing SQLITE_ENABLE_FTS5 builds fine and fails only when someone runs a query, three tests exist purely to catch that — verified by building with the flag removed: the build still succeeded and all three failed with no such module: fts5.

CI build-time impact (issue asks for it) — clang -O3, Apple M4; the delta is one extra translation unit:

Measurement main this PR delta
Serial build (-j1), whole cpp/ tree 82.6s 92.3s +9.7s (+12%)
sqlite3.c alone (best of 3) 9.2s one TU, the longest in the build
database.cpp + test_database.cpp 1.7s
Parallel wall-clock (-j4, runner width) +3–7s, run-to-run noise exceeds the signal

Incremental builds are unaffected — sqlite3.c only recompiles when the amalgamation or its flags change. Repo grows 10.2 MB in the working tree (~2.6 MB compressed in git). One-time cost worth flagging: the FetchContent cache key is hashFiles('cpp/CMakeLists.txt'), which this PR changes, so the first run on each OS re-fetches json/httplib/ftxui/gtest.

Three things a reviewer may want to know the reasoning for: the version file is SQLITE_VERSION.txt rather than VERSION because the directory is on the include path and a case-insensitive filesystem resolves #include <version> to it, breaking every TU in gaia_core (this bit during development); the amalgamation is an OBJECT library consumed via $<TARGET_OBJECTS:> rather than target_link_libraries so it stays out of gaia_core's link interface and install(EXPORT) has nothing extra to resolve; and .gitattributes marks the vendored sources -diff so 9.5 MB stays out of every diff (the reviewable diff is 2,554 lines).

Closes #2793

Test plan

  • cmake -S cpp -B cpp/build -DGAIA_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release && cmake --build cpp/build --parallel && ctest --test-dir cpp/build519/519 pass (463 pre-existing + 56 new), zero new warnings
  • FTS5 flag actually takes: virtual table creation, MATCH, bm25() ranking, prefix queries, and update/delete reindexing
  • Negative control — rebuild with SQLITE_ENABLE_FTS5 removed: build still succeeds, all 3 FTS5 tests fail with no such module: fts5
  • CRUD via prepared statements; typed binds for every storage class including NULL, blobs with embedded NULs (256-byte round-trip), text with an embedded NUL, and empty-blob-is-not-NULL
  • Transactions: commit, implicit rollback, rollback on a throwing scope, explicit rollback, IMMEDIATE, and nested savepoints
  • Migration from an older on-disk schema (v1 → v3) preserving existing rows; idempotent re-run; resume after a partially-applied step; failing step rolls back and keeps the old version; malformed step lists and downgrades refused
  • busy_timeout with a concurrent writer thread — the waiter blocks ~300 ms and succeeds; counterpart test with busyTimeoutMs = 0 fails immediately, proving the timeout did the work
  • Install round-trip: cmake --install then a find_package(gaia_core) consumer that opens a database, creates an FTS5 table, and queries it — links and runs (sqlite=3.53.4 fts5=1 hits=1) without sqlite3.h on its include path
  • BUILD_SHARED_LIBS=ON shared-library build links cleanly
  • Upstream authenticity: SHA3-256 of the downloaded zip matches sqlite.org's published manifest (628a44cf…); both checksums recorded in SQLITE_VERSION.txt
  • No handle leak when construction throws after sqlite3_open_v2 succeeds — leaks reports 0; negative control with the fix removed reports 61 leaks / 161,680 bytes

The C++ framework had no database — SessionStore and AllowedToolsStore
persist as loose JSON files, and there was nothing for agent memory,
scratchpad tables, or the email agent's state store to build on. This adds
one.

SQLite ships as the vendored amalgamation under cpp/third_party/sqlite/,
compiled straight into gaia_core with SQLITE_ENABLE_FTS5. There is
deliberately no find_package fallback: a distro SQLite may lack FTS5 or
carry different compile-time defaults, and that only surfaces at query time
on one platform. Vendoring makes all three platform builds identical.

gaia/database.h exposes RAII types over it — Database (WAL + busy_timeout
applied at open, serialized threading mode), Statement with typed binds and
column accessors, a Transaction scope guard that rolls back unless
committed, and an ordered migration helper mirroring
MemoryStore._migrate_schema_locked. The migration version lives in
PRAGMA user_version so no table is imposed on the caller's schema, and each
step runs inside a transaction that also stamps the new version — a step
that throws rolls back completely and the stored version does not advance.

Every failure raises DatabaseError carrying the SQLite message plus the
database path and offending statement. Nothing is swallowed.

Ships with no consumer; Phase 5 and the email milestone build on it.

Notes:
- The version file is SQLITE_VERSION.txt, not VERSION: the directory is on
  the include path, and on a case-insensitive filesystem a file named
  VERSION is picked up as libc++'s <version> header.
- The amalgamation is an OBJECT library consumed via $<TARGET_OBJECTS:>
  rather than target_link_libraries, keeping it out of gaia_core's link
  interface so install(EXPORT) has nothing extra to resolve.
- .gitattributes marks the vendored sources linguist-vendored and -diff so
  a 9.5 MB amalgamation stays out of every diff.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve

This adds a vendored SQLite amalgamation and a RAII gaia::Database wrapper (connection, prepared statements, transactions, and versioned schema migrations) to the C++ SDK, with a comprehensive GoogleTest suite and API docs. The design is exemplary and lands squarely on GAIA's "fail loudly, no silent fallbacks" philosophy — no find_package fallback, WAL that can't take is a loud error rather than a mystery SQLITE_BUSY later, and every SQLite failure raises DatabaseError carrying the code, db path, and offending SQL.

The one thing worth a glance before merge is minor and non-blocking: the two security-hardening connection settings (disabling extension loading, enabling defensive mode) don't check their return code, while the adjacent busy_timeout call does — a small consistency gap against the fail-loudly rule, not a correctness bug.

Real-world evidence

N/A — this is a C++ SDK library, not a Python Agent-UI / CLI / MCP / HTTP surface, so none of the rubric's evidence surfaces apply. The appropriate proof for this layer is ctest, and the PR ships a thorough cpp/tests/test_database.cpp (lifecycle, every storage class incl. embedded-NUL blobs/text, WAL/busy-timeout under a real concurrent-writer thread, the full migration chain incl. partial-apply resume and downgrade refusal, and FTS5/BM25). No evidence bundle was produced for this run; verdict rests on static review plus the included test coverage.

🔍 Technical details

🟢 Minor — unchecked return on the two hardening sqlite3_db_config calls (cpp/src/database.cpp, constructor)

busy_timeout checks its rc and fail()s, but the two sqlite3_db_config calls that disable extension loading and enable defensive mode ignore theirs. These opcodes don't realistically fail on a freshly-opened handle, so this is cosmetic — but a silently-failing security config is exactly the kind of thing the fail-loudly rule targets, and checking it keeps the constructor internally consistent. For example:

        if (sqlite3_db_config(db_, SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, 0, nullptr) != SQLITE_OK) {
            fail("cannot disable extension loading", sqlite3_errcode(db_));
        }
        // Defensive mode: SQL cannot corrupt the schema or write FTS5 shadow tables.
        if (sqlite3_db_config(db_, SQLITE_DBCONFIG_DEFENSIVE, 1, nullptr) != SQLITE_OK) {
            fail("cannot enable defensive mode", sqlite3_errcode(db_));
        }

(Only wire this if fail()/sqlite3_errcode are reachable at that point; otherwise a plain rc-check + throw matching the surrounding style is fine. Your call — it's a nit.)

Strengths

  • Fail-loudly done right. The WAL-mode verification (cpp/src/database.cpp) reads back PRAGMA journal_mode and throws with an actionable message ("network mount? set Options::walMode = false") rather than letting a silently-non-WAL database surprise the caller later. Same spirit in the empty-path rejection, the downgrade refusal in migrate(), and DatabaseError carrying code/dbPath/sql.
  • Clean dependency hygiene. sqlite3.h is forward-declared in the public header so consumers of gaia_core never need it on their include path; the OBJECT-library + $<TARGET_OBJECTS:> approach keeps SQLite out of the install export set and its warnings out of -Wall/-W4, matching how the file already handles httplib/json.
  • Migrations mirror the Python MemoryStore model (ordered steps, PRAGMA user_version, idempotent addColumnIfMissing, per-step transaction that also stamps the version) — so a step that dies half-way is safely re-runnable, and the tests prove exactly that (MigrateResumesAfterAPartiallyAppliedStep).
  • Test coverage is genuinely adversarial, not box-checking: embedded-NUL round-trips, SQLITE_DQS=0 enforcement, real two-thread busy-timeout contention with a timing assertion, and an FTS5 BM25-ranking check that a stub module couldn't pass.
  • Docs and vendoring are honestSQLITE_VERSION.txt records both checksums, the README calls out the SHA3-256 (not SHA-256) gotcha and the <version> header collision, and .gitattributes keeps the 9 MB amalgamation out of every diff.

@kovtcharov

Copy link
Copy Markdown
Contributor Author

The failing C++ Integration Tests (STX) check is not this PR

Diagnosed during the milestone #63 sweep — all three open Wave 1 PRs (#2807, #2809, #2816) fail this same check, and none for a reason related to their diffs. The build never reaches compilation:

CMake Error: Could not find CMAKE_ROOT !!!
Modules directory not found in
C:/Windows/Temp/cmake/cmake-3.31.4-windows-x86_64/share/cmake-3.31

.github/workflows/build_cpp.yml caches CMake under $env:TEMP on the self-hosted runner and gates re-download on Test-Path "$cmakeCached\cmake.exe" — it validates that bin/cmake.exe exists but never that share/cmake-3.31/Modules/ does, which is what CMAKE_ROOT resolves to. Temp cleanup removed share/ and left bin/, so the guard sees a healthy cache, skips the re-download, and prepends a broken CMake to PATH.

Tracked as #2817, fix in flight. Nothing to do on this PR for it.

kovtcharov-amd pushed a commit to Jonesxq/gaia that referenced this pull request Aug 6, 2026
…xe (amd#2818)

Every `cpp/**` PR has been failing the `C++ Integration Tests (STX)`
check before it compiles anything — amd#2807, amd#2809 and amd#2816 are all red
for a reason unrelated to their diffs. The self-hosted runner cached
CMake under `$env:TEMP` and re-downloaded it only when `bin\cmake.exe`
was missing; Windows Temp cleanup deleted `share\cmake-3.31\Modules` and
left `bin\`, so the job kept trusting a CMake that cannot resolve
`CMAKE_ROOT` and every run died the same way until someone cleared Temp
by hand. Now each candidate toolchain is probed for the thing the build
actually depends on, a failed probe falls through to a clean
re-download, and tools live in the runner tool cache instead of a
directory the OS sweeps.

One measurement drove the design and is worth flagging for review:
**exit codes cannot detect this failure.** A CMake missing its Modules
tree prints `Could not find CMAKE_ROOT` to stderr and still exits 0 —
for `--version` and for `--help-module-list` (measured on 4.4.2). So the
issue's suggested `cmake --version` exit-0 check would not have caught
it on its own; validity requires the Modules tree on disk *and* a probe
that does not report a broken root.

The stale `%TEMP%\cmake` tree on the runner is now inert — nothing reads
it — so no manual cleanup is needed to make this work; deleting it just
reclaims disk.

Closes amd#2817

## Test plan

- [ ] `pwsh -File .github/scripts/tests/CppBuildTools.Tests.ps1` passes
(21/21). It asserts the exact regression: `bin/cmake.exe` present +
`share/` absent reports **invalid**, both present reports **valid**, and
includes negative controls showing the old `Test-Path cmake.exe` check
and an exit-code-only check would both have accepted the broken install.
- [ ] New `C++ toolchain script tests` job is green (parse-checks every
`.github/scripts/*.ps1`, then runs the unit tests).
- [ ] `C++ Integration Tests (STX)` on this PR gets past `Ensure C++
build tools are available` and reaches compilation. The step log should
name which CMake it accepted and, if it rejected one, why.
- [ ] Reproduce the root cause on any machine: copy a `cmake` binary
alone into an empty directory and run `--version` — it prints the
`CMAKE_ROOT` error and exits 0.
- [ ] After merge, re-run CI on amd#2807, amd#2809 and amd#2816 with no changes
to their diffs and confirm the STX check goes green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp documentation Documentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cpp): SQLite integration and gaia::Database

1 participant