From 92d71e6b87ea43c2baa77ed7561c91e979b6d706 Mon Sep 17 00:00:00 2001 From: JATMN Date: Thu, 22 Oct 2026 05:44:44 -0700 Subject: [PATCH] test: restore leaked settings, config and path state across test files (TA-001 slice 3) (#2331) * test: restore leaked settings, config-home and path state in single-process runs (TA-001 slice 3) * test(printMaxTurns): release shared mutation lock if setup throws --- src/cli/printMaxTurns.test.ts | 91 +++++++++++------- src/components/TrustDialog/utils.test.ts | 30 +++--- src/services/mcp/config.test.ts | 95 +++++++++++-------- src/utils/auth.test.ts | 25 ++--- src/utils/hookChains.integration.test.ts | 9 +- src/utils/modelCost.modelGate.test.ts | 11 ++- src/utils/openclaudePaths.test.ts | 71 ++++++-------- src/utils/providerProfile.test.ts | 8 ++ .../sessionStorage.commandOnlySession.test.ts | 51 ++++++---- .../sessionStorage.deletedSession.test.ts | 50 ++++++---- 10 files changed, 251 insertions(+), 190 deletions(-) diff --git a/src/cli/printMaxTurns.test.ts b/src/cli/printMaxTurns.test.ts index 8f1855e1a..919ca0745 100644 --- a/src/cli/printMaxTurns.test.ts +++ b/src/cli/printMaxTurns.test.ts @@ -1,5 +1,10 @@ import { afterEach, describe, expect, spyOn, test } from 'bun:test' +import { + acquireSharedMutationLock, + releaseSharedMutationLock, +} from '../test/sharedMutationLock.js' +import { settingsChangeDetector } from '../utils/settings/changeDetector.js' import type { Command } from '../commands.js' import type { SDKMessage } from '../entrypoints/agentSdkTypes.js' import * as queryEngineModule from '../QueryEngine.js' @@ -170,45 +175,61 @@ describe('headless --print max-turns', () => { }) test('forwards maxTurns 0 through runHeadless into ask()', async () => { - process.env.CLAUDE_CODE_SIMPLE = '1' - - const shutdownSpy = spyOn(shutdownModule, 'gracefulShutdownSync').mockImplementation(() => {}) - const stdoutSpy = spyOn(processModule, 'writeToStdout').mockImplementation( - () => {}, - ) - const askSpy = spyOn(queryEngineModule, 'ask').mockImplementation( - async function* () { - yield createHeadlessSuccessResult() - }, - ) - - let state = getDefaultAppState() - const getAppState = () => state - const setAppState = (update: (previous: AppState) => AppState) => { - state = update(state) - } - - let runPromise: Promise | undefined + await acquireSharedMutationLock('cli/printMaxTurns.test.ts') try { - runPromise = runHeadless( - 'headless prompt', - getAppState, - setAppState, - [] as Command[], - [] as Tools, - {}, - [], - createHeadlessRunOptions(0), + const unsubscribes: Array<() => void> = [] + const realSubscribe = settingsChangeDetector.subscribe + const subscribeSpy = spyOn(settingsChangeDetector, 'subscribe').mockImplementation( + listener => { + const unsubscribe = realSubscribe(listener) + unsubscribes.push(unsubscribe) + return unsubscribe + }, + ) + process.env.CLAUDE_CODE_SIMPLE = '1' + + const shutdownSpy = spyOn(shutdownModule, 'gracefulShutdownSync').mockImplementation(() => {}) + const stdoutSpy = spyOn(processModule, 'writeToStdout').mockImplementation( + () => {}, + ) + const askSpy = spyOn(queryEngineModule, 'ask').mockImplementation( + async function* () { + yield createHeadlessSuccessResult() + }, ) - await runPromise - expect(askSpy.mock.calls[0]?.[0]?.maxTurns).toBe(0) - expect(shutdownSpy).toHaveBeenCalledWith(0) + let state = getDefaultAppState() + const getAppState = () => state + const setAppState = (update: (previous: AppState) => AppState) => { + state = update(state) + } + + let runPromise: Promise | undefined + try { + runPromise = runHeadless( + 'headless prompt', + getAppState, + setAppState, + [] as Command[], + [] as Tools, + {}, + [], + createHeadlessRunOptions(0), + ) + + await runPromise + expect(askSpy.mock.calls[0]?.[0]?.maxTurns).toBe(0) + expect(shutdownSpy).toHaveBeenCalledWith(0) + } finally { + await runPromise?.catch(() => {}) + for (const unsubscribe of unsubscribes) unsubscribe() + subscribeSpy.mockRestore() + askSpy.mockRestore() + stdoutSpy.mockRestore() + shutdownSpy.mockRestore() + } } finally { - await runPromise?.catch(() => {}) - askSpy.mockRestore() - stdoutSpy.mockRestore() - shutdownSpy.mockRestore() + releaseSharedMutationLock() } }) }) diff --git a/src/components/TrustDialog/utils.test.ts b/src/components/TrustDialog/utils.test.ts index 8ef3c9b28..aa5446a21 100644 --- a/src/components/TrustDialog/utils.test.ts +++ b/src/components/TrustDialog/utils.test.ts @@ -1,10 +1,13 @@ -import { afterAll, beforeEach, describe, expect, mock, test } from 'bun:test' +import { afterAll, beforeEach, describe, expect, spyOn, test } from 'bun:test' import { acquireSharedMutationLock, releaseSharedMutationLock } from '../../test/sharedMutationLock.js' import { BASH_TOOL_NAME } from '../../tools/BashTool/toolName.js' import type { PermissionRule } from '../../utils/permissions/PermissionRule.js' -import { getRelativeSettingsFilePathForSource as REAL_canonicalPath } from '../../utils/settings/settings.js' +import * as settingsModule from '../../utils/settings/settings.js' +import * as permissionsLoader from '../../utils/permissions/permissionsLoader.js' import type { SettingsJson } from '../../utils/settings/types.js' +const REAL_canonicalPath = settingsModule.getRelativeSettingsFilePathForSource + // TrustDialog/utils.ts emits file-path strings shown to users in the trust // dialog. These MUST come from getRelativeSettingsFilePathForSource — the // single source of truth — not hardcoded literals. The previous code hardcoded @@ -52,24 +55,17 @@ const permissionRulesState: { localSettings: [], } -mock.module('../../utils/settings/settings.js', () => ({ - // Stateful — needs mocking to control test inputs. - getSettingsForSource: (source: 'projectSettings' | 'localSettings') => - settingsState[source], - // Pure — pass the REAL function through. The mock exists only because - // utils.ts imports both names from the same module; we must provide both. - getRelativeSettingsFilePathForSource: REAL_canonicalPath, -})) - -mock.module('../../utils/permissions/permissionsLoader.js', () => ({ - getPermissionRulesForSource: ( - source: 'projectSettings' | 'localSettings', - ): PermissionRule[] => permissionRulesState[source], -})) +const settingsSpy = spyOn(settingsModule, 'getSettingsForSource').mockImplementation( + source => settingsState[source as keyof typeof settingsState], +) +const permissionRulesSpy = spyOn(permissionsLoader, 'getPermissionRulesForSource').mockImplementation( + source => permissionRulesState[source as keyof typeof permissionRulesState], +) afterAll(() => { try { - mock.restore() + settingsSpy.mockRestore() + permissionRulesSpy.mockRestore() } finally { releaseSharedMutationLock() } diff --git a/src/services/mcp/config.test.ts b/src/services/mcp/config.test.ts index f958985fc..679292de8 100644 --- a/src/services/mcp/config.test.ts +++ b/src/services/mcp/config.test.ts @@ -1,30 +1,47 @@ -import { afterEach, beforeEach, expect, test } from 'bun:test' +import { afterAll, afterEach, beforeAll, beforeEach, expect, mock, test } from 'bun:test' import { acquireSharedMutationLock, releaseSharedMutationLock, } from '../../test/sharedMutationLock.js' -import { - getCurrentProjectConfig, - getGlobalConfig, - saveCurrentProjectConfig, - saveGlobalConfig, -} from '../../utils/config.js' -import { isMcpServerDisabled, setMcpServerEnabled } from './config.js' +type ConfigModule = typeof import('../../utils/config.js') +type McpConfigModule = typeof import('./config.js') + +let realConfig: ConfigModule +let incomingConfig: ConfigModule +let isMcpServerDisabled: McpConfigModule['isMcpServerDisabled'] +let setMcpServerEnabled: McpConfigModule['setMcpServerEnabled'] + +beforeAll(async () => { + await acquireSharedMutationLock('services/mcp/config.test.ts') + incomingConfig = { ...await import('../../utils/config.js') } + realConfig = { ...await import(`../../utils/config.js?real=${Date.now()}-${Math.random()}`) } + mock.module('../../utils/config.js', () => realConfig) + const mcpConfig = await import(`./config.js?test=${Date.now()}-${Math.random()}`) + isMcpServerDisabled = mcpConfig.isMcpServerDisabled + setMcpServerEnabled = mcpConfig.setMcpServerEnabled +}) + +afterAll(() => { + try { + mock.module('../../utils/config.js', () => incomingConfig) + } finally { + releaseSharedMutationLock() + } +}) let savedNodeEnv: string | undefined let savedGlobalDisabled: string[] | undefined let savedProjectDisabled: string[] | undefined let savedProjectEnabled: string[] | undefined -beforeEach(async () => { - await acquireSharedMutationLock('services/mcp/config.test.ts') +beforeEach(() => { savedNodeEnv = process.env.NODE_ENV process.env.NODE_ENV = 'test' - savedGlobalDisabled = getGlobalConfig().disabledMcpServers - savedProjectDisabled = getCurrentProjectConfig().disabledMcpServers - savedProjectEnabled = getCurrentProjectConfig().enabledMcpServers - saveGlobalConfig(c => ({ ...c, disabledMcpServers: undefined })) - saveCurrentProjectConfig(c => ({ + savedGlobalDisabled = realConfig.getGlobalConfig().disabledMcpServers + savedProjectDisabled = realConfig.getCurrentProjectConfig().disabledMcpServers + savedProjectEnabled = realConfig.getCurrentProjectConfig().enabledMcpServers + realConfig.saveGlobalConfig(c => ({ ...c, disabledMcpServers: undefined })) + realConfig.saveCurrentProjectConfig(c => ({ ...c, disabledMcpServers: undefined, enabledMcpServers: undefined, @@ -32,18 +49,14 @@ beforeEach(async () => { }) afterEach(() => { - try { - saveGlobalConfig(c => ({ ...c, disabledMcpServers: savedGlobalDisabled })) - saveCurrentProjectConfig(c => ({ - ...c, - disabledMcpServers: savedProjectDisabled, - enabledMcpServers: savedProjectEnabled, - })) - if (savedNodeEnv === undefined) delete process.env.NODE_ENV - else process.env.NODE_ENV = savedNodeEnv - } finally { - releaseSharedMutationLock() - } + realConfig.saveGlobalConfig(c => ({ ...c, disabledMcpServers: savedGlobalDisabled })) + realConfig.saveCurrentProjectConfig(c => ({ + ...c, + disabledMcpServers: savedProjectDisabled, + enabledMcpServers: savedProjectEnabled, + })) + if (savedNodeEnv === undefined) delete process.env.NODE_ENV + else process.env.NODE_ENV = savedNodeEnv }) test('without project or global lists, servers are enabled', () => { @@ -52,32 +65,32 @@ test('without project or global lists, servers are enabled', () => { }) test('an undefined project list inherits only global disables', () => { - getGlobalConfig().disabledMcpServers = ['a', 'b'] + realConfig.getGlobalConfig().disabledMcpServers = ['a', 'b'] expect(isMcpServerDisabled('a')).toBe(true) expect(isMcpServerDisabled('b')).toBe(true) expect(isMcpServerDisabled('other')).toBe(false) }) test.each([{ list: [] }, { list: ['local'] }])('an explicit project list %j overrides global disables', ({ list }) => { - getGlobalConfig().disabledMcpServers = ['global'] - saveCurrentProjectConfig(c => ({ ...c, disabledMcpServers: [...list] })) + realConfig.getGlobalConfig().disabledMcpServers = ['global'] + realConfig.saveCurrentProjectConfig(c => ({ ...c, disabledMcpServers: [...list] })) expect(isMcpServerDisabled('global')).toBe(false) expect(isMcpServerDisabled('local')).toBe(list.length > 0) }) test('disabling preserves inherited entries and propagates globally without duplicates', () => { - getGlobalConfig().disabledMcpServers = ['a'] + realConfig.getGlobalConfig().disabledMcpServers = ['a'] setMcpServerEnabled('b', false) setMcpServerEnabled('b', false) expect(isMcpServerDisabled('a')).toBe(true) expect(isMcpServerDisabled('b')).toBe(true) - expect(getCurrentProjectConfig().disabledMcpServers).toEqual(['a', 'b']) - expect(getGlobalConfig().disabledMcpServers).toEqual(['a', 'b']) + expect(realConfig.getCurrentProjectConfig().disabledMcpServers).toEqual(['a', 'b']) + expect(realConfig.getGlobalConfig().disabledMcpServers).toEqual(['a', 'b']) }) test('toggling an explicit project list preserves its overrides and enables locally', () => { - getGlobalConfig().disabledMcpServers = ['global'] - saveCurrentProjectConfig(c => ({ ...c, disabledMcpServers: ['local'] })) + realConfig.getGlobalConfig().disabledMcpServers = ['global'] + realConfig.saveCurrentProjectConfig(c => ({ ...c, disabledMcpServers: ['local'] })) setMcpServerEnabled('target', false) expect(isMcpServerDisabled('target')).toBe(true) setMcpServerEnabled('target', true) @@ -85,22 +98,22 @@ test('toggling an explicit project list preserves its overrides and enables loca expect(isMcpServerDisabled('target')).toBe(false) expect(isMcpServerDisabled('local')).toBe(true) expect(isMcpServerDisabled('global')).toBe(false) - expect(getGlobalConfig().disabledMcpServers).toEqual(['global', 'target']) + expect(realConfig.getGlobalConfig().disabledMcpServers).toEqual(['global', 'target']) }) test('enabling one inherited server preserves other disables and stays local', () => { - getGlobalConfig().disabledMcpServers = ['a', 'b'] + realConfig.getGlobalConfig().disabledMcpServers = ['a', 'b'] setMcpServerEnabled('a', true) setMcpServerEnabled('a', true) expect(isMcpServerDisabled('a')).toBe(false) expect(isMcpServerDisabled('b')).toBe(true) - expect(getGlobalConfig().disabledMcpServers).toEqual(['a', 'b']) + expect(realConfig.getGlobalConfig().disabledMcpServers).toEqual(['a', 'b']) }) test('enabling the last inherited server keeps an explicit empty override', () => { - getGlobalConfig().disabledMcpServers = ['a'] + realConfig.getGlobalConfig().disabledMcpServers = ['a'] setMcpServerEnabled('a', true) - expect(getCurrentProjectConfig().disabledMcpServers).toEqual([]) + expect(realConfig.getCurrentProjectConfig().disabledMcpServers).toEqual([]) expect(isMcpServerDisabled('a')).toBe(false) - expect(getGlobalConfig().disabledMcpServers).toEqual(['a']) + expect(realConfig.getGlobalConfig().disabledMcpServers).toEqual(['a']) }) diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index 02cc24478..63caaf18a 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -1,5 +1,6 @@ -import { afterEach, expect, mock, test } from 'bun:test' +import { afterEach, expect, spyOn, test } from 'bun:test' import * as originalSettings from './settings/settings.js' +import type { SettingsJson } from './settings/types.js' type MockSource = | 'userSettings' @@ -14,11 +15,10 @@ async function importAuthFresh() { return await import(`./auth.js?ts=${Date.now()}-${Math.random()}`) } -// Restore Bun's module mocks after each test so leaked settings behavior -// cannot influence later auth/settings tests in the same process. -// Addresses jatmn's P3 on #1731: test isolation for mock.module(). +const settingsSpies: Array<{ mockRestore(): void }> = [] + afterEach(() => { - mock.restore() + for (const spy of settingsSpies.splice(0)) spy.mockRestore() }) // Helper: mock settings to return the given subscriptionType from a specific @@ -26,17 +26,18 @@ afterEach(() => { // from user settings only — project, local, flag, and policy settings must // NOT propagate subscriptionType. function mockSettings( - subscriptionType: string | undefined, + subscriptionType: SettingsJson['subscriptionType'], source: MockSource = 'userSettings', ) { - mock.module('./settings/settings.js', () => ({ - ...originalSettings, - getSettings_DEPRECATED: () => (source === 'none' ? {} : { subscriptionType }), - getSettingsForSource: (s: string) => { + settingsSpies.push( + spyOn(originalSettings, 'getSettings_DEPRECATED').mockImplementation( + () => (source === 'none' ? {} : { subscriptionType }), + ), + spyOn(originalSettings, 'getSettingsForSource').mockImplementation(s => { if (source === 'none') return null return s === source ? { subscriptionType } : null - }, - })) + }), + ) } async function withControlledAuthEnv( diff --git a/src/utils/hookChains.integration.test.ts b/src/utils/hookChains.integration.test.ts index 12553779d..be50a8603 100644 --- a/src/utils/hookChains.integration.test.ts +++ b/src/utils/hookChains.integration.test.ts @@ -7,6 +7,10 @@ import { releaseSharedMutationLock, } from '../test/sharedMutationLock.js' +import * as realTeammate from './teammate.js' + +const realTeammateSnapshot = { ...realTeammate } + type HookChainsModule = typeof import('./hookChains.js') type ImportHarnessOptions = { allowRemoteSessions?: boolean @@ -45,7 +49,6 @@ async function importRealModules() { policyLimits: await import(`../services/policyLimits/index.js?real=${ts}-${rand}`) as typeof import('../services/policyLimits/index.js'), teamHelpers: await import(`./swarm/teamHelpers.js?real=${ts}-${rand}`) as typeof import('./swarm/teamHelpers.js'), teammateMailbox: await import(`./teammateMailbox.js?real=${ts}-${rand}`) as typeof import('./teammateMailbox.js'), - teammate: await import(`./teammate.js?real=${ts}-${rand}`) as typeof import('./teammate.js'), replBridge: await import(`../bridge/replBridgeHandle.js?real=${ts}-${rand}`) as typeof import('../bridge/replBridgeHandle.js'), agentTool: await import(`../tools/AgentTool/AgentTool.js?real=${ts}-${rand}`) as typeof import('../tools/AgentTool/AgentTool.js'), } @@ -58,7 +61,7 @@ async function restorePersistentModuleMocks(): Promise { mock.module('../services/policyLimits/index.js', () => real.policyLimits) mock.module('./swarm/teamHelpers.js', () => real.teamHelpers) mock.module('./teammateMailbox.js', () => real.teammateMailbox) - mock.module('./teammate.js', () => real.teammate) + mock.module('./teammate.js', () => realTeammateSnapshot) mock.module('../bridge/replBridgeHandle.js', () => real.replBridge) mock.module('../tools/AgentTool/AgentTool.js', () => real.agentTool) } @@ -112,7 +115,7 @@ async function importHookChainsHarness( })) mock.module('./teammate.js', () => ({ - ...real.teammate, + ...realTeammateSnapshot, getAgentName: () => senderName, getTeamName: () => teamName, getTeammateColor: () => 'blue', diff --git a/src/utils/modelCost.modelGate.test.ts b/src/utils/modelCost.modelGate.test.ts index 659c09888..15718c82e 100644 --- a/src/utils/modelCost.modelGate.test.ts +++ b/src/utils/modelCost.modelGate.test.ts @@ -17,6 +17,7 @@ import * as realFastMode from './fastMode.js' import * as realModel from './model/model.js' import { resetSettingsCache } from './settings/settingsCache.js' +const realFastModeSnapshot = { ...realFastMode } const realModelSnapshot = { ...realModel } type PricingOverride = { @@ -38,6 +39,7 @@ async function importFreshModelCost() { beforeEach(async () => { await acquireSharedMutationLock('utils/modelCost.modelGate.test.ts') + resetCostState() originalSources = [...getAllowedSettingSources()] originalFlagPath = getFlagSettingsPath() originalFlagInline = getFlagSettingsInline() @@ -50,8 +52,9 @@ beforeEach(async () => { afterEach(() => { try { + resetCostState() mock.restore() - mock.module('./fastMode.js', () => realFastMode) + mock.module('./fastMode.js', () => realFastModeSnapshot) mock.module('./model/model.js', () => realModelSnapshot) setAllowedSettingSources(originalSources) setFlagSettingsPath(originalFlagPath) @@ -90,7 +93,7 @@ test('unknown models do not inherit the configured default model price', async ( // billed at the normal rate while the picker advertised the fast-mode price. test('fast-mode Opus 4.8 is charged the elevated fast-mode tier, normal otherwise', async () => { mock.module('./fastMode.js', () => ({ - ...realFastMode, + ...realFastModeSnapshot, isFastModeEnabled: () => true, })) const { getModelCosts, COST_TIER_30_150, COST_TIER_5_25 } = @@ -199,7 +202,7 @@ test('all-zero exact override is authoritative for unknown and fast-mode known m pricingByModel['nvidia/free-model'] = zero pricingByModel['claude-opus-4-8'] = zero mock.module('./fastMode.js', () => ({ - ...realFastMode, + ...realFastModeSnapshot, isFastModeEnabled: () => true, })) const { calculateUSDCost, getModelCosts, getModelPricingString } = @@ -223,7 +226,7 @@ test('all-zero exact override is authoritative for unknown and fast-mode known m test('custom pricing matches exact resolved ids only, including unusual own keys', async () => { mock.restore() - mock.module('./fastMode.js', () => realFastMode) + mock.module('./fastMode.js', () => realFastModeSnapshot) mock.module('./model/model.js', () => realModelSnapshot) const configured = { inputTokens: 7, diff --git a/src/utils/openclaudePaths.test.ts b/src/utils/openclaudePaths.test.ts index 4eeb21005..707fac484 100644 --- a/src/utils/openclaudePaths.test.ts +++ b/src/utils/openclaudePaths.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, mock, test } from 'bun:test' +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test' import { existsSync, mkdirSync, @@ -7,12 +7,29 @@ import { writeFileSync, } from 'fs' import * as fsPromises from 'fs/promises' +import * as os from 'os' import { homedir, tmpdir } from 'os' import { join } from 'path' import { acquireEnvMutex, releaseEnvMutex } from '../entrypoints/sdk/shared.js' +import { + getClaudeConfigHomeDir, + getClaudeConfigHomeDirOverrideForTesting, + setClaudeConfigHomeDirForTesting, +} from './envUtils.js' -const originalEnv = { ...process.env } -const originalArgv = [...process.argv] +let originalEnv: NodeJS.ProcessEnv +let originalArgv: string[] +let originalConfigOverride: string | undefined +const pathSpies: Array<{ mockRestore(): void }> = [] + +beforeEach(async () => { + await acquireEnvMutex() + originalEnv = { ...process.env } + originalArgv = [...process.argv] + originalConfigOverride = getClaudeConfigHomeDirOverrideForTesting() + setClaudeConfigHomeDirForTesting(undefined) + getClaudeConfigHomeDir.cache.clear() +}) async function importFreshEnvUtils() { return import(`./envUtils.ts?ts=${Date.now()}-${Math.random()}`) @@ -34,7 +51,9 @@ afterEach(() => { try { process.env = { ...originalEnv } process.argv = [...originalArgv] - mock.restore() + for (const spy of pathSpies.splice(0)) spy.mockRestore() + setClaudeConfigHomeDirForTesting(originalConfigOverride) + getClaudeConfigHomeDir.cache.clear() } finally { releaseEnvMutex() } @@ -42,7 +61,6 @@ afterEach(() => { describe('OpenClaude paths', () => { test('defaults user config home to ~/.openclaude', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR delete process.env.CLAUDE_CONFIG_DIR const { resolveClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -55,7 +73,6 @@ describe('OpenClaude paths', () => { }) test('hard-cuts user config home to ~/.openclaude by default', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR delete process.env.CLAUDE_CONFIG_DIR const { resolveClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -68,7 +85,6 @@ describe('OpenClaude paths', () => { }) test('does not migrate legacy .claude config into .openclaude', async () => { - await acquireEnvMutex() const tempHome = mkdtempSync(join(tmpdir(), 'openclaude-paths-test-')) try { mkdirSync(join(tempHome, '.claude', 'skills', 'legacy-skill'), { @@ -91,15 +107,11 @@ describe('OpenClaude paths', () => { }) test('config home does not fall back to legacy .claude', async () => { - await acquireEnvMutex() const tempHome = mkdtempSync(join(tmpdir(), 'openclaude-paths-test-')) try { writeFileSync(join(tempHome, '.openclaude'), 'not a directory') mkdirSync(join(tempHome, '.claude'), { recursive: true }) - mock.module('os', () => ({ - homedir: () => tempHome, - tmpdir, - })) + pathSpies.push(spyOn(os, 'homedir').mockReturnValue(tempHome)) delete process.env.OPENCLAUDE_CONFIG_DIR delete process.env.CLAUDE_CONFIG_DIR @@ -112,7 +124,6 @@ describe('OpenClaude paths', () => { }) test('default plans directory uses ~/.openclaude/plans', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR delete process.env.CLAUDE_CONFIG_DIR const { getDefaultPlansDirectory } = await importFreshPlans() @@ -123,7 +134,6 @@ describe('OpenClaude paths', () => { }) test('default plans directory respects explicit configDirEnv argument', async () => { - await acquireEnvMutex() const { getDefaultPlansDirectory } = await importFreshPlans() expect( @@ -132,7 +142,6 @@ describe('OpenClaude paths', () => { }) test('default plans directory respects OPENCLAUDE_CONFIG_DIR', async () => { - await acquireEnvMutex() process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/preferred-openclaude' delete process.env.CLAUDE_CONFIG_DIR const { getDefaultPlansDirectory } = await importFreshPlans() @@ -143,7 +152,6 @@ describe('OpenClaude paths', () => { }) test('OPENCLAUDE_CONFIG_DIR wins for default plans directory', async () => { - await acquireEnvMutex() process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/preferred-openclaude' process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-openclaude' const { getDefaultPlansDirectory } = await importFreshPlans() @@ -154,7 +162,6 @@ describe('OpenClaude paths', () => { }) test('default plans directory normalizes generated path to NFC', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR const { getDefaultPlansDirectory } = await importFreshPlans() @@ -164,7 +171,6 @@ describe('OpenClaude paths', () => { }) test('default plans directory normalizes explicit configDirEnv argument to NFC', async () => { - await acquireEnvMutex() const { getDefaultPlansDirectory } = await importFreshPlans() expect( @@ -173,20 +179,15 @@ describe('OpenClaude paths', () => { }) test('ignores CLAUDE_CONFIG_DIR override when provided', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR process.env.CLAUDE_CONFIG_DIR = '/tmp/custom-openclaude' - mock.module('os', () => ({ - homedir: () => '/tmp/home', - tmpdir, - })) + pathSpies.push(spyOn(os, 'homedir').mockReturnValue('/tmp/home')) const { getClaudeConfigHomeDir } = await importFreshEnvUtils() expect(getClaudeConfigHomeDir()).toBe('/tmp/home/.openclaude') }) test('OPENCLAUDE_CONFIG_DIR overrides the default (issue #454)', async () => { - await acquireEnvMutex() delete process.env.CLAUDE_CONFIG_DIR process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/oc-config-only' const { getClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -195,7 +196,6 @@ describe('OpenClaude paths', () => { }) test('OPENCLAUDE_CONFIG_DIR wins when both env vars are set with different values', async () => { - await acquireEnvMutex() process.env.OPENCLAUDE_CONFIG_DIR = '/tmp/oc-wins' process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-loses' const { getClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -204,7 +204,6 @@ describe('OpenClaude paths', () => { }) test('CLAUDE_CONFIG_DIR is ignored when OPENCLAUDE_CONFIG_DIR is unset', async () => { - await acquireEnvMutex() delete process.env.OPENCLAUDE_CONFIG_DIR process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-only' const { getClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -213,7 +212,6 @@ describe('OpenClaude paths', () => { }) test('empty OPENCLAUDE_CONFIG_DIR does not fall through to CLAUDE_CONFIG_DIR', async () => { - await acquireEnvMutex() process.env.OPENCLAUDE_CONFIG_DIR = '' process.env.CLAUDE_CONFIG_DIR = '/tmp/legacy-fallback' const { getClaudeConfigHomeDir } = await importFreshEnvUtils() @@ -222,7 +220,6 @@ describe('OpenClaude paths', () => { }) test('resolveConfigDirEnv ignores CLAUDE_CONFIG_DIR without warning', async () => { - await acquireEnvMutex() const { resolveConfigDirEnv, __resetConfigDirEnvWarningForTesting } = await importFreshEnvUtils() __resetConfigDirEnvWarningForTesting() @@ -246,7 +243,6 @@ describe('OpenClaude paths', () => { }) test('resolveConfigDirEnv ignores legacy env for silent and warning callers', async () => { - await acquireEnvMutex() const { resolveConfigDirEnv, __resetConfigDirEnvWarningForTesting } = await importFreshEnvUtils() __resetConfigDirEnvWarningForTesting() @@ -270,7 +266,6 @@ describe('OpenClaude paths', () => { }) test('resolveConfigDirEnv does not warn when both env vars agree', async () => { - await acquireEnvMutex() const { resolveConfigDirEnv, __resetConfigDirEnvWarningForTesting } = await importFreshEnvUtils() __resetConfigDirEnvWarningForTesting() @@ -287,7 +282,6 @@ describe('OpenClaude paths', () => { }) test('resolveConfigDirEnv returns undefined when neither env var is set', async () => { - await acquireEnvMutex() const { resolveConfigDirEnv } = await importFreshEnvUtils() expect( @@ -299,7 +293,6 @@ describe('OpenClaude paths', () => { }) test('project and local settings paths use .openclaude', async () => { - await acquireEnvMutex() const { getRelativeSettingsFilePathForSource } = await importFreshSettings() expect(getRelativeSettingsFilePathForSource('projectSettings')).toBe( @@ -311,7 +304,6 @@ describe('OpenClaude paths', () => { }) test('local installer uses openclaude wrapper path', async () => { - await acquireEnvMutex() process.env.OPENCLAUDE_CONFIG_DIR = join(homedir(), '.openclaude') delete process.env.CLAUDE_CONFIG_DIR const { getLocalClaudePath } = await importFreshLocalInstaller() @@ -322,7 +314,6 @@ describe('OpenClaude paths', () => { }) test('local installation detection matches .openclaude path', async () => { - await acquireEnvMutex() const { isManagedLocalInstallationPath } = await importFreshLocalInstaller() @@ -334,7 +325,6 @@ describe('OpenClaude paths', () => { }) test('local installation detection ignores legacy .claude path', async () => { - await acquireEnvMutex() const { isManagedLocalInstallationPath } = await importFreshLocalInstaller() @@ -346,7 +336,6 @@ describe('OpenClaude paths', () => { }) test('candidate local install dirs include only openclaude path', async () => { - await acquireEnvMutex() const { getCandidateLocalInstallDirs } = await importFreshLocalInstaller() expect( @@ -359,18 +348,16 @@ describe('OpenClaude paths', () => { }) test('legacy local installs are ignored even when they expose the claude binary', async () => { - await acquireEnvMutex() - mock.module('fs/promises', () => ({ - ...fsPromises, - access: async (path: string) => { + pathSpies.push( + spyOn(fsPromises, 'access').mockImplementation(async path => { if ( path === join(homedir(), '.claude', 'local', 'node_modules', '.bin', 'claude') ) { return } throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) - }, - })) + }), + ) const { getDetectedLocalInstallDir, localInstallationExists } = await importFreshLocalInstaller() diff --git a/src/utils/providerProfile.test.ts b/src/utils/providerProfile.test.ts index 675f308d4..623e53573 100644 --- a/src/utils/providerProfile.test.ts +++ b/src/utils/providerProfile.test.ts @@ -10,6 +10,10 @@ import { resolveRouteCredentialValue, } from '../integrations/routeMetadata.js' import { DEFAULT_CODEX_BASE_URL } from '../services/api/providerConfig.js' +import { + getClaudeConfigHomeDirOverrideForTesting, + setClaudeConfigHomeDirForTesting, +} from './envUtils.js' import { getProviderValidationError } from './providerValidation.js' import { applySavedProfileToCurrentSession, @@ -2002,8 +2006,10 @@ test('deleteProfileFile clears the default profile and legacy workspace fallback const configDir = mkdtempSync(join(tmpdir(), 'openclaude-delete-config-profile-')) const previousConfigDir = process.env.OPENCLAUDE_CONFIG_DIR const previousCwd = process.cwd() + const previousConfigOverride = getClaudeConfigHomeDirOverrideForTesting() try { + setClaudeConfigHomeDirForTesting(undefined) process.env.OPENCLAUDE_CONFIG_DIR = configDir process.chdir(cwd) @@ -2015,6 +2021,7 @@ test('deleteProfileFile clears the default profile and legacy workspace fallback OPENAI_MODEL: 'llama3.1:8b', }) + assert.equal(getDefaultProfileFilePath(), join(configDir, PROFILE_FILE_NAME)) saveProfileFile(configProfile) assert.equal(existsSync(join(configDir, PROFILE_FILE_NAME)), true) writeFileSync( @@ -2029,6 +2036,7 @@ test('deleteProfileFile clears the default profile and legacy workspace fallback assert.equal(existsSync(join(cwd, PROFILE_FILE_NAME)), false) assert.equal(loadProfileFile(), null) } finally { + setClaudeConfigHomeDirForTesting(previousConfigOverride) process.chdir(previousCwd) if (previousConfigDir === undefined) { delete process.env.OPENCLAUDE_CONFIG_DIR diff --git a/src/utils/sessionStorage.commandOnlySession.test.ts b/src/utils/sessionStorage.commandOnlySession.test.ts index fc1babcc2..9087913ce 100644 --- a/src/utils/sessionStorage.commandOnlySession.test.ts +++ b/src/utils/sessionStorage.commandOnlySession.test.ts @@ -3,7 +3,7 @@ import { type UUID } from 'node:crypto' import { existsSync } from 'node:fs' import { mkdtemp, readFile, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, sep } from 'node:path' import { acquireSharedMutationLock, @@ -16,7 +16,10 @@ import { switchSession, } from '../bootstrap/state.js' import type { Message } from '../types/message.js' -import { setClaudeConfigHomeDirForTesting } from './envUtils.js' +import { + getClaudeConfigHomeDirOverrideForTesting, + setClaudeConfigHomeDirForTesting, +} from './envUtils.js' import { createAssistantMessage, createUserMessage, @@ -24,6 +27,7 @@ import { } from './messages.js' import { flushSessionStorage, + getProjectDir, getTranscriptPathForSession, recordTranscript, resetProjectForTesting, @@ -34,16 +38,19 @@ import { const SESSION = '00000000-0000-4000-8000-00000000c001' as UUID const OTHER = '00000000-0000-4000-8000-00000000c002' as UUID const tempDirs: string[] = [] -const savedEnv = { - NODE_ENV: process.env.NODE_ENV, - TEST_ENABLE_SESSION_PERSISTENCE: process.env.TEST_ENABLE_SESSION_PERSISTENCE, - ENABLE_SESSION_PERSISTENCE: process.env.ENABLE_SESSION_PERSISTENCE, -} +let savedEnv: Record +let originalConfigOverride: string | undefined let originalSessionId: string let originalPersistenceDisabled: boolean beforeEach(async () => { await acquireSharedMutationLock('utils/sessionStorage.commandOnlySession.test.ts') + savedEnv = { + NODE_ENV: process.env.NODE_ENV, + TEST_ENABLE_SESSION_PERSISTENCE: process.env.TEST_ENABLE_SESSION_PERSISTENCE, + ENABLE_SESSION_PERSISTENCE: process.env.ENABLE_SESSION_PERSISTENCE, + } + originalConfigOverride = getClaudeConfigHomeDirOverrideForTesting() originalSessionId = getSessionId() originalPersistenceDisabled = isSessionPersistenceDisabled() process.env.NODE_ENV = 'development' @@ -53,25 +60,33 @@ beforeEach(async () => { const configDir = await mkdtemp(join(tmpdir(), 'openclaude-command-only-')) tempDirs.push(configDir) setClaudeConfigHomeDirForTesting(configDir) + getProjectDir.cache.clear() resetProjectForTesting() switchSession(SESSION as never) await resetSessionFilePointer() + expect(getTranscriptPathForSession(SESSION).startsWith(`${configDir}${sep}`)).toBe(true) }) afterEach(async () => { try { - for (const [key, value] of Object.entries(savedEnv)) { - if (value === undefined) delete process.env[key] - else process.env[key] = value - } - setSessionPersistenceDisabled(originalPersistenceDisabled) - switchSession(originalSessionId as never) - resetProjectForTesting() - await Promise.all( - tempDirs.splice(0).map(dir => rm(dir, { recursive: true, force: true })), - ) + await flushSessionStorage() } finally { - releaseSharedMutationLock() + try { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + setSessionPersistenceDisabled(originalPersistenceDisabled) + switchSession(originalSessionId as never) + resetProjectForTesting() + setClaudeConfigHomeDirForTesting(originalConfigOverride) + getProjectDir.cache.clear() + await Promise.all( + tempDirs.splice(0).map(dir => rm(dir, { recursive: true, force: true })), + ) + } finally { + releaseSharedMutationLock() + } } }) diff --git a/src/utils/sessionStorage.deletedSession.test.ts b/src/utils/sessionStorage.deletedSession.test.ts index b927a246b..ff070ac0b 100644 --- a/src/utils/sessionStorage.deletedSession.test.ts +++ b/src/utils/sessionStorage.deletedSession.test.ts @@ -17,11 +17,15 @@ import { switchSession, } from '../bootstrap/state.js' import { createGoalState } from '../services/goal/state.js' -import { setClaudeConfigHomeDirForTesting } from './envUtils.js' +import { + getClaudeConfigHomeDirOverrideForTesting, + setClaudeConfigHomeDirForTesting, +} from './envUtils.js' import { deleteSessionFiles } from './sessionDeletion.js' import * as sessionStorage from './sessionStorage.js' import { flushSessionStorage, + getProjectDir, getTranscriptPathForSession, recordGoalState, resetProjectForTesting, @@ -34,16 +38,19 @@ import { const DELETED = '00000000-0000-4000-8000-00000000d001' as UUID const ACTIVE = '00000000-0000-4000-8000-00000000d002' as UUID const tempDirs: string[] = [] -const savedEnv = { - NODE_ENV: process.env.NODE_ENV, - TEST_ENABLE_SESSION_PERSISTENCE: process.env.TEST_ENABLE_SESSION_PERSISTENCE, - ENABLE_SESSION_PERSISTENCE: process.env.ENABLE_SESSION_PERSISTENCE, -} +let savedEnv: Record +let originalConfigOverride: string | undefined let originalSessionId: string let originalPersistenceDisabled: boolean beforeEach(async () => { await acquireSharedMutationLock('utils/sessionStorage.deletedSession.test.ts') + savedEnv = { + NODE_ENV: process.env.NODE_ENV, + TEST_ENABLE_SESSION_PERSISTENCE: process.env.TEST_ENABLE_SESSION_PERSISTENCE, + ENABLE_SESSION_PERSISTENCE: process.env.ENABLE_SESSION_PERSISTENCE, + } + originalConfigOverride = getClaudeConfigHomeDirOverrideForTesting() originalSessionId = getSessionId() originalPersistenceDisabled = isSessionPersistenceDisabled() process.env.NODE_ENV = 'development' @@ -53,24 +60,31 @@ beforeEach(async () => { const configDir = await mkdtemp(join(tmpdir(), 'openclaude-deleted-session-')) tempDirs.push(configDir) setClaudeConfigHomeDirForTesting(configDir) + getProjectDir.cache.clear() resetProjectForTesting() }) afterEach(async () => { try { - mock.restore() - for (const [key, value] of Object.entries(savedEnv)) { - if (value === undefined) delete process.env[key] - else process.env[key] = value - } - setSessionPersistenceDisabled(originalPersistenceDisabled) - switchSession(originalSessionId as never) - resetProjectForTesting() - await Promise.all( - tempDirs.splice(0).map(dir => rm(dir, { recursive: true, force: true })), - ) + await flushSessionStorage() } finally { - releaseSharedMutationLock() + try { + mock.restore() + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + setSessionPersistenceDisabled(originalPersistenceDisabled) + switchSession(originalSessionId as never) + resetProjectForTesting() + setClaudeConfigHomeDirForTesting(originalConfigOverride) + getProjectDir.cache.clear() + await Promise.all( + tempDirs.splice(0).map(dir => rm(dir, { recursive: true, force: true })), + ) + } finally { + releaseSharedMutationLock() + } } })