Skip to content

fix(ci): verify the RESTORED console dist bundles this tree's spec, not only the built one - #9706

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9667-console-cache-staleness
Aug 19, 2026
Merged

fix(ci): verify the RESTORED console dist bundles this tree's spec, not only the built one#9706
os-steve merged 1 commit into
mainfrom
claude/issue-9667-console-cache-staleness

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9667

Ruled shape B: keep the cache key, and run the injection assertion against the restored dist rather than only inside the build.

H1 — the gap does reopen, and there is a second lock the card did not know about

Confirmed on origin/main. The key is spelled in exactly three places, no fourth copy anywhere in the repo (swept all files, not just workflows): ci.yml:1439 (restore), ci.yml:1485 (save), release.yml:491.

A spec-only change moves neither .objectui-sha nor scripts/build-console.sh, so the key is unchanged, the restore hits, and if: steps.console-dist.outputs.cache-hit != 'true' skips the build step — and with it the assertion, which lives inside build-console.sh. Nothing on the restore path re-validates: the two steps that do run on a hit are the dist-presence assert and check:console-sha, and neither looks at spec. PR #9660 busts the key exactly once, so the next spec-only change reopens it.

But the cache key is the second lock, not the first.ci.yml's paths filter defines the console filter as .objectui-sha, scripts/build-console.sh, scripts/check-console-sha.mjs, .github/workflows/ci.yml. packages/spec/** is not in it, and the job is if: needs.filter.outputs.console != 'false'. So on a spec-only PR the Console Pin Gate job does not run at all — the cache never even gets consulted. Lifting the assertion makes every run that consumes a restored dist verify it; it does not by itself put a spec-only PR in front of the gate. Widening the console filter to packages/spec/** is the change that would, and it is a cost decision (it adds the job, including its client-closure turbo build, to every spec PR) of exactly the kind that got option A rejected — so it is reported, not taken.

H2 — the assertion does not lift cleanly, and this is what shaped the design

assert-console-spec-injection.mjs takes --injected, --vendored, --assets. Two of those survive a cache hit; the third does not:

  • --injected — this tree's packages/spec. Present (the job already builds the client closure, which contains spec).
  • --assets — the restored packages/console/dist/assets. Present.
  • --vendored${BUILD_ROOT}/node_modules/@objectstack/spec, where BUILD_ROOT is the objectui shallow clone under .cache/objectui-*. On a cache hit that tree does not exist, because the step that creates it is the one being skipped.

That is the side that matters. The stale detector — text only the published spec carries, which must be absent — is the probe that actually catches this defect; the fresh witness alone cannot tell "injection worked" from "some other copy" (the bundle holds a second, transitive copy of this tree's spec via the injected client). So re-running the script on the restore path is not merely awkward, it is impossible: it would exit 2 for want of a package.json. Fetching the published spec instead would put a network round-trip on every PR, which ruling 3 forbids.

Design: the build stamps the probes it derived into packages/console/dist/.objectstack-injection.json, and a new gate replays them against whatever dist is on disk. Cost is one node process over files already present — no network, no build.

scripts/build-console.sh is not touched (#9659 owns it). The stamp is written by assert-console-spec-injection.mjs, which build-console.sh already calls, so the existing call site picks it up unchanged and local pnpm objectui:build stamps too.

Fail vs. invalidate-and-rebuild (ruling 2)

It fails. Reasoning, because the other option looks more attractive than it is:

Actions cache keys are immutable. A gate that responded to a bad restore by rebuilding could not evict the offending entry — the next run would restore the same bad entry and rebuild again, a ~20 min rebuild on every run until the pin moved. That is strictly worse than the one-off cost that got option A rejected. "Invalidate and rebuild" is not actually available here; only "rebuild forever" is.

So it fails once, and every failure names its remedy, including the exact eviction command for the key that produced it — the workflow passes the resolved key in CONSOLE_DIST_CACHE_KEY, so the message reads gh cache delete "Linux-console-dist-HASH" with the real hash, not a template. That path is marked ⛔ MAINTAINER-ONLY (it needs write scope and the entry is shared with release.yml), with pnpm objectui:build as the local remedy. The message also states plainly that the restored artifact is a cache entry and nothing in the PR's diff can fix it, so a contributor who did nothing wrong is not left hunting their own changes.

Which reds are even reachable matters here, and none of them fire on an innocent spec change:

ConditionResponse
Bundle carries the published specfail — the real defect, never acceptable
Stamp's own witness missing from the assetsfail — partial restore or modified dist
Stamped probe no longer discriminatesfail — unverifiable, see below
Dist carries no stampfail under --require-stamp (CI), advisory locally
Build recorded no observable skewpass, and says so
Spec moved forward since the dist was builtnot a failure — the ruled cache design accepts lag

That last row is the point: the probes are stamped beside the dist and describe that build, so they are assertions about the artifact, not about the tree. A spec change does not move them.

H3 — the probe cannot go silently useless

The stamped stale detector is only evidence while it still tells the two specs apart. If the published spec later catches up, a frozen probe would pass forever while proving nothing — the exact failure #8134 exists to end. So the gate re-checks the stamped detector against this tree's spec on every run and reports expiry instead of passing. Probe derivation is substring-based, not set-difference, because rewording a description by appending a clause makes the old text a prefix of the new one; that property is asserted directly in the self-test.

Both consumers import one derivation module (scripts/console-spec-probes.mjs) rather than keeping a copy each — the second script replays strings the first one chose, so two implementations that drifted would silently stop agreeing on what a probe is.

H4 — sweep of every actions/cache in the repo

Four distinct cached paths exist across all 25 workflows:

PathSitesKey omits a real input?Same shape?
${{ env.STORE_PATH }} (pnpm store)16nono — content-addressed, integrity-verified from the lockfile
.turbo/cache14n/ano — turbo hashes its own declared inputs; a mismatched entry simply misses
apps/docs/.next/cache1yesno — see below
packages/console/dist3yesyes — the only one

The docs cache is worth recording: its key is lockfile plus apps/docs/**.[jt]s(x) plus content/**/*.mdx, and apps/docs depends on @objectstack/spec at workspace:* — so the key genuinely omits an input the build consumes. It is still not this defect, because the cached path is Next's incremental compiler cache (internally content-hashed, and self-invalidating), not a build product, and no later step asserts anything about it.

That distinction is the generalisable finding, and it is sharper than "the key omits an input" — turbo and Next keys omit inputs too and are fine. The property that makes the console dist different is: a cache hit skips the step that produces the proof, and nothing on the restore path re-establishes it. By that definition the console dist is the only instance, so this is a one-off gate rather than a shared convention. Nothing else was changed.

Not touched, deliberately

Verification

Union re-run on the final commit 3497032ac, all green: check:nul-bytes, check:required-contexts, check:workflow-status-functions, check:node-version, check:shard-attestation, check:cross-package-test-inputs, check:ratchet-remedy-authority, check:console-injection. Gate list derived with node scripts/pm/dispatch-gates.mjs over the five changed paths.

The new gate ships a --self-test (repo convention), 21 assertions over real fixture trees driving the real evaluate() path, including a round trip that runs the actual assert script and feeds its stamp back in — the drift the shared module exists to prevent.

Reverse verification, three mutations, each reddening as predicted:

  1. Blind the stale-detector check ⇒ expected 1, got 0. First attempt at this passed, which exposed a genuine vacuity in the fixture: the bundle held only the published string, so the missing-fresh-witness branch returned 1 for the wrong reason and both branches print the same remedy block. Fixed by making the fixture carry both strings (also the realistic shape) and keying assertions on branch-unique wording.
  2. pickProbe swapped to set-difference ⇒ caught, naming the reworded prefix it wrongly called unique.
  3. Stamping removed from the assert script ⇒ round trip caught it.

The refactored assert script was exercised across all five of its exit paths (proven, no-skew, published-in-bundle, neither-present, vendored-missing) with identical exit codes and first-line messages to before, and the stamp is written on the two success paths only, never on a failure.

Positive control against the real package rather than fixtures only: readSpecBlob resolves all 18 export subpaths of packages/spec into a 12.5 MB blob yielding 2995 usable probe candidates, and evaluate() returns the right verdict for a healthy dist, one carrying the published spec, and an expired probe — confirming the scheme is not silently inert on the real thing.

Not run locally: a full build-console.sh (clones objectui, ~20 min). The build-side path is covered by the round-trip case and the exit-path matrix.

Notes

  • No changeset: workflow, root scripts, and a private root package.json script entry. Nothing publishes. skip-changeset applied.
  • pnpm check:console-injection --require-stamp works because pnpm appends forwarded args to the tail command of the && chain, so the self-test still runs first. Verified from the echoed command line.

Generated by Claude Code

…ot only the built one
The console dist cache key is hashFiles('.objectui-sha', 'scripts/build-console.sh')
— spelled identically in ci.yml (restore + save) and release.yml. packages/spec is
not in it, and scripts/assert-console-spec-injection.mjs runs INSIDE
build-console.sh, whose step is skipped on a cache hit. So the run that did not
build the dist is exactly the run that never asks whether the dist is right.
Adding packages/spec to the key was rejected on cost (~20 min cold rebuild per
spec change, ~18 merges/day). This keeps the key and the deliberate split
restore/save untouched and removes only the silence:
- scripts/console-spec-probes.mjs — probe derivation, extracted so the build-time
assertion and the restore-time gate cannot drift apart.
- assert-console-spec-injection.mjs — same behaviour and exit codes; now stamps
the probes it chose into dist/.objectstack-injection.json, written only after
every assertion is green.
- scripts/check-console-injection.mjs — replays the stamped probes against
whatever dist is on disk, cache hit or miss, and re-checks that the stamped
probe still discriminates instead of trusting it forever.
Fails rather than rebuilds: Actions cache keys are immutable, so a gate that
responded by rebuilding could not evict the entry and would rebuild on every run
until the pin moved. Every failure names its remedy, including the exact
`gh cache delete` line for the key that produced it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@github-actionsgithub-actionsBot added size/l ci/cd dependencies Pull requests that update a dependency file labels Aug 18, 2026
@os-steveos-steve added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026 — with Claude
@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9667 / PR #9706 · three rulings below

Verified independently: 5 files +764/-100, zero governed-surface hits, no non-green gates (four still running). scripts/build-console.shuntouched#9659's ownership respected, and the stamp rides the call site that already existed.


⭐ The H1 refinement is a better finding than the card

the cache key is the SECOND lock, not the first: ci.yml's paths filter defines console as .objectui-sha + build-console.sh + check-console-sha.mjs + ci.yml, and the job is if: needs.filter.outputs.console != 'false' — so a spec-only PR never runs Console Pin Gate at all and never consults the cache.

I verified the filter myself (ci.yml:102-106) and it is exactly those four paths. So the card — and my ruling on it — were reasoning about the wrong lock. The cache key genuinely omits packages/spec, and the gap genuinely reopens on the next spec-only change (three copies of the key, no fourth anywhere, confirmed) — but the scenario the card names is blocked one layer earlier by the filter.

Lifting the assertion makes every run that consumes a restored dist verify it, which is real and is what B was for. It does not put a spec-only PR in front of the gate. Reporting that distinction instead of quietly shipping a fix that does not cover the card's headline scenario is the whole value of this report.

Ruling on Q1: A, as implemented. Take the transient red.

H2's answer is decisive and I accept it: the assertion does not lift cleanly--vendored points at ${BUILD_ROOT}/node_modules/@objectstack/spec under .cache/objectui-*, which does not exist on a cache hit, and that is the stale-detector side, the only side that catches this defect. So the check cannot re-derive; it must replay what the build recorded. A stamp inside the dist is the right shape, and the packages[] array generalising to #9659's other injected packages is the right forward-compatibility.

B is forbidden by ruling 3 (network round-trip per PR). C is measured useless — a fresh-witness-only check "passes with no injection at all, because the bundle already holds a transitive copy of this tree's spec via the injected client." That is a one-sided canary, and #9660 already established that a one-sided canary is exactly what let this defect survive.

On the transient red — pay it. Two facts make it cheap, and you should know both:

  1. Console Pin Gate is NOT in the required-context set. I read the live ruleset today: the six are TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Lint & Repo Gates. So the transient cannot block a merge — it is advisory.
  2. It is self-clearing: one run, with the gh cache delete remedy named in the failure. And Four more @objectstack/* packages still reach the Console bundle from objectui's lockfile — the same publish-ordering trap #8134 closes for spec #9659 is in flight on build-console.sh right now — if it lands any edit there, the key busts naturally and the transient never happens at all.

⛔ I am specifically not asking you to touch build-console.sh to bust the key. You declined to do that unilaterally because #9659 owns the file, and that was right. Do not contrive a comment edit to dodge one advisory red.

Ruling on Q2 (release.yml): A — follow-up card, and the fence is unambiguous here.

release.yml is named explicitly in epic #9465's declared territory, not inside the scoping parenthetical (which covers only root package.json's @changesets/cli range and version script). So it is fenced regardless of how that parenthetical question resolves. You stopped and told me, which is exactly what ruling 4 asked for.

And your reason for rejecting C is the one that decides it: "the console job is filter-gated, so there is no guarantee a PR run inspects the entry before release.yml restores it" — combined with the ci.yml comment noting release's combined cache action whose post-step saves even when the job failed. That is the concrete path a poisoned entry enters the shared repo-scoped cache. Filing the follow-up.

Ruling on Q3 (filter widening): B now, C carded.

A is rejected for the same reason the original option A was: it re-introduces per-spec-change CI cost — checkout + install + turbo run build --filter=@objectstack/client... on every spec PR. That is the axis this card was decided on and I am not reversing it one question later.

C is the right shape — a separate light job restoring only the dist and running one node process, no pnpm install, no turbo build, filtered on packages/spec/**. It is the only one of the three that satisfies ruling 3. Filing it as its own card rather than growing this PR.


⭐ Your first mutation passed, and that exposed a real defect in your own error design

mutation (1) PASSED on first attempt and thereby exposed a genuine vacuity in my own fixture — the bundle held only the published string, so the missing-fresh-witness branch returned 1 for the wrong reason, and keying on the remedy text could not tell the branches apart because every failure prints the same remedy block.

Two distinct findings in one sentence, and you fixed both: the fixture now carries both strings (also the realistic shape), and the assertions key on branch-unique wording rather than the shared remedy block. Re-baselined 19 → 21 and re-ran the mutation.

The second half is the one worth keeping: a failure vocabulary where every branch prints the same remedy is untestable by construction — no assertion can distinguish which limb fired. That is a design constraint on error messages, discovered by an ablation that refused to stay green.

You are the third dev in this lane today to catch a test passing for the wrong reason and report it as an invalid run rather than banking it.

The rest of the verification

Positive control on the real package, not fixtures: readSpecBlob resolves all 18 export subpaths into a 12.5 MB blob yielding 2995 usable describe() probe candidates, and evaluate() returned the correct verdict for a healthy dist (0), one carrying the published spec (1), and an expired probe (1) — so the scheme is not silently inert on the real thing. Fixtures prove the code handles the case; the real package proves the case exists.

All five exit paths of the refactored assert script exercised with exit codes and first-line messages identical to before, and the stamp written on the two success paths only, never on failure. A -100/+57 refactor of a script that gates a 20-minute build needs exactly that kind of equivalence evidence.

And the entry-point guard, again: the gate ran its CLI on import, so export function evaluate was unusable — the first attempt to drive it exited 0 with "no console dist" before reaching the caller. That is the fourth instance of the #9554 class in this lane today (sync-template-versions.mjs, check-docs-redirects.mjs, check-published-readme-exports.mjs, now this). It is becoming a repo-wide pattern rather than a series of coincidences.

Also good: the gate list was derived via dispatch-gates.mjs, and check:console-injection was auto-discovered from your own new script — confirming the guard-runs-the-guard filter entry works.

Verdict: ACCEPT. Arming once the four running gates converge.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cddependenciesPull requests that update a dependency filesize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-steve@claude