Skip to content

ci(web): check facts drift before regenerating tracked facts (#3771)#3776

Merged
Hmbown merged 1 commit into
mainfrom
codex/0866-3771-facts-drift-order
Jun 29, 2026
Merged

ci(web): check facts drift before regenerating tracked facts (#3771)#3776
Hmbown merged 1 commit into
mainfrom
codex/0866-3771-facts-drift-order

Conversation

@Hmbown

@Hmbown Hmbown commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #3771. web/lib/facts.generated.ts is tracked, but .github/workflows/web.yml ran npm run prebuild (regenerate) before npm run check:facts, so CI regenerated the file and then compared it against itself — a stale committed file could pass.

Change

Reordered: check:facts now runs before prebuild. The committed file exists at checkout and check:facts ignores the volatile generatedAt/latestRelease fields, so it compares the committed copy against the workspace and fails on stale. prebuild still runs afterward for tsc/build. The misleading "gitignored" comment is corrected to "tracked".

Note: a git diff --exit-code approach would false-positive because generatedAt always changes; running check:facts first is the correct fix.

Acceptance criteria

  • Web CI fails when committed facts are stale
  • check:facts runs before prebuild
  • Workflow no longer claims the tracked file is gitignored
  • cd web && npm run check:facts still passes when committed facts match

`web/lib/facts.generated.ts` is tracked, but the web workflow ran
`npm run prebuild` (regenerate) before `npm run check:facts`. That let CI
self-heal the working tree, so a stale committed facts file could pass the
drift gate. The step comment also wrongly claimed the file was gitignored.

Run `check:facts` before `prebuild`. The committed file exists at checkout, and
`check:facts` ignores the volatile `generatedAt` / `latestRelease` fields by
design, so it correctly compares the committed copy against the workspace; a
stale committed file now fails. `prebuild` still runs afterward so tsc/build
use a current file. Comment corrected to say the file is tracked.

Fixes #3771
@Hmbown Hmbown added this to the v0.8.66 milestone Jun 29, 2026
@claude

claude Bot commented Jun 29, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 1m 17s —— View job


PR Review: ci(web): check facts drift before regenerating tracked facts

  • Read context and understand request
  • Read the changed file and supporting scripts
  • Analyze the diff
  • Post review findings

Summary

This is a correct, minimal fix for a real CI bug. The old ordering (prebuildcheck:facts) caused check:facts to compare a just-regenerated file against itself, making the drift gate a no-op. The new ordering (check:factsprebuild) compares the committed file against workspace sources before overwriting it.

Analysis

Correctness — ✅

check-facts.mjs reads facts.generated.ts directly from disk via parseCommittedFacts() (line 36), then derives fresh facts via buildFacts() (line 92) and diffs them. Running prebuild first overwrites that file in the working tree, so the committed vs. fresh comparison was trivially equal. The reorder is the exact right fix.

generatedAt/latestRelease exclusion — ✅

diffFacts() (line 59–82) explicitly skips both volatile fields. The comment in the workflow is accurate: it is safe to run check:facts against the checkout copy.

npm lifecycle — ✅

"prebuild" is an npm lifecycle hook name. When the deploy job runs npm run build, npm automatically executes node scripts/derive-facts.mjs before next build. The explicit prebuild step in the lint job is needed so tsc --noEmit has the file; the deploy job gets it for free from the build lifecycle.

Comment corrections — ✅

The old comment calling the file "gitignored" was factually wrong and could confuse contributors into thinking they shouldn't commit it. The corrected "TRACKED (committed)" wording is clearer.

deploy job — ✅

No check:facts step needed there; deploy already needs: lint, so stale facts are caught upstream before deploy can run.

No issues found

The change is surgical and accurate. Nothing to flag.

@Hmbown Hmbown merged commit e73dcf9 into main Jun 29, 2026
21 checks passed
@Hmbown Hmbown deleted the codex/0866-3771-facts-drift-order branch June 29, 2026 16:55
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.

v0.8.66: Run web facts drift check before regenerating tracked facts

1 participant