test: dedup test framework (dagger#157)

* refactor: introduce globalConfigPath for cleaner test isolation

The previous OpenWithBasePath approach exposed test implementation details
in the public API. This change uses a function variable pattern to allow
tests to override the config path without polluting the API.

Production code remains unaware of test-specific concerns while tests get
the isolation they need.

Trade-off: SetTestConfigPath() modifies global state, so tests using it
cannot run in parallel.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* test: remove t.Parallel() calls to support SetTestConfigPath

SetTestConfigPath modifies global state, making it incompatible with
parallel test execution. Remove all t.Parallel() calls from integration
tests to ensure tests run sequentially and avoid race conditions.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract WriteEnvironmentFile from MCP handler

Extract the core logic from environment_file_write handler into a public
function that can be used by both the MCP handler and test helpers.

This eliminates duplication between the handler and test helpers while
maintaining the same behavior. The handler now acts as a thin wrapper
that handles MCP-specific concerns (parameter extraction, error formatting)
while the business logic lives in the extracted function.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract DeleteEnvironmentFile from MCP handler

Extract the core logic from environment_file_delete handler into a public
function. This follows the same pattern as WriteEnvironmentFile, allowing
both the MCP handler and test helpers to use the same implementation.

The handler preserves the original error message for compatibility.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract ReadEnvironmentFile from MCP handler

Extract the core logic from environment_file_read handler. This is a
read-only operation that doesn't update the repository, making it simpler
than the write operations.

The handler now uses err.Error() for the error message to preserve the
original behavior where different errors could occur.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract RunEnvironmentCommand from MCP handler

Extract the core logic from environment_run_cmd handler. This handles both
foreground and background command execution, including the repository update
that must happen even if the command fails.

The function returns 'any' to accommodate both string output (foreground)
and endpoint mappings (background). The handler formats the appropriate
response based on the command type.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract CreateEnvironment from MCP handler

Extract the core logic from environment_create handler. This simplifies
the handler to focus on MCP-specific concerns while the business logic
lives in the extracted function.

The handler preserves the original 'failed to create environment' error
message for compatibility.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract UpdateEnvironment from MCP handler

Extract the core logic from environment_update handler. This handles
updating environment configuration including title, instructions, base
image, setup commands, environment variables, and secrets.

The handler preserves the original 'unable to update the environment'
error message and the detailed success response format.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract ListEnvironmentFiles from MCP handler

Extract the core logic from environment_file_list handler. This is a
read-only operation that lists directory contents without modifying
the environment.

The handler preserves the original 'failed to list directory' error
message for compatibility.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract GetEnvironmentFromSource from MCP handler

Extract the core logic from environment_open handler. This is a simple
operation that opens an existing environment from a source repository.

The handler preserves the original 'unable to open the environment' error
message for compatibility.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* refactor: extract AddEnvironmentService from MCP handler

Extract the core logic from environment_add_service handler. This handles
adding services (like databases, caches) to an environment including their
configuration, ports, environment variables, and secrets.

The handler preserves the original 'failed to update env' error message
that was returned when either adding the service or updating the repository
failed.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* fix: CreateEnvironment returns repo to enable dirty check

The refactored CreateEnvironment function wasn't returning the repository,
preventing the handler from checking for uncommitted changes.

Return both repo and environment from CreateEnvironment to maintain the
same behavior as the original handler while keeping the extraction clean.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* chore: apply go fmt

Run go fmt to fix formatting issues:
- Remove trailing whitespace
- Remove extra blank lines
- Ensure consistent formatting

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* fix: remove t.Parallel() from tests using SetTestConfigPath

SetTestConfigPath modifies a global variable, making it unsafe for
parallel test execution. Tests were racing and causing CI failures.

Added warning comment to WithRepository to prevent future issues.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* feat: add lint check for t.Parallel() with WithRepository

Add a simple grep-based check to prevent using t.Parallel() in test
files that also use WithRepository. This prevents race conditions
caused by WithRepository's use of SetTestConfigPath global variable.

The check is file-level (not function-level) for simplicity, which
may cause false positives but ensures all real issues are caught.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

* chore: keep rely on the proper daggerClienKey

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

---------

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>

Author: grouville

.dagger/main.go

@@ -87,10 +87,58 @@ func (m *ContainerUse) Test(ctx context.Context,
 		Stdout(ctx)
 }
 
-// Test runs the linter
+// Lint runs the linter and custom checks
 func (m *ContainerUse) Lint(ctx context.Context) error {
-	return dag.
+	// Run golangci-lint
+	err := dag.
 		Golangci().
 		Lint(m.Source, dagger.GolangciLintOpts{}).
 		Assert(ctx)
+	if err != nil {
+		return err
+	}
+
+	// Check for t.Parallel() in tests using WithRepository
+	// This is a simple grep-based check that prevents race conditions
+	// WithRepository uses SetTestConfigPath which modifies global state
+	checkScript := `#!/bin/bash
+set -e
+
+echo "Checking for t.Parallel() in tests using WithRepository..."
+
+# Find test files that use WithRepository
+files_with_repo=$(grep -l "WithRepository" environment/integration/*_test.go 2>/dev/null || true)
+
+if [ -z "$files_with_repo" ]; then
+  echo "No test files found using WithRepository"
+  exit 0
+fi
+
+# Check each file for t.Parallel()
+found_issues=false
+for file in $files_with_repo; do
+  if grep -q "t\.Parallel()" "$file"; then
+    echo "ERROR: $file uses both WithRepository and t.Parallel()"
+    echo "  WithRepository modifies global state and is not safe for parallel execution"
+    found_issues=true
+  fi
+done
+
+if [ "$found_issues" = true ]; then
+  echo ""
+  echo "Tests using WithRepository must not call t.Parallel() as it causes race conditions."
+  exit 1
+fi
+
+echo "✓ No parallel test issues found"
+`
+
+	_, err = dag.Go(m.Source).
+		Base().
+		WithMountedDirectory("/src", m.Source).
+		WithWorkdir("/src").
+		WithExec([]string{"bash", "-c", checkScript}).
+		Sync(ctx)
+
+	return err
 }

environment/integration/helpers.go

@@ -10,6 +10,7 @@ import (
 
 	"dagger.io/dagger"
 	"github.com/dagger/container-use/environment"
+	"github.com/dagger/container-use/mcpserver"
 	"github.com/dagger/container-use/repository"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -35,6 +36,10 @@ func init() {
 }
 
 // WithRepository runs a test function with an isolated repository and UserActions
+//
+// WARNING: Tests using WithRepository MUST NOT call t.Parallel() because
+// SetTestConfigPath modifies a global variable. Parallel tests will race
+// and cause unpredictable failures.
 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)
@@ -48,6 +53,10 @@ func WithRepository(t *testing.T, name string, setup RepositorySetup, fn func(t
 	configDir, err := os.MkdirTemp("", "cu-test-config-"+name+"-*")
 	require.NoError(t, err, "Failed to create config dir")
 
+	// Override the global config path for this test
+	cleanup := repository.SetTestConfigPath(configDir)
+	t.Cleanup(cleanup)
+
 	// Initialize git repo
 	cmds := [][]string{
 		{"init"},
@@ -66,19 +75,19 @@ func WithRepository(t *testing.T, name string, setup RepositorySetup, fn func(t
 		setup(t, repoDir)
 	}
 
-	// Open repository with isolated base path
-	repo, err := repository.OpenWithBasePath(ctx, repoDir, configDir)
+	// Open repository - it will use the isolated base path from context
+	repo, err := repository.Open(ctx, repoDir)
 	require.NoError(t, err, "Failed to open repository")
 
 	// Create UserActions with extended capabilities
-	user := NewUserActions(t, repo, testDaggerClient).WithDirectAccess(repoDir, configDir)
+	user := NewUserActions(ctx, t, repo, testDaggerClient).WithDirectAccess(repoDir, configDir)
 
 	// Cleanup
 	t.Cleanup(func() {
 		// Clean up any environments created during the test
-		envs, _ := repo.List(context.Background())
+		envs, _ := repo.List(ctx)
 		for _, env := range envs {
-			repo.Delete(context.Background(), env.ID)
+			repo.Delete(ctx, env.ID)
 		}
 
 		// Remove directories
@@ -184,10 +193,10 @@ type UserActions struct {
 	configDir string // Container-use config directory
 }
 
-func NewUserActions(t *testing.T, repo *repository.Repository, dag *dagger.Client) *UserActions {
+func NewUserActions(ctx context.Context, t *testing.T, repo *repository.Repository, dag *dagger.Client) *UserActions {
 	return &UserActions{
 		t:    t,
-		ctx:  context.Background(),
+		ctx:  ctx,
 		repo: repo,
 		dag:  dag,
 	}
@@ -202,71 +211,45 @@ func (u *UserActions) WithDirectAccess(repoDir, configDir string) *UserActions {
 
 // 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)
+	err := mcpserver.WriteEnvironmentFile(u.ctx, u.dag, u.repoDir, envID, targetFile, contents, explanation)
 	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)
+	result, err := mcpserver.RunEnvironmentCommand(u.ctx, u.dag, u.repoDir, envID, command, "/bin/sh", explanation, false, false, nil)
 	require.NoError(u.t, err, "Run command should succeed")
+	require.NotNil(u.t, result, "Run command should return a result")
 
-	err = u.repo.Update(u.ctx, env, explanation)
-	require.NoError(u.t, err, "repo.Update after Run should succeed")
-
+	// For non-background commands, result is a string
+	output, ok := result.(string)
+	require.True(u.t, ok, "Run command should return string output")
 	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)
+	_, env, err := mcpserver.CreateEnvironment(u.ctx, u.dag, u.repoDir, title, explanation)
 	require.NoError(u.t, err, "Create environment should succeed")
+	require.NotNil(u.t, env, "Create environment should return an environment")
 	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, explanation, 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")
+	_, err := mcpserver.UpdateEnvironment(u.ctx, u.dag, u.repoDir, envID, title, config.Instructions, config.BaseImage, explanation, config.SetupCommands, config.Env, config.Secrets)
+	require.NoError(u.t, err, "UpdateEnvironment 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)
+	err := mcpserver.DeleteEnvironmentFile(u.ctx, u.dag, u.repoDir, envID, targetFile, explanation)
 	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)
+	content, err := mcpserver.ReadEnvironmentFile(u.ctx, u.dag, u.repoDir, envID, targetFile, true, 0, 0)
 	require.NoError(u.t, err, "FileRead should succeed")
 	return content
 }
@@ -280,6 +263,13 @@ func (u *UserActions) FileReadExpectError(envID, targetFile string) {
 	assert.Error(u.t, err, "FileRead should fail for %s", targetFile)
 }
 
+// FileList mirrors environment_file_list MCP tool behavior
+func (u *UserActions) FileList(envID, path string) string {
+	content, err := mcpserver.ListEnvironmentFiles(u.ctx, u.dag, u.repoDir, envID, path)
+	require.NoError(u.t, err, "FileList should succeed")
+	return content
+}
+
 // 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 {
@@ -288,6 +278,22 @@ func (u *UserActions) GetEnvironment(envID string) *environment.Environment {
 	return env
 }
 
+// OpenEnvironment mirrors environment_open MCP tool behavior
+func (u *UserActions) OpenEnvironment(envID string) *environment.Environment {
+	env, err := mcpserver.GetEnvironmentFromSource(u.ctx, u.dag, u.repoDir, envID)
+	require.NoError(u.t, err, "OpenEnvironment should succeed")
+	require.NotNil(u.t, env, "OpenEnvironment should return an environment")
+	return env
+}
+
+// AddService mirrors environment_add_service MCP tool behavior
+func (u *UserActions) AddService(envID, name, image, command, explanation string, ports []int, envs []string, secrets []string) *environment.Service {
+	service, err := mcpserver.AddEnvironmentService(u.ctx, u.dag, u.repoDir, envID, name, image, command, explanation, ports, envs, secrets)
+	require.NoError(u.t, err, "AddService should succeed")
+	require.NotNil(u.t, service, "AddService should return a service")
+	return service
+}
+
 // --- Direct manipulation methods for edge case testing ---
 
 // WriteSourceFile writes directly to the source repository

environment/integration/integration_test.go

@@ -18,7 +18,6 @@ import (
 
 // TestGitAuditTrail verifies that all operations are tracked in git
 func TestGitAuditTrail(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}
@@ -61,7 +60,6 @@ func TestGitAuditTrail(t *testing.T) {
 
 // TestEnvironmentIsolation verifies that changes in one environment don't affect others
 func TestEnvironmentIsolation(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}
@@ -106,7 +104,6 @@ func TestEnvironmentIsolation(t *testing.T) {
 
 // TestSystemHandlesProblematicFiles verifies edge cases don't break the system
 func TestSystemHandlesProblematicFiles(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}
@@ -183,7 +180,6 @@ func TestSystemHandlesProblematicFiles(t *testing.T) {
 
 // Large project performance ensures the system scales to real-world codebases
 func TestLargeProjectPerformance(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping performance test")
 	}
@@ -216,7 +212,6 @@ func TestLargeProjectPerformance(t *testing.T) {
 
 // TestWorktreeUpdatesAreVisibleAfterRebuild verifies file changes persist through rebuilds
 func TestWorktreeUpdatesAreVisibleAfterRebuild(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}
@@ -256,7 +251,6 @@ func TestWorktreeUpdatesAreVisibleAfterRebuild(t *testing.T) {
 
 // TestWeirdUserScenarios verifies edge case handling
 func TestWeirdUserScenarios(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}
@@ -400,7 +394,6 @@ func TestWeirdUserScenarios(t *testing.T) {
 
 // TestEnvironmentConfigurationPersists verifies configuration persistence
 func TestEnvironmentConfigurationPersists(t *testing.T) {
-	t.Parallel()
 	if testing.Short() {
 		t.Skip("Skipping integration test")
 	}

environment/integration/merge_test.go

@@ -15,7 +15,6 @@ import (
 
 // TestRepositoryMerge tests merging an environment into the main branch
 func TestRepositoryMerge(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-merge", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -63,7 +62,6 @@ func TestRepositoryMerge(t *testing.T) {
 
 // TestRepositoryMergeNonExistent tests merging a non-existent environment
 func TestRepositoryMergeNonExistent(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-merge-nonexistent", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -77,7 +75,6 @@ func TestRepositoryMergeNonExistent(t *testing.T) {
 
 // TestRepositoryMergeWithConflicts tests merge behavior when there are conflicts
 func TestRepositoryMergeWithConflicts(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-merge-conflicts", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -108,7 +105,6 @@ func TestRepositoryMergeWithConflicts(t *testing.T) {
 // TestRepositoryMergeCompleted tests merging the same environment multiple times
 // This should result in fast-forward merges since the main branch doesn't diverge
 func TestRepositoryMergeCompleted(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-merge-completed", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 

environment/integration/repository_test.go

@@ -14,7 +14,6 @@ import (
 
 // TestRepositoryCreate tests creating a new environment
 func TestRepositoryCreate(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-create", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		// Create an environment
 		env := user.CreateEnvironment("Test Create", "Testing repository create")
@@ -34,7 +33,6 @@ func TestRepositoryCreate(t *testing.T) {
 
 // TestRepositoryGet tests retrieving an existing environment
 func TestRepositoryGet(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-get", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -56,7 +54,6 @@ func TestRepositoryGet(t *testing.T) {
 
 // TestRepositoryList tests listing all environments
 func TestRepositoryList(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-list", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -81,7 +78,6 @@ func TestRepositoryList(t *testing.T) {
 
 // TestRepositoryDelete tests deleting an environment
 func TestRepositoryDelete(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-delete", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -106,7 +102,6 @@ func TestRepositoryDelete(t *testing.T) {
 
 // TestRepositoryCheckout tests checking out an environment branch
 func TestRepositoryCheckout(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-checkout", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -131,7 +126,6 @@ func TestRepositoryCheckout(t *testing.T) {
 
 // TestRepositoryLog tests retrieving commit history for an environment
 func TestRepositoryLog(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-log", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()
 
@@ -170,7 +164,6 @@ func TestRepositoryLog(t *testing.T) {
 
 // TestRepositoryDiff tests retrieving changes between commits
 func TestRepositoryDiff(t *testing.T) {
-	t.Parallel()
 	WithRepository(t, "repository-diff", SetupEmptyRepo, func(t *testing.T, repo *repository.Repository, user *UserActions) {
 		ctx := context.Background()