From 609355b5e4615e4c3c2b1cc076f94a62d3b8e088 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 20 Aug 2026 05:56:39 +0000 Subject: [PATCH] Add bounded output continuation contracts --- evals/run-evals.sh | 35 +- .../.system/files/netclaw-operations/SKILL.md | 8 +- .../make-agent-tools-pit-of-success/tasks.md | 10 +- .../Tools/DispatchingToolExecutorTests.cs | 35 +- ...l_tool_schemas_match_snapshot.verified.txt | 439 ++++++++++++++++++ .../Tools/GeneratedToolSchemaMetaTests.cs | 170 +++++++ .../Tools/ToolOutputSpillTests.cs | 201 +++++++- ...hema_footprint_match_snapshot.verified.txt | 7 +- .../Reminders/SetReminderTool.cs | 5 +- src/Netclaw.Actors/Tools/SkillManageTool.cs | 6 + .../Tools/ToolOutputReadTool.cs | 147 ++++++ src/Netclaw.Actors/Tools/ToolOutputSpill.cs | 48 +- .../Tools/ToolOutputSpillLocation.cs | 110 +++++ .../Tools/ToolRegistrationExtensions.cs | 1 + .../ToolAudienceProfileDefaultsTests.cs | 2 + .../ToolAudienceProfiles.cs | 5 +- .../ToolArgumentVariantAttribute.cs | 24 + .../ToolExecutionContext.cs | 2 +- .../NetclawToolGenerator.cs | 255 +++++++++- 19 files changed, 1416 insertions(+), 94 deletions(-) create mode 100644 src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.Conditional_tool_schemas_match_snapshot.verified.txt create mode 100644 src/Netclaw.Actors/Tools/ToolOutputReadTool.cs create mode 100644 src/Netclaw.Actors/Tools/ToolOutputSpillLocation.cs create mode 100644 src/Netclaw.Tools.Abstractions/ToolArgumentVariantAttribute.cs diff --git a/evals/run-evals.sh b/evals/run-evals.sh index c4e8f38e4..af45680eb 100755 --- a/evals/run-evals.sh +++ b/evals/run-evals.sh @@ -1867,27 +1867,18 @@ assert_complex_diagnose_self() { stdout_contains '\[tool:call\] shell_execute' && stdout_contains 'netclaw.*doctor' } -# bounded-tool-output coverage (bound-tool-output-with-file-spill change). -# These two cases assert on OUTCOME, not mechanism: the prompts state only the -# goal and give the agent NO instructions about spilling, redirecting, re-running, -# file_read, StartLine/Limit, or grep. How the agent handles oversized output must -# come entirely from AGENTS.md, the netclaw-operations skill, and the steer text -# in the tool result — coaching it in the prompt would be testing instruction- -# following, not whether the real guidance surfaces work. +# These cases do not tell the agent how to continue bounded output. +# The guidance and the tool result must supply that behavior. # -# The data is a deterministic Lehmer PRNG (pure integer modular arithmetic, -# identical across awk implementations and the host that computed the expected -# values), so the value at a deep line is reproducible AND un-fabricatable by the -# model. Because the tool bounds any single read to ~N=2000 inline chars, the -# deep-line value is unreachable from one read — so a correct answer can ONLY -# come from the agent paging/reading the oversized output the way the steer asks. -# Outcome therefore implies correct handling; no mechanism assertion is needed. - -# Large SHELL output: ~210 KB on stdout exceeds N, so the daemon spills it and -# steers. Line 200 (value 872671849) sits past the inline window; reporting it -# proves the agent retrieved it from the bounded/spilled output unaided. +# A deterministic Lehmer generator supplies one reproducible value. +# The value is outside the inline output window. +# The assertion checks the continuation tool and the final value. + +# This shell command produces about 210 KB of output. +# Line 200 is outside the inline window. assert_complex_large_shell_output_spill() { stdout_contains '\[tool:call\] shell_execute' && \ + stdout_contains '\[tool:call\] tool_output_read' && \ stdout_response_contains '872671849' } @@ -2782,12 +2773,8 @@ run_all() { run_case complex_diagnose_self "shell_execute with netclaw doctor" \ "Run netclaw doctor and summarize any problems" - # bounded-tool-output: oversized SHELL output. The prompt states only the - # goal — run a command and report a deep line of its output. How to cope with - # the output being too large to return inline (read the spill the steer hands - # back, rather than re-running) must come from the agent's own guidance, not - # this prompt. The number is a deterministic-but-opaque Lehmer PRNG value; the - # assertion checks the agent reports the correct line-200 value (872671849). + # The prompt gives the goal but does not name the continuation tool. + # The assertion requires the structured tool and the exact line value. run_case complex_large_shell_output_spill "retrieves a deep line from oversized shell output unaided" \ "Run this command with shell_execute and tell me the number it prints on line 200: awk 'BEGIN{x=1;for(i=1;i<=20000;i++){x=(x*48271)%2147483647;print x}}'" \ "Using shell_execute, run: awk 'BEGIN{x=1;for(i=1;i<=20000;i++){x=(x*48271)%2147483647;print x}}' — then tell me which number is printed on the 200th line of its output." diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index aead910a8..3ebd508cc 100644 --- a/feeds/skills/.system/files/netclaw-operations/SKILL.md +++ b/feeds/skills/.system/files/netclaw-operations/SKILL.md @@ -3,7 +3,7 @@ name: netclaw-operations description: "REQUIRED when the user asks about scheduling, reminders, cron jobs, timers, background jobs, diagnostics, troubleshooting, MCP tools, daemon health, identity updates, or Netclaw capabilities and self-maintenance." metadata: author: netclaw - version: "2.61.0" + version: "2.62.0" --- # Netclaw Operations @@ -142,9 +142,9 @@ Tool output is bounded to a small inline budget the context window. When a tool's output exceeds that budget you get a head+tail view inline plus a pointer to the full output — not the whole thing: -- **`shell_execute`** spills the full (redacted) output to - `{session}/tool-calls/{toolCallId}.log` and gives you the path. Read a slice with - `file_read` (`StartLine`/`Limit`) or `grep` it — do NOT re-run the command to see more. +- **`shell_execute`** retains the full redacted output inside the current session. + Use `tool_output_read` with the returned `CallId`, `Start`, and `Limit` values. + Do not request a path or rerun the source tool to read more. - **`file_read`** on a large file returns the head and steers you to read a specific range with `StartLine`/`Limit` or `grep` (`StartLine` is a 1-based line number — line 1 is the first line). Don't `cat` a huge file through diff --git a/openspec/changes/make-agent-tools-pit-of-success/tasks.md b/openspec/changes/make-agent-tools-pit-of-success/tasks.md index 37da5596d..c6b7a9ce8 100644 --- a/openspec/changes/make-agent-tools-pit-of-success/tasks.md +++ b/openspec/changes/make-agent-tools-pit-of-success/tasks.md @@ -44,11 +44,11 @@ ## 6. PR 6 - Spill continuation and conditional schemas -- [ ] 6.1 Implement core tool_output_read by opaque call id with bounded windows and current-session-only spill resolution. -- [ ] 6.2 Make spill creation and continuation share one call-id sanitizer and reject traversal, controls, missing ids, and cross-session access. -- [ ] 6.3 Add source-generator support for explicit conditional tool variants and oneOf schemas without changing single-shape schemas. -- [ ] 6.4 Convert the observed mode-dependent first-party tools and reject zero/multiple matching branches before execution. -- [ ] 6.5 Add schema snapshots, generated-code tests, malformed-branch tests, spill-redaction tests, and public API compatibility checks. +- [x] 6.1 Implement core tool_output_read by opaque call id with bounded windows and current-session-only spill resolution. +- [x] 6.2 Make spill creation and continuation share one call-id sanitizer and reject traversal, controls, missing ids, and cross-session access. +- [x] 6.3 Add source-generator support for explicit conditional tool variants and oneOf schemas without changing single-shape schemas. +- [x] 6.4 Convert the observed mode-dependent first-party tools and reject zero/multiple matching branches before execution. +- [x] 6.5 Add schema snapshots, generated-code tests, malformed-branch tests, spill-redaction tests, and public API compatibility checks. ## 7. PR 7 - Replay, documentation, and rollout proof diff --git a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs index 161d0cb64..4a2e382c8 100644 --- a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs @@ -124,9 +124,10 @@ public async Task Verbose_tool_output_over_budget_is_windowed_and_spilled() var result = await _executor.ExecuteAsync(toolCall, context, CancellationToken.None); Assert.True(result.Length < 3000); // windowed inline, not the full 3000 - Assert.Contains("output saved to", result); - Assert.Contains("file_read", result); - var spill = Path.Combine(sessionDir, "tool-calls", "call-spill.log"); + Assert.Contains("tool_output_read", result); + Assert.Contains("CallId='call-spill'", result); + Assert.True(ToolOutputSpillLocation.TryResolve( + sessionDir, "call-spill", out _, out var spill)); Assert.True(File.Exists(spill)); Assert.Contains(new string('x', 100), await File.ReadAllTextAsync(spill, CancellationToken.None)); } @@ -152,8 +153,9 @@ public async Task Spilled_output_is_redacted_before_write() }); var result = await _executor.ExecuteAsync(toolCall, context, CancellationToken.None); - var onDisk = await File.ReadAllTextAsync( - Path.Combine(sessionDir, "tool-calls", "call-redact.log"), CancellationToken.None); + Assert.True(ToolOutputSpillLocation.TryResolve( + sessionDir, "call-redact", out _, out var spillPath)); + var onDisk = await File.ReadAllTextAsync(spillPath, CancellationToken.None); Assert.DoesNotContain("supersecret123", result); Assert.DoesNotContain("supersecret123", onDisk); // redacted before the spill write @@ -313,14 +315,33 @@ public async Task File_read_spill_file_is_redacted_even_when_model_result_is_not // The inline result (model-facing) should NOT contain the redacted sentinel Assert.DoesNotContain("***REDACTED***", result); // But it should be truncated (spilled) - Assert.Contains("output saved to", result); + Assert.Contains("tool_output_read", result); // The spill file on disk SHOULD be redacted - var spillPath = Path.Combine(sessionDir, "tool-calls", "call-spill-secret.log"); + Assert.True(ToolOutputSpillLocation.TryResolve( + sessionDir, "call-spill-secret", out _, out var spillPath)); Assert.True(File.Exists(spillPath)); var spillContent = await File.ReadAllTextAsync(spillPath, CancellationToken.None); Assert.Contains("***REDACTED***", spillContent); Assert.DoesNotContain("real-secret-value", spillContent); + + var continuationContext = TestToolExecutionContext.CreateBound( + "slack/thread-1", + sessionDir, + new TestToolExecutionContextOptions { Audience = TrustAudience.Personal }); + var continuation = await _executor.ExecuteAsync( + CreateToolCall( + "call-continuation", + "tool_output_read", + ToolInput.Create("CallId", "call-spill-secret", "Limit", 256)), + continuationContext, + CancellationToken.None); + Assert.Contains("***REDACTED***", continuation); + Assert.DoesNotContain("real-secret-value", continuation); + Assert.Equal( + ToolInvocationOutcomeCategory.Success, + continuationContext.Invocation.Receipt?.Category); + Assert.Empty(continuationContext.Invocation.Receipt?.FileActivity ?? []); } finally { diff --git a/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.Conditional_tool_schemas_match_snapshot.verified.txt b/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.Conditional_tool_schemas_match_snapshot.verified.txt new file mode 100644 index 000000000..146173e19 --- /dev/null +++ b/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.Conditional_tool_schemas_match_snapshot.verified.txt @@ -0,0 +1,439 @@ +{ + Reminder: { + type: object, + properties: { + Id: { + type: string, + description: Stable identifier for this reminder (kebab-case slug, e.g. 'daily-standup'). If a reminder with this ID exists it will be updated. + }, + Name: { + type: string, + description: A short human-readable title for this reminder. + }, + Prompt: { + type: string, + description: Execution instructions for this reminder. + }, + ScheduleType: { + type: string, + description: Schedule type: 'once', 'interval', or 'cron'. + }, + Schedule: { + type: string, + description: Schedule value: relative time, ISO 8601 datetime, interval duration, or cron expression (optional 'CRON_TZ=' prefix for timezone-aware cron, e.g. 'CRON_TZ=Europe/Brussels 0 9 * * *'). + }, + DeliveryKind: { + type: string, + description: How to deliver results: 'current_session' (reply here), 'channel' (post to a target), or 'none' (silent execution). + }, + DeliveryTransport: { + type: string, + description: Transport for channel delivery (e.g., 'slack' or 'discord'). Required when delivery_kind='channel'. + }, + DeliveryAddress: { + type: string, + description: Target for channel delivery (e.g., '#general', '@user', '<@discordUserId>', channel ID). Required when delivery_kind='channel'. + }, + DeliveryRequired: { + type: boolean, + description: Fail execution if delivery doesn't succeed. Default true. Set false for audit/cleanup tasks. + }, + DeliveryInstructions: { + type: string, + description: Optional guidance for what to include in the delivery to the user. Content guidance only. + }, + Audience: { + type: string, + description: Trust audience for this reminder's execution: 'personal' (all tools including web_search, shell), 'team' (restricted tools), or 'public' (minimal tools). Omit to inherit the creating session/channel audience. + }, + ExpiresIn: { + type: string, + description: Optional expiration for recurring reminders. After this duration, the reminder auto-disables. Use relative time like '24h', '7d'. Recommended for task-specific reminders (e.g., CI checks, deploy monitoring). + }, + _rationale: { + type: string, + description: State your intent for this tool call in one sentence — what are you trying to accomplish and why? + }, + _timeout_seconds: { + type: integer, + description: Requested timeout in seconds. Only set when the default is insufficient. + }, + _background: { + type: boolean, + description: Set to true to run this tool in the background and receive results later. + } + }, + required: [ + Id, + Name, + Prompt, + ScheduleType, + Schedule, + _rationale + ], + additionalProperties: false, + oneOf: [ + { + properties: { + DeliveryKind: { + enum: [ + current_session + ] + } + }, + required: [ + DeliveryKind + ], + not: { + anyOf: [ + { + required: [ + DeliveryTransport + ] + }, + { + required: [ + DeliveryAddress + ] + } + ] + } + }, + { + properties: { + DeliveryKind: { + enum: [ + channel + ] + } + }, + required: [ + DeliveryKind, + DeliveryTransport, + DeliveryAddress + ] + }, + { + properties: { + DeliveryKind: { + enum: [ + none + ] + } + }, + required: [ + DeliveryKind + ], + not: { + anyOf: [ + { + required: [ + DeliveryTransport + ] + }, + { + required: [ + DeliveryAddress + ] + } + ] + } + } + ] + }, + SkillManage: { + type: object, + properties: { + Action: { + type: string, + description: Action to perform: create, edit, patch, delete, write_file, remove_file + }, + Name: { + type: string, + description: Skill name (lowercase letters, numbers, hyphens) + }, + Content: { + type: string, + description: Full SKILL.md content for create/edit actions + }, + FilePath: { + type: string, + description: Relative file path within the skill for write_file/remove_file/patch + }, + FileContent: { + type: string, + description: File content for write_file action + }, + OldString: { + type: string, + description: String to find for patch action + }, + NewString: { + type: string, + description: Replacement string for patch action + }, + ReplaceAll: { + type: boolean, + description: Replace all occurrences for patch action (default: false) + }, + _rationale: { + type: string, + description: State your intent for this tool call in one sentence — what are you trying to accomplish and why? + }, + _timeout_seconds: { + type: integer, + description: Requested timeout in seconds. Only set when the default is insufficient. + }, + _background: { + type: boolean, + description: Set to true to run this tool in the background and receive results later. + } + }, + required: [ + Action, + Name, + _rationale + ], + additionalProperties: false, + oneOf: [ + { + properties: { + Action: { + enum: [ + create + ] + } + }, + required: [ + Action, + Content + ], + not: { + anyOf: [ + { + required: [ + FilePath + ] + }, + { + required: [ + FileContent + ] + }, + { + required: [ + OldString + ] + }, + { + required: [ + NewString + ] + }, + { + required: [ + ReplaceAll + ] + } + ] + } + }, + { + properties: { + Action: { + enum: [ + edit + ] + } + }, + required: [ + Action, + Content + ], + not: { + anyOf: [ + { + required: [ + FilePath + ] + }, + { + required: [ + FileContent + ] + }, + { + required: [ + OldString + ] + }, + { + required: [ + NewString + ] + }, + { + required: [ + ReplaceAll + ] + } + ] + } + }, + { + properties: { + Action: { + enum: [ + patch + ] + } + }, + required: [ + Action, + OldString, + NewString + ], + not: { + anyOf: [ + { + required: [ + Content + ] + }, + { + required: [ + FileContent + ] + } + ] + } + }, + { + properties: { + Action: { + enum: [ + delete + ] + } + }, + required: [ + Action + ], + not: { + anyOf: [ + { + required: [ + Content + ] + }, + { + required: [ + FilePath + ] + }, + { + required: [ + FileContent + ] + }, + { + required: [ + OldString + ] + }, + { + required: [ + NewString + ] + }, + { + required: [ + ReplaceAll + ] + } + ] + } + }, + { + properties: { + Action: { + enum: [ + write_file + ] + } + }, + required: [ + Action, + FilePath, + FileContent + ], + not: { + anyOf: [ + { + required: [ + Content + ] + }, + { + required: [ + OldString + ] + }, + { + required: [ + NewString + ] + }, + { + required: [ + ReplaceAll + ] + } + ] + } + }, + { + properties: { + Action: { + enum: [ + remove_file + ] + } + }, + required: [ + Action, + FilePath + ], + not: { + anyOf: [ + { + required: [ + Content + ] + }, + { + required: [ + FileContent + ] + }, + { + required: [ + OldString + ] + }, + { + required: [ + NewString + ] + }, + { + required: [ + ReplaceAll + ] + } + ] + } + } + ] + } +} \ No newline at end of file diff --git a/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.cs b/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.cs index 42787c4bd..556c3c59b 100644 --- a/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/GeneratedToolSchemaMetaTests.cs @@ -6,6 +6,7 @@ using Netclaw.Tests.Utilities; using System.Text.Json; using Netclaw.Actors.Skills; +using Netclaw.Actors.Reminders; using Netclaw.Actors.Tools; using Netclaw.Configuration; using Netclaw.Security.Skills; @@ -102,6 +103,10 @@ public void Generated_schema_describes_string_arrays_without_scalar_coercion() Assert.Equal("array", paths.GetProperty("type").GetString()); Assert.Equal("string", paths.GetProperty("items").GetProperty("type").GetString()); + Assert.Contains( + "Relative paths use the current project", + paths.GetProperty("description").GetString(), + StringComparison.Ordinal); Assert.Contains( "Paths", tool.ParameterSchema.GetProperty("required").EnumerateArray().Select(item => item.GetString())); @@ -118,6 +123,132 @@ public void GeneratedDictionaryBinderSupportsAllDeclaredMapShapes() Assert.Equal("concrete", parsed.ConcreteMap["kind"]); } + [Fact] + public void Reminder_schema_exposes_closed_delivery_variants() + { + var schema = CreateReminderTool().ParameterSchema; + + Assert.False(schema.GetProperty("additionalProperties").GetBoolean()); + var variants = schema.GetProperty("oneOf").EnumerateArray().ToArray(); + Assert.Equal(3, variants.Length); + Assert.Equal( + ["current_session", "channel", "none"], + variants.Select(static variant => variant + .GetProperty("properties") + .GetProperty("DeliveryKind") + .GetProperty("enum")[0] + .GetString()!)); + + var channelRequired = variants[1] + .GetProperty("required") + .EnumerateArray() + .Select(static value => value.GetString()!) + .ToArray(); + Assert.Equal(["DeliveryKind", "DeliveryTransport", "DeliveryAddress"], channelRequired); + } + + [Fact] + public Task Conditional_tool_schemas_match_snapshot() + { + var document = JsonSerializer.Serialize(new + { + Reminder = CreateReminderTool().ParameterSchema, + SkillManage = CreateSkillManageTool().ParameterSchema + }); + return Verifier.VerifyJson(document); + } + + [Fact] + public void Single_shape_schema_does_not_gain_conditional_keywords() + { + var schema = new FileListTool( + new ToolConfig(), + new NetclawPaths(), + new Netclaw.Security.ToolPathPolicy([])).ParameterSchema; + + Assert.False(schema.TryGetProperty("oneOf", out _)); + Assert.False(schema.TryGetProperty("additionalProperties", out _)); + } + + [Fact] + public void Conditional_schema_and_spill_resolver_seams_are_not_public_api() + { + var attributeType = typeof(NetclawToolAttribute).Assembly.GetType( + "Netclaw.Tools.ToolArgumentVariantAttribute", + throwOnError: true)!; + + Assert.False(attributeType.IsPublic); + Assert.False(typeof(ToolOutputReadTool).IsPublic); + Assert.False(typeof(ToolOutputSpillLocation).IsPublic); + } + + [Theory] + [MemberData(nameof(InvalidReminderArguments))] + public void Reminder_binder_rejects_invalid_delivery_branches(Dictionary arguments) + { + var error = Assert.Throws(() => CreateReminderTool().ParseArguments(arguments)); + + Assert.Contains("exactly one declared variant", error.Message, StringComparison.Ordinal); + Assert.Contains("NOT executed", error.Message, StringComparison.Ordinal); + } + + [Fact] + public void Reminder_binder_accepts_complete_channel_branch() + { + var parsed = CreateReminderTool().ParseArguments(CreateReminderArguments( + "channel", + deliveryTransport: "slack", + deliveryAddress: "channel-token")); + + Assert.Equal("channel", parsed.DeliveryKind); + Assert.Equal("slack", parsed.DeliveryTransport); + Assert.Equal("channel-token", parsed.DeliveryAddress); + } + + [Fact] + public void Reminder_binder_treats_internal_null_optional_fields_as_omitted() + { + var arguments = CreateReminderArguments("none"); + arguments["DeliveryTransport"] = null; + arguments["DeliveryAddress"] = JsonDocument.Parse("null").RootElement.Clone(); + + var parsed = CreateReminderTool().ParseArguments(arguments); + + Assert.Equal("none", parsed.DeliveryKind); + Assert.Null(parsed.DeliveryTransport); + Assert.True(string.IsNullOrEmpty(parsed.DeliveryAddress)); + } + + [Fact] + public void Skill_manage_schema_exposes_each_action_as_one_branch() + { + var tool = CreateSkillManageTool(); + var values = tool.ParameterSchema + .GetProperty("oneOf") + .EnumerateArray() + .Select(static variant => variant + .GetProperty("properties") + .GetProperty("Action") + .GetProperty("enum")[0] + .GetString()!) + .ToArray(); + + Assert.Equal(["create", "edit", "patch", "delete", "write_file", "remove_file"], values); + } + + public static TheoryData> InvalidReminderArguments => new() + { + CreateReminderArguments(null), + CreateReminderArguments("unsupported"), + CreateReminderArguments("channel", deliveryTransport: "slack"), + CreateReminderArguments("current_session", deliveryTransport: "slack"), + CreateReminderArguments("none", deliveryAddress: "channel-token"), + WithConflictingDiscriminator(CreateReminderArguments( + "channel", + deliveryTransport: "slack", + deliveryAddress: "channel-token")) + }; + [Theory] [InlineData("ReadOnlyMap")] [InlineData("InterfaceMap")] @@ -142,6 +273,45 @@ public void GeneratedDictionaryBinderRejectsMissingRequiredMap(string missingPar ["ConcreteMap"] = new Dictionary { ["kind"] = "concrete" }, }; + private static SetReminderTool CreateReminderTool() + => new(null!, TimeProvider.System, new SchedulingConfig()); + + private static SkillManageTool CreateSkillManageTool() + => new( + new SkillRegistry(), + new NetclawPaths(), + new NoOpSkillContentScanner(), + null!); + + private static Dictionary CreateReminderArguments( + string? deliveryKind, + string? deliveryTransport = null, + string? deliveryAddress = null) + { + var arguments = new Dictionary(StringComparer.Ordinal) + { + ["Id"] = "test-reminder", + ["Name"] = "Test reminder", + ["Prompt"] = "Check status.", + ["ScheduleType"] = "once", + ["Schedule"] = "15m" + }; + if (deliveryKind is not null) + arguments["DeliveryKind"] = deliveryKind; + if (deliveryTransport is not null) + arguments["DeliveryTransport"] = deliveryTransport; + if (deliveryAddress is not null) + arguments["DeliveryAddress"] = deliveryAddress; + return arguments; + } + + private static Dictionary WithConflictingDiscriminator( + Dictionary arguments) + { + arguments["delivery_kind"] = "none"; + return arguments; + } + private sealed class UnavailablePromptLoader : IMcpPromptSkillLoader { public ValueTask LoadAsync( diff --git a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs index cc006d5b7..b039f1fca 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs @@ -5,6 +5,7 @@ // ----------------------------------------------------------------------- using Netclaw.Actors.Tools; using Netclaw.Configuration; +using Netclaw.Tests.Utilities; using Netclaw.Tools; using Xunit; @@ -50,14 +51,14 @@ public async Task Over_budget_spills_full_output_and_steers() var result = await ToolOutputSpill.BoundAndSpillAsync( input, "call_2", budget: 100, Context(), CancellationToken.None); - var spillPath = Path.Combine(ToolCallsDir, "call_2.log"); + Assert.True(ToolOutputSpillLocation.TryResolve( + _sessionDir, "call_2", out _, out var spillPath)); Assert.True(File.Exists(spillPath)); Assert.Equal(input, await File.ReadAllTextAsync(spillPath, CancellationToken.None)); // full output on disk Assert.StartsWith(new string('H', 50), result); // inline head - Assert.Contains("output saved to", result); - Assert.Contains(spillPath, result); - Assert.Contains("file_read", result); - Assert.Contains("grep", result); + Assert.Contains("tool_output_read", result); + Assert.Contains("CallId='call_2'", result); + Assert.DoesNotContain(spillPath, result); } [Fact] @@ -85,18 +86,196 @@ public async Task No_session_directory_degrades_to_inline_only() input, "call_5", budget: 100, ctx, CancellationToken.None); Assert.StartsWith(new string('H', 50), result); // inline still produced - Assert.DoesNotContain("saved to", result); // but no spill path + Assert.DoesNotContain("tool_output_read", result); // but no continuation } [Fact] - public async Task Unsafe_call_id_cannot_escape_tool_calls_directory() + public async Task Unsafe_call_id_does_not_create_a_spill() { var input = new string('H', 200) + new string('T', 200); - await ToolOutputSpill.BoundAndSpillAsync( + var result = await ToolOutputSpill.BoundAndSpillAsync( input, "../../evil", budget: 100, Context(), CancellationToken.None); - var written = Directory.GetFiles(ToolCallsDir); - Assert.Single(written); - Assert.StartsWith(ToolCallsDir, Path.GetFullPath(written[0])); + Assert.False(Directory.Exists(ToolCallsDir)); + Assert.DoesNotContain("tool_output_read", result); + } + + [Fact] + public async Task Continuation_reads_only_the_requested_character_window() + { + var input = string.Concat(Enumerable.Range(0, 100).Select(static index => index.ToString("D2"))); + await ToolOutputSpill.BoundAndSpillAsync( + input, "call_window", budget: 20, Context(), CancellationToken.None); + var tool = new ToolOutputReadTool(); + + var result = await tool.ExecuteAsync( + ToolInput.Create("CallId", "call_window", "Start", 20, "Limit", 128), + Context(), + CancellationToken.None); + + Assert.StartsWith(input.Substring(20, 40), result, StringComparison.Ordinal); + Assert.Contains("next_start=", result, StringComparison.Ordinal); + Assert.Contains("complete=false", result, StringComparison.Ordinal); + Assert.True(result.Length <= 128); + } + + [Fact] + public async Task Final_continuation_reports_completion_inside_the_limit() + { + const string content = "short retained result"; + await ToolOutputSpill.BoundAndSpillAsync( + content + new string('x', 100), "call_complete", budget: 5, Context(), CancellationToken.None); + + var result = await new ToolOutputReadTool().ExecuteAsync( + ToolInput.Create("CallId", "call_complete", "Start", content.Length + 100, "Limit", 128), + Context(), + CancellationToken.None); + + Assert.Contains("complete=true", result, StringComparison.Ordinal); + Assert.Contains("next_start=none", result, StringComparison.Ordinal); + Assert.True(result.Length <= 128); + } + + [Theory] + [InlineData("../call")] + [InlineData("call/other")] + [InlineData("call\nother")] + [InlineData("")] + public async Task Continuation_rejects_path_like_or_invalid_call_ids(string callId) + { + var tool = new ToolOutputReadTool(); + + var result = await tool.ExecuteAsync( + ToolInput.Create("CallId", callId), + Context(), + CancellationToken.None); + + Assert.Contains("opaque identifier", result, StringComparison.Ordinal); + } + + [Fact] + public async Task Continuation_cannot_read_another_session_spill() + { + const string callId = "call_private"; + await ToolOutputSpill.BoundAndSpillAsync( + new string('s', 200), callId, budget: 20, Context(), CancellationToken.None); + var otherSession = Path.Combine(Path.GetTempPath(), "nc-spill-other-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(otherSession); + try + { + var otherContext = TestToolExecutionContext.CreateBound( + "session/other", + otherSession, + new TestToolExecutionContextOptions { Audience = TrustAudience.Personal }).Invocation; + + var result = await new ToolOutputReadTool().ExecuteAsync( + ToolInput.Create("CallId", callId), + otherContext, + CancellationToken.None); + + Assert.Contains("No retained output exists", result, StringComparison.Ordinal); + } + finally + { + Directory.Delete(otherSession, recursive: true); + } + } + + [Fact] + public async Task Continuation_returns_not_found_for_a_missing_spill() + { + var result = await new ToolOutputReadTool().ExecuteAsync( + ToolInput.Create("CallId", "call_missing"), + Context(), + CancellationToken.None); + + Assert.Contains("No retained output exists", result, StringComparison.Ordinal); + } + + [Theory] + [InlineData(-1, 128, "Start")] + [InlineData(256001, 128, "Start")] + [InlineData(0, 0, "Limit")] + [InlineData(0, 127, "Limit")] + [InlineData(0, 10001, "Limit")] + public async Task Continuation_rejects_out_of_range_windows(int start, int limit, string parameter) + { + var result = await new ToolOutputReadTool().ExecuteAsync( + ToolInput.Create("CallId", "call_range", "Start", start, "Limit", limit), + Context(), + CancellationToken.None); + + Assert.Contains(parameter, result, StringComparison.Ordinal); + } + + [Fact] + public async Task Opaque_provider_punctuation_round_trips_through_the_hash_name() + { + const string callId = "provider:call.123=value"; + const string content = "retained output"; + await ToolOutputSpill.BoundAndSpillAsync( + content + new string('x', 100), callId, budget: 5, Context(), CancellationToken.None); + + var result = await new ToolOutputReadTool().ExecuteAsync( + ToolInput.Create("CallId", callId, "Limit", 128), + Context(), + CancellationToken.None); + + Assert.StartsWith(content, result, StringComparison.Ordinal); + Assert.Contains("complete=false", result, StringComparison.Ordinal); + } + + [Fact] + public async Task Continuation_rejects_a_symlinked_tool_calls_directory() + { + if (OperatingSystem.IsWindows()) + return; + + var outside = Path.Combine(Path.GetTempPath(), "nc-spill-target-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(outside); + Directory.CreateSymbolicLink(ToolCallsDir, outside); + try + { + var result = await ToolOutputSpill.BoundAndSpillAsync( + new string('x', 100), "call_link", budget: 5, Context(), CancellationToken.None); + + Assert.DoesNotContain("tool_output_read", result); + Assert.Empty(Directory.GetFiles(outside)); + } + finally + { + Directory.Delete(ToolCallsDir); + Directory.Delete(outside, recursive: true); + } + } + + [Fact] + public async Task Spill_rejects_a_symlinked_session_root_before_directory_creation() + { + if (OperatingSystem.IsWindows()) + return; + + var target = Path.Combine(Path.GetTempPath(), "nc-spill-session-target-" + Guid.NewGuid().ToString("N")); + var link = Path.Combine(Path.GetTempPath(), "nc-spill-session-link-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(target); + Directory.CreateSymbolicLink(link, target); + try + { + var context = TestToolExecutionContext.CreateBound( + "session/link", + link, + new TestToolExecutionContextOptions { Audience = TrustAudience.Personal }).Invocation; + + var result = await ToolOutputSpill.BoundAndSpillAsync( + new string('x', 100), "call_linked_session", budget: 5, context, CancellationToken.None); + + Assert.DoesNotContain("tool_output_read", result); + Assert.False(Directory.Exists(Path.Combine(target, "tool-calls"))); + } + finally + { + Directory.Delete(link); + Directory.Delete(target, recursive: true); + } } } diff --git a/src/Netclaw.Actors.Tests/Tools/ToolRegistrationExtensionsTests.Core_tool_names_and_schema_footprint_match_snapshot.verified.txt b/src/Netclaw.Actors.Tests/Tools/ToolRegistrationExtensionsTests.Core_tool_names_and_schema_footprint_match_snapshot.verified.txt index f90996c03..03d4017b0 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolRegistrationExtensionsTests.Core_tool_names_and_schema_footprint_match_snapshot.verified.txt +++ b/src/Netclaw.Actors.Tests/Tools/ToolRegistrationExtensionsTests.Core_tool_names_and_schema_footprint_match_snapshot.verified.txt @@ -12,10 +12,11 @@ set_working_directory, shell_execute, skill_load, - skill_read_resource + skill_read_resource, + tool_output_read ], Footprint: { - Count: 13, - SerializedDefinitionBytes: 13466 + Count: 14, + SerializedDefinitionBytes: 17407 } } \ No newline at end of file diff --git a/src/Netclaw.Actors/Reminders/SetReminderTool.cs b/src/Netclaw.Actors/Reminders/SetReminderTool.cs index 879742551..bdecfc8e2 100644 --- a/src/Netclaw.Actors/Reminders/SetReminderTool.cs +++ b/src/Netclaw.Actors/Reminders/SetReminderTool.cs @@ -24,6 +24,9 @@ namespace Netclaw.Actors.Reminders; "The time zone id must be an IANA identifier without spaces (e.g. 'Europe/Brussels', 'America/New_York'); " + "Windows names like 'Eastern Standard Time' are not supported.", Grant = "scheduling")] +[ToolArgumentVariant("DeliveryKind", "current_session", Forbidden = ["DeliveryTransport", "DeliveryAddress"])] +[ToolArgumentVariant("DeliveryKind", "channel", Required = ["DeliveryTransport", "DeliveryAddress"])] +[ToolArgumentVariant("DeliveryKind", "none", Forbidden = ["DeliveryTransport", "DeliveryAddress"])] public sealed partial class SetReminderTool : NetclawTool { private readonly IActorRef _reminderManager; @@ -42,7 +45,7 @@ public record Params( string ScheduleType, [property: Description("Schedule value: relative time, ISO 8601 datetime, interval duration, or cron expression (optional 'CRON_TZ=' prefix for timezone-aware cron, e.g. 'CRON_TZ=Europe/Brussels 0 9 * * *').")] string Schedule, - [property: Description("How to deliver results: 'current_session' (reply in this conversation), 'channel' (post to a specific target), or 'none' (silent execution). Required unless `delivery.kind` is provided.")] + [property: Description("How to deliver results: 'current_session' (reply here), 'channel' (post to a target), or 'none' (silent execution).")] string? DeliveryKind = null, [property: Description("Transport for channel delivery (e.g., 'slack' or 'discord'). Required when delivery_kind='channel'.")] string? DeliveryTransport = null, diff --git a/src/Netclaw.Actors/Tools/SkillManageTool.cs b/src/Netclaw.Actors/Tools/SkillManageTool.cs index 4b381351a..f45a88373 100644 --- a/src/Netclaw.Actors/Tools/SkillManageTool.cs +++ b/src/Netclaw.Actors/Tools/SkillManageTool.cs @@ -22,6 +22,12 @@ namespace Netclaw.Actors.Tools; "Create, edit, patch, or delete skills and their resource files. " + "Actions: create, edit, patch, delete, write_file, remove_file.", Grant = "builtin")] +[ToolArgumentVariant("Action", "create", Required = ["Content"], Forbidden = ["FilePath", "FileContent", "OldString", "NewString", "ReplaceAll"])] +[ToolArgumentVariant("Action", "edit", Required = ["Content"], Forbidden = ["FilePath", "FileContent", "OldString", "NewString", "ReplaceAll"])] +[ToolArgumentVariant("Action", "patch", Required = ["OldString", "NewString"], Forbidden = ["Content", "FileContent"])] +[ToolArgumentVariant("Action", "delete", Forbidden = ["Content", "FilePath", "FileContent", "OldString", "NewString", "ReplaceAll"])] +[ToolArgumentVariant("Action", "write_file", Required = ["FilePath", "FileContent"], Forbidden = ["Content", "OldString", "NewString", "ReplaceAll"])] +[ToolArgumentVariant("Action", "remove_file", Required = ["FilePath"], Forbidden = ["Content", "FileContent", "OldString", "NewString", "ReplaceAll"])] public sealed partial class SkillManageTool : NetclawTool { [GeneratedRegex(@"^[a-z0-9]([a-z0-9-]*[a-z0-9])?$")] diff --git a/src/Netclaw.Actors/Tools/ToolOutputReadTool.cs b/src/Netclaw.Actors/Tools/ToolOutputReadTool.cs new file mode 100644 index 000000000..7dfb5b19e --- /dev/null +++ b/src/Netclaw.Actors/Tools/ToolOutputReadTool.cs @@ -0,0 +1,147 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.ComponentModel; +using System.Text; +using Netclaw.Tools; + +namespace Netclaw.Actors.Tools; + +[NetclawTool(ToolName, + "Continue a truncated tool result from this session by opaque call id. Returns at most Limit characters from Start and never accepts a file path.", + Grant = "builtin")] +internal sealed partial class ToolOutputReadTool : NetclawTool +{ + public const string ToolName = "tool_output_read"; + internal const int DefaultLimit = 8_000; + internal const int MinimumLimit = 128; + internal const int MaximumLimit = 10_000; + internal const int MaximumStart = 256_000; + private static readonly Encoding StrictUtf8 = + new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true); + + public record Params( + [property: Description("Opaque call id shown by the truncated tool result. Never pass a file path.")] + string CallId, + [property: Description("Zero-based character offset (default 0, maximum 256000).")] int? Start = null, + [property: Description("Maximum total characters returned, including continuation metadata (default 8000, minimum 128, maximum 10000).")] int? Limit = null); + + protected override async Task ExecuteAsync( + Params args, + ToolInvocationContext context, + CancellationToken ct) + { + if (!ToolOutputSpillLocation.TryResolve( + context.SessionDirectory, + args.CallId, + out _, + out var path)) + { + return context.InvalidInput( + "Error: CallId must be an opaque identifier from a truncated tool result in this session."); + } + + var start = args.Start ?? 0; + if (start is < 0 or > MaximumStart) + return context.InvalidInput($"Error: Start must be between 0 and {MaximumStart}."); + + var limit = args.Limit ?? DefaultLimit; + if (limit is < MinimumLimit or > MaximumLimit) + return context.InvalidInput($"Error: Limit must be between {MinimumLimit} and {MaximumLimit}."); + + if (!ToolOutputSpillLocation.IsSafeForIo(context.SessionDirectory!, path)) + return context.AccessDenied("Error: Retained output path is not safe to access."); + + if (!File.Exists(path)) + { + return context.NotFound( + "Error: No retained output exists for that call in this session. Re-run the source tool with narrower output bounds."); + } + + try + { + await using var stream = new FileStream( + path, + FileMode.Open, + FileAccess.Read, + FileShare.Read, + bufferSize: 4096, + FileOptions.Asynchronous | FileOptions.SequentialScan); + using var reader = new StreamReader( + stream, + StrictUtf8, + detectEncodingFromByteOrderMarks: false, + bufferSize: 4096, + leaveOpen: false); + + var remaining = start; + var buffer = new char[Math.Min(4096, Math.Max(limit, 1))]; + while (remaining > 0) + { + ct.ThrowIfCancellationRequested(); + var read = await reader.ReadAsync( + buffer.AsMemory(0, Math.Min(buffer.Length, remaining)), + ct); + if (read == 0) + return context.InvalidInput("Error: Start exceeds the retained output length."); + remaining -= read; + } + + var captured = new StringBuilder(limit + 1); + while (captured.Length <= limit) + { + ct.ThrowIfCancellationRequested(); + var read = await reader.ReadAsync( + buffer.AsMemory(0, Math.Min(buffer.Length, limit + 1 - captured.Length)), + ct); + if (read == 0) + break; + captured.Append(buffer, 0, read); + } + + return context.Success(ComposeWindow(captured, start, limit)); + } + catch (DecoderFallbackException) + { + return context.InvalidInput("Error: Retained output is not valid UTF-8."); + } + catch (UnauthorizedAccessException) + { + return context.AccessDenied("Error: Retained output is not accessible."); + } + catch (FileNotFoundException) + { + return context.NotFound("Error: Retained output no longer exists. Re-run the source tool with narrower output bounds."); + } + catch (DirectoryNotFoundException) + { + return context.NotFound("Error: Retained output no longer exists. Re-run the source tool with narrower output bounds."); + } + catch (IOException ex) + { + return context.TransientFailure($"Error reading retained output: {ex.Message}"); + } + } + + private static string ComposeWindow(StringBuilder captured, int start, int limit) + { + var reachedEnd = captured.Length <= limit; + var contentLength = Math.Min(captured.Length, limit); + string metadata; + + while (true) + { + var complete = reachedEnd && contentLength == captured.Length; + var next = complete ? "none" : (start + contentLength).ToString(System.Globalization.CultureInfo.InvariantCulture); + metadata = $"\n[range start={start} end={start + contentLength}; next_start={next}; complete={complete.ToString().ToLowerInvariant()}]"; + var boundedContentLength = Math.Min(captured.Length, limit - metadata.Length); + if (boundedContentLength >= contentLength) + break; + contentLength = boundedContentLength; + } + + return captured.ToString(0, contentLength) + metadata; + } +} diff --git a/src/Netclaw.Actors/Tools/ToolOutputSpill.cs b/src/Netclaw.Actors/Tools/ToolOutputSpill.cs index fedc77265..3b79a5c37 100644 --- a/src/Netclaw.Actors/Tools/ToolOutputSpill.cs +++ b/src/Netclaw.Actors/Tools/ToolOutputSpill.cs @@ -12,9 +12,8 @@ namespace Netclaw.Actors.Tools; /// /// Bounds a tool result to the inline budget N /// () and, when it -/// exceeds N, spills the full result to -/// {SessionDirectory}/tool-calls/{toolCallId}.log and steers the model to -/// read a slice (file_read offset/limit) or grep it instead of re-running. +/// exceeds N, spills the full result to the current session and steers +/// the model to continue through tool_output_read by opaque call id. /// /// /// Called from DispatchingToolExecutor for every tool, right after @@ -27,8 +26,6 @@ namespace Netclaw.Actors.Tools; /// internal static class ToolOutputSpill { - private const string ToolCallsSubdirectory = "tool-calls"; - // Content budget used when neither the tool nor the context supplies one // (sub-agent / Empty / direct construction). Matches // SessionTuning.MaxInlineToolResultChars's default so un-plumbed paths bound the @@ -87,13 +84,24 @@ public static async Task BoundAndSpillAsync( _ = ct; try { - var dir = Path.Combine(context.SessionDirectory, ToolCallsSubdirectory); - Directory.CreateDirectory(dir); - // Sanitize the (provider-supplied) call id so a spill can never escape - // the tool-calls directory. - var path = Path.Combine(dir, SafeFileName(toolCallId!) + ".log"); + if (!ToolOutputSpillLocation.TryResolve( + context.SessionDirectory, + toolCallId, + out var directory, + out var path)) + { + return null; + } + + if (!ToolOutputSpillLocation.IsSafeForIo(context.SessionDirectory!, path)) + return null; + + Directory.CreateDirectory(directory); + if (!ToolOutputSpillLocation.IsSafeForIo(context.SessionDirectory!, path)) + return null; + await File.WriteAllTextAsync(path, redacted, CancellationToken.None); - return path; + return toolCallId; } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException @@ -105,23 +113,15 @@ or NotSupportedException } } - private static string Compose(string inline, string? spillPath, int fullLength, int budget) + private static string Compose(string inline, string? spillCallId, int fullLength, int budget) { var sb = new StringBuilder(inline); sb.Append($"\n\n[output truncated to {budget} chars of {fullLength}"); - if (spillPath is not null) - sb.Append($"; output saved to {spillPath} — read a slice with file_read (offset/limit) or grep it instead of re-running"); + if (spillCallId is not null) + { + sb.Append($"; continue with tool_output_read using CallId='{spillCallId}' and a bounded Start/Limit window instead of re-running"); + } sb.Append(']'); return sb.ToString(); } - - private static string SafeFileName(string id) - { - var invalid = Path.GetInvalidFileNameChars(); - Span buffer = id.Length is > 0 and <= 256 ? stackalloc char[id.Length] : new char[id.Length]; - for (var i = 0; i < id.Length; i++) - buffer[i] = invalid.Contains(id[i]) ? '_' : id[i]; - var safe = new string(buffer); - return string.IsNullOrWhiteSpace(safe) ? "tool-call" : safe; - } } diff --git a/src/Netclaw.Actors/Tools/ToolOutputSpillLocation.cs b/src/Netclaw.Actors/Tools/ToolOutputSpillLocation.cs new file mode 100644 index 000000000..a194889be --- /dev/null +++ b/src/Netclaw.Actors/Tools/ToolOutputSpillLocation.cs @@ -0,0 +1,110 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Security.Cryptography; +using System.Text; +using Netclaw.Security; + +namespace Netclaw.Actors.Tools; + +/// +/// Resolves one opaque tool call id inside one immutable session directory. +/// +internal static class ToolOutputSpillLocation +{ + internal const int MaximumCallIdLength = 200; + private const string ToolCallsSubdirectory = "tool-calls"; + + public static bool TryResolve( + string? sessionDirectory, + string? callId, + out string directory, + out string path) + { + directory = string.Empty; + path = string.Empty; + + if (!IsValidSessionDirectory(sessionDirectory) || !IsValidCallId(callId)) + return false; + + try + { + directory = Path.GetFullPath(Path.Combine(sessionDirectory!, ToolCallsSubdirectory)); + var fileName = Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(callId!))) + ".log"; + path = Path.GetFullPath(Path.Combine(directory, fileName)); + return string.Equals(Path.GetDirectoryName(path), directory, PathComparison()); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + directory = string.Empty; + path = string.Empty; + return false; + } + } + + internal static bool IsValidCallId(string? callId) + { + if (string.IsNullOrWhiteSpace(callId) || callId.Length > MaximumCallIdLength) + return false; + + if (callId is "." or "..") + return false; + + foreach (var value in callId) + { + if (char.IsControl(value) + || char.IsWhiteSpace(value) + || value is '/' or '\\') + return false; + } + + return true; + } + + private static bool IsValidSessionDirectory(string? sessionDirectory) + { + if (string.IsNullOrWhiteSpace(sessionDirectory) + || sessionDirectory.Any(char.IsControl) + || !Path.IsPathFullyQualified(sessionDirectory) + || !Directory.Exists(sessionDirectory)) + { + return false; + } + + try + { + return string.Equals( + sessionDirectory, + Path.GetFullPath(sessionDirectory), + PathComparison()); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return false; + } + } + + private static StringComparison PathComparison() + => OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + + public static bool IsSafeForIo(string sessionDirectory, string path) + { + try + { + if ((File.GetAttributes(sessionDirectory) & FileAttributes.ReparsePoint) != 0) + return false; + + return !PathUtility.ContainsSymlinkSegment(sessionDirectory, path); + } + catch (Exception ex) when (ex is IOException + or UnauthorizedAccessException + or System.Security.SecurityException) + { + return false; + } + } +} diff --git a/src/Netclaw.Actors/Tools/ToolRegistrationExtensions.cs b/src/Netclaw.Actors/Tools/ToolRegistrationExtensions.cs index 806a71a0c..1eac8e748 100644 --- a/src/Netclaw.Actors/Tools/ToolRegistrationExtensions.cs +++ b/src/Netclaw.Actors/Tools/ToolRegistrationExtensions.cs @@ -43,6 +43,7 @@ public static ToolRegistry WithFirstPartyTools( registry.RegisterCore(new FileSearchTool(config, paths, pathPolicy)); registry.RegisterCore(new FileReadManyTool(config, paths, pathPolicy)); registry.RegisterCore(new JsonReadTool(config, paths, pathPolicy)); + registry.RegisterCore(new ToolOutputReadTool()); registry.RegisterCore(new FileWriteTool(config, paths, pathPolicy)); registry.RegisterCore(new FileEditTool(config, paths, pathPolicy)); registry.Register(new AttachFileTool(config, paths, pathPolicy)); diff --git a/src/Netclaw.Configuration.Tests/ToolAudienceProfileDefaultsTests.cs b/src/Netclaw.Configuration.Tests/ToolAudienceProfileDefaultsTests.cs index 61d03a7eb..19578f978 100644 --- a/src/Netclaw.Configuration.Tests/ToolAudienceProfileDefaultsTests.cs +++ b/src/Netclaw.Configuration.Tests/ToolAudienceProfileDefaultsTests.cs @@ -24,6 +24,7 @@ public sealed class ToolAudienceProfileDefaultsTests ToolAudienceProfileToolCatalog.FileSearch, ToolAudienceProfileToolCatalog.FileReadMany, ToolAudienceProfileToolCatalog.JsonRead, + ToolAudienceProfileToolCatalog.ToolOutputRead, ToolAudienceProfileToolCatalog.AttachFile ] }, @@ -35,6 +36,7 @@ public sealed class ToolAudienceProfileDefaultsTests ToolAudienceProfileToolCatalog.FileSearch, ToolAudienceProfileToolCatalog.FileReadMany, ToolAudienceProfileToolCatalog.JsonRead, + ToolAudienceProfileToolCatalog.ToolOutputRead, ToolAudienceProfileToolCatalog.FileWrite, ToolAudienceProfileToolCatalog.FileEdit, ToolAudienceProfileToolCatalog.AttachFile, diff --git a/src/Netclaw.Configuration/ToolAudienceProfiles.cs b/src/Netclaw.Configuration/ToolAudienceProfiles.cs index 1805dd1b5..7a2f1dc1b 100644 --- a/src/Netclaw.Configuration/ToolAudienceProfiles.cs +++ b/src/Netclaw.Configuration/ToolAudienceProfiles.cs @@ -135,6 +135,7 @@ public static class ToolAudienceProfileToolCatalog public const string FileSearch = "file_search"; public const string FileReadMany = "file_read_many"; public const string JsonRead = "json_read"; + public const string ToolOutputRead = "tool_output_read"; public const string AttachFile = "attach_file"; public const string FileWrite = "file_write"; public const string FileEdit = "file_edit"; @@ -151,7 +152,7 @@ public static class ToolAudienceProfileToolCatalog public const string SetWorkingDirectory = "set_working_directory"; public static IReadOnlyList FileTools { get; } = - [FileRead, FileList, FileSearch, FileReadMany, JsonRead, FileWrite, FileEdit, AttachFile]; + [FileRead, FileList, FileSearch, FileReadMany, JsonRead, ToolOutputRead, FileWrite, FileEdit, AttachFile]; public static IReadOnlyList WebTools { get; } = [WebSearch, WebFetch]; public static IReadOnlyList SkillTools { get; } = [SkillManage]; public static IReadOnlyList WebhookTools { get; } = [SetWebhook, ListWebhooks, DeleteWebhook]; @@ -159,7 +160,7 @@ public static class ToolAudienceProfileToolCatalog public static IReadOnlyList WorkingDirectoryTools { get; } = [SetWorkingDirectory]; public static IReadOnlyList PublicDefaultAllowedTools { get; } = - [FileRead, FileList, FileSearch, FileReadMany, JsonRead, AttachFile]; + [FileRead, FileList, FileSearch, FileReadMany, JsonRead, ToolOutputRead, AttachFile]; public static IReadOnlyList TeamDefaultAllowedTools { get; } = [ diff --git a/src/Netclaw.Tools.Abstractions/ToolArgumentVariantAttribute.cs b/src/Netclaw.Tools.Abstractions/ToolArgumentVariantAttribute.cs new file mode 100644 index 000000000..8a5fb2fc4 --- /dev/null +++ b/src/Netclaw.Tools.Abstractions/ToolArgumentVariantAttribute.cs @@ -0,0 +1,24 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +namespace Netclaw.Tools; + +/// +/// Declares one closed argument branch for a generated first-party tool. +/// +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = false)] +internal sealed class ToolArgumentVariantAttribute : Attribute +{ + public ToolArgumentVariantAttribute(string discriminatorParameter, string discriminatorValue) + { + DiscriminatorParameter = discriminatorParameter; + DiscriminatorValue = discriminatorValue; + } + + public string DiscriminatorParameter { get; } + public string DiscriminatorValue { get; } + public string[] Required { get; set; } = []; + public string[] Forbidden { get; set; } = []; +} diff --git a/src/Netclaw.Tools.Abstractions/ToolExecutionContext.cs b/src/Netclaw.Tools.Abstractions/ToolExecutionContext.cs index 59e234426..1ecc205d2 100644 --- a/src/Netclaw.Tools.Abstractions/ToolExecutionContext.cs +++ b/src/Netclaw.Tools.Abstractions/ToolExecutionContext.cs @@ -533,7 +533,7 @@ public ChannelDeliveryTargetInfo? EffectiveDeliveryTarget /// The session content inline budget /// (SessionTuning.MaxInlineToolResultChars), surfaced here so /// DispatchingToolExecutor can bound a tool result and spill the - /// overflow to {SessionDirectory}/tool-calls/{callId}.log. The dispatcher + /// overflow inside the current session for opaque call-id continuation. The dispatcher /// uses a tool's own InlineOutputBudgetChars override when set (verbose /// tools), else this content budget. Zero when unset (the dispatcher falls back /// to its built-in content default). diff --git a/src/Netclaw.Tools.Generators/NetclawToolGenerator.cs b/src/Netclaw.Tools.Generators/NetclawToolGenerator.cs index a6b971358..21fd3b62b 100644 --- a/src/Netclaw.Tools.Generators/NetclawToolGenerator.cs +++ b/src/Netclaw.Tools.Generators/NetclawToolGenerator.cs @@ -18,6 +18,7 @@ namespace Netclaw.Tools.Generators; public sealed class NetclawToolGenerator : IIncrementalGenerator { private const string AttributeFullName = "Netclaw.Tools.NetclawToolAttribute"; + private const string VariantAttributeFullName = "Netclaw.Tools.ToolArgumentVariantAttribute"; private const string BaseClassPrefix = "Netclaw.Tools.NetclawTool<"; public void Initialize(IncrementalGeneratorInitializationContext context) @@ -85,16 +86,14 @@ public void Initialize(IncrementalGeneratorInitializationContext context) { ct.ThrowIfCancellationRequested(); - var paramDescription = ""; - foreach (var paramAttr in param.GetAttributes()) + var paramDescription = ReadDescription(param.GetAttributes()); + if (paramDescription.Length == 0) { - if (paramAttr.AttributeClass?.Name == "DescriptionAttribute" && - paramAttr.ConstructorArguments.Length > 0 && - paramAttr.ConstructorArguments[0].Value is string desc) - { - paramDescription = desc; - break; - } + var property = paramsType.GetMembers(param.Name) + .OfType() + .FirstOrDefault(); + if (property is not null) + paramDescription = ReadDescription(property.GetAttributes()); } var isNullable = param.Type.NullableAnnotation == NullableAnnotation.Annotated; @@ -116,6 +115,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) ? null : classSymbol.ContainingNamespace.ToDisplayString(); + var variants = ExtractVariants(classSymbol, parameters, out var variantError); + return new ToolModel( classNamespace, classSymbol.Name, @@ -124,7 +125,141 @@ public void Initialize(IncrementalGeneratorInitializationContext context) grant, liveness, paramsType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), - [.. parameters]); + [.. parameters], + variants, + variantError); + } + + private static string ReadDescription(ImmutableArray attributes) + { + foreach (var attribute in attributes) + { + if (attribute.AttributeClass?.Name == "DescriptionAttribute" + && attribute.ConstructorArguments.Length > 0 + && attribute.ConstructorArguments[0].Value is string description) + { + return description; + } + } + + return string.Empty; + } + + private static ImmutableArray ExtractVariants( + INamedTypeSymbol classSymbol, + IReadOnlyList parameters, + out string? error) + { + error = null; + var attributes = classSymbol.GetAttributes() + .Where(static attribute => attribute.AttributeClass?.ToDisplayString() == VariantAttributeFullName) + .ToArray(); + if (attributes.Length == 0) + return []; + + var parameterNames = new HashSet( + parameters.Select(static parameter => parameter.Name), + System.StringComparer.Ordinal); + var variants = new List(attributes.Length); + string? sharedDiscriminator = null; + var values = new HashSet(System.StringComparer.OrdinalIgnoreCase); + + foreach (var attribute in attributes) + { + if (attribute.ConstructorArguments.Length != 2 + || attribute.ConstructorArguments[0].Value is not string discriminator + || attribute.ConstructorArguments[1].Value is not string value + || string.IsNullOrWhiteSpace(discriminator) + || string.IsNullOrWhiteSpace(value)) + { + error = "the discriminator parameter and value must be non-empty strings"; + return []; + } + + if (!parameterNames.Contains(discriminator)) + { + error = $"the discriminator parameter '{discriminator}' does not exist"; + return []; + } + + var discriminatorParameter = parameters.First(parameter => parameter.Name == discriminator); + if (discriminatorParameter.JsonType != "string") + { + error = $"the discriminator parameter '{discriminator}' must be a string"; + return []; + } + + sharedDiscriminator ??= discriminator; + if (!string.Equals(sharedDiscriminator, discriminator, System.StringComparison.Ordinal)) + { + error = "all variants must use the same discriminator parameter"; + return []; + } + + if (!values.Add(value)) + { + error = $"the discriminator value '{value}' is duplicated"; + return []; + } + + if (!TryReadStringArray(attribute, "Required", out var required) + || !TryReadStringArray(attribute, "Forbidden", out var forbidden)) + { + error = $"variant '{value}' contains a null parameter name"; + return []; + } + + if (required.Any(name => !parameterNames.Contains(name)) + || forbidden.Any(name => !parameterNames.Contains(name))) + { + error = $"variant '{value}' references an unknown parameter"; + return []; + } + + if (required.Contains(discriminator, System.StringComparer.Ordinal) + || forbidden.Contains(discriminator, System.StringComparer.Ordinal) + || required.Distinct(System.StringComparer.Ordinal).Count() != required.Length + || forbidden.Distinct(System.StringComparer.Ordinal).Count() != forbidden.Length + || required.Intersect(forbidden, System.StringComparer.Ordinal).Any()) + { + error = $"variant '{value}' has conflicting required or forbidden parameters"; + return []; + } + + variants.Add(new ToolVariant(discriminator, value, required, forbidden)); + } + + return [.. variants]; + } + + private static bool TryReadStringArray( + AttributeData attribute, + string name, + out ImmutableArray values) + { + foreach (var argument in attribute.NamedArguments) + { + if (argument.Key != name || argument.Value.Kind != TypedConstantKind.Array) + continue; + + var result = ImmutableArray.CreateBuilder(argument.Value.Values.Length); + foreach (var value in argument.Value.Values) + { + if (value.Value is not string parameterName) + { + values = []; + return false; + } + + result.Add(parameterName); + } + + values = result.MoveToImmutable(); + return true; + } + + values = []; + return true; } private static string GetJsonType(ITypeSymbol type) @@ -197,6 +332,15 @@ value.Type is not INamedTypeSymbol enumType || private static void GenerateSource(SourceProductionContext spc, ToolModel model) { + if (model.VariantError is not null) + { + var error = $"#error NETCLAWTOOL001 Tool '{model.ToolName}' has an invalid conditional variant: {model.VariantError}"; + spc.AddSource( + $"{model.ClassName}.variant-error.g.cs", + SourceText.From(error, Encoding.UTF8)); + return; + } + var sb = new StringBuilder(); sb.AppendLine("// "); @@ -256,7 +400,36 @@ private static void GenerateSource(SourceProductionContext spc, ToolModel model) var requiredNames = required.Select(p => $"\"{p.Name}\"").Append("\"_rationale\""); sb.Append(" \"required\": ["); sb.Append(string.Join(", ", requiredNames)); - sb.AppendLine("]"); + sb.Append("]"); + + if (model.Variants.Length > 0) + { + sb.AppendLine(","); + sb.AppendLine(" \"additionalProperties\": false,"); + sb.AppendLine(" \"oneOf\": ["); + for (var index = 0; index < model.Variants.Length; index++) + { + var variant = model.Variants[index]; + sb.AppendLine(" {"); + sb.AppendLine($" \"properties\": {{ \"{variant.DiscriminatorParameter}\": {{ \"enum\": [\"{EscapeJson(variant.DiscriminatorValue)}\"] }} }},"); + var branchRequired = variant.Required + .Prepend(variant.DiscriminatorParameter) + .Select(static name => $"\"{name}\""); + sb.AppendLine($" \"required\": [{string.Join(", ", branchRequired)}]{(variant.Forbidden.Length > 0 ? "," : string.Empty)}"); + if (variant.Forbidden.Length > 0) + { + var forbidden = variant.Forbidden + .Select(static name => $"{{ \"required\": [\"{name}\"] }}"); + sb.AppendLine($" \"not\": {{ \"anyOf\": [{string.Join(", ", forbidden)}] }}"); + } + sb.AppendLine(index == model.Variants.Length - 1 ? " }" : " },"); + } + sb.AppendLine(" ]"); + } + else + { + sb.AppendLine(); + } sb.AppendLine(" }"); sb.AppendLine(" \"\"\").RootElement.Clone();"); @@ -358,6 +531,39 @@ private static void GenerateSource(SourceProductionContext spc, ToolModel model) } } + if (model.Variants.Length > 0) + { + sb.AppendLine(); + sb.AppendLine(" static int __ArgumentValueCount(System.Collections.Generic.IDictionary source, string parameter)"); + sb.AppendLine(" {"); + sb.AppendLine(" var normalized = Netclaw.Tools.ToolArgumentHelper.NormalizeKey(parameter);"); + sb.AppendLine(" var count = 0;"); + sb.AppendLine(" foreach (var pair in source)"); + sb.AppendLine(" {"); + sb.AppendLine(" if (!string.Equals(Netclaw.Tools.ToolArgumentHelper.NormalizeKey(pair.Key), normalized, System.StringComparison.OrdinalIgnoreCase))"); + sb.AppendLine(" continue;"); + sb.AppendLine(" if (pair.Value is not null and not JsonElement { ValueKind: JsonValueKind.Null })"); + sb.AppendLine(" count++;"); + sb.AppendLine(" }"); + sb.AppendLine(" return count;"); + sb.AppendLine(" }"); + sb.AppendLine(); + sb.AppendLine(" var __variantMatches = 0;"); + foreach (var variant in model.Variants) + { + sb.AppendLine($" if (__ArgumentValueCount(arguments, \"{variant.DiscriminatorParameter}\") == 1"); + sb.AppendLine($" && string.Equals(__{variant.DiscriminatorParameter}, \"{EscapeJson(variant.DiscriminatorValue)}\", System.StringComparison.OrdinalIgnoreCase)"); + foreach (var requiredParameter in variant.Required) + sb.AppendLine($" && __ArgumentValueCount(arguments, \"{requiredParameter}\") == 1"); + foreach (var forbiddenParameter in variant.Forbidden) + sb.AppendLine($" && __ArgumentValueCount(arguments, \"{forbiddenParameter}\") == 0"); + sb.AppendLine(" )"); + sb.AppendLine(" __variantMatches++;"); + } + sb.AppendLine(" if (__variantMatches != 1)"); + sb.AppendLine($" throw new System.ArgumentException(\"Arguments for tool '{EscapeJson(model.ToolName)}' must match exactly one declared variant. The tool was NOT executed.\");"); + } + sb.AppendLine(); sb.Append($" return new {model.ParamsTypeName}("); sb.Append(string.Join(", ", model.Parameters.Select(p => $"__{p.Name}"))); @@ -376,7 +582,8 @@ private static string EscapeJson(string value) => internal sealed class ToolModel { public ToolModel(string? ns, string className, string toolName, string toolDescription, - string grant, string liveness, string paramsTypeName, ImmutableArray parameters) + string grant, string liveness, string paramsTypeName, ImmutableArray parameters, + ImmutableArray variants, string? variantError) { Namespace = ns; ClassName = className; @@ -386,6 +593,8 @@ public ToolModel(string? ns, string className, string toolName, string toolDescr Liveness = liveness; ParamsTypeName = paramsTypeName; Parameters = parameters; + Variants = variants; + VariantError = variantError; } public string? Namespace { get; } @@ -396,6 +605,28 @@ public ToolModel(string? ns, string className, string toolName, string toolDescr public string Liveness { get; } public string ParamsTypeName { get; } public ImmutableArray Parameters { get; } + public ImmutableArray Variants { get; } + public string? VariantError { get; } +} + +internal sealed class ToolVariant +{ + public ToolVariant( + string discriminatorParameter, + string discriminatorValue, + ImmutableArray required, + ImmutableArray forbidden) + { + DiscriminatorParameter = discriminatorParameter; + DiscriminatorValue = discriminatorValue; + Required = required; + Forbidden = forbidden; + } + + public string DiscriminatorParameter { get; } + public string DiscriminatorValue { get; } + public ImmutableArray Required { get; } + public ImmutableArray Forbidden { get; } } internal sealed class ToolParameter