FileRW: Dateiaktion ohne path abweisen, Hostpfad nicht in Fehlermeldungen
Testinstanz 2026-10-02: write mit destinationPath statt path loeste auf die Wurzel des Arbeitsbereichs auf; die Ausnahme nannte den absoluten Ordner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
0a75afa51b
commit
2699b98781
@@ -149,6 +149,9 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
return ToolResult.Fail($"Access denied: Action '{action}' is not allowed in workspace '{area.Name}' for this agent.");
|
return ToolResult.Fail($"Access denied: Action '{action}' is not allowed in workspace '{area.Name}' for this agent.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (MissingPath(action, input) is { } missing)
|
||||||
|
return ToolResult.Fail(missing);
|
||||||
|
|
||||||
// A workspace with a member list comes into being when it is first used.
|
// A workspace with a member list comes into being when it is first used.
|
||||||
if (!area.IsPersonal && area.Name != WorkspaceLayout.Shared)
|
if (!area.IsPersonal && area.Name != WorkspaceLayout.Shared)
|
||||||
Directory.CreateDirectory(rootPath);
|
Directory.CreateDirectory(rootPath);
|
||||||
@@ -177,10 +180,26 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
context.Logger.LogError(ex, "Error during FileRW action {Action}", action);
|
context.Logger.LogError(ex, "Error during FileRW action {Action}", action);
|
||||||
return ToolResult.Fail($"Error: {ex.Message}");
|
// The host's folder layout is none of the model's business — the workspace name says enough.
|
||||||
|
return ToolResult.Fail($"Error: {ex.Message.Replace(rootPath, $"<{area.Name}>", StringComparison.Ordinal)}");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Actions on one file need its path. Without one, the path resolved to the workspace root:
|
||||||
|
/// a write then tried to move its temp file onto the folder itself (test instance, 2026-10-02,
|
||||||
|
/// a model that sent <c>destinationPath</c> instead of <c>path</c>).
|
||||||
|
/// </summary>
|
||||||
|
internal static string? MissingPath(string action, JsonElement input)
|
||||||
|
{
|
||||||
|
if (action is not ("read" or "write" or "append" or "delete" or "copy") || !string.IsNullOrWhiteSpace(RelativerPfad(input)))
|
||||||
|
return null;
|
||||||
|
var hint = action != "copy" && input.ValueKind == JsonValueKind.Object && input.TryGetProperty("destinationPath", out _)
|
||||||
|
? " 'destinationPath' belongs to copy only — name the file in 'path'."
|
||||||
|
: "";
|
||||||
|
return $"'path' is required for {action}.{hint}";
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// #579: Rechte und Pfad pruefen, bevor ein Aufruf in die Freigabeliste kommt. Dieselben
|
/// #579: Rechte und Pfad pruefen, bevor ein Aufruf in die Freigabeliste kommt. Dieselben
|
||||||
/// Pruefungen wie in <see cref="ExecuteAsync"/>, ohne etwas anzufassen.
|
/// Pruefungen wie in <see cref="ExecuteAsync"/>, ohne etwas anzufassen.
|
||||||
@@ -203,6 +222,9 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
if (!IsActionAllowed(action, area))
|
if (!IsActionAllowed(action, area))
|
||||||
return $"Access denied: Action '{action}' is not allowed in workspace '{area.Name}' for this agent.";
|
return $"Access denied: Action '{action}' is not allowed in workspace '{area.Name}' for this agent.";
|
||||||
|
|
||||||
|
if (MissingPath(action, input) is { } missing)
|
||||||
|
return missing;
|
||||||
|
|
||||||
if (action != "delete" || string.IsNullOrWhiteSpace(area.Root))
|
if (action != "delete" || string.IsNullOrWhiteSpace(area.Root))
|
||||||
return null;
|
return null;
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
using System.Text.Json;
|
||||||
|
using ClawdDotNet.Core.Tools;
|
||||||
|
using ClawdDotNet.Tools.FileRW;
|
||||||
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
|
using Shouldly;
|
||||||
|
|
||||||
|
namespace ClawdDotNet.Tools.Tests.FileRW;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Test instance, 2026-10-02: a model sent <c>destinationPath</c> instead of <c>path</c> to write.
|
||||||
|
/// The path resolved to the workspace root, the move onto the folder failed, and the error named
|
||||||
|
/// the host's absolute folder. A missing path is the model's mistake and says so plainly.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class MissingPathTests : IDisposable
|
||||||
|
{
|
||||||
|
private readonly string _workspace = Path.Combine(Path.GetTempPath(), "clawd-tests", Guid.NewGuid().ToString("N"), "Workspace");
|
||||||
|
|
||||||
|
public MissingPathTests() => Directory.CreateDirectory(_workspace);
|
||||||
|
|
||||||
|
public void Dispose()
|
||||||
|
{
|
||||||
|
try { Directory.Delete(Path.GetDirectoryName(_workspace)!, recursive: true); }
|
||||||
|
catch { /* cleanup is secondary */ }
|
||||||
|
}
|
||||||
|
|
||||||
|
private AgentToolContext Context() => new("desk", "inst", new Dictionary<string, object?>(), new NullStateStore(), NullLogger.Instance,
|
||||||
|
CancellationToken.None, WorkspacePath: _workspace);
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("""{"action":"write","destinationPath":"wetter/x.txt","content":"x"}""", "destinationPath")]
|
||||||
|
[InlineData("""{"action":"write","content":"x"}""", "'path' is required for write")]
|
||||||
|
[InlineData("""{"action":"append","path":" ","content":"x"}""", "'path' is required for append")]
|
||||||
|
[InlineData("""{"action":"read"}""", "'path' is required for read")]
|
||||||
|
public async Task A_file_action_without_a_path_is_refused_without_naming_the_host_folder(string json, string expected)
|
||||||
|
{
|
||||||
|
var input = JsonDocument.Parse(json).RootElement;
|
||||||
|
var tool = new FileRWTool();
|
||||||
|
|
||||||
|
var result = await tool.ExecuteAsync(input, Context(), CancellationToken.None);
|
||||||
|
|
||||||
|
result.Success.ShouldBeFalse();
|
||||||
|
result.ErrorMessage.ShouldContain(expected);
|
||||||
|
result.ErrorMessage!.ShouldNotContain(_workspace);
|
||||||
|
tool.Preflight(input, Context()).ShouldNotBeNull("the check runs before staging as well");
|
||||||
|
Directory.EnumerateFileSystemEntries(_workspace).ShouldBeEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Listing_the_root_still_needs_no_path()
|
||||||
|
{
|
||||||
|
File.WriteAllText(Path.Combine(_workspace, "a.txt"), "x");
|
||||||
|
|
||||||
|
var result = await new FileRWTool().ExecuteAsync(JsonDocument.Parse("""{"action":"list"}""").RootElement, Context(), CancellationToken.None);
|
||||||
|
|
||||||
|
result.Success.ShouldBeTrue(result.ErrorMessage);
|
||||||
|
result.Content.ShouldContain("a.txt");
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user