Make SQLite the default local/Docker database#13
Merged
Conversation
Replaces the Firestore emulator sidecar with a new SqliteDatabase (IDatabase impl) as the sole local/Docker-dev backend. Firestore now only connects to an already-hosted, GCP-managed instance (DATABASE_TYPE=firestore) or a manually-run emulator process (DATABASE_TYPE=emulator) — SpecFlow never deploys/manages Firestore itself locally anymore. - docker-compose.yml: drop firestore-emulator/firestore-exporter services; backend defaults to DATABASE_TYPE=sqlite and bind-mounts the host's ~/.specflow/ directory (one central db shared across local projects/MCP sessions, matching the old shared-emulator model). - Makefile: init-firestore(-dry) -> init-db(-dry) (aliases kept), isolated test-stack gets its own nested sqlite path so tests never touch the real central db. - init_firestore.py -> init_db.py; emulator-host requirement now conditional on DATABASE_TYPE. - create_generation_session_repos.py: DATABASE_TYPE gate now rejects only memory (was firestore-only). - startup_validation.py: generalized Firestore-only connectivity check to a DB-agnostic one; TOKEN_ENCRYPTION_KEY/GitHub-secrets requirement now covers sqlite too (it persists across restarts, unlike memory). - Extracted shared IDatabase contract tests (db_contract.py) run against both memory and sqlite backends.
The TUI's container-readiness check and specflow-init.sh's onboarding flow still assumed a firestore-emulator container existed. Fixes the resulting breakage from the sqlite-default switch: - local_env.py: containers_running() now checks only the backend container (sqlite has no separate container — it's a bind-mounted file). - specflow-init.sh: seeding step no longer hardcodes FIRESTORE_EMULATOR_HOST unconditionally; --reset-local-db now clears the SQLite file + WAL/SHM sidecars instead of an emulator export directory. - .env.quickstart.example: DATABASE_TYPE default flips to sqlite, documents SQLITE_DB_PATH/SPECFLOW_HOME_MOUNT_PATH, reframes Firestore vars as hosted-connect-only. - mcp_server/tests/e2e/runner.py + cli.py: default to sqlite for API-key fetch and update --reset-local-db help text.
Follow-up to the sqlite migration — these still described the removed Firestore emulator.
mkonopelski-gd
force-pushed
the
feature/use-sqllite-instead-firestore
branch
from
d9a1896 to
6712186
Compare
mkonopelski-gd
commented
Jul 2, 2026
- Nest specflow.db/-wal/-shm under a db/ subdirectory (~/.specflow/db/, container /root/.specflow/db/) instead of directly in ~/.specflow/, so the database files stay separate from other files (e.g. config.json) in that directory. Updated docker-compose.yml, config.py, Makefile, specflow-init.sh, .env.quickstart.example, and the mcp_server e2e runner default accordingly. - docs/ARCHITECTURE.md: answer where ~/.specflow/ and its db/ subdir get created on a fresh install (Docker bind-mount + SqliteDatabase.__init__). - Trim two over-long inline comments (backend/Dockerfile, local_env.py) to one-liners per review feedback.
…tion - init_db.py: default DATABASE_TYPE to sqlite (not memory) when unset, matching the sqlite-is-the-default intent everywhere else. Previously, running the script directly without exporting DATABASE_TYPE silently fell back to the ephemeral in-memory backend. - scripts/README.md: documents the sqlite default; drops the stale "defaults to memory, data won't persist" warning. - docs/backend/ARCHITECTURE.md, docs/backend/DEVELOPMENT.md: backend contributor docs updated for the sqlite migration (missed in the earlier top-level docs/ARCHITECTURE.md and QUICKSTART.md pass), using the db/ subdirectory path from the PR review fixup.
Host-side seeding (uv run scripts/init_db.py) resolved SQLITE_DB_PATH from .env, which is the container-internal path (/root/.specflow/db/specflow.db). On the host that path is unwritable for non-root users (init_db.py crashes on mkdir) and, even when writable, seeds a file the container never reads (it reads the bind-mount source ~/.specflow/db/specflow.db) — so the backend boots with an unseeded DB and LocalAuthMiddleware rejects every request. This broke the from-scratch local quickstart. Derive the host seed path from SPECFLOW_HOME_PATH (the bind-mount source), the single knob (via SPECFLOW_HOME_MOUNT_PATH) shared by host and container. Comment out the container-internal SQLITE_DB_PATH in .env.quickstart.example (already defaulted by docker-compose) and add a regression test. Addresses backend-review Finding #1 (CRITICAL).
Both host-side seeders carried their own copy of the workspace-document builder, id-assignment, and upsert loop. The two create_workspace_document copies had already drifted — the provisioner's was missing the cleaning_started_at field. Introduce app/services/workspace_pool_seeding.py as the single source of truth: WorkspacePoolEntry (typed, validated), parse_pool_entries, assign_pool_entries (ordered + prefix id assignment), build_workspace_document (one schema, incl. cleaning_started_at), and seed_workspace_pool (idempotent upsert with created/ updated/skipped counts). init_db.py and create_generation_session_repos.py now delegate to it. No behavior change; file-based seeding contracts preserved. Adds unit tests for the module. Part of moving local workspace config to SQLite as the single source of truth.
…spaces.json
The workspace pool was duplicated: a transient .specflow-local/workspaces.json flat
file AND the DB workspaces collection. Eliminate the flat file so the SQLite DB is the
single source of truth for the local pool.
- create_generation_session_repos.py now seeds the DB directly for BOTH the {prefix}{num}
and --provide-own-repos (arbitrary-name) paths — id assignment handles ordered repos, so
the arbitrary-name limitation that forced the file handoff is gone.
- init_db.py: --workspace-config is now optional. Without it (local quickstart) it seeds only
the bootstrap API key + local-auth identity; the file input is retained for e2e / BYO repos.
- specflow-init.sh: provisioning writes the pool straight into the DB (drops --skip-firestore
and --output-workspace-config, passes the host SQLITE_DB_PATH); init_db.py then seeds API key
+ identity. .specflow-local/workspaces.json is gone entirely.
Tests updated to the new contract; MEMORY.md note refreshed.
…nstead-firestore # Conflicts: # agents/MEMORY.md
Replace the single generic documents JSON-blob table with dedicated api_keys / generation_sessions / workspaces tables. Fields actually filtered/ordered are promoted to typed, indexed columns; the full document still lives in a data JSON column (source of truth on read). Unregistered collections fall back to the generic documents table, so the document-shaped IDatabase interface and all callers are unchanged. - New sqlite_schema.py: Open/Closed registry (table + promoted columns + indexes), single source of truth for DDL and query routing. - get_api_key_by_uid becomes an indexed column lookup. - Shared module helpers keep the class and transaction context DRY. - Add relational-schema tests; shared db_contract suite unchanged.
…ted columns The SQLite backend now supports exactly the registered collections (api_keys, generation_sessions, workspaces); an unregistered collection raises a clear 'register it in sqlite_schema.py' error instead of silently landing in an unindexed JSON blob. The app only ever uses these three collections, so the fallback was dead weight at runtime. Also promote exactly the fields used in a query filter/order_by — no more, no less: drop never-queried columns (failed_at, outputs_archived, cleaning_started_at, allocated_at, is_active, user_id) and add the one that was missing (clean_verified). Non-promoted fields stay queryable via json_extract on the same table's data column. The shared db_contract.py parity suite now runs against registered collections (transparent to memory/Firestore) so SQLite is still covered without a fork; this also exercises the promoted status column path.
Remove the module-level _row_*(conn, ...) helper layer and its conn threading. All connection-level SQL now lives as methods on SqliteTransactionContext (get/set/update/delete/query/array_union/ list_subcollection/subdocuments/get_api_key_by_uid), with _require_schema folded in as its _schema method. SqliteDatabase composes one context over its connection and just adds the lock + lifecycle (schema init, transactions, maintenance, close), delegating every op to it. No behavior change; module-level functions drop from ~15 to 6 pure value/serialization helpers. Same SQL, same tests.
… table Apply the same rule to subcollections as to top-level collections: no generic blob table. Replace the catch-all 'subdocuments' table with a named child table per registered subcollection, keyed by real columns. Today that's the only one in use — workspace_model_usage under generation_sessions, keyed by (generation_id, workspace_id). An unregistered subcollection is rejected loudly (SqliteTransactionContext ._sub_schema), same as unknown top-level collections. No payload field is ever filtered (readers read the whole subdoc), so no promoted columns — just the composite key + data JSON. Behavior is unchanged; the generic IDatabase subcollection methods keep their signatures and route to the named table internally.
self._path was write-only (never read) — remove it. Reword the column_for docstring away from 'JSON fallback' now that the only fallback concept (the removed documents/subdocuments blob tables) is gone; a non-promoted field is simply read from the data column.
Fold sqlite_schema.py into sqlite.py and simplify the registry:
- table name == collection/subcollection name, so drop the redundant
'table' field.
- replace the Column dataclass + column_names/column_for with a plain
{field: sqltype} dict per schema.
- drop the schema_for/all_schemas/subcollection_schema_for/
all_subcollection_schemas accessor shims (only needed to cross a module
boundary that no longer exists); _schema/_sub_schema read the module
dicts directly.
- strip verbose docstrings/comments down to a short module docstring plus
a couple of one-liners for the genuine footguns (!= includes missing
field; datetimes stored as ISO text).
No behavior change; ~250 fewer lines across the two files.
_quote_ident wrapped table/column names in double quotes, but every identifier interpolated into SQL is either a literal from our own hardcoded _SCHEMAS/_SUBSCHEMAS dicts or a caller value already validated against those dicts by _schema()/_sub_schema() before use — never attacker- or user-controlled, and never containing spaces, special characters, or reserved words. get_api_key_by_uid already interpolated 'api_keys' unquoted, proving the quoting elsewhere added no protection. Inline identifiers directly, consistent with that. No behavior change.
…aller _container_name() had exactly one call site (containers_running). Read the env var directly there instead of through a one-line wrapper.
…ion concept) 'Subcollection' is a Firestore word for what SQL calls a table with a compound primary key. Collapse the two registries (_SCHEMAS + _SUBSCHEMAS) and two resolvers (_schema + _sub_schema) into a single _Table dataclass and _TABLES list, keyed by name. A _Table has a primary_key (one column for a document store, several for a child table), indexed_columns promoted out of the JSON data blob, indexes, and an optional parent for cascade-clear + subdocument addressing. - _init_schema and clear collapse to one loop over _TABLES; the compound PK falls out of primary_key naturally. - clear_all(None) now clears every registered table (auto-includes future ones) instead of a hardcoded list; a named subset still cascades to child tables via parent. - The Firestore vocabulary (collection/subcollection/subdocument) now survives only in the shared IDatabase method names, not in storage. Behavior-identical: same tables, same SQL, same tests (148 db/session tests pass).
…est instead An unregistered table name can only come from an internal constant, so it's a developer error, not an app/runtime condition. Remove the bespoke _table()/ _child_table() validators (the 'Unknown SQLite table … Register it' raises and the parent check): the name is used directly, so an unregistered one fails loudly on its own (KeyError from _TABLE[name], or 'no such table' from SQLite). Add test_every_used_collection_is_registered, which asserts every collection/ subcollection constant the app addresses maps to a registered _TABLE — so 'added a collection, forgot the table' fails at CI, not in production. Existing rejection tests now assert the natural KeyError. The parent_collection arg on the subdocument methods is now unused (SQL identifies a child table by its own name); kept for the shared interface.
The connectivity probe queried a sentinel '_health_check' collection, relying on 'query on a nonexistent collection returns []' — true for Firestore/memory but not for the SQLite backend, which rejects unregistered table names (KeyError). With DATABASE_TYPE=sqlite (the new default) the check therefore always failed, marking the instance unhealthy so /health/ready never went green and the quickstart readiness gate hung. Query COL_WORKSPACES (a real, always-created table) instead — an empty result still validates connectivity/auth on every backend. Add a regression test that runs the connectivity check against a real SqliteDatabase (the existing tests only covered memory/firestore, which is how this slipped through).
Integrates PR #29 (removal of the unused server_timestamp() API). Drops this branch's now-orphaned mirror copies of that dead code: - sqlite.py: _ServerTimestamp sentinel, its _encode_for_storage branch, and SqliteDatabase.server_timestamp() - db_contract.py: the shared TestServerTimestamp contract test (+ now-unused datetime import) - test_sqlite_db.py / test_memory_db.py: the TestServerTimestamp inheritors Conflict in test_memory_db.py resolved in favor of this branch's inherit-from-db_contract structure, minus the removed server_timestamp test.
…user-facing knob
The container's SQLite file always lives under the /root/.specflow bind mount, so its
path was never a real user knob — and the docs told users to change it, which silently
breaks (the container reads the new path off-mount while host-side seeding keeps writing
the bind-mount source). Overriding the *container* SQLITE_DB_PATH to a host path (as the
Makefile test stack did via compose ${SQLITE_DB_PATH:-}) is the same latent split-brain.
- docker-compose: pin SQLITE_DB_PATH=/root/.specflow/db/specflow.db (no .env override).
Isolation/relocation is done via SPECFLOW_HOME_MOUNT_PATH (the bind mount) — the one
legitimate host-side knob — so container and host-seeding now resolve to the same file.
- config.py: name the fixed path CONTAINER_SQLITE_DB_PATH and use it as the field default.
The SQLITE_DB_PATH env field stays only as the host-seeding/test override mechanism.
- .env.quickstart.example: remove the misleading SQLITE_DB_PATH block; keep
SPECFLOW_HOME_MOUNT_PATH as the real (host, default ~/.specflow) knob.
- update the compose guard test + the init-script comment.
Note: the docker integration/e2e stack should be validated with 'make integration-tests'
since the container-path change isn't covered by the (docker-skipped) unit suite.
Widen the promotion rule from 'queried fields only' to 'every stable scalar field the app writes' — the goal is a genuinely inspectable SQL schema (browsable in DB Browser for SQLite), not just query performance. Indexing stays a separate, narrower concern (only the column combos something actually filters/orders on). - workspaces: 7 -> 21 promoted columns (repo_url, p10y_repository_id, locked_at, lease_expires_at, cleaning_started_at, last_used_by, last_cleaned_at, error, stuck_reason, stuck_at, force_released + force_release_reason/by/at). allocation_history (array) stays JSON-only. - generation_sessions: 6 -> 22 promoted columns (checkpoint, started_at, completed_at, failed_at, error, retry_count, max_retries, user_email, workspace_pool, specification_dir, outputs_archived, code_archived, archive_status, artifact_path, emergency_archived, total_usd_cost). Nested/open-schema fields (state_history, workspace_ids, parameters, planning_data, workspace_phases*, progress, result, workflow_usage_metrics, model_usage) stay JSON-only by design — no Pydantic model exists for this collection, so these are genuinely document-shaped, not scalars we missed. Field list verified against every real writer (workspace_pool_seeding, workspace_state_machine, generation_session_state_machine, generation_session.py, artifact_store.py, generation_session_retry.py) — not guessed from memory. Excluded generation_id: always equals doc_id, pure redundant storage. Renamed _Table.indexed_columns -> columns now that promotion and indexing are explicitly decoupled. Added _reconcile_columns: on open, ALTER TABLE ADD COLUMN + backfill from the JSON data blob for any registry column missing from an existing db file, so a schema change never requires a manual reset (verified: JSON true/false extracts as SQL integer 1/0, matching the direct-write bool encoding, so backfill can't drift from the write path). No interface/caller changes — SqliteTransactionContext.set() already mirrored every registered column from the document; this only grows the registry. Verified: 184 db/service tests pass; full suite 1987 passed (only the pre-existing unrelated factory env-flake); complexity flat (Δ-0.01 vs main).
Same policy as workspaces/generation_sessions: every stable scalar the
APIKey model/writers put on the document becomes a real column, not just
the queried key_uid. api_keys is the cleanest case of the three — it
already has a Pydantic model (APIKey) backing it.
1 -> 12 promoted columns: workspace_pool, user_id, user_name, created_at,
last_used_at, expires_at, is_active, github_token_ciphertext (already
Fernet-encrypted before storage — no new exposure vs sitting in the JSON
blob), github_token_set_at, git_user_name, max_concurrent_sessions.
Excluded api_key: always equals doc_id, pure redundant storage (same
reasoning as excluding generation_id from generation_sessions).
Stays JSON-only: permissions (list), metadata (arbitrary dict),
active_generation_sessions (list of dicts, managed by transactional
appends in api_key_session_concurrency.py).
No new indexes: verified every query("api_keys", ...) call site — the
only filtered lookup is key_uid (already indexed); every other call is
list-all with no filter. Field list verified against every real writer
(api/v1/auth.py key creation/revoke/reactivate, middleware/auth.py
heartbeat, scripts/init_db.py local sentinel doc, the APIKey model
itself) — is_active and user_id were in fact in the original schema
before the earlier queried-only trim, so restoring them under the new
policy is consistent, not a reversal.
Verified: 52 db tests pass; full suite 1988 passed (only the
pre-existing unrelated factory env-flake); complexity flat (Δ-0.01).
The SQLite file is a host bind mount; WAL's -shm mmap is not coherent across the container/host boundary, so a host-side open/close orphans -wal/-shm under the container's live connection and its committed writes become invisible. DELETE mode keeps every commit in the main .db — durable and inspectable regardless of who opens it.
awrobel-gd
reviewed
Jul 8, 2026
awrobel-gd
reviewed
Jul 8, 2026
Contributor
There was a problem hiding this comment.
These must work
- - unit tests
- - integration tests
- -
make skip-mode-e2e-tests
awrobel-gd
reviewed
Jul 8, 2026
awrobel-gd
reviewed
Jul 8, 2026
| # are separate concerns. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
I would create new files in database/schema folder
- sqlite_schemas.py
- sqllite_tables.py
and import them here
also add database/ser_deser.py (serialization/deserialiation)
where we put the functions that handle all that blobbing and unblobbing
also add database/transaction.py - with the context
it helps to structure the codebase. the actual sqlite.py then will be left with the interface implementation
Contributor
Author
There was a problem hiding this comment.
I did - funny enough it was more modular before - I joined everything so each database has own module and sqlite has 4. But I think this is OK - refactored
awrobel-gd
reviewed
Jul 8, 2026
|
|
||
| def get(self, collection: str, doc_id: str) -> Optional[Dict[str, Any]]: | ||
| row = self._conn.execute( | ||
| f"SELECT data FROM {collection} WHERE doc_id = ?", (doc_id,) |
Contributor
There was a problem hiding this comment.
lets do a single commit where we only mass replace a field that is hard-coded with its name imported from schemas, where its also used in schema definitions
like
DOC_ID = "doc_id"
used in the list of columns
also used in all the SQL string templates
follow up - simple DSL to avoid all the string templating
def select(cols, from):
return f"SELECT {*cols}..."
Contributor
Author
There was a problem hiding this comment.
Actually second is good idea. I implemented it. First I see not much cleanness unless this is setup for future change of name. But I did it still.
awrobel-gd
reviewed
Jul 8, 2026
| return self._ops.query(collection, filters, order_by, limit) | ||
|
|
||
| def run_transaction(self, callback: Callable[[ITransactionContext], T]) -> T: | ||
| with self._lock: |
Contributor
There was a problem hiding this comment.
locking is one way, other is consuming from a queue or something. I think this can stay
…qlite/ Keeps sqlite.py as a thin IDatabase implementation, matching firestore.py and memory.py, instead of one 549-line file mixing schema, blob encode/decode, and connection-level SQL.
…name The literal "doc_id" was hard-coded independently in three primary-key tuples (tables.py) and six SQL templates (transaction.py). Single source of truth now lives in schemas.py.
The five read methods each hand-wrote their own "SELECT ... FROM ..." prefix. _select() composes it from a table, column list, and equality columns, removing the duplication.
Making SQLite the default removed the firestore-emulator container, but `make integration-tests` still exported FIRESTORE_EMULATOR_HOST and test_firestore_db.py skipped only on its absence — so the tests ran against a dead port and failed instead of skipping. Gate them on an actual socket probe of the emulator host (mirroring the retired test_firestore_emulator_persistence.py guard), and only default FIRESTORE_EMULATOR_HOST for the emulator backend so the sqlite/firestore stacks stop advertising an emulator that isn't there.
archive_status is a per-workspace confirmation map ({ws_id: "confirmed"|
"failed"}) built in GenerationSessionStateMachine.complete(), but the
scalar-core promotion refactor added it as a TEXT column. _to_sql_param
only serializes datetime/Enum/bool, so binding the dict raised "Error
binding parameter 20: type 'dict' is not supported" and failed every
generation at the estimation-completion step.
Drop it from the column registry so it stays in the JSON data blob, like
the sibling workspace_phases map — matching the module's own rule that
nested/open-ended structures are not promoted. Fix the test fixture that
had modelled it as a scalar to use the real per-workspace-map shape.
…arios Two e2e-infra fixes exposed by running skip-mode-e2e-tests on the sqlite default: - TEST_WORKSPACE_MOUNT_PATH was repo-root-relative, but several targets `cd backend &&` before deriving SQLITE_DB_PATH for init_db.py, so it resolved against backend/ — init_db.py seeded a second, never-cleaned sqlite file while the container read the empty repo-root one it bind- mounts, surfacing as a 503 "Local identity not seeded". Anchor it with $(CURDIR) so every derived path is stable regardless of cwd. - skip-mode-e2e-tests chains two scenarios against one workspace set, but a completed generation leaves its set in CLEANING (reclaimed only by the 2h stuck-cleaning job), so contract-validation found nothing available. Re-seed the pool with init-db --replace between scenarios.
# Conflicts: # backend/test/scripts/test_create_generation_session_repos.py
awrobel-gd
approved these changes
Jul 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes SQLite the default local/Docker-dev database, replacing the Firestore emulator sidecar with a SqliteDatabase implementation of the same IDatabase interface (Firestore now only connects to a hosted GCP instance or a manually-run emulator). Known collections are stored in real relational tables — api_keys, generation_sessions, and workspaces — with the frequently-queried fields promoted to typed, indexed columns rather than opaque JSON rows.
Entrypoint
backend/app/database/factory.py get_database() selects the backend from DATABASE_TYPE; the relational layout is declared once in backend/app/database/sqlite_schema.py and drives DDL and query routing inside backend/app/database/sqlite.py.
Details
Each promoted column is mirrored out of the document on write, but the full document still lives in a data JSON column that stays the source of truth on read, so the document-shaped IDatabase interface and every caller are unchanged; any unregistered collection falls back to a generic documents table. Timestamps are stored as fixed-width ISO-8601 UTC text so lexical order equals chronological order in both columns and blob. No data migration is included — this is a new, undeployed feature, so local dev DBs are simply recreated and re-seeded via init_db.py.
Test plan
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com