fix: rename wrapper env vars and preserve result writing
This commit is contained in:
@@ -104,12 +104,12 @@ async function runWrapper(meta, startedAt) {
|
|||||||
const childEnv = { ...process.env };
|
const childEnv = { ...process.env };
|
||||||
// Ensure the copilot initiator flag is not accidentally inherited from the parent
|
// Ensure the copilot initiator flag is not accidentally inherited from the parent
|
||||||
// environment; set it only for github-copilot models.
|
// environment; set it only for github-copilot models.
|
||||||
delete childEnv.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR;
|
delete childEnv.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR;
|
||||||
// Mark every child run as a nested tmux subagent so it cannot spawn further subagents.
|
// Mark every child run as a subagent child so it cannot spawn further subagents.
|
||||||
childEnv.PI_TMUX_SUBAGENT_CHILD = "1";
|
childEnv.PI_SUBAGENTS_CHILD = "1";
|
||||||
|
|
||||||
if (typeof effectiveModel === "string" && effectiveModel.startsWith("github-copilot/")) {
|
if (typeof effectiveModel === "string" && effectiveModel.startsWith("github-copilot/")) {
|
||||||
childEnv.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR = "agent";
|
childEnv.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR = "agent";
|
||||||
}
|
}
|
||||||
|
|
||||||
const child = spawn("pi", args, {
|
const child = spawn("pi", args, {
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import test from "node:test";
|
import test from "node:test";
|
||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { chmod, mkdtemp, readFile, writeFile } from "node:fs/promises";
|
import { chmod, mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises";
|
||||||
import { spawn } from "node:child_process";
|
import { spawn } from "node:child_process";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { dirname, join } from "node:path";
|
import { dirname, join } from "node:path";
|
||||||
@@ -26,7 +26,7 @@ function waitForExit(child: ReturnType<typeof spawn>, timeoutMs = 1500): Promise
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function runWrapperWithFakePi(requestedModel: string, resolvedModel?: string) {
|
async function runWrapperWithFakePi(requestedModel: string, resolvedModel?: string) {
|
||||||
const dir = await mkdtemp(join(tmpdir(), "tmux-subagent-wrapper-"));
|
const dir = await mkdtemp(join(tmpdir(), "pi-subagents-wrapper-"));
|
||||||
const metaPath = join(dir, "meta.json");
|
const metaPath = join(dir, "meta.json");
|
||||||
const resultPath = join(dir, "result.json");
|
const resultPath = join(dir, "result.json");
|
||||||
const capturePath = join(dir, "capture.json");
|
const capturePath = join(dir, "capture.json");
|
||||||
@@ -42,8 +42,8 @@ async function runWrapperWithFakePi(requestedModel: string, resolvedModel?: stri
|
|||||||
"const fs = require('fs');",
|
"const fs = require('fs');",
|
||||||
`const capturePath = ${JSON.stringify(capturePath)};`,
|
`const capturePath = ${JSON.stringify(capturePath)};`,
|
||||||
"const obj = {",
|
"const obj = {",
|
||||||
" PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR: process.env.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR || '',",
|
" PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR: process.env.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR || '',",
|
||||||
" PI_TMUX_SUBAGENT_CHILD: process.env.PI_TMUX_SUBAGENT_CHILD || '',",
|
" PI_SUBAGENTS_CHILD: process.env.PI_SUBAGENTS_CHILD || '',",
|
||||||
" argv: process.argv.slice(2)",
|
" argv: process.argv.slice(2)",
|
||||||
"};",
|
"};",
|
||||||
"fs.writeFileSync(capturePath, JSON.stringify(obj), 'utf8');",
|
"fs.writeFileSync(capturePath, JSON.stringify(obj), 'utf8');",
|
||||||
@@ -96,28 +96,28 @@ async function runWrapperWithFakePi(requestedModel: string, resolvedModel?: stri
|
|||||||
return { flags: captureJson };
|
return { flags: captureJson };
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dedicated tests: every child run must have PI_TMUX_SUBAGENT_CHILD=1
|
// Dedicated tests: every child run must have PI_SUBAGENTS_CHILD=1
|
||||||
test("wrapper marks github-copilot child run as a tmux subagent child", async () => {
|
test("wrapper marks github-copilot child run as a subagent child", async () => {
|
||||||
const captured = await runWrapperWithFakePi("github-copilot/gpt-4o");
|
const captured = await runWrapperWithFakePi("github-copilot/gpt-4o");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_CHILD, "1");
|
assert.equal(captured.flags.PI_SUBAGENTS_CHILD, "1");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("wrapper marks anthropic child run as a tmux subagent child", async () => {
|
test("wrapper marks anthropic child run as a subagent child", async () => {
|
||||||
const captured = await runWrapperWithFakePi("anthropic/claude-sonnet-4-5");
|
const captured = await runWrapperWithFakePi("anthropic/claude-sonnet-4-5");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_CHILD, "1");
|
assert.equal(captured.flags.PI_SUBAGENTS_CHILD, "1");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("wrapper marks github-copilot child runs as agent-initiated", async () => {
|
test("wrapper marks github-copilot child runs as agent-initiated", async () => {
|
||||||
const captured = await runWrapperWithFakePi("github-copilot/gpt-4o");
|
const captured = await runWrapperWithFakePi("github-copilot/gpt-4o");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR, "agent");
|
assert.equal(captured.flags.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR, "agent");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_CHILD, "1");
|
assert.equal(captured.flags.PI_SUBAGENTS_CHILD, "1");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("wrapper leaves non-copilot child runs unchanged", async () => {
|
test("wrapper leaves non-copilot child runs unchanged", async () => {
|
||||||
const captured = await runWrapperWithFakePi("anthropic/claude-sonnet-4-5");
|
const captured = await runWrapperWithFakePi("anthropic/claude-sonnet-4-5");
|
||||||
// The wrapper should not inject the copilot initiator for non-copilot models.
|
// The wrapper should not inject the copilot initiator for non-copilot models.
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR, "");
|
assert.equal(captured.flags.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR, "");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_CHILD, "1");
|
assert.equal(captured.flags.PI_SUBAGENTS_CHILD, "1");
|
||||||
});
|
});
|
||||||
|
|
||||||
// Regression test: ensure when requestedModel and resolvedModel differ, the
|
// Regression test: ensure when requestedModel and resolvedModel differ, the
|
||||||
@@ -130,8 +130,8 @@ test("wrapper uses effective model for both argv and env when requested/resolved
|
|||||||
const captured = await runWrapperWithFakePi(requested, resolved);
|
const captured = await runWrapperWithFakePi(requested, resolved);
|
||||||
|
|
||||||
// The effective model should be the resolved model in this case.
|
// The effective model should be the resolved model in this case.
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_GITHUB_COPILOT_INITIATOR, "agent");
|
assert.equal(captured.flags.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR, "agent");
|
||||||
assert.equal(captured.flags.PI_TMUX_SUBAGENT_CHILD, "1");
|
assert.equal(captured.flags.PI_SUBAGENTS_CHILD, "1");
|
||||||
|
|
||||||
// Verify the child argv contains the effective model after a --model flag.
|
// Verify the child argv contains the effective model after a --model flag.
|
||||||
const argv = captured.flags.argv;
|
const argv = captured.flags.argv;
|
||||||
@@ -141,7 +141,7 @@ test("wrapper uses effective model for both argv and env when requested/resolved
|
|||||||
});
|
});
|
||||||
|
|
||||||
test("wrapper exits and writes result.json when the pi child cannot be spawned", async () => {
|
test("wrapper exits and writes result.json when the pi child cannot be spawned", async () => {
|
||||||
const dir = await mkdtemp(join(tmpdir(), "tmux-subagent-wrapper-"));
|
const dir = await mkdtemp(join(tmpdir(), "pi-subagents-wrapper-"));
|
||||||
const metaPath = join(dir, "meta.json");
|
const metaPath = join(dir, "meta.json");
|
||||||
const resultPath = join(dir, "result.json");
|
const resultPath = join(dir, "result.json");
|
||||||
|
|
||||||
@@ -190,3 +190,61 @@ test("wrapper exits and writes result.json when the pi child cannot be spawned",
|
|||||||
assert.equal(result.exitCode, 1);
|
assert.equal(result.exitCode, 1);
|
||||||
assert.match(result.errorMessage ?? "", /ENOENT|not found|spawn pi/i);
|
assert.match(result.errorMessage ?? "", /ENOENT|not found|spawn pi/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("wrapper still writes result.json when transcript/stdout artifact writes fail", async () => {
|
||||||
|
const dir = await mkdtemp(join(tmpdir(), "pi-subagents-wrapper-"));
|
||||||
|
const metaPath = join(dir, "meta.json");
|
||||||
|
const resultPath = join(dir, "result.json");
|
||||||
|
const piPath = join(dir, "pi");
|
||||||
|
const brokenArtifactPath = join(dir, "broken-artifact");
|
||||||
|
await mkdir(brokenArtifactPath);
|
||||||
|
|
||||||
|
await writeFile(
|
||||||
|
piPath,
|
||||||
|
[
|
||||||
|
`#!${process.execPath}`,
|
||||||
|
"console.log(JSON.stringify({type:'message_end',message:{role:'assistant',content:[{type:'text',text:'done'}],model:'openai/gpt-5',stopReason:'stop'}}));",
|
||||||
|
].join("\n"),
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
await chmod(piPath, 0o755);
|
||||||
|
|
||||||
|
await writeFile(
|
||||||
|
metaPath,
|
||||||
|
JSON.stringify(
|
||||||
|
{
|
||||||
|
runId: "run-1",
|
||||||
|
mode: "single",
|
||||||
|
agent: "scout",
|
||||||
|
agentSource: "builtin",
|
||||||
|
task: "inspect auth",
|
||||||
|
cwd: dir,
|
||||||
|
requestedModel: "openai/gpt-5",
|
||||||
|
resolvedModel: "openai/gpt-5",
|
||||||
|
sessionPath: join(dir, "child-session.jsonl"),
|
||||||
|
eventsPath: join(dir, "events.jsonl"),
|
||||||
|
resultPath,
|
||||||
|
stdoutPath: brokenArtifactPath,
|
||||||
|
stderrPath: join(dir, "stderr.log"),
|
||||||
|
transcriptPath: brokenArtifactPath,
|
||||||
|
systemPromptPath: join(dir, "system-prompt.md"),
|
||||||
|
},
|
||||||
|
null,
|
||||||
|
2,
|
||||||
|
),
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
|
||||||
|
const wrapperPath = join(dirname(fileURLToPath(import.meta.url)), "cli.mjs");
|
||||||
|
const child = spawn(process.execPath, [wrapperPath, metaPath], {
|
||||||
|
env: { ...process.env, PATH: dir },
|
||||||
|
stdio: ["ignore", "pipe", "pipe"],
|
||||||
|
});
|
||||||
|
|
||||||
|
const exitCode = await waitForExit(child);
|
||||||
|
assert.equal(exitCode, 0);
|
||||||
|
|
||||||
|
const result = JSON.parse(await readFile(resultPath, "utf8"));
|
||||||
|
assert.equal(result.exitCode, 0);
|
||||||
|
assert.equal(result.finalText, "done");
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user