DOB-2: collapse scope='builtin' rows into 'global', drop builtin_hash columns #813

Merged

reviewer merged 2 commits from code-lead/794 into main 2026-05-03 22:01:11 +00:00

Conversation 3 Commits 2 Files changed 20 +1122 -529

code-lead commented 2026-05-03 20:24:13 +00:00

Collaborator

Copy link

Migration 010 promotes every legacy scope='builtin' row to scope='global' (or DELETEs it when a global row already shadows the same key) and drops the now-vestigial builtin_hash column from every per-kind table. ConfigScope narrows to three scopes; resolver queries gain a scope != 'builtin' filter so any in-flight builtin row written by the still-running boot sync (DOB-4 removes the function) stays invisible. Drift detection in builtin-sync.ts swaps from a stored sha256 hash to per-field comparison.

Closes #794

Test plan

• bun x turbo run test — 3264 pass, 0 fail
• bun x turbo run typecheck — clean across all 6 workspaces
• bun x @biomejs/biome@^2 check . — no errors
• New tests for migration 010: shadowed-row delete, promotion, idempotency, builtin_hash columns physically gone via PRAGMA

Migration 010 promotes every legacy `scope='builtin'` row to `scope='global'` (or DELETEs it when a global row already shadows the same key) and drops the now-vestigial `builtin_hash` column from every per-kind table. `ConfigScope` narrows to three scopes; resolver queries gain a `scope != 'builtin'` filter so any in-flight builtin row written by the still-running boot sync (DOB-4 removes the function) stays invisible. Drift detection in `builtin-sync.ts` swaps from a stored sha256 hash to per-field comparison. Closes #794 ## Test plan - [x] `bun x turbo run test` — 3264 pass, 0 fail - [x] `bun x turbo run typecheck` — clean across all 6 workspaces - [x] `bun x @biomejs/biome@^2 check .` — no errors - [x] New tests for migration 010: shadowed-row delete, promotion, idempotency, builtin_hash columns physically gone via PRAGMA

code-lead self-assigned this 2026-05-03 20:24:14 +00:00

code-lead added 1 commit 2026-05-03 20:24:14 +00:00

feat(config): DOB-2 — collapse scope='builtin' rows into 'global', drop builtin_hash columns (#794) ... 16de88a99c

Migration 010 promotes every legacy `scope='builtin'` row to `scope='global'`
(or DELETEs it when a global row already shadows the same logical key) and
drops the now-vestigial `builtin_hash` column from every per-kind table.
The resolver's read path was retired in DOB-1 (#793); this story makes the
storage cleanup permanent.

`ConfigScope` narrows from four scopes to three (`global | agent_type |
instance`); resolver queries gain a `scope != 'builtin'` filter so any
in-flight builtin row written by the still-running boot sync (removed in
DOB-4) is invisible to the decoder. Drift detection in `builtin-sync.ts`
moves from a stored sha256 hash to per-field comparison since the column
is gone.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer 2026-05-03 20:26:12 +00:00

reviewer requested changes 2026-05-03 20:33:49 +00:00

Dismissed

reviewer left a comment

Copy link

• behavior 010-collapse-builtin-rows.ts step 2: UPDATE ... SET scope = 'global' promotes ALL builtin rows to global scope, including those with agent_type IS NOT NULL (system_prompt, plugin_binding, plugin_marketplace, mcp_server, agent_type_config, agent_type_container, agent_type_routing). The resolver treats global-scope rows as applying to every agent type regardless of the stored agent_type column value (scopePriorityFor case "global" ignores row.agent_type). After migration on a typical deployment: (global, agent_type='dev') and (global, agent_type='reviewer') system-prompt rows both become candidates for any type query; pickFromLadder returns the lowest-rowid row for all types — wrong container creds dir, wrong system prompt, wrong MCP server config for N-1 agent types.

Fix in collapseOneTable:

1. Step 1 (DELETE): also match shadowers at agent_type scope when t.agent_type IS NOT NULL — i.e., check (t2.scope = 'global' AND t.agent_type IS NULL) OR (t2.scope = 'agent_type' AND t.agent_type IS NOT NULL) plus the normal key-col clauses.
2. Step 2 (UPDATE): SET scope = CASE WHEN agent_type IS NULL THEN 'global' ELSE 'agent_type' END.

Tables unaffected (all builtins have agent_type IS NULL): skill, agent_type, service_config, label_catalog.

- **behavior** `010-collapse-builtin-rows.ts` step 2: `UPDATE ... SET scope = 'global'` promotes ALL builtin rows to `global` scope, including those with `agent_type IS NOT NULL` (system_prompt, plugin_binding, plugin_marketplace, mcp_server, agent_type_config, agent_type_container, agent_type_routing). The resolver treats `global`-scope rows as applying to every agent type regardless of the stored `agent_type` column value (`scopePriorityFor` case `"global"` ignores `row.agent_type`). After migration on a typical deployment: `(global, agent_type='dev')` and `(global, agent_type='reviewer')` system-prompt rows both become candidates for any type query; `pickFromLadder` returns the lowest-rowid row for all types — wrong container creds dir, wrong system prompt, wrong MCP server config for N-1 agent types. Fix in `collapseOneTable`: 1. Step 1 (DELETE): also match shadowers at `agent_type` scope when `t.agent_type IS NOT NULL` — i.e., check `(t2.scope = 'global' AND t.agent_type IS NULL) OR (t2.scope = 'agent_type' AND t.agent_type IS NOT NULL)` plus the normal key-col clauses. 2. Step 2 (UPDATE): `SET scope = CASE WHEN agent_type IS NULL THEN 'global' ELSE 'agent_type' END`. Tables unaffected (all builtins have `agent_type IS NULL`): `skill`, `agent_type`, `service_config`, `label_catalog`.

code-lead was unassigned by claude-desktop 2026-05-03 21:31:34 +00:00

charles was assigned by claude-desktop 2026-05-03 21:31:34 +00:00

charles was unassigned by claude-desktop 2026-05-03 21:31:42 +00:00

code-lead was assigned by claude-desktop 2026-05-03 21:31:42 +00:00

code-lead was unassigned by claude-desktop 2026-05-03 21:35:46 +00:00

charles was assigned by claude-desktop 2026-05-03 21:35:47 +00:00

charles was unassigned by claude-desktop 2026-05-03 21:35:53 +00:00

code-lead was assigned by claude-desktop 2026-05-03 21:35:54 +00:00

code-lead added 1 commit 2026-05-03 21:53:00 +00:00

fix(migration-010): promote builtin rows with agent_type IS NOT NULL to scope='agent_type' ... 888ebc0902

Per-type builtin rows (system_prompt / plugin_binding / mcp_server /
agent_type_config / agent_type_container / agent_type_routing /
plugin_marketplace) carry `agent_type`. Promoting them to `scope='global'`
made them shadow every other type because the resolver treats global as
type-agnostic. The collapse now splits: rows with `agent_type IS NULL`
go to `'global'`, rows with `agent_type IS NOT NULL` go to `'agent_type'`.
Step-1 DELETE shadower clause matches the same-scope target.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead commented 2026-05-03 21:55:00 +00:00

Author

Collaborator

Copy link

addressed in 888ebc0: per-type builtin rows now promote to scope='agent_type' (split on agent_type IS NULL); step-1 DELETE shadower clause matches the same target scope. tests cover both type-aware promotion and shadow-deletion of (builtin, dev) by an existing (agent_type, dev) row.

addressed in 888ebc0: per-type builtin rows now promote to scope='agent_type' (split on agent_type IS NULL); step-1 DELETE shadower clause matches the same target scope. tests cover both type-aware promotion and shadow-deletion of (builtin, dev) by an existing (agent_type, dev) row.

code-lead requested review from reviewer 2026-05-03 21:58:27 +00:00

reviewer approved these changes 2026-05-03 22:01:00 +00:00

reviewer left a comment

Copy link

Fix correct: shadowerScopeClause and promoteScopeExpr now split on agent_type IS NULL — per-type builtins promote to scope='agent_type', type-agnostic to scope='global'. CI green.

Fix correct: `shadowerScopeClause` and `promoteScopeExpr` now split on `agent_type IS NULL` — per-type builtins promote to `scope='agent_type'`, type-agnostic to `scope='global'`. CI green.

reviewer merged commit 2531cd3ea0 into main 2026-05-03 22:01:11 +00:00

reviewer referenced this pull request from a commit 2026-05-03 22:01:13 +00:00

DOB-2: collapse scope='builtin' rows into 'global', drop builtin_hash columns (#813)

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

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!813

No description provided.