diff --git a/docs/design/2026-08-01-caller-supplied-session-id.md b/docs/design/2026-08-01-caller-supplied-session-id.md new file mode 100644 index 00000000000..3f313f70f23 --- /dev/null +++ b/docs/design/2026-08-01-caller-supplied-session-id.md @@ -0,0 +1,93 @@ +# Caller-supplied daemon session IDs + +## Context + +Daemon clients sometimes need to choose a session ID before creation so they can persist the identity atomically with their own workflow state. The existing REST implementation forwarded an optional ID, but uniqueness and recovery coordination remained local to one route and one workspace runtime. ACP, SDK, runtime replacement, and direct stdio-agent entry points could therefore observe different behavior. + +This design makes caller-supplied IDs one daemon-wide contract without changing the core session format or adding a persistent global index. + +## Contract + +A caller-supplied ID is optional. `undefined` and `null` mean that no ID was supplied. A supplied value must be a string containing an RFC-variant UUID v1-v5. The daemon normalizes it to lowercase and rejects nil UUIDs, unsupported versions, non-RFC variants, path characters, Arena `-agent-*` suffixes, and all non-string values. + +The internal session-ID validator continues to accept the existing Arena suffix. Caller validation is intentionally narrower because the public ID must remain addressable by the persisted-session and CLI resume paths. + +Supplying an ID means “create a new independent thread session with this ID.” It is not an idempotent attach operation. After an ambiguous create response, a caller should use the known ID with load or resume. + +## Ownership boundaries + +| Layer | Responsibility | +| ----------------------------- | ------------------------------------------------------------------------------------------------------------------------- | +| Shared parser | Public UUID validation and lowercase normalization; internal Arena compatibility remains separate. | +| `RequestedSessionIdAdmission` | Daemon-wide live, pending, and persisted conflict detection for create and restore. | +| REST and ACP dispatchers | Parse protocol fields, acquire admission before side effects, map errors, and verify the returned ID. | +| ACP bridge | Force direct caller-supplied creates to thread scope, forward the ID, and include it in fresh-session capacity admission. | +| stdio ACP agent | Validate before settings or filesystem access, serialize same-ID startup, and preserve the shared child on errors. | +| SDK and MCP clients | Negotiate `session_id_override`, serialize the field, and verify the success response. | + +The existing total-session admission controller remains responsible only for capacity and drain state. ID uniqueness is deliberately separate so the two policies cannot accidentally release or count each other's reservations. + +## Daemon-wide admission + +The admission component owns an in-memory map keyed by normalized session ID. A claim is either: + +- `create`, owned by one bridge generation; or +- `restore`, owned by one bridge generation with a reference count so concurrent load/resume calls on that generation can share recovery. + +Create performs these steps: + +1. Enumerate every live bridge supplied by the daemon, including draining and replaced generations that have not completed shutdown. +2. Reject any live owner or pending claim. +3. Install the pending create claim synchronously, before the first asynchronous persistence, branch, or worktree operation. +4. Under the session archive coordinator's shared lock, scan every currently registered workspace with a `SessionService` pinned to that runtime's captured `sessionRuntimeBaseDir`. +5. Reject active, archived, or worktree-backed persisted history; otherwise return an identity-bound reservation. + +Restore performs no disk scan because its purpose is to open existing history. It rejects a live or pending owner on another bridge generation, shares an existing restore claim only when it belongs to the same bridge object (the workspace spelling may differ between calls on that bridge), and otherwise installs a new restore claim. + +Reservations are idempotent and remove state only when the map still contains the exact state object they captured. This identity check prevents a delayed release from deleting a newer claim for the same ID. Restore references decrement individually and remove the state at zero. + +Bridge enumeration and persistence inspection errors fail closed with retryable `session_id_admission_unavailable`. A normal `SessionNotFoundError` means only that the bridge does not own the ID. Persistence read failures do not inherit `SessionService`'s treat-error-as-exists behavior: the scan surfaces them as retryable `503 session_id_admission_unavailable` instead of a `409` conflict. Clients should bound their 503 retries — a permanently unreadable transcript directory keeps returning 503 and never resolves on its own. + +## Runtime replacement and workspace scope + +Production passes a dynamic bridge provider backed by the runtime lifecycle array. A replaced bridge stays in that array until shutdown is confirmed, so a new generation cannot create the same ID while the old generation is draining. Enabling runtime replacement or removal without this provider is a startup error. + +Persistence scans use the current workspace registry and each runtime's fixed base directory. They do not depend on ambient storage context. This guarantees uniqueness across all workspaces currently registered with the daemon while avoiding a new global on-disk index. + +Existing duplicate history is not migrated or renamed. If a workspace containing a historical duplicate is registered later, workspace-qualified routing continues to disambiguate it; the admission guarantee applies to creates relative to the runtimes registered at admission time. + +## Transport behavior + +REST accepts `POST /session { sessionId }`. ACP accepts `session/new._meta["qwen-code/sessionId"]`. Both use the same admission instance, including primary and workspace-qualified ACP mounts. REST and ACP load/resume also share restore claims, closing cross-transport races. + +Both create paths force `sessionScope: "thread"`. After the bridge returns, the dispatcher compares the actual and requested IDs. A mismatch returns `session_id_not_honored` and removes the newly created live and persisted orphan before releasing admission. + +The stdio agent repeats validation before loading settings. It guards specified creates and non-live load/resume startup with a per-child pending set. Duplicate startup returns a structured ACP `INVALID_PARAMS` error; it never exits the process or damages sibling sessions. Core `Config` remains unchanged and still receives `throwOnSessionIdConflict` for its final disk-conflict defense. + +## Public compatibility + +The daemon advertises `session_id_override`. TypeScript, Java, and daemon MCP clients require that capability before sending a create mutation with a chosen ID. This prevents an older daemon from silently ignoring an additive field. + +TypeScript maps the field to REST JSON or ACP `_meta` depending on the active transport. Java exposes `CreateSessionRequest.Builder.sessionId(String)`. MCP exposes `session_create.session_id`. A successful response is checked again by each SDK; Java reports mismatches as `SessionCreationOutcomeUnknownException` because the unexpected session may have been created. + +Web UI consumers inherit the optional TypeScript field but do not set it. The Python SDK has no daemon client and is unchanged. + +## Error contract + +| Condition | REST | ACP | +| ------------------------------------------------------- | ------------------------------------------------------------- | --------------------------------------------------------------- | +| Invalid requested ID | `400 invalid_session_id` | `INVALID_PARAMS`, `data.httpStatus=400` | +| Create conflicts with live, pending, or persisted state | `409 session_id_conflict` | `INVALID_PARAMS`, `data.httpStatus=409` | +| Restore belongs to another runtime generation | `409 session_workspace_conflict` | `INVALID_PARAMS`, `data.httpStatus=409` | +| Live or persisted ownership cannot be checked | `503 session_id_admission_unavailable` with `retryable: true` | internal error with `data.httpStatus=503` and `retryable: true` | +| Downstream returns a different ID | `500 session_id_not_honored` | internal error with `data.httpStatus=500` | + +## Alternatives rejected + +A persistent daemon-global ID index would make future workspace registration easier to reason about, but it introduces transactional recovery, migration, and stale-entry cleanup for a feature that can be enforced from current live bridges and existing session storage. Per-route maps are smaller locally but cannot close REST/ACP or cross-workspace races. Treating create as idempotent attach would also hide ambiguous mutation outcomes and conflict with ACP `session/new` semantics. + +## Verification + +Unit coverage exercises UUID versions and variants, normalization, Arena compatibility, synchronous claims, all live bridge generations, pinned persistence targets, restore reference counting, failure release, stale releases, structured stdio errors, scope forcing, orphan cleanup, capability gates, transport mapping, and SDK response verification. + +The manual daemon scenario creates mixed-case fixed IDs through raw REST, TypeScript REST/ACP, Java, and MCP; prompts and persists the winning session; restarts and restores it; verifies cross-workspace and cross-transport conflicts; and confirms invalid direct ACP metadata neither creates files nor terminates the shared child. diff --git a/docs/developers/daemon/08-session-lifecycle.md b/docs/developers/daemon/08-session-lifecycle.md index c04ad43b4c7..59035aa7274 100644 --- a/docs/developers/daemon/08-session-lifecycle.md +++ b/docs/developers/daemon/08-session-lifecycle.md @@ -310,7 +310,7 @@ new session arrives. - `BridgeOptions.sessionScope` (default `'single'`; optional `'thread'`). - `BridgeOptions.initializeTimeoutMs` (default 10s) — ACP `initialize` handshake. - `BridgeOptions.channelIdleTimeoutMs` (default 0; reap the ACP child immediately). -- Capability tags: `session_create`, `session_scope_override`, `session_load`, `session_resume`, `unstable_session_resume` (deprecated alias), `session_list`, `session_info`, `session_close`, `session_metadata`, `session_set_model`, `client_identity`, `client_heartbeat`, `session_recap`, `session_generation`, `session_btw`, `session_context_usage`, `session_tasks`, `session_monitor_tool_correlation`, `session_stats`, `session_lsp`, `session_status`, `non_blocking_prompt`. +- Capability tags: `session_create`, `session_id_override`, `session_scope_override`, `session_load`, `session_resume`, `unstable_session_resume` (deprecated alias), `session_list`, `session_info`, `session_close`, `session_metadata`, `session_set_model`, `client_identity`, `client_heartbeat`, `session_recap`, `session_generation`, `session_btw`, `session_context_usage`, `session_tasks`, `session_monitor_tool_correlation`, `session_stats`, `session_lsp`, `session_status`, `non_blocking_prompt`. ### Stateless generation (`session_generation` capability tag) diff --git a/docs/developers/daemon/11-capabilities-versioning.md b/docs/developers/daemon/11-capabilities-versioning.md index 69beca1b272..0a147c19d46 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_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_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 65e1ec00964..adca66600e4 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -161,7 +161,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design §10). ``` -['health', 'capabilities', 'session_create', 'session_scope_override', +['health', 'capabilities', 'session_create', 'session_id_override', 'session_scope_override', 'session_load', 'session_resume', 'session_transcript', 'unstable_session_resume', 'session_list', 'session_info', 'session_prompt', 'session_mid_turn_message_mutation', @@ -204,6 +204,8 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design `session_scope_override` is the negotiation handle for the per-request `sessionScope` field on `POST /session` (see below). Older daemons silently ignore the field, so SDK clients should pre-flight `caps.features` for this tag before sending it. +`session_id_override` is the negotiation handle for the optional caller-supplied `sessionId` on `POST /session` and ACP `session/new` metadata. Clients must confirm that `caps.features` contains this tag before sending the field because older daemons may silently ignore it. + `persistent_workspace_registration` advertises durable registration for workspaces added at runtime. `POST /workspaces` accepts `{ "cwd": "/absolute/path", "persist": true }`; success includes `persisted: true`. Registrations are scoped to the daemon's canonical primary workspace under the user's Qwen home and are restored on the next daemon start. Omitting `persist` preserves process-local registration. `GET /workspace-registrations` lists the stored desired set, and `DELETE /workspace-registrations/:id` forgets an entry for the next restart without hot-removing an active runtime. `workspace_display_name` advertises optional `displayName` input on `POST /workspaces`, workspace metadata updates through `PATCH /workspaces/:workspace`, and optional display-name fields in workspace projections. Names do not participate in lookup or routing: `id` and canonical `cwd` remain the only selectors, and duplicate names are allowed. @@ -1956,6 +1958,7 @@ Request: { "cwd": "/absolute/path/to/workspace", "modelServiceId": "qwen-prod", + "sessionId": "550e8400-e29b-41d4-a716-446655440000", "sessionScope": "thread" } ``` @@ -1964,6 +1967,7 @@ Request: | ---------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `cwd` | no | Absolute path matching one registered workspace. If omitted, the route falls back to the primary workspace (read it off `/capabilities.workspaceCwd`). A mismatched non-empty `cwd` returns `400 workspace_mismatch`. When `features` contains `multi_workspace_sessions`, clients may pass any trusted `workspaces[].cwd`; otherwise only the primary workspace is accepted. Workspace paths are canonicalized via `realpathSync.native` (with a resolve-only fallback for non-existent paths) so case-insensitive filesystems don't reject sessions per spelling. | | `modelServiceId` | no | Selects which configured _model service_ the agent will route through (the back-end provider — Alibaba ModelStudio, OpenRouter, etc). If omitted the agent uses its default. If the workspace already has a session, this calls `setSessionModel` on the existing one and broadcasts `model_switched`. Distinct from `modelId` on `POST /session/:id/model`, which selects the model **within** an already-bound service. The `modelServices` array on `/capabilities` is reserved for advertising configured services; in Stage 1 it is always `[]` (the agent's default service is used and not enumerated over HTTP). | +| `sessionId` | no | RFC-variant UUID v1-v5 chosen by the caller. The daemon normalizes it to lowercase and always creates a fresh thread session; it never treats this field as an idempotent attach. Confirm that `caps.features` contains `session_id_override` before sending it because older daemons may ignore unknown fields. `null` is equivalent to omission. | | `sessionScope` | no | Per-request override for session sharing. `'single'` (the daemon-wide default) makes a second same-workspace `POST /session` reuse the existing session (`attached: true`); `'thread'` forces a fresh distinct session every call. Omit to inherit the daemon-wide default. Values outside the enum return `400 { code: 'invalid_session_scope' }`. Old daemons (pre-#4175 PR 5) silently ignore the field — pre-flight `caps.features.session_scope_override` before sending. The daemon-wide default is hardcoded to `'single'` in production today; #4175 may add a `--sessionScope` CLI flag in a follow-up. | Response: @@ -1978,6 +1982,8 @@ Response: `attached: true` means a session for that workspace already existed and you're now sharing it. +Caller-supplied IDs are unique across all currently registered workspace runtimes and every still-live bridge generation, including draining replacements. A live, pending, active, archived, or worktree-backed duplicate returns `409 session_id_conflict`. Invalid values return `400 invalid_session_id`; an unavailable live-owner or persisted-state check returns retryable `503 session_id_admission_unavailable`. Retry with bounded backoff after bridge or storage health changes; `retryable` means another attempt is safe, not that an immediate retry will succeed. If the downstream agent returns a different ID, the daemon removes that orphan and returns `500 session_id_not_honored`. After an ambiguous response, load or resume the known ID instead of retrying create as an attach. + Multi-client integrations that want independent conversations should send `sessionScope: "thread"` on each `POST /session`. Use the default `single` scope only when clients intentionally share one collaborative session; shared @@ -2000,6 +2006,28 @@ Concurrent `POST /session` calls for the same workspace are **coalesced** to one > event (covers the spawn-time `model_switch_failed` even if the > subscribe lands a few ms after the create response). +### ACP `session/new` caller-supplied ID + +ACP clients request the same behavior through the extension metadata field: + +```json +{ + "jsonrpc": "2.0", + "id": 1, + "method": "session/new", + "params": { + "cwd": "/absolute/path/to/workspace", + "_meta": { + "qwen-code/sessionId": "550E8400-E29B-41D4-A716-446655440000" + } + } +} +``` + +The response contains the normalized lowercase ID. Primary and workspace-qualified ACP mounts share admission with REST, including `session/load` and `session/resume`. Invalid IDs use ACP `INVALID_PARAMS` with `data.httpStatus=400` and `data.errorKind="invalid_session_id"`; conflicts use `data.httpStatus=409`; unavailable live-owner or persisted-state checks use `data.httpStatus=503` and `data.retryable=true`. + +An ACP-created session that never receives a prompt leaves no persisted trace, and the daemon reaps it when its owning connection closes with zero attached sessions. After that reap the same ID can be created again — that is connection lifecycle, not ID reuse: while the connection (or any attachment) is live, admission rejects the duplicate. + ### `POST /session/:id/load` Restore a persisted ACP session by id and replay its history through SSE. The path id is authoritative; any `sessionId` field in the body is ignored. Pre-flight `caps.features.session_load` — older daemons return `404` for this route. diff --git a/docs/developers/sdk-java.md b/docs/developers/sdk-java.md index 890817b4cb6..5e5c7d8f5c3 100644 --- a/docs/developers/sdk-java.md +++ b/docs/developers/sdk-java.md @@ -86,6 +86,20 @@ try (DaemonClient daemon = DaemonClient.builder() } ``` +Callers that need to allocate the session identity before creation can pass an RFC UUID v1-v5. The SDK checks `session_id_override` before the mutation and reports a different returned ID as `SessionCreationOutcomeUnknownException`: + +```java +CreateSessionRequest request = CreateSessionRequest.builder() + .sessionId("550E8400-E29B-41D4-A716-446655440000") + .build(); + +try (DaemonSessionClient session = daemon.createSession(request)) { + System.out.println(session.getSession().getSessionId()); +} +``` + +The daemon normalizes the ID to lowercase and creates a new thread session. This is not an idempotent attach; after an ambiguous create outcome, recover with the known ID rather than retrying creation. + If `qwen serve` requires authentication, add `.bearerToken(System.getenv("QWEN_SERVER_TOKEN"))` to the `DaemonClient` builder. The SDK sends the bearer on REST and SSE requests and never puts it in @@ -101,7 +115,7 @@ Creation-time model selection is intentionally not exposed by the Java daemon SD `PromptRequest.Builder.deadline(Duration)` requests a daemon-enforced prompt deadline and is accepted only when the daemon advertises `prompt_absolute_deadline`; otherwise the SDK fails before sending the prompt. The value must be between 1 and 2,147,483,647 milliseconds, matching the daemon's Node timer range. This is separate from `observationTimeout(Duration)`, which only bounds local SSE observation and never sends a cancel mutation. -Before creating a session, the SDK requires the daemon to advertise the REST transport and `session_scope_override`; this prevents an older daemon from silently ignoring the requested `thread` scope and attaching the client to a shared session. When `client_heartbeat` is advertised, an open session sends a fresh heartbeat every minute so the daemon does not reap an otherwise idle client. Set `heartbeatInterval(Duration.ZERO)` on the `DaemonClient` builder to disable this behavior, or choose a different positive interval. A heartbeat is never retried; the next scheduled heartbeat is a separate keepalive. Prompt observation is bounded to 32 concurrent prompts per client by default and can be adjusted with `maximumConcurrentPrompts`. Admission and terminal future callbacks run away from transport workers; callbacks that remain blocked consume bounded publication capacity. SSE stream cleanup is also bounded, and a close that remains blocked retains its cleanup reservation. Either condition can cause a later `startPrompt` to fail with `DaemonClientCapacityException` rather than dropping a timeout close or growing threads and queued work without limit. +Before creating a session, the SDK requires the daemon to advertise the REST transport and `session_scope_override`; this prevents an older daemon from silently ignoring the requested `thread` scope and attaching the client to a shared session. When a caller supplies a session ID, the SDK additionally requires `session_id_override` before sending the mutation. When `client_heartbeat` is advertised, an open session sends a fresh heartbeat every minute so the daemon does not reap an otherwise idle client. Set `heartbeatInterval(Duration.ZERO)` on the `DaemonClient` builder to disable this behavior, or choose a different positive interval. A heartbeat is never retried; the next scheduled heartbeat is a separate keepalive. Prompt observation is bounded to 32 concurrent prompts per client by default and can be adjusted with `maximumConcurrentPrompts`. Admission and terminal future callbacks run away from transport workers; callbacks that remain blocked consume bounded publication capacity. SSE stream cleanup is also bounded, and a close that remains blocked retains its cleanup reservation. Either condition can cause a later `startPrompt` to fail with `DaemonClientCapacityException` rather than dropping a timeout close or growing threads and queued work without limit. An indeterminate completion is an outcome boundary, not a session-reuse boundary. After `PromptAdmissionUnknownException` or `PromptOutcomeIndeterminateException`, that `DaemonSessionClient` permanently rejects further prompts even if local stream cleanup later succeeds; close or destroy the session instead. An observation timeout is published without waiting forever for a blocked stream close, while cleanup continues asynchronously and retains bounded client capacity until it finishes. diff --git a/docs/developers/sdk-typescript.md b/docs/developers/sdk-typescript.md index 4a4d6018075..147376919ec 100644 --- a/docs/developers/sdk-typescript.md +++ b/docs/developers/sdk-typescript.md @@ -162,6 +162,26 @@ an async iterable prompt, the query and its input stream remain open, so later messages from the iterable are processed normally. Use `close()` or abort the configured `AbortController` when you want to end the entire session. +## Daemon caller-supplied session IDs + +`DaemonClient.createOrAttachSession` accepts an optional `sessionId` for callers that must persist an identity before session creation: + +```typescript +import { DaemonClient } from '@qwen-code/sdk'; + +const daemon = new DaemonClient({ baseUrl: 'http://127.0.0.1:4170' }); +const session = await daemon.createOrAttachSession({ + workspaceCwd: '/path/to/project', + sessionId: '550E8400-E29B-41D4-A716-446655440000', +}); + +console.log(session.sessionId); // 550e8400-e29b-41d4-a716-446655440000 +``` + +The SDK requires the daemon's `session_id_override` capability before sending the mutation. REST mode serializes `sessionId` directly; an active ACP adapter maps it to `session/new._meta["qwen-code/sessionId"]`. The SDK verifies the success response and throws `DaemonSessionIdProtocolError` if the daemon returns a different ID. + +This option always creates a new thread session and is not an idempotent attach. If the create outcome is ambiguous, use the known ID with load or resume. Omitting the option preserves the existing create-or-attach behavior. + ## Permission Modes The SDK supports different permission modes for controlling tool execution: diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index 95d6371d580..40eb1f529af 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -34,6 +34,7 @@ import { DaemonHttpError, type DaemonSessionSummary, } from '@qwen-code/sdk'; +import { AcpWsTransport } from '@qwen-code/sdk/daemon/transports'; import { SESSION_TRANSCRIPT_MAX_INDEX_BYTES, Storage, @@ -303,6 +304,7 @@ describe('qwen serve — capabilities envelope', () => { 'daemon_status', 'capabilities', 'session_create', + 'session_id_override', 'session_scope_override', 'session_load', 'session_resume', @@ -552,6 +554,93 @@ describe('qwen serve — POST /session validation + concurrent coalescing', () = expect(res.status).toBe(400); }); + it('honors and reserves a normalized caller-supplied session ID', async () => { + const requestedId = '550E8400-E29B-41D4-A716-446655440000'; + const normalizedId = requestedId.toLowerCase(); + const created = await fetch(`${base}/session`, { + method: 'POST', + headers: { + Authorization: `Bearer ${TOKEN}`, + 'content-type': 'application/json', + }, + body: JSON.stringify({ + cwd: REPO_ROOT, + sessionId: requestedId, + sessionScope: 'single', + }), + }); + expect(created.status).toBe(200); + await expect(created.json()).resolves.toMatchObject({ + sessionId: normalizedId, + attached: false, + }); + + try { + let conflict: unknown; + try { + await client.createOrAttachSession({ + workspaceCwd: REPO_ROOT, + sessionId: normalizedId, + }); + } catch (error) { + conflict = error; + } + expect(conflict).toBeInstanceOf(DaemonHttpError); + expect((conflict as DaemonHttpError).status).toBe(409); + expect((conflict as DaemonHttpError).body).toMatchObject({ + code: 'session_id_conflict', + sessionId: normalizedId, + }); + + await client.closeSession(normalizedId); + + const sdkRest = await client.createOrAttachSession({ + workspaceCwd: REPO_ROOT, + sessionId: requestedId, + }); + expect(sdkRest).toMatchObject({ + sessionId: normalizedId, + attached: false, + }); + await client.closeSession(normalizedId); + + const acpTransport = new AcpWsTransport( + `ws://127.0.0.1:${port}/acp`, + TOKEN, + ); + const acpClient = new DaemonClient({ + baseUrl: base, + token: TOKEN, + transport: acpTransport, + }); + try { + const sdkAcp = await acpClient.createOrAttachSession({ + workspaceCwd: REPO_ROOT, + sessionId: requestedId, + }); + expect(sdkAcp).toMatchObject({ sessionId: normalizedId }); + await client.closeSession(normalizedId); + } finally { + acpClient.dispose(); + } + + const invalid = await fetch(`${base}/session`, { + method: 'POST', + headers: { + Authorization: `Bearer ${TOKEN}`, + 'content-type': 'application/json', + }, + body: JSON.stringify({ cwd: REPO_ROOT, sessionId: '../escape' }), + }); + expect(invalid.status).toBe(400); + await expect(invalid.json()).resolves.toMatchObject({ + code: 'invalid_session_id', + }); + } finally { + await client.closeSession(normalizedId).catch(() => undefined); + } + }); + it('two parallel POSTs same workspace coalesce to one session', async () => { const cwd = REPO_ROOT; const [a, b] = await Promise.all([ diff --git a/integration-tests/vitest.config.ts b/integration-tests/vitest.config.ts index 204ea4a0adf..5c8dfac1ce4 100644 --- a/integration-tests/vitest.config.ts +++ b/integration-tests/vitest.config.ts @@ -38,6 +38,10 @@ export default defineConfig({ resolve: { alias: { // Use built SDK bundle for e2e tests + '@qwen-code/sdk/daemon/transports': resolve( + __dirname, + '../packages/sdk-typescript/dist/daemon/transports.js', + ), '@qwen-code/sdk/daemon': resolve( __dirname, '../packages/sdk-typescript/dist/daemon/index.js', diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 0801c9761bf..be04d362611 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -59,7 +59,10 @@ import { EXTERNAL_TOOL_GUARD_REQUIRED_VALUE, } from './externalToolGuard.js'; import type { ChannelFactory } from './channel.js'; -import type { BridgeTelemetry } from './bridgeOptions.js'; +import type { + BridgeFreshSessionAdmissionContext, + BridgeTelemetry, +} from './bridgeOptions.js'; import { createInMemoryChannel } from './inMemoryChannel.js'; import { EventBus, type BridgeEvent } from './eventBus.js'; import { TurnBoundaryCompactionEngine } from './compactionEngine.js'; @@ -2068,23 +2071,57 @@ describe('createAcpSessionBridge', () => { expect(handles[0]?.killed).toBe(true); }); - it('injects REQUESTED_SESSION_ID_META_KEY into newSession _meta when sessionId is set', async () => { + it('forces caller-supplied session ids into fresh thread admission and ACP meta', async () => { const handles: ChannelHandle[] = []; + const admissionContexts: BridgeFreshSessionAdmissionContext[] = []; const factory: ChannelFactory = async () => { - const h = makeChannel(); + const h = makeChannel({ + newSessionImpl: async (params) => ({ + sessionId: String(params._meta?.[REQUESTED_SESSION_ID_META_KEY]), + }), + }); handles.push(h); return h.channel; }; - const bridge = makeBridge({ channelFactory: factory }); + const bridge = makeBridge({ + channelFactory: factory, + sessionScope: 'single', + freshSessionAdmission: (context) => { + admissionContexts.push(context); + return { release: vi.fn() }; + }, + }); - await bridge.spawnOrAttach({ + const first = await bridge.spawnOrAttach({ workspaceCwd: WS_A, sessionId: '550e8400-e29b-41d4-a716-446655440000', + sessionScope: 'single', + }); + const second = await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + sessionScope: 'single', }); + expect(first.attached).toBe(false); + expect(second.attached).toBe(false); + expect(first.sessionId).not.toBe(second.sessionId); + expect(handles[0]?.agent.newSessionCalls).toHaveLength(2); expect(handles[0]?.agent.newSessionCalls[0]!._meta).toMatchObject({ [REQUESTED_SESSION_ID_META_KEY]: '550e8400-e29b-41d4-a716-446655440000', }); + expect(admissionContexts).toEqual([ + { + operation: 'spawn', + workspaceCwd: WS_A, + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }, + { + operation: 'spawn', + workspaceCwd: WS_A, + sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + }, + ]); await bridge.shutdown(); }); diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index bb34333027e..c7307af41f6 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -5715,7 +5715,10 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ) { throw new InvalidSessionScopeError(req.sessionScope); } - const effectiveScope = req.sessionScope ?? defaultSessionScope; + const effectiveScope = + req.sessionId !== undefined + ? 'thread' + : (req.sessionScope ?? defaultSessionScope); const source = parseSessionSource(req.sourceType, req.sourceId); if ('error' in source) { throw new InvalidSessionMetadataError('sourceType', source.error); @@ -5881,6 +5884,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const admission = reserveFreshSession({ operation: 'spawn', workspaceCwd: workspaceKey, + ...(req.sessionId !== undefined ? { sessionId: req.sessionId } : {}), }); let admissionReleased = false; const releaseAdmissionOnce = () => { diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 64dce116fce..c9395f971a1 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -869,7 +869,7 @@ import type { McpServer, ResumeSessionResponse, } from '@agentclientprotocol/sdk'; -import { AgentSideConnection, RequestError } from '@agentclientprotocol/sdk'; +import { AgentSideConnection } from '@agentclientprotocol/sdk'; import { loadSettings, SettingScope } from '../config/settings.js'; import { resetTrustedFoldersForTesting } from '../config/trustedFolders.js'; import { @@ -2991,6 +2991,72 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('rejects invalid sessionId meta before settings access without closing the child', async () => { + await setupSessionMocks('meta-session'); + const { agent, agentPromise } = await bootAcpAgent(); + vi.mocked(loadSettings).mockClear(); + + await expect( + agent.newSession({ + cwd: '/tmp', + mcpServers: [], + _meta: { 'qwen-code/sessionId': '../../escape' }, + }), + ).rejects.toMatchObject({ + code: -32602, + data: { errorKind: 'invalid_session_id', httpStatus: 400 }, + }); + expect(loadSettings).not.toHaveBeenCalled(); + expect(loadCliConfig).not.toHaveBeenCalled(); + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + expect(loadCliConfig).toHaveBeenCalledTimes(1); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('rejects a concurrent duplicate requested sessionId without closing the child', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440001'; + const innerConfig = await setupSessionMocks(sessionId); + let releaseFirst!: () => void; + const firstGate = new Promise((resolve) => { + releaseFirst = resolve; + }); + vi.mocked(loadCliConfig) + .mockImplementationOnce(async () => { + await firstGate; + return innerConfig as unknown as Config; + }) + .mockResolvedValue(innerConfig as unknown as Config); + const { agent, agentPromise } = await bootAcpAgent(); + const request = { + cwd: '/tmp', + mcpServers: [], + _meta: { 'qwen-code/sessionId': sessionId }, + }; + + const first = agent.newSession(request); + await vi.waitFor(() => expect(loadCliConfig).toHaveBeenCalledOnce()); + await expect(agent.newSession(request)).rejects.toMatchObject({ + code: -32602, + data: { errorKind: 'session_id_conflict', sessionId }, + }); + expect(loadCliConfig).toHaveBeenCalledOnce(); + + releaseFirst(); + await expect(first).resolves.toMatchObject({ sessionId }); + vi.mocked(innerConfig.getSessionId).mockReturnValue( + '550e8400-e29b-41d4-a716-446655440002', + ); + await expect( + agent.newSession({ cwd: '/tmp', mcpServers: [] }), + ).resolves.toBeDefined(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('maps a duplicate sessionId conflict to a RequestError instead of crashing the child', async () => { const conflict = new SessionIdConflictError( '550e8400-e29b-41d4-a716-446655440000', @@ -3011,8 +3077,13 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await expect( agent.newSession({ cwd: '/tmp', mcpServers: [] }), - ).rejects.toThrow('already exists'); - expect(RequestError.invalidParams).toHaveBeenCalled(); + ).rejects.toMatchObject({ + code: -32602, + data: { + errorKind: 'session_id_conflict', + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }, + }); mockConnectionState.resolve(); await agentPromise; @@ -14160,6 +14231,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { unstable_resumeSession: ( args: Record, ) => Promise; + cancel: (args: Record) => Promise; }) | undefined; @@ -14186,6 +14258,7 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { beginCloseIfAvailable: ReturnType; waitForCloseGateToRelease: ReturnType; waitForActiveTurnsToSettle: ReturnType; + cancelPendingPrompt: ReturnType; sendUpdate: ReturnType; dispose: ReturnType; } @@ -14424,6 +14497,85 @@ describe('QwenAgent loadSession / unstable_resumeSession', () => { await agentPromise; }); + it.each(['load', 'resume'] as const)( + '%s normalizes mixed-case caller UUIDs before persisted lookup', + async (action) => { + const sessionId = '550e8400-e29b-41d4-a716-446655440000'; + const innerConfig = bindRestoreMocks({ sessionExists: true }); + innerConfig.getSessionId.mockReturnValue(sessionId); + const { agent, agentPromise } = await spawnAgent(); + + try { + const params = { + cwd: '/tmp', + sessionId: sessionId.toUpperCase(), + mcpServers: [], + }; + if (action === 'load') { + await agent.loadSession(params); + } else { + await agent.unstable_resumeSession(params); + } + + const sessionService = vi.mocked(SessionService).mock.results[0]?.value; + expect(sessionService).toBeDefined(); + expect(sessionService!.sessionExists).toHaveBeenCalledWith(sessionId); + + await agent.cancel({ sessionId: params.sessionId }); + expect(lastSessionMock?.cancelPendingPrompt).toHaveBeenCalledOnce(); + } finally { + mockConnectionState.resolve(); + await agentPromise; + } + }, + ); + + it('serializes non-live load and resume before settings or disk work', async () => { + const innerConfig = bindRestoreMocks({ sessionExists: true }); + let releaseExists!: () => void; + const existsGate = new Promise((resolve) => { + releaseExists = resolve; + }); + const sessionExists = vi.fn(async () => { + await existsGate; + return true; + }); + const loadSession = vi + .fn() + .mockImplementation(() => innerConfig.getResumedSessionData()); + vi.mocked(SessionService).mockImplementation( + () => + ({ sessionExists, loadSession }) as unknown as InstanceType< + typeof SessionService + >, + ); + const { agent, agentPromise } = await spawnAgent(); + vi.mocked(loadSettings).mockClear(); + const params = { + cwd: '/tmp', + sessionId: 'persisted-1', + mcpServers: [], + }; + + const first = agent.loadSession(params); + await vi.waitFor(() => expect(sessionExists).toHaveBeenCalledOnce()); + await expect(agent.unstable_resumeSession(params)).rejects.toMatchObject({ + code: -32602, + data: { + errorKind: 'session_id_conflict', + sessionId: 'persisted-1', + }, + }); + expect(loadSettings).toHaveBeenCalledOnce(); + expect(sessionExists).toHaveBeenCalledOnce(); + + releaseExists(); + await expect(first).resolves.toBeDefined(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('loadSession preserves initialization failure and retries deferred cleanup', async () => { const initializationError = new Error('initialize boom'); const cleanupError = new Error('shutdown boom'); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 5e2f7d00560..3a3da44147d 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -181,6 +181,10 @@ import { SettingScope, } from '../config/settings.js'; import { loadSettingsCached } from '../config/settings-cache.js'; +import { + normalizeSessionIdForLookup, + parseCallerSuppliedSessionId, +} from '../config/session-id.js'; import { loadMcpApprovals } from '../config/mcpApprovals.js'; import { assembleMcpServers } from '../config/mcpServers.js'; import { recomputeMcpGating } from '../config/hot-reload.js'; @@ -242,6 +246,7 @@ import { writeOutputLanguageAndRegisterPath, } from '../utils/languageUtils.js'; import { runWithAcpRuntimeOutputDir } from './runtimeOutputDirContext.js'; +import { ACP_ERROR_CODES } from './errorCodes.js'; import { runExitCleanup } from '../utils/cleanup.js'; import { appEvents, AppEvent } from '../utils/events.js'; import { @@ -3574,6 +3579,7 @@ function isOwnerOnlyDirectory(stats: Stats): boolean { class QwenAgent implements Agent { private sessions: Map = new Map(); + private readonly startingSessionIds = new Set(); private activePromptCalls = new Map>(); private workspaceMcpDiscoveryConfig: Config | undefined; private workspaceMcpDiscoveryPromise: Promise | undefined; @@ -4697,84 +4703,117 @@ class QwenAgent implements Agent { } } + private reserveStartingSessionId(sessionId: string): () => void { + if ( + this.sessions.has(sessionId) || + this.startingSessionIds.has(sessionId) + ) { + throw new RequestError( + ACP_ERROR_CODES.INVALID_PARAMS, + `Session ${sessionId} is already active or starting.`, + { errorKind: 'session_id_conflict', sessionId }, + ); + } + this.startingSessionIds.add(sessionId); + let released = false; + return () => { + if (released) return; + released = true; + this.startingSessionIds.delete(sessionId); + }; + } + async newSession(params: NewSessionRequest): Promise { const { cwd, mcpServers } = params; + const parsedSessionId = parseCallerSuppliedSessionId( + params._meta?.[REQUESTED_SESSION_ID_META_KEY], + ); + if (parsedSessionId.kind === 'invalid') { + throw new RequestError( + ACP_ERROR_CODES.INVALID_PARAMS, + `\`_meta["${REQUESTED_SESSION_ID_META_KEY}"]\` must be an RFC UUID v1-v5`, + { errorKind: 'invalid_session_id', httpStatus: 400 }, + ); + } const requestedSessionId = - typeof params._meta?.[REQUESTED_SESSION_ID_META_KEY] === 'string' - ? (params._meta[REQUESTED_SESSION_ID_META_KEY] as string) - : undefined; - const sessionSource = getSessionSource(params); - const parentContext = extractDaemonTraceContext(params); - return await withDaemonSpan( - 'qwen-code.daemon.session_start', - { 'qwen-code.daemon.operation': 'acp_session_new' }, - async (span) => { - const profiler = createAcpSessionStartProfiler(span); - // Per-request settings: session handlers run concurrently, and - // `this.settings` is only a "latest loaded" cache for agent-level - // readers. Threading the instance explicitly keeps a slow session - // creation from picking up whichever workspace loaded last — Session - // persists model changes through this instance, so a mix-up writes to - // another workspace's settings.json. - const settings = profiler.timeSync('settings_load', () => - loadSettingsCached(cwd), - ); - this.settings = settings; - const config = await profiler.time('config_setup', () => - this.newSessionConfig( - cwd, - mcpServers, - settings, - sessionSource, - requestedSessionId, - undefined, - shouldDeferMcpDiscovery(params) - ? { skipMcpDiscovery: true } - : undefined, - ), - ); - let session: Session; - try { - await profiler.time('auth', () => this.ensureAuthenticated(config)); - profiler.timeSync('file_system_setup', () => - this.setupFileSystem(config), + parsedSessionId.kind === 'valid' ? parsedSessionId.sessionId : undefined; + const releaseStartingSessionId = requestedSessionId + ? this.reserveStartingSessionId(requestedSessionId) + : undefined; + try { + const sessionSource = getSessionSource(params); + const parentContext = extractDaemonTraceContext(params); + return await withDaemonSpan( + 'qwen-code.daemon.session_start', + { 'qwen-code.daemon.operation': 'acp_session_new' }, + async (span) => { + const profiler = createAcpSessionStartProfiler(span); + // Per-request settings: session handlers run concurrently, and + // `this.settings` is only a "latest loaded" cache for agent-level + // readers. Threading the instance explicitly keeps a slow session + // creation from picking up whichever workspace loaded last — Session + // persists model changes through this instance, so a mix-up writes to + // another workspace's settings.json. + const settings = profiler.timeSync('settings_load', () => + loadSettingsCached(cwd), ); - session = await profiler.time('session_register', () => - this.createAndStoreSession(config, settings), + this.settings = settings; + const config = await profiler.time('config_setup', () => + this.newSessionConfig( + cwd, + mcpServers, + settings, + sessionSource, + requestedSessionId, + undefined, + shouldDeferMcpDiscovery(params) + ? { skipMcpDiscovery: true } + : undefined, + ), ); - } catch (error) { - return this.cleanupAfterRequestFailure(error, async () => { - if ( - this.sessions.get(config.getSessionId())?.getConfig() !== config - ) { - await this.cleanupUnstoredConfig(config); - } - }); - } - profiler.setSessionId(session.getId()); - return profiler.timeSync('response_build', () => ({ - sessionId: session.getId(), - models: this.buildAvailableModels(config), - modes: this.buildModesData(config), - configOptions: this.buildConfigOptions(config), - })); - }, - parentContext ? { parentContext } : {}, - ); + let session: Session; + try { + await profiler.time('auth', () => this.ensureAuthenticated(config)); + profiler.timeSync('file_system_setup', () => + this.setupFileSystem(config), + ); + session = await profiler.time('session_register', () => + this.createAndStoreSession(config, settings), + ); + } catch (error) { + return this.cleanupAfterRequestFailure(error, async () => { + if ( + this.sessions.get(config.getSessionId())?.getConfig() !== config + ) { + await this.cleanupUnstoredConfig(config); + } + }); + } + profiler.setSessionId(session.getId()); + return profiler.timeSync('response_build', () => ({ + sessionId: session.getId(), + models: this.buildAvailableModels(config), + modes: this.buildModesData(config), + configOptions: this.buildConfigOptions(config), + })); + }, + parentContext ? { parentContext } : {}, + ); + } finally { + releaseStartingSessionId?.(); + } } async loadSession(params: LoadSessionRequest): Promise { + let sessionId = normalizeSessionIdForLookup(params.sessionId); const sessionSource = getSessionSource(params); - // Load per-request settings BEFORE the existence check: the check must - // resolve `advanced.runtimeOutputDir` from THIS request's cwd, not from - // whichever settings a concurrent handler loaded last. - const settings = loadSettingsCached(params.cwd); - const liveSession = this.sessions.get(params.sessionId); + const liveSession = this.sessions.get(sessionId); if (liveSession) { + const settings = loadSettingsCached(params.cwd); const liveConfig = liveSession.getConfig(); await this.assertLiveSessionScope(liveConfig, settings, params.cwd); return this.withLiveSessionRestore( - params.sessionId, + sessionId, liveSession, async (config, sessionData) => { const response: LoadSessionResponse = { @@ -4800,7 +4839,7 @@ class QwenAgent implements Agent { ) : { records: visibleRecords, hasMore: false }; const replay = await collectHistoryReplayUpdates({ - sessionId: params.sessionId, + sessionId, config, records: replayPage.records, gaps: sessionData.historyGaps, @@ -4843,142 +4882,157 @@ class QwenAgent implements Agent { }, ); } - const exists = await this.runWithPinnedRuntimeBaseDir( - settings, - params.cwd, - async () => { - const sessionService = new SessionService(params.cwd); - return sessionService.sessionExists(params.sessionId); - }, - ); - if (!exists) { - throw RequestError.resourceNotFound(`session:${params.sessionId}`); - } - // Adopt into the "latest loaded" cache only once the session is - // confirmed — a failed probe for a stale id must not repoint - // agent-level readers at this request's workspace. - this.settings = settings; - - const config = await this.newSessionConfig( - params.cwd, - // `LoadSessionRequest.mcpServers` is required in today's ACP - // schema, but mirror `unstable_resumeSession` and tolerate a - // future loosening — `newSessionConfig` iterates the list, so - // a `null`/`undefined` would otherwise throw `TypeError`. - params.mcpServers ?? [], - settings, - sessionSource, - params.sessionId, - true, - ); - const sessionData = config.getResumedSessionData(); - const bulkReplay = isBulkLoadReplayRequest(params); - const replayPageSize = bulkReplay - ? getLoadReplayPageSize(params) - : undefined; - let replayEnvelope: BridgeLoadReplayEnvelope | undefined; + const releaseStartingSessionId = this.reserveStartingSessionId(sessionId); try { - await this.ensureAuthenticated(config); - this.setupFileSystem(config); - await this.createAndStoreSession(config, settings, sessionData, { - enableLiveScreenContext: isCompatibleLiveSessionSource( - sessionSource ?? {}, - ), - ...(bulkReplay ? { replayHistory: false } : {}), - beforeStartPostReplayServices: async (createdSession) => { - if (bulkReplay) { - const records = sessionData?.conversation.messages; - let replayUpdates: SessionUpdate[] = []; - if (records) { - createdSession.primeTurnFromHistory(records); - const visibleRecords = selectVisibleHistoryRecords( - records, - shouldHideInheritedHistory(params), - ); - const replayPage = selectRecentHistoryRecords( - visibleRecords, - replayPageSize, - ); - const replayUsage = createReplayCumulativeUsage(); - const replay = await collectHistoryReplayUpdates({ - sessionId: params.sessionId, - config, - records: replayPage.records, - gaps: sessionData?.historyGaps, - cumulativeUsage: replayUsage, - supersedeUnrestorableGoal: true, - logger: debugLogger, - }); - replayUpdates = replay.updates; - copyCumulativeUsage(createdSession.cumulativeUsage, replayUsage); - if (replay.replayError !== undefined) { - replayEnvelope = { + // Load per-request settings only after reserving a non-live id. The check + // must resolve `advanced.runtimeOutputDir` from this request's cwd. + const settings = loadSettingsCached(params.cwd); + const persistedSessionId = await this.runWithPinnedRuntimeBaseDir( + settings, + params.cwd, + async () => { + const sessionService = new SessionService(params.cwd); + if (await sessionService.sessionExists(sessionId)) return sessionId; + return sessionService.findSessionIdIgnoringCase?.(sessionId); + }, + ); + if (!persistedSessionId) { + throw RequestError.resourceNotFound(`session:${sessionId}`); + } + sessionId = persistedSessionId; + // Adopt into the "latest loaded" cache only once the session is + // confirmed — a failed probe for a stale id must not repoint + // agent-level readers at this request's workspace. + this.settings = settings; + + const config = await this.newSessionConfig( + params.cwd, + // `LoadSessionRequest.mcpServers` is required in today's ACP + // schema, but mirror `unstable_resumeSession` and tolerate a + // future loosening — `newSessionConfig` iterates the list, so + // a `null`/`undefined` would otherwise throw `TypeError`. + params.mcpServers ?? [], + settings, + sessionSource, + sessionId, + true, + ); + const sessionData = config.getResumedSessionData(); + const bulkReplay = isBulkLoadReplayRequest(params); + const replayPageSize = bulkReplay + ? getLoadReplayPageSize(params) + : undefined; + let replayEnvelope: BridgeLoadReplayEnvelope | undefined; + try { + await this.ensureAuthenticated(config); + this.setupFileSystem(config); + await this.createAndStoreSession(config, settings, sessionData, { + enableLiveScreenContext: isCompatibleLiveSessionSource( + sessionSource ?? {}, + ), + ...(bulkReplay ? { replayHistory: false } : {}), + beforeStartPostReplayServices: async (createdSession) => { + if (bulkReplay) { + const records = sessionData?.conversation.messages; + let replayUpdates: SessionUpdate[] = []; + if (records) { + createdSession.primeTurnFromHistory(records); + const visibleRecords = selectVisibleHistoryRecords( + records, + shouldHideInheritedHistory(params), + ); + const replayPage = selectRecentHistoryRecords( + visibleRecords, + replayPageSize, + ); + const replayUsage = createReplayCumulativeUsage(); + const replay = await collectHistoryReplayUpdates({ + sessionId, + config, + records: replayPage.records, + gaps: sessionData?.historyGaps, + cumulativeUsage: replayUsage, + supersedeUnrestorableGoal: true, + logger: debugLogger, + }); + replayUpdates = replay.updates; + copyCumulativeUsage( + createdSession.cumulativeUsage, + replayUsage, + ); + if (replay.replayError !== undefined) { + replayEnvelope = { + v: LOAD_REPLAY_VERSION, + updates: replayUpdates, + partial: true, + replayError: replay.replayError, + ...(replayPage.hasMore ? { hasMore: true } : {}), + }; + } + replayEnvelope ??= { v: LOAD_REPLAY_VERSION, updates: replayUpdates, - partial: true, - replayError: replay.replayError, ...(replayPage.hasMore ? { hasMore: true } : {}), }; } replayEnvelope ??= { v: LOAD_REPLAY_VERSION, updates: replayUpdates, - ...(replayPage.hasMore ? { hasMore: true } : {}), }; } - replayEnvelope ??= { - v: LOAD_REPLAY_VERSION, - updates: replayUpdates, - }; + + await this.#restoreWorktreeOnResume(config, createdSession); + await this.#restoreBackgroundAgentsOnResume(config, createdSession); + this.#restoreGoalOnResume(config, createdSession); + }, + }); + } catch (error) { + return this.cleanupAfterRequestFailure(error, async () => { + if ( + this.sessions.get(config.getSessionId())?.getConfig() !== config + ) { + await this.cleanupUnstoredConfig(config); } + }); + } + const modesData = this.buildModesData(config); + const availableModels = this.buildAvailableModels(config); + const configOptions = this.buildConfigOptions(config); - await this.#restoreWorktreeOnResume(config, createdSession); - await this.#restoreBackgroundAgentsOnResume(config, createdSession); - this.#restoreGoalOnResume(config, createdSession); + const response: LoadSessionResponse = { + modes: modesData, + models: availableModels, + configOptions, + ...(sessionData?.artifactSnapshot + ? { artifactSnapshot: sessionData.artifactSnapshot } + : {}), + } as LoadSessionResponse; + if (!replayEnvelope) { + return response; + } + return { + ...response, + _meta: { + [LOAD_REPLAY_META_KEY]: replayEnvelope, }, - }); - } catch (error) { - return this.cleanupAfterRequestFailure(error, async () => { - if (this.sessions.get(config.getSessionId())?.getConfig() !== config) { - await this.cleanupUnstoredConfig(config); - } - }); - } - const modesData = this.buildModesData(config); - const availableModels = this.buildAvailableModels(config); - const configOptions = this.buildConfigOptions(config); - - const response: LoadSessionResponse = { - modes: modesData, - models: availableModels, - configOptions, - ...(sessionData?.artifactSnapshot - ? { artifactSnapshot: sessionData.artifactSnapshot } - : {}), - } as LoadSessionResponse; - if (!replayEnvelope) { - return response; + }; + } finally { + releaseStartingSessionId(); } - return { - ...response, - _meta: { - [LOAD_REPLAY_META_KEY]: replayEnvelope, - }, - }; } async unstable_resumeSession( params: ResumeSessionRequest, ): Promise { + let sessionId = normalizeSessionIdForLookup(params.sessionId); const sessionSource = getSessionSource(params); - // Same per-request settings discipline as `loadSession`. - const settings = loadSettingsCached(params.cwd); - const liveSession = this.sessions.get(params.sessionId); + const liveSession = this.sessions.get(sessionId); if (liveSession) { + const settings = loadSettingsCached(params.cwd); const liveConfig = liveSession.getConfig(); await this.assertLiveSessionScope(liveConfig, settings, params.cwd); return this.withLiveSessionRestore( - params.sessionId, + sessionId, liveSession, async (config, sessionData) => ({ @@ -4991,67 +5045,81 @@ class QwenAgent implements Agent { }) as ResumeSessionResponse, ); } - const exists = await this.runWithPinnedRuntimeBaseDir( - settings, - params.cwd, - async () => { - const sessionService = new SessionService(params.cwd); - return sessionService.sessionExists(params.sessionId); - }, - ); - if (!exists) { - throw RequestError.resourceNotFound(`session:${params.sessionId}`); - } - this.settings = settings; - - const config = await this.newSessionConfig( - params.cwd, - params.mcpServers ?? [], - settings, - sessionSource, - params.sessionId, - true, - ); + const releaseStartingSessionId = this.reserveStartingSessionId(sessionId); try { - await this.ensureAuthenticated(config); - this.setupFileSystem(config); - await this.createAndStoreSession( - config, + // Same per-request settings discipline as `loadSession`. + const settings = loadSettingsCached(params.cwd); + const persistedSessionId = await this.runWithPinnedRuntimeBaseDir( settings, - config.getResumedSessionData(), - { - enableLiveScreenContext: isCompatibleLiveSessionSource( - sessionSource ?? {}, - ), - replayHistory: false, - beforeStartPostReplayServices: async (createdSession) => { - await this.#restoreWorktreeOnResume(config, createdSession); - await this.#restoreBackgroundAgentsOnResume(config, createdSession); - this.#restoreGoalOnResume(config, createdSession); - }, + params.cwd, + async () => { + const sessionService = new SessionService(params.cwd); + if (await sessionService.sessionExists(sessionId)) return sessionId; + return sessionService.findSessionIdIgnoringCase?.(sessionId); }, ); - } catch (error) { - return this.cleanupAfterRequestFailure(error, async () => { - if (this.sessions.get(config.getSessionId())?.getConfig() !== config) { - await this.cleanupUnstoredConfig(config); - } - }); - } + if (!persistedSessionId) { + throw RequestError.resourceNotFound(`session:${sessionId}`); + } + sessionId = persistedSessionId; + this.settings = settings; - const modesData = this.buildModesData(config); - const availableModels = this.buildAvailableModels(config); - const configOptions = this.buildConfigOptions(config); + const config = await this.newSessionConfig( + params.cwd, + params.mcpServers ?? [], + settings, + sessionSource, + sessionId, + true, + ); + try { + await this.ensureAuthenticated(config); + this.setupFileSystem(config); + await this.createAndStoreSession( + config, + settings, + config.getResumedSessionData(), + { + enableLiveScreenContext: isCompatibleLiveSessionSource( + sessionSource ?? {}, + ), + replayHistory: false, + beforeStartPostReplayServices: async (createdSession) => { + await this.#restoreWorktreeOnResume(config, createdSession); + await this.#restoreBackgroundAgentsOnResume( + config, + createdSession, + ); + this.#restoreGoalOnResume(config, createdSession); + }, + }, + ); + } catch (error) { + return this.cleanupAfterRequestFailure(error, async () => { + if ( + this.sessions.get(config.getSessionId())?.getConfig() !== config + ) { + await this.cleanupUnstoredConfig(config); + } + }); + } - const sessionData = config.getResumedSessionData(); - return { - modes: modesData, - models: availableModels, - configOptions, - ...(sessionData?.artifactSnapshot - ? { artifactSnapshot: sessionData.artifactSnapshot } - : {}), - } as ResumeSessionResponse; + const modesData = this.buildModesData(config); + const availableModels = this.buildAvailableModels(config); + const configOptions = this.buildConfigOptions(config); + + const sessionData = config.getResumedSessionData(); + return { + modes: modesData, + models: availableModels, + configOptions, + ...(sessionData?.artifactSnapshot + ? { artifactSnapshot: sessionData.artifactSnapshot } + : {}), + } as ResumeSessionResponse; + } finally { + releaseStartingSessionId(); + } } /** @@ -5187,33 +5255,36 @@ class QwenAgent implements Agent { async setSessionMode( params: SetSessionModeRequest, ): Promise { - const session = this.sessions.get(params.sessionId); + const sessionId = normalizeSessionIdForLookup(params.sessionId); + const session = this.sessions.get(sessionId); if (!session) { throw RequestError.invalidParams( undefined, - `Session not found for id: ${params.sessionId}`, + `Session not found for id: ${sessionId}`, ); } - return session.setMode(params); + return session.setMode({ ...params, sessionId }); } async unstable_setSessionModel( params: SetSessionModelRequest, ): Promise { - const session = this.sessions.get(params.sessionId); + const sessionId = normalizeSessionIdForLookup(params.sessionId); + const session = this.sessions.get(sessionId); if (!session) { throw RequestError.invalidParams( undefined, - `Session not found for id: ${params.sessionId}`, + `Session not found for id: ${sessionId}`, ); } - return await session.setModel(params); + return await session.setModel({ ...params, sessionId }); } async setSessionConfigOption( params: SetSessionConfigOptionRequest, ): Promise { - const { sessionId, configId, value } = params; + const sessionId = normalizeSessionIdForLookup(params.sessionId); + const { configId, value } = params; const session = this.sessions.get(sessionId); if (!session) { @@ -5254,11 +5325,12 @@ class QwenAgent implements Agent { } async prompt(params: PromptRequest): Promise { - const session = this.sessions.get(params.sessionId); + const sessionId = normalizeSessionIdForLookup(params.sessionId); + const session = this.sessions.get(sessionId); if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); + throw new Error(`Session not found: ${sessionId}`); } - const sanitizedParams = { ...params }; + const sanitizedParams = { ...params, sessionId }; const meta = params._meta && typeof params._meta === 'object' ? { ...params._meta } @@ -5311,10 +5383,10 @@ class QwenAgent implements Agent { settleCall = resolve; }), }; - let calls = this.activePromptCalls.get(params.sessionId); + let calls = this.activePromptCalls.get(sessionId); if (!calls) { calls = new Set(); - this.activePromptCalls.set(params.sessionId, calls); + this.activePromptCalls.set(sessionId, calls); } calls.add(call); try { @@ -5327,7 +5399,7 @@ class QwenAgent implements Agent { } finally { calls.delete(call); if (calls.size === 0) { - this.activePromptCalls.delete(params.sessionId); + this.activePromptCalls.delete(sessionId); } settleCall(); // Order a fresh snapshot ahead of this response on the same stream. The @@ -5340,9 +5412,10 @@ class QwenAgent implements Agent { } async cancel(params: CancelNotification): Promise { - const session = this.sessions.get(params.sessionId); + const sessionId = normalizeSessionIdForLookup(params.sessionId); + const session = this.sessions.get(sessionId); if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); + throw new Error(`Session not found: ${sessionId}`); } try { await session.cancelPendingPrompt(); @@ -7621,6 +7694,14 @@ class QwenAgent implements Agent { params: Record, ): Promise> { try { + const rawSessionId = params['sessionId']; + const normalizedParams = + typeof rawSessionId === 'string' + ? { + ...params, + sessionId: normalizeSessionIdForLookup(rawSessionId), + } + : params; if ( method === SERVE_CONTROL_EXT_METHODS.sessionBackgroundNotification && this.privateParentState !== 'trusted' @@ -7630,7 +7711,7 @@ class QwenAgent implements Agent { 'Background notifications require a trusted private ACP parent', ); } - return await this.extMethodInternal(method, params); + return await this.extMethodInternal(method, normalizedParams); } catch (error) { const writerError = getSessionWriterError(error); if (writerError) { @@ -11560,7 +11641,10 @@ class QwenAgent implements Agent { }); } catch (error) { if (error instanceof SessionIdConflictError) { - throw RequestError.invalidParams(undefined, error.message); + throw new RequestError(ACP_ERROR_CODES.INVALID_PARAMS, error.message, { + errorKind: 'session_id_conflict', + sessionId: error.sessionId, + }); } const writerError = getSessionWriterError(error); if (writerError) { @@ -11882,7 +11966,7 @@ class QwenAgent implements Agent { } = {}, ): Promise { this.assertManagedSessionAdmission(); - const sessionId = config.getSessionId(); + const sessionId = normalizeSessionIdForLookup(config.getSessionId()); const geminiClient = config.getGeminiClient(); const needsInitialize = !geminiClient.isInitialized(); @@ -11892,7 +11976,11 @@ class QwenAgent implements Agent { this.assertManagedSessionAdmission(); if (this.sessions.has(sessionId)) { - throw new Error(`Session ${sessionId} is already active.`); + throw new RequestError( + ACP_ERROR_CODES.INVALID_PARAMS, + `Session ${sessionId} is already active.`, + { errorKind: 'session_id_conflict', sessionId }, + ); } const session = new Session( diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index f113867030e..d27226420ef 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -72,19 +72,9 @@ import { reviewCommand } from '../commands/review.js'; import { serveCommand } from '../commands/serve.js'; import { sessionsCommand } from '../commands/sessions.js'; import { updateCommand } from '../commands/update.js'; +import { isValidSessionId } from './session-id.js'; -// UUID v4 regex pattern for validation -const SESSION_ID_REGEX = - /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}(-agent-[a-zA-Z0-9_.-]+)?$/i; - -/** - * Validates if a string is a valid session ID format. - * Accepts a standard UUID, or a UUID followed by `-agent-{suffix}` - * (used by Arena to give each agent a deterministic session ID). - */ -export function isValidSessionId(value: string): boolean { - return SESSION_ID_REGEX.test(value); -} +export { isValidSessionId } from './session-id.js'; import { isWorkspaceTrusted } from './trustedFolders.js'; import { assembleMcpServers } from './mcpServers.js'; diff --git a/packages/cli/src/config/session-id.test.ts b/packages/cli/src/config/session-id.test.ts new file mode 100644 index 00000000000..d8abe9da465 --- /dev/null +++ b/packages/cli/src/config/session-id.test.ts @@ -0,0 +1,77 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + isValidSessionId, + normalizeSessionIdForLookup, + parseCallerSuppliedSessionId, +} from './session-id.js'; + +describe('parseCallerSuppliedSessionId', () => { + it.each([ + '550e8400-e29b-11d4-a716-446655440000', + '550e8400-e29b-21d4-a716-446655440000', + '550e8400-e29b-31d4-a716-446655440000', + '550e8400-e29b-41d4-a716-446655440000', + '550e8400-e29b-51d4-a716-446655440000', + ])('accepts RFC-variant UUID %s', (sessionId) => { + expect(parseCallerSuppliedSessionId(sessionId)).toEqual({ + kind: 'valid', + sessionId, + }); + }); + + it('normalizes mixed case and treats nullish values as absent', () => { + expect( + parseCallerSuppliedSessionId('550E8400-E29B-41D4-A716-446655440000'), + ).toEqual({ + kind: 'valid', + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }); + expect(parseCallerSuppliedSessionId(undefined)).toEqual({ kind: 'absent' }); + expect(parseCallerSuppliedSessionId(null)).toEqual({ kind: 'absent' }); + }); + + it.each([ + '', + 42, + false, + {}, + [], + '00000000-0000-0000-0000-000000000000', + '01930000-0000-6000-a000-000000000001', + '01930000-0000-7000-a000-000000000001', + '550e8400-e29b-41d4-c716-446655440000', + '550e8400-e29b-41d4-a716-446655440000-agent-a', + '../../550e8400-e29b-41d4-a716-446655440000', + ])('rejects caller value %j', (value) => { + expect(parseCallerSuppliedSessionId(value)).toEqual({ kind: 'invalid' }); + }); +}); + +describe('normalizeSessionIdForLookup', () => { + it('lowercases caller-visible UUIDs', () => { + expect( + normalizeSessionIdForLookup('550E8400-E29B-41D4-A716-446655440000'), + ).toBe('550e8400-e29b-41d4-a716-446655440000'); + }); + + it.each([ + '550e8400-e29b-41d4-a716-446655440000-agent-WorkerA', + 'legacy-session-ID', + ])('leaves internal or legacy ID %s unchanged', (sessionId) => { + expect(normalizeSessionIdForLookup(sessionId)).toBe(sessionId); + }); +}); + +describe('isValidSessionId', () => { + it('keeps internal Arena agent session IDs valid', () => { + expect( + isValidSessionId('550e8400-e29b-41d4-a716-446655440000-agent-arena_1'), + ).toBe(true); + }); +}); diff --git a/packages/cli/src/config/session-id.ts b/packages/cli/src/config/session-id.ts new file mode 100644 index 00000000000..8a2c7a80e01 --- /dev/null +++ b/packages/cli/src/config/session-id.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +const INTERNAL_SESSION_ID_REGEX = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}(-agent-[a-zA-Z0-9_.-]+)?$/i; + +// Caller IDs stay a strict subset of internal IDs so every accepted value is +// also reachable through CLI resume validation and cannot claim Arena suffixes. +const CALLER_SUPPLIED_SESSION_ID_REGEX = + /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + +export type CallerSuppliedSessionIdParseResult = + | { kind: 'absent' } + | { kind: 'invalid' } + | { kind: 'valid'; sessionId: string }; + +export function isValidSessionId(value: string): boolean { + return INTERNAL_SESSION_ID_REGEX.test(value); +} + +/** + * Canonicalize caller-visible UUIDs without changing internal or legacy IDs. + * Internal Arena agent IDs and legacy IDs preserve their existing spelling. + */ +export function normalizeSessionIdForLookup(value: string): string { + return CALLER_SUPPLIED_SESSION_ID_REGEX.test(value) + ? value.toLowerCase() + : value; +} + +export function parseCallerSuppliedSessionId( + value: unknown, +): CallerSuppliedSessionIdParseResult { + if (value === undefined || value === null) return { kind: 'absent' }; + if ( + typeof value !== 'string' || + !CALLER_SUPPLIED_SESSION_ID_REGEX.test(value) + ) { + return { kind: 'invalid' }; + } + return { kind: 'valid', sessionId: normalizeSessionIdForLookup(value) }; +} diff --git a/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts b/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts index d5dc7cc9623..c2b5e9df3fd 100644 --- a/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts +++ b/packages/cli/src/serve/acp-http/client-mcp-ws.test.ts @@ -15,11 +15,13 @@ import { type Client, } from '@modelcontextprotocol/sdk/client/index.js'; import type { JSONRPCMessage } from '@modelcontextprotocol/sdk/types.js'; -import { SdkControlClientTransport } from '@qwen-code/qwen-code-core'; +import { SdkControlClientTransport, Storage } from '@qwen-code/qwen-code-core'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import { mountAcpHttp } from './index.js'; import type { ClientMcpServerProvider } from './client-mcp-ws.js'; import { WorkspaceRememberTaskLane } from '../workspace-remember.js'; +import { SessionArchiveCoordinator } from '../server/session-archive.js'; +import { createRequestedSessionIdAdmission } from '../session-id-admission.js'; vi.mock('../../utils/stdioHelpers.js', () => ({ writeStderrLine: vi.fn(), @@ -174,6 +176,7 @@ describe('client_mcp_over_ws reverse channel (serve layer)', () => { return new Promise((resolve) => { const app = express(); app.use(express.json()); + const archiveCoordinator = new SessionArchiveCoordinator(); const handle = mountAcpHttp(app, fakeBridge, { boundWorkspace: '/ws', workspace: fakeWorkspace, @@ -181,6 +184,17 @@ describe('client_mcp_over_ws reverse channel (serve layer)', () => { workspaceRememberLane: new WorkspaceRememberTaskLane(fakeBridge), clientMcpOverWs: opts.clientMcpOverWs ?? true, ...(opts.withProvider === false ? {} : { clientMcpProvider: provider }), + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [fakeBridge], + getPersistenceTargets: () => [ + { + workspaceCwd: '/ws', + runtimeBaseDir: Storage.getRuntimeBaseDir(), + }, + ], + }), }); server = app.listen(0, '127.0.0.1', () => { port = (server.address() as AddressInfo).port; diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index 5c291d64106..f3ba4c0532a 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -41,7 +41,10 @@ import { UnsupportedDeviceFlowProviderError, UpstreamDeviceFlowError, } from '../auth/device-flow.js'; -import type { HttpAcpBridge } from '@qwen-code/acp-bridge/bridgeTypes'; +import { + REQUESTED_SESSION_ID_META_KEY, + type HttpAcpBridge, +} from '@qwen-code/acp-bridge/bridgeTypes'; import { parseSessionSource } from '@qwen-code/acp-bridge'; import { isReservedLiveSessionSource, @@ -76,6 +79,10 @@ import { readPermissionRuleSet, } from '../../config/permission-settings.js'; import { loadSettings } from '../../config/settings.js'; +import { + normalizeSessionIdForLookup, + parseCallerSuppliedSessionId, +} from '../../config/session-id.js'; import { WorkspaceVoiceError } from '../../services/voice-service.js'; import { SetupGithubError, setupGithub } from '../../services/setup-github.js'; import { @@ -93,6 +100,10 @@ import { type WorkspaceRememberTaskLane, } from '../workspace-remember.js'; import { extractRememberErrorCode } from '../../runtime/workspace-remember-errors.js'; +import { + RequestedSessionIdAdmissionError, + type RequestedSessionIdAdmission, +} from '../session-id-admission.js'; import { MAX_REMEMBER_CONTENT_BYTES } from '../../runtime/workspace-memory-remember-constants.js'; import type { DeviceFlowRegistry } from '../auth/device-flow.js'; import { collectWorkspaceMemoryStatus } from '../workspace-memory.js'; @@ -327,6 +338,9 @@ const TRUSTED_WORKSPACE_METHODS = new Set([ ]); const WORKSPACE_GENERATION_MUTATION_METHODS = new Set([ + 'session/new', + 'session/load', + 'session/resume', `${QWEN_METHOD_NS}workspace/init`, `${QWEN_METHOD_NS}workspace/trust/request`, `${QWEN_METHOD_NS}workspace/permissions/set`, @@ -387,6 +401,19 @@ const MAX_FILE_LINE_LIMIT = 2000; class AcpParamError extends Error {} +class InvalidRequestedSessionIdError extends Error {} + +class RequestedSessionIdNotHonoredError extends Error { + constructor( + readonly requestedSessionId: string, + readonly actualSessionId: string, + ) { + super( + `The ACP agent returned session "${actualSessionId}" instead of requested session "${requestedSessionId}".`, + ); + } +} + function parseOptionalPositiveInteger( value: unknown, fallback: number, @@ -574,6 +601,38 @@ export function toRpcError(err: unknown): { message: string; data?: Record; } { + if (err instanceof InvalidRequestedSessionIdError) { + return { + code: RPC.INVALID_PARAMS, + message: err.message, + data: { httpStatus: 400, errorKind: 'invalid_session_id' }, + }; + } + if (err instanceof RequestedSessionIdAdmissionError) { + const unavailable = err.code === 'session_id_admission_unavailable'; + return { + code: unavailable ? RPC.INTERNAL_ERROR : RPC.INVALID_PARAMS, + message: err.message, + data: { + httpStatus: unavailable ? 503 : 409, + errorKind: err.code, + sessionId: err.sessionId, + ...err.details, + }, + }; + } + if (err instanceof RequestedSessionIdNotHonoredError) { + return { + code: RPC.INTERNAL_ERROR, + message: err.message, + data: { + httpStatus: 500, + errorKind: 'session_id_not_honored', + requestedSessionId: err.requestedSessionId, + actualSessionId: err.actualSessionId, + }, + }; + } if (err instanceof DaemonDrainingError) { return { code: RPC.INTERNAL_ERROR, @@ -804,6 +863,12 @@ export interface LiveSessionIsolation { isSessionActive?(sessionId: string): boolean; } +interface AcpSessionRuntimeContext { + readonly bridge: HttpAcpBridge; + readonly sessionRuntimeBaseDir: string; + readonly workspaceId?: string; +} + /** * Routes JSON-RPC messages between the HTTP transport and the * `HttpAcpBridge`. Inbound client messages map to bridge calls; the @@ -819,6 +884,7 @@ export class AcpDispatcher { private readonly getEnv: () => Readonly, private readonly workspace: DaemonWorkspaceService, private readonly workspaceRememberLane: WorkspaceRememberTaskLane, + private readonly requestedSessionIdAdmission: RequestedSessionIdAdmission, private readonly fsFactory?: WorkspaceFileSystemFactory, private readonly deviceFlowRegistry?: DeviceFlowRegistry, private readonly sessionShellCommandEnabled: boolean = false, @@ -830,29 +896,43 @@ export class AcpDispatcher { | undefined = () => undefined, private readonly liveSessionIsolation?: LiveSessionIsolation, private readonly sessionRuntimeBaseDir: string = Storage.getRuntimeBaseDir(), + private readonly getSessionRuntimeContext: () => AcpSessionRuntimeContext = () => ({ + bridge, + sessionRuntimeBaseDir, + }), ) { this.agentManager = createDaemonSubagentManager(boundWorkspace); } - private killOrphanSession( + private removeOrphanSession( sessionId: string, removePersistedSession = false, - ): void { + runtime: AcpSessionRuntimeContext = this.getSessionRuntimeContext(), + ): Promise { const cleanup = removePersistedSession ? deleteDaemonSessionIfOrphan({ sessionId, service: new SessionService(this.boundWorkspace, { - runtimeBaseDir: this.sessionRuntimeBaseDir, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, }), - bridge: this.bridge, + bridge: runtime.bridge, coordinator: this.archiveCoordinator, }) - : this.bridge.killSession(sessionId, { requireZeroAttaches: true }); - void cleanup.catch((err) => + : runtime.bridge.killSession(sessionId, { requireZeroAttaches: true }); + return cleanup.catch((err) => { writeStderrLine( `qwen serve: /acp orphan killSession(${logSafe(sessionId)}) failed: ${logSafe(errMsg(err))}`, - ), - ); + ); + return undefined; + }); + } + + private killOrphanSession( + sessionId: string, + removePersistedSession = false, + runtime?: AcpSessionRuntimeContext, + ): void { + void this.removeOrphanSession(sessionId, removePersistedSession, runtime); } /** @@ -987,9 +1067,10 @@ export class AcpDispatcher { */ private async configOptionsFor( sessionId: string, + bridge: HttpAcpBridge = this.bridge, ): Promise { try { - const ctx = (await this.bridge.getSessionContextStatus(sessionId)) as { + const ctx = (await bridge.getSessionContextStatus(sessionId)) as { state?: { configOptions?: unknown }; }; const co = ctx?.state?.configOptions; @@ -1264,10 +1345,15 @@ export class AcpDispatcher { if (!isRequest(msg) && !isNotification(msg)) return; const method = msg.method; - const params = (isObject(msg.params) ? msg.params : {}) as Record< - string, - unknown - >; + const params = { + ...((isObject(msg.params) ? msg.params : {}) as Record), + }; + if (typeof params['sessionId'] === 'string') { + params['sessionId'] = normalizeSessionIdForLookup(params['sessionId']); + } + const normalizedSessionHeader = sessionHeader + ? normalizeSessionIdForLookup(sessionHeader) + : undefined; const id = isRequest(msg) ? msg.id : undefined; const generationScoped = @@ -1300,9 +1386,9 @@ export class AcpDispatcher { // `sessionId` param MUST agree — reject divergence rather than let a // POST act on a session other than the one the header names. if ( - sessionHeader && + normalizedSessionHeader && typeof params['sessionId'] === 'string' && - params['sessionId'] !== sessionHeader + params['sessionId'] !== normalizedSessionHeader ) { if (id !== undefined) { conn.sendConn( @@ -1325,6 +1411,19 @@ export class AcpDispatcher { return; case 'session/new': { + const meta = isObject(params['_meta']) ? params['_meta'] : undefined; + const parsedSessionId = parseCallerSuppliedSessionId( + meta?.[REQUESTED_SESSION_ID_META_KEY], + ); + if (parsedSessionId.kind === 'invalid') { + throw new InvalidRequestedSessionIdError( + `\`_meta["${REQUESTED_SESSION_ID_META_KEY}"]\` must be an RFC UUID v1-v5`, + ); + } + const requestedSessionId = + parsedSessionId.kind === 'valid' + ? parsedSessionId.sessionId + : undefined; const cwd = this.parseSessionWorkspaceCwd(params); if (this.liveSessionIsolation) { if (id !== undefined) { @@ -1339,6 +1438,7 @@ export class AcpDispatcher { } return; } + const sessionRuntime = this.getSessionRuntimeContext(); const source = parseSessionSource( params['sourceType'], params['sourceId'], @@ -1361,55 +1461,113 @@ export class AcpDispatcher { } return; } - // ACP standard: session/new MUST create a new isolated session. - // Always use sessionScope 'thread' regardless of client params. - // The REST surface (POST /session) supports 'single' for - // backward compat, but the ACP endpoint follows the standard. - const session = await this.bridge.spawnOrAttach({ - workspaceCwd: cwd, - clientId: conn.clientId, - sessionScope: 'thread', - ...source, - }); - // Teardown raced the spawn: the connection was destroyed while the - // bridge call was in flight, so nothing will tear this session down. - // Kill the orphan (no other client could have attached yet). - if (conn.destroyed) { - this.killOrphanSession(session.sessionId, true); - return; - } - conn.getOrCreateSession(session.sessionId).clientId = - session.clientId; - conn.ownSession(session.sessionId); - const configOptions = await this.configOptionsFor(session.sessionId); - if (conn.destroyed) { - this.killOrphanSession(session.sessionId, true); + const reservation = requestedSessionId + ? await this.requestedSessionIdAdmission.reserveCreate( + requestedSessionId, + { + bridge: sessionRuntime.bridge, + workspaceCwd: cwd, + ...(sessionRuntime.workspaceId + ? { workspaceId: sessionRuntime.workspaceId } + : {}), + }, + ) + : undefined; + try { + assertGenerationOpen?.(); + // ACP standard: session/new MUST create a new isolated session. + // Always use sessionScope 'thread' regardless of client params. + // The REST surface (POST /session) supports 'single' for + // backward compat, but the ACP endpoint follows the standard. + const session = await sessionRuntime.bridge.spawnOrAttach({ + workspaceCwd: cwd, + clientId: conn.clientId, + sessionScope: 'thread', + ...source, + ...(requestedSessionId ? { sessionId: requestedSessionId } : {}), + }); + const rollbackSession = async (): Promise => { + if (session.attached) { + await sessionRuntime.bridge + .detachClient(session.sessionId, session.clientId) + .catch(() => {}); + } else { + await this.removeOrphanSession( + session.sessionId, + true, + sessionRuntime, + ); + } + }; + try { + assertGenerationOpen?.(); + } catch (error) { + await rollbackSession(); + throw error; + } + if ( + requestedSessionId !== undefined && + session.sessionId !== requestedSessionId + ) { + await rollbackSession(); + throw new RequestedSessionIdNotHonoredError( + requestedSessionId, + session.sessionId, + ); + } + // Teardown raced the spawn: the connection was destroyed while the + // bridge call was in flight, so nothing will tear this session down. + // Kill the orphan (no other client could have attached yet). + if (conn.destroyed) { + this.killOrphanSession(session.sessionId, true, sessionRuntime); + return; + } + const configOptions = await this.configOptionsFor( + session.sessionId, + sessionRuntime.bridge, + ); + try { + assertGenerationOpen?.(); + } catch (error) { + await rollbackSession(); + throw error; + } + if (conn.destroyed) { + this.killOrphanSession(session.sessionId, true, sessionRuntime); + return; + } + conn.getOrCreateSession(session.sessionId).clientId = + session.clientId; + conn.ownSession(session.sessionId); + // Build ACP-standard models/modes from configOptions. + // configOptions carry model/mode as category-tagged entries; + // the standard also expects top-level models/modes objects. + const models = this.extractModelState(configOptions); + const modes = this.extractModeState(configOptions); + this.replyConn(conn, id, { + sessionId: session.sessionId, + ...(session.sourceType ? { sourceType: session.sourceType } : {}), + ...(session.sourceId !== undefined + ? { sourceId: session.sourceId } + : {}), + ...(session.sourcePersisted !== undefined + ? { sourcePersisted: session.sourcePersisted } + : {}), + ...(configOptions ? { configOptions } : {}), + ...(models ? { models } : {}), + ...(modes ? { modes } : {}), + }); return; + } finally { + reservation?.release(); } - // Build ACP-standard models/modes from configOptions. - // configOptions carry model/mode as category-tagged entries; - // the standard also expects top-level models/modes objects. - const models = this.extractModelState(configOptions); - const modes = this.extractModeState(configOptions); - this.replyConn(conn, id, { - sessionId: session.sessionId, - ...(session.sourceType ? { sourceType: session.sourceType } : {}), - ...(session.sourceId !== undefined - ? { sourceId: session.sourceId } - : {}), - ...(session.sourcePersisted !== undefined - ? { sourcePersisted: session.sourcePersisted } - : {}), - ...(configOptions ? { configOptions } : {}), - ...(models ? { models } : {}), - ...(modes ? { modes } : {}), - }); - return; } case 'session/load': case 'session/resume': { - const sessionId = String(params['sessionId'] ?? ''); + const sessionId = normalizeSessionIdForLookup( + String(params['sessionId'] ?? ''), + ); if (!sessionId) { if (id !== undefined) { conn.sendConn( @@ -1438,177 +1596,225 @@ export class AcpDispatcher { return; } const cwd = this.parseSessionWorkspaceCwd(params); - const restored = await this.archiveCoordinator.runSharedMany( - [sessionId], - async () => { - await assertSessionLoadable(cwd, sessionId); - // Re-seed the persisted parent lineage so a restored sub-session - // still reports its parent over the ACP transport (parity with the - // REST restore handler); the bridge creates the entry without it. - const sessionService = new SessionService(cwd); - const metadata = this.liveSessionIsolation - ? await readLoadableLiveConversationMetadata( - sessionId, - (candidateId) => - sessionService.readCreationMetadata(candidateId), - ) - : await sessionService.readCreationMetadata(sessionId); - if (metadata === undefined) { - throw new SessionNotFoundError(sessionId); - } - const liveConversationCwd = this.liveSessionIsolation - ? await this.liveSessionIsolation.materializeConversationDirectory( - sessionId, - ) - : undefined; - const session = - method === 'session/load' - ? await this.bridge.loadSession({ + const sessionRuntime = this.getSessionRuntimeContext(); + const reservation = this.requestedSessionIdAdmission.reserveRestore( + sessionId, + { + bridge: sessionRuntime.bridge, + workspaceCwd: cwd, + ...(sessionRuntime.workspaceId + ? { workspaceId: sessionRuntime.workspaceId } + : {}), + }, + ); + try { + const restored = await this.archiveCoordinator.runSharedMany( + [sessionId], + async () => { + assertGenerationOpen?.(); + await assertSessionLoadable( + cwd, + sessionId, + sessionRuntime.sessionRuntimeBaseDir, + ); + // Re-seed the persisted parent lineage so a restored sub-session + // still reports its parent over the ACP transport (parity with the + // REST restore handler); the bridge creates the entry without it. + const sessionService = new SessionService(cwd, { + runtimeBaseDir: sessionRuntime.sessionRuntimeBaseDir, + }); + const metadata = this.liveSessionIsolation + ? await readLoadableLiveConversationMetadata( sessionId, - workspaceCwd: cwd, - clientId: conn.clientId, - historyReplay: 'response', - ...metadata, - }) - : await this.bridge.resumeSession({ + (candidateId) => + sessionService.readCreationMetadata(candidateId), + ) + : await sessionService.readCreationMetadata(sessionId); + if (metadata === undefined) { + throw new SessionNotFoundError(sessionId); + } + const liveConversationCwd = this.liveSessionIsolation + ? await this.liveSessionIsolation.materializeConversationDirectory( sessionId, - workspaceCwd: cwd, - clientId: conn.clientId, - ...metadata, - }); - // Live creation and cold restore reserve this relocation before - // returning an id that can be prompted. An active entry has - // therefore already crossed the same isolation boundary. - if (liveConversationCwd === undefined) { - return session; - } - if (session.hasActivePrompt) { - if (session.currentCwd === liveConversationCwd) return session; - try { - if (session.clientId) { - await this.bridge.detachClient( - session.sessionId, - session.clientId, - ); - } - } catch { - // Preserve the isolation error. Never kill an active owner. + ) + : undefined; + assertGenerationOpen?.(); + const session = + method === 'session/load' + ? await sessionRuntime.bridge.loadSession({ + sessionId, + workspaceCwd: cwd, + clientId: conn.clientId, + historyReplay: 'response', + ...metadata, + }) + : await sessionRuntime.bridge.resumeSession({ + sessionId, + workspaceCwd: cwd, + clientId: conn.clientId, + ...metadata, + }); + // Live creation and cold restore reserve this relocation before + // returning an id that can be prompted. An active entry has + // therefore already crossed the same isolation boundary. + if (liveConversationCwd === undefined) { + return session; } - throw new Error( - 'Active Live session is outside its isolated conversation directory.', - ); - } - try { - const changed = await this.bridge.changeSessionCwd(sessionId, { - path: liveConversationCwd, - allowedRoots: [cwd], - managedRelocation: 'live-conversation', - }); - if (changed.newCwd !== liveConversationCwd) { + if (session.hasActivePrompt) { + if (session.currentCwd === liveConversationCwd) + return session; + try { + if (session.clientId) { + await sessionRuntime.bridge.detachClient( + session.sessionId, + session.clientId, + ); + } + } catch { + // Preserve the isolation error. Never kill an active owner. + } throw new Error( - 'Live conversation directory relocation was rejected.', + 'Active Live session is outside its isolated conversation directory.', ); } - session.currentCwd = changed.newCwd; - } catch (error) { try { - if (session.attached && session.clientId) { - await this.bridge.detachClient( - session.sessionId, - session.clientId, + const changed = await sessionRuntime.bridge.changeSessionCwd( + sessionId, + { + path: liveConversationCwd, + allowedRoots: [cwd], + managedRelocation: 'live-conversation', + }, + ); + if (changed.newCwd !== liveConversationCwd) { + throw new Error( + 'Live conversation directory relocation was rejected.', ); - } else if (!session.attached) { - await this.bridge.killSession(session.sessionId, { - requireZeroAttaches: true, - }); } - } catch { - // Preserve the relocation error. + session.currentCwd = changed.newCwd; + } catch (error) { + try { + if (session.attached && session.clientId) { + await sessionRuntime.bridge.detachClient( + session.sessionId, + session.clientId, + ); + } else if (!session.attached) { + await sessionRuntime.bridge.killSession( + session.sessionId, + { + requireZeroAttaches: true, + }, + ); + } + } catch { + // Preserve the relocation error. + } + throw error; } - throw error; + return session; + }, + ); + const rollbackRestore = async (): Promise => { + if (restored.attached) { + await sessionRuntime.bridge + .detachClient(sessionId, restored.clientId) + .catch(() => {}); + } else { + await sessionRuntime.bridge + .killSession(sessionId, { requireZeroAttaches: true }) + .catch(() => {}); } - return session; - }, - ); - // Teardown raced the restore — EITHER the whole connection was - // destroyed (`conn.destroyed`) OR a `session/close` for this id - // started DURING the await (`closingSessions`); in the latter the - // close's `finally` teardown would destroy the binding we're about - // to create. Both need the same cleanup; only the client reply - // differs. Cleanup depends on what restore did: - // - attached:true → detachClient rolls back just our attach. - // - attached:false → restore SPAWNED a fresh session from disk; - // detachClient only decrements attachCount and does NOT reap - // (reaping is the spawn-owner's job) — so kill it. - const closeRaced = conn.closingSessions.has(sessionId); - if (conn.destroyed || closeRaced) { - const cleanup = restored.attached - ? this.bridge.detachClient(sessionId, restored.clientId) - : this.bridge.killSession(sessionId, { - requireZeroAttaches: true, - }); - void cleanup.catch((err) => - writeStderrLine( - `qwen serve: /acp orphan ${restored.attached ? 'detach' : 'kill'}(${logSafe(sessionId)}) teardown-race: ${logSafe(errMsg(err))}`, - ), + }; + try { + assertGenerationOpen?.(); + } catch (error) { + await rollbackRestore(); + throw error; + } + // ACP standard: load/resume response includes configOptions + models + modes + const loadConfigOptions = await this.configOptionsFor( + sessionId, + sessionRuntime.bridge, ); - // Connection-still-alive close race → tell the client to retry. - // Same rationale as the pre-await guard: a transient server-side - // race, so INTERNAL_ERROR (-32603), not INVALID_PARAMS. - if (closeRaced && !conn.destroyed && id !== undefined) { - conn.sendConn( - error( - id, - RPC.INTERNAL_ERROR, - `session ${sessionId} was closed during load; retry`, + const loadModels = this.extractModelState(loadConfigOptions); + const loadModes = this.extractModeState(loadConfigOptions); + const loadState = restored.state ?? {}; + const loadMeta = isObject(loadState._meta) + ? loadState._meta + : undefined; + const loadQwenMeta = isObject(loadMeta?.[QWEN_META_KEY]) + ? loadMeta[QWEN_META_KEY] + : undefined; + const replayStatus = + method === 'session/load' && restored.partial === true + ? { + partial: true as const, + ...(typeof restored.replayError === 'string' + ? { replayError: restored.replayError } + : {}), + } + : undefined; + try { + assertGenerationOpen?.(); + } catch (error) { + await rollbackRestore(); + throw error; + } + // Teardown raced the restore — EITHER the whole connection was + // destroyed (`conn.destroyed`) OR a `session/close` for this id + // started while the restore response was being assembled. Cleanup + // depends on what restore did: an attach is rolled back, while a + // freshly restored session must be killed by the spawn owner. + const closeRaced = conn.closingSessions.has(sessionId); + if (conn.destroyed || closeRaced) { + void rollbackRestore().catch((err) => + writeStderrLine( + `qwen serve: /acp orphan ${restored.attached ? 'detach' : 'kill'}(${logSafe(sessionId)}) teardown-race: ${logSafe(errMsg(err))}`, ), ); + // Connection-still-alive close race → tell the client to retry. + // Same rationale as the pre-await guard: a transient server-side + // race, so INTERNAL_ERROR (-32603), not INVALID_PARAMS. + if (closeRaced && !conn.destroyed && id !== undefined) { + conn.sendConn( + error( + id, + RPC.INTERNAL_ERROR, + `session ${sessionId} was closed during load; retry`, + ), + ); + } + return; + } + conn.getOrCreateSession(sessionId).clientId = restored.clientId; + if (method === 'session/load') { + conn.markInitialReplayPending(sessionId); } + conn.ownSession(sessionId); + this.replyConn(conn, id, { + ...loadState, + ...(replayStatus + ? { + _meta: { + ...(loadMeta ?? {}), + [QWEN_META_KEY]: { + ...(loadQwenMeta ?? {}), + sessionLoadReplay: replayStatus, + }, + }, + } + : {}), + ...(loadConfigOptions + ? { configOptions: loadConfigOptions } + : {}), + ...(loadModels ? { models: loadModels } : {}), + ...(loadModes ? { modes: loadModes } : {}), + }); return; + } finally { + reservation.release(); } - conn.getOrCreateSession(sessionId).clientId = restored.clientId; - if (method === 'session/load') { - conn.markInitialReplayPending(sessionId); - } - conn.ownSession(sessionId); - // ACP standard: load/resume response includes configOptions + models + modes - const loadConfigOptions = await this.configOptionsFor(sessionId); - const loadModels = this.extractModelState(loadConfigOptions); - const loadModes = this.extractModeState(loadConfigOptions); - const loadState = restored.state ?? {}; - const loadMeta = isObject(loadState._meta) - ? loadState._meta - : undefined; - const loadQwenMeta = isObject(loadMeta?.[QWEN_META_KEY]) - ? loadMeta[QWEN_META_KEY] - : undefined; - const replayStatus = - method === 'session/load' && restored.partial === true - ? { - partial: true as const, - ...(typeof restored.replayError === 'string' - ? { replayError: restored.replayError } - : {}), - } - : undefined; - this.replyConn(conn, id, { - ...loadState, - ...(replayStatus - ? { - _meta: { - ...(loadMeta ?? {}), - [QWEN_META_KEY]: { - ...(loadQwenMeta ?? {}), - sessionLoadReplay: replayStatus, - }, - }, - } - : {}), - ...(loadConfigOptions ? { configOptions: loadConfigOptions } : {}), - ...(loadModels ? { models: loadModels } : {}), - ...(loadModes ? { modes: loadModes } : {}), - }); - return; } case 'session/list': { diff --git a/packages/cli/src/serve/acp-http/index.ts b/packages/cli/src/serve/acp-http/index.ts index b11ee67b917..3a76a01263c 100644 --- a/packages/cli/src/serve/acp-http/index.ts +++ b/packages/cli/src/serve/acp-http/index.ts @@ -14,6 +14,7 @@ import { RUNTIME_MCP_IF_ABSENT_CONFIG_FLAG, Storage, } from '@qwen-code/qwen-code-core'; +import { normalizeSessionIdForLookup } from '../../config/session-id.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import type { WorkspaceFileSystemFactory } from '../fs/index.js'; @@ -41,6 +42,7 @@ import { SseStream } from './sse-stream.js'; import { WsStream } from './ws-stream.js'; import type { RateLimitTier } from '../rate-limit.js'; import { SessionArchiveCoordinator } from '../server/session-archive.js'; +import type { RequestedSessionIdAdmission } from '../session-id-admission.js'; import { RPC, error as rpcError, @@ -104,7 +106,8 @@ function isActiveDrainCorrelation( ? (message.params as { sessionId?: unknown }) : undefined; return ( - typeof params?.sessionId === 'string' && conn.ownsSession(params.sessionId) + typeof params?.sessionId === 'string' && + conn.ownsSession(normalizeSessionIdForLookup(params.sessionId)) ); } @@ -391,6 +394,12 @@ export interface MountAcpHttpOptions { /** Effective direct session shell policy for ACP initialize/dispatch. */ sessionShellCommandEnabled?: boolean; archiveCoordinator?: SessionArchiveCoordinator; + /** + * The daemon-wide session-id admission shared with every other transport. + * Required: a mount-local fallback could not see draining generations, so + * the host must inject the shared instance (as `createServeApp` does). + */ + requestedSessionIdAdmission: RequestedSessionIdAdmission; /** Shared lane for sessionless workspace remember tasks. */ workspaceRememberLane: WorkspaceRememberTaskLane; /** Rate limit checker for WS messages (WS bypasses Express middleware). */ @@ -603,6 +612,12 @@ export function mountAcpHttp( if (!enabled) return undefined; const daemonEnv = opts.daemonEnv ?? process.env; + const primarySessionRuntimeBaseDir = + opts.workspaceRegistry?.primary.sessionRuntimeBaseDir ?? + Storage.getRuntimeBaseDir(); + const archiveCoordinator = + opts.archiveCoordinator ?? new SessionArchiveCoordinator(); + const requestedSessionIdAdmission = opts.requestedSessionIdAdmission; const getPrimaryEnv = () => opts.workspaceRegistry ? runtimeEffectiveEnv(opts.workspaceRegistry.primary, daemonEnv) @@ -792,11 +807,12 @@ export function mountAcpHttp( getPrimaryEnv, opts.workspace, opts.workspaceRememberLane, + requestedSessionIdAdmission, opts.fsFactory, opts.deviceFlowRegistry, opts.sessionShellCommandEnabled === true, registry, - opts.archiveCoordinator ?? new SessionArchiveCoordinator(), + archiveCoordinator, opts.isPrimaryWorkspaceTrusted ?? (() => { const entry = opts.workspaceRegistry?.primaryEntry; @@ -809,8 +825,20 @@ export function mountAcpHttp( return guard ? () => guard.assertOpen() : undefined; }, undefined, - opts.workspaceRegistry?.primary.sessionRuntimeBaseDir ?? - Storage.getRuntimeBaseDir(), + primarySessionRuntimeBaseDir, + () => { + const runtime = opts.workspaceRegistry?.primary; + return runtime + ? { + bridge: runtime.bridge, + sessionRuntimeBaseDir: runtime.sessionRuntimeBaseDir, + workspaceId: runtime.workspaceId, + } + : { + bridge, + sessionRuntimeBaseDir: primarySessionRuntimeBaseDir, + }; + }, ); dispatcherRef.current = dispatcher; @@ -1060,7 +1088,10 @@ export function mountAcpHttp( res.status(404).json({ error: 'Unknown Acp-Connection-Id' }); return; } - const sessionId = headerOf(req, ACP_SESSION_HEADER); + const rawSessionId = headerOf(req, ACP_SESSION_HEADER); + const sessionId = rawSessionId + ? normalizeSessionIdForLookup(rawSessionId) + : undefined; if (!sessionId) { // Connection-scoped stream. onClose logs the disconnect so a @@ -1269,12 +1300,20 @@ export function mountAcpHttp( rt.bridge, rt.workspaceCwd, ); + const registeredGeneration = opts.workspaceRegistry?.getEntryByWorkspaceId( + rt.workspaceId, + )?.current; + const generationGuard = + registeredGeneration?.runtime === rt + ? registeredGeneration.guard + : rt.generationGuard; const secondaryDispatcher = new AcpDispatcher( rt.bridge, rt.workspaceCwd, () => runtimeEffectiveEnv(rt, daemonEnv), rt.workspaceService, workspaceRememberLane, + requestedSessionIdAdmission, rt.routeFileSystemFactory, // Phase 4: secondary mounts share the daemon-global device-flow registry // (single instance per daemon; OAuth credentials are global state). The @@ -1284,16 +1323,18 @@ export function mountAcpHttp( opts.deviceFlowRegistry, opts.sessionShellCommandEnabled === true, secondaryRegistry, - opts.archiveCoordinator ?? new SessionArchiveCoordinator(), + archiveCoordinator, () => rt.trusted, - () => { - const guard = rt.generationGuard; - return guard ? () => guard.assertOpen() : undefined; - }, + () => (generationGuard ? () => generationGuard.assertOpen() : undefined), rt.provenance === 'live-conversation' ? opts.liveSessionIsolation : undefined, rt.sessionRuntimeBaseDir, + () => ({ + bridge: rt.bridge, + sessionRuntimeBaseDir: rt.sessionRuntimeBaseDir, + workspaceId: rt.workspaceId, + }), ); secondaryDispatcherRef.current = secondaryDispatcher; return { @@ -2220,9 +2261,13 @@ export function mountAcpHttp( message.params && typeof message.params === 'object' ) { - const sid = (message.params as Record)[ + const rawSid = (message.params as Record)[ 'sessionId' ]; + const sid = + typeof rawSid === 'string' + ? normalizeSessionIdForLookup(rawSid) + : rawSid; if (typeof sid === 'string' && conn.ownsSession(sid)) { const binding = conn.sessions.get(sid); if ( diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index 535ca755aa9..48775ec579b 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -32,6 +32,7 @@ import { PermissionPolicyNotImplementedError, PromptQueueFullError, SessionLimitExceededError, + SessionNotFoundError, SessionShellClientRequiredError, SessionShellDisabledError, TotalSessionLimitExceededError, @@ -75,6 +76,8 @@ import { type WorkspaceGenerationGuard, type WorkspaceRuntime, } from '../workspace-registry.js'; +import { SessionArchiveCoordinator } from '../server/session-archive.js'; +import { createRequestedSessionIdAdmission } from '../session-id-admission.js'; import { MAX_TRUST_REASON_LENGTH, MAX_VOICE_MODEL_LENGTH, @@ -166,6 +169,7 @@ class FakeBridge { | undefined; lastSetModel: unknown; lastSpawnScope: string | undefined; + lastRequestedSessionId: string | undefined; closeShouldThrow = false; closeError: Error | undefined; killed: string[] = []; @@ -177,6 +181,8 @@ class FakeBridge { /** `attached` value loadSession returns (false = spawned-from-disk). */ loadAttached = true; spawnSessionId = 'sess-1'; + honorRequestedSessionId = true; + spawnAttached = false; spawnClientId: string | undefined = 'client-1'; loadRequests: Array<{ sessionId: string; @@ -190,13 +196,17 @@ class FakeBridge { closedSessions: string[] = []; - async spawnOrAttach(req: { sessionScope?: string }) { + async spawnOrAttach(req: { sessionScope?: string; sessionId?: string }) { this.lastSpawnScope = req?.sessionScope; + this.lastRequestedSessionId = req?.sessionId; if (this.gate) await this.gate; return { - sessionId: this.spawnSessionId, + sessionId: + this.honorRequestedSessionId && req.sessionId + ? req.sessionId + : this.spawnSessionId, workspaceCwd: TEST_WORKSPACE, - attached: false, + attached: this.spawnAttached, clientId: this.spawnClientId, }; } @@ -358,7 +368,7 @@ class FakeBridge { if (sessionId === 'sess-1') { return { sessionId, workspaceCwd: TEST_WORKSPACE }; } - throw new Error(`Session not found: ${sessionId}`); + throw new SessionNotFoundError(sessionId); } detached: Array<{ sessionId: string; clientId?: string }> = []; @@ -893,11 +903,23 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { return raw as Record; }, }); + const archiveCoordinator = new SessionArchiveCoordinator(); acpHandle = mountAcpHttp(app, bridge as unknown as HttpAcpBridge, { boundWorkspace, workspace: fakeWorkspace, enabled: true, workspaceRememberLane, + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => [ + { + workspaceCwd: boundWorkspace, + runtimeBaseDir: Storage.getRuntimeBaseDir(), + }, + ], + }), }); await new Promise((resolve) => { server = app.listen(0, '127.0.0.1', () => resolve()); @@ -951,6 +973,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { generationGuard: opts.generationGuard, }) : undefined; + const archiveCoordinator = new SessionArchiveCoordinator(); mountAcpHttp(app, bridge as unknown as HttpAcpBridge, { boundWorkspace, workspace: fakeWorkspace, @@ -963,6 +986,26 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { workspaceRememberLane: new WorkspaceRememberTaskLane( bridge as unknown as HttpAcpBridge, ), + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => + workspaceRegistry + ? workspaceRegistry.listManaged().map((runtime) => runtime.bridge) + : [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => + workspaceRegistry + ? workspaceRegistry.listManaged().map((runtime) => ({ + workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, + })) + : [ + { + workspaceCwd: boundWorkspace, + runtimeBaseDir: Storage.getRuntimeBaseDir(), + }, + ], + }), }); await new Promise((resolve) => { server = app.listen(0, '127.0.0.1', () => resolve()); @@ -3998,6 +4041,71 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { }, ); + it.each(['session/load', 'session/resume'] as const)( + '%s normalizes mixed-case caller UUIDs before restore', + async (method) => { + await withRuntimeDir(async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440132'; + const mixedCaseSessionId = sessionId.toUpperCase(); + await writeStoredSession(sessionId); + + let restoredSessionId: string | undefined; + bridge.loadSession = async (req) => { + restoredSessionId = req.sessionId; + return { + sessionId: req.sessionId, + workspaceCwd: TEST_WORKSPACE, + attached: true, + clientId: 'client-load', + state: { replayed: true }, + }; + }; + bridge.resumeSession = async (req) => { + restoredSessionId = req.sessionId; + return { + sessionId: req.sessionId, + workspaceCwd: TEST_WORKSPACE, + attached: true, + clientId: 'client-resume', + state: { resumed: true }, + }; + }; + + const connId = await initialize(); + const stream = await openStream(connId); + const reader = frameReader(stream); + await post(connId, { + jsonrpc: '2.0', + id: 216, + method, + params: { sessionId: mixedCaseSessionId }, + }); + expect(await reader.next()).toMatchObject({ id: 216 }); + reader.close(); + + expect(restoredSessionId).toBe(sessionId); + + const sessionStream = await openStream(connId, mixedCaseSessionId); + expect(sessionStream.status).toBe(200); + const sessionReader = frameReader(sessionStream); + await post(connId, { + jsonrpc: '2.0', + id: 217, + method: 'session/prompt', + params: { + sessionId: mixedCaseSessionId, + prompt: [{ type: 'text', text: 'continue after restore' }], + }, + }); + expect(await sessionReader.next()).toMatchObject({ + id: 217, + result: { stopReason: 'end_turn' }, + }); + sessionReader.close(); + }); + }, + ); + it('session/prompt reports an archive conflict while prompt is in flight', async () => { await withRuntimeDir(async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440127'; @@ -4730,6 +4838,149 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { expect(bridge.lastSpawnScope).toBe('thread'); }); + it('session/new validates, normalizes, and forwards caller-supplied sessionId meta', async () => { + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 440, + method: 'session/new', + params: { + _meta: { + 'qwen-code/sessionId': '550E8400-E29B-41D4-A716-446655440000', + }, + }, + }); + + const [frame] = (await got) as Array<{ + result: { sessionId: string }; + }>; + expect(bridge.lastRequestedSessionId).toBe( + '550e8400-e29b-41d4-a716-446655440000', + ); + expect(frame.result.sessionId).toBe('550e8400-e29b-41d4-a716-446655440000'); + }); + + it('pins fallback sessionId persistence admission to the mount runtime base', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440004'; + await writeStoredSession(sessionId); + const laterRuntimeDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'qwen-acp-later-runtime-'), + ); + process.env['QWEN_RUNTIME_DIR'] = laterRuntimeDir; + + try { + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 443, + method: 'session/new', + params: { + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }); + + const [frame] = (await got) as Array<{ + error: { code: number; data: Record }; + }>; + expect(frame.error).toMatchObject({ + code: -32602, + data: { + httpStatus: 409, + errorKind: 'session_id_conflict', + conflict: 'persisted', + }, + }); + expect(bridge.lastRequestedSessionId).toBeUndefined(); + } finally { + process.env['QWEN_RUNTIME_DIR'] = runtimeDir; + await fs.rm(laterRuntimeDir, { recursive: true, force: true }); + } + }); + + it('session/new rejects invalid sessionId meta without spawning', async () => { + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 441, + method: 'session/new', + params: { _meta: { 'qwen-code/sessionId': '../../escape' } }, + }); + + const [frame] = (await got) as Array<{ + error: { code: number; data: Record }; + }>; + expect(frame.error).toMatchObject({ + code: -32602, + data: { httpStatus: 400, errorKind: 'invalid_session_id' }, + }); + expect(bridge.lastRequestedSessionId).toBeUndefined(); + }); + + it('session/new removes a mismatched orphan and reports a protocol error', async () => { + bridge.honorRequestedSessionId = false; + bridge.spawnSessionId = '550e8400-e29b-41d4-a716-446655440999'; + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 442, + method: 'session/new', + params: { + _meta: { + 'qwen-code/sessionId': '550e8400-e29b-41d4-a716-446655440000', + }, + }, + }); + + const [frame] = (await got) as Array<{ + error: { code: number; data: Record }; + }>; + expect(frame.error).toMatchObject({ + code: -32603, + data: { httpStatus: 500, errorKind: 'session_id_not_honored' }, + }); + expect(bridge.killed).toContain('550e8400-e29b-41d4-a716-446655440999'); + }); + + it('session/new rolls back a mismatched attach without killing it', async () => { + bridge.honorRequestedSessionId = false; + bridge.spawnSessionId = 'existing-session'; + bridge.spawnAttached = true; + const connId = await initialize(); + const connStream = await openStream(connId); + const got = takeFrames(connStream, 1); + await post(connId, { + jsonrpc: '2.0', + id: 444, + method: 'session/new', + params: { + _meta: { + 'qwen-code/sessionId': '550e8400-e29b-41d4-a716-446655440000', + }, + }, + }); + + const [frame] = (await got) as Array<{ + error: { code: number; data: Record }; + }>; + expect(frame.error).toMatchObject({ + code: -32603, + data: { httpStatus: 500, errorKind: 'session_id_not_honored' }, + }); + expect(bridge.detached).toContainEqual({ + sessionId: 'existing-session', + clientId: 'client-1', + }); + expect(bridge.killed).not.toContain('existing-session'); + }); + it('session/prompt with empty prompt → INVALID_PARAMS', async () => { const connId = await initialize(); await newSession(connId); @@ -4827,6 +5078,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { it('connection cap → 503 on initialize', async () => { const app2 = express(); app2.use(express.json()); + const archiveCoordinator = new SessionArchiveCoordinator(); mountAcpHttp(app2, bridge as unknown as HttpAcpBridge, { boundWorkspace: TEST_WORKSPACE, workspace: fakeWorkspace, @@ -4835,6 +5087,17 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { workspaceRememberLane: new WorkspaceRememberTaskLane( bridge as unknown as HttpAcpBridge, ), + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => [ + { + workspaceCwd: TEST_WORKSPACE, + runtimeBaseDir: Storage.getRuntimeBaseDir(), + }, + ], + }), }); const srv = app2.listen(0, '127.0.0.1'); await new Promise((r) => srv.once('listening', r)); @@ -8581,6 +8844,7 @@ describe('ACP WebSocket transport security', () => { bridge = new FakeBridge(); const app = express(); app.use(express.json()); + const archiveCoordinator = new SessionArchiveCoordinator(); const handle = mountAcpHttp(app, bridge as unknown as HttpAcpBridge, { boundWorkspace: TEST_WORKSPACE, workspace: fakeWorkspace, @@ -8591,6 +8855,17 @@ describe('ACP WebSocket transport security', () => { workspaceRememberLane: new WorkspaceRememberTaskLane( bridge as unknown as HttpAcpBridge, ), + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => [ + { + workspaceCwd: TEST_WORKSPACE, + runtimeBaseDir: Storage.getRuntimeBaseDir(), + }, + ], + }), // eslint-disable-next-line @typescript-eslint/no-explicit-any checkRate: opts.checkRate as any, ...(opts.cdpTunnelOverWs diff --git a/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts b/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts index a613d7600be..87d75ac8670 100644 --- a/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts +++ b/packages/cli/src/serve/acp-http/workspace-qualified-acp.test.ts @@ -28,6 +28,9 @@ import type { WorkspaceFileSystemFactory } from '../fs/index.js'; import type { DaemonWorkspaceService } from '../workspace-service/types.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { createSessionOrganizationService } from '../session-organization-helpers.js'; +import { SessionNotFoundError } from '../acp-session-bridge.js'; +import { SessionArchiveCoordinator } from '../server/session-archive.js'; +import { createRequestedSessionIdAdmission } from '../session-id-admission.js'; const setupGithubMock = vi.hoisted(() => vi.fn()); @@ -44,14 +47,23 @@ const PARENT_ENV: WorkspaceRuntimeEnvMetadata = { function makeBridge(): HttpAcpBridge { return { - spawnOrAttach: vi.fn(async (req: { workspaceCwd: string }) => ({ - sessionId: - req.workspaceCwd === '/ws-b' ? 'secondary-session' : 'primary-session', - workspaceCwd: req.workspaceCwd, - attached: false, - clientId: - req.workspaceCwd === '/ws-b' ? 'secondary-client' : 'primary-client', - })), + spawnOrAttach: vi.fn( + async (req: { workspaceCwd: string; sessionId?: string }) => ({ + sessionId: + req.sessionId ?? + (req.workspaceCwd === '/ws-b' + ? 'secondary-session' + : 'primary-session'), + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: + req.workspaceCwd === '/ws-b' ? 'secondary-client' : 'primary-client', + }), + ), + getSessionSummary: vi.fn((sessionId: string) => { + throw new SessionNotFoundError(sessionId); + }), + killSession: vi.fn(async () => true), detachClient: vi.fn(async () => {}), executeShellCommand: vi.fn(async () => ({ exitCode: 0, @@ -249,11 +261,28 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => { const app = express(); app.use(express.json()); + const archiveCoordinator = new SessionArchiveCoordinator(); handle = mountAcpHttp(app, primaryBridge, { boundWorkspace: '/ws', workspace: {} as unknown as DaemonWorkspaceService, fsFactory: workspaceRegistry.primary.routeFileSystemFactory, enabled: true, + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => + workspaceRegistry.listManaged().map((runtime) => runtime.bridge), + getPersistenceTargets: () => + workspaceRegistry.listManaged().map((runtime) => ({ + workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, + })), + getBridgeWorkspaceId: (bridge) => + workspaceRegistry + .listEntries() + .find((entry) => entry.current?.runtime.bridge === bridge) + ?.workspaceId, + }), daemonEnv: { ...process.env, HTTPS_PROXY: 'http://primary-proxy.example:8080', @@ -606,6 +635,244 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => { expect(primaryBridge.executeShellCommand).not.toHaveBeenCalled(); }); + it('shares caller-supplied sessionId admission across primary and qualified mounts', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440181'; + let releasePrimary!: () => void; + const primaryGate = new Promise((resolve) => { + releasePrimary = resolve; + }); + vi.mocked(primaryBridge.spawnOrAttach).mockImplementationOnce( + async (request) => { + await primaryGate; + return { + sessionId: request.sessionId!, + workspaceCwd: request.workspaceCwd, + attached: false, + clientId: 'primary-client', + }; + }, + ); + + const primary = sendWsRequest('/acp', { + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { + workspaceCwd: '/ws', + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }); + await vi.waitFor(() => + expect(primaryBridge.spawnOrAttach).toHaveBeenCalledOnce(), + ); + + const secondary = await sendWsRequest('/workspaces/secondary-id/acp', { + jsonrpc: '2.0', + id: 3, + method: 'session/new', + params: { + workspaceCwd: '/ws-b', + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }); + expect(secondary['error']).toMatchObject({ + code: -32602, + data: { + httpStatus: 409, + errorKind: 'session_id_conflict', + conflict: 'pending', + }, + }); + expect(secondaryBridge.spawnOrAttach).not.toHaveBeenCalled(); + + releasePrimary(); + await expect(primary).resolves.toMatchObject({ + result: { sessionId }, + }); + }); + + it('uses the concrete primary bridge generation for restore admission', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440182'; + await writeStoredSession(sessionId, '/ws'); + let releaseRestore!: () => void; + const restoreGate = new Promise((resolve) => { + releaseRestore = resolve; + }); + primaryBridge.loadSession = vi.fn(async (request) => { + await restoreGate; + return { + sessionId, + workspaceCwd: request.workspaceCwd, + attached: false, + clientId: request.clientId ?? 'old-primary-client', + state: {}, + hasActivePrompt: false, + }; + }); + + const first = sendWsRequest('/acp', { + jsonrpc: '2.0', + id: 2, + method: 'session/load', + params: { sessionId, workspaceCwd: '/ws' }, + }); + await vi.waitFor(() => + expect(primaryBridge.loadSession).toHaveBeenCalledOnce(), + ); + + const entry = workspaceRegistry.primaryEntry; + expect(workspaceRegistry.beginReplacement(entry, 'policy-2')).toBe(true); + const replacementBridge = makeBridge(); + replacementBridge.resumeSession = vi.fn(async (request) => ({ + sessionId, + workspaceCwd: request.workspaceCwd, + attached: false, + clientId: request.clientId ?? 'new-primary-client', + state: {}, + hasActivePrompt: false, + })); + workspaceRegistry.activateReplacement( + entry, + makeRuntime({ + id: 'primary-id', + cwd: '/ws', + primary: true, + trusted: true, + bridge: replacementBridge, + }), + 'policy-2', + ); + + const second = await sendWsRequest('/acp', { + jsonrpc: '2.0', + id: 3, + method: 'session/resume', + params: { sessionId, workspaceCwd: '/ws' }, + }); + expect(second['error']).toMatchObject({ + code: -32602, + data: { + httpStatus: 409, + errorKind: 'session_workspace_conflict', + conflict: 'pending', + workspaceId: 'primary-id', + }, + }); + expect(replacementBridge.resumeSession).not.toHaveBeenCalled(); + + releaseRestore(); + await expect(first).resolves.toMatchObject({ + error: { + code: -32603, + data: { + httpStatus: 503, + errorKind: 'workspace_runtime_unavailable', + retryable: true, + }, + }, + }); + expect(primaryBridge.killSession).toHaveBeenCalledWith(sessionId, { + requireZeroAttaches: true, + }); + }); + + it('rolls back session/new when its generation changes while building the response', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440183'; + let releaseContext!: () => void; + const contextGate = new Promise((resolve) => { + releaseContext = resolve; + }); + primaryBridge.getSessionContextStatus = vi.fn(async () => { + await contextGate; + return { v: 1 as const, sessionId, workspaceCwd: '/ws', state: {} }; + }); + + const pending = sendWsRequest('/acp', { + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { + workspaceCwd: '/ws', + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }); + await vi.waitFor(() => + expect(primaryBridge.getSessionContextStatus).toHaveBeenCalledOnce(), + ); + + const entry = workspaceRegistry.primaryEntry; + expect(workspaceRegistry.beginReplacement(entry, 'policy-2')).toBe(true); + workspaceRegistry.activateReplacement( + entry, + makeRuntime({ + id: 'primary-id', + cwd: '/ws', + primary: true, + trusted: true, + bridge: makeBridge(), + }), + 'policy-2', + ); + releaseContext(); + + await expect(pending).resolves.toMatchObject({ + error: { + code: -32603, + data: { + httpStatus: 503, + errorKind: 'workspace_runtime_unavailable', + retryable: true, + }, + }, + }); + expect(primaryBridge.killSession).toHaveBeenCalledWith(sessionId, { + requireZeroAttaches: true, + }); + }); + + it('uses the registry generation guard for qualified ACP mounts', async () => { + const sessionId = '550e8400-e29b-41d4-a716-446655440184'; + let releaseContext!: () => void; + const contextGate = new Promise((resolve) => { + releaseContext = resolve; + }); + secondaryBridge.getSessionContextStatus = vi.fn(async () => { + await contextGate; + return { v: 1 as const, sessionId, workspaceCwd: '/ws-b', state: {} }; + }); + + const pending = sendWsRequest('/workspaces/secondary-id/acp', { + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { + workspaceCwd: '/ws-b', + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }); + await vi.waitFor(() => + expect(secondaryBridge.getSessionContextStatus).toHaveBeenCalledOnce(), + ); + + const entry = workspaceRegistry.getEntryByWorkspaceId('secondary-id')!; + expect(workspaceRegistry.beginReplacement(entry, 'policy-2')).toBe(true); + releaseContext(); + + await expect(pending).resolves.toMatchObject({ + error: { + code: -32603, + data: { + httpStatus: 503, + errorKind: 'workspace_runtime_unavailable', + retryable: true, + }, + }, + }); + expect(secondaryBridge.killSession).toHaveBeenCalledWith(sessionId, { + requireZeroAttaches: true, + }); + }); + it('rejects a body workspaceCwd that differs from the selected mount', async () => { const response = await sendWsRequest('/workspaces/secondary-id/acp', { jsonrpc: '2.0', @@ -995,12 +1262,29 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => { ]); const app = express(); app.use(express.json()); + const archiveCoordinator = new SessionArchiveCoordinator(); const singleHandle = mountAcpHttp(app, primaryBridge, { boundWorkspace: '/ws', workspace: {} as DaemonWorkspaceService, enabled: true, workspaceRegistry: registry, workspaceRememberLane: new WorkspaceRememberTaskLane(primaryBridge), + archiveCoordinator, + requestedSessionIdAdmission: createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => + registry.listManaged().map((runtime) => runtime.bridge), + getPersistenceTargets: () => + registry.listManaged().map((runtime) => ({ + workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, + })), + getBridgeWorkspaceId: (bridge) => + registry + .listEntries() + .find((entry) => entry.current?.runtime.bridge === bridge) + ?.workspaceId, + }), })!; const singleServer = await new Promise((resolve) => { const listening = app.listen(0, '127.0.0.1', () => resolve(listening)); diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index 2ea74998605..b5709c0bbef 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -34,6 +34,7 @@ export const SERVE_CAPABILITY_REGISTRY = { daemon_status: { since: 'v1' }, capabilities: { since: 'v1' }, session_create: { since: 'v1' }, + session_id_override: { since: 'v1' }, session_scope_override: { since: 'v1' }, session_load: { since: 'v1' }, session_resume: { since: 'v1' }, diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 634fca8602c..29475279b97 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -13,7 +13,6 @@ import { GROUP_COLOR_OPTIONS, GitWorktreeService, SessionOrganizationError, - SessionService, SESSION_TRANSCRIPT_MAX_LIMIT, SESSION_TRANSCRIPT_MAX_EXPANDED_PAGE_BYTES, SESSION_TRANSCRIPT_MAX_PAGE_BYTES, @@ -21,7 +20,6 @@ import { SessionTranscriptCursorCodec, SessionTranscriptReader, SessionTranscriptSnapshotUnavailableError, - Storage, addDaemonRequestAttribute, runWithoutDebugLogSession, writeWorktreeSessionMarker, @@ -40,7 +38,7 @@ import { } from '../live/session-source.js'; import type { Application, Request, RequestHandler, Response } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import { loadSettingsCached } from '../../config/settings-cache.js'; +import { parseCallerSuppliedSessionId } from '../../config/session-id.js'; import { isChannelDeliveryError } from '../../runtime/channel-delivery-ipc.js'; import { parseChannelDelivery } from '../../runtime/channel-delivery.js'; import { @@ -127,6 +125,12 @@ import { runWithWorkspaceRuntimeStorage, } from '../workspace-runtime-storage.js'; import type { ChannelDeliveryAuthorizationStore } from '../channel-delivery-authorization.js'; +import { + createRequestedSessionIdAdmission, + RequestedSessionIdAdmissionError, + type RequestedSessionIdAdmission, + type RequestedSessionIdReservation, +} from '../session-id-admission.js'; // `HEAD` is the most prominent ref name git rejects as a branch name. // The surrounding predicate covers the remaining reserved forms (`@`, `-`, @@ -145,25 +149,12 @@ const GIT_RESERVED_BRANCH = 'HEAD'; const MAX_BRANCH_NAME_BYTES = 1000; const MAX_BRANCH_COMPONENT_BYTES = 200; -// A strict SUBSET of config.ts's isValidSessionId: same v4 version/variant -// nibbles, minus the `-agent-{suffix}` form (SessionService.SESSION_FILE_PATTERN -// only matches 32-36 hex/hyphen chars, so a suffixed id would write a transcript -// the session list can never see). -// -// Keeping it a subset in BOTH directions matters: every id the daemon accepts -// must also be a valid `--session-id` and `/resume ` argument, otherwise a -// session created over HTTP is unreachable from the CLI (resumeCommand.ts gates -// on isValidSessionId and falls through to title matching when it fails). That -// rules out UUIDv7, the nil UUID, and non-RFC-4122 variants even though they are -// harmless as filenames. -const HTTP_SESSION_ID_REGEX = - /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; - interface RegisterSessionRoutesDeps { boundWorkspace: string; bridge: AcpSessionBridge; workspaceRegistry: WorkspaceRegistry; archiveCoordinator: SessionArchiveCoordinator; + requestedSessionIdAdmission?: RequestedSessionIdAdmission; mutate: (opts?: { strict?: boolean }) => RequestHandler; sendBridgeError: SendBridgeError; daemonLog?: DaemonLogger; @@ -408,23 +399,6 @@ function shouldPreserveTranscriptResolutionError(err: unknown): boolean { ); } -/** - * Whether a session with this id is currently live on the daemon. - * `getSessionSummary` is a `byId` lookup that signals absence by throwing - * `SessionNotFoundError`. Anything else is treated as "assume live" rather - * than "not live" — for a uniqueness guard, failing closed on an unreadable - * bridge is the safe direction, and it matches how - * `SessionService.sessionExistsInAnyState` handles its own read errors. - */ -function isSessionLive(bridge: AcpSessionBridge, sessionId: string): boolean { - try { - bridge.getSessionSummary(sessionId); - return true; - } catch (err) { - return !(err instanceof SessionNotFoundError); - } -} - function parseOptionalApprovalMode( body: Record, res: Response, @@ -463,6 +437,35 @@ export function registerSessionRoutes( sessionShellCommandEnabled, virtualSubagentSessions, } = deps; + const requestedSessionIdAdmission = + deps.requestedSessionIdAdmission ?? + createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => + workspaceRegistry.listManaged().map((runtime) => runtime.bridge), + getPersistenceTargets: () => + workspaceRegistry.listManaged().map((runtime) => ({ + workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, + })), + getBridgeWorkspaceId: (bridge) => + workspaceRegistry + .listEntries() + .find((entry) => entry.current?.runtime.bridge === bridge) + ?.workspaceId, + }); + const captureRuntimeGenerationAssertion = ( + runtime: WorkspaceRuntime, + ): (() => void) | undefined => { + const registeredGeneration = workspaceRegistry.getEntryByWorkspaceId( + runtime.workspaceId, + )?.current; + const guard = + registeredGeneration?.runtime === runtime + ? registeredGeneration.guard + : runtime.generationGuard; + return guard ? () => guard.assertOpen() : undefined; + }; const LANGUAGE_CODES = deps.languageCodes; const transcriptCursorMasterKey = crypto.randomBytes(32); const transcriptCursorCodecs = new Map< @@ -483,10 +486,6 @@ export function registerSessionRoutes( // after spawn). Closes the TOCTOU where two concurrent requests both pass // the guard before either populates `activeBranchSessions`. const inFlightBranchWorkspaces = new Set(); - // Caller-supplied session ids with a creation currently in flight. Closes - // the TOCTOU where two concurrent requests with the same sessionId both - // pass sessionExistsInAnyState before either session is created. - const inFlightSessionIds = new Set(); /** Remove the branch-session tracking entry when a session ends. */ const clearBranchSessionEntry = (sessionId: string): void => { @@ -578,6 +577,29 @@ export function registerSessionRoutes( }); }; + const sendRequestedSessionIdAdmissionError = ( + res: Response, + error: RequestedSessionIdAdmissionError, + route: string, + ): void => { + const resolutionKind = + error.code === 'session_workspace_conflict' + ? 'workspace_conflict' + : error.code; + logSessionRoutingFailure(route, resolutionKind, { + sessionId: error.sessionId, + ...error.details, + }); + const status = + error.code === 'session_id_admission_unavailable' ? 503 : 409; + res.status(status).json({ + error: error.message, + code: error.code, + sessionId: error.sessionId, + ...error.details, + }); + }; + const sendWorkspaceMismatch = ( res: Response, requestedWorkspace: string, @@ -919,11 +941,6 @@ export function registerSessionRoutes( sendUntrustedSessionOwner(res, route, sessionId, runtime); return false; }; - const inFlightRestoreOwners = new Map< - string, - { workspaceId: string; workspaceCwd: string; count: number } - >(); - const sendSessionWorkspaceConflict = ( res: Response, route: string, @@ -949,40 +966,6 @@ export function registerSessionRoutes( }); }; - const enterRestoreOwner = ( - res: Response, - route: string, - sessionId: string, - runtime: WorkspaceRuntime, - ): (() => void) | undefined => { - const existing = inFlightRestoreOwners.get(sessionId); - if (existing && existing.workspaceCwd !== runtime.workspaceCwd) { - sendSessionWorkspaceConflict(res, route, sessionId, runtime, existing); - return undefined; - } - - if (existing) { - existing.count += 1; - } else { - inFlightRestoreOwners.set(sessionId, { - workspaceId: runtime.workspaceId, - workspaceCwd: runtime.workspaceCwd, - count: 1, - }); - } - let released = false; - return () => { - if (released) return; - released = true; - const current = inFlightRestoreOwners.get(sessionId); - if (!current || current.workspaceCwd !== runtime.workspaceCwd) return; - current.count -= 1; - if (current.count <= 0) { - inFlightRestoreOwners.delete(sessionId); - } - }; - }; - const resolveRuntimeForSessionRestore = ( body: Record, res: Response, @@ -1328,6 +1311,8 @@ export function registerSessionRoutes( }); return; } + const assertRuntimeGenerationOpen = + captureRuntimeGenerationAssertion(runtime); const modelServiceId = typeof body['modelServiceId'] === 'string' ? (body['modelServiceId'] as string) @@ -1367,78 +1352,35 @@ export function registerSessionRoutes( const clientId = parseClientIdHeader(req, res); if (clientId === null) return; - // Optional caller-supplied session id. Validated at the route boundary - // so a 400 surfaces before touching the bridge. The core Config - // constructor uses it verbatim (falling back to randomUUID when absent). - const rawSessionId = body['sessionId']; - let requestedSessionId: string | undefined; - if (rawSessionId !== undefined && rawSessionId !== null) { - if ( - typeof rawSessionId !== 'string' || - !HTTP_SESSION_ID_REGEX.test(rawSessionId) - ) { - res.status(400).json({ - error: - '`sessionId` must be a UUID (e.g. "550e8400-e29b-41d4-a716-446655440000")', - code: 'invalid_session_id', - }); - return; - } - requestedSessionId = rawSessionId.toLowerCase(); - const sessionIdToCheck = requestedSessionId; - // Reject an id that is already LIVE on this workspace's bridge. - // The disk check below cannot see these: the transcript JSONL is - // only written on a session's first message, so a session that was - // created and never prompted leaves no file behind — for its whole - // lifetime, not just a brief window. Without this, a sequential retry - // with the same id falls through to the agent's own - // `Session is already active.` guard, which is a bare Error and - // surfaces as an opaque `500 / -32603` instead of the 409 this route - // documents. `inFlightSessionIds` below only covers requests that - // overlap in time; this covers a first request that already returned. - // - // Per-workspace scope: checks only the current workspace's bridge. - // The daemon-wide `inFlightSessionIds` guard covers concurrent - // cross-workspace reuse; sequential cross-workspace reuse of the same - // id can still produce two live sessions sharing an id (routing then - // fails as `ambiguous_session_owner`). - if (isSessionLive(runtime.bridge, sessionIdToCheck)) { - res.status(409).json({ - error: `Session "${requestedSessionId}" already exists`, - code: 'session_id_conflict', - }); - return; - } - // Reject an id that already exists on disk (active or archived) at the - // route boundary: loadCliConfig calls process.exit(1) on a duplicate, - // which would terminate the shared ACP child and every session on its - // channel. - const runtimeOutputDir = - loadSettingsCached(workspaceCwd).merged.advanced?.runtimeOutputDir; - if ( - await Storage.runWithRuntimeBaseDir( - runtimeOutputDir, - workspaceCwd, - () => - new SessionService(workspaceCwd).sessionExistsInAnyState( - sessionIdToCheck, - ), - ) - ) { - res.status(409).json({ - error: `Session "${requestedSessionId}" already exists`, - code: 'session_id_conflict', - }); - return; - } - if (inFlightSessionIds.has(requestedSessionId)) { - res.status(409).json({ - error: `Session "${requestedSessionId}" creation already in progress`, - code: 'session_id_conflict', - }); - return; + const parsedSessionId = parseCallerSuppliedSessionId(body['sessionId']); + if (parsedSessionId.kind === 'invalid') { + res.status(400).json({ + error: + '`sessionId` must be an RFC UUID v1-v5 (e.g. "550e8400-e29b-41d4-a716-446655440000")', + code: 'invalid_session_id', + }); + return; + } + const requestedSessionId = + parsedSessionId.kind === 'valid' ? parsedSessionId.sessionId : undefined; + let sessionIdReservation: RequestedSessionIdReservation | undefined; + if (requestedSessionId !== undefined) { + try { + sessionIdReservation = await requestedSessionIdAdmission.reserveCreate( + requestedSessionId, + { + bridge: runtime.bridge, + workspaceCwd, + workspaceId: runtime.workspaceId, + }, + ); + } catch (error) { + if (error instanceof RequestedSessionIdAdmissionError) { + sendRequestedSessionIdAdmissionError(res, error, 'POST /session'); + return; + } + throw error; } - inFlightSessionIds.add(requestedSessionId); } let branchMeta: { name: string; baseBranch: string } | undefined; @@ -1452,6 +1394,10 @@ export function registerSessionRoutes( // When `branch` is present, create and checkout a new git branch // before spawning. The session runs in the same working directory // but on the new branch. Mutually exclusive with `worktree`. + // Caller-supplied IDs perform an asynchronous persistence scan before + // this block. Re-check the captured runtime before any branch, worktree, + // or bridge side effect in case its generation changed during the scan. + assertRuntimeGenerationOpen?.(); const rawBranch = body['branch']; if (rawBranch !== undefined && rawBranch !== null) { if (body['worktree'] !== undefined && body['worktree'] !== null) { @@ -1615,6 +1561,7 @@ export function registerSessionRoutes( }); return; } + assertRuntimeGenerationOpen?.(); inFlightBranchWorkspaces.add(workspaceCwd); try { await createBranch(workspaceCwd, branchName); @@ -1707,6 +1654,7 @@ export function registerSessionRoutes( const baseBranch = await wtService .getCurrentBranch() .catch(() => undefined); + assertRuntimeGenerationOpen?.(); const wtResult = await wtService.createUserWorktree(slug, baseBranch); if (!wtResult.success || !wtResult.worktree) { res.status(500).json({ @@ -1730,6 +1678,7 @@ export function registerSessionRoutes( sessionScope = 'thread'; } + assertRuntimeGenerationOpen?.(); const session = await runtime.bridge.spawnOrAttach({ workspaceCwd, modelServiceId, @@ -1748,8 +1697,7 @@ export function registerSessionRoutes( }); // Defensive: the bridge/agent must honor a caller-supplied id. If it was // dropped anywhere in the chain (older agent binary, coalesced attach), - // never return a surprise id — fail the request instead. Same silent-drop - // class as #7831, one layer down. + // never return a surprise id — fail the request instead. if ( requestedSessionId !== undefined && session.sessionId !== requestedSessionId @@ -1769,6 +1717,10 @@ export function registerSessionRoutes( coordinator: archiveCoordinator, }), ).catch(() => false); + } else { + await runtime.bridge + .detachClient(session.sessionId, session.clientId) + .catch(() => {}); } // This early return runs inside the outer try, but a return skips // that try's catch — so replicate the catch's resource cleanup here. @@ -1794,7 +1746,7 @@ export function registerSessionRoutes( return; } try { - runtime.generationGuard?.assertOpen(); + assertRuntimeGenerationOpen?.(); } catch (error) { if (!session.attached) { try { @@ -2043,9 +1995,7 @@ export function registerSessionRoutes( } sendBridgeError(res, err, { route: 'POST /session' }); } finally { - if (requestedSessionId !== undefined) { - inFlightSessionIds.delete(requestedSessionId); - } + sessionIdReservation?.release(); } }); @@ -2120,28 +2070,31 @@ export function registerSessionRoutes( } if (resolvedRuntime === undefined) return; const { runtime, workspaceCwd } = resolvedRuntime; - const releaseRestoreOwner = enterRestoreOwner( - res, - route, - sessionId, - runtime, - ); - if (!releaseRestoreOwner) return; + const assertRuntimeGenerationOpen = + captureRuntimeGenerationAssertion(runtime); const approvalMode = parseOptionalApprovalMode(body, res); - if (approvalMode === null) { - releaseRestoreOwner(); - return; - } + if (approvalMode === null) return; const historyPageSize = action === 'load' ? parseHistoryPageSize(body ?? {}, res) : undefined; - if (historyPageSize === null) { - releaseRestoreOwner(); - return; - } + if (historyPageSize === null) return; const clientId = parseClientIdHeader(req, res); - if (clientId === null) { - releaseRestoreOwner(); - return; + if (clientId === null) return; + let sessionIdReservation: RequestedSessionIdReservation; + try { + sessionIdReservation = requestedSessionIdAdmission.reserveRestore( + sessionId, + { + bridge: runtime.bridge, + workspaceCwd, + workspaceId: runtime.workspaceId, + }, + ); + } catch (error) { + if (error instanceof RequestedSessionIdAdmissionError) { + sendRequestedSessionIdAdmissionError(res, error, route); + return; + } + throw error; } try { const session = await archiveCoordinator.runSharedMany( @@ -2176,7 +2129,7 @@ export function registerSessionRoutes( } liveConversationCwd = await materialize(sessionId); } - runtime.generationGuard?.assertOpen(); + assertRuntimeGenerationOpen?.(); const restored = action === 'load' ? await runtime.bridge.loadSession({ @@ -2258,7 +2211,7 @@ export function registerSessionRoutes( }, ); try { - runtime.generationGuard?.assertOpen(); + assertRuntimeGenerationOpen?.(); } catch (error) { if (!session.attached) { await runtime.bridge @@ -2410,7 +2363,7 @@ export function registerSessionRoutes( sessionId, }); } finally { - releaseRestoreOwner(); + sessionIdReservation.release(); } }; diff --git a/packages/cli/src/serve/routes/workspace-setup-github.test.ts b/packages/cli/src/serve/routes/workspace-setup-github.test.ts index 0e14be9c975..3b50d407c1b 100644 --- a/packages/cli/src/serve/routes/workspace-setup-github.test.ts +++ b/packages/cli/src/serve/routes/workspace-setup-github.test.ts @@ -137,6 +137,7 @@ async function makeHarness( ...(opts.hotReload ? { workspaceTrustHotReloadAvailable: true, + getSessionBridges: () => [bridge], primaryWorkspaceTrusted: true, } : {}), diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index 23385f53b0f..3078a7a2ca6 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -5644,6 +5644,7 @@ async function runQwenServeImpl( const app = runtime.createServeApp(opts, () => actualPort, { workspaceRegistry, + getSessionBridges: () => runtimeBridges, createWorkspaceRuntime: createDynamicWorkspaceRuntime, ...(workspaceTrustHotReloadAvailable ? { diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 773d76fb6d0..c1b3079da51 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -7,6 +7,7 @@ import { existsSync, realpathSync, promises as fsp } from 'node:fs'; import { EventEmitter } from 'node:events'; import type { ServerResponse } from 'node:http'; +import type { AddressInfo } from 'node:net'; import * as os from 'node:os'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -153,6 +154,7 @@ import { ClientMcpSenderRegistry, createClientMcpServerProvider, } from './acp-http/client-mcp-sender-registry.js'; +import type { AcpHttpHandle } from './acp-http/index.js'; import { DeviceFlowRegistry, TooManyActiveDeviceFlowsError, @@ -384,6 +386,7 @@ const EXPECTED_STAGE1_FEATURES = [ 'daemon_status', 'capabilities', 'session_create', + 'session_id_override', 'session_scope_override', 'session_load', 'session_resume', @@ -3734,6 +3737,8 @@ describe('createServeApp', () => { }, workspaceRuntimeRemoval: {}, voiceCoordinator: {}, + getSessionBridges: () => + registry.listManaged().map((runtime) => runtime.bridge), } as Parameters[2]); const supported = await request(app) @@ -3988,6 +3993,8 @@ describe('createServeApp', () => { bridge, workspaceRegistry: registry, workspaceTrustHotReloadAvailable: true, + getSessionBridges: () => + registry.listManaged().map((runtime) => runtime.bridge), daemonEnv: {}, }); @@ -9408,6 +9415,50 @@ describe('createServeApp', () => { ]); }); + it('does not create after the generation closes during requested-id admission', async () => { + const bridge = fakeBridge(); + const runtime = makeWorkspaceRuntimeForTest({ + workspaceId: 'session-primary', + workspaceCwd: WS_BOUND, + primary: true, + bridge, + }); + const workspaceRegistry = createWorkspaceRegistry([runtime]); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { workspaceRegistry }, + ); + const scan = deferred(); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockReturnValue(scan.promise); + + try { + const pending = request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440004' }) + .then((response) => response); + await vi.waitFor(() => expect(locationSpy).toHaveBeenCalledOnce()); + expect( + workspaceRegistry.beginReplacement( + workspaceRegistry.primaryEntry, + 'policy-2', + ), + ).toBe(true); + scan.resolve(undefined); + + const res = await pending; + expect(res.status).toBe(503); + expect(res.body.code).toBe('workspace_runtime_unavailable'); + expect(bridge.calls).toEqual([]); + } finally { + scan.resolve(undefined); + locationSpy.mockRestore(); + } + }); + it('forwards valid session source metadata to the bridge', async () => { const bridge = fakeBridge(); const app = createServeApp( @@ -9532,6 +9583,33 @@ describe('createServeApp', () => { ]); }); + it('detaches when a bridge mismatch attached to an existing session', async () => { + const bridge = fakeBridge({ + spawnImpl: async (req) => ({ + sessionId: 'existing-session', + workspaceCwd: req.workspaceCwd, + attached: true, + clientId: 'client-x', + }), + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(500); + expect(res.body.code).toBe('session_id_not_honored'); + expect(bridge.detachCalls).toEqual([ + { sessionId: 'existing-session', clientId: 'client-x' }, + ]); + expect(bridge.killCalls).toEqual([]); + }); + it('409 when sessionId already exists (active or archived)', async () => { const bridge = fakeBridge(); const app = createServeApp( @@ -9556,7 +9634,34 @@ describe('createServeApp', () => { } }); - it('runs the sessionId existence check inside runWithRuntimeBaseDir', async () => { + it('503 when persisted session state cannot be inspected', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockRejectedValue(new Error('EACCES: runtime directory unreadable')); + try { + const res = await request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId: '550e8400-e29b-41d4-a716-446655440000' }); + + expect(res.status).toBe(503); + expect(res.body).toMatchObject({ + code: 'session_id_admission_unavailable', + retryable: true, + }); + expect(bridge.calls).toHaveLength(0); + } finally { + locationSpy.mockRestore(); + } + }); + + it('uses the runtime-pinned SessionService without ambient storage context', async () => { const bridge = fakeBridge(); const app = createServeApp( { ...baseOpts, workspace: WS_BOUND }, @@ -9575,7 +9680,7 @@ describe('createServeApp', () => { expect(res.status).toBe(409); expect(res.body.code).toBe('session_id_conflict'); - expect(runWithSpy).toHaveBeenCalled(); + expect(runWithSpy).not.toHaveBeenCalled(); expect(bridge.calls).toHaveLength(0); } finally { locationSpy.mockRestore(); @@ -9626,9 +9731,8 @@ describe('createServeApp', () => { .post('/session') .set('Host', `127.0.0.1:${baseOpts.port}`) .send({ sessionId: sid }); - // Fire both concurrently; the 20 ms gap ensures the first reaches - // spawnOrAttach (and registers in inFlightSessionIds) before the - // second arrives at the guard. + // Fire both concurrently; the 20 ms gap ensures the first holds the + // daemon-wide requested-id claim before the second reaches admission. const [firstRes, secondRes] = await Promise.all([ makeReq(), new Promise((r) => setTimeout(r, 20)).then(() => makeReq()), @@ -9641,7 +9745,112 @@ describe('createServeApp', () => { expect(spawnCallCount).toBe(1); }); - it('releases inFlightSessionIds after a spawn failure (finally cleanup)', async () => { + it('shares requested sessionId admission between REST and ACP', async () => { + let releaseSpawn!: () => void; + const spawnGate = new Promise((resolve) => { + releaseSpawn = resolve; + }); + let spawnCallCount = 0; + const bridge = fakeBridge({ + spawnImpl: async (req) => { + spawnCallCount++; + await spawnGate; + return { + sessionId: req.sessionId!, + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: req.clientId ?? 'rest-client', + }; + }, + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const server = app.listen(0, '127.0.0.1'); + await new Promise((resolve) => server.once('listening', resolve)); + const acpHandle = app.locals['acpHandle'] as AcpHttpHandle; + acpHandle.attachServer(server); + const port = (server.address() as AddressInfo).port; + const sessionId = '550e8400-e29b-41d4-a716-446655440003'; + + try { + const rest = request(app) + .post('/session') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ sessionId }) + .then((response) => response); + await vi.waitFor(() => expect(spawnCallCount).toBe(1)); + + const acp = await new Promise>( + (resolve, reject) => { + const ws = new WebSocket(`ws://127.0.0.1:${port}/acp`, { + handshakeTimeout: 2000, + }); + ws.on('open', () => + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + }), + ), + ); + ws.on('message', (data) => { + try { + const message = JSON.parse(data.toString()) as Record< + string, + unknown + >; + if (message['id'] === 1) { + ws.send( + JSON.stringify({ + jsonrpc: '2.0', + id: 2, + method: 'session/new', + params: { + workspaceCwd: WS_BOUND, + _meta: { 'qwen-code/sessionId': sessionId }, + }, + }), + ); + return; + } + if (message['id'] === 2) { + ws.close(); + resolve(message); + } + } catch (error) { + ws.terminate(); + reject(error as Error); + } + }); + ws.on('error', reject); + }, + ); + expect(acp['error']).toMatchObject({ + code: -32602, + data: { + httpStatus: 409, + errorKind: 'session_id_conflict', + conflict: 'pending', + }, + }); + expect(spawnCallCount).toBe(1); + + releaseSpawn(); + await expect(rest).resolves.toMatchObject({ status: 200 }); + expect(bridge.calls).toHaveLength(1); + } finally { + releaseSpawn(); + acpHandle.dispose(); + server.closeAllConnections?.(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + it('releases requested-id admission after a spawn failure', async () => { let spawnCallCount = 0; const bridge = fakeBridge({ spawnImpl: (req) => { @@ -11447,13 +11656,15 @@ describe('createServeApp', () => { sessionId: 'live-primary', workspaceCwd: WS_DIFFERENT, liveWorkspaceCwd: WS_BOUND, + liveWorkspaceId: 'ws-primary', }); expect(secondaryBridge.loadCalls).toHaveLength(0); }); - it('keeps a transitioning in-flight restore owner scoped to its workspace', async () => { + it('keeps a transitioning restore scoped and rolls its stale result back', async () => { const secondaryStarted = deferred(); const releaseSecondary = deferred(); + const daemonLog = fakeDaemonLog(); const primaryBridge = fakeBridge(); const secondaryBridge = fakeBridge({ loadImpl: async (req) => { @@ -11486,7 +11697,7 @@ describe('createServeApp', () => { const app = createServeApp( { ...baseOpts, workspace: WS_BOUND }, undefined, - { workspaceRegistry: registry }, + { workspaceRegistry: registry, daemonLog }, ); const secondaryRequest = request(app) @@ -11518,14 +11729,25 @@ describe('createServeApp', () => { liveWorkspaceCwd: WS_DIFFERENT, liveWorkspaceId: 'ws-secondary', }); + expect(daemonLog.warn).toHaveBeenCalledWith( + 'session routing failed', + expect.objectContaining({ + route: 'POST /session/:id/load', + resolutionKind: 'workspace_conflict', + sessionId: 'concurrent-restore', + workspaceId: 'ws-primary', + workspaceCwd: WS_BOUND, + liveWorkspaceId: 'ws-secondary', + liveWorkspaceCwd: WS_DIFFERENT, + }), + ); expect(primaryBridge.loadCalls).toHaveLength(0); const secondaryRes = await secondaryRequest; - expect(secondaryRes.status).toBe(200); + expect(secondaryRes.status).toBe(503); expect(secondaryRes.body).toMatchObject({ - sessionId: 'concurrent-restore', - workspaceCwd: WS_DIFFERENT, + code: 'workspace_runtime_unavailable', }); expect(secondaryBridge.loadCalls).toEqual([ { @@ -11534,6 +11756,12 @@ describe('createServeApp', () => { historyReplay: 'response', }, ]); + expect(secondaryBridge.killCalls).toEqual([ + { + sessionId: 'concurrent-restore', + opts: { requireZeroAttaches: true }, + }, + ]); }); it('surfaces live owner scan failures before restoring into a workspace runtime', async () => { @@ -16941,6 +17169,8 @@ describe('createServeApp', () => { boundWorkspace: WS_BOUND, workspaceRegistry, workspaceTrustHotReloadAvailable: true, + getSessionBridges: () => + workspaceRegistry.listManaged().map((runtime) => runtime.bridge), getWorkspaceTrustPolicySnapshot, }); @@ -17947,9 +18177,11 @@ describe('createServeApp', () => { it('loads fallback workspace settings fail-closed during trust hot reload', async () => { const settingsRuntime = await import('../config/settings.js'); const loadSettings = vi.spyOn(settingsRuntime, 'loadSettings'); + const bridge = fakeBridge(); const app = createServeApp(tokenOpts, undefined, { - bridge: fakeBridge(), + bridge, workspaceTrustHotReloadAvailable: true, + getSessionBridges: () => [bridge], }); const res = await auth(request(app).get('/workspace/trust')); @@ -25273,6 +25505,8 @@ describe('createServeApp ServeAppDeps.fsFactory wiring (#4175 PR 18)', () => { { workspaceRegistry: registry, workspaceTrustHotReloadAvailable: true, + getSessionBridges: () => + registry.listManaged().map((runtime) => runtime.bridge), } as Parameters[2], ); const nextForRequest = vi.fn(() => ({ marker: 'next-fs' })); @@ -25341,6 +25575,24 @@ describe('createServeApp ServeAppDeps.fsFactory wiring (#4175 PR 18)', () => { ).toThrow(/workspaceRuntimeRemoval requires.*voiceCoordinator/); }); + it('requires a live bridge provider when runtime generations can change', async () => { + const { createServeApp } = await import('./server.js'); + + expect(() => + createServeApp( + { + port: 0, + hostname: '127.0.0.1', + workspace: '/work/bound', + } as Parameters[0], + () => 0, + { + workspaceTrustHotReloadAvailable: true, + } as Parameters[2], + ), + ).toThrow(/requires deps\.getSessionBridges/); + }); + it('uses the injected registry sender when client-MCP over WS is enabled', async () => { const { createServeApp } = await import('./server.js'); const runtime = makeInjectedWorkspaceRuntime(); @@ -27581,6 +27833,8 @@ describe('Live conversation runtime lifecycle', () => { liveConversationWorkspace: conversationWorkspace, workspaceRuntimeRemoval, voiceCoordinator: new WorkspaceVoiceCoordinator(), + getSessionBridges: () => + registry.listManaged().map((runtime) => runtime.bridge), daemonEnv: { QWEN_SERVE_ACP_HTTP: '1' }, runtimePlatform: 'darwin', webShellDir: path.join(os.tmpdir(), 'qwen-live-web-shell'), diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 00bc553b575..1f054d7b743 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -106,6 +106,7 @@ import { } from './routes/workspace-trust.js'; import { registerPermissionRoutes } from './routes/permission.js'; import { registerSessionRoutes } from './routes/session.js'; +import { createRequestedSessionIdAdmission } from './session-id-admission.js'; import { registerScheduledTasksRoutes, registerWorkspaceQualifiedScheduledTasksRoutes, @@ -549,6 +550,11 @@ export interface ServeAppDeps { */ clientMcpSenderRegistry?: ClientMcpSenderRegistry; workspaceRegistry?: WorkspaceRegistry; + /** + * Returns every bridge generation that is still alive, including draining + * generations no longer exposed by the workspace registry. + */ + getSessionBridges?: () => readonly AcpSessionBridge[]; workspaceTrustHotReloadAvailable?: boolean; getWorkspaceTrustPolicySnapshot?: () => | DaemonTrustPolicySnapshot @@ -674,6 +680,15 @@ export function createServeApp( 'createServeApp: deps.workspaceRuntimeRemoval requires the matching deps.voiceCoordinator.', ); } + if ( + (deps.workspaceTrustHotReloadAvailable === true || + deps.workspaceRuntimeRemoval !== undefined) && + deps.getSessionBridges === undefined + ) { + throw new Error( + 'createServeApp: runtime replacement/removal requires deps.getSessionBridges so session-id admission can inspect draining generations.', + ); + } const app = express(); // Forward `maxSessions` into the default-constructed bridge so // direct callers of `createServeApp` (tests, embeds) get the same @@ -1136,6 +1151,21 @@ export function createServeApp( ); (app.locals as { workspaceRegistry?: WorkspaceRegistry }).workspaceRegistry = workspaceRegistry; + const requestedSessionIdAdmission = createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: + deps.getSessionBridges ?? + (() => workspaceRegistry.listManaged().map((runtime) => runtime.bridge)), + getPersistenceTargets: () => + workspaceRegistry.listManaged().map((runtime) => ({ + workspaceCwd: runtime.workspaceCwd, + runtimeBaseDir: runtime.sessionRuntimeBaseDir, + })), + getBridgeWorkspaceId: (bridge) => + workspaceRegistry + .listEntries() + .find((entry) => entry.current?.runtime.bridge === bridge)?.workspaceId, + }); primaryTrustRegistry = workspaceRegistry; const primaryRuntime = createLiveWorkspaceDelegate( () => workspaceRegistry.primary, @@ -2248,6 +2278,7 @@ export function createServeApp( bridge: primaryBridge, workspaceRegistry, archiveCoordinator, + requestedSessionIdAdmission, mutate, sendBridgeError, daemonLog, @@ -2603,6 +2634,7 @@ export function createServeApp( workspaceRegistry, isPrimaryWorkspaceTrusted, archiveCoordinator, + requestedSessionIdAdmission, workspace: primaryWorkspace, fsFactory: primaryRouteFileSystemFactory, deviceFlowRegistry, diff --git a/packages/cli/src/serve/server/request-helpers.test.ts b/packages/cli/src/serve/server/request-helpers.test.ts new file mode 100644 index 00000000000..49d5bd72a5c --- /dev/null +++ b/packages/cli/src/serve/server/request-helpers.test.ts @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Request, Response } from 'express'; +import { describe, expect, it, vi } from 'vitest'; +import { requireSessionId } from './request-helpers.js'; + +function mockRes(): { res: Response; status: ReturnType } { + const status = vi.fn().mockReturnValue({ json: vi.fn() }); + return { res: { status } as unknown as Response, status }; +} + +describe('requireSessionId', () => { + it('normalizes caller-visible UUID route parameters', () => { + const { res, status } = mockRes(); + const req = { + params: { id: '550E8400-E29B-41D4-A716-446655440000' }, + } as unknown as Request; + + expect(requireSessionId(req, res)).toBe( + '550e8400-e29b-41d4-a716-446655440000', + ); + expect(status).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/serve/server/request-helpers.ts b/packages/cli/src/serve/server/request-helpers.ts index 11465df493b..2843bce98d1 100644 --- a/packages/cli/src/serve/server/request-helpers.ts +++ b/packages/cli/src/serve/server/request-helpers.ts @@ -10,6 +10,7 @@ import { } from '@qwen-code/acp-bridge/workspacePaths'; import type { Request, Response } from 'express'; import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; +import { normalizeSessionIdForLookup } from '../../config/session-id.js'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import type { WorkspaceRequestContext } from '../workspace-service/index.js'; @@ -165,7 +166,7 @@ export function requireSessionId(req: Request, res: Response): string | null { res.status(400).json({ error: '`sessionId` route parameter is required' }); return null; } - return sessionId; + return normalizeSessionIdForLookup(sessionId); } export function parseClientIdHeader( diff --git a/packages/cli/src/serve/session-id-admission.test.ts b/packages/cli/src/serve/session-id-admission.test.ts new file mode 100644 index 00000000000..9f18dd6adb0 --- /dev/null +++ b/packages/cli/src/serve/session-id-admission.test.ts @@ -0,0 +1,460 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { promises as fsp } from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { AcpSessionBridge } from './acp-session-bridge.js'; +import { SessionNotFoundError } from './acp-session-bridge.js'; +import { SessionArchiveCoordinator } from './server/session-archive.js'; +import { + createRequestedSessionIdAdmission, + RequestedSessionIdAdmissionError, +} from './session-id-admission.js'; + +const sessionServiceMock = vi.hoisted(() => ({ + exists: + vi.fn< + ( + cwd: string, + runtimeBaseDir: string, + sessionId: string, + ) => Promise + >(), + sidecarPath: + vi.fn< + ( + cwd: string, + runtimeBaseDir: string, + sessionId: string, + state: 'active' | 'archived', + ) => string + >(), +})); + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + SessionService: class { + constructor( + private readonly cwd: string, + private readonly options: { runtimeBaseDir: string }, + ) {} + + async getSessionLocation( + sessionId: string, + ): Promise<'active' | undefined> { + return (await sessionServiceMock.exists( + this.cwd, + this.options.runtimeBaseDir, + sessionId, + )) + ? 'active' + : undefined; + } + + getWorktreeSessionPathForArchiveState( + sessionId: string, + state: 'active' | 'archived', + ): string { + return sessionServiceMock.sidecarPath( + this.cwd, + this.options.runtimeBaseDir, + sessionId, + state, + ); + } + }, + }; +}); + +const SESSION_ID = '550e8400-e29b-41d4-a716-446655440000'; +const tempDirs: string[] = []; + +function fakeBridge(liveSessionIds: string[] = []): AcpSessionBridge { + const live = new Set(liveSessionIds); + return { + getSessionSummary(sessionId: string) { + if (!live.has(sessionId)) throw new SessionNotFoundError(sessionId); + return { sessionId, workspaceCwd: '/live' }; + }, + } as unknown as AcpSessionBridge; +} + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +describe('RequestedSessionIdAdmission', () => { + beforeEach(() => { + sessionServiceMock.exists.mockReset(); + sessionServiceMock.exists.mockResolvedValue(false); + sessionServiceMock.sidecarPath.mockReset(); + sessionServiceMock.sidecarPath.mockReturnValue( + path.join(os.tmpdir(), `missing-${crypto.randomUUID()}`), + ); + }); + + afterEach(async () => { + await Promise.all( + tempDirs + .splice(0) + .map((dir) => fsp.rm(dir, { recursive: true, force: true })), + ); + }); + + it('claims synchronously before the persisted scan finishes', async () => { + const scan = deferred(); + sessionServiceMock.exists.mockReturnValue(scan.promise); + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [ + { workspaceCwd: '/one', runtimeBaseDir: '/runtime-one' }, + ], + }); + + const first = admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }); + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ code: 'session_id_conflict' }); + + scan.resolve(false); + (await first).release(); + }); + + it('checks every live bridge and every registered persistence target', async () => { + const current = fakeBridge(); + const draining = fakeBridge([SESSION_ID]); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [current, draining], + getPersistenceTargets: () => [], + }); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge: current, + workspaceCwd: '/current', + }), + ).rejects.toMatchObject({ + code: 'session_id_conflict', + details: { conflict: 'live', liveWorkspaceCwd: '/live' }, + }); + + const diskAdmission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [current], + getPersistenceTargets: () => [ + { workspaceCwd: '/one', runtimeBaseDir: '/runtime-one' }, + { workspaceCwd: '/two', runtimeBaseDir: '/runtime-two' }, + ], + }); + sessionServiceMock.exists.mockImplementation(async (cwd) => cwd === '/two'); + await expect( + diskAdmission.reserveCreate(SESSION_ID, { + bridge: current, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_conflict', + details: { conflict: 'persisted', liveWorkspaceCwd: '/two' }, + }); + expect(sessionServiceMock.exists).toHaveBeenCalledWith( + '/two', + '/runtime-two', + SESSION_ID, + ); + }); + + it.each(['active', 'archived'] as const)( + 'treats an %s worktree sidecar as persisted history', + async (persistedState) => { + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [ + { workspaceCwd: '/one', runtimeBaseDir: '/runtime-one' }, + ], + }); + const tempDir = await fsp.mkdtemp( + path.join(os.tmpdir(), 'requested-session-id-'), + ); + tempDirs.push(tempDir); + const persistedSidecar = path.join( + tempDir, + `${persistedState}.worktree.json`, + ); + await fsp.writeFile(persistedSidecar, '{}'); + sessionServiceMock.sidecarPath.mockImplementation( + (_cwd, _runtimeBaseDir, _sessionId, state) => + state === persistedState + ? persistedSidecar + : path.join(tempDir, `${state}.missing.json`), + ); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_conflict', + details: { conflict: 'persisted' }, + }); + }, + ); + + it('shares restore claims only on the same bridge generation', () => { + const firstBridge = fakeBridge(); + const secondBridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [firstBridge, secondBridge], + getPersistenceTargets: () => [], + }); + const first = admission.reserveRestore(SESSION_ID, { + bridge: firstBridge, + workspaceCwd: '/one', + }); + const second = admission.reserveRestore(SESSION_ID, { + bridge: firstBridge, + workspaceCwd: '/alias-for-one', + workspaceId: 'same-generation', + }); + expect(() => + admission.reserveRestore(SESSION_ID, { + bridge: secondBridge, + workspaceCwd: '/two', + }), + ).toThrowError(RequestedSessionIdAdmissionError); + first.release(); + expect(() => + admission.reserveRestore(SESSION_ID, { + bridge: secondBridge, + workspaceCwd: '/two', + }), + ).toThrowError(RequestedSessionIdAdmissionError); + second.release(); + admission + .reserveRestore(SESSION_ID, { + bridge: secondBridge, + workspaceCwd: '/two', + }) + .release(); + }); + + it('treats mixed-case caller UUIDs as the same restore claim', () => { + const firstBridge = fakeBridge(); + const secondBridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [firstBridge, secondBridge], + getPersistenceTargets: () => [], + }); + const first = admission.reserveRestore(SESSION_ID, { + bridge: firstBridge, + workspaceCwd: '/one', + }); + + expect(() => + admission.reserveRestore(SESSION_ID.toUpperCase(), { + bridge: secondBridge, + workspaceCwd: '/two', + }), + ).toThrowError(RequestedSessionIdAdmissionError); + + first.release(); + }); + + it('accepts an equivalent workspace spelling on the same live bridge', () => { + const bridge = fakeBridge([SESSION_ID]); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [], + }); + + admission + .reserveRestore(SESSION_ID, { + bridge, + workspaceCwd: '/alias-for-live', + }) + .release(); + }); + + it('names the live foreign owner workspace id on restore conflicts', () => { + const liveBridge = fakeBridge([SESSION_ID]); + const foreignBridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [liveBridge, foreignBridge], + getPersistenceTargets: () => [], + getBridgeWorkspaceId: (bridge) => + bridge === liveBridge ? 'live-workspace' : undefined, + }); + + let error: unknown; + try { + admission.reserveRestore(SESSION_ID, { + bridge: foreignBridge, + workspaceCwd: '/two', + }); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(RequestedSessionIdAdmissionError); + expect((error as RequestedSessionIdAdmissionError).details).toMatchObject({ + conflict: 'live', + liveWorkspaceCwd: '/live', + liveWorkspaceId: 'live-workspace', + }); + }); + + it('does not let a stale release remove a newer claim', async () => { + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [], + }); + const stale = admission.reserveRestore(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }); + stale.release(); + const current = await admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }); + stale.release(); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ code: 'session_id_conflict' }); + current.release(); + }); + + it('returns retryable unavailable and releases when a scan fails', async () => { + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [ + { workspaceCwd: '/one', runtimeBaseDir: '/runtime-one' }, + ], + }); + sessionServiceMock.exists.mockRejectedValueOnce(new Error('scan failed')); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_admission_unavailable', + details: { retryable: true }, + }); + + const retry = await admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }); + retry.release(); + }); + + it('returns retryable unavailable for a non-ENOENT sidecar error', async () => { + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [ + { workspaceCwd: '/one', runtimeBaseDir: '/runtime-one' }, + ], + }); + const tempDir = await fsp.mkdtemp( + path.join(os.tmpdir(), 'requested-session-id-error-'), + ); + tempDirs.push(tempDir); + const nonDirectory = path.join(tempDir, 'not-a-directory'); + await fsp.writeFile(nonDirectory, 'file'); + sessionServiceMock.sidecarPath.mockReturnValue( + path.join(nonDirectory, 'sidecar.json'), + ); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_admission_unavailable', + details: { retryable: true }, + }); + }); + + it('fails closed when a bridge cannot be inspected', async () => { + const bridge = { + getSessionSummary() { + throw new Error('bridge unavailable'); + }, + } as unknown as AcpSessionBridge; + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => [bridge], + getPersistenceTargets: () => [], + }); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_admission_unavailable', + details: { retryable: true }, + }); + }); + + it('fails closed when live bridges cannot be enumerated', async () => { + const bridge = fakeBridge(); + const admission = createRequestedSessionIdAdmission({ + archiveCoordinator: new SessionArchiveCoordinator(), + getBridges: () => { + throw new Error('registry unavailable'); + }, + getPersistenceTargets: () => [], + }); + + await expect( + admission.reserveCreate(SESSION_ID, { + bridge, + workspaceCwd: '/one', + }), + ).rejects.toMatchObject({ + code: 'session_id_admission_unavailable', + details: { retryable: true }, + }); + }); +}); diff --git a/packages/cli/src/serve/session-id-admission.ts b/packages/cli/src/serve/session-id-admission.ts new file mode 100644 index 00000000000..72c15c66557 --- /dev/null +++ b/packages/cli/src/serve/session-id-admission.ts @@ -0,0 +1,324 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { SessionService } from '@qwen-code/qwen-code-core'; +import { access } from 'node:fs/promises'; +import { + SessionNotFoundError, + type AcpSessionBridge, +} from './acp-session-bridge.js'; +import { normalizeSessionIdForLookup } from '../config/session-id.js'; +import type { SessionArchiveCoordinator } from './server/session-archive.js'; + +export interface RequestedSessionIdTarget { + readonly bridge: AcpSessionBridge; + readonly workspaceCwd: string; + readonly workspaceId?: string; +} + +export interface RequestedSessionIdPersistenceTarget { + readonly workspaceCwd: string; + readonly runtimeBaseDir: string; +} + +export interface RequestedSessionIdReservation { + release(): void; +} + +export type RequestedSessionIdAdmissionErrorCode = + | 'session_id_conflict' + | 'session_workspace_conflict' + | 'session_id_admission_unavailable'; + +export class RequestedSessionIdAdmissionError extends Error { + override readonly name = 'RequestedSessionIdAdmissionError'; + + constructor( + readonly code: RequestedSessionIdAdmissionErrorCode, + readonly sessionId: string, + message: string, + readonly details: { + conflict?: 'live' | 'pending' | 'persisted'; + workspaceCwd?: string; + workspaceId?: string; + liveWorkspaceCwd?: string; + liveWorkspaceId?: string; + retryable?: boolean; + } = {}, + ) { + super(message); + } +} + +interface PendingCreate { + readonly kind: 'create'; + readonly target: RequestedSessionIdTarget; +} + +interface PendingRestore { + readonly kind: 'restore'; + readonly target: RequestedSessionIdTarget; + count: number; +} + +type PendingAdmission = PendingCreate | PendingRestore; + +async function persistedSessionExists( + sessionService: SessionService, + sessionId: string, +): Promise { + if ((await sessionService.getSessionLocation(sessionId)) !== undefined) { + return true; + } + if ( + (await sessionService.findSessionIdIgnoringCase?.(sessionId)) !== undefined + ) { + return true; + } + + for (const state of ['active', 'archived'] as const) { + try { + await access( + sessionService.getWorktreeSessionPathForArchiveState(sessionId, state), + ); + return true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } + } + return false; +} + +export interface RequestedSessionIdAdmissionOptions { + readonly archiveCoordinator: SessionArchiveCoordinator; + readonly getBridges: () => readonly AcpSessionBridge[]; + readonly getPersistenceTargets: () => readonly RequestedSessionIdPersistenceTarget[]; + /** + * Resolves the registered workspace id for a live bridge, so conflict + * responses can name the foreign owner. Absent for bridges whose runtime + * is no longer registered (a replaced generation still draining). + */ + readonly getBridgeWorkspaceId?: ( + bridge: AcpSessionBridge, + ) => string | undefined; +} + +export interface RequestedSessionIdAdmission { + reserveCreate( + sessionId: string, + target: RequestedSessionIdTarget, + ): Promise; + reserveRestore( + sessionId: string, + target: RequestedSessionIdTarget, + ): RequestedSessionIdReservation; +} + +export function createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges, + getPersistenceTargets, + getBridgeWorkspaceId, +}: RequestedSessionIdAdmissionOptions): RequestedSessionIdAdmission { + const pending = new Map(); + + const liveOwners = ( + sessionId: string, + ): Array<{ + bridge: AcpSessionBridge; + workspaceCwd: string; + workspaceId?: string; + }> => { + const owners: Array<{ + bridge: AcpSessionBridge; + workspaceCwd: string; + workspaceId?: string; + }> = []; + let bridges: readonly AcpSessionBridge[]; + try { + bridges = getBridges(); + } catch { + throw new RequestedSessionIdAdmissionError( + 'session_id_admission_unavailable', + sessionId, + `Unable to enumerate live bridges for session "${sessionId}".`, + { retryable: true }, + ); + } + for (const bridge of new Set(bridges)) { + try { + const summary = bridge.getSessionSummary(sessionId); + owners.push({ + bridge, + workspaceCwd: summary.workspaceCwd, + workspaceId: getBridgeWorkspaceId?.(bridge), + }); + } catch (error) { + if (error instanceof SessionNotFoundError) continue; + throw new RequestedSessionIdAdmissionError( + 'session_id_admission_unavailable', + sessionId, + `Unable to verify whether session "${sessionId}" is already live.`, + { retryable: true }, + ); + } + } + return owners; + }; + + const conflict = ( + sessionId: string, + conflictKind: 'live' | 'pending' | 'persisted', + workspaceCwd?: string, + ) => + new RequestedSessionIdAdmissionError( + 'session_id_conflict', + sessionId, + `Session "${sessionId}" already exists or is being created.`, + { + conflict: conflictKind, + ...(workspaceCwd ? { liveWorkspaceCwd: workspaceCwd } : {}), + }, + ); + + const workspaceConflict = ( + sessionId: string, + target: RequestedSessionIdTarget, + conflictKind: 'live' | 'pending', + liveWorkspaceCwd?: string, + liveWorkspaceId?: string, + ) => + new RequestedSessionIdAdmissionError( + 'session_workspace_conflict', + sessionId, + `Session "${sessionId}" is already live or restoring in another workspace runtime.`, + { + conflict: conflictKind, + workspaceCwd: target.workspaceCwd, + ...(target.workspaceId ? { workspaceId: target.workspaceId } : {}), + ...(liveWorkspaceCwd ? { liveWorkspaceCwd } : {}), + ...(liveWorkspaceId ? { liveWorkspaceId } : {}), + }, + ); + + const createReservation = ( + sessionId: string, + state: PendingAdmission, + ): RequestedSessionIdReservation => { + let released = false; + return { + release() { + if (released) return; + released = true; + if (pending.get(sessionId) !== state) return; + if (state.kind === 'restore' && state.count > 1) { + state.count--; + return; + } + pending.delete(sessionId); + }, + }; + }; + + return { + async reserveCreate(rawSessionId, target) { + const sessionId = normalizeSessionIdForLookup(rawSessionId); + const live = liveOwners(sessionId)[0]; + if (live) throw conflict(sessionId, 'live', live.workspaceCwd); + if (pending.has(sessionId)) throw conflict(sessionId, 'pending'); + + const state: PendingCreate = { kind: 'create', target }; + pending.set(sessionId, state); + const reservation = createReservation(sessionId, state); + try { + let persisted: RequestedSessionIdPersistenceTarget | undefined; + try { + persisted = await archiveCoordinator.runSharedMany( + [sessionId], + async () => { + const targets = [ + ...new Map( + getPersistenceTargets().map((entry) => [ + `${entry.runtimeBaseDir}\0${entry.workspaceCwd}`, + entry, + ]), + ).values(), + ]; + const results = await Promise.all( + targets.map(async (entry) => { + const sessionService = new SessionService( + entry.workspaceCwd, + { runtimeBaseDir: entry.runtimeBaseDir }, + ); + return { + entry, + exists: await persistedSessionExists( + sessionService, + sessionId, + ), + }; + }), + ); + return results.find((result) => result.exists)?.entry; + }, + ); + } catch { + throw new RequestedSessionIdAdmissionError( + 'session_id_admission_unavailable', + sessionId, + `Unable to verify persisted state for session "${sessionId}".`, + { retryable: true }, + ); + } + if (persisted) { + throw conflict(sessionId, 'persisted', persisted.workspaceCwd); + } + return reservation; + } catch (error) { + reservation.release(); + throw error; + } + }, + + reserveRestore(rawSessionId, target) { + const sessionId = normalizeSessionIdForLookup(rawSessionId); + const foreignLive = liveOwners(sessionId).find( + (owner) => owner.bridge !== target.bridge, + ); + if (foreignLive) { + throw workspaceConflict( + sessionId, + target, + 'live', + foreignLive.workspaceCwd, + foreignLive.workspaceId, + ); + } + + const existing = pending.get(sessionId); + if (existing?.kind === 'create') { + throw conflict(sessionId, 'pending'); + } + if (existing?.kind === 'restore') { + if (existing.target.bridge !== target.bridge) { + throw workspaceConflict( + sessionId, + target, + 'pending', + existing.target.workspaceCwd, + existing.target.workspaceId, + ); + } + existing.count++; + return createReservation(sessionId, existing); + } + + const state: PendingRestore = { kind: 'restore', target, count: 1 }; + pending.set(sessionId, state); + return createReservation(sessionId, state); + }, + }; +} diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index 77a6583e7d6..3a4548dcb29 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -2255,6 +2255,21 @@ describe('SessionService', () => { }); }); + describe('findSessionIdIgnoringCase', () => { + it('finds a legacy mixed-case transcript', async () => { + const legacySessionId = sessionIdA.toUpperCase(); + readdirSyncSpy.mockReturnValue([`${legacySessionId}.jsonl`] as never); + vi.spyOn(sessionService, 'getSessionLocation').mockImplementation( + async (sessionId) => + sessionId === legacySessionId ? 'active' : undefined, + ); + + await expect( + sessionService.findSessionIdIgnoringCase(sessionIdA), + ).resolves.toBe(legacySessionId); + }); + }); + describe('loadLastSession', () => { it('should return the most recent session (same as getLatestSession)', async () => { const now = Date.now(); diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index 6823a5b2065..a28443ac275 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -541,6 +541,33 @@ export class SessionService { return undefined; } + /** + * Finds a persisted session whose UUID filename differs only by case. + * Legacy CLI sessions may have been written with `uuidgen`'s uppercase + * spelling, while daemon-facing caller IDs are canonicalized to lowercase. + */ + async findSessionIdIgnoringCase( + sessionId: string, + ): Promise { + const expectedFileName = `${sessionId}.jsonl`.toLowerCase(); + for (const state of ['active', 'archived'] as const) { + let fileNames: string[]; + try { + fileNames = fs.readdirSync(this.getChatsDirForState(state)); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; + throw error; + } + for (const fileName of fileNames) { + if (fileName.toLowerCase() !== expectedFileName) continue; + const candidateSessionId = fileName.slice(0, -'.jsonl'.length); + const location = await this.getSessionLocation(candidateSessionId); + if (location !== undefined) return candidateSessionId; + } + } + return undefined; + } + private removeFileIfExists(filePath: string): void { try { fs.unlinkSync(filePath); diff --git a/packages/sdk-java/qwencode/README.md b/packages/sdk-java/qwencode/README.md index bd55730ca64..c72825bdee7 100644 --- a/packages/sdk-java/qwencode/README.md +++ b/packages/sdk-java/qwencode/README.md @@ -86,6 +86,20 @@ try (DaemonClient daemon = DaemonClient.builder() } ``` +Callers that need to allocate the session identity before creation can pass an RFC UUID v1-v5. The SDK checks `session_id_override` before the mutation and reports a different returned ID as `SessionCreationOutcomeUnknownException`: + +```java +CreateSessionRequest request = CreateSessionRequest.builder() + .sessionId("550E8400-E29B-41D4-A716-446655440000") + .build(); + +try (DaemonSessionClient session = daemon.createSession(request)) { + System.out.println(session.getSession().getSessionId()); +} +``` + +The daemon normalizes the ID to lowercase and creates a new thread session. This is not an idempotent attach; after an ambiguous create outcome, recover with the known ID rather than retrying creation. + If `qwen serve` requires authentication, add `.bearerToken(System.getenv("QWEN_SERVER_TOKEN"))` to the `DaemonClient` builder. The SDK sends the bearer on REST and SSE requests and never puts it in @@ -101,7 +115,7 @@ Creation-time model selection is intentionally not exposed by the Java daemon SD `PromptRequest.Builder.deadline(Duration)` requests a daemon-enforced prompt deadline and is accepted only when the daemon advertises `prompt_absolute_deadline`; otherwise the SDK fails before sending the prompt. The value must be between 1 and 2,147,483,647 milliseconds, matching the daemon's Node timer range. This is separate from `observationTimeout(Duration)`, which only bounds local SSE observation and never sends a cancel mutation. -Before creating a session, the SDK requires the daemon to advertise the REST transport and `session_scope_override`; this prevents an older daemon from silently ignoring the requested `thread` scope and attaching the client to a shared session. When `client_heartbeat` is advertised, an open session sends a fresh heartbeat every minute so the daemon does not reap an otherwise idle client. Set `heartbeatInterval(Duration.ZERO)` on the `DaemonClient` builder to disable this behavior, or choose a different positive interval. A heartbeat is never retried; the next scheduled heartbeat is a separate keepalive. Prompt observation is bounded to 32 concurrent prompts per client by default and can be adjusted with `maximumConcurrentPrompts`. Admission and terminal future callbacks run away from transport workers; callbacks that remain blocked consume bounded publication capacity. SSE stream cleanup is also bounded, and a close that remains blocked retains its cleanup reservation. Either condition can cause a later `startPrompt` to fail with `DaemonClientCapacityException` rather than dropping a timeout close or growing threads and queued work without limit. +Before creating a session, the SDK requires the daemon to advertise the REST transport and `session_scope_override`; this prevents an older daemon from silently ignoring the requested `thread` scope and attaching the client to a shared session. When a caller supplies a session ID, the SDK additionally requires `session_id_override` before sending the mutation. When `client_heartbeat` is advertised, an open session sends a fresh heartbeat every minute so the daemon does not reap an otherwise idle client. Set `heartbeatInterval(Duration.ZERO)` on the `DaemonClient` builder to disable this behavior, or choose a different positive interval. A heartbeat is never retried; the next scheduled heartbeat is a separate keepalive. Prompt observation is bounded to 32 concurrent prompts per client by default and can be adjusted with `maximumConcurrentPrompts`. Admission and terminal future callbacks run away from transport workers; callbacks that remain blocked consume bounded publication capacity. SSE stream cleanup is also bounded, and a close that remains blocked retains its cleanup reservation. Either condition can cause a later `startPrompt` to fail with `DaemonClientCapacityException` rather than dropping a timeout close or growing threads and queued work without limit. An indeterminate completion is an outcome boundary, not a session-reuse boundary. After `PromptAdmissionUnknownException` or `PromptOutcomeIndeterminateException`, that `DaemonSessionClient` permanently rejects further prompts even if local stream cleanup later succeeds; close or destroy the session instead. An observation timeout is published without waiting forever for a blocked stream close, while cleanup continues asynchronously and retains bounded client capacity until it finishes. diff --git a/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/CreateSessionRequest.java b/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/CreateSessionRequest.java index 162fc086b47..5618d8ab665 100644 --- a/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/CreateSessionRequest.java +++ b/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/CreateSessionRequest.java @@ -8,11 +8,13 @@ public final class CreateSessionRequest { private final String workspaceCwd; private final String approvalMode; private final String sessionScope; + private final String sessionId; private CreateSessionRequest(Builder builder) { this.workspaceCwd = builder.workspaceCwd; this.approvalMode = builder.approvalMode; this.sessionScope = builder.sessionScope; + this.sessionId = builder.sessionId; } public static Builder builder() { @@ -31,14 +33,22 @@ Map toJson() { if (approvalMode != null) { result.put("approvalMode", approvalMode); } + if (sessionId != null) { + result.put("sessionId", sessionId); + } result.put("sessionScope", sessionScope); return result; } + String getSessionId() { + return sessionId; + } + public static final class Builder { private String workspaceCwd; private String approvalMode; private String sessionScope = "thread"; + private String sessionId; private Builder() { } @@ -69,6 +79,11 @@ public Builder sessionScope(String sessionScope) { return this; } + public Builder sessionId(String sessionId) { + this.sessionId = requireNonBlank(sessionId, "sessionId"); + return this; + } + public CreateSessionRequest build() { return new CreateSessionRequest(this); } diff --git a/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/DaemonClient.java b/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/DaemonClient.java index 2e64317e8ea..9f8e6461fa0 100644 --- a/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/DaemonClient.java +++ b/packages/sdk-java/qwencode/src/main/java/com/alibaba/qwen/code/daemon/DaemonClient.java @@ -11,6 +11,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; @@ -171,6 +172,12 @@ public DaemonSessionClient createSession(CreateSessionRequest request) { "The daemon does not advertise session_scope_override; " + "the SDK cannot guarantee the requested session scope"); } + if (request.getSessionId() != null + && !capabilities.supports("session_id_override")) { + throw new DaemonProtocolException( + "The daemon does not advertise session_id_override; " + + "the SDK cannot guarantee the requested session ID"); + } synchronized (lifecycleLock) { ensureOpen(); } @@ -196,8 +203,21 @@ public DaemonSessionClient createSession(CreateSessionRequest request) { String clientId = JsonSupport.requiredString(json, "clientId", "session"); validateClientId(clientId); + String sessionId = JsonSupport.requiredString(json, "sessionId", + "session"); + String requestedSessionId = request.getSessionId() == null + ? null + : request.getSessionId().toLowerCase(Locale.ROOT); + if (requestedSessionId != null + && !requestedSessionId.equals(sessionId)) { + throw new SessionCreationOutcomeUnknownException( + new DaemonProtocolException( + "Daemon returned session \"" + sessionId + + "\" instead of requested session \"" + + requestedSessionId + "\"")); + } DaemonSession session = new DaemonSession( - JsonSupport.requiredString(json, "sessionId", "session"), + sessionId, JsonSupport.requiredString(json, "workspaceCwd", "session"), JsonSupport.requiredBoolean(json, "attached", "session"), clientId, diff --git a/packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/daemon/DaemonSessionClientTest.java b/packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/daemon/DaemonSessionClientTest.java index bcd962bbdd3..374d74bf337 100644 --- a/packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/daemon/DaemonSessionClientTest.java +++ b/packages/sdk-java/qwencode/src/test/java/com/alibaba/qwen/code/daemon/DaemonSessionClientTest.java @@ -48,7 +48,7 @@ void setUp() throws IOException { server.createContext("/capabilities", exchange -> sendJson(exchange, 200, "{\"v\":1,\"mode\":\"http-bridge\",\"features\":[" + "\"session_scope_override\",\"client_heartbeat\"," - + "\"prompt_absolute_deadline\"]," + + "\"prompt_absolute_deadline\",\"session_id_override\"]," + "\"transports\":[\"rest\"]}")); server.createContext("/session", exchange -> { if ("POST".equals(exchange.getRequestMethod()) @@ -200,6 +200,59 @@ void refusesToCreateWhenRestTransportIsNotAdvertised() { } } + @Test + void serializesAndVerifiesCallerSuppliedSessionId() { + String sessionId = "550e8400-e29b-41d4-a716-446655440000"; + String requestedSessionId = sessionId.toUpperCase(java.util.Locale.ROOT); + server.removeContext("/session"); + server.createContext("/session", exchange -> { + createBody.set(new String(exchange.getRequestBody().readAllBytes(), + StandardCharsets.UTF_8)); + sendJson(exchange, 200, sessionJson(sessionId, "client-fixed")); + }); + server.createContext("/session/" + sessionId + "/detach", noContent()); + + try (DaemonClient daemon = newClient(); + DaemonSessionClient ignored = daemon.createSession( + CreateSessionRequest.builder().sessionId(requestedSessionId).build())) { + assertTrue(createBody.get().contains( + "\"sessionId\":\"" + requestedSessionId + "\"")); + } + } + + @Test + void refusesCallerSuppliedSessionIdBeforeMutationWhenCapabilityIsMissing() { + server.removeContext("/capabilities"); + server.createContext("/capabilities", exchange -> sendJson(exchange, 200, + "{\"v\":1,\"mode\":\"http-bridge\",\"features\":[" + + "\"session_scope_override\"]," + + "\"transports\":[\"rest\"]}")); + + try (DaemonClient daemon = newClient()) { + assertThrows(DaemonProtocolException.class, + () -> daemon.createSession(CreateSessionRequest.builder() + .sessionId("550e8400-e29b-41d4-a716-446655440000") + .build())); + assertNull(createBody.get()); + } + } + + @Test + void mismatchedCallerSuppliedSessionIdIsOutcomeUnknown() { + server.removeContext("/session"); + server.createContext("/session", exchange -> sendJson(exchange, 200, + sessionJson("550e8400-e29b-41d4-a716-446655440999", "client-fixed"))); + + try (DaemonClient daemon = newClient()) { + SessionCreationOutcomeUnknownException failure = assertThrows( + SessionCreationOutcomeUnknownException.class, + () -> daemon.createSession(CreateSessionRequest.builder() + .sessionId("550e8400-e29b-41d4-a716-446655440000") + .build())); + assertInstanceOf(DaemonProtocolException.class, failure.getCause()); + } + } + @Test void slowSessionCreationDoesNotBlockExistingSessionMutation() throws Exception { diff --git a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/README.md b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/README.md index ae3e5b12f3a..f888e4693b6 100644 --- a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/README.md +++ b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/README.md @@ -129,14 +129,14 @@ await server.instance.connect(transport); ### Session Lifecycle（6） -| 工具名 | 说明 | -| ------------------------- | --------------------------------- | -| `session_create` | 创建/附加会话（自动设为默认会话） | -| `session_load` | 恢复会话（含历史回放） | -| `session_resume` | 恢复会话（无历史） | -| `session_close` | 关闭会话 | -| `session_update_metadata` | 更新会话元数据 | -| `session_list` | 列出工作区会话 | +| 工具名 | 说明 | +| ------------------------- | --------------------------------------------------------------------- | +| `session_create` | 创建/附加会话；可用 `session_id` 指定新 thread ID（自动设为默认会话） | +| `session_load` | 恢复会话（含历史回放） | +| `session_resume` | 恢复会话（无历史） | +| `session_close` | 关闭会话 | +| `session_update_metadata` | 更新会话元数据 | +| `session_list` | 列出工作区会话 | ### Agent Interaction（4） @@ -185,6 +185,8 @@ MCP 协议是无状态的，但大部分工具需要 `session_id`。本 MCP Serv 3. 也可显式传入 `session_id` 操作多个会话 4. `session_close` 关闭默认会话时自动清除缓存 +`session_create.session_id` 仅接受 UUID v1-v5。提供该字段时，bridge 会在发出 mutation 前检查 daemon 的 `session_id_override` capability，并要求返回同一个小写 ID。它表示创建新 thread，不是幂等 attach；结果不确定时应使用已知 ID 调用 load/resume。 + ## 验证 ```bash diff --git a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/session.ts b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/session.ts index 4336c4c5233..f976e796b60 100644 --- a/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/session.ts +++ b/packages/sdk-typescript/src/daemon-mcp/serve-bridge/tools/session.ts @@ -26,6 +26,10 @@ export function sessionTools(state: BridgeState): any[] { .string() .optional() .describe('Model service to use.'), + session_id: z + .string() + .optional() + .describe('UUID v1-v5 to assign to the new session.'), session_scope: z .enum(['single', 'thread']) .optional() @@ -34,6 +38,7 @@ export function sessionTools(state: BridgeState): any[] { handler(async (args) => { const session = await state.client.createOrAttachSession({ workspaceCwd: args.workspace_cwd ?? state.workspaceCwd, + sessionId: args.session_id, modelServiceId: args.model_service_id, sessionScope: args.session_scope, }); diff --git a/packages/sdk-typescript/src/daemon/AcpWsTransport.ts b/packages/sdk-typescript/src/daemon/AcpWsTransport.ts index b56c309ec75..d95d32c1fb1 100644 --- a/packages/sdk-typescript/src/daemon/AcpWsTransport.ts +++ b/packages/sdk-typescript/src/daemon/AcpWsTransport.ts @@ -66,8 +66,9 @@ const INIT_TIMEOUT_MS = 30_000; * single WS connection using JSON-RPC 2.0 framing. * * Lazy-init: the WebSocket connection is established on the first - * `fetch()` call. An `initialize` JSON-RPC request is sent on - * connect and its result is cached for `GET /capabilities` requests. + * `fetch()` call. An `initialize` JSON-RPC request is sent on connect. Daemon + * capabilities use REST discovery, with the initialize result retained only + * as a fallback for ACP-only deployments. * * **Browser limitation**: The browser WebSocket API does not support * custom headers on the upgrade request. In Node (>=22), the token @@ -109,7 +110,7 @@ export class AcpWsTransport implements DaemonTransport { readonly type = 'acp-ws' as const; readonly supportsReplay = false; - readonly restFetch: typeof globalThis.fetch | undefined; + readonly restFetch: typeof globalThis.fetch; constructor( wsUrl: string, @@ -118,7 +119,10 @@ export class AcpWsTransport implements DaemonTransport { ) { this.wsUrl = wsUrl; this.token = token; - this.restFetch = restFetch; + // Resolve globalThis.fetch lazily so the transport still constructs in + // environments where fetch only exists later (or is injected per call). + this.restFetch = + restFetch ?? ((input, init) => globalThis.fetch(input, init)); } get connected(): boolean { @@ -162,9 +166,33 @@ export class AcpWsTransport implements DaemonTransport { const { mapping, segments } = match; - // Special handling for capabilities — return cached init result. + // The ACP initialize result has a different schema from the daemon + // capabilities envelope. Prefer the REST discovery response when this + // transport was constructed with a REST fetch (as negotiateTransport + // does), and retain the initialize result only as an ACP-only fallback. + // The envelope must actually carry a `features` array: a reverse proxy + // or SPA can answer 200 with HTML for unknown paths, and accepting that + // body would resurface the very `caps.features` TypeError this path + // exists to avoid. if (mapping.method === '_capabilities') { - return synthesizeResponse(200, this.initResult ?? { v: 1 }); + try { + const response = await this.restFetch(url, init); + if (!response.ok) { + if (response.status !== 404) return response; + } else { + const envelope: unknown = await response.clone().json(); + if (isRecord(envelope) && Array.isArray(envelope['features'])) { + return response; + } + } + } catch { + // ACP-only deployments can still use the initialize fallback. + } + return synthesizeResponse(200, { + ...(isRecord(this.initResult) ? this.initResult : {}), + v: 1, + features: [], + }); } // For notifications, send and return 204 immediately. diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index a842ccd635b..3dfadc3ce67 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -463,6 +463,18 @@ export class DaemonPendingPromptLimitError extends Error { } } +export class DaemonSessionIdProtocolError extends Error { + constructor( + readonly requestedSessionId: string, + readonly actualSessionId: string, + ) { + super( + `Daemon returned session "${actualSessionId}" instead of requested session "${requestedSessionId}".`, + ); + this.name = 'DaemonSessionIdProtocolError'; + } +} + export interface DaemonTurnError extends DaemonHttpError { _daemonTurnError: true; } @@ -489,6 +501,11 @@ export interface CreateSessionRequest { * `400 workspace_mismatch` `DaemonHttpError`. */ workspaceCwd?: string; + /** + * UUID v1-v5 to assign to a new thread session. This is creation, not an + * idempotent attach; use load/resume after an ambiguous response. + */ + sessionId?: string; modelServiceId?: string; /** * Per-request session-scope override. The production daemon defaults @@ -1045,7 +1062,7 @@ export class DaemonClient { async requireCapability(capability: string): Promise { const caps = await this.capabilities(); - if (!caps.features.includes(capability)) { + if (!Array.isArray(caps.features) || !caps.features.includes(capability)) { throw new DaemonCapabilityMissingError( capability, `daemon does not advertise the ${capability} feature`, @@ -2270,6 +2287,9 @@ export class DaemonClient { req: CreateSessionRequest, clientId?: string, ): Promise { + if (req.sessionId !== undefined && req.sessionId !== null) { + await this.requireCapability('session_id_override'); + } if (req.sourceType !== undefined || req.sourceId !== undefined) { await this.requireCapability('session_source_metadata'); } @@ -2292,6 +2312,7 @@ export class DaemonClient { headers: this.headers({ 'Content-Type': 'application/json' }, clientId), body: JSON.stringify({ cwd: req.workspaceCwd, + ...(req.sessionId !== undefined ? { sessionId: req.sessionId } : {}), ...(req.modelServiceId ? { modelServiceId: req.modelServiceId } : {}), // `!== undefined` (not truthy) so a buggy caller passing // `sessionScope: '' | null` doesn't get the field silently @@ -2315,7 +2336,17 @@ export class DaemonClient { }, async (res) => { if (!res.ok) throw await this.failOnError(res, 'POST /session'); - return (await res.json()) as DaemonSession; + const session = (await res.json()) as DaemonSession; + if ( + typeof req.sessionId === 'string' && + session.sessionId !== req.sessionId.toLowerCase() + ) { + throw new DaemonSessionIdProtocolError( + req.sessionId.toLowerCase(), + session.sessionId, + ); + } + return session; }, ); } diff --git a/packages/sdk-typescript/src/daemon/acpRouteTable.ts b/packages/sdk-typescript/src/daemon/acpRouteTable.ts index 3d301210f0e..f29cd5e408a 100644 --- a/packages/sdk-typescript/src/daemon/acpRouteTable.ts +++ b/packages/sdk-typescript/src/daemon/acpRouteTable.ts @@ -14,6 +14,8 @@ import { isRecord } from './acpTransportUtils.js'; +const REQUESTED_SESSION_ID_META_KEY = 'qwen-code/sessionId'; + export interface RouteMapping { method: string; /** @@ -105,8 +107,22 @@ export const ROUTE_TABLE: readonly RouteEntry[] = [ method: 'session/new', extractParams: (_s, body) => { if (!isRecord(body)) return {}; - const { sessionScope: _, ...rest } = body as Record; - return rest; + const { + sessionScope: _, + sessionId, + _meta, + ...rest + } = body as Record; + if (sessionId === undefined) { + return { ...rest, ...(_meta !== undefined ? { _meta } : {}) }; + } + return { + ...rest, + _meta: { + ...(isRecord(_meta) ? _meta : {}), + [REQUESTED_SESSION_ID_META_KEY]: sessionId, + }, + }; }, }, }, diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 20143cb946b..d5d626f277d 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -8,6 +8,7 @@ export { DaemonClient, DaemonHttpError, DaemonPendingPromptLimitError, + DaemonSessionIdProtocolError, EXTENSION_ARCHIVE_UPLOAD_TIMEOUT_MS, WorkspaceDaemonClient, isDaemonTurnError, diff --git a/packages/sdk-typescript/src/index.ts b/packages/sdk-typescript/src/index.ts index dabb7efbb4d..df04bd0de40 100644 --- a/packages/sdk-typescript/src/index.ts +++ b/packages/sdk-typescript/src/index.ts @@ -15,6 +15,7 @@ export { DaemonClient, DaemonHttpError, DaemonPendingPromptLimitError, + DaemonSessionIdProtocolError, WorkspaceDaemonClient, DaemonSessionClient, asKnownDaemonEvent, diff --git a/packages/sdk-typescript/src/utils/validation.ts b/packages/sdk-typescript/src/utils/validation.ts index a4719c44b80..8213749d9ea 100644 --- a/packages/sdk-typescript/src/utils/validation.ts +++ b/packages/sdk-typescript/src/utils/validation.ts @@ -2,7 +2,7 @@ * UUID validation utilities */ -// UUID v4 regex pattern +// RFC-variant UUID v1-v5 regex pattern const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; diff --git a/packages/sdk-typescript/test/unit/AcpWsTransport.test.ts b/packages/sdk-typescript/test/unit/AcpWsTransport.test.ts index cb889c1263b..666132c3346 100644 --- a/packages/sdk-typescript/test/unit/AcpWsTransport.test.ts +++ b/packages/sdk-typescript/test/unit/AcpWsTransport.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { AcpWsTransport } from '../../src/daemon/AcpWsTransport.js'; import { DaemonTransportClosedError } from '../../src/daemon/DaemonTransport.js'; import { @@ -56,6 +56,175 @@ describe('AcpWsTransport', () => { expect(t2.type).toBe('acp-ws'); t2.dispose(); }); + + it('uses REST capabilities instead of the ACP initialize envelope', async () => { + const originalWebSocket = globalThis.WebSocket; + const originalFetch = Object.getOwnPropertyDescriptor( + globalThis, + 'fetch', + ); + class FakeWebSocket { + onopen: (() => void) | null = null; + onmessage: ((event: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: (() => void) | null = null; + + constructor() { + queueMicrotask(() => this.onopen?.()); + } + + send(payload: string) { + const request = JSON.parse(payload) as { id: number }; + queueMicrotask(() => + this.onmessage?.({ + data: JSON.stringify({ + jsonrpc: '2.0', + id: request.id, + result: { protocolVersion: 1, agentCapabilities: {} }, + }), + }), + ); + } + + close() { + this.onclose?.(); + } + } + Object.defineProperty(globalThis, 'WebSocket', { + configurable: true, + value: FakeWebSocket, + }); + const restFetch = vi.fn(async () => + synthesizeResponse(200, { + v: 1, + mode: 'http-bridge', + features: ['session_id_override'], + transports: ['acp-ws'], + }), + ); + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + value: restFetch, + }); + const transport = new AcpWsTransport('ws://daemon/acp'); + + try { + const response = await transport.fetch('http://daemon/capabilities', { + method: 'GET', + }); + await expect(response.json()).resolves.toMatchObject({ + features: ['session_id_override'], + }); + expect(restFetch).toHaveBeenCalledOnce(); + } finally { + transport.dispose(); + Object.defineProperty(globalThis, 'WebSocket', { + configurable: true, + value: originalWebSocket, + }); + if (originalFetch) { + Object.defineProperty(globalThis, 'fetch', originalFetch); + } else { + Reflect.deleteProperty(globalThis, 'fetch'); + } + } + }); + + it('constructs when globalThis.fetch is undefined', () => { + const originalFetch = Object.getOwnPropertyDescriptor( + globalThis, + 'fetch', + ); + Reflect.deleteProperty(globalThis, 'fetch'); + try { + expect(() => new AcpWsTransport('ws://host/acp')).not.toThrow(); + } finally { + if (originalFetch) { + Object.defineProperty(globalThis, 'fetch', originalFetch); + } + } + }); + + it('falls back for a malformed 200 and preserves REST errors', async () => { + const originalWebSocket = globalThis.WebSocket; + const originalFetch = Object.getOwnPropertyDescriptor( + globalThis, + 'fetch', + ); + class FakeWebSocket { + onopen: (() => void) | null = null; + onmessage: ((event: { data: string }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: (() => void) | null = null; + + constructor() { + queueMicrotask(() => this.onopen?.()); + } + + send(payload: string) { + const request = JSON.parse(payload) as { id: number }; + queueMicrotask(() => + this.onmessage?.({ + data: JSON.stringify({ + jsonrpc: '2.0', + id: request.id, + result: { protocolVersion: 1, agentCapabilities: {} }, + }), + }), + ); + } + + close() { + this.onclose?.(); + } + } + Object.defineProperty(globalThis, 'WebSocket', { + configurable: true, + value: FakeWebSocket, + }); + // A reverse proxy / SPA answering 200 with a body that is not the + // daemon capabilities envelope must not be accepted as one. + const restFetch = vi + .fn() + .mockResolvedValueOnce( + synthesizeResponse(200, { index: 'spa fallback, no features' }), + ) + .mockResolvedValueOnce( + synthesizeResponse(401, { error: 'unauthorized' }), + ); + Object.defineProperty(globalThis, 'fetch', { + configurable: true, + value: restFetch, + }); + const transport = new AcpWsTransport('ws://daemon/acp'); + + try { + const response = await transport.fetch('http://daemon/capabilities', { + method: 'GET', + }); + await expect(response.json()).resolves.toMatchObject({ + protocolVersion: 1, + features: [], + }); + const unauthorized = await transport.fetch( + 'http://daemon/capabilities', + { method: 'GET' }, + ); + expect(unauthorized.status).toBe(401); + expect(restFetch).toHaveBeenCalledTimes(2); + } finally { + transport.dispose(); + Object.defineProperty(globalThis, 'WebSocket', { + configurable: true, + value: originalWebSocket, + }); + if (originalFetch) { + Object.defineProperty(globalThis, 'fetch', originalFetch); + } else { + Reflect.deleteProperty(globalThis, 'fetch'); + } + } + }); }); // ---- dispose() -------------------------------------------------------- diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index e75a1c7802d..1955a96d2d4 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -1894,6 +1894,110 @@ describe('DaemonClient', () => { }); describe('createOrAttachSession', () => { + it('gates sessionId before mutation, serializes it, and verifies the response', async () => { + const requested = '550E8400-E29B-41D4-A716-446655440000'; + const { fetch, calls } = recordingFetch((request) => + request.url.endsWith('/capabilities') + ? jsonResponse(200, { + v: 1, + mode: 'http-bridge', + features: ['session_id_override'], + }) + : jsonResponse(200, { + sessionId: requested.toLowerCase(), + workspaceCwd: '/work/a', + attached: false, + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + const session = await client.createOrAttachSession({ + workspaceCwd: '/work/a', + sessionId: requested, + }); + + expect(session.sessionId).toBe(requested.toLowerCase()); + expect(calls.map((call) => call.url)).toEqual([ + 'http://daemon/capabilities', + 'http://daemon/session', + ]); + expect(JSON.parse(calls[1]!.body!)).toMatchObject({ + sessionId: requested, + }); + }); + + it('does not mutate when session_id_override is unavailable', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + v: 1, + mode: 'http-bridge', + features: ['session_create'], + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await expect( + client.createOrAttachSession({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }), + ).rejects.toMatchObject({ + name: 'DaemonCapabilityMissingError', + capability: 'session_id_override', + }); + expect(calls.map((call) => call.url)).toEqual([ + 'http://daemon/capabilities', + ]); + }); + + it('treats a malformed capabilities envelope as missing capability', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + v: 1, + mode: 'http-bridge', + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await expect( + client.createOrAttachSession({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }), + ).rejects.toMatchObject({ + name: 'DaemonCapabilityMissingError', + capability: 'session_id_override', + }); + expect(calls.map((call) => call.url)).toEqual([ + 'http://daemon/capabilities', + ]); + }); + + it('throws a protocol error when the daemon returns a different sessionId', async () => { + const { fetch } = recordingFetch((request) => + request.url.endsWith('/capabilities') + ? jsonResponse(200, { + v: 1, + mode: 'http-bridge', + features: ['session_id_override'], + }) + : jsonResponse(200, { + sessionId: '550e8400-e29b-41d4-a716-446655440999', + workspaceCwd: '/work/a', + attached: false, + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + + await expect( + client.createOrAttachSession({ + sessionId: '550e8400-e29b-41d4-a716-446655440000', + }), + ).rejects.toMatchObject({ + name: 'DaemonSessionIdProtocolError', + requestedSessionId: '550e8400-e29b-41d4-a716-446655440000', + actualSessionId: '550e8400-e29b-41d4-a716-446655440999', + }); + }); + it('POSTs cwd in the body', async () => { const { fetch, calls } = recordingFetch(() => jsonResponse(200, { diff --git a/packages/sdk-typescript/test/unit/acpRouteTable.test.ts b/packages/sdk-typescript/test/unit/acpRouteTable.test.ts index dfab3032d7e..5a13fd66204 100644 --- a/packages/sdk-typescript/test/unit/acpRouteTable.test.ts +++ b/packages/sdk-typescript/test/unit/acpRouteTable.test.ts @@ -57,6 +57,25 @@ describe('acpRouteTable – matchRoute', () => { expect(params).toEqual({ model: 'gpt-4' }); }); + it('POST /session maps sessionId into ACP metadata', () => { + const result = matchRoute('/session', 'POST')!; + const params = result.mapping.extractParams( + result.segments, + { + sessionId: '550E8400-E29B-41D4-A716-446655440000', + sessionScope: 'single', + _meta: { existing: true }, + }, + 'POST', + ); + expect(params).toEqual({ + _meta: { + existing: true, + 'qwen-code/sessionId': '550E8400-E29B-41D4-A716-446655440000', + }, + }); + }); + it('POST /session with non-record body returns empty params', () => { const result = matchRoute('/session', 'POST')!; const params = result.mapping.extractParams( diff --git a/packages/sdk-typescript/test/unit/serve-bridge.test.ts b/packages/sdk-typescript/test/unit/serve-bridge.test.ts index 92e7a9d7088..c74add477d1 100644 --- a/packages/sdk-typescript/test/unit/serve-bridge.test.ts +++ b/packages/sdk-typescript/test/unit/serve-bridge.test.ts @@ -191,6 +191,71 @@ describe('serve-bridge', () => { }); describe('session_create', () => { + it('exposes session_id and forwards it after capability gating', async () => { + const requested = '550e8400-e29b-41d4-a716-446655440000'; + const { state, calls } = makeMockState({ + fetchReply: (req) => { + if (req.url.endsWith('/capabilities')) { + return jsonResponse(200, { + v: 1, + mode: 'http-bridge', + features: ['session_id_override'], + }); + } + if (req.url.endsWith('/session') && req.method === 'POST') { + return jsonResponse(200, { + sessionId: requested, + workspaceCwd: '/tmp', + attached: false, + }); + } + return jsonResponse(404, {}); + }, + }); + const { sessionTools } = await import( + '../../src/daemon-mcp/serve-bridge/tools/session.js' + ); + const createTool = sessionTools(state).find( + (tool: { name: string }) => tool.name === 'session_create', + ); + + await createTool.handler({ session_id: requested }, {}); + + const mutation = calls.find( + (call) => call.url.endsWith('/session') && call.method === 'POST', + ); + expect(JSON.parse(mutation!.body!)).toMatchObject({ + sessionId: requested, + }); + }); + + it('does not mutate when session_id_override is unavailable', async () => { + const { state, calls } = makeMockState({ + fetchReply: () => + jsonResponse(200, { + v: 1, + mode: 'http-bridge', + features: [], + }), + }); + const { sessionTools } = await import( + '../../src/daemon-mcp/serve-bridge/tools/session.js' + ); + const createTool = sessionTools(state).find( + (tool: { name: string }) => tool.name === 'session_create', + ); + + const result = await createTool.handler( + { session_id: '550e8400-e29b-41d4-a716-446655440000' }, + {}, + ); + + expect(result.isError).toBe(true); + expect(calls.map((call) => call.url)).toEqual([ + 'http://127.0.0.1:4170/capabilities', + ]); + }); + it('should set defaultSessionId after successful creation', async () => { const { state } = makeMockState({ fetchReply: (req) => {