real coding agent tools

1 files changed, 154 insertions, 23 deletions

diff --git a/src/lua_runtime.zig b/src/lua_runtime.zig
index 71e4ae7..5efadaa 100644
--- a/src/lua_runtime.zig
+++ b/src/lua_runtime.zig
@@ -67,6 +67,11 @@ pub const LuaRuntime = struct {
     /// tick libuv between coroutine resumes. `0` until
     /// `installScheduler` runs.
     uv_run_ref: c_int = 0,
+    /// Registry ref to `require("luv").loop_alive`, used by the
+    /// scheduler to detect handler coroutines that yielded without
+    /// arming any libuv work (a deadlock we surface rather than hang
+    /// on). `0` until `installScheduler` runs.
+    uv_loop_alive_ref: c_int = 0,
 
     /// Pointer to the in-flight batch, valid only for the duration of
     /// one `invoke_batch` call. The `panto._record_result` C function
@@ -129,6 +134,9 @@ pub const LuaRuntime = struct {
         if (self.uv_run_ref != 0) {
             c.luaL_unref(self.L, lua_bridge.LUA_REGISTRYINDEX, self.uv_run_ref);
         }
+        if (self.uv_loop_alive_ref != 0) {
+            c.luaL_unref(self.L, lua_bridge.LUA_REGISTRYINDEX, self.uv_loop_alive_ref);
+        }
 
         c.lua_close(self.L);
 
@@ -467,7 +475,7 @@ fn runBatch(
     }
 
     while (pending > 0) {
-        const active = try driveUvOnce(self);
+        try driveUvOnce(self);
         // Reap any coroutines that terminated during the tick.
         var reaped: usize = 0;
         for (thread_refs, 0..) |tref, i| {
@@ -504,10 +512,17 @@ fn runBatch(
             }
             continue;
         }
-        if (active == 0) {
+        if (!try loopAlive(self)) {
             // No libuv handles are pending, but we still have alive
             // coroutines. They yielded without arranging to be woken.
             // Mark them as failed and break.
+            //
+            // Note: we ask `uv.loop_alive` rather than relying on the
+            // return value of `uv.run("once")`, which is a boolean
+            // signalling whether `uv.stop()` was called — not a count
+            // of active handles. Conflating the two used to break
+            // multi-tick tools (e.g. `bash`) on their second tick.
+
             for (thread_refs, 0..) |tref, i| {
                 if (tref == 0) continue;
                 slots[i] = .{
@@ -527,9 +542,25 @@ fn runBatch(
     }
 
     // Translate slots into the libpanto-shaped results.
+    //
+    // Important: a handler that raised (`ok == false`) or otherwise
+    // misbehaved (`!recorded`) is surfaced to the model as an `.ok`
+    // result whose body is the formatted error message. We do *not*
+    // return `.err` here, because libpanto treats any per-call `.err`
+    // as an unrecoverable failure that aborts the entire turn (see
+    // `agent.dispatchToolCalls`). Aborting the turn over a Lua-level
+    // bug — in either a builtin tool or a user extension — is far
+    // more disruptive than handing the model a readable error and
+    // letting it correct course on the next turn.
     for (slots, 0..) |slot, i| {
         if (!slot.recorded) {
-            results[i] = .{ .err = RuntimeError.BadHandlerReturn };
+            results[i] = .{
+                .ok = try formatToolError(
+                    allocator,
+                    calls[i].tool_name,
+                    "handler terminated without recording a result",
+                ),
+            };
             continue;
         }
         if (slot.ok) {
@@ -538,9 +569,6 @@ fn runBatch(
             if (slot.err_msg) |m| allocator.free(m);
         } else {
             if (slot.value) |v| allocator.free(v);
-            // The error message was logged for the user; we still
-            // surface a typed error to libpanto so it can route the
-            // failure to the agent's error path.
             std.log.warn(
                 "panto-lua: tool '{s}' failed: {s}",
                 .{
@@ -548,12 +576,33 @@ fn runBatch(
                     slot.err_msg orelse "(no message)",
                 },
             );
+            results[i] = .{
+                .ok = try formatToolError(
+                    allocator,
+                    calls[i].tool_name,
+                    slot.err_msg orelse "(no message)",
+                ),
+            };
             if (slot.err_msg) |m| allocator.free(m);
-            results[i] = .{ .err = RuntimeError.LuaHandlerCrashed };
         }
     }
 }
 
+/// Format a tool-level failure as a textual result the model can read.
+/// The prefix mirrors what the user sees in `panto-lua` log lines so
+/// the model and the developer are looking at the same string.
+fn formatToolError(
+    allocator: Allocator,
+    tool_name: []const u8,
+    message: []const u8,
+) ![]u8 {
+    return std.fmt.allocPrint(
+        allocator,
+        "panto-lua: tool '{s}' failed: {s}",
+        .{ tool_name, message },
+    );
+}
+
 /// Start one coroutine: create a thread under the runtime's lua_State,
 /// push the wrapper closure + (idx, handler, input), `lua_resume` once.
 ///
@@ -592,9 +641,10 @@ fn startCoroutine(
     };
 }
 
-/// Call `uv.run("once")`. Returns the number of active handles luv
-/// reports remain pending (0 means the loop is drained).
-fn driveUvOnce(self: *LuaRuntime) !c_int {
+/// Call `uv.run("once")`. The return value (a boolean meaning "was
+/// `uv.stop()` called with handles still alive?") is not useful to us
+/// — to decide whether to keep ticking we call `loopAlive` separately.
+fn driveUvOnce(self: *LuaRuntime) !void {
     const L = self.L;
     _ = c.lua_rawgeti(L, lua_bridge.LUA_REGISTRYINDEX, @intCast(self.uv_run_ref));
     _ = c.lua_pushlstring(L, "once", 4);
@@ -603,9 +653,25 @@ fn driveUvOnce(self: *LuaRuntime) !c_int {
         c.lua_settop(L, c.lua_gettop(L) - 1);
         return error.UvRunFailed;
     }
-    const n = c.lua_tointegerx(L, -1, null);
+    // Discard the boolean return value.
+    c.lua_settop(L, c.lua_gettop(L) - 1);
+}
+
+/// Call `uv.loop_alive()`. Returns true iff libuv has any referenced
+/// active handles, requests, or closing handles. We use this — not
+/// the return value of `uv.run` — to detect coroutines that yielded
+/// without arranging to be woken.
+fn loopAlive(self: *LuaRuntime) !bool {
+    const L = self.L;
+    _ = c.lua_rawgeti(L, lua_bridge.LUA_REGISTRYINDEX, @intCast(self.uv_loop_alive_ref));
+    if (c.lua_pcallk(L, 0, 1, 0, 0, null) != 0) {
+        logTopAsError(L, "panto-lua: uv.loop_alive failed");
+        c.lua_settop(L, c.lua_gettop(L) - 1);
+        return error.UvRunFailed;
+    }
+    const alive = c.lua_toboolean(L, -1) != 0;
     c.lua_settop(L, c.lua_gettop(L) - 1);
-    return @intCast(n);
+    return alive;
 }
 
 /// Pre-scheduler fallback (used in unit tests and during early
@@ -616,19 +682,47 @@ fn runLegacySync(
     allocator: Allocator,
 ) panto.ToolCallResult {
     const handler_ref = self.handlers.get(call.tool_name) orelse {
-        return .{ .err = RuntimeError.LuaHandlerNotFound };
+        const bytes = formatToolError(
+            allocator,
+            call.tool_name,
+            "unknown tool name",
+        ) catch return .{ .err = error.OutOfMemory };
+        return .{ .ok = bytes };
     };
-    const out_bytes = invokeCoroutineSync(self.L, handler_ref, call.input, allocator) catch |e| {
-        return .{ .err = e };
+    var err_msg: ?[]u8 = null;
+    defer if (err_msg) |m| allocator.free(m);
+    const out_bytes = invokeCoroutineSync(
+        self.L,
+        handler_ref,
+        call.input,
+        allocator,
+        &err_msg,
+    ) catch |e| {
+        // Surface the failure to the model as a textual `.ok` result
+        // rather than aborting the turn. Prefer the Lua-side error
+        // message (captured into `err_msg` by invokeCoroutineSync
+        // when available); fall back to the typed error name.
+        const message: []const u8 = err_msg orelse @errorName(e);
+        const bytes = formatToolError(
+            allocator,
+            call.tool_name,
+            message,
+        ) catch return .{ .err = error.OutOfMemory };
+        return .{ .ok = bytes };
     };
     return .{ .ok = out_bytes };
 }
 
+/// Run a handler synchronously, with no event loop. On Lua-level
+/// failure the error message string is duped into `*err_msg_out`
+/// (caller owns) before returning the typed error. `err_msg_out` is
+/// only written on the error path; on success it is left untouched.
 fn invokeCoroutineSync(
     L: *c.lua_State,
     handler_ref: c_int,
     input: []const u8,
     allocator: Allocator,
+    err_msg_out: *?[]u8,
 ) ![]u8 {
     const co = c.lua_newthread(L) orelse return RuntimeError.LuaInitFailed;
     defer c.lua_settop(L, c.lua_gettop(L) - 1);
@@ -660,6 +754,15 @@ fn invokeCoroutineSync(
             return RuntimeError.LuaHandlerYielded;
         },
         else => {
+            // Capture the Lua error message into the caller's slot
+            // *before* logging+returning, so the legacy sync path can
+            // surface it to the model rather than just the typed
+            // error name.
+            var msg_len: usize = 0;
+            const msg_ptr = c.lua_tolstring(co, -1, &msg_len);
+            if (msg_ptr != null) {
+                err_msg_out.* = allocator.dupe(u8, msg_ptr[0..msg_len]) catch null;
+            }
             logTopAsError(co, "lua: handler crashed");
             return RuntimeError.LuaHandlerCrashed;
         },
@@ -760,27 +863,32 @@ fn installWrapperClosure(self: *LuaRuntime) !void {
     self.wrapper_ref = c.luaL_ref(L, lua_bridge.LUA_REGISTRYINDEX);
 }
 
-/// Cache `require("luv").run` in the registry so the scheduler can
-/// invoke it cheaply per tick.
+/// Cache `require("luv").run` and `require("luv").loop_alive` in the
+/// registry so the scheduler can invoke them cheaply per tick.
 fn cacheUvRun(self: *LuaRuntime) !void {
     const L = self.L;
     const snippet =
-        \\return require("luv").run
+        \\local uv = require("luv")
+        \\return uv.run, uv.loop_alive
     ;
     if (c.luaL_loadstring(L, snippet) != 0) {
         logTopAsError(L, "panto-lua: failed to compile luv lookup");
         c.lua_settop(L, c.lua_gettop(L) - 1);
         return RuntimeError.LuaInitFailed;
     }
-    if (c.lua_pcallk(L, 0, 1, 0, 0, null) != 0) {
+    if (c.lua_pcallk(L, 0, 2, 0, 0, null) != 0) {
         logTopAsError(L, "panto-lua: require('luv') failed (was the bootstrap successful?)");
         c.lua_settop(L, c.lua_gettop(L) - 1);
         return RuntimeError.LuaInitFailed;
     }
-    if (c.lua_type(L, -1) != lua_bridge.T_FUNCTION) {
-        c.lua_settop(L, c.lua_gettop(L) - 1);
+    // Stack: [..., uv.run, uv.loop_alive]. luaL_ref pops the top.
+    if (c.lua_type(L, -1) != lua_bridge.T_FUNCTION or
+        c.lua_type(L, -2) != lua_bridge.T_FUNCTION)
+    {
+        c.lua_settop(L, c.lua_gettop(L) - 2);
         return RuntimeError.LuaInitFailed;
     }
+    self.uv_loop_alive_ref = c.luaL_ref(L, lua_bridge.LUA_REGISTRYINDEX);
     self.uv_run_ref = c.luaL_ref(L, lua_bridge.LUA_REGISTRYINDEX);
 }
 
@@ -1003,7 +1111,17 @@ test "handler crash: per-call error surfaces, sibling calls succeed" {
     };
 
     try testing.expectEqualStrings("fine", results[0].ok);
-    try testing.expectEqual(@as(anyerror, RuntimeError.LuaHandlerCrashed), results[1].err);
+    // A handler crash is surfaced as an *ok* result whose payload is
+    // the formatted error message — not as `.err` — because libpanto
+    // would otherwise abort the entire turn on per-call `.err`. The
+    // payload starts with the well-known `panto-lua: tool '...' failed:`
+    // prefix and includes the Lua-side error message.
+    try testing.expect(std.mem.startsWith(
+        u8,
+        results[1].ok,
+        "panto-lua: tool 'boom' failed:",
+    ));
+    try testing.expect(std.mem.indexOf(u8, results[1].ok, "kaboom") != null);
     try testing.expectEqualStrings("fine", results[2].ok);
 }
 
@@ -1076,8 +1194,21 @@ test "yielding handler with no event loop surfaces LuaHandlerYielded" {
     const calls = [_]panto.ToolCall{.{ .tool_name = "sleeper", .input = "{}" }};
     var results: [1]panto.ToolCallResult = .{.{ .err = error.SourceDroppedCall }};
     try src.vtable.invoke_batch(src.ctx, &calls, &results, testing.allocator);
+    defer for (results) |r| switch (r) {
+        .ok => |b| testing.allocator.free(b),
+        .err => {},
+    };
 
-    try testing.expectEqual(@as(anyerror, RuntimeError.LuaHandlerYielded), results[0].err);
+    // Same policy as the crash test: the failure is surfaced as `.ok`
+    // text so libpanto doesn't abort the turn. The error type name
+    // (`LuaHandlerYielded`) is included via `@errorName` in the legacy
+    // sync path's formatToolError call.
+    try testing.expect(std.mem.startsWith(
+        u8,
+        results[0].ok,
+        "panto-lua: tool 'sleeper' failed:",
+    ));
+    try testing.expect(std.mem.indexOf(u8, results[0].ok, "LuaHandlerYielded") != null);
 }
 
 // Integration test: requires a `$PANTO_HOME` with luv already