diff --git a/docs/design/2026-08-20-webshell-session-pr-binding.md b/docs/design/2026-08-20-webshell-session-pr-binding.md new file mode 100644 index 00000000000..af7578aa4b1 --- /dev/null +++ b/docs/design/2026-08-20-webshell-session-pr-binding.md @@ -0,0 +1,86 @@ +# Web Shell 会话绑定 GitHub PR 号 + +日期：2026-08-20 +状态：已确认 MVP 范围 + +## 问题 + +Web Shell 同时运行 20+ 会话时，侧栏信息不足以回答"哪个会话对应 PR #N"。 +当前链路全断： + +1. `GitDialog.doCreatePr` 创建 PR 拿到 `{url, number}` 后只显示状态消息，不回写（`packages/web-shell/client/components/dialogs/GitDialog.tsx:502-553`）。 +2. `DaemonSessionSummary` / `BridgeSessionSummary` 无 PR 字段；`updateSessionMetadata` 只放行 `displayName`。 +3. 侧栏搜索只匹配标题和 sessionId（`WebShellSidebar.tsx:3289-3302`），不匹配分支名、worktree slug、PR 号。 +4. 无任何持久化载体，daemon 重启后即使内存绑定也会丢。 + +## 方案 + +### 数据模型 + +`DaemonSessionSummary` 与 `BridgeSessionSummary`（镜像，需同步）增加： + +```json +"prs": [{ "number": 9517, "url": "https://github.com/owner/repo/pull/9517" }] +``` + +- 一个会话可能创建多个 PR（stacked PR、连续修复），`prs` 按绑定时间排序（最后一个 = 最新），上限 10 个（超出丢弃最旧）。同号重复绑定刷新 url 并移到最新位。 +- `number`：正整数；`url`：http(s) URL（badge/tooltip 直接作为链接目标渲染，拒绝 `javascript:` 等 scheme——route、bridge、SDK 校验器、sidecar 校验四层统一要求）。 +- 字段可选、可缺省；不提供"清除"语义。 +- 写入 API 保持单条：`updateSessionMetadata(sessionId, { pr: {number, url} })` 每次绑定一个，daemon 负责 upsert 进列表；读取/事件/响应均为完整 `prs` 数组。 + +### 写入端 + +- SDK `DaemonClient.updateSessionMetadata` 的 metadata 参数扩展 `pr?: { number: number; url: string }`（单条）；响应解析完整 `prs` 数组。 +- daemon 两个 PATCH metadata 路由（`/session/:id/metadata` 与 workspace 作用域版本）校验 `pr` 后透传，bridge 更新成功后将 sidecar upsert 的完整列表回显在响应里。 +- bridge `updateSessionMetadata`（`packages/acp-bridge/src/bridge.ts`）先做全部校验再变更（组合请求不允许部分生效）；upsert 进 live entry.prs（去重按 number，上限 10），`session_metadata_updated` SSE 事件 data 带完整 `prs`。 +- ACP `session/update_metadata`（`acp-http/dispatch.ts`）同样把最新绑定 upsert 进 sidecar。 +- `GitDialog.doCreatePr` 成功后：仅用 dialog 已有的 `sessionId`（`sessionIdRef.current`，即连接会话或 dialog 已为提交信息生成等操作解析出的会话）调用 `updateSessionMetadata(sessionId, { pr })`。**不调 `resolveSessionForWorkspace`**——它可能创建幽灵会话或误绑"最近会话"。写入失败仅降级为 console 警告，不影响 PR 创建成功的状态展示。 + +### 持久化 + +新增 sidecar `/.pr.json`，复刻 worktree sidecar 模式： + +- 新 core 服务 `packages/core/src/services/session-pr-service.ts`：`SessionPr` 接口、数组 schema 校验（`{prs: [...]}`，容忍 ENOENT/JSON 损坏）、`readSessionPrs` / `writeSessionPrs` / `upsertSessionPr`（按 number 去重、移到最新、cap 10）。 +- `SessionService` 增加 `getPrSessionPathForArchiveState` 路径助手；归档/取消归档移动 sidecar、删除会话时清理（与 worktree sidecar 一一对应）。 +- `session-list.ts` 的 `enrichPrSidecars` 回填 persisted summary 的 `prs`；live 会话的 entry.prs 只含本 daemon 生命周期内的绑定，回填时与 sidecar 历史按 number 合并（live 的 url 优先，live-only 的排最后）。 + +### 展示与搜索（web-shell 侧栏） + +- `renderSessionRow`：会话行标题旁渲染小号 badge（`session.prs` 非空时），显示最新 PR 号，多于一个时追加 `+N`；点击经 `useExternalLinkOpener` 打开最新 PR（desktop webview 下 `target="_blank"` 会被静默丢弃）；click/doubleClick/keydown 均 stopPropagation（双击 badge 不触发重命名）。 +- `SessionDetailsTooltip`：列出全部绑定 PR（最新在前），各为外链。 +- `filteredSessions` 匹配逻辑扩展：`label`、`sessionId` 之外，增加**任意一个**绑定 PR 号（输入 `9517` 或 `#9517` 都命中）、`branch.name`、`worktree.branch`、`worktree.slug`（`sessionMatchesGitQuery`，WebShellSidebar 与 WorkspaceSection 共用）。 +- SSE 消费侧：web-shell 不直接消费 `session_metadata_updated` 更新 store；bridge 的 `markSessionCatalogChanged()` 触发 catalog revision bump，侧栏 live-state 轮询（2s 周期）发现后自动 refetch——badge 在绑定后 ~2s 内出现（与改名等其他客户端变更的传播机制一致）。 +- i18n：新增 `sidebar.sessionPr` / `sidebar.sessionPrMultiple` 两个 key（EN/ZH）。 + +## 关键决策 + +- **绑定时机 = GitDialog 创建 PR 成功时**。Agent 在 shell 里自行 `gh pr create` 的路径无法拦截，MVP 不覆盖；用户主力流程是 GitDialog。 +- **sidecar 而非 transcript 记录**：displayName 走 `custom_title` transcript 记录是因为标题属于会话内容流；PR 绑定是会话外部元数据，worktree sidecar 是同类先例，改动面更小。 +- **多 PR 列表（cap 10）**：一个会话可能创建多个 PR（stacked PR、连续修复），只保留最新一个会让"按 PR 号反查会话"在这些场景失效。绑定按 number 去重、重复绑定移到最新位；badge 显示最新号 + `+N`，tooltip 列全部，搜索匹配任意一个。上限 10 防无界增长。 +- **workspace 级打开 GitDialog（无会话上下文）时不回写**：dialog 没有已解析的会话就跳过，不报错；绝不通过 `resolveSessionForWorkspace` 创建新会话来绑定（会产生幽灵会话/误绑）。 + +## 影响文件 + +| 层 | 文件 | +| ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| SDK 类型 | `packages/sdk-typescript/src/daemon/types.ts`（DaemonSessionSummary.pr） | +| SDK 事件 | `packages/sdk-typescript/src/daemon/events.ts`（MetadataUpdated data + 校验） | +| SDK 客户端 | `packages/sdk-typescript/src/daemon/DaemonClient.ts`（updateSessionMetadata 参数） | +| bridge 类型 | `packages/acp-bridge/src/bridgeTypes.ts`（BridgeSessionSummary.pr、metadata 参数） | +| bridge | `packages/acp-bridge/src/bridge.ts`（updateSessionMetadata 校验/存储/广播） | +| core | `packages/core/src/services/session-pr-service.ts`（新增）+ SessionService 路径助手/归档移动/删除清理 | +| daemon 路由 | `packages/cli/src/serve/routes/session.ts`（两个 PATCH 路由校验 + sidecar 写入）、`acp-http/dispatch.ts`（ACP `session/update_metadata` 的 sidecar 写入） | +| daemon 列表 | `packages/cli/src/serve/server/session-list.ts`（enrichPrSidecars） | +| web-shell | `GitDialog.tsx`（回写）、`WebShellSidebar.tsx`（badge + 搜索）、`SessionDetailsTooltip.tsx`（PR 行）、locale 文件 | +| 测试 | 上述各层的 collocated 单测 | + +## 范围边界（明确不做） + +- 服务端分页过滤（20+ 会话规模客户端搜索足够；`sourceType/sourceId` 过滤管道是将来扩展的样板）。 +- 历史会话迁移；CLI `--worktree=#` 的 `pr-` slug 由搜索匹配 slug 顺带覆盖。 +- 纯 branch 会话 branch 信息重启丢失的独立 bug，另行处理。 +- Agent shell 内 `gh pr create` 的自动发现。 + +## 开放问题 + +无。 diff --git a/package-lock.json b/package-lock.json index 6aa7138c573..2e38b43b40a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -17578,6 +17578,7 @@ "os": [ "android" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17599,6 +17600,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17620,6 +17622,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17641,6 +17644,7 @@ "os": [ "freebsd" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17662,6 +17666,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17683,6 +17688,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17704,6 +17710,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17725,6 +17732,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17746,6 +17754,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17767,6 +17776,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, @@ -17788,6 +17798,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">= 12.0.0" }, diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 458d7560ac9..f3d3867206a 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -26396,6 +26396,357 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('stores a pr binding, exposes it in summaries, and publishes an event', async () => { + const handles: Array<{ killed: boolean }> = []; + const factory: ChannelFactory = async () => { + const h = makeChannel(); + handles.push(h); + return h.channel; + }; + const bridge = makeBridge({ channelFactory: factory }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const events: BridgeEvent[] = []; + const sub = bridge.subscribeEvents(session.sessionId); + const drain = (async () => { + for await (const ev of sub) events.push(ev); + })(); + await new Promise((r) => setImmediate(r)); + + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const effective = bridge.updateSessionMetadata(session.sessionId, { + pr, + }); + + expect(effective.prs).toEqual([pr]); + expect(bridge.getSessionSummary(session.sessionId).prs).toEqual([pr]); + await new Promise((r) => setImmediate(r)); + const metaEvent = events.find( + (e) => + e.type === 'session_metadata_updated' && + (e.data as { prs?: unknown }).prs !== undefined, + ); + expect(metaEvent).toBeDefined(); + expect((metaEvent?.data as { prs: Array }).prs).toEqual([pr]); + + await bridge.closeSession(session.sessionId); + await drain; + await bridge.shutdown(); + }); + + it('publishes the full seeded binding history in the reply and event after an entry re-creation', async () => { + // Daemon restart / close / archive-restore re-creates the entry with + // an empty in-memory pr list; the serve layer re-hydrates it from the + // persisted sidecar before binding, and both the reply and the + // `session_metadata_updated` event must carry the full history, not + // just the fresh binding. + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const events: BridgeEvent[] = []; + const sub = bridge.subscribeEvents(session.sessionId); + const drain = (async () => { + for await (const ev of sub) events.push(ev); + })(); + await new Promise((r) => setImmediate(r)); + + const persisted = { + number: 9500, + url: 'https://github.com/o/r/pull/9500', + }; + bridge.seedSessionPrs?.(session.sessionId, [persisted]); + + const fresh = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const effective = bridge.updateSessionMetadata(session.sessionId, { + pr: fresh, + }); + + expect(effective.prs).toEqual([persisted, fresh]); + expect(bridge.getSessionSummary(session.sessionId).prs).toEqual([ + persisted, + fresh, + ]); + await new Promise((r) => setImmediate(r)); + const metaEvent = events.find( + (e) => + e.type === 'session_metadata_updated' && + (e.data as { prs?: unknown }).prs !== undefined, + ); + expect(metaEvent).toBeDefined(); + expect((metaEvent?.data as { prs: Array }).prs).toEqual([ + persisted, + fresh, + ]); + + await bridge.closeSession(session.sessionId); + await drain; + await bridge.shutdown(); + }); + + it('keeps this-daemon-lifetime bindings over a late seed', async () => { + // Seeding is recovery for re-created entries only; once the entry + // holds bindings from this daemon lifetime they are authoritative. + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const bound = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + bridge.updateSessionMetadata(session.sessionId, { pr: bound }); + bridge.seedSessionPrs?.(session.sessionId, [ + { number: 1, url: 'https://github.com/o/r/pull/1' }, + ]); + + expect(bridge.getSessionSummary(session.sessionId).prs).toEqual([bound]); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it('logs a stderr audit record when a pr binding is added', async () => { + const stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + try { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + bridge.updateSessionMetadata(session.sessionId, { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining('updated session metadata'), + ); + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining(session.sessionId), + ); + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining('pr=9517'), + ); + + await bridge.shutdown(); + } finally { + stderrSpy.mockRestore(); + } + }); + + it('accumulates multiple bindings and re-binding moves a number to latest', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const prA = { number: 9500, url: 'https://github.com/o/r/pull/9500' }; + const prB = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + bridge.updateSessionMetadata(session.sessionId, { pr: prA }); + const effective = bridge.updateSessionMetadata(session.sessionId, { + pr: prB, + }); + expect(effective.prs).toEqual([prA, prB]); + + const prA2 = { + number: 9500, + url: 'https://github.com/o/r/pull/9500?v=2', + }; + const rebound = bridge.updateSessionMetadata(session.sessionId, { + pr: prA2, + }); + expect(rebound.prs).toEqual([prB, prA2]); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it('does not republish when the same pr is bound again', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const events: BridgeEvent[] = []; + const sub = bridge.subscribeEvents(session.sessionId); + const drain = (async () => { + for await (const ev of sub) events.push(ev); + })(); + await new Promise((r) => setImmediate(r)); + + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + bridge.updateSessionMetadata(session.sessionId, { pr }); + bridge.updateSessionMetadata(session.sessionId, { pr }); + + await new Promise((r) => setImmediate(r)); + const prEvents = events.filter( + (e) => + e.type === 'session_metadata_updated' && + (e.data as { prs?: unknown }).prs !== undefined, + ); + expect(prEvents).toHaveLength(1); + + await bridge.closeSession(session.sessionId); + await drain; + await bridge.shutdown(); + }); + + it('bumps the catalog revision when a pr is bound', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const before = bridge.getSessionCatalogVersion().revision; + + bridge.updateSessionMetadata(session.sessionId, { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + + expect(bridge.getSessionCatalogVersion().revision).toBe(before + 1); + // Repeating the same binding must not bump again. + bridge.updateSessionMetadata(session.sessionId, { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(bridge.getSessionCatalogVersion().revision).toBe(before + 1); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it('echoes the current displayName on the pr metadata event', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + bridge.updateSessionMetadata(session.sessionId, { + displayName: 'My Session', + }); + + const events: BridgeEvent[] = []; + const sub = bridge.subscribeEvents(session.sessionId); + const drain = (async () => { + for await (const ev of sub) events.push(ev); + })(); + await new Promise((r) => setImmediate(r)); + + bridge.updateSessionMetadata(session.sessionId, { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + + await new Promise((r) => setImmediate(r)); + const prEvent = events.find( + (e) => + e.type === 'session_metadata_updated' && + (e.data as { prs?: unknown }).prs !== undefined, + ); + // SDK folds treat an absent displayName as "cleared", so the pr event + // must echo the current name instead of blanking the title. + expect((prEvent?.data as { displayName?: string }).displayName).toBe( + 'My Session', + ); + + await bridge.closeSession(session.sessionId); + await drain; + await bridge.shutdown(); + }); + + it('caps the binding list at MAX_SESSION_PRS, dropping the oldest', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + for (let i = 1; i <= 12; i++) { + bridge.updateSessionMetadata(session.sessionId, { + pr: { number: i, url: `https://github.com/o/r/pull/${i}` }, + }); + } + + const prs = bridge.getSessionSummary(session.sessionId).prs; + expect(prs).toHaveLength(10); + expect(prs?.[0]?.number).toBe(3); + expect(prs?.[9]?.number).toBe(12); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it('does not apply displayName when the combined pr is invalid', async () => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + expect(() => + bridge.updateSessionMetadata(session.sessionId, { + displayName: 'should-not-apply', + pr: { number: -1, url: 'https://github.com/o/r/pull/1' }, + }), + ).toThrow(InvalidSessionMetadataError); + expect( + bridge.getSessionSummary(session.sessionId).displayName, + ).toBeUndefined(); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it('does not apply the pr binding when the combined displayName is invalid', async () => { + // Mirror of the invalid-pr case: the validate-everything-first rule + // must protect both directions — a reorder regression would persist, + // publish, and catalog-bump a binding for a rejected request. + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + expect(() => + bridge.updateSessionMetadata(session.sessionId, { + displayName: 'bad\nname', + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }), + ).toThrow(InvalidSessionMetadataError); + expect(bridge.getSessionSummary(session.sessionId).prs).toBeUndefined(); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + + it.each([ + [ + 'non-integer number', + { number: 1.5, url: 'https://github.com/o/r/pull/1' }, + ], + ['missing url', { number: 1 }], + ['empty url', { number: 1, url: '' }], + ['non-http url', { number: 1, url: 'javascript:alert(1)' }], + [ + 'url with a control character', + // \n in the url would forge a second line in the stderr audit log. + { number: 1, url: 'https://github.com/o/r/pull/1\nforged' }, + ], + [ + 'url over 2048 characters', + { number: 1, url: `https://github.com/${'a'.repeat(2048)}` }, + ], + ['null', null], + ])('rejects an invalid pr: %s', async (_label, pr) => { + const bridge = makeBridge({ + channelFactory: async () => makeChannel().channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + expect(() => + bridge.updateSessionMetadata(session.sessionId, { + pr: pr as { number: number; url: string }, + }), + ).toThrow(InvalidSessionMetadataError); + + await bridge.closeSession(session.sessionId); + await bridge.shutdown(); + }); + it('throws SessionNotFoundError for unknown session', () => { const bridge = makeBridge(); expect(() => diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 70a75ce7ee8..9e7df54df74 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -36,6 +36,8 @@ import { PRIVATE_ACP_CAPABILITY_ENV, PRIVATE_PARENT_CAPABILITY_META_KEY, SESSION_ARTIFACT_PERSISTENCE_VERSION, + SESSION_PR_LIST_LIMIT, + SESSION_PR_URL_MAX_LENGTH, SESSION_TRANSCRIPT_MAX_LIMIT, TURN_RESULT_CODE_TEXT_TRUNCATED, TURN_RESULT_TEXT_MAX_CHARS, @@ -166,6 +168,7 @@ import type { BridgeRestoredSession, BridgeSessionGoal, BridgeSessionSummary, + SessionPrInfo, BridgeTurnStatus, BridgeSessionCatalogVersion, BridgePendingInteraction, @@ -962,6 +965,8 @@ interface SessionEntry { worktree?: { slug: string; path: string; branch: string }; /** Branch metadata, when created with branch param. */ branch?: { name: string; baseBranch: string }; + /** GitHub PRs bound via updateSessionMetadata, in binding order. */ + prs?: SessionPrInfo[]; channel: AcpChannel; connection: ClientSideConnection; /** Per-session event bus drives `GET /session/:id/events`. */ @@ -3635,6 +3640,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { pendingInteractions: [...entry.pendingInteractions.values()], ...(entry.worktree ? { worktree: entry.worktree } : {}), ...(entry.branch ? { branch: entry.branch } : {}), + ...(entry.prs && entry.prs.length > 0 ? { prs: entry.prs } : {}), }; }; // Pending + resolved permission state lives in @@ -9655,6 +9661,31 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { context?.clientId !== undefined ? resolveTrustedClientId(entry, context.clientId) : undefined; + // Validate everything before mutating anything: a combined + // displayName+pr request must not partially apply when the pr is + // invalid. + if (metadata.pr !== undefined) { + const pr = metadata.pr as unknown; + if ( + pr === null || + typeof pr !== 'object' || + typeof (pr as SessionPrInfo).number !== 'number' || + !Number.isInteger((pr as SessionPrInfo).number) || + (pr as SessionPrInfo).number <= 0 || + typeof (pr as SessionPrInfo).url !== 'string' || + (pr as SessionPrInfo).url.length > SESSION_PR_URL_MAX_LENGTH || + !/^https?:\/\//i.test((pr as SessionPrInfo).url) || + // The url is interpolated into the stderr audit line — control + // characters would let a client forge log lines (the displayName + // branch rejects them for the same reason). + hasControlCharacter((pr as SessionPrInfo).url) + ) { + throw new InvalidSessionMetadataError( + 'pr', + `must be an object with a positive integer \`number\` and an http(s) \`url\` of at most ${SESSION_PR_URL_MAX_LENGTH} characters, without control characters`, + ); + } + } if (metadata.displayName !== undefined) { if ( typeof metadata.displayName !== 'string' || @@ -9721,7 +9752,60 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { } } } - return { displayName: entry.displayName }; + if (metadata.pr !== undefined) { + // Already validated above, before any mutation. + const bound = metadata.pr; + const existing = entry.prs ?? []; + const latest = existing[existing.length - 1]; + if (latest?.number === bound.number && latest.url === bound.url) { + // Same binding repeated — no change, no event. + } else { + // Re-binding a number refreshes it and moves it to latest. + entry.prs = [ + ...existing.filter((p) => p.number !== bound.number), + { number: bound.number, url: bound.url }, + ].slice(-SESSION_PR_LIST_LIMIT); + markSessionCatalogChanged(); + writeStderrLine( + `qwen serve: updated session metadata ${JSON.stringify(sessionId)} ` + + `pr=${bound.number} bound (${bound.url})` + + (context?.clientId + ? ` by client ${JSON.stringify(context.clientId)}` + : ''), + ); + try { + entry.events.publish({ + type: 'session_metadata_updated', + // Echo the current name: SDK folds treat an absent displayName + // as "cleared", so a pr-only event must not blank the title. + data: { + sessionId, + ...(entry.displayName !== undefined + ? { displayName: entry.displayName } + : {}), + prs: entry.prs, + }, + ...(metadataOriginatorClientId + ? { originatorClientId: metadataOriginatorClientId } + : {}), + }); + } catch { + /* bus already closed */ + } + } + } + return { + displayName: entry.displayName, + ...(entry.prs && entry.prs.length > 0 ? { prs: entry.prs } : {}), + }; + }, + + seedSessionPrs(sessionId, prs) { + const entry = byId.get(sessionId); + if (!entry || (entry.prs && entry.prs.length > 0)) return; + entry.prs = prs + .map(({ number, url }) => ({ number, url })) + .slice(-SESSION_PR_LIST_LIMIT); }, async getSessionArtifacts(sessionId, context) { diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 1e2d087a056..2b45b024638 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -687,6 +687,11 @@ export interface BridgeSessionSummary { worktree?: { slug: string; path: string; branch: string }; /** Present when the session was created with a new branch. */ branch?: { name: string; baseBranch: string }; + /** + * GitHub PRs bound to the session, in binding order (last = latest). A + * session can produce several PRs (stacked or follow-up work). + */ + prs?: SessionPrInfo[]; } /** @@ -717,8 +722,16 @@ export interface BridgeSessionGoal { } | null; } +export interface SessionPrInfo { + number: number; + url: string; +} + export interface SessionMetadataUpdate { displayName?: string; + pr?: SessionPrInfo; + /** Full binding list after the update (return value only; ignored on input). */ + prs?: SessionPrInfo[]; } export interface CloseSessionOpts { @@ -1415,7 +1428,7 @@ export interface AcpSessionBridge { ): Promise; /** - * Update mutable session metadata. Currently supports `displayName` only. + * Update mutable session metadata. Supports `displayName` and `pr`. * Throws `SessionNotFoundError` for unknown ids. */ updateSessionMetadata( @@ -1424,6 +1437,16 @@ export interface AcpSessionBridge { context?: BridgeClientRequestContext, ): SessionMetadataUpdate; + /** + * Re-hydrate the in-memory PR binding list of a live session from the + * persisted sidecar after the entry was re-created empty (daemon + * restart, close/reload, archive/restore). No-op when the entry is + * unknown or already holds bindings, so this-daemon-lifetime state + * always wins. Callers own sidecar I/O; the bridge stays + * storage-agnostic. Optional so lightweight fakes may omit it. + */ + seedSessionPrs?(sessionId: string, prs: SessionPrInfo[]): void; + /** * List the structured artifacts registered for a live session. Throws * `SessionNotFoundError` when the id is unknown. diff --git a/packages/cli/src/serve/acp-http/dispatch-error.test.ts b/packages/cli/src/serve/acp-http/dispatch-error.test.ts index adc43c5d109..222ebb88533 100644 --- a/packages/cli/src/serve/acp-http/dispatch-error.test.ts +++ b/packages/cli/src/serve/acp-http/dispatch-error.test.ts @@ -9,6 +9,7 @@ import { SessionIdCaseConflictError } from '@qwen-code/qwen-code-core'; import { DaemonDrainingError } from '../server/session-archive.js'; import { BridgeChannelQuarantinedError, + InvalidSessionMetadataError, RestoreInProgressError, SessionRestoreTimeoutError, } from '../acp-session-bridge.js'; @@ -105,6 +106,22 @@ describe('toRpcError', () => { }); }); + it('maps invalid session metadata to the REST-equivalent invalid_metadata contract', () => { + // Without an arm, every invalid `pr`/`displayName` over ACP degrades to + // an opaque -32603 Internal error and clients cannot tell their own bad + // input from a daemon fault. REST maps the same error to 400 + // `invalid_metadata` with the offending `field`. + const error = new InvalidSessionMetadataError( + 'pr', + 'must be an object with a positive integer `number`', + ); + expect(toRpcError(error)).toEqual({ + code: RPC.INVALID_PARAMS, + message: error.message, + data: { httpStatus: 400, errorKind: 'invalid_metadata', field: 'pr' }, + }); + }); + it('maps persisted case conflicts to the session_conflict contract', () => { const error = new SessionIdCaseConflictError( '550e8400-e29b-41d4-a716-446655440149', diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index 461cacfabd7..717a634f923 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -22,6 +22,8 @@ import { WorkspaceMemoryFileTooLargeError, WorkspaceMemoryWriteTimeoutError, writeWorkspaceContextFile, + readSessionPrs, + upsertSessionPr, type SessionArchiveState, type SubagentLevel, IMAGE_CAPABILITY, @@ -90,6 +92,7 @@ import { } from '../../config/permission-settings.js'; import { loadSettings } from '../../config/settings.js'; import { + isValidSessionId, normalizeSessionIdForLookup, parseCallerSuppliedSessionId, } from '../../config/session-id.js'; @@ -872,6 +875,16 @@ export function toRpcError(err: unknown): { sessionId: (err as { sessionId?: unknown }).sessionId, }, }; + case 'InvalidSessionMetadataError': + return { + code: RPC.INVALID_PARAMS, + message: errMsg(err), + data: { + httpStatus: 400, + errorKind: 'invalid_metadata', + field: (err as { field?: unknown }).field, + }, + }; case 'SessionNotFoundError': case 'InvalidSessionScopeError': case 'WorkspaceMismatchError': @@ -2895,10 +2908,39 @@ export class AcpDispatcher { case `${QWEN_METHOD_NS}session/update_metadata`: { const sessionId = String(params['sessionId'] ?? ''); + // Same gate as the REST metadata routes: the id becomes a sidecar + // path component (upsertSessionPr's mkdir + JSON write), so reject + // separator-bearing spellings before any sidecar I/O. + if (!isValidSessionId(sessionId)) { + throw new AcpParamError('`sessionId` must be a valid session id'); + } await this.withMutableOwned(conn, sessionId, id, async () => { const metadata = isObject(params['metadata']) ? (params['metadata'] as Record) : {}; + const service = new SessionService(this.boundWorkspace, { + runtimeBaseDir: this.sessionRuntimeBaseDir, + }); + // Bridge entries are re-created without prs on daemon restart, + // close/reload, and archive/restore, and the bridge itself is + // storage-agnostic — so hydrate the persisted binding history + // before the mutation. Otherwise the reply AND the + // `session_metadata_updated` event echo only this daemon + // lifetime's bindings, silently dropping earlier ones. The read + // is best-effort: readSessionPrs rethrows non-ENOENT I/O errors + // (EISDIR/EACCES/EIO), and an unreadable sidecar must degrade the + // event's history, not block a pr-less rename. + let hydratedPrs: Awaited>; + try { + hydratedPrs = await readSessionPrs( + service.getPrSessionPathForArchiveState(sessionId, 'active'), + ); + } catch { + hydratedPrs = null; + } + if (hydratedPrs && hydratedPrs.length > 0) { + this.bridge.seedSessionPrs?.(sessionId, hydratedPrs); + } let result: ReturnType; try { result = this.bridge.updateSessionMetadata( @@ -2908,6 +2950,34 @@ export class AcpDispatcher { >[1], this.sessionCtx(conn, sessionId, loopback), ); + // The bridge keeps the binding in live memory only; persist it + // as a sidecar so it survives daemon restarts, matching the + // REST metadata routes. Gate on this call actually binding a + // PR — the bridge echoes `prs` whenever any binding exists, so + // a displayName-only rename must not re-upsert (it would + // refresh createdAt and could evict an older entry early). + const boundPr = metadata['pr']; + if ( + isObject(boundPr) && + typeof boundPr['number'] === 'number' && + typeof boundPr['url'] === 'string' + ) { + const persistedPrs = ( + await upsertSessionPr( + service.getPrSessionPathForArchiveState( + sessionId, + 'active', + ), + { + number: boundPr['number'], + url: boundPr['url'], + }, + ) + ).map(({ number, url }) => ({ number, url })); + // Reply with the authoritative persisted list, mirroring the + // REST metadata routes. + result = { ...result, prs: persistedPrs }; + } } finally { this.invalidateSessionLists(['active']); } diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index 9b0a68af3c9..fc85bf520a1 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -42,6 +42,8 @@ import { SessionIdCaseConflictError, SessionService, Storage, + readSessionPrs, + upsertSessionPr, } from '@qwen-code/qwen-code-core'; import { resetHomeEnvBootstrapForTesting, @@ -418,8 +420,59 @@ class FakeBridge { async getSessionSupportedCommandsStatus(sessionId: string) { return { v: 1, sessionId, availableCommands: [], availableSkills: [] }; } - updateSessionMetadata(_s: string, metadata: unknown) { - return metadata; + /** Per-session in-memory pr bindings, mirroring the real bridge's + * `entry.prs`: a re-created entry (restart/close/archive-restore) starts + * empty and is only re-hydrated when the serve layer seeds it. */ + metadataPrsBySession = new Map< + string, + Array<{ number: number; url: string }> + >(); + seedSessionPrsCalls: Array<{ + sessionId: string; + prs: Array<{ number: number; url: string }>; + }> = []; + /** Shared seed/update call sequence — pins that hydration runs BEFORE the + * mutation (a seed-after-mutation order would let the bridge publish an + * event carrying only this-daemon-lifetime bindings). */ + metadataCallLog: Array<'seed' | 'update'> = []; + + seedSessionPrs( + sessionId: string, + prs: Array<{ number: number; url: string }>, + ) { + this.metadataCallLog.push('seed'); + this.seedSessionPrsCalls.push({ sessionId, prs }); + const existing = this.metadataPrsBySession.get(sessionId) ?? []; + if (existing.length > 0) return; + this.metadataPrsBySession.set( + sessionId, + prs.map(({ number, url }) => ({ number, url })), + ); + } + + updateSessionMetadata( + sessionId: string, + metadata: { displayName?: string; pr?: { number: number; url: string } }, + ) { + this.metadataCallLog.push('update'); + if (metadata.pr) { + const bound = metadata.pr; + const existing = this.metadataPrsBySession.get(sessionId) ?? []; + const latest = existing[existing.length - 1]; + if (!(latest?.number === bound.number && latest.url === bound.url)) { + this.metadataPrsBySession.set(sessionId, [ + ...existing.filter((entry) => entry.number !== bound.number), + { number: bound.number, url: bound.url }, + ]); + } + } + const prs = this.metadataPrsBySession.get(sessionId) ?? []; + return { + ...(metadata.displayName !== undefined + ? { displayName: metadata.displayName } + : {}), + ...(prs.length > 0 ? { prs } : {}), + }; } recordHeartbeat() { @@ -5314,6 +5367,232 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { } }); + it('replies the full persisted binding history when binding a pr over ACP after an entry re-creation', async () => { + // Daemon restart / close / archive-restore re-creates the bridge entry + // with an empty in-memory pr list. Binding a new PR over ACP must then + // reply (and broadcast) the FULL persisted history from the sidecar — + // the `SessionMetadataUpdate.prs` contract is "full binding list after + // the update", not just this daemon lifetime's bindings. + const sessionId = '550e8400-e29b-41d4-a716-446655440137'; + const runtimeBaseDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'qwen-acp-pr-metadata-'), + ); + const archiveCoordinator = new SessionArchiveCoordinator(); + const registry = new ConnectionRegistry(); + const rememberLane = new WorkspaceRememberTaskLane( + bridge as unknown as HttpAcpBridge, + ); + const dispatcher = new AcpDispatcher( + bridge as unknown as HttpAcpBridge, + TEST_WORKSPACE, + () => process.env, + fakeWorkspace, + rememberLane, + createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => [ + { workspaceCwd: TEST_WORKSPACE, runtimeBaseDir }, + ], + }), + undefined, + undefined, + false, + registry, + archiveCoordinator, + () => true, + () => undefined, + undefined, + runtimeBaseDir, + ); + const conn = registry.create(true)!; + conn.ownSession(sessionId); + conn.getOrCreateSession(sessionId).clientId = 'client-pr-metadata'; + const frames: unknown[] = []; + conn.attachConnStream({ + kind: 'sse', + isClosed: false, + async send(message: unknown): Promise { + frames.push(message); + }, + async sendSerialized(payload: Buffer) { + frames.push(JSON.parse(payload.toString('utf8'))); + return 'delivered' as const; + }, + close(): void {}, + } satisfies TransportStream); + + try { + const service = new SessionService(TEST_WORKSPACE, { runtimeBaseDir }); + const sidecarPath = service.getPrSessionPathForArchiveState( + sessionId, + 'active', + ); + // History persisted before the "restart"; the bridge entry is fresh. + await upsertSessionPr(sidecarPath, { + number: 9100, + url: 'https://github.com/o/r/pull/9100', + }); + + await dispatcher.handle(conn, { + jsonrpc: '2.0', + id: 471, + method: '_qwen/session/update_metadata', + params: { + sessionId, + metadata: { + pr: { number: 9101, url: 'https://github.com/o/r/pull/9101' }, + }, + }, + }); + await waitUntil(() => + frames.some( + (frame) => + typeof frame === 'object' && + frame !== null && + 'id' in frame && + (frame as { id: unknown }).id === 471, + ), + ); + + const reply = frames.find( + (frame) => + typeof frame === 'object' && + frame !== null && + 'id' in frame && + (frame as { id: unknown }).id === 471, + ) as + | { result?: { prs?: Array<{ number: number; url: string }> } } + | undefined; + expect(reply?.result?.prs?.map((entry) => entry.number)).toEqual([ + 9100, 9101, + ]); + // The entry must be re-hydrated from the sidecar BEFORE the mutation + // so the `session_metadata_updated` event payload is complete too. + expect(bridge.metadataCallLog).toEqual(['seed', 'update']); + expect(bridge.seedSessionPrsCalls).toHaveLength(1); + expect(bridge.seedSessionPrsCalls[0]?.sessionId).toBe(sessionId); + expect( + bridge.seedSessionPrsCalls[0]?.prs.map((entry) => entry.number), + ).toEqual([9100]); + const persisted = await readSessionPrs(sidecarPath); + expect(persisted?.map((entry) => entry.number)).toEqual([9100, 9101]); + } finally { + registry.dispose(); + rememberLane.dispose(); + await fs.rm(runtimeBaseDir, { recursive: true, force: true }); + } + }); + + it('replies the persisted sidecar list, not the bridge echo, when they disagree over ACP', async () => { + // The in-memory list can diverge from the sidecar when an earlier + // upsert failed after the bridge already recorded the binding. The + // reply contract is the AUTHORITATIVE persisted list (what the REST + // routes echo), not this-daemon memory. + const sessionId = '550e8400-e29b-41d4-a716-446655440138'; + const runtimeBaseDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'qwen-acp-pr-disagree-'), + ); + const archiveCoordinator = new SessionArchiveCoordinator(); + const registry = new ConnectionRegistry(); + const rememberLane = new WorkspaceRememberTaskLane( + bridge as unknown as HttpAcpBridge, + ); + const dispatcher = new AcpDispatcher( + bridge as unknown as HttpAcpBridge, + TEST_WORKSPACE, + () => process.env, + fakeWorkspace, + rememberLane, + createRequestedSessionIdAdmission({ + archiveCoordinator, + getBridges: () => [bridge as unknown as HttpAcpBridge], + getPersistenceTargets: () => [ + { workspaceCwd: TEST_WORKSPACE, runtimeBaseDir }, + ], + }), + undefined, + undefined, + false, + registry, + archiveCoordinator, + () => true, + () => undefined, + undefined, + runtimeBaseDir, + ); + const conn = registry.create(true)!; + conn.ownSession(sessionId); + conn.getOrCreateSession(sessionId).clientId = 'client-pr-disagree'; + const frames: unknown[] = []; + conn.attachConnStream({ + kind: 'sse', + isClosed: false, + async send(message: unknown): Promise { + frames.push(message); + }, + async sendSerialized(payload: Buffer) { + frames.push(JSON.parse(payload.toString('utf8'))); + return 'delivered' as const; + }, + close(): void {}, + } satisfies TransportStream); + + try { + const service = new SessionService(TEST_WORKSPACE, { runtimeBaseDir }); + const sidecarPath = service.getPrSessionPathForArchiveState( + sessionId, + 'active', + ); + await upsertSessionPr(sidecarPath, { + number: 9100, + url: 'https://github.com/o/r/pull/9100', + }); + // This-daemon memory holds a binding whose persistence failed earlier. + bridge.metadataPrsBySession.set(sessionId, [ + { number: 9999, url: 'https://github.com/o/r/pull/9999' }, + ]); + + await dispatcher.handle(conn, { + jsonrpc: '2.0', + id: 472, + method: '_qwen/session/update_metadata', + params: { + sessionId, + metadata: { + pr: { number: 9101, url: 'https://github.com/o/r/pull/9101' }, + }, + }, + }); + await waitUntil(() => + frames.some( + (frame) => + typeof frame === 'object' && + frame !== null && + 'id' in frame && + (frame as { id: unknown }).id === 472, + ), + ); + + const reply = frames.find( + (frame) => + typeof frame === 'object' && + frame !== null && + 'id' in frame && + (frame as { id: unknown }).id === 472, + ) as + | { result?: { prs?: Array<{ number: number; url: string }> } } + | undefined; + expect(reply?.result?.prs?.map((entry) => entry.number)).toEqual([ + 9100, 9101, + ]); + } finally { + registry.dispose(); + rememberLane.dispose(); + await fs.rm(runtimeBaseDir, { recursive: true, force: true }); + } + }); + it('session/prompt reports an archive conflict while prompt is in flight', async () => { await withRuntimeDir(async () => { const sessionId = '550e8400-e29b-41d4-a716-446655440127'; diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index 3444c9af91a..bbfb4a5e85b 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -14,6 +14,7 @@ import { SessionService, Storage, createDebugLogger, + readSessionPrs, resetDebugLoggingState, setDebugLogSession, } from '@qwen-code/qwen-code-core'; @@ -619,7 +620,10 @@ function makeBridge( }, updateSessionMetadata( sessionId: string, - metadata: { displayName?: string }, + metadata: { + displayName?: string; + pr?: { number: number; url: string }; + }, context?: BridgeClientRequestContext, ) { metadataCalls.push({ @@ -629,6 +633,7 @@ function makeBridge( }); return { displayName: `${workspaceCwd}:${metadata.displayName ?? ''}`, + ...(metadata.pr ? { prs: [metadata.pr] } : {}), }; }, async generateSessionRecap( @@ -998,13 +1003,15 @@ function makeHarness(opts?: { }) { const primaryBridge = makeBridge( PRIMARY_CWD, - opts?.primarySummaries ?? [makeSummary('primary-session', PRIMARY_CWD)], + opts?.primarySummaries ?? [ + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD), + ], { channelLive: true }, ); const secondaryBridge = makeBridge( SECONDARY_CWD, opts?.secondarySummaries ?? [ - makeSummary('secondary-session', SECONDARY_CWD), + makeSummary('22222222-2222-4222-a222-222222222222', SECONDARY_CWD), ], { channelLive: opts?.secondaryChannelLive ?? true, @@ -1172,7 +1179,10 @@ describe('multi-workspace session dispatch', () => { full.body.full.sessions .map((session: { sessionId: string }) => session.sessionId) .sort(), - ).toEqual(['primary-session', 'secondary-session']); + ).toEqual([ + '11111111-1111-4111-a111-111111111111', + '22222222-2222-4222-a222-222222222222', + ]); }); it('rolls up secondary runtime channel issues in daemon status', async () => { @@ -1290,13 +1300,13 @@ describe('multi-workspace session dispatch', () => { const { app, secondaryBridge } = makeHarness({ secondaryTrusted: false }); const res = await request(app) - .get('/session/secondary-session/status') + .get('/session/22222222-2222-4222-a222-222222222222/status') .set('Host', host()); expect(res.status).toBe(403); expect(res.body.code).toBe('untrusted_workspace'); expect(res.body.error).toBe('Workspace is not trusted.'); - expect(res.body.sessionId).toBe('secondary-session'); + expect(res.body.sessionId).toBe('22222222-2222-4222-a222-222222222222'); expect(res.body.workspaceCwd).toBe(SECONDARY_CWD); expect(res.body.workspaceId).toBe('secondary-id'); expect(secondaryBridge.promptCalls).toEqual([]); @@ -1306,41 +1316,50 @@ describe('multi-workspace session dispatch', () => { const { app, primaryBridge, secondaryBridge } = makeHarness(); await request(app) - .post('/session/secondary-session/prompt') + .post('/session/22222222-2222-4222-a222-222222222222/prompt') .set('Host', host()) .set('X-Qwen-Client-Id', 'client-2') .send({ prompt: [{ type: 'text', text: 'hello' }] }) .expect(202); expect(primaryBridge.promptCalls).toEqual([]); expect(secondaryBridge.promptCalls).toMatchObject([ - { sessionId: 'secondary-session', context: { clientId: 'client-2' } }, + { + sessionId: '22222222-2222-4222-a222-222222222222', + context: { clientId: 'client-2' }, + }, ]); const status = await request(app) - .get('/session/secondary-session/status') + .get('/session/22222222-2222-4222-a222-222222222222/status') .set('Host', host()) .expect(200); expect(status.body.workspaceCwd).toBe(SECONDARY_CWD); await request(app) - .post('/session/secondary-session/cancel') + .post('/session/22222222-2222-4222-a222-222222222222/cancel') .set('Host', host()) .send({}) .expect(204); await request(app) - .post('/session/secondary-session/heartbeat') + .post('/session/22222222-2222-4222-a222-222222222222/heartbeat') .set('Host', host()) .send({}) .expect(200); await request(app) - .post('/session/secondary-session/detach') + .post('/session/22222222-2222-4222-a222-222222222222/detach') .set('Host', host()) .send({}) .expect(204); - expect(secondaryBridge.cancelCalls).toEqual(['secondary-session']); - expect(secondaryBridge.heartbeatCalls).toEqual(['secondary-session']); - expect(secondaryBridge.detachCalls).toEqual(['secondary-session']); + expect(secondaryBridge.cancelCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); + expect(secondaryBridge.heartbeatCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); + expect(secondaryBridge.detachCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); }); it('routes secondary rewind snapshots, rewind, and shell only to the owner bridge', async () => { @@ -1353,7 +1372,9 @@ describe('multi-workspace session dispatch', () => { test.set('Host', host()).set('Authorization', 'Bearer secret'); const snapshots = await auth( - request(app).get('/session/secondary-session/rewind/snapshots'), + request(app).get( + '/session/22222222-2222-4222-a222-222222222222/rewind/snapshots', + ), ); expect(snapshots.status).toBe(200); expect(snapshots.body.snapshots[0].promptId).toBe( @@ -1361,7 +1382,7 @@ describe('multi-workspace session dispatch', () => { ); const rewind = await auth( - request(app).post('/session/secondary-session/rewind'), + request(app).post('/session/22222222-2222-4222-a222-222222222222/rewind'), ) .set('X-Qwen-Client-Id', 'client-2') .send({ promptId: 'secondary-prompt', rewindFiles: true }); @@ -1369,7 +1390,7 @@ describe('multi-workspace session dispatch', () => { expect(rewind.body.filesChanged).toEqual(['tracked.txt']); const shell = await auth( - request(app).post('/session/secondary-session/shell'), + request(app).post('/session/22222222-2222-4222-a222-222222222222/shell'), ) .set('X-Qwen-Client-Id', 'client-2') .send({ command: ' pwd ' }); @@ -1379,30 +1400,32 @@ describe('multi-workspace session dispatch', () => { expect(primaryBridge.rewindSnapshotCalls).toEqual([]); expect(primaryBridge.rewindCalls).toEqual([]); expect(primaryBridge.shellCalls).toEqual([]); - expect(secondaryBridge.rewindSnapshotCalls).toEqual(['secondary-session']); + expect(secondaryBridge.rewindSnapshotCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); expect(secondaryBridge.rewindCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', req: { promptId: 'secondary-prompt', rewindFiles: true }, context: { clientId: 'client-2' }, }, ]); expect(secondaryBridge.shellCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', command: 'pwd', signal: expect.any(AbortSignal), context: { clientId: 'client-2' }, }, ]); expect(daemonLog.info).toHaveBeenCalledWith('rewind snapshots loaded', { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', snapshotCount: 1, workspaceId: 'secondary-id', workspaceCwd: SECONDARY_CWD, }); expect(daemonLog.info).toHaveBeenCalledWith('session rewind completed', { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', promptId: 'secondary-prompt', rewindFiles: true, rewound: true, @@ -1412,7 +1435,7 @@ describe('multi-workspace session dispatch', () => { workspaceCwd: SECONDARY_CWD, }); expect(daemonLog.info).toHaveBeenCalledWith('shell command completed', { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', clientId: 'client-2', exitCode: 0, workspaceId: 'secondary-id', @@ -1426,7 +1449,7 @@ describe('multi-workspace session dispatch', () => { }); const rewind = (body: Record) => request(app) - .post('/session/secondary-session/rewind') + .post('/session/22222222-2222-4222-a222-222222222222/rewind') .set('Host', host()) .set('Authorization', 'Bearer secret') .send(body); @@ -1468,7 +1491,7 @@ describe('multi-workspace session dispatch', () => { const untrusted = makeHarness({ secondaryTrusted: false }); const untrustedRes = await request(untrusted.app) - .get('/session/secondary-session/rewind/snapshots') + .get('/session/22222222-2222-4222-a222-222222222222/rewind/snapshots') .set('Host', host()); expect(untrustedRes.status).toBe(403); expect(untrustedRes.body.code).toBe('untrusted_workspace'); @@ -1497,13 +1520,13 @@ describe('multi-workspace session dispatch', () => { test.set('Host', host()).set('Authorization', 'Bearer secret'); const rewind = await auth( - request(app).post('/session/secondary-session/rewind'), + request(app).post('/session/22222222-2222-4222-a222-222222222222/rewind'), ).send({ promptId: 'secondary-prompt' }); expect(rewind.status).toBe(403); expect(rewind.body.code).toBe('untrusted_workspace'); const shell = await auth( - request(app).post('/session/secondary-session/shell'), + request(app).post('/session/22222222-2222-4222-a222-222222222222/shell'), ) .set('X-Qwen-Client-Id', 'client-2') .send({ command: 'pwd' }); @@ -1542,7 +1565,7 @@ describe('multi-workspace session dispatch', () => { }, }); const pending = request(app) - .post('/session/secondary-session/shell') + .post('/session/22222222-2222-4222-a222-222222222222/shell') .set('Host', host()) .set('Authorization', 'Bearer secret') .set('X-Qwen-Client-Id', 'client-2') @@ -1564,7 +1587,7 @@ describe('multi-workspace session dispatch', () => { it('preserves strict shell validation order for a secondary owner', async () => { const disabled = makeHarness({ serveOptions: { token: 'secret' } }); const disabledResponse = await request(disabled.app) - .post('/session/secondary-session/shell') + .post('/session/22222222-2222-4222-a222-222222222222/shell') .set('Host', host()) .set('Authorization', 'Bearer secret') .send({ command: '' }); @@ -1575,14 +1598,14 @@ describe('multi-workspace session dispatch', () => { serveOptions: { token: 'secret', enableSessionShell: true }, }); const tokenRequired = await request(enabled.app) - .post('/session/secondary-session/shell') + .post('/session/22222222-2222-4222-a222-222222222222/shell') .set('Host', host()) .send({ command: 'pwd' }); expect(tokenRequired.status).toBe(401); expect(tokenRequired.body.error).toBe('Unauthorized'); const clientRequired = await request(enabled.app) - .post('/session/secondary-session/shell') + .post('/session/22222222-2222-4222-a222-222222222222/shell') .set('Host', host()) .set('Authorization', 'Bearer secret') .send({ command: '' }); @@ -1590,7 +1613,7 @@ describe('multi-workspace session dispatch', () => { expect(clientRequired.body.code).toBe('client_id_required'); const emptyCommand = await request(enabled.app) - .post('/session/secondary-session/shell') + .post('/session/22222222-2222-4222-a222-222222222222/shell') .set('Host', host()) .set('Authorization', 'Bearer secret') .set('X-Qwen-Client-Id', 'client-2') @@ -1608,25 +1631,27 @@ describe('multi-workspace session dispatch', () => { }); await request(app) - .get('/session/primary-session/rewind/snapshots') + .get('/session/11111111-1111-4111-a111-111111111111/rewind/snapshots') .set('Host', host()) .set('Authorization', 'Bearer secret') .expect(200); await request(app) - .post('/session/primary-session/rewind') + .post('/session/11111111-1111-4111-a111-111111111111/rewind') .set('Host', host()) .set('Authorization', 'Bearer secret') .send({ promptId: 'primary-prompt', rewindFiles: false }) .expect(200); await request(app) - .post('/session/primary-session/shell') + .post('/session/11111111-1111-4111-a111-111111111111/shell') .set('Host', host()) .set('Authorization', 'Bearer secret') .set('X-Qwen-Client-Id', 'client-1') .send({ command: 'pwd' }) .expect(200); - expect(primaryBridge.rewindSnapshotCalls).toEqual(['primary-session']); + expect(primaryBridge.rewindSnapshotCalls).toEqual([ + '11111111-1111-4111-a111-111111111111', + ]); expect(primaryBridge.rewindCalls).toHaveLength(1); expect(primaryBridge.shellCalls).toHaveLength(1); expect(secondaryBridge.rewindSnapshotCalls).toEqual([]); @@ -1636,7 +1661,7 @@ describe('multi-workspace session dispatch', () => { it('keeps rewind and shell behavior in a single-workspace daemon', async () => { const bridge = makeBridge(PRIMARY_CWD, [ - makeSummary('primary-session', PRIMARY_CWD), + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD), ]); const app = createServeApp( { @@ -1660,17 +1685,25 @@ describe('multi-workspace session dispatch', () => { ); await auth( - request(app).get('/session/primary-session/rewind/snapshots'), + request(app).get( + '/session/11111111-1111-4111-a111-111111111111/rewind/snapshots', + ), ).expect(200); - await auth(request(app).post('/session/primary-session/rewind')) + await auth( + request(app).post('/session/11111111-1111-4111-a111-111111111111/rewind'), + ) .send({ promptId: 'primary-prompt', rewindFiles: false }) .expect(200); - await auth(request(app).post('/session/primary-session/shell')) + await auth( + request(app).post('/session/11111111-1111-4111-a111-111111111111/shell'), + ) .set('X-Qwen-Client-Id', 'client-1') .send({ command: 'pwd' }) .expect(200); - expect(bridge.rewindSnapshotCalls).toEqual(['primary-session']); + expect(bridge.rewindSnapshotCalls).toEqual([ + '11111111-1111-4111-a111-111111111111', + ]); expect(bridge.rewindCalls).toHaveLength(1); expect(bridge.shellCalls).toHaveLength(1); }); @@ -1681,7 +1714,7 @@ describe('multi-workspace session dispatch', () => { promptId: string, ) => request(app) - .post('/session/secondary-session/rewind') + .post('/session/22222222-2222-4222-a222-222222222222/rewind') .set('Host', host()) .set('Authorization', 'Bearer secret') .send({ promptId }); @@ -1730,16 +1763,20 @@ describe('multi-workspace session dispatch', () => { const { app, primaryBridge, secondaryBridge } = makeHarness(); await request(app) - .get('/session/secondary-session/events?snapshot=1&maxQueued=16') + .get( + '/session/22222222-2222-4222-a222-222222222222/events?snapshot=1&maxQueued=16', + ) .set('Host', host()) .expect(200); expect(primaryBridge.eventsCalls).toEqual([]); expect(secondaryBridge.eventsCalls).toEqual([ - expect.objectContaining({ sessionId: 'secondary-session' }), + expect.objectContaining({ + sessionId: '22222222-2222-4222-a222-222222222222', + }), ]); await request(app) - .post('/session/secondary-session/permission/perm-1') + .post('/session/22222222-2222-4222-a222-222222222222/permission/perm-1') .set('Host', host()) .set('X-Qwen-Client-Id', 'client-2') .send({ outcome: { outcome: 'cancelled' } }) @@ -1747,13 +1784,13 @@ describe('multi-workspace session dispatch', () => { expect(primaryBridge.permissionCalls).toEqual([]); expect(secondaryBridge.permissionCalls).toEqual([ expect.objectContaining({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', requestId: 'perm-1', }), ]); const pending = await request(app) - .get('/session/secondary-session/pending-prompts') + .get('/session/22222222-2222-4222-a222-222222222222/pending-prompts') .set('Host', host()) .set('X-Qwen-Client-Id', 'client-2') .expect(200); @@ -1762,24 +1799,33 @@ describe('multi-workspace session dispatch', () => { ]); await request(app) - .delete('/session/secondary-session/pending-prompts/prompt-1') + .delete( + '/session/22222222-2222-4222-a222-222222222222/pending-prompts/prompt-1', + ) .set('Host', host()) .set('X-Qwen-Client-Id', 'client-2') .expect(200); expect(primaryBridge.pendingPromptCalls).toEqual([]); expect(primaryBridge.removePendingPromptCalls).toEqual([]); - expect(secondaryBridge.pendingPromptCalls).toEqual(['secondary-session']); + expect(secondaryBridge.pendingPromptCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); expect(secondaryBridge.removePendingPromptCalls).toEqual([ - { sessionId: 'secondary-session', promptId: 'prompt-1' }, + { + sessionId: '22222222-2222-4222-a222-222222222222', + promptId: 'prompt-1', + }, ]); await request(app) - .delete('/session/secondary-session') + .delete('/session/22222222-2222-4222-a222-222222222222') .set('Host', host()) .set('X-Qwen-Client-Id', 'client-2') .expect(204); expect(primaryBridge.closeCalls).toEqual([]); - expect(secondaryBridge.closeCalls).toEqual(['secondary-session']); + expect(secondaryBridge.closeCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); }); it('returns session_not_found instead of falling back to primary on live owner miss', async () => { @@ -1844,7 +1890,7 @@ describe('multi-workspace session dispatch', () => { for (const action of ['load', 'resume'] as const) { const res = await request(app) - .post(`/session/secondary-session/${action}`) + .post(`/session/22222222-2222-4222-a222-222222222222/${action}`) .set('Host', host()) .send({ cwd: SECONDARY_CWD }); @@ -1857,14 +1903,14 @@ describe('multi-workspace session dispatch', () => { { action: 'load', req: expect.objectContaining({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', workspaceCwd: SECONDARY_CWD, }), }, { action: 'resume', req: expect.objectContaining({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', workspaceCwd: SECONDARY_CWD, }), }, @@ -2329,7 +2375,7 @@ describe('multi-workspace session dispatch', () => { }); const res = await request(app) - .post(`/session/secondary-session/${suffix}`) + .post(`/session/22222222-2222-4222-a222-222222222222/${suffix}`) .set('Host', host()) .send(body); @@ -2337,7 +2383,7 @@ describe('multi-workspace session dispatch', () => { expect(res.body).toEqual({ error: `Route "${route}" is only available for primary workspace sessions.`, code: 'non_primary_session_route_not_supported', - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', workspaceId: 'secondary-id', workspaceCwd: SECONDARY_CWD, route, @@ -2347,7 +2393,7 @@ describe('multi-workspace session dispatch', () => { expect(daemonLog.warn).toHaveBeenCalledWith('session routing failed', { route, resolutionKind: 'non_primary_session_route_not_supported', - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', workspaceId: 'secondary-id', workspaceCwd: SECONDARY_CWD, }); @@ -2360,7 +2406,7 @@ describe('multi-workspace session dispatch', () => { }); const response = await request(app) - .post('/session/secondary-session/cd') + .post('/session/22222222-2222-4222-a222-222222222222/cd') .set('Host', host()) .send({ path: path.resolve(path.sep, 'work', 'next') }); @@ -2369,7 +2415,7 @@ describe('multi-workspace session dispatch', () => { error: 'Route "POST /session/:id/cd" is only available for primary workspace sessions.', code: 'non_primary_session_route_not_supported', - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', route: 'POST /session/:id/cd', }); }); @@ -2401,14 +2447,14 @@ describe('multi-workspace session dispatch', () => { }); const response = await request(app) - .post(`/session/secondary-session/${suffix}`) + .post(`/session/22222222-2222-4222-a222-222222222222/${suffix}`) .set('Host', host()) .send(body); expect(response.status).toBe(expectedStatus); expect(primaryBridge.primaryOnlyMutationCalls).toEqual([]); expect(secondaryBridge.primaryOnlyMutationCalls).toEqual([ - { route: mutation, sessionId: 'secondary-session' }, + { route: mutation, sessionId: '22222222-2222-4222-a222-222222222222' }, ]); }, ); @@ -2417,7 +2463,7 @@ describe('multi-workspace session dispatch', () => { const { app, primaryBridge, secondaryBridge } = makeHarness(); const res = await request(app) - .post('/session/secondary-session/model') + .post('/session/22222222-2222-4222-a222-222222222222/model') .set('Host', host()) .set('X-Qwen-Client-Id', 'client-1') .send({ modelId: 'qwen3-coder' }); @@ -2426,7 +2472,7 @@ describe('multi-workspace session dispatch', () => { expect(res.body).toMatchObject({ _meta: { applied: true } }); expect(secondaryBridge.setModelCalls).toHaveLength(1); expect(secondaryBridge.setModelCalls[0]?.sessionId).toBe( - 'secondary-session', + '22222222-2222-4222-a222-222222222222', ); expect(secondaryBridge.setModelCalls[0]?.req.modelId).toBe('qwen3-coder'); expect(secondaryBridge.setModelCalls[0]?.context).toEqual({ @@ -2441,19 +2487,19 @@ describe('multi-workspace session dispatch', () => { const { app, primaryBridge, secondaryBridge } = makeHarness(); const res = await request(app) - .post('/session/secondary-session/approval-mode') + .post('/session/22222222-2222-4222-a222-222222222222/approval-mode') .set('Host', host()) .send({ mode: 'yolo', persist: true }); expect(res.status).toBe(200); expect(res.body).toMatchObject({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', mode: 'yolo', persisted: true, }); expect(secondaryBridge.setApprovalModeCalls).toHaveLength(1); expect(secondaryBridge.setApprovalModeCalls[0]).toMatchObject({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', mode: 'yolo', opts: { persist: true }, }); @@ -2466,7 +2512,7 @@ describe('multi-workspace session dispatch', () => { const { app, secondaryBridge } = makeHarness({ secondaryTrusted: false }); const modelRes = await request(app) - .post('/session/secondary-session/model') + .post('/session/22222222-2222-4222-a222-222222222222/model') .set('Host', host()) .send({ modelId: 'qwen3-coder' }); expect(modelRes.status).toBe(403); @@ -2474,7 +2520,7 @@ describe('multi-workspace session dispatch', () => { expect(secondaryBridge.setModelCalls).toEqual([]); const approvalRes = await request(app) - .post('/session/secondary-session/approval-mode') + .post('/session/22222222-2222-4222-a222-222222222222/approval-mode') .set('Host', host()) .send({ mode: 'yolo' }); expect(approvalRes.status).toBe(403); @@ -2488,43 +2534,43 @@ describe('multi-workspace session dispatch', () => { }); const metadataRes = await request(app) - .patch('/session/secondary-session/metadata') + .patch('/session/22222222-2222-4222-a222-222222222222/metadata') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .set('X-Qwen-Client-Id', 'secondary-client') .send({ displayName: 'renamed' }); expect(metadataRes.status).toBe(200); expect(metadataRes.body).toEqual({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', displayName: `${SECONDARY_CWD}:renamed`, }); const recapRes = await request(app) - .post('/session/secondary-session/recap') + .post('/session/22222222-2222-4222-a222-222222222222/recap') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .set('X-Qwen-Client-Id', 'secondary-client') .send({}); expect(recapRes.status).toBe(200); expect(recapRes.body).toEqual({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', recap: `${SECONDARY_CWD}:recap`, }); const btwRes = await request(app) - .post('/session/secondary-session/btw') + .post('/session/22222222-2222-4222-a222-222222222222/btw') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .set('X-Qwen-Client-Id', 'secondary-client') .send({ question: ' why? ' }); expect(btwRes.status).toBe(200); expect(btwRes.body).toEqual({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', answer: `${SECONDARY_CWD}:answer`, }); const midTurnRes = await request(app) - .post('/session/secondary-session/mid-turn-message') + .post('/session/22222222-2222-4222-a222-222222222222/mid-turn-message') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .set('X-Qwen-Client-Id', 'secondary-client') @@ -2536,7 +2582,9 @@ describe('multi-workspace session dispatch', () => { }); const removeMidTurnRes = await request(app) - .delete('/session/secondary-session/mid-turn-messages/mid-secondary') + .delete( + '/session/22222222-2222-4222-a222-222222222222/mid-turn-messages/mid-secondary', + ) .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .set('X-Qwen-Client-Id', 'secondary-client'); @@ -2544,7 +2592,7 @@ describe('multi-workspace session dispatch', () => { expect(removeMidTurnRes.body).toEqual({ removed: true }); const taskCancelRes = await request(app) - .post('/session/secondary-session/tasks/task-1/cancel') + .post('/session/22222222-2222-4222-a222-222222222222/tasks/task-1/cancel') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ kind: 'shell' }); @@ -2552,7 +2600,7 @@ describe('multi-workspace session dispatch', () => { expect(taskCancelRes.body).toEqual({ cancelled: true }); const goalClearRes = await request(app) - .post('/session/secondary-session/goal/clear') + .post('/session/22222222-2222-4222-a222-222222222222/goal/clear') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({}); @@ -2564,20 +2612,20 @@ describe('multi-workspace session dispatch', () => { expect(secondaryBridge.metadataCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', metadata: { displayName: 'renamed' }, context: { clientId: 'secondary-client' }, }, ]); expect(secondaryBridge.recapCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', context: { clientId: 'secondary-client' }, }, ]); expect(secondaryBridge.btwCalls).toEqual([ expect.objectContaining({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', question: 'why?', signal: expect.any(AbortSignal), context: { clientId: 'secondary-client' }, @@ -2586,26 +2634,28 @@ describe('multi-workspace session dispatch', () => { expect(secondaryBridge.btwCalls[0]?.signal?.aborted).toBe(false); expect(secondaryBridge.midTurnMessageCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', message: 'remember this', context: { clientId: 'secondary-client' }, }, ]); expect(secondaryBridge.removeMidTurnMessageCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', messageId: 'mid-secondary', context: { clientId: 'secondary-client' }, }, ]); expect(secondaryBridge.taskCancelCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', taskId: 'task-1', taskKind: 'shell', }, ]); - expect(secondaryBridge.goalClearCalls).toEqual(['secondary-session']); + expect(secondaryBridge.goalClearCalls).toEqual([ + '22222222-2222-4222-a222-222222222222', + ]); for (const calls of [ primaryBridge.metadataCalls, @@ -2620,6 +2670,46 @@ describe('multi-workspace session dispatch', () => { } }); + it('persists a cross-workspace pr sidecar in the OWNING workspace chats dir', async () => { + // A primary-route metadata PATCH against a secondary-owned session must + // write the sidecar under the SECONDARY runtime — landing it under the + // primary would hide the binding from the owning workspace's listing. + const { app, secondaryBridge } = makeHarness({ token: TEST_TOKEN }); + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const secondaryPath = new SessionService( + SECONDARY_CWD, + ).getPrSessionPathForArchiveState( + '22222222-2222-4222-a222-222222222222', + 'active', + ); + const primaryPath = new SessionService( + PRIMARY_CWD, + ).getPrSessionPathForArchiveState( + '22222222-2222-4222-a222-222222222222', + 'active', + ); + await fsp.rm(secondaryPath, { force: true }); + await fsp.rm(primaryPath, { force: true }); + + try { + const res = await request(app) + .patch('/session/22222222-2222-4222-a222-222222222222/metadata') + .set('Host', host()) + .set('Authorization', TEST_AUTHORIZATION) + .set('X-Qwen-Client-Id', 'secondary-client') + .send({ pr }); + expect(res.status).toBe(200); + expect(res.body.prs).toEqual([pr]); + expect(secondaryBridge.metadataCalls[0]?.metadata).toEqual({ pr }); + + const persisted = await readSessionPrs(secondaryPath); + expect(persisted?.map((entry) => entry.number)).toEqual([9517]); + await expect(fsp.access(primaryPath)).rejects.toThrow(); + } finally { + await fsp.rm(secondaryPath, { force: true }); + } + }); + it('routes continue, language, and artifact mutations to the owning non-primary bridge', async () => { const { app, primaryBridge, secondaryBridge } = makeHarness({ token: TEST_TOKEN, @@ -2628,12 +2718,16 @@ describe('multi-workspace session dispatch', () => { test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); const firstContinue = await auth( - request(app).post('/session/secondary-session/continue'), + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/continue', + ), ) .set('X-Qwen-Client-Id', 'secondary-client') .send({}); const secondContinue = await auth( - request(app).post('/session/secondary-session/continue'), + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/continue', + ), ) .set('X-Qwen-Client-Id', 'secondary-client') .send({}); @@ -2646,7 +2740,9 @@ describe('multi-workspace session dispatch', () => { expect(secondContinue.body.promptId).not.toBe(firstContinue.body.promptId); const language = await auth( - request(app).post('/session/secondary-session/language'), + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/language', + ), ) .set('X-Qwen-Client-Id', 'secondary-client') .send({ language: 'zh', syncOutputLanguage: true }); @@ -2658,7 +2754,9 @@ describe('multi-workspace session dispatch', () => { }); const addArtifact = await auth( - request(app).post('/session/secondary-session/artifacts'), + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/artifacts', + ), ) .set('X-Qwen-Client-Id', 'secondary-client') .send({ @@ -2669,24 +2767,24 @@ describe('multi-workspace session dispatch', () => { expect(addArtifact.status).toBe(200); expect(addArtifact.body).toMatchObject({ v: 1, - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', }); const removeArtifact = await auth( request(app).delete( - '/session/secondary-session/artifacts/artifact-secondary', + '/session/22222222-2222-4222-a222-222222222222/artifacts/artifact-secondary', ), ).set('X-Qwen-Client-Id', 'secondary-client'); expect(removeArtifact.status).toBe(200); expect(removeArtifact.body).toMatchObject({ v: 1, - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', }); expect(secondaryBridge.continueCalls).toHaveLength(2); for (const call of secondaryBridge.continueCalls) { expect(call).toMatchObject({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', context: { clientId: 'secondary-client', promptId: expect.any(String), @@ -2695,14 +2793,14 @@ describe('multi-workspace session dispatch', () => { } expect(secondaryBridge.languageCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', params: { language: 'zh', syncOutputLanguage: true }, context: { clientId: 'secondary-client' }, }, ]); expect(secondaryBridge.addArtifactCalls).toEqual([ expect.objectContaining({ - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', artifact: expect.objectContaining({ title: 'Secondary artifact', url: 'https://example.com/secondary', @@ -2713,7 +2811,7 @@ describe('multi-workspace session dispatch', () => { ]); expect(secondaryBridge.removeArtifactCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', artifactId: 'artifact-secondary', context: { clientId: 'secondary-client' }, }, @@ -2729,16 +2827,18 @@ describe('multi-workspace session dispatch', () => { const responses = await Promise.all([ request(app) - .post('/session/secondary-session/continue') + .post('/session/22222222-2222-4222-a222-222222222222/continue') .set('Host', host()) .send({}), request(app) - .post('/session/secondary-session/artifacts') + .post('/session/22222222-2222-4222-a222-222222222222/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') + .delete( + '/session/22222222-2222-4222-a222-222222222222/artifacts/artifact-secondary', + ) .set('Host', host()) .set('X-Qwen-Client-Id', 'secondary-client'), ]); @@ -2747,13 +2847,13 @@ describe('multi-workspace session dispatch', () => { ]); const language = await request(app) - .post('/session/secondary-session/language') + .post('/session/22222222-2222-4222-a222-222222222222/language') .set('Host', host()) .send({ language: 'zh' }); expect(language.status).toBe(200); expect(secondaryBridge.languageCalls).toEqual([ { - sessionId: 'secondary-session', + sessionId: '22222222-2222-4222-a222-222222222222', params: { language: 'zh', syncOutputLanguage: false }, }, ]); @@ -2773,16 +2873,28 @@ describe('multi-workspace session dispatch', () => { 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({ + auth( + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/continue', + ), + ).send({}), + auth( + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/language', + ), + ).send({ language: 'zh', }), - auth(request(app).post('/session/secondary-session/artifacts')) + auth( + request(app).post( + '/session/22222222-2222-4222-a222-222222222222/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', + '/session/22222222-2222-4222-a222-222222222222/artifacts/artifact-secondary', ), ).set('X-Qwen-Client-Id', 'secondary-client'), ]); @@ -2878,13 +2990,25 @@ describe('multi-workspace session dispatch', () => { test.set('Host', host()).set('Authorization', TEST_AUTHORIZATION); const responses = await Promise.all([ - auth(request(app).post('/session/primary-session/continue')) + auth( + request(app).post( + '/session/11111111-1111-4111-a111-111111111111/continue', + ), + ) .set('X-Qwen-Client-Id', 'primary-client') .send({}), - auth(request(app).post('/session/primary-session/language')) + auth( + request(app).post( + '/session/11111111-1111-4111-a111-111111111111/language', + ), + ) .set('X-Qwen-Client-Id', 'primary-client') .send({ language: 'en', syncOutputLanguage: true }), - auth(request(app).post('/session/primary-session/artifacts')) + auth( + request(app).post( + '/session/11111111-1111-4111-a111-111111111111/artifacts', + ), + ) .set('X-Qwen-Client-Id', 'primary-client') .send({ title: 'Primary artifact', @@ -2892,7 +3016,7 @@ describe('multi-workspace session dispatch', () => { }), auth( request(app).delete( - '/session/primary-session/artifacts/artifact-primary', + '/session/11111111-1111-4111-a111-111111111111/artifacts/artifact-primary', ), ).set('X-Qwen-Client-Id', 'primary-client'), ]); @@ -2901,7 +3025,7 @@ describe('multi-workspace session dispatch', () => { ]); expect(primaryBridge.continueCalls).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', context: { clientId: 'primary-client', promptId: expect.any(String), @@ -2910,14 +3034,14 @@ describe('multi-workspace session dispatch', () => { ]); expect(primaryBridge.languageCalls).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', params: { language: 'en', syncOutputLanguage: true }, context: { clientId: 'primary-client' }, }, ]); expect(primaryBridge.addArtifactCalls).toEqual([ expect.objectContaining({ - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', artifact: expect.objectContaining({ title: 'Primary artifact', url: 'https://example.com/primary', @@ -2927,7 +3051,7 @@ describe('multi-workspace session dispatch', () => { ]); expect(primaryBridge.removeArtifactCalls).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', artifactId: 'artifact-primary', context: { clientId: 'primary-client' }, }, @@ -2944,7 +3068,7 @@ describe('multi-workspace session dispatch', () => { }); const res = await request(app) - .patch('/session/primary-session/metadata') + .patch('/session/11111111-1111-4111-a111-111111111111/metadata') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ displayName: 'primary renamed' }); @@ -2953,7 +3077,7 @@ describe('multi-workspace session dispatch', () => { expect(res.body.displayName).toBe(`${PRIMARY_CWD}:primary renamed`); expect(primaryBridge.metadataCalls).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', metadata: { displayName: 'primary renamed' }, }, ]); @@ -2967,19 +3091,23 @@ describe('multi-workspace session dispatch', () => { const responses = await Promise.all([ request(app) - .patch('/session/secondary-session/metadata') + .patch('/session/22222222-2222-4222-a222-222222222222/metadata') .set('Host', host()) .send({ displayName: 'unauthorized' }), request(app) - .post('/session/secondary-session/tasks/task-1/cancel') + .post( + '/session/22222222-2222-4222-a222-222222222222/tasks/task-1/cancel', + ) .set('Host', host()) .send({ kind: 'shell' }), request(app) - .post('/session/secondary-session/goal/clear') + .post('/session/22222222-2222-4222-a222-222222222222/goal/clear') .set('Host', host()) .send({}), request(app) - .delete('/session/secondary-session/mid-turn-messages/mid-1') + .delete( + '/session/22222222-2222-4222-a222-222222222222/mid-turn-messages/mid-1', + ) .set('Host', host()), ]); @@ -3003,22 +3131,24 @@ describe('multi-workspace session dispatch', () => { const responses = await Promise.all([ request(app) - .patch('/session/secondary-session/metadata') + .patch('/session/22222222-2222-4222-a222-222222222222/metadata') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ displayName: 42 }), request(app) - .post('/session/secondary-session/btw') + .post('/session/22222222-2222-4222-a222-222222222222/btw') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ question: ' ' }), request(app) - .post('/session/secondary-session/mid-turn-message') + .post('/session/22222222-2222-4222-a222-222222222222/mid-turn-message') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ message: ' ' }), request(app) - .post('/session/secondary-session/tasks/task-1/cancel') + .post( + '/session/22222222-2222-4222-a222-222222222222/tasks/task-1/cancel', + ) .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ kind: 'invalid' }), @@ -3043,40 +3173,44 @@ describe('multi-workspace session dispatch', () => { const responses = await Promise.all([ request(app) - .patch('/session/secondary-session/metadata') + .patch('/session/22222222-2222-4222-a222-222222222222/metadata') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ displayName: 'blocked' }), request(app) - .post('/session/secondary-session/recap') + .post('/session/22222222-2222-4222-a222-222222222222/recap') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({}), request(app) - .post('/session/secondary-session/btw') + .post('/session/22222222-2222-4222-a222-222222222222/btw') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ question: 'blocked?' }), request(app) - .post('/session/secondary-session/mid-turn-message') + .post('/session/22222222-2222-4222-a222-222222222222/mid-turn-message') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ message: 'blocked' }), request(app) - .get('/session/secondary-session/mid-turn-messages') + .get('/session/22222222-2222-4222-a222-222222222222/mid-turn-messages') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION), request(app) - .post('/session/secondary-session/tasks/task-1/cancel') + .post( + '/session/22222222-2222-4222-a222-222222222222/tasks/task-1/cancel', + ) .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({ kind: 'agent' }), request(app) - .delete('/session/secondary-session/mid-turn-messages/mid-blocked') + .delete( + '/session/22222222-2222-4222-a222-222222222222/mid-turn-messages/mid-blocked', + ) .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION), request(app) - .post('/session/secondary-session/goal/clear') + .post('/session/22222222-2222-4222-a222-222222222222/goal/clear') .set('Host', host()) .set('Authorization', TEST_AUTHORIZATION) .send({}), @@ -5800,7 +5934,7 @@ describe('workspace session live-state route', () => { it('returns the exact v1 shape with projected volatile fields and no-store', async () => { const { app } = makeHarness({ primarySummaries: [ - makeSummary('primary-session', PRIMARY_CWD, { + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD, { displayName: 'Visible name', updatedAt: '2026-07-08T00:02:00.000Z', clientCount: 2, @@ -5829,7 +5963,7 @@ describe('workspace session live-state route', () => { // extras (pendingInteractionCount, hasTurnError) all stay out. expect(res.body.sessions).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', clientCount: 2, hasActivePrompt: true, isWaitingForPermission: true, @@ -5845,7 +5979,9 @@ describe('workspace session live-state route', () => { // absent rather than null so old clients decode the response unchanged. const { app } = makeHarness({ primarySummaries: [ - makeSummary('primary-session', PRIMARY_CWD, { updatedAt: undefined }), + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD, { + updatedAt: undefined, + }), ], }); @@ -5863,7 +5999,9 @@ describe('workspace session live-state route', () => { // them must not serialize a missing key where the SDK snapshot promises a // boolean. const { app } = makeHarness({ - primarySummaries: [makeSummary('primary-session', PRIMARY_CWD)], + primarySummaries: [ + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD), + ], }); const res = await request(app) @@ -5873,7 +6011,7 @@ describe('workspace session live-state route', () => { expect(res.body.sessions).toEqual([ { - sessionId: 'primary-session', + sessionId: '11111111-1111-4111-a111-111111111111', clientCount: 1, hasActivePrompt: false, isWaitingForPermission: false, @@ -5896,7 +6034,7 @@ describe('workspace session live-state route', () => { it('reads only the selected workspace bridge for trusted selectors', async () => { const { app, primaryBridge, secondaryBridge } = makeHarness({ primarySummaries: [ - makeSummary('primary-session', PRIMARY_CWD, { + makeSummary('11111111-1111-4111-a111-111111111111', PRIMARY_CWD, { updatedAt: '2026-07-08T00:05:00.000Z', }), ], @@ -5931,7 +6069,9 @@ describe('workspace session live-state route', () => { it('rejects an untrusted runtime with 403 before any bridge read', async () => { const { app, secondaryBridge } = makeHarness({ secondaryTrusted: false, - secondarySummaries: [makeSummary('secondary-session', SECONDARY_CWD)], + secondarySummaries: [ + makeSummary('22222222-2222-4222-a222-222222222222', SECONDARY_CWD), + ], }); const res = await request(app) @@ -6253,7 +6393,7 @@ describe('workspace session live-state route', () => { const { app, primaryBridge } = makeHarness({ token: 'secret' }); const v0 = primaryBridge.getSessionCatalogVersion().revision; await request(app) - .patch('/session/primary-session/metadata') + .patch('/session/11111111-1111-4111-a111-111111111111/metadata') .set('Host', host()) .set('Authorization', 'Bearer secret') .send({ displayName: 'Renamed' }) diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 010253e727b..3cf188566b1 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -26,6 +26,9 @@ import { writeWorktreeSessionMarker, writeWorktreeSession, readWorktreeSession, + readSessionPrs, + upsertSessionPr, + SESSION_PR_URL_MAX_LENGTH, type ApprovalMode, type SessionGroupColor, type SessionGroupPresetColor, @@ -54,7 +57,11 @@ import express, { type Response, } from 'express'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; -import { parseCallerSuppliedSessionId } from '../../config/session-id.js'; +import { + isValidSessionId, + normalizeSessionIdForLookup, + parseCallerSuppliedSessionId, +} from '../../config/session-id.js'; import { isChannelDeliveryError } from '../../runtime/channel-delivery-ipc.js'; import { parseChannelDelivery } from '../../runtime/channel-delivery.js'; import { @@ -2094,6 +2101,48 @@ export function registerSessionRoutes( return [...new Set(sessionIds as string[])]; }; + // Mirrors the bridge's displayName control-character rule (ESLint forbids + // control-char regexes). + const hasControlCharacter = (value: string): boolean => + Array.from(value).some((character) => { + const code = character.charCodeAt(0); + return code <= 31 || code === 127; + }); + + // Tri-state: absent → undefined (skip); invalid → null (400 already sent); + // valid → the PR binding to apply. + const parseSessionPrBody = ( + req: Request, + res: Response, + ): { number: number; url: string } | null | undefined => { + const raw: unknown = safeBody(req)['pr']; + if (raw === undefined) return undefined; + const candidate = raw as Record | null; + const number = candidate?.['number']; + const url = candidate?.['url']; + if ( + candidate === null || + typeof candidate !== 'object' || + typeof number !== 'number' || + !Number.isInteger(number) || + number <= 0 || + typeof url !== 'string' || + url.length > SESSION_PR_URL_MAX_LENGTH || + !/^https?:\/\//i.test(url) || + // The url lands in the bridge's stderr audit line — control + // characters would let a caller forge log lines. + hasControlCharacter(url) + ) { + res.status(400).json({ + error: `\`pr\` must be an object with a positive integer \`number\` and an http(s) \`url\` of at most ${SESSION_PR_URL_MAX_LENGTH} characters, without control characters`, + code: 'invalid_metadata', + field: 'pr', + }); + return null; + } + return { number, url }; + }; + const serializeSessionErrors = ( errors: Array<{ sessionId: string; error: unknown }>, redactDetails = false, @@ -5216,9 +5265,25 @@ export function registerSessionRoutes( app.patch( '/session/:id/metadata', mutate({ strict: true }), + // Gate BEFORE runtime resolution (withOwnerMutableSession): a + // traversal id must be rejected identically on single- and + // multi-workspace daemons — runtime resolution would 404 it first on a + // multi-entry registry, making the error contract + // configuration-dependent. + (req, res, next) => { + const raw = req.params['id']; + if (raw && !isValidSessionId(normalizeSessionIdForLookup(raw))) { + res.status(400).json({ + error: '`sessionId` must be a valid session id', + code: 'invalid_session_id', + }); + return; + } + next(); + }, withOwnerMutableSession( 'PATCH /session/:id/metadata', - (req, res, sessionId, runtime) => { + async (req, res, sessionId, runtime) => { const body = safeBody(req); const clientId = parseClientIdHeader(req, res); if (clientId === null) return; @@ -5234,17 +5299,51 @@ export function registerSessionRoutes( }); return; } + const pr = parseSessionPrBody(req, res); + if (pr === null) return; const displayName = typeof rawDisplayName === 'string' ? rawDisplayName.slice(0, 256) : undefined; let effective: ReturnType; try { + const service = createWorkspaceRuntimeSessionService(runtime); + // Bridge entries are re-created without prs on daemon restart, + // close/reload, and archive/restore. Hydrate the persisted + // binding history before the mutation so the + // `session_metadata_updated` event the bridge publishes carries + // the full list, not just this daemon lifetime's bindings. The + // read is best-effort: readSessionPrs rethrows non-ENOENT I/O + // errors (EISDIR/EACCES/EIO), and an unreadable sidecar must + // degrade the event's history, not block a pr-less rename. + let hydratedPrs: Awaited>; + try { + hydratedPrs = await readSessionPrs( + service.getPrSessionPathForArchiveState(sessionId, 'active'), + ); + } catch { + hydratedPrs = null; + } + if (hydratedPrs && hydratedPrs.length > 0) { + runtime.bridge.seedSessionPrs?.(sessionId, hydratedPrs); + } + // Bridge first: it resolves session liveness, client trust, and + // metadata content. Persisting the sidecar only after it succeeds + // keeps a rejected request from leaving a durable binding behind. effective = runtime.bridge.updateSessionMetadata( sessionId, - { displayName }, + { displayName, ...(pr ? { pr } : {}) }, clientId !== undefined ? { clientId } : undefined, ); + if (pr) { + const persistedPrs = ( + await upsertSessionPr( + service.getPrSessionPathForArchiveState(sessionId, 'active'), + pr, + ) + ).map(({ number, url }) => ({ number, url })); + effective = { ...effective, prs: persistedPrs }; + } } finally { invalidateSessionLists(runtime, ['active']); } @@ -5262,10 +5361,20 @@ export function registerSessionRoutes( if (!runtime) return; const sessionId = requireSessionId(req, res); if (sessionId === null) return; + // The session id is embedded in the sidecar filesystem path; reject + // anything that is not a session id before it can reach the chats + // directory. + if (!isValidSessionId(sessionId)) { + res.status(400).json({ + error: '`sessionId` must be a valid session id', + code: 'invalid_session_id', + }); + return; + } const clientId = parseClientIdHeader(req, res); if (clientId === null) return; const rawDisplayName = safeBody(req)['displayName']; - if (typeof rawDisplayName !== 'string') { + if (rawDisplayName !== undefined && typeof rawDisplayName !== 'string') { res.status(400).json({ error: '`displayName` must be a string', code: 'invalid_metadata', @@ -5273,26 +5382,41 @@ export function registerSessionRoutes( }); return; } + const pr = parseSessionPrBody(req, res); + if (pr === null) return; + if (rawDisplayName === undefined && pr === undefined) { + res.status(400).json({ + error: 'at least one of `displayName` or `pr` is required', + code: 'invalid_metadata', + field: 'displayName', + }); + return; + } try { - const displayName = rawDisplayName.slice(0, 256); - if (displayName.trim() === '') { - // An empty name would append an empty custom_title record to - // persisted sessions, which the title readers disagree on. - throw new InvalidSessionMetadataError( - 'displayName', - 'must not be empty', - ); - } - if ( - Array.from(displayName).some((character) => { - const code = character.charCodeAt(0); - return code <= 31 || code === 127; - }) - ) { - throw new InvalidSessionMetadataError( - 'displayName', - 'must not contain control characters', - ); + const displayName = + typeof rawDisplayName === 'string' + ? rawDisplayName.slice(0, 256) + : undefined; + if (displayName !== undefined) { + if (displayName.trim() === '') { + // An empty name would append an empty custom_title record to + // persisted sessions, which the title readers disagree on. + throw new InvalidSessionMetadataError( + 'displayName', + 'must not be empty', + ); + } + if ( + Array.from(displayName).some((character) => { + const code = character.charCodeAt(0); + return code <= 31 || code === 127; + }) + ) { + throw new InvalidSessionMetadataError( + 'displayName', + 'must not contain control characters', + ); + } } await archiveCoordinator.runExclusiveMany([sessionId], async () => { const assertRuntimeGenerationOpen = @@ -5327,40 +5451,104 @@ export function registerSessionRoutes( return; } await runWithWorkspaceRuntimeStorage(runtime, async () => { - let effective: { displayName?: string }; + let effective: { + displayName?: string; + prs?: Array<{ number: number; url: string }>; + }; + const service = createWorkspaceRuntimeSessionService(runtime); try { + // Bridge entries are re-created without prs on daemon + // restart, close/reload, and archive/restore. Hydrate the + // persisted binding history before the mutation so the + // `session_metadata_updated` event the bridge publishes + // carries the full list, not just this daemon lifetime's + // bindings. The read is best-effort: readSessionPrs rethrows + // non-ENOENT I/O errors (EISDIR/EACCES/EIO), and an + // unreadable sidecar must degrade the event's history, not + // block a pr-less rename. + let hydratedPrs: Awaited>; + try { + hydratedPrs = await readSessionPrs( + service.getPrSessionPathForArchiveState(sessionId, 'active'), + ); + } catch { + hydratedPrs = null; + } + if (hydratedPrs && hydratedPrs.length > 0) { + runtime.bridge.seedSessionPrs?.(sessionId, hydratedPrs); + } + // Bridge first: it resolves client trust and metadata + // content, and reports non-live sessions. Persisting the + // sidecar only after it succeeds keeps a rejected request + // from leaving a durable binding behind — and a live + // session's sidecar always lives in the active chats dir, so + // 'active' is known-correct here. Non-live sessions are + // handled by the fallback below, which persists at the + // located archive state. effective = runtime.bridge.updateSessionMetadata( sessionId, - { displayName }, + { displayName, ...(pr ? { pr } : {}) }, clientId !== undefined ? { clientId } : undefined, ); assertRuntimeGenerationOpen?.(); + if (pr) { + const persistedPrs = ( + await upsertSessionPr( + service.getPrSessionPathForArchiveState( + sessionId, + 'active', + ), + pr, + ) + ).map(({ number, url }) => ({ number, url })); + assertRuntimeGenerationOpen?.(); + effective = { ...effective, prs: persistedPrs }; + } } catch (err) { if (!(err instanceof SessionNotFoundError)) throw err; - const service = createWorkspaceRuntimeSessionService(runtime); const location = await service.getSessionLocation(sessionId); assertRuntimeGenerationOpen?.(); if (location === 'conflict') { throw new SessionConflictError(sessionId); } - const renamed = location - ? await service.renameSession( - sessionId, - displayName, - 'manual', - location, - ) - : false; - assertRuntimeGenerationOpen?.(); - if (!renamed) { + if (!location) { throw new SessionNotFoundError(sessionId); } - // The persisted rename appends a custom_title record the next - // catalog scan serves, so this fallback must advance the same - // catalog revision the live rename marks — otherwise - // version-watching clients keep the stale name. + effective = {}; + // Persist the PR sidecar BEFORE the rename: the catalog bump + // below only runs when every write succeeds, so the write most + // likely to fail (the newer sidecar path) must run first — a + // failed sidecar write may not strand an already-persisted + // rename that the error response never announces. + if (pr) { + const persisted = await upsertSessionPr( + service.getPrSessionPathForArchiveState(sessionId, location), + pr, + ); + assertRuntimeGenerationOpen?.(); + effective.prs = persisted.map(({ number, url }) => ({ + number, + url, + })); + } + if (displayName !== undefined) { + const renamed = await service.renameSession( + sessionId, + displayName, + 'manual', + location, + ); + assertRuntimeGenerationOpen?.(); + if (!renamed) { + throw new SessionNotFoundError(sessionId); + } + effective.displayName = displayName; + } + // The persisted mutation is picked up by the next catalog + // scan, so this fallback must advance the same catalog + // revision the live update marks — otherwise version-watching + // clients keep the stale metadata. runtime.bridge.markSessionCatalogChanged(); - effective = { displayName: displayName || undefined }; } invalidateSessionLists(runtime, ['active', 'archived']); res.status(200).json({ sessionId, ...effective }); diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 0aa05c9261f..0029c91881c 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -84,6 +84,8 @@ import { SessionService, Storage, TrustGateError, + readSessionPrs, + upsertSessionPr, type Extension, type CommittedExtensionMutation, type PrepareExtensionInstallOptions, @@ -1305,6 +1307,10 @@ interface FakeBridge extends AcpSessionBridge { metadata: SessionMetadataUpdate; context?: BridgeClientRequestContext; }>; + seedSessionPrsCalls: Array<{ + sessionId: string; + prs: Array<{ number: number; url: string }>; + }>; heartbeatCalls: Array<{ sessionId: string; context?: BridgeClientRequestContext; @@ -1410,6 +1416,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { let workspaceMemoryDreamCalls = 0; const closeCalls: FakeBridge['closeCalls'] = []; const updateMetadataCalls: FakeBridge['updateMetadataCalls'] = []; + const seedSessionPrsCalls: FakeBridge['seedSessionPrsCalls'] = []; const heartbeatCalls: FakeBridge['heartbeatCalls'] = []; const heartbeatStateCalls: string[] = []; let shutdownCalls = 0; @@ -1908,6 +1915,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { opts.updateMetadataImpl ?? ((_sid: string, m: SessionMetadataUpdate) => ({ displayName: m.displayName, + ...(m.pr ? { prs: [m.pr] } : {}), })); const heartbeatImpl = opts.heartbeatImpl ?? @@ -2029,6 +2037,7 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { removeRuntimeMcpServerCalls, closeCalls, updateMetadataCalls, + seedSessionPrsCalls, heartbeatCalls, heartbeatStateCalls, get shutdownCalls() { @@ -2535,6 +2544,9 @@ function fakeBridge(opts: FakeBridgeOpts = {}): FakeBridge { }); return updateMetadataImpl(sessionId, metadata, context); }, + seedSessionPrs(sessionId, prs) { + seedSessionPrsCalls.push({ sessionId, prs }); + }, recordHeartbeat(sessionId, context) { heartbeatCalls.push({ sessionId, @@ -15455,6 +15467,175 @@ describe('createServeApp', () => { expect(res.body.sessions[0].sessionId).toBe(id); }); + it('merges sidecar pr history with the live entry bindings on list', async () => { + // The live entry only knows bindings from this daemon lifetime; the + // sidecar holds the full history. Binding A pre-restart, restarting + // (live entry resets), then binding B must still list [A, B] — the + // stacked-PR-across-restart case. + const id = '550e8400-e29b-41d4-a716-446655440003'; + await writeStoredSession({ + sessionId: id, + cwd: WS_BOUND, + timestamp: '2026-05-17T12:00:00.000Z', + prompt: 'stored prompt', + mtime: new Date('2026-05-17T12:00:05.000Z'), + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState(id, 'active'); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9500, + url: 'https://github.com/o/r/pull/9500', + }); + const bridge = fakeBridge({ + listImpl: () => [ + { + sessionId: id, + workspaceCwd: WS_BOUND, + createdAt: '2026-05-17T12:00:00.000Z', + clientCount: 1, + hasActivePrompt: false, + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }, + ], + }); + + const result = await listWorkspaceSessionsForResponse(bridge, WS_BOUND); + + const merged = result.sessions.find((s) => s.sessionId === id); + expect(merged?.prs?.map((p) => p.number)).toEqual([9500, 9517]); + }); + + it('dedupes by number on merge, preferring the live url', async () => { + // Overlap is the common production case (a route binding is persisted + // AND enters the live entry). Without the number-keyed filter the + // merged list duplicates the number and the badge renders `#9517 +1` + // for a one-PR session. + const id = '550e8400-e29b-41d4-a716-446655440004'; + await writeStoredSession({ + sessionId: id, + cwd: WS_BOUND, + timestamp: '2026-05-17T12:00:00.000Z', + prompt: 'stored prompt', + mtime: new Date('2026-05-17T12:00:05.000Z'), + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState(id, 'active'); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9517, + url: 'https://github.com/o/r/pull/9517', + }); + const bridge = fakeBridge({ + listImpl: () => [ + { + sessionId: id, + workspaceCwd: WS_BOUND, + createdAt: '2026-05-17T12:00:00.000Z', + clientCount: 1, + hasActivePrompt: false, + prs: [ + { number: 9517, url: 'https://github.com/o/r/pull/9517?v=2' }, + ], + }, + ], + }); + + const result = await listWorkspaceSessionsForResponse(bridge, WS_BOUND); + + const merged = result.sessions.find((s) => s.sessionId === id); + expect(merged?.prs).toEqual([ + { number: 9517, url: 'https://github.com/o/r/pull/9517?v=2' }, + ]); + }); + + it('survives PR sidecars on the organized listing path', async () => { + const id = '550e8400-e29b-41d4-a716-446655440005'; + await writeStoredSession({ + sessionId: id, + cwd: WS_BOUND, + timestamp: '2026-05-17T12:00:00.000Z', + prompt: 'stored prompt', + mtime: new Date('2026-05-17T12:00:05.000Z'), + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState(id, 'active'); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9500, + url: 'https://github.com/o/r/pull/9500', + }); + + const result = await listWorkspaceSessionsForResponse( + fakeBridge(), + WS_BOUND, + { view: 'organized', group: 'all' }, + ); + + const listed = result.sessions.find((s) => s.sessionId === id); + expect(listed?.prs?.map((p) => p.number)).toEqual([9500]); + }); + + it('survives PR sidecars on the archived listing path', async () => { + const id = '550e8400-e29b-41d4-a716-446655440006'; + await writeStoredSession({ + sessionId: id, + cwd: WS_BOUND, + timestamp: '2026-05-17T12:00:00.000Z', + prompt: 'stored prompt', + mtime: new Date('2026-05-17T12:00:05.000Z'), + state: 'archived', + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + id, + 'archived', + ); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9500, + url: 'https://github.com/o/r/pull/9500', + }); + + const result = await listWorkspaceSessionsForResponse( + fakeBridge(), + WS_BOUND, + { archiveState: 'archived' }, + ); + + const listed = result.sessions.find((s) => s.sessionId === id); + expect(listed?.prs?.map((p) => p.number)).toEqual([9500]); + }); + + it('survives PR sidecars on the metadata-filtered listing path', async () => { + const id = '550e8400-e29b-41d4-a716-446655440007'; + await writeStoredSession({ + sessionId: id, + cwd: WS_BOUND, + timestamp: '2026-05-17T12:00:00.000Z', + prompt: 'stored prompt', + mtime: new Date('2026-05-17T12:00:05.000Z'), + sourceType: 'scheduled_task', + sourceId: 'task-1', + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState(id, 'active'); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9500, + url: 'https://github.com/o/r/pull/9500', + }); + + const result = await listWorkspaceSessionsForResponse( + fakeBridge(), + WS_BOUND, + { sourceType: 'scheduled_task', sourceId: 'task-1' }, + ); + + const listed = result.sessions.find((s) => s.sessionId === id); + expect(listed?.prs?.map((p) => p.number)).toEqual([9500]); + }); + it('passes fractional cursor values to SessionService without truncating', async () => { const listSessionsSpy = vi .spyOn(SessionService.prototype, 'listSessions') @@ -25736,15 +25917,19 @@ describe('createServeApp', () => { const bridge = fakeBridge(); const app = createServeApp(tokenOpts, undefined, { bridge }); const res = await auth( - request(app).patch('/session/session-A/metadata'), + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), ).send({ displayName: 'My Session' }); expect(res.status).toBe(200); expect(res.body).toEqual({ - sessionId: 'session-A', + sessionId: '550e8400-e29b-41d4-a716-446655440321', displayName: 'My Session', }); expect(bridge.updateMetadataCalls).toHaveLength(1); - expect(bridge.updateMetadataCalls[0]?.sessionId).toBe('session-A'); + expect(bridge.updateMetadataCalls[0]?.sessionId).toBe( + '550e8400-e29b-41d4-a716-446655440321', + ); expect(bridge.updateMetadataCalls[0]?.metadata).toEqual({ displayName: 'My Session', }); @@ -25755,7 +25940,7 @@ describe('createServeApp', () => { const noTokenApp = createServeApp(baseOpts, undefined, { bridge }); const noToken = await request(noTokenApp) - .patch('/session/session-A/metadata') + .patch('/session/550e8400-e29b-41d4-a716-446655440321/metadata') .set('Host', `127.0.0.1:${baseOpts.port}`) .send({ displayName: 'blocked' }); expect(noToken.status).toBe(401); @@ -25764,7 +25949,9 @@ describe('createServeApp', () => { const app = createServeApp(tokenOpts, undefined, { bridge }); const authed = await auth( - request(app).patch('/session/session-A/metadata'), + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), ).send({ displayName: 'allowed' }); expect(authed.status).toBe(200); expect(bridge.updateMetadataCalls).toHaveLength(1); @@ -25776,7 +25963,11 @@ describe('createServeApp', () => { it('passes client identity context', async () => { const bridge = fakeBridge(); const app = createServeApp(tokenOpts, undefined, { bridge }); - const res = await auth(request(app).patch('/session/session-A/metadata')) + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ) .set('X-Qwen-Client-Id', 'client-1') .send({ displayName: 'test' }); expect(res.status).toBe(200); @@ -25789,13 +25980,485 @@ describe('createServeApp', () => { const bridge = fakeBridge(); const app = createServeApp(tokenOpts, undefined, { bridge }); const res = await auth( - request(app).patch('/session/session-A/metadata'), + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), ).send({ displayName: 123 }); expect(res.status).toBe(400); expect(res.body.code).toBe('invalid_metadata'); expect(res.body.field).toBe('displayName'); }); + it('200 on pr-only update and echoes the pr binding', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr }); + expect(res.status).toBe(200); + expect(res.body.sessionId).toBe('550e8400-e29b-41d4-a716-446655440321'); + expect(res.body.prs).toEqual([pr]); + expect(bridge.updateMetadataCalls).toHaveLength(1); + expect(bridge.updateMetadataCalls[0]?.metadata).toEqual({ + displayName: undefined, + pr, + }); + // The echoed list reflects the sidecar actually written to disk. + expect( + (await readSessionPrs(sidecarPath))?.map(({ number, url }) => ({ + number, + url, + })), + ).toEqual([pr]); + await fsp.rm(sidecarPath, { force: true }); + }); + + it('echoes the sidecar list, not the bridge echo, when they disagree on the primary route', async () => { + // Without the persisted-readback overwrite the primary route would + // echo the bridge's live list; without the sidecar hydration before + // the bridge call the published event would drop persisted history. + const bridge = fakeBridge({ + updateMetadataImpl: (_sid, m) => ({ + displayName: m.displayName, + ...(m.pr + ? { + prs: [ + { + number: 9001, + url: 'https://github.com/o/r/pull/9001', + }, + ], + } + : {}), + }), + }); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9000, + url: 'https://github.com/o/r/pull/9000', + }); + try { + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ + pr: { number: 9002, url: 'https://github.com/o/r/pull/9002' }, + }); + expect(res.status).toBe(200); + expect(res.body.prs.map((p: { number: number }) => p.number)).toEqual([ + 9000, 9002, + ]); + expect( + bridge.seedSessionPrsCalls.map((call) => + call.prs.map((p) => p.number), + ), + ).toEqual([[9000]]); + } finally { + await fsp.rm(sidecarPath, { force: true }); + } + }); + + it('400 invalid_metadata for a malformed pr', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { bridge }); + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr: { number: 'x', url: 'https://github.com/o/r/pull/1' } }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_metadata'); + expect(res.body.field).toBe('pr'); + const nonHttp = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr: { number: 1, url: 'javascript:alert(1)' } }); + expect(nonHttp.status).toBe(400); + expect(nonHttp.body.code).toBe('invalid_metadata'); + expect(nonHttp.body.field).toBe('pr'); + expect(bridge.updateMetadataCalls).toHaveLength(0); + }); + + it('400 invalid_metadata when the pr url exceeds the length cap', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ + pr: { + number: 9517, + url: `https://github.com/o/r/pull/${'a'.repeat(2048)}`, + }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_metadata'); + expect(res.body.field).toBe('pr'); + expect(bridge.updateMetadataCalls).toHaveLength(0); + expect(await readSessionPrs(sidecarPath)).toBeNull(); + }); + + it('does not persist the pr sidecar when the bridge rejects a combined request', async () => { + const bridge = fakeBridge({ + updateMetadataImpl: () => { + throw new InvalidSessionMetadataError( + 'displayName', + 'must not contain control characters', + ); + }, + }); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ + displayName: 'bad\u0001name', + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_metadata'); + // A rejected request must not leave a durable binding behind. + expect(await readSessionPrs(sidecarPath)).toBeNull(); + }); + + it('does not persist the pr sidecar for a session the bridge does not know', async () => { + const bridge = fakeBridge({ + updateMetadataImpl: (sessionId) => { + throw new SessionNotFoundError(sessionId); + }, + }); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440999', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + const res = await auth( + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440999/metadata', + ), + ).send({ pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' } }); + expect(res.status).toBe(404); + expect(await readSessionPrs(sidecarPath)).toBeNull(); + }); + + it('400 invalid_session_id and no escaped write for a traversal session id', async () => { + const bridge = fakeBridge(); + const app = createServeApp(tokenOpts, undefined, { + bridge, + boundWorkspace: WS_BOUND, + }); + const service = new SessionService(WS_BOUND); + const escapedPath = service.getPrSessionPathForArchiveState( + '../../pwn', + 'active', + ); + await fsp.rm(escapedPath, { force: true }); + const res = await auth( + request(app).patch('/session/..%2F..%2Fpwn/metadata'), + ).send({ pr: { number: 1, url: 'https://evil.example/x' } }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_session_id'); + expect(bridge.updateMetadataCalls).toHaveLength(0); + await expect(fsp.access(escapedPath)).rejects.toThrow(); + }); + + it('400 invalid_session_id for a traversal id on a multi-workspace registry', async () => { + // The gate sits before runtime resolution, so the answer is 400 even + // when the registry has two entries (runtime resolution would + // otherwise 404 the unknown id first, making the error contract + // configuration-dependent). + const secondaryBridge = fakeBridge(); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const res = await auth( + request(app).patch('/session/..%2F..%2Fpwn/metadata'), + ).send({ pr: { number: 1, url: 'https://evil.example/x' } }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_session_id'); + expect(secondaryBridge.updateMetadataCalls).toHaveLength(0); + }); + + it('200 on pr-only update on the workspace route', async () => { + const secondaryBridge = fakeBridge(); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const service = new SessionService(WS_DIFFERENT); + await fsp.rm( + service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ), + { + force: true, + }, + ); + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr }); + expect(res.status).toBe(200); + expect(res.body.prs).toEqual([pr]); + expect(secondaryBridge.updateMetadataCalls).toEqual([ + { + sessionId: '550e8400-e29b-41d4-a716-446655440321', + metadata: { displayName: undefined, pr }, + context: undefined, + }, + ]); + const sidecar = await readSessionPrs( + service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ), + ); + expect(sidecar?.map((p) => p.number)).toEqual([9517]); + }); + + it('accumulates multiple pr bindings on the workspace route', async () => { + const secondaryBridge = fakeBridge(); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const service = new SessionService(WS_DIFFERENT); + await fsp.rm( + service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ), + { + force: true, + }, + ); + for (const number of [9600, 9601]) { + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ + pr: { number, url: `https://github.com/o/r/pull/${number}` }, + }); + expect(res.status).toBe(200); + } + const last = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr: { number: 9602, url: 'https://github.com/o/r/pull/9602' } }); + // The sidecar persists the full history, so the response echoes every + // binding in binding order. + expect(last.body.prs.map((p: { number: number }) => p.number)).toEqual([ + 9600, 9601, 9602, + ]); + }); + + it('echoes the sidecar list, not the bridge echo, when they disagree', async () => { + const secondaryBridge = fakeBridge({ + updateMetadataImpl: (_sid, m) => ({ + displayName: m.displayName, + ...(m.pr + ? { + prs: [ + { + number: 9001, + url: 'https://github.com/o/r/pull/9001', + }, + ], + } + : {}), + }), + }); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const service = new SessionService(WS_DIFFERENT); + const sidecarPath = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + await fsp.rm(sidecarPath, { force: true }); + await upsertSessionPr(sidecarPath, { + number: 9000, + url: 'https://github.com/o/r/pull/9000', + }); + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr: { number: 9002, url: 'https://github.com/o/r/pull/9002' } }); + expect(res.status).toBe(200); + // The bridge (this-daemon memory) only knows 9001; the sidecar is the + // complete history and wins the echo. + expect(res.body.prs.map((p: { number: number }) => p.number)).toEqual([ + 9000, 9002, + ]); + // The route re-hydrates the bridge entry from the sidecar BEFORE the + // mutation, so the session_metadata_updated event the bridge publishes + // carries the full history too, not just this daemon lifetime's share. + expect( + secondaryBridge.seedSessionPrsCalls.map((call) => + call.prs.map((p) => p.number), + ), + ).toEqual([[9000]]); + }); + + it('binds an archived session at the archived sidecar without orphaning an active one', async () => { + const secondaryBridge = fakeBridge({ + updateMetadataImpl: (sessionId) => { + throw new SessionNotFoundError(sessionId); + }, + }); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const service = new SessionService(WS_DIFFERENT); + const activeSidecar = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ); + const archivedSidecar = service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'archived', + ); + await fsp.rm(activeSidecar, { force: true }); + await fsp.rm(archivedSidecar, { force: true }); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockResolvedValue('archived'); + try { + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr }); + expect(res.status).toBe(200); + expect(res.body.prs).toEqual([pr]); + // The binding lands at the located (archived) state only — no + // orphan in the active chats dir that an unarchive would later + // conflict with. + const archived = await readSessionPrs(archivedSidecar); + expect(archived?.map((p) => p.number)).toEqual([9517]); + expect(await readSessionPrs(activeSidecar)).toBeNull(); + } finally { + locationSpy.mockRestore(); + await fsp.rm(archivedSidecar, { force: true }); + } + }); + + it('400 invalid_session_id and no escaped write for a traversal id on the workspace route', async () => { + const secondaryBridge = fakeBridge(); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const service = new SessionService(WS_DIFFERENT); + const escapedPath = service.getPrSessionPathForArchiveState( + '../../pwn', + 'active', + ); + await fsp.rm(escapedPath, { force: true }); + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/..%2F..%2Fpwn/metadata', + ), + ).send({ pr: { number: 1, url: 'https://evil.example/x' } }); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_session_id'); + expect(secondaryBridge.updateMetadataCalls).toHaveLength(0); + await expect(fsp.access(escapedPath)).rejects.toThrow(); + }); + + it('400 when neither displayName nor pr is provided on the workspace route', async () => { + const secondaryBridge = fakeBridge(); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({}); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_metadata'); + expect(secondaryBridge.updateMetadataCalls).toHaveLength(0); + }); + + it('200 on pr-only update for a persisted (non-live) session', async () => { + const secondaryBridge = fakeBridge({ + updateMetadataImpl: (sessionId) => { + throw new SessionNotFoundError(sessionId); + }, + }); + const { app } = createWorkspaceMetadataApp(secondaryBridge); + const locationSpy = vi + .spyOn(SessionService.prototype, 'getSessionLocation') + .mockResolvedValue('active'); + try { + const pr = { number: 9517, url: 'https://github.com/o/r/pull/9517' }; + const service = new SessionService(WS_DIFFERENT); + await fsp.rm( + service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ), + { force: true }, + ); + const res = await auth( + request(app).patch( + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), + ).send({ pr }); + expect(res.status).toBe(200); + expect(res.body.prs).toEqual([pr]); + const sidecar = await readSessionPrs( + service.getPrSessionPathForArchiveState( + '550e8400-e29b-41d4-a716-446655440321', + 'active', + ), + ); + expect(sidecar?.map((p) => p.number)).toEqual([9517]); + } finally { + locationSpy.mockRestore(); + } + }); + it('404 on unknown session', async () => { const bridge = fakeBridge({ updateMetadataImpl: (sessionId) => { @@ -25804,10 +26467,12 @@ describe('createServeApp', () => { }); const app = createServeApp(tokenOpts, undefined, { bridge }); const res = await auth( - request(app).patch('/session/missing/metadata'), + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440999/metadata', + ), ).send({ displayName: 'test' }); expect(res.status).toBe(404); - expect(res.body.sessionId).toBe('missing'); + expect(res.body.sessionId).toBe('550e8400-e29b-41d4-a716-446655440999'); }); it('400 invalid_metadata when displayName exceeds max length', async () => { @@ -25821,7 +26486,9 @@ describe('createServeApp', () => { }); const app = createServeApp(tokenOpts, undefined, { bridge }); const res = await auth( - request(app).patch('/session/session-A/metadata'), + request(app).patch( + '/session/550e8400-e29b-41d4-a716-446655440321/metadata', + ), ).send({ displayName: 'x'.repeat(300) }); expect(res.status).toBe(400); expect(res.body.code).toBe('invalid_metadata'); @@ -25833,7 +26500,7 @@ describe('createServeApp', () => { createWorkspaceMetadataApp(secondaryBridge); const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ) .set('X-Qwen-Client-Id', 'client-1') @@ -25842,7 +26509,7 @@ describe('createServeApp', () => { expect(res.status).toBe(200); expect(secondaryBridge.updateMetadataCalls).toEqual([ { - sessionId: 'session-A', + sessionId: '550e8400-e29b-41d4-a716-446655440321', metadata: { displayName: 'Secondary session' }, context: { clientId: 'client-1' }, }, @@ -25859,7 +26526,7 @@ describe('createServeApp', () => { const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ).send({ displayName: 'Blocked' }); @@ -25882,7 +26549,7 @@ describe('createServeApp', () => { const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ).send({ displayName: 'Blocked' }); @@ -25903,7 +26570,7 @@ describe('createServeApp', () => { const { app } = createWorkspaceMetadataApp(secondaryBridge); const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ).send(body); @@ -25918,18 +26585,18 @@ describe('createServeApp', () => { const { app } = createWorkspaceMetadataApp(secondaryBridge); const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ).send({ displayName: 'x'.repeat(300) }); expect(res.status).toBe(200); expect(res.body).toEqual({ - sessionId: 'session-A', + sessionId: '550e8400-e29b-41d4-a716-446655440321', displayName: 'x'.repeat(256), }); expect(secondaryBridge.updateMetadataCalls).toEqual([ { - sessionId: 'session-A', + sessionId: '550e8400-e29b-41d4-a716-446655440321', metadata: { displayName: 'x'.repeat(256) }, }, ]); @@ -25942,7 +26609,7 @@ describe('createServeApp', () => { }); const res = await auth( request(app).patch( - '/workspaces/ws-secondary/session/session-A/metadata', + '/workspaces/ws-secondary/session/550e8400-e29b-41d4-a716-446655440321/metadata', ), ).send({ displayName: 'Blocked' }); @@ -26014,6 +26681,99 @@ describe('createServeApp', () => { }, ); + it('applies no durable rename when the pr sidecar write fails in the non-live fallback', async () => { + // A combined displayName+pr PATCH on a non-live session persists two + // writes sequentially and advances the catalog revision only after + // BOTH succeed. The sidecar write must run first: when it fails, the + // client receives a total-failure response, and nothing durable may + // be left behind unannounced. + const runtimeBaseDir = await fsp.mkdtemp( + path.join(os.tmpdir(), 'qwen-workspace-metadata-prfail-'), + ); + const sessionId = '550e8400-e29b-41d4-a716-446655440035'; + const chatsDir = path.join( + new Storage(WS_DIFFERENT, runtimeBaseDir).getProjectDir(), + 'chats', + ); + const filePath = path.join(chatsDir, `${sessionId}.jsonl`); + await fsp.mkdir(chatsDir, { recursive: true }); + await fsp.writeFile( + filePath, + `${JSON.stringify({ + uuid: 'record-1', + parentUuid: null, + sessionId, + timestamp: '2026-05-17T12:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'original' }] }, + cwd: WS_DIFFERENT, + })}\n`, + 'utf8', + ); + const secondaryBridge = fakeBridge({ + updateMetadataImpl: () => { + throw new SessionNotFoundError(sessionId); + }, + }); + const { app } = createWorkspaceMetadataApp(secondaryBridge, { + sessionRuntimeBaseDir: runtimeBaseDir, + }); + const service = new SessionService(WS_DIFFERENT, { + runtimeBaseDir, + }); + const sidecarPath = service.getPrSessionPathForArchiveState( + sessionId, + 'active', + ); + // Force the sidecar write to fail: a directory squatting the file + // path makes both the read and the write fail with EISDIR. + await fsp.mkdir(sidecarPath, { recursive: true }); + + try { + const versionBefore = secondaryBridge.getSessionCatalogVersion(); + const res = await auth( + request(app).patch( + `/workspaces/ws-secondary/session/${sessionId}/metadata`, + ), + ).send({ + displayName: 'Doomed rename', + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(res.status).toBe(500); + // The failed sidecar write must not strand a durable rename that + // the 500 response never announces. + expect(await fsp.readFile(filePath, 'utf8')).not.toContain( + 'Doomed rename', + ); + expect(secondaryBridge.getSessionCatalogVersion().revision).toBe( + versionBefore.revision, + ); + + // CONTROL: once the sidecar path is writable, the same combined + // request applies both mutations and advances the catalog revision. + await fsp.rm(sidecarPath, { recursive: true, force: true }); + const retry = await auth( + request(app).patch( + `/workspaces/ws-secondary/session/${sessionId}/metadata`, + ), + ).send({ + displayName: 'Doomed rename', + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(retry.status).toBe(200); + expect(retry.body.displayName).toBe('Doomed rename'); + expect(retry.body.prs).toEqual([ + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ]); + expect(await fsp.readFile(filePath, 'utf8')).toContain('Doomed rename'); + expect( + secondaryBridge.getSessionCatalogVersion().revision, + ).toBeGreaterThan(versionBefore.revision); + } finally { + await fsp.rm(runtimeBaseDir, { recursive: true, force: true }); + } + }); + it('returns 404 for a missing persisted session and 409 for a store conflict', async () => { const runtimeBaseDir = await fsp.mkdtemp( path.join(os.tmpdir(), 'qwen-workspace-metadata-conflict-'), diff --git a/packages/cli/src/serve/server/session-list.ts b/packages/cli/src/serve/server/session-list.ts index 83fce6fef08..5b7f1609879 100644 --- a/packages/cli/src/serve/server/session-list.ts +++ b/packages/cli/src/serve/server/session-list.ts @@ -10,6 +10,7 @@ import { SessionOrganizationError, Storage, readWorktreeSession, + readSessionPrs, type SessionArchiveState, type SessionGroupPresetColor, } from '@qwen-code/qwen-code-core'; @@ -399,6 +400,46 @@ async function enrichWorktreeSidecars( } } +/** + * Enrich persisted session summaries with GitHub PR bindings from sidecar + * files so the binding survives daemon restarts. Same pattern as + * {@link enrichWorktreeSidecars}. Runs before the live merge, when the map + * only holds persisted summaries — {@link mergeLiveSessionSummary} merges + * these with the live entry's daemon-lifetime bindings. + */ +async function enrichPrSidecars( + bySessionId: Map, + sessionService: SessionService, + // Required (no default): silently defaulting to 'active' would let a + // future archived-listing call site that omits the argument enrich from + // the wrong chats dir and drop every binding. + archiveState: SessionArchiveState, + signal?: AbortSignal, +): Promise { + for (const [sessionId, summary] of bySessionId) { + signal?.throwIfAborted(); + let sidecar: Awaited>; + try { + const sidecarPath = sessionService.getPrSessionPathForArchiveState( + sessionId, + archiveState, + ); + sidecar = signal + ? await readSessionPrs(sidecarPath, { signal }) + : await readSessionPrs(sidecarPath); + } catch { + signal?.throwIfAborted(); + sidecar = null; + } + if (sidecar) { + bySessionId.set(sessionId, { + ...summary, + prs: sidecar.map(({ number, url }) => ({ number, url })), + }); + } + } +} + function toSummary(item: { sessionId: string; cwd: string; @@ -439,7 +480,7 @@ function mergeLiveSessionSummary( existing: BridgeSessionSummary, live: BridgeSessionSummary, ): BridgeSessionSummary { - return { + const merged: BridgeSessionSummary = { ...existing, ...live, createdAt: existing.createdAt, @@ -455,6 +496,20 @@ function mergeLiveSessionSummary( hasActivePrompt: live.hasActivePrompt, isArchived: false, }; + // The live entry only knows PR bindings from this daemon lifetime while the + // sidecar-enriched persisted summary holds the full history — merge by PR + // number (live url wins, live-only bindings sort latest) instead of letting + // the spread overwrite the history. + if (existing.prs || live.prs) { + const livePrs = live.prs ?? []; + merged.prs = [ + ...(existing.prs ?? []).filter( + (p) => !livePrs.some((l) => l.number === p.number), + ), + ...livePrs, + ]; + } + return merged; } function clonePersistedSummary( @@ -514,6 +569,7 @@ async function loadAllPersistedSummaries( archiveState, signal, ); + await enrichPrSidecars(bySessionId, sessionService, archiveState, signal); signal.throwIfAborted(); return { sessions: [...bySessionId.values()], @@ -1184,6 +1240,12 @@ async function listWorkspaceSessionsForResponseInRuntime( archiveState, readOptions.signal, ); + await enrichPrSidecars( + bySessionId, + sessionService, + archiveState, + readOptions.signal, + ); readOptions.signal?.throwIfAborted(); if (archiveState === 'archived' || readOptions.mergeLive === false) { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 224bb10d9b9..8cf2dfa204c 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -6343,6 +6343,7 @@ describe('Server Config (config.ts)', () => { oldChatsDir, `${sessionId}.worktree.json`, ); + const oldPrSessionPath = path.join(oldChatsDir, `${sessionId}.pr.json`); const newTranscriptPath = path.join(newChatsDir, `${sessionId}.jsonl`); const newRuntimeStatusPath = path.join( newChatsDir, @@ -6352,6 +6353,7 @@ describe('Server Config (config.ts)', () => { newChatsDir, `${sessionId}.worktree.json`, ); + const newPrSessionPath = path.join(newChatsDir, `${sessionId}.pr.json`); const chdirSpy = vi.spyOn(process, 'chdir').mockImplementation(() => { // Keep the test process in its original directory. }); @@ -6360,6 +6362,7 @@ describe('Server Config (config.ts)', () => { oldTranscriptPath, oldRuntimeStatusPath, oldWorktreeSessionPath, + oldPrSessionPath, ]; vi.mocked(fs.existsSync).mockImplementation((pathToCheck) => { const checked = pathToCheck.toString(); @@ -6383,6 +6386,10 @@ describe('Server Config (config.ts)', () => { oldWorktreeSessionPath, newWorktreeSessionPath, ); + expect(fs.renameSync).toHaveBeenCalledWith( + oldPrSessionPath, + newPrSessionPath, + ); expect(config.getTranscriptPath()).toBe(newTranscriptPath); chdirSpy.mockRestore(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 10f144734ac..301eb4d6685 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -5068,6 +5068,7 @@ export class Config { `${this.sessionId}.jsonl`, `${this.sessionId}.runtime.json`, `${this.sessionId}.worktree.json`, + `${this.sessionId}.pr.json`, ].map((fileName) => ({ from: path.join(oldChatsDir, fileName), to: path.join(newChatsDir, fileName), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cf035912f98..2787c2fb78c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -375,6 +375,7 @@ export type { TokenUsageTotals, } from './services/tokenUsageService.js'; export * from './services/worktreeSessionService.js'; +export * from './services/session-pr-service.js'; export { stripTerminalControlSequences, stripDisplayControlChars, diff --git a/packages/core/src/services/session-pr-service.test.ts b/packages/core/src/services/session-pr-service.test.ts new file mode 100644 index 00000000000..828cfbc508b --- /dev/null +++ b/packages/core/src/services/session-pr-service.test.ts @@ -0,0 +1,232 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { + SESSION_PR_LIST_LIMIT, + mergeSessionPrLists, + readSessionPrs, + upsertSessionPr, + writeSessionPrs, + type SessionPr, +} from './session-pr-service.js'; + +const entry = (number: number): SessionPr => ({ + number, + url: `https://github.com/owner/repo/pull/${number}`, + createdAt: '2026-08-20T00:00:00.000Z', +}); + +let tmpDir: string; +let filePath: string; + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'session-pr-test-')); + filePath = path.join(tmpDir, 'test.pr.json'); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +describe('writeSessionPrs / readSessionPrs', () => { + it('round-trips a PR list', async () => { + const prs = [entry(9517), entry(9519)]; + await writeSessionPrs(filePath, prs); + expect(await readSessionPrs(filePath)).toEqual(prs); + }); + + it('creates missing parent directories on write', async () => { + const nested = path.join(tmpDir, 'a', 'b', 'test.pr.json'); + await writeSessionPrs(nested, [entry(1)]); + expect(await readSessionPrs(nested)).toEqual([entry(1)]); + }); +}); + +describe('readSessionPrs', () => { + it('returns null when the file does not exist', async () => { + expect(await readSessionPrs(filePath)).toBeNull(); + }); + + it('returns null for invalid JSON', async () => { + await fs.writeFile(filePath, '{not json', 'utf-8'); + expect(await readSessionPrs(filePath)).toBeNull(); + }); + + it.each([ + ['bare object (legacy single shape)', entry(1)], + ['empty list', { prs: [] }], + ['entry missing url', { prs: [{ number: 1, createdAt: 'x' }] }], + ['entry non-integer number', { prs: [{ ...entry(1), number: 1.5 }] }], + ['entry non-positive number', { prs: [entry(0)] }], + [ + 'entry non-http url', + { prs: [{ ...entry(1), url: 'javascript:alert(1)' }] }, + ], + [ + 'entry url with a control character', + { prs: [{ ...entry(1), url: 'https://github.com/o/r/pull/1\nforged' }] }, + ], + [ + 'entry url over 2048 characters', + { prs: [{ ...entry(1), url: `https://github.com/${'a'.repeat(2048)}` }] }, + ], + ['entry missing createdAt', { prs: [{ number: 1, url: entry(1).url }] }], + ])('returns null for a malformed sidecar: %s', async (_label, value) => { + await fs.writeFile(filePath, JSON.stringify(value), 'utf-8'); + expect(await readSessionPrs(filePath)).toBeNull(); + }); + + it('propagates the caller abort reason', async () => { + const controller = new AbortController(); + const reason = new Error('pr sidecar read cancelled'); + controller.abort(reason); + + await expect( + readSessionPrs(filePath, { signal: controller.signal }), + ).rejects.toBe(reason); + }); +}); + +describe('upsertSessionPr', () => { + it('appends bindings in binding order', async () => { + await upsertSessionPr(filePath, { number: 100, url: entry(100).url }); + const prs = await upsertSessionPr(filePath, { + number: 101, + url: entry(101).url, + }); + expect(prs.map((p) => p.number)).toEqual([100, 101]); + }); + + it('re-binding the same number refreshes it and moves it to latest', async () => { + await upsertSessionPr(filePath, { number: 100, url: entry(100).url }); + await upsertSessionPr(filePath, { number: 101, url: entry(101).url }); + const prs = await upsertSessionPr(filePath, { + number: 100, + url: 'https://github.com/owner/repo/pull/100?updated=1', + }); + expect(prs.map((p) => p.number)).toEqual([101, 100]); + expect(prs[1]?.url).toContain('updated=1'); + }); + + it('caps the list at SESSION_PR_LIST_LIMIT, dropping the oldest', async () => { + for (let i = 1; i <= SESSION_PR_LIST_LIMIT + 2; i++) { + await upsertSessionPr(filePath, { + number: i, + url: `https://github.com/owner/repo/pull/${i}`, + }); + } + const prs = await readSessionPrs(filePath); + expect(prs).toHaveLength(SESSION_PR_LIST_LIMIT); + expect(prs?.[0]?.number).toBe(3); + expect(prs?.[SESSION_PR_LIST_LIMIT - 1]?.number).toBe( + SESSION_PR_LIST_LIMIT + 2, + ); + }); + + it('serializes concurrent upserts so no binding is dropped', async () => { + // Without the per-path queue, interleaved read-modify-write cycles would + // let a later writer overwrite an earlier binding (read [] → read [] → + // write [A] → write [B]). + await Promise.all([ + upsertSessionPr(filePath, { number: 100, url: entry(100).url }), + upsertSessionPr(filePath, { number: 101, url: entry(101).url }), + upsertSessionPr(filePath, { number: 102, url: entry(102).url }), + ]); + const prs = await readSessionPrs(filePath); + expect(prs?.map((p) => p.number)).toEqual([100, 101, 102]); + }); +}); + +describe('upsertSessionPr failure handling', () => { + it('surfaces the failure to the caller without leaking an unhandled rejection', async () => { + // The queue cleanup chain derives from the upsert promise; a derived + // finally/catch would reject unhandled on every sidecar I/O failure even + // though callers await the returned promise. + const unhandled: unknown[] = []; + const onUnhandledRejection = (reason: unknown): void => { + unhandled.push(reason); + }; + process.on('unhandledRejection', onUnhandledRejection); + try { + // filePath does not exist and its would-be parent path component is a + // regular file once created below, so both the read (ENOTDIR) and any + // mkdir/write fail. + await fs.writeFile(filePath, 'blocker', 'utf-8'); + const blockedPath = path.join(filePath, 'nested.pr.json'); + await expect( + upsertSessionPr(blockedPath, { number: 1, url: entry(1).url }), + ).rejects.toThrow(); + // Give the rejection a turn to be reported as unhandled if the + // cleanup chain does not absorb it. + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(unhandled).toHaveLength(0); + // A failed predecessor must not wedge the queue entry: the same path + // can be retried (still failing here — the path is still blocked — + // but with its own rejection, not hung behind the dead predecessor), + // and other paths keep working. + await expect( + upsertSessionPr(blockedPath, { number: 2, url: entry(2).url }), + ).rejects.toThrow(); + const recovered = path.join(tmpDir, 'recovered.pr.json'); + await expect( + upsertSessionPr(recovered, { number: 3, url: entry(3).url }), + ).resolves.toHaveLength(1); + } finally { + process.off('unhandledRejection', onUnhandledRejection); + } + }); +}); + +describe('mergeSessionPrLists', () => { + const at = (number: number, createdAt: string, url?: string): SessionPr => ({ + number, + url: url ?? `https://github.com/owner/repo/pull/${number}`, + createdAt, + }); + + it('unions disjoint lists in binding-time order', () => { + const merged = mergeSessionPrLists( + [at(100, '2026-08-20T00:00:00.000Z')], + [at(101, '2026-08-20T01:00:00.000Z')], + ); + expect(merged.map((p) => p.number)).toEqual([100, 101]); + }); + + it('dedupes by number, keeping the freshest entry', () => { + const merged = mergeSessionPrLists( + [at(100, '2026-08-20T00:00:00.000Z', 'https://old.example/100')], + [at(100, '2026-08-20T01:00:00.000Z', 'https://new.example/100')], + ); + expect(merged).toEqual([ + at(100, '2026-08-20T01:00:00.000Z', 'https://new.example/100'), + ]); + }); + + it('orders by binding time regardless of which side an entry came from', () => { + const merged = mergeSessionPrLists( + [at(102, '2026-08-20T02:00:00.000Z')], + [at(101, '2026-08-20T01:00:00.000Z')], + ); + expect(merged.map((p) => p.number)).toEqual([101, 102]); + }); + + it('caps the merged list, dropping the oldest', () => { + const base = Array.from({ length: SESSION_PR_LIST_LIMIT }, (_, i) => + at(i + 1, `2026-08-20T00:00:${String(i).padStart(2, '0')}.000Z`), + ); + const incoming = [ + at(SESSION_PR_LIST_LIMIT + 1, '2026-08-20T01:00:00.000Z'), + ]; + const merged = mergeSessionPrLists(base, incoming); + expect(merged).toHaveLength(SESSION_PR_LIST_LIMIT); + expect(merged[0]?.number).toBe(2); + expect(merged[merged.length - 1]?.number).toBe(SESSION_PR_LIST_LIMIT + 1); + }); +}); diff --git a/packages/core/src/services/session-pr-service.ts b/packages/core/src/services/session-pr-service.ts new file mode 100644 index 00000000000..b49b701988b --- /dev/null +++ b/packages/core/src/services/session-pr-service.ts @@ -0,0 +1,180 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { isNodeError } from '../utils/errors.js'; +import { atomicWriteJSON } from '../utils/atomicFileWrite.js'; + +/** + * Persisted GitHub pull request binding for a session. Written by the daemon + * when a PR is created from the session (e.g. the Web Shell Git dialog), and + * read on session listing so the binding survives daemon restarts. A session + * may produce several PRs (stacked or unrelated), so the sidecar keeps a + * bounded list ordered by binding time — the last entry is the latest. + * + * Stored as a sidecar JSON file alongside the session's JSONL transcript at + * `/.pr.json`. + */ +export interface SessionPr { + number: number; + url: string; + createdAt: string; +} + +/** Bound on the persisted PR list; oldest bindings are dropped beyond it. */ +export const SESSION_PR_LIST_LIMIT = 10; + +/** Upper bound for a bound PR URL; generous for enterprise hosts + long paths. */ +export const SESSION_PR_URL_MAX_LENGTH = 2048; + +interface SessionPrList { + prs: SessionPr[]; +} + +// Mirrors the bridge's hasControlCharacter (ESLint forbids control-char +// regexes). +function hasControlCharacter(value: string): boolean { + return Array.from(value).some((character) => { + const code = character.charCodeAt(0); + return code <= 31 || code === 127; + }); +} + +/** + * Runtime shape check for one entry. The url is rendered as a link target, + * so only http(s) URLs are accepted. + */ +function isValidSessionPr(value: unknown): value is SessionPr { + if (value === null || typeof value !== 'object') return false; + const v = value as Record; + return ( + typeof v['number'] === 'number' && + Number.isInteger(v['number']) && + v['number'] > 0 && + typeof v['url'] === 'string' && + v['url'].length <= SESSION_PR_URL_MAX_LENGTH && + /^https?:\/\//i.test(v['url']) && + // The url is interpolated into a stderr audit line by the bridge — + // control characters would forge log lines. + !hasControlCharacter(v['url']) && + typeof v['createdAt'] === 'string' + ); +} + +/** + * Runtime shape check for a parsed sidecar object. Guards against partial + * writes and manual edits (same rationale as the worktree sidecar check). + */ +function isValidSessionPrList(value: unknown): value is SessionPrList { + if (value === null || typeof value !== 'object') return false; + const prs = (value as Record)['prs']; + return Array.isArray(prs) && prs.length > 0 && prs.every(isValidSessionPr); +} + +/** + * Read the sidecar. Returns null when the file does not exist, is invalid + * JSON, or fails the shape check. Throws only on unexpected I/O errors. + */ +export async function readSessionPrs( + filePath: string, + options: { signal?: AbortSignal } = {}, +): Promise { + let raw: string; + try { + options.signal?.throwIfAborted(); + raw = options.signal + ? await fs.readFile(filePath, { + encoding: 'utf-8', + signal: options.signal, + }) + : await fs.readFile(filePath, 'utf-8'); + } catch (error) { + options.signal?.throwIfAborted(); + if (isNodeError(error) && error.code === 'ENOENT') return null; + throw error; + } + options.signal?.throwIfAborted(); + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + options.signal?.throwIfAborted(); + if (!isValidSessionPrList(parsed)) return null; + return parsed.prs; +} + +/** Writes the PR sidecar via `atomicWriteJSON`. */ +export async function writeSessionPrs( + filePath: string, + prs: SessionPr[], +): Promise { + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await atomicWriteJSON(filePath, { prs } satisfies SessionPrList); +} + +/** + * Union two binding lists, deduping by PR number and keeping each number's + * freshest entry (by createdAt), ordered by binding time and capped. Used + * when an archive-state move finds both halves of a split pair: the sidecar + * is the append-only binding history, so the halves are merged instead of + * one being stranded. + */ +export function mergeSessionPrLists( + base: SessionPr[], + incoming: SessionPr[], +): SessionPr[] { + const byNumber = new Map(); + for (const entry of [...base, ...incoming]) { + const known = byNumber.get(entry.number); + if (!known || entry.createdAt >= known.createdAt) { + byNumber.set(entry.number, entry); + } + } + return [...byNumber.values()] + .sort((left, right) => left.createdAt.localeCompare(right.createdAt)) + .slice(-SESSION_PR_LIST_LIMIT); +} + +// Serializes read-modify-write cycles per sidecar path: concurrent bindings +// for the same session must not interleave (read [] → read [] → write [A] → +// write [B] would silently drop A). A failed predecessor must not block +// later bindings. +const upsertQueue = new Map>(); + +/** + * Insert or refresh a binding (matched by PR number) and persist the list, + * keeping at most {@link SESSION_PR_LIST_LIMIT} latest entries. A re-bound + * number moves to the end (latest) with a fresh createdAt. + */ +export function upsertSessionPr( + filePath: string, + pr: { number: number; url: string }, +): Promise { + const run = async (): Promise => { + const existing = (await readSessionPrs(filePath)) ?? []; + const rest = existing.filter((entry) => entry.number !== pr.number); + const next = [ + ...rest, + { number: pr.number, url: pr.url, createdAt: new Date().toISOString() }, + ].slice(-SESSION_PR_LIST_LIMIT); + await writeSessionPrs(filePath, next); + return next; + }; + const previous = upsertQueue.get(filePath) ?? Promise.resolve(); + const next = previous.catch(() => undefined).then(run); + upsertQueue.set(filePath, next); + // The cleanup chain must absorb `next`'s rejection too — a derived + // finally/catch promise would otherwise reject unhandled whenever the + // queued write fails, even though every caller awaits `next` itself. + const cleanup = (): void => { + if (upsertQueue.get(filePath) === next) upsertQueue.delete(filePath); + }; + void next.then(cleanup, cleanup); + return next; +} diff --git a/packages/core/src/services/sessionService.test.ts b/packages/core/src/services/sessionService.test.ts index ff185969f5d..702530d05bd 100644 --- a/packages/core/src/services/sessionService.test.ts +++ b/packages/core/src/services/sessionService.test.ts @@ -38,6 +38,7 @@ import { SessionOrganizationService } from './session-organization-service.js'; import { CompressionStatus } from '../core/turn.js'; import type { ChatRecord } from './chatRecordingService.js'; import * as jsonl from '../utils/jsonl-utils.js'; +import { readSessionPrs, writeSessionPrs } from './session-pr-service.js'; vi.mock('./usageHistoryService.js', () => ({ persistUsageBeforeTranscriptDeletion: vi.fn().mockResolvedValue(true), @@ -46,6 +47,12 @@ vi.mock('node:path'); vi.mock('../utils/paths.js'); vi.mock('../utils/runtimeStatus.js'); vi.mock('../utils/jsonl-utils.js'); +// Keep the real merge logic; only the sidecar I/O is controlled per test. +vi.mock('./session-pr-service.js', async (importOriginal) => ({ + ...(await importOriginal()), + readSessionPrs: vi.fn(), + writeSessionPrs: vi.fn(), +})); describe('SessionService', () => { let sessionService: SessionService; @@ -1834,6 +1841,30 @@ describe('SessionService', () => { ); }); + it('should remove pr sidecars in both states when removing a session', async () => { + vi.mocked(jsonl.readLines).mockImplementation( + async (filePath: string) => { + if (filePath.includes('/chats/archive/')) return [recordA1]; + const error = new Error('ENOENT') as NodeJS.ErrnoException; + error.code = 'ENOENT'; + throw error; + }, + ); + existsSyncSpy.mockImplementation((filePath: fs.PathLike) => + filePath.toString().endsWith(`${sessionIdA}.pr.json`), + ); + + const result = await sessionService.removeSession(sessionIdA); + + expect(result).toBe(true); + expect(unlinkSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.pr.json`), + ); + expect(unlinkSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + ); + }); + it('should remove both JSONL files when active and archived copies conflict', async () => { vi.mocked(jsonl.readLines).mockResolvedValue([recordA1]); existsSyncSpy.mockImplementation((filePath: fs.PathLike) => @@ -1937,6 +1968,63 @@ describe('SessionService', () => { ); }); + it('should move the pr sidecar into the archive directory', async () => { + mockActiveSessionOnly(); + existsSyncSpy.mockImplementation((filePath) => + filePath.toString().endsWith(`/chats/${sessionIdA}.pr.json`), + ); + + const result = await sessionService.archiveSessions([sessionIdA]); + + expect(result.archived).toEqual([sessionIdA]); + expect(result.errors).toEqual([]); + expect(renameSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.pr.json`), + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + ); + }); + + it('should merge split pr sidecars on archive instead of wedging', async () => { + mockActiveSessionOnly(); + existsSyncSpy.mockImplementation((filePath) => { + const value = filePath.toString(); + return ( + value.endsWith(`/chats/${sessionIdA}.pr.json`) || + value.endsWith(`/chats/archive/${sessionIdA}.pr.json`) + ); + }); + const archivedEntry = { + number: 100, + url: 'https://github.com/o/r/pull/100', + createdAt: '2026-08-20T00:00:00.000Z', + }; + const activeEntry = { + number: 101, + url: 'https://github.com/o/r/pull/101', + createdAt: '2026-08-20T01:00:00.000Z', + }; + vi.mocked(readSessionPrs) + .mockResolvedValueOnce([archivedEntry]) + .mockResolvedValueOnce([activeEntry]); + const warnings: string[] = []; + const service = new SessionService('/test/project/root', { + onWarning: (message) => warnings.push(message), + }); + + const result = await service.archiveSessions([sessionIdA]); + + expect(result.archived).toEqual([sessionIdA]); + expect(result.errors).toEqual([]); + expect(warnings).toEqual([]); + expect(writeSessionPrs).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + [archivedEntry, activeEntry], + ); + expect(unlinkSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.pr.json`), + ); + }); + it('should archive JSONL and warn when archiving worktree sidecar fails', async () => { mockActiveSessionOnly(); mockActiveWorktreeSidecarOnly(); @@ -2182,6 +2270,68 @@ describe('SessionService', () => { ); }); + it('should move the pr sidecar back to the active directory', async () => { + mockArchivedSessionOnly(); + existsSyncSpy.mockImplementation((filePath) => + filePath.toString().endsWith(`/chats/archive/${sessionIdA}.pr.json`), + ); + + const result = await sessionService.unarchiveSessions([sessionIdA]); + + expect(result.unarchived).toEqual([sessionIdA]); + expect(result.errors).toEqual([]); + expect(renameSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + expect.stringContaining(`/chats/${sessionIdA}.pr.json`), + ); + }); + + it('should merge a split pr sidecar pair on unarchive, keeping the full history', async () => { + mockArchivedSessionOnly(); + existsSyncSpy.mockImplementation((filePath) => { + const value = filePath.toString(); + return ( + value.endsWith(`/chats/archive/${sessionIdA}.pr.json`) || + value.endsWith(`/chats/${sessionIdA}.pr.json`) + ); + }); + const olderOne = { + number: 100, + url: 'https://github.com/o/r/pull/100', + createdAt: '2026-08-20T00:00:00.000Z', + }; + const olderTwo = { + number: 101, + url: 'https://github.com/o/r/pull/101', + createdAt: '2026-08-20T00:30:00.000Z', + }; + const orphan = { + number: 102, + url: 'https://github.com/o/r/pull/102', + createdAt: '2026-08-20T01:00:00.000Z', + }; + vi.mocked(readSessionPrs) + .mockResolvedValueOnce([orphan]) + .mockResolvedValueOnce([olderOne, olderTwo]); + const warnings: string[] = []; + const service = new SessionService('/test/project/root', { + onWarning: (message) => warnings.push(message), + }); + + const result = await service.unarchiveSessions([sessionIdA]); + + expect(result.unarchived).toEqual([sessionIdA]); + expect(result.errors).toEqual([]); + expect(warnings).toEqual([]); + expect(writeSessionPrs).toHaveBeenCalledWith( + expect.stringContaining(`/chats/${sessionIdA}.pr.json`), + [olderOne, olderTwo, orphan], + ); + expect(unlinkSyncSpy).toHaveBeenCalledWith( + expect.stringContaining(`/chats/archive/${sessionIdA}.pr.json`), + ); + }); + it('should skip location reads when unarchiving known archived sessions', async () => { mockArchivedSessionOnly(); const getLocationSpy = vi.spyOn(sessionService, 'getSessionLocation'); diff --git a/packages/core/src/services/sessionService.ts b/packages/core/src/services/sessionService.ts index ce9270fa64f..88c386abb1d 100644 --- a/packages/core/src/services/sessionService.ts +++ b/packages/core/src/services/sessionService.ts @@ -46,6 +46,11 @@ import { type RebuiltSessionArtifactSnapshot, } from './session-artifact-persistence.js'; import { SessionOrganizationService } from './session-organization-service.js'; +import { + mergeSessionPrLists, + readSessionPrs, + writeSessionPrs, +} from './session-pr-service.js'; import { SessionTranscriptReader, SessionTranscriptTooLargeError, @@ -559,6 +564,13 @@ export class SessionService { ); } + private getPrSessionPathForState( + sessionId: string, + state: SessionArchiveState, + ): string { + return path.join(this.getChatsDirForState(state), `${sessionId}.pr.json`); + } + private async sessionBelongsToCurrentProject( sessionId: string, recordCwd: string, @@ -648,6 +660,18 @@ export class SessionService { return this.getWorktreeSessionPathForState(sessionId, state); } + /** + * Returns the absolute path to the sidecar JSON file that stores the + * session's GitHub PR binding for the given session id. The file may not + * exist yet — consumers must handle ENOENT as "no PR binding". + */ + getPrSessionPathForArchiveState( + sessionId: string, + state: SessionArchiveState, + ): string { + return this.getPrSessionPathForState(sessionId, state); + } + private async readProjectSessionHead( sessionId: string, filePath: string, @@ -936,6 +960,15 @@ export class SessionService { } } + private removePrSidecars(sessionId: string): void { + for (const state of ['active', 'archived'] as const) { + const sidecar = this.getPrSessionPathForState(sessionId, state); + if (fs.existsSync(sidecar)) { + this.removeFileIfExists(sidecar); + } + } + } + private removePromptLedgers(sessionId: string): void { for (const state of ['active', 'archived'] as const) { const ledger = this.getPromptLedgerPathForState(sessionId, state); @@ -1025,6 +1058,37 @@ export class SessionService { fs.unlinkSync(sourcePath); } + /** + * Move a PR sidecar across archive states. Same policy as + * {@link moveLedgerSidecar}: the sidecar is the append-only binding + * history, so when both halves of a split pair exist (a crash between + * the transcript rename and the sidecar move, or an orphaned write) + * they are merged by PR number instead of wedging the pair forever — + * no transition would ever reunite them otherwise. Throws propagate to + * the caller, which owns the warn-only policy. + */ + private async movePrSidecar( + sourcePath: string, + destinationPath: string, + ): Promise { + if (!fs.existsSync(sourcePath)) { + return; + } + fs.mkdirSync(path.dirname(destinationPath), { recursive: true }); + if (!fs.existsSync(destinationPath)) { + fs.renameSync(sourcePath, destinationPath); + return; + } + const merged = mergeSessionPrLists( + (await readSessionPrs(destinationPath)) ?? [], + (await readSessionPrs(sourcePath)) ?? [], + ); + if (merged.length > 0) { + await writeSessionPrs(destinationPath, merged); + } + fs.unlinkSync(sourcePath); + } + private sessionFileMoveError( action: 'archive' | 'unarchive', error: unknown, @@ -1837,6 +1901,7 @@ export class SessionService { this.removeFileIfExists(archivedPath); } this.removeWorktreeSidecars(sessionId); + this.removePrSidecars(sessionId); this.removePromptLedgers(sessionId); this.removeFileHistoryBackups(sessionId); return true; @@ -1852,6 +1917,7 @@ export class SessionService { await this.salvageUsageBestEffort(archivedPath); this.removeFileIfExists(archivedPath); this.removeWorktreeSidecars(sessionId); + this.removePrSidecars(sessionId); this.removePromptLedgers(sessionId); this.removeFileHistoryBackups(sessionId); return true; @@ -1928,6 +1994,16 @@ export class SessionService { `archiveSessions: failed to move worktree sidecar for ${sessionId} from ${activeSidecar} to ${archivedSidecar}: ${sidecarError}`, ); } + try { + await this.movePrSidecar( + this.getPrSessionPathForState(sessionId, 'active'), + this.getPrSessionPathForState(sessionId, 'archived'), + ); + } catch (sidecarError) { + this.warn( + `archiveSessions: failed to move pr sidecar for ${sessionId}: ${sidecarError}`, + ); + } try { this.moveLedgerSidecar(activeLedger, archivedLedger); } catch (ledgerError) { @@ -2000,6 +2076,16 @@ export class SessionService { `unarchiveSessions: failed to move worktree sidecar for ${sessionId} from ${archivedSidecar} to ${activeSidecar}: ${sidecarError}`, ); } + try { + await this.movePrSidecar( + this.getPrSessionPathForState(sessionId, 'archived'), + this.getPrSessionPathForState(sessionId, 'active'), + ); + } catch (sidecarError) { + this.warn( + `unarchiveSessions: failed to move pr sidecar for ${sessionId}: ${sidecarError}`, + ); + } const archivedLedger = this.getPromptLedgerPathForState( sessionId, 'archived', diff --git a/packages/sdk-typescript/scripts/build.js b/packages/sdk-typescript/scripts/build.js index c41d95ca80e..d0665a0387a 100644 --- a/packages/sdk-typescript/scripts/build.js +++ b/packages/sdk-typescript/scripts/build.js @@ -101,6 +101,8 @@ const rootDir = join(__dirname, '..'); // (`unrecognizedDiagnostics` routing + selector, #8823). // Bumped from 198KB to 199KB for persistent session attachment read/remove and // binary resource hydration. +// Bumped from 199KB to 200KB for the session PR binding (`DaemonSessionPrInfo` +// + validators). // Bumped from 199KB to 200KB for the retention byte budget (block byte // estimation + budget-aware trimming) and backing-store-detached string caps // (#9303 review round 3). diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index fc8a351cc9e..819c029435d 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -10,6 +10,7 @@ import { } from '@qwen-code/acp-bridge/mcpTimeouts'; import { CHANNEL_CONTROL_DEFAULT_TIMEOUT_MS } from '@qwen-code/acp-bridge/channelControlTimeouts'; import { DaemonAuthFlow } from './DaemonAuthFlow.js'; +import { isDaemonSessionPrInfo } from './session-pr.js'; import { DaemonHttpError } from './DaemonHttpError.js'; import type { DaemonSseConnectReason, @@ -65,6 +66,7 @@ import type { DaemonSessionOrganizationResult, DaemonSessionOrganizationUpdate, DaemonSessionSummary, + DaemonSessionPrInfo, DaemonSessionSupportedCommandsStatus, DaemonSessionStatsStatus, DaemonUsageDashboard, @@ -505,6 +507,8 @@ export function isDaemonTurnError(error: unknown): error is DaemonTurnError { ); } +export { isDaemonSessionPrInfo } from './session-pr.js'; + /** * The daemon rejected a session branch because the requested checkpoint is * no longer on the session's active history path. Daemon action layers and @@ -5339,7 +5343,7 @@ export class DaemonClient { */ async updateSessionMetadata( sessionId: string, - metadata: { displayName?: string }, + metadata: { displayName?: string; pr?: DaemonSessionPrInfo }, clientId?: string, ): Promise { return await this.fetchWithTimeout( @@ -5353,10 +5357,20 @@ export class DaemonClient { if (res.status === 200) { const body = (await res.json()) as { displayName?: unknown; + prs?: unknown; }; - return typeof body.displayName === 'string' - ? { displayName: body.displayName } - : {}; + const result: SessionMetadataResult = {}; + if (typeof body.displayName === 'string') { + result.displayName = body.displayName; + } + if (Array.isArray(body.prs)) { + // Per-entry gate: a buggy or hostile daemon response cannot + // surface a non-http(s) url or malformed number downstream (the + // tooltip renders these as links). Valid entries survive. + const valid = body.prs.filter(isDaemonSessionPrInfo); + if (valid.length > 0) result.prs = valid; + } + return result; } throw await this.failOnError(res, 'PATCH /session/:id/metadata'); }, @@ -6114,7 +6128,7 @@ export class WorkspaceDaemonClient { updateSessionMetadata( sessionId: string, - metadata: { displayName: string }, + metadata: { displayName?: string; pr?: DaemonSessionPrInfo }, clientId?: string, ): Promise { return this.client.workspaceJsonRequest( diff --git a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts index 3f6997704d1..b1efc77d66a 100644 --- a/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonSessionClient.ts @@ -57,6 +57,7 @@ import type { PromptResult, SetModelResult, SessionMetadataResult, + DaemonSessionPrInfo, } from './types.js'; /** Compacted replay snapshot returned by the daemon on session load. */ @@ -945,6 +946,7 @@ export class DaemonSessionClient { async updateMetadata(metadata: { displayName?: string; + pr?: DaemonSessionPrInfo; }): Promise { return await this.client.updateSessionMetadata( this.sessionId, diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index bf72777e677..864ec50755d 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -10,10 +10,12 @@ import type { DaemonErrorKind, DaemonMcpTransport, DaemonSessionArtifactChange, + DaemonSessionPrInfo, DaemonSkillToggleMutation, PermissionOutcome, PromptContentBlock, } from './types.js'; +import { isDaemonSessionPrInfo } from './session-pr.js'; // Single source of truth: the daemon publisher owns the wire literal in // acp-bridge's dependency-free `daemonEventTypes` module. We re-export it so the // validator/reducer below, and the browser consumer via `@qwen-code/sdk/daemon`, @@ -296,6 +298,7 @@ export interface DaemonSessionClosedData { export interface DaemonSessionMetadataUpdatedData { sessionId: string; displayName?: string; + prs?: DaemonSessionPrInfo[]; [key: string]: unknown; } @@ -2649,10 +2652,17 @@ function isSessionClosedData(value: unknown): value is DaemonSessionClosedData { function isSessionMetadataUpdatedData( value: unknown, ): value is DaemonSessionMetadataUpdatedData { + if ( + !isRecord(value) || + !isNonEmptyString(value['sessionId']) || + !isOptionalStringOrNull(value['displayName']) + ) { + return false; + } + const prs = value['prs']; return ( - isRecord(value) && - isNonEmptyString(value['sessionId']) && - isOptionalStringOrNull(value['displayName']) + prs === undefined || + (Array.isArray(prs) && prs.every(isDaemonSessionPrInfo)) ); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index df05ba8da7d..9386beee487 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -539,6 +539,7 @@ export type { DaemonSessionArchiveState, DaemonWorktreeInfo, DaemonBranchInfo, + DaemonSessionPrInfo, DaemonBranchPoint, DaemonSessionExportFormat, DaemonSessionExportResult, diff --git a/packages/sdk-typescript/src/daemon/session-pr.ts b/packages/sdk-typescript/src/daemon/session-pr.ts new file mode 100644 index 00000000000..4c63cb99ada --- /dev/null +++ b/packages/sdk-typescript/src/daemon/session-pr.ts @@ -0,0 +1,41 @@ +/** + * @license + * Copyright 2025 Alibaba Group Holding Limited. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 + */ + +import type { DaemonSessionPrInfo } from './types.js'; + +/** Upper bound for a bound PR URL; generous for enterprise hosts + long paths. */ +export const MAX_SESSION_PR_URL_LENGTH = 2048; + +// Mirrors the bridge's hasControlCharacter (ESLint forbids control-char +// regexes). +function hasControlCharacter(value: string): boolean { + return Array.from(value).some((character) => { + const code = character.charCodeAt(0); + return code <= 31 || code === 127; + }); +} + +/** + * Runtime guard for a session PR binding received from the daemon. The url + * is rendered as a link target, so only http(s) URLs are accepted. + */ +export function isDaemonSessionPrInfo( + value: unknown, +): value is DaemonSessionPrInfo { + if (typeof value !== 'object' || value === null) return false; + const v = value as Record; + return ( + typeof v['number'] === 'number' && + Number.isInteger(v['number']) && + v['number'] > 0 && + typeof v['url'] === 'string' && + v['url'].length <= MAX_SESSION_PR_URL_LENGTH && + /^https?:\/\//i.test(v['url']) && + // The daemon interpolates the url into a stderr audit line — control + // characters would forge log lines downstream of this gate. + !hasControlCharacter(v['url']) + ); +} diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index d024b65b5bf..7d851a36987 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -1024,6 +1024,12 @@ export interface DaemonBranchInfo { baseBranch: string; } +/** GitHub pull request bound to a session (e.g. created from the Web Shell Git dialog). */ +export interface DaemonSessionPrInfo { + number: number; + url: string; +} + /** Returned from `POST /session`. */ export interface DaemonSession { sessionId: string; @@ -1267,6 +1273,8 @@ export interface DaemonSessionSummary { worktree?: DaemonWorktreeInfo; /** Present when the session was created with a new branch. */ branch?: DaemonBranchInfo; + /** Present when GitHub PRs have been bound to the session (last = latest). */ + prs?: DaemonSessionPrInfo[]; } export type DaemonSessionExportFormat = 'html' | 'md' | 'json' | 'jsonl'; @@ -1456,6 +1464,7 @@ export interface DaemonUnarchiveSessionsResult { /** Effective mutable metadata returned from `PATCH /session/:id/metadata`. */ export interface SessionMetadataResult { displayName?: string; + prs?: DaemonSessionPrInfo[]; } type OpenStringUnion = T | (string & {}); diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index d62ef63dfbd..ca086d02df5 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -3706,6 +3706,48 @@ describe('DaemonClient', () => { client.updateSessionMetadata('s-1', { displayName: 'test' }), ).rejects.toMatchObject({ status: 404 }); }); + + it('sends a pr binding and parses the returned prs list', async () => { + const { fetch, calls } = recordingFetch(() => + jsonResponse(200, { + sessionId: 's-1', + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.updateSessionMetadata('s-1', { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(JSON.parse(calls[0]!.body!)).toEqual({ + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(result.prs).toEqual([ + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ]); + }); + + it('drops prs entries that fail the shape gate', async () => { + // A buggy or hostile daemon response must not surface a javascript: + // url or a non-integer number downstream (the tooltip renders these + // as links). + const { fetch } = recordingFetch(() => + jsonResponse(200, { + sessionId: 's-1', + prs: [ + { number: 1, url: 'javascript:alert(1)' }, + { number: 1.5, url: 'https://github.com/o/r/pull/1' }, + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ], + }), + ); + const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); + const result = await client.updateSessionMetadata('s-1', { + pr: { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + }); + expect(result.prs).toEqual([ + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ]); + }); }); describe('subscribeEvents', () => { diff --git a/packages/sdk-typescript/test/unit/daemonEvents.test.ts b/packages/sdk-typescript/test/unit/daemonEvents.test.ts index 8f005aa2f5c..2b3a2e63b29 100644 --- a/packages/sdk-typescript/test/unit/daemonEvents.test.ts +++ b/packages/sdk-typescript/test/unit/daemonEvents.test.ts @@ -1152,6 +1152,25 @@ describe('daemon event schema', () => { expect(cleared.displayName).toBeUndefined(); }); + it('keeps displayName on a pr-binding metadata event that echoes the name', () => { + // The bridge echoes the current displayName on pr-binding events because + // the fold treats an absent name as "cleared" — a pr event without the + // echo would blank the title until the next rename. + const state = reduceDaemonSessionEvents([ + { + id: 1, + v: 1, + type: 'session_metadata_updated', + data: { + sessionId: 's-1', + displayName: 'My Session', + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }, + }, + ]); + expect(state.displayName).toBe('My Session'); + }); + it('recognizes slow_client_warning frames as known events', () => { // PR 14b fix (codex round 8 — sibling consistency): `satisfies // DaemonEvent` keeps `v: 1` / `type: 'slow_client_warning'` diff --git a/packages/web-shell/client/components/SessionOverviewPanel.test.tsx b/packages/web-shell/client/components/SessionOverviewPanel.test.tsx index 40f4c28e015..4ff4ba09f08 100644 --- a/packages/web-shell/client/components/SessionOverviewPanel.test.tsx +++ b/packages/web-shell/client/components/SessionOverviewPanel.test.tsx @@ -268,6 +268,17 @@ describe('deriveSessionCards', () => { expect(cards[0].clientCount).toBe(2); }); + it('passes bound PRs through to the card', () => { + const prs = [ + { number: 9500, url: 'https://github.com/o/r/pull/9500' }, + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ]; + const cards = deriveSessionCards([session('s', { prs })], [], undefined); + expect(cards[0].prs).toEqual(prs); + const bare = deriveSessionCards([session('bare')], [], undefined); + expect(bare[0].prs).toBeUndefined(); + }); + it('does not expose opaque route ids as model names', () => { const cards = deriveSessionCards( [session('s')], diff --git a/packages/web-shell/client/components/SessionOverviewPanel.tsx b/packages/web-shell/client/components/SessionOverviewPanel.tsx index 34315e632f3..3e68f9572cf 100644 --- a/packages/web-shell/client/components/SessionOverviewPanel.tsx +++ b/packages/web-shell/client/components/SessionOverviewPanel.tsx @@ -11,10 +11,12 @@ import { } from '@qwen-code/webui/daemon-react-sdk'; import type { DaemonSessionGroupPresetColor, + DaemonSessionPrInfo, DaemonSessionSummary, DaemonStatusReportSession, } from '@qwen-code/sdk/daemon'; import { useI18n } from '../i18n'; +import { SessionPrBadge } from './SessionPrBadge'; import { formatRelativeTime } from '../utils/formatRelativeTime'; import { buildSplitUrl, MAX_SPLIT_PANES } from '../utils/splitUrl'; import { @@ -57,6 +59,8 @@ export interface SessionCard { updatedAt?: string; color?: DaemonSessionGroupPresetColor | null; isCurrent: boolean; + /** GitHub PRs bound to the session, in binding order (last = latest). */ + prs?: DaemonSessionPrInfo[]; /** The workspace the session lives in. */ workspaceCwd: string; /** True when the session belongs to a non-primary workspace. */ @@ -104,6 +108,7 @@ export function deriveSessionCards( updatedAt: session.updatedAt || session.createdAt, color: session.color, isCurrent: session.sessionId === currentSessionId, + prs: session.prs, workspaceCwd: session.workspaceCwd, isNonPrimary: isNonPrimaryWorkspaceSession( session.workspaceCwd, @@ -422,6 +427,7 @@ function SessionOverviewPanelInner({ {t('sessionsOverview.current')} )} +
isExternalOpenUrl(pr.url)); + if (openable.length === 0) return null; + const latest = openable[openable.length - 1]; + const multiple = openable.length > 1; + const label = multiple + ? t('sidebar.sessionPrMultiple', { + number: latest.number, + count: openable.length, + }) + : t('sidebar.sessionPr', { number: latest.number }); + return ( + { + event.stopPropagation(); + openExternalLink(event, latest.url); + }} + onDoubleClick={(event) => event.stopPropagation()} + onKeyDown={(event) => { + // Only swallow Enter — the badge must not block roving-listbox + // navigation keys (ArrowUp/Down/Home/End/Space) in picker dialogs. + // Enter would otherwise activate the native anchor on top of the + // dialog's confirm handling. + if (event.key === 'Enter') event.stopPropagation(); + }} + > + #{latest.number} + {multiple ? ` +${openable.length - 1}` : ''} + + ); +} diff --git a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx index 11ab2058d37..053d37f6be5 100644 --- a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.test.tsx @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { I18nProvider } from '../../i18n'; +import type { DaemonSessionSummary } from '@qwen-code/sdk/daemon'; import { dp } from './dialogStyles'; Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); @@ -11,21 +12,24 @@ if (!Element.prototype.scrollIntoView) { Element.prototype.scrollIntoView = () => {}; } -let sessions = [ +let sessions: DaemonSessionSummary[] = [ { sessionId: 's0', + workspaceCwd: '/work/repo', displayName: 'S0', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', }, { sessionId: 's1', + workspaceCwd: '/work/repo', displayName: 'S1', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', }, { sessionId: 'me', + workspaceCwd: '/work/repo', displayName: 'Current Session', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', @@ -221,6 +225,34 @@ describe('DeleteSessionDialog selection', () => { expect(dangerButton().disabled).toBe(true); }); + it('matches a session by its bound PR number in the filter', () => { + sessions = [ + { + sessionId: 'pr-session', + workspaceCwd: '/work/repo', + displayName: 'Fix CI', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }, + { + sessionId: 'other', + workspaceCwd: '/work/repo', + displayName: 'Unrelated', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + ]; + mount(); + + typeFilter('#9517'); + expect(rows()).toHaveLength(1); + expect(rows()[0].textContent).toContain('Fix CI'); + + typeFilter('#9999'); + expect(rows()).toHaveLength(0); + }); + it('prunes stale checked ids after an unfiltered session refresh', async () => { mount(); diff --git a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx index 7781a9c2b95..e2332d12091 100644 --- a/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx +++ b/packages/web-shell/client/components/dialogs/DeleteSessionDialog.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { dp } from './dialogStyles'; +import { sessionMatchesGitQuery } from '../sidebar/sessionSearch'; import { useConnection } from '@qwen-code/webui/daemon-react-sdk'; import { useI18n } from '../../i18n'; import { useListboxKeyboard } from '../../hooks/useListboxKeyboard'; @@ -61,7 +62,8 @@ export function DeleteSessionDialog({ const q = filterQuery.toLowerCase(); return ( (s.displayName || '').toLowerCase().includes(q) || - s.sessionId.toLowerCase().includes(q) + s.sessionId.toLowerCase().includes(q) || + sessionMatchesGitQuery(s, q) ); }) : sessions, diff --git a/packages/web-shell/client/components/dialogs/GitDialog.test.tsx b/packages/web-shell/client/components/dialogs/GitDialog.test.tsx index ec5849d5787..a27d5dc9e74 100644 --- a/packages/web-shell/client/components/dialogs/GitDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/GitDialog.test.tsx @@ -26,6 +26,8 @@ const { workspaceGitHubDefaultBranch, workspaceGit, workspaceGitHubCreatePullRequest, + updateSessionMetadata, + btwSession, workspaceClient, mockState, } = vi.hoisted(() => { @@ -38,7 +40,10 @@ const { const workspaceGitHubDefaultBranch = vi.fn(); const workspaceGit = vi.fn(); const workspaceGitHubCreatePullRequest = vi.fn(); + const updateSessionMetadata = vi.fn(); + const btwSession = vi.fn(); const workspaceClient = { + btwSession, workspaceByCwd: () => ({ workspaceGitDiff, workspaceGitDiffFile: vi.fn(), @@ -51,6 +56,7 @@ const { workspaceGitHubDefaultBranch, workspaceGit, workspaceGitHubCreatePullRequest, + updateSessionMetadata, }), }; const mockState = { capabilities: undefined as unknown }; @@ -64,6 +70,8 @@ const { workspaceGitHubDefaultBranch, workspaceGit, workspaceGitHubCreatePullRequest, + updateSessionMetadata, + btwSession, workspaceClient, mockState, }; @@ -844,4 +852,262 @@ describe('GitDialog', () => { ); expect(document.body.textContent).toContain('#99'); }); + + it('binds the created PR to the current session', async () => { + workspaceGitDiff.mockResolvedValue({ + files: [], + linesAdded: 0, + linesRemoved: 0, + hiddenCount: 0, + }); + workspaceGit.mockResolvedValue({ branch: 'feat/x', detached: false }); + workspaceGitHubDefaultBranch.mockResolvedValue({ branch: 'origin/main' }); + workspaceGitBranches.mockResolvedValue({ + local: [{ name: 'main' }], + remote: [{ name: 'origin/main' }], + }); + workspaceGitHubCreatePullRequest.mockResolvedValue({ + number: 99, + url: 'https://github.com/o/r/pull/99', + }); + updateSessionMetadata.mockResolvedValue({}); + mockState.capabilities = { features: ['workspace_github_prs'] }; + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + await flush(); + + const createPrBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find((b) => b.textContent?.includes('Create Pull Request')); + expect(createPrBtn).toBeTruthy(); + await act(async () => { + createPrBtn!.click(); + }); + await flush(); + + const titleInput = document.body.querySelector( + '[data-web-shell-dialog] input', + ); + expect(titleInput).toBeTruthy(); + await act(async () => { + const nativeSetter = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value', + )?.set; + nativeSetter?.call(titleInput, 'feat: add new feature'); + titleInput!.dispatchEvent(new Event('input', { bubbles: true })); + }); + await flush(); + + const submitBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find( + (b) => + b.textContent?.includes('Create') && + !b.textContent?.includes('Pull Request'), + ); + expect(submitBtn).toBeTruthy(); + await act(async () => { + submitBtn!.click(); + }); + await flush(); + + expect(updateSessionMetadata).toHaveBeenCalledWith('sess-1', { + pr: { number: 99, url: 'https://github.com/o/r/pull/99' }, + }); + }); + + it('keeps the PR-creation success when the session binding fails', async () => { + workspaceGitDiff.mockResolvedValue({ + files: [], + linesAdded: 0, + linesRemoved: 0, + hiddenCount: 0, + }); + workspaceGit.mockResolvedValue({ branch: 'feat/x', detached: false }); + workspaceGitHubDefaultBranch.mockResolvedValue({ branch: 'origin/main' }); + workspaceGitBranches.mockResolvedValue({ + local: [{ name: 'main' }], + remote: [{ name: 'origin/main' }], + }); + workspaceGitHubCreatePullRequest.mockResolvedValue({ + number: 99, + url: 'https://github.com/o/r/pull/99', + }); + updateSessionMetadata.mockRejectedValue(new Error('daemon gone')); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + mockState.capabilities = { features: ['workspace_github_prs'] }; + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + await flush(); + + const createPrBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find((b) => b.textContent?.includes('Create Pull Request')); + await act(async () => { + createPrBtn!.click(); + }); + await flush(); + + const titleInput = document.body.querySelector( + '[data-web-shell-dialog] input', + ); + await act(async () => { + const nativeSetter = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value', + )?.set; + nativeSetter?.call(titleInput, 'feat: add new feature'); + titleInput!.dispatchEvent(new Event('input', { bubbles: true })); + }); + await flush(); + + const submitBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find( + (b) => + b.textContent?.includes('Create') && + !b.textContent?.includes('Pull Request'), + ); + await act(async () => { + submitBtn!.click(); + }); + await flush(); + // Let the rejected binding promise settle. + await flush(); + + expect(updateSessionMetadata).toHaveBeenCalledWith('sess-1', { + pr: { number: 99, url: 'https://github.com/o/r/pull/99' }, + }); + // The binding failure is a warning only — the created PR status stays. + expect(document.body.textContent).toContain('#99'); + expect(warnSpy).toHaveBeenCalled(); + warnSpy.mockRestore(); + }); + + it('binds the PR to the freshly resolved session after a stale-id retry', async () => { + workspaceGitDiff.mockResolvedValue({ + files: [{ path: 'a.ts', added: 3, removed: 1, isBinary: false }], + linesAdded: 3, + linesRemoved: 1, + hiddenCount: 0, + }); + workspaceGit.mockResolvedValue({ branch: 'feat/x', detached: false }); + workspaceGitHubDefaultBranch.mockResolvedValue({ branch: 'origin/main' }); + workspaceGitBranches.mockResolvedValue({ + local: [{ name: 'main' }], + remote: [{ name: 'origin/main' }], + }); + workspaceGitHubCreatePullRequest.mockResolvedValue({ + number: 99, + url: 'https://github.com/o/r/pull/99', + }); + updateSessionMetadata.mockResolvedValue({}); + // The prop session is dead (daemon restarted); the dialog's side query + // force-creates a fresh one and the ref must keep it across re-renders + // until doCreatePr binds the PR. + btwSession + .mockRejectedValueOnce(new Error('no session')) + .mockResolvedValue({ answer: 'feat: change a' }); + // The resolver's list path returns the dead id (generation's first + // btwSession fails on it); the force-create path returns the fresh id. + const resolveSessionForWorkspace = vi + .fn() + .mockImplementation((_cwd: string, force?: boolean) => + Promise.resolve(force ? 'sess-fresh' : 'sess-stale'), + ); + mockState.capabilities = { features: ['workspace_github_prs'] }; + + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + await flush(); + await flush(); + + // Commit-message generation ran through the retry: first call on the + // stale id failed, the retry hit the fresh id. + expect(btwSession).toHaveBeenCalledTimes(2); + expect(btwSession.mock.calls[0]?.[0]).toBe('sess-stale'); + expect(btwSession.mock.calls[1]?.[0]).toBe('sess-fresh'); + + const createPrBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find((b) => b.textContent?.includes('Create Pull Request')); + expect(createPrBtn).toBeTruthy(); + await act(async () => { + createPrBtn!.click(); + }); + await flush(); + + const titleInput = document.body.querySelector( + '[data-web-shell-dialog] input', + ); + await act(async () => { + const nativeSetter = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value', + )?.set; + nativeSetter?.call(titleInput, 'feat: change a'); + titleInput!.dispatchEvent(new Event('input', { bubbles: true })); + }); + await flush(); + + const submitBtn = Array.from( + document.body.querySelectorAll('[data-web-shell-dialog] button'), + ).find( + (b) => + b.textContent?.includes('Create') && + !b.textContent?.includes('Pull Request'), + ); + expect(submitBtn).toBeTruthy(); + await act(async () => { + submitBtn!.click(); + }); + await flush(); + + expect(updateSessionMetadata).toHaveBeenCalledWith('sess-fresh', { + pr: { number: 99, url: 'https://github.com/o/r/pull/99' }, + }); + }); }); diff --git a/packages/web-shell/client/components/dialogs/GitDialog.tsx b/packages/web-shell/client/components/dialogs/GitDialog.tsx index 2256cd98ceb..cc64145c9e8 100644 --- a/packages/web-shell/client/components/dialogs/GitDialog.tsx +++ b/packages/web-shell/client/components/dialogs/GitDialog.tsx @@ -103,7 +103,12 @@ export function GitDialog({ const resolveSessionRef = useRef(resolveSessionForWorkspace); resolveSessionRef.current = resolveSessionForWorkspace; const sessionIdRef = useRef(sessionId); - sessionIdRef.current = sessionId; + // Reset only when the prop actually changes — an unconditional render-body + // sync would clobber the fresh id the dialog resolves for its own side + // queries (commit-message generation) before the PR binding reads it. + useEffect(() => { + sessionIdRef.current = sessionId; + }, [sessionId]); // btwSession with automatic retry: if the resolved session is stale // (daemon restarted, session evicted), force-create a new one and retry. @@ -535,6 +540,25 @@ export function GitDialog({ url: result.url, }); setPrFormOpen(false); + // Bind the PR to the current session so the sidebar can badge and + // search sessions by PR number. Best-effort: a binding failure must + // not shadow the successful PR creation. Binding never creates a + // session — when the dialog has no session context (workspace-level + // open), the PR simply stays unbound. A session from another + // workspace is rejected by the daemon route's workspace-conflict + // check and surfaces here as a warning only. + if (typeof result.number === 'number' && result.url) { + const pr = { number: result.number, url: result.url }; + const sid = sessionIdRef.current; + if (sid) { + ws.updateSessionMetadata(sid, { pr }).catch((err: unknown) => { + console.warn( + 'Failed to bind PR to session:', + err instanceof Error ? err.message : String(err), + ); + }); + } + } } catch (err) { setPrStatus({ msg: err instanceof Error ? err.message : String(err), diff --git a/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.test.tsx b/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.test.tsx index ed140022561..c211af2598f 100644 --- a/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.test.tsx @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import { I18nProvider } from '../../i18n'; +import type { DaemonSessionSummary } from '@qwen-code/sdk/daemon'; import { dp } from './dialogStyles'; Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); @@ -11,33 +12,38 @@ if (!Element.prototype.scrollIntoView) { Element.prototype.scrollIntoView = () => {}; } -const sessions = [ +let sessions: DaemonSessionSummary[] = [ { sessionId: 's0', + workspaceCwd: '/work/repo', displayName: 'S0', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', }, { sessionId: 's1', + workspaceCwd: '/work/repo', displayName: 'S1', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', }, { sessionId: 'me', + workspaceCwd: '/work/repo', displayName: 'Current Session', clientCount: 1, updatedAt: '2026-01-01T00:00:00Z', }, { sessionId: 'inactive', + workspaceCwd: '/work/repo', displayName: 'Inactive Session', clientCount: 0, hasActivePrompt: false, updatedAt: '2026-01-01T00:00:00Z', }, ]; +const initialSessions = sessions.slice(); const releaseSessionMock = vi.fn().mockResolvedValue(undefined); let scopedSessionsOptions: unknown; @@ -112,6 +118,7 @@ afterEach(() => { container?.remove(); root = null; container = null; + sessions = initialSessions.slice(); }); describe('ReleaseSessionDialog selection', () => { @@ -241,6 +248,36 @@ describe('ReleaseSessionDialog selection', () => { expect(dangerButton().disabled).toBe(true); }); + it('matches a session by its bound PR number in the filter', () => { + sessions.splice( + 0, + sessions.length, + { + sessionId: 'pr-session', + workspaceCwd: '/work/repo', + displayName: 'Fix CI', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }, + { + sessionId: 'other', + workspaceCwd: '/work/repo', + displayName: 'Unrelated', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + ); + mount(); + + typeFilter('#9517'); + expect(rows()).toHaveLength(1); + expect(rows()[0].textContent).toContain('Fix CI'); + + typeFilter('#9999'); + expect(rows()).toHaveLength(0); + }); + it('does not confirm any row when all visible rows are non-releasable', () => { const original = sessions.slice(); try { diff --git a/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.tsx b/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.tsx index a4c25897f95..6719b69c7a1 100644 --- a/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.tsx +++ b/packages/web-shell/client/components/dialogs/ReleaseSessionDialog.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { dp } from './dialogStyles'; +import { sessionMatchesGitQuery } from '../sidebar/sessionSearch'; import { useConnection, type DaemonSessionSummary, @@ -61,7 +62,8 @@ export function ReleaseSessionDialog({ const q = filterQuery.toLowerCase(); return ( (s.displayName || '').toLowerCase().includes(q) || - s.sessionId.toLowerCase().includes(q) + s.sessionId.toLowerCase().includes(q) || + sessionMatchesGitQuery(s, q) ); }) : sessions; diff --git a/packages/web-shell/client/components/dialogs/ResumeDialog.test.tsx b/packages/web-shell/client/components/dialogs/ResumeDialog.test.tsx index 6c4522bcf25..e1113360c16 100644 --- a/packages/web-shell/client/components/dialogs/ResumeDialog.test.tsx +++ b/packages/web-shell/client/components/dialogs/ResumeDialog.test.tsx @@ -11,7 +11,7 @@ if (!Element.prototype.scrollIntoView) { Element.prototype.scrollIntoView = () => {}; } -const sessions = [ +let sessions = [ { sessionId: 'alpha-id', displayName: 'Alpha', @@ -31,6 +31,7 @@ const sessions = [ updatedAt: '2026-01-01T00:00:00Z', }, ]; +const initialSessions = sessions.slice(); let scopedSessionsOptions: unknown; vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ @@ -104,6 +105,7 @@ afterEach(() => { container?.remove(); root = null; container = null; + sessions = initialSessions.slice(); }); describe('ResumeDialog', () => { @@ -159,4 +161,30 @@ describe('ResumeDialog', () => { expect(onSelect).toHaveBeenCalledWith('beta-id'); expect(onClose).toHaveBeenCalledTimes(1); }); + + it('matches a session by its bound PR number in the filter', () => { + sessions = [ + { + sessionId: 'pr-id', + displayName: 'Fix CI', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + prs: [{ number: 9517, url: 'https://github.com/o/r/pull/9517' }], + }, + { + sessionId: 'other', + displayName: 'Unrelated', + clientCount: 1, + updatedAt: '2026-01-01T00:00:00Z', + }, + ]; + mount(); + + typeFilter('#9517'); + expect(rows()).toHaveLength(1); + expect(rows()[0].textContent).toContain('Fix CI'); + + typeFilter('#9999'); + expect(rows()).toHaveLength(0); + }); }); diff --git a/packages/web-shell/client/components/dialogs/ResumeDialog.tsx b/packages/web-shell/client/components/dialogs/ResumeDialog.tsx index c7da2f5f410..856fb5a2df7 100644 --- a/packages/web-shell/client/components/dialogs/ResumeDialog.tsx +++ b/packages/web-shell/client/components/dialogs/ResumeDialog.tsx @@ -5,6 +5,7 @@ import { useI18n } from '../../i18n'; import { useListboxKeyboard } from '../../hooks/useListboxKeyboard'; import { useFilterInput } from '../../hooks/useFilterInput'; import { SessionRow } from './SessionRow'; +import { sessionMatchesGitQuery } from '../sidebar/sessionSearch'; import { useScopedSessions } from '../../hooks/useScopedSessions'; interface ResumeDialogProps { @@ -43,7 +44,8 @@ export function ResumeDialog({ const q = filterQuery.toLowerCase(); return ( (s.displayName || '').toLowerCase().includes(q) || - s.sessionId.toLowerCase().includes(q) + s.sessionId.toLowerCase().includes(q) || + sessionMatchesGitQuery(s, q) ); }) : sessions; diff --git a/packages/web-shell/client/components/dialogs/SessionRow.test.tsx b/packages/web-shell/client/components/dialogs/SessionRow.test.tsx index 593a96b3b24..6f3ac95d2c4 100644 --- a/packages/web-shell/client/components/dialogs/SessionRow.test.tsx +++ b/packages/web-shell/client/components/dialogs/SessionRow.test.tsx @@ -57,6 +57,48 @@ describe('SessionRow', () => { expect(row().textContent).toContain('2'); }); + it('renders the bound PR badge (latest + overflow) without triggering row selection', () => { + const onClick = vi.fn(); + const withPrs = { + ...session, + prs: [ + { number: 9500, url: 'https://github.com/o/r/pull/9500' }, + { number: 9517, url: 'https://github.com/o/r/pull/9517' }, + ], + } as unknown as DaemonSessionSummary; + mount( + , + ); + const badge = row().querySelector( + 'a[href="https://github.com/o/r/pull/9517"]', + ); + expect(badge).not.toBeNull(); + expect(badge!.textContent).toBe('#9517 +1'); + act(() => { + badge!.click(); + }); + expect(onClick).not.toHaveBeenCalled(); + }); + + it('renders no badge when the session has no bound PR', () => { + mount( + , + ); + expect(row().querySelector('a')).toBeNull(); + }); + it('defaults aria-selected to `current` (not the roving highlight), explicit value wins', () => { // The roving highlight must NOT be announced as "selected" — per WAI-ARIA // it is conveyed by aria-activedescendant, while aria-selected marks the diff --git a/packages/web-shell/client/components/dialogs/SessionRow.tsx b/packages/web-shell/client/components/dialogs/SessionRow.tsx index d42e8b77644..87596b37de2 100644 --- a/packages/web-shell/client/components/dialogs/SessionRow.tsx +++ b/packages/web-shell/client/components/dialogs/SessionRow.tsx @@ -2,6 +2,7 @@ import { type ReactNode } from 'react'; import { type DaemonSessionSummary } from '@qwen-code/webui/daemon-react-sdk'; import { dp } from './dialogStyles'; import { useI18n } from '../../i18n'; +import { SessionPrBadge } from '../SessionPrBadge'; import { formatRelativeTime } from '../../utils/formatRelativeTime'; interface SessionRowProps { @@ -64,6 +65,7 @@ export function SessionRow({ }: SessionRowProps) { const { t } = useI18n(); const timestamp = session.updatedAt || session.createdAt; + const prs = session.prs ?? []; return (
{session.displayName || session.sessionId.slice(0, 8)} + {/* role="option" rows own roving-tabindex keyboard navigation, so + the badge must not steal a tab stop. */} + {trailing}
diff --git a/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx b/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx index 849e40dc21c..c102b4658c2 100644 --- a/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx +++ b/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx @@ -80,6 +80,60 @@ describe('SessionDetailsTooltip', () => { act(() => root.unmount()); }); + it('shows the bound pull request as a link', async () => { + vi.useFakeTimers(); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + + act(() => { + root.render( + + + + + , + ); + }); + + await openDetails(container); + + const details = document.querySelector('[role="dialog"]'); + expect(details?.textContent).toContain('Pull Request #9517'); + expect(details?.textContent).toContain('Pull Request #9500'); + const link = details?.querySelector( + 'a[href="https://github.com/o/r/pull/9517"]', + ); + expect(link).not.toBeNull(); + expect(link?.getAttribute('target')).toBe('_blank'); + // Latest binding listed first. + const links = details?.querySelectorAll('a[href*="/pull/"]'); + expect(links?.[0]?.getAttribute('href')).toBe( + 'https://github.com/o/r/pull/9517', + ); + // Non-http(s) bindings are dropped, matching the badge surface. + expect(details?.querySelector('a[href="javascript:alert(1)"]')).toBeNull(); + expect(details?.textContent).not.toContain('#9999'); + + act(() => root.unmount()); + }); + it('does not reopen after a row action opens its menu', async () => { vi.useFakeTimers(); const container = document.createElement('div'); diff --git a/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx b/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx index 394b4576df2..06b017d533c 100644 --- a/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx +++ b/packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx @@ -5,10 +5,13 @@ import { CopyIcon, FolderClosedIcon, GitBranchIcon, + GitPullRequestIcon, RadioTowerIcon, } from 'lucide-react'; import { useI18n } from '../../i18n'; +import { useExternalLinkOpener } from '../../hooks/useExternalLinkOpener'; import { writeClipboardText } from '../../utils/clipboard'; +import { isExternalOpenUrl } from '../../utils/externalOpen'; import { workspaceBasename } from '../../utils/workspace'; import { Popover, PopoverAnchor, PopoverContent } from '../ui/popover'; import styles from './WebShellSidebar.module.css'; @@ -30,6 +33,7 @@ export function SessionDetailsTooltip({ children, }: SessionDetailsTooltipProps) { const { t } = useI18n(); + const openExternalLink = useExternalLinkOpener(); const [open, setOpen] = useState(false); const [copyStatus, setCopyStatus] = useState<'idle' | 'copied' | 'failed'>( 'idle', @@ -140,6 +144,33 @@ export function SessionDetailsTooltip({ {branch}
)} + {[...(session.prs ?? [])] + .reverse() + .filter((pr) => isExternalOpenUrl(pr.url)) + .map((pr, index) => ( + // Index composite: a hand-edited sidecar can carry duplicate + // numbers (the reader validates shape, not uniqueness), and a + // duplicate key would reconcile rows against each other. The + // list is a stable per-snapshot order, so index keys are safe. + + ))}