Skip to content

Hook Engine + Chained Command Rewriting (PR #131 Part 1)#156

Open
ahundt wants to merge 271 commits intortk-ai:masterfrom
ahundt:feat/rust-hooks-v2
Open

Hook Engine + Chained Command Rewriting (PR #131 Part 1)#156
ahundt wants to merge 271 commits intortk-ai:masterfrom
ahundt:feat/rust-hooks-v2

Conversation

@ahundt
Copy link

@ahundt ahundt commented Feb 16, 2026

PR 131 Part 1: Hook Engine + Chained Command Rewriting

Branch: feat/rust-hooks-v2 | Base: master | Tests: 541 pass
Closes: #112 | Split from: PR #131
New dep: which = "7"
PR: #156

TL;DR: PR #156 replaces the current shell script with a fully tested Rust hook engine featuring an integrated lexer that prevents AI timeouts via line-by-line output streaming, increases tokens saved by successfully rewriting compound commands, and stops RTK from breaking standard tools like find and vitest.


Context

FlorianBruniaux requested splitting PR #131 (52 files, 8K+ additions) into separate PRs:

  1. Gemini CLI support — standalone, no deps on the rest
  2. Data safety rules (rm->trash, git reset --hard->stash) + rtk.*.md files
  3. Chained command rewriting (cd && git status) — note: feat(hooks): add cross-platform Node.js/Bun hook for Windows support #141 also implements this
  4. Rust-based hooks — the hook infrastructure changes

This PR combines items 3 + 4 because they are architecturally inseparable: the hook protocol handler calls lexer::tokenize() then analysis::parse_chain() to process chained commands. Separating them would require duplicating the lexer.

Coordination with PR #141: FlorianBruniaux noted overlap with #141's JS-based hook for Windows. This PR achieves Windows support via compiled Rust binary instead -- no bash, node, or bun required. CI/CD already builds Windows binaries. exec.rs uses cfg!(windows) for shell selection.


Merge Sequence

1. This PR -> master (foundation)
2. Retarget PR 2 (data safety) and PR 3 (Gemini) from feat/rust-hooks-v2 -> master
3. PR 2 and PR 3 can merge in any order (zero file conflicts between them)

Summary

Replaces the 204-line bash hook with a native Rust binary that provides quote-aware chained command rewriting. Closes #112 where cd /path && git status only rewrote cd.

Impact: Captures ~12-20M tokens/month in previously-missed optimizations across chained commands.

Why Rust over bash:

  1. Chained commands work (cd && git status rewrites both)
  2. Extensible (data safety rules in Part 2)
  3. Debuggable (rtk hook check shows exact rewrites)
  4. Multi-platform (Windows support, no JS dependencies)
  5. Backward compatible (legacy .sh becomes 4-line shim)

rtk hook claude -- Claude Code PreToolUse handler

Reads JSON from stdin, applies rewriting, outputs JSON to stdout. Fail-open: malformed input exits 0 with no output so Claude proceeds unchanged.

stdin:  {"tool_input":{"command":"git status"}}
stdout: {"hookSpecificOutput":{"permissionDecision":"allow","updatedInput":{"command":"rtk run -c 'git status'"}}}

Chained command rewriting (closes #112)

Before: cd /tmp && git status -- hook only saw cd, missed git status
After: lexer splits on &&/||/; respecting quotes, each command wrapped independently

git commit -m "Fix && Bug" is NOT split (quote-aware).

rtk run -c <command> -- Command executor

Parses chains, detects shellisms (globs/pipes/subshells -> passthrough to sh/cmd), handles builtins (cd/export/pwd), applies output filters, prevents recursion via RTK_ACTIVE env guard.

rtk hook check -- Debugger

rtk hook check "cd /tmp && git status"
# Output: rtk run -c 'cd /tmp' && rtk run -c 'git status'

Changes

16 files changed (+2969, -221)

New (src/cmd/): mod.rs, hook.rs, claude_hook.rs, lexer.rs, analysis.rs, builtins.rs, exec.rs, filters.rs, predicates.rs, test_helpers.rs

Modified: src/main.rs (+Commands::Run, +Commands::Hook), src/init.rs (register binary hook), hooks/rtk-rewrite.sh (204-line script -> 4-line shim), Cargo.toml (+which), INSTALL.md (+Windows section)

Intentionally excluded (stacked PRs):

  • Safety rules -> PR 2 (feat/data-safety-rules-v2)
  • Gemini support -> PR 3 (feat/gemini-support-v2)

Review Guide

Focus areas:

  1. src/cmd/lexer.rs + analysis.rs -- Chain parsing correctness (quote handling)
  2. src/cmd/claude_hook.rs -- Protocol compliance, fail-open design
  3. src/cmd/exec.rs -- Builtin handling, Windows shell selection (cfg!(windows))
  4. src/cmd/hook.rs -- Shared decision logic (used by Parts 2 and 3)

Implementation Notes

Binary size: Compiled with LTO + stripping. Size increase from which dependency minimal (<0.1 MB). Full size impact measurable after all 3 parts merge (PR #131 reported 5.1 MB total, +0.3 MB from combined deps).

Backward compatible: All existing RTK features work unchanged. Legacy bash hook becomes 4-line shim forwarding to rtk hook claude.


Test Plan

  • cargo test -- 541 tests pass (hook:22, claude_hook:18, lexer:28, analysis:10, builtins:8, exec:22, filters:5, predicates:4)
  • echo '{"tool_input":{"command":"git status"}}' | cargo run -- hook claude -- JSON rewrite works
  • echo '{"tool_input":{"command":"cd /tmp && git status"}}' | cargo run -- hook claude -- chain split works
  • cargo run -- hook check "git status" -- text debugger works
  • cargo run -- run -c "echo hello" -- executor works
  • grep 'cfg!(windows)' src/cmd/exec.rs -- Windows shell selection present

Related PRs (Split from PR #131)

Part PR Description
1 #156 Hook Engine + Chained Commands (this PR)
2 #157 Data Safety Rules
3 #158 Gemini CLI Support

Merge order: Part 1 first → retarget Parts 2 & 3 to master → merge in any order

FlorianBruniaux and others added 30 commits January 29, 2026 16:45
## 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>
…dit-system

feat: Comprehensive Temporal Audit System for Token Savings Analytics
…s--master--components--rtk

chore(master): release 0.4.0
Implement `rtk cc-economics` command combining ccusage spending data
with rtk savings analytics for economic impact reporting.

Features:
- Dual metric system (active vs blended cost-per-token)
- Daily/weekly/monthly granularity with ISO-8601 week alignment
- JSON/CSV export support for data analysis
- Graceful degradation when ccusage unavailable
- Real-time data merge with O(n+m) HashMap performance

Architecture:
- src/ccusage.rs: Isolated ccusage CLI interface (7 tests)
- src/cc_economics.rs: Business logic + display (10 tests)
- src/utils.rs: Shared formatting utilities (8 tests)
- Refactored gain.rs to use shared format_tokens()

Test coverage: 17 new tests, all passing
Validated with real-world data (2 months, $3.4K spent, 1.2M saved)

Addresses: #economics-integration
Impact: 24.4% cost savings identified ($830.91 active pricing)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Apply systematic quality fixes identified in code audit:

Phase 1: Remove dead code (4 warnings → 0)
- Remove unused run_compact() function in gain.rs
- Remove unused track_tokens() function in tracking.rs
- Remove unused TRACKER singleton (Mutex)
- Clean up CommandRecord struct (remove unused fields)

Phase 2: Add error context and quality fixes
- Add .context() to all ? operators in cc_economics.rs (15+ callsites)
- Add .context() to all ? operators in gain.rs (10+ callsites)
- Fix bounds check panic risk in gain.rs:252 (week_start slice)
- Reduce visibility: estimate_tokens pub → private
- Replace 6 manual loops with idiomatic .collect::<Result<Vec<_>, _>>()?
- Remove residual dev comment (// psk:)
- Remove unnecessary .clone() in main.rs

Phase 3: Refactor duplication (132 lines eliminated)
- Create display_helpers.rs module with PeriodStats trait
- Unify print_daily_full/weekly/monthly in gain.rs (152 lines → 9 lines)
- Implement trait for DayStats, WeekStats, MonthStats
- Zero-overhead compile-time dispatch (monomorphization)
- Output bit-identical, all tests passing

Impact:
- Dead code: -20 lines
- Error context: Errors now actionable instead of opaque
- Duplication: -132 lines of pure duplication
- Safety: Bounds check prevents potential panic
- Idiomaticity: .collect() over manual loops

Metrics:
- Build: 0 errors, 1 warning (pre-existing cache_* fields)
- Tests: 79/82 passing (3 pre-existing failures)
- Clippy: 13 warnings (all pre-existing)
- Functional: rtk gain --daily, rtk cc-economics validated

Addresses code audit feedback from parallel session
Detailed report: claudedocs/refactoring-report.md

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Applies targeted code quality improvements:

Issue rtk-ai#5: Reduce clones in HashMap merge
- Destructure CcusagePeriod to avoid cloning key
- Use .or_insert_with_key() for ccusage data merging
- Keep necessary clones for rtk data (HashMap ownership requirement)
- Affects: merge_daily, merge_weekly, merge_monthly

Issue rtk-ai#7: Replace magic number with named constant
- Add const BILLION: f64 = 1e9
- Improves readability in cache token calculation

Issue rtk-ai#8: Add NaN/Infinity safety check
- Guard format_usd() against invalid float values
- Return "$0.00" for non-finite inputs

Build: Clean (1 pre-existing warning)
Tests: 79/82 passing (3 pre-existing failures)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…dit-system

feat: comprehensive economics analysis and code quality improvements
…s--master--components--rtk

chore(master): release 0.5.0
1. CRITICAL: Fix 'latest' tag creation after releases
   - Move update-latest-tag job from release.yml to release-please.yml
   - release-please creates tags via API (no push event) → must run in same workflow
   - Job now conditional on release_created output

2. IMPORTANT: Add npx fallback for ccusage + improve message
   - Check binary in PATH first, fallback to 'npx ccusage'
   - Updated message: "npm i -g ccusage (or use npx ccusage)"
   - Consistent with other JS tooling (next_cmd, tsc_cmd, prettier_cmd)

3. PROCESS: Slow down version bumps with release-please config
   - Add release-please-config.json with bump-patch-for-minor-pre-major
   - In 0.x versions: feat: → patch bump instead of minor
   - Prevents rapid version inflation (0.3.1 → 0.5.0 in 21h)

Fixes issues raised by Patrick after PR rtk-ai#21 merge.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
fix: 3 issues (latest tag, ccusage fallback, versioning)
…s--master--components--rtk

chore(master): release 0.5.1
Patrick reported 2 critical issues post-merge PR rtk-ai#23:

1. **release.yml never triggers**: release-please creates tags via GitHub API
   → no push event generated → build workflow never runs
   → v0.5.1 released without binaries

2. **README install URLs 404**: DEB/RPM URLs hardcode version 0.3.1
   → /releases/latest/download/ serves different filenames
   → all releases > 0.3.1 break installation instructions

Root cause analysis:
- release-please creates GitHub Releases (triggers `release.published` event)
- release.yml only listens to `on: push: tags` (doesn't fire for API-created tags)
- Standard pattern: release-please + binary builds = `on: release.published`

Fixes:
1. release.yml trigger:
   - Add `on: release: types: [published]` (standard release-please pattern)
   - Remove `on: push: tags: ['v*']` (dead code with release-please)
   - Update version extraction to handle `release` event
   - Split release job: upload assets (release event) vs create release (workflow_dispatch)

2. Version-agnostic package naming:
   - Create copies: rtk_0.5.0-1_amd64.deb → rtk_amd64.deb
   - Create copies: rtk-0.5.0-1.x86_64.rpm → rtk.x86_64.rpm
   - Update README URLs to use version-agnostic names
   - /releases/latest/download/ now serves stable filenames

Impact:
- release-please releases now auto-trigger binary builds
- Installation URLs work for all future releases
- No manual workflow_dispatch needed for new versions

Manual action required:
Patrick needs to re-run build for v0.5.1 via workflow_dispatch
(or create v0.5.2 with a trivial fix commit)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
fix: release pipeline trigger and version-agnostic package URLs
ahundt added 3 commits March 5, 2026 10:31
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)
@FlorianBruniaux
Copy link
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.

ahundt added 5 commits March 5, 2026 17:26
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
@ahundt
Copy link
Author

ahundt commented Mar 6, 2026

I updated it again, but new changes keep being merged every time i update it to resolve all the conflicts. Also there is far less code than it seems because there is a massive number of unit tests, check out #361 for details and the discussion as @FlorianBruniaux requested!

ahundt added 7 commits March 6, 2026 03:00
Resolves stale merge-base from previous v0.27.0 merge (commit 598b5c7
lost its merge parent due to stash/pop clearing MERGE_HEAD).

Conflict resolution (registry.rs only):
- Keep wc tests (ours, not in upstream)
- Accept upstream cargo fmt style for gh --json test

1031 tests pass, 0 failures.
Clap with `trailing_var_arg=true` consumes the `--` separator token,
so `rtk cargo test -- --test-threads=1` would pass `--test-threads=1`
to cargo without the preceding `--`, causing cargo to reject it as
an unexpected argument. Same issue affected clippy (`-- -W ...`).

Add `split_at_double_dash()` pure function that detects `--` in raw
process args and computes the split point in Clap-parsed args.
Wire via `build_cargo_args()` into `run_test()` and `run_cargo_filtered()`.

6 new unit tests, 1037 total pass.
…efore routing

zsh builtins like `noglob`, `command`, `builtin`, `exec`, `nocorrect`
modify execution of the NEXT command but are not standalone executables.
The hook was wrapping them inside `rtk run -c 'noglob ...'` which fails
because rtk cannot invoke shell builtins.

Fix: detect shell prefix builtins in route_native_command(), strip the
prefix, route the real command through the normal RTK routing table,
then re-prepend the prefix. Same pattern as env prefix stripping.

Example: `noglob gh release create v0.3.0-rc1 --title "..."` now
correctly becomes `noglob rtk gh release create v0.3.0-rc1 --title "..."`
instead of `rtk run -c 'noglob gh release create v0.3.0-rc1 ...'`.

5 TDD tests added covering noglob, command, builtin, nocorrect prefixes
with both known RTK commands and unknown commands.
Add 6 new TDD tests covering edge cases for shell prefix builtin
handling (noglob, command, exec, nocorrect, builtin):

- Exact bug report regression test: noglob gh release create
- Nested prefixes: noglob command git status
- Shell prefix + env prefix combo: noglob GIT_PAGER=cat git log
- exec prefix routing: exec git status
- Bare prefix passthrough: noglob (no following command)

All tests verify the recursive stripping design handles multiple
layers correctly via route_native_command recursion.

1052 tests pass, 0 failures.
Unknown single commands (e.g. `gh release create v0.3.0`) were wrapped
in `rtk run -c '<cmd>'` which added an unnecessary shell layer causing
zsh NOMATCH/globbing bugs for zero token savings.

Changes:
- check_for_hook_inner: use try_route_native_command for single cmds;
  return raw unchanged when None (unknown command)
- route_native_command: shell prefix and env prefix paths now use
  try_route_native_command; unknown inner cmds pass through with
  prefix intact
- hook_lookup: extract basename from full-path binaries so
  /opt/homebrew/bin/git matches "git"
- Err(_) parse fallback: pass through unchanged instead of wrapping

8 new TDD tests + 10 existing tests updated to reflect correct behavior.
1055 tests pass, 0 failures.
Incorporates 52 upstream commits (v0.27.0 → v0.28.2):
- TOML filter DSL engine + 30 built-in filters (PRs rtk-ai#349, rtk-ai#351, rtk-ai#386)
- Graphite CLI support (PR rtk-ai#290)
- git commit -am/--amend fix via trailing_var_arg (PR rtk-ai#327)
- restore_double_dash for cargo (PR rtk-ai#326)
- gh -R/--repo passthrough, pr edit/comment fix (PRs rtk-ai#328, rtk-ai#332)
- docker compose subcommand filtering (PR rtk-ai#336)
- Telemetry tokens_saved + install_method (PRs rtk-ai#462, rtk-ai#469, rtk-ai#471)
- proxy streaming (PR rtk-ai#268)
- Diff limits increased (100→500 lines, 10→30 hunk lines)

Conflict resolution (5 files):
- cargo_cmd.rs: adopted upstream restore_double_dash, adapted streaming
  run_test() to use it, converted old split_at_double_dash tests
- git.rs: adopted upstream simplified Commit unit variant (fixes -am),
  adapted all commit tests to flat args API, added 6 new edge case tests
- init.rs: added TOML template generation alongside hook manifest
- main.rs: merged both upstream (gt, toml_filter, verify) and hooks-v2
  (cmd, hook, stream, pipe) modules, kept all tests from both sides
- utils.rs: kept hooks-v2 command_in_path/which_command + upstream English docs

Hook engine additions during merge:
- Added gt to hook_lookup() whitelist with 4 routing test cases

All 5 hook bug fixes from issue rtk-ai#361 preserved:
1. Streaming (stream.rs BufReader)
2. Handler coordination (parallel-merge + run_manifest_handlers on both paths)
3. Stderr deny (exit 2)
4. Routing whitelist (hook_lookup)
5. Vitest run injection

1182 tests pass (1 environment-dependent upstream test excluded).
test_git_status_not_a_repo_exits_nonzero relied on temp dir having no
parent .git directory. On machines where temp dir is inside a git repo
(or has ancestor .git), git status succeeds unexpectedly.

Fix: set GIT_DIR to a nonexistent path, forcing git to fail regardless
of parent directory structure. Same fix applied to multi-platform branch.
@ahundt
Copy link
Author

ahundt commented Mar 11, 2026

updated again, ready for review! PR #156 is independent of the other two and should be considered on its own, then the other two can be updated if they are desired as they are much smaller changes on top of 156.

ahundt and others added 10 commits March 12, 2026 18:07
1. grep_cmd.rs: restore 3 lost behaviors from develop
   - Exit code propagation (was always 0, breaks CI pipelines)
   - BRE \| → | translation for rg (grep users expect this)
   - -r/--recursive stripping (rg interprets -r as --replace)
   - Add 3 tests from develop: BRE translation, recursive stripping, rg -n

2. hooks/rtk-rewrite.sh: fix exit code collection
   - Remove `|| true` from `wait "$pid"` so $? captures the
     actual handler exit code instead of always 0

3. src/cmd/trash.rs: remove orphaned file
   - Was commented out in mod.rs, never compiled or wired into main.rs
   - Remove file and dead comment

1312 tests pass (1309 + 3 restored develop tests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…imit parsing)

Resolve 15 merge conflicts:
- src/utils.rs: remove command_in_path()/which_command(), use tool_exists()/resolved_command()
- src/mypy_cmd.rs, src/pip_cmd.rs: remove dead which_command() wrappers
- src/pytest_cmd.rs: keep pub(crate) filter_pytest_output using PyTestStreamFilter
- src/next_cmd.rs, src/tsc_cmd.rs, src/tree.rs, src/prisma_cmd.rs, src/ccusage.rs: command_in_path -> tool_exists
- src/grep_cmd.rs: use resolved_command("rg") in test
- src/cargo_cmd.rs: Command::new("cargo") -> resolved_command("cargo")
- src/git.rs: take upstream's improved limit parsing (parse_user_limit), filter_log_output(3-arg)
- src/main.rs: keep our Run/Pipe/Hook commands, add session_cmd + stream modules, fix Rewrite args
- src/init.rs: merge hook_type + install_opencode params into all mode functions
- src/discover/rules.rs: keep wc rule, add upstream TOML-filtered command rules
- Cargo.toml: keep which = "8" only
- Cargo.lock: regenerated from upstream + our deps

1342 tests pass, 0 failures.
Address PR rtk-ai#536 review comments from KuSh:
- get_summary() delegates to get_summary_filtered(None), so the match
  on project_scope was unnecessary — call get_summary_filtered() directly
- Same simplification for get_recent_filtered()

src/gain.rs: 2 match blocks replaced with direct .as_deref() calls
…based version

Replace the 129-line byte-scanner rewrite_compound() with the 107-line
lexer-based version that uses cmd::lexer::tokenize() for quote-aware
operator detection and byte-offset segment extraction.

Changes:
- Add byte offset tracking to ParsedToken and tokenize() in lexer.rs
- Import tokenize/TokenKind in discover/registry.rs
- Replace rewrite_compound with lexer-based version

Improvements over old byte scanner:
- Handles escape sequences (\", \\)
- Detects $() subshells, backticks, globs as shellisms
- Uses TokenKind::Redirect context for 2>&1 vs && disambiguation
- Supports unicode via chars() iterator
- 22 lines shorter, shares lexer with binary hook path

All existing rewrite_compound + lexer tests pass unchanged.
Squashed cherry-pick from feat/rust-hooks-v2-develop:

- analysis.rs: make split_safe_suffix() iterative for compound suffixes
  like `cargo test 2>&1 | tail -50` (was single-pass, now loops)
- main.rs: add -A/-B/-C/-i/-w as named Clap args for grep command,
  converted to extra_args at dispatch site (no grep_cmd.rs changes)
- hook/mod.rs: add git branch and git worktree to hook_lookup whitelist
  (both have dedicated handlers in git.rs)

Includes 13 TDD tests (6 compound suffix, 3 grep args, 4 hook_lookup).
Remove after_context, before_context, grep_context Clap args and their
dispatch logic. These flags correctly reached rg but RTK's output parser
at grep_cmd.rs:77-88 uses splitn(3, ':') which misparses rg context
lines (dash-separated format) as filenames, producing garbled output.

Keep working -i (ignore-case) and -w (word-regexp) flags which only
affect matching, not output format.

Discovered via post-implementation audit: rtk grep -A 2 "fn run_branch"
src/git.rs showed context lines misidentified as separate files.
filter_branch_output() hardcoded "remotes/origin/" prefix check at
git.rs:1111, causing branches from other remotes (upstream, fork, etc.)
to be dumped verbatim into the local section. On multi-remote repos
this yielded only 2.7% savings instead of expected 40-70%.

Fix: match any "remotes/*/" prefix using strip_prefix + find('/').
Deduplicate across remotes using HashSet before filtering against
local branches.

Add TDD tests: multi-remote correctness (3 remotes, dedup, HEAD skip)
and multi-remote savings assertion (>=40% on 3x9 remote branches).
…ooks

When Claude Code updates a plugin, both old and new version directories
remain on disk. patch_plugin_caches() previously processed ALL versions,
creating manifest entries for each. This caused double-execution: the
active version's hook ran via direct match AND via fallthrough from the
old version's manifest entry.

Fix: collect version directories, sort by semver (descending via
parse_semver()), process only the highest (active) version. Remove
manifest entries for non-active versions of the same plugin using
retain() with plugin_prefix/active_prefix path matching.

Add parse_semver() helper (major.minor.patch tuple comparison).
Add TDD tests: test_parse_semver, test_version_dedup_removes_older_entries,
test_catch_all_not_registered_as_fallthrough.
Cargo.toml: add [profile.dev.package."*"] with debug=0, opt-level=1
to reduce target/ size (8.2GB → 0.73GB measured) and speed up dep builds.

CLAUDE.md: add Debug & Profiling Builds section documenting debugging/
profiling named profiles. Append RTK command reference (rtk-instructions
v2) with full command list organized by workflow category.
Merge 23 upstream commits (v0.29.0→v0.30.0) including:
- trust boundary for project-local TOML filters (trust.rs)
- git log --oneline regression fix (user_format param)
- 6 critical bug fixes (exit codes, unwrap, lazy regex)
- container.rs exit code handling improvements
- log_cmd.rs lazy_static regex refactor

Conflict resolution:
- src/main.rs: kept both our Pipe variant and upstream Trust/Untrust
- src/git.rs: kept our pub(crate) visibility + upstream user_format param
- src/pipe_cmd.rs: added 4th arg to filter_log_output call

Tests: 1370 passed (was 1358), 0 failures, 6 ignored
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid This doesn't seem right P1-critical Bloque des utilisateurs, fix ASAP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hook: chained commands (cd dir && cmd) are never rewritten

10 participants