Skip to content

Base map bundles are published under a different compatibility key than the one they are looked up with #341

Description

@BenSheridanEdwards

Summary

In the styleproof-ci flow, the base bundle's publish key and its lookup key are computed from different working trees. When those trees differ in any compatibility input — most commonly the lockfile — the base bundle is written under a key nothing will ever ask for. The restore then misses permanently and the store accumulates bundles that can never be hit.

I hit this chasing a consumer whose map store was 0-for-5 on restores. I am filing rather than sending a patch because the fix depends on what the key is meant to mean, and getting that wrong in a cache key risks a silently wrong baseline — worse than the miss.

Where the two keys come from

Lookupsrc/map-store.ts:1096, in restoreCachedCaptureDirs. One key, computed from the consumer (head) checkout, used for both restores:

const compatibilityKey = expectedCompatibilityKey({ cwd, spec: options.spec, baseUrl: options.baseUrl });

restoreMapBundle({ sha: baseSha,, compatibilityKey });   // ← head-derived key
restoreMapBundle({ sha: headSha,, compatibilityKey });

Publishbin/styleproof-map.mjs:324, which defaults cwd to process.cwd():

const compatibilityKey = expectedCompatibilityKey({ spec });

For the base capture, styleproof-ci runs that in the cold base worktree (bin/styleproof-ci.mjs, the --dir base --upload invocation with coldBaseCwd). So the base bundle is keyed by the base tree, and looked up by the head tree.

compatibilityInput (src/map-store.ts:~360) reads lockfile, lockfileHash, playwrightVersion and packageVersion from that cwd. --spec-ref already equalises specHash by overlaying the head harness onto the base — the lockfile is not overlaid, by design ("pin the capture spec bytes from another commit while rendering the base checkout's app and lockfile", src/ci-spec-ref.ts).

Reproduction

Two trees with identical spec bytes — exactly what --spec-ref guarantees — differing only in package-lock.json:

import { expectedCompatibilityKey } from 'styleproof/dist/map-store.js';
// baseTree and headTree both contain e2e/styleproof.spec.ts with identical bytes;
// only package-lock.json differs.
expectedCompatibilityKey({ cwd: baseTree, spec: 'e2e/styleproof.spec.ts' });
expectedCompatibilityKey({ cwd: headTree, spec: 'e2e/styleproof.spec.ts' });
spec bytes identical in both trees; only package-lock.json differs
base worktree key (what the base bundle is PUBLISHED under): c2bf4f553dcf2948
head checkout key (what the base bundle is LOOKED UP under):  0a4cdb727604cf72
DIFFER → base restore can NEVER hit

So any pull request that touches the lockfile can never restore its own base, on any run, no matter how many times it re-runs — and each attempt uploads another unreachable bundle. playwrightVersion and packageVersion resolve per-tree too, so a base worktree that resolves them differently has the same effect.

What this costs

In the consumer where I found it, base capture is 7.6–13.5 minutes of every visual job — roughly half of the most expensive job in the repository, and the half that is pure repetition since the base is the default branch and identical across pull requests sharing a commit.

Worth separating from the above, because it is not a bug: when the base bundle is found, keys match. A consumer diagnostic printed base compatibilityKey == head compatibilityKey on a run that still logged base-hit=false, which is simply the first run at that base SHA — a legitimate cold miss. The defect here is narrower and sharper: publish and lookup disagree whenever the trees' non-spec inputs differ.

Why I have not sent a patch

The obvious repairs each imply a different answer to "what does the compatibility key certify?", and I do not want to pick for you:

  1. Publish the base bundle under the head-derived key (thread the key into the cold base capture). Makes publish/lookup symmetric and restores hit. But the stored <sha>/<key> path would then disagree with the compatibilityKey recorded inside that bundle's own manifest, which buildManifest computes from the base tree — a new inconsistency, and consumer tooling reads those manifest fields.
  2. Look the base up under a base-derived key. Honest, but the base worktree does not exist at lookup time — materialising it is the very work the restore exists to avoid. Chicken-and-egg.
  3. Narrow the key for base bundles to inputs that are genuinely equalised across the two trees (spec, platform, arch, nodeMajor, packageVersion), excluding per-tree ones like lockfileHash. Restores hit and cross-pull-request sharing works, at the cost of a weaker compatibility guarantee — and the existing "maps were captured in different runtime environments" refusal (src/map-store.ts:~613) exists precisely to catch what that would let through.

Storage being <sha>/<compatibilityKey> is what makes (1) arguably coherent — the SHA already pins the base tree, so the key would only need to pin comparability with the head — but that is a judgement about intent, not something I can read off the code.

Happy to implement whichever you prefer, with tests.

Environment

  • styleproof 4.7.4 (81628143)
  • Consumer: pull_request CI on self-hosted runners, styleproof-ci --base <base> --head <head> --spec-ref <head>

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions