From 9b7225b5804ba5ab9a424bf8eeefed0cc9f04a2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 18 Sep 2026 00:44:49 +0200 Subject: [PATCH 1/2] agent: don't drop REPL tool lines or cut result text mid-codepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a REPL whose stderr isn't a tty the spinner is disabled, so `agentToolDone`'s `emitAbove` returned false and the `● [tool: …]` line was discarded. `printToolOutcome` already fell back to a raw stderr write in that case; share that fallback through `emitStderr`. The non-REPL result line sliced `text` at a byte offset, which could split a multi-byte codepoint and emit garbage. Truncate on a UTF-8 boundary instead. --- src/agent/Terminal.zig | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/agent/Terminal.zig b/src/agent/Terminal.zig index e07a89b55..925cd0a99 100644 --- a/src/agent/Terminal.zig +++ b/src/agent/Terminal.zig @@ -18,6 +18,7 @@ const std = @import("std"); const lp = @import("lightpanda"); +const string = @import("../string.zig"); const Config = lp.Config; const Schema = lp.Schema; const SlashCommand = @import("SlashCommand.zig"); @@ -117,7 +118,7 @@ pub fn agentToolDone(self: *Terminal, name: []const u8, args: []const u8, ok: bo const a = if (self.repl_arena) |*ra| ra else return; defer _ = a.reset(.retain_capacity); const bytes = formatBulletLine(a.allocator(), name, args, ok) catch return; - _ = self.spinner.emitAbove(bytes); + self.emitStderr(bytes); return; } if (self.stderr_is_tty) { @@ -131,6 +132,13 @@ pub fn agentToolDone(self: *Terminal, name: []const u8, args: []const u8, ok: bo } } +/// Commit a finished line above the spinner, or straight to stderr when the +/// spinner isn't running (non-tty REPL) so the line isn't silently dropped. +fn emitStderr(self: *Terminal, bytes: []const u8) void { + if (self.spinner.emitAbove(bytes)) return; + _ = std.c.write(std.posix.STDERR_FILENO, bytes.ptr, bytes.len); +} + fn formatBulletLine(arena: std.mem.Allocator, name: []const u8, args: []const u8, ok: bool) ![]const u8 { var aw: std.Io.Writer.Allocating = .init(arena); const w = &aw.writer; @@ -286,12 +294,10 @@ pub fn printToolOutcome(self: *Terminal, name: []const u8, text: []const u8, is_ if (self.repl_arena) |*a| { defer _ = a.reset(.retain_capacity); const bytes = formatReplOutcome(a.allocator(), text, is_error) catch return; - if (self.spinner.emitAbove(bytes)) return; - _ = std.c.write(std.posix.STDERR_FILENO, (bytes).ptr, (bytes).len); - return; + return self.emitStderr(bytes); } if (!is_error and !self.verbosity.atLeast(.medium)) return; - const truncated = text[0..@min(text.len, max_result_display_len)]; + const truncated = string.truncateUtf8(text, max_result_display_len); const ellipsis: []const u8 = if (text.len > max_result_display_len) "..." else ""; const color: []const u8 = if (is_error) ansi.red else ansi.green; std.debug.print("{s}{s}[result: {s}]{s} {s}{s}\n", .{ ansi.dim, color, name, ansi.reset, truncated, ellipsis }); From 290e4f81d94bab3999f51423e136e6a9204fe9f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 18 Sep 2026 00:45:55 +0200 Subject: [PATCH 2/2] agent: simplify REPL command dispatch and provider bookkeeping - `available_providers` holds the static enum tag names; drop the dupe loop, its errdefer and the per-string frees. - `reconcileModel` returns `error.ModelNotAvailable` directly instead of a `use`/`abort` union the caller only mapped to that same error. - `runCommand`/`printCommandResult` take `Command.ToolCall`; the caller already has it, so the unreachable "no tool mapping" branches go. - `handleSave` reuses `rememberSavePath`, which now propagates its allocation failure so a first save under OOM warns instead of unwrapping a null `save_path`. - `SlashCommand.all_names` is a comptime `++` of the three name lists. - `buildUserMessageParts` reads the attachment once for both text and image, and prepends the text part instead of copying the list. - `printSeverity`/`formatBulletLine` use `allocPrint` and the shared `emitStderr`; drop an unreachable `ends_ws` check in `renderMetaHint`. --- src/agent/Agent.zig | 114 ++++++++++++------------------------ src/agent/SlashCommand.zig | 28 ++++----- src/agent/Terminal.zig | 13 +--- src/agent/prompt_assist.zig | 4 +- src/agent/settings.zig | 22 +++---- 5 files changed, 59 insertions(+), 122 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index e4d6b495c..5207605ec 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -194,14 +194,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent var providers_buf: [@typeInfo(Config.AiProvider).@"enum".fields.len]Candidate = undefined; const found_providers = settings.availableProviders(&providers_buf); const available_providers = try allocator.alloc([]const u8, found_providers.len); - var provider_count: usize = 0; - errdefer { - for (available_providers[0..provider_count]) |p| allocator.free(p); - allocator.free(available_providers); - } for (found_providers, 0..) |f, i| { - available_providers[i] = try allocator.dupe(u8, @tagName(f.provider)); - provider_count = i + 1; + available_providers[i] = @tagName(f.provider); } if (opts.task != null and opts.script_file != null) { @@ -282,13 +276,9 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent if (resolved) |*r| if (!will_repl) { const remembered_matches = remembered != null and remembered.?.provider == r.credential.provider; const explicit = opts.model != null or remembered_matches; - switch (try settings.reconcileModel(allocator, &r.credential, model, opts.base_url, explicit)) { - .use => |m| { - allocator.free(model); - model = m; - }, - .abort => return error.ModelNotAvailable, - } + const resolved_model = try settings.reconcileModel(allocator, &r.credential, model, opts.base_url, explicit); + allocator.free(model); + model = resolved_model; }; const effort = settings.resolveEffort(opts, remembered, will_repl, if (resolved) |r| r.credential.provider else null); @@ -373,7 +363,6 @@ pub fn deinit(self: *Agent) void { if (self.ai_client) |ai_client| ai_client.deinit(self.allocator); if (self.credential) |*c| c.deinit(self.allocator); self.allocator.free(self.model); - for (self.available_providers) |p| self.allocator.free(p); self.allocator.free(self.available_providers); self.allocator.destroy(self); } @@ -678,9 +667,9 @@ fn runRepl(self: *Agent) void { }, .tool_call => |tc| { self.terminal.beginTool(tc.name(), slash_split.?.rest); - const result = self.runCommand(aa, cmd); + const result = self.runCommand(aa, tc); self.terminal.endTool(); - self.printCommandResult(cmd, result); + self.printCommandResult(tc, result); if (!result.is_error) { self.recordSaveCommand(navigationGoto(aa, tc.tool, tc.args) orelse cmd); } @@ -1123,33 +1112,18 @@ fn handleSave(self: *Agent, arena: std.mem.Allocator, rest: []const u8) void { self.terminal.printWarning("prompt ignored without an LLM; saving the recorded commands as-is", .{}); } const resolved = self.resolveSavePathAndMode(arena, parsed.filename) orelse return; - const path = resolved.path; - const mode = resolved.mode; - // `path` aliases either an arena-owned string (first save) or - // `self.save_path` (subsequent saves to the same destination); only the - // former needs persisting into agent-owned memory. - var new_save_path: ?[]u8 = if (self.save_path == null) - self.allocator.dupe(u8, path) catch |err| { - self.terminal.printError("failed to remember save destination {s}: {s}", .{ path, @errorName(err) }); - return; - } - else - null; - defer if (new_save_path) |p| self.allocator.free(p); - - save.writeContentFile(path, self.save_buffer.bytes(), mode) catch |err| { - self.terminal.printError("failed to save {s}: {s}", .{ path, @errorName(err) }); + save.writeContentFile(resolved.path, self.save_buffer.bytes(), resolved.mode) catch |err| { + self.terminal.printError("failed to save {s}: {s}", .{ resolved.path, @errorName(err) }); return; }; - if (new_save_path) |p| { - self.save_path = p; - new_save_path = null; - } + self.rememberSavePath(resolved.path) catch |err| { + self.terminal.printWarning("failed to remember save destination {s}: {s}", .{ resolved.path, @errorName(err) }); + }; const saved_lines = self.save_buffer.lines; self.save_buffer.reset(); - self.terminal.printInfo("Saved {d} line(s) to {s}", .{ saved_lines, self.save_path.? }); + self.terminal.printInfo("Saved {d} line(s) to {s}", .{ saved_lines, resolved.path }); } fn promptSaveMode(self: *Agent, path: []const u8) ?save.Mode { @@ -1313,17 +1287,19 @@ fn synthesizeSaveTo(self: *Agent, arena: std.mem.Allocator, path: []const u8, mo return; }; - self.rememberSavePath(path); + self.rememberSavePath(path) catch |err| { + self.terminal.printWarning("failed to remember save destination {s}: {s}", .{ path, @errorName(err) }); + }; self.save_buffer.reset(); self.terminal.printInfo("Saved synthesized script to {s}", .{path}); } /// Persist `path` as the destination reused by a subsequent bare `/save`. -fn rememberSavePath(self: *Agent, path: []const u8) void { +fn rememberSavePath(self: *Agent, path: []const u8) !void { if (self.save_path) |old| { if (std.mem.eql(u8, old, path)) return; } - const dup = self.allocator.dupe(u8, path) catch return; + const dup = try self.allocator.dupe(u8, path); if (self.save_path) |old| self.allocator.free(old); self.save_path = dup; } @@ -1466,13 +1442,7 @@ fn printSlashHelp(self: *Agent, arena: std.mem.Allocator, target: []const u8) vo self.terminal.printInfo("/{s} — {s}", .{ tool_schema.tool_name, tool_schema.description }); } -/// Caller contract: `cmd` must be `.tool_call` — `.comment` and `.llm` are -/// filtered upstream, having 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 }, - }; +fn runCommand(self: *Agent, arena: std.mem.Allocator, tc: Command.ToolCall) browser_tools.ToolResult { // The terminal can't show an image, but the conversation can. return browser_tools.call(arena, self.ts.session, &self.ts.registry, tc.name(), tc.args, .{ .inline_image = self.ai_client != null }) catch |err| .{ .text = switch (err) { @@ -1487,14 +1457,9 @@ fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tool /// Data output (/extract, /evaluate, /markdown, /tree, …) → plain stdout on /// success so a caller can pipe it. Everything else routes through /// `printToolOutcome`, which lays down the green ● / red ● dot shared with the -/// LLM tool-call path. Callers only invoke this for `.tool_call` commands (the -/// comment/login/acceptCookies branches take other paths). -fn printCommandResult(self: *Agent, cmd: Command, result: browser_tools.ToolResult) void { - const tc = switch (cmd) { - .tool_call => |t| t, - else => return, - }; - if (cmd.producesData() and !result.is_error) { +/// LLM tool-call path. +fn printCommandResult(self: *Agent, tc: Command.ToolCall, result: browser_tools.ToolResult) void { + if (tc.tool.producesData() and !result.is_error) { self.printData(tc.tool, result.text); return; } @@ -1858,26 +1823,23 @@ fn buildUserMessageParts( return error.UnsupportedAttachment; }; - if (std.mem.startsWith(u8, mime, "text/")) { - const bytes = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(512 * 1024)) catch |err| { - log.err(.app, "read attachment failed", .{ .path = path, .err = err }); - self.terminal.printError("could not read attachment: {s}", .{path}); - return error.AttachmentReadFailed; - }; + const is_text = std.mem.startsWith(u8, mime, "text/"); + const limit: usize = if (is_text) 512 * 1024 else 20 * 1024 * 1024; + const content = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(limit)) catch |err| { + log.err(.app, "read attachment failed", .{ .path = path, .err = err }); + self.terminal.printError("could not read attachment: {s}", .{path}); + return error.AttachmentReadFailed; + }; + + if (is_text) { try text_prefix.print( ma, "[Attached file: {s}]\n{s}\n[End of attachment]\n\n", - .{ path, bytes }, + .{ path, content }, ); } else { - const raw = std.Io.Dir.cwd().readFileAlloc(lp.io, path, ma, .limited(20 * 1024 * 1024)) catch |err| { - log.err(.app, "read attachment failed", .{ .path = path, .err = err }); - self.terminal.printError("could not read attachment: {s}", .{path}); - return error.AttachmentReadFailed; - }; - const b64_len = std.base64.standard.Encoder.calcSize(raw.len); - const b64 = try ma.alloc(u8, b64_len); - _ = std.base64.standard.Encoder.encode(b64, raw); + const b64 = try ma.alloc(u8, std.base64.standard.Encoder.calcSize(content.len)); + _ = std.base64.standard.Encoder.encode(b64, content); try inline_parts.append(ma, .{ .image = .{ .data = b64, .mime_type = try ma.dupe(u8, mime), @@ -1885,11 +1847,9 @@ fn buildUserMessageParts( } } - var parts: std.ArrayList(zenai.provider.ContentPart) = .empty; try text_prefix.appendSlice(ma, user_input); - try parts.append(ma, .{ .text = try text_prefix.toOwnedSlice(ma) }); - for (inline_parts.items) |p| try parts.append(ma, p); - return parts.toOwnedSlice(ma); + try inline_parts.insert(ma, 0, .{ .text = try text_prefix.toOwnedSlice(ma) }); + return inline_parts.toOwnedSlice(ma); } // Tool results are re-sent with every subsequent turn, so an unscoped read of @@ -2015,9 +1975,7 @@ fn completionProviders(context: *anyopaque, arena: std.mem.Allocator) []const [] if (reachable[i]) extra += 1; } const names = arena.alloc([]const u8, self.available_providers.len + auth.registry.len + 1 + extra) catch return &.{}; - for (self.available_providers, 0..) |p, i| { - names[i] = arena.dupe(u8, p) catch return &.{}; - } + @memcpy(names[0..self.available_providers.len], self.available_providers); var n = self.available_providers.len; // Subscription providers complete even without a stored token — selecting // one is what starts the login. diff --git a/src/agent/SlashCommand.zig b/src/agent/SlashCommand.zig index 20db5aceb..76f3b59cb 100644 --- a/src/agent/SlashCommand.zig +++ b/src/agent/SlashCommand.zig @@ -88,25 +88,21 @@ pub fn findMeta(name: []const u8) ?*const MetaCommand { const browser_tools = lp.tools; const llm_values = std.enums.values(Command.LlmCommand); -/// Every slash-invocable name: browser tools, LLM triggers, meta commands. -pub const all_names: [browser_tools.names.len + meta_commands.len + llm_values.len][]const u8 = blk: { - var arr: [browser_tools.names.len + meta_commands.len + llm_values.len][]const u8 = undefined; - var idx: usize = 0; - for (browser_tools.names) |n| { - arr[idx] = n; - idx += 1; - } - for (llm_values) |lc| { - arr[idx] = @tagName(lc); - idx += 1; - } - for (meta_commands) |m| { - arr[idx] = m.name; - idx += 1; - } +const llm_names = blk: { + var arr: [llm_values.len][]const u8 = undefined; + for (llm_values, &arr) |v, *slot| slot.* = @tagName(v); break :blk arr; }; +const meta_names = blk: { + var arr: [meta_commands.len][]const u8 = undefined; + for (meta_commands, &arr) |m, *slot| slot.* = m.name; + break :blk arr; +}; + +/// Every slash-invocable name: browser tools, LLM triggers, meta commands. +pub const all_names = browser_tools.names ++ llm_names ++ meta_names; + /// Closest command name within two edits, or null — for "did you mean?" on typos. pub fn closestCommand(name: []const u8) ?[]const u8 { return string.closest(name, &all_names, 2); diff --git a/src/agent/Terminal.zig b/src/agent/Terminal.zig index 925cd0a99..6dcd2a295 100644 --- a/src/agent/Terminal.zig +++ b/src/agent/Terminal.zig @@ -140,11 +140,8 @@ fn emitStderr(self: *Terminal, bytes: []const u8) void { } fn formatBulletLine(arena: std.mem.Allocator, name: []const u8, args: []const u8, ok: bool) ![]const u8 { - var aw: std.Io.Writer.Allocating = .init(arena); - const w = &aw.writer; const bullet_color = if (ok) ansi.green else ansi.red; - try w.print(bullet_line_fmt, .{ bullet_color, ansi.reset, ansi.dim, name, ansi.reset, args }); - return aw.written(); + return std.fmt.allocPrint(arena, bullet_line_fmt, .{ bullet_color, ansi.reset, ansi.dim, name, ansi.reset, args }); } pub fn setIdleCallback(fun: ?*const c.ic_idle_fun_t, arg: ?*anyopaque) void { @@ -357,12 +354,8 @@ pub fn printWarning(self: *Terminal, comptime fmt: []const u8, args: anytype) vo fn printSeverity(self: *Terminal, color: []const u8, label: []const u8, comptime fmt: []const u8, args: anytype) void { if (self.repl_arena) |*a| { defer _ = a.reset(.retain_capacity); - var aw: std.Io.Writer.Allocating = .init(a.allocator()); - aw.writer.print("{s}●{s} " ++ fmt ++ "\n", .{ color, ansi.reset } ++ args) catch return; - const bytes = aw.written(); - if (self.spinner.emitAbove(bytes)) return; - _ = std.c.write(std.posix.STDERR_FILENO, (bytes).ptr, (bytes).len); - return; + const bytes = std.fmt.allocPrint(a.allocator(), "{s}●{s} " ++ fmt ++ "\n", .{ color, ansi.reset } ++ args) catch return; + return self.emitStderr(bytes); } std.debug.print("{s}{s}{s}: " ++ fmt ++ "{s}\n", .{ ansi.bold, color, label } ++ args ++ .{ansi.reset}); } diff --git a/src/agent/prompt_assist.zig b/src/agent/prompt_assist.zig index f8cb27fa4..eaa5be3be 100644 --- a/src/agent/prompt_assist.zig +++ b/src/agent/prompt_assist.zig @@ -569,10 +569,8 @@ fn renderMetaHint(state: *State, meta: *const SlashCommand.MetaCommand, body: [] } if (body.len == 0) { - var frags: [1][]const u8 = .{meta.hint}; - return writeHints(if (ends_ws) "" else " ", &frags); + return writeHints(if (ends_ws) "" else " ", &.{meta.hint}); } - if (ends_ws) return null; if (meta.tag == .load or meta.tag == .save) return ghostPathFirstMatch(body); return ghostFirstMatch(meta.values, body, ""); } diff --git a/src/agent/settings.zig b/src/agent/settings.zig index 031335ed1..1e90b714b 100644 --- a/src/agent/settings.zig +++ b/src/agent/settings.zig @@ -336,9 +336,7 @@ pub fn resolveModelName(opts: Config.Agent, resolved: ?ResolvedProvider, remembe if (resolved) |r| { // Use the remembered model whenever it matches the chosen provider, // not only when the provider itself came from the remembered file. - if (remembered) |rem| { - if (rem.provider) |p| if (p == r.credential.provider) return rem.model; - } + if (remembered) |rem| if (rem.provider == r.credential.provider) return rem.model; return zenai.provider.defaultModel(r.credential.provider); } return ""; @@ -378,12 +376,6 @@ pub fn resolveSearchEngine(remembered: ?Remembered) lp.tools.SearchEngine { return .auto; } -const ReconciledModel = union(enum) { - /// Owned by the allocator passed to reconcileModel. - use: []u8, - abort, -}; - /// Validate `desired` against the provider's catalog, mirroring the interactive /// `/model` command. Empty list (unreachable server) leaves it unchecked; an /// explicit unlisted model is fatal. The local servers (Ollama, llama.cpp) have @@ -395,23 +387,23 @@ pub fn reconcileModel( desired: []const u8, base_url: ?[:0]const u8, explicit: bool, -) !ReconciledModel { +) ![]u8 { // A subscription provider can't list models via the provider API; trust the // desired model as-is rather than error against `/models`. - if (auth.descriptorFor(credential.provider) != null) return .{ .use = try allocator.dupe(u8, desired) }; + if (auth.descriptorFor(credential.provider) != null) return try allocator.dupe(u8, desired); var arena: std.heap.ArenaAllocator = .init(allocator); defer arena.deinit(); const ids: []const []const u8 = zenai.provider.listChatModelIds(lp.io, allocator, arena.allocator(), credential.provider, credential.keySlice(), .{ .base_url = base_url, .environ = lp.environ() }) catch &.{}; - if (ids.len == 0 or string.isOneOf(desired, ids)) return .{ .use = try allocator.dupe(u8, desired) }; + if (ids.len == 0 or string.isOneOf(desired, ids)) return try allocator.dupe(u8, desired); if (!explicit) { switch (credential.provider) { .ollama, .llama_cpp => {}, - else => return .{ .use = try allocator.dupe(u8, desired) }, + else => return try allocator.dupe(u8, desired), } std.debug.print("Default {s} model '{s}' is not loaded; using '{s}'.\n", .{ @tagName(credential.provider), desired, ids[0] }); - return .{ .use = try allocator.dupe(u8, ids[0]) }; + return try allocator.dupe(u8, ids[0]); } if (credential.provider == .ollama) { @@ -426,7 +418,7 @@ pub fn reconcileModel( .{ desired, @tagName(credential.provider) }, ); } - return .abort; + return error.ModelNotAvailable; } const testing = @import("../testing.zig");