Hooks prefixing: follow-up improvements from PR #1246 review

[dyoshikawa]

Summary

Follow-up improvements identified during the code review of PR #1246 (fix(hooks): keep executable hook commands unprefixed). These items are not blockers for the PR itself but should be addressed for long-term maintainability and consistency.

Motivation / Purpose

PR #1246 correctly fixes the bug where bare executable commands (e.g., npx prettier --write ./src) were incorrectly prefixed with project directory variables. However, the review surfaced several code quality and consistency concerns that should be tracked and addressed.

Details

1. DRY violation: Duplicated prefixing logic in Gemini CLI (High)

geminicli-hooks.ts (canonicalToGeminicliHooks) duplicates nearly identical prefixing logic that already exists in the shared tool-hooks-converter.ts (canonicalToToolHooks). Claude Code and Factory Droid use the shared converter, but Gemini CLI maintains its own inline implementation.

• Affected files: src/features/hooks/geminicli-hooks.ts, src/features/hooks/tool-hooks-converter.ts
• Action: Refactor Gemini CLI to use the shared converter by adding event name mapping constants and a ToolHooksConverterConfig for Gemini CLI (similar to how Claude Code and Factory Droid are configured). The Gemini CLI converter has extra fields (name, description) that the shared converter doesn't currently handle, so the shared converter may need to support an extensible output shape.

2. Factory Droid behavioral inconsistency (Medium)

Claude Code and Gemini CLI no longer prefix bare executable commands, but Factory Droid still does (since prefixDotRelativeCommandsOnly defaults to false/undefined). A command like npx prettier --write . in hooks.json would be correctly handled for Claude Code and Gemini CLI but produce an invalid $FACTORY_PROJECT_DIR/npx prettier --write . for Factory Droid.

• Affected files: src/features/hooks/factorydroid-hooks.ts
• Action: Evaluate whether Factory Droid should also set prefixDotRelativeCommandsOnly: true. If the current behavior is intentional, document this difference explicitly.

3. Redundant type guard in command assignment (Low)

In both tool-hooks-converter.ts and geminicli-hooks.ts, the ternary condition shouldPrefix && typeof trimmedCommand === "string" has a redundant second check — shouldPrefix already guarantees trimmedCommand is a string.

• Action: Simplify to shouldPrefix ? ... : ... (with appropriate TypeScript narrowing).

4. Missing edge case tests (Low)

The following edge cases lack test coverage:

• ../script.sh (parent-relative path — starts with ., so it gets prefixed, producing $CLAUDE_PROJECT_DIR/../script.sh)
• Leading whitespace commands (e.g., " ./script.sh" — trimmed for prefix check but the trimmed value is used in the result, while non-prefixed commands keep original whitespace)
• Absolute paths (e.g., /usr/local/bin/script.sh)
• Empty command string

5. Incomplete JSDoc for prefixDotRelativeCommandsOnly (Low)

The JSDoc on prefixDotRelativeCommandsOnly in tool-hooks-converter.ts only documents the true behavior. It should also state what happens when false or undefined (default): all commands that don't start with $ are prefixed.

Additional Context

• Related PR: fix(hooks): keep executable hook commands unprefixed #1246
• Related Issue: Claude Code hook commands are always getting prefixed with $CLAUDE_PROJECT_DIR/, even for global binaries #1229