diff --git a/AGENTS.md b/AGENTS.md index b3a884c92..721ea4cbd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -272,10 +272,12 @@ See `docs/bun-migration-guide.md` for full migration reference. ## Commands -- After code changes: `npm run check` (get full output, no tail) -- NEVER run: `npm run dev`, `npm run build`, `npm test` -- Only run specific tests if user instructs: `npm test -- test/specific.test.ts` +- After code changes: `bun run check` (get full output, no tail) +- For auto-fixable lint issues: `bun run fix` (includes unsafe fixes) +- NEVER run: `bun run dev`, `bun run build`, `bun test` +- Only run specific tests if user instructs: `bun test test/specific.test.ts` - NEVER commit unless user asks +- Do NOT use `tsc` or `npx tsc` - always use `bun run check` which runs the correct type checker ## GitHub Issues diff --git a/package.json b/package.json index ddab99740..df60a46b3 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "build": "bun --cwd=packages/web-ui run build:css", "dev": "bun --cwd=packages/web-ui run dev", "check": "biome check --write . && bun --cwd=packages/coding-agent run check && bun --cwd=packages/web-ui run check", + "fix": "biome check --write --unsafe . && bun --cwd=packages/web-ui run fix", "test": "bun run --filter '*' test", "version:patch": "npm version patch -ws --no-git-tag-version && bun scripts/sync-versions.js && rm -rf node_modules packages/*/node_modules bun.lockb && bun install", "version:minor": "npm version minor -ws --no-git-tag-version && bun scripts/sync-versions.js && rm -rf node_modules packages/*/node_modules bun.lockb && bun install", diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 65f7a2eb6..f939a9c4d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,9 +1,10 @@ # Changelog ## [Unreleased] - ### Added +- Added subprocess tool registry for extracting and rendering tool data from subprocess agents in real-time +- Added combined review result rendering showing verdict and findings in a tree structure - Auto-read file mentions: Reference files with `@path/to/file.ext` syntax in prompts to automatically inject their contents, eliminating manual Read tool calls - Added `hidden` property for custom tools to exclude them from default tool list unless explicitly requested - Added `explicitTools` option to `createAgentSession` for enabling hidden tools by name @@ -16,9 +17,18 @@ ### Changed +- Changed `/review` command from markdown to interactive TypeScript with mode selection menu (branch comparison, uncommitted changes, commit review, custom) +- Changed bundled commands to be overridable by user/project commands with same name +- Changed subprocess termination to wait for message_end event to capture accurate token counts +- Changed token counting in subprocess to accumulate across messages instead of overwriting - Updated bundled `reviewer` agent to use structured review tools with priority-based findings (P0-P3) and formal verdict submission - Task tool now streams artifacts in real-time: input written before spawn, session jsonl written by subprocess, output written at completion +### Removed + +- Removed `findings_count` parameter from `submit_review` tool - findings are now counted automatically +- Removed artifacts location display from task tool output + ### Fixed - Fixed Task tool output artifacts (`out.md`) containing duplicated text from streaming updates diff --git a/packages/coding-agent/package.json b/packages/coding-agent/package.json index 967b53a72..7eee84744 100644 --- a/packages/coding-agent/package.json +++ b/packages/coding-agent/package.json @@ -33,7 +33,7 @@ "clean": "rm -rf dist", "build": "tsgo -p tsconfig.build.json && chmod +x dist/cli.js && npm run copy-assets", "build:binary": "npm run build && bun build --compile ./dist/cli.js --outfile dist/pi && npm run copy-binary-assets", - "copy-assets": "mkdir -p dist/modes/interactive/theme && cp src/modes/interactive/theme/*.json dist/modes/interactive/theme/ && mkdir -p dist/core/export-html && cp src/core/export-html/template.html src/core/export-html/template.css src/core/export-html/template.js dist/core/export-html/", + "copy-assets": "mkdir -p dist/modes/interactive/theme && cp src/modes/interactive/theme/*.json dist/modes/interactive/theme/ && mkdir -p dist/core/export-html && cp src/core/export-html/template.html src/core/export-html/template.css src/core/export-html/template.js dist/core/export-html/ && mkdir -p dist/core/tools/task/bundled-agents && cp src/core/tools/task/bundled-agents/*.md dist/core/tools/task/bundled-agents/", "copy-binary-assets": "cp package.json dist/ && cp README.md dist/ && cp CHANGELOG.md dist/ && mkdir -p dist/theme && cp src/modes/interactive/theme/*.json dist/theme/ && mkdir -p dist/export-html && cp src/core/export-html/template.html src/core/export-html/template.css src/core/export-html/template.js dist/export-html/ && cp -r docs dist/ && cp -r examples dist/", "test": "vitest --run", "prepublishOnly": "npm run clean && npm run build" diff --git a/packages/coding-agent/src/commands/review.md b/packages/coding-agent/src/commands/review.md deleted file mode 100644 index 5e30e9cbe..000000000 --- a/packages/coding-agent/src/commands/review.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -name: review -description: Launch code review with the reviewer agent ---- - -Use the Task tool to run the "reviewer" agent with this task: - -$@ - -The reviewer agent will analyze the code changes and report findings using the report_finding and submit_review tools. - -If no specific instructions are provided, review the most recent changes (uncommitted or last commit). diff --git a/packages/coding-agent/src/core/custom-commands/bundled/review/index.ts b/packages/coding-agent/src/core/custom-commands/bundled/review/index.ts new file mode 100644 index 000000000..1ba1ea7ec --- /dev/null +++ b/packages/coding-agent/src/core/custom-commands/bundled/review/index.ts @@ -0,0 +1,156 @@ +/** + * /review command - Interactive code review launcher + * + * Provides a menu to select review mode: + * 1. Review against a base branch (PR style) + * 2. Review uncommitted changes + * 3. Review a specific commit + * 4. Custom review instructions + */ + +import type { HookCommandContext } from "../../../hooks/types"; +import type { CustomCommand, CustomCommandAPI } from "../../types"; + +export function createReviewCommand(api: CustomCommandAPI): CustomCommand { + return { + name: "review", + description: "Launch interactive code review", + + async execute(_args: string[], ctx: HookCommandContext): Promise { + if (!ctx.hasUI) { + return "Use the Task tool to run the 'reviewer' agent to review recent code changes."; + } + + // Main menu + const mode = await ctx.ui.select("Review Mode", [ + "1. Review against a base branch (PR Style)", + "2. Review uncommitted changes", + "3. Review a specific commit", + "4. Custom review instructions", + ]); + + if (!mode) return undefined; + + const modeNum = parseInt(mode[0], 10); + + switch (modeNum) { + case 1: { + // PR-style review against base branch + const branches = await getGitBranches(api); + if (branches.length === 0) { + ctx.ui.notify("No git branches found", "error"); + return undefined; + } + + const baseBranch = await ctx.ui.select("Select base branch to compare against", branches); + if (!baseBranch) return undefined; + + const currentBranch = await getCurrentBranch(api); + return `Use the Task tool to run the "reviewer" agent with this task: + +Review the changes between "${baseBranch}" and "${currentBranch}". + +Run \`git diff ${baseBranch}...${currentBranch}\` to see the changes, then analyze the modified files.`; + } + + case 2: { + // Uncommitted changes + const status = await getGitStatus(api); + if (!status.trim()) { + ctx.ui.notify("No uncommitted changes found", "warning"); + return undefined; + } + + return `Use the Task tool to run the "reviewer" agent with this task: + +Review all uncommitted changes in the working directory. + +Run \`git diff\` for unstaged changes and \`git diff --cached\` for staged changes.`; + } + + case 3: { + // Specific commit + const commits = await getRecentCommits(api, 20); + if (commits.length === 0) { + ctx.ui.notify("No commits found", "error"); + return undefined; + } + + const selected = await ctx.ui.select("Select commit to review", commits); + if (!selected) return undefined; + + // Extract commit hash from selection (format: "abc1234 message") + const hash = selected.split(" ")[0]; + + return `Use the Task tool to run the "reviewer" agent with this task: + +Review commit ${hash}. + +Run \`git show ${hash}\` to see the changes introduced by this commit.`; + } + + case 4: { + // Custom instructions + const instructions = await ctx.ui.editor( + "Enter custom review instructions", + "Review the following:\n\n", + ); + if (!instructions?.trim()) return undefined; + + return `Use the Task tool to run the "reviewer" agent with this task: + +${instructions}`; + } + + default: + return undefined; + } + }, + }; +} + +async function getGitBranches(api: CustomCommandAPI): Promise { + try { + const result = await api.exec("git", ["branch", "-a", "--format=%(refname:short)"]); + if (result.code !== 0) return []; + return result.stdout + .split("\n") + .map((b) => b.trim()) + .filter(Boolean); + } catch { + return []; + } +} + +async function getCurrentBranch(api: CustomCommandAPI): Promise { + try { + const result = await api.exec("git", ["branch", "--show-current"]); + return result.stdout.trim() || "HEAD"; + } catch { + return "HEAD"; + } +} + +async function getGitStatus(api: CustomCommandAPI): Promise { + try { + const result = await api.exec("git", ["status", "--porcelain"]); + return result.stdout; + } catch { + return ""; + } +} + +async function getRecentCommits(api: CustomCommandAPI, count: number): Promise { + try { + const result = await api.exec("git", ["log", `-${count}`, "--oneline", "--no-decorate"]); + if (result.code !== 0) return []; + return result.stdout + .split("\n") + .map((c) => c.trim()) + .filter(Boolean); + } catch { + return []; + } +} + +export default createReviewCommand; diff --git a/packages/coding-agent/src/core/custom-commands/loader.ts b/packages/coding-agent/src/core/custom-commands/loader.ts index 34be9f8c3..f3e126d67 100644 --- a/packages/coding-agent/src/core/custom-commands/loader.ts +++ b/packages/coding-agent/src/core/custom-commands/loader.ts @@ -11,6 +11,7 @@ import * as typebox from "@sinclair/typebox"; import { CONFIG_DIR_NAME, getAgentDir } from "../../config"; import * as piCodingAgent from "../../index"; import { execCommand } from "../exec"; +import { createReviewCommand } from "./bundled/review"; import type { CustomCommand, CustomCommandAPI, @@ -141,6 +142,24 @@ export interface LoadCustomCommandsOptions { agentDir?: string; } +/** + * Load bundled commands (shipped with pi-coding-agent). + */ +function loadBundledCommands(sharedApi: CustomCommandAPI): LoadedCustomCommand[] { + const bundled: LoadedCustomCommand[] = []; + + // Add bundled commands here + const reviewCommand = createReviewCommand(sharedApi); + bundled.push({ + path: "bundled:review", + resolvedPath: "bundled:review", + command: reviewCommand, + source: "bundled", + }); + + return bundled; +} + /** * Discover and load custom commands from standard locations. */ @@ -163,6 +182,13 @@ export async function loadCustomCommands(options: LoadCustomCommandsOptions = {} pi: piCodingAgent, }; + // 1. Load bundled commands first (lowest priority - can be overridden) + for (const loaded of loadBundledCommands(sharedApi)) { + seenNames.add(loaded.command.name); + commands.push(loaded); + } + + // 2. Load user/project commands (can override bundled) for (const { path: commandPath, source } of paths) { const { commands: loadedCommands, error } = await loadCommandModule(commandPath, cwd, sharedApi); @@ -173,13 +199,22 @@ export async function loadCustomCommands(options: LoadCustomCommandsOptions = {} if (loadedCommands) { for (const command of loadedCommands) { - // Check for name conflicts - if (seenNames.has(command.name)) { - errors.push({ - path: commandPath, - error: `Command name "${command.name}" conflicts with existing command`, - }); - continue; + // Allow overriding bundled commands, but not user/project conflicts + const existingIdx = commands.findIndex((c) => c.command.name === command.name); + if (existingIdx !== -1) { + const existing = commands[existingIdx]; + if (existing.source === "bundled") { + // Override bundled command + commands.splice(existingIdx, 1); + seenNames.delete(command.name); + } else { + // Conflict between user/project commands + errors.push({ + path: commandPath, + error: `Command name "${command.name}" conflicts with existing command`, + }); + continue; + } } seenNames.add(command.name); diff --git a/packages/coding-agent/src/core/custom-commands/types.ts b/packages/coding-agent/src/core/custom-commands/types.ts index 244ebd62a..d046e7ae8 100644 --- a/packages/coding-agent/src/core/custom-commands/types.ts +++ b/packages/coding-agent/src/core/custom-commands/types.ts @@ -91,7 +91,7 @@ export type CustomCommandFactory = ( ) => CustomCommand | CustomCommand[] | Promise; /** Source of a loaded custom command */ -export type CustomCommandSource = "user" | "project"; +export type CustomCommandSource = "bundled" | "user" | "project"; /** Loaded custom command with metadata */ export interface LoadedCustomCommand { diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 49205bf9a..b1c5aa544 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -582,12 +582,11 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const contextFiles = options.contextFiles ?? discoverContextFiles(cwd, agentDir); time("discoverContextFiles"); - // Hook runner - created early for hooks - let hookRunner: HookRunner | undefined; + // Hook runner - always created (needed for custom command context even without hooks) + let loadedHooks: LoadedHook[] = []; if (options.hooks !== undefined) { if (options.hooks.length > 0) { - const loadedHooks = createLoadedHooksFromDefinitions(options.hooks); - hookRunner = new HookRunner(loadedHooks, cwd, sessionManager, modelRegistry); + loadedHooks = createLoadedHooksFromDefinitions(options.hooks); } } else { // Discover hooks, merging with additional paths @@ -597,10 +596,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} for (const { path, error } of errors) { console.error(`Failed to load hook "${path}": ${error}`); } - if (hooks.length > 0) { - hookRunner = new HookRunner(hooks, cwd, sessionManager, modelRegistry); - } + loadedHooks = hooks; } + const hookRunner = new HookRunner(loadedHooks, cwd, sessionManager, modelRegistry); const sessionContext = { getSessionFile: () => sessionManager.getSessionFile() ?? null, diff --git a/packages/coding-agent/src/core/tools/review.ts b/packages/coding-agent/src/core/tools/review.ts index 45cee43ac..da2679fb4 100644 --- a/packages/coding-agent/src/core/tools/review.ts +++ b/packages/coding-agent/src/core/tools/review.ts @@ -18,7 +18,7 @@ const PRIORITY_LABELS: Record = { 3: "P3", }; -const PRIORITY_DESCRIPTIONS: Record = { +const _PRIORITY_DESCRIPTIONS: Record = { 0: "Drop everything to fix. Blocking release, operations, or major usage.", 1: "Urgent. Should be addressed in the next cycle.", 2: "Normal. To be fixed eventually.", @@ -121,16 +121,12 @@ const SubmitReviewParams = Type.Object({ maximum: 1, description: "Overall confidence score 0.0-1.0", }), - findings_count: Type.Number({ - description: "Total number of findings reported", - }), }); interface SubmitReviewDetails { overall_correctness: "correct" | "incorrect"; explanation: string; confidence: number; - findings_count: number; } export const submitReviewTool: AgentTool = { @@ -141,17 +137,16 @@ export const submitReviewTool: AgentTool 0) { - container.addChild(new Spacer(1)); - container.addChild(new Text(theme.fg("muted", `${details.findings_count} finding(s) reported`), 0, 0)); - } - if (expanded) { container.addChild(new Spacer(1)); container.addChild(new Text(theme.fg("dim", details.explanation), 0, 0)); @@ -205,3 +195,74 @@ export function createReportFindingTool(): AgentTool { return submitReviewTool; } + +// Re-export types for external use +export type { ReportFindingDetails, SubmitReviewDetails }; + +// ───────────────────────────────────────────────────────────────────────────── +// Subprocess tool handlers - registered for extraction/rendering in task tool +// ───────────────────────────────────────────────────────────────────────────── + +import path from "node:path"; +import { subprocessToolRegistry } from "./task/subprocess-tool-registry"; + +// Register report_finding handler +subprocessToolRegistry.register("report_finding", { + extractData: (event) => event.result?.details as ReportFindingDetails | undefined, + + renderInline: (data, theme) => { + const priority = PRIORITY_LABELS[data.priority] ?? "P?"; + const color = data.priority === 0 ? "error" : data.priority === 1 ? "warning" : "muted"; + const titleText = data.title.replace(/^\[P\d\]\s*/, ""); + const loc = `${path.basename(data.file_path)}:${data.line_start}`; + return new Text(`${theme.fg(color, `[${priority}]`)} ${titleText} ${theme.fg("dim", loc)}`, 0, 0); + }, + + renderFinal: (allData, theme, expanded) => { + const container = new Container(); + const displayCount = expanded ? allData.length : Math.min(3, allData.length); + + for (let i = 0; i < displayCount; i++) { + const data = allData[i]; + const priority = PRIORITY_LABELS[data.priority] ?? "P?"; + const color = data.priority === 0 ? "error" : data.priority === 1 ? "warning" : "muted"; + const titleText = data.title.replace(/^\[P\d\]\s*/, ""); + const loc = `${path.basename(data.file_path)}:${data.line_start}`; + + container.addChild( + new Text(` ${theme.fg(color, `[${priority}]`)} ${titleText} ${theme.fg("dim", loc)}`, 0, 0), + ); + + if (expanded && data.body) { + container.addChild(new Text(` ${theme.fg("dim", data.body)}`, 0, 0)); + } + } + + if (allData.length > displayCount) { + container.addChild(new Text(theme.fg("dim", ` ... ${allData.length - displayCount} more findings`), 0, 0)); + } + + return container; + }, +}); + +// Register submit_review handler +subprocessToolRegistry.register("submit_review", { + extractData: (event) => event.result?.details as SubmitReviewDetails | undefined, + + // Terminate subprocess after review is submitted + shouldTerminate: () => true, + + renderInline: (data, theme) => { + const verdictColor = data.overall_correctness === "correct" ? "success" : "error"; + const verdictIcon = data.overall_correctness === "correct" ? "✓" : "✗"; + return new Text( + `${theme.fg(verdictColor, verdictIcon)} Review: ${theme.fg(verdictColor, data.overall_correctness)} (${(data.confidence * 100).toFixed(0)}%)`, + 0, + 0, + ); + }, + + // Note: renderFinal is NOT used for submit_review - we use the combined + // renderReviewResult in render.ts to show verdict + findings together +}); diff --git a/packages/coding-agent/src/core/tools/task/bundled-agents/reviewer.md b/packages/coding-agent/src/core/tools/task/bundled-agents/reviewer.md index 2f00dc5e2..cba131026 100644 --- a/packages/coding-agent/src/core/tools/task/bundled-agents/reviewer.md +++ b/packages/coding-agent/src/core/tools/task/bundled-agents/reviewer.md @@ -1,7 +1,7 @@ --- name: reviewer description: Code review specialist for quality and security analysis -tools: read, grep, glob, ls, bash, report_finding, submit_review +tools: read, grep, find, ls, bash, report_finding, submit_review model: pi/slow, gpt-5.2-codex, gpt-5.2, codex, gpt --- @@ -48,6 +48,12 @@ Only flag issues where ALL of these apply: 7. Write so the author can immediately grasp the idea without close reading 8. Avoid flattery and phrases like "Great job...", "Thanks for..." +# CRITICAL + +You MUST call `submit_review` before ending your response, even if you found no issues. +The review is only considered complete when `submit_review` is called. +Failure to call `submit_review` means the review was not submitted. + # Output - Use `report_finding` for each issue. Continue until you've listed every qualifying finding. diff --git a/packages/coding-agent/src/core/tools/task/executor.ts b/packages/coding-agent/src/core/tools/task/executor.ts index 748835d74..ff5447ff9 100644 --- a/packages/coding-agent/src/core/tools/task/executor.ts +++ b/packages/coding-agent/src/core/tools/task/executor.ts @@ -232,6 +232,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise { + if (!resolved) { + proc.kill("SIGTERM"); + resolved = true; + } + }, 2000); } } break; @@ -345,7 +353,13 @@ export async function runSubprocess(options: ExecutorOptions): Promise string | null; diff --git a/packages/coding-agent/src/core/tools/task/render.ts b/packages/coding-agent/src/core/tools/task/render.ts index 83b184469..05b1bd2bc 100644 --- a/packages/coding-agent/src/core/tools/task/render.ts +++ b/packages/coding-agent/src/core/tools/task/render.ts @@ -10,9 +10,18 @@ import type { Component } from "@oh-my-pi/pi-tui"; import { Container, Text } from "@oh-my-pi/pi-tui"; import type { Theme } from "../../../modes/interactive/theme/theme"; import type { RenderResultOptions } from "../../custom-tools/types"; +import type { ReportFindingDetails, SubmitReviewDetails } from "../review"; import { subprocessToolRegistry } from "./subprocess-tool-registry"; import type { AgentProgress, SingleResult, TaskParams, TaskToolDetails } from "./types"; +/** Priority labels for review findings */ +const PRIORITY_LABELS: Record = { + 0: "P0", + 1: "P1", + 2: "P2", + 3: "P3", +}; + /** * Format token count for display (e.g., 1.5k, 25k). */ @@ -160,6 +169,78 @@ function renderAgentProgress(progress: AgentProgress, isLast: boolean, expanded: return lines; } +/** + * Render review result with combined verdict + findings in tree structure. + */ +function renderReviewResult( + summary: SubmitReviewDetails, + findings: ReportFindingDetails[], + continuePrefix: string, + expanded: boolean, + theme: Theme, +): string[] { + const lines: string[] = []; + + // Verdict line + const verdictColor = summary.overall_correctness === "correct" ? "success" : "error"; + const verdictIcon = summary.overall_correctness === "correct" ? "✓" : "✗"; + lines.push( + `${continuePrefix}${theme.fg(verdictColor, verdictIcon)} Patch is ${theme.fg(verdictColor, summary.overall_correctness)} ${theme.fg("dim", `(${(summary.confidence * 100).toFixed(0)}% confidence)`)}`, + ); + + // Explanation preview (first ~80 chars when collapsed, full when expanded) + if (summary.explanation) { + if (expanded) { + // Full explanation, wrapped + const explanationLines = summary.explanation.split("\n"); + for (const line of explanationLines) { + lines.push(`${continuePrefix}${theme.fg("dim", line)}`); + } + } else { + // Preview: first sentence or ~100 chars + const preview = truncate(`${summary.explanation.split(/[.!?]/)[0]}.`, 100); + lines.push(`${continuePrefix}${theme.fg("dim", preview)}`); + } + } + + // Findings in tree structure + if (findings.length > 0) { + lines.push(`${continuePrefix}`); // Spacing + const displayCount = expanded ? findings.length : Math.min(3, findings.length); + + for (let i = 0; i < displayCount; i++) { + const finding = findings[i]; + const isLastFinding = i === displayCount - 1 && (expanded || findings.length <= 3); + const findingPrefix = isLastFinding ? "└─" : "├─"; + const findingContinue = isLastFinding ? " " : "│ "; + + const priority = PRIORITY_LABELS[finding.priority] ?? "P?"; + const color = finding.priority === 0 ? "error" : finding.priority === 1 ? "warning" : "muted"; + const titleText = finding.title.replace(/^\[P\d\]\s*/, ""); + const loc = `${path.basename(finding.file_path)}:${finding.line_start}`; + + lines.push( + `${continuePrefix}${findingPrefix} ${theme.fg(color, `[${priority}]`)} ${titleText} ${theme.fg("dim", loc)}`, + ); + + // Show body when expanded + if (expanded && finding.body) { + // Wrap body text + const bodyLines = finding.body.split("\n"); + for (const bodyLine of bodyLines) { + lines.push(`${continuePrefix}${findingContinue}${theme.fg("dim", bodyLine)}`); + } + } + } + + if (!expanded && findings.length > 3) { + lines.push(`${continuePrefix}${theme.fg("dim", `... ${findings.length - 3} more findings`)}`); + } + } + + return lines; +} + /** * Render final result for a single agent. */ @@ -177,7 +258,9 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool // Main status line let statusLine = `${prefix} ${theme.fg(iconColor, icon)} ${theme.fg("accent", result.agent)}`; statusLine += `: ${theme.fg(iconColor, statusText)}`; - statusLine += ` · ${theme.fg("dim", `${formatTokens(result.tokens)} tokens`)}`; + if (result.tokens > 0) { + statusLine += ` · ${theme.fg("dim", `${formatTokens(result.tokens)} tokens`)}`; + } statusLine += ` · ${theme.fg("dim", formatDuration(result.durationMs))}`; if (result.truncated) { @@ -186,10 +269,25 @@ function renderAgentResult(result: SingleResult, isLast: boolean, expanded: bool lines.push(statusLine); - // Check for extracted tool data with custom renderers + // Check for review result (submit_review + report_finding) + const submitReviewData = result.extractedToolData?.submit_review as SubmitReviewDetails[] | undefined; + const reportFindingData = result.extractedToolData?.report_finding as ReportFindingDetails[] | undefined; + + if (submitReviewData && submitReviewData.length > 0) { + // Use combined review renderer + const summary = submitReviewData[submitReviewData.length - 1]; + const findings = reportFindingData ?? []; + lines.push(...renderReviewResult(summary, findings, continuePrefix, expanded, theme)); + return lines; + } + + // Check for extracted tool data with custom renderers (skip review tools) let hasCustomRendering = false; if (result.extractedToolData) { for (const [toolName, dataArray] of Object.entries(result.extractedToolData)) { + // Skip review tools - handled above + if (toolName === "submit_review" || toolName === "report_finding") continue; + const handler = subprocessToolRegistry.getHandler(toolName); if (handler?.renderFinal && (dataArray as unknown[]).length > 0) { hasCustomRendering = true; @@ -287,11 +385,7 @@ export function renderResult( summary += ` · ${theme.fg("dim", formatDuration(details.totalDurationMs))}`; lines.push(summary); - // Artifacts location - if (details.outputPaths && details.outputPaths.length > 0) { - const artifactsDir = path.dirname(details.outputPaths[0]); - lines.push(`${theme.fg("dim", "Artifacts:")} ${theme.fg("muted", artifactsDir)}`); - } + // Artifacts suppressed from user view - available via session file } if (lines.length === 0) { diff --git a/packages/coding-agent/src/core/tools/task/subprocess-tool-registry.ts b/packages/coding-agent/src/core/tools/task/subprocess-tool-registry.ts new file mode 100644 index 000000000..2a3fd92c5 --- /dev/null +++ b/packages/coding-agent/src/core/tools/task/subprocess-tool-registry.ts @@ -0,0 +1,89 @@ +/** + * Registry for handling tool events from subprocess agents. + * + * Tools can register handlers to: + * - Extract structured data from their execution results + * - Trigger subprocess termination on completion + * - Provide custom rendering for realtime/final display + */ + +import type { Component } from "@oh-my-pi/pi-tui"; +import type { Theme } from "../../../modes/interactive/theme/theme"; + +/** Event from subprocess tool execution (parsed from JSONL) */ +export interface SubprocessToolEvent { + toolName: string; + toolCallId: string; + args?: Record; + result?: { + content: Array<{ type: string; text?: string }>; + details?: unknown; + }; + isError?: boolean; +} + +/** Handler for subprocess tool events */ +export interface SubprocessToolHandler { + /** + * Extract structured data from tool result. + * Extracted data is accumulated in progress.extractedToolData[toolName][]. + */ + extractData?: (event: SubprocessToolEvent) => TData | undefined; + + /** + * Whether this tool's completion should terminate the subprocess. + * Return true to send SIGTERM after the tool completes. + */ + shouldTerminate?: (event: SubprocessToolEvent) => boolean; + + /** + * Render a single data item inline during streaming progress. + * Called for each tool execution end event. + */ + renderInline?: (data: TData, theme: Theme) => Component; + + /** + * Render accumulated data in the final result view. + * Called once with all accumulated data for this tool. + */ + renderFinal?: (allData: TData[], theme: Theme, expanded: boolean) => Component; +} + +/** Registry for subprocess tool handlers */ +class SubprocessToolRegistryImpl { + private handlers = new Map(); + + /** + * Register a handler for a tool's subprocess events. + */ + register(toolName: string, handler: SubprocessToolHandler): void { + this.handlers.set(toolName, handler as SubprocessToolHandler); + } + + /** + * Get the handler for a tool, if registered. + */ + getHandler(toolName: string): SubprocessToolHandler | undefined { + return this.handlers.get(toolName); + } + + /** + * Check if a tool has a registered handler. + */ + hasHandler(toolName: string): boolean { + return this.handlers.has(toolName); + } + + /** + * Get all registered tool names. + */ + getRegisteredTools(): string[] { + return Array.from(this.handlers.keys()); + } +} + +/** Singleton registry instance */ +export const subprocessToolRegistry = new SubprocessToolRegistryImpl(); + +/** Type helper for extracted tool data in progress/result */ +export type ExtractedToolData = Record; diff --git a/packages/coding-agent/src/core/tools/task/types.ts b/packages/coding-agent/src/core/tools/task/types.ts index 9338923c9..e86bf5c9a 100644 --- a/packages/coding-agent/src/core/tools/task/types.ts +++ b/packages/coding-agent/src/core/tools/task/types.ts @@ -57,7 +57,6 @@ export interface ReviewSummary { overall_correctness: "correct" | "incorrect"; explanation: string; confidence: number; - findings_count: number; } /** Structured review data extracted from reviewer agent */ diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index eb38322de..d7259c6c5 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## [Unreleased] +### Added + +- Added `getText()` method to Text component for retrieving current text content ## [1.341.0] - 2026-01-03 @@ -66,4 +69,4 @@ Initial release under @oh-my-pi scope. See previous releases at [badlogic/pi-mon ### Fixed -- **Readline-style Ctrl+W**: Now skips trailing whitespace before deleting the preceding word, matching standard readline behavior. ([#306](https://github.com/badlogic/pi-mono/pull/306) by [@kim0](https://github.com/kim0)) +- **Readline-style Ctrl+W**: Now skips trailing whitespace before deleting the preceding word, matching standard readline behavior. ([#306](https://github.com/badlogic/pi-mono/pull/306) by [@kim0](https://github.com/kim0)) \ No newline at end of file diff --git a/packages/tui/src/components/text.ts b/packages/tui/src/components/text.ts index 59ce892b3..3c5bd5a5d 100644 --- a/packages/tui/src/components/text.ts +++ b/packages/tui/src/components/text.ts @@ -22,6 +22,10 @@ export class Text implements Component { this.customBgFn = customBgFn; } + getText(): string { + return this.text; + } + setText(text: string): void { this.text = text; this.cachedText = undefined; diff --git a/packages/web-ui/example/package.json b/packages/web-ui/example/package.json index 91ffb185b..f60695129 100644 --- a/packages/web-ui/example/package.json +++ b/packages/web-ui/example/package.json @@ -8,7 +8,8 @@ "build": "vite build", "preview": "vite preview", "clean": "rm -rf dist", - "check": "biome check --write . && tsgo --noEmit" + "check": "biome check --write . && tsgo --noEmit", + "fix": "biome check --write --unsafe ." }, "dependencies": { "@mariozechner/mini-lit": "^0.2.0", diff --git a/packages/web-ui/package.json b/packages/web-ui/package.json index 2cc3bc7c9..d862c6e9e 100644 --- a/packages/web-ui/package.json +++ b/packages/web-ui/package.json @@ -14,7 +14,8 @@ "dev:example": "bun --cwd=example run dev", "dev": "bun run dev:css & bun run dev:example", "build:css": "tailwindcss -i ./src/app.css -o ./dist/app.css --minify", - "check": "biome check --write . && tsgo --noEmit && bun --cwd=example run check" + "check": "biome check --write . && tsgo --noEmit && bun --cwd=example run check", + "fix": "biome check --write --unsafe . && bun --cwd=example run fix" }, "dependencies": { "@lmstudio/sdk": "^1.5.0",