From 6261ebf433ebb14fe2414868aa86cccd761986aa Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Mon, 13 Jul 2026 10:28:55 +1000 Subject: [PATCH] fix(core): use os.homedir() instead of trusting HOME env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getHomeDir() prioritized process.env.HOME, which Git Bash/MSYS shells can mistranslate into a bare drive root (e.g. "C:\") if HOME is misconfigured — causing user-scope sync to treat the whole drive as home and walk into permission-restricted system folders. os.homedir() ignores HOME on Windows entirely, so it's immune to this failure mode. Also updates the ~16 tests that stub HOME to sandbox user-scope behavior in a temp dir, since os.homedir() no longer reads it on Windows; added tests/helpers/env.ts to centralize the stub/restore logic across them. Closes #433 --- src/constants.ts | 10 +++++- tests/e2e/user-scope.test.ts | 6 ++-- tests/helpers/env.ts | 22 +++++++++++++ tests/unit/constants.test.ts | 33 +++++++++++++++++++ .../unit/core/marketplace-add-branch.test.ts | 8 ++--- .../unit/core/marketplace-auto-update.test.ts | 8 ++--- .../marketplace-branch-separation.test.ts | 8 ++--- tests/unit/core/marketplace-dedup.test.ts | 8 ++--- tests/unit/core/marketplace-refresh.test.ts | 8 ++--- .../core/marketplace-remove-cascade.test.ts | 8 ++--- tests/unit/core/marketplace-scope.test.ts | 22 ++++++------- tests/unit/core/marketplace-update.test.ts | 8 ++--- .../native-marketplace-registration.test.ts | 8 ++--- tests/unit/core/prune.test.ts | 8 ++--- tests/unit/core/status-both-scopes.test.ts | 19 ++++++----- tests/unit/core/sync-resilient.test.ts | 8 ++--- tests/unit/core/sync-user.test.ts | 8 ++--- tests/unit/core/user-workspace-skills.test.ts | 14 ++++---- tests/unit/core/user-workspace.test.ts | 8 ++--- tests/unit/utils/plugin-path.test.ts | 3 +- 20 files changed, 146 insertions(+), 79 deletions(-) create mode 100644 tests/helpers/env.ts create mode 100644 tests/unit/constants.test.ts diff --git a/src/constants.ts b/src/constants.ts index e9aaeaf7..9fa38b85 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,8 +1,16 @@ +import { homedir } from 'node:os'; + /** * Get the user's home directory (cross-platform). + * + * Delegates to Node's os.homedir() rather than reading process.env.HOME + * directly: on Windows, os.homedir() ignores HOME and resolves via + * USERPROFILE, so it isn't affected by shells (e.g. Git Bash/MSYS) that + * mistranslate a misconfigured HOME into a bare drive root like "C:\\" + * (see EntityProcess/allagents#433). */ export function getHomeDir(): string { - return process.env.HOME || process.env.USERPROFILE || '~'; + return homedir(); } /** diff --git a/tests/e2e/user-scope.test.ts b/tests/e2e/user-scope.test.ts index ba7ffa5c..bd842bf4 100644 --- a/tests/e2e/user-scope.test.ts +++ b/tests/e2e/user-scope.test.ts @@ -7,6 +7,7 @@ import { addUserPlugin, removeUserPlugin, getUserWorkspaceConfig } from '../../s import { addPlugin } from '../../src/core/workspace-modify.js'; import { syncUserWorkspace, syncWorkspace } from '../../src/core/sync.js'; import { initWorkspace } from '../../src/core/workspace.js'; +import { stubHomeDir } from '../helpers/env.js'; // E2E tests require network access and gh authentication. // They are skipped by default; run with ALLAGENTS_E2E=1 to enable. @@ -17,6 +18,7 @@ describe.skipIf(!e2eEnabled)('E2E: user scope vs project scope', () => { let tempProject: string; let originalHome: string; let originalGhConfigDir: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { tempHome = await mkdtemp(join(tmpdir(), 'allagents-e2e-home-')); @@ -29,11 +31,11 @@ describe.skipIf(!e2eEnabled)('E2E: user scope vs project scope', () => { process.env.GH_CONFIG_DIR = join(originalHome, '.config', 'gh'); } - process.env.HOME = tempHome; + restoreHomeDir = stubHomeDir(tempHome); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); if (originalGhConfigDir !== undefined) { process.env.GH_CONFIG_DIR = originalGhConfigDir; } else { diff --git a/tests/helpers/env.ts b/tests/helpers/env.ts new file mode 100644 index 00000000..4a8b6b5f --- /dev/null +++ b/tests/helpers/env.ts @@ -0,0 +1,22 @@ +/** + * Temporarily override the resolved home directory for a test. + * + * os.homedir() (what src/constants.ts#getHomeDir now delegates to) reads + * $HOME on POSIX and %USERPROFILE% on Windows — never both — so tests that + * only stubbed HOME silently stopped taking effect on Windows. Stubbing both + * keeps tests platform-independent. Returns a restore function that deletes + * (rather than stringifies `undefined` into) any var that wasn't originally set. + */ +export function stubHomeDir(path: string): () => void { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + process.env.HOME = path; + process.env.USERPROFILE = path; + + return () => { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + }; +} diff --git a/tests/unit/constants.test.ts b/tests/unit/constants.test.ts new file mode 100644 index 00000000..0f3845ba --- /dev/null +++ b/tests/unit/constants.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, test, afterEach } from 'bun:test'; +import { getHomeDir } from '../../src/constants.js'; + +describe('getHomeDir', () => { + const originalHome = process.env.HOME; + const originalUserProfile = process.env.USERPROFILE; + + afterEach(() => { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + }); + + // Windows-only: os.homedir() ignores HOME entirely on win32 (uses USERPROFILE), + // so this needs HOME and USERPROFILE set to *different* values to prove the + // point — stubHomeDir (which sets both to the same path) doesn't apply here. + // On POSIX, os.homedir() reads $HOME directly, so a mistranslated HOME isn't + // a platform-level failure mode there the way it is on Windows. + test.skipIf(process.platform !== 'win32')( + 'ignores a HOME env var that has been mistranslated to a bare drive root', + () => { + // Reproduces the Git-Bash/MSYS failure mode: a misconfigured HOME=/c gets + // translated to the literal Windows path "C:\\" for the spawned node process. + // Trusting it would make every user-scope sync operation treat the whole + // drive as "home" (see EntityProcess/allagents#433). + process.env.HOME = 'C:\\'; + process.env.USERPROFILE = 'C:\\Users\\realuser'; + + expect(getHomeDir()).toBe('C:\\Users\\realuser'); + }, + ); +}); diff --git a/tests/unit/core/marketplace-add-branch.test.ts b/tests/unit/core/marketplace-add-branch.test.ts index b671d6e1..a99616fa 100644 --- a/tests/unit/core/marketplace-add-branch.test.ts +++ b/tests/unit/core/marketplace-add-branch.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track clone calls to verify arguments const cloneCalls: Array<{ url: string; dest: string; ref?: string }> = []; @@ -49,19 +50,18 @@ mock.module('simple-git', () => ({ const { addMarketplace, loadRegistry } = await import('../../../src/core/marketplace.js'); describe('addMarketplace branch support', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-add-branch-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); mkdirSync(join(testHome, '.allagents'), { recursive: true }); cloneCalls.length = 0; }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); }); diff --git a/tests/unit/core/marketplace-auto-update.test.ts b/tests/unit/core/marketplace-auto-update.test.ts index d9e8311b..1e31ec7d 100644 --- a/tests/unit/core/marketplace-auto-update.test.ts +++ b/tests/unit/core/marketplace-auto-update.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track calls const pullCalls: Array<{ path: string }> = []; @@ -55,13 +56,12 @@ const { } = await import('../../../src/core/marketplace.js'); describe('resolvePluginSpecWithAutoRegister auto-updates marketplace', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-auto-update-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); pullCalls.length = 0; simpleGitCalls.length = 0; pullShouldFail = false; @@ -69,7 +69,7 @@ describe('resolvePluginSpecWithAutoRegister auto-updates marketplace', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); }); diff --git a/tests/unit/core/marketplace-branch-separation.test.ts b/tests/unit/core/marketplace-branch-separation.test.ts index 474f0ed6..dd4bc79d 100644 --- a/tests/unit/core/marketplace-branch-separation.test.ts +++ b/tests/unit/core/marketplace-branch-separation.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock, spyOn } from 'bun:te import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track cloneTo calls const cloneToCalls: Array<{ url: string; path: string; branch?: string }> = []; @@ -56,21 +57,20 @@ const { } = await import('../../../src/core/marketplace.js'); describe('branch separation — each branch is a separate marketplace', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let consoleLogSpy: ReturnType; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-branch-sep-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); mkdirSync(join(testHome, '.allagents'), { recursive: true }); cloneToCalls.length = 0; consoleLogSpy = spyOn(console, 'log').mockImplementation(() => {}); }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); consoleLogSpy.mockRestore(); }); diff --git a/tests/unit/core/marketplace-dedup.test.ts b/tests/unit/core/marketplace-dedup.test.ts index 5eb45eca..c8f5741b 100644 --- a/tests/unit/core/marketplace-dedup.test.ts +++ b/tests/unit/core/marketplace-dedup.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock, spyOn } from 'bun:te import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track cloneTo calls const cloneToCalls: Array<{ url: string; path: string; branch?: string }> = []; @@ -51,15 +52,14 @@ const { } = await import('../../../src/core/marketplace.js'); describe('marketplace deduplication', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let consoleLogSpy: ReturnType; let logMessages: string[]; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-dedup-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); cloneToCalls.length = 0; resetAutoRegisterCache(); @@ -71,7 +71,7 @@ describe('marketplace deduplication', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); consoleLogSpy.mockRestore(); }); diff --git a/tests/unit/core/marketplace-refresh.test.ts b/tests/unit/core/marketplace-refresh.test.ts index 0449ab6a..00269544 100644 --- a/tests/unit/core/marketplace-refresh.test.ts +++ b/tests/unit/core/marketplace-refresh.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, writeFileSync, rmSync, existsSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track cloneTo calls to verify refresh behavior const cloneToCalls: Array<{ url: string; path: string; branch?: string }> = []; @@ -32,18 +33,17 @@ const { resolvePluginSpecWithAutoRegister } = await import( ); describe('resolvePluginSpecWithAutoRegister refresh', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-refresh-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); cloneToCalls.length = 0; }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); }); diff --git a/tests/unit/core/marketplace-remove-cascade.test.ts b/tests/unit/core/marketplace-remove-cascade.test.ts index 62be3b85..feae5ffa 100644 --- a/tests/unit/core/marketplace-remove-cascade.test.ts +++ b/tests/unit/core/marketplace-remove-cascade.test.ts @@ -7,19 +7,19 @@ import { removeMarketplace, saveRegistry } from '../../../src/core/marketplace.j import type { MarketplaceRegistry } from '../../../src/core/marketplace.js'; import { WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; import type { WorkspaceConfig } from '../../../src/models/workspace-config.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('removeMarketplace cascade', () => { let testDir: string; - let originalHome: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { testDir = await mkdtemp(join(tmpdir(), 'allagents-cascade-test-')); - originalHome = process.env.HOME; - process.env.HOME = testDir; + restoreHomeDir = stubHomeDir(testDir); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(testDir, { recursive: true, force: true }); }); diff --git a/tests/unit/core/marketplace-scope.test.ts b/tests/unit/core/marketplace-scope.test.ts index 6228249a..e2ced25b 100644 --- a/tests/unit/core/marketplace-scope.test.ts +++ b/tests/unit/core/marketplace-scope.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Mock git module before importing marketplace (needed for addMarketplace tests) mock.module('../../../src/core/git.js', () => ({ @@ -344,14 +345,13 @@ describe('scope-aware registry loading and saving', () => { }); describe('addMarketplace with scope', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let tmpProject: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-scope-add-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); mkdirSync(join(testHome, '.allagents'), { recursive: true }); tmpProject = join(tmpdir(), `marketplace-scope-project-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -359,7 +359,7 @@ describe('addMarketplace with scope', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); rmSync(tmpProject, { recursive: true, force: true }); }); @@ -407,7 +407,7 @@ describe('addMarketplace with scope', () => { }); describe('removeMarketplace with scope', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let tmpProject: string; let userRegistryPath: string; @@ -420,9 +420,8 @@ describe('removeMarketplace with scope', () => { }); beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-scope-remove-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); mkdirSync(join(testHome, '.allagents'), { recursive: true }); tmpProject = join(tmpdir(), `marketplace-scope-remove-project-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -433,7 +432,7 @@ describe('removeMarketplace with scope', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); rmSync(tmpProject, { recursive: true, force: true }); }); @@ -529,14 +528,13 @@ describe('removeMarketplace with scope', () => { }); describe('runtime resolution with merged registries', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let tmpProject: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-resolve-test-${Date.now()}-${Math.random().toString(36).slice(2)}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); mkdirSync(join(testHome, '.allagents'), { recursive: true }); tmpProject = join(tmpdir(), `marketplace-resolve-project-${Date.now()}-${Math.random().toString(36).slice(2)}`); @@ -544,7 +542,7 @@ describe('runtime resolution with merged registries', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); rmSync(tmpProject, { recursive: true, force: true }); }); diff --git a/tests/unit/core/marketplace-update.test.ts b/tests/unit/core/marketplace-update.test.ts index c354750e..3f5c2223 100644 --- a/tests/unit/core/marketplace-update.test.ts +++ b/tests/unit/core/marketplace-update.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../helpers/env.js'; // Track calls for assertions const simpleGitCalls: Array<{ method: string; args: unknown[] }> = []; @@ -49,14 +50,13 @@ mock.module('../../../src/core/git.js', () => ({ const { updateMarketplace } = await import('../../../src/core/marketplace.js'); describe('updateMarketplace', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let marketplacePath: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `marketplace-update-test-${Date.now()}`); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); // Create marketplace directory marketplacePath = join(testHome, '.allagents', 'plugins', 'marketplaces', 'test-mp'); @@ -87,7 +87,7 @@ describe('updateMarketplace', () => { }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); }); diff --git a/tests/unit/core/native/native-marketplace-registration.test.ts b/tests/unit/core/native/native-marketplace-registration.test.ts index 194ddc6b..6f3c2901 100644 --- a/tests/unit/core/native/native-marketplace-registration.test.ts +++ b/tests/unit/core/native/native-marketplace-registration.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from 'bun:test'; import { mkdirSync, writeFileSync, rmSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import { stubHomeDir } from '../../../helpers/env.js'; /** * Integration test verifying that syncWorkspace calls addMarketplace @@ -78,21 +79,20 @@ const { syncWorkspace } = await import('../../../../src/core/sync.js'); const { resetUpdatedMarketplaceCache } = await import('../../../../src/core/marketplace.js'); describe('native marketplace registration during syncWorkspace', () => { - let originalHome: string | undefined; + let restoreHomeDir: () => void; let testHome: string; let testDir: string; beforeEach(() => { - originalHome = process.env.HOME; testHome = join(tmpdir(), `native-mp-reg-test-${Date.now()}`); testDir = join(testHome, 'workspace'); - process.env.HOME = testHome; + restoreHomeDir = stubHomeDir(testHome); executeCommandCalls.length = 0; resetUpdatedMarketplaceCache(); }); afterEach(() => { - process.env.HOME = originalHome; + restoreHomeDir(); rmSync(testHome, { recursive: true, force: true }); }); diff --git a/tests/unit/core/prune.test.ts b/tests/unit/core/prune.test.ts index e0ad21a2..1483942e 100644 --- a/tests/unit/core/prune.test.ts +++ b/tests/unit/core/prune.test.ts @@ -8,19 +8,19 @@ import { saveRegistry } from '../../../src/core/marketplace.js'; import type { MarketplaceRegistry } from '../../../src/core/marketplace.js'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; import type { WorkspaceConfig } from '../../../src/models/workspace-config.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('pruneOrphanedPlugins', () => { let testDir: string; - let originalHome: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { testDir = await mkdtemp(join(tmpdir(), 'allagents-prune-test-')); - originalHome = process.env.HOME; - process.env.HOME = testDir; + restoreHomeDir = stubHomeDir(testDir); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(testDir, { recursive: true, force: true }); }); diff --git a/tests/unit/core/status-both-scopes.test.ts b/tests/unit/core/status-both-scopes.test.ts index c27f0078..523a4b07 100644 --- a/tests/unit/core/status-both-scopes.test.ts +++ b/tests/unit/core/status-both-scopes.test.ts @@ -6,19 +6,19 @@ import { dump } from 'js-yaml'; import { getWorkspaceStatus } from '../../../src/core/status.js'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; import type { WorkspaceConfig } from '../../../src/models/workspace-config.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('workspace status - both scopes', () => { let testDir: string; - let originalHome: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { testDir = await mkdtemp(join(tmpdir(), 'allagents-status-test-')); - originalHome = process.env.HOME; - process.env.HOME = testDir; + restoreHomeDir = stubHomeDir(testDir); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(testDir, { recursive: true, force: true }); }); @@ -49,9 +49,12 @@ describe('workspace status - both scopes', () => { } it('should include userPlugins in status result', async () => { - // Use a separate HOME so user config doesn't overlap with project dir + // Use a separate HOME so user config doesn't overlap with project dir. + // Discarding the returned restore fn is intentional: the outer afterEach's + // restoreHomeDir() already unwinds straight back to the pre-suite original + // regardless of how many times HOME/USERPROFILE were reassigned in between. const homeDir = await mkdtemp(join(tmpdir(), 'allagents-status-home-')); - process.env.HOME = homeDir; + stubHomeDir(homeDir); const projectPlugin = await createLocalPlugin('project-plugin'); const userPlugin = await createLocalPlugin('user-plugin'); @@ -82,7 +85,7 @@ describe('workspace status - both scopes', () => { it('should fall back to user plugins when no project workspace exists', async () => { // Use a separate HOME so user config doesn't overlap with project dir const homeDir = await mkdtemp(join(tmpdir(), 'allagents-status-home-')); - process.env.HOME = homeDir; + stubHomeDir(homeDir); const userPlugin = await createLocalPlugin('user-plugin'); const allagentsDir = join(homeDir, '.allagents'); @@ -126,7 +129,7 @@ describe('workspace status - both scopes', () => { it('should show empty userPlugins when no user config exists', async () => { // Use a separate HOME dir so there's no user config const separateHome = await mkdtemp(join(tmpdir(), 'allagents-status-home-')); - process.env.HOME = separateHome; + stubHomeDir(separateHome); const projectPlugin = await createLocalPlugin('project-plugin'); diff --git a/tests/unit/core/sync-resilient.test.ts b/tests/unit/core/sync-resilient.test.ts index b1463d6f..c9a15b19 100644 --- a/tests/unit/core/sync-resilient.test.ts +++ b/tests/unit/core/sync-resilient.test.ts @@ -6,6 +6,7 @@ import { dump } from 'js-yaml'; import { syncWorkspace, syncUserWorkspace } from '../../../src/core/sync.js'; import { CONFIG_DIR, WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; import type { WorkspaceConfig } from '../../../src/models/workspace-config.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('sync resilience - project scope', () => { let testDir: string; @@ -92,16 +93,15 @@ describe('sync resilience - project scope', () => { describe('sync resilience - user scope', () => { let testDir: string; - let originalHome: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { testDir = await mkdtemp(join(tmpdir(), 'allagents-sync-user-resilient-')); - originalHome = process.env.HOME; - process.env.HOME = testDir; + restoreHomeDir = stubHomeDir(testDir); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(testDir, { recursive: true, force: true }); }); diff --git a/tests/unit/core/sync-user.test.ts b/tests/unit/core/sync-user.test.ts index 6531bc6c..ba29d3a9 100644 --- a/tests/unit/core/sync-user.test.ts +++ b/tests/unit/core/sync-user.test.ts @@ -7,19 +7,19 @@ import { dump } from 'js-yaml'; import { syncUserWorkspace } from '../../../src/core/sync.js'; import { WORKSPACE_CONFIG_FILE } from '../../../src/constants.js'; import type { WorkspaceConfig } from '../../../src/models/workspace-config.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('syncUserWorkspace', () => { let testDir: string; - let originalHome: string | undefined; + let restoreHomeDir: () => void; beforeEach(async () => { testDir = await mkdtemp(join(tmpdir(), 'allagents-sync-user-test-')); - originalHome = process.env.HOME; - process.env.HOME = testDir; + restoreHomeDir = stubHomeDir(testDir); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(testDir, { recursive: true, force: true }); }); diff --git a/tests/unit/core/user-workspace-skills.test.ts b/tests/unit/core/user-workspace-skills.test.ts index 5d516792..52f58614 100644 --- a/tests/unit/core/user-workspace-skills.test.ts +++ b/tests/unit/core/user-workspace-skills.test.ts @@ -3,21 +3,20 @@ import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { dump } from 'js-yaml'; - -// Mock home directory -const originalHome = process.env.HOME; +import { stubHomeDir } from '../../helpers/env.js'; describe('user-scope disabledSkills helpers', () => { let tmpDir: string; + let restoreHomeDir: () => void; beforeEach(async () => { tmpDir = await mkdtemp(join(tmpdir(), 'allagents-user-test-')); - process.env.HOME = tmpDir; + restoreHomeDir = stubHomeDir(tmpDir); await mkdir(join(tmpDir, '.allagents'), { recursive: true }); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(tmpDir, { recursive: true, force: true }); }); @@ -64,15 +63,16 @@ describe('user-scope disabledSkills helpers', () => { describe('setUserPluginSkillsMode', () => { let tmpDir: string; + let restoreHomeDir: () => void; beforeEach(async () => { tmpDir = await mkdtemp(join(tmpdir(), 'allagents-user-test-')); - process.env.HOME = tmpDir; + restoreHomeDir = stubHomeDir(tmpDir); await mkdir(join(tmpDir, '.allagents'), { recursive: true }); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(tmpDir, { recursive: true, force: true }); }); diff --git a/tests/unit/core/user-workspace.test.ts b/tests/unit/core/user-workspace.test.ts index f4bc1208..50848c7b 100644 --- a/tests/unit/core/user-workspace.test.ts +++ b/tests/unit/core/user-workspace.test.ts @@ -11,19 +11,19 @@ import { getInstalledUserPlugins, getInstalledProjectPlugins, } from '../../../src/core/user-workspace.js'; +import { stubHomeDir } from '../../helpers/env.js'; describe('user-workspace', () => { let tempHome: string; - let originalHome: string; + let restoreHomeDir: () => void; beforeEach(async () => { tempHome = await mkdtemp(join(tmpdir(), 'allagents-test-')); - originalHome = process.env.HOME || ''; - process.env.HOME = tempHome; + restoreHomeDir = stubHomeDir(tempHome); }); afterEach(async () => { - process.env.HOME = originalHome; + restoreHomeDir(); await rm(tempHome, { recursive: true, force: true }); }); diff --git a/tests/unit/utils/plugin-path.test.ts b/tests/unit/utils/plugin-path.test.ts index 1adbb70b..642d8582 100644 --- a/tests/unit/utils/plugin-path.test.ts +++ b/tests/unit/utils/plugin-path.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, mock, beforeEach } from 'bun:test'; import { join, resolve, sep } from 'node:path'; import { mkdirSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; +import { getHomeDir } from '../../../src/constants.js'; // Mock the git module for verifyGitHubUrlExists tests const repoExistsMock = mock(() => Promise.resolve(true)); @@ -272,7 +273,7 @@ describe('getPluginCachePath', () => { it('should use home directory', () => { const result = getPluginCachePath('owner', 'repo'); - const homeDir = resolve(process.env.HOME || process.env.USERPROFILE || '~'); + const homeDir = resolve(getHomeDir()); expect(result.startsWith(homeDir)).toBe(true); }); });