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)
|