fix(hook): make hook-augment work on Windows drive-letter paths#619
Open
catsmonster wants to merge 1 commit into
Open
fix(hook): make hook-augment work on Windows drive-letter paths#619catsmonster wants to merge 1 commit into
catsmonster wants to merge 1 commit into
Conversation
hook-augment's PreToolUse Grep/Glob augmenter required POSIX-style '/'-prefixed absolute paths in two guards, so a Windows cwd (C:\repo or C:/repo) always failed: the _WIN32 cwd check returned early, and the walk-up loop guard (dir[0] == '/') never iterated. The result was a structural no-op on Windows — zero additionalContext for every Grep/Glob, even for an exactly-indexed symbol. Add ha_is_abs(), which accepts POSIX "/..." and Windows "X:/..." roots; normalize backslashes to forward slashes on the Windows cwd; and stop the parent-climb at the drive root as well as the POSIX root. POSIX behavior is unchanged. Adds a cli_hook_augment_path_is_abs regression test (the predicate is exposed as cbm_hook_path_is_abs via cli.h) covering POSIX and Windows drive roots — it fails on the pre-fix POSIX-only guard and passes after. Verified with that test plus a standalone walk self-test (full path resolves first, then climbs to one level below the drive root, symmetric to POSIX); clang-format clean on all three files; clang -fsyntax-only on the translation unit. Not built through the full toolchain locally (no make/zlib) — relying on Windows CI to confirm the build. Fixes DeusData#618 Signed-off-by: catsmonster <shaked.brand@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
catsmonster
force-pushed
the
fix/hook-augment-windows-paths
branch
from
48a2738 to
26e85d5
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Fixes the
hook-augmentPreToolUse Grep/Glob augmenter being a structural no-op on Windows.Closes #618.
Why
src/cli/hook_augment.chad two POSIX-only path guards that a Windows drive-lettercwd(C:\repo/C:/repo) can never satisfy:cbm_cmd_hook_augment,_WIN32branch —if (!cwd || cwd[0] != '/')returned early on every invocation (a Windows absolute path never starts with/).ha_resolve_and_querywalk-up loop —for (... && dir[0] == '/'; ...)never iterated for aC:/...path, and the parent-climb terminator assumed a/-root.cbm_project_name_from_pathalready handles Windows paths (it produced the indexed project name from theC:\...root), so the fix is purely in these path assumptions.How
cbm_hook_path_is_abs()— true for POSIX"/..."and Windows"X:/..."roots.\→/on the Windowscwdbefore validation.cbm_hook_path_is_abs(dir); stop the parent-climb at the drive root (X:/) as well as the POSIX root.cwd[0] != '/'→getcwdfallback is preserved via the predicate).Testing
The predicate is exposed as
cbm_hook_path_is_abs(declared incli.h) so it has direct regression coverage.tests/test_cli.c::cli_hook_augment_path_is_abs(registered in theclisuite): asserts the predicate accepts/...,C:/Users/me/proj,C:/,C:,d:/...and rejectsrelative/path,proj,"",NULL. It fails on the old POSIX-only guard and passes after the fix. Being pure string logic, it also runs on the Linux CI.C:\Users\me\projthe resolver queriesC:/Users/me/proj→C:/Users/me→C:/Users(stops one below the drive root), exactly symmetric to POSIX/home/u/proj→/home/u→/home.clang-formatclean on all three changed files;clang -fsyntax-only -I src -I vendored src/cli/hook_augment.cpasses.make/zliblocally, so I'm relying on Windows CI to confirm the compile/link. Flagging that explicitly.clang-tidy note: my additions follow the file's existing idiom (
[4096]buffers, uncheckedsnprintf, drive-index access) that the surrounding code already uses, so I kept them consistent rather than adding one-off named-constants/void-casts. (CI runslint.sh --ci, which skips clang-tidy.)Notes
The "never block a tool" guarantee is untouched — every path still ends in
exit 0with output written exactly once. This only changes whether context is added on Windows; it can still never deny a tool. Fits under the Windows umbrella tracker #394.