Conversation
Add --inter-hunk-context to options-with-value set and expand checkout known options with --no-* variants. Fix unknown long option handling to not consume following tokens, preventing false negatives in ambiguous ref/path detection.
Add tests for --inter-hunk-context, --no-* options, and verify unknown long options do not consume subsequent tokens.
Add unit tests for shell parsing helpers, rm flags detection, find delete patterns, dangerous text detection, xargs child command extraction, and parallel command parsing.
…ds table Document that 'git checkout <ref> <path>' may overwrite working tree when Git disambiguates ref vs pathspec, matching the parser fix.
- Add set -e to pre-commit for better error handling - Move bun run build and dist staging logic from pre-push to pre-commit - Simplify setup-hooks script to only set executable permissions
📝 WalkthroughWalkthroughPre-commit split into explicit steps (knip → lint-staged → build) and conditionally stages Changes
Sequence Diagram(s)mermaid Dev->>Hook: git commit (pre-commit) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #32 +/- ##
=======================================
Coverage 99.53% 99.53%
=======================================
Files 50 50
Lines 4528 4540 +12
=======================================
+ Hits 4507 4519 +12
Misses 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR fixes a false-negative bug in Key changes:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["token = next token\nfrom git checkout args"] --> B{starts with '-'?}
B -- No --> C["Push to positional[]"]
B -- Yes --> D{token == '--'?}
D -- Yes --> E["STOP – break loop"]
D -- No --> F{in CHECKOUT_OPTS_WITH_VALUE?}
F -- Yes --> G["i += 2\n(consume option + value)"]
F -- No --> H{"starts with '--'\nAND contains '='?"}
H -- Yes --> I["i++\n(skip foo=bar token)"]
H -- No --> J{in CHECKOUT_OPTS_WITH_OPTIONAL_VALUE?}
J -- Yes --> K{"nextToken is a\nrecognised mode value?"}
K -- Yes --> L["i += 2\n(consume option + mode)"]
K -- No --> M["i++\n(option only)"]
J -- No --> N{"starts with '--' AND\nnot in CHECKOUT_KNOWN_OPTS_NO_VALUE\nnot in WITH_VALUE\nnot in WITH_OPTIONAL_VALUE?"}
N -- Yes --> O["i++\n⚠️ FAIL-SAFE: unknown option\ndoes NOT consume next token\n(PR fix)"]
N -- No --> P["i++\n(known no-value flag,\nor short option)"]
style O fill:#ffd700,stroke:#b8860b
style C fill:#90ee90,stroke:#228b22
Last reviewed commit: 5ccaadf |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
.husky/pre-commit
Outdated
| if ! git diff --quiet dist/; then | ||
| git add -A dist/ | ||
| fi |
There was a problem hiding this comment.
git diff only detects changes to already-tracked files
git diff --quiet dist/ compares the working tree to the index (staged content). This means brand-new, previously-untracked files inside dist/ (e.g., after bun run clean wipes the directory in a fresh worktree where dist/ was never committed) will not be detected and therefore won't be auto-staged.
A more robust check would use git status --porcelain dist/, which captures both modified and untracked files:
| if ! git diff --quiet dist/; then | |
| git add -A dist/ | |
| fi | |
| if [ -n "$(git status --porcelain dist/)" ]; then | |
| git add -A dist/ | |
| fi |
This is a minor edge case for the typical development workflow where dist/ is already tracked, but worth considering for contributors who clone and build for the first time before committing.
| "cc-safety-net": "bun run src/bin/cc-safety-net.ts", | ||
| "prepare": "husky && bun run setup-hooks", | ||
| "setup-hooks": "bun -e 'await Bun.write(\".husky/pre-commit\", \"#!/usr/bin/env sh\\n\\nbun run knip && bun run lint-staged\\n\")' && chmod +x .husky/pre-commit" | ||
| "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push" |
There was a problem hiding this comment.
setup-hooks implicitly depends on .husky/pre-push existing
The chmod +x will silently succeed today because .husky/pre-push is still committed, but if the file is ever removed the script will fail. Since the pre-push hook only runs bun run check, it might be intentionally kept minimal — but the dependency is non-obvious. Consider adding a guard or a comment so future maintainers know the file must remain:
| "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push" | |
| "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push || true" |
Using || true makes the prepare lifecycle script resilient to the case where a contributor deletes .husky/pre-push in a follow-up PR, without silently swallowing real errors from the pre-commit chmod.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.husky/pre-commit (1)
4-5: Consider enforcing the full pre-commit quality gate withbun run check.At Line 4-5, the hook runs partial checks. Using the full check command here keeps local gating consistent.
Based on learnings: Run `bun run check` before committing which executes: biome check → typecheck → knip → ast-grep scan → bun test.💡 Suggested adjustment
-bun run knip bun run lint-staged +bun run check🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.husky/pre-commit around lines 4 - 5, Replace the partial pre-commit commands that only run "bun run knip" and "bun run lint-staged" with a full quality gate by invoking "bun run check" (which runs biome check → typecheck → knip → ast-grep scan → bun test) at the start of the hook; update the pre-commit script so the command sequence includes "bun run check" (and optionally retain "bun run lint-staged" afterward if staged-only linting should still run) to ensure the complete suite runs before commits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.husky/pre-commit:
- Around line 9-10: The pre-commit check uses "git diff --quiet dist/" which
misses untracked files; replace the condition so it detects both tracked changes
and new untracked files (e.g., use "git status --porcelain --untracked-files=all
dist/" and check for non-empty output) before running the existing "git add -A
dist/" logic; update the conditional that currently references "git diff --quiet
dist/" to call and test "git status --porcelain --untracked-files=all dist/" (or
an equivalent "git ls-files --others --exclude-standard dist/") so newly
generated files under dist/ are correctly staged.
In `@package.json`:
- Line 30: The "setup-hooks" npm script still references a removed hook file and
will fail when running prepare; update the "setup-hooks" script (the
"setup-hooks" entry in package.json) to stop referencing ".husky/pre-push" —
either remove ".husky/pre-push" so it only chmods ".husky/pre-commit", or change
the command to check for the file's existence before chmod so prepare doesn't
fail on fresh installs.
---
Nitpick comments:
In @.husky/pre-commit:
- Around line 4-5: Replace the partial pre-commit commands that only run "bun
run knip" and "bun run lint-staged" with a full quality gate by invoking "bun
run check" (which runs biome check → typecheck → knip → ast-grep scan → bun
test) at the start of the hook; update the pre-commit script so the command
sequence includes "bun run check" (and optionally retain "bun run lint-staged"
afterward if staged-only linting should still run) to ensure the complete suite
runs before commits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 635926ac-7953-4cf5-be7c-fe150cd61d36
⛔ Files ignored due to path filters (2)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (7)
.husky/pre-commit.husky/pre-pushREADME.mdpackage.jsonsrc/core/rules-git.tstests/core/analyze/parsing-helpers.test.tstests/core/rules-git.test.ts
💤 Files with no reviewable changes (1)
- .husky/pre-push
| "cc-safety-net": "bun run src/bin/cc-safety-net.ts", | ||
| "prepare": "husky && bun run setup-hooks", | ||
| "setup-hooks": "bun -e 'await Bun.write(\".husky/pre-commit\", \"#!/usr/bin/env sh\\n\\nbun run knip && bun run lint-staged\\n\")' && chmod +x .husky/pre-commit" | ||
| "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push" |
There was a problem hiding this comment.
setup-hooks still references .husky/pre-push, which can break prepare.
At Line 30, this will fail on fresh installs if .husky/pre-push was removed in this PR.
💡 Proposed fix
- "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push"
+ "setup-hooks": "chmod +x .husky/pre-commit"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "setup-hooks": "chmod +x .husky/pre-commit .husky/pre-push" | |
| "setup-hooks": "chmod +x .husky/pre-commit" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 30, The "setup-hooks" npm script still references a
removed hook file and will fail when running prepare; update the "setup-hooks"
script (the "setup-hooks" entry in package.json) to stop referencing
".husky/pre-push" — either remove ".husky/pre-push" so it only chmods
".husky/pre-commit", or change the command to check for the file's existence
before chmod so prepare doesn't fail on fresh installs.
The previous git diff --quiet approach only detected changes to tracked files, missing newly generated untracked files in dist/. Using git status --porcelain --untracked-files=all ensures both modified and new files are properly detected and staged.
The previous git diff --quiet approach only detected changes to tracked files, missing newly generated untracked files in dist/. Using git status --porcelain --untracked-files=all ensures both modified and new files are properly detected and staged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/scripts/pre-commit.test.ts (1)
17-77: Align private helper signatures with TS repo conventions.Module-private helpers should use a leading underscore, and non-mutated command args should be
readonly string[]in these helper signatures.As per coding guidelines: "Use leading underscore for private/internal module functions in TypeScript" and "Use
readonlyarrays where mutation isn't needed in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/scripts/pre-commit.test.ts` around lines 17 - 77, Rename the module-private helper functions to use a leading underscore and mark non-mutated command argument arrays as readonly: change runCommand(args: string[], ...) to _runCommand(args: readonly string[], ...), runChecked(args: string[], ...) to _runChecked(args: readonly string[], ...), and similarly rename runPreCommit to _runPreCommit and distStatus to _distStatus (and createTestRepo to _createTestRepo if it's intended module-private); update all internal call sites to the new names and adjust signatures so any functions that accept CLI arg arrays use readonly string[] rather than string[].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/scripts/pre-commit.test.ts`:
- Around line 49-52: The test setup calls runChecked to configure git user and
gpg but doesn’t set line-ending config, which causes LF→CRLF warnings on Windows
and makes runChecked’s empty-stderr assertion fail; update the repo setup in
tests/scripts/pre-commit.test.ts where runChecked is invoked to also disable
autocrlf and safecrlf by running git config core.autocrlf false and git config
core.safecrlf false (use the same runChecked helper) so the test repo won’t emit
CRLF conversion warnings.
---
Nitpick comments:
In `@tests/scripts/pre-commit.test.ts`:
- Around line 17-77: Rename the module-private helper functions to use a leading
underscore and mark non-mutated command argument arrays as readonly: change
runCommand(args: string[], ...) to _runCommand(args: readonly string[], ...),
runChecked(args: string[], ...) to _runChecked(args: readonly string[], ...),
and similarly rename runPreCommit to _runPreCommit and distStatus to _distStatus
(and createTestRepo to _createTestRepo if it's intended module-private); update
all internal call sites to the new names and adjust signatures so any functions
that accept CLI arg arrays use readonly string[] rather than string[].
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a589232-6c33-4773-a45a-6181550e14ab
📒 Files selected for processing (2)
.husky/pre-committests/scripts/pre-commit.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/scripts/pre-commit.test.ts (1)
87-122: Consider adding a test for when dist/ has no changes.The existing tests cover staging of new untracked and modified tracked files. A test verifying that nothing is staged when dist/ is unchanged would complete the coverage matrix.
💡 Suggested test case
test('does not stage when dist is unchanged', () => { const { repoDir, env } = createTestRepo(); mkdirSync(join(repoDir, 'dist'), { recursive: true }); writeFileSync(join(repoDir, 'dist', 'index.js'), 'console.log("committed");\n'); runChecked(['git', 'add', '-A'], repoDir, env); runChecked(['git', 'commit', '-qm', 'initial'], repoDir, env); runPreCommit(repoDir, env); expect(distStatus(repoDir, env)).toBe(''); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/scripts/pre-commit.test.ts` around lines 87 - 122, Add a new test that verifies no files are staged when dist/ is unchanged: use createTestRepo() to get { repoDir, env }, create dist/ and write an initial file (e.g., writeFileSync(join(repoDir,'dist','index.js'), 'console.log("committed");\n')), stage and commit it with runChecked(['git','add','-A'], repoDir, env) and runChecked(['git','commit','-qm','initial'], repoDir, env), then call runPreCommit(repoDir, env) and assert distStatus(repoDir, env) returns an empty string; place this alongside the other tests in the same describe block to complete coverage for unchanged dist/.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/scripts/pre-commit.test.ts`:
- Around line 87-122: Add a new test that verifies no files are staged when
dist/ is unchanged: use createTestRepo() to get { repoDir, env }, create dist/
and write an initial file (e.g., writeFileSync(join(repoDir,'dist','index.js'),
'console.log("committed");\n')), stage and commit it with
runChecked(['git','add','-A'], repoDir, env) and
runChecked(['git','commit','-qm','initial'], repoDir, env), then call
runPreCommit(repoDir, env) and assert distStatus(repoDir, env) returns an empty
string; place this alongside the other tests in the same describe block to
complete coverage for unchanged dist/.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 756874e5-0fe4-46b3-8db8-5d32cc37aef9
📒 Files selected for processing (1)
tests/scripts/pre-commit.test.ts
1 participant
Summary
git checkoutpositional args parser to correctly handle known no-value options, options with required values, and options with optional values — preventing false negatives wheregit checkout <ref> <pathspec>was incorrectly alloweddist/build & auto-stage from pre-push to pre-commit hook and simplifysetup-hooksscriptChanges
Core rule fix (
src/core/rules-git.ts)CHECKOUT_OPTS_WITH_VALUEset for options that always consume the next token (e.g.-B,--orphan,--conflict,--inter-hunk-context,--pathspec-from-file,--unified)CHECKOUT_OPTS_WITH_OPTIONAL_VALUEset for options that may or may not consume a value (e.g.--recurse-submodules,--track)CHECKOUT_KNOWN_OPTS_NO_VALUEset for all boolean/flag-only options (e.g.--quiet,--force,--detach,--merge,--patch,--guess,--overlay, etc.)getCheckoutPositionalArgs()to correctly classify tokens using these sets, so thatgit checkout --no-quiet main file.txtis properly detected as having two positional args and blockedTests
tests/core/rules-git.test.ts— Added tests for--no-quiet,--guess,--recurse-submodules(with and without=value),--no-recurse-submodules,--inter-hunk-context, and unknown long option casestests/core/analyze/parsing-helpers.test.ts— Added unit tests for_getCheckoutPositionalArgscovering known no-value options, optional-value options, required-value options, and unknown option handlingCI / Hooks
.husky/pre-commit— Now runsbun run buildand auto-stagesdist/if changed (moved from pre-push).husky/pre-push— Removed entirely (replaced by pre-commit build step)package.json— Simplifiedsetup-hooksscript to justchmod +x .husky/pre-commit .husky/pre-pushDocumentation
README.md— Addedgit checkout <ref> <path>to the blocked commands table with a note about Git ref vs pathspec disambiguationScreenshots
N/A — CLI-only changes, no visual UI.
Testing
Related Issue
N/A
PR Checklist
bun run checkpasses (lint, types, dead code, rules, tests)package.jsonSummary by CodeRabbit
Chores
New Features
Documentation
Tests