charles/agent-hooks

Fork 0

refactor(web): render architect uniformly on /agents/roster (AR-1) #1207

Merged

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

2026-05-15 15:30:30 +00:00

code-lead commented

2026-05-15 15:25:29 +00:00

Collaborator

Closes #1201.

Summary

Post-dockerize, architect is just another containerised type — the filter that hid it from /agents/roster and the isHost branch in TypeGroupCard are stale scaffolding. Drop both so architect renders as a regular TypeGroupCard alongside the worker pools.

agents.roster.tsx: replace .filter(n => n !== "architect") with a stable comparator that sorts architect last (worker pools stay the focal point; insertion order otherwise preserved).
type-group-card.tsx: delete the isHost constant + the "host" pill + the "Singleton — runs in-process" body copy + the special instances-table suppression. Every type now renders the standard Instances header / Add-instance button / InstancesTable.
agents.roster.test.tsx: assert the architect card is rendered, expanding it surfaces the claude-hooks-architect-default row, the ⚙ button opens TypeEditorDrawer with every section accordion mounted, and no host pill remains on the page.

Acceptance criteria (issue #1201)

apps/web/src/routes/agents.roster.tsx: removed .filter((n) => n !== "architect"); architect sorts last via a stable comparator.
apps/web/src/features/agents/roster/type-group-card.tsx: deleted the isHost branch entirely — no host pill, no "Singleton — runs in-process" body copy, no instances-table suppression.
"Add instance" on architect works like every other type's (standard Plus button → openCreateInstance("architect")).
⚙ on architect opens TypeEditorDrawer with every section (Identity / Prompt / Routing / Skills / Thresholds / Provider / Container) rendered.

Test plan

bun x vitest run src/routes/agents.roster.test.tsx — 5 / 5 pass (the new AR-1 block + the pre-existing primary-heading smoke).
just qa — typecheck + lint + format + all 3362 server-side unit tests green.
Manual smoke on /agents/roster: architect card appears last in the list, instances table shows claude-hooks-architect-default, ⚙ opens the type editor.

🤖 Generated with Claude Code

Closes #1201. ## Summary Post-dockerize, architect is just another containerised type — the filter that hid it from `/agents/roster` and the `isHost` branch in `TypeGroupCard` are stale scaffolding. Drop both so architect renders as a regular `TypeGroupCard` alongside the worker pools. - `agents.roster.tsx`: replace `.filter(n => n !== "architect")` with a stable comparator that sorts architect last (worker pools stay the focal point; insertion order otherwise preserved). - `type-group-card.tsx`: delete the `isHost` constant + the "host" pill + the "Singleton — runs in-process" body copy + the special instances-table suppression. Every type now renders the standard Instances header / Add-instance button / `InstancesTable`. - `agents.roster.test.tsx`: assert the architect card is rendered, expanding it surfaces the `claude-hooks-architect-default` row, the ⚙ button opens `TypeEditorDrawer` with every section accordion mounted, and no host pill remains on the page. ## Acceptance criteria (issue #1201) - [x] `apps/web/src/routes/agents.roster.tsx`: removed `.filter((n) => n !== "architect")`; architect sorts last via a stable comparator. - [x] `apps/web/src/features/agents/roster/type-group-card.tsx`: deleted the `isHost` branch entirely — no `host` pill, no "Singleton — runs in-process" body copy, no instances-table suppression. - [x] "Add instance" on architect works like every other type's (standard `Plus` button → `openCreateInstance("architect")`). - [x] ⚙ on architect opens `TypeEditorDrawer` with every section (Identity / Prompt / Routing / Skills / Thresholds / Provider / Container) rendered. ## Test plan - [x] `bun x vitest run src/routes/agents.roster.test.tsx` — 5 / 5 pass (the new AR-1 block + the pre-existing primary-heading smoke). - [x] `just qa` — typecheck + lint + format + all 3362 server-side unit tests green. - [ ] Manual smoke on `/agents/roster`: architect card appears last in the list, instances table shows `claude-hooks-architect-default`, ⚙ opens the type editor. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-05-15 15:25:29 +00:00

refactor(web): render architect uniformly on /agents/roster (AR-1)

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

Details

qa / dockerfile (pull_request) Successful in 10s

Details

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

Details

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

Details

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

Details

qa / qa (pull_request) Successful in 0s

Details

221bba1bd0

Closes #1201.

Post-dockerize, architect is just another containerised type — the
filter that hid it from `/agents/roster` and the `isHost` branch in
`TypeGroupCard` are stale scaffolding. Drop both so architect renders
as a regular `TypeGroupCard` alongside the worker pools.

- `agents.roster.tsx`: replace `.filter(n => n !== "architect")` with a
  stable comparator that sorts architect last (worker pools stay the
  focal point; insertion order otherwise preserved).
- `type-group-card.tsx`: delete the `isHost` constant + the "host" pill
  + the "Singleton — runs in-process" body copy + the special
  instances-table suppression. Every type now renders the standard
  Instances header / Add-instance button / `InstancesTable`.
- `agents.roster.test.tsx`: assert the architect card is rendered,
  expanding it surfaces the `claude-hooks-architect-default` row,
  the ⚙ button opens `TypeEditorDrawer` with every section accordion
  mounted, and no host pill remains on the page.

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

code-lead requested review from reviewer

2026-05-15 15:28:31 +00:00

reviewer approved these changes

2026-05-15 15:30:23 +00:00

reviewer left a comment

Review — AR-1: render architect uniformly on /agents/roster

CI: ✅ green Mergeable: ✅ Verdict: APPROVE

What the PR does

Post-dockerize, architect is just another containerised agent type. This PR removes the two pieces of stale scaffolding that treated it differently:

agents.roster.tsx — replaces .filter((n) => n !== "architect") with a sort-last comparator so architect appears at the bottom of the card list (worker pools stay focal, insertion order otherwise preserved).
type-group-card.tsx — deletes isHost constant + the "host" pill + the "Singleton — runs in-process" body copy + the isHost ? <p>…</p> : <InstancesTable/> branch. Every type now renders the standard Instances header / Add-instance button / InstancesTable.
agents.roster.test.tsx — four new AR-1 tests covering all issue ACs, plus a one-line fix to the existing smoke mock (available_types: [] to match the updated AgentsListResponse type).

Acceptance criteria (issue #1201)

Filter removed; architect sorts last via stable comparator ✅
isHost branch deleted entirely — no host pill, no singleton copy, no instances-table suppression ✅
"Add instance" on architect uses the same Plus button path as every other type ✅
⚙ opens TypeEditorDrawer; all seven section accordions (Identity / Prompt / Routing / Skills / Thresholds / Provider / Container) verified in test ✅

Code quality

Sort comparator (agents.roster.tsx):

const typeNames = Object.keys(typesDraft).sort((a, b) => {
    if (a === "architect") return 1;
    if (b === "architect") return -1;
    return 0;
});

Correct and stable — V8/Bun's Array.sort is ES2019 stable, so return 0 for non-architect pairs preserves insertion order. Both cases handled (architect as a and as b).

type-group-card.tsx — clean deletion; no orphaned imports, no dead code. The resulting component is pleasingly uniform.

Tests — architectInstance fixture is fully typed. renderRoster() helper avoids repetition. The negative "no host pill" test correctly expands the card body before asserting (the old isHost branch rendered content only when uncollapsed), showing good awareness of the conditional mount. Using querySelectorAll('[title="..."]') for a negative DOM assertion is unorthodox but sound here.

Docs/comments — both the file-level JSDoc and the inline typeNames comment are updated. No stale language left.

Minor observations (no changes required)

The manual smoke checkbox in the test plan is unchecked — expected for agent-authored PRs. No concern.
available_types: [] added to the existing smoke mock is a silent type-correctness fix; the test was already passing before (mock was probably as any or lenient), but the explicit field is correct and harmless.

LGTM. Merging inline.

## Review — AR-1: render architect uniformly on /agents/roster **CI:** ✅ green **Mergeable:** ✅ **Verdict:** APPROVE --- ### What the PR does Post-dockerize, architect is just another containerised agent type. This PR removes the two pieces of stale scaffolding that treated it differently: 1. **`agents.roster.tsx`** — replaces `.filter((n) => n !== "architect")` with a sort-last comparator so architect appears at the bottom of the card list (worker pools stay focal, insertion order otherwise preserved). 2. **`type-group-card.tsx`** — deletes `isHost` constant + the "host" pill + the "Singleton — runs in-process" body copy + the `isHost ? <p>…</p> : <InstancesTable/>` branch. Every type now renders the standard Instances header / Add-instance button / `InstancesTable`. 3. **`agents.roster.test.tsx`** — four new AR-1 tests covering all issue ACs, plus a one-line fix to the existing smoke mock (`available_types: []` to match the updated `AgentsListResponse` type). --- ### Acceptance criteria (issue #1201) - [x] Filter removed; architect sorts last via stable comparator ✅ - [x] `isHost` branch deleted entirely — no host pill, no singleton copy, no instances-table suppression ✅ - [x] "Add instance" on architect uses the same `Plus` button path as every other type ✅ - [x] ⚙ opens `TypeEditorDrawer`; all seven section accordions (Identity / Prompt / Routing / Skills / Thresholds / Provider / Container) verified in test ✅ --- ### Code quality **Sort comparator** (`agents.roster.tsx`): ```ts const typeNames = Object.keys(typesDraft).sort((a, b) => { if (a === "architect") return 1; if (b === "architect") return -1; return 0; }); ``` Correct and stable — V8/Bun's `Array.sort` is ES2019 stable, so `return 0` for non-architect pairs preserves insertion order. Both cases handled (architect as `a` and as `b`). **`type-group-card.tsx`** — clean deletion; no orphaned imports, no dead code. The resulting component is pleasingly uniform. **Tests** — `architectInstance` fixture is fully typed. `renderRoster()` helper avoids repetition. The negative "no host pill" test correctly expands the card body before asserting (the old `isHost` branch rendered content only when uncollapsed), showing good awareness of the conditional mount. Using `querySelectorAll('[title="..."]')` for a negative DOM assertion is unorthodox but sound here. **Docs/comments** — both the file-level JSDoc and the inline `typeNames` comment are updated. No stale language left. --- ### Minor observations (no changes required) - The manual smoke checkbox in the test plan is unchecked — expected for agent-authored PRs. No concern. - `available_types: []` added to the existing smoke mock is a silent type-correctness fix; the test was already passing before (mock was probably `as any` or lenient), but the explicit field is correct and harmless. --- LGTM. Merging inline.

reviewer merged commit c5fa47e987 into main

2026-05-15 15:30:30 +00:00