Skip to content

test(hosted): isolate package-manager caches in the production e2e suite - #144

Open
John-David Dalton (jdalton) wants to merge 1 commit into
test/isolate-package-manager-cachesfrom
test/hosted-production-cache-isolation
Open

test(hosted): isolate package-manager caches in the production e2e suite#144
John-David Dalton (jdalton) wants to merge 1 commit into
test/isolate-package-manager-cachesfrom
test/hosted-production-cache-isolation

Conversation

@jdalton

@jdalton John-David Dalton (jdalton) commented Aug 2, 2026

Copy link
Copy Markdown

e2e_hosted_production.rs is now the one CLI integration suite that still writes package-manager caches into whoever's home directory ran it. #143 fixed every other suite in crates/socket-patch-cli/tests/; this file needs the identical one-liner, and that is the whole diff — a cache_env::isolate(&mut cmd) call in the shared tool() helper, plus the mod declaration that brings the helper in.

It is a separate PR because the CI job that covers this file, hosted-e2e, is currently red for a reason that has nothing to do with this change. To be straight about what the split does and does not buy: that job is a required check with no path filter, so it runs — and fails — on every open PR right now, #143 included. Splitting this file out does not turn #143 green. What it buys is a clean diff, so #143 stays a pure "isolate the caches" change and the whole argument about a production-registry failure lives here, in the one PR that actually touches the file.

Stacked on #143. The helper module tests/common/cache_env.rs arrives in #143, so this branch is based on test/isolate-package-manager-caches and the base branch is set to it. Merge #143 first; GitHub will retarget this one to main automatically. Based directly on main, this diff does not compile at all (couldn't read crates/socket-patch-cli/tests/common/cache_env.rs), which would turn every Rust check red instead of just the one.

What the change does — sandboxes the caches the per-leg env list does not already name

The suite drives a real install for every ecosystem it covers, and every one of those installs goes through a single helper, tool(). That helper already hands each leg a private cache for the toolchains the leg names by hand. Anything it does not name still comes out of the real home directory.

Corepack is the big one. The first pnpm or yarn call downloads an entire package manager into ~/.cache/node/corepack — roughly 900 files from a single invocation. cache_env::isolate() closes that and the rest of the set in one call.

Placement matters, and it is the same placement used everywhere else in #143:

Order What runs Why it has to be there
1 the existing ambient-env scrubs they iterate the parent environment, so running them later would strip the sandbox values right back out
2 cache_env::isolate(&mut cmd) this PR
3 the per-leg env entries the caller passes Command's env ops are keyed by name and last-write-wins, so the caller still gets the final say

Step 3 is what keeps the deliberately-cold legs cold. Several legs pass an empty cache directory on purpose, to prove a fresh checkout installs the patched bytes rather than a warm host copy of the pristine one. Those values are applied last, so they still win.

The red check is a server-side change, not this diff — one leg, and it fails the same way with the file untouched

Expect hosted-e2e to be red on this PR. It was already red before this change existed, and it will stay red until the registry side clears. Please do not read it as a signal about this diff.

The failing leg is gem_bundler_hosted_redirect_and_known_install_defect. Bundler falls back from the compact index to the dependency API against Socket's gem patch-registry, and gets back a body it cannot unmarshal:

ArgumentError: marshal data too short

The test is written to tolerate one specific known server defect — APIResponseMismatchError — and to report anything else as a new regression. This is a different error, so the test flags it. Thirteen of the suite's fourteen legs pass.

Three runs say it is not the code:

Run What the file looked like Result
Job 91472097503 with this one-line change that leg fails; 13 of 14 pass
Job 91471604467 restored byte-identical to main fails the same way
Job 91508490068#143's current head byte-identical to main, on a branch that no longer touches the file at all fails the same way; 13 of 14 pass

The last green run of hosted-e2e on main was 31 July. So the code is not the variable — something moved on the server between then and now.

Unblocking the check itself is not something this PR can do. It needs either the gem patch-registry fix, or a repo admin setting the documented escape hatch (HOSTED_E2E_DISABLED = true under Settings → Secrets and variables → Actions → Variables, then re-running the failed job, and deleting the variable afterwards to re-arm).

Why the job cannot just be skipped for this PR — the workflow rules it out on purpose

Worth knowing before anyone suggests a path filter or an if:. The job carries explicit invariants in ci.yml, and they are there for good reasons:

Invariant Why
no job-level if: a skipped required check is ambiguous to branch protection and wedges the PR at "Expected — waiting for status"
no needs: an upstream failure would skip the job, same wedge
no matrix, no rename branch protection matches the check name hosted-e2e exactly
no continue-on-error a bypass has to be visible, not silent

So the job always runs and always lands on success or failure. The kill switch gates the steps, never the job.

What I checked — it compiles and it is clean; the suite itself is not something to run from a laptop

Rancargo check --workspace --all-targets on this branch. Clean. The mod cache_env; declaration resolves because #143 supplies the file.

Rancargo fmt --check. No diff in this file. (Two unrelated pre-existing diffs show up in src/commands/lock_cli.rs and src/lib.rs; both reproduce on main.)

Rancargo clippy --workspace --all-targets. No new warnings from this file.

Did not run — the suite itself. It contacts production Socket endpoints and real package registries for every ecosystem. Driving that traffic from a laptop to verify a cache path is not a reasonable trade, so CI is the check here.

Trade-off — the sandbox is a stable directory under the OS temp dir, not a fresh one per run, so the fixture packages are not re-downloaded on every run. That is the same trade-off #143 makes and the same directory it uses; the details are in that PR.

e2e_hosted_production.rs drives a real install for every ecosystem it
covers — npm, pnpm, yarn, bun, go, pip, gem, bundler, composer, nuget —
through one `tool()` helper. That helper pins a private cache per leg for
the toolchains each leg names, but everything it does not name still
resolves out of the caller's home directory. Corepack is the big one: the
first `pnpm`/`yarn` invocation downloads a whole package manager into
~/.cache/node/corepack.

`cache_env::isolate()` is the same one-liner the rest of the CLI
integration suites use. It goes in before the per-leg `env` entries so
those still win, which is what keeps the deliberately-cold reinstall legs
cold.

Split out of the sibling change that isolates the other suites, because
this file's CI leg (`hosted-e2e`) is currently red for an unrelated
server-side reason and should not hold that PR up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant