244 lines
9.5 KiB
Markdown
244 lines
9.5 KiB
Markdown
# M6.2 — Postgres schema for agent-manager sessions
|
||
|
||
| Field | Value |
|
||
|---|---|
|
||
| Phase | M6 — agent-manager migration |
|
||
| Size | M — 1–3 days |
|
||
| Status | ⬜ Not started |
|
||
| Flags | — |
|
||
| Spec | inlined below |
|
||
| Blocks | M6.1 |
|
||
|
||
## Goal
|
||
|
||
The sqlite schema agent-manager's `internal/store/store.go` builds up
|
||
through 14 `ALTER TABLE` migrations, collapsed into one Postgres-native
|
||
schema — with the sqlite workarounds (integer booleans, dual-encoded
|
||
timestamps) removed rather than carried over.
|
||
|
||
## Facts (inlined — no spec read needed)
|
||
|
||
Current schema, read directly from `internal/store/store.go` (6 tables, no
|
||
FK constraints anywhere — every relationship is enforced in Go, not SQL):
|
||
|
||
```mermaid
|
||
erDiagram
|
||
GROUPS ||--o{ SESSIONS : "group_name (app-level, no FK)"
|
||
SESSIONS ||--o{ SESSIONS : "parent_id (self-ref, app-level, no FK)"
|
||
SESSIONS ||--o| REVIEW_TARGETS : "session_id (app-level, no FK)"
|
||
SESSIONS ||--o{ REVIEW_BASES : "session_id (app-level, no FK)"
|
||
SESSIONS ||--o| REVIEW_SCOPES : "session_id (app-level, no FK)"
|
||
SETTINGS {
|
||
text key PK
|
||
text value
|
||
}
|
||
SESSIONS {
|
||
text id PK
|
||
text name
|
||
text tool
|
||
text cwd
|
||
text group_name "app-level FK -> GROUPS.name"
|
||
text status
|
||
int archived "bool 0/1 in sqlite"
|
||
int created_at "unix nanos, dual-encoded in sqlite"
|
||
int last_status_at "unix nanos"
|
||
text agent_session_id
|
||
text pending_inputs "JSON array blob"
|
||
int pending_claimed "bool 0/1"
|
||
text launch_prompt
|
||
int sort_order
|
||
int acked "bool 0/1"
|
||
text snapshot
|
||
text worktree_repo
|
||
text worktree_branch
|
||
int agent_launched_at "unix nanos"
|
||
text retired_agent_session_id
|
||
text parent_id "app-level FK -> SESSIONS.id, self"
|
||
}
|
||
GROUPS {
|
||
text name PK
|
||
int sort_order
|
||
text path
|
||
int archived "bool 0/1"
|
||
text worktree
|
||
}
|
||
REVIEW_TARGETS {
|
||
text session_id PK
|
||
text repo_root
|
||
}
|
||
REVIEW_BASES {
|
||
text session_id PK
|
||
text repo_root PK
|
||
text base_ref
|
||
}
|
||
REVIEW_SCOPES {
|
||
text session_id PK
|
||
text scope
|
||
}
|
||
```
|
||
|
||
Target Postgres DDL:
|
||
|
||
```sql
|
||
CREATE TABLE groups (
|
||
name TEXT PRIMARY KEY,
|
||
sort_order INTEGER NOT NULL DEFAULT 0,
|
||
path TEXT NOT NULL DEFAULT '',
|
||
archived BOOLEAN NOT NULL DEFAULT FALSE,
|
||
worktree TEXT NOT NULL DEFAULT ''
|
||
);
|
||
|
||
CREATE TABLE sessions (
|
||
id TEXT PRIMARY KEY,
|
||
name TEXT NOT NULL,
|
||
tool TEXT NOT NULL,
|
||
cwd TEXT NOT NULL,
|
||
group_name TEXT NOT NULL REFERENCES groups(name),
|
||
status TEXT NOT NULL,
|
||
archived BOOLEAN NOT NULL DEFAULT FALSE,
|
||
created_at TIMESTAMPTZ NOT NULL,
|
||
last_status_at TIMESTAMPTZ NOT NULL,
|
||
agent_session_id TEXT NOT NULL DEFAULT '',
|
||
pending_inputs JSONB NOT NULL DEFAULT '[]',
|
||
pending_claimed BOOLEAN NOT NULL DEFAULT FALSE,
|
||
launch_prompt TEXT NOT NULL DEFAULT '',
|
||
sort_order INTEGER NOT NULL DEFAULT 0,
|
||
acked BOOLEAN NOT NULL DEFAULT FALSE,
|
||
snapshot TEXT NOT NULL DEFAULT '',
|
||
worktree_repo TEXT NOT NULL DEFAULT '',
|
||
worktree_branch TEXT NOT NULL DEFAULT '',
|
||
agent_launched_at TIMESTAMPTZ,
|
||
retired_agent_session_id TEXT NOT NULL DEFAULT '',
|
||
parent_id TEXT REFERENCES sessions(id)
|
||
);
|
||
|
||
CREATE TABLE settings (
|
||
key TEXT PRIMARY KEY,
|
||
value TEXT NOT NULL
|
||
);
|
||
|
||
CREATE TABLE review_targets (
|
||
session_id TEXT PRIMARY KEY REFERENCES sessions(id) ON DELETE CASCADE,
|
||
repo_root TEXT NOT NULL
|
||
);
|
||
|
||
CREATE TABLE review_bases (
|
||
session_id TEXT NOT NULL REFERENCES sessions(id) ON DELETE CASCADE,
|
||
repo_root TEXT NOT NULL,
|
||
base_ref TEXT NOT NULL,
|
||
PRIMARY KEY (session_id, repo_root)
|
||
);
|
||
|
||
CREATE TABLE review_scopes (
|
||
session_id TEXT PRIMARY KEY REFERENCES sessions(id) ON DELETE CASCADE,
|
||
scope TEXT NOT NULL
|
||
);
|
||
```
|
||
|
||
**Decision: add real FKs, not app-level-only.** Current Go code manually
|
||
cascades `review_targets`/`review_bases`/`review_scopes` deletes before
|
||
deleting a `sessions` row (`Delete`, `DeleteChild` in store.go) — `ON
|
||
DELETE CASCADE` on those three removes that manual bookkeeping entirely.
|
||
`group_name -> groups(name)` and `parent_id -> sessions(id)` get FKs too,
|
||
since Postgres makes them free and they catch the exact class of bug
|
||
`validParent`/`ensureGroup` exist in Go to prevent by hand. This is a
|
||
behavior change from sqlite (constraint violation now possible on write
|
||
paths that previously just wrote garbage) — M6.3 must add error handling
|
||
for FK violations at the ~4 call sites that write `group_name`/
|
||
`parent_id` without having already validated the reference through
|
||
`validParent`/`ensureGroup`.
|
||
|
||
**Booleans** — `BOOLEAN`, not `INTEGER`. Removes `boolToInt()`/`!= 0` at
|
||
every read/write site in store.go.
|
||
|
||
**Timestamps** — `TIMESTAMPTZ`, not the `encodeTime`/`decodeTime`
|
||
nanosecond-or-legacy-seconds hack (store.go:1369-1396). That hack exists
|
||
only because sqlite has no native timestamp type and old rows needed a
|
||
seconds/nanos disambiguation heuristic (`secondsCeiling`). A fresh
|
||
Postgres database has no legacy rows — the whole function pair is deleted,
|
||
not ported. `agent_launched_at` becomes nullable (`NULL` = "never
|
||
restarted") instead of the sqlite sentinel `0`.
|
||
|
||
**`pending_inputs`** — `JSONB`, not `TEXT` holding a JSON string. Same
|
||
`json.Marshal`/`Unmarshal` round-trip in Go either way, but `JSONB` lets
|
||
Postgres validate the shape on write instead of accepting malformed JSON
|
||
that only fails on the next read.
|
||
|
||
## Steps
|
||
|
||
1. Write `agent-manager` migration files (this repo's `sqlx`-migration
|
||
convention doesn't apply — agent-manager is a separate Go repo; use
|
||
whatever migration tool its `add-headless-spawn` branch already has, or
|
||
a plain `.sql` file run once at `Open()` if it has none — check before
|
||
introducing a new dependency).
|
||
2. `CREATE TABLE` in FK-dependency order: `groups`, then `sessions`
|
||
(references `groups` and itself), then `settings`,
|
||
`review_targets`/`review_bases`/`review_scopes`.
|
||
3. No indexes beyond the primary keys are needed yet — `ListSessions`
|
||
filters on `archived` and orders by `group_name, sort_order,
|
||
created_at`; add a composite index only if M6.6's gate shows it's slow,
|
||
not preemptively.
|
||
4. Confirm `CREATE EXTENSION` is not needed anywhere (plain relational
|
||
schema, no pgvector) — unlike this project's own `memory-db`.
|
||
|
||
## Acceptance
|
||
|
||
- Schema applies to a clean `agent-manager-db` database.
|
||
- `archived`/`acked`/`pending_claimed` are `BOOLEAN`, not `INTEGER`.
|
||
- `created_at`/`last_status_at`/`agent_launched_at` are `TIMESTAMPTZ`.
|
||
- Deleting a `sessions` row cascades `review_targets`/`review_bases`/
|
||
`review_scopes` without the Go code doing it manually.
|
||
- Inserting a session with an unknown `group_name` is rejected by the FK,
|
||
not silently written.
|
||
|
||
## Verify
|
||
|
||
**Harness:** disposable Postgres, same image as M6.1's `Cluster`
|
||
(`ghcr.io/cloudnative-pg/postgresql:16.2`) — or a local `postgres:16`
|
||
container for fast iteration, since agent-manager's own test suite doesn't
|
||
need the real cluster.
|
||
|
||
**Integration test** — extend agent-manager's existing store tests
|
||
(`internal/store/*_test.go` already has `timeenc_test.go`; that file is
|
||
deleted in M6.3, its coverage folded into this schema's tests) with:
|
||
1. `a1_migrate_clean` — apply to an empty database, assert all 6 tables
|
||
exist.
|
||
2. `a2_bool_columns_are_boolean` — `information_schema.columns` reports
|
||
`boolean` for `archived`, `acked`, `pending_claimed`.
|
||
3. `a3_timestamp_columns_are_timestamptz` — same, for `created_at`,
|
||
`last_status_at`, `agent_launched_at`.
|
||
4. `a4_review_cascade` — insert a session + a `review_targets` row for it,
|
||
delete the session, assert `review_targets` is empty without a separate
|
||
`DELETE FROM review_targets` call.
|
||
5. `a5_group_fk_rejects_unknown` — insert a session with a `group_name`
|
||
that has no matching `groups` row, assert it's rejected.
|
||
6. `a6_parent_fk_self_ref` — insert two sessions where the second's
|
||
`parent_id` points at the first, assert it succeeds; point it at a
|
||
nonexistent id, assert it's rejected.
|
||
|
||
**Command:** `go test ./internal/store/... -run TestSchema`
|
||
|
||
**False pass:**
|
||
- Testing the schema against sqlite (leftover `modernc.org/sqlite` import)
|
||
instead of real Postgres. `BOOLEAN`/`TIMESTAMPTZ`/`JSONB`/FK-cascade
|
||
behavior all differ or are silently accepted-but-ignored by sqlite —
|
||
every assertion above becomes meaningless against the wrong engine.
|
||
|
||
## Traps
|
||
|
||
- Keeping `ON DELETE CASCADE` off `group_name`/`parent_id` FKs by
|
||
accident (only adding it to the `review_*` ones). A group delete or
|
||
session delete then hits an FK violation instead of the graceful
|
||
"session has terminals of its own; move them out first" error
|
||
`PlaceSession` already gives — check M6.3 preserves that message instead
|
||
of leaking a raw constraint-violation error to the CLI.
|
||
- Making `parent_id` `NOT NULL DEFAULT ''` (matching the sqlite default)
|
||
instead of nullable. `'' REFERENCES sessions(id)` is never satisfiable
|
||
except by a literal empty-string-id row, which doesn't exist — every
|
||
top-level session's insert then fails the FK. Must be `NULL` for "no
|
||
parent."
|
||
|
||
---
|
||
|
||
Background: [M6.1](M6.1-agent-manager-db-manifest.md) · `internal/store/store.go` (agent-manager, `add-headless-spawn` branch)
|