FileRW: Arbeitsbereich im Pfad annehmen, Muster-Hinweis nur bei Treffern (#757)

- "shared:betrieb/protokoll.md" wird zu workspace + Pfad (wie FTP/RocketChat seit #718); muss zu einem angegebenen workspace passen, eine Pfadliste nennt nur einen Bereich - read_many mit Muster: Hinweis auf einen anderen Bereich nur, wenn dort Dateien passen; ohne Pfad und Muster kein Hinweis (die Wurzel gibt es ueberall) - verweigerter Pfad: kein doppeltes "Access denied", Warnung statt ERR mit Stacktrace - Tests: WorkspaceInPathTests (6) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-01 09:04:38 +02:00
parent b6405a57d3
commit 92b2075e14
2 changed files with 194 additions and 6 deletions
@@ -129,6 +129,10 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
        var action = input.GetProperty("action").GetString()
            ?? throw new ArgumentException("'action' is required");

+        // #718: "shared:betrieb/protokoll.md" names the workspace in the path — as in FTP and RocketChat.
+        if (WorkspaceFromPath(ref input) is { } prefixProblem)
+            return ToolResult.Fail(prefixProblem);
+
        var requested = input.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String ? w.GetString() : null;
        if (ResolveArea(requested, context) is not { } area)
            return ToolResult.Fail(UnknownWorkspace(requested!, context));
@@ -164,6 +168,12 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
                _ => ToolResult.Fail($"Unknown action: {action}")
            };
        }
+        catch (UnauthorizedAccessException ex)
+        {
+            // A refused path is the agent's mistake, not the tool's — no stack trace in the error log.
+            context.Logger.LogWarning("FileRW {Action} refused: {Reason}", action, ex.Message);
+            return ToolResult.Fail(Denied(ex.Message));
+        }
        catch (Exception ex)
        {
            context.Logger.LogError(ex, "Error during FileRW action {Action}", action);
@@ -182,6 +192,8 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
            return null;

        var action = a.GetString() ?? "";
+        if (WorkspaceFromPath(ref input) is { } prefixProblem)
+            return prefixProblem;
        var requested = input.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String
            ? w.GetString() : null;

@@ -205,12 +217,79 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
        catch (Exception ex) when (ex is not OperationCanceledException)
        {
            // Dieselbe Pfadpruefung wirft bei der Ausfuehrung - dort wird daraus ein Fehler.
-            return $"Access denied: {ex.Message}";
+            return Denied(ex.Message);
        }

        return null;
    }

+    private static string Denied(string message)
+        => message.StartsWith("Access denied", StringComparison.Ordinal) ? message : $"Access denied: {message}";
+
+    /// <summary>
+    /// #718: A path may name its workspace — <c>shared:betrieb/protokoll.md</c>, the form FTP and
+    /// RocketChat take. Agents carry it over to FileRW, and every refusal cost them a step. The
+    /// prefix becomes the <c>workspace</c> argument; it must agree with one given, and a list of
+    /// paths names one workspace. Returns the refusal, or <c>null</c>.
+    /// </summary>
+    internal static string? WorkspaceFromPath(ref JsonElement input)
+    {
+        if (input.ValueKind != JsonValueKind.Object)
+            return null;
+
+        // The relative path, or null without a prefix; "\0" when it names a second workspace.
+        string? named = null;
+        string? Strip(string path)
+        {
+            if (path.IndexOf(':') <= 0)
+                return null;
+            var (workspace, relative) = NamedWorkspaces.Split(path);
+            if (relative.Length == path.Length)
+                return null;
+            if (named is not null && !Same(named, workspace))
+                return "\0";
+            named = workspace;
+            return relative;
+        }
+
+        var node = System.Text.Json.Nodes.JsonNode.Parse(input.GetRawText())!.AsObject();
+        var changed = false;
+
+        if (node["path"] is System.Text.Json.Nodes.JsonValue pv && pv.TryGetValue<string>(out var single) && Strip(single) is { } rel)
+        {
+            node["path"] = rel;
+            changed = true;
+        }
+        if (node["paths"] is System.Text.Json.Nodes.JsonArray list)
+        {
+            for (var i = 0; i < list.Count; i++)
+            {
+                if (list[i] is not System.Text.Json.Nodes.JsonValue v || !v.TryGetValue<string>(out var entry))
+                    continue;
+                var stripped = Strip(entry);
+                if (stripped == "\0")
+                    return "'paths' names more than one workspace. Read from one workspace per call (parameter 'workspace').";
+                if (stripped is not null)
+                {
+                    list[i] = stripped;
+                    changed = true;
+                }
+            }
+        }
+        if (!changed || named is null)
+            return null;
+
+        if (node["workspace"] is System.Text.Json.Nodes.JsonValue wv && wv.TryGetValue<string>(out var given)
+            && !string.IsNullOrWhiteSpace(given) && !Same(given, named))
+            return $"The path names the workspace '{named}', but 'workspace' says '{given}'. Give one of them.";
+
+        node["workspace"] = named;
+        input = JsonSerializer.SerializeToElement(node);
+        return null;
+    }
+
+    private static bool Same(string a, string b) => string.Equals(a, b, StringComparison.OrdinalIgnoreCase);
+
    // ─── Workspaces (#684) ───

    /// <summary>The folder in the shared workspace that holds the task files.</summary>
@@ -436,6 +515,8 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe

            if (action is not ("write" or "append"))
                return ProbeFinding.Unknown($"Nothing to probe for action '{action ?? "?"}'.");
+            if (WorkspaceFromPath(ref arguments) is { } prefixProblem)
+                return ProbeFinding.Unknown(prefixProblem);

            var requested = arguments.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String ? w.GetString() : null;
            if (ResolveArea(requested, context) is not { Root: { Length: > 0 } root } area)
@@ -532,8 +613,12 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
    /// <para>Seit #684 gibt es mehr als zwei: gesucht wird in allen, die der Agent kennt.</para>
    /// </summary>
    private string AndererWorkspaceHinweis(
-        string action, Area area, string? relativePath, AgentToolContext context, bool verzeichnis)
+        string action, Area area, string? relativePath, AgentToolContext context, bool verzeichnis, string? muster = null)
    {
+        // The root of a workspace exists everywhere — pointing there would send the agent in circles.
+        if (string.IsNullOrWhiteSpace(relativePath) && string.IsNullOrWhiteSpace(muster))
+            return "";
+
        var candidates = new List<string> { WorkspaceLayout.Personal, WorkspaceLayout.Shared };
        if (context.Organisation is { } org)
            candidates.AddRange(org.WorkspacesFor(context.AgentId).Select(g => g.Workspace.Id));
@@ -548,8 +633,13 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
                var pfad = WorkspacePath.Resolve(Path.GetFullPath(root), relativePath, other.Name);
                if (IsBlocked(pfad, other, context))
                    continue;
-                if (verzeichnis ? Directory.Exists(pfad) : File.Exists(pfad))
-                    return $" The path exists in the {other.Name} workspace — call with workspace: \"{other.Name}\".";
+                var dort = verzeichnis
+                    ? Directory.Exists(pfad) && (string.IsNullOrWhiteSpace(muster) || Directory.EnumerateFiles(pfad, muster).Any())
+                    : File.Exists(pfad);
+                if (dort)
+                    return string.IsNullOrWhiteSpace(muster)
+                        ? $" The path exists in the {other.Name} workspace — call with workspace: \"{other.Name}\"."
+                        : $" Matching files exist in the {other.Name} workspace — call with workspace: \"{other.Name}\".";
            }
            catch
            {
@@ -618,7 +708,8 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
        {
            return ToolResult.Fail(
                $"No matching file found in {area.Name} workspace."
-                + AndererWorkspaceHinweis("read_many", area, RelativerPfad(input), context, verzeichnis: true));
+                + AndererWorkspaceHinweis("read_many", area, RelativerPfad(input), context, verzeichnis: true,
+                    muster: input.TryGetProperty("pattern", out var m) && m.ValueKind == JsonValueKind.String ? m.GetString() : null));
        }

        var uebersprungen = new List<string>();
@@ -702,7 +793,7 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
        }
        catch (UnauthorizedAccessException ex)
        {
-            return ([], $"Access denied: {ex.Message}");
+            return ([], Denied(ex.Message));
        }
    }

@@ -0,0 +1,97 @@
+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>
+/// Found in operation on 0.1.16: an agent wrote <c>shared:betrieb/protokoll.md</c> — the form
+/// FTP and RocketChat take (#718) — and FileRW refused it every morning. And a pattern that
+/// matched nowhere sent the agent back and forth between two workspaces.
+/// </summary>
+public sealed class WorkspaceInPathTests : IDisposable
+{
+    private readonly string _agents = Path.Combine(Path.GetTempPath(), "clawd-tests", Guid.NewGuid().ToString("N"), "Agents");
+    private readonly FileRWTool _tool = new();
+
+    public WorkspaceInPathTests()
+    {
+        Directory.CreateDirectory(Path.Combine(_agents, "SharedWorkspace", "betrieb"));
+        File.WriteAllText(Path.Combine(_agents, "SharedWorkspace", "betrieb", "protokoll.md"), "shared log");
+        Directory.CreateDirectory(Path.Combine(_agents, "redaktion", "Workspace"));
+        File.WriteAllText(Path.Combine(_agents, "redaktion", "Workspace", "notiz.md"), "personal note");
+    }
+
+    public void Dispose()
+    {
+        try { Directory.Delete(Path.GetDirectoryName(_agents)!, recursive: true); }
+        catch { /* cleaning up is secondary */ }
+    }
+
+    private Task<ToolResult> Run(object input)
+    {
+        var context = new AgentToolContext("redaktion", "inst",
+            new Dictionary<string, object?> { ["sharedAccessLevel"] = "ReadWrite" },
+            new NullStateStore(), NullLogger.Instance, CancellationToken.None,
+            WorkspacePath: Path.Combine(_agents, "redaktion", "Workspace"), SharedWorkspacePath: Path.Combine(_agents, "SharedWorkspace"));
+        return _tool.ExecuteAsync(JsonSerializer.SerializeToElement(input), context, CancellationToken.None);
+    }
+
+    [Fact]
+    public async Task A_path_that_names_its_workspace_is_read_from_there()
+    {
+        var result = await Run(new { action = "read", path = "shared:betrieb/protokoll.md" });
+
+        result.Success.ShouldBeTrue(result.ErrorMessage);
+        result.Content.ShouldContain("shared log");
+    }
+
+    [Fact]
+    public async Task A_list_of_paths_from_one_workspace_is_read_from_there()
+    {
+        var result = await Run(new { action = "read_many", paths = new[] { "shared:betrieb/protokoll.md" } });
+
+        result.Success.ShouldBeTrue(result.ErrorMessage);
+        result.Content.ShouldContain("shared log");
+    }
+
+    [Fact]
+    public async Task Two_workspaces_in_one_list_or_a_contradiction_are_refused_plainly()
+    {
+        (await Run(new { action = "read_many", paths = new[] { "shared:betrieb/protokoll.md", "personal:notiz.md" } }))
+            .ErrorMessage!.ShouldContain("more than one workspace");
+        (await Run(new { action = "read", workspace = "personal", path = "shared:betrieb/protokoll.md" }))
+            .ErrorMessage!.ShouldContain("'workspace' says 'personal'");
+    }
+
+    [Fact]
+    public async Task A_colon_that_names_no_workspace_is_still_refused_and_only_once_denied()
+    {
+        var result = await Run(new { action = "read", path = "notiz.md:versteckt" });
+
+        result.Success.ShouldBeFalse();
+        result.ErrorMessage!.ShouldStartWith("Access denied: Path contains");
+        result.ErrorMessage.ShouldNotContain("Access denied: Access denied");
+    }
+
+    [Fact]
+    public async Task A_pattern_that_matches_nowhere_points_nowhere()
+    {
+        var result = await Run(new { action = "read_many", workspace = "shared", pattern = "berlin_*.txt" });
+
+        result.Success.ShouldBeFalse();
+        result.ErrorMessage!.ShouldNotContain("workspace:", customMessage: "nothing matches in personal either — no hint");
+    }
+
+    [Fact]
+    public async Task A_pattern_that_matches_elsewhere_says_where()
+    {
+        File.WriteAllText(Path.Combine(_agents, "redaktion", "Workspace", "berlin_1.txt"), "x");
+
+        var result = await Run(new { action = "read_many", workspace = "shared", pattern = "berlin_*.txt" });
+
+        result.ErrorMessage!.ShouldContain("Matching files exist in the personal workspace");
+    }
+}