From d36a51358efbc48de3d5b732727455efc60acae1 Mon Sep 17 00:00:00 2001 From: t Date: Sun, 7 Jun 2026 10:57:49 -0600 Subject: R1: move tool registry off Config onto Agent The tool set is no longer part of the per-turn Config snapshot. Agent now owns a ToolRegistry (created empty at init), populated via the new Agent.registerTool / registerToolSource methods. openStream and OpenStreamFn take the registry as an explicit parameter rather than reading it from cfg.registry, so swapping provider/model between turns (setConfig) no longer disturbs the tool set. CLI migrated to register the Lua tool source onto the agent instead of a locally-owned registry. Test harnesses updated to stage tools on the agent. --- libpanto/src/agent.zig | 149 ++++++++++++++++++++++++++++++------------------- 1 file changed, 93 insertions(+), 56 deletions(-) (limited to 'libpanto/src/agent.zig') diff --git a/libpanto/src/agent.zig b/libpanto/src/agent.zig index 6749ffd..e8be325 100644 --- a/libpanto/src/agent.zig +++ b/libpanto/src/agent.zig @@ -184,10 +184,15 @@ pub const Agent = struct { io: Io, /// The active configuration snapshot, consulted fresh at the top of /// every turn. Immutable while a turn is in flight; swap this pointer - /// (`setConfig`) between turns to change provider/model/base_url and/or - /// the visible tool set atomically. The pointee and its registry are - /// owned by the embedder, not the agent. + /// (`setConfig`) between turns to change provider/model/base_url + /// atomically. The pointee is owned by the embedder, not the agent. The + /// tool set is no longer part of the snapshot — it lives on `registry`. config: *const Config, + /// The tool set this agent exposes. Owned by the agent: created empty at + /// `init`, populated via `registerTool`/`registerToolSource`, torn down + /// in `deinit`. Read fresh by the agent loop each turn, so a + /// registration between turns is visible at the next turn boundary. + registry: ToolRegistry, /// The live conversation the agent drives. Owned by the agent (adopted /// at `init`); torn down in `deinit`. Turn-driving methods operate on /// this directly rather than taking a `*Conversation` parameter. @@ -241,18 +246,31 @@ pub const Agent = struct { .allocator = allocator, .io = io, .config = config, + .registry = ToolRegistry.init(allocator), .conversation = maybe_conversation orelse conversation.Conversation.init(allocator), .session_store = store, }; } pub fn deinit(self: *Agent) void { - // The agent owns the conversation; it borrows the config snapshot, - // the registry, and the session store, which the embedder tears - // down. + // The agent owns the conversation and the tool registry; it borrows + // the config snapshot and the session store, which the embedder + // tears down. + self.registry.deinit(); self.conversation.deinit(); } + /// Add a single tool to this agent's tool set. Visible at the next turn. + pub fn registerTool(self: *Agent, tool: Tool) !void { + try self.registry.register(tool); + } + + /// Add a tool source (a dynamic group of tools) to this agent's tool + /// set. Visible at the next turn. + pub fn registerToolSource(self: *Agent, src: ToolSource) !void { + try self.registry.registerSource(src); + } + /// The provider/model identity used to stamp persisted entries. Uses /// the embedder-supplied display names when set, falling back to the /// active config snapshot's model string (and the `APIStyle` tag name @@ -279,10 +297,7 @@ pub const Agent = struct { self.config = config; } - /// The registry exposed by the active snapshot. - pub fn registry(self: *const Agent) *const ToolRegistry { - return self.config.registry; - } + /// Add a system message (append or replace mode) to the conversation /// and persist it. The persisted entry records the mode so replay @@ -418,7 +433,7 @@ pub const Agent = struct { var attempt: usize = 1; while (true) { var diag: provider_mod.ProviderDiagnostic = .{}; - const ps = self.open_stream_fn(self.allocator, self.io, cfg, &self.conversation, &diag) catch |err| { + const ps = self.open_stream_fn(self.allocator, self.io, cfg, &self.registry, &self.conversation, &diag) catch |err| { if (err == error.ContextOverflow) { return self.handleContextOverflow(cfg, out, err); } @@ -471,7 +486,7 @@ pub const Agent = struct { } }); // Retry the same request against the compacted context. var diag: provider_mod.ProviderDiagnostic = .{}; - return self.open_stream_fn(self.allocator, self.io, cfg, &self.conversation, &diag); + return self.open_stream_fn(self.allocator, self.io, cfg, &self.registry, &self.conversation, &diag); } /// Compute the backoff delay (ms) for the just-failed `attempt` @@ -747,10 +762,9 @@ pub const Agent = struct { if (self.config.compaction.model) |comp_provider| { const cfg: config_mod.Config = .{ .provider = comp_provider, - .registry = &empty_registry, .compaction = self.config.compaction, }; - if (self.runSingleCompactionTurn(&cfg, sys_text, body)) |summary| { + if (self.runSingleCompactionTurn(&cfg, &empty_registry, sys_text, body)) |summary| { return summary; } else |err| { std.log.warn("compaction model failed ({t}); falling back to active model", .{err}); @@ -759,10 +773,9 @@ pub const Agent = struct { const cfg: config_mod.Config = .{ .provider = self.config.provider, - .registry = &empty_registry, .compaction = self.config.compaction, }; - return self.runSingleCompactionTurn(&cfg, sys_text, body); + return self.runSingleCompactionTurn(&cfg, &empty_registry, sys_text, body); } /// A generated compaction summary plus the token size to attribute to @@ -782,6 +795,7 @@ pub const Agent = struct { fn runSingleCompactionTurn( self: *Agent, cfg: *const config_mod.Config, + registry: *const ToolRegistry, system_prompt: []const u8, body: []const u8, ) !CompactionSummary { @@ -797,7 +811,7 @@ pub const Agent = struct { // stream until it commits the assistant message. var queue = stream_mod.EventQueue.init(alloc); defer queue.deinit(); - var ps = try self.open_stream_fn(alloc, self.io, cfg, &conv, null); + var ps = try self.open_stream_fn(alloc, self.io, cfg, registry, &conv, null); defer ps.deinit(); while (true) { const status = try ps.produce(&queue); @@ -855,7 +869,7 @@ pub const Agent = struct { }); continue; } - const entry = self.config.registry.lookup(tu.name) orelse { + const entry = self.registry.lookup(tu.name) orelse { // Unknown tool: don't abort. Synthesize an error result so // the model can correct, and so this ToolUse still gets its // matching ToolResult (providers reject a follow-up request @@ -1572,6 +1586,7 @@ fn stubOpenStream( allocator: Allocator, _: Io, _: *const config_mod.Config, + _: *const ToolRegistry, conv: *conversation.Conversation, diag: ?*provider_mod.ProviderDiagnostic, ) anyerror!provider_mod.ProviderStream { @@ -1596,9 +1611,11 @@ fn stubOpenStream( return StubResponse.create(allocator, conv, turn); } -/// Build a stack registry + active `Config` snapshot wired together, for -/// tests that drive the agent. The caller owns both and must keep them -/// alive for the agent's lifetime. +/// Build a stack registry + active `Config` snapshot for tests that drive +/// the agent. Post-R1 the registry no longer lives on `Config` — it lives +/// on the `Agent`. The harness still owns a registry so tests can pre-stage +/// tools and copy them onto the agent (`seed`) after `init`. The caller +/// owns both and must keep them alive for the agent's lifetime. const TestHarness = struct { registry: ToolRegistry, config: config_mod.Config, @@ -1607,16 +1624,25 @@ const TestHarness = struct { return .{ .registry = ToolRegistry.init(allocator), .config = undefined }; } - /// Finalize the config snapshot to point at this harness's registry. - /// Must be called after `init` and before constructing the agent, once - /// the harness has a stable address. + /// Finalize the config snapshot. Must be called after `init` and before + /// constructing the agent, once the harness has a stable address. fn activate(self: *TestHarness) void { self.config = .{ .provider = .{ .openai_chat = .{ .api_key = "k", .base_url = "u", .model = "m" } }, - .registry = &self.registry, }; } + /// Move the tools pre-staged on the harness registry onto a freshly + /// `init`ed agent's own registry. Post-R1 the agent owns its tool set, + /// so tests stage tools on the harness then transplant them here. The + /// agent's empty registry is freed and replaced; the harness registry + /// is left empty (its `deinit` becomes a no-op free). + fn seedInto(self: *TestHarness, agent: *Agent) void { + agent.registry.deinit(); + agent.registry = self.registry; + self.registry = ToolRegistry.init(self.registry.allocator); + } + fn deinit(self: *TestHarness) void { self.registry.deinit(); } @@ -1856,6 +1882,7 @@ test "agent persists user, assistant, and tool-result messages of a turn" { defer cap.deinit(); var agent = Agent.init(allocator, io, &h.config, cap.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); agent.persist_provider = "openai"; agent.persist_model = "gpt-4o"; @@ -1894,6 +1921,7 @@ test "agent runs a turn against NullStore without persisting or erroring" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); try drainTurn(&agent, "hello"); @@ -2183,6 +2211,7 @@ test "runStep dispatches a tool call and loops to a final text turn" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2233,6 +2262,7 @@ test "runStep dispatches multiple tool calls in parallel" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2272,6 +2302,7 @@ test "runStep: native tool handler error becomes an error result and the model g var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2305,6 +2336,7 @@ test "runStep: unknown tool becomes an error tool result and the loop continues" var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2337,6 +2369,7 @@ test "runStep with no tool calls returns after one provider step" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2363,6 +2396,7 @@ test "runStep surfaces EmptyAssistantResponse when provider commits an empty mes var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); @@ -2393,6 +2427,7 @@ test "runStep delivers all source-backed calls in one batch on one thread" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2400,7 +2435,7 @@ test "runStep delivers all source-backed calls in one batch on one thread" { try drainTurn(&agent, "go"); // Locate the source and inspect its observed batches. - const view = h.registry.lookup("lua_x") orelse return error.NotFound; + const view = agent.registry.lookup("lua_x") orelse return error.NotFound; const src_ptr = view.entry.source.source; const test_src: *TestSource = @ptrCast(@alignCast(src_ptr.ctx)); @@ -2444,13 +2479,14 @@ test "runStep: distinct sources run on distinct threads in parallel" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); try drainTurn(&agent, "go"); - const view_a = h.registry.lookup("src_a_t") orelse return error.NotFound; - const view_b = h.registry.lookup("src_b_t") orelse return error.NotFound; + const view_a = agent.registry.lookup("src_a_t") orelse return error.NotFound; + const view_b = agent.registry.lookup("src_b_t") orelse return error.NotFound; const sa: *TestSource = @ptrCast(@alignCast(view_a.entry.source.source.ctx)); const sb: *TestSource = @ptrCast(@alignCast(view_b.entry.source.source.ctx)); @@ -2481,6 +2517,7 @@ test "runStep: source whole-batch error becomes per-call error results and conti var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2524,6 +2561,7 @@ test "runStep: mixed single Tools and source-backed tools coexist in one turn" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2537,12 +2575,11 @@ test "runStep: mixed single Tools and source-backed tools coexist in one turn" { try testing.expectEqualStrings("src_t2->Z", trText(tr_msg.content.items[2].ToolResult)); } -test "setConfig swaps the visible tool set between turns" { - // The core RCU promise: the agent reads `*const Config` fresh each - // turn, so swapping the pointer mid-conversation changes the tool set - // the next turn sees. Config A exposes only `echo`; config B only - // `late`. After `setConfig(&cfg_b)`, a turn that calls `late` resolves - // — proving both the swap and per-turn re-consultation. +test "setConfig swaps provider between turns; agent tool set persists" { + // Post-R1 the tool set lives on the `Agent`, not on `Config`. Swapping + // the config pointer (`setConfig`) changes provider/model at the next + // turn boundary but leaves the agent's registered tools intact: a turn + // after the swap still resolves a tool registered before it. const allocator = testing.allocator; const scripted = [_]StubProvider.ScriptedTurn{ @@ -2554,40 +2591,26 @@ test "setConfig swaps the visible tool set between turns" { defer threaded.deinit(); const io = threaded.io(); - // Config A: only `echo`. - var reg_a = ToolRegistry.init(allocator); - defer reg_a.deinit(); - try reg_a.register(try EchoTool.create(allocator, "echo", "A:")); const cfg_a: config_mod.Config = .{ - .provider = .{ .openai_chat = .{ .api_key = "k", .base_url = "u", .model = "m" } }, - .registry = ®_a, + .provider = .{ .openai_chat = .{ .api_key = "k", .base_url = "a", .model = "m" } }, }; - - // Config B: only `late`. - var reg_b = ToolRegistry.init(allocator); - defer reg_b.deinit(); - try reg_b.register(try EchoTool.create(allocator, "late", "B:")); const cfg_b: config_mod.Config = .{ - .provider = .{ .openai_chat = .{ .api_key = "k", .base_url = "u", .model = "m" } }, - .registry = ®_b, + .provider = .{ .openai_chat = .{ .api_key = "k", .base_url = "b", .model = "m" } }, }; var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &cfg_a, ns.store(), null); defer agent.deinit(); + try agent.registerTool(try EchoTool.create(allocator, "late", "B:")); agent.open_stream_fn = stub.install(); - // Under A: `echo` visible, `late` not. - try testing.expect(agent.config.registry.lookup("echo") != null); - try testing.expect(agent.config.registry.lookup("late") == null); - - // Swap. Under B: the visibility inverts. + // The tool is visible regardless of which config is active. + try testing.expect(agent.registry.lookup("late") != null); agent.setConfig(&cfg_b); - try testing.expect(agent.config.registry.lookup("echo") == null); - try testing.expect(agent.config.registry.lookup("late") != null); + try testing.expect(agent.registry.lookup("late") != null); - // A real turn under B resolves `late` (which would have been - // UnknownTool under A), then loops to the final text turn. + // A real turn after the swap still resolves `late`, then loops to the + // final text turn. const conv = &agent.conversation; try drainTurn(&agent, "go"); @@ -2617,6 +2640,7 @@ test "compact: summarizes prefix, keeps suffix, system survives" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2679,6 +2703,7 @@ test "compact: restated suffix usage reconstructs a fresh cumulative chain" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2752,6 +2777,7 @@ test "compact: no-op when conversation already fits the budget" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2789,6 +2815,7 @@ test "compact: extra instructions are appended to the system prompt" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2829,6 +2856,7 @@ test "runStep: auto-compacts on context overflow and retries once" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); agent.compaction_system_prompt = "Summarize the conversation."; @@ -2873,6 +2901,7 @@ test "runStep: context overflow without compaction prompt propagates" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); // No compaction_system_prompt set -> overflow propagates. @@ -2944,6 +2973,7 @@ test "runStep: provider 429 retries then succeeds without duplicate messages" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -2987,6 +3017,7 @@ test "runStep: provider 500 retries with backoff notification" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); @@ -3024,6 +3055,7 @@ test "runStep: provider auth failure does not retry" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); @@ -3062,6 +3094,7 @@ test "runStep: retries exhaust and hard-fail after max_attempts" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); @@ -3099,6 +3132,7 @@ test "runStep: Retry-After is honored and reported" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); @@ -3133,6 +3167,7 @@ test "runStep: cancellation from a tool still hard-fails" { var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -3163,6 +3198,7 @@ test "runStep: source per-call error produces a per-call error result and contin var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); const conv = &agent.conversation; @@ -3201,6 +3237,7 @@ test "runStep: context-overflow compaction fires a compaction retry notification var ns = null_store_mod.NullStore.init(allocator); var agent = Agent.init(allocator, io, &h.config, ns.store(), null); defer agent.deinit(); + h.seedInto(&agent); agent.open_stream_fn = stub.install(); agent.compaction_system_prompt = "Summarize the conversation."; -- cgit v1.3