diff --git a/docs/design/session-writer-lease.md b/docs/design/session-writer-lease.md new file mode 100644 index 00000000000..1be818a1253 --- /dev/null +++ b/docs/design/session-writer-lease.md @@ -0,0 +1,91 @@ +# Session writer lease and consistent recovery + +## Problem + +Session transcripts are append-only JSONL trees. Before this change, write +serialization was process-local: two processes could restore the same physical +tail, then append different children. Both writes succeeded, but the next +restore could reconstruct only one branch. A response already shown to the user +could therefore disappear after restart. + +## Invariants + +1. At most one process owns write access to a `(runtime base, session id)`. +2. A writer reloads the transcript after acquiring ownership; preloaded session + data is only a UI preview. +3. Every append verifies both ownership and the expected physical byte length. +4. The runtime base and session id used by a recorder are immutable. Its + transcript path changes only during an explicit lease-held workspace + migration. +5. A session transition keeps the old binding recoverable until the new Core + and UI state are committed. +6. Offline mutations acquire the same lease and fail closed on live sessions. + +## Lease + +The lock lives at +`/tmp/session-writer-locks/.lock`, independent of the +project directory and archive state. It is created atomically with `wx` and +mode `0600`. Its random `owner_id` is the fencing token. + +A live local PID with the same process start time and every foreign-host owner +cause a conflict. A dead or reused local PID is moved aside atomically, re-read, +and reclaimed only after the moved record is proved stale. A malformed record +is retried to tolerate an in-progress write; young or otherwise unverifiable +records fail closed. Runtime sidecars apply the same start-time check before a +reused PID can keep a malformed lock alive. +Release removes the lock only when its on-disk `owner_id` still matches. + +Each lease tracks the transcript's expected UTF-8 byte length. Appends validate +the current lock and file length, append one buffered JSON line with flush, and +advance the expected length only after success. Ownership loss and unexpected +length changes are integrity failures, distinct from ordinary recording I/O +degradation. + +## Lifecycle and recovery + +`Config.initialize()` activates recording before model initialization. It +acquires the lease, captures the byte length, reloads the complete JSONL under +the lease, verifies that the length did not change, and rebases the recorder on +that authoritative history. Initialization and target-Core construction run in +the target session's async context. Inputs and owner-bearing runtime sidecars +are enabled only after activation; read-only Configs do not publish sidecars. + +Recorders move through inactive, active, paused, closed, or integrity-failed +states. Pausing synchronously refuses new records and cancels auto-title work, +then drains the existing write chain before ownership can be released. +Integrity failure rejects subsequent model turns while leaving recovery +commands available. + +Session transitions are prepare/commit/rollback operations. Preparation pauses +the old recorder without releasing it, acquires and loads the target, and +constructs target state. Commit changes Core and UI state together and releases +the old owner afterward. Any pre-commit error restores the exact old recorder, +client, and session state; an uncertain rollback closes both sides. Destructive +cleanup of old background registries happens only after commit, so a UI commit +failure cannot erase state that rollback must preserve. Resume rechecks the +background-work gate after its asynchronous preview load, and branch requires +an idle, healthy source. + +## Maintenance operations + +Offline rename, remove, archive, unarchive, and fork acquire temporary +maintenance leases. A live source fork uses the paused recorder's stable +snapshot; an offline fork reads its leased source once. Project relocation +keeps the global lease and uses same-filesystem atomic rename only. + +Runtime status sidecars include an optional owner token. Live-session cleanup +removes a sidecar only when that token still matches; offline maintenance +cleanup is instead protected by the session writer lease. Read-only or +auxiliary Config instances explicitly disable chat recording. ACP runs the +complete Config load/initialize sequence inside the selected runtime-output +context and pins that resolved root on the live Session. ACP's textual +`/clear` is not advertised because the protocol cannot atomically commit a +changed session id; clients close and create a session explicitly instead. + +## Compatibility and rollout + +Old binaries do not honor this lock. Deployments and rollbacks must drain all +old daemons and interactive processes before changing binaries. There is no +feature flag and no automatic repair or branch selection for already-diverged +transcripts. diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index d61501b2b81..b866fc703e6 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -12674,7 +12674,7 @@ describe('createAcpSessionBridge', () => { describe('closeSession', () => { it('publishes session_closed and removes session from maps', async () => { - const handles: Array<{ killed: boolean }> = []; + const handles: ChannelHandle[] = []; const factory: ChannelFactory = async () => { const h = makeChannel(); handles.push(h); @@ -12694,6 +12694,10 @@ describe('createAcpSessionBridge', () => { await bridge.closeSession(session.sessionId); await drain; + expect(handles[0]?.agent.extMethodCalls).toContainEqual({ + method: SERVE_CONTROL_EXT_METHODS.sessionClose, + params: { sessionId: session.sessionId }, + }); expect(bridge.sessionCount).toBe(0); const closedEvent = events.find((e) => e.type === 'session_closed'); expect(closedEvent).toBeDefined(); @@ -12704,6 +12708,184 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('preserves bridge state when a normal agent close fails', async () => { + let failClose = true; + const handle = makeChannel({ + extMethodImpl: (method) => { + if (method === SERVE_CONTROL_EXT_METHODS.sessionClose && failClose) { + failClose = false; + throw new Error('release failed'); + } + return {}; + }, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + await expect(bridge.closeSession(session.sessionId)).rejects.toThrow(); + expect(bridge.sessionCount).toBe(1); + + const reattached = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + expect(reattached.sessionId).toBe(session.sessionId); + expect(handle.agent.newSessionCalls).toHaveLength(1); + + await bridge.closeSession(session.sessionId); + expect(bridge.sessionCount).toBe(0); + expect(handle.agent.extMethodCalls).toEqual([ + { + method: SERVE_CONTROL_EXT_METHODS.sessionClose, + params: { sessionId: session.sessionId }, + }, + { + method: SERVE_CONTROL_EXT_METHODS.sessionClose, + params: { sessionId: session.sessionId }, + }, + ]); + + await bridge.shutdown(); + }); + + it('rejects attaches while the child writer is closing', async () => { + const closeGate = deferred(); + const handle = makeChannel({ + extMethodImpl: (method) => + method === SERVE_CONTROL_EXT_METHODS.sessionClose + ? closeGate.promise + : {}, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const closing = bridge.closeSession(session.sessionId); + await vi.waitFor(() => { + expect(handle.agent.extMethodCalls).toHaveLength(1); + }); + await expect( + bridge.spawnOrAttach({ workspaceCwd: WS_A }), + ).rejects.toBeInstanceOf(SessionBusyError); + + closeGate.resolve({}); + await closing; + expect(bridge.sessionCount).toBe(0); + await bridge.shutdown(); + }); + + it('remains fail-closed when the child writer close times out', async () => { + const closeGate = deferred(); + const handle = makeChannel({ + extMethodImpl: (method) => + method === SERVE_CONTROL_EXT_METHODS.sessionClose + ? closeGate.promise + : {}, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + initializeTimeoutMs: 10, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + await expect( + bridge.closeSession(session.sessionId), + ).rejects.toBeInstanceOf(BridgeTimeoutError); + expect(bridge.sessionCount).toBe(1); + await expect( + bridge.spawnOrAttach({ workspaceCwd: WS_A }), + ).rejects.toBeInstanceOf(SessionBusyError); + expect(() => + bridge.sendPrompt(session.sessionId, { + sessionId: session.sessionId, + prompt: [{ type: 'text', text: 'must stay blocked' }], + }), + ).toThrow(SessionBusyError); + await expect( + bridge.closeSession(session.sessionId), + ).rejects.toBeInstanceOf(SessionBusyError); + + closeGate.resolve({}); + await bridge.shutdown(); + }); + + it('cancels an active prompt before closing the child writer and rejects new prompts', async () => { + let rejectPrompt: ((error: Error) => void) | undefined; + const closeGate = deferred(); + const handle = makeChannel({ + promptImpl: () => + new Promise((_resolve, reject) => { + rejectPrompt = reject; + }), + cancelImpl: (_params, self) => { + rejectPrompt?.(new Error('cancelled')); + expect(self.cancelCalls).toHaveLength(1); + }, + extMethodImpl: (method, params, self) => { + if (method !== SERVE_CONTROL_EXT_METHODS.sessionClose) return {}; + expect(self.cancelCalls).toContainEqual({ + sessionId: params['sessionId'], + }); + return closeGate.promise as Promise>; + }, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const activePrompt = bridge + .sendPrompt(session.sessionId, { + sessionId: session.sessionId, + prompt: [{ type: 'text', text: 'long turn' }], + }) + .catch(() => undefined); + await vi.waitFor(() => { + expect(handle.agent.promptCalls).toHaveLength(1); + }); + + const closing = bridge.closeSession(session.sessionId); + await vi.waitFor(() => { + expect(handle.agent.extMethodCalls).toHaveLength(1); + }); + expect(() => + bridge.sendPrompt(session.sessionId, { + sessionId: session.sessionId, + prompt: [{ type: 'text', text: 'must not queue' }], + }), + ).toThrow(SessionBusyError); + expect(handle.agent.promptCalls).toHaveLength(1); + + closeGate.resolve({}); + await closing; + await activePrompt; + expect(bridge.sessionCount).toBe(0); + await bridge.shutdown(); + }); + + it('preserves bridge state when kill cannot release the child writer', async () => { + let failClose = true; + const handle = makeChannel({ + extMethodImpl: (method) => { + if (method === SERVE_CONTROL_EXT_METHODS.sessionClose && failClose) { + failClose = false; + throw new Error('release failed'); + } + return {}; + }, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + await expect(bridge.killSession(session.sessionId)).rejects.toThrow(); + expect(bridge.sessionCount).toBe(1); + + await expect(bridge.killSession(session.sessionId)).resolves.toBe(true); + expect(bridge.sessionCount).toBe(0); + await bridge.shutdown(); + }); + it('throws SessionNotFoundError for unknown session', async () => { const bridge = makeBridge(); await expect(bridge.closeSession('nonexistent')).rejects.toThrow( diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index a0d0133e786..952def70307 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -558,6 +558,8 @@ interface SessionEntry { * an originator clientId is known. Used by the session reaper to avoid * killing sessions mid-prompt. */ promptActive: boolean; + /** True while the ACP child is releasing this session's writer. */ + closing?: boolean; /** Terminal error from the prior turn, cleared when the next turn starts. */ turnError?: { message: string; @@ -2881,6 +2883,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { entry: SessionEntry, ): void => { const info = channelInfoForEntry(entry); + if (entry.closing) { + throw new SessionBusyError(sessionId, `Session ${sessionId} is closing`); + } if (byId.get(sessionId) !== entry || !info || info.isDying) { throw new SessionNotFoundError(sessionId); } @@ -3807,6 +3812,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const existing = byId.get(req.sessionId); if (existing) { + if (existing.closing) { + throw new SessionBusyError( + req.sessionId, + `Session ${req.sessionId} is closing`, + ); + } existing.attachCount++; const clientId = registerClient(existing, req.clientId); if (req.approvalMode) { @@ -3873,6 +3884,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { 'the agent child likely crashed during session restore — retry to restore the session', ); } + if (entry.closing) { + entry.attachCount = Math.max(0, entry.attachCount - 1); + throw new SessionBusyError( + entry.sessionId, + `Session ${entry.sessionId} is closing`, + ); + } // NOTE: do NOT bump entry.attachCount here — `createSessionEntry` // already initialized it from coalesceState.count synchronously // when the IIFE registered the entry. Spread `restored` so the @@ -4037,6 +4055,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const racedEntry = byId.get(req.sessionId); if (racedEntry) { restoreEvents.close(); + if (racedEntry.closing) { + throw new SessionBusyError( + req.sessionId, + `Session ${req.sessionId} is closing`, + ); + } // Self + any coalescers we accumulated while the restore was // in flight. Coalescers must not bump attachCount themselves // (they read it off the registered entry on the next tick). @@ -4230,6 +4254,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ): Promise { const entry = byId.get(sessionId); if (!entry) throw new SessionNotFoundError(sessionId); + if (entry.closing) { + throw new SessionBusyError(sessionId, `Session ${sessionId} is closing`); + } let originatorClientId: string | undefined; if (context?.clientId !== undefined) { originatorClientId = resolveTrustedClientId(entry, context.clientId); @@ -4242,12 +4269,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ? ` by client ${JSON.stringify(originatorClientId)}` : ''), ); - telemetry.event('session.close', { - 'qwen-code.daemon.bridge.operation': 'session.close', - 'session.id': sessionId, - 'session.close.reason': reason, - }); - if (defaultEntry === entry) defaultEntry = undefined; + entry.closing = true; // HAZARD: Resolve the channel via `channelInfoForEntry(entry)` (search // `aliveChannels` for the entry's actual channel) instead of the // module-scoped `channelInfo` (the CURRENT attach target). The two @@ -4267,19 +4289,40 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ); } const requireAgentClose = closeOpts?.requireAgentClose === true; - if (requireAgentClose) { + try { + try { + await entry.connection.cancel({ sessionId }); + } catch { + // No active prompt, or it already settled. The child close below is + // still authoritative for draining and releasing the recorder. + } await notifyAgentSessionClose(entry, ci, 'closeSession', { throwOnFailure: true, - requireFlush: true, + requireFlush: requireAgentClose, }); + } catch (error) { + // `withTimeout` does not cancel the in-flight close RPC. A timeout + // therefore leaves ownership uncertain: the child may still release its + // writer after this call rejects. Keep the bridge entry fail-closed so a + // new prompt or attach cannot overlap that late completion. A definite + // child rejection proves the writer was retained and remains retryable. + if (!(error instanceof BridgeTimeoutError)) { + entry.closing = false; + } + throw error; } + telemetry.event('session.close', { + 'qwen-code.daemon.bridge.operation': 'session.close', + 'session.id': sessionId, + 'session.close.reason': reason, + }); + if (defaultEntry === entry) defaultEntry = undefined; if (ci && ci.channel === entry.channel) { ci.sessionIds.delete(sessionId); } - // For normal close, tombstone + event publish + bus close run before the - // best-effort agent notification. Strict archive close is different: the - // agent flush must succeed before bridge state is removed, so a failed - // archive close can be retried against the same live session. + // The agent must release its recorder before bridge state is removed so a + // failed close can be retried against the same live session. Archive/delete + // additionally require a healthy flush before the release above. permissionMediator.forgetSession(sessionId); entry.pendingPermissionIds.clear(); entry.pendingInteractions.clear(); @@ -4320,22 +4363,6 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // `session_closed` is terminal. Close the bus before ACP cancel so any // late cancellation frames from the agent are intentionally dropped. entry.events.close(); - if (!requireAgentClose) { - await notifyAgentSessionClose(entry, ci, 'closeSession'); - } - try { - await telemetry.withSpan( - 'session.close.cancel_active_prompt', - { - 'qwen-code.daemon.bridge.operation': - 'session.close.cancel_active_prompt', - 'session.id': sessionId, - }, - async () => await entry.connection.cancel({ sessionId }), - ); - } catch { - /* no active prompt or session already torn down */ - } if (ci && hasNoChannelWork(ci)) { await reapPendingEmptyChannel(ci); if (!ci.isDying) { @@ -4497,6 +4524,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { if (effectiveScope === 'single') { const existing = defaultEntry; if (existing) { + if (existing.closing) { + throw new SessionBusyError( + existing.sessionId, + `Session ${existing.sessionId} is closing`, + ); + } // BRSCi: bump attach counter BEFORE any await so the // spawn-owner's disconnect reaper (server.ts: // `requireZeroAttaches: true`) sees this attach even when @@ -4593,6 +4626,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { 'the agent child likely crashed during initialization — retry to spawn a new session', ); } + if (attachedEntry.closing) { + throw new SessionBusyError( + attachedEntry.sessionId, + `Session ${attachedEntry.sessionId} is closing`, + ); + } const clientId = registerClient(attachedEntry, req.clientId); if (req.modelServiceId) { // Same swallow as above — we picked up an in-flight @@ -4692,6 +4731,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const queuedAt = Date.now(); const entry = byId.get(sessionId); if (!entry) return Promise.reject(new SessionNotFoundError(sessionId)); + if (entry.closing) { + throw new SessionBusyError( + sessionId, + `Session ${sessionId} is closing`, + ); + } const originatorClientId = resolveTrustedClientId( entry, context?.clientId, @@ -7468,6 +7513,36 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { entry.spawnOwnerWantedKill = true; return false; } + if (entry.closing) { + throw new SessionBusyError( + sessionId, + `Session ${sessionId} is closing`, + ); + } + entry.closing = true; + const ci = channelInfoForEntry(entry); + if (!ci) { + writeStderrLine( + `qwen serve: killSession channelInfoForEntry returned undefined ` + + `for session ${JSON.stringify(sessionId)} — channel cleanup skipped (entry's channel already torn down)`, + ); + } + try { + try { + await entry.connection.cancel({ sessionId }); + } catch { + // No active prompt, or it already settled. The child close below is + // still authoritative for draining and releasing the recorder. + } + await notifyAgentSessionClose(entry, ci, 'killSession', { + throwOnFailure: true, + }); + } catch (error) { + if (!(error instanceof BridgeTimeoutError)) { + entry.closing = false; + } + throw error; + } // Mediator-driven cancel cascade. Must run BEFORE byId.delete so // the mediator's emit callback can still reach entry.events via // byId.get(sessionId) (same order as closeSession). @@ -7494,28 +7569,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // session leaves — other sessions on the same channel keep // running. // - // HAZARD: Same channel-overlap fix as in `closeSession` above. - // `channelInfoForEntry(entry)` returns the entry's actual - // channel rather than the module-scoped `channelInfo` (current - // attach target), preventing the "kill operates on the freshly- - // spawned channel B instead of the dying channel A" cascade - // during the overlap window. The regression test is single-channel - // smoke only and WILL NOT fail if this reverts to module-scoped - // channelInfo. Keep `channelInfoForEntry(entry)` until a - // deterministic overlap test lands. - const ci = channelInfoForEntry(entry); - if (!ci) { - // Same diagnostic as `closeSession` — when the entry's channel - // is already gone, the cleanup below short-circuits silently. - writeStderrLine( - `qwen serve: killSession channelInfoForEntry returned undefined ` + - `for session ${JSON.stringify(sessionId)} — channel cleanup skipped (entry's channel already torn down)`, - ); - } if (ci && ci.channel === entry.channel) { ci.sessionIds.delete(sessionId); } - await notifyAgentSessionClose(entry, ci, 'killSession'); // Tombstone the killed sessionId so any in-flight // `extNotification` from the (about-to-be-killed) child can't // seed the early-event buffer for a subsequent load/resume of diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 436bbf56be0..98da79e6683 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -433,7 +433,7 @@ export interface SessionMetadataUpdate { export interface CloseSessionOpts { /** Override the default `'client_close'` reason in the `session_closed` event. */ reason?: string; - /** Require the ACP child to acknowledge session close before resolving. */ + /** Require the ACP child to flush a healthy transcript before closing. */ requireAgentClose?: boolean; } diff --git a/packages/acp-bridge/src/status.test.ts b/packages/acp-bridge/src/status.test.ts index 8d61d0ff5bb..f4f3d1eb032 100644 --- a/packages/acp-bridge/src/status.test.ts +++ b/packages/acp-bridge/src/status.test.ts @@ -22,7 +22,8 @@ describe('SERVE_ERROR_KINDS', () => { // non-ENOENT stat failures on workspace memory discovery (see // #4175 PR 16). Issue #4514 T2.8 added three runtime-mutation // error kinds; T2.9 appended prompt_deadline_exceeded and - // writer_idle_timeout. Future additions append to this list. + // writer_idle_timeout. Session writer fencing appended its four + // integrity classifications. Future additions append to this list. expect(SERVE_ERROR_KINDS).toEqual([ 'missing_binary', 'blocked_egress', @@ -38,6 +39,10 @@ describe('SERVE_ERROR_KINDS', () => { 'invalid_config', 'prompt_deadline_exceeded', 'writer_idle_timeout', + 'session_writer_conflict', + 'session_writer_lost', + 'session_transcript_changed', + 'session_writer_unavailable', ]); }); @@ -91,6 +96,14 @@ describe('MissingCliEntryError', () => { }); describe('mapDomainErrorToErrorKind', () => { + it('classifies session writer errors nested in bridge RPC data', () => { + expect( + mapDomainErrorToErrorKind({ + data: { errorKind: 'session_writer_conflict' }, + }), + ).toBe('session_writer_conflict'); + }); + it('classifies BridgeTimeoutError as init_timeout', () => { expect(mapDomainErrorToErrorKind(new BridgeTimeoutError('init', 100))).toBe( 'init_timeout', diff --git a/packages/acp-bridge/src/status.ts b/packages/acp-bridge/src/status.ts index 214dfb2b673..e3053e29f66 100644 --- a/packages/acp-bridge/src/status.ts +++ b/packages/acp-bridge/src/status.ts @@ -36,6 +36,10 @@ export const SERVE_ERROR_KINDS = [ // Prompt deadline + writer idle timeout 'prompt_deadline_exceeded', 'writer_idle_timeout', + 'session_writer_conflict', + 'session_writer_lost', + 'session_transcript_changed', + 'session_writer_unavailable', ] as const; export type ServeErrorKind = (typeof SERVE_ERROR_KINDS)[number]; @@ -1376,6 +1380,21 @@ export function mapDomainErrorToErrorKind( if (err instanceof BridgeTimeoutError) return 'init_timeout'; if (err instanceof BridgeChannelClosedError) return 'protocol_error'; if (err instanceof MissingCliEntryError) return 'missing_binary'; + if (err && typeof err === 'object') { + const candidate = err as { + errorKind?: unknown; + data?: { errorKind?: unknown }; + }; + const errorKind = candidate.errorKind ?? candidate.data?.errorKind; + if ( + errorKind === 'session_writer_conflict' || + errorKind === 'session_writer_lost' || + errorKind === 'session_transcript_changed' || + errorKind === 'session_writer_unavailable' + ) { + return errorKind; + } + } // `SkillError` is defined in `@qwen-code/qwen-code-core/skills`; same // cross-package bundling concern as `TrustGateError` below — when this // function is consumed from outside the monorepo (or under a bundler diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index ebe01b4ee40..dcf0ae09d51 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -412,6 +412,9 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ registerGoalHook: vi.fn(), setGoalTerminalObserver: vi.fn(), setLastGoalTerminal: vi.fn(), + computeUniqueBranchTitle: vi.fn( + async (baseName: string) => `${baseName} (Branch)`, + ), uiTelemetryService: { removeSession: vi.fn(), }, @@ -497,6 +500,10 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ getGlobalQwenDir: vi.fn(() => '/tmp/qwen-global-test'), getGlobalTempDir: vi.fn(() => '/tmp/qwen-global-temp'), getUserExtensionsDir: vi.fn(() => '/tmp/qwen-extensions'), + getRuntimeBaseDir: vi.fn(() => '/tmp/qwen-runtime'), + runWithRuntimeBaseDir: vi.fn( + (_dir: string, _cwd: string | undefined, fn: () => T): T => fn(), + ), }, parseRule: vi.fn((raw: string) => { const trimmed = raw.trim(); @@ -751,6 +758,7 @@ import { createWorkspaceMcpBudget, deliverClientMcpMessage, } from './acpAgent.js'; +import { runWithAcpRuntimeOutputDir } from './runtimeOutputDirContext.js'; import { gzipSync } from 'node:zlib'; import type { Config } from '@qwen-code/qwen-code-core'; import type { LoadedSettings } from '../config/settings.js'; @@ -1393,6 +1401,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { getRewindableUserTurnCount: ReturnType; clearTodoStopGuardTrust: ReturnType; releaseTodoStopGuardQueuedPromptWait: ReturnType; + withExclusiveMaintenance: ReturnType; } | undefined; let processExitSpy: MockInstance; @@ -1660,6 +1669,57 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('preserves an active session when another config resolves to the same id', async () => { + const firstConfig = { + ...makeInnerConfig(), + shutdown: vi.fn().mockResolvedValue(undefined), + }; + const collidingConfig = { + ...makeInnerConfig(), + shutdown: vi.fn().mockResolvedValue(undefined), + }; + vi.mocked(loadCliConfig) + .mockResolvedValueOnce(firstConfig as unknown as Config) + .mockResolvedValueOnce(collidingConfig as unknown as Config); + const firstSession = { dispose: vi.fn() }; + vi.mocked(Session).mockImplementation( + () => + ({ + getId: vi.fn().mockReturnValue('test-session-id'), + sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), + replayHistory: vi.fn().mockResolvedValue(undefined), + installRewriter: vi.fn(), + startCronScheduler: vi.fn(), + dispose: firstSession.dispose, + getConfig: vi.fn().mockReturnValue(firstConfig), + }) as unknown as InstanceType, + ); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/workspace-a', mcpServers: [] }); + await expect( + agent.newSession({ cwd: '/workspace-b', mcpServers: [] }), + ).rejects.toThrow('Session test-session-id is already active.'); + + expect(firstSession.dispose).not.toHaveBeenCalled(); + expect(firstConfig.shutdown).not.toHaveBeenCalled(); + expect(collidingConfig.shutdown).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('profiles newSession stages under the daemon trace context', async () => { const parentContext = { trace: 'parent' }; mockExtractDaemonTraceContext.mockReturnValue(parentContext); @@ -1843,6 +1903,53 @@ describe('QwenAgent MCP SSE/HTTP support', () => { } }); + it('keeps cleanup failure primary and preserves the newSession failure as its cause', async () => { + const fileSystemError = new Error('file system setup failed'); + const cleanupError = new Error('cleanup failed'); + vi.mocked(AcpFileSystemService).mockImplementationOnce(() => { + throw fileSystemError; + }); + const innerConfig = { + ...makeInnerConfig(), + shutdown: vi.fn().mockRejectedValue(cleanupError), + storage: { + getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'), + getProjectDir: vi.fn().mockReturnValue('/tmp'), + getUserSkillsDirs: vi.fn().mockReturnValue([]), + }, + }; + vi.mocked(loadCliConfig).mockResolvedValue( + innerConfig as unknown as Config, + ); + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + await agent.initialize({ + clientCapabilities: { + fs: { readTextFile: true, writeTextFile: true }, + }, + }); + + try { + await expect( + agent.newSession({ cwd: '/tmp', mcpServers: [] }), + ).rejects.toBe(cleanupError); + expect(cleanupError.cause).toBe(fileSystemError); + expect(Object.keys(cleanupError)).not.toContain('cause'); + } finally { + mockConnectionState.resolve(); + await agentPromise; + } + }); + it('does not return discontinued qwen-oauth as the only ACP auth option', async () => { vi.mocked(buildAuthMethods).mockReturnValue([ { @@ -1968,11 +2075,23 @@ describe('QwenAgent MCP SSE/HTTP support', () => { getFileSystemService: vi.fn().mockReturnValue(undefined), getChatRecordingService: vi.fn().mockReturnValue({ flush: vi.fn().mockResolvedValue(undefined), + markIntegrityFailure: vi + .fn() + .mockReturnValue(new Error('Session write state is uncertain')), + }), + getFileHistoryService: vi.fn().mockReturnValue({ + getSnapshots: vi.fn().mockReturnValue([]), + restoreFromSnapshots: vi.fn(), + rewind: vi.fn().mockResolvedValue({ + filesChanged: [], + filesFailed: [], + }), }), setFileSystemService: vi.fn(), getHookSystem: vi.fn().mockReturnValue(undefined), getDisableAllHooks: vi.fn().mockReturnValue(true), hasHooksForEvent: vi.fn().mockReturnValue(false), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), }; } @@ -2193,16 +2312,21 @@ describe('QwenAgent MCP SSE/HTTP support', () => { } async function setupSessionMocks(sessionId: string) { - const innerConfig = makeInnerConfig(); + const innerConfig = makeInnerConfig() as ReturnType< + typeof makeInnerConfig + > & { + getSessionService: ReturnType; + }; innerConfig.getSessionId = vi.fn().mockReturnValue(sessionId); + innerConfig.getSessionService = vi.fn(() => new SessionService('/tmp')); vi.mocked(loadSettings).mockReturnValue(makeSessionSettings()); vi.mocked(loadCliConfig).mockResolvedValue( innerConfig as unknown as Config, ); - vi.mocked(Session).mockImplementation(() => { + vi.mocked(Session).mockImplementation((id, config) => { const sessionMock = { - getId: vi.fn().mockReturnValue(sessionId), - getConfig: vi.fn().mockReturnValue(innerConfig), + getId: vi.fn().mockReturnValue(id), + getConfig: vi.fn().mockReturnValue(config), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), @@ -2220,6 +2344,9 @@ describe('QwenAgent MCP SSE/HTTP support', () => { getRewindableUserTurnCount: vi.fn().mockReturnValue(1), clearTodoStopGuardTrust: vi.fn(), releaseTodoStopGuardQueuedPromptWait: vi.fn().mockReturnValue(true), + withExclusiveMaintenance: vi.fn( + async (operation: () => Promise) => await operation(), + ), }; lastSessionMock = sessionMock; return sessionMock as unknown as InstanceType; @@ -2663,6 +2790,36 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('records user text elements through the live recorder under the maintenance gate', async () => { + const sessionId = 'session-A'; + const recording = { + flush: vi.fn().mockResolvedValue(undefined), + recordUserTextElements: vi.fn().mockResolvedValue(undefined), + }; + const innerConfig = await setupSessionMocks(sessionId); + innerConfig.getChatRecordingService = vi.fn().mockReturnValue(recording); + const { agent, agentPromise } = await bootAcpAgent(); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod('qwen/session/recordTextElements', { + sessionId, + content: '@skill', + textElements: [{ type: 'skill' }], + }), + ).resolves.toEqual({ sessionId, persisted: true }); + + expect(lastSessionMock?.withExclusiveMaintenance).toHaveBeenCalledTimes(1); + expect(innerConfig.assertCanStartTurn).toHaveBeenCalledTimes(1); + expect(recording.recordUserTextElements).toHaveBeenCalledWith({ + content: '@skill', + textElements: [{ type: 'skill' }], + }); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('sessionArtifactsPersist rejects malformed event and snapshot payloads', async () => { const sessionId = 'session-A'; const recording = { @@ -7354,7 +7511,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }; const innerConfig = await setupSessionMocks('test-session-id'); ( - innerConfig as ReturnType & { + innerConfig as unknown as ReturnType & { getPermissionManager: () => typeof permissionManager; } ).getPermissionManager = vi.fn(() => permissionManager); @@ -9337,16 +9494,21 @@ describe('QwenAgent MCP SSE/HTTP support', () => { innerConfig.hasHooksForEvent = vi.fn().mockReturnValue(true); innerConfig.getModel = vi.fn().mockReturnValue('test-model'); innerConfig.getApprovalMode = vi.fn().mockReturnValue('default'); - innerConfig.getGeminiClient = vi - .fn() - .mockReturnValueOnce({ - isInitialized: vi.fn().mockReturnValue(false), - initialize, - }) - .mockReturnValueOnce({ + innerConfig.getGeminiClient = vi.fn().mockReturnValue({ + isInitialized: vi.fn().mockReturnValue(false), + initialize, + }); + const followupConfig = { + ...innerConfig, + getSessionId: vi.fn().mockReturnValue('session-followup-session-start-2'), + getGeminiClient: vi.fn().mockReturnValue({ isInitialized: vi.fn().mockReturnValue(true), initialize, - }); + }), + } as unknown as Config; + vi.mocked(loadCliConfig) + .mockResolvedValueOnce(innerConfig as unknown as Config) + .mockResolvedValueOnce(followupConfig); const agentPromise = runAcpAgent( mockConfig, @@ -9516,13 +9678,14 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); - it('marks the artifact snapshot unavailable when rewind flush fails', async () => { + it('rejects rewind before mutation when recording is already degraded', async () => { const sessionId = '11111111-1111-1111-1111-111111111111'; const innerConfig = await setupSessionMocks(sessionId); const privateError = "EACCES: permission denied, open '/private/transcripts/session.jsonl'"; innerConfig.getChatRecordingService = vi.fn().mockReturnValue({ flush: vi.fn().mockRejectedValue(new Error(privateError)), + markIntegrityFailure: vi.fn(), }); const agentPromise = runAcpAgent( @@ -9538,19 +9701,56 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }) as AgentLike; await agent.newSession({ cwd: '/tmp', mcpServers: [] }); - const response = await agent.extMethod('rewindSession', { - sessionId, - targetTurnIndex: 1, - cwd: '/tmp', - }); + await expect( + agent.extMethod('rewindSession', { + sessionId, + targetTurnIndex: 1, + cwd: '/tmp', + }), + ).rejects.toThrow('Session recording is degraded; rewind was not applied.'); + expect(lastSessionMock?.rewindToTurn).not.toHaveBeenCalled(); - expect(response).toMatchObject({ - success: true, - artifactSnapshotUnavailable: 'artifact snapshot unavailable after rewind', - }); - expect(response).not.toHaveProperty('artifactSnapshot'); - expect(JSON.stringify(response)).not.toContain('/private/transcripts'); - expect(JSON.stringify(response)).not.toContain('EACCES'); + mockConnectionState.resolve(); + await agentPromise; + }); + + it('fails closed when rewind persistence becomes ambiguous', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + const innerConfig = await setupSessionMocks(sessionId); + const integrityFailure = new Error('Session write state is uncertain'); + const recording = { + flush: vi + .fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('EIO after append')), + markIntegrityFailure: vi.fn().mockReturnValue(integrityFailure), + }; + innerConfig.getChatRecordingService = vi.fn().mockReturnValue(recording); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod('rewindSession', { + sessionId, + targetTurnIndex: 1, + cwd: '/tmp', + }), + ).rejects.toBe(integrityFailure); + expect(lastSessionMock?.rewindToTurn).toHaveBeenCalledOnce(); + expect(recording.markIntegrityFailure).toHaveBeenCalledWith( + 'rewind_persistence', + ); mockConnectionState.resolve(); await agentPromise; @@ -10269,6 +10469,8 @@ describe('QwenAgent extMethod renameSession routing', () => { | undefined; let mockConfig: Config; let liveCancelPendingPrompt: ReturnType; + let liveBeginClose: ReturnType; + let liveWaitForActiveTurnsToSettle: ReturnType; // Live session sessionId is whatever `getSessionId()` on the inner config // returns; matches the existing test scaffolding. @@ -10279,6 +10481,8 @@ describe('QwenAgent extMethod renameSession routing', () => { mockConnectionState.reset(); capturedAgentFactory = undefined; liveCancelPendingPrompt = vi.fn().mockResolvedValue(undefined); + liveBeginClose = vi.fn().mockReturnValue(vi.fn()); + liveWaitForActiveTurnsToSettle = vi.fn().mockResolvedValue(undefined); vi.mocked(AgentSideConnection).mockImplementation((factory: unknown) => { capturedAgentFactory = factory as typeof capturedAgentFactory; @@ -10308,7 +10512,10 @@ describe('QwenAgent extMethod renameSession routing', () => { function makeRecordingService() { return { recordCustomTitle: vi.fn().mockResolvedValue(true), + finalize: vi.fn(), flush: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + hasWriteOwnership: vi.fn().mockReturnValue(false), }; } @@ -10323,6 +10530,10 @@ describe('QwenAgent extMethod renameSession routing', () => { }), refreshAuth: vi.fn().mockResolvedValue(undefined), getModel: vi.fn().mockReturnValue('m'), + getTargetDir: vi.fn().mockReturnValue('/tmp'), + storage: { + getRuntimeBaseDir: vi.fn().mockReturnValue('/tmp/runtime'), + }, getContentGeneratorConfig: vi.fn().mockReturnValue({}), getAvailableModels: vi.fn().mockReturnValue([]), getModes: vi.fn().mockReturnValue([]), @@ -10355,6 +10566,7 @@ describe('QwenAgent extMethod renameSession routing', () => { async function bootAgent( innerConfig: ReturnType, + options: { dispose?: ReturnType } = {}, ) { vi.mocked(loadSettings).mockReturnValue(makeAcpSettings()); vi.mocked(loadCliConfig).mockResolvedValue( @@ -10365,13 +10577,21 @@ describe('QwenAgent extMethod renameSession routing', () => { ({ getId: vi.fn().mockReturnValue(liveSessionId), getConfig: vi.fn().mockReturnValue(innerConfig), + beginClose: liveBeginClose, cancelPendingPrompt: liveCancelPendingPrompt, + waitForActiveTurnsToSettle: liveWaitForActiveTurnsToSettle, sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), - dispose: vi.fn(), + dispose: options.dispose ?? vi.fn(), + withExclusiveTranscriptSnapshot: vi.fn( + async (operation: (snapshot: Buffer) => Promise) => { + await innerConfig.getChatRecordingService()?.flush(); + return operation(Buffer.from('snapshot')); + }, + ), }) as unknown as InstanceType, ); @@ -10454,6 +10674,41 @@ describe('QwenAgent extMethod renameSession routing', () => { await agentPromise; }); + it('maps writer failures from offline session operations', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + const renameSpy = vi.fn().mockRejectedValue( + Object.assign(new Error('Session write ownership failed.'), { + errorKind: 'session_writer_conflict', + rpcCode: -32016, + }), + ); + vi.mocked(SessionService).mockImplementation( + () => + ({ + renameSession: renameSpy, + }) as unknown as InstanceType, + ); + + await expect( + agent.extMethod('renameSession', { + cwd: '/tmp', + sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + title: 'Renamed Offline', + }), + ).rejects.toMatchObject({ + code: -32016, + data: { errorKind: 'session_writer_conflict' }, + }); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('returns success=false when the live ChatRecordingService rejects the title (I/O error)', async () => { const recording = makeRecordingService(); recording.recordCustomTitle.mockResolvedValue(false); @@ -10570,6 +10825,55 @@ describe('QwenAgent extMethod renameSession routing', () => { await agentPromise; }); + it('waits for failed branch cleanup before releasing the operation', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const forkSessionFromSnapshot = vi.fn().mockResolvedValue({ + filePath: '/tmp/fork.jsonl', + copiedCount: 1, + }); + const renameSession = vi.fn().mockResolvedValue(false); + let resolveCleanup!: (removed: boolean) => void; + const removeSession = vi.fn( + () => + new Promise((resolve) => { + resolveCleanup = resolve; + }), + ); + vi.mocked(SessionService).mockImplementation( + () => + ({ + forkSessionFromSnapshot, + renameSession, + removeSession, + }) as unknown as InstanceType, + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + let settled = false; + const branch = agent + .extMethod(SERVE_CONTROL_EXT_METHODS.sessionBranch, { + cwd: '/tmp', + sessionId: liveSessionId, + name: 'Copy', + }) + .finally(() => { + settled = true; + }); + await vi.waitFor(() => expect(removeSession).toHaveBeenCalledOnce()); + expect(settled).toBe(false); + + resolveCleanup(true); + await expect(branch).rejects.toThrow('Failed to set title'); + expect(forkSessionFromSnapshot).toHaveBeenCalledOnce(); + expect(renameSession).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('keeps the live session open when strict session close flush fails', async () => { const recording = makeRecordingService(); recording.flush.mockRejectedValue(new Error('flush failed')); @@ -10631,6 +10935,241 @@ describe('QwenAgent extMethod renameSession routing', () => { mockConnectionState.resolve(); await agentPromise; }); + + it('waits for active turns to settle before the final strict close flush', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + let releaseTurn!: () => void; + liveWaitForActiveTurnsToSettle.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseTurn = resolve; + }), + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + let closed = false; + const closing = agent + .extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + requireFlush: true, + }) + .finally(() => { + closed = true; + }); + await vi.waitFor(() => + expect(liveWaitForActiveTurnsToSettle).toHaveBeenCalledOnce(), + ); + expect(liveBeginClose).toHaveBeenCalledOnce(); + expect(liveCancelPendingPrompt).toHaveBeenCalledOnce(); + expect(recording.flush).toHaveBeenCalledOnce(); + expect(closed).toBe(false); + + releaseTurn(); + await expect(closing).resolves.toEqual({ + sessionId: liveSessionId, + closed: true, + }); + expect(recording.flush).toHaveBeenCalledTimes(2); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('keeps the live session open when recorder ownership cannot be released', async () => { + const recording = makeRecordingService(); + recording.close.mockRejectedValueOnce(new Error('release failed')); + recording.hasWriteOwnership.mockReturnValue(true); + const innerConfig = makeLiveSessionInnerConfig(recording); + const toolRegistry = { stop: vi.fn().mockResolvedValue(undefined) }; + innerConfig.getToolRegistry.mockReturnValue(toolRegistry); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + requireFlush: true, + }), + ).rejects.toThrow('release failed'); + expect(toolRegistry.stop).not.toHaveBeenCalled(); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).toContain(liveSessionId); + + recording.hasWriteOwnership.mockReturnValue(false); + recording.close.mockResolvedValue(undefined); + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + requireFlush: true, + }), + ).resolves.toEqual({ sessionId: liveSessionId, closed: true }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('keeps the live session reachable while config-owned leases need a retry', async () => { + const recording = makeRecordingService(); + let ownsCompensationLease = false; + recording.close.mockImplementation(async () => { + if (ownsCompensationLease) ownsCompensationLease = false; + }); + const innerConfig = makeLiveSessionInnerConfig(recording); + const shutdown = vi + .fn() + .mockImplementationOnce(async () => { + ownsCompensationLease = true; + throw new Error('compensation release failed'); + }) + .mockResolvedValue(undefined); + Object.assign(innerConfig, { + shutdown, + hasSessionWriterOwnership: vi.fn(() => ownsCompensationLease), + }); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + }), + ).rejects.toThrow('compensation release failed'); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).toContain(liveSessionId); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + }), + ).resolves.toEqual({ sessionId: liveSessionId, closed: true }); + expect(shutdown).toHaveBeenCalledTimes(2); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('acknowledges close after writer release when Session.dispose throws', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const shutdown = vi.fn().mockResolvedValue(undefined); + Object.assign(innerConfig, { shutdown }); + const dispose = vi.fn(() => { + throw new Error('dispose failed'); + }); + const { agent, agentPromise } = await bootAgent(innerConfig, { dispose }); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + }), + ).resolves.toEqual({ sessionId: liveSessionId, closed: true }); + expect(shutdown).toHaveBeenCalledWith({ shutdownTelemetry: false }); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).not.toContain(liveSessionId); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('shuts down every config when Session.dispose throws during daemon disposal', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const shutdown = vi.fn().mockResolvedValue(undefined); + Object.assign(innerConfig, { shutdown }); + const dispose = vi.fn(() => { + throw new Error('dispose failed'); + }); + const { agent, agentPromise } = await bootAgent(innerConfig, { dispose }); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + mockConnectionState.resolve(); + await agentPromise; + + expect(dispose).toHaveBeenCalledOnce(); + expect(shutdown).toHaveBeenCalledWith({ shutdownTelemetry: false }); + }); + + it('waits for active turns before releasing writers during daemon disposal', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + let releaseTurn!: () => void; + liveWaitForActiveTurnsToSettle.mockImplementationOnce( + () => + new Promise((resolve) => { + releaseTurn = resolve; + }), + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + mockConnectionState.resolve(); + let disposed = false; + void agentPromise.finally(() => { + disposed = true; + }); + + await vi.waitFor(() => + expect(liveWaitForActiveTurnsToSettle).toHaveBeenCalledOnce(), + ); + expect(liveBeginClose).toHaveBeenCalledOnce(); + expect(recording.close).not.toHaveBeenCalled(); + expect(disposed).toBe(false); + + releaseTurn(); + await agentPromise; + expect(recording.finalize).toHaveBeenCalled(); + expect(recording.close).toHaveBeenCalled(); + }); + + it('retains a daemon session when disposal cannot release its writer', async () => { + const recording = makeRecordingService(); + recording.close.mockRejectedValue(new Error('release failed')); + recording.hasWriteOwnership.mockReturnValue(true); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + mockConnectionState.resolve(); + await agentPromise; + + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).toContain(liveSessionId); + }); }); describe('QwenAgent unstable_listSessions cursor parsing', () => { @@ -10899,6 +11438,55 @@ describe('QwenAgent unstable_listSessions cursor parsing', () => { await agentPromise; } }); + + it('keeps the bootstrap runtime root after runtimeOutputDir settings change', async () => { + Object.assign(mockConfig, { + getTargetDir: vi.fn().mockReturnValue('/tmp/project'), + storage: { + getRuntimeBaseDir: vi.fn().mockReturnValue('/tmp/runtime-at-start'), + }, + }); + const listSessions = vi.fn().mockResolvedValue({ + items: [], + nextCursor: undefined, + }); + vi.mocked(SessionService).mockImplementation( + () => + ({ + listSessions, + }) as unknown as InstanceType, + ); + vi.mocked(loadSettings) + .mockReturnValueOnce({ + merged: { advanced: { runtimeOutputDir: '/tmp/runtime-new-a' } }, + } as LoadedSettings) + .mockReturnValueOnce({ + merged: { advanced: { runtimeOutputDir: '/tmp/runtime-new-b' } }, + } as LoadedSettings); + const { agent, agentPromise } = await bootAgent(); + + try { + await agent.unstable_listSessions({ cwd: '/tmp/project' }); + await agent.unstable_listSessions({ cwd: '/tmp/project' }); + + expect(runWithAcpRuntimeOutputDir).not.toHaveBeenCalled(); + expect(Storage.runWithRuntimeBaseDir).toHaveBeenNthCalledWith( + 1, + '/tmp/runtime-at-start', + undefined, + expect.any(Function), + ); + expect(Storage.runWithRuntimeBaseDir).toHaveBeenNthCalledWith( + 2, + '/tmp/runtime-at-start', + undefined, + expect.any(Function), + ); + } finally { + mockConnectionState.resolve(); + await agentPromise; + } + }); }); // Tests for QwenAgent.loadSession() and QwenAgent.unstable_resumeSession() @@ -10995,6 +11583,10 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { ) { const recording = { rebuildTurnBoundaries: vi.fn(), + finalize: vi.fn(), + flush: vi.fn().mockResolvedValue(undefined), + close: vi.fn().mockResolvedValue(undefined), + hasWriteOwnership: vi.fn().mockReturnValue(false), }; return { initialize: vi.fn().mockResolvedValue(undefined), @@ -11009,6 +11601,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { getModes: vi.fn().mockReturnValue([]), getApprovalMode: vi.fn().mockReturnValue('default'), getSessionId: vi.fn().mockReturnValue('persisted-1'), + getTargetDir: vi.fn().mockReturnValue('/tmp'), getAuthType: vi.fn().mockReturnValue('api-key'), getAllConfiguredModels: vi.fn().mockReturnValue([]), getGeminiClient: vi.fn().mockReturnValue({ @@ -11026,6 +11619,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { getHookSystem: vi.fn().mockReturnValue(undefined), getDisableAllHooks: vi.fn().mockReturnValue(true), hasHooksForEvent: vi.fn().mockReturnValue(false), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), getChatRecordingService: vi.fn().mockReturnValue(recording), // load path reads back the persisted conversation here and feeds // it to `session.replayHistory`. resume path doesn't read this. @@ -11051,6 +11645,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { sessionExists: boolean; resumedConversation?: { messages: unknown[] }; primeTurnFromHistoryImpl?: (...args: unknown[]) => unknown; + replayHistoryImpl?: (...args: unknown[]) => unknown; }) { const innerConfig = makeRestoreInnerConfig({ resumedConversation: opts.resumedConversation, @@ -11070,7 +11665,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { getId: vi.fn().mockReturnValue('persisted-1'), getConfig: vi.fn().mockReturnValue(innerConfig), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), - replayHistory: vi.fn().mockResolvedValue(undefined), + replayHistory: vi.fn(opts.replayHistoryImpl), primeTurnFromHistory: vi.fn(opts.primeTurnFromHistoryImpl), cumulativeUsage: { promptTokens: 7, @@ -11081,6 +11676,9 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { installRewriter: vi.fn(), installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), + beginClose: vi.fn().mockReturnValue(vi.fn()), + cancelPendingPrompt: vi.fn().mockResolvedValue(undefined), + waitForActiveTurnsToSettle: vi.fn().mockResolvedValue(undefined), dispose: vi.fn(), }; lastSessionMock = sessionMock; @@ -11498,6 +12096,55 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { mockConnectionState.resolve(); await agentPromise; }); + it.each([ + [ + 'session_writer_conflict', + -32016, + 'This session is already open in another Qwen process.', + ], + [ + 'session_writer_lost', + -32017, + 'Write ownership for this session was lost.', + ], + [ + 'session_transcript_changed', + -32018, + 'The session transcript changed outside its active writer.', + ], + [ + 'session_writer_unavailable', + -32019, + 'Session write ownership could not be verified.', + ], + ] as const)( + 'loadSession maps %s to its stable ACP error code', + async (errorKind, rpcCode, expectedMessage) => { + bindRestoreMocks({ sessionExists: true }); + vi.mocked(loadCliConfig).mockRejectedValue( + Object.assign(new Error('Session write ownership failed.'), { + errorKind, + rpcCode, + }), + ); + const { agent, agentPromise } = await spawnAgent(); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toMatchObject({ + code: rpcCode, + data: { errorKind }, + message: expectedMessage, + }); + + mockConnectionState.resolve(); + await agentPromise; + }, + ); it('loadSession returns LoadSessionResponse and replays history on the session', async () => { const messages = [{ role: 'user', parts: [{ text: 'hi' }] }]; @@ -11839,6 +12486,42 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); + it('loadSession removes a normal restore if history replay throws', async () => { + let replayCalls = 0; + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'hi' }] }], + }, + replayHistoryImpl: () => { + replayCalls++; + if (replayCalls === 1) throw new Error('replay boom'); + }, + }); + const { agent, agentPromise } = await spawnAgent(); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toThrow('replay boom'); + const failedSession = lastSessionMock; + expect(failedSession?.dispose).toHaveBeenCalledOnce(); + + await expect( + agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + }), + ).resolves.toMatchObject({ modes: expect.anything() }); + expect(lastSessionMock).not.toBe(failedSession); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('loadSession skips history replay when getResumedSessionData() returns undefined', async () => { // Distinct code path: `createAndStoreSession(config, undefined)` // takes the no-conversation branch, so `replayHistory` must @@ -11866,7 +12549,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); - it('loadSession disposes the existing session when reloading the same sessionId', async () => { + it('loadSession reuses the existing live session for the same sessionId', async () => { bindRestoreMocks({ sessionExists: true, resumedConversation: { @@ -11885,18 +12568,84 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { expect(firstSession).toBeDefined(); expect(firstSession!.dispose).not.toHaveBeenCalled(); - // Second loadSession with the same sessionId should dispose the first + // A duplicate attach must reuse the existing owner instead of creating a + // second Config and competing for the same transcript lease. await agent.loadSession({ cwd: '/tmp', sessionId: 'persisted-1', mcpServers: [], }); - expect(firstSession!.dispose).toHaveBeenCalledTimes(1); + expect(firstSession!.dispose).not.toHaveBeenCalled(); + expect(loadCliConfig).toHaveBeenCalledTimes(1); mockConnectionState.resolve(); await agentPromise; }); + it.each(['loadSession', 'unstable_resumeSession'] as const)( + '%s rejects a live session owned by another workspace with invalid params', + async (method) => { + bindRestoreMocks({ sessionExists: true }); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + + const request = { + cwd: '/tmp/other', + sessionId: 'persisted-1', + ...(method === 'loadSession' ? { mcpServers: [] } : {}), + }; + await expect(agent[method](request)).rejects.toMatchObject({ + code: -32602, + data: { errorKind: 'invalid_params' }, + message: 'The live session belongs to another workspace.', + }); + expect(loadCliConfig).toHaveBeenCalledTimes(1); + + mockConnectionState.resolve(); + await agentPromise; + }, + ); + + it.each(['loadSession', 'unstable_resumeSession'] as const)( + '%s preserves writer integrity errors for an existing live session', + async (method) => { + const innerConfig = bindRestoreMocks({ sessionExists: true }); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + vi.mocked(innerConfig.assertCanStartTurn).mockRejectedValueOnce( + Object.assign(new Error('unsafe local writer details'), { + errorKind: 'session_writer_lost', + rpcCode: -32017, + }), + ); + + const request = { + cwd: '/tmp', + sessionId: 'persisted-1', + ...(method === 'loadSession' ? { mcpServers: [] } : {}), + }; + await expect(agent[method](request)).rejects.toMatchObject({ + code: -32017, + data: { errorKind: 'session_writer_lost' }, + message: 'Write ownership for this session was lost.', + }); + expect(loadCliConfig).toHaveBeenCalledTimes(1); + + mockConnectionState.resolve(); + await agentPromise; + }, + ); + it('unstable_resumeSession throws resourceNotFound when the persisted session is missing', async () => { bindRestoreMocks({ sessionExists: false }); const { agent, agentPromise } = await spawnAgent(); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 75f1953346f..65768498898 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -31,6 +31,7 @@ import { MCPServerConfig, runForkedAgent, SessionService, + SessionWriterUnavailableError, SESSION_TITLE_MAX_LENGTH, Storage, tokenLimit, @@ -409,6 +410,120 @@ function workspaceMemoryErrorData( }; } +const SESSION_WRITER_RPC_CODES = { + session_writer_conflict: -32016, + session_writer_lost: -32017, + session_transcript_changed: -32018, + session_writer_unavailable: -32019, +} as const; + +const SESSION_WRITER_MESSAGES = { + session_writer_conflict: + 'This session is already open in another Qwen process.', + session_writer_lost: 'Write ownership for this session was lost.', + session_transcript_changed: + 'The session transcript changed outside its active writer.', + session_writer_unavailable: 'Session write ownership could not be verified.', +} as const; + +function getSessionWriterError(error: unknown): + | { + rpcCode: number; + errorKind: keyof typeof SESSION_WRITER_RPC_CODES; + message: string; + } + | undefined { + if (!error || typeof error !== 'object') return undefined; + const candidate = error as Record; + const errorKind = candidate['errorKind']; + if ( + typeof errorKind !== 'string' || + !(errorKind in SESSION_WRITER_RPC_CODES) + ) { + return undefined; + } + const typedKind = errorKind as keyof typeof SESSION_WRITER_RPC_CODES; + if (candidate['rpcCode'] !== SESSION_WRITER_RPC_CODES[typedKind]) { + return undefined; + } + return { + rpcCode: SESSION_WRITER_RPC_CODES[typedKind], + errorKind: typedKind, + message: SESSION_WRITER_MESSAGES[typedKind], + }; +} + +async function assertConfigCanStartTurn(config: Config): Promise { + try { + await config.assertCanStartTurn?.(); + } catch (error) { + const writerError = getSessionWriterError(error); + if (writerError) { + throw new RequestError(writerError.rpcCode, writerError.message, { + errorKind: writerError.errorKind, + }); + } + throw error; + } +} + +async function shutdownConfig(config: Config): Promise { + const compatible = config as Config & { + shutdown?: (options?: { shutdownTelemetry?: boolean }) => Promise; + getToolRegistry?: () => { stop?: () => Promise } | undefined; + hasSessionWriterOwnership?: () => boolean; + }; + const recorder = compatible.getChatRecordingService?.(); + let recorderCloseError: unknown; + try { + recorder?.finalize(); + await recorder?.close(); + } catch (error) { + recorderCloseError = error; + } + let shutdownError: unknown; + if (compatible.shutdown) { + try { + await compatible.shutdown({ shutdownTelemetry: false }); + } catch (error) { + shutdownError = error; + } + } else { + try { + await compatible.getToolRegistry?.()?.stop?.(); + } catch (error) { + shutdownError = error; + } + } + if ( + compatible.hasSessionWriterOwnership?.() ?? + recorder?.hasWriteOwnership?.() + ) { + throw ( + recorderCloseError ?? shutdownError ?? new SessionWriterUnavailableError() + ); + } + if (shutdownError !== undefined) throw shutdownError; +} + +async function shutdownConfigAfterFailure( + config: Config, + failure: unknown, +): Promise { + try { + await shutdownConfig(config); + } catch (shutdownError) { + if (shutdownError instanceof Error && shutdownError.cause === undefined) { + Reflect.defineProperty(shutdownError, 'cause', { + value: failure, + configurable: true, + }); + } + return shutdownError; + } + return failure; +} + const logWorkspaceMemoryExtractionError = createWorkspaceMemoryExtractionErrorLogger(debugLogger); @@ -2669,7 +2784,7 @@ export async function runAcpAgent( try { // Fire SessionEnd hook for all active sessions (aligned with core path) await fireSessionEndOnce(SessionEndReason.Other); - agentInstance?.disposeSessions(); + await agentInstance?.disposeSessions(); try { process.stdin.destroy(); @@ -2710,7 +2825,7 @@ export async function runAcpAgent( // Mirror the SIGTERM handler's pool drain on the IDE-initiated // normal close path to avoid leaking shared MCP entries. await drainPoolBeforeExit('ide_close'); - agentInstance?.disposeSessions(); + await agentInstance?.disposeSessions(); } finally { process.off('SIGTERM', shutdownHandler); process.off('SIGINT', shutdownHandler); @@ -2882,6 +2997,7 @@ interface PendingMcpAuthentication { class QwenAgent implements Agent { private sessions: Map = new Map(); + private readonly runtimeBaseDirsByWorkspace = new Map(); private workspaceMcpDiscoveryConfig: Config | undefined; private workspaceMcpDiscoveryPromise: Promise | undefined; private workspaceMcpDiscoveryError: string | undefined; @@ -3039,51 +3155,52 @@ class QwenAgent implements Agent { ): Promise { if (this.workspaceMcpDiscoveryConfig) return; const cwd = this.config.getTargetDir(); - const config = await loadCliConfig( - settings.merged, - { - ...this.argv, - sessionId: 'workspace-mcp-discovery', - resume: undefined, - continue: false, - }, + const config = await this.runWithPinnedRuntimeBaseDir( + settings, cwd, - undefined, - { - userHooks: settings.getUserHooks(), - projectHooks: settings.getProjectHooks(), + async () => { + const config = await loadCliConfig( + settings.merged, + { + ...this.argv, + sessionId: 'workspace-mcp-discovery', + resume: undefined, + continue: false, + chatRecording: false, + }, + cwd, + undefined, + { + userHooks: settings.getUserHooks(), + projectHooks: settings.getProjectHooks(), + }, + buildDisabledSkillNamesProvider(settings), + ); + config.setMcpTransportPool(this.mcpPool); + try { + await config.initialize({ + skipGeminiInitialization: true, + skipFileCheckpointing: true, + skipHooks: true, + skipSkillManager: true, + skipMcpDiscovery: true, + lenientToolWarmup: true, + }); + const manager = config.getToolRegistry()?.getMcpClientManager(); + if (!manager) throw new Error('MCP client manager is unavailable'); + await manager.discoverAllMcpToolsIncremental(config); + if (manager.getDiscoveryState() === MCPDiscoveryState.NOT_STARTED) { + throw new Error( + 'MCP discovery did not start. The workspace may not be trusted.', + ); + } + return config; + } catch (error) { + throw await shutdownConfigAfterFailure(config, error); + } }, - buildDisabledSkillNamesProvider(settings), ); - config.setMcpTransportPool(this.mcpPool); - try { - await config.initialize({ - skipGeminiInitialization: true, - skipFileCheckpointing: true, - skipHooks: true, - skipSkillManager: true, - skipMcpDiscovery: true, - lenientToolWarmup: true, - }); - const manager = config.getToolRegistry()?.getMcpClientManager(); - if (!manager) { - throw new Error('MCP client manager is unavailable'); - } - await manager.discoverAllMcpToolsIncremental(config); - if (manager.getDiscoveryState() === MCPDiscoveryState.NOT_STARTED) { - throw new Error( - 'MCP discovery did not start. The workspace may not be trusted.', - ); - } - this.workspaceMcpDiscoveryConfig = config; - } catch (error) { - try { - await config.getToolRegistry()?.stop(); - } catch { - // Preserve the initialization failure that made this config unusable. - } - throw error; - } + this.workspaceMcpDiscoveryConfig = config; } private initializeWorkspaceMcpDiscovery(): { accepted: boolean } { @@ -3204,83 +3321,135 @@ class QwenAgent implements Agent { } } + const releaseCloseGate = session.beginClose(); + let removedFromStore = false; try { - await session.cancelPendingPrompt(); - } catch (err) { - debugLogger.debug( - `Session ${sessionId} cancel during close failed: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - } + try { + await session.cancelPendingPrompt(); + } catch (err) { + debugLogger.debug( + `Session ${sessionId} cancel during close failed: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + await session.waitForActiveTurnsToSettle(); - const flushError = await flushRecording(); - if (flushError !== undefined && requireFlush) { - throw flushError; - } + session.getConfig().getChatRecordingService()?.finalize(); + const flushError = await flushRecording(); + if (flushError !== undefined && requireFlush) { + throw flushError; + } - try { - await session.getConfig().getToolRegistry()?.stop(); - } catch (err) { - debugLogger.debug( - `Session ${sessionId} tool registry stop during close failed: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - } + const recorder = session.getConfig().getChatRecordingService(); + let recorderCloseError: unknown; + try { + await recorder?.close(); + } catch (error) { + recorderCloseError = error; + } + if (recorder?.hasWriteOwnership?.()) { + // Keep an otherwise-usable Config intact when its current writer could + // not be released. A later close can retry without having stopped the + // session's tools and watchers first. + throw recorderCloseError ?? new SessionWriterUnavailableError(); + } - unregisterGoalHook(session.getConfig(), sessionId); - this.mcpPool?.releaseSession(sessionId); - uiTelemetryService.removeSession(sessionId); - this.sessions.delete(sessionId); + const cleanupErrors: unknown[] = []; + try { + // Config owns every writer associated with this live session, including + // retired and not-yet-bound leases left by a failed transition. Closing + // only the current recorder can never release those safely. + await shutdownConfig(session.getConfig()); + } catch (error) { + cleanupErrors.push(error); + } + if ( + ( + session.getConfig() as Config & { + hasSessionWriterOwnership?: () => boolean; + } + ).hasSessionWriterOwnership?.() + ) { + throw cleanupErrors.at(-1) ?? new SessionWriterUnavailableError(); + } + try { + session.dispose(); + } catch (error) { + cleanupErrors.push(error); + } + try { + unregisterGoalHook(session.getConfig(), sessionId); + } catch (error) { + cleanupErrors.push(error); + } + try { + this.mcpPool?.releaseSession(sessionId); + } catch (error) { + cleanupErrors.push(error); + } + try { + uiTelemetryService.removeSession(sessionId); + } catch (error) { + cleanupErrors.push(error); + } + this.sessions.delete(sessionId); + removedFromStore = true; + if (cleanupErrors.length > 0) { + debugLogger.warn( + `Session ${sessionId} closed after ${cleanupErrors.length} ancillary cleanup failure(s): ${cleanupErrors + .map((error) => + error instanceof Error ? error.message : String(error), + ) + .join('; ')}`, + ); + } + } finally { + if (!removedFromStore) releaseCloseGate(); + } } - private discardStoredSessionIfCurrent( + private async discardStoredSessionIfCurrent( sessionId: string, session: Session, - ): void { + ): Promise { if (this.sessions.get(sessionId) !== session) { return; } - const logCleanupFailure = (action: string, err: unknown) => { - debugLogger.debug( - `Session ${sessionId} ${action} during failed restore cleanup failed: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - }; - try { - session.dispose(); - } catch (err) { - logCleanupFailure('dispose', err); - } - try { - unregisterGoalHook(session.getConfig(), sessionId); - } catch (err) { - logCleanupFailure('goal hook unregister', err); - } - try { - this.mcpPool?.releaseSession(sessionId); - } catch (err) { - logCleanupFailure('MCP pool release', err); - } - try { - uiTelemetryService.removeSession(sessionId); - } catch (err) { - logCleanupFailure('telemetry removal', err); - } - this.sessions.delete(sessionId); + await this.closeStoredSession(sessionId); } - disposeSessions(): void { + async disposeSessions(): Promise { for (const generation of this.generationControllers.values()) { generation.controller.abort(); } this.generationControllers.clear(); - for (const session of this.sessions.values()) { - session.dispose(); + const sessionIds = [...this.sessions.keys()]; + const closeResults = await Promise.allSettled( + sessionIds.map((sessionId) => this.closeStoredSession(sessionId)), + ); + closeResults.forEach((result, index) => { + if (result.status === 'fulfilled') return; + const sessionId = sessionIds[index]; + debugLogger.debug( + `Session ${sessionId ?? 'unknown'} close during daemon dispose failed: ${ + result.reason instanceof Error + ? result.reason.message + : String(result.reason) + }`, + ); + }); + const discoveryConfig = this.workspaceMcpDiscoveryConfig; + this.workspaceMcpDiscoveryConfig = undefined; + if (discoveryConfig) { + await shutdownConfig(discoveryConfig).catch((error) => { + debugLogger.debug( + `Workspace MCP discovery config shutdown failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + }); } - this.sessions.clear(); this.disposeTranscriptReplayConfigs(); } @@ -3290,6 +3459,14 @@ class QwenAgent implements Agent { private argv: CliArgs, private connection: AgentSideConnection, ) { + const bootstrapCwd = this.config.getTargetDir?.(); + const bootstrapRuntimeBaseDir = this.config.storage?.getRuntimeBaseDir?.(); + if (bootstrapCwd && bootstrapRuntimeBaseDir) { + this.runtimeBaseDirsByWorkspace.set( + path.resolve(bootstrapCwd), + bootstrapRuntimeBaseDir, + ); + } // Pool kill switch via env var so operators can A/B compare or // roll back without rebuilding. `run-qwen-serve.ts` sets this when // `--no-mcp-pool` is passed at daemon startup. @@ -3323,6 +3500,22 @@ class QwenAgent implements Agent { } } + private runWithPinnedRuntimeBaseDir( + settings: LoadedSettings, + cwd: string, + operation: () => T, + ): T { + const workspace = path.resolve(cwd); + let runtimeBaseDir = this.runtimeBaseDirsByWorkspace.get(workspace); + if (runtimeBaseDir === undefined) { + runtimeBaseDir = runWithAcpRuntimeOutputDir(settings, cwd, () => + Storage.getRuntimeBaseDir(), + ); + this.runtimeBaseDirsByWorkspace.set(workspace, runtimeBaseDir); + } + return Storage.runWithRuntimeBaseDir(runtimeBaseDir, undefined, operation); + } + /** Expose the pool's workspace-scoped budget controller for snapshot builders. */ getWorkspaceMcpBudget(): WorkspaceMcpBudget | undefined { return this.workspaceMcpBudget; @@ -3474,14 +3667,18 @@ class QwenAgent implements Agent { const config = await profiler.time('config_setup', () => this.newSessionConfig(cwd, mcpServers, settings), ); - await profiler.time('auth', () => this.ensureAuthenticated(config)); - profiler.timeSync('file_system_setup', () => - this.setupFileSystem(config), - ); - - const session = await profiler.time('session_register', () => - this.createAndStoreSession(config, settings), - ); + let session: Session; + try { + await profiler.time('auth', () => this.ensureAuthenticated(config)); + profiler.timeSync('file_system_setup', () => + this.setupFileSystem(config), + ); + session = await profiler.time('session_register', () => + this.createAndStoreSession(config, settings), + ); + } catch (error) { + throw await shutdownConfigAfterFailure(config, error); + } profiler.setSessionId(session.getId()); return profiler.timeSync('response_build', () => ({ sessionId: session.getId(), @@ -3495,11 +3692,30 @@ class QwenAgent implements Agent { } async loadSession(params: LoadSessionRequest): Promise { - // Load per-request settings BEFORE the existence check: the check must - // resolve `advanced.runtimeOutputDir` from THIS request's cwd, not from - // whichever settings a concurrent handler loaded last. + // Load per-request settings before the existence check. An unseen + // workspace resolves its runtime root from this instance; an existing + // workspace keeps the root pinned when the agent first adopted it. const settings = loadSettingsCached(params.cwd); - const exists = await runWithAcpRuntimeOutputDir( + const liveSession = this.sessions.get(params.sessionId); + if (liveSession) { + const liveConfig = liveSession.getConfig(); + if ( + path.resolve(liveConfig.getTargetDir()) !== path.resolve(params.cwd) + ) { + throw new RequestError( + -32602, + 'The live session belongs to another workspace.', + { errorKind: 'invalid_params' }, + ); + } + await assertConfigCanStartTurn(liveConfig); + return { + modes: this.buildModesData(liveConfig), + models: this.buildAvailableModels(liveConfig), + configOptions: this.buildConfigOptions(liveConfig), + } as LoadSessionResponse; + } + const exists = await this.runWithPinnedRuntimeBaseDir( settings, params.cwd, async () => { @@ -3526,22 +3742,26 @@ class QwenAgent implements Agent { params.sessionId, true, ); - await this.ensureAuthenticated(config); - this.setupFileSystem(config); - + let session: Session; const sessionData = config.getResumedSessionData(); const bulkReplay = isBulkLoadReplayRequest(params); const replayPageSize = bulkReplay ? getLoadReplayPageSize(params) : undefined; - const session = await this.createAndStoreSession( - config, - settings, - sessionData, - bulkReplay - ? { replayHistory: false, startPostReplayServices: false } - : {}, - ); + try { + await this.ensureAuthenticated(config); + this.setupFileSystem(config); + session = await this.createAndStoreSession( + config, + settings, + sessionData, + bulkReplay + ? { replayHistory: false, startPostReplayServices: false } + : {}, + ); + } catch (error) { + throw await shutdownConfigAfterFailure(config, error); + } let replayEnvelope: BridgeLoadReplayEnvelope | undefined; if (bulkReplay) { try { @@ -3588,7 +3808,7 @@ class QwenAgent implements Agent { session.installRewriter(); session.startCronScheduler(); } catch (err) { - this.discardStoredSessionIfCurrent(params.sessionId, session); + await this.discardStoredSessionIfCurrent(params.sessionId, session); throw err; } } @@ -3624,7 +3844,26 @@ class QwenAgent implements Agent { ): Promise { // Same per-request settings discipline as `loadSession`. const settings = loadSettingsCached(params.cwd); - const exists = await runWithAcpRuntimeOutputDir( + const liveSession = this.sessions.get(params.sessionId); + if (liveSession) { + const liveConfig = liveSession.getConfig(); + if ( + path.resolve(liveConfig.getTargetDir()) !== path.resolve(params.cwd) + ) { + throw new RequestError( + -32602, + 'The live session belongs to another workspace.', + { errorKind: 'invalid_params' }, + ); + } + await assertConfigCanStartTurn(liveConfig); + return { + modes: this.buildModesData(liveConfig), + models: this.buildAvailableModels(liveConfig), + configOptions: this.buildConfigOptions(liveConfig), + } as ResumeSessionResponse; + } + const exists = await this.runWithPinnedRuntimeBaseDir( settings, params.cwd, async () => { @@ -3644,15 +3883,19 @@ class QwenAgent implements Agent { params.sessionId, true, ); - await this.ensureAuthenticated(config); - this.setupFileSystem(config); - - const session = await this.createAndStoreSession( - config, - settings, - config.getResumedSessionData(), - { replayHistory: false }, - ); + let session: Session; + try { + await this.ensureAuthenticated(config); + this.setupFileSystem(config); + session = await this.createAndStoreSession( + config, + settings, + config.getResumedSessionData(), + { replayHistory: false }, + ); + } catch (error) { + throw await shutdownConfigAfterFailure(config, error); + } await this.#restoreWorktreeOnResume(config, session); this.#restoreGoalOnResume(config, session); @@ -3764,7 +4007,8 @@ class QwenAgent implements Agent { // (same pattern filesystem.ts uses for `_meta.bom` / `_meta.encoding`). const size = normalizeAcpSessionListSize(params._meta?.['size']); - const result = await runWithAcpRuntimeOutputDir(this.settings, cwd, () => { + const settings = loadSettingsCached(cwd); + const result = await this.runWithPinnedRuntimeBaseDir(settings, cwd, () => { const sessionService = new SessionService(cwd); return sessionService.listSessions({ cursor: numericCursor, @@ -6085,6 +6329,23 @@ class QwenAgent implements Agent { async extMethod( method: string, params: Record, + ): Promise> { + try { + return await this.extMethodInner(method, params); + } catch (error) { + const writerError = getSessionWriterError(error); + if (writerError) { + throw new RequestError(writerError.rpcCode, writerError.message, { + errorKind: writerError.errorKind, + }); + } + throw error; + } + } + + private async extMethodInner( + method: string, + params: Record, ): Promise> { const requestedCwd = typeof params['cwd'] === 'string' ? params['cwd'] : undefined; @@ -6427,44 +6688,51 @@ class QwenAgent implements Agent { try { const settings = loadSettingsCached(cwd); - return await runWithAcpRuntimeOutputDir(settings, cwd, async () => { - const reader = new SessionTranscriptReader(cwd); - const page = await reader.readPage(sessionId, { - ...(typeof rawCursor === 'string' ? { cursor: rawCursor } : {}), - ...(typeof rawBeforeRecordId === 'string' - ? { beforeRecordId: rawBeforeRecordId } - : {}), - ...(typeof rawLimit === 'number' ? { limit: rawLimit } : {}), - maxBytes: SESSION_TRANSCRIPT_MAX_PAGE_BYTES, - }); - const config = await this.getTranscriptReplayConfig(cwd, settings); - const replay = await replayTranscriptRecordPage({ - sessionId, - page, - config, - encodeCursor: (state) => - encodeSessionTranscriptCursor(state, cwd), - logger: debugLogger, - }); - return { - v: 1, - sessionId, - events: replay.updates.map((update) => ({ + return await this.runWithPinnedRuntimeBaseDir( + settings, + cwd, + async () => { + const reader = new SessionTranscriptReader(cwd); + const page = await reader.readPage(sessionId, { + ...(typeof rawCursor === 'string' ? { cursor: rawCursor } : {}), + ...(typeof rawBeforeRecordId === 'string' + ? { beforeRecordId: rawBeforeRecordId } + : {}), + ...(typeof rawLimit === 'number' ? { limit: rawLimit } : {}), + maxBytes: SESSION_TRANSCRIPT_MAX_PAGE_BYTES, + }); + const config = await this.getTranscriptReplayConfig( + cwd, + settings, + ); + const replay = await replayTranscriptRecordPage({ + sessionId, + page, + config, + encodeCursor: (state) => + encodeSessionTranscriptCursor(state, cwd), + logger: debugLogger, + }); + return { v: 1, - type: 'session_update', - data: update, - })), - ...(replay.nextCursor !== undefined - ? { nextCursor: replay.nextCursor } - : {}), - hasMore: replay.hasMore, - startTime: replay.startTime, - lastUpdated: replay.lastUpdated, - ...(replay.replayError !== undefined - ? { partial: true, replayError: replay.replayError } - : {}), - } as Record; - }); + sessionId, + events: replay.updates.map((update) => ({ + v: 1, + type: 'session_update', + data: update, + })), + ...(replay.nextCursor !== undefined + ? { nextCursor: replay.nextCursor } + : {}), + hasMore: replay.hasMore, + startTime: replay.startTime, + lastUpdated: replay.lastUpdated, + ...(replay.replayError !== undefined + ? { partial: true, replayError: replay.replayError } + : {}), + } as Record; + }, + ); } catch (error) { if ( error instanceof InvalidSessionTranscriptCursorError || @@ -7411,6 +7679,36 @@ class QwenAgent implements Agent { } return { sessionId, persisted: true, kind }; } + case 'qwen/session/recordTextElements': { + const sessionId = params['sessionId']; + const content = params['content']; + const textElements = params['textElements']; + if (typeof sessionId !== 'string' || sessionId.length === 0) { + throw RequestError.invalidParams( + undefined, + 'Invalid or missing sessionId', + ); + } + if (typeof content !== 'string' || !Array.isArray(textElements)) { + throw RequestError.invalidParams( + undefined, + 'Invalid user text elements payload', + ); + } + const session = this.sessionOrThrow(sessionId); + await session.withExclusiveMaintenance(async () => { + await session.getConfig().assertCanStartTurn(); + const recording = session.getConfig().getChatRecordingService(); + if (!recording) { + throw RequestError.internalError( + undefined, + 'Chat recording service unavailable', + ); + } + await recording.recordUserTextElements({ content, textElements }); + }); + return { sessionId, persisted: true }; + } case SERVE_CONTROL_EXT_METHODS.sessionTitle: { const sessionId = params['sessionId']; const displayName = params['displayName']; @@ -7589,41 +7887,42 @@ class QwenAgent implements Agent { } } - // Relocate working directory (skip process.chdir and artifact - // migration for ACP — storage stays at the bound workspace so - // branch/load/lifecycle paths remain consistent). - const warnings: string[] = []; - const relocation = await config.relocateWorkingDirectory( - canonicalPath, - canonicalPath, - { skipProcessChdir: true, skipArtifactMigration: true }, - ); - if (relocation.memoryRefreshError) { - warnings.push( - `Memory refresh failed: ${ - relocation.memoryRefreshError instanceof Error - ? relocation.memoryRefreshError.message - : String(relocation.memoryRefreshError) - }`, - ); - } - - // Update model context - try { - await config - .getGeminiClient() - ?.addWorkingDirectoryChangedContext(previousCwd, canonicalPath); - } catch (error) { - warnings.push( - `Model context refresh failed: ${ - error instanceof Error ? error.message : String(error) - }`, - ); - } + return session.withExclusiveMaintenance(async () => { + // Relocate working directory without changing the daemon process cwd. + // The Config owns a global session lease, so its transcript and + // sidecars can move atomically without allowing another writer in. + const warnings: string[] = []; + const relocation = await config.relocateWorkingDirectory( + canonicalPath, + canonicalPath, + { skipProcessChdir: true }, + ); + if (relocation.memoryRefreshError) { + warnings.push( + `Memory refresh failed: ${ + relocation.memoryRefreshError instanceof Error + ? relocation.memoryRefreshError.message + : String(relocation.memoryRefreshError) + }`, + ); + } - session.clearTodoStopGuardTrust(); + // Update model context + try { + await config + .getGeminiClient() + ?.addWorkingDirectoryChangedContext(previousCwd, canonicalPath); + } catch (error) { + warnings.push( + `Model context refresh failed: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } - return { previousCwd, newCwd: canonicalPath, warnings }; + session.clearTodoStopGuardTrust(); + return { previousCwd, newCwd: canonicalPath, warnings }; + }); } case SERVE_CONTROL_EXT_METHODS.sessionApprovalMode: { const sessionId = params['sessionId']; @@ -8461,14 +8760,25 @@ class QwenAgent implements Agent { 'Invalid or missing sessionId', ); } - const success = await runWithAcpRuntimeOutputDir( - this.settings, - cwd, - async () => { - const sessionService = new SessionService(cwd); - return sessionService.removeSession(sessionId); - }, - ); + const liveConfig = this.sessions.get(sessionId)?.getConfig(); + const liveSessionService = liveConfig + ? new SessionService(liveConfig.getTargetDir(), { + runtimeBaseDir: liveConfig.storage.getRuntimeBaseDir(), + }) + : undefined; + if (liveConfig) { + await this.closeStoredSession(sessionId, { requireFlush: true }); + } + const success = liveSessionService + ? await liveSessionService.removeSession(sessionId) + : await this.runWithPinnedRuntimeBaseDir( + loadSettingsCached(cwd), + cwd, + async () => { + const sessionService = new SessionService(cwd); + return sessionService.removeSession(sessionId); + }, + ); return { success }; } case 'renameSession': { @@ -8508,8 +8818,8 @@ class QwenAgent implements Agent { const ok = await liveRecording.recordCustomTitle(title, 'manual'); return { success: ok }; } - const success = await runWithAcpRuntimeOutputDir( - this.settings, + const success = await this.runWithPinnedRuntimeBaseDir( + loadSettingsCached(cwd), cwd, async () => { const sessionService = new SessionService(cwd); @@ -8582,96 +8892,118 @@ class QwenAgent implements Agent { } const rewindFiles = params['rewindFiles'] !== false; - const historyBeforeRewind = session.captureHistorySnapshot(); - let rewindResult; - try { - rewindResult = session.rewindToTurn(turnIndex as number, { - rewindFiles, - }); - } catch (err) { - if (err instanceof RequestError) { - const msg = err.message; - if (msg.includes('Cannot rewind while a prompt is running')) { - throw new RequestError(err.code, msg, { - errorKind: 'session_busy', - }); - } - if (msg.includes('compressed or does not exist')) { - throw new RequestError(err.code, msg, { - errorKind: 'invalid_rewind_target', - }); + return await session.withExclusiveMaintenance(async () => { + const config = session.getConfig(); + await config.assertCanStartTurn?.(); + const recording = config.getChatRecordingService(); + if (!recording) { + throw RequestError.internalError( + undefined, + 'Chat recording service unavailable', + ); + } + try { + await recording.flush(); + } catch (error) { + if (getSessionWriterError(error)) throw error; + throw RequestError.internalError( + undefined, + 'Session recording is degraded; rewind was not applied.', + ); + } + const historyBeforeRewind = session.captureHistorySnapshot(); + const fileHistoryService = config.getFileHistoryService(); + let rewindResult; + try { + rewindResult = session.rewindToTurn(turnIndex as number, { + rewindFiles, + }); + } catch (err) { + if (err instanceof RequestError) { + const msg = err.message; + if (msg.includes('Cannot rewind while a prompt is running')) { + throw new RequestError(err.code, msg, { + errorKind: 'session_busy', + }); + } + if (msg.includes('compressed or does not exist')) { + throw new RequestError(err.code, msg, { + errorKind: 'invalid_rewind_target', + }); + } } + throw err; } - throw err; - } - let filesChanged: string[] = []; - let filesFailed: string[] = []; - if (rewindFiles && promptId) { - const fhs = session.getConfig().getFileHistoryService(); try { - const fileResult = await fhs.rewind(promptId, true); - filesChanged = fileResult.filesChanged; - filesFailed = fileResult.filesFailed; + await recording.flush(); + } catch { + throw recording.markIntegrityFailure('rewind_persistence'); + } + + let filesChanged: string[] = []; + let filesFailed: string[] = []; + if (rewindFiles && promptId) { + try { + const fileResult = await fileHistoryService.rewind( + promptId, + true, + ); + filesChanged = fileResult.filesChanged; + filesFailed = fileResult.filesFailed; + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + debugLogger.error( + `[ACP] File-history rewind failed for session=${sessionId} promptId=${promptId}: ${reason}`, + ); + filesFailed = [`file-history-rewind: ${reason}`]; + } + } + let artifactSnapshot: unknown; + let artifactSnapshotUnavailable: string | undefined; + try { + const sessionData = await config + .getSessionService() + .loadSession(sessionId); + if (sessionData === undefined) { + artifactSnapshotUnavailable = + 'session data unavailable after rewind'; + } else if (sessionData.artifactSnapshot) { + artifactSnapshot = sessionData.artifactSnapshot; + } else { + // A successful reload with no artifact records is a valid empty + // artifact timeline, distinct from an unavailable reload. + artifactSnapshot = { + v: SESSION_ARTIFACT_PERSISTENCE_VERSION, + sessionId, + sequence: 0, + artifacts: [], + tombstonedIds: [], + stickyEphemeralIds: [], + warnings: [], + }; + } } catch (err) { const reason = err instanceof Error ? err.message : String(err); - debugLogger.error( - `[ACP] File-history rewind failed for session=${sessionId} promptId=${promptId}: ${reason}`, - ); - filesFailed = [`file-history-rewind: ${reason}`]; - } - } - let artifactSnapshot: unknown; - let artifactSnapshotUnavailable: string | undefined; - try { - await session.getConfig().getChatRecordingService()?.flush(); - const cwd = session.getConfig().getProjectRoot(); - const sessionData = await runWithAcpRuntimeOutputDir( - this.settings, - cwd, - async () => { - const sessionService = new SessionService(cwd); - return sessionService.loadSession(sessionId); - }, - ); - if (sessionData === undefined) { artifactSnapshotUnavailable = - 'session data unavailable after rewind'; - } else if (sessionData.artifactSnapshot) { - artifactSnapshot = sessionData.artifactSnapshot; - } else { - // A successful reload with no artifact records is a valid empty - // artifact timeline, distinct from an unavailable reload. - artifactSnapshot = { - v: SESSION_ARTIFACT_PERSISTENCE_VERSION, - sessionId, - sequence: 0, - artifacts: [], - tombstonedIds: [], - stickyEphemeralIds: [], - warnings: [], - }; + 'artifact snapshot unavailable after rewind'; + debugLogger.warn( + `[ACP] Failed to rebuild artifact snapshot after rewind for session=${sessionId}: ${reason}`, + ); } - } catch (err) { - const reason = err instanceof Error ? err.message : String(err); - artifactSnapshotUnavailable = - 'artifact snapshot unavailable after rewind'; - debugLogger.warn( - `[ACP] Failed to rebuild artifact snapshot after rewind for session=${sessionId}: ${reason}`, - ); - } - return { - success: true, - historyBeforeRewind, - ...rewindResult, - filesChanged, - filesFailed, - ...(artifactSnapshot ? { artifactSnapshot } : {}), - ...(artifactSnapshotUnavailable - ? { artifactSnapshotUnavailable } - : {}), - }; + return { + success: true, + historyBeforeRewind, + ...rewindResult, + filesChanged, + filesFailed, + ...(artifactSnapshot ? { artifactSnapshot } : {}), + ...(artifactSnapshotUnavailable + ? { artifactSnapshotUnavailable } + : {}), + }; + }); } case 'qwen/session/loadUpdates': { const sessionId = params['sessionId'] as string; @@ -8682,14 +9014,17 @@ class QwenAgent implements Agent { ); } - const sessionData = await runWithAcpRuntimeOutputDir( - this.settings, - cwd, - async () => { - const sessionService = new SessionService(cwd); - return sessionService.loadSession(sessionId); - }, - ); + const liveConfig = this.sessions.get(sessionId)?.getConfig(); + const sessionData = liveConfig + ? await liveConfig.getSessionService().loadSession(sessionId) + : await this.runWithPinnedRuntimeBaseDir( + loadSettingsCached(cwd), + cwd, + async () => { + const sessionService = new SessionService(cwd); + return sessionService.loadSession(sessionId); + }, + ); if (!sessionData?.conversation) { return { updates: [] }; } @@ -8770,18 +9105,20 @@ class QwenAgent implements Agent { }); } - const recording = sourceSession.getConfig().getChatRecordingService(); - if (recording) { - await recording.flush(); - } - const newSessionId = randomUUID(); - return await runWithAcpRuntimeOutputDir( - this.settings, - cwd, - async () => { - const sessionService = new SessionService(cwd); - await sessionService.forkSession(sessionId, newSessionId); + const sourceConfig = sourceSession.getConfig(); + const sourceCwd = sourceConfig.getTargetDir(); + const sourceRuntimeBaseDir = sourceConfig.storage.getRuntimeBaseDir(); + return await sourceSession.withExclusiveTranscriptSnapshot( + async (sourceSnapshot) => { + const sessionService = new SessionService(sourceCwd, { + runtimeBaseDir: sourceRuntimeBaseDir, + }); + await sessionService.forkSessionFromSnapshot( + sessionId, + newSessionId, + sourceSnapshot, + ); let title: string; try { @@ -8789,7 +9126,10 @@ class QwenAgent implements Agent { if (typeof name === 'string' && name.trim().length > 0) { baseName = name.trim(); } else { - const existingTitle = recording?.getCurrentCustomTitle(); + const existingTitle = sourceSession + .getConfig() + .getChatRecordingService() + ?.getCurrentCustomTitle(); const stripped = existingTitle ?.replace(/\s*\(Branch(?:\s+\d+)?\)\s*$/, '') .trim(); @@ -8814,11 +9154,13 @@ class QwenAgent implements Agent { ); } } catch (err) { - sessionService.removeSession(newSessionId).catch((rmErr) => { - process.stderr.write( - `qwen serve: failed to clean up orphan session ${newSessionId}: ${rmErr instanceof Error ? rmErr.message : rmErr}\n`, - ); - }); + await sessionService + .removeSession(newSessionId) + .catch((rmErr) => { + process.stderr.write( + `qwen serve: failed to clean up orphan session ${newSessionId}: ${rmErr instanceof Error ? rmErr.message : rmErr}\n`, + ); + }); throw err; } @@ -9288,21 +9630,13 @@ class QwenAgent implements Agent { } private disposeTranscriptReplayConfig(config: Config): void { - try { - void Promise.resolve(config.getToolRegistry()?.stop()).catch((err) => { - debugLogger.debug( - `Transcript replay config tool registry stop failed: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - }); - } catch (err) { + void shutdownConfig(config).catch((err) => { debugLogger.debug( - `Transcript replay config tool registry stop failed: ${ + `Transcript replay config shutdown failed: ${ err instanceof Error ? err.message : String(err) }`, ); - } + }); } private disposeTranscriptReplayConfigs(): void { @@ -9332,17 +9666,25 @@ class QwenAgent implements Agent { } const entry: TranscriptReplayConfigCacheEntry = { settings }; - const pending = this.newSessionConfig(cwd, [], settings, undefined, false, { - skipMcpDiscovery: true, - skipHooks: true, - skipSkillManager: true, - skipFileCheckpointing: true, - // Read-only replay: tolerate tools that cannot construct without the - // subsystems skipped above (e.g. SkillTool needs the SkillManager). The - // registry is only consulted for optional tool_call metadata during - // replay, and ToolCallEmitter falls back to the recorded tool name. - lenientToolWarmup: true, - }); + const pending = this.newSessionConfig( + cwd, + [], + settings, + undefined, + false, + { + skipMcpDiscovery: true, + skipHooks: true, + skipSkillManager: true, + skipFileCheckpointing: true, + // Read-only replay: tolerate tools that cannot construct without the + // subsystems skipped above (e.g. SkillTool needs the SkillManager). The + // registry is only consulted for optional tool_call metadata during + // replay, and ToolCallEmitter falls back to the recorded tool name. + lenientToolWarmup: true, + }, + false, + ); entry.pending = pending; this.transcriptReplayConfigCache.set(key, entry); try { @@ -9378,6 +9720,39 @@ class QwenAgent implements Agent { sessionId?: string, resume?: boolean, initializeOptions: ConfigInitializeOptions = {}, + chatRecording = true, + ): Promise { + try { + return await this.runWithPinnedRuntimeBaseDir(settings, cwd, () => + this.newSessionConfigInRuntimeContext( + cwd, + mcpServers, + settings, + sessionId, + resume, + initializeOptions, + chatRecording, + ), + ); + } catch (error) { + const writerError = getSessionWriterError(error); + if (writerError) { + throw new RequestError(writerError.rpcCode, writerError.message, { + errorKind: writerError.errorKind, + }); + } + throw error; + } + } + + private async newSessionConfigInRuntimeContext( + cwd: string, + mcpServers: McpServer[], + settings: LoadedSettings, + sessionId?: string, + resume?: boolean, + initializeOptions: ConfigInitializeOptions = {}, + chatRecording = true, ): Promise { // ACP/IDE-injected servers are session-level: they must outrank a project // `.mcp.json` and stay un-gated. Collect them separately and pass them as @@ -9449,6 +9824,7 @@ class QwenAgent implements Agent { ...this.argv, ...sessionArg, continue: false, + chatRecording, }; const config = await loadCliConfig( @@ -9548,15 +9924,22 @@ class QwenAgent implements Agent { }); }); } - await config.initialize({ - ...initializeOptions, - // Reverse tool channel (issue #5626, Phase 2): bind the session - // manager's SDK MCP callback to the `client_mcp/message` ext-method so a - // client-hosted (extension) MCP server added at runtime reaches the - // daemon WS. Servers that aren't client-hosted never use this callback - // (the daemon only adds SDK-type runtime servers for client MCP). - sendSdkMcpMessage: this.buildClientMcpSender(wiredSessionId), - }); + try { + await config.initialize({ + ...initializeOptions, + // Reverse tool channel (issue #5626, Phase 2): bind the session + // manager's SDK MCP callback to the `client_mcp/message` ext-method so a + // client-hosted (extension) MCP server added at runtime reaches the + // daemon WS. Servers that aren't client-hosted never use this callback + // (the daemon only adds SDK-type runtime servers for client MCP). + sendSdkMcpMessage: this.buildClientMcpSender(wiredSessionId), + }); + } catch (error) { + throw await shutdownConfigAfterFailure(config, error); + } + if (config.getSessionWriterOwnerId?.()) { + config.startRuntimeStatus?.(); + } // ACP sessions served to WebUI clients are interactive: MCP tools can // arrive progressively, but session creation/loading must not wait for a // slow or wedged server discovery. @@ -9640,42 +10023,52 @@ class QwenAgent implements Agent { await geminiClient.initialize(); } - this.sessions.get(sessionId)?.dispose(); + if (this.sessions.has(sessionId)) { + throw new Error(`Session ${sessionId} is already active.`); + } const session = new Session(sessionId, config, this.connection, settings); this.sessions.set(sessionId, session); - setTimeout(async () => { - await session.sendAvailableCommandsUpdate(); - }, 0); - - if (sessionData?.fileHistorySnapshots?.length) { - config - .getFileHistoryService() - .restoreFromSnapshots(sessionData.fileHistorySnapshots); - } + try { + if (sessionData?.fileHistorySnapshots?.length) { + config + .getFileHistoryService() + .restoreFromSnapshots(sessionData.fileHistorySnapshots); + } - if (sessionData?.conversation.messages) { - config - .getChatRecordingService() - ?.rebuildTurnBoundaries(sessionData.conversation.messages); - } + if (sessionData?.conversation.messages) { + config + .getChatRecordingService() + ?.rebuildTurnBoundaries(sessionData.conversation.messages); + } - if (options.replayHistory !== false && sessionData?.conversation.messages) { - await session.replayHistory( - sessionData.conversation.messages, - sessionData.historyGaps, - ); - } + if ( + options.replayHistory !== false && + sessionData?.conversation.messages + ) { + await session.replayHistory( + sessionData.conversation.messages, + sessionData.historyGaps, + ); + } - if (options.startPostReplayServices !== false) { - // Install rewriter AFTER history replay to avoid rewriting historical messages - session.installRewriter(); + if (options.startPostReplayServices !== false) { + // Install rewriter AFTER history replay to avoid rewriting historical messages + session.installRewriter(); - // After replay so a durable cron fire can't interleave with it. - session.startCronScheduler(); + // After replay so a durable cron fire can't interleave with it. + session.startCronScheduler(); + } + } catch (error) { + await this.discardStoredSessionIfCurrent(sessionId, session); + throw error; } + setTimeout(async () => { + await session.sendAvailableCommandsUpdate(); + }, 0); + return session; } diff --git a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts index b239d03c933..aed7e031e23 100644 --- a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts @@ -163,6 +163,12 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ CONNECTING: 'connecting', CONNECTED: 'connected', }, + Storage: { + getRuntimeBaseDir: vi.fn(() => '/tmp/qwen-runtime'), + runWithRuntimeBaseDir: vi.fn( + (_dir: string, _cwd: string | undefined, fn: () => T): T => fn(), + ), + }, McpTransportPool: vi.fn().mockImplementation(() => ({ acquire: vi.fn(), release: vi.fn(), diff --git a/packages/cli/src/acp-integration/runtimeOutputDirContext.test.ts b/packages/cli/src/acp-integration/runtimeOutputDirContext.test.ts index e1068a74ccc..088ee44f8e2 100644 --- a/packages/cli/src/acp-integration/runtimeOutputDirContext.test.ts +++ b/packages/cli/src/acp-integration/runtimeOutputDirContext.test.ts @@ -31,4 +31,40 @@ describe('runWithAcpRuntimeOutputDir', () => { expect(Storage.getRuntimeBaseDir()).toBe(Storage.getGlobalQwenDir()); }); + + it('isolates concurrent workspace runtime directories across awaits', async () => { + const cwdA = path.resolve('workspace', 'project-a'); + const cwdB = path.resolve('workspace', 'project-b'); + const settingsA = { + merged: { advanced: { runtimeOutputDir: '.runtime-a' } }, + } as LoadedSettings; + const settingsB = { + merged: { advanced: { runtimeOutputDir: '.runtime-b' } }, + } as LoadedSettings; + let arrivals = 0; + let release!: () => void; + const barrier = new Promise((resolve) => { + release = resolve; + }); + const waitForBoth = async () => { + arrivals++; + if (arrivals === 2) release(); + await barrier; + }; + + await Promise.all([ + runWithAcpRuntimeOutputDir(settingsA, cwdA, async () => { + const storage = new Storage(cwdA); + await waitForBoth(); + expect(Storage.getRuntimeBaseDir()).toBe(path.join(cwdA, '.runtime-a')); + expect(storage.getRuntimeBaseDir()).toBe(path.join(cwdA, '.runtime-a')); + }), + runWithAcpRuntimeOutputDir(settingsB, cwdB, async () => { + const storage = new Storage(cwdB); + await waitForBoth(); + expect(Storage.getRuntimeBaseDir()).toBe(path.join(cwdB, '.runtime-b')); + expect(storage.getRuntimeBaseDir()).toBe(path.join(cwdB, '.runtime-b')); + }), + ]); + }); }); diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 52effc52150..bb6b82e35c0 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -319,6 +319,11 @@ describe('Session', () => { recordFileHistorySnapshot: ReturnType; rewindRecording: ReturnType; setTitleRecordedCallback: ReturnType; + finalize: ReturnType; + pause: ReturnType; + readStableTranscriptSnapshot: ReturnType; + isIntegrityFailed: ReturnType; + resume: ReturnType; }; let mockFileHistoryService: { makeSnapshot: ReturnType; @@ -498,6 +503,13 @@ describe('Session', () => { recordFileHistorySnapshot: vi.fn(), rewindRecording: vi.fn(), setTitleRecordedCallback: vi.fn(), + finalize: vi.fn(), + pause: vi.fn().mockResolvedValue(undefined), + readStableTranscriptSnapshot: vi + .fn() + .mockResolvedValue(Buffer.from('snapshot')), + isIntegrityFailed: vi.fn().mockReturnValue(false), + resume: vi.fn(), }; mockFileHistoryService = { makeSnapshot: vi.fn().mockResolvedValue(undefined), @@ -516,6 +528,9 @@ describe('Session', () => { }; mockConfig = { + storage: { + getRuntimeBaseDir: vi.fn(() => core.Storage.getRuntimeBaseDir()), + }, setApprovalMode: vi.fn(), // #buildInitialSystemReminders branches on ApprovalMode.PLAN on every // session.prompt(), so the default must be defined. Individual tests @@ -524,6 +539,7 @@ describe('Session', () => { switchModel: switchModelSpy, getModel: vi.fn().mockImplementation(() => currentModel), getSessionId: vi.fn().mockReturnValue('test-session-id'), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), getWorkingDir: vi.fn().mockReturnValue(process.cwd()), getProjectRoot: vi.fn().mockReturnValue('/repo'), // Folder trust gates the project `.qwen/loop.md`; default trusted (the @@ -671,6 +687,146 @@ describe('Session', () => { ); }); + it('takes an exclusive, paused transcript snapshot for live branching', async () => { + const operation = vi.fn().mockResolvedValue('branched'); + + await expect( + session.withExclusiveTranscriptSnapshot(operation), + ).resolves.toBe('branched'); + + expect(mockChatRecordingService.finalize).toHaveBeenCalledOnce(); + expect(mockChatRecordingService.pause).toHaveBeenCalledWith({ + requireHealthy: true, + }); + expect(operation).toHaveBeenCalledWith(Buffer.from('snapshot')); + expect(mockChatRecordingService.resume).toHaveBeenCalledOnce(); + }); + + it('preserves a snapshot integrity error without resuming the recorder', async () => { + const error = new core.SessionTranscriptChangedError(); + mockChatRecordingService.readStableTranscriptSnapshot.mockRejectedValueOnce( + error, + ); + mockChatRecordingService.isIntegrityFailed.mockReturnValueOnce(true); + + await expect(session.withExclusiveTranscriptSnapshot(vi.fn())).rejects.toBe( + error, + ); + expect(mockChatRecordingService.resume).not.toHaveBeenCalled(); + }); + + it('rejects prompts while exclusive session maintenance is running', async () => { + let finishMaintenance!: () => void; + const maintenance = session.withExclusiveMaintenance( + () => + new Promise((resolve) => { + finishMaintenance = resolve; + }), + ); + await vi.waitFor(() => expect(session.isIdle()).toBe(false)); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toMatchObject({ code: -32602 }); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + + finishMaintenance(); + await maintenance; + expect(session.isIdle()).toBe(true); + }); + + it('holds the close gate until released and waits for active turns', async () => { + let resolveTurn!: () => void; + const turnCompletion = new Promise((resolve) => { + resolveTurn = resolve; + }); + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = turnCompletion; + + const releaseClose = session.beginClose(); + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toMatchObject({ code: -32602 }); + + let settled = false; + const waiting = session.waitForActiveTurnsToSettle().then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + + resolveTurn(); + await waiting; + releaseClose(); + expect(session.isIdle()).toBe(false); + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = null; + expect(session.isIdle()).toBe(true); + }); + + it('maps writer ownership loss before an ACP user turn starts', async () => { + vi.mocked(mockConfig.assertCanStartTurn).mockRejectedValueOnce( + new core.SessionWriterLostError(), + ); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toMatchObject({ + code: -32017, + data: { errorKind: 'session_writer_lost' }, + }); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + }); + + it('does not let an ACP textual recovery command bypass writer admission', async () => { + vi.mocked(mockConfig.assertCanStartTurn).mockRejectedValueOnce( + new core.SessionWriterLostError(), + ); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: '/resume missing-session' }], + }), + ).rejects.toMatchObject({ + code: -32017, + data: { errorKind: 'session_writer_lost' }, + }); + expect(nonInteractiveCliCommands.handleSlashCommand).not.toHaveBeenCalled(); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + }); + + it('maps writer ownership loss detected by the lower model send path', async () => { + vi.mocked(mockChat.sendMessageStream).mockRejectedValueOnce( + new core.SessionWriterLostError(), + ); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toMatchObject({ + code: -32017, + data: { errorKind: 'session_writer_lost' }, + }); + }); + describe('continueLastTurn', () => { it('returns none and starts no continuation when the last turn ended cleanly', async () => { vi.mocked(mockChat.getHistory).mockReturnValue([ @@ -9956,8 +10112,11 @@ describe('Session', () => { }); it('runs prompt inside runtime output dir context', async () => { - const runtimeDir = path.resolve('runtime', 'from-settings'); - core.Storage.setRuntimeBaseDir(runtimeDir); + const runtimeDir = path.resolve('runtime', 'pinned-to-config'); + core.Storage.setRuntimeBaseDir(path.resolve('runtime', 'process-global')); + mockConfig.storage = { + getRuntimeBaseDir: vi.fn().mockReturnValue(runtimeDir), + } as unknown as Config['storage']; session = new Session( 'test-session-id', mockConfig, diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 1d8b2ae0887..3bc789f1e75 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -111,6 +111,7 @@ import { shouldRunAutoModeForCall, extractDaemonTraceContext, withInteractionSpan, + SessionWriterError, startToolSpan, endToolSpan, runInToolSpanContext, @@ -1130,6 +1131,7 @@ export class Session implements SessionContext { // or session reload), which would otherwise execute orphaned cron prompts // on a session whose registries are already unregistered. private disposed = false; + private exclusiveMaintenance = false; private unsubscribeChatRecordingFailure?: () => void; // Modular components @@ -1166,7 +1168,7 @@ export class Session implements SessionContext { private readonly settings: LoadedSettings, ) { this.sessionId = id; - this.runtimeBaseDir = Storage.getRuntimeBaseDir(); + this.runtimeBaseDir = config.storage.getRuntimeBaseDir(); const todoStopGuardEnabled = this.settings.merged.experimental?.todoStopGuard === true && !this.config.getBareMode() && @@ -1493,6 +1495,19 @@ export class Session implements SessionContext { return this.config; } + async #assertCanStartTurn(): Promise { + try { + await this.config.assertCanStartTurn?.(); + } catch (error) { + if (error instanceof SessionWriterError) { + throw new RequestError(error.rpcCode, error.message, { + errorKind: error.errorKind, + }); + } + throw error; + } + } + isIdle(): boolean { return ( !this.pendingPrompt && @@ -1500,10 +1515,74 @@ export class Session implements SessionContext { !this.cronProcessing && !this.cronAbortController && !this.notificationProcessing && - !this.notificationAbortController + !this.notificationAbortController && + !this.exclusiveMaintenance ); } + async withExclusiveMaintenance(operation: () => Promise): Promise { + if (!this.isIdle()) { + throw RequestError.invalidParams( + undefined, + 'Cannot modify the session while a turn is running', + ); + } + this.exclusiveMaintenance = true; + try { + return await operation(); + } finally { + this.exclusiveMaintenance = false; + void this.#drainCronQueue(); + void this.#drainNotificationQueue(); + } + } + + beginClose(): () => void { + if (this.exclusiveMaintenance) { + throw RequestError.invalidParams( + undefined, + 'Session maintenance is already in progress', + ); + } + this.exclusiveMaintenance = true; + let released = false; + return () => { + if (released) return; + released = true; + this.exclusiveMaintenance = false; + void this.#drainCronQueue(); + void this.#drainNotificationQueue(); + }; + } + + async waitForActiveTurnsToSettle(): Promise { + const pending = [ + this.pendingPromptCompletion, + this.cronCompletion, + this.notificationCompletion, + ].filter((completion): completion is Promise => completion !== null); + await Promise.allSettled(pending); + } + + async withExclusiveTranscriptSnapshot( + operation: (snapshot: Uint8Array) => Promise, + ): Promise { + return this.withExclusiveMaintenance(async () => { + const recorder = this.config.getChatRecordingService(); + if (!recorder) throw new Error('Session recording is disabled'); + let paused = false; + try { + recorder.finalize(); + await recorder.pause({ requireHealthy: true }); + paused = true; + const snapshot = await recorder.readStableTranscriptSnapshot(); + return await operation(snapshot); + } finally { + if (paused && !recorder.isIntegrityFailed()) recorder.resume(); + } + }); + } + getTurnCount(): number { return this.turn; } @@ -1849,6 +1928,12 @@ export class Session implements SessionContext { } async prompt(params: PromptRequest): Promise { + if (this.exclusiveMaintenance) { + throw RequestError.invalidParams( + undefined, + 'Session maintenance is in progress', + ); + } const todoStopGuardPreparation = this.#prepareTodoStopGuardForPrompt(params); // Install this prompt's AbortController before awaiting the previous @@ -1935,6 +2020,13 @@ export class Session implements SessionContext { void this.#drainNotificationQueue(); this.#maybeEmitFollowupSuggestion(result); return result; + } catch (error) { + if (error instanceof SessionWriterError) { + throw new RequestError(error.rpcCode, error.message, { + errorKind: error.errorKind, + }); + } + throw error; } finally { this.pendingPrompt = null; const shouldDrainAutomaticQueues = @@ -2138,6 +2230,7 @@ export class Session implements SessionContext { this.runtimeBaseDir, this.config.getWorkingDir(), async () => { + await this.#assertCanStartTurn(); // Increment turn counter for each user prompt this.turn += 1; @@ -4122,6 +4215,7 @@ export class Session implements SessionContext { if (this.pendingPrompt) return; if (this.notificationProcessing) return; if (this.#nextCronQueueIndex() < 0) return; + if (this.exclusiveMaintenance) return; this.cronProcessing = true; let resolveCompletion!: () => void; @@ -4228,6 +4322,7 @@ export class Session implements SessionContext { async () => { let turnCount = 0; try { + await this.#assertCanStartTurn(); // A `<>` / `<>` sentinel is expanded at // fire time into the loop.md task block — full on the first or a // changed fire, a short reminder when unchanged. Non-sentinel @@ -4702,6 +4797,7 @@ export class Session implements SessionContext { async #drainNotificationQueue(): Promise { if (this.disposed) return; if (this.notificationProcessing) return; + if (this.exclusiveMaintenance) return; if (this.pendingPrompt || this.cronProcessing || this.cronAbortController) { return; } @@ -4777,6 +4873,7 @@ export class Session implements SessionContext { const promptId = this.config.getSessionId() + '########notification' + Date.now(); try { + await this.#assertCanStartTurn(); await this.#emitBackgroundNotificationDisplay(item); const notificationParts: Part[] = [{ text: item.modelText }]; diff --git a/packages/cli/src/acp-integration/session/Session.worktree.test.ts b/packages/cli/src/acp-integration/session/Session.worktree.test.ts index 19f93ea70ac..2ef33f3402f 100644 --- a/packages/cli/src/acp-integration/session/Session.worktree.test.ts +++ b/packages/cli/src/acp-integration/session/Session.worktree.test.ts @@ -98,6 +98,9 @@ describe('Session.pendingWorktreeNotice', () => { }; mockConfig = { + storage: { + getRuntimeBaseDir: vi.fn().mockReturnValue('/tmp/qwen-runtime'), + }, setApprovalMode: vi.fn(), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), switchModel: vi.fn(), diff --git a/packages/cli/src/commands/mcp/reconnect.ts b/packages/cli/src/commands/mcp/reconnect.ts index 64ae47dc1b0..c931bbab459 100644 --- a/packages/cli/src/commands/mcp/reconnect.ts +++ b/packages/cli/src/commands/mcp/reconnect.ts @@ -71,6 +71,7 @@ async function createMinimalConfig(): Promise { targetDir: cwd, cwd, debugMode: false, + chatRecording: false, mcpServers, pendingMcpServers: getPendingGatedMcpServers(mcpServers, cwd), fileDiscoveryService: fileService, diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index cdc122205e2..a34142064a3 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -12,6 +12,7 @@ import { DEFAULT_QWEN_MODEL, OutputFormat, NativeLspService, + SessionWriterConflictError, Storage, } from '@qwen-code/qwen-code-core'; import { loadCliConfig, parseArguments, type CliArgs } from './config.js'; @@ -1448,6 +1449,34 @@ describe('loadCliConfig', () => { expect(mockExit).toHaveBeenCalledWith(1); }); + it('should direct a conflicting offline fork to the owning session', async () => { + const sourceSessionId = '123e4567-e89b-42d3-a456-426614174000'; + mockSessionServiceInstance.loadSession.mockResolvedValue({ + conversation: { sessionId: sourceSessionId, messages: [] }, + uiHistory: [], + }); + mockSessionServiceInstance.forkSession.mockRejectedValue( + new SessionWriterConflictError(), + ); + const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit called'); + }); + + await expect( + loadCliConfig({}, { + resume: sourceSessionId, + forkSession: true, + } as CliArgs), + ).rejects.toThrow('process.exit called'); + + expect(mockWriteStderrLine).toHaveBeenCalledWith( + expect.stringContaining( + 'Open the owning Qwen session and run /branch there.', + ), + ); + expect(mockExit).toHaveBeenCalledWith(1); + }); + it('should explain when --continue --fork-session has no saved session to fork', async () => { const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => { throw new Error('process.exit called'); @@ -4236,6 +4265,29 @@ describe('loadCliConfig runtimeOutputDir', () => { await loadCliConfig({}, argv); expect(Storage.getRuntimeBaseDir()).toBe(Storage.getGlobalQwenDir()); }); + + it('does not pollute the process default when loaded in a runtime context', async () => { + const argv = await parseArguments(); + const globalRuntimeDir = path.resolve('global', 'runtime'); + const cwd = path.resolve('workspace', 'contextual-project'); + Storage.setRuntimeBaseDir(globalRuntimeDir); + + const config = await Storage.runWithRuntimeBaseDir( + '.context-runtime', + cwd, + () => + loadCliConfig( + { advanced: { runtimeOutputDir: '.context-runtime' } }, + argv, + cwd, + ), + ); + + expect(config.storage.getRuntimeBaseDir()).toBe( + path.join(cwd, '.context-runtime'), + ); + expect(Storage.getRuntimeBaseDir()).toBe(globalRuntimeDir); + }); }); describe('loadCliConfig plansDirectory', () => { diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 2fbadccc3d2..082ad5c8f59 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -21,6 +21,7 @@ import { InputFormat, OutputFormat, SessionService, + SessionWriterConflictError, ideContextStore, type ResumedSessionData, type LspClient, @@ -1529,7 +1530,9 @@ export async function loadCliConfig( // Set runtime output directory from settings (env var QWEN_RUNTIME_DIR // is auto-detected inside getRuntimeBaseDir() at each call site). // Pass cwd so that relative paths like ".qwen" resolve per-project. - Storage.setRuntimeBaseDir(settings.advanced?.runtimeOutputDir, cwd); + if (!Storage.hasRuntimeBaseDirContext()) { + Storage.setRuntimeBaseDir(settings.advanced?.runtimeOutputDir, cwd); + } const ideMode = settings.ide?.enabled ?? false; @@ -1926,8 +1929,12 @@ export async function loadCliConfig( try { await sessionService.forkSession(sourceSessionId, forkedSessionId); } catch (err) { + const ownerHint = + err instanceof SessionWriterConflictError + ? ' Open the owning Qwen session and run /branch there.' + : ''; writeStderrLine( - `Failed to fork session ${sourceSessionId}: ${err instanceof Error ? err.message : String(err)}`, + `Failed to fork session ${sourceSessionId}: ${err instanceof Error ? err.message : String(err)}${ownerHint}`, ); process.exit(1); } diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index dd5e9d2a9b3..752196e1f06 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -445,7 +445,7 @@ export async function main() { if (sandboxConfig) { const partialConfig = await loadCliConfig( settings.merged, - argv, + { ...argv, chatRecording: false }, undefined, [], // Pass separated hooks for proper source attribution @@ -713,9 +713,13 @@ export async function main() { settingsWatcher?.startWatching(); markAcpStartup('configConstructionStart'); + const bootstrapArgv = + argv.acp || argv.experimentalAcp + ? { ...argv, chatRecording: false } + : argv; const config = await loadCliConfig( settings.merged, - argv, + bootstrapArgv, process.cwd(), argv.extensions, // Pass separated hooks for proper source attribution diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index 43600a10121..08f7a6466e3 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -140,6 +140,53 @@ function errMsg(err: unknown): string { return err instanceof Error ? err.message : String(err); } +const SESSION_WRITER_RPC_ERRORS = { + session_writer_conflict: { + code: -32016, + message: 'This session is already open in another Qwen process.', + }, + session_writer_lost: { + code: -32017, + message: 'Write ownership for this session was lost.', + }, + session_transcript_changed: { + code: -32018, + message: 'The session transcript changed outside its active writer.', + }, + session_writer_unavailable: { + code: -32019, + message: 'Session write ownership could not be verified.', + }, +} as const; + +function sessionWriterRpcError(err: unknown): + | { + code: number; + message: string; + data: { errorKind: keyof typeof SESSION_WRITER_RPC_ERRORS }; + } + | undefined { + if (!err || typeof err !== 'object') return undefined; + const candidate = err as Record; + const data = isObject(candidate['data']) ? candidate['data'] : undefined; + const errorKind = data?.['errorKind'] ?? candidate['errorKind']; + if ( + typeof errorKind !== 'string' || + !(errorKind in SESSION_WRITER_RPC_ERRORS) + ) { + return undefined; + } + const typedKind = errorKind as keyof typeof SESSION_WRITER_RPC_ERRORS; + const expected = SESSION_WRITER_RPC_ERRORS[typedKind]; + const code = candidate['code'] ?? candidate['rpcCode']; + if (code !== expected.code) return undefined; + return { + code: expected.code, + message: expected.message, + data: { errorKind: typedKind }, + }; +} + const debugLogger = createDebugLogger('ACP_HTTP_DISPATCH'); type PermissionResponse = Parameters< @@ -449,6 +496,8 @@ function toRpcError(err: unknown): { message: string; data?: Record; } { + const writerError = sessionWriterRpcError(err); + if (writerError) return writerError; if (err instanceof AcpParamError || err instanceof InvalidCursorError) { return { code: RPC.INVALID_PARAMS, message: err.message }; } @@ -675,6 +724,7 @@ export class AcpDispatcher { private readonly sessionShellCommandEnabled: boolean = false, private readonly registry?: ConnectionRegistry, private readonly archiveCoordinator: SessionArchiveCoordinator = new SessionArchiveCoordinator(), + private readonly runtimeBaseDir?: string, ) { this.agentManager = createDaemonSubagentManager(boundWorkspace); } @@ -687,9 +737,9 @@ export class AcpDispatcher { .killSession(sessionId, { requireZeroAttaches: true }) .then(async (killed) => { if (killed && removePersistedSession) { - await new SessionService(this.boundWorkspace).removeSession( - sessionId, - ); + await new SessionService(this.boundWorkspace, { + runtimeBaseDir: this.runtimeBaseDir, + }).removeSession(sessionId); } }) .catch((err) => @@ -1178,13 +1228,13 @@ export class AcpDispatcher { const restored = await this.archiveCoordinator.runSharedMany( [sessionId], async () => { - await assertSessionLoadable(cwd, sessionId); + await assertSessionLoadable(cwd, sessionId, this.runtimeBaseDir); // Re-seed the persisted parent lineage so a restored sub-session // still reports its parent over the ACP transport (parity with the // REST restore handler); the bridge creates the entry without it. - const metadata = await new SessionService( - cwd, - ).readCreationMetadata(sessionId); + const metadata = await new SessionService(cwd, { + runtimeBaseDir: this.runtimeBaseDir, + }).readCreationMetadata(sessionId); return method === 'session/load' ? await this.bridge.loadSession({ sessionId, @@ -1360,6 +1410,7 @@ export class AcpDispatcher { parentSessionId, ...parsedSource, }, + { runtimeBaseDir: this.runtimeBaseDir }, ); this.replyConn(conn, id, { sessions: result.sessions.map((s) => ({ @@ -1401,25 +1452,20 @@ export class AcpDispatcher { // concurrent closes from this connection cannot both reach the bridge. conn.ownedSessions.delete(sessionId); conn.closingSessions.add(sessionId); - let closeStarted = false; const closeSession = async () => { - closeStarted = true; + await this.bridge.closeSession( + sessionId, + this.sessionCtx(conn, sessionId, loopback), + ); + // The bridge has confirmed that the ACP child released its writer. + // Only now can the local stream, abort controller, buffered frames, + // and pending permissions be torn down. try { - await this.bridge.closeSession( - sessionId, - this.sessionCtx(conn, sessionId, loopback), + conn.closeSessionStream(sessionId); + } catch (teardownErr) { + writeStderrLine( + `qwen serve: /acp session/close local teardown failed (${logSafe(sessionId)}): ${logSafe(errMsg(teardownErr))}`, ); - } finally { - // Local teardown must run even if the bridge close throws — - // otherwise the SSE stream, abort controller, buffered frames and - // pending permissions leak until idle TTL. - try { - conn.closeSessionStream(sessionId); - } catch (teardownErr) { - writeStderrLine( - `qwen serve: /acp session/close local teardown failed (${logSafe(sessionId)}): ${logSafe(errMsg(teardownErr))}`, - ); - } } }; try { @@ -1444,9 +1490,10 @@ export class AcpDispatcher { } } } catch (err) { - if (!closeStarted) { - conn.ownedSessions.add(sessionId); - } + // The bridge retains the live session when child close fails. Keep + // local ownership and the stream binding too so the client can + // retry instead of orphaning a still-owned writer. + conn.ownedSessions.add(sessionId); throw err; } finally { conn.closingSessions.delete(sessionId); @@ -2062,7 +2109,9 @@ export class AcpDispatcher { ); } await this.archiveCoordinator.runSharedMany([sessionId], async () => { - const sessionService = new SessionService(this.boundWorkspace); + const sessionService = new SessionService(this.boundWorkspace, { + runtimeBaseDir: this.runtimeBaseDir, + }); let exists = await sessionService.sessionExistsInAnyState(sessionId); if (!exists) { @@ -2078,6 +2127,7 @@ export class AcpDispatcher { } const organization = await createSessionOrganizationService( this.boundWorkspace, + this.runtimeBaseDir, ).updateSessionOrganization(sessionId, { ...(typeof params['isPinned'] === 'boolean' ? { isPinned: params['isPinned'] } @@ -2096,8 +2146,10 @@ export class AcpDispatcher { case `${QWEN_METHOD_NS}workspace/session_groups/list`: { const workspaceCwd = this.parseBoundWorkspaceParam(params); - const groups = - await createSessionOrganizationService(workspaceCwd).listGroups(); + const groups = await createSessionOrganizationService( + workspaceCwd, + this.runtimeBaseDir, + ).listGroups(); this.replyConn(conn, id, groups); return; } @@ -2106,6 +2158,7 @@ export class AcpDispatcher { const workspaceCwd = this.parseBoundWorkspaceParam(params); const group = await createSessionOrganizationService( workspaceCwd, + this.runtimeBaseDir, ).createGroup({ name: params['name'] as string, color: params['color'] as SessionGroupColor, @@ -2122,6 +2175,7 @@ export class AcpDispatcher { } const group = await createSessionOrganizationService( workspaceCwd, + this.runtimeBaseDir, ).updateGroup(groupId, { ...('name' in params ? { name: params['name'] as string } : {}), ...('color' in params @@ -2139,10 +2193,10 @@ export class AcpDispatcher { if (!groupId) { throw new AcpParamError('`groupId` is required'); } - const deleted = - await createSessionOrganizationService(workspaceCwd).deleteGroup( - groupId, - ); + const deleted = await createSessionOrganizationService( + workspaceCwd, + this.runtimeBaseDir, + ).deleteGroup(groupId); this.replyConn(conn, id, { deleted }); return; } @@ -3623,7 +3677,9 @@ export class AcpDispatcher { case `${QWEN_METHOD_NS}sessions/delete`: { const ids = this.parseSessionIds(params); - const svc = new SessionService(this.boundWorkspace); + const svc = new SessionService(this.boundWorkspace, { + runtimeBaseDir: this.runtimeBaseDir, + }); const result = await deleteDaemonSessions({ sessionIds: ids, service: svc, @@ -3645,6 +3701,7 @@ export class AcpDispatcher { const ids = this.parseSessionIds(params); const svc = new SessionService(this.boundWorkspace, { onWarning: logSessionArchiveWarning, + runtimeBaseDir: this.runtimeBaseDir, }); const result = await archiveDaemonSessions({ sessionIds: ids, @@ -3665,6 +3722,7 @@ export class AcpDispatcher { const ids = this.parseSessionIds(params); const svc = new SessionService(this.boundWorkspace, { onWarning: logSessionArchiveWarning, + runtimeBaseDir: this.runtimeBaseDir, }); const result = await unarchiveDaemonSessions({ sessionIds: ids, diff --git a/packages/cli/src/serve/acp-http/index.ts b/packages/cli/src/serve/acp-http/index.ts index 1e8307c2450..d74aebb8783 100644 --- a/packages/cli/src/serve/acp-http/index.ts +++ b/packages/cli/src/serve/acp-http/index.ts @@ -774,6 +774,7 @@ export function mountAcpHttp( opts.sessionShellCommandEnabled === true, registry, opts.archiveCoordinator ?? new SessionArchiveCoordinator(), + opts.workspaceRegistry?.primary.runtimeBaseDir, ); dispatcherRef.current = dispatcher; @@ -1234,6 +1235,7 @@ export function mountAcpHttp( opts.sessionShellCommandEnabled === true, secondaryRegistry, opts.archiveCoordinator ?? new SessionArchiveCoordinator(), + rt.runtimeBaseDir, ); secondaryDispatcherRef.current = secondaryDispatcher; return { diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index 58b6e7ef3c5..738e3fc1d9e 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -192,6 +192,7 @@ class FakeBridge { } loadShouldThrow = false; + loadError: unknown; async loadSession(req: { sessionId: string; @@ -199,6 +200,7 @@ class FakeBridge { clientId?: string; }) { this.loadRequests.push(req); + if (this.loadError) throw this.loadError; if (this.loadShouldThrow) throw new Error('load failed'); if (this.gate) await this.gate; return { @@ -4155,6 +4157,45 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { expect(frame.error.code).toBe(-32603); }); + it.each([ + ['session_writer_conflict', -32016], + ['session_writer_lost', -32017], + ['session_transcript_changed', -32018], + ['session_writer_unavailable', -32019], + ] as const)( + 'session/load preserves %s as a safe structured error', + async (errorKind, code) => { + bridge.loadError = Object.assign(new Error('/private/path leaked'), { + code, + data: { errorKind, details: 'pid=123 host=secret' }, + }); + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await new Promise((r) => setTimeout(r, 50)); + await post(connId, { + jsonrpc: '2.0', + id: 31, + method: 'session/load', + params: { sessionId: 'x' }, + }); + const [frame] = (await got) as Array<{ + id: number; + error: { + code: number; + message: string; + data: Record; + }; + }>; + expect(frame).toMatchObject({ + id: 31, + error: { code, data: { errorKind } }, + }); + expect(frame.error.message).not.toContain('/private/path'); + expect(frame.error.data).not.toHaveProperty('details'); + }, + ); + it('connection teardown detaches the session client from the bridge', async () => { const connId = await initialize(); await newSession(connId); @@ -4542,7 +4583,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }); }); - it('session/close runs local cleanup even if the bridge close throws', async () => { + it('session/close keeps local ownership when the bridge close throws', async () => { bridge.closeShouldThrow = true; const connId = await initialize(); await newSession(connId); // creates + owns sess-1 @@ -4555,9 +4596,20 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }); await new Promise((r) => setTimeout(r, 50)); expect(bridge.closedSessions).toContain('sess-1'); // bridge was called (then threw) - // Local teardown ran in `finally` despite the throw → session unowned now. + // The bridge still owns the child writer, so the connection must retain + // ownership and its binding to make a close retry possible. const after = await openStream(connId, 'sess-1'); - expect(after.status).toBe(403); + expect(after.status).toBe(200); + + bridge.closeShouldThrow = false; + await post(connId, { + jsonrpc: '2.0', + id: 47, + method: 'session/close', + params: { sessionId: 'sess-1' }, + }); + await new Promise((r) => setTimeout(r, 50)); + expect(bridge.closedSessions).toEqual(['sess-1', 'sess-1']); }); it('connection cap → 503 on initialize', async () => { diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index 3af710a338f..439214e00f2 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -178,12 +178,16 @@ function makeSummary( async function writeStoredSession(input: { sessionId: string; cwd: string; + runtimeBaseDir?: string; timestamp: string; prompt: string; mtime: Date; parentSessionId?: string; }): Promise { - const chatsDir = path.join(new Storage(input.cwd).getProjectDir(), 'chats'); + const chatsDir = path.join( + new Storage(input.cwd, input.runtimeBaseDir).getProjectDir(), + 'chats', + ); await fsp.mkdir(chatsDir, { recursive: true }); const filePath = path.join(chatsDir, `${input.sessionId}.jsonl`); const records: Array> = [ @@ -690,6 +694,7 @@ function makeBridge( function makeRuntime(input: { workspaceId: string; workspaceCwd: string; + runtimeBaseDir?: string; primary: boolean; trusted: boolean; bridge: AcpSessionBridge; @@ -736,6 +741,8 @@ function makeHarness(opts?: { token?: string; secondaryRewindImpl?: AcpSessionBridge['rewindSession']; secondaryShellImpl?: AcpSessionBridge['executeShellCommand']; + primaryRuntimeBaseDir?: string; + secondaryRuntimeBaseDir?: string; serveOptions?: Partial; }) { const primaryBridge = makeBridge( @@ -762,6 +769,7 @@ function makeHarness(opts?: { makeRuntime({ workspaceId: 'primary-id', workspaceCwd: PRIMARY_CWD, + runtimeBaseDir: opts?.primaryRuntimeBaseDir, primary: true, trusted: opts?.primaryTrusted ?? true, bridge: primaryBridge, @@ -769,6 +777,7 @@ function makeHarness(opts?: { makeRuntime({ workspaceId: 'secondary-id', workspaceCwd: SECONDARY_CWD, + runtimeBaseDir: opts?.secondaryRuntimeBaseDir, primary: false, trusted: opts?.secondaryTrusted ?? true, bridge: secondaryBridge, @@ -2368,6 +2377,65 @@ describe('multi-workspace session dispatch', () => { }); }); + it('uses the owning runtime base for persisted session reads and maintenance', async () => { + const previousRuntimeBaseDir = Storage.getRuntimeBaseDir(); + const defaultRuntimeBaseDir = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-default-runtime-'), + ); + const secondaryRuntimeBaseDir = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-secondary-runtime-'), + ); + const sessionId = '550e8400-e29b-41d4-a716-446655440159'; + Storage.setRuntimeBaseDir(defaultRuntimeBaseDir); + try { + await writeStoredSession({ + sessionId, + cwd: SECONDARY_CWD, + runtimeBaseDir: secondaryRuntimeBaseDir, + timestamp: '2026-07-08T00:20:00.000Z', + prompt: 'secondary custom runtime target', + mtime: new Date('2026-07-08T00:20:00.000Z'), + }); + const { app } = makeHarness({ + secondarySummaries: [], + secondaryRuntimeBaseDir, + }); + + const listed = await request(app) + .get('/workspaces/secondary-id/sessions') + .set('Host', host()) + .expect(200); + expect( + listed.body.sessions.map( + (session: { sessionId: string }) => session.sessionId, + ), + ).toEqual([sessionId]); + + await request(app) + .post('/workspaces/secondary-id/sessions/archive') + .set('Host', host()) + .send({ sessionIds: [sessionId] }) + .expect(200); + + await expect( + new SessionService(SECONDARY_CWD, { + runtimeBaseDir: secondaryRuntimeBaseDir, + }).getSessionLocation(sessionId), + ).resolves.toBe('archived'); + await expect( + new SessionService(SECONDARY_CWD, { + runtimeBaseDir: defaultRuntimeBaseDir, + }).getSessionLocation(sessionId), + ).resolves.toBeUndefined(); + } finally { + Storage.setRuntimeBaseDir(previousRuntimeBaseDir); + await Promise.all([ + fsp.rm(defaultRuntimeBaseDir, { recursive: true, force: true }), + fsp.rm(secondaryRuntimeBaseDir, { recursive: true, force: true }), + ]); + } + }); + it('lists organized non-primary workspace sessions with pinned first for trusted workspaces', async () => { await withRuntimeDir(async () => { const pinnedOlderId = '550e8400-e29b-41d4-a716-446655440131'; diff --git a/packages/cli/src/serve/routes/scheduled-tasks.test.ts b/packages/cli/src/serve/routes/scheduled-tasks.test.ts index a7f7b0c4dcd..8c3f98b7ba4 100644 --- a/packages/cli/src/serve/routes/scheduled-tasks.test.ts +++ b/packages/cli/src/serve/routes/scheduled-tasks.test.ts @@ -1166,6 +1166,7 @@ describe('scheduledTaskSessionName', () => { interface QualifiedRuntime { workspaceId: string; workspaceCwd: string; + runtimeBaseDir: string; trusted: boolean; bridge: StubBridge; } @@ -1204,10 +1205,12 @@ async function makeQualifiedHarness(): Promise { trusted: boolean, ): Promise => { const workspaceCwd = path.join(scratch, name); + const runtimeBaseDir = path.join(scratch, 'runtimes', name); await fsp.mkdir(workspaceCwd, { recursive: true }); return { workspaceId: `id-${name}`, workspaceCwd, + runtimeBaseDir, trusted, bridge: makeStubBridge(), }; @@ -1225,6 +1228,7 @@ async function makeQualifiedHarness(): Promise { // route. `bridge`-per-runtime comes from the registry. registerScheduledTasksRoutes(app, { boundWorkspace: primary.workspaceCwd, + runtimeBaseDir: primary.runtimeBaseDir, mutate: () => (_req, _res, next) => next(), safeBody, bridge: primary.bridge, @@ -1275,12 +1279,24 @@ describe('workspace-qualified scheduled-tasks routes', () => { .post(qualified(h.secondary.workspaceId)) .send({ cron: '0 9 * * *', prompt: 'p' }); const onDisk = JSON.parse( - await fsp.readFile(getCronFilePath(h.secondary.workspaceCwd), 'utf-8'), + await fsp.readFile( + Storage.runWithRuntimeBaseDir( + h.secondary.runtimeBaseDir, + undefined, + () => getCronFilePath(h.secondary.workspaceCwd), + ), + 'utf-8', + ), ); expect(onDisk).toHaveLength(1); // The primary's file was never created. await expect( - fsp.readFile(getCronFilePath(h.primary.workspaceCwd), 'utf-8'), + fsp.readFile( + Storage.runWithRuntimeBaseDir(h.primary.runtimeBaseDir, undefined, () => + getCronFilePath(h.primary.workspaceCwd), + ), + 'utf-8', + ), ).rejects.toThrow(); }); diff --git a/packages/cli/src/serve/routes/scheduled-tasks.ts b/packages/cli/src/serve/routes/scheduled-tasks.ts index e4c0e386136..6a1a2412ef3 100644 --- a/packages/cli/src/serve/routes/scheduled-tasks.ts +++ b/packages/cli/src/serve/routes/scheduled-tasks.ts @@ -41,6 +41,7 @@ import { nextFireTime, nextDurableFireMs, SessionService, + Storage, stripTerminalControlSequences, MAX_JOBS, type DurableCronTask, @@ -124,9 +125,22 @@ export function scheduledTaskSessionName(label: string): string { */ interface ScheduledTaskTarget { workspaceCwd: string; + runtimeBaseDir?: string; bridge?: ScheduledTasksSessionBridge; } +function runInTargetRuntime( + target: ScheduledTaskTarget, + operation: () => T, +): T { + if (target.runtimeBaseDir === undefined) return operation(); + return Storage.runWithRuntimeBaseDir( + target.runtimeBaseDir, + undefined, + operation, + ); +} + /** * Resolves the target workspace for one request. Returns null when it can't be * resolved (unknown or untrusted `:workspace`), in which case the resolver has @@ -148,6 +162,7 @@ interface RegisterScheduledTaskCrudRoutesDeps { interface RegisterScheduledTasksRoutesDeps { boundWorkspace: string; + runtimeBaseDir?: string; mutate: (opts?: { strict?: boolean }) => RequestHandler; safeBody: (req: Request) => Record; /** @@ -285,7 +300,9 @@ function registerScheduledTaskCrudRoutes( const target = resolveTarget(req, res); if (!target) return; try { - const tasks = await readCronTasks(target.workspaceCwd); + const tasks = await runInTargetRuntime(target, () => + readCronTasks(target.workspaceCwd), + ); res.status(200).json({ v: 1, tasks: tasks.map(toView) }); } catch (err) { // A malformed/corrupt file throws (fix-or-delete contract) rather than @@ -305,7 +322,7 @@ function registerScheduledTaskCrudRoutes( app.post(base, mutate(), async (req, res) => { const target = resolveTarget(req, res); if (!target) return; - const { workspaceCwd, bridge } = target; + const { workspaceCwd, runtimeBaseDir, bridge } = target; const body = safeBody(req); const cron = typeof body['cron'] === 'string' ? body['cron'].trim() : ''; @@ -398,7 +415,10 @@ function registerScheduledTaskCrudRoutes( // an orphan with no owning task. Best-effort — the write-lock cap check // below stays authoritative for the concurrent-create race. try { - if ((await readCronTasks(workspaceCwd)).length >= MAX_SCHEDULED_TASKS) { + if ( + (await runInTargetRuntime(target, () => readCronTasks(workspaceCwd))) + .length >= MAX_SCHEDULED_TASKS + ) { res.status(409).json({ error: `Maximum number of scheduled tasks (${MAX_SCHEDULED_TASKS}) reached`, code: 'max_tasks_reached', @@ -461,7 +481,7 @@ function registerScheduledTaskCrudRoutes( const rollbackSession = async () => { if (boundSessionId !== undefined && bridge) { await bridge.closeSession(boundSessionId).catch(() => {}); - await new SessionService(workspaceCwd) + await new SessionService(workspaceCwd, { runtimeBaseDir }) .removeSession(boundSessionId) .catch(() => {}); } @@ -469,16 +489,18 @@ function registerScheduledTaskCrudRoutes( let overCap = false; try { - await updateCronTasks(workspaceCwd, (tasks) => { - // Cap check under the write lock so two concurrent creates can't both - // slip past a stale count. Returning the input unchanged is a no-op - // (no write), which the flag below turns into a 409. - if (tasks.length >= MAX_SCHEDULED_TASKS) { - overCap = true; - return tasks; - } - return [...tasks, task]; - }); + await runInTargetRuntime(target, () => + updateCronTasks(workspaceCwd, (tasks) => { + // Cap check under the write lock so two concurrent creates can't both + // slip past a stale count. Returning the input unchanged is a no-op + // (no write), which the flag below turns into a 409. + if (tasks.length >= MAX_SCHEDULED_TASKS) { + overCap = true; + return tasks; + } + return [...tasks, task]; + }), + ); } catch (err) { await rollbackSession(); writeStderrLine( @@ -600,84 +622,86 @@ function registerScheduledTaskCrudRoutes( let blockedByArchive = false; let blockedLegacy = false; try { - await updateCronTasks(workspaceCwd, (tasks) => { - const idx = tasks.findIndex((t) => t.id === id); - if (idx === -1) return tasks; // not found → no write - found = true; - const current = tasks[idx]!; - // A legacy guarded task (isolated + precondition, both removed) can't be - // enabled: `toView` reports it disabled, so the only PATCH the Web Shell - // sends for it is the Enable toggle — which would 200 here and then read - // back disabled again, an Enable control that can never succeed with no - // error explaining why. Reject the enable with the recreate remediation - // instead of acknowledging an update that changes nothing runnable. - if (patch.enabled === true && taskHasLegacyCondition(current)) { - blockedLegacy = true; - return tasks; // no write - } - // A task disabled BY archiving its session (`disabledByArchive`) can't - // be re-enabled through this generic PATCH: its bound session is still - // archived and can't fire, so flipping `enabled: true` here would show - // an enabled task with a countdown that never runs. The task/session - // lifecycle must stay coupled — the caller has to unarchive the session - // (which clears the marker and reloads it). Reject and leave the file - // untouched. - if (patch.enabled === true && current.disabledByArchive === true) { - blockedByArchive = true; - return tasks; // no write - } - const next: DurableCronTask = { ...current, ...patch }; - // `name: null/""` clears the field rather than storing an empty name, - // so toView reports it as unnamed and isValidTask never sees a "". - if (clearName) delete next.name; - // Re-seat the task's schedule anchor to "now" whenever an edit would - // otherwise let the scheduler retroactively fire an already-past slot. - const justReEnabled = - current.enabled === false && patch.enabled === true; - // Compare the EFFECTIVE schedule, not the raw string: a cosmetic edit - // (`0 9 * * *` → `00 9 * * *`, whitespace) must not re-seat the anchor - // and drop a legitimately-pending catch-up fire. - const cronChanged = - patch.cron !== undefined && - canonicalCron(patch.cron) !== canonicalCron(current.cron); - const becameRecurring = - patch.recurring === true && current.recurring !== true; - const becameOneShot = - patch.recurring === false && current.recurring !== false; - // Re-seated REGARDLESS of enabled: a schedule edit made while the task - // is paused must not leave a stale anchor that fires retroactively when - // it's later re-enabled in a SEPARATE request (the re-enable patch has no - // schedule change of its own to trigger the re-seat). Re-seating a paused - // task's anchor is harmless — it doesn't fire until enabled. - { - const now = Date.now(); - const minute = now - (now % 60_000); - if ( - next.recurring && - (justReEnabled || cronChanged || becameRecurring) - ) { - // A recurring task's anchor is lastFiredAt: resume from now so a - // re-enable / cron edit / one-shot→recurring flip doesn't retroactively - // fire a past slot (matters most for a bound task, whose catch-up runs - // on every file-watch reload). - next.lastFiredAt = minute; - } else if ( - !next.recurring && - (justReEnabled || cronChanged || becameOneShot) - ) { - // A one-shot's anchor is createdAt. Re-seat it on a schedule change - // (cron edit, or recurring→one-shot) OR a re-enable so the task fires - // at its NEXT occurrence — otherwise the scheduler reads its original - // long-past slot as a MISSED one-shot and fires + permanently deletes - // it. A one-shot disabled past its slot then re-enabled would - // otherwise be silently destroyed on the next reload. - next.createdAt = now; - next.lastFiredAt = minute; + await runInTargetRuntime(target, () => + updateCronTasks(workspaceCwd, (tasks) => { + const idx = tasks.findIndex((t) => t.id === id); + if (idx === -1) return tasks; // not found → no write + found = true; + const current = tasks[idx]!; + // A legacy guarded task (isolated + precondition, both removed) can't be + // enabled: `toView` reports it disabled, so the only PATCH the Web Shell + // sends for it is the Enable toggle — which would 200 here and then read + // back disabled again, an Enable control that can never succeed with no + // error explaining why. Reject the enable with the recreate remediation + // instead of acknowledging an update that changes nothing runnable. + if (patch.enabled === true && taskHasLegacyCondition(current)) { + blockedLegacy = true; + return tasks; // no write } - } - updated = next; - return tasks.map((t, i) => (i === idx ? next : t)); - }); + // A task disabled BY archiving its session (`disabledByArchive`) can't + // be re-enabled through this generic PATCH: its bound session is still + // archived and can't fire, so flipping `enabled: true` here would show + // an enabled task with a countdown that never runs. The task/session + // lifecycle must stay coupled — the caller has to unarchive the session + // (which clears the marker and reloads it). Reject and leave the file + // untouched. + if (patch.enabled === true && current.disabledByArchive === true) { + blockedByArchive = true; + return tasks; // no write + } + const next: DurableCronTask = { ...current, ...patch }; + // `name: null/""` clears the field rather than storing an empty name, + // so toView reports it as unnamed and isValidTask never sees a "". + if (clearName) delete next.name; + // Re-seat the task's schedule anchor to "now" whenever an edit would + // otherwise let the scheduler retroactively fire an already-past slot. + const justReEnabled = + current.enabled === false && patch.enabled === true; + // Compare the EFFECTIVE schedule, not the raw string: a cosmetic edit + // (`0 9 * * *` → `00 9 * * *`, whitespace) must not re-seat the anchor + // and drop a legitimately-pending catch-up fire. + const cronChanged = + patch.cron !== undefined && + canonicalCron(patch.cron) !== canonicalCron(current.cron); + const becameRecurring = + patch.recurring === true && current.recurring !== true; + const becameOneShot = + patch.recurring === false && current.recurring !== false; + // Re-seated REGARDLESS of enabled: a schedule edit made while the task + // is paused must not leave a stale anchor that fires retroactively when + // it's later re-enabled in a SEPARATE request (the re-enable patch has no + // schedule change of its own to trigger the re-seat). Re-seating a paused + // task's anchor is harmless — it doesn't fire until enabled. + { + const now = Date.now(); + const minute = now - (now % 60_000); + if ( + next.recurring && + (justReEnabled || cronChanged || becameRecurring) + ) { + // A recurring task's anchor is lastFiredAt: resume from now so a + // re-enable / cron edit / one-shot→recurring flip doesn't retroactively + // fire a past slot (matters most for a bound task, whose catch-up runs + // on every file-watch reload). + next.lastFiredAt = minute; + } else if ( + !next.recurring && + (justReEnabled || cronChanged || becameOneShot) + ) { + // A one-shot's anchor is createdAt. Re-seat it on a schedule change + // (cron edit, or recurring→one-shot) OR a re-enable so the task fires + // at its NEXT occurrence — otherwise the scheduler reads its original + // long-past slot as a MISSED one-shot and fires + permanently deletes + // it. A one-shot disabled past its slot then re-enabled would + // otherwise be silently destroyed on the next reload. + next.createdAt = now; + next.lastFiredAt = minute; + } + } + updated = next; + return tasks.map((t, i) => (i === idx ? next : t)); + }), + ); } catch (err) { writeStderrLine( `qwen serve: PATCH ${base}/${id} failed: ${err instanceof Error ? err.message : String(err)}`, @@ -749,16 +773,18 @@ function registerScheduledTaskCrudRoutes( let boundSessionId: string | undefined; let removed = false; try { - await updateCronTasks(workspaceCwd, (tasks) => { - const idx = tasks.findIndex((t) => t.id === id); - if (idx === -1) return tasks; // not found → no write - const match = tasks[idx]!.sessionId; - if (typeof match === 'string' && match.length > 0) { - boundSessionId = match; - } - removed = true; - return tasks.filter((_, i) => i !== idx); - }); + await runInTargetRuntime(target, () => + updateCronTasks(workspaceCwd, (tasks) => { + const idx = tasks.findIndex((t) => t.id === id); + if (idx === -1) return tasks; // not found → no write + const match = tasks[idx]!.sessionId; + if (typeof match === 'string' && match.length > 0) { + boundSessionId = match; + } + removed = true; + return tasks.filter((_, i) => i !== idx); + }), + ); } catch (err) { writeStderrLine( `qwen serve: DELETE ${base}/${id} failed: ${err instanceof Error ? err.message : String(err)}`, @@ -808,49 +834,51 @@ function registerScheduledTaskCrudRoutes( let blockedLegacy = false; let updated: DurableCronTask | undefined; try { - await updateCronTasks(workspaceCwd, (tasks) => { - const idx = tasks.findIndex((t) => t.id === id); - if (idx === -1) return tasks; // not found → no write - found = true; - const current = tasks[idx]!; - // A legacy guarded task (isolated + precondition, both removed) must not - // run from ANY path. The scheduler already skips it and the list view - // reports it disabled; reject a direct `/run` too — its on-disk - // `enabled` may still be true, so the disabled check below is not enough. - // Executing it here would run the prompt with its safety gate ignored, - // which is exactly what the removal must never allow. - if (taskHasLegacyCondition(current)) { - blockedLegacy = true; - return tasks; // no write - } - // A disabled task must not record a manual run: it's paused (and if it - // was disabled by archiving its session, that session can't even fire), - // so stamping lastFiredAt + a 'manual' entry would write a phantom "ran" - // record. Mirrors the PATCH route's refusal to re-enable such tasks and - // the UI, where onRunPrompt already rejects before recording. - if (current.enabled === false) { - blockedDisabled = true; - return tasks; // no write - } - const next: DurableCronTask = { - ...current, - lastFiredAt: now, - runs: appendCronRun(current.runs, { - at: now, - kind: 'manual', - ...(current.sessionId ? { sessionId: current.sessionId } : {}), - }), - }; - updated = next; - // A one-shot's manual run IS its single fire — remove it from the store - // so the scheduler doesn't ALSO fire it at its original scheduled time - // (its slot is still in the future, so stamping lastFiredAt=now wouldn't - // stop that fire). The response still returns the recorded run. - if (!current.recurring) { - return tasks.filter((_, i) => i !== idx); - } - return tasks.map((t, i) => (i === idx ? next : t)); - }); + await runInTargetRuntime(target, () => + updateCronTasks(workspaceCwd, (tasks) => { + const idx = tasks.findIndex((t) => t.id === id); + if (idx === -1) return tasks; // not found → no write + found = true; + const current = tasks[idx]!; + // A legacy guarded task (isolated + precondition, both removed) must not + // run from ANY path. The scheduler already skips it and the list view + // reports it disabled; reject a direct `/run` too — its on-disk + // `enabled` may still be true, so the disabled check below is not enough. + // Executing it here would run the prompt with its safety gate ignored, + // which is exactly what the removal must never allow. + if (taskHasLegacyCondition(current)) { + blockedLegacy = true; + return tasks; // no write + } + // A disabled task must not record a manual run: it's paused (and if it + // was disabled by archiving its session, that session can't even fire), + // so stamping lastFiredAt + a 'manual' entry would write a phantom "ran" + // record. Mirrors the PATCH route's refusal to re-enable such tasks and + // the UI, where onRunPrompt already rejects before recording. + if (current.enabled === false) { + blockedDisabled = true; + return tasks; // no write + } + const next: DurableCronTask = { + ...current, + lastFiredAt: now, + runs: appendCronRun(current.runs, { + at: now, + kind: 'manual', + ...(current.sessionId ? { sessionId: current.sessionId } : {}), + }), + }; + updated = next; + // A one-shot's manual run IS its single fire — remove it from the store + // so the scheduler doesn't ALSO fire it at its original scheduled time + // (its slot is still in the future, so stamping lastFiredAt=now wouldn't + // stop that fire). The response still returns the recorded run. + if (!current.recurring) { + return tasks.filter((_, i) => i !== idx); + } + return tasks.map((t, i) => (i === idx ? next : t)); + }), + ); } catch (err) { writeStderrLine( `qwen serve: POST ${base}/${id}/run failed: ${err instanceof Error ? err.message : String(err)}`, @@ -899,10 +927,14 @@ export function registerScheduledTasksRoutes( app: Application, deps: RegisterScheduledTasksRoutesDeps, ): void { - const { boundWorkspace, mutate, safeBody, bridge } = deps; + const { boundWorkspace, runtimeBaseDir, mutate, safeBody, bridge } = deps; registerScheduledTaskCrudRoutes(app, { prefix: '', - resolveTarget: () => ({ workspaceCwd: boundWorkspace, bridge }), + resolveTarget: () => ({ + workspaceCwd: boundWorkspace, + runtimeBaseDir, + bridge, + }), mutate, safeBody, }); @@ -934,6 +966,7 @@ export function registerWorkspaceQualifiedScheduledTasksRoutes( if (!requireTrustedWorkspaceRuntime(runtime, res)) return null; return { workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, // Mirror the primary surface: only bind a session when management is on, // so a bound task always has something to keep it resident + rehydrate it. bridge: manageScheduledTaskSessions ? runtime.bridge : undefined, diff --git a/packages/cli/src/serve/routes/session-telemetry.test.ts b/packages/cli/src/serve/routes/session-telemetry.test.ts index a8f94e5695f..0be6704da99 100644 --- a/packages/cli/src/serve/routes/session-telemetry.test.ts +++ b/packages/cli/src/serve/routes/session-telemetry.test.ts @@ -220,6 +220,7 @@ describe('special session resolver telemetry publication', () => { expect(archiveMocks.assertSessionLoadable).toHaveBeenCalledWith( secondaryCwd, 'secondary-session', + undefined, ); expect(telemetryMocks.setDaemonTelemetryWorkspace).toHaveBeenCalledTimes(1); expect(telemetryMocks.setDaemonTelemetryWorkspace).toHaveBeenCalledWith( @@ -255,10 +256,12 @@ describe('special session resolver telemetry publication', () => { expect(archiveMocks.assertSessionLoadable).toHaveBeenCalledWith( primaryCwd, 'stored-secondary', + undefined, ); expect(archiveMocks.assertSessionLoadable).toHaveBeenCalledWith( secondaryCwd, 'stored-secondary', + undefined, ); expect(telemetryMocks.setDaemonTelemetryWorkspace).toHaveBeenCalledTimes(1); expect(telemetryMocks.setDaemonTelemetryWorkspace).toHaveBeenCalledWith( diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index a8ef3705400..9fe3d642574 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -18,6 +18,7 @@ import { SessionTranscriptCursorCodec, SessionTranscriptReader, SessionTranscriptSnapshotUnavailableError, + Storage, addDaemonRequestAttribute, runWithoutDebugLogSession, type ApprovalMode, @@ -410,6 +411,14 @@ export function registerSessionRoutes( string, SessionTranscriptCursorCodec >(); + const createRuntimeSessionService = ( + runtime: WorkspaceRuntime, + options: { onWarning?: (message: string) => void } = {}, + ) => + new SessionService(runtime.workspaceCwd, { + ...options, + runtimeBaseDir: runtime.runtimeBaseDir, + }); const getTranscriptCursorCodec = ( runtime: WorkspaceRuntime, @@ -574,9 +583,9 @@ export function registerSessionRoutes( return runtime; }; - const hasActivePersistedSessions = async (workspaceCwd: string) => { + const hasActivePersistedSessions = async (runtime: WorkspaceRuntime) => { try { - const page = await new SessionService(workspaceCwd).listSessions({ + const page = await createRuntimeSessionService(runtime).listSessions({ archiveState: 'active', size: 1, }); @@ -626,6 +635,7 @@ export function registerSessionRoutes( target: { route: string; workspaceCwd: string; + runtimeBaseDir?: string; workspaceQualified?: boolean; archiveState?: SessionArchiveState; }, @@ -648,12 +658,21 @@ export function registerSessionRoutes( [sessionId], async () => { if (target.archiveState === 'archived') { - await assertSessionArchived(target.workspaceCwd, sessionId); + await assertSessionArchived( + target.workspaceCwd, + sessionId, + target.runtimeBaseDir, + ); } else { - await assertSessionLoadable(target.workspaceCwd, sessionId); + await assertSessionLoadable( + target.workspaceCwd, + sessionId, + target.runtimeBaseDir, + ); } return exportSessionTranscript({ workspaceCwd: target.workspaceCwd, + runtimeBaseDir: target.runtimeBaseDir, sessionId, format, archiveState: target.archiveState, @@ -968,6 +987,7 @@ export function registerSessionRoutes( const location = await assertSessionLoadable( runtime.workspaceCwd, sessionId, + runtime.runtimeBaseDir, ); return location === 'active'; }; @@ -1257,7 +1277,7 @@ export function registerSessionRoutes( requireZeroAttaches: true, }); if (killed) { - await new SessionService(runtime.workspaceCwd).removeSession( + await createRuntimeSessionService(runtime).removeSession( session.sessionId, ); } @@ -1337,13 +1357,18 @@ export function registerSessionRoutes( const session = await archiveCoordinator.runSharedMany( [sessionId], async () => { - await assertSessionLoadable(workspaceCwd, sessionId); + await assertSessionLoadable( + workspaceCwd, + sessionId, + runtime.runtimeBaseDir, + ); // Recover the persisted parent lineage so the restored live entry // reports it (the bridge otherwise creates the entry without it, and // status calls would show a restored sub-session as top-level). - const metadata = await new SessionService( - workspaceCwd, - ).readCreationMetadata(sessionId); + const metadata = + await createRuntimeSessionService(runtime).readCreationMetadata( + sessionId, + ); return action === 'load' ? await runtime.bridge.loadSession({ sessionId, @@ -1530,6 +1555,7 @@ export function registerSessionRoutes( await handleSessionExport(req, res, { route: 'GET /session/:id/export', workspaceCwd: boundWorkspace, + runtimeBaseDir: workspaceRegistry.primary.runtimeBaseDir, }); }); @@ -1540,6 +1566,7 @@ export function registerSessionRoutes( await handleSessionExport(req, res, { route, workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, workspaceQualified: true, }); }); @@ -1553,6 +1580,7 @@ export function registerSessionRoutes( await handleSessionExport(req, res, { route, workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, workspaceQualified: true, archiveState: 'archived', }); @@ -1654,15 +1682,22 @@ export function registerSessionRoutes( try { const result = await runWithoutDebugLogSession(() => archiveCoordinator.runSharedMany([sessionId], async () => { - const service = new SessionService(runtime.workspaceCwd); + const service = createRuntimeSessionService(runtime); if (cursor === undefined) { - await assertSessionLoadable(runtime.workspaceCwd, sessionId); + await assertSessionLoadable( + runtime.workspaceCwd, + sessionId, + runtime.runtimeBaseDir, + ); } const codec = getTranscriptCursorCodec(runtime); - const reader = new SessionTranscriptReader( - runtime.workspaceCwd, - codec, - ); + const reader = runtime.runtimeBaseDir + ? Storage.runWithRuntimeBaseDir( + runtime.runtimeBaseDir, + undefined, + () => new SessionTranscriptReader(runtime.workspaceCwd, codec), + ) + : new SessionTranscriptReader(runtime.workspaceCwd, codec); let page; try { page = await reader.readPage(sessionId, { @@ -2335,7 +2370,7 @@ export function registerSessionRoutes( const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; try { - const service = new SessionService(boundWorkspace); + const service = createRuntimeSessionService(workspaceRegistry.primary); const result = await deleteDaemonSessions({ sessionIds: uniqueIds, service, @@ -2357,7 +2392,7 @@ export function registerSessionRoutes( const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; - const service = new SessionService(boundWorkspace, { + const service = createRuntimeSessionService(workspaceRegistry.primary, { onWarning: logSessionArchiveWarning, }); @@ -2383,7 +2418,7 @@ export function registerSessionRoutes( const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; - const service = new SessionService(boundWorkspace, { + const service = createRuntimeSessionService(workspaceRegistry.primary, { onWarning: logSessionArchiveWarning, }); @@ -2416,7 +2451,7 @@ export function registerSessionRoutes( const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; try { - const service = new SessionService(runtime.workspaceCwd); + const service = createRuntimeSessionService(runtime); const result = await deleteDaemonSessions({ sessionIds: uniqueIds, service, @@ -2444,7 +2479,7 @@ export function registerSessionRoutes( if (!runtime) return; const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; - const service = new SessionService(runtime.workspaceCwd, { + const service = createRuntimeSessionService(runtime, { onWarning: logSessionArchiveWarning, }); try { @@ -2475,7 +2510,7 @@ export function registerSessionRoutes( if (!runtime) return; const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; - const service = new SessionService(runtime.workspaceCwd, { + const service = createRuntimeSessionService(runtime, { onWarning: logSessionArchiveWarning, }); try { @@ -2533,6 +2568,7 @@ export function registerSessionRoutes( type SessionOrganizationTarget = { workspaceCwd: string; + runtimeBaseDir?: string; bridge: AcpSessionBridge; route: string; }; @@ -2548,7 +2584,9 @@ export function registerSessionRoutes( await archiveCoordinator.runSharedMany([sessionId], async () => { // Organization is workspace-scoped sidecar state, not live-session // metadata. It intentionally applies to persisted and archived sessions. - const sessionService = new SessionService(target.workspaceCwd); + const sessionService = new SessionService(target.workspaceCwd, { + runtimeBaseDir: target.runtimeBaseDir, + }); let exists = await sessionService.sessionExistsInAnyState(sessionId); if (!exists) { try { @@ -2606,6 +2644,7 @@ export function registerSessionRoutes( const organization = await createSessionOrganizationService( target.workspaceCwd, + target.runtimeBaseDir, ).updateSessionOrganization(sessionId, { ...(rawIsPinned !== undefined ? { isPinned: rawIsPinned } : {}), ...(rawGroupId !== undefined @@ -2630,6 +2669,7 @@ export function registerSessionRoutes( app.patch('/session/:id/organization', mutate(), async (req, res) => { await handleSessionOrganizationUpdate(req, res, { workspaceCwd: boundWorkspace, + runtimeBaseDir: workspaceRegistry.primary.runtimeBaseDir, bridge, route: 'PATCH /session/:id/organization', }); @@ -2644,6 +2684,7 @@ export function registerSessionRoutes( if (!runtime) return; await handleSessionOrganizationUpdate(req, res, { workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, bridge: runtime.bridge, route, }); @@ -2660,7 +2701,10 @@ export function registerSessionRoutes( .status(200) .json( await runWorkspaceInspectionWithLogPolicy(runtime, () => - createSessionOrganizationService(runtime.workspaceCwd).listGroups(), + createSessionOrganizationService( + runtime.workspaceCwd, + runtime.runtimeBaseDir, + ).listGroups(), ), ); } catch (err) { @@ -2675,7 +2719,10 @@ export function registerSessionRoutes( if (key === null) return; const body = safeBody(req); try { - const group = await createSessionOrganizationService(key).createGroup({ + const group = await createSessionOrganizationService( + key, + workspaceRegistry.primary.runtimeBaseDir, + ).createGroup({ name: body['name'] as string, color: body['color'] as SessionGroupColor, }); @@ -2696,20 +2743,20 @@ export function registerSessionRoutes( if (key === null) return; const body = safeBody(req); try { - const group = await createSessionOrganizationService(key).updateGroup( - req.params['groupId'] ?? '', - { - ...(Object.prototype.hasOwnProperty.call(body, 'name') - ? { name: body['name'] as string } - : {}), - ...(Object.prototype.hasOwnProperty.call(body, 'color') - ? { color: body['color'] as SessionGroupColor } - : {}), - ...(Object.prototype.hasOwnProperty.call(body, 'order') - ? { order: body['order'] as number } - : {}), - }, - ); + const group = await createSessionOrganizationService( + key, + workspaceRegistry.primary.runtimeBaseDir, + ).updateGroup(req.params['groupId'] ?? '', { + ...(Object.prototype.hasOwnProperty.call(body, 'name') + ? { name: body['name'] as string } + : {}), + ...(Object.prototype.hasOwnProperty.call(body, 'color') + ? { color: body['color'] as SessionGroupColor } + : {}), + ...(Object.prototype.hasOwnProperty.call(body, 'order') + ? { order: body['order'] as number } + : {}), + }); res.status(200).json({ group }); } catch (err) { if (sendSessionOrganizationError(res, err)) return; @@ -2727,9 +2774,10 @@ export function registerSessionRoutes( const key = resolveWorkspaceParam(req, res); if (key === null) return; try { - const deleted = await createSessionOrganizationService(key).deleteGroup( - req.params['groupId'] ?? '', - ); + const deleted = await createSessionOrganizationService( + key, + workspaceRegistry.primary.runtimeBaseDir, + ).deleteGroup(req.params['groupId'] ?? ''); res.status(200).json({ deleted }); } catch (err) { if (sendSessionOrganizationError(res, err)) return; @@ -2749,7 +2797,10 @@ export function registerSessionRoutes( .status(200) .json( await runWorkspaceInspectionWithLogPolicy(runtime, () => - createSessionOrganizationService(runtime.workspaceCwd).listGroups(), + createSessionOrganizationService( + runtime.workspaceCwd, + runtime.runtimeBaseDir, + ).listGroups(), ), ); } catch (err) { @@ -2768,6 +2819,7 @@ export function registerSessionRoutes( try { const group = await createSessionOrganizationService( runtime.workspaceCwd, + runtime.runtimeBaseDir, ).createGroup({ name: body['name'] as string, color: body['color'] as SessionGroupColor, @@ -2791,6 +2843,7 @@ export function registerSessionRoutes( try { const group = await createSessionOrganizationService( runtime.workspaceCwd, + runtime.runtimeBaseDir, ).updateGroup(req.params['groupId'] ?? '', { ...(Object.prototype.hasOwnProperty.call(body, 'name') ? { name: body['name'] as string } @@ -2820,6 +2873,7 @@ export function registerSessionRoutes( try { const deleted = await createSessionOrganizationService( runtime.workspaceCwd, + runtime.runtimeBaseDir, ).deleteGroup(req.params['groupId'] ?? ''); res.status(200).json({ deleted }); } catch (err) { @@ -2944,7 +2998,7 @@ export function registerSessionRoutes( parsedSource.sourceType !== undefined || (cursor !== undefined && cursor !== '' ? isNumericSessionCursor(cursor) - : await hasActivePersistedSessions(key)); + : await hasActivePersistedSessions(runtime)); // The live path only reads cursor/size; persisted-only options // (organized view or archived state) would be silently dropped there. // usePersisted already routes those to the persisted path — assert it so @@ -2964,6 +3018,7 @@ export function registerSessionRoutes( ? await runWorkspaceInspectionWithLogPolicy(runtime, () => listWorkspaceSessionsForResponse(runtime.bridge, key, options, { mergeLive: !readOnlySecondary, + runtimeBaseDir: runtime.runtimeBaseDir, }), ) : listLiveWorkspaceSessionsForResponse(runtime.bridge, key, options); @@ -3025,6 +3080,7 @@ export function registerSessionRoutes( const info = await runWorkspaceInspectionWithLogPolicy(runtime, () => getWorkspaceSessionInfoForResponse(runtime.bridge, key, { includeLive: !isReadOnlyWorkspaceInspection(runtime), + runtimeBaseDir: runtime.runtimeBaseDir, }), ); res.status(200).json(info); diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index bb7bac9e01b..d6835e83a76 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -3472,6 +3472,11 @@ async function runQwenServeImpl( { workspaceId: daemonWorkspaceHash, workspaceCwd: boundWorkspace, + runtimeBaseDir: core.Storage.runWithRuntimeBaseDir( + runtimeBootSettings?.merged.advanced?.runtimeOutputDir, + boundWorkspace, + () => core.Storage.getRuntimeBaseDir(), + ), primary: true, trusted: trustedWorkspace, removable: false, @@ -3766,6 +3771,11 @@ async function runQwenServeImpl( workspaceRuntimes.push({ workspaceId: secondaryWorkspaceHash, workspaceCwd: workspaceInput.cwd, + runtimeBaseDir: core.Storage.runWithRuntimeBaseDir( + secondarySettings?.merged.advanced?.runtimeOutputDir, + workspaceInput.cwd, + () => core.Storage.getRuntimeBaseDir(), + ), primary: false, trusted: secondaryTrusted, removable: workspaceInput.removable, @@ -4160,6 +4170,11 @@ async function runQwenServeImpl( return { workspaceId: wsHash, workspaceCwd: cwd, + runtimeBaseDir: core.Storage.runWithRuntimeBaseDir( + wsSettings?.merged.advanced?.runtimeOutputDir, + cwd, + () => core.Storage.getRuntimeBaseDir(), + ), primary: false, trusted, removable: true, diff --git a/packages/cli/src/serve/scheduled-task-keepalive.test.ts b/packages/cli/src/serve/scheduled-task-keepalive.test.ts index ca36a796738..5a326745fdc 100644 --- a/packages/cli/src/serve/scheduled-task-keepalive.test.ts +++ b/packages/cli/src/serve/scheduled-task-keepalive.test.ts @@ -734,13 +734,20 @@ describe('scheduled-task keepalive', () => { task({ id: 'ok', sessionId: 'healthy-sess', prompt: 'fine' }), ]); let releaseSpawn: (() => void) | undefined; + let releaseClose: (() => void) | undefined; + const removeSpy = vi + .spyOn(SessionService.prototype, 'removeSession') + .mockResolvedValue(true); const hungBridge = { ...bridge, spawnOrAttach: () => new Promise<{ sessionId: string }>((resolve) => { releaseSpawn = () => resolve({ sessionId: 'late-sess' }); }), - closeSession: async () => {}, + closeSession: () => + new Promise((resolve) => { + releaseClose = resolve; + }), updateSessionMetadata: () => {}, }; const ka = startScheduledTaskKeepalive({ @@ -756,5 +763,10 @@ describe('scheduled-task keepalive', () => { ka.stop(); // Clean up the hung spawn. releaseSpawn?.(); + await vi.waitFor(() => expect(releaseClose).toBeDefined()); + expect(removeSpy).not.toHaveBeenCalled(); + releaseClose?.(); + await vi.waitFor(() => expect(removeSpy).toHaveBeenCalledWith('late-sess')); + removeSpy.mockRestore(); }); }); diff --git a/packages/cli/src/serve/scheduled-task-keepalive.ts b/packages/cli/src/serve/scheduled-task-keepalive.ts index 54f22e24f61..9f114706029 100644 --- a/packages/cli/src/serve/scheduled-task-keepalive.ts +++ b/packages/cli/src/serve/scheduled-task-keepalive.ts @@ -36,6 +36,7 @@ import { getCronFilePath, createDebugLogger, SessionService, + Storage, taskHasLegacyCondition, type DurableCronTask, } from '@qwen-code/qwen-code-core'; @@ -105,6 +106,14 @@ const KEEPALIVE_SPAWN_TIMEOUT_MS = 30_000; * every interval forever. */ const MAX_REVIVE_BACKOFF_MS = 30 * 60_000; +function runInRuntimeBase( + runtimeBaseDir: string | undefined, + operation: () => T, +): T { + if (runtimeBaseDir === undefined) return operation(); + return Storage.runWithRuntimeBaseDir(runtimeBaseDir, undefined, operation); +} + /** * Bind unbound durable tasks to dedicated sessions, and rename bound * sessions that don't yet have the ⏰ prefix. The cron_create tool leaves @@ -122,6 +131,7 @@ const MAX_REVIVE_BACKOFF_MS = 30 * 60_000; async function bindAndNameSessions( bridge: KeepaliveBridge, boundWorkspace: string, + runtimeBaseDir: string | undefined, tasks: readonly DurableCronTask[], renamed: Set, spawnTimeoutMs: number, @@ -158,15 +168,15 @@ async function bindAndNameSessions( // binding guard on TRUE settlement so retries are possible. let timedOut = false; rawSpawn - .then(({ sessionId }) => { + .then(async ({ sessionId }) => { if (timedOut) { log.debug( 'keepalive: late spawn resolved, cleaning up', task.id, sessionId, ); - bridge.closeSession(sessionId).catch(() => {}); - new SessionService(boundWorkspace) + await bridge.closeSession(sessionId).catch(() => {}); + await new SessionService(boundWorkspace, { runtimeBaseDir }) .removeSession(sessionId) .catch(() => {}); } @@ -193,26 +203,28 @@ async function bindAndNameSessions( // naming is non-critical — the session still fires correctly } let matched = false; - await updateCronTasks(boundWorkspace, (list) => { - // Another process may have bound or disabled this task between our - // read and this write-lock acquisition — only attach when the task is - // still unbound and enabled. Otherwise return unchanged so the - // orphan spawn is rolled back below. - if ( - !list.some( - (t) => t.id === task.id && !t.sessionId && t.enabled !== false, - ) - ) { - return list; - } - const result = list.map((t) => - t.id === task.id && !t.sessionId && t.enabled !== false - ? { ...t, sessionId } - : t, - ); - matched = true; - return result; - }); + await runInRuntimeBase(runtimeBaseDir, () => + updateCronTasks(boundWorkspace, (list) => { + // Another process may have bound or disabled this task between our + // read and this write-lock acquisition — only attach when the task is + // still unbound and enabled. Otherwise return unchanged so the + // orphan spawn is rolled back below. + if ( + !list.some( + (t) => t.id === task.id && !t.sessionId && t.enabled !== false, + ) + ) { + return list; + } + const result = list.map((t) => + t.id === task.id && !t.sessionId && t.enabled !== false + ? { ...t, sessionId } + : t, + ); + matched = true; + return result; + }), + ); if (!matched) { // Task was deleted between read and write — roll back the orphan. throw new Error(`task ${task.id} no longer on disk`); @@ -227,7 +239,7 @@ async function bindAndNameSessions( log.debug('keepalive: failed to bind task', task.id, err); if (spawnedSessionId !== undefined) { await bridge.closeSession(spawnedSessionId).catch(() => {}); - await new SessionService(boundWorkspace) + await new SessionService(boundWorkspace, { runtimeBaseDir }) .removeSession(spawnedSessionId) .catch(() => {}); } @@ -257,6 +269,7 @@ export interface ScheduledTaskKeepalive { export interface StartScheduledTaskKeepaliveOptions { bridge: KeepaliveBridge; boundWorkspace: string; + runtimeBaseDir?: string; /** How often to heartbeat; must be comfortably under the reaper timeout. */ intervalMs: number; /** Per-session revive timeout; defaults to KEEPALIVE_REVIVE_TIMEOUT_MS. */ @@ -268,7 +281,7 @@ export interface StartScheduledTaskKeepaliveOptions { export function startScheduledTaskKeepalive( opts: StartScheduledTaskKeepaliveOptions, ): ScheduledTaskKeepalive { - const { bridge, boundWorkspace, intervalMs } = opts; + const { bridge, boundWorkspace, intervalMs, runtimeBaseDir } = opts; const reviveTimeoutMs = opts.reviveTimeoutMs ?? KEEPALIVE_REVIVE_TIMEOUT_MS; const spawnTimeoutMs = opts.spawnTimeoutMs ?? KEEPALIVE_SPAWN_TIMEOUT_MS; @@ -296,7 +309,9 @@ export function startScheduledTaskKeepalive( const tick = async (): Promise => { let tasks; try { - tasks = await readCronTasks(boundWorkspace); + tasks = await runInRuntimeBase(runtimeBaseDir, () => + readCronTasks(boundWorkspace), + ); } catch (err) { // A read failure (missing file already maps to [], so this is a real // EACCES/corruption) just skips this pass; the next one retries. The @@ -323,9 +338,9 @@ export function startScheduledTaskKeepalive( } log.debug('keepalive: recordHeartbeat failed for', sessionId, err); reviving.add(sessionId); - const metadata = await new SessionService( - boundWorkspace, - ).readCreationMetadata(sessionId); + const metadata = await new SessionService(boundWorkspace, { + runtimeBaseDir, + }).readCreationMetadata(sessionId); const load = bridge.loadSession({ sessionId, workspaceCwd: boundWorkspace, @@ -378,6 +393,7 @@ export function startScheduledTaskKeepalive( await bindAndNameSessions( bridge, boundWorkspace, + runtimeBaseDir, tasks, renamed, spawnTimeoutMs, @@ -404,7 +420,9 @@ export function startScheduledTaskKeepalive( // dedicated session immediately, not after the next interval. Same // directory-watch + debounce pattern the scheduler uses. let bindDebounce: ReturnType | undefined; - const cronFilePath = getCronFilePath(boundWorkspace); + const cronFilePath = runInRuntimeBase(runtimeBaseDir, () => + getCronFilePath(boundWorkspace), + ); const cronDir = path.dirname(cronFilePath); const cronFileName = path.basename(cronFilePath); let fileWatcher: ReturnType | undefined; @@ -486,6 +504,7 @@ const REHYDRATE_MAX_CONCURRENCY = 4; export async function rehydrateScheduledTaskSessions(deps: { bridge: RehydrateBridge; boundWorkspace: string; + runtimeBaseDir?: string; onError?: (sessionId: string, err: unknown) => void; loadTimeoutMs?: number; }): Promise { @@ -493,7 +512,9 @@ export async function rehydrateScheduledTaskSessions(deps: { const timeoutMs = deps.loadTimeoutMs ?? REHYDRATE_LOAD_TIMEOUT_MS; let tasks; try { - tasks = await readCronTasks(boundWorkspace); + tasks = await runInRuntimeBase(deps.runtimeBaseDir, () => + readCronTasks(boundWorkspace), + ); } catch (err) { log.debug('rehydrate: readCronTasks failed', err); return { loaded: [], failed: [] }; @@ -505,9 +526,9 @@ export async function rehydrateScheduledTaskSessions(deps: { const loaded: string[] = []; const failed: string[] = []; const loadOne = async (sessionId: string) => { - const metadata = await new SessionService( - boundWorkspace, - ).readCreationMetadata(sessionId); + const metadata = await new SessionService(boundWorkspace, { + runtimeBaseDir: deps.runtimeBaseDir, + }).readCreationMetadata(sessionId); const load = bridge.loadSession({ sessionId, workspaceCwd: boundWorkspace, diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 41fa453b234..c7db7ed1fa6 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -7,7 +7,7 @@ import express from 'express'; import type { Application } from 'express'; import type { DaemonStatusProvider } from '@qwen-code/acp-bridge'; -import { hashDaemonWorkspace } from '@qwen-code/qwen-code-core'; +import { hashDaemonWorkspace, Storage } from '@qwen-code/qwen-code-core'; import type { DaemonLogger } from './daemon-logger.js'; import type { DaemonMetricsBucket, @@ -882,6 +882,7 @@ export function createServeApp( { workspaceId: hashDaemonWorkspace(boundWorkspace), workspaceCwd: boundWorkspace, + runtimeBaseDir: Storage.getRuntimeBaseDir(), primary: true, trusted: deps.primaryWorkspaceTrusted ?? false, env: primaryRuntimeEnvMetadata ?? { @@ -1551,6 +1552,7 @@ export function createServeApp( // get UNBOUND tasks (shared-owner firing) instead. registerScheduledTasksRoutes(app, { boundWorkspace: primaryBoundWorkspace, + runtimeBaseDir: primaryRuntime.runtimeBaseDir, mutate, safeBody, bridge: deps.manageScheduledTaskSessions ? bridge : undefined, @@ -1597,13 +1599,11 @@ export function createServeApp( // restart (a bound task fires only in its own session, which nothing else // reloads). Fire-and-forget so it never delays the server coming up; a // no-op when there are no bound tasks. Deliberately not awaited. - const rehydrateWorkspace = ( - taskBridge: AcpSessionBridge, - workspaceCwd: string, - ) => { + const rehydrateWorkspace = (runtime: WorkspaceRuntime) => { void rehydrateScheduledTaskSessions({ - bridge: taskBridge, - boundWorkspace: workspaceCwd, + bridge: runtime.bridge, + boundWorkspace: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, onError: (sessionId, err) => { process.stderr.write( `qwen serve: failed to rehydrate scheduled-task session ${sessionId}: ${ @@ -1636,9 +1636,10 @@ export function createServeApp( const keepalive = startScheduledTaskKeepalive({ bridge: runtime.bridge, boundWorkspace: runtime.workspaceCwd, + runtimeBaseDir: runtime.runtimeBaseDir, intervalMs: keepaliveIntervalMs, }); - rehydrateWorkspace(runtime.bridge, runtime.workspaceCwd); + rehydrateWorkspace(runtime); keepaliveStops.set(runtime.workspaceCwd, keepalive.stop); }; for (const runtime of workspaceRegistry.list()) { diff --git a/packages/cli/src/serve/server/error-response.test.ts b/packages/cli/src/serve/server/error-response.test.ts new file mode 100644 index 00000000000..1a8591062b6 --- /dev/null +++ b/packages/cli/src/serve/server/error-response.test.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Response } from 'express'; +import { describe, expect, it, vi } from 'vitest'; +import { + SessionTranscriptChangedError, + SessionWriterConflictError, + SessionWriterLostError, + SessionWriterUnavailableError, +} from '@qwen-code/qwen-code-core'; +import { sendBridgeError } from './error-response.js'; + +describe('sendBridgeError session writer errors', () => { + it.each([ + [new SessionWriterConflictError(), 409, 'session_writer_conflict'], + [new SessionWriterLostError(), 409, 'session_writer_lost'], + [new SessionTranscriptChangedError(), 409, 'session_transcript_changed'], + [new SessionWriterUnavailableError(), 503, 'session_writer_unavailable'], + ] as const)('maps %s to HTTP %i', (error, expectedStatus, errorKind) => { + const json = vi.fn(); + const status = vi.fn(() => ({ json })); + + sendBridgeError({ status } as unknown as Response, error); + + expect(status).toHaveBeenCalledWith(expectedStatus); + expect(json).toHaveBeenCalledWith({ + error: error.message, + code: errorKind, + errorKind, + }); + }); + + it('does not forward structural ACP writer error details', () => { + const json = vi.fn(); + const status = vi.fn(() => ({ json })); + + sendBridgeError({ status } as unknown as Response, { + message: "EACCES: '/private/transcripts/session.jsonl'", + data: { errorKind: 'session_writer_unavailable' }, + }); + + expect(status).toHaveBeenCalledWith(503); + expect(json).toHaveBeenCalledWith({ + error: 'Session write ownership could not be verified.', + code: 'session_writer_unavailable', + errorKind: 'session_writer_unavailable', + }); + }); +}); diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index 961d87ac5aa..eb5abfeb1c6 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -12,6 +12,7 @@ import { SessionTranscriptPageTooLargeError, SessionTranscriptSnapshotUnavailableError, SessionTranscriptTooLargeError, + SessionWriterError, TrustGateError, } from '@qwen-code/qwen-code-core'; import type { Response } from 'express'; @@ -67,6 +68,15 @@ export type SendBridgeError = ( ctx?: BridgeErrorContext, ) => void; +const SESSION_WRITER_ERROR_MESSAGES = { + session_writer_conflict: + 'This session is already open in another Qwen process.', + session_writer_lost: 'Write ownership for this session was lost.', + session_transcript_changed: + 'The session transcript changed outside its active writer.', + session_writer_unavailable: 'Session write ownership could not be verified.', +} as const; + function bridgeErrorExtraContext( ctx: BridgeErrorContext | undefined, ): Record { @@ -158,6 +168,14 @@ export function sendBridgeError( ctx?: BridgeErrorContext, daemonLog?: DaemonLogger, ): void { + if (err instanceof SessionWriterError) { + res.status(err.httpStatus).json({ + error: err.message, + code: err.errorKind, + errorKind: err.errorKind, + }); + return; + } if (err instanceof WorkspaceSkillNotFoundError) { res.status(404).json({ error: err.message, @@ -546,6 +564,26 @@ export function sendBridgeError( const data = (err as { data?: unknown }).data; if (data && typeof data === 'object') { const kind = (data as { errorKind?: unknown }).errorKind; + if ( + kind === 'session_writer_conflict' || + kind === 'session_writer_lost' || + kind === 'session_transcript_changed' + ) { + res.status(409).json({ + error: SESSION_WRITER_ERROR_MESSAGES[kind], + code: kind, + errorKind: kind, + }); + return; + } + if (kind === 'session_writer_unavailable') { + res.status(503).json({ + error: SESSION_WRITER_ERROR_MESSAGES[kind], + code: kind, + errorKind: kind, + }); + return; + } if (kind === 'mcp_budget_would_exceed') { const d = data as { serverName?: string }; res.status(409).json({ diff --git a/packages/cli/src/serve/server/session-archive.test.ts b/packages/cli/src/serve/server/session-archive.test.ts index 9e1153dc08b..3410e78dbd0 100644 --- a/packages/cli/src/serve/server/session-archive.test.ts +++ b/packages/cli/src/serve/server/session-archive.test.ts @@ -476,17 +476,41 @@ describe('deleteDaemonSessions', () => { }, ]); + const closeSession = vi.fn().mockResolvedValue(undefined); const result = await deleteDaemonSessions({ sessionIds: [sessionId], service: new SessionService(workspaceDir), - bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + bridge: { closeSession }, coordinator: new SessionArchiveCoordinator(), }); expect(result.removed).toEqual([sessionId]); + expect(closeSession).toHaveBeenCalledWith(sessionId, undefined, { + requireAgentClose: true, + }); const ids = (await readCronTasks(workspaceDir)).map((t) => t.id).sort(); expect(ids).toEqual(['other']); // bound task deleted, unbound survives }); + + it('preserves the transcript when strict live-session close fails', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440071'; + writeSessionFile(workspaceDir, sessionId, 'active'); + + const result = await deleteDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { + closeSession: vi.fn().mockRejectedValue(new Error('flush failed')), + }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result.removed).toEqual([]); + expect(result.errors).toEqual([{ sessionId, error: 'flush failed' }]); + expect(fs.existsSync(sessionPath(workspaceDir, sessionId, 'active'))).toBe( + true, + ); + }); }); function writeSessionFile( diff --git a/packages/cli/src/serve/server/session-archive.ts b/packages/cli/src/serve/server/session-archive.ts index 15abaa48329..9ff40e88354 100644 --- a/packages/cli/src/serve/server/session-archive.ts +++ b/packages/cli/src/serve/server/session-archive.ts @@ -6,6 +6,7 @@ import { SessionService, + Storage, type SessionLocation, } from '@qwen-code/qwen-code-core'; import type { AcpSessionBridge } from '../acp-session-bridge.js'; @@ -136,7 +137,9 @@ export async function deleteDaemonSessions(params: { let shouldRemove = false; try { // Intentional: batch delete bypasses per-tab ownership. - await bridge.closeSession(sessionId); + await bridge.closeSession(sessionId, undefined, { + requireAgentClose: true, + }); shouldRemove = true; } catch (closeErr) { if ( @@ -190,15 +193,17 @@ export async function deleteDaemonSessions(params: { // the archive/unarchive paths) — the session is already gone, so a swallowed // write failure leaves the still-enabled bound task a permanent ghost the // keepalive retries a doomed revive on every tick. - await removeTasksForSessions(service.getProjectRoot(), removed).catch( - (err: unknown) => { - logSessionArchiveWarning( - `removeTasksForSessions failed for [${removed.join(', ')}]: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - }, - ); + await Storage.runWithRuntimeBaseDir( + service.getRuntimeBaseDir(), + undefined, + () => removeTasksForSessions(service.getProjectRoot(), removed), + ).catch((err: unknown) => { + logSessionArchiveWarning( + `removeTasksForSessions failed for [${removed.join(', ')}]: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + }); return { removed, notFound, errors: [...closeErrors, ...removeErrors] }; } @@ -206,10 +211,11 @@ export async function deleteDaemonSessions(params: { export async function assertSessionLoadable( workspaceCwd: string, sessionId: string, + runtimeBaseDir?: string, ): Promise { - const location = await new SessionService(workspaceCwd).getSessionLocation( - sessionId, - ); + const location = await new SessionService(workspaceCwd, { + runtimeBaseDir, + }).getSessionLocation(sessionId); if (location === 'archived') { throw new SessionArchivedError(sessionId); } @@ -222,10 +228,11 @@ export async function assertSessionLoadable( export async function assertSessionArchived( workspaceCwd: string, sessionId: string, + runtimeBaseDir?: string, ): Promise { - const location = await new SessionService(workspaceCwd).getSessionLocation( - sessionId, - ); + const location = await new SessionService(workspaceCwd, { + runtimeBaseDir, + }).getSessionLocation(sessionId); if (location === 'active') { throw new SessionNotArchivedError(sessionId); } @@ -431,15 +438,17 @@ export async function archiveDaemonSessions(params: { // keepalive still sees it enabled + bound and will revive the just-archived // session so the task keeps firing. Logging makes that broken coupling // diagnosable rather than silent. - await disableTasksForSessions(service.getProjectRoot(), archived).catch( - (err: unknown) => { - logSessionArchiveWarning( - `disableTasksForSessions failed for [${archived.join(', ')}]: ${ - err instanceof Error ? err.message : String(err) - } — bound tasks may keep firing until reconciled`, - ); - }, - ); + await Storage.runWithRuntimeBaseDir( + service.getRuntimeBaseDir(), + undefined, + () => disableTasksForSessions(service.getProjectRoot(), archived), + ).catch((err: unknown) => { + logSessionArchiveWarning( + `disableTasksForSessions failed for [${archived.join(', ')}]: ${ + err instanceof Error ? err.message : String(err) + } — bound tasks may keep firing until reconciled`, + ); + }); return { archived, alreadyArchived, notFound, errors }; } @@ -508,7 +517,11 @@ export async function unarchiveDaemonSessions(params: { // swallowing, so a stranded task isn't left silent. const resumeSessionIds = [...new Set([...unarchived, ...alreadyActive])]; try { - await enableTasksForSessions(service.getProjectRoot(), resumeSessionIds); + await Storage.runWithRuntimeBaseDir( + service.getRuntimeBaseDir(), + undefined, + () => enableTasksForSessions(service.getProjectRoot(), resumeSessionIds), + ); } catch (err) { logSessionArchiveWarning( `enableTasksForSessions failed for [${resumeSessionIds.join(', ')}]: ${ diff --git a/packages/cli/src/serve/server/session-export.ts b/packages/cli/src/serve/server/session-export.ts index 50952c94d7c..ad407ed9e89 100644 --- a/packages/cli/src/serve/server/session-export.ts +++ b/packages/cli/src/serve/server/session-export.ts @@ -73,13 +73,16 @@ export function sessionExportFormatValues(): SessionExportFormat[] { export async function exportSessionTranscript(params: { workspaceCwd: string; + runtimeBaseDir?: string; sessionId: string; format: SessionExportFormat; archiveState?: SessionArchiveState; config?: ExportConfig; }): Promise { const { workspaceCwd, sessionId, format } = params; - const service = new SessionService(workspaceCwd); + const service = new SessionService(workspaceCwd, { + runtimeBaseDir: params.runtimeBaseDir, + }); const sessionData = params.archiveState === 'archived' ? await service.loadArchivedSession(sessionId, { diff --git a/packages/cli/src/serve/server/session-list.ts b/packages/cli/src/serve/server/session-list.ts index ee51a282329..1fa05e6b150 100644 --- a/packages/cli/src/serve/server/session-list.ts +++ b/packages/cli/src/serve/server/session-list.ts @@ -72,6 +72,8 @@ export interface WorkspaceSessionInfoResult { export interface ListWorkspaceSessionsReadOptions { /** Merge live bridge state into persisted summaries. */ mergeLive?: boolean; + /** Runtime output base pinned by the owning workspace runtime. */ + runtimeBaseDir?: string; } export class InvalidCursorError extends Error { @@ -450,8 +452,13 @@ async function listOrganizedWorkspaceSessionsForResponse( readOptions: ListWorkspaceSessionsReadOptions, ): Promise { const archiveState = options.archiveState ?? 'active'; - const sessionService = new SessionService(workspaceCwd); - const organizationService = createSessionOrganizationService(workspaceCwd); + const sessionService = new SessionService(workspaceCwd, { + runtimeBaseDir: readOptions.runtimeBaseDir, + }); + const organizationService = createSessionOrganizationService( + workspaceCwd, + readOptions.runtimeBaseDir, + ); const snapshot = await organizationService.readSnapshot(); const knownGroupIds = new Set(snapshot.groups.map((group) => group.id)); const group = options.group ?? 'all'; @@ -596,7 +603,9 @@ async function listWorkspaceSessionsByMetadataForResponse( readOptions: ListWorkspaceSessionsReadOptions, ): Promise { const archiveState = options.archiveState ?? 'active'; - const sessionService = new SessionService(workspaceCwd); + const sessionService = new SessionService(workspaceCwd, { + runtimeBaseDir: readOptions.runtimeBaseDir, + }); const bySessionId = new Map(); const persisted = await listAllPersistedSummaries( sessionService, @@ -736,7 +745,9 @@ export async function listWorkspaceSessionsForResponse( } const isFirstPage = numericCursor === undefined; - const sessionService = new SessionService(workspaceCwd); + const sessionService = new SessionService(workspaceCwd, { + runtimeBaseDir: readOptions.runtimeBaseDir, + }); const archiveState = options?.archiveState ?? 'active'; const persisted = await sessionService.listSessions({ cursor: numericCursor, @@ -841,9 +852,11 @@ export function listLiveWorkspaceSessionsForResponse( export async function getWorkspaceSessionInfoForResponse( bridge: AcpSessionBridge, workspaceCwd: string, - options: { includeLive?: boolean } = {}, + options: { includeLive?: boolean; runtimeBaseDir?: string } = {}, ): Promise { - const counts = await new SessionService(workspaceCwd).getSessionInfoCounts(); + const counts = await new SessionService(workspaceCwd, { + runtimeBaseDir: options.runtimeBaseDir, + }).getSessionInfoCounts(); return { active: counts.active, archived: counts.archived, diff --git a/packages/cli/src/serve/session-organization-helpers.ts b/packages/cli/src/serve/session-organization-helpers.ts index f131368bc42..1c9f19af071 100644 --- a/packages/cli/src/serve/session-organization-helpers.ts +++ b/packages/cli/src/serve/session-organization-helpers.ts @@ -4,13 +4,24 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { SessionOrganizationService } from '@qwen-code/qwen-code-core'; +import { SessionOrganizationService, Storage } from '@qwen-code/qwen-code-core'; import { writeStderrLine } from '../utils/stdioHelpers.js'; export function createSessionOrganizationService( workspaceCwd: string, + runtimeBaseDir?: string, ): SessionOrganizationService { - return new SessionOrganizationService(workspaceCwd, (message) => { - writeStderrLine(`qwen serve: session-org: ${message}`); - }); + if (runtimeBaseDir === undefined) { + return new SessionOrganizationService(workspaceCwd, (message) => { + writeStderrLine(`qwen serve: session-org: ${message}`); + }); + } + return Storage.runWithRuntimeBaseDir( + runtimeBaseDir, + undefined, + () => + new SessionOrganizationService(workspaceCwd, (message) => { + writeStderrLine(`qwen serve: session-org: ${message}`); + }), + ); } diff --git a/packages/cli/src/serve/workspace-registry.ts b/packages/cli/src/serve/workspace-registry.ts index 98b60fdd764..fedd99b8511 100644 --- a/packages/cli/src/serve/workspace-registry.ts +++ b/packages/cli/src/serve/workspace-registry.ts @@ -28,6 +28,8 @@ export interface WorkspaceRuntimeEnvMetadata { export interface WorkspaceRuntime { readonly workspaceId: string; readonly workspaceCwd: string; + /** Absolute runtime output base pinned when this workspace runtime starts. */ + readonly runtimeBaseDir?: string; readonly primary: boolean; readonly trusted: boolean; /** Whether this runtime may be removed without restarting the daemon. */ diff --git a/packages/cli/src/ui/AppContainer.test.tsx b/packages/cli/src/ui/AppContainer.test.tsx index a4023810e36..083be6e2d32 100644 --- a/packages/cli/src/ui/AppContainer.test.tsx +++ b/packages/cli/src/ui/AppContainer.test.tsx @@ -54,6 +54,7 @@ import { makeFakeConfig, type GeminiClient, type SubagentManager, + SessionWriterConflictError, } from '@qwen-code/qwen-code-core'; import type { LoadedSettings } from '../config/settings.js'; import type { InitializationResult } from '../core/initializer.js'; @@ -470,9 +471,11 @@ describe('AppContainer State Management', () => { }; fileRewindError?: Error; noGeminiClient?: boolean; + recordingFlushErrorAt?: 'pre' | 'post'; }; const renderRewindHarness = (options: RewindHarnessOptions = {}) => { + vi.spyOn(mockConfig, 'assertCanStartTurn').mockResolvedValue(undefined); const history: HistoryItem[] = [ rewindUserItem(1, 'first prompt', 'prompt-1'), { id: 2, type: 'gemini', text: 'first response' }, @@ -540,8 +543,22 @@ describe('AppContainer State Management', () => { } as unknown as ReturnType); const rewindRecording = vi.fn(); + const flush = vi.fn(); + if (options.recordingFlushErrorAt === 'pre') { + flush.mockRejectedValue(new Error('EIO before rewind')); + } else if (options.recordingFlushErrorAt === 'post') { + flush + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('EIO after rewind')); + } else { + flush.mockResolvedValue(undefined); + } + const integrityFailure = new Error('Session write state is uncertain'); + const markIntegrityFailure = vi.fn().mockReturnValue(integrityFailure); vi.spyOn(mockConfig, 'getChatRecordingService').mockReturnValue({ rewindRecording, + flush, + markIntegrityFailure, } as unknown as NonNullable>); render( @@ -562,6 +579,8 @@ describe('AppContainer State Management', () => { getHistoryShallow, truncateHistory, rewindRecording, + flush, + markIntegrityFailure, snapshots, }; }; @@ -576,6 +595,40 @@ describe('AppContainer State Management', () => { }; describe('Basic Rendering', () => { + it('surfaces interactive config initialization failures', async () => { + const error = new SessionWriterConflictError(); + let rejectInitialization!: (error: Error) => void; + vi.spyOn(mockConfig, 'initialize').mockReturnValue( + new Promise((_resolve, reject) => { + rejectInitialization = reject; + }), + ); + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + + try { + render( + , + ); + + await vi.waitFor(() => { + expect(mockConfig.initialize).toHaveBeenCalledOnce(); + }); + rejectInitialization(error); + await vi.waitFor(() => { + expect(stderrSpy).toHaveBeenCalledWith(`${error.message}\n`); + }); + } finally { + stderrSpy.mockRestore(); + } + }); + it('continues quitting when cancelling the active request fails', () => { vi.useFakeTimers(); const cancelOngoingRequest = vi.fn(() => { @@ -4201,6 +4254,54 @@ describe('AppContainer State Management', () => { { truncatedCount: 2 }, harness.snapshots.slice(0, 2), ); + expect(harness.flush).toHaveBeenCalledTimes(2); + expect(harness.flush.mock.invocationCallOrder[1]).toBeLessThan( + harness.truncateHistory.mock.invocationCallOrder[0]!, + ); + }); + + it('fails closed before changing UI when rewind persistence is ambiguous', async () => { + const harness = renderRewindHarness({ + recordingFlushErrorAt: 'post', + }); + + await runRewind(harness.target, 'conversation'); + + expect(harness.rewindRecording).toHaveBeenCalledOnce(); + expect(harness.markIntegrityFailure).toHaveBeenCalledWith( + 'rewind_persistence', + ); + expect(harness.truncateHistory).not.toHaveBeenCalled(); + expect(harness.loadHistory).not.toHaveBeenCalled(); + expect(harness.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'error', + text: 'Rewind failed: Session write state is uncertain', + }), + expect.any(Number), + ); + }); + + it('does not start rewind when the pre-flight recording flush fails', async () => { + const harness = renderRewindHarness({ + recordingFlushErrorAt: 'pre', + }); + + await runRewind(harness.target, 'both'); + + expect(harness.flush).toHaveBeenCalledOnce(); + expect(harness.rewindRecording).not.toHaveBeenCalled(); + expect(harness.rewind).not.toHaveBeenCalled(); + expect(harness.truncateHistory).not.toHaveBeenCalled(); + expect(harness.loadHistory).not.toHaveBeenCalled(); + expect(harness.setText).not.toHaveBeenCalled(); + expect(harness.addItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'error', + text: 'Rewind failed: Session recording is degraded; rewind was not applied.', + }), + expect.any(Number), + ); }); it('shows an error and returns for conversation-only rewind with no client', async () => { diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index 514cbf6cbb6..719c2e192cd 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -153,7 +153,7 @@ import { formatSessionWindowTitle, writeTerminalTitle, } from '../utils/windowTitle.js'; -import { clearScreen } from '../utils/stdioHelpers.js'; +import { clearScreen, writeStderrLine } from '../utils/stdioHelpers.js'; import { useTextBuffer } from './components/shared/text-buffer.js'; import { useLogger } from './hooks/useLogger.js'; import { @@ -588,6 +588,9 @@ export const AppContainer = (props: AppContainerProps) => { const [currentModel, setCurrentModel] = useState(() => config.getModel()); const [isConfigInitialized, setConfigInitialized] = useState(false); + const [configInitializationError, setConfigInitializationError] = useState< + string | null + >(null); const [userMessages, setUserMessages] = useState([]); @@ -675,6 +678,7 @@ export const AppContainer = (props: AppContainerProps) => { * parent checkout. (PR #4174 review #3259975249.) */ const pendingWorktreeNoticeRef = useRef(null); + const configInitializationStartedRef = useRef(false); const activeWorktree = useMemo( () => worktreeSession @@ -727,11 +731,19 @@ export const AppContainer = (props: AppContainerProps) => { // Initialize config (runs once on mount) useEffect(() => { - (async () => { - // Note: the program will not work if this fails so let errors be - // handled by the global catch. + if (configInitializationStartedRef.current) return; + configInitializationStartedRef.current = true; + void (async () => { profileCheckpoint('config_initialize_start'); - await config.initialize(); + try { + await config.initialize(); + } catch (error) { + const message = getErrorMessage(error); + writeStderrLine(message); + setConfigInitializationError(message); + return; + } + config.startRuntimeStatus(); setStartupWarnings((currentWarnings) => mergeStartupWarnings(currentWarnings, config.getWarnings()), ); @@ -1870,7 +1882,7 @@ export const AppContainer = (props: AppContainerProps) => { const { streamingState, submitQuery, - initError, + initError: geminiInitializationError, pendingHistoryItems: pendingGeminiHistoryItems, thought, cancelOngoingRequest, @@ -1907,6 +1919,7 @@ export const AppContainer = (props: AppContainerProps) => { terminalWidthRef, midTurnRestoreRef, ); + const initError = configInitializationError ?? geminiInitializationError; cancelOngoingRequestRef.current = cancelOngoingRequest; // Now that streamingState is available, keep isIdleRef in sync and @@ -3157,6 +3170,21 @@ export const AppContainer = (props: AppContainerProps) => { const geminiClient = needsConversation ? config.getGeminiClient() : null; + const conversationRecording = needsConversation + ? config.getChatRecordingService() + : undefined; + if (needsConversation && geminiClient) { + await config.assertCanStartTurn(); + if (conversationRecording) { + try { + await conversationRecording.flush(); + } catch { + throw new Error( + 'Session recording is degraded; rewind was not applied.', + ); + } + } + } let apiTruncateIndex = -1; let conversationSkippedNoClient = false; if (needsConversation) { @@ -3271,13 +3299,34 @@ export const AppContainer = (props: AppContainerProps) => { if (isRealUserTurn(h)) targetTurnIndex++; } - geminiClient.truncateHistory(apiTruncateIndex); - // Strip suppressOnRestore flags and filter out collapse-summary items // so rewound items remain visible without stale summary text const truncatedUi = expandCollapsedHistory( originalHistory.filter((h) => h.id < userItem.id), ); + const survivingSnapshots = !hasRestoreFailure + ? config + .getFileHistoryService() + .getSnapshots() + .slice(0, targetTurnIndex + 1) + : undefined; + + if (conversationRecording) { + conversationRecording.rewindRecording( + targetTurnIndex, + { truncatedCount: effectiveLength - truncatedUi.length }, + survivingSnapshots, + ); + try { + await conversationRecording.flush(); + } catch { + throw conversationRecording.markIntegrityFailure( + 'rewind_persistence', + ); + } + } + + geminiClient.truncateHistory(apiTruncateIndex); historyManager.loadHistory(truncatedUi); refreshStatic(); @@ -3295,17 +3344,6 @@ export const AppContainer = (props: AppContainerProps) => { }, Date.now(), ); - - config.getChatRecordingService()?.rewindRecording( - targetTurnIndex, - { truncatedCount: effectiveLength - truncatedUi.length }, - !hasRestoreFailure - ? config - .getFileHistoryService() - .getSnapshots() - .slice(0, targetTurnIndex + 1) - : undefined, - ); } // Show file restore result after conversation truncation so the diff --git a/packages/cli/src/ui/commands/clearCommand.test.ts b/packages/cli/src/ui/commands/clearCommand.test.ts index 57f69fbf1ee..ba97a3e5e69 100644 --- a/packages/cli/src/ui/commands/clearCommand.test.ts +++ b/packages/cli/src/ui/commands/clearCommand.test.ts @@ -39,6 +39,7 @@ describe('clearCommand', () => { let mockResetBackgroundTasks: ReturnType; let mockResetMonitors: ReturnType; let mockResetBackgroundShells: ReturnType; + let mockDebugWarn: ReturnType; beforeEach(() => { mockResetChat = vi.fn().mockResolvedValue(undefined); @@ -55,6 +56,7 @@ describe('clearCommand', () => { mockResetBackgroundTasks = vi.fn(); mockResetMonitors = vi.fn(); mockResetBackgroundShells = vi.fn(); + mockDebugWarn = vi.fn(); vi.clearAllMocks(); mockContext = createMockCommandContext({ @@ -76,9 +78,19 @@ describe('clearCommand', () => { abortAll: mockAbortBackgroundShells, }), startNewSession: mockStartNewSession, + prepareSessionTransition: vi.fn(async () => { + const sessionId = mockStartNewSession(); + await mockResetChat(); + return { + sessionId, + sessionData: undefined, + commit: async (uiCommit: () => void) => uiCommit(), + rollback: vi.fn().mockResolvedValue(undefined), + }; + }), getHookSystem: mockGetHookSystem, getDebugLogger: () => ({ - warn: vi.fn(), + warn: mockDebugWarn, }), getModel: () => 'test-model', getToolRegistry: () => undefined, @@ -103,6 +115,13 @@ describe('clearCommand', () => { }); }); + it('does not advertise an ACP session-id switch that the protocol cannot commit', () => { + expect(clearCommand.supportedModes).toEqual([ + 'interactive', + 'non_interactive', + ]); + }); + it('should set debug message, start a new session, reset chat, and clear UI when config is available', async () => { if (!clearCommand.action) { throw new Error('clearCommand must have an action.'); @@ -144,7 +163,7 @@ describe('clearCommand', () => { expect(mockFireSessionStartEvent).not.toHaveBeenCalled(); }); - it('aborts old background work before starting a new session', async () => { + it('aborts old background work after the committed session switch', async () => { if (!clearCommand.action) { throw new Error('clearCommand must have an action.'); } @@ -154,14 +173,18 @@ describe('clearCommand', () => { expect(mockAbortBackgroundTasks).toHaveBeenCalledWith({ notify: false }); expect(mockAbortMonitors).toHaveBeenCalledWith({ notify: false }); expect(mockAbortBackgroundShells).toHaveBeenCalledTimes(1); - expect(mockAbortBackgroundTasks.mock.invocationCallOrder[0]).toBeLessThan( - mockStartNewSession.mock.invocationCallOrder[0], + expect( + mockAbortBackgroundTasks.mock.invocationCallOrder[0], + ).toBeGreaterThan( + mockContext.session.startNewSession.mock.invocationCallOrder[0], ); - expect(mockAbortMonitors.mock.invocationCallOrder[0]).toBeLessThan( - mockStartNewSession.mock.invocationCallOrder[0], + expect(mockAbortMonitors.mock.invocationCallOrder[0]).toBeGreaterThan( + mockContext.session.startNewSession.mock.invocationCallOrder[0], ); - expect(mockAbortBackgroundShells.mock.invocationCallOrder[0]).toBeLessThan( - mockResetBackgroundShells.mock.invocationCallOrder[0], + expect( + mockAbortBackgroundShells.mock.invocationCallOrder[0], + ).toBeGreaterThan( + mockContext.session.startNewSession.mock.invocationCallOrder[0], ); expect(mockAbortBackgroundTasks.mock.invocationCallOrder[0]).toBeLessThan( mockResetBackgroundTasks.mock.invocationCallOrder[0], @@ -169,14 +192,33 @@ describe('clearCommand', () => { expect(mockAbortMonitors.mock.invocationCallOrder[0]).toBeLessThan( mockResetMonitors.mock.invocationCallOrder[0], ); - expect(mockResetBackgroundShells.mock.invocationCallOrder[0]).toBeLessThan( - mockStartNewSession.mock.invocationCallOrder[0], - ); - expect(mockResetBackgroundTasks.mock.invocationCallOrder[0]).toBeLessThan( - mockStartNewSession.mock.invocationCallOrder[0], + expect(mockAbortBackgroundShells.mock.invocationCallOrder[0]).toBeLessThan( + mockResetBackgroundShells.mock.invocationCallOrder[0], ); - expect(mockResetMonitors.mock.invocationCallOrder[0]).toBeLessThan( - mockStartNewSession.mock.invocationCallOrder[0], + }); + + it('keeps the committed session when post-commit cleanup fails', async () => { + if (!clearCommand.action) { + throw new Error('clearCommand must have an action.'); + } + const rollback = vi.fn().mockResolvedValue(undefined); + const config = mockContext.services.config!; + vi.mocked(config.prepareSessionTransition).mockResolvedValueOnce({ + sessionId: 'new-session-id', + sessionData: undefined, + commit: vi.fn(async (uiCommit: () => void) => uiCommit()), + rollback, + }); + mockAbortBackgroundTasks.mockImplementationOnce(() => { + throw new Error('cleanup failed'); + }); + + await clearCommand.action(mockContext, ''); + + expect(rollback).not.toHaveBeenCalled(); + expect(mockContext.ui.clear).toHaveBeenCalledOnce(); + expect(mockDebugWarn).toHaveBeenCalledWith( + expect.stringContaining('Failed to reset background state after /clear'), ); }); @@ -226,7 +268,7 @@ describe('clearCommand', () => { expect(mockContext.ui.clear).toHaveBeenCalledTimes(1); }); - it('should clear UI before resetChat for immediate responsiveness', async () => { + it('should initialize the target core before committing the UI clear', async () => { if (!clearCommand.action) { throw new Error('clearCommand must have an action.'); } @@ -243,12 +285,11 @@ describe('clearCommand', () => { await clearCommand.action(mockContext, ''); - // ui.clear should be called before resetChat for immediate UI feedback const clearIndex = callOrder.indexOf('ui.clear'); const resetIndex = callOrder.indexOf('resetChat'); expect(clearIndex).toBeGreaterThanOrEqual(0); expect(resetIndex).toBeGreaterThanOrEqual(0); - expect(clearIndex).toBeLessThan(resetIndex); + expect(resetIndex).toBeLessThan(clearIndex); }); it('should not await hook events (fire-and-forget)', async () => { @@ -335,6 +376,16 @@ describe('clearCommand', () => { abortAll: mockAbortBackgroundShells, }), startNewSession: mockStartNewSession, + prepareSessionTransition: vi.fn(async () => { + const sessionId = mockStartNewSession(); + await mockResetChat(); + return { + sessionId, + sessionData: undefined, + commit: async (uiCommit: () => void) => uiCommit(), + rollback: vi.fn().mockResolvedValue(undefined), + }; + }), getGeminiClient: vi.fn().mockReturnValue({ resetChat: mockResetChat, } as unknown as GeminiClient), diff --git a/packages/cli/src/ui/commands/clearCommand.ts b/packages/cli/src/ui/commands/clearCommand.ts index 4dd3ecadf13..28e071e9d82 100644 --- a/packages/cli/src/ui/commands/clearCommand.ts +++ b/packages/cli/src/ui/commands/clearCommand.ts @@ -13,6 +13,7 @@ import { ToolNames, persistSessionUsage, createDebugLogger, + SessionStartSource, } from '@qwen-code/qwen-code-core'; import { hasBlockingBackgroundWork, @@ -29,7 +30,7 @@ export const clearCommand: SlashCommand = { return t('Clear conversation history and free up context'); }, kind: CommandKind.BUILT_IN, - supportedModes: ['interactive', 'non_interactive', 'acp'] as const, + supportedModes: ['interactive', 'non_interactive'] as const, action: async (context, _args) => { const { config } = context.services; @@ -65,13 +66,6 @@ export const clearCommand: SlashCommand = { config.getDebugLogger().warn(`SessionEnd hook failed: ${err}`); }); - // Abort old-session async work before creating the new session so - // cancellation notifications cannot leak across the reset boundary. - config.getBackgroundTaskRegistry().abortAll({ notify: false }); - config.getMonitorRegistry().abortAll({ notify: false }); - config.getBackgroundShellRegistry().abortAll(); - resetBackgroundStateForSessionSwitch(config); - // Persist current session's usage before resetting metrics const metrics = uiTelemetryService.getMetrics(); const hasActivity = Object.values(metrics.models).some( @@ -91,37 +85,56 @@ export const clearCommand: SlashCommand = { } } - const newSessionId = config.startNewSession(); - - // Reset UI telemetry metrics for the new session - uiTelemetryService.reset(); - - // Clear loaded-skills tracking so /context doesn't show stale data + const hadUnpersistedRecording = + config.getChatRecordingService?.()?.hasWriteFailure?.() ?? false; + const transition = await config.prepareSessionTransition( + undefined, + undefined, + { + sessionStartSource: SessionStartSource.Clear, + requireNew: true, + }, + ); + try { + await transition.commit(() => { + context.session.startNewSession?.(transition.sessionId); + context.ui.clear(); + context.ui.setDebugMessage( + hadUnpersistedRecording + ? t( + 'Started a new session. Some changes in the previous session could not be saved.', + ) + : t( + 'Starting a new session, resetting chat, and clearing terminal.', + ), + ); + }); + } catch (error) { + await transition.rollback(); + throw error; + } + try { + config.getBackgroundTaskRegistry().abortAll({ notify: false }); + config.getMonitorRegistry().abortAll({ notify: false }); + config.getBackgroundShellRegistry().abortAll(); + resetBackgroundStateForSessionSwitch(config); + } catch (error) { + config + .getDebugLogger() + .warn(`Failed to reset background state after /clear: ${error}`); + } const skillTool = config .getToolRegistry() ?.getAllTools() .find((tool) => tool.name === ToolNames.SKILL); if (skillTool && 'clearLoadedSkills' in skillTool) { - (skillTool as { clearLoadedSkills(): void }).clearLoadedSkills(); - } - - if (newSessionId && context.session.startNewSession) { - context.session.startNewSession(newSessionId); - } - - // Clear UI first for immediate responsiveness - context.ui.clear(); - - const geminiClient = config.getGeminiClient(); - if (geminiClient) { - context.ui.setDebugMessage( - t('Starting a new session, resetting chat, and clearing terminal.'), - ); - // If resetChat fails, the exception will propagate and halt the command, - // which is the correct behavior to signal a failure to the user. - await geminiClient.resetChat(); - } else { - context.ui.setDebugMessage(t('Starting a new session and clearing.')); + try { + (skillTool as { clearLoadedSkills(): void }).clearLoadedSkills(); + } catch (error) { + config + .getDebugLogger() + .warn(`Failed to clear loaded skills after /clear: ${error}`); + } } } else { context.ui.setDebugMessage(t('Starting a new session and clearing.')); diff --git a/packages/cli/src/ui/components/Footer.test.tsx b/packages/cli/src/ui/components/Footer.test.tsx index 394afba0070..489efa2d322 100644 --- a/packages/cli/src/ui/components/Footer.test.tsx +++ b/packages/cli/src/ui/components/Footer.test.tsx @@ -385,6 +385,19 @@ describe('