From d05e20aa935cfeda254b1a0b1dd98409ac479295 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 03:42:35 +0000
Subject: [PATCH 1/7] fix: disambiguate signal-killed errors from context
 cancellation

CommandDevContainer now attaches ctx.Err() to a killed docker-exec
error so cancellation-driven kills (e.g. a lifecycle hook cut short by
a canceled/timed-out context) are no longer indistinguishable from an
opaque "signal: killed".

RunWithResult's ctx.Done() branch used to unconditionally return
(t.result, nil), silently reporting success even when the tunnel was
canceled before any result ever arrived. It now only does that when a
result was actually received via SendResult, otherwise it surfaces
ctx.Err(). Access to the shared result field is now mutex-guarded
since it's written concurrently by the gRPC server's SendResult
handler.
---
 pkg/agent/tunnelserver/tunnelserver.go      | 27 ++++++++--
 pkg/agent/tunnelserver/tunnelserver_test.go | 59 +++++++++++++++++++++
 pkg/driver/docker/lifecycle.go              | 11 +++-
 pkg/driver/docker/lifecycle_test.go         | 41 ++++++++++++++
 4 files changed, 133 insertions(+), 5 deletions(-)

diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go
index 11d1de67a..d1add6723 100644
--- a/pkg/agent/tunnelserver/tunnelserver.go
+++ b/pkg/agent/tunnelserver/tunnelserver.go
@@ -14,6 +14,7 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
+	"sync"
 
 	"github.com/devsy-org/api/pkg/devsy"
 	"github.com/devsy-org/devsy/pkg/agent/tunnel"
@@ -114,6 +115,7 @@ type tunnelServer struct {
 	allowDockerCredentials bool
 	allowKubeConfig        bool
 	allowPlatformOptions   bool
+	resultMu               sync.Mutex
 	result                 *config.Result
 	workspace              *provider2.Workspace
 
@@ -147,12 +149,17 @@ func (t *tunnelServer) RunWithResult(
 
 	select {
 	case err := <-errChan:
-		if t.result != nil {
-			return t.result, nil
+		if result := t.getResult(); result != nil {
+			return result, nil
 		}
 		return nil, err
 	case <-ctx.Done():
-		return t.result, nil
+		// Only mask cancellation as success if a result already arrived;
+		// otherwise report ctx.Err() instead of a misleading (nil, nil).
+		if result := t.getResult(); result != nil {
+			return result, nil
+		}
+		return nil, ctx.Err()
 	}
 }
 
@@ -409,7 +416,7 @@ func (t *tunnelServer) SendResult(
 		return nil, err
 	}
 
-	t.result = parsedResult
+	t.setResult(parsedResult)
 	return &tunnel.Empty{}, nil
 }
 
@@ -662,3 +669,15 @@ func (t *tunnelServer) workspaceIgnoreExcludes() []string {
 	}
 	return excludes
 }
+
+func (t *tunnelServer) getResult() *config.Result {
+	t.resultMu.Lock()
+	defer t.resultMu.Unlock()
+	return t.result
+}
+
+func (t *tunnelServer) setResult(result *config.Result) {
+	t.resultMu.Lock()
+	defer t.resultMu.Unlock()
+	t.result = result
+}
diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go
index d0676b405..ae67400a6 100644
--- a/pkg/agent/tunnelserver/tunnelserver_test.go
+++ b/pkg/agent/tunnelserver/tunnelserver_test.go
@@ -4,10 +4,12 @@ import (
 	"archive/tar"
 	"bytes"
 	"context"
+	"errors"
 	"io"
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/devsy-org/devsy/pkg/agent/tunnel"
 	"github.com/devsy-org/devsy/pkg/devcontainer/config"
@@ -72,3 +74,60 @@ func TestStreamSnapshotVolumes_TarsMountTargets(t *testing.T) {
 	}
 	require.True(t, found, "expected tar entry %q not found", wantName)
 }
+
+func TestRunWithResult_CancelBeforeResult(t *testing.T) {
+	srv := New()
+
+	reader, writer := io.Pipe()
+	defer func() { _ = reader.Close() }()
+	defer func() { _ = writer.Close() }()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		time.Sleep(20 * time.Millisecond)
+		cancel()
+	}()
+
+	result, err := srv.RunWithResult(ctx, reader, writer)
+	require.Nil(t, result)
+	require.True(t, errors.Is(err, context.Canceled), "got: %v", err)
+}
+
+func TestRunWithResult_CancelAfterResult(t *testing.T) {
+	srv := New()
+	want := &config.Result{}
+	srv.setResult(want)
+
+	reader, writer := io.Pipe()
+	defer func() { _ = reader.Close() }()
+	defer func() { _ = writer.Close() }()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		time.Sleep(20 * time.Millisecond)
+		cancel()
+	}()
+
+	result, err := srv.RunWithResult(ctx, reader, writer)
+	require.NoError(t, err)
+	require.Same(t, want, result)
+}
+
+// TestRunWithResult_ConcurrentSendResult exercises SendResult's write racing
+// against RunWithResult's read under -race, guarding the getResult/setResult
+// mutex added to fix that unsynchronized access.
+func TestRunWithResult_ConcurrentSendResult(t *testing.T) {
+	srv := New()
+
+	reader, writer := io.Pipe()
+	defer func() { _ = reader.Close() }()
+	defer func() { _ = writer.Close() }()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		_, _ = srv.SendResult(context.Background(), &tunnel.Message{Message: "{}"})
+		cancel()
+	}()
+
+	_, _ = srv.RunWithResult(ctx, reader, writer)
+}
diff --git a/pkg/driver/docker/lifecycle.go b/pkg/driver/docker/lifecycle.go
index aad0929b8..a8dc99e19 100644
--- a/pkg/driver/docker/lifecycle.go
+++ b/pkg/driver/docker/lifecycle.go
@@ -43,11 +43,20 @@ func (d *dockerDriver) CommandDevContainer(
 		args = append(args, "-i")
 	}
 	args = append(args, "-u", params.User, container.ID, "sh", "-c", params.Command)
-	return d.Docker.Run(ctx, args, docker.Streams{
+	err = d.Docker.Run(ctx, args, docker.Streams{
 		Stdin:  params.Stdin,
 		Stdout: params.Stdout,
 		Stderr: params.Stderr,
 	})
+	if err != nil {
+		// A signal-killed exit looks identical whether it's from ctx
+		// cancellation or something else; attach ctx.Err() to disambiguate.
+		if ctxErr := ctx.Err(); ctxErr != nil {
+			return fmt.Errorf("run command in container: %w: %w", ctxErr, err)
+		}
+		return fmt.Errorf("run command in container: %w", err)
+	}
+	return nil
 }
 
 // ensureContainerRunning checks that the given container is running, and if
diff --git a/pkg/driver/docker/lifecycle_test.go b/pkg/driver/docker/lifecycle_test.go
index 84b05c1ac..53638a9ef 100644
--- a/pkg/driver/docker/lifecycle_test.go
+++ b/pkg/driver/docker/lifecycle_test.go
@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"testing"
+	"time"
 
 	"github.com/devsy-org/devsy/pkg/devcontainer/config"
 	"github.com/devsy-org/devsy/pkg/docker"
@@ -188,3 +189,43 @@ esac
 	err := d.CommitContainer(context.Background(), "missing-ws", "tag:latest")
 	require.Error(t, err)
 }
+
+func TestCommandDevContainer_CancelWrapsCtxErr(t *testing.T) {
+	dir := t.TempDir()
+	ready := filepath.Join(dir, "ready")
+	script := `#!/bin/sh
+case "$1" in
+  inspect)
+    echo '[{"ID":"c1","State":{"Status":"running"}}]'
+    ;;
+  exec)
+    touch "` + ready + `"
+    sleep 5
+    ;;
+esac
+`
+	bin := filepath.Join(dir, "docker-fake")
+	require.NoError(t, os.WriteFile(bin, []byte(script), 0o755)) //nolint:gosec
+
+	d := &dockerDriver{Docker: &docker.DockerHelper{DockerCommand: bin, ContainerID: "c1"}}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		deadline := time.Now().Add(time.Second)
+		for time.Now().Before(deadline) {
+			if _, err := os.Stat(ready); err == nil {
+				break
+			}
+			time.Sleep(time.Millisecond)
+		}
+		cancel()
+	}()
+
+	err := d.CommandDevContainer(ctx, &driver.CommandParams{
+		User:    rootUser,
+		Command: "true",
+	})
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.Canceled)
+}

From 2b140221ca7a8b313b9fccf10a2f529cbefc8015 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 04:59:31 +0000
Subject: [PATCH 2/7] fix: disambiguate signal-killed errors from context
 cancellation

CommandDevContainer now attaches ctx.Err() to a killed docker-exec
error, so a cancellation-driven kill reads as "context canceled" or
"context deadline exceeded" instead of an opaque "signal: killed".

Add tunnelserver.ReportResult, a shared helper that guarantees a
remote agent command's outcome is sent over the tunnel via SendResult
exactly once, success or failure, using a context independent of the
command's own (possibly cancelled) context. up, build, and setup all
now funnel through it instead of each maintaining its own copy of
this logic, closing gaps where an early-return path (setup's
prepareWorkspace) or a command with no result at all (build) never
reported completion. With every RunWithResult caller now
participating in this explicit-completion contract, its ctx.Done()
branch can honestly report ctx.Err() when no result ever arrived,
instead of silently treating an interrupted run as success.

Access to the tunnel server's shared result field is now
mutex-guarded, since it's written concurrently by the gRPC server's
SendResult handler while RunWithResult reads it.
---
 cmd/internal/agentcontainer/setup.go          |  58 ++-----
 cmd/internal/agentworkspace/build.go          |  13 +-
 cmd/internal/agentworkspace/up.go             |  51 ++----
 pkg/agent/tunnelserver/result_reporter.go     |  69 +++++++++
 .../tunnelserver/result_reporter_test.go      | 145 ++++++++++++++++++
 pkg/agent/tunnelserver/tunnelserver.go        |  14 +-
 pkg/agent/tunnelserver/tunnelserver_test.go   |  19 ++-
 7 files changed, 287 insertions(+), 82 deletions(-)
 create mode 100644 pkg/agent/tunnelserver/result_reporter.go
 create mode 100644 pkg/agent/tunnelserver/result_reporter_test.go

diff --git a/cmd/internal/agentcontainer/setup.go b/cmd/internal/agentcontainer/setup.go
index fae899201..055bfa925 100644
--- a/cmd/internal/agentcontainer/setup.go
+++ b/cmd/internal/agentcontainer/setup.go
@@ -108,11 +108,17 @@ func (cmd *SetupContainerCmd) Run(ctx context.Context) error {
 		tunnelClient:  tunnelClient,
 	}
 
-	if err := cmd.prepareWorkspace(sctx); err != nil {
-		return err
-	}
-
-	return cmd.finalizeSetup(sctx)
+	_, err = tunnelserver.ReportResult(
+		ctx,
+		tunnelClient,
+		func(ctx context.Context) (*config.Result, error) {
+			if err := cmd.prepareWorkspace(sctx); err != nil {
+				return nil, err
+			}
+			return cmd.finalizeSetup(sctx)
+		},
+	)
+	return err
 }
 
 func (cmd *SetupContainerCmd) registerFlags(setupContainerCmd *cobra.Command) {
@@ -258,10 +264,10 @@ func fetchSecrets(
 	return env, mount, nil
 }
 
-func (cmd *SetupContainerCmd) finalizeSetup(sctx *setupContext) error {
+func (cmd *SetupContainerCmd) finalizeSetup(sctx *setupContext) (*config.Result, error) {
 	secretsEnv, secretsMount, err := fetchSecrets(sctx.ctx, sctx.tunnelClient)
 	if err != nil {
-		return cmd.reportSetupFailure(sctx, err)
+		return nil, err
 	}
 	sctx.secretsEnv = secretsEnv
 
@@ -287,31 +293,16 @@ func (cmd *SetupContainerCmd) finalizeSetup(sctx *setupContext) error {
 
 	deferred, err := setup.SetupContainerPreAttach(sctx.ctx, cfg)
 	if err != nil {
-		return cmd.reportSetupFailure(sctx, err)
+		return nil, err
 	}
 
 	if !cmd.Prebuild {
 		if err := cmd.setupPostAttach(sctx, deferred); err != nil {
-			return cmd.reportSetupFailure(sctx, err)
+			return nil, err
 		}
 	}
 
-	return cmd.sendSetupResult(sctx.ctx, sctx.setupInfo, sctx.tunnelClient)
-}
-
-// reportSetupFailure forwards a structured error result through the tunnel
-// before returning the original error. Without this, the outer agent only
-// sees the SSH exit code and the underlying cause (e.g. an IDE install
-// failure) gets lost to a generic wrapper on the host side.
-func (cmd *SetupContainerCmd) reportSetupFailure(sctx *setupContext, cause error) error {
-	errResult := &config.Result{Error: cause.Error()}
-	if sendErr := cmd.sendSetupResult(sctx.ctx, errResult, sctx.tunnelClient); sendErr != nil {
-		// Failure-on-failure: the host will see only the SSH exit code, so
-		// log the original cause alongside the send failure to leave a
-		// breadcrumb for debugging.
-		log.Errorf("failed to forward setup error %q to host: %v", cause, sendErr)
-	}
-	return cause
+	return sctx.setupInfo, nil
 }
 
 func (cmd *SetupContainerCmd) setupPostAttach(
@@ -645,23 +636,6 @@ func (cmd *SetupContainerCmd) startPostAttachHooks(sctx *setupContext) error {
 	})
 }
 
-func (cmd *SetupContainerCmd) sendSetupResult(
-	ctx context.Context,
-	setupInfo *config.Result,
-	tunnelClient tunnel.TunnelClient,
-) error {
-	out, err := json.Marshal(setupInfo)
-	if err != nil {
-		return fmt.Errorf("marshal setup info: %w", err)
-	}
-
-	if _, err := tunnelClient.SendResult(ctx, &tunnel.Message{Message: string(out)}); err != nil {
-		return fmt.Errorf("send result: %w", err)
-	}
-
-	return nil
-}
-
 func fillContainerEnv(setupInfo *config.Result) error {
 	// set remote-env
 	if setupInfo.MergedConfig.RemoteEnv == nil {
diff --git a/cmd/internal/agentworkspace/build.go b/cmd/internal/agentworkspace/build.go
index 8687e5217..f1d36c78c 100644
--- a/cmd/internal/agentworkspace/build.go
+++ b/cmd/internal/agentworkspace/build.go
@@ -7,7 +7,9 @@ import (
 
 	"github.com/devsy-org/devsy/cmd/flags"
 	"github.com/devsy-org/devsy/pkg/agent"
+	"github.com/devsy-org/devsy/pkg/agent/tunnelserver"
 	"github.com/devsy-org/devsy/pkg/devcontainer"
+	config2 "github.com/devsy-org/devsy/pkg/devcontainer/config"
 	cliflags "github.com/devsy-org/devsy/pkg/flags"
 	"github.com/devsy-org/devsy/pkg/flags/names"
 	"github.com/devsy-org/devsy/pkg/log"
@@ -65,7 +67,7 @@ func (cmd *BuildCmd) Run(ctx context.Context) error {
 	// initialize the workspace
 	cancelCtx, cancel := context.WithCancel(ctx)
 	defer cancel()
-	_, credentialsDir, err := initWorkspace(cancelCtx, initWorkspaceParams{
+	tunnelClient, credentialsDir, err := initWorkspace(cancelCtx, initWorkspaceParams{
 		workspaceInfo:       workspaceInfo,
 		debug:               cmd.Debug,
 		shouldInstallDaemon: false,
@@ -83,7 +85,14 @@ func (cmd *BuildCmd) Run(ctx context.Context) error {
 		return err
 	}
 
-	return buildAndPushImages(ctx, runner, workspaceInfo)
+	_, err = tunnelserver.ReportResult(
+		ctx,
+		tunnelClient,
+		func(ctx context.Context) (*config2.Result, error) {
+			return nil, buildAndPushImages(ctx, runner, workspaceInfo)
+		},
+	)
+	return err
 }
 
 func buildAndPushImages(
diff --git a/cmd/internal/agentworkspace/up.go b/cmd/internal/agentworkspace/up.go
index cd4b15906..c3af1f26c 100644
--- a/cmd/internal/agentworkspace/up.go
+++ b/cmd/internal/agentworkspace/up.go
@@ -150,53 +150,36 @@ func (cmd *UpCmd) up(
 	workspaceInfo *provider.AgentWorkspaceInfo,
 	tunnelClient tunnel.TunnelClient,
 ) error {
-	result, err := cmd.devsyUp(ctx, workspaceInfo, tunnelClient)
+	result, err := tunnelserver.ReportResult(
+		ctx,
+		tunnelClient,
+		func(ctx context.Context) (*config2.Result, error) {
+			result, err := cmd.devsyUp(ctx, workspaceInfo, tunnelClient)
+			if err != nil {
+				return &config2.Result{
+					Error:             err.Error(),
+					RecoveryAvailable: errors.Is(err, clierr.ErrBuildFailedRecoverable),
+				}, err
+			}
+			// Persist so the daemon, started before the build resolved the
+			// config, can read the workspace's shutdownAction on the first up.
+			persistResolvedConfig(workspaceInfo, result)
+			return result, nil
+		},
+	)
 	if err != nil {
-		errResult := &config2.Result{
-			Error:             err.Error(),
-			RecoveryAvailable: errors.Is(err, clierr.ErrBuildFailedRecoverable),
-		}
-		if sendErr := cmd.sendResult(ctx, errResult, tunnelClient); sendErr != nil {
-			log.Errorf("failed to forward up error %q to host: %v", err, sendErr)
-		}
 		return err
 	}
-
-	// Persist so the daemon, started before the build resolved the config, can
-	// read the workspace's shutdownAction on the first up.
-	persistResolvedConfig(workspaceInfo, result)
-
 	// runner.Up can return (result, nil) where result carries a structured
 	// Error forwarded from the inner container-setup step. Treat that as a
 	// failure so the agent process exits non-zero and the host doesn't try
 	// to proceed with a half-populated result.
-	if err := cmd.sendResult(ctx, result, tunnelClient); err != nil {
-		return err
-	}
 	if result != nil && result.Error != "" {
 		return fmt.Errorf("%s", result.Error)
 	}
 	return nil
 }
 
-func (cmd *UpCmd) sendResult(
-	ctx context.Context,
-	result *config2.Result,
-	tunnelClient tunnel.TunnelClient,
-) error {
-	out, err := json.Marshal(result)
-	if err != nil {
-		return err
-	}
-
-	_, err = tunnelClient.SendResult(ctx, &tunnel.Message{Message: string(out)})
-	if err != nil {
-		return fmt.Errorf("send result: %w", err)
-	}
-
-	return nil
-}
-
 func (cmd *UpCmd) devsyUp(
 	ctx context.Context,
 	workspaceInfo *provider.AgentWorkspaceInfo,
diff --git a/pkg/agent/tunnelserver/result_reporter.go b/pkg/agent/tunnelserver/result_reporter.go
new file mode 100644
index 000000000..a4379b9a2
--- /dev/null
+++ b/pkg/agent/tunnelserver/result_reporter.go
@@ -0,0 +1,69 @@
+package tunnelserver
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"time"
+
+	"github.com/devsy-org/devsy/pkg/agent/tunnel"
+	"github.com/devsy-org/devsy/pkg/devcontainer/config"
+	"github.com/devsy-org/devsy/pkg/log"
+)
+
+// sendResultTimeout bounds the final SendResult call so it isn't tied to the
+// job's own (possibly already-cancelled) context: a cancellation-driven
+// failure is exactly the case where the host still needs to hear about it.
+const sendResultTimeout = 5 * time.Second
+
+// ReportResult runs fn and guarantees its outcome is sent over the tunnel
+// via SendResult exactly once before returning, whether fn succeeds or
+// fails. RunWithResult's ctx.Done() branch treats "cancelled with no result"
+// as a failure, so every RunUpServer/RunSetupServer caller (up, build,
+// setup) must funnel through here rather than each remembering to call
+// SendResult on its own exit paths — a gap on any one of them silently
+// reintroduces that ambiguity.
+func ReportResult(
+	ctx context.Context,
+	tunnelClient tunnel.TunnelClient,
+	fn func(ctx context.Context) (*config.Result, error),
+) (*config.Result, error) {
+	result, err := fn(ctx)
+
+	toSend := result
+	if toSend == nil {
+		toSend = &config.Result{}
+	}
+	if err != nil && toSend.Error == "" {
+		toSend.Error = err.Error()
+	}
+
+	if sendErr := sendResult(tunnelClient, toSend); sendErr != nil {
+		if err != nil {
+			log.Errorf("failed to forward result to host: %v", sendErr)
+			return result, err
+		}
+		return result, sendErr
+	}
+
+	return result, err
+}
+
+func sendResult(tunnelClient tunnel.TunnelClient, result *config.Result) error {
+	out, err := json.Marshal(result)
+	if err != nil {
+		return err
+	}
+
+	// A context independent of the job's own ctx: if the job failed because
+	// its context was cancelled, using that same context here would prevent
+	// this final completion signal from ever reaching the host.
+	sendCtx, cancel := context.WithTimeout(context.Background(), sendResultTimeout)
+	defer cancel()
+
+	message := &tunnel.Message{Message: string(out)}
+	if _, err := tunnelClient.SendResult(sendCtx, message); err != nil {
+		return fmt.Errorf("send result: %w", err)
+	}
+	return nil
+}
diff --git a/pkg/agent/tunnelserver/result_reporter_test.go b/pkg/agent/tunnelserver/result_reporter_test.go
new file mode 100644
index 000000000..aba895541
--- /dev/null
+++ b/pkg/agent/tunnelserver/result_reporter_test.go
@@ -0,0 +1,145 @@
+package tunnelserver
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/devsy-org/devsy/pkg/agent/tunnel"
+	"github.com/devsy-org/devsy/pkg/devcontainer/config"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+)
+
+type fakeSendResultClient struct {
+	tunnel.TunnelClient
+	sent           *tunnel.Message
+	ctxErrAtCall   error
+	ctxHadDeadline bool
+}
+
+func (f *fakeSendResultClient) SendResult(
+	ctx context.Context,
+	in *tunnel.Message,
+	_ ...grpc.CallOption,
+) (*tunnel.Empty, error) {
+	f.sent = in
+	f.ctxErrAtCall = ctx.Err()
+	_, f.ctxHadDeadline = ctx.Deadline()
+	return &tunnel.Empty{}, nil
+}
+
+func decodeSentResult(t *testing.T, client *fakeSendResultClient) *config.Result {
+	t.Helper()
+	require.NotNil(t, client.sent)
+	result := &config.Result{}
+	require.NoError(t, json.Unmarshal([]byte(client.sent.Message), result))
+	return result
+}
+
+func TestReportResult_NilResultSuccess(t *testing.T) {
+	client := &fakeSendResultClient{}
+
+	result, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return nil, nil },
+	)
+
+	require.NoError(t, err)
+	require.Nil(t, result)
+	require.Empty(t, decodeSentResult(t, client).Error)
+}
+
+func TestReportResult_NilResultFailure_SynthesizesError(t *testing.T) {
+	client := &fakeSendResultClient{}
+	jobErr := errors.New("build: exit status 1")
+
+	result, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return nil, jobErr },
+	)
+
+	require.ErrorIs(t, err, jobErr)
+	require.Nil(t, result)
+	require.Equal(t, jobErr.Error(), decodeSentResult(t, client).Error)
+}
+
+func TestReportResult_RichResultFailure_PreservesFields(t *testing.T) {
+	client := &fakeSendResultClient{}
+	jobErr := errors.New("devcontainer up failed")
+	richResult := &config.Result{Error: jobErr.Error(), RecoveryAvailable: true}
+
+	result, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return richResult, jobErr },
+	)
+
+	require.ErrorIs(t, err, jobErr)
+	require.Same(t, richResult, result)
+	sent := decodeSentResult(t, client)
+	require.Equal(t, jobErr.Error(), sent.Error)
+	require.True(t, sent.RecoveryAvailable)
+}
+
+func TestReportResult_UsesContextIndependentOfCaller(t *testing.T) {
+	client := &fakeSendResultClient{}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	_, err := ReportResult(
+		ctx,
+		client,
+		func(context.Context) (*config.Result, error) { return nil, nil },
+	)
+
+	require.NoError(t, err)
+	require.NoError(t, client.ctxErrAtCall)
+	require.True(
+		t,
+		client.ctxHadDeadline,
+		"expected SendResult to be called with a bounded context",
+	)
+}
+
+func TestReportResult_SendFailure_SuccessfulJobReportsSendErr(t *testing.T) {
+	client := &erroringSendResultClient{err: errors.New("transport down")}
+
+	_, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return nil, nil },
+	)
+
+	require.ErrorIs(t, err, client.err)
+}
+
+func TestReportResult_SendFailure_JobErrorTakesPrecedence(t *testing.T) {
+	client := &erroringSendResultClient{err: errors.New("transport down")}
+	jobErr := errors.New("build: exit status 1")
+
+	_, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return nil, jobErr },
+	)
+
+	require.ErrorIs(t, err, jobErr)
+}
+
+type erroringSendResultClient struct {
+	tunnel.TunnelClient
+	err error
+}
+
+func (f *erroringSendResultClient) SendResult(
+	context.Context,
+	*tunnel.Message,
+	...grpc.CallOption,
+) (*tunnel.Empty, error) {
+	return nil, f.err
+}
diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go
index d1add6723..08c278eec 100644
--- a/pkg/agent/tunnelserver/tunnelserver.go
+++ b/pkg/agent/tunnelserver/tunnelserver.go
@@ -154,8 +154,10 @@ func (t *tunnelServer) RunWithResult(
 		}
 		return nil, err
 	case <-ctx.Done():
-		// Only mask cancellation as success if a result already arrived;
-		// otherwise report ctx.Err() instead of a misleading (nil, nil).
+		// Every RunWithResult caller (up, build, setup) always sends a result
+		// over SendResult before its remote process exits, success or
+		// failure, so cancellation before one ever arrived means the run was
+		// cut short. Don't mask that as (nil, nil); report why.
 		if result := t.getResult(); result != nil {
 			return result, nil
 		}
@@ -163,8 +165,16 @@ func (t *tunnelServer) RunWithResult(
 	}
 }
 
+// Run adapts RunWithResult for callers with no result to report (e.g. the
+// long-lived services tunnel providing port forwarding/credentials): unlike
+// RunWithResult's callers, there's no explicit completion signal to wait for
+// here, so the caller's own cancellation is how this normally ends, not a
+// failure.
 func (t *tunnelServer) Run(ctx context.Context, reader io.Reader, writer io.WriteCloser) error {
 	_, err := t.RunWithResult(ctx, reader, writer)
+	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
+		return nil
+	}
 	return err
 }
 
diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go
index ae67400a6..a0a73c499 100644
--- a/pkg/agent/tunnelserver/tunnelserver_test.go
+++ b/pkg/agent/tunnelserver/tunnelserver_test.go
@@ -4,7 +4,6 @@ import (
 	"archive/tar"
 	"bytes"
 	"context"
-	"errors"
 	"io"
 	"os"
 	"path/filepath"
@@ -90,7 +89,23 @@ func TestRunWithResult_CancelBeforeResult(t *testing.T) {
 
 	result, err := srv.RunWithResult(ctx, reader, writer)
 	require.Nil(t, result)
-	require.True(t, errors.Is(err, context.Canceled), "got: %v", err)
+	require.ErrorIs(t, err, context.Canceled)
+}
+
+func TestRun_CancelBeforeResultIsExpected(t *testing.T) {
+	srv := New()
+
+	reader, writer := io.Pipe()
+	defer func() { _ = reader.Close() }()
+	defer func() { _ = writer.Close() }()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		time.Sleep(20 * time.Millisecond)
+		cancel()
+	}()
+
+	require.NoError(t, srv.Run(ctx, reader, writer))
 }
 
 func TestRunWithResult_CancelAfterResult(t *testing.T) {

From c042e81098a8751968dfada41b2dbdeb7587c1fb Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 00:05:05 -0500
Subject: [PATCH 3/7] style: update comments

---
 pkg/agent/tunnelserver/result_reporter.go   | 12 +-----------
 pkg/agent/tunnelserver/tunnelserver.go      | 11 +----------
 pkg/agent/tunnelserver/tunnelserver_test.go |  3 ---
 pkg/driver/docker/lifecycle.go              |  2 --
 4 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/pkg/agent/tunnelserver/result_reporter.go b/pkg/agent/tunnelserver/result_reporter.go
index a4379b9a2..43bb559ad 100644
--- a/pkg/agent/tunnelserver/result_reporter.go
+++ b/pkg/agent/tunnelserver/result_reporter.go
@@ -11,18 +11,11 @@ import (
 	"github.com/devsy-org/devsy/pkg/log"
 )
 
-// sendResultTimeout bounds the final SendResult call so it isn't tied to the
-// job's own (possibly already-cancelled) context: a cancellation-driven
-// failure is exactly the case where the host still needs to hear about it.
 const sendResultTimeout = 5 * time.Second
 
 // ReportResult runs fn and guarantees its outcome is sent over the tunnel
 // via SendResult exactly once before returning, whether fn succeeds or
-// fails. RunWithResult's ctx.Done() branch treats "cancelled with no result"
-// as a failure, so every RunUpServer/RunSetupServer caller (up, build,
-// setup) must funnel through here rather than each remembering to call
-// SendResult on its own exit paths — a gap on any one of them silently
-// reintroduces that ambiguity.
+// fails.
 func ReportResult(
 	ctx context.Context,
 	tunnelClient tunnel.TunnelClient,
@@ -55,9 +48,6 @@ func sendResult(tunnelClient tunnel.TunnelClient, result *config.Result) error {
 		return err
 	}
 
-	// A context independent of the job's own ctx: if the job failed because
-	// its context was cancelled, using that same context here would prevent
-	// this final completion signal from ever reaching the host.
 	sendCtx, cancel := context.WithTimeout(context.Background(), sendResultTimeout)
 	defer cancel()
 
diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go
index 08c278eec..54c5139ef 100644
--- a/pkg/agent/tunnelserver/tunnelserver.go
+++ b/pkg/agent/tunnelserver/tunnelserver.go
@@ -153,11 +153,7 @@ func (t *tunnelServer) RunWithResult(
 			return result, nil
 		}
 		return nil, err
-	case <-ctx.Done():
-		// Every RunWithResult caller (up, build, setup) always sends a result
-		// over SendResult before its remote process exits, success or
-		// failure, so cancellation before one ever arrived means the run was
-		// cut short. Don't mask that as (nil, nil); report why.
+	case <-ctx.Done():.
 		if result := t.getResult(); result != nil {
 			return result, nil
 		}
@@ -165,11 +161,6 @@ func (t *tunnelServer) RunWithResult(
 	}
 }
 
-// Run adapts RunWithResult for callers with no result to report (e.g. the
-// long-lived services tunnel providing port forwarding/credentials): unlike
-// RunWithResult's callers, there's no explicit completion signal to wait for
-// here, so the caller's own cancellation is how this normally ends, not a
-// failure.
 func (t *tunnelServer) Run(ctx context.Context, reader io.Reader, writer io.WriteCloser) error {
 	_, err := t.RunWithResult(ctx, reader, writer)
 	if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go
index a0a73c499..cbbfbf77c 100644
--- a/pkg/agent/tunnelserver/tunnelserver_test.go
+++ b/pkg/agent/tunnelserver/tunnelserver_test.go
@@ -128,9 +128,6 @@ func TestRunWithResult_CancelAfterResult(t *testing.T) {
 	require.Same(t, want, result)
 }
 
-// TestRunWithResult_ConcurrentSendResult exercises SendResult's write racing
-// against RunWithResult's read under -race, guarding the getResult/setResult
-// mutex added to fix that unsynchronized access.
 func TestRunWithResult_ConcurrentSendResult(t *testing.T) {
 	srv := New()
 
diff --git a/pkg/driver/docker/lifecycle.go b/pkg/driver/docker/lifecycle.go
index a8dc99e19..09d0dc14c 100644
--- a/pkg/driver/docker/lifecycle.go
+++ b/pkg/driver/docker/lifecycle.go
@@ -49,8 +49,6 @@ func (d *dockerDriver) CommandDevContainer(
 		Stderr: params.Stderr,
 	})
 	if err != nil {
-		// A signal-killed exit looks identical whether it's from ctx
-		// cancellation or something else; attach ctx.Err() to disambiguate.
 		if ctxErr := ctx.Err(); ctxErr != nil {
 			return fmt.Errorf("run command in container: %w: %w", ctxErr, err)
 		}

From ebbd65f59f78182d6ed17df3b8e66fc64ea61901 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 00:07:43 -0500
Subject: [PATCH 4/7] fix: typo

---
 pkg/agent/tunnelserver/tunnelserver.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/agent/tunnelserver/tunnelserver.go b/pkg/agent/tunnelserver/tunnelserver.go
index 54c5139ef..9c29d15d4 100644
--- a/pkg/agent/tunnelserver/tunnelserver.go
+++ b/pkg/agent/tunnelserver/tunnelserver.go
@@ -153,7 +153,7 @@ func (t *tunnelServer) RunWithResult(
 			return result, nil
 		}
 		return nil, err
-	case <-ctx.Done():.
+	case <-ctx.Done():
 		if result := t.getResult(); result != nil {
 			return result, nil
 		}

From 64ed7cb95280143694bc27596a98587b5a27ab76 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 06:00:28 +0000
Subject: [PATCH 5/7] fix: extend signal-killed disambiguation to all docker
 CLI invocations

CommandDevContainer (docker exec, used for lifecycle hooks) was the
only call site wrapping a killed docker subprocess with ctx.Err().
The machine-provider e2e flake showed the same bare "signal: killed"
coming from a different, unwrapped call site (docker pull/create/
start), which happens before any container exists and never goes
through CommandDevContainer.

Move the disambiguation to DockerHelper's actual choke point: every
plain cmd.Run() (Pull, RunWithDir, RunWithEnv, GetContainerLogs) now
goes through a shared runCmd helper that attaches ctx.Err() when
present. CommandDevContainer's own wrapping is simplified since the
context is now supplied by Docker.Run itself, avoiding double-wrapping.
---
 pkg/agent/tunnelserver/tunnelserver_test.go | 10 +++++--
 pkg/docker/helper.go                        | 21 ++++++++++++---
 pkg/docker/helper_test.go                   | 29 +++++++++++++++++++++
 pkg/driver/docker/lifecycle.go              |  3 ---
 4 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/pkg/agent/tunnelserver/tunnelserver_test.go b/pkg/agent/tunnelserver/tunnelserver_test.go
index cbbfbf77c..12fd047db 100644
--- a/pkg/agent/tunnelserver/tunnelserver_test.go
+++ b/pkg/agent/tunnelserver/tunnelserver_test.go
@@ -136,10 +136,16 @@ func TestRunWithResult_ConcurrentSendResult(t *testing.T) {
 	defer func() { _ = writer.Close() }()
 
 	ctx, cancel := context.WithCancel(context.Background())
+	sendDone := make(chan error, 1)
 	go func() {
-		_, _ = srv.SendResult(context.Background(), &tunnel.Message{Message: "{}"})
+		_, err := srv.SendResult(context.Background(), &tunnel.Message{Message: "{}"})
+		sendDone <- err
 		cancel()
 	}()
 
-	_, _ = srv.RunWithResult(ctx, reader, writer)
+	result, err := srv.RunWithResult(ctx, reader, writer)
+
+	require.NoError(t, <-sendDone)
+	require.NoError(t, err)
+	require.NotNil(t, result)
 }
diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go
index b8bec9d2e..cc57209c0 100644
--- a/pkg/docker/helper.go
+++ b/pkg/docker/helper.go
@@ -238,6 +238,19 @@ type PullOptions struct {
 	Stderr   io.Writer
 }
 
+// runCmd disambiguates a signal-killed command failure by attaching
+// ctx.Err() when present: "signal: killed" looks identical whether it came
+// from this ctx being cancelled/timing out or from something else entirely.
+func runCmd(ctx context.Context, cmd *exec.Cmd) error {
+	err := cmd.Run()
+	if err != nil {
+		if ctxErr := ctx.Err(); ctxErr != nil {
+			return fmt.Errorf("%w: %w", ctxErr, err)
+		}
+	}
+	return err
+}
+
 func (r *DockerHelper) Pull(ctx context.Context, opts PullOptions) error {
 	args := []string{"pull"}
 	if opts.Platform != "" {
@@ -248,7 +261,7 @@ func (r *DockerHelper) Pull(ctx context.Context, opts PullOptions) error {
 	cmd.Stdin = opts.Stdin
 	cmd.Stdout = opts.Stdout
 	cmd.Stderr = opts.Stderr
-	return cmd.Run()
+	return runCmd(ctx, cmd)
 }
 
 func (r *DockerHelper) Remove(ctx context.Context, id string) error {
@@ -281,7 +294,7 @@ func (r *DockerHelper) RunWithDir(
 	cmd.Stdin = streams.Stdin
 	cmd.Stdout = streams.Stdout
 	cmd.Stderr = streams.Stderr
-	return cmd.Run()
+	return runCmd(ctx, cmd)
 }
 
 // RunWithEnv runs a command with extra environment variables for this
@@ -302,7 +315,7 @@ func (r *DockerHelper) RunWithEnv(
 	cmd.Stdin = streams.Stdin
 	cmd.Stdout = streams.Stdout
 	cmd.Stderr = streams.Stderr
-	return cmd.Run()
+	return runCmd(ctx, cmd)
 }
 
 func (r *DockerHelper) StartContainer(ctx context.Context, containerId string) error {
@@ -536,7 +549,7 @@ func (r *DockerHelper) GetContainerLogs(
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
 
-	return cmd.Run()
+	return runCmd(ctx, cmd)
 }
 
 // containerStateError returns an error describing the container's state, including its
diff --git a/pkg/docker/helper_test.go b/pkg/docker/helper_test.go
index c7db629c2..48411abbb 100644
--- a/pkg/docker/helper_test.go
+++ b/pkg/docker/helper_test.go
@@ -279,3 +279,32 @@ exit 1
 	assert.NoError(t, err, "should not propagate error on command failure")
 	assert.False(t, got, "should fall back to no GPU on command failure")
 }
+
+func TestRunCmd_AttachesCtxErrOnFailure(t *testing.T) {
+	bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh
+exit 1
+`)
+	h := &DockerHelper{DockerCommand: bin}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
+	err := h.Run(ctx, []string{"exec", "c1", "cmd"}, streams)
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.Canceled)
+}
+
+func TestRunCmd_NoCtxErrWhenNotCancelled(t *testing.T) {
+	bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh
+exit 1
+`)
+	h := &DockerHelper{DockerCommand: bin}
+
+	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
+	err := h.Run(context.Background(), []string{"exec", "c1", "cmd"}, streams)
+
+	require.Error(t, err)
+	assert.NotErrorIs(t, err, context.Canceled)
+}
diff --git a/pkg/driver/docker/lifecycle.go b/pkg/driver/docker/lifecycle.go
index 09d0dc14c..deafa4985 100644
--- a/pkg/driver/docker/lifecycle.go
+++ b/pkg/driver/docker/lifecycle.go
@@ -49,9 +49,6 @@ func (d *dockerDriver) CommandDevContainer(
 		Stderr: params.Stderr,
 	})
 	if err != nil {
-		if ctxErr := ctx.Err(); ctxErr != nil {
-			return fmt.Errorf("run command in container: %w: %w", ctxErr, err)
-		}
 		return fmt.Errorf("run command in container: %w", err)
 	}
 	return nil

From 27b7f12a3624e67bca0c6603224e9025f059552d Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 07:24:27 +0000
Subject: [PATCH 6/7] fix: address CodeRabbit review feedback on signal-killed
 fix

- runCmd now hooks cmd.Cancel to record whether os/exec itself acted
  on this ctx, instead of checking ctx.Err() after cmd.Run() returns,
  which could race an unrelated concurrent cancellation against the
  command's own independent failure.
- ReportResult no longer mutates the caller-owned *config.Result when
  synthesizing an Error field; it copies first.
- ReportResult recovers a panic in its job function so its "exactly
  once" completion guarantee holds even when the job crashes outright.
- build.go passes cancelCtx (matching its sibling calls) instead of
  the outer ctx to ReportResult.
- setup.go's ReportResult callback no longer shadows an unused ctx
  parameter; sctx.ctx is already the authoritative context there.
---
 cmd/internal/agentcontainer/setup.go          |  2 +-
 cmd/internal/agentworkspace/build.go          |  2 +-
 pkg/agent/tunnelserver/result_reporter.go     | 24 +++++--
 .../tunnelserver/result_reporter_test.go      | 36 +++++++++++
 pkg/docker/helper.go                          | 20 ++++--
 pkg/docker/helper_test.go                     | 64 ++++++++++++++++++-
 6 files changed, 134 insertions(+), 14 deletions(-)

diff --git a/cmd/internal/agentcontainer/setup.go b/cmd/internal/agentcontainer/setup.go
index 055bfa925..9d82681f9 100644
--- a/cmd/internal/agentcontainer/setup.go
+++ b/cmd/internal/agentcontainer/setup.go
@@ -111,7 +111,7 @@ func (cmd *SetupContainerCmd) Run(ctx context.Context) error {
 	_, err = tunnelserver.ReportResult(
 		ctx,
 		tunnelClient,
-		func(ctx context.Context) (*config.Result, error) {
+		func(_ context.Context) (*config.Result, error) {
 			if err := cmd.prepareWorkspace(sctx); err != nil {
 				return nil, err
 			}
diff --git a/cmd/internal/agentworkspace/build.go b/cmd/internal/agentworkspace/build.go
index f1d36c78c..eb18e4c57 100644
--- a/cmd/internal/agentworkspace/build.go
+++ b/cmd/internal/agentworkspace/build.go
@@ -86,7 +86,7 @@ func (cmd *BuildCmd) Run(ctx context.Context) error {
 	}
 
 	_, err = tunnelserver.ReportResult(
-		ctx,
+		cancelCtx,
 		tunnelClient,
 		func(ctx context.Context) (*config2.Result, error) {
 			return nil, buildAndPushImages(ctx, runner, workspaceInfo)
diff --git a/pkg/agent/tunnelserver/result_reporter.go b/pkg/agent/tunnelserver/result_reporter.go
index 43bb559ad..ae2ffd075 100644
--- a/pkg/agent/tunnelserver/result_reporter.go
+++ b/pkg/agent/tunnelserver/result_reporter.go
@@ -21,11 +21,12 @@ func ReportResult(
 	tunnelClient tunnel.TunnelClient,
 	fn func(ctx context.Context) (*config.Result, error),
 ) (*config.Result, error) {
-	result, err := fn(ctx)
+	result, err := runJob(ctx, fn)
 
-	toSend := result
-	if toSend == nil {
-		toSend = &config.Result{}
+	toSend := &config.Result{}
+	if result != nil {
+		copied := *result
+		toSend = &copied
 	}
 	if err != nil && toSend.Error == "" {
 		toSend.Error = err.Error()
@@ -42,6 +43,21 @@ func ReportResult(
 	return result, err
 }
 
+// runJob recovers a panic in fn so ReportResult's completion guarantee holds
+// even when the job crashes outright, instead of the host waiting forever
+// for a result that will never arrive.
+func runJob(
+	ctx context.Context,
+	fn func(ctx context.Context) (*config.Result, error),
+) (result *config.Result, err error) {
+	defer func() {
+		if r := recover(); r != nil {
+			err = fmt.Errorf("panic: %v", r)
+		}
+	}()
+	return fn(ctx)
+}
+
 func sendResult(tunnelClient tunnel.TunnelClient, result *config.Result) error {
 	out, err := json.Marshal(result)
 	if err != nil {
diff --git a/pkg/agent/tunnelserver/result_reporter_test.go b/pkg/agent/tunnelserver/result_reporter_test.go
index aba895541..2b1de42fc 100644
--- a/pkg/agent/tunnelserver/result_reporter_test.go
+++ b/pkg/agent/tunnelserver/result_reporter_test.go
@@ -131,6 +131,42 @@ func TestReportResult_SendFailure_JobErrorTakesPrecedence(t *testing.T) {
 	require.ErrorIs(t, err, jobErr)
 }
 
+func TestReportResult_PanicInFnIsReported(t *testing.T) {
+	client := &fakeSendResultClient{}
+
+	result, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) {
+			panic("boom")
+		},
+	)
+
+	require.Nil(t, result)
+	require.ErrorContains(t, err, "boom")
+	require.Contains(t, decodeSentResult(t, client).Error, "boom")
+}
+
+func TestReportResult_DoesNotMutateCallersResult(t *testing.T) {
+	client := &fakeSendResultClient{}
+	jobErr := errors.New("devcontainer up failed")
+	callerResult := &config.Result{}
+
+	_, err := ReportResult(
+		context.Background(),
+		client,
+		func(context.Context) (*config.Result, error) { return callerResult, jobErr },
+	)
+
+	require.ErrorIs(t, err, jobErr)
+	require.Empty(
+		t,
+		callerResult.Error,
+		"ReportResult must not mutate the caller-owned result struct",
+	)
+	require.Equal(t, jobErr.Error(), decodeSentResult(t, client).Error)
+}
+
 type erroringSendResultClient struct {
 	tunnel.TunnelClient
 	err error
diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go
index cc57209c0..7ad5264fa 100644
--- a/pkg/docker/helper.go
+++ b/pkg/docker/helper.go
@@ -10,6 +10,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/devsy-org/devsy/pkg/command"
@@ -239,14 +240,21 @@ type PullOptions struct {
 }
 
 // runCmd disambiguates a signal-killed command failure by attaching
-// ctx.Err() when present: "signal: killed" looks identical whether it came
-// from this ctx being cancelled/timing out or from something else entirely.
+// ctx.Err() when this ctx is what caused the kill: "signal: killed" looks
+// identical whether it came from that or something else entirely. Checking
+// ctx.Err() only after cmd.Run() returns would race an unrelated concurrent
+// cancellation against the command's own, independent failure, so this hooks
+// cmd.Cancel to record only a cancellation this exec package itself acted on.
 func runCmd(ctx context.Context, cmd *exec.Cmd) error {
+	var cancelledByCtx atomic.Bool
+	cmd.Cancel = func() error {
+		cancelledByCtx.Store(true)
+		return cmd.Process.Kill()
+	}
+
 	err := cmd.Run()
-	if err != nil {
-		if ctxErr := ctx.Err(); ctxErr != nil {
-			return fmt.Errorf("%w: %w", ctxErr, err)
-		}
+	if err != nil && cancelledByCtx.Load() {
+		return fmt.Errorf("%w: %w", ctx.Err(), err)
 	}
 	return err
 }
diff --git a/pkg/docker/helper_test.go b/pkg/docker/helper_test.go
index 48411abbb..081d36b6b 100644
--- a/pkg/docker/helper_test.go
+++ b/pkg/docker/helper_test.go
@@ -14,6 +14,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const testFakeCommand = "cmd"
+
+var testExecArgs = []string{"exec", "c1", testFakeCommand}
+
 func writeScript(t *testing.T, dir, name, script string) string {
 	t.Helper()
 	path := filepath.Join(dir, name)
@@ -281,6 +285,34 @@ exit 1
 }
 
 func TestRunCmd_AttachesCtxErrOnFailure(t *testing.T) {
+	tmp := t.TempDir()
+	ready := filepath.Join(tmp, "ready")
+	bin := writeScript(t, tmp, "docker-fake", `#!/bin/sh
+touch `+ready+`
+exec sleep 30
+`)
+	h := &DockerHelper{DockerCommand: bin}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go func() {
+		deadline := time.Now().Add(time.Second)
+		for time.Now().Before(deadline) {
+			if _, err := os.Stat(ready); err == nil {
+				break
+			}
+			time.Sleep(time.Millisecond)
+		}
+		cancel()
+	}()
+
+	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
+	err := h.Run(ctx, testExecArgs, streams)
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.Canceled)
+}
+
+func TestRunCmd_AlreadyCancelledBeforeStart(t *testing.T) {
 	bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh
 exit 1
 `)
@@ -290,7 +322,7 @@ exit 1
 	cancel()
 
 	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
-	err := h.Run(ctx, []string{"exec", "c1", "cmd"}, streams)
+	err := h.Run(ctx, testExecArgs, streams)
 
 	require.Error(t, err)
 	assert.ErrorIs(t, err, context.Canceled)
@@ -303,7 +335,35 @@ exit 1
 	h := &DockerHelper{DockerCommand: bin}
 
 	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
-	err := h.Run(context.Background(), []string{"exec", "c1", "cmd"}, streams)
+	err := h.Run(context.Background(), testExecArgs, streams)
+
+	require.Error(t, err)
+	assert.NotErrorIs(t, err, context.Canceled)
+}
+
+// TestRunCmd_CancelAfterReturnNotRetroactivelyAttributed verifies that a
+// cancellation arriving after the command already completed on its own
+// never gets attributed to that unrelated cancellation. This relies on
+// runCmd using cmd.Cancel — which os/exec itself only invokes if ctx becomes
+// done before the process is observed to have exited — rather than a
+// post-hoc ctx.Err() check racing an independent process failure. A properly
+// concurrent version of this test (cancelling from a goroutine with no
+// synchronization) was tried and always hit the pre-Start() rejection path
+// instead: os/exec's own synchronization makes the misattribution this test
+// guards against unreachable by construction, so there's no genuine race to
+// exercise here beyond this sequential check.
+func TestRunCmd_CancelAfterReturnNotRetroactivelyAttributed(t *testing.T) {
+	bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh
+exit 1
+`)
+	h := &DockerHelper{DockerCommand: bin}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	streams := Streams{Stdout: io.Discard, Stderr: io.Discard}
+	err := h.Run(ctx, testExecArgs, streams)
+	cancel()
 
 	require.Error(t, err)
 	assert.NotErrorIs(t, err, context.Canceled)

From a875f7fede6f0e7c5968d68e6281cab447c9f75b Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Fri, 7 Aug 2026 07:36:23 +0000
Subject: [PATCH 7/7] style: remove explanatory comments in docker helper and
 tests

Code should be self-documenting; per CodeRabbit review feedback.
---
 pkg/docker/helper.go      |  6 ------
 pkg/docker/helper_test.go | 17 -----------------
 2 files changed, 23 deletions(-)

diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go
index 7ad5264fa..b8f112802 100644
--- a/pkg/docker/helper.go
+++ b/pkg/docker/helper.go
@@ -239,12 +239,6 @@ type PullOptions struct {
 	Stderr   io.Writer
 }
 
-// runCmd disambiguates a signal-killed command failure by attaching
-// ctx.Err() when this ctx is what caused the kill: "signal: killed" looks
-// identical whether it came from that or something else entirely. Checking
-// ctx.Err() only after cmd.Run() returns would race an unrelated concurrent
-// cancellation against the command's own, independent failure, so this hooks
-// cmd.Cancel to record only a cancellation this exec package itself acted on.
 func runCmd(ctx context.Context, cmd *exec.Cmd) error {
 	var cancelledByCtx atomic.Bool
 	cmd.Cancel = func() error {
diff --git a/pkg/docker/helper_test.go b/pkg/docker/helper_test.go
index 081d36b6b..216636b1b 100644
--- a/pkg/docker/helper_test.go
+++ b/pkg/docker/helper_test.go
@@ -127,9 +127,6 @@ echo "$@" > `+argsFile+`
 
 func TestFindContainerJSON_MatchesAllLabels(t *testing.T) {
 	tmp := t.TempDir()
-	// Fake docker: `ps -q -a` lists three containers; `inspect` returns each
-	// container's labels. c1 matches both query labels; c2 matches only the
-	// last label (an earlier label differs); c3 inspect returns an empty array.
 	bin := writeScript(t, tmp, "docker-fake", `#!/bin/sh
 case "$1" in
   ps) printf 'c1\nc2\nc3\n' ;;
@@ -146,9 +143,6 @@ esac
 	got, err := h.FindContainerJSON(context.Background(), []string{"a=x", "b=y"})
 
 	require.NoError(t, err)
-	// Only c1 satisfies every label. c2 must be excluded (the AND-logic bug
-	// previously matched it on the last label alone), and c3's empty inspect
-	// result must not panic.
 	assert.Equal(t, []string{"c1"}, got)
 }
 
@@ -341,17 +335,6 @@ exit 1
 	assert.NotErrorIs(t, err, context.Canceled)
 }
 
-// TestRunCmd_CancelAfterReturnNotRetroactivelyAttributed verifies that a
-// cancellation arriving after the command already completed on its own
-// never gets attributed to that unrelated cancellation. This relies on
-// runCmd using cmd.Cancel — which os/exec itself only invokes if ctx becomes
-// done before the process is observed to have exited — rather than a
-// post-hoc ctx.Err() check racing an independent process failure. A properly
-// concurrent version of this test (cancelling from a goroutine with no
-// synchronization) was tried and always hit the pre-Start() rejection path
-// instead: os/exec's own synchronization makes the misattribution this test
-// guards against unreachable by construction, so there's no genuine race to
-// exercise here beyond this sequential check.
 func TestRunCmd_CancelAfterReturnNotRetroactivelyAttributed(t *testing.T) {
 	bin := writeScript(t, t.TempDir(), "docker-fake", `#!/bin/sh
 exit 1