fix(tool): flatten resolveChildModel adapter, remove unsafe cast, add compatibility shim and comments

This commit is contained in:
pi
2026-04-12 11:32:28 +01:00
parent 7691915e38
commit f0cd7ef27a
4 changed files with 179 additions and 24 deletions

102
debug-cli-test45.js Normal file
View File

@@ -0,0 +1,102 @@
import { chmod, mkdtemp, readFile, writeFile } from 'node:fs/promises';
import { spawn } from 'node:child_process';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
function waitForExit(child, timeoutMs = 1500) {
return new Promise((resolve, reject) => {
const timeout = setTimeout(() => {
child.kill('SIGKILL');
reject(new Error(`wrapper did not exit within ${timeoutMs}ms`));
}, timeoutMs);
child.on('error', (error) => {
clearTimeout(timeout);
reject(error);
});
child.on('close', (code) => {
clearTimeout(timeout);
resolve(code ?? 0);
});
});
}
async function runWrapperWithFakePi(requestedModel) {
const dir = await mkdtemp(join(tmpdir(), 'pi-subagents-wrapper-'));
const metaPath = join(dir, 'meta.json');
const resultPath = join(dir, 'result.json');
const capturePath = join(dir, 'capture.json');
const piPath = join(dir, 'pi');
const resolved = requestedModel;
await writeFile(
piPath,
[
`#!${process.execPath}`,
"const fs = require('fs');",
`const capturePath = ${JSON.stringify(capturePath)};`,
"const obj = {",
" PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR: process.env.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR || '',",
" PI_SUBAGENTS_CHILD: process.env.PI_SUBAGENTS_CHILD || '',",
" argv: process.argv.slice(2)",
"};",
"fs.writeFileSync(capturePath, JSON.stringify(obj), 'utf8');",
"console.log(JSON.stringify({type:'message_end',message:{role:'assistant',content:[{type:'text',text:'done'}],model:'github-copilot/gpt-4o',stopReason:'stop'}}));",
].join('\n'),
'utf8',
);
await chmod(piPath, 0o755);
await writeFile(
metaPath,
JSON.stringify(
{
runId: 'run-1',
mode: 'single',
task: 'inspect auth',
cwd: dir,
requestedModel,
resolvedModel: resolved,
startedAt: '2026-04-09T00:00:00.000Z',
sessionPath: join(dir, 'child-session.jsonl'),
eventsPath: join(dir, 'events.jsonl'),
resultPath,
stdoutPath: join(dir, 'stdout.log'),
stderrPath: join(dir, 'stderr.log'),
transcriptPath: join(dir, 'transcript.log'),
tools: ['read', 'grep'],
systemPromptPath: join(dir, 'system-prompt.md'),
},
null,
2,
),
'utf8',
);
const wrapperPath = join(dirname(fileURLToPath(import.meta.url)), 'src/wrapper/cli.mjs');
const child = spawn(process.execPath, [wrapperPath, metaPath], {
env: {
...process.env,
PATH: dir,
},
stdio: ['ignore', 'pipe', 'pipe'],
});
const exitCode = await waitForExit(child);
console.log('exitCode', exitCode);
const captureJson = JSON.parse(await readFile(capturePath, 'utf8'));
return { flags: captureJson };
}
(async () => {
try {
const captured = await runWrapperWithFakePi('github-copilot/gpt-4o');
console.log('captured', captured);
} catch (err) {
console.error('error', err);
process.exit(1);
}
})();

48
debug-run-wrapper.js Normal file
View File

@@ -0,0 +1,48 @@
import { mkdtemp, writeFile, chmod } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { spawn } from 'node:child_process';
import { dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
async function run() {
const dir = await mkdtemp(join(tmpdir(), 'pi-subagents-wrapper-'));
const metaPath = join(dir, 'meta.json');
const resultPath = join(dir, 'result.json');
const capturePath = join(dir, 'capture.json');
const piPath = join(dir, 'pi');
await writeFile(piPath, `#!${process.execPath}\nconst fs = require('fs');\nconst capturePath = ${JSON.stringify(capturePath)};\nconst obj = { PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR: process.env.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR || '', PI_SUBAGENTS_CHILD: process.env.PI_SUBAGENTS_CHILD || '', argv: process.argv.slice(2) }; fs.writeFileSync(capturePath, JSON.stringify(obj), 'utf8'); console.log(JSON.stringify({type:'message_end',message:{role:'assistant',content:[{type:'text',text:'done'}],model:'github-copilot/gpt-4o',stopReason:'stop'}}));`, 'utf8');
await chmod(piPath, 0o755);
const meta = {
runId: 'run-1',
mode: 'single',
task: 'inspect auth',
cwd: dir,
requestedModel: 'github-copilot/gpt-4o',
resolvedModel: 'github-copilot/gpt-4o',
startedAt: '2026-04-09T00:00:00.000Z',
sessionPath: join(dir, 'child-session.jsonl'),
eventsPath: join(dir, 'events.jsonl'),
resultPath,
stdoutPath: join(dir, 'stdout.log'),
stderrPath: join(dir, 'stderr.log'),
transcriptPath: join(dir, 'transcript.log'),
tools: ['read', 'grep'],
systemPromptPath: join(dir, 'system-prompt.md'),
};
await writeFile(metaPath, JSON.stringify(meta, null, 2), 'utf8');
const wrapperPath = join(dirname(fileURLToPath(import.meta.url)), 'src/wrapper/cli.mjs');
console.log('wrapperPath', wrapperPath, 'metaPath', metaPath);
const child = spawn(process.execPath, [wrapperPath, metaPath], { env: { ...process.env, PATH: dir }, stdio: ['ignore','pipe','pipe'] });
child.stdout.on('data', (c) => console.log('wrapper stdout:', c.toString()));
child.stderr.on('data', (c) => console.error('wrapper stderr:', c.toString()));
child.on('close', (code) => { console.log('wrapper exited', code); process.exit(code ?? 0); });
}
run().catch(err => { console.error(err); process.exit(1); });

View File

@@ -43,6 +43,9 @@ function loadPresetDir(dir: string, source: "global" | "project"): SubagentPrese
const filePath = join(dir, entry.name); const filePath = join(dir, entry.name);
let content: string; let content: string;
try { try {
// Fail-open: if a preset file is unreadable for any reason, ignore it
// rather than failing the entire discovery process. This keeps preset
// discovery robust in the presence of partially-broken user files.
content = readFileSync(filePath, "utf8"); content = readFileSync(filePath, "utf8");
} catch (_err) { } catch (_err) {
continue; continue;
@@ -85,8 +88,11 @@ export function discoverSubagentPresets(cwd: string, options: { homeDir?: string
} }
if (projectPresetsDir) { if (projectPresetsDir) {
// Project presets override global presets by name. This is intentional
// to let local projects stage or refine presets without modifying the
// user's global agent presets. There is no confirmation gate for a
// project override; the nearest project takes precedence.
for (const preset of loadPresetDir(projectPresetsDir, "project")) { for (const preset of loadPresetDir(projectPresetsDir, "project")) {
// project overrides global by name
map.set(preset.name, preset); map.set(preset.name, preset);
} }
} }

View File

@@ -60,12 +60,15 @@ export function createSubagentTool(deps: {
listAvailableModelReferences?: typeof listAvailableModelReferences; listAvailableModelReferences?: typeof listAvailableModelReferences;
normalizeAvailableModelReference?: typeof normalizeAvailableModelReference; normalizeAvailableModelReference?: typeof normalizeAvailableModelReference;
parameters?: typeof SubagentParamsSchema; parameters?: typeof SubagentParamsSchema;
// Compatibility: accept injected resolveChildModel functions with either the // Compatibility: injected resolveChildModel may be the new API
// new API ({ callModel?, presetModel? }) or the older test/hooks API // ({ callModel?, presetModel? }) or the older test/hooks API
// ({ taskModel?, topLevelModel? }). We adapt at callsite below. // ({ taskModel?, topLevelModel? }). The local adapter below exposes a
// flattened, simple boundary that takes only { callModel?, presetModel? }
// and adapts to either injected shape internally by providing both key
// names when calling the injected resolver.
resolveChildModel?: resolveChildModel?:
| typeof resolveChildModel | ((input: { callModel?: string; presetModel?: string; taskModel?: string; topLevelModel?: string }) => ModelSelection)
| ((input: { taskModel?: string; topLevelModel?: string }) => ModelSelection); | typeof resolveChildModel;
runSingleTask?: (input: { runSingleTask?: (input: {
cwd: string; cwd: string;
meta: Record<string, unknown>; meta: Record<string, unknown>;
@@ -136,21 +139,22 @@ export function createSubagentTool(deps: {
step.model = normalizedStepModel; step.model = normalizedStepModel;
} }
const callResolveChildModel = (input: { // Adapter: accept only the flattened shape { callModel?, presetModel? }
callModel?: string; // to keep the tool logic simple. If a resolver was injected with the
presetModel?: string; // older test/hooks shape, we call it with both key names so it can read
taskModel?: string; // the old keys. This is a minimal, internal compatibility shim.
topLevelModel?: string; const callResolveChildModel = (input: { callModel?: string; presetModel?: string }) => {
}) => {
// If an injected resolveChildModel exists, call it with the older-shape
// keys (taskModel/topLevelModel) for compatibility. Otherwise, use the
// internal resolveChildModel which expects { callModel, presetModel }.
if (deps.resolveChildModel) { if (deps.resolveChildModel) {
const injected = deps.resolveChildModel as unknown as (arg: { taskModel?: string; topLevelModel?: string }) => unknown; // Provide both naming variants so old and new resolvers both work.
return injected({ taskModel: input.callModel ?? input.taskModel, topLevelModel: input.presetModel ?? input.topLevelModel }) as ModelSelection; return deps.resolveChildModel({
callModel: input.callModel,
presetModel: input.presetModel,
taskModel: input.callModel,
topLevelModel: input.presetModel,
});
} }
return resolveChildModel({ callModel: input.callModel ?? input.taskModel, presetModel: input.presetModel ?? input.topLevelModel }); return resolveChildModel({ callModel: input.callModel, presetModel: input.presetModel });
}; };
const runTask = async (input: { const runTask = async (input: {
@@ -161,12 +165,7 @@ export function createSubagentTool(deps: {
step?: number; step?: number;
mode: "single" | "parallel" | "chain"; mode: "single" | "parallel" | "chain";
}) => { }) => {
const model = callResolveChildModel({ const model = callResolveChildModel({ callModel: input.taskModel, presetModel: params.model });
callModel: input.taskModel,
presetModel: params.model,
taskModel: input.taskModel,
topLevelModel: params.model,
});
const progressFormatter = createProgressFormatter(); const progressFormatter = createProgressFormatter();