Gemini CLI Hook Support (PR #131 Part 3)#158
Open
ahundt wants to merge 256 commits intortk-ai:masterfrom
Open
Conversation
Release v0.2.1
Add installation guide for AI coding assistants
Add utils.rs with common utilities used across modern JavaScript tooling commands: - truncate(): Smart string truncation with ellipsis - strip_ansi(): Remove ANSI escape codes from output - execute_command(): Centralized command execution with error handling These utilities enable consistent output formatting and filtering across multiple command modules. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add comprehensive support for modern JS/TS development stack: Commands added: - rtk lint: ESLint/Biome output with grouped rule violations (84% reduction) - rtk tsc: TypeScript compiler errors grouped by file (83% reduction) - rtk next: Next.js build output with route/bundle metrics (87% reduction) - rtk prettier: Format checker showing only files needing changes (70% reduction) - rtk playwright: E2E test results showing failures only (94% reduction) - rtk prisma: Prisma CLI without ASCII art (88% reduction) Features: - Auto-detects package managers (pnpm/yarn/npm/npx) - Preserves exit codes for CI/CD compatibility - Groups errors by file and error code for quick navigation - Strips verbose output while retaining critical information Total: 6 new commands, ~2,000 LOC Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Document the 6 new commands and shared utils module in CHANGELOG.md. Focuses on token reduction metrics and CI/CD compatibility. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add benchmarks for the 6 new commands in scripts/benchmark.sh: - tsc: TypeScript compiler error grouping - prettier: Format checker with file filtering - lint: ESLint/Biome grouped violations - next: Next.js build metrics extraction - playwright: E2E test failure filtering - prisma: Prisma CLI without ASCII art All benchmarks are conditional (skip if tools not available or not applicable to current project). Tests only run on projects with package.json and relevant configuration files. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implements heuristic calculation of monthly quota savings percentage with support for Pro, Max 5x, and Max 20x subscription tiers. Features: - --quota flag displays monthly quota analysis - --tier <pro|5x|20x> selects subscription tier (default: 20x) - Heuristic based on ~44K tokens/5h Pro baseline - Estimates: Pro=6M, 5x=30M, 20x=120M tokens/month - Clear disclaimer about rolling 5-hour windows vs monthly caps Example output for Max 20x: Subscription tier: Max 20x ($200/mo) Estimated monthly quota: 120.0M Tokens saved (lifetime): 356.7K Quota preserved: 0.3% Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
feat: add modern JavaScript tooling support (lint, tsc, next, prettier, playwright, prisma)
Add rtk gh command for GitHub CLI operations with intelligent output filtering: Commands: - rtk gh pr list/view/checks/status: PR management (53-87% reduction) - rtk gh issue list/view: Issue tracking (26% reduction) - rtk gh run list/view: Workflow monitoring (82% reduction) - rtk gh repo view: Repository info (29% reduction) Features: - Level 1 optimizations (default): Remove header counts, @ prefix, compact mergeable status (+12-18% savings, zero UX loss) - Level 2 optimizations (--ultra-compact flag): ASCII icons, inline checks format (+22% total savings on PR view) - GraphQL response parsing and grouping - Preserves all critical information for code review Token Savings (validated on production repo): - rtk gh pr view: 87% (24.7K → 3.2K chars) - rtk gh pr checks: 79% (8.9K → 1.8K chars) - rtk gh run list: 82% (10.2K → 1.8K chars) Global --ultra-compact flag added to enable Level 2 optimizations across all GitHub commands. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add utils.rs as Key Architectural Component - Expand Module Responsibilities table (7→17 modules) - Document PR rtk-ai#9 in Fork-Specific Features section - Include token reduction metrics for all new commands
Change dtolnay/rust-action to dtolnay/rust-toolchain (correct name)
feat: add GitHub CLI integration (depends on rtk-ai#9)
feat: add quota analysis with multi-tier support
## Release automation - Add release-please workflow for automatic semantic versioning - Configure release.yml to only trigger on tags (avoid double-release) ## Benchmark automation - Extend benchmark.yml with README auto-update - Add permissions for contents and pull-requests writes - Auto-create PR with updated metrics via peter-evans/create-pull-request - Add scripts/update-readme-metrics.sh for CI integration ## Verification - ✅ Workflows ready for CI/CD pipeline - ✅ No breaking changes to existing functionality Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
feat: CI/CD automation (versioning, benchmarks, README auto-update)
…s--master--components--rtk chore(master): release 0.3.0
## Fixes ### Lint crash handling - Add graceful error handling for linter crashes (SIGABRT, OOM) - Display warning message when process terminates abnormally - Show first 5 lines of stderr for debugging context ### Grep command - Add --type/-t flag for file type filtering (e.g., --type ts, --type py) - Passes --type argument to ripgrep for efficient filtering ### Find command - Add --type/-t flag for file/directory filtering - Default: "f" (files only) - Options: "f" (file), "d" (directory) ## Testing - ✅ cargo check passes - ✅ cargo build --release succeeds - ✅ rtk grep --help shows --file-type flag - ✅ rtk find --help shows --file-type flag with default ## Breaking Changes None - all changes are backwards compatible additions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…d-bugs fix: improve command robustness and flag support
…s--master--components--rtk chore(master): release 0.3.1
## Overview Complete architectural documentation (1133 lines) covering all 30 modules, design patterns, and extensibility guidelines. ## Critical Fixes (🔴) - ✅ Module count: 30 documented (not 27) - added deps, env_cmd, find_cmd, local_llm, summary, wget_cmd - ✅ Language: Fully translated to English for consistency with README.md - ✅ Shared Infrastructure: New section documenting utils.rs and package manager detection - ✅ Exit codes: Correct documentation (git.rs preserves exit codes for CI/CD) - ✅ Database: Correct path ~/.local/share/rtk/history.db (not tracking.db) ## Important Additions (🟡) - ✅ Global Flags Architecture: Verbosity (-v/-vv/-vvv) and ultra-compact (-u) - ✅ Complete patterns: Package manager detection, exit code preservation, lazy static regex - ✅ Config system: TOML format documented - ✅ Performance: Verified binary size (4.1 MB) and estimated overhead - ✅ Filter levels: Before/after examples with Rust code ## Bonus Improvements (🟢) - ✅ Table of Contents (12 sections) - ✅ Extensibility Guide (7-step process for adding commands) - ✅ Architecture Decision Records (Why Rust? Why SQLite?) - ✅ Glossary (7 technical terms) - ✅ Module Development Pattern (template + 3 common patterns) - ✅ 15+ ASCII diagrams for visual clarity ## Stats - Lines: 1133 (+118% vs original 520) - Sections: 12 main + subsections - Code examples: 10+ Rust/bash snippets - Accuracy: 100% verified against source code Production-ready for new contributors, experienced developers, and LLM teams. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
docs: add comprehensive ARCHITECTURE.md v2.0
Updates documentation to reflect all features added in recent PRs: **Version Updates** - Update installation commands to v0.3.1 (DEB/RPM packages) **New Sections** - Add Global Flags section (-u/--ultra-compact, -v/--verbose) - Add JavaScript/TypeScript Stack section (10 new commands) **New Commands Documented** - Files: `rtk smart` (heuristic code summary) - Commands: `rtk gh` (GitHub CLI), `rtk wget`, `rtk config` - Data: `rtk gain --quota` and `--tier` flags - Containers: `rtk kubectl services` - JS/TS Stack: lint, tsc, next, prettier, vitest, playwright, prisma **Features Coverage** This update documents functionality from: - PR rtk-ai#5: Git argument parsing improvements - PR rtk-ai#6: pnpm support - PR rtk-ai#9: Modern JavaScript/TypeScript stack support - PR rtk-ai#10: GitHub CLI integration - PR rtk-ai#11: Quota analysis features - PR rtk-ai#14: Additional command improvements All commands documented are available in v0.3.1. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…update docs: comprehensive documentation update for v0.3.1
Add automated workflow step to update the 'latest' tag after each successful release. This ensures 'latest' always points to the most recent stable version without manual intervention. The new job: - Runs after successful release completion - Updates 'latest' tag to point to the new semver tag - Uses force push to move the tag reference - Includes version info in tag annotation message Benefits: - Install scripts can reliably use /releases/latest/download/ - No manual tag management needed - Consistent reference for "current stable" across platforms Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…-tag ci: automate 'latest' tag update on releases
…tics Implement day-by-day, week-by-week, and monthly breakdowns with JSON/CSV export capabilities for in-depth token savings analysis and reporting. New Features: - Daily breakdown (--daily): Complete day-by-day statistics without 30-day limit - Weekly breakdown (--weekly): Sunday-to-Saturday week aggregations with date ranges - Monthly breakdown (--monthly): Calendar month aggregations (YYYY-MM format) - Combined view (--all): All temporal breakdowns in single output - JSON export (--format json): Structured data for APIs, dashboards, scripts - CSV export (--format csv): Tabular data for Excel, Google Sheets, data science Technical Implementation: - src/tracking.rs: Add DayStats, WeekStats, MonthStats structures with Serialize - src/tracking.rs: Implement get_all_days(), get_by_week(), get_by_month() SQL queries - src/main.rs: Extend Commands::Gain with --daily, --weekly, --monthly, --all, --format flags - src/gain.rs: Add print_daily_full(), print_weekly(), print_monthly() display functions - src/gain.rs: Implement export_json() and export_csv() for data export Documentation: - docs/AUDIT_GUIDE.md: Comprehensive guide with examples, workflows, integrations - README.md: Update Data section with new audit commands and export formats - claudedocs/audit-feature-summary.md: Technical summary and implementation details Database Scope: - Global machine storage: ~/.local/share/rtk/history.db - Shared across all projects, worktrees, and Claude sessions - 90-day retention policy with automatic cleanup - SQLite with indexed timestamp for fast aggregations Use Cases: - Trend analysis: identify daily/weekly patterns in token usage - Cost reporting: monthly savings reports for budget tracking - Data science: export CSV/JSON for pandas, R, Excel analysis - Dashboards: integrate JSON export with Chart.js, D3.js, Grafana - CI/CD: automated weekly/monthly savings reports via GitHub Actions Examples: rtk gain --daily # Day-by-day breakdown rtk gain --weekly # Weekly aggregations rtk gain --all # All breakdowns combined rtk gain --all --format json | jq . # JSON export with jq rtk gain --all --format csv # CSV for Excel/analysis Backwards Compatibility: - All existing flags (--graph, --history, --quota) preserved - Default behavior unchanged (summary view) - No database migration required - Zero breaking changes Performance: - Efficient SQL aggregations with timestamp index - No impact on rtk command execution speed - Instant queries even with 90 days of data Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
d2c2dca to
3c4f7fa
Compare
# Conflicts: # .release-please-manifest.json # ARCHITECTURE.md # CHANGELOG.md # CLAUDE.md # Cargo.lock # Cargo.toml # INSTALL.md # README.md # hooks/rtk-rewrite.sh # src/cargo_cmd.rs # src/container.rs # src/discover/registry.rs # src/gh_cmd.rs # src/git.rs # src/go_cmd.rs # src/grep_cmd.rs # src/init.rs # src/lint_cmd.rs # src/main.rs # src/playwright_cmd.rs # src/pytest_cmd.rs # src/tsc_cmd.rs # src/vitest_cmd.rs
Add rtk hook gemini for Gemini CLI BeforeTool hook handler. Different wire format from Claude: uses decision/reason fields, hook_event_name/tool_name guards, and run_shell_command matcher. Add rtk init --gemini for Gemini CLI integration with: - RTK.md deployment to ~/.gemini/ - GEMINI.md patching with @RTK.md reference - settings.json hook registration Multi-platform init: --claude (default) and --gemini flags. Both can coexist. Shared infrastructure: patch_settings_shared, patch_instruction_file, remove_hook_from_settings_file. Additive platform selection: rtk init with no flags sets up both Claude Code and Gemini CLI. --claude alone skips Gemini, --gemini alone skips Claude. Tests: 560 total (19 gemini_hook tests)
3c4f7fa to
670cd3f
Compare
Commands were being tracked with incorrect labels: - "git status" → "rtk run git status" (wrong) - "rtk git status" → "rtk run rtk git status" (double-rtk bug) Fixes: - Use try_route_native_command() to get the correct RTK command label - Handle "rtk" binary specially to avoid "rtk run rtk ..." tracking - Unknown commands still tracked as "rtk run <cmd>" Changes: - src/cmd/hook.rs: Make route_native_command and try_route_native_command pub(crate) so they can be used from exec.rs - src/cmd/exec.rs: Fix tracking in spawn_with_filter() to use routed command names instead of always using "rtk run <cmd>" Tests added: - test_tracking_routed_command_uses_rtk_prefix - test_tracking_git_status_uses_rtk_git - test_tracking_cargo_test_uses_rtk_cargo - test_tracking_unknown_command_uses_rtk_run - test_tracking_rtk_self_reference_no_double_rtk - test_tracking_grep_uses_rtk_grep - test_tracking_find_uses_rtk_run All 902 tests pass.
The default trash crate uses Finder's AppleScript method which plays the system trash sound effect. This is disruptive when RTK is used in the background during Claude Code sessions. Solution: Use NSFileManager method on macOS which: - Does NOT produce the trash sound - Is faster than the Finder method - Does not require additional permissions Tradeoff: The "Put Back" option in Finder is lost (macOS bug in NSFileManager implementation), but silent operation is more important for background CLI usage. Files: - src/cmd/trash_cmd.rs:39-52 - Add macOS-specific code path with TrashContext configured to use DeleteMethod::NsFileManager Tests: All 902 tests pass including 4 trash_cmd tests.
Two fixes: 1. exec.rs: Handle empty args in tracking to avoid trailing space - When args is empty, return "binary" not "binary " - When rtk has no subcommand, return "rtk" not "rtk " - Fixed in both production code and test helper 2. tracking.rs: Fix flaky test_custom_db_path_env by using EnvGuard - Tests that modify RTK_DB_PATH now use EnvGuard for serialization - Added RTK_DB_PATH to EnvGuard cleanup list - Prevents race conditions when tests run in parallel All 902 tests pass.
Collaborator
|
Hi @ahundt, this PR has conflicts with master. Could you rebase on current master? thanks, |
Merge upstream/master (5cfaecc) to resolve PR rtk-ai#156 conflicts. Upstream additions integrated: - mypy_cmd: new command with grouped error output (80% reduction) - gain: per-project token savings with -p flag - git: exit code propagation for push/pull/fetch/stash/worktree - go_cmd: build-output/build-fail event handling in NDJSON parser - registry: fi/done moved to IGNORED_EXACT (fix find shadowing) - CI: musl static binaries for Linux Conflict resolutions: - hooks/rtk-rewrite.sh: kept upstream shell script for backward compat (users transition gradually to Rust hook engine) - src/discover/registry.rs: merged both sides (branch Route/lookup + upstream mypy pattern/rule + fi/done fix) - src/go_cmd.rs: kept branch streaming architecture, ported upstream build-output/build-fail handling into GoTestStreamFilter Additional integration fixes: - Add mypy to ROUTES table for O(1) hook routing - Add python -m mypy routing in Rust hook engine - Add git worktree to ROUTES Only subcommand list - Update init.rs test to match kept shell hook (not binary shim) Tests: 799 passed, 0 failed
…ch tables pipe_cmd.rs: - Add mypy, ruff-check, ruff-format, prettier to resolve_filter() - Add auto-detect heuristic for mypy output (.py + ": error:" pattern) - Update error message and doc comment with new filter names - Add tests for all new resolve_filter entries and auto-detect cmd/filters.rs: - Add mypy, ruff, golangci-lint to get_filter_type() -> FilterType::Test - Add same to get_filter_mode() -> Buffered(filter_test_output) - Add tests for new filter type and mode entries These were integration gaps where new upstream commands (mypy) and existing branch commands (ruff, prettier, golangci-lint) had dedicated modules but were missing from the pipe filter dispatch and fallback filter tables used by `rtk pipe` and `rtk run -c`.
- Add wc Route entry (binary: wc, subcmds: Any, rtk_cmd: wc) - Add wc PATTERN regex and RULE (category: Files, 40% savings) - Remove "wc " from IGNORED_PREFIXES since wc now has a dedicated module - Add tests: test_classify_wc, test_classify_wc_bare, test_route_wc The wc command was added upstream as a dedicated module (wc_cmd.rs) but was still in the ignored prefixes list, preventing hook rewriting of bare `wc` commands to `rtk wc`.
…e tests Previous behavior: - filters.rs had dead `FilterType` enum, `get_filter_type()`, and `apply_to_string()` that were fully superseded by `get_filter_mode()` - stream.rs used `.filter_map(Result::ok)` which silently skips I/O errors and can spin indefinitely on a broken pipe fd - `get_filter_mode()` was missing entries for npm/npx/pnpm and go - `FORMAT_PRESERVING`/`TRANSPARENT_SINKS` were pub(crate) but only used in tests, triggering dead code warnings - Several bare `matches!()` calls in tests were not wrapped in `assert!()` making them no-ops that never actually validated anything What changed: - src/cmd/filters.rs: remove dead FilterType/get_filter_type/apply_to_string (get_filter_mode is the sole dispatch path via exec.rs:148). Add npm/npx/ pnpm streaming ANSI-strip entry. Move truncate_lines into #[cfg(test)]. Wrap all bare matches!() in assert!(). Add 6 edge case tests: go passthrough, npx streaming, npm ANSI strip, empty test output, pure Compiling output, test separator lines. - src/stream.rs: change all 5 .filter_map(Result::ok) to .map_while(Result::ok) on BufReader::lines() — stops iteration on first I/O error instead of spinning. Safe because pipes from std::process::Child are blocking fds where EINTR is retried by std::io internally and EAGAIN cannot occur. - src/cmd/hook.rs: move FORMAT_PRESERVING/TRANSPARENT_SINKS behind #[cfg(test)]. Merge assert_blocked helpers. Split depth limit test. - src/cmd/exec.rs: wire predicates::is_interactive() and has_unstaged_changes() into verbose>1 debug logging (eliminates dead code). - src/cmd/predicates.rs: strengthen test assertions (was `let _ =`, now actually verifies behavior). - src/gain.rs: split get_summary/get_recent dispatch to use unfiltered variants when no project scope. - src/tracking.rs: add CommandStats type alias for readability. - src/init.rs: fix clippy warnings (repeat_n, remove unused vars). - 13 upstream files: mechanical clippy auto-fixes (map_or→is_some_and, last→next_back, collapsed else-if, derived Default, etc.) Why: branch-specific code accumulated clippy warnings after merge with upstream/master. The filter_map→map_while fix addresses a real bug where repeated I/O errors could cause infinite loops. Dead code removal reduces maintenance surface. Edge case tests guard against regressions like the go-passthrough bug caught during review. Files affected: - src/cmd/filters.rs: dead code removal, npm/pnpm entry, 6 new tests - src/stream.rs: filter_map→map_while at 5 sites - src/cmd/hook.rs: cfg(test) constants, test helper merge - src/cmd/exec.rs: verbose debug logging with predicates - src/cmd/predicates.rs: stronger test assertions - src/gain.rs: summary/recent dispatch split - src/tracking.rs: CommandStats type alias - src/init.rs: clippy fixes (repeat_n, unused vars) - 13 upstream files: mechanical clippy auto-fixes Testable: cargo test --all (814 passed, 0 failed)
… backup registry
Same fix as feat/multi-platform-hooks: makes RTK the sole Bash hook responder
by patching plugin caches, then calling registered handlers via manifest fallthrough.
Root cause: parallel Bash hooks from settings.json (RTK) and plugin cache (autorun)
caused Claude Code to drop updatedInput from both. RTK rewrites were silently lost.
What changed (identical to multi-platform-hooks branch except no Gemini hook):
src/init.rs:
- Add `patch_plugin_caches()`: scans ~/.claude/plugins/cache/*/*/hooks/*.json,
removes "Bash" from PreToolUse matchers, resolves ${CLAUDE_PLUGIN_ROOT},
writes ~/.claude/hooks/rtk-bash-manifest.json
- Add `patch_single_cache_file()`: idempotent; guards against empty-matcher result
- Add `restore_plugin_caches_from_manifest()`: uninstall restores original matchers;
skips atomic_write when patched matcher not found (no unnecessary file touches)
- Add `backup_file_once()`: backs up to <file>.rtk-backup; never overwrites existing;
registers path in ~/.claude/hooks/rtk-backups.json via append_to_backup_registry()
- Add `append_to_backup_registry()` / `read_backup_registry()` / `print_backup_registry()`:
persistent, deduplicated backup registry; printed at end of install and uninstall
- Replace `.json.bak` fatal copy with `let _ = backup_file_once()`
- `insert_hook_entry()`: changed from `-> ()` with `.expect()` panics to `-> Result<()>`
with warn-and-overwrite guards for malformed "hooks"/"PreToolUse" fields
- `BashManifest`: `impl Default` (version=1), collapse init block to `.unwrap_or_default()`
- `map_or(false, |v| ...)` → `is_some_and(|v| ...)` (idiomatic/clippy)
- `to_string_lossy().to_string()` → `.into_owned()` (communicates ownership intent)
- `patch_plugin_caches()` called from `run_default_mode` and `run_hook_only_mode`
- `uninstall()`: restores plugin caches, removes manifest and Part 1 wrapper,
prints preserved backup paths from registry
src/cmd/claude_hook.rs:
- `run()` reads stdin once; passes buffer to `run_inner(buffer: &str)`
- Add `run_manifest_fallthrough(payload)`: called on NoOpinion only; spawns each
fallthrough_command via `sh -c`; gates exit(2) on write_ok; inherits stdout/stderr
- Invariant comment: only called on NoOpinion, never on Allow/Deny paths
Testable:
- `rtk init -g` → "Plugin caches: N patched", backup paths listed
- Re-run → "already up-to-date (re-run safe)", same backup list
- New Claude Code session: `git status` → rewrites to `rtk git status`
- `rtk init -g --uninstall` → restored caches + preserved backups shown
Before: `cargo build &` classified entirely as Shellism → shell passthrough with 0% token savings. The `&` was never stripped as a suffix. What changed: - `split_safe_suffix()`: add 1-token `&` pattern with Shellism-in-core guard - `cargo build &` → core=[cargo,build], suffix="&" → `rtk cargo build &` (savings) - `cargo build 2>&1 &` → core has Shellism from `>&1` → guard fires → no strip → shell handles - Doc comment: add `&` to the recognized patterns list with caveat - 4 new regression tests: - `test_background_job_suffix_simple`: verifies stripping and clean core - `test_background_job_suffix_git_status`: RTK-routed command with trailing & - `test_background_job_suffix_blocked_by_fd_redirect_shellism`: `2>&1 &` guard - `test_background_job_suffix_single_token_not_stripped`: bare `&` edge case (n<2 guard) Why: The guard is critical. `cargo build 2>&1` already has `&` as Shellism in the `2>&1` tokens. Without the guard, stripping the trailing `&` would leave the `2>&1` Shellism in the core causing a double-passthrough instead of the simpler correct behavior (shell handles the whole command).
Move protocol-specific modules into src/cmd/hook/ to group LLM adapters together and mirror the structure of PR rtk-ai#150's src/hook/ directory: - hook.rs → hook/mod.rs (dispatch engine becomes module root) - claude_hook.rs → hook/claude.rs (protocol adapter, no _hook suffix) - trash_cmd.rs → trash.rs (no _cmd suffix, consistent with builtins.rs) Update module declarations (cmd/mod.rs), dispatch (main.rs:1523), and internal import path in hook/claude.rs (super::hook:: → super::). 818 tests pass; pre-existing warnings unchanged.
…ation (Part 4) Add tests and fixes identified during PR comparison review with rtk-ai#150 and rtk-ai#241: analysis.rs: - test_cargo_test_pipe_grep_is_not_safe_suffix: regression guard for pipe-to-grep routing — verifies "cargo test | grep FAILED" is not classified as a safe suffix and triggers shell passthrough (pipe target is format-sensitive) - test_nohup_background_strips_ampersand: edge case — "nohup cargo build &" strips trailing & as safe suffix; core [nohup, cargo, build] does not require shell builtins.rs: - is_valid_env_name(): POSIX shell identifier validation [A-Za-z_][A-Za-z0-9_]* - builtin_export(): rejects invalid identifiers (e.g. "123=x") with fail-open behavior (silent skip, never error — preserves RTK's fail-open principle) - tests: test_export_invalid_identifier_ignored, test_export_empty_name_ignored, test_is_valid_env_name (7 assertions covering valid/invalid identifier patterns) hook/mod.rs: - test_cat_multi_file_rewrites_to_rtk_read: documents that on this branch (without data-safety rules), cat → rtk read for all arities via defensive fallback in route_native_command(); contrasts with feat/multi-platform-hooks where cat is Blocked by src/rules/rtk.safety.block-cat.md - test_cat_single_file_rewrites_to_rtk_read: same fallback path, single-file case All 825 tests pass.
… path resolution (v2)
Same fixes as main branch but for feat/rust-hooks-v2 which uses rtk-rewrite.sh
as the primary hook (phased-transition design per reviewer).
RC2 (binary hook Allow arm): Added run_manifest_handlers() replacing
run_manifest_fallthrough() in src/cmd/hook/claude.rs. Both NoOpinion AND
Allow arms now call all manifest handlers. Deny from any handler wins over
RTK rewrite. Added is_json_deny() with CC + Gemini dual-format detection.
RC3 (manifest gap): Fixed patch_single_cache_file() reconstruction path for
when Bash was already removed from plugin PreToolUse matchers.
Plugin path resolution: Fixed resolve_plugin_root_in_command() with 3-level
fallback (plugin_name -> vendor_name -> scan for hooks/ dir) to handle the
ar/autorun naming mismatch in cache vs source directory.
RC1 (shell hook): Added parallel-merge coordinator logic to hooks/rtk-rewrite.sh
with BEGIN/END_RTK_BASH_HANDLERS markers for rtk init to populate handlers.
init.rs fixes for v2:
- Fixed patch_settings_json() to use script path instead of hardcoded binary
- Added extract_handler_section() + merge_hook_with_handlers() for upgrade safety
- ensure_hook_installed() now preserves registered handlers across script upgrades
--hook-type flag: Added HookType{Binary,Script} with default Script for v2.
Add check_environment() and report_env_issues() to src/init.rs, called at the start of run_default_mode() and run_hook_only_mode() (Unix paths) before any files are modified. Same checks as feat/multi-platform-hooks: - $HOME set and ~/.claude/ exists (Hard) - ~/.claude/settings.json exists (Soft) - jq on PATH for --hook-type script (Hard; default in this branch) - rtk on PATH (Hard) On hard failures: prints ❌ SETUP REQUIRED with numbered steps and links (docs.anthropic.com/en/docs/claude-code/hooks, jqlang.org/download/), then bails. Soft warnings continue. Includes tip to paste output into AI.
Same fixes as feat/multi-platform-hooks: Issue 1 — wrong rtk binary (name collision) not detected: Replace `command -v rtk` with `rtk hook --help` probe. If rtk is on PATH but hook subcommand fails, report "wrong package installed". Issue 2 — spurious settings.json soft warning on new installs: Remove settings.json check; patch_settings_shared creates it if absent. Issue 3 — stale docs.anthropic.com links: Update to code.claude.com/docs/en/* (current canonical domain). Issue 4 — jq PATH false positive on Homebrew + .zshrc-only PATH: Add note to check ~/.zprofile vs .zshrc for PATH exports. Issue 5 — PATH instructions pointed at wrong shell profile: Use ~/.zprofile (macOS) / ~/.profile (Linux) in rtk-not-found message.
Previous behavior: check_environment() hardcoded ~/.zprofile (macOS) /
~/.profile (Linux) in two places — jq PATH hint and rtk-not-found PATH
setup. This gave wrong instructions to fish, nushell, and other users.
Also used `sh -c "command -v rtk"` for rtk on-PATH probe.
What changed (src/init.rs):
- Added path_setup_instructions(cargo_bin: &str) -> Vec<String>: reads
$SHELL, dispatches to zsh (.zprofile), bash (.bash_profile), fish
(fish_add_path), nushell (env.nu), and generic POSIX fallback.
- Added jq_path_profile_hint() -> String: same $SHELL dispatch for
jq-not-detected advisory (replaces hardcoded .zprofile/.zshrc/.bashrc).
- check_environment() jq block: replaced hardcoded profile strings with
jq_path_profile_hint(); added instrs.retain() to drop empty strings.
- check_environment() rtk-not-found block: replaced hardcoded export /
reload advice with path_setup_instructions(&cargo_bin).
- rtk on-PATH probe: Command::new("which").arg("rtk") replaces
sh -c "command -v rtk" (avoids extra shell subprocess).
…helpers
Previous behavior: Each filter module had its own local which_command() that
called Command::new("which") directly — broken on Windows where the command is
"where", not "which". hook_audit_cmd.rs used HOME→"/tmp" fallback (no /tmp on
Windows). claude.rs manifest_path() used HOME without USERPROFILE fallback.
What changed:
- src/utils.rs: added two public helpers command_in_path(cmd) and
which_command(cmd) that dispatch between "which" (Unix) and "where"
(Windows) via cfg!(windows); which_command() takes only the first line
of `where` output to handle Windows returning all matches
- src/utils.rs: package_manager_exec() now uses command_in_path() instead
of inline Command::new("which")
- src/next_cmd.rs, tsc_cmd.rs, prisma_cmd.rs, ccusage.rs, tree.rs: each
replaced inline Command::new("which") with crate::utils::command_in_path()
- src/pytest_cmd.rs, pip_cmd.rs, mypy_cmd.rs: each replaced local 8-12 line
which_command() duplicate with single-line crate::utils::which_command()
delegation
- src/hook_audit_cmd.rs: replaced HOME→"/tmp" fallback with
dirs::data_local_dir() (→ %APPDATA% on Windows, ~/.local/share on Linux)
with temp_dir() as last resort — no /tmp hardcoding
- src/cmd/hook/claude.rs: manifest_path() adds USERPROFILE fallback when
HOME is not set (Windows standard home env var)
Why: RTK has cross-platform CI (macOS, Linux x86_64/ARM64, Windows) but
several modules silently broke on Windows due to "which" not existing there.
Consolidating into utils.rs helpers ensures every future module gets
cross-platform path probing for free. Note: mypy_cmd.rs is v2-only.
Files affected:
- src/utils.rs: +38/-5 (new helpers + updated package_manager_exec)
- src/next_cmd.rs, tsc_cmd.rs, prisma_cmd.rs, ccusage.rs, tree.rs: -4 each
- src/pytest_cmd.rs, pip_cmd.rs: -8 each (remove duplicate which_command)
- src/mypy_cmd.rs: -8 (remove duplicate which_command, v2-only file)
- src/hook_audit_cmd.rs: +6/-2 (dirs::data_local_dir instead of HOME+/tmp)
- src/cmd/hook/claude.rs: +4/-1 (USERPROFILE fallback in manifest_path)
Collaborator
|
Hi @ahundt! PRs #156 and #158 are showing as CONFLICTING after recent merges. Before we can review the full series (#156, #157, #158), they need to be rebased. Also, given the scope (Hook Engine + Data Safety Rules + Gemini = ~28K lines across 100+ files), could we discuss the rollout strategy in an issue first? We want to make sure we integrate this thoughtfully rather than all at once. |
Conflict resolutions (6 files): - Cargo.lock: theirs (new sha2, reqwest, reqwest-blocking crates for integrity+telemetry) - src/go_cmd.rs: theirs (wording update to build_go_test_summary) + pub visibility restored for filter_go_build, filter_go_test_json (called by pipe_cmd.rs) - src/main.rs: additive — keep Hook/Run/Pipe/HookCommands from v2 + add upstream Rewrite/Verify variants and match arms alongside existing variants - src/discover/registry.rs: theirs (has rewrite_command:252, rewrite_segment:429, rewrite_compound:279, rewrite_head_numeric:444, ENV_PREFIX:50, classify_command:54) + added wc tests (test_classify_wc, test_classify_wc_bare, test_route_wc) - src/init.rs: additive — keep v2's 4 test functions (test_extract_handler_section_*, test_merge_hook_with_handlers_*) + add upstream's guard-ordering assertion (jq_pos < rtk_delegate_pos) inside test_hook_has_guards - hooks/rtk-rewrite.sh: delete bash rewrite engine (lines 60-223, superseded by `rtk rewrite "$CMD"`); keep parallel-merge coordinator (lines 225-287) atop upstream thin delegator + no-change guard (`if [ "$CMD" = "$REWRITTEN" ]`) Post-merge compile fixes: - src/cmd/hook/mod.rs: replace registry::lookup() (removed in upstream) with hook_lookup() — conservative subcommand whitelist matching v2 test expectations. classify_command() (discover::registry) is for history analysis and routes too broadly (find, tree, wget, docker run/exec/build excluded by hook tests). Added wc, playwright, prisma, curl, pytest to hook_lookup. Lifetime annotation: hook_lookup<'a>(binary: &'a str, sub: &str) - src/discover/rules.rs: port wc RtkRule + pattern from v2 inline registry; remove "wc " from IGNORED_PREFIXES; PATTERNS[32] = r"^wc(\s|$)" Architecture note (preserved from v2, no regressions): - check_for_hook (binary hook, hook/mod.rs:61): uses Rust lexer + suffix-aware routing (split_safe_suffix) + special cases (vitest run injection, uv pip, python -m pytest). Routes via hook_lookup() whitelist. - rtk rewrite (script hook, rewrite_cmd.rs→registry::rewrite_command:252): uses simpler regex-based compound rewriting. Script hook routes via this. - Both share RULES table: binary via hook_lookup→RULES, script via rewrite_command→RULES. - route_native_command already called registry::lookup() in v2; now hook_lookup() replaces that with an equivalent conservative whitelist. - run_manifest_handlers (claude.rs:375): unchanged — deny wins over rewrite for both NoOpinion and Allow arms. Verified: 1002 tests passing (was 841 in v2 pre-merge), 0 failures. rtk rewrite "cargo test" → "rtk cargo test" ✓ rtk hook claude vitest → "rtk vitest run" (no regression) ✓ rtk hook claude "wc -l src/main.rs" → "rtk wc -l src/main.rs" ✓ shell hook: cargo test → updatedInput.command="rtk cargo test" ✓
- src/main.rs: remove stray blank line (rustfmt) - src/init.rs: reformat long lines to rustfmt style (semantic no-op) - Cargo.lock: add which/env_home/either/winsafe transitive deps for the which crate added in 83bbfd1 (which_command helper)
rtk git commit only accepted -m/--message; any other flag (-F <file>, --amend, --no-edit, -a, --no-verify, --allow-empty) caused a Clap parse error. Add extra_args: Vec<String> with trailing_var_arg + allow_hyphen_values to GitCommands::Commit and GitCommand::Commit. build_commit_command appends extra_args after the -m chain. run_commit includes them in the logged original_cmd string. Adds 6 tests: -F /tmp/msg.txt, --amend --no-edit, -m msg --amend, plus unit tests for build_commit_command with each new flag.
Conflict resolution (1 conflict, additive):
- src/discover/registry.rs: keep all tests from both sides
- ours: test_classify_wc, test_classify_wc_bare, test_route_wc
- upstream: test_rewrite_gh_json_skipped, test_rewrite_gh_jq_skipped,
test_rewrite_gh_template_skipped, test_rewrite_gh_api_json_skipped,
test_rewrite_gh_without_json_still_works (rtk-ai#196)
Upstream v0.27.0 changes absorbed:
- fix(registry): RTK_DISABLED=1 env prefix skips rewrite entirely (rtk-ai#345)
- fix(registry): gh --json/--jq/--template skips rewrite to avoid
corrupting structured output (rtk-ai#196)
- fix: RTK_DISABLED ignored, 2>&1 broken, json TOML error (rtk-ai#345,rtk-ai#346,rtk-ai#347)
- docs: version refs, module count, CHANGELOG, ARCHITECTURE
Also fixed pre-existing test race exposed by new parallel tests:
- fix(test): add EnvGuard to test_shared_is_hook_disabled_* in claude.rs
to prevent RTK_ACTIVE race with test_raii_guard_clears_on_panic
(both tests set RTK_ACTIVE without holding ENV_LOCK)
Tests: 1029 pass, 0 fail (parallel), 5 ignored
…nged Bug: rtk hook claude was routing gh pr list --json ... to rtk gh pr list --json ... which corrupts structured JSON output. Mirrors upstream fix registry::rewrite_segment rtk-ai#196 to the binary hook path. Fix: should_passthrough() returns true for gh commands containing --json, --jq, or --template flags — hook emits no output (NoOpinion) so Claude Code runs the original gh command unchanged. Tests: 2 new (test_gh_json_flag_passes_through, test_gh_without_json_not_passthrough); 1031 pass total
…at/gemini-support-v2
Conflict resolution (1 conflict, additive):
- src/main.rs Init command: merge --claude/--gemini flags (gemini branch)
with --hook-type flag (rust-hooks-v2); keep all three flags
- Dispatch: multi-platform logic calls init::run(..., hook_type, ...)
for Claude and init::run_gemini() for Gemini; hook_type threads through
Result: rtk init now supports --claude, --gemini, and --hook-type together
rtk init → Claude + Gemini, script hook (default)
rtk init --claude → Claude only
rtk init --gemini → Gemini only
rtk init --hook-type binary → Claude + Gemini with binary hook
Inherits from rust-hooks-v2 (dbc46c7):
- fix(hook): binary hook passes gh --json/--jq/--template through unchanged
(mirrors upstream registry::rewrite_segment fix rtk-ai#196)
- 2 new tests: test_gh_json_flag_passes_through, test_gh_without_json_not_passthrough
Tests: 1050 pass, 0 fail (parallel), 5 ignored
36076a2 to
feb5375
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
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.
PR 131 Part 3: Gemini CLI Hook Support
Branch:
feat/gemini-support-v2| Base:master(stacks on Part 1)Tests: 560 pass | Parallel to: Part 2 (zero file conflicts)
Split from: PR #131
New deps: None
PR: #158
Context
Addresses item 1 from FlorianBruniaux's split request:
FlorianBruniaux said "standalone" but code analysis shows
gemini_hook.rsimports 6 items fromhook.rs(types and functions). This PR depends on PR 1 for the shared hook infrastructure, but is parallel to PR 2 (data safety).Coordination with PR #141: FlorianBruniaux noted overlap with #141's JS-based hook. This PR achieves the same Windows support goal via compiled Rust binary -- no bash, node, or bun. Both PRs wanted cross-platform support; this approach has zero external runtime dependencies.
Merge Sequence
After PR 1 merges: retarget
feat/gemini-support-v2->master, then merge. Can merge before or after PR 2.Summary
Adds Gemini CLI support via
rtk hook geminiandrtk init --gemini. Shares 65% of hook logic with Claude Code -- only the JSON wire format differs.Impact: First multi-platform support. Future platforms (Cursor, Windsurf, etc.) only need a new protocol handler -- 35% code per platform, 65% shared.
rtk hook gemini-- BeforeTool handlerDifferent wire format from Claude:
hook_event_name(must be "BeforeTool") +tool_name(must be "run_shell_command")decision/reason/updated_input(notpermissionDecision/updatedInput)decision: "deny"works (unlike Claude bug #4669)rtk init --gemini-- SetupPatches
~/.gemini/settings.json, creates~/.gemini/GEMINI.mdwith@RTK.mdreference.Manual setup:
Multi-platform init logic
rtk initrtk init --claudertk init --geminirtk init --claude --geminiShared infrastructure extracted from
init.rs:patch_settings_shared()-- JSON patching for both Claude and Geminipatch_instruction_file()-- Adds @RTK.md reference to CLAUDE.md or GEMINI.mdremove_hook_from_settings_file()-- Uninstall hook entryChanges
6 files changed (+1037, -6)
New:
src/cmd/gemini_hook.rs(490 lines, 19 tests)Modified:
src/cmd/mod.rs(+gemini_hook),src/main.rs(+HookCommands::Gemini, +--gemini/--claude init flags),src/init.rs(+run_gemini(), +uninstall_gemini(), +patch_gemini_settings(), extracted shared functions),INSTALL.md(+Gemini setup),README.md(+Gemini integration section)Review Guide
Focus areas:
src/cmd/gemini_hook.rs-- Gemini protocol compliance, guard checkssrc/init.rs-- Multi-platform initialization, shared function extractionNot Included (Future)
Additional protocol handlers for other LLM CLIs (Cursor, Windsurf, Aider) will follow the same 35% code pattern established here.
Test Plan
cargo test-- 560 tests pass (gemini_hook: 19 new)echo '{"hook_event_name":"BeforeTool","tool_name":"run_shell_command","tool_input":{"command":"git status"}}' | cargo run -- hook gemini-- allow with rewriteecho '{"hook_event_name":"BeforeTool","tool_name":"write_file","tool_input":{"path":"test.txt"}}' | cargo run -- hook gemini-- NoOpinion (exit 0, no output)cargo run -- init --gemini --auto-patch-- settings patchedcargo run -- init-- both platforms configuredRelated PRs (Split from PR #131)
Merge order: Part 1 first → retarget Parts 2 & 3 to
master→ merge in any order