From e12187fbfddc3a86d018624a733625d9932a3903 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:14:29 -0500 Subject: [PATCH 1/9] refactor(integration): split helpers.go into setup, actions, and direct files --- environment/integration/actions.go | 127 +++++++++++++++ environment/integration/direct.go | 75 +++++++++ environment/integration/setup.go | 251 +++++++++++++++++++++++++++++ 3 files changed, 453 insertions(+) create mode 100644 environment/integration/actions.go create mode 100644 environment/integration/direct.go create mode 100644 environment/integration/setup.go diff --git a/environment/integration/actions.go b/environment/integration/actions.go new file mode 100644 index 00000000..482e8da4 --- /dev/null +++ b/environment/integration/actions.go @@ -0,0 +1,127 @@ +package integration + +import ( + "context" + "testing" + + "dagger.io/dagger" + "github.com/dagger/container-use/environment" + "github.com/dagger/container-use/repository" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// UserActions provides test helpers that mirror MCP tool behavior exactly +// These represent what a user would experience when using the MCP tools +type UserActions struct { + t *testing.T + ctx context.Context + repo *repository.Repository + dag *dagger.Client + repoDir string // Source directory (for direct manipulation) + configDir string // Container-use config directory +} + +func NewUserActions(t *testing.T, repo *repository.Repository, dag *dagger.Client) *UserActions { + return &UserActions{ + t: t, + ctx: context.Background(), + repo: repo, + dag: dag, + } +} + +// WithDirectAccess adds direct filesystem access for edge case testing +func (u *UserActions) WithDirectAccess(repoDir, configDir string) *UserActions { + u.repoDir = repoDir + u.configDir = configDir + return u +} + +// FileWrite mirrors environment_file_write MCP tool behavior +func (u *UserActions) FileWrite(envID, targetFile, contents, explanation string) { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + err = env.FileWrite(u.ctx, explanation, targetFile, contents) + require.NoError(u.t, err, "FileWrite should succeed") + + err = u.repo.Update(u.ctx, env, explanation) + require.NoError(u.t, err, "repo.Update after FileWrite should succeed") +} + +// RunCommand mirrors environment_run_cmd MCP tool behavior +func (u *UserActions) RunCommand(envID, command, explanation string) string { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + output, err := env.Run(u.ctx, command, "/bin/sh", false) + require.NoError(u.t, err, "Run command should succeed") + + err = u.repo.Update(u.ctx, env, explanation) + require.NoError(u.t, err, "repo.Update after Run should succeed") + + return output +} + +// CreateEnvironment mirrors environment_create MCP tool behavior +func (u *UserActions) CreateEnvironment(title, explanation string) *environment.Environment { + env, err := u.repo.Create(u.ctx, u.dag, title, explanation, "HEAD") + require.NoError(u.t, err, "Create environment should succeed") + return env +} + +// UpdateEnvironment mirrors environment_update MCP tool behavior +func (u *UserActions) UpdateEnvironment(envID, title, explanation string, config *environment.EnvironmentConfig) { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + if title != "" { + env.State.Title = title + } + + err = env.UpdateConfig(u.ctx, config) + require.NoError(u.t, err, "UpdateConfig should succeed") + + err = u.repo.Update(u.ctx, env, explanation) + require.NoError(u.t, err, "repo.Update after UpdateConfig should succeed") +} + +// FileDelete mirrors environment_file_delete MCP tool behavior +func (u *UserActions) FileDelete(envID, targetFile, explanation string) { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + err = env.FileDelete(u.ctx, explanation, targetFile) + require.NoError(u.t, err, "FileDelete should succeed") + + err = u.repo.Update(u.ctx, env, explanation) + require.NoError(u.t, err, "repo.Update after FileDelete should succeed") +} + +// FileRead mirrors environment_file_read MCP tool behavior (read-only, no update) +func (u *UserActions) FileRead(envID, targetFile string) string { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + content, err := env.FileRead(u.ctx, targetFile, true, 0, 0) + require.NoError(u.t, err, "FileRead should succeed") + return content +} + +// FileReadExpectError is for testing expected failures +func (u *UserActions) FileReadExpectError(envID, targetFile string) { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Failed to get environment %s", envID) + + _, err = env.FileRead(u.ctx, targetFile, true, 0, 0) + assert.Error(u.t, err, "FileRead should fail for %s", targetFile) +} + +// GetEnvironment retrieves an environment by ID - mirrors how MCP tools work +// Each MCP tool call starts fresh by getting the environment from the repository +func (u *UserActions) GetEnvironment(envID string) *environment.Environment { + env, err := u.repo.Get(u.ctx, u.dag, envID) + require.NoError(u.t, err, "Should be able to get environment %s", envID) + return env +} diff --git a/environment/integration/direct.go b/environment/integration/direct.go new file mode 100644 index 00000000..972a4ba4 --- /dev/null +++ b/environment/integration/direct.go @@ -0,0 +1,75 @@ +package integration + +import ( + "os" + "path/filepath" + + "github.com/dagger/container-use/repository" + "github.com/stretchr/testify/require" +) + +// --- Direct manipulation methods for edge case testing --- + +// WriteSourceFile writes directly to the source repository +func (u *UserActions) WriteSourceFile(path, content string) { + require.NotEmpty(u.t, u.repoDir, "Need direct access for source file manipulation") + fullPath := filepath.Join(u.repoDir, path) + dir := filepath.Dir(fullPath) + + err := os.MkdirAll(dir, 0755) + require.NoError(u.t, err, "Failed to create dir") + + err = os.WriteFile(fullPath, []byte(content), 0600) + require.NoError(u.t, err, "Failed to write source file") +} + +// WorktreePath returns the worktree path for an environment, handling errors +func (u *UserActions) WorktreePath(envID string) string { + worktreePath, err := u.repo.WorktreePath(envID) + require.NoError(u.t, err, "Failed to get worktree path for environment %s", envID) + return worktreePath +} + +// ReadWorktreeFile reads directly from an environment's worktree +func (u *UserActions) ReadWorktreeFile(envID, path string) string { + worktreePath := u.WorktreePath(envID) + fullPath := filepath.Join(worktreePath, path) + content, err := os.ReadFile(fullPath) + require.NoError(u.t, err, "Failed to read worktree file") + return string(content) +} + +// CorruptWorktree simulates worktree corruption for recovery testing +func (u *UserActions) CorruptWorktree(envID string) { + worktreePath := u.WorktreePath(envID) + + // Remove .git directory to corrupt the worktree + gitDir := filepath.Join(worktreePath, ".git") + err := os.RemoveAll(gitDir) + require.NoError(u.t, err, "Failed to corrupt worktree") +} + +// GitCommand runs a git command in the source repository +func (u *UserActions) GitCommand(args ...string) string { + require.NotEmpty(u.t, u.repoDir, "Need direct access for git commands") + output, err := repository.RunGitCommand(u.ctx, u.repoDir, args...) + require.NoError(u.t, err, "Git command failed: %v", args) + return output +} + +// WriteFileInSourceRepo writes a file to the source repo and commits it +func (u *UserActions) WriteFileInSourceRepo(path, content, commitMessage string) { + require.NotEmpty(u.t, u.repoDir, "Need direct access for source file manipulation") + writeFile(u.t, u.repoDir, path, content) + gitCommit(u.t, u.repoDir, commitMessage) +} + +// CreateBranchInSourceRepo creates and checks out a new branch in the source repo +func (u *UserActions) CreateBranchInSourceRepo(branchName string) { + u.GitCommand("checkout", "-b", branchName) +} + +// CheckoutBranchInSourceRepo checks out an existing branch in the source repo +func (u *UserActions) CheckoutBranchInSourceRepo(branchName string) { + u.GitCommand("checkout", branchName) +} diff --git a/environment/integration/setup.go b/environment/integration/setup.go new file mode 100644 index 00000000..172b645b --- /dev/null +++ b/environment/integration/setup.go @@ -0,0 +1,251 @@ +package integration + +import ( + "context" + "log/slog" + "os" + "path/filepath" + "sync" + "testing" + + "dagger.io/dagger" + "github.com/dagger/container-use/repository" + "github.com/stretchr/testify/require" +) + +var ( + testDaggerClient *dagger.Client + daggerOnce sync.Once + daggerErr error +) + +// init sets up logging for tests +func init() { + // Only show warnings and errors in tests unless TEST_VERBOSE is set + level := slog.LevelWarn + if os.Getenv("TEST_VERBOSE") != "" { + level = slog.LevelInfo + } + + slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{ + Level: level, + }))) +} + +// WithRepository runs a test function with an isolated repository and UserActions +func WithRepository(t *testing.T, name string, setup RepositorySetup, fn func(t *testing.T, repo *repository.Repository, user *UserActions)) { + // Initialize Dagger (needed for environment operations) + initializeDaggerOnce(t) + + ctx := context.Background() + + // Create isolated temp directories + repoDir, err := os.MkdirTemp("", "cu-test-"+name+"-*") + require.NoError(t, err, "Failed to create repo dir") + + configDir, err := os.MkdirTemp("", "cu-test-config-"+name+"-*") + require.NoError(t, err, "Failed to create config dir") + + // Initialize git repo + cmds := [][]string{ + {"init", "--initial-branch=main"}, + {"config", "user.email", "test@example.com"}, + {"config", "user.name", "Test User"}, + {"config", "commit.gpgsign", "false"}, + } + + for _, cmd := range cmds { + _, err := repository.RunGitCommand(ctx, repoDir, cmd...) + require.NoError(t, err, "Failed to run git %v", cmd) + } + + // Run setup to populate repo + if setup != nil { + setup(t, repoDir) + } + + // Open repository with isolated base path + repo, err := repository.OpenWithBasePath(ctx, repoDir, configDir) + require.NoError(t, err, "Failed to open repository") + + // Create UserActions with extended capabilities + user := NewUserActions(t, repo, testDaggerClient).WithDirectAccess(repoDir, configDir) + + // Cleanup + t.Cleanup(func() { + // Clean up any environments created during the test + envs, _ := repo.List(context.Background()) + for _, env := range envs { + repo.Delete(context.Background(), env.ID) + } + + // Remove directories + os.RemoveAll(repoDir) + os.RemoveAll(configDir) + }) + + // Run the test function + fn(t, repo, user) +} + +// RepositorySetup is a function that prepares a test repository +type RepositorySetup func(t *testing.T, repoDir string) + +// Common repository setups +var ( + SetupPythonRepo = func(t *testing.T, repoDir string) { + writeFile(t, repoDir, "main.py", "def main():\n print('Hello World')\n\nif __name__ == '__main__':\n main()\n") + writeFile(t, repoDir, "requirements.txt", "requests==2.31.0\nnumpy==1.24.0\n") + writeFile(t, repoDir, ".gitignore", "__pycache__/\n*.pyc\n.env\nvenv/\n") + gitCommit(t, repoDir, "Initial Python project") + } + + SetupPythonRepoNoGitignore = func(t *testing.T, repoDir string) { + writeFile(t, repoDir, "main.py", "def main():\n print('Hello World')\n\nif __name__ == '__main__':\n main()\n") + writeFile(t, repoDir, "requirements.txt", "requests==2.31.0\nnumpy==1.24.0\n") + gitCommit(t, repoDir, "Initial Python project") + } + + SetupNodeRepo = func(t *testing.T, repoDir string) { + packageJSON := `{ + "name": "test-project", + "version": "1.0.0", + "main": "index.js", + "scripts": { + "start": "node index.js", + "test": "jest" + }, + "dependencies": { + "express": "^4.18.0" + } +}` + writeFile(t, repoDir, "package.json", packageJSON) + writeFile(t, repoDir, "index.js", "console.log('Hello from Node.js');\n") + writeFile(t, repoDir, ".gitignore", "node_modules/\n.env\n") + gitCommit(t, repoDir, "Initial Node project") + } + + SetupEmptyRepo = func(t *testing.T, repoDir string) { + writeFile(t, repoDir, "README.md", "# Test Project\n") + gitCommit(t, repoDir, "Initial commit") + } + + SetupRepoWithGitConfig = func(t *testing.T, repoDir string) { + // Set project-specific git config + ctx := context.Background() + _, err := repository.RunGitCommand(ctx, repoDir, "config", "user.name", "Project User") + require.NoError(t, err, "Failed to set project user.name") + _, err = repository.RunGitCommand(ctx, repoDir, "config", "user.email", "project@example.com") + require.NoError(t, err, "Failed to set project user.email") + + writeFile(t, repoDir, "README.md", "# Project with custom git config\n") + gitCommit(t, repoDir, "Initial commit with project config") + } + + SetupRepoWithConflictingConfig = func(t *testing.T, repoDir string) { + // This assumes there might be global config, and sets local config to override it + ctx := context.Background() + _, err := repository.RunGitCommand(ctx, repoDir, "config", "user.name", "Local Project User") + require.NoError(t, err, "Failed to set local user.name") + _, err = repository.RunGitCommand(ctx, repoDir, "config", "user.email", "local@project.com") + require.NoError(t, err, "Failed to set local user.email") + + writeFile(t, repoDir, "README.md", "# Project with conflicting config\n") + gitCommit(t, repoDir, "Initial commit with local config") + } + + SetupRepoWithGitHooks = func(t *testing.T, repoDir string) { + // Create git hooks directory + hooksDir := filepath.Join(repoDir, ".git/hooks") + err := os.MkdirAll(hooksDir, 0755) + require.NoError(t, err, "Failed to create hooks directory") + + // Pre-commit hook that would block "forbidden.txt" + preCommitHook := `#!/bin/sh +echo "This pre-commit hook should never run in container-use" +if [ -f "forbidden.txt" ]; then + echo "Error: forbidden.txt is not allowed" + exit 1 +fi +exit 0` + writeFile(t, hooksDir, "pre-commit", preCommitHook) + err = os.Chmod(filepath.Join(hooksDir, "pre-commit"), 0755) + require.NoError(t, err, "Failed to make pre-commit hook executable") + + // Post-commit hook that creates evidence file + postCommitHook := `#!/bin/sh +echo "Hook ran at $(date)" >> .hook-evidence` + writeFile(t, hooksDir, "post-commit", postCommitHook) + err = os.Chmod(filepath.Join(hooksDir, "post-commit"), 0755) + require.NoError(t, err, "Failed to make post-commit hook executable") + + writeFile(t, repoDir, "README.md", "# Project with git hooks\n") + gitCommit(t, repoDir, "Initial commit with hooks") + } + + SetupRepoWithFailingHooks = func(t *testing.T, repoDir string) { + // Create git hooks directory + hooksDir := filepath.Join(repoDir, ".git/hooks") + err := os.MkdirAll(hooksDir, 0755) + require.NoError(t, err, "Failed to create hooks directory") + + // Pre-commit hook that always fails + preCommitHook := `#!/bin/sh +echo "This failing pre-commit hook should never run" +exit 1` + writeFile(t, hooksDir, "pre-commit", preCommitHook) + err = os.Chmod(filepath.Join(hooksDir, "pre-commit"), 0755) + require.NoError(t, err, "Failed to make pre-commit hook executable") + + // Pre-push hook that also fails + prePushHook := `#!/bin/sh +echo "This failing pre-push hook should never run" +exit 1` + writeFile(t, hooksDir, "pre-push", prePushHook) + err = os.Chmod(filepath.Join(hooksDir, "pre-push"), 0755) + require.NoError(t, err, "Failed to make pre-push hook executable") + + writeFile(t, repoDir, "README.md", "# Project with failing hooks\n") + gitCommit(t, repoDir, "Initial commit with failing hooks") + } +) + +// Helper functions for repository setup +func writeFile(t *testing.T, repoDir, path, content string) { + fullPath := filepath.Join(repoDir, path) + dir := filepath.Dir(fullPath) + err := os.MkdirAll(dir, 0755) + require.NoError(t, err, "Failed to create dir") + err = os.WriteFile(fullPath, []byte(content), 0600) + require.NoError(t, err, "Failed to write file") +} + +func gitCommit(t *testing.T, repoDir, message string) { + ctx := context.Background() + _, err := repository.RunGitCommand(ctx, repoDir, "add", ".") + require.NoError(t, err, "Failed to stage files") + _, err = repository.RunGitCommand(ctx, repoDir, "-c", "core.hooksPath=/dev/null", "commit", "-m", message) + require.NoError(t, err, "Failed to commit") +} + +// initializeDaggerOnce initializes Dagger client once for all tests +func initializeDaggerOnce(t *testing.T) { + daggerOnce.Do(func() { + if testDaggerClient != nil { + return + } + + ctx := context.Background() + client, err := dagger.Connect(ctx) + if err != nil { + daggerErr = err + return + } + + testDaggerClient = client + }) + + if daggerErr != nil { + t.Skipf("Skipping test - Dagger not available: %v", daggerErr) + } +} From a6b9039836865f097d9a713fc8023ae86d9d3606 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:19:21 -0500 Subject: [PATCH 2/9] refactor(integration): remove stale helpers.go after split --- environment/integration/helpers.go | 434 ----------------------------- 1 file changed, 434 deletions(-) delete mode 100644 environment/integration/helpers.go diff --git a/environment/integration/helpers.go b/environment/integration/helpers.go deleted file mode 100644 index 979fbfee..00000000 --- a/environment/integration/helpers.go +++ /dev/null @@ -1,434 +0,0 @@ -package integration - -import ( - "context" - "log/slog" - "os" - "path/filepath" - "sync" - "testing" - - "dagger.io/dagger" - "github.com/dagger/container-use/environment" - "github.com/dagger/container-use/repository" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -var ( - testDaggerClient *dagger.Client - daggerOnce sync.Once - daggerErr error -) - -// init sets up logging for tests -func init() { - // Only show warnings and errors in tests unless TEST_VERBOSE is set - level := slog.LevelWarn - if os.Getenv("TEST_VERBOSE") != "" { - level = slog.LevelInfo - } - - slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{ - Level: level, - }))) -} - -// WithRepository runs a test function with an isolated repository and UserActions -func WithRepository(t *testing.T, name string, setup RepositorySetup, fn func(t *testing.T, repo *repository.Repository, user *UserActions)) { - // Initialize Dagger (needed for environment operations) - initializeDaggerOnce(t) - - ctx := context.Background() - - // Create isolated temp directories - repoDir, err := os.MkdirTemp("", "cu-test-"+name+"-*") - require.NoError(t, err, "Failed to create repo dir") - - configDir, err := os.MkdirTemp("", "cu-test-config-"+name+"-*") - require.NoError(t, err, "Failed to create config dir") - - // Initialize git repo - cmds := [][]string{ - {"init", "--initial-branch=main"}, - {"config", "user.email", "test@example.com"}, - {"config", "user.name", "Test User"}, - {"config", "commit.gpgsign", "false"}, - } - - for _, cmd := range cmds { - _, err := repository.RunGitCommand(ctx, repoDir, cmd...) - require.NoError(t, err, "Failed to run git %v", cmd) - } - - // Run setup to populate repo - if setup != nil { - setup(t, repoDir) - } - - // Open repository with isolated base path - repo, err := repository.OpenWithBasePath(ctx, repoDir, configDir) - require.NoError(t, err, "Failed to open repository") - - // Create UserActions with extended capabilities - user := NewUserActions(t, repo, testDaggerClient).WithDirectAccess(repoDir, configDir) - - // Cleanup - t.Cleanup(func() { - // Clean up any environments created during the test - envs, _ := repo.List(context.Background()) - for _, env := range envs { - repo.Delete(context.Background(), env.ID) - } - - // Remove directories - os.RemoveAll(repoDir) - os.RemoveAll(configDir) - }) - - // Run the test function - fn(t, repo, user) -} - -// RepositorySetup is a function that prepares a test repository -type RepositorySetup func(t *testing.T, repoDir string) - -// Common repository setups -var ( - SetupPythonRepo = func(t *testing.T, repoDir string) { - writeFile(t, repoDir, "main.py", "def main():\n print('Hello World')\n\nif __name__ == '__main__':\n main()\n") - writeFile(t, repoDir, "requirements.txt", "requests==2.31.0\nnumpy==1.24.0\n") - writeFile(t, repoDir, ".gitignore", "__pycache__/\n*.pyc\n.env\nvenv/\n") - gitCommit(t, repoDir, "Initial Python project") - } - - SetupPythonRepoNoGitignore = func(t *testing.T, repoDir string) { - writeFile(t, repoDir, "main.py", "def main():\n print('Hello World')\n\nif __name__ == '__main__':\n main()\n") - writeFile(t, repoDir, "requirements.txt", "requests==2.31.0\nnumpy==1.24.0\n") - gitCommit(t, repoDir, "Initial Python project") - } - - SetupNodeRepo = func(t *testing.T, repoDir string) { - packageJSON := `{ - "name": "test-project", - "version": "1.0.0", - "main": "index.js", - "scripts": { - "start": "node index.js", - "test": "jest" - }, - "dependencies": { - "express": "^4.18.0" - } -}` - writeFile(t, repoDir, "package.json", packageJSON) - writeFile(t, repoDir, "index.js", "console.log('Hello from Node.js');\n") - writeFile(t, repoDir, ".gitignore", "node_modules/\n.env\n") - gitCommit(t, repoDir, "Initial Node project") - } - - SetupEmptyRepo = func(t *testing.T, repoDir string) { - writeFile(t, repoDir, "README.md", "# Test Project\n") - gitCommit(t, repoDir, "Initial commit") - } - - SetupRepoWithGitConfig = func(t *testing.T, repoDir string) { - // Set project-specific git config - ctx := context.Background() - _, err := repository.RunGitCommand(ctx, repoDir, "config", "user.name", "Project User") - require.NoError(t, err, "Failed to set project user.name") - _, err = repository.RunGitCommand(ctx, repoDir, "config", "user.email", "project@example.com") - require.NoError(t, err, "Failed to set project user.email") - - writeFile(t, repoDir, "README.md", "# Project with custom git config\n") - gitCommit(t, repoDir, "Initial commit with project config") - } - - SetupRepoWithConflictingConfig = func(t *testing.T, repoDir string) { - // This assumes there might be global config, and sets local config to override it - ctx := context.Background() - _, err := repository.RunGitCommand(ctx, repoDir, "config", "user.name", "Local Project User") - require.NoError(t, err, "Failed to set local user.name") - _, err = repository.RunGitCommand(ctx, repoDir, "config", "user.email", "local@project.com") - require.NoError(t, err, "Failed to set local user.email") - - writeFile(t, repoDir, "README.md", "# Project with conflicting config\n") - gitCommit(t, repoDir, "Initial commit with local config") - } - - SetupRepoWithGitHooks = func(t *testing.T, repoDir string) { - // Create git hooks directory - hooksDir := filepath.Join(repoDir, ".git/hooks") - err := os.MkdirAll(hooksDir, 0755) - require.NoError(t, err, "Failed to create hooks directory") - - // Pre-commit hook that would block "forbidden.txt" - preCommitHook := `#!/bin/sh -echo "This pre-commit hook should never run in container-use" -if [ -f "forbidden.txt" ]; then - echo "Error: forbidden.txt is not allowed" - exit 1 -fi -exit 0` - writeFile(t, hooksDir, "pre-commit", preCommitHook) - err = os.Chmod(filepath.Join(hooksDir, "pre-commit"), 0755) - require.NoError(t, err, "Failed to make pre-commit hook executable") - - // Post-commit hook that creates evidence file - postCommitHook := `#!/bin/sh -echo "Hook ran at $(date)" >> .hook-evidence` - writeFile(t, hooksDir, "post-commit", postCommitHook) - err = os.Chmod(filepath.Join(hooksDir, "post-commit"), 0755) - require.NoError(t, err, "Failed to make post-commit hook executable") - - writeFile(t, repoDir, "README.md", "# Project with git hooks\n") - gitCommit(t, repoDir, "Initial commit with hooks") - } - - SetupRepoWithFailingHooks = func(t *testing.T, repoDir string) { - // Create git hooks directory - hooksDir := filepath.Join(repoDir, ".git/hooks") - err := os.MkdirAll(hooksDir, 0755) - require.NoError(t, err, "Failed to create hooks directory") - - // Pre-commit hook that always fails - preCommitHook := `#!/bin/sh -echo "This failing pre-commit hook should never run" -exit 1` - writeFile(t, hooksDir, "pre-commit", preCommitHook) - err = os.Chmod(filepath.Join(hooksDir, "pre-commit"), 0755) - require.NoError(t, err, "Failed to make pre-commit hook executable") - - // Pre-push hook that also fails - prePushHook := `#!/bin/sh -echo "This failing pre-push hook should never run" -exit 1` - writeFile(t, hooksDir, "pre-push", prePushHook) - err = os.Chmod(filepath.Join(hooksDir, "pre-push"), 0755) - require.NoError(t, err, "Failed to make pre-push hook executable") - - writeFile(t, repoDir, "README.md", "# Project with failing hooks\n") - gitCommit(t, repoDir, "Initial commit with failing hooks") - } -) - -// Helper functions for repository setup -func writeFile(t *testing.T, repoDir, path, content string) { - fullPath := filepath.Join(repoDir, path) - dir := filepath.Dir(fullPath) - err := os.MkdirAll(dir, 0755) - require.NoError(t, err, "Failed to create dir") - err = os.WriteFile(fullPath, []byte(content), 0600) - require.NoError(t, err, "Failed to write file") -} - -func gitCommit(t *testing.T, repoDir, message string) { - ctx := context.Background() - _, err := repository.RunGitCommand(ctx, repoDir, "add", ".") - require.NoError(t, err, "Failed to stage files") - _, err = repository.RunGitCommand(ctx, repoDir, "-c", "core.hooksPath=/dev/null", "commit", "-m", message) - require.NoError(t, err, "Failed to commit") -} - -// initializeDaggerOnce initializes Dagger client once for all tests -func initializeDaggerOnce(t *testing.T) { - daggerOnce.Do(func() { - if testDaggerClient != nil { - return - } - - ctx := context.Background() - client, err := dagger.Connect(ctx) - if err != nil { - daggerErr = err - return - } - - testDaggerClient = client - }) - - if daggerErr != nil { - t.Skipf("Skipping test - Dagger not available: %v", daggerErr) - } -} - -// UserActions provides test helpers that mirror MCP tool behavior exactly -// These represent what a user would experience when using the MCP tools -type UserActions struct { - t *testing.T - ctx context.Context - repo *repository.Repository - dag *dagger.Client - repoDir string // Source directory (for direct manipulation) - configDir string // Container-use config directory -} - -func NewUserActions(t *testing.T, repo *repository.Repository, dag *dagger.Client) *UserActions { - return &UserActions{ - t: t, - ctx: context.Background(), - repo: repo, - dag: dag, - } -} - -// WithDirectAccess adds direct filesystem access for edge case testing -func (u *UserActions) WithDirectAccess(repoDir, configDir string) *UserActions { - u.repoDir = repoDir - u.configDir = configDir - return u -} - -// FileWrite mirrors environment_file_write MCP tool behavior -func (u *UserActions) FileWrite(envID, targetFile, contents, explanation string) { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - err = env.FileWrite(u.ctx, explanation, targetFile, contents) - require.NoError(u.t, err, "FileWrite should succeed") - - err = u.repo.Update(u.ctx, env, explanation) - require.NoError(u.t, err, "repo.Update after FileWrite should succeed") -} - -// RunCommand mirrors environment_run_cmd MCP tool behavior -func (u *UserActions) RunCommand(envID, command, explanation string) string { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - output, err := env.Run(u.ctx, command, "/bin/sh", false) - require.NoError(u.t, err, "Run command should succeed") - - err = u.repo.Update(u.ctx, env, explanation) - require.NoError(u.t, err, "repo.Update after Run should succeed") - - return output -} - -// CreateEnvironment mirrors environment_create MCP tool behavior -func (u *UserActions) CreateEnvironment(title, explanation string) *environment.Environment { - env, err := u.repo.Create(u.ctx, u.dag, title, explanation, "HEAD") - require.NoError(u.t, err, "Create environment should succeed") - return env -} - -// UpdateEnvironment mirrors environment_update MCP tool behavior -func (u *UserActions) UpdateEnvironment(envID, title, explanation string, config *environment.EnvironmentConfig) { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - if title != "" { - env.State.Title = title - } - - err = env.UpdateConfig(u.ctx, config) - require.NoError(u.t, err, "UpdateConfig should succeed") - - err = u.repo.Update(u.ctx, env, explanation) - require.NoError(u.t, err, "repo.Update after UpdateConfig should succeed") -} - -// FileDelete mirrors environment_file_delete MCP tool behavior -func (u *UserActions) FileDelete(envID, targetFile, explanation string) { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - err = env.FileDelete(u.ctx, explanation, targetFile) - require.NoError(u.t, err, "FileDelete should succeed") - - err = u.repo.Update(u.ctx, env, explanation) - require.NoError(u.t, err, "repo.Update after FileDelete should succeed") -} - -// FileRead mirrors environment_file_read MCP tool behavior (read-only, no update) -func (u *UserActions) FileRead(envID, targetFile string) string { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - content, err := env.FileRead(u.ctx, targetFile, true, 0, 0) - require.NoError(u.t, err, "FileRead should succeed") - return content -} - -// FileReadExpectError is for testing expected failures -func (u *UserActions) FileReadExpectError(envID, targetFile string) { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Failed to get environment %s", envID) - - _, err = env.FileRead(u.ctx, targetFile, true, 0, 0) - assert.Error(u.t, err, "FileRead should fail for %s", targetFile) -} - -// GetEnvironment retrieves an environment by ID - mirrors how MCP tools work -// Each MCP tool call starts fresh by getting the environment from the repository -func (u *UserActions) GetEnvironment(envID string) *environment.Environment { - env, err := u.repo.Get(u.ctx, u.dag, envID) - require.NoError(u.t, err, "Should be able to get environment %s", envID) - return env -} - -// --- Direct manipulation methods for edge case testing --- - -// WriteSourceFile writes directly to the source repository -func (u *UserActions) WriteSourceFile(path, content string) { - require.NotEmpty(u.t, u.repoDir, "Need direct access for source file manipulation") - fullPath := filepath.Join(u.repoDir, path) - dir := filepath.Dir(fullPath) - - err := os.MkdirAll(dir, 0755) - require.NoError(u.t, err, "Failed to create dir") - - err = os.WriteFile(fullPath, []byte(content), 0600) - require.NoError(u.t, err, "Failed to write source file") -} - -// WorktreePath returns the worktree path for an environment, handling errors -func (u *UserActions) WorktreePath(envID string) string { - worktreePath, err := u.repo.WorktreePath(envID) - require.NoError(u.t, err, "Failed to get worktree path for environment %s", envID) - return worktreePath -} - -// ReadWorktreeFile reads directly from an environment's worktree -func (u *UserActions) ReadWorktreeFile(envID, path string) string { - worktreePath := u.WorktreePath(envID) - fullPath := filepath.Join(worktreePath, path) - content, err := os.ReadFile(fullPath) - require.NoError(u.t, err, "Failed to read worktree file") - return string(content) -} - -// CorruptWorktree simulates worktree corruption for recovery testing -func (u *UserActions) CorruptWorktree(envID string) { - worktreePath := u.WorktreePath(envID) - - // Remove .git directory to corrupt the worktree - gitDir := filepath.Join(worktreePath, ".git") - err := os.RemoveAll(gitDir) - require.NoError(u.t, err, "Failed to corrupt worktree") -} - -// GitCommand runs a git command in the source repository -func (u *UserActions) GitCommand(args ...string) string { - require.NotEmpty(u.t, u.repoDir, "Need direct access for git commands") - output, err := repository.RunGitCommand(u.ctx, u.repoDir, args...) - require.NoError(u.t, err, "Git command failed: %v", args) - return output -} - -// WriteFileInSourceRepo writes a file to the source repo and commits it -func (u *UserActions) WriteFileInSourceRepo(path, content, commitMessage string) { - require.NotEmpty(u.t, u.repoDir, "Need direct access for source file manipulation") - writeFile(u.t, u.repoDir, path, content) - gitCommit(u.t, u.repoDir, commitMessage) -} - -// CreateBranchInSourceRepo creates and checks out a new branch in the source repo -func (u *UserActions) CreateBranchInSourceRepo(branchName string) { - u.GitCommand("checkout", "-b", branchName) -} - -// CheckoutBranchInSourceRepo checks out an existing branch in the source repo -func (u *UserActions) CheckoutBranchInSourceRepo(branchName string) { - u.GitCommand("checkout", branchName) -} From d2ef95935361e5a43e38943616eed5a2cb8fea8c Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:19:24 -0500 Subject: [PATCH 3/9] refactor(mcpserver): collapse wrapTool and wrapToolWithClient into one wrapper --- mcpserver/tools.go | 48 ++++++++++++++++------------------------- mcpserver/tools_test.go | 4 ++-- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/mcpserver/tools.go b/mcpserver/tools.go index 93aea439..ff19c292 100644 --- a/mcpserver/tools.go +++ b/mcpserver/tools.go @@ -171,7 +171,7 @@ func RunStdioServer(ctx context.Context, dag *dagger.Client, singleTenant bool) }) for _, t := range createTools(singleTenant) { - s.AddTool(t.Definition, wrapToolWithClient(t, dag, singleTenant).Handler) + s.AddTool(t.Definition, wrapTool(t, dag, singleTenant).Handler) } slog.Info("starting server") @@ -191,21 +191,21 @@ func RunStdioServer(ctx context.Context, dag *dagger.Client, singleTenant bool) func createTools(singleTenant bool) []*Tool { return []*Tool{ - wrapTool(createEnvironmentOpenTool()), - wrapTool(createEnvironmentCreateTool(singleTenant)), - wrapTool(createEnvironmentUpdateMetadataTool(singleTenant)), - wrapTool(createEnvironmentConfigTool(singleTenant)), - wrapTool(createEnvironmentListTool(singleTenant)), - wrapTool(createEnvironmentRunCmdTool(singleTenant)), - wrapTool(createEnvironmentFileReadTool(singleTenant)), - wrapTool(createEnvironmentFileListTool(singleTenant)), - wrapTool(createEnvironmentFileWriteTool(singleTenant)), - wrapTool(createEnvironmentFileEditTool(singleTenant)), - wrapTool(createEnvironmentFileDeleteTool(singleTenant)), - wrapTool(createEnvironmentAddServiceTool(singleTenant)), - wrapTool(createEnvironmentCheckpointTool(singleTenant)), - wrapTool(createEnvironmentLogTool()), - wrapTool(createEnvironmentDiffTool()), + createEnvironmentOpenTool(), + createEnvironmentCreateTool(singleTenant), + createEnvironmentUpdateMetadataTool(singleTenant), + createEnvironmentConfigTool(singleTenant), + createEnvironmentListTool(singleTenant), + createEnvironmentRunCmdTool(singleTenant), + createEnvironmentFileReadTool(singleTenant), + createEnvironmentFileListTool(singleTenant), + createEnvironmentFileWriteTool(singleTenant), + createEnvironmentFileEditTool(singleTenant), + createEnvironmentFileDeleteTool(singleTenant), + createEnvironmentAddServiceTool(singleTenant), + createEnvironmentCheckpointTool(singleTenant), + createEnvironmentLogTool(), + createEnvironmentDiffTool(), } } @@ -213,7 +213,7 @@ func Tools() []*Tool { return createTools(false) // Default to multi-tenant mode when called outside of RunStdioServer } -func wrapTool(tool *Tool) *Tool { +func wrapTool(tool *Tool, dag *dagger.Client, singleTenant bool) *Tool { return &Tool{ Definition: tool.Definition, Handler: func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { @@ -221,6 +221,8 @@ func wrapTool(tool *Tool) *Tool { defer func() { slog.Info("Tool finished", "tool", tool.Definition.Name) }() + ctx = context.WithValue(ctx, daggerClientKey{}, dag) + ctx = context.WithValue(ctx, singleTenantKey{}, singleTenant) response, err := tool.Handler(ctx, request) if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -230,18 +232,6 @@ func wrapTool(tool *Tool) *Tool { } } -// keeping this modular for now. we could move tool registration to RunStdioServer and collapse the 2 wrapTool functions. -func wrapToolWithClient(tool *Tool, dag *dagger.Client, singleTenant bool) *Tool { - return &Tool{ - Definition: tool.Definition, - Handler: func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - ctx = context.WithValue(ctx, daggerClientKey{}, dag) - ctx = context.WithValue(ctx, singleTenantKey{}, singleTenant) - return tool.Handler(ctx, request) - }, - } -} - type EnvironmentResponse struct { ID string `json:"id"` Title string `json:"title"` diff --git a/mcpserver/tools_test.go b/mcpserver/tools_test.go index 665d1166..77233630 100644 --- a/mcpserver/tools_test.go +++ b/mcpserver/tools_test.go @@ -97,7 +97,7 @@ func TestWrapTool(t *testing.T) { return mcp.NewToolResultText("ok"), nil } - wrapped := wrapTool(tool) + wrapped := wrapTool(tool, nil, false) assert.Equal(t, tool.Definition.Name, wrapped.Definition.Name) _, err := wrapped.Handler(context.Background(), mcp.CallToolRequest{}) @@ -119,7 +119,7 @@ func TestWrapToolWithClient(t *testing.T) { return mcp.NewToolResultText("ok"), nil } - wrapped := wrapToolWithClient(tool, sentinel, true) + wrapped := wrapTool(tool, sentinel, true) _, err := wrapped.Handler(context.Background(), mcp.CallToolRequest{}) require.NoError(t, err) assert.True(t, called) From 9a74f14d41fa78aa8ee6263b7413923ef3ddd93e Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:21:33 -0500 Subject: [PATCH 4/9] refactor(environment): share service startup and tunneling between RunBackground and startService --- environment/environment.go | 56 ++++--------------------- environment/service.go | 84 +++++++++++++++++++++++--------------- 2 files changed, 59 insertions(+), 81 deletions(-) diff --git a/environment/environment.go b/environment/environment.go index ebb2b4fe..bd2bd7ae 100644 --- a/environment/environment.go +++ b/environment/environment.go @@ -313,65 +313,27 @@ func (env *Environment) RunBackground(ctx context.Context, command, shell string args = []string{shell, "-c", command} } displayCommand := command + " &" - serviceState := env.container() - // Expose ports - for _, port := range ports { - serviceState = serviceState.WithExposedPort(port, dagger.ContainerWithExposedPortOpts{ - Protocol: dagger.NetworkProtocolTcp, - Description: fmt.Sprintf("Port %d", port), - }) - } - - // Start the service - startCtx, cancel := context.WithTimeout(ctx, serviceStartTimeout) - defer cancel() - svc, err := serviceState.AsService(dagger.ContainerAsServiceOpts{ - Args: args, - UseEntrypoint: useEntrypoint, - }).Start(startCtx) + svc, err := env.exposeAndStartService(ctx, env.container(), args, ports, useEntrypoint) if err != nil { + err = translateServiceStartError(err) var exitErr *dagger.ExecError if errors.As(err, &exitErr) { env.Notes.AddCommand(displayCommand, exitErr.ExitCode, exitErr.Stdout, exitErr.Stderr) - return nil, fmt.Errorf("command failed with exit code %d.\nstdout: %s\nstderr: %s", exitErr.ExitCode, exitErr.Stdout, exitErr.Stderr) - } - if errors.Is(err, context.DeadlineExceeded) { - err = fmt.Errorf("service failed to start within %s timeout", serviceStartTimeout) + } else if errors.Is(err, context.DeadlineExceeded) { env.Notes.AddCommand(displayCommand, 137, "", err.Error()) - return nil, err } return nil, err } env.Notes.AddCommand(displayCommand, 0, "", "") - endpoints := EndpointMappings{} - for _, port := range ports { - endpoint := &EndpointMapping{} - endpoints[port] = endpoint - - // Expose port on the host - tunnel, err := env.dag.Host().Tunnel(svc, dagger.HostTunnelOpts{ - Ports: []dagger.PortForward{ - { - Backend: port, - Protocol: dagger.NetworkProtocolTcp, - }, - }, - }).Start(ctx) - if err != nil { - return nil, err - } - - externalEndpoint, err := tunnel.Endpoint(ctx, dagger.ServiceEndpointOpts{ - Scheme: "tcp", - }) - if err != nil { - return nil, err - } - endpoint.HostExternal = externalEndpoint + endpoints, err := env.tunnelServiceEndpoints(ctx, svc, ports) + if err != nil { + return nil, err + } + for _, port := range ports { internalEndpoint, err := svc.Endpoint(ctx, dagger.ServiceEndpointOpts{ Port: port, Scheme: "tcp", @@ -379,7 +341,7 @@ func (env *Environment) RunBackground(ctx context.Context, command, shell string if err != nil { return nil, err } - endpoint.EnvironmentInternal = internalEndpoint + endpoints[port].EnvironmentInternal = internalEndpoint } return endpoints, nil diff --git a/environment/service.go b/environment/service.go index 33674cd7..b3e15a3a 100644 --- a/environment/service.go +++ b/environment/service.go @@ -39,56 +39,39 @@ func (env *Environment) startServices(ctx context.Context) ([]*Service, error) { return services, nil } -func (env *Environment) startService(ctx context.Context, cfg *ServiceConfig) (*Service, error) { - container := env.dag.Container().From(cfg.Image) - container, err := containerWithEnvAndSecrets(env.dag, container, cfg.Env, env.State.Config.Secrets) - if err != nil { - return nil, err +func translateServiceStartError(err error) error { + var exitErr *dagger.ExecError + if errors.As(err, &exitErr) { + return fmt.Errorf("command failed with exit code %d.\nstdout: %s\nstderr: %s", exitErr.ExitCode, exitErr.Stdout, exitErr.Stderr) } - - if cfg.Command != "" { - container = container.WithExec([]string{"sh", "-c", cfg.Command}) - } - - args := []string{} - if cfg.Command != "" { - args = []string{"sh", "-c", cfg.Command} + if errors.Is(err, context.DeadlineExceeded) { + return fmt.Errorf("service failed to start within %s timeout", serviceStartTimeout) } + return err +} - // Expose ports - for _, port := range cfg.ExposedPorts { +func (env *Environment) exposeAndStartService(ctx context.Context, container *dagger.Container, args []string, ports []int, useEntrypoint bool) (*dagger.Service, error) { + for _, port := range ports { container = container.WithExposedPort(port, dagger.ContainerWithExposedPortOpts{ Protocol: dagger.NetworkProtocolTcp, Description: fmt.Sprintf("Port %d", port), }) } - // Start the service startCtx, cancel := context.WithTimeout(ctx, serviceStartTimeout) defer cancel() - svc, err := container.AsService(dagger.ContainerAsServiceOpts{ + return container.AsService(dagger.ContainerAsServiceOpts{ Args: args, - UseEntrypoint: true, + UseEntrypoint: useEntrypoint, }).Start(startCtx) - if err != nil { - var exitErr *dagger.ExecError - if errors.As(err, &exitErr) { - return nil, fmt.Errorf("command failed with exit code %d.\nstdout: %s\nstderr: %s", exitErr.ExitCode, exitErr.Stdout, exitErr.Stderr) - } - if errors.Is(err, context.DeadlineExceeded) { - return nil, fmt.Errorf("service failed to start within %s timeout", serviceStartTimeout) - } - return nil, err - } +} +func (env *Environment) tunnelServiceEndpoints(ctx context.Context, svc *dagger.Service, ports []int) (EndpointMappings, error) { endpoints := EndpointMappings{} - for _, port := range cfg.ExposedPorts { - endpoint := &EndpointMapping{ - EnvironmentInternal: fmt.Sprintf("tcp://%s:%d", cfg.Name, port), - } + for _, port := range ports { + endpoint := &EndpointMapping{} endpoints[port] = endpoint - // Expose ports on the host tunnel, err := env.dag.Host().Tunnel(svc, dagger.HostTunnelOpts{ Ports: []dagger.PortForward{ { @@ -106,11 +89,44 @@ func (env *Environment) startService(ctx context.Context, cfg *ServiceConfig) (* Scheme: "tcp", }) if err != nil { - return nil, fmt.Errorf("failed to get endpoint for service %s: %w", cfg.Name, err) + return nil, err } endpoint.HostExternal = externalEndpoint } + return endpoints, nil +} + +func (env *Environment) startService(ctx context.Context, cfg *ServiceConfig) (*Service, error) { + container := env.dag.Container().From(cfg.Image) + container, err := containerWithEnvAndSecrets(env.dag, container, cfg.Env, env.State.Config.Secrets) + if err != nil { + return nil, err + } + + if cfg.Command != "" { + container = container.WithExec([]string{"sh", "-c", cfg.Command}) + } + + args := []string{} + if cfg.Command != "" { + args = []string{"sh", "-c", cfg.Command} + } + + svc, err := env.exposeAndStartService(ctx, container, args, cfg.ExposedPorts, true) + if err != nil { + return nil, translateServiceStartError(err) + } + + endpoints, err := env.tunnelServiceEndpoints(ctx, svc, cfg.ExposedPorts) + if err != nil { + return nil, fmt.Errorf("failed to get endpoint for service %s: %w", cfg.Name, err) + } + + for _, port := range cfg.ExposedPorts { + endpoints[port].EnvironmentInternal = fmt.Sprintf("tcp://%s:%d", cfg.Name, port) + } + return &Service{ Config: cfg, Endpoints: endpoints, From 936b5ff98c1c38b52fbc5134ed536f2c9ffc7411 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:22:15 -0500 Subject: [PATCH 5/9] refactor(mcpserver): remove single-field single-tenant setters used only by tests --- mcpserver/singletenant.go | 14 -------------- mcpserver/singletenant_test.go | 7 +++---- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/mcpserver/singletenant.go b/mcpserver/singletenant.go index 6baf0646..bc1a600f 100644 --- a/mcpserver/singletenant.go +++ b/mcpserver/singletenant.go @@ -44,20 +44,6 @@ func getCurrentEnvironmentSource() (string, error) { return currentEnvironmentSource, nil } -// setCurrentEnvironmentID sets the current environment ID for single-tenant mode -func setCurrentEnvironmentID(envID string) { - currentEnvMutex.Lock() - defer currentEnvMutex.Unlock() - currentEnvironmentID = envID -} - -// setCurrentEnvironmentSource sets the current environment source for single-tenant mode -func setCurrentEnvironmentSource(envSource string) { - currentEnvMutex.Lock() - defer currentEnvMutex.Unlock() - currentEnvironmentSource = envSource -} - // setCurrentEnvironment sets both the current environment ID and source for single-tenant mode func setCurrentEnvironment(envID, envSource string) { currentEnvMutex.Lock() diff --git a/mcpserver/singletenant_test.go b/mcpserver/singletenant_test.go index c6080138..d3c76a73 100644 --- a/mcpserver/singletenant_test.go +++ b/mcpserver/singletenant_test.go @@ -9,9 +9,8 @@ func TestSingleTenantEnvironmentStorage(t *testing.T) { testEnvID := "test-env-id" testEnvSource := "/test/source/path" - // Test individual setters and getters - setCurrentEnvironmentID(testEnvID) - setCurrentEnvironmentSource(testEnvSource) + // Test setting and getting environment ID and source + setCurrentEnvironment(testEnvID, testEnvSource) retrievedID, err := getCurrentEnvironmentID() if err != nil { @@ -29,7 +28,7 @@ func TestSingleTenantEnvironmentStorage(t *testing.T) { t.Fatalf("Expected environment source %s, got: %s", testEnvSource, retrievedSource) } - // Test combined setter + // Test updating environment ID and source newEnvID := "new-env-id" newEnvSource := "/new/source/path" setCurrentEnvironment(newEnvID, newEnvSource) From 4f22b8e3bde1903feac515773a0b491de6cf7c80 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:27:07 -0500 Subject: [PATCH 6/9] refactor(agent): centralize config-file error wrapping in writeMcpConfig --- cmd/container-use/agent/configure.go | 27 +++++++++++++++++++++ cmd/container-use/agent/configure_claude.go | 7 +----- cmd/container-use/agent/configure_codex.go | 8 +----- cmd/container-use/agent/configure_cursor.go | 8 +----- cmd/container-use/agent/configure_goose.go | 7 +----- cmd/container-use/agent/configure_q.go | 8 +----- 6 files changed, 32 insertions(+), 33 deletions(-) diff --git a/cmd/container-use/agent/configure.go b/cmd/container-use/agent/configure.go index 9ce47dd6..c8b06b8d 100644 --- a/cmd/container-use/agent/configure.go +++ b/cmd/container-use/agent/configure.go @@ -174,3 +174,30 @@ func tools(prefix string) []string { } return tools } + +// writeMcpConfig handles the common read-update-write cycle for agent MCP +// configuration files. It creates the parent directory if needed, reads and +// unmarshals any existing config, applies the update, and writes the result +// back to disk. +func writeMcpConfig[T any](path string, unmarshal func([]byte, *T) error, update func(T) ([]byte, error)) error { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return fmt.Errorf("failed to create config directory: %w", err) + } + + var cfg T + if data, err := os.ReadFile(path); err == nil { + if err := unmarshal(data, &cfg); err != nil { + return fmt.Errorf("failed to parse existing config: %w", err) + } + } + + data, err := update(cfg) + if err != nil { + return fmt.Errorf("failed to update config: %w", err) + } + + if err := os.WriteFile(path, data, 0600); err != nil { + return fmt.Errorf("failed to write config: %w", err) + } + return nil +} diff --git a/cmd/container-use/agent/configure_claude.go b/cmd/container-use/agent/configure_claude.go index 6f26e419..95e2e635 100644 --- a/cmd/container-use/agent/configure_claude.go +++ b/cmd/container-use/agent/configure_claude.go @@ -97,12 +97,7 @@ func (c *ConfigureClaude) updateSettingsLocal(config ClaudeSettingsLocal) ([]byt allows = append(allows, tools...) config.Permissions.Allow = allows - // Write config back - data, err := json.MarshalIndent(config, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - return data, nil + return json.MarshalIndent(config, "", " ") } func (c *ConfigureClaude) editRules() error { diff --git a/cmd/container-use/agent/configure_codex.go b/cmd/container-use/agent/configure_codex.go index c15ee925..4ef86891 100644 --- a/cmd/container-use/agent/configure_codex.go +++ b/cmd/container-use/agent/configure_codex.go @@ -1,7 +1,6 @@ package agent import ( - "fmt" "os" "os/exec" "path/filepath" @@ -84,12 +83,7 @@ func (a *ConfigureCodex) updateCodexConfig(config map[string]any) ([]byte, error "auto_approve": tools(""), } - // Write config back - data, err := toml.Marshal(&config) - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - return data, nil + return toml.Marshal(&config) } // Save the agent rules with the container-use prompt diff --git a/cmd/container-use/agent/configure_cursor.go b/cmd/container-use/agent/configure_cursor.go index 27346834..9ac8fc50 100644 --- a/cmd/container-use/agent/configure_cursor.go +++ b/cmd/container-use/agent/configure_cursor.go @@ -2,7 +2,6 @@ package agent import ( "encoding/json" - "fmt" "os" "path/filepath" @@ -72,12 +71,7 @@ func (a *ConfigureCursor) updateMcpConfig(config MCPServersConfig) ([]byte, erro Args: []string{"stdio"}, } - // Write config back - data, err := json.MarshalIndent(config, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - return data, nil + return json.MarshalIndent(config, "", " ") } // Save the agent rules with the container-use prompt diff --git a/cmd/container-use/agent/configure_goose.go b/cmd/container-use/agent/configure_goose.go index 86323280..818e4faf 100644 --- a/cmd/container-use/agent/configure_goose.go +++ b/cmd/container-use/agent/configure_goose.go @@ -101,12 +101,7 @@ func (a *ConfigureGoose) updateGooseConfig(config map[string]any) ([]byte, error "envs": map[string]any{}, } - // Write config back - data, err := yaml.Marshal(&config) - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - return data, nil + return yaml.Marshal(&config) } // Save the agent rules with the container-use prompt diff --git a/cmd/container-use/agent/configure_q.go b/cmd/container-use/agent/configure_q.go index 29a0dbef..069b2e90 100644 --- a/cmd/container-use/agent/configure_q.go +++ b/cmd/container-use/agent/configure_q.go @@ -2,7 +2,6 @@ package agent import ( "encoding/json" - "fmt" "os" "os/exec" "path/filepath" @@ -75,12 +74,7 @@ func (a *ConfigureQ) updateMcpConfig(config MCPServersConfig) ([]byte, error) { Timeout: &[]int{60000}[0], } - // Write config back - data, err := json.MarshalIndent(config, "", " ") - if err != nil { - return nil, fmt.Errorf("failed to marshal config: %w", err) - } - return data, nil + return json.MarshalIndent(config, "", " ") } // Save the agent rules with the container-use prompt From a5eaa051f6de6bd7b9b2de368493bc25ab6e4441 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:25:39 -0500 Subject: [PATCH 7/9] refactor(agent): shared writeMcpConfig helper and remove narrating comments --- cmd/container-use/agent/configure.go | 2 +- cmd/container-use/agent/configure_claude.go | 42 ++++----------- cmd/container-use/agent/configure_codex.go | 44 +++++---------- cmd/container-use/agent/configure_cursor.go | 41 ++++---------- cmd/container-use/agent/configure_goose.go | 60 +++++++-------------- cmd/container-use/agent/configure_q.go | 41 ++++---------- 6 files changed, 60 insertions(+), 170 deletions(-) diff --git a/cmd/container-use/agent/configure.go b/cmd/container-use/agent/configure.go index c8b06b8d..a443f4eb 100644 --- a/cmd/container-use/agent/configure.go +++ b/cmd/container-use/agent/configure.go @@ -193,7 +193,7 @@ func writeMcpConfig[T any](path string, unmarshal func([]byte, *T) error, update data, err := update(cfg) if err != nil { - return fmt.Errorf("failed to update config: %w", err) + return err } if err := os.WriteFile(path, data, 0600); err != nil { diff --git a/cmd/container-use/agent/configure_claude.go b/cmd/container-use/agent/configure_claude.go index 95e2e635..a1b1aee7 100644 --- a/cmd/container-use/agent/configure_claude.go +++ b/cmd/container-use/agent/configure_claude.go @@ -3,7 +3,6 @@ package agent import ( "encoding/json" "fmt" - "os" "os/exec" "path/filepath" "strings" @@ -42,49 +41,25 @@ func (c *ConfigureClaude) description() string { } func (c *ConfigureClaude) editMcpConfig() error { - // Remove existing MCP server (ignore errors if it doesn't exist) removeCmd := exec.Command("claude", "mcp", "remove", "container-use") - _ = removeCmd.Run() // Ignore error - server might not exist + _ = removeCmd.Run() - // Add MCP server cmd := exec.Command("claude", "mcp", "add", "container-use", "--", ContainerUseBinary, "stdio") - err := cmd.Run() - if err != nil { + if err := cmd.Run(); err != nil { return fmt.Errorf("could not automatically add MCP server: %w", err) } - // Configure auto approve settings configPath := filepath.Join(".claude", "settings.local.json") - // Create directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } - var config ClaudeSettingsLocal - if data, err := os.ReadFile(configPath); err == nil { - if err := json.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing config: %w", err) - } - } - - data, err := c.updateSettingsLocal(config) - if err != nil { - return err - } - - err = os.WriteFile(configPath, data, 0600) - if err != nil { - return fmt.Errorf("failed to write config: %w", err) - } - return nil + return writeMcpConfig(configPath, func(data []byte, cfg *ClaudeSettingsLocal) error { + return json.Unmarshal(data, cfg) + }, c.updateSettingsLocal) } func (c *ConfigureClaude) updateSettingsLocal(config ClaudeSettingsLocal) ([]byte, error) { - // Initialize permissions map if nil if config.Permissions == nil { config.Permissions = &ClaudePermissions{Allow: []string{}} } - // remove save non-container-use items from allow allows := []string{} for _, tool := range config.Permissions.Allow { if !strings.HasPrefix(tool, "mcp__container-use") { @@ -92,12 +67,15 @@ func (c *ConfigureClaude) updateSettingsLocal(config ClaudeSettingsLocal) ([]byt } } - // Add container-use tools to allow tools := tools("mcp__container-use__") allows = append(allows, tools...) config.Permissions.Allow = allows - return json.MarshalIndent(config, "", " ") + data, err := json.MarshalIndent(config, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + return data, nil } func (c *ConfigureClaude) editRules() error { diff --git a/cmd/container-use/agent/configure_codex.go b/cmd/container-use/agent/configure_codex.go index 4ef86891..6cf33d45 100644 --- a/cmd/container-use/agent/configure_codex.go +++ b/cmd/container-use/agent/configure_codex.go @@ -1,7 +1,7 @@ package agent import ( - "os" + "fmt" "os/exec" "path/filepath" @@ -22,52 +22,30 @@ func NewConfigureCodex() *ConfigureCodex { } } -// Return the agents full name func (a *ConfigureCodex) name() string { return a.Name } -// Return a description of the agent func (a *ConfigureCodex) description() string { return a.Description } -// Save the MCP config with container-use enabled func (a *ConfigureCodex) editMcpConfig() error { configPath, err := homedir.Expand(filepath.Join("~", ".codex", "config.toml")) if err != nil { return err } - // Create directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } + return writeMcpConfig(configPath, func(data []byte, cfg *map[string]any) error { + return toml.Unmarshal(data, cfg) + }, a.updateCodexConfig) +} - // Read existing config or create new - var config map[string]any - if data, err := os.ReadFile(configPath); err == nil { - if err := toml.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing config: %w", err) - } - } else { +func (a *ConfigureCodex) updateCodexConfig(config map[string]any) ([]byte, error) { + if config == nil { config = make(map[string]any) } - data, err := a.updateCodexConfig(config) - if err != nil { - return err - } - - err = os.WriteFile(configPath, data, 0600) - if err != nil { - return fmt.Errorf("failed to write config: %w", err) - } - return nil -} - -func (a *ConfigureCodex) updateCodexConfig(config map[string]any) ([]byte, error) { - // Get mcp_servers map var mcpServers map[string]any if servers, ok := config["mcp_servers"]; ok { mcpServers = servers.(map[string]any) @@ -76,17 +54,19 @@ func (a *ConfigureCodex) updateCodexConfig(config map[string]any) ([]byte, error config["mcp_servers"] = mcpServers } - // Add container-use server mcpServers["container-use"] = map[string]any{ "command": ContainerUseBinary, "args": []any{"stdio"}, "auto_approve": tools(""), } - return toml.Marshal(&config) + data, err := toml.Marshal(&config) + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + return data, nil } -// Save the agent rules with the container-use prompt func (a *ConfigureCodex) editRules() error { agentsFile := "AGENTS.md" return saveRulesFile(agentsFile, rules.AgentRules) diff --git a/cmd/container-use/agent/configure_cursor.go b/cmd/container-use/agent/configure_cursor.go index 9ac8fc50..fb26cfc9 100644 --- a/cmd/container-use/agent/configure_cursor.go +++ b/cmd/container-use/agent/configure_cursor.go @@ -2,7 +2,7 @@ package agent import ( "encoding/json" - "os" + "fmt" "path/filepath" "github.com/dagger/container-use/rules" @@ -20,43 +20,18 @@ func NewConfigureCursor() *ConfigureCursor { } } -// Return the agents full name func (a *ConfigureCursor) name() string { return a.Name } -// Return a description of the agent func (a *ConfigureCursor) description() string { return a.Description } -// Save the MCP config with container-use enabled func (a *ConfigureCursor) editMcpConfig() error { - configPath := filepath.Join(".cursor", "mcp.json") - - // Create directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } - - // Read existing config or create new - var config MCPServersConfig - if data, err := os.ReadFile(configPath); err == nil { - if err := json.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing config: %w", err) - } - } - - data, err := a.updateMcpConfig(config) - if err != nil { - return err - } - - err = os.WriteFile(configPath, data, 0600) - if err != nil { - return fmt.Errorf("failed to write config: %w", err) - } - return nil + return writeMcpConfig(filepath.Join(".cursor", "mcp.json"), func(data []byte, cfg *MCPServersConfig) error { + return json.Unmarshal(data, cfg) + }, a.updateMcpConfig) } func (a *ConfigureCursor) updateMcpConfig(config MCPServersConfig) ([]byte, error) { @@ -65,16 +40,18 @@ func (a *ConfigureCursor) updateMcpConfig(config MCPServersConfig) ([]byte, erro config.MCPServers = make(map[string]MCPServer) } - // Add container-use server config.MCPServers["container-use"] = MCPServer{ Command: ContainerUseBinary, Args: []string{"stdio"}, } - return json.MarshalIndent(config, "", " ") + data, err := json.MarshalIndent(config, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + return data, nil } -// Save the agent rules with the container-use prompt func (a *ConfigureCursor) editRules() error { rulesFile := filepath.Join(".cursor", "rules", "container-use.mdc") return saveRulesFile(rulesFile, rules.CursorRules) diff --git a/cmd/container-use/agent/configure_goose.go b/cmd/container-use/agent/configure_goose.go index 818e4faf..6cd3302b 100644 --- a/cmd/container-use/agent/configure_goose.go +++ b/cmd/container-use/agent/configure_goose.go @@ -24,65 +24,41 @@ func NewConfigureGoose() *ConfigureGoose { } } -// Return the agents full name func (a *ConfigureGoose) name() string { return a.Name } -// Return a description of the agent func (a *ConfigureGoose) description() string { return a.Description } -// Save the MCP config with container-use enabled -func (a *ConfigureGoose) editMcpConfig() error { - var configPath string - var err error - +func gooseConfigPath() (string, error) { if runtime.GOOS == "windows" { - // Windows: %APPDATA%\Block\goose\config\config.yaml - // Reference: https://block.github.io/goose/docs/guides/config-file appData := os.Getenv("APPDATA") if appData == "" { - return fmt.Errorf("APPDATA environment variable not set") + return "", fmt.Errorf("APPDATA environment variable not set") } - configPath = filepath.Join(appData, "Block", "goose", "config", "config.yaml") - } else { - // macOS/Linux: ~/.config/goose/config.yaml - configPath, err = homedir.Expand(filepath.Join("~", ".config", "goose", "config.yaml")) - if err != nil { - return err - } - } - - // Create directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } - - // Read existing config or create new - var config map[string]any - if data, err := os.ReadFile(configPath); err == nil { - if err := yaml.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing config: %w", err) - } - } else { - config = make(map[string]any) + return filepath.Join(appData, "Block", "goose", "config", "config.yaml"), nil } + return homedir.Expand(filepath.Join("~", ".config", "goose", "config.yaml")) +} - data, err := a.updateGooseConfig(config) +func (a *ConfigureGoose) editMcpConfig() error { + configPath, err := gooseConfigPath() if err != nil { return err } - if err := os.WriteFile(configPath, data, 0600); err != nil { - return fmt.Errorf("failed to write config: %w", err) - } - return nil + return writeMcpConfig(configPath, func(data []byte, cfg *map[string]any) error { + return yaml.Unmarshal(data, cfg) + }, a.updateGooseConfig) } func (a *ConfigureGoose) updateGooseConfig(config map[string]any) ([]byte, error) { - // Get extensions map + if config == nil { + config = make(map[string]any) + } + var extensions map[string]any if ext, ok := config["extensions"]; ok { extensions = ext.(map[string]any) @@ -91,7 +67,6 @@ func (a *ConfigureGoose) updateGooseConfig(config map[string]any) ([]byte, error config["extensions"] = extensions } - // Add container-use extension extensions["container-use"] = map[string]any{ "name": "container-use", "type": "stdio", @@ -101,10 +76,13 @@ func (a *ConfigureGoose) updateGooseConfig(config map[string]any) ([]byte, error "envs": map[string]any{}, } - return yaml.Marshal(&config) + data, err := yaml.Marshal(&config) + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + return data, nil } -// Save the agent rules with the container-use prompt func (a *ConfigureGoose) editRules() error { return saveRulesFile(".goosehints", rules.AgentRules) } diff --git a/cmd/container-use/agent/configure_q.go b/cmd/container-use/agent/configure_q.go index 069b2e90..d2627cea 100644 --- a/cmd/container-use/agent/configure_q.go +++ b/cmd/container-use/agent/configure_q.go @@ -2,7 +2,7 @@ package agent import ( "encoding/json" - "os" + "fmt" "os/exec" "path/filepath" @@ -21,43 +21,18 @@ func NewConfigureQ() *ConfigureQ { } } -// Return the agents full name func (a *ConfigureQ) name() string { return a.Name } -// Return a description of the agent func (a *ConfigureQ) description() string { return a.Description } -// Save the MCP config with container-use enabled func (a *ConfigureQ) editMcpConfig() error { - configPath := filepath.Join(".amazonq", "mcp.json") - - // Create directory if it doesn't exist - if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) - } - - // Read existing config or create new - var config MCPServersConfig - if data, err := os.ReadFile(configPath); err == nil { - if err := json.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing config: %w", err) - } - } - - data, err := a.updateMcpConfig(config) - if err != nil { - return err - } - - err = os.WriteFile(configPath, data, 0600) - if err != nil { - return fmt.Errorf("failed to write config: %w", err) - } - return nil + return writeMcpConfig(filepath.Join(".amazonq", "mcp.json"), func(data []byte, cfg *MCPServersConfig) error { + return json.Unmarshal(data, cfg) + }, a.updateMcpConfig) } func (a *ConfigureQ) updateMcpConfig(config MCPServersConfig) ([]byte, error) { @@ -66,7 +41,6 @@ func (a *ConfigureQ) updateMcpConfig(config MCPServersConfig) ([]byte, error) { config.MCPServers = make(map[string]MCPServer) } - // Add container-use server config.MCPServers["container-use"] = MCPServer{ Command: ContainerUseBinary, Args: []string{"stdio"}, @@ -74,10 +48,13 @@ func (a *ConfigureQ) updateMcpConfig(config MCPServersConfig) ([]byte, error) { Timeout: &[]int{60000}[0], } - return json.MarshalIndent(config, "", " ") + data, err := json.MarshalIndent(config, "", " ") + if err != nil { + return nil, fmt.Errorf("failed to marshal config: %w", err) + } + return data, nil } -// Save the agent rules with the container-use prompt func (a *ConfigureQ) editRules() error { return saveRulesFile(".amazonq/rules/container-use.md", rules.AgentRules) } From 388a0127247dbfca61f993ec738b9bae0681a114 Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:28:58 -0500 Subject: [PATCH 8/9] refactor(environment): introduce FileEditRequest to reduce FileEdit parameter count --- environment/filesystem.go | 48 ++++++++++++++++++--------------------- mcpserver/tools.go | 14 ++++++------ 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/environment/filesystem.go b/environment/filesystem.go index f1eee215..3c16a996 100644 --- a/environment/filesystem.go +++ b/environment/filesystem.go @@ -54,22 +54,28 @@ func (env *Environment) FileWrite(ctx context.Context, explanation, targetFile, return nil } -func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, search, replace, matchID string) error { - // Check if the file is within a submodule - if err := env.validateNotSubmoduleFile(targetFile); err != nil { +type FileEditRequest struct { + Explanation string + TargetFile string + Search string + Replace string + MatchID string +} + +func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) error { + if err := env.validateNotSubmoduleFile(req.TargetFile); err != nil { return err } - contents, err := env.container().File(targetFile).Contents(ctx) + contents, err := env.container().File(req.TargetFile).Contents(ctx) if err != nil { return err } - // Find all matches of the search text matches := []int{} cursor := 0 for { - index := strings.Index(contents[cursor:], search) + index := strings.Index(contents[cursor:], req.Search) if index == -1 { break } @@ -79,58 +85,48 @@ func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, s } if len(matches) == 0 { - return fmt.Errorf("search text not found in file %s", targetFile) + return fmt.Errorf("search text not found in file %s", req.TargetFile) } - // If there are multiple matches and no matchID is provided, return an error with all matches - if len(matches) > 1 && matchID == "" { + if len(matches) > 1 && req.MatchID == "" { var matchDescriptions []string for i, matchIndex := range matches { - // Generate a unique ID for each match - id := generateMatchID(targetFile, search, replace, i) - - // Get context around the match (3 lines before and after) + id := generateMatchID(req.TargetFile, req.Search, req.Replace, i) context := getMatchContext(contents, matchIndex) - matchDescriptions = append(matchDescriptions, fmt.Sprintf("Match %d (ID: %s):\n%s", i+1, id, context)) } return fmt.Errorf("multiple matches found for search text in %s. Please specify which_match parameter with one of the following IDs:\n\n%s", - targetFile, strings.Join(matchDescriptions, "\n\n")) + req.TargetFile, strings.Join(matchDescriptions, "\n\n")) } - // Determine which match to replace var targetMatchIndex int if len(matches) == 1 { targetMatchIndex = matches[0] } else { - // Find the match with the specified ID found := false for i, matchIndex := range matches { - id := generateMatchID(targetFile, search, replace, i) - if id == matchID { + id := generateMatchID(req.TargetFile, req.Search, req.Replace, i) + if id == req.MatchID { targetMatchIndex = matchIndex found = true break } } if !found { - return fmt.Errorf("match ID %s not found", matchID) + return fmt.Errorf("match ID %s not found", req.MatchID) } } - // Replace the specific match - newContents := contents[:targetMatchIndex] + replace + contents[targetMatchIndex+len(search):] + newContents := contents[:targetMatchIndex] + req.Replace + contents[targetMatchIndex+len(req.Search):] - // Apply the changes using `Directory.withPatch` so we don't have to spit out - // the entire contents - patch := godiffpatch.GeneratePatch(targetFile, contents, newContents) + patch := godiffpatch.GeneratePatch(req.TargetFile, contents, newContents) ctr := env.container() err = env.apply(ctx, ctr.WithDirectory(".", ctr.Directory(".").WithPatch(patch))) if err != nil { return fmt.Errorf("failed applying file edit, skipping git propagation: %w", err) } - env.Notes.Add("Edit %s", targetFile) + env.Notes.Add("Edit %s", req.TargetFile) return nil } diff --git a/mcpserver/tools.go b/mcpserver/tools.go index ff19c292..458e5b8d 100644 --- a/mcpserver/tools.go +++ b/mcpserver/tools.go @@ -783,13 +783,13 @@ func createEnvironmentFileEditTool(singleTenant bool) *Tool { return nil, err } - if err := env.FileEdit(ctx, - request.GetString("explanation", ""), - targetFile, - search, - replace, - request.GetString("which_match", ""), - ); err != nil { + if err := env.FileEdit(ctx, environment.FileEditRequest{ + Explanation: request.GetString("explanation", ""), + TargetFile: targetFile, + Search: search, + Replace: replace, + MatchID: request.GetString("which_match", ""), + }); err != nil { return mcp.NewToolResultErrorFromErr("failed to write file", err), nil } From 8deacef9ce19997c33715c28b07430431d16377e Mon Sep 17 00:00:00 2001 From: Sisyphus Date: Sun, 9 Aug 2026 12:30:29 -0500 Subject: [PATCH 9/9] refactor(environment): split FileRead into FileRead and FileReadRange --- environment/filesystem.go | 9 +++++---- environment/integration/actions.go | 4 ++-- environment/integration/integration_test.go | 4 ++-- environment/integration/repository_test.go | 20 ++++++++++---------- mcpserver/tools.go | 11 ++++++++--- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/environment/filesystem.go b/environment/filesystem.go index 3c16a996..a322c616 100644 --- a/environment/filesystem.go +++ b/environment/filesystem.go @@ -10,14 +10,15 @@ import ( godiffpatch "github.com/sourcegraph/go-diff-patch" ) -func (env *Environment) FileRead(ctx context.Context, targetFile string, shouldReadEntireFile bool, startLineOneIndexedInclusive int, endLineOneIndexedInclusive int) (string, error) { +func (env *Environment) FileRead(ctx context.Context, targetFile string) (string, error) { + return env.container().File(targetFile).Contents(ctx) +} + +func (env *Environment) FileReadRange(ctx context.Context, targetFile string, startLineOneIndexedInclusive, endLineOneIndexedInclusive int) (string, error) { file, err := env.container().File(targetFile).Contents(ctx) if err != nil { return "", err } - if shouldReadEntireFile { - return file, err - } lines := strings.Split(file, "\n") start := startLineOneIndexedInclusive - 1 diff --git a/environment/integration/actions.go b/environment/integration/actions.go index 482e8da4..718f665d 100644 --- a/environment/integration/actions.go +++ b/environment/integration/actions.go @@ -104,7 +104,7 @@ func (u *UserActions) FileRead(envID, targetFile string) string { env, err := u.repo.Get(u.ctx, u.dag, envID) require.NoError(u.t, err, "Failed to get environment %s", envID) - content, err := env.FileRead(u.ctx, targetFile, true, 0, 0) + content, err := env.FileRead(u.ctx, targetFile) require.NoError(u.t, err, "FileRead should succeed") return content } @@ -114,7 +114,7 @@ func (u *UserActions) FileReadExpectError(envID, targetFile string) { env, err := u.repo.Get(u.ctx, u.dag, envID) require.NoError(u.t, err, "Failed to get environment %s", envID) - _, err = env.FileRead(u.ctx, targetFile, true, 0, 0) + _, err = env.FileRead(u.ctx, targetFile) assert.Error(u.t, err, "FileRead should fail for %s", targetFile) } diff --git a/environment/integration/integration_test.go b/environment/integration/integration_test.go index 7e06def6..9a62b752 100644 --- a/environment/integration/integration_test.go +++ b/environment/integration/integration_test.go @@ -388,11 +388,11 @@ func TestWeirdUserScenarios(t *testing.T) { require.NoError(t, err) // Try to use env1 while in repo2 (should fail) - _, err = env1.FileRead(ctx, "main.py", true, 0, 0) + _, err = env1.FileRead(ctx, "main.py") assert.Error(t, err, "Should fail to read repo2 files from repo1 environment") // The environment is still tied to repo1 - jsContent, err := env1.FileRead(ctx, "app.js", true, 0, 0) + jsContent, err := env1.FileRead(ctx, "app.js") require.NoError(t, err) assert.Contains(t, jsContent, "repo1", "Environment should still access its original repo") }) diff --git a/environment/integration/repository_test.go b/environment/integration/repository_test.go index 9e0ede64..c6b825c6 100644 --- a/environment/integration/repository_test.go +++ b/environment/integration/repository_test.go @@ -192,7 +192,7 @@ func TestRepositoryCreateFromGitRef(t *testing.T) { // Test creating environment from HEAD (default behavior) envFromHead := user.CreateEnvironment("From HEAD", "Environment from HEAD") - content, err := envFromHead.FileRead(ctx, "main.txt", true, 0, 0) + content, err := envFromHead.FileRead(ctx, "main.txt") require.NoError(t, err) assert.Contains(t, content, "main content") @@ -202,11 +202,11 @@ func TestRepositoryCreateFromGitRef(t *testing.T) { assert.NotNil(t, envFromBranch) // Should have feature.txt but not main.txt - featureContent, err := envFromBranch.FileRead(ctx, "feature.txt", true, 0, 0) + featureContent, err := envFromBranch.FileRead(ctx, "feature.txt") require.NoError(t, err) assert.Contains(t, featureContent, "feature content") - _, err = envFromBranch.FileRead(ctx, "main.txt", true, 0, 0) + _, err = envFromBranch.FileRead(ctx, "main.txt") assert.Error(t, err, "main.txt should not exist in feature branch environment") // Test creating environment from specific SHA @@ -215,14 +215,14 @@ func TestRepositoryCreateFromGitRef(t *testing.T) { assert.NotNil(t, envFromSHA) // Should have only initial.txt - initialContent, err := envFromSHA.FileRead(ctx, "initial.txt", true, 0, 0) + initialContent, err := envFromSHA.FileRead(ctx, "initial.txt") require.NoError(t, err) assert.Contains(t, initialContent, "initial content") - _, err = envFromSHA.FileRead(ctx, "main.txt", true, 0, 0) + _, err = envFromSHA.FileRead(ctx, "main.txt") assert.Error(t, err, "main.txt should not exist in SHA environment") - _, err = envFromSHA.FileRead(ctx, "feature.txt", true, 0, 0) + _, err = envFromSHA.FileRead(ctx, "feature.txt") assert.Error(t, err, "feature.txt should not exist in SHA environment") // Test invalid git ref @@ -265,7 +265,7 @@ func TestRepositoryWithSubmodule(t *testing.T) { // check that the contents of the repo are being cloned into the env checkSubmoduleReadme := func(submodulePath string) { - readmeContent, readErr := env.FileRead(ctx, submodulePath+"/README.md", true, 0, 0) + readmeContent, readErr := env.FileRead(ctx, submodulePath+"/README.md") require.NoError(t, readErr, "Should be able to read %s/README.md from inside container", submodulePath) assert.Contains(t, readmeContent, "Test fixtures used by dagger integration tests.") } @@ -280,7 +280,7 @@ func TestRepositoryWithSubmodule(t *testing.T) { require.NoError(t, err, "env_run_cmd should be able to write files in submodules") // Verify the file was created inside the container - fileContent, err := env.FileRead(ctx, "submodule/test-from-cmd.txt", true, 0, 0) + fileContent, err := env.FileRead(ctx, "submodule/test-from-cmd.txt") require.NoError(t, err, "Should be able to read the file created by env_run_cmd") assert.Contains(t, fileContent, "content from env_run_cmd") @@ -330,7 +330,7 @@ func TestRepositoryWithRecursiveSubmodule(t *testing.T) { // check that the contents of the repo are being cloned into the env checkSubmoduleReadme := func(submodulePath string) { - readmeContent, readErr := env.FileRead(ctx, submodulePath+"/README.md", true, 0, 0) + readmeContent, readErr := env.FileRead(ctx, submodulePath+"/README.md") require.NoError(t, readErr, "Should be able to read %s/README.md from inside container", submodulePath) assert.Contains(t, readmeContent, "A test repository that uses submodules") } @@ -341,7 +341,7 @@ func TestRepositoryWithRecursiveSubmodule(t *testing.T) { // Check nested submodules (recursive submodules) checkNestedSubmoduleReadme := func(submodulePath string) { - nestedReadmeContent, readErr := env.FileRead(ctx, submodulePath+"/rebase/base/README.md", true, 0, 0) + nestedReadmeContent, readErr := env.FileRead(ctx, submodulePath+"/rebase/base/README.md") require.NoError(t, readErr, "Should be able to read %s/rebase/base/README.md from inside container", submodulePath) assert.Contains(t, nestedReadmeContent, "A simple test repository") } diff --git a/mcpserver/tools.go b/mcpserver/tools.go index 458e5b8d..ba73195d 100644 --- a/mcpserver/tools.go +++ b/mcpserver/tools.go @@ -693,10 +693,15 @@ func createEnvironmentFileReadTool(singleTenant bool) *Tool { } shouldReadEntireFile := request.GetBool("should_read_entire_file", false) - startLineOneIndexedInclusive := request.GetInt("start_line_one_indexed_inclusive", 0) - endLineOneIndexedInclusive := request.GetInt("end_line_one_indexed_inclusive", 0) - fileContents, err := env.FileRead(ctx, targetFile, shouldReadEntireFile, startLineOneIndexedInclusive, endLineOneIndexedInclusive) + var fileContents string + if shouldReadEntireFile { + fileContents, err = env.FileRead(ctx, targetFile) + } else { + startLine := request.GetInt("start_line_one_indexed_inclusive", 0) + endLine := request.GetInt("end_line_one_indexed_inclusive", 0) + fileContents, err = env.FileReadRange(ctx, targetFile, startLine, endLine) + } if err != nil { return nil, fmt.Errorf("failed to read file: %w", err) }