Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions docs/design/2026-08-22-session-storage-lifecycle-repair.md
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 20 additions & 0 deletions docs/developers/daemon/08-session-lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion docs/developers/daemon/11-capabilities-versioning.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
14 changes: 9 additions & 5 deletions docs/developers/qwen-serve-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,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.

Expand Down Expand Up @@ -2432,7 +2432,7 @@ Archive one or more sessions. Archive is a state transition, not deletion: the J
Request:

```json
{ "sessionIds": ["<uuid>"] }
{ "sessionIds": ["<uuid>"], "resolveConflicts": true }
```

`sessionIds` must be a non-empty string array with at most 100 ids. Duplicates are collapsed.
Expand All @@ -2443,12 +2443,15 @@ Response:
{
"archived": ["<uuid>"],
"alreadyArchived": [],
"resolvedConflicts": ["<uuid>"],
"notFound": [],
"errors": []
}
```

`errors` entries have `{ "sessionId": "<uuid>", "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": "<uuid>", "error": "message" }`.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferring after independent triage: this is a documentation clarification, not a Critical correctness or data-loss defect. Runtime behavior already fails closed and is covered by the mixed local/foreign ownership tests. This PR is in review round 10 and is restricted to Critical fixes, so the prose clarification should be handled separately.


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`

Expand All @@ -2457,7 +2460,7 @@ Restore archived sessions to the active directory. This does not resume the sess
Request:

```json
{ "sessionIds": ["<uuid>"] }
{ "sessionIds": ["<uuid>"], "resolveConflicts": true }
```

Response:
Expand All @@ -2466,12 +2469,13 @@ Response:
{
"unarchived": ["<uuid>"],
"alreadyActive": [],
"resolvedConflicts": ["<uuid>"],
"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.

Expand Down
1 change: 1 addition & 0 deletions integration-tests/cli/qwen-serve-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ describe('qwen serve — capabilities envelope', () => {
'session_status',
'session_close',
'session_archive',
'session_storage_conflict_repair',
'session_metadata',
Comment thread
doudouOUC marked this conversation as resolved.
Resolved
'session_organization',
'session_export',
Expand Down
4 changes: 2 additions & 2 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1951,7 +1951,10 @@ export interface AcpSessionBridge {
): Promise<boolean>;

/** Delete all persisted attachments after the session itself is deleted. */
deleteSessionAttachments(sessionId: string): Promise<void>;
deleteSessionAttachments(
sessionId: string,
options?: { assertCanCommit?: () => void },
): Promise<void>;

/** Remove a queued or promoted mid-turn message. */
removeMidTurnMessage(
Expand Down
58 changes: 58 additions & 0 deletions packages/acp-bridge/src/sessionAttachments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
20 changes: 18 additions & 2 deletions packages/acp-bridge/src/sessionAttachments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -555,9 +556,11 @@ export class SessionAttachmentStore {
await fs.rm(directory, { recursive: true, force: true });
}

async delete(): Promise<void> {
async delete(options: { assertCanCommit?: () => void } = {}): Promise<void> {
options.assertCanCommit?.();
this.closing = true;
await this.waitForCopy();
options.assertCanCommit?.();
if (!this.closed) {
this.closed = true;
this.pendingItems = 0;
Expand All @@ -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 });
}
Comment thread
doudouOUC marked this conversation as resolved.
Resolved
}

private assertStored(reference: SessionAttachmentReference): void {
Expand Down
Loading
Loading