fix(tests): isolated CI-environment assumptions across the coding-agent suite
The CI gate failed in several non-overlapping ways once the full coding-agent suite ran here: tests wrote into the real $HOME (`/srv/agent-home`) which is read-only, a fixture rotated stored Anthropic API keys but Settings reloaded the user models.yml and shadowed them, an OAuth callback server bound to `hostname:"localhost"` (loopback unreachable in this runtime), a built-in tool metadata assertion only saw `github` when `gh` was installed, and the GithubTool `pr_checkout` worktree assertions assumed `~/.omp/wt` but `XDG_DATA_HOME` redirected `getWorktreesDir()` to `$XDG_DATA_HOME/omp/wt`. Fixes: - `packages/ai/src/registry/oauth/callback-server.ts`: drop `hostname: "localhost"` when no caller-supplied hostname overrides it. Bun on Linux refused inbound connections to the listener when bound explicitly to `localhost`; defaulting to Bun.serves default binding restores loopback connectivity. - `packages/coding-agent/test/status-line-path.test.ts`: route the `~/Projects` fixtures through a writable temp home (spy `os.homedir()`), housed under the repo `.wt/` worktree scratch so the temp home is not classified as a status-line scratch root. - `packages/coding-agent/test/skills.test.ts`: ditto for the `~/.pi-skills-test-*` mkdtemp in the tilde-expansion test. - `packages/coding-agent/test/marketplace/project-scope.test.ts`: stop writing `~/.git`; build the entire home-dir guard fixture in a temp dir and spy `os.homedir()`. - `packages/coding-agent/test/oauth-flow.test.ts`: wrap each callback fetch in a brief retry so the simulated browser redirect tolerates the few-ms gap before the Bun callback server starts accepting connections. - `packages/coding-agent/test/tools/gh.test.ts`: extend `setupTempHome()` to clear `XDG_DATA_HOME`/`XDG_STATE_HOME`/`XDG_CACHE_HOME` for the duration of the test so the rebuilt dirs resolver routes `getWorktreesDir()` back through the spied home, then restore them on cleanup. - `packages/coding-agent/test/tool-discovery/initial-tools.test.ts`: instantiate `GithubTool` directly in the metadata fixture so the assertion runs even when `gh` is unavailable (GithubTool.createIf returns null without `gh`). - `packages/coding-agent/test/agent-session-retry-cap.test.ts`: pass an isolated `models.yml` path to `ModelRegistry` so the two-Anthropic-key fixture is the authoritative credential source instead of any user-level command-backed Anthropic key. Verification: - `bun check` → passed - `bun run test` (full coding-agent suite, 4 buckets, all chunks) → 0 fails Fixes #3639
This commit is contained in:
roboomp
@@ -144,8 +144,9 @@ export abstract class OAuthCallbackFlow {
|
||||
* Create HTTP server for OAuth callback.
|
||||
*/
|
||||
#createServer(port: number, expectedState: string): Bun.Server<unknown> {
|
||||
const hostname = this.callbackHostname === DEFAULT_HOSTNAME ? undefined : this.callbackHostname;
|
||||
return Bun.serve({
|
||||
hostname: this.callbackHostname,
|
||||
...(hostname === undefined ? {} : { hostname }),
|
||||
port,
|
||||
reusePort: false,
|
||||
fetch: req => this.#handleCallback(req, expectedState),
|
||||
|
||||
@@ -55,7 +55,7 @@ describe("AgentSession retry delay cap", () => {
|
||||
tempDir = TempDir.createSync("@pi-retry-cap-");
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
authStorage.setRuntimeApiKey("anthropic", "anthropic-test-key");
|
||||
modelRegistry = new ModelRegistry(authStorage);
|
||||
modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
* This file imports from helpers.ts directly — the native addon IS present in the
|
||||
* test environment (verified: `bun run import-helpers.ts` succeeds).
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
@@ -48,6 +48,7 @@ describe("resolveActiveProjectRegistryPath", () => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
removeSyncWithRetries(tmpDir);
|
||||
});
|
||||
|
||||
@@ -99,23 +100,19 @@ describe("resolveActiveProjectRegistryPath", () => {
|
||||
it("does not treat ~/.git as a project root (pass-2 home-dir guard)", async () => {
|
||||
// Simulate a dotfiles repo managed with a bare-git technique: ~/.git exists.
|
||||
// resolveActiveProjectRegistryPath must NOT return ~/.omp/.../installed_plugins.json.
|
||||
const homeDir = os.homedir();
|
||||
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-proj-scope-home-"));
|
||||
vi.spyOn(os, "homedir").mockReturnValue(homeDir);
|
||||
const fakeHomeGit = path.join(homeDir, ".git");
|
||||
const hadGit = await fs.promises
|
||||
.stat(fakeHomeGit)
|
||||
.then(() => true)
|
||||
.catch(() => false);
|
||||
if (!hadGit) {
|
||||
await fs.promises.mkdir(fakeHomeGit, { recursive: true });
|
||||
}
|
||||
await fs.promises.mkdir(fakeHomeGit, { recursive: true });
|
||||
const cwd = path.join(homeDir, "work");
|
||||
await fs.promises.mkdir(cwd, { recursive: true });
|
||||
try {
|
||||
// Start from a tmpDir that has no .omp/ or .git/ of its own.
|
||||
const result = await resolveActiveProjectRegistryPath(tmpDir);
|
||||
// Must not resolve to the home-dir OMP registry.
|
||||
const result = await resolveActiveProjectRegistryPath(cwd);
|
||||
const homeOmpPath = path.join(homeDir, ".omp", "plugins", "installed_plugins.json");
|
||||
expect(result).not.toBe(homeOmpPath);
|
||||
expect(result).toBeNull();
|
||||
} finally {
|
||||
if (!hadGit) await fs.promises.rm(fakeHomeGit, { recursive: true, force: true });
|
||||
removeSyncWithRetries(homeDir);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -45,6 +45,21 @@ function mockFigmaRegistration(onRegistration: (payload: Record<string, unknown>
|
||||
};
|
||||
}
|
||||
|
||||
async function completeLocalOAuthCallback(url: string): Promise<void> {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 0; attempt < 20; attempt++) {
|
||||
try {
|
||||
const response = await fetch(url);
|
||||
await response.text();
|
||||
return;
|
||||
} catch (error) {
|
||||
lastError = error;
|
||||
await Bun.sleep(5);
|
||||
}
|
||||
}
|
||||
throw lastError;
|
||||
}
|
||||
|
||||
describe("mcp oauth flow", () => {
|
||||
it("uses Codex client name for dynamic client registration", async () => {
|
||||
let registrationPayload: Record<string, unknown> | null = null;
|
||||
@@ -152,7 +167,7 @@ describe("mcp oauth flow", () => {
|
||||
observedRedirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`${observedRedirectUri}?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(`${observedRedirectUri}?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
@@ -192,7 +207,7 @@ describe("mcp oauth flow", () => {
|
||||
const redirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`${redirectUri}?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(`${redirectUri}?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
@@ -228,7 +243,7 @@ describe("mcp oauth flow", () => {
|
||||
const redirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`${redirectUri}?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(`${redirectUri}?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
@@ -265,7 +280,9 @@ describe("mcp oauth flow", () => {
|
||||
observedRedirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`http://localhost:14568/slack/oauth_redirect?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(
|
||||
`http://localhost:14568/slack/oauth_redirect?code=test-code&state=${state}`,
|
||||
);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
@@ -305,7 +322,7 @@ describe("mcp oauth flow", () => {
|
||||
observedRedirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`http://localhost:14571/?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(`http://localhost:14571/?code=test-code&state=${state}`);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
@@ -343,7 +360,9 @@ describe("mcp oauth flow", () => {
|
||||
observedRedirectUri = authUrl.searchParams.get("redirect_uri") ?? "";
|
||||
const state = authUrl.searchParams.get("state") ?? "";
|
||||
queueMicrotask(() => {
|
||||
void fetch(`http://localhost:14570/slack/oauth_redirect?code=test-code&state=${state}`);
|
||||
void completeLocalOAuthCallback(
|
||||
`http://localhost:14570/slack/oauth_redirect?code=test-code&state=${state}`,
|
||||
);
|
||||
});
|
||||
},
|
||||
signal: AbortSignal.timeout(1_000),
|
||||
|
||||
@@ -360,8 +360,10 @@ enabled: false
|
||||
});
|
||||
|
||||
it("should expand ~ in customDirectories", async () => {
|
||||
const tempHomeSkillsDir = await fs.mkdtemp(path.join(os.homedir(), ".pi-skills-test-"));
|
||||
const relativeToHome = path.relative(os.homedir(), tempHomeSkillsDir);
|
||||
const fakeHome = await fs.mkdtemp(path.join(os.tmpdir(), "pi-skills-home-"));
|
||||
const homedirSpy = spyOn(os, "homedir").mockReturnValue(fakeHome);
|
||||
const tempHomeSkillsDir = await fs.mkdtemp(path.join(fakeHome, ".pi-skills-test-"));
|
||||
const relativeToHome = path.relative(fakeHome, tempHomeSkillsDir);
|
||||
const tildeDir = `~/${relativeToHome.split(path.sep).join("/")}`;
|
||||
const skillDir = path.join(tempHomeSkillsDir, "tilde-skill");
|
||||
const skillPath = path.join(skillDir, "SKILL.md");
|
||||
@@ -389,7 +391,8 @@ description: Skill loaded from a tilde-expanded custom directory.
|
||||
expect(withTilde.length).toBe(withoutTilde.length);
|
||||
expect(withTilde.some(skill => skill.name === "tilde-skill")).toBe(true);
|
||||
} finally {
|
||||
await removeWithRetries(tempHomeSkillsDir);
|
||||
homedirSpy.mockRestore();
|
||||
await removeWithRetries(fakeHome);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterEach, beforeAll, describe, expect, it } from "bun:test";
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
@@ -58,6 +58,7 @@ function createPathContext(): SegmentContext {
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
setProjectDir(originalProjectDir);
|
||||
});
|
||||
|
||||
@@ -69,12 +70,21 @@ function expectContentToContainPath(content: string, expected: string): void {
|
||||
expect(content).toContain(expected);
|
||||
}
|
||||
|
||||
function createFakeHome(): { home: string; projectsRoot: string } {
|
||||
const homeRoot = path.join(originalProjectDir, ".wt");
|
||||
fs.mkdirSync(homeRoot, { recursive: true });
|
||||
const home = fs.mkdtempSync(path.join(homeRoot, "omp-status-line-home-"));
|
||||
const projectsRoot = path.join(home, "Projects");
|
||||
fs.mkdirSync(projectsRoot, { recursive: true });
|
||||
vi.spyOn(os, "homedir").mockReturnValue(home);
|
||||
return { home, projectsRoot };
|
||||
}
|
||||
|
||||
describe("status line path segment", () => {
|
||||
it("strips the Projects root for symlink-equivalent aliases", () => {
|
||||
if (process.platform === "win32") return;
|
||||
|
||||
const projectsRoot = path.join(os.homedir(), "Projects");
|
||||
fs.mkdirSync(projectsRoot, { recursive: true });
|
||||
const { home, projectsRoot } = createFakeHome();
|
||||
|
||||
const realProjectDir = fs.mkdtempSync(path.join(projectsRoot, "omp-status-line-"));
|
||||
const nestedDir = path.join(realProjectDir, "nested");
|
||||
@@ -83,7 +93,7 @@ describe("status line path segment", () => {
|
||||
|
||||
try {
|
||||
fs.mkdirSync(nestedDir, { recursive: true });
|
||||
fs.symlinkSync(os.homedir(), homeAlias, "dir");
|
||||
fs.symlinkSync(home, homeAlias, "dir");
|
||||
|
||||
const aliasedDir = path.join(homeAlias, "Projects", path.basename(realProjectDir), "nested");
|
||||
setProjectDir(aliasedDir);
|
||||
@@ -99,6 +109,7 @@ describe("status line path segment", () => {
|
||||
setProjectDir(originalProjectDir);
|
||||
removeSyncWithRetries(aliasRoot);
|
||||
removeSyncWithRetries(realProjectDir);
|
||||
removeSyncWithRetries(home);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -156,8 +167,7 @@ describe("status line path segment", () => {
|
||||
});
|
||||
|
||||
it("keeps the folder icon for paths outside any scratch root", () => {
|
||||
const projectsRoot = path.join(os.homedir(), "Projects");
|
||||
fs.mkdirSync(projectsRoot, { recursive: true });
|
||||
const { home, projectsRoot } = createFakeHome();
|
||||
const realProjectDir = fs.mkdtempSync(path.join(projectsRoot, "omp-status-line-real-"));
|
||||
try {
|
||||
setProjectDir(realProjectDir);
|
||||
@@ -169,6 +179,7 @@ describe("status line path segment", () => {
|
||||
} finally {
|
||||
setProjectDir(originalProjectDir);
|
||||
removeSyncWithRetries(realProjectDir);
|
||||
removeSyncWithRetries(home);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
createTools,
|
||||
DEFAULT_ESSENTIAL_TOOL_NAMES,
|
||||
filterInitialToolsForDiscoveryAll,
|
||||
GithubTool,
|
||||
IrcTool,
|
||||
JobTool,
|
||||
SshTool,
|
||||
@@ -47,6 +48,7 @@ async function getToolMetadata(): Promise<Map<string, { loadMode?: string; summa
|
||||
const metadata = new Map(tools.map(tool => [tool.name, { loadMode: tool.loadMode, summary: tool.summary }]));
|
||||
for (const tool of [
|
||||
new AskTool({ ...toolSession, hasUI: true }),
|
||||
new GithubTool(toolSession),
|
||||
new SshTool(toolSession, [], new Map(), ""),
|
||||
new JobTool(toolSession),
|
||||
new IrcTool(toolSession),
|
||||
|
||||
@@ -188,15 +188,28 @@ interface TempHome {
|
||||
async function setupTempHome(): Promise<{ home: string; cleanup: () => Promise<void> }> {
|
||||
const home = await fs.mkdtemp(path.join(os.tmpdir(), "gh-pr-tool-home-"));
|
||||
vi.spyOn(os, "homedir").mockReturnValue(home);
|
||||
// Clear XDG_*_HOME so the rebuilt resolver routes `dirs.rootSubdir("wt", "data")`
|
||||
// through the spied homedir instead of `$XDG_DATA_HOME/omp/wt` (CI sets these).
|
||||
const xdgKeys = ["XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME"] as const;
|
||||
const xdgPrevious: Partial<Record<(typeof xdgKeys)[number], string | undefined>> = {};
|
||||
for (const key of xdgKeys) {
|
||||
xdgPrevious[key] = process.env[key];
|
||||
delete process.env[key];
|
||||
}
|
||||
// `dirs.configRoot` is computed at constructor time from `os.homedir()`, so
|
||||
// we must rebuild the resolver after the spy is in place. `setAgentDir`
|
||||
// recreates it; we point it at the temp home's default agent dir.
|
||||
// we must rebuild the resolver after the spy + env scrub are in place.
|
||||
// `setAgentDir` recreates it; we point it at the temp home's default agent dir.
|
||||
const originalAgentDir = getAgentDir();
|
||||
setAgentDir(path.join(home, ".omp", "agent"));
|
||||
return {
|
||||
home,
|
||||
cleanup: async () => {
|
||||
setAgentDir(originalAgentDir);
|
||||
for (const key of xdgKeys) {
|
||||
const previous = xdgPrevious[key];
|
||||
if (previous === undefined) delete process.env[key];
|
||||
else process.env[key] = previous;
|
||||
}
|
||||
await removeWithRetries(home);
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user