Skip to content

fix(diagrams): keep diagram sources LF so the receipt check passes on Windows - #167

Merged
CodeWithJuber merged 1 commit into
masterfrom
claude/forgekit-deep-review-issues-ww6y0h
Sep 26, 2026
Merged

CodeWithJuber merged 1 commit into
masterfrom
claude/forgekit-deep-review-issues-ww6y0h

Conversation

@CodeWithJuber

Copy link
Copy Markdown
Owner

What & why

Test (Windows / Git Bash) failed on #166's head after the merge, because the job was still running when #166 merged. So master is red on Windows.

The failing test is the repository's diagrams match their receipts and are embedded where the manifest says. On the Windows runner, core.autocrlf checks docs/diagrams/src/*.json out with CRLF line endings. Each receipt in docs/diagrams/diagrams.json records the sha256 of the source's exact bytes, so all thirteen diagrams read as "changed since its last verified render". A Windows contributor running node scripts/diagrams.mjs check would see the same failure.

Fix:

  • .gitattributes pins docs/diagrams/** and mintlify/images/diagrams/*.svg to LF on every platform. Content-addressed files have to be byte-stable, the same way the repo already pins *.sh.
  • A new test asks git which eol applies to a source, the manifest, an SVG and a docs-site copy.
  • The CHANGELOG has a ### Fixed entry under [Unreleased], and the docs-site changelog page is re-rendered.

Reproduced and verified with a core.autocrlf=true clone:

before (061f361) after (b1263e9)
git check-attr eol on a source unspecified lf
carriage returns in core-loop.workflow.json 43 0 (README still gets 649, so autocrlf is active)
node scripts/diagrams.mjs check 13 "changed" ok: 13 diagrams match their receipts
test/diagrams.test.js fails 8/8 pass

Checklist

  • npm test passes: 1,701 tests, 1,697 pass, 0 fail, 4 platform-gated skips (Node 22)
  • npm run check passes (Biome lint + format)
  • New public functions have a test (n/a: config-only fix, with a test that locks it in)
  • Conventional commit message (feat:/fix:/docs: …)
  • CHANGELOG.md updated under ## [Unreleased]
  • No new runtime dependency (dev deps ok)
  • Substrate/docs updated if this changes forge substrate, forge impact, router/gate, or MCP substrate tools (n/a)

Risk & rollback

  • Risk level: very low. The change is a .gitattributes rule for files that are already LF in the repository, plus one test. Nothing changes on Linux or macOS checkouts.
  • An existing Windows clone that already checked these files out with CRLF picks up LF after git add --renormalize . or a fresh checkout.
  • Rollback plan: revert the commit.

Extra checks (tick if applicable)

  • npm run typecheck passes
  • Input validated at boundaries; errors handled (no swallowing): n/a
  • Authorization/ownership checked (if it touches access): n/a
  • Logs contain no secrets/PII
  • If AI-assisted: I understand it, verified the package APIs, and it has tests

🤖 Generated with Claude Code

https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2


Generated by Claude Code

… Windows

The Windows CI job failed "the repository's diagrams match their receipts"
after #166 merged: with core.autocrlf, Git checked docs/diagrams/src/*.json out
with CRLF line endings, so each source's sha256 differed from the receipt in
docs/diagrams/diagrams.json and all thirteen diagrams read as changed.

The receipts hash exact bytes, so the sources must be byte-stable across
platforms: .gitattributes now pins docs/diagrams/** and the docs-site SVG
copies to LF. A new test asks git which eol applies to those paths.
Reproduced with a core.autocrlf=true clone (check fails before, passes after).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GVVG2VDETWsDxMu6MBWPz2
@CodeWithJuber
CodeWithJuber marked this pull request as ready for review September 26, 2026 23:10
@CodeWithJuber
CodeWithJuber merged commit 21cae80 into master Sep 26, 2026
13 checks passed
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.

2 participants