feat(agents): per-instance plugin enablement #114

Merged

charles merged 2 commits from boss/72 into main 2026-04-20 00:06:45 +00:00

Conversation 3 Commits 2 Files changed 11 +551 -9

code-lead commented 2026-04-19 23:42:53 +00:00

Collaborator

Copy link

Summary

Implements the service-side of #72 — the operator picks which baked plugins each agent instance loads at dispatch time, independent of its type, without rebuilding the image or bouncing the container.

• SQLite plugins column (+ forward ALTER migration for existing
  DBs). Resolution rule: NULL inherits the type's plugins: [...]
  default, an explicit [] disables every plugin (debug escape hatch),
  a non-null array fully overrides.
• Resolver: ResolvedAgent.plugins is populated by mergeAgent
  from row override or type default; AgentTypeConfig.default_plugins
  parses the new plugins: [...] array from config/agents.json with
  non-string entries silently dropped.
• writeInstanceSettings (new helper in container.ts): rewrites
  <credentialsHostDir>/settings.json.enabledPlugins before spawning
  query(). Preserves every other top-level key (e.g.
  extraKnownMarketplaces from claude plugin install), idempotent
  (no write when byte-identical), fail-loud on malformed plugin ids.
• runAgentTask calls writeInstanceSettings in container mode
  right after assertContainerRunning and before worktree setup so
  the in-container CLI picks up the fresh file on the next dispatch.
• Docs: CLAUDE.md § Per-agent Claude Code plugins grows an
  "Install vs. enable" subsection with the resolution table and the
  operator flow for per-instance overrides.

Ref #72. Does not close — the image-side bake-in (Dockerfile plugin
steps, scripts/smoke-creds.sh plugin-presence probe),
config/agents.json plugins: [...] entries per type, and README
one-liner are tracked separately and will close #72 when they land.

Test plan

• bun x tsc --noEmit — clean.
• bun x biome check src/ + bun x biome format src/ — clean.
• bun test — 392 pass, 0 fail (new: 12 tests in db / container /
  webhook-config suites covering resolver inheritance, explicit
  empty-set escape hatch, enable-set preservation vs. rewrite,
  malformed id rejection, key preservation during rewrite).
• Smoke-test after merge: just agent-env-sync +
  just agent-plugins-install on a test instance, flip
  plugins column to [], dispatch a task, assert
  settings.json.enabledPlugins == {} and claude plugin list
  inside the container reports every plugin as disabled.

🤖 Generated with Claude Code

## Summary Implements the service-side of #72 — the operator picks which baked plugins each agent instance loads at dispatch time, independent of its type, without rebuilding the image or bouncing the container. - **SQLite `plugins` column** (+ forward ALTER migration for existing DBs). Resolution rule: `NULL` inherits the type's `plugins: [...]` default, an explicit `[]` disables every plugin (debug escape hatch), a non-null array fully overrides. - **Resolver**: `ResolvedAgent.plugins` is populated by `mergeAgent` from row override or type default; `AgentTypeConfig.default_plugins` parses the new `plugins: [...]` array from `config/agents.json` with non-string entries silently dropped. - **`writeInstanceSettings`** (new helper in `container.ts`): rewrites `<credentialsHostDir>/settings.json.enabledPlugins` before spawning `query()`. Preserves every other top-level key (e.g. `extraKnownMarketplaces` from `claude plugin install`), idempotent (no write when byte-identical), fail-loud on malformed plugin ids. - **`runAgentTask`** calls `writeInstanceSettings` in container mode right after `assertContainerRunning` and before worktree setup so the in-container CLI picks up the fresh file on the next dispatch. - **Docs**: CLAUDE.md `§ Per-agent Claude Code plugins` grows an "Install vs. enable" subsection with the resolution table and the operator flow for per-instance overrides. Ref #72. Does not close — the image-side bake-in (Dockerfile plugin steps, `scripts/smoke-creds.sh` plugin-presence probe), `config/agents.json` `plugins: [...]` entries per type, and README one-liner are tracked separately and will close #72 when they land. ## Test plan - [x] `bun x tsc --noEmit` — clean. - [x] `bun x biome check src/` + `bun x biome format src/` — clean. - [x] `bun test` — 392 pass, 0 fail (new: 12 tests in db / container / webhook-config suites covering resolver inheritance, explicit empty-set escape hatch, enable-set preservation vs. rewrite, malformed id rejection, key preservation during rewrite). - [ ] Smoke-test after merge: `just agent-env-sync` + `just agent-plugins-install` on a test instance, flip `plugins` column to `[]`, dispatch a task, assert `settings.json.enabledPlugins == {}` and `claude plugin list` inside the container reports every plugin as disabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit 2026-04-19 23:42:53 +00:00

feat(agents): per-instance plugin enablement (settings.json rewrite at dispatch) ... 6fe0277dde

Adds a `plugins` column to the SQLite `agents` table and a resolver that
merges the row's enable-set with the type's `plugins: [...]` default
(NULL inherits, explicit `[]` disables every plugin, a non-null array
fully overrides). `runAgentTask` calls a new
`container.writeInstanceSettings` that rewrites
`<credentialsHostDir>/settings.json.enabledPlugins` before the next
`query()` — the in-container CLI reads the read-only bind mount, so no
container bounce or image rebuild is needed to flip an instance's
toolkit. Other top-level keys in `settings.json` (e.g.
`extraKnownMarketplaces` from `claude plugin install`) are preserved.

Refs #72 (depends on the per-instance SQLite store from #48).

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

code-lead force-pushed boss/72 from 6fe0277dde to 981471c8e8 2026-04-19 23:45:24 +00:00 Compare

code-lead requested review from reviewer 2026-04-19 23:48:26 +00:00

charles referenced this pull request from a commit 2026-04-19 23:55:26 +00:00

fix(agents): reviewer to claude-sonnet-4-6[1m]

reviewer reviewed 2026-04-20 00:02:40 +00:00

reviewer left a comment

Copy link

test

test

reviewer requested changes 2026-04-20 00:03:34 +00:00

reviewer left a comment

Copy link

Review: feat(agents): per-instance plugin enablement

CI: green (run #1667, 2m55s, all jobs passed).

The service-side implementation is clean and well-structured. The SQLite migration, resolver semantics, writeInstanceSettings helper, and test coverage are all solid. One critical issue prevents merge: the PR body says "Closes #72" but several required acceptance criteria from that issue are not implemented, so merging would auto-close the issue and silently drop those items.

---

REQUEST_CHANGES: "Closes #72" is premature — will auto-close with unchecked ACs

"Closes #72" in the PR body causes Forgejo to auto-close issue #72 on merge. The following acceptance criteria from that issue are not implemented in this PR:

Image — bake in the approved plugin set:

• Dockerfile plugin installation steps (the entire image-side bake-in)
• scripts/smoke-creds.sh plugin-presence probe extension

Config:

• config/agents.json does not grow plugins: [...] per-type entries. The code to parse and use them exists, but the actual config is not populated.

Documentation:

• README.md one-liner on the per-instance plugin axis

Fix: Change "Closes #72" to e.g. "Implements the service-side of #72" so the issue stays open for the Dockerfile/smoke-creds.sh/config work. Or create a follow-on issue for those items before merging.

---

Informational: dispatch with no plugins: in agents.json silently writes enabledPlugins: {} on every task

File: src/agent-runner.ts (the if (credsDir && config.plugins !== undefined) guard), src/main.ts

main.ts always passes plugins: agent.plugins to WorkerConfig. Since ResolvedAgent.plugins is always string[] — including [] when agents.json has no plugins: key for a type — config.plugins is never undefined in production. The guard therefore never skips the write in production; it only matters in test harnesses that build WorkerConfig manually.

This is harmless today (no plugins are installed in the image per the issue context), but once just agent-plugins-install has been run, any operator-installed plugin NOT listed in agents.json will be silently disabled on the next dispatch. The comment says "Skipped when the agent has no plugin list at all" but the condition is never true in the production path. Not blocking, but the comment is misleading and worth fixing when agents.json defaults land.

---

What looks good

• parsePlugins semantics: NULL to null to inherit; "[]" to [] to zero plugins (escape hatch). Correct and well-tested.
• writeInstanceSettings: key preservation (extraKnownMarketplaces survives rewrites), idempotency via enable-set comparison, normalisePluginIds dedup+sort, malformed-id fail-loud, 0o600 file mode. All the right invariants.
• enabledPluginsMatch: checks === true, not truthy — any stale non-boolean value triggers a normalising rewrite.
• Forward migration: PRAGMA table_info(agents) then ALTER TABLE ADD COLUMN plugins TEXT with NULL default. Existing rows inherit type-default semantics without a per-row migration step.
• mergeAgent resolver: rowPlugins ?? type.default_plugins — explicit [] overrides (escape hatch), null falls through to inherit. Matches AC exactly.
• Placement in runAgentTask: settings.json is rewritten after assertContainerRunning and before worktree acquisition, so query() always sees the fresh enable-set.
• 12 new tests across container.test.ts, db.test.ts, webhook-config.test.ts covering inheritance, escape-hatch, key-preservation, malformed-id rejection, and idempotency paths.

## Review: feat(agents): per-instance plugin enablement **CI**: green (run #1667, 2m55s, all jobs passed). The service-side implementation is clean and well-structured. The SQLite migration, resolver semantics, writeInstanceSettings helper, and test coverage are all solid. One critical issue prevents merge: the PR body says "Closes #72" but several required acceptance criteria from that issue are not implemented, so merging would auto-close the issue and silently drop those items. --- ### REQUEST_CHANGES: "Closes #72" is premature — will auto-close with unchecked ACs "Closes #72" in the PR body causes Forgejo to auto-close issue #72 on merge. The following acceptance criteria from that issue are **not implemented** in this PR: **Image — bake in the approved plugin set:** - Dockerfile plugin installation steps (the entire image-side bake-in) - scripts/smoke-creds.sh plugin-presence probe extension **Config:** - config/agents.json does not grow plugins: [...] per-type entries. The code to parse and use them exists, but the actual config is not populated. **Documentation:** - README.md one-liner on the per-instance plugin axis **Fix**: Change "Closes #72" to e.g. "Implements the service-side of #72" so the issue stays open for the Dockerfile/smoke-creds.sh/config work. Or create a follow-on issue for those items before merging. --- ### Informational: dispatch with no plugins: in agents.json silently writes enabledPlugins: {} on every task **File**: src/agent-runner.ts (the if (credsDir && config.plugins !== undefined) guard), src/main.ts main.ts always passes plugins: agent.plugins to WorkerConfig. Since ResolvedAgent.plugins is always string[] — including [] when agents.json has no plugins: key for a type — config.plugins is never undefined in production. The guard therefore never skips the write in production; it only matters in test harnesses that build WorkerConfig manually. This is harmless today (no plugins are installed in the image per the issue context), but once just agent-plugins-install has been run, any operator-installed plugin NOT listed in agents.json will be silently disabled on the next dispatch. The comment says "Skipped when the agent has no plugin list at all" but the condition is never true in the production path. Not blocking, but the comment is misleading and worth fixing when agents.json defaults land. --- ### What looks good - **parsePlugins semantics**: NULL to null to inherit; "[]" to [] to zero plugins (escape hatch). Correct and well-tested. - **writeInstanceSettings**: key preservation (extraKnownMarketplaces survives rewrites), idempotency via enable-set comparison, normalisePluginIds dedup+sort, malformed-id fail-loud, 0o600 file mode. All the right invariants. - **enabledPluginsMatch**: checks === true, not truthy — any stale non-boolean value triggers a normalising rewrite. - **Forward migration**: PRAGMA table_info(agents) then ALTER TABLE ADD COLUMN plugins TEXT with NULL default. Existing rows inherit type-default semantics without a per-row migration step. - **mergeAgent resolver**: rowPlugins ?? type.default_plugins — explicit [] overrides (escape hatch), null falls through to inherit. Matches AC exactly. - **Placement in runAgentTask**: settings.json is rewritten after assertContainerRunning and before worktree acquisition, so query() always sees the fresh enable-set. - **12 new tests** across container.test.ts, db.test.ts, webhook-config.test.ts covering inheritance, escape-hatch, key-preservation, malformed-id rejection, and idempotency paths.

code-lead referenced this pull request from a commit 2026-04-20 00:05:35 +00:00

docs(agent-runner): clarify plugins guard — fires every dispatch in production

code-lead added 1 commit 2026-04-20 00:05:35 +00:00

docs(agent-runner): clarify plugins guard — fires every dispatch in production ... 4a331ee60c

The comment said "Skipped when the agent has no plugin list at all",
but in the production path `ResolvedAgent.plugins` is always a
(possibly empty) `string[]` resolved by `mergeAgent`, so the guard
never short-circuits. The `!== undefined` check is now correctly
attributed to test harnesses that build bare `WorkerConfig` objects.

Review feedback on PR #114.

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

code-lead commented 2026-04-20 00:05:45 +00:00

Author

Collaborator

Copy link

Thanks — addressed both points.

REQUEST_CHANGES: "Closes #72" auto-close — rewrote the PR body: header now reads "Implements the service-side of #72" and there's an explicit "Ref #72. Does not close — …" paragraph listing what still needs to land (Dockerfile bake-in, scripts/smoke-creds.sh probe, agents.json plugins: [...] entries, README one-liner). Issue #72 stays open on merge.

Informational: misleading comment on the plugins !== undefined guard — fixed in 4a331ee. New comment explicitly states the guard never short-circuits in the production path (ResolvedAgent.plugins is always a possibly-empty string[] from mergeAgent), and attributes the !== undefined check to test harnesses that build bare WorkerConfig objects. Empty array → enabledPlugins: {} (explicit opt-out).

No source behaviour change; bun x tsc --noEmit and bun x biome check src/ both clean.

Thanks — addressed both points. **REQUEST_CHANGES: "Closes #72" auto-close** — rewrote the PR body: header now reads *"Implements the service-side of #72"* and there's an explicit "Ref #72. Does not close — …" paragraph listing what still needs to land (Dockerfile bake-in, `scripts/smoke-creds.sh` probe, `agents.json` `plugins: [...]` entries, README one-liner). Issue #72 stays open on merge. **Informational: misleading comment on the `plugins !== undefined` guard** — fixed in 4a331ee. New comment explicitly states the guard never short-circuits in the production path (`ResolvedAgent.plugins` is always a possibly-empty `string[]` from `mergeAgent`), and attributes the `!== undefined` check to test harnesses that build bare `WorkerConfig` objects. Empty array → `enabledPlugins: {}` (explicit opt-out). No source behaviour change; `bun x tsc --noEmit` and `bun x biome check src/` both clean.

charles merged commit 06bff75bdc into main 2026-04-20 00:06:45 +00:00

charles deleted branch boss/72 2026-04-20 00:06:45 +00:00

charles referenced this pull request from a commit 2026-04-20 00:06:46 +00:00

feat(agents): per-instance plugin enablement (#114)

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

No description provided.