Skip to content

fix(guard): apply normalizeEol to packaged leaf boundary check (#6192) - #6220

Open
xxiaoxiong wants to merge 1 commit into
nexu-io:mainfrom
xxiaoxiong:fix/6192-crlf-guard-packaged-leaf
Open

fix(guard): apply normalizeEol to packaged leaf boundary check (#6192)#6220
xxiaoxiong wants to merge 1 commit into
nexu-io:mainfrom
xxiaoxiong:fix/6192-crlf-guard-packaged-leaf

Conversation

@xxiaoxiong

Copy link
Copy Markdown
Contributor

摘要

scripts/check-packaged-leaf-boundary.tsString.prototype.includes 字节级比较 requiredWorkspaceUnitBlock 模板字面量和 .github/workflows/ci.yml。在 Windows (core.autocrlf=true) 下工作树读 ci.yml 带 CRLF,行尾是 \r\n,模板字面量用 LF (\n),includes 永远 false,guard 误报 "ci.yml no longer contains the guarded … block"。

scripts/check-design-system-manifests.ts (#5176) 已经引入了 normalizeEol 但只是 inline 字符串替换。这个 PR 把 helper 提取到 scripts/lib/eol.ts,packaged leaf boundary check 也调它。

改动

  • scripts/lib/eol.ts (new, 16 行) — normalizeEol(input: string): string,CRLF → LF。
  • scripts/check-packaged-leaf-boundary.ts — import normalizeEol.includes() 两边都过一遍。
  • scripts/check-design-system-manifests.ts — 用 ./lib/eol.ts 的 helper,删掉 inline 替换。
  • e2e/tests/scripts/eol-crlf-regression.test.ts (new, 63 行, 7 tests) — pin LF/CRLF 行为;CRLF haystack 原始 includes 失败,normalizeEol 后过。

验证

  • pnpm guard: EXIT 0
  • pnpm --filter @open-design/e2e exec vitest run tests/scripts/eol-crlf-regression.test.ts: 7/7 pass
  • 基于 upstream/main (30501a2),不引入 fork 独有改动。

不变量

  • CI: 跟着 guard 的现有 PR check 走
  • Scope: 只动脚本 + e2e test,不碰业务代码

Closes

…io#6192)

The multi-line `requiredWorkspaceUnitBlock` template literal in
`scripts/check-packaged-leaf-boundary.ts` is compared byte-for-byte
against `.github/workflows/ci.yml` via `String.prototype.includes`.
On Windows (`core.autocrlf=true`) the working tree reads ci.yml with
CRLF endings while the template literal uses LF, so `includes()` always
returns false and the guard reports a phantom "ci.yml no longer contains
the guarded … block" violation on pristine upstream/main.

Apply `normalizeEol` (the same helper nexu-io#5176 introduced for design-system
manifests) to both sides of the comparison. Add `scripts/lib/eol.ts` as
the canonical home for the helper so future guards stop re-implementing
it inline.

Regression coverage in `eol-crlf-regression.test.ts` pins the LF/CRLF
behaviour and asserts a multi-line anchor that fails raw `includes()`
against a CRLF haystack but matches after `normalizeEol` on both sides.

EXIT 0: pnpm guard; 7/7 vitest pass.
@xxiaoxiong
xxiaoxiong requested a review from a team as a code owner July 29, 2026 08:19
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @xxiaoxiong — scoping this to the guard-side EOL normalization plus a focused regression test is a nice, tight follow-up to #6192.

I'll get the right reviewer and labels on it now.

@lefarcen lefarcen added size/S PR changes 20-100 lines risk/medium Medium risk: regular code changes labels Jul 29, 2026
@lefarcen
lefarcen requested a review from nettee July 29, 2026 08:22
@lefarcen lefarcen added type/bugfix Bug fix skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. labels Jul 29, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks for keeping the fix scoped to the guard path and for spelling out the Windows CRLF repro clearly.

Could you reshape the description so pool review can scan it faster with the current template — specifically add Why, What users will see, Surface area, and Validation as explicit sections.

Related: #6192 (issue context), #6195 by @jfpaezl (similar Windows CRLF guard fix touching the same boundary).

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xxiaoxiong I verified the CRLF guard fix end to end in the prepared worktree. The normalizeEol extraction keeps the existing design-system manifest behavior intact, applies the same normalization to the packaged leaf boundary check, and the focused regression test covers both the raw CRLF failure mode and the normalized success path.

I also reran pnpm guard and pnpm --filter @open-design/e2e exec vitest run tests/scripts/eol-crlf-regression.test.ts locally on this head, and both passed. Nice tight follow-up on the Windows false-positive case.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @jfpaezl — that's useful context.

Understood that #6220 addresses the packaged-leaf includes() false positive, while #6195 is carrying the second CRLF-sensitive path in scripts/check-certain-exempt-consumption.ts. We won't treat this PR by itself as the full closure of #6192 while that second check is still outstanding.

And agreed on the helper shape — rebasing the remaining call site onto scripts/lib/eol.ts sounds like the cleanest next step.

@xxiaoxiong

Copy link
Copy Markdown
Contributor Author

Heads-up for maintainers: this PR's ci workflow run (https://github.com/nexu-io/open-design/actions/runs/30435045516) has been queued for ~23 hours with no runner picking up the Static gate / Detect validation scopes jobs. The fork-pr-approval gate already passed (approve+label jobs done at 08:19-08:20 UTC). The branch is mergeable and the change is small (one commit, scoped to scripts/check-packaged-leaf-boundary.ts + tests). If the runner queue is genuinely stuck, re-triggering or kicking the runners would unblock merge.

@lefarcen

Copy link
Copy Markdown
Contributor

Thanks for flagging this.

With reviewer approval already in place, this looks like runner availability / workflow queueing rather than anything still blocked on your change itself. We'll treat the stuck Static gate / Detect validation scopes jobs as CI infra state, not author-side follow-up, and keep the PR open while a maintainer re-runs or unsticks that workflow.

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

Labels

risk/medium Medium risk: regular code changes size/S PR changes 20-100 lines skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants