Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/mcp.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ The in-graph coverage tools above (`analyze kind=coverage*`, `index_health` lang
| Tool | Description |
|------|-------------|
| `scaffold` | Generate code, registration wiring, and test stubs from an example symbol |
| `batch_edit` | Apply multiple edits in dependency order, re-index between steps |
| `batch_edit` | Atomically apply `edit_symbol`, `edit_file`, `move_file`, and `delete_file` operations with durable rollback receipts |
| `diff_context` | Git diff enriched with callers, callees, community, processes, per-file risk |
| `prefetch_context` | Predict needed symbols from task description and recent activity. Accepts `max_bytes` / `max_tokens` budget caps |

Expand Down
6 changes: 4 additions & 2 deletions internal/mcp/batch_edit_hetero_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,21 @@ func TestBatchEditItemKind(t *testing.T) {
require.Equal(t, "edit_file", batchEditItem{Path: "p"}.kind(), "a path infers edit_file")
require.Equal(t, "edit_file", batchEditItem{Op: "edit_file", Path: "p"}.kind())
require.Equal(t, "edit_symbol", batchEditItem{Op: "edit_symbol", Path: "p"}.kind(), "explicit op wins over inference")
require.Equal(t, "move_file", batchEditItem{Op: "move_file"}.kind())
require.Equal(t, "delete_file", batchEditItem{Op: "delete_file"}.kind())
}

func TestBatchEditItemsSchemaOneOf(t *testing.T) {
schema := batchEditItemsSchema()
branches, ok := schema["oneOf"].([]any)
require.True(t, ok, "items schema must be a oneOf")
require.Len(t, branches, 2)
require.Len(t, branches, 4)
for _, b := range branches {
m := b.(map[string]any)
require.Equal(t, "object", m["type"])
props := m["properties"].(map[string]any)
op := props["op"].(map[string]any)
require.Contains(t, []any{"edit_symbol", "edit_file"}, op["const"], "each branch is discriminated by an op const")
require.Contains(t, []any{"edit_symbol", "edit_file", "move_file", "delete_file"}, op["const"], "each branch is discriminated by an op const")
require.NotEmpty(t, m["required"], "each branch declares required fields")
}
}
Expand Down
41 changes: 39 additions & 2 deletions internal/mcp/batch_edit_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,31 @@ func TestParseBatchEditsInfersCompleteLegacyShapes(t *testing.T) {
require.Equal(t, "edit_symbol", legacy[0].Op)
}

func TestParseBatchEditsAcceptsExplicitLifecycleShapes(t *testing.T) {
const digest = "0000000000000000000000000000000000000000000000000000000000000000"
branches := batchEditItemsSchema()["oneOf"].([]any)
for _, index := range []int{2, 3} {
required := branches[index].(map[string]any)["required"].([]any)
require.Contains(t, required, "op", "lifecycle operations must be explicitly discriminated")
}

items, err := parseBatchEdits([]any{
map[string]any{
"op": "move_file", "source": "old.txt", "destination": "new.txt",
"expected_sha256": digest,
},
map[string]any{"op": "delete_file", "path": "obsolete.txt"},
})
require.NoError(t, err)
require.Len(t, items, 2)
require.Equal(t, "move_file", items[0].Op)
require.Equal(t, "old.txt", items[0].SourcePath)
require.Equal(t, "new.txt", items[0].DestinationPath)
require.Equal(t, digest, items[0].ExpectedSHA256)
require.Equal(t, "delete_file", items[1].Op)
require.Equal(t, "obsolete.txt", items[1].Path)
}

func TestParseBatchEditsRejectsUnknownOpInLegacyJSONString(t *testing.T) {
_, err := parseBatchEdits(`[{"op":"replace_file","path":"a.go","old_string":"before","new_string":"after"}]`)
require.Error(t, err)
Expand Down Expand Up @@ -72,6 +97,18 @@ func TestParseBatchEditsRejectsAmbiguousAndIncompleteShapes(t *testing.T) {
item: map[string]any{"file": "a.go"},
want: "does not match a supported batch edit shape",
},
{
name: "lifecycle-without-discriminator",
item: map[string]any{"source": "old.txt", "destination": "new.txt"},
want: "move_file and delete_file require an explicit op",
},
{
name: "mixed-lifecycle",
item: map[string]any{
"op": "move_file", "source": "old.txt", "destination": "new.txt", "path": "other.txt",
},
want: "mixes fields from multiple batch edit operations",
},
} {
t.Run(test.name, func(t *testing.T) {
_, err := parseBatchEdits([]any{test.item})
Expand Down Expand Up @@ -110,8 +147,8 @@ func TestBatchEditUnknownDiscriminatorIsStructuredAndWritesNothing(t *testing.T)
require.Contains(t, payload["message"], `unknown op "replace_file"`)
data := payload["data"].(map[string]any)
require.Equal(t, float64(1), data["item_index"])
require.ElementsMatch(t, []any{"edit_file", "edit_symbol"}, data["accepted_values"])
require.Len(t, data["accepted_shapes"], 2)
require.ElementsMatch(t, []any{"edit_file", "edit_symbol", "move_file", "delete_file"}, data["accepted_values"])
require.Len(t, data["accepted_shapes"], 4)

content, err := os.ReadFile(path)
require.NoError(t, err)
Expand Down
58 changes: 58 additions & 0 deletions internal/mcp/batch_file_lifecycle_recovery_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package mcp

import (
"context"
"errors"
"os"
"path/filepath"
"testing"

"github.com/zzet/gortex/internal/agents"
)

func TestAtomicBatchLifecycleRecoveryRollsBackPartialMove(t *testing.T) {
s := newAtomicBatchTestServer(t, mutationTestWatcher{})
dir := t.TempDir()
source := writeAtomicBatchFixture(t, dir, "source.txt", "source\n")
destination := filepath.Join(dir, "destination.txt")
buffers := map[string]*batchFileBuffer{
source: {
absPath: source, relPath: "source.txt", mode: 0o644,
original: []byte("source\n"), content: []byte("source\n"),
existsBefore: true, existsAfter: false, existenceSet: true,
},
destination: {
absPath: destination, relPath: "destination.txt", mode: 0o644,
content: []byte("source\n"), existsAfter: true, existenceSet: true,
},
}
results := []batchEditResult{{
Op: "move_file", FilePath: "source.txt", DestinationPath: "destination.txt", Status: "validated",
}}
receipt := batchTransactionReceipt{
Version: batchTransactionVersion, TransactionID: "recover-partial-move", Fingerprint: "recovery-fixture",
Status: "preparing", DiskStatus: "unchanged", GraphStatus: "not_started",
Results: results, Summary: batchSummary(results),
}
if err := s.prepareBatchJournal(&receipt, buffers, []string{destination, source}); err != nil {
t.Fatal(err)
}
if err := agents.AtomicWriteFile(destination, []byte("source\n"), 0o644); err != nil {
t.Fatal(err)
}

restarted := &Server{watcher: mutationTestWatcher{}, session: newSessionState()}
recovered, err := restarted.batchTransactionStatus(context.Background(), "recover-partial-move")
if err != nil {
t.Fatal(err)
}
if !recovered.Recovered || recovered.Status != "aborted" || recovered.DiskStatus != "rolled_back" {
t.Fatalf("recovered receipt = %+v", recovered)
}
if got := readAtomicBatchFixture(t, source); got != "source\n" {
t.Fatalf("source = %q", got)
}
if _, err := os.Stat(destination); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("destination survived rollback: %v", err)
}
}
203 changes: 203 additions & 0 deletions internal/mcp/batch_file_lifecycle_security_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
package mcp

import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
)

func TestAtomicBatchLifecycleOutsideRootRefused(t *testing.T) {
repoRoot := t.TempDir()
outsideRoot := t.TempDir()
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newReadGuardServer(t, repoRoot)
source := writeAtomicBatchFixture(t, repoRoot, "source.txt", "source\n")
destination := filepath.Join(outsideRoot, "destination.txt")

receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileMove(source, destination, ""),
}, "file-lifecycle-outside-root")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" || !strings.Contains(receipt.Error, "outside") {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, source); got != "source\n" {
t.Fatalf("source = %q", got)
}
if _, err := os.Stat(destination); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("destination was created: %v", err)
}
}

func TestAtomicBatchLifecycleInvalidDigestAndOverlapRefused(t *testing.T) {
t.Run("invalid digest", func(t *testing.T) {
s := newAtomicBatchTestServer(t, mutationTestWatcher{})
path := writeAtomicBatchFixture(t, t.TempDir(), "source.txt", "source\n")
receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileDelete(path, "not-a-sha256"),
}, "file-lifecycle-invalid-digest")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, path); got != "source\n" {
t.Fatalf("source = %q", got)
}
})

t.Run("overlapping path ownership", func(t *testing.T) {
s := newAtomicBatchTestServer(t, mutationTestWatcher{})
dir := t.TempDir()
path := writeAtomicBatchFixture(t, dir, "source.txt", "source\n")
receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileEdit(path, "source", "edited"),
atomicFileDelete(path, ""),
}, "file-lifecycle-overlap")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" || !strings.Contains(receipt.Error, "overlaps") {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, path); got != "source\n" {
t.Fatalf("source = %q", got)
}
})
}

func TestPrepareBatchJournalRequiresExplicitExistenceState(t *testing.T) {
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newAtomicBatchTestServer(t, mutationTestWatcher{})
path := filepath.Join(t.TempDir(), "empty.txt")
receipt := batchTransactionReceipt{TransactionID: "unset-existence-state"}
err := s.prepareBatchJournal(&receipt, map[string]*batchFileBuffer{
path: {absPath: path, relPath: "empty.txt"},
}, []string{path})
if err == nil || !strings.Contains(err.Error(), "existence state is unset") {
t.Fatalf("prepareBatchJournal error = %v", err)
}
}

func TestAtomicBatchLifecycleDestinationGuards(t *testing.T) {
t.Run("destination symlink", func(t *testing.T) {
if os.PathSeparator == '\\' {
t.Skip("symlink creation is not reliably available on Windows CI")
}
repoRoot := t.TempDir()
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newReadGuardServer(t, repoRoot)
source := writeAtomicBatchFixture(t, repoRoot, "source.txt", "source\n")
target := writeAtomicBatchFixture(t, repoRoot, "target.txt", "target\n")
destination := filepath.Join(repoRoot, "destination.txt")
if err := os.Symlink(target, destination); err != nil {
t.Fatal(err)
}

receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileMove(source, destination, ""),
}, "file-lifecycle-destination-symlink")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" || !strings.Contains(receipt.Error, "symlink") {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, source); got != "source\n" {
t.Fatalf("source = %q", got)
}
if got := readAtomicBatchFixture(t, target); got != "target\n" {
t.Fatalf("target = %q", got)
}
})

t.Run("symlinked destination parent", func(t *testing.T) {
if os.PathSeparator == '\\' {
t.Skip("symlink creation is not reliably available on Windows CI")
}
repoRoot := t.TempDir()
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newReadGuardServer(t, repoRoot)
source := writeAtomicBatchFixture(t, repoRoot, "source.txt", "source\n")
realParent := filepath.Join(repoRoot, "real-parent")
if err := os.Mkdir(realParent, 0o755); err != nil {
t.Fatal(err)
}
linkedParent := filepath.Join(repoRoot, "linked-parent")
if err := os.Symlink(realParent, linkedParent); err != nil {
t.Fatal(err)
}
destination := filepath.Join(linkedParent, "destination.txt")

receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileMove(source, destination, ""),
}, "file-lifecycle-symlinked-parent")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" || !strings.Contains(receipt.Error, "symlink") {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, source); got != "source\n" {
t.Fatalf("source = %q", got)
}
if _, err := os.Stat(filepath.Join(realParent, "destination.txt")); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("destination was created: %v", err)
}
})

t.Run("dot-dot traversal destination", func(t *testing.T) {
repoRoot := t.TempDir()
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newReadGuardServer(t, repoRoot)
source := writeAtomicBatchFixture(t, repoRoot, "source.txt", "source\n")
outsideName := "traversal-destination-" + filepath.Base(repoRoot) + ".txt"
destination := repoRoot + string(os.PathSeparator) + ".." + string(os.PathSeparator) + outsideName

receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileMove(source, destination, ""),
}, "file-lifecycle-dot-dot-destination")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" || !strings.Contains(receipt.Error, "outside") {
t.Fatalf("receipt = %+v", receipt)
}
if got := readAtomicBatchFixture(t, source); got != "source\n" {
t.Fatalf("source = %q", got)
}
if _, err := os.Stat(filepath.Clean(destination)); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("destination was created: %v", err)
}
})

t.Run("delete directory", func(t *testing.T) {
repoRoot := t.TempDir()
t.Setenv(batchTransactionDirEnv, filepath.Join(t.TempDir(), "transactions"))
s := newReadGuardServer(t, repoRoot)
directory := filepath.Join(repoRoot, "directory")
if err := os.Mkdir(directory, 0o755); err != nil {
t.Fatal(err)
}

receipt, err := s.runBatchTransaction(context.Background(), []batchEditItem{
atomicFileDelete(directory, ""),
}, "file-lifecycle-delete-directory")
if err != nil {
t.Fatal(err)
}
if receipt.Status != "aborted" || receipt.DiskStatus != "unchanged" {
t.Fatalf("receipt = %+v", receipt)
}
info, err := os.Stat(directory)
if err != nil || !info.IsDir() {
t.Fatalf("directory was changed: info=%v err=%v", info, err)
}
})
}
Loading
Loading