feat(ci): add fast local validation hooks - #4132
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for trying to shorten the local feedback loop. Reusing the existing Biome, ASF-header, and protocol-epoch checks is the right direction; the hooks do not need to become another validation authority.
I left one inline P2 on executable resolution. The current npx --no invocation can execute an unrelated cached package when the repository-local dependency is absent, which I reproduced in an isolated offline directory. This affects a recoverable local-development path rather than product or repository data, so I have classified it as P2.
There is also a broader behavior decision worth discussing before making this the default for every clone. Installing dependencies would configure hooks that affect every local commit, including intermediate commits that the repository does not currently require to satisfy commitlint. The PR does not link to a reported problem or explain the opt-out and omit-dev recovery behavior. I suggest opening a Discussion or focused issue to confirm that default-installed hooks are the desired project policy. This is a suggestion about where to make the community decision, rather than an objection to the implementation.
Reviewed with Codex and two independent @Reviewer agents. I verified the exact head, staged-check ownership, dependency-resolution behavior, omit-dev path, and passing CI.
中文对照
谢谢你尝试缩短本地反馈周期。复用现有的 Biome、ASF header 和 protocol epoch 检查是正确方向;这些 hook 不需要成为另一套验证 authority。
我在可执行文件解析处留了一条 P2 行内评论。当前的 npx --no 在仓库本地依赖缺失时可能执行无关的缓存包,我已经在隔离的离线目录中复现。它影响的是可以恢复的本地开发路径,不涉及产品或仓库数据,因此定为 P2。
此外,在让它成为每个 clone 的默认行为之前,还有一个更适合先讨论的行为决策:安装依赖后,hook 会影响每一次本地提交,包括仓库目前并未要求通过 commitlint 的中间提交。PR 没有关联一个已经报告的问题,也没有说明 opt-out 和 omit-dev 后的恢复行为。我建议先创建一个 Discussion 或聚焦 issue,确认默认安装 hook 是否是社区希望采用的项目策略。这是在建议把社区层面的决定放到更合适的地方讨论,并不是对当前实现的否定。
本次审查使用了 Codex 和两个独立的 @Reviewer 子代理;我核对了精确 head、staged check 的职责、依赖解析行为、omit-dev 路径和 CI 结果。
Withdrawing this approval after reconsidering the product and community decision boundary. The implementation finding remains a recoverable P2, but the broader change—installing hooks that affect every local commit by default—does not yet have a linked issue, demonstrated need, or community decision. This should remain under discussion rather than be treated as approved implementation.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks again for working on faster local validation. I am following up to correct my earlier approval.
Reusing the existing validators is technically sensible, and the inline npx --no finding remains a recoverable P2. However, absence of a blocking implementation defect is not enough by itself to establish that the PR should merge.
This change configures hooks for every clone and affects every local commit, including intermediate commits that the project does not currently require to satisfy commitlint. There is no linked issue or evidence showing that this default behavior is needed. I suggest moving the behavior proposal to a Discussion first, so contributors and maintainers can decide whether mandatory-by-default hooks are the desired policy and what the opt-out and omit-dev behavior should be.
This follow-up is about correcting the approval state and putting the community decision at the right boundary, not dismissing the work already done.
Reviewed with Codex and two independent @Reviewer agents.
中文对照
再次谢谢你处理本地快速验证。我补充这条评论,是为了纠正之前的 Approval。
复用现有 validator 在技术方向上是合理的,行内提到的 npx --no 问题也仍然只是一个可以恢复的 P2。不过,没有阻塞性的实现缺陷,并不能单独证明这篇 PR 应该合并。
这个改动会为每个 clone 配置 hook,并影响每一次本地提交,包括项目目前并未要求通过 commitlint 的中间提交。现在没有关联 issue,也没有证据说明必须默认启用这个行为。我建议先把行为提案放到 Discussion,让贡献者和维护者决定:默认强制 hook 是否是希望采用的项目策略,以及 opt-out 和 omit-dev 行为应该如何定义。
这条跟进是为了纠正 Approval 状态,并把社区决策放回正确的边界,不是否定已经完成的工作。
本次审查使用了 Codex 和两个独立的 @Reviewer 子代理。
80d3920 to
b220391
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for addressing the executable-resolution issue and for opening Discussion #4156. I reviewed exact head b22039191fc73cf327d241481a642250a85c60bd.
The earlier implementation P2 is resolved: both hooks now resolve tooling from this checkout, omit-dev setup skips installation cleanly, and stale hooks fail explicitly instead of executing cached or global packages. I did not find a remaining code-level P0-P2.
I am still holding approval because the remaining question is the PR’s product for contributors, not another code fix. Discussion #4156 currently has no maintainer/community conclusion on whether hooks should install by default, whether commitlint should govern every intermediate commit, or whether setup should be opt-in. A green implementation cannot decide that policy by itself.
No request for changes from me: once the Discussion records a clear direction and this PR matches it, the technical implementation is ready for a short exact-head rereview.
中文对照
感谢修复 executable resolution,并创建 Discussion #4156。我复核了最新提交 b22039191fc73cf327d241481a642250a85c60bd。
之前的实现 P2 已经解决:两个 hook 都只解析当前 checkout 内的工具,omit-dev 会干净地跳过安装,陈旧 hook 在缺少本地依赖时会明确失败,而不会执行缓存或全局 package。代码层面没有发现剩余的 P0-P2。
目前仍不批准,是因为剩下的不是另一个代码问题,而是这个 PR 对贡献者产生的行为是否被社区接受。Discussion #4156 还没有维护者/社区结论:hook 是否应默认安装、commitlint 是否应约束每一个中间 commit、还是应该 opt-in。实现绿色本身不能代替这项 policy 决策。
这里不发 Request Changes:Discussion 形成明确方向、且 PR 与之对齐后,技术实现只需要一次很短的 latest-head 复核。
AI-assisted review: Codex checked the incremental implementation and live Discussion state; I independently verified the remaining decision boundary and exact-head conclusion.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for tightening the local validation path and for removing the earlier npx ambiguity. The exact-head hosted checks are green. I found one staged-content mismatch in the hook itself.
This is a suggestion from an outside review; please push back if Biome has an index-reading mode here that the isolated reproduction did not exercise.
中文摘要
感谢收紧本地校验路径并移除此前的 npx 歧义。当前仍有 1 个 P2:biome check --staged 选择的是 staged 文件名,但读取的是工作树内容,不是 index 中将要提交的 bytes,因此会放行坏的 staged 内容或误拦好的 staged 内容。
AI-assisted review disclosure: Codex coordinated an independent reviewer lane; Astro-Han independently checked the exact head, hook behavior, merge state, and severity, and owns this review.
b220391 to
28d4212
Compare
Catch staged formatting, license, and protocol epoch issues before CI. Keep the hook fast enough to run on every commit. Generated-by: OpenAI Codex Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
Use standard commitlint rules for local messages. Keep them aligned with the repository contribution format. Generated-by: OpenAI Codex Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
Keep dev-dependency-free installs working. Prevent hooks from downloading fallback packages. Generated-by: OpenAI Codex Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
Read index snapshots for partially staged files. Check the bytes that will actually be committed. Generated-by: OpenAI Codex Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
Intermediate commits are squashed and never reach main. Only the pull request title needs conventional validation. Generated-by: OpenAI Codex Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
fb4ed7e to
ce8b7c1
Compare
Summary
Agent-assisted changes can violate repository invariants that only surface
after a branch is pushed, causing an avoidable CI failure and another
edit-push cycle.
Install lightweight local Git hooks through the normal npm setup so agents
and developers receive that feedback at commit time:
pre-commitchecks staged formatting and lint, ASF headers for newly addedfiles, Runtime Host protocol epoch changes, and basic diff errors.
commit-msgvalidates Conventional Commit messages with the standardcommitlint configuration.
commands never download fallback packages.
Discussion
Installing these hooks by default affects every local commit, including
intermediate commits. The project-policy question is open for community input
in Discussion #4156.
Developers can bypass the hooks once with
git commit --no-verify, or disableHusky for a command or environment with
HUSKY=0.npm ci --omit=devskipsHusky setup. If hooks already exist but local development dependencies are
later omitted, run a normal
npm installto restore the tools, or bypass thehooks intentionally. Missing local tools fail clearly instead of falling back
to cached or global packages.
Verification
npm install --package-lock-only --ignore-scripts --no-audit --no-fundnpm run lintnpm run format:checknode --test --test-concurrency=1 scripts/protocol-epoch-check.test.mjs scripts/asf-license-headers.test.mjs(56 passed)npm run check:asf-headersnpm run buildnpm run typecheckcommit-msgrejects a non-conventional message.omitted.
when their local tools are absent.
AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex implemented the hooks and staged checks,
added tests, and ran verification.
Checklist
Does this PR entail a change in behavior?