Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): 去掉让 useDatasetFields 对 grep 变 binary 的裸控制字节,并给仓库加扫描门禁 - #3388
Merged
Conversation
…binary to grep, and gate the repo (objectstack#5425) `useDatasetFieldCatalog` joined and split its React dependency key on a raw U+0000 byte, chosen as "a character no path can contain". One such byte makes grep and ripgrep classify the WHOLE file as binary: they print `binary file matches` and no line at all, so the file was invisible to every content search and every grep-based lint, with nothing to say so. The key is now `JSON.stringify(include)` / `JSON.parse(includeKey)`. That needs no impossible character at all, so the round-trip cannot be broken by any path value, and the source is plain ASCII. Second half, which is the part that stops a recurrence: `scripts/check-control-bytes.mjs` scans every tracked TEXT file (carrier, not extension - objectstack#4890) and rejects the C0 range apart from tab, line feed and carriage return, plus U+007F. Wired as `pnpm check:control-bytes` and as its own workflow with no path filter, because `ci.yml` and `lint.yml` both paths-ignore markdown - the carrier of the worst instance of this bug. Measured rather than assumed: on GNU grep 3.11 and ripgrep 14, U+0000 is the only byte that triggers binary classification; U+0001 and the rest print the matching line normally. Both classes fail the gate, for the two different reasons the report names. Four pre-existing offenders in other packages are baselined in KNOWN_OFFENDERS against objectstack#5450 rather than fixed here (out of this issue's scope). The baseline is a ratchet, not a skip-list: the scan fails on a stale entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt
The separator swap in the previous commit is only safe if the encode/decode pair still yields the same walked paths, so pin the behaviour it is responsible for: single-hop and multi-hop include paths, several paths at once, the empty list (the old ternary's job), and re-running on content change but not on a new array identity. One case is aimed at the fix itself: a path segment containing a comma still round-trips. The obvious repair for a U+0000 separator is "pick a printable character no name contains", which merely relocates the collision; JSON has no such assumption and the test says so. The client mock is deliberately identity-stable. The real useMetadataClient memoizes, and the effect lists `client` in its deps, so a per-render object loops forever and OOMs the worker instead of failing an assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Matches scripts/check-changeset-no-major.mjs: resolve(process.argv[1]) against fileURLToPath(import.meta.url), which survives relative and symlinked paths. Flag handling now sits inside that guard, so importing the module for tests can never trigger a repo scan or a process.exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 5, 2026 12:58
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixesobjectstack-ai/objectstack#5425
U+XXXX形式描述控制字符,不贴字节本体(issue Notes 的写作纪律;这一族五次前科里有两次就发生在写规则本身的时候)。问题
useDatasetFieldCatalog把include: string[]压成一个标量放进React.useEffect依赖数组,再在 effect 内还原。这对编解码原先是join/split在一个裸 U+0000 字节上,理由是"字段路径里不可能出现的字符"。一个裸 U+0000 会让 grep 和 ripgrep 把整个文件判为 binary —— 它们只打印
binary file matches,一行内容都不给:也就是说这个文件对所有按内容搜索的人和 agent 都是隐形的,对每一个基于 grep 的 lint 也一样,而且没有任何报错提示。git 同样不会告警:它只看前 8000 字节判断 binary,所以偏移量更靠后的控制字节会一直以普通文本形式参与 diff。
改动一:源文件
依赖键改为
JSON.stringify(include)/JSON.parse(includeKey)。选它而不是把分隔符换成转义拼写,是因为它根本不需要"数据里不可能出现的字符"这个前提。把 U+0000 换成一个可打印字符(逗号、竖线)只是把碰撞挪了个位置;JSON 没有这个假设,任何路径值都能无损往返。源文件也因此变成纯 ASCII。
行为不变:这个值只做 effect 依赖比较,从不落库、不上网、不进快照。全仓
grep -a确认它只有三处引用,都在本文件内。改动二:仓级门禁(issue 的第 2 点)
新增
scripts/check-control-bytes.mjs,接成pnpm check:control-bytes与独立 workflowcontrol-bytes.yml。按载体划范围,而不是按后缀(objectstack#4890):扫描每一个 tracked 的文本文件。危害是 grep 的性质,不是 JavaScript 的性质 —— markdown、YAML、
.claude/下的 agent 指令一样中招。文本/二进制只按内容判定:先剥掉被查的控制字节再做 UTF-8 严格解码,所以控制字节永远不能成为"不检查它自己"的理由(这正是 git 掉进去的循环)。实测扫 3573 个文本文件、跳过 85 个真二进制资源,零误判。覆盖面超出 U+0000,这是 issue 明确要求的。但这里有一处我把 issue 的前提测了一遍,结论需要纠正:
每种字节单独写进 fixture 实测的结果是:只有 U+0000 会触发 binary 判定,U+0001 / U+0002 / U+0007 / U+0008 / U+000B / U+000C / U+000E / U+001A / U+001B / U+001F / U+007F 都不会。所以 objectstack#5157 的教训仍然成立(NUL-only 扫描器确实会漏),但漏掉的那颗的危害是"隐形、不可 review",不是"grep 变瞎"。两类都拦,报告分开说,不把一个夸大的理由套在两类上。
为什么单独一个 workflow:
ci.yml和lint.yml都把'**/*.md'、content/**、docs/**、.changeset/**放进了paths-ignore,而 GitHub 没有 job 级路径过滤。markdown 恰恰是这个 bug 最严重那次的载体(objectstack#4890:写"禁止裸 NUL"这条规则的那个 PR 自己往.claude/skill 文件里写了一颗)。带路径过滤的门禁看不见那种 PR。changeset-guard.yml在本仓就是同样理由,它的头注释也这么写。测试会在有人给这个 workflow 加paths或paths-ignore时变红。顺带发现:另有四个文件(未在本 PR 修)
扫描一上线就报出四个既有问题文件,已按 Prime Directive #10 单独立 issue objectstack#5450,不在本 PR 修(超出本 issue 的文件面):
packages/core/src/evaluator/listConditional.ts— U+0000packages/core/src/utils/record-title.ts— U+0000packages/fields/src/widgets/PeoplePicker.tsx— U+0000packages/plugin-dashboard/src/DatasetWidget.tsx— U+0001前三个今天就是 grep 隐形的。它们通过
KNOWN_OFFENDERS基线放行,让门禁能以 ratchet 方式先上线。这是基线,不是 skip-list:条目声明了它覆盖哪些字节值,文件多出别的控制字节照样红;文件被修干净或删除而条目没删,扫描同样红。所以修 objectstack#5450 时必须在同一个 PR 里删掉对应条目,两边不可能漂移。形制照本仓
check-lint-coverage.mjs/check-type-check-coverage.mjs的DEBT惯例(带 issue 号、注明是临时债)。反向验证(方向在跑之前先写死)
四种情况,预测全是"变红",但红的原因各不相同 —— 全部命中:
exit=0exit=1,报在INVISIBLE TO CODE SEARCH下exit=1;同时确认该文件不含 U+0000,即 NUL-only 扫描器确实看不见exit=1;grep -n includeKey打印 0 行,回到binary file matchesexit=1,报stale KNOWN_OFFENDERS第三行是这次真正想钉住的那条:证明的不是"字节没了",而是"grep 又能看见这个文件了" —— 这才是 issue 报的那个危害。测试里也是这么断言的。
测试
scripts/__tests__/check-control-bytes.test.ts(28 例):分类器、覆盖字节集、载体范围、行列定位跨过 8000 字节窗口、基线 ratchet 的四个方向、仓库现状、以及接线本身。packages/app-shell/src/views/metadata-admin/inspectors/useDatasetFields.catalog.test.tsx(7 例):往返行为 —— 单跳、多跳(ADR-0071)、多路径、空数组、内容变化才重跑而不是数组身份变化就重跑,外加一条"路径段里带逗号仍然可用"专门盯着这次修法本身。pnpm check:control-bytes、check-changeset-fixed、check-lint-coverage、check-type-check-coverage全绿;app-shelltype-check通过;改动文件 eslint 0 error(6 条是未触碰行上既有的 React Compiler warning,该门禁按设计只拦 error)。没加 changeset
.changeset/README.md明确列出"内部重构、无 API 变化"不该开 changeset。源码这处是内部 memo key 的编码方式,无任何可观测行为差异;其余都是工具链(脚本、workflow、文档、测试)。仓内也没有"改 packages 必须带 changeset"的门禁 ——ci.yml的 changeset job 只校验 fixed group 成员资格。如果维护者希望这类修复仍然出现在 release notes 里,补一个 patch 即可。Generated by Claude Code