feat(agents): SQLite store + type-defaults config refactor #105

Merged

code-lead merged 2 commits from boss/48 into main 2026-04-19 22:07:00 +00:00

Conversation 4 Commits 2 Files changed 24 +1261 -255

code-lead commented 2026-04-19 21:49:19 +00:00

Collaborator

Copy link

Closes #48.

Summary

Splits config/agents.json into per-type defaults (top-level types: {}) and a SQLite-backed agents table for per-instance overrides. Adding a second dev or reviewer is now a row insert rather than a JSON edit + restart.

First boot after this lands seeds one <type>-default per type and is behaviour-identical to today; one generation of session-key miss on the existing running instances is the only operator-visible effect (per the Out-of-scope notes on the ticket).

Changes

• src/db.ts — new module. Opens the agents DB at stateRoot()/agents.db. Full CRUD: createAgent, getAgent, listAgents, listAgentsByType, updateAgent, deleteAgent. seedDefaultAgents(types) inserts one <type>-default row per declared type when the table is empty.
• src/webhook-config.ts — adds ResolvedAgent (the merged instance + type-defaults shape), resolveAgent(name), resolveAgentByType(type), resolveAgentByUser(login), listResolvedAgents(). The legacy agents: shape fails fast at load with a migration hint pointing at this issue. Required-field validation surfaces types.<name>.<field> paths.
• WorkerConfig carries both name (instance) and type. Workers register per-instance; sessionKey is now <type>:<repo>:<issueOrPr> so pool members of the same type can resume each other's conversations.
• Assign-event dedup keys off the resolved instance name (worker key) instead of the Forgejo login. With one <type>-default per type today behaviour is identical; prepares for the A2 pool scheduler.
• Webhook handlers (issues / reviews / PR sync / fix-ci / merge / post-merge rebase / cleanup) all migrate off cfg.agents[login] to the resolver helpers.
• src/util/test-state-dir.ts — new test helper. Isolates the SQLite DB to a per-test temp dir; six webhook test files migrated onto it.
• justfile + scripts/smoke-creds.sh — every .agents jq path updated to .types so labels-bootstrap, agent-env-sync, agent-plugins-install, containers-* continue to work.
• CLAUDE.md grows an "Agent types vs. instances" section and the module table picks up db.ts.

Acceptance criteria

• Config shape — config/agents.json rewritten as { types: {...} }; loader validates required fields per type with path-qualified errors.
• SQLite layer — src/db.ts with the prescribed schema (agents table + agents_type_idx) and full CRUD surface.
• Merged instance resolver — resolveAgent merges row overrides with type defaults; match_labels parsed to string[], defaults to [].
• Seed migration — first boot inserts one <type>-default per type with no overrides; non-empty table is left alone (operator edits survive).
• Old-shape error — loader throws with a clear migration message referencing #48 when the file still has agents:.
• Session key — sessionKey(type, repo, issueOrPr); old per-instance entries stay on disk and expire via the sweeper.
• Tests — db.test.ts (CRUD, JSON encoding, default seed, schema persistence). webhook-config.test.ts (loader validation, container-dir resolution, first-boot seed, resolveAgent / resolveAgentByType / resolveAgentByUser covering type-default-only, full override, partial override, unknown name/type).

Out of scope (per ticket)

• Pool scheduler (A2)
• Container reconciliation (A5)
• Dashboard UI (A6)
• Migrating historical sessions — accepting one generation of session-key miss on the next service restart.

Test plan

• bun run tsc --noEmit — no errors
• bun x biome check src/ — no errors
• bun test — 339 pass / 0 fail (30 of those new across db.test.ts + webhook-config.test.ts)
• Operator restart on the desktop machine — verify the seed log line ([agents] seeded N default instance(s): boss-default, dev-default, …) appears once on first boot, then stops.
• Verify a routine assignment (@boss on a fresh issue) still dispatches to boss-default and the resulting PR carries the boss Forgejo identity.

🤖 Generated with Claude Code

Closes #48. ## Summary Splits `config/agents.json` into per-*type* defaults (top-level `types: {}`) and a SQLite-backed `agents` table for per-*instance* overrides. Adding a second `dev` or `reviewer` is now a row insert rather than a JSON edit + restart. First boot after this lands seeds one `<type>-default` per type and is behaviour-identical to today; one generation of session-key miss on the existing running instances is the only operator-visible effect (per the Out-of-scope notes on the ticket). ## Changes - **`src/db.ts`** — new module. Opens the agents DB at `stateRoot()/agents.db`. Full CRUD: `createAgent`, `getAgent`, `listAgents`, `listAgentsByType`, `updateAgent`, `deleteAgent`. `seedDefaultAgents(types)` inserts one `<type>-default` row per declared type when the table is empty. - **`src/webhook-config.ts`** — adds `ResolvedAgent` (the merged instance + type-defaults shape), `resolveAgent(name)`, `resolveAgentByType(type)`, `resolveAgentByUser(login)`, `listResolvedAgents()`. The legacy `agents:` shape fails fast at load with a migration hint pointing at this issue. Required-field validation surfaces `types.<name>.<field>` paths. - **`WorkerConfig`** carries both `name` (instance) and `type`. Workers register per-instance; `sessionKey` is now `<type>:<repo>:<issueOrPr>` so pool members of the same type can resume each other's conversations. - **Assign-event dedup** keys off the resolved instance name (worker key) instead of the Forgejo login. With one `<type>-default` per type today behaviour is identical; prepares for the A2 pool scheduler. - **Webhook handlers** (issues / reviews / PR sync / fix-ci / merge / post-merge rebase / cleanup) all migrate off `cfg.agents[login]` to the resolver helpers. - **`src/util/test-state-dir.ts`** — new test helper. Isolates the SQLite DB to a per-test temp dir; six webhook test files migrated onto it. - **`justfile` + `scripts/smoke-creds.sh`** — every `.agents` jq path updated to `.types` so `labels-bootstrap`, `agent-env-sync`, `agent-plugins-install`, `containers-*` continue to work. - **CLAUDE.md** grows an "Agent types vs. instances" section and the module table picks up `db.ts`. ## Acceptance criteria - [x] **Config shape** — `config/agents.json` rewritten as `{ types: {...} }`; loader validates required fields per type with path-qualified errors. - [x] **SQLite layer** — `src/db.ts` with the prescribed schema (`agents` table + `agents_type_idx`) and full CRUD surface. - [x] **Merged instance resolver** — `resolveAgent` merges row overrides with type defaults; `match_labels` parsed to `string[]`, defaults to `[]`. - [x] **Seed migration** — first boot inserts one `<type>-default` per type with no overrides; non-empty table is left alone (operator edits survive). - [x] **Old-shape error** — loader throws with a clear migration message referencing #48 when the file still has `agents:`. - [x] **Session key** — `sessionKey(type, repo, issueOrPr)`; old per-instance entries stay on disk and expire via the sweeper. - [x] **Tests** — `db.test.ts` (CRUD, JSON encoding, default seed, schema persistence). `webhook-config.test.ts` (loader validation, container-dir resolution, first-boot seed, `resolveAgent` / `resolveAgentByType` / `resolveAgentByUser` covering type-default-only, full override, partial override, unknown name/type). ## Out of scope (per ticket) - Pool scheduler (A2) - Container reconciliation (A5) - Dashboard UI (A6) - Migrating historical sessions — accepting one generation of session-key miss on the next service restart. ## Test plan - [x] `bun run tsc --noEmit` — no errors - [x] `bun x biome check src/` — no errors - [x] `bun test` — 339 pass / 0 fail (30 of those new across `db.test.ts` + `webhook-config.test.ts`) - [ ] Operator restart on the desktop machine — verify the seed log line (`[agents] seeded N default instance(s): boss-default, dev-default, …`) appears once on first boot, then stops. - [ ] Verify a routine assignment (`@boss` on a fresh issue) still dispatches to `boss-default` and the resulting PR carries the `boss` Forgejo identity. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit 2026-04-19 21:49:19 +00:00

feat(agents): SQLite store + type-defaults config refactor (#48) ... bb56a803cf

Split `config/agents.json` into per-*type* defaults (top-level
`types: {}`) and a SQLite-backed `agents` table for per-*instance*
overrides. Adding a second `dev` or `reviewer` is now a row insert
rather than a JSON edit + restart.

- New `src/db.ts` opens the agents DB at `stateRoot()/agents.db`
  with full CRUD (`createAgent`, `getAgent`, `listAgents`,
  `listAgentsByType`, `updateAgent`, `deleteAgent`) and a
  `seedDefaultAgents` helper that inserts one `<type>-default` row
  per declared type when the table is empty.
- `webhook-config.ts` exports `ResolvedAgent` (the merged view of an
  instance + its type defaults), `resolveAgent(name)`,
  `resolveAgentByType(type)`, `resolveAgentByUser(login)`, and
  `listResolvedAgents()`. The legacy `agents:` shape fails fast at
  load with a migration hint pointing at this issue.
- `WorkerConfig` carries both `name` (instance) and `type`. Workers
  are registered per instance; the session key (`sessionKey`) is now
  `<type>:<repo>:<issueOrPr>` so pool members of the same type can
  resume each other's conversations.
- The assign-event dedup map keys off the resolved instance name
  (worker key) instead of the assignee Forgejo login. With one
  `<type>-default` per type today behaviour is identical; the change
  prepares for the A2 pool scheduler.
- Webhook handlers (issues / reviews / PR sync / fix-ci / merge /
  post-merge rebase / cleanup) all migrated off
  `cfg.agents[login]` to the resolver helpers.
- New `src/util/test-state-dir.ts` test helper isolates the SQLite
  DB to a per-test temp dir; six webhook test files migrated onto it.
- `justfile` recipes (labels-bootstrap, agent-env-sync,
  agent-plugins-install, containers-*) and `scripts/smoke-creds.sh`
  updated to read `.types` instead of `.agents` from agents.json.
- CLAUDE.md grows a new "Agent types vs. instances" section and the
  module table picks up `db.ts`.

First boot after this lands seeds one `<type>-default` per type and
is behaviour-identical to today; one generation of session-key miss
on the existing running instances is the only operator-visible
effect, per the Out-of-scope notes on the ticket.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-lead requested review from reviewer 2026-04-19 21:52:09 +00:00

reviewer requested changes 2026-04-19 21:55:28 +00:00

Dismissed

reviewer left a comment

Copy link

CI: ✅ Green (bb56a803 — 1 run, success, 2m46s)

The architecture is solid. SQLite layer, type/instance split, seed migration, resolver, session-key change, and test coverage all look correct. Found one correctness regression and two minor issues.

---

🔴 Bug: /reset drops the wrong session key — src/main.ts

handleReset passes the instance name to sessionKey(), but sessions are now keyed by agent type. The endpoint will never successfully drop a session after this PR.

In src/main.ts (handleReset):

const agentName = String(b.agent);               // e.g. "boss-default"
const sKey = sessionKey(agentName, repo, issue); // → "boss-default:repo:42"

But sessions are written with the type in every skillForEvent call in webhook-handlers.ts:

sessionKey(agent.type, repo, issueOrPr)          // → "boss:repo:42"

dropSession("boss-default:repo:42") always returns false — no such key exists. The response silently reports { dropped: { session: false } } even when a real session is present. Operator calls /reset to unstick an issue, nothing happens.

Fix — look up the type from the registered worker:

const worker = workers.get(agentName);
const sKey = sessionKey(worker?.config.type ?? agentName, repo, issue);

---

🟡 resolveAgent(name) does a full table scan instead of a primary-key lookup — src/webhook-config.ts

export function resolveAgent(name: string): ResolvedAgent | null {
    if (!config) return null;
    const rows = listAgents();                    // fetches every row
    const row = rows.find((r) => r.name === name); // JS linear scan

getAgent(name) already exists in db.ts and uses the name TEXT PRIMARY KEY index. At today's agent count the cost is negligible, but the semantics are wrong — there's a purpose-built function for this.

Fix:

const row = getAgent(name);
if (!row) return null;

---

🟡 db.test.ts and webhook-config.test.ts duplicate the boilerplate that useTempStateDir was introduced to eliminate

The PR creates src/util/test-state-dir.ts specifically to centralise the mkdtempSync + setenv + teardown pattern and migrates six webhook test files onto it. But both new test files (db.test.ts lines 12–37, webhook-config.test.ts lines 21–54) still inline their own setEnv / restoreEnv / tempStateDir copies.

Drop the inline boilerplate in both files and use the helper:

import { useTempStateDir } from "./util/test-state-dir";
const stateDir = useTempStateDir();
beforeEach(() => { stateDir.setup(); resetDb(); });
afterEach(() => { resetDb(); stateDir.teardown(); });

---

The /reset session-key issue is a silent correctness regression — the fix is two lines. Everything else (config shape, SQLite schema, seed logic, resolver merging, session-key rename in handlers, test coverage) is correct and well-structured.

**CI: ✅ Green** (`bb56a803` — 1 run, success, 2m46s) The architecture is solid. SQLite layer, type/instance split, seed migration, resolver, session-key change, and test coverage all look correct. Found one correctness regression and two minor issues. --- ## 🔴 Bug: `/reset` drops the wrong session key — `src/main.ts` `handleReset` passes the **instance name** to `sessionKey()`, but sessions are now keyed by **agent type**. The endpoint will never successfully drop a session after this PR. In `src/main.ts` (handleReset): ```typescript const agentName = String(b.agent); // e.g. "boss-default" const sKey = sessionKey(agentName, repo, issue); // → "boss-default:repo:42" ``` But sessions are written with the type in every `skillForEvent` call in `webhook-handlers.ts`: ```typescript sessionKey(agent.type, repo, issueOrPr) // → "boss:repo:42" ``` `dropSession("boss-default:repo:42")` always returns `false` — no such key exists. The response silently reports `{ dropped: { session: false } }` even when a real session is present. Operator calls `/reset` to unstick an issue, nothing happens. **Fix** — look up the type from the registered worker: ```typescript const worker = workers.get(agentName); const sKey = sessionKey(worker?.config.type ?? agentName, repo, issue); ``` --- ## 🟡 `resolveAgent(name)` does a full table scan instead of a primary-key lookup — `src/webhook-config.ts` ```typescript export function resolveAgent(name: string): ResolvedAgent | null { if (!config) return null; const rows = listAgents(); // fetches every row const row = rows.find((r) => r.name === name); // JS linear scan ``` `getAgent(name)` already exists in `db.ts` and uses the `name TEXT PRIMARY KEY` index. At today's agent count the cost is negligible, but the semantics are wrong — there's a purpose-built function for this. **Fix:** ```typescript const row = getAgent(name); if (!row) return null; ``` --- ## 🟡 `db.test.ts` and `webhook-config.test.ts` duplicate the boilerplate that `useTempStateDir` was introduced to eliminate The PR creates `src/util/test-state-dir.ts` specifically to centralise the `mkdtempSync + setenv + teardown` pattern and migrates six webhook test files onto it. But both **new** test files (`db.test.ts` lines 12–37, `webhook-config.test.ts` lines 21–54) still inline their own `setEnv` / `restoreEnv` / `tempStateDir` copies. Drop the inline boilerplate in both files and use the helper: ```typescript import { useTempStateDir } from "./util/test-state-dir"; const stateDir = useTempStateDir(); beforeEach(() => { stateDir.setup(); resetDb(); }); afterEach(() => { resetDb(); stateDir.teardown(); }); ``` --- The `/reset` session-key issue is a silent correctness regression — the fix is two lines. Everything else (config shape, SQLite schema, seed logic, resolver merging, session-key rename in handlers, test coverage) is correct and well-structured.

code-lead referenced this pull request from a commit 2026-04-19 21:58:22 +00:00

fix(reset+resolver): session-key uses agent type; resolveAgent uses PK lookup

code-lead added 1 commit 2026-04-19 21:58:22 +00:00

fix(reset+resolver): session-key uses agent type; resolveAgent uses PK lookup ... e6e62fd716

Three review fixes from PR #105:

- handleReset passed the instance name to sessionKey(), but sessions are
  keyed by agent type since #48. dropSession was always returning false
  for legitimate resets — operator-visible silent regression. Look up
  the type from the registered worker.
- resolveAgent fetched the whole agents table and JS-filtered by name.
  Replace with the existing getAgent(name) PK lookup.
- db.test.ts and webhook-config.test.ts re-inlined the setEnv /
  restoreEnv / mkdtempSync boilerplate that useTempStateDir was added to
  centralise. Migrate both onto the helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-lead commented 2026-04-19 21:58:30 +00:00

Author

Collaborator

Copy link

All three addressed in e6e62fd.

• /reset session-key bug — fixed. handleReset now reads worker.config.type and passes that to sessionKey(), matching what webhook-handlers.ts writes. The worker is already looked up earlier in the handler, so this is a one-line change with no extra DB hit.
• resolveAgent full-scan — replaced with getAgent(name). Imported alongside the existing listAgents / listAgentsByType from db.ts.
• Test boilerplate duplication — both db.test.ts and webhook-config.test.ts now use useTempStateDir. Net -32 lines across the two files; behaviour identical (agentsDbPath test now reads stateDir.path() instead of the local tempRoot).

bun test → 339 pass / 0 fail. Typecheck + biome clean.

All three addressed in `e6e62fd`. - **`/reset` session-key bug** — fixed. `handleReset` now reads `worker.config.type` and passes that to `sessionKey()`, matching what `webhook-handlers.ts` writes. The worker is already looked up earlier in the handler, so this is a one-line change with no extra DB hit. - **`resolveAgent` full-scan** — replaced with `getAgent(name)`. Imported alongside the existing `listAgents` / `listAgentsByType` from `db.ts`. - **Test boilerplate duplication** — both `db.test.ts` and `webhook-config.test.ts` now use `useTempStateDir`. Net `-32` lines across the two files; behaviour identical (`agentsDbPath` test now reads `stateDir.path()` instead of the local `tempRoot`). `bun test` → 339 pass / 0 fail. Typecheck + biome clean.

code-lead requested review from reviewer 2026-04-19 22:01:13 +00:00

reviewer approved these changes 2026-04-19 22:05:39 +00:00

Dismissed

reviewer left a comment

Copy link

Review: feat(agents): SQLite store + type-defaults config refactor

CI: ✅ Run #1647 — success (2m49s) on head e6e62fd.

---

Acceptance criteria — all met

Criterion	Status
Config shape { types: {...} } with required-field validation	✅
src/db.ts with exact prescribed schema + agents_type_idx	✅
resolveAgent / resolveAgentByType / resolveAgentByUser mergers	✅
seedDefaultAgents seeds once, leaves non-empty table alone	✅
Old-shape guard throws with clear #48 migration hint	✅
sessionKey(type, repo, issueOrPr) — type-keyed	✅
db.test.ts + webhook-config.test.ts — 30 new tests	✅

---

Code review

The implementation is clean and well-structured. A few positive observations worth calling out explicitly:

parseMatchLabels defensive degradation (src/db.ts) — null, malformed JSON, non-array, and mixed-type arrays all safely return []. Test coverage on this is thorough including the mixed-type case.

updateAgent null-vs-undefined semantics (src/db.ts) — undefined leaves a field untouched; explicit null clears the override so the type default takes over on next resolution. This is the right API shape for per-instance overrides.

getDb() path-keyed re-open (src/db.ts) — when CLAUDE_HOOKS_STATE_DIR flips between tests, the stale handle is closed and a fresh one opened. Clean isolation without requiring a global reset function visible outside the module.

Worker registration carries both name and type (src/main.ts, src/worker.ts) — the instance name is the worker-registry key; config.type is used for session-key derivation and the promptTooLongHint table. Consistent throughout handlers, CI dispatch, and the /reset endpoint.

Session-key continuity on upgrade — since old instance names (boss, dev, …) match the new type names exactly, the new key <type>:<repo>:<issueOrPr> produces the same strings as the old key. Existing sessions survive the restart without a miss — the PR description is more cautious than necessary here.

---

One minor note (not a blocker)

seedDefaultAgents — no explicit transaction (src/db.ts, inside the for loop): the five INSERTs commit individually. A process kill between rows would leave a partially-seeded table that the c > 0 guard would then skip on next boot. With synchronous SQLite on modern hardware the window is negligible, and an operator can recover by hand, but wrapping the loop in db.transaction(...)() would make the seed atomic at no cost. Not blocking; flagging for a future cleanup.

---

All acceptance criteria met, CI green, no safety issues. Approved.

## Review: feat(agents): SQLite store + type-defaults config refactor **CI:** ✅ Run #1647 — success (2m49s) on head `e6e62fd`. --- ### Acceptance criteria — all met | Criterion | Status | |---|---| | Config shape `{ types: {...} }` with required-field validation | ✅ | | `src/db.ts` with exact prescribed schema + `agents_type_idx` | ✅ | | `resolveAgent` / `resolveAgentByType` / `resolveAgentByUser` mergers | ✅ | | `seedDefaultAgents` seeds once, leaves non-empty table alone | ✅ | | Old-shape guard throws with clear `#48` migration hint | ✅ | | `sessionKey(type, repo, issueOrPr)` — type-keyed | ✅ | | `db.test.ts` + `webhook-config.test.ts` — 30 new tests | ✅ | --- ### Code review The implementation is clean and well-structured. A few positive observations worth calling out explicitly: **`parseMatchLabels` defensive degradation** (`src/db.ts`) — null, malformed JSON, non-array, and mixed-type arrays all safely return `[]`. Test coverage on this is thorough including the mixed-type case. **`updateAgent` null-vs-undefined semantics** (`src/db.ts`) — `undefined` leaves a field untouched; explicit `null` clears the override so the type default takes over on next resolution. This is the right API shape for per-instance overrides. **`getDb()` path-keyed re-open** (`src/db.ts`) — when `CLAUDE_HOOKS_STATE_DIR` flips between tests, the stale handle is closed and a fresh one opened. Clean isolation without requiring a global reset function visible outside the module. **Worker registration carries both `name` and `type`** (`src/main.ts`, `src/worker.ts`) — the instance name is the worker-registry key; `config.type` is used for session-key derivation and the `promptTooLongHint` table. Consistent throughout handlers, CI dispatch, and the `/reset` endpoint. **Session-key continuity on upgrade** — since old instance names (`boss`, `dev`, …) match the new type names exactly, the new key `<type>:<repo>:<issueOrPr>` produces the same strings as the old key. Existing sessions survive the restart without a miss — the PR description is more cautious than necessary here. --- ### One minor note (not a blocker) **`seedDefaultAgents` — no explicit transaction** (`src/db.ts`, inside the `for` loop): the five INSERTs commit individually. A process kill between rows would leave a partially-seeded table that the `c > 0` guard would then skip on next boot. With synchronous SQLite on modern hardware the window is negligible, and an operator can recover by hand, but wrapping the loop in `db.transaction(...)()` would make the seed atomic at no cost. Not blocking; flagging for a future cleanup. --- All acceptance criteria met, CI green, no safety issues. **Approved.**

reviewer approved these changes 2026-04-19 22:05:49 +00:00

reviewer left a comment

Copy link

Review: feat(agents): SQLite store + type-defaults config refactor

CI: ✅ Run #1647 — success (2m49s) on head e6e62fd.

---

Acceptance criteria — all met

Criterion	Status
Config shape { types: {...} } with required-field validation	✅
src/db.ts with exact prescribed schema + agents_type_idx	✅
resolveAgent / resolveAgentByType / resolveAgentByUser mergers	✅
seedDefaultAgents seeds once, leaves non-empty table alone	✅
Old-shape guard throws with clear #48 migration hint	✅
sessionKey(type, repo, issueOrPr) — type-keyed	✅
db.test.ts + webhook-config.test.ts — 30 new tests	✅

---

Code review

The implementation is clean and well-structured.

parseMatchLabels defensive degradation (src/db.ts) — null, malformed JSON, non-array, and mixed-type arrays all safely return []. Test coverage is thorough.

updateAgent null-vs-undefined semantics (src/db.ts) — undefined leaves a field untouched; explicit null clears the override so the type default takes over on next resolution. Correct API shape.

getDb() path-keyed re-open (src/db.ts) — when CLAUDE_HOOKS_STATE_DIR flips between tests the stale handle is closed and a fresh one opened. Clean test isolation.

Worker registration carries both name and type (src/main.ts, src/worker.ts) — instance name is the registry key; config.type drives session-key derivation and promptTooLongHint. Consistent across handlers, CI dispatch, and /reset.

Session-key continuity on upgrade — old instance names (boss, dev, …) match the new type names exactly, so <type>:<repo>:<issueOrPr> produces the same strings as the old key. Existing sessions survive the restart with no miss. The PR description is more cautious than necessary.

---

One minor note (not a blocker)

seedDefaultAgents — no explicit transaction (src/db.ts, for loop): the five INSERTs commit individually. A crash between rows leaves a partially-seeded table that the c > 0 guard skips on next boot. Wrapping the loop in db.transaction(...)() would make the seed atomic at no cost. Not blocking.

---

All acceptance criteria met, CI green, no safety issues. Approved.

## Review: feat(agents): SQLite store + type-defaults config refactor **CI:** ✅ Run #1647 — success (2m49s) on head `e6e62fd`. --- ### Acceptance criteria — all met | Criterion | Status | |---|---| | Config shape `{ types: {...} }` with required-field validation | ✅ | | `src/db.ts` with exact prescribed schema + `agents_type_idx` | ✅ | | `resolveAgent` / `resolveAgentByType` / `resolveAgentByUser` mergers | ✅ | | `seedDefaultAgents` seeds once, leaves non-empty table alone | ✅ | | Old-shape guard throws with clear `#48` migration hint | ✅ | | `sessionKey(type, repo, issueOrPr)` — type-keyed | ✅ | | `db.test.ts` + `webhook-config.test.ts` — 30 new tests | ✅ | --- ### Code review The implementation is clean and well-structured. **`parseMatchLabels` defensive degradation** (`src/db.ts`) — null, malformed JSON, non-array, and mixed-type arrays all safely return `[]`. Test coverage is thorough. **`updateAgent` null-vs-undefined semantics** (`src/db.ts`) — `undefined` leaves a field untouched; explicit `null` clears the override so the type default takes over on next resolution. Correct API shape. **`getDb()` path-keyed re-open** (`src/db.ts`) — when `CLAUDE_HOOKS_STATE_DIR` flips between tests the stale handle is closed and a fresh one opened. Clean test isolation. **Worker registration carries both `name` and `type`** (`src/main.ts`, `src/worker.ts`) — instance name is the registry key; `config.type` drives session-key derivation and `promptTooLongHint`. Consistent across handlers, CI dispatch, and `/reset`. **Session-key continuity on upgrade** — old instance names (`boss`, `dev`, …) match the new type names exactly, so `<type>:<repo>:<issueOrPr>` produces the same strings as the old key. Existing sessions survive the restart with no miss. The PR description is more cautious than necessary. --- ### One minor note (not a blocker) **`seedDefaultAgents` — no explicit transaction** (`src/db.ts`, `for` loop): the five INSERTs commit individually. A crash between rows leaves a partially-seeded table that the `c > 0` guard skips on next boot. Wrapping the loop in `db.transaction(...)()` would make the seed atomic at no cost. Not blocking. --- All acceptance criteria met, CI green, no safety issues. **Approved.**

code-lead merged commit c630900819 into main 2026-04-19 22:07:00 +00:00

code-lead deleted branch boss/48 2026-04-19 22:07:01 +00:00

code-lead referenced this pull request from a commit 2026-04-19 22:07:02 +00:00

feat(agents): SQLite store + type-defaults config refactor (#105)

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!105

No description provided.