From bd9c5860a6495a2677ba91df186bfe97b61552ef Mon Sep 17 00:00:00 2001 From: Ryan Moran Date: Mon, 16 Mar 2026 09:26:10 -0700 Subject: [PATCH] Cleanup --- internal/apple/container.go | 2 +- internal/apple/container_test.go | 10 +- internal/apple/runtime.go | 6 +- internal/apple/runtime_test.go | 2 +- internal/config.go | 13 +- internal/config/config.go | 5 +- internal/config_test.go | 19 +- internal/docker/client.go | 1 - internal/docker/client_unit_test.go | 4 + internal/docker/container.go | 20 +- internal/docker/tty.go | 12 +- internal/docker/tty_unit_test.go | 20 +- internal/errors_test.go | 30 +- internal/git/archive.go | 453 ++++++++++++++-------------- internal/git/archive_test.go | 187 ++++++++++-- internal/git/errors_test.go | 121 +++++++- internal/git/server.go | 2 +- internal/runtime/runtime.go | 2 +- main.go | 35 ++- 19 files changed, 599 insertions(+), 345 deletions(-) diff --git a/internal/apple/container.go b/internal/apple/container.go index 4eb0c39..f8dec2a 100644 --- a/internal/apple/container.go +++ b/internal/apple/container.go @@ -128,7 +128,7 @@ func (c *Container) Start(ctx context.Context) error { // Attach runs the actual user command inside the container using // `container exec --tty --interactive`. Apple Container handles TTY natively. -func (c *Container) Attach(ctx context.Context, w internal.Writer) error { +func (c *Container) Attach(ctx context.Context, cancel context.CancelFunc, w internal.Writer) error { args := []string{"exec", "--tty", "--interactive"} if c.workingDir != "" { diff --git a/internal/apple/container_test.go b/internal/apple/container_test.go index e3c7ab5..a2a6f3b 100644 --- a/internal/apple/container_test.go +++ b/internal/apple/container_test.go @@ -214,7 +214,7 @@ func TestContainerAttach(t *testing.T) { container := createTestContainer(t, runner) w := &mockWriter{} - err := container.Attach(context.Background(), w) + err := container.Attach(context.Background(), func() {}, w) require.NoError(t, err) require.Len(t, runner.calls, 1) @@ -244,7 +244,7 @@ func TestContainerAttach(t *testing.T) { container := createTestContainer(t, runner) w := &mockWriter{} - err := container.Attach(context.Background(), w) + err := container.Attach(context.Background(), func() {}, w) require.Error(t, err) require.Contains(t, err.Error(), "failed to exec in container") }) @@ -260,7 +260,7 @@ func TestContainerWait(t *testing.T) { container := createTestContainer(t, runner) w := &mockWriter{} - err := container.Attach(context.Background(), w) + err := container.Attach(context.Background(), func() {}, w) require.NoError(t, err) // Use a writer that captures output @@ -279,7 +279,7 @@ func TestContainerWait(t *testing.T) { container := createTestContainer(t, runner) w := &mockWriter{} - err := container.Attach(context.Background(), w) + err := container.Attach(context.Background(), func() {}, w) require.NoError(t, err) outWriter := &capturingWriter{} @@ -306,7 +306,7 @@ func TestContainerWait(t *testing.T) { container := createTestContainer(t, runner) w := &mockWriter{} - err := container.Attach(context.Background(), w) + err := container.Attach(context.Background(), func() {}, w) require.NoError(t, err) outWriter := &capturingWriter{} diff --git a/internal/apple/runtime.go b/internal/apple/runtime.go index 2d17e54..384c3f2 100644 --- a/internal/apple/runtime.go +++ b/internal/apple/runtime.go @@ -3,16 +3,12 @@ package apple import ( "context" "fmt" - "math" "os" - "strconv" "github.com/ryanmoran/contagent/internal" "github.com/ryanmoran/contagent/internal/runtime" ) -const MaxSleepTime = math.MaxInt32 - // Compile-time check that Runtime implements runtime.Runtime. var _ runtime.Runtime = (*Runtime)(nil) @@ -63,7 +59,7 @@ func (r *Runtime) CreateContainer(ctx context.Context, opts runtime.CreateContai args = append(args, "--workdir", opts.WorkingDir) } - args = append(args, opts.Image.Name, "sleep", strconv.Itoa(MaxSleepTime)) + args = append(args, opts.Image.Name, "sleep", "infinity") err := r.runner.Run(ctx, nil, os.Stdout, os.Stderr, "container", args...) if err != nil { diff --git a/internal/apple/runtime_test.go b/internal/apple/runtime_test.go index a6f2eaf..9511b1a 100644 --- a/internal/apple/runtime_test.go +++ b/internal/apple/runtime_test.go @@ -136,7 +136,7 @@ func TestRuntimeCreateContainer(t *testing.T) { require.Contains(t, call.Args, "/app") require.Contains(t, call.Args, "myimage:latest") require.Contains(t, call.Args, "sleep") - require.Contains(t, call.Args, "2147483647") + require.Contains(t, call.Args, "infinity") }) t.Run("returns error on create failure", func(t *testing.T) { diff --git a/internal/config.go b/internal/config.go index e6db62f..3268976 100644 --- a/internal/config.go +++ b/internal/config.go @@ -2,7 +2,6 @@ package internal import ( "fmt" - "os" "os/exec" goruntime "runtime" "strings" @@ -53,12 +52,7 @@ type GitUserConfig struct { // the configuration for running a container. It uses the new config package to load // and merge configuration from multiple sources (defaults, config files, CLI flags). // Returns an error if config loading fails (e.g., invalid config file, bad flags). -func ParseConfig(args []string, environment []string) (Config, error) { - startDir, err := os.Getwd() - if err != nil { - startDir = "." - } - +func ParseConfig(args []string, environment []string, startDir string) (Config, error) { // Use the new config package to load and parse configuration cfg, programArgs, err := config.Load(args, environment, startDir) if err != nil { @@ -152,8 +146,9 @@ func buildEnvironment(environment []string, configEnv map[string]string, rt stri env = append(env, fmt.Sprintf("COLORTERM=%s", value)) // Add ANTHROPIC_API_KEY if present - value = lookup["ANTHROPIC_API_KEY"] - env = append(env, fmt.Sprintf("ANTHROPIC_API_KEY=%s", value)) + if value := lookup["ANTHROPIC_API_KEY"]; value != "" { + env = append(env, fmt.Sprintf("ANTHROPIC_API_KEY=%s", value)) + } // Set SSH_AUTH_SOCK for Docker runtime only (Apple uses --ssh flag natively) if rt == "docker" { diff --git a/internal/config/config.go b/internal/config/config.go index 21c06ef..d363fbf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -129,8 +129,9 @@ func Load(cliArgs []string, environment []string, startDir string) (Config, []st fs.Var(&envFlags, "env", "Environment variable (KEY=VALUE)") fs.Var(&volumeFlags, "volume", "Volume mount") - // Ignore parse errors since we want to handle remaining args separately - _ = fs.Parse(cliArgs) //nolint:errcheck // Intentionally parsing what we can and handling remaining args + if err := fs.Parse(cliArgs); err != nil { + return Config{}, nil, err + } // Extract remaining program arguments programArgs := fs.Args() diff --git a/internal/config_test.go b/internal/config_test.go index ac77a7a..71e2466 100644 --- a/internal/config_test.go +++ b/internal/config_test.go @@ -26,7 +26,7 @@ func TestConfig(t *testing.T) { "OTHER_KEY=other-value", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-command", "--some-option"}), config.Args) require.Equal(t, internal.Environment([]string{ @@ -50,7 +50,7 @@ func TestConfig(t *testing.T) { "ANTHROPIC_API_KEY=some-api-key", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-program", "--arg"}), config.Args) require.ElementsMatch(t, []string{ @@ -72,7 +72,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-program"}), config.Args) require.Equal(t, []string{ @@ -93,7 +93,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-program"}), config.Args) require.Equal(t, []string{ @@ -115,13 +115,12 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-program", "--arg"}), config.Args) require.ElementsMatch(t, []string{ "TERM=some-term", "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", "SSH_AUTH_SOCK=/run/host-services/ssh-auth.sock", "VAR1=value1", "VAR2=value2", @@ -142,7 +141,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.Error(t, err) require.Contains(t, err.Error(), "time: invalid duration") require.Equal(t, internal.Config{}, config) @@ -155,7 +154,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.Error(t, err) require.Contains(t, err.Error(), "file does not exist") require.Equal(t, internal.Config{}, config) @@ -170,7 +169,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, "/some/path/to/a/Dockerfile", config.DockerfilePath) }) @@ -184,7 +183,7 @@ func TestConfig(t *testing.T) { "TERM=some-term", } - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, "some-network", config.Network) }) diff --git a/internal/docker/client.go b/internal/docker/client.go index 9247713..1dad148 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -106,7 +106,6 @@ func (c Client) BuildImage(ctx context.Context, dockerfilePath string, imageName } case <-ctx.Done(): return runtime.Image{}, ctx.Err() - default: } decoder := json.NewDecoder(response.Body) diff --git a/internal/docker/client_unit_test.go b/internal/docker/client_unit_test.go index 3fbef1a..f080178 100644 --- a/internal/docker/client_unit_test.go +++ b/internal/docker/client_unit_test.go @@ -42,6 +42,7 @@ func TestBuildImageWithMock(t *testing.T) { mock := &mockDockerClient{ imageBuildFunc: func(ctx context.Context, buildContext io.Reader, options client.ImageBuildOptions) (client.ImageBuildResult, error) { + io.Copy(io.Discard, buildContext) //nolint:errcheck // draining pipe for goroutine completion return client.ImageBuildResult{ Body: io.NopCloser(bytes.NewReader(outputBytes)), }, nil @@ -69,6 +70,7 @@ func TestBuildImageWithMock(t *testing.T) { mock := &mockDockerClient{ imageBuildFunc: func(ctx context.Context, buildContext io.Reader, options client.ImageBuildOptions) (client.ImageBuildResult, error) { + io.Copy(io.Discard, buildContext) //nolint:errcheck // draining pipe for goroutine completion return client.ImageBuildResult{}, errors.New("build failed") }, } @@ -103,6 +105,7 @@ func TestBuildImageWithMock(t *testing.T) { mock := &mockDockerClient{ imageBuildFunc: func(ctx context.Context, buildContext io.Reader, options client.ImageBuildOptions) (client.ImageBuildResult, error) { + io.Copy(io.Discard, buildContext) //nolint:errcheck // draining pipe for goroutine completion return client.ImageBuildResult{ Body: io.NopCloser(bytes.NewReader(outputBytes)), }, nil @@ -129,6 +132,7 @@ func TestBuildImageWithMock(t *testing.T) { mock := &mockDockerClient{ imageBuildFunc: func(ctx context.Context, buildContext io.Reader, options client.ImageBuildOptions) (client.ImageBuildResult, error) { + io.Copy(io.Discard, buildContext) //nolint:errcheck // draining pipe for goroutine completion return client.ImageBuildResult{}, context.Canceled }, } diff --git a/internal/docker/container.go b/internal/docker/container.go index 6691fe4..b6bb171 100644 --- a/internal/docker/container.go +++ b/internal/docker/container.go @@ -5,10 +5,7 @@ import ( "errors" "fmt" "io" - "os" - "os/signal" "sync" - "syscall" "time" "github.com/docker/cli/cli/streams" @@ -65,7 +62,7 @@ func (c Container) Start(ctx context.Context) error { // It sets the terminal to raw mode, monitors terminal resize events, and forwards I/O between // the local terminal and the container. Returns an error if terminal setup fails, TTY monitoring // fails, or container attachment fails. -func (c Container) Attach(ctx context.Context, w internal.Writer) error { +func (c Container) Attach(ctx context.Context, cancel context.CancelFunc, w internal.Writer) error { stdin, stdout, _ := term.StdStreams() in := streams.NewIn(stdin) out := streams.NewOut(stdout) @@ -80,8 +77,8 @@ func (c Container) Attach(ctx context.Context, w internal.Writer) error { w.Warningf("failed to resize tty: %v", err) } - tty := NewTTY(c.client, out, c.ID, c.TTYRetries, c.RetryDelay, w) - err = tty.Monitor(ctx) + tty := NewTTY(c.client, out, c.ID, c.TTYRetries, c.RetryDelay, w, cancel) + err = tty.Monitor(ctx, cancel) if err != nil { return fmt.Errorf("failed to monitor tty size: %w", err) } @@ -157,17 +154,14 @@ func (c Container) Attach(ctx context.Context, w internal.Writer) error { return nil } -// Wait waits for the container to exit or for an interrupt signal (SIGINT, SIGTERM). -// If a signal is received, it attempts to gracefully stop the container with the configured +// Wait waits for the container to exit or for context cancellation. +// If the context is cancelled, it attempts to gracefully stop the container with the configured // timeout. Returns an error if waiting for the container fails. func (c Container) Wait(ctx context.Context, w internal.Writer) error { wait := c.client.ContainerWait(ctx, c.ID, client.ContainerWaitOptions{ Condition: container.WaitConditionNotRunning, }) - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) - select { case err := <-wait.Error: if err != nil { @@ -175,10 +169,10 @@ func (c Container) Wait(ctx context.Context, w internal.Writer) error { } case status := <-wait.Result: w.Printf("\nContainer exited with status: %d\n", status.StatusCode) - case <-sigChan: + case <-ctx.Done(): w.Println("\nReceived signal, stopping container...") timeout := c.StopTimeout - _, err := c.client.ContainerStop(ctx, c.ID, client.ContainerStopOptions{Timeout: &timeout}) + _, err := c.client.ContainerStop(context.Background(), c.ID, client.ContainerStopOptions{Timeout: &timeout}) if err != nil { w.Warningf("failed to stop container: %v", err) } diff --git a/internal/docker/tty.go b/internal/docker/tty.go index e2fb8ce..351d51e 100644 --- a/internal/docker/tty.go +++ b/internal/docker/tty.go @@ -19,12 +19,14 @@ type TTY struct { maxRetries int retryDelay time.Duration writer internal.Writer + cancel context.CancelFunc } // NewTTY creates a TTY handler for monitoring and resizing the container's terminal. // The maxRetries parameter controls how many times to retry initial resize operations, -// and retryDelay specifies the base delay between retries. -func NewTTY(client DockerClient, out *streams.Out, id string, maxRetries int, retryDelay time.Duration, writer internal.Writer) TTY { +// and retryDelay specifies the base delay between retries. The cancel function is called +// if retries are exhausted to signal context cancellation instead of calling Fatal. +func NewTTY(client DockerClient, out *streams.Out, id string, maxRetries int, retryDelay time.Duration, writer internal.Writer, cancel context.CancelFunc) TTY { return TTY{ client: client, out: out, @@ -32,6 +34,7 @@ func NewTTY(client DockerClient, out *streams.Out, id string, maxRetries int, re maxRetries: maxRetries, retryDelay: retryDelay, writer: writer, + cancel: cancel, } } @@ -39,7 +42,7 @@ func NewTTY(client DockerClient, out *streams.Out, id string, maxRetries int, re // the container's TTY to match. If the initial resize fails, it retries with exponential // backoff up to the configured maximum retries. Returns nil after starting background // monitoring goroutines, or an error if the context is cancelled during setup. -func (t TTY) Monitor(ctx context.Context) error { +func (t TTY) Monitor(ctx context.Context, cancel context.CancelFunc) error { err := t.Resize(ctx) if err != nil { go func() { @@ -55,7 +58,8 @@ func (t TTY) Monitor(ctx context.Context) error { } } if err != nil { - t.writer.Fatalf("failed to resize tty: %v", err) + t.writer.Warningf("failed to resize tty: %v", err) + cancel() } }() } diff --git a/internal/docker/tty_unit_test.go b/internal/docker/tty_unit_test.go index b2838f9..c1c7134 100644 --- a/internal/docker/tty_unit_test.go +++ b/internal/docker/tty_unit_test.go @@ -29,7 +29,7 @@ func TestTTYResizeWithMock(t *testing.T) { out := streams.NewOut(nil) writer := newMockWriter() - tty := docker.NewTTY(mock, out, "container123", 5, 100*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", 5, 100*time.Millisecond, writer, func() {}) ctx := context.Background() // Resize will return nil if height and width are 0 (terminal not detected) @@ -51,7 +51,7 @@ func TestTTYResizeWithMock(t *testing.T) { out := streams.NewOut(nil) writer := newMockWriter() - tty := docker.NewTTY(mock, out, "container123", 5, 100*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", 5, 100*time.Millisecond, writer, func() {}) ctx := context.Background() // In test environment, this will return nil because TTY size is 0x0 @@ -72,11 +72,11 @@ func TestTTYMonitorWithMock(t *testing.T) { out := streams.NewOut(nil) writer := newMockWriter() - tty := docker.NewTTY(mock, out, "container123", 5, 10*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", 5, 10*time.Millisecond, writer, func() {}) ctx := context.Background() // Monitor starts a goroutine and returns immediately - err := tty.Monitor(ctx) + err := tty.Monitor(ctx, func() {}) require.NoError(t, err) // Give the retry goroutine time to start @@ -98,10 +98,10 @@ func TestTTYMonitorWithMock(t *testing.T) { out := streams.NewOut(nil) writer := newMockWriter() - tty := docker.NewTTY(mock, out, "container123", 5, 10*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", 5, 10*time.Millisecond, writer, func() {}) ctx := context.Background() - err := tty.Monitor(ctx) + err := tty.Monitor(ctx, func() {}) require.NoError(t, err) // Give the retry goroutine time to complete @@ -121,10 +121,10 @@ func TestTTYMonitorWithMock(t *testing.T) { writer := newMockWriter() maxRetries := 3 - tty := docker.NewTTY(mock, out, "container123", maxRetries, 10*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", maxRetries, 10*time.Millisecond, writer, func() {}) ctx := context.Background() - err := tty.Monitor(ctx) + err := tty.Monitor(ctx, func() {}) require.NoError(t, err) // Give the retry goroutine time to exhaust retries @@ -142,7 +142,7 @@ func TestTTYCreation(t *testing.T) { out := streams.NewOut(nil) writer := newMockWriter() - tty := docker.NewTTY(mock, out, "container123", 10, 100*time.Millisecond, writer) + tty := docker.NewTTY(mock, out, "container123", 10, 100*time.Millisecond, writer, func() {}) // We can't directly inspect the fields since they're private, // but we can verify the TTY was created without panicking @@ -184,7 +184,7 @@ func TestContainerResizeIntegration(t *testing.T) { require.NoError(t, err) writer := newMockWriter() - _ = container.Attach(ctx, writer) //nolint:errcheck // Test verifies resize behavior, not attach success + _ = container.Attach(ctx, func() {}, writer) //nolint:errcheck // Test verifies resize behavior, not attach success // In test environment, resize is called but with 0x0 dimensions // The resize call itself should not fail diff --git a/internal/errors_test.go b/internal/errors_test.go index 16fd75c..1111237 100644 --- a/internal/errors_test.go +++ b/internal/errors_test.go @@ -19,7 +19,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{} env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Empty(t, config.Args) require.NotEmpty(t, config.Env) // Should still have default env @@ -29,12 +29,12 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"some-command"} env := []string{} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"some-command"}), config.Args) // Should have default values for missing env vars require.Contains(t, config.Env, "COLORTERM=truecolor") - require.Contains(t, config.Env, "ANTHROPIC_API_KEY=") + require.NotContains(t, config.Env, "ANTHROPIC_API_KEY=") }) t.Run("only flags, no command", func(t *testing.T) { @@ -45,7 +45,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Empty(t, config.Args) // No command after flags require.Equal(t, []string{ @@ -58,7 +58,7 @@ func TestConfigErrorCases(t *testing.T) { env := []string{"TERM=xterm"} // ParseConfig doesn't validate format, just passes through - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) // "some-command" is treated as the value for --env // Next arg would be the command, but there isn't one @@ -69,7 +69,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"--env", "VARVALUE", "command"} env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) // VARVALUE is not added to env because it lacks '=' @@ -85,7 +85,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) // "command" is treated as volume value require.Empty(t, config.Args) @@ -106,7 +106,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"cmd", "arg1", "arg2"}), config.Args) require.Contains(t, config.Env, "VAR1=val1") @@ -122,7 +122,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"--env", "VAR=val", "--", "--command-with-dashes", "--flag"} env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) // Without special handling of --, all args after VAR=val become the command // The behavior depends on implementation @@ -138,7 +138,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) require.Contains(t, config.Env, "VAR=value with spaces") @@ -150,7 +150,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"--env", "EMPTY=", "command"} env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) require.Contains(t, config.Env, "EMPTY=") @@ -164,7 +164,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) // Both are added to the list (behavior may vary) @@ -179,7 +179,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"--env", "TERM=override", "command"} env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) // Both TERM values might be present @@ -193,7 +193,7 @@ func TestConfigErrorCases(t *testing.T) { } env := []string{"TERM=xterm"} - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Len(t, config.Args, 1001) }) @@ -202,7 +202,7 @@ func TestConfigErrorCases(t *testing.T) { args := []string{"command"} env := []string{} // No TERM, COLORTERM, or ANTHROPIC_API_KEY - config, err := internal.ParseConfig(args, env) + config, err := internal.ParseConfig(args, env, ".") require.NoError(t, err) require.Equal(t, internal.Command([]string{"command"}), config.Args) // Should provide defaults for missing values diff --git a/internal/git/archive.go b/internal/git/archive.go index 1bcba45..bdee45c 100644 --- a/internal/git/archive.go +++ b/internal/git/archive.go @@ -15,28 +15,40 @@ import ( "github.com/ryanmoran/contagent/internal" ) +// ArchiveOptions holds the configuration for creating a git archive. +type ArchiveOptions struct { + Path string + Remote string + Branch string + GitUserName string + GitUserEmail string + UID int + GID int + DestDir string +} + // CreateArchive creates a tar archive of the Git repository at the specified path, configured // with the given remote URL and branch name. It checks out HEAD into a temporary directory, // configures the remote, creates a new branch, and archives the .git directory and all tracked // files. The git user name and email are configured in the temporary repository. // -// uid and gid are applied to all tar headers so that extracted files are owned by the correct -// container user. +// opts.UID and opts.GID are applied to all tar headers so that extracted files are owned by +// the correct container user. // -// When destDir is non-empty, all archive paths are prefixed with destDir and a root directory -// entry is written first. This allows copying to the parent directory so Docker creates destDir -// as a new entry with the correct uid/gid ownership, rather than copying into an already-existing -// root-owned directory. +// When opts.DestDir is non-empty, all archive paths are prefixed with DestDir and a root +// directory entry is written first. This allows copying to the parent directory so Docker +// creates DestDir as a new entry with the correct uid/gid ownership, rather than copying +// into an already-existing root-owned directory. // // Returns an io.ReadCloser that streams the tar archive. The caller must close it to clean up -// resources. Returns an error if the Git root cannot be determined, the temporary directory cannot -// be created, .git copying fails, git operations fail, or archive creation fails. -func CreateArchive(path, remote, branch, gitUserName, gitUserEmail string, uid, gid int, destDir string, w internal.Writer) (io.ReadCloser, error) { +// resources. Returns an error if the Git root cannot be determined, the temporary directory +// cannot be created, .git copying fails, git operations fail, or archive creation fails. +func CreateArchive(opts ArchiveOptions, w internal.Writer) (io.ReadCloser, error) { cmd := exec.Command("git", "rev-parse", "--show-toplevel") - cmd.Dir = path + cmd.Dir = opts.Path output, err := cmd.Output() if err != nil { - return nil, fmt.Errorf("failed to get git root path from %q: %w\nEnsure you're in a git repository", path, err) + return nil, fmt.Errorf("failed to get git root path from %q: %w\nEnsure you're in a git repository", opts.Path, err) } root := strings.TrimSpace(string(output)) @@ -48,211 +60,215 @@ func CreateArchive(path, remote, branch, gitUserName, gitUserEmail string, uid, pr, pw := io.Pipe() - archive := func(tw *tar.Writer, gitRoot, tempRoot string) error { - defer os.RemoveAll(tempRoot) // Clean up temp directory - - src := filepath.Join(gitRoot, ".git") - dst := filepath.Join(tempRoot, ".git") - - if err := copyDirectory(src, dst); err != nil { - return fmt.Errorf("failed to copy .git directory from %q to %q: %w\nCheck disk space and permissions", src, dst, err) - } - - cmd := exec.Command("git", "checkout", "HEAD", ".") - cmd.Dir = tempRoot - err := cmd.Run() - if err != nil { - return fmt.Errorf("failed to checkout HEAD in temporary repo: %w\nYou may have uncommitted changes or detached HEAD", err) - } + go func() { + tw := tar.NewWriter(pw) - cmd = exec.Command("git", "remote", "remove", "origin") - cmd.Dir = tempRoot - err = cmd.Run() + err := buildArchive(tw, opts, root, tempDir) if err != nil { - if exitError, ok := errors.AsType[*exec.ExitError](err); !ok || exitError.ExitCode() != 2 { - return fmt.Errorf("failed to remove remote \"origin\": %w", err) + pw.CloseWithError(fmt.Errorf("failed to create git archive: %w", err)) + } else { + err = tw.Close() + if err != nil { + pw.CloseWithError(fmt.Errorf("failed to close tar writer: %w", err)) } - } - cmd = exec.Command("git", "remote", "add", "origin", remote) - cmd.Dir = tempRoot - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to add git remote %q: %w\nCheck that the URL is valid", remote, err) + pw.Close() } + }() - cmd = exec.Command("git", "config", "user.email", gitUserEmail) - cmd.Dir = tempRoot - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to configure git user.email to %q: %w", gitUserEmail, err) - } + return &archiveCloser{pr: pr}, nil +} - cmd = exec.Command("git", "config", "user.name", gitUserName) - cmd.Dir = tempRoot - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to configure git user.name to %q: %w", gitUserName, err) - } +// buildArchive performs the actual archive creation: copying .git, running git commands, +// and writing all tracked files into the tar writer. +func buildArchive(tw *tar.Writer, opts ArchiveOptions, gitRoot, tempRoot string) error { + defer os.RemoveAll(tempRoot) // Clean up temp directory - cmd = exec.Command("git", "config", "push.autoSetupRemote", "true") - cmd.Dir = tempRoot - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to configure git push.autoSetupRemote: %w", err) - } + src := filepath.Join(gitRoot, ".git") + dst := filepath.Join(tempRoot, ".git") - cmd = exec.Command("git", "checkout", "-b", branch) - cmd.Dir = tempRoot - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to create and checkout branch %q: %w\nBranch may already exist", branch, err) - } + if err := copyDirectory(src, dst); err != nil { + return fmt.Errorf("failed to copy .git directory from %q to %q: %w\nCheck disk space and permissions", src, dst, err) + } - prefix := func(name string) string { - if destDir == "" { - return name - } - return destDir + "/" + name - } + cmd := exec.Command("git", "checkout", "HEAD", ".") + cmd.Dir = tempRoot + err := cmd.Run() + if err != nil { + return fmt.Errorf("failed to checkout HEAD in temporary repo: %w\nYou may have uncommitted changes or detached HEAD", err) + } - if destDir != "" { - rootHeader := &tar.Header{ - Name: destDir + "/", - Mode: 0755, - Typeflag: tar.TypeDir, - Uid: uid, - Gid: gid, - } - if err := tw.WriteHeader(rootHeader); err != nil { - return fmt.Errorf("failed to write root directory header: %w", err) - } + cmd = exec.Command("git", "remote", "remove", "origin") + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + if exitError, ok := errors.AsType[*exec.ExitError](err); !ok || exitError.ExitCode() != 2 { + return fmt.Errorf("failed to remove remote \"origin\": %w", err) } + } - if err := addDirectoryToArchive(tw, dst, prefix(".git"), uid, gid); err != nil { - return fmt.Errorf("failed to add .git directory: %w", err) - } + cmd = exec.Command("git", "remote", "add", "origin", opts.Remote) //nolint:gosec // args are controlled by internal config, not user input + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to add git remote %q: %w\nCheck that the URL is valid", opts.Remote, err) + } - cmd = exec.Command("git", "ls-files") - cmd.Dir = tempRoot - output, err := cmd.Output() - if err != nil { - return fmt.Errorf("failed to list git tracked files: %w\nRepository may be corrupted", err) - } + cmd = exec.Command("git", "config", "user.email", opts.GitUserEmail) //nolint:gosec // args are controlled by internal config, not user input + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to configure git user.email to %q: %w", opts.GitUserEmail, err) + } + + cmd = exec.Command("git", "config", "user.name", opts.GitUserName) //nolint:gosec // args are controlled by internal config, not user input + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to configure git user.name to %q: %w", opts.GitUserName, err) + } - // Collect file paths and all unique parent directories - var filePaths []string - dirsSeen := make(map[string]bool) - var sortedDirs []string + cmd = exec.Command("git", "config", "push.autoSetupRemote", "true") + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to configure git push.autoSetupRemote: %w", err) + } - scanner := bufio.NewScanner(strings.NewReader(string(output))) - for scanner.Scan() { - relPath := scanner.Text() - if relPath == "" { - continue - } - filePaths = append(filePaths, relPath) - - // Collect all parent directories of this file - dir := filepath.Dir(relPath) - for dir != "." && dir != "/" { - dirPath := strings.ReplaceAll(dir, "\\", "/") - if !dirsSeen[dirPath] { - dirsSeen[dirPath] = true - sortedDirs = append(sortedDirs, dirPath) - } - dir = filepath.Dir(dir) - } + cmd = exec.Command("git", "checkout", "-b", opts.Branch) //nolint:gosec // args are controlled by internal config, not user input + cmd.Dir = tempRoot + err = cmd.Run() + if err != nil { + return fmt.Errorf("failed to create and checkout branch %q: %w\nBranch may already exist", opts.Branch, err) + } + + prefix := func(name string) string { + if opts.DestDir == "" { + return name } + return opts.DestDir + "/" + name + } - if err := scanner.Err(); err != nil { - return fmt.Errorf("error reading git file list: %w", err) + if opts.DestDir != "" { + rootHeader := &tar.Header{ + Name: opts.DestDir + "/", + Mode: 0755, + Typeflag: tar.TypeDir, + Uid: opts.UID, + Gid: opts.GID, + } + if err := tw.WriteHeader(rootHeader); err != nil { + return fmt.Errorf("failed to write root directory header: %w", err) } + } - // Sort so parent directories come before their children - sort.Strings(sortedDirs) + if err := addDirectoryToArchive(tw, dst, prefix(".git"), opts.UID, opts.GID); err != nil { + return fmt.Errorf("failed to add .git directory: %w", err) + } - // Write directory headers first so they have proper permissions when extracted - for _, dirPath := range sortedDirs { - fullPath := filepath.Join(tempRoot, dirPath) - info, err := os.Lstat(fullPath) //nolint:gosec // path is constructed from a controlled temp root - if err != nil { - continue - } + cmd = exec.Command("git", "ls-files") + cmd.Dir = tempRoot + output, err := cmd.Output() + if err != nil { + return fmt.Errorf("failed to list git tracked files: %w\nRepository may be corrupted", err) + } - header := &tar.Header{ - Name: prefix(dirPath) + "/", - Mode: int64(info.Mode()), - ModTime: info.ModTime(), - Typeflag: tar.TypeDir, - Uid: uid, - Gid: gid, - } - if err := tw.WriteHeader(header); err != nil { - return fmt.Errorf("failed to write directory header for %s: %w", dirPath, err) + // Collect file paths and all unique parent directories + var filePaths []string + dirsSeen := make(map[string]bool) + var sortedDirs []string + + scanner := bufio.NewScanner(strings.NewReader(string(output))) + for scanner.Scan() { + relPath := scanner.Text() + if relPath == "" { + continue + } + filePaths = append(filePaths, relPath) + + // Collect all parent directories of this file + dir := filepath.Dir(relPath) + for dir != "." && dir != "/" { + dirPath := strings.ReplaceAll(dir, "\\", "/") + if !dirsSeen[dirPath] { + dirsSeen[dirPath] = true + sortedDirs = append(sortedDirs, dirPath) } + dir = filepath.Dir(dir) } + } - // Write tracked files - for _, relPath := range filePaths { - fullPath := filepath.Join(tempRoot, relPath) - info, err := os.Lstat(fullPath) //nolint:gosec // path is constructed from a controlled temp root - if err != nil { - continue - } + if err := scanner.Err(); err != nil { + return fmt.Errorf("error reading git file list: %w", err) + } - if info.Mode()&os.ModeSymlink != 0 { - continue - } + // Sort so parent directories come before their children + sort.Strings(sortedDirs) - if info.IsDir() { - continue - } + // Write directory headers first so they have proper permissions when extracted + for _, dirPath := range sortedDirs { + fullPath := filepath.Join(tempRoot, dirPath) + info, err := os.Lstat(fullPath) //nolint:gosec // path is constructed from a controlled temp root + if err != nil { + continue + } - file, err := os.Open(fullPath) //nolint:gosec // path is constructed from a controlled temp root - if err != nil { - return fmt.Errorf("failed to open tracked file %q: %w\nFile may have been deleted", relPath, err) - } - defer file.Close() + header := &tar.Header{ + Name: prefix(dirPath) + "/", + Mode: int64(info.Mode()), + ModTime: info.ModTime(), + Typeflag: tar.TypeDir, + Uid: opts.UID, + Gid: opts.GID, + } + if err := tw.WriteHeader(header); err != nil { + return fmt.Errorf("failed to write directory header for %s: %w", dirPath, err) + } + } - header := &tar.Header{ - Name: prefix(relPath), - Mode: int64(info.Mode()), - Size: info.Size(), - ModTime: info.ModTime(), - Uid: uid, - Gid: gid, - } + // Write tracked files + for _, relPath := range filePaths { + fullPath := filepath.Join(tempRoot, relPath) + info, err := os.Lstat(fullPath) //nolint:gosec // path is constructed from a controlled temp root + if err != nil { + continue + } - if err := tw.WriteHeader(header); err != nil { - return fmt.Errorf("failed to write header for %s: %w", relPath, err) - } + if info.Mode()&os.ModeSymlink != 0 { + continue + } - if _, err := io.Copy(tw, file); err != nil { - return fmt.Errorf("failed to write file %s: %w", relPath, err) - } + if info.IsDir() { + continue } - return nil - } + file, err := os.Open(fullPath) //nolint:gosec // path is constructed from a controlled temp root + if err != nil { + return fmt.Errorf("failed to open tracked file %q: %w\nFile may have been deleted", relPath, err) + } - go func() { - tw := tar.NewWriter(pw) + header := &tar.Header{ + Name: prefix(relPath), + Mode: int64(info.Mode()), + Size: info.Size(), + ModTime: info.ModTime(), + Uid: opts.UID, + Gid: opts.GID, + } - err := archive(tw, root, tempDir) - if err != nil { - pw.CloseWithError(fmt.Errorf("failed to create git archive: %w", err)) - } else { - err = tw.Close() - if err != nil { - pw.CloseWithError(fmt.Errorf("failed to close tar writer: %w", err)) - } + if err := tw.WriteHeader(header); err != nil { + file.Close() + return fmt.Errorf("failed to write header for %s: %w", relPath, err) + } - pw.Close() + if _, err := io.Copy(tw, file); err != nil { + file.Close() + return fmt.Errorf("failed to write file %s: %w", relPath, err) } - }() + file.Close() + } - return &archiveCloser{pr: pr}, nil + return nil } // archiveCloser wraps the pipe reader to ensure proper cleanup @@ -268,24 +284,32 @@ func (a *archiveCloser) Close() error { return a.pr.Close() } -func addDirectoryToArchive(tw *tar.Writer, srcDir, tarPath string, uid, gid int) error { - return filepath.Walk(srcDir, func(filePath string, info os.FileInfo, err error) error { +// walkDir walks a directory tree, skipping symlinks, and calls visitor for each entry. +// visitor receives the relative path, file info, and absolute path of each entry. +func walkDir(root string, visitor func(relPath string, info os.FileInfo, absPath string) error) error { + return filepath.Walk(root, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - relPath, err := filepath.Rel(srcDir, filePath) + if info.Mode()&os.ModeSymlink != 0 { + return nil + } + + relPath, err := filepath.Rel(root, path) if err != nil { return fmt.Errorf("failed to get relative path: %w", err) } + return visitor(relPath, info, path) + }) +} + +func addDirectoryToArchive(tw *tar.Writer, srcDir, tarPath string, uid, gid int) error { + return walkDir(srcDir, func(relPath string, info os.FileInfo, absPath string) error { fullTarPath := filepath.Join(tarPath, relPath) fullTarPath = strings.ReplaceAll(fullTarPath, "\\", "/") - if info.Mode()&os.ModeSymlink != 0 { - return nil - } - if info.IsDir() { header := &tar.Header{ Name: fullTarPath + "/", @@ -296,29 +320,29 @@ func addDirectoryToArchive(tw *tar.Writer, srcDir, tarPath string, uid, gid int) Gid: gid, } return tw.WriteHeader(header) - } else { - file, err := os.Open(filePath) - if err != nil { - return fmt.Errorf("failed to open file %s: %w", filePath, err) - } - defer file.Close() + } - header := &tar.Header{ - Name: fullTarPath, - Mode: int64(info.Mode()), - Size: info.Size(), - ModTime: info.ModTime(), - Uid: uid, - Gid: gid, - } + file, err := os.Open(absPath) + if err != nil { + return fmt.Errorf("failed to open file %s: %w", absPath, err) + } + defer file.Close() + + header := &tar.Header{ + Name: fullTarPath, + Mode: int64(info.Mode()), + Size: info.Size(), + ModTime: info.ModTime(), + Uid: uid, + Gid: gid, + } - if err := tw.WriteHeader(header); err != nil { - return fmt.Errorf("failed to write header for %s: %w", filePath, err) - } + if err := tw.WriteHeader(header); err != nil { + return fmt.Errorf("failed to write header for %s: %w", absPath, err) + } - if _, err := io.Copy(tw, file); err != nil { - return fmt.Errorf("failed to write file %s: %w", filePath, err) - } + if _, err := io.Copy(tw, file); err != nil { + return fmt.Errorf("failed to write file %s: %w", absPath, err) } return nil @@ -326,27 +350,14 @@ func addDirectoryToArchive(tw *tar.Writer, srcDir, tarPath string, uid, gid int) } func copyDirectory(src, dst string) error { - return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - relPath, err := filepath.Rel(src, path) - if err != nil { - return fmt.Errorf("failed to get relative path: %w", err) - } - + return walkDir(src, func(relPath string, info os.FileInfo, absPath string) error { dstPath := filepath.Join(dst, relPath) - if info.Mode()&os.ModeSymlink != 0 { - return nil - } - if info.IsDir() { return os.MkdirAll(dstPath, info.Mode()) - } else { - return copyFile(path, dstPath, info.Mode()) } + + return copyFile(absPath, dstPath, info.Mode()) }) } diff --git a/internal/git/archive_test.go b/internal/git/archive_test.go index d5b85ab..d6deada 100644 --- a/internal/git/archive_test.go +++ b/internal/git/archive_test.go @@ -57,7 +57,16 @@ func TestCreateArchive(t *testing.T) { userName := "Archive User" userEmail := "archive@example.com" - reader, err := git.CreateArchive(dir, remote, branch, userName, userEmail, 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: remote, + Branch: branch, + GitUserName: userName, + GitUserEmail: userEmail, + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -150,7 +159,16 @@ func TestCreateArchive(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dir) - _, err = git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + _, err = git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.ErrorContains(t, err, "failed to get git root path") }) @@ -184,7 +202,16 @@ func TestCreateArchive(t *testing.T) { // Create archive should succeed remote := "http://example.com/repo.git" - reader, err := git.CreateArchive(dir, remote, "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: remote, + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -227,7 +254,16 @@ func TestCreateArchive(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -281,7 +317,16 @@ func TestCreateArchive(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive from subdirectory - reader, err := git.CreateArchive(subDir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: subDir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -312,7 +357,16 @@ func TestCreateArchive(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive from empty repo - returns reader but will error when reading - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) // CreateArchive returns immediately with a reader, error happens in goroutine require.NoError(t, err) if reader != nil { @@ -373,7 +427,16 @@ func TestCopyDirectory(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive (this will internally use copyDirectory) - reader, err := git.CreateArchive(gitDir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: gitDir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -447,7 +510,16 @@ func TestCopyDirectory(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -500,7 +572,16 @@ func TestCopyDirectory(t *testing.T) { require.NoError(t, cmd.Run()) // Create archive - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -570,7 +651,16 @@ func TestCopyFile(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(gitDir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: gitDir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -635,7 +725,16 @@ func TestCopyFile(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -687,7 +786,16 @@ func TestArchiveOwnership(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 1001, 1001, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 1001, + GID: 1001, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -731,7 +839,16 @@ func TestArchiveOwnership(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 1001, 1001, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 1001, + GID: 1001, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -779,7 +896,16 @@ func TestArchiveOwnership(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 1001, 1001, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 1001, + GID: 1001, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -825,7 +951,16 @@ func TestArchiveOwnership(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 1001, 1001, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 1001, + GID: 1001, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -876,7 +1011,16 @@ func TestAddDirectoryToArchive(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() @@ -926,7 +1070,16 @@ func TestAddDirectoryToArchive(t *testing.T) { ) require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) defer reader.Close() diff --git a/internal/git/errors_test.go b/internal/git/errors_test.go index 08ba756..041f868 100644 --- a/internal/git/errors_test.go +++ b/internal/git/errors_test.go @@ -94,13 +94,31 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(dir) - _, err = git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + _, err = git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.Error(t, err) require.Contains(t, err.Error(), "failed to get git root path") }) t.Run("non-existent directory", func(t *testing.T) { - _, err := git.CreateArchive("/nonexistent/path", "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + _, err := git.CreateArchive(git.ArchiveOptions{ + Path: "/nonexistent/path", + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.Error(t, err) require.Contains(t, err.Error(), "failed to get git root path") }) @@ -115,7 +133,16 @@ func TestGitArchiveErrorCases(t *testing.T) { cmd.Dir = dir require.NoError(t, cmd.Run()) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) // Returns immediately if reader != nil { defer reader.Close() @@ -169,7 +196,16 @@ func TestGitArchiveErrorCases(t *testing.T) { } }) - _, err = git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + _, err = git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.Error(t, err) }) @@ -203,7 +239,16 @@ func TestGitArchiveErrorCases(t *testing.T) { // Git accepts most URLs, but we test that it doesn't fail during archive creation // The URL validation happens when actually using the remote - reader, err := git.CreateArchive(dir, "not-a-valid-url", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "not-a-valid-url", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) // Archive creation succeeds even with invalid URL if reader != nil { reader.Close() @@ -239,7 +284,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, cmd.Run()) // Try to create archive with invalid branch name (contains spaces) - reader, err := git.CreateArchive(dir, "http://example.com", "invalid branch name", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "invalid branch name", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) if err == nil && reader != nil { defer reader.Close() // Error may occur when reading @@ -282,7 +336,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, cmd.Run()) // Empty user name and email are technically valid in git - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "", "", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "", + GitUserEmail: "", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) if reader != nil { reader.Close() @@ -323,7 +386,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, cmd.Run()) // Archive will fail because it tries to create a branch that already exists in the copied .git - reader, err := git.CreateArchive(dir, "http://example.com", "test-branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "test-branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) if err == nil && reader != nil { defer reader.Close() // Error may occur during read @@ -372,7 +444,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, cmd.Run()) // Archive should still work with detached HEAD - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) if reader != nil { defer reader.Close() @@ -413,7 +494,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, os.WriteFile(testFile, []byte("uncommitted\n"), 0600)) // Archive should only include committed content (archives HEAD, not working tree) - reader, err := git.CreateArchive(dir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: dir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) require.NotNil(t, reader) reader.Close() @@ -447,7 +537,16 @@ func TestGitArchiveErrorCases(t *testing.T) { require.NoError(t, cmd.Run()) // Archive should handle repos with .gitmodules (even if submodules not initialized) - reader, err := git.CreateArchive(mainDir, "http://example.com", "branch", "user", "user@example.com", 0, 0, "", internal.NewStandardWriter()) + reader, err := git.CreateArchive(git.ArchiveOptions{ + Path: mainDir, + Remote: "http://example.com", + Branch: "branch", + GitUserName: "user", + GitUserEmail: "user@example.com", + UID: 0, + GID: 0, + DestDir: "", + }, internal.NewStandardWriter()) require.NoError(t, err) if reader != nil { reader.Close() diff --git a/internal/git/server.go b/internal/git/server.go index 3dc5f9b..11cfe0b 100644 --- a/internal/git/server.go +++ b/internal/git/server.go @@ -68,7 +68,7 @@ func NewServer(path string, w internal.Writer) (Server, error) { "GIT_HTTP_VERBOSE=1", "SSH_AUTH_SOCK=" + os.Getenv("SSH_AUTH_SOCK"), }, - Logger: log.New(os.Stdout, "[GIT SERVER}", 0), + Logger: log.New(os.Stdout, "[GIT SERVER] ", 0), Stderr: os.Stderr, } diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index 8605918..faaacee 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -46,7 +46,7 @@ type Container interface { InspectUser(ctx context.Context) (ImageUser, error) CopyTo(ctx context.Context, content io.Reader, path string) error Start(ctx context.Context) error - Attach(ctx context.Context, w internal.Writer) error + Attach(ctx context.Context, cancel context.CancelFunc, w internal.Writer) error Wait(ctx context.Context, w internal.Writer) error ForceRemove(ctx context.Context) error } diff --git a/main.go b/main.go index 1e053c7..5260cef 100644 --- a/main.go +++ b/main.go @@ -38,7 +38,12 @@ func run(args, env []string) error { cleanup := internal.NewCleanupManager() defer cleanup.Execute() - config, err := internal.ParseConfig(args[1:], env) + workingDirectory, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current working directory: %w\nThis is a system error - check file system permissions", err) + } + + config, err := internal.ParseConfig(args[1:], env, workingDirectory) if err != nil { return fmt.Errorf("failed to parse configuration: %w", err) } @@ -57,11 +62,6 @@ func run(args, env []string) error { session := internal.GenerateSession() - workingDirectory, err := os.Getwd() - if err != nil { - return fmt.Errorf("failed to get current working directory: %w\nThis is a system error - check file system permissions", err) - } - remote, err := git.NewServer(workingDirectory, w) if err != nil { return fmt.Errorf("failed to start git server in directory %q: %w", workingDirectory, err) @@ -125,17 +125,16 @@ func run(args, env []string) error { return fmt.Errorf("failed to inspect user for image %q: %w", image.Name, err) } - archive, err := git.CreateArchive( - workingDirectory, - fmt.Sprintf("http://%s:%d", rt.HostAddress(), remote.Port()), - session.Branch(), - config.GitUser.Name, - config.GitUser.Email, - imageUser.UID, - imageUser.GID, - filepath.Base(config.WorkingDir), - w, - ) + archive, err := git.CreateArchive(git.ArchiveOptions{ + Path: workingDirectory, + Remote: fmt.Sprintf("http://%s:%d", rt.HostAddress(), remote.Port()), + Branch: session.Branch(), + GitUserName: config.GitUser.Name, + GitUserEmail: config.GitUser.Email, + UID: imageUser.UID, + GID: imageUser.GID, + DestDir: filepath.Base(config.WorkingDir), + }, w) if err != nil { return fmt.Errorf("failed to create git archive from %q on branch %q: %w", workingDirectory, session.Branch(), err) } @@ -151,7 +150,7 @@ func run(args, env []string) error { return fmt.Errorf("failed to start container %q: %w", session.ID(), err) } - err = container.Attach(ctx, w) + err = container.Attach(ctx, cancel, w) if err != nil { return fmt.Errorf("failed to attach to container %q: %w\nThis may indicate a TTY configuration issue", session.ID(), err) }