diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 947e06408fc..9f9d764551d 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -83,6 +83,7 @@ import { } from './internal/testUtils.js'; import { SessionArtifactAuthorizationError } from './sessionArtifacts.js'; import { + REQUESTED_SESSION_ID_META_KEY, MID_TURN_QUEUE_DRAIN_METHOD, PROMPT_CANCEL_METHOD, TODO_STOP_GUARD_CONTINUATION_CLAIM_METHOD, @@ -1282,6 +1283,27 @@ describe('createAcpSessionBridge', () => { expect(handles[0]?.killed).toBe(true); }); + it('injects REQUESTED_SESSION_ID_META_KEY into newSession _meta when sessionId is set', async () => { + const handles: ChannelHandle[] = []; + const factory: ChannelFactory = async () => { + const h = makeChannel(); + handles.push(h); + return h.channel; + }; + const bridge = makeBridge({ channelFactory: factory }); + + await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }); + + expect(handles[0]?.agent.newSessionCalls[0]!._meta).toMatchObject({ + [REQUESTED_SESSION_ID_META_KEY]: '550e8400-e29b-41d4-a716-446655440000', + }); + + await bridge.shutdown(); + }); + it('reuses the existing session under sessionScope:single', async () => { const handles: ChannelHandle[] = []; const factory: ChannelFactory = async () => { diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 3dcdcfe968c..410a7d507e4 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -109,6 +109,7 @@ import { LOAD_REPLAY_PAGE_SIZE_META_KEY, LOAD_REPLAY_VERSION, PROMPT_CANCEL_METHOD, + REQUESTED_SESSION_ID_META_KEY, TODO_STOP_GUARD_QUEUE_RELEASE_METHOD, WORKTREE_MCP_DEFER_META_KEY, } from './bridgeTypes.js'; @@ -2602,6 +2603,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { sourceId?: string, worktree?: { slug: string; path: string; branch: string }, branch?: { name: string; baseBranch: string }, + requestedSessionId?: string, ): Promise { // Get-or-create the daemon's single channel, then call // `connection.newSession()` on it. Sessions share the child's @@ -2658,8 +2660,17 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const request = telemetry.injectPromptContext({ cwd: boundWorkspace, mcpServers: [], - ...(sourceType - ? { _meta: sessionSourceRequestMeta(sourceType, sourceId) } + ...(requestedSessionId || sourceType + ? { + _meta: { + ...sessionSourceRequestMeta(sourceType, sourceId), + ...(requestedSessionId + ? { + [REQUESTED_SESSION_ID_META_KEY]: requestedSessionId, + } + : {}), + }, + } : {}), }); const response = await withTimeout( @@ -5355,6 +5366,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { source.sourceId, req.worktree, req.branch, + req.sessionId, ); // Track in-flight spawns regardless of scope. Under `single` // this also serves the coalescing path above (a parallel diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index f9973bfe81c..a040ec01099 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -98,6 +98,14 @@ export interface BridgeSpawnRequest { worktree?: { slug: string; path: string; branch: string }; /** Branch metadata, set by the daemon route before spawn. */ branch?: { name: string; baseBranch: string }; + /** + * Optional caller-supplied session id. When provided, the agent uses this + * id instead of generating a random UUID. Must be validated at the route + * boundary since the core Config constructor uses it verbatim. Passed + * through ACP `_meta` since the protocol's NewSessionRequest has no native + * sessionId field. + */ + sessionId?: string; } export interface BridgeSession { @@ -172,6 +180,8 @@ export const LOAD_REPLAY_HIDE_INHERITED_META_KEY = export const LOAD_REPLAY_BULK_MODE = 'bulk'; export const LOAD_REPLAY_VERSION = 1 as const; +export const REQUESTED_SESSION_ID_META_KEY = 'qwen-code/sessionId'; + export const CHANNEL_STARTUP_PROFILE_META_KEY = 'qwen.daemon.channelStartupProfile'; export const CHANNEL_STARTUP_PROFILE_VERSION = 1 as const; diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index fd967bde581..f2e046ff6eb 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -723,6 +723,14 @@ vi.mock('../config/loadedSettingsAdapter.js', () => ({ vi.mock('../config/config.js', () => ({ loadCliConfig: vi.fn(), buildDisabledSkillNamesProvider: vi.fn(() => () => new Set()), + SessionIdConflictError: class SessionIdConflictError extends Error { + sessionId: string; + constructor(sessionId: string, message: string) { + super(message); + this.name = 'SessionIdConflictError'; + this.sessionId = sessionId; + } + }, })); vi.mock('../ui/commands/contextCommand.js', () => ({ collectContextData: vi.fn().mockResolvedValue({ @@ -843,13 +851,13 @@ import type { McpServer, ResumeSessionResponse, } from '@agentclientprotocol/sdk'; -import { AgentSideConnection } from '@agentclientprotocol/sdk'; +import { AgentSideConnection, RequestError } from '@agentclientprotocol/sdk'; import { loadSettings, SettingScope } from '../config/settings.js'; import { MAX_PERMISSION_RULE_LENGTH, MAX_PERMISSION_RULES_COUNT, } from '../config/permission-settings.js'; -import { loadCliConfig } from '../config/config.js'; +import { loadCliConfig, SessionIdConflictError } from '../config/config.js'; import { createLoadedSettingsAdapter } from '../config/loadedSettingsAdapter.js'; import { AcpFileSystemService } from './service/filesystem.js'; import { Session, buildAvailableCommandsSnapshot } from './session/Session.js'; @@ -2804,6 +2812,56 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('forwards a caller-supplied sessionId from _meta to loadCliConfig', async () => { + await setupSessionMocks('meta-session'); + const { agent, agentPromise } = await bootAcpAgent(); + + await agent.newSession({ + cwd: '/tmp', + mcpServers: [], + _meta: { 'qwen-code/sessionId': '550e8400-e29b-41d4-a716-446655440000' }, + }); + + const argv = vi.mocked(loadCliConfig).mock.calls[0]![1]; + expect(argv).toMatchObject({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }); + // Index 8 is `throwOnSessionIdConflict`: it must be true so a duplicate + // caller-supplied id throws (mapped to a RequestError) instead of + // process.exit(1)-ing the shared ACP child. + expect(vi.mocked(loadCliConfig).mock.calls[0]![8]).toBe(true); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('maps a duplicate sessionId conflict to a RequestError instead of crashing the child', async () => { + const conflict = new SessionIdConflictError( + '550e8400-e29b-41d4-a716-446655440000', + 'Error: Session Id 550e8400-e29b-41d4-a716-446655440000 already exists (active or archived). Delete or unarchive it first.', + ); + vi.mocked(loadCliConfig).mockRejectedValue(conflict); + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await expect( + agent.newSession({ cwd: '/tmp', mcpServers: [] }), + ).rejects.toThrow('already exists'); + expect(RequestError.invalidParams).toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('creates a session when OpenTelemetry is disabled', async () => { mockWithDaemonSpan.mockImplementationOnce( async (_name, _attributes, fn) => await fn(undefined), diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index bdfc381826e..709e21c79a1 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -199,6 +199,7 @@ import type { CliArgs } from '../config/config.js'; import { buildDisabledSkillNamesProvider, loadCliConfig, + SessionIdConflictError, } from '../config/config.js'; import { resolveSkillSettings } from '../config/skill-settings.js'; import { @@ -306,6 +307,7 @@ import { LOAD_REPLAY_PAGE_SIZE_META_KEY, LOAD_REPLAY_VERSION, PROMPT_CANCEL_METHOD, + REQUESTED_SESSION_ID_META_KEY, TODO_STOP_GUARD_QUEUE_RELEASE_METHOD, WORKTREE_MCP_DEFER_META_KEY, type ClientMcpOverWsRuntimeConfig, @@ -4422,6 +4424,10 @@ class QwenAgent implements Agent { async newSession(params: NewSessionRequest): Promise { const { cwd, mcpServers } = params; + const requestedSessionId = + typeof params._meta?.[REQUESTED_SESSION_ID_META_KEY] === 'string' + ? (params._meta[REQUESTED_SESSION_ID_META_KEY] as string) + : undefined; const sessionSource = getSessionSource(params); const parentContext = extractDaemonTraceContext(params); return await withDaemonSpan( @@ -4445,7 +4451,7 @@ class QwenAgent implements Agent { mcpServers, settings, sessionSource, - undefined, + requestedSessionId, undefined, shouldDeferMcpDiscovery(params) ? { skipMcpDiscovery: true } @@ -10900,6 +10906,9 @@ class QwenAgent implements Agent { ); }); } catch (error) { + if (error instanceof SessionIdConflictError) { + throw RequestError.invalidParams(undefined, error.message); + } const writerError = getSessionWriterError(error); if (writerError) { throw new RequestError(writerError.rpcCode, writerError.message, { @@ -11025,6 +11034,12 @@ class QwenAgent implements Agent { // into the first at cold start. buildDisabledSkillNamesProvider(settings), sessionMcpServers, + // The daemon owns the settings watcher lifecycle. + undefined, + // A duplicate caller-supplied session id must fail this one request, + // not process.exit(1) the shared ACP child and every session on its + // channel. newSessionConfig maps the throw to a RequestError. + true, ); if (sessionSource) { config.setSessionSource(sessionSource.sourceType, sessionSource.sourceId); diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index e9e4367956c..4fcebc7f4d1 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -14,7 +14,12 @@ import { NativeLspService, Storage, } from '@qwen-code/qwen-code-core'; -import { loadCliConfig, parseArguments, type CliArgs } from './config.js'; +import { + loadCliConfig, + parseArguments, + SessionIdConflictError, + type CliArgs, +} from './config.js'; import type { Settings } from './settings.js'; import * as ServerConfig from '@qwen-code/qwen-code-core'; import { isWorkspaceTrusted } from './trustedFolders.js'; @@ -28,6 +33,7 @@ const mockSessionServiceInstance = vi.hoisted(() => ({ loadSession: vi.fn(), forkSession: vi.fn(), sessionExists: vi.fn(), + sessionExistsInAnyState: vi.fn(), })); const mockSessionServiceCtor = vi.hoisted(() => vi.fn(() => mockSessionServiceInstance), @@ -1011,6 +1017,7 @@ describe('loadCliConfig', () => { copiedCount: 1, }); mockSessionServiceInstance.sessionExists.mockResolvedValue(false); + mockSessionServiceInstance.sessionExistsInAnyState.mockResolvedValue(false); vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); vi.stubEnv('GEMINI_API_KEY', 'test-api-key'); resetMcpApprovalsForTesting(); @@ -1675,6 +1682,63 @@ describe('loadCliConfig', () => { expect(mockExit).toHaveBeenCalledWith(1); }); + it('should exit when a caller-supplied sessionId already exists (default CLI behavior)', async () => { + const sessionId = '123e4567-e89b-12d3-a456-426614174000'; + mockSessionServiceInstance.sessionExistsInAnyState.mockResolvedValue(true); + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + await expect(loadCliConfig({}, { sessionId } as CliArgs)).rejects.toThrow( + 'process.exit called', + ); + + expect(mockExit).toHaveBeenCalledWith(1); + }); + + it('should throw SessionIdConflictError instead of exiting when throwOnSessionIdConflict is set', async () => { + const sessionId = '123e4567-e89b-12d3-a456-426614174000'; + mockSessionServiceInstance.sessionExistsInAnyState.mockResolvedValue(true); + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + const promise = loadCliConfig( + {}, + { sessionId } as CliArgs, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + true, + ); + + await expect(promise).rejects.toBeInstanceOf(SessionIdConflictError); + await expect(promise).rejects.toMatchObject({ sessionId }); + expect(mockExit).not.toHaveBeenCalled(); + }); + + it('should not throw for a fresh caller-supplied sessionId when throwOnSessionIdConflict is set', async () => { + const sessionId = '123e4567-e89b-12d3-a456-426614174000'; + mockSessionServiceInstance.sessionExistsInAnyState.mockResolvedValue(false); + + const config = await loadCliConfig( + {}, + { sessionId } as CliArgs, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + true, + ); + + expect(config.getSessionId()).toBe(sessionId); + }); + it('should use internal sandbox session ID without treating it as a new session', async () => { const sessionId = '123e4567-e89b-12d3-a456-426614174000'; vi.stubEnv('SANDBOX', 'sandbox-exec'); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 9fceff2488f..34ab284f810 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1475,6 +1475,22 @@ export function buildDisabledSkillNamesProvider( return () => resolveSkillSettings(loadedSettings).disabledNames; } +/** + * Thrown (instead of `process.exit(1)`) when a caller-supplied session id + * already exists and `throwOnSessionIdConflict` is set. The interactive CLI + * exits the process on a duplicate id, but that would kill a shared ACP child + * and every session on its channel — embedded callers catch this and fail the + * single request instead. + */ +export class SessionIdConflictError extends Error { + readonly sessionId: string; + constructor(sessionId: string, message: string) { + super(message); + this.name = 'SessionIdConflictError'; + this.sessionId = sessionId; + } +} + export async function loadCliConfig( settings: Settings, argv: CliArgs, @@ -1519,6 +1535,13 @@ export async function loadCliConfig( * core decoupled from the CLI-owned `SettingsWatcher` implementation. */ settingsWatcher?: { stopWatching(): void }, + /** + * When true, a duplicate caller-supplied session id throws + * `SessionIdConflictError` instead of calling `process.exit(1)`. Embedded + * callers (ACP/daemon) set this so one conflicting `newSession` degrades a + * single request rather than terminating the shared child process. + */ + throwOnSessionIdConflict = false, ): Promise { const debugMode = isDebugMode(argv); if (debugMode && process.env['QWEN_DEBUG_LOG_FILE'] === undefined) { @@ -1997,6 +2020,9 @@ export async function loadCliConfig( ); if (exists) { const message = `Error: Session Id ${argv['sessionId']} already exists (active or archived). Delete or unarchive it first.`; + if (throwOnSessionIdConflict) { + throw new SessionIdConflictError(argv['sessionId'], message); + } writeStderrLine(message); process.exit(1); } diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 4093077773a..57c37d3e469 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -13,12 +13,14 @@ import { GROUP_COLOR_OPTIONS, GitWorktreeService, SessionOrganizationError, + SessionService, SESSION_TRANSCRIPT_MAX_LIMIT, SESSION_TRANSCRIPT_MAX_PAGE_BYTES, SessionTranscriptPageTooLargeError, SessionTranscriptCursorCodec, SessionTranscriptReader, SessionTranscriptSnapshotUnavailableError, + Storage, addDaemonRequestAttribute, runWithoutDebugLogSession, writeWorktreeSessionMarker, @@ -33,6 +35,7 @@ import type { SessionArtifactInput } from '@qwen-code/acp-bridge/sessionArtifact import { parseSessionSource } from '@qwen-code/acp-bridge'; import type { Application, Request, RequestHandler, Response } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; +import { loadSettingsCached } from '../../config/settings-cache.js'; import { isChannelDeliveryError } from '../channel-delivery-ipc.js'; import { parseChannelDelivery } from '../channel-delivery.js'; import { @@ -137,6 +140,20 @@ const GIT_RESERVED_BRANCH = 'HEAD'; const MAX_BRANCH_NAME_BYTES = 1000; const MAX_BRANCH_COMPONENT_BYTES = 200; +// A strict SUBSET of config.ts's isValidSessionId: same v4 version/variant +// nibbles, minus the `-agent-{suffix}` form (SessionService.SESSION_FILE_PATTERN +// only matches 32-36 hex/hyphen chars, so a suffixed id would write a transcript +// the session list can never see). +// +// Keeping it a subset in BOTH directions matters: every id the daemon accepts +// must also be a valid `--session-id` and `/resume ` argument, otherwise a +// session created over HTTP is unreachable from the CLI (resumeCommand.ts gates +// on isValidSessionId and falls through to title matching when it fails). That +// rules out UUIDv7, the nil UUID, and non-RFC-4122 variants even though they are +// harmless as filenames. +const HTTP_SESSION_ID_REGEX = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + interface RegisterSessionRoutesDeps { boundWorkspace: string; bridge: AcpSessionBridge; @@ -376,6 +393,23 @@ function shouldPreserveTranscriptResolutionError(err: unknown): boolean { ); } +/** + * Whether a session with this id is currently live on the daemon. + * `getSessionSummary` is a `byId` lookup that signals absence by throwing + * `SessionNotFoundError`. Anything else is treated as "assume live" rather + * than "not live" — for a uniqueness guard, failing closed on an unreadable + * bridge is the safe direction, and it matches how + * `SessionService.sessionExistsInAnyState` handles its own read errors. + */ +function isSessionLive(bridge: AcpSessionBridge, sessionId: string): boolean { + try { + bridge.getSessionSummary(sessionId); + return true; + } catch (err) { + return !(err instanceof SessionNotFoundError); + } +} + function parseOptionalApprovalMode( body: Record, res: Response, @@ -434,6 +468,10 @@ export function registerSessionRoutes( // after spawn). Closes the TOCTOU where two concurrent requests both pass // the guard before either populates `activeBranchSessions`. const inFlightBranchWorkspaces = new Set(); + // Caller-supplied session ids with a creation currently in flight. Closes + // the TOCTOU where two concurrent requests with the same sessionId both + // pass sessionExistsInAnyState before either session is created. + const inFlightSessionIds = new Set(); /** Remove the branch-session tracking entry when a session ends. */ const clearBranchSessionEntry = (sessionId: string): void => { @@ -1281,286 +1319,369 @@ export function registerSessionRoutes( const clientId = parseClientIdHeader(req, res); if (clientId === null) return; - // ── Branch creation ──────────────────────────────────────────── - // When `branch` is present, create and checkout a new git branch - // before spawning. The session runs in the same working directory - // but on the new branch. Mutually exclusive with `worktree`. - let branchMeta: { name: string; baseBranch: string } | undefined; - let branchBaseCommit: string | undefined; - const rawBranch = body['branch']; - if (rawBranch !== undefined && rawBranch !== null) { - if (body['worktree'] !== undefined && body['worktree'] !== null) { - res.status(400).json({ - error: '`branch` and `worktree` are mutually exclusive', - code: 'branch_and_worktree_conflict', - }); - return; - } - if (typeof rawBranch !== 'object' || Array.isArray(rawBranch)) { - res.status(400).json({ - error: - '`branch` must be an object (e.g. `{"name":"feat/my-feature"}`)', - code: 'invalid_branch', - }); - return; - } - const branchReq = rawBranch as Record; - const branchName = branchReq['name']; - if (typeof branchName !== 'string' || branchName.length === 0) { - res.status(400).json({ - error: '`branch.name` must be a non-empty string', - code: 'branch_invalid_name', - }); - return; - } - // Validate git branch name characters and reserved names. - // Mirrors validateBranchName in GitModePopover.tsx; keep in sync. + // Optional caller-supplied session id. Validated at the route boundary + // so a 400 surfaces before touching the bridge. The core Config + // constructor uses it verbatim (falling back to randomUUID when absent). + const rawSessionId = body['sessionId']; + let requestedSessionId: string | undefined; + if (rawSessionId !== undefined && rawSessionId !== null) { if ( - /[^\p{L}\p{N}._/-]/u.test(branchName) || - branchName.includes('..') || - branchName.includes('//') || - branchName.startsWith('.') || - branchName.startsWith('-') || - branchName.startsWith('/') || - branchName.endsWith('/') || - branchName.endsWith('.') || - branchName.endsWith('.git') || - branchName.includes('@{') || - branchName - .split('/') - .some((c) => c.startsWith('.') || c.endsWith('.lock')) || - branchName.toUpperCase() === GIT_RESERVED_BRANCH || - Buffer.byteLength(branchName, 'utf8') > MAX_BRANCH_NAME_BYTES || - branchName - .split('/') - .some( - (c) => Buffer.byteLength(c, 'utf8') > MAX_BRANCH_COMPONENT_BYTES, - ) + typeof rawSessionId !== 'string' || + !HTTP_SESSION_ID_REGEX.test(rawSessionId) ) { res.status(400).json({ - error: `Invalid branch name: ${branchName}`, - code: 'branch_invalid_name', - }); - return; - } - // Reject when another branch session is already active for this - // workspace — concurrent branch sessions conflict on HEAD. Runs after - // shape/name validation so a malformed body gets 400, not 409. - const existingBranchSession = activeBranchSessions.get(workspaceCwd); - if (existingBranchSession) { - try { - // Throws if the session is gone, letting us clean up the stale entry. - runtime.bridge.getSessionSummary(existingBranchSession); - res.status(409).json({ - error: 'A branch session is already active for this workspace', - code: 'branch_session_conflict', - existingSessionId: existingBranchSession, - }); - return; - } catch { - activeBranchSessions.delete(workspaceCwd); - } - } - // Reject when any other live (client-attached) non-worktree session - // already runs in this workspace. `git checkout -b` moves the shared - // HEAD, so a concurrent current-branch session with a clean tree would - // be silently relocated onto the new branch and commit to the wrong - // ref. Worktree sessions are exempt (they run in their own cwd). Scoped - // to sessions with an attached client so a detached session left behind - // by a "new chat" does not block a fresh branch session. - const sharedCheckoutSession = runtime.bridge - .listWorkspaceSessions(workspaceCwd) - .find((session) => !session.worktree && session.clientCount > 0); - if (sharedCheckoutSession) { - res.status(409).json({ error: - 'Another session is already active in this workspace; creating a branch would move its shared checkout', - code: 'branch_session_conflict', - existingSessionId: sharedCheckoutSession.sessionId, - }); - return; - } - let wtService: GitWorktreeService; - try { - wtService = new GitWorktreeService(workspaceCwd); - } catch { - res.status(500).json({ - error: 'Failed to initialize git service', - code: 'branch_init_failed', - }); - return; - } - if (!(await wtService.isGitRepository())) { - res.status(400).json({ - error: 'Branch creation requires a git repository', - code: 'branch_not_git_repo', - }); - return; - } - // Check the branch doesn't already exist. - if (await branchExists(workspaceCwd, branchName)) { - res.status(409).json({ - error: `Branch "${branchName}" already exists`, - code: 'branch_already_exists', - }); - return; - } - // Gate on a dirty tree as surprise-prevention: `git checkout -b` carries - // uncommitted tracked changes onto the new branch, which would silently - // mix the user's WIP with a fresh branch. Untracked files are excluded - // (`--untracked-files=no`) because they survive any checkout unchanged. - let dirty: boolean; - try { - dirty = await isDirtyTree(workspaceCwd); - } catch { - res.status(500).json({ - error: 'Failed to check working tree status', - code: 'branch_status_failed', + '`sessionId` must be a UUID (e.g. "550e8400-e29b-41d4-a716-446655440000")', + code: 'invalid_session_id', }); return; } - if (dirty) { + requestedSessionId = rawSessionId.toLowerCase(); + const sessionIdToCheck = requestedSessionId; + // Reject an id that is already LIVE on this workspace's bridge. + // The disk check below cannot see these: the transcript JSONL is + // only written on a session's first message, so a session that was + // created and never prompted leaves no file behind — for its whole + // lifetime, not just a brief window. Without this, a sequential retry + // with the same id falls through to the agent's own + // `Session is already active.` guard, which is a bare Error and + // surfaces as an opaque `500 / -32603` instead of the 409 this route + // documents. `inFlightSessionIds` below only covers requests that + // overlap in time; this covers a first request that already returned. + // + // Per-workspace scope: checks only the current workspace's bridge. + // The daemon-wide `inFlightSessionIds` guard covers concurrent + // cross-workspace reuse; sequential cross-workspace reuse of the same + // id can still produce two live sessions sharing an id (routing then + // fails as `ambiguous_session_owner`). + if (isSessionLive(runtime.bridge, sessionIdToCheck)) { res.status(409).json({ - error: 'Uncommitted changes detected. Commit or stash first.', - code: 'branch_dirty_tree', + error: `Session "${requestedSessionId}" already exists`, + code: 'session_id_conflict', }); return; } - const baseCommit = await getHeadCommit(workspaceCwd); - const baseBranch = await wtService.getCurrentBranch().catch(() => 'HEAD'); - // Reserve the workspace before mutating HEAD. The conflict guard above - // runs before several awaits (rev-parse, status, checkout), so two - // concurrent `POST /session { branch }` can both pass it and race on - // `git checkout -b`. This synchronous check-and-add (no await between) - // serializes the checkout; every exit path below clears the reservation - // (transferred to `activeBranchSessions` on success). Re-check - // `activeBranchSessions` here too: a request that passed the early guard - // before a concurrent request registered can still be in flight while - // the first request has already completed and populated the map. + // Reject an id that already exists on disk (active or archived) at the + // route boundary: loadCliConfig calls process.exit(1) on a duplicate, + // which would terminate the shared ACP child and every session on its + // channel. + const runtimeOutputDir = + loadSettingsCached(workspaceCwd).merged.advanced?.runtimeOutputDir; if ( - inFlightBranchWorkspaces.has(workspaceCwd) || - activeBranchSessions.has(workspaceCwd) + await Storage.runWithRuntimeBaseDir( + runtimeOutputDir, + workspaceCwd, + () => + new SessionService(workspaceCwd).sessionExistsInAnyState( + sessionIdToCheck, + ), + ) ) { res.status(409).json({ - error: 'A branch session is already being created for this workspace', - code: 'branch_session_conflict', + error: `Session "${requestedSessionId}" already exists`, + code: 'session_id_conflict', }); return; } - inFlightBranchWorkspaces.add(workspaceCwd); - try { - await createBranch(workspaceCwd, branchName); - } catch (checkoutErr) { - // `git checkout -b` can reject AFTER git already created the ref and - // moved HEAD — a failing post-checkout hook or a timeout past the ref - // update both leave the workspace on the new branch while the command - // exits nonzero. Roll back transactionally (restore the base ref, then - // delete the partial branch) so the shared workspace is never silently - // left on the new branch; when nothing was created the rollback is a - // harmless no-op. Log the full git error but return a generic detail — - // git stderr can embed the absolute workspace path, which must not - // reach the caller in the 500 body. - daemonLog?.warn('branch checkout failed', { - error: - checkoutErr instanceof Error - ? checkoutErr.message - : String(checkoutErr), - }); - await rollbackBranchCreation( - workspaceCwd, - { name: branchName, baseBranch }, - baseCommit, - daemonLog, - ); - res.status(500).json({ - error: 'Failed to create branch', - code: 'branch_checkout_failed', + if (inFlightSessionIds.has(requestedSessionId)) { + res.status(409).json({ + error: `Session "${requestedSessionId}" creation already in progress`, + code: 'session_id_conflict', }); return; } - branchMeta = { name: branchName, baseBranch }; - branchBaseCommit = baseCommit; - sessionScope = 'thread'; + inFlightSessionIds.add(requestedSessionId); } - // ── Worktree isolation ────────────────────────────────────────── - // When `worktree` is present, create a git worktree before spawning - // and relocate the session into it immediately after. The workspace - // runtime resolution still uses the main workspace cwd; only the - // child process's effective working directory changes. + let branchMeta: { name: string; baseBranch: string } | undefined; + let branchBaseCommit: string | undefined; let worktreeMeta: | { slug: string; path: string; branch: string } | undefined; - const rawWorktree = body['worktree']; - if (rawWorktree !== undefined && rawWorktree !== null) { - if (typeof rawWorktree !== 'object' || Array.isArray(rawWorktree)) { - res.status(400).json({ - error: - '`worktree` must be an object (e.g. `{}` or `{"slug":"my-task"}`)', - code: 'invalid_worktree', - }); - return; - } - const wtReq = rawWorktree as Record; - let wtService: GitWorktreeService; - try { - wtService = new GitWorktreeService(workspaceCwd); - } catch { - res.status(500).json({ - error: 'Failed to initialize worktree service', - code: 'worktree_init_failed', - }); - return; - } - if (!(await wtService.isGitRepository())) { - res.status(400).json({ - error: 'Worktree isolation requires a git repository', - code: 'worktree_not_git_repo', - }); - return; - } - const rawSlug = wtReq['slug']; - let slug: string; - if (rawSlug === undefined || rawSlug === null) { - slug = GitWorktreeService.generateAutoSlug(); - } else if (typeof rawSlug !== 'string' || rawSlug.length === 0) { - res.status(400).json({ - error: '`worktree.slug` must be a non-empty string when provided', - code: 'worktree_invalid_slug', - }); - return; - } else { - slug = rawSlug; + + try { + // ── Branch creation ──────────────────────────────────────────── + // When `branch` is present, create and checkout a new git branch + // before spawning. The session runs in the same working directory + // but on the new branch. Mutually exclusive with `worktree`. + const rawBranch = body['branch']; + if (rawBranch !== undefined && rawBranch !== null) { + if (body['worktree'] !== undefined && body['worktree'] !== null) { + res.status(400).json({ + error: '`branch` and `worktree` are mutually exclusive', + code: 'branch_and_worktree_conflict', + }); + return; + } + if (typeof rawBranch !== 'object' || Array.isArray(rawBranch)) { + res.status(400).json({ + error: + '`branch` must be an object (e.g. `{"name":"feat/my-feature"}`)', + code: 'invalid_branch', + }); + return; + } + const branchReq = rawBranch as Record; + const branchName = branchReq['name']; + if (typeof branchName !== 'string' || branchName.length === 0) { + res.status(400).json({ + error: '`branch.name` must be a non-empty string', + code: 'branch_invalid_name', + }); + return; + } + // Validate git branch name characters and reserved names. + // Mirrors validateBranchName in GitModePopover.tsx; keep in sync. + if ( + /[^\p{L}\p{N}._/-]/u.test(branchName) || + branchName.includes('..') || + branchName.includes('//') || + branchName.startsWith('.') || + branchName.startsWith('-') || + branchName.startsWith('/') || + branchName.endsWith('/') || + branchName.endsWith('.') || + branchName.endsWith('.git') || + branchName.includes('@{') || + branchName + .split('/') + .some((c) => c.startsWith('.') || c.endsWith('.lock')) || + branchName.toUpperCase() === GIT_RESERVED_BRANCH || + Buffer.byteLength(branchName, 'utf8') > MAX_BRANCH_NAME_BYTES || + branchName + .split('/') + .some( + (c) => Buffer.byteLength(c, 'utf8') > MAX_BRANCH_COMPONENT_BYTES, + ) + ) { + res.status(400).json({ + error: `Invalid branch name: ${branchName}`, + code: 'branch_invalid_name', + }); + return; + } + // Reject when another branch session is already active for this + // workspace — concurrent branch sessions conflict on HEAD. Runs after + // shape/name validation so a malformed body gets 400, not 409. + const existingBranchSession = activeBranchSessions.get(workspaceCwd); + if (existingBranchSession) { + try { + // Throws if the session is gone, letting us clean up the stale entry. + runtime.bridge.getSessionSummary(existingBranchSession); + res.status(409).json({ + error: 'A branch session is already active for this workspace', + code: 'branch_session_conflict', + existingSessionId: existingBranchSession, + }); + return; + } catch { + activeBranchSessions.delete(workspaceCwd); + } + } + // Reject when any other live (client-attached) non-worktree session + // already runs in this workspace. `git checkout -b` moves the shared + // HEAD, so a concurrent current-branch session with a clean tree would + // be silently relocated onto the new branch and commit to the wrong + // ref. Worktree sessions are exempt (they run in their own cwd). Scoped + // to sessions with an attached client so a detached session left behind + // by a "new chat" does not block a fresh branch session. + const sharedCheckoutSession = runtime.bridge + .listWorkspaceSessions(workspaceCwd) + .find((session) => !session.worktree && session.clientCount > 0); + if (sharedCheckoutSession) { + res.status(409).json({ + error: + 'Another session is already active in this workspace; creating a branch would move its shared checkout', + code: 'branch_session_conflict', + existingSessionId: sharedCheckoutSession.sessionId, + }); + return; + } + let wtService: GitWorktreeService; + try { + wtService = new GitWorktreeService(workspaceCwd); + } catch { + res.status(500).json({ + error: 'Failed to initialize git service', + code: 'branch_init_failed', + }); + return; + } + if (!(await wtService.isGitRepository())) { + res.status(400).json({ + error: 'Branch creation requires a git repository', + code: 'branch_not_git_repo', + }); + return; + } + // Check the branch doesn't already exist. + if (await branchExists(workspaceCwd, branchName)) { + res.status(409).json({ + error: `Branch "${branchName}" already exists`, + code: 'branch_already_exists', + }); + return; + } + // Gate on a dirty tree as surprise-prevention: `git checkout -b` carries + // uncommitted tracked changes onto the new branch, which would silently + // mix the user's WIP with a fresh branch. Untracked files are excluded + // (`--untracked-files=no`) because they survive any checkout unchanged. + let dirty: boolean; + try { + dirty = await isDirtyTree(workspaceCwd); + } catch { + res.status(500).json({ + error: 'Failed to check working tree status', + code: 'branch_status_failed', + }); + return; + } + if (dirty) { + res.status(409).json({ + error: 'Uncommitted changes detected. Commit or stash first.', + code: 'branch_dirty_tree', + }); + return; + } + const baseCommit = await getHeadCommit(workspaceCwd); + const baseBranch = await wtService + .getCurrentBranch() + .catch(() => 'HEAD'); + // Reserve the workspace before mutating HEAD. The conflict guard above + // runs before several awaits (rev-parse, status, checkout), so two + // concurrent `POST /session { branch }` can both pass it and race on + // `git checkout -b`. This synchronous check-and-add (no await between) + // serializes the checkout; every exit path below clears the reservation + // (transferred to `activeBranchSessions` on success). Re-check + // `activeBranchSessions` here too: a request that passed the early guard + // before a concurrent request registered can still be in flight while + // the first request has already completed and populated the map. + if ( + inFlightBranchWorkspaces.has(workspaceCwd) || + activeBranchSessions.has(workspaceCwd) + ) { + res.status(409).json({ + error: + 'A branch session is already being created for this workspace', + code: 'branch_session_conflict', + }); + return; + } + inFlightBranchWorkspaces.add(workspaceCwd); + try { + await createBranch(workspaceCwd, branchName); + } catch (checkoutErr) { + // `git checkout -b` can reject AFTER git already created the ref and + // moved HEAD — a failing post-checkout hook or a timeout past the ref + // update both leave the workspace on the new branch while the command + // exits nonzero. Roll back transactionally (restore the base ref, then + // delete the partial branch) so the shared workspace is never silently + // left on the new branch; when nothing was created the rollback is a + // harmless no-op. Log the full git error but return a generic detail — + // git stderr can embed the absolute workspace path, which must not + // reach the caller in the 500 body. + daemonLog?.warn('branch checkout failed', { + error: + checkoutErr instanceof Error + ? checkoutErr.message + : String(checkoutErr), + }); + await rollbackBranchCreation( + workspaceCwd, + { name: branchName, baseBranch }, + baseCommit, + daemonLog, + ); + res.status(500).json({ + error: 'Failed to create branch', + code: 'branch_checkout_failed', + }); + return; + } + branchMeta = { name: branchName, baseBranch }; + branchBaseCommit = baseCommit; + sessionScope = 'thread'; } - const slugError = GitWorktreeService.validateUserWorktreeSlug(slug); - if (slugError) { - res - .status(400) - .json({ error: slugError, code: 'worktree_invalid_slug' }); - return; + + // ── Worktree isolation ────────────────────────────────────────── + // When `worktree` is present, create a git worktree before spawning + // and relocate the session into it immediately after. The workspace + // runtime resolution still uses the main workspace cwd; only the + // child process's effective working directory changes. + const rawWorktree = body['worktree']; + if (rawWorktree !== undefined && rawWorktree !== null) { + if (typeof rawWorktree !== 'object' || Array.isArray(rawWorktree)) { + res.status(400).json({ + error: + '`worktree` must be an object (e.g. `{}` or `{"slug":"my-task"}`)', + code: 'invalid_worktree', + }); + return; + } + const wtReq = rawWorktree as Record; + let wtService: GitWorktreeService; + try { + wtService = new GitWorktreeService(workspaceCwd); + } catch { + res.status(500).json({ + error: 'Failed to initialize worktree service', + code: 'worktree_init_failed', + }); + return; + } + if (!(await wtService.isGitRepository())) { + res.status(400).json({ + error: 'Worktree isolation requires a git repository', + code: 'worktree_not_git_repo', + }); + return; + } + const rawSlug = wtReq['slug']; + let slug: string; + if (rawSlug === undefined || rawSlug === null) { + slug = GitWorktreeService.generateAutoSlug(); + } else if (typeof rawSlug !== 'string' || rawSlug.length === 0) { + res.status(400).json({ + error: '`worktree.slug` must be a non-empty string when provided', + code: 'worktree_invalid_slug', + }); + return; + } else { + slug = rawSlug; + } + const slugError = GitWorktreeService.validateUserWorktreeSlug(slug); + if (slugError) { + res + .status(400) + .json({ error: slugError, code: 'worktree_invalid_slug' }); + return; + } + const baseBranch = await wtService + .getCurrentBranch() + .catch(() => undefined); + const wtResult = await wtService.createUserWorktree(slug, baseBranch); + if (!wtResult.success || !wtResult.worktree) { + res.status(500).json({ + error: wtResult.error ?? 'Failed to create worktree', + code: 'worktree_create_failed', + }); + return; + } + worktreeMeta = { + slug, + path: wtResult.worktree.path, + branch: wtResult.worktree.branch, + }; + // Worktree sessions must be independent — never coalesce onto an + // existing single-scope session that lives in the main checkout. + sessionScope = 'thread'; } - const baseBranch = await wtService - .getCurrentBranch() - .catch(() => undefined); - const wtResult = await wtService.createUserWorktree(slug, baseBranch); - if (!wtResult.success || !wtResult.worktree) { - res.status(500).json({ - error: wtResult.error ?? 'Failed to create worktree', - code: 'worktree_create_failed', - }); - return; + // A caller-supplied sessionId implies a new, distinct session — + // never coalesce onto an existing single-scope session. + if (requestedSessionId !== undefined) { + sessionScope = 'thread'; } - worktreeMeta = { - slug, - path: wtResult.worktree.path, - branch: wtResult.worktree.branch, - }; - // Worktree sessions must be independent — never coalesce onto an - // existing single-scope session that lives in the main checkout. - sessionScope = 'thread'; - } - try { const session = await runtime.bridge.spawnOrAttach({ workspaceCwd, modelServiceId, @@ -1573,7 +1694,57 @@ export function registerSessionRoutes( ...(source.sourceId !== undefined ? { sourceId: source.sourceId } : {}), ...(worktreeMeta ? { worktree: worktreeMeta } : {}), ...(branchMeta ? { branch: branchMeta } : {}), + ...(requestedSessionId !== undefined + ? { sessionId: requestedSessionId } + : {}), }); + // Defensive: the bridge/agent must honor a caller-supplied id. If it was + // dropped anywhere in the chain (older agent binary, coalesced attach), + // never return a surprise id — fail the request instead. Same silent-drop + // class as #7831, one layer down. + if ( + requestedSessionId !== undefined && + session.sessionId !== requestedSessionId + ) { + if (daemonLog) { + daemonLog.warn('session id not honored by agent', { + requested: requestedSessionId, + actual: session.sessionId, + }); + } + if (!session.attached) { + await runWithWorkspaceRuntimeStorage(runtime, () => + deleteDaemonSessionIfOrphan({ + sessionId: session.sessionId, + service: createWorkspaceRuntimeSessionService(runtime), + bridge: runtime.bridge, + coordinator: archiveCoordinator, + }), + ).catch(() => false); + } + // This early return runs inside the outer try, but a return skips + // that try's catch — so replicate the catch's resource cleanup here. + // Otherwise the branch/worktree created for this request is orphaned + // and inFlightBranchWorkspaces permanently blocks the workspace. + if (worktreeMeta) { + await new GitWorktreeService(workspaceCwd) + .removeUserWorktree(worktreeMeta.slug, { deleteBranch: true }) + .catch(() => {}); + } + if (branchMeta) { + await rollbackBranchCreation( + workspaceCwd, + branchMeta, + branchBaseCommit, + daemonLog, + ); + } + res.status(500).json({ + error: 'Agent did not honor the requested session id', + code: 'session_id_not_honored', + }); + return; + } try { runtime.generationGuard?.assertOpen(); } catch (error) { @@ -1823,6 +1994,10 @@ export function registerSessionRoutes( ); } sendBridgeError(res, err, { route: 'POST /session' }); + } finally { + if (requestedSessionId !== undefined) { + inFlightSessionIds.delete(requestedSessionId); + } } }); diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 289c78987e8..73878a47643 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -122,6 +122,7 @@ import type { ServeWorkspaceToolsStatus, } from '@qwen-code/acp-bridge/status'; import { CAPABILITIES_SCHEMA_VERSION, type ServeOptions } from './types.js'; +import { isValidSessionId } from '../config/config.js'; import type { DaemonLogger } from './daemon-logger.js'; import { FsError, type WorkspaceFileSystemFactory } from './fs/index.js'; import { getRateLimiter } from './rate-limit.js'; @@ -1102,7 +1103,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { const spawnImpl = opts.spawnImpl ?? (async (req) => ({ - sessionId: `fake-${calls.length}`, + sessionId: req.sessionId ?? `fake-${calls.length}`, workspaceCwd: req.workspaceCwd, attached: false, clientId: `client-${calls.length}`, @@ -8596,6 +8597,307 @@ describe('createServeApp', () => { expect(bridge.calls).toHaveLength(0); }); + it('forwards a valid UUID sessionId to the bridge', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(200); + expect(bridge.calls[0]).toMatchObject({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + sessionScope: 'thread', + }); + }); + + it('normalizes uppercase sessionId to lowercase', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550E8400-E29B-41D4-A716-446655440000' }); + + expect(res.status).toBe(200); + expect(bridge.calls[0]).toMatchObject({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }); + }); + + it('500 when the bridge does not honor the requested sessionId', async () => { + const bridge = fakeBridge({ + spawnImpl: async (req) => ({ + sessionId: 'a-different-id-than-requested', + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-x', + }), + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(500); + expect(res.body.code).toBe('session_id_not_honored'); + // The orphaned spawn (the id the agent returned instead of the + // requested one) must be reaped via killSession with requireZeroAttaches. + expect(bridge.killCalls).toEqual([ + { + sessionId: 'a-different-id-than-requested', + opts: { requireZeroAttaches: true }, + }, + ]); + }); + + it('409 when sessionId already exists (active or archived)', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockResolvedValue('active'); + try { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('session_id_conflict'); + expect(bridge.calls).toHaveLength(0); + } finally { + locationSpy.mockRestore(); + } + }); + + it('runs the sessionId existence check inside runWithRuntimeBaseDir', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockResolvedValue('active'); + const runWithSpy = vi.spyOn(Storage, 'runWithRuntimeBaseDir'); + try { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('session_id_conflict'); + expect(runWithSpy).toHaveBeenCalled(); + expect(bridge.calls).toHaveLength(0); + } finally { + locationSpy.mockRestore(); + runWithSpy.mockRestore(); + } + }); + + it('409 when concurrent request has same sessionId in flight', async () => { + let resolveFirstSpawn!: (v: BridgeSession) => void; + const firstSpawnBarrier = new Promise((r) => { + resolveFirstSpawn = r; + }); + let spawnCallCount = 0; + const bridge = fakeBridge({ + spawnImpl: (req) => { + spawnCallCount++; + if (spawnCallCount === 1) { + // First call hangs until released; resolve after a short delay + // so the overall test does not deadlock. + setTimeout( + () => + resolveFirstSpawn({ + sessionId: req.sessionId ?? 'fake-0', + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-0', + }), + 100, + ); + return firstSpawnBarrier; + } + return Promise.resolve({ + sessionId: req.sessionId ?? 'fake-dup', + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-dup', + }); + }, + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const sid = '550e8400-e29b-41d4-a716-446655440000'; + const makeReq = () => + request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: sid }); + // Fire both concurrently; the 20 ms gap ensures the first reaches + // spawnOrAttach (and registers in inFlightSessionIds) before the + // second arrives at the guard. + const [firstRes, secondRes] = await Promise.all([ + makeReq(), + new Promise((r) => setTimeout(r, 20)).then(() => makeReq()), + ]); + const statuses = [firstRes.status, secondRes.status].sort(); + expect(statuses).toEqual([200, 409]); + const conflict = firstRes.status === 409 ? firstRes : secondRes; + expect(conflict.body.code).toBe('session_id_conflict'); + // Only the first request should have reached spawnOrAttach. + expect(spawnCallCount).toBe(1); + }); + + it('releases inFlightSessionIds after a spawn failure (finally cleanup)', async () => { + let spawnCallCount = 0; + const bridge = fakeBridge({ + spawnImpl: (req) => { + spawnCallCount++; + if (spawnCallCount === 1) { + return Promise.reject(new Error('spawn boom')); + } + return Promise.resolve({ + sessionId: req.sessionId ?? 'fake-retry', + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-retry', + }); + }, + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const sid = '550e8400-e29b-41d4-a716-446655440000'; + const makeReq = () => + request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: sid }); + const firstRes = await makeReq(); + expect(firstRes.status).toBe(500); + const secondRes = await makeReq(); + expect(secondRes.status).toBe(200); + expect(spawnCallCount).toBe(2); + }); + + it('409 when the sessionId is live on the daemon but not yet on disk', async () => { + // A session's transcript JSONL is only written on its first message, so + // a created-but-never-prompted session is invisible to the disk check. + // Without the bridge liveness check the request falls through to the + // agent's own guard and surfaces as an opaque 500 / -32603. + const bridge = fakeBridge({ + summaryImpl: (sessionId) => ({ + sessionId, + workspaceCwd: WS_BOUND, + createdAt: '2026-05-17T12:00:00.000Z', + clientCount: 1, + hasActivePrompt: false, + }), + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + // Disk lookup explicitly reports "absent" so the 409 can only come from + // the liveness check. + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockResolvedValue(undefined); + try { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(409); + expect(res.body.code).toBe('session_id_conflict'); + expect(bridge.calls).toHaveLength(0); + } finally { + locationSpy.mockRestore(); + } + }); + + it('400 for UUID shapes the CLI isValidSessionId rejects', async () => { + // The HTTP gate must stay a strict subset of config.ts's + // isValidSessionId — an id the daemon accepts but the CLI rejects + // yields a session `/resume ` can never address. + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const bad = [ + '01930000-0000-7000-a000-000000000001', // UUIDv7 (version nibble 7) + '00000000-0000-0000-0000-000000000000', // nil UUID (version nibble 0) + '550e8400-e29b-41d4-c716-446655440000', // variant nibble c + ]; + for (const sessionId of bad) { + expect(isValidSessionId(sessionId)).toBe(false); + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_session_id'); + } + expect(bridge.calls).toHaveLength(0); + }); + + it('400 when sessionId is not a UUID (path traversal guard)', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const bad = [ + '../../etc/cron.d/evil', + 'my-feature-branch', + 123, + '', + '550e8400-e29b-41d4-a716-446655440000-agent-foo', + ]; + for (const sessionId of bad) { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_session_id'); + } + expect(bridge.calls).toHaveLength(0); + }); + it('400 when cwd is relative', async () => { const bridge = fakeBridge(); const app = createServeApp(