From 892ce9bf09eee6cfb16dcecc5d5b0f7e8e13d5d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 13 Jul 2026 13:56:40 +0100 Subject: [PATCH] fix(worktree): preserve colon paths in copy files (#1674) --- apps/backend/AGENTS.md | 2 +- .../lifecycle/remote_copyfiles_test.go | 2 +- .../repository/sqlite/repository_entity.go | 87 +++++++------- .../internal/task/service/service_test.go | 15 ++- .../internal/worktree/copyfiles/copyfiles.go | 67 ++++++----- .../copyfiles/copyfiles_symlink_test.go | 102 ++++++++++++++++- .../worktree/manager_copyfiles_test.go | 32 ++++++ .../settings/repository-copy-files-help.tsx | 106 +++++++++--------- .../settings/copy-files-symlink-helpers.ts | 61 ++++++++++ .../tests/settings/copy-files-symlink.spec.ts | 46 ++++++++ .../mobile-copy-files-symlink.spec.ts | 46 ++++++++ apps/web/lib/types/http.ts | 3 +- docs/decisions/0010-worktree-copy-files.md | 64 +++++++---- 13 files changed, 465 insertions(+), 168 deletions(-) create mode 100644 apps/web/e2e/tests/settings/copy-files-symlink-helpers.ts create mode 100644 apps/web/e2e/tests/settings/copy-files-symlink.spec.ts create mode 100644 apps/web/e2e/tests/settings/mobile-copy-files-symlink.spec.ts diff --git a/apps/backend/AGENTS.md b/apps/backend/AGENTS.md index 00e0f0f0e9..140c3e8638 100644 --- a/apps/backend/AGENTS.md +++ b/apps/backend/AGENTS.md @@ -180,7 +180,7 @@ Client (WS) ← Orchestrator ← Lifecycle Manager ←──── stream update **Worktrees:** `internal/worktree/Manager` provides workspace isolation. Each session can have its own worktree (branch) to prevent conflicts between concurrent agents. -**Worktree file materialization:** `copy_files` (`Repository.CopyFiles`) is a single comma-separated spec of gitignored files/globs seeded into each new worktree. Each entry may carry an optional `:keyword` suffix; the only keyword is `:symlink` (e.g. `.env.local:symlink`), which links the entry back to the main repo instead of copying it so shared files stay centralized. No suffix = copy (the default, fully backward compatible). Parsing, validation, and materialization live in `internal/worktree/copyfiles/` (`ParseSpecs` → `[]PatternSpec{Pattern, Symlink}`, `ValidateSpec` gates unknown keywords at save time, `Copy` honors symlink mode host-side). `Manager.copyConfiguredFiles` (in `manager_lifecycle.go`) runs it during worktree creation. Non-fatal: per-entry failures become warnings. Symlinks use relative targets (unix/macOS) and only apply on the host path — the remote-executor path (`Plan`/`WriteEntries`) strips the keyword and falls back to a byte copy, since a link back to the host repo can't apply remotely. +**Worktree file materialization:** `copy_files` (`Repository.CopyFiles`) is a comma-separated spec of repository-relative gitignored paths/doublestar globs seeded into each new worktree. Copy is the default; the exact terminal `:symlink` suffix (for example `.env.local:symlink`) creates a relative host-worktree link so source changes propagate live. Other colons stay literal (`config:dev`, `.env:`); use `::symlink` to copy a literal path ending in `:symlink`. Malformed reserved syntax is rejected at save time. Parsing and materialization live in `internal/worktree/copyfiles/` (`ParseSpecs`, `ValidateSpec`, `Copy`); duplicates and overlapping matches are first-entry-wins. `Manager.copyConfiguredFiles` runs before setup during worktree creation. Source and destination containment checks reject traversal and symlinked destination parents. Failures are non-fatal warnings. Windows link creation is best-effort. Remote executors cannot link to the host, so `Parse`/`Plan` preserve literal colon paths but turn symlink-mode entries into copied bytes delivered through `WriteEntries`. **Executor default scripts:** Default prepare scripts are in `internal/agent/runtime/lifecycle/default_scripts.go`; `internal/scriptengine/` handles placeholder resolution. diff --git a/apps/backend/internal/agent/runtime/lifecycle/remote_copyfiles_test.go b/apps/backend/internal/agent/runtime/lifecycle/remote_copyfiles_test.go index 5d44df5225..fdc4913b3b 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/remote_copyfiles_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/remote_copyfiles_test.go @@ -109,7 +109,7 @@ func TestRunRemoteCopyfiles_HappyPath_EmitsStep(t *testing.T) { runRemoteCopyfiles(context.Background(), logger.Default(), remoteCopyfilesRequest{ SourceRepoPath: src, - CopyFilesSpec: ".env", + CopyFilesSpec: ".env:symlink", Client: cli, OnProgress: cb, }) diff --git a/apps/backend/internal/task/repository/sqlite/repository_entity.go b/apps/backend/internal/task/repository/sqlite/repository_entity.go index ca9fd78267..2dd32ee1d2 100644 --- a/apps/backend/internal/task/repository/sqlite/repository_entity.go +++ b/apps/backend/internal/task/repository/sqlite/repository_entity.go @@ -26,51 +26,29 @@ func (r *Repository) CreateRepository(ctx context.Context, repository *models.Re _, err := r.db.ExecContext(ctx, r.db.Rebind(` INSERT INTO repositories ( id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_owner, - provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, - copy_files, created_at, updated_at, deleted_at + provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, copy_files, created_at, updated_at, deleted_at ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `), repository.ID, repository.WorkspaceID, repository.Name, repository.SourceType, repository.LocalPath, repository.Provider, repository.ProviderRepoID, repository.ProviderOwner, repository.ProviderName, repository.DefaultBranch, repository.WorktreeBranchPrefix, - repository.WorktreeBranchTemplate, dialect.BoolToInt(repository.PullBeforeWorktree), repository.SetupScript, repository.CleanupScript, repository.DevScript, - repository.CopyFiles, - repository.CreatedAt, repository.UpdatedAt, repository.DeletedAt) + repository.WorktreeBranchTemplate, dialect.BoolToInt(repository.PullBeforeWorktree), repository.SetupScript, repository.CleanupScript, repository.DevScript, repository.CopyFiles, repository.CreatedAt, repository.UpdatedAt, repository.DeletedAt) return err } -// repositoryColumns is the shared SELECT column list for repository rows, in the -// order expected by scanRepository. -const repositoryColumns = `id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_owner, - provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, - copy_files, created_at, updated_at, deleted_at` - -// rowScanner abstracts *sql.Row and *sql.Rows so a single scan routine serves -// both single-row and iterating queries. -type rowScanner interface { - Scan(dest ...any) error -} - -// scanRepository decodes one repository row into a models.Repository. -func scanRepository(s rowScanner) (*models.Repository, error) { - repository := &models.Repository{} - err := s.Scan( - &repository.ID, &repository.WorkspaceID, &repository.Name, &repository.SourceType, &repository.LocalPath, - &repository.Provider, &repository.ProviderRepoID, &repository.ProviderOwner, &repository.ProviderName, - &repository.DefaultBranch, &repository.WorktreeBranchPrefix, &repository.WorktreeBranchTemplate, &repository.PullBeforeWorktree, &repository.SetupScript, - &repository.CleanupScript, &repository.DevScript, &repository.CopyFiles, - &repository.CreatedAt, &repository.UpdatedAt, &repository.DeletedAt, - ) - if err != nil { - return nil, err - } - return repository, nil -} - // GetRepository retrieves a repository by ID func (r *Repository) GetRepository(ctx context.Context, id string) (*models.Repository, error) { - row := r.ro.QueryRowContext(ctx, r.ro.Rebind( - `SELECT `+repositoryColumns+` FROM repositories WHERE id = ? AND deleted_at IS NULL`), id) - repository, err := scanRepository(row) + repository := &models.Repository{} + + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_owner, + provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, copy_files, created_at, updated_at, deleted_at + FROM repositories WHERE id = ? AND deleted_at IS NULL + `), id).Scan( + &repository.ID, &repository.WorkspaceID, &repository.Name, &repository.SourceType, &repository.LocalPath, + &repository.Provider, &repository.ProviderRepoID, &repository.ProviderOwner, &repository.ProviderName, + &repository.DefaultBranch, &repository.WorktreeBranchPrefix, &repository.WorktreeBranchTemplate, &repository.PullBeforeWorktree, &repository.SetupScript, &repository.CleanupScript, &repository.DevScript, &repository.CopyFiles, &repository.CreatedAt, &repository.UpdatedAt, &repository.DeletedAt, + ) + if err == sql.ErrNoRows { return nil, fmt.Errorf("%w: %s", repoerrors.ErrRepositoryNotFound, id) } @@ -84,14 +62,11 @@ func (r *Repository) UpdateRepository(ctx context.Context, repository *models.Re result, err := r.db.ExecContext(ctx, r.db.Rebind(` UPDATE repositories SET name = ?, source_type = ?, local_path = ?, provider = ?, provider_repo_id = ?, provider_owner = ?, - provider_name = ?, default_branch = ?, worktree_branch_prefix = ?, worktree_branch_template = ?, pull_before_worktree = ?, setup_script = ?, cleanup_script = ?, dev_script = ?, - copy_files = ?, updated_at = ? + provider_name = ?, default_branch = ?, worktree_branch_prefix = ?, worktree_branch_template = ?, pull_before_worktree = ?, setup_script = ?, cleanup_script = ?, dev_script = ?, copy_files = ?, updated_at = ? WHERE id = ? AND deleted_at IS NULL `), repository.Name, repository.SourceType, repository.LocalPath, repository.Provider, repository.ProviderRepoID, repository.ProviderOwner, repository.ProviderName, repository.DefaultBranch, repository.WorktreeBranchPrefix, repository.WorktreeBranchTemplate, dialect.BoolToInt(repository.PullBeforeWorktree), - repository.SetupScript, repository.CleanupScript, repository.DevScript, - repository.CopyFiles, - repository.UpdatedAt, repository.ID) + repository.SetupScript, repository.CleanupScript, repository.DevScript, repository.CopyFiles, repository.UpdatedAt, repository.ID) if err != nil { return err } @@ -152,8 +127,11 @@ func (r *Repository) DeleteRepositoryIfNoActiveTaskSessions(ctx context.Context, // ListRepositories returns all repositories for a workspace func (r *Repository) ListRepositories(ctx context.Context, workspaceID string) ([]*models.Repository, error) { - rows, err := r.ro.QueryContext(ctx, r.ro.Rebind( - `SELECT `+repositoryColumns+` FROM repositories WHERE workspace_id = ? AND deleted_at IS NULL ORDER BY created_at DESC`), workspaceID) + rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(` + SELECT id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_owner, + provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, copy_files, created_at, updated_at, deleted_at + FROM repositories WHERE workspace_id = ? AND deleted_at IS NULL ORDER BY created_at DESC + `), workspaceID) if err != nil { return nil, err } @@ -161,7 +139,12 @@ func (r *Repository) ListRepositories(ctx context.Context, workspaceID string) ( var result []*models.Repository for rows.Next() { - repository, err := scanRepository(rows) + repository := &models.Repository{} + err := rows.Scan( + &repository.ID, &repository.WorkspaceID, &repository.Name, &repository.SourceType, &repository.LocalPath, + &repository.Provider, &repository.ProviderRepoID, &repository.ProviderOwner, &repository.ProviderName, + &repository.DefaultBranch, &repository.WorktreeBranchPrefix, &repository.WorktreeBranchTemplate, &repository.PullBeforeWorktree, &repository.SetupScript, &repository.CleanupScript, &repository.DevScript, &repository.CopyFiles, &repository.CreatedAt, &repository.UpdatedAt, &repository.DeletedAt, + ) if err != nil { return nil, err } @@ -173,11 +156,17 @@ func (r *Repository) ListRepositories(ctx context.Context, workspaceID string) ( // GetRepositoryByProviderInfo finds a repository by workspace, provider, owner, and name. // Returns nil, nil if not found. func (r *Repository) GetRepositoryByProviderInfo(ctx context.Context, workspaceID, provider, owner, name string) (*models.Repository, error) { - row := r.ro.QueryRowContext(ctx, r.ro.Rebind( - `SELECT `+repositoryColumns+` FROM repositories - WHERE workspace_id = ? AND provider = ? AND provider_owner = ? AND provider_name = ? AND deleted_at IS NULL`), - workspaceID, provider, owner, name) - repository, err := scanRepository(row) + repository := &models.Repository{} + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_owner, + provider_name, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, copy_files, created_at, updated_at, deleted_at + FROM repositories + WHERE workspace_id = ? AND provider = ? AND provider_owner = ? AND provider_name = ? AND deleted_at IS NULL + `), workspaceID, provider, owner, name).Scan( + &repository.ID, &repository.WorkspaceID, &repository.Name, &repository.SourceType, &repository.LocalPath, + &repository.Provider, &repository.ProviderRepoID, &repository.ProviderOwner, &repository.ProviderName, + &repository.DefaultBranch, &repository.WorktreeBranchPrefix, &repository.WorktreeBranchTemplate, &repository.PullBeforeWorktree, &repository.SetupScript, &repository.CleanupScript, &repository.DevScript, &repository.CopyFiles, &repository.CreatedAt, &repository.UpdatedAt, &repository.DeletedAt, + ) if err == sql.ErrNoRows { return nil, nil } diff --git a/apps/backend/internal/task/service/service_test.go b/apps/backend/internal/task/service/service_test.go index d4ae300a15..9ff289efe0 100644 --- a/apps/backend/internal/task/service/service_test.go +++ b/apps/backend/internal/task/service/service_test.go @@ -1098,22 +1098,25 @@ func TestService_CreateRepository_CopyFilesSymlinkKeyword(t *testing.T) { } } -func TestService_CreateRepository_InvalidCopyFilesKeyword(t *testing.T) { +func TestService_CreateRepository_ColonBearingCopyFilesPath(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}) - _, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{ + created, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{ WorkspaceID: "ws-1", Name: "Test Repo", CopyFiles: ".env.local:hardlink", }) - if !errors.Is(err, ErrInvalidRepositorySettings) { - t.Fatalf("expected ErrInvalidRepositorySettings, got %v", err) + if err != nil { + t.Fatalf("CreateRepository failed: %v", err) + } + if created.CopyFiles != ".env.local:hardlink" { + t.Fatalf("copy_files not persisted verbatim: %q", created.CopyFiles) } } -func TestService_UpdateRepository_InvalidCopyFilesKeyword(t *testing.T) { +func TestService_UpdateRepository_MalformedCopyFilesMode(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}) @@ -1122,7 +1125,7 @@ func TestService_UpdateRepository_InvalidCopyFilesKeyword(t *testing.T) { t.Fatalf("CreateRepository failed: %v", err) } - bad := ".env:move" + bad := ":symlink" _, err = svc.UpdateRepository(ctx, created.ID, &UpdateRepositoryRequest{CopyFiles: &bad}) if !errors.Is(err, ErrInvalidRepositorySettings) { t.Fatalf("expected ErrInvalidRepositorySettings, got %v", err) diff --git a/apps/backend/internal/worktree/copyfiles/copyfiles.go b/apps/backend/internal/worktree/copyfiles/copyfiles.go index 12086943dc..fdae8a345b 100644 --- a/apps/backend/internal/worktree/copyfiles/copyfiles.go +++ b/apps/backend/internal/worktree/copyfiles/copyfiles.go @@ -51,11 +51,14 @@ type Entry struct { Content []byte `json:"content"` } -// KeywordSymlink is the only per-entry keyword recognized after the `:` -// separator. `.env.local:symlink` links the entry back to the source repo -// instead of copying it. +// KeywordSymlink is the only reserved per-entry mode suffix. const KeywordSymlink = "symlink" +const ( + symlinkSuffix = ":" + KeywordSymlink + escapedSymlinkSuffix = ":" + symlinkSuffix +) + // PatternSpec is a single copy-files entry: a pattern plus its per-entry mode. // Symlink is true when the entry carried the `:symlink` keyword. type PatternSpec struct { @@ -63,20 +66,27 @@ type PatternSpec struct { Symlink bool } -// splitKeyword splits a raw entry into its pattern and optional trailing -// `:keyword`. The keyword is whatever follows the last `:`; entries without a -// colon have an empty keyword. Repo-relative env/config paths don't contain -// colons in practice, so the last-colon rule is unambiguous for real specs. -func splitKeyword(entry string) (pattern, keyword string) { - idx := strings.LastIndex(entry, ":") - if idx < 0 { - return entry, "" +// parsePatternSpec recognizes only the exact terminal :symlink suffix. Other +// colons remain literal path characters for compatibility with existing specs. +// A doubled colon escapes a literal filename ending in :symlink. +func parsePatternSpec(entry string) (PatternSpec, error) { + entry = strings.TrimSpace(entry) + if strings.HasSuffix(entry, escapedSymlinkSuffix) { + pattern := strings.TrimSpace(strings.TrimSuffix(entry, escapedSymlinkSuffix)) + symlinkSuffix + return PatternSpec{Pattern: pattern}, nil } - return strings.TrimSpace(entry[:idx]), strings.TrimSpace(entry[idx+1:]) + if !strings.HasSuffix(entry, symlinkSuffix) { + return PatternSpec{Pattern: entry}, nil + } + pattern := strings.TrimSpace(strings.TrimSuffix(entry, symlinkSuffix)) + if pattern == "" { + return PatternSpec{}, fmt.Errorf("invalid copy-files mode syntax %q: missing path before %q", entry, symlinkSuffix) + } + return PatternSpec{Pattern: pattern, Symlink: true}, nil } // Parse splits a comma-separated user spec into trimmed, deduplicated, -// non-empty patterns with any `:keyword` suffix stripped. Order is preserved +// non-empty patterns with the exact `:symlink` suffix stripped. Order is preserved // (first occurrence wins on dedupe). Commas inside `{...}` are treated as part // of the pattern (brace alternation), so `config/{local,dev}.yml` is parsed as // a single pattern. This is the copy-only view used by the remote-executor @@ -94,9 +104,10 @@ func Parse(spec string) []string { } // ParseSpecs splits a comma-separated user spec into trimmed, deduplicated, -// non-empty PatternSpecs, extracting the per-entry `:symlink` mode. Dedupe is -// by pattern (first occurrence wins). An unrecognized keyword is treated as -// copy mode here — ValidateSpec is the gate that rejects it at save time. +// non-empty PatternSpecs, extracting the exact per-entry `:symlink` mode. +// Dedupe is by normalized pattern and the first occurrence wins. Invalid +// entries are skipped; ValidateSpec reports them before repository settings +// are persisted. func ParseSpecs(spec string) []PatternSpec { if spec == "" { return nil @@ -105,15 +116,15 @@ func ParseSpecs(spec string) []PatternSpec { out := make([]PatternSpec, 0, len(parts)) seen := make(map[string]struct{}, len(parts)) for _, p := range parts { - pattern, keyword := splitKeyword(strings.TrimSpace(p)) - if pattern == "" { + parsed, err := parsePatternSpec(p) + if err != nil || parsed.Pattern == "" { continue } - if _, ok := seen[pattern]; ok { + if _, ok := seen[parsed.Pattern]; ok { continue } - seen[pattern] = struct{}{} - out = append(out, PatternSpec{Pattern: pattern, Symlink: keyword == KeywordSymlink}) + seen[parsed.Pattern] = struct{}{} + out = append(out, parsed) } if len(out) == 0 { return nil @@ -121,10 +132,8 @@ func ParseSpecs(spec string) []PatternSpec { return out } -// ValidateSpec reports the first invalid entry in a copy-files spec: a `:` -// suffix whose keyword is not in the whitelist (currently only `symlink`). -// Empty entries and plain patterns are always valid. Called at save time so -// misconfigured keywords surface immediately instead of silently copying. +// ValidateSpec reports malformed use of the reserved :symlink suffix. Unknown +// colon suffixes remain literal paths for backward compatibility. func ValidateSpec(spec string) error { if spec == "" { return nil @@ -134,12 +143,8 @@ func ValidateSpec(spec string) error { if p == "" { continue } - if !strings.Contains(p, ":") { - continue - } - _, keyword := splitKeyword(p) - if keyword != "" && keyword != KeywordSymlink { - return fmt.Errorf("invalid copy-files keyword %q in %q (only %q is supported)", keyword, p, KeywordSymlink) + if _, err := parsePatternSpec(p); err != nil { + return err } } return nil diff --git a/apps/backend/internal/worktree/copyfiles/copyfiles_symlink_test.go b/apps/backend/internal/worktree/copyfiles/copyfiles_symlink_test.go index 0ea8555ecd..6ee9842c58 100644 --- a/apps/backend/internal/worktree/copyfiles/copyfiles_symlink_test.go +++ b/apps/backend/internal/worktree/copyfiles/copyfiles_symlink_test.go @@ -11,11 +11,14 @@ import ( func TestParseSpecs(t *testing.T) { t.Parallel() - got := ParseSpecs(".env, .env.local:symlink, config/{local,dev}.yml") + got := ParseSpecs(".env, .env.local:symlink, config/{local,dev}.yml, config:dev, .env:, literal::symlink") want := []PatternSpec{ {Pattern: ".env"}, {Pattern: ".env.local", Symlink: true}, {Pattern: "config/{local,dev}.yml"}, + {Pattern: "config:dev"}, + {Pattern: ".env:"}, + {Pattern: "literal:symlink"}, } if !reflect.DeepEqual(got, want) { t.Fatalf("ParseSpecs = %#v, want %#v", got, want) @@ -24,18 +27,34 @@ func TestParseSpecs(t *testing.T) { func TestValidateSpec(t *testing.T) { t.Parallel() - for _, ok := range []string{"", ".env", ".env, .env.local:symlink", "config/{local,dev}.yml", ".env:"} { + for _, ok := range []string{ + "", ".env", ".env, .env.local:symlink", "config/{local,dev}.yml", + ".env:", "config:dev", ".env:hardlink", "literal::symlink", + } { if err := ValidateSpec(ok); err != nil { t.Errorf("ValidateSpec(%q) unexpected error: %v", ok, err) } } - for _, bad := range []string{".env:hardlink", ".env, foo:move"} { + for _, bad := range []string{":symlink", ".env, :symlink"} { if err := ValidateSpec(bad); err == nil { t.Errorf("ValidateSpec(%q) = nil, want error", bad) } } } +func TestParseSpecs_DuplicateAndOverlappingPatternsUseFirstEntry(t *testing.T) { + t.Parallel() + + got := ParseSpecs(".env:symlink, .env, .env:symlink, .*") + want := []PatternSpec{ + {Pattern: ".env", Symlink: true}, + {Pattern: ".*"}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("ParseSpecs = %#v, want %#v", got, want) + } +} + func TestCopy_SymlinkMode_File(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("symlink creation often requires privilege on Windows") @@ -67,26 +86,97 @@ func TestCopy_SymlinkMode_File(t *testing.T) { if string(got) != "SECRET=1" { t.Fatalf("symlink content = %q", got) } + writeFile(t, filepath.Join(src, ".env.local"), "SECRET=updated", 0o644) + if got := readFile(t, filepath.Join(dst, ".env.local")); got != "SECRET=updated" { + t.Fatalf("updated symlink content = %q, want live source content", got) + } if len(copied) != 1 || copied[0] != ".env.local" { t.Fatalf("copied = %v, want [.env.local]", copied) } } +func TestCopy_DuplicateAndOverlappingPatternsUseFirstEntry(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation often requires privilege on Windows") + } + + for _, tc := range []struct { + name string + spec string + wantSymlink bool + }{ + {name: "symlink before copy overlap", spec: ".env:symlink, .*", wantSymlink: true}, + {name: "copy before symlink duplicate", spec: ".*, .env:symlink", wantSymlink: false}, + } { + t.Run(tc.name, func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + writeFile(t, filepath.Join(src, ".env"), "SECRET=1", 0o644) + + _, warnings, err := Copy(context.Background(), src, dst, ParseSpecs(tc.spec), nil) + if err != nil { + t.Fatalf("Copy err: %v", err) + } + if len(warnings) != 0 { + t.Fatalf("unexpected warnings: %v", warnings) + } + info, err := os.Lstat(filepath.Join(dst, ".env")) + if err != nil { + t.Fatalf("lstat: %v", err) + } + if got := info.Mode()&os.ModeSymlink != 0; got != tc.wantSymlink { + t.Fatalf("is symlink = %v, want %v", got, tc.wantSymlink) + } + }) + } +} + +func TestCopy_EscapedSymlinkSuffixIsLiteralPath(t *testing.T) { + t.Parallel() + src := t.TempDir() + dst := t.TempDir() + writeFile(t, filepath.Join(src, "config:symlink"), "literal", 0o644) + + _, warnings, err := Copy(context.Background(), src, dst, ParseSpecs("config::symlink"), nil) + if err != nil { + t.Fatalf("Copy err: %v", err) + } + if len(warnings) != 0 { + t.Fatalf("unexpected warnings: %v", warnings) + } + if got := readFile(t, filepath.Join(dst, "config:symlink")); got != "literal" { + t.Fatalf("content = %q, want literal", got) + } +} + // Plan is the remote path; symlink entries must fall back to a byte copy there // since a link back to the host repo can't apply on a remote executor. func TestCopy_SymlinkMode_IgnoredInPlanMode(t *testing.T) { src := t.TempDir() writeFile(t, filepath.Join(src, ".env.local"), "SECRET=1", 0o644) + writeFile(t, filepath.Join(src, "config:dev"), "MODE=dev", 0o644) + writeFile(t, filepath.Join(src, ".env:"), "TRAILING=colon", 0o644) - entries, warnings, err := Plan(context.Background(), src, Parse(".env.local:symlink"), nil) + entries, warnings, err := Plan(context.Background(), src, + Parse(".env.local:symlink, config:dev, .env:"), nil) if err != nil { t.Fatalf("Plan err: %v", err) } if len(warnings) != 0 { t.Fatalf("unexpected warnings: %v", warnings) } - if len(entries) != 1 || entries[0].RelPath != ".env.local" || string(entries[0].Content) != "SECRET=1" { - t.Fatalf("expected a copied entry with content, got %#v", entries) + want := map[string]string{ + ".env.local": "SECRET=1", + "config:dev": "MODE=dev", + ".env:": "TRAILING=colon", + } + if len(entries) != len(want) { + t.Fatalf("entries = %#v, want %d entries", entries, len(want)) + } + for _, entry := range entries { + if string(entry.Content) != want[entry.RelPath] { + t.Fatalf("entry %q content = %q, want %q", entry.RelPath, entry.Content, want[entry.RelPath]) + } } } diff --git a/apps/backend/internal/worktree/manager_copyfiles_test.go b/apps/backend/internal/worktree/manager_copyfiles_test.go index 104103dc4f..b745d4e9e7 100644 --- a/apps/backend/internal/worktree/manager_copyfiles_test.go +++ b/apps/backend/internal/worktree/manager_copyfiles_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "runtime" "testing" ) @@ -94,6 +95,37 @@ func TestManagerCreate_CopiesFiles_HappyPath(t *testing.T) { } } +func TestManagerCreate_SymlinksConfiguredFileToSource(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation often requires privilege on Windows") + } + + repoPath := initGitRepoForWorktreeTest(t) + sourcePath := writeSourceFile(t, repoPath, ".env", "SECRET=source\n") + provider := &fakeRepoProvider{ + repo: &Repository{ID: "repo-symlink", CopyFiles: ".env:symlink"}, + } + mgr := newManagerForCopyTest(t, provider) + + wt, err := mgr.Create(context.Background(), createReqForCopyTest(repoPath, "symlink")) + if err != nil { + t.Fatalf("Create() unexpected error: %v", err) + } + + linkPath := filepath.Join(wt.Path, ".env") + target, err := os.Readlink(linkPath) + if err != nil { + t.Fatalf("Readlink(%q): %v", linkPath, err) + } + if filepath.IsAbs(target) { + t.Fatalf("symlink target = %q, want relative path", target) + } + resolved := filepath.Clean(filepath.Join(filepath.Dir(linkPath), target)) + if resolved != sourcePath { + t.Fatalf("resolved symlink = %q, want %q", resolved, sourcePath) + } +} + func TestManagerCreate_CopyFilesEmpty_NoOp(t *testing.T) { repoPath := initGitRepoForWorktreeTest(t) // Add an untracked file that would be copied if CopyFiles were set — diff --git a/apps/web/components/settings/repository-copy-files-help.tsx b/apps/web/components/settings/repository-copy-files-help.tsx index 5f88a0190a..a5c3b4cff4 100644 --- a/apps/web/components/settings/repository-copy-files-help.tsx +++ b/apps/web/components/settings/repository-copy-files-help.tsx @@ -1,9 +1,8 @@ "use client"; -import { IconInfoCircle } from "@tabler/icons-react"; +import { IconChevronDown, IconInfoCircle } from "@tabler/icons-react"; import { Label } from "@kandev/ui/label"; import { Textarea } from "@kandev/ui/textarea"; -import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@kandev/ui/tooltip"; import type { Repository } from "@/lib/types/http"; type CopyFilesFieldProps = { @@ -13,71 +12,76 @@ type CopyFilesFieldProps = { }; export function CopyFilesField({ repositoryId, copyFiles, onUpdate }: CopyFilesFieldProps) { + const inputId = `copy-files-${repositoryId}`; + const helpId = `copy-files-help-${repositoryId}`; return (
- +