Skip to content

Commit da0c5b4

Browse files
ARHAEEMclaude
andcommitted
test(mcp): make process-tree + idmap tests deterministic across OSes
Two pre-existing tests only failed on CI (which never ran tests before — install died on the lockfile), one per platform: - findProfileBrowserPids (Linux): readProcessArgs read the runner's REAL /proc first, so the fake PIDs 55/66/77 resolved to kernel threads with EMPTY cmdlines and the mocked `ps` path was unreachable → []. Make the /proc reader injectable (production default unchanged) and have the posix test pass readProc:()=>null to force the `ps` mock. - latestDiffId (Windows): coarse NTFS mtime resolution stamps two same-tick saves identically, so d2 wasn't > d1. Backdate d1 with utimesSync so d2 is unambiguously newer (mirrors production, where diffs are saved seconds/minutes apart). macos CI was already green; these close the ubuntu + windows gaps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cf4e987 commit da0c5b4

3 files changed

Lines changed: 26 additions & 10 deletions

File tree

packages/mcp-server/src/process-tree.js

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ async function collectDescendantPids(rootPid, exec) {
8787
*/
8888
export async function findProfileBrowserPids(
8989
userDataDir,
90-
{ platform = process.platform, exec = execFile } = {},
90+
{ platform = process.platform, exec = execFile, readProc } = {},
9191
) {
9292
const marker = `--user-data-dir=${userDataDir}`;
9393

@@ -133,32 +133,40 @@ export async function findProfileBrowserPids(
133133
return [];
134134
}
135135
}
136-
return filterRootBrowserPids(pids, exec);
136+
return filterRootBrowserPids(pids, exec, readProc);
137137
}
138138

139139
/**
140140
* Drop Chromium helper processes (`--type=...`) so we only kill root browsers.
141141
* Linux: /proc/<pid>/cmdline. macOS/BSD: ps -p <pid> -o args=.
142142
*/
143-
async function filterRootBrowserPids(pids, exec) {
143+
async function filterRootBrowserPids(pids, exec, readProc = readProcCmdline) {
144144
const roots = [];
145145
for (const pid of pids) {
146-
const args = await readProcessArgs(pid, exec);
146+
const args = await readProcessArgs(pid, exec, readProc);
147147
if (!args) continue;
148148
if (args.includes('--type=')) continue;
149149
roots.push(pid);
150150
}
151151
return roots;
152152
}
153153

154-
async function readProcessArgs(pid, exec) {
155-
// Prefer /proc on Linux (no shell).
154+
// Default /proc reader (Linux fast-path, no shell). Returns the space-joined cmdline (may be '' for
155+
// a kernel thread), or null when /proc is unavailable (not Linux / pid gone) so the caller falls
156+
// back to `ps`. Injectable so tests can bypass the runner's REAL /proc (whose low PIDs 55/66/77 are
157+
// kernel threads with empty cmdlines — that made the mocked `ps` path unreachable and the test fail
158+
// only on Linux CI).
159+
function readProcCmdline(pid) {
156160
try {
157-
const raw = readFileSync(`/proc/${pid}/cmdline`, 'utf8');
158-
return raw.replace(/\0/g, ' ');
161+
return readFileSync(`/proc/${pid}/cmdline`, 'utf8').replace(/\0/g, ' ');
159162
} catch {
160-
// not Linux or pid gone
163+
return null;
161164
}
165+
}
166+
167+
async function readProcessArgs(pid, exec, readProc = readProcCmdline) {
168+
const fromProc = readProc(pid);
169+
if (fromProc != null) return fromProc;
162170
try {
163171
const { stdout } = await exec('ps', ['-p', String(pid), '-o', 'args='], { timeout: 5_000 });
164172
return String(stdout || '');

packages/mcp-server/test/sync/test-idmap.test.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, it } from 'node:test';
22
import assert from 'node:assert/strict';
33
import { join } from 'node:path';
4-
import { mkdtempSync, existsSync } from 'node:fs';
4+
import { mkdtempSync, existsSync, utimesSync } from 'node:fs';
55
import { tmpdir } from 'node:os';
66
import { matchByName, saveIdmap, loadIdmap, syncDir, saveDiff, loadDiff, latestDiffId } from '../../src/sync/idmap.js';
77

@@ -81,6 +81,11 @@ describe('idmap diff I/O', () => {
8181
it('latestDiffId returns the id of the most recently saved diff', () => {
8282
process.env.AIRTABLE_USER_MCP_HOME = mkdtempSync(join(tmpdir(), 'sync-diff-test-'));
8383
saveDiff(SRC, DEST, { diffId: 'd1', tables: [] });
84+
// Coarse FS mtime resolution (Windows/NTFS) stamps two same-tick writes identically, so
85+
// latestDiffId's mtime compare can't tell d2 from d1. Backdate d1 so d2 is unambiguously
86+
// newer — mirrors production, where diffs are saved seconds/minutes apart.
87+
const past = new Date(Date.now() - 5000);
88+
utimesSync(join(syncDir(SRC, DEST), 'diff-d1.json'), past, past);
8489
assert.equal(latestDiffId(SRC, DEST), 'd1');
8590
// Save a second diff later — it should win
8691
saveDiff(SRC, DEST, { diffId: 'd2', tables: ['x'] });

packages/mcp-server/test/test-process-tree.test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ describe("findProfileBrowserPids", () => {
7575
const pids = await findProfileBrowserPids("/home/u/.chrome-profile", {
7676
platform: "linux",
7777
exec: (f, a, o) => s.exec(f, a, o),
78+
// Force the mocked `ps` path: on real Linux CI, /proc/55|66|77/cmdline are kernel threads
79+
// with EMPTY cmdlines, which would bypass the mock and make this test fail only on Linux.
80+
readProc: () => null,
7881
});
7982
assert.deepEqual(pids, [55]);
8083
assert.equal(s.calls[0].file, "pgrep");

0 commit comments

Comments
 (0)