diff --git a/feeds/skills/.system/files/netclaw-operations/SKILL.md b/feeds/skills/.system/files/netclaw-operations/SKILL.md index e9aacc3d5..9f8d8bbe9 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.107.0" + version: "2.107.1" --- # Netclaw Operations @@ -230,17 +230,19 @@ This failure does not prove that the turn exhausted its tool budget. ## Large tool output -Netclaw bounds each tool result to an inline budget -(`Session.Tuning.MaxInlineToolResultChars`, default 12,000 characters; -`shell_execute` uses 2,000). A longer result gives you its first part and its -last part, not the whole text: +Netclaw limits each inline tool result to a character budget. +`Session.Tuning.MaxInlineToolResultChars` defaults to 12,000 characters. +`shell_execute` uses 2,000 characters. +A longer result contains only its tail, with a separator before the retained text. +The shell tail retains the process exit status. - **Each tool, also `skill_load`, `skill_read_resource`, and MCP tools**: Netclaw keeps the full redacted result inside the current session. The last line of the result names `tool_output_read` and a `CallId`. Use `tool_output_read` with that - `CallId` and a `Start`/`Limit` window to read the middle. + `CallId` and a `Start`/`Limit` window to read the omitted prefix or middle. + Set `Start=0` to read from the start of the retained result. Do not request a path or rerun the source tool to read more. - If the last line says that Netclaw did not keep the full output, the middle is + If the last line says that Netclaw did not keep the full output, the omitted text is not available: narrow the call if the tool has a bound, or read one specific resource with `skill_read_resource`. - **`file_read`** on a large file returns the head and steers you to read a diff --git a/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/.openspec.yaml b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/.openspec.yaml new file mode 100644 index 000000000..81264324b --- /dev/null +++ b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-09 diff --git a/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/design.md b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/design.md new file mode 100644 index 000000000..63f754a35 --- /dev/null +++ b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/design.md @@ -0,0 +1,44 @@ +## Context + +`ShellTool` prefixes completed process output with the exit status. +The dispatcher keeps only the last 2,000 characters of a large shell result. +That tail removes the prefix on both execution paths. + +Use the [engineering glossary](../../../../docs/spec/GLOSSARY.md) for shared terms. + +## Goals / Non-Goals + +Goals include tail-only inline output, visible exit status, deterministic tests, and accurate model guidance. +New result types, approval changes, and configuration changes are outside this fix. + +## Decisions + +`ShellTool` owns the authoritative process status as call-local data. +Reuse one result formatter for both completion paths. +Keep the current prefix and append the same status only when the result exceeds the existing shell inline budget. +This preserves small results and avoids a parser for result text. + +The dispatcher retains its current order: + +```text +execute the authorized tool +format the shell capture and process status +redact the result +keep the inline tail +write the session-scoped redacted spill +append the continuation notice +``` + +The dispatcher owns the inline budget and spill decision as call-local data. +The spill remains durable under the current session directory. +Actor boundaries and persistent record types do not change. + +A new structured metadata contract would require more changes than this fix needs. +A larger inline budget would only move the failure threshold. + +## Risks / Trade-offs + +Large retained shell results contain the status at both ends. +The inline result contains the final status and needs no extra continuation call to determine the process outcome. +A failed spill still returns that status and the current explicit loss notice. +Command timeout and cancellation retain their current error results. diff --git a/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/proposal.md b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/proposal.md new file mode 100644 index 000000000..11e9cd934 --- /dev/null +++ b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/proposal.md @@ -0,0 +1,35 @@ +## Why + +Tail-only tool results reduce the context that models must interpret. +The new window also removes shell exit status and contradicts the current operational skill and specification. + +## What Changes + +- Retain only the tail of an oversized tool result. +- Preserve the shell exit status in that tail for both execution paths. +- Add deterministic tests for oversized results with zero and nonzero exit codes. +- Update the operational skill and the main specification. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `bounded-tool-output`: Require tail-only inline results and visible shell exit status after truncation. + +## Impact + +Source requirements: [PRD-001 FR-011](../../../../docs/prd/PRD-001-netclaw-mvp.md#fr-011-tool-access) and +[PRD-006 MCP-003](../../../../docs/prd/PRD-006-mcp-tool-integration.md#mcp-003-policy-gating). + +MVP scope includes shell result text, dispatcher tests, operational guidance, and the output contract. +New tools, approval scopes, configuration properties, and unbounded capture are out of scope. + +## Security and operational impact + +The dispatcher retains the current redaction, default-deny authorization, and session-scoped continuation gates. +Models can read the exit status without another tool call. +Operators retain the current capture ceiling and spill behavior. diff --git a/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/specs/bounded-tool-output/spec.md b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/specs/bounded-tool-output/spec.md new file mode 100644 index 000000000..a2ab06fc0 --- /dev/null +++ b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/specs/bounded-tool-output/spec.md @@ -0,0 +1,120 @@ +## MODIFIED Requirements + +### Requirement: Central bound + spill for every tool result + +`DispatchingToolExecutor` SHALL bound every tool result to an inline budget and, +when the result exceeds that budget, return only its last budget characters plus +a separator and a continuation notice. This applies uniformly to every +tool, for the main session and for sub-agents (both run tools through the +dispatcher). The bound SHALL be applied after the dispatcher's central secret +redaction, so the inline result and any spilled file are redacted from one pass. +Individual tools SHALL NOT window, redact, or spill their results; they only bound +their own capture for memory safety and return the raw bounded result. + +#### Scenario: Result under budget returned unchanged + +- **WHEN** a tool returns a result at or below its inline budget +- **THEN** the dispatcher returns it unchanged +- **AND** no spill file is created + +#### Scenario: Result over budget windowed to tail only + +- **WHEN** a tool returns a result larger than its inline budget +- **THEN** the inline result contains only the last budget characters of the result +- **AND** a separator marks the discarded prefix +- **AND** the retained spill contains the prefix that the inline result omits + +#### Scenario: Same bounding for sub-agent tool calls + +- **WHEN** a sub-agent runs a tool that returns an oversized result +- **THEN** the same dispatcher bound + spill applies (sub-agents are not exempt) + +### Requirement: Spill to a session-scoped file with a steer + +When a result exceeds its inline budget, the dispatcher SHALL write the full redacted result to an internal file under the current session `tool-calls` directory. The dispatcher SHALL derive the file name from the sanitized call id. It SHALL return the opaque call id and a steer to `tool_output_read`. It SHALL NOT reveal the raw spill path or direct the model to shell, grep, or `file_read`. When no session directory or call id is available, the dispatcher SHALL return the inline window without a spill steer. + +Concrete result shape: + +```text +tool result size = 40,000 characters +inline budget = 12,000 characters +call id = call-example + +model receives: + + + [output truncated to 12000 chars of 40000; continue with + tool_output_read using CallId='call-example' and a bounded Start/Limit + window instead of re-running] + +internal storage may use: + /tool-calls/call-example.txt + +model never receives: + /tool-calls/call-example.txt + "run grep on the saved file" + "use file_read on the saved file" +``` + +Counterexamples: + +| Input state | Required behavior | +|---|---| +| Result fits the inline budget | Return it without a spill or continuation steer. | +| Result is oversized but the call id is absent | Return only the bounded inline window. Do not invent an id or reveal a path. | +| Call id is `../../outside` | Sanitize or reject the spill location; never write outside the session spill directory. | +| Spill write fails | Preserve the bounded inline result without claiming that continuation exists. | + +#### Scenario: Spill file stays internal + +- **WHEN** a result over budget is produced in a session with a directory +- **THEN** the full redacted result is written under the session `tool-calls` directory +- **AND** the inline result includes the opaque call id +- **AND** the steer names `tool_output_read` +- **AND** the steer contains no filesystem path + +#### Scenario: Spilled file is redacted + +- **WHEN** a result that contains a secret is spilled +- **THEN** the internal spill file has the secret redacted +- **AND** redaction occurs before the spill write + +#### Scenario: Call id cannot escape the spill directory + +- **WHEN** the call id contains path-traversal characters +- **THEN** the spill file stays inside the tool-calls directory +- **AND** the dispatcher reveals no raw path + +#### Scenario: Missing spill identity has no false continuation + +- **GIVEN** a result exceeds its inline budget +- **AND** the invocation has no usable session directory or call id +- **WHEN** the dispatcher bounds the result +- **THEN** the model receives the bounded inline window +- **AND** the result does not claim that `tool_output_read` can continue it + +## ADDED Requirements + +### Requirement: Shell exit status survives inline truncation + +`ShellTool` SHALL include the process exit status in every successful process-completion result. +For an oversized result, it SHALL place that status at the end of its result before the dispatcher applies the inline budget. +The dispatcher SHALL retain that status in the inline tail. +This rule applies to both execution paths and to zero and nonzero exit codes. +The status describes the process outcome, not approval or tool authorization. + +#### Scenario: Oversized successful command retains its exit status + +- **GIVEN** an authorized shell command produces more than 2,000 result characters +- **WHEN** the command exits with code zero through either execution path +- **THEN** the inline result contains `Exit code: 0` and the output tail +- **AND** the inline result omits the output prefix +- **AND** the retained spill contains the bounded capture and its exit status + +#### Scenario: Oversized failed command retains its exit status + +- **GIVEN** an authorized shell command produces more than 2,000 result characters +- **WHEN** the command exits with a nonzero code through either execution path +- **THEN** the inline result contains that exact exit code and the output tail +- **AND** the inline result does not report `Exit code: 0` +- **AND** the retained spill contains the same exit status diff --git a/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/tasks.md b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/tasks.md new file mode 100644 index 000000000..4b33ff70e --- /dev/null +++ b/openspec/changes/archive/2026-10-09-preserve-shell-status-with-tail-output/tasks.md @@ -0,0 +1,11 @@ +## 1. Preserve shell status + +- [x] 1.1 Add dispatcher regression tests for oversized output with zero and nonzero exit codes on both execution paths. +- [x] 1.2 Preserve the exit status in the tail and keep small result text unchanged. +- [x] 1.3 Verify the original approval retry test and the output suites. + +## 2. Align the operational contract + +- [x] 2.1 Update the operational skill, increment its version, and sync the delta specification. +- [x] 2.2 Run the repository quality checks. +- [x] 2.3 Verify the implementation against the change and archive the completed artifacts. diff --git a/openspec/specs/bounded-tool-output/spec.md b/openspec/specs/bounded-tool-output/spec.md index 9e0a701da..fcd3b4c7d 100644 --- a/openspec/specs/bounded-tool-output/spec.md +++ b/openspec/specs/bounded-tool-output/spec.md @@ -13,8 +13,8 @@ Use the [Netclaw engineering glossary](../../../docs/spec/GLOSSARY.md) for cross ### Requirement: Central bound + spill for every tool result `DispatchingToolExecutor` SHALL bound every tool result to an inline budget and, -when the result exceeds that budget, return a head+tail window of the budget plus -a steer pointing the model at the full output. This applies uniformly to every +when the result exceeds that budget, return only its last budget characters plus +a separator and a continuation notice. This applies uniformly to every tool, for the main session and for sub-agents (both run tools through the dispatcher). The bound SHALL be applied after the dispatcher's central secret redaction, so the inline result and any spilled file are redacted from one pass. @@ -27,11 +27,12 @@ their own capture for memory safety and return the raw bounded result. - **THEN** the dispatcher returns it unchanged - **AND** no spill file is created -#### Scenario: Result over budget windowed to head and tail +#### Scenario: Result over budget windowed to tail only - **WHEN** a tool returns a result larger than its inline budget -- **THEN** the inline result contains the head and the tail of the result within - the budget, with the discarded middle marked +- **THEN** the inline result contains only the last budget characters of the result +- **AND** a separator marks the discarded prefix +- **AND** the retained spill contains the prefix that the inline result omits #### Scenario: Same bounding for sub-agent tool calls @@ -72,7 +73,7 @@ inline budget = 12,000 characters call id = call-example model receives: - + [output truncated to 12000 chars of 40000; continue with tool_output_read using CallId='call-example' and a bounded Start/Limit @@ -166,3 +167,27 @@ return more than the configured limit. - **WHEN** `tool_output_read` executes - **THEN** the outcome is `not_found` - **AND** the bounded result suggests a narrower source call + +### Requirement: Shell exit status survives inline truncation + +`ShellTool` SHALL include the process exit status in every successful process-completion result. +For an oversized result, it SHALL place that status at the end of its result before the dispatcher applies the inline budget. +The dispatcher SHALL retain that status in the inline tail. +This rule applies to both execution paths and to zero and nonzero exit codes. +The status describes the process outcome, not approval or tool authorization. + +#### Scenario: Oversized successful command retains its exit status + +- **GIVEN** an authorized shell command produces more than 2,000 result characters +- **WHEN** the command exits with code zero through either execution path +- **THEN** the inline result contains `Exit code: 0` and the output tail +- **AND** the inline result omits the output prefix +- **AND** the retained spill contains the bounded capture and its exit status + +#### Scenario: Oversized failed command retains its exit status + +- **GIVEN** an authorized shell command produces more than 2,000 result characters +- **WHEN** the command exits with a nonzero code through either execution path +- **THEN** the inline result contains that exact exit code and the output tail +- **AND** the inline result does not report `Exit code: 0` +- **AND** the retained spill contains the same exit status diff --git a/src/Netclaw.Actors.Tests/Tools/BoundedOutputReaderTests.cs b/src/Netclaw.Actors.Tests/Tools/BoundedOutputReaderTests.cs index af7363a05..e76160bed 100644 --- a/src/Netclaw.Actors.Tests/Tools/BoundedOutputReaderTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/BoundedOutputReaderTests.cs @@ -42,35 +42,35 @@ public async Task DrainToWindow_output_exactly_at_cap_not_truncated() } [Fact] - public async Task DrainToWindow_long_output_truncated_with_head_and_tail() + public async Task DrainToWindow_long_output_truncated_to_tail_only() { - // 100-char head marker + separator + 100-char tail marker, with filler in the middle + // 200-char tail marker with head and middle filler before it. Only the + // last `budget` chars survive, prefixed by the separator. var head = new string('H', 100); var middle = new string('M', 5000); - var tail = new string('T', 100); + var tail = new string('T', 200); var input = head + middle + tail; var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 200, CancellationToken.None); Assert.True(truncated); Assert.False(cancelled); // budget cut, not a cancelled/grace cut - Assert.StartsWith(new string('H', 100), text); // head preserved - Assert.EndsWith(new string('T', 100), text); // tail preserved - Assert.Contains("...", text); // separator present + Assert.EndsWith(new string('T', 200), text); // tail preserved + Assert.StartsWith(BoundedOutputReader.Separator, text); // separator preface present + Assert.DoesNotContain("H", text); // head discarded Assert.DoesNotContain("M", text); // middle discarded } [Fact] - public async Task DrainToWindow_head_and_tail_split_evenly() + public async Task DrainToWindow_keeps_last_budget_chars_with_tail_only() { - // budget=10 → headCap=5, tailCap=5 - var input = "AAAAAXXXXXXBBBBB"; // 16 chars: 5 head, 6 overflow discard, 5 tail + // budget=10 → the last 10 chars of "AAAAAXXXXXXBBBBB" = "XXXXXBBBBB". + var input = "AAAAAXXXXXXBBBBB"; // 16 chars: 6 head + 4 middle discarded, 10 tail kept var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(new StringReader(input), 10, CancellationToken.None); Assert.True(truncated); Assert.False(cancelled); - Assert.StartsWith("AAAAA", text); - Assert.EndsWith("BBBBB", text); + Assert.Equal($"{Environment.NewLine}...{Environment.NewLine}XXXXXBBBBB", text); } [Fact] @@ -87,16 +87,16 @@ public async Task DrainToWindow_disabled_cap_returns_full_output() public async Task DrainToWindow_tail_ring_wraps_across_small_chunks() { // Drives the ring's wraparound + start-advance path that the StringReader - // tests skip: each read delivers a chunk smaller than tailCap, so the tail - // window is rebuilt incrementally and must wrap rather than reset wholesale. - // budget=10 → headCap=5 ("ABCDE"), tailCap=5; last 5 of "FGHIJKLMNO" = "KLMNO". + // tests skip: each read delivers a chunk smaller than the budget, so the + // tail window is rebuilt incrementally and must wrap rather than reset + // wholesale. budget=10 → last 10 of "ABCDEFGHIJKLMNO" = "ABCDEFGHIJKL"[^10..] = "FGHIJKLMNO". var reader = new ChunkedReader("ABCDEFGHIJKLMNO", chunkSize: 3); var (text, truncated, cancelled) = await BoundedOutputReader.DrainToWindowAsync(reader, 10, CancellationToken.None); Assert.True(truncated); Assert.False(cancelled); - Assert.Equal($"ABCDE{Environment.NewLine}...{Environment.NewLine}KLMNO", text); + Assert.Equal($"{Environment.NewLine}...{Environment.NewLine}FGHIJKLMNO", text); } [Fact] @@ -115,7 +115,7 @@ public async Task DrainToWindow_cancelled_before_eof_reports_cancelled_true() Assert.Equal("", text); } - // ── Window (pure string head+tail) ── + // ── Window (pure string tail-only) ── [Fact] public void Window_under_budget_returned_unchanged() @@ -124,14 +124,15 @@ public void Window_under_budget_returned_unchanged() } [Fact] - public void Window_over_budget_keeps_head_and_tail() + public void Window_over_budget_keeps_tail_only() { - var input = new string('H', 50) + new string('M', 500) + new string('T', 50); + var input = new string('H', 50) + new string('M', 500) + new string('T', 100); var result = BoundedOutputReader.Window(input, 100); - Assert.StartsWith(new string('H', 50), result); - Assert.EndsWith(new string('T', 50), result); - Assert.DoesNotContain("M", result); + Assert.EndsWith(new string('T', 100), result); // last budget chars kept + Assert.StartsWith(BoundedOutputReader.Separator, result); // separator preface + Assert.DoesNotContain("H", result); // head discarded + Assert.DoesNotContain("M", result); // middle discarded } // ── BoundedOutputAccumulator ── @@ -147,18 +148,18 @@ public void Accumulator_short_input_returned_verbatim() } [Fact] - public void Accumulator_long_input_truncates_with_head_and_tail() + public void Accumulator_long_input_truncates_to_tail_only() { var acc = new BoundedOutputAccumulator(200); acc.Append(new string('H', 100).AsSpan()); acc.Append(new string('M', 5000).AsSpan()); - acc.Append(new string('T', 100).AsSpan()); + acc.Append(new string('T', 200).AsSpan()); var (text, truncated) = acc.Finish(); Assert.True(truncated); - Assert.StartsWith(new string('H', 100), text); - Assert.EndsWith(new string('T', 100), text); - Assert.Contains("...", text); + Assert.EndsWith(new string('T', 200), text); + Assert.StartsWith(BoundedOutputReader.Separator, text); + Assert.DoesNotContain("H", text); Assert.DoesNotContain("M", text); } @@ -174,13 +175,13 @@ public void Accumulator_many_small_chunks_wraps_tail_ring() var (text, truncated) = acc.Finish(); Assert.True(truncated); - Assert.Equal($"ABCDE{Environment.NewLine}...{Environment.NewLine}KLMNO", text); + Assert.Equal($"{Environment.NewLine}...{Environment.NewLine}FGHIJKLMNO", text); } [Fact] public async Task Accumulator_matches_drain_output() { - var input = new string('H', 100) + new string('M', 5000) + new string('T', 100); + var input = new string('H', 100) + new string('M', 5000) + new string('T', 200); const int budget = 200; var (drainText, drainTruncated, _) = await BoundedOutputReader.DrainToWindowAsync( diff --git a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs index 65a633e64..57485bc5d 100644 --- a/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/DispatchingToolExecutorTests.cs @@ -140,6 +140,53 @@ public async Task Verbose_tool_output_over_budget_is_windowed_and_spilled() } } + [Theory] + [InlineData(0, false)] + [InlineData(7, false)] + [InlineData(0, true)] + [InlineData(7, true)] + public async Task Oversized_shell_output_retains_exit_status_and_only_the_tail(int exitCode, bool stream) + { + const string head = "OUTPUT-HEAD"; + const string tail = "OUTPUT-TAIL"; + var output = head + new string('x', 3000) + tail; + var callId = $"call-exit-status-{exitCode}-{stream}"; + var call = CreateToolCall(callId, "shell_execute", + ToolInput.Create("Command", $"echo '{output}'; exit {exitCode}")); + var context = TestToolExecutionContext.CreateBound( + "signalr/exit-status", BoundSessionDirectory, TrustAudience.Personal); + + string? result = null; + if (stream) + { + await foreach (var update in _executor.ExecuteStreamAsync(call, context, TestContext.Current.CancellationToken)) + { + if (update is ToolCompletedUpdate completed) + result = completed.Result; + } + } + else + { + result = await _executor.ExecuteAsync(call, context, TestContext.Current.CancellationToken); + } + + Assert.NotNull(result); + Assert.Contains($"Exit code: {exitCode}", result, StringComparison.Ordinal); + Assert.StartsWith(BoundedOutputReader.Separator, result, StringComparison.Ordinal); + Assert.DoesNotContain(head, result, StringComparison.Ordinal); + Assert.Contains(tail, result, StringComparison.Ordinal); + Assert.Contains($"tool_output_read using CallId='{callId}'", result, StringComparison.Ordinal); + + var inline = result[..result.IndexOf("[output truncated", StringComparison.Ordinal)]; + Assert.EndsWith($"Exit code: {exitCode}", inline.TrimEnd(), StringComparison.Ordinal); + Assert.True(ToolOutputSpillLocation.TryResolve(BoundSessionDirectory, callId, out _, out var spill)); + var retained = await File.ReadAllTextAsync(spill, TestContext.Current.CancellationToken); + Assert.StartsWith($"Exit code: {exitCode}{Environment.NewLine}", retained, StringComparison.Ordinal); + Assert.Contains(output, retained, StringComparison.Ordinal); + if (exitCode != 0) + Assert.DoesNotContain("Exit code: 0", result, StringComparison.Ordinal); + } + [Fact] public async Task Spilled_output_is_redacted_before_write() { diff --git a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillFreshSessionTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillFreshSessionTests.cs index a4a3a955d..df3aa494d 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillFreshSessionTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillFreshSessionTests.cs @@ -125,7 +125,7 @@ await Assert.ThrowsAsync(() => ExecuteAsync( private async Task AssertHiddenMiddleIsReadableAsync( string callId, string result, TrustAudience audience = TrustAudience.Personal) { - // The inline window has the head and the tail, not the middle. + // The inline tail omits the middle. The continuation must recover it. Assert.Contains("[output truncated to", result, StringComparison.Ordinal); Assert.DoesNotContain(HiddenRule, result, StringComparison.Ordinal); Assert.Contains($"tool_output_read using CallId='{callId}'", result, StringComparison.Ordinal); diff --git a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs index d709c1fcd..193d2015e 100644 --- a/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs +++ b/src/Netclaw.Actors.Tests/Tools/ToolOutputSpillTests.cs @@ -57,7 +57,8 @@ public async Task Over_budget_spills_full_output_and_steers() _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(new string('T', 100), result); // inline tail only + Assert.DoesNotContain("H", result); // inline head dropped Assert.Contains("tool_output_read", result); Assert.Contains("CallId='call_2'", result); Assert.DoesNotContain(spillPath, result); @@ -87,7 +88,9 @@ public async Task No_session_directory_degrades_to_inline_only() var result = await ToolOutputSpill.BoundAndSpillAsync( input, "call_5", budget: 100, ctx, NullLogger.Instance, CancellationToken.None); - Assert.StartsWith(new string('H', 50), result); // inline still produced + var inline = result[..result.IndexOf("[output truncated", StringComparison.Ordinal)]; + Assert.Contains(new string('T', 100), inline); // inline tail still produced + Assert.DoesNotContain("H", inline); // inline head dropped // No continuation, and the text says so. It does not offer a call id. Assert.Contains("did not keep the full output", result); Assert.DoesNotContain("CallId=", result); diff --git a/src/Netclaw.Actors/Tools/BoundedOutputReader.cs b/src/Netclaw.Actors/Tools/BoundedOutputReader.cs index 12b7d365c..7aa3b94e5 100644 --- a/src/Netclaw.Actors/Tools/BoundedOutputReader.cs +++ b/src/Netclaw.Actors/Tools/BoundedOutputReader.cs @@ -9,24 +9,26 @@ namespace Netclaw.Actors.Tools; /// -/// Bounds external tool output (process pipes, files) into a head+tail window of -/// a fixed character budget, in bounded memory regardless of total output size. -/// Extracted from ShellTool.BoundedDrainAsync (#1293) so the ring/window -/// logic is reviewed and fixed once and reused by shell_execute, +/// Bounds external tool output (process pipes, files) to the LAST +/// budget chars of a fixed character budget, in bounded memory regardless +/// of total output size. The dropped head is not kept inline: callers spill the +/// full result and steer the model to tool_output_read for it. Extracted +/// from ShellTool.BoundedDrainAsync (#1293) so the ring/window logic is +/// reviewed and fixed once and reused by shell_execute, /// background_job, and file_read. /// /// /// The reader is a pure leaf: it does no redaction and no file IO — callers /// redact (SecretOutputRedactor) and spill. Allocation is /// O(budget), not O(total output): the scratch read buffer is pooled, the tail -/// ring is allocated only once the head fills, and reads go through the +/// ring is allocated only once it first receives data, and reads go through the /// overload so a pipe that already has data buffered /// completes synchronously without a per-chunk allocation. /// internal static class BoundedOutputReader { /// - /// Drains into a head+tail window bounded by + /// Drains into a tail-only window bounded by /// chars. Chars beyond the budget are discarded but /// the source continues to be read so a still-running child never deadlocks on /// a full pipe buffer. A non-positive disables the @@ -91,25 +93,25 @@ internal static class BoundedOutputReader } /// - /// Head+tail window of an already-in-memory string to - /// chars. Returns the string unchanged when it already fits (or the budget is - /// non-positive). Used to derive the inline window from a larger (redacted) - /// capture — see ToolOutputSpill. + /// Tail-only window of an already-in-memory string: the LAST + /// chars plus a leading separator. Returns the + /// string unchanged when it already fits (or the budget is non-positive). + /// Used to derive the inline window from a larger (redacted) capture — see + /// ToolOutputSpill. The dropped head is only in the spill file; the + /// model pulls it back with tool_output_read. /// /// /// bounds the retained content, not the returned /// string length: a truncated result is budget + Separator.Length chars - /// (the head, the "…" separator, and the tail). Callers enforcing a hard - /// character ceiling must account for the separator. + /// (the separator and the tail). Callers enforcing a hard character ceiling + /// must account for the separator. /// public static string Window(string text, int budget) { if (budget <= 0 || text.Length <= budget) return text; - var headCap = budget / 2 + budget % 2; - var tailCap = budget / 2; - return string.Concat(text.AsSpan(0, headCap), Separator, text.AsSpan(text.Length - tailCap)); + return string.Concat(Separator, text.AsSpan(text.Length - budget)); } internal static readonly string Separator = $"{Environment.NewLine}...{Environment.NewLine}"; @@ -163,17 +165,15 @@ internal static void AppendRing(StringBuilder sb, char[] ring, int start, int le } /// -/// Stateful head+tail accumulator that accepts incremental Append calls -/// and produces the same bounded window as . -/// Used by the streaming ShellTool path where pipe chunks must be fed to -/// both the activity channel and the bounded capture simultaneously. +/// Stateful tail-only accumulator that accepts incremental Append calls +/// and produces the same bounded window as : +/// the last budget chars, with a leading separator when truncated. Used by +/// the streaming ShellTool path where pipe chunks must be fed to both the +/// activity channel and the bounded capture simultaneously. /// internal sealed class BoundedOutputAccumulator { private readonly int _budget; - private readonly int _headCap; - private readonly int _tailCap; - private readonly StringBuilder _head; private char[]? _tailBuf; private int _tailStart; private int _tailLen; @@ -182,37 +182,28 @@ internal sealed class BoundedOutputAccumulator public BoundedOutputAccumulator(int budget) { _budget = budget; - _headCap = budget / 2 + budget % 2; - _tailCap = budget / 2; - _head = new StringBuilder(Math.Min(_headCap, 4096)); } public void Append(ReadOnlySpan chunk) { _totalChars += chunk.Length; - var span = chunk; - - if (_head.Length < _headCap) - { - var headChunk = Math.Min(_headCap - _head.Length, span.Length); - _head.Append(span[..headChunk]); - span = span[headChunk..]; - } - - if (span.IsEmpty || _tailCap == 0) + if (chunk.IsEmpty || _budget <= 0) return; - _tailBuf ??= new char[_tailCap]; - BoundedOutputReader.AppendToTailRing(_tailBuf, span, ref _tailStart, ref _tailLen); + _tailBuf ??= new char[_budget]; + BoundedOutputReader.AppendToTailRing(_tailBuf, chunk, ref _tailStart, ref _tailLen); } public (string Text, bool Truncated) Finish() { var truncated = _totalChars > _budget; + if (_tailBuf is null || _tailLen == 0) + return (string.Empty, truncated); + + var sb = new StringBuilder(_tailLen + (truncated ? BoundedOutputReader.Separator.Length : 0)); if (truncated) - _head.Append(BoundedOutputReader.Separator); - if (_tailBuf is not null && _tailLen > 0) - BoundedOutputReader.AppendRing(_head, _tailBuf, _tailStart, _tailLen); - return (_head.ToString(), truncated); + sb.Append(BoundedOutputReader.Separator); + BoundedOutputReader.AppendRing(sb, _tailBuf, _tailStart, _tailLen); + return (sb.ToString(), truncated); } } diff --git a/src/Netclaw.Actors/Tools/ShellTool.cs b/src/Netclaw.Actors/Tools/ShellTool.cs index c60ec3545..2254399af 100644 --- a/src/Netclaw.Actors/Tools/ShellTool.cs +++ b/src/Netclaw.Actors/Tools/ShellTool.cs @@ -41,10 +41,9 @@ public sealed partial class ShellTool : NetclawTool // The required execution context carries the session default or the // agent's validated _timeout_seconds hint as a semantic value. - // Shell output is mostly verbose noise the model skims, so bound it - // aggressively: small inline head+tail, full output spilled to a session file - // to grep. Content tools (file_read, web_fetch, MCP) keep the larger session - // content budget because the model fetched them to read in full. + // The small inline tail retains recent shell output and the exit status. + // The dispatcher retains the full capture for tool_output_read. + // Content tools use the larger session budget. public override int InlineOutputBudgetChars => 2000; private readonly ToolConfig _config; @@ -147,7 +146,7 @@ private async Task ExecuteCoreAsync( // drain would hang for the grandchild's full life span instead of // returning once the command itself is done. // - // BoundedOutputReader reads into a head+tail window bounded by + // BoundedOutputReader retains the tail within // MaxOutputChars but continues draining after the cap is reached so // the pipe never fills up and deadlocks a still-running child. using var drainCts = CancellationTokenSource.CreateLinkedTokenSource(linkedCts.Token); @@ -228,7 +227,7 @@ private async Task ExecuteCoreAsync( if (stdoutGraceCut || stderrGraceCut) captured += GraceCutMarker; - return $"Exit code: {process.ExitCode}{Environment.NewLine}{captured}"; + return FormatCompletedOutput(captured, process.ExitCode); } } @@ -236,7 +235,7 @@ private async Task ExecuteCoreAsync( /// Streams stdout/stderr as items while the /// process runs. Shell output is live display data only; the parent pipeline /// still treats shell as opaque and enforces a wall-clock budget. The terminal - /// carries the same bounded head+tail result + /// carries the same bounded tail result /// as the non-streaming path. /// public override IAsyncEnumerable ExecuteStreamAsync( @@ -414,8 +413,7 @@ private async Task ExecuteStreamCoreAsync( if (graceCut) captured += GraceCutMarker; - output.TryWrite(new ToolCompletedUpdate( - $"Exit code: {process.ExitCode}{Environment.NewLine}{captured}")); + output.TryWrite(new ToolCompletedUpdate(FormatCompletedOutput(captured, process.ExitCode))); } } catch (Exception ex) when (ex is ToolApprovalRequiredException or ToolCorrectionRequiredException or ToolAccessDeniedException) @@ -563,6 +561,17 @@ private static async Task KillAndDrainAsync(Process process, Task drainStd } } + private string FormatCompletedOutput(string captured, int exitCode) + { + var status = $"Exit code: {exitCode}"; + var result = $"{status}{Environment.NewLine}{captured}"; + + // The dispatcher discards the prefix of a large result. Repeat the status in its retained tail. + return result.Length > InlineOutputBudgetChars + ? $"{result}{Environment.NewLine}{status}" + : result; + } + // Retained for compatibility with tests/benchmark that call it directly; the // main execution path no longer uses this — output is bounded at read time by // BoundedOutputReader. diff --git a/src/Netclaw.Actors/Tools/ToolOutputSpill.cs b/src/Netclaw.Actors/Tools/ToolOutputSpill.cs index 10d426cc8..eb6d5346c 100644 --- a/src/Netclaw.Actors/Tools/ToolOutputSpill.cs +++ b/src/Netclaw.Actors/Tools/ToolOutputSpill.cs @@ -36,10 +36,11 @@ internal static class ToolOutputSpill /// /// Returns unchanged if it fits - /// ; otherwise returns a -char - /// head+tail window plus a steer, having spilled the full result to a session - /// file. The dispatcher resolves from the tool's - /// per-tool override or the session content budget. + /// ; otherwise returns the last + /// chars (the tail) plus a steer, having spilled + /// the full result to a session file. The model pulls the dropped head back + /// with tool_output_read. The dispatcher resolves + /// from the tool's per-tool override or the session content budget. /// public static Task BoundAndSpillAsync( string redactedResult, string? toolCallId, int budget, ToolInvocationContext context, @@ -67,8 +68,8 @@ public static async Task BoundAndSpillAsync( var spill = await TryWriteSpillAsync(spillContent, toolCallId, context, ct); if (spill.FailureReason is { } reason) { - // The model loses the middle of this result and cannot read it again. - // The result text says so; this record tells the operator why. + // The model loses everything before the inline tail and cannot read + // it again. The result text says so; this record tells the operator why. logger.LogWarning( "tool_output_spill_not_retained reason={Reason} sessionId={SessionId} callId={CallId} fullLength={FullLength} budget={Budget} detail={Detail}", reason, @@ -93,7 +94,7 @@ private static async Task TryWriteSpillAsync( return SpillOutcome.NotRetained(SpillFailureReason.UnusableCallId); // A failed (or cancelled) on-disk copy must not fail the tool call — the - // inline head+tail is always returned — so the write is decoupled from the + // inline tail is always returned — so the write is decoupled from the // request's CancellationToken (the body is bounded by the capture ceiling, // so the write is small and fast). The `ct` is kept in the signature for // symmetry / future use. @@ -145,12 +146,13 @@ private static string Compose(string inline, SpillOutcome spill, int fullLength, } else { - // No silent fallback: the model must know that the middle is gone and - // that tool_output_read has nothing for this call. The text must be - // true for each tool, so it names a narrower bound only as an option. - // It names no path, and it must not send the model to the file system. + // No silent fallback: the model must know that everything before the + // inline tail is gone and that tool_output_read has nothing for this + // call. The text must be true for each tool, so it names a narrower + // bound only as an option. It names no path, and it must not send the + // model to the file system. sb.Append("; Netclaw did not keep the full output, so tool_output_read cannot continue this call "); - sb.Append("and the middle is not available. If the source tool has an output bound, call it again "); + sb.Append("and everything before the inline tail is not available. If the source tool has an output bound, call it again "); sb.Append("with a narrower bound. For a skill, read one specific resource with skill_read_resource"); } sb.Append(']');