Merge fix/filerw-workspace-in-path (#757)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -129,6 +129,10 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
var action = input.GetProperty("action").GetString()
|
var action = input.GetProperty("action").GetString()
|
||||||
?? throw new ArgumentException("'action' is required");
|
?? 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;
|
var requested = input.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String ? w.GetString() : null;
|
||||||
if (ResolveArea(requested, context) is not { } area)
|
if (ResolveArea(requested, context) is not { } area)
|
||||||
return ToolResult.Fail(UnknownWorkspace(requested!, context));
|
return ToolResult.Fail(UnknownWorkspace(requested!, context));
|
||||||
@@ -164,6 +168,12 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
_ => ToolResult.Fail($"Unknown action: {action}")
|
_ => 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)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
context.Logger.LogError(ex, "Error during FileRW action {Action}", action);
|
context.Logger.LogError(ex, "Error during FileRW action {Action}", action);
|
||||||
@@ -182,6 +192,8 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
return null;
|
return null;
|
||||||
|
|
||||||
var action = a.GetString() ?? "";
|
var action = a.GetString() ?? "";
|
||||||
|
if (WorkspaceFromPath(ref input) is { } prefixProblem)
|
||||||
|
return prefixProblem;
|
||||||
var requested = input.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String
|
var requested = input.TryGetProperty("workspace", out var w) && w.ValueKind == JsonValueKind.String
|
||||||
? w.GetString() : null;
|
? w.GetString() : null;
|
||||||
|
|
||||||
@@ -205,12 +217,79 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
catch (Exception ex) when (ex is not OperationCanceledException)
|
catch (Exception ex) when (ex is not OperationCanceledException)
|
||||||
{
|
{
|
||||||
// Dieselbe Pfadpruefung wirft bei der Ausfuehrung - dort wird daraus ein Fehler.
|
// Dieselbe Pfadpruefung wirft bei der Ausfuehrung - dort wird daraus ein Fehler.
|
||||||
return $"Access denied: {ex.Message}";
|
return Denied(ex.Message);
|
||||||
}
|
}
|
||||||
|
|
||||||
return null;
|
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) ───
|
// ─── Workspaces (#684) ───
|
||||||
|
|
||||||
/// <summary>The folder in the shared workspace that holds the task files.</summary>
|
/// <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"))
|
if (action is not ("write" or "append"))
|
||||||
return ProbeFinding.Unknown($"Nothing to probe for action '{action ?? "?"}'.");
|
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;
|
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)
|
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>
|
/// <para>Seit #684 gibt es mehr als zwei: gesucht wird in allen, die der Agent kennt.</para>
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private string AndererWorkspaceHinweis(
|
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 };
|
var candidates = new List<string> { WorkspaceLayout.Personal, WorkspaceLayout.Shared };
|
||||||
if (context.Organisation is { } org)
|
if (context.Organisation is { } org)
|
||||||
candidates.AddRange(org.WorkspacesFor(context.AgentId).Select(g => g.Workspace.Id));
|
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);
|
var pfad = WorkspacePath.Resolve(Path.GetFullPath(root), relativePath, other.Name);
|
||||||
if (IsBlocked(pfad, other, context))
|
if (IsBlocked(pfad, other, context))
|
||||||
continue;
|
continue;
|
||||||
if (verzeichnis ? Directory.Exists(pfad) : File.Exists(pfad))
|
var dort = verzeichnis
|
||||||
return $" The path exists in the {other.Name} workspace — call with workspace: \"{other.Name}\".";
|
? 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
|
catch
|
||||||
{
|
{
|
||||||
@@ -618,7 +708,8 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
{
|
{
|
||||||
return ToolResult.Fail(
|
return ToolResult.Fail(
|
||||||
$"No matching file found in {area.Name} workspace."
|
$"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>();
|
var uebersprungen = new List<string>();
|
||||||
@@ -702,7 +793,7 @@ public sealed class FileRWTool : IAgentTool, IToolStateProbe
|
|||||||
}
|
}
|
||||||
catch (UnauthorizedAccessException ex)
|
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");
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user