From f0cd7ef27a262f34035692ad1fe6bc8b3d4868d6 Mon Sep 17 00:00:00 2001 From: pi Date: Sun, 12 Apr 2026 11:32:28 +0100 Subject: [PATCH] fix(tool): flatten resolveChildModel adapter, remove unsafe cast, add compatibility shim and comments --- debug-cli-test45.js | 102 +++++++++++++++++++++++++++++++++++++++++++ debug-run-wrapper.js | 48 ++++++++++++++++++++ src/presets.ts | 8 +++- src/tool.ts | 45 ++++++++++--------- 4 files changed, 179 insertions(+), 24 deletions(-) create mode 100644 debug-cli-test45.js create mode 100644 debug-run-wrapper.js diff --git a/debug-cli-test45.js b/debug-cli-test45.js new file mode 100644 index 0000000..e8c640a --- /dev/null +++ b/debug-cli-test45.js @@ -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); + } +})(); diff --git a/debug-run-wrapper.js b/debug-run-wrapper.js new file mode 100644 index 0000000..ab9a608 --- /dev/null +++ b/debug-run-wrapper.js @@ -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); }); diff --git a/src/presets.ts b/src/presets.ts index fc0c955..34816e1 100644 --- a/src/presets.ts +++ b/src/presets.ts @@ -43,6 +43,9 @@ function loadPresetDir(dir: string, source: "global" | "project"): SubagentPrese const filePath = join(dir, entry.name); let content: string; 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"); } catch (_err) { continue; @@ -85,8 +88,11 @@ export function discoverSubagentPresets(cwd: string, options: { homeDir?: string } 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")) { - // project overrides global by name map.set(preset.name, preset); } } diff --git a/src/tool.ts b/src/tool.ts index cf7dbce..eb185c7 100644 --- a/src/tool.ts +++ b/src/tool.ts @@ -60,12 +60,15 @@ export function createSubagentTool(deps: { listAvailableModelReferences?: typeof listAvailableModelReferences; normalizeAvailableModelReference?: typeof normalizeAvailableModelReference; parameters?: typeof SubagentParamsSchema; - // Compatibility: accept injected resolveChildModel functions with either the - // new API ({ callModel?, presetModel? }) or the older test/hooks API - // ({ taskModel?, topLevelModel? }). We adapt at callsite below. + // Compatibility: injected resolveChildModel may be the new API + // ({ callModel?, presetModel? }) or the older test/hooks API + // ({ 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?: - | typeof resolveChildModel - | ((input: { taskModel?: string; topLevelModel?: string }) => ModelSelection); + | ((input: { callModel?: string; presetModel?: string; taskModel?: string; topLevelModel?: string }) => ModelSelection) + | typeof resolveChildModel; runSingleTask?: (input: { cwd: string; meta: Record; @@ -136,21 +139,22 @@ export function createSubagentTool(deps: { step.model = normalizedStepModel; } - const callResolveChildModel = (input: { - callModel?: string; - presetModel?: string; - taskModel?: string; - topLevelModel?: 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 }. + // Adapter: accept only the flattened shape { callModel?, presetModel? } + // to keep the tool logic simple. If a resolver was injected with the + // older test/hooks shape, we call it with both key names so it can read + // the old keys. This is a minimal, internal compatibility shim. + const callResolveChildModel = (input: { callModel?: string; presetModel?: string }) => { if (deps.resolveChildModel) { - const injected = deps.resolveChildModel as unknown as (arg: { taskModel?: string; topLevelModel?: string }) => unknown; - return injected({ taskModel: input.callModel ?? input.taskModel, topLevelModel: input.presetModel ?? input.topLevelModel }) as ModelSelection; + // Provide both naming variants so old and new resolvers both work. + 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: { @@ -161,12 +165,7 @@ export function createSubagentTool(deps: { step?: number; mode: "single" | "parallel" | "chain"; }) => { - const model = callResolveChildModel({ - callModel: input.taskModel, - presetModel: params.model, - taskModel: input.taskModel, - topLevelModel: params.model, - }); + const model = callResolveChildModel({ callModel: input.taskModel, presetModel: params.model }); const progressFormatter = createProgressFormatter();