From 92b2075e14280fe0a10694c15012547cc0c82974 Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 1 Oct 2026 09:04:38 +0200 Subject: [PATCH] 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 --- src/ClawdDotNet.Tools.FileRW/FileRWTool.cs | 103 +++++++++++++++++- .../FileRW/WorkspaceInPathTests.cs | 97 +++++++++++++++++ 2 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 tests/ClawdDotNet.Tools.Tests/FileRW/WorkspaceInPathTests.cs diff --git a/src/ClawdDotNet.Tools.FileRW/FileRWTool.cs b/src/ClawdDotNet.Tools.FileRW/FileRWTool.cs index 799ca57..5459e70 100644 --- a/src/ClawdDotNet.Tools.FileRW/FileRWTool.cs +++ b/src/ClawdDotNet.Tools.FileRW/FileRWTool.cs @@ -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}"; + + /// + /// #718: A path may name its workspace — shared:betrieb/protokoll.md, the form FTP and + /// RocketChat take. Agents carry it over to FileRW, and every refusal cost them a step. The + /// prefix becomes the workspace argument; it must agree with one given, and a list of + /// paths names one workspace. Returns the refusal, or null. + /// + 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(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(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(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) ─── /// The folder in the shared workspace that holds the task files. @@ -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 /// Seit #684 gibt es mehr als zwei: gesucht wird in allen, die der Agent kennt. /// 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 { 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(); @@ -702,7 +793,7 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe } catch (UnauthorizedAccessException ex) { - return ([], $"Access denied: {ex.Message}"); + return ([], Denied(ex.Message)); } } diff --git a/tests/ClawdDotNet.Tools.Tests/FileRW/WorkspaceInPathTests.cs b/tests/ClawdDotNet.Tools.Tests/FileRW/WorkspaceInPathTests.cs new file mode 100644 index 0000000..c09c13b --- /dev/null +++ b/tests/ClawdDotNet.Tools.Tests/FileRW/WorkspaceInPathTests.cs @@ -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; + +/// +/// Found in operation on 0.1.16: an agent wrote shared:betrieb/protokoll.md — 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. +/// +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 Run(object input) + { + var context = new AgentToolContext("redaktion", "inst", + new Dictionary { ["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"); + } +}