feat: enhanced subagent config, reviewer output schema, and consolidated discovery helpers

- Added thinkingLevel field to agent frontmatter allowing subagents to override thinking level with clamping to model capabilities.
- Added structured output schema to reviewer agent ensuring consistent review output format with findings, correctness verdict, and confidence.
- Expanded system prompt with defensive reasoning guidance including assumption checks, edge case awareness, and completion reflex inhibition.
- Consolidated expandPath, parseFrontmatter, and parseAgentFields utilities into discovery/helpers module eliminating code duplication across loaders.
- Replaced silent error handling with logger.warn calls across config parsing, migrations, extension loading, and OAuth refresh failures.
This commit is contained in:
can1357
2026-01-11 13:33:11 +01:00
parent ff744750ac
commit 5e9ef021bb
41 changed files with 358 additions and 318 deletions
@@ -16,7 +16,7 @@ export async function getCurrentTime(timezone?: string): Promise<GetCurrentTimeR
content: [{ type: "text", text: timeStr }],
details: { utcTimestamp: date.getTime() },
};
} catch (_e) {
} catch {
throw new Error(`Invalid timezone: ${timezone}. Current UTC time: ${date.toISOString()}`);
}
}
+1 -1
View File
@@ -68,7 +68,7 @@ export async function pollCursorAuth(
}
throw new Error(`Poll failed: ${response.status}`);
} catch (_error) {
} catch {
consecutiveErrors++;
if (consecutiveErrors >= 3) {
throw new Error("Too many consecutive errors during Cursor auth polling");
+1 -1
View File
@@ -124,7 +124,7 @@ export async function getOAuthApiKey(
if (Date.now() >= creds.expires) {
try {
creds = await refreshOAuthToken(provider, creds);
} catch (_error) {
} catch {
throw new Error(`Failed to refresh OAuth token for ${provider}`);
}
}
+4 -2
View File
@@ -266,7 +266,7 @@ if (!isBrowserExtension) {
strict: false,
});
addFormats(ajv);
} catch (_e) {
} catch {
// AJV initialization failed (likely CSP restriction)
console.warn("AJV validation disabled due to CSP restrictions");
}
@@ -333,7 +333,9 @@ export function validateToolArguments(tool: Tool, toolCall: ToolCall): any {
}
: originalArgs;
const errorMessage = `Validation failed for tool "${toolCall.name}":\n${errors}\n\nReceived arguments:\n${JSON.stringify(receivedArgs, null, 2)}`;
const errorMessage = `Validation failed for tool "${
toolCall.name
}":\n${errors}\n\nReceived arguments:\n${JSON.stringify(receivedArgs, null, 2)}`;
throw new Error(errorMessage);
}
+1 -1
View File
@@ -468,7 +468,7 @@ describe("Context overflow error handling", () => {
console.log("Pulling gpt-oss:20b model for Ollama overflow tests...");
try {
execSync("ollama pull gpt-oss:20b", { stdio: "inherit" });
} catch (_e) {
} catch {
console.warn("Failed to pull gpt-oss:20b model, tests will be skipped");
return;
}
+4 -2
View File
@@ -185,7 +185,9 @@ async function handleThinking<TApi extends Api>(model: Model<TApi>, options?: Op
messages: [
{
role: "user",
content: `Think long and hard about ${(Math.random() * 255) | 0} + 27. Think step by step. Then output the result.`,
content: `Think long and hard about ${
(Math.random() * 255) | 0
} + 27. Think step by step. Then output the result.`,
timestamp: Date.now(),
},
],
@@ -930,7 +932,7 @@ describe("Generate E2E Tests", () => {
console.log("Pulling gpt-oss:20b model for Ollama tests...");
try {
execSync("ollama pull gpt-oss:20b", { stdio: "inherit" });
} catch (_e) {
} catch {
console.warn("Failed to pull gpt-oss:20b model, tests will be skipped");
return;
}
+9
View File
@@ -2,6 +2,15 @@
## [Unreleased]
### Changed
- Expanded system prompt with defensive reasoning guidance and assumption checks
- Allowed agent frontmatter to override subagent thinking level, clamped to model capabilities
### Fixed
- Ensured reviewer agents use structured output schemas and include reported findings in task outputs
## [4.3.0] - 2026-01-11
### Added
+2 -2
View File
@@ -144,8 +144,8 @@ async function updateViaBun(): Promise<void> {
try {
execSync(`bun update -g ${PACKAGE}`, { stdio: "inherit" });
console.log(chalk.green(`\n${theme.status.success} Update complete`));
} catch {
throw new Error("bun update failed");
} catch (error) {
throw new Error("bun update failed", { cause: error });
}
}
+5 -5
View File
@@ -1,9 +1,9 @@
import { existsSync, readFileSync, statSync } from "node:fs";
import { homedir } from "node:os";
import { dirname, join, resolve } from "node:path";
// Embed package.json at build time for config
import packageJson from "../package.json" with { type: "json" };
import { logger } from "./core/logger";
// =============================================================================
// App Config (from embedded package.json)
@@ -244,8 +244,8 @@ export function readConfigFile<T = unknown>(
content: JSON.parse(content) as T,
};
}
} catch {
// Continue to next file on parse error
} catch (error) {
logger.warn("Failed to parse config file", { path: filePath, error: String(error) });
}
}
@@ -275,8 +275,8 @@ export function readAllConfigFiles<T = unknown>(
content: JSON.parse(content) as T,
});
}
} catch {
// Skip files that fail to parse
} catch (error) {
logger.warn("Failed to parse config file", { path: filePath, error: String(error) });
}
}
@@ -939,7 +939,12 @@ export class AuthStorage {
this.recordSessionCredential(provider, sessionId, "oauth", selection.index);
return result.apiKey;
} catch {
} catch (error) {
logger.warn("OAuth token refresh failed, removing credential", {
provider,
index: selection.index,
error: String(error),
});
this.removeCredentialAt(provider, selection.index);
if (this.getCredentialsForProvider(provider).some((credential) => credential.type === "oauth")) {
return this.getApiKey(provider, sessionId, options);
@@ -11,6 +11,7 @@ import * as typebox from "@sinclair/typebox";
import { getAgentDir, getConfigDirs } from "../../config";
import * as piCodingAgent from "../../index";
import { execCommand } from "../exec";
import { logger } from "../logger";
import { createReviewCommand } from "./bundled/review";
import { createWorktreeCommand } from "./bundled/wt";
import type {
@@ -110,7 +111,8 @@ export function discoverCustomCommands(options: DiscoverCustomCommandsOptions =
let entries: Dirent[];
try {
entries = readdirSync(commandsDir, { withFileTypes: true });
} catch {
} catch (error) {
logger.warn("Failed to read custom commands directory", { path: commandsDir, error: String(error) });
continue;
}
for (const entry of entries) {
@@ -5,11 +5,11 @@
* to avoid import resolution issues with custom tools loaded from user directories.
*/
import * as os from "node:os";
import * as path from "node:path";
import * as typebox from "@sinclair/typebox";
import { toolCapability } from "../../capability/tool";
import { type CustomTool, loadCapability } from "../../discovery";
import { expandPath } from "../../discovery/helpers";
import * as piCodingAgent from "../../index";
import { theme } from "../../modes/interactive/theme/theme";
import type { ExecOptions } from "../exec";
@@ -19,23 +19,6 @@ import { logger } from "../logger";
import { getAllPluginToolPaths } from "../plugins/loader";
import type { CustomToolAPI, CustomToolFactory, CustomToolsLoadResult, LoadedCustomTool } from "./types";
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
function normalizeUnicodeSpaces(str: string): string {
return str.replace(UNICODE_SPACES, " ");
}
function expandPath(p: string): string {
const normalized = normalizeUnicodeSpaces(p);
if (normalized.startsWith("~/")) {
return path.join(os.homedir(), normalized.slice(2));
}
if (normalized.startsWith("~")) {
return path.join(os.homedir(), normalized.slice(1));
}
return normalized;
}
/**
* Resolve tool path.
* - Absolute paths used as-is
@@ -3,13 +3,12 @@
*/
import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
import { homedir } from "node:os";
import * as path from "node:path";
import type { KeyId } from "@oh-my-pi/pi-tui";
import * as TypeBox from "@sinclair/typebox";
import { type ExtensionModule, extensionModuleCapability } from "../../capability/extension-module";
import { loadCapability } from "../../discovery";
import { getExtensionNameFromPath } from "../../discovery/helpers";
import { expandPath, getExtensionNameFromPath } from "../../discovery/helpers";
import * as piCodingAgent from "../../index";
import { createEventBus, type EventBus } from "../event-bus";
import type { ExecOptions } from "../exec";
@@ -27,23 +26,6 @@ import type {
ToolDefinition,
} from "./types";
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
function normalizeUnicodeSpaces(str: string): string {
return str.replace(UNICODE_SPACES, " ");
}
function expandPath(p: string): string {
const normalized = normalizeUnicodeSpaces(p);
if (normalized.startsWith("~/")) {
return path.join(homedir(), normalized.slice(2));
}
if (normalized.startsWith("~")) {
return path.join(homedir(), normalized.slice(1));
}
return normalized;
}
function resolvePath(extPath: string, cwd: string): string {
const expanded = expandPath(extPath);
if (path.isAbsolute(expanded)) {
@@ -291,7 +273,8 @@ function readExtensionManifest(packageJsonPath: string): ExtensionManifest | nul
return manifest;
}
return null;
} catch {
} catch (error) {
logger.warn("Failed to read extension manifest", { path: packageJsonPath, error: String(error) });
return null;
}
}
@@ -370,7 +353,8 @@ function discoverExtensionsInDir(dir: string): string[] {
}
}
}
} catch {
} catch (error) {
logger.warn("Failed to discover extensions in directory", { path: dir, error: String(error) });
return [];
}
+1 -18
View File
@@ -2,12 +2,12 @@
* Hook loader - loads TypeScript hook modules using native Bun import.
*/
import * as os from "node:os";
import * as path from "node:path";
import * as typebox from "@sinclair/typebox";
import { hookCapability } from "../../capability/hook";
import type { Hook } from "../../discovery";
import { loadCapability } from "../../discovery";
import { expandPath } from "../../discovery/helpers";
import * as piCodingAgent from "../../index";
import { logger } from "../logger";
import type { HookMessage } from "../messages";
@@ -84,23 +84,6 @@ export interface LoadHooksResult {
errors: Array<{ path: string; error: string }>;
}
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
function normalizeUnicodeSpaces(str: string): string {
return str.replace(UNICODE_SPACES, " ");
}
function expandPath(p: string): string {
const normalized = normalizeUnicodeSpaces(p);
if (normalized.startsWith("~/")) {
return path.join(os.homedir(), normalized.slice(2));
}
if (normalized.startsWith("~")) {
return path.join(os.homedir(), normalized.slice(1));
}
return normalized;
}
/**
* Resolve hook path.
* - Absolute paths used as-is
@@ -10,6 +10,7 @@ import {
setEditorKeybindings,
} from "@oh-my-pi/pi-tui";
import { getAgentDir } from "../config";
import { logger } from "./logger";
/**
* Application-level actions (coding agent specific).
@@ -136,7 +137,8 @@ export class KeybindingsManager {
if (!existsSync(path)) return {};
try {
return JSON.parse(readFileSync(path, "utf-8"));
} catch {
} catch (error) {
logger.warn("Failed to parse keybindings config", { path, error: String(error) });
return {};
}
}
+1 -2
View File
@@ -10,11 +10,10 @@ import { homedir } from "node:os";
import { join } from "node:path";
import winston from "winston";
import DailyRotateFile from "winston-daily-rotate-file";
import { CONFIG_DIR_NAME } from "../config";
/** Get the logs directory (~/.omp/logs/) */
function getLogsDir(): string {
return join(homedir(), CONFIG_DIR_NAME, "logs");
return join(homedir(), ".omp", "logs");
}
/** Ensure logs directory exists */
@@ -1,6 +1,7 @@
import { join, resolve } from "node:path";
import Handlebars from "handlebars";
import { CONFIG_DIR_NAME, getPromptsDir } from "../config";
import { logger } from "./logger";
/**
* Represents a prompt template loaded from a markdown file
@@ -448,12 +449,12 @@ async function loadTemplatesFromDir(
source: sourceStr,
});
}
} catch (_error) {
// Silently skip files that can't be read
} catch (error) {
logger.warn("Failed to load prompt template", { path: fullPath, error: String(error) });
}
}
} catch (_error) {
// Silently skip directories that can't be read
} catch (error) {
logger.warn("Failed to scan prompt templates directory", { dir, error: String(error) });
}
return templates;
+5 -3
View File
@@ -28,7 +28,7 @@
import { join } from "node:path";
import { Agent, type AgentEvent, type AgentMessage, type AgentTool, type ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import type { Message, Model } from "@oh-my-pi/pi-ai";
import { type Message, type Model, supportsXhigh } from "@oh-my-pi/pi-ai";
import type { Component } from "@oh-my-pi/pi-tui";
import chalk from "chalk";
// Import discovery to register all providers on startup
@@ -631,6 +631,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
// Clamp to model capabilities
if (!model || !model.reasoning) {
thinkingLevel = "off";
} else if (thinkingLevel === "xhigh" && !supportsXhigh(model)) {
thinkingLevel = "high";
}
let skills: Skill[];
@@ -1021,8 +1023,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
});
lspServers = result.servers;
time("warmupLspServers");
} catch {
// Ignore warmup errors
} catch (error) {
logger.warn("LSP server warmup failed", { cwd, error: String(error) });
}
}
+5 -4
View File
@@ -7,6 +7,7 @@ import type { SourceMeta } from "../capability/types";
import type { Skill as CapabilitySkill, SkillFrontmatter as ImportedSkillFrontmatter } from "../discovery";
import { loadCapability } from "../discovery";
import { parseFrontmatter } from "../discovery/helpers";
import { logger } from "./logger";
import type { SkillsSettings } from "./settings-manager";
// Re-export SkillFrontmatter for backward compatibility
@@ -67,8 +68,8 @@ export function loadSkillsFromDir(options: LoadSkillsFromDirOptions): LoadSkills
source: options.source,
});
}
} catch {
// Skip invalid skills
} catch (error) {
logger.warn("Failed to load skill", { path: skillFile, error: String(error) });
}
}
@@ -131,8 +132,8 @@ function scanDirectoryForSkills(dir: string): LoadSkillsResult {
source: "custom",
});
}
} catch {
// Skip invalid skills
} catch (error) {
logger.warn("Failed to load skill", { path: skillFile, error: String(error) });
}
}
@@ -292,8 +292,8 @@ export async function fetchMCPToolSchema(
mcpSchemaCache.set(cacheKey, tool);
return tool;
}
} catch {
// Fall through to return null
} catch (error) {
logger.warn("Failed to fetch MCP tool schema", { mcpToolName, isWebsetsTool, error: String(error) });
}
return null;
}
@@ -4,6 +4,7 @@
* Agents are embedded at build time via Bun's import with { type: "text" }.
*/
import { parseAgentFields, parseFrontmatter } from "../../../discovery/helpers";
import exploreMd from "../../../prompts/agents/explore.md" with { type: "text" };
// Embed agent markdown files at build time
import agentFrontmatterTemplate from "../../../prompts/agents/frontmatter.md" with { type: "text" };
@@ -18,6 +19,7 @@ interface AgentFrontmatter {
description: string;
spawns?: string;
model?: string;
thinkingLevel?: string;
}
interface EmbeddedAgentDef {
@@ -71,80 +73,19 @@ const EMBEDDED_AGENTS: { name: string; content: string }[] = EMBEDDED_AGENT_DEFS
content: buildAgentContent(def),
}));
/**
* Parse YAML frontmatter from markdown content.
*/
function parseFrontmatter(content: string): { frontmatter: Record<string, string>; body: string } {
const frontmatter: Record<string, string> = {};
const normalized = content.replace(/\r\n/g, "\n");
if (!normalized.startsWith("---")) {
return { frontmatter, body: normalized };
}
const endIndex = normalized.indexOf("\n---", 3);
if (endIndex === -1) {
return { frontmatter, body: normalized };
}
const frontmatterBlock = normalized.slice(4, endIndex);
const body = normalized.slice(endIndex + 4).trim();
for (const line of frontmatterBlock.split("\n")) {
const match = line.match(/^([\w-]+):\s*(.*)$/);
if (match) {
let value = match[2].trim();
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
value = value.slice(1, -1);
}
frontmatter[match[1]] = value;
}
}
return { frontmatter, body };
}
/**
* Parse an agent from embedded content.
*/
function parseAgent(fileName: string, content: string, source: AgentSource): AgentDefinition | null {
const { frontmatter, body } = parseFrontmatter(content);
const fields = parseAgentFields(frontmatter);
if (!frontmatter.name || !frontmatter.description) {
if (!fields) {
return null;
}
const tools = frontmatter.tools
?.split(",")
.map((t) => t.trim())
.filter(Boolean);
// Parse spawns field
let spawns: string[] | "*" | undefined;
if (frontmatter.spawns !== undefined) {
const spawnsRaw = frontmatter.spawns.trim();
if (spawnsRaw === "*") {
spawns = "*";
} else if (spawnsRaw) {
spawns = spawnsRaw
.split(",")
.map((s) => s.trim())
.filter(Boolean);
if (spawns.length === 0) spawns = undefined;
}
}
// Backward compat: infer spawns: "*" when tools includes "task"
if (spawns === undefined && tools?.includes("task")) {
spawns = "*";
}
return {
name: frontmatter.name,
description: frontmatter.description,
tools: tools && tools.length > 0 ? tools : undefined,
spawns,
model: frontmatter.model,
...fields,
systemPrompt: body,
source,
filePath: `embedded:${fileName}`,
@@ -7,6 +7,7 @@
import * as path from "node:path";
import { type SlashCommand, slashCommandCapability } from "../../../capability/slash-command";
import { loadCapability } from "../../../discovery";
import { parseFrontmatter } from "../../../discovery/helpers";
// Embed command markdown files at build time
import initMd from "../../../prompts/agents/init.md" with { type: "text" };
@@ -27,37 +28,10 @@ export interface WorkflowCommand {
filePath: string;
}
/**
* Parse YAML frontmatter from markdown content.
*/
function parseFrontmatter(content: string): { frontmatter: Record<string, string>; body: string } {
const frontmatter: Record<string, string> = {};
const normalized = content.replace(/\r\n/g, "\n");
if (!normalized.startsWith("---")) {
return { frontmatter, body: normalized };
}
const endIndex = normalized.indexOf("\n---", 3);
if (endIndex === -1) {
return { frontmatter, body: normalized };
}
const frontmatterBlock = normalized.slice(4, endIndex);
const body = normalized.slice(endIndex + 4).trim();
for (const line of frontmatterBlock.split("\n")) {
const match = line.match(/^([\w-]+):\s*(.*)$/);
if (match) {
let value = match[2].trim();
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
value = value.slice(1, -1);
}
frontmatter[match[1]] = value;
}
}
return { frontmatter, body };
/** Extract string value from frontmatter field */
function getString(frontmatter: Record<string, unknown>, key: string): string {
const value = frontmatter[key];
return typeof value === "string" ? value : "";
}
/** Cache for bundled commands */
@@ -79,7 +53,7 @@ export function loadBundledCommands(): WorkflowCommand[] {
commands.push({
name: cmdName,
description: frontmatter.description || "",
description: getString(frontmatter, "description"),
instructions: body,
source: "bundled",
filePath: `embedded:${name}`,
@@ -115,7 +89,7 @@ export async function discoverCommands(cwd: string): Promise<WorkflowCommand[]>
commands.push({
name: cmd.name,
description: frontmatter.description || "",
description: getString(frontmatter, "description"),
instructions: body,
source,
filePath: cmd.path,
@@ -15,6 +15,7 @@
import * as fs from "node:fs";
import * as path from "node:path";
import { findAllNearestProjectConfigDirs, getConfigDirs } from "../../../config";
import { parseAgentFields, parseFrontmatter } from "../../../discovery/helpers";
import { loadBundledAgents } from "./agents";
import type { AgentDefinition, AgentSource } from "./types";
@@ -24,40 +25,6 @@ export interface DiscoveryResult {
projectAgentsDir: string | null;
}
/**
* Parse YAML frontmatter from markdown content.
*/
function parseFrontmatter(content: string): { frontmatter: Record<string, string>; body: string } {
const frontmatter: Record<string, string> = {};
const normalized = content.replace(/\r\n/g, "\n");
if (!normalized.startsWith("---")) {
return { frontmatter, body: normalized };
}
const endIndex = normalized.indexOf("\n---", 3);
if (endIndex === -1) {
return { frontmatter, body: normalized };
}
const frontmatterBlock = normalized.slice(4, endIndex);
const body = normalized.slice(endIndex + 4).trim();
for (const line of frontmatterBlock.split("\n")) {
const match = line.match(/^([\w-]+):\s*(.*)$/);
if (match) {
let value = match[2].trim();
// Strip quotes
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
value = value.slice(1, -1);
}
frontmatter[match[1]] = value;
}
}
return { frontmatter, body };
}
/**
* Load agents from a directory.
*/
@@ -95,43 +62,14 @@ function loadAgentsFromDir(dir: string, source: AgentSource): AgentDefinition[]
}
const { frontmatter, body } = parseFrontmatter(content);
const fields = parseAgentFields(frontmatter);
// Require name and description
if (!frontmatter.name || !frontmatter.description) {
if (!fields) {
continue;
}
const tools = frontmatter.tools
?.split(",")
.map((t) => t.trim())
.filter(Boolean);
// Parse spawns field
let spawns: string[] | "*" | undefined;
if (frontmatter.spawns !== undefined) {
const spawnsRaw = frontmatter.spawns.trim();
if (spawnsRaw === "*") {
spawns = "*";
} else if (spawnsRaw) {
spawns = spawnsRaw
.split(",")
.map((s) => s.trim())
.filter(Boolean);
if (spawns.length === 0) spawns = undefined;
}
}
// Backward compat: infer spawns: "*" when tools includes "task"
if (spawns === undefined && tools?.includes("task")) {
spawns = "*";
}
agents.push({
name: frontmatter.name,
description: frontmatter.description,
tools: tools && tools.length > 0 ? tools : undefined,
spawns,
model: frontmatter.model,
...fields,
systemPrompt: body,
source,
filePath,
@@ -4,7 +4,7 @@
* Runs each subagent in a Bun Worker and forwards AgentEvents for progress tracking.
*/
import type { AgentEvent } from "@oh-my-pi/pi-agent-core";
import type { AgentEvent, ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import type { AuthStorage } from "../../auth-storage";
import type { EventBus } from "../../event-bus";
import { callTool } from "../../mcp/client";
@@ -18,6 +18,7 @@ import {
type AgentProgress,
MAX_OUTPUT_BYTES,
MAX_OUTPUT_LINES,
type ReviewFinding,
type SingleResult,
TASK_SUBAGENT_EVENT_CHANNEL,
TASK_SUBAGENT_PROGRESS_CHANNEL,
@@ -39,6 +40,7 @@ export interface ExecutorOptions {
taskId: string;
context?: string;
modelOverride?: string;
thinkingLevel?: ThinkingLevel;
outputSchema?: unknown;
enableLsp?: boolean;
signal?: AbortSignal;
@@ -183,8 +185,20 @@ function extractMCPToolMetadata(mcpManager: MCPManager): MCPToolMetadata[] {
* Run a single agent in a worker.
*/
export async function runSubprocess(options: ExecutorOptions): Promise<SingleResult> {
const { cwd, agent, task, index, taskId, context, modelOverride, outputSchema, enableLsp, signal, onProgress } =
options;
const {
cwd,
agent,
task,
index,
taskId,
context,
modelOverride,
thinkingLevel,
outputSchema,
enableLsp,
signal,
onProgress,
} = options;
const startTime = Date.now();
// Initialize progress
@@ -578,6 +592,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise<SingleRes
task: fullTask,
systemPrompt: agent.systemPrompt,
model: resolvedModel,
thinkingLevel,
toolNames,
outputSchema,
sessionFile,
@@ -751,6 +766,20 @@ export async function runSubprocess(options: ExecutorOptions): Promise<SingleRes
// Not valid JSON, keep as string
}
}
// Special case: merge report_finding data into review output for parent visibility
const reportFindings = progress.extractedToolData?.report_finding as ReviewFinding[] | undefined;
if (
Array.isArray(reportFindings) &&
reportFindings.length > 0 &&
completeData &&
typeof completeData === "object" &&
!Array.isArray(completeData)
) {
const record = completeData as Record<string, unknown>;
if (!("findings" in record)) {
completeData = { ...record, findings: reportFindings };
}
}
try {
rawOutput = JSON.stringify(completeData, null, 2) ?? "null";
} catch (err) {
@@ -156,6 +156,11 @@ export async function createTaskTool(
const shouldInheritSessionModel = model === undefined && isDefaultModelAlias(agent.model);
const sessionModel = shouldInheritSessionModel ? session.getActiveModelString?.() : undefined;
const modelOverride = model ?? sessionModel ?? session.getModelString?.();
const thinkingLevelOverride = agent.thinkingLevel;
// Output schema priority: agent frontmatter > params > inherited from parent session
const schemaOverridden = outputSchema !== undefined && agent.output !== undefined;
const effectiveOutputSchema = agent.output ?? outputSchema ?? session.outputSchema;
// Handle empty or missing tasks
if (!params.tasks || params.tasks.length === 0) {
@@ -345,7 +350,8 @@ export async function createTaskTool(
taskId: task.taskId,
context: undefined, // Already prepended above
modelOverride,
outputSchema,
thinkingLevel: thinkingLevelOverride,
outputSchema: effectiveOutputSchema,
sessionFile,
persistArtifacts: !!artifactsDir,
artifactsDir: effectiveArtifactsDir,
@@ -399,9 +405,12 @@ export async function createTaskTool(
const outputIds = results.map((r) => r.taskId);
const outputHint =
outputIds.length > 0 ? `\n\nUse output tool for full logs: output ids ${outputIds.join(", ")}` : "";
const schemaNote = schemaOverridden
? `\n\nNote: Agent '${agentName}' has a fixed output schema; your 'output' parameter was ignored.\nRequired schema: ${JSON.stringify(agent.output)}`
: "";
const summary = `${successCount}/${results.length} succeeded [${formatDuration(
totalDuration,
)}]\n\n${summaries.join("\n\n---\n\n")}${outputHint}`;
)}]\n\n${summaries.join("\n\n---\n\n")}${outputHint}${schemaNote}`;
// Cleanup temp directory if used
if (tempArtifactsDir) {
@@ -369,18 +369,28 @@ function renderAgentProgress(
}
for (const [toolName, dataArray] of Object.entries(progress.extractedToolData)) {
// Handle report_finding with tree formatting
if (toolName === "report_finding" && (dataArray as ReportFindingDetails[]).length > 0) {
const findings = dataArray as ReportFindingDetails[];
lines.push(`${continuePrefix}${formatFindingSummary(findings, theme)}`);
lines.push(...renderFindings(findings, continuePrefix, expanded, theme));
continue;
}
const handler = subprocessToolRegistry.getHandler(toolName);
if (handler?.renderInline) {
// Show last few items inline
const recentData = (dataArray as unknown[]).slice(-3);
const displayCount = expanded ? (dataArray as unknown[]).length : 3;
const recentData = (dataArray as unknown[]).slice(-displayCount);
for (const data of recentData) {
const component = handler.renderInline(data, theme);
if (component instanceof Text) {
lines.push(`${continuePrefix}${component.getText()}`);
}
}
if (dataArray.length > 3) {
lines.push(`${continuePrefix}${theme.fg("dim", formatMoreItems(dataArray.length - 3, "item", theme))}`);
if ((dataArray as unknown[]).length > displayCount) {
lines.push(
`${continuePrefix}${theme.fg("dim", formatMoreItems((dataArray as unknown[]).length - displayCount, "item", theme))}`,
);
}
}
}
@@ -436,7 +446,6 @@ function renderReviewResult(
lines.push(`${continuePrefix}${formatFindingSummary(findings, theme)}`);
if (findings.length > 0) {
lines.push(`${continuePrefix}`); // Spacing
lines.push(...renderFindings(findings, continuePrefix, expanded, theme));
}
@@ -453,11 +462,14 @@ function renderFindings(
theme: Theme,
): string[] {
const lines: string[] = [];
const displayCount = expanded ? findings.length : Math.min(3, findings.length);
// Sort by priority (lower = more severe) when collapsed to show most important first
const sortedFindings = expanded ? findings : [...findings].sort((a, b) => a.priority - b.priority);
const displayCount = expanded ? sortedFindings.length : Math.min(3, sortedFindings.length);
for (let i = 0; i < displayCount; i++) {
const finding = findings[i];
const isLastFinding = i === displayCount - 1 && (expanded || findings.length <= 3);
const finding = sortedFindings[i];
const isLastFinding = i === displayCount - 1 && (expanded || sortedFindings.length <= 3);
const findingPrefix = isLastFinding ? theme.tree.last : theme.tree.branch;
const findingContinue = isLastFinding ? " " : `${theme.tree.vertical} `;
@@ -538,14 +550,12 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool
return lines;
}
if (reportFindingData && reportFindingData.length > 0) {
lines.push(
`${continuePrefix}${theme.fg("warning", theme.status.warning)} ${theme.fg(
"dim",
"Review summary missing (complete not called)",
)}`,
);
const hasCompleteData = completeData && completeData.length > 0;
const message = hasCompleteData
? "Review verdict missing expected fields"
: "Review incomplete (complete not called)";
lines.push(`${continuePrefix}${theme.fg("warning", theme.status.warning)} ${theme.fg("dim", message)}`);
lines.push(`${continuePrefix}${formatFindingSummary(reportFindingData, theme)}`);
lines.push(`${continuePrefix}`); // Spacing
lines.push(...renderFindings(reportFindingData, continuePrefix, expanded, theme));
return lines;
}
@@ -1,3 +1,4 @@
import type { ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import type { Usage } from "@oh-my-pi/pi-ai";
import { type Static, Type } from "@sinclair/typebox";
@@ -106,6 +107,8 @@ export interface AgentDefinition {
tools?: string[];
spawns?: string[] | "*";
model?: string;
thinkingLevel?: ThinkingLevel;
output?: unknown;
source: AgentSource;
filePath?: string;
}
@@ -1,4 +1,4 @@
import type { AgentEvent } from "@oh-my-pi/pi-agent-core";
import type { AgentEvent, ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import type { SerializedAuthStorage } from "../../auth-storage";
import type { SerializedModelRegistry } from "../../model-registry";
@@ -43,6 +43,7 @@ export interface SubagentWorkerStartPayload {
task: string;
systemPrompt: string;
model?: string;
thinkingLevel?: ThinkingLevel;
toolNames?: string[];
outputSchema?: unknown;
enableLsp?: boolean;
@@ -287,7 +287,8 @@ async function runTask(runState: RunState, payload: SubagentWorkerStartPayload):
const mcpProxyTools = payload.mcpTools?.map(createMCPProxyTool) ?? [];
// Resolve model override (equivalent to CLI's parseModelPattern with --model)
const { model, thinkingLevel } = resolveModelOverride(payload.model, modelRegistry);
const { model, thinkingLevel: modelThinkingLevel } = resolveModelOverride(payload.model, modelRegistry);
const thinkingLevel = modelThinkingLevel ?? payload.thinkingLevel;
// Create session manager (equivalent to CLI's --session or --no-session)
const sessionManager = payload.sessionFile
@@ -379,7 +379,7 @@ export const handleHuggingFace: SpecialHandler = async (url: string, timeout: nu
default:
return null;
}
} catch (_err) {
} catch {
return null;
}
};
@@ -95,7 +95,7 @@ export const handleReadTheDocs: SpecialHandler = async (
content = sourceResult.content;
notes.push(`Fetched raw source from ${sourceUrl}`);
}
} catch (_err) {
} catch {
// Ignore errors, fall back to HTML
}
}
@@ -170,7 +170,7 @@ export async function loadPage(url: string, options: LoadPageOptions = {}): Prom
}
return { content, contentType, finalUrl, ok: true, status: response.status };
} catch (_err) {
} catch {
if (signal?.aborted) {
return { content: "", contentType: "", finalUrl: url, ok: false };
}
@@ -14,6 +14,7 @@ import { buildBetaHeader, claudeCodeHeaders, claudeCodeVersion } from "@oh-my-pi
import { getAgentDbPath, getConfigDirPaths } from "../../../config";
import { AgentStorage } from "../../agent-storage";
import type { AuthCredential, AuthCredentialEntry, AuthStorageData } from "../../auth-storage";
import { logger } from "../../logger";
import { migrateJsonStorage } from "../../storage-migration";
import type { AnthropicAuthConfig, AnthropicOAuthCredential, ModelsJson } from "./types";
@@ -48,8 +49,8 @@ async function parseEnvFile(filePath: string): Promise<Record<string, string>> {
result[key] = value;
}
} catch {
// Ignore read errors
} catch (error) {
logger.warn("Failed to read .env file", { path: filePath, error: String(error) });
}
return result;
}
@@ -82,7 +83,8 @@ async function readJson<T>(filePath: string): Promise<T | null> {
if (!(await file.exists())) return null;
const content = await file.text();
return JSON.parse(content) as T;
} catch {
} catch (error) {
logger.warn("Failed to parse JSON file", { path: filePath, error: String(error) });
return null;
}
}
+3 -1
View File
@@ -29,6 +29,7 @@ import { slashCommandCapability } from "../capability/slash-command";
import type { CustomTool } from "../capability/tool";
import { toolCapability } from "../capability/tool";
import type { LoadContext, LoadResult } from "../capability/types";
import { logger } from "../core/logger";
import {
createSourceMeta,
discoverExtensionModulePaths,
@@ -117,7 +118,8 @@ async function loadTomlConfig(_ctx: LoadContext, path: string): Promise<Record<s
try {
return parseToml(content) as Record<string, unknown>;
} catch (_err) {
} catch (error) {
logger.warn("Failed to parse TOML config", { path, error: String(error) });
return null;
}
}
+124 -3
View File
@@ -2,11 +2,38 @@
* Shared helpers for discovery providers.
*/
import { homedir } from "node:os";
import { join, resolve } from "node:path";
import type { ThinkingLevel } from "@oh-my-pi/pi-agent-core";
import { parse as parseYAML } from "yaml";
import { readDirEntries, readFile } from "../capability/fs";
import type { Skill, SkillFrontmatter } from "../capability/skill";
import type { LoadContext, LoadResult, SourceMeta } from "../capability/types";
import { logger } from "../core/logger";
const VALID_THINKING_LEVELS: readonly string[] = ["off", "minimal", "low", "medium", "high", "xhigh"];
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
/**
* Normalize unicode spaces to regular spaces.
*/
export function normalizeUnicodeSpaces(str: string): string {
return str.replace(UNICODE_SPACES, " ");
}
/**
* Expand ~ to home directory and normalize unicode spaces.
*/
export function expandPath(p: string): string {
const normalized = normalizeUnicodeSpaces(p);
if (normalized.startsWith("~/")) {
return join(homedir(), normalized.slice(2));
}
if (normalized.startsWith("~")) {
return join(homedir(), normalized.slice(1));
}
return normalized;
}
/**
* Standard paths for each config source.
@@ -117,14 +144,108 @@ export function parseFrontmatter(content: string): {
const body = normalized.slice(endIndex + 4).trim();
try {
const frontmatter = parseYAML(raw) as Record<string, unknown> | null;
// Replace tabs with spaces for YAML compatibility, use failsafe mode for robustness
const frontmatter = parseYAML(raw.replaceAll("\t", " "), { compat: "failsafe" }) as Record<
string,
unknown
> | null;
return { frontmatter: frontmatter ?? {}, body, raw };
} catch {
// Fallback to empty frontmatter on parse error
} catch (error) {
logger.warn("Failed to parse YAML frontmatter", { error: String(error) });
return { frontmatter: {}, body, raw };
}
}
/**
* Parse thinking level from frontmatter.
* Supports keys: thinkingLevel, thinking-level, thinking
*/
export function parseThinkingLevel(frontmatter: Record<string, unknown>): ThinkingLevel | undefined {
const raw = frontmatter.thinkingLevel ?? frontmatter["thinking-level"] ?? frontmatter.thinking;
if (typeof raw === "string" && VALID_THINKING_LEVELS.includes(raw)) {
return raw as ThinkingLevel;
}
return undefined;
}
/**
* Parse a comma-separated string into an array of trimmed, non-empty strings.
*/
export function parseCSV(value: string): string[] {
return value
.split(",")
.map((s) => s.trim())
.filter(Boolean);
}
/**
* Parse a value that may be an array of strings or a comma-separated string.
* Returns undefined if the result would be empty.
*/
export function parseArrayOrCSV(value: unknown): string[] | undefined {
if (Array.isArray(value)) {
const filtered = value.filter((item): item is string => typeof item === "string");
return filtered.length > 0 ? filtered : undefined;
}
if (typeof value === "string") {
const parsed = parseCSV(value);
return parsed.length > 0 ? parsed : undefined;
}
return undefined;
}
/** Parsed agent fields from frontmatter (excludes source/filePath/systemPrompt) */
export interface ParsedAgentFields {
name: string;
description: string;
tools?: string[];
spawns?: string[] | "*";
model?: string;
output?: unknown;
thinkingLevel?: ThinkingLevel;
}
/**
* Parse agent fields from frontmatter.
* Returns null if required fields (name, description) are missing.
*/
export function parseAgentFields(frontmatter: Record<string, unknown>): ParsedAgentFields | null {
const name = typeof frontmatter.name === "string" ? frontmatter.name : undefined;
const description = typeof frontmatter.description === "string" ? frontmatter.description : undefined;
if (!name || !description) {
return null;
}
const tools = parseArrayOrCSV(frontmatter.tools);
// Parse spawns field (array, "*", or CSV)
let spawns: string[] | "*" | undefined;
if (frontmatter.spawns === "*") {
spawns = "*";
} else if (typeof frontmatter.spawns === "string") {
const trimmed = frontmatter.spawns.trim();
if (trimmed === "*") {
spawns = "*";
} else {
spawns = parseArrayOrCSV(trimmed);
}
} else {
spawns = parseArrayOrCSV(frontmatter.spawns);
}
// Backward compat: infer spawns: "*" when tools includes "task"
if (spawns === undefined && tools?.includes("task")) {
spawns = "*";
}
const output = frontmatter.output !== undefined ? frontmatter.output : undefined;
const model = typeof frontmatter.model === "string" ? frontmatter.model : undefined;
const thinkingLevel = parseThinkingLevel(frontmatter);
return { name, description, tools, spawns, model, output, thinkingLevel };
}
export async function loadSkillsFromDir(
_ctx: LoadContext,
options: {
+11 -9
View File
@@ -8,6 +8,7 @@ import chalk from "chalk";
import { getAgentDbPath, getAgentDir, getBinDir } from "./config";
import { AgentStorage } from "./core/agent-storage";
import type { AuthCredential } from "./core/auth-storage";
import { logger } from "./core/logger";
/**
* Migrate PI_* environment variables to OMP_* equivalents.
@@ -55,8 +56,8 @@ export function migrateAuthToAgentDb(): string[] {
providers.push(provider);
}
renameSync(oauthPath, `${oauthPath}.migrated`);
} catch {
// Skip on error
} catch (error) {
logger.warn("Failed to migrate oauth.json", { path: oauthPath, error: String(error) });
}
}
@@ -75,8 +76,8 @@ export function migrateAuthToAgentDb(): string[] {
delete settings.apiKeys;
writeFileSync(settingsPath, JSON.stringify(settings, null, 2));
}
} catch {
// Skip on error
} catch (error) {
logger.warn("Failed to migrate settings.json apiKeys", { path: settingsPath, error: String(error) });
}
}
@@ -105,7 +106,8 @@ export function migrateSessionsFromAgentRoot(): void {
files = readdirSync(agentDir)
.filter((f) => f.endsWith(".jsonl"))
.map((f) => join(agentDir, f));
} catch {
} catch (error) {
logger.warn("Failed to read agent directory for session migration", { path: agentDir, error: String(error) });
return;
}
@@ -137,8 +139,8 @@ export function migrateSessionsFromAgentRoot(): void {
if (existsSync(newPath)) continue; // Skip if target exists
renameSync(file, newPath);
} catch {
// Skip files that can't be migrated
} catch (error) {
logger.warn("Failed to migrate session file", { path: file, error: String(error) });
}
}
}
@@ -168,8 +170,8 @@ function migrateToolsToBin(): void {
try {
renameSync(oldPath, newPath);
movedAny = true;
} catch {
// Ignore errors
} catch (error) {
logger.warn("Failed to migrate binary", { from: oldPath, to: newPath, error: String(error) });
}
} else {
// Target exists, just delete the old one
@@ -13,6 +13,7 @@ import type { Skill } from "../../../../capability/skill";
import type { SlashCommand } from "../../../../capability/slash-command";
import type { CustomTool } from "../../../../capability/tool";
import type { SourceMeta } from "../../../../capability/types";
import { logger } from "../../../../core/logger";
import {
disableProvider,
enableProvider,
@@ -105,8 +106,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
getDescription: (s) => s.frontmatter?.description,
getTrigger: (s) => s.frontmatter?.globs?.join(", "),
});
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load skills capability", { error: String(error) });
}
// Load rules
@@ -116,8 +117,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
getDescription: (r) => r.description,
getTrigger: (r) => r.globs?.join(", ") || (r.alwaysApply ? "always" : undefined),
});
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load rules capability", { error: String(error) });
}
// Load custom tools
@@ -126,8 +127,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
addItems(tools.all, "tool", {
getDescription: (t) => t.description,
});
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load tools capability", { error: String(error) });
}
// Load extension modules
@@ -135,8 +136,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
const modules = await loadCapability<ExtensionModule>("extension-modules", loadOpts);
const nativeModules = modules.all.filter((module) => module._source.provider === "native");
addItems(nativeModules, "extension-module");
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load extension-modules capability", { error: String(error) });
}
// Load MCP servers
@@ -178,8 +179,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
raw: server,
});
}
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load mcps capability", { error: String(error) });
}
// Load prompts
@@ -189,8 +190,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
getDescription: () => undefined,
getTrigger: (p) => `/prompts:${p.name}`,
});
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load prompts capability", { error: String(error) });
}
// Load slash commands
@@ -200,8 +201,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
getDescription: () => undefined,
getTrigger: (c) => `/${c.name}`,
});
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load slash-commands capability", { error: String(error) });
}
// Load hooks
@@ -243,8 +244,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
raw: hook,
});
}
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load hooks capability", { error: String(error) });
}
// Load context files
@@ -288,8 +289,8 @@ export async function loadAllExtensions(cwd?: string, disabledIds?: string[]): P
raw: file,
});
}
} catch {
// Capability may not be registered
} catch (error) {
logger.warn("Failed to load context-files capability", { error: String(error) });
}
return extensions;
@@ -3,5 +3,6 @@ name: {{name}}
description: {{description}}
{{#if spawns}}spawns: {{spawns}}
{{/if}}{{#if model}}model: {{model}}
{{/if}}{{#if thinkingLevel}}thinkingLevel: {{thinkingLevel}}
{{/if}}---
{{body}}
@@ -4,6 +4,33 @@ description: Code review specialist for quality and security analysis
tools: read, grep, find, ls, bash, report_finding
spawns: explore, task
model: pi/slow, gpt-5.2-codex, gpt-5.2, codex, gpt
output:
properties:
overall_correctness:
enum: [correct, incorrect]
explanation:
type: string
confidence:
type: number
optionalProperties:
findings:
elements:
properties:
title:
type: string
body:
type: string
priority:
type: number
confidence:
type: number
file_path:
type: string
line_start:
type: number
line_end:
type: number
required: [overall_correctness, explanation, confidence]
---
You are a senior engineer reviewing a proposed code change. Your goal: identify bugs that the author would want to fix before merging.
@@ -64,11 +91,12 @@ Each `report_finding` requires:
- `file_path`: Absolute path
- `line_start`, `line_end`: Range ≤10 lines, must overlap the diff
Final `complete` call:
Final `complete` call (payload goes under `data`):
- `overall_correctness`: "correct" (no bugs/blockers) or "incorrect"
- `explanation`: 1-3 sentences
- `confidence`: 0.0-1.0
- `data.overall_correctness`: "correct" (no bugs/blockers) or "incorrect"
- `data.explanation`: Plain text, 1-3 sentences summarizing your verdict. Do NOT include JSON, do NOT repeat findings here (they're already captured via `report_finding`).
- `data.confidence`: 0.0-1.0
- `data.findings`: Optional; MUST omit (it is populated from `report_finding` calls)
Correctness judgment ignores non-blocking issues (style, docs, nits).
@@ -12,12 +12,14 @@ If you discussed requirements, plans, schemas, or decisions with the user, you M
## Available Agents
{{#list agents prefix="- " join="\n"}}
{{name}}: {{description}} (Tools: {{default (join tools ", ") "All tools"}})
{{name}}: {{description}} (Tools: {{default (join tools ", ") "All tools"}}{{#if output}}, Output: structured{{/if}})
{{/list}}
{{#if moreAgents}}
...and {{moreAgents}} more agents
{{/if}}
Agents with "Output: structured" have a fixed schema enforced via frontmatter; your `output` parameter will be ignored for these agents.
## When NOT to Use
- Reading a specific file path → Use Read tool instead
+1 -1
View File
@@ -14,7 +14,7 @@ let imageBuffer: Uint8Array;
try {
const file = Bun.file(testImagePath);
imageBuffer = await file.bytes();
} catch (_e) {
} catch {
console.error(`Failed to load image: ${testImagePath}`);
console.error("Usage: bun test/image-test.ts [path-to-image.png]");
process.exit(1);