feat(refacto-core): binary hook w/ native cmd exec + streaming#956
feat(refacto-core): binary hook w/ native cmd exec + streaming#956
Conversation
Co-Authored-By: ahundt <ATHundt@gmail.com>
|
Removed strip_quote from last release requirements, will be added back for this |
+ remove unused import
|
not ready for review, need fix for a full standardized flow |
What changed:
- Add `run_claude()` with permissions check, audit logging, tool_input
preservation, and Ask/Allow/Deny support
- Add `run_cursor()` with flat JSON format (`permission`/`updated_input`)
- Add `audit_log()` (best-effort append when RTK_HOOK_AUDIT=1)
- Fix `run_gemini()` to load exclude_commands from config
- Convert all hook stdout to `writeln!` with `#[deny(clippy::print_stdout)]`
to prevent JSON protocol corruption (Claude Code bug #4669)
- Replace string-based heredoc detection with lexer-based `has_heredoc()`
(quote-aware: `<<` inside quotes no longer false-positives)
- Add shell prefix peeling (noglob, command, builtin, exec, nocorrect)
to `rewrite_segment()` in registry.rs
- Fix python3 -m pytest pattern, add pip show, add gt (Graphite) to RULES
- Remove `command ` from IGNORED_PREFIXES (was blocking `command git status`)
- Register `rtk hook claude`/`rtk hook cursor` binary commands in
settings.json instead of writing bash script files
- Add legacy script migration (deletes old rtk-rewrite.sh on `rtk init`)
- Simplify hook_check and integrity for script-free model
Integrates ~30 develop commits (PR #997): AWS expansion (8→25 cmds), SSH signing for git commit/push, go test context, grep stdin leak fix, default-to-ask permissions, gh pr merge passthrough. Conflict resolution (4 files): - git.rs: kept .output()+stdin(inherit) for commit/push (SSH/GPG signing) - go_cmd.rs: accepted incoming + added pub(crate) visibility - hook_check.rs: merged binary_hook_registered + other_integration_installed - hook_cmd.rs: fixed permissions path, println→writeln for Gemini deny Verified: 1445 tests pass, 0 clippy errors, all manual integration tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
pszymkowiak
left a comment
There was a problem hiding this comment.
Tested on multipass VM (Ubuntu 24.04, aarch64): 106/110 passed, avg 85% savings. All 15 ecosystems pass. 1450 unit tests pass. Same 2 pre-existing failures as other PRs (clippy warnings + diff test bug).
P0 — must fix:
1. stream.rs:190-198 — Silent 1MB truncation in Buffered/CaptureOnly modes
Lines beyond RAW_CAP (1 MiB) are silently dropped. cargo test with verbose output or pytest with many tests easily exceeds 1 MiB. The filter receives truncated input and produces wrong output (e.g. missing test summary). Add a warning when the cap triggers:
} else if !capped {
capped = true;
eprintln!("[rtk] warning: output exceeds 1 MiB — filter input truncated");
}2. main.rs:2031 — Commands::Run comment says "with token tracking" but has no tracking
No TimedExecution, no timer.track(). All rtk run commands show 0 in rtk gain. Either add tracking or remove the misleading comment.
P1 — important:
3. stream.rs — Streaming, Buffered, Passthrough variants have zero production callers
22 clippy dead_code warnings from this PR. Either gate with #[cfg(test)] or wire to at least one real command.
4. hook_cmd.rs:288 — run_claude silently swallows JSON parse errors
run_copilot (line 53) logs to stderr: "[rtk hook] Failed to parse JSON input: {e}". run_claude just returns Ok(()). Match the copilot pattern.
5. runner.rs — skip_filter_on_failure stderr ordering changed
Previously stderr was printed after filtering; now it streams live during execution. Confirm this is intentional and update cmds/README.md (lines 65-68).
Fix P0s + P1.4 (one-liner) and it's ready.
…r, signal guard, pipe rewrite
- pipe_cmd: fix panic on multi-byte UTF-8 at 1024 byte boundary (floor_char_boundary in auto_detect_filter)
- pipe_cmd: cap stdin at 10 MiB to prevent OOM (reuses RAW_CAP)
- stream: hoist RAW_CAP to pub const at module level
- hook_cmd: check deny before get_rewritten in handle_vscode
(matches handle_copilot_cli and run_claude order)
- hook_cmd: escape backslash and pipe in audit log sanitizer
- tsc_cmd: hoist duplicate TSC_ERROR regex to single module-level
lazy_static
+ trigger feat release tag
4 fixes applied (all confirmed introduced by PR #956, all tests pass): - P0 NEW-passthrough — pipe_cmd.rs: passthrough before cap read - P1 BUFFERED-panic — stream.rs: catch_unwind on Buffered filter - P1 STREAM-postcap — stream.rs: stop feeding filter after cap - P2 OFFBYONE-rawcap — stream.rs: 5 cap boundary checks fixed 5 findings dropped (not introduced by PR or not bugs): - DENY-claude: pre-existing on master - AUDIT-asymmetry: intentional scope choice, not a bug - GEMINI-test: pre-existing test pattern from master - SAVINGS-threshold: 40% is correct (filters achieve ~46%) - STDERR-test: cosmetic CI, not correctness
|
@pszymkowiak Merging this anytime soon? |
|
Hey @pszymkowiak I might be impatient but I will appreciate if you review, |
|
@aeppling press the button |
From the idea of #536 to implement with current code structure and standardize with codebase (no duplication/divided flow)
Summary
Binary Hook Engine
rtk hook claude|cursor|copilot|geminihandlers replacing bash scriptsrtk hook check [--agent] <command>dry-run testingrtk initmigrates legacy script hooks to binary automaticallyPermission System
permissionDecision— omitted for Ask/Default so updatedInput works in bypassPermissionsRTK_HOOK_AUDIT=1Streaming & Execution Infrastructure
exec_capture()standardized.output()wrapper →CaptureResult{stdout, stderr, exit_code}— migrated across 22modules
ChildGuardRAII zombie process preventionFilterModeenum: Streaming, Buffered, CaptureOnly, PassthroughStdinModeenum: Inherit, Filter, Nullwriteln!(io::stdout())replacingprintln!rtk pipeCommandresolve_filter(name)auto_detect_filter()from first 1KB of input--filter <name>and--passthroughmodesrtk proxyStreamingDiscovery & Registry
has_heredoc()andstrip_quotes()Git
git branch -aInit System
Test plan
Should fix :
ISSUE #154 : migrate bash hook to Rust subcommand
ISSUE #222 : proxy streaming for long-running commands
ISSUE #530 : strip trailing stderr redirects before pattern matching
ISSUE #532 : env var prefix and cd chaining in rewrite
ISSUE #897 : subprocess memory leak / zombie process prevention (ChildGuard)
Closes #968 : EAGAIN posix_spawn resource exhaustion (binary hooks)
ISSUE #886 : RTK bypasses Claude Code permissions (permission verdict system)
ISSUE #712 : rtk hook subcommands added
ISSUE #918 : env-var-prefixed exclude_commands (partial: -h flag conflict not addressed)
ISSUE #893 : updatedInput in bypassPermissions mode (partial: default/ask fixed, allow edge case remains)
ISSUE #928 : python3 -m pytest/mypy rewrite (partial: heredoc/script.py not covered)
ISSUE #361 : 5 of 6 hook bugs addressed (streaming, compound commands, conservative routing)
ISSUE #295 : uv sync/pip patterns (~5% of requested scope)
BREAKING CHANGE
For claude code user, they'll need to re-install with "rtk init -g", because claude code now use native binary instead of rewrite
Old script supported until June 13th 2026