From b4fb1e500e10aa263e591ba439cea77f0b7d7f48 Mon Sep 17 00:00:00 2001 From: tomsen-ai <230283659+tomsen-ai@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:19:23 +0800 Subject: [PATCH 1/3] fix(cli): resolve session-management settings per request, not from the stale this.settings cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a multi-workspace daemon, unstable_listSessions, deleteSession and the dead-session renameSession path derived the runtime output root from this.settings — the agent-level "latest loaded" cache — so after another workspace's activity they operated on the wrong runtime root: listSessions returned an empty/foreign list, deleteSession silently no-oped (or could remove a stale same-id copy under the wrong root), and renameSession mis-targeted the same way. The misrouting persists until the requesting workspace's settings happen to be reloaded. Load settings per request with loadSettingsCached(cwd), mirroring the pattern the sessionTranscript and session-restore handlers in the same file already use. Follow-up to #6292, which fixed this class on the session-creation entry points and deliberately left this.settings in place for workspace-neutral agent-level readers — these three handlers were misfiled in that bucket. Fixes #10094. Verification: three regression tests assert each handler passes the per-request settings instance (mutation-verified: reverting a call site to this.settings fails its test); acpAgent.test.ts 486/486; typecheck, ESLint and Prettier clean. Real-filesystem probe in the issue. Co-Authored-By: Claude Fable 5 --- .../cli/src/acp-integration/acpAgent.test.ts | 95 +++++++++++++++++++ packages/cli/src/acp-integration/acpAgent.ts | 17 +++- 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index f6d54d52ee7..63958871e38 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -1005,6 +1005,7 @@ import type { } from '@agentclientprotocol/sdk'; import { AgentSideConnection, RequestError } from '@agentclientprotocol/sdk'; import { loadSettings, SettingScope } from '../config/settings.js'; +import { runWithAcpRuntimeOutputDir } from './runtimeOutputDirContext.js'; import { resetTrustedFoldersForTesting } from '../config/trustedFolders.js'; import { MAX_PERMISSION_RULE_LENGTH, @@ -16312,6 +16313,100 @@ describe('QwenAgent extMethod renameSession routing', () => { await agentPromise; }); + it('resolves deleteSession settings per request, not from the this.settings cache', async () => { + const innerConfig = makeLiveSessionInnerConfig(null); + const { agent, agentPromise } = await bootAgent(innerConfig); + + // Multi-workspace daemon shape: this.settings holds the boot workspace's + // settings; the delete targets a cwd whose own settings (and therefore + // advanced.runtimeOutputDir) resolve differently. Routing through the + // stale cache would scan the wrong runtime root. + const perRequestSettings = makeAcpSettings(); + vi.mocked(loadSettings).mockReturnValue(perRequestSettings); + const removeSession = vi.fn().mockResolvedValue(true); + vi.mocked(SessionService).mockImplementation( + () => + ({ removeSession }) as unknown as InstanceType, + ); + vi.mocked(runWithAcpRuntimeOutputDir).mockClear(); + + await agent.extMethod('deleteSession', { + cwd: '/tmp/workspace-a', + sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + }); + + expect(runWithAcpRuntimeOutputDir).toHaveBeenCalledTimes(1); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( + perRequestSettings, + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('resolves dead-session renameSession settings per request, not from the this.settings cache', async () => { + const innerConfig = makeLiveSessionInnerConfig(null); + const { agent, agentPromise } = await bootAgent(innerConfig); + + const perRequestSettings = makeAcpSettings(); + vi.mocked(loadSettings).mockReturnValue(perRequestSettings); + const renameSession = vi.fn().mockResolvedValue(true); + vi.mocked(SessionService).mockImplementation( + () => + ({ renameSession }) as unknown as InstanceType, + ); + vi.mocked(runWithAcpRuntimeOutputDir).mockClear(); + + // No newSession: the target is not live in this process, so the rename + // takes the disk-only SessionService path. + await agent.extMethod('renameSession', { + cwd: '/tmp/workspace-a', + sessionId: '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + title: 'renamed elsewhere', + }); + + expect(renameSession).toHaveBeenCalledWith( + '6ba7b810-9dad-11d1-80b4-00c04fd430c8', + 'renamed elsewhere', + ); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( + perRequestSettings, + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + + it('resolves unstable_listSessions settings per request, not from the this.settings cache', async () => { + const innerConfig = makeLiveSessionInnerConfig(null); + const { agent, agentPromise } = await bootAgent(innerConfig); + + const perRequestSettings = makeAcpSettings(); + vi.mocked(loadSettings).mockReturnValue(perRequestSettings); + const listSessions = vi + .fn() + .mockResolvedValue({ items: [], nextCursor: undefined }); + vi.mocked(SessionService).mockImplementation( + () => + ({ listSessions }) as unknown as InstanceType, + ); + vi.mocked(runWithAcpRuntimeOutputDir).mockClear(); + + await ( + agent as unknown as { + unstable_listSessions: (p: Record) => Promise; + } + ).unstable_listSessions({ cwd: '/tmp/workspace-a' }); + + expect(listSessions).toHaveBeenCalled(); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( + perRequestSettings, + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('returns success=false when the live ChatRecordingService rejects the title (I/O error)', async () => { const recording = makeRecordingService(); recording.recordCustomTitle.mockResolvedValue(false); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 80f4053bac1..510e622c4f4 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -5274,7 +5274,12 @@ class QwenAgent implements Agent { // (same pattern filesystem.ts uses for `_meta.bom` / `_meta.encoding`). const size = normalizeAcpSessionListSize(params._meta?.['size']); - const result = await runWithAcpRuntimeOutputDir(this.settings, cwd, () => { + // Per-request settings: `this.settings` is a "latest loaded" cache, so in + // a multi-workspace daemon it may hold another workspace's + // advanced.runtimeOutputDir and this listing would scan the wrong runtime + // root (returning an empty/foreign list for this cwd). + const settings = loadSettingsCached(cwd); + const result = await runWithAcpRuntimeOutputDir(settings, cwd, () => { const sessionService = new SessionService(cwd); return sessionService.listSessions({ cursor: numericCursor, @@ -10736,8 +10741,13 @@ class QwenAgent implements Agent { 'Invalid or missing sessionId', ); } + // Per-request settings, not the "latest loaded" this.settings cache: + // another workspace's advanced.runtimeOutputDir would point this + // destructive lookup at the wrong runtime root — silently returning + // success:false for a session that exists, or deleting a stale + // same-id copy under the wrong root. const success = await runWithAcpRuntimeOutputDir( - this.settings, + loadSettingsCached(cwd), cwd, async () => { const sessionService = new SessionService(cwd); @@ -10786,8 +10796,9 @@ class QwenAgent implements Agent { const ok = await liveRecording.recordCustomTitle(title, 'manual'); return { success: ok }; } + // Per-request settings for the same reason as deleteSession above. const success = await runWithAcpRuntimeOutputDir( - this.settings, + loadSettingsCached(cwd), cwd, async () => { const sessionService = new SessionService(cwd); From 88b21883f6af84391798ade9a3a9b27e4fac3d35 Mon Sep 17 00:00:00 2001 From: tomsen-ai <230283659+tomsen-ai@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:39:51 +0800 Subject: [PATCH 2/3] chore: retrigger review after template-conformant PR body The round-1 gate blocked solely on PR-body template structure; the body now carries Reviewer Test Plan (How to verify / Evidence / Tested on) and Linked Issues. No code changes. From addeb5642238b3d567839f5f01a822c7887ee28d Mon Sep 17 00:00:00 2001 From: tomsen-ai <230283659+tomsen-ai@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:22:00 +0800 Subject: [PATCH 3/3] test(cli): pin the per-request cwd in the session-management settings tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-1 R1-1: the three regression tests pinned the settings object but not the directory it was resolved from or routed to — a regression that resolves settings from the boot workspace's dir (or drifts the routed cwd) would pass all three. Each test now also asserts loadSettings('/tmp/workspace-a') and runWithAcpRuntimeOutputDir's cwd argument, per the review's suggestion. Mutation-verified: rerouting deleteSession through this.config.getTargetDir() fails its test. Co-Authored-By: Claude Fable 5 --- packages/cli/src/acp-integration/acpAgent.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 63958871e38..61a2a5ed846 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -16339,6 +16339,13 @@ describe('QwenAgent extMethod renameSession routing', () => { expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( perRequestSettings, ); + // Pin the per-request cwd too: settings must be resolved FROM the + // request's directory and routing must target it — not the boot + // workspace's dir. + expect(loadSettings).toHaveBeenCalledWith('/tmp/workspace-a'); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![1]).toBe( + '/tmp/workspace-a', + ); mockConnectionState.resolve(); await agentPromise; @@ -16372,6 +16379,10 @@ describe('QwenAgent extMethod renameSession routing', () => { expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( perRequestSettings, ); + expect(loadSettings).toHaveBeenCalledWith('/tmp/workspace-a'); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![1]).toBe( + '/tmp/workspace-a', + ); mockConnectionState.resolve(); await agentPromise; @@ -16402,6 +16413,10 @@ describe('QwenAgent extMethod renameSession routing', () => { expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![0]).toBe( perRequestSettings, ); + expect(loadSettings).toHaveBeenCalledWith('/tmp/workspace-a'); + expect(vi.mocked(runWithAcpRuntimeOutputDir).mock.calls[0]![1]).toBe( + '/tmp/workspace-a', + ); mockConnectionState.resolve(); await agentPromise;