From 72afae0724c2ccd4606eeaf1e4e3397fe5f411fb Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Tue, 2 Jun 2026 17:48:34 -0700 Subject: [PATCH] feat(mcp): add file delete tool --- cmd/afs/afs_mcp.go | 29 ++++ cmd/afs/afs_mcp_test.go | 119 +++++++++++++++- cmd/afs/embedded/skills/afs/SKILL.md | 23 +++- docs/reference/mcp.md | 13 +- internal/controlplane/mcp_hosted.go | 46 ++++++- internal/controlplane/mcp_hosted_test.go | 150 +++++++++++++++++++++ internal/controlplane/mcp_profiles.go | 1 + internal/controlplane/mcp_profiles_test.go | 19 +++ 8 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 internal/controlplane/mcp_profiles_test.go diff --git a/cmd/afs/afs_mcp.go b/cmd/afs/afs_mcp.go index 954e9d4..ac94f31 100644 --- a/cmd/afs/afs_mcp.go +++ b/cmd/afs/afs_mcp.go @@ -382,6 +382,18 @@ func (s *afsMCPServer) Tools(_ context.Context) []mcpproto.Tool { "required": []string{"path"}, }, }, + { + Name: "file_delete", + Description: "Delete one file, symlink, or empty directory from a workspace. Refuses root and non-empty directories.", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "workspace": map[string]string{"type": "string", "description": "Workspace name (defaults to current workspace)"}, + "path": map[string]string{"type": "string", "description": "Absolute workspace path, for example /src/main.go"}, + }, + "required": []string{"path"}, + }, + }, { Name: "file_lines", Description: "Read a specific line range from a text file. Use this instead of file_read when the file is large or you only need a slice. This is for text files only. Do not use it for directory listing or cross-file search. Paths must be absolute inside the workspace, for example /src/main.go.", @@ -637,6 +649,8 @@ func (s *afsMCPServer) CallTool(ctx context.Context, name string, args map[strin value, err = s.toolFileRestoreVersion(ctx, args) case "file_undelete": value, err = s.toolFileUndelete(ctx, args) + case "file_delete": + value, err = s.toolFileDelete(ctx, args) case "file_lines": value, err = s.toolFileLines(ctx, args) case "file_list": @@ -1509,6 +1523,21 @@ func (s *afsMCPServer) toolFileDeleteLines(ctx context.Context, args map[string] }) } +func (s *afsMCPServer) toolFileDelete(ctx context.Context, args map[string]any) (any, error) { + return s.mutateWorkspaceFile(ctx, args, func(ctx context.Context, fsClient client.Client, normalizedPath string, stat *client.StatResult) (map[string]any, error) { + if stat == nil { + return nil, os.ErrNotExist + } + if err := fsClient.Rm(ctx, normalizedPath); err != nil { + return nil, err + } + return map[string]any{ + "operation": "delete", + "kind": stat.Type, + }, nil + }) +} + func (s *afsMCPServer) toolFilePatch(ctx context.Context, args map[string]any) (any, error) { var input mcpFilePatchInput if err := decodeMCPArgs(args, &input); err != nil { diff --git a/cmd/afs/afs_mcp_test.go b/cmd/afs/afs_mcp_test.go index ff9fdf1..7deb228 100644 --- a/cmd/afs/afs_mcp_test.go +++ b/cmd/afs/afs_mcp_test.go @@ -17,6 +17,7 @@ import ( "github.com/alicebob/miniredis/v2" "github.com/redis/agent-filesystem/internal/controlplane" "github.com/redis/agent-filesystem/internal/mcptools" + mountclient "github.com/redis/agent-filesystem/mount/client" ) func TestAFSMCPServerInitializeAndToolsList(t *testing.T) { @@ -71,15 +72,23 @@ func TestAFSMCPServerInitializeAndToolsList(t *testing.T) { if len(tools) == 0 { t.Fatal("tools/list returned no tools") } + foundFileDelete := false for _, rawTool := range tools { tool, ok := rawTool.(map[string]any) if !ok { continue } - if name, _ := tool["name"].(string); name == "file_delete_version" { + name, _ := tool["name"].(string) + if name == "file_delete" { + foundFileDelete = true + } + if name == "file_delete_version" { t.Fatalf("tools/list unexpectedly exposes %q", name) } } + if !foundFileDelete { + t.Fatal("tools/list did not expose file_delete") + } } func TestAFSMCPFileWriteLeavesWorkspaceDirtyAndReadReturnsContent(t *testing.T) { @@ -626,6 +635,114 @@ func TestAFSMCPFilePatchAppliesStructuredEdits(t *testing.T) { } } +func TestAFSMCPFileDeleteRemovesFile(t *testing.T) { + t.Helper() + + server, closeFn := setupAFSMCPTestServer(t) + defer closeFn() + + if _, err := server.toolFileWrite(context.Background(), map[string]any{ + "path": "/docs/remove-me.md", + "content": "delete me\n", + }); err != nil { + t.Fatalf("toolFileWrite(remove-me.md) returned error: %v", err) + } + + result := server.callTool(context.Background(), "file_delete", map[string]any{ + "path": "/docs/remove-me.md", + }) + if result.IsError { + t.Fatalf("file_delete returned error result: %+v", result) + } + + var payload map[string]any + if err := decodeStructuredContent(result.StructuredContent, &payload); err != nil { + t.Fatalf("decodeStructuredContent(delete) returned error: %v", err) + } + if got, _ := payload["operation"].(string); got != "delete" { + t.Fatalf("operation = %#v, want %q", payload["operation"], "delete") + } + + readResult := server.callTool(context.Background(), "file_read", map[string]any{ + "path": "/docs/remove-me.md", + }) + if !readResult.IsError { + t.Fatalf("file_read after delete succeeded: %+v", readResult) + } +} + +func TestAFSMCPFileDeleteRemovesEmptyDirectory(t *testing.T) { + t.Helper() + + ctx := context.Background() + server, closeFn := setupAFSMCPTestServer(t) + defer closeFn() + + fsKey, _, _, err := server.store.ensureWorkspaceRoot(ctx, "repo") + if err != nil { + t.Fatalf("ensureWorkspaceRoot() returned error: %v", err) + } + fsClient := mountclient.New(server.store.rdb, fsKey) + if err := fsClient.Mkdir(ctx, "/docs/empty"); err != nil { + t.Fatalf("Mkdir(/docs/empty) returned error: %v", err) + } + + result := server.callTool(ctx, "file_delete", map[string]any{ + "path": "/docs/empty", + }) + if result.IsError { + t.Fatalf("file_delete returned error result: %+v", result) + } + + var payload map[string]any + if err := decodeStructuredContent(result.StructuredContent, &payload); err != nil { + t.Fatalf("decodeStructuredContent(delete) returned error: %v", err) + } + if got, _ := payload["kind"].(string); got != "dir" { + t.Fatalf("kind = %#v, want %q", payload["kind"], "dir") + } + if stat, err := mountclient.New(server.store.rdb, fsKey).Stat(ctx, "/docs/empty"); err != nil { + t.Fatalf("Stat(/docs/empty) returned error after delete: %v", err) + } else if stat != nil { + t.Fatalf("Stat(/docs/empty) after delete = %#v, want nil", stat) + } +} + +func TestAFSMCPFileDeleteRefusesNonEmptyDirectory(t *testing.T) { + t.Helper() + + server, closeFn := setupAFSMCPTestServer(t) + defer closeFn() + + if _, err := server.toolFileWrite(context.Background(), map[string]any{ + "path": "/docs/keep/file.md", + "content": "still here\n", + }); err != nil { + t.Fatalf("toolFileWrite(file.md) returned error: %v", err) + } + + result := server.callTool(context.Background(), "file_delete", map[string]any{ + "path": "/docs/keep", + }) + if !result.IsError { + t.Fatal("file_delete should refuse a non-empty directory") + } +} + +func TestAFSMCPFileDeleteRefusesRoot(t *testing.T) { + t.Helper() + + server, closeFn := setupAFSMCPTestServer(t) + defer closeFn() + + result := server.callTool(context.Background(), "file_delete", map[string]any{ + "path": "/", + }) + if !result.IsError { + t.Fatal("file_delete should refuse root") + } +} + func TestAFSMCPStatusAndWorkspaceCurrentPreferActiveSyncWorkspace(t *testing.T) { t.Helper() diff --git a/cmd/afs/embedded/skills/afs/SKILL.md b/cmd/afs/embedded/skills/afs/SKILL.md index 175f98b..665453b 100644 --- a/cmd/afs/embedded/skills/afs/SKILL.md +++ b/cmd/afs/embedded/skills/afs/SKILL.md @@ -1,6 +1,6 @@ --- name: agent-filesystem -description: Persistent Redis-backed workspaces for agents. Use via `afs mcp`, the `afs` CLI, sync mode, live mounts, and explicit checkpoints. +description: "Use when agents need persistent shared storage, when saving or restoring workspace state, or when coordinating file access across multiple agents and machines. Creates Redis-backed workspaces, checkpoints and restores agent state, mounts shared filesystems locally, searches workspace contents, and forks workspaces for parallel work." --- # Agent Filesystem @@ -12,10 +12,11 @@ explicit checkpoints and easy movement between MCP, sync mode, and live mounts. ## When to Use This Skill **Use for:** -- Persistent agent workspaces -- Code or docs that should live in a normal directory -- Shared notes/config/state that benefit from checkpoints and forks -- Searchable workspaces where `afs fs grep` or MCP file tools are useful +- Persistent agent workspaces that survive across sessions and machines +- Code, docs, or shared state that should live in a normal directory backed by Redis +- Saving and restoring workspace snapshots with explicit checkpoints +- Searching workspace contents with `afs fs grep` or MCP file tools +- Forking a workspace to run parallel experiments without losing the original **Avoid for:** - Large build output, media, or disposable artifacts @@ -46,11 +47,13 @@ workspace exposed directly as a mount rather than through the sync daemon. ```bash ./afs ws create my-project ./afs ws import my-project ./existing-dir +./afs ws list # verify the workspace exists ``` ### Start working locally ```bash ./afs ws mount my-project ~/my-project +./afs status # verify the mount is active cd ~/my-project ``` @@ -63,13 +66,15 @@ cd ~/my-project ### Save and restore stable points ```bash ./afs cp create my-project before-refactor -./afs cp list my-project +./afs cp list my-project # verify the checkpoint was saved ./afs cp restore my-project before-refactor +./afs cp list my-project # confirm the restore completed ``` ### Fork work for a second line of effort ```bash ./afs ws fork my-project my-project-experiment +./afs ws list # verify the fork appears ``` ## Key Points @@ -81,3 +86,9 @@ cd ~/my-project - File edits change the live workspace immediately. - Create checkpoints explicitly when you want a restore point. - `.afsignore` controls what gets imported from an existing local directory. + +## Further Reading + +- `docs/guides/agent-filesystem.md` — agent-facing usage guide +- `docs/reference/cli.md` — full CLI command reference +- `docs/reference/mcp.md` — MCP tool reference for agent integrations diff --git a/docs/reference/mcp.md b/docs/reference/mcp.md index 4854123..45b2ddf 100644 --- a/docs/reference/mcp.md +++ b/docs/reference/mcp.md @@ -71,7 +71,7 @@ use a workspace-scoped hosted token. | Status/admin | `afs_status`, `workspace_list`, `workspace_create`, `workspace_fork` | | Checkpoints | `checkpoint_list`, `checkpoint_create`, `checkpoint_restore` | | File reads | `file_read`, `file_lines`, `file_list`, `file_glob`, `file_grep`, `file_query` | -| File writes | `file_write`, `file_create_exclusive`, `file_replace`, `file_insert`, `file_delete_lines`, `file_patch` | +| File writes | `file_write`, `file_create_exclusive`, `file_replace`, `file_insert`, `file_delete`, `file_delete_lines`, `file_patch` | | Hosted token administration | `mcp_token_issue`, `mcp_token_revoke` | Hosted workspace-scoped MCP exposes the workspace file and checkpoint tools. @@ -395,6 +395,17 @@ Arguments: | `start` | Yes | Start line, 1-indexed. | | `end` | Yes | End line, inclusive. | +### `file_delete` + +Deletes one file, symlink, or empty directory. Non-empty directories are refused. + +Arguments: + +| Field | Required | Meaning | +| --- | --- | --- | +| `workspace` | No | Local MCP workspace override. | +| `path` | Yes | Absolute file, symlink, or empty directory path. | + ### `file_patch` Applies one or more structured text patches. diff --git a/internal/controlplane/mcp_hosted.go b/internal/controlplane/mcp_hosted.go index fbba2b7..6b4784b 100644 --- a/internal/controlplane/mcp_hosted.go +++ b/internal/controlplane/mcp_hosted.go @@ -395,6 +395,17 @@ func (p *hostedMCPProvider) workspaceTools() []mcpproto.Tool { "required": []string{"path"}, }, }, + { + Name: "file_delete", + Description: "Delete one file, symlink, or empty directory from the current workspace. Refuses root and non-empty directories.", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "path": map[string]string{"type": "string", "description": "Absolute path inside the workspace"}, + }, + "required": []string{"path"}, + }, + }, { Name: "file_lines", Description: "Read a specific line range from a text file", @@ -710,6 +721,11 @@ func (origProvider *hostedMCPProvider) callWorkspaceTool(ctx context.Context, na if err == nil { value, err = p.toolFileUndelete(ctx, args) } + case "file_delete": + err = p.ensureWritable() + if err == nil { + value, err = p.toolFileDelete(ctx, args) + } case "file_lines": value, err = p.toolFileLines(ctx, args) case "file_list": @@ -1443,6 +1459,21 @@ func (p *hostedMCPProvider) toolFileDeleteLines(ctx context.Context, args map[st }) } +func (p *hostedMCPProvider) toolFileDelete(ctx context.Context, args map[string]any) (any, error) { + return p.mutateWorkspaceFile(ctx, args, func(ctx context.Context, fsClient afsclient.Client, normalizedPath string, stat *afsclient.StatResult) (map[string]any, error) { + if stat == nil { + return nil, os.ErrNotExist + } + if err := fsClient.Rm(ctx, normalizedPath); err != nil { + return nil, err + } + return map[string]any{ + "operation": "delete", + "kind": stat.Type, + }, nil + }) +} + func (p *hostedMCPProvider) toolFilePatch(ctx context.Context, args map[string]any) (any, error) { var input mcpFilePatchInput if err := decodeMCPArgs(args, &input); err != nil { @@ -1761,7 +1792,7 @@ func (p *hostedMCPProvider) mutateWorkspaceFile(ctx context.Context, args map[st } entry := template entry.Path = normalizedPath - entry.Op = ChangeOpPut + entry.Op = hostedMCPMutationChangeOp(stat, updatedStat, beforeSnapshot, afterSnapshot) entry.PrevHash = beforeSnapshot.ContentHash entry.DeltaBytes = -beforeSnapshot.SizeBytes if afterSnapshot.Exists { @@ -1780,6 +1811,19 @@ func (p *hostedMCPProvider) mutateWorkspaceFile(ctx context.Context, args map[st return payload, nil } +func hostedMCPMutationChangeOp(beforeStat, afterStat *afsclient.StatResult, beforeSnapshot, afterSnapshot VersionedFileSnapshot) string { + if afterStat == nil && beforeStat != nil { + return deleteOpFor(beforeStat.Type) + } + if afterStat != nil { + return modifyOpFor(afterStat.Type) + } + if !afterSnapshot.Exists && beforeSnapshot.Exists { + return deleteOpFor(beforeSnapshot.Kind) + } + return ChangeOpPut +} + func ensureHostedWorkspaceParentDirs(ctx context.Context, fsClient afsclient.Client, normalizedPath string) error { trimmed := strings.Trim(normalizedPath, "/") if trimmed == "" { diff --git a/internal/controlplane/mcp_hosted_test.go b/internal/controlplane/mcp_hosted_test.go index 1c3407c..99e7a90 100644 --- a/internal/controlplane/mcp_hosted_test.go +++ b/internal/controlplane/mcp_hosted_test.go @@ -81,6 +81,156 @@ func TestHostedMCPFileCreateExclusiveFailsWhenFileExists(t *testing.T) { } } +func TestHostedMCPFileDeleteRemovesFile(t *testing.T) { + t.Helper() + + manager, databaseID := newTestManager(t) + provider := &hostedMCPProvider{ + manager: manager, + databaseID: databaseID, + workspace: "repo", + profile: MCPProfileWorkspaceRW, + } + + writeResult := provider.CallTool(context.Background(), "file_write", map[string]any{ + "path": "/docs/remove-me.md", + "content": "delete me\n", + }) + if writeResult.IsError { + t.Fatalf("file_write returned error result: %+v", writeResult) + } + + deleteResult := provider.CallTool(context.Background(), "file_delete", map[string]any{ + "path": "/docs/remove-me.md", + }) + if deleteResult.IsError { + t.Fatalf("file_delete returned error result: %+v", deleteResult) + } + var deletePayload map[string]any + if err := decodeHostedStructuredContent(deleteResult.StructuredContent, &deletePayload); err != nil { + t.Fatalf("decodeHostedStructuredContent(delete) returned error: %v", err) + } + if got, _ := deletePayload["operation"].(string); got != "delete" { + t.Fatalf("operation = %#v, want %q", deletePayload["operation"], "delete") + } + readResult := provider.CallTool(context.Background(), "file_read", map[string]any{ + "path": "/docs/remove-me.md", + }) + if !readResult.IsError { + t.Fatalf("file_read after delete succeeded: %+v", readResult) + } + + changelog, err := manager.ListChangelog(context.Background(), databaseID, "repo", ChangelogListRequest{Limit: 5}) + if err != nil { + t.Fatalf("ListChangelog() returned error: %v", err) + } + if len(changelog.Entries) == 0 { + t.Fatal("len(changelog.Entries) = 0, want at least one row") + } + foundDelete := false + for _, entry := range changelog.Entries { + if entry.Path == "/docs/remove-me.md" && entry.Op == ChangeOpDelete { + foundDelete = true + break + } + } + if !foundDelete { + t.Fatalf("changelog entries missing delete for /docs/remove-me.md: %+v", changelog.Entries) + } +} + +func TestHostedMCPFileDeleteRemovesEmptyDirectoryWithRmdirChangelog(t *testing.T) { + t.Helper() + + ctx := context.Background() + manager, databaseID := newTestManager(t) + provider := &hostedMCPProvider{ + manager: manager, + databaseID: databaseID, + workspace: "repo", + profile: MCPProfileWorkspaceRW, + } + + resolved, err := provider.resolveWorkspaceContext(ctx) + if err != nil { + t.Fatalf("resolveWorkspaceContext() returned error: %v", err) + } + if err := resolved.fsClient.Mkdir(ctx, "/docs/empty"); err != nil { + t.Fatalf("Mkdir(/docs/empty) returned error: %v", err) + } + + deleteResult := provider.CallTool(ctx, "file_delete", map[string]any{ + "path": "/docs/empty", + }) + if deleteResult.IsError { + t.Fatalf("file_delete returned error result: %+v", deleteResult) + } + var deletePayload map[string]any + if err := decodeHostedStructuredContent(deleteResult.StructuredContent, &deletePayload); err != nil { + t.Fatalf("decodeHostedStructuredContent(delete) returned error: %v", err) + } + if got, _ := deletePayload["kind"].(string); got != "dir" { + t.Fatalf("kind = %#v, want %q", deletePayload["kind"], "dir") + } + + changelog, err := manager.ListChangelog(ctx, databaseID, "repo", ChangelogListRequest{Limit: 10}) + if err != nil { + t.Fatalf("ListChangelog() returned error: %v", err) + } + foundRmdir := false + for _, entry := range changelog.Entries { + if entry.Path == "/docs/empty" && entry.Op == ChangeOpRmdir { + foundRmdir = true + break + } + } + if !foundRmdir { + t.Fatalf("changelog entries missing rmdir for /docs/empty: %+v", changelog.Entries) + } +} + +func TestHostedMCPFileDeleteRefusesRootAndNonEmptyDirectory(t *testing.T) { + t.Helper() + + ctx := context.Background() + manager, databaseID := newTestManager(t) + provider := &hostedMCPProvider{ + manager: manager, + databaseID: databaseID, + workspace: "repo", + profile: MCPProfileWorkspaceRW, + } + + writeResult := provider.CallTool(ctx, "file_write", map[string]any{ + "path": "/docs/keep/file.md", + "content": "still here\n", + }) + if writeResult.IsError { + t.Fatalf("file_write returned error result: %+v", writeResult) + } + + rootResult := provider.CallTool(ctx, "file_delete", map[string]any{ + "path": "/", + }) + if !rootResult.IsError { + t.Fatal("file_delete should refuse root") + } + + dirResult := provider.CallTool(ctx, "file_delete", map[string]any{ + "path": "/docs/keep", + }) + if !dirResult.IsError { + t.Fatal("file_delete should refuse a non-empty directory") + } + + readResult := provider.CallTool(ctx, "file_read", map[string]any{ + "path": "/docs/keep/file.md", + }) + if readResult.IsError { + t.Fatalf("file_read after refused directory delete returned error result: %+v", readResult) + } +} + func TestHostedMCPFileQueryRanksWorkspaceContent(t *testing.T) { t.Helper() diff --git a/internal/controlplane/mcp_profiles.go b/internal/controlplane/mcp_profiles.go index 9d7e08a..f14d935 100644 --- a/internal/controlplane/mcp_profiles.go +++ b/internal/controlplane/mcp_profiles.go @@ -38,6 +38,7 @@ var ( "file_create_exclusive": {}, "file_replace": {}, "file_insert": {}, + "file_delete": {}, "file_delete_lines": {}, "file_patch": {}, "file_restore_version": {}, diff --git a/internal/controlplane/mcp_profiles_test.go b/internal/controlplane/mcp_profiles_test.go new file mode 100644 index 0000000..7414144 --- /dev/null +++ b/internal/controlplane/mcp_profiles_test.go @@ -0,0 +1,19 @@ +package controlplane + +import "testing" + +func TestMCPProfileFileDeleteRequiresWriteAccess(t *testing.T) { + t.Helper() + + for _, profile := range []string{MCPProfileWorkspaceRO, MCPProfileAdminRO} { + if MCPProfileAllowsTool(profile, "file_delete") { + t.Fatalf("MCPProfileAllowsTool(%q, file_delete) = true, want false", profile) + } + } + + for _, profile := range []string{MCPProfileWorkspaceRW, MCPProfileWorkspaceRWCheckpoint, MCPProfileAdminRW} { + if !MCPProfileAllowsTool(profile, "file_delete") { + t.Fatalf("MCPProfileAllowsTool(%q, file_delete) = false, want true", profile) + } + } +}