diff --git a/docs/design/session-writer-lease-p0a.md b/docs/design/session-writer-lease-p0a.md new file mode 100644 index 00000000000..4a5f0079a5e --- /dev/null +++ b/docs/design/session-writer-lease-p0a.md @@ -0,0 +1,80 @@ +# Session writer lease P0a + +## Problem + +A persisted session can currently be loaded by a second Qwen process while the original process is still producing and recording a turn. Both recorders cache the same parent UUID. When they append independently, the JSONL transcript gains two unmarked children from that parent. Resume follows the physical tail and can therefore hide the first process's complete answer. + +The production incident had exactly this ordering: the original process recorded a tool result, the daemon fresh-loaded that session, the original process recorded the remaining tool work and final answer, and the daemon later recorded a user message using the earlier tool result as its parent. + +## Scope + +P0a establishes one cross-process writer for each ACP/daemon `(runtime base, session ID)` and protects the ordinary linear append path involved in this incident. It includes: + +- an atomic owner-token lease with dead-process recovery; +- an authoritative transcript reload after lease acquisition; +- owner, file-identity, and byte-length fencing on every JSONL append; +- turn admission before user, cron, notification, and teammate work starts; +- reuse of an already-live session inside one daemon; +- owner-barrier reads for live transcript replay and Desktop history refresh; +- deterministic ACP/HTTP conflict errors; and +- lease draining and release on session close and failed initialization. + +P0a does not make session switching, rewind, branch/fork, working-directory migration, archive/delete/rename maintenance, or transcript repair transactional. It also does not introduce an initializing registry entry that serializes every same-daemon load/resume against close; a repeated load reuses the owner after that owner is registered, while the cross-process lease still rejects a second writer during initialization. Full load/close outcome coalescing belongs to P0b. Session switching and persistence-root migration fail closed while an ACP Config owns a lease. ACP's logical working-directory change remains supported because it keeps the recorder and SessionService bound to the original persistence root. Same-owner rewind loads through that Config-pinned SessionService under the recorder write barrier; rename and branch retain their existing recorder or flush-before-copy paths. Daemon archive/delete and maintenance of non-live sessions retain their existing semantics. Concurrent maintenance from outside the live owner remains unsupported and is part of the P0b boundary. Interactive and headless CLI recorders retain their existing unleased behavior so `/clear`, `/resume`, `/branch`, and `/cd` do not regress; they must not write the same session concurrently with an ACP owner until P0b broadens the protocol. + +## Invariants + +1. At most one cooperating ACP process owns a session writer lease under a runtime base. +2. A leased ACP recorder is inactive until it owns the lease and has reloaded the transcript while holding it. +3. Preview data loaded before the lease is never the recorder's authoritative tail. +4. Every leased ACP append verifies the owner token and the expected transcript file identity, metadata, and byte length. +5. An ownership or transcript-integrity failure permanently rejects later top-level turns in that leased ACP Config. +6. A daemon never constructs a second writable Config for a session already live in that daemon. +7. A live entry is removed only after its recorder has drained and released the lease. +8. Runtime output roots are pinned per Config so the lock and transcript cannot resolve through different async workspace contexts. + +## Lease protocol + +The lock is stored at: + +```text +/tmp/session-writer-locks/.lock +``` + +Its immutable record contains a random owner token, PID, host, process kind, acquisition time, Qwen version, and (when available) a stable OS process-start identity. Linux uses the kernel boot ID plus the process start ticks, so wall-clock corrections cannot make a live owner appear stale. Darwin normalizes the process-start probe to the C locale and UTC so two processes with different environments compare the same identity. The identity distinguishes PID reuse when the platform exposes it reliably. A foreign-host owner and any state whose safety cannot be proven fail closed. + +Acquisition creates a fully written temporary record and links it into the lock name atomically. A valid live owner returns `session_writer_conflict`. A valid dead local owner can be renamed, rechecked, and reclaimed. Reclaim guards form bounded owner generations so another process can recover if a reclaimer itself crashes. A malformed, symlink, or non-regular lock returns `session_writer_unavailable` rather than being guessed stale. + +The lease snapshots whether the transcript exists, its file identity and metadata, and its byte length. `appendJsonLine` checks the immutable owner record and snapshot immediately before writing through the same file handle, then advances the expected state only after a successful durable append and post-write path verification. New transcript creation uses exclusive creation. + +## Activation and close + +An ACP `Config.initialize()` acquires the lease before extension, hook, tool, model, or scheduler initialization. While holding the lease it resolves active/archive state, reloads the active transcript when one exists, verifies that the transcript did not change during the reload, replaces any pre-lock preview, and activates the recorder. Non-ACP Configs continue through the legacy recorder path without acquiring this P0a lease. + +Any later initialization failure closes the recorder and releases the lease. Normal shutdown and ACP session close finalize pending metadata, drain the recorder queue, release the owner token, and only then remove the live session entry. Cleanup is identity-checked so a failed older initialization cannot close a newer same-ID entry, and an unreturned Config whose first release fails is retried before the daemon creates another fresh session. A definitive child refusal leaves the session live so close can be retried. Close draining is bounded; a timeout or transport failure has an unknown result, so the bridge terminates the shared ACP channel and its process-owned leases become recoverable as stale. Other sessions on that channel are also reaped by that recovery action. + +## Error contract + +| Kind | JSON-RPC | HTTP | Meaning | +| ---------------------------- | -------: | ---: | ------------------------------------------------------- | +| `session_writer_conflict` | `-32020` | 409 | Another live process owns the session. | +| `session_writer_lost` | `-32021` | 409 | This Config no longer owns its lock. | +| `session_transcript_changed` | `-32022` | 409 | The JSONL changed outside the expected append sequence. | +| `session_writer_unavailable` | `-32023` | 503 | Ownership could not be verified safely. | + +External responses use fixed messages and `errorKind`; they do not expose PID, host, owner token, lock path, or transcript path. + +## Compatibility and rollout + +The protocol only coordinates ACP binaries that understand it. Deployment and rollback must drain old ACP/daemon writer processes before the new version starts. Mixed-version ACP operation is not safe because an old writer ignores the lock. Concurrent interactive or headless access to the same persisted session remains outside P0a and is unsupported until P0b. + +The runtime filesystem must support same-directory hard links with atomic no-replace behavior. If that prerequisite is unavailable, acquisition fails closed with `session_writer_unavailable`. + +Existing branched transcripts are not automatically repaired. P0a prevents a new stale-load branch after rollout; repair and explicit branch semantics remain separate work. + +## Verification + +Unit coverage exercises lock contention, dead-owner and crashed-reclaimer recovery, malformed and non-regular locks, concurrent and retryable owner-token release, truncated and externally changed transcripts, equal-length file replacement, UTF-8 byte accounting, recorder activation/fencing/close, authoritative reload, initialization cleanup, runtime-root pinning, turn admission, same-daemon replay reuse, disabled-recording compatibility, legacy interactive recorder behavior, and error sanitization. Darwin coverage also verifies that processes with different time zones derive the same owner identity. PID-reuse handling is implemented but is not claimed as test evidence because process-start probing is platform dependent. + +A real two-process regression recreates the incident timing: process A holds the writer after a tool-result tail, process B is rejected before loading as a writer, A appends its final answer and closes, and B then acquires, reloads that final answer, and appends the next user record with the final answer as its parent. + +Desktop coverage verifies that a writer conflict is surfaced to the user instead of silently replacing the requested persisted session with a fresh session. Live history refresh is served through the owner's write barrier and Config-pinned SessionService, including after a logical `/cd`. diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index d61501b2b81..c9e9be588f2 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -12737,7 +12737,11 @@ describe('createAcpSessionBridge', () => { expect(handle.agent.extMethodCalls).toHaveLength(1); expect(handle.agent.extMethodCalls[0]).toEqual({ method: 'qwen/control/session/close', - params: { sessionId: session.sessionId, requireFlush: true }, + params: { + sessionId: session.sessionId, + drainTimeoutMs: 8_000, + requireFlush: true, + }, }); expect(bridge.sessionCount).toBe(1); expect(() => @@ -12827,6 +12831,134 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('resolves pending permissions before waiting for agent close during kill', async () => { + let capturedConn: AgentSideConnection | undefined; + const permissionResponse: { current?: Promise } = {}; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: async (method) => { + if (method !== 'qwen/control/session/close') return {}; + const result = (await permissionResponse.current) as { + outcome: { outcome: string }; + }; + expect(result.outcome.outcome).toBe('cancelled'); + return {}; + }, + }); + capturedConn = new AgentSideConnection(() => fakeAgent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { + exitCode: number | null; + signalCode: NodeJS.Signals | null; + } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + const bridge = makeBridge({ channelFactory: factory }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + permissionResponse.current = ( + capturedConn as unknown as { + requestPermission(p: unknown): Promise; + } + ).requestPermission({ + sessionId: session.sessionId, + toolCall: { toolCallId: 'tc-kill', title: 'dangerous command' }, + options: [ + { optionId: 'allow', name: 'Allow', kind: 'allow_once' }, + { optionId: 'deny', name: 'Deny', kind: 'reject_once' }, + ], + }); + + await vi.waitFor(() => expect(bridge.pendingPermissionCount).toBe(1)); + await expect(bridge.killSession(session.sessionId)).resolves.toBe(true); + await expect(permissionResponse.current).resolves.toMatchObject({ + outcome: { outcome: 'cancelled' }, + }); + expect(bridge.pendingPermissionCount).toBe(0); + expect(bridge.sessionCount).toBe(0); + + await bridge.shutdown(); + }); + + it('force-kills the channel when kill follows a stuck acknowledged close', async () => { + const closeStarted = deferred(); + const closeGate = deferred>(); + const handle = makeChannel({ + extMethodImpl: async (method) => { + if (method !== 'qwen/control/session/close') return {}; + closeStarted.resolve(); + return closeGate.promise; + }, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const closeResult = bridge.closeSession(session.sessionId).then( + () => 'resolved' as const, + () => 'rejected' as const, + ); + await closeStarted.promise; + + await expect(bridge.killSession(session.sessionId)).resolves.toBe(true); + await expect(closeResult).resolves.toBe('rejected'); + await vi.waitFor(() => expect(bridge.sessionCount).toBe(0)); + expect(handle.killed).toBe(true); + + await bridge.shutdown(); + }); + + it('keeps multiplexed siblings live while one close is still draining', async () => { + const firstCloseGate = deferred>(); + const handle = makeChannel({ + extMethodImpl: async (method) => { + if (method !== 'qwen/control/session/close') return {}; + return firstCloseGate.promise; + }, + }); + const bridge = makeBridge({ + channelFactory: async () => handle.channel, + sessionScope: 'thread', + initializeTimeoutMs: 20, + }); + const first = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const sibling = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const close = bridge.closeSession(first.sessionId); + await vi.waitFor(() => + expect(handle.agent.extMethodCalls).toContainEqual({ + method: 'qwen/control/session/close', + params: { + sessionId: first.sessionId, + drainTimeoutMs: 16, + }, + }), + ); + await new Promise((resolve) => setTimeout(resolve, 40)); + + expect(handle.killed).toBe(false); + expect(bridge.sessionCount).toBe(2); + expect(() => + bridge.recordHeartbeat(sibling.sessionId, { + clientId: sibling.clientId, + }), + ).not.toThrow(); + + firstCloseGate.resolve({}); + await close; + expect(bridge.sessionCount).toBe(1); + + await bridge.shutdown(); + }); + it('routes per-entry channel bookkeeping via channelInfoForEntry, not the module-scoped channelInfo (#4325)', async () => { // Regression guard for #4325 (wenshao review on F1 #4319). // diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 3a461d201e4..91a2d86393d 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -9,6 +9,7 @@ import * as path from 'node:path'; import { ClientSideConnection, PROTOCOL_VERSION, + RequestError, } from '@agentclientprotocol/sdk'; import type { CancelNotification, @@ -197,6 +198,16 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +function isDefinitiveAcpRequestError(error: unknown): boolean { + if (error instanceof RequestError) return true; + if (!isRecord(error)) return false; + return ( + typeof error['code'] === 'number' && + Number.isInteger(error['code']) && + typeof error['message'] === 'string' + ); +} + function getCanonicalModelId(response: unknown, fallback: string): string { if (!isRecord(response) || !isRecord(response['_meta'])) return fallback; const modelSwitch = response['_meta']['qwenModelSwitch']; @@ -445,6 +456,8 @@ interface SessionEntry { artifacts: SessionArtifactStore; /** Sticky in-memory health state for the session's transcript recorder. */ recordingDegraded: boolean; + /** Set synchronously while agent-owned state and its writer lease close. */ + closing: boolean; /** * Tail of the per-session prompt queue. Each new prompt chains off the * resolved (or rejected) state of this promise so prompts run one at a @@ -3186,7 +3199,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { entry: SessionEntry, ci: ChannelInfo | undefined, label: 'closeSession' | 'killSession', - opts?: { throwOnFailure?: boolean; requireFlush?: boolean }, + opts?: { + throwOnFailure?: boolean; + requireFlush?: boolean; + timeoutMs?: number; + }, ): Promise => { if (!ci || ci.channel !== entry.channel) { if (opts?.throwOnFailure === true) { @@ -3201,15 +3218,25 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return; } try { + const closeRequest = entry.connection.extMethod( + SERVE_CONTROL_EXT_METHODS.sessionClose, + { + sessionId: entry.sessionId, + drainTimeoutMs: Math.max(1, Math.floor(initTimeoutMs * 0.8)), + ...(opts?.requireFlush === true ? { requireFlush: true } : {}), + }, + ); + const observedCloseRequest = opts?.timeoutMs + ? withTimeout(closeRequest, opts.timeoutMs, label) + : closeRequest; await Promise.race([ - withTimeout( - entry.connection.extMethod(SERVE_CONTROL_EXT_METHODS.sessionClose, { - sessionId: entry.sessionId, - ...(opts?.requireFlush === true ? { requireFlush: true } : {}), - }), - initTimeoutMs, - SERVE_CONTROL_EXT_METHODS.sessionClose, - ), + opts?.throwOnFailure === true + ? observedCloseRequest + : withTimeout( + observedCloseRequest, + initTimeoutMs, + SERVE_CONTROL_EXT_METHODS.sessionClose, + ), getTransportClosedReject(entry), ]); } catch (err) { @@ -3489,6 +3516,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { persistence: createSessionArtifactPersistence(ci.connection, sessionId), }), recordingDegraded: false, + closing: false, promptQueue: Promise.resolve(), pendingPromptCount: 0, pendingPromptList: [], @@ -3814,6 +3842,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const existing = byId.get(req.sessionId); if (existing) { + if (existing.closing) { + throw new SessionNotFoundError( + req.sessionId, + 'The session is closing; retry after close completes', + ); + } existing.attachCount++; const clientId = registerClient(existing, req.clientId); if (req.approvalMode) { @@ -4237,10 +4271,17 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ): Promise { const entry = byId.get(sessionId); if (!entry) throw new SessionNotFoundError(sessionId); + if (entry.closing) { + throw new SessionNotFoundError( + sessionId, + 'The session is already closing', + ); + } let originatorClientId: string | undefined; if (context?.clientId !== undefined) { originatorClientId = resolveTrustedClientId(entry, context.clientId); } + entry.closing = true; const reason = closeOpts?.reason ?? 'client_close'; writeStderrLine( `qwen serve: closing session ${JSON.stringify(sessionId)}` + @@ -4254,7 +4295,6 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { 'session.id': sessionId, 'session.close.reason': reason, }); - if (defaultEntry === entry) defaultEntry = undefined; // 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 @@ -4273,23 +4313,40 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { `for session ${JSON.stringify(sessionId)} — channel cleanup skipped (entry's channel already torn down)`, ); } - const requireAgentClose = closeOpts?.requireAgentClose === true; - if (requireAgentClose) { + try { + // Resolve permission waits before asking the agent to drain active turns; + // otherwise a turn blocked in requestPermission can deadlock close. + permissionMediator.forgetSession(sessionId); + entry.pendingPermissionIds.clear(); + entry.pendingInteractions.clear(); await notifyAgentSessionClose(entry, ci, 'closeSession', { throwOnFailure: true, - requireFlush: true, + requireFlush: closeOpts?.requireAgentClose === true, }); + } catch (error) { + // A child RequestError is a definitive close refusal: the child kept + // the session live, so a retry is safe. A transport failure has an + // unknown outcome because the close RPC may already have succeeded. + // Terminate that process so its leases become stale and channel-exit + // cleanup removes every bridge entry it owned. + if (isDefinitiveAcpRequestError(error)) { + entry.closing = false; + } else if (ci) { + await killChannelWithLog( + ci, + `recover unknown close outcome for session ${JSON.stringify(sessionId)}`, + ); + } else { + entry.closing = false; + } + throw error; } + 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. - permissionMediator.forgetSession(sessionId); - entry.pendingPermissionIds.clear(); - entry.pendingInteractions.clear(); + // Agent-owned state, including the writer lease, is gone before bridge + // visibility is removed. A failed strict close remains retryable. if (entry.promptActive) { entry.promptActive = false; activePromptCounter--; @@ -4327,9 +4384,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', @@ -4504,6 +4558,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { if (effectiveScope === 'single') { const existing = defaultEntry; if (existing) { + if (existing.closing) { + throw new SessionNotFoundError( + existing.sessionId, + 'The session is closing; retry after close completes', + ); + } // BRSCi: bump attach counter BEFORE any await so the // spawn-owner's disconnect reaper (server.ts: // `requireZeroAttaches: true`) sees this attach even when @@ -4700,6 +4760,14 @@ 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) { + return Promise.reject( + new SessionNotFoundError( + sessionId, + 'The session is closing; retry after close completes', + ), + ); + } const originatorClientId = resolveTrustedClientId( entry, context?.clientId, @@ -7077,6 +7145,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { async rewindSession(sessionId, req, context) { const entry = byId.get(sessionId); if (!entry) throw new SessionNotFoundError(sessionId); + if (entry.closing) { + throw new SessionNotFoundError(sessionId, 'The session is closing'); + } const info = channelInfoForEntry(entry); if (!info || info.isDying) throw new SessionNotFoundError(sessionId); const originatorClientId = resolveTrustedClientId( @@ -7476,12 +7547,44 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { entry.spawnOwnerWantedKill = true; return false; } - // 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). + if (entry.closing) { + const closingChannel = channelInfoForEntry(entry); + if (!closingChannel) return false; + await killChannelWithLog( + closingChannel, + `force kill closing session ${JSON.stringify(sessionId)}`, + ); + return true; + } + 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)`, + ); + } + // Resolve permission waits before asking the agent to drain active turns; + // otherwise a turn blocked in requestPermission can deadlock kill. permissionMediator.forgetSession(sessionId); entry.pendingPermissionIds.clear(); entry.pendingInteractions.clear(); + try { + await notifyAgentSessionClose(entry, ci, 'killSession', { + throwOnFailure: true, + timeoutMs: initTimeoutMs, + }); + } catch (error) { + if (ci) { + await killChannelWithLog( + ci, + `force kill session ${JSON.stringify(sessionId)}`, + ); + return true; + } + entry.closing = false; + throw error; + } if (entry.promptActive) { entry.promptActive = false; activePromptCounter--; @@ -7511,19 +7614,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // 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 2f0c4343539..3ab1836604f 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -439,7 +439,11 @@ 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 pending recorder writes to flush successfully. All closes await + * the ACP child acknowledgement and may cancel in-flight turns even when + * the close attempt ultimately fails. + */ requireAgentClose?: boolean; } diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index ebe01b4ee40..6386580c4df 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -493,10 +493,31 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ })), MCP_BUDGET_WARN_FRACTION: 0.75, SessionService: vi.fn(), + SESSION_WRITER_RPC_CODES: { + session_writer_conflict: -32020, + session_writer_lost: -32021, + session_transcript_changed: -32022, + session_writer_unavailable: -32023, + }, + SessionWriterUnavailableError: class SessionWriterUnavailableError extends Error { + readonly rpcCode = -32023; + readonly errorKind = 'session_writer_unavailable'; + }, + computeUniqueBranchTitle: vi.fn( + async (baseName: string) => `${baseName} (Branch)`, + ), Storage: { 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-test'), + runWithRuntimeBaseDir: vi.fn( + ( + _runtimeBaseDir: string, + _sessionId: string | undefined, + operation: () => unknown, + ) => operation(), + ), }, parseRule: vi.fn((raw: string) => { const trimmed = raw.trim(); @@ -783,7 +804,11 @@ import { mcpServerRequiresOAuth, APPROVAL_MODES, } from '@qwen-code/qwen-code-core'; -import type { McpServer } from '@agentclientprotocol/sdk'; +import type { + LoadSessionResponse, + McpServer, + ResumeSessionResponse, +} from '@agentclientprotocol/sdk'; import { AgentSideConnection } from '@agentclientprotocol/sdk'; import { loadSettings, SettingScope } from '../config/settings.js'; import { @@ -1540,7 +1565,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); - it('configures ACP file system fallback roots for read_file allowed local roots', async () => { + it('configures ACP file system fallback roots from the pinned session runtime', async () => { const previousRoots = process.env[acpLocalReadRootsEnv]; delete process.env[acpLocalReadRootsEnv]; @@ -1554,6 +1579,21 @@ describe('QwenAgent MCP SSE/HTTP support', () => { } }); + it('does not reuse another runtime root for ACP file system fallback', async () => { + const previousRoots = process.env[acpLocalReadRootsEnv]; + delete process.env[acpLocalReadRootsEnv]; + + try { + await expectAcpLocalReadRoots( + 'session-with-other-runtime', + expectedDefaultAcpLocalReadRoots('/runtime-b'), + '/runtime-b', + ); + } finally { + restoreOptionalEnv(acpLocalReadRootsEnv, previousRoots); + } + }); + it('appends QWEN_ACP_LOCAL_READ_ROOTS absolute entries to ACP file system fallback roots', async () => { const previousRoots = process.env[acpLocalReadRootsEnv]; const envRootA = path.resolve('/custom/acp-a'); @@ -1943,6 +1983,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { function makeInnerConfig() { return { initialize: vi.fn().mockResolvedValue(undefined), + shutdown: vi.fn().mockResolvedValue(undefined), waitForMcpReady: vi.fn().mockResolvedValue(undefined), getModelsConfig: vi.fn().mockReturnValue({ getCurrentAuthType: vi.fn().mockReturnValue('api-key'), @@ -1951,6 +1992,9 @@ describe('QwenAgent MCP SSE/HTTP support', () => { reloadModelProvidersConfig: vi.fn(), refreshAuth: vi.fn().mockResolvedValue(undefined), getModel: vi.fn().mockReturnValue('m'), + storage: { + getProjectRoot: vi.fn().mockReturnValue('/tmp'), + }, getProjectRoot: vi.fn().mockReturnValue('/tmp'), getTargetDir: vi.fn().mockReturnValue('/tmp'), getContentGeneratorConfig: vi.fn().mockReturnValue({}), @@ -1968,7 +2012,16 @@ describe('QwenAgent MCP SSE/HTTP support', () => { getFileSystemService: vi.fn().mockReturnValue(undefined), getChatRecordingService: vi.fn().mockReturnValue({ flush: vi.fn().mockResolvedValue(undefined), + finalize: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + hasWriteOwnership: vi.fn().mockReturnValue(false), + runWithWriteBarrier: vi.fn( + async (operation: () => Promise): Promise => operation(), + ), }), + getSessionService: vi.fn(() => new SessionService('/tmp')), + hasSessionWriteOwnership: vi.fn().mockReturnValue(false), + getSessionRuntimeBaseDir: vi.fn().mockReturnValue('/runtime-a'), setFileSystemService: vi.fn(), getHookSystem: vi.fn().mockReturnValue(undefined), getDisableAllHooks: vi.fn().mockReturnValue(true), @@ -1976,11 +2029,13 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }; } - function expectedDefaultAcpLocalReadRoots(): string[] { + function expectedDefaultAcpLocalReadRoots( + runtimeBaseDir = '/runtime-a', + ): string[] { return [ '/project/.qwen/tmp', path.join('/project', 'subagents'), - '/tmp/qwen-global-temp', + path.join(runtimeBaseDir, 'tmp'), '/project/.qwen/memory', '/tmp/user-memory', '/home/test/.qwen/skills', @@ -2000,6 +2055,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { async function expectAcpLocalReadRoots( sessionId: string, expectedLocalReadRoots: string[], + runtimeBaseDir = '/runtime-a', ): Promise { const fsCapabilities = { readTextFile: true, writeTextFile: true }; const fallbackFileSystem: Record = {}; @@ -2007,6 +2063,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { ...makeInnerConfig(), getTargetDir: vi.fn().mockReturnValue('/project'), getSessionId: vi.fn().mockReturnValue(sessionId), + getSessionRuntimeBaseDir: vi.fn().mockReturnValue(runtimeBaseDir), getFileSystemService: vi.fn().mockReturnValue(fallbackFileSystem), setFileSystemService: vi.fn(), storage: { @@ -2199,15 +2256,21 @@ describe('QwenAgent MCP SSE/HTTP support', () => { vi.mocked(loadCliConfig).mockResolvedValue( innerConfig as unknown as Config, ); - vi.mocked(Session).mockImplementation(() => { + vi.mocked(Session).mockImplementation((createdSessionId, createdConfig) => { const sessionMock = { - getId: vi.fn().mockReturnValue(sessionId), - getConfig: vi.fn().mockReturnValue(innerConfig), + getId: vi.fn().mockReturnValue(createdSessionId), + getConfig: vi.fn().mockReturnValue(createdConfig), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), + beginClose: vi.fn().mockReturnValue(vi.fn()), + beginCloseIfAvailable: vi.fn().mockReturnValue(vi.fn()), + waitForCloseGateToRelease: vi.fn().mockResolvedValue(undefined), + waitForActiveTurnsToSettle: vi.fn().mockResolvedValue(undefined), + cancelPendingPrompt: vi.fn().mockResolvedValue(undefined), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), dispose: vi.fn(), emitGoalStatus: vi.fn(), captureHistorySnapshot: vi @@ -9347,6 +9410,13 @@ describe('QwenAgent MCP SSE/HTTP support', () => { isInitialized: vi.fn().mockReturnValue(true), initialize, }); + const followupConfig = { + ...innerConfig, + getSessionId: vi.fn().mockReturnValue('session-followup-session-start-2'), + }; + vi.mocked(loadCliConfig) + .mockResolvedValueOnce(innerConfig as unknown as Config) + .mockResolvedValueOnce(followupConfig as unknown as Config); const agentPromise = runAcpAgent( mockConfig, @@ -9462,7 +9532,8 @@ describe('QwenAgent MCP SSE/HTTP support', () => { it('rewindSession extension method rewinds the active session', async () => { const sessionId = '11111111-1111-1111-1111-111111111111'; - await setupSessionMocks(sessionId); + const innerConfig = await setupSessionMocks(sessionId); + innerConfig.getProjectRoot.mockReturnValue('/tmp/after-cd'); const artifactSnapshot = { v: 1, sessionId, @@ -9502,6 +9573,8 @@ describe('QwenAgent MCP SSE/HTTP support', () => { expect(lastSessionMock?.rewindToTurn).toHaveBeenCalledWith(1, { rewindFiles: true, }); + expect(SessionService).toHaveBeenCalledWith('/tmp'); + expect(innerConfig.getSessionService).toHaveBeenCalled(); expect(response).toEqual({ success: true, historyBeforeRewind: [{ role: 'user', parts: [{ text: 'before' }] }], @@ -10246,6 +10319,34 @@ describe('QwenAgent MCP SSE/HTTP support', () => { mockConnectionState.resolve(); await agentPromise; }); + + it('does not override disabled chat recording for a real ACP session', async () => { + await setupSessionMocks('session-recording-disabled'); + const settings = makeSessionSettings(); + settings.merged.general = { chatRecording: false }; + vi.mocked(loadSettings).mockReturnValue(settings); + + const agentPromise = runAcpAgent(mockConfig, settings, mockArgv); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + const realSessionCall = vi + .mocked(loadCliConfig) + .mock.calls.find(([, argv]) => !('chatRecording' in argv)); + expect(realSessionCall?.[0]).toMatchObject({ + general: { chatRecording: false }, + }); + expect(realSessionCall?.[1]).not.toHaveProperty('chatRecording'); + + mockConnectionState.resolve(); + await agentPromise; + }); }); // Regression coverage for the MR-review finding that ACP renameSession @@ -10269,6 +10370,11 @@ describe('QwenAgent extMethod renameSession routing', () => { | undefined; let mockConfig: Config; let liveCancelPendingPrompt: ReturnType; + let liveWaitForActiveTurnsToSettle: ReturnType; + let liveBeginCloseIfAvailable: ReturnType; + let liveWaitForCloseGateToRelease: ReturnType; + let liveReleaseCloseGate: ReturnType; + let liveBeginClose: ReturnType; // Live session sessionId is whatever `getSessionId()` on the inner config // returns; matches the existing test scaffolding. @@ -10279,6 +10385,11 @@ describe('QwenAgent extMethod renameSession routing', () => { mockConnectionState.reset(); capturedAgentFactory = undefined; liveCancelPendingPrompt = vi.fn().mockResolvedValue(undefined); + liveWaitForActiveTurnsToSettle = vi.fn().mockResolvedValue(undefined); + liveBeginCloseIfAvailable = vi.fn(() => liveBeginClose()); + liveWaitForCloseGateToRelease = vi.fn().mockResolvedValue(undefined); + liveReleaseCloseGate = vi.fn(); + liveBeginClose = vi.fn().mockReturnValue(liveReleaseCloseGate); vi.mocked(AgentSideConnection).mockImplementation((factory: unknown) => { capturedAgentFactory = factory as typeof capturedAgentFactory; @@ -10308,7 +10419,15 @@ describe('QwenAgent extMethod renameSession routing', () => { function makeRecordingService() { return { recordCustomTitle: vi.fn().mockResolvedValue(true), + recordUserTextElements: vi.fn().mockResolvedValue(undefined), + getCurrentCustomTitle: vi.fn().mockReturnValue('Source session'), flush: vi.fn().mockResolvedValue(undefined), + runWithWriteBarrier: vi.fn( + async (operation: () => Promise): Promise => operation(), + ), + finalize: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + hasWriteOwnership: vi.fn().mockReturnValue(false), }; } @@ -10317,12 +10436,17 @@ describe('QwenAgent extMethod renameSession routing', () => { ) { return { initialize: vi.fn().mockResolvedValue(undefined), + shutdown: vi.fn().mockResolvedValue(undefined), waitForMcpReady: vi.fn().mockResolvedValue(undefined), getModelsConfig: vi.fn().mockReturnValue({ getCurrentAuthType: vi.fn().mockReturnValue('api-key'), }), refreshAuth: vi.fn().mockResolvedValue(undefined), getModel: vi.fn().mockReturnValue('m'), + storage: { + getProjectRoot: vi.fn().mockReturnValue('/tmp'), + }, + getTargetDir: vi.fn().mockReturnValue('/tmp'), getContentGeneratorConfig: vi.fn().mockReturnValue({}), getAvailableModels: vi.fn().mockReturnValue([]), getModes: vi.fn().mockReturnValue([]), @@ -10342,6 +10466,11 @@ describe('QwenAgent extMethod renameSession routing', () => { hasHooksForEvent: vi.fn().mockReturnValue(false), getToolRegistry: vi.fn().mockReturnValue(undefined), getChatRecordingService: vi.fn().mockReturnValue(recording), + hasSessionWriteOwnership: vi + .fn() + .mockImplementation(() => recording?.hasWriteOwnership() === true), + getSessionRuntimeBaseDir: vi.fn().mockReturnValue('/runtime-a'), + getSessionService: vi.fn(() => new SessionService('/tmp')), }; } @@ -10366,6 +10495,11 @@ describe('QwenAgent extMethod renameSession routing', () => { getId: vi.fn().mockReturnValue(liveSessionId), getConfig: vi.fn().mockReturnValue(innerConfig), cancelPendingPrompt: liveCancelPendingPrompt, + beginClose: liveBeginClose, + beginCloseIfAvailable: liveBeginCloseIfAvailable, + waitForCloseGateToRelease: liveWaitForCloseGateToRelease, + waitForActiveTurnsToSettle: liveWaitForActiveTurnsToSettle, + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), replayHistory: vi.fn().mockResolvedValue(undefined), installRewriter: vi.fn(), @@ -10420,6 +10554,90 @@ describe('QwenAgent extMethod renameSession routing', () => { await agentPromise; }); + it('validates and records live user text elements', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + await expect( + agent.extMethod('qwen/session/recordTextElements', { + sessionId: liveSessionId, + content: 42, + textElements: [], + }), + ).rejects.toThrow('Invalid user text elements payload'); + + const payload = { + sessionId: liveSessionId, + content: 'hello', + textElements: [{ text: 'hello', start: 0, end: 5 }], + }; + await expect( + agent.extMethod('qwen/session/recordTextElements', payload), + ).resolves.toEqual({ sessionId: liveSessionId, persisted: true }); + expect(recording.recordUserTextElements).toHaveBeenCalledWith({ + content: payload.content, + textElements: payload.textElements, + }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('rejects registering a second session with the same id', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.newSession({ cwd: '/tmp', mcpServers: [] }), + ).rejects.toThrow(`Session ${liveSessionId} is already active.`); + expect(Session).toHaveBeenCalledTimes(1); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loads live transcript updates through the owner barrier and pinned session service', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + innerConfig.getSessionRuntimeBaseDir.mockReturnValue( + '/tmp/qwen-runtime-test', + ); + const loadSession = vi.fn().mockResolvedValue({ + conversation: { + messages: [], + startTime: 'start', + lastUpdated: 'end', + }, + }); + innerConfig.getSessionService = vi.fn( + () => + ({ + loadSession, + }) as unknown as InstanceType, + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + await expect( + agent.extMethod('qwen/session/loadUpdates', { + cwd: '/tmp', + sessionId: liveSessionId, + }), + ).resolves.toMatchObject({ startTime: 'start', lastUpdated: 'end' }); + + expect(recording.runWithWriteBarrier).toHaveBeenCalledOnce(); + expect(innerConfig.getSessionService).toHaveBeenCalledOnce(); + expect(loadSession).toHaveBeenCalledWith(liveSessionId); + expect(SessionService).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('falls back to SessionService.renameSession when no live session matches the sessionId', async () => { const recording = makeRecordingService(); const innerConfig = makeLiveSessionInnerConfig(recording); @@ -10570,12 +10788,56 @@ describe('QwenAgent extMethod renameSession routing', () => { await agentPromise; }); + it('branches a live session through its pinned SessionService', async () => { + const recording = makeRecordingService(); + const sessionService = { + forkSession: vi.fn().mockResolvedValue(undefined), + findSessionTitlesByPrefix: vi.fn().mockResolvedValue([]), + renameSession: vi.fn().mockResolvedValue(true), + removeSession: vi.fn().mockResolvedValue(undefined), + }; + const innerConfig = makeLiveSessionInnerConfig(recording); + innerConfig.getSessionRuntimeBaseDir.mockReturnValue('/runtime-source'); + innerConfig.storage.getProjectRoot.mockReturnValue('/workspace-source'); + innerConfig.getSessionService.mockReturnValue( + sessionService as unknown as SessionService, + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + + await agent.newSession({ cwd: '/workspace-source', mcpServers: [] }); + const result = await agent.extMethod( + SERVE_CONTROL_EXT_METHODS.sessionBranch, + { + cwd: '/workspace-other', + sessionId: liveSessionId, + }, + ); + + expect(recording.flush).toHaveBeenCalledOnce(); + expect(innerConfig.getSessionService).toHaveBeenCalledOnce(); + expect(SessionService).not.toHaveBeenCalled(); + expect(sessionService.forkSession).toHaveBeenCalledWith( + liveSessionId, + expect.any(String), + ); + expect(sessionService.renameSession).toHaveBeenCalledWith( + expect.any(String), + 'Source session (Branch)', + 'manual', + ); + expect(result).toMatchObject({ + title: 'Source session (Branch)', + displayName: 'Source session (Branch)', + }); + + 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')); 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: [] }); @@ -10597,7 +10859,7 @@ describe('QwenAgent extMethod renameSession routing', () => { ).toContain(liveSessionId); expect(recording.flush).toHaveBeenCalledOnce(); expect(liveCancelPendingPrompt).not.toHaveBeenCalled(); - expect(toolRegistry.stop).not.toHaveBeenCalled(); + expect(innerConfig.shutdown).not.toHaveBeenCalled(); await expect( agent.extMethod('qwen/control/session/close', { @@ -10607,7 +10869,7 @@ describe('QwenAgent extMethod renameSession routing', () => { ).rejects.toThrow('flush failed'); expect(recording.flush).toHaveBeenCalledTimes(2); expect(liveCancelPendingPrompt).not.toHaveBeenCalled(); - expect(toolRegistry.stop).not.toHaveBeenCalled(); + expect(innerConfig.shutdown).not.toHaveBeenCalled(); await expect( agent.extMethod('qwen/control/session/close', { @@ -10617,7 +10879,7 @@ describe('QwenAgent extMethod renameSession routing', () => { ).resolves.toEqual({ sessionId: liveSessionId, closed: true }); expect(recording.flush).toHaveBeenCalledTimes(3); expect(liveCancelPendingPrompt).toHaveBeenCalledOnce(); - expect(toolRegistry.stop).toHaveBeenCalledOnce(); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); expect( ( agent as unknown as { @@ -10631,6 +10893,163 @@ describe('QwenAgent extMethod renameSession routing', () => { mockConnectionState.resolve(); await agentPromise; }); + + it('does not abort an active generation when the close gate is unavailable', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + const controller = new AbortController(); + const abort = vi.spyOn(controller, 'abort'); + ( + agent as unknown as { + generationControllers: Map< + string, + { sessionId: string; controller: AbortController } + >; + } + ).generationControllers.set('active-generation', { + sessionId: liveSessionId, + controller, + }); + liveBeginClose.mockImplementationOnce(() => { + throw new Error('close gate unavailable'); + }); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + }), + ).rejects.toThrow('close gate unavailable'); + expect(abort).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('waits for a held close gate before disposing the live writer', async () => { + let gateHeld = true; + let releaseHeldGate!: () => void; + liveBeginCloseIfAvailable.mockImplementation(() => + gateHeld ? null : liveBeginClose(), + ); + liveWaitForCloseGateToRelease.mockReturnValueOnce( + new Promise((resolve) => { + releaseHeldGate = () => { + gateHeld = false; + resolve(); + }; + }), + ); + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + let settled = false; + const disposing = ( + agent as unknown as { disposeSessions: () => Promise } + ) + .disposeSessions() + .finally(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + expect(recording.close).not.toHaveBeenCalled(); + + releaseHeldGate(); + await disposing; + expect(recording.close).toHaveBeenCalledOnce(); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('times out a stuck close drain without releasing the live writer', async () => { + const recording = makeRecordingService(); + const innerConfig = makeLiveSessionInnerConfig(recording); + const { agent, agentPromise } = await bootAgent(innerConfig); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + liveWaitForActiveTurnsToSettle.mockReturnValueOnce( + new Promise(() => {}), + ); + + await expect( + agent.extMethod('qwen/control/session/close', { + sessionId: liveSessionId, + drainTimeoutMs: 5, + }), + ).rejects.toThrow('Session close timed out'); + expect(recording.close).not.toHaveBeenCalled(); + expect(liveReleaseCloseGate).toHaveBeenCalledOnce(); + 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, + drainTimeoutMs: 50, + }), + ).resolves.toEqual({ sessionId: liveSessionId, closed: true }); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('retries cleanup for an unreturned fresh session before the next creation', async () => { + const recording = makeRecordingService(); + recording.hasWriteOwnership.mockReturnValue(true); + const innerConfig = makeLiveSessionInnerConfig(recording); + const nonOwnerConfig = makeLiveSessionInnerConfig(makeRecordingService()); + innerConfig.getSessionRuntimeBaseDir.mockReturnValue( + '/tmp/qwen-runtime-test', + ); + nonOwnerConfig.getSessionRuntimeBaseDir.mockReturnValue( + '/tmp/qwen-runtime-test', + ); + const { agent, agentPromise } = await bootAgent(innerConfig); + const cleanup = agent as unknown as { + cleanupUnstoredConfig(config: Config): Promise; + retryPendingConfigCleanup( + runtimeBaseDir: string, + sessionId: string, + ): Promise; + }; + + await expect( + cleanup.cleanupUnstoredConfig(innerConfig as unknown as Config), + ).rejects.toMatchObject({ + code: -32023, + data: { errorKind: 'session_writer_unavailable' }, + }); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + + await expect( + cleanup.cleanupUnstoredConfig(nonOwnerConfig as unknown as Config), + ).resolves.toBeUndefined(); + expect(nonOwnerConfig.shutdown).toHaveBeenCalledOnce(); + + recording.hasWriteOwnership.mockReturnValue(false); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + expect(innerConfig.shutdown).toHaveBeenCalledTimes(2); + await cleanup.retryPendingConfigCleanup( + '/tmp/qwen-runtime-test', + liveSessionId, + ); + expect(innerConfig.shutdown).toHaveBeenCalledTimes(2); + + mockConnectionState.resolve(); + await agentPromise; + }); }); describe('QwenAgent unstable_listSessions cursor parsing', () => { @@ -10932,6 +11351,12 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { installRewriter: ReturnType; installGoalTerminalObserver: ReturnType; startCronScheduler: ReturnType; + assertCanStartTurn: ReturnType; + beginClose: ReturnType; + beginCloseIfAvailable: ReturnType; + waitForCloseGateToRelease: ReturnType; + waitForActiveTurnsToSettle: ReturnType; + sendUpdate: ReturnType; dispose: ReturnType; } | undefined; @@ -10943,6 +11368,9 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { beforeEach(() => { vi.clearAllMocks(); + vi.mocked(Storage.getRuntimeBaseDir).mockReturnValue( + '/tmp/qwen-runtime-test', + ); mockConnectionState.reset(); lastSessionMock = undefined; capturedAgentFactory = undefined; @@ -10995,15 +11423,27 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { ) { const recording = { rebuildTurnBoundaries: vi.fn(), + flush: vi.fn().mockResolvedValue(undefined), + finalize: vi.fn(), + close: vi.fn().mockResolvedValue(undefined), + hasWriteOwnership: vi.fn().mockReturnValue(false), + runWithWriteBarrier: vi.fn( + async (operation: () => Promise): Promise => operation(), + ), }; return { initialize: vi.fn().mockResolvedValue(undefined), + shutdown: vi.fn().mockResolvedValue(undefined), waitForMcpReady: vi.fn().mockResolvedValue(undefined), getModelsConfig: vi.fn().mockReturnValue({ getCurrentAuthType: vi.fn().mockReturnValue('api-key'), }), refreshAuth: vi.fn().mockResolvedValue(undefined), getModel: vi.fn().mockReturnValue('m'), + storage: { + getProjectRoot: vi.fn().mockReturnValue('/tmp'), + }, + getTargetDir: vi.fn().mockReturnValue('/tmp'), getContentGeneratorConfig: vi.fn().mockReturnValue({}), getAvailableModels: vi.fn().mockReturnValue([]), getModes: vi.fn().mockReturnValue([]), @@ -11027,6 +11467,14 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { getDisableAllHooks: vi.fn().mockReturnValue(true), hasHooksForEvent: vi.fn().mockReturnValue(false), getChatRecordingService: vi.fn().mockReturnValue(recording), + hasSessionWriteOwnership: vi + .fn() + .mockImplementation(() => recording.hasWriteOwnership()), + getSessionRuntimeBaseDir: vi + .fn() + .mockReturnValue('/tmp/qwen-runtime-test'), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), + getSessionService: vi.fn(), // load path reads back the persisted conversation here and feeds // it to `session.replayHistory`. resume path doesn't read this. getResumedSessionData: vi @@ -11050,11 +11498,16 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { function bindRestoreMocks(opts: { sessionExists: boolean; resumedConversation?: { messages: unknown[] }; + replayHistoryImpl?: (...args: unknown[]) => Promise; primeTurnFromHistoryImpl?: (...args: unknown[]) => unknown; }) { const innerConfig = makeRestoreInnerConfig({ resumedConversation: opts.resumedConversation, }); + const loadSession = vi + .fn() + .mockImplementation(() => innerConfig.getResumedSessionData()); + innerConfig.getSessionService.mockReturnValue({ loadSession }); vi.mocked(loadSettings).mockReturnValue(makeRestoreSettings()); vi.mocked(loadCliConfig).mockResolvedValue( innerConfig as unknown as Config, @@ -11063,14 +11516,20 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { () => ({ sessionExists: vi.fn().mockResolvedValue(opts.sessionExists), + loadSession, }) as unknown as InstanceType, ); vi.mocked(Session).mockImplementation(() => { + const releaseCloseGate = vi.fn(); const sessionMock = { getId: vi.fn().mockReturnValue('persisted-1'), getConfig: vi.fn().mockReturnValue(innerConfig), sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), - replayHistory: vi.fn().mockResolvedValue(undefined), + replayHistory: vi + .fn() + .mockImplementation( + opts.replayHistoryImpl ?? (async () => undefined), + ), primeTurnFromHistory: vi.fn(opts.primeTurnFromHistoryImpl), cumulativeUsage: { promptTokens: 7, @@ -11081,6 +11540,13 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { installRewriter: vi.fn(), installGoalTerminalObserver: vi.fn(), startCronScheduler: vi.fn(), + beginClose: vi.fn().mockReturnValue(releaseCloseGate), + beginCloseIfAvailable: vi.fn().mockReturnValue(releaseCloseGate), + waitForCloseGateToRelease: vi.fn().mockResolvedValue(undefined), + waitForActiveTurnsToSettle: vi.fn().mockResolvedValue(undefined), + cancelPendingPrompt: vi.fn().mockResolvedValue(undefined), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), + sendUpdate: vi.fn().mockResolvedValue(undefined), dispose: vi.fn(), }; lastSessionMock = sessionMock; @@ -11123,6 +11589,31 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); + it('loadSession preserves initialization failure and retries deferred cleanup', async () => { + const initializationError = new Error('initialize boom'); + const cleanupError = new Error('shutdown boom'); + const innerConfig = bindRestoreMocks({ sessionExists: true }); + innerConfig.initialize.mockRejectedValue(initializationError); + innerConfig.shutdown.mockRejectedValue(cleanupError); + const { agent, agentPromise } = await spawnAgent(); + + const result = await agent + .loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }) + .catch((error: unknown) => error); + + expect(result).toBe(initializationError); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + + innerConfig.shutdown.mockResolvedValue(undefined); + mockConnectionState.resolve(); + await agentPromise; + expect(innerConfig.shutdown).toHaveBeenCalledTimes(2); + }); + /** * A persisted `system` / `slash_command` record carrying goal cards — the only * place a daemon transcript stores them. @@ -11839,11 +12330,62 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { 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 - // NOT be called even though the persisted session existed - // (covers the case where the on-disk record has a session row + it('loadSession preserves a bulk replay setup error when cleanup also fails', async () => { + const setupError = new Error('prime boom'); + const cleanupError = new Error('cleanup boom'); + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'hi' }] }], + }, + primeTurnFromHistoryImpl: () => { + throw setupError; + }, + }); + const recording = innerConfig.getChatRecordingService(); + recording.close.mockRejectedValue(cleanupError); + recording.hasWriteOwnership.mockReturnValue(true); + mockHistoryReplay.mockReset(); + const { agent, agentPromise } = await spawnAgent(); + + const result = await agent + .loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + _meta: { 'qwen.session.loadReplayMode': 'bulk' }, + }) + .catch((error: unknown) => error); + + expect(result).toBe(setupError); + expect(recording.close).toHaveBeenCalledOnce(); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + expect(recording.hasWriteOwnership()).toBe(true); + expect(lastSessionMock?.dispose).toHaveBeenCalledOnce(); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).not.toContain('persisted-1'); + + innerConfig.shutdown.mockImplementation(async () => { + recording.hasWriteOwnership.mockReturnValue(false); + }); + mockConnectionState.resolve(); + await agentPromise; + expect(innerConfig.shutdown).toHaveBeenCalledTimes(2); + expect(recording.hasWriteOwnership()).toBe(false); + }); + + it('loadSession skips history replay when getResumedSessionData() returns undefined', async () => { + // Distinct code path: `createAndStoreSession(config, undefined)` + // takes the no-conversation branch, so `replayHistory` must + // NOT be called even though the persisted session existed + // (covers the case where the on-disk record has a session row // but no resumable conversation, e.g. corrupted / partially // written history). bindRestoreMocks({ sessionExists: true /* no resumedConversation */ }); @@ -11866,12 +12408,168 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); - it('loadSession disposes the existing session when reloading the same sessionId', async () => { - bindRestoreMocks({ + it('removes a stored session when replay and the first lease release fail', async () => { + const replayError = new Error('replay failed'); + const innerConfig = bindRestoreMocks({ sessionExists: true, resumedConversation: { messages: [{ role: 'user', parts: [{ text: 'first' }] }], }, + replayHistoryImpl: async () => { + throw replayError; + }, + }); + const recording = innerConfig.getChatRecordingService(); + let ownsLease = true; + recording.hasWriteOwnership.mockImplementation(() => ownsLease); + recording.close + .mockRejectedValueOnce(new Error('lease release failed')) + .mockImplementation(async () => { + ownsLease = false; + }); + innerConfig.shutdown.mockImplementation(async () => { + await recording.close(); + }); + const { agent, agentPromise } = await spawnAgent(); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toBe(replayError); + + const failedSession = lastSessionMock!; + expect(failedSession.dispose).toHaveBeenCalledOnce(); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).not.toContain('persisted-1'); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + expect(recording.close).toHaveBeenCalledTimes(2); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('cleans Config once when replay fails and lease release succeeds', async () => { + const replayError = new Error('replay failed'); + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + replayHistoryImpl: async () => { + throw replayError; + }, + }); + const recording = innerConfig.getChatRecordingService(); + const { agent, agentPromise } = await spawnAgent(); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toBe(replayError); + + expect(lastSessionMock?.dispose).toHaveBeenCalledOnce(); + expect(recording.close).toHaveBeenCalledOnce(); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + expect( + ( + agent as unknown as { + getActiveSessions: () => Array<{ getId: () => string }>; + } + ) + .getActiveSessions() + .map((session) => session.getId()), + ).not.toContain('persisted-1'); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('does not close a replacement session when an older replay fails', async () => { + const replayError = new Error('old replay failed'); + let rejectReplay!: (error: Error) => void; + const replayPending = new Promise((_resolve, reject) => { + rejectReplay = reject; + }); + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + replayHistoryImpl: () => replayPending, + }); + const { agent, agentPromise } = await spawnAgent(); + const loadPending = agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + await vi.waitFor(() => expect(lastSessionMock).toBeDefined()); + const failedSession = lastSessionMock!; + + const replacementConfig = makeRestoreInnerConfig(); + const replacementDispose = vi.fn(); + const replacement = { + getId: vi.fn().mockReturnValue('persisted-1'), + getConfig: vi.fn().mockReturnValue(replacementConfig), + beginClose: vi.fn().mockReturnValue(vi.fn()), + beginCloseIfAvailable: vi.fn().mockReturnValue(vi.fn()), + waitForCloseGateToRelease: vi.fn().mockResolvedValue(undefined), + cancelPendingPrompt: vi.fn().mockResolvedValue(undefined), + waitForActiveTurnsToSettle: vi.fn().mockResolvedValue(undefined), + dispose: replacementDispose, + } as unknown as InstanceType; + const sessions = ( + agent as unknown as { + sessions: Map>; + } + ).sessions; + sessions.set('persisted-1', replacement); + + const rejection = loadPending.catch((error: unknown) => error); + rejectReplay(replayError); + await expect(rejection).resolves.toBe(replayError); + + expect(sessions.get('persisted-1')).toBe(replacement); + expect(replacementDispose).not.toHaveBeenCalled(); + expect(failedSession.dispose).not.toHaveBeenCalled(); + expect(innerConfig.shutdown).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession reuses the live owner for the same sessionId', async () => { + const replayUpdate = { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'first answer' }, + }; + const initialMessages = [{ role: 'user', parts: [{ text: 'first' }] }]; + const authoritativeMessages = [ + ...initialMessages, + { role: 'model', parts: [{ text: 'first answer' }] }, + ]; + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: initialMessages, + }, + }); + mockHistoryReplay.mockImplementation(async (context, history) => { + expect(history).toBe(authoritativeMessages); + await context.sendUpdate(replayUpdate); }); const { agent, agentPromise } = await spawnAgent(); @@ -11884,14 +12582,297 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { const firstSession = lastSessionMock; expect(firstSession).toBeDefined(); expect(firstSession!.dispose).not.toHaveBeenCalled(); + firstSession!.getConfig().getTargetDir.mockReturnValue('/tmp/after-cd'); + firstSession!.getConfig().getResumedSessionData.mockReturnValue({ + conversation: { messages: authoritativeMessages }, + }); + + // The daemon must not fresh-load a second writer for an already-live id. + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + expect(Session).toHaveBeenCalledTimes(1); + expect(firstSession!.dispose).not.toHaveBeenCalled(); + expect(firstSession!.assertCanStartTurn).toHaveBeenCalledTimes(1); + expect(firstSession!.beginClose).toHaveBeenCalledTimes(1); + expect(firstSession!.waitForActiveTurnsToSettle).toHaveBeenCalledTimes(1); + expect( + firstSession!.getConfig().getChatRecordingService().runWithWriteBarrier, + ).toHaveBeenCalledOnce(); + expect(firstSession!.beginClose.mock.results[0]?.value).toHaveBeenCalled(); + expect(firstSession!.sendUpdate).toHaveBeenCalledWith(replayUpdate); + expect(mockHistoryReplay).toHaveBeenCalledTimes(1); + + mockConnectionState.resolve(); + await agentPromise; + }); - // Second loadSession with the same sessionId should dispose the first + it('times out a live load drain and releases its close gate', async () => { + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + }); + const { agent, agentPromise } = await spawnAgent(); await agent.loadSession({ cwd: '/tmp', sessionId: 'persisted-1', mcpServers: [], }); - expect(firstSession!.dispose).toHaveBeenCalledTimes(1); + const firstSession = lastSessionMock!; + const releaseCloseGate = vi.fn(); + firstSession.beginClose.mockReturnValueOnce(releaseCloseGate); + firstSession.waitForActiveTurnsToSettle.mockReturnValueOnce( + new Promise(() => {}), + ); + + vi.useFakeTimers(); + try { + const result = agent + .loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }) + .catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(30_000); + + await expect(result).resolves.toMatchObject({ + message: 'Session restore timed out after 30000ms', + }); + expect(releaseCloseGate).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + mockConnectionState.resolve(); + await agentPromise; + } + }); + + it('loadSession rejects a live owner from another runtime base', async () => { + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + }); + const { agent, agentPromise } = await spawnAgent(); + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + const firstSession = lastSessionMock!; + vi.mocked(Storage.getRuntimeBaseDir).mockReturnValue( + '/tmp/qwen-runtime-other', + ); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toMatchObject({ + code: -32023, + data: { errorKind: 'session_writer_unavailable' }, + }); + expect(Session).toHaveBeenCalledTimes(1); + expect(firstSession.beginClose).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('loadSession reuses a live read-only session when recording is disabled', async () => { + const messages = [{ role: 'user', parts: [{ text: 'first' }] }]; + const innerConfig = bindRestoreMocks({ + sessionExists: true, + resumedConversation: { messages }, + }); + innerConfig.getChatRecordingService.mockReturnValue(undefined); + mockHistoryReplay.mockResolvedValue(undefined); + const { agent, agentPromise } = await spawnAgent(); + + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).resolves.toMatchObject({ + modes: expect.anything(), + models: expect.anything(), + configOptions: expect.anything(), + }); + + expect(Session).toHaveBeenCalledTimes(1); + expect(innerConfig.getSessionService().loadSession).toHaveBeenCalledOnce(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('live load returns authoritative bulk replay and artifacts without mutating usage', async () => { + const initialMessages = [{ role: 'user', parts: [{ text: 'first' }] }]; + const authoritativeMessages = [ + ...initialMessages, + { role: 'model', parts: [{ text: 'latest answer' }] }, + ]; + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { messages: initialMessages }, + }); + const { agent, agentPromise } = await spawnAgent(); + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + const firstSession = lastSessionMock!; + const artifactSnapshot = { v: 1, artifacts: [], warnings: [] }; + firstSession.getConfig().getResumedSessionData.mockReturnValue({ + conversation: { messages: authoritativeMessages }, + artifactSnapshot, + }); + const originalUsage = { ...firstSession.cumulativeUsage }; + const replayUpdate = { sessionUpdate: 'agent_message_chunk' }; + mockHistoryReplay.mockImplementation(async (context, history) => { + expect(history).toBe(authoritativeMessages); + context.cumulativeUsage!.promptTokens = 999; + await context.sendUpdate(replayUpdate); + }); + + const response = (await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + _meta: { 'qwen.session.loadReplayMode': 'bulk' }, + })) as LoadSessionResponse & { + artifactSnapshot?: unknown; + _meta?: Record; + }; + + expect(Session).toHaveBeenCalledTimes(1); + expect(response.artifactSnapshot).toBe(artifactSnapshot); + expect(response._meta?.['qwen.session.loadReplay']?.updates).toEqual([ + replayUpdate, + ]); + expect(firstSession.cumulativeUsage).toEqual(originalUsage); + expect(firstSession.sendUpdate).not.toHaveBeenCalled(); + expect(firstSession.installRewriter).toHaveBeenCalledTimes(1); + expect(firstSession.startCronScheduler).toHaveBeenCalledTimes(1); + expect(firstSession.beginClose.mock.results[0]?.value).toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('live resume refreshes artifacts without replaying UI history', async () => { + const messages = [{ role: 'user', parts: [{ text: 'first' }] }]; + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { messages }, + }); + const { agent, agentPromise } = await spawnAgent(); + await agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + }); + const firstSession = lastSessionMock!; + const artifactSnapshot = { v: 1, artifacts: [], warnings: [] }; + firstSession.getConfig().getResumedSessionData.mockReturnValue({ + conversation: { messages }, + artifactSnapshot, + }); + + const response = (await agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + })) as ResumeSessionResponse & { artifactSnapshot?: unknown }; + + expect(Session).toHaveBeenCalledTimes(1); + expect(response.artifactSnapshot).toBe(artifactSnapshot); + expect(firstSession.replayHistory).not.toHaveBeenCalled(); + expect(mockHistoryReplay).not.toHaveBeenCalled(); + expect(firstSession.beginClose.mock.results[0]?.value).toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('live resume rejects an owner from another runtime base', async () => { + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + }); + const { agent, agentPromise } = await spawnAgent(); + await agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + }); + const firstSession = lastSessionMock!; + vi.mocked(Storage.getRuntimeBaseDir).mockReturnValue( + '/tmp/qwen-runtime-other', + ); + + await expect( + agent.unstable_resumeSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + }), + ).rejects.toMatchObject({ + code: -32023, + data: { errorKind: 'session_writer_unavailable' }, + }); + expect(Session).toHaveBeenCalledTimes(1); + expect(firstSession.beginClose).not.toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('live load fails closed when the authoritative transcript disappears', async () => { + bindRestoreMocks({ + sessionExists: true, + resumedConversation: { + messages: [{ role: 'user', parts: [{ text: 'first' }] }], + }, + }); + const { agent, agentPromise } = await spawnAgent(); + await agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }); + const firstSession = lastSessionMock!; + firstSession + .getConfig() + .getSessionService() + .loadSession.mockResolvedValue(undefined); + + await expect( + agent.loadSession({ + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }), + ).rejects.toMatchObject({ + code: -32023, + data: { errorKind: 'session_writer_unavailable' }, + }); + expect(Session).toHaveBeenCalledTimes(1); + expect(firstSession.dispose).not.toHaveBeenCalled(); + expect(firstSession.beginClose.mock.results[0]?.value).toHaveBeenCalled(); mockConnectionState.resolve(); await agentPromise; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 75f1953346f..1fc952ad7d1 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -31,6 +31,8 @@ import { MCPServerConfig, runForkedAgent, SessionService, + SESSION_WRITER_RPC_CODES, + SessionWriterUnavailableError, SESSION_TITLE_MAX_LENGTH, Storage, tokenLimit, @@ -326,6 +328,7 @@ const POSIX_TMP_LOCAL_READ_ROOT = '/tmp'; // aborts before the bridge's backstop timer fires. const BTW_CHILD_TIMEOUT_MS = 55_000; const MCP_OAUTH_START_TIMEOUT_MS = 30_000; +const SESSION_DRAIN_TIMEOUT_MS = 30_000; // Must be less than WORKSPACE_MEMORY_REMEMBER_TIMEOUT_MS (300s) in bridge.ts. const WORKSPACE_MEMORY_REMEMBER_CHILD_TIMEOUT_MS = 295_000; @@ -409,6 +412,98 @@ function workspaceMemoryErrorData( }; } +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], + }; +} + +function mapSessionWriterRequestError(error: unknown): unknown { + const writerError = getSessionWriterError(error); + return writerError + ? new RequestError(writerError.rpcCode, writerError.message, { + errorKind: writerError.errorKind, + }) + : error; +} + +async function shutdownSessionConfig(config: Config): Promise { + await config.shutdown({ shutdownTelemetry: false }); + if (config.hasSessionWriteOwnership()) { + throw new SessionWriterUnavailableError(); + } +} + +async function waitForSessionDrain( + operation: Promise, + timeoutMs: number, + kind: 'close' | 'restore', +): Promise { + let timer: NodeJS.Timeout | undefined; + const timeout = new Promise((_, reject) => { + timer = setTimeout( + () => reject(new Error(`Session ${kind} timed out after ${timeoutMs}ms`)), + timeoutMs, + ); + timer.unref(); + }); + try { + await Promise.race([operation, timeout]); + } finally { + if (timer) clearTimeout(timer); + } +} + +async function beginSessionCloseAfterCurrentGate( + session: Session, + timeoutMs: number, +): Promise<() => void> { + const deadline = Date.now() + timeoutMs; + while (true) { + const releaseGate = session.beginCloseIfAvailable(); + if (releaseGate) return releaseGate; + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) { + throw new Error(`Session close timed out after ${timeoutMs}ms`); + } + await waitForSessionDrain( + session.waitForCloseGateToRelease(), + remainingMs, + 'close', + ); + } +} + const logWorkspaceMemoryExtractionError = createWorkspaceMemoryExtractionErrorLogger(debugLogger); @@ -434,7 +529,7 @@ function buildAcpLocalReadRoots(config: Config): string[] { // local read fallback, not read_file's default permission. config.storage.getProjectTempDir(), path.join(config.storage.getProjectDir(), 'subagents'), - Storage.getGlobalTempDir(), + path.join(config.getSessionRuntimeBaseDir(), 'tmp'), getAutoMemoryRoot(config.getTargetDir()), getUserAutoMemoryRoot(), ...config.storage.getUserSkillsDirs(), @@ -2669,7 +2764,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 +2805,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); @@ -2901,6 +2996,7 @@ class QwenAgent implements Agent { string, TranscriptReplayConfigCacheEntry >(); + private readonly pendingConfigCleanup = new Map>(); private clientCapabilities: ClientCapabilities | undefined; // CPU-usage delta baseline for the daemon's `workspaceResource` extMethod // (Daemon Status child-resource chart). The daemon polls this at a fixed @@ -3039,21 +3135,24 @@ 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, - }, - cwd, - undefined, - { - userHooks: settings.getUserHooks(), - projectHooks: settings.getProjectHooks(), - }, - buildDisabledSkillNamesProvider(settings), + const config = await this.runWithPinnedRuntimeBaseDir(settings, cwd, () => + 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 { @@ -3167,120 +3266,266 @@ class QwenAgent implements Agent { } } - private async closeStoredSession( + private async removeStoredSessionEntry( sessionId: string, - opts?: { requireFlush?: boolean }, + session: Session, + cleanupErrors: unknown[] = [], + options: { shutdownConfig?: boolean } = {}, ): Promise { - for (const [requestId, generation] of this.generationControllers) { - if (generation.sessionId !== sessionId) continue; - generation.controller.abort(); - this.generationControllers.delete(requestId); - } - const session = this.sessions.get(sessionId); - if (!session) { - this.mcpPool?.releaseSession(sessionId); - return; + if (this.sessions.get(sessionId) !== session) return; + try { + session.dispose(); + } catch (error) { + cleanupErrors.push(error); } - - const requireFlush = opts?.requireFlush === true; - const flushRecording = async (): Promise => { + if (options.shutdownConfig !== false) { try { - await session.getConfig().getChatRecordingService()?.flush(); - return undefined; - } catch (err) { - debugLogger.debug( - `Session ${sessionId} chat recording flush during close failed: ${ - err instanceof Error ? err.message : String(err) - }`, - ); - return err; - } - }; - - if (requireFlush) { - const preCancelFlushError = await flushRecording(); - if (preCancelFlushError !== undefined) { - throw preCancelFlushError; + await session.getConfig().shutdown({ shutdownTelemetry: false }); + } 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); + if (cleanupErrors.length > 0) { + debugLogger.warn( + `Session ${sessionId} closed after ${cleanupErrors.length} cleanup failure(s): ${cleanupErrors + .map((error) => + error instanceof Error ? error.message : String(error), + ) + .join('; ')}`, + ); + } + } + private async withLiveSessionRestore( + sessionId: string, + session: Session, + operation: (config: Config, data: ResumedSessionData) => Promise, + ): Promise { + await session.assertCanStartTurn(); + const config = session.getConfig(); + const releaseGate = session.beginClose(); try { - await session.cancelPendingPrompt(); - } catch (err) { - debugLogger.debug( - `Session ${sessionId} cancel during close failed: ${ - err instanceof Error ? err.message : String(err) - }`, + await waitForSessionDrain( + session.waitForActiveTurnsToSettle(), + SESSION_DRAIN_TIMEOUT_MS, + 'restore', ); + const recorder = config.getChatRecordingService(); + const loadAuthoritative = () => + config.getSessionService().loadSession(sessionId); + const data = recorder + ? await recorder.runWithWriteBarrier(loadAuthoritative) + : await loadAuthoritative(); + if (!data) throw new SessionWriterUnavailableError(); + return await operation(config, data); + } catch (error) { + throw mapSessionWriterRequestError(error); + } finally { + releaseGate(); } + } - const flushError = await flushRecording(); - if (flushError !== undefined && requireFlush) { - throw flushError; + private async cleanupUnstoredConfig(config: Config): Promise { + const sessionId = config.getSessionId(); + const cleanupKey = this.pendingConfigCleanupKey( + config.getSessionRuntimeBaseDir(), + sessionId, + ); + try { + await shutdownSessionConfig(config); + } catch (error) { + const pending = this.pendingConfigCleanup.get(cleanupKey) ?? new Set(); + pending.add(config); + this.pendingConfigCleanup.set(cleanupKey, pending); + throw mapSessionWriterRequestError(error); } + const pending = this.pendingConfigCleanup.get(cleanupKey); + pending?.delete(config); + if (pending?.size === 0) { + this.pendingConfigCleanup.delete(cleanupKey); + } + } + private async cleanupAfterRequestFailure( + error: unknown, + cleanup: () => Promise, + ): Promise { 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) - }`, + await cleanup(); + } catch (cleanupError) { + debugLogger.warn( + `Session cleanup failed while preserving the original request error: ${cleanupError instanceof Error ? cleanupError.message : String(cleanupError)}`, ); } + throw error; + } - unregisterGoalHook(session.getConfig(), sessionId); - this.mcpPool?.releaseSession(sessionId); - uiTelemetryService.removeSession(sessionId); - this.sessions.delete(sessionId); + private pendingConfigCleanupKey( + runtimeBaseDir: string, + sessionId: string, + ): string { + return `${path.resolve(runtimeBaseDir)}\0${sessionId}`; + } + + private async retryPendingConfigCleanup( + runtimeBaseDir: string, + requiredSessionId?: string, + ): Promise { + const resolvedRuntimeBaseDir = path.resolve(runtimeBaseDir); + const configs = new Set(); + for (const pending of this.pendingConfigCleanup.values()) { + for (const config of pending) { + if ( + path.resolve(config.getSessionRuntimeBaseDir()) === + resolvedRuntimeBaseDir + ) { + configs.add(config); + } + } + } + for (const config of configs) { + try { + await this.cleanupUnstoredConfig(config); + } catch (error) { + if (config.getSessionId() === requiredSessionId) throw error; + debugLogger.warn( + `Deferred Config cleanup retry failed for session ${config.getSessionId()}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } } - private discardStoredSessionIfCurrent( + private async closeStoredSession( sessionId: string, - session: Session, - ): void { - if (this.sessions.get(sessionId) !== session) { + opts?: { + requireFlush?: boolean; + drainTimeoutMs?: number; + shutdownConfig?: boolean; + waitForCloseGate?: boolean; + }, + ): Promise { + const session = this.sessions.get(sessionId); + if (!session) { + this.mcpPool?.releaseSession(sessionId); 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); + + const recorder = session.getConfig().getChatRecordingService(); + const requireFlush = opts?.requireFlush === true; + if (requireFlush) { + await recorder?.flush(); } - try { - unregisterGoalHook(session.getConfig(), sessionId); - } catch (err) { - logCleanupFailure('goal hook unregister', err); + + const drainTimeoutMs = opts?.drainTimeoutMs ?? SESSION_DRAIN_TIMEOUT_MS; + const cancelClose = opts?.waitForCloseGate + ? await beginSessionCloseAfterCurrentGate(session, drainTimeoutMs) + : session.beginClose(); + for (const [requestId, generation] of this.generationControllers) { + if (generation.sessionId !== sessionId) continue; + generation.controller.abort(); + this.generationControllers.delete(requestId); } + let removedFromStore = false; try { - this.mcpPool?.releaseSession(sessionId); - } catch (err) { - logCleanupFailure('MCP pool release', err); + await waitForSessionDrain( + (async () => { + try { + await session.cancelPendingPrompt(); + } catch (err) { + debugLogger.debug( + `Session ${sessionId} cancel during close failed: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + await session.waitForActiveTurnsToSettle(); + })(), + drainTimeoutMs, + 'close', + ); + + recorder?.finalize(); + let flushError: unknown; + try { + await recorder?.flush(); + } catch (error) { + flushError = error; + } + if (flushError !== undefined && requireFlush) { + throw flushError; + } + + let closeError: unknown; + try { + await recorder?.close(); + } catch (error) { + closeError = error; + } + if (recorder?.hasWriteOwnership()) { + throw closeError ?? new SessionWriterUnavailableError(); + } + + const cleanupErrors: unknown[] = []; + if (flushError !== undefined) cleanupErrors.push(flushError); + if (closeError !== undefined) cleanupErrors.push(closeError); + await this.removeStoredSessionEntry(sessionId, session, cleanupErrors, { + shutdownConfig: opts?.shutdownConfig, + }); + removedFromStore = true; + } finally { + if (!removedFromStore) cancelClose(); } - try { - uiTelemetryService.removeSession(sessionId); - } catch (err) { - logCleanupFailure('telemetry removal', err); + } + + private async discardStoredSessionIfCurrent( + sessionId: string, + session: Session, + opts?: { + requireFlush?: boolean; + drainTimeoutMs?: number; + shutdownConfig?: boolean; + waitForCloseGate?: boolean; + }, + ): Promise { + if (this.sessions.get(sessionId) !== session) { + return; } - this.sessions.delete(sessionId); + await this.closeStoredSession(sessionId, opts); } - 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(); - } - this.sessions.clear(); + await Promise.allSettled( + [...this.sessions.entries()].map(([sessionId, session]) => + this.discardStoredSessionIfCurrent(sessionId, session, { + waitForCloseGate: true, + }), + ), + ); + await Promise.allSettled( + [...this.pendingConfigCleanup.values()] + .flatMap((configs) => [...configs]) + .map((config) => this.cleanupUnstoredConfig(config)), + ); this.disposeTranscriptReplayConfigs(); } @@ -3323,6 +3568,38 @@ class QwenAgent implements Agent { } } + private runWithPinnedRuntimeBaseDir( + settings: LoadedSettings, + cwd: string, + operation: () => T, + ): T { + return runWithAcpRuntimeOutputDir(settings, cwd, operation); + } + + private async assertLiveSessionScope( + config: Config, + settings: LoadedSettings, + cwd: string, + ): Promise { + if (path.resolve(config.storage.getProjectRoot()) !== path.resolve(cwd)) { + throw RequestError.invalidParams( + undefined, + 'The live session belongs to another workspace.', + ); + } + const requestedRuntimeBaseDir = await this.runWithPinnedRuntimeBaseDir( + settings, + cwd, + () => Storage.getRuntimeBaseDir(), + ); + if ( + path.resolve(config.getSessionRuntimeBaseDir()) !== + path.resolve(requestedRuntimeBaseDir) + ) { + throw mapSessionWriterRequestError(new SessionWriterUnavailableError()); + } + } + /** Expose the pool's workspace-scoped budget controller for snapshot builders. */ getWorkspaceMcpBudget(): WorkspaceMcpBudget | undefined { return this.workspaceMcpBudget; @@ -3474,14 +3751,24 @@ 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) { + return this.cleanupAfterRequestFailure(error, async () => { + if ( + this.sessions.get(config.getSessionId())?.getConfig() !== config + ) { + await this.cleanupUnstoredConfig(config); + } + }); + } profiler.setSessionId(session.getId()); return profiler.timeSync('response_build', () => ({ sessionId: session.getId(), @@ -3499,7 +3786,64 @@ class QwenAgent implements Agent { // resolve `advanced.runtimeOutputDir` from THIS request's cwd, not from // whichever settings a concurrent handler loaded last. const settings = loadSettingsCached(params.cwd); - const exists = await runWithAcpRuntimeOutputDir( + const liveSession = this.sessions.get(params.sessionId); + if (liveSession) { + const liveConfig = liveSession.getConfig(); + await this.assertLiveSessionScope(liveConfig, settings, params.cwd); + return this.withLiveSessionRestore( + params.sessionId, + liveSession, + async (config, sessionData) => { + const response: LoadSessionResponse = { + modes: this.buildModesData(config), + models: this.buildAvailableModels(config), + configOptions: this.buildConfigOptions(config), + ...(sessionData.artifactSnapshot + ? { artifactSnapshot: sessionData.artifactSnapshot } + : {}), + } as LoadSessionResponse; + const records = sessionData.conversation.messages; + if (records.length === 0) return response; + + const bulkReplay = isBulkLoadReplayRequest(params); + const replayPage = bulkReplay + ? selectRecentHistoryRecords(records, getLoadReplayPageSize(params)) + : { records, hasMore: false }; + const replay = await collectHistoryReplayUpdates({ + sessionId: params.sessionId, + config, + records: replayPage.records, + gaps: sessionData.historyGaps, + cumulativeUsage: createReplayCumulativeUsage(), + logger: debugLogger, + }); + if (!bulkReplay) { + for (const update of replay.updates) { + await liveSession.sendUpdate(update); + } + if (replay.replayError !== undefined) { + throw RequestError.internalError(undefined, replay.replayError); + } + return response; + } + + return { + ...response, + _meta: { + [LOAD_REPLAY_META_KEY]: { + v: LOAD_REPLAY_VERSION, + updates: replay.updates, + ...(replay.replayError !== undefined + ? { partial: true as const, replayError: replay.replayError } + : {}), + ...(replayPage.hasMore ? { hasMore: true as const } : {}), + }, + }, + }; + }, + ); + } + const exists = await this.runWithPinnedRuntimeBaseDir( settings, params.cwd, async () => { @@ -3526,22 +3870,30 @@ class QwenAgent implements Agent { params.sessionId, true, ); - await this.ensureAuthenticated(config); - this.setupFileSystem(config); - 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 } - : {}, - ); + let session: Session; + try { + await this.ensureAuthenticated(config); + this.setupFileSystem(config); + session = await this.createAndStoreSession( + config, + settings, + sessionData, + bulkReplay + ? { replayHistory: false, startPostReplayServices: false } + : {}, + ); + } catch (error) { + return this.cleanupAfterRequestFailure(error, async () => { + if (this.sessions.get(config.getSessionId())?.getConfig() !== config) { + await this.cleanupUnstoredConfig(config); + } + }); + } let replayEnvelope: BridgeLoadReplayEnvelope | undefined; if (bulkReplay) { try { @@ -3588,8 +3940,21 @@ class QwenAgent implements Agent { session.installRewriter(); session.startCronScheduler(); } catch (err) { - this.discardStoredSessionIfCurrent(params.sessionId, session); - throw err; + return this.cleanupAfterRequestFailure(err, async () => { + try { + await this.discardStoredSessionIfCurrent(params.sessionId, session); + } catch (cleanupError) { + await this.removeStoredSessionEntry( + params.sessionId, + session, + [cleanupError], + { + shutdownConfig: false, + }, + ); + await this.cleanupUnstoredConfig(config); + } + }); } } @@ -3624,7 +3989,25 @@ 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(); + await this.assertLiveSessionScope(liveConfig, settings, params.cwd); + return this.withLiveSessionRestore( + params.sessionId, + liveSession, + async (config, sessionData) => + ({ + modes: this.buildModesData(config), + models: this.buildAvailableModels(config), + configOptions: this.buildConfigOptions(config), + ...(sessionData.artifactSnapshot + ? { artifactSnapshot: sessionData.artifactSnapshot } + : {}), + }) as ResumeSessionResponse, + ); + } + const exists = await this.runWithPinnedRuntimeBaseDir( settings, params.cwd, async () => { @@ -3644,15 +4027,23 @@ 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) { + return this.cleanupAfterRequestFailure(error, async () => { + if (this.sessions.get(config.getSessionId())?.getConfig() !== config) { + await this.cleanupUnstoredConfig(config); + } + }); + } await this.#restoreWorktreeOnResume(config, session); this.#restoreGoalOnResume(config, session); @@ -6085,6 +6476,23 @@ class QwenAgent implements Agent { async extMethod( method: string, params: Record, + ): Promise> { + try { + return await this.extMethodInternal(method, params); + } catch (error) { + const writerError = getSessionWriterError(error); + if (writerError) { + throw new RequestError(writerError.rpcCode, writerError.message, { + errorKind: writerError.errorKind, + }); + } + throw error; + } + } + + private async extMethodInternal( + method: string, + params: Record, ): Promise> { const requestedCwd = typeof params['cwd'] === 'string' ? params['cwd'] : undefined; @@ -7411,6 +7819,34 @@ 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.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']; @@ -7515,8 +7951,24 @@ class QwenAgent implements Agent { 'Invalid or missing sessionId', ); } + const rawDrainTimeoutMs = params['drainTimeoutMs']; + if ( + rawDrainTimeoutMs !== undefined && + (typeof rawDrainTimeoutMs !== 'number' || + !Number.isSafeInteger(rawDrainTimeoutMs) || + rawDrainTimeoutMs < 1 || + rawDrainTimeoutMs > 2_147_483_647) + ) { + throw RequestError.invalidParams( + undefined, + 'Invalid session close drain timeout', + ); + } await this.closeStoredSession(sessionId, { requireFlush: params['requireFlush'] === true, + ...(typeof rawDrainTimeoutMs === 'number' + ? { drainTimeoutMs: rawDrainTimeoutMs } + : {}), }); return { sessionId, closed: true }; } @@ -8624,16 +9076,14 @@ class QwenAgent implements Agent { 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); - }, - ); + const config = session.getConfig(); + const recording = config.getChatRecordingService(); + await recording?.flush(); + const loadAuthoritative = () => + config.getSessionService().loadSession(sessionId); + const sessionData = recording + ? await recording.runWithWriteBarrier(loadAuthoritative) + : await loadAuthoritative(); if (sessionData === undefined) { artifactSnapshotUnavailable = 'session data unavailable after rewind'; @@ -8682,21 +9132,41 @@ class QwenAgent implements Agent { ); } - const sessionData = await runWithAcpRuntimeOutputDir( - this.settings, - cwd, - async () => { - const sessionService = new SessionService(cwd); - return sessionService.loadSession(sessionId); - }, - ); + const liveSession = this.sessions.get(sessionId); + let replayConfig = this.config; + let sessionData: ResumedSessionData | undefined; + if (liveSession) { + const config = liveSession.getConfig(); + await this.assertLiveSessionScope( + config, + loadSettingsCached(cwd), + cwd, + ); + const recording = config.getChatRecordingService(); + const loadAuthoritative = () => + config.getSessionService().loadSession(sessionId); + sessionData = recording + ? await recording.runWithWriteBarrier(loadAuthoritative) + : await loadAuthoritative(); + replayConfig = config; + } else { + const settings = loadSettingsCached(cwd); + sessionData = await this.runWithPinnedRuntimeBaseDir( + settings, + cwd, + async () => { + const sessionService = new SessionService(cwd); + return sessionService.loadSession(sessionId); + }, + ); + } if (!sessionData?.conversation) { return { updates: [] }; } const replay = await collectHistoryReplayUpdates({ sessionId, - config: this.config, + config: replayConfig, records: sessionData.conversation.messages, gaps: sessionData.historyGaps, cumulativeUsage: createReplayCumulativeUsage(), @@ -8770,61 +9240,56 @@ class QwenAgent implements Agent { }); } - const recording = sourceSession.getConfig().getChatRecordingService(); + const sourceConfig = sourceSession.getConfig(); + const recording = sourceConfig.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 sessionService = sourceConfig.getSessionService(); + await sessionService.forkSession(sessionId, newSessionId); - let title: string; - try { - let baseName: string; - if (typeof name === 'string' && name.trim().length > 0) { - baseName = name.trim(); - } else { - const existingTitle = recording?.getCurrentCustomTitle(); - const stripped = existingTitle - ?.replace(/\s*\(Branch(?:\s+\d+)?\)\s*$/, '') - .trim(); - if (stripped && stripped.length > 0) { - baseName = stripped; - } else { - baseName = sessionId.slice(0, 8); - } - } - - title = await computeUniqueBranchTitle(baseName, sessionService); - const renamed = await sessionService.renameSession( - newSessionId, - title, - 'manual', - ); - if (!renamed) { - throw new RequestError( - -32603, - `Failed to set title on forked session ${newSessionId}`, - { errorKind: 'internal', sessionId: newSessionId }, - ); - } - } 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`, - ); - }); - throw err; + let title: string; + try { + let baseName: string; + if (typeof name === 'string' && name.trim().length > 0) { + baseName = name.trim(); + } else { + const existingTitle = recording?.getCurrentCustomTitle(); + const stripped = existingTitle + ?.replace(/\s*\(Branch(?:\s+\d+)?\)\s*$/, '') + .trim(); + if (stripped && stripped.length > 0) { + baseName = stripped; + } else { + baseName = sessionId.slice(0, 8); } + } - return { newSessionId, title, displayName: title }; - }, - ); + title = await computeUniqueBranchTitle(baseName, sessionService); + const renamed = await sessionService.renameSession( + newSessionId, + title, + 'manual', + ); + if (!renamed) { + throw new RequestError( + -32603, + `Failed to set title on forked session ${newSessionId}`, + { errorKind: 'internal', sessionId: newSessionId }, + ); + } + } 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`, + ); + }); + throw err; + } + + return { newSessionId, title, displayName: title }; } case 'qwen/settings/getCore': { const settings = loadSettings(cwd); @@ -9332,17 +9797,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 +9851,43 @@ class QwenAgent implements Agent { sessionId?: string, resume?: boolean, initializeOptions: ConfigInitializeOptions = {}, + chatRecording?: boolean, + ): Promise { + try { + return await this.runWithPinnedRuntimeBaseDir(settings, cwd, async () => { + await this.retryPendingConfigCleanup( + Storage.getRuntimeBaseDir(), + sessionId, + ); + return 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?: boolean, ): 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 +9959,7 @@ class QwenAgent implements Agent { ...this.argv, ...sessionArg, continue: false, + ...(chatRecording !== undefined ? { chatRecording } : {}), }; const config = await loadCliConfig( @@ -9548,15 +10059,21 @@ 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) { + return this.cleanupAfterRequestFailure(error, () => + this.cleanupUnstoredConfig(config), + ); + } // 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,43 +10157,62 @@ 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); + try { + if (sessionData?.fileHistorySnapshots?.length) { + config + .getFileHistoryService() + .restoreFromSnapshots(sessionData.fileHistorySnapshots); + } - setTimeout(async () => { - await session.sendAvailableCommandsUpdate(); - }, 0); - - 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(); + setTimeout(() => { + void session.sendAvailableCommandsUpdate(); + }, 0); + return session; + } catch (error) { + try { + await this.discardStoredSessionIfCurrent(sessionId, session, { + shutdownConfig: false, + }); + } catch (cleanupError) { + await this.removeStoredSessionEntry( + sessionId, + session, + [cleanupError], + { shutdownConfig: false }, + ); + } + throw error; } - - return session; } private buildAvailableModels(config: Config): NewSessionResponse['models'] { diff --git a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts index b239d03c933..61b7e0e0d1b 100644 --- a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts @@ -148,6 +148,9 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ _args: args, })), SessionService: vi.fn(), + Storage: { + getRuntimeBaseDir: vi.fn(() => '/tmp/qwen-runtime-test'), + }, SESSION_TITLE_MAX_LENGTH: 200, DEFAULT_TOOL_OUTPUT_BATCH_BUDGET: 200_000, tokenLimit: vi.fn(), diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 84e5ecd02de..52b52c13196 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -517,6 +517,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 @@ -526,6 +529,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 @@ -673,6 +677,184 @@ describe('Session', () => { ); }); + it('rejects writer loss before mutating an existing ACP turn', async () => { + const activePrompt = new AbortController(); + const abort = vi.spyOn(activePrompt, 'abort'); + ( + session as unknown as { pendingPrompt: AbortController | null } + ).pendingPrompt = activePrompt; + vi.mocked(mockConfig.assertCanStartTurn).mockRejectedValueOnce( + new core.SessionWriterLostError(), + ); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }), + ).rejects.toMatchObject({ + code: -32021, + data: { errorKind: 'session_writer_lost' }, + }); + expect(abort).not.toHaveBeenCalled(); + expect(mockChatRecordingService.recordUserMessage).not.toHaveBeenCalled(); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + }); + + it('does not let a 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: -32021, + data: { errorKind: 'session_writer_lost' }, + }); + expect(nonInteractiveCliCommands.handleSlashCommand).not.toHaveBeenCalled(); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + }); + + it('holds the close gate until active turns settle', 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; + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = null; + releaseClose(); + }); + + it('does not reopen a disposed session when a close gate releases late', async () => { + const releaseClose = session.beginClose(); + const closeGateCompletion = session.waitForCloseGateToRelease(); + + expect(session.beginCloseIfAvailable()).toBeNull(); + session.dispose(); + await expect(closeGateCompletion).resolves.toBeUndefined(); + expect(() => session.beginCloseIfAvailable()).toThrow( + 'Session has been disposed', + ); + releaseClose(); + + expect(session.isIdle()).toBe(false); + await expect(session.assertCanStartTurn()).rejects.toMatchObject({ + code: -32602, + }); + }); + + it('pins durable cron startup, prompt restart, and stop to the session runtime', async () => { + const runtimeDir = path.resolve('runtime', 'cron-session'); + const observedStarts: string[] = []; + const observedStops: string[] = []; + const scheduler = { + hasPendingWork: false, + enableDurable: vi.fn().mockImplementation(async () => { + observedStarts.push(core.Storage.getRuntimeBaseDir()); + }), + start: vi.fn(), + stop: vi.fn().mockImplementation(() => { + observedStops.push(core.Storage.getRuntimeBaseDir()); + }), + list: vi.fn().mockReturnValue([]), + getExitSummary: vi.fn().mockReturnValue(undefined), + }; + session.dispose(); + core.Storage.setRuntimeBaseDir(runtimeDir); + mockConfig.isCronEnabled = vi.fn().mockReturnValue(true); + mockConfig.getCronScheduler = vi.fn().mockReturnValue(scheduler); + mockConfig.getWorkingDir = vi.fn().mockReturnValue('/logical-after-cd'); + mockChat.sendMessageStream = vi.fn().mockResolvedValue(createEmptyStream()); + session = new Session( + 'test-session-id', + mockConfig, + mockClient, + mockSettings, + ); + + session.startCronScheduler(); + await vi.waitFor(() => expect(scheduler.enableDurable).toHaveBeenCalled()); + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'hello' }], + }); + await vi.waitFor(() => + expect(scheduler.enableDurable).toHaveBeenCalledTimes(2), + ); + session.dispose(); + + expect(observedStarts).toEqual([runtimeDir, runtimeDir]); + expect(observedStops).toEqual([runtimeDir]); + }); + + it('does not resume automatic turns until an aborted prompt settles', async () => { + let resolvePromptCompletion!: () => void; + const promptCompletion = new Promise((resolve) => { + resolvePromptCompletion = resolve; + }); + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = promptCompletion; + mockChat.sendMessageStream = vi.fn().mockResolvedValue(createEmptyStream()); + + const releaseClose = session.beginClose(); + const callback = mockBackgroundTaskRegistry.setNotificationCallback.mock + .calls[0][0] as ( + displayText: string, + modelText: string, + meta: { agentId: string; status: string }, + ) => void; + callback('Background task completed.', '', { + agentId: 'agent-1', + status: 'completed', + }); + + releaseClose(); + await new Promise((resolve) => setImmediate(resolve)); + expect(mockChat.sendMessageStream).not.toHaveBeenCalled(); + + resolvePromptCompletion(); + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = null; + await vi.waitFor(() => { + expect(mockChat.sendMessageStream).toHaveBeenCalledOnce(); + }); + }); + describe('continueLastTurn', () => { it('returns none and starts no continuation when the last turn ended cleanly', async () => { vi.mocked(mockChat.getHistory).mockReturnValue([ @@ -1164,6 +1346,35 @@ describe('Session', () => { expect(mockChat.truncateHistory).not.toHaveBeenCalled(); }); + it('rejects history mutation until an aborted prompt actually settles', () => { + ( + session as unknown as { + pendingPromptCompletion: Promise | null; + } + ).pendingPromptCompletion = new Promise(() => {}); + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(() => session.restoreHistory([])).toThrow( + 'Cannot restore history while a prompt is running', + ); + expect(mockChat.truncateHistory).not.toHaveBeenCalled(); + expect(mockChat.setHistory).not.toHaveBeenCalled(); + }); + + it('rejects history mutation while close is in progress', () => { + const releaseClose = session.beginClose(); + + expect(() => session.rewindToTurn(0)).toThrow( + 'Cannot rewind while a prompt is running', + ); + expect(() => session.restoreHistory([])).toThrow( + 'Cannot restore history while a prompt is running', + ); + releaseClose(); + }); + it('rejects rewinds while a cron abort is active', () => { ( session as unknown as { cronAbortController: AbortController } diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 7e30586f5d1..95c4d39275d 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -117,6 +117,7 @@ import { shouldRunAutoModeForCall, extractDaemonTraceContext, withInteractionSpan, + SessionWriterError, startToolSpan, endToolSpan, runInToolSpanContext, @@ -1072,6 +1073,7 @@ export class Session implements SessionContext { * process termination is slow. */ private pendingPromptCompletion: Promise | null = null; + private automaticDrainRetry: Promise | null = null; /** * Per-turn AbortController for the fire-and-forget follow-up suggestion * generation. Aborted on the top of the next `prompt()` and on @@ -1137,6 +1139,9 @@ 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 closing = false; + private closeGateCompletion: Promise | null = null; + private resolveCloseGate: (() => void) | null = null; private unsubscribeChatRecordingFailure?: () => void; // Modular components @@ -1173,7 +1178,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() && @@ -1489,7 +1494,7 @@ export class Session implements SessionContext { */ startCronScheduler(): void { // Best-effort: a cron startup failure must not break session creation. - this.#startCronSchedulerIfNeeded().catch((error) => { + this.#startCronSchedulerInRuntime().catch((error) => { debugLogger.warn( `Cron scheduler startup failed [session ${this.sessionId}]: ${error}`, ); @@ -1500,17 +1505,112 @@ export class Session implements SessionContext { return this.config; } + async assertCanStartTurn(): Promise { + if (this.closing) { + throw RequestError.invalidParams(undefined, 'Session is closing'); + } + try { + await this.config.assertCanStartTurn(); + } catch (error) { + if (error instanceof SessionWriterError) { + throw new RequestError(error.rpcCode, error.message, { + errorKind: error.errorKind, + }); + } + throw error; + } + if (this.closing) { + throw RequestError.invalidParams(undefined, 'Session is closing'); + } + } + isIdle(): boolean { - return ( - !this.pendingPrompt && - !this.pendingPromptCompletion && - !this.cronProcessing && - !this.cronAbortController && - !this.notificationProcessing && - !this.notificationAbortController + return !this.closing && !this.#hasActiveTurn(); + } + + #hasActiveTurn(): boolean { + return Boolean( + this.pendingPrompt || + this.pendingPromptCompletion || + this.cronProcessing || + this.cronAbortController || + this.cronCompletion || + this.notificationProcessing || + this.notificationAbortController || + this.notificationCompletion, ); } + beginClose(): () => void { + if (this.closing) { + throw RequestError.invalidParams( + undefined, + 'Session close is already in progress', + ); + } + this.closing = true; + let resolveGate!: () => void; + const completion = new Promise((resolve) => { + resolveGate = resolve; + }); + this.closeGateCompletion = completion; + this.resolveCloseGate = resolveGate; + let released = false; + return () => { + if (released) return; + released = true; + if (this.closeGateCompletion === completion) { + this.closeGateCompletion = null; + this.resolveCloseGate = null; + } + resolveGate(); + if (this.disposed) return; + this.closing = false; + void this.#drainCronQueue(); + void this.#drainNotificationQueue(); + }; + } + + beginCloseIfAvailable(): (() => void) | null { + if (this.disposed) { + throw RequestError.invalidParams(undefined, 'Session has been disposed'); + } + return this.closing ? null : this.beginClose(); + } + + waitForCloseGateToRelease(): Promise { + return this.closeGateCompletion ?? Promise.resolve(); + } + + async waitForActiveTurnsToSettle(): Promise { + const pending = [ + this.pendingPromptCompletion, + this.cronCompletion, + this.notificationCompletion, + ].filter((completion): completion is Promise => completion !== null); + await Promise.allSettled(pending); + } + + #deferAutomaticQueueDrainUntilTurnsSettle(): boolean { + const completions = [ + this.pendingPromptCompletion, + this.cronCompletion, + this.notificationCompletion, + ].filter((completion): completion is Promise => completion !== null); + if (completions.length === 0) return false; + if (this.automaticDrainRetry) return true; + + const retry = Promise.allSettled(completions).then(() => { + if (this.automaticDrainRetry !== retry) return; + this.automaticDrainRetry = null; + if (this.disposed) return; + void this.#drainCronQueue(); + void this.#drainNotificationQueue(); + }); + this.automaticDrainRetry = retry; + return true; + } + getTurnCount(): number { return this.turn; } @@ -1521,6 +1621,10 @@ export class Session implements SessionContext { dispose(): void { this.disposed = true; + this.closing = true; + this.resolveCloseGate?.(); + this.resolveCloseGate = null; + this.closeGateCompletion = null; this.todoStopGuardQueuedPromptPriority = false; this.todoStopGuardDrainAutomaticQueuesWhenIdle = false; this.todoStopGuard.clearTrust(); @@ -1543,7 +1647,7 @@ export class Session implements SessionContext { // one-shots from disk without executing them) and the held lock // would block another session from taking over. if (this.config.isCronEnabled()) { - this.config.getCronScheduler().stop(); + this.#stopCronSchedulerInRuntime(); } this.config.getBackgroundTaskRegistry().setNotificationCallback(undefined); @@ -1647,13 +1751,7 @@ export class Session implements SessionContext { ); } - if ( - this.pendingPrompt || - this.cronProcessing || - this.cronAbortController || - this.notificationProcessing || - this.notificationAbortController - ) { + if (this.closing || this.#hasActiveTurn()) { throw RequestError.invalidParams( undefined, 'Cannot rewind while a prompt is running', @@ -1730,13 +1828,7 @@ export class Session implements SessionContext { } restoreHistory(history: Content[]): void { - if ( - this.pendingPrompt || - this.cronProcessing || - this.cronAbortController || - this.notificationProcessing || - this.notificationAbortController - ) { + if (this.closing || this.#hasActiveTurn()) { throw RequestError.invalidParams( undefined, 'Cannot restore history while a prompt is running', @@ -1848,7 +1940,7 @@ export class Session implements SessionContext { : null; if (scheduler) { const summary = scheduler.getExitSummary(); - scheduler.stop(); + this.#stopCronSchedulerInRuntime(); if (summary) { await this.messageEmitter.emitAgentMessage(summary); } @@ -1856,10 +1948,15 @@ export class Session implements SessionContext { } async prompt(params: PromptRequest): Promise { + if (this.closing) { + throw RequestError.invalidParams(undefined, 'Session is closing'); + } + await this.assertCanStartTurn(); const todoStopGuardPreparation = this.#prepareTodoStopGuardForPrompt(params); - // Install this prompt's AbortController before awaiting the previous - // prompt, so that a session/cancel during the wait targets us. + // After writer admission, install this prompt's AbortController before + // awaiting the previous prompt so a session/cancel during that wait + // targets us. A cancel during admission cannot target this pending prompt. this.pendingPrompt?.abort(); const pendingSend = new AbortController(); this.pendingPrompt = pendingSend; @@ -1942,6 +2039,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 = @@ -1960,7 +2064,7 @@ export class Session implements SessionContext { // hasPendingWork/disposed/disabled, so it only starts when a wakeup (or // cron job) is actually pending — otherwise the loop dies silently on // any post-arm error. - void this.#startCronSchedulerIfNeeded(); + void this.#startCronSchedulerInRuntime(); resolveCompletion(); this.pendingPromptCompletion = null; } @@ -2145,6 +2249,10 @@ export class Session implements SessionContext { this.runtimeBaseDir, this.config.getWorkingDir(), async () => { + await this.assertCanStartTurn(); + if (pendingSend.signal.aborted) { + return { stopReason: 'cancelled' }; + } // Increment turn counter for each user prompt this.turn += 1; @@ -4082,6 +4190,22 @@ export class Session implements SessionContext { }); } + #startCronSchedulerInRuntime(): Promise { + return Storage.runWithRuntimeBaseDir( + this.runtimeBaseDir, + this.config.getWorkingDir(), + () => this.#startCronSchedulerIfNeeded(), + ); + } + + #stopCronSchedulerInRuntime(): void { + Storage.runWithRuntimeBaseDir( + this.runtimeBaseDir, + this.config.getWorkingDir(), + () => this.config.getCronScheduler().stop(), + ); + } + #enqueueCronPrompt(item: CronQueueItem): void { if ( (this.todoStopGuard.blocksUnrelatedAutomaticTurns || @@ -4123,12 +4247,33 @@ export class Session implements SessionContext { */ async #drainCronQueue(): Promise { if (this.disposed) return; + if (this.closing) return; if (this.cronProcessing) return; // Don't process cron while a user prompt is active — the queue will be // drained after the prompt completes (see end of prompt()). if (this.pendingPrompt) return; if (this.notificationProcessing) return; + if (this.#deferAutomaticQueueDrainUntilTurnsSettle()) return; if (this.#nextCronQueueIndex() < 0) return; + try { + await this.assertCanStartTurn(); + } catch (error) { + debugLogger.warn( + `Cron turn rejected [session ${this.sessionId}]: ${error instanceof Error ? error.message : String(error)}`, + ); + return; + } + if ( + this.disposed || + this.closing || + this.cronProcessing || + this.pendingPrompt || + this.notificationProcessing || + this.#nextCronQueueIndex() < 0 + ) { + return; + } + if (this.#deferAutomaticQueueDrainUntilTurnsSettle()) return; this.cronProcessing = true; let resolveCompletion!: () => void; @@ -4158,7 +4303,7 @@ export class Session implements SessionContext { if (this.config.isCronEnabled()) { const scheduler = this.config.getCronScheduler(); if (!scheduler.hasPendingWork) { - scheduler.stop(); + this.#stopCronSchedulerInRuntime(); } } } @@ -4235,6 +4380,8 @@ export class Session implements SessionContext { async () => { let turnCount = 0; try { + await this.assertCanStartTurn(); + if (ac.signal.aborted) return; // 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 @@ -4708,13 +4855,36 @@ export class Session implements SessionContext { async #drainNotificationQueue(): Promise { if (this.disposed) return; + if (this.closing) return; if (this.notificationProcessing) return; if (this.pendingPrompt || this.cronProcessing || this.cronAbortController) { return; } + if (this.#deferAutomaticQueueDrainUntilTurnsSettle()) return; if (this.notificationQueue.length === 0) return; if (this.#nextNotificationQueueIndex() < 0) return; + try { + await this.assertCanStartTurn(); + } catch (error) { + debugLogger.warn( + `Notification turn rejected [session ${this.sessionId}]: ${error instanceof Error ? error.message : String(error)}`, + ); + return; + } + if ( + this.disposed || + this.closing || + this.notificationProcessing || + this.pendingPrompt || + this.cronProcessing || + this.cronAbortController || + this.#nextNotificationQueueIndex() < 0 + ) { + return; + } + if (this.#deferAutomaticQueueDrainUntilTurnsSettle()) return; + this.notificationProcessing = true; let resolveCompletion!: () => void; this.notificationCompletion = new Promise((resolve) => { @@ -4784,6 +4954,8 @@ export class Session implements SessionContext { const promptId = this.config.getSessionId() + '########notification' + Date.now(); try { + await this.assertCanStartTurn(); + if (ac.signal.aborted) return; 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..ad1d39f0c87 100644 --- a/packages/cli/src/acp-integration/session/Session.worktree.test.ts +++ b/packages/cli/src/acp-integration/session/Session.worktree.test.ts @@ -98,11 +98,15 @@ describe('Session.pendingWorktreeNotice', () => { }; mockConfig = { + storage: { + getRuntimeBaseDir: vi.fn(() => Storage.getRuntimeBaseDir()), + }, setApprovalMode: vi.fn(), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), switchModel: vi.fn(), getModel: vi.fn().mockReturnValue('qwen3'), getSessionId: vi.fn().mockReturnValue(SESSION_ID), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), getWorkingDir: vi.fn().mockReturnValue('/tmp'), getTelemetryLogPromptsEnabled: vi.fn().mockReturnValue(false), getUsageStatisticsEnabled: vi.fn().mockReturnValue(false), 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.ts b/packages/cli/src/config/config.ts index 2fbadccc3d2..e2ed40664bd 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1529,7 +1529,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; diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 08634692799..59e5621636b 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -716,7 +716,9 @@ export async function main() { markAcpStartup('configConstructionStart'); const config = await loadCliConfig( settings.merged, - argv, + argv.acp || argv.experimentalAcp + ? { ...argv, chatRecording: false } + : argv, 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..affb6b193e9 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -13,6 +13,7 @@ import { GROUP_COLOR_OPTIONS, SessionService, SessionOrganizationError, + SESSION_WRITER_RPC_CODES, type SessionGroupColor, type SessionGroupPresetColor, BuiltinAgentRegistry, @@ -140,6 +141,53 @@ function errMsg(err: unknown): string { return err instanceof Error ? err.message : String(err); } +const SESSION_WRITER_RPC_ERRORS = { + session_writer_conflict: { + code: SESSION_WRITER_RPC_CODES.session_writer_conflict, + message: 'This session is already open in another Qwen process.', + }, + session_writer_lost: { + code: SESSION_WRITER_RPC_CODES.session_writer_lost, + message: 'Write ownership for this session was lost.', + }, + session_transcript_changed: { + code: SESSION_WRITER_RPC_CODES.session_transcript_changed, + message: 'The session transcript changed outside its active writer.', + }, + session_writer_unavailable: { + code: SESSION_WRITER_RPC_CODES.session_writer_unavailable, + 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 +497,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 }; } @@ -1402,26 +1452,22 @@ export class AcpDispatcher { conn.ownedSessions.delete(sessionId); conn.closingSessions.add(sessionId); let closeStarted = false; - const closeSession = async () => { - closeStarted = true; + const closeLocalSessionStream = () => { 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))}`, - ); - } } }; + const closeSession = async () => { + closeStarted = true; + await this.bridge.closeSession( + sessionId, + this.sessionCtx(conn, sessionId, loopback), + ); + }; try { try { await this.archiveCoordinator.runExclusiveMany( @@ -1446,11 +1492,19 @@ export class AcpDispatcher { } catch (err) { if (!closeStarted) { conn.ownedSessions.add(sessionId); + } else { + try { + this.bridge.getSessionSummary(sessionId); + conn.ownedSessions.add(sessionId); + } catch { + closeLocalSessionStream(); + } } throw err; } finally { conn.closingSessions.delete(sessionId); } + closeLocalSessionStream(); this.replyConn(conn, id, {}); 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..7a9106e7b4f 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 !== undefined) throw this.loadError; if (this.loadShouldThrow) throw new Error('load failed'); if (this.gate) await this.gate; return { @@ -3658,6 +3660,46 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }); }); + it('session/load preserves sanitized session writer RPC errors', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440322'; + await writeStoredSession(sessionId); + bridge.loadError = Object.assign(new Error('private lock details'), { + code: -32020, + data: { errorKind: 'session_writer_conflict' }, + }); + + 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: 213, + method: 'session/load', + params: { sessionId }, + }); + + const [frame] = (await got) as Array<{ + id: number; + error: { + code: number; + message: string; + data?: { errorKind?: string }; + }; + }>; + expect(frame).toEqual({ + id: 213, + error: { + code: -32020, + message: 'This session is already open in another Qwen process.', + data: { errorKind: 'session_writer_conflict' }, + }, + jsonrpc: '2.0', + }); + }); + }); + it('session/load holds archive gate while restore is in flight', async () => { await withRuntimeDir(async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440124'; @@ -4544,6 +4586,9 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { it('session/close runs local cleanup even if the bridge close throws', async () => { bridge.closeShouldThrow = true; + bridge.getSessionSummary = () => { + throw new Error('session already gone'); + }; const connId = await initialize(); await newSession(connId); // creates + owns sess-1 await new Promise((r) => setTimeout(r, 30)); @@ -4560,6 +4605,35 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { expect(after.status).toBe(403); }); + it('session/close can be retried when the bridge reports a live refusal', async () => { + bridge.closeError = new Error('close drain refused'); + const connId = await initialize(); + await newSession(connId); + + await post(connId, { + jsonrpc: '2.0', + id: 146, + method: 'session/close', + params: { sessionId: 'sess-1' }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + const stillOwned = await openStream(connId, 'sess-1'); + expect(stillOwned.status).toBe(200); + + bridge.closeError = undefined; + await post(connId, { + jsonrpc: '2.0', + id: 147, + method: 'session/close', + params: { sessionId: 'sess-1' }, + }); + await new Promise((resolve) => setTimeout(resolve, 30)); + expect(bridge.closedSessions).toEqual(['sess-1', 'sess-1']); + const closed = await openStream(connId, 'sess-1'); + expect(closed.status).toBe(403); + await stillOwned.body?.cancel().catch(() => {}); + }); + it('connection cap → 503 on initialize', async () => { const app2 = express(); app2.use(express.json()); 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..1ed6fdacf18 --- /dev/null +++ b/packages/cli/src/serve/server/error-response.test.ts @@ -0,0 +1,89 @@ +/** + * @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'; + +function responseMock(): { + response: Response; + status: ReturnType; + json: ReturnType; +} { + const status = vi.fn(); + const json = vi.fn(); + const response = { status, json }; + status.mockReturnValue(response); + json.mockReturnValue(response); + return { response: response as unknown as Response, status, json }; +} + +describe('sendBridgeError session writer errors', () => { + it.each([ + { + error: new SessionWriterConflictError(), + status: 409, + kind: 'session_writer_conflict', + message: 'This session is already open in another Qwen process.', + }, + { + error: new SessionWriterLostError(), + status: 409, + kind: 'session_writer_lost', + message: 'Write ownership for this session was lost.', + }, + { + error: new SessionTranscriptChangedError(), + status: 409, + kind: 'session_transcript_changed', + message: 'The session transcript changed outside its active writer.', + }, + { + error: new SessionWriterUnavailableError({ + cause: new Error('private lock details'), + }), + status: 503, + kind: 'session_writer_unavailable', + message: 'Session write ownership could not be verified.', + }, + ])( + 'maps $kind without exposing diagnostics', + ({ error, status: expectedStatus, kind, message }) => { + const { response, status, json } = responseMock(); + + sendBridgeError(response, error); + + expect(status).toHaveBeenCalledWith(expectedStatus); + expect(json).toHaveBeenCalledWith({ + error: message, + code: kind, + errorKind: kind, + }); + }, + ); + + it('maps a serialized writer error with the fixed public message', () => { + const { response, status, json } = responseMock(); + const error = Object.assign(new Error('private lock details'), { + data: { errorKind: 'session_writer_unavailable' }, + }); + + sendBridgeError(response, error); + + 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/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 9d08aa0be3e..f561a3e160a 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -77,6 +77,10 @@ import { HookSystem } from '../hooks/index.js'; import { GOAL_HOOK_ID_OUTPUT_KEY } from '../goals/goalHook.js'; import type { FileHistorySnapshot } from '../services/fileHistoryService.js'; import type { ChatRecordingFailureEvent } from '../services/chatRecordingService.js'; +import { + SessionTranscriptChangedError, + SessionWriterLease, +} from '../services/session-writer-lease.js'; import * as jsonl from '../utils/jsonl-utils.js'; function createToolMock(toolName: string) { @@ -471,6 +475,7 @@ describe('Server Config (config.ts)', () => { userMemory: USER_MEMORY, telemetry: TELEMETRY_SETTINGS, model: MODEL, + chatRecording: false, usageStatisticsEnabled: false, overrideExtensions: [], }; @@ -516,7 +521,14 @@ describe('Server Config (config.ts)', () => { it('drops its session entry on shutdown — no daemon leak', async () => { const sessionId = 'cfg-shutdown-test-session'; const config = new Config({ ...baseParams, sessionId }); - // Registered in the constructor, resolvable while alive. + expect(getSessionProjectDir(sessionId)).toBeUndefined(); + await config.initialize({ + skipGeminiInitialization: true, + skipHooks: true, + skipMcpDiscovery: true, + skipSkillManager: true, + skipFileCheckpointing: true, + }); expect(getSessionProjectDir(sessionId)).toBeDefined(); await config.shutdown(); // In daemon mode this is what stops the map growing per session. @@ -2119,6 +2131,34 @@ describe('Server Config (config.ts)', () => { }); describe('startNewSession', () => { + it('rejects a session switch while the current recorder owns the writer lease', () => { + const config = new Config({ ...baseParams, chatRecording: true }); + const originalSessionId = config.getSessionId(); + const finalize = vi.fn(); + const flush = vi.fn().mockResolvedValue(undefined); + const recorder = { + finalize, + flush, + hasWriteOwnership: () => true, + }; + ( + config as unknown as { + chatRecordingService: typeof recorder; + } + ).chatRecordingService = recorder; + + expect(() => config.startNewSession('replacement-session')).toThrow( + expect.objectContaining({ + name: 'SessionWriterUnavailableError', + errorKind: 'session_writer_unavailable', + }), + ); + expect(config.getSessionId()).toBe(originalSessionId); + expect(config.getChatRecordingService()).toBe(recorder); + expect(finalize).not.toHaveBeenCalled(); + expect(flush).not.toHaveBeenCalled(); + }); + it('clears the FileReadCache so a new session does not inherit prior reads', () => { // Regression guard: the file-read cache backs ReadFile's // file_unchanged placeholder, whose correctness depends on the @@ -2165,9 +2205,14 @@ describe('Server Config (config.ts)', () => { chatRecordingService?: { finalize: () => void; flush: () => Promise; + hasWriteOwnership: () => boolean; }; } - ).chatRecordingService = { finalize, flush }; + ).chatRecordingService = { + finalize, + flush, + hasWriteOwnership: () => false, + }; config.startNewSession(); @@ -2220,6 +2265,7 @@ describe('Server Config (config.ts)', () => { await expect(recorder.flush()).rejects.toBe(error); expect(listener).toHaveBeenCalledOnce(); + await expect(config.assertCanStartTurn()).resolves.toBeUndefined(); expect(listener).toHaveBeenCalledWith({ sessionId, error }); } finally { writeLine.mockRestore(); @@ -2262,6 +2308,7 @@ describe('Server Config (config.ts)', () => { chatRecordingService: { finalize: () => void; flush: () => Promise; + close: () => Promise; }; } ).initialized = true; @@ -2270,6 +2317,7 @@ describe('Server Config (config.ts)', () => { chatRecordingService: { finalize: () => void; flush: () => Promise; + close: () => Promise; }; } ).chatRecordingService = { @@ -2277,6 +2325,7 @@ describe('Server Config (config.ts)', () => { flush: async () => { notify(config, event); }, + close: async () => {}, }; await config.shutdown(); @@ -2546,6 +2595,75 @@ describe('Server Config (config.ts)', () => { }); describe('initialize', () => { + it('preserves activation and lease release failures', async () => { + const config = new Config({ + ...baseParams, + chatRecording: true, + experimentalZedIntegration: true, + }); + const activationError = new SessionTranscriptChangedError(); + const releaseError = new Error('lease release failed'); + const release = vi.fn().mockRejectedValue(releaseError); + const acquire = vi + .spyOn(SessionWriterLease, 'acquire') + .mockResolvedValue({ + transcriptExistedAtAcquire: false, + release, + } as unknown as SessionWriterLease); + vi.spyOn( + config.getSessionService(), + 'getSessionLocation', + ).mockRejectedValue(activationError); + + const result = await ( + config as unknown as { activateChatRecording(): Promise } + ) + .activateChatRecording() + .catch((error: unknown) => error); + + expect(result).toMatchObject({ + name: 'SessionWriterUnavailableError', + errorKind: 'session_writer_unavailable', + rpcCode: -32023, + httpStatus: 503, + cause: expect.any(AggregateError), + }); + expect( + (result as Error & { cause: AggregateError }).cause.errors, + ).toEqual([activationError, releaseError]); + expect(release).toHaveBeenCalledOnce(); + acquire.mockRestore(); + }); + + it('preserves initialization and recording close failures', async () => { + const config = new Config(baseParams); + const initializationError = new Error('initialization failed'); + const closeError = new Error('recording close failed'); + vi.spyOn( + config as unknown as { + initializeInternal: () => Promise; + }, + 'initializeInternal', + ).mockRejectedValue(initializationError); + ( + config as unknown as { + chatRecordingService: { close: () => Promise }; + } + ).chatRecordingService = { + close: vi.fn().mockRejectedValue(closeError), + }; + + const result = await config.initialize().catch((error: unknown) => error); + + expect(result).toMatchObject({ + name: 'SessionWriterUnavailableError', + cause: expect.any(AggregateError), + }); + expect( + (result as Error & { cause: AggregateError }).cause.errors, + ).toEqual([initializationError, closeError]); + }); + it('should throw an error if initialized more than once', async () => { const config = new Config({ ...baseParams, @@ -4028,6 +4146,36 @@ describe('Server Config (config.ts)', () => { cwdSpy.mockRestore(); }); + it('relocateWorkingDirectory should preserve leased storage for an ACP cwd change', async () => { + const config = new Config(baseParams); + const originalStorage = config.storage; + const originalPersistenceRoot = originalStorage.getProjectRoot(); + const newDir = path.resolve('/path/to/other'); + ( + config as unknown as { + chatRecordingService: { hasWriteOwnership: () => boolean }; + } + ).chatRecordingService = { hasWriteOwnership: () => true }; + + await expect( + config.relocateWorkingDirectory(newDir, newDir, { + skipProcessChdir: true, + skipArtifactMigration: true, + }), + ).resolves.toEqual({}); + + expect(config.getTargetDir()).toBe(newDir); + expect(config.storage).toBe(originalStorage); + expect(config.getSessionService().getProjectRoot()).toBe( + originalPersistenceRoot, + ); + await expect(config.relocateWorkingDirectory(newDir)).rejects.toMatchObject( + { + errorKind: 'session_writer_unavailable', + }, + ); + }); + it('relocateWorkingDirectory should recreate cwd-derived file service', async () => { const config = new Config(baseParams); const newDir = path.resolve('/path/to/other'); @@ -4057,9 +4205,15 @@ describe('Server Config (config.ts)', () => { finalize: () => void; flush: () => Promise; resetStoragePaths: () => void; + hasWriteOwnership: () => boolean; }; } - ).chatRecordingService = { finalize, flush, resetStoragePaths }; + ).chatRecordingService = { + finalize, + flush, + resetStoragePaths, + hasWriteOwnership: () => false, + }; const chdirSpy = vi.spyOn(process, 'chdir').mockImplementation(() => { // Keep the test process in its original directory. }); @@ -4077,7 +4231,7 @@ describe('Server Config (config.ts)', () => { }); it('relocateWorkingDirectory should move current session artifacts to the new workspace', async () => { - const config = new Config(baseParams); + const config = new Config({ ...baseParams, chatRecording: true }); const sessionId = config.getSessionId(); const newDir = path.resolve('/path/to/other'); const oldStorage = new Storage(config.getTargetDir()); @@ -4178,7 +4332,7 @@ describe('Server Config (config.ts)', () => { }); it('relocateWorkingDirectory should reject and roll back when session artifact migration fails', async () => { - const config = new Config(baseParams); + const config = new Config({ ...baseParams, chatRecording: true }); const oldDir = config.getTargetDir(); const sessionId = config.getSessionId(); const newDir = path.resolve('/path/to/other'); @@ -5736,6 +5890,7 @@ describe('setApprovalMode with folder trust', () => { debugMode: false, model: 'test-model', cwd: '.', + chatRecording: false, }; it('should throw a TrustGateError when setting YOLO mode in an untrusted folder', () => { @@ -6551,6 +6706,7 @@ describe('disabledTools runtime sync (#4282 fold-in 5 P2-2 / #4297 fold-in 5)', debugMode: false, model: 'test-model', cwd: '.', + chatRecording: false, }; it('initializes from `disabledTools` ConfigParameters', () => { @@ -6613,6 +6769,7 @@ describe('visibleTools', () => { debugMode: false, model: 'test-model', cwd: '.', + chatRecording: false, }; it('initializes from `visibleTools` ConfigParameters', () => { @@ -6659,6 +6816,7 @@ describe('computer use settings', () => { debugMode: false, model: 'test-model', cwd: '.', + chatRecording: false, }; it('exposes the configured idle timeout', () => { @@ -6697,6 +6855,7 @@ describe('BaseLlmClient Lifecycle', () => { userMemory: USER_MEMORY, telemetry: TELEMETRY_SETTINGS, model: MODEL, + chatRecording: false, usageStatisticsEnabled: false, }; @@ -6735,6 +6894,7 @@ describe('Model Switching and Config Updates', () => { targetDir: '/path/to/target', debugMode: false, model: 'qwen3-coder-plus', + chatRecording: false, usageStatisticsEnabled: false, telemetry: { enabled: false }, }; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 7d95333ff26..86bedb8e350 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -196,6 +196,13 @@ import { SessionService, type ResumedSessionData, } from '../services/sessionService.js'; +import { + SessionTranscriptChangedError, + SessionWriterError, + SessionWriterLease, + SessionWriterLostError, + SessionWriterUnavailableError, +} from '../services/session-writer-lease.js'; import { randomUUID } from 'node:crypto'; import { loadServerHierarchicalMemory } from '../utils/memoryDiscovery.js'; import { ConditionalRulesRegistry } from '../utils/rulesDiscovery.js'; @@ -1568,6 +1575,9 @@ export type SubSessionSpawner = ( export class Config { private sessionId: string; private sessionData?: ResumedSessionData; + private readonly sessionRuntimeBaseDir: string; + private sessionProjectDirRegistered = false; + private pendingSessionWriterLease?: SessionWriterLease; /** * One-shot notice produced by `setupStartupWorktree` (Phase D-1) when the * CLI was launched with `--worktree`. The active entry point (TUI XOR @@ -1870,6 +1880,7 @@ export class Config { private readonly settingsWatcher?: { stopWatching(): void }; constructor(params: ConfigParameters) { + this.sessionRuntimeBaseDir = Storage.getRuntimeBaseDir(); this.sessionId = params.sessionId ?? randomUUID(); // Only set the global env marker once per process lifetime, so // throwaway Config instances (e.g. telemetry-only) don't clobber @@ -2126,7 +2137,7 @@ export class Config { this.jsonSchema = params.jsonSchema; this.inputFile = params.inputFile; this.defaultFileEncoding = params.defaultFileEncoding; - this.storage = new Storage(this.targetDir); + this.storage = new Storage(this.targetDir, this.sessionRuntimeBaseDir); // Publish the project dir a subprocess needs to find this session's harness // records. It is derived from the session's *launch* cwd, so a subprocess // that has `cd`-ed elsewhere — which the /review skill explicitly does, into @@ -2138,7 +2149,6 @@ export class Config { // booted first, and every later session would hand its subprocesses another // session's directory. The env var is still set for the single-session CLI, // where it is the only consumer and there is nothing to collide with. - registerSessionProjectDir(this.sessionId, this.storage.getProjectDir()); if (!projectDirEnvClaimed && process.env) { process.env['QWEN_CODE_PROJECT_DIR'] = this.storage.getProjectDir(); projectDirEnvClaimed = true; @@ -2270,6 +2280,33 @@ export class Config { throw Error('Config was already initialized'); } this.initialized = true; + try { + await this.activateChatRecording(); + registerSessionProjectDir(this.sessionId, this.storage.getProjectDir()); + this.sessionProjectDirRegistered = true; + await this.initializeInternal(options); + } catch (error) { + if (this.sessionProjectDirRegistered) { + unregisterSessionProjectDir(this.sessionId); + this.sessionProjectDirRegistered = false; + } + try { + await this.chatRecordingService?.close(); + } catch (closeError) { + throw new SessionWriterUnavailableError({ + cause: new AggregateError( + [error, closeError], + 'Chat recording close failed during failed initialization', + ), + }); + } + throw error; + } + } + + private async initializeInternal( + options?: ConfigInitializeOptions, + ): Promise { this.debugLogger.info('Config initialization started'); if (options?.skipFileCheckpointing === true) { this.fileCheckpointingEnabled = false; @@ -2734,6 +2771,78 @@ export class Config { } } + private async activateChatRecording(): Promise { + if (!this.chatRecordingEnabled || !this.experimentalZedIntegration) return; + const recorder = this.chatRecordingService; + if (!recorder) throw new SessionWriterUnavailableError(); + let lease: SessionWriterLease | undefined; + try { + lease = await SessionWriterLease.acquire({ + runtimeBaseDir: this.sessionRuntimeBaseDir, + sessionId: this.sessionId, + transcriptPath: this.getTranscriptPath(), + processKind: 'acp', + qwenVersion: this.cliVersion ?? null, + onOwnershipAcquired: (acquiredLease) => { + lease = acquiredLease; + this.pendingSessionWriterLease = acquiredLease; + }, + }); + const location = await this.getSessionService().getSessionLocation( + this.sessionId, + ); + if (location === 'conflict' || location === 'archived') { + throw new SessionTranscriptChangedError(); + } + let authoritative: ResumedSessionData | undefined; + if (this.sessionData || lease.transcriptExistedAtAcquire) { + authoritative = await this.getSessionService().loadSession( + this.sessionId, + ); + if (!authoritative) throw new SessionWriterUnavailableError(); + } else if (location !== undefined) { + throw new SessionTranscriptChangedError(); + } + const persistedTitleInfo = authoritative + ? this.getSessionService().getSessionTitleInfo(this.sessionId) + : undefined; + await lease.assertOwnedAndUnchanged(); + this.sessionData = authoritative; + recorder.activate(lease, authoritative, persistedTitleInfo); + this.pendingSessionWriterLease = undefined; + lease = undefined; + } catch (error) { + let failure: unknown = error; + if ( + !(failure instanceof SessionWriterError) && + failure && + typeof failure === 'object' && + typeof (failure as NodeJS.ErrnoException).code === 'string' + ) { + failure = new SessionWriterUnavailableError({ cause: failure }); + } + try { + const ownedLease = lease ?? this.pendingSessionWriterLease; + await ownedLease?.release(); + if (this.pendingSessionWriterLease === ownedLease) { + this.pendingSessionWriterLease = undefined; + } + } catch (releaseError) { + if (releaseError instanceof SessionWriterLostError) { + this.pendingSessionWriterLease = undefined; + } else { + failure = new SessionWriterUnavailableError({ + cause: new AggregateError( + [failure, releaseError], + 'Session writer lease release failed during activation cleanup', + ), + }); + } + } + throw failure; + } + } + /** * In-flight background MCP discovery promise. Captured so non-interactive * code paths can await it before invoking the model (see @@ -3251,6 +3360,9 @@ export class Config { sessionId?: string, sessionData?: ResumedSessionData, ): string { + if (this.chatRecordingService?.hasWriteOwnership()) { + throw new SessionWriterUnavailableError(); + } // Finalize the outgoing session before switching. const outgoingChatRecordingService = this.chatRecordingService; try { @@ -4066,6 +4178,12 @@ export class Config { expectedCanonicalDir?: string, opts?: { skipProcessChdir?: boolean; skipArtifactMigration?: boolean }, ): Promise<{ memoryRefreshError?: unknown }> { + if ( + !opts?.skipArtifactMigration && + this.chatRecordingService?.hasWriteOwnership() + ) { + throw new SessionWriterUnavailableError(); + } const oldDir = opts?.skipProcessChdir ? this.cwd : fs.realpathSync(process.cwd()); @@ -4102,7 +4220,7 @@ export class Config { const oldStorage = this.storage; if (!opts?.skipArtifactMigration) { - const newStorage = new Storage(expected); + const newStorage = new Storage(expected, this.sessionRuntimeBaseDir); await this.prepareSessionArtifactMigration( oldStorage, newStorage, @@ -4174,13 +4292,16 @@ export class Config { * This method is idempotent and safe to call multiple times. * It handles the case where initialization was not completed. */ - async shutdown(): Promise { + async shutdown(options?: { shutdownTelemetry?: boolean }): Promise { try { - // Drop this session's project-dir registry entry. It is registered in the - // constructor, so it is released here regardless of initialization state — + // Drop this session's project-dir registry entry. It is registered during + // initialization, so it is released here whenever that step completed — // in daemon mode, where one process serves many sessions, an unreleased // entry per session is a leak that grows for the life of the process. - unregisterSessionProjectDir(this.sessionId); + if (this.sessionProjectDirRegistered) { + unregisterSessionProjectDir(this.sessionId); + this.sessionProjectDirRegistered = false; + } // Stop the settings watcher regardless of initialization state — // it is started before Config.initialize() and would leak otherwise. @@ -4216,8 +4337,31 @@ export class Config { // Log but don't throw - cleanup should be best-effort this.debugLogger.error('Error during Config shutdown:', error); } finally { + await this.chatRecordingService?.close().catch((error) => { + this.debugLogger.error( + 'Failed to release session writer lease:', + error, + ); + }); + const pendingLease = this.pendingSessionWriterLease; + if (pendingLease) { + try { + await pendingLease.release(); + if (this.pendingSessionWriterLease === pendingLease) { + this.pendingSessionWriterLease = undefined; + } + } catch (error) { + if (error instanceof SessionWriterLostError) { + this.pendingSessionWriterLease = undefined; + } + this.debugLogger.error( + 'Failed to release pending session writer lease:', + error, + ); + } + } this.chatRecordingFailureListeners.clear(); - if (isTelemetrySdkInitialized()) { + if (options?.shutdownTelemetry !== false && isTelemetrySdkInitialized()) { await shutdownTelemetry(); } } @@ -6194,9 +6338,13 @@ export class Config { } private createChatRecordingService(): ChatRecordingService { - return new ChatRecordingService(this, (event) => { - this.notifyChatRecordingFailure(event); - }); + return new ChatRecordingService( + this, + (event) => { + this.notifyChatRecordingFailure(event); + }, + this.experimentalZedIntegration, + ); } private notifyChatRecordingFailure(event: ChatRecordingFailureEvent): void { @@ -6232,12 +6380,31 @@ export class Config { return path.join(projectDir, 'chats', safeFilename); } + async assertCanStartTurn(): Promise { + if (this.chatRecordingService?.hasWriteOwnership()) { + await this.chatRecordingService.assertCanStartTurn(); + } + } + + hasSessionWriteOwnership(): boolean { + return ( + this.pendingSessionWriterLease !== undefined || + this.chatRecordingService?.hasWriteOwnership() === true + ); + } + + getSessionRuntimeBaseDir(): string { + return this.sessionRuntimeBaseDir; + } + /** * Gets or creates a SessionService for managing chat sessions. */ getSessionService(): SessionService { if (!this.sessionService) { - this.sessionService = new SessionService(this.targetDir); + this.sessionService = new SessionService(this.storage.getProjectRoot(), { + runtimeBaseDir: this.sessionRuntimeBaseDir, + }); } return this.sessionService; } diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 4d7a7bfaf54..cb54ba6db9f 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -641,4 +641,24 @@ describe('Storage – runtime base dir async context isolation', () => { expect(a).toBe(path.join(cwdA, '.qwen-a')); expect(b).toBe(path.join(cwdB, '.qwen-b')); }); + + it('pins an instance to the runtime dir where it was created', () => { + const cwd = path.resolve('workspace', 'pinned'); + const runtimeDir = path.join(cwd, '.qwen-a'); + const storage = Storage.runWithRuntimeBaseDir( + '.qwen-a', + cwd, + () => new Storage(cwd), + ); + + Storage.runWithRuntimeBaseDir('.qwen-b', cwd, () => { + expect(storage.getRuntimeBaseDir()).toBe(runtimeDir); + expect(storage.getProjectDir()).toContain( + path.join(runtimeDir, 'projects'), + ); + expect(storage.getProjectTempDir()).toContain( + path.join(runtimeDir, 'tmp'), + ); + }); + }); }); diff --git a/packages/core/src/config/storage.ts b/packages/core/src/config/storage.ts index 98d21386ad7..7d0666d695e 100644 --- a/packages/core/src/config/storage.ts +++ b/packages/core/src/config/storage.ts @@ -35,6 +35,7 @@ function isResolvedPathWithinDirectory(childPath: string, parentPath: string) { export class Storage { private readonly targetDir: string; + private readonly runtimeBaseDir: string; /** * Custom runtime output base directory set via settings. @@ -45,8 +46,12 @@ export class Storage { string | null >(); - constructor(targetDir: string) { + constructor( + targetDir: string, + runtimeBaseDir: string = Storage.getRuntimeBaseDir(), + ) { this.targetDir = targetDir; + this.runtimeBaseDir = path.resolve(runtimeBaseDir); } /** @@ -126,6 +131,10 @@ export class Storage { return Storage.runtimeBaseDirContext.run(resolved, fn); } + static hasRuntimeBaseDirContext(): boolean { + return Storage.runtimeBaseDirContext.getStore() !== undefined; + } + /** * Returns the base directory for all runtime output (temp files, debug logs, * session data, todos, insights, etc.). @@ -310,18 +319,19 @@ export class Storage { return path.join(this.targetDir, QWEN_DIR); } + getRuntimeBaseDir(): string { + return this.runtimeBaseDir; + } + getProjectDir(): string { const projectId = sanitizeCwd(this.getProjectRoot()); - const projectsDir = path.join( - Storage.getRuntimeBaseDir(), - PROJECT_DIR_NAME, - ); + const projectsDir = path.join(this.runtimeBaseDir, PROJECT_DIR_NAME); return path.join(projectsDir, projectId); } getProjectTempDir(): string { const hash = getProjectHash(this.getProjectRoot()); - const tempDir = Storage.getGlobalTempDir(); + const tempDir = path.join(this.runtimeBaseDir, TMP_DIR_NAME); const targetDir = path.join(tempDir, hash); return targetDir; } diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index afd100ca2c9..45be811b669 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -598,6 +598,7 @@ describe('Gemini Client (client.ts)', () => { // Explicit values are hard caps; the cap tests below set a finite value // and rely on hard-cap behavior. isMaxToolCallsPerTurnExplicit: vi.fn().mockReturnValue(true), + assertCanStartTurn: vi.fn().mockResolvedValue(undefined), getChatRecordingService: vi.fn().mockReturnValue(undefined), getFileHistoryService: vi.fn().mockReturnValue(mockFileHistoryService), getResumedSessionData: vi.fn().mockReturnValue(undefined), @@ -4131,6 +4132,47 @@ describe('Gemini Client (client.ts)', () => { expect(history).toContain('pdf-bytes'); }); + it.each([ + SendMessageType.UserQuery, + SendMessageType.Cron, + SendMessageType.Notification, + SendMessageType.Teammate, + ])('checks session writer admission before a %s turn', async (type) => { + const failure = new Error('writer admission failed'); + vi.mocked(mockConfig.assertCanStartTurn).mockRejectedValueOnce(failure); + + const stream = client.sendMessageStream( + [{ text: 'blocked' }], + new AbortController().signal, + `prompt-${type}`, + { type }, + ); + + await expect(stream.next()).rejects.toBe(failure); + expect(mockTurnRunFn).not.toHaveBeenCalled(); + }); + + it('does not re-run session writer admission for a mid-turn hook continuation', async () => { + mockTurnRunFn.mockReturnValue( + (async function* () { + yield { type: GeminiEventType.Content, value: 'continued' }; + })(), + ); + + const stream = client.sendMessageStream( + [{ text: 'continue' }], + new AbortController().signal, + 'prompt-hook', + { type: SendMessageType.Hook }, + ); + for await (const _ of stream) { + // drain + } + + expect(mockConfig.assertCanStartTurn).not.toHaveBeenCalled(); + expect(mockTurnRunFn).toHaveBeenCalled(); + }); + it('should merge editor context into the user request when ideMode is enabled', async () => { // Arrange vi.mocked(ideContextStore.get).mockReturnValue({ diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index c6e4c9fda75..4e7a3ebb178 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -1843,6 +1843,14 @@ export class GeminiClient { turns: number = MAX_TURNS, ): AsyncGenerator { const messageType = options?.type ?? SendMessageType.UserQuery; + if ( + messageType === SendMessageType.UserQuery || + messageType === SendMessageType.Cron || + messageType === SendMessageType.Notification || + messageType === SendMessageType.Teammate + ) { + await this.config.assertCanStartTurn(); + } let strippedRetryEntries: Content[] = []; // Snapshot of GeminiChat's user-content push counter, taken right after the // strip. The Retry's re-submitted content is the first thing the send diff --git a/packages/core/src/extension/extensionManager.ts b/packages/core/src/extension/extensionManager.ts index 4f23daf605c..f20f9ec85b4 100644 --- a/packages/core/src/extension/extensionManager.ts +++ b/packages/core/src/extension/extensionManager.ts @@ -361,6 +361,7 @@ function getTelemetryConfig( cwd, model: '', debugMode: false, + chatRecording: false, }); return config; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0f760933d94..e09a83af2ac 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -247,6 +247,7 @@ export * from './services/visionBridge/image-capability.js'; export * from './services/sessionRecap.js'; export * from './services/session-artifact-persistence.js'; export * from './services/sessionService.js'; +export * from './services/session-writer-lease.js'; export { decodeSessionTranscriptCursor, encodeSessionTranscriptCursor, diff --git a/packages/core/src/services/chatRecordingService.autoTitle.test.ts b/packages/core/src/services/chatRecordingService.autoTitle.test.ts index 912f8f8b662..08c51889b08 100644 --- a/packages/core/src/services/chatRecordingService.autoTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.autoTitle.test.ts @@ -15,6 +15,7 @@ import { type ChatRecord, } from './chatRecordingService.js'; import * as jsonl from '../utils/jsonl-utils.js'; +import type { SessionWriterLease } from './session-writer-lease.js'; const tryGenerateSessionTitleMock = vi.fn(); @@ -73,9 +74,42 @@ function findCustomTitleRecord(): ChatRecord | undefined { .find((r) => r.type === 'system' && r.subtype === 'custom_title'); } +function resumedSessionWithTitle( + title: string, + source?: 'manual' | 'auto', +): NonNullable> { + return { + conversation: { + sessionId: 'test-session-id', + projectHash: 'test-project', + startTime: '2026-01-01T00:00:00.000Z', + lastUpdated: '2026-01-01T00:00:00.000Z', + messages: [ + { + uuid: 'title-uuid', + parentUuid: null, + sessionId: 'test-session-id', + timestamp: '2026-01-01T00:00:00.000Z', + type: 'system', + subtype: 'custom_title', + cwd: '/test/project/root', + version: '1.0.0', + systemPayload: { + customTitle: title, + ...(source ? { titleSource: source } : {}), + }, + }, + ], + }, + filePath: '/test/session.jsonl', + lastCompletedUuid: 'parent-uuid', + }; +} + describe('ChatRecordingService - auto-title trigger', () => { let chatRecordingService: ChatRecordingService; let mockConfig: Config; + let mockLease: SessionWriterLease; let fastModelValue: string | undefined; let uuidCounter = 0; @@ -133,13 +167,41 @@ describe('ChatRecordingService - auto-title trigger', () => { vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined); vi.spyOn(fs, 'existsSync').mockReturnValue(false); - chatRecordingService = new ChatRecordingService(mockConfig); - // writeLine is async; mockResolvedValue lets the writeChain settle when // tests await flushMicrotasks() / chatRecordingService.flush(). vi.mocked(jsonl.writeLine).mockResolvedValue(undefined); + mockLease = { + sessionId: 'test-session-id', + ownerId: 'test-owner-id', + appendJsonLine: vi.fn((record: unknown) => + jsonl.writeLine('/test/session.jsonl', record), + ), + assertOwnedAndUnchanged: vi.fn().mockResolvedValue(undefined), + release: vi.fn().mockResolvedValue(undefined), + } as unknown as SessionWriterLease; + chatRecordingService = activateRecording( + new ChatRecordingService(mockConfig, undefined, true), + mockConfig, + ); }); + function activateRecording( + service: ChatRecordingService, + config: Config, + ): ChatRecordingService { + const resumed = config.getResumedSessionData(); + service.activate( + mockLease, + resumed && !resumed.conversation + ? { + conversation: { messages: [] }, + lastCompletedUuid: resumed.lastCompletedUuid, + } + : resumed, + ); + return service; + } + afterEach(() => { vi.restoreAllMocks(); }); @@ -352,13 +414,18 @@ describe('ChatRecordingService - auto-title trigger', () => { }; const resumedConfig = { ...mockConfig, - getResumedSessionData: vi.fn().mockReturnValue({ - lastCompletedUuid: 'parent-uuid', - }), + getResumedSessionData: vi + .fn() + .mockReturnValue( + resumedSessionWithTitle('Auto-generated title', 'auto'), + ), getSessionService: vi.fn().mockReturnValue(mockSessionService), } as unknown as Config; - const svc = new ChatRecordingService(resumedConfig); + const svc = activateRecording( + new ChatRecordingService(resumedConfig, undefined, true), + resumedConfig, + ); expect(svc.getCurrentCustomTitle()).toBe('Auto-generated title'); expect(svc.getCurrentTitleSource()).toBe('auto'); @@ -390,13 +457,16 @@ describe('ChatRecordingService - auto-title trigger', () => { }; const resumedConfig = { ...mockConfig, - getResumedSessionData: vi.fn().mockReturnValue({ - lastCompletedUuid: 'parent-uuid', - }), + getResumedSessionData: vi + .fn() + .mockReturnValue(resumedSessionWithTitle('User chose this', 'manual')), getSessionService: vi.fn().mockReturnValue(mockSessionService), } as unknown as Config; - const svc = new ChatRecordingService(resumedConfig); + const svc = activateRecording( + new ChatRecordingService(resumedConfig, undefined, true), + resumedConfig, + ); expect(svc.getCurrentCustomTitle()).toBe('User chose this'); expect(svc.getCurrentTitleSource()).toBe('manual'); @@ -423,13 +493,16 @@ describe('ChatRecordingService - auto-title trigger', () => { }; const resumedConfig = { ...mockConfig, - getResumedSessionData: vi.fn().mockReturnValue({ - lastCompletedUuid: 'parent-uuid', - }), + getResumedSessionData: vi + .fn() + .mockReturnValue(resumedSessionWithTitle('Legacy title')), getSessionService: vi.fn().mockReturnValue(mockSessionService), } as unknown as Config; - const svc = new ChatRecordingService(resumedConfig); + const svc = activateRecording( + new ChatRecordingService(resumedConfig, undefined, true), + resumedConfig, + ); expect(svc.getCurrentCustomTitle()).toBe('Legacy title'); // Must stay undefined so the JSONL isn't upgraded to a misleading diff --git a/packages/core/src/services/chatRecordingService.customTitle.test.ts b/packages/core/src/services/chatRecordingService.customTitle.test.ts index 4d975c72d02..7c7be1276a1 100644 --- a/packages/core/src/services/chatRecordingService.customTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.customTitle.test.ts @@ -16,6 +16,7 @@ import { type CustomTitleRecordPayload, } from './chatRecordingService.js'; import * as jsonl from '../utils/jsonl-utils.js'; +import type { SessionWriterLease } from './session-writer-lease.js'; vi.mock('node:path'); vi.mock('node:child_process'); @@ -29,9 +30,42 @@ vi.mock('node:crypto', () => ({ })); vi.mock('../utils/jsonl-utils.js'); +function resumedSessionWithTitle( + title: string, + source?: 'manual' | 'auto', +): NonNullable> { + return { + conversation: { + sessionId: 'test-session-id', + projectHash: 'test-project', + startTime: '2026-01-01T00:00:00.000Z', + lastUpdated: '2026-01-01T00:00:00.000Z', + messages: [ + { + uuid: 'title-uuid', + parentUuid: null, + sessionId: 'test-session-id', + timestamp: '2026-01-01T00:00:00.000Z', + type: 'system', + subtype: 'custom_title', + cwd: '/test/project/root', + version: '1.0.0', + systemPayload: { + customTitle: title, + ...(source ? { titleSource: source } : {}), + }, + }, + ], + }, + filePath: '/test/session.jsonl', + lastCompletedUuid: null, + }; +} + describe('ChatRecordingService - recordCustomTitle', () => { let chatRecordingService: ChatRecordingService; let mockConfig: Config; + let mockLease: SessionWriterLease; let uuidCounter = 0; @@ -79,12 +113,38 @@ describe('ChatRecordingService - recordCustomTitle', () => { vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined); vi.spyOn(fs, 'existsSync').mockReturnValue(false); - chatRecordingService = new ChatRecordingService(mockConfig); - // writeLine is async; mockResolvedValue lets the writeChain settle on flush. vi.mocked(jsonl.writeLine).mockResolvedValue(undefined); + mockLease = { + sessionId: 'test-session-id', + ownerId: 'test-owner-id', + appendJsonLine: vi.fn((record: unknown) => + jsonl.writeLine('/test/session.jsonl', record), + ), + assertOwnedAndUnchanged: vi.fn().mockResolvedValue(undefined), + release: vi.fn().mockResolvedValue(undefined), + } as unknown as SessionWriterLease; + chatRecordingService = activateRecording( + new ChatRecordingService(mockConfig), + ); }); + function activateRecording( + service: ChatRecordingService, + ): ChatRecordingService { + const resumed = mockConfig.getResumedSessionData(); + service.activate( + mockLease, + resumed && !resumed.conversation + ? { + conversation: { messages: [] }, + lastCompletedUuid: resumed.lastCompletedUuid, + } + : resumed, + ); + return service; + } + afterEach(() => { vi.restoreAllMocks(); }); @@ -167,7 +227,9 @@ describe('ChatRecordingService - recordCustomTitle', () => { it('returns false after an async failure and permanently rejects later titles', async () => { const failureListener = vi.fn(); - const service = new ChatRecordingService(mockConfig, failureListener); + const service = activateRecording( + new ChatRecordingService(mockConfig, failureListener), + ); const callback = vi.fn(); service.setTitleRecordedCallback(callback); const writeError = new Error('disk full'); @@ -198,8 +260,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { expect(chatRecordingService.getCurrentTitleSource()).toBe('manual'); }); - it('allows retry after a synchronous conversation-file failure', async () => { - const service = new ChatRecordingService(mockConfig); + it('allows legacy retry after a synchronous conversation-file failure', async () => { + const service = new ChatRecordingService(mockConfig, undefined, false); vi.mocked(fs.writeFileSync).mockImplementationOnce(() => { throw Object.assign(new Error('permission denied'), { code: 'EACCES' }); }); @@ -384,8 +446,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { it('should not re-append a resumed title without new content', async () => { vi.mocked(mockConfig.getResumedSessionData).mockReturnValue({ - lastCompletedUuid: null, - } as unknown as ReturnType); + ...resumedSessionWithTitle('resumed-title', 'manual'), + }); const getSessionTitleInfo = vi.fn().mockReturnValue({ title: 'resumed-title', source: 'manual', @@ -398,7 +460,7 @@ describe('ChatRecordingService - recordCustomTitle', () => { } ).getSessionService = () => ({ getSessionTitleInfo }); - const svc = new ChatRecordingService(mockConfig); + const svc = activateRecording(new ChatRecordingService(mockConfig)); svc.finalize(); await svc.flush(); @@ -518,8 +580,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { // resuming a legacy session on a current build would silently // reclassify it the first time the threshold fires. vi.mocked(mockConfig.getResumedSessionData).mockReturnValue({ - lastCompletedUuid: null, - } as unknown as ReturnType); + ...resumedSessionWithTitle('legacy-title'), + }); const getSessionTitleInfo = vi .fn() .mockReturnValue({ title: 'legacy-title', source: undefined }); @@ -531,7 +593,7 @@ describe('ChatRecordingService - recordCustomTitle', () => { } ).getSessionService = () => ({ getSessionTitleInfo }); - const svc = new ChatRecordingService(mockConfig); + const svc = activateRecording(new ChatRecordingService(mockConfig)); await svc.flush(); expect(jsonl.writeLine).not.toHaveBeenCalled(); diff --git a/packages/core/src/services/chatRecordingService.parentSession.test.ts b/packages/core/src/services/chatRecordingService.parentSession.test.ts index c8f2f4b4572..1e3dc47c8e3 100644 --- a/packages/core/src/services/chatRecordingService.parentSession.test.ts +++ b/packages/core/src/services/chatRecordingService.parentSession.test.ts @@ -15,6 +15,7 @@ import { type ChatRecord, } from './chatRecordingService.js'; import * as jsonl from '../utils/jsonl-utils.js'; +import type { SessionWriterLease } from './session-writer-lease.js'; vi.mock('node:path'); vi.mock('node:child_process'); @@ -31,6 +32,7 @@ vi.mock('../utils/jsonl-utils.js'); describe('ChatRecordingService - recordParentSession', () => { let chatRecordingService: ChatRecordingService; let mockConfig: Config; + let mockLease: SessionWriterLease; let uuidCounter = 0; @@ -78,10 +80,19 @@ describe('ChatRecordingService - recordParentSession', () => { vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined); vi.spyOn(fs, 'existsSync').mockReturnValue(false); - chatRecordingService = new ChatRecordingService(mockConfig); - // writeLine is async; mockResolvedValue lets the writeChain settle on flush. vi.mocked(jsonl.writeLine).mockResolvedValue(undefined); + mockLease = { + sessionId: 'test-session-id', + ownerId: 'test-owner-id', + appendJsonLine: vi.fn((record: unknown) => + jsonl.writeLine('/test/session.jsonl', record), + ), + assertOwnedAndUnchanged: vi.fn().mockResolvedValue(undefined), + release: vi.fn().mockResolvedValue(undefined), + } as unknown as SessionWriterLease; + chatRecordingService = new ChatRecordingService(mockConfig); + chatRecordingService.activate(mockLease); }); afterEach(() => { diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index 97560081357..300f2218727 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -24,6 +24,10 @@ import { serializeSnapshot, type FileHistorySnapshot, } from './fileHistoryService.js'; +import type { + SessionWriterUnavailableError, + SessionWriterLease, +} from './session-writer-lease.js'; vi.mock('node:path'); vi.mock('node:child_process'); @@ -40,6 +44,7 @@ vi.mock('../utils/jsonl-utils.js'); describe('ChatRecordingService', () => { let chatRecordingService: ChatRecordingService; let mockConfig: Config; + let mockLease: SessionWriterLease; let uuidCounter = 0; @@ -87,14 +92,41 @@ describe('ChatRecordingService', () => { vi.spyOn(fs, 'writeFileSync').mockImplementation(() => undefined); vi.spyOn(fs, 'existsSync').mockReturnValue(false); - chatRecordingService = new ChatRecordingService(mockConfig); - // Mock jsonl-utils. writeLine is async — mockResolvedValue returns // a settled Promise so the writeChain in ChatRecordingService advances // when flushed. vi.mocked(jsonl.writeLine).mockResolvedValue(undefined); + + mockLease = { + sessionId: 'test-session-id', + ownerId: 'test-owner-id', + appendJsonLine: vi.fn((record: unknown) => + jsonl.writeLine('/test/session.jsonl', record), + ), + assertOwnedAndUnchanged: vi.fn().mockResolvedValue(undefined), + release: vi.fn().mockResolvedValue(undefined), + } as unknown as SessionWriterLease; + chatRecordingService = activateRecording( + new ChatRecordingService(mockConfig), + ); }); + function activateRecording( + service: ChatRecordingService, + ): ChatRecordingService { + const resumed = mockConfig.getResumedSessionData(); + service.activate( + mockLease, + resumed && !resumed.conversation + ? { + conversation: { messages: [] }, + lastCompletedUuid: resumed.lastCompletedUuid, + } + : resumed, + ); + return service; + } + afterEach(() => { vi.restoreAllMocks(); }); @@ -119,6 +151,48 @@ describe('ChatRecordingService', () => { expect(record.gitBranch).toBe('main'); }); + it('blocks later turns after a generic durable write failure', async () => { + const failure = new Error('disk full'); + vi.mocked(mockLease.appendJsonLine).mockRejectedValueOnce(failure); + + chatRecordingService.recordUserMessage([{ text: 'not durable' }]); + await expect(chatRecordingService.flush()).rejects.toBe(failure); + await expect( + chatRecordingService.assertCanStartTurn(), + ).rejects.toMatchObject({ + name: 'SessionWriterUnavailableError', + cause: failure, + } satisfies Partial); + chatRecordingService.recordUserMessage([{ text: 'must be blocked' }]); + expect(mockLease.appendJsonLine).toHaveBeenCalledTimes(1); + }); + + it('orders new appends after an authoritative read barrier', async () => { + let releaseRead!: () => void; + let markReadStarted!: () => void; + const readStarted = new Promise((resolve) => { + markReadStarted = resolve; + }); + const readGate = new Promise((resolve) => { + releaseRead = resolve; + }); + const snapshot = chatRecordingService.runWithWriteBarrier(async () => { + markReadStarted(); + await readGate; + return 'snapshot'; + }); + await readStarted; + + chatRecordingService.recordUserMessage([{ text: 'after snapshot' }]); + expect(mockLease.appendJsonLine).not.toHaveBeenCalled(); + releaseRead(); + + await expect(snapshot).resolves.toBe('snapshot'); + await chatRecordingService.flush(); + expect(mockLease.appendJsonLine).toHaveBeenCalledOnce(); + expect(mockLease.assertOwnedAndUnchanged).toHaveBeenCalledTimes(2); + }); + it('should chain messages correctly with parentUuid', async () => { chatRecordingService.recordUserMessage([{ text: 'First message' }]); chatRecordingService.recordAssistantTurn({ @@ -174,7 +248,9 @@ describe('ChatRecordingService', () => { vi.mocked(mockConfig.getResumedSessionData).mockReturnValue({ lastCompletedUuid: 'assistant-1', } as unknown as ReturnType); - chatRecordingService = new ChatRecordingService(mockConfig); + chatRecordingService = activateRecording( + new ChatRecordingService(mockConfig), + ); chatRecordingService.rebuildTurnBoundaries([ { @@ -210,6 +286,23 @@ describe('ChatRecordingService', () => { }); }); + describe('recordUserTextElements', () => { + it('records user text elements as a strict system payload', async () => { + const payload = { + content: 'hello', + textElements: [{ text: 'hello', start: 0, end: 5 }], + }; + + await chatRecordingService.recordUserTextElements(payload); + + expect(jsonl.writeLine).toHaveBeenCalledTimes(1); + const record = vi.mocked(jsonl.writeLine).mock.calls[0][1] as ChatRecord; + expect(record.type).toBe('system'); + expect(record.subtype).toBe('user_text_elements'); + expect(record.systemPayload).toEqual(payload); + }); + }); + describe('recordAtCommand', () => { it('should record @-command metadata as a system payload', async () => { const userParts: Part[] = [{ text: 'Hello, world!' }]; @@ -956,7 +1049,9 @@ describe('ChatRecordingService', () => { chatRecordingService.recordUserMessage([{ text: 'first' }]); await expect(chatRecordingService.flush()).rejects.toThrow('disk full'); - const nextRecordingService = new ChatRecordingService(mockConfig); + const nextRecordingService = activateRecording( + new ChatRecordingService(mockConfig), + ); nextRecordingService.recordUserMessage([{ text: 'new session' }]); await expect(nextRecordingService.flush()).resolves.toBeUndefined(); @@ -985,7 +1080,9 @@ describe('ChatRecordingService', () => { }), ); const listener = vi.fn(); - const service = new ChatRecordingService(mockConfig, listener); + const service = activateRecording( + new ChatRecordingService(mockConfig, listener), + ); service.recordUserMessage([{ text: 'first' }]); service.recordUserMessage([{ text: 'queued descendant' }]); @@ -1011,11 +1108,15 @@ describe('ChatRecordingService', () => { .mockRejectedValueOnce(new Error('first failure')) .mockRejectedValueOnce(new Error('second failure')); - const first = new ChatRecordingService(mockConfig, firstListener); + const first = activateRecording( + new ChatRecordingService(mockConfig, firstListener), + ); first.recordUserMessage([{ text: 'first' }]); await expect(first.flush()).rejects.toThrow('first failure'); - const second = new ChatRecordingService(mockConfig, secondListener); + const second = activateRecording( + new ChatRecordingService(mockConfig, secondListener), + ); second.recordUserMessage([{ text: 'second' }]); await expect(second.flush()).rejects.toThrow('second failure'); @@ -1028,9 +1129,11 @@ describe('ChatRecordingService', () => { const handler = (error: unknown) => unhandled.push(error); process.on('unhandledRejection', handler); try { - const syncFailure = new ChatRecordingService(mockConfig, () => { - throw new Error('listener threw'); - }); + const syncFailure = activateRecording( + new ChatRecordingService(mockConfig, () => { + throw new Error('listener threw'); + }), + ); vi.mocked(jsonl.writeLine).mockRejectedValueOnce( new Error('sync observer write failure'), ); @@ -1039,9 +1142,11 @@ describe('ChatRecordingService', () => { 'sync observer write failure', ); - const asyncFailure = new ChatRecordingService(mockConfig, async () => { - throw new Error('listener rejected'); - }); + const asyncFailure = activateRecording( + new ChatRecordingService(mockConfig, async () => { + throw new Error('listener rejected'); + }), + ); vi.mocked(jsonl.writeLine).mockRejectedValueOnce( new Error('async observer write failure'), ); @@ -1057,12 +1162,8 @@ describe('ChatRecordingService', () => { }); }); - describe('ensureChatsDir caching', () => { - it('does not cache when mkdirSync throws so the next write retries', async () => { - // Regression: a transient mkdir failure used to poison the cache and - // silently drop the rest of the session's records. We have to fail - // both mkdir AND the wx-create, otherwise ensureConversationFile's - // own cache short-circuits ensureChatsDir on the second call. + describe('legacy recorder', () => { + it('retries directory setup after a synchronous failure', async () => { const mkdirSpy = vi.spyOn(fs, 'mkdirSync'); mkdirSpy.mockImplementationOnce(() => { throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); @@ -1075,12 +1176,13 @@ describe('ChatRecordingService', () => { }); writeSpy.mockImplementation(() => undefined); - chatRecordingService.recordUserMessage([{ text: 'retry me' }]); + const service = new ChatRecordingService(mockConfig, undefined, false); + service.recordUserMessage([{ text: 'retry me' }]); + await expect(service.flush()).resolves.toBeUndefined(); expect(jsonl.writeLine).not.toHaveBeenCalled(); - await expect(chatRecordingService.flush()).resolves.toBeUndefined(); - chatRecordingService.recordUserMessage([{ text: 'retry me' }]); - await expect(chatRecordingService.flush()).resolves.toBeUndefined(); + service.recordUserMessage([{ text: 'retry me' }]); + await expect(service.flush()).resolves.toBeUndefined(); expect(mkdirSpy.mock.calls.length).toBeGreaterThanOrEqual(2); expect(jsonl.writeLine).toHaveBeenCalledTimes(1); @@ -1090,7 +1192,7 @@ describe('ChatRecordingService', () => { it('does not notify for a synchronous conversation-file failure', () => { const listener = vi.fn(); - const service = new ChatRecordingService(mockConfig, listener); + const service = new ChatRecordingService(mockConfig, listener, false); vi.spyOn(fs, 'writeFileSync').mockImplementationOnce(() => { throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); }); @@ -1101,20 +1203,45 @@ describe('ChatRecordingService', () => { expect(jsonl.writeLine).not.toHaveBeenCalled(); }); - it('caches after a successful mkdir so steady-state writes skip the syscall', async () => { + it('caches successful directory setup', async () => { const mkdirSpy = vi .spyOn(fs, 'mkdirSync') .mockImplementation(() => undefined); + const service = new ChatRecordingService(mockConfig, undefined, false); - chatRecordingService.recordUserMessage([{ text: 'first' }]); - await chatRecordingService.flush(); - chatRecordingService.recordUserMessage([{ text: 'second' }]); - await chatRecordingService.flush(); - chatRecordingService.recordUserMessage([{ text: 'third' }]); - await chatRecordingService.flush(); + service.recordUserMessage([{ text: 'first' }]); + await service.flush(); + service.recordUserMessage([{ text: 'second' }]); + await service.flush(); + service.recordUserMessage([{ text: 'third' }]); + await service.flush(); expect(mkdirSpy).toHaveBeenCalledTimes(1); }); + + it('retries an identical attribution snapshot after a synchronous failure', async () => { + const snapshot = { + type: 'attribution-snapshot' as const, + version: 1, + surface: 'cli', + fileStates: {}, + promptCount: 0, + promptCountAtLastCommit: 0, + }; + const writeFileSpy = vi.spyOn(fs, 'writeFileSync'); + writeFileSpy.mockImplementationOnce(() => { + throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); + }); + const service = new ChatRecordingService(mockConfig, undefined, false); + + service.recordAttributionSnapshot(snapshot); + await service.flush(); + expect(jsonl.writeLine).not.toHaveBeenCalled(); + + service.recordAttributionSnapshot(snapshot); + await service.flush(); + expect(jsonl.writeLine).toHaveBeenCalledTimes(1); + }); }); describe('recordAttributionSnapshot', () => { @@ -1340,37 +1467,6 @@ describe('ChatRecordingService', () => { expect(after.parentUuid).toBe(before.uuid); expect(after.parentUuid).not.toBe(artifact.uuid); }); - - // appendRecord can throw SYNCHRONOUSLY before returning a promise - // (e.g. ensureConversationFile fails because the conversation - // file can't be created). Without rollback in the outer catch, - // the dedup key stays set on a write that never happened, so - // all future identical snapshots get suppressed. - it('should retry an identical snapshot after a synchronous failure', async () => { - // First call: force writeFileSync (used by ensureConversationFile - // to wx-create the JSONL file) to throw a non-EEXIST error. - // ensureConversationFile rethrows that, which propagates through - // appendRecord SYNCHRONOUSLY before any promise is returned. - const writeFileSpy = vi.spyOn(fs, 'writeFileSync'); - writeFileSpy.mockImplementationOnce(() => { - const e = new Error( - 'EACCES: permission denied', - ) as NodeJS.ErrnoException; - e.code = 'EACCES'; - throw e; - }); - - chatRecordingService.recordAttributionSnapshot(baseSnapshot); - await chatRecordingService.flush(); - // Sync failure: writeLine never reached. - expect(vi.mocked(jsonl.writeLine)).not.toHaveBeenCalled(); - - // Identical snapshot on retry: dedup key should have been - // rolled back so this fires a fresh write. - chatRecordingService.recordAttributionSnapshot(baseSnapshot); - await chatRecordingService.flush(); - expect(vi.mocked(jsonl.writeLine)).toHaveBeenCalledTimes(1); - }); }); // Note: Session management tests (listSessions, loadSession, deleteSession, etc.) diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 5fac1050e2b..ad414076fe5 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -38,6 +38,12 @@ import type { SessionArtifactEventRecordPayload, SessionArtifactSnapshotRecordPayload, } from './session-artifact-persistence.js'; +import { + SessionTranscriptChangedError, + SessionWriterLostError, + SessionWriterUnavailableError, + type SessionWriterLease, +} from './session-writer-lease.js'; const debugLogger = createDebugLogger('CHAT_RECORDING'); @@ -258,6 +264,7 @@ export interface ChatRecord { | 'agent_bootstrap' | 'agent_launch_prompt' | 'file_history_snapshot' + | 'user_text_elements' | 'session_artifact_event' | 'session_artifact_snapshot'; /** Working directory at time of message */ @@ -309,6 +316,7 @@ export interface ChatRecord { | RewindRecordPayload | AgentBootstrapRecordPayload | FileHistorySnapshotRecordPayload + | UserTextElementsRecordPayload | SessionArtifactEventRecordPayload | SessionArtifactSnapshotRecordPayload; @@ -482,6 +490,11 @@ export interface FileHistorySnapshotRecordPayload { snapshots: SerializedFileHistorySnapshot[]; } +export interface UserTextElementsRecordPayload { + content: string; + textElements: unknown[]; +} + export interface ChatRecordingFailureEvent { sessionId: string; error: Error; @@ -531,22 +544,26 @@ export class ChatRecordingService { * record). */ private turnParentUuids: Array = []; - /** - * Cached chats-dir / conversation-file path so per-record appendRecord - * doesn't re-stat them on every write. The first call performs the - * mkdir / wx-create; subsequent calls short-circuit. - */ private chatsDirEnsured = false; private cachedConversationFile: string | undefined; - /** - * Serialized async write queue for appendRecord. A rejected write leaves the - * canonical chain rejected so later queued records cannot be persisted with - * a parentUuid that never reached disk. Must be flushed before process exit - * (see {@link flush}). - */ - private writeChain: Promise = Promise.resolve(); + private state: + | 'inactive' + | 'active' + | 'closing' + | 'closed' + | 'integrity_failed' = 'inactive'; + private binding: + | { + readonly sessionId: string; + readonly lease: SessionWriterLease; + } + | undefined; + /** Serializes appends and authoritative read barriers. Always settles. */ + private operationTail: Promise = Promise.resolve(); /** First async JSONL write failure; permanently degrades this recorder. */ private writeFailure: Error | undefined; + private integrityFailure: Error | undefined; + private readonly writerLeaseRequired: boolean; /** In-memory cache of the current session's custom title (for re-append on exit) */ private currentCustomTitle: string | undefined; /** @@ -620,34 +637,36 @@ export class ChatRecordingService { constructor( config: Config, private readonly onWriteFailure?: ChatRecordingFailureListener, + writerLeaseRequired = config.getExperimentalZedIntegration?.() ?? true, ) { this.config = config; - this.lastRecordUuid = - config.getResumedSessionData()?.lastCompletedUuid ?? null; - - // On resume, load the cached custom title AND its source from the - // session file. Preserving the persisted source is load-bearing: the - // SessionPicker dim-styling depends on it, and hardcoding `'manual'` - // would silently downgrade auto-titled sessions every time they get - // resumed. Legacy records (no `titleSource` field) stay `undefined` — - // treated as manual for safety without rewriting the JSONL. - // - // Do not re-append during construction: loading/resuming a session is a - // read operation from the user's perspective, and touching the JSONL mtime - // would make session lists treat it as fresh activity. - if (config.getResumedSessionData()) { - try { - const sessionService = config.getSessionService(); - const info = sessionService.getSessionTitleInfo(config.getSessionId()); - this.currentCustomTitle = info.title; - this.currentTitleSource = info.source; - if (info.title) { - // Prime the threshold so the first real content write re-anchors. - this.bytesSinceTitleAnchor = TITLE_REANCHOR_BYTES; - } - } catch { - // Best-effort — don't block construction - } + this.writerLeaseRequired = writerLeaseRequired; + const resumed = config.getResumedSessionData(); + if (writerLeaseRequired) { + this.lastRecordUuid = resumed?.lastCompletedUuid ?? null; + } else { + this.state = 'active'; + this.restoreSessionState( + resumed + ? { + conversation: resumed.conversation ?? { messages: [] }, + lastCompletedUuid: resumed.lastCompletedUuid, + } + : undefined, + resumed ? this.readPersistedTitleInfo() : undefined, + ); + } + } + + private readPersistedTitleInfo(): + | { title?: string; source?: TitleSource } + | undefined { + try { + return this.config + .getSessionService() + .getSessionTitleInfo(this.config.getSessionId()); + } catch { + return undefined; } } @@ -673,66 +692,105 @@ export class ChatRecordingService { * @returns The session ID. */ private getSessionId(): string { - return this.config.getSessionId(); + return this.binding?.sessionId ?? this.config.getSessionId(); } - /** - * Ensures the chats directory exists, creating it if it doesn't exist. - * @returns The path to the chats directory. - * @throws Error if the directory cannot be created. - */ private ensureChatsDir(): string { - const projectDir = this.config.storage.getProjectDir(); - const chatsDir = path.join(projectDir, 'chats'); - - if (this.chatsDirEnsured) { - return chatsDir; - } + const chatsDir = path.join(this.config.storage.getProjectDir(), 'chats'); + if (this.chatsDirEnsured) return chatsDir; try { fs.mkdirSync(chatsDir, { recursive: true }); - // Only cache success — keep transient mkdir failures self-healing. this.chatsDirEnsured = true; } catch { - // ignored + // The file creation below reports the actionable error. } return chatsDir; } - /** - * Ensures the conversation file exists, creating it if it doesn't exist. - * Uses atomic file creation to avoid race conditions. Result is cached so - * subsequent appendRecord calls skip the wx-create entirely. - * @returns The path to the conversation file. - * @throws Error if the file cannot be created or accessed. - */ private ensureConversationFile(): string { - if (this.cachedConversationFile) { - return this.cachedConversationFile; - } - const chatsDir = this.ensureChatsDir(); - const sessionId = this.getSessionId(); - const safeFilename = `${sessionId}.jsonl`; - const conversationFile = path.join(chatsDir, safeFilename); - + if (this.cachedConversationFile) return this.cachedConversationFile; + const conversationFile = path.join( + this.ensureChatsDir(), + `${this.getSessionId()}.jsonl`, + ); try { - // Use 'wx' flag for exclusive creation - atomic operation that fails if - // the file already exists. EEXIST is the expected steady-state path on - // resume; we treat it as success. fs.writeFileSync(conversationFile, '', { flag: 'wx', encoding: 'utf8' }); } catch (error) { - const nodeError = error as NodeJS.ErrnoException; - if (nodeError.code !== 'EEXIST') { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') { const message = error instanceof Error ? error.message : String(error); throw new Error( `Failed to create conversation file at ${conversationFile}: ${message}`, ); } } - this.cachedConversationFile = conversationFile; return conversationFile; } + private restoreSessionState( + sessionData?: { + conversation: { messages: ChatRecord[] }; + lastCompletedUuid: string | null; + }, + persistedTitleInfo?: { title?: string; source?: TitleSource }, + ): void { + this.lastRecordUuid = sessionData?.lastCompletedUuid ?? null; + this.currentCustomTitle = undefined; + this.currentTitleSource = undefined; + this.currentParentSessionId = undefined; + this.currentSourceType = undefined; + this.currentSourceId = undefined; + if (!sessionData) return; + this.rebuildTurnBoundaries(sessionData.conversation.messages); + for (const record of sessionData.conversation.messages) { + if (record.type !== 'system') continue; + if (record.subtype === 'custom_title') { + const payload = record.systemPayload as + | CustomTitleRecordPayload + | undefined; + this.currentCustomTitle = payload?.customTitle; + this.currentTitleSource = payload?.titleSource; + } else if (record.subtype === 'parent_session') { + this.currentParentSessionId = ( + record.systemPayload as ParentSessionRecordPayload | undefined + )?.parentSessionId; + } else if (record.subtype === 'session_source') { + const payload = record.systemPayload as + | SessionSourceRecordPayload + | undefined; + this.currentSourceType = payload?.sourceType; + this.currentSourceId = payload?.sourceId; + } + } + if (persistedTitleInfo !== undefined) { + this.currentCustomTitle = persistedTitleInfo.title; + this.currentTitleSource = persistedTitleInfo.source; + } + if (this.currentCustomTitle) { + this.bytesSinceTitleAnchor = TITLE_REANCHOR_BYTES; + } + } + + activate( + lease: SessionWriterLease, + sessionData?: { + conversation: { messages: ChatRecord[] }; + lastCompletedUuid: string | null; + }, + persistedTitleInfo?: { title?: string; source?: TitleSource }, + ): void { + if ( + !this.writerLeaseRequired || + this.state !== 'inactive' || + lease.sessionId !== this.config.getSessionId() + ) { + throw new SessionWriterUnavailableError(); + } + this.binding = { sessionId: lease.sessionId, lease }; + this.restoreSessionState(sessionData, persistedTitleInfo); + this.state = 'active'; + } + /** * Creates base fields for a ChatRecord. */ @@ -759,11 +817,27 @@ export class ChatRecordingService { return this.cachedGitBranch.branch; } - private enterWriteFailure(cause: unknown, sessionId: string): Error { + private enterWriteFailure( + cause: unknown, + sessionId: string, + operation = 'append', + ): Error { + const failure = cause instanceof Error ? cause : new Error(String(cause)); + if ( + !this.integrityFailure && + (failure instanceof SessionWriterLostError || + failure instanceof SessionTranscriptChangedError || + failure instanceof SessionWriterUnavailableError) + ) { + this.integrityFailure = failure; + this.state = 'integrity_failed'; + debugLogger.error( + `Session writer failure sessionId=${sessionId} operation=${operation} errorKind=${failure.errorKind}`, + ); + } if (!this.writeFailure) { - this.writeFailure = - cause instanceof Error ? cause : new Error(String(cause)); - debugLogger.error('Error appending record (async):', this.writeFailure); + this.writeFailure = failure; + debugLogger.error('Chat recording failure:', this.writeFailure); try { const notification = this.onWriteFailure?.({ sessionId, @@ -781,29 +855,37 @@ export class ChatRecordingService { debugLogger.debug('Chat recording failure listener threw:', error); } } - return this.writeFailure; + return this.integrityFailure ?? this.writeFailure; } private enqueueRecordWrite( - conversationFile: string, record: ChatRecord, + legacyConversationFile?: string, ): Promise { - const pendingWrite = this.writeChain.then(async () => { + const pendingWrite = this.operationTail.then(async () => { + if (this.writeFailure) throw this.writeFailure; try { - await jsonl.writeLine(conversationFile, record); + const lease = this.binding?.lease; + if (lease) { + await lease.appendJsonLine(record); + } else if (!this.writerLeaseRequired && legacyConversationFile) { + await jsonl.writeLine(legacyConversationFile, record); + } else { + throw new SessionWriterUnavailableError(); + } } catch (error) { throw this.enterWriteFailure(error, record.sessionId); } }); - this.writeChain = pendingWrite; - // Mark fire-and-forget writes as handled without replacing the canonical - // rejected chain that flush() and strict callers must continue to observe. - void pendingWrite.catch(() => {}); + this.operationTail = pendingWrite.then( + () => undefined, + () => undefined, + ); return pendingWrite; } /** - * Fire-and-forget: queues a JSONL write on the internal writeChain. + * Fire-and-forget: queues a JSONL write on the internal operation tail. * A failed write permanently degrades this recorder; already-queued * descendants are skipped and later fire-and-forget calls become no-ops. */ @@ -811,19 +893,14 @@ export class ChatRecordingService { record: ChatRecord, options?: { updateActiveTail?: boolean }, ): void { - if (this.writeFailure) return; - - let conversationFile: string; - try { - conversationFile = this.ensureConversationFile(); - } catch (error) { - debugLogger.error('Error appending record:', error); - throw error; - } + if (this.writeFailure || this.state !== 'active') return; + const legacyConversationFile = this.writerLeaseRequired + ? undefined + : this.ensureConversationFile(); if (options?.updateActiveTail !== false) { this.lastRecordUuid = record.uuid; } - this.enqueueRecordWrite(conversationFile, record); + this.enqueueRecordWrite(record, legacyConversationFile); this.updateTitleAnchorTracking(record); } @@ -832,21 +909,20 @@ export class ChatRecordingService { options?: { updateActiveTail?: boolean }, ): Promise { if (this.writeFailure) throw this.writeFailure; + if (this.state !== 'active') throw new SessionWriterUnavailableError(); const previousLastRecordUuid = this.lastRecordUuid; const updateActiveTail = options?.updateActiveTail !== false; - let conversationFile: string; - try { - conversationFile = this.ensureConversationFile(); - } catch (error) { - debugLogger.error('Error appending record:', error); - throw error; - } - + const legacyConversationFile = this.writerLeaseRequired + ? undefined + : this.ensureConversationFile(); if (updateActiveTail) { this.lastRecordUuid = record.uuid; } - const pendingWrite = this.enqueueRecordWrite(conversationFile, record); + const pendingWrite = this.enqueueRecordWrite( + record, + legacyConversationFile, + ); // Keep anchor accounting in logical queue order, matching appendRecord. // Once accepted, a failed write permanently stops this recorder, so no // rollback of this bookkeeping is needed on rejection. @@ -952,7 +1028,88 @@ export class ChatRecordingService { * teardown to ensure no records are dropped. */ async flush(): Promise { - await this.writeChain; + await this.operationTail; + if (this.writeFailure) throw this.writeFailure; + } + + async runWithWriteBarrier(operation: () => Promise): Promise { + if (this.writeFailure) throw this.writeFailure; + if (this.state !== 'active') throw new SessionWriterUnavailableError(); + const pending = this.operationTail.then(async () => { + if (this.writeFailure) throw this.writeFailure; + if (this.state !== 'active') throw new SessionWriterUnavailableError(); + const lease = this.binding?.lease; + try { + if (lease) { + await lease.assertOwnedAndUnchanged(); + } else if (this.writerLeaseRequired) { + throw new SessionWriterUnavailableError(); + } + const result = await operation(); + await lease?.assertOwnedAndUnchanged(); + return result; + } catch (error) { + if ( + error instanceof SessionWriterLostError || + error instanceof SessionTranscriptChangedError || + error instanceof SessionWriterUnavailableError + ) { + throw this.enterWriteFailure( + error, + this.getSessionId(), + 'read_barrier', + ); + } + throw error; + } + }); + this.operationTail = pending.then( + () => undefined, + () => undefined, + ); + return pending; + } + + async assertCanStartTurn(): Promise { + try { + await this.runWithWriteBarrier(async () => undefined); + } catch (error) { + if (this.integrityFailure) throw this.integrityFailure; + if (this.writeFailure) { + throw new SessionWriterUnavailableError({ cause: this.writeFailure }); + } + throw error; + } + } + + async close(): Promise { + if (this.state === 'closed') return; + this.autoTitleController?.abort(); + if (this.state === 'active') this.state = 'closing'; + let flushFailure: unknown; + try { + await this.flush(); + } catch (error) { + flushFailure = error; + } + try { + await this.binding?.lease.release(); + this.binding = undefined; + this.state = 'closed'; + } catch (error) { + if (error instanceof SessionWriterLostError) { + this.binding = undefined; + this.state = 'closed'; + } else { + this.state = 'integrity_failed'; + } + throw error; + } + if (flushFailure !== undefined) throw flushFailure; + } + + hasWriteOwnership(): boolean { + return this.binding !== undefined; } /** @@ -961,6 +1118,9 @@ export class ChatRecordingService { * resolve the JSONL path through the updated Config.storage. */ resetStoragePaths(): void { + if (this.writerLeaseRequired && this.state === 'active') { + throw new SessionWriterUnavailableError(); + } this.chatsDirEnsured = false; this.cachedConversationFile = undefined; } @@ -1677,6 +1837,18 @@ export class ChatRecordingService { } } + async recordUserTextElements( + payload: UserTextElementsRecordPayload, + ): Promise { + const record: ChatRecord = { + ...this.createBaseRecord('system'), + type: 'system', + subtype: 'user_text_elements', + systemPayload: payload, + }; + await this.appendRecordStrict(record); + } + private appendSerializedFileHistorySnapshotBatch( snapshots: SerializedFileHistorySnapshot[], ): void { diff --git a/packages/core/src/services/session-writer-lease.test-helper.ts b/packages/core/src/services/session-writer-lease.test-helper.ts new file mode 100644 index 00000000000..85bf8c3c0d2 --- /dev/null +++ b/packages/core/src/services/session-writer-lease.test-helper.ts @@ -0,0 +1,65 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + SessionWriterError, + SessionWriterLease, + type AcquireSessionWriterLeaseOptions, +} from './session-writer-lease.js'; + +export type SessionWriterLeaseTestCommandInput = + | { + type: 'acquire'; + options: AcquireSessionWriterLeaseOptions; + } + | { type: 'append'; value: unknown } + | { type: 'release' }; + +export type SessionWriterLeaseTestCommand = + SessionWriterLeaseTestCommandInput & { id: number }; + +export interface SessionWriterLeaseTestResponse { + id: number; + ok: boolean; + ownerId?: string; + errorKind?: string; + message?: string; +} + +let lease: SessionWriterLease | undefined; + +async function handleCommand( + command: SessionWriterLeaseTestCommand, +): Promise { + try { + if (command.type === 'acquire') { + lease = await SessionWriterLease.acquire(command.options); + process.send?.({ id: command.id, ok: true, ownerId: lease.ownerId }); + return; + } + if (!lease) throw new Error('Lease has not been acquired'); + if (command.type === 'append') { + await lease.appendJsonLine(command.value); + } else { + await lease.release(); + lease = undefined; + } + process.send?.({ id: command.id, ok: true }); + } catch (error) { + process.send?.({ + id: command.id, + ok: false, + ...(error instanceof SessionWriterError + ? { errorKind: error.errorKind } + : {}), + message: error instanceof Error ? error.message : String(error), + }); + } +} + +process.on('message', (message: unknown) => { + void handleCommand(message as SessionWriterLeaseTestCommand); +}); diff --git a/packages/core/src/services/session-writer-lease.test.ts b/packages/core/src/services/session-writer-lease.test.ts new file mode 100644 index 00000000000..bbba98a008f --- /dev/null +++ b/packages/core/src/services/session-writer-lease.test.ts @@ -0,0 +1,911 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { fork, type ChildProcess } from 'node:child_process'; +import { chmodSync, unlinkSync } from 'node:fs'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { Config } from '../config/config.js'; +import { Storage } from '../config/storage.js'; +import { + resetDebugLoggingState, + setDebugLogSession, +} from '../utils/debugLogger.js'; +import { + ChatRecordingService, + type ChatRecord, +} from './chatRecordingService.js'; +import { SessionService } from './sessionService.js'; +import { + getSessionWriterLockPath, + SessionTranscriptChangedError, + SessionWriterConflictError, + SessionWriterLease, + SessionWriterLostError, + SessionWriterUnavailableError, + type AcquireSessionWriterLeaseOptions, +} from './session-writer-lease.js'; +import type { + SessionWriterLeaseTestCommandInput, + SessionWriterLeaseTestResponse, +} from './session-writer-lease.test-helper.js'; + +const helperPath = fileURLToPath( + new URL('./session-writer-lease.test-helper.ts', import.meta.url), +); + +let nextRequestId = 0; +const children = new Set(); +const temporaryDirectories = new Set(); + +async function createFixture(sessionId = 'test-session'): Promise<{ + runtimeBaseDir: string; + projectRoot: string; + transcriptPath: string; + options: AcquireSessionWriterLeaseOptions; +}> { + const root = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-writer-lease-')); + temporaryDirectories.add(root); + const runtimeBaseDir = path.join(root, 'runtime'); + const projectRoot = path.join(root, 'project'); + await fs.mkdir(projectRoot, { recursive: true }); + const storage = new Storage(projectRoot, runtimeBaseDir); + const transcriptPath = path.join( + storage.getProjectDir(), + 'chats', + `${sessionId}.jsonl`, + ); + return { + runtimeBaseDir, + projectRoot, + transcriptPath, + options: { runtimeBaseDir, sessionId, transcriptPath }, + }; +} + +function startLeaseProcess(env?: NodeJS.ProcessEnv): ChildProcess { + const child = fork(helperPath, [], { + execArgv: ['--import', 'tsx'], + stdio: ['ignore', 'ignore', 'pipe', 'ipc'], + ...(env ? { env: { ...process.env, ...env } } : {}), + }); + children.add(child); + child.once('close', () => children.delete(child)); + return child; +} + +async function requestChild( + child: ChildProcess, + command: SessionWriterLeaseTestCommandInput, +): Promise { + const id = ++nextRequestId; + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error(`Timed out waiting for lease helper command ${id}`)); + }, 10_000); + const onMessage = (message: SessionWriterLeaseTestResponse) => { + if (message.id !== id) return; + clearTimeout(timeout); + child.off('message', onMessage); + resolve(message); + }; + child.on('message', onMessage); + child.send({ ...command, id }, (error) => { + if (!error) return; + clearTimeout(timeout); + child.off('message', onMessage); + reject(error); + }); + }); +} + +async function waitForClose(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) return; + await new Promise((resolve) => child.once('close', () => resolve())); +} + +function record( + uuid: string, + parentUuid: string | null, + sessionId: string, + cwd: string, + type: 'user' | 'assistant', + text: string, +): ChatRecord { + return { + uuid, + parentUuid, + sessionId, + timestamp: '2026-01-01T00:00:00.000Z', + type, + cwd, + version: 'test', + message: { + role: type === 'user' ? 'user' : 'model', + parts: [{ text }], + }, + }; +} + +afterEach(async () => { + setDebugLogSession(null); + resetDebugLoggingState(); + Storage.setRuntimeBaseDir(null); + for (const child of children) child.kill('SIGKILL'); + await Promise.all([...children].map((child) => waitForClose(child))); + await Promise.all( + [...temporaryDirectories].map((directory) => + fs.rm(directory, { recursive: true, force: true }), + ), + ); + children.clear(); + temporaryDirectories.clear(); +}); + +describe('SessionWriterLease', () => { + it('activates a real ACP Config from the authoritative physical tail', async () => { + const fixture = await createFixture('config-authoritative-session'); + const firstUser = record( + 'user-1', + null, + fixture.options.sessionId, + fixture.projectRoot, + 'user', + 'start', + ); + const previewTail = record( + 'tool-tail', + firstUser.uuid, + fixture.options.sessionId, + fixture.projectRoot, + 'assistant', + 'tool result', + ); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile( + fixture.transcriptPath, + `${JSON.stringify(firstUser)}\n${JSON.stringify(previewTail)}\n`, + 'utf8', + ); + const sessionService = new SessionService(fixture.projectRoot, { + runtimeBaseDir: fixture.runtimeBaseDir, + }); + const stalePreview = await sessionService.loadSession( + fixture.options.sessionId, + ); + expect(stalePreview?.lastCompletedUuid).toBe(previewTail.uuid); + + const physicalFinal = record( + 'physical-final', + previewTail.uuid, + fixture.options.sessionId, + fixture.projectRoot, + 'assistant', + 'final answer', + ); + await fs.writeFile( + fixture.transcriptPath, + `${JSON.stringify(firstUser)}\n${JSON.stringify(previewTail)}\n${JSON.stringify(physicalFinal)}\n`, + 'utf8', + ); + const config = Storage.runWithRuntimeBaseDir( + fixture.runtimeBaseDir, + fixture.projectRoot, + () => + new Config({ + sessionId: fixture.options.sessionId, + sessionData: stalePreview, + cwd: fixture.projectRoot, + targetDir: fixture.projectRoot, + debugMode: false, + model: 'test-model', + chatRecording: true, + experimentalZedIntegration: true, + bareMode: true, + telemetry: { enabled: false }, + usageStatisticsEnabled: false, + }), + ); + + await config.initialize({ + skipGeminiInitialization: true, + skipHooks: true, + skipMcpDiscovery: true, + skipSkillManager: true, + skipFileCheckpointing: true, + lenientToolWarmup: true, + }); + expect(config.getResumedSessionData()?.lastCompletedUuid).toBe( + physicalFinal.uuid, + ); + const recorder = config.getChatRecordingService(); + expect(recorder).toBeDefined(); + recorder?.recordUserMessage('next'); + await recorder?.flush(); + + const written = (await fs.readFile(fixture.transcriptPath, 'utf8')) + .trim() + .split('\n') + .map((line) => JSON.parse(line) as ChatRecord); + expect(written.at(-1)).toMatchObject({ + type: 'user', + parentUuid: physicalFinal.uuid, + message: { parts: [{ text: 'next' }] }, + }); + + await config.shutdown({ shutdownTelemetry: false }); + expect(config.hasSessionWriteOwnership()).toBe(false); + await expect( + fs.lstat( + getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ), + ), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('restores and re-anchors a persisted title outside the active UUID chain', async () => { + const fixture = await createFixture('11111111-1111-4111-8111-111111111111'); + const firstUser = record( + 'user-1', + null, + fixture.options.sessionId, + fixture.projectRoot, + 'user', + 'start', + ); + const titleRecord: ChatRecord = { + uuid: 'title-1', + parentUuid: firstUser.uuid, + sessionId: fixture.options.sessionId, + timestamp: '2026-01-01T00:00:01.000Z', + type: 'system', + subtype: 'custom_title', + cwd: fixture.projectRoot, + version: 'test', + systemPayload: { + customTitle: 'operator-title', + titleSource: 'manual', + }, + }; + const rewindRecord: ChatRecord = { + uuid: 'rewind-1', + parentUuid: firstUser.uuid, + sessionId: fixture.options.sessionId, + timestamp: '2026-01-01T00:00:02.000Z', + type: 'system', + subtype: 'rewind', + cwd: fixture.projectRoot, + version: 'test', + systemPayload: { truncatedCount: 1 }, + }; + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile( + fixture.transcriptPath, + `${JSON.stringify(firstUser)}\n${JSON.stringify(titleRecord)}\n${JSON.stringify(rewindRecord)}\n`, + 'utf8', + ); + const sessionService = new SessionService(fixture.projectRoot, { + runtimeBaseDir: fixture.runtimeBaseDir, + }); + const preview = await sessionService.loadSession(fixture.options.sessionId); + expect( + preview?.conversation.messages.some( + (message) => message.subtype === 'custom_title', + ), + ).toBe(false); + expect( + sessionService.getSessionTitleInfo(fixture.options.sessionId), + ).toEqual({ title: 'operator-title', source: 'manual' }); + + const config = Storage.runWithRuntimeBaseDir( + fixture.runtimeBaseDir, + fixture.projectRoot, + () => + new Config({ + sessionId: fixture.options.sessionId, + sessionData: preview, + cwd: fixture.projectRoot, + targetDir: fixture.projectRoot, + debugMode: false, + model: 'test-model', + chatRecording: true, + experimentalZedIntegration: true, + bareMode: true, + telemetry: { enabled: false }, + usageStatisticsEnabled: false, + }), + ); + + await config.initialize({ + skipGeminiInitialization: true, + skipHooks: true, + skipMcpDiscovery: true, + skipSkillManager: true, + skipFileCheckpointing: true, + lenientToolWarmup: true, + }); + const recorder = config.getChatRecordingService(); + expect(recorder?.getCurrentCustomTitle()).toBe('operator-title'); + recorder?.recordUserMessage('after rewind'); + await recorder?.flush(); + + const physicalRecords = (await fs.readFile(fixture.transcriptPath, 'utf8')) + .trim() + .split('\n') + .map((line) => JSON.parse(line) as ChatRecord); + expect(physicalRecords.at(-1)).toMatchObject({ + type: 'system', + subtype: 'custom_title', + systemPayload: { + customTitle: 'operator-title', + titleSource: 'manual', + }, + }); + + await config.shutdown({ shutdownTelemetry: false }); + }); + + it('preserves transcript-changed during Config activation cleanup', async () => { + const fixture = await createFixture('config-truncated-session'); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile(fixture.transcriptPath, '{"truncated":true}', 'utf8'); + const config = Storage.runWithRuntimeBaseDir( + fixture.runtimeBaseDir, + fixture.projectRoot, + () => + new Config({ + sessionId: fixture.options.sessionId, + cwd: fixture.projectRoot, + targetDir: fixture.projectRoot, + debugMode: false, + model: 'test-model', + chatRecording: true, + experimentalZedIntegration: true, + bareMode: true, + telemetry: { enabled: false }, + usageStatisticsEnabled: false, + }), + ); + + await expect(config.initialize()).rejects.toBeInstanceOf( + SessionTranscriptChangedError, + ); + expect(config.hasSessionWriteOwnership()).toBe(false); + await expect( + fs.lstat( + getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ), + ), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it.runIf(process.platform !== 'win32')( + 'exposes the owned lease when transcript inspection cleanup must be retried', + async () => { + const fixture = await createFixture(); + await fs.mkdir(fixture.transcriptPath, { recursive: true }); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const lockDir = path.dirname(lockPath); + let recoveryLease: SessionWriterLease | undefined; + + try { + await expect( + SessionWriterLease.acquire({ + ...fixture.options, + onOwnershipAcquired: (lease) => { + recoveryLease = lease; + chmodSync(lockDir, 0o500); + }, + }), + ).rejects.toBeInstanceOf(SessionWriterUnavailableError); + expect(recoveryLease).toBeDefined(); + await expect(fs.readFile(lockPath, 'utf8')).resolves.toContain( + fixture.options.sessionId, + ); + } finally { + chmodSync(lockDir, 0o700); + } + + await recoveryLease?.release(); + await expect(fs.lstat(lockPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }, + ); + + it.runIf(process.platform === 'linux')( + 'uses a clock-independent Linux process identity', + async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const lockRecord = JSON.parse(await fs.readFile(lockPath, 'utf8')) as { + process_start_identity?: string; + }; + const [bootId, stat] = await Promise.all([ + fs.readFile('/proc/sys/kernel/random/boot_id', 'utf8'), + fs.readFile(`/proc/${process.pid}/stat`, 'utf8'), + ]); + const startTicks = stat + .slice(stat.lastIndexOf(')') + 1) + .trim() + .split(/\s+/)[19]; + + expect(lockRecord.process_start_identity).toBe( + `linux:${bootId.trim()}:${startTicks}`, + ); + await lease.release(); + }, + ); + + it.runIf(process.platform === 'darwin')( + 'does not reclaim a live Darwin owner across different time zones', + async () => { + const fixture = await createFixture(); + const owner = startLeaseProcess({ TZ: 'Pacific/Honolulu' }); + const contender = startLeaseProcess({ TZ: 'Asia/Shanghai' }); + expect( + await requestChild(owner, { + type: 'acquire', + options: fixture.options, + }), + ).toMatchObject({ ok: true }); + + expect( + await requestChild(contender, { + type: 'acquire', + options: fixture.options, + }), + ).toMatchObject({ + ok: false, + errorKind: 'session_writer_conflict', + }); + expect(await requestChild(owner, { type: 'release' })).toMatchObject({ + ok: true, + }); + }, + ); + + it('rejects a second process and reclaims its lock after SIGKILL', async () => { + const fixture = await createFixture(); + const child = startLeaseProcess(); + expect( + await requestChild(child, { type: 'acquire', options: fixture.options }), + ).toMatchObject({ ok: true }); + + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterConflictError); + + child.kill('SIGKILL'); + await waitForClose(child); + const replacement = await SessionWriterLease.acquire(fixture.options); + await replacement.release(); + }); + + it('fails closed when process liveness cannot be determined', async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const lockRecord = await fs.readFile(lockPath, 'utf8'); + await lease.release(); + await fs.writeFile(lockPath, lockRecord); + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => { + throw Object.assign(new Error('probe unavailable'), { code: 'EIO' }); + }); + + try { + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterConflictError); + } finally { + killSpy.mockRestore(); + await fs.unlink(lockPath).catch(() => {}); + } + }); + + it('detects external transcript and lock changes', async () => { + const fixture = await createFixture(); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile(fixture.transcriptPath, '{"seed":true}\n'); + const lease = await SessionWriterLease.acquire(fixture.options); + + await fs.appendFile(fixture.transcriptPath, '{"external":true}\n'); + await expect(lease.assertOwnedAndUnchanged()).rejects.toBeInstanceOf( + SessionTranscriptChangedError, + ); + + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + await fs.unlink(lockPath); + await fs.writeFile(lockPath, '{"replacement":true}'); + await expect(lease.assertOwnedAndUnchanged()).rejects.toBeInstanceOf( + SessionWriterLostError, + ); + await expect(lease.release()).rejects.toBeInstanceOf( + SessionWriterLostError, + ); + await expect(fs.readFile(lockPath, 'utf8')).resolves.toBe( + '{"replacement":true}', + ); + }); + + it.runIf(process.platform !== 'win32')( + 'classifies an unreadable owned lock as unavailable', + async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + await fs.chmod(lockPath, 0o000); + + try { + await expect(lease.assertOwnedAndUnchanged()).rejects.toBeInstanceOf( + SessionWriterUnavailableError, + ); + } finally { + await fs.chmod(lockPath, 0o600); + await lease.release(); + } + }, + ); + + it('fails closed on a malformed lock', async () => { + const fixture = await createFixture(); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + await fs.mkdir(path.dirname(lockPath), { recursive: true }); + await fs.writeFile(lockPath, 'not-json'); + + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterUnavailableError); + }); + + it('logs acquisition diagnostics without changing the public error', async () => { + const fixture = await createFixture('diagnostic-session'); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + await fs.mkdir(path.dirname(lockPath), { recursive: true }); + await fs.writeFile(lockPath, 'not-json'); + const previousDebugLogFile = process.env['QWEN_DEBUG_LOG_FILE']; + process.env['QWEN_DEBUG_LOG_FILE'] = '1'; + Storage.setRuntimeBaseDir(fixture.runtimeBaseDir); + resetDebugLoggingState(); + setDebugLogSession({ + getSessionId: () => fixture.options.sessionId, + }); + + try { + let failure: unknown; + try { + await SessionWriterLease.acquire(fixture.options); + } catch (error) { + failure = error; + } + expect(failure).toMatchObject({ + errorKind: 'session_writer_unavailable', + message: 'Session write ownership could not be verified.', + }); + + await vi.waitFor(async () => { + const log = await fs.readFile( + Storage.getDebugLogPath(fixture.options.sessionId), + 'utf8', + ); + expect(log).toContain( + 'stage=acquire errorKind=session_writer_unavailable', + ); + expect(log).toContain(`lockPath=${JSON.stringify(lockPath)}`); + expect(log).toContain( + 'cause=Error: Existing session writer lock is malformed', + ); + }); + } finally { + setDebugLogSession(null); + resetDebugLoggingState(); + Storage.setRuntimeBaseDir(null); + if (previousDebugLogFile === undefined) { + delete process.env['QWEN_DEBUG_LOG_FILE']; + } else { + process.env['QWEN_DEBUG_LOG_FILE'] = previousDebugLogFile; + } + } + }); + + it('fails closed on a non-regular lock', async () => { + const fixture = await createFixture(); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + await fs.mkdir(lockPath, { recursive: true }); + + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterUnavailableError); + }); + + it('fails closed on a truncated transcript tail', async () => { + const fixture = await createFixture(); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile( + fixture.transcriptPath, + '{"complete":true}\n{"partial":', + ); + + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionTranscriptChangedError); + await expect( + fs.access( + getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ), + ), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('detects an equal-length atomic transcript replacement', async () => { + const fixture = await createFixture(); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile(fixture.transcriptPath, '{"a":1}\n'); + const lease = await SessionWriterLease.acquire(fixture.options); + const replacement = `${fixture.transcriptPath}.replacement`; + await fs.writeFile(replacement, '{"b":2}\n'); + await fs.rename(replacement, fixture.transcriptPath); + + await expect(lease.assertOwnedAndUnchanged()).rejects.toBeInstanceOf( + SessionTranscriptChangedError, + ); + await lease.release(); + }); + + it('accounts for UTF-8 bytes and releases concurrently without losing ownership', async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + const value = { text: '调度🙂' }; + const expectedBytes = Buffer.byteLength(`${JSON.stringify(value)}\n`); + + await lease.appendJsonLine(value); + expect((await fs.readFile(fixture.transcriptPath)).byteLength).toBe( + expectedBytes, + ); + await expect( + Promise.all([lease.release(), lease.release()]), + ).resolves.toEqual([undefined, undefined]); + }); + + it.runIf(process.platform !== 'win32')( + 'creates the transcript directory with owner-only permissions', + async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + + await lease.appendJsonLine({ text: 'private' }); + + const [directoryStat, transcriptStat] = await Promise.all([ + fs.stat(path.dirname(fixture.transcriptPath)), + fs.stat(fixture.transcriptPath), + ]); + expect(directoryStat.mode & 0o777).toBe(0o700); + expect(transcriptStat.mode & 0o777).toBe(0o600); + await lease.release(); + }, + ); + + it.runIf(process.platform !== 'freebsd')( + 'retries release after a transient filesystem failure', + async () => { + const fixture = await createFixture(); + const lease = await SessionWriterLease.acquire(fixture.options); + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const backupPath = `${lockPath}.backup`; + await fs.rename(lockPath, backupPath); + await fs.mkdir(lockPath); + + await expect(lease.release()).rejects.toBeInstanceOf( + SessionWriterUnavailableError, + ); + + await fs.rmdir(lockPath); + await fs.rename(backupPath, lockPath); + await expect(lease.release()).resolves.toBeUndefined(); + }, + ); + + it('elects only one stale-lock reclaimer across processes', async () => { + const fixture = await createFixture(); + const owner = startLeaseProcess(); + expect( + await requestChild(owner, { type: 'acquire', options: fixture.options }), + ).toMatchObject({ ok: true }); + owner.kill('SIGKILL'); + await waitForClose(owner); + + const contenders = [startLeaseProcess(), startLeaseProcess()]; + const results = await Promise.all( + contenders.map((child) => + requestChild(child, { type: 'acquire', options: fixture.options }), + ), + ); + expect(results.filter((result) => result.ok)).toHaveLength(1); + const winner = contenders[results.findIndex((result) => result.ok)]!; + expect(await requestChild(winner, { type: 'release' })).toMatchObject({ + ok: true, + }); + }); + + it('recovers after a stale-lock reclaimer dies while holding its guard', async () => { + const fixture = await createFixture(); + const owner = startLeaseProcess(); + const acquired = await requestChild(owner, { + type: 'acquire', + options: fixture.options, + }); + expect(acquired).toMatchObject({ ok: true }); + expect(acquired.ownerId).toBeDefined(); + owner.kill('SIGKILL'); + await waitForClose(owner); + + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const reclaimPath = `${lockPath}.reclaim.${encodeURIComponent( + acquired.ownerId!, + )}`; + await fs.copyFile(lockPath, reclaimPath); + + const replacement = await SessionWriterLease.acquire(fixture.options); + await replacement.release(); + }); + + it('keeps the primary lock when reclaim guard cleanup is already complete', async () => { + const fixture = await createFixture(); + const owner = startLeaseProcess(); + const acquired = await requestChild(owner, { + type: 'acquire', + options: fixture.options, + }); + expect(acquired).toMatchObject({ ok: true }); + expect(acquired.ownerId).toBeDefined(); + owner.kill('SIGKILL'); + await waitForClose(owner); + + const lockPath = getSessionWriterLockPath( + fixture.runtimeBaseDir, + fixture.options.sessionId, + ); + const reclaimPath = `${lockPath}.reclaim.${encodeURIComponent( + acquired.ownerId!, + )}`; + const replacement = await SessionWriterLease.acquire({ + ...fixture.options, + onOwnershipAcquired: () => unlinkSync(reclaimPath), + }); + + expect((await fs.lstat(lockPath)).isFile()).toBe(true); + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterConflictError); + await replacement.release(); + }); + + it('reloads the authoritative tail before the next writer appends', async () => { + const sessionId = 'incident-session'; + const fixture = await createFixture(sessionId); + const firstUser = record( + 'user-1', + null, + sessionId, + fixture.projectRoot, + 'user', + '看下调度的 wiki', + ); + const firstToolTail = record( + 'tool-tail', + firstUser.uuid, + sessionId, + fixture.projectRoot, + 'assistant', + 'first tool result', + ); + await fs.mkdir(path.dirname(fixture.transcriptPath), { recursive: true }); + await fs.writeFile( + fixture.transcriptPath, + `${JSON.stringify(firstUser)}\n${JSON.stringify(firstToolTail)}\n`, + ); + + const processA = startLeaseProcess(); + expect( + await requestChild(processA, { + type: 'acquire', + options: fixture.options, + }), + ).toMatchObject({ ok: true }); + await expect( + SessionWriterLease.acquire(fixture.options), + ).rejects.toBeInstanceOf(SessionWriterConflictError); + + const finalAnswer = record( + 'final-answer', + firstToolTail.uuid, + sessionId, + fixture.projectRoot, + 'assistant', + '完整调度 Wiki 回答', + ); + expect( + await requestChild(processA, { type: 'append', value: finalAnswer }), + ).toMatchObject({ ok: true }); + expect(await requestChild(processA, { type: 'release' })).toMatchObject({ + ok: true, + }); + + const processBLease = await SessionWriterLease.acquire(fixture.options); + const sessionService = new SessionService(fixture.projectRoot, { + runtimeBaseDir: fixture.runtimeBaseDir, + }); + const authoritative = await sessionService.loadSession(sessionId); + expect(authoritative?.lastCompletedUuid).toBe(finalAnswer.uuid); + expect( + authoritative?.conversation.messages.map((message) => message.uuid), + ).toEqual([firstUser.uuid, firstToolTail.uuid, finalAnswer.uuid]); + + const config = { + getSessionId: () => sessionId, + getResumedSessionData: () => authoritative, + getProjectRoot: () => fixture.projectRoot, + getCliVersion: () => 'test', + getFastModel: () => undefined, + isInteractive: () => false, + } as unknown as Config; + const recorder = new ChatRecordingService(config); + recorder.activate(processBLease, authoritative); + recorder.recordUserMessage([{ text: '你好' }]); + await recorder.flush(); + await recorder.close(); + + const physicalRecords = (await fs.readFile(fixture.transcriptPath, 'utf8')) + .trim() + .split('\n') + .map((line) => JSON.parse(line) as ChatRecord); + expect(physicalRecords.at(-1)?.parentUuid).toBe(finalAnswer.uuid); + const reloaded = await sessionService.loadSession(sessionId); + expect( + reloaded?.conversation.messages.map((message) => message.uuid), + ).toEqual(physicalRecords.map((message) => message.uuid)); + }); +}); diff --git a/packages/core/src/services/session-writer-lease.ts b/packages/core/src/services/session-writer-lease.ts new file mode 100644 index 00000000000..b8e19c4e81b --- /dev/null +++ b/packages/core/src/services/session-writer-lease.ts @@ -0,0 +1,859 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { execFile } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; +import type { Stats } from 'node:fs'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { createDebugLogger } from '../utils/debugLogger.js'; + +const LOCK_SCHEMA_VERSION = 1; +const MALFORMED_RETRY_COUNT = 3; +const MALFORMED_RETRY_DELAY_MS = 50; +const ACQUIRE_ATTEMPTS = 8; +const debugLogger = createDebugLogger('SESSION_WRITER_LEASE'); + +function describeError(error: unknown): string { + if (!(error instanceof Error)) return String(error); + const code = (error as NodeJS.ErrnoException).code; + return `${error.name}: ${error.message}${code ? ` code=${code}` : ''}`; +} + +function describeDiagnosticError(error: unknown): string { + const description = describeError(error); + return error instanceof Error && error.cause !== undefined + ? `${description} cause=${describeError(error.cause)}` + : description; +} + +export type SessionWriterProcessKind = + | 'interactive' + | 'acp' + | 'daemon' + | 'unknown'; + +export type SessionWriterErrorKind = + | 'session_writer_conflict' + | 'session_writer_lost' + | 'session_transcript_changed' + | 'session_writer_unavailable'; + +export abstract class SessionWriterError extends Error { + abstract readonly rpcCode: number; + abstract readonly errorKind: SessionWriterErrorKind; + abstract readonly httpStatus: 409 | 503; +} + +export const SESSION_WRITER_RPC_CODES = { + session_writer_conflict: -32020, + session_writer_lost: -32021, + session_transcript_changed: -32022, + session_writer_unavailable: -32023, +} as const; + +export class SessionWriterConflictError extends SessionWriterError { + override readonly name = 'SessionWriterConflictError'; + readonly rpcCode = SESSION_WRITER_RPC_CODES.session_writer_conflict; + readonly errorKind = 'session_writer_conflict'; + readonly httpStatus = 409; + + constructor() { + super('This session is already open in another Qwen process.'); + } +} + +export class SessionWriterLostError extends SessionWriterError { + override readonly name = 'SessionWriterLostError'; + readonly rpcCode = SESSION_WRITER_RPC_CODES.session_writer_lost; + readonly errorKind = 'session_writer_lost'; + readonly httpStatus = 409; + + constructor() { + super('Write ownership for this session was lost.'); + } +} + +export class SessionTranscriptChangedError extends SessionWriterError { + override readonly name = 'SessionTranscriptChangedError'; + readonly rpcCode = SESSION_WRITER_RPC_CODES.session_transcript_changed; + readonly errorKind = 'session_transcript_changed'; + readonly httpStatus = 409; + + constructor() { + super('The session transcript changed outside its active writer.'); + } +} + +export class SessionWriterUnavailableError extends SessionWriterError { + override readonly name = 'SessionWriterUnavailableError'; + readonly rpcCode = SESSION_WRITER_RPC_CODES.session_writer_unavailable; + readonly errorKind = 'session_writer_unavailable'; + readonly httpStatus = 503; + + constructor(options?: ErrorOptions) { + super('Session write ownership could not be verified.', options); + } +} + +interface SessionWriterLockRecord { + schema_version: number; + session_id: string; + owner_id: string; + pid: number; + process_start_identity?: string; + hostname: string; + process_kind: SessionWriterProcessKind; + acquired_at: string; + qwen_version: string | null; +} + +export interface AcquireSessionWriterLeaseOptions { + runtimeBaseDir: string; + sessionId: string; + transcriptPath: string; + processKind?: SessionWriterProcessKind; + qwenVersion?: string | null; + onOwnershipAcquired?: (lease: SessionWriterLease) => void; +} + +type ExistingLockState = + | { kind: 'missing' } + | { kind: 'live' } + | { kind: 'stale'; record: SessionWriterLockRecord } + | { kind: 'malformed' }; + +interface TranscriptFingerprint { + dev: number; + ino: number; + birthtimeMs: number; + ctimeMs: number; + mtimeMs: number; +} + +type TranscriptState = + | { exists: false; byteLength: 0 } + | { + exists: true; + byteLength: number; + fingerprint: TranscriptFingerprint; + }; + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code !== 'ESRCH'; + } +} + +async function execFileText( + file: string, + args: readonly string[], + env?: NodeJS.ProcessEnv, +): Promise { + return new Promise((resolve) => { + try { + execFile( + file, + args, + { + encoding: 'utf8', + timeout: 1_000, + windowsHide: true, + ...(env ? { env } : {}), + }, + (error, stdout) => { + const value = stdout.trim(); + resolve(error || value.length === 0 ? null : value); + }, + ); + } catch { + resolve(null); + } + }); +} + +async function readProcessStartIdentity(pid: number): Promise { + if (process.platform === 'linux') { + try { + const [stat, bootId] = await Promise.all([ + fs.readFile(`/proc/${pid}/stat`, 'utf8'), + fs.readFile('/proc/sys/kernel/random/boot_id', 'utf8'), + ]); + const fields = stat + .slice(stat.lastIndexOf(')') + 1) + .trim() + .split(/\s+/); + const startTicks = fields[19]; + if ( + !startTicks || + !/^\d+$/.test(startTicks) || + !/^[0-9a-f-]+$/i.test(bootId.trim()) + ) { + return null; + } + return `linux:${bootId.trim()}:${startTicks}`; + } catch { + return null; + } + } + if (process.platform === 'darwin') { + const startedAt = await execFileText( + '/bin/ps', + ['-o', 'lstart=', '-p', String(pid)], + { ...process.env, LC_ALL: 'C', LANG: 'C', TZ: 'UTC' }, + ); + return startedAt ? `darwin:${startedAt}` : null; + } + if (process.platform === 'win32') { + const startedAt = await execFileText('powershell.exe', [ + '-NoProfile', + '-NonInteractive', + '-Command', + `$targetProcess = Get-Process -Id ${pid} -ErrorAction Stop; $targetProcess.StartTime.ToUniversalTime().Ticks`, + ]); + return startedAt && /^\d+$/.test(startedAt) ? `win32:${startedAt}` : null; + } + return null; +} + +function isLockRecord(value: unknown): value is SessionWriterLockRecord { + if (!value || typeof value !== 'object' || Array.isArray(value)) return false; + const record = value as Record; + const processKind = record['process_kind']; + return ( + record['schema_version'] === LOCK_SCHEMA_VERSION && + typeof record['session_id'] === 'string' && + record['session_id'].length > 0 && + typeof record['owner_id'] === 'string' && + record['owner_id'].length > 0 && + Number.isInteger(record['pid']) && + (record['pid'] as number) > 0 && + (record['process_start_identity'] === undefined || + (typeof record['process_start_identity'] === 'string' && + record['process_start_identity'].length > 0)) && + typeof record['hostname'] === 'string' && + record['hostname'].length > 0 && + typeof processKind === 'string' && + ['interactive', 'acp', 'daemon', 'unknown'].includes(processKind) && + typeof record['acquired_at'] === 'string' && + Number.isFinite(Date.parse(record['acquired_at'])) && + (record['qwen_version'] === null || + typeof record['qwen_version'] === 'string') + ); +} + +function parseLockRecord(raw: string): SessionWriterLockRecord | null { + try { + const parsed: unknown = JSON.parse(raw); + return isLockRecord(parsed) ? parsed : null; + } catch { + return null; + } +} + +async function lockStateForRecord( + record: SessionWriterLockRecord, +): Promise { + if (record.hostname !== os.hostname()) return { kind: 'live' }; + if (!isProcessAlive(record.pid)) return { kind: 'stale', record }; + if (!record.process_start_identity) return { kind: 'live' }; + const currentStartIdentity = await readProcessStartIdentity(record.pid); + return currentStartIdentity !== null && + currentStartIdentity !== record.process_start_identity + ? { kind: 'stale', record } + : { kind: 'live' }; +} + +async function delay(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)); +} + +function transcriptFingerprint(stat: Stats): TranscriptFingerprint { + return { + dev: stat.dev, + ino: stat.ino, + birthtimeMs: stat.birthtimeMs, + ctimeMs: stat.ctimeMs, + mtimeMs: stat.mtimeMs, + }; +} + +function sameFileIdentity( + left: TranscriptFingerprint, + right: TranscriptFingerprint, +): boolean { + return ( + left.dev === right.dev && + left.ino === right.ino && + left.birthtimeMs === right.birthtimeMs + ); +} + +function sameTranscriptState( + left: TranscriptState, + right: TranscriptState, +): boolean { + if (left.exists !== right.exists) return false; + if (!left.exists || !right.exists) return true; + return ( + left.byteLength === right.byteLength && + sameFileIdentity(left.fingerprint, right.fingerprint) && + left.fingerprint.ctimeMs === right.fingerprint.ctimeMs && + left.fingerprint.mtimeMs === right.fingerprint.mtimeMs + ); +} + +async function getTranscriptState(filePath: string): Promise { + let handle: fs.FileHandle | undefined; + try { + try { + handle = await fs.open(filePath, 'r'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { exists: false, byteLength: 0 }; + } + throw error; + } + const [handleStat, pathStat] = await Promise.all([ + handle.stat(), + fs.lstat(filePath), + ]); + if ( + !handleStat.isFile() || + !pathStat.isFile() || + pathStat.isSymbolicLink() + ) { + throw new SessionWriterUnavailableError(); + } + const handleFingerprint = transcriptFingerprint(handleStat); + const pathFingerprint = transcriptFingerprint(pathStat); + if (!sameFileIdentity(handleFingerprint, pathFingerprint)) { + throw new SessionTranscriptChangedError(); + } + if (handleStat.size > 0) { + const lastByte = Buffer.allocUnsafe(1); + const { bytesRead } = await handle.read( + lastByte, + 0, + 1, + handleStat.size - 1, + ); + if (bytesRead !== 1 || lastByte[0] !== 0x0a) { + throw new SessionTranscriptChangedError(); + } + } + return { + exists: true, + byteLength: handleStat.size, + fingerprint: handleFingerprint, + }; + } catch (error) { + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } finally { + await handle?.close().catch(() => {}); + } +} + +async function restoreMovedLock( + movedPath: string, + lockPath: string, +): Promise { + try { + await fs.link(movedPath, lockPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') { + await fs.unlink(movedPath).catch(() => {}); + return; + } + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + await fs.unlink(movedPath).catch(() => {}); +} + +async function installLockRecord( + lockPath: string, + record: SessionWriterLockRecord, +): Promise { + const temporaryPath = `${lockPath}.${record.owner_id}.tmp`; + let handle: fs.FileHandle | undefined; + try { + handle = await fs.open(temporaryPath, 'wx', 0o600); + await handle.writeFile(JSON.stringify(record), 'utf8'); + await handle.sync(); + await handle.close(); + handle = undefined; + try { + await fs.link(temporaryPath, lockPath); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'EEXIST') return false; + throw error; + } + } catch (error) { + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } finally { + await handle?.close().catch(() => {}); + await fs.unlink(temporaryPath).catch(() => {}); + } +} + +async function acquireReclaimGuard( + lockPath: string, + staleOwnerId: string, + record: SessionWriterLockRecord, + inspect: ( + lockPath: string, + expectedSessionId: string, + ) => Promise, +): Promise { + const basePath = `${lockPath}.reclaim.${encodeURIComponent(staleOwnerId)}`; + let guardPath = basePath; + for (let attempt = 0; attempt < ACQUIRE_ATTEMPTS; attempt++) { + if (await installLockRecord(guardPath, record)) return guardPath; + const state = await inspect(guardPath, record.session_id); + if (state.kind === 'missing') continue; + if (state.kind === 'live') throw new SessionWriterUnavailableError(); + if (state.kind === 'malformed') { + throw new SessionWriterUnavailableError(); + } + guardPath = `${basePath}.${encodeURIComponent(state.record.owner_id)}`; + } + throw new SessionWriterUnavailableError(); +} + +async function removeOwnedLock( + lockPath: string, + ownerId: string, +): Promise { + const record = parseLockRecord(await fs.readFile(lockPath, 'utf8')); + if (!record || record.owner_id !== ownerId) { + throw new SessionWriterLostError(); + } + await fs.unlink(lockPath); +} + +export function getSessionWriterLockPath( + runtimeBaseDir: string, + sessionId: string, +): string { + return path.join( + runtimeBaseDir, + 'tmp', + 'session-writer-locks', + `${encodeURIComponent(sessionId)}.lock`, + ); +} + +export class SessionWriterLease { + readonly ownerId: string; + readonly sessionId: string; + readonly runtimeBaseDir: string; + readonly transcriptPath: string; + private expectedTranscriptState: TranscriptState | undefined; + private released = false; + private releasePromise: Promise | undefined; + + private constructor( + private readonly lockPath: string, + lockRecord: SessionWriterLockRecord, + options: AcquireSessionWriterLeaseOptions, + ) { + this.ownerId = lockRecord.owner_id; + this.sessionId = options.sessionId; + this.runtimeBaseDir = options.runtimeBaseDir; + this.transcriptPath = options.transcriptPath; + } + + get transcriptExistedAtAcquire(): boolean { + if (!this.expectedTranscriptState) { + throw new SessionWriterUnavailableError(); + } + return this.expectedTranscriptState.exists; + } + + static async acquire( + options: AcquireSessionWriterLeaseOptions, + ): Promise { + try { + return await SessionWriterLease.acquireInternal(options); + } catch (error) { + const lockPath = getSessionWriterLockPath( + path.resolve(options.runtimeBaseDir), + options.sessionId, + ); + const errorKind = + error instanceof SessionWriterError ? error.errorKind : 'unknown'; + debugLogger.debug( + `Session writer lease acquisition failed stage=acquire errorKind=${errorKind} ` + + `lockPath=${JSON.stringify(lockPath)} ` + + `transcriptPath=${JSON.stringify(path.resolve(options.transcriptPath))} ` + + `error=${describeDiagnosticError(error)}`, + ); + throw error; + } + } + + private static async acquireInternal( + options: AcquireSessionWriterLeaseOptions, + ): Promise { + const normalizedOptions = { + ...options, + runtimeBaseDir: path.resolve(options.runtimeBaseDir), + transcriptPath: path.resolve(options.transcriptPath), + }; + const lockPath = getSessionWriterLockPath( + normalizedOptions.runtimeBaseDir, + normalizedOptions.sessionId, + ); + const lockDir = path.dirname(lockPath); + try { + await fs.mkdir(lockDir, { recursive: true, mode: 0o700 }); + const lockDirStat = await fs.lstat(lockDir); + if (!lockDirStat.isDirectory() || lockDirStat.isSymbolicLink()) { + throw new SessionWriterUnavailableError({ + cause: new Error( + 'Session writer lock directory is not a regular directory', + ), + }); + } + } catch (error) { + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + + const processStartIdentity = await readProcessStartIdentity(process.pid); + const lockRecord: SessionWriterLockRecord = { + schema_version: LOCK_SCHEMA_VERSION, + session_id: normalizedOptions.sessionId, + owner_id: randomUUID(), + pid: process.pid, + ...(processStartIdentity + ? { process_start_identity: processStartIdentity } + : {}), + hostname: os.hostname(), + process_kind: normalizedOptions.processKind ?? 'unknown', + acquired_at: new Date().toISOString(), + qwen_version: normalizedOptions.qwenVersion ?? null, + }; + + for (let attempt = 0; attempt < ACQUIRE_ATTEMPTS; attempt++) { + if (await installLockRecord(lockPath, lockRecord)) { + return SessionWriterLease.finishAcquisition( + lockPath, + lockRecord, + normalizedOptions, + ); + } + + const state = await SessionWriterLease.inspectExistingLock( + lockPath, + normalizedOptions.sessionId, + ); + if (state.kind === 'missing') continue; + if (state.kind === 'live') throw new SessionWriterConflictError(); + if (state.kind === 'malformed') { + throw new SessionWriterUnavailableError({ + cause: new Error('Existing session writer lock is malformed'), + }); + } + + const staleOwnerId = state.record.owner_id; + const reclaimPath = await acquireReclaimGuard( + lockPath, + staleOwnerId, + lockRecord, + (candidatePath, sessionId) => + SessionWriterLease.inspectExistingLock(candidatePath, sessionId), + ); + let primaryInstalled = false; + let staleMoved = false; + const stalePath = `${lockPath}.stale.${process.pid}.${randomUUID()}`; + try { + const currentState = await SessionWriterLease.inspectExistingLock( + lockPath, + normalizedOptions.sessionId, + ); + if ( + currentState.kind !== 'stale' || + currentState.record.owner_id !== staleOwnerId + ) { + throw currentState.kind === 'live' + ? new SessionWriterConflictError() + : new SessionWriterUnavailableError(); + } + await fs.rename(lockPath, stalePath); + staleMoved = true; + const movedState = await SessionWriterLease.inspectExistingLock( + stalePath, + normalizedOptions.sessionId, + ); + if ( + movedState.kind !== 'stale' || + movedState.record.owner_id !== staleOwnerId + ) { + await restoreMovedLock(stalePath, lockPath); + staleMoved = false; + throw movedState.kind === 'live' + ? new SessionWriterConflictError() + : new SessionWriterUnavailableError(); + } + await fs.unlink(stalePath); + staleMoved = false; + if (!(await installLockRecord(lockPath, lockRecord))) { + throw new SessionWriterUnavailableError(); + } + primaryInstalled = true; + const lease = await SessionWriterLease.finishAcquisition( + lockPath, + lockRecord, + normalizedOptions, + ); + await removeOwnedLock(reclaimPath, lockRecord.owner_id).catch(() => {}); + return lease; + } catch (error) { + if (staleMoved) { + await restoreMovedLock(stalePath, lockPath).catch(() => {}); + } + if (primaryInstalled) { + await removeOwnedLock(lockPath, lockRecord.owner_id).catch(() => {}); + } + await removeOwnedLock(reclaimPath, lockRecord.owner_id).catch(() => {}); + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + } + + throw new SessionWriterUnavailableError(); + } + + private static async finishAcquisition( + lockPath: string, + lockRecord: SessionWriterLockRecord, + options: AcquireSessionWriterLeaseOptions, + ): Promise { + const lease = new SessionWriterLease(lockPath, lockRecord, options); + try { + options.onOwnershipAcquired?.(lease); + lease.expectedTranscriptState = await getTranscriptState( + options.transcriptPath, + ); + return lease; + } catch (error) { + try { + await removeOwnedLock(lockPath, lockRecord.owner_id); + } catch { + throw new SessionWriterUnavailableError(); + } + throw error; + } + } + + private static async inspectExistingLock( + lockPath: string, + expectedSessionId: string, + ): Promise { + for (let attempt = 0; attempt < MALFORMED_RETRY_COUNT; attempt++) { + let stat: Awaited>; + try { + stat = await fs.lstat(lockPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { kind: 'missing' }; + } + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + if (!stat.isFile() || stat.isSymbolicLink()) { + throw new SessionWriterUnavailableError({ + cause: new Error('Session writer lock is not a regular file'), + }); + } + + let raw: string; + try { + raw = await fs.readFile(lockPath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { kind: 'missing' }; + } + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + const record = parseLockRecord(raw); + if (record) { + if (record.session_id !== expectedSessionId) { + throw new SessionWriterUnavailableError({ + cause: new Error('Session writer lock belongs to another session'), + }); + } + return lockStateForRecord(record); + } + if (attempt + 1 < MALFORMED_RETRY_COUNT) { + await delay(MALFORMED_RETRY_DELAY_MS); + } + } + return { kind: 'malformed' }; + } + + private async readOwnedLock(): Promise { + if (this.released) throw new SessionWriterLostError(); + let stat: Awaited>; + try { + stat = await fs.lstat(this.lockPath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new SessionWriterLostError(); + } + throw new SessionWriterUnavailableError(); + } + if (!stat.isFile() || stat.isSymbolicLink()) { + throw new SessionWriterLostError(); + } + let raw: string; + try { + raw = await fs.readFile(this.lockPath, 'utf8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + throw new SessionWriterLostError(); + } + throw new SessionWriterUnavailableError(); + } + const record = parseLockRecord(raw); + if (!record || record.owner_id !== this.ownerId) { + throw new SessionWriterLostError(); + } + return record; + } + + async assertOwnedAndUnchanged(): Promise { + await this.readOwnedLock(); + if (!this.expectedTranscriptState) { + throw new SessionWriterUnavailableError(); + } + const transcriptState = await getTranscriptState(this.transcriptPath); + if (!sameTranscriptState(transcriptState, this.expectedTranscriptState)) { + throw new SessionTranscriptChangedError(); + } + } + + async appendJsonLine(value: unknown): Promise { + let serialized: string | undefined; + try { + serialized = JSON.stringify(value); + } catch (error) { + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } + if (serialized === undefined) throw new SessionWriterUnavailableError(); + const bytes = Buffer.from(`${serialized}\n`, 'utf8'); + await this.assertOwnedAndUnchanged(); + const expectedBefore = this.expectedTranscriptState; + if (!expectedBefore) throw new SessionWriterUnavailableError(); + const nextByteLength = expectedBefore.byteLength + bytes.byteLength; + let handle: fs.FileHandle | undefined; + try { + await fs.mkdir(path.dirname(this.transcriptPath), { + recursive: true, + mode: 0o700, + }); + handle = await fs.open( + this.transcriptPath, + expectedBefore.exists ? 'a+' : 'ax+', + 0o600, + ); + const beforeStat = await handle.stat(); + const beforeState: TranscriptState = { + exists: true, + byteLength: beforeStat.size, + fingerprint: transcriptFingerprint(beforeStat), + }; + if ( + expectedBefore.exists + ? !sameTranscriptState(beforeState, expectedBefore) + : beforeStat.size !== 0 + ) { + throw new SessionTranscriptChangedError(); + } + await this.readOwnedLock(); + await handle.writeFile(bytes); + await handle.sync(); + const afterStat = await handle.stat(); + if (afterStat.size !== nextByteLength) { + throw new SessionTranscriptChangedError(); + } + const writtenFingerprint = transcriptFingerprint(afterStat); + await handle.close(); + handle = undefined; + const transcriptState = await getTranscriptState(this.transcriptPath); + if ( + !transcriptState.exists || + transcriptState.byteLength !== nextByteLength || + !sameFileIdentity(transcriptState.fingerprint, writtenFingerprint) + ) { + throw new SessionTranscriptChangedError(); + } + await this.readOwnedLock(); + this.expectedTranscriptState = transcriptState; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EEXIST' || code === 'ENOENT') { + throw new SessionTranscriptChangedError(); + } + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError({ + cause: error instanceof Error ? error : undefined, + }); + } finally { + await handle?.close().catch(() => {}); + } + } + + release(): Promise { + this.releasePromise ??= this.releaseOnce().catch((error: unknown) => { + if (!this.released) this.releasePromise = undefined; + throw error; + }); + return this.releasePromise; + } + + private async releaseOnce(): Promise { + if (this.released) return; + try { + await removeOwnedLock(this.lockPath, this.ownerId); + this.released = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + this.released = true; + throw new SessionWriterLostError(); + } + if (error instanceof SessionWriterLostError) { + this.released = true; + throw error; + } + if (error instanceof SessionWriterError) throw error; + throw new SessionWriterUnavailableError(); + } + } +} diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 90e947d6783..63c835ff038 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -192,6 +192,7 @@ export interface UnarchiveSessionsOptions { export interface SessionServiceOptions { onWarning?: (message: string) => void; + runtimeBaseDir?: string; } /** @@ -323,7 +324,7 @@ export class SessionService { private readonly onWarning: ((message: string) => void) | undefined; constructor(cwd: string, options: SessionServiceOptions = {}) { - this.storage = new Storage(cwd); + this.storage = new Storage(cwd, options.runtimeBaseDir); this.projectRoot = cwd; this.projectHash = getProjectHash(cwd); this.onWarning = options.onWarning; diff --git a/packages/desktop/packages/shared/src/agent/__tests__/qwen-agent-slash-history.test.ts b/packages/desktop/packages/shared/src/agent/__tests__/qwen-agent-slash-history.test.ts index 96c3293d5d0..59aba4e828a 100644 --- a/packages/desktop/packages/shared/src/agent/__tests__/qwen-agent-slash-history.test.ts +++ b/packages/desktop/packages/shared/src/agent/__tests__/qwen-agent-slash-history.test.ts @@ -29,7 +29,7 @@ type QwenHistoryInternals = { sessionId: string, cwd: string, sourceElements?: NonNullable, - ) => void; + ) => Promise; applyQwenTranscriptTextElements: ( messages: Message[], sessionId: string, @@ -71,6 +71,8 @@ type QwenAvailableCommandsInternals = { signal: { aborted: boolean }; } | null; qwenSessionId: string | null; + persistedQwenSessionId: string | null; + qwenPersistenceCwd: string | null; _isProcessing: boolean; currentTurnId?: string; handleExtMethod: ( @@ -80,9 +82,12 @@ type QwenAvailableCommandsInternals = { suppressedSessionUpdates: Set; eventQueue: { hasPending: boolean; + isComplete: boolean; drain: () => AsyncGenerator; }; ensureProcess: () => Promise; + ensureQwenSession: () => Promise; + waitForCurrentTurnUsage: () => Promise; startProcess: () => Promise; callAcp: ( method: string, @@ -100,6 +105,17 @@ type QwenAvailableCommandsInternals = { flushPendingAvailableCommandsUpdate: (sessionId: string) => void; }; +function deferred(): { + promise: Promise; + resolve: (value: T) => void; +} { + let resolve!: (value: T) => void; + const promise = new Promise((next) => { + resolve = next; + }); + return { promise, resolve }; +} + type QwenSpawnInternals = { buildSpawnCommand: ( qwenCliPath: string, @@ -578,9 +594,7 @@ describe('QwenAgent slash command history', () => { }, ], }); - expect(onMidTurnMessagesDrained).toHaveBeenCalledWith([ - 'optimistic-image', - ]); + expect(onMidTurnMessagesDrained).toHaveBeenCalledWith(['optimistic-image']); agent.destroy(); }); @@ -817,7 +831,7 @@ describe('QwenAgent slash command history', () => { ]); }); - it('writes slash command text elements into the Qwen transcript user record', () => { + it('records slash command text elements through ACP without rewriting the transcript', async () => { const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); tempRoots.push(runtimeRoot, cwd); @@ -838,7 +852,12 @@ describe('QwenAgent slash command history', () => { ]); const agent = createAgent(cwd); - ( + const extMethod = mock(async () => ({})); + (agent as unknown as QwenAvailableCommandsInternals).callAcp = async ( + _method, + execute, + ) => await execute({ extMethod }); + await ( agent as unknown as QwenHistoryInternals ).persistQwenTranscriptTextElements(sessionId, cwd, [ { @@ -853,7 +872,75 @@ describe('QwenAgent slash command history', () => { const records = readQwenTranscript(runtimeRoot, cwd, sessionId); agent.destroy(); - expect(records[0]?.textElements).toEqual([ + expect(records[0]?.textElements).toBeUndefined(); + expect(extMethod).toHaveBeenCalledWith('qwen/session/recordTextElements', { + sessionId, + content: '/qc-helper hello', + textElements: [ + { + type: 'slash_command', + byte_range: { start: 0, end: 10 }, + placeholder: '/qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ], + }); + }); + + it('keeps text-element persistence and replay on the pre-cd transcript root', async () => { + const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); + const persistenceCwd = mkdtempSync(join(tmpdir(), 'qwen-original-cwd-')); + const logicalCwd = mkdtempSync(join(tmpdir(), 'qwen-after-cd-')); + tempRoots.push(runtimeRoot, persistenceCwd, logicalCwd); + process.env.QWEN_RUNTIME_DIR = runtimeRoot; + + const sessionId = 'session-after-logical-cd'; + writeQwenTranscript(runtimeRoot, persistenceCwd, sessionId, [ + { + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-30T08:02:52.927Z', + type: 'user', + cwd: logicalCwd, + version: 'test', + message: { role: 'user', parts: [{ text: '/qc-helper' }] }, + }, + { + uuid: 'm1', + parentUuid: 'u1', + sessionId, + timestamp: '2026-04-30T08:02:53.927Z', + type: 'system', + subtype: 'user_text_elements', + cwd: logicalCwd, + version: 'test', + systemPayload: { + content: '/qc-helper', + textElements: [ + { + type: 'slash_command', + byte_range: { start: 0, end: 10 }, + placeholder: '/qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ], + }, + }, + ]); + + const agent = createAgent(persistenceCwd); + const internals = agent as unknown as QwenAvailableCommandsInternals & + QwenHistoryInternals; + internals.qwenSessionId = sessionId; + internals.qwenPersistenceCwd = persistenceCwd; + const extMethod = mock(async () => ({})); + internals.callAcp = async (_method, execute) => + await execute({ extMethod }); + + await internals.persistQwenTranscriptTextElements(sessionId, logicalCwd, [ { type: 'slash_command', byte_range: { start: 0, end: 10 }, @@ -862,9 +949,180 @@ describe('QwenAgent slash command history', () => { target: 'qc-helper', }, ]); + const messages = internals.applyQwenTranscriptTextElements( + [ + { + id: 'message-1', + role: 'user', + content: '/qc-helper', + timestamp: Date.parse('2026-04-30T08:02:52.927Z'), + }, + ], + sessionId, + logicalCwd, + ); + agent.destroy(); + + expect(extMethod).toHaveBeenCalledWith( + 'qwen/session/recordTextElements', + expect.objectContaining({ sessionId, content: '/qc-helper' }), + ); + expect(messages[0]?.textElements).toEqual([ + { + type: 'slash_command', + byte_range: { start: 0, end: 10 }, + placeholder: '/qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ]); + }); + + it('does not complete a newer turn when prior text-element persistence resumes', async () => { + const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); + const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); + tempRoots.push(runtimeRoot, cwd); + process.env.QWEN_RUNTIME_DIR = runtimeRoot; + + const sessionId = 'session-with-delayed-text-elements'; + const firstRecord = { + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-30T08:02:52.927Z', + type: 'user', + cwd, + version: 'test', + message: { role: 'user', parts: [{ text: '/first' }] }, + }; + const secondRecord = { + uuid: 'u2', + parentUuid: 'u1', + sessionId, + timestamp: '2026-04-30T08:02:53.927Z', + type: 'user', + cwd, + version: 'test', + message: { role: 'user', parts: [{ text: '/second' }] }, + }; + writeQwenTranscript(runtimeRoot, cwd, sessionId, [firstRecord]); + + const firstPersistStarted = deferred(); + const firstPersistGate = deferred>(); + const secondPromptStarted = deferred(); + const secondPromptGate = deferred>(); + const agent = createAgent(cwd); + const internals = agent as unknown as QwenAvailableCommandsInternals; + internals.qwenSessionId = sessionId; + internals.ensureProcess = async () => {}; + internals.ensureQwenSession = async () => {}; + internals.waitForCurrentTurnUsage = async () => {}; + let promptCalls = 0; + let persistenceCalls = 0; + internals.callAcp = (async (method: string): Promise => { + if (method === 'session/prompt') { + promptCalls++; + if (promptCalls === 1) { + return { stopReason: 'end_turn' } as T; + } + secondPromptStarted.resolve(); + return (await secondPromptGate.promise) as T; + } + if (method === 'ext/qwen/session/recordTextElements') { + persistenceCalls++; + if (persistenceCalls === 1) { + firstPersistStarted.resolve(); + return (await firstPersistGate.promise) as T; + } + } + return {} as T; + }) as QwenAvailableCommandsInternals['callAcp']; + + const textElements = [ + { + type: 'slash_command' as const, + byte_range: { start: 0, end: 6 }, + placeholder: '/first', + label: 'first', + target: 'first', + }, + ]; + const firstIterator = agent.chat('/first', undefined, { textElements }); + void firstIterator.next(); + await firstPersistStarted.promise; + + writeQwenTranscript(runtimeRoot, cwd, sessionId, [ + firstRecord, + secondRecord, + ]); + const secondIterator = agent.chat('/second', undefined, { textElements }); + const secondNext = secondIterator.next(); + await secondPromptStarted.promise; + firstPersistGate.resolve({}); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(internals.eventQueue.isComplete).toBe(false); + + secondPromptGate.resolve({ stopReason: 'end_turn' }); + await expect(secondNext).resolves.toMatchObject({ + value: { type: 'complete' }, + done: false, + }); + await secondIterator.return(undefined); + agent.destroy(); + }); + + it('does not replace a writer-conflicted persisted session with a fresh session', async () => { + const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); + tempRoots.push(cwd); + + const agent = createAgent(cwd); + const internals = agent as unknown as QwenAvailableCommandsInternals; + internals.persistedQwenSessionId = 'persisted-session'; + internals.qwenPersistenceCwd = cwd; + internals.ensureProcess = async () => {}; + let newSessionCalls = 0; + internals.callAcp = async (method, execute) => { + if (method === 'session/load') { + return execute({ + loadSession: async () => { + throw { + code: -32020, + message: 'Writer conflict', + data: { errorKind: 'session_writer_conflict' }, + }; + }, + }); + } + if (method === 'session/new') { + newSessionCalls += 1; + return execute({ + newSession: async () => ({ + sessionId: 'unexpected-fresh-session', + models: {}, + modes: {}, + }), + }); + } + throw new Error(`Unexpected ACP method ${method}`); + }; + + const events: AgentEvent[] = []; + for await (const event of agent.chat('hello')) { + events.push(event); + } + agent.destroy(); + + expect(newSessionCalls).toBe(0); + expect(events).toEqual([ + { + type: 'error', + message: 'Writer conflict: {"errorKind":"session_writer_conflict"}', + }, + { type: 'complete' }, + ]); }); - it('writes skill text elements into the Qwen transcript user record', () => { + it('records skill text elements through ACP', async () => { const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); tempRoots.push(runtimeRoot, cwd); @@ -885,7 +1143,12 @@ describe('QwenAgent slash command history', () => { ]); const agent = createAgent(cwd); - ( + const extMethod = mock(async () => ({})); + (agent as unknown as QwenAvailableCommandsInternals).callAcp = async ( + _method, + execute, + ) => await execute({ extMethod }); + await ( agent as unknown as QwenHistoryInternals ).persistQwenTranscriptTextElements(sessionId, cwd, [ { @@ -897,18 +1160,24 @@ describe('QwenAgent slash command history', () => { }, ]); - const records = readQwenTranscript(runtimeRoot, cwd, sessionId); agent.destroy(); - expect(records[0]?.textElements).toEqual([ - { - type: 'skill', - byte_range: { start: 0, end: 10 }, - placeholder: '@qc-helper', - label: 'qc-helper', - target: 'qc-helper', - }, - ]); + expect(extMethod).toHaveBeenCalledWith( + 'qwen/session/recordTextElements', + expect.objectContaining({ + sessionId, + content: '@qc-helper', + textElements: [ + { + type: 'skill', + byte_range: { start: 0, end: 10 }, + placeholder: '@qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ], + }), + ); }); it('loads text elements back from the Qwen transcript', () => { @@ -968,6 +1237,76 @@ describe('QwenAgent slash command history', () => { ]); }); + it('loads append-only text element records from the Qwen transcript', () => { + const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); + const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); + tempRoots.push(runtimeRoot, cwd); + process.env.QWEN_RUNTIME_DIR = runtimeRoot; + + const sessionId = 'session-with-append-only-text-elements'; + writeQwenTranscript(runtimeRoot, cwd, sessionId, [ + { + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-30T08:02:52.927Z', + type: 'user', + cwd, + version: 'test', + message: { role: 'user', parts: [{ text: '@qc-helper' }] }, + }, + { + uuid: 'm1', + parentUuid: 'u1', + sessionId, + timestamp: '2026-04-30T08:02:53.927Z', + type: 'system', + subtype: 'user_text_elements', + cwd, + version: 'test', + systemPayload: { + content: '@qc-helper', + textElements: [ + { + type: 'skill', + byte_range: { start: 0, end: 10 }, + placeholder: '@qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ], + }, + }, + ]); + + const agent = createAgent(cwd); + const messages = ( + agent as unknown as QwenHistoryInternals + ).applyQwenTranscriptTextElements( + [ + { + id: 'message-1', + role: 'user', + content: '@qc-helper', + timestamp: Date.parse('2026-04-30T08:02:52.927Z'), + }, + ], + sessionId, + cwd, + ); + agent.destroy(); + + expect(messages[0]?.textElements).toEqual([ + { + type: 'skill', + byte_range: { start: 0, end: 10 }, + placeholder: '@qc-helper', + label: 'qc-helper', + target: 'qc-helper', + }, + ]); + }); + it('formats slash command JSON output as a markdown json block', () => { const runtimeRoot = mkdtempSync(join(tmpdir(), 'qwen-runtime-')); const cwd = mkdtempSync(join(tmpdir(), 'qwen-cwd-')); diff --git a/packages/desktop/packages/shared/src/agent/qwen-agent.ts b/packages/desktop/packages/shared/src/agent/qwen-agent.ts index 0bd90ae1767..1845e9fceed 100644 --- a/packages/desktop/packages/shared/src/agent/qwen-agent.ts +++ b/packages/desktop/packages/shared/src/agent/qwen-agent.ts @@ -7,7 +7,7 @@ */ import { spawn, type ChildProcess } from 'node:child_process'; -import { existsSync, readFileSync, renameSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import { homedir, platform, tmpdir } from 'node:os'; import { isAbsolute, join, resolve } from 'node:path'; import { Readable, Writable } from 'node:stream'; @@ -106,9 +106,23 @@ const QWEN_TOOL_RESULT_MISSING_MESSAGE = 'Tool result was not recorded.'; const MAX_MID_TURN_CONTENT_BUILD_FAILURES = 3; const MID_TURN_ATTACHMENT_PROCESSING_FAILURE_TEXT = '[Attachment could not be processed]'; +const SESSION_WRITER_ERROR_KINDS = new Set([ + 'session_writer_conflict', + 'session_writer_lost', + 'session_transcript_changed', + 'session_writer_unavailable', +]); function getErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error); + if (error instanceof Error) return error.message; + if ( + error && + typeof error === 'object' && + typeof (error as JsonRecord).message === 'string' + ) { + return (error as JsonRecord).message as string; + } + return String(error); } function getAcpErrorDetail(data: unknown): string | undefined { @@ -145,6 +159,19 @@ export function formatQwenAcpErrorMessage(error: unknown): string { return `${message}: ${detail}`; } +function isSessionWriterAcpError(error: unknown): boolean { + if (!error || typeof error !== 'object') return false; + const record = error as JsonRecord; + const data = + record.data && typeof record.data === 'object' + ? (record.data as JsonRecord) + : undefined; + const errorKind = data?.errorKind ?? record.errorKind; + return ( + typeof errorKind === 'string' && SESSION_WRITER_ERROR_KINDS.has(errorKind) + ); +} + type AcpPermissionOption = { optionId?: string; name?: string; @@ -1648,6 +1675,7 @@ export class QwenAgent extends BaseAgent { private _isProcessing = false; private abortReason?: AbortReason; private persistedQwenSessionId: string | null = null; + private qwenPersistenceCwd: string | null = null; private activePromptRunId: number | null = null; private promptRunCounter = 0; private permissionRequestCounter = 0; @@ -1689,6 +1717,9 @@ export class QwenAgent extends BaseAgent { super(config, config.model || ''); this._supportsBranching = false; this.persistedQwenSessionId = config.session?.sdkSessionId || null; + this.qwenPersistenceCwd = this.persistedQwenSessionId + ? this.resolvedCwd() + : null; this.pendingModeOverride = config.session?.permissionMode && !config.session?.sdkSessionId ? config.session.permissionMode @@ -1714,10 +1745,16 @@ export class QwenAgent extends BaseAgent { } override setSessionId(sessionId: string | null): void { + const previousSessionId = this.qwenSessionId ?? this.persistedQwenSessionId; super.setSessionId(sessionId); if (this.qwenSessionId) this.unregisterAcpSession(this.qwenSessionId); this.qwenSessionId = sessionId; this.persistedQwenSessionId = sessionId; + this.qwenPersistenceCwd = sessionId + ? sessionId === previousSessionId + ? (this.qwenPersistenceCwd ?? this.resolvedCwd()) + : this.resolvedCwd() + : null; if (sessionId) this.registerAcpSession(sessionId); } @@ -1726,6 +1763,7 @@ export class QwenAgent extends BaseAgent { if (this.qwenSessionId) this.unregisterAcpSession(this.qwenSessionId); this.qwenSessionId = null; this.persistedQwenSessionId = null; + this.qwenPersistenceCwd = null; this.pendingAvailableCommandsUpdates.clear(); this.latestAvailableCommandsSnapshot = null; this.resolveAvailableCommandsWaiters(null); @@ -1771,6 +1809,7 @@ export class QwenAgent extends BaseAgent { this.unregisterAcpSession(this.qwenSessionId); this.qwenSessionId = null; this.persistedQwenSessionId = null; + this.qwenPersistenceCwd = null; this.pendingAvailableCommandsUpdates.clear(); this.latestAvailableCommandsSnapshot = null; this.resolveAvailableCommandsWaiters(null); @@ -1820,12 +1859,14 @@ export class QwenAgent extends BaseAgent { try { await this.ensureQwenSession(); } catch (error) { + if (isSessionWriterAcpError(error)) throw error; if (this.persistedQwenSessionId || this.config.session?.sdkSessionId) { this.debug( `Qwen resume failed, starting a fresh session: ${error instanceof Error ? error.message : String(error)}`, ); this.qwenSessionId = null; this.persistedQwenSessionId = null; + this.qwenPersistenceCwd = null; this.config.onSdkSessionIdCleared?.(); const recoveryContext = this.buildRecoveryContext(); if (recoveryContext && !isSlashCommandPrompt(message, attachments)) { @@ -1842,12 +1883,12 @@ export class QwenAgent extends BaseAgent { const prompt = this.buildPromptBlocks(message, attachments); let transcriptTextElementsPersisted = false; - const persistTranscriptTextElements = () => { + const persistTranscriptTextElements = async () => { if (transcriptTextElementsPersisted) return; transcriptTextElementsPersisted = true; - this.persistQwenTranscriptTextElements( + await this.persistQwenTranscriptTextElements( sessionId, - this.resolvedCwd(), + this.qwenPersistenceCwd ?? this.resolvedCwd(), options?.textElements, ); }; @@ -1863,7 +1904,8 @@ export class QwenAgent extends BaseAgent { const stopReason = asString(toRecord(result).stopReason); await this.waitForCurrentTurnUsage(); if (this.activePromptRunId !== promptRunId) return; - persistTranscriptTextElements(); + await persistTranscriptTextElements(); + if (this.activePromptRunId !== promptRunId) return; this.flushThoughtText(); this.flushAssistantText(); this.eventQueue.enqueue({ type: 'complete' }); @@ -1872,15 +1914,17 @@ export class QwenAgent extends BaseAgent { `Qwen prompt complete${stopReason ? ` (${stopReason})` : ''}`, ); }) - .catch((error) => { + .catch(async (error) => { if (this.activePromptRunId !== promptRunId) return; if (this.abortReason) { - persistTranscriptTextElements(); + await persistTranscriptTextElements(); + if (this.activePromptRunId !== promptRunId) return; this.eventQueue.complete(); return; } const message = formatQwenAcpErrorMessage(error); - persistTranscriptTextElements(); + await persistTranscriptTextElements(); + if (this.activePromptRunId !== promptRunId) return; this.eventQueue.enqueue({ type: 'error', message }); this.eventQueue.enqueue({ type: 'complete' }); this.eventQueue.complete(); @@ -2515,7 +2559,8 @@ export class QwenAgent extends BaseAgent { sessionId: string, options: { cwd?: string } = {}, ): Promise { - const cwd = options.cwd || this.resolvedCwd(); + const requestedCwd = options.cwd || this.resolvedCwd(); + const cwd = this.resolveQwenPersistenceCwd(sessionId, requestedCwd); await this.ensureProcess(); const buildResultFromUpdates = ( @@ -2859,7 +2904,10 @@ export class QwenAgent extends BaseAgent { items.push({ content: [ { type: 'text', text: displayText }, - { type: 'text', text: MID_TURN_ATTACHMENT_PROCESSING_FAILURE_TEXT }, + { + type: 'text', + text: MID_TURN_ATTACHMENT_PROCESSING_FAILURE_TEXT, + }, ], displayText, }); @@ -3033,6 +3081,7 @@ export class QwenAgent extends BaseAgent { ); this.qwenSessionId = existingSessionId; this.persistedQwenSessionId = existingSessionId; + this.qwenPersistenceCwd = cwd; this.registerAcpSession(existingSessionId); this.recordSessionModels(result); this.recordSessionModes(result); @@ -3067,6 +3116,7 @@ export class QwenAgent extends BaseAgent { this.qwenSessionId = sessionId; this.persistedQwenSessionId = sessionId; + this.qwenPersistenceCwd = cwd; this.registerAcpSession(sessionId); this.recordSessionModels(result); this.recordSessionModes(result); @@ -3097,7 +3147,10 @@ export class QwenAgent extends BaseAgent { (connection) => connection.loadSession({ sessionId, - cwd: this.resolvedCwd(), + cwd: this.resolveQwenPersistenceCwd( + sessionId, + this.resolvedCwd(), + ), mcpServers: this.buildAcpMcpServers(), }), 60_000, @@ -3308,6 +3361,16 @@ export class QwenAgent extends BaseAgent { ); } + private resolveQwenPersistenceCwd( + sessionId: string, + fallback: string, + ): string { + return sessionId === this.qwenSessionId || + sessionId === this.persistedQwenSessionId + ? (this.qwenPersistenceCwd ?? fallback) + : fallback; + } + private extractQwenRecordText(record: JsonRecord): string { const message = toRecord(record.message); const parts = Array.isArray(message.parts) @@ -3340,12 +3403,15 @@ export class QwenAgent extends BaseAgent { return toRecord(record.systemPayload).phase === 'invocation'; } - private persistQwenTranscriptTextElements( + private async persistQwenTranscriptTextElements( sessionId: string, cwd: string, sourceElements?: MessageTextElement[], - ): void { - const transcriptPath = getQwenTranscriptPath(sessionId, cwd); + ): Promise { + const transcriptPath = getQwenTranscriptPath( + sessionId, + this.resolveQwenPersistenceCwd(sessionId, cwd), + ); if (!existsSync(transcriptPath)) return; let fileContent: string; @@ -3358,7 +3424,6 @@ export class QwenAgent extends BaseAgent { return; } - const hadTrailingNewline = fileContent.endsWith('\n'); const lines = fileContent.split(/\r?\n/); if (lines[lines.length - 1] === '') lines.pop(); @@ -3386,19 +3451,19 @@ export class QwenAgent extends BaseAgent { const next = JSON.stringify(textElements); if (existing === next) return; - record.textElements = textElements; - lines[index] = JSON.stringify(record); - - const tmpPath = `${transcriptPath}.craft-text-elements-${process.pid}-${Date.now()}.tmp`; try { - writeFileSync( - tmpPath, - lines.join('\n') + (hadTrailingNewline ? '\n' : ''), - 'utf8', + await this.callAcp( + 'ext/qwen/session/recordTextElements', + (connection) => + connection.extMethod('qwen/session/recordTextElements', { + sessionId, + content, + textElements, + }), + 30_000, ); - renameSync(tmpPath, transcriptPath); this.debug( - `Wrote ${textElements.length} text element(s) into Qwen transcript ${transcriptPath}`, + `Recorded ${textElements.length} text element(s) for Qwen session ${sessionId}`, ); } catch (error) { this.debug( @@ -3413,7 +3478,10 @@ export class QwenAgent extends BaseAgent { sessionId: string, cwd: string, ): Array<{ content: string; textElements: MessageTextElement[] }> { - const transcriptPath = getQwenTranscriptPath(sessionId, cwd); + const transcriptPath = getQwenTranscriptPath( + sessionId, + this.resolveQwenPersistenceCwd(sessionId, cwd), + ); if (!existsSync(transcriptPath)) return []; let fileContent: string; @@ -3437,11 +3505,26 @@ export class QwenAgent extends BaseAgent { continue; } - if (!this.isPatchableQwenUserRecord(record, sessionId)) continue; - const textElements = toQwenTranscriptTextElements(record.textElements); + const textElementPayload = + record.sessionId === sessionId && + record.type === 'system' && + record.subtype === 'user_text_elements' + ? toRecord(record.systemPayload) + : undefined; + if ( + !textElementPayload && + !this.isPatchableQwenUserRecord(record, sessionId) + ) { + continue; + } + const textElements = toQwenTranscriptTextElements( + textElementPayload?.textElements ?? record.textElements, + ); if (!textElements) continue; - const content = this.getQwenTranscriptPatchContent(record); + const content = textElementPayload + ? asString(textElementPayload.content) || '' + : this.getQwenTranscriptPatchContent(record); if (!content) continue; records.push({ content, textElements }); } @@ -3544,9 +3627,10 @@ export class QwenAgent extends BaseAgent { } const textParts: string[] = []; - const context = includeContext && INCLUDE_CRAFT_CONTEXT_IN_QWEN_PROMPTS - ? this.buildCraftContext() - : ''; + const context = + includeContext && INCLUDE_CRAFT_CONTEXT_IN_QWEN_PROMPTS + ? this.buildCraftContext() + : ''; for (const attachment of attachments ?? []) { if (attachment.mimeType?.startsWith('image/') && attachment.base64) {