fix: address code review findings from #1333 redaction PR

- Redact tool results in SessionLogActor before writing to session.log
  so file_read's SuppressOutputRedaction doesn't leak secrets to the
  observability log on disk
- Tighten FileEditTool Content mutual-exclusivity guard to also reject
  NewString and ReplaceAll (not just OldString) when Content is supplied
- Always redact streaming ToolActivityUpdate chunks regardless of
  SuppressOutputRedaction — activity chunks are progressive display, not
  content the model writes back
- Fix stale doc comment in ToolOutputSpill that claimed inputs are
  always pre-redacted
- Normalize error message casing in FileWriteTool ('path' → 'Path')

Author: Aaronontheweb

src/Netclaw.Actors.Tests/Tools/FileEditToolTests.cs

@@ -228,6 +228,30 @@ public async Task Content_and_OldString_are_mutually_exclusive()
         Assert.False(File.Exists(filePath));
     }
 
+    [Fact]
+    public async Task Content_and_NewString_are_mutually_exclusive()
+    {
+        var filePath = Path.Combine(_dir.Path, "conflict2.txt");
+
+        var result = await _tool.ExecuteAsync(ToolInput.Create(
+            "Path", filePath, "Content", "full", "NewString", "new"), CreatePersonalContext(), CancellationToken.None);
+
+        Assert.Contains("mutually exclusive", result);
+        Assert.False(File.Exists(filePath));
+    }
+
+    [Fact]
+    public async Task Content_and_ReplaceAll_are_mutually_exclusive()
+    {
+        var filePath = Path.Combine(_dir.Path, "conflict3.txt");
+
+        var result = await _tool.ExecuteAsync(ToolInput.Create(
+            "Path", filePath, "Content", "full", "ReplaceAll", true), CreatePersonalContext(), CancellationToken.None);
+
+        Assert.Contains("mutually exclusive", result);
+        Assert.False(File.Exists(filePath));
+    }
+
     private ToolExecutionContext CreatePersonalContext()
         => new("signalr/thread-1", _sessionDir)
         {

src/Netclaw.Actors/Sessions/SessionLogActor.cs

@@ -7,6 +7,7 @@
 using Akka.Event;
 using Netclaw.Actors.Protocol;
 using Netclaw.Actors.SubAgents;
+using Netclaw.Security;
 
 namespace Netclaw.Actors.Sessions;
 
@@ -107,7 +108,7 @@ private void OnOutput(SessionOutput output)
             {
                 TextOutput text => $"Assistant: {TextTruncation.EllipsisAppend(text.Text, 1000)}",
                 ToolCallOutput toolCall => FormatToolCall(toolCall),
-                ToolResultOutput toolResult => $"Tool result: {toolResult.ToolName} (call={toolResult.CallId}) → {TextTruncation.EllipsisAppend(toolResult.Result, 1000)}",
+                ToolResultOutput toolResult => $"Tool result: {toolResult.ToolName} (call={toolResult.CallId}) → {TextTruncation.EllipsisAppend(SecretOutputRedactor.Redact(toolResult.Result), 1000)}",
                 ThinkingOutput thinking => $"Thinking: {TextTruncation.EllipsisAppend(thinking.Text, 1000)}",
                 ThinkingDeltaOutput thinkingDelta => $"Thinking delta: {TextTruncation.EllipsisAppend(thinkingDelta.Delta, 1000)}",
                 UsageOutput usage => $"Usage: in={usage.InputTokens} out={usage.OutputTokens} cached={usage.CachedInputTokens} reasoning={usage.ReasoningTokens} context={usage.UsagePercent:P0}",

src/Netclaw.Actors/Tools/DispatchingToolExecutor.cs

@@ -149,12 +149,7 @@ public async IAsyncEnumerable<ToolCallUpdate> ExecuteStreamAsync(
                     yield return new ToolCompletedUpdate(modelResult);
                     break;
                 case ToolActivityUpdate { OutputChunk: not null } activity:
-                    yield return activity with
-                    {
-                        OutputChunk = tool.SuppressOutputRedaction
-                            ? activity.OutputChunk
-                            : SecretOutputRedactor.Redact(activity.OutputChunk)
-                    };
+                    yield return activity with { OutputChunk = SecretOutputRedactor.Redact(activity.OutputChunk) };
                     break;
                 default:
                     yield return update;

src/Netclaw.Actors/Tools/FileEditTool.cs

@@ -58,8 +58,8 @@ protected override async Task<string> ExecuteAsync(Params args, ToolExecutionCon
         // Full-write mode: Content is supplied without OldString
         if (args.Content is not null)
         {
-            if (args.OldString is not null)
-                return "Error: 'Content' and 'OldString' are mutually exclusive. " +
+            if (args.OldString is not null || args.NewString is not null || args.ReplaceAll is not null)
+                return "Error: 'Content' and 'OldString'/'NewString'/'ReplaceAll' are mutually exclusive. " +
                        "Use Content for a full file write, or OldString/NewString for targeted replacement.";
 
             return await WriteFileAsync(args.Path, args.Content, context, ct);

src/Netclaw.Actors/Tools/FileWriteTool.cs

@@ -45,7 +45,7 @@ protected override Task<string> ExecuteAsync(Params args, CancellationToken ct)
     protected override Task<string> ExecuteAsync(Params args, ToolExecutionContext context, CancellationToken ct)
     {
         if (string.IsNullOrWhiteSpace(args.Path))
-            return Task.FromResult("Error: 'path' parameter is required.");
+            return Task.FromResult("Error: 'Path' parameter is required.");
 
         return _editTool.WriteFileAsync(args.Path, args.Content, context, ct);
     }

src/Netclaw.Actors/Tools/ToolOutputSpill.cs

@@ -10,7 +10,7 @@
 namespace Netclaw.Actors.Tools;
 
 /// <summary>
-/// Bounds an already-redacted tool result to the inline budget <c>N</c>
+/// Bounds a tool result to the inline budget <c>N</c>
 /// (<see cref="ToolExecutionContext.MaxInlineToolResultChars"/>) and, when it
 /// exceeds <c>N</c>, spills the full result to
 /// <c>{SessionDirectory}/tool-calls/{toolCallId}.log</c> and steers the model to
@@ -19,12 +19,11 @@ namespace Netclaw.Actors.Tools;
 /// <remarks>
 /// Called from <c>DispatchingToolExecutor</c> for <i>every</i> tool, right after
 /// the central redaction — so bounding + spill happen once, uniformly, for the
-/// main session and sub-agents alike (both funnel through the dispatcher), and
-/// the spilled file is redacted for free. Tools only bound their own <i>capture</i>
-/// for memory safety; they do not window or spill. The input here is already
-/// redacted, so this method does NOT redact again. <c>file_read</c> keeps its
-/// result at or under <c>N</c> and steers to the original file, so it never spills
-/// here (its content is its own backing store).
+/// main session and sub-agents alike (both funnel through the dispatcher). Tools
+/// only bound their own <i>capture</i> for memory safety; they do not window or
+/// spill. The two-param overload accepts separate model-facing and spill content
+/// so that tools with <c>SuppressOutputRedaction</c> can return raw results to the
+/// model while still writing redacted content to the spill file on disk.
 /// </remarks>
 internal static class ToolOutputSpill
 {