From 02d84bcab0e490138e3710af7f9391d77296c98b Mon Sep 17 00:00:00 2001 From: claude-code-best Date: Sat, 20 Jun 2026 12:09:11 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20listSessions=20=E4=B8=A5=E6=A0=BC?= =?UTF-8?q?=E6=8C=89=20cwd=20=E8=BF=87=E6=BB=A4=E5=B9=B6=E7=A7=BB=E9=99=A4?= =?UTF-8?q?=20session/load=20=E8=BF=87=E4=B8=A5=E6=A0=A1=E9=AA=8C?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - listSessions: 客户端省略 cwd 时回退到 getOriginalCwd(),并对每个候选会话的 存储 cwd 做 canonicalizePath 规范化后与请求 cwd 严格匹配,确保只返回真正属 于当前工作区的会话(符合 session-list.mdx "Only sessions with a matching cwd are returned") - sessionLifecycle: 移除 getOrCreateSession 中审计 2.2 添加的 cwd 一致性校验, 它会拒绝 resolveSessionFilePath worktree fallback 找到的合法会话加载 - 补充 listSessions 的 5 个测试用例覆盖 cwd 透传/fallback/分页拒绝/无 cwd 过滤 Co-Authored-By: glm-5.2 --- src/services/acp/__tests__/agent.test.ts | 67 +++++++++++++++++++++- src/services/acp/agent/AcpAgent.ts | 26 ++++++++- src/services/acp/agent/sessionLifecycle.ts | 31 ++-------- 3 files changed, 96 insertions(+), 28 deletions(-) diff --git a/src/services/acp/__tests__/agent.test.ts b/src/services/acp/__tests__/agent.test.ts index 141e16493..365921c25 100644 --- a/src/services/acp/__tests__/agent.test.ts +++ b/src/services/acp/__tests__/agent.test.ts @@ -71,10 +71,13 @@ mockModulePreservingExports('../../../utils/config.ts', { const mockSwitchSession = mock(() => {}) +const mockGetOriginalCwd = mock(() => '/current/working/dir') mockModulePreservingExports('../../../bootstrap/state.ts', { setOriginalCwd: mock(() => {}), switchSession: mockSwitchSession, addSlowOperation: mock(() => {}), + getOriginalCwd: mockGetOriginalCwd, + getSessionProjectDir: mock(() => null), }) const mockGetDefaultAppState = mock(() => ({ @@ -116,8 +119,9 @@ mockModulePreservingExports('../bridge.ts', { })), }) +const mockListSessionsImpl = mock(async () => []) mockModulePreservingExports('../../../utils/listSessionsImpl.ts', { - listSessionsImpl: mock(async () => []), + listSessionsImpl: mockListSessionsImpl, }) const mockResolveSessionFilePath = mock(async () => ({ @@ -241,6 +245,10 @@ describe('AcpAgent', () => { mockGetDefaultAppState.mockClear() mockGetSettings.mockReset() mockGetSettings.mockImplementation(() => ({})) + mockListSessionsImpl.mockReset() + mockListSessionsImpl.mockImplementation(async () => []) + mockGetOriginalCwd.mockReset() + mockGetOriginalCwd.mockImplementation(() => '/current/working/dir') ;(forwardSessionUpdates as ReturnType).mockReset() ;(forwardSessionUpdates as ReturnType).mockImplementation( async () => ({ stopReason: 'end_turn' as const }), @@ -1320,6 +1328,63 @@ describe('AcpAgent', () => { }) }) + describe('listSessions', () => { + test('passes params.cwd through to listSessionsImpl when provided', async () => { + const agent = new AcpAgent(makeConn()) + await agent.listSessions({ cwd: '/explicit/path' } as any) + expect(mockListSessionsImpl).toHaveBeenCalledWith({ + dir: '/explicit/path', + }) + }) + + test('falls back to current working dir when client omits cwd', async () => { + // Standard clients (Goose, possibly others) call session/list with + // empty params. Without a fallback, listSessionsImpl treats undefined + // dir as "all projects" and returns every session on disk. + mockGetOriginalCwd.mockImplementation(() => '/active/project') + const agent = new AcpAgent(makeConn()) + await agent.listSessions({} as any) + expect(mockListSessionsImpl).toHaveBeenCalledWith({ + dir: '/active/project', + }) + }) + + test('falls back to current working dir when client sends null cwd', async () => { + mockGetOriginalCwd.mockImplementation(() => '/active/project') + const agent = new AcpAgent(makeConn()) + await agent.listSessions({ cwd: null } as any) + expect(mockListSessionsImpl).toHaveBeenCalledWith({ + dir: '/active/project', + }) + }) + + test('rejects client-supplied cursor (pagination not implemented)', async () => { + const agent = new AcpAgent(makeConn()) + await expect( + agent.listSessions({ cursor: 'page2' } as any), + ).rejects.toThrow(/Pagination cursor not supported/) + }) + + test('filters out candidates without a cwd field', async () => { + mockListSessionsImpl.mockImplementation( + async () => + [ + { + sessionId: 'with-cwd', + cwd: '/p', + summary: 'Has cwd', + lastModified: 0, + }, + { sessionId: 'no-cwd', summary: 'No cwd', lastModified: 0 }, + ] as any, + ) + const agent = new AcpAgent(makeConn()) + const res = await agent.listSessions({ cwd: '/p' } as any) + expect(res.sessions).toHaveLength(1) + expect(res.sessions[0].sessionId).toBe('with-cwd') + }) + }) + describe('sessionId alignment with global state', () => { test('newSession calls switchSession with the generated sessionId', async () => { const agent = new AcpAgent(makeConn()) diff --git a/src/services/acp/agent/AcpAgent.ts b/src/services/acp/agent/AcpAgent.ts index c57350a74..757d8a925 100644 --- a/src/services/acp/agent/AcpAgent.ts +++ b/src/services/acp/agent/AcpAgent.ts @@ -49,7 +49,11 @@ import { unlink } from 'node:fs/promises' import type { Message } from '../../../types/message.js' import { sanitizeTitle } from '../utils.js' import { listSessionsImpl } from '../../../utils/listSessionsImpl.js' -import { resolveSessionFilePath } from '../../../utils/sessionStoragePortable.js' +import { + resolveSessionFilePath, + canonicalizePath, +} from '../../../utils/sessionStoragePortable.js' +import { getOriginalCwd } from '../../../bootstrap/state.js' import type { AcpSession } from './sessionTypes.js' // ── Agent class ─────────────────────────────────────────────────── @@ -190,13 +194,31 @@ export class AcpAgent implements Agent { ) } + // Resolve the effective cwd: client-provided wins, fall back to the + // agent's current working directory (set by the most recent session/new + // or session/load). Standard ACP clients (e.g. Goose) call session/list + // with empty params and no cwd — without a fallback, listSessionsImpl + // treats undefined dir as "all projects" and returns every session on + // disk, which is unrelated to the workspace the user actually has open. + const requestedCwd = params.cwd || getOriginalCwd() + const canonicalRequested = await canonicalizePath(requestedCwd) + const candidates = await listSessionsImpl({ - dir: params.cwd ?? undefined, + dir: requestedCwd, }) const sessions = [] for (const candidate of candidates) { if (!candidate.cwd) continue + // Per session-list.mdx: "Only sessions with a matching cwd are + // returned." listSessionsImpl filters by which project directory + // the file lives in, but a project directory can hold sessions + // whose stored cwd points elsewhere (e.g. a session created in + // env_A whose file ended up in the parent repo's project dir via + // session/load's worktree fallback). Apply a strict canonical-cwd + // filter so the list reflects what the spec promises. + const canonicalCandidate = await canonicalizePath(candidate.cwd) + if (canonicalCandidate !== canonicalRequested) continue // Only include title when non-empty; schema allows null/omitted title. const title = sanitizeTitle(candidate.summary ?? '') sessions.push({ diff --git a/src/services/acp/agent/sessionLifecycle.ts b/src/services/acp/agent/sessionLifecycle.ts index 50d23d332..d0ab0edee 100644 --- a/src/services/acp/agent/sessionLifecycle.ts +++ b/src/services/acp/agent/sessionLifecycle.ts @@ -9,7 +9,6 @@ */ import { type UUID } from 'node:crypto' import { dirname } from 'node:path' -import * as path from 'node:path' import type { NewSessionRequest, NewSessionResponse, @@ -22,11 +21,7 @@ import { setOriginalCwd, switchSession } from '../../../bootstrap/state.js' import type { SessionId } from '../../../types/ids.js' import { replayHistoryMessages } from '../bridge.js' import { computeSessionFingerprint } from '../utils.js' -import { - resolveSessionFilePath, - readSessionLite, - extractJsonStringField, -} from '../../../utils/sessionStoragePortable.js' +import { resolveSessionFilePath } from '../../../utils/sessionStoragePortable.js' import { AcpAgent } from './AcpAgent.js' import type { AcpSession } from './sessionTypes.js' import { isPermissionMode } from './permissionMode.js' @@ -89,28 +84,14 @@ async function getOrCreateSession( await this.teardownSession(params.sessionId) } - // Locate the session file by sessionId across all project directories. - // params.cwd may not match the project directory where the session was - // originally created (e.g. client sends a subdirectory path), so we - // search by sessionId first and fall back to cwd-based lookup. + // Locate the session file by sessionId. resolveSessionFilePath searches + // the requested cwd's project dir first, then falls back to sibling git + // worktrees — sessions created inside a repo (including from subdirectories + // or ephemeral test envs nested in the repo) all persist under the same + // parent project dir. const resolved = await resolveSessionFilePath(params.sessionId, params.cwd) const projectDir = resolved ? dirname(resolved.filePath) : null - // Per session-setup.mdx "Working Directory": the cwd MUST be the absolute - // path used for the session regardless of where the Agent was spawned. - // Reject cross-project loads where the persisted session's original cwd - // does not match the requested cwd, otherwise the client could load a - // session belonging to project B while passing project A's cwd. - if (resolved) { - const lite = await readSessionLite(resolved.filePath) - const originalCwd = lite && extractJsonStringField(lite.head, 'cwd') - if (originalCwd && path.resolve(originalCwd) !== path.resolve(params.cwd)) { - throw new Error( - `Session cwd mismatch: session belongs to ${originalCwd}, requested ${params.cwd}`, - ) - } - } - switchSession(params.sessionId as SessionId, projectDir) setOriginalCwd(params.cwd)