Evaluate cacheable deploy specs from the git object database

[aqeelvn]

Stacked on #1496 (→ #1495 → #1494 → #1493). Final piece of the deploy-jobs OOM/clone-storm remediation: makes the legitimate spec-cache work ~100x cheaper instead of just reducing the wasted work.

Ships inert: Shipit.checkout_less_deploy_spec defaults to :disabled — zero behavior change at merge. Design doc reviewed adversarially over 3 iterations before implementation.

Why

CacheDeploySpecJob clones from the local git cache and checks out the entire working tree (multi-GB for large repos) to read ~25 small files. Spec evaluation runs no user code — unlike deploys/commit-checks, it doesn't need a tree at all. Every file it touches can be served straight from the git object database (git cat-file/ls-tree) in milliseconds, writing only the ~25 small files actually accessed (materializing real files rather than virtualizing preserves exact Pathname/Dir[] semantics, which is what makes the golden tests meaningful).

Design

GitObjectFileSystem < DeploySpec::FileSystem — constructed on an empty tmpdir. Spec evaluation touches disk through exactly two seams (verified across all 7 discovery modules): file() and build_config() (the inherit_from chain). Both are overridden to materialize the requested path before returning, through a single choke point that:

1. validates containment (cleanpath prefix — inherit_from: /etc/passwd or ../../ → fallback; note the checkout path happily reads those off the worker filesystem today)
2. walks path components against cached ls-tree listings, rejecting symlinks (cat-file on a symlink returns the link target text — must never be served as content), submodule gitlinks, and files-as-directories
3. falls back if any ancestor directory carries .gitattributes (checkout applies eol/text/smudge filters; cat-file emits raw bytes) — correctly scoped: attributes only affect their own subtree, so unrelated ones don't trigger it
4. caps inherit_from chains at 10 (the checkout path loops forever on a cycle today)

StackCommands#cacheable_deploy_spec(commit:) wraps both strategies:

• :disabled → today's checkout path
• :enabled → object-database path; any doubt or error → checkout fallback (reason-tagged ActiveSupport::Notifications + logs)
• :shadow → runs both, returns the checkout result, reports divergence — the production proving mode. Comparison normalizes each side's ephemeral evaluation root out first (specs embed it, e.g. release-gem <dir>/x.gemspec)

The wrapper returns a detached plain DeploySpec, so serialization can never touch a dead tmpdir (structural version of the guarantee the old code gets by calling update! inside the block).

Implementation notes

• Object reads bypass Shipit::Command deliberately: it spawns via PTY, which rewrites \n→\r\n and can't carry NUL-delimited (ls-tree -z) or raw binary output. Open3.capture3 with binmode instead; failures raise Command::Failed for uniform rescue.
• ls-tree parsing splits records on the first tab only (filenames may contain tabs); listings are memoized per canonical repo-relative key.
• Metrics via ActiveSupport::Notifications (checkout_less_deploy_spec.shipit) so the engine stays dependency-free; host apps subscribe for StatsD.

Tests (38 new)

• Golden equivalence on real git init repos: checkout-based vs object-based evaluation must produce identical configs — config variants (bare/env/.shipit/priority), inherit chains (same-dir/subdir/3-deep), machine.directory nesting, gemspec globs (0/1/many, filename with space), content-reading discoveries (package.json/lerna.json), .gitattributes in an unrelated subtree (must NOT fall back)
• Fallback guards: every reason — escape (relative + absolute), cycle/depth, symlinked file, symlinked intermediate dir, gitlink, file-in-path, .gitattributes at root and in machine.directory
• Idempotency: repeated access = one object read
• Wrapper: all three modes, full rescue matrix, shadow never propagates new-path errors, old-path errors propagate unchanged

Notable catch during implementation: build_config checks inherits_from_path.exist? before read_config, so intercepting only read_config silently dropped inherit chains — caught by the golden tests, which is exactly the failure mode they exist for. A comment now marks the seam invariant in FileSystem.

Rollout (separate one-line PRs in Shopify/shipit)

1. Merge (inert) → 2. preflight: confirm workers' gitconfig sets no core.autocrlf/filters → 3. :shadow off-peak, advance on 7 days of zero mismatches + <1% explained fallbacks → 4. :enabled (fallback stays forever) → 5. optional shadow-plumbing cleanup.

[timothysmith0609]

Approving for merge — it ships inert, and the design is unusually careful. Everything below is about what should change before :shadow, not before merge.

I independently verified the central claim rather than taking it on faith: across all seven discovery modules (Npm, Lerna, Pypi, Rubygems, Capistrano, Bundler, Kubernetes), file() and build_config() really are the only paths to disk. The build_config-checks-exist?-before-read_config catch is real — intercepting read_config alone would have silently dropped every inherit chain — and adding the seam comment to FileSystem is the right way to keep that from rotting.

Also worth calling out: returning a detached plain DeploySpec is a genuine structural improvement over serializing a live FileSystem inside the block. I checked that it's behavior-preserving — DeploySpec.dump calls spec.cacheable.config, and base DeploySpec#cacheable returns self — so the job change is a no-op today and a real safety property tomorrow.

---

🔴 The glob branch of file() skips path-component validation

if path.to_s.match?(GLOB_CHARS)
  dir = repo_rel(pathname.dirname)
  entries(dir).each_key { ... }        # straight to ls-tree
else
  materialize(repo_rel(pathname))      # blob_mode walks and validates every component
end

entries(dir) issues git ls-tree -z <sha> -- '<dir>/' without walking dir's own components through blob_mode. Verified against a real repo:

$ git ls-tree -z $SHA -- 'linkdir/'    # symlinked directory
                                        (empty, exit 0)
$ git ls-tree -z $SHA -- 'plain.txt/'  # regular file used as a directory
                                        (empty, exit 0)

So the glob branch sees an empty directory exactly where the non-glob branch would raise :symlink or :file_in_path. Meanwhile Dir[] on a real checkout happily follows the symlink and finds the files. That's a wrong spec, not a fallback — the one place in the design where "any doubt → fall back" doesn't hold.

It's latent today: the only glob caller is gemspec, and review_checklist → PypiDiscovery#egg? → file('setup.py') is evaluated earlier in cacheable_config's hash literal and does go through blob_mode, so a symlinked machine.directory raises before the glob is ever reached. But that's an ordering accident in a hash literal, not an invariant — it breaks the day someone reorders cacheable_config or adds a second glob probe. Given how carefully everything else fails closed, this should too:

def validate_dir!(dir)
  return if dir.empty?

  case blob_mode(dir)
  when TREE_MODE, :absent then nil
  when SYMLINK_MODE then raise FallbackRequired.new(:symlink, dir)
  when GITLINK_MODE then raise FallbackRequired.new(:submodule, dir)
  else raise FallbackRequired.new(:file_in_path, dir)
  end
end

Worth a golden test with a glob under a symlinked machine.directory where no earlier probe fires, so the ordering accident can't quietly become the thing holding it up.

🟡 Shadow mode destroys the data its own rollout gate needs

rescue StandardError => e
  notify_checkout_less(:fallback, reason: :shadow_error, detail: "#{e.class}: #{e.message}")

Step 3 advances on "7 days of zero mismatches and <1% explained fallbacks." Shadow is the only mode that will ever produce that data — and in shadow every FallbackRequired (:symlink, :gitattributes, :escape, :inherit_depth) arrives tagged :shadow_error, categorizable only by string-parsing detail. The :enabled path already does this correctly; shadow just needs the same rescue ahead of the catch-all:

rescue DeploySpec::GitObjectFileSystem::FallbackRequired => e
  notify_checkout_less(:fallback, reason: e.reason, detail: e.detail)
rescue StandardError => e
  notify_checkout_less(:fallback, reason: :shadow_error, detail: "#{e.class}: #{e.message}")

Related: the payload carries no mode:, so a shadow :hit and an enabled :hit are indistinguishable in the metric stream. Cheap to add and useful during a staged rollout where both will be live.

🟡 git_read runs git with the worker's ambient environment

Open3.capture3('git', *args, chdir: @stack.git_path.to_s, binmode: true)

Every other git invocation in shipit goes through Shipit::Command, which uses BASE_ENV — Bundler's unbundled env with everything else explicitly nulled. This one inherits whatever the worker process happens to have. If GIT_DIR, GIT_WORK_TREE, or GIT_OBJECT_DIRECTORY is set, chdir: is silently ignored and reads target the wrong repository.

Bypassing Command for the PTY reason is clearly right, and the comment explains it well. Inheriting the ambient env reads like an unintended side effect of that rather than a decision — worth passing an explicit env that at minimum clears the GIT_* repo-selection vars.

🟡 The .gitattributes guard leaves config-level conversions to a manual preflight

The tree scan itself is correct — I traced the ancestor walk and every listed ancestor plus the file's own directory is checked, with unrelated subtrees genuinely not triggering it. It also covers the case that matters most, since LFS and smudge filters require an in-tree .gitattributes.

But core.autocrlf, core.eol, and global core.attributesfile also make a checkout diverge from cat-file, and leave no trace in the tree. Confirmed with global core.autocrlf=true and no in-tree .gitattributes:

checkout:  d e p l o y :  \r \n  ...
cat-file:  d e p l o y :  \n     ...

Practical impact is small — YAML and JSON both tolerate CRLF — so this isn't urgent. The structural issue is that rollout step 2 is a point-in-time human check on a value a base-image bump can change months later, at which point :enabled silently serves different bytes with no signal at all. One git config --get per instance, falling back with reason: :git_config, converts a procedural guarantee into a permanent one.

Flip side worth recording in the doc comment: $GIT_DIR/info/attributes needs no coverage, because git clone doesn't copy it, so the checkout path never sees it either. That's a non-obvious piece of the safety argument.

🟢 :inherit_depth falls back into the same infinite recursion

The depth cap is a real improvement over the parent, but on FallbackRequired(:inherit_depth) the :enabled path falls back to checkout_cacheable_deploy_spec, which runs the uncapped parent build_config. On a genuine cycle that recurses to SystemStackError — not a StandardError, so it propagates past every rescue in the wrapper — or gets killed by #1494's 15-minute Timeout. For that one reason the fallback is strictly worse than not falling back.

Capping the parent is ~2 lines and fixes the bug for everyone, including today's :disabled path, which is the only path running in production right now. Might be worth pulling out as its own small PR ahead of this one.

🟢 Minor

• git_ls_dir builds the pathspec as "#{dir}/" with no magic prefix, so a directory containing *, ?, [, or a leading : is interpreted as a glob or pathspec magic. ":(literal)#{dir}/" closes it for free.
• The object path reads @stack.git_path live where the checkout path clones a snapshot first. A concurrent ClearGitCacheJob or git gc degrades to Command::Failed → fallback, which is correct — and since every read is pinned to @sha there's no torn-read hazard. Worth a line in the class doc, since it's a new reliance on the fallback for a case that previously couldn't arise.
• "no disk or memory footprint" slightly overstates it — ~25 small files are still written to a tmpdir. The decision to materialize rather than virtualize is a good one: it preserves Pathname#exist? / #read / Dir[] semantics exactly, which is precisely what makes the golden tests meaningful. Just worth describing accurately.
• The :disabled wrapper test stubs with_temporary_working_directory to return [spec, root] without yielding, so the block and the .first unwrap are never exercised together. The golden tests cover the real path; noting only that the wrapper suite is pure mock choreography.
• No end-to-end assertion that CacheDeploySpecJob persists an :enabled-mode result — the job test still mocks with_temporary_working_directory. The FileSystem → plain DeploySpec change is behavior-preserving as noted above, but it's the kind of invariant worth one assertion rather than one reviewer's trace.

---

The golden-equivalence suite is the right shape for this problem, and the fact that it caught the build_config seam during implementation is the best available evidence that it works. My only real ask is the glob-branch validation and the shadow-mode reason tagging before flipping to :shadow — the first because it's the one silent-divergence path left, the second because without it step 3's gate can't actually be evaluated.

[aqeelvn]

All items addressed in 26eab67f + one extraction:

• 🔴 Glob validation gap closed with your validate_dir! shape — the glob branch's directory now goes through the same component walk; reproduced your empty-listing-exit-0 behavior in a test (file('linkdir/*.gemspec') under a symlink → :symlink fallback, exercised directly so the cacheable_config ordering accident can't mask it).
• 🟡 Shadow taxonomy preserved: FallbackRequired rescued ahead of the catch-all with its reason/detail intact; events now carry mode:. Both tested.
• 🟡 git_read env: clears the GIT_* repository-selection variables explicitly.
• 🟡 Config-level conversions: now a runtime guard (core.autocrlf/core.attributesfile → FallbackRequired(:git_config)) instead of the point-in-time preflight. autocrlf=false/input allowed; core.eol deliberately excluded — inert without text attributes, which the in-tree .gitattributes guard and the attributesfile check cover. Your info/attributes-needs-no-coverage observation is recorded in the guard's comment.
• 🟢 Parent recursion cap extracted to Cap inherit_from recursion in DeploySpec::FileSystem #1498 as suggested — fixes today's :disabled path and removes the fallback-is-worse case.
• Minors: :(literal) pathspec, live-cache reliance documented in the class docstring, description wording fixed.