localenv: reap uv subprocesses on SIGINT/SIGTERM instead of orphaning them

cmd/environments/sync.go

@@ -1,8 +1,11 @@
 package environments
 
 import (
+	"context"
 	"os"
+	"os/signal"
 	"path/filepath"
+	"syscall"
 
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/cmdctx"
@@ -51,9 +54,55 @@ func addComputeFlags(cmd *cobra.Command) {
 	// consumer relies on, instead of a bare pre-RunE Cobra error.
 }
 
+// watchInterruptSignals cancels ctx on the first SIGINT (Ctrl-C) or SIGTERM (how
+// a supervisor, CI timeout, or VS Code stops the child), which propagates to the
+// uv subprocesses the pipeline spawns so they are reaped instead of orphaned
+// mid-provision. The CLI root installs no signal handler of its own.
+//
+// The returned stop function uninstalls the handler and joins the goroutine; the
+// caller must defer it.
+//
+// The handler must give the *second* signal back to the OS. signal.Notify (like
+// signal.NotifyContext, which wraps it) disables the default disposition for
+// SIGINT/SIGTERM for as long as the channel stays registered, so without the
+// signal.Stop below a second Ctrl-C is merely buffered and dropped: the user
+// would have no way to abort during the process group's SIGKILL grace window.
+// Stopping the relay as soon as the first signal lands restores SIG_DFL, so a
+// second signal terminates the CLI immediately. That matters more here than in
+// most commands because WithProcessGroup moves uv out of the foreground process
+// group, so the tty no longer delivers Ctrl-C to it directly — this handler is
+// the only delivery path.
+func watchInterruptSignals(ctx context.Context, cancel context.CancelFunc) func() {
+	sigCh := make(chan os.Signal, 1)
+	signal.Notify(sigCh, os.Interrupt, syscall.SIGTERM)
+
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		// Selecting on ctx.Done() too lets the goroutine exit on the normal (no
+		// signal) path rather than blocking on sigCh for the rest of the process:
+		// signal.Stop unregisters the channel but never closes it.
+		select {
+		case <-sigCh:
+			signal.Stop(sigCh)
+			cancel()
+		case <-ctx.Done():
+		}
+	}()
+
+	return func() {
+		signal.Stop(sigCh)
+		// Wake the goroutine in case neither sigCh nor ctx.Done has fired.
+		cancel()
+		<-done
+	}
+}
+
 // runPipeline builds and runs the setup-local Pipeline.
 func runPipeline(cmd *cobra.Command) error {
-	ctx := cmd.Context()
+	ctx, cancel := context.WithCancel(cmd.Context())
+	defer cancel()
+	defer watchInterruptSignals(ctx, cancel)()
 
 	cluster, _ := cmd.Flags().GetString("cluster-id")
 	clusterName, _ := cmd.Flags().GetString("cluster-name")

cmd/environments/sync_signal_test.go

@@ -0,0 +1,131 @@
+//go:build unix
+
+// The second-signal escape hatch is a Unix signal-delivery behavior: the test
+// re-execs itself and sends SIGINT/SIGTERM, which os/signal does not support on
+// Windows (the CLI's own signal handling there is likewise a no-op path).
+package environments
+
+import (
+	"bufio"
+	"context"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The second-signal behavior can only be observed in a real process: signal
+// dispositions are process-wide, and the test binary's own handlers would mask
+// them. So the test re-executes itself as a child (Go resets dispositions to
+// SIG_DFL across exec, unlike a shell background job, which inherits SIGINT as
+// SIG_IGN and would silently invalidate the result).
+const signalChildEnv = "TEST_ENVIRONMENTS_SIGNAL_CHILD"
+
+// TestMain runs the signal-handler-under-test when re-executed as the child.
+func TestMain(m *testing.M) {
+	if os.Getenv(signalChildEnv) == "" {
+		os.Exit(m.Run())
+	}
+
+	// TestMain has no *testing.T, so t.Context() is unavailable here.
+	//nolint:gocritic
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	stop := watchInterruptSignals(ctx, cancel)
+	defer stop()
+
+	// Stand in for the pipeline still draining the uv process group during the
+	// SIGKILL grace window — the interval in which the user needs the hatch.
+	os.Stdout.WriteString("READY\n")
+	<-ctx.Done()
+	os.Stdout.WriteString("CANCELLED\n")
+	time.Sleep(30 * time.Second)
+	os.Stdout.WriteString("SURVIVED\n")
+	os.Exit(0)
+}
+
+// TestWatchInterruptSignalsSecondSignalStillKills is the regression test for the
+// escape hatch: signal.Notify disables the default SIGINT/SIGTERM disposition
+// process-wide, so a handler that only relays the first signal leaves the user
+// unable to abort. The first signal must cancel the context, and the second must
+// terminate the process outright.
+func TestWatchInterruptSignalsSecondSignalStillKills(t *testing.T) {
+	for _, sig := range []syscall.Signal{syscall.SIGINT, syscall.SIGTERM} {
+		t.Run(sig.String(), func(t *testing.T) {
+			cmd := exec.Command(os.Args[0], "-test.run=TestWatchInterruptSignalsSecondSignalStillKills")
+			cmd.Env = append(os.Environ(), signalChildEnv+"=1")
+			stdout, err := cmd.StdoutPipe()
+			require.NoError(t, err)
+			require.NoError(t, cmd.Start())
+			t.Cleanup(func() { _ = cmd.Process.Kill() })
+
+			lines := make(chan string, 8)
+			go func() {
+				sc := bufio.NewScanner(stdout)
+				for sc.Scan() {
+					lines <- sc.Text()
+				}
+				close(lines)
+			}()
+			await := func(want string) bool {
+				deadline := time.After(30 * time.Second)
+				for {
+					select {
+					case l, ok := <-lines:
+						if !ok {
+							return false
+						}
+						if strings.Contains(l, want) {
+							return true
+						}
+					case <-deadline:
+						return false
+					}
+				}
+			}
+
+			require.True(t, await("READY"), "child never started")
+			require.NoError(t, cmd.Process.Signal(sig))
+			require.True(t, await("CANCELLED"), "first %s did not cancel the context", sig)
+
+			// The second signal must reach the default disposition and kill the
+			// child rather than being buffered and dropped by the handler.
+			require.NoError(t, cmd.Process.Signal(sig))
+			waitErr := make(chan error, 1)
+			go func() { waitErr <- cmd.Wait() }()
+			select {
+			case err := <-waitErr:
+				// Killed by the signal, so a non-nil (non-exit-zero) error.
+				assert.Error(t, err, "child should die by signal, not exit cleanly")
+			case <-time.After(15 * time.Second):
+				t.Fatalf("second %s was swallowed: the user has no escape hatch "+
+					"during the process group's SIGKILL grace window", sig)
+			}
+		})
+	}
+}
+
+// TestWatchInterruptSignalsStopsWithoutSignal covers the no-signal path: stop()
+// must return (joining its goroutine) rather than leaving it parked on the
+// channel for the life of the process.
+func TestWatchInterruptSignalsStopsWithoutSignal(t *testing.T) {
+	ctx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+	stop := watchInterruptSignals(ctx, cancel)
+
+	returned := make(chan struct{})
+	go func() {
+		stop()
+		close(returned)
+	}()
+	select {
+	case <-returned:
+	case <-time.After(10 * time.Second):
+		t.Fatal("stop() blocked: the signal goroutine was never joined")
+	}
+}

libs/localenv/pipeline.go

@@ -82,6 +82,38 @@ func (p *Pipeline) Run(ctx context.Context) (*Result, error) {
 	p.res.Phases = initialPhases()
 
 	if err := p.run(ctx); err != nil {
+		// A cancelled context means the user or parent interrupted us (SIGINT/
+		// SIGTERM). The phase that was running reports its own failure (e.g. uv
+		// sync exiting on the signal surfaces as E_PROVISION with "signal:
+		// terminated"), which misleads a --json consumer into thinking something
+		// broke. Reclassify to E_CANCELED here — the single funnel where ctx is in
+		// scope — keeping the recorded FailurePhase and diskMutated so the consumer
+		// still knows where we stopped and whether disk was touched.
+		//
+		// The phase's own error is kept as the wrapped cause rather than replaced:
+		// a real failure can race with the signal (uv sync failing on a dependency
+		// conflict while the user gives up and hits Ctrl-C), and that cause is the
+		// only diagnostic there is.
+		if ctx.Err() != nil && p.res.Error != nil {
+			// Snapshot the phase's error *before* overwriting Code/Msg below. uvFailure
+			// folds uv's stderr — the actual diagnostic (e.g. a dependency-conflict
+			// "no solution found") — into Msg, so wrapping only the inner .Err would
+			// drop it, leaving less than main in exactly the racing-failure case this
+			// is meant to preserve. Wrapping the whole original PipelineError keeps
+			// Msg (stderr and all) in the chain.
+			orig := &PipelineError{Code: p.res.Error.Code, Msg: p.res.Error.Msg, Err: p.res.Error.Err}
+			p.res.Error.Code = ErrCanceled
+			p.res.Error.Msg = "interrupted"
+			// Two %w verbs keep both the context error and the phase's original error
+			// matchable by errors.Is, on one line — errors.Join would embed a newline
+			// and break the single-line phase row text mode prints.
+			p.res.Error.Err = fmt.Errorf("%w; %w", ctx.Err(), orig)
+			// fail() already snapshotted the pre-reclassification text into the
+			// errored phase's Detail, which is what text mode prints. Re-sync it so
+			// text and --json agree on cancellation (see PipelineError.MarshalJSON).
+			p.syncFailureDetail()
+			return p.res, p.res.Error
+		}
 		return p.res, err
 	}
 	p.res.OK = true
@@ -474,6 +506,22 @@ func (p *Pipeline) fail(phase PhaseName, diskMutated bool, pe *PipelineError) er
 	return pe
 }
 
+// syncFailureDetail re-copies the recorded error's text into its phase's Detail.
+// fail() sets Detail when the failure happens; a caller that rewrites the error
+// afterwards (Run's E_CANCELED reclassification) must call this so text output —
+// which prints Detail — keeps agreeing with the --json error object.
+func (p *Pipeline) syncFailureDetail() {
+	if p.res.Error == nil {
+		return
+	}
+	for i := range p.res.Phases {
+		if p.res.Phases[i].Phase == p.res.Error.FailurePhase {
+			p.res.Phases[i].Detail = p.res.Error.Error()
+			return
+		}
+	}
+}
+
 // asPipelineError returns err as a *PipelineError if it already is one, otherwise
 // wraps it with the fallback code and message.
 func asPipelineError(err error, fallback ErrorCode, format string, args ...any) *PipelineError {

libs/localenv/pipeline_test.go

@@ -8,12 +8,19 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 
+	"github.com/databricks/cli/libs/process"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+// cancelPMStderr is the stderr the interrupted uv sync emits. It stands in for a
+// real resolver diagnostic — the thing the cancellation reclassification must not
+// drop when it races with a Ctrl-C.
+const cancelPMStderr = "error: no solution found: databricks-connect==17.2 conflicts with pyspark==3.5"
+
 type fakePM struct{ py, dbc string }
 
 func (fakePM) Name() string                                    { return "fake" }
@@ -60,6 +67,30 @@ func (uvMissingPM) EnsureAvailable(context.Context) (string, error) {
 	return "", errors.New("uv not found and install failed")
 }
 
+// cancelPM simulates uv being interrupted: Provision closes entered (so the test
+// knows the pipeline reached this phase), blocks until the context is cancelled,
+// then returns a *process.ProcessError carrying uv's stderr (NOT context.Canceled),
+// exactly as a real `uv sync` does when it exits on SIGTERM mid-resolution. The
+// stderr is the real diagnostic; the pipeline's uvFailure folds it into the
+// PipelineError's Msg, which the cancellation reclassification must preserve.
+type cancelPM struct {
+	fakePM
+	entered chan struct{}
+}
+
+func (c cancelPM) Provision(ctx context.Context, _, _ string) error {
+	close(c.entered)
+	<-ctx.Done()
+	// Mirror uvManager.Provision's real return: a *PipelineError from uvFailure,
+	// which folds uv's stderr into Msg. Returning a bare ProcessError would not
+	// reproduce the stderr-in-Msg shape the reclassification must preserve.
+	return uvFailure(ErrProvision, &process.ProcessError{
+		Command: "uv sync",
+		Err:     errors.New("signal: terminated"),
+		Stderr:  cancelPMStderr,
+	}, "uv sync")
+}
+
 func writeProject(t *testing.T) string {
 	dir := t.TempDir()
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "pyproject.toml"), []byte(`[project]
@@ -125,6 +156,71 @@ func TestPipelineCheckMutatesNothing(t *testing.T) {
 	assert.Empty(t, entries)
 }
 
+func TestPipelineReportsCancellationNotProvisionFailure(t *testing.T) {
+	// When the context is cancelled mid-provision (a Ctrl-C / SIGTERM), the run
+	// must surface E_CANCELED, not E_PROVISION — the provision phase's own error
+	// ("signal: terminated") would otherwise imply something broke.
+	dir := writeProject(t)
+	srv := newTestServer(t)
+	defer srv.Close()
+
+	ctx, cancel := context.WithCancel(t.Context())
+	pm := cancelPM{fakePM: fakePM{py: "3.12", dbc: "17.2.0"}, entered: make(chan struct{})}
+	p := &Pipeline{
+		Mode: ModeDefault, Check: false, ProjectDir: dir,
+		ConstraintBaseURL: srv.URL, CacheDir: t.TempDir(),
+		Flags:   ComputeFlags{Serverless: "v4"},
+		Compute: stubCompute{}, PM: pm,
+	}
+
+	// Cancel once Provision is running, so the run unblocks and returns through the
+	// interrupt path (mirrors a Ctrl-C landing mid-`uv sync`).
+	go func() {
+		<-pm.entered
+		cancel()
+	}()
+
+	res, err := p.Run(ctx)
+	var pe *PipelineError
+	require.ErrorAs(t, err, &pe)
+	assert.Equal(t, ErrCanceled, pe.Code)
+	assert.Equal(t, PhaseProvision, pe.FailurePhase, "should still record where it stopped")
+	require.NotNil(t, res.Error)
+	assert.Equal(t, ErrCanceled, res.Error.Code)
+	assert.False(t, res.OK)
+	// The wrapped cause is the context error, so errors.Is works upstream.
+	assert.ErrorIs(t, pe, context.Canceled)
+
+	// The phase's own error is kept as a second cause, not discarded: a genuine
+	// failure can race with the signal, and its stderr is the only diagnostic
+	// there is. uvFailure folds that stderr into Msg, so preserving only the inner
+	// .Err would drop it — assert the actual resolver output survives.
+	assert.Contains(t, pe.Error(), "signal: terminated",
+		"the phase's cause must survive the reclassification")
+	assert.Contains(t, pe.Error(), cancelPMStderr,
+		"uv's stderr (the real diagnostic) must survive the cancellation reclassification")
+
+	// Text mode prints the errored phase's Detail while --json prints the error
+	// object; they must agree (see PipelineError.MarshalJSON). Detail is set when
+	// the phase fails, i.e. before the reclassification, so this catches a stale one.
+	var detail string
+	for _, ph := range res.Phases {
+		if ph.Phase == PhaseProvision {
+			detail = ph.Detail
+		}
+	}
+	assert.Equal(t, pe.Error(), detail, "text-mode phase detail must match the JSON error")
+	// A Ctrl-C must be *classified* as cancellation, not a provision failure: the
+	// message leads with "interrupted" (Code is E_CANCELED). The retained cause may
+	// still contain the phase's own "... failed" text — that is the preserved
+	// diagnostic, not the classification — so assert the prefix, not absence.
+	assert.True(t, strings.HasPrefix(detail, "interrupted"),
+		"a Ctrl-C must read as interrupted, not a provision failure: %q", detail)
+
+	// Both causes render on one line: a phase row is a single line of output.
+	assert.NotContains(t, pe.Error(), "\n", "the error must stay single-line")
+}
+
 func TestPipelineCheckReRunPlanMatchesRealRun(t *testing.T) {
 	// On a re-run where the .bak already exists and the live file already equals
 	// the merged output, --dry-run must report a plan a real run would perform: no

libs/localenv/result.go

@@ -87,6 +87,13 @@ const (
 	ErrPythonInstall      ErrorCode = "E_PYTHON_INSTALL"      // provision: uv python install failed
 	ErrProvision          ErrorCode = "E_PROVISION"           // provision: uv sync failed
 	ErrValidate           ErrorCode = "E_VALIDATE"            // validate: post-provision version mismatch
+
+	// ErrCanceled is not in the spec's error-code table: it reports a user/parent
+	// interrupt (SIGINT/SIGTERM cancels the context), not a failure of the phase
+	// it happened to be in. Without it an interrupt mid-`uv sync` surfaces as
+	// E_PROVISION with a "provision failed" message, implying something broke when
+	// the user simply pressed Ctrl-C. FailurePhase still records where it stopped.
+	ErrCanceled ErrorCode = "E_CANCELED" // any phase: interrupted by SIGINT/SIGTERM
 )
 
 // PipelineError is a failure carrying a stable code, the phase at which it