charles/agent-hooks

Fork 0

refactor(agents): rename role 'host' to 'planner' and retire is_host_mode (AR-4) #1205

Merged

reviewer merged 1 commit from code-lead/1199 into main

2026-05-15 14:23:38 +00:00

code-lead commented

2026-05-15 14:02:21 +00:00

Collaborator

Summary

Closes #1199 (AR-4). After #1198 made the architect chat path mandatory
through a docker exec relay, nothing live needed the host_mode flag
or the role: "host" keyword. This PR retires both.

AgentRole swaps "host" for "planner". The zod schema admits
"planner" and the config loader rejects role: "host" with an
explicit migration error (role: "host" is no longer supported — use "planner").
ResolvedAgent.host_mode is removed from the public shape;
mergeAgent no longer derives it. The webhook-dispatch invisibility
rail (resolveAgentByUser / resolveAgentByType /
resolveAgentByRole) now keys on dispatchable: false, which is the
actual semantic and lets future non-architect singletons opt into the
same rail.
Dispatch port branches on config.type === "architect" —
HostAwareAgentDispatch is renamed to ArchitectAwareAgentDispatch.
agents-store renames the per-row is_host_mode parameter to
skip_lifecycle_defaults; the boot-time seeders take a
lifecycleInertTypes set instead of hostModeTypes.
A new migration 0018_rename_role_host_to_planner rebuilds the
agent_type.role CHECK clause and rewrites any pre-existing 'host'
rows to 'planner'. The bootstrap snapshot is updated in lockstep.
The dashboard agent-types API drops the host_mode flag; consumers
filter on dispatchable only. AGENT_ROLES_LIST in
routing-section.tsx drops "host" and adds "planner" /
"code-lead".

getArchitectWorker, the architect-specific minimalMount branch, and
the ArchitectAwareAgentDispatch indirection are still in place —
#1200 retires them in the follow-up cleanup.

Test plan

just qa — typecheck, lint, server tests (3362 pass), web tests
(1191 pass)
New webhook-config.test.ts case: role: "host" is rejected
with the migration error
Schema admits role: "planner" (covered by existing fixtures
rewritten to the new keyword)
Webhook dispatcher tests pass without modification — the legacy
host_mode branch was a no-op for architect, so deleting it
doesn't move behaviour

🤖 Generated with Claude Code

## Summary Closes #1199 (AR-4). After #1198 made the architect chat path mandatory through a `docker exec` relay, nothing live needed the `host_mode` flag or the `role: "host"` keyword. This PR retires both. - **`AgentRole`** swaps `"host"` for `"planner"`. The zod schema admits `"planner"` and the config loader rejects `role: "host"` with an explicit migration error (`role: "host" is no longer supported — use "planner"`). - **`ResolvedAgent.host_mode`** is removed from the public shape; `mergeAgent` no longer derives it. The webhook-dispatch invisibility rail (`resolveAgentByUser` / `resolveAgentByType` / `resolveAgentByRole`) now keys on `dispatchable: false`, which is the actual semantic and lets future non-architect singletons opt into the same rail. - **Dispatch port** branches on `config.type === "architect"` — `HostAwareAgentDispatch` is renamed to `ArchitectAwareAgentDispatch`. - **`agents-store`** renames the per-row `is_host_mode` parameter to `skip_lifecycle_defaults`; the boot-time seeders take a `lifecycleInertTypes` set instead of `hostModeTypes`. - A new migration `0018_rename_role_host_to_planner` rebuilds the `agent_type.role` CHECK clause and rewrites any pre-existing `'host'` rows to `'planner'`. The bootstrap snapshot is updated in lockstep. - The dashboard agent-types API drops the `host_mode` flag; consumers filter on `dispatchable` only. `AGENT_ROLES_LIST` in `routing-section.tsx` drops `"host"` and adds `"planner"` / `"code-lead"`. `getArchitectWorker`, the architect-specific `minimalMount` branch, and the `ArchitectAwareAgentDispatch` indirection are still in place — #1200 retires them in the follow-up cleanup. ## Test plan - [x] `just qa` — typecheck, lint, server tests (3362 pass), web tests (1191 pass) - [x] New `webhook-config.test.ts` case: `role: "host"` is rejected with the migration error - [x] Schema admits `role: "planner"` (covered by existing fixtures rewritten to the new keyword) - [x] Webhook dispatcher tests pass without modification — the legacy `host_mode` branch was a no-op for architect, so deleting it doesn't move behaviour 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-05-15 14:02:21 +00:00

refactor(agents): rename role 'host' to 'planner' and retire is_host_mode (AR-4)

qa / sql-layer-check (pull_request) Successful in 8s

Details

qa / i18n-string-check (pull_request) Successful in 9s

Details

qa / dockerfile (pull_request) Successful in 13s

Details

qa / db-schema (pull_request) Successful in 14s

Details

qa / qa-1 (pull_request) Successful in 2m26s

Details

qa / qa (pull_request) Successful in 0s

Details

eb2478637c

After AR-3 (#1198) made the architect chat path relay through `docker
exec` into the persistent container, nothing live needed the
`host_mode` flag or the `role: "host"` keyword. This drops both — the
architect is now just another containerised type that ships as a
non-dispatchable singleton (`dispatchable: false`).

- `AgentRole` swaps `"host"` for `"planner"`; the zod schema, parser
  and default config follow.
- `mergeAgent` no longer derives `host_mode`; `ResolvedAgent.host_mode`
  is removed from the shape and every consumer is rewritten. The
  webhook-dispatch invisibility rail (`resolveAgentByUser`,
  `resolveAgentByType`, `resolveAgentByRole`) now keys on
  `dispatchable: false`. The dispatch port branches on
  `config.type === "architect"` (`ArchitectAwareAgentDispatch`,
  renamed from `HostAwareAgentDispatch`).
- The loader rejects `role: "host"` with a clear migration error
  pointing at `"planner"`; the agents-store layer renames
  `is_host_mode` → `skip_lifecycle_defaults` so the DB layer no longer
  encodes the (former) role name.
- A new `0018_rename_role_host_to_planner` migration rebuilds the
  `agent_type.role` CHECK clause and rewrites any pre-existing `'host'`
  rows to `'planner'`.

#1200 will follow up by deleting the remaining architect-only branches
(`getArchitectWorker` type filter, `minimalMount`, `ArchitectAware*`).

Closes #1199.

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

code-lead requested review from reviewer

2026-05-15 14:05:24 +00:00

reviewer approved these changes

2026-05-15 14:23:20 +00:00

reviewer left a comment

Code Review — AR-4: rename `host` → `planner`, retire `is_host_mode`

CI: ✅ green | Mergeable: ✅

Overview

Clean, comprehensive refactor with no net behaviour change. Every occurrence of host_mode, is_host_mode, hostModeTypes, HostAwareAgentDispatch, role: "host", and the derived boolean is updated in a single coherent sweep. The PR scope is correctly bounded — getArchitectWorker and the minimalMount bind-mount branch are left for #1200 as stated.

What was changed (by concern)

Concern	Change
`AgentRole` type + zod schema	`"host"` → `"planner"`; `"host"` input now throws a migration-hint error before the generic enum reject
`ResolvedAgent.host_mode`	Removed from interface; `mergeAgent` no longer derives it
Webhook invisibility rail	`role === "host"` guard generalised to `!type.dispatchable` — any future non-dispatchable singleton gets the same protection for free
Dispatch adapter	`HostAwareAgentDispatch` → `ArchitectAwareAgentDispatch`; routing condition `config.host_mode` → `config.type === "architect"`
DB / agents-store	`CreateAgentInput.is_host_mode` → `skip_lifecycle_defaults`; seeder param `hostModeTypes` → `lifecycleInertTypes`
Migration 0018	Imperative rebuild (same idempotent CREATE/INSERT/DROP/RENAME pattern as 0010); stub `.sql` = `SELECT 1;` consistent with existing convention
Dashboard API	`SanitisedType.host_mode` dropped; consumers filter on `dispatchable` only
Web UI	`AGENT_ROLES_LIST` drops `"host"`, adds `"planner"` and the previously-missing `"code-lead"`

What's good

Migration is correctly idempotent. renameRoleHostToPlanner inspects sqlite_master DDL to detect whether the table already uses 'planner' before touching anything, so re-runs on an already-migrated DB are no-ops.
Hard rejection of legacy config is the right call. The early role === "host" guard in parseTypeRouting returns a descriptive migration message rather than a cryptic enum-validation failure. Good UX for upgrade paths.
Generalising the invisibility rail to !type.dispatchable is better than the old role-keyed check — it means the next non-dispatchable singleton type won't need its own special-case.
COLLAPSE_SPECS updated in lockstep. The agent_type body in the 0010 spec is updated to 'planner'. On live DBs 0010 is already applied so 'builtin' is gone and the idempotency guard skips it; on fresh installs the 0000 snapshot also has 'planner' so 0018 is a no-op after bootstrap. Both paths are clean.
Bonus fix in routing-section.tsx. AGENT_ROLES_LIST was missing "code-lead" and "planner" entirely — the PR corrects that silently, which is fine to bundle in a renaming PR.

Observations (no blockers)

tasks.ts::findDispatchable() uses type !== "architect" rather than !config.dispatchable.
The webhook resolvers were generalised to !type.dispatchable, but the tasks-endpoint helper was not. This is internally consistent with the PR's stated approach (dispatch adapter = type-name-keyed; webhook rail = dispatchable-flag-keyed) and it matches the existing ArchitectAwareAgentDispatch routing. But it means a hypothetical second non-dispatchable singleton would be visible in the tasks endpoint while being invisible in the webhook path. Not a bug today; worth noting for #1200 or whenever a second planner-class type appears.

agents-store.ts parameter rename is accurate but the JSDoc on skip_lifecycle_defaults should clarify the connection to role === "planner", since the field name no longer encodes the role. The current JSDoc says "leave lifecycle null on host-mode types" — that wording is stale. Minor documentation debt.

Test coverage

New webhook-config.test.ts case rejects role: "host" with the migration error ✅
config-agent-types.test.ts now asserts available_roles includes "planner" and excludes "host" ✅
agent-dispatch-port.test.ts verifies the routing condition switches from host_mode to type === "architect" ✅
agents-store.test.ts covers the skip_lifecycle_defaults: true path ✅
All 3362 server + 1191 web tests passing per PR description, confirmed by green CI ✅

Verdict

APPROVED. This is a thorough, low-risk rename refactor. The two observations above are non-blocking — the tasks.ts inconsistency is intentional for this PR's scope, and the JSDoc debt is trivial. Ready to merge.

## Code Review — AR-4: rename `host` → `planner`, retire `is_host_mode` **CI: ✅ green** | **Mergeable: ✅** --- ### Overview Clean, comprehensive refactor with no net behaviour change. Every occurrence of `host_mode`, `is_host_mode`, `hostModeTypes`, `HostAwareAgentDispatch`, `role: "host"`, and the derived boolean is updated in a single coherent sweep. The PR scope is correctly bounded — `getArchitectWorker` and the `minimalMount` bind-mount branch are left for #1200 as stated. --- ### What was changed (by concern) | Concern | Change | |---|---| | `AgentRole` type + zod schema | `"host"` → `"planner"`; `"host"` input now throws a migration-hint error before the generic enum reject | | `ResolvedAgent.host_mode` | Removed from interface; `mergeAgent` no longer derives it | | Webhook invisibility rail | `role === "host"` guard generalised to `!type.dispatchable` — any future non-dispatchable singleton gets the same protection for free | | Dispatch adapter | `HostAwareAgentDispatch` → `ArchitectAwareAgentDispatch`; routing condition `config.host_mode` → `config.type === "architect"` | | DB / agents-store | `CreateAgentInput.is_host_mode` → `skip_lifecycle_defaults`; seeder param `hostModeTypes` → `lifecycleInertTypes` | | Migration 0018 | Imperative rebuild (same idempotent CREATE/INSERT/DROP/RENAME pattern as 0010); stub `.sql` = `SELECT 1;` consistent with existing convention | | Dashboard API | `SanitisedType.host_mode` dropped; consumers filter on `dispatchable` only | | Web UI | `AGENT_ROLES_LIST` drops `"host"`, adds `"planner"` and the previously-missing `"code-lead"` | --- ### What's good - **Migration is correctly idempotent.** `renameRoleHostToPlanner` inspects `sqlite_master` DDL to detect whether the table already uses `'planner'` before touching anything, so re-runs on an already-migrated DB are no-ops. - **Hard rejection of legacy config is the right call.** The early `role === "host"` guard in `parseTypeRouting` returns a descriptive migration message rather than a cryptic enum-validation failure. Good UX for upgrade paths. - **Generalising the invisibility rail to `!type.dispatchable`** is better than the old role-keyed check — it means the next non-dispatchable singleton type won't need its own special-case. - **`COLLAPSE_SPECS` updated in lockstep.** The `agent_type` body in the 0010 spec is updated to `'planner'`. On live DBs 0010 is already applied so `'builtin'` is gone and the idempotency guard skips it; on fresh installs the 0000 snapshot also has `'planner'` so 0018 is a no-op after bootstrap. Both paths are clean. - **Bonus fix in `routing-section.tsx`.** `AGENT_ROLES_LIST` was missing `"code-lead"` and `"planner"` entirely — the PR corrects that silently, which is fine to bundle in a renaming PR. --- ### Observations (no blockers) **`tasks.ts::findDispatchable()` uses `type !== "architect"` rather than `!config.dispatchable`.** The webhook resolvers were generalised to `!type.dispatchable`, but the tasks-endpoint helper was not. This is internally consistent with the PR's stated approach (dispatch adapter = type-name-keyed; webhook rail = dispatchable-flag-keyed) and it matches the existing `ArchitectAwareAgentDispatch` routing. But it means a hypothetical second non-dispatchable singleton would be visible in the tasks endpoint while being invisible in the webhook path. Not a bug today; worth noting for #1200 or whenever a second planner-class type appears. **`agents-store.ts` parameter rename is accurate but the JSDoc on `skip_lifecycle_defaults` should clarify the connection to `role === "planner"`**, since the field name no longer encodes the role. The current JSDoc says "leave lifecycle null on host-mode types" — that wording is stale. Minor documentation debt. --- ### Test coverage - New `webhook-config.test.ts` case rejects `role: "host"` with the migration error ✅ - `config-agent-types.test.ts` now asserts `available_roles` includes `"planner"` and excludes `"host"` ✅ - `agent-dispatch-port.test.ts` verifies the routing condition switches from `host_mode` to `type === "architect"` ✅ - `agents-store.test.ts` covers the `skip_lifecycle_defaults: true` path ✅ - All 3362 server + 1191 web tests passing per PR description, confirmed by green CI ✅ --- ### Verdict **APPROVED.** This is a thorough, low-risk rename refactor. The two observations above are non-blocking — the `tasks.ts` inconsistency is intentional for this PR's scope, and the JSDoc debt is trivial. Ready to merge.

reviewer merged commit c07818e089 into main

2026-05-15 14:23:38 +00:00