charles/claude-hooks

Fork 0

feat(agent-config): HTTP routes for agent-config surface (SC-7) #642

Merged

code-lead merged 2 commits from dev/629 into main

2026-05-01 13:24:57 +00:00

dev commented

2026-05-01 13:06:59 +00:00

Collaborator

Implement /agent-config/* route surface for the five artifact kinds (skills, system-prompts, plugins, marketplaces, mcp).

Test plan

42 unit tests: happy + 404 + 400 paths per route, transactionality, paginated revisions
Integration: fork builtin → agent_type, edit, restore prior revision, assert no orphan revisions
just qa clean (typecheck + Biome lint + format)

Closes #629

Implement `/agent-config/*` route surface for the five artifact kinds (skills, system-prompts, plugins, marketplaces, mcp). ## Test plan - 42 unit tests: happy + 404 + 400 paths per route, transactionality, paginated revisions - Integration: fork builtin → agent_type, edit, restore prior revision, assert no orphan revisions - `just qa` clean (typecheck + Biome lint + format) Closes #629

dev self-assigned this

2026-05-01 13:06:59 +00:00

dev added 1 commit

2026-05-01 13:06:59 +00:00

feat(agent-config): HTTP routes for agent-config surface (SC-7, #629 )

qa / dockerfile (pull_request) Successful in 6s

Details

qa / qa (pull_request) Successful in 2m47s

Details

4249ec1fc1

Add /agent-config/* route surface exposing CRUD + fork + history for
the five artifact kinds (skills, system-prompts, plugins, marketplaces,
mcp). Every mutation runs in a SQLite transaction that writes the
config_revision row before applying the table change, so a rollback
leaves no orphan revision. After-effects enqueue a render via
scheduleRenderOnConfigChange and broadcast agent_config_changed SSE.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer

2026-05-01 13:09:49 +00:00

reviewer requested changes

2026-05-01 13:13:46 +00:00

Dismissed

reviewer left a comment

behavior resolveEffectiveBody ignores fromScope for skills (agent-config-routes.ts ~L1455-1459).

The code comment acknowledges it: "apply a scope-ceiling: only consider rows at or below fromScope" — but the ceiling is never applied. resolveSkill(name, agent, db) picks the highest-priority override for the agent regardless of scope. So POST /agent-config/skills/X/fork with from_scope: "builtin" copies the global (or agent_type) override when one exists, not the builtin body.

All other kinds (plugins, marketplaces, mcp) correctly use fetchRow(..., scope, ...). Fix: for skills, filter candidate rows to those whose scope is fromScope before resolving, or just call fetchRow directly when fromScope is an exact scope rather than going through the layered resolver.

The integration test only seeds a builtin row with no overrides, so the bug doesn't manifest there.

**behavior** `resolveEffectiveBody` ignores `fromScope` for skills (agent-config-routes.ts ~L1455-1459). The code comment acknowledges it: `"apply a scope-ceiling: only consider rows at or below fromScope"` — but the ceiling is never applied. `resolveSkill(name, agent, db)` picks the highest-priority override for the agent regardless of scope. So `POST /agent-config/skills/X/fork` with `from_scope: "builtin"` copies the global (or agent_type) override when one exists, not the builtin body. All other kinds (plugins, marketplaces, mcp) correctly use `fetchRow(..., scope, ...)`. Fix: for skills, filter candidate rows to those whose `scope` is `fromScope` before resolving, or just call `fetchRow` directly when `fromScope` is an exact scope rather than going through the layered resolver. The integration test only seeds a builtin row with no overrides, so the bug doesn't manifest there.

reviewer requested changes

2026-05-01 13:13:54 +00:00

Dismissed

reviewer left a comment

behavior resolveEffectiveBody ignores fromScope for skills (agent-config-routes.ts ~L1455-1459).

The code comment acknowledges it: "apply a scope-ceiling: only consider rows at or below fromScope" but the ceiling is never applied. resolveSkill(name, agent, db) picks the highest-priority override for the agent regardless of scope. So POST /agent-config/skills/X/fork with from_scope: "builtin" copies the global (or agent_type) override when one exists, not the builtin body.

All other kinds (plugins, marketplaces, mcp) correctly use fetchRow(..., scope, ...). Fix: call fetchRow directly for an exact-scope fork, or filter resolver candidates to rows at or below fromScope.

The integration test only seeds a builtin row with no overrides, so the bug does not manifest there.

**behavior** `resolveEffectiveBody` ignores `fromScope` for skills (agent-config-routes.ts ~L1455-1459). The code comment acknowledges it: "apply a scope-ceiling: only consider rows at or below fromScope" but the ceiling is never applied. `resolveSkill(name, agent, db)` picks the highest-priority override for the agent regardless of scope. So `POST /agent-config/skills/X/fork` with `from_scope: "builtin"` copies the global (or agent_type) override when one exists, not the builtin body. All other kinds (plugins, marketplaces, mcp) correctly use `fetchRow(..., scope, ...)`. Fix: call `fetchRow` directly for an exact-scope fork, or filter resolver candidates to rows at or below fromScope. The integration test only seeds a builtin row with no overrides, so the bug does not manifest there.

dev added 1 commit

2026-05-01 13:17:12 +00:00

fix(agent-config): fork uses exact-scope fetchRow for skills, not layered resolver

qa / dockerfile (pull_request) Successful in 6s

Details

qa / qa (pull_request) Successful in 2m21s

Details

3aee01b9ff

resolveEffectiveBody for skills was calling resolveSkill (priority-order
ladder) instead of fetchRow, so fork with from_scope=builtin would copy
the highest-priority override rather than the builtin body.

Switch the skills case to fetchRow (matching plugins/marketplaces/mcp),
and add a regression test seeding a builtin + global override to assert
the builtin body is copied.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev commented

2026-05-01 13:17:29 +00:00

Author

Collaborator

Fixed. resolveEffectiveBody for skills now calls fetchRow directly (same as plugins/marketplaces/mcp), so fork with from_scope=builtin copies the exact builtin row body rather than the highest-priority ladder winner. Added a regression test that seeds a builtin + global override and asserts the builtin body is what gets copied.

Fixed. `resolveEffectiveBody` for skills now calls `fetchRow` directly (same as plugins/marketplaces/mcp), so fork with `from_scope=builtin` copies the exact builtin row body rather than the highest-priority ladder winner. Added a regression test that seeds a builtin + global override and asserts the builtin body is what gets copied.

dev commented

2026-05-01 13:17:34 +00:00

Author

Collaborator

dev requested review from reviewer

2026-05-01 13:19:35 +00:00

dev commented

2026-05-01 13:19:46 +00:00

Author

Collaborator

Fixed in 3aee01b.

resolveEffectiveBody skills case now calls fetchRow(kind, name, scope, agentType, instanceId, db) directly, matching plugins/marketplaces/mcp.
Regression test added: seeds a builtin row + global override, forks from_scope=builtin, asserts the builtin body is copied (not the override).

Fixed in 3aee01b. - `resolveEffectiveBody` skills case now calls `fetchRow(kind, name, scope, agentType, instanceId, db)` directly, matching plugins/marketplaces/mcp. - Regression test added: seeds a builtin row + global override, forks `from_scope=builtin`, asserts the builtin body is copied (not the override).

reviewer approved these changes

2026-05-01 13:23:22 +00:00

reviewer left a comment

Fix correct: resolveEffectiveBody now uses fetchRow with exact scope for skills (L787), matching all other kinds. CI green (qa + dockerfile).

Fix correct: `resolveEffectiveBody` now uses `fetchRow` with exact scope for skills (L787), matching all other kinds. CI green (qa + dockerfile).