Skip to content

feat(ci): add fast local validation hooks - #4132

Open
Phoenix500526 wants to merge 3 commits into
apache:mainfrom
Phoenix500526:ci/pre-commit-fast-checks
Open

feat(ci): add fast local validation hooks#4132
Phoenix500526 wants to merge 3 commits into
apache:mainfrom
Phoenix500526:ci/pre-commit-fast-checks

Conversation

@Phoenix500526

@Phoenix500526 Phoenix500526 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

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-commit checks staged formatting and lint, ASF headers for newly added
    files, Runtime Host protocol epoch changes, and basic diff errors.
  • commit-msg validates Conventional Commit messages with the standard
    commitlint configuration.
  • Installs that omit root development dependencies continue to work, and hook
    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 disable
Husky for a command or environment with HUSKY=0. npm ci --omit=dev skips
Husky setup. If hooks already exist but local development dependencies are
later omitted, run a normal npm install to restore the tools, or bypass the
hooks 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-fund
  • npm run lint
  • npm run format:check
  • node --test --test-concurrency=1 scripts/protocol-epoch-check.test.mjs scripts/asf-license-headers.test.mjs (56 passed)
  • npm run check:asf-headers
  • npm run build
  • npm run typecheck
  • Confirmed commit-msg rejects a non-conventional message.
  • Confirmed Husky setup skips cleanly when root development dependencies are
    omitted.
  • Confirmed both hooks resolve executables from this checkout and fail clearly
    when their local tools are absent.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the hooks and staged checks,
added tests, and ran verification.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes, described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 29, 2026
Astro-Han
Astro-Han previously approved these changes Aug 29, 2026

@Astro-Han Astro-Han 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.

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 结果。

Comment thread .husky/pre-commit Outdated
@Astro-Han
Astro-Han dismissed their stale review August 29, 2026 07:13

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 Astro-Han 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.

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 子代理。

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>
@Phoenix500526
Phoenix500526 force-pushed the ci/pre-commit-fast-checks branch from 80d3920 to b220391 Compare August 29, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants