From 486d0d53a9182cc43dd9c28d907a8b1fc401c6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 22 May 2026 15:50:36 +0200 Subject: [PATCH] agent: inline CommandRunner and simplify Command union --- src/agent/Agent.zig | 52 ++++++++++++++++++++--------- src/agent/CommandRunner.zig | 65 ------------------------------------- src/agent/Terminal.zig | 14 ++++---- src/script/Iterator.zig | 2 +- src/script/Schema.zig | 12 ++++--- src/script/command.zig | 59 +++++++++++++-------------------- 6 files changed, 76 insertions(+), 128 deletions(-) delete mode 100644 src/agent/CommandRunner.zig diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 4794460d..c5317cf1 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -35,7 +35,6 @@ const Credentials = zenai.provider.Credentials; const App = @import("../App.zig"); const CDPNode = @import("../cdp/Node.zig"); const Terminal = @import("Terminal.zig"); -const CommandRunner = @import("CommandRunner.zig"); const SlashCommand = @import("SlashCommand.zig"); const Agent = @This(); @@ -147,7 +146,6 @@ browser: lp.Browser, session: *lp.Session, node_registry: CDPNode.Registry, terminal: Terminal, -cmd_runner: CommandRunner, verifier: Verifier, recorder: ?Recorder, messages: std.ArrayList(zenai.provider.Message), @@ -243,7 +241,6 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent .session = undefined, .node_registry = CDPNode.Registry.init(allocator), .terminal = .init(allocator, history_path, Config.agentVerbosity(opts), will_repl), - .cmd_runner = undefined, .verifier = undefined, .recorder = null, .messages = .empty, @@ -265,7 +262,6 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent 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: { @@ -476,20 +472,20 @@ fn runRepl(self: *Agent) void { switch (cmd) { .comment => continue :repl, - .login, .accept_cookies => { + .login, .acceptCookies => { if (self.ai_client == null) { self.terminal.printError("/login and /acceptCookies require an LLM. Drop --no-llm and set an API key (ANTHROPIC_API_KEY, OPENAI_API_KEY, or GOOGLE_API_KEY)."); continue :repl; } const prompt = if (cmd == .login) login_prompt else accept_cookies_prompt; - const label: []const u8 = if (cmd == .login) "/" ++ @tagName(Command.LlmCommand.login) else "/" ++ @tagName(Command.LlmCommand.acceptCookies); + const label: []const u8 = if (cmd == .login) "/login" else "/acceptCookies"; _ = self.runTurn(.{ .prompt = prompt, .record_comment = line, .label = label }); }, .tool_call => |tc| { self.terminal.beginTool(tc.name(), slash_split.?.rest); - const result = self.cmd_runner.executeWithResult(aa, cmd); + const result = self.runCommand(aa, cmd); self.terminal.endTool(); - self.cmd_runner.printResult(cmd, result); + self.printCommandResult(cmd, result); if (self.recorder) |*r| r.record(cmd); }, } @@ -545,7 +541,7 @@ fn printSlashHelp(self: *Agent, target: []const u8) void { } return; } - const tool_schema = Schema.find(Schema.all(), lookup) orelse { + const tool_schema = Schema.findByName(lookup) orelse { self.terminal.printErrorFmt("unknown tool: {s}", .{lookup}); return; }; @@ -584,6 +580,32 @@ fn firstSentence(text: []const u8) []const u8 { const Replacement = script.Replacement; +/// Caller contract: `cmd` must be `.tool_call` — `.comment`, `.login`, and +/// `.acceptCookies` are filtered upstream because they have no tool mapping. +fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tools.ToolResult { + const tc = switch (cmd) { + .tool_call => |t| t, + else => return .{ .text = "internal: command has no tool mapping", .is_error = true }, + }; + return browser_tools.call(arena, self.session, &self.node_registry, tc.name(), tc.args) catch |err| .{ + .text = if (err == error.OutOfMemory) + "out of memory" + else + std.fmt.allocPrint(arena, "{s} failed: {s}", .{ tc.name(), @errorName(err) }) catch "tool failed", + .is_error = true, + }; +} + +/// Data output (extract/eval/markdown/tree/…) → stdout on success; everything +/// else, including failures from those same commands, → stderr. +fn printCommandResult(self: *Agent, cmd: Command, result: browser_tools.ToolResult) void { + if (cmd.producesData() and !result.is_error) { + self.terminal.printAssistant(result.text); + } else { + self.terminal.printActionResult(result.text); + } +} + fn runScript(self: *Agent, path: []const u8) bool { self.terminal.printInfoFmt("Running script: {s}", .{path}); @@ -616,7 +638,7 @@ fn runScript(self: *Agent, path: []const u8) bool { } continue; }, - .login, .accept_cookies => { + .login, .acceptCookies => { if (self.ai_client == null) { self.terminal.printErrorFmt("line {d}: {s} requires --provider", .{ entry.line_num, @@ -677,8 +699,8 @@ fn runActionEntry(self: *Agent, sa: std.mem.Allocator, entry: script.Iterator.En defer cmd_arena.deinit(); const ca = cmd_arena.allocator(); - const result = self.cmd_runner.executeWithResult(ca, entry.command); - self.cmd_runner.printResult(entry.command, result); + const result = self.runCommand(ca, entry.command); + self.printCommandResult(entry.command, result); const verification: Verifier.VerifyResult = if (!result.is_error and self.self_heal) self.verifier.verify(ca, entry.command) @@ -731,10 +753,10 @@ fn retryCommand(self: *Agent, ca: std.mem.Allocator, cmd: Command) bool { for (0..3) |i| { std.Thread.sleep((500 + i * 250) * std.time.ns_per_ms); self.terminal.printInfo("Retrying command..."); - const retry_result = self.cmd_runner.executeWithResult(ca, cmd); + const retry_result = self.runCommand(ca, cmd); if (retry_result.is_error) continue; if (self.verifier.verify(ca, cmd) == .failed) continue; - self.cmd_runner.printResult(cmd, retry_result); + self.printCommandResult(cmd, retry_result); return true; } return false; @@ -1357,7 +1379,7 @@ test "canHeal: only page-local DOM commands are allowed" { } try std.testing.expect(!(Command{ .login = {} }).canHeal()); - try std.testing.expect(!(Command{ .accept_cookies = {} }).canHeal()); + try std.testing.expect(!(Command{ .acceptCookies = {} }).canHeal()); try std.testing.expect(!(Command{ .comment = {} }).canHeal()); } diff --git a/src/agent/CommandRunner.zig b/src/agent/CommandRunner.zig deleted file mode 100644 index 929e2b38..00000000 --- a/src/agent/CommandRunner.zig +++ /dev/null @@ -1,65 +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 browser_tools = lp.tools; -const Command = lp.script.Command; -const CDPNode = @import("../cdp/Node.zig"); -const Terminal = @import("Terminal.zig"); - -const CommandRunner = @This(); - -session: *lp.Session, -node_registry: *CDPNode.Registry, -terminal: *Terminal, - -pub fn init(session: *lp.Session, node_registry: *CDPNode.Registry, terminal: *Terminal) CommandRunner { - return .{ - .session = session, - .node_registry = node_registry, - .terminal = terminal, - }; -} - -/// Caller contract: `cmd` must be `.tool_call` — `.comment`, `.login`, and -/// `.accept_cookies` are filtered upstream (see `Agent.runRepl`) because they -/// have no tool mapping. -pub fn executeWithResult(self: *CommandRunner, arena: std.mem.Allocator, cmd: Command) browser_tools.ToolResult { - const tc = switch (cmd) { - .tool_call => |t| t, - else => return .{ .text = "internal: command has no tool mapping", .is_error = true }, - }; - return browser_tools.call(arena, self.session, self.node_registry, tc.name(), tc.args) catch |err| .{ - .text = if (err == error.OutOfMemory) - "out of memory" - else - std.fmt.allocPrint(arena, "{s} failed: {s}", .{ tc.name(), @errorName(err) }) catch "tool failed", - .is_error = true, - }; -} - -/// Data output (extract/eval/markdown/tree/…) → stdout on success; everything -/// else, including failures from those same commands, → stderr. -pub fn printResult(self: *CommandRunner, cmd: Command, result: browser_tools.ToolResult) void { - if (cmd.producesData() and !result.is_error) { - self.terminal.printAssistant(result.text); - } else { - self.terminal.printActionResult(result.text); - } -} diff --git a/src/agent/Terminal.zig b/src/agent/Terminal.zig index 54485ef2..f5e2f5a9 100644 --- a/src/agent/Terminal.zig +++ b/src/agent/Terminal.zig @@ -66,15 +66,15 @@ stderr_is_tty: bool, spinner: Spinner, // Flat name list for the "match any slash command" search/completion paths. -const all_slash_names: [browser_tools.names.len + SlashCommand.meta_commands.len + Command.llm_commands.len][]const u8 = blk: { - var arr: [browser_tools.names.len + SlashCommand.meta_commands.len + Command.llm_commands.len][]const u8 = undefined; +const all_slash_names: [browser_tools.names.len + SlashCommand.meta_commands.len + Command.llm_tags.len][]const u8 = blk: { + var arr: [browser_tools.names.len + SlashCommand.meta_commands.len + Command.llm_tags.len][]const u8 = undefined; var idx: usize = 0; for (browser_tools.names) |n| { arr[idx] = n; idx += 1; } - for (Command.llm_commands) |l| { - arr[idx] = @tagName(l); + for (Command.llm_tags) |tag| { + arr[idx] = @tagName(tag); idx += 1; } for (SlashCommand.meta_commands) |m| { @@ -342,7 +342,7 @@ fn completionCallback(cenv: ?*c.ic_completion_env_t, prefix: [*c]const u8) callc if (input[0] == '/') { if (has_space) { if (Schema.parseSlashCommand(input)) |parts| { - if (Schema.find(Schema.all(), parts.name)) |schema| { + if (Schema.findByName(parts.name)) |schema| { addPartialKeyCompletions(cenv, input, parts.rest, schema, &buf); } else if (SlashCommand.findMeta(parts.name)) |meta| { addMetaValueCompletions(cenv, input, parts.rest, meta, &buf); @@ -381,7 +381,7 @@ fn hintsCallback(input_c: [*c]const u8, arg: ?*anyopaque) callconv(.c) [*c]const if (Schema.parseSlashCommand(input)) |parts| { const ends_ws = input[input.len - 1] == ' '; - if (Schema.find(Schema.all(), parts.name)) |schema| { + if (Schema.findByName(parts.name)) |schema| { return renderSchemaHint(schema, parts.rest, ends_ws); } if (SlashCommand.findMeta(parts.name)) |meta| { @@ -522,7 +522,7 @@ fn slashHasPrefix(name: []const u8) bool { } fn slashHasParams(name: []const u8) bool { - if (Schema.find(Schema.all(), name)) |s| return s.hints.len > 0; + if (Schema.findByName(name)) |s| return s.hints.len > 0; if (SlashCommand.findMeta(name)) |m| return m.hint.len > 0; return false; } diff --git a/src/script/Iterator.zig b/src/script/Iterator.zig index 2d07bd55..3eac2721 100644 --- a/src/script/Iterator.zig +++ b/src/script/Iterator.zig @@ -106,7 +106,7 @@ const BlockOpener = struct { fn tryBlockOpener(line: []const u8) ?BlockOpener { const split = Schema.parseSlashCommand(line) orelse return null; - const s = Schema.find(Schema.all(), split.name) orelse return null; + const s = Schema.findByName(split.name) orelse return null; if (!s.isMultiLineCapable()) return null; const qt = Schema.QuoteType.fromLiteral(split.rest) orelse return null; return .{ .tool = s.tool, .field = s.required[0], .quote_type = qt }; diff --git a/src/script/Schema.zig b/src/script/Schema.zig index c5e8d25b..ed077690 100644 --- a/src/script/Schema.zig +++ b/src/script/Schema.zig @@ -87,19 +87,19 @@ pub fn isMultiLineCapable(self: Schema) bool { return self.required.len == 1 and self.fieldType(self.required[0]) == .string; } -pub fn findField(self: Schema, key: []const u8) ?FieldEntry { +fn findField(self: Schema, key: []const u8) ?FieldEntry { for (self.fields) |f| { if (std.mem.eql(u8, f.name, key)) return f; } return null; } -pub fn fieldType(self: Schema, key: []const u8) FieldType { +fn fieldType(self: Schema, key: []const u8) FieldType { if (self.findField(key)) |f| return f.field_type; return .other; } -pub fn isFieldDefaultTrue(self: Schema, key: []const u8) bool { +fn isFieldDefaultTrue(self: Schema, key: []const u8) bool { if (self.findField(key)) |f| return f.default_true; return false; } @@ -181,7 +181,7 @@ pub fn parseValue(self: Schema, arena: std.mem.Allocator, rest: []const u8) Pars return try self.buildValue(arena, list.items); } -pub fn validateAndFillObject(self: Schema, obj: *std.json.ObjectMap) ParseError!void { +fn validateAndFillObject(self: Schema, obj: *std.json.ObjectMap) ParseError!void { var it = obj.iterator(); while (it.next()) |entry| { if (self.findField(entry.key_ptr.*) == null) return error.UnknownField; @@ -255,6 +255,10 @@ pub fn find(schemas: []const Schema, name: []const u8) ?*const Schema { return null; } +pub fn findByName(name: []const u8) ?*const Schema { + return find(all(), name); +} + /// Lazy process-wide cache, keyed by `@intFromEnum(BrowserTool)`. /// Panics on init failure — `tool_defs` is comptime-constant, so any /// parse/build error is a build-time bug. diff --git a/src/script/command.zig b/src/script/command.zig index dc3679ab..16ea854b 100644 --- a/src/script/command.zig +++ b/src/script/command.zig @@ -32,24 +32,12 @@ pub const ParseError = Schema.ParseError || error{ pub const Command = union(enum) { tool_call: ToolCall, login: void, - accept_cookies: void, + acceptCookies: void, comment: void, - /// Variant names are the wire-format slash names — `@tagName` is the - /// single source of truth for parse, format, and autocomplete. - pub const LlmCommand = enum { - login, - acceptCookies, - - pub fn toCommand(self: LlmCommand) Command { - return switch (self) { - .login => .{ .login = {} }, - .acceptCookies => .{ .accept_cookies = {} }, - }; - } - }; - - pub const llm_commands = std.enums.values(LlmCommand); + /// Union tags that fire an LLM trigger. Tag names match the wire-format + /// slash command, so `@tagName` is the single source of truth. + pub const llm_tags: []const std.meta.Tag(Command) = &.{ .login, .acceptCookies }; pub const ToolCall = struct { tool: BrowserTool, @@ -59,7 +47,7 @@ pub const Command = union(enum) { return @tagName(self.tool); } - pub fn schema(self: ToolCall) *const Schema { + fn schema(self: ToolCall) *const Schema { return &Schema.all()[@intFromEnum(self.tool)]; } @@ -67,7 +55,7 @@ pub const Command = union(enum) { /// - no `selector` AND (tool needs one OR only locator is the /// ephemeral `backendNodeId`); /// - a string field can't be quoted unambiguously. - pub fn isRecorded(self: ToolCall) bool { + fn isRecorded(self: ToolCall) bool { if (!self.tool.isRecorded()) return false; const s = self.schema(); const args = self.args orelse return s.required.len == 0; @@ -90,7 +78,7 @@ pub const Command = union(enum) { } /// Canonical recorder format. Round-trips with `Command.parse`. - pub fn format(self: ToolCall, writer: *std.Io.Writer) (std.Io.Writer.Error || error{AmbiguousQuoting})!void { + fn format(self: ToolCall, writer: *std.Io.Writer) (std.Io.Writer.Error || error{AmbiguousQuoting})!void { const s = self.schema(); try writer.writeByte('/'); try writer.writeAll(s.tool_name); @@ -127,7 +115,7 @@ pub const Command = union(enum) { pub fn isRecorded(self: Command) bool { return switch (self) { .comment => false, - .login, .accept_cookies => true, + .login, .acceptCookies => true, .tool_call => |tc| tc.isRecorded(), }; } @@ -139,13 +127,6 @@ pub const Command = union(enum) { }; } - pub fn needsLlm(self: Command) bool { - return switch (self) { - .login, .accept_cookies => true, - else => false, - }; - } - pub fn canHeal(self: Command) bool { return switch (self) { .tool_call => |tc| tc.tool.canHeal(), @@ -153,6 +134,12 @@ pub const Command = union(enum) { }; } + pub fn needsLlm(self: Command) bool { + return inline for (llm_tags) |tag| { + if (self == tag) break true; + } else false; + } + pub fn isRetryable(self: Command) bool { return switch (self) { .tool_call => |tc| tc.tool.isRetryable(), @@ -168,13 +155,14 @@ pub const Command = union(enum) { const split = Schema.splitNameRest(trimmed[1..]) orelse return error.MissingName; - for (llm_commands) |lc| { - if (!std.ascii.eqlIgnoreCase(split.name, @tagName(lc))) continue; - if (split.rest.len > 0) return error.MalformedKv; - return lc.toCommand(); + inline for (llm_tags) |tag| { + if (std.ascii.eqlIgnoreCase(split.name, @tagName(tag))) { + if (split.rest.len > 0) return error.MalformedKv; + return @unionInit(Command, @tagName(tag), {}); + } } - const s = Schema.find(Schema.all(), split.name) orelse return error.UnknownTool; + const s = Schema.findByName(split.name) orelse return error.UnknownTool; const args = try s.parseValue(arena, split.rest); return .{ .tool_call = .{ .tool = s.tool, .args = args } }; } @@ -182,8 +170,7 @@ pub const Command = union(enum) { /// Canonical recorder format. Round-trips with `parse`. pub fn format(self: Command, writer: *std.Io.Writer) (std.Io.Writer.Error || error{AmbiguousQuoting})!void { switch (self) { - .login => try writer.writeAll("/" ++ @tagName(LlmCommand.login)), - .accept_cookies => try writer.writeAll("/" ++ @tagName(LlmCommand.acceptCookies)), + inline .login, .acceptCookies => |_, tag| try writer.writeAll("/" ++ @tagName(tag)), .comment => try writer.writeAll("#"), .tool_call => |tc| try tc.format(writer), } @@ -221,7 +208,7 @@ test "parse: /login and /acceptCookies" { var arena: std.heap.ArenaAllocator = .init(testing.allocator); defer arena.deinit(); try testing.expect((try Command.parse(arena.allocator(), "/login")) == .login); - try testing.expect((try Command.parse(arena.allocator(), "/acceptCookies")) == .accept_cookies); + try testing.expect((try Command.parse(arena.allocator(), "/acceptCookies")) == .acceptCookies); } test "parse: /goto positional" { @@ -326,7 +313,7 @@ test "format: /login and /acceptCookies" { var aw2: std.Io.Writer.Allocating = .init(testing.allocator); defer aw2.deinit(); - try (Command{ .accept_cookies = {} }).format(&aw2.writer); + try (Command{ .acceptCookies = {} }).format(&aw2.writer); try testing.expectString("/acceptCookies", aw2.written()); }