agent: simplify command execution and REPL logic

- Use `std.json.Stringify` for safer JS escaping in `execExtract`.
- Streamline `CommandExecutor` and remove redundant helpers.
- Add "QUIT" as an alias for the "EXIT" command.
- Clean up REPL loop control flow and error reporting in `Agent`.
- Simplify `Recorder` file operations and logging.
This commit is contained in:
Adrià Arrufat
2026-04-11 15:33:06 +02:00
parent b606d28454
commit 3cc652776b
4 changed files with 63 additions and 118 deletions

View File

@@ -80,10 +80,10 @@ self_heal: bool,
interactive: bool,
pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Self {
// Pure replay (positional script, no -i) is the only mode that skips the REPL.
// Pure replay (positional script, no -i) skips the REPL.
const will_repl = opts.interactive or opts.script_file == null;
// --self-heal is meaningless without a provider to heal through.
// --self-heal needs a provider to heal through.
if (opts.self_heal and opts.provider == null) {
log.fatal(.app, "missing --provider", .{
.hint = "--self-heal requires --provider; drop one or add the other",
@@ -91,9 +91,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Self
return error.SelfHealWithoutProvider;
}
// An API key is only needed when we're going to open the REPL *and* a
// provider was selected. Without a provider we run the REPL in "dumb"
// Pandascript-only mode.
// An API key is only required when the REPL will run — pure replay with
// a provider is fine without one since no AI turn ever executes.
const api_key: ?[:0]const u8 = if (opts.provider) |p|
getEnvApiKey(p) orelse if (will_repl) {
log.fatal(.app, "missing API key", .{
@@ -127,12 +126,11 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Self
return error.ToolInitFailed;
};
// Persist REPL history in a cwd-relative `.lp-history`. Skipped in pure
// replay mode (no REPL is opened).
// Persist REPL history in a cwd-relative `.lp-history`; skipped in pure replay.
const history_path: ?[:0]const u8 = if (will_repl) ".lp-history" else null;
// Record REPL commands into the positional script file only when both
// are present — `-i <file>` means "replay then grow this file".
// `-i <file>` means "replay then grow this file"; a script path alone is
// pure replay and must not be mutated.
const recorder_path: ?[]const u8 = if (opts.interactive) opts.script_file else null;
self.* = .{
@@ -173,9 +171,8 @@ pub fn deinit(self: *Self) void {
self.allocator.destroy(self);
}
/// Returns true on success. In interactive mode the REPL always runs after
/// the (optional) replay phase and the function always returns true; in pure
/// replay mode it returns whatever `runScript` returned.
/// Returns true on success. Interactive mode always returns true; pure
/// replay mirrors `runScript`'s result.
pub fn run(self: *Self) bool {
if (self.script_file) |path| {
const script_ok = self.runScript(path);
@@ -194,7 +191,7 @@ fn runRepl(self: *Self) void {
self.terminal.printInfo("Dumb REPL (no --provider) — Pandascript only. Pass --provider for natural-language, LOGIN, and ACCEPT_COOKIES.");
}
while (true) {
repl: while (true) {
const line = self.terminal.readLine("> ") orelse break;
defer self.terminal.freeLine(line);
@@ -202,24 +199,22 @@ fn runRepl(self: *Self) void {
const cmd = Command.parse(line);
if (cmd == .exit or (cmd == .natural_language and std.mem.eql(u8, line, "quit"))) break;
if (cmd == .comment) continue;
if (cmd.needsLlm() and self.ai_client == null) {
self.terminal.printError("This command requires --provider. Pandascript commands (GOTO, CLICK, EXTRACT, ...) work without one.");
continue;
}
switch (cmd) {
.exit, .comment => unreachable, // handled above
.exit => break :repl,
.comment => continue :repl,
.login => self.processUserMessage(login_prompt, line) catch |err| {
self.printAllocError("LOGIN failed: {s}", .{@errorName(err)});
self.terminal.printErrorFmt("LOGIN failed: {s}", .{@errorName(err)});
},
.accept_cookies => self.processUserMessage(accept_cookies_prompt, line) catch |err| {
self.printAllocError("ACCEPT_COOKIES failed: {s}", .{@errorName(err)});
self.terminal.printErrorFmt("ACCEPT_COOKIES failed: {s}", .{@errorName(err)});
},
.natural_language => self.processUserMessage(line, line) catch |err| {
self.printAllocError("Request failed: {s}", .{@errorName(err)});
self.terminal.printErrorFmt("Request failed: {s}", .{@errorName(err)});
},
else => {
self.cmd_executor.execute(cmd);
@@ -231,19 +226,15 @@ fn runRepl(self: *Self) void {
self.terminal.printInfo("Goodbye!");
}
fn printAllocError(self: *Self, comptime fmt: []const u8, args: anytype) void {
self.terminal.printErrorFmt(fmt, args);
}
fn runScript(self: *Self, path: []const u8) bool {
const file = std.fs.cwd().openFile(path, .{}) catch |err| {
self.printAllocError("Failed to open script '{s}': {s}", .{ path, @errorName(err) });
self.terminal.printErrorFmt("Failed to open script '{s}': {s}", .{ path, @errorName(err) });
return false;
};
defer file.close();
const content = file.readToEndAlloc(self.allocator, 10 * 1024 * 1024) catch |err| {
self.printAllocError("Failed to read script: {s}", .{@errorName(err)});
self.terminal.printErrorFmt("Failed to read script: {s}", .{@errorName(err)});
return false;
};
defer self.allocator.free(content);
@@ -269,12 +260,12 @@ fn runScript(self: *Self, path: []const u8) bool {
continue;
},
.natural_language => {
self.printAllocError("line {d}: unrecognized command: {s}", .{ entry.line_num, entry.raw_line });
self.terminal.printErrorFmt("line {d}: unrecognized command: {s}", .{ entry.line_num, entry.raw_line });
return false;
},
.login, .accept_cookies => {
if (self.ai_client == null) {
self.printAllocError("line {d}: {s} requires --provider", .{
self.terminal.printErrorFmt("line {d}: {s} requires --provider", .{
entry.line_num,
entry.raw_line,
});
@@ -282,7 +273,7 @@ fn runScript(self: *Self, path: []const u8) bool {
}
const prompt = if (entry.command == .login) login_prompt else accept_cookies_prompt;
self.processUserMessage(prompt, "") catch |err| {
self.printAllocError("line {d}: {s} failed: {s}", .{
self.terminal.printErrorFmt("line {d}: {s} failed: {s}", .{
entry.line_num,
entry.raw_line,
@errorName(err),
@@ -306,7 +297,7 @@ fn runScript(self: *Self, path: []const u8) bool {
continue;
}
}
self.printAllocError("line {d}: command failed: {s}", .{
self.terminal.printErrorFmt("line {d}: command failed: {s}", .{
entry.line_num,
entry.raw_line,
});
@@ -338,7 +329,7 @@ fn attemptSelfHeal(self: *Self, intent: ?[]const u8, failed_command: []const u8)
var attempt: u8 = 0;
while (attempt < self_heal_max_attempts) : (attempt += 1) {
self.processUserMessage(prompt, "") catch |err| {
self.printAllocError("self-heal attempt {d}/{d} failed: {s}", .{
self.terminal.printErrorFmt("self-heal attempt {d}/{d} failed: {s}", .{
attempt + 1,
self_heal_max_attempts,
@errorName(err),

View File

@@ -213,7 +213,7 @@ pub fn parse(line: []const u8) Command {
return .{ .accept_cookies = {} };
}
if (std.ascii.eqlIgnoreCase(cmd_word, "EXIT")) {
if (std.ascii.eqlIgnoreCase(cmd_word, "EXIT") or std.ascii.eqlIgnoreCase(cmd_word, "QUIT")) {
return .{ .exit = {} };
}
@@ -309,9 +309,9 @@ const QuotedResult = struct {
fn extractQuotedWithRemainder(s: []const u8) ?QuotedResult {
if (s.len < 2) return null;
const quote = s[0];
if (quote != '"' and quote != '\'') return null;
const end = std.mem.indexOfScalarPos(u8, s, 1, quote) orelse return null;
const q = s[0];
if (q != '"' and q != '\'') return null;
const end = std.mem.indexOfScalarPos(u8, s, 1, q) orelse return null;
return .{
.value = s[1..end],
.remainder = s[end + 1 ..],

View File

@@ -26,9 +26,15 @@ pub const ExecResult = struct {
pub fn executeWithResult(self: *Self, a: std.mem.Allocator, cmd: Command.Command) ExecResult {
const Action = browser_tools.Action;
return switch (cmd) {
.goto => |url| self.execGoto(a, url),
.goto => |url| self.callTool(a, @tagName(Action.goto), buildJson(a, .{ .url = substituteEnvVars(a, url) })),
.click => |sel| self.callTool(a, @tagName(Action.click), buildJson(a, .{ .selector = substituteEnvVars(a, sel) })),
.type_cmd => |args| self.execType(a, args),
// execFill in browser/tools.zig substitutes `value` itself so the
// displayed result keeps the `$LP_*` reference instead of leaking
// the resolved secret back to the terminal.
.type_cmd => |args| self.callTool(a, @tagName(Action.fill), buildJson(a, .{
.selector = substituteEnvVars(a, args.selector),
.value = args.value,
})),
.wait => |selector| self.callTool(a, @tagName(Action.waitForSelector), buildJson(a, .{ .selector = selector })),
.scroll => |args| self.callTool(a, @tagName(Action.scroll), buildJson(a, .{ .x = args.x, .y = args.y })),
.hover => |sel| self.callTool(a, @tagName(Action.hover), buildJson(a, .{ .selector = substituteEnvVars(a, sel) })),
@@ -56,9 +62,8 @@ pub fn execute(self: *Self, cmd: Command.Command) void {
self.printResult(cmd, result);
}
/// Route a command's output to stdout (for data-producing commands like
/// EXTRACT/EVAL/MARKDOWN/TREE) or stderr (for action commands like
/// GOTO/CLICK/...) so that shell-redirecting stdout captures only data.
/// Data-producing commands (EXTRACT/EVAL/MARKDOWN/TREE) go to stdout so shell
/// redirection captures only their output; action commands go to stderr.
pub fn printResult(self: *Self, cmd: Command.Command, result: ExecResult) void {
if (cmd.producesData()) {
self.terminal.printAssistant(result.output);
@@ -74,80 +79,34 @@ fn callTool(self: *Self, arena: std.mem.Allocator, tool_name: []const u8, argume
return .{ .output = std.fmt.allocPrint(arena, "{s} failed: {s}", .{ tool_name, @errorName(err) }) catch "tool failed", .failed = true };
}
fn execGoto(self: *Self, arena: std.mem.Allocator, raw_url: []const u8) ExecResult {
const url = substituteEnvVars(arena, raw_url);
return self.callTool(arena, @tagName(browser_tools.Action.goto), buildJson(arena, .{ .url = url }));
}
fn execType(self: *Self, arena: std.mem.Allocator, args: Command.TypeArgs) ExecResult {
const selector = substituteEnvVars(arena, args.selector);
const value = substituteEnvVars(arena, args.value);
return self.callTool(arena, @tagName(browser_tools.Action.fill), buildJson(arena, .{ .selector = selector, .value = value }));
}
fn execExtract(self: *Self, arena: std.mem.Allocator, raw_selector: []const u8) ExecResult {
const selector = escapeJs(arena, substituteEnvVars(arena, raw_selector));
const selector = substituteEnvVars(arena, raw_selector);
const script = std.fmt.allocPrint(arena,
\\JSON.stringify(Array.from(document.querySelectorAll("{s}")).map(el => el.textContent.trim()))
, .{selector}) catch return .{ .output = "failed to build extract script", .failed = true };
// `std.json.Stringify.value` emits a quoted, JS-safe string literal, which
// is also a valid JS string literal — reuse it to splice the selector into
// the querySelectorAll call.
var aw: std.Io.Writer.Allocating = .init(arena);
std.json.Stringify.value(selector, .{}, &aw.writer) catch
return .{ .output = "failed to encode selector", .failed = true };
const encoded = aw.written();
const script = std.fmt.allocPrint(
arena,
"JSON.stringify(Array.from(document.querySelectorAll({s})).map(el => el.textContent.trim()))",
.{encoded},
) catch return .{ .output = "failed to build extract script", .failed = true };
return self.callTool(arena, @tagName(browser_tools.Action.eval), buildJson(arena, .{ .script = script }));
}
const substituteEnvVars = browser_tools.substituteEnvVars;
fn escapeJs(arena: std.mem.Allocator, input: []const u8) []const u8 {
const needs_escape = for (input) |ch| {
if (ch == '"' or ch == '\\' or ch == '\n' or ch == '\r' or ch == '\t') break true;
} else false;
if (!needs_escape) return input;
var out: std.ArrayListUnmanaged(u8) = .empty;
for (input) |ch| {
switch (ch) {
'\\' => out.appendSlice(arena, "\\\\") catch return input,
'"' => out.appendSlice(arena, "\\\"") catch return input,
'\n' => out.appendSlice(arena, "\\n") catch return input,
'\r' => out.appendSlice(arena, "\\r") catch return input,
'\t' => out.appendSlice(arena, "\\t") catch return input,
else => out.append(arena, ch) catch return input,
}
}
return out.toOwnedSlice(arena) catch input;
}
fn buildJson(arena: std.mem.Allocator, value: anytype) []const u8 {
var aw: std.Io.Writer.Allocating = .init(arena);
std.json.Stringify.value(value, .{}, &aw.writer) catch return "{}";
return aw.written();
}
// --- Tests ---
test "escapeJs no escaping needed" {
const result = escapeJs(std.testing.allocator, "hello world");
try std.testing.expectEqualStrings("hello world", result);
}
test "escapeJs quotes and backslashes" {
const result = escapeJs(std.testing.allocator, "say \"hello\\world\"");
defer std.testing.allocator.free(result);
try std.testing.expectEqualStrings("say \\\"hello\\\\world\\\"", result);
}
test "escapeJs newlines and tabs" {
const result = escapeJs(std.testing.allocator, "line1\nline2\ttab");
defer std.testing.allocator.free(result);
try std.testing.expectEqualStrings("line1\\nline2\\ttab", result);
}
test "escapeJs injection attempt" {
const result = escapeJs(std.testing.allocator, "\"; alert(1); //");
defer std.testing.allocator.free(result);
try std.testing.expectEqualStrings("\\\"; alert(1); //", result);
}
test "substituteEnvVars no vars" {
const result = substituteEnvVars(std.testing.allocator, "hello world");
try std.testing.expectEqualStrings("hello world", result);

View File

@@ -1,4 +1,6 @@
const std = @import("std");
const lp = @import("lightpanda");
const log = lp.log;
const Command = @import("Command.zig");
const Self = @This();
@@ -6,17 +8,16 @@ const Self = @This();
file: ?std.fs.File,
needs_separator: bool,
/// Open `path` for append. The file is created if missing; if it already has
/// content, a leading newline is written so the appended block starts on a
/// fresh line. A null path disables recording (no-op).
/// Append-open `path`, inserting a leading newline if the file is non-empty.
/// A null path disables recording.
pub fn init(path: ?[]const u8) Self {
const file: ?std.fs.File = if (path) |p| blk: {
const f = std.fs.cwd().createFile(p, .{ .truncate = false }) catch |err| {
std.debug.print("Warning: could not open recording file: {s}\n", .{@errorName(err)});
log.warn(.app, "could not open recording file", .{ .err = @errorName(err) });
break :blk null;
};
f.seekFromEnd(0) catch |err| {
std.debug.print("Warning: could not seek in recording file: {s}\n", .{@errorName(err)});
log.warn(.app, "could not seek recording file", .{ .err = @errorName(err) });
f.close();
break :blk null;
};
@@ -36,25 +37,19 @@ pub fn record(self: *Self, cmd: Command.Command) void {
const f = self.file orelse return;
if (!cmd.isRecorded()) return;
var buf: [4096]u8 = undefined;
var file_writer = f.writerStreaming(&buf);
const writer = &file_writer.interface;
writer.print("{f}\n", .{cmd}) catch return;
writer.flush() catch return;
var buf: [1024]u8 = undefined;
const line = std.fmt.bufPrint(&buf, "{f}\n", .{cmd}) catch return;
_ = f.write(line) catch return;
self.needs_separator = true;
}
pub fn recordComment(self: *Self, comment: []const u8) void {
const f = self.file orelse return;
var buf: [4096]u8 = undefined;
var file_writer = f.writerStreaming(&buf);
const writer = &file_writer.interface;
if (self.needs_separator) writer.writeByte('\n') catch return;
var buf: [1024]u8 = undefined;
const prefix: []const u8 = if (self.needs_separator) "\n# " else "# ";
const line = std.fmt.bufPrint(&buf, "{s}{s}\n", .{ prefix, comment }) catch return;
_ = f.write(line) catch return;
self.needs_separator = true;
writer.writeAll("# ") catch return;
writer.writeAll(comment) catch return;
writer.writeByte('\n') catch return;
writer.flush() catch return;
}
test "record writes state-mutating commands" {