From 6794510b759b75deaa35d2394426c751187ed8bc Mon Sep 17 00:00:00 2001 From: Ame Date: Tue, 16 Jun 2026 20:20:28 +0800 Subject: [PATCH] fix(workspaces): every create path enables all adapters, not just template head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Quick-chat (Ask Alice → POST /quick-chat) created its daily chat workspace with only claude+codex. Root cause: the "every registered adapter enabled" policy lived solely in the frontend create hook, which expanded the list before POSTing. Quick-chat is a backend-only caller — it calls WorkspaceCreator.create() directly with no agent set, so it fell through to the bare template.defaultAgents (["claude","codex"]), the same default the hook's comment already called "wrong". Move the policy into the domain layer: new resolveCreateAgents() in workspace-creator.ts is the single home — explicit subset wins verbatim, otherwise all registered adapters with template.defaultAgents as the order-head (so agents[0], the new-session default, still follows template intent). Form, quick-chat, and headless now converge on it. Thin the frontend: useCreateWorkspace sends no agent set; createWorkspace's agents arg is optional; the now-dead `agents` prop is dropped from CreateWorkspaceForm/Dialog and its three call sites (display-only `agents` in Sidebar/TemplateDetail is untouched). Demo: /quick-chat handler returned demoWorkspaces[0] (the aapl-q1 workspace, agents:['claude']) — wrong target and it reproduced the same visual bug. Point it at demoChatWorkspace, which already carries all four agents. Test plan: - npx tsc --noEmit (Alice) clean; cd ui && npx tsc -b clean - workspaces suite green (139 tests incl. 5 new resolveCreateAgents cases) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/workspaces/workspace-creator.spec.ts | 33 ++++++++++++++++- src/workspaces/workspace-creator.ts | 34 ++++++++++++++++-- .../workspace/ChatWorkspaceSection.tsx | 1 - .../workspace/CreateWorkspaceDialog.tsx | 4 +-- .../workspace/CreateWorkspaceForm.tsx | 8 ++--- ui/src/components/workspace/Sidebar.tsx | 1 - ui/src/components/workspace/api.ts | 9 +++-- ui/src/demo/handlers/workspaces.ts | 6 ++-- ui/src/hooks/useCreateWorkspace.ts | 36 +++++-------------- ui/src/pages/TemplateDetailPage.tsx | 1 - 10 files changed, 84 insertions(+), 49 deletions(-) diff --git a/src/workspaces/workspace-creator.spec.ts b/src/workspaces/workspace-creator.spec.ts index 5dc68830e..d61f71663 100644 --- a/src/workspaces/workspace-creator.spec.ts +++ b/src/workspaces/workspace-creator.spec.ts @@ -14,7 +14,7 @@ import { EventEmitter } from 'node:events'; import * as childProcess from 'node:child_process'; import { afterEach, describe, expect, it, vi } from 'vitest'; -import { runScript } from './workspace-creator.js'; +import { resolveCreateAgents, runScript } from './workspace-creator.js'; vi.mock('node:child_process', () => ({ spawn: vi.fn(), @@ -42,6 +42,37 @@ function setPlatform(value: NodeJS.Platform): void { Object.defineProperty(process, 'platform', { value, configurable: true }); } +describe('resolveCreateAgents — single home of the agent policy', () => { + const ALL = ['claude', 'codex', 'opencode', 'pi']; + + it('enables EVERY registered adapter when the caller pins nothing', () => { + // The quick-chat bug: it called create() with no explicit set, so it used + // to get only the template head (claude+codex). Policy now expands here. + expect(resolveCreateAgents(undefined, ['claude', 'codex'], ALL)).toEqual(ALL); + }); + + it('honors template defaultAgents only as the ORDER head (agents[0] default)', () => { + // A template that wants codex first still gets all four, codex leading. + expect(resolveCreateAgents(undefined, ['codex'], ALL)).toEqual([ + 'codex', 'claude', 'opencode', 'pi', + ]); + }); + + it('first-wins dedupes when the head repeats a registered id', () => { + expect(resolveCreateAgents(undefined, ['pi', 'claude'], ALL)).toEqual([ + 'pi', 'claude', 'codex', 'opencode', + ]); + }); + + it('an explicit non-empty request wins verbatim (subset pinning)', () => { + expect(resolveCreateAgents(['claude'], ['claude', 'codex'], ALL)).toEqual(['claude']); + }); + + it('treats an empty explicit request as "not pinned" → full expansion', () => { + expect(resolveCreateAgents([], ['claude', 'codex'], ALL)).toEqual(ALL); + }); +}); + describe('runScript platform branching', () => { const originalPlatform = process.platform; diff --git a/src/workspaces/workspace-creator.ts b/src/workspaces/workspace-creator.ts index 9dc43fada..e5a0f0f49 100644 --- a/src/workspaces/workspace-creator.ts +++ b/src/workspaces/workspace-creator.ts @@ -51,6 +51,30 @@ export type CreateResult = const TAG_RE = /^[a-z0-9][a-z0-9_-]{0,32}$/; +/** + * Resolve the adapter set a new workspace is created with. This is the single + * home of the agent policy, so every create path — the form, quick-chat, + * headless — converges on it: + * + * - An explicit `agentsRequested` (a caller pinning a subset) wins verbatim. + * - Otherwise a workspace gets EVERY registered adapter enabled; restricting + * it was a create-time decision with no first-action basis. The template's + * `defaultAgents` is honored only as the HEAD of the list (first-wins + * dedupe), so `agents[0]` — the "spawn a new session" default — follows + * template intent without limiting what's available. + * + * This used to live in the frontend create hook alone, which silently left + * backend-only callers (quick-chat) on the bare-`defaultAgents` set. + */ +export function resolveCreateAgents( + agentsRequested: readonly string[] | undefined, + templateDefaultAgents: readonly string[], + allAdapterIds: readonly string[], +): readonly string[] { + if (agentsRequested && agentsRequested.length > 0) return agentsRequested; + return [...new Set([...templateDefaultAgents, ...allAdapterIds])]; +} + /** * Creates a workspace by invoking the template's bootstrap script. * @@ -87,9 +111,13 @@ export class WorkspaceCreator { }; } - const agents = agentsRequested && agentsRequested.length > 0 - ? agentsRequested - : template.defaultAgents; + // Agent policy lives in `resolveCreateAgents` (this file) so every create + // path — form, quick-chat, headless — converges on it. + const agents = resolveCreateAgents( + agentsRequested, + template.defaultAgents, + this.opts.adapterRegistry.list().map((a) => a.id), + ); // Validate every requested adapter exists in the registry. for (const a of agents) { diff --git a/ui/src/components/workspace/ChatWorkspaceSection.tsx b/ui/src/components/workspace/ChatWorkspaceSection.tsx index 8b8409821..899d708ea 100644 --- a/ui/src/components/workspace/ChatWorkspaceSection.tsx +++ b/ui/src/components/workspace/ChatWorkspaceSection.tsx @@ -145,7 +145,6 @@ export function ChatWorkspaceSection(): ReactElement | null { {showCreate && ( { ctx.refresh() diff --git a/ui/src/components/workspace/CreateWorkspaceDialog.tsx b/ui/src/components/workspace/CreateWorkspaceDialog.tsx index 2152e04a4..0d4256d36 100644 --- a/ui/src/components/workspace/CreateWorkspaceDialog.tsx +++ b/ui/src/components/workspace/CreateWorkspaceDialog.tsx @@ -12,11 +12,10 @@ import { useTranslation } from 'react-i18next' import { Dialog } from '../uta/Dialog' import { CreateWorkspaceForm } from './CreateWorkspaceForm' -import type { AgentInfo, TemplateInfo, Workspace } from './api' +import type { TemplateInfo, Workspace } from './api' export interface CreateWorkspaceDialogProps { readonly templates: readonly TemplateInfo[] - readonly agents: readonly AgentInfo[] /** Pin the template (Chat section). Omit for the general sidebar create. */ readonly presetTemplate?: string /** Seed the tag input (e.g. the Chat section's date-based default). */ @@ -38,7 +37,6 @@ export function CreateWorkspaceDialog(props: CreateWorkspaceDialogProps): ReactE
{ props.onChanged(); props.onSelectWorkspace(workspace.id); diff --git a/ui/src/components/workspace/api.ts b/ui/src/components/workspace/api.ts index 4e03455d3..35c80c7de 100644 --- a/ui/src/components/workspace/api.ts +++ b/ui/src/components/workspace/api.ts @@ -78,15 +78,20 @@ export async function listWorkspaces(): Promise { return body.workspaces; } +/** + * Create a workspace. `agents` is optional and normally omitted — the backend + * owns the "every registered adapter, template-headed" policy (see + * `WorkspaceCreator.create`). Pass an explicit set only to pin a subset. + */ export async function createWorkspace( tag: string, template: string, - agents: readonly string[], + agents?: readonly string[], ): Promise { const res = await fetch('/api/workspaces', { method: 'POST', headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ tag, template, agents }), + body: JSON.stringify(agents && agents.length > 0 ? { tag, template, agents } : { tag, template }), }); if (res.ok) { const body = (await res.json()) as { workspace: Workspace }; diff --git a/ui/src/demo/handlers/workspaces.ts b/ui/src/demo/handlers/workspaces.ts index 61ee6a0eb..d3e218143 100644 --- a/ui/src/demo/handlers/workspaces.ts +++ b/ui/src/demo/handlers/workspaces.ts @@ -1,5 +1,5 @@ import { http, HttpResponse } from 'msw' -import { demoWorkspaces, demoTemplates } from '../fixtures/workspaces' +import { demoChatWorkspace, demoWorkspaces, demoTemplates } from '../fixtures/workspaces' import { demoWorkspaceFiles } from '../fixtures/inbox' export const workspacesHandlers = [ @@ -64,13 +64,13 @@ export const workspacesHandlers = [ // Quick-chat launch — reuse the first demo chat workspace and hand back the // scripted demo session (the Terminal short-circuits to DemoTerminalReplay). http.post('/api/workspaces/quick-chat', () => { - const ws = demoWorkspaces[0] + const ws = demoChatWorkspace return HttpResponse.json( { workspace: ws, session: { sessionId: 'demo-session', - wsId: ws?.id ?? 'demo-ws', + wsId: ws.id, name: 'c1', pid: 0, startedAt: Date.now(), diff --git a/ui/src/hooks/useCreateWorkspace.ts b/ui/src/hooks/useCreateWorkspace.ts index 880bdd848..2eb4e9504 100644 --- a/ui/src/hooks/useCreateWorkspace.ts +++ b/ui/src/hooks/useCreateWorkspace.ts @@ -1,5 +1,5 @@ import { useCallback, useState } from 'react' -import { createWorkspace, type AgentInfo, type Workspace } from '../components/workspace/api' +import { createWorkspace, type Workspace } from '../components/workspace/api' export const TAG_HINT = 'a-z, 0-9, "-", "_", up to 33 chars' export const TAG_RE = /^[a-z0-9][a-z0-9_-]{0,32}$/ @@ -25,15 +25,6 @@ export function defaultTagFor(template: string, workspaces: readonly Workspace[] interface UseCreateWorkspaceOpts { /** Workspace template to create from. Empty string = not yet selected. */ template: string - /** - * Template's declared defaultAgents. Used to determine agents[0], which - * is the default adapter when the user spawns a new session via "+". - * The full set of adapters is always enabled regardless — this only - * sets the head of the list. - */ - templateDefaultAgents?: readonly string[] - /** All adapters registered with the workspace launcher. All get enabled. */ - availableAgents: readonly AgentInfo[] /** Called with the new workspace after a successful create. */ onCreated: (workspace: Workspace) => void } @@ -54,14 +45,12 @@ interface UseCreateWorkspaceState { * agent-checkbox state + submit handler. They've drifted in small ways * over time; bundling here keeps them in lockstep. * - * Agent policy: every workspace gets every available adapter enabled. - * The CLI-checkbox row that previously asked users to pick was a - * decision with no first-action judgement basis; defaults were also - * wrong (only claude when a template didn't explicitly opt in to more). - * `templateDefaultAgents` is still honored as the head of the list so - * `agents[0]` — the "spawn a new session" default — follows template - * intent. Template authors can still steer the new-session default - * without restricting what's available. + * Agent policy lives in the backend (`WorkspaceCreator.create`): every + * workspace gets every registered adapter enabled, template-headed so + * `agents[0]` (the new-session default) follows template intent. This hook + * sends NO agent set, so the form, quick-chat, and headless all converge on + * that one source of truth — it used to expand the list here, which silently + * left backend-only callers (quick-chat) on the bare-defaultAgents set. */ export function useCreateWorkspace(opts: UseCreateWorkspaceOpts): UseCreateWorkspaceState { const [tag, setTag] = useState('') @@ -78,18 +67,9 @@ export function useCreateWorkspace(opts: UseCreateWorkspaceOpts): UseCreateWorks setError('no template selected') return } - const head = opts.templateDefaultAgents ?? [] - const seen = new Set(head) - const agents: string[] = [...head] - for (const a of opts.availableAgents) { - if (!seen.has(a.id)) { - agents.push(a.id) - seen.add(a.id) - } - } setCreating(true) setError(null) - const result = await createWorkspace(t, opts.template, agents) + const result = await createWorkspace(t, opts.template) setCreating(false) if (result.ok) { setTag('') diff --git a/ui/src/pages/TemplateDetailPage.tsx b/ui/src/pages/TemplateDetailPage.tsx index 5399f87e9..8dc19e3e9 100644 --- a/ui/src/pages/TemplateDetailPage.tsx +++ b/ui/src/pages/TemplateDetailPage.tsx @@ -163,7 +163,6 @@ export function TemplateDetailPage({ spec }: Props) { {showCreate && ( setShowCreate(false)} onCreated={(workspace) => {