-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(acp): isolate workspace settings and context file resolution for worktree sessions #8152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3046b94
79f2170
f8635ac
f1ac5e2
89df65e
731126b
76868ae
7af1462
c0f9b91
0099742
55696ea
ec57679
52a068c
959c362
a12c58b
18530dc
0c0512b
7d9c586
b5a8b2a
078e788
2b2bec8
18e2a98
10dad43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,13 @@ import { ACP_EVENT_LOOP_STALL_RESTART_MS } from '@qwen-code/channel-base'; | |||||||||||||
| const { mockRunExitCleanup } = vi.hoisted(() => ({ | ||||||||||||||
| mockRunExitCleanup: vi.fn().mockResolvedValue(undefined), | ||||||||||||||
| })); | ||||||||||||||
| const { mockExistsSync } = vi.hoisted(() => ({ | ||||||||||||||
| mockExistsSync: vi.fn().mockReturnValue(true), | ||||||||||||||
| })); | ||||||||||||||
| vi.mock('node:fs', async (importOriginal) => { | ||||||||||||||
| const actual = await importOriginal<typeof import('node:fs')>(); | ||||||||||||||
| return { ...actual, existsSync: mockExistsSync }; | ||||||||||||||
| }); | ||||||||||||||
| const { mockStartNonInteractiveOpenAILogHousekeeping } = vi.hoisted(() => ({ | ||||||||||||||
| mockStartNonInteractiveOpenAILogHousekeeping: vi.fn(), | ||||||||||||||
| })); | ||||||||||||||
|
|
@@ -1994,6 +2001,10 @@ describe('QwenAgent MCP SSE/HTTP support', () => { | |||||||||||||
| stdoutDestroySpy.mockRestore(); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| afterEach(() => { | ||||||||||||||
| mockExistsSync.mockReturnValue(true); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('initialize response includes mcpCapabilities with sse and http', async () => { | ||||||||||||||
| const mockSettings = { | ||||||||||||||
| merged: { mcpServers: {} }, | ||||||||||||||
|
|
@@ -4378,6 +4389,63 @@ describe('QwenAgent MCP SSE/HTTP support', () => { | |||||||||||||
| await agentPromise; | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('session/cd sets worktreeCwd and getCore resolves against the worktree path', async () => { | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-14: The new tests cannot establish real worktree state with plain Concrete cost: the PR's only test evidence for the #8138 fix can never exercise the isolation logic; once R3-1 is fixed these tests either stay red or get "fixed" in a way that bypasses the containment logic — leaving the mis-routing path without real coverage. Fix: build worktree-shaped fixtures — 中文说明[Suggestion] R3-14:新测试无法用普通 具体代价:本 PR 对 #8138 修复的唯一测试证据永远无法执行隔离逻辑;R3-1 修复后这些测试要么继续红,要么以绕过包含逻辑的方式被"修好"——错位路径失去真实覆盖。 修复:构造 worktree 形状的 fixture—— — qwen3.8-max via Qwen Code /review (v0.21.10) |
||||||||||||||
| const sessionId = '11111111-1111-1111-1111-111111111111'; | ||||||||||||||
| const tmpDir = await fs.mkdtemp( | ||||||||||||||
| path.join(os.tmpdir(), 'qwen-cd-worktree-settings-'), | ||||||||||||||
| ); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const targetDir = path.join(repoRoot, '.qwen', 'worktrees', 'test'); | ||||||||||||||
| await fs.mkdir(targetDir, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(targetDir, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const canonicalTargetDir = await fs.realpath(targetDir); | ||||||||||||||
| const innerConfig = await setupSessionMocks(sessionId); | ||||||||||||||
| const relocateWorkingDirectory = vi.fn().mockResolvedValue({}); | ||||||||||||||
| Object.assign(innerConfig, { | ||||||||||||||
| getTargetDir: vi.fn().mockReturnValue('/tmp'), | ||||||||||||||
| isRestrictiveSandbox: vi.fn().mockReturnValue(false), | ||||||||||||||
| relocateWorkingDirectory, | ||||||||||||||
| }); | ||||||||||||||
| Object.assign(innerConfig.getGeminiClient(), { | ||||||||||||||
| addWorkingDirectoryChangedContext: vi.fn().mockResolvedValue(undefined), | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
|
|
||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
| await agent.newSession({ cwd: '/tmp', mcpServers: [] }); | ||||||||||||||
|
|
||||||||||||||
| try { | ||||||||||||||
| await expect( | ||||||||||||||
| agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionCd, { | ||||||||||||||
| sessionId, | ||||||||||||||
| path: targetDir, | ||||||||||||||
| }), | ||||||||||||||
| ).resolves.toMatchObject({ | ||||||||||||||
| previousCwd: '/tmp', | ||||||||||||||
| newCwd: canonicalTargetDir, | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| expect( | ||||||||||||||
| (lastSessionMock as Record<string, unknown>)?.['worktreeCwd'], | ||||||||||||||
| ).toBe(canonicalTargetDir); | ||||||||||||||
|
Comment on lines
+4434
to
+4436
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The new tests cannot establish real worktree state with plain
Suggested change
(build worktree-shaped fixtures — 中文说明[Suggestion] 新测试无法用普通 修复:构造 worktree 形状的 fixture( — qwen3.8-max via Qwen Code /review (v0.21.9) |
||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', {}); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(canonicalTargetDir); | ||||||||||||||
| } finally { | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('allows a private Live managed relocation without a global folder trust rule', async () => { | ||||||||||||||
| await withEmptyTrustedFolders(async (directory) => { | ||||||||||||||
| const root = path.join(directory, 'Conversations'); | ||||||||||||||
|
|
@@ -9899,6 +9967,197 @@ describe('QwenAgent MCP SSE/HTTP support', () => { | |||||||||||||
| await agentPromise; | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/settings handlers resolve against worktree cwd set by createAndStoreSession', async () => { | ||||||||||||||
| const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-wt-s1-')); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const WORKTREE_DIR = path.join(repoRoot, '.qwen', 'worktrees', 'test'); | ||||||||||||||
| await fs.mkdir(WORKTREE_DIR, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(WORKTREE_DIR, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const innerConfig = await setupSessionMocks('wt-settings-session'); | ||||||||||||||
|
Comment on lines
+9970
to
+9977
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] No test replays the issue's actual end-to-end shape: 中文说明[Suggestion] 没有测试复现 issue 的实际端到端形态:会话中调用 — qwen3.8-max-preview via Qwen Code /review (v0.21.3) |
||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
|
|
||||||||||||||
| await agent.newSession({ cwd: '/fake/project', mcpServers: [] }); | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', {}); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue(process.cwd()); | ||||||||||||||
| innerConfig.getSessionId = vi.fn().mockReturnValue('regular-session'); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| await agent.newSession({ cwd: '/fake/project', mcpServers: [] }); | ||||||||||||||
|
|
||||||||||||||
| // The fallback loop scans all sessions for worktreeCwd; the first | ||||||||||||||
| // session's worktreeCwd is still set, so it wins over process.cwd(). | ||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', {}); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/settings handlers fall back to process.cwd() after worktree session closes', async () => { | ||||||||||||||
| const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-wt-s2-')); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const WORKTREE_DIR = path.join(repoRoot, '.qwen', 'worktrees', 'test'); | ||||||||||||||
| await fs.mkdir(WORKTREE_DIR, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(WORKTREE_DIR, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const innerConfig = await setupSessionMocks('wt-close-session'); | ||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
|
|
||||||||||||||
| await agent.newSession({ cwd: '/fake/project', mcpServers: [] }); | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', {}); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| await agent.extMethod('qwen/control/session/close', { | ||||||||||||||
| sessionId: 'wt-close-session', | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', {}); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(process.cwd()); | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/settings/getCore resolves per-session worktreeCwd via sessionId param', async () => { | ||||||||||||||
| const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-wt-s3-')); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const WORKTREE_DIR = path.join(repoRoot, '.qwen', 'worktrees', 'test'); | ||||||||||||||
| await fs.mkdir(WORKTREE_DIR, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(WORKTREE_DIR, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const innerConfig = await setupSessionMocks('per-session-wt-id'); | ||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
|
|
||||||||||||||
| const newResult = (await agent.newSession({ | ||||||||||||||
| cwd: '/fake/project', | ||||||||||||||
| mcpServers: [], | ||||||||||||||
| })) as { sessionId: string }; | ||||||||||||||
| const sessionId = newResult.sessionId; | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', { sessionId }); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(WORKTREE_DIR); | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/settings/getCore falls through to configWt when worktreeCwd dir is deleted', async () => { | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This test is named "falls through to configWt" but never exercises the it('qwen/settings/getCore uses configWt when worktreeCwd is deleted but activeWorktree is set', async () => {
const ACTIVE_WT = '/fake/active-wt';
const innerConfig = await setupSessionMocks('deleted-wt-session');
innerConfig.getTargetDir = vi.fn().mockReturnValue('/fake/deleted-wt');
(innerConfig as Record<string, unknown>)['getActiveWorktree'] = vi
.fn()
.mockReturnValue(ACTIVE_WT);
mockExistsSync.mockReturnValue(false); // session.worktreeCwd dir is gone
// ... call extMethod('qwen/settings/getCore', { sessionId }) and assert
// loadSettings was called with ACTIVE_WT (not process.cwd())
});中文说明[Suggestion] 此测试名为 “falls through to configWt”,却从未真正执行 — qwen3.8-max-preview via Qwen Code /review |
||||||||||||||
| const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-wt-s4-')); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const WORKTREE_DIR = path.join(repoRoot, '.qwen', 'worktrees', 'deleted'); | ||||||||||||||
| await fs.mkdir(WORKTREE_DIR, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(WORKTREE_DIR, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const innerConfig = await setupSessionMocks('deleted-wt-session'); | ||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue(WORKTREE_DIR); | ||||||||||||||
| (innerConfig as Record<string, unknown>)['getActiveWorktree'] = vi | ||||||||||||||
| .fn() | ||||||||||||||
| .mockReturnValue(null); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
|
|
||||||||||||||
| const newResult = (await agent.newSession({ | ||||||||||||||
| cwd: '/fake/project', | ||||||||||||||
| mcpServers: [], | ||||||||||||||
| })) as { sessionId: string }; | ||||||||||||||
| const sessionId = newResult.sessionId; | ||||||||||||||
|
|
||||||||||||||
| await fs.rm(WORKTREE_DIR, { recursive: true, force: true }); | ||||||||||||||
| try { | ||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', { sessionId }); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(process.cwd()); | ||||||||||||||
| } finally { | ||||||||||||||
| mockExistsSync.mockReturnValue(true); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/settings/getCore uses configWt when worktreeCwd is deleted but activeWorktree exists', async () => { | ||||||||||||||
| const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-wt-s5-')); | ||||||||||||||
| const repoRoot = path.join(tmpDir, 'repo'); | ||||||||||||||
| const ACTIVE_WT = path.join(repoRoot, '.qwen', 'worktrees', 'active'); | ||||||||||||||
| await fs.mkdir(ACTIVE_WT, { recursive: true }); | ||||||||||||||
| await fs.mkdir(path.join(repoRoot, '.git')); | ||||||||||||||
| await fs.writeFile(path.join(ACTIVE_WT, '.git'), 'gitdir: /fake'); | ||||||||||||||
| const innerConfig = await setupSessionMocks('active-wt-session'); | ||||||||||||||
| innerConfig.getTargetDir = vi.fn().mockReturnValue('/fake/deleted-wt'); | ||||||||||||||
| (innerConfig as Record<string, unknown>)['getActiveWorktree'] = vi | ||||||||||||||
| .fn() | ||||||||||||||
| .mockReturnValue(ACTIVE_WT); | ||||||||||||||
|
|
||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue( | ||||||||||||||
| innerConfig as unknown as Config, | ||||||||||||||
| ); | ||||||||||||||
| const { agent, agentPromise } = await bootAcpAgent(); | ||||||||||||||
|
|
||||||||||||||
| const newResult = (await agent.newSession({ | ||||||||||||||
| cwd: '/fake/project', | ||||||||||||||
| mcpServers: [], | ||||||||||||||
| })) as { sessionId: string }; | ||||||||||||||
| const sessionId = newResult.sessionId; | ||||||||||||||
|
|
||||||||||||||
| mockExistsSync.mockImplementation( | ||||||||||||||
| (p: string | Buffer | URL) => p === ACTIVE_WT, | ||||||||||||||
| ); | ||||||||||||||
| try { | ||||||||||||||
| vi.mocked(loadSettings).mockClear(); | ||||||||||||||
| await agent.extMethod('qwen/settings/getCore', { sessionId }); | ||||||||||||||
| expect(vi.mocked(loadSettings)).toHaveBeenCalledWith(ACTIVE_WT); | ||||||||||||||
| } finally { | ||||||||||||||
| mockExistsSync.mockReturnValue(true); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| mockConnectionState.resolve(); | ||||||||||||||
| await agentPromise; | ||||||||||||||
| await fs.rm(tmpDir, { recursive: true, force: true }); | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| it('qwen/permissions/setRules validates scope and ruleType', async () => { | ||||||||||||||
| const settings = makeCoreSettings(); | ||||||||||||||
| const { agent, agentPromise } = await bootCoreSettingsAgent(settings); | ||||||||||||||
|
|
@@ -17948,12 +18207,22 @@ describe('sessionLanguage multi-session propagation', () => { | |||||||||||||
| getUserHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| getProjectHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| } as unknown as LoadedSettings; | ||||||||||||||
| const reloadedSettings = { | ||||||||||||||
| merged: { modelProviders: providerConfig }, | ||||||||||||||
| getUserHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| getProjectHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| } as unknown as LoadedSettings; | ||||||||||||||
| const cfg = makeConfig({ | ||||||||||||||
| getSessionId: vi.fn().mockReturnValue('s-reload'), | ||||||||||||||
| getAuthType: vi.fn().mockReturnValue('openai'), | ||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadSettings).mockImplementation((...args) => { | ||||||||||||||
| if (args[1] && (args[1] as Record<string, unknown>).skipLoadEnvironment) { | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R5-1: PR-added test code accesses an index-signature property with dot notation, violating the repo's pre-existing Fix (both lines 18221 and 18302): if (args[1] && (args[1] as Record<string, unknown>)['skipLoadEnvironment']) {中文说明[Critical] R5-1:本 PR 新增的测试代码用点号访问索引签名属性,违反仓库既有的 修复(第 18221 与 18302 两处): if (args[1] && (args[1] as Record<string, unknown>)['skipLoadEnvironment']) {— qwen3.8-max via Qwen Code /review (v0.21.10) |
||||||||||||||
| return reloadedSettings; | ||||||||||||||
| } | ||||||||||||||
| return settings; | ||||||||||||||
| }); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue(cfg as unknown as Config); | ||||||||||||||
| vi.mocked(Session).mockImplementation( | ||||||||||||||
| () => | ||||||||||||||
|
|
@@ -18012,6 +18281,11 @@ describe('sessionLanguage multi-session propagation', () => { | |||||||||||||
| getUserHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| getProjectHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| } as unknown as LoadedSettings; | ||||||||||||||
| const reloadedSettings = { | ||||||||||||||
| merged: { tools: { approvalMode: 'plan' } }, | ||||||||||||||
| getUserHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| getProjectHooks: vi.fn().mockReturnValue({}), | ||||||||||||||
| } as unknown as LoadedSettings; | ||||||||||||||
| let approvalMode = 'default'; | ||||||||||||||
| const cfg = makeConfig({ | ||||||||||||||
| getSessionId: vi.fn().mockReturnValue('s-plan-reload'), | ||||||||||||||
|
|
@@ -18024,7 +18298,12 @@ describe('sessionLanguage multi-session propagation', () => { | |||||||||||||
| const clearActiveTodoPlanRevision = vi.fn(); | ||||||||||||||
| const clearTodoStopGuardTrust = vi.fn(); | ||||||||||||||
|
|
||||||||||||||
| vi.mocked(loadSettings).mockReturnValue(settings); | ||||||||||||||
| vi.mocked(loadSettings).mockImplementation((...args) => { | ||||||||||||||
| if (args[1] && (args[1] as Record<string, unknown>).skipLoadEnvironment) { | ||||||||||||||
| return reloadedSettings; | ||||||||||||||
| } | ||||||||||||||
| return settings; | ||||||||||||||
| }); | ||||||||||||||
| vi.mocked(loadCliConfig).mockResolvedValue(cfg as unknown as Config); | ||||||||||||||
| vi.mocked(Session).mockImplementation( | ||||||||||||||
| () => | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The
existsSyncstub is installed file-wide (viavi.mock('node:fs')) and defaults totruefor the entire 17,507-line test file, with no per-test reset. Two consequences: (1) everyexistsSynccall in every unrelated test now reports every path as existing, so a regression that breaks missing-file handling would pass silently here; (2) the two worktree tests that override the mock inline (~9236, ~9268) restore it only as their last statement — not intry/finallyorafterEach— so if an assertion throws mid-block, the override leaks into every later test in the file (vitest runs in file order), producing a cascade of misleading failures or masking a real regression. The diff already usestry/finallyfor cleanup in thesession/cdtest (~4068), so the safer pattern is established. Suggested direction:中文说明
[Suggestion]
existsSync桩通过vi.mock('node:fs')被安装为全文件生效,并在整个 17,507 行的测试文件中默认返回true,且没有按测试重置。两个后果:(1) 每个无关测试中的existsSync调用现在都报告所有路径都存在,因此破坏"文件缺失处理"的回归在此处会静默通过;(2) 两个在内联覆盖该 mock 的 worktree 测试(约 9236、约 9268)只在其最后一条语句恢复它——而非在try/finally或afterEach中——所以若某个断言在块中途抛出,该覆盖会泄漏到文件中所有后续测试(vitest 按文件顺序运行),产生一连串误导性失败或掩盖真实回归。本 diff 在session/cd测试(约 4068)中已使用try/finally做清理,因此更安全的模式已经确立。修复方向:默认调用真实实现,仅在需要的测试内 stub true/false,并配合afterEach重置或try/finally。— qwen3.8-max-preview via Qwen Code /review