From 8c0f9abe55425043c12d96f510419e395cd57535 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Sun, 4 Jan 2026 21:02:25 -0800 Subject: [PATCH 1/2] feat(devcontainer): add smart CLI fallback with local npm installation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current behavior: When devcontainer CLI is not found in PATH, the command fails with an error message telling users to install it manually. New behavior: The CLI resolution now follows a progressive fallback: 1. Check if devcontainer is in PATH 2. Check if devcontainer.path is set in config and binary exists 3. If npm is available, prompt user to install CLI locally 4. Install to ~/.local/share/headjack/devcontainer-cli/ and save path to config Also extracts the huh-based prompter from internal/auth into a shared internal/prompt package for reuse across the codebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/auth/auth.go | 14 -- internal/auth/mocks/prompter.go | 170 ----------------------- internal/auth/prompter.go | 72 ---------- internal/cmd/auth.go | 13 +- internal/cmd/run.go | 45 +++--- internal/config/config.go | 16 ++- internal/devcontainer/cli.go | 208 ++++++++++++++++++++++++++++ internal/prompt/mocks/prompter.go | 220 ++++++++++++++++++++++++++++++ internal/prompt/prompt.go | 115 ++++++++++++++++ 9 files changed, 582 insertions(+), 291 deletions(-) delete mode 100644 internal/auth/mocks/prompter.go delete mode 100644 internal/auth/prompter.go create mode 100644 internal/devcontainer/cli.go create mode 100644 internal/prompt/mocks/prompter.go create mode 100644 internal/prompt/prompt.go diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 823be6f..4ee3f71 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -99,20 +99,6 @@ type Provider interface { Load(storage Storage) (*Credential, error) } -// Prompter abstracts user interaction for credential collection. -// -//go:generate go run github.com/matryer/moq@latest -pkg mocks -out mocks/prompter.go . Prompter -type Prompter interface { - // Print outputs text to the user. - Print(message string) - - // PromptSecret prompts for secret input (no echo). - PromptSecret(prompt string) (string, error) - - // PromptChoice prompts user to select from options, returns 0-based index. - PromptChoice(prompt string, options []string) (int, error) -} - // StoreCredential is a helper function to store a credential in JSON format. func StoreCredential(storage Storage, account string, cred Credential) error { data, err := json.Marshal(cred) diff --git a/internal/auth/mocks/prompter.go b/internal/auth/mocks/prompter.go deleted file mode 100644 index bebe4ad..0000000 --- a/internal/auth/mocks/prompter.go +++ /dev/null @@ -1,170 +0,0 @@ -// Code generated by moq; DO NOT EDIT. -// github.com/matryer/moq - -package mocks - -import ( - "sync" - - "github.com/jmgilman/headjack/internal/auth" -) - -// Ensure, that PrompterMock does implement auth.Prompter. -// If this is not the case, regenerate this file with moq. -var _ auth.Prompter = &PrompterMock{} - -// PrompterMock is a mock implementation of auth.Prompter. -// -// func TestSomethingThatUsesPrompter(t *testing.T) { -// -// // make and configure a mocked auth.Prompter -// mockedPrompter := &PrompterMock{ -// PrintFunc: func(message string) { -// panic("mock out the Print method") -// }, -// PromptChoiceFunc: func(prompt string, options []string) (int, error) { -// panic("mock out the PromptChoice method") -// }, -// PromptSecretFunc: func(prompt string) (string, error) { -// panic("mock out the PromptSecret method") -// }, -// } -// -// // use mockedPrompter in code that requires auth.Prompter -// // and then make assertions. -// -// } -type PrompterMock struct { - // PrintFunc mocks the Print method. - PrintFunc func(message string) - - // PromptChoiceFunc mocks the PromptChoice method. - PromptChoiceFunc func(prompt string, options []string) (int, error) - - // PromptSecretFunc mocks the PromptSecret method. - PromptSecretFunc func(prompt string) (string, error) - - // calls tracks calls to the methods. - calls struct { - // Print holds details about calls to the Print method. - Print []struct { - // Message is the message argument value. - Message string - } - // PromptChoice holds details about calls to the PromptChoice method. - PromptChoice []struct { - // Prompt is the prompt argument value. - Prompt string - // Options is the options argument value. - Options []string - } - // PromptSecret holds details about calls to the PromptSecret method. - PromptSecret []struct { - // Prompt is the prompt argument value. - Prompt string - } - } - lockPrint sync.RWMutex - lockPromptChoice sync.RWMutex - lockPromptSecret sync.RWMutex -} - -// Print calls PrintFunc. -func (mock *PrompterMock) Print(message string) { - if mock.PrintFunc == nil { - panic("PrompterMock.PrintFunc: method is nil but Prompter.Print was just called") - } - callInfo := struct { - Message string - }{ - Message: message, - } - mock.lockPrint.Lock() - mock.calls.Print = append(mock.calls.Print, callInfo) - mock.lockPrint.Unlock() - mock.PrintFunc(message) -} - -// PrintCalls gets all the calls that were made to Print. -// Check the length with: -// -// len(mockedPrompter.PrintCalls()) -func (mock *PrompterMock) PrintCalls() []struct { - Message string -} { - var calls []struct { - Message string - } - mock.lockPrint.RLock() - calls = mock.calls.Print - mock.lockPrint.RUnlock() - return calls -} - -// PromptChoice calls PromptChoiceFunc. -func (mock *PrompterMock) PromptChoice(prompt string, options []string) (int, error) { - if mock.PromptChoiceFunc == nil { - panic("PrompterMock.PromptChoiceFunc: method is nil but Prompter.PromptChoice was just called") - } - callInfo := struct { - Prompt string - Options []string - }{ - Prompt: prompt, - Options: options, - } - mock.lockPromptChoice.Lock() - mock.calls.PromptChoice = append(mock.calls.PromptChoice, callInfo) - mock.lockPromptChoice.Unlock() - return mock.PromptChoiceFunc(prompt, options) -} - -// PromptChoiceCalls gets all the calls that were made to PromptChoice. -// Check the length with: -// -// len(mockedPrompter.PromptChoiceCalls()) -func (mock *PrompterMock) PromptChoiceCalls() []struct { - Prompt string - Options []string -} { - var calls []struct { - Prompt string - Options []string - } - mock.lockPromptChoice.RLock() - calls = mock.calls.PromptChoice - mock.lockPromptChoice.RUnlock() - return calls -} - -// PromptSecret calls PromptSecretFunc. -func (mock *PrompterMock) PromptSecret(prompt string) (string, error) { - if mock.PromptSecretFunc == nil { - panic("PrompterMock.PromptSecretFunc: method is nil but Prompter.PromptSecret was just called") - } - callInfo := struct { - Prompt string - }{ - Prompt: prompt, - } - mock.lockPromptSecret.Lock() - mock.calls.PromptSecret = append(mock.calls.PromptSecret, callInfo) - mock.lockPromptSecret.Unlock() - return mock.PromptSecretFunc(prompt) -} - -// PromptSecretCalls gets all the calls that were made to PromptSecret. -// Check the length with: -// -// len(mockedPrompter.PromptSecretCalls()) -func (mock *PrompterMock) PromptSecretCalls() []struct { - Prompt string -} { - var calls []struct { - Prompt string - } - mock.lockPromptSecret.RLock() - calls = mock.calls.PromptSecret - mock.lockPromptSecret.RUnlock() - return calls -} diff --git a/internal/auth/prompter.go b/internal/auth/prompter.go deleted file mode 100644 index 5006c54..0000000 --- a/internal/auth/prompter.go +++ /dev/null @@ -1,72 +0,0 @@ -package auth - -import ( - "errors" - "fmt" - "strings" - - "github.com/charmbracelet/huh" -) - -// HuhPrompter implements Prompter using charmbracelet/huh for interactive forms. -type HuhPrompter struct{} - -// NewTerminalPrompter creates a new HuhPrompter for interactive terminal prompts. -func NewTerminalPrompter() *HuhPrompter { - return &HuhPrompter{} -} - -// Print outputs text to the user. -func (p *HuhPrompter) Print(message string) { - fmt.Println(message) -} - -// PromptSecret prompts for secret input with masked display. -func (p *HuhPrompter) PromptSecret(prompt string) (string, error) { - var value string - - err := huh.NewInput(). - Title(prompt). - EchoMode(huh.EchoModePassword). - Value(&value). - Run() - - if err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return "", errors.New("canceled by user") - } - return "", fmt.Errorf("prompt input: %w", err) - } - - return strings.TrimSpace(value), nil -} - -// PromptChoice prompts user to select from options and returns the 0-based index. -func (p *HuhPrompter) PromptChoice(prompt string, options []string) (int, error) { - if len(options) == 0 { - return 0, errors.New("no options provided") - } - - // Build huh options with display labels and index values - huhOptions := make([]huh.Option[int], len(options)) - for i, opt := range options { - huhOptions[i] = huh.NewOption(opt, i) - } - - var selected int - - err := huh.NewSelect[int](). - Title(prompt). - Options(huhOptions...). - Value(&selected). - Run() - - if err != nil { - if errors.Is(err, huh.ErrUserAborted) { - return 0, errors.New("canceled by user") - } - return 0, fmt.Errorf("prompt choice: %w", err) - } - - return selected, nil -} diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go index ab12c77..1c259e9 100644 --- a/internal/cmd/auth.go +++ b/internal/cmd/auth.go @@ -8,6 +8,7 @@ import ( "github.com/jmgilman/headjack/internal/auth" "github.com/jmgilman/headjack/internal/keychain" + "github.com/jmgilman/headjack/internal/prompt" ) var authCmd = &cobra.Command{ @@ -128,13 +129,13 @@ func runAuthFlow(provider auth.Provider) error { return fmt.Errorf("initialize credential storage: %w", err) } - prompter := auth.NewTerminalPrompter() + prompter := prompt.New() info := provider.Info() prompter.Print(fmt.Sprintf("Configure %s authentication", info.Name)) prompter.Print("") - choice, err := prompter.PromptChoice("Authentication method:", []string{ + choice, err := prompter.Choice("Authentication method:", []string{ "Subscription", "API Key", }) @@ -169,7 +170,7 @@ func runAuthFlow(provider auth.Provider) error { // handleSubscriptionAuth handles subscription-based authentication. // For Claude, prompts for manual token entry. // For Gemini/Codex, attempts to read existing credentials from config files. -func handleSubscriptionAuth(provider auth.Provider, prompter auth.Prompter) (auth.Credential, error) { +func handleSubscriptionAuth(provider auth.Provider, prompter prompt.Prompter) (auth.Credential, error) { // Try to auto-detect existing credentials value, err := provider.CheckSubscription() if err == nil { @@ -188,7 +189,7 @@ func handleSubscriptionAuth(provider auth.Provider, prompter auth.Prompter) (aut prompter.Print(err.Error()) prompter.Print("") - value, err = prompter.PromptSecret("Paste your credential: ") + value, err = prompter.Secret("Paste your credential: ") if err != nil { return auth.Credential{}, fmt.Errorf("read credential: %w", err) } @@ -204,13 +205,13 @@ func handleSubscriptionAuth(provider auth.Provider, prompter auth.Prompter) (aut } // handleAPIKeyAuth handles API key authentication. -func handleAPIKeyAuth(provider auth.Provider, prompter auth.Prompter) (auth.Credential, error) { +func handleAPIKeyAuth(provider auth.Provider, prompter prompt.Prompter) (auth.Credential, error) { info := provider.Info() prompter.Print(fmt.Sprintf("Enter your %s API key.", info.Name)) prompter.Print("") - value, err := prompter.PromptSecret("API key: ") + value, err := prompter.Secret("API key: ") if err != nil { return auth.Credential{}, fmt.Errorf("read API key: %w", err) } diff --git a/internal/cmd/run.go b/internal/cmd/run.go index 1e0862c..7370d1f 100644 --- a/internal/cmd/run.go +++ b/internal/cmd/run.go @@ -3,13 +3,13 @@ package cmd import ( "errors" "fmt" - "os/exec" "github.com/spf13/cobra" "github.com/jmgilman/headjack/internal/container" "github.com/jmgilman/headjack/internal/devcontainer" "github.com/jmgilman/headjack/internal/instance" + "github.com/jmgilman/headjack/internal/prompt" ) var runCmd = &cobra.Command{ @@ -131,14 +131,11 @@ func getOrCreateInstance(cmd *cobra.Command, mgr *instance.Manager, repoPath, br return inst, nil } -// devcontainerCLI is the name of the devcontainer CLI binary. -const devcontainerCLI = "devcontainer" - // buildCreateConfig builds the instance creation config, detecting devcontainer mode if applicable. // Devcontainer mode is used when: // - No --image flag was explicitly passed (imageExplicit is false) // - A devcontainer.json exists in the repo -// - The devcontainer CLI is available +// - The devcontainer CLI is available (or can be installed) // // Returns an error if no devcontainer.json is found and no image is configured. func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, imageExplicit bool) (instance.CreateConfig, error) { @@ -147,14 +144,6 @@ func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, image Image: image, } - // Always check if devcontainer CLI is available and warn if not - devcontainerAvailable := isDevcontainerCLIAvailable() - if !devcontainerAvailable { - fmt.Println("Warning: devcontainer CLI not found in PATH") - fmt.Println(" Install with: npm install -g @devcontainers/cli") - fmt.Println(" See: https://github.com/devcontainers/cli") - } - // If image was explicitly passed, use vanilla mode if imageExplicit { return cfg, nil @@ -164,9 +153,21 @@ func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, image hasDevcontainer := devcontainer.HasConfig(repoPath) if hasDevcontainer { - if !devcontainerAvailable { - // Devcontainer exists but CLI not available - error - return cfg, errors.New("devcontainer.json found but devcontainer CLI is not installed") + // Resolve devcontainer CLI (may prompt for installation) + mgr := ManagerFromContext(cmd.Context()) + if mgr == nil { + return cfg, errors.New("manager not available") + } + + loader := LoaderFromContext(cmd.Context()) + if loader == nil { + return cfg, errors.New("config loader not available") + } + + resolver := devcontainer.NewCLIResolver(loader, prompt.New(), mgr.Executor()) + cliPath, err := resolver.Resolve(cmd.Context()) + if err != nil { + return cfg, err } // Create devcontainer runtime wrapping the underlying runtime @@ -174,7 +175,7 @@ func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, image if appCfg := ConfigFromContext(cmd.Context()); appCfg != nil && appCfg.Runtime.Name != "" { runtimeName = appCfg.Runtime.Name } - dcRuntime := createDevcontainerRuntime(cmd, runtimeName) + dcRuntime := createDevcontainerRuntime(cmd, runtimeName, cliPath) if dcRuntime == nil { return cfg, errors.New("failed to create devcontainer runtime") } @@ -196,14 +197,8 @@ func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, image return cfg, nil } -// isDevcontainerCLIAvailable checks if the devcontainer CLI is in PATH. -func isDevcontainerCLIAvailable() bool { - _, err := exec.LookPath(devcontainerCLI) - return err == nil -} - // createDevcontainerRuntime creates a DevcontainerRuntime wrapping the appropriate underlying runtime. -func createDevcontainerRuntime(cmd *cobra.Command, runtimeName string) container.Runtime { +func createDevcontainerRuntime(cmd *cobra.Command, runtimeName, cliPath string) container.Runtime { // Get the underlying runtime from the manager mgr := ManagerFromContext(cmd.Context()) if mgr == nil { @@ -224,7 +219,7 @@ func createDevcontainerRuntime(cmd *cobra.Command, runtimeName string) container return devcontainer.NewRuntime( mgr.Runtime(), mgr.Executor(), - "devcontainer", // CLI path - assumes it's in PATH + cliPath, dockerPath, ) } diff --git a/internal/config/config.go b/internal/config/config.go index 36d09a0..d792f4e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -54,10 +54,11 @@ var validate = validator.New() // Config represents the full Headjack configuration. type Config struct { - Default DefaultConfig `mapstructure:"default" validate:"required"` - Agents map[string]AgentConfig `mapstructure:"agents" validate:"dive,keys,oneof=claude gemini codex,endkeys"` - Storage StorageConfig `mapstructure:"storage" validate:"required"` - Runtime RuntimeConfig `mapstructure:"runtime"` + Default DefaultConfig `mapstructure:"default" validate:"required"` + Agents map[string]AgentConfig `mapstructure:"agents" validate:"dive,keys,oneof=claude gemini codex,endkeys"` + Storage StorageConfig `mapstructure:"storage" validate:"required"` + Runtime RuntimeConfig `mapstructure:"runtime"` + Devcontainer DevcontainerConfig `mapstructure:"devcontainer"` } // DefaultConfig holds default values for new instances. @@ -84,6 +85,11 @@ type RuntimeConfig struct { Flags map[string]any `mapstructure:"flags"` } +// DevcontainerConfig holds devcontainer CLI configuration. +type DevcontainerConfig struct { + Path string `mapstructure:"path"` +} + // Validate checks the configuration for errors using struct tags. func (c *Config) Validate() error { if err := validate.Struct(c); err != nil { @@ -150,6 +156,7 @@ func (l *Loader) setDefaults() { l.v.SetDefault("agents.codex.env", map[string]string{}) l.v.SetDefault("runtime.name", "docker") l.v.SetDefault("runtime.flags", map[string]any{}) + l.v.SetDefault("devcontainer.path", "") } // Load reads the configuration file, creating defaults if it doesn't exist. @@ -175,6 +182,7 @@ func (l *Loader) Load() (*Config, error) { cfg.Storage.Worktrees = l.expandPath(cfg.Storage.Worktrees) cfg.Storage.Catalog = l.expandPath(cfg.Storage.Catalog) cfg.Storage.Logs = l.expandPath(cfg.Storage.Logs) + cfg.Devcontainer.Path = l.expandPath(cfg.Devcontainer.Path) return &cfg, nil } diff --git a/internal/devcontainer/cli.go b/internal/devcontainer/cli.go new file mode 100644 index 0000000..43d16c0 --- /dev/null +++ b/internal/devcontainer/cli.go @@ -0,0 +1,208 @@ +package devcontainer + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/jmgilman/headjack/internal/config" + "github.com/jmgilman/headjack/internal/exec" + "github.com/jmgilman/headjack/internal/prompt" +) + +const ( + // devcontainerBin is the name of the devcontainer CLI binary. + devcontainerBin = "devcontainer" + + // npmBin is the name of the npm binary. + npmBin = "npm" + + // devcontainerPackage is the npm package name for the devcontainer CLI. + devcontainerPackage = "@devcontainers/cli" + + // cliInstallDir is the subdirectory under XDG data dir for CLI installation. + cliInstallDir = "devcontainer-cli" +) + +// CLIResolver resolves the path to the devcontainer CLI, offering to install +// it locally if not found in PATH or config. +type CLIResolver struct { + loader *config.Loader + prompter prompt.Prompter + executor exec.Executor + homeDir string +} + +// NewCLIResolver creates a new CLIResolver. +func NewCLIResolver(loader *config.Loader, prompter prompt.Prompter, executor exec.Executor) *CLIResolver { + homeDir, err := os.UserHomeDir() + if err != nil { + // Fall back to empty string; installDir will fail gracefully + homeDir = "" + } + return &CLIResolver{ + loader: loader, + prompter: prompter, + executor: executor, + homeDir: homeDir, + } +} + +// Resolve returns the path to the devcontainer CLI binary. +// It checks in order: +// 1. devcontainer in PATH +// 2. devcontainer.path from config (if set and binary exists) +// 3. Offers to install via npm if available +// +// Returns an error if the CLI cannot be found or installed. +func (r *CLIResolver) Resolve(ctx context.Context) (string, error) { + // 1. Check if devcontainer is in PATH + if path, err := r.executor.LookPath(devcontainerBin); err == nil { + return path, nil + } + + // 2. Check if devcontainer.path is set in config + cfg, err := r.loader.Load() + if err != nil { + return "", fmt.Errorf("load config: %w", err) + } + + if cfg.Devcontainer.Path != "" { + if _, statErr := os.Stat(cfg.Devcontainer.Path); statErr == nil { + return cfg.Devcontainer.Path, nil + } + // Path is set but binary doesn't exist - fall through to install + } + + // 3. Check if npm is available + if _, lookErr := r.executor.LookPath(npmBin); lookErr != nil { + return "", r.noNpmError() + } + + // 4. Prompt user to install + confirmed, err := r.prompter.Confirm( + "Install devcontainer CLI?", + "The devcontainer CLI is required to use devcontainer.json configurations.\nWould you like to install it locally via npm?", + ) + if err != nil { + return "", fmt.Errorf("prompt: %w", err) + } + + if !confirmed { + return "", r.declinedError() + } + + // 5. Install the CLI + path, err := r.install(ctx) + if err != nil { + return "", err + } + + // 6. Save path to config + if err := r.loader.Set("devcontainer.path", path); err != nil { + // Non-fatal: CLI is installed but config wasn't saved + r.prompter.Print(fmt.Sprintf("Warning: could not save config: %v", err)) + } + + return path, nil +} + +// install installs the devcontainer CLI via npm and returns the path to the binary. +func (r *CLIResolver) install(ctx context.Context) (string, error) { + installDir := r.installDir() + binPath := filepath.Join(installDir, "node_modules", ".bin", devcontainerBin) + + // Track success for cleanup + success := false + defer func() { + if !success { + _ = os.RemoveAll(installDir) + } + }() + + // Create install directory + if err := os.MkdirAll(installDir, 0o750); err != nil { + return "", fmt.Errorf("create install directory: %w", err) + } + + r.prompter.Print("Installing devcontainer CLI...") + + // Run npm install + result, err := r.executor.Run(ctx, &exec.RunOptions{ + Name: npmBin, + Args: []string{"install", "--prefix", installDir, devcontainerPackage}, + }) + if err != nil { + stderr := "" + if result != nil { + stderr = strings.TrimSpace(string(result.Stderr)) + } + if stderr != "" { + return "", fmt.Errorf("npm install failed: %s", stderr) + } + return "", fmt.Errorf("npm install failed: %w", err) + } + + // Validate the installation + if err := r.validate(ctx, binPath); err != nil { + return "", fmt.Errorf("validate installation: %w", err) + } + + r.prompter.Print("devcontainer CLI installed successfully.") + + success = true + return binPath, nil +} + +// validate verifies that the devcontainer CLI is working by running --version. +func (r *CLIResolver) validate(ctx context.Context, cliPath string) error { + result, err := r.executor.Run(ctx, &exec.RunOptions{ + Name: cliPath, + Args: []string{"--version"}, + }) + if err != nil { + stderr := "" + if result != nil { + stderr = strings.TrimSpace(string(result.Stderr)) + } + if stderr != "" { + return fmt.Errorf("devcontainer --version failed: %s", stderr) + } + return fmt.Errorf("devcontainer --version failed: %w", err) + } + return nil +} + +// installDir returns the directory where the CLI should be installed. +func (r *CLIResolver) installDir() string { + return filepath.Join(r.homeDir, ".local", "share", "headjack", cliInstallDir) +} + +// noNpmError returns an error with instructions when npm is not available. +func (r *CLIResolver) noNpmError() error { + return errors.New(`devcontainer CLI not found + +The devcontainer CLI is required to use devcontainer.json configurations. + +To install: + npm install -g @devcontainers/cli + +Or install Node.js first: + https://nodejs.org/ + +See: https://github.com/devcontainers/cli`) +} + +// declinedError returns an error with instructions when user declines installation. +func (r *CLIResolver) declinedError() error { + return errors.New(`devcontainer CLI not found + +To install manually: + npm install -g @devcontainers/cli + +Or use a container image instead: + hjk run --image `) +} diff --git a/internal/prompt/mocks/prompter.go b/internal/prompt/mocks/prompter.go new file mode 100644 index 0000000..46e3540 --- /dev/null +++ b/internal/prompt/mocks/prompter.go @@ -0,0 +1,220 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package mocks + +import ( + "sync" + + "github.com/jmgilman/headjack/internal/prompt" +) + +// Ensure, that PrompterMock does implement prompt.Prompter. +// If this is not the case, regenerate this file with moq. +var _ prompt.Prompter = &PrompterMock{} + +// PrompterMock is a mock implementation of prompt.Prompter. +// +// func TestSomethingThatUsesPrompter(t *testing.T) { +// +// // make and configure a mocked prompt.Prompter +// mockedPrompter := &PrompterMock{ +// ChoiceFunc: func(prompt string, options []string) (int, error) { +// panic("mock out the Choice method") +// }, +// ConfirmFunc: func(title string, description string) (bool, error) { +// panic("mock out the Confirm method") +// }, +// PrintFunc: func(message string) { +// panic("mock out the Print method") +// }, +// SecretFunc: func(prompt string) (string, error) { +// panic("mock out the Secret method") +// }, +// } +// +// // use mockedPrompter in code that requires prompt.Prompter +// // and then make assertions. +// +// } +type PrompterMock struct { + // ChoiceFunc mocks the Choice method. + ChoiceFunc func(prompt string, options []string) (int, error) + + // ConfirmFunc mocks the Confirm method. + ConfirmFunc func(title string, description string) (bool, error) + + // PrintFunc mocks the Print method. + PrintFunc func(message string) + + // SecretFunc mocks the Secret method. + SecretFunc func(prompt string) (string, error) + + // calls tracks calls to the methods. + calls struct { + // Choice holds details about calls to the Choice method. + Choice []struct { + // Prompt is the prompt argument value. + Prompt string + // Options is the options argument value. + Options []string + } + // Confirm holds details about calls to the Confirm method. + Confirm []struct { + // Title is the title argument value. + Title string + // Description is the description argument value. + Description string + } + // Print holds details about calls to the Print method. + Print []struct { + // Message is the message argument value. + Message string + } + // Secret holds details about calls to the Secret method. + Secret []struct { + // Prompt is the prompt argument value. + Prompt string + } + } + lockChoice sync.RWMutex + lockConfirm sync.RWMutex + lockPrint sync.RWMutex + lockSecret sync.RWMutex +} + +// Choice calls ChoiceFunc. +func (mock *PrompterMock) Choice(prompt string, options []string) (int, error) { + if mock.ChoiceFunc == nil { + panic("PrompterMock.ChoiceFunc: method is nil but Prompter.Choice was just called") + } + callInfo := struct { + Prompt string + Options []string + }{ + Prompt: prompt, + Options: options, + } + mock.lockChoice.Lock() + mock.calls.Choice = append(mock.calls.Choice, callInfo) + mock.lockChoice.Unlock() + return mock.ChoiceFunc(prompt, options) +} + +// ChoiceCalls gets all the calls that were made to Choice. +// Check the length with: +// +// len(mockedPrompter.ChoiceCalls()) +func (mock *PrompterMock) ChoiceCalls() []struct { + Prompt string + Options []string +} { + var calls []struct { + Prompt string + Options []string + } + mock.lockChoice.RLock() + calls = mock.calls.Choice + mock.lockChoice.RUnlock() + return calls +} + +// Confirm calls ConfirmFunc. +func (mock *PrompterMock) Confirm(title string, description string) (bool, error) { + if mock.ConfirmFunc == nil { + panic("PrompterMock.ConfirmFunc: method is nil but Prompter.Confirm was just called") + } + callInfo := struct { + Title string + Description string + }{ + Title: title, + Description: description, + } + mock.lockConfirm.Lock() + mock.calls.Confirm = append(mock.calls.Confirm, callInfo) + mock.lockConfirm.Unlock() + return mock.ConfirmFunc(title, description) +} + +// ConfirmCalls gets all the calls that were made to Confirm. +// Check the length with: +// +// len(mockedPrompter.ConfirmCalls()) +func (mock *PrompterMock) ConfirmCalls() []struct { + Title string + Description string +} { + var calls []struct { + Title string + Description string + } + mock.lockConfirm.RLock() + calls = mock.calls.Confirm + mock.lockConfirm.RUnlock() + return calls +} + +// Print calls PrintFunc. +func (mock *PrompterMock) Print(message string) { + if mock.PrintFunc == nil { + panic("PrompterMock.PrintFunc: method is nil but Prompter.Print was just called") + } + callInfo := struct { + Message string + }{ + Message: message, + } + mock.lockPrint.Lock() + mock.calls.Print = append(mock.calls.Print, callInfo) + mock.lockPrint.Unlock() + mock.PrintFunc(message) +} + +// PrintCalls gets all the calls that were made to Print. +// Check the length with: +// +// len(mockedPrompter.PrintCalls()) +func (mock *PrompterMock) PrintCalls() []struct { + Message string +} { + var calls []struct { + Message string + } + mock.lockPrint.RLock() + calls = mock.calls.Print + mock.lockPrint.RUnlock() + return calls +} + +// Secret calls SecretFunc. +func (mock *PrompterMock) Secret(prompt string) (string, error) { + if mock.SecretFunc == nil { + panic("PrompterMock.SecretFunc: method is nil but Prompter.Secret was just called") + } + callInfo := struct { + Prompt string + }{ + Prompt: prompt, + } + mock.lockSecret.Lock() + mock.calls.Secret = append(mock.calls.Secret, callInfo) + mock.lockSecret.Unlock() + return mock.SecretFunc(prompt) +} + +// SecretCalls gets all the calls that were made to Secret. +// Check the length with: +// +// len(mockedPrompter.SecretCalls()) +func (mock *PrompterMock) SecretCalls() []struct { + Prompt string +} { + var calls []struct { + Prompt string + } + mock.lockSecret.RLock() + calls = mock.calls.Secret + mock.lockSecret.RUnlock() + return calls +} diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go new file mode 100644 index 0000000..65e9bb5 --- /dev/null +++ b/internal/prompt/prompt.go @@ -0,0 +1,115 @@ +// Package prompt provides user interaction primitives using charmbracelet/huh. +package prompt + +import ( + "errors" + "fmt" + "strings" + + "github.com/charmbracelet/huh" +) + +// ErrCanceled is returned when the user cancels a prompt. +var ErrCanceled = errors.New("canceled by user") + +// Prompter abstracts user interaction for testability. +// +//go:generate go run github.com/matryer/moq@latest -pkg mocks -out mocks/prompter.go . Prompter +type Prompter interface { + // Print outputs text to the user. + Print(message string) + + // Confirm prompts for yes/no confirmation. + Confirm(title, description string) (bool, error) + + // Secret prompts for secret input (no echo). + Secret(prompt string) (string, error) + + // Choice prompts user to select from options, returns 0-based index. + Choice(prompt string, options []string) (int, error) +} + +// HuhPrompter implements Prompter using charmbracelet/huh for interactive forms. +type HuhPrompter struct{} + +// New creates a new HuhPrompter for interactive terminal prompts. +func New() *HuhPrompter { + return &HuhPrompter{} +} + +// Print outputs text to the user. +func (p *HuhPrompter) Print(message string) { + fmt.Println(message) +} + +// Confirm prompts for yes/no confirmation. +func (p *HuhPrompter) Confirm(title, description string) (bool, error) { + var confirmed bool + + err := huh.NewConfirm(). + Title(title). + Description(description). + Affirmative("Yes"). + Negative("No"). + Value(&confirmed). + Run() + + if err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return false, ErrCanceled + } + return false, fmt.Errorf("confirm prompt: %w", err) + } + + return confirmed, nil +} + +// Secret prompts for secret input with masked display. +func (p *HuhPrompter) Secret(prompt string) (string, error) { + var value string + + err := huh.NewInput(). + Title(prompt). + EchoMode(huh.EchoModePassword). + Value(&value). + Run() + + if err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return "", ErrCanceled + } + return "", fmt.Errorf("secret prompt: %w", err) + } + + return strings.TrimSpace(value), nil +} + +// Choice prompts user to select from options and returns the 0-based index. +func (p *HuhPrompter) Choice(prompt string, options []string) (int, error) { + if len(options) == 0 { + return 0, errors.New("no options provided") + } + + // Build huh options with display labels and index values + huhOptions := make([]huh.Option[int], len(options)) + for i, opt := range options { + huhOptions[i] = huh.NewOption(opt, i) + } + + var selected int + + err := huh.NewSelect[int](). + Title(prompt). + Options(huhOptions...). + Value(&selected). + Run() + + if err != nil { + if errors.Is(err, huh.ErrUserAborted) { + return 0, ErrCanceled + } + return 0, fmt.Errorf("choice prompt: %w", err) + } + + return selected, nil +} From 61925272ebb147ecbd4a9b4a296c19f131f3ee5c Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Mon, 5 Jan 2026 07:15:59 -0800 Subject: [PATCH 2/2] feat(cli): add passthrough flags for run and agent commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current behavior: Runtime flags required complex map[string]any configuration in config file. No way to pass flags directly via CLI to container runtime or agent CLIs. Devcontainer exec didn't stream stdout/stderr for non-interactive commands. Tmux sessions leaked escape sequences after short-lived commands. New behavior: - Runtime flags simplified to []string in config - `hjk run -- ` passes flags to container runtime - `hjk agent -- ` passes flags to agent CLI - Config flags merged with CLI flags (config first, CLI appended) - Devcontainer exec streams stdout/stderr for non-interactive mode - Tmux drains stdin with timeout to catch late escape sequences - Removed internal/flags package (no longer needed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/cmd/agent.go | 39 +++-- internal/cmd/helpers.go | 23 +++ internal/cmd/root.go | 25 +-- internal/cmd/run.go | 37 ++-- internal/config/config.go | 19 +- internal/devcontainer/runtime.go | 25 ++- internal/flags/flags.go | 132 -------------- internal/flags/flags_test.go | 279 ------------------------------ internal/instance/instance.go | 1 + internal/instance/manager.go | 33 +++- internal/instance/manager_test.go | 96 +++++----- internal/multiplexer/tmux.go | 35 +++- 12 files changed, 215 insertions(+), 529 deletions(-) delete mode 100644 internal/flags/flags.go delete mode 100644 internal/flags/flags_test.go diff --git a/internal/cmd/agent.go b/internal/cmd/agent.go index c6a1ad6..d0130fa 100644 --- a/internal/cmd/agent.go +++ b/internal/cmd/agent.go @@ -14,7 +14,7 @@ import ( ) var agentCmd = &cobra.Command{ - Use: "agent [agent_name]", + Use: "agent [agent_name] [-- ...]", Short: "Start an agent session in an existing instance", Long: `Start an agent session within an existing instance for the specified branch. @@ -25,6 +25,8 @@ to it unless --detached is specified. If agent_name is not specified, the default agent from configuration is used. Set the default with 'hjk config default.agent '. +Additional flags can be passed to the agent CLI by placing them after a -- separator. + All session output is captured to a log file regardless of attached/detached mode.`, Example: ` # Start default agent on existing instance hjk agent feat/auth @@ -40,8 +42,11 @@ All session output is captured to a log file regardless of attached/detached mod hjk agent feat/auth gemini --name auth-session # Start agent in detached mode (run in background) - hjk agent feat/auth -d --prompt "Refactor the auth module"`, - Args: cobra.RangeArgs(1, 2), + hjk agent feat/auth -d --prompt "Refactor the auth module" + + # Pass additional flags to the agent CLI + hjk agent feat/auth claude -- --dangerously-skip-permissions`, + Args: cobra.MinimumNArgs(1), RunE: runAgentCmd, } @@ -50,10 +55,11 @@ type agentFlags struct { sessionName string detached bool prompt string + agentFlags []string // flags to pass to the agent CLI (after --) } // parseAgentFlags extracts and validates flags from the command. -func parseAgentFlags(cmd *cobra.Command) (*agentFlags, error) { +func parseAgentFlags(cmd *cobra.Command, args []string) (*agentFlags, error) { sessionName, err := cmd.Flags().GetString("name") if err != nil { return nil, fmt.Errorf("get name flag: %w", err) @@ -71,6 +77,7 @@ func parseAgentFlags(cmd *cobra.Command) (*agentFlags, error) { sessionName: sessionName, detached: detached, prompt: prompt, + agentFlags: parsePassthroughArgs(cmd, args), }, nil } @@ -136,17 +143,22 @@ func injectAuthCredential(agent string, cfg *instance.CreateSessionConfig) error } // buildAgentCommand builds the command for launching an agent. -func buildAgentCommand(agent, prompt string) []string { +// The command is: agent [prompt] [flags...] +func buildAgentCommand(agent, prompt string, flags []string) []string { cmd := []string{agent} if prompt != "" { cmd = append(cmd, prompt) } + cmd = append(cmd, flags...) return cmd } // resolveAgentName determines the agent name from args or config default. -func resolveAgentName(ctx context.Context, args []string) (string, error) { - if len(args) > 1 { +// dashIdx is the index of the -- separator (-1 if not present). +func resolveAgentName(ctx context.Context, args []string, dashIdx int) (string, error) { + // Check if args[1] exists and is before the -- separator (or no separator) + hasAgentArg := len(args) > 1 && (dashIdx < 0 || dashIdx > 1) + if hasAgentArg { return args[1], nil } @@ -173,12 +185,12 @@ func runAgentCmd(cmd *cobra.Command, args []string) error { return err } - flags, err := parseAgentFlags(cmd) + flags, err := parseAgentFlags(cmd, args) if err != nil { return err } - agentName, err := resolveAgentName(cmd.Context(), args) + agentName, err := resolveAgentName(cmd.Context(), args, cmd.ArgsLenAtDash()) if err != nil { return err } @@ -194,11 +206,18 @@ func runAgentCmd(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid agent %q (valid: %s)", agentName, formatList(config.ValidAgentNames())) } + // Merge config flags with CLI flags + var configFlags []string + if loader := LoaderFromContext(cmd.Context()); loader != nil { + configFlags = loader.GetAgentFlags(agentName) + } + mergedFlags := mergeFlags(configFlags, flags.agentFlags) + // Build session config sessionCfg := &instance.CreateSessionConfig{ Type: agentName, Name: flags.sessionName, - Command: buildAgentCommand(agentName, flags.prompt), + Command: buildAgentCommand(agentName, flags.prompt, mergedFlags), } // Inject agent-specific environment variables from config diff --git a/internal/cmd/helpers.go b/internal/cmd/helpers.go index cbb30bd..92503e9 100644 --- a/internal/cmd/helpers.go +++ b/internal/cmd/helpers.go @@ -73,6 +73,29 @@ func formatNotRunningHint(cmd *cobra.Command, err *instance.NotRunningError) str return fmt.Sprintf("container %s is %s; check logs with `%s`", err.ContainerID, err.Status, logsCmd) } +// parsePassthroughArgs extracts arguments after the -- separator from a cobra command. +// Returns nil if no -- separator was used. +func parsePassthroughArgs(cmd *cobra.Command, args []string) []string { + dashIdx := cmd.ArgsLenAtDash() + if dashIdx < 0 || dashIdx >= len(args) { + return nil + } + return args[dashIdx:] +} + +// mergeFlags combines base flags with additional flags. +// Base flags come first, additional flags are appended. +// Returns nil if both inputs are empty. +func mergeFlags(base, additional []string) []string { + if len(base) == 0 && len(additional) == 0 { + return nil + } + result := make([]string, 0, len(base)+len(additional)) + result = append(result, base...) + result = append(result, additional...) + return result +} + // getInstanceByBranch gets an existing instance by branch, returning an error with hint if not found. // If the instance is stopped, it will be automatically restarted. func getInstanceByBranch(ctx context.Context, mgr *instance.Manager, branch string) (*instance.Instance, error) { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 38af073..18e0037 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -17,7 +17,6 @@ import ( "github.com/jmgilman/headjack/internal/config" "github.com/jmgilman/headjack/internal/container" hjexec "github.com/jmgilman/headjack/internal/exec" - "github.com/jmgilman/headjack/internal/flags" "github.com/jmgilman/headjack/internal/git" "github.com/jmgilman/headjack/internal/instance" "github.com/jmgilman/headjack/internal/multiplexer" @@ -178,17 +177,11 @@ func initManager() error { // Map runtime name to RuntimeType runtimeType := runtimeNameToType(runtimeName) - // Parse config flags - configFlags, err := getConfigFlags() - if err != nil { - return err - } - - mgr = instance.NewManager(store, runtime, opener, mux, instance.ManagerConfig{ + mgr = instance.NewManager(store, runtime, opener, mux, &instance.ManagerConfig{ WorktreesDir: worktreesDir, LogsDir: logsDir, RuntimeType: runtimeType, - ConfigFlags: configFlags, + ConfigFlags: getConfigFlags(), Executor: executor, }) @@ -205,16 +198,12 @@ func runtimeNameToType(name string) instance.RuntimeType { } } -// getConfigFlags parses runtime flags from config. -func getConfigFlags() (flags.Flags, error) { - if appConfig == nil || appConfig.Runtime.Flags == nil { - return make(flags.Flags), nil - } - configFlags, err := flags.FromConfig(appConfig.Runtime.Flags) - if err != nil { - return nil, fmt.Errorf("parse runtime flags: %w", err) +// getConfigFlags returns runtime flags from config. +func getConfigFlags() []string { + if appConfig == nil { + return nil } - return configFlags, nil + return appConfig.Runtime.Flags } // formatList joins strings with commas and "and" before the last item. diff --git a/internal/cmd/run.go b/internal/cmd/run.go index 7370d1f..60c2305 100644 --- a/internal/cmd/run.go +++ b/internal/cmd/run.go @@ -13,7 +13,7 @@ import ( ) var runCmd = &cobra.Command{ - Use: "run ", + Use: "run [-- ...]", Short: "Create a new instance for the specified branch", Long: `Create a new instance (worktree + container) for the specified branch. @@ -25,6 +25,9 @@ stopped). The container environment is determined by: 2. Base image: Use --image to specify a container image directly, bypassing devcontainer detection. +Additional flags can be passed to the container runtime (or devcontainer CLI) +by placing them after a -- separator. + This command only creates the instance. To start a session, use: - 'hjk agent ' to start an agent session - 'hjk exec ' to start a shell session`, @@ -34,21 +37,25 @@ This command only creates the instance. To start a session, use: # Use a specific container image (bypasses devcontainer) hjk run feat/auth --image my-registry.io/custom-image:latest + # Pass additional flags to the container runtime + hjk run feat/auth -- --memory=4g --privileged + # Typical workflow: create instance, then start agent hjk run feat/auth hjk agent feat/auth claude --prompt "Implement JWT authentication"`, - Args: cobra.ExactArgs(1), + Args: cobra.MinimumNArgs(1), RunE: runRunCmd, } // runFlags holds parsed flags for the run command. type runFlags struct { image string - imageExplicit bool // true if --image was explicitly passed + imageExplicit bool // true if --image was explicitly passed + runtimeFlags []string // flags to pass to the container runtime (after --) } // parseRunFlags extracts and validates flags from the command. -func parseRunFlags(cmd *cobra.Command) (*runFlags, error) { +func parseRunFlags(cmd *cobra.Command, args []string) (*runFlags, error) { image, err := cmd.Flags().GetString("image") if err != nil { return nil, fmt.Errorf("get image flag: %w", err) @@ -60,6 +67,7 @@ func parseRunFlags(cmd *cobra.Command) (*runFlags, error) { return &runFlags{ image: image, imageExplicit: imageExplicit, + runtimeFlags: parsePassthroughArgs(cmd, args), }, nil } @@ -71,7 +79,7 @@ func runRunCmd(cmd *cobra.Command, args []string) error { return err } - flags, err := parseRunFlags(cmd) + flags, err := parseRunFlags(cmd, args) if err != nil { return err } @@ -81,7 +89,7 @@ func runRunCmd(cmd *cobra.Command, args []string) error { return err } - inst, err := getOrCreateInstance(cmd, mgr, repoPath, branch, flags.image, flags.imageExplicit) + inst, err := getOrCreateInstance(cmd, mgr, repoPath, branch, flags) if err != nil { return err } @@ -93,7 +101,7 @@ func runRunCmd(cmd *cobra.Command, args []string) error { // getOrCreateInstance retrieves an existing instance or creates a new one. // If the instance exists but is stopped, it restarts the container. // If imageExplicit is false and a devcontainer.json exists, devcontainer mode is used. -func getOrCreateInstance(cmd *cobra.Command, mgr *instance.Manager, repoPath, branch, image string, imageExplicit bool) (*instance.Instance, error) { +func getOrCreateInstance(cmd *cobra.Command, mgr *instance.Manager, repoPath, branch string, flags *runFlags) (*instance.Instance, error) { // Try to get existing instance inst, err := mgr.GetByBranch(cmd.Context(), repoPath, branch) if err == nil { @@ -116,13 +124,13 @@ func getOrCreateInstance(cmd *cobra.Command, mgr *instance.Manager, repoPath, br } // Build create config - detect devcontainer mode if applicable - createCfg, err := buildCreateConfig(cmd, repoPath, branch, image, imageExplicit) + createCfg, err := buildCreateConfig(cmd, repoPath, branch, flags) if err != nil { return nil, err } // Create new instance - inst, err = mgr.Create(cmd.Context(), repoPath, createCfg) + inst, err = mgr.Create(cmd.Context(), repoPath, &createCfg) if err != nil { return nil, fmt.Errorf("create instance: %w", err) } @@ -138,14 +146,15 @@ func getOrCreateInstance(cmd *cobra.Command, mgr *instance.Manager, repoPath, br // - The devcontainer CLI is available (or can be installed) // // Returns an error if no devcontainer.json is found and no image is configured. -func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, imageExplicit bool) (instance.CreateConfig, error) { +func buildCreateConfig(cmd *cobra.Command, repoPath, branch string, flags *runFlags) (instance.CreateConfig, error) { cfg := instance.CreateConfig{ - Branch: branch, - Image: image, + Branch: branch, + Image: flags.image, + RuntimeFlags: flags.runtimeFlags, } // If image was explicitly passed, use vanilla mode - if imageExplicit { + if flags.imageExplicit { return cfg, nil } @@ -190,7 +199,7 @@ func buildCreateConfig(cmd *cobra.Command, repoPath, branch, image string, image } // No devcontainer.json - need an image - if image == "" { + if flags.image == "" { return cfg, errors.New("no devcontainer.json found and no image configured\n\nTo fix this, either:\n 1. Add a devcontainer.json to your repository\n 2. Use --image to specify a container image\n 3. Set default.base_image in your config") } diff --git a/internal/config/config.go b/internal/config/config.go index d792f4e..4102f61 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -69,7 +69,8 @@ type DefaultConfig struct { // AgentConfig holds agent-specific configuration. type AgentConfig struct { - Env map[string]string `mapstructure:"env"` + Env map[string]string `mapstructure:"env"` + Flags []string `mapstructure:"flags"` } // StorageConfig holds storage location configuration. @@ -81,8 +82,8 @@ type StorageConfig struct { // RuntimeConfig holds container runtime configuration. type RuntimeConfig struct { - Name string `mapstructure:"name" validate:"omitempty,oneof=podman docker"` - Flags map[string]any `mapstructure:"flags"` + Name string `mapstructure:"name" validate:"omitempty,oneof=podman docker"` + Flags []string `mapstructure:"flags"` } // DevcontainerConfig holds devcontainer CLI configuration. @@ -152,10 +153,13 @@ func (l *Loader) setDefaults() { l.v.SetDefault("storage.catalog", "~/.local/share/headjack/catalog.json") l.v.SetDefault("storage.logs", "~/.local/share/headjack/logs") l.v.SetDefault("agents.claude.env", map[string]string{"CLAUDE_CODE_MAX_TURNS": "100"}) + l.v.SetDefault("agents.claude.flags", []string{}) l.v.SetDefault("agents.gemini.env", map[string]string{}) + l.v.SetDefault("agents.gemini.flags", []string{}) l.v.SetDefault("agents.codex.env", map[string]string{}) + l.v.SetDefault("agents.codex.flags", []string{}) l.v.SetDefault("runtime.name", "docker") - l.v.SetDefault("runtime.flags", map[string]any{}) + l.v.SetDefault("runtime.flags", []string{}) l.v.SetDefault("devcontainer.path", "") } @@ -211,6 +215,13 @@ func (l *Loader) GetAgentEnv(agent string) map[string]string { return raw } +// GetAgentFlags returns the CLI flags for a specific agent. +// Returns nil if the agent has no flags configuration. +func (l *Loader) GetAgentFlags(agent string) []string { + key := fmt.Sprintf("agents.%s.flags", agent) + return l.v.GetStringSlice(key) +} + // Set sets a configuration value by dot-notation key. func (l *Loader) Set(key, value string) error { if err := ValidateKey(key); err != nil { diff --git a/internal/devcontainer/runtime.go b/internal/devcontainer/runtime.go index 872f612..942ac88 100644 --- a/internal/devcontainer/runtime.go +++ b/internal/devcontainer/runtime.go @@ -58,6 +58,9 @@ func (r *Runtime) Run(ctx context.Context, cfg *container.RunConfig) (*container "--docker-path", r.dockerPath, } + // Append any additional flags (passed via --) + args = append(args, cfg.Flags...) + result, err := r.exec.Run(ctx, &exec.RunOptions{ Name: r.cliPath, Args: args, @@ -136,22 +139,14 @@ func (r *Runtime) Exec(ctx context.Context, id string, cfg *container.ExecConfig return r.execInteractive(ctx, args) } - result, err := r.exec.Run(ctx, &exec.RunOptions{ - Name: r.cliPath, - Args: args, + // Non-interactive mode: connect stdout/stderr directly + _, err = r.exec.Run(ctx, &exec.RunOptions{ + Name: r.cliPath, + Args: args, + Stdout: os.Stdout, + Stderr: os.Stderr, }) - if err != nil { - stderr := "" - if result != nil { - stderr = strings.TrimSpace(string(result.Stderr)) - } - if stderr != "" { - return fmt.Errorf("devcontainer exec: %s", stderr) - } - return fmt.Errorf("devcontainer exec: %w", err) - } - - return nil + return err } // execInteractive runs a devcontainer exec command with TTY support. diff --git a/internal/flags/flags.go b/internal/flags/flags.go deleted file mode 100644 index 089e15d..0000000 --- a/internal/flags/flags.go +++ /dev/null @@ -1,132 +0,0 @@ -// Package flags provides parsing, merging, and reconstruction of container runtime flags. -package flags - -import ( - "errors" - "fmt" - "sort" -) - -// Flags represents runtime flags as a key-value map. -// Values can be: -// - string: generates --key=value -// - bool: true generates --key, false omits the flag -// - []string: generates --key=v for each element -type Flags map[string]any - -// Sentinel errors for flag operations. -var ( - // ErrInvalidFlagValue is returned when a flag value has an unsupported type. - ErrInvalidFlagValue = errors.New("invalid flag value type") -) - -// FromConfig validates and normalizes config values into Flags. -// Accepts string, bool, []string, and []any (converted to []string). -func FromConfig(cfg map[string]any) (Flags, error) { - if cfg == nil { - return make(Flags), nil - } - - result := make(Flags, len(cfg)) - for k, v := range cfg { - switch val := v.(type) { - case string: - result[k] = val - case bool: - result[k] = val - case []string: - result[k] = val - case []any: - // Convert []any to []string (common from YAML parsing) - strs := make([]string, 0, len(val)) - for _, item := range val { - s, ok := item.(string) - if !ok { - return nil, fmt.Errorf("%w: %s array contains non-string value %T", ErrInvalidFlagValue, k, item) - } - strs = append(strs, s) - } - result[k] = strs - default: - return nil, fmt.Errorf("%w: %s has unsupported type %T", ErrInvalidFlagValue, k, v) - } - } - return result, nil -} - -// Merge combines two Flags maps with override taking precedence. -// Keys in override replace keys in base. -func Merge(base, override Flags) Flags { - if base == nil && override == nil { - return make(Flags) - } - if base == nil { - return copyFlags(override) - } - if override == nil { - return copyFlags(base) - } - - result := make(Flags, len(base)+len(override)) - - // Copy base - for k, v := range base { - result[k] = v - } - - // Override with higher precedence values - for k, v := range override { - result[k] = v - } - - return result -} - -// copyFlags creates a shallow copy of Flags. -func copyFlags(f Flags) Flags { - result := make(Flags, len(f)) - for k, v := range f { - result[k] = v - } - return result -} - -// ToArgs reconstructs Flags into CLI arguments. -// Output is sorted by key for deterministic ordering. -// -// Conversion rules: -// - string: "--key=value" -// - bool true: "--key" -// - bool false: (omitted) -// - []string: "--key=v1", "--key=v2", ... -func ToArgs(f Flags) []string { - if len(f) == 0 { - return nil - } - - // Sort keys for deterministic output - keys := make([]string, 0, len(f)) - for k := range f { - keys = append(keys, k) - } - sort.Strings(keys) - - var args []string - for _, k := range keys { - v := f[k] - switch val := v.(type) { - case string: - args = append(args, fmt.Sprintf("--%s=%s", k, val)) - case bool: - if val { - args = append(args, "--"+k) - } - // false: omit entirely - case []string: - for _, s := range val { - args = append(args, fmt.Sprintf("--%s=%s", k, s)) - } - } - } - return args -} diff --git a/internal/flags/flags_test.go b/internal/flags/flags_test.go deleted file mode 100644 index d7dcbd7..0000000 --- a/internal/flags/flags_test.go +++ /dev/null @@ -1,279 +0,0 @@ -package flags - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestFromConfig(t *testing.T) { - t.Run("nil input returns empty flags", func(t *testing.T) { - result, err := FromConfig(nil) - - require.NoError(t, err) - assert.Empty(t, result) - }) - - t.Run("string values", func(t *testing.T) { - input := map[string]any{ - "memory": "2g", - "systemd": "always", - } - - result, err := FromConfig(input) - - require.NoError(t, err) - assert.Equal(t, "2g", result["memory"]) - assert.Equal(t, "always", result["systemd"]) - }) - - t.Run("bool values", func(t *testing.T) { - input := map[string]any{ - "privileged": true, - "debug": false, - } - - result, err := FromConfig(input) - - require.NoError(t, err) - assert.Equal(t, true, result["privileged"]) - assert.Equal(t, false, result["debug"]) - }) - - t.Run("string slice values", func(t *testing.T) { - input := map[string]any{ - "volume": []string{"/path1:/cont1", "/path2:/cont2"}, - } - - result, err := FromConfig(input) - - require.NoError(t, err) - assert.Equal(t, []string{"/path1:/cont1", "/path2:/cont2"}, result["volume"]) - }) - - t.Run("any slice converted to string slice", func(t *testing.T) { - input := map[string]any{ - "volume": []any{"/path1:/cont1", "/path2:/cont2"}, - } - - result, err := FromConfig(input) - - require.NoError(t, err) - assert.Equal(t, []string{"/path1:/cont1", "/path2:/cont2"}, result["volume"]) - }) - - t.Run("mixed types", func(t *testing.T) { - input := map[string]any{ - "memory": "2g", - "privileged": true, - "volume": []string{"/a:/b"}, - } - - result, err := FromConfig(input) - - require.NoError(t, err) - assert.Equal(t, "2g", result["memory"]) - assert.Equal(t, true, result["privileged"]) - assert.Equal(t, []string{"/a:/b"}, result["volume"]) - }) - - t.Run("error on unsupported type", func(t *testing.T) { - input := map[string]any{ - "invalid": 123, // int not supported - } - - _, err := FromConfig(input) - - require.ErrorIs(t, err, ErrInvalidFlagValue) - assert.Contains(t, err.Error(), "invalid") - }) - - t.Run("error on non-string in array", func(t *testing.T) { - input := map[string]any{ - "volume": []any{"/path", 123}, // int in array - } - - _, err := FromConfig(input) - - require.ErrorIs(t, err, ErrInvalidFlagValue) - assert.Contains(t, err.Error(), "volume") - }) -} - -func TestMerge(t *testing.T) { - t.Run("nil inputs return empty flags", func(t *testing.T) { - result := Merge(nil, nil) - - assert.NotNil(t, result) - assert.Empty(t, result) - }) - - t.Run("nil base returns copy of override", func(t *testing.T) { - override := Flags{"memory": "2g"} - - result := Merge(nil, override) - - assert.Equal(t, "2g", result["memory"]) - // Verify it's a copy - override["memory"] = "4g" - assert.Equal(t, "2g", result["memory"]) - }) - - t.Run("nil override returns copy of base", func(t *testing.T) { - base := Flags{"memory": "2g"} - - result := Merge(base, nil) - - assert.Equal(t, "2g", result["memory"]) - // Verify it's a copy - base["memory"] = "4g" - assert.Equal(t, "2g", result["memory"]) - }) - - t.Run("override takes precedence for same key", func(t *testing.T) { - base := Flags{"memory": "1g", "cpus": "2"} - override := Flags{"memory": "4g"} - - result := Merge(base, override) - - assert.Equal(t, "4g", result["memory"]) - assert.Equal(t, "2", result["cpus"]) - }) - - t.Run("combines keys from both", func(t *testing.T) { - base := Flags{"memory": "2g"} - override := Flags{"cpus": "4"} - - result := Merge(base, override) - - assert.Equal(t, "2g", result["memory"]) - assert.Equal(t, "4", result["cpus"]) - }) - - t.Run("override bool replaces base string", func(t *testing.T) { - base := Flags{"flag": "value"} - override := Flags{"flag": true} - - result := Merge(base, override) - - assert.Equal(t, true, result["flag"]) - }) - - t.Run("override array replaces base string", func(t *testing.T) { - base := Flags{"volume": "/a:/b"} - override := Flags{"volume": []string{"/c:/d", "/e:/f"}} - - result := Merge(base, override) - - assert.Equal(t, []string{"/c:/d", "/e:/f"}, result["volume"]) - }) -} - -func TestToArgs(t *testing.T) { - t.Run("nil input returns nil", func(t *testing.T) { - result := ToArgs(nil) - - assert.Nil(t, result) - }) - - t.Run("empty input returns nil", func(t *testing.T) { - result := ToArgs(Flags{}) - - assert.Nil(t, result) - }) - - t.Run("string value", func(t *testing.T) { - result := ToArgs(Flags{"memory": "2g"}) - - assert.Equal(t, []string{"--memory=2g"}, result) - }) - - t.Run("bool true", func(t *testing.T) { - result := ToArgs(Flags{"privileged": true}) - - assert.Equal(t, []string{"--privileged"}, result) - }) - - t.Run("bool false omitted", func(t *testing.T) { - result := ToArgs(Flags{"debug": false}) - - assert.Empty(t, result) - }) - - t.Run("string array", func(t *testing.T) { - result := ToArgs(Flags{"volume": []string{"/a:/b", "/c:/d"}}) - - assert.Equal(t, []string{"--volume=/a:/b", "--volume=/c:/d"}, result) - }) - - t.Run("mixed types sorted by key", func(t *testing.T) { - result := ToArgs(Flags{ - "privileged": true, - "memory": "2g", - "cpus": "4", - }) - - // Should be sorted: cpus, memory, privileged - assert.Equal(t, []string{"--cpus=4", "--memory=2g", "--privileged"}, result) - }) - - t.Run("complex example", func(t *testing.T) { - result := ToArgs(Flags{ - "systemd": "always", - "privileged": true, - "debug": false, // omitted - "volume": []string{"/a:/b", "/c:/d"}, - }) - - // Sorted: privileged, systemd, volume (debug omitted) - assert.Equal(t, []string{ - "--privileged", - "--systemd=always", - "--volume=/a:/b", - "--volume=/c:/d", - }, result) - }) - - t.Run("value with equals sign", func(t *testing.T) { - result := ToArgs(Flags{"env": "FOO=bar"}) - - assert.Equal(t, []string{"--env=FOO=bar"}, result) - }) -} - -func TestRoundTrip(t *testing.T) { - t.Run("config to flags to args", func(t *testing.T) { - cfg := map[string]any{ - "memory": "2g", - "privileged": true, - "volume": []string{"/a:/b"}, - } - - flags, err := FromConfig(cfg) - require.NoError(t, err) - - args := ToArgs(flags) - - assert.Contains(t, args, "--memory=2g") - assert.Contains(t, args, "--privileged") - assert.Contains(t, args, "--volume=/a:/b") - }) - - t.Run("merge then to args", func(t *testing.T) { - baseFlags, err := FromConfig(map[string]any{"systemd": "always", "memory": "1g"}) - require.NoError(t, err) - configFlags, err := FromConfig(map[string]any{"memory": "4g", "privileged": true}) - require.NoError(t, err) - - merged := Merge(baseFlags, configFlags) - args := ToArgs(merged) - - // Config should win for memory - assert.Contains(t, args, "--memory=4g") - assert.NotContains(t, args, "--memory=1g") - // Both sources contribute - assert.Contains(t, args, "--systemd=always") - assert.Contains(t, args, "--privileged") - }) -} diff --git a/internal/instance/instance.go b/internal/instance/instance.go index d45dc00..853dd42 100644 --- a/internal/instance/instance.go +++ b/internal/instance/instance.go @@ -63,6 +63,7 @@ type CreateConfig struct { Image string // OCI image to use for container (vanilla mode) WorkspaceFolder string // Path to folder with devcontainer.json (devcontainer mode) Runtime container.Runtime // Optional runtime override (for devcontainer) + RuntimeFlags []string // Additional flags to pass to the container runtime } // AttachConfig configures instance attachment. diff --git a/internal/instance/manager.go b/internal/instance/manager.go index 8fcd5fa..f5c4c82 100644 --- a/internal/instance/manager.go +++ b/internal/instance/manager.go @@ -14,7 +14,6 @@ import ( "github.com/jmgilman/headjack/internal/catalog" "github.com/jmgilman/headjack/internal/container" "github.com/jmgilman/headjack/internal/exec" - "github.com/jmgilman/headjack/internal/flags" "github.com/jmgilman/headjack/internal/git" "github.com/jmgilman/headjack/internal/logging" "github.com/jmgilman/headjack/internal/multiplexer" @@ -73,7 +72,7 @@ type ManagerConfig struct { WorktreesDir string // Directory for storing worktrees (e.g., ~/.local/share/headjack/git) LogsDir string // Directory for storing logs (e.g., ~/.local/share/headjack/logs) RuntimeType RuntimeType // Container runtime type (docker or podman) - ConfigFlags flags.Flags // Flags from config file (take precedence over image labels) + ConfigFlags []string // Additional flags to pass to the container runtime Executor exec.Executor // Command executor (for devcontainer runtime creation) } @@ -88,11 +87,11 @@ type Manager struct { logPaths *logging.PathManager worktreesDir string runtimeType RuntimeType - configFlags flags.Flags + configFlags []string } // NewManager creates a new instance manager. -func NewManager(store catalogStore, runtime containerRuntime, opener gitOpener, mux sessionMultiplexer, cfg ManagerConfig) *Manager { +func NewManager(store catalogStore, runtime containerRuntime, opener gitOpener, mux sessionMultiplexer, cfg *ManagerConfig) *Manager { runtimeType := cfg.RuntimeType if runtimeType == "" { runtimeType = RuntimeDocker @@ -131,7 +130,7 @@ func (m *Manager) Executor() exec.Executor { } // Create creates a new instance for the given repository and branch. -func (m *Manager) Create(ctx context.Context, repoPath string, cfg CreateConfig) (*Instance, error) { +func (m *Manager) Create(ctx context.Context, repoPath string, cfg *CreateConfig) (*Instance, error) { // Open the repository repo, err := m.git.Open(ctx, repoPath) if err != nil { @@ -251,7 +250,11 @@ func (m *Manager) selectRuntime(override containerRuntime) containerRuntime { // buildRunConfig creates a container.RunConfig based on the creation mode. // For devcontainer mode (WorkspaceFolder set), it configures for devcontainer CLI. // For vanilla mode, it applies config flags. -func (m *Manager) buildRunConfig(cfg CreateConfig, containerName, worktreePath string) *container.RunConfig { +// CLI flags (from --) are appended after config flags for both modes. +func (m *Manager) buildRunConfig(cfg *CreateConfig, containerName, worktreePath string) *container.RunConfig { + // Merge config flags with CLI flags (config first, CLI flags appended) + flags := m.mergeFlags(cfg.RuntimeFlags) + // Devcontainer mode: minimal config, devcontainer CLI handles the rest if cfg.WorkspaceFolder != "" { return &container.RunConfig{ @@ -260,18 +263,32 @@ func (m *Manager) buildRunConfig(cfg CreateConfig, containerName, worktreePath s Mounts: []container.Mount{ {Source: worktreePath, Target: "/workspace", ReadOnly: false}, }, + Flags: flags, } } - // Vanilla mode: use config flags only + // Vanilla mode: use merged flags return &container.RunConfig{ Name: containerName, Image: cfg.Image, Mounts: []container.Mount{ {Source: worktreePath, Target: "/workspace", ReadOnly: false}, }, - Flags: flags.ToArgs(m.configFlags), + Flags: flags, + } +} + +// mergeFlags combines config flags with CLI flags. +// Config flags come first, CLI flags are appended (allowing override via runtime behavior). +func (m *Manager) mergeFlags(cliFlags []string) []string { + if len(m.configFlags) == 0 && len(cliFlags) == 0 { + return nil } + + result := make([]string, 0, len(m.configFlags)+len(cliFlags)) + result = append(result, m.configFlags...) + result = append(result, cliFlags...) + return result } // Get retrieves an instance by ID, including live container status. diff --git a/internal/instance/manager_test.go b/internal/instance/manager_test.go index 81133c9..2388b4a 100644 --- a/internal/instance/manager_test.go +++ b/internal/instance/manager_test.go @@ -28,20 +28,20 @@ const ( func TestNewManager(t *testing.T) { t.Run("sets worktrees directory", func(t *testing.T) { - mgr := NewManager(nil, nil, nil, nil, ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) + mgr := NewManager(nil, nil, nil, nil, &ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) require.NotNil(t, mgr) assert.Equal(t, "/data/worktrees", mgr.worktreesDir) }) t.Run("defaults RuntimeType to Docker when not specified", func(t *testing.T) { - mgr := NewManager(nil, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(nil, nil, nil, nil, &ManagerConfig{}) assert.Equal(t, RuntimeDocker, mgr.runtimeType) }) t.Run("respects explicit RuntimeType", func(t *testing.T) { - mgr := NewManager(nil, nil, nil, nil, ManagerConfig{RuntimeType: RuntimePodman}) + mgr := NewManager(nil, nil, nil, nil, &ManagerConfig{RuntimeType: RuntimePodman}) assert.Equal(t, RuntimePodman, mgr.runtimeType) }) @@ -85,9 +85,9 @@ func TestManager_Create(t *testing.T) { }, } - mgr := NewManager(store, runtime, opener, nil, ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) + mgr := NewManager(store, runtime, opener, nil, &ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) - inst, err := mgr.Create(ctx, "/path/to/repo", CreateConfig{ + inst, err := mgr.Create(ctx, "/path/to/repo", &CreateConfig{ Branch: "feature/auth", Image: "myimage:latest", }) @@ -124,9 +124,9 @@ func TestManager_Create(t *testing.T) { }, } - mgr := NewManager(store, nil, opener, nil, ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) + mgr := NewManager(store, nil, opener, nil, &ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) - _, err := mgr.Create(ctx, testRepoPath, CreateConfig{Branch: "main"}) + _, err := mgr.Create(ctx, testRepoPath, &CreateConfig{Branch: "main"}) assert.ErrorIs(t, err, ErrAlreadyExists) }) @@ -156,9 +156,9 @@ func TestManager_Create(t *testing.T) { }, } - mgr := NewManager(store, nil, opener, nil, ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) + mgr := NewManager(store, nil, opener, nil, &ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) - _, err := mgr.Create(ctx, testRepoPath, CreateConfig{Branch: "main"}) + _, err := mgr.Create(ctx, testRepoPath, &CreateConfig{Branch: "main"}) require.Error(t, err) assert.Contains(t, err.Error(), "worktree error") @@ -199,9 +199,9 @@ func TestManager_Create(t *testing.T) { }, } - mgr := NewManager(store, runtime, opener, nil, ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) + mgr := NewManager(store, runtime, opener, nil, &ManagerConfig{WorktreesDir: "/data/worktrees", LogsDir: "/data/logs"}) - _, err := mgr.Create(ctx, testRepoPath, CreateConfig{Branch: "main"}) + _, err := mgr.Create(ctx, testRepoPath, &CreateConfig{Branch: "main"}) require.Error(t, err) assert.Contains(t, err.Error(), "container error") @@ -236,7 +236,7 @@ func TestManager_Get(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) inst, err := mgr.Get(ctx, "abc123") @@ -253,7 +253,7 @@ func TestManager_Get(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.Get(ctx, "nonexistent") @@ -293,7 +293,7 @@ func TestManager_GetByBranch(t *testing.T) { }, } - mgr := NewManager(store, runtime, opener, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, opener, nil, &ManagerConfig{}) inst, err := mgr.GetByBranch(ctx, "/path/to/repo", "main") @@ -317,7 +317,7 @@ func TestManager_GetByBranch(t *testing.T) { }, } - mgr := NewManager(store, nil, opener, nil, ManagerConfig{}) + mgr := NewManager(store, nil, opener, nil, &ManagerConfig{}) _, err := mgr.GetByBranch(ctx, "/path/to/repo", "nonexistent") @@ -346,7 +346,7 @@ func TestManager_List(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) instances, err := mgr.List(ctx, ListFilter{}) @@ -369,7 +369,7 @@ func TestManager_List(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) instances, err := mgr.List(ctx, ListFilter{Status: StatusRunning}) @@ -402,7 +402,7 @@ func TestManager_Stop(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) err := mgr.Stop(ctx, "abc123") @@ -418,7 +418,7 @@ func TestManager_Stop(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) err := mgr.Stop(ctx, "nonexistent") @@ -462,7 +462,7 @@ func TestManager_Remove(t *testing.T) { }, } - mgr := NewManager(store, runtime, opener, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, opener, nil, &ManagerConfig{}) err := mgr.Remove(ctx, "abc123") @@ -480,7 +480,7 @@ func TestManager_Remove(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) err := mgr.Remove(ctx, "nonexistent") @@ -514,7 +514,7 @@ func TestManager_Attach(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) err := mgr.Attach(ctx, "abc123", AttachConfig{ Command: []string{"bash", "-c", "echo hello"}, @@ -546,7 +546,7 @@ func TestManager_Attach(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) err := mgr.Attach(ctx, "abc123", AttachConfig{}) @@ -571,7 +571,7 @@ func TestManager_Attach(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) err := mgr.Attach(ctx, "abc123", AttachConfig{}) @@ -652,7 +652,7 @@ func TestManager_CreateSession(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, mux, ManagerConfig{LogsDir: logsDir}) + mgr := NewManager(store, runtime, nil, mux, &ManagerConfig{LogsDir: logsDir}) session, err := mgr.CreateSession(ctx, "abc12345", &CreateSessionConfig{}) @@ -698,7 +698,7 @@ func TestManager_CreateSession(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, mux, ManagerConfig{LogsDir: logsDir}) + mgr := NewManager(store, runtime, nil, mux, &ManagerConfig{LogsDir: logsDir}) session, err := mgr.CreateSession(ctx, "abc12345", &CreateSessionConfig{Name: "my-session"}) @@ -724,7 +724,7 @@ func TestManager_CreateSession(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) _, err := mgr.CreateSession(ctx, "abc12345", &CreateSessionConfig{Name: "existing-session"}) @@ -747,7 +747,7 @@ func TestManager_CreateSession(t *testing.T) { }, } - mgr := NewManager(store, runtime, nil, nil, ManagerConfig{}) + mgr := NewManager(store, runtime, nil, nil, &ManagerConfig{}) _, err := mgr.CreateSession(ctx, "abc12345", &CreateSessionConfig{}) @@ -761,7 +761,7 @@ func TestManager_CreateSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.CreateSession(ctx, "nonexistent", &CreateSessionConfig{}) @@ -786,7 +786,7 @@ func TestManager_GetSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) session, err := mgr.GetSession(ctx, "abc12345", "second-session") @@ -806,7 +806,7 @@ func TestManager_GetSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetSession(ctx, "abc12345", "nonexistent") @@ -820,7 +820,7 @@ func TestManager_GetSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetSession(ctx, "nonexistent", "any") @@ -845,7 +845,7 @@ func TestManager_ListSessions(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) sessions, err := mgr.ListSessions(ctx, "abc12345") @@ -865,7 +865,7 @@ func TestManager_ListSessions(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) sessions, err := mgr.ListSessions(ctx, "abc12345") @@ -880,7 +880,7 @@ func TestManager_ListSessions(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.ListSessions(ctx, "nonexistent") @@ -919,7 +919,7 @@ func TestManager_KillSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, mux, ManagerConfig{LogsDir: logsDir}) + mgr := NewManager(store, nil, nil, mux, &ManagerConfig{LogsDir: logsDir}) err := mgr.KillSession(ctx, "abc12345", "my-session") @@ -951,7 +951,7 @@ func TestManager_KillSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, mux, ManagerConfig{}) + mgr := NewManager(store, nil, nil, mux, &ManagerConfig{}) err := mgr.KillSession(ctx, "abc12345", "my-session") @@ -968,7 +968,7 @@ func TestManager_KillSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) err := mgr.KillSession(ctx, "abc12345", "nonexistent") @@ -982,7 +982,7 @@ func TestManager_KillSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) err := mgr.KillSession(ctx, "nonexistent", "any") @@ -1022,7 +1022,7 @@ func TestManager_AttachSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, mux, ManagerConfig{}) + mgr := NewManager(store, nil, nil, mux, &ManagerConfig{}) err := mgr.AttachSession(ctx, "abc12345", "my-session") @@ -1041,7 +1041,7 @@ func TestManager_AttachSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) err := mgr.AttachSession(ctx, "abc12345", "nonexistent") @@ -1081,7 +1081,7 @@ func TestManager_AttachSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, mux, ManagerConfig{}) + mgr := NewManager(store, nil, nil, mux, &ManagerConfig{}) err := mgr.AttachSession(ctx, "abc12345", "my-session") @@ -1111,7 +1111,7 @@ func TestManager_GetMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) session, err := mgr.GetMRUSession(ctx, "abc12345") @@ -1129,7 +1129,7 @@ func TestManager_GetMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetMRUSession(ctx, "abc12345") @@ -1143,7 +1143,7 @@ func TestManager_GetMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetMRUSession(ctx, "nonexistent") @@ -1183,7 +1183,7 @@ func TestManager_GetGlobalMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) result, err := mgr.GetGlobalMRUSession(ctx) @@ -1202,7 +1202,7 @@ func TestManager_GetGlobalMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetGlobalMRUSession(ctx) @@ -1216,7 +1216,7 @@ func TestManager_GetGlobalMRUSession(t *testing.T) { }, } - mgr := NewManager(store, nil, nil, nil, ManagerConfig{}) + mgr := NewManager(store, nil, nil, nil, &ManagerConfig{}) _, err := mgr.GetGlobalMRUSession(ctx) diff --git a/internal/multiplexer/tmux.go b/internal/multiplexer/tmux.go index 3f6aaf6..ca3f5a2 100644 --- a/internal/multiplexer/tmux.go +++ b/internal/multiplexer/tmux.go @@ -9,6 +9,7 @@ import ( "os/signal" "strings" "syscall" + "time" "golang.org/x/term" @@ -128,7 +129,14 @@ func (t *tmux) AttachSession(ctx context.Context, sessionName string) error { if err != nil { return fmt.Errorf("set terminal raw mode: %w", err) } - defer func() { _ = term.Restore(stdinFd, oldState) }() + defer func() { + // Drain stdin with timeout BEFORE restoring terminal mode. + // This catches in-flight terminal responses (escape sequences) that + // arrive asynchronously after tmux exits for short-lived sessions. + // We do this while still in raw mode so responses are consumed properly. + drainStdinWithTimeout(stdinFd, 100*time.Millisecond) + _ = term.Restore(stdinFd, oldState) + }() // Handle window resize signals sigCh := make(chan os.Signal, 1) @@ -222,3 +230,28 @@ func shellEscape(s string) string { escaped := strings.ReplaceAll(s, "'", `'\''`) return "'" + escaped + "'" } + +// drainStdinWithTimeout reads and discards input from stdin for the specified duration. +// This is used to consume stray terminal escape sequence responses that arrive +// asynchronously after tmux exits. The timeout allows in-flight responses to arrive. +func drainStdinWithTimeout(fd int, timeout time.Duration) { + // Set stdin to non-blocking temporarily + if err := syscall.SetNonblock(fd, true); err != nil { + return + } + //nolint:errcheck // best-effort cleanup + defer func() { _ = syscall.SetNonblock(fd, false) }() + + deadline := time.Now().Add(timeout) + buf := make([]byte, 1024) + + for time.Now().Before(deadline) { + //nolint:errcheck // best-effort drain, errors expected when no data + n, _ := syscall.Read(fd, buf) + if n <= 0 { + // No data available, wait briefly and try again + time.Sleep(10 * time.Millisecond) + } + // If we read data, continue immediately to drain more + } +}