diff --git a/docs/design/2026-08-22-session-storage-lifecycle-repair.md b/docs/design/2026-08-22-session-storage-lifecycle-repair.md new file mode 100644 index 00000000000..4721b432737 --- /dev/null +++ b/docs/design/2026-08-22-session-storage-lifecycle-repair.md @@ -0,0 +1,65 @@ +# Session storage lifecycle repair + +## Problem + +Session lifecycle operations currently depend on loading a transcript as a usable conversation. That is too strong for delete, archive, and unarchive. A regular transcript file can still need maintenance when it is empty, has a torn or malformed head, or belongs to a legacy child whose parent no longer exists. + +This leaves physical files stranded even though the selected workspace and archive state are unambiguous. + +## Scope + +This change makes delete, archive, and unarchive operate on exact-spelling regular transcript files owned by the selected workspace. It also adds an explicit repair option for an active/archive conflict. + +The change does not alter transcript loading, listing, export, organization, Live session identity, or general case-insensitive lookup. Normal generated session UUIDs remain canonical lowercase values. + +## Storage classification + +The lifecycle classifier inspects the requested UUID file in the active and archived directories. + +- A missing path contributes no state. +- A regular empty or damaged file contributes physical state. +- A readable first record must identify the requested session and the selected workspace. +- A readable record that identifies another workspace is treated as not found. +- A symlink, directory, or other non-regular entry fails closed. +- Active and archived files together form a conflict. + +The classifier captures file identity for physical-only maintenance. Immediately before unlink or rename, Core verifies that the same regular files still occupy the same paths. + +## Routing + +Workspace-qualified routes mutate only the selected trusted runtime. + +Workspace-less ordinary routes retain their existing primary-workspace behavior. Internal Conversations runtimes may be selected only when the requested batch has one unambiguous owner. Runtime selection is repeated while holding the lifecycle coordinator lock; a changed or unavailable generation returns the existing retryable runtime error instead of mutating stale storage. + +Provenance remains authoritative for loading and listing, but it does not prevent maintenance of an otherwise owned regular transcript. This permits cleanup of legacy orphan children without making them loadable. + +## Conflict repair + +Archive and unarchive keep the non-destructive default behavior: an active/archive conflict is returned as a per-session error and neither persisted copy is moved, removed, or overwritten. Archive still strictly closes a live session before classifying the conflict, so queued records may be flushed to the active transcript. The batch lifecycle routes, including workspace-qualified routes, return that outcome in a `200` response instead of the earlier workspace-qualified `409 session_conflict` envelope. + +Callers may send `resolveConflicts: true`: + +- archive keeps the archived transcript and removes the active transcript; +- unarchive keeps the active transcript and removes the archived transcript. + +The response adds `resolvedConflicts`, containing IDs repaired by that request. Delete keeps its existing compatibility behavior and removes both copies without requiring an option. + +## Writer and generation safety + +Daemon maintenance runs under the per-session lifecycle coordinator and writer lock. Damaged transcripts use the same writer lock with a maintenance sentinel because the normal writer proof intentionally rejects torn JSONL. The sentinel does not bypass a certified handoff: if transcript bytes change after a writer seals its lock, takeover continues to fail closed with `SessionTranscriptChangedError` and requires operator intervention. Core still snapshots and verifies the actual transcript paths before committing the mutation. + +The selected runtime generation is checked after coordinator acquisition, before the filesystem commit, and after the operation. A generation replacement therefore cannot silently redirect an in-flight mutation to another runtime. + +## API compatibility + +The REST and ACP archive/unarchive request bodies accept the optional boolean `resolveConflicts`. Omitting it preserves the non-mutating conflict behavior, while the workspace-qualified REST transport now reports the conflict in the batch result as described above. Non-boolean values are invalid requests. + +Archive and unarchive responses add `resolvedConflicts`. SDK result types keep that field optional so clients remain compatible with older daemons, while new daemons always return the array. Existing SDK client-ID call forms remain valid; options use an additional overload position. + +The `session_storage_conflict_repair` capability advertises support for the option and response field. + +## Verification + +The regression matrix covers empty, damaged, and legacy-orphan transcripts across delete, archive, and unarchive in Core, workspace-qualified internal routes, and unqualified unique-owner routing. Each success asserts the final active/archive file state, and moves preserve the original bytes. + +Compatibility tests retain default archive and unarchive conflict behavior and delete-both behavior. Additional tests cover explicit repair in both directions, foreign or ambiguous ownership rejection, invalid option types, SDK request bodies, generation fencing, and replacement detection. diff --git a/docs/developers/daemon/08-session-lifecycle.md b/docs/developers/daemon/08-session-lifecycle.md index ce9016fa4d5..a78f0a19e92 100644 --- a/docs/developers/daemon/08-session-lifecycle.md +++ b/docs/developers/daemon/08-session-lifecycle.md @@ -244,6 +244,26 @@ only a storage-state transition; clients must call `session/load` or load/resume, and mutations racing an archive transition return `409 session_archiving`. +Empty, damaged, and orphaned regular transcript files remain eligible for these +lifecycle operations even when they cannot be loaded as conversations. +Ownership-safety checks can intentionally fail closed and require operator +intervention. A file changed after a writer sealed its certified handoff proof +fails with `SessionTranscriptChangedError` until the operator resolves the +sealed lock and changed bytes. A JSON-shaped first physical record that exceeds +the bounded ownership-read window fails with +`SessionTranscriptIdentityUnavailableError` until the record is repaired or +reduced; oversized damaged records with a non-object prefix remain eligible. A +parseable recovered record must contain string `sessionId` and `cwd` ownership +fields, and mixed local/foreign archive states also fail closed. When +`session_storage_conflict_repair` is advertised, archive and unarchive accept +`resolveConflicts: true`: archive keeps the archived copy, while unarchive keeps +the active copy. Without that option, active/archive conflicts do not move, +remove, or overwrite either persisted copy and are returned in the batch +`errors` array. Archive still strictly closes a live session before classifying +the conflict, which may flush queued records to the active transcript. +Workspace-qualified lifecycle routes now use that HTTP `200` batch envelope +instead of their earlier HTTP `409 session_conflict` response. + ### Context Usage (`session_context_usage` capability tag) `GET /session/:id/context-usage` returns structured context-window usage. diff --git a/docs/developers/daemon/11-capabilities-versioning.md b/docs/developers/daemon/11-capabilities-versioning.md index 1a2e3cd5249..1f792404d1e 100644 --- a/docs/developers/daemon/11-capabilities-versioning.md +++ b/docs/developers/daemon/11-capabilities-versioning.md @@ -108,7 +108,7 @@ Baseline tags are not present in the `Map` and are advertised unconditionally. T Foundation: `health`, `daemon_status`, `capabilities`. -Sessions: `session_create`, `session_id_override`, `session_scope_override`, `session_load`, `session_resume`, `unstable_session_resume`, `session_list`, `session_info`, `session_prompt`, `session_mid_turn_message_mutation`, `session_cancel`, `session_events`, `session_set_model`, `session_close`, `session_metadata`, `session_archive`, `session_export`, `session_transcript`, `session_context`, `session_context_usage`, `session_supported_commands`, `session_tasks`, `session_monitor_tool_correlation`, `session_stats`, `session_lsp`, `session_status`, `session_approval_mode_control`, `session_recap`, `session_btw`, **`session_shell_command`** (conditional), `session_language`, `session_rewind`, `session_hooks`, `session_branch`. +Sessions: `session_create`, `session_id_override`, `session_scope_override`, `session_load`, `session_resume`, `unstable_session_resume`, `session_list`, `session_info`, `session_prompt`, `session_mid_turn_message_mutation`, `session_cancel`, `session_events`, `session_set_model`, `session_close`, `session_metadata`, `session_archive`, `session_storage_conflict_repair`, `session_export`, `session_transcript`, `session_context`, `session_context_usage`, `session_supported_commands`, `session_tasks`, `session_monitor_tool_correlation`, `session_stats`, `session_lsp`, `session_status`, `session_approval_mode_control`, `session_recap`, `session_btw`, **`session_shell_command`** (conditional), `session_language`, `session_rewind`, `session_hooks`, `session_branch`. Streaming: `slow_client_warning`, `typed_event_schema`. diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 50d7011a777..d786d3ae7b8 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -256,7 +256,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design `session_organization` advertises custom session groups and pinning. It adds `GET/POST/PATCH/DELETE /workspace/:id/session-groups`, `PATCH /session/:id/organization`, and the opt-in organized list view `GET /workspace/:id/sessions?view=organized`. When both `session_organization` and `workspace_qualified_rest_core` are advertised, the workspace-qualified organization mutation `PATCH /workspaces/:workspace/session/:id/organization` is also available. The legacy mutation remains primary-workspace-only. Older daemons return `404` for the mutation/group routes and ignore the organized view contract, so WebShell/SDK clients must pre-flight these tags before showing the matching grouping or pinning UI. -`session_archive` advertises the v1 directory-state archive API: `POST /sessions/archive`, `POST /sessions/unarchive`, and `GET /workspace/:id/sessions?archiveState=active|archived`. Archived sessions cannot be loaded or resumed until they are unarchived. +`session_archive` advertises the v1 directory-state archive API: `POST /sessions/archive`, `POST /sessions/unarchive`, and `GET /workspace/:id/sessions?archiveState=active|archived`. Archived sessions cannot be loaded or resumed until they are unarchived. `session_storage_conflict_repair` advertises the additive `resolveConflicts` request option and `resolvedConflicts` response bucket described below. `workspace_qualified_rest_core` advertises plural core REST routes under `/workspaces/:workspace/...`. The selector resolves as exact workspace id first, then as a URL-encoded absolute cwd after canonicalization. Newer single-workspace daemons include the primary runtime in `workspaces[]` even when `multi_workspace_sessions` is absent, allowing clients to discover the id required by workspace-qualified routes; clients should fall back to `capabilities.workspaceCwd` for older daemons that omit the array. Trust status and trust request routes are available for registered untrusted workspaces; file read routes follow the existing filesystem read policy. Registered untrusted secondary workspaces also expose persisted-only session and session-group catalogs: these reads do not attach to a session, start ACP, or merge live bridge state. File writes, catalog mutations, and other plural core routes require a trusted workspace unless a separate capability explicitly defines a narrower read-only policy, such as `workspace_persisted_transcript`. An untrusted primary continues to receive `403 { code: "untrusted_workspace" }` from the plural catalog and transcript routes; legacy singular primary routes keep their existing compatibility behavior. This tag covers the core file, status, settings, permissions, trust, lifecycle, MCP control, tool and skill toggles, memory, workspace agent CRUD, and session storage surfaces. It does not cover auth, voice, extensions, ACP/WebSocket transport, channel-worker routing, or workspace-qualified session export; pre-flight `workspace_session_export` or `workspace_archived_session_export` separately. Workspace trust is not an ACL: a client holding the daemon token can read every registered workspace surface allowed by this policy. @@ -2434,7 +2434,7 @@ Archive one or more sessions. Archive is a state transition, not deletion: the J Request: ```json -{ "sessionIds": [""] } +{ "sessionIds": [""], "resolveConflicts": true } ``` `sessionIds` must be a non-empty string array with at most 100 ids. Duplicates are collapsed. @@ -2445,12 +2445,15 @@ Response: { "archived": [""], "alreadyArchived": [], + "resolvedConflicts": [""], "notFound": [], "errors": [] } ``` -`errors` entries have `{ "sessionId": "", "error": "message" }`. Active and archived files with the same id are treated as a conflict and reported in `errors`; no file is overwritten. +`resolveConflicts` is optional and defaults to `false`. By default, active and archived files with the same id are reported in `errors`, and neither copy is moved, removed, or overwritten. Archiving a live session still performs the strict close described above before classifying the conflict, so that close may flush queued records to the active transcript. With `resolveConflicts: true`, archive keeps the archived copy, removes the active copy, and reports the id in both `archived` and `resolvedConflicts`. `errors` entries have `{ "sessionId": "", "error": "message" }`. + +Lifecycle conflicts are batch item outcomes: the workspace-less and workspace-qualified routes return HTTP `200` with the conflict in `errors`. This replaces the earlier workspace-qualified HTTP `409 session_conflict` envelope; clients that called that route must inspect the batch response. Internal-runtime REST batches preserve the safe conflict message while continuing to redact other per-session failure details. ### `POST /sessions/unarchive` @@ -2459,7 +2462,7 @@ Restore archived sessions to the active directory. This does not resume the sess Request: ```json -{ "sessionIds": [""] } +{ "sessionIds": [""], "resolveConflicts": true } ``` Response: @@ -2468,12 +2471,13 @@ Response: { "unarchived": [""], "alreadyActive": [], + "resolvedConflicts": [""], "notFound": [], "errors": [] } ``` -If an active JSONL already exists for the id, unarchive reports a conflict in `errors` and does not overwrite it. Archive or unarchive in flight for the same id returns `409 session_archiving` before starting the batch. +`resolveConflicts` is optional and defaults to `false`. By default, simultaneous active and archived JSONL files produce a conflict in `errors`, and neither copy is moved, removed, or overwritten; an active-only session is returned in `alreadyActive`. With `resolveConflicts: true`, unarchive keeps the active copy, removes the archived copy, and reports the id in both `unarchived` and `resolvedConflicts`. Archive or unarchive in flight for the same id returns `409 session_archiving` before starting the batch. ACP-over-HTTP uses the same request and response bodies through vendor methods `_qwen/sessions/archive` and `_qwen/sessions/unarchive`. The REST route table maps `POST /sessions/archive` and `POST /sessions/unarchive` to those methods for ACP transports. diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index bfe0f385af0..084c1fe1a48 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -353,6 +353,7 @@ describe('qwen serve — capabilities envelope', () => { 'session_status', 'session_close', 'session_archive', + 'session_storage_conflict_repair', 'session_metadata', 'session_organization', 'session_export', diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index c21d48af7df..b01835a5aa1 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -11162,11 +11162,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return await entry.attachments.remove(attachmentId); }, - async deleteSessionAttachments(sessionId) { + async deleteSessionAttachments(sessionId, options) { const store = byId.get(sessionId)?.attachments ?? new SessionAttachmentStore(opts.sessionAttachmentsRoot, sessionId); - await store.delete(); + await store.delete(options); }, removePendingPrompt(sessionId, promptId, context) { diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index b43939ec37f..f7af4b1da43 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -1952,7 +1952,10 @@ export interface AcpSessionBridge { ): Promise; /** Delete all persisted attachments after the session itself is deleted. */ - deleteSessionAttachments(sessionId: string): Promise; + deleteSessionAttachments( + sessionId: string, + options?: { assertCanCommit?: () => void }, + ): Promise; /** Remove a queued or promoted mid-turn message. */ removeMidTurnMessage( diff --git a/packages/acp-bridge/src/sessionAttachments.test.ts b/packages/acp-bridge/src/sessionAttachments.test.ts index 3cba87c64c2..5fec06a73dc 100644 --- a/packages/acp-bridge/src/sessionAttachments.test.ts +++ b/packages/acp-bridge/src/sessionAttachments.test.ts @@ -763,6 +763,64 @@ describe('SessionAttachmentStore', () => { } }); + it('checks the runtime generation before deleting persisted attachments', async () => { + const root = await fs.mkdtemp( + path.join(tmpdir(), 'qwen-attachment-fence-'), + ); + const store = new SessionAttachmentStore(root, 'session-a'); + const reference = await store.putAttachment( + Uint8Array.of(1), + 'application/octet-stream', + 'kept.bin', + ); + const generationClosed = new Error('generation closed'); + + await expect( + store.delete({ + assertCanCommit: () => { + throw generationClosed; + }, + }), + ).rejects.toBe(generationClosed); + await expect( + fs.stat(path.join(root, 'session-session-a', reference.attachmentId)), + ).resolves.toBeDefined(); + await fs.rm(root, { recursive: true, force: true }); + }); + + it('deletes a claimed tombstone without touching a successor directory', async () => { + const root = await fs.mkdtemp( + path.join(tmpdir(), 'qwen-attachment-successor-'), + ); + const store = new SessionAttachmentStore(root, 'session-a'); + const reference = await store.putAttachment( + Uint8Array.of(1), + 'application/octet-stream', + 'old.bin', + ); + const directory = path.join(root, 'session-session-a'); + const remove = fs.rm.bind(fs); + const rmSpy = vi.spyOn(fs, 'rm').mockImplementationOnce(async (target) => { + expect(target).not.toBe(directory); + await fs.mkdir(directory, { recursive: true }); + await fs.writeFile(path.join(directory, 'new.bin'), Uint8Array.of(2)); + await remove(target, { recursive: true, force: true }); + }); + + try { + await store.delete(); + await expect( + fs.stat(path.join(directory, 'new.bin')), + ).resolves.toBeDefined(); + await expect( + fs.stat(path.join(directory, reference.attachmentId)), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + rmSpy.mockRestore(); + await fs.rm(root, { recursive: true, force: true }); + } + }); + it('forgets attachments whose backing file disappeared', async () => { const root = await fs.mkdtemp(path.join(tmpdir(), 'qwen-attachment-gone-')); const store = new SessionAttachmentStore(root, 'session-a'); diff --git a/packages/acp-bridge/src/sessionAttachments.ts b/packages/acp-bridge/src/sessionAttachments.ts index 428af956371..4ff8500ed69 100644 --- a/packages/acp-bridge/src/sessionAttachments.ts +++ b/packages/acp-bridge/src/sessionAttachments.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { randomUUID } from 'node:crypto'; import { promises as fs, statSync } from 'node:fs'; import { tmpdir } from 'node:os'; import * as path from 'node:path'; @@ -555,9 +556,11 @@ export class SessionAttachmentStore { await fs.rm(directory, { recursive: true, force: true }); } - async delete(): Promise { + async delete(options: { assertCanCommit?: () => void } = {}): Promise { + options.assertCanCommit?.(); this.closing = true; await this.waitForCopy(); + options.assertCanCommit?.(); if (!this.closed) { this.closed = true; this.pendingItems = 0; @@ -567,7 +570,20 @@ export class SessionAttachmentStore { const directory = this.persistentDirectory ?? (await this.directoryPromise?.catch(() => undefined)); - if (directory) await fs.rm(directory, { recursive: true, force: true }); + if (directory) { + options.assertCanCommit?.(); + const tombstone = path.join( + path.dirname(directory), + `.${path.basename(directory)}.deleting-${randomUUID()}`, + ); + try { + await fs.rename(directory, tombstone); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; + throw error; + } + await fs.rm(tombstone, { recursive: true, force: true }); + } } private assertStored(reference: SessionAttachmentReference): void { diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index cd5b4759034..3637fb3978e 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -342,6 +342,9 @@ const TRUSTED_WORKSPACE_METHODS = new Set([ `${QWEN_METHOD_NS}workspace/mcp/resources`, `${QWEN_METHOD_NS}workspace/mcp/servers/add`, `${QWEN_METHOD_NS}workspace/mcp/servers/remove`, + `${QWEN_METHOD_NS}sessions/delete`, + `${QWEN_METHOD_NS}sessions/archive`, + `${QWEN_METHOD_NS}sessions/unarchive`, `${QWEN_METHOD_NS}workspace/agents/list`, `${QWEN_METHOD_NS}workspace/agents/get`, `${QWEN_METHOD_NS}workspace/agents/create`, @@ -372,6 +375,9 @@ const WORKSPACE_GENERATION_MUTATION_METHODS = new Set([ `${QWEN_METHOD_NS}file/edit`, `${QWEN_METHOD_NS}workspace/mcp/servers/add`, `${QWEN_METHOD_NS}workspace/mcp/servers/remove`, + `${QWEN_METHOD_NS}sessions/delete`, + `${QWEN_METHOD_NS}sessions/archive`, + `${QWEN_METHOD_NS}sessions/unarchive`, `${QWEN_METHOD_NS}workspace/agents/create`, `${QWEN_METHOD_NS}workspace/agents/update`, `${QWEN_METHOD_NS}workspace/agents/delete`, @@ -1211,6 +1217,15 @@ export class AcpDispatcher { return [...new Set(sessionIds as string[])]; } + private parseResolveConflicts(params: Record): boolean { + const value = params['resolveConflicts']; + if (value === undefined) return false; + if (typeof value !== 'boolean') { + throw new AcpParamError('`resolveConflicts` must be a boolean'); + } + return value; + } + private rejectActiveLiveSessionMutation( conn: AcpConnection, id: JsonRpcId | undefined, @@ -4630,6 +4645,7 @@ export class AcpDispatcher { service: svc, bridge: this.bridge, coordinator: this.archiveCoordinator, + assertCanMutate: assertGenerationOpen, onError: ({ phase, sessionId, error }) => { const safeSessionId = logSafe(sessionId.slice(0, 8)); const safeMessage = logSafe(error); @@ -4639,12 +4655,14 @@ export class AcpDispatcher { }, }), ); + assertGenerationOpen?.(); this.replyConn(conn, id, result as unknown); return; } case `${QWEN_METHOD_NS}sessions/archive`: { const ids = this.parseSessionIds(params); + const resolveConflicts = this.parseResolveConflicts(params); if (this.rejectActiveLiveSessionMutation(conn, id, ids)) return; const svc = new SessionService(this.boundWorkspace, { onWarning: logSessionArchiveWarning, @@ -4657,11 +4675,15 @@ export class AcpDispatcher { service: svc, bridge: this.bridge, coordinator: this.archiveCoordinator, + resolveConflicts, + assertCanMutate: assertGenerationOpen, }), ); + assertGenerationOpen?.(); this.replyConn(conn, id, { archived: result.archived, alreadyArchived: result.alreadyArchived, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: this.serializeSessionErrors(result.errors), } as unknown); @@ -4670,6 +4692,7 @@ export class AcpDispatcher { case `${QWEN_METHOD_NS}sessions/unarchive`: { const ids = this.parseSessionIds(params); + const resolveConflicts = this.parseResolveConflicts(params); const svc = new SessionService(this.boundWorkspace, { onWarning: logSessionArchiveWarning, }); @@ -4680,11 +4703,15 @@ export class AcpDispatcher { sessionIds: ids, service: svc, coordinator: this.archiveCoordinator, + resolveConflicts, + assertCanMutate: assertGenerationOpen, }), ); + assertGenerationOpen?.(); this.replyConn(conn, id, { unarchived: result.unarchived, alreadyActive: result.alreadyActive, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: this.serializeSessionErrors(result.errors), } as unknown); diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index 80c1b28390c..b611ec6cf0c 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -3691,6 +3691,27 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { expect(frame.error.message).toContain('sessionIds'); }); + it('_qwen/sessions/archive rejects a non-boolean conflict repair option', async () => { + 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: 141, + method: '_qwen/sessions/archive', + params: { sessionIds: ['session-1'], resolveConflicts: 'yes' }, + }); + + const [frame] = (await got) as Array<{ + id: number; + error: { code: number; message: string }; + }>; + expect(frame.id).toBe(141); + expect(frame.error.code).toBe(-32602); + expect(frame.error.message).toContain('resolveConflicts'); + }); + it('_qwen/sessions/unarchive returns per-id result buckets', async () => { const sessionId = '22222222-bbbb-cccc-dddd-eeeeeeeeeeee'; const connId = await initialize(); @@ -3709,6 +3730,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { result: { unarchived: string[]; alreadyActive: string[]; + resolvedConflicts: string[]; notFound: string[]; errors: unknown[]; }; @@ -3717,11 +3739,55 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { expect(frame.result).toEqual({ unarchived: [], alreadyActive: [], + resolvedConflicts: [], notFound: [sessionId], errors: [], }); }); + it('_qwen/sessions archive and unarchive repair conflicts only when requested', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440142'; + await writeStoredSession(sessionId, 'active'); + await writeStoredSession(sessionId, 'archived'); + const connId = await initialize(); + const connStream = await openStream(connId); + const reader = frameReader(connStream); + + await post(connId, { + jsonrpc: '2.0', + id: 142, + method: '_qwen/sessions/archive', + params: { sessionIds: [sessionId], resolveConflicts: true }, + }); + expect(await reader.next()).toMatchObject({ + id: 142, + result: { + archived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }, + }); + + await writeStoredSession(sessionId, 'active'); + await post(connId, { + jsonrpc: '2.0', + id: 143, + method: '_qwen/sessions/unarchive', + params: { sessionIds: [sessionId], resolveConflicts: true }, + }); + expect(await reader.next()).toMatchObject({ + id: 143, + result: { + unarchived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }, + }); + reader.close(); + }); + }); + it('unknown method → JSON-RPC method-not-found on conn stream', async () => { const connId = await initialize(); const connStream = await openStream(connId); @@ -6840,7 +6906,12 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { release(); // spawn resolves AFTER destroy await vi.waitFor(() => { expect(bridge.killed).toContain(sessionId); - expect(removeSession).toHaveBeenCalledWith(sessionId); + expect(removeSession).toHaveBeenCalledWith( + sessionId, + expect.objectContaining({ + assertStorageUnchanged: expect.any(Function), + }), + ); }); removeSession.mockRestore(); }); @@ -8826,6 +8897,60 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }); }); + it.each([ + '_qwen/sessions/delete', + '_qwen/sessions/archive', + '_qwen/sessions/unarchive', + ])('gates lifecycle method %s by trust and generation', async (method) => { + await restartServer({ primaryTrusted: false }); + const untrustedConnId = await initialize(); + const untrustedStream = await openStream(untrustedConnId); + const untrustedFrames = takeFrames(untrustedStream, 1); + await new Promise((resolve) => setTimeout(resolve, 30)); + await post(untrustedConnId, { + jsonrpc: '2.0', + id: 594, + method, + params: { sessionIds: ['session-1'] }, + }); + await expect(untrustedFrames).resolves.toEqual([ + expect.objectContaining({ + id: 594, + error: expect.objectContaining({ + data: expect.objectContaining({ + errorKind: 'untrusted_workspace', + httpStatus: 403, + }), + }), + }), + ]); + + const generationGuard = createWorkspaceGenerationGuard(); + await restartServer({ generationGuard }); + const closedConnId = await initialize(); + const closedStream = await openStream(closedConnId); + const closedFrames = takeFrames(closedStream, 1); + await new Promise((resolve) => setTimeout(resolve, 30)); + generationGuard.close(); + await post(closedConnId, { + jsonrpc: '2.0', + id: 595, + method, + params: { sessionIds: ['session-1'] }, + }); + await expect(closedFrames).resolves.toEqual([ + expect.objectContaining({ + id: 595, + error: expect.objectContaining({ + data: expect.objectContaining({ + errorKind: 'workspace_runtime_unavailable', + httpStatus: 503, + }), + }), + }), + ]); + }); + it.each([ [ '_qwen/workspace/session_groups/create', @@ -9675,7 +9800,12 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { errors: [{ sessionId, error: removeError }], }, }); - expect(removeSessionSpy).toHaveBeenCalledWith(sessionId); + expect(removeSessionSpy).toHaveBeenCalledWith( + sessionId, + expect.objectContaining({ + assertStorageUnchanged: expect.any(Function), + }), + ); const deleteLog = stdioMocks.writeStderrLine.mock.calls .map(([line]) => line) diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 46044d94f72..d4b3460a3b6 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -118,6 +118,7 @@ export const SERVE_CAPABILITY_REGISTRY = { session_status: { since: 'v1' }, session_close: { since: 'v1' }, session_archive: { since: 'v1' }, + session_storage_conflict_repair: { since: 'v1' }, session_metadata: { since: 'v1' }, session_organization: { since: 'v1' }, session_export: { since: 'v1' }, diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index 685830c9391..133da32e04d 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -12,6 +12,7 @@ import request from 'supertest'; import { SESSION_TRANSCRIPT_MAX_INDEX_BYTES, SessionService, + SessionTranscriptChangedError, Storage, createDebugLogger, readSessionPrs, @@ -47,6 +48,7 @@ import { serializeWorkspaceTranscriptResponseForTesting, workspaceTranscriptCursorExceedsLimitForTesting, } from './routes/session.js'; +import { SessionArchiveCoordinator } from './server/session-archive.js'; const PRIMARY_CWD = path.resolve(path.sep, 'work', 'primary'); const SECONDARY_CWD = path.resolve(path.sep, 'work', 'secondary'); @@ -278,6 +280,53 @@ async function archiveStoredSession( ); } +async function writeLifecycleFixture(input: { + sessionId: string; + shape: 'empty' | 'damaged' | 'orphan'; + state: 'active' | 'archived'; +}): Promise<{ + activePath: string; + archivedPath: string; + contents: Buffer; +}> { + const chatsDir = path.join( + new Storage(SECONDARY_CWD).getProjectDir(), + 'chats', + ); + const activePath = path.join(chatsDir, `${input.sessionId}.jsonl`); + const archivedPath = path.join( + chatsDir, + 'archive', + `${input.sessionId}.jsonl`, + ); + if (input.shape === 'orphan') { + await writeStoredSession({ + sessionId: input.sessionId, + cwd: SECONDARY_CWD, + timestamp: '2026-07-08T00:00:00.000Z', + prompt: 'orphan lifecycle fixture', + mtime: new Date('2026-07-08T00:00:00.000Z'), + parentSessionId: '00000000-0000-4000-8000-000000000000', + }); + } else { + await fsp.mkdir(chatsDir, { recursive: true }); + await fsp.writeFile( + activePath, + input.shape === 'empty' ? '' : '{"uuid":"torn-head"', + ); + } + if (input.state === 'archived') { + await archiveStoredSession(SECONDARY_CWD, input.sessionId); + } + return { + activePath, + archivedPath, + contents: await fsp.readFile( + input.state === 'active' ? activePath : archivedPath, + ), + }; +} + async function withRuntimeDir(fn: () => Promise): Promise { const previousRuntimeDir = process.env['QWEN_RUNTIME_DIR']; const runtimeDir = await fsp.mkdtemp( @@ -2085,11 +2134,368 @@ describe('multi-workspace session dispatch', () => { expect(exported.status).toBe(200); expect(exported.text).toContain('active internal copy'); expect(exported.text).not.toContain('archived internal copy'); - expect(archive.status).toBe(409); - expect(archive.body.code).toBe('session_conflict'); + expect(archive.status).toBe(200); + expect(archive.body).toMatchObject({ + archived: [], + alreadyArchived: [], + resolvedConflicts: [], + notFound: [], + errors: [ + { + sessionId, + error: `Session "${sessionId}" exists in both active and archived directories. Retry with resolveConflicts: true to keep one copy.`, + }, + ], + }); }); }); + it.each([ + ['empty', 'delete', '201'], + ['empty', 'archive', '202'], + ['empty', 'unarchive', '203'], + ['damaged', 'delete', '204'], + ['damaged', 'archive', '205'], + ['damaged', 'unarchive', '206'], + ['orphan', 'delete', '207'], + ['orphan', 'archive', '208'], + ['orphan', 'unarchive', '209'], + ] as const)( + 'maintains %s transcripts through %s in qualified and owner-routed batches', + async (shape, action, suffix) => { + await withRuntimeDir(async () => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const qualifiedId = `550e8400-e29b-41d4-a716-446655440${suffix}`; + const unqualifiedId = `550e8400-e29b-41d4-a716-446655441${suffix}`; + const qualifiedFixture = await writeLifecycleFixture({ + sessionId: qualifiedId, + shape, + state, + }); + const unqualifiedFixture = await writeLifecycleFixture({ + sessionId: unqualifiedId, + shape, + state, + }); + const { app } = makeHarness({ + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + secondarySummaries: [], + }); + + const qualified = await request(app) + .post(`/workspaces/secondary-id/sessions/${action}`) + .set('Host', host()) + .send({ sessionIds: [qualifiedId] }); + const unqualified = await request(app) + .post(`/sessions/${action}`) + .set('Host', host()) + .send({ sessionIds: [unqualifiedId] }); + + expect(qualified.status).toBe(200); + expect(unqualified.status).toBe(200); + const successKey = + action === 'delete' + ? 'removed' + : action === 'archive' + ? 'archived' + : 'unarchived'; + expect(qualified.body).toMatchObject({ + [successKey]: [qualifiedId], + notFound: [], + errors: [], + }); + expect(unqualified.body).toMatchObject({ + [successKey]: [unqualifiedId], + notFound: [], + errors: [], + }); + + for (const [sessionId, fixture] of [ + [qualifiedId, qualifiedFixture], + [unqualifiedId, unqualifiedFixture], + ] as const) { + if (action === 'delete') { + await expect(fsp.stat(fixture.activePath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + await expect(fsp.stat(fixture.archivedPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + continue; + } + const targetPath = + action === 'archive' ? fixture.archivedPath : fixture.activePath; + const sourcePath = + action === 'archive' ? fixture.activePath : fixture.archivedPath; + await expect(fsp.readFile(targetPath)).resolves.toEqual( + fixture.contents, + ); + await expect(fsp.stat(sourcePath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + expect(path.basename(targetPath)).toBe(`${sessionId}.jsonl`); + } + }); + }, + ); + + it.each([ + ['qualified', '/workspaces/secondary-id/sessions/archive'], + ['owner-routed', '/sessions/archive'], + ])( + 'keeps non-regular lifecycle failures scoped to their %s batch item', + async (_kind, route) => { + await withRuntimeDir(async () => { + const healthyId = '550e8400-e29b-41d4-a716-446655440211'; + const invalidId = '550e8400-e29b-41d4-a716-446655440212'; + const healthy = await writeLifecycleFixture({ + sessionId: healthyId, + shape: 'orphan', + state: 'active', + }); + const invalidPath = path.join( + path.dirname(healthy.activePath), + `${invalidId}.jsonl`, + ); + await fsp.mkdir(invalidPath); + const { app } = makeHarness({ + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + secondarySummaries: [], + }); + + const response = await request(app) + .post(route) + .set('Host', host()) + .send({ sessionIds: [healthyId, invalidId] }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + archived: [healthyId], + notFound: [], + errors: [ + { + sessionId: invalidId, + error: 'Session operation failed.', + }, + ], + }); + await expect(fsp.stat(healthy.activePath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + await expect(fsp.readFile(healthy.archivedPath)).resolves.toEqual( + healthy.contents, + ); + expect((await fsp.lstat(invalidPath)).isDirectory()).toBe(true); + }); + }, + ); + + it.each([ + [ + 'filesystem', + 'qualified', + '/workspaces/secondary-id/sessions/archive', + () => Object.assign(new Error('permission denied'), { code: 'EACCES' }), + ], + [ + 'filesystem', + 'owner-routed', + '/sessions/archive', + () => Object.assign(new Error('permission denied'), { code: 'EACCES' }), + ], + [ + 'transcript-change', + 'qualified', + '/workspaces/secondary-id/sessions/archive', + () => new SessionTranscriptChangedError(), + ], + [ + 'transcript-change', + 'owner-routed', + '/sessions/archive', + () => new SessionTranscriptChangedError(), + ], + ])( + 'keeps %s classifier failures scoped to their %s batch item', + async (_failureKind, _routeKind, route, createError) => { + await withRuntimeDir(async () => { + const healthyId = '550e8400-e29b-41d4-a716-446655440217'; + const invalidId = '550e8400-e29b-41d4-a716-446655440218'; + const healthy = await writeLifecycleFixture({ + sessionId: healthyId, + shape: 'orphan', + state: 'active', + }); + const invalid = await writeLifecycleFixture({ + sessionId: invalidId, + shape: 'orphan', + state: 'active', + }); + const originalClassifier = + SessionService.prototype.getMaintainableSessionLocation; + const classifier = vi + .spyOn(SessionService.prototype, 'getMaintainableSessionLocation') + .mockImplementation(async function (this: SessionService, sessionId) { + if ( + this.getProjectRoot() === SECONDARY_CWD && + sessionId === invalidId + ) { + throw createError(); + } + return originalClassifier.call(this, sessionId); + }); + const { app } = makeHarness({ + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + secondarySummaries: [], + }); + + const response = await (async () => { + try { + return await request(app) + .post(route) + .set('Host', host()) + .send({ sessionIds: [healthyId, invalidId] }); + } finally { + classifier.mockRestore(); + } + })(); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + archived: [healthyId], + notFound: [], + errors: [ + { + sessionId: invalidId, + error: 'Session operation failed.', + }, + ], + }); + await expect(fsp.stat(healthy.activePath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + await expect(fsp.readFile(healthy.archivedPath)).resolves.toEqual( + healthy.contents, + ); + await expect(fsp.readFile(invalid.activePath)).resolves.toEqual( + invalid.contents, + ); + await expect(fsp.stat(invalid.archivedPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + }, + ); + + it('does not fall back to primary for mixed foreign and local internal storage', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440215'; + const fixture = await writeLifecycleFixture({ + sessionId, + shape: 'orphan', + state: 'archived', + }); + const foreignActive = `${JSON.stringify({ + sessionId, + cwd: PRIMARY_CWD, + uuid: 'foreign-u1', + parentUuid: null, + timestamp: '2026-07-08T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'foreign' }] }, + })}\n`; + await fsp.writeFile(fixture.activePath, foreignActive); + const { app, primaryBridge, secondaryBridge } = makeHarness({ + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + secondarySummaries: [], + }); + + const response = await request(app) + .post('/sessions/archive') + .set('Host', host()) + .send({ sessionIds: [sessionId], resolveConflicts: true }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + archived: [], + notFound: [], + errors: [ + { + sessionId, + error: 'Session operation failed.', + }, + ], + }); + expect(primaryBridge.closeCalls).toEqual([]); + expect(secondaryBridge.closeCalls).toEqual([sessionId]); + await expect(fsp.readFile(fixture.activePath, 'utf8')).resolves.toBe( + foreignActive, + ); + await expect(fsp.readFile(fixture.archivedPath)).resolves.toEqual( + fixture.contents, + ); + }); + }); + + it.each([ + ['qualified', '/workspaces/secondary-id/sessions/archive'], + ['owner-routed', '/sessions/archive'], + ])( + 'keeps indeterminate lifecycle ownership scoped to its %s batch item', + async (_kind, route) => { + await withRuntimeDir(async () => { + const healthyId = '550e8400-e29b-41d4-a716-446655440213'; + const invalidId = '550e8400-e29b-41d4-a716-446655440214'; + const healthy = await writeLifecycleFixture({ + sessionId: healthyId, + shape: 'orphan', + state: 'active', + }); + const invalidPath = path.join( + path.dirname(healthy.activePath), + `${invalidId}.jsonl`, + ); + await fsp.writeFile( + invalidPath, + `{"sessionId":"${invalidId}","filler":"${'x'.repeat(2 * 1024 * 1024)}`, + ); + const { app } = makeHarness({ + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + secondarySummaries: [], + }); + + const response = await request(app) + .post(route) + .set('Host', host()) + .send({ sessionIds: [healthyId, invalidId] }); + + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + archived: [healthyId], + notFound: [], + errors: [ + { + sessionId: invalidId, + error: 'Session operation failed.', + }, + ], + }); + await expect(fsp.stat(healthy.activePath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + await expect(fsp.readFile(healthy.archivedPath)).resolves.toEqual( + healthy.contents, + ); + await expect(fsp.stat(invalidPath)).resolves.toBeDefined(); + }); + }, + ); + it('keeps the private directory canonical when restoring a mixed-case transcript', async () => { const storageSessionId = LIVE_PROJECTLESS_TASK_ID.toUpperCase(); await withStoredProjectlessLiveTasks( @@ -2178,6 +2584,172 @@ describe('multi-workspace session dispatch', () => { }); }); + it('rejects an internal physical-only owner that collides with an ordinary live-only owner', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440111'; + const fixture = await writeLifecycleFixture({ + sessionId, + shape: 'damaged', + state: 'active', + }); + const { app } = makeHarness({ + primarySummaries: [makeSummary(sessionId, PRIMARY_CWD)], + secondarySummaries: [], + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + }); + + const response = await request(app) + .post('/sessions/archive') + .set('Host', host()) + .send({ sessionIds: [sessionId] }); + + expect(response.status).toBe(500); + expect(response.body).toMatchObject({ + code: 'ambiguous_session_owner', + sessionId, + }); + await expect(fsp.readFile(fixture.activePath)).resolves.toEqual( + fixture.contents, + ); + await expect(fsp.stat(fixture.archivedPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + }); + + it('rejects a cross-workspace lifecycle batch independently of id order', async () => { + await withRuntimeDir(async () => { + const ordinaryId = '550e8400-e29b-41d4-a716-446655440113'; + const internalId = '550e8400-e29b-41d4-a716-446655440114'; + const fixture = await writeLifecycleFixture({ + sessionId: internalId, + shape: 'damaged', + state: 'active', + }); + const { app } = makeHarness({ + primarySummaries: [makeSummary(ordinaryId, PRIMARY_CWD)], + secondarySummaries: [], + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + }); + + for (const sessionIds of [ + [ordinaryId, internalId], + [internalId, ordinaryId], + ]) { + const response = await request(app) + .post('/sessions/archive') + .set('Host', host()) + .send({ sessionIds }); + + expect(response.status).toBe(409); + expect(response.body.code).toBe('session_workspace_conflict'); + } + await expect(fsp.readFile(fixture.activePath)).resolves.toEqual( + fixture.contents, + ); + await expect(fsp.stat(fixture.archivedPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + }); + + it('rejects an internal physical-only owner when an ordinary persisted owner is transitioning', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440112'; + await writeStoredSession({ + sessionId, + cwd: PRIMARY_CWD, + timestamp: '2026-07-08T00:00:00.000Z', + prompt: 'transitioning ordinary collision', + mtime: new Date('2026-07-08T00:00:00.000Z'), + }); + const fixture = await writeLifecycleFixture({ + sessionId, + shape: 'damaged', + state: 'active', + }); + const { app, registry } = makeHarness({ + primarySummaries: [], + secondarySummaries: [], + secondaryProvenance: 'live-conversation', + secondaryRuntimeBaseDir: Storage.getRuntimeBaseDir(), + }); + expect(registry.beginReplacement(registry.primaryEntry, 'policy-2')).toBe( + true, + ); + + const response = await request(app) + .post('/sessions/archive') + .set('Host', host()) + .send({ sessionIds: [sessionId] }); + + expect(response.status).toBe(503); + expect(response.body.code).toBe('workspace_runtime_unavailable'); + await expect(fsp.readFile(fixture.activePath)).resolves.toEqual( + fixture.contents, + ); + await expect(fsp.stat(fixture.archivedPath)).rejects.toMatchObject({ + code: 'ENOENT', + }); + }); + }); + + it('rejects an ordinary batch when the primary generation changes under the lifecycle lock', async () => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440210'; + await writeStoredSession({ + sessionId, + cwd: PRIMARY_CWD, + timestamp: '2026-07-08T00:00:00.000Z', + prompt: 'primary generation target', + mtime: new Date('2026-07-08T00:00:00.000Z'), + }); + const { app, registry, primaryBridge } = makeHarness({ + primarySummaries: [], + }); + const replacementBridge = makeBridge(PRIMARY_CWD, []); + const replacement = makeRuntime({ + workspaceId: 'primary-id', + workspaceCwd: PRIMARY_CWD, + primary: true, + trusted: true, + bridge: replacementBridge, + }); + const runExclusive = vi + .spyOn(SessionArchiveCoordinator.prototype, 'runExclusiveMany') + .mockImplementationOnce(async (_ids, run) => { + expect( + registry.beginReplacement(registry.primaryEntry, 'policy-2'), + ).toBe(true); + registry.activateReplacement( + registry.primaryEntry, + replacement, + 'policy-2', + ); + return run(); + }); + + try { + const response = await request(app) + .post('/sessions/archive') + .set('Host', host()) + .send({ sessionIds: [sessionId] }); + + expect(response.status).toBe(503); + expect(response.body.code).toBe('workspace_runtime_unavailable'); + expect(primaryBridge.closeCalls).toEqual([]); + expect(replacementBridge.closeCalls).toEqual([]); + await expect( + new SessionService(PRIMARY_CWD).getSessionLocation(sessionId), + ).resolves.toBe('active'); + } finally { + runExclusive.mockRestore(); + } + }); + }); + it('opens attached active Live workers without waiting for cwd relocation', async () => { await withStoredLiveWorkers( [LIVE_ACTIVE_LOAD_ID, LIVE_ACTIVE_RESUME_ID], diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 826df24ff39..cd8ef0c0e64 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -13,6 +13,9 @@ import { GROUP_COLOR_OPTIONS, GitWorktreeService, SessionOrganizationError, + SessionStorageEntryError, + SessionTranscriptChangedError, + SessionTranscriptIdentityUnavailableError, SESSION_TRANSCRIPT_MAX_LIMIT, SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTES, SESSION_TRANSCRIPT_MAX_PAGE_BYTES, @@ -1144,12 +1147,42 @@ export function registerSessionRoutes( return true; }; + const hasLifecycleStorageEvidence = async ( + service: ReturnType, + sessionId: string, + ): Promise => { + try { + return ( + (await service.getMaintainableSessionLocation(sessionId)) !== undefined + ); + } catch (error) { + if (error instanceof SessionStorageEntryError) { + return error.reason !== 'foreign_project'; + } + if (error instanceof SessionTranscriptIdentityUnavailableError) { + return true; + } + if (error instanceof SessionTranscriptChangedError) { + return true; + } + if ( + error !== null && + typeof error === 'object' && + typeof (error as NodeJS.ErrnoException).code === 'string' + ) { + return true; + } + throw error; + } + }; + const resolveQualifiedSessionRuntime = async ( req: Request, res: Response, route: string, sessionIds: readonly string[], archiveState: SessionArchiveState | 'any', + options: { lifecycleMaintenance?: boolean } = {}, ): Promise => { const target = resolveQualifiedSessionTarget(req, res); if (!target) return undefined; @@ -1164,6 +1197,12 @@ export function registerSessionRoutes( const runtime = generation.runtime; const service = createWorkspaceRuntimeSessionService(runtime); for (const sessionId of sessionIds) { + if (options.lifecycleMaintenance) { + if (!(await hasLifecycleStorageEvidence(service, sessionId))) { + throw new SessionNotFoundError(sessionId); + } + continue; + } const location = await service.getSessionLocation(sessionId); if (location === 'conflict') { if (archiveState !== 'active') { @@ -1974,22 +2013,52 @@ export function registerSessionRoutes( sessionIds: readonly string[], ): Promise => { if (req) { - return resolveQualifiedSessionRuntime(req, res, route, sessionIds, 'any'); + return resolveQualifiedSessionRuntime( + req, + res, + route, + sessionIds, + 'any', + { lifecycleMaintenance: true }, + ); } let internalRuntime: WorkspaceRuntime | undefined; let hasInternalSession = false; - const hasOrdinarySession = async (sessionId: string): Promise => { - for (const runtime of workspaceRegistry.list()) { + let hasOrdinaryLiveSession = false; + const findOrdinarySessions = async ( + sessionId: string, + ): Promise | undefined> => { + const runtimes = new Set(); + for (const entry of workspaceRegistry.listAllEntries()) { + const generation = entry.current; + if (entry.internal || !generation) continue; if ( - await createWorkspaceRuntimeSessionService( - runtime, - ).sessionExistsInAnyState(sessionId) + entry.state !== 'active' || + entry.current !== generation || + generation.guard.closed ) { - return true; + sendWorkspaceRuntimeUnavailable(res); + return undefined; } + generation.guard.assertOpen(); + const exists = await hasLifecycleStorageEvidence( + createWorkspaceRuntimeSessionService(generation.runtime), + sessionId, + ); + if ( + entry.state !== 'active' || + entry.current !== generation || + generation.guard.closed + ) { + sendWorkspaceRuntimeUnavailable(res); + return undefined; + } + if (!exists) continue; + generation.guard.assertOpen(); + runtimes.add(generation.runtime); } - return false; + return runtimes; }; const sendBatchWorkspaceConflict = (): void => { res.status(409).json({ @@ -1999,17 +2068,28 @@ export function registerSessionRoutes( }; for (const sessionId of sessionIds) { const candidates = new Set(); + const ordinaryLiveCandidates = new Set(); const owner = workspaceRegistry.resolveLiveSessionOwner(sessionId); if (owner.kind === 'unavailable') { sendWorkspaceRuntimeUnavailable(res); return undefined; } - if (owner.kind === 'found' && isInternalWorkspaceRuntime(owner.runtime)) { - candidates.add(owner.runtime); + if (owner.kind === 'found') { + if (isInternalWorkspaceRuntime(owner.runtime)) { + candidates.add(owner.runtime); + } else { + ordinaryLiveCandidates.add(owner.runtime); + hasOrdinaryLiveSession = true; + } } if (owner.kind === 'ambiguous') { for (const runtime of owner.runtimes) { - if (isInternalWorkspaceRuntime(runtime)) candidates.add(runtime); + if (isInternalWorkspaceRuntime(runtime)) { + candidates.add(runtime); + } else { + ordinaryLiveCandidates.add(runtime); + hasOrdinaryLiveSession = true; + } } } for (const entry of workspaceRegistry.listAllEntries()) { @@ -2020,32 +2100,32 @@ export function registerSessionRoutes( } const runtime = generation.runtime; const service = createWorkspaceRuntimeSessionService(runtime); - const exists = await service.sessionExistsInAnyState(sessionId); + const exists = await hasLifecycleStorageEvidence(service, sessionId); if (!assertCurrentInternalGeneration(entry, generation, res)) { return undefined; } if (!exists) continue; - const metadata = await readLoadableLiveConversationMetadata( - sessionId, - service, - ); - if (!assertCurrentInternalGeneration(entry, generation, res)) { - return undefined; - } - if (!metadata) continue; candidates.add(runtime); } - if (candidates.size > 0) { - for (const runtime of workspaceRegistry.list()) { - const service = createWorkspaceRuntimeSessionService(runtime); - if (await service.sessionExistsInAnyState(sessionId)) { - candidates.add(runtime); - } + if (candidates.size > 0 || hasInternalSession) { + if (hasOrdinaryLiveSession && ordinaryLiveCandidates.size === 0) { + sendBatchWorkspaceConflict(); + return undefined; + } + for (const runtime of ordinaryLiveCandidates) { + candidates.add(runtime); + } + const ordinarySessions = await findOrdinarySessions(sessionId); + if (!ordinarySessions) return undefined; + for (const runtime of ordinarySessions) { + candidates.add(runtime); } } if (candidates.size === 0) { if (hasInternalSession) { - if (await hasOrdinarySession(sessionId)) { + const ordinarySessions = await findOrdinarySessions(sessionId); + if (!ordinarySessions) return undefined; + if (ordinarySessions.size > 0) { sendBatchWorkspaceConflict(); return undefined; } @@ -2073,20 +2153,15 @@ export function registerSessionRoutes( for (const sessionId of sessionIds) { const service = createWorkspaceRuntimeSessionService(internalRuntime); - if (!(await service.sessionExistsInAnyState(sessionId))) { - if (await hasOrdinarySession(sessionId)) { + if (!(await hasLifecycleStorageEvidence(service, sessionId))) { + const ordinarySessions = await findOrdinarySessions(sessionId); + if (!ordinarySessions) return undefined; + if (ordinarySessions.size > 0) { sendBatchWorkspaceConflict(); return undefined; } throw new SessionNotFoundError(sessionId); } - const metadata = await readLoadableLiveConversationMetadata( - sessionId, - service, - ); - if (!metadata) { - throw new SessionNotFoundError(sessionId); - } } const internalEntry = workspaceRegistry.getManagedEntryByWorkspaceCwd( internalRuntime.workspaceCwd, @@ -2136,6 +2211,20 @@ export function registerSessionRoutes( return [...new Set(sessionIds as string[])]; }; + const parseResolveConflicts = ( + req: Request, + res: Response, + ): boolean | undefined => { + const value: unknown = safeBody(req)['resolveConflicts']; + if (value === undefined) return false; + if (typeof value === 'boolean') return value; + res.status(400).json({ + error: '`resolveConflicts` must be a boolean', + code: 'invalid_request', + }); + return undefined; + }; + // Mirrors the bridge's displayName control-character rule (ESLint forbids // control-char regexes). const hasControlCharacter = (value: string): boolean => @@ -2184,11 +2273,12 @@ export function registerSessionRoutes( ): Array<{ sessionId: string; error: string }> => errors.map((e) => ({ sessionId: e.sessionId, - error: redactDetails - ? 'Session operation failed.' - : e.error instanceof Error - ? e.error.message - : String(e.error), + error: + redactDetails && !(e.error instanceof SessionConflictError) + ? 'Session operation failed.' + : e.error instanceof Error + ? e.error.message + : String(e.error), })); const runResolvedSessionBatch = async (params: { @@ -2224,7 +2314,10 @@ export function registerSessionRoutes( sendWorkspaceRuntimeUnavailable(res); return undefined; } - return { result: await run(verifiedRuntime, true), internal: true }; + return { + result: await run(verifiedRuntime, true), + internal: isInternalWorkspaceRuntime(verifiedRuntime), + }; }); }; @@ -2238,9 +2331,10 @@ export function registerSessionRoutes( runtime: WorkspaceRuntime, coordinatorLockHeld: boolean, ) => { - captureRuntimeGenerationAssertion(runtime)?.(); + const assertCanMutate = captureRuntimeGenerationAssertion(runtime); + assertCanMutate?.(); const service = createWorkspaceRuntimeSessionService(runtime); - return runWithSessionListInvalidation( + const result = await runWithSessionListInvalidation( runtime, ['active', 'archived'], () => @@ -2251,6 +2345,7 @@ export function registerSessionRoutes( bridge: runtime.bridge, coordinator: archiveCoordinator, coordinatorLockHeld, + assertCanMutate, onError: ({ phase, sessionId, error }) => { writeStderrLine( `qwen serve: ${phase}Session failed for ${safeLogValue(sessionId)}: ${safeLogValue(error)}`, @@ -2259,6 +2354,8 @@ export function registerSessionRoutes( }), ), ); + assertCanMutate?.(); + return result; }; return runResolvedSessionBatch({ req, res, route, sessionIds, run }); }; @@ -2268,16 +2365,18 @@ export function registerSessionRoutes( res: Response, route: string, sessionIds: string[], + resolveConflicts = false, ) => { const run = async ( runtime: WorkspaceRuntime, coordinatorLockHeld: boolean, ) => { - captureRuntimeGenerationAssertion(runtime)?.(); + const assertCanMutate = captureRuntimeGenerationAssertion(runtime); + assertCanMutate?.(); const service = createWorkspaceRuntimeSessionService(runtime, { onWarning: logSessionArchiveWarning, }); - return runWithSessionListInvalidation( + const result = await runWithSessionListInvalidation( runtime, ['active', 'archived'], () => @@ -2288,9 +2387,13 @@ export function registerSessionRoutes( bridge: runtime.bridge, coordinator: archiveCoordinator, coordinatorLockHeld, + resolveConflicts, + assertCanMutate, }), ), ); + assertCanMutate?.(); + return result; }; return runResolvedSessionBatch({ req, res, route, sessionIds, run }); }; @@ -2300,16 +2403,18 @@ export function registerSessionRoutes( res: Response, route: string, sessionIds: string[], + resolveConflicts = false, ) => { const run = async ( runtime: WorkspaceRuntime, coordinatorLockHeld: boolean, ) => { - captureRuntimeGenerationAssertion(runtime)?.(); + const assertCanMutate = captureRuntimeGenerationAssertion(runtime); + assertCanMutate?.(); const service = createWorkspaceRuntimeSessionService(runtime, { onWarning: logSessionArchiveWarning, }); - return runWithSessionListInvalidation( + const result = await runWithSessionListInvalidation( runtime, ['active', 'archived'], () => @@ -2319,9 +2424,13 @@ export function registerSessionRoutes( service, coordinator: archiveCoordinator, coordinatorLockHeld, + resolveConflicts, + assertCanMutate, }), ), ); + assertCanMutate?.(); + return result; }; return runResolvedSessionBatch({ req, res, route, sessionIds, run }); }; @@ -5151,6 +5260,8 @@ export function registerSessionRoutes( app.post('/sessions/archive', mutate(), async (req, res) => { const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; + const resolveConflicts = parseResolveConflicts(req, res); + if (resolveConflicts === undefined) return; if (rejectActiveLiveSessionMutation(res, uniqueIds)) return; try { @@ -5159,12 +5270,14 @@ export function registerSessionRoutes( res, 'POST /sessions/archive', uniqueIds, + resolveConflicts, ); if (!operation) return; const { result } = operation; res.status(200).json({ archived: result.archived, alreadyArchived: result.alreadyArchived, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: serializeSessionErrors(result.errors, operation.internal), }); @@ -5176,6 +5289,8 @@ export function registerSessionRoutes( app.post('/sessions/unarchive', mutate(), async (req, res) => { const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; + const resolveConflicts = parseResolveConflicts(req, res); + if (resolveConflicts === undefined) return; try { const operation = await unarchiveSessions( @@ -5183,12 +5298,14 @@ export function registerSessionRoutes( res, 'POST /sessions/unarchive', uniqueIds, + resolveConflicts, ); if (!operation) return; const { result } = operation; res.status(200).json({ unarchived: result.unarchived, alreadyActive: result.alreadyActive, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: serializeSessionErrors(result.errors, operation.internal), }); @@ -5233,14 +5350,23 @@ export function registerSessionRoutes( const route = 'POST /workspaces/:workspace/sessions/archive'; const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; + const resolveConflicts = parseResolveConflicts(req, res); + if (resolveConflicts === undefined) return; if (rejectActiveLiveSessionMutation(res, uniqueIds)) return; try { - const operation = await archiveSessions(req, res, route, uniqueIds); + const operation = await archiveSessions( + req, + res, + route, + uniqueIds, + resolveConflicts, + ); if (!operation) return; const { result } = operation; res.status(200).json({ archived: result.archived, alreadyArchived: result.alreadyArchived, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: serializeSessionErrors(result.errors, operation.internal), }); @@ -5257,13 +5383,22 @@ export function registerSessionRoutes( const route = 'POST /workspaces/:workspace/sessions/unarchive'; const uniqueIds = parseSessionIdsBody(req, res); if (uniqueIds === undefined) return; + const resolveConflicts = parseResolveConflicts(req, res); + if (resolveConflicts === undefined) return; try { - const operation = await unarchiveSessions(req, res, route, uniqueIds); + const operation = await unarchiveSessions( + req, + res, + route, + uniqueIds, + resolveConflicts, + ); if (!operation) return; const { result } = operation; res.status(200).json({ unarchived: result.unarchived, alreadyActive: result.alreadyActive, + resolvedConflicts: result.resolvedConflicts, notFound: result.notFound, errors: serializeSessionErrors(result.errors, operation.internal), }); diff --git a/packages/cli/src/serve/scheduled-task-session-lifecycle.test.ts b/packages/cli/src/serve/scheduled-task-session-lifecycle.test.ts index ba013c37a5c..fa3db989632 100644 --- a/packages/cli/src/serve/scheduled-task-session-lifecycle.test.ts +++ b/packages/cli/src/serve/scheduled-task-session-lifecycle.test.ts @@ -135,4 +135,18 @@ describe('scheduled-task session lifecycle', () => { await enableTasksForSessions(workspace, []); expect(Object.keys(await byId())).toEqual(['a']); }); + + it('checks the runtime generation at the task-store commit point', async () => { + await seed([task({ id: 'a', sessionId: 'sess-1' })]); + const generationClosed = new Error('generation closed'); + + await expect( + disableTasksForSessions(workspace, ['sess-1'], { + assertCanCommit: () => { + throw generationClosed; + }, + }), + ).rejects.toBe(generationClosed); + expect((await byId())['a']!.enabled).toBeUndefined(); + }); }); diff --git a/packages/cli/src/serve/scheduled-task-session-lifecycle.ts b/packages/cli/src/serve/scheduled-task-session-lifecycle.ts index c247d319a1e..db55e51d5ab 100644 --- a/packages/cli/src/serve/scheduled-task-session-lifecycle.ts +++ b/packages/cli/src/serve/scheduled-task-session-lifecycle.ts @@ -45,24 +45,29 @@ function isBoundTask( export async function disableTasksForSessions( projectRoot: string, sessionIds: string[], + options: { assertCanCommit?: () => void } = {}, ): Promise { if (sessionIds.length === 0) return; const targets = new Set(sessionIds); - await updateCronTasks(projectRoot, (tasks) => { - let changed = false; - const next = tasks.map((task) => { - if ( - isBoundTask(task) && - targets.has(task.sessionId) && - task.enabled !== false - ) { - changed = true; - return { ...task, enabled: false, disabledByArchive: true }; - } - return task; - }); - return changed ? next : tasks; - }); + await updateCronTasks( + projectRoot, + (tasks) => { + let changed = false; + const next = tasks.map((task) => { + if ( + isBoundTask(task) && + targets.has(task.sessionId) && + task.enabled !== false + ) { + changed = true; + return { ...task, enabled: false, disabledByArchive: true }; + } + return task; + }); + return changed ? next : tasks; + }, + options, + ); } /** @@ -77,54 +82,64 @@ export async function enableTasksForSessions( projectRoot: string, sessionIds: string[], now: number = Date.now(), + options: { assertCanCommit?: () => void } = {}, ): Promise { if (sessionIds.length === 0) return; const targets = new Set(sessionIds); - await updateCronTasks(projectRoot, (tasks) => { - let changed = false; - const next = tasks.map((task) => { - if ( - isBoundTask(task) && - targets.has(task.sessionId) && - task.enabled === false && - task.disabledByArchive === true - ) { - changed = true; - const resumed: DurableCronTask = { ...task, enabled: true }; - delete resumed.disabledByArchive; - const minute = now - (now % 60_000); - if (resumed.recurring) { - // Recurring anchor is lastFiredAt: resume from now, not catching up - // fires missed while archived. - resumed.lastFiredAt = minute; - } else { - // A one-shot anchors on createdAt: without re-seating it, the - // scheduler reads the original long-past slot as a MISSED one-shot on - // reload and fires + permanently deletes the task. (Reachable: archive - // a task, PATCH it to recurring:false while disabled — the route - // re-seat only touches recurring anchors — then unarchive.) - resumed.createdAt = now; - resumed.lastFiredAt = minute; + await updateCronTasks( + projectRoot, + (tasks) => { + let changed = false; + const next = tasks.map((task) => { + if ( + isBoundTask(task) && + targets.has(task.sessionId) && + task.enabled === false && + task.disabledByArchive === true + ) { + changed = true; + const resumed: DurableCronTask = { ...task, enabled: true }; + delete resumed.disabledByArchive; + const minute = now - (now % 60_000); + if (resumed.recurring) { + // Recurring anchor is lastFiredAt: resume from now, not catching up + // fires missed while archived. + resumed.lastFiredAt = minute; + } else { + // A one-shot anchors on createdAt: without re-seating it, the + // scheduler reads the original long-past slot as a MISSED one-shot on + // reload and fires + permanently deletes the task. (Reachable: archive + // a task, PATCH it to recurring:false while disabled — the route + // re-seat only touches recurring anchors — then unarchive.) + resumed.createdAt = now; + resumed.lastFiredAt = minute; + } + return resumed; } - return resumed; - } - return task; - }); - return changed ? next : tasks; - }); + return task; + }); + return changed ? next : tasks; + }, + options, + ); } /** Removes every task bound to one of `sessionIds` (deleted sessions). */ export async function removeTasksForSessions( projectRoot: string, sessionIds: string[], + options: { assertCanCommit?: () => void } = {}, ): Promise { if (sessionIds.length === 0) return; const targets = new Set(sessionIds); - await updateCronTasks(projectRoot, (tasks) => { - const next = tasks.filter( - (task) => !isBoundTask(task) || !targets.has(task.sessionId), - ); - return next.length === tasks.length ? tasks : next; - }); + await updateCronTasks( + projectRoot, + (tasks) => { + const next = tasks.filter( + (task) => !isBoundTask(task) || !targets.has(task.sessionId), + ); + return next.length === tasks.length ? tasks : next; + }, + options, + ); } diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 7ed168e17d7..c0d78a3dea0 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -580,6 +580,7 @@ const EXPECTED_STAGE1_FEATURES = [ 'session_status', 'session_close', 'session_archive', + 'session_storage_conflict_repair', 'session_metadata', 'session_organization', 'session_export', @@ -25169,6 +25170,7 @@ describe('createServeApp', () => { expect(res.body).toEqual({ archived: [sid], alreadyArchived: [], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -25181,6 +25183,51 @@ describe('createServeApp', () => { ).resolves.toBeUndefined(); }); + it('requires an explicit boolean to repair active/archive conflicts', async () => { + const sid = '11111111-bbbb-cccc-dddd-eeeeeeeeeeea'; + await writeSession(sid); + await writeSession(sid, 'archived'); + const app = createArchiveApp(); + + const invalid = await request(app) + .post('/sessions/archive') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionIds: [sid], resolveConflicts: 'yes' }); + expect(invalid.status).toBe(400); + expect(invalid.body.code).toBe('invalid_request'); + + const archived = await request(app) + .post('/sessions/archive') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionIds: [sid], resolveConflicts: true }); + expect(archived.status).toBe(200); + expect(archived.body).toMatchObject({ + archived: [sid], + resolvedConflicts: [sid], + errors: [], + }); + await expect(fsp.access(sessionFilePath(sid))).rejects.toThrow(); + await expect( + fsp.access(sessionFilePath(sid, 'archived')), + ).resolves.toBeUndefined(); + + await writeSession(sid); + const unarchived = await request(app) + .post('/sessions/unarchive') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionIds: [sid], resolveConflicts: true }); + expect(unarchived.status).toBe(200); + expect(unarchived.body).toMatchObject({ + unarchived: [sid], + resolvedConflicts: [sid], + errors: [], + }); + await expect(fsp.access(sessionFilePath(sid))).resolves.toBeUndefined(); + await expect( + fsp.access(sessionFilePath(sid, 'archived')), + ).rejects.toThrow(); + }); + it('invalidates active and archived catalogs across archive, unarchive, and delete', async () => { const sid = '11111111-bbbb-cccc-dddd-eeeeeeeeeeef'; await writeSession(sid); @@ -25447,6 +25494,7 @@ describe('createServeApp', () => { expect(res.body).toEqual({ unarchived: [sid], alreadyActive: [], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -34134,6 +34182,20 @@ describe('Live conversation runtime lifecycle', () => { const getLocation = vi .spyOn(SessionService.prototype, 'getSessionLocation') .mockResolvedValue('active'); + const getMaintainableLocation = vi + .spyOn(SessionService.prototype, 'getMaintainableSessionLocation') + .mockImplementation(async function (candidateId) { + if (candidateId === 'ordinary-session') { + return this.getProjectRoot() === '/work/live-primary' + ? 'active' + : undefined; + } + return this.getProjectRoot() === setup.root.canonicalRoot && + candidateId !== 'not-live' && + candidateId !== 'missing-live-session' + ? 'active' + : undefined; + }); const sessionExists = vi .spyOn(SessionService.prototype, 'sessionExistsInAnyState') .mockImplementation(async function (candidateId) { @@ -34244,6 +34306,7 @@ describe('Live conversation runtime lifecycle', () => { expect(updateOrganization).toHaveBeenCalledOnce(); } finally { getLocation.mockRestore(); + getMaintainableLocation.mockRestore(); sessionExists.mockRestore(); readMetadata.mockRestore(); readMetadataIfReadable.mockRestore(); diff --git a/packages/cli/src/serve/server/session-archive.test.ts b/packages/cli/src/serve/server/session-archive.test.ts index 6e496735f75..531542cabb0 100644 --- a/packages/cli/src/serve/server/session-archive.test.ts +++ b/packages/cli/src/serve/server/session-archive.test.ts @@ -7,10 +7,12 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { execFileSync } from 'node:child_process'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SessionIdCaseConflictError, SessionService, + SessionStorageEntryError, SessionWriterConflictError, SessionWriterLostError, type SessionWriterLease, @@ -388,6 +390,7 @@ describe('archiveDaemonSessions', () => { expect(result).toEqual({ archived: [sessionId], alreadyArchived: [], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -415,6 +418,7 @@ describe('archiveDaemonSessions', () => { expect(result).toEqual({ archived: [sessionId], alreadyArchived: [], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -465,6 +469,35 @@ describe('archiveDaemonSessions', () => { expect(byId['other']!.enabled).toBeUndefined(); // unrelated — untouched }); + it('reports task maintenance failure after archiving the transcript', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440051'; + writeSessionFile(workspaceDir, sessionId, 'active'); + await updateCronTasks(workspaceDir, () => [ + { + id: 'bound', + cron: '0 9 * * *', + prompt: 'p', + recurring: true, + createdAt: 1_700_000_000_000, + lastFiredAt: null, + sessionId, + }, + ]); + const cronFile = getCronFilePath(workspaceDir); + fs.rmSync(cronFile); + fs.mkdirSync(cronFile); + + const result = await archiveDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result.archived).toEqual([sessionId]); + expect(result.errors).toEqual([{ sessionId, error: expect.any(Error) }]); + }); + it('does not acquire writer leases for ids already archived or missing', async () => { const archivedId = '550e8400-e29b-41d4-a716-446655440003'; const missingId = '550e8400-e29b-41d4-a716-446655440004'; @@ -483,6 +516,7 @@ describe('archiveDaemonSessions', () => { expect(result).toEqual({ archived: [], alreadyArchived: [archivedId], + resolvedConflicts: [], notFound: [missingId], errors: [], }); @@ -521,6 +555,70 @@ describe('archiveDaemonSessions', () => { expect(retried.archived).toEqual([sessionId]); }); + it('takes over a sealed empty transcript before maintenance', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440006'; + const activePath = sessionPath(workspaceDir, sessionId, 'active'); + fs.mkdirSync(path.dirname(activePath), { recursive: true }); + fs.writeFileSync(activePath, ''); + const service = new SessionService(workspaceDir); + const previous = await service.acquireSessionWriterLease(sessionId, { + processKind: 'daemon', + reclaimPolicy: 'never', + }); + await previous.sealForHandoff(); + + const result = await archiveDaemonSessions({ + sessionIds: [sessionId], + service, + bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result).toMatchObject({ + archived: [sessionId], + errors: [], + }); + expect(fs.existsSync(activePath)).toBe(false); + expect( + fs.existsSync(sessionPath(workspaceDir, sessionId, 'archived')), + ).toBe(true); + }); + + it.runIf(process.platform !== 'win32')( + 'rejects a transcript FIFO without waiting for a writer', + async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440106'; + const activePath = sessionPath(workspaceDir, sessionId, 'active'); + fs.mkdirSync(path.dirname(activePath), { recursive: true }); + execFileSync('mkfifo', [activePath]); + let writer: number | undefined; + const unblock = setTimeout(() => { + writer = fs.openSync( + activePath, + fs.constants.O_WRONLY | (fs.constants.O_NONBLOCK ?? 0), + ); + }, 500); + const startedAt = Date.now(); + + try { + const result = await archiveDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result.errors[0]?.error).toBeInstanceOf( + SessionStorageEntryError, + ); + expect(Date.now() - startedAt).toBeLessThan(400); + } finally { + clearTimeout(unblock); + if (writer !== undefined) fs.closeSync(writer); + } + }, + ); + it('keeps independent batch sessions moving when one writer conflicts', async () => { const blockedId = '550e8400-e29b-41d4-a716-446655440008'; const availableId = '550e8400-e29b-41d4-a716-446655440009'; @@ -592,10 +690,13 @@ describe('archiveDaemonSessions', () => { const availableId = '550e8400-e29b-41d4-a716-446655440020'; writeSessionFile(workspaceDir, availableId, 'active'); const service = new SessionService(workspaceDir); - const getLocation = service.getSessionLocation.bind(service); + const getLocation = service.getMaintainableSessionLocation.bind(service); const failure = new Error('classification failed'); - vi.spyOn(service, 'getSessionLocation').mockImplementation((sessionId) => - sessionId === failedId ? Promise.reject(failure) : getLocation(sessionId), + vi.spyOn(service, 'getMaintainableSessionLocation').mockImplementation( + (sessionId) => + sessionId === failedId + ? Promise.reject(failure) + : getLocation(sessionId), ); const result = await archiveDaemonSessions({ @@ -635,21 +736,25 @@ describe('archiveDaemonSessions', () => { const sessionId = '550e8400-e29b-41d4-a716-446655440010'; writeSessionFile(workspaceDir, sessionId, 'active'); const service = new SessionService(workspaceDir); - const originalGetLocation = service.getSessionLocation.bind(service); + const originalGetLocation = + service.getMaintainableSessionLocation.bind(service); let classifications = 0; - vi.spyOn(service, 'getSessionLocation').mockImplementation(async (id) => { - classifications++; - if (classifications === 2) { - fs.mkdirSync(path.dirname(sessionPath(workspaceDir, id, 'archived')), { - recursive: true, - }); - fs.renameSync( - sessionPath(workspaceDir, id, 'active'), - sessionPath(workspaceDir, id, 'archived'), - ); - } - return originalGetLocation(id); - }); + vi.spyOn(service, 'getMaintainableSessionLocation').mockImplementation( + async (id) => { + classifications++; + if (classifications === 2) { + fs.mkdirSync( + path.dirname(sessionPath(workspaceDir, id, 'archived')), + { recursive: true }, + ); + fs.renameSync( + sessionPath(workspaceDir, id, 'active'), + sessionPath(workspaceDir, id, 'archived'), + ); + } + return originalGetLocation(id); + }, + ); const result = await archiveDaemonSessions({ sessionIds: [sessionId], @@ -661,6 +766,7 @@ describe('archiveDaemonSessions', () => { expect(result).toEqual({ archived: [], alreadyArchived: [sessionId], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -690,6 +796,32 @@ describe('archiveDaemonSessions', () => { expect(acquire).not.toHaveBeenCalled(); }); + it('repairs an active/archive conflict by keeping the archived copy', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440116'; + writeSessionFile(workspaceDir, sessionId, 'active'); + writeSessionFile(workspaceDir, sessionId, 'archived'); + const archivedPath = sessionPath(workspaceDir, sessionId, 'archived'); + const archivedBytes = fs.readFileSync(archivedPath); + + const result = await archiveDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + coordinator: new SessionArchiveCoordinator(), + resolveConflicts: true, + }); + + expect(result).toMatchObject({ + archived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.existsSync(sessionPath(workspaceDir, sessionId, 'active'))).toBe( + false, + ); + expect(fs.readFileSync(archivedPath)).toEqual(archivedBytes); + }); + it('does not report success after release fails but reconciles the task to the applied archive', async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440006'; writeSessionFile(workspaceDir, sessionId, 'active'); @@ -814,6 +946,36 @@ describe('archiveDaemonSessions', () => { true, ); }); + + it('recovers an enabled task whose session is already archived', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440063'; + writeSessionFile(workspaceDir, sessionId, 'archived'); + await updateCronTasks(workspaceDir, () => [ + { + id: 'stranded', + cron: '0 9 * * *', + prompt: 'p', + recurring: true, + createdAt: 1_700_000_000_000, + lastFiredAt: null, + sessionId, + }, + ]); + + const result = await archiveDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { closeSession: vi.fn().mockResolvedValue(undefined) }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result.alreadyArchived).toEqual([sessionId]); + const stranded = (await readCronTasks(workspaceDir)).find( + (task) => task.id === 'stranded', + ); + expect(stranded!.enabled).toBe(false); + expect(stranded!.disabledByArchive).toBe(true); + }); }); describe('unarchiveDaemonSessions', () => { @@ -849,6 +1011,7 @@ describe('unarchiveDaemonSessions', () => { expect(result).toEqual({ unarchived: [archivedId], alreadyActive: [activeId], + resolvedConflicts: [], notFound: [missingId], errors: [], }); @@ -874,6 +1037,7 @@ describe('unarchiveDaemonSessions', () => { expect(result).toEqual({ unarchived: [sessionId], alreadyActive: [], + resolvedConflicts: [], notFound: [], errors: [], }); @@ -885,6 +1049,31 @@ describe('unarchiveDaemonSessions', () => { ).toBe(false); }); + it('repairs an active/archive conflict by keeping the active copy', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440117'; + writeSessionFile(workspaceDir, sessionId, 'active'); + writeSessionFile(workspaceDir, sessionId, 'archived'); + const activePath = sessionPath(workspaceDir, sessionId, 'active'); + const activeBytes = fs.readFileSync(activePath); + + const result = await unarchiveDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + coordinator: new SessionArchiveCoordinator(), + resolveConflicts: true, + }); + + expect(result).toMatchObject({ + unarchived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.readFileSync(activePath)).toEqual(activeBytes); + expect( + fs.existsSync(sessionPath(workspaceDir, sessionId, 'archived')), + ).toBe(false); + }); + it('does not unarchive while another writer holds the lease', async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440015'; writeSessionFile(workspaceDir, sessionId, 'archived'); @@ -924,6 +1113,7 @@ describe('unarchiveDaemonSessions', () => { expect(result).toEqual({ unarchived: [], alreadyActive: [], + resolvedConflicts: [], notFound: [], errors: [{ sessionId: archivedId, error: failure }], }); @@ -939,10 +1129,13 @@ describe('unarchiveDaemonSessions', () => { const availableId = '550e8400-e29b-41d4-a716-446655440022'; writeSessionFile(workspaceDir, availableId, 'archived'); const service = new SessionService(workspaceDir); - const getLocation = service.getSessionLocation.bind(service); + const getLocation = service.getMaintainableSessionLocation.bind(service); const failure = new Error('classification failed'); - vi.spyOn(service, 'getSessionLocation').mockImplementation((sessionId) => - sessionId === failedId ? Promise.reject(failure) : getLocation(sessionId), + vi.spyOn(service, 'getMaintainableSessionLocation').mockImplementation( + (sessionId) => + sessionId === failedId + ? Promise.reject(failure) + : getLocation(sessionId), ); const result = await unarchiveDaemonSessions({ @@ -961,22 +1154,24 @@ describe('unarchiveDaemonSessions', () => { writeSessionFile(workspaceDir, unarchivedId, 'archived'); writeSessionFile(workspaceDir, blockedId, 'archived'); const service = new SessionService(workspaceDir); - const getLocation = service.getSessionLocation.bind(service); + const getLocation = service.getMaintainableSessionLocation.bind(service); const coordinator = new SessionArchiveCoordinator(); let releaseBlocked!: () => void; const blocked = new Promise((resolve) => { releaseBlocked = resolve; }); let competingMaintenance: Promise | undefined; - vi.spyOn(service, 'getSessionLocation').mockImplementation((sessionId) => { - if (sessionId === unarchivedId && !competingMaintenance) { - competingMaintenance = coordinator.runExclusiveMany( - [blockedId], - () => blocked, - ); - } - return getLocation(sessionId); - }); + vi.spyOn(service, 'getMaintainableSessionLocation').mockImplementation( + (sessionId) => { + if (sessionId === unarchivedId && !competingMaintenance) { + competingMaintenance = coordinator.runExclusiveMany( + [blockedId], + () => blocked, + ); + } + return getLocation(sessionId); + }, + ); const result = await unarchiveDaemonSessions({ sessionIds: [unarchivedId, blockedId], @@ -1171,6 +1366,66 @@ describe('deleteDaemonSessions', () => { expect(ids).toEqual(['other']); // bound task deleted, unbound survives }); + it('repairs task maintenance on retry after deleting the transcript', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440171'; + writeSessionFile(workspaceDir, sessionId, 'active'); + await updateCronTasks(workspaceDir, () => [ + { + id: 'bound', + cron: '0 9 * * *', + prompt: 'p', + recurring: true, + createdAt: 1_700_000_000_000, + lastFiredAt: null, + sessionId, + }, + ]); + const cronFile = getCronFilePath(workspaceDir); + const cronContents = fs.readFileSync(cronFile); + fs.rmSync(cronFile); + fs.mkdirSync(cronFile); + const deleteSessionAttachments = vi.fn().mockResolvedValue(undefined); + + const result = await deleteDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { + closeSession: vi.fn().mockResolvedValue(undefined), + deleteSessionAttachments, + }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(result.removed).toEqual([sessionId]); + expect(result.errors).toEqual([ + { + sessionId, + error: 'Scheduled task lifecycle update failed.', + }, + ]); + expect(deleteSessionAttachments).toHaveBeenCalledWith(sessionId); + + fs.rmSync(cronFile, { recursive: true, force: true }); + fs.writeFileSync(cronFile, cronContents); + + const retry = await deleteDaemonSessions({ + sessionIds: [sessionId], + service: new SessionService(workspaceDir), + bridge: { + closeSession: vi.fn().mockResolvedValue(undefined), + deleteSessionAttachments, + }, + coordinator: new SessionArchiveCoordinator(), + }); + + expect(retry).toEqual({ + removed: [], + notFound: [sessionId], + errors: [], + }); + expect(await readCronTasks(workspaceDir)).toEqual([]); + }); + it('collapses case-variant spellings in one batch to a single delete', async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440170'; writeSessionFile(workspaceDir, sessionId, 'active'); @@ -1376,6 +1631,42 @@ describe('deleteDaemonSessions', () => { expect(markSessionCatalogChanged).toHaveBeenCalledTimes(1); }); + it('returns true when task maintenance fails after orphan deletion', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440086'; + writeSessionFile(workspaceDir, sessionId, 'active'); + await updateCronTasks(workspaceDir, () => [ + { + id: 'bound', + cron: '0 9 * * *', + prompt: 'p', + recurring: true, + createdAt: 1_700_000_000_000, + lastFiredAt: null, + sessionId, + }, + ]); + const cronFile = getCronFilePath(workspaceDir); + fs.rmSync(cronFile); + fs.mkdirSync(cronFile); + const markSessionCatalogChanged = vi.fn(); + + await expect( + deleteDaemonSessionIfOrphan({ + sessionId, + service: new SessionService(workspaceDir), + bridge: { + killSession: vi.fn().mockResolvedValue(true), + markSessionCatalogChanged, + }, + coordinator: new SessionArchiveCoordinator(), + }), + ).resolves.toBe(true); + expect(fs.existsSync(sessionPath(workspaceDir, sessionId, 'active'))).toBe( + false, + ); + expect(markSessionCatalogChanged).toHaveBeenCalledTimes(1); + }); + it('deletes the transcript when killSession throws SessionNotFoundError', async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440084'; writeSessionFile(workspaceDir, sessionId, 'active'); diff --git a/packages/cli/src/serve/server/session-archive.ts b/packages/cli/src/serve/server/session-archive.ts index 168f5095822..964a4d3f8be 100644 --- a/packages/cli/src/serve/server/session-archive.ts +++ b/packages/cli/src/serve/server/session-archive.ts @@ -7,6 +7,8 @@ import { SessionIdCaseConflictError, SessionService, + SessionTranscriptChangedError, + SessionWriterUnavailableError, type SessionLocation, } from '@qwen-code/qwen-code-core'; import type { AcpSessionBridge } from '../acp-session-bridge.js'; @@ -29,6 +31,7 @@ import { export interface DaemonArchiveSessionsResult { archived: string[]; alreadyArchived: string[]; + resolvedConflicts: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: unknown }>; } @@ -36,6 +39,7 @@ export interface DaemonArchiveSessionsResult { export interface DaemonUnarchiveSessionsResult { unarchived: string[]; alreadyActive: string[]; + resolvedConflicts: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: unknown }>; } @@ -43,7 +47,7 @@ export interface DaemonUnarchiveSessionsResult { export interface DaemonDeleteSessionsResult { removed: string[]; notFound: string[]; - errors: Array<{ sessionId: string; error: unknown }>; + errors: Array<{ sessionId: string; error: string }>; } export type DaemonDeleteErrorPhase = 'close' | 'remove' | 'delete'; @@ -192,10 +196,25 @@ async function runWithDaemonWriterLease(params: { } = params; let lease; try { - lease = await service.acquireSessionWriterLease(sessionId, { - processKind: 'daemon', - reclaimPolicy: 'never', - }); + const leaseOptions = { + processKind: 'daemon' as const, + reclaimPolicy: 'never' as const, + takeoverPolicy: 'certified' as const, + }; + try { + lease = await service.acquireSessionWriterLease(sessionId, leaseOptions); + } catch (error) { + if ( + !(error instanceof SessionWriterUnavailableError) && + !(error instanceof SessionTranscriptChangedError) + ) { + throw error; + } + lease = await service.acquireSessionMaintenanceLease( + sessionId, + leaseOptions, + ); + } } catch (error) { return { mutationApplied: false, error }; } @@ -290,28 +309,40 @@ async function classifySessionLocation( service: SessionService, sessionId: string, ): Promise { - return service.getSessionLocation(sessionId); + return service.getMaintainableSessionLocation(sessionId); } -function sessionLocationError(sessionId: string): Error { - return new Error(`Session archive conflict: ${sessionId}`); +function sessionLocationError(sessionId: string): SessionConflictError { + const error = new SessionConflictError(sessionId); + error.message = `Session "${sessionId}" exists in both active and archived directories. Retry with resolveConflicts: true to keep one copy.`; + return error; } function updateScheduledTaskForMaintenance( service: SessionService, sessionId: string, action: DaemonMaintenanceAction, + assertCanMutate?: () => void, ): Promise { if (action === 'archive') { - return disableTasksForSessions(service.getProjectRoot(), [sessionId]); + return disableTasksForSessions(service.getProjectRoot(), [sessionId], { + assertCanCommit: assertCanMutate, + }); } if (action === 'unarchive') { - return enableTasksForSessions(service.getProjectRoot(), [sessionId]); + return enableTasksForSessions( + service.getProjectRoot(), + [sessionId], + Date.now(), + { assertCanCommit: assertCanMutate }, + ); } - return removeTasksForSessions(service.getProjectRoot(), [sessionId]); + return removeTasksForSessions(service.getProjectRoot(), [sessionId], { + assertCanCommit: assertCanMutate, + }); } -type DeleteOneResult = +type DeleteOneResult = ( | { kind: 'removed'; mutationApplied: boolean; @@ -324,15 +355,40 @@ type DeleteOneResult = kind: 'error'; error: unknown; mutationApplied: boolean; - }; + } +) & { maintenanceError?: unknown }; async function deletePersistedSessionWithLease( service: SessionService, sessionId: string, + assertCanMutate?: () => void, ): Promise { const initialLocation = await classifySessionLocation(service, sessionId); if (initialLocation === undefined) { - return { kind: 'notFound', mutationApplied: false }; + let maintenanceError: unknown; + try { + assertCanMutate?.(); + await updateScheduledTaskForMaintenance( + service, + sessionId, + 'delete', + assertCanMutate, + ); + } catch (error) { + maintenanceError = error; + logSessionArchiveWarning( + `scheduled task lifecycle update failed action=delete workspace=${safeLogValue( + service.getProjectRoot(), + )} session=${safeLogValue(sessionId)} error=${safeLogValue( + errorMessage(error), + )}`, + ); + } + return { + kind: 'notFound', + mutationApplied: false, + maintenanceError, + }; } const mutation = await runWithDaemonWriterLease({ @@ -347,8 +403,10 @@ async function deletePersistedSessionWithLease( mutationApplied: false, }; } - await assertOwnedAndUnchanged(); - const removed = await service.removeSession(sessionId); + const removed = await service.removeSession(sessionId, { + assertStorageUnchanged: assertOwnedAndUnchanged, + assertCanMutate, + }); return { value: removed ? ('removed' as const) : ('notFound' as const), mutationApplied: removed, @@ -356,19 +414,28 @@ async function deletePersistedSessionWithLease( }, mutationAppliedAfterError: async () => (await classifySessionLocation(service, sessionId)) === undefined, - afterMutationApplied: () => - updateScheduledTaskForMaintenance(service, sessionId, 'delete'), + afterMutationApplied: async () => { + assertCanMutate?.(); + await updateScheduledTaskForMaintenance( + service, + sessionId, + 'delete', + assertCanMutate, + ); + }, }); if (mutation.error !== undefined) { return { kind: 'error', error: mutation.error, mutationApplied: mutation.mutationApplied, + maintenanceError: mutation.maintenanceError, }; } return { kind: mutation.value ?? 'notFound', mutationApplied: mutation.mutationApplied, + maintenanceError: mutation.maintenanceError, }; } @@ -378,6 +445,7 @@ export async function deleteDaemonSessions(params: { bridge: Pick; coordinator: SessionArchiveCoordinator; coordinatorLockHeld?: boolean; + assertCanMutate?: () => void; onError?: (entry: { phase: DaemonDeleteErrorPhase; sessionId: string; @@ -390,6 +458,7 @@ export async function deleteDaemonSessions(params: { bridge, coordinator, coordinatorLockHeld = false, + assertCanMutate, onError, } = params; const uniqueSessionIds = [ @@ -404,10 +473,12 @@ export async function deleteDaemonSessions(params: { uniqueSessionIds.map(async (sessionId) => { try { const mutateSession = async () => { + assertCanMutate?.(); const removePersistedSession = async () => { const result = await deletePersistedSessionWithLease( service, sessionId, + assertCanMutate, ); if (result.kind === 'error') { onError?.({ @@ -418,7 +489,13 @@ export async function deleteDaemonSessions(params: { return result; } try { - await bridge.deleteSessionAttachments(sessionId); + if (assertCanMutate) { + await bridge.deleteSessionAttachments(sessionId, { + assertCanCommit: assertCanMutate, + }); + } else { + await bridge.deleteSessionAttachments(sessionId); + } return result; } catch (error) { onError?.({ @@ -430,6 +507,7 @@ export async function deleteDaemonSessions(params: { kind: 'error' as const, error, mutationApplied: result.mutationApplied, + maintenanceError: result.maintenanceError, }; } }; @@ -476,7 +554,7 @@ export async function deleteDaemonSessions(params: { const removed: string[] = []; const notFound: string[] = []; - const errors: Array<{ sessionId: string; error: unknown }> = []; + const errors: Array<{ sessionId: string; error: string }> = []; for (let i = 0; i < results.length; i++) { const sessionId = uniqueSessionIds[i]!; const result = results[i]!; @@ -487,6 +565,12 @@ export async function deleteDaemonSessions(params: { } else { errors.push({ sessionId, error: errorMessage(result.error) }); } + if (result.maintenanceError !== undefined) { + errors.push({ + sessionId, + error: 'Scheduled task lifecycle update failed.', + }); + } } return { removed, notFound, errors }; @@ -685,6 +769,8 @@ export async function archiveDaemonSessions(params: { bridge: Pick; coordinator: SessionArchiveCoordinator; coordinatorLockHeld?: boolean; + resolveConflicts?: boolean; + assertCanMutate?: () => void; }): Promise { const { sessionIds, @@ -692,6 +778,8 @@ export async function archiveDaemonSessions(params: { bridge, coordinator, coordinatorLockHeld = false, + resolveConflicts = false, + assertCanMutate, } = params; const uniqueSessionIds = [ ...new Set(sessionIds.map(normalizeSessionIdForLookup)), @@ -705,6 +793,7 @@ export async function archiveDaemonSessions(params: { uniqueSessionIds.map(async (sessionId) => { try { const mutateSession = async () => { + assertCanMutate?.(); try { await bridge.closeSession(sessionId, undefined, { requireAgentClose: true, @@ -727,12 +816,31 @@ export async function archiveDaemonSessions(params: { return { kind: 'notFound' as const, mutationApplied: false }; } if (initialLocation === 'archived') { + let maintenanceError: unknown; + try { + await updateScheduledTaskForMaintenance( + service, + sessionId, + 'archive', + assertCanMutate, + ); + } catch (error) { + maintenanceError = error; + logSessionArchiveWarning( + `scheduled task lifecycle update failed action=archive workspace=${safeLogValue( + service.getProjectRoot(), + )} session=${safeLogValue(sessionId)} error=${safeLogValue( + errorMessage(error), + )}`, + ); + } return { kind: 'alreadyArchived' as const, mutationApplied: false, + maintenanceError, }; } - if (initialLocation === 'conflict') { + if (initialLocation === 'conflict' && !resolveConflicts) { return { kind: 'error' as const, error: sessionLocationError(sessionId), @@ -761,17 +869,20 @@ export async function archiveDaemonSessions(params: { mutationApplied: false, }; } - if (lockedLocation === 'conflict') { + if (lockedLocation === 'conflict' && !resolveConflicts) { throw sessionLocationError(sessionId); } - await assertOwnedAndUnchanged(); const result = await service.archiveSessions([sessionId], { - knownLocation: 'active', + resolveConflicts, + assertStorageUnchanged: assertOwnedAndUnchanged, + assertCanMutate, }); if (result.errors[0]) throw result.errors[0].error; if (result.archived.length > 0) { return { - value: 'archived' as const, + value: result.resolvedConflicts.length + ? ('resolvedConflict' as const) + : ('archived' as const), mutationApplied: true, }; } @@ -786,19 +897,28 @@ export async function archiveDaemonSessions(params: { mutationAppliedAfterError: async () => (await classifySessionLocation(service, sessionId)) === 'archived', - afterMutationApplied: () => - updateScheduledTaskForMaintenance(service, sessionId, 'archive'), + afterMutationApplied: async () => { + assertCanMutate?.(); + await updateScheduledTaskForMaintenance( + service, + sessionId, + 'archive', + assertCanMutate, + ); + }, }); if (mutation.error !== undefined) { return { kind: 'error' as const, error: mutation.error, mutationApplied: mutation.mutationApplied, + maintenanceError: mutation.maintenanceError, }; } return { kind: mutation.value ?? 'notFound', mutationApplied: mutation.mutationApplied, + maintenanceError: mutation.maintenanceError, }; }; return await (coordinatorLockHeld @@ -820,16 +940,23 @@ export async function archiveDaemonSessions(params: { const archived: string[] = []; const alreadyArchived: string[] = []; + const resolvedConflicts: string[] = []; const notFound: string[] = []; const errors: Array<{ sessionId: string; error: unknown }> = []; for (let i = 0; i < results.length; i++) { const sessionId = uniqueSessionIds[i]!; const result = results[i]!; if (result.kind === 'archived') archived.push(sessionId); - else if (result.kind === 'alreadyArchived') { + else if (result.kind === 'resolvedConflict') { + archived.push(sessionId); + resolvedConflicts.push(sessionId); + } else if (result.kind === 'alreadyArchived') { alreadyArchived.push(sessionId); } else if (result.kind === 'notFound') notFound.push(sessionId); else errors.push({ sessionId, error: result.error }); + if ('maintenanceError' in result && result.maintenanceError !== undefined) { + errors.push({ sessionId, error: result.maintenanceError }); + } } logSessionArchiveResult('archive', { @@ -840,7 +967,13 @@ export async function archiveDaemonSessions(params: { errors, }); - return { archived, alreadyArchived, notFound, errors }; + return { + archived, + alreadyArchived, + resolvedConflicts, + notFound, + errors, + }; } export async function unarchiveDaemonSessions(params: { @@ -848,12 +981,16 @@ export async function unarchiveDaemonSessions(params: { service: SessionService; coordinator: SessionArchiveCoordinator; coordinatorLockHeld?: boolean; + resolveConflicts?: boolean; + assertCanMutate?: () => void; }): Promise { const { sessionIds, service, coordinator, coordinatorLockHeld = false, + resolveConflicts = false, + assertCanMutate, } = params; const uniqueSessionIds = [ ...new Set(sessionIds.map(normalizeSessionIdForLookup)), @@ -867,6 +1004,7 @@ export async function unarchiveDaemonSessions(params: { uniqueSessionIds.map(async (sessionId) => { try { const mutateSession = async () => { + assertCanMutate?.(); const initialLocation = await classifySessionLocation( service, sessionId, @@ -881,6 +1019,7 @@ export async function unarchiveDaemonSessions(params: { service, sessionId, 'unarchive', + assertCanMutate, ); } catch (error) { maintenanceError = error; @@ -898,7 +1037,7 @@ export async function unarchiveDaemonSessions(params: { maintenanceError, }; } - if (initialLocation === 'conflict') { + if (initialLocation === 'conflict' && !resolveConflicts) { return { kind: 'error' as const, error: sessionLocationError(sessionId), @@ -927,17 +1066,20 @@ export async function unarchiveDaemonSessions(params: { mutationApplied: false, }; } - if (lockedLocation === 'conflict') { + if (lockedLocation === 'conflict' && !resolveConflicts) { throw sessionLocationError(sessionId); } - await assertOwnedAndUnchanged(); const result = await service.unarchiveSessions([sessionId], { - knownLocation: 'archived', + resolveConflicts, + assertStorageUnchanged: assertOwnedAndUnchanged, + assertCanMutate, }); if (result.errors[0]) throw result.errors[0].error; if (result.unarchived.length > 0) { return { - value: 'unarchived' as const, + value: result.resolvedConflicts.length + ? ('resolvedConflict' as const) + : ('unarchived' as const), mutationApplied: true, }; } @@ -951,18 +1093,22 @@ export async function unarchiveDaemonSessions(params: { }, mutationAppliedAfterError: async () => (await classifySessionLocation(service, sessionId)) === 'active', - afterMutationApplied: () => - updateScheduledTaskForMaintenance( + afterMutationApplied: async () => { + assertCanMutate?.(); + await updateScheduledTaskForMaintenance( service, sessionId, 'unarchive', - ), + assertCanMutate, + ); + }, }); if (mutation.error !== undefined) { return { kind: 'error' as const, error: mutation.error, mutationApplied: mutation.mutationApplied, + maintenanceError: mutation.maintenanceError, }; } return { @@ -990,13 +1136,17 @@ export async function unarchiveDaemonSessions(params: { const unarchived: string[] = []; const alreadyActive: string[] = []; + const resolvedConflicts: string[] = []; const notFound: string[] = []; const errors: Array<{ sessionId: string; error: unknown }> = []; for (let i = 0; i < results.length; i++) { const sessionId = uniqueSessionIds[i]!; const result = results[i]!; if (result.kind === 'unarchived') unarchived.push(sessionId); - else if (result.kind === 'alreadyActive') alreadyActive.push(sessionId); + else if (result.kind === 'resolvedConflict') { + unarchived.push(sessionId); + resolvedConflicts.push(sessionId); + } else if (result.kind === 'alreadyActive') alreadyActive.push(sessionId); else if (result.kind === 'notFound') notFound.push(sessionId); else errors.push({ sessionId, error: result.error }); if (result.maintenanceError !== undefined) { @@ -1012,5 +1162,11 @@ export async function unarchiveDaemonSessions(params: { errors, }); - return { unarchived, alreadyActive, notFound, errors }; + return { + unarchived, + alreadyActive, + resolvedConflicts, + notFound, + errors, + }; } diff --git a/packages/core/src/services/session-organization-service.test.ts b/packages/core/src/services/session-organization-service.test.ts index 9c05774d773..a9f50820d66 100644 --- a/packages/core/src/services/session-organization-service.test.ts +++ b/packages/core/src/services/session-organization-service.test.ts @@ -595,6 +595,20 @@ describe('SessionOrganizationService', () => { expect(snapshot.sessions.has(sessionIdA)).toBe(false); }); + it('checks the runtime generation before removing an organization entry', async () => { + await service.updateSessionOrganization(sessionIdA, { isPinned: true }); + const generationClosed = new Error('generation closed'); + + await expect( + service.removeSession(sessionIdA, { + assertCanCommit: () => { + throw generationClosed; + }, + }), + ).rejects.toBe(generationClosed); + expect((await service.readSnapshot()).sessions.has(sessionIdA)).toBe(true); + }); + it('removes multiple session organization entries in one call', async () => { const group = await service.createGroup({ name: 'Cleanup', diff --git a/packages/core/src/services/session-organization-service.ts b/packages/core/src/services/session-organization-service.ts index c5045deeaeb..585d8d8ac6e 100644 --- a/packages/core/src/services/session-organization-service.ts +++ b/packages/core/src/services/session-organization-service.ts @@ -415,18 +415,24 @@ export class SessionOrganizationService { }); } - async removeSession(sessionId: string): Promise { + async removeSession( + sessionId: string, + options: { assertCanCommit?: () => void } = {}, + ): Promise { await this.withStoreLock(async () => { const store = await this.readStore(); if (!Object.prototype.hasOwnProperty.call(store.sessions, sessionId)) { return; } delete store.sessions[sessionId]; - await this.writeStore(store); + await this.writeStore(store, options); }); } - async removeSessions(sessionIds: string[]): Promise { + async removeSessions( + sessionIds: string[], + options: { assertCanCommit?: () => void } = {}, + ): Promise { const uniqueSessionIds = [...new Set(sessionIds)]; if (uniqueSessionIds.length === 0) return; @@ -440,7 +446,7 @@ export class SessionOrganizationService { } } if (changed) { - await this.writeStore(store); + await this.writeStore(store, options); } }); } @@ -538,7 +544,10 @@ export class SessionOrganizationService { return emptyStore(); } - private async writeStore(store: SessionOrganizationStoreV1): Promise { + private async writeStore( + store: SessionOrganizationStoreV1, + options: { assertCanCommit?: () => void } = {}, + ): Promise { await fs.mkdir(path.dirname(this.getStorePath()), { recursive: true }); if (this.readFailed) { throw new SessionOrganizationError( @@ -546,11 +555,15 @@ export class SessionOrganizationService { 'session_organization_store_unreadable', ); } - await atomicWriteJSON(this.getStorePath(), { - schemaVersion: SCHEMA_VERSION, - groups: this.sortGroups(store.groups), - sessions: store.sessions, - }); + await atomicWriteJSON( + this.getStorePath(), + { + schemaVersion: SCHEMA_VERSION, + groups: this.sortGroups(store.groups), + sessions: store.sessions, + }, + { assertCanCommit: options.assertCanCommit }, + ); this.readFailed = false; } diff --git a/packages/core/src/services/session-pr-service.ts b/packages/core/src/services/session-pr-service.ts index b49b701988b..8edf2a80156 100644 --- a/packages/core/src/services/session-pr-service.ts +++ b/packages/core/src/services/session-pr-service.ts @@ -113,9 +113,10 @@ export async function readSessionPrs( export async function writeSessionPrs( filePath: string, prs: SessionPr[], + options: { assertCanCommit?: () => void } = {}, ): Promise { await fs.mkdir(path.dirname(filePath), { recursive: true }); - await atomicWriteJSON(filePath, { prs } satisfies SessionPrList); + await atomicWriteJSON(filePath, { prs } satisfies SessionPrList, options); } /** diff --git a/packages/core/src/services/sessionService.corruption.test.ts b/packages/core/src/services/sessionService.corruption.test.ts index b2bf3440532..6c982bfd310 100644 --- a/packages/core/src/services/sessionService.corruption.test.ts +++ b/packages/core/src/services/sessionService.corruption.test.ts @@ -18,10 +18,22 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; -import { SessionService } from './sessionService.js'; +import { execFileSync } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; +import { SessionService, SessionStorageEntryError } from './sessionService.js'; +import { SessionTranscriptIdentityUnavailableError } from './session-writer-lease.js'; import type { ChatRecord } from './chatRecordingService.js'; import type { HistoryGap } from '../utils/conversation-chain.js'; +import { readSessionPrs, writeSessionPrs } from './session-pr-service.js'; let tmpRoot: string; @@ -319,6 +331,824 @@ describe('SessionService.readLastRecordUuid (corruption recovery)', () => { }); }); +describe('SessionService lifecycle maintenance', () => { + type Privates = { + getSessionFilePath: (id: string, state: 'active' | 'archived') => string; + getPrSessionPathForState: ( + id: string, + state: 'active' | 'archived', + ) => string; + sessionBelongsToCurrentProject: ( + sessionId: string, + cwd: string, + ) => Promise; + }; + + function createHarness(content: string, state: 'active' | 'archived') { + const runtimeBaseDir = fs.mkdtempSync(path.join(tmpRoot, 'lifecycle-')); + const cwd = path.join(runtimeBaseDir, 'workspace'); + fs.mkdirSync(cwd, { recursive: true }); + const service = new SessionService(cwd, { runtimeBaseDir }); + const sessionId = randomUUID(); + const paths = { + active: (service as unknown as Privates).getSessionFilePath( + sessionId, + 'active', + ), + archived: (service as unknown as Privates).getSessionFilePath( + sessionId, + 'archived', + ), + }; + fs.mkdirSync(path.dirname(paths[state]), { recursive: true }); + fs.writeFileSync(paths[state], content); + return { service, sessionId, paths, content, cwd }; + } + + const unreadableShapes = [ + { name: 'empty', content: '' }, + { name: 'damaged', content: '{"uuid":"torn-head"' }, + ]; + + for (const shape of unreadableShapes) { + it(`deletes an owned ${shape.name} transcript`, async () => { + const { service, sessionId, paths } = createHarness( + shape.content, + 'active', + ); + + await expect(service.removeSession(sessionId)).resolves.toBe(true); + expect(fs.existsSync(paths.active)).toBe(false); + }); + + it(`archives an owned ${shape.name} transcript without rewriting it`, async () => { + const { service, sessionId, paths, content } = createHarness( + shape.content, + 'active', + ); + + const result = await service.archiveSessions([sessionId]); + + expect(result).toMatchObject({ + archived: [sessionId], + notFound: [], + errors: [], + }); + expect(fs.existsSync(paths.active)).toBe(false); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe(content); + }); + + it(`unarchives an owned ${shape.name} transcript without rewriting it`, async () => { + const { service, sessionId, paths, content } = createHarness( + shape.content, + 'archived', + ); + + const result = await service.unarchiveSessions([sessionId]); + + expect(result).toMatchObject({ + unarchived: [sessionId], + notFound: [], + errors: [], + }); + expect(fs.existsSync(paths.archived)).toBe(false); + expect(fs.readFileSync(paths.active, 'utf8')).toBe(content); + }); + } + + it('maintains an owned legacy child whose parent no longer exists', async () => { + const orphanContent = (sessionId: string, cwd: string) => + `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n${JSON.stringify({ + ...recordFor('u2', 'assistant', 'u1'), + sessionId, + cwd, + type: 'system', + subtype: 'parent_session', + systemPayload: { parentSessionId: randomUUID() }, + })}\n`; + + const removal = createHarness('', 'active'); + fs.writeFileSync( + removal.paths.active, + orphanContent(removal.sessionId, removal.cwd), + ); + await expect( + removal.service.removeSession(removal.sessionId), + ).resolves.toBe(true); + + const archive = createHarness('', 'active'); + fs.writeFileSync( + archive.paths.active, + orphanContent(archive.sessionId, archive.cwd), + ); + await expect( + archive.service.archiveSessions([archive.sessionId]), + ).resolves.toMatchObject({ archived: [archive.sessionId], errors: [] }); + + const unarchive = createHarness('', 'archived'); + fs.writeFileSync( + unarchive.paths.archived, + orphanContent(unarchive.sessionId, unarchive.cwd), + ); + await expect( + unarchive.service.unarchiveSessions([unarchive.sessionId]), + ).resolves.toMatchObject({ + unarchived: [unarchive.sessionId], + errors: [], + }); + }); + + it('preserves default archive conflicts and explicitly keeps the archived copy', async () => { + const { service, sessionId, paths } = createHarness('active', 'active'); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, 'archived'); + + const defaultResult = await service.archiveSessions([sessionId]); + expect(defaultResult.errors).toHaveLength(1); + expect(fs.readFileSync(paths.active, 'utf8')).toBe('active'); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe('archived'); + + const repaired = await service.archiveSessions([sessionId], { + resolveConflicts: true, + }); + expect(repaired).toMatchObject({ + archived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.existsSync(paths.active)).toBe(false); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe('archived'); + }); + + it.each(['archive', 'unarchive'] as const)( + 'merges pr bindings into the retained copy during %s conflict repair', + async (action) => { + const { service, sessionId, paths, cwd } = createHarness('', 'active'); + const activeContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + const archivedContent = `${JSON.stringify({ + ...recordFor('u2', 'user', null), + sessionId, + cwd, + })}\n`; + fs.writeFileSync(paths.active, activeContent); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, archivedContent); + const internals = service as unknown as Privates; + const activePr = internals.getPrSessionPathForState(sessionId, 'active'); + const archivedPr = internals.getPrSessionPathForState( + sessionId, + 'archived', + ); + const activeEntry = { + number: 1, + url: 'https://github.com/o/r/pull/1', + createdAt: '2026-08-20T00:00:00.000Z', + }; + const archivedEntry = { + number: 2, + url: 'https://github.com/o/r/pull/2', + createdAt: '2026-08-20T01:00:00.000Z', + }; + await writeSessionPrs(activePr, [activeEntry]); + await writeSessionPrs(archivedPr, [archivedEntry]); + + const result = await service[`${action}Sessions`]([sessionId], { + resolveConflicts: true, + }); + + expect(result.errors).toEqual([]); + const retainedPr = action === 'archive' ? archivedPr : activePr; + const losingPr = action === 'archive' ? activePr : archivedPr; + await expect(readSessionPrs(retainedPr)).resolves.toEqual([ + activeEntry, + archivedEntry, + ]); + expect(fs.existsSync(losingPr)).toBe(false); + }, + ); + + it('preserves default unarchive conflicts and explicitly keeps the active copy', async () => { + const { service, sessionId, paths } = createHarness('active', 'active'); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, 'archived'); + + const defaultResult = await service.unarchiveSessions([sessionId]); + expect(defaultResult.errors).toHaveLength(1); + expect(fs.readFileSync(paths.active, 'utf8')).toBe('active'); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe('archived'); + + const repaired = await service.unarchiveSessions([sessionId], { + resolveConflicts: true, + }); + expect(repaired).toMatchObject({ + unarchived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.readFileSync(paths.active, 'utf8')).toBe('active'); + expect(fs.existsSync(paths.archived)).toBe(false); + }); + + it('does not overwrite a damaged archived copy when the active copy is readable', async () => { + const { service, sessionId, paths, cwd } = createHarness('', 'active'); + fs.writeFileSync( + paths.active, + `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`, + ); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, '{"uuid":"torn-archived"'); + + const result = await service.archiveSessions([sessionId]); + + expect(result.errors).toHaveLength(1); + expect(fs.existsSync(paths.active)).toBe(true); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe( + '{"uuid":"torn-archived"', + ); + }); + + it('repairs an archive conflict when only the archived copy is readable', async () => { + const { service, sessionId, paths, cwd } = createHarness( + '{"uuid":"torn-active"', + 'active', + ); + const archivedContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, archivedContent); + + const defaultResult = await service.archiveSessions([sessionId]); + expect(defaultResult.errors).toHaveLength(1); + + const repaired = await service.archiveSessions([sessionId], { + resolveConflicts: true, + }); + expect(repaired).toMatchObject({ + archived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.existsSync(paths.active)).toBe(false); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe(archivedContent); + }); + + it('does not overwrite a damaged active copy when the archived copy is readable', async () => { + const { service, sessionId, paths, cwd } = createHarness('', 'archived'); + fs.writeFileSync(paths.active, '{"uuid":"torn-active"'); + fs.writeFileSync( + paths.archived, + `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`, + ); + + const result = await service.unarchiveSessions([sessionId]); + + expect(result.errors).toHaveLength(1); + expect(fs.readFileSync(paths.active, 'utf8')).toBe('{"uuid":"torn-active"'); + expect(fs.existsSync(paths.archived)).toBe(true); + }); + + it('repairs an unarchive conflict when only the active copy is readable', async () => { + const { service, sessionId, paths, cwd } = createHarness('', 'active'); + const activeContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + fs.writeFileSync(paths.active, activeContent); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, '{"uuid":"torn-archived"'); + + const defaultResult = await service.unarchiveSessions([sessionId]); + expect(defaultResult.errors).toHaveLength(1); + + const repaired = await service.unarchiveSessions([sessionId], { + resolveConflicts: true, + }); + expect(repaired).toMatchObject({ + unarchived: [sessionId], + resolvedConflicts: [sessionId], + errors: [], + }); + expect(fs.readFileSync(paths.active, 'utf8')).toBe(activeContent); + expect(fs.existsSync(paths.archived)).toBe(false); + }); + + it.each(['archive', 'unarchive'] as const)( + 'reclassifies a conflict that appears during %s validation', + async (action) => { + const state = action === 'archive' ? 'active' : 'archived'; + const { service, sessionId, paths, cwd } = createHarness('', state); + const sourcePath = paths[state]; + const sourceContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + const targetPath = action === 'archive' ? paths.archived : paths.active; + const targetContent = 'late conflict'; + fs.writeFileSync(sourcePath, sourceContent); + + const internals = service as unknown as { + resolveMaintainableSessionSnapshot: (id: string) => Promise; + }; + const resolveSnapshot = + internals.resolveMaintainableSessionSnapshot.bind(service); + vi.spyOn( + internals, + 'resolveMaintainableSessionSnapshot', + ).mockImplementationOnce(async (id) => { + fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + fs.writeFileSync(targetPath, targetContent); + return resolveSnapshot(id); + }); + + const result = await service[`${action}Sessions`]([sessionId]); + + expect(result.errors).toHaveLength(1); + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(sourceContent); + expect(fs.readFileSync(targetPath, 'utf8')).toBe(targetContent); + }, + ); + + it.each(['delete', 'unarchive'] as const)( + 'does not %s a readable archived transcript replaced after validation', + async (action) => { + const { service, sessionId, paths, cwd } = createHarness('', 'archived'); + fs.writeFileSync( + paths.archived, + `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`, + ); + const replacement = 'replacement'; + const assertStorageUnchanged = async () => { + fs.renameSync(paths.archived, `${paths.archived}.original`); + fs.writeFileSync(paths.archived, replacement); + }; + + if (action === 'delete') { + await expect( + service.removeSession(sessionId, { assertStorageUnchanged }), + ).rejects.toThrow('changed outside its active writer'); + } else { + const result = await service.unarchiveSessions([sessionId], { + assertStorageUnchanged, + }); + expect(result.errors[0]?.error.message).toContain( + 'changed outside its active writer', + ); + } + + expect(fs.readFileSync(paths.archived, 'utf8')).toBe(replacement); + expect(fs.existsSync(paths.active)).toBe(false); + }, + ); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'does not %s a transcript that disappears and reappears during classification', + async (action) => { + const state = action === 'archive' ? 'active' : 'archived'; + const { service, sessionId, paths, cwd } = createHarness('', state); + const sourcePath = paths[state]; + const originalContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + fs.writeFileSync(sourcePath, originalContent); + const replacement = 'replacement'; + + const internals = service as unknown as { + resolveMaintainableSessionSnapshot: (id: string) => Promise; + }; + const resolveSnapshot = + internals.resolveMaintainableSessionSnapshot.bind(service); + vi.spyOn( + internals, + 'resolveMaintainableSessionSnapshot', + ).mockImplementationOnce(async (id) => { + fs.renameSync(sourcePath, `${sourcePath}.original`); + const snapshot = await resolveSnapshot(id); + fs.writeFileSync(sourcePath, replacement); + return snapshot; + }); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).resolves.toBe(false); + } else { + const result = await service[`${action}Sessions`]([sessionId]); + expect(result).toMatchObject({ + notFound: [sessionId], + errors: [], + }); + } + + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(replacement); + expect(fs.readFileSync(`${sourcePath}.original`, 'utf8')).toBe( + originalContent, + ); + expect( + fs.existsSync(action === 'archive' ? paths.archived : paths.active), + ).toBe(false); + }, + ); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'does not %s a readable transcript whose record identifies another session', + async (action) => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths, cwd } = createHarness('', state); + const sourcePath = paths[state]; + const content = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId: randomUUID(), + cwd, + })}\n`; + fs.writeFileSync(sourcePath, content); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).resolves.toBe(false); + } else { + await expect( + service[`${action}Sessions`]([sessionId]), + ).resolves.toMatchObject({ + notFound: [sessionId], + errors: [], + }); + } + + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(content); + expect( + fs.existsSync(action === 'unarchive' ? paths.active : paths.archived), + ).toBe(false); + }, + ); + + it.each([ + ['missing cwd', undefined], + ['non-string cwd', 42], + ] as const)('fails closed on a record with %s', async (_name, cwdValue) => { + for (const action of ['delete', 'archive', 'unarchive'] as const) { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths } = createHarness('', state); + const sourcePath = paths[state]; + const record = { + ...recordFor('u1', 'user', null), + sessionId, + } as Record; + if (cwdValue === undefined) { + delete record['cwd']; + } else { + record['cwd'] = cwdValue; + } + fs.writeFileSync(sourcePath, `${JSON.stringify(record)}\n`); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).rejects.toMatchObject({ + reason: 'unknown_project', + }); + } else { + const result = await service[`${action}Sessions`]([sessionId]); + expect(result.errors[0]?.error).toMatchObject({ + reason: 'unknown_project', + }); + } + expect(fs.existsSync(sourcePath)).toBe(true); + if (action !== 'delete') { + expect( + fs.existsSync(action === 'archive' ? paths.archived : paths.active), + ).toBe(false); + } + } + }); + + it.each([ + ['missing session id', undefined], + ['non-string session id', 42], + ] as const)( + 'fails closed on a foreign record with %s', + async (_name, sessionIdValue) => { + const { service, sessionId, paths } = createHarness('', 'active'); + const foreignCwd = fs.mkdtempSync(path.join(tmpRoot, 'foreign-')); + const record = { + ...recordFor('u1', 'user', null), + cwd: foreignCwd, + } as Record; + if (sessionIdValue === undefined) { + delete record['sessionId']; + } else { + record['sessionId'] = sessionIdValue; + } + const content = `${JSON.stringify(record)}\n`; + fs.writeFileSync(paths.active, content); + + await expect(service.removeSession(sessionId)).rejects.toMatchObject({ + reason: 'unknown_project', + }); + expect(fs.readFileSync(paths.active, 'utf8')).toBe(content); + }, + ); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'fails closed on mixed foreign and local storage during %s', + async (action) => { + const { service, sessionId, paths, cwd } = createHarness('', 'active'); + const foreignCwd = fs.mkdtempSync(path.join(tmpRoot, 'foreign-')); + const activeContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd: foreignCwd, + })}\n`; + const archivedContent = `${JSON.stringify({ + ...recordFor('u2', 'user', null), + sessionId, + cwd, + })}\n`; + fs.writeFileSync(paths.active, activeContent); + fs.mkdirSync(path.dirname(paths.archived), { recursive: true }); + fs.writeFileSync(paths.archived, archivedContent); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).rejects.toMatchObject({ + reason: 'ambiguous_project', + }); + } else { + const result = await service[`${action}Sessions`]([sessionId]); + expect(result.errors[0]?.error).toMatchObject({ + reason: 'ambiguous_project', + }); + } + expect(fs.readFileSync(paths.active, 'utf8')).toBe(activeContent); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe(archivedContent); + }, + ); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'fails closed when %s sees another session id without cwd ownership', + async (action) => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths } = createHarness('', state); + const sourcePath = paths[state]; + const record = { + ...recordFor('u1', 'user', null), + sessionId: randomUUID(), + } as Record; + delete record['cwd']; + const content = `${JSON.stringify(record)}\n`; + fs.writeFileSync(sourcePath, content); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).rejects.toMatchObject({ + reason: 'unknown_project', + }); + } else { + const result = await service[`${action}Sessions`]([sessionId]); + expect(result.errors[0]?.error).toMatchObject({ + reason: 'unknown_project', + }); + } + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(content); + }, + ); + + it('maintains an oversized first physical record without buffering the whole file', async () => { + const { service, sessionId, paths } = createHarness('', 'active'); + const content = 'x'.repeat(1024 * 1024 + 1); + fs.writeFileSync(paths.active, content); + + await expect( + service.getMaintainableSessionLocation(sessionId), + ).resolves.toBe('active'); + await expect(service.archiveSessions([sessionId])).resolves.toMatchObject({ + archived: [sessionId], + errors: [], + }); + expect(fs.existsSync(paths.active)).toBe(false); + expect(fs.readFileSync(paths.archived, 'utf8')).toBe(content); + }); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'does not %s a just-over-limit readable transcript from another workspace', + async (action) => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths } = createHarness('', state); + const sourcePath = paths[state]; + const foreignCwd = fs.mkdtempSync(path.join(tmpRoot, 'foreign-')); + const content = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd: foreignCwd, + filler: 'x'.repeat(1024 * 1024), + })}\n`; + expect(Buffer.byteLength(content)).toBeGreaterThan(1024 * 1024); + expect(Buffer.byteLength(content)).toBeLessThan(1024 * 1024 + 64 * 1024); + fs.writeFileSync(sourcePath, content); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).resolves.toBe(false); + } else { + await expect( + service[`${action}Sessions`]([sessionId]), + ).resolves.toMatchObject({ notFound: [sessionId], errors: [] }); + } + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(content); + expect( + fs.existsSync(action === 'unarchive' ? paths.active : paths.archived), + ).toBe(false); + }, + ); + + it('fails closed on a readable transcript whose first record exceeds the bounded read window', async () => { + const { service, sessionId, paths } = createHarness('', 'active'); + const foreignCwd = fs.mkdtempSync(path.join(tmpRoot, 'foreign-')); + const content = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd: foreignCwd, + filler: 'x'.repeat(2 * 1024 * 1024), + })}\n`; + fs.writeFileSync(paths.active, content); + + await expect( + service.getMaintainableSessionLocation(sessionId), + ).rejects.toBeInstanceOf(SessionTranscriptIdentityUnavailableError); + expect(fs.readFileSync(paths.active, 'utf8')).toBe(content); + }); + + it.each(['archive', 'unarchive'] as const)( + 'does not swallow a generation rejection at the %s ledger fence', + async (action) => { + const state = action === 'archive' ? 'active' : 'archived'; + const { service, sessionId, paths } = createHarness('transcript', state); + const sourcePath = paths[state]; + const destinationPath = + action === 'archive' ? paths.archived : paths.active; + const sourceLedger = sourcePath.replace(/\.jsonl$/, '.ledger.jsonl'); + const destinationLedger = destinationPath.replace( + /\.jsonl$/, + '.ledger.jsonl', + ); + fs.writeFileSync(sourceLedger, '{"promptId":"p1"}\n'); + const generationChanged = new Error('generation changed'); + const assertCanMutate = vi + .fn() + .mockImplementationOnce(() => undefined) + .mockImplementation(() => { + throw generationChanged; + }); + + const result = await service[`${action}Sessions`]([sessionId], { + assertCanMutate, + }); + + expect(result.errors[0]?.error).toBe(generationChanged); + expect(fs.existsSync(sourceLedger)).toBe(true); + expect(fs.existsSync(destinationLedger)).toBe(false); + }, + ); + + it('rejects an in-place rewrite during ownership classification', async () => { + const { service, sessionId, paths, cwd } = createHarness('', 'active'); + fs.writeFileSync( + paths.active, + `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`, + ); + const inode = fs.statSync(paths.active).ino; + vi.spyOn( + service as unknown as Privates, + 'sessionBelongsToCurrentProject', + ).mockImplementation(async () => { + fs.writeFileSync(paths.active, 'replacement'); + return true; + }); + + await expect( + service.getMaintainableSessionLocation(sessionId), + ).rejects.toThrow('changed outside its active writer'); + expect(fs.statSync(paths.active).ino).toBe(inode); + expect(fs.readFileSync(paths.active, 'utf8')).toBe('replacement'); + }); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'does not %s a readable transcript symlink', + async (action) => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths, cwd } = createHarness('', state); + const sourcePath = paths[state]; + const targetPath = `${sourcePath}.target`; + const targetContent = `${JSON.stringify({ + ...recordFor('u1', 'user', null), + sessionId, + cwd, + })}\n`; + fs.unlinkSync(sourcePath); + fs.writeFileSync(targetPath, targetContent); + fs.symlinkSync(targetPath, sourcePath); + + if (action === 'delete') { + await expect(service.removeSession(sessionId)).rejects.toBeInstanceOf( + SessionStorageEntryError, + ); + } else { + const result = await service[`${action}Sessions`]([sessionId]); + expect(result.errors[0]?.error).toBeInstanceOf( + SessionStorageEntryError, + ); + } + + expect(fs.lstatSync(sourcePath).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(targetPath, 'utf8')).toBe(targetContent); + }, + ); + + it.runIf(process.platform !== 'win32')( + 'rejects a transcript FIFO without waiting for a writer', + async () => { + const { service, sessionId, paths } = createHarness('', 'active'); + fs.unlinkSync(paths.active); + execFileSync('mkfifo', [paths.active]); + let writer: number | undefined; + const unblock = setTimeout(() => { + writer = fs.openSync( + paths.active, + fs.constants.O_WRONLY | (fs.constants.O_NONBLOCK ?? 0), + ); + }, 500); + const startedAt = Date.now(); + + try { + await expect( + service.getMaintainableSessionLocation(sessionId), + ).rejects.toBeInstanceOf(SessionStorageEntryError); + expect(Date.now() - startedAt).toBeLessThan(400); + } finally { + clearTimeout(unblock); + if (writer !== undefined) fs.closeSync(writer); + } + }, + ); + + it.each(['delete', 'archive', 'unarchive'] as const)( + 'does not %s a damaged transcript replaced after validation', + async (action) => { + const state = action === 'unarchive' ? 'archived' : 'active'; + const { service, sessionId, paths } = createHarness( + '{"uuid":"torn-head"', + state, + ); + const sourcePath = paths[state]; + const replacement = 'replacement'; + const assertStorageUnchanged = async () => { + fs.renameSync(sourcePath, `${sourcePath}.original`); + fs.writeFileSync(sourcePath, replacement); + }; + + if (action === 'delete') { + await expect( + service.removeSession(sessionId, { assertStorageUnchanged }), + ).rejects.toThrow('changed outside its active writer'); + } else { + const result = await service[`${action}Sessions`]([sessionId], { + assertStorageUnchanged, + }); + expect(result.errors[0]?.error.message).toContain( + 'changed outside its active writer', + ); + } + + expect(fs.readFileSync(sourcePath, 'utf8')).toBe(replacement); + expect( + fs.existsSync(action === 'archive' ? paths.archived : paths.active), + ).toBe(action !== 'archive' && state === 'active'); + }, + ); +}); + describe('SessionService.reconstructHistory (history-gap detection)', () => { // reconstructHistory is private; cast to reach it directly, matching the // pattern above. Integration point under test: the sessionService delegate diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index 9cdde468023..36bdb537e33 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -5,7 +5,11 @@ */ import fs from 'node:fs'; -import { persistUsageBeforeTranscriptDeletion } from './usageHistoryService.js'; +import { + commitUsageBeforeTranscriptDeletion, + prepareUsageBeforeTranscriptDeletion, + type PreparedUsageBeforeTranscriptDeletion, +} from './usageHistoryService.js'; import path from 'node:path'; import { Readable } from 'node:stream'; import { @@ -41,7 +45,11 @@ import * as jsonl from '../utils/jsonl-utils.js'; import { readSessionPrs, writeSessionPrs } from './session-pr-service.js'; vi.mock('./usageHistoryService.js', () => ({ - persistUsageBeforeTranscriptDeletion: vi.fn().mockResolvedValue(true), + prepareUsageBeforeTranscriptDeletion: vi.fn().mockResolvedValue({ + usagePath: '/usage.jsonl', + record: { sessionId: 'salvage-session' }, + }), + commitUsageBeforeTranscriptDeletion: vi.fn().mockReturnValue(true), })); vi.mock('node:path'); vi.mock('../utils/paths.js'); @@ -78,7 +86,15 @@ describe('SessionService', () => { sessionService = new SessionService('/test/project/root'); // Module mocks are not reset by restoreAllMocks; clear the salvage spy // so per-test call/order assertions never read stale invocations. - vi.mocked(persistUsageBeforeTranscriptDeletion).mockClear(); + vi.mocked(prepareUsageBeforeTranscriptDeletion) + .mockReset() + .mockResolvedValue({ + usagePath: '/usage.jsonl', + record: { sessionId: 'salvage-session' }, + } as PreparedUsageBeforeTranscriptDeletion); + vi.mocked(commitUsageBeforeTranscriptDeletion) + .mockReset() + .mockReturnValue(true); readdirSyncSpy = vi.spyOn(fs, 'readdirSync').mockReturnValue([]); statSyncSpy = vi.spyOn(fs, 'statSync').mockImplementation( @@ -114,6 +130,60 @@ describe('SessionService', () => { vi.mocked(jsonl.readLines).mockResolvedValue([]); vi.mocked(jsonl.parseLineTolerant).mockReturnValue([]); vi.mocked(readRuntimeStatus).mockResolvedValue(null); + + type MaintenanceInternals = { + getSessionFilePath: ( + sessionId: string, + state: 'active' | 'archived', + ) => string; + resolveMaintainableSessionSnapshot: (sessionId: string) => Promise<{ + location: 'active' | 'archived' | 'conflict' | undefined; + identities: Array<{ + state: 'active' | 'archived'; + filePath: string; + dev: number; + ino: number; + size: number; + mtimeMs: number; + ctimeMs: number; + }>; + }>; + assertMaintainableSessionUnchanged: () => void; + }; + const maintenancePrototype = + SessionService.prototype as unknown as MaintenanceInternals; + vi.spyOn( + maintenancePrototype, + 'resolveMaintainableSessionSnapshot', + ).mockImplementation(async function ( + this: MaintenanceInternals, + sessionId, + ) { + const service = this as unknown as SessionService; + const location = await service.getSessionLocation(sessionId); + const states = + location === 'conflict' + ? (['active', 'archived'] as const) + : location === undefined + ? [] + : [location]; + return { + location, + identities: states.map((state, index) => ({ + state, + filePath: this.getSessionFilePath(sessionId, state), + dev: 1, + ino: index + 1, + size: 1, + mtimeMs: 1, + ctimeMs: 1, + })), + }; + }); + vi.spyOn( + maintenancePrototype, + 'assertMaintainableSessionUnchanged', + ).mockImplementation(() => undefined); }); afterEach(() => { @@ -1698,13 +1768,17 @@ describe('SessionService', () => { expect(unlinkSyncSpy).toHaveBeenCalled(); // #7384: the usage salvage must see the transcript BEFORE it is // unlinked, or the summary is unrecoverable. - const salvage = vi.mocked(persistUsageBeforeTranscriptDeletion); + const salvage = vi.mocked(prepareUsageBeforeTranscriptDeletion); expect(salvage).toHaveBeenCalledWith( expect.stringContaining(`${sessionIdA}.jsonl`), ); expect(salvage.mock.invocationCallOrder[0]!).toBeLessThan( unlinkSyncSpy.mock.invocationCallOrder[0]!, ); + expect( + vi.mocked(commitUsageBeforeTranscriptDeletion).mock + .invocationCallOrder[0]!, + ).toBeGreaterThan(unlinkSyncSpy.mock.invocationCallOrder[0]!); expect(rmSyncSpy).toHaveBeenCalledWith( expect.stringContaining(`file-history/${sessionIdA}`), { recursive: true, force: true }, @@ -1713,7 +1787,7 @@ describe('SessionService', () => { it('still deletes the session when the usage salvage fails', async () => { // Contract: the salvage must never block deletion. - vi.mocked(persistUsageBeforeTranscriptDeletion).mockRejectedValueOnce( + vi.mocked(prepareUsageBeforeTranscriptDeletion).mockRejectedValueOnce( new Error('salvage exploded'), ); vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); @@ -1724,6 +1798,70 @@ describe('SessionService', () => { expect(unlinkSyncSpy).toHaveBeenCalled(); }); + it('does not commit usage when a mutation fence rejects deletion', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + const rejected = new Error('generation changed'); + + await expect( + sessionService.removeSession(sessionIdA, { + assertCanMutate: () => { + throw rejected; + }, + }), + ).rejects.toBe(rejected); + + expect(prepareUsageBeforeTranscriptDeletion).toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(unlinkSyncSpy).not.toHaveBeenCalled(); + }); + + it('does not commit usage when transcript deletion fails', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + const unlinkError = Object.assign(new Error('permission denied'), { + code: 'EACCES', + }); + unlinkSyncSpy.mockImplementationOnce(() => { + throw unlinkError; + }); + + await expect(sessionService.removeSession(sessionIdA)).rejects.toBe( + unlinkError, + ); + + expect(prepareUsageBeforeTranscriptDeletion).toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + }); + + it('commits usage when a later transcript deletion fails', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + existsSyncSpy.mockImplementation((filePath: fs.PathLike) => + filePath.toString().endsWith(`/chats/archive/${sessionIdA}.jsonl`), + ); + const unlinkError = Object.assign(new Error('permission denied'), { + code: 'EACCES', + }); + unlinkSyncSpy + .mockImplementationOnce(() => undefined) + .mockImplementationOnce(() => { + throw unlinkError; + }); + + await expect(sessionService.removeSession(sessionIdA)).rejects.toBe( + unlinkError, + ); + + expect(unlinkSyncSpy).toHaveBeenCalledTimes(2); + expect(commitUsageBeforeTranscriptDeletion).toHaveBeenCalledTimes(1); + const commitOrder = vi.mocked(commitUsageBeforeTranscriptDeletion).mock + .invocationCallOrder[0]!; + expect(commitOrder).toBeGreaterThan( + unlinkSyncSpy.mock.invocationCallOrder[0]!, + ); + expect(commitOrder).toBeLessThan( + unlinkSyncSpy.mock.invocationCallOrder[1]!, + ); + }); + it('should clear session organization when removing a session', async () => { const warnings: string[] = []; sessionService = new SessionService('/test/project/root', { @@ -1885,7 +2023,7 @@ describe('SessionService', () => { // the only holder of the session's history. Pin the call so removing // the "redundant-looking" archived salvage fails this test. expect( - vi.mocked(persistUsageBeforeTranscriptDeletion), + vi.mocked(prepareUsageBeforeTranscriptDeletion), ).toHaveBeenCalledWith( expect.stringContaining(`/chats/archive/${sessionIdA}.jsonl`), ); @@ -2025,6 +2163,37 @@ describe('SessionService', () => { ); }); + it('passes the generation fence to an asynchronous pr-sidecar commit', async () => { + mockActiveSessionOnly(); + existsSyncSpy.mockImplementation((filePath) => { + const value = filePath.toString(); + return ( + value.endsWith(`/chats/${sessionIdA}.pr.json`) || + value.endsWith(`/chats/archive/${sessionIdA}.pr.json`) + ); + }); + const entry = { + number: 100, + url: 'https://github.com/o/r/pull/100', + createdAt: '2026-08-20T00:00:00.000Z', + }; + vi.mocked(readSessionPrs) + .mockResolvedValueOnce([entry]) + .mockResolvedValueOnce([entry]); + const assertCanMutate = vi.fn(); + + const result = await sessionService.archiveSessions([sessionIdA], { + assertCanMutate, + }); + + expect(result.errors).toEqual([]); + expect(writeSessionPrs).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + [entry], + { assertCanCommit: assertCanMutate }, + ); + }); + it('should archive JSONL and warn when archiving worktree sidecar fails', async () => { mockActiveSessionOnly(); mockActiveWorktreeSidecarOnly(); @@ -2058,6 +2227,32 @@ describe('SessionService', () => { ); }); + it('rechecks the generation before moving the active worktree sidecar', async () => { + mockActiveSessionOnly(); + mockActiveWorktreeSidecarOnly(); + const generationChanged = new Error('generation changed'); + const assertCanMutate = vi + .fn() + .mockImplementationOnce(() => undefined) + .mockImplementation(() => { + throw generationChanged; + }); + + const result = await sessionService.archiveSessions([sessionIdA], { + assertCanMutate, + }); + + expect(result.errors[0]?.error).toBe(generationChanged); + expect(renameSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.jsonl`), + expect.stringContaining(`/chats/archive/${sessionIdA}.jsonl`), + ); + expect(renameSyncSpy).not.toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.worktree.json`), + expect.anything(), + ); + }); + it('should move the prompt ledger alongside the archived session', async () => { mockActiveSessionOnly(); existsSyncSpy.mockImplementation((filePath) => { @@ -2178,8 +2373,8 @@ describe('SessionService', () => { ); }); - it('should skip location reads when archiving known active sessions', async () => { - const getLocationSpy = vi.spyOn(sessionService, 'getSessionLocation'); + it('should archive known active sessions', async () => { + mockActiveSessionOnly(); const result = await sessionService.archiveSessions([sessionIdA], { knownLocation: 'active', @@ -2187,7 +2382,6 @@ describe('SessionService', () => { expect(result.archived).toEqual([sessionIdA]); expect(result.errors).toEqual([]); - expect(getLocationSpy).not.toHaveBeenCalled(); expect(renameSyncSpy).toHaveBeenCalledWith( expect.stringContaining(`/chats/${sessionIdA}.jsonl`), expect.stringContaining(`/chats/archive/${sessionIdA}.jsonl`), @@ -2221,6 +2415,38 @@ describe('SessionService', () => { expect(result.errors[0]?.error.message).toMatch(/conflict/i); expect(renameSyncSpy).not.toHaveBeenCalled(); }); + + it('does not commit usage when conflict repair is rejected', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + const result = await sessionService.archiveSessions([sessionIdA], { + resolveConflicts: true, + assertCanMutate: () => { + throw new Error('generation changed'); + }, + }); + + expect(result.errors).toHaveLength(1); + expect(prepareUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(unlinkSyncSpy).not.toHaveBeenCalled(); + }); + + it('does not persist a final usage snapshot during conflict repair', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + const result = await sessionService.archiveSessions([sessionIdA], { + resolveConflicts: true, + }); + + expect(result).toMatchObject({ + archived: [sessionIdA], + resolvedConflicts: [sessionIdA], + errors: [], + }); + expect(prepareUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + }); }); describe('unarchiveSessions', () => { @@ -2332,9 +2558,8 @@ describe('SessionService', () => { ); }); - it('should skip location reads when unarchiving known archived sessions', async () => { + it('should unarchive known archived sessions', async () => { mockArchivedSessionOnly(); - const getLocationSpy = vi.spyOn(sessionService, 'getSessionLocation'); const result = await sessionService.unarchiveSessions([sessionIdA], { knownLocation: 'archived', @@ -2342,13 +2567,44 @@ describe('SessionService', () => { expect(result.unarchived).toEqual([sessionIdA]); expect(result.errors).toEqual([]); - expect(getLocationSpy).not.toHaveBeenCalled(); expect(renameSyncSpy).toHaveBeenCalledWith( expect.stringContaining(`/chats/archive/${sessionIdA}.jsonl`), expect.stringContaining(`/chats/${sessionIdA}.jsonl`), ); }); + it('does not commit usage when conflict repair is rejected', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + const result = await sessionService.unarchiveSessions([sessionIdA], { + resolveConflicts: true, + assertCanMutate: () => { + throw new Error('generation changed'); + }, + }); + + expect(result.errors).toHaveLength(1); + expect(prepareUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(unlinkSyncSpy).not.toHaveBeenCalled(); + }); + + it('does not persist a final usage snapshot for the retained active copy', async () => { + vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); + + const result = await sessionService.unarchiveSessions([sessionIdA], { + resolveConflicts: true, + }); + + expect(result).toMatchObject({ + unarchived: [sessionIdA], + resolvedConflicts: [sessionIdA], + errors: [], + }); + expect(prepareUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + expect(commitUsageBeforeTranscriptDeletion).not.toHaveBeenCalled(); + }); + it('should recreate active chats directory before moving archived sessions', async () => { mockArchivedSessionOnly(); @@ -2435,6 +2691,32 @@ describe('SessionService', () => { ); }); + it('rechecks the generation before moving the archived worktree sidecar', async () => { + mockArchivedSessionOnly(); + mockArchivedWorktreeSidecarOnly(); + const generationChanged = new Error('generation changed'); + const assertCanMutate = vi + .fn() + .mockImplementationOnce(() => undefined) + .mockImplementation(() => { + throw generationChanged; + }); + + const result = await sessionService.unarchiveSessions([sessionIdA], { + assertCanMutate, + }); + + expect(result.errors[0]?.error).toBe(generationChanged); + expect(renameSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.jsonl`), + expect.stringContaining(`/chats/${sessionIdA}.jsonl`), + ); + expect(renameSyncSpy).not.toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.worktree.json`), + expect.anything(), + ); + }); + it('should move the prompt ledger back to the active directory when unarchiving', async () => { mockArchivedSessionOnly(); existsSyncSpy.mockImplementation((filePath) => { diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index a8e12f8ef66..2ea691cae90 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -5,7 +5,11 @@ */ import { Storage } from '../config/storage.js'; -import { persistUsageBeforeTranscriptDeletion } from './usageHistoryService.js'; +import { + commitUsageBeforeTranscriptDeletion, + prepareUsageBeforeTranscriptDeletion, + type PreparedUsageBeforeTranscriptDeletion, +} from './usageHistoryService.js'; import { getProjectHash } from '../utils/paths.js'; import path from 'node:path'; import fs from 'node:fs'; @@ -60,6 +64,8 @@ import { } from './session-transcript-reader.js'; import { SessionWriterLease, + SessionTranscriptChangedError, + SessionTranscriptIdentityUnavailableError, SessionWriterUnavailableError, type SessionWriterProcessKind, } from './session-writer-lease.js'; @@ -149,6 +155,44 @@ export type SessionArchiveState = 'active' | 'archived'; export type SessionLocation = SessionArchiveState | 'conflict' | undefined; +interface MaintainableSessionFileIdentity { + state: SessionArchiveState; + filePath: string; + dev: number; + ino: number; + size: number; + mtimeMs: number; + ctimeMs: number; +} + +interface MaintainableSessionSnapshot { + location: SessionLocation; + identities: MaintainableSessionFileIdentity[]; +} + +const MAX_MAINTAINABLE_FIRST_RECORD_BYTES = 1024 * 1024; + +export class SessionStorageEntryError extends Error { + override readonly name = 'SessionStorageEntryError'; + + constructor( + readonly sessionId: string, + readonly reason: + | 'non_regular' + | 'foreign_project' + | 'unknown_project' + | 'ambiguous_project', + ) { + super( + reason === 'non_regular' + ? `Session storage entry for "${sessionId}" is not a regular file.` + : reason === 'foreign_project' + ? `Session "${sessionId}" belongs to a different workspace.` + : `Session "${sessionId}" does not have unambiguous workspace ownership.`, + ); + } +} + export class SessionIdCaseConflictError extends Error { override readonly name = 'SessionIdCaseConflictError'; @@ -243,26 +287,39 @@ export interface RemoveSessionsResult { errors: Array<{ sessionId: string; error: Error }>; } +export interface RemoveSessionOptions { + assertStorageUnchanged?: () => Promise; + assertCanMutate?: () => void; +} + export interface ArchiveSessionsResult { archived: string[]; alreadyArchived: string[]; + resolvedConflicts: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: Error }>; } export interface ArchiveSessionsOptions { knownLocation?: 'active'; + resolveConflicts?: boolean; + assertStorageUnchanged?: () => Promise; + assertCanMutate?: () => void; } export interface UnarchiveSessionsResult { unarchived: string[]; alreadyActive: string[]; + resolvedConflicts: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: Error }>; } export interface UnarchiveSessionsOptions { knownLocation?: 'archived'; + resolveConflicts?: boolean; + assertStorageUnchanged?: () => Promise; + assertCanMutate?: () => void; } export interface SessionServiceOptions { @@ -515,6 +572,7 @@ export class SessionService { processKind: SessionWriterProcessKind; qwenVersion?: string | null; reclaimPolicy: 'local' | 'never'; + takeoverPolicy?: 'never' | 'certified'; }, ): Promise { if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { @@ -528,6 +586,25 @@ export class SessionService { }); } + async acquireSessionMaintenanceLease( + sessionId: string, + options: { + processKind: SessionWriterProcessKind; + qwenVersion?: string | null; + reclaimPolicy: 'local' | 'never'; + }, + ): Promise { + if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { + throw new SessionWriterUnavailableError(); + } + return SessionWriterLease.acquire({ + runtimeBaseDir: this.storage.getRuntimeBaseDir(), + sessionId, + transcriptPath: `${this.getSessionFilePath(sessionId, 'active')}.maintenance`, + ...options, + }); + } + private warn(message: string): void { debugLogger.warn(message); this.onWarning?.(message); @@ -683,6 +760,9 @@ export class SessionService { } const firstRecord = records[0]; if ( + typeof firstRecord.sessionId !== 'string' || + typeof firstRecord.cwd !== 'string' || + firstRecord.sessionId.toLowerCase() !== sessionId.toLowerCase() || !(await this.sessionBelongsToCurrentProject(sessionId, firstRecord.cwd)) ) { return undefined; @@ -809,6 +889,276 @@ export class SessionService { return undefined; } + /** + * Classifies exact-spelling transcript files for lifecycle maintenance. + * Unlike {@link getSessionLocation}, empty or damaged regular files still + * occupy their state. A readable head that explicitly belongs to another + * workspace remains unavailable to this service. + */ + async getMaintainableSessionLocation( + sessionId: string, + ): Promise { + try { + return (await this.resolveMaintainableSessionSnapshot(sessionId, false)) + .location; + } catch (error) { + if ( + error instanceof SessionStorageEntryError && + error.reason === 'foreign_project' + ) { + return undefined; + } + throw error; + } + } + + private async resolveMaintainableSessionSnapshot( + sessionId: string, + captureIdentity = true, + ): Promise { + if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { + return { location: undefined, identities: [] }; + } + + const identities: MaintainableSessionFileIdentity[] = []; + let hasForeignState = false; + for (const state of ['active', 'archived'] as const) { + try { + const identity = await this.readMaintainableSessionIdentity( + sessionId, + state, + captureIdentity, + ); + if (identity) identities.push(identity); + } catch (error) { + if ( + error instanceof SessionStorageEntryError && + error.reason === 'foreign_project' + ) { + hasForeignState = true; + continue; + } + throw error; + } + } + if (hasForeignState) { + if (identities.length === 0) { + throw new SessionStorageEntryError(sessionId, 'foreign_project'); + } + throw new SessionStorageEntryError(sessionId, 'ambiguous_project'); + } + const active = identities.some((identity) => identity.state === 'active'); + const archived = identities.some( + (identity) => identity.state === 'archived', + ); + return { + location: + active && archived + ? 'conflict' + : active + ? 'active' + : archived + ? 'archived' + : undefined, + identities, + }; + } + + private async readMaintainableSessionIdentity( + sessionId: string, + state: SessionArchiveState, + captureIdentity: boolean, + ): Promise { + const filePath = this.getSessionFilePath(sessionId, state); + let fileHandle: fs.promises.FileHandle; + try { + fileHandle = await fs.promises.open( + filePath, + fs.constants.O_RDONLY | + (process.platform === 'win32' + ? 0 + : (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_NONBLOCK ?? 0)), + ); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ENOENT') return undefined; + if (code === 'ELOOP') { + throw new SessionStorageEntryError(sessionId, 'non_regular'); + } + throw error; + } + + try { + const opened = this.maintainableSessionIdentity( + sessionId, + state, + filePath, + await fileHandle.stat(), + ); + const firstRecord = await this.readFirstMaintainableRecord( + fileHandle, + filePath, + ); + if (firstRecord) { + if ( + typeof firstRecord.sessionId !== 'string' || + typeof firstRecord.cwd !== 'string' + ) { + throw new SessionStorageEntryError(sessionId, 'unknown_project'); + } + if (firstRecord.sessionId.toLowerCase() !== sessionId.toLowerCase()) { + throw new SessionStorageEntryError(sessionId, 'foreign_project'); + } + if ( + !(await this.sessionBelongsToCurrentProject( + firstRecord.sessionId, + firstRecord.cwd, + )) + ) { + throw new SessionStorageEntryError(sessionId, 'foreign_project'); + } + } + + const current = this.maintainableSessionIdentity( + sessionId, + state, + filePath, + await fs.promises.lstat(filePath), + ); + const stillOpened = this.maintainableSessionIdentity( + sessionId, + state, + filePath, + await fileHandle.stat(), + ); + if ( + !this.sameMaintainableSessionIdentity(opened, current) || + !this.sameMaintainableSessionIdentity(opened, stillOpened) + ) { + throw new SessionTranscriptChangedError(); + } + return captureIdentity ? current : opened; + } finally { + await fileHandle.close(); + } + } + + private async readFirstMaintainableRecord( + fileHandle: fs.promises.FileHandle, + filePath: string, + ): Promise | undefined> { + const buffer = Buffer.allocUnsafe(64 * 1024); + const decoder = new TextDecoder(); + let pending = ''; + while (true) { + const { bytesRead } = await fileHandle.read(buffer); + pending += decoder.decode(buffer.subarray(0, bytesRead), { + stream: bytesRead > 0, + }); + let newline = pending.indexOf('\n'); + const firstLineLength = newline >= 0 ? newline : pending.length; + if ( + firstLineLength > MAX_MAINTAINABLE_FIRST_RECORD_BYTES && + newline < 0 + ) { + const prefix = pending.trimStart(); + if (!prefix || prefix.startsWith('{')) { + throw new SessionTranscriptIdentityUnavailableError(); + } + return undefined; + } + while (newline >= 0) { + const line = pending.slice(0, newline).trim(); + pending = pending.slice(newline + 1); + if (line) { + const record = jsonl.parseLineTolerant>( + line, + filePath, + )[0]; + if (record) return record; + } + newline = pending.indexOf('\n'); + } + if (bytesRead === 0) { + if (!pending.trim()) return undefined; + return jsonl.parseLineTolerant>( + pending.trim(), + filePath, + )[0]; + } + } + } + + private maintainableSessionIdentity( + sessionId: string, + state: SessionArchiveState, + filePath: string, + stats: fs.Stats, + ): MaintainableSessionFileIdentity { + if (!stats.isFile()) { + throw new SessionStorageEntryError(sessionId, 'non_regular'); + } + if (!hasVerifiableInode(stats.ino)) { + throw new SessionTranscriptIdentityUnavailableError(); + } + return { + state, + filePath, + dev: stats.dev, + ino: stats.ino, + size: stats.size, + mtimeMs: stats.mtimeMs, + ctimeMs: stats.ctimeMs, + }; + } + + private sameMaintainableSessionIdentity( + expected: MaintainableSessionFileIdentity, + actual: MaintainableSessionFileIdentity, + ): boolean { + return ( + expected.state === actual.state && + expected.filePath === actual.filePath && + expected.dev === actual.dev && + expected.ino === actual.ino && + expected.size === actual.size && + expected.mtimeMs === actual.mtimeMs && + expected.ctimeMs === actual.ctimeMs + ); + } + + private assertMaintainableSessionUnchanged( + sessionId: string, + snapshot: MaintainableSessionSnapshot, + ): void { + for (const state of ['active', 'archived'] as const) { + const expected = snapshot.identities.find( + (identity) => identity.state === state, + ); + const filePath = this.getSessionFilePath(sessionId, state); + let stats: fs.Stats; + try { + stats = fs.lstatSync(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + if (!expected) continue; + throw new SessionTranscriptChangedError(); + } + throw error; + } + if (!expected) throw new SessionTranscriptChangedError(); + const actual = this.maintainableSessionIdentity( + sessionId, + state, + filePath, + stats, + ); + if (!this.sameMaintainableSessionIdentity(expected, actual)) { + throw new SessionTranscriptChangedError(); + } + } + } + /** * Finds a persisted session whose UUID filename differs only by case. * Legacy CLI sessions may have been written with `uuidgen`'s uppercase @@ -985,12 +1335,26 @@ export class SessionService { ); } - private async removeSessionOrganization(sessionId: string): Promise { + private async removeSessionOrganization( + sessionId: string, + assertCanMutate?: () => void, + ): Promise { try { - await new SessionOrganizationService(this.projectRoot, (message) => { - this.warn(message); - }).removeSession(sessionId); + const service = new SessionOrganizationService( + this.projectRoot, + (message) => { + this.warn(message); + }, + ); + if (assertCanMutate) { + await service.removeSession(sessionId, { + assertCanCommit: assertCanMutate, + }); + } else { + await service.removeSession(sessionId); + } } catch (error) { + assertCanMutate?.(); this.warn( `removeSession: failed to clear session organization for ${sessionId}: ${error}`, ); @@ -1070,12 +1434,14 @@ export class SessionService { private async movePrSidecar( sourcePath: string, destinationPath: string, + assertCanMutate?: () => void, ): Promise { if (!fs.existsSync(sourcePath)) { return; } fs.mkdirSync(path.dirname(destinationPath), { recursive: true }); if (!fs.existsSync(destinationPath)) { + assertCanMutate?.(); fs.renameSync(sourcePath, destinationPath); return; } @@ -1084,8 +1450,15 @@ export class SessionService { (await readSessionPrs(sourcePath)) ?? [], ); if (merged.length > 0) { - await writeSessionPrs(destinationPath, merged); + if (assertCanMutate) { + await writeSessionPrs(destinationPath, merged, { + assertCanCommit: assertCanMutate, + }); + } else { + await writeSessionPrs(destinationPath, merged); + } } + assertCanMutate?.(); fs.unlinkSync(sourcePath); } @@ -1852,76 +2225,98 @@ export class SessionService { * @param sessionId The session ID to remove * @returns true if removed, false if not found */ - async removeSession(sessionId: string): Promise { - const removed = await this.removeSessionFiles(sessionId); + async removeSession( + sessionId: string, + options: RemoveSessionOptions = {}, + ): Promise { + const removed = await this.removeSessionFiles(sessionId, options); if (removed) { - await this.removeSessionOrganization(sessionId); + await this.removeSessionOrganization(sessionId, options.assertCanMutate); } return removed; } /** * Usage salvage wrapper enforcing the "never blocks deletion" contract at - * the call site: persistUsageBeforeTranscriptDeletion catches its own - * errors, but this second layer keeps the guarantee structural rather - * than an implementation detail of another module. + * the call site. Preparation is read-only so rejected lifecycle mutations + * cannot pin an incomplete usage summary; commit happens only after every + * mutation fence passes. */ - private async salvageUsageBestEffort(transcriptPath: string): Promise { + private async prepareUsageSalvageBestEffort( + transcriptPath: string, + ): Promise { try { - await persistUsageBeforeTranscriptDeletion(transcriptPath); + return await prepareUsageBeforeTranscriptDeletion(transcriptPath); } catch (error) { this.warn( `usage salvage failed for ${transcriptPath}: ${error}; deleting anyway`, ); + return null; + } + } + + private commitUsageSalvageBestEffort( + prepared: PreparedUsageBeforeTranscriptDeletion | null, + ): void { + if (!prepared) return; + try { + commitUsageBeforeTranscriptDeletion(prepared); + } catch (error) { + this.warn(`usage salvage commit failed: ${error}; deleting anyway`); } } - private async removeSessionFiles(sessionId: string): Promise { + private async removeSessionFiles( + sessionId: string, + options: RemoveSessionOptions = {}, + ): Promise { if (!SESSION_FILE_PATTERN.test(`${sessionId}.jsonl`)) { return false; } try { - const activePath = this.getSessionFilePath(sessionId, 'active'); - const active = await this.readProjectSessionHead(sessionId, activePath); - if (active) { - // #7384: the usage-history rebuild reads transcripts, so salvage - // the session's usage summary before the file is gone. Never - // blocks deletion (the salvage swallows its own errors). - await this.salvageUsageBestEffort(activePath); - this.removeFileIfExists(activePath); - const archivedPath = this.getSessionFilePath(sessionId, 'archived'); - if (fs.existsSync(archivedPath)) { - // When both copies co-exist (e.g. an interrupted archive), the - // active transcript may hold no telemetry while the archived one - // carries the session's history — salvage it too. The dedup - // guard inside the salvage makes this a no-op whenever the - // active copy already produced a record. - await this.salvageUsageBestEffort(archivedPath); - this.removeFileIfExists(archivedPath); + const physicalSnapshot = + await this.resolveMaintainableSessionSnapshot(sessionId); + if (physicalSnapshot.location === undefined) return false; + const preparedUsage = new Map< + string, + PreparedUsageBeforeTranscriptDeletion + >(); + const preparedSessionIds = new Set(); + for (const identity of physicalSnapshot.identities) { + const prepared = await this.prepareUsageSalvageBestEffort( + identity.filePath, + ); + if (prepared && !preparedSessionIds.has(prepared.record.sessionId)) { + preparedUsage.set(identity.filePath, prepared); + preparedSessionIds.add(prepared.record.sessionId); } - this.removeWorktreeSidecars(sessionId); - this.removePrSidecars(sessionId); - this.removePromptLedgers(sessionId); - this.removeFileHistoryBackups(sessionId); - return true; } - const archivedPath = this.getSessionFilePath(sessionId, 'archived'); - const archived = await this.readProjectSessionHead( - sessionId, - archivedPath, - ); - if (!archived) { - return false; + await options.assertStorageUnchanged?.(); + options.assertCanMutate?.(); + this.assertMaintainableSessionUnchanged(sessionId, physicalSnapshot); + for (const identity of physicalSnapshot.identities) { + this.removeFileIfExists(identity.filePath); + this.commitUsageSalvageBestEffort( + preparedUsage.get(identity.filePath) ?? null, + ); } - await this.salvageUsageBestEffort(archivedPath); - this.removeFileIfExists(archivedPath); + options.assertCanMutate?.(); this.removeWorktreeSidecars(sessionId); + options.assertCanMutate?.(); this.removePrSidecars(sessionId); + options.assertCanMutate?.(); this.removePromptLedgers(sessionId); + options.assertCanMutate?.(); this.removeFileHistoryBackups(sessionId); return true; } catch (error) { + if ( + error instanceof SessionStorageEntryError && + error.reason === 'foreign_project' + ) { + return false; + } if ((error as NodeJS.ErrnoException).code === 'ENOENT') { return false; } @@ -1935,6 +2330,7 @@ export class SessionService { ): Promise { const archived: string[] = []; const alreadyArchived: string[] = []; + const resolvedConflicts: string[] = []; const notFound: string[] = []; const errors: Array<{ sessionId: string; error: Error }> = []; @@ -1944,27 +2340,56 @@ export class SessionService { notFound.push(sessionId); continue; } - if (options.knownLocation !== 'active') { - const location = await this.getSessionLocation(sessionId); - if (location === undefined) { - notFound.push(sessionId); - continue; - } - if (location === 'archived') { - alreadyArchived.push(sessionId); - continue; - } - if (location === 'conflict') { + const snapshot = + await this.resolveMaintainableSessionSnapshot(sessionId); + const location = snapshot.location; + if (location === undefined) { + notFound.push(sessionId); + continue; + } + if (location === 'archived') { + alreadyArchived.push(sessionId); + continue; + } + if (location === 'conflict') { + if (!options.resolveConflicts) { throw new Error(`Session archive conflict: ${sessionId}`); } + const active = snapshot.identities.find( + (identity) => identity.state === 'active', + )!; + await options.assertStorageUnchanged?.(); + options.assertCanMutate?.(); + this.assertMaintainableSessionUnchanged(sessionId, snapshot); + this.removeFileIfExists(active.filePath); + try { + options.assertCanMutate?.(); + await this.movePrSidecar( + this.getPrSessionPathForState(sessionId, 'active'), + this.getPrSessionPathForState(sessionId, 'archived'), + options.assertCanMutate, + ); + } catch (sidecarError) { + options.assertCanMutate?.(); + this.warn( + `archiveSessions: failed to merge active pr sidecar for ${sessionId}: ${sidecarError}`, + ); + } + options.assertCanMutate?.(); + this.removeFileIfExists( + this.getWorktreeSessionPathForState(sessionId, 'active'), + ); + options.assertCanMutate?.(); + this.removeFileIfExists( + this.getPromptLedgerPathForState(sessionId, 'active'), + ); + archived.push(sessionId); + resolvedConflicts.push(sessionId); + continue; } const sourcePath = this.getSessionFilePath(sessionId, 'active'); const targetPath = this.getSessionFilePath(sessionId, 'archived'); - if (fs.existsSync(targetPath)) { - throw new Error(`Session archive conflict: ${sessionId}`); - } - fs.mkdirSync(this.getArchiveChatsDir(), { recursive: true }); const activeSidecar = this.getWorktreeSessionPathForState( sessionId, @@ -1982,11 +2407,15 @@ export class SessionService { sessionId, 'archived', ); + await options.assertStorageUnchanged?.(); + options.assertCanMutate?.(); + this.assertMaintainableSessionUnchanged(sessionId, snapshot); try { fs.renameSync(sourcePath, targetPath); } catch (error) { throw this.sessionFileMoveError('archive', error); } + options.assertCanMutate?.(); try { this.moveOptionalFile(activeSidecar, archivedSidecar); } catch (sidecarError) { @@ -1998,21 +2427,32 @@ export class SessionService { await this.movePrSidecar( this.getPrSessionPathForState(sessionId, 'active'), this.getPrSessionPathForState(sessionId, 'archived'), + options.assertCanMutate, ); } catch (sidecarError) { + options.assertCanMutate?.(); this.warn( `archiveSessions: failed to move pr sidecar for ${sessionId}: ${sidecarError}`, ); } try { + options.assertCanMutate?.(); this.moveLedgerSidecar(activeLedger, archivedLedger); } catch (ledgerError) { + options.assertCanMutate?.(); this.warn( `archiveSessions: failed to move prompt ledger for ${sessionId} from ${activeLedger} to ${archivedLedger}: ${ledgerError}`, ); } archived.push(sessionId); } catch (error) { + if ( + error instanceof SessionStorageEntryError && + error.reason === 'foreign_project' + ) { + notFound.push(sessionId); + continue; + } errors.push({ sessionId, error: error instanceof Error ? error : new Error(String(error)), @@ -2020,7 +2460,13 @@ export class SessionService { } } - return { archived, alreadyArchived, notFound, errors }; + return { + archived, + alreadyArchived, + resolvedConflicts, + notFound, + errors, + }; } async unarchiveSessions( @@ -2029,32 +2475,62 @@ export class SessionService { ): Promise { const unarchived: string[] = []; const alreadyActive: string[] = []; + const resolvedConflicts: string[] = []; const notFound: string[] = []; const errors: Array<{ sessionId: string; error: Error }> = []; for (const sessionId of [...new Set(sessionIds)]) { try { - if (options.knownLocation !== 'archived') { - const location = await this.getSessionLocation(sessionId); - if (location === undefined) { - notFound.push(sessionId); - continue; - } - if (location === 'active') { - alreadyActive.push(sessionId); - continue; - } - if (location === 'conflict') { + const snapshot = + await this.resolveMaintainableSessionSnapshot(sessionId); + const location = snapshot.location; + if (location === undefined) { + notFound.push(sessionId); + continue; + } + if (location === 'active') { + alreadyActive.push(sessionId); + continue; + } + if (location === 'conflict') { + if (!options.resolveConflicts) { throw new Error(`Session archive conflict: ${sessionId}`); } + const archived = snapshot.identities.find( + (identity) => identity.state === 'archived', + )!; + await options.assertStorageUnchanged?.(); + options.assertCanMutate?.(); + this.assertMaintainableSessionUnchanged(sessionId, snapshot); + this.removeFileIfExists(archived.filePath); + try { + options.assertCanMutate?.(); + await this.movePrSidecar( + this.getPrSessionPathForState(sessionId, 'archived'), + this.getPrSessionPathForState(sessionId, 'active'), + options.assertCanMutate, + ); + } catch (sidecarError) { + options.assertCanMutate?.(); + this.warn( + `unarchiveSessions: failed to merge archived pr sidecar for ${sessionId}: ${sidecarError}`, + ); + } + options.assertCanMutate?.(); + this.removeFileIfExists( + this.getWorktreeSessionPathForState(sessionId, 'archived'), + ); + options.assertCanMutate?.(); + this.removeFileIfExists( + this.getPromptLedgerPathForState(sessionId, 'archived'), + ); + unarchived.push(sessionId); + resolvedConflicts.push(sessionId); + continue; } const sourcePath = this.getSessionFilePath(sessionId, 'archived'); const targetPath = this.getSessionFilePath(sessionId, 'active'); - if (fs.existsSync(targetPath)) { - throw new Error(`Session archive conflict: ${sessionId}`); - } - const archivedSidecar = this.getWorktreeSessionPathForState( sessionId, 'archived', @@ -2064,11 +2540,15 @@ export class SessionService { 'active', ); fs.mkdirSync(path.dirname(targetPath), { recursive: true }); + await options.assertStorageUnchanged?.(); + options.assertCanMutate?.(); + this.assertMaintainableSessionUnchanged(sessionId, snapshot); try { fs.renameSync(sourcePath, targetPath); } catch (error) { throw this.sessionFileMoveError('unarchive', error); } + options.assertCanMutate?.(); try { this.moveOptionalFile(archivedSidecar, activeSidecar); } catch (sidecarError) { @@ -2080,8 +2560,10 @@ export class SessionService { await this.movePrSidecar( this.getPrSessionPathForState(sessionId, 'archived'), this.getPrSessionPathForState(sessionId, 'active'), + options.assertCanMutate, ); } catch (sidecarError) { + options.assertCanMutate?.(); this.warn( `unarchiveSessions: failed to move pr sidecar for ${sessionId}: ${sidecarError}`, ); @@ -2095,14 +2577,23 @@ export class SessionService { 'active', ); try { + options.assertCanMutate?.(); this.moveLedgerSidecar(archivedLedger, activeLedger); } catch (ledgerError) { + options.assertCanMutate?.(); this.warn( `unarchiveSessions: failed to move prompt ledger for ${sessionId} from ${archivedLedger} to ${activeLedger}: ${ledgerError}`, ); } unarchived.push(sessionId); } catch (error) { + if ( + error instanceof SessionStorageEntryError && + error.reason === 'foreign_project' + ) { + notFound.push(sessionId); + continue; + } errors.push({ sessionId, error: error instanceof Error ? error : new Error(String(error)), @@ -2110,7 +2601,13 @@ export class SessionService { } } - return { unarchived, alreadyActive, notFound, errors }; + return { + unarchived, + alreadyActive, + resolvedConflicts, + notFound, + errors, + }; } /** diff --git a/packages/core/src/services/usageHistoryService.test.ts b/packages/core/src/services/usageHistoryService.test.ts index f7664fe5831..2820e980a51 100644 --- a/packages/core/src/services/usageHistoryService.test.ts +++ b/packages/core/src/services/usageHistoryService.test.ts @@ -15,6 +15,8 @@ import { loadUsageHistoryWithLive, persistSessionUsage, persistUsageBeforeTranscriptDeletion, + prepareUsageBeforeTranscriptDeletion, + commitUsageBeforeTranscriptDeletion, } from './usageHistoryService.js'; import { ToolCallDecision } from '../telemetry/tool-call-decision.js'; import type { SessionMetrics } from '../telemetry/uiTelemetry.js'; @@ -908,6 +910,24 @@ describe('persistUsageBeforeTranscriptDeletion (issue #7384)', () => { expect(lines).toHaveLength(1); }); + it('does not append stale salvage after authoritative usage is persisted', async () => { + const sessionId = 'sess-salvage-race'; + const filePath = plantTranscript(sessionId, true); + const prepared = await prepareUsageBeforeTranscriptDeletion(filePath); + expect(prepared).not.toBeNull(); + persistSessionUsage({ + sessionId, + project: '/salvage/project', + startTime: new Date('2026-07-01T00:00:00Z'), + endTime: new Date('2026-07-01T00:01:00Z'), + metrics: makeMetrics(), + }); + + expect(commitUsageBeforeTranscriptDeletion(prepared!)).toBe(false); + const lines = fs.readFileSync(usagePath(), 'utf8').trim().split('\n'); + expect(lines).toHaveLength(1); + }); + it('returns false for a transcript with no telemetry and writes nothing', async () => { const filePath = plantTranscript('sess-salvage-3', false); await expect(persistUsageBeforeTranscriptDeletion(filePath)).resolves.toBe( diff --git a/packages/core/src/services/usageHistoryService.ts b/packages/core/src/services/usageHistoryService.ts index e17233baf8e..44d1b58e8dd 100644 --- a/packages/core/src/services/usageHistoryService.ts +++ b/packages/core/src/services/usageHistoryService.ts @@ -76,6 +76,11 @@ export interface UsageSummaryRecord { }; } +export interface PreparedUsageBeforeTranscriptDeletion { + usagePath: string; + record: UsageSummaryRecord; +} + export type TimeRange = 'today' | 'week' | 'month' | 'all'; export interface AggregatedReport { @@ -270,32 +275,20 @@ function summarizeTranscript( }; } -/** - * Salvages a session's usage summary into `usage_record.jsonl` right before - * its transcript is deleted (#7384): the usage-history rebuild reads - * transcripts, so deleting a session that was never `/clear`ed or cleanly - * exited previously erased its usage from the records forever. - * - * Never throws — deletion must proceed even when salvage fails — and skips - * the write when the persisted history already carries a record for the - * session (a `/clear` or exit already wrote the authoritative summary; - * duplicating it would re-open #4994). Returns true when a record was - * written. - */ -export async function persistUsageBeforeTranscriptDeletion( +export async function prepareUsageBeforeTranscriptDeletion( transcriptPath: string, -): Promise { +): Promise { try { const records = await jsonl.read(transcriptPath); const summarized = summarizeTranscript(records); - if (!summarized?.record) return false; + if (!summarized?.record) return null; const usagePath = getUsageHistoryPath(); try { if (fs.existsSync(usagePath)) { const existing = await jsonl.read(usagePath); if (existing.some((r) => r?.sessionId === summarized.sessionId)) { - return false; + return null; } } } catch (e) { @@ -306,14 +299,54 @@ export async function persistUsageBeforeTranscriptDeletion( `persistUsageBeforeTranscriptDeletion: cannot read history: ${e}`, ); } - jsonl.writeLineSync(usagePath, summarized.record); + return { usagePath, record: summarized.record }; + } catch (e) { + debugLogger.debug(`prepareUsageBeforeTranscriptDeletion: ${e}`); + return null; + } +} + +export function commitUsageBeforeTranscriptDeletion( + prepared: PreparedUsageBeforeTranscriptDeletion, +): boolean { + try { + if (fs.existsSync(prepared.usagePath)) { + const alreadyPersisted = fs + .readFileSync(prepared.usagePath, 'utf8') + .split(/\r?\n/) + .some((line) => { + const trimmed = line.trim(); + return ( + trimmed.length > 0 && + jsonl + .parseLineTolerant( + trimmed, + prepared.usagePath, + ) + .some((record) => record.sessionId === prepared.record.sessionId) + ); + }); + if (alreadyPersisted) return false; + } + jsonl.writeLineSync(prepared.usagePath, prepared.record); return true; } catch (e) { - debugLogger.debug(`persistUsageBeforeTranscriptDeletion: ${e}`); + debugLogger.debug(`commitUsageBeforeTranscriptDeletion: ${e}`); return false; } } +/** + * Salvages a session's usage before transcript deletion (#7384). Never throws, + * and skips sessions that already have a persisted authoritative summary. + */ +export async function persistUsageBeforeTranscriptDeletion( + transcriptPath: string, +): Promise { + const prepared = await prepareUsageBeforeTranscriptDeletion(transcriptPath); + return prepared ? commitUsageBeforeTranscriptDeletion(prepared) : false; +} + interface RebuildFromSessionJsonlOptions { /** * Session to exclude from the one-time persistence migration (the caller's diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 7ea1dbd9aba..8a4d7777a32 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -5019,14 +5019,25 @@ export class DaemonClient { async archiveSessionsData( sessionIds: string[], + clientIdOrOptions?: string | { resolveConflicts?: boolean }, clientId?: string, ): Promise { + const options = + typeof clientIdOrOptions === 'object' ? clientIdOrOptions : undefined; return await this.fetchWithTimeout( `${this.baseUrl}/sessions/archive`, { method: 'POST', - headers: this.headers({ 'Content-Type': 'application/json' }, clientId), - body: JSON.stringify({ sessionIds }), + headers: this.headers( + { 'Content-Type': 'application/json' }, + typeof clientIdOrOptions === 'string' ? clientIdOrOptions : clientId, + ), + body: JSON.stringify({ + sessionIds, + ...(options?.resolveConflicts !== undefined + ? { resolveConflicts: options.resolveConflicts } + : {}), + }), }, async (res) => { if (res.ok) { @@ -5039,14 +5050,25 @@ export class DaemonClient { async unarchiveSessionsData( sessionIds: string[], + clientIdOrOptions?: string | { resolveConflicts?: boolean }, clientId?: string, ): Promise { + const options = + typeof clientIdOrOptions === 'object' ? clientIdOrOptions : undefined; return await this.fetchWithTimeout( `${this.baseUrl}/sessions/unarchive`, { method: 'POST', - headers: this.headers({ 'Content-Type': 'application/json' }, clientId), - body: JSON.stringify({ sessionIds }), + headers: this.headers( + { 'Content-Type': 'application/json' }, + typeof clientIdOrOptions === 'string' ? clientIdOrOptions : clientId, + ), + body: JSON.stringify({ + sessionIds, + ...(options?.resolveConflicts !== undefined + ? { resolveConflicts: options.resolveConflicts } + : {}), + }), }, async (res) => { if (res.ok) { @@ -6211,25 +6233,41 @@ export class WorkspaceDaemonClient { archiveSessionsData( sessionIds: string[], + clientIdOrOptions?: string | { resolveConflicts?: boolean }, clientId?: string, ): Promise { + const options = + typeof clientIdOrOptions === 'object' ? clientIdOrOptions : undefined; return this.post( '/sessions/archive', 'POST /workspaces/:workspace/sessions/archive', - { sessionIds }, - clientId, + { + sessionIds, + ...(options?.resolveConflicts !== undefined + ? { resolveConflicts: options.resolveConflicts } + : {}), + }, + typeof clientIdOrOptions === 'string' ? clientIdOrOptions : clientId, ); } unarchiveSessionsData( sessionIds: string[], + clientIdOrOptions?: string | { resolveConflicts?: boolean }, clientId?: string, ): Promise { + const options = + typeof clientIdOrOptions === 'object' ? clientIdOrOptions : undefined; return this.post( '/sessions/unarchive', 'POST /workspaces/:workspace/sessions/unarchive', - { sessionIds }, - clientId, + { + sessionIds, + ...(options?.resolveConflicts !== undefined + ? { resolveConflicts: options.resolveConflicts } + : {}), + }, + typeof clientIdOrOptions === 'string' ? clientIdOrOptions : clientId, ); } diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 654e569e93b..8154eca9033 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -1455,6 +1455,7 @@ export interface DaemonWorkspaceSessionInfo { export interface DaemonArchiveSessionsResult { archived: string[]; alreadyArchived: string[]; + resolvedConflicts?: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: string }>; } @@ -1462,6 +1463,7 @@ export interface DaemonArchiveSessionsResult { export interface DaemonUnarchiveSessionsResult { unarchived: string[]; alreadyActive: string[]; + resolvedConflicts?: string[]; notFound: string[]; errors: Array<{ sessionId: string; error: string }>; } diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index 0c0414dccd4..6fdadbf6839 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -3654,6 +3654,7 @@ describe('DaemonClient', () => { const result = { archived: ['s-1'], alreadyArchived: ['s-2'], + resolvedConflicts: [], notFound: [], errors: [], }; @@ -3672,6 +3673,7 @@ describe('DaemonClient', () => { const result = { unarchived: ['s-1'], alreadyActive: ['s-2'], + resolvedConflicts: [], notFound: [], errors: [], }; @@ -3688,6 +3690,31 @@ describe('DaemonClient', () => { sessionIds: ['s-1', 's-2'], }); }); + + it('sends explicit conflict repair without changing the client-id overload', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + archived: ['s-1'], + alreadyArchived: [], + resolvedConflicts: ['s-1'], + notFound: [], + errors: [], + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await client.archiveSessionsData( + ['s-1'], + { resolveConflicts: true }, + 'client-1', + ); + + expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1'); + expect(JSON.parse(calls[0]!.body!)).toEqual({ + sessionIds: ['s-1'], + resolveConflicts: true, + }); + }); }); describe('updateSessionMetadata', () => { @@ -7639,12 +7666,14 @@ describe('DaemonClient', () => { '/sessions/archive': { archived: ['s-1'], alreadyArchived: [], + resolvedConflicts: [], notFound: [], errors: [], }, '/sessions/unarchive': { unarchived: ['s-1'], alreadyActive: [], + resolvedConflicts: [], notFound: [], errors: [], }, @@ -7661,7 +7690,11 @@ describe('DaemonClient', () => { workspace.deleteSessionsData(['s-1'], 'client-1'), ).resolves.toEqual(replies['/sessions/delete']); await expect( - workspace.archiveSessionsData(['s-1'], 'client-2'), + workspace.archiveSessionsData( + ['s-1'], + { resolveConflicts: true }, + 'client-2', + ), ).resolves.toEqual(replies['/sessions/archive']); await expect( workspace.unarchiveSessionsData(['s-1'], 'client-3'), @@ -7677,9 +7710,11 @@ describe('DaemonClient', () => { 'client-2', 'client-3', ]); - for (const call of calls) { - expect(JSON.parse(call.body!)).toEqual({ sessionIds: ['s-1'] }); - } + expect(calls.map((call) => JSON.parse(call.body!))).toEqual([ + { sessionIds: ['s-1'] }, + { sessionIds: ['s-1'], resolveConflicts: true }, + { sessionIds: ['s-1'] }, + ]); }); it('workspace metadata update uses encoded direct REST and client identity', async () => {