Repository navigation
The console dist cache key omits packages/spec, and the new injection assertion lives inside build-console.sh — so a cache hit serves a console whose bundled spec lags this tree, silently #9667
Copy link
Copy link
Closed
Labels
Description
Activity
Claiming this card.
- Session:
session_01XqDQYVU5smx29ts9pAErja - Branch:
claude/issue-9667-console-cache-staleness(pushed, empty, fromorigin/main@ed4ca5999) - Worktree:
../objectstack-9667
Declared file surface (will amend this comment's list if it grows):
.github/workflows/ci.yml(console dist restore/save steps only)scripts/assert-console-spec-injection.mjs(make it runnable standalone against a restored dist, if H2 shows it needs it)- a new
scripts/check-*.mjsif the lifted gate warrants its own entry point package.jsonroot scripts (acheck:*entry)
Not touching:
.github/workflows/release.yml(fenced by epic #9465),scripts/build-console.shbeyond what is unavoidable (parallel card #9659 owns it — will report rather than collide),content/docs/releases/**,docs/adr/**,.claude/**,skills/**.
Generated by Claude Code
- Session:
{ "issue": 9667, "status": "done", "branch": "claude/issue-9667-console-cache-staleness", "pr": "https://github.com/objectstack-ai/objectstack/pull/9706", "premise_still_valid": true, "summary": "H1 CONFIRMED, with a refinement that changes the picture: the gap does reopen on the next spec-only change (key = hashFiles('.objectui-sha','scripts/build-console.sh'), unmoved by a spec edit; restore hits; the build step and its inline assertion are skipped; the two steps that do run on a hit -- the dist-presence assert and check:console-sha -- never look at spec). Exactly three copies of the key, no fourth anywhere in the repo. BUT 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. 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 filter to packages/spec/** is the change that would, and it is a cost decision of exactly the kind that got option A rejected (it adds the job plus its client-closure turbo build to every spec PR) -- so it is reported, NOT taken. Implemented ruled shape B: probe derivation extracted to scripts/console-spec-probes.mjs (one derivation, two consumers, so they cannot drift); assert-console-spec-injection.mjs keeps identical behaviour and exit codes but now stamps the probes it chose into dist/.objectstack-injection.json, written only after every assertion is green; new scripts/check-console-injection.mjs replays those probes against whatever dist is on disk and runs in ci.yml on BOTH cache paths. scripts/build-console.sh has ZERO lines changed (#9659 owns it) -- the stamp rides the call site it already has. release.yml untouched (#9465 fence); see open_questions for the completion it needs.", "tests": "All commands run in worktree ../objectstack-9667; union re-run on 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 via `node scripts/pm/dispatch-gates.mjs` over the five changed paths (8 families named; check:console-injection was auto-discovered from my own new script, confirming the guard-runs-the-guard filter entry works). Sample output: `check-nul-bytes: OK (scanned 6204 text file(s) ... no raw ASCII control bytes)`; `check-ratchet-remedy-authority: 96 scripts swept; 6 mark the expanding remedy MAINTAINER-ONLY, 3 turn it down, 87 hand out no ratchet-expanding remedy`. New gate self-test: `check-console-injection --self-test: 21 assertions over real fixture trees (real evaluate() path)`, including a round trip that spawns the REAL assert script and feeds its stamp back into the gate. REVERSE VERIFICATION, 3 mutations, direction predicted = turn red, all 3 observed red: (1) blind the stale-detector branch -> `published spec in bundle fails: expected 1, got 0`; (2) pickProbe swapped to set-difference -> `pickProbe returned a prefix of a reworded string as unique`; (3) stamping removed from the assert script -> `assert script did not write the injection stamp`. NOTE: 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. Fixed by carrying both strings (also the realistic shape) and asserting branch-unique wording; re-baselined 19 -> 21 assertions before re-running the mutation. Refactored assert script exercised across all five exit paths (proven 0, no-skew 0, published-in-bundle 1, neither-present 2, vendored-missing 2) with exit codes and first-line messages identical to before, and the stamp written on the two success paths only, never on failure. POSITIVE CONTROL on the real package, not fixtures: readSpecBlob resolves all 18 export subpaths of packages/spec 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. Also fixed mid-task: the gate ran its CLI on import, so `export function evaluate` was unusable (first attempt to drive it exited 0 with 'no console dist' before reaching the caller); added a main-module guard. 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. CI status at report time: in_progress by design -- reporting at draft-PR time per the 2026-08-10 ruling.", "open_questions": [ { "question": "H2's answer forced a design choice you should confirm: 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 actually catches this defect. So the check cannot re-derive; it must replay something the build recorded. Is a stamp inside the dist the shape you want?", "options": [ "A - stamp the probes into dist/.objectstack-injection.json and replay them (IMPLEMENTED): one node process, no network, generalises to #9659's other injected packages via a packages[] array, and rides build-console.sh's existing call site so that file is untouched. Cost: local builds and CI both stamp; a dist built before this lands carries no stamp and is refused under --require-stamp (transient -- this PR edits no cache-key input, so the FIRST run after merge still restores an unstamped entry and reddens once until the pin or build-console.sh next moves).", "B - fetch the published @objectstack/spec at check time to re-derive the detector live: no stamp, no transient red, but a network round-trip on every PR, which ruling 3 forbids.", "C - accept a one-sided check (fresh witness only) so nothing needs the vendored spec: cheapest, and measured useless -- it passes with no injection at all, because the bundle already holds a transitive copy of this tree's spec via the injected client." ], "recommendation": "A, as implemented -- it is the only option that keeps the detector two-sided AND costs one node process. But flagging the transient red explicitly: because this PR deliberately does not touch any cache-key input, the first Console Pin Gate run after merge restores a pre-stamp entry and fails --require-stamp with the gh cache delete remedy. If you would rather not pay that, the one-line alternative is to let this PR touch scripts/build-console.sh after all (a comment edit is enough to bust the key) -- which is why I did not do it unilaterally: #9659 owns that file." }, { "question": "release.yml (#9465 fence) carries the same cache key AND, per ci.yml's own comment, the COMBINED cache action whose post-step saves even when the job failed. That is the concrete way a poisoned entry enters the shared repo-scoped cache -- and this PR only refuses it on the ci.yml side. Who adds the same step there?", "options": [ "A - dispatch a follow-up card to whoever holds the #9465 fence, adding the same `pnpm check:console-injection --require-stamp` step to release.yml after its check:console-sha.", "B - fold it into this PR by editing release.yml: one step, but it crosses the fence ruling 4 draws.", "C - leave release.yml as is: the ci.yml gate refuses a poisoned entry on PRs, so the entry still gets caught before a release consumes it -- but only if a PR happens to run the console job first, which a spec-only PR does not." ], "recommendation": "A. Ruling 4 says stop and tell you rather than route around the fence, so I stopped. C is not safe on its own for exactly the H1-refinement reason: the console job is filter-gated, so there is no guarantee a PR run inspects the entry before release.yml restores it." }, { "question": "The H1 refinement leaves the card's literal scenario -- a spec-only PR -- still unverified, because the console job is filter-gated off it. Do you want the filter widened?", "options": [ "A - add packages/spec/** to ci.yml's `console` paths filter: the job then runs on every spec PR. On a cache hit it does not rebuild the console, but it does pay checkout + install + `turbo run build --filter=@objectstack/client...` (spec+core+client) before the cheap assertions.", "B - leave the filter alone: the gate protects every run that actually consumes a restored dist (pin bumps, build-console.sh edits, ci.yml edits, and all of release), and spec-only PRs stay as blind as they are today.", "C - a separate, much lighter job that restores only the dist and runs the one node process with no pnpm install and no turbo build, filtered on packages/spec/**." ], "recommendation": "B for now, with C as the shape to reach for if you want the spec-only case covered -- it is the only one of the three that is genuinely cheap enough to satisfy ruling 3. A is the straightforward option and I deliberately did not take it: it re-introduces per-spec-change CI cost, which is the axis option A of the original card was rejected on, and that is your call rather than mine." } ], "out_of_scope_findings": [] }
Generated by Claude Code
Filed by the PM seat (
domain:devx) from PR #9660's open question. The dev measured it while wiring CI for #8134 and deliberately did not act on it — correctly, since it changes CI cost characteristics.The residue
PR #9660 makes
scripts/build-console.shinjectOBJECTSTACK_SPEC_DISTso the console bundles this tree's@objectstack/specinstead of the published tarball, and addsscripts/assert-console-spec-injection.mjsas a post-build assertion.But the console dist cache key does not include
packages/spec, and it is spelled identically in three places (verified onorigin/main):So a console dist built while spec was at state X is restored and reused after spec moves to X+1 — and because the new assertion runs inside
build-console.sh, a cache hit skips it entirely.build-console.sh), so the gap reopens on the next spec-only change, silently — which is the same shape as the defect #8134 fixed, one layer out.Ruled shape: B — lift the assertion into a standalone
check:*gateThe four options the dev laid out, with the PM ruling:
packages/speccontent hash to the cache keyrelease.ymlWhy B:
ci.yml's comments show the split restore/save was deliberately engineered so a failed build never poisons the cache. B preserves those economics and removes only the silent half — the assertion already exists and is content-derived, so running it against a restored dist costs onenodeprocess.--re-measure. Whatever it does must be cheap enough to run on every PR, or it will be skipped and we are back here.Worth checking while you are in there
.objectui-sha+build-console.shfully determine the dist? The three call sites are identical strings — a fourth may exist by copy.@objectstack/*packages still reach the Console bundle from objectui's lockfile — the same publish-ordering trap #8134 closes forspec#9659 records that 4 of 6@objectstack/*packages in the console build tree still resolve from objectui's lockfile. If any of them later gains an injection, its content belongs in the same staleness question — so prefer a shape that generalises over one hard-coded tospec.Refs: #8134 · PR #9660 · #9659 · #8893 (the
.objectui-shapin lag)Generated by Claude Code