diff --git a/docs/design/daemon-multi-workspace-phase2a-sessions.md b/docs/design/daemon-multi-workspace-phase2a-sessions.md index 53d4265eac2..8c3437b54e8 100644 --- a/docs/design/daemon-multi-workspace-phase2a-sessions.md +++ b/docs/design/daemon-multi-workspace-phase2a-sessions.md @@ -3,7 +3,10 @@ > **Historical status:** The live-session rewind snapshots, rewind, and shell > limitations recorded in this document are superseded by > [`daemon-multi-workspace-session-file-ops.md`](./daemon-multi-workspace-session-file-ops.md). -> The remaining Phase 2a scope statements are unchanged. +> The later primary-only classification of live-session continue, language, +> and artifact mutations is also superseded: those singular REST routes now +> dispatch to the owning trusted workspace runtime. The remaining Phase 2a +> scope statements are unchanged. ## Summary diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index bac4bef6a7e..f142c471af0 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -1780,7 +1780,7 @@ ACP-over-HTTP uses the same request and response bodies through vendor methods ` ### Multi-workspace live-session routing -When `multi_workspace_sessions` is advertised, live-session operations identify their workspace from the `sessionId`; clients do not add a workspace selector to the URL. In addition to the existing owner-routed lifecycle operations, this applies to `PATCH /session/:id/metadata`, `POST /session/:id/recap`, `POST /session/:id/btw`, `POST /session/:id/mid-turn-message`, `POST /session/:id/tasks/:taskId/cancel`, and `POST /session/:id/goal/clear`. The daemon routes each request to the trusted runtime that owns the live session. An untrusted non-primary owner returns `403 untrusted_workspace`, a missing live owner returns `404 session_not_found`, and an ambiguous owner fails closed with `500 ambiguous_session_owner`. +When `multi_workspace_sessions` is advertised, live-session operations identify their workspace from the `sessionId`; clients do not add a workspace selector to the URL. In addition to the existing owner-routed lifecycle operations, this applies to `PATCH /session/:id/metadata`, `POST /session/:id/recap`, `POST /session/:id/btw`, `POST /session/:id/mid-turn-message`, `POST /session/:id/tasks/:taskId/cancel`, `POST /session/:id/goal/clear`, `POST /session/:id/continue`, `POST /session/:id/language`, `POST /session/:id/artifacts`, and `DELETE /session/:id/artifacts/:artifactId`. The daemon routes each request to the trusted runtime that owns the live session. An untrusted non-primary owner returns `403 untrusted_workspace`, a missing live owner returns `404 session_not_found`, and an ambiguous owner fails closed with `500 ambiguous_session_owner`. This rule is live-session-only and does not make every workspace-less session route multi-workspace-aware. Persisted or archived operations use their documented workspace-qualified routes, while remaining Phase 2a primary-only routes continue to return `non_primary_session_route_not_supported` for non-primary owners. diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index a2cc23edfb9..981e1b6b7a7 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -119,6 +119,25 @@ interface FakeBridge extends AcpSessionBridge { taskKind: 'agent' | 'shell' | 'monitor'; }>; readonly goalClearCalls: string[]; + readonly continueCalls: Array<{ + sessionId: string; + context?: BridgeClientRequestContext; + }>; + readonly languageCalls: Array<{ + sessionId: string; + params: Parameters[1]; + context?: BridgeClientRequestContext; + }>; + readonly addArtifactCalls: Array<{ + sessionId: string; + artifact: Parameters[1]; + context?: BridgeClientRequestContext; + }>; + readonly removeArtifactCalls: Array<{ + sessionId: string; + artifactId: string; + context?: BridgeClientRequestContext; + }>; readonly rewindSnapshotCalls: string[]; readonly rewindCalls: Array<{ sessionId: string; @@ -256,6 +275,10 @@ function makeBridge( const midTurnMessageCalls: FakeBridge['midTurnMessageCalls'] = []; const taskCancelCalls: FakeBridge['taskCancelCalls'] = []; const goalClearCalls: string[] = []; + const continueCalls: FakeBridge['continueCalls'] = []; + const languageCalls: FakeBridge['languageCalls'] = []; + const addArtifactCalls: FakeBridge['addArtifactCalls'] = []; + const removeArtifactCalls: FakeBridge['removeArtifactCalls'] = []; const rewindSnapshotCalls: string[] = []; const rewindCalls: FakeBridge['rewindCalls'] = []; const shellCalls: FakeBridge['shellCalls'] = []; @@ -282,6 +305,10 @@ function makeBridge( midTurnMessageCalls, taskCancelCalls, goalClearCalls, + continueCalls, + languageCalls, + addArtifactCalls, + removeArtifactCalls, rewindSnapshotCalls, rewindCalls, shellCalls, @@ -469,6 +496,61 @@ function makeBridge( condition: workspaceCwd, }; }, + async continueSession( + sessionId: string, + context?: BridgeClientRequestContext, + ) { + continueCalls.push({ + sessionId, + ...(context ? { context } : {}), + }); + return { + accepted: true, + interruption: 'interrupted_turn' as const, + promptId: context?.promptId, + lastEventId: 42, + }; + }, + async setSessionLanguage( + sessionId: string, + params: Parameters[1], + context?: BridgeClientRequestContext, + ) { + languageCalls.push({ + sessionId, + params, + ...(context ? { context } : {}), + }); + return { + language: params.language, + outputLanguage: params.syncOutputLanguage ? params.language : null, + refreshed: params.syncOutputLanguage, + }; + }, + async addSessionArtifact( + sessionId: string, + artifact: Parameters[1], + context?: BridgeClientRequestContext, + ) { + addArtifactCalls.push({ + sessionId, + artifact, + ...(context ? { context } : {}), + }); + return { v: 1 as const, sessionId, changes: [] }; + }, + async removeSessionArtifact( + sessionId: string, + artifactId: string, + context?: BridgeClientRequestContext, + ) { + removeArtifactCalls.push({ + sessionId, + artifactId, + ...(context ? { context } : {}), + }); + return { v: 1 as const, sessionId, changes: [] }; + }, async getRewindSnapshots(sessionId: string) { rewindSnapshotCalls.push(sessionId); return { @@ -1665,6 +1747,310 @@ describe('multi-workspace session dispatch', () => { } }); + it('routes continue, language, and artifact mutations to the owning non-primary bridge', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness({ + token: TEST_TOKEN, + }); + const auth = (test: request.Test) => + test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); + + const firstContinue = await auth( + request(app).post('/session/secondary-session/continue'), + ) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({}); + const secondContinue = await auth( + request(app).post('/session/secondary-session/continue'), + ) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({}); + expect(firstContinue.status).toBe(200); + expect(secondContinue.status).toBe(200); + expect(firstContinue.body.promptId).toEqual(expect.any(String)); + expect(secondContinue.body.promptId).toEqual(expect.any(String)); + expect(firstContinue.body.promptId).not.toBe(''); + expect(secondContinue.body.promptId).not.toBe(''); + expect(secondContinue.body.promptId).not.toBe(firstContinue.body.promptId); + + const language = await auth( + request(app).post('/session/secondary-session/language'), + ) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ language: 'zh', syncOutputLanguage: true }); + expect(language.status).toBe(200); + expect(language.body).toEqual({ + language: 'zh', + outputLanguage: 'zh', + refreshed: true, + }); + + const addArtifact = await auth( + request(app).post('/session/secondary-session/artifacts'), + ) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ + title: 'Secondary artifact', + url: 'https://example.com/secondary', + retention: 'ephemeral', + }); + expect(addArtifact.status).toBe(200); + expect(addArtifact.body).toMatchObject({ + v: 1, + sessionId: 'secondary-session', + }); + + const removeArtifact = await auth( + request(app).delete( + '/session/secondary-session/artifacts/artifact-secondary', + ), + ).set('X-Qwen-Client-Id', 'secondary-client'); + expect(removeArtifact.status).toBe(200); + expect(removeArtifact.body).toMatchObject({ + v: 1, + sessionId: 'secondary-session', + }); + + expect(secondaryBridge.continueCalls).toHaveLength(2); + for (const call of secondaryBridge.continueCalls) { + expect(call).toMatchObject({ + sessionId: 'secondary-session', + context: { + clientId: 'secondary-client', + promptId: expect.any(String), + }, + }); + } + expect(secondaryBridge.languageCalls).toEqual([ + { + sessionId: 'secondary-session', + params: { language: 'zh', syncOutputLanguage: true }, + context: { clientId: 'secondary-client' }, + }, + ]); + expect(secondaryBridge.addArtifactCalls).toEqual([ + expect.objectContaining({ + sessionId: 'secondary-session', + artifact: expect.objectContaining({ + title: 'Secondary artifact', + url: 'https://example.com/secondary', + retention: 'ephemeral', + }), + context: { clientId: 'secondary-client' }, + }), + ]); + expect(secondaryBridge.removeArtifactCalls).toEqual([ + { + sessionId: 'secondary-session', + artifactId: 'artifact-secondary', + context: { clientId: 'secondary-client' }, + }, + ]); + expect(primaryBridge.continueCalls).toEqual([]); + expect(primaryBridge.languageCalls).toEqual([]); + expect(primaryBridge.addArtifactCalls).toEqual([]); + expect(primaryBridge.removeArtifactCalls).toEqual([]); + }); + + it('preserves mutation auth while leaving language on its existing non-strict gate', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness(); + + const responses = await Promise.all([ + request(app) + .post('/session/secondary-session/continue') + .set('Host', host()) + .send({}), + request(app) + .post('/session/secondary-session/artifacts') + .set('Host', host()) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ title: 'blocked', url: 'https://example.com/blocked' }), + request(app) + .delete('/session/secondary-session/artifacts/artifact-secondary') + .set('Host', host()) + .set('X-Qwen-Client-Id', 'secondary-client'), + ]); + expect(responses.map((response) => response.status)).toEqual([ + 401, 401, 401, + ]); + + const language = await request(app) + .post('/session/secondary-session/language') + .set('Host', host()) + .send({ language: 'zh' }); + expect(language.status).toBe(200); + expect(secondaryBridge.languageCalls).toEqual([ + { + sessionId: 'secondary-session', + params: { language: 'zh', syncOutputLanguage: false }, + }, + ]); + for (const bridge of [primaryBridge, secondaryBridge]) { + expect(bridge.continueCalls).toEqual([]); + expect(bridge.addArtifactCalls).toEqual([]); + expect(bridge.removeArtifactCalls).toEqual([]); + } + }); + + it('rejects remaining mutations for an untrusted non-primary owner', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness({ + secondaryTrusted: false, + token: TEST_TOKEN, + }); + const auth = (test: request.Test) => + test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); + + const responses = await Promise.all([ + auth(request(app).post('/session/secondary-session/continue')).send({}), + auth(request(app).post('/session/secondary-session/language')).send({ + language: 'zh', + }), + auth(request(app).post('/session/secondary-session/artifacts')) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ title: 'blocked', url: 'https://example.com/blocked' }), + auth( + request(app).delete( + '/session/secondary-session/artifacts/artifact-secondary', + ), + ).set('X-Qwen-Client-Id', 'secondary-client'), + ]); + + expect(responses.map((response) => response.status)).toEqual([ + 403, 403, 403, 403, + ]); + for (const response of responses) { + expect(response.body.code).toBe('untrusted_workspace'); + } + for (const bridge of [primaryBridge, secondaryBridge]) { + expect(bridge.continueCalls).toEqual([]); + expect(bridge.languageCalls).toEqual([]); + expect(bridge.addArtifactCalls).toEqual([]); + expect(bridge.removeArtifactCalls).toEqual([]); + } + }); + + it('fails closed for missing and ambiguous remaining mutation owners', async () => { + const missing = makeHarness({ token: TEST_TOKEN }); + const auth = (test: request.Test) => + test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); + const missingResponses = await Promise.all([ + auth(request(missing.app).post('/session/missing/continue')).send({}), + auth(request(missing.app).post('/session/missing/language')).send({ + language: 'zh', + }), + auth(request(missing.app).post('/session/missing/artifacts')) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ title: 'missing', url: 'https://example.com/missing' }), + auth( + request(missing.app).delete('/session/missing/artifacts/artifact-1'), + ).set('X-Qwen-Client-Id', 'secondary-client'), + ]); + expect(missingResponses.map((response) => response.status)).toEqual([ + 404, 404, 404, 404, + ]); + for (const response of missingResponses) { + expect(response.body.code).toBe('session_not_found'); + } + for (const bridge of [missing.primaryBridge, missing.secondaryBridge]) { + expect(bridge.continueCalls).toEqual([]); + expect(bridge.languageCalls).toEqual([]); + expect(bridge.addArtifactCalls).toEqual([]); + expect(bridge.removeArtifactCalls).toEqual([]); + } + + const duplicate = makeSummary('duplicate-session', PRIMARY_CWD); + const ambiguous = makeHarness({ + token: TEST_TOKEN, + primarySummaries: [duplicate], + secondarySummaries: [makeSummary('duplicate-session', SECONDARY_CWD)], + }); + const ambiguousResponses = await Promise.all([ + auth( + request(ambiguous.app).post('/session/duplicate-session/language'), + ).send({ language: 'zh' }), + auth( + request(ambiguous.app).post('/session/duplicate-session/continue'), + ).send({}), + ]); + expect(ambiguousResponses.map((response) => response.status)).toEqual([ + 500, 500, + ]); + for (const response of ambiguousResponses) { + expect(response.body.code).toBe('ambiguous_session_owner'); + } + expect(ambiguous.primaryBridge.languageCalls).toEqual([]); + expect(ambiguous.secondaryBridge.languageCalls).toEqual([]); + expect(ambiguous.primaryBridge.continueCalls).toEqual([]); + expect(ambiguous.secondaryBridge.continueCalls).toEqual([]); + }); + + it('preserves primary routing for remaining mutations', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness({ + token: TEST_TOKEN, + }); + const auth = (test: request.Test) => + test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); + + const responses = await Promise.all([ + auth(request(app).post('/session/primary-session/continue')) + .set('X-Qwen-Client-Id', 'primary-client') + .send({}), + auth(request(app).post('/session/primary-session/language')) + .set('X-Qwen-Client-Id', 'primary-client') + .send({ language: 'en', syncOutputLanguage: true }), + auth(request(app).post('/session/primary-session/artifacts')) + .set('X-Qwen-Client-Id', 'primary-client') + .send({ + title: 'Primary artifact', + url: 'https://example.com/primary', + }), + auth( + request(app).delete( + '/session/primary-session/artifacts/artifact-primary', + ), + ).set('X-Qwen-Client-Id', 'primary-client'), + ]); + expect(responses.map((response) => response.status)).toEqual([ + 200, 200, 200, 200, + ]); + expect(primaryBridge.continueCalls).toEqual([ + { + sessionId: 'primary-session', + context: { + clientId: 'primary-client', + promptId: expect.any(String), + }, + }, + ]); + expect(primaryBridge.languageCalls).toEqual([ + { + sessionId: 'primary-session', + params: { language: 'en', syncOutputLanguage: true }, + context: { clientId: 'primary-client' }, + }, + ]); + expect(primaryBridge.addArtifactCalls).toEqual([ + expect.objectContaining({ + sessionId: 'primary-session', + artifact: expect.objectContaining({ + title: 'Primary artifact', + url: 'https://example.com/primary', + }), + context: { clientId: 'primary-client' }, + }), + ]); + expect(primaryBridge.removeArtifactCalls).toEqual([ + { + sessionId: 'primary-session', + artifactId: 'artifact-primary', + context: { clientId: 'primary-client' }, + }, + ]); + expect(secondaryBridge.continueCalls).toEqual([]); + expect(secondaryBridge.languageCalls).toEqual([]); + expect(secondaryBridge.addArtifactCalls).toEqual([]); + expect(secondaryBridge.removeArtifactCalls).toEqual([]); + }); + it('preserves primary routing for owner-local actions', async () => { const { app, primaryBridge, secondaryBridge } = makeHarness({ token: TEST_TOKEN, diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 83b4721f977..4e6bf7edd6a 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -1613,9 +1613,9 @@ export function registerSessionRoutes( app.post( '/session/:id/artifacts', mutate({ strict: true }), - withMutableSession( + withOwnerMutableSession( 'POST /session/:id/artifacts', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { const clientId = parseClientIdHeader(req, res); if (clientId === null) return; if (!requireSessionArtifactClientId(clientId, res)) return; @@ -1641,9 +1641,11 @@ export function registerSessionRoutes( 'clientRetained' ] as SessionArtifactInput['clientRetained'], }; - const result = await bridge.addSessionArtifact(sessionId, artifact, { - clientId, - }); + const result = await runtime.bridge.addSessionArtifact( + sessionId, + artifact, + { clientId }, + ); res.status(200).json(result); } catch (err) { if (sendArtifactValidationError(res, err)) return; @@ -1659,9 +1661,9 @@ export function registerSessionRoutes( app.delete( '/session/:id/artifacts/:artifactId', mutate({ strict: true }), - withMutableSession( + withOwnerMutableSession( 'DELETE /session/:id/artifacts/:artifactId', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { const artifactId = req.params['artifactId']; const clientId = parseClientIdHeader(req, res); if (clientId === null) return; @@ -1678,7 +1680,7 @@ export function registerSessionRoutes( return; } try { - const result = await bridge.removeSessionArtifact( + const result = await runtime.bridge.removeSessionArtifact( sessionId, artifactId, { clientId }, @@ -1739,9 +1741,9 @@ export function registerSessionRoutes( app.post( '/session/:id/continue', mutate({ strict: true }), - withMutableSession( + withOwnerMutableSession( 'POST /session/:id/continue', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { // Forward the originator and a generated promptId so the bridge can // attribute and correlate the continuation turn (it now runs through the // prompt-admission path, same as POST /session/:id/prompt). The accepted @@ -1750,7 +1752,7 @@ export function registerSessionRoutes( if (clientId === null) return; const promptId = crypto.randomUUID(); res.status(200).json( - await bridge.continueSession(sessionId, { + await runtime.bridge.continueSession(sessionId, { ...(clientId !== undefined ? { clientId } : {}), promptId, }), @@ -3068,9 +3070,9 @@ export function registerSessionRoutes( app.post( '/session/:id/language', mutate(), - withMutableSession( + withOwnerMutableSession( 'POST /session/:id/language', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { const body = safeBody(req); const language = body['language']; const syncOutputLanguage = body['syncOutputLanguage']; @@ -3103,7 +3105,7 @@ export function registerSessionRoutes( const clientId = parseClientIdHeader(req, res); if (clientId === null) return; - const response = await bridge.setSessionLanguage( + const response = await runtime.bridge.setSessionLanguage( sessionId, { language,