Skip submodule clones in CacheDeploySpecJob

[aqeelvn]

What

Pass recursive: false to with_temporary_working_directory in CacheDeploySpecJob, so caching a deploy spec no longer clones the repo's submodules.

Why

CacheDeploySpecJob runs on every GithubSyncJob completion (every push webhook, per stack on the branch). Each run creates a temporary working directory via git clone --recursive:

• The superproject clone is cheap — it comes from the worker's local git cache on the same filesystem (hardlinked object store).
• The submodules are not in that cache, so --recursive fetches every one of them from their remotes over the network, on every run, into the tmpdir on the worker's ephemeral disk.

The --recursive default exists for the other users of this helper (deploys, commit checks, fetch-deployed-revision), which execute user-defined commands that may genuinely need submodule content (CHANGELOG 0.33.0, #1110). Spec caching executes nothing: the complete file-access surface of DeploySpec::FileSystem#cacheable (verified across all seven discovery modules) is config candidates (shipit*.yml at root/.shipit/), inherit_from chains, and single-level discovery probes (Gemfile, package.json, setup.py, *.gemspec, …) resolved against the superproject tree — optionally re-rooted under machine.directory.

Risk

The only way this change can alter a spec is a stack whose machine.directory (or an inherit_from target) resolves inside a submodule — such a stack would get a silently different spec, not an error.

Verified empirically against production (2026-08-12), instead of by inference — an earlier revision of this description leaned on build_cacheable_deploy_spec as an already-recursive: false precedent, which review correctly identified as dead code (orphaned by 2841e17; no callers in this engine, the host app, or any indexed Shopify repo):

• 3 non-archived stacks set machine.directory in their cached spec.
• 2 verified safe: no .gitmodules overlap with the configured directory.
• 1 (shopify/extension-points/eps_gem) is a zombie: the repository was renamed (Shopify/scripts-apis) and the configured directory eps_gem no longer exists on the branch — this change cannot alter its (already broken) evaluation. Flagged for archiving/fixing independently.
• 0 stacks whose spec evaluation depends on submodule content.

Known limitation of the query: cached specs are post-merge, so inherit_from targets are not directly queryable; an inherit-target-inside-a-submodule would be invisible to it. #1497's shadow mode provides the empirical backstop for any residual case before the checkout-less path trusts its own results.

Impact

For deployments with many stacks and busy branches (e.g. Shopify's shipit-production during the 2026-08-11 incident), this cuts per-run network egress, ephemeral disk usage, job runtime, and memory pressure from the highest-volume git-touching job in the system. Shorter runtimes also narrow the duplicate-admission window that #1494 closes fully.

Test updated to expect the new argument; deploy_commands_test, commit_checks_test, and background_job_test pass unchanged.

[timothysmith0609]

Reviewed as part of a pass over the whole stack (#1493 → #1497). The change itself is right — CacheDeploySpecJob reads config files and single-level discovery probes, none of which need submodule content, and fetching every submodule from its remote on every push webhook is pure waste.

Two things before this merges, though, because unlike #1497 this takes effect immediately at merge on the system that's currently OOMKilling.

🔴 The risk-section argument rests on a dead code path

StackCommands#build_cacheable_deploy_spec, which computes the same cacheable spec, already passes recursive: false. This change simply aligns the job with the existing behavior of that sibling code path.

build_cacheable_deploy_spec has no callers. git grep at this branch finds only its own definition plus one test, and git log -S build_cacheable_deploy_spec shows it was orphaned by "Get rid of an unused job" (2841e17).

So the follow-on claim — "such a stack would already get a wrong spec from build_cacheable_deploy_spec today" — isn't true. Nothing exercises that path, which means no stack is currently getting the recursive: false behavior for its cached spec, and this PR is the first thing to introduce it. Worth grepping Shopify/shipit for a host-app caller, but either way the risk argument needs to stand on its own rather than leaning on the sibling.

🟡 The actual exposure is worth measuring, not just reasoning about

A stack whose machine.directory — or an inherit_from target — resolves inside a submodule gets a silently different spec, not an error. The submodule directory is simply empty, so .exist? returns false and discovery falls through to different steps.

That's cheap to rule out empirically rather than argue about: query existing cached_deploy_spec values for machine.directory and cross-reference against .gitmodules in the relevant repos. If it comes back empty, the risk section becomes a fact instead of an inference, and I'd approve immediately.

Happy to be talked out of the second point if you think the population is obviously safe — but the first one should be fixed in the description regardless, since a future reader will otherwise take the sibling-path claim at face value.

[aqeelvn]

Both points addressed:

Dead code path — you're right, and thanks for the catch: build_cacheable_deploy_spec has no callers in this engine, in Shopify/shipit, or in any indexed Shopify repo (orphaned by 2841e174). The description has been rewritten so the safety case stands on the file-access inventory + empirical data rather than the sibling-path inference.

Empirical check — ran against production just now:

Stacks with machine.directory set: 3
SAFE:       2   (no .gitmodules overlap with the configured directory)
SUSPICIOUS: 0
ZOMBIE:     1   (shopify/extension-points/eps_gem)

The third deserves its footnote: every API probe against it initially returned "Moved Permanently" message bodies (which a naive check would have mis-parsed as "no submodules" — we chased it down via the git trees API by repo id). The repo was renamed to Shopify/scripts-apis, and the configured machine.directory: eps_gem no longer exists on the branch. Its cached spec is already stale regardless of this PR; probes into a nonexistent directory find nothing with or without submodules. Flagged for archiving/fixing separately.

Net: 0 stacks whose spec evaluation depends on submodule content. Query limitation noted in the description: inherit_from targets aren't visible in post-merge cached specs; #1497's shadow mode is the backstop for that residual.