diff --git a/apps/agor-daemon/src/services/sessions.agentic-tool-guard.test.ts b/apps/agor-daemon/src/services/sessions.agentic-tool-guard.test.ts new file mode 100644 index 000000000..5a9adb3fd --- /dev/null +++ b/apps/agor-daemon/src/services/sessions.agentic-tool-guard.test.ts @@ -0,0 +1,154 @@ +/** + * `agentic_tool` immutability was previously only a client-side convention — + * `SessionPanel.tsx` hides "Switch tool" once a session has tasks, but + * nothing on the server stopped a direct `sessions.patch` call (a stale tab, + * the MCP session-update tool, CLI, a future UI surface) from changing + * `agentic_tool` on a session with existing tasks/messages, desyncing it + * from the tool that actually produced them. + * + * These tests pin the server-side guard down against a real database. + */ +import { + BranchRepository, + generateId, + RepoRepository, + SessionRepository, + TaskRepository, +} from '@agor/core/db'; +import type { Application } from '@agor/core/feathers'; +import type { Session, Task, UUID } from '@agor/core/types'; +import { SessionStatus, TaskStatus } from '@agor/core/types'; +import { describe, expect } from 'vitest'; +import { dbTest } from '../../../../packages/core/src/db/test-helpers'; +import { SessionsService } from './sessions'; + +// The guard only touches the session/task repos built from `db`; the stored +// `app` is never read on this path. A bare cast keeps the harness minimal. +const STUB_APP = {} as unknown as Application; + +async function createBranch(db: any): Promise { + const repoRepo = new RepoRepository(db); + const branchRepo = new BranchRepository(db); + const repo = await repoRepo.create({ + repo_id: generateId(), + slug: `repo-${generateId()}`, + name: 'Test Repo', + repo_type: 'remote' as const, + remote_url: 'https://github.com/test/repo.git', + local_path: '/tmp/test-repo', + default_branch: 'main', + }); + const branch = await branchRepo.create({ + branch_id: generateId(), + repo_id: repo.repo_id, + name: 'feature', + ref: 'feature', + branch_unique_id: Math.floor(Math.random() * 1_000_000), + path: '/tmp/test-repo', + base_ref: 'main', + new_branch: false, + created_by: 'test-user' as UUID, + }); + return branch.branch_id as UUID; +} + +async function createSession( + db: any, + branchId: UUID, + overrides: Partial = {} +): Promise { + const sessionRepo = new SessionRepository(db); + const session = await sessionRepo.create({ + session_id: generateId(), + branch_id: branchId, + agentic_tool: 'claude-code', + status: SessionStatus.IDLE, + created_by: 'test-user' as UUID, + git_state: { ref: 'main', base_sha: 'abc', current_sha: 'def' }, + tasks: [], + contextFiles: [], + genealogy: { children: [] }, + ...overrides, + }); + return session.session_id as UUID; +} + +async function createTask(db: any, sessionId: UUID, overrides: Partial = {}): Promise { + const taskRepo = new TaskRepository(db); + const task = await taskRepo.create({ + task_id: generateId(), + session_id: sessionId, + created_by: 'test-user' as UUID, + full_prompt: 'Do a thing', + status: TaskStatus.COMPLETED, + message_range: { start_index: 0, end_index: 2, start_timestamp: new Date().toISOString() }, + tool_use_count: 1, + git_state: { ref_at_start: 'main', sha_at_start: 'abc123' }, + ...overrides, + }); + return task.task_id as UUID; +} + +describe('SessionsService.patch — agentic_tool immutability guard', () => { + dbTest('rejects an agentic_tool change on a session that already has a task', async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId, { agentic_tool: 'claude-code' }); + await createTask(db, sessionId); + + await expect(service.patch(sessionId, { agentic_tool: 'codex' })).rejects.toThrow( + /agentic_tool/ + ); + + const sessionRepo = new SessionRepository(db); + const reloaded = await sessionRepo.findById(sessionId); + expect(reloaded?.agentic_tool).toBe('claude-code'); + }); + + dbTest('rejects on a batch (multi) patch touching a session with a task', async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const withTask = await createSession(db, branchId, { agentic_tool: 'claude-code' }); + await createTask(db, withTask); + + await expect( + service.patch(null, { agentic_tool: 'codex' }, { query: { session_id: withTask } } as any) + ).rejects.toThrow(/agentic_tool/); + + const sessionRepo = new SessionRepository(db); + const reloaded = await sessionRepo.findById(withTask); + expect(reloaded?.agentic_tool).toBe('claude-code'); + }); + + dbTest('allows an agentic_tool change on a session with zero tasks', async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId, { agentic_tool: 'claude-code' }); + + const result = (await service.patch(sessionId, { agentic_tool: 'codex' })) as Session; + expect(result.agentic_tool).toBe('codex'); + }); + + dbTest( + 'allows a patch that sets agentic_tool to its current value even with tasks', + async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId, { agentic_tool: 'claude-code' }); + await createTask(db, sessionId); + + const result = (await service.patch(sessionId, { agentic_tool: 'claude-code' })) as Session; + expect(result.agentic_tool).toBe('claude-code'); + } + ); + + dbTest('allows unrelated field patches on a session with tasks', async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId, { agentic_tool: 'claude-code' }); + await createTask(db, sessionId); + + const result = (await service.patch(sessionId, { title: 'New title' })) as Session; + expect(result.title).toBe('New title'); + }); +}); diff --git a/apps/agor-daemon/src/services/sessions.swap-remove-guard.test.ts b/apps/agor-daemon/src/services/sessions.swap-remove-guard.test.ts new file mode 100644 index 000000000..d52a183aa --- /dev/null +++ b/apps/agor-daemon/src/services/sessions.swap-remove-guard.test.ts @@ -0,0 +1,148 @@ +/** + * TOCTOU on the "Switch tool" swap: `canSwitchTool` is evaluated once when + * the picker opens (session has zero tasks), but on a multiplayer canvas a + * task can land on that same session — another tab, a collaborator, an MCP + * `agor_sessions_prompt` call — before the swap *completes* and + * `chooseAgenticTool` calls `sessions.remove(replacingSessionId)`. Without a + * server-side re-check, that remove cascade-deletes the now-in-flight + * session and its task with no confirmation. + * + * The client marks a swap-triggered removal via `query._swapReplace` so the + * daemon can refuse *that* removal (Conflict) while leaving a normal, + * user-intentional delete of a session with history unaffected. + */ +import { + BranchRepository, + generateId, + RepoRepository, + SessionRepository, + TaskRepository, +} from '@agor/core/db'; +import type { Application } from '@agor/core/feathers'; +import type { Session, Task, UUID } from '@agor/core/types'; +import { SessionStatus, TaskStatus } from '@agor/core/types'; +import { describe, expect } from 'vitest'; +import { dbTest } from '../../../../packages/core/src/db/test-helpers'; +import { SessionsService } from './sessions'; + +const STUB_APP = {} as unknown as Application; + +async function createBranch(db: any): Promise { + const repoRepo = new RepoRepository(db); + const branchRepo = new BranchRepository(db); + const repo = await repoRepo.create({ + repo_id: generateId(), + slug: `repo-${generateId()}`, + name: 'Test Repo', + repo_type: 'remote' as const, + remote_url: 'https://github.com/test/repo.git', + local_path: '/tmp/test-repo', + default_branch: 'main', + }); + const branch = await branchRepo.create({ + branch_id: generateId(), + repo_id: repo.repo_id, + name: 'feature', + ref: 'feature', + branch_unique_id: Math.floor(Math.random() * 1_000_000), + path: '/tmp/test-repo', + base_ref: 'main', + new_branch: false, + created_by: 'test-user' as UUID, + }); + return branch.branch_id as UUID; +} + +async function createSession( + db: any, + branchId: UUID, + overrides: Partial = {} +): Promise { + const sessionRepo = new SessionRepository(db); + const session = await sessionRepo.create({ + session_id: generateId(), + branch_id: branchId, + agentic_tool: 'claude-code', + status: SessionStatus.IDLE, + created_by: 'test-user' as UUID, + git_state: { ref: 'main', base_sha: 'abc', current_sha: 'def' }, + tasks: [], + contextFiles: [], + genealogy: { children: [] }, + ...overrides, + }); + return session.session_id as UUID; +} + +async function createTask(db: any, sessionId: UUID, overrides: Partial = {}): Promise { + const taskRepo = new TaskRepository(db); + const task = await taskRepo.create({ + task_id: generateId(), + session_id: sessionId, + created_by: 'test-user' as UUID, + full_prompt: 'Do a thing', + status: TaskStatus.RUNNING, + message_range: { start_index: 0, end_index: 2, start_timestamp: new Date().toISOString() }, + tool_use_count: 1, + git_state: { ref_at_start: 'main', sha_at_start: 'abc123' }, + ...overrides, + }); + return task.task_id as UUID; +} + +describe('SessionsService.remove — swap-replace TOCTOU guard', () => { + dbTest( + 'refuses a _swapReplace removal when a task has landed on the session since the swap was initiated', + async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId); + + // Simulates the TOCTOU window: the picker opened while the session had + // zero tasks, but a task lands (another tab / collaborator / MCP call) + // before the swap's remove() call reaches the server. + await createTask(db, sessionId); + + await expect( + service.remove(sessionId, { query: { _swapReplace: true } } as any) + ).rejects.toThrow(/gained.*task/i); + + const sessionRepo = new SessionRepository(db); + const stillThere = await sessionRepo.findById(sessionId); + expect(stillThere).not.toBeNull(); + } + ); + + dbTest('allows a _swapReplace removal when the session still has zero tasks', async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId); + + const removed = (await service.remove(sessionId, { + query: { _swapReplace: true }, + } as any)) as Session; + expect(removed.session_id).toBe(sessionId); + + const sessionRepo = new SessionRepository(db); + expect(await sessionRepo.findById(sessionId)).toBeNull(); + }); + + dbTest( + 'a normal (non-swap) delete of a session with tasks is unaffected — no confirmation gate added', + async ({ db }) => { + const service = new SessionsService(db, STUB_APP); + const branchId = await createBranch(db); + const sessionId = await createSession(db, branchId); + await createTask(db, sessionId); + + // No `_swapReplace` marker — this is a user deleting a real session + // with history via the normal delete affordance, which must keep + // working exactly as before. + const removed = (await service.remove(sessionId)) as Session; + expect(removed.session_id).toBe(sessionId); + + const sessionRepo = new SessionRepository(db); + expect(await sessionRepo.findById(sessionId)).toBeNull(); + } + ); +}); diff --git a/apps/agor-daemon/src/services/sessions.ts b/apps/agor-daemon/src/services/sessions.ts index 4e5db5fbf..ac85ac174 100644 --- a/apps/agor-daemon/src/services/sessions.ts +++ b/apps/agor-daemon/src/services/sessions.ts @@ -13,10 +13,11 @@ import { SessionRelationshipRepository, SessionRepository, type SessionWithLastMessage, + TaskRepository, type TenantScopeAwareDatabase, UsersRepository, } from '@agor/core/db'; -import { type Application, BadRequest, Forbidden } from '@agor/core/feathers'; +import { type Application, BadRequest, Conflict, Forbidden } from '@agor/core/feathers'; import { formatModelToolMismatchWarning, formatUnsupportedAgorCodexModelMessage, @@ -162,6 +163,7 @@ export class SessionsService extends DrizzleService, S private sessionEnvSelectionRepo: SessionEnvSelectionRepository; private usersRepo: UsersRepository; private branchRepo: BranchRepository; + private taskRepo: TaskRepository; private assertSupportedModelConfig( agenticTool: Session['agentic_tool'], @@ -198,6 +200,33 @@ export class SessionsService extends DrizzleService, S // without going through app.service('users') — matches the convention used // by scheduler.ts / gateway.ts / terminals.ts. this.usersRepo = new UsersRepository(db); + this.taskRepo = new TaskRepository(db); + } + + /** + * `agentic_tool` picks the SDK a session's tasks are executed with — it + * can't change mid-session once a task exists (the messages/tasks already + * on the session were produced by a specific tool's SDK). The UI only + * offers "Switch tool" while `session.tasks.length === 0`, but that's a + * client-side convenience, not a security boundary: any other caller of + * `sessions.patch` (a stale tab, the MCP session-update tool, CLI) could + * otherwise desync `agentic_tool` from the tool that actually produced a + * session's existing tasks/messages. Enforce it here so the constraint + * holds regardless of caller. + */ + private async assertAgenticToolMutable(sessionId: string, nextTool: unknown): Promise { + if (nextTool === undefined) return; + + const existing = await this.sessionRepo.findById(sessionId); + if (!existing || existing.agentic_tool === nextTool) return; + + const taskCount = await this.taskRepo.countBySession(sessionId); + if (taskCount > 0) { + throw new Forbidden( + `Cannot change agentic_tool on session ${sessionId}: it already has ${taskCount} task(s). ` + + "The tool that produced a session's existing tasks/messages cannot be changed after the fact." + ); + } } protected async fetchData(_query: Query, params?: SessionParams): Promise { @@ -785,6 +814,24 @@ export class SessionsService extends DrizzleService, S // Get the session before deleting const session = await this.get(String(id), params); + // "Switch tool" (`chooseAgenticTool` in the UI) removes the session it's + // replacing as an implementation detail of swapping — never a user-visible + // delete. It's only offered on a session with zero tasks at the moment the + // swap is *initiated*, but on a multiplayer canvas a task can land on that + // same session (another tab, a collaborator, an MCP `agor_sessions_prompt` + // call) before the swap *completes*. Callers performing that specific swap + // mark the request via `query._swapReplace`; a normal user-intentional + // delete of a session with history is unaffected and still allowed. + if ((params?.query as { _swapReplace?: boolean } | undefined)?._swapReplace) { + const taskCount = await this.taskRepo.countBySession(String(id)); + if (taskCount > 0) { + throw new Conflict( + `Cannot complete tool switch: session ${id} has gained ${taskCount} task(s) since the switch ` + + 'was initiated. Refresh and try again — the in-flight work has not been touched.' + ); + } + } + // Find all children (forks and subsessions) const children = await this.sessionRepo.findChildren(String(id)); @@ -813,6 +860,18 @@ export class SessionsService extends DrizzleService, S data: Partial, params?: SessionParams ): Promise { + if (data.agentic_tool !== undefined) { + if (id === null) { + const found = await super.find(params); + const sessions = Array.isArray(found) ? found : (found as Paginated).data; + for (const session of sessions) { + await this.assertAgenticToolMutable(session.session_id, data.agentic_tool); + } + } else { + await this.assertAgenticToolMutable(String(id), data.agentic_tool); + } + } + const result = (await super.patch(id, data, params)) as Session | Session[]; const callbackEnabled = data.callback_config?.enabled; diff --git a/apps/agor-daemon/src/services/tasks.auto-title.test.ts b/apps/agor-daemon/src/services/tasks.auto-title.test.ts new file mode 100644 index 000000000..17db6ad68 --- /dev/null +++ b/apps/agor-daemon/src/services/tasks.auto-title.test.ts @@ -0,0 +1,208 @@ +import { type Session, type Task, TaskStatus } from '@agor/core/types'; +import { describe, expect, it, vi } from 'vitest'; +import { TasksService } from './tasks'; + +const sessionId = '018f0000-0000-7000-8000-000000000101'; +const taskId = '018f0000-0000-7000-8000-000000000201'; +const userId = '018f0000-0000-7000-8000-000000000401'; + +function makeTask(overrides: Partial = {}): Task { + return { + task_id: taskId, + session_id: sessionId, + created_by: userId, + full_prompt: 'Add a JWT-based authentication system with refresh token rotation', + status: TaskStatus.RUNNING, + message_range: { + start_index: 0, + end_index: 2, + start_timestamp: '2026-01-01T00:00:00.000Z', + }, + tool_use_count: 1, + git_state: { ref_at_start: 'main', sha_at_start: 'abc123' }, + created_at: '2026-01-01T00:00:00.000Z', + ...overrides, + } as Task; +} + +function makeSession(overrides: Partial = {}): Session { + return { + session_id: sessionId, + branch_id: undefined, + created_by: userId, + agentic_tool: 'claude-code', + status: 'running', + tasks: [taskId], + ready_for_prompt: false, + archived: false, + genealogy: { children: [] }, + git_state: {}, + contextFiles: [], + created_at: '2026-01-01T00:00:00.000Z', + updated_at: '2026-01-01T00:00:00.000Z', + ...overrides, + } as Session; +} + +function makeService(options: { task?: Partial; session?: Partial } = {}) { + const initialTask = makeTask(options.task); + const tasksById = new Map([[initialTask.task_id, initialTask]]); + const session = makeSession(options.session); + + const repository = { + findById: vi.fn(async (id: string) => tasksById.get(id) ?? null), + update: vi.fn(async (id: string, updates: Partial) => { + const current = tasksById.get(id) ?? makeTask({ task_id: id as Task['task_id'] }); + const updated = { ...current, ...updates } as Task; + tasksById.set(id, updated); + return updated; + }), + create: vi.fn(), + findAll: vi.fn(async () => [...tasksById.values()]), + delete: vi.fn(), + }; + + const sessionsPatch = vi.fn(async (id: string, updates: Partial) => { + Object.assign(session, updates); + return { ...session }; + }); + const triggerQueueProcessing = vi.fn(async () => undefined); + + const service = Object.create(TasksService.prototype) as TasksService & { + repository: typeof repository; + taskRepo: typeof repository & { createPending: ReturnType }; + id: string; + emit: ReturnType; + app: { service: ReturnType }; + completionCallbackDispatches: Map>; + }; + service.repository = repository; + service.taskRepo = { ...repository, createPending: vi.fn() }; + service.id = 'task_id'; + service.emit = vi.fn(); + service.completionCallbackDispatches = new Map(); + service.app = { + service: vi.fn((name: string) => { + if (name === 'sessions') { + return { get: vi.fn(async () => session), patch: sessionsPatch, triggerQueueProcessing }; + } + if (name === 'messages') return { find: vi.fn(async () => []) }; + if (name === 'branches') return { get: vi.fn() }; + throw new Error(`unexpected service ${name}`); + }), + }; + + return { service, sessionsPatch, session }; +} + +describe('TasksService auto-title', () => { + it('derives a title from the first prompt when the first task completes with no explicit title', async () => { + const { service, sessionsPatch } = makeService({ session: { title: undefined } }); + + await service.patch(taskId, { + status: TaskStatus.COMPLETED, + completed_at: '2026-01-01T00:00:05.000Z', + }); + + expect(sessionsPatch).toHaveBeenCalledWith( + sessionId, + expect.objectContaining({ + title: 'Add a JWT-based authentication system with refresh token…', + }), + undefined + ); + }); + + it('does not overwrite an explicit title', async () => { + const { service, sessionsPatch } = makeService({ session: { title: 'My session' } }); + + await service.patch(taskId, { + status: TaskStatus.COMPLETED, + completed_at: '2026-01-01T00:00:05.000Z', + }); + + const [, updates] = sessionsPatch.mock.calls[0]; + expect(updates).not.toHaveProperty('title'); + }); + + it('still derives a title from a later task when the session has none yet', async () => { + // Not a one-shot-at-task-1 gate: keeps attempting on every completed task + // until a title is set. Covers a first task that failed/was retried, or + // otherwise never produced an explicit title. + const earlierTaskId = '018f0000-0000-7000-8000-000000000202'; + const { service, sessionsPatch } = makeService({ + session: { title: undefined, tasks: [earlierTaskId, taskId] }, + }); + + await service.patch(taskId, { + status: TaskStatus.COMPLETED, + completed_at: '2026-01-01T00:00:05.000Z', + }); + + expect(sessionsPatch).toHaveBeenCalledWith( + sessionId, + expect.objectContaining({ + title: 'Add a JWT-based authentication system with refresh token…', + }), + undefined + ); + }); + + it('does not derive a title from an image-only (text-empty) first prompt', async () => { + const { service, sessionsPatch } = makeService({ + session: { title: undefined }, + task: { full_prompt: '' }, + }); + + await service.patch(taskId, { + status: TaskStatus.COMPLETED, + completed_at: '2026-01-01T00:00:05.000Z', + }); + + const [, updates] = sessionsPatch.mock.calls[0]; + expect(updates).not.toHaveProperty('title'); + }); + + it('picks up an auto-title on a second task after an image-only first prompt', async () => { + // The degenerate-first-prompt case this fix targets: an image-only paste + // derives '' and is skipped (previous test), but the session must not be + // permanently stuck titleless — a later task with real text should still + // fire the heuristic. + const firstTaskId = '018f0000-0000-7000-8000-000000000202'; + const secondTaskId = taskId; + const { service, sessionsPatch, session } = makeService({ + session: { title: undefined, tasks: [firstTaskId, secondTaskId] }, + task: { full_prompt: 'Summarize what changed in this screenshot' }, + }); + + // First task: image-only prompt, completes with no derivable title. + // (Simulated directly against session state — the first task isn't + // routed through this service instance in this test, only its effect: + // the session still has no title afterward.) + expect(session.title).toBeUndefined(); + + // Second task: real text, completes — should now derive and set a title. + await service.patch(secondTaskId, { + status: TaskStatus.COMPLETED, + completed_at: '2026-01-01T00:00:10.000Z', + }); + + expect(sessionsPatch).toHaveBeenCalledWith( + sessionId, + expect.objectContaining({ title: 'Summarize what changed in this screenshot' }), + undefined + ); + }); + + it('does not set a title when the task fails', async () => { + const { service, sessionsPatch } = makeService({ session: { title: undefined } }); + + await service.patch(taskId, { + status: TaskStatus.FAILED, + completed_at: '2026-01-01T00:00:05.000Z', + }); + + const [, updates] = sessionsPatch.mock.calls[0]; + expect(updates).not.toHaveProperty('title'); + }); +}); diff --git a/apps/agor-daemon/src/services/tasks.ts b/apps/agor-daemon/src/services/tasks.ts index 12c54f15b..8b1494698 100644 --- a/apps/agor-daemon/src/services/tasks.ts +++ b/apps/agor-daemon/src/services/tasks.ts @@ -18,6 +18,7 @@ import { type TenantScopeAwareDatabase, } from '@agor/core/db'; import type { Application } from '@agor/core/feathers'; +import { deriveTitleFromPrompt } from '@agor/core/sessions'; import type { ContentBlock, Paginated, @@ -609,12 +610,29 @@ export class TasksService extends DrizzleService, TaskParams `⏭️ [TasksService] Skipping session terminal-state update for task ${shortId(task.task_id)} (${data.status}) due to internal patch params` ); } else { + // Auto-title: cheap heuristic, no LLM call — see + // `deriveTitleFromPrompt`. Fires on any completed task while the + // session still has no explicit title (a user-set title, including + // one typed while this task was still running, always wins — this + // is a no-op by the time we get here) — not just the first one. + // A degenerate first prompt (image-only paste, whitespace) derives + // an empty title and is skipped below; without retrying on later + // tasks, a session whose first prompt happened to be text-empty — + // or whose first task failed/was retried — would never get an + // auto-title at all. Once a non-empty title is derived and + // persisted, `!session.title` goes false and this stops firing. + const autoTitle = + data.status === TaskStatus.COMPLETED && !session.title && task.full_prompt + ? deriveTitleFromPrompt(task.full_prompt) + : ''; + await this.app.service('sessions').patch( task.session_id, { status: data.status === TaskStatus.FAILED ? SessionStatus.FAILED : SessionStatus.IDLE, ready_for_prompt: true, + ...(autoTitle ? { title: autoTitle } : {}), }, params ); diff --git a/apps/agor-ui/src/components/AgentSelectionCard/AgentSelectionCard.tsx b/apps/agor-ui/src/components/AgentSelectionCard/AgentSelectionCard.tsx index 51e2268c6..657627ea2 100644 --- a/apps/agor-ui/src/components/AgentSelectionCard/AgentSelectionCard.tsx +++ b/apps/agor-ui/src/components/AgentSelectionCard/AgentSelectionCard.tsx @@ -24,14 +24,14 @@ export const AgentSelectionCard: React.FC = ({ cursor: 'pointer', }} styles={{ - body: { padding: 12 }, + body: { padding: 8 }, }} > - - - - - + + + + + {agent.name} {agent.beta && BETA} @@ -39,13 +39,13 @@ export const AgentSelectionCard: React.FC = ({ {agent.version && ( - + Version: {agent.version} )} {agent.description && ( - + {agent.description} )} diff --git a/apps/agor-ui/src/components/AgentSelectionGrid/AgentSelectionGrid.tsx b/apps/agor-ui/src/components/AgentSelectionGrid/AgentSelectionGrid.tsx index 0dfffd86f..c6d246385 100644 --- a/apps/agor-ui/src/components/AgentSelectionGrid/AgentSelectionGrid.tsx +++ b/apps/agor-ui/src/components/AgentSelectionGrid/AgentSelectionGrid.tsx @@ -89,7 +89,7 @@ export const AgentSelectionGrid: React.FC = ({ style={{ display: 'grid', gridTemplateColumns: `repeat(${columns}, 1fr)`, - gap: 8, + gap: 6, marginTop: 8, }} > diff --git a/apps/agor-ui/src/components/App/App.tsx b/apps/agor-ui/src/components/App/App.tsx index 76dca333a..820ae81d7 100644 --- a/apps/agor-ui/src/components/App/App.tsx +++ b/apps/agor-ui/src/components/App/App.tsx @@ -1,4 +1,5 @@ import type { + AgenticToolName, AgorClient, Artifact, Board, @@ -66,6 +67,7 @@ import { buildAssistantBootstrapPrompt } from '../../utils/assistantBootstrapPro import { createAssistantBranch } from '../../utils/assistantCreation'; import { initializeAudioOnInteraction } from '../../utils/audio'; import { useThemedMessage } from '../../utils/message'; +import { resolveQuickStartMcpServerIds } from '../../utils/resolveQuickStartMcpServerIds'; import { hasExplicitEntityRouteTarget } from '../../utils/routeTargets'; import { startAssistantBootstrapSession } from '../../utils/startAssistantBootstrapSession'; import { AppHeader } from '../AppHeader'; @@ -83,6 +85,7 @@ import { NewSessionButton } from '../NewSessionButton'; import { type NewSessionConfig, NewSessionModal } from '../NewSessionModal'; import { SessionCanvas, type SessionCanvasRef } from '../SessionCanvas'; import { SessionPanel } from '../SessionPanel'; +import { PendingToolChoicePanel } from '../SessionPanel/PendingToolChoicePanel'; import { SessionSettingsModal } from '../SessionSettingsModal'; import { SettingsModal, UserSettingsModal } from '../SettingsModal'; import { TerminalModal, WEB_TERMINAL_MIN_ROLE } from '../TerminalModal'; @@ -334,7 +337,7 @@ export const App: React.FC = ({ const gatewayChannelById = useAgorStore(selectGatewayChannelById); const artifactById = useAgorStore(selectArtifactById); - const { showWarning } = useThemedMessage(); + const { showWarning, showError } = useThemedMessage(); const location = useLocation(); const routeParams = useParams<{ sessionShortId?: string; @@ -346,6 +349,11 @@ export const App: React.FC = ({ const [pendingHomeNavigation, setPendingHomeNavigation] = useState(false); const sessionCanvasRef = useRef(null); const [newSessionBranchId, setNewSessionBranchId] = useState(null); + // Set instead of creating a session immediately when quick-start can't + // resolve a tool (no preference, no prior session for this user). Opens + // the drawer straight away in a "pick a tool" empty state rather than the + // old blocking modal — see `chooseAgenticTool` / `handleQuickStartSession`. + const [pendingToolChoiceBranchId, setPendingToolChoiceBranchId] = useState(null); const [createDialogOpen, setCreateDialogOpen] = useState(false); const [createDialogDefaultTab, setCreateDialogDefaultTab] = useState< 'branch' | 'assistant' | 'board' | 'repository' @@ -386,6 +394,11 @@ export const App: React.FC = ({ [isRootHomePath, pendingHomeNavigation, selectedSessionId, sessionById] ); + // A real selected session always wins; the pending tool-choice empty state + // only matters when there's no real session to show yet (see + // `handleQuickStartSession`). Both open the same drawer slot. + const sessionPanelTargetOpen = !!effectiveSelectedSessionId || !!pendingToolChoiceBranchId; + const [leftPanelTab, setLeftPanelTab] = useState('assistant'); const [userSettingsOpen, setUserSettingsOpen] = useState(false); const [viewportWidth, setViewportWidth] = useState(() => @@ -591,10 +604,10 @@ export const App: React.FC = ({ }, [effectiveCommentsPanelSize, leftPanelCollapsed]); useEffect(() => { - if (sessionPanelRef.current && (effectiveSelectedSessionId || !eventStreamPanelCollapsed)) { + if (sessionPanelRef.current && (sessionPanelTargetOpen || !eventStreamPanelCollapsed)) { sessionPanelRef.current.resize(effectiveSessionPanelSize); } - }, [effectiveSelectedSessionId, effectiveSessionPanelSize, eventStreamPanelCollapsed]); + }, [sessionPanelTargetOpen, effectiveSessionPanelSize, eventStreamPanelCollapsed]); // URL state synchronization - bidirectional sync between URL and state useUrlState({ @@ -748,6 +761,7 @@ export const App: React.FC = ({ // With the flat entity-URL scheme there's no `closeSession` — closing // the panel is the same as navigating to the board we're already on. const handleCloseSessionPanel = useCallback(() => { + setPendingToolChoiceBranchId(null); if (currentBoardId) navigation.goToBoard(currentBoardId); }, [navigation, currentBoardId]); @@ -769,6 +783,76 @@ export const App: React.FC = ({ } }; + // Single mechanism behind both "pick a tool" entry points: the quick-start + // empty-state tiles (no `replacingSessionId`, session doesn't exist yet) + // and the in-drawer "Switch tool" affordance (`replacingSessionId` set — + // only ever offered on a session with zero completed tasks, so there's no + // conversation to lose). Creates a session with the chosen tool, swaps out + // the session it's replacing (if any) with a silent delete — no confirm + // dialog, this is an implementation detail of "switching", not a + // user-visible delete — and navigates to the result. Does NOT remember the + // pick as a default — every "Add session" always shows the tile picker, + // by design (no silent auto-skip). + const chooseAgenticTool = useCallback( + async (branchId: string, tool: AgenticToolName, replacingSessionId?: string) => { + const branch = branchById.get(branchId as Branch['branch_id']); + const mcpServerIds = resolveQuickStartMcpServerIds(user, branch, tool); + + const sessionId = await onCreateSession?.( + { branch_id: branchId, agent: tool, mcpServerIds }, + currentBoardId + ); + if (!sessionId) return null; + + if (replacingSessionId && client) { + try { + // `_swapReplace` tells the daemon this is a switch-tool swap, not a + // user-intentional delete — it refuses the removal (Conflict) if a + // task has landed on `replacingSessionId` since the swap was + // initiated (e.g. a collaborator or another tab prompted it in the + // TOCTOU window between opening the picker and clicking a tile), + // instead of silently cascade-deleting in-flight work. + await client + .service('sessions') + .remove(replacingSessionId, { query: { _swapReplace: true } }); + } catch (error) { + console.error(`Failed to remove replaced session ${replacingSessionId}:`, error); + showError( + "Switched tools, but couldn't clean up the previous session — it may still be visible on this branch." + ); + } + } + + navigation.goToSession(sessionId); + return sessionId; + }, + [branchById, user, onCreateSession, currentBoardId, client, navigation, showError] + ); + + // Default "Add session" entry point: always opens the drawer straight away + // in the tool-choice empty state (no old blocking modal) — never silently + // resolves a tool and skips the picker. Every session starts with an + // explicit tool choice. + // + // The render ternary (`effectiveSelectedSessionId ? : + // pendingToolChoiceBranchId ? : ...`) lets an + // already-open session win unconditionally, so setting + // pendingToolChoiceBranchId alone would be a no-op while a session is + // open — clicking "Add session" would do nothing visible. Route away from + // the open session via the URL first, same pattern as + // handleCloseSessionPanel / handleCreateSession: useUrlState's URL→state + // sync effect clears selectedSessionId once the URL stops pointing at a + // session, which is what lets the ternary fall through to the picker. + const handleQuickStartSession = useCallback( + (branchId: string) => { + if (effectiveSelectedSessionId) { + navigation.goToBranch(branchId); + } + setPendingToolChoiceBranchId(branchId); + }, + [effectiveSelectedSessionId, navigation] + ); + const handleCreateBranch = async (config: BranchTabConfig) => { // Thread board placement (boardId + position) through the create // call so it lands atomically. The previous shape did a follow-up @@ -934,6 +1018,7 @@ export const App: React.FC = ({ // Route through URL nav so deep links / back-forward / cross-board // recenter all funnel through the same pipe. setSelectedSessionId // happens via useUrlState's onSessionChange callback. + setPendingToolChoiceBranchId(null); navigation.goToSession(sessionId); }, [client, navigation] @@ -1083,6 +1168,8 @@ export const App: React.FC = ({ setBranchModalTab(tab); }, onOpenTerminal: canOpenTerminal ? handleOpenTerminal : undefined, + onChooseAgenticTool: chooseAgenticTool, + availableAgents, }), [ onSendPrompt, @@ -1098,6 +1185,8 @@ export const App: React.FC = ({ handleSessionClick, handleOpenTerminal, canOpenTerminal, + chooseAgenticTool, + availableAgents, ] ); @@ -1241,7 +1330,7 @@ export const App: React.FC = ({ currentUserId={user?.user_id} selectedSessionId={effectiveSelectedSessionId} onSessionClick={handleSessionClick} - onCreateSession={setNewSessionBranchId} + onCreateSession={handleQuickStartSession} onForkSession={stableOnForkSession} onSpawnSession={stableOnSpawnSession} onArchiveOrDelete={stableOnArchiveOrDeleteBranch} @@ -1320,7 +1409,7 @@ export const App: React.FC = ({
@@ -1355,7 +1444,7 @@ export const App: React.FC = ({ onSpawnSession={stableOnSpawnSession} onUpdateSessionMcpServers={stableOnUpdateSessionMcpServers} onOpenSettings={setSessionSettingsId} - onCreateSessionForBranch={setNewSessionBranchId} + onCreateSessionForBranch={handleQuickStartSession} onOpenBranch={setBranchModalBranchId} onArchiveOrDeleteBranch={stableOnArchiveOrDeleteBranch} onOpenTerminal={canOpenTerminal ? handleOpenTerminal : undefined} @@ -1380,7 +1469,7 @@ export const App: React.FC = ({ )}
- {(effectiveSelectedSessionId || !eventStreamPanelCollapsed) && ( + {(sessionPanelTargetOpen || !eventStreamPanelCollapsed) && ( <> = ({ open={!!effectiveSelectedSessionId} onClose={handleCloseSessionPanel} /> + ) : pendingToolChoiceBranchId ? ( + { + await chooseAgenticTool(pendingToolChoiceBranchId, tool); + }} + onClose={handleCloseSessionPanel} + onAdvancedSetup={() => { + setNewSessionBranchId(pendingToolChoiceBranchId); + setPendingToolChoiceBranchId(null); + }} + /> ) : ( = ({ client={client} branchActions={{ onSessionClick: handleSessionClick, - onCreateSession: (branchId) => setNewSessionBranchId(branchId), + onCreateSession: (branchId) => handleQuickStartSession(branchId), onOpenSettings: (branchId) => setBranchModalBranchId(branchId), onNukeEnvironment, }} diff --git a/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.test.tsx b/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.test.tsx new file mode 100644 index 000000000..82fdd5047 --- /dev/null +++ b/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.test.tsx @@ -0,0 +1,91 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { App, ConfigProvider } from 'antd'; +import type React from 'react'; +import { describe, expect, it, vi } from 'vitest'; +import type { AgenticToolOption } from '../AgentSelectionGrid/AgentSelectionGrid'; +import { PendingToolChoicePanel } from './PendingToolChoicePanel'; + +const Wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( + + {children} + +); + +const agents: AgenticToolOption[] = [ + { id: 'claude-code', name: 'Claude Code', icon: '🤖', description: 'Anthropic' }, + { id: 'codex', name: 'Codex', icon: '💻', description: 'OpenAI' }, +]; + +describe('PendingToolChoicePanel', () => { + it('renders one tile per available agent and the inert composer bar', () => { + render( + + + + ); + + expect(screen.getByText('Claude Code')).toBeInTheDocument(); + expect(screen.getByText('Codex')).toBeInTheDocument(); + expect(screen.getByText('Untitled session')).toBeInTheDocument(); + + const textarea = screen.getByPlaceholderText('Pick a tool above to start typing…'); + expect(textarea).toHaveAttribute('disabled'); + expect(screen.getByText('Options').closest('button')).toHaveAttribute('disabled'); + expect(screen.getByText('Send').closest('button')).toHaveAttribute('disabled'); + }); + + it('calls onChoose with the tool id when a tile is clicked', () => { + const onChoose = vi.fn(); + render( + + + + ); + + fireEvent.click(screen.getByText('Codex')); + expect(onChoose).toHaveBeenCalledWith('codex'); + }); + + it('only shows the advanced-setup escape hatch when a handler is provided', () => { + const { rerender } = render( + + + + ); + expect(screen.queryByText(/advanced setup/i)).not.toBeInTheDocument(); + + const onAdvancedSetup = vi.fn(); + rerender( + + + + ); + fireEvent.click(screen.getByText(/advanced setup/i)); + expect(onAdvancedSetup).toHaveBeenCalled(); + }); +}); diff --git a/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.tsx b/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.tsx new file mode 100644 index 000000000..0ebf40320 --- /dev/null +++ b/apps/agor-ui/src/components/SessionPanel/PendingToolChoicePanel.tsx @@ -0,0 +1,198 @@ +/** + * PendingToolChoicePanel — quick-start empty state + * + * Shown in place of the session drawer's conversation + composer when + * "Add session" can't resolve a tool on its own (no preference, no prior + * session for this user). Rather than blocking on the old modal, the drawer + * still opens immediately with this panel: tiles instead of a form, one per + * available agentic tool. Picking one creates the session and this panel is + * swapped out for the real `SessionPanel`. + * + * Chrome (header layout, footer bar shape/spacing, muted secondary copy) + * mirrors `SessionPanel`'s own header/footer and the Connect-AI empty state + * (`MissingCredentialPanel`, feature-connect-ai-empty-state) so the drawer + * doesn't visibly change shape between "picking a tool" and "using a + * session" — just swap tiles for a form, same panel chrome throughout. + * + * The composer bar below the tiles is intentionally inert (not hidden): a + * disabled textarea + send + options button in the same position the real + * `SessionFooter` renders them, so there's no valid-looking-but-broken + * affordance before a tool is chosen. It unlocks the instant a tile is + * picked, when this panel unmounts in favor of the real session. + */ + +import type { AgenticToolName, Branch } from '@agor-live/client'; +import { CloseOutlined, RobotOutlined, SendOutlined, SettingOutlined } from '@ant-design/icons'; +import { Button, Input, Spin, Typography, theme } from 'antd'; +import type React from 'react'; +import { useState } from 'react'; +import { + type AgenticToolOption, + AgentSelectionGrid, +} from '../AgentSelectionGrid/AgentSelectionGrid'; + +export interface PendingToolChoicePanelProps { + open: boolean; + branch: Branch | null; + availableAgents: AgenticToolOption[]; + onChoose: (tool: AgenticToolName) => void | Promise; + onClose: () => void; + /** Escape hatch for the rare case someone wants title/prompt/MCP/env vars set up front. */ + onAdvancedSetup?: () => void; +} + +export const PendingToolChoicePanel: React.FC = ({ + open, + branch, + availableAgents, + onChoose, + onClose, + onAdvancedSetup, +}) => { + const { token } = theme.useToken(); + const [choosingTool, setChoosingTool] = useState(null); + + const handleChoose = async (toolId: string) => { + if (choosingTool) return; + setChoosingTool(toolId); + try { + await onChoose(toolId as AgenticToolName); + } finally { + setChoosingTool(null); + } + }; + + return ( +
+ {/* Header — mirrors SessionPanel's header chrome */} +
+
+ + + {branch ? `Untitled session — ${branch.name}` : 'Untitled session'} + +
+
+ + {/* Body — tile picker, one per available agentic tool */} +
+
+ + Choose which AI tool this session should use. + + +
+ + {choosingTool && ( +
+ +
+ )} +
+ + {onAdvancedSetup && ( + + )} +
+
+ + {/* Footer — same position/shape as SessionFooter, inert until a tile is picked */} +
+ +
+ +
+ +
+
+
+ ); +}; diff --git a/apps/agor-ui/src/components/SessionPanel/SessionPanel.quickstart.test.tsx b/apps/agor-ui/src/components/SessionPanel/SessionPanel.quickstart.test.tsx new file mode 100644 index 000000000..ce57fbf55 --- /dev/null +++ b/apps/agor-ui/src/components/SessionPanel/SessionPanel.quickstart.test.tsx @@ -0,0 +1,207 @@ +import type { Branch, Session } from '@agor-live/client'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { App as AntApp } from 'antd'; +import { describe, expect, it, vi } from 'vitest'; +import { AppActionsProvider } from '../../contexts/AppActionsContext'; +import { ConnectionProvider } from '../../contexts/ConnectionContext'; +import type { AgenticToolOption } from '../../types'; +import SessionPanel from './SessionPanel'; + +vi.mock('../AutocompleteTextarea', () => ({ + AutocompleteTextarea: () =>