fix(ai,coding-agent): preserve non-vision image placeholders closes #3429
This commit is contained in:
@@ -9,6 +9,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed built-in tool wrapping to use the same extension-runner context path as extension tools, so built-in tools receive execution context and `read` can warn when the current model does not support images ([#3429](https://github.com/badlogic/pi-mono/issues/3429))
|
||||
- Fixed threaded `/resume` session relationships and current-session detection to canonicalize symlinked session paths during selector comparisons, so shared session directories no longer break parent-child matching or active-session delete protection ([#3364](https://github.com/badlogic/pi-mono/issues/3364))
|
||||
- Fixed `/session`, Sessions docs, and CLI help to consistently document that session reuse supports both file paths and session IDs, and that `/session` shows the current session ID ([#3390](https://github.com/badlogic/pi-mono/issues/3390))
|
||||
- Fixed Windows pnpm global install detection to recognize `\\.pnpm\\` store paths, so update notices now suggest `pnpm install -g @mariozechner/pi-coding-agent` instead of falling back to npm ([#3378](https://github.com/badlogic/pi-mono/issues/3378))
|
||||
|
||||
@@ -98,7 +98,7 @@ export class AgentSessionRuntime {
|
||||
targetSessionFile?: string,
|
||||
): Promise<{ cancelled: boolean }> {
|
||||
const runner = this.session.extensionRunner;
|
||||
if (!runner?.hasHandlers("session_before_switch")) {
|
||||
if (!runner.hasHandlers("session_before_switch")) {
|
||||
return { cancelled: false };
|
||||
}
|
||||
|
||||
@@ -115,7 +115,7 @@ export class AgentSessionRuntime {
|
||||
options: { position: "before" | "at" },
|
||||
): Promise<{ cancelled: boolean }> {
|
||||
const runner = this.session.extensionRunner;
|
||||
if (!runner?.hasHandlers("session_before_fork")) {
|
||||
if (!runner.hasHandlers("session_before_fork")) {
|
||||
return { cancelled: false };
|
||||
}
|
||||
|
||||
|
||||
@@ -79,7 +79,7 @@ import { createSyntheticSourceInfo, type SourceInfo } from "./source-info.js";
|
||||
import { buildSystemPrompt } from "./system-prompt.js";
|
||||
import { type BashOperations, createLocalBashOperations } from "./tools/bash.js";
|
||||
import { createAllToolDefinitions } from "./tools/index.js";
|
||||
import { createToolDefinitionFromAgentTool, wrapToolDefinition } from "./tools/tool-definition-wrapper.js";
|
||||
import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.js";
|
||||
|
||||
// ============================================================================
|
||||
// Skill Block Parsing
|
||||
@@ -269,7 +269,7 @@ export class AgentSession {
|
||||
private _pendingBashMessages: BashExecutionMessage[] = [];
|
||||
|
||||
// Extension system
|
||||
private _extensionRunner: ExtensionRunner | undefined = undefined;
|
||||
private _extensionRunner!: ExtensionRunner;
|
||||
private _turnIndex = 0;
|
||||
|
||||
private _resourceLoader: ResourceLoader;
|
||||
@@ -365,7 +365,7 @@ export class AgentSession {
|
||||
private _installAgentToolHooks(): void {
|
||||
this.agent.beforeToolCall = async ({ toolCall, args }) => {
|
||||
const runner = this._extensionRunner;
|
||||
if (!runner?.hasHandlers("tool_call")) {
|
||||
if (!runner.hasHandlers("tool_call")) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -388,7 +388,7 @@ export class AgentSession {
|
||||
|
||||
this.agent.afterToolCall = async ({ toolCall, args, result, isError }) => {
|
||||
const runner = this._extensionRunner;
|
||||
if (!runner?.hasHandlers("tool_result")) {
|
||||
if (!runner.hasHandlers("tool_result")) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -604,8 +604,6 @@ export class AgentSession {
|
||||
|
||||
/** Emit extension events based on agent events */
|
||||
private async _emitExtensionEvent(event: AgentEvent): Promise<void> {
|
||||
if (!this._extensionRunner) return;
|
||||
|
||||
if (event.type === "agent_start") {
|
||||
this._turnIndex = 0;
|
||||
await this._extensionRunner.emit({ type: "agent_start" });
|
||||
@@ -949,7 +947,7 @@ export class AgentSession {
|
||||
// Emit input event for extension interception (before skill/template expansion)
|
||||
let currentText = text;
|
||||
let currentImages = options?.images;
|
||||
if (this._extensionRunner?.hasHandlers("input")) {
|
||||
if (this._extensionRunner.hasHandlers("input")) {
|
||||
const inputResult = await this._extensionRunner.emitInput(
|
||||
currentText,
|
||||
currentImages,
|
||||
@@ -1042,33 +1040,31 @@ export class AgentSession {
|
||||
this._pendingNextTurnMessages = [];
|
||||
|
||||
// Emit before_agent_start extension event
|
||||
if (this._extensionRunner) {
|
||||
const result = await this._extensionRunner.emitBeforeAgentStart(
|
||||
expandedText,
|
||||
currentImages,
|
||||
this._baseSystemPrompt,
|
||||
);
|
||||
// Add all custom messages from extensions
|
||||
if (result?.messages) {
|
||||
for (const msg of result.messages) {
|
||||
messages.push({
|
||||
role: "custom",
|
||||
customType: msg.customType,
|
||||
content: msg.content,
|
||||
display: msg.display,
|
||||
details: msg.details,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
}
|
||||
// Apply extension-modified system prompt, or reset to base
|
||||
if (result?.systemPrompt) {
|
||||
this.agent.state.systemPrompt = result.systemPrompt;
|
||||
} else {
|
||||
// Ensure we're using the base prompt (in case previous turn had modifications)
|
||||
this.agent.state.systemPrompt = this._baseSystemPrompt;
|
||||
const result = await this._extensionRunner.emitBeforeAgentStart(
|
||||
expandedText,
|
||||
currentImages,
|
||||
this._baseSystemPrompt,
|
||||
);
|
||||
// Add all custom messages from extensions
|
||||
if (result?.messages) {
|
||||
for (const msg of result.messages) {
|
||||
messages.push({
|
||||
role: "custom",
|
||||
customType: msg.customType,
|
||||
content: msg.content,
|
||||
display: msg.display,
|
||||
details: msg.details,
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
}
|
||||
}
|
||||
// Apply extension-modified system prompt, or reset to base
|
||||
if (result?.systemPrompt) {
|
||||
this.agent.state.systemPrompt = result.systemPrompt;
|
||||
} else {
|
||||
// Ensure we're using the base prompt (in case previous turn had modifications)
|
||||
this.agent.state.systemPrompt = this._baseSystemPrompt;
|
||||
}
|
||||
} catch (error) {
|
||||
preflightResult?.(false);
|
||||
throw error;
|
||||
@@ -1087,8 +1083,6 @@ export class AgentSession {
|
||||
* Try to execute an extension command. Returns true if command was found and executed.
|
||||
*/
|
||||
private async _tryExecuteExtensionCommand(text: string): Promise<boolean> {
|
||||
if (!this._extensionRunner) return false;
|
||||
|
||||
// Parse command name and args
|
||||
const spaceIndex = text.indexOf(" ");
|
||||
const commandName = spaceIndex === -1 ? text.slice(1) : text.slice(1, spaceIndex);
|
||||
@@ -1136,7 +1130,7 @@ export class AgentSession {
|
||||
return args ? `${skillBlock}\n\n${args}` : skillBlock;
|
||||
} catch (err) {
|
||||
// Emit error like extension commands do
|
||||
this._extensionRunner?.emitError({
|
||||
this._extensionRunner.emitError({
|
||||
extensionPath: skill.filePath,
|
||||
event: "skill_expansion",
|
||||
error: err instanceof Error ? err.message : String(err),
|
||||
@@ -1224,8 +1218,6 @@ export class AgentSession {
|
||||
* Throw an error if the text is an extension command.
|
||||
*/
|
||||
private _throwIfExtensionCommand(text: string): void {
|
||||
if (!this._extensionRunner) return;
|
||||
|
||||
const spaceIndex = text.indexOf(" ");
|
||||
const commandName = spaceIndex === -1 ? text.slice(1) : text.slice(1, spaceIndex);
|
||||
const command = this._extensionRunner.getCommand(commandName);
|
||||
@@ -1376,7 +1368,6 @@ export class AgentSession {
|
||||
previousModel: Model<any> | undefined,
|
||||
source: "set" | "cycle" | "restore",
|
||||
): Promise<void> {
|
||||
if (!this._extensionRunner) return;
|
||||
if (modelsAreEqual(previousModel, nextModel)) return;
|
||||
await this._extensionRunner.emit({
|
||||
type: "model_select",
|
||||
@@ -1628,7 +1619,7 @@ export class AgentSession {
|
||||
let extensionCompaction: CompactionResult | undefined;
|
||||
let fromExtension = false;
|
||||
|
||||
if (this._extensionRunner?.hasHandlers("session_before_compact")) {
|
||||
if (this._extensionRunner.hasHandlers("session_before_compact")) {
|
||||
const result = (await this._extensionRunner.emit({
|
||||
type: "session_before_compact",
|
||||
preparation,
|
||||
@@ -1886,7 +1877,7 @@ export class AgentSession {
|
||||
let extensionCompaction: CompactionResult | undefined;
|
||||
let fromExtension = false;
|
||||
|
||||
if (this._extensionRunner?.hasHandlers("session_before_compact")) {
|
||||
if (this._extensionRunner.hasHandlers("session_before_compact")) {
|
||||
const extensionResult = (await this._extensionRunner.emit({
|
||||
type: "session_before_compact",
|
||||
preparation,
|
||||
@@ -2038,15 +2029,13 @@ export class AgentSession {
|
||||
this._extensionErrorListener = bindings.onError;
|
||||
}
|
||||
|
||||
if (this._extensionRunner) {
|
||||
this._applyExtensionBindings(this._extensionRunner);
|
||||
await this._extensionRunner.emit(this._sessionStartEvent);
|
||||
await this.extendResourcesFromExtensions(this._sessionStartEvent.reason === "reload" ? "reload" : "startup");
|
||||
}
|
||||
this._applyExtensionBindings(this._extensionRunner);
|
||||
await this._extensionRunner.emit(this._sessionStartEvent);
|
||||
await this.extendResourcesFromExtensions(this._sessionStartEvent.reason === "reload" ? "reload" : "startup");
|
||||
}
|
||||
|
||||
private async extendResourcesFromExtensions(reason: "startup" | "reload"): Promise<void> {
|
||||
if (!this._extensionRunner?.hasHandlers("resources_discover")) {
|
||||
if (!this._extensionRunner.hasHandlers("resources_discover")) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -2233,7 +2222,7 @@ export class AgentSession {
|
||||
const previousRegistryNames = new Set(this._toolRegistry.keys());
|
||||
const previousActiveToolNames = this.getActiveToolNames();
|
||||
|
||||
const registeredTools = this._extensionRunner?.getAllRegisteredTools() ?? [];
|
||||
const registeredTools = this._extensionRunner.getAllRegisteredTools();
|
||||
const allCustomTools = [
|
||||
...registeredTools,
|
||||
...this._customTools.map((definition) => ({
|
||||
@@ -2273,16 +2262,17 @@ export class AgentSession {
|
||||
})
|
||||
.filter((entry): entry is readonly [string, string[]] => entry !== undefined),
|
||||
);
|
||||
const wrappedExtensionTools = this._extensionRunner
|
||||
? wrapRegisteredTools(allCustomTools, this._extensionRunner)
|
||||
: [];
|
||||
|
||||
const toolRegistry = new Map(
|
||||
Array.from(this._baseToolDefinitions.values()).map((definition) => [
|
||||
definition.name,
|
||||
wrapToolDefinition(definition),
|
||||
]),
|
||||
const runner = this._extensionRunner;
|
||||
const wrappedExtensionTools = wrapRegisteredTools(allCustomTools, runner);
|
||||
const wrappedBuiltInTools = wrapRegisteredTools(
|
||||
Array.from(this._baseToolDefinitions.values()).map((definition) => ({
|
||||
definition,
|
||||
sourceInfo: createSyntheticSourceInfo(`<builtin:${definition.name}>`, { source: "builtin" }),
|
||||
})),
|
||||
runner,
|
||||
);
|
||||
|
||||
const toolRegistry = new Map(wrappedBuiltInTools.map((tool) => [tool.name, tool]));
|
||||
for (const tool of wrappedExtensionTools as AgentTool[]) {
|
||||
toolRegistry.set(tool.name, tool);
|
||||
}
|
||||
@@ -2337,25 +2327,18 @@ export class AgentSession {
|
||||
}
|
||||
}
|
||||
|
||||
const hasExtensions = extensionsResult.extensions.length > 0;
|
||||
const hasCustomTools = this._customTools.length > 0;
|
||||
this._extensionRunner =
|
||||
hasExtensions || hasCustomTools
|
||||
? new ExtensionRunner(
|
||||
extensionsResult.extensions,
|
||||
extensionsResult.runtime,
|
||||
this._cwd,
|
||||
this.sessionManager,
|
||||
this._modelRegistry,
|
||||
)
|
||||
: undefined;
|
||||
this._extensionRunner = new ExtensionRunner(
|
||||
extensionsResult.extensions,
|
||||
extensionsResult.runtime,
|
||||
this._cwd,
|
||||
this.sessionManager,
|
||||
this._modelRegistry,
|
||||
);
|
||||
if (this._extensionRunnerRef) {
|
||||
this._extensionRunnerRef.current = this._extensionRunner;
|
||||
}
|
||||
if (this._extensionRunner) {
|
||||
this._bindExtensionCore(this._extensionRunner);
|
||||
this._applyExtensionBindings(this._extensionRunner);
|
||||
}
|
||||
this._bindExtensionCore(this._extensionRunner);
|
||||
this._applyExtensionBindings(this._extensionRunner);
|
||||
|
||||
const defaultActiveToolNames = this._baseToolsOverride
|
||||
? Object.keys(this._baseToolsOverride)
|
||||
@@ -2368,8 +2351,8 @@ export class AgentSession {
|
||||
}
|
||||
|
||||
async reload(): Promise<void> {
|
||||
const previousFlagValues = this._extensionRunner?.getFlagValues();
|
||||
await this._extensionRunner?.emit({ type: "session_shutdown" });
|
||||
const previousFlagValues = this._extensionRunner.getFlagValues();
|
||||
await this._extensionRunner.emit({ type: "session_shutdown" });
|
||||
await this.settingsManager.reload();
|
||||
resetApiProviders();
|
||||
await this._resourceLoader.reload();
|
||||
@@ -2384,7 +2367,7 @@ export class AgentSession {
|
||||
this._extensionCommandContextActions ||
|
||||
this._extensionShutdownHandler ||
|
||||
this._extensionErrorListener;
|
||||
if (this._extensionRunner && hasBindings) {
|
||||
if (hasBindings) {
|
||||
await this._extensionRunner.emit({ type: "session_start", reason: "reload" });
|
||||
await this.extendResourcesFromExtensions("reload");
|
||||
}
|
||||
@@ -2713,7 +2696,7 @@ export class AgentSession {
|
||||
let fromExtension = false;
|
||||
|
||||
// Emit session_before_tree event
|
||||
if (this._extensionRunner?.hasHandlers("session_before_tree")) {
|
||||
if (this._extensionRunner.hasHandlers("session_before_tree")) {
|
||||
const result = (await this._extensionRunner.emit({
|
||||
type: "session_before_tree",
|
||||
preparation,
|
||||
@@ -2827,15 +2810,13 @@ export class AgentSession {
|
||||
this.agent.state.messages = sessionContext.messages;
|
||||
|
||||
// Emit session_tree event
|
||||
if (this._extensionRunner) {
|
||||
await this._extensionRunner.emit({
|
||||
type: "session_tree",
|
||||
newLeafId: this.sessionManager.getLeafId(),
|
||||
oldLeafId,
|
||||
summaryEntry,
|
||||
fromExtension: summaryText ? fromExtension : undefined,
|
||||
});
|
||||
}
|
||||
await this._extensionRunner.emit({
|
||||
type: "session_tree",
|
||||
newLeafId: this.sessionManager.getLeafId(),
|
||||
oldLeafId,
|
||||
summaryEntry,
|
||||
fromExtension: summaryText ? fromExtension : undefined,
|
||||
});
|
||||
|
||||
// Emit to custom tools
|
||||
|
||||
@@ -3067,13 +3048,13 @@ export class AgentSession {
|
||||
* Check if extensions have handlers for a specific event type.
|
||||
*/
|
||||
hasExtensionHandlers(eventType: string): boolean {
|
||||
return this._extensionRunner?.hasHandlers(eventType) ?? false;
|
||||
return this._extensionRunner.hasHandlers(eventType);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the extension runner (for setting UI context and error handlers).
|
||||
*/
|
||||
get extensionRunner(): ExtensionRunner | undefined {
|
||||
get extensionRunner(): ExtensionRunner {
|
||||
return this._extensionRunner;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import { wrapToolDefinition } from "../tools/tool-definition-wrapper.js";
|
||||
import { wrapToolDefinition, wrapToolDefinitions } from "../tools/tool-definition-wrapper.js";
|
||||
import type { ExtensionRunner } from "./runner.js";
|
||||
import type { RegisteredTool } from "./types.js";
|
||||
|
||||
@@ -23,5 +23,8 @@ export function wrapRegisteredTool(registeredTool: RegisteredTool, runner: Exten
|
||||
* Uses the runner's createContext() for consistent context across tools and event handlers.
|
||||
*/
|
||||
export function wrapRegisteredTools(registeredTools: RegisteredTool[], runner: ExtensionRunner): AgentTool[] {
|
||||
return registeredTools.map((rt) => wrapRegisteredTool(rt, runner));
|
||||
return wrapToolDefinitions(
|
||||
registeredTools.map((registeredTool) => registeredTool.definition),
|
||||
() => runner.createContext(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import type { ImageContent, TextContent } from "@mariozechner/pi-ai";
|
||||
import type { Api, ImageContent, Model, TextContent } from "@mariozechner/pi-ai";
|
||||
import { Text } from "@mariozechner/pi-tui";
|
||||
import { type Static, Type } from "@sinclair/typebox";
|
||||
import { constants } from "fs";
|
||||
@@ -78,6 +78,13 @@ function trimTrailingEmptyLines(lines: string[]): string[] {
|
||||
return lines.slice(0, end);
|
||||
}
|
||||
|
||||
function getNonVisionImageNote(model: Model<Api> | undefined): string | undefined {
|
||||
if (!model || model.input.includes("image")) {
|
||||
return undefined;
|
||||
}
|
||||
return "[Current model does not support images. The image will be omitted from this request.]";
|
||||
}
|
||||
|
||||
function formatReadResult(
|
||||
args: { path?: string; file_path?: string; offset?: number; limit?: number } | undefined,
|
||||
result: { content: (TextContent | ImageContent)[]; details?: ReadToolDetails },
|
||||
@@ -129,7 +136,7 @@ export function createReadToolDefinition(
|
||||
{ path, offset, limit }: { path: string; offset?: number; limit?: number },
|
||||
signal?: AbortSignal,
|
||||
_onUpdate?,
|
||||
_ctx?,
|
||||
ctx?,
|
||||
) {
|
||||
const absolutePath = resolveReadPath(path, cwd);
|
||||
return new Promise<{ content: (TextContent | ImageContent)[]; details: ReadToolDetails | undefined }>(
|
||||
@@ -153,6 +160,7 @@ export function createReadToolDefinition(
|
||||
const mimeType = ops.detectImageMimeType ? await ops.detectImageMimeType(absolutePath) : undefined;
|
||||
let content: (TextContent | ImageContent)[];
|
||||
let details: ReadToolDetails | undefined;
|
||||
const nonVisionImageNote = getNonVisionImageNote(ctx?.model);
|
||||
if (mimeType) {
|
||||
// Read image as binary.
|
||||
const buffer = await ops.readFile(absolutePath);
|
||||
@@ -161,24 +169,24 @@ export function createReadToolDefinition(
|
||||
// Resize image if needed before sending it back to the model.
|
||||
const resized = await resizeImage({ type: "image", data: base64, mimeType });
|
||||
if (!resized) {
|
||||
content = [
|
||||
{
|
||||
type: "text",
|
||||
text: `Read image file [${mimeType}]\n[Image omitted: could not be resized below the inline image size limit.]`,
|
||||
},
|
||||
];
|
||||
let textNote = `Read image file [${mimeType}]\n[Image omitted: could not be resized below the inline image size limit.]`;
|
||||
if (nonVisionImageNote) textNote += `\n${nonVisionImageNote}`;
|
||||
content = [{ type: "text", text: textNote }];
|
||||
} else {
|
||||
const dimensionNote = formatDimensionNote(resized);
|
||||
let textNote = `Read image file [${resized.mimeType}]`;
|
||||
if (dimensionNote) textNote += `\n${dimensionNote}`;
|
||||
if (nonVisionImageNote) textNote += `\n${nonVisionImageNote}`;
|
||||
content = [
|
||||
{ type: "text", text: textNote },
|
||||
{ type: "image", data: resized.data, mimeType: resized.mimeType },
|
||||
];
|
||||
}
|
||||
} else {
|
||||
let textNote = `Read image file [${mimeType}]`;
|
||||
if (nonVisionImageNote) textNote += `\n${nonVisionImageNote}`;
|
||||
content = [
|
||||
{ type: "text", text: `Read image file [${mimeType}]` },
|
||||
{ type: "text", text: textNote },
|
||||
{ type: "image", data: base64, mimeType },
|
||||
];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user