diff --git a/README.md b/README.md index c3f58b2..0b69de1 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,10 @@ xf init ./my-project \ xf init ./existing-xf-project --existing xf init ./existing-xf-project --existing --up +# Composer dependencies are installed automatically when the target +# tracks a composer.json (repository checkouts). Release packages ship +# vendor/ prebuilt and are skipped. + # .env overrides (file + inline; inline wins) xf init ./my-project \ --env-file ./my.env \ @@ -215,6 +219,41 @@ xf compose exec xf mysql -u root xf exec xf ls -la ``` +### Worktrees + +A worktree is a second checkout of the same repository on its own branch, with +its own Docker containers and database. Worktrees are created alongside the +source checkout: `~/Sites/main` gains `~/Sites/main.worktrees/`, named +after the branch's last segment. + +By default `create` clones the source environment — database, `data/` and +`internal_data/` — and points the cloned board at its own URL, labelling its +title with the worktree name. + +```bash +# Create a worktree and set up its environment +xf worktree create dev/24x/feature + +# Branch from somewhere other than the current HEAD +xf worktree create dev/24x/feature --base main + +# Create the worktree without setting anything up +xf worktree create dev/24x/feature --no-setup + +# List worktrees (this project / all known projects) +xf worktree list +xf worktree list-all + +# Print the path of a worktree (bare output, shell-substitution safe) +cd "$(xf worktree path dev/24x/feature)" + +# Remove a worktree and its containers and volumes +xf worktree remove dev/24x/feature + +# Drop registry entries for worktrees that no longer exist +xf worktree prune +``` + ### PHP / Composer / Debug ```bash diff --git a/cmd/xf/composerdetect_test.go b/cmd/xf/composerdetect_test.go new file mode 100644 index 0000000..9833abb --- /dev/null +++ b/cmd/xf/composerdetect_test.go @@ -0,0 +1,72 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestShouldRunComposer(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + files []string + want bool + }{ + { + name: "composer.json present", + files: []string{"composer.json"}, + want: true, + }, + { + name: "composer.json and lock present", + files: []string{"composer.json", "composer.lock"}, + want: true, + }, + { + name: "no composer files", + files: nil, + want: false, + }, + { + name: "lock without json is not a composer project", + files: []string{"composer.lock"}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + for _, name := range tt.files { + if err := os.WriteFile(filepath.Join(dir, name), []byte("{}"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + + if got := shouldRunComposer(dir); got != tt.want { + t.Errorf("shouldRunComposer = %v, want %v", got, tt.want) + } + }) + } +} + +// TestShouldRunComposerIgnoresDirectory guards against a directory named +// composer.json being mistaken for a manifest. +func TestShouldRunComposerIgnoresDirectory(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + if err := os.MkdirAll(filepath.Join(dir, "composer.json"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + if shouldRunComposer(dir) { + t.Error("a directory named composer.json must not count as a manifest") + } +} diff --git a/cmd/xf/init.go b/cmd/xf/init.go index 512b48b..c90001c 100644 --- a/cmd/xf/init.go +++ b/cmd/xf/init.go @@ -35,7 +35,8 @@ Fresh Install Mode (default): 4. Sets up Docker configuration 5. Configures the .env file 6. Runs 'up' to start the containers - 7. Runs 'xf:install' to complete the installation + 7. Runs 'composer install' if composer.json is present + 8. Runs 'xf:install' to complete the installation Existing Directory Mode (--existing flag): For core developers who already have XenForo source files checked out. @@ -46,6 +47,10 @@ Existing Directory Mode (--existing flag): 3. Configures the .env file 4. Optionally starts containers (with --up flag) + Repository checkouts track composer.json, so dependencies are installed + automatically once the containers are running. Release packages ship + vendor/ prebuilt and have no manifest, so they are skipped. + Examples: # Fresh install (interactive) xf init ./my-project @@ -86,6 +91,7 @@ type InitOptions struct { InstanceName string SkipUp bool SkipInstall bool + SkipComposer bool ExistingOnly bool Contexts []string StartContainers bool @@ -109,6 +115,7 @@ var ( flagInitInstance string flagInitSkipUp bool flagInitSkipInstall bool + flagInitSkipComposer bool flagInitExisting bool flagInitContexts []string flagInitUp bool @@ -127,6 +134,7 @@ func init() { initCmd.Flags().StringVar(&flagInitInstance, "instance", "", "Docker instance name") initCmd.Flags().BoolVar(&flagInitSkipUp, "skip-up", false, "skip starting Docker containers") initCmd.Flags().BoolVar(&flagInitSkipInstall, "skip-install", false, "skip running xf:install") + initCmd.Flags().BoolVar(&flagInitSkipComposer, "skip-composer", false, "skip running composer install") initCmd.Flags().BoolVar(&flagInitExisting, "existing", false, "initialize Docker in an existing XenForo directory (skips download)") initCmd.Flags().StringSliceVar(&flagInitContexts, "contexts", nil, "Docker contexts to enable (e.g., caddy,mysql,development,redis)") initCmd.Flags().BoolVar(&flagInitUp, "up", false, "start containers after initialization (for --existing mode)") @@ -162,6 +170,7 @@ func runInit(cmd *cobra.Command, args []string) error { InstanceName: flagInitInstance, SkipUp: flagInitSkipUp, SkipInstall: flagInitSkipInstall, + SkipComposer: flagInitSkipComposer, ExistingOnly: flagInitExisting, Contexts: flagInitContexts, StartContainers: flagInitUp, @@ -232,6 +241,36 @@ func detectXenForo(path string) (bool, error) { return false, fmt.Errorf("failed to check XenForo path: %w", err) } +// validateAdminDetails reports whether the installer has everything it needs. +// +// installExistingXenForo passes these straight to xf:install, so a missing +// value would install a broken administrator or an empty board title instead +// of failing. +func validateAdminDetails(opts *InitOptions) error { + var missing []string + + if opts.AdminUser == "" { + missing = append(missing, "--admin-user") + } + + if opts.AdminPassword == "" { + missing = append(missing, "--admin-password") + } + + if opts.AdminEmail == "" { + missing = append(missing, "--admin-email") + } + + if len(missing) > 0 { + return newUsageError(fmt.Errorf( + "missing required flags for the XenForo installation: %s: %w", + strings.Join(missing, ", "), ErrInvalidInput, + )) + } + + return nil +} + func initExisting(ctx context.Context, opts *InitOptions) error { ui.Println(ui.Bold.Render("Initializing Docker environment in existing XenForo directory...")) ui.Println() @@ -251,6 +290,11 @@ func initExisting(ctx context.Context, opts *InitOptions) error { ui.PrintSuccess("Docker Compose is available") ui.Println() + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("failed to load configuration: %w", err) + } + step := 1 totalSteps := 3 @@ -279,6 +323,11 @@ func initExisting(ctx context.Context, opts *InitOptions) error { ui.PrintStep(step, totalSteps, "Starting environment") + // Detection can fail or return nothing, and installing --url= empty would + // leave the board with no address at all, so the predictable instance URL + // is the starting point. + siteURL := fallbackBoardURL(opts.InstanceName) + if opts.StartContainers { runner, err := dockercompose.NewRunner(xfDir) if err != nil { @@ -291,8 +340,43 @@ func initExisting(ctx context.Context, opts *InitOptions) error { url, err := runner.GetURL(ctx) if err == nil && url != "" { + siteURL = url + ui.PrintDetail("Site: " + url) } + + // Composer and the installer both run inside the container, so they can + // only follow a successful start. + if shouldRunComposer(xfDir) && !opts.SkipComposer { + ui.Println() + + if err := runComposerInstall(ctx, runner, cfg.Verbose); err != nil { + return err + } + } + + if !opts.SkipInstall && opts.AdminUser != "" { + // The installer receives these verbatim, so an empty value becomes + // an empty board title or an unusable administrator rather than a + // reported error. + if opts.SiteTitle == "" { + opts.SiteTitle = opts.EnvResolved["XF_TITLE"] + } + + if opts.SiteTitle == "" { + opts.SiteTitle = fmt.Sprintf("XenForo [%s]", opts.InstanceName) + } + + if err := validateAdminDetails(opts); err != nil { + return err + } + + ui.Println() + + if err := installExistingXenForo(ctx, runner, opts, siteURL, cfg.Verbose); err != nil { + return err + } + } } else { ui.PrintDetail("Skipped (use --up flag to start containers)") } @@ -576,3 +660,58 @@ func runInteractiveSetup(ctx context.Context, opts *InitOptions) error { return nil } + +// installExistingXenForo runs xf:install in an already-configured environment. +// +// The password is passed through the environment rather than the command line, +// so it does not appear in the container's process list. +func installExistingXenForo( + ctx context.Context, + runner *dockercompose.Runner, + opts *InitOptions, + siteURL string, + verbose bool, +) error { + if err := runner.WaitForDatabase(ctx, 2*time.Second); err != nil { + return fmt.Errorf("failed waiting for database to become ready: %w", err) + } + + installArgs := []string{ + "xf:install", + "--no-interaction", + "--clear", + "--user=" + opts.AdminUser, + "--email=" + opts.AdminEmail, + "--title=" + opts.SiteTitle, + "--url=" + siteURL, + } + + installEnv := map[string]string{"XF_INSTALL_PASSWORD": opts.AdminPassword} + shellInstallArgs := []string{"sh", "-c", installShellCommand(installArgs)} + + if verbose { + ui.PrintSubstep("Running XenForo installation...") + + if err := runner.ExecOrRunWithEnv(ctx, "xf", true, installEnv, shellInstallArgs...); err != nil { + return fmt.Errorf("failed to install XenForo: %w", err) + } + + return nil + } + + spinner := ui.NewSpinner("Installing XenForo...") + spinner.Start() + + tracker := newPhaseTrackerWriter(spinner, "Installing XenForo", installPhaseRules()) + + if err := runner.ExecOrRunWithEnvAndOutput(ctx, "xf", true, installEnv, tracker, tracker, shellInstallArgs...); err != nil { + spinner.Stop() + printHiddenOutputTail("Installer output", tracker.TailLines()) + + return fmt.Errorf("failed to install XenForo: %w", err) + } + + spinner.StopWithMessage("success", "XenForo installed") + + return nil +} diff --git a/cmd/xf/init_execute.go b/cmd/xf/init_execute.go index ee7a1df..452fcb9 100644 --- a/cmd/xf/init_execute.go +++ b/cmd/xf/init_execute.go @@ -42,7 +42,20 @@ func executeInit(ctx context.Context, opts *InitOptions) error { titleMap := getProductTitleMap(ctx, client, opts.LicenseKey) + // A repository checkout is the only source that needs Composer: it tracks + // a composer.json, while release packages ship vendor/ prebuilt and have + // none. That is knowable before the files land, so the total is correct + // from the first step rather than changing halfway through. + // + // --existing installs run from an existing checkout, so the target's own + // composer.json is the answer there. + runComposer := !opts.SkipComposer && shouldRunComposer(opts.TargetPath) + totalSteps := 7 + if runComposer { + totalSteps++ + } + step := 1 ui.Println() @@ -150,6 +163,16 @@ func executeInit(ctx context.Context, opts *InitOptions) error { ui.PrintWarning(fmt.Sprintf("Could not auto-detect site URL, using fallback %s: %v", siteURL, detectedErr)) } + if runComposer { + ui.Println() + ui.PrintStep(step, totalSteps, "Installing Composer dependencies") + step++ + + if err := runComposerInstall(ctx, runner, cfg.Verbose); err != nil { + return err + } + } + ui.Println() ui.PrintStep(step, totalSteps, "Installing XenForo") @@ -173,9 +196,7 @@ func executeInit(ctx context.Context, opts *InitOptions) error { "XF_INSTALL_PASSWORD": opts.AdminPassword, } - installArgs = append(installArgs, "--password=$(printenv XF_INSTALL_PASSWORD)") - shellCmd := shellJoinArgs(append([]string{"php", "cmd.php"}, installArgs...)) - shellInstallArgs := []string{"sh", "-c", shellCmd} + shellInstallArgs := []string{"sh", "-c", installShellCommand(installArgs)} if cfg.Verbose { ui.PrintSubstep("Running XenForo installation...") @@ -650,3 +671,46 @@ func configureEnvironment(opts *InitOptions) error { return nil } + +// shouldRunComposer reports whether a directory is a Composer project. +// +// Repository checkouts track composer.json, so a fresh worktree has one and its +// dependencies must be installed. Release packages ship vendor/ prebuilt and +// have no manifest, so they are skipped automatically. +func shouldRunComposer(targetPath string) bool { + info, err := os.Stat(filepath.Join(targetPath, "composer.json")) + + return err == nil && !info.IsDir() +} + +// runComposerInstall installs Composer dependencies inside the container. +func runComposerInstall(ctx context.Context, runner *dockercompose.Runner, verbose bool) error { + args := []string{"install", "--no-interaction"} + + if verbose { + ui.PrintSubstep("Running composer install...") + + if err := runner.Composer(ctx, args...); err != nil { + return fmt.Errorf("failed to install Composer dependencies: %w", err) + } + + return nil + } + + spinner := ui.NewSpinner("Installing Composer dependencies...") + spinner.Start() + + tracker := newPhaseTrackerWriter(spinner, "Installing Composer dependencies", nil) + + composerArgs := append([]string{"composer"}, args...) + if err := runner.ExecOrRunWithOutput(ctx, "xf", true, tracker, tracker, composerArgs...); err != nil { + spinner.StopWithMessage("error", "Failed to install Composer dependencies") + printHiddenOutputTail("Composer output", tracker.TailLines()) + + return fmt.Errorf("failed to install Composer dependencies: %w", err) + } + + spinner.StopWithMessage("success", "Composer dependencies installed") + + return nil +} diff --git a/cmd/xf/init_helpers.go b/cmd/xf/init_helpers.go index 11b7c0b..bbdd878 100644 --- a/cmd/xf/init_helpers.go +++ b/cmd/xf/init_helpers.go @@ -238,15 +238,49 @@ func chooseBoardURL(instanceName, detectedURL string, detectedErr error) (string return detectedURL, true } +// installShellCommand builds the shell command that runs xf:install. +// +// Every argument is shell-quoted, so installer values such as the site title +// cannot inject shell syntax. The password substitution is quoted too, so a +// password containing spaces or glob characters reaches the installer +// verbatim. +// +// The password is passed through the environment and expanded by sh rather +// than being interpolated here. That keeps it out of xf's own argv, out of the +// docker compose invocation, and out of anything that logs either of those. +// +// It does not keep it out of the php process's argv inside the container: +// XF\Cli\Command\Install accepts the administrator password only via +// --password or an interactive hidden prompt, and --no-interaction rules the +// prompt out. So for the lifetime of the install, the password is visible to +// anything that can list processes in that container. The container is a +// single-tenant development environment created by this tool, so that exposure +// is accepted; it should be revisited if XenForo ever accepts the password on +// stdin or from an environment variable of its own. +func installShellCommand(installArgs []string) string { + command := shellJoinArgs(append([]string{"php", "cmd.php"}, installArgs...)) + + // Expanded directly rather than through $(printenv ...): command + // substitution strips trailing newlines, so a password ending in one would + // reach the installer altered. + return command + ` --password="$XF_INSTALL_PASSWORD"` +} + func shellJoinArgs(args []string) string { parts := make([]string, len(args)) for i, arg := range args { - if strings.ContainsAny(arg, " \t\"\\") && !strings.Contains(arg, "$(") { - parts[i] = "'" + strings.ReplaceAll(arg, "'", "'\"'\"'") + "'" - } else { - parts[i] = arg - } + parts[i] = shellQuote(arg) } return strings.Join(parts, " ") } + +// shellQuote renders a string as a single-quoted POSIX shell word. +// +// Every argument is quoted unconditionally. Quoting only those that look +// dangerous is how injection gets through: a value such as +// `--title=x;rm -rf /` contains no spaces or quotes, so a +// looks-dangerous test passes it to the shell verbatim. +func shellQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" +} diff --git a/cmd/xf/init_steps_test.go b/cmd/xf/init_steps_test.go new file mode 100644 index 0000000..669da2d --- /dev/null +++ b/cmd/xf/init_steps_test.go @@ -0,0 +1,68 @@ +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// The step total and the Composer gate read the same decision, so the printed +// "step N of M" sequence always ends at M. +func TestComposerDecisionDrivesTheStepTotal(t *testing.T) { + cases := []struct { + name string + composerJSON bool + skipComposer bool + wantComposer bool + wantTotalStep int + }{ + {"repository checkout", true, false, true, 8}, + {"release package", false, false, false, 7}, + {"checkout with --skip-composer", true, true, false, 7}, + {"release with --skip-composer", false, true, false, 7}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + + if tc.composerJSON { + path := filepath.Join(dir, "composer.json") + if err := os.WriteFile(path, []byte("{}"), 0o600); err != nil { + t.Fatalf("write composer.json: %v", err) + } + } + + runComposer := !tc.skipComposer && shouldRunComposer(dir) + if runComposer != tc.wantComposer { + t.Errorf("runComposer = %v, want %v", runComposer, tc.wantComposer) + } + + totalSteps := 7 + if runComposer { + totalSteps++ + } + + if totalSteps != tc.wantTotalStep { + t.Errorf("totalSteps = %d, want %d", totalSteps, tc.wantTotalStep) + } + }) + } +} + +func TestShouldRunComposerRequiresARegularFile(t *testing.T) { + dir := t.TempDir() + + if shouldRunComposer(dir) { + t.Error("no composer.json, want false") + } + + // A directory named composer.json is not a manifest. + if err := os.Mkdir(filepath.Join(dir, "composer.json"), 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + if shouldRunComposer(dir) { + t.Error("composer.json is a directory, want false") + } +} diff --git a/cmd/xf/install_shell_test.go b/cmd/xf/install_shell_test.go new file mode 100644 index 0000000..cb297b9 --- /dev/null +++ b/cmd/xf/install_shell_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "context" + "os/exec" + "strings" + "testing" +) + +func TestShellQuoteNeutralisesShellSyntax(t *testing.T) { + cases := []struct { + name string + in string + }{ + {"command separator", "x; touch /tmp/pwned"}, + {"command substitution", "x$(touch /tmp/pwned)"}, + {"backticks", "x`touch /tmp/pwned`"}, + {"pipe", "x | touch /tmp/pwned"}, + {"glob", "*"}, + {"single quote", "it's"}, + {"newline", "x\ntouch /tmp/pwned"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Round-trip through a real shell: echo must reproduce the input + // exactly, which is only true if nothing was interpreted. + script := "printf %s " + shellQuote(tc.in) + + out, err := exec.CommandContext(context.Background(), "sh", "-c", script).Output() + if err != nil { + t.Fatalf("sh -c failed: %v", err) + } + + if string(out) != tc.in { + t.Errorf("shell interpreted the value: got %q, want %q", string(out), tc.in) + } + }) + } +} + +func TestInstallShellCommandQuotesInstallerValues(t *testing.T) { + command := installShellCommand([]string{ + "xf:install", + "--title=Chris' Forum; touch /tmp/pwned", + }) + + // The dangerous value must be quoted, so the separator cannot terminate + // the installer command. + if strings.Contains(command, "; touch /tmp/pwned'") == false { + t.Errorf("value was not quoted as a single word: %s", command) + } + + if strings.HasSuffix(command, "touch /tmp/pwned") { + t.Errorf("command ends with an unquoted injection: %s", command) + } +} + +func TestInstallShellCommandKeepsThePasswordOutOfArgv(t *testing.T) { + command := installShellCommand([]string{"xf:install"}) + + // The password must be read from the environment at run time, and the + // substitution must be quoted so spaces and globs survive intact. + if !strings.Contains(command, `--password="$XF_INSTALL_PASSWORD"`) { + t.Errorf("password is not read from the environment: %s", command) + } +} + +func TestInstallShellCommandPassesAwkwardPasswordsVerbatim(t *testing.T) { + command := installShellCommand([]string{"xf:install"}) + + // Run the built command with a stand-in for php so the installer's view + // of the password can be observed. printf %s\n prints each argument on + // its own line, so a split password would show up as extra lines. + script := strings.Replace(command, `'php' 'cmd.php'`, `printf '%s\n'`, 1) + if script == command { + t.Fatalf("could not substitute the interpreter in %q", command) + } + + cmd := exec.CommandContext(context.Background(), "sh", "-c", script) + cmd.Env = append(cmd.Environ(), "XF_INSTALL_PASSWORD=a b * c'd") + + out, err := cmd.Output() + if err != nil { + t.Fatalf("sh -c failed: %v", err) + } + + lines := strings.Split(strings.TrimRight(string(out), "\n"), "\n") + + last := lines[len(lines)-1] + if last != `--password=a b * c'd` { + t.Errorf("password reached the installer as %q", last) + } +} + +// Command substitution strips trailing newlines, so the password must be +// expanded directly: a password ending in one would otherwise reach the +// installer altered. +func TestInstallShellCommandPreservesTrailingNewlinesInThePassword(t *testing.T) { + command := installShellCommand([]string{"xf:install"}) + + script := strings.Replace(command, `'php' 'cmd.php'`, `printf '%s'`, 1) + if script == command { + t.Fatalf("could not substitute the interpreter in %q", command) + } + + cmd := exec.CommandContext(context.Background(), "sh", "-c", script) + cmd.Env = append(cmd.Environ(), "XF_INSTALL_PASSWORD=secret\n") + + out, err := cmd.Output() + if err != nil { + t.Fatalf("sh -c failed: %v", err) + } + + if !strings.HasSuffix(string(out), "--password=secret\n") { + t.Errorf("trailing newline lost: output ended %q", string(out)) + } +} diff --git a/cmd/xf/worktree.go b/cmd/xf/worktree.go new file mode 100644 index 0000000..ff0808d --- /dev/null +++ b/cmd/xf/worktree.go @@ -0,0 +1,698 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/xenforo-ltd/cli/internal/dockercompose" + "github.com/xenforo-ltd/cli/internal/ui" + "github.com/xenforo-ltd/cli/internal/worktree" +) + +var worktreeCmd = &cobra.Command{ + Use: "worktree", + Short: "Create and manage development worktrees", + Long: `Create a git worktree with a fully configured XenForo environment. + +A worktree is a second checkout of the same repository on its own branch, with +its own Docker containers and database. It lets you work on a feature without +disturbing your main checkout. + +Worktrees are created alongside the source checkout, so ~/Sites/main gains +~/Sites/main.worktrees/. The path is derived from the branch name and is +always predictable. + +'xf worktree create ' creates the worktree and then initialises the +environment: Docker configuration, containers, Composer dependencies and the +XenForo installation. + +Examples: + # Create a worktree and set up its environment + xf worktree create dev/24x/feature + + # Branch from somewhere other than the current HEAD + xf worktree create dev/24x/feature --base main + + # Create the worktree without setting anything up + xf worktree create dev/24x/feature --no-setup + + # Print the path of an existing worktree + cd "$(xf worktree path dev/24x/feature)"`, + // This command only dispatches subcommands. Taking a branch here too would + // make `xf worktree lst` ambiguous, and cobra would resolve it by silently + // creating a branch called "lst" rather than reporting a mistyped + // subcommand. + Args: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + + // `xf worktree help` reads as a request for help. Cobra reserves "help" + // at the root only, so it arrives here as an unknown subcommand. + if args[0] == "help" { + return nil + } + + return fmt.Errorf("unknown command %q for %q: %w", args[0], cmd.CommandPath(), ErrInvalidInput) + }, + RunE: func(cmd *cobra.Command, _ []string) error { + return cmd.Help() + }, +} + +var worktreeCreateCmd = &cobra.Command{ + Use: "create ", + Short: "Create a worktree and set up its environment", + Args: cobra.MaximumNArgs(1), + RunE: runWorktreeCreate, +} + +var worktreeListCmd = &cobra.Command{ + Use: "list", + Short: "List worktrees for this project", + Args: cobra.NoArgs, + RunE: runWorktreeList, +} + +var worktreeListAllCmd = &cobra.Command{ + Use: "list-all", + Short: "List worktrees across all known projects", + Args: cobra.NoArgs, + RunE: runWorktreeListAll, +} + +var worktreePathCmd = &cobra.Command{ + Use: "path ", + Short: "Print the path of a worktree", + Long: `Print the resolved path for a branch's worktree. + +The path is derived from the branch name, so this works whether or not the +worktree exists. Useful for shell and agent use: + + cd "$(xf worktree path dev/24x/feature)" + +Branch names that resolve to no directory of their own, such as "." or "..", +are rejected rather than printing the directory that holds every worktree.`, + Args: cobra.ExactArgs(1), + RunE: runWorktreePath, +} + +var worktreeRemoveCmd = &cobra.Command{ + Use: "remove ", + Short: "Remove a worktree and its containers", + Long: `Remove a worktree, its branch, and its Docker containers and volumes. + +Refuses when the worktree contains uncommitted changes or commits that exist on +no remote, listing what would be lost. Use --force to remove it anyway.`, + Args: cobra.ExactArgs(1), + RunE: runWorktreeRemove, +} + +var worktreePruneCmd = &cobra.Command{ + Use: "prune", + Short: "Drop registry entries for worktrees that no longer exist", + Args: cobra.NoArgs, + RunE: runWorktreePrune, +} + +// Defaults for the throwaway installation a worktree gets. +const ( + defaultWorktreeAdminUser = "admin" + defaultWorktreeAdminPassword = "password" + defaultWorktreeAdminEmail = "admin@example.com" +) + +var ( + flagWorktreeBase string + flagWorktreeAdminUser string + flagWorktreeAdminPassword string + flagWorktreeAdminEmail string + flagWorktreeTitle string + flagWorktreeNoSetup bool + flagWorktreeNoUp bool + flagWorktreeInstance string + flagWorktreeJSON bool + flagWorktreeForce bool + flagWorktreeKeepContainers bool + flagWorktreeFresh bool +) + +func init() { + worktreeCreateCmd.Flags().StringVar(&flagWorktreeBase, "base", "", "ref to branch from (defaults to current HEAD)") + worktreeCreateCmd.Flags().BoolVar(&flagWorktreeNoSetup, "no-setup", false, "create the worktree only, without setting up the environment") + worktreeCreateCmd.Flags().BoolVar(&flagWorktreeNoUp, "no-up", false, "configure the environment but do not start containers") + worktreeCreateCmd.Flags().BoolVar(&flagWorktreeFresh, "fresh", false, "install a clean forum instead of cloning the source environment") + worktreeCreateCmd.Flags().StringVar(&flagWorktreeInstance, "instance", "", "Docker instance name") + // Known limitation: setup progress from init and cloning still goes to + // stdout, so the stream is only pure JSON when --no-setup is used. The + // output layer writes through package-level helpers with no injectable + // writer, so routing it to stderr is a wider change than this command. + worktreeCreateCmd.Flags().BoolVar(&flagWorktreeJSON, "json", false, "output as JSON (setup progress is still written to stdout)") + worktreeCreateCmd.Flags().StringVar(&flagWorktreeAdminUser, "admin-user", "", "admin username (default \"admin\")") + worktreeCreateCmd.Flags().StringVar(&flagWorktreeAdminPassword, "admin-password", "", "admin password (default \"password\")") + worktreeCreateCmd.Flags().StringVar(&flagWorktreeAdminEmail, "admin-email", "", "admin email (default \"admin@example.com\")") + worktreeCreateCmd.Flags().StringVar(&flagWorktreeTitle, "title", "", "site title (defaults to the branch name)") + + worktreeListCmd.Flags().BoolVar(&flagWorktreeJSON, "json", false, "output as JSON") + worktreeListAllCmd.Flags().BoolVar(&flagWorktreeJSON, "json", false, "output as JSON") + worktreeRemoveCmd.Flags().BoolVar(&flagWorktreeForce, "force", false, "remove even if there are uncommitted changes or unpushed commits") + worktreeRemoveCmd.Flags().BoolVar(&flagWorktreeKeepContainers, "keep-containers", false, "leave the Docker containers and volumes in place") + + worktreeCmd.AddCommand(worktreeCreateCmd) + worktreeCmd.AddCommand(worktreeListCmd) + worktreeCmd.AddCommand(worktreeListAllCmd) + worktreeCmd.AddCommand(worktreePathCmd) + worktreeCmd.AddCommand(worktreeRemoveCmd) + worktreeCmd.AddCommand(worktreePruneCmd) + + rootCmd.AddCommand(worktreeCmd) +} + +// worktreeOutput is the machine-readable form of a created worktree. +type worktreeOutput struct { + Path string `json:"path"` + Branch string `json:"branch"` + SourcePath string `json:"source_path"` + SourceBranch string `json:"source_branch"` + Instance string `json:"instance"` + Cloned bool `json:"cloned"` + CreatedAt time.Time `json:"created_at"` +} + +func runWorktreeCreate(cmd *cobra.Command, args []string) error { + branch, err := resolveBranchArg(args) + if err != nil { + return err + } + + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current directory: %w", err) + } + + result, err := worktree.Create(cmd.Context(), worktree.Options{ + SourcePath: cwd, + Branch: branch, + Base: flagWorktreeBase, + Instance: flagWorktreeInstance, + }) + if err != nil { + return err + } + + entry := worktree.Entry{ + SourcePath: result.SourcePath, + SourceBranch: result.SourceBranch, + WorktreePath: result.Path, + Branch: result.Branch, + Instance: result.Instance, + CreatedAt: result.CreatedAt, + } + + if err := recordWorktree(entry); err != nil { + // The worktree exists and is usable; a registry failure must not + // present itself as a failed creation. + ui.PrintWarning(fmt.Sprintf("Could not record worktree in the registry: %v", err)) + } + + if !flagWorktreeJSON { + ui.PrintSuccess("Created worktree " + result.Path) + ui.PrintKeyValuePadded([]ui.KVPair{ + ui.KV("Branch", result.Branch), + ui.KV("Based on", result.SourceBranch), + ui.KV("Instance", result.Instance), + }) + } + + // Cloning imports a database that is already installed, so xf:install must + // not run over it: it would wipe the data that was just copied. + // + // Cloning needs running containers, so --no-up rules it out. Treating the + // worktree as cloning anyway would suppress xf:install as well, leaving it + // with neither an imported database nor an installed one. + cloning := false + + if !flagWorktreeFresh && !flagWorktreeNoUp { + installed, err := sourceIsInstalled(result.SourcePath) + if err != nil { + return err + } + + cloning = installed + } + + if !flagWorktreeNoSetup { + if err := setUpWorktree(cmd.Context(), result, worktreeInitOptions(result, cloning)); err != nil { + return err + } + + if cloning { + if err := cloneEnvironment(cmd.Context(), result.SourcePath, result.Path); err != nil { + return fmt.Errorf("worktree created at %s, but cloning the environment failed: %w", result.Path, err) + } + + entry.Cloned = true + + if err := recordWorktree(entry); err != nil { + ui.PrintWarning(fmt.Sprintf("Could not record worktree in the registry: %v", err)) + } + } + + // Only a fresh install has credentials worth reporting. A cloned + // worktree keeps the source's own logins, so printing the defaults + // would be wrong. + if !cloning && !flagWorktreeJSON && !flagWorktreeNoUp { + ui.Println() + ui.PrintKeyValuePadded([]ui.KVPair{ + ui.KV("Admin user", defaultString(flagWorktreeAdminUser, defaultWorktreeAdminUser)), + ui.KV("Admin password", defaultString(flagWorktreeAdminPassword, defaultWorktreeAdminPassword)), + }) + } + } + + if flagWorktreeJSON { + return printJSON(worktreeOutput{ + Path: result.Path, + Branch: result.Branch, + SourcePath: result.SourcePath, + SourceBranch: result.SourceBranch, + Instance: result.Instance, + Cloned: entry.Cloned, + CreatedAt: result.CreatedAt, + }) + } + + return nil +} + +// worktreeInitOptions builds the init options for a new worktree. +// +// cloning reports whether the source environment will be copied in, which +// suppresses xf:install: the imported database is already installed, and +// reinstalling would wipe the data that was just copied. +func worktreeInitOptions(result *worktree.Result, cloning bool) *InitOptions { + // A worktree is a disposable development environment, so a fresh install + // uses fixed credentials rather than prompting. Knowing the login without + // being asked is the point: one command produces a usable forum. A cloned + // worktree keeps the source's own credentials. + return &InitOptions{ + TargetPath: result.Path, + InstanceName: result.Instance, + ExistingOnly: true, + SkipUp: flagWorktreeNoUp, + StartContainers: !flagWorktreeNoUp, + SkipInstall: cloning, + AdminUser: defaultString(flagWorktreeAdminUser, defaultWorktreeAdminUser), + AdminPassword: defaultString(flagWorktreeAdminPassword, defaultWorktreeAdminPassword), + AdminEmail: defaultString(flagWorktreeAdminEmail, defaultWorktreeAdminEmail), + SiteTitle: defaultString(flagWorktreeTitle, result.Branch), + EnvResolved: map[string]string{}, + EnvSources: map[string]string{}, + ProductOverrides: map[string]int{}, + ProductTitleMap: map[string]string{}, + } +} + +// setUpWorktree initialises the environment by delegating to init, which +// already handles Docker configuration, containers, Composer and installation. +func setUpWorktree(ctx context.Context, result *worktree.Result, opts *InitOptions) error { + if err := initExisting(ctx, opts); err != nil { + return fmt.Errorf("worktree created at %s, but setting up its environment failed: %w", result.Path, err) + } + + return nil +} + +func runWorktreePath(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current directory: %w", err) + } + + source, err := worktree.SourceCheckout(cmd.Context(), cwd) + if err != nil { + return err + } + + target, err := worktree.ResolveExistingPath(source, args[0]) + if err != nil { + return err + } + + // Printed bare, with no decoration, so it can be used directly in a shell. + fmt.Println(target) + + return nil +} + +func runWorktreeList(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current directory: %w", err) + } + + source, err := worktree.SourceCheckout(cmd.Context(), cwd) + if err != nil { + return err + } + + registry, err := worktree.NewRegistry() + if err != nil { + return err + } + + entries, err := registry.ForSource(source) + if err != nil { + return err + } + + return printWorktrees(entries) +} + +func runWorktreeListAll(cmd *cobra.Command, args []string) error { + registry, err := worktree.NewRegistry() + if err != nil { + return err + } + + entries, err := registry.All() + if err != nil { + return err + } + + return printWorktrees(entries) +} + +func runWorktreeRemove(cmd *cobra.Command, args []string) error { + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current directory: %w", err) + } + + source, err := worktree.SourceCheckout(cmd.Context(), cwd) + if err != nil { + return err + } + + target, err := worktree.ResolveExistingPath(source, args[0]) + if err != nil { + return err + } + + // The path is derived from the branch's last segment, so dev/a/foo and + // dev/b/foo resolve to the same directory. Removing without checking which + // branch is actually there would destroy the wrong worktree, its branch, + // and its volumes. + if err := verifyWorktreeBranch(cmd.Context(), target, args[0]); err != nil { + return err + } + + // The safety check runs before anything is destroyed. Tearing down first + // and refusing afterwards would report that the worktree was kept while + // its database and volumes had already been deleted. + if !flagWorktreeForce { + if err := worktree.CheckRemovable(cmd.Context(), target); err != nil { + return err + } + } + + // Containers must be torn down before the directory goes: compose reads + // compose.yaml from the worktree to know what it owns, so removing the + // files first would strand the containers and volumes. + if !flagWorktreeKeepContainers { + if err := destroyWorktreeEnvironment(cmd.Context(), target); err != nil { + return err + } + } + + if err := worktree.Remove(cmd.Context(), source, target, flagWorktreeForce); err != nil { + return err + } + + registry, regErr := worktree.NewRegistry() + if regErr != nil { + // Reported rather than ignored: the worktree is gone but the registry + // still lists it, and only this message tells the user why. + ui.PrintWarning(fmt.Sprintf("Could not open the worktree registry: %v", regErr)) + } else if err := registry.Remove(target); err != nil { + ui.PrintWarning(fmt.Sprintf("Could not update the worktree registry: %v", err)) + } + + ui.PrintSuccess("Removed worktree " + target) + + return nil +} + +func runWorktreePrune(cmd *cobra.Command, args []string) error { + registry, err := worktree.NewRegistry() + if err != nil { + return err + } + + entries, err := registry.All() + if err != nil { + return err + } + + pruned := 0 + + for _, entry := range entries { + if _, statErr := os.Stat(entry.WorktreePath); os.IsNotExist(statErr) { + if err := registry.Remove(entry.WorktreePath); err != nil { + return err + } + + pruned++ + } + } + + if pruned == 0 { + ui.PrintInfo("No stale worktree entries found.") + + return nil + } + + ui.PrintSuccess(fmt.Sprintf("Pruned %d stale worktree %s.", pruned, plural(pruned, "entry", "entries"))) + + return nil +} + +// worktreeState reports whether a registered worktree still exists on disk. +func worktreeState(worktreePath string) string { + if _, err := os.Stat(worktreePath); os.IsNotExist(err) { + return "missing" + } + + return "ok" +} + +// printWorktrees renders entries, reconciling them against the filesystem. +// +// The registry is a record, not the source of truth: worktrees get removed +// outside xf, so entries are checked rather than trusted. +func printWorktrees(entries []worktree.Entry) error { + if flagWorktreeJSON { + // The same reconciliation the table performs, so machine consumers + // are not told about worktrees that no longer exist on disk. + type worktreeListEntry struct { + worktree.Entry + + State string `json:"state"` + } + + listed := make([]worktreeListEntry, 0, len(entries)) + for _, entry := range entries { + listed = append(listed, worktreeListEntry{ + Entry: entry, + State: worktreeState(entry.WorktreePath), + }) + } + + return printJSON(listed) + } + + if len(entries) == 0 { + ui.PrintInfo("No worktrees found.") + + return nil + } + + headers := []string{"BRANCH", "PATH", "INSTANCE", "STATE"} + rows := make([][]string, 0, len(entries)) + + for _, entry := range entries { + rows = append(rows, []string{ + entry.Branch, + shortenPath(entry.WorktreePath), + entry.Instance, + worktreeState(entry.WorktreePath), + }) + } + + ui.Println(ui.NewTable(headers, rows)) + + return nil +} + +func recordWorktree(entry worktree.Entry) error { + registry, err := worktree.NewRegistry() + if err != nil { + return err + } + + return registry.Add(entry) +} + +func printJSON(v any) error { + data, err := json.MarshalIndent(v, "", " ") + if err != nil { + return fmt.Errorf("failed to encode output: %w", err) + } + + ui.Println(string(data)) + + return nil +} + +// resolveBranchArg returns the branch to create. +func resolveBranchArg(args []string) (string, error) { + if len(args) == 0 || strings.TrimSpace(args[0]) == "" { + return "", fmt.Errorf( + "a branch name is required, for example %q: %w", + "xf worktree create dev/24x/feature", ErrInvalidInput, + ) + } + + return args[0], nil +} + +// shortenPath replaces the home directory with ~ for display. +func shortenPath(path string) string { + home, err := os.UserHomeDir() + if err != nil { + return path + } + + if rel, err := filepath.Rel(home, path); err == nil && !strings.HasPrefix(rel, "..") { + return filepath.Join("~", rel) + } + + return path +} + +func plural(n int, singular, pluralForm string) string { + if n == 1 { + return singular + } + + return pluralForm +} + +// defaultString returns value, or fallback when value is empty. +func defaultString(value, fallback string) string { + if value != "" { + return value + } + + return fallback +} + +// verifyWorktreeBranch reports whether the worktree at path has branch checked +// out. +// +// Worktree paths are derived from a branch's last segment, so dev/a/foo and +// dev/b/foo share a directory. Without this check, removing one branch would +// silently destroy the other's worktree, branch, containers and volumes. +func verifyWorktreeBranch(ctx context.Context, worktreePath, branch string) error { + if _, err := os.Stat(worktreePath); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("no worktree at %s: %w", worktreePath, err) + } + + return fmt.Errorf("failed to inspect %s: %w", worktreePath, err) + } + + current, err := worktree.CurrentBranch(ctx, worktreePath) + if err != nil { + return fmt.Errorf("failed to determine the branch checked out at %s: %w", worktreePath, err) + } + + if current != branch { + return fmt.Errorf( + "%s has %s checked out, not %s: refusing to remove it%.0w", + worktreePath, current, branch, ErrInvalidInput, + ) + } + + return nil +} + +// destroyWorktreeEnvironment removes a worktree's containers and volumes. +// +// A worktree that was never set up has no compose configuration, which is not +// an error: there is simply nothing to tear down. +func destroyWorktreeEnvironment(ctx context.Context, worktreePath string) error { + runner, err := dockercompose.NewRunner(worktreePath) + if err != nil { + // A worktree that was never set up has nothing to tear down, and a + // directory that is already gone cannot have running containers. + if errors.Is(err, dockercompose.ErrEnvNotInitialized) || errors.Is(err, os.ErrNotExist) { + return nil + } + + // Any other failure means the environment could not be inspected, not + // that it is absent. Continuing would delete the worktree and strand + // its containers and volumes, so stop instead. + return fmt.Errorf("failed to inspect the worktree environment: %w", err) + } + + spinner := ui.NewSpinner("Removing containers and volumes...") + spinner.Start() + + if err := runner.Destroy(ctx); err != nil { + spinner.StopWithMessage("error", "Failed to remove containers") + + return fmt.Errorf("failed to remove the worktree environment: %w", err) + } + + spinner.StopWithMessage("success", "Containers and volumes removed") + + return nil +} + +// sourceIsInstalled reports whether the source checkout holds a XenForo +// installation that can be cloned. +// +// A checkout that has never been installed has no database or attachments to +// copy, so a new worktree gets a fresh install instead. +func sourceIsInstalled(sourcePath string) (bool, error) { + // XenForo writes this once installation completes. + markers := []string{ + filepath.Join(sourcePath, "internal_data", "install-lock.php"), + // Without compose configuration there is no database to dump. + filepath.Join(sourcePath, "compose.yaml"), + } + + for _, marker := range markers { + if _, err := os.Stat(marker); err != nil { + if os.IsNotExist(err) { + return false, nil + } + + // A permission or I/O failure is not an absent marker. Treating it + // as one would quietly install a fresh forum where the user asked + // for a clone of an existing one. + return false, fmt.Errorf("failed to inspect %s: %w", marker, err) + } + } + + return true, nil +} diff --git a/cmd/xf/worktree_args_test.go b/cmd/xf/worktree_args_test.go new file mode 100644 index 0000000..67aae19 --- /dev/null +++ b/cmd/xf/worktree_args_test.go @@ -0,0 +1,76 @@ +package main + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" +) + +// TestWorktreeParentTakesNoArguments is the guard against the original footgun: +// `xf worktree lst` silently created a branch called "lst" instead of reporting +// a mistyped subcommand. The parent dispatches subcommands only, so an +// unrecognised name is now an error rather than a new worktree. +func TestWorktreeParentTakesNoArguments(t *testing.T) { + configureErrorHandling(rootCmd) + + var out bytes.Buffer + + rootCmd.SetOut(&out) + rootCmd.SetErr(&out) + rootCmd.SetArgs([]string{"worktree", "lst"}) + + t.Cleanup(func() { + rootCmd.SetArgs(nil) + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + }) + + err := rootCmd.ExecuteContext(context.Background()) + if err == nil { + t.Fatal("expected an unrecognised subcommand to be rejected") + } + + if !strings.Contains(err.Error(), "lst") { + t.Errorf("error %q does not name the unrecognised argument", err) + } +} + +// TestWorktreeCreateAcceptsAnyBranchName confirms the explicit form removes the +// ambiguity: once "create" is given, a branch may be named anything, including +// something that matches a subcommand. +func TestWorktreeCreateAcceptsAnyBranchName(t *testing.T) { + t.Parallel() + + for _, name := range []string{ + "dev/24x/feature", + "feature", + "list", + "help", + "remove", + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + got, err := resolveBranchArg([]string{name}) + if err != nil { + t.Errorf("resolveBranchArg(%q) returned %v, want it accepted", name, err) + } + + if got != name { + t.Errorf("resolveBranchArg(%q) = %q", name, got) + } + }) + } +} + +func TestWorktreeCreateRequiresABranch(t *testing.T) { + t.Parallel() + + for _, args := range [][]string{nil, {}, {""}, {" "}} { + if _, err := resolveBranchArg(args); !errors.Is(err, ErrInvalidInput) { + t.Errorf("resolveBranchArg(%v) = %v, want a rejection", args, err) + } + } +} diff --git a/cmd/xf/worktree_clone.go b/cmd/xf/worktree_clone.go new file mode 100644 index 0000000..004741b --- /dev/null +++ b/cmd/xf/worktree_clone.go @@ -0,0 +1,268 @@ +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/xenforo-ltd/cli/internal/dockercompose" + "github.com/xenforo-ltd/cli/internal/ui" + "github.com/xenforo-ltd/cli/internal/worktree" +) + +// clonedDirectories are the directories copied from the source installation. +// +// data/ holds public attachments and assets; internal_data/ holds private +// attachments, which are the main reason to clone at all. Everything else a +// XenForo install needs is either tracked in git or regenerated. +var clonedDirectories = []string{"data", "internal_data"} + +// cloneEnvironment reproduces a source installation in a new worktree: its +// database first, then its files. +// +// The database is dumped and imported rather than copied, because each instance +// has its own named volume that the target's containers already own. +func cloneEnvironment(ctx context.Context, sourcePath, worktreePath string) error { + sourceRunner, err := dockercompose.NewRunner(sourcePath) + if err != nil { + return fmt.Errorf("cannot clone from %s: %w", sourcePath, err) + } + + targetRunner, err := dockercompose.NewRunner(worktreePath) + if err != nil { + return fmt.Errorf("cannot clone into %s: %w", worktreePath, err) + } + + if err := cloneDatabase(ctx, sourceRunner, targetRunner); err != nil { + return err + } + + if err := cloneFiles(ctx, sourcePath, worktreePath); err != nil { + return err + } + + return retargetBoardIdentity(ctx, targetRunner, filepath.Base(worktreePath)) +} + +// retargetBoardIdentity points the cloned board at its own address and marks +// its title with the worktree name. +// +// boardUrl is stored in the database, so a clone inherits the source's URL and +// generates links back to the forum it was copied from. XenForo has no config +// override for options, so the value must be updated in the database. +// +// OptionRepository::updateOptions is used rather than a direct UPDATE because +// it also rebuilds the option cache. Writing the row alone would leave the old +// URL in service until something else happened to rebuild it. +func retargetBoardIdentity(ctx context.Context, target *dockercompose.Runner, label string) error { + url, err := target.GetURL(ctx) + if err != nil || url == "" { + ui.PrintWarning("Could not determine the worktree's URL; the board URL still points at the source") + + return nil + } + + spinner := ui.NewSpinner("Updating board URL and title...") + spinner.Start() + + // Run through XenForo's own bootstrap so the repository and cache rebuild + // behave exactly as they do for xf:install. + // php -r takes bare statements, without an opening tag. + // Both options are set in one call so the option cache rebuilds once. The + // title is derived inside PHP because it depends on the current value, + // which is only known once XenForo has booted. + script := fmt.Sprintf( + `require __DIR__ . '/src/XF.php';`+ + `XF::start(__DIR__);`+ + `$app = XF::setupApp(XF\App::class);`+ + `$title = rtrim(preg_replace('/\s*\[[^\[\]]*\]\s*$/', '', $app->options()->boardTitle));`+ + `$title = $title === '' ? %[2]s : $title . ' ' . %[2]s;`+ + `$app->repository(XF\Repository\OptionRepository::class)`+ + `->updateOptions(['boardUrl' => %[1]s, 'boardTitle' => $title]);`, + phpQuote(url), + phpQuote("["+label+"]"), + ) + + if err := target.PHP(ctx, "-r", script); err != nil { + spinner.Stop() + ui.PrintWarning(fmt.Sprintf("Could not update the board URL to %s: %v", url, err)) + ui.Println(" Set it in the admin control panel under Options > Basic board information.") + + return nil + } + + spinner.StopWithMessage("success", "Board URL set to "+url) + + return nil +} + +// phpQuote renders a string as a single-quoted PHP literal. +func phpQuote(value string) string { + escaped := strings.ReplaceAll(value, "\\", "\\\\") + escaped = strings.ReplaceAll(escaped, "'", "\\'") + + return "'" + escaped + "'" +} + +// cloneDatabase streams a dump from the source instance into the target's. +func cloneDatabase(ctx context.Context, source, target *dockercompose.Runner) error { + user, password := source.DatabaseCredentials() + database := source.DatabaseName() + + spinner := ui.NewSpinner("Exporting database from source...") + spinner.Start() + + // CreateTemp generates an unpredictable name and creates the file mode + // 0600. A fixed path in the shared temp directory would leave the whole + // forum database, including password hashes, readable by other users on + // the host, and would let them pre-create the path as a symlink. + dump, err := os.CreateTemp("", "xf-clone-"+target.Instance()+"-*.sql") + if err != nil { + spinner.StopWithMessage("error", "Failed to export database") + + return fmt.Errorf("failed to create dump file: %w", err) + } + + dumpPath := dump.Name() + + defer func() { + _ = os.Remove(dumpPath) + }() + + // The password goes in the environment: an argument would be visible to + // anything that can list processes in the container. + dumpEnv := map[string]string{"MYSQL_PWD": password} + + // --single-transaction keeps the source usable during the dump. + dumpCmd := []string{ + "mariadb-dump", + "--user=" + user, + "--single-transaction", + "--routines", + "--events", + database, + } + + if err := source.ExecCaptureWithEnv(ctx, "mysql", dumpEnv, dump, dumpCmd...); err != nil { + _ = dump.Close() + spinner.StopWithMessage("error", "Failed to export database") + + return fmt.Errorf("failed to export the source database: %w", err) + } + + if err := dump.Close(); err != nil { + spinner.StopWithMessage("error", "Failed to export database") + + return fmt.Errorf("failed to finish the dump: %w", err) + } + + info, err := os.Stat(dumpPath) + if err != nil { + spinner.StopWithMessage("error", "Failed to export database") + + return fmt.Errorf("failed to inspect the dump: %w", err) + } + + spinner.StopWithMessage("success", "Database exported ("+ui.FormatBytes(info.Size())+")") + + spinner = ui.NewSpinner("Importing database into worktree...") + spinner.Start() + + restore, err := os.Open(dumpPath) + if err != nil { + spinner.StopWithMessage("error", "Failed to import database") + + return fmt.Errorf("failed to read the dump: %w", err) + } + + defer func() { + _ = restore.Close() + }() + + targetUser, targetPassword := target.DatabaseCredentials() + + importEnv := map[string]string{"MYSQL_PWD": targetPassword} + + importCmd := []string{ + "mariadb", + "--user=" + targetUser, + target.DatabaseName(), + } + + if err := target.ExecInputWithEnv(ctx, "mysql", importEnv, restore, importCmd...); err != nil { + spinner.StopWithMessage("error", "Failed to import database") + + return fmt.Errorf("failed to import the database: %w", err) + } + + spinner.StopWithMessage("success", "Database imported") + + return nil +} + +// cloneFiles copies the source's user content into the worktree. +func cloneFiles(ctx context.Context, sourcePath, worktreePath string) error { + for _, dir := range clonedDirectories { + src := filepath.Join(sourcePath, dir) + + if _, err := os.Stat(src); os.IsNotExist(err) { + continue + } + + spinner := ui.NewSpinner("Copying " + dir + "...") + spinner.Start() + + var lastReported int + + err := worktree.CopyTree(ctx, src, filepath.Join(worktreePath, dir), func(copied, total int) { + // Updating on every file would spend more time rendering than + // copying, since code_cache alone is thousands of small files. + if total > 0 && (copied == total || copied-lastReported >= progressUpdateInterval) { + lastReported = copied + + spinner.UpdateMessage(fmt.Sprintf("Copying %s... %d/%d files", dir, copied, total)) + } + }) + if err != nil { + spinner.StopWithMessage("error", "Failed to copy "+dir) + + return fmt.Errorf("failed to copy %s: %w", dir, err) + } + + spinner.StopWithMessage("success", "Copied "+dir) + } + + return nil +} + +// progressUpdateInterval is how many files to copy between progress updates. +const progressUpdateInterval = 100 + +// retitleBoard appends a worktree label to a board title, replacing any label +// already present. +// +// A clone inherits the source forum's title, so several worktrees would +// otherwise be indistinguishable in a browser tab. +// +// This mirrors the expression used in retargetBoardIdentity, which has to run +// inside PHP because it depends on the live option value. It exists separately +// so the behaviour can be tested directly. +func retitleBoard(title, label string) string { + trimmed := strings.TrimRight(trailingLabel.ReplaceAllString(title, ""), " \t") + + suffix := "[" + label + "]" + + if trimmed == "" { + return suffix + } + + return trimmed + " " + suffix +} + +// trailingLabel matches a bracketed label at the end of a board title. Nested +// brackets are excluded so a title ending in "[a [b]]" is left alone rather +// than partly consumed. +var trailingLabel = regexp.MustCompile(`\s*\[[^\[\]]*\]\s*$`) diff --git a/cmd/xf/worktree_json_test.go b/cmd/xf/worktree_json_test.go new file mode 100644 index 0000000..5b9412e --- /dev/null +++ b/cmd/xf/worktree_json_test.go @@ -0,0 +1,75 @@ +package main + +import ( + "encoding/json" + "testing" + "time" + + "github.com/xenforo-ltd/cli/internal/worktree" +) + +// The JSON output reports whether the source environment was cloned, so +// automation can tell an installed worktree from an empty one. +func TestWorktreeOutputReportsTheCloneResult(t *testing.T) { + entry := worktree.Entry{ + SourcePath: "/src", + SourceBranch: "main", + WorktreePath: "/src.worktrees/feature", + Branch: "dev/feature", + Instance: "feature", + CreatedAt: time.Date(2026, 8, 19, 12, 0, 0, 0, time.UTC), + } + + // cloneEnvironment succeeding is what sets this, immediately before the + // output is built. + entry.Cloned = true + + data, err := json.Marshal(worktreeOutput{ + Path: entry.WorktreePath, + Branch: entry.Branch, + SourcePath: entry.SourcePath, + SourceBranch: entry.SourceBranch, + Instance: entry.Instance, + Cloned: entry.Cloned, + CreatedAt: entry.CreatedAt, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var decoded map[string]any + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + if decoded["cloned"] != true { + t.Errorf("cloned = %v, want true after a successful clone", decoded["cloned"]) + } +} + +func TestWorktreeOutputReportsAnUnclonedWorktree(t *testing.T) { + data, err := json.Marshal(worktreeOutput{ + Path: "/src.worktrees/feature", + Branch: "dev/feature", + Cloned: false, + }) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var decoded map[string]any + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + // The key must always be present, so consumers can branch on it without + // checking for existence first. + value, ok := decoded["cloned"] + if !ok { + t.Fatal("cloned key is missing") + } + + if value != false { + t.Errorf("cloned = %v, want false", value) + } +} diff --git a/cmd/xf/worktree_title_test.go b/cmd/xf/worktree_title_test.go new file mode 100644 index 0000000..3139ec5 --- /dev/null +++ b/cmd/xf/worktree_title_test.go @@ -0,0 +1,86 @@ +package main + +import "testing" + +func TestRetitleBoard(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + title string + label string + want string + }{ + { + name: "plain title gains a suffix", + title: "XenForo", + label: "slack-unfurl", + want: "XenForo [slack-unfurl]", + }, + { + name: "existing suffix is replaced", + title: "XenForo [main]", + label: "slack-unfurl", + want: "XenForo [slack-unfurl]", + }, + { + name: "version-style suffix is replaced", + title: "XenForo [2.4]", + label: "slack-unfurl", + want: "XenForo [slack-unfurl]", + }, + { + name: "brackets elsewhere are left alone", + title: "XenForo [beta] forums", + label: "feature", + want: "XenForo [beta] forums [feature]", + }, + { + name: "trailing whitespace is tidied", + title: "XenForo ", + label: "feature", + want: "XenForo [feature]", + }, + { + name: "empty title becomes just the label", + title: "", + label: "feature", + want: "[feature]", + }, + { + name: "empty suffix is replaced rather than kept", + title: "XenForo []", + label: "feature", + want: "XenForo [feature]", + }, + { + name: "nested brackets are not mangled", + title: "XenForo [a [b]]", + label: "feature", + want: "XenForo [a [b]] [feature]", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got := retitleBoard(tt.title, tt.label); got != tt.want { + t.Errorf("retitleBoard(%q, %q) = %q, want %q", tt.title, tt.label, got, tt.want) + } + }) + } +} + +// TestRetitleBoardIsIdempotent matters because cloning a clone must not +// accumulate suffixes. +func TestRetitleBoardIsIdempotent(t *testing.T) { + t.Parallel() + + once := retitleBoard("XenForo [main]", "feature") + twice := retitleBoard(once, "feature") + + if once != twice { + t.Errorf("applying twice changed the result: %q then %q", once, twice) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index db51c53..d0e7bfe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,6 +15,11 @@ var ( cacheOnce sync.Once cache Config errCache error + + // initMu guards Init, which configures viper's package-level singleton. + // Concurrent callers — notably parallel tests that each run the CLI — + // would otherwise race on that shared state. + initMu sync.Mutex ) // Config holds all CLI configuration values. @@ -69,6 +74,9 @@ func (cfg *OAuthConfig) Endpoints() *OAuthEndpoints { // Init sets up the configuration system and reads the config file if it exists. func Init(configFile string) error { + initMu.Lock() + defer initMu.Unlock() + if configFile != "" { viper.SetConfigFile(configFile) } else { @@ -111,6 +119,12 @@ func Init(configFile string) error { // Load reads the configuration from the config file. func Load() (Config, error) { cacheOnce.Do(func() { + // Unmarshal reads the same package-level viper instance that Init + // writes, so it takes the same lock: without it, a caller loading + // config while another initializes it races on that shared state. + initMu.Lock() + defer initMu.Unlock() + if err := viper.Unmarshal(&cache); err != nil { errCache = fmt.Errorf("failed to unmarshal config: %w", err) } diff --git a/internal/dockercompose/credentials_test.go b/internal/dockercompose/credentials_test.go new file mode 100644 index 0000000..d402a0f --- /dev/null +++ b/internal/dockercompose/credentials_test.go @@ -0,0 +1,109 @@ +package dockercompose + +import ( + "os" + "path/filepath" + "testing" +) + +// newRunnerWithEnv builds a Runner whose .env contains the given contents. +func newRunnerWithEnv(t *testing.T, env string) *Runner { + t.Helper() + + dir := t.TempDir() + + if err := os.MkdirAll(filepath.Join(dir, "src"), 0o750); err != nil { + t.Fatalf("mkdir src: %v", err) + } + + if err := os.WriteFile(filepath.Join(dir, "src", "XF.php"), []byte(" maxInstanceNameLength { + t.Errorf("instance name %q exceeds %d characters", result.Instance, maxInstanceNameLength) + } +} diff --git a/internal/worktree/git.go b/internal/worktree/git.go new file mode 100644 index 0000000..6cdae62 --- /dev/null +++ b/internal/worktree/git.go @@ -0,0 +1,136 @@ +package worktree + +import ( + "context" + "errors" + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +// ErrNotARepository indicates a path is not inside a git repository. +var ErrNotARepository = errors.New("not a git repository") + +// SourceCheckout returns the main checkout for the repository containing dir. +// +// When dir is inside a linked worktree this returns the *original* checkout, +// not the worktree. That keeps worktrees siblings of the source rather than +// nesting them, so running the command from within a worktree behaves the same +// as running it from the source. +func SourceCheckout(ctx context.Context, dir string) (string, error) { + // --git-common-dir points at the shared .git directory, which belongs to the + // main checkout even when called from a linked worktree. + out, err := gitOutput(ctx, dir, "rev-parse", "--git-common-dir") + if err != nil { + return "", fmt.Errorf("%w: %s", ErrNotARepository, dir) + } + + gitDir := out + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(dir, gitDir) + } + + // The checkout is the parent of its .git directory. + checkout := filepath.Dir(filepath.Clean(gitDir)) + + abs, err := filepath.Abs(checkout) + if err != nil { + return "", fmt.Errorf("failed to resolve checkout path: %w", err) + } + + return abs, nil +} + +// BranchExists reports whether a local branch of the given name exists. +func BranchExists(ctx context.Context, repoDir, branch string) (bool, error) { + ref := "refs/heads/" + branch + + cmd := exec.CommandContext(ctx, "git", "show-ref", "--verify", "--quiet", ref) + cmd.Dir = repoDir + + err := cmd.Run() + if err == nil { + return true, nil + } + + // show-ref exits 1 when the ref is absent, which is not an error here. + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return false, nil + } + + return false, fmt.Errorf("failed to check branch %q: %w", branch, err) +} + +// CurrentBranch returns the branch checked out in repoDir. +func CurrentBranch(ctx context.Context, repoDir string) (string, error) { + out, err := gitOutput(ctx, repoDir, "rev-parse", "--abbrev-ref", "HEAD") + if err != nil { + return "", fmt.Errorf("failed to determine current branch: %w", err) + } + + return out, nil +} + +// gitOutput runs git in dir and returns its trimmed standard output. +func gitOutput(ctx context.Context, dir string, args ...string) (string, error) { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + + out, err := cmd.Output() + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } + + return "", err + } + + return strings.TrimSpace(string(out)), nil +} + +// worktreeOwner returns the branch checked out at a worktree path, or an empty +// string when no worktree is registered there. +// +// This makes a collision actionable: the user is told which branch already owns +// the directory, rather than only that something does. +func worktreeOwner(ctx context.Context, repoDir, worktreePath string) (string, error) { + out, err := gitOutput(ctx, repoDir, "worktree", "list", "--porcelain") + if err != nil { + return "", fmt.Errorf("failed to list worktrees: %w", err) + } + + want, err := filepath.Abs(worktreePath) + if err != nil { + return "", fmt.Errorf("failed to resolve %s: %w", worktreePath, err) + } + + want = resolveSymlinks(want) + + var current string + + for _, line := range strings.Split(out, "\n") { + switch { + case strings.HasPrefix(line, "worktree "): + current = resolveSymlinks(strings.TrimPrefix(line, "worktree ")) + + case strings.HasPrefix(line, "branch ") && current == want: + // Reported as a full ref, e.g. refs/heads/dev/xfs/feature. + return strings.TrimPrefix(strings.TrimPrefix(line, "branch "), "refs/heads/"), nil + } + } + + return "", nil +} + +// resolveSymlinks resolves a path for comparison, falling back to the cleaned +// path when it cannot be resolved. Temporary directories on macOS are symlinked +// via /var, so comparing unresolved paths gives false mismatches. +func resolveSymlinks(path string) string { + if resolved, err := filepath.EvalSymlinks(path); err == nil { + return filepath.Clean(resolved) + } + + return filepath.Clean(path) +} diff --git a/internal/worktree/git_test.go b/internal/worktree/git_test.go new file mode 100644 index 0000000..7bece1c --- /dev/null +++ b/internal/worktree/git_test.go @@ -0,0 +1,159 @@ +package worktree + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// newTestRepo creates a git repository with one commit and returns its path. +func newTestRepo(t *testing.T) string { + t.Helper() + + dir := t.TempDir() + + run := func(args ...string) { + t.Helper() + + cmd := exec.CommandContext(t.Context(), "git", args...) + cmd.Dir = dir + + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + + run("init", "-q") + run("config", "user.email", "test@example.com") + run("config", "user.name", "Test") + + if err := os.WriteFile(filepath.Join(dir, "file.txt"), []byte("x"), 0o600); err != nil { + t.Fatalf("write file: %v", err) + } + + run("add", "-A") + run("commit", "-qm", "initial") + + return dir +} + +func TestSourceCheckoutFromRepoRoot(t *testing.T) { + repo := newTestRepo(t) + + got, err := SourceCheckout(t.Context(), repo) + if err != nil { + t.Fatalf("SourceCheckout: %v", err) + } + + if !samePath(t, got, repo) { + t.Errorf("SourceCheckout = %q, want %q", got, repo) + } +} + +func TestSourceCheckoutFromSubdirectory(t *testing.T) { + repo := newTestRepo(t) + + sub := filepath.Join(repo, "src", "nested") + if err := os.MkdirAll(sub, 0o750); err != nil { + t.Fatalf("mkdir: %v", err) + } + + got, err := SourceCheckout(t.Context(), sub) + if err != nil { + t.Fatalf("SourceCheckout: %v", err) + } + + if !samePath(t, got, repo) { + t.Errorf("SourceCheckout = %q, want the repo root %q", got, repo) + } +} + +// TestSourceCheckoutFromWorktreeReturnsMainCheckout is the important case: +// running the command from inside a worktree must anchor new worktrees to the +// original checkout, not nest them inside the current one. +func TestSourceCheckoutFromWorktreeReturnsMainCheckout(t *testing.T) { + repo := newTestRepo(t) + + wt := filepath.Join(t.TempDir(), "linked") + + cmd := exec.CommandContext(t.Context(), "git", "worktree", "add", "-q", wt, "-b", "linked-branch") + cmd.Dir = repo + + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("worktree add: %v\n%s", err, out) + } + + got, err := SourceCheckout(t.Context(), wt) + if err != nil { + t.Fatalf("SourceCheckout: %v", err) + } + + if !samePath(t, got, repo) { + t.Errorf("SourceCheckout from a worktree = %q, want the main checkout %q", got, repo) + } +} + +func TestSourceCheckoutOutsideRepository(t *testing.T) { + if _, err := SourceCheckout(t.Context(), t.TempDir()); err == nil { + t.Fatal("expected an error outside a git repository") + } +} + +func TestBranchExists(t *testing.T) { + repo := newTestRepo(t) + + exists, err := BranchExists(t.Context(), repo, "no-such-branch") + if err != nil { + t.Fatalf("BranchExists: %v", err) + } + + if exists { + t.Error("reported a non-existent branch as existing") + } + + cmd := exec.CommandContext(t.Context(), "git", "branch", "real-branch") + cmd.Dir = repo + + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git branch: %v\n%s", err, out) + } + + exists, err = BranchExists(t.Context(), repo, "real-branch") + if err != nil { + t.Fatalf("BranchExists: %v", err) + } + + if !exists { + t.Error("did not report an existing branch") + } +} + +func TestCurrentBranch(t *testing.T) { + repo := newTestRepo(t) + + got, err := CurrentBranch(t.Context(), repo) + if err != nil { + t.Fatalf("CurrentBranch: %v", err) + } + + if got != "main" && got != "master" { + t.Errorf("CurrentBranch = %q, want main or master", got) + } +} + +func samePath(t *testing.T, a, b string) bool { + t.Helper() + + ra, err := filepath.EvalSymlinks(a) + if err != nil { + ra = a + } + + rb, err := filepath.EvalSymlinks(b) + if err != nil { + rb = b + } + + return filepath.Clean(ra) == filepath.Clean(rb) +} diff --git a/internal/worktree/naming_test.go b/internal/worktree/naming_test.go new file mode 100644 index 0000000..a11a61b --- /dev/null +++ b/internal/worktree/naming_test.go @@ -0,0 +1,125 @@ +package worktree + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestBranchToDirNameUsesLastSegment(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + branch string + want string + }{ + {name: "conventional branch", branch: "dev/xfs/slack-unfurl", want: "slack-unfurl"}, + {name: "two segments", branch: "dev/feature", want: "feature"}, + {name: "single segment", branch: "feature", want: "feature"}, + {name: "trailing slash ignored", branch: "dev/feature/", want: "feature"}, + {name: "dots preserved", branch: "release/2.4.0", want: "2.4.0"}, + {name: "spaces become dashes", branch: "dev/my feature", want: "my-feature"}, + {name: "unsafe characters stripped", branch: "dev/feat:x*y", want: "feat-x-y"}, + {name: "traversal neutralised", branch: "../escape", want: "escape"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got := BranchToDirName(tt.branch); got != tt.want { + t.Errorf("BranchToDirName(%q) = %q, want %q", tt.branch, got, tt.want) + } + }) + } +} + +// TestBranchToDirNameCollides documents the trade-off that last-segment naming +// accepts: different branches can want the same directory. Preflight rejects +// the second one rather than silently renaming it. +func TestBranchToDirNameCollides(t *testing.T) { + t.Parallel() + + a := BranchToDirName("dev/xfs/slack-unfurl") + b := BranchToDirName("dev/xf/slack-unfurl") + + if a != b { + t.Fatalf("expected these to collide, got %q and %q", a, b) + } +} + +// TestPreflightRejectsCollisionWithExistingBranch is the safeguard: a second +// branch wanting an occupied directory must be refused, and the error must name +// the branch already using it so the fix is obvious. +func TestPreflightRejectsCollisionWithExistingBranch(t *testing.T) { + repo := newXenForoRepo(t) + + if _, err := Create(t.Context(), Options{ + SourcePath: repo, + Branch: "dev/xfs/slack-unfurl", + }); err != nil { + t.Fatalf("Create: %v", err) + } + + err := Preflight(t.Context(), repo, "dev/xf/slack-unfurl") + if err == nil { + t.Fatal("expected a colliding branch to be rejected") + } + + msg := err.Error() + + if !strings.Contains(msg, "slack-unfurl") { + t.Errorf("error %q does not identify the directory in conflict", msg) + } + + if !strings.Contains(msg, "dev/xfs/slack-unfurl") { + t.Errorf("error %q does not name the branch already using it", msg) + } +} + +// TestPreflightAllowsDistinctLastSegments confirms the common case still works. +func TestPreflightAllowsDistinctLastSegments(t *testing.T) { + repo := newXenForoRepo(t) + + if _, err := Create(t.Context(), Options{SourcePath: repo, Branch: "dev/xfs/one"}); err != nil { + t.Fatalf("Create: %v", err) + } + + if err := Preflight(t.Context(), repo, "dev/xfs/two"); err != nil { + t.Errorf("distinct feature names must not conflict: %v", err) + } +} + +// TestWorktreeOwnerReportsTheBranchUsingADirectory covers the lookup that makes +// the rejection message actionable. +func TestWorktreeOwnerReportsTheBranchUsingADirectory(t *testing.T) { + repo := newXenForoRepo(t) + + result, err := Create(t.Context(), Options{SourcePath: repo, Branch: "dev/xfs/slack-unfurl"}) + if err != nil { + t.Fatalf("Create: %v", err) + } + + owner, err := worktreeOwner(t.Context(), repo, result.Path) + if err != nil { + t.Fatalf("worktreeOwner: %v", err) + } + + if owner != "dev/xfs/slack-unfurl" { + t.Errorf("owner = %q, want the branch that owns the worktree", owner) + } +} + +func TestWorktreeOwnerForUnknownPath(t *testing.T) { + repo := newXenForoRepo(t) + + owner, err := worktreeOwner(t.Context(), repo, filepath.Join(t.TempDir(), "absent")) + if err != nil { + t.Fatalf("worktreeOwner: %v", err) + } + + if owner != "" { + t.Errorf("owner = %q, want empty for an unknown path", owner) + } +} diff --git a/internal/worktree/paths.go b/internal/worktree/paths.go new file mode 100644 index 0000000..02c71dc --- /dev/null +++ b/internal/worktree/paths.go @@ -0,0 +1,105 @@ +// Package worktree manages git worktrees for XenForo development environments. +package worktree + +import ( + "fmt" + "path/filepath" + "regexp" + "strings" +) + +// worktreesSuffix is appended to a checkout's directory name to form the +// directory that holds its worktrees. +const worktreesSuffix = ".worktrees" + +// unsafeChars matches anything not allowed in a worktree directory name. +// Letters, digits, dots, underscores and dashes are kept; everything else +// becomes a separator. +var unsafeChars = regexp.MustCompile(`[^a-zA-Z0-9._-]+`) + +// BranchToDirName converts a branch name into a single, safe path segment. +// +// Only the last segment is used, so dev/xfs/slack-unfurl becomes slack-unfurl. +// The prefix in a conventional branch name describes where the work belongs +// rather than what it is, and repeating it in directory names, URLs and Docker +// instance names makes all three harder to read. +// +// The result is always a single segment: it can never contain a separator or +// resolve to a parent directory, whatever the branch name contains. +// +// This is deliberately lossy. dev/xfs/slack-unfurl and dev/xf/slack-unfurl both +// yield slack-unfurl, so callers must check whether the directory is already +// taken. Preflight rejects a collision rather than renaming around it, which +// keeps the branch-to-path mapping computable without consulting any state. +func BranchToDirName(branch string) string { + // Take the last non-empty segment, so a trailing slash does not produce an + // empty name. + segments := strings.Split(branch, "/") + + last := "" + + for i := len(segments) - 1; i >= 0; i-- { + if strings.TrimSpace(segments[i]) != "" { + last = segments[i] + + break + } + } + + if last == "" { + last = branch + } + + name := unsafeChars.ReplaceAllString(last, "-") + + // Leading dots would create a hidden directory, and a name of only dots + // would resolve to "." or "..". + name = strings.Trim(name, "-") + name = strings.TrimLeft(name, ".") + name = strings.Trim(name, "-") + + // Collapse runs introduced by the substitutions above. + for strings.Contains(name, "--") { + name = strings.ReplaceAll(name, "--", "-") + } + + if name == "." || name == ".." { + return "" + } + + return name +} + +// WorktreesDir returns the directory holding the worktrees for a checkout. +// +// Worktrees are siblings of the source checkout, so ~/Sites/main yields +// ~/Sites/main.worktrees. This keeps them on the same filesystem as the source, +// which matters for Docker bind mounts, and makes them discoverable without +// knowing an xf-specific convention. +func WorktreesDir(sourcePath string) string { + cleaned := filepath.Clean(sourcePath) + + return cleaned + worktreesSuffix +} + +// ResolvePath returns the worktree path for a branch of the given checkout. +// +// The result depends only on its arguments, so any tool can predict it without +// consulting the registry or git. +func ResolvePath(sourcePath, branch string) string { + return filepath.Join(WorktreesDir(sourcePath), BranchToDirName(branch)) +} + +// ResolveExistingPath is ResolvePath for branch names that came from the user. +// +// BranchToDirName yields an empty name for inputs such as "." or "..", which +// would resolve to the directory holding every worktree for the checkout. A +// command acting on that path would operate on all of them at once, so those +// inputs are rejected rather than resolved. +func ResolveExistingPath(sourcePath, branch string) (string, error) { + if BranchToDirName(branch) == "" { + return "", fmt.Errorf("%w: %q does not name a worktree", ErrInvalidBranch, branch) + } + + return ResolvePath(sourcePath, branch), nil +} diff --git a/internal/worktree/paths_test.go b/internal/worktree/paths_test.go new file mode 100644 index 0000000..751b31b --- /dev/null +++ b/internal/worktree/paths_test.go @@ -0,0 +1,102 @@ +package worktree + +import ( + "path/filepath" + "testing" +) + +func TestBranchToDirName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + branch string + want string + }{ + {name: "simple", branch: "feature", want: "feature"}, + {name: "last segment used", branch: "dev/24x/feature", want: "feature"}, + {name: "leading slash trimmed", branch: "/leading", want: "leading"}, + {name: "trailing slash trimmed", branch: "trailing/", want: "trailing"}, + {name: "consecutive slashes collapse", branch: "a//b", want: "b"}, + {name: "spaces become dashes", branch: "my feature", want: "my-feature"}, + {name: "uppercase preserved", branch: "dev/MyAddon/Fix", want: "Fix"}, + {name: "dots preserved", branch: "release/2.4.0", want: "2.4.0"}, + {name: "path traversal neutralised", branch: "../escape", want: "escape"}, + {name: "unsafe characters stripped", branch: "feat:x*y?", want: "feat-x-y"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got := BranchToDirName(tt.branch); got != tt.want { + t.Errorf("BranchToDirName(%q) = %q, want %q", tt.branch, got, tt.want) + } + }) + } +} + +// TestBranchToDirNameNeverEscapes is the security-relevant property: whatever +// the branch name, the result must be a single path segment that cannot climb +// out of the worktrees directory. +func TestBranchToDirNameNeverEscapes(t *testing.T) { + t.Parallel() + + for _, branch := range []string{ + "../../etc/passwd", + "..", + ".", + "/absolute/path", + "a/../../b", + "....//", + } { + got := BranchToDirName(branch) + + if got == "" { + continue // rejected outright, which is also safe + } + + if filepath.Base(got) != got { + t.Errorf("BranchToDirName(%q) = %q, which is not a single path segment", branch, got) + } + + if got == ".." || got == "." { + t.Errorf("BranchToDirName(%q) = %q, which escapes or self-references", branch, got) + } + } +} + +func TestWorktreesDir(t *testing.T) { + t.Parallel() + + got := WorktreesDir("/Users/x/Sites/main") + want := filepath.Join("/Users/x/Sites", "main.worktrees") + + if got != want { + t.Errorf("WorktreesDir = %q, want %q", got, want) + } +} + +func TestResolvePath(t *testing.T) { + t.Parallel() + + got := ResolvePath("/Users/x/Sites/main", "dev/24x/feature") + want := filepath.Join("/Users/x/Sites", "main.worktrees", "feature") + + if got != want { + t.Errorf("ResolvePath = %q, want %q", got, want) + } +} + +// TestResolvePathIsDeterministic covers the promise that the path can be +// predicted without consulting any state. +func TestResolvePathIsDeterministic(t *testing.T) { + t.Parallel() + + a := ResolvePath("/Users/x/Sites/main", "dev/24x/feature") + b := ResolvePath("/Users/x/Sites/main/", "dev/24x/feature") + + if a != b { + t.Errorf("a trailing separator changed the result: %q vs %q", a, b) + } +} diff --git a/internal/worktree/registry.go b/internal/worktree/registry.go new file mode 100644 index 0000000..8c02ae1 --- /dev/null +++ b/internal/worktree/registry.go @@ -0,0 +1,342 @@ +package worktree + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "sync" + "time" +) + +// lockRetryInterval and lockTimeout bound how long a mutating call waits for +// another process to release the registry lock, so a crashed process cannot +// wedge every other xf invocation forever. +const ( + lockRetryInterval = 25 * time.Millisecond + lockTimeout = 5 * time.Second + lockStaleAfter = 30 * time.Second +) + +// ErrRegistryCorrupt indicates the registry file exists but cannot be parsed. +// +// Mutating calls tolerate this and rebuild from an empty registry, so a damaged +// file never blocks cleanup. Read failures are not tolerated: they mean the +// existing entries are unknown rather than absent. +var ErrRegistryCorrupt = errors.New("worktree registry is corrupt") + +// Entry records a worktree created by xf. +type Entry struct { + // SourcePath is the checkout the worktree was created from. + SourcePath string `json:"source_path"` + + // SourceBranch is the branch the source was on at creation time. + SourceBranch string `json:"source_branch"` + + // WorktreePath is the resolved location of the worktree. + WorktreePath string `json:"worktree_path"` + + // Branch is the branch checked out in the worktree. + Branch string `json:"branch"` + + // Instance is the Docker instance name for the worktree. + Instance string `json:"instance"` + + // Cloned records whether the environment was cloned from the source. + Cloned bool `json:"cloned"` + + // CreatedAt is when the worktree was created. + CreatedAt time.Time `json:"created_at"` +} + +// Registry is the on-disk record of worktrees xf has created. +// +// It exists so that worktrees can be listed across projects, which git alone +// cannot do. It is deliberately *not* the source of truth: git and Docker are. +// Worktrees can be removed behind xf's back, so callers must reconcile entries +// against reality rather than trusting the file. A missing or damaged registry +// degrades listing across projects but never blocks an operation. +type Registry struct { + mu sync.Mutex + path string +} + +// NewRegistry opens the registry in the user's configuration directory. +func NewRegistry() (*Registry, error) { + dir, err := os.UserConfigDir() + if err != nil { + return nil, fmt.Errorf("could not determine user config directory: %w", err) + } + + return &Registry{path: filepath.Join(dir, "xf", "worktrees.json")}, nil +} + +// Path returns the registry file location. +func (r *Registry) Path() string { + return r.path +} + +// All returns every recorded entry. +// +// A missing registry returns no entries and no error. +func (r *Registry) All() ([]Entry, error) { + r.mu.Lock() + defer r.mu.Unlock() + + return r.load() +} + +// ForSource returns the entries belonging to a single checkout. +func (r *Registry) ForSource(sourcePath string) ([]Entry, error) { + entries, err := r.All() + if err != nil { + return nil, err + } + + want := filepath.Clean(sourcePath) + + var matched []Entry + + for _, e := range entries { + if filepath.Clean(e.SourcePath) == want { + matched = append(matched, e) + } + } + + return matched, nil +} + +// Add records a worktree, replacing any existing entry for the same path. +func (r *Registry) Add(entry Entry) error { + r.mu.Lock() + defer r.mu.Unlock() + + unlock, err := r.lock() + if err != nil { + return err + } + defer unlock() + + // A damaged registry must not block recording new work, so parse failures + // are treated as an empty registry and overwritten. A read failure is + // different: the entries are unreadable rather than absent, and saving + // over them would discard every other worktree's record. + entries, err := r.load() + if err != nil && !errors.Is(err, ErrRegistryCorrupt) { + return err + } + + want := filepath.Clean(entry.WorktreePath) + replaced := false + + for i, e := range entries { + if filepath.Clean(e.WorktreePath) == want { + entries[i] = entry + replaced = true + + break + } + } + + if !replaced { + entries = append(entries, entry) + } + + return r.save(entries) +} + +// Remove drops the entry for a worktree path. Removing an absent entry is not +// an error, so cleanup is idempotent. +func (r *Registry) Remove(worktreePath string) error { + r.mu.Lock() + defer r.mu.Unlock() + + unlock, err := r.lock() + if err != nil { + return err + } + defer unlock() + + // A damaged registry must not block cleanup: worktree removal and prune + // have to be able to proceed even when the file cannot be parsed, so a + // parse failure is treated the same as an empty registry, as in Add. + // + // Only a parse failure. A permission or I/O error means the existing + // entries could not be read at all, and saving over them would delete + // every other worktree's record. + entries, err := r.load() + if err != nil && !errors.Is(err, ErrRegistryCorrupt) { + return err + } + + want := filepath.Clean(worktreePath) + kept := make([]Entry, 0, len(entries)) + + for _, e := range entries { + if filepath.Clean(e.WorktreePath) != want { + kept = append(kept, e) + } + } + + return r.save(kept) +} + +// lockPath returns the path of the cross-process lockfile guarding r.path. +func (r *Registry) lockPath() string { + return r.path + ".lock" +} + +// lock acquires a cross-process lock covering a load/modify/save transaction +// and returns a function that releases it. +// +// r.mu only guards one process's own goroutines; separate "xf worktree" +// invocations are separate processes that would otherwise read, modify and +// write the same JSON file with no coordination, silently losing whichever +// write happened first. A plain O_CREATE|O_EXCL lockfile is used rather than +// syscall.Flock so the same code works on Windows, where xf also builds. +func (r *Registry) lock() (func(), error) { + dir := filepath.Dir(r.path) + if err := os.MkdirAll(dir, 0o750); err != nil { + return nil, fmt.Errorf("failed to create registry directory: %w", err) + } + + path := r.lockPath() + deadline := time.Now().Add(lockTimeout) + + for { + f, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600) + if err == nil { + token := fmt.Sprintf("%d.%d", os.Getpid(), time.Now().UnixNano()) + + _, _ = io.WriteString(f, token) + _ = f.Close() + + return func() { + // Released by renaming to a private name and deleting that, + // rather than reading the token and then removing the path: + // between those two steps the lock could be taken over as + // stale and recreated by another process, and the remove would + // delete a lock that is now theirs. + // + // Rename is atomic, so at most one process moves this file. If + // the content is not ours, it was taken over and is put back + // untouched. + release := fmt.Sprintf("%s.release.%d", path, os.Getpid()) + if err := os.Rename(path, release); err != nil { + return + } + + held, readErr := os.ReadFile(release) + if readErr == nil && string(held) == token { + _ = os.Remove(release) + + return + } + + // Someone else's lock: restore it. + _ = os.Rename(release, path) + }, nil + } + + if !os.IsExist(err) { + return nil, fmt.Errorf("failed to lock worktree registry: %w", err) + } + + // A lockfile left behind by a process that crashed before releasing it + // would otherwise wedge every future call, so a lock older than + // lockStaleAfter is treated as abandoned and cleared. + // + // The takeover renames rather than removes: rename is atomic, so of + // several processes that all see the same stale lock, only the one + // whose rename succeeds clears it. Removing directly is a + // check-then-act race in which two processes can each delete the + // other's fresh lock and both believe they hold it. + // + // A failed takeover falls through to the deadline check and the sleep + // rather than retrying immediately: a lockfile that cannot be removed, + // on a read-only parent for instance, would otherwise spin forever. + if info, statErr := os.Stat(path); statErr == nil && time.Since(info.ModTime()) > lockStaleAfter { + stale := fmt.Sprintf("%s.stale.%d", path, os.Getpid()) + if renameErr := os.Rename(path, stale); renameErr == nil { + _ = os.Remove(stale) + + continue + } + } + + if time.Now().After(deadline) { + return nil, fmt.Errorf("timed out waiting for worktree registry lock at %s", path) + } + + time.Sleep(lockRetryInterval) + } +} + +func (r *Registry) load() ([]Entry, error) { + data, err := os.ReadFile(r.path) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + + return nil, fmt.Errorf("failed to read worktree registry: %w", err) + } + + var entries []Entry + if err := json.Unmarshal(data, &entries); err != nil { + return nil, fmt.Errorf("%w at %s: %w", ErrRegistryCorrupt, r.path, err) + } + + return entries, nil +} + +// save writes the registry atomically, so a crash or a concurrent run cannot +// leave a half-written file. +func (r *Registry) save(entries []Entry) error { + if entries == nil { + entries = []Entry{} + } + + data, err := json.MarshalIndent(entries, "", " ") + if err != nil { + return fmt.Errorf("failed to encode worktree registry: %w", err) + } + + dir := filepath.Dir(r.path) + if err := os.MkdirAll(dir, 0o750); err != nil { + return fmt.Errorf("failed to create registry directory: %w", err) + } + + tmp, err := os.CreateTemp(dir, "worktrees-*.json") + if err != nil { + return fmt.Errorf("failed to create temporary registry file: %w", err) + } + + tmpName := tmp.Name() + + defer func() { + _ = os.Remove(tmpName) + }() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + + return fmt.Errorf("failed to write worktree registry: %w", err) + } + + if err := tmp.Close(); err != nil { + return fmt.Errorf("failed to close worktree registry: %w", err) + } + + if err := os.Chmod(tmpName, 0o600); err != nil { + return fmt.Errorf("failed to set registry permissions: %w", err) + } + + if err := os.Rename(tmpName, r.path); err != nil { + return fmt.Errorf("failed to replace worktree registry: %w", err) + } + + return nil +} diff --git a/internal/worktree/registry_test.go b/internal/worktree/registry_test.go new file mode 100644 index 0000000..1de7f08 --- /dev/null +++ b/internal/worktree/registry_test.go @@ -0,0 +1,276 @@ +package worktree + +import ( + "fmt" + "os" + "path/filepath" + "runtime" + "sync" + "testing" + "time" +) + +func TestRegistryRoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "worktrees.json") + + reg := &Registry{path: path} + + entry := Entry{ + SourcePath: "/Users/x/Sites/main", + SourceBranch: "main", + WorktreePath: "/Users/x/Sites/main.worktrees/dev-24x-feature", + Branch: "dev/24x/feature", + Instance: "dev-24x-feature", + Cloned: true, + CreatedAt: time.Now().UTC().Truncate(time.Second), + } + + if err := reg.Add(entry); err != nil { + t.Fatalf("Add: %v", err) + } + + entries, err := reg.All() + if err != nil { + t.Fatalf("All: %v", err) + } + + if len(entries) != 1 { + t.Fatalf("got %d entries, want 1", len(entries)) + } + + got := entries[0] + if got.Branch != entry.Branch || got.Instance != entry.Instance || !got.Cloned { + t.Errorf("round-trip mismatch: %+v", got) + } +} + +// TestRegistryMissingFileIsNotAnError covers the design rule that the registry +// is never load-bearing: a missing file yields no entries, not a failure. +func TestRegistryMissingFileIsNotAnError(t *testing.T) { + reg := &Registry{path: filepath.Join(t.TempDir(), "absent.json")} + + entries, err := reg.All() + if err != nil { + t.Fatalf("a missing registry must not be an error, got %v", err) + } + + if len(entries) != 0 { + t.Errorf("got %d entries, want 0", len(entries)) + } +} + +// TestRegistryCorruptFileIsNotFatal covers the same rule for damaged content. +func TestRegistryCorruptFileIsNotFatal(t *testing.T) { + path := filepath.Join(t.TempDir(), "corrupt.json") + if err := os.WriteFile(path, []byte("{not json"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + reg := &Registry{path: path} + + if _, err := reg.All(); err == nil { + t.Log("corrupt registry returned no error; acceptable if entries are empty") + } + + // Adding must still succeed, replacing the damaged file. + if err := reg.Add(Entry{Branch: "x", WorktreePath: "/tmp/x"}); err != nil { + t.Fatalf("Add over a corrupt registry: %v", err) + } + + entries, err := reg.All() + if err != nil { + t.Fatalf("All after recovery: %v", err) + } + + if len(entries) != 1 { + t.Errorf("got %d entries, want 1 after recovery", len(entries)) + } +} + +// TestRegistryRemoveOverCorruptFileIsNotFatal covers the same tolerance as +// TestRegistryCorruptFileIsNotFatal, but for Remove: cleanup during +// "xf worktree remove" or prune must not fail just because the registry is +// unreadable. +func TestRegistryRemoveOverCorruptFileIsNotFatal(t *testing.T) { + path := filepath.Join(t.TempDir(), "corrupt.json") + if err := os.WriteFile(path, []byte("{not json"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + reg := &Registry{path: path} + + if err := reg.Remove("/tmp/anything"); err != nil { + t.Fatalf("Remove over a corrupt registry: %v", err) + } +} + +func TestRegistryRemove(t *testing.T) { + reg := &Registry{path: filepath.Join(t.TempDir(), "worktrees.json")} + + for _, p := range []string{"/tmp/a", "/tmp/b"} { + if err := reg.Add(Entry{WorktreePath: p, Branch: filepath.Base(p)}); err != nil { + t.Fatalf("Add: %v", err) + } + } + + if err := reg.Remove("/tmp/a"); err != nil { + t.Fatalf("Remove: %v", err) + } + + entries, err := reg.All() + if err != nil { + t.Fatalf("All: %v", err) + } + + if len(entries) != 1 || entries[0].WorktreePath != "/tmp/b" { + t.Errorf("unexpected entries after removal: %+v", entries) + } +} + +// TestRegistryAddIsIdempotent ensures re-registering the same path updates the +// entry rather than duplicating it. +func TestRegistryAddIsIdempotent(t *testing.T) { + reg := &Registry{path: filepath.Join(t.TempDir(), "worktrees.json")} + + first := Entry{WorktreePath: "/tmp/a", Branch: "old", Instance: "one"} + second := Entry{WorktreePath: "/tmp/a", Branch: "new", Instance: "two"} + + if err := reg.Add(first); err != nil { + t.Fatalf("Add: %v", err) + } + + if err := reg.Add(second); err != nil { + t.Fatalf("Add again: %v", err) + } + + entries, err := reg.All() + if err != nil { + t.Fatalf("All: %v", err) + } + + if len(entries) != 1 { + t.Fatalf("got %d entries, want 1", len(entries)) + } + + if entries[0].Branch != "new" || entries[0].Instance != "two" { + t.Errorf("entry was not updated: %+v", entries[0]) + } +} + +// TestRegistryAddSurvivesConcurrentProcesses exercises the cross-process lock: +// separate Registry values sharing one file stand in for separate "xf +// worktree" invocations, which previously could read-modify-write the same +// JSON concurrently and lose each other's entries. +func TestRegistryAddSurvivesConcurrentProcesses(t *testing.T) { + path := filepath.Join(t.TempDir(), "worktrees.json") + + const writers = 8 + + var wg sync.WaitGroup + + for i := range writers { + wg.Add(1) + + go func(i int) { + defer wg.Done() + + reg := &Registry{path: path} + entry := Entry{WorktreePath: fmt.Sprintf("/tmp/wt-%d", i), Branch: fmt.Sprintf("b%d", i)} + + if err := reg.Add(entry); err != nil { + t.Errorf("Add from writer %d: %v", i, err) + } + }(i) + } + + wg.Wait() + + reg := &Registry{path: path} + + entries, err := reg.All() + if err != nil { + t.Fatalf("All: %v", err) + } + + if len(entries) != writers { + t.Errorf("got %d entries, want %d: entries were lost to a race", len(entries), writers) + } +} + +func TestRegistryForSource(t *testing.T) { + reg := &Registry{path: filepath.Join(t.TempDir(), "worktrees.json")} + + add := func(source, branch string) { + t.Helper() + + if err := reg.Add(Entry{ + SourcePath: source, + Branch: branch, + WorktreePath: filepath.Join(source+".worktrees", branch), + }); err != nil { + t.Fatalf("Add: %v", err) + } + } + + add("/Users/x/Sites/main", "one") + add("/Users/x/Sites/main", "two") + add("/Users/x/Sites/other", "three") + + entries, err := reg.ForSource("/Users/x/Sites/main") + if err != nil { + t.Fatalf("ForSource: %v", err) + } + + if len(entries) != 2 { + t.Errorf("got %d entries for the source, want 2", len(entries)) + } +} + +// A registry that cannot be read is not an empty registry. Treating it as one +// would let the following save discard every entry it failed to read. +func TestRegistryDoesNotDiscardEntriesItCannotRead(t *testing.T) { + if runtime.GOOS == windowsOS { + t.Skip("unreadable-file permissions are not enforced the same way on Windows") + } + + if os.Geteuid() == 0 { + t.Skip("root bypasses file permissions") + } + + dir := t.TempDir() + path := filepath.Join(dir, "worktrees.json") + + reg := &Registry{path: path} + + if err := reg.Add(Entry{WorktreePath: "/src.worktrees/keep", Branch: "keep"}); err != nil { + t.Fatalf("seed Add: %v", err) + } + + if err := os.Chmod(path, 0o000); err != nil { + t.Fatalf("chmod: %v", err) + } + + t.Cleanup(func() { _ = os.Chmod(path, 0o600) }) + + if err := reg.Add(Entry{WorktreePath: "/src.worktrees/new", Branch: "new"}); err == nil { + t.Error("Add over an unreadable registry should fail rather than overwrite it") + } + + if err := reg.Remove("/src.worktrees/keep"); err == nil { + t.Error("Remove over an unreadable registry should fail rather than overwrite it") + } + + // The original entry must still be there once the file is readable again. + if err := os.Chmod(path, 0o600); err != nil { + t.Fatalf("chmod back: %v", err) + } + + entries, err := reg.All() + if err != nil { + t.Fatalf("All: %v", err) + } + + if len(entries) != 1 || entries[0].Branch != "keep" { + t.Errorf("entries = %+v, want the seeded entry preserved", entries) + } +} diff --git a/internal/worktree/remove.go b/internal/worktree/remove.go new file mode 100644 index 0000000..3f606b3 --- /dev/null +++ b/internal/worktree/remove.go @@ -0,0 +1,157 @@ +package worktree + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" +) + +var ( + // ErrDirtyWorktree indicates uncommitted changes would be lost. + ErrDirtyWorktree = errors.New("worktree has uncommitted changes") + + // ErrUnmergedCommits indicates commits exist only in this worktree. + ErrUnmergedCommits = errors.New("worktree has commits not present on any remote") +) + +// WorktreeStatus describes the state of a worktree's working tree and branch. +type WorktreeStatus struct { + // Modified lists paths with uncommitted changes, including untracked files. + Modified []string + + // UnmergedCommits lists commits not reachable from any remote branch. + UnmergedCommits []string +} + +// Clean reports whether the worktree holds no work that removal would lose. +func (s WorktreeStatus) Clean() bool { + return len(s.Modified) == 0 && len(s.UnmergedCommits) == 0 +} + +// Status inspects a worktree for work that would be lost by removing it. +func Status(ctx context.Context, worktreePath string) (WorktreeStatus, error) { + var status WorktreeStatus + + // --porcelain includes untracked files, which are easy to forget and just + // as easy to lose. + out, err := gitOutput(ctx, worktreePath, "status", "--porcelain") + if err != nil { + return status, fmt.Errorf("failed to inspect worktree: %w", err) + } + + for _, line := range strings.Split(out, "\n") { + if trimmed := strings.TrimSpace(line); trimmed != "" { + status.Modified = append(status.Modified, trimmed) + } + } + + // "git remote" with no configured remotes exits 0 with empty output, so a + // failure here is a real inspection problem, not the "no remotes" case. + remotes, err := gitOutput(ctx, worktreePath, "remote") + if err != nil { + return status, fmt.Errorf("failed to list remotes: %w", err) + } + + // "Unpushed" is only meaningful when there is somewhere to push to. In a + // repository with no remotes every commit is unreachable from a remote, so + // the check would flag every worktree and be worse than useless. + if strings.TrimSpace(remotes) == "" { + return status, nil + } + + // An unborn branch (no commits yet) has no HEAD to inspect, and "git log" + // on one fails distinctly from a real command failure, so check for that + // case explicitly rather than treating every "log" error as "no commits". + if _, err := gitOutput(ctx, worktreePath, "rev-parse", "--verify", "HEAD"); err != nil { + return status, nil + } + + // Commits reachable from HEAD but from no remote branch exist only here. + // HEAD must be named explicitly: "--not --remotes" alone gives git no + // starting point and silently lists nothing. + commits, err := gitOutput(ctx, worktreePath, "log", "--oneline", "HEAD", "--not", "--remotes") + if err != nil { + return status, fmt.Errorf("failed to inspect commit history: %w", err) + } + + for _, line := range strings.Split(commits, "\n") { + if trimmed := strings.TrimSpace(line); trimmed != "" { + status.UnmergedCommits = append(status.UnmergedCommits, trimmed) + } + } + + return status, nil +} + +// CheckRemovable reports whether a worktree can be removed without losing work. +// +// It is separate from Remove so callers can run the check before destroying +// anything the removal depends on. Tearing down containers and volumes first +// and only then discovering that the worktree is dirty would refuse the +// removal after the data it was protecting had already been deleted. +func CheckRemovable(ctx context.Context, worktreePath string) error { + status, err := Status(ctx, worktreePath) + if err != nil { + return err + } + + if len(status.Modified) > 0 { + return fmt.Errorf("%w:\n %s", ErrDirtyWorktree, strings.Join(status.Modified, "\n ")) + } + + if len(status.UnmergedCommits) > 0 { + return fmt.Errorf("%w:\n %s", ErrUnmergedCommits, strings.Join(status.UnmergedCommits, "\n ")) + } + + return nil +} + +// Remove deletes a worktree and its branch. +// +// Unless force is set, it refuses when the worktree holds uncommitted changes +// or commits that exist nowhere else, listing what would be lost. Removing +// containers and volumes is the caller's responsibility. +func Remove(ctx context.Context, sourcePath, worktreePath string, force bool) error { + if !force { + if err := CheckRemovable(ctx, worktreePath); err != nil { + return err + } + } + + branch, err := CurrentBranch(ctx, worktreePath) + if err != nil { + branch = "" + } + + args := []string{"worktree", "remove", worktreePath} + if force { + args = append(args, "--force") + } + + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = sourcePath + + if out, err := cmd.CombinedOutput(); err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + + return fmt.Errorf("failed to remove worktree: %w: %s", err, strings.TrimSpace(string(out))) + } + + if branch == "" || branch == "HEAD" { + return nil + } + + // Deleting the branch is best effort: the worktree is already gone, and a + // branch that will not delete is not worth failing the whole operation for. + deleteArgs := []string{"branch", "-D", branch} + + del := exec.CommandContext(ctx, "git", deleteArgs...) + del.Dir = sourcePath + _ = del.Run() + + return nil +} diff --git a/internal/worktree/remove_test.go b/internal/worktree/remove_test.go new file mode 100644 index 0000000..29d6723 --- /dev/null +++ b/internal/worktree/remove_test.go @@ -0,0 +1,177 @@ +package worktree + +import ( + "errors" + "os" + "os/exec" + "path/filepath" + "testing" +) + +// createdWorktree makes a worktree and returns the source and worktree paths. +func createdWorktree(t *testing.T, branch string) (string, string) { + t.Helper() + + repo := newXenForoRepo(t) + + result, err := Create(t.Context(), Options{SourcePath: repo, Branch: branch}) + if err != nil { + t.Fatalf("Create: %v", err) + } + + return repo, result.Path +} + +func TestRemoveDeletesACleanWorktree(t *testing.T) { + repo, wt := createdWorktree(t, "feature") + + if err := Remove(t.Context(), repo, wt, false); err != nil { + t.Fatalf("Remove: %v", err) + } + + if _, err := os.Stat(wt); !os.IsNotExist(err) { + t.Error("worktree directory still exists") + } + + exists, err := BranchExists(t.Context(), repo, "feature") + if err != nil { + t.Fatalf("BranchExists: %v", err) + } + + if exists { + t.Error("branch was left behind") + } +} + +// TestRemoveRefusesUncommittedChanges is the guard against losing work. +func TestRemoveRefusesUncommittedChanges(t *testing.T) { + repo, wt := createdWorktree(t, "feature") + + if err := os.WriteFile(filepath.Join(wt, "new-file.txt"), []byte("work"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + err := Remove(t.Context(), repo, wt, false) + if !errors.Is(err, ErrDirtyWorktree) { + t.Fatalf("expected ErrDirtyWorktree, got %v", err) + } + + if _, statErr := os.Stat(wt); statErr != nil { + t.Error("a refused removal must leave the worktree intact") + } +} + +func TestRemoveForceDiscardsChanges(t *testing.T) { + repo, wt := createdWorktree(t, "feature") + + if err := os.WriteFile(filepath.Join(wt, "new-file.txt"), []byte("work"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + if err := Remove(t.Context(), repo, wt, true); err != nil { + t.Fatalf("forced Remove: %v", err) + } + + if _, err := os.Stat(wt); !os.IsNotExist(err) { + t.Error("forced removal did not delete the worktree") + } +} + +// TestRemoveRefusesUnpushedCommits guards commits that exist nowhere else. +// +// The repository needs a remote for this to be meaningful: with no remote there +// is nowhere to push, so "unpushed" would describe every commit ever made. +func TestRemoveRefusesUnpushedCommits(t *testing.T) { + repo, wt := createdWorktree(t, "feature") + + addRemote(t, repo) + + if err := os.WriteFile(filepath.Join(wt, "committed.txt"), []byte("work"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + for _, args := range [][]string{{"add", "-A"}, {"commit", "-qm", "local work"}} { + cmd := exec.CommandContext(t.Context(), "git", args...) + cmd.Dir = wt + + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + + err := Remove(t.Context(), repo, wt, false) + if !errors.Is(err, ErrUnmergedCommits) { + t.Fatalf("expected ErrUnmergedCommits, got %v", err) + } +} + +func TestRemoveUnknownPath(t *testing.T) { + repo := newXenForoRepo(t) + + err := Remove(t.Context(), repo, filepath.Join(t.TempDir(), "nope"), false) + if err == nil { + t.Fatal("expected an error for an unknown worktree path") + } +} + +func TestStatusReportsCleanliness(t *testing.T) { + _, wt := createdWorktree(t, "feature") + + status, err := Status(t.Context(), wt) + if err != nil { + t.Fatalf("Status: %v", err) + } + + if !status.Clean() { + t.Errorf("a fresh worktree should be clean, got %+v", status) + } + + if err := os.WriteFile(filepath.Join(wt, "dirty.txt"), []byte("x"), 0o600); err != nil { + t.Fatalf("write: %v", err) + } + + status, err = Status(t.Context(), wt) + if err != nil { + t.Fatalf("Status: %v", err) + } + + if status.Clean() { + t.Error("an untracked file should make the worktree dirty") + } +} + +// TestStatusReturnsErrorOnInspectionFailure guards against treating a broken +// git invocation as "nothing to lose": Status must surface a real command +// failure rather than silently reporting a clean worktree, since Remove would +// otherwise delete the worktree and force-delete its branch unverified. +func TestStatusReturnsErrorOnInspectionFailure(t *testing.T) { + notARepo := t.TempDir() + + if _, err := Status(t.Context(), notARepo); err == nil { + t.Fatal("expected an error when inspecting a path that is not a git repository") + } +} + +// addRemote gives a repository a real remote with the current history, so that +// "not present on any remote" can distinguish new commits from existing ones. +func addRemote(t *testing.T, repo string) { + t.Helper() + + remote := t.TempDir() + + run := func(dir string, args ...string) { + t.Helper() + + cmd := exec.CommandContext(t.Context(), "git", args...) + cmd.Dir = dir + + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + + run(remote, "init", "-q", "--bare") + run(repo, "remote", "add", "origin", remote) + run(repo, "push", "-q", "origin", "HEAD") + run(repo, "fetch", "-q", "origin") +} diff --git a/internal/worktree/umask_other_test.go b/internal/worktree/umask_other_test.go new file mode 100644 index 0000000..050ce81 --- /dev/null +++ b/internal/worktree/umask_other_test.go @@ -0,0 +1,7 @@ +//go:build !(aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || (js && wasm)) + +package worktree + +// setUmask is a no-op on platforms without a umask. Tests that depend on mode +// bits skip themselves before calling it. +func setUmask(int) int { return 0 } diff --git a/internal/worktree/umask_unix_test.go b/internal/worktree/umask_unix_test.go new file mode 100644 index 0000000..35a94a7 --- /dev/null +++ b/internal/worktree/umask_unix_test.go @@ -0,0 +1,14 @@ +//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || (js && wasm) + +package worktree + +import "syscall" + +// setUmask sets the process umask and returns the previous value. +// +// The build constraint lists the platforms that provide syscall.Umask rather +// than using !windows: Plan 9 lacks it, so excluding only Windows would still +// fail to compile there. +func setUmask(mask int) int { + return syscall.Umask(mask) +}