fix: isolate pi provider smoke
This commit is contained in:
@@ -1,4 +1,7 @@
|
||||
import { spawn as nodeSpawn, type ChildProcessWithoutNullStreams } from "node:child_process";
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { homedir, tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import type { AppConfig } from "../config.js";
|
||||
import { secretValue } from "../config/secret-bundle.js";
|
||||
import { clearPrincipalEnvironment } from "../auth/principal.js";
|
||||
@@ -7,7 +10,18 @@ import { loadPiAuthProviders } from "./auth-providers.js";
|
||||
import { buildPiChildEnv, canonicalPiProvider } from "./provider-credentials.js";
|
||||
import type { PiReasoning } from "./management.js";
|
||||
|
||||
const SMOKE_PROMPT = "Provider health check only. Reply with exactly OK without using tools.";
|
||||
const SMOKE_PROMPT = "Provider health check. Reply with exactly OK.";
|
||||
const SMOKE_ARGS = [
|
||||
"--mode", "rpc",
|
||||
"--no-session",
|
||||
"--no-tools",
|
||||
"--no-extensions",
|
||||
"--no-skills",
|
||||
"--no-prompt-templates",
|
||||
"--no-themes",
|
||||
"--no-context-files",
|
||||
"--no-approve",
|
||||
] as const;
|
||||
|
||||
export interface PiProviderSmokeRequest {
|
||||
provider: string;
|
||||
@@ -25,6 +39,7 @@ interface ProviderSmokeOptions {
|
||||
options: { cwd: string; env: NodeJS.ProcessEnv },
|
||||
) => ChildProcessWithoutNullStreams;
|
||||
authProviders?: () => ReadonlySet<string>;
|
||||
readAuthStore?: () => string;
|
||||
}
|
||||
|
||||
export function createPiProviderSmoke(
|
||||
@@ -36,23 +51,43 @@ export function createPiProviderSmoke(
|
||||
|
||||
return async ({ provider, model, reasoning, timeoutMs }): Promise<void> => {
|
||||
let child: ChildProcessWithoutNullStreams | undefined;
|
||||
let isolatedRoot: string | undefined;
|
||||
let timer: NodeJS.Timeout | undefined;
|
||||
try {
|
||||
const canonicalProvider = canonicalPiProvider(provider);
|
||||
if (!canonicalProvider || timeoutMs <= 0) throw providerFailure();
|
||||
const configuredAuthProviders = authProviders();
|
||||
const env = buildPiChildEnv({
|
||||
provider: canonicalProvider,
|
||||
authProviders: authProviders(),
|
||||
authProviders: configuredAuthProviders,
|
||||
credentialValue: secretValue(config, "THT_MODEL_API_KEY"),
|
||||
credentialFile: config.modelApiKeyFile,
|
||||
});
|
||||
clearPrincipalEnvironment(env);
|
||||
delete env.THT_DATA_ROOT;
|
||||
if (config.dataRoot !== undefined) env.THT_DATA_ROOT = config.dataRoot;
|
||||
delete env.THT_SESSION;
|
||||
delete env.THT_AUTHOR;
|
||||
delete env.THT_CONFIG;
|
||||
delete env.PI_CODING_AGENT_SESSION_DIR;
|
||||
|
||||
child = spawnFn(config.piBin, ["--mode", "rpc"], { cwd: config.harnessDir, env });
|
||||
isolatedRoot = mkdtempSync(join(tmpdir(), "thothii-pi-smoke-"));
|
||||
const isolatedCwd = join(isolatedRoot, "work");
|
||||
const isolatedAgentDir = join(isolatedRoot, "agent");
|
||||
mkdirSync(isolatedCwd, { mode: 0o700 });
|
||||
mkdirSync(isolatedAgentDir, { mode: 0o700 });
|
||||
if (configuredAuthProviders.has(canonicalProvider)) {
|
||||
const authStore = selectedProviderAuthStore(
|
||||
options.readAuthStore?.() ?? readConfiguredAuthStore(),
|
||||
canonicalProvider,
|
||||
);
|
||||
writeFileSync(join(isolatedAgentDir, "auth.json"), authStore, { mode: 0o600, flag: "wx" });
|
||||
}
|
||||
env.PI_CODING_AGENT_DIR = isolatedAgentDir;
|
||||
|
||||
child = spawnFn(config.piBin, [...SMOKE_ARGS], { cwd: isolatedCwd, env });
|
||||
child.stderr.resume();
|
||||
const rpc = new RpcClient(child);
|
||||
const capabilityGuard = failOnUnexpectedCapabilities(rpc);
|
||||
const turn = async (): Promise<void> => {
|
||||
requireSuccessfulResponse(await rpc.request({
|
||||
type: "set_model", provider: canonicalProvider, modelId: model,
|
||||
@@ -64,6 +99,7 @@ export function createPiProviderSmoke(
|
||||
};
|
||||
await Promise.race([
|
||||
turn(),
|
||||
capabilityGuard,
|
||||
new Promise<never>((_resolve, reject) => {
|
||||
timer = setTimeout(() => reject(providerTimeout()), timeoutMs);
|
||||
}),
|
||||
@@ -76,10 +112,55 @@ export function createPiProviderSmoke(
|
||||
if (child) {
|
||||
try { child.kill(); } catch { /* preserve the sanitized smoke outcome */ }
|
||||
}
|
||||
if (isolatedRoot) {
|
||||
try { rmSync(isolatedRoot, { recursive: true, force: true, maxRetries: 2 }); } catch {
|
||||
/* preserve the sanitized smoke outcome; the OS temp directory remains isolated */
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
function failOnUnexpectedCapabilities(rpc: RpcClient): Promise<never> {
|
||||
return new Promise((_resolve, reject) => {
|
||||
rpc.on("event", (event) => {
|
||||
if (isUnexpectedCapabilityEvent(event)) reject(providerFailure());
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function isUnexpectedCapabilityEvent(event: any): boolean {
|
||||
const type = typeof event?.type === "string" ? event.type : "";
|
||||
if (type.startsWith("tool_") || type.startsWith("toolcall_") || type.startsWith("extension_")) {
|
||||
return true;
|
||||
}
|
||||
const updateType = event?.assistantMessageEvent?.type;
|
||||
if (typeof updateType === "string" && updateType.startsWith("toolcall_")) return true;
|
||||
if (Array.isArray(event?.toolResults) && event.toolResults.length > 0) return true;
|
||||
if (messageUsesTool(event?.message)) return true;
|
||||
return Array.isArray(event?.messages) && event.messages.some(messageUsesTool);
|
||||
}
|
||||
|
||||
function messageUsesTool(message: any): boolean {
|
||||
return message?.role === "toolResult" || message?.stopReason === "toolUse"
|
||||
|| (Array.isArray(message?.content)
|
||||
&& message.content.some((content: any) => content?.type === "toolCall"));
|
||||
}
|
||||
|
||||
function readConfiguredAuthStore(): string {
|
||||
const configuredAgentDir = process.env.PI_CODING_AGENT_DIR ?? join(homedir(), ".pi", "agent");
|
||||
return readFileSync(join(configuredAgentDir, "auth.json"), "utf8");
|
||||
}
|
||||
|
||||
function selectedProviderAuthStore(raw: string, provider: string): string {
|
||||
const parsed: unknown = JSON.parse(raw);
|
||||
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) throw providerFailure();
|
||||
const entry = Object.entries(parsed as Record<string, unknown>)
|
||||
.find(([key]) => key.trim().toLowerCase() === provider);
|
||||
if (!entry) throw providerFailure();
|
||||
return JSON.stringify({ [entry[0]]: entry[1] });
|
||||
}
|
||||
|
||||
function waitForProviderTurn(rpc: RpcClient, child: ChildProcessWithoutNullStreams): Promise<void> {
|
||||
return new Promise((resolve, reject) => {
|
||||
let failed = false;
|
||||
|
||||
@@ -47,7 +47,10 @@ test("status parses only a Pi version from a fixed execFile argument array", asy
|
||||
config: { provider: "zai", model: "glm-5.2", reasoning: "medium" },
|
||||
checkedAt: "2026-08-05T10:00:00.000Z",
|
||||
});
|
||||
expect(calls).toEqual([{ command: "/usr/local/bin/pi", args: ["--version"], timeout: 750 }]);
|
||||
expect(calls).toHaveLength(1);
|
||||
expect(calls[0]).toMatchObject({ command: "/usr/local/bin/pi", args: ["--version"] });
|
||||
expect(calls[0].timeout).toBeGreaterThan(0);
|
||||
expect(calls[0].timeout).toBeLessThanOrEqual(750);
|
||||
});
|
||||
|
||||
// Catches an options response that leaks provider metadata or lets callers choose model IDs that
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import { EventEmitter } from "node:events";
|
||||
import { expect, test, vi } from "vitest";
|
||||
import { existsSync, readFileSync, readdirSync } from "node:fs";
|
||||
import { dirname } from "node:path";
|
||||
import { afterEach, expect, test, vi } from "vitest";
|
||||
import { loadConfig } from "../src/config.js";
|
||||
import { createPiProviderSmoke } from "../src/pi/provider-smoke.js";
|
||||
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
|
||||
function rpcChild(onCommand: (command: any, emit: (message: unknown) => void) => void) {
|
||||
const child: any = new EventEmitter();
|
||||
child.stdout = new EventEmitter();
|
||||
@@ -20,10 +24,17 @@ function rpcChild(onCommand: (command: any, emit: (message: unknown) => void) =>
|
||||
return child;
|
||||
}
|
||||
|
||||
// Catches a provider smoke implementation that merely selects a model, leaks generated output,
|
||||
// or fails to terminate its ephemeral Pi process after a real model turn.
|
||||
test("provider smoke selects the configured model and completes a fixed output-discarding turn", async () => {
|
||||
// Catches a provider smoke process that runs from the trusted harness or leaves Pi tools,
|
||||
// extensions, skills, context files, templates, themes, or session persistence enabled.
|
||||
test("provider smoke makes one configured request from an isolated no-capability Pi process", async () => {
|
||||
vi.stubEnv("THT_DATA_ROOT", "/mounted-workflow-state");
|
||||
vi.stubEnv("THT_SESSION", "mounted-session-id");
|
||||
vi.stubEnv("THT_AUTHOR", "mounted-author");
|
||||
vi.stubEnv("THT_CONFIG", "/mounted-workflow-state/config.yaml");
|
||||
vi.stubEnv("PI_CODING_AGENT_DIR", "/home/thoth/.pi/agent");
|
||||
vi.stubEnv("PI_CODING_AGENT_SESSION_DIR", "/mounted-session-state");
|
||||
const commands: any[] = [];
|
||||
const spawns: any[][] = [];
|
||||
const child = rpcChild((command, emit) => {
|
||||
commands.push(command);
|
||||
if (command.type === "set_model" || command.type === "set_thinking_level") {
|
||||
@@ -43,14 +54,47 @@ test("provider smoke selects the configured model and completes a fixed output-d
|
||||
const smoke = createPiProviderSmoke(loadConfig({
|
||||
THT_HARNESS_DIR: "/app/harness",
|
||||
PI_BIN: "/usr/local/bin/pi",
|
||||
THT_DATA_ROOT: "/mounted-workflow-state",
|
||||
}), {
|
||||
spawnFn: () => child,
|
||||
spawnFn: (...args) => {
|
||||
spawns.push(args);
|
||||
expect(args[2].cwd).not.toBe("/app/harness");
|
||||
expect(readdirSync(args[2].cwd)).toEqual([]);
|
||||
expect(args[2].env.PI_CODING_AGENT_DIR).not.toBe("/home/thoth/.pi/agent");
|
||||
expect(readdirSync(args[2].env.PI_CODING_AGENT_DIR)).toEqual(["auth.json"]);
|
||||
expect(JSON.parse(readFileSync(`${args[2].env.PI_CODING_AGENT_DIR}/auth.json`, "utf8")))
|
||||
.toEqual({ zai: { type: "api_key", key: "test-only" } });
|
||||
return child;
|
||||
},
|
||||
authProviders: () => new Set(["zai"]),
|
||||
readAuthStore: () => JSON.stringify({
|
||||
zai: { type: "api_key", key: "test-only" },
|
||||
deepseek: { type: "api_key", key: "must-not-enter-isolated-context" },
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(smoke({
|
||||
provider: "zai", model: "glm-5.2", reasoning: "medium", timeoutMs: 750,
|
||||
})).resolves.toBeUndefined();
|
||||
expect(spawns).toHaveLength(1);
|
||||
expect(spawns[0][0]).toBe("/usr/local/bin/pi");
|
||||
expect(spawns[0][1]).toEqual([
|
||||
"--mode", "rpc",
|
||||
"--no-session",
|
||||
"--no-tools",
|
||||
"--no-extensions",
|
||||
"--no-skills",
|
||||
"--no-prompt-templates",
|
||||
"--no-themes",
|
||||
"--no-context-files",
|
||||
"--no-approve",
|
||||
]);
|
||||
expect(spawns[0][2].env).not.toHaveProperty("THT_DATA_ROOT");
|
||||
expect(spawns[0][2].env).not.toHaveProperty("THT_SESSION");
|
||||
expect(spawns[0][2].env).not.toHaveProperty("THT_AUTHOR");
|
||||
expect(spawns[0][2].env).not.toHaveProperty("THT_CONFIG");
|
||||
expect(spawns[0][2].env).not.toHaveProperty("PI_CODING_AGENT_SESSION_DIR");
|
||||
expect(existsSync(dirname(spawns[0][2].cwd))).toBe(false);
|
||||
expect(commands.map(({ id: _id, ...command }) => command)).toEqual([
|
||||
{ type: "set_model", provider: "zai", modelId: "glm-5.2" },
|
||||
{ type: "set_thinking_level", level: "medium" },
|
||||
@@ -59,6 +103,55 @@ test("provider smoke selects the configured model and completes a fixed output-d
|
||||
expect(child.kill).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
const unexpectedToolEvents = [
|
||||
{
|
||||
name: "streamed tool call",
|
||||
event: {
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "toolcall_start", contentIndex: 0 },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "tool execution",
|
||||
event: { type: "tool_execution_start", toolCallId: "tool-1", toolName: "read" },
|
||||
},
|
||||
{
|
||||
name: "completed message tool call",
|
||||
event: {
|
||||
type: "message_end",
|
||||
message: { role: "assistant", stopReason: "toolUse", content: [{ type: "toolCall" }] },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "turn tool result",
|
||||
event: { type: "turn_end", toolResults: [{ role: "toolResult" }] },
|
||||
},
|
||||
];
|
||||
|
||||
// Catches Pi/provider regressions that surface a tool capability despite the fixed no-tools argv;
|
||||
// accepting agent_end after any such event could hide a mounted-state read or mutation.
|
||||
test.each(unexpectedToolEvents)("provider smoke fails closed on an unexpected $name event", async ({ event }) => {
|
||||
const child = rpcChild((command, emit) => {
|
||||
if (command.type === "set_model" || command.type === "set_thinking_level") {
|
||||
emit({ type: "response", id: command.id, success: true });
|
||||
}
|
||||
if (command.type === "prompt") {
|
||||
emit(event);
|
||||
emit({ type: "agent_end", messages: [] });
|
||||
}
|
||||
});
|
||||
const smoke = createPiProviderSmoke(loadConfig({}), {
|
||||
spawnFn: () => child,
|
||||
authProviders: () => new Set(["zai"]),
|
||||
readAuthStore: () => '{"zai":{"type":"api_key","key":"test-only"}}',
|
||||
});
|
||||
|
||||
await expect(smoke({
|
||||
provider: "zai", model: "glm-5.2", reasoning: "medium", timeoutMs: 750,
|
||||
})).rejects.toThrow("Pi provider smoke check failed");
|
||||
expect(child.kill).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
// Catches provider errors that are accepted as a successful health check or returned with raw
|
||||
// credential/output diagnostics.
|
||||
test("provider smoke rejects a failed model turn with a stable non-secret error", async () => {
|
||||
@@ -80,6 +173,7 @@ test("provider smoke rejects a failed model turn with a stable non-secret error"
|
||||
const smoke = createPiProviderSmoke(loadConfig({}), {
|
||||
spawnFn: () => child,
|
||||
authProviders: () => new Set(["zai"]),
|
||||
readAuthStore: () => '{"zai":{"type":"api_key","key":"test-only"}}',
|
||||
});
|
||||
|
||||
let caught: unknown;
|
||||
|
||||
Reference in New Issue
Block a user