Skip to content

Commit a3c5839

Browse files
fix(core): isolate cache env vars in splitArgs spec (#35584)
## Current Behavior The `splitArgs` describe block in `packages/nx/src/utils/command-line-utils.spec.ts` saves and clears `NX_BASE`, `NX_HEAD`, and `NX_PARALLEL` so the surrounding shell environment doesn't bleed into assertions. However, the production code in `splitArgsIntoNxArgsAndOverrides` also reads four cache-related env vars as fallbacks for the `skipNxCache` / `skipRemoteCache` defaults: - `NX_SKIP_NX_CACHE` - `NX_DISABLE_NX_CACHE` - `NX_SKIP_REMOTE_CACHE` - `NX_DISABLE_REMOTE_CACHE` These are not isolated. When a developer runs the test suite via the outer `nx` invocation with flags like `--skipNxCache`, nx propagates them to spawned child processes as `NX_SKIP_NX_CACHE=true`. That env var leaks into the Jest worker, flips `skipNxCache` to `true`, and breaks every test in the `splitArgs` block that asserts on the defaults — six failures in total (`should split nx specific arguments into nxArgs`, `should default to having a base of main`, `should return configured base branch from nx.json`, `should return a default base branch if not configured in nx.json`, `should split projects when it is a string`, `should set base and head based on environment variables in affected mode`). ## Expected Behavior The `splitArgs` specs should pass regardless of which cache-related flags or env vars are set in the surrounding environment, just like they already do for `NX_BASE` / `NX_HEAD` / `NX_PARALLEL`. This PR extends the existing save/clear/restore pattern to cover the four cache env vars and factors the conditional-restore logic (delete when originally unset, otherwise reassign — to avoid Node's `process.env[key] = undefined` coercing to the string `"undefined"`) into a small `restoreEnv` helper. ## Related Issue(s) N/A --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com>
1 parent 509c7d1 commit a3c5839

3 files changed

Lines changed: 48 additions & 20 deletions

File tree

packages/nx/src/ai/clone-ai-config-repo.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,17 +133,34 @@ export function getAiConfigRepoPath(): string {
133133
// 1. Get latest commit hash (first 10 chars)
134134
const commitHash = getLatestCommitHash();
135135

136-
// 2. Check if cached version exists
136+
// 2. Reuse cached version if it still has content (macOS may have
137+
// swept its files but left the directory tree).
137138
const cachedPath = join(CACHE_DIR, commitHash);
138-
if (existsSync(cachedPath)) {
139+
if (hasRootFile(cachedPath)) {
139140
return cachedPath;
140141
}
141142

142-
// 3. Clone fresh
143+
// 3. Wipe any empty skeleton, then clone fresh
144+
if (existsSync(cachedPath)) {
145+
rmSync(cachedPath, { recursive: true, force: true });
146+
}
143147
cloneRepo(cachedPath);
144148

145149
// 4. Clean up old cached versions
146150
cleanupOldCaches(commitHash);
147151

148152
return cachedPath;
149153
}
154+
155+
/**
156+
* The repo always has at least one regular file at its root (e.g. README).
157+
* If everything at the root is a directory, the cache was swept by macOS
158+
* tmp cleanup and we should re-clone.
159+
*/
160+
function hasRootFile(dir: string): boolean {
161+
try {
162+
return readdirSync(dir, { withFileTypes: true }).some((e) => e.isFile());
163+
} catch {
164+
return false;
165+
}
166+
}

packages/nx/src/native/watch/watcher.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,9 @@ mod tests {
495495
}
496496
});
497497
w.watch_inner(callback).expect("start watch");
498-
std::thread::sleep(Duration::from_millis(150));
498+
// Drop any startup events FSEvents leaks from before the watch began.
499+
std::thread::sleep(Duration::from_millis(300));
500+
captured.lock().unwrap().clear();
499501
(w, captured)
500502
}
501503

packages/nx/src/utils/command-line-utils.spec.ts

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,36 @@ import { withEnvironmentVariables as withEnvironment } from '../internal-testing
44
jest.mock('../project-graph/file-utils');
55

66
describe('splitArgs', () => {
7-
let originalBase: string;
8-
let originalHead: string;
9-
let originalParallel: string;
7+
const blockedEnvVars = [
8+
'NX_BASE',
9+
'NX_HEAD',
10+
'NX_PARALLEL',
11+
'NX_SKIP_NX_CACHE',
12+
'NX_DISABLE_NX_CACHE',
13+
'NX_SKIP_REMOTE_CACHE',
14+
'NX_DISABLE_REMOTE_CACHE',
15+
];
16+
let envVarsToRestore: Record<string, string | undefined> = {};
17+
18+
function blockParentEnvVar(key: string) {
19+
envVarsToRestore[key] = process.env[key];
20+
delete process.env[key];
21+
}
1022

1123
beforeEach(() => {
12-
originalBase = process.env.NX_BASE;
13-
originalHead = process.env.NX_HEAD;
14-
originalParallel = process.env.NX_PARALLEL;
15-
16-
delete process.env.NX_BASE;
17-
delete process.env.NX_HEAD;
18-
delete process.env.NX_PARALLEL;
24+
envVarsToRestore = {};
25+
for (const key of blockedEnvVars) {
26+
blockParentEnvVar(key);
27+
}
1928
});
2029

2130
afterEach(() => {
22-
process.env.NX_BASE = originalBase;
23-
process.env.NX_HEAD = originalHead;
24-
if (originalParallel === undefined) {
25-
delete process.env.NX_PARALLEL;
26-
} else {
27-
process.env.NX_PARALLEL = originalParallel;
31+
for (const [key, value] of Object.entries(envVarsToRestore)) {
32+
if (value === undefined) {
33+
delete process.env[key];
34+
} else {
35+
process.env[key] = value;
36+
}
2837
}
2938
});
3039

0 commit comments

Comments
 (0)