diff --git a/.claude/rules/ci.md b/.claude/rules/ci.md index 15ab2f5..3854b3f 100644 --- a/.claude/rules/ci.md +++ b/.claude/rules/ci.md @@ -27,4 +27,4 @@ CI config: `.github/workflows/ci.yml` - Tests run on Windows — `syscall.Flock` is Unix-only - File-locking code (`state/`, `registry/`) needs build tags or conditional compilation if tests fail on Windows - Use `filepath.Join()` not string concatenation for paths -- Use `os.UserConfigDir()` not hardcoded `~/.config` +- Config path uses `~/.config/fr8/` (intentional — `os.UserConfigDir()` maps to `~/Library/Application Support` on macOS which is wrong for CLI tools) diff --git a/.claude/rules/go-style.md b/.claude/rules/go-style.md index 72e8e46..5fc749a 100644 --- a/.claude/rules/go-style.md +++ b/.claude/rules/go-style.md @@ -18,7 +18,7 @@ - Define interfaces at the consumer site, not the implementation site - Avoid premature interfaces — only when 2+ implementations exist or needed for testing - Zero value should be useful where possible (nil slices work as empty) -- Use `os.UserConfigDir()` / `os.UserCacheDir()` for platform-appropriate paths +- Config path: `~/.config/fr8/` via `os.UserHomeDir()` — not `os.UserConfigDir()` (which maps to `~/Library/Application Support` on macOS, wrong for CLI tools) ## Error Messages diff --git a/.claude/settings.json b/.claude/settings.json index f4a83d7..b0c1fdd 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -2,6 +2,7 @@ "permissions": { "allow": [ "Bash(go:*)", + "Bash(bin/ci)", "Bash(git log:*)", "Bash(git diff:*)", "Bash(git show:*)", diff --git a/CLAUDE.md b/CLAUDE.md index 324d70e..ec3accd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,15 +12,16 @@ Go CLI for managing git worktrees as isolated dev workspaces. See `README.md` fo ## Build & Test ```bash -go build ./... # Build all packages -go build -o fr8 . # Build binary -go vet ./... # Static analysis -go test ./... # Run tests +bin/ci # Run all CI checks locally (build, vet, test, lint, tidy) +go build ./... # Build all packages +go build -o fr8 . # Build binary +go vet ./... # Static analysis +go test ./... # Run tests go test -race -count=1 ./... # Exactly what CI runs -go install . # Install to GOPATH/bin +go install . # Install to GOPATH/bin ``` -CI also runs `golangci-lint` v2 — see `.claude/rules/ci.md`. +CI also runs `golangci-lint` v2 — see `.claude/rules/ci.md`. Run `bin/ci` to approximate the full CI pipeline locally before pushing. ## Project Layout @@ -78,4 +79,4 @@ Key architectural notes: - `createWorkspace()` in `cmd/new.go` is the shared creation function used by both CLI and TUI dashboard - Background process management uses tmux sessions named `fr8//`; graceful degradation when tmux is not installed -- Workspace openers are stored at `~/.config/fr8/openers.json`; TUI picker shown when multiple are configured +- Workspace openers are stored at `~/.config/fr8/openers.json`; TUI picker shown when multiple workspaces are configured diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..938acb1 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Protocollar + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md index dafb520..a4f8625 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ All workspace commands live under `fr8 ws` (alias `fr8 workspace`). | `fr8 ws browser [name]` | Open workspace dev server in the browser | | `fr8 ws archive [name] [--force]` | Tear down workspace (archive script + remove worktree) | | `fr8 dashboard` | Interactive TUI for browsing repos and workspaces | -| `fr8 config show\|validate` | View and validate configuration | +| `fr8 config show\|doctor [--fix]` | View config or check health (fix issues with --fix) | | `fr8 repo add\|list\|remove` | Manage the global repo registry | | `fr8 opener add\|list\|remove\|set-default` | Manage workspace openers (e.g. VSCode, Cursor) | | `fr8 completion [bash\|zsh\|fish]` | Generate shell completions | @@ -82,9 +82,9 @@ Create `fr8.json` in your repo root: "run": "bin/run-workspace", "archive": "bin/archive-workspace" }, - "portRange": 10, - "basePort": 8000, - "worktreePath": "~/fr8" + "port_range": 10, + "base_port": 8000, + "worktree_path": "~/fr8" } ``` @@ -93,13 +93,15 @@ Create `fr8.json` in your repo root: | `scripts.setup` | | Command to run after creating a workspace | | `scripts.run` | | Command to start the dev server | | `scripts.archive` | | Command to run before removing a workspace | -| `portRange` | `10` | Number of consecutive ports per workspace | -| `basePort` | `8000` | Starting port for allocation | -| `worktreePath` | `~/fr8` | Where to create worktrees (supports `~`, relative, or absolute paths) | +| `port_range` | `10` | Number of consecutive ports per workspace | +| `base_port` | `8000` | Starting port for allocation | +| `worktree_path` | `~/fr8` | Where to create worktrees (supports `~`, relative, or absolute paths) | Falls back to `conductor.json` if `fr8.json` doesn't exist, so projects using [Conductor](https://conductor.build) work without changes. -Use `fr8 config show` to see the resolved configuration (with defaults applied) and `fr8 config validate` to check for issues. +Legacy camelCase keys (`portRange`, `basePort`, `worktreePath`) are still accepted but deprecated. Run `fr8 config doctor --fix` to migrate automatically. + +Use `fr8 config show` to see the resolved configuration (with defaults applied) and `fr8 config doctor` to check for issues. ## How It Works @@ -201,9 +203,9 @@ Supports glob patterns including `**`. Files are only copied when their content ### Port Allocation -Ports are allocated sequentially in blocks of `portRange` (default 10) starting from `basePort`. Each workspace gets exclusive use of its block. Your scripts can use the base port (`FR8_PORT`) and offset from it for additional services (e.g. Redis on `FR8_PORT + 1`). +Ports are allocated sequentially in blocks of `port_range` (default 10) starting from `base_port`. Each workspace gets exclusive use of its block. Your scripts can use the base port (`FR8_PORT`) and offset from it for additional services (e.g. Redis on `FR8_PORT + 1`). -When allocating ports, fr8 checks all registered repos (see `fr8 repo list`) to avoid conflicts across projects that share the same `basePort`. If the global registry is unavailable, allocation falls back to the current repo's ports only. +When allocating ports, fr8 checks all registered repos (see `fr8 repo list`) to avoid conflicts across projects that share the same `base_port`. If the global registry is unavailable, allocation falls back to the current repo's ports only. ### State @@ -339,7 +341,7 @@ The MCP server exposes 12 tools: | `workspace_rename` | Rename a workspace | | `repo_list` | List registered repos (optionally include workspace details) | | `config_show` | Show resolved fr8 configuration for a repo | -| `config_validate` | Validate fr8 configuration and report errors/warnings | +| `config_doctor` | Check fr8 configuration health and report errors/warnings | All tools accept an optional `repo` parameter to target a specific registered repo. The MCP server uses the global registry for workspace resolution (it does not auto-detect from CWD since it runs as a long-lived process). @@ -402,4 +404,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md). ## License -MIT +[MIT](LICENSE) diff --git a/bin/ci b/bin/ci new file mode 100755 index 0000000..88d4e16 --- /dev/null +++ b/bin/ci @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +set -euo pipefail + +cd "$(git rev-parse --show-toplevel)" + +pass=0 +fail=0 +failed_steps=() + +step() { + printf "\n\033[1;34m==> %s\033[0m\n" "$1" +} + +ok() { + printf "\033[1;32m ✓ %s\033[0m\n" "$1" + ((pass++)) || true +} + +err() { + printf "\033[1;31m ✗ %s\033[0m\n" "$1" + ((fail++)) || true + failed_steps+=("$1") +} + +# Build +step "Build" +if go build ./...; then + ok "go build ./..." +else + err "go build ./..." +fi + +# Vet +step "Vet" +if go vet ./...; then + ok "go vet ./..." +else + err "go vet ./..." +fi + +# Test +step "Test" +if go test -race -count=1 ./...; then + ok "go test -race -count=1 ./..." +else + err "go test -race -count=1 ./..." +fi + +# Lint +step "Lint" +if command -v golangci-lint &>/dev/null; then + if golangci-lint run; then + ok "golangci-lint run" + else + err "golangci-lint run" + fi +else + printf " \033[1;33m⚠ golangci-lint not installed — skipping\033[0m\n" +fi + +# Tidy +step "Tidy" +go mod tidy +if git diff --exit-code go.mod go.sum &>/dev/null; then + ok "go mod tidy (no diff)" +else + err "go mod tidy produced changes" +fi + +# Summary +printf "\n\033[1;34m==> Summary\033[0m\n" +printf " %d passed, %d failed\n" "$pass" "$fail" +if ((fail > 0)); then + printf "\n\033[1;31m Failed:\033[0m\n" + for s in "${failed_steps[@]}"; do + printf " ✗ %s\n" "$s" + done + exit 1 +else + printf "\n\033[1;32m All checks passed.\033[0m\n" +fi diff --git a/cmd/config.go b/cmd/config.go index 6c06865..2c0558c 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -5,23 +5,30 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "github.com/spf13/cobra" "github.com/protocollar/fr8/internal/config" + "github.com/protocollar/fr8/internal/exitcode" "github.com/protocollar/fr8/internal/git" "github.com/protocollar/fr8/internal/jsonout" ) +var doctorFix bool + func init() { + configDoctorCmd.Flags().BoolVar(&doctorFix, "fix", false, "auto-fix correctable issues") configCmd.AddCommand(configShowCmd) - configCmd.AddCommand(configValidateCmd) + configCmd.AddCommand(configDoctorCmd) + configCmd.AddCommand(configValidateCmd) // alias + configCmd.AddCommand(configOpenCmd) rootCmd.AddCommand(configCmd) } var configCmd = &cobra.Command{ Use: "config", - Short: "View and validate configuration", + Short: "View and manage configuration", } var configShowCmd = &cobra.Command{ @@ -31,11 +38,54 @@ var configShowCmd = &cobra.Command{ RunE: runConfigShow, } +var configDoctorCmd = &cobra.Command{ + Use: "doctor", + Short: "Check configuration health and optionally fix issues", + Args: cobra.NoArgs, + RunE: runConfigDoctor, +} + var configValidateCmd = &cobra.Command{ - Use: "validate", - Short: "Validate configuration", + Use: "validate", + Short: "Check configuration health and optionally fix issues", + Args: cobra.NoArgs, + RunE: runConfigDoctor, + Hidden: true, // deprecated alias for doctor +} + +var configOpenCmd = &cobra.Command{ + Use: "open", + Short: "Open the fr8 config directory in the file manager", Args: cobra.NoArgs, - RunE: runConfigValidate, + RunE: runConfigOpen, +} + +// configDir returns the fr8 global config directory (~/.config/fr8). +func configDir() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("finding home directory: %w", err) + } + return filepath.Join(home, ".config", "fr8"), nil +} + +func runConfigOpen(cmd *cobra.Command, args []string) error { + dir, err := configDir() + if err != nil { + return err + } + + if err := os.MkdirAll(dir, 0755); err != nil { + return fmt.Errorf("creating config directory: %w", err) + } + + if jsonout.Enabled { + return jsonout.Write(struct { + Path string `json:"path"` + }{Path: dir}) + } + + return openBrowser(dir) } func runConfigShow(cmd *cobra.Command, args []string) error { @@ -60,10 +110,10 @@ func runConfigShow(cmd *cobra.Command, args []string) error { "run": cfg.Scripts.Run, "archive": cfg.Scripts.Archive, }, - "portRange": cfg.PortRange, - "basePort": cfg.BasePort, - "worktreePath": cfg.WorktreePath, - "resolvedWorktreePath": config.ResolveWorktreePath(cfg, rootPath), + "port_range": cfg.PortRange, + "base_port": cfg.BasePort, + "worktree_path": cfg.WorktreePath, + "resolved_worktree_path": config.ResolveWorktreePath(cfg, rootPath), } if jsonout.Enabled { @@ -78,7 +128,7 @@ func runConfigShow(cmd *cobra.Command, args []string) error { return nil } -func runConfigValidate(cmd *cobra.Command, args []string) error { +func runConfigDoctor(cmd *cobra.Command, args []string) error { cwd, err := os.Getwd() if err != nil { return err @@ -96,6 +146,18 @@ func runConfigValidate(cmd *cobra.Command, args []string) error { var warnings []string var configErrors []string + var fixableFiles []string // config files with legacy keys + + // Check for deprecated camelCase keys + for _, name := range []string{"fr8.json", "conductor.json"} { + p := filepath.Join(rootPath, name) + if legacy := config.HasLegacyKeys(p); len(legacy) > 0 { + fixableFiles = append(fixableFiles, p) + for _, key := range legacy { + warnings = append(warnings, fmt.Sprintf("%s: deprecated key %q — rename to %q (fixable)", name, key, config.LegacyKeyReplacement(key))) + } + } + } // Check script paths for name, script := range map[string]string{ @@ -119,23 +181,58 @@ func runConfigValidate(cmd *cobra.Command, args []string) error { wtPath := config.ResolveWorktreePath(cfg, rootPath) if info, err := os.Stat(wtPath); err == nil { if !info.IsDir() { - configErrors = append(configErrors, fmt.Sprintf("worktreePath: %q exists but is not a directory", wtPath)) + configErrors = append(configErrors, fmt.Sprintf("worktree_path: %q exists but is not a directory", wtPath)) } } // Parent must exist or be creatable — not an error if it doesn't exist yet // Check port ranges if cfg.BasePort < 1024 { - warnings = append(warnings, fmt.Sprintf("basePort: %d is a privileged port (< 1024)", cfg.BasePort)) + warnings = append(warnings, fmt.Sprintf("base_port: %d is a privileged port (< 1024)", cfg.BasePort)) } if cfg.BasePort > 65535 { - configErrors = append(configErrors, fmt.Sprintf("basePort: %d is out of range (> 65535)", cfg.BasePort)) + configErrors = append(configErrors, fmt.Sprintf("base_port: %d is out of range (> 65535)", cfg.BasePort)) } if cfg.PortRange < 1 { - configErrors = append(configErrors, fmt.Sprintf("portRange: %d must be at least 1", cfg.PortRange)) + configErrors = append(configErrors, fmt.Sprintf("port_range: %d must be at least 1", cfg.PortRange)) } if cfg.BasePort+cfg.PortRange*100 > 65535 { - warnings = append(warnings, fmt.Sprintf("basePort %d + portRange %d may exhaust available ports with many workspaces", cfg.BasePort, cfg.PortRange)) + warnings = append(warnings, fmt.Sprintf("base_port %d + port_range %d may exhaust available ports with many workspaces", cfg.BasePort, cfg.PortRange)) + } + + // Handle --fix + var fixed []string + if doctorFix && len(fixableFiles) > 0 { + switch { + case isInteractive(): + fmt.Println("The following config files have deprecated camelCase keys:") + for _, f := range fixableFiles { + fmt.Printf(" %s\n", filepath.Base(f)) + } + fmt.Printf("\nMigrate to snake_case? [y/N] ") + + var response string + _, _ = fmt.Scanln(&response) + if response != "y" && response != "Y" { + fmt.Println("Skipped.") + fixableFiles = nil + } + case jsonout.Enabled: + // In JSON mode --fix applies without prompting + default: + return exitcode.New("interactive_only", exitcode.InteractiveOnly, + "--fix requires an interactive terminal or --json mode") + } + + for _, f := range fixableFiles { + migrated, err := config.MigrateKeys(f) + if err != nil { + return fmt.Errorf("migrating %s: %w", filepath.Base(f), err) + } + for _, key := range migrated { + fixed = append(fixed, fmt.Sprintf("%s: %s -> %s", filepath.Base(f), key, config.LegacyKeyReplacement(key))) + } + } } if jsonout.Enabled { @@ -143,10 +240,12 @@ func runConfigValidate(cmd *cobra.Command, args []string) error { Valid bool `json:"valid"` Errors []string `json:"errors"` Warnings []string `json:"warnings"` + Fixed []string `json:"fixed"` }{ Valid: len(configErrors) == 0, Errors: orEmpty(configErrors), Warnings: orEmpty(warnings), + Fixed: orEmpty(fixed), }) } @@ -162,11 +261,22 @@ func runConfigValidate(cmd *cobra.Command, args []string) error { fmt.Printf(" ⚠ %s\n", w) } } + if len(fixed) > 0 { + fmt.Println("\nFixed:") + for _, f := range fixed { + fmt.Printf(" ✓ %s\n", f) + } + } switch { + case len(fixed) > 0 && len(configErrors) == 0: + fmt.Println("\nFixed issues. Configuration is valid.") case len(configErrors) == 0 && len(warnings) == 0: fmt.Println("Configuration is valid.") case len(configErrors) == 0: + if len(fixableFiles) > 0 && !doctorFix { + fmt.Println("\nRun with --fix to auto-correct fixable issues.") + } fmt.Println("\nConfiguration is valid with warnings.") default: return fmt.Errorf("configuration has %d error(s); fix the issues above in fr8.json", len(configErrors)) diff --git a/cmd/config_test.go b/cmd/config_test.go new file mode 100644 index 0000000..2b870b5 --- /dev/null +++ b/cmd/config_test.go @@ -0,0 +1,20 @@ +package cmd + +import ( + "os" + "path/filepath" + "testing" +) + +func TestConfigDir(t *testing.T) { + dir, err := configDir() + if err != nil { + t.Fatalf("configDir: %v", err) + } + + home, _ := os.UserHomeDir() + want := filepath.Join(home, ".config", "fr8") + if dir != want { + t.Errorf("configDir() = %q, want %q", dir, want) + } +} diff --git a/cmd/mcp.go b/cmd/mcp.go index 3059227..ce4eb48 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -55,7 +55,7 @@ AVAILABLE TOOLS workspace_rename Rename a workspace repo_list List registered repos config_show Show resolved fr8 configuration for a repo - config_validate Validate fr8 configuration and report errors/warnings + config_doctor Check fr8 configuration health and report errors/warnings All tools accept an optional "repo" parameter to target a specific registered repo. The server uses the global registry for workspace resolution. diff --git a/cmd/mcp_tools.go b/cmd/mcp_tools.go index d81bf3b..a594f5c 100644 --- a/cmd/mcp_tools.go +++ b/cmd/mcp_tools.go @@ -53,43 +53,30 @@ func mcpResolveWorkspace(name, repo string) (*state.Workspace, string, string, e } // mcpResolveRepo resolves a repo's root path and git common dir from a repo name. -// If repo is empty, tries to detect from CWD. +// Unlike the CLI, this never detects from CWD — the MCP server runs as a long-lived process. func mcpResolveRepo(repo string) (rootPath, commonDir string, err error) { - if repo != "" { - regPath, err := registry.DefaultPath() - if err != nil { - return "", "", err - } - reg, err := registry.Load(regPath) - if err != nil { - return "", "", fmt.Errorf("loading registry: %w", err) - } - r := reg.Find(repo) - if r == nil { - return "", "", fmt.Errorf("repo %q not found in registry (see: fr8 repo list)", repo) - } - rootPath, err = git.RootWorktreePath(r.Path) - if err != nil { - rootPath = r.Path - } - commonDir, err = git.CommonDir(r.Path) - if err != nil { - return "", "", fmt.Errorf("finding git common dir: %w", err) - } - return rootPath, commonDir, nil + if repo == "" { + return "", "", fmt.Errorf("repo parameter is required") } - - cwd, err := os.Getwd() + regPath, err := registry.DefaultPath() if err != nil { return "", "", err } - commonDir, err = git.CommonDir(cwd) + reg, err := registry.Load(regPath) + if err != nil { + return "", "", fmt.Errorf("loading registry: %w", err) + } + r := reg.Find(repo) + if r == nil { + return "", "", fmt.Errorf("repo %q not found in registry (see: fr8 repo list)", repo) + } + rootPath, err = git.RootWorktreePath(r.Path) if err != nil { - return "", "", fmt.Errorf("not inside a git repository (specify the repo parameter)") + rootPath = r.Path } - rootPath, err = git.RootWorktreePath(cwd) + commonDir, err = git.CommonDir(r.Path) if err != nil { - return "", "", fmt.Errorf("finding root worktree: %w", err) + return "", "", fmt.Errorf("finding git common dir: %w", err) } return rootPath, commonDir, nil } @@ -129,6 +116,7 @@ func registerMCPTools(s *server.MCPServer) { mcp.WithString("repo", mcp.Description("Target repo name from registry")), mcp.WithBoolean("no_setup", mcp.Description("Skip running the setup script")), mcp.WithBoolean("if_not_exists", mcp.Description("Succeed silently if workspace already exists")), + mcp.WithReadOnlyHintAnnotation(false), mcp.WithDestructiveHintAnnotation(false), ), handleWorkspaceCreate, @@ -197,6 +185,7 @@ func registerMCPTools(s *server.MCPServer) { mcp.WithString("old_name", mcp.Description("Current workspace name"), mcp.Required()), mcp.WithString("new_name", mcp.Description("New workspace name"), mcp.Required()), mcp.WithString("repo", mcp.Description("Repo name")), + mcp.WithReadOnlyHintAnnotation(false), mcp.WithDestructiveHintAnnotation(false), ), handleWorkspaceRename, @@ -223,13 +212,13 @@ func registerMCPTools(s *server.MCPServer) { ) s.AddTool( - mcp.NewTool("config_validate", - mcp.WithDescription("Validate fr8 configuration for a repo."), + mcp.NewTool("config_doctor", + mcp.WithDescription("Check fr8 configuration health for a repo. Reports errors, warnings, and fixable issues."), mcp.WithString("repo", mcp.Description("Repo name")), mcp.WithReadOnlyHintAnnotation(true), mcp.WithDestructiveHintAnnotation(false), ), - handleConfigValidate, + handleConfigDoctor, ) } @@ -547,15 +536,14 @@ func handleWorkspaceStop(ctx context.Context, req mcp.CallToolRequest) (*mcp.Cal sessionName := tmux.SessionName(tmux.RepoName(rootPath), ws.Name) if !tmux.IsRunning(sessionName) { - action := "not_running" if !ifRunning { - action = "not_running" + return mcpError(fmt.Sprintf("workspace %q is not running", ws.Name)) } return mcpResult(struct { Action string `json:"action"` Workspace string `json:"workspace"` Session string `json:"session"` - }{Action: action, Workspace: ws.Name, Session: sessionName}) + }{Action: "already_stopped", Workspace: ws.Name, Session: sessionName}) } if err := tmux.Stop(sessionName); err != nil { @@ -722,16 +710,16 @@ func handleConfigShow(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallTo "run": cfg.Scripts.Run, "archive": cfg.Scripts.Archive, }, - "portRange": cfg.PortRange, - "basePort": cfg.BasePort, - "worktreePath": cfg.WorktreePath, - "resolvedWorktreePath": config.ResolveWorktreePath(cfg, rootPath), + "port_range": cfg.PortRange, + "base_port": cfg.BasePort, + "worktree_path": cfg.WorktreePath, + "resolved_worktree_path": config.ResolveWorktreePath(cfg, rootPath), } return mcpResult(resolved) } -func handleConfigValidate(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { +func handleConfigDoctor(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { repo := req.GetString("repo", "") rootPath, _, err := mcpResolveRepo(repo) if err != nil { @@ -765,18 +753,18 @@ func handleConfigValidate(ctx context.Context, req mcp.CallToolRequest) (*mcp.Ca wtPath := config.ResolveWorktreePath(cfg, rootPath) if info, err := os.Stat(wtPath); err == nil { if !info.IsDir() { - configErrors = append(configErrors, fmt.Sprintf("worktreePath: %q exists but is not a directory", wtPath)) + configErrors = append(configErrors, fmt.Sprintf("worktree_path: %q exists but is not a directory", wtPath)) } } if cfg.BasePort < 1024 { - warnings = append(warnings, fmt.Sprintf("basePort: %d is a privileged port (< 1024)", cfg.BasePort)) + warnings = append(warnings, fmt.Sprintf("base_port: %d is a privileged port (< 1024)", cfg.BasePort)) } if cfg.BasePort > 65535 { - configErrors = append(configErrors, fmt.Sprintf("basePort: %d is out of range (> 65535)", cfg.BasePort)) + configErrors = append(configErrors, fmt.Sprintf("base_port: %d is out of range (> 65535)", cfg.BasePort)) } if cfg.PortRange < 1 { - configErrors = append(configErrors, fmt.Sprintf("portRange: %d must be at least 1", cfg.PortRange)) + configErrors = append(configErrors, fmt.Sprintf("port_range: %d must be at least 1", cfg.PortRange)) } if configErrors == nil { diff --git a/cmd/mcp_tools_test.go b/cmd/mcp_tools_test.go index 59f089b..567a115 100644 --- a/cmd/mcp_tools_test.go +++ b/cmd/mcp_tools_test.go @@ -2,8 +2,6 @@ package cmd import ( "encoding/json" - "os" - "os/exec" "testing" "github.com/mark3labs/mcp-go/mcp" @@ -83,59 +81,14 @@ func TestMcpResolveWorkspaceWithRepoNoName(t *testing.T) { } } -func TestMcpResolveRepoFromCWD(t *testing.T) { - dir := initTestRepo(t) - - origDir, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - defer func() { - if err := os.Chdir(origDir); err != nil { - t.Fatal(err) - } - }() - - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - - rootPath, commonDir, err := mcpResolveRepo("") - if err != nil { - t.Fatalf("mcpResolveRepo from git repo: %v", err) - } - if rootPath == "" { - t.Error("rootPath should not be empty") - } - if commonDir == "" { - t.Error("commonDir should not be empty") - } -} - -func TestMcpResolveRepoNotGitRepo(t *testing.T) { - dir := t.TempDir() - - origDir, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - defer func() { - if err := os.Chdir(origDir); err != nil { - t.Fatal(err) - } - }() - - if err := os.Chdir(dir); err != nil { - t.Fatal(err) - } - - _, _, err = mcpResolveRepo("") +func TestMcpResolveRepoRequiresParam(t *testing.T) { + _, _, err := mcpResolveRepo("") if err == nil { - t.Fatal("expected error outside git repo") + t.Fatal("expected error for empty repo param") } - want := "not inside a git repository" - if !contains(err.Error(), want) { - t.Errorf("error = %q, want to contain %q", err.Error(), want) + want := "repo parameter is required" + if err.Error() != want { + t.Errorf("error = %q, want %q", err.Error(), want) } } @@ -156,7 +109,7 @@ func TestRegisterMCPTools(t *testing.T) { "workspace_rename", "repo_list", "config_show", - "config_validate", + "config_doctor", } if len(tools) != len(expectedTools) { @@ -168,33 +121,3 @@ func TestRegisterMCPTools(t *testing.T) { } } } - -// contains checks if s contains substr (same as exitcode.contains but local). -func contains(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} - -// initTestRepo creates a temporary git repo for integration tests. -func initTestRepo(t *testing.T) string { - t.Helper() - dir := t.TempDir() - cmds := [][]string{ - {"git", "init"}, - {"git", "config", "user.email", "test@test.com"}, - {"git", "config", "user.name", "Test"}, - {"git", "commit", "--allow-empty", "-m", "init"}, - } - for _, args := range cmds { - cmd := exec.Command(args[0], args[1:]...) - cmd.Dir = dir - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("%v failed: %s", args, out) - } - } - return dir -} diff --git a/cmd/skill_template.md b/cmd/skill_template.md index 142163c..6119775 100644 --- a/cmd/skill_template.md +++ b/cmd/skill_template.md @@ -37,7 +37,7 @@ Human messages are suppressed in JSON mode. Errors appear on stderr as JSON: | Rename workspace | `fr8 ws rename --json` | | | List repos | `fr8 repo list --json` | `-w` (include workspaces) | | Show config | `fr8 config show --json` | `--repo ` | -| Validate config | `fr8 config validate --json` | `--repo ` | +| Check config | `fr8 config doctor --json` | `--fix`, `--repo ` | ## Exit Codes diff --git a/internal/config/config.go b/internal/config/config.go index 450017c..5016652 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,9 +11,59 @@ import ( // Config represents the fr8.json (or conductor.json) configuration. type Config struct { Scripts Scripts `json:"scripts"` - PortRange int `json:"portRange"` - BasePort int `json:"basePort"` - WorktreePath string `json:"worktreePath"` + PortRange int `json:"port_range"` + BasePort int `json:"base_port"` + WorktreePath string `json:"worktree_path"` +} + +// UnmarshalJSON supports both snake_case (preferred) and legacy camelCase keys. +func (c *Config) UnmarshalJSON(data []byte) error { + // Decode into a raw map to handle both key styles. + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + if v, ok := raw["scripts"]; ok { + if err := json.Unmarshal(v, &c.Scripts); err != nil { + return fmt.Errorf("parsing scripts: %w", err) + } + } + + // port_range (preferred) or portRange (legacy) + if v, ok := raw["port_range"]; ok { + if err := json.Unmarshal(v, &c.PortRange); err != nil { + return fmt.Errorf("parsing port_range: %w", err) + } + } else if v, ok := raw["portRange"]; ok { + if err := json.Unmarshal(v, &c.PortRange); err != nil { + return fmt.Errorf("parsing portRange: %w", err) + } + } + + // base_port (preferred) or basePort (legacy) + if v, ok := raw["base_port"]; ok { + if err := json.Unmarshal(v, &c.BasePort); err != nil { + return fmt.Errorf("parsing base_port: %w", err) + } + } else if v, ok := raw["basePort"]; ok { + if err := json.Unmarshal(v, &c.BasePort); err != nil { + return fmt.Errorf("parsing basePort: %w", err) + } + } + + // worktree_path (preferred) or worktreePath (legacy) + if v, ok := raw["worktree_path"]; ok { + if err := json.Unmarshal(v, &c.WorktreePath); err != nil { + return fmt.Errorf("parsing worktree_path: %w", err) + } + } else if v, ok := raw["worktreePath"]; ok { + if err := json.Unmarshal(v, &c.WorktreePath); err != nil { + return fmt.Errorf("parsing worktreePath: %w", err) + } + } + + return nil } // Scripts defines the lifecycle commands. @@ -23,6 +73,82 @@ type Scripts struct { Archive string `json:"archive"` } +// legacyKeys are the deprecated camelCase config keys and their snake_case replacements. +var legacyKeys = map[string]string{ + "portRange": "port_range", + "basePort": "base_port", + "worktreePath": "worktree_path", +} + +// LegacyKeyReplacement returns the snake_case replacement for a legacy camelCase key. +func LegacyKeyReplacement(key string) string { + return legacyKeys[key] +} + +// HasLegacyKeys checks whether a config file at path uses deprecated camelCase keys. +// Returns the list of legacy keys found. +func HasLegacyKeys(path string) []string { + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return nil + } + var found []string + for old := range legacyKeys { + if _, ok := raw[old]; ok { + found = append(found, old) + } + } + return found +} + +// MigrateKeys rewrites a config file, replacing deprecated camelCase keys with snake_case. +// Returns the list of keys that were migrated. +func MigrateKeys(path string) ([]string, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", filepath.Base(path), err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("parsing %s: %w", filepath.Base(path), err) + } + + var migrated []string + for old, new := range legacyKeys { + v, ok := raw[old] + if !ok { + continue + } + // Only migrate if the new key doesn't already exist + if _, exists := raw[new]; !exists { + raw[new] = v + } + delete(raw, old) + migrated = append(migrated, old) + } + + if len(migrated) == 0 { + return nil, nil + } + + out, err := json.MarshalIndent(raw, "", " ") + if err != nil { + return nil, fmt.Errorf("marshaling config: %w", err) + } + out = append(out, '\n') + + if err := os.WriteFile(path, out, 0644); err != nil { + return nil, fmt.Errorf("writing %s: %w", filepath.Base(path), err) + } + + return migrated, nil +} + // Load reads fr8.json from rootPath, falling back to conductor.json. // Returns config with defaults applied. func Load(rootPath string) (*Config, error) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 008d317..faa8cd6 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -10,6 +10,34 @@ func TestLoadFr8Json(t *testing.T) { dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "fr8.json"), []byte(`{ "scripts": {"setup": "make setup", "run": "make run", "archive": "make clean"}, + "port_range": 5, + "base_port": 3000, + "worktree_path": "/tmp/ws" + }`), 0644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(dir) + if err != nil { + t.Fatal(err) + } + if cfg.Scripts.Setup != "make setup" { + t.Errorf("Setup = %q, want %q", cfg.Scripts.Setup, "make setup") + } + if cfg.PortRange != 5 { + t.Errorf("PortRange = %d, want 5", cfg.PortRange) + } + if cfg.BasePort != 3000 { + t.Errorf("BasePort = %d, want 3000", cfg.BasePort) + } + if cfg.WorktreePath != "/tmp/ws" { + t.Errorf("WorktreePath = %q, want /tmp/ws", cfg.WorktreePath) + } +} + +func TestLoadLegacyCamelCaseKeys(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "fr8.json"), []byte(`{ "portRange": 5, "basePort": 3000, "worktreePath": "/tmp/ws" @@ -21,8 +49,91 @@ func TestLoadFr8Json(t *testing.T) { if err != nil { t.Fatal(err) } - if cfg.Scripts.Setup != "make setup" { - t.Errorf("Setup = %q, want %q", cfg.Scripts.Setup, "make setup") + if cfg.PortRange != 5 { + t.Errorf("PortRange = %d, want 5", cfg.PortRange) + } + if cfg.BasePort != 3000 { + t.Errorf("BasePort = %d, want 3000", cfg.BasePort) + } + if cfg.WorktreePath != "/tmp/ws" { + t.Errorf("WorktreePath = %q, want /tmp/ws", cfg.WorktreePath) + } +} + +func TestHasLegacyKeys(t *testing.T) { + dir := t.TempDir() + + // File with legacy keys + legacyPath := filepath.Join(dir, "legacy.json") + if err := os.WriteFile(legacyPath, []byte(`{"portRange": 5, "basePort": 3000}`), 0644); err != nil { + t.Fatal(err) + } + found := HasLegacyKeys(legacyPath) + if len(found) != 2 { + t.Errorf("HasLegacyKeys = %v, want 2 keys", found) + } + + // File with snake_case keys + modernPath := filepath.Join(dir, "modern.json") + if err := os.WriteFile(modernPath, []byte(`{"port_range": 5, "base_port": 3000}`), 0644); err != nil { + t.Fatal(err) + } + found = HasLegacyKeys(modernPath) + if len(found) != 0 { + t.Errorf("HasLegacyKeys = %v, want 0 keys", found) + } +} + +func TestLegacyKeyReplacement(t *testing.T) { + tests := []struct { + name string + key string + want string + }{ + {name: "portRange", key: "portRange", want: "port_range"}, + {name: "basePort", key: "basePort", want: "base_port"}, + {name: "worktreePath", key: "worktreePath", want: "worktree_path"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := LegacyKeyReplacement(tt.key) + if got != tt.want { + t.Errorf("LegacyKeyReplacement(%q) = %q, want %q", tt.key, got, tt.want) + } + }) + } +} + +func TestMigrateKeys(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "fr8.json") + if err := os.WriteFile(p, []byte(`{ + "scripts": {"setup": "make setup"}, + "portRange": 5, + "basePort": 3000, + "worktreePath": "/tmp/ws" +}`), 0644); err != nil { + t.Fatal(err) + } + + migrated, err := MigrateKeys(p) + if err != nil { + t.Fatal(err) + } + if len(migrated) != 3 { + t.Errorf("MigrateKeys returned %d keys, want 3", len(migrated)) + } + + // Verify the file was rewritten with snake_case keys + legacy := HasLegacyKeys(p) + if len(legacy) != 0 { + t.Errorf("HasLegacyKeys after migration = %v, want none", legacy) + } + + // Verify the config still loads correctly + cfg, err := Load(dir) + if err != nil { + t.Fatal(err) } if cfg.PortRange != 5 { t.Errorf("PortRange = %d, want 5", cfg.PortRange) @@ -35,6 +146,40 @@ func TestLoadFr8Json(t *testing.T) { } } +func TestMigrateKeysNoOp(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "fr8.json") + if err := os.WriteFile(p, []byte(`{"port_range": 5, "base_port": 3000}`), 0644); err != nil { + t.Fatal(err) + } + + migrated, err := MigrateKeys(p) + if err != nil { + t.Fatal(err) + } + if migrated != nil { + t.Errorf("MigrateKeys = %v, want nil (nothing to migrate)", migrated) + } +} + +func TestSnakeCaseTakesPrecedenceOverCamelCase(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "fr8.json"), []byte(`{ + "port_range": 20, + "portRange": 5 + }`), 0644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(dir) + if err != nil { + t.Fatal(err) + } + if cfg.PortRange != 20 { + t.Errorf("PortRange = %d, want 20 (snake_case should take precedence)", cfg.PortRange) + } +} + func TestLoadFallbackToConductorJson(t *testing.T) { dir := t.TempDir() if err := os.WriteFile(filepath.Join(dir, "conductor.json"), []byte(`{ diff --git a/internal/flock/flock_test.go b/internal/flock/flock_test.go new file mode 100644 index 0000000..654657d --- /dev/null +++ b/internal/flock/flock_test.go @@ -0,0 +1,82 @@ +package flock + +import ( + "os" + "path/filepath" + "syscall" + "testing" +) + +func TestLockUnlock(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.lock") + + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + + if err := Lock(f.Fd()); err != nil { + t.Fatalf("Lock: %v", err) + } + + if err := Unlock(f.Fd()); err != nil { + t.Fatalf("Unlock: %v", err) + } +} + +func TestLockIsExclusive(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.lock") + + f1, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f1.Close() }() + + if err := Lock(f1.Fd()); err != nil { + t.Fatalf("Lock f1: %v", err) + } + defer func() { _ = Unlock(f1.Fd()) }() + + // Open a second fd and try a non-blocking lock — it should fail + // with EWOULDBLOCK since f1 holds the exclusive lock. + f2, err := os.OpenFile(path, os.O_RDWR, 0644) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f2.Close() }() + + err = syscall.Flock(int(f2.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) + if err == nil { + t.Error("expected non-blocking lock to fail while f1 holds lock") + _ = Unlock(f2.Fd()) + } +} + +func TestRelockAfterUnlock(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "test.lock") + + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + + // Lock, unlock, then lock again — should succeed + if err := Lock(f.Fd()); err != nil { + t.Fatalf("first Lock: %v", err) + } + if err := Unlock(f.Fd()); err != nil { + t.Fatalf("Unlock: %v", err) + } + if err := Lock(f.Fd()); err != nil { + t.Fatalf("second Lock: %v", err) + } + if err := Unlock(f.Fd()); err != nil { + t.Fatalf("second Unlock: %v", err) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index a880d20..55b2a51 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -223,7 +223,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.batchArchiveNames = nil m.err = nil if len(msg.failed) > 0 { - m.err = fmt.Errorf("failed to archive: %s", strings.Join(msg.failed, ", ")) + m.err = fmt.Errorf("archiving: %s", strings.Join(msg.failed, ", ")) } m.view = viewWorkspaceList return m, nil