From ecb95231c7b1b346f2c6a63eaa51d84e3d8d725b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mads=20B=C3=B8geskov?= Date: Tue, 28 Jul 2026 13:42:50 +0200 Subject: [PATCH 1/2] feat: skip pulling an already cloned plan when CI is set Every shuttle invocation pulls the plan again. In CI the plan was just cloned at the start of the job and cannot change during it, so every invocation after the first pays for a pull that can never find anything. Skip pulling when CI is set, which most CI systems do. SHUTTLE_SKIP_PULL is the explicit control and takes precedence, so pulling can be forced back on with a falsy value. This only affects pulling an existing clone. A plan that is not yet available locally is still cloned. --- README.md | 19 +++++++++ pkg/git/git.go | 33 ++++++++++++++- pkg/git/skip_pull_test.go | 86 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 pkg/git/skip_pull_test.go diff --git a/README.md b/README.md index d7abdaa..6b0ee30 100644 --- a/README.md +++ b/README.md @@ -204,6 +204,25 @@ export SHUTTLE_CACHE_DURATION_MIN=60 # Cache a plan for 60 minutes This feature caches pr. repo, as such the cache isn't shared between working repositories. +#### Skipping the pull + +Every shuttle invocation pulls the plan again, which is wasted time in CI where +the plan was just cloned and cannot have changed during the job. Shuttle +therefore skips pulling an already cloned plan when the `CI` environment +variable is set, as most CI systems do. + +Use `SHUTTLE_SKIP_PULL` to control this explicitly. It takes precedence over +`CI`, so it can also force pulling back on: + +```bash +export SHUTTLE_SKIP_PULL=true # never pull an already cloned plan +export SHUTTLE_SKIP_PULL=false # always pull, even when CI is set +``` + +The `--skip-pull` flag does the same for a single invocation. Note that none of +these prevent the initial clone; a plan that isn't available locally yet is +always cloned. + ### Overloading the plan It is possible to overload the plan specified in `shuttle.yaml` file by using diff --git a/pkg/git/git.go b/pkg/git/git.go index 25e356a..da30cab 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -29,6 +29,34 @@ var gitRegex = regexp.MustCompile( const cacheDurationMinKey = "SHUTTLE_CACHE_DURATION_MIN" +const skipPullKey = "SHUTTLE_SKIP_PULL" + +// skipPullFromEnv reports whether plan pulling should be skipped based on the +// environment. SHUTTLE_SKIP_PULL is the explicit opt in and takes precedence, +// so it can also force pulling back on with a falsy value. Otherwise CI is +// honoured, as CI jobs start from a fresh clone and pulling the plan again on +// every shuttle invocation only costs time. +func skipPullFromEnv(uii *ui.UI) bool { + if v, ok := os.LookupEnv(skipPullKey); ok { + skip, err := strconv.ParseBool(v) + if err != nil { + // An unparsable value is treated as set, matching how CI systems + // tend to use env vars as mere presence flags. + uii.Verboseln("%s is not a boolean, treating '%s' as true", skipPullKey, v) + return true + } + uii.Verboseln("Skipping git plan pulling because %s=%s", skipPullKey, v) + return skip + } + + if os.Getenv("CI") != "" { + uii.Verboseln("Skipping git plan pulling because CI is set") + return true + } + + return false +} + func ParsePlan(plan string) Plan { if !gitRegex.MatchString(plan) { return Plan{ @@ -121,6 +149,9 @@ func GetGitPlan( uii.Verboseln("Skipping git plan pulling") return planPath, nil } + if skipPullFromEnv(uii) { + return planPath, nil + } valid, err := cacheIsValid(planPath) if err != nil { return "", err @@ -172,7 +203,7 @@ func GetGitPlan( if cloneToken != "" { uii.Verboseln("Found clone token in env, but shuttle path was ssh-based. This override will not work.") } - + cloneArg = parsedGitPlan.User + "@" + parsedGitPlan.Repository } else { panic(fmt.Sprintf("Unknown protocol '%s'", parsedGitPlan.Protocol)) diff --git a/pkg/git/skip_pull_test.go b/pkg/git/skip_pull_test.go new file mode 100644 index 0000000..e66a85c --- /dev/null +++ b/pkg/git/skip_pull_test.go @@ -0,0 +1,86 @@ +package git + +import ( + "io" + "os" + "testing" + + "github.com/lunarway/shuttle/pkg/ui" + "github.com/stretchr/testify/assert" +) + +func TestSkipPullFromEnv(t *testing.T) { + tt := []struct { + name string + skipPull *string + ci *string + skipsPull bool + }{ + { + name: "nothing set", + skipsPull: false, + }, + { + name: "CI set", + ci: strPtr("true"), + skipsPull: true, + }, + { + name: "CI set to any non-empty value", + ci: strPtr("1"), + skipsPull: true, + }, + { + name: "CI set but empty", + ci: strPtr(""), + skipsPull: false, + }, + { + name: "SHUTTLE_SKIP_PULL set", + skipPull: strPtr("true"), + skipsPull: true, + }, + { + name: "SHUTTLE_SKIP_PULL set but empty is treated as set", + skipPull: strPtr(""), + skipsPull: true, + }, + { + name: "SHUTTLE_SKIP_PULL takes precedence over CI", + skipPull: strPtr("false"), + ci: strPtr("true"), + skipsPull: false, + }, + { + name: "SHUTTLE_SKIP_PULL with an unparsable value is treated as set", + skipPull: strPtr("yes-please"), + skipsPull: true, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + // Unset by default so the developer's own environment does not leak + // into the test. + t.Setenv(skipPullKey, "") + os.Unsetenv(skipPullKey) + t.Setenv("CI", "") + os.Unsetenv("CI") + + if tc.skipPull != nil { + t.Setenv(skipPullKey, *tc.skipPull) + } + if tc.ci != nil { + t.Setenv("CI", *tc.ci) + } + + uii := ui.Create(io.Discard, io.Discard) + + assert.Equal(t, tc.skipsPull, skipPullFromEnv(uii)) + }) + } +} + +func strPtr(s string) *string { + return &s +} From baf2f990163923522c0125dedce45ecc1b66f8e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mads=20B=C3=B8geskov?= Date: Tue, 4 Aug 2026 10:22:08 +0200 Subject: [PATCH 2/2] fix(git): default unparsable SHUTTLE_SKIP_PULL to false and fix skip log Per review: default an unparsable SHUTTLE_SKIP_PULL to false rather than true, so pulling stays on unless explicitly skipped. Also only log the skip message when pulling is actually being skipped, since it was logged unconditionally even for falsy values like SHUTTLE_SKIP_PULL=false. --- pkg/git/git.go | 10 +++++----- pkg/git/skip_pull_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/git/git.go b/pkg/git/git.go index da30cab..4a106c1 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -40,12 +40,12 @@ func skipPullFromEnv(uii *ui.UI) bool { if v, ok := os.LookupEnv(skipPullKey); ok { skip, err := strconv.ParseBool(v) if err != nil { - // An unparsable value is treated as set, matching how CI systems - // tend to use env vars as mere presence flags. - uii.Verboseln("%s is not a boolean, treating '%s' as true", skipPullKey, v) - return true + uii.Verboseln("%s is not a boolean, treating '%s' as false", skipPullKey, v) + return false + } + if skip { + uii.Verboseln("Skipping git plan pulling because %s=%s", skipPullKey, v) } - uii.Verboseln("Skipping git plan pulling because %s=%s", skipPullKey, v) return skip } diff --git a/pkg/git/skip_pull_test.go b/pkg/git/skip_pull_test.go index e66a85c..b9e49c6 100644 --- a/pkg/git/skip_pull_test.go +++ b/pkg/git/skip_pull_test.go @@ -41,9 +41,9 @@ func TestSkipPullFromEnv(t *testing.T) { skipsPull: true, }, { - name: "SHUTTLE_SKIP_PULL set but empty is treated as set", + name: "SHUTTLE_SKIP_PULL set but empty is treated as false", skipPull: strPtr(""), - skipsPull: true, + skipsPull: false, }, { name: "SHUTTLE_SKIP_PULL takes precedence over CI", @@ -52,9 +52,9 @@ func TestSkipPullFromEnv(t *testing.T) { skipsPull: false, }, { - name: "SHUTTLE_SKIP_PULL with an unparsable value is treated as set", + name: "SHUTTLE_SKIP_PULL with an unparsable value is treated as false", skipPull: strPtr("yes-please"), - skipsPull: true, + skipsPull: false, }, }