Skip constraint cache writes under --check

A --check dry run must not mutate disk, but FetchConstraints wrote the
fetched artifact into the cache dir on every successful live fetch — so a
dry run still populated the cache whenever it was writable, even though the
pipeline skips every other write-side step under --check.

Thread a writeCache flag through FetchConstraints; the pipeline passes
!p.Check. An existing cache is still read for offline fallback, since
reading is not a mutation. Assert both directly (FetchConstraints with
writeCache=false leaves the cache dir empty) and end-to-end (the --check
pipeline test now checks the cache dir stays empty too).

Reported by codex review of the pipeline PR.

Co-authored-by: Isaac

Author: rugpanov

libs/localenv/constraints.go

@@ -137,7 +137,12 @@ func writeCacheAtomic(path string, data []byte) error {
 // baseURL points at the repo hosting the constraint artifacts (see
 // RepoConstraintBaseURL); it is empty when no source is configured, which is
 // reported below as a fetch-phase error.
-func FetchConstraints(ctx context.Context, baseURL, envKey, cacheDir string) (*Constraints, error) {
+//
+// writeCache controls whether a successful live fetch populates the on-disk
+// cache. Callers pass false for a dry run (--check), which must not mutate
+// disk; an existing cache is still read for offline fallback, since reading is
+// not a mutation.
+func FetchConstraints(ctx context.Context, baseURL, envKey, cacheDir string, writeCache bool) (*Constraints, error) {
 	if baseURL == "" {
 		// No constraint host is configured. This is reported at the fetch phase (as
 		// E_FETCH) rather than aborting earlier, so the failure flows through the
@@ -158,9 +163,12 @@ func FetchConstraints(ctx context.Context, baseURL, envKey, cacheDir string) (*C
 			return nil, fmt.Errorf("parse constraints for %s: %w", envKey, err)
 		}
 		// Write the cache copy (creating cacheDir if needed, atomically); non-fatal
-		// so a read-only cacheDir doesn't break the command.
-		if err := writeCacheAtomic(cachePath, data); err != nil {
-			log.Debugf(ctx, "failed to write constraint cache %s: %v", filepath.ToSlash(cachePath), err)
+		// so a read-only cacheDir doesn't break the command. Skipped under a dry
+		// run so --check performs no disk writes at all.
+		if writeCache {
+			if err := writeCacheAtomic(cachePath, data); err != nil {
+				log.Debugf(ctx, "failed to write constraint cache %s: %v", filepath.ToSlash(cachePath), err)
+			}
 		}
 		return &Constraints{
 			EnvKey:            envKey,

libs/localenv/constraints_test.go

@@ -30,7 +30,7 @@ func TestRepoConstraintBaseURL(t *testing.T) {
 func TestFetchConstraintsNoSourceConfigured(t *testing.T) {
 	// An empty base URL means no constraint host is configured; it must classify as
 	// E_FETCH (surfaced at the fetch phase) and name the env var to set.
-	_, err := FetchConstraints(t.Context(), "", "serverless/serverless-v4", t.TempDir())
+	_, err := FetchConstraints(t.Context(), "", "serverless/serverless-v4", t.TempDir(), true)
 	var pe *PipelineError
 	require.ErrorAs(t, err, &pe)
 	assert.Equal(t, ErrFetch, pe.Code)
@@ -116,14 +116,31 @@ func TestFetchConstraintsCreatesCacheDir(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", cacheDir)
+	_, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", cacheDir, true)
 	require.NoError(t, err)
 	// The cache file was written into the freshly created directory.
 	written, err := os.ReadFile(filepath.Join(cacheDir, cacheFileName("serverless/serverless-v4")))
 	require.NoError(t, err)
 	assert.Equal(t, sampleToml, string(written))
 }
 
+func TestFetchConstraintsSkipsCacheWriteWhenDisabled(t *testing.T) {
+	// With writeCache=false (the --check dry-run path), a successful live fetch
+	// must not write anything to cacheDir.
+	cacheDir := t.TempDir()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte(sampleToml))
+	}))
+	defer srv.Close()
+
+	c, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", cacheDir, false)
+	require.NoError(t, err)
+	assert.False(t, c.FromCache)
+	entries, err := os.ReadDir(cacheDir)
+	require.NoError(t, err)
+	assert.Empty(t, entries, "no cache file should be written under --check")
+}
+
 func TestCacheFileNameInjective(t *testing.T) {
 	// Distinct env keys that flatten to the same slug must not collide, so a
 	// cache hit can never serve another environment's constraints.
@@ -140,7 +157,7 @@ func TestFetchConstraintsHTTP(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	c, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", t.TempDir())
+	c, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", t.TempDir(), true)
 	require.NoError(t, err)
 	assert.False(t, c.FromCache)
 	assert.Equal(t, "databricks-connect~=17.2.0", c.DatabricksConnect)
@@ -155,7 +172,7 @@ func TestFetchConstraintsEnvKeyNotFound(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := FetchConstraints(t.Context(), srv.URL, "dbr/99.9.x-scala2.12", t.TempDir())
+	_, err := FetchConstraints(t.Context(), srv.URL, "dbr/99.9.x-scala2.12", t.TempDir(), true)
 	var pe *PipelineError
 	require.ErrorAs(t, err, &pe)
 	assert.Equal(t, ErrEnvUnsupported, pe.Code)
@@ -167,7 +184,7 @@ func TestFetchConstraintsTransportFailureNoCache(t *testing.T) {
 	url := down.URL
 	down.Close()
 
-	_, err := FetchConstraints(t.Context(), url, "serverless/serverless-v4", t.TempDir())
+	_, err := FetchConstraints(t.Context(), url, "serverless/serverless-v4", t.TempDir(), true)
 	var pe *PipelineError
 	require.ErrorAs(t, err, &pe)
 	assert.Equal(t, ErrFetch, pe.Code)
@@ -182,7 +199,7 @@ func TestFetchConstraintsRejectsOversizedBody(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", t.TempDir())
+	_, err := FetchConstraints(t.Context(), srv.URL, "serverless/serverless-v4", t.TempDir(), true)
 	var pe *PipelineError
 	require.ErrorAs(t, err, &pe)
 	assert.Equal(t, ErrFetch, pe.Code)
@@ -194,12 +211,12 @@ func TestFetchConstraintsFallsBackToCache(t *testing.T) {
 	good := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		_, _ = w.Write([]byte(sampleToml))
 	}))
-	_, err := FetchConstraints(t.Context(), good.URL, "serverless/serverless-v4", cacheDir)
+	_, err := FetchConstraints(t.Context(), good.URL, "serverless/serverless-v4", cacheDir, true)
 	require.NoError(t, err)
 	good.Close()
 
 	// Now the server is down; fetch must serve the cache.
-	c, err := FetchConstraints(t.Context(), good.URL, "serverless/serverless-v4", cacheDir)
+	c, err := FetchConstraints(t.Context(), good.URL, "serverless/serverless-v4", cacheDir, true)
 	require.NoError(t, err)
 	assert.True(t, c.FromCache)
 }

libs/localenv/pipeline.go

@@ -187,8 +187,10 @@ func (p *Pipeline) resolve(ctx context.Context) (*TargetInfo, error) {
 }
 
 // fetch fetches constraints for the resolved target and records the fetch phase.
+// Under --check the cache is not populated, so a dry run performs no disk writes
+// (an existing cache is still read for offline fallback).
 func (p *Pipeline) fetch(ctx context.Context, target *TargetInfo) (*Constraints, error) {
-	c, err := FetchConstraints(ctx, p.ConstraintBaseURL, target.EnvKey, p.CacheDir)
+	c, err := FetchConstraints(ctx, p.ConstraintBaseURL, target.EnvKey, p.CacheDir, !p.Check)
 	if err != nil {
 		// FetchConstraints classifies the cause: E_ENV_UNSUPPORTED for a missing
 		// key (404) versus E_FETCH for transport failure with no cache. Both are

libs/localenv/pipeline_test.go

@@ -81,12 +81,13 @@ func newTestServer(t *testing.T) *httptest.Server {
 func TestPipelineCheckMutatesNothing(t *testing.T) {
 	dir := writeProject(t)
 	before, _ := os.ReadFile(filepath.Join(dir, "pyproject.toml"))
+	cacheDir := t.TempDir()
 	srv := newTestServer(t)
 	defer srv.Close()
 
 	p := &Pipeline{
 		Mode: ModeDefault, Check: true, ProjectDir: dir,
-		ConstraintBaseURL: srv.URL, CacheDir: t.TempDir(),
+		ConstraintBaseURL: srv.URL, CacheDir: cacheDir,
 		Flags:   TargetFlags{Serverless: "v4"},
 		Compute: stubCompute{}, PM: fakePM{py: "3.12", dbc: "17.2.0"},
 	}
@@ -98,6 +99,11 @@ func TestPipelineCheckMutatesNothing(t *testing.T) {
 	assert.Contains(t, res.Plan.Diff, "==3.12.*")
 	after, _ := os.ReadFile(filepath.Join(dir, "pyproject.toml"))
 	assert.Equal(t, string(before), string(after)) // unchanged
+
+	// --check must not populate the constraint cache either (no disk writes).
+	entries, err := os.ReadDir(cacheDir)
+	require.NoError(t, err)
+	assert.Empty(t, entries)
 }
 
 func TestPipelineCheckDoesNotProvision(t *testing.T) {