From 8cf431b771cba2c0b80a9c280c871303b0924cea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 21 May 2026 22:02:27 +0200 Subject: [PATCH] agent: remove ToolExecutor abstraction Inlines the browser, session, and node registry management directly into Agent. Updates CommandRunner to use Session and Registry directly, and calls browser_tools.call without the ToolExecutor wrapper. --- src/agent/Agent.zig | 103 +++++++++++++++++++++----------- src/agent/CommandRunner.zig | 12 ++-- src/agent/SlashCommand.zig | 5 -- src/agent/Terminal.zig | 3 - src/agent/ToolExecutor.zig | 114 ------------------------------------ src/script/schema.zig | 3 +- 6 files changed, 77 insertions(+), 163 deletions(-) delete mode 100644 src/agent/ToolExecutor.zig diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 90cc89d8..c958d8aa 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -30,7 +30,7 @@ const Verifier = lp.script.Verifier; const Credentials = zenai.provider.Credentials; const App = @import("../App.zig"); -const ToolExecutor = @import("ToolExecutor.zig"); +const CDPNode = @import("../cdp/Node.zig"); const Terminal = @import("Terminal.zig"); const CommandRunner = @import("CommandRunner.zig"); const SlashCommand = @import("SlashCommand.zig"); @@ -139,7 +139,14 @@ const synthesis_prompt = allocator: std.mem.Allocator, ai_client: ?zenai.provider.Client, -tool_executor: *ToolExecutor, +notification: *lp.Notification, +browser: lp.Browser, +session: *lp.Session, +node_registry: CDPNode.Registry, +tool_schema_arena: std.heap.ArenaAllocator, +/// Schemas parsed once at init from `browser_tools.tool_defs`. The slice and +/// every JSON `Value` inside live in `tool_schema_arena`. +tools: []const zenai.provider.Tool, terminal: Terminal, cmd_runner: CommandRunner, verifier: Verifier, @@ -207,7 +214,7 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent } // Resolve model BEFORE the heavy init so --pick-model's prompt fires - // before tool_executor / ai_client setup. + // before browser / ai_client setup. // Precedence: --model > --pick-model > defaultModel. const model: []u8 = if (opts.model) |m| try allocator.dupe(u8, m) @@ -217,30 +224,12 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent try allocator.dupe(u8, ""); errdefer allocator.free(model); - const tool_executor: *ToolExecutor = try .init(allocator, app); - errdefer tool_executor.deinit(); + const notification: *lp.Notification = try .init(allocator); + errdefer notification.deinit(); const self = try allocator.create(Agent); errdefer allocator.destroy(self); - const ai_client: ?zenai.provider.Client = if (llm) |l| switch (l.provider) { - inline else => |tag| blk: { - const ProviderClient = zenai.provider.Client; - const ClientPtr = @FieldType(ProviderClient, @tagName(tag)); - const Client = @typeInfo(ClientPtr).pointer.child; - const client = try allocator.create(Client); - const url: ?[]const u8 = opts.base_url orelse if (tag == .ollama) "http://localhost:11434/v1" else null; - client.* = Client.init(allocator, l.key, if (url) |u| .{ .base_url = u } else .{}); - break :blk @unionInit(ProviderClient, @tagName(tag), client); - }, - } else null; - errdefer if (ai_client) |c| switch (c) { - inline else => |client| { - client.deinit(); - allocator.destroy(client); - }, - }; - const history_path: ?[:0]const u8 = if (will_repl) ".lp-history" else null; // `-i ` means "replay then grow this file"; a script path alone is @@ -249,11 +238,16 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent self.* = .{ .allocator = allocator, - .ai_client = ai_client, - .tool_executor = tool_executor, + .ai_client = null, + .notification = notification, + .browser = undefined, + .session = undefined, + .node_registry = CDPNode.Registry.init(allocator), + .tool_schema_arena = .init(allocator), + .tools = &.{}, .terminal = .init(allocator, history_path, Config.agentVerbosity(opts), will_repl), .cmd_runner = undefined, - .verifier = .{ .session = tool_executor.session, .node_registry = &tool_executor.node_registry }, + .verifier = undefined, .recorder = null, .messages = .empty, .message_arena = .init(allocator), @@ -265,8 +259,35 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent .one_shot_task = opts.task, .one_shot_attachments = if (opts.attach.items.len == 0) null else opts.attach.items, }; + errdefer self.tool_schema_arena.deinit(); + errdefer self.node_registry.deinit(); - self.cmd_runner = .init(tool_executor, &self.terminal); + self.tools = try buildTools(self.tool_schema_arena.allocator()); + + try self.browser.init(app, .{}, null); + errdefer self.browser.deinit(); + + self.session = try self.browser.newSession(notification); + self.verifier = .{ .session = self.session, .node_registry = &self.node_registry }; + self.cmd_runner = .init(self.session, &self.node_registry, &self.terminal); + + self.ai_client = if (llm) |l| switch (l.provider) { + inline else => |tag| blk: { + const ProviderClient = zenai.provider.Client; + const ClientPtr = @FieldType(ProviderClient, @tagName(tag)); + const Client = @typeInfo(ClientPtr).pointer.child; + const client = try allocator.create(Client); + const url: ?[]const u8 = opts.base_url orelse if (tag == .ollama) "http://localhost:11434/v1" else null; + client.* = Client.init(allocator, l.key, if (url) |u| .{ .base_url = u } else .{}); + break :blk @unionInit(ProviderClient, @tagName(tag), client); + }, + } else null; + errdefer if (self.ai_client) |c| switch (c) { + inline else => |client| { + client.deinit(); + allocator.destroy(client); + }, + }; if (will_repl) self.terminal.attachCompleter(); @@ -287,7 +308,10 @@ pub fn deinit(self: *Agent) void { self.terminal.deinit(); self.message_arena.deinit(); self.messages.deinit(self.allocator); - self.tool_executor.deinit(); + self.tool_schema_arena.deinit(); + self.node_registry.deinit(); + self.browser.deinit(); + self.notification.deinit(); if (self.ai_client) |ai_client| { switch (ai_client) { inline else => |c| { @@ -300,6 +324,15 @@ pub fn deinit(self: *Agent) void { self.allocator.destroy(self); } +fn buildTools(arena: std.mem.Allocator) ![]const zenai.provider.Tool { + const tools = try arena.alloc(zenai.provider.Tool, browser_tools.tool_defs.len); + for (browser_tools.tool_defs, 0..) |t, i| { + const parsed = try std.json.parseFromSliceLeaky(std.json.Value, arena, t.input_schema, .{}); + tools[i] = .{ .name = t.name, .description = t.description, .parameters = parsed }; + } + return tools; +} + /// Called from the sighandler thread — sets the flag only, no terminal /// or V8 touches from this context. pub fn requestCancel(self: *Agent) void { @@ -338,7 +371,7 @@ fn checkCancel(ctx: *anyopaque) bool { /// already happened on its path. fn drainCancellation(self: *Agent, baseline: usize) error{UserCancelled} { self.rollbackMessages(baseline); - self.tool_executor.browser.env.cancelTerminate(); + self.browser.env.cancelTerminate(); self.cancel_requested.store(false, .release); return error.UserCancelled; } @@ -390,7 +423,7 @@ fn runTurn(self: *Agent, input: TurnInput) bool { fn runRepl(self: *Agent) void { self.terminal.printInfo("Lightpanda Agent (type '/quit' to exit)"); self.terminal.printInfo("Tab completes/cycles through commands; the dim grey ghost shows the first match."); - log.debug(.app, "tools loaded", .{ .count = self.tool_executor.tools.len }); + log.debug(.app, "tools loaded", .{ .count = self.tools.len }); if (self.ai_client) |ai_client| { self.terminal.printInfoFmt("Provider: {s}, Model: {s}", .{ @tagName(std.meta.activeTag(ai_client)), self.model }); } else { @@ -406,7 +439,7 @@ fn runRepl(self: *Agent) void { // Slash commands and idle Ctrl-C set the cancel flag without // clearing V8's terminate state; drain both before the next turn. if (self.cancel_requested.swap(false, .acq_rel)) { - self.tool_executor.browser.env.cancelTerminate(); + self.browser.env.cancelTerminate(); } if (line.len == 0) continue; @@ -800,7 +833,7 @@ fn runHealTurn(self: *Agent, arena: std.mem.Allocator, prompt: []const u8) ![]Co ma, .{ .context = @ptrCast(self), .callFn = handleToolCall }, .{ - .tools = self.tool_executor.tools, + .tools = self.tools, .max_tool_calls = 4, .max_tokens = 4096, .tool_choice = .auto, @@ -846,7 +879,7 @@ fn attemptSelfHeal(self: *Agent, arena: std.mem.Allocator, failed_command: []con self_heal_prompt_prefix, failed_command, self_heal_prompt_page_state, - self.tool_executor.getCurrentUrl(), + browser_tools.currentUrlOrPlaceholder(self.session), }) catch return null; if (context_comment) |c| aw.writer.print("\n\nThe original user request that generated this command was:\n{s}", .{c}) catch return null; @@ -954,7 +987,7 @@ fn processUserMessage(self: *Agent, input: TurnInput) !?[]const u8 { ma, .{ .context = @ptrCast(self), .callFn = handleToolCall }, .{ - .tools = self.tool_executor.tools, + .tools = self.tools, .max_turns = 30, // Safety net; max_turns is the primary terminal. .max_tool_calls = 200, @@ -1142,7 +1175,7 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co self.terminal.spinner.setTool(tool_name, args_str); defer self.terminal.spinner.setThinking(); - if (self.tool_executor.callValue(allocator, tool_name, arguments)) |result| { + if (browser_tools.call(allocator, self.session, &self.node_registry, tool_name, arguments)) |result| { const capped = capToolOutput(allocator, result.text); self.terminal.agentToolDone(tool_name, args_str, !result.is_error); if (self.terminal.verbosity == .high) self.terminal.printToolResult(tool_name, capped); diff --git a/src/agent/CommandRunner.zig b/src/agent/CommandRunner.zig index 97742d16..c3a2d0b2 100644 --- a/src/agent/CommandRunner.zig +++ b/src/agent/CommandRunner.zig @@ -20,17 +20,19 @@ const std = @import("std"); const lp = @import("lightpanda"); const browser_tools = lp.tools; const Command = lp.script.Command; -const ToolExecutor = @import("ToolExecutor.zig"); +const CDPNode = @import("../cdp/Node.zig"); const Terminal = @import("Terminal.zig"); const CommandRunner = @This(); -tool_executor: *ToolExecutor, +session: *lp.Session, +node_registry: *CDPNode.Registry, terminal: *Terminal, -pub fn init(tool_executor: *ToolExecutor, terminal: *Terminal) CommandRunner { +pub fn init(session: *lp.Session, node_registry: *CDPNode.Registry, terminal: *Terminal) CommandRunner { return .{ - .tool_executor = tool_executor, + .session = session, + .node_registry = node_registry, .terminal = terminal, }; } @@ -45,7 +47,7 @@ pub fn executeWithResult(self: *CommandRunner, arena: std.mem.Allocator, cmd: Co }; const substituted = substituteStringArgs(arena, tc.name, tc.args) catch return .{ .text = "out of memory", .is_error = true }; - return self.tool_executor.callValue(arena, tc.name, substituted) catch |err| .{ + return browser_tools.call(arena, self.session, self.node_registry, tc.name, substituted) catch |err| .{ .text = std.fmt.allocPrint(arena, "{s} failed: {s}", .{ tc.name, @errorName(err) }) catch "tool failed", .is_error = true, }; diff --git a/src/agent/SlashCommand.zig b/src/agent/SlashCommand.zig index 932ab810..6a460686 100644 --- a/src/agent/SlashCommand.zig +++ b/src/agent/SlashCommand.zig @@ -23,20 +23,15 @@ const std = @import("std"); const lp = @import("lightpanda"); -const browser_tools = lp.tools; const schema = lp.script.schema; // Re-export so existing call sites (Agent, Terminal) keep their import path. -pub const FieldType = schema.FieldType; -pub const FieldEntry = schema.FieldEntry; -pub const HintSlot = schema.HintSlot; pub const SchemaInfo = schema.SchemaInfo; pub const ParseError = schema.ParseError; pub const Split = schema.Split; pub const max_hint_slots = schema.max_hint_slots; -pub const buildSchemas = schema.buildSchemas; pub const globalSchemas = schema.globalSchemas; pub const findSchema = schema.findSchema; pub const findSchemaCanonical = schema.findSchemaCanonical; diff --git a/src/agent/Terminal.zig b/src/agent/Terminal.zig index 72592797..0456d00f 100644 --- a/src/agent/Terminal.zig +++ b/src/agent/Terminal.zig @@ -20,7 +20,6 @@ const std = @import("std"); const lp = @import("lightpanda"); const browser_tools = lp.tools; const Config = lp.Config; -const Command = lp.script.Command; const SlashCommand = @import("SlashCommand.zig"); const Spinner = @import("Spinner.zig"); const c = @cImport({ @@ -29,7 +28,6 @@ const c = @cImport({ const Terminal = @This(); -const style_cmd = "ps-cmd"; const style_slash = "ps-slash"; const style_string = "ps-string"; const style_var = "ps-var"; @@ -93,7 +91,6 @@ pub fn init(allocator: std.mem.Allocator, history_path: ?[:0]const u8, verbosity _ = c.ic_set_hint_delay(0); _ = c.ic_enable_brace_insertion(true); // `ps-*` namespace avoids colliding with isocline's built-in `ic-*` styles. - c.ic_style_def(style_cmd, "ansi-cyan bold"); c.ic_style_def(style_slash, "ansi-magenta bold"); c.ic_style_def(style_string, "ansi-green"); c.ic_style_def(style_var, "ansi-yellow bold"); diff --git a/src/agent/ToolExecutor.zig b/src/agent/ToolExecutor.zig deleted file mode 100644 index 26eb37f8..00000000 --- a/src/agent/ToolExecutor.zig +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) -// -// Francis Bouvier -// Pierre Tachoire -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as -// published by the Free Software Foundation, either version 3 of the -// License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -const std = @import("std"); -const lp = @import("lightpanda"); -const zenai = @import("zenai"); - -const App = @import("../App.zig"); -const CDPNode = @import("../cdp/Node.zig"); -const browser_tools = lp.tools; - -const ToolExecutor = @This(); - -allocator: std.mem.Allocator, -app: *App, -notification: *lp.Notification, -browser: lp.Browser, -session: *lp.Session, -node_registry: CDPNode.Registry, -tool_schema_arena: std.heap.ArenaAllocator, -/// Schemas parsed once at init from `browser_tools.tool_defs`. The slice and -/// every JSON `Value` inside live in `tool_schema_arena`. -tools: []const zenai.provider.Tool, - -pub fn init(allocator: std.mem.Allocator, app: *App) !*ToolExecutor { - const notification: *lp.Notification = try .init(allocator); - errdefer notification.deinit(); - - const self = try allocator.create(ToolExecutor); - errdefer allocator.destroy(self); - - self.* = .{ - .allocator = allocator, - .app = app, - .notification = notification, - .browser = undefined, - .session = undefined, - .node_registry = CDPNode.Registry.init(allocator), - .tool_schema_arena = std.heap.ArenaAllocator.init(allocator), - .tools = &.{}, - }; - errdefer self.tool_schema_arena.deinit(); - errdefer self.node_registry.deinit(); - - self.tools = try buildTools(self.tool_schema_arena.allocator()); - - try self.browser.init(app, .{}, null); - errdefer self.browser.deinit(); - - self.session = try self.browser.newSession(self.notification); - return self; -} - -pub fn buildTools(arena: std.mem.Allocator) ![]const zenai.provider.Tool { - const tools = try arena.alloc(zenai.provider.Tool, browser_tools.tool_defs.len); - for (browser_tools.tool_defs, 0..) |t, i| { - const parsed = try std.json.parseFromSliceLeaky(std.json.Value, arena, t.input_schema, .{}); - tools[i] = .{ .name = t.name, .description = t.description, .parameters = parsed }; - } - return tools; -} - -pub fn deinit(self: *ToolExecutor) void { - self.tool_schema_arena.deinit(); - self.node_registry.deinit(); - self.browser.deinit(); - self.notification.deinit(); - self.allocator.destroy(self); -} - -pub const CallError = browser_tools.ToolError || error{InvalidJsonArguments}; - -/// Allocator backing the parsed tool schemas. Lives for the executor's -/// lifetime, so callers can hand back slices that need the same lifetime -/// (e.g. derived caches over `getTools` output). -pub fn schemaAllocator(self: *ToolExecutor) std.mem.Allocator { - return self.tool_schema_arena.allocator(); -} - -pub fn getCurrentUrl(self: *ToolExecutor) []const u8 { - return browser_tools.currentUrlOrPlaceholder(self.session); -} - -pub fn call(self: *ToolExecutor, arena: std.mem.Allocator, tool_name: []const u8, arguments_json: []const u8) CallError!browser_tools.ToolResult { - const arguments: ?std.json.Value = if (arguments_json.len > 0) - std.json.parseFromSliceLeaky(std.json.Value, arena, arguments_json, .{}) catch - return error.InvalidJsonArguments - else - null; - - return self.callValue(arena, tool_name, arguments); -} - -/// Like `call` but takes an already-parsed JSON value. Skips the -/// stringify+reparse for callers (e.g. PandaScript replay) that already -/// have a `std.json.Value`. -pub fn callValue(self: *ToolExecutor, arena: std.mem.Allocator, tool_name: []const u8, arguments: ?std.json.Value) browser_tools.ToolError!browser_tools.ToolResult { - return browser_tools.call(arena, self.session, &self.node_registry, tool_name, arguments); -} diff --git a/src/script/schema.zig b/src/script/schema.zig index a0e240c2..adb4fcda 100644 --- a/src/script/schema.zig +++ b/src/script/schema.zig @@ -237,7 +237,8 @@ pub fn splitNameRest(input: []const u8) ?Split { /// Parse `rest` (the args portion of a slash command) into a `std.json.Value` /// shaped for the tool. Returns null when the schema takes no args and `rest` -/// is empty; that lets the caller pass `null` straight to `tool_executor.call`. +/// is empty; that lets the caller pass `null` straight to `tool_executor.call` +/// without allocating an empty object. /// /// Argument-binding rules: /// - Bare `{json}` payload — returned as-is after JSON parse. Pass-through