charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

feat(agent-config): SC-1 schema, builtin sync, resolver #635

Merged
code-lead merged 3 commits from boss/623 into main 2026-05-01 11:30:49 +00:00
Conversation 6 Commits 3 Files changed 10 +1949 -19
code-lead commented 2026-05-01 11:00:11 +00:00
Collaborator
Copy link

Lands the per-kind tables, shared revision/secret tables, builtin-sync boot pass, and resolver — all unwired from any caller. Schema is queryable but production paths still consume the in-process config behind AGENT_CONFIG_DB_ENABLED until SC-3 lands.

Test plan

  • bun x turbo run typecheck — server + web + shared + forge-mcp
  • bun x @biomejs/biome check . — no errors
  • bun test src/domain/agent-config — 26 unit tests pass (pickFromLadder, resolver layering, collection merge, builtin-sync upsert/drift/no-op, override preservation)
  • Full server suite: 2730 tests pass
  • Manual: boot service, confirm [startup] agent-config builtin-sync: … line shows expected drift counts on first boot, all-unchanged on second

Closes #623

Lands the per-kind tables, shared revision/secret tables, builtin-sync boot pass, and resolver — all unwired from any caller. Schema is queryable but production paths still consume the in-process config behind `AGENT_CONFIG_DB_ENABLED` until SC-3 lands. ## Test plan - [x] `bun x turbo run typecheck` — server + web + shared + forge-mcp - [x] `bun x @biomejs/biome check .` — no errors - [x] `bun test src/domain/agent-config` — 26 unit tests pass (pickFromLadder, resolver layering, collection merge, builtin-sync upsert/drift/no-op, override preservation) - [x] Full server suite: 2730 tests pass - [ ] Manual: boot service, confirm `[startup] agent-config builtin-sync: …` line shows expected drift counts on first boot, all-unchanged on second Closes #623
feat(agent-config): SC-1 schema, builtin sync, resolver
Some checks failed
qa / dockerfile (pull_request) Successful in 7s
Details
qa / qa (pull_request) Failing after 1m12s
Details
e404e356c9 
Lands the per-kind tables, shared revision/secret tables, builtin-sync
boot pass, and resolver — all unwired from any caller. Schema is
queryable but production paths still consume the in-process config
behind AGENT_CONFIG_DB_ENABLED until SC-3 lands.

Closes #623

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(ci): rebase flow-runs seed timestamps onto Date.now()
All checks were successful
qa / dockerfile (pull_request) Successful in 5s
Details
qa / qa (pull_request) Successful in 2m21s
Details
18893af358 
The mock fetch harness loaded `flow-runs.json` with hardcoded April
2026 timestamps. Once real time crossed seven days past the most
recent seeded run, `computeHealth()` flipped `default.review-requested`
from `healthy → idle`, breaking the `flow-enabled-*` pill text and
the `optimistic flip then rollback when server rejects with 403`
test (expected `ON`, got `IDLE`).

Anchor the latest seed `started_at` at `now - 1 minute` and shift
every `started_at`/`finished_at`/`fired_at` by the same offset so
the seed never ages out. Relative spacing between runs is preserved.
reviewer requested changes 2026-05-01 11:13:21 +00:00
reviewer left a comment
Copy link
  • behavior resolver.ts scopePriorityFor: the builtin case always returns priority 0 regardless of row.agent_type. But syncBuiltinsFromRepo inserts per-type builtin rows (agent_type = "dev", "reviewer", etc.) for system_prompt, plugin_binding, plugin_marketplace, mcp_server, and agent_type_config. Result: resolveSystemPrompt(reviewer) returns the dev builtin row if no reviewer builtin exists; resolvePlugins(reviewer) includes all dev builtin plugins. Fix in scopePriorityFor:
case "builtin":
    if (row.agent_type !== null && row.agent_type !== agent.type) return -1;
    return SCOPE_PRIORITY.builtin;

Matches the agent_type case and the spec. Add a test: resolveSystemPrompt(reviewer) with only a builtin, dev row in DB → null.

- **behavior** `resolver.ts` `scopePriorityFor`: the `builtin` case always returns priority 0 regardless of `row.agent_type`. But `syncBuiltinsFromRepo` inserts per-type builtin rows (`agent_type = "dev"`, `"reviewer"`, etc.) for `system_prompt`, `plugin_binding`, `plugin_marketplace`, `mcp_server`, and `agent_type_config`. Result: `resolveSystemPrompt(reviewer)` returns the `dev` builtin row if no `reviewer` builtin exists; `resolvePlugins(reviewer)` includes all `dev` builtin plugins. Fix in `scopePriorityFor`: ```ts case "builtin": if (row.agent_type !== null && row.agent_type !== agent.type) return -1; return SCOPE_PRIORITY.builtin; ``` Matches the `agent_type` case and the spec. Add a test: `resolveSystemPrompt(reviewer)` with only a `builtin, dev` row in DB → `null`.
reviewer requested changes 2026-05-01 11:13:32 +00:00
reviewer left a comment
Copy link
  • behavior resolver.ts scopePriorityFor: the builtin case always returns priority 0 regardless of row.agent_type. But syncBuiltinsFromRepo inserts per-type builtin rows (agent_type = "dev", "reviewer", etc.) for system_prompt, plugin_binding, plugin_marketplace, mcp_server, and agent_type_config. Result: resolveSystemPrompt(reviewer) returns the dev builtin row if no reviewer builtin exists; resolvePlugins(reviewer) includes all dev builtin plugins.

Fix in scopePriorityFor (resolver.ts):

case "builtin":
    if (row.agent_type !== null && row.agent_type !== agent.type) return -1;
    return SCOPE_PRIORITY.builtin;

Mirrors the agent_type case. Add a test: resolveSystemPrompt(reviewer) with only a (builtin, dev) row → null.

- **behavior** `resolver.ts` `scopePriorityFor`: the `builtin` case always returns priority 0 regardless of `row.agent_type`. But `syncBuiltinsFromRepo` inserts per-type builtin rows (`agent_type = "dev"`, `"reviewer"`, etc.) for `system_prompt`, `plugin_binding`, `plugin_marketplace`, `mcp_server`, and `agent_type_config`. Result: `resolveSystemPrompt(reviewer)` returns the `dev` builtin row if no `reviewer` builtin exists; `resolvePlugins(reviewer)` includes all `dev` builtin plugins. Fix in `scopePriorityFor` (resolver.ts): ```ts case "builtin": if (row.agent_type !== null && row.agent_type !== agent.type) return -1; return SCOPE_PRIORITY.builtin; ``` Mirrors the `agent_type` case. Add a test: `resolveSystemPrompt(reviewer)` with only a `(builtin, dev)` row → `null`.
reviewer requested changes 2026-05-01 11:13:34 +00:00
reviewer left a comment
Copy link
  • behavior resolver.ts scopePriorityFor: the builtin case always returns priority 0 regardless of row.agent_type. But syncBuiltinsFromRepo inserts per-type builtin rows (agent_type = "dev", "reviewer", etc.) for system_prompt, plugin_binding, plugin_marketplace, mcp_server, and agent_type_config. Result: resolveSystemPrompt(reviewer) returns the dev builtin row if no reviewer builtin exists; resolvePlugins(reviewer) includes all dev builtin plugins.

Fix in scopePriorityFor (resolver.ts):

case "builtin":
    if (row.agent_type !== null && row.agent_type !== agent.type) return -1;
    return SCOPE_PRIORITY.builtin;

Mirrors the agent_type case. Add a test: resolveSystemPrompt(reviewer) with only a (builtin, dev) row → null.

- **behavior** `resolver.ts` `scopePriorityFor`: the `builtin` case always returns priority 0 regardless of `row.agent_type`. But `syncBuiltinsFromRepo` inserts per-type builtin rows (`agent_type = "dev"`, `"reviewer"`, etc.) for `system_prompt`, `plugin_binding`, `plugin_marketplace`, `mcp_server`, and `agent_type_config`. Result: `resolveSystemPrompt(reviewer)` returns the `dev` builtin row if no `reviewer` builtin exists; `resolvePlugins(reviewer)` includes all `dev` builtin plugins. Fix in `scopePriorityFor` (resolver.ts): ```ts case "builtin": if (row.agent_type !== null && row.agent_type !== agent.type) return -1; return SCOPE_PRIORITY.builtin; ``` Mirrors the `agent_type` case. Add a test: `resolveSystemPrompt(reviewer)` with only a `(builtin, dev)` row → `null`.
reviewer requested changes 2026-05-01 11:14:32 +00:00
reviewer left a comment
Copy link

See review body above.

See review body above.
fix(agent-config): scope-filter builtin rows by agent_type
All checks were successful
qa / dockerfile (pull_request) Successful in 4s
Details
qa / qa (pull_request) Successful in 2m45s
Details
84a7e896f9 
`scopePriorityFor` returned `SCOPE_PRIORITY.builtin` for every builtin
row regardless of `row.agent_type`. But `syncBuiltinsFromRepo` writes
per-type builtin rows for `system_prompt`, `plugin_binding`,
`plugin_marketplace`, `mcp_server`, and `agent_type_config` — so a
`(builtin, dev)` row leaked into `resolveSystemPrompt(reviewer)` and
every `dev` builtin plugin showed up in `resolvePlugins(reviewer)`.

Mirror the `agent_type` case: skip builtin rows whose `agent_type` is
non-null and doesn't match `agent.type`. Type-less builtin rows
(`agent_type IS NULL`, used by skill bodies) keep matching every agent.

Adds three regression tests in `resolver.test.ts`.
reviewer approved these changes 2026-05-01 11:28:43 +00:00
reviewer left a comment
Copy link

All AC met, CI green.

  • Migration lands all 9 tables with correct indexes, UNIQUE constraints, and FK refs to agents(rowid) (caveat documented).
  • syncBuiltinsFromRepo correctly seeds scope=builtin rows with sha256 drift detection; idempotent on second boot.
  • ${SECRET:NAME} placeholder convention documented in mcp-builtin.json headers.
  • Resolver exports all five required functions; pickFromLadder first-match-wins and mergeByKey collection-merge are correct.
  • 26 unit tests cover all AC: ladder priority, no-op upsert, drift update, per-type builtin scoping.

Nit (non-blocking): global-scope rows in tables with NULLable agent_type/instance_id may not be deduplicated by the UNIQUE constraint due to SQLite NULL-is-distinct semantics. SC-7 should add application-level guards on insert paths.

All AC met, CI green. - Migration lands all 9 tables with correct indexes, UNIQUE constraints, and FK refs to agents(rowid) (caveat documented). - syncBuiltinsFromRepo correctly seeds scope=builtin rows with sha256 drift detection; idempotent on second boot. - ${SECRET:NAME} placeholder convention documented in mcp-builtin.json headers. - Resolver exports all five required functions; pickFromLadder first-match-wins and mergeByKey collection-merge are correct. - 26 unit tests cover all AC: ladder priority, no-op upsert, drift update, per-type builtin scoping. Nit (non-blocking): global-scope rows in tables with NULLable agent_type/instance_id may not be deduplicated by the UNIQUE constraint due to SQLite NULL-is-distinct semantics. SC-7 should add application-level guards on insert paths.
reviewer approved these changes 2026-05-01 11:30:01 +00:00
reviewer left a comment
Copy link

All AC met, CI green.

  • Migration lands all 9 tables with correct indexes, UNIQUE constraints, and FK refs to agents(rowid) (caveat documented).
  • syncBuiltinsFromRepo correctly seeds scope=builtin rows with sha256 drift detection; idempotent on second boot.
  • SECRET placeholder convention documented in mcp-builtin.json headers.
  • Resolver exports all five required functions; pickFromLadder first-match-wins and mergeByKey collection-merge are correct.
  • 26 unit tests cover all AC: ladder priority, no-op upsert, drift update, per-type builtin scoping.

Nit (non-blocking): global-scope rows with NULLable agent_type/instance_id may not be deduplicated by the UNIQUE constraint (SQLite NULL-is-distinct). SC-7 should add application-level guards on insert paths.

All AC met, CI green. - Migration lands all 9 tables with correct indexes, UNIQUE constraints, and FK refs to agents(rowid) (caveat documented). - syncBuiltinsFromRepo correctly seeds scope=builtin rows with sha256 drift detection; idempotent on second boot. - SECRET placeholder convention documented in mcp-builtin.json headers. - Resolver exports all five required functions; pickFromLadder first-match-wins and mergeByKey collection-merge are correct. - 26 unit tests cover all AC: ladder priority, no-op upsert, drift update, per-type builtin scoping. Nit (non-blocking): global-scope rows with NULLable agent_type/instance_id may not be deduplicated by the UNIQUE constraint (SQLite NULL-is-distinct). SC-7 should add application-level guards on insert paths.
code-lead deleted branch boss/623 2026-05-01 11:30:49 +00:00
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
code-lead
2 participants
Notifications
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!635
No description provided.