fix(step): gate check_diff forced check_first on Fix mode only by nkakouros · Pull Request #717 · jdx/hk

nkakouros · 2026-03-02T23:02:01Z

When a step has check_diff defined, job_builder unconditionally forces check_first=true. In Check mode this causes check_diff to run instead of check, which may exit 0 for non-auto-fixable violations, silently hiding errors. Gate can_apply_diff on RunType::Fix since diff application is only relevant when fixing.

gemini-code-assist · 2026-03-02T23:02:16Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug where the presence of a check_diff configuration could inadvertently suppress error reporting in Check mode. By refining the can_apply_diff logic to activate exclusively during Fix operations, the system now ensures that the primary check command is executed and its results are accurately reported, preventing silent failures and enhancing the reliability of the checking process. A new test case has been added to validate this corrected behavior.

Highlights

Corrected check_diff behavior in Check mode: Previously, if a step had check_diff defined, the job_builder would unconditionally force check_first=true. In Check mode, this could lead to check_diff running instead of check, potentially hiding non-auto-fixable errors if check_diff exited successfully.
Gated can_apply_diff to Fix mode: The can_apply_diff flag, which dictates whether check_first is forced to allow diff application, is now explicitly gated to RunType::Fix mode. This ensures that the logic for applying diffs is only active when the system is intended to fix issues, not just check them.
Added new test coverage: A new Bats test was introduced to specifically verify that check mode correctly reports errors, even when check_diff is defined and would otherwise exit successfully (indicating no auto-fixable issues).

Changelog

src/step/job_builder.rs
- Modified the can_apply_diff variable assignment to include a conditional check for RunType::Fix.
- Added a clarifying comment explaining why can_apply_diff is only relevant in Fix mode.
test/check_diff_check_mode.bats
- Added a new Bats test file to validate the corrected check_diff behavior.
- Implemented a mock linter that simulates a scenario where --diff exits successfully but a regular check fails.
- Configured a hk.pkl file within the test to use the mock linter for both check and check_diff.
- Asserted that running hk check fails, confirming that non-auto-fixable errors are no longer hidden.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses a bug where defining check_diff in a step would incorrectly force check_first=true during check mode, potentially masking non-auto-fixable errors. The fix correctly restricts this behavior to fix mode only. A comprehensive test case has been added to validate this logic and prevent future regressions. My review includes a minor suggestion to update a comment to reflect this change accurately, improving code clarity. Overall, the change is correct and well-tested.

When a step has check_diff defined, job_builder unconditionally forces check_first=true. In Check mode this causes check_diff to run instead of check, which may exit 0 for non-auto-fixable violations, silently hiding errors. Gate can_apply_diff on RunType::Fix since diff application is only relevant when fixing.

greptile-apps · 2026-03-02T23:06:52Z

Greptile Summary

Fixed a bug where steps with check_diff defined would unconditionally force check_first=true in both Check and Fix modes. In Check mode, this caused check_diff to run instead of the regular check command, which could silently hide non-auto-fixable violations when check_diff exits 0 (no auto-fixable issues) but check would have exited 1 (reporting errors).

Gated can_apply_diff on RunType::Fix since diff application is only relevant when fixing
In Check mode, the regular check command now runs, properly reporting all violations
Added comprehensive test validating the fix with a mock linter that demonstrates the scenario

Confidence Score: 5/5

This PR is safe to merge with minimal risk
Targeted bug fix with clear logic change, comprehensive test coverage validating the fix, and no side effects on existing functionality
No files require special attention

Important Files Changed

Filename	Overview
src/step/job_builder.rs	Fixed logic to only force `check_first=true` in Fix mode when `check_diff` is defined, preventing Check mode from silently hiding non-auto-fixable errors
test/check_diff_check_mode.bats	New test validates that Check mode reports errors when `check_diff` exits 0 but regular `check` exits 1 (non-auto-fixable violations)

_{Last reviewed commit: faf06de}

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [hk](https://github.com/jdx/hk) | minor | `1.36.0` → `1.38.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jdx/hk (hk)</summary> ### [`v1.38.0`](https://github.com/jdx/hk/blob/HEAD/CHANGELOG.md#1380---2026-03-06) [Compare Source](jdx/hk@v1.37.0...v1.38.0) ##### 🚀 Features - **(hook)** add `fail_on_fix` option by [@jdx](https://github.com/jdx) in [#725](jdx/hk#725) ##### 🐛 Bug Fixes - **(builtins)** remove redundant check/check\_diff from builtins by [@nkakouros](https://github.com/nkakouros) in [#726](jdx/hk#726) ##### 📦️ Dependency Updates - update anthropics/claude-code-action digest to [`26ec041`](jdx/hk@26ec041) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#720](jdx/hk#720) - update jdx/mise-action digest to [`e79ddf6`](jdx/hk@e79ddf6) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#721](jdx/hk#721) - update actions-rust-lang/setup-rust-toolchain digest to [`a0b538f`](jdx/hk@a0b538f) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#719](jdx/hk#719) - update rust crate tokio to v1.50.0 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#722](jdx/hk#722) ### [`v1.37.0`](https://github.com/jdx/hk/blob/HEAD/CHANGELOG.md#1370---2026-03-03) [Compare Source](jdx/hk@v1.36.0...v1.37.0) ##### 🚀 Features - **(hook)** add env support to hooks by [@jdx](https://github.com/jdx) in [#709](jdx/hk#709) - parse Go-style diffs by [@jdx](https://github.com/jdx) in [#704](jdx/hk#704) ##### 🐛 Bug Fixes - **(builtins)** strip extra trailing newlines in end-of-file-fixer by [@jdx](https://github.com/jdx) in [#708](jdx/hk#708) - **(docs)** correctly document what --all is about by [@nkakouros](https://github.com/nkakouros) in [#715](jdx/hk#715) - **(git)** exclude untracked files from unstaged\_files set by [@nkakouros](https://github.com/nkakouros) in [#716](jdx/hk#716) - **(hkrc)** config format and load order by [@ivy](https://github.com/ivy) in [#710](jdx/hk#710) - **(release)** write release notes to file instead of capturing stdout by [@jdx](https://github.com/jdx) in [#688](jdx/hk#688) - **(release)** make release notes editorialization non-blocking by [@jdx](https://github.com/jdx) in [#690](jdx/hk#690) - **(step)** gate check\_diff forced check\_first on Fix mode only by [@nkakouros](https://github.com/nkakouros) in [#717](jdx/hk#717) ##### 📚 Documentation - **(shanty)** add audio player with sea shanty recording by [@jdx](https://github.com/jdx) in [67a25ad](jdx/hk@67a25ad) - document config file search paths by [@ivy](https://github.com/ivy) in [#701](jdx/hk#701) - require AI disclosure on GitHub comments by [@jdx](https://github.com/jdx) in [#703](jdx/hk#703) ##### 🔍 Other Changes - replace gen-release-notes script with communique by [@jdx](https://github.com/jdx) in [#700](jdx/hk#700) - add autofix.ci workflow by [@jdx](https://github.com/jdx) in [#705](jdx/hk#705) ##### 📦️ Dependency Updates - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#686](jdx/hk#686) - update taiki-e/upload-rust-binary-action digest to [`f391289`](jdx/hk@f391289) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#692](jdx/hk#692) - update anthropics/claude-code-action digest to [`c22f7c3`](jdx/hk@c22f7c3) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#691](jdx/hk#691) - update rust crate libc to v0.2.181 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#694](jdx/hk#694) - update rust crate clap to v4.5.58 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#693](jdx/hk#693) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#695](jdx/hk#695) - update anthropics/claude-code-action digest to [`edd85d6`](jdx/hk@edd85d6) by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#698](jdx/hk#698) - update rust crate clap to v4.5.60 by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#699](jdx/hk#699) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#702](jdx/hk#702) - lock file maintenance by [@renovate\[bot\]](https://github.com/renovate\[bot]) in [#711](jdx/hk#711) ##### New Contributors - [@ivy](https://github.com/ivy) made their first contribution in [#710](jdx/hk#710) - [@nkakouros](https://github.com/nkakouros) made their first contribution in [#715](jdx/hk#715) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

gemini-code-assist Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread src/step/job_builder.rs Outdated

nkakouros force-pushed the nkakouros/fix/gate-check-diff branch from 317efee to faf06de Compare March 2, 2026 23:06

jdx merged commit fd82019 into jdx:main Mar 3, 2026
19 checks passed

jdx mentioned this pull request Mar 3, 2026

chore: release v1.38.0 #687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(step): gate check_diff forced check_first on Fix mode only#717

fix(step): gate check_diff forced check_first on Fix mode only#717
jdx merged 1 commit intojdx:mainfrom
nkakouros:nkakouros/fix/gate-check-diff

nkakouros commented Mar 2, 2026

Uh oh!

gemini-code-assist Bot commented Mar 2, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nkakouros commented Mar 2, 2026

Uh oh!

gemini-code-assist Bot commented Mar 2, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 2, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants