Skip to content

fix(web): fail check-docs on stale install snippets (#3770)#3775

Open
Hmbown wants to merge 1 commit into
mainfrom
codex/0866-3770-check-docs-exit
Open

fix(web): fail check-docs on stale install snippets (#3770)#3775
Hmbown wants to merge 1 commit into
mainfrom
codex/0866-3770-check-docs-exit

Conversation

@Hmbown

@Hmbown Hmbown commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #3770. web/scripts/check-docs.mjs printed FAIL for stale install snippets but then fell through to [check-docs] PASS without exiting non-zero, so CI could miss public install-copy drift.

Change

The stale-snippet branch now calls process.exit(1) (mirroring checks 1 & 2), and the final PASS is only printed when every check passes.

Validation

  • cd web && node scripts/check-docs.mjs on a fresh tree → [check-docs] PASS, exit 0.
  • Stale snippets now take the exit(1) path before any PASS line.

Acceptance criteria

  • Exits status 1 when checkInstallSnippets() returns stale entries
  • Final PASS printed only if every check passed
  • Clean npm run check:docs still passes

`check-docs.mjs` detected stale `codewhale --version # X` install snippets and
printed `FAIL`, but the stale branch fell through to the final
`console.log("[check-docs] PASS")` without `process.exit(1)`. CI could miss
public install-copy drift the gate exists to catch.

Mirror the `exit(1)` used by the missing-sources and version checks: on stale
snippets, print the details and exit non-zero; the final PASS is reached only
when every check passes.

Fixes #3770
@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 6s —— View job


Code Review

  • Gather context and read changed files
  • Analyze the diff for correctness and edge cases
  • Post review findings

The fix is correct and focused. Here's the full analysis:

Logic before and after

Before:

if (!install.ok && !install.note) {
  // prints FAIL + stale details…
} else {
  console.log(`[check-docs] OK — install snippets…`);
}
console.log("[check-docs] PASS");  // ← always reached, even on failure

After:

if (!install.ok && !install.note) {
  // prints FAIL + stale details…
  process.exit(1);  // ← gate now actually closes
}
console.log(`[check-docs] OK — install snippets…`);  // only reached on success
console.log("[check-docs] PASS");

The early-exit transformation is equivalent to the original else branch but now prevents fall-through. ✅

Edge cases — all handled correctly

ok note Condition !ok && !note Behaviour
false "could not derive version" false Soft-pass (inconclusive) — logs note, no exit ✅
true "install page not found" false Soft-pass — logs note, exits 0 ✅
false undefined true Hard-fail — logs stale entries, exits 1 ✅
true undefined false Clean pass ✅

The distinction between "inconclusive" (ok: false + note) and "definitively stale" (ok: false, no note) is preserved.

Minor nit — comment references issue number

web/scripts/check-docs.mjs:146-148 — The two-line comment (// #3770: a stale install snippet…) references the issue number. Per repo guidelines, task/fix context belongs in the PR description rather than inline, since it rots as the codebase evolves. The comment could either be dropped entirely (the exit(1) mirrors checks 1 & 2 above and is self-evident) or trimmed to a one-liner without the issue reference. This is a minor style nit, not a blocker.

Verdict

LGTM. The bug fix is minimal, correct, and consistent with how checks 1 and 2 are structured. The one nit above is optional cleanup.

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: Make check-docs fail when install snippets are stale

1 participant