awslabs / awslabs/cli-agent-orchestrator
[Feat] Decouple data access, fix SQLite's concurrency settings, and make upgrades safe
- Dominant language
- Python
- Stars
- 1.3k
- Forks
- 267
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 70
Description
[Feat] Decouple data access, fix SQLite's concurrency settings, and make upgrades safe
Part of #777 (CAO 3.0).
**Scope note:** an earlier version of this issue proposed adopting PostgreSQL for shared deployments. **That has been dropped.** CAO keeps SQLite. The reasoning is recorded below, because it is worth not relitigating.
## In plain terms
CAO stores structured metadata in SQLite; memory content, workflow files and logs also live on disk. SQLite is staying. Three parts of data access need attention for 3.0:
1. **Data access is spread throughout the code**, tied to SQLite-specific details, so nothing can change without touching everything.
2. **WAL is not enabled and the timeout policy is inconsistent or implicit.** Measure contention before choosing settings; SQLite still permits only one writer at a time in WAL mode.
3. **CAO cannot tell which version of the data layout it is looking at**, and a failed upgrade step is written off quietly.
The third is the one that blocks 3.0: #778, #774, #779 and #780 must agree identity, membership, session, ownership and visibility storage before changing data people already care about.
## Why we are not adopting PostgreSQL
Worth writing down, because "shared service" instinctively suggests "real database server".
**Today workers run their own CAO server and database; 3.0 removes that.** In the current EKS example each worker runs the ordinary CAO image whose entrypoint ends in `exec cao-server`, initialises its own SQLite database from a seed, and mounts a per-worker `emptyDir` as CAO state. Workers therefore do open SQLite — their own ephemeral copy, not the supervisor's — and only selected shared state (memory, inbox callbacks) is proxied over HTTP. Processes outside cao-server open the database directly too: `cli/commands/init.py`, `cli/commands/schedule.py`, and `services/memory_gateway.py` when `CAO_MEMORY_API_URL` is unset.
So this is a **3.0 invariant to establish and test**, not a property to cite: *in the #745 target topology, a worker neither mounts nor opens the central database, and persistent CAO control-plane/metadata mutations go through the central server.* This covers canonical CAO resource state, not every byte an agent writes: approved execution-workspace and artifact writes remain with the runtime under #745. Until an automated check enforces it, the reasoning below rests on an assumption rather than a fact.
Note also that worker count still drives central **database operation rate** — more workers mean more API requests, state transitions, inbox rows and journal writes — even once they no longer hold connections of their own.
**cao-server runs as a single replica**, and that is deliberate (#745). One process owning one file on an attached volume is exactly the situation SQLite is designed for. The existing deployment already does this — a `StatefulSet` with `replicas: 1` and a `ReadWriteOnce` volume.
**PostgreSQL is not what stands between CAO and multiple replicas.** Running several cao-servers is blocked by a long list of process-local state, of which the database is one item:
- the in-memory event bus (`services/event_bus.py:37-56`);
- per-terminal FIFOs on local disk (`services/fifo_reader.py:149-193`);
- pseudo-terminals and tmux being host-local (`backends/tmux_backend.py:20-40`, `api/main.py:6901-6937`);
- a process-local session-env store (`services/session_env.py:1-17`);
- duplicated startup work — `api/main.py:1257-1329` starts **six unconditional long-running tasks** (flow daemon `:1268`, status monitor `:1275`, log writer `:1276`, inbox service `:1277`, OpenCode inbox daemon `:1307`, inbox reconciliation daemon `:1311`), **up to two optional ones** (AGUI approval bridge `:1302`, Herdr inbox service `:1329`), and **three one-shot tasks** (`:1258`, `:1259`, `:1265`). Every replica would run its own copy of all of them. *(An earlier version of this issue said "15 long-running loops". That was wrong.)*
And several the earlier list missed entirely: per-session lifecycle locks (`services/session_lock.py:45-89`), provider objects (`providers/manager.py:28-32`), workflow ownership and cancellation (`services/workflow_service.py:210-227`), status buffers and detection tasks (`services/status_monitor.py:130-167`), SSE subscriptions (`services/sse_bus.py:63-73`), and terminal logs, memory files and profiles on local disk.
So the accurate claim is narrower than the earlier one: **PostgreSQL is not required solely to enable multiple replicas**, and adopting it would leave the list above untouched. That is not the same as "it buys almost nothing" — a database server also brings real operational benefits CAO would otherwise have to build: backup and restore, an independent lifecycle, better observability, online maintenance, stronger concurrent-write behaviour, and row-level tenant controls. Those are reasons to keep the door open, not reasons to walk through it now.
Note too that some of these blockers are easier than the list implies — session env can be persisted and background jobs can use leader election — and that tmux/PTY locality is precisely what #745 relocates, so it should not be treated as permanent.
**It also can't be "run separately".** SQLite is a library linked into the process, not a server. There is no daemon to give its own pod — embedded is the only shape it has.
So PostgreSQL stays a possibility the seam below *enables*, to be adopted if measurement ever demands it — not on principle.
## What this delivers
### 1. One place that talks to the database
All data access moves behind a single interface. This is what "not tightly coupled" actually means in practice — not a separate service, but one boundary instead of SQLite details spread across the codebase.
It also makes the tenant filter from #778 enforceable in one place rather than trusted to every future query.
**Include automatic maintenance in that boundary.** Startup schedules the workflow-journal sweep (`api/main.py:1216-1265`). Today it selects only run IDs and start times, applies one count window, and cascades deletion without checking execution state (`services/workflow_journal.py:1724-1799`, `services/workflow_retention.py:238-309`). Carrying that behavior unchanged into #745 can remove recovery records while a remote worker still exists. Scoped queries alone also do not partition a count limit if their results are combined again.
**Preserve read failures across callers, not just inside the database helper.** `workflow_journal.get_run` already propagates storage errors, but the workflow service can convert them to absence (`services/workflow_journal.py:868-899`, `services/workflow_service.py:1220-1289`). The inspection API then returns 404 on a cold cache (`api/main.py:5192-5207`), even though the row still exists; the same locked store returns a server error on the warm path. This operational failure must not become evidence that shared execution state has disappeared.
The current coupling, measured: **7 hand-written `CREATE TABLE` statements, 10 `PRAGMA` calls (9 of them `PRAGMA table_info`, used as hand-rolled migration existence-gates; exactly one, `busy_timeout`, sets configuration), 3 `INSERT OR IGNORE`/`INSERT OR REPLACE` statements, and 6 files importing `sqlite3` directly.** Counted from string literals in executable positions, excluding comments and docstrings — an earlier version of this issue said 12 and 11, which were raw text matches.
*Correction: an earlier version of this issue gave these as 17 / 24 / 10 / 7. Those figures came from raw text searches that also counted the same phrases where they appear in docstrings and comments. The numbers above count executed statements only, and are the ones to work from.*
### 2. Turn on the settings SQLite needs
This is the real defect. The main engine sets neither **WAL mode** nor **`busy_timeout`** explicitly, and CAO's own code says so (`services/session_service.py:520-523`):
> *"the engine sets neither busy_timeout nor WAL, so `database is locked` is an ordinary outcome under CAO's concurrent writers."*
**That comment is accurate about the code but misleading about the effect, and this issue should not repeat its error.** Measured rather than read: Python's `sqlite3.connect` applies a default five-second timeout, so a connection reports `PRAGMA busy_timeout = 5000` even though nothing in CAO sets it — the same value the workflow journal sets by hand. WAL genuinely is absent: a fresh connection reports `journal_mode = delete`.
So the two halves are not equally cheap:
- **WAL is the real change.** It lets readers continue while a write is in progress. It is off today and turning it on is a genuine improvement.
- **`busy_timeout` is already five seconds in practice.** Setting it to 5000 explicitly would change nothing. The work is to make the value deliberate, consistent and documented rather than inherited from a library default.
Anyone starting this issue should first reproduce a `database is locked` failure against the real five-second timeout. If it cannot be reproduced, the concurrency premise needs revisiting before code is written.
Both are already understood in the codebase — the workflow journal sets its own `busy_timeout`, and WAL is noted as a database-level property that was left out of that change's scope. This issue is where it gets addressed properly.
### 3. Know which version the data is in
There is no schema version marker anywhere. (`event_schema_version` exists, but that is a field on workflow *events*, not the database layout.) Without one, CAO cannot tell a 2.5 database from a 3.0 one, refuse to run an old binary against newer data, or order its migrations.
Every install upgrading to 3.0 depends on this working.
### 4. Make upgrades fail loudly
Migrations today check whether a column exists and add it, swallowing errors. One journal comment admits the consequence outright: *"returning is not succeeding."*
For one person with one copy, survivable. For a team's shared data mid-migration, not. A failed migration must stop and say so, not continue with a half-changed database.
## Phased delivery
**Step 1 — One interface for data access.** No behaviour change.
*Exit: nothing outside the boundary references SQLite specifics; existing tests pass unchanged.*
**Step 2 — Turn on WAL, and make the busy timeout deliberate.**
*Exit: the reproduced contention case is addressed under the documented workload, settings agree across connections, and remaining busy/locked errors are handled explicitly. WAL improves reader/writer concurrency; it is not a guarantee that writes never contend.*
**WAL changes storage and backup handling.** SQLite documents these properties in its [WAL guide](https://sqlite.org/wal.html):
- **Use the real database filename.** It remains `CAO_HOME_DIR/db/cli-agent-orchestrator.db` (`constants.py:122`, `:365`), not the `cao.db` name used in the earlier experiment. While WAL is in use, `-wal` and normally `-shm` companion files can appear; they are usually removed after the last connection closes cleanly.
- **WAL mode persists across connections until explicitly changed.** The setting is not limited to the process that enabled it.
- **A live file copy is not a consistent backup.** Committed data may still be in the WAL. Rollback-journal databases also require coordination: being a 2.5 database or reporting `delete` does not make an uncoordinated copy safe.
- **Storage support and activation failure are separate questions.** Normal multi-host access over NFS/EFS is not a supported WAL topology. A locally synced folder is not automatically the same thing as a network filesystem, and successful activation does not make live file synchronisation a safe backup. `PRAGMA journal_mode=WAL` can return the old mode when conversion is unsupported, or raise an error such as `database is locked`; check both outcomes and report failure clearly. Keep central CAO state on supported storage rather than silently accepting an unsafe fallback.
**Step 3 — Add a schema version, and record 2.5's layout as the baseline.**
*Exit: an existing 2.5 database is correctly identified, and a 3.0-or-later binary refuses a schema newer than it supports with a clear message.*
**Recognise supported starting schemas, not just a release label.** At minimum, handle both v2.5.0 and v2.5.1: `idempotency_keys` is absent from the former and present in the latter (see #774). The existing best-effort migrators also mean a database can be partly upgraded. Inspect the required tables, columns and constraints; either follow an explicitly supported migration path or stop with a recovery message. One table's presence is not proof that the rest of the layout is valid.
**Step 4 — Make migrations fail loudly, with a defined recovery.**
*Exit: an induced mid-migration failure leaves a detectable state and an actionable message, never a silently half-migrated database. The documented pre-upgrade backup and recovery procedure restores the matching database and any files the migration moves or rewrites.*
**Step 5 — Carry the 3.0 migration** — tenant, owner and visibility, together with the identity/membership/session records they depend on, in one owned change (#778, #774, #779, #780). Preserve stable internal IDs for #778's later, operator-controlled binding of the local owner to a work identity; sign-in implementation must not invent a second unversioned store.
*Exit: a populated 2.5 database upgrades with nothing lost, everything assigned to the local tenant and user.*
Steps 1–4 are useful on their own and fix real defects today, whether or not anything else in 3.0 lands.
## Acceptance criteria
- [ ] All data access sits behind one interface.
- [ ] Automatic workflow-journal retention prunes only eligible history. Records still needed for current execution, reconciliation, cancellation or final-result capture remain protected under #745; startup cleanup must not race that classification. Apply the count window independently within each tenant/owner history (#774/#778), so another scope's activity cannot evict it. Reuse the existing sweep/delete machinery and configured age/count values, including zero-disable and local behavior; normal expiry continues once records are eligible. Cover a server restart with a surviving worker under age/count pressure, competing ownership scopes, and an eligible completed-run control.
- [ ] WAL is enabled on supported storage. A returned non-WAL mode and an activation exception both produce an explicit, documented outcome; a successful pragma is not used as proof that network access or live file synchronisation is safe.
- [ ] The busy timeout is set **explicitly** to a deliberate, documented value rather than inherited from Python's five-second default, and the main engine and the workflow journal agree on it.
- [ ] Across data, service and API callers, an operational store read failure (busy/locked, I/O failure or unavailability) remains distinguishable from a successful authorized no-match in warm- and cold-cache paths. It must not be returned or acted on as confirmed absence, including by create, replay/resume or delete decisions that require an absence result. Cover healthy existing, healthy absent, failed-read and recovery-after-failure cases. Preserve existing authorization-denial, genuine-not-found and resource-validation behavior; reuse the existing error-handling boundaries rather than introducing a new error framework.
- [ ] A schema version exists and is checked before any supported 3.0+ entry point migrates or writes persistent state, including local CLI and gateway paths that can open the database without starting `cao-server`.
- [ ] CAO refuses a database whose schema version is newer than the version it supports.
**This binds 3.0 and later only.** Released 2.5 binaries do not understand a new version marker, so opening a migrated 3.0 database with them is unsupported. Recovery means restoring the pre-upgrade state, not pointing the old binary at the new schema. Use SQLite's [backup API](https://sqlite.org/backup.html) or another supported consistent snapshot method before upgrading. An offline file copy is acceptable only after all users of the database have stopped and recovery/checkpointing is complete; do not discard a needed journal or WAL. This applies before and after 3.0 ([SQLite's backup-corruption guidance](https://sqlite.org/howtocorrupt.html)). Where #774 moves workflow, flow or memory files, keep those files and the database at the same recovery point. A future 2.5-side version guard would be separate work.
- [ ] A failed migration stops loudly and leaves a recoverable, detectable state.
- [ ] A real 2.5 database upgrades to 3.0 losing nothing — proven for **both** a v2.5.0 database (no `idempotency_keys`) and a v2.5.1 one.
- [ ] On a laptop, no new configuration or prompts are needed, and the database remains `cli-agent-orchestrator.db` in the same directory. Preserve existing default content paths through #774's local storage mapping. Document WAL companion files and consistent backup/recovery without requiring users to migrate data by hand.
- [ ] PostgreSQL is **not** required, and the seam leaves it possible later.
- [ ] An automated check proves no worker image or pod mounts or opens the central database, and that persistent CAO control-plane/metadata mutations go through the central server. Approved runtime workspace/artifact writes remain supported under #745; artifact registration and other canonical CAO state still cross the server boundary. Until this passes, the premise of this issue is unenforced.
**Include worker and MCP startup in that check.** Importing `clients/database.py` calls `_ensure_db_dir()` (`:402`) and can create `DB_DIR` (`:394`) before any connection is opened. The memory plugins already avoid this during MCP discovery by deferring their server-only database imports (`plugins/builtin/claude_code_memory.py:39-55`); preserve that valid fix rather than requiring a replacement abstraction. Exercise startup with central database storage unavailable and catch accidental CAO database imports or initialisation. This does not prohibit the worker-local FIFOs, logs or provider-owned storage needed to run the agent.
- [ ] Classify all direct state-access callers in #745's capability matrix, including `cli/commands/init.py`, `cli/commands/schedule.py`, `cli/commands/memory.py` and `services/memory_gateway.py`. With a shared target selected, user operations go through the central server; they must not silently read or mutate a client/worker copy of the data.
- [ ] Genuine client-local work and operator-local initialization/offline maintenance remain explicitly identified. An unreachable shared server is not permission to fall back to local state, and declaring a user capability local-only is not a substitute for preserving it remotely.
- [ ] The HTTP path preserves the capabilities of the local service path. In particular, connecting schedule commands to today's narrower flow-create model must not silently discard engine or pre-script metadata (#745).
## Out of scope
- Adopting PostgreSQL, or any separate database service.
- Running several cao-server replicas — a distinct problem with several non-database blockers.
- A general-purpose scheduled backup service, disaster recovery automation and failover. The pre-upgrade backup and recovery procedure needed for this migration is in scope.
- Cross-process event delivery — that's #776, and it is where the real difficulty in 3.0 lives.
## Evidence
| Claim | Where |
| --- | --- |
| Main engine passes no `busy_timeout` and no `journal_mode` | `clients/database.py:403` |
| But the effective timeout is already 5000 ms, from Python's default | measured: `PRAGMA busy_timeout` on a fresh connection |
| WAL genuinely absent | measured: `PRAGMA journal_mode` reports `delete` |
| WAL deliberately left out of earlier scope | `constants.py:814`; `services/workflow_journal.py:292` |
| Journal sets its own `busy_timeout` | `services/workflow_journal.py:316` |
| Migrations swallow failures | `services/workflow_journal.py:266` — *"returning is not succeeding"* |
| No schema version marker | 0 matches for `PRAGMA user_version` |
| SQLite coupling | 7 hand-written `CREATE TABLE`, 10 `PRAGMA` (9 are `table_info` migration gates, 1 is `busy_timeout`), 3 `INSERT OR IGNORE/REPLACE`, 6 files importing `sqlite3` |
| Single replica on an attached volume | `examples/cao-clusters/kubernetes/eks/supervisor.yaml:8,17,183-187` |
| SQLite/flock/FIFOs unsafe over network storage | same file, storage comment |
All repository observations verified on `main` at `29b235cf62ed0f9d624bc9ad9afce09ab72f8ddf`.
Contributor guide
Research direction
Start by reproducing a database-locked failure under Python’s real five-second timeout, then inspect the six files importing sqlite3 and the migration paths using PRAGMA table_info. Read services/session_service.py, workflow_journal.py, workflow_retention.py, constants.py, and api/main.py for the named access and maintenance paths. Done means the phased exits hold: one data-access boundary, deliberate SQLite settings, a schema version marker, and migrations that fail loudly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, sqlite
- Domain
- backend, databases
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100