Preserve pipeline input while rewriting safe shell commands

[ahundt]

Preserve pipeline input while rewriting safe shell commands

Summary

RTK now lexes shell command structure before rewriting it. Filters and other content consumers receive the producer's original output, while display-only tails can still use RTK's compact output. Redirections and shell-block boundaries remain attached to the command that owns them.

This fixes issue #1560, where rewriting the left side of a pipe changed the input seen by downstream filters. The lexer consolidates the boundary and suffix behavior developed in PR #536 and supports long, multi-line shell blocks without treating their internal operators as independent top-level commands.

Before and after

Command	Before	After
git log | grep feat	RTK could replace the producer before grep read it.	The command stays raw, so grep receives the original Git stream.
cargo test | tee /tmp/test.log	The file could contain RTK's compact display instead of the test stream.	The command stays raw, so tee saves the original output.
git log | head -20	Conservative pipe handling could leave a safe display tail unfiltered.	RTK can rewrite the producer because head only consumes a bounded display prefix.
git status > /tmp/status.log	The file-target redirect prevented the supported rewrite path.	RTK preserves the suffix and returns an ask-only rewrite: rtk git status > /tmp/status.log.
git status $(whoami)	Shell substitution could be mistaken for a safe rewrite boundary.	The command stays raw because its execution boundary is opaque.
`if git status; then git log	grep feat; fi`	A text-only split could analyze the nested pipe as an independent top-level command.

Implementation

src/discover/lexer.rs tokenizes quotes, escapes, comments, CRLF/newline boundaries, pipes, redirects, case alternatives, and shell blocks using typed tokens and byte offsets. src/discover/registry.rs applies consumer-aware producer routing. src/discover/suffix.rs preserves the exact trailing output suffix and assigns an ask-only policy to file-target redirects.

The policy path is compatible with the multiline rewrite work in PR #2781: each statement is analyzed independently, and auto-allow requires both an attestable command and a rewrite that does not require confirmation. This also preserves the least-privilege behavior tracked in issue #1155.

Content consumers including grep, rg, tee, xargs, wc, awk, and sed remain raw. Safe display tails are limited to supported stdin-only numeric head and tail forms, plain cat, and equivalent absolute paths. Input redirects, substitutions, heredocs, and opaque grouped commands remain conservative.

Compatibility and scope

The existing string rewrite API remains available. Hook adapters use the policy-returning path when a file redirect requires confirmation. This change adds no parser dependency and does not execute native commands through the lexer.

Verification

• 109 lexer tests cover quoting, escaping, comments, CRLF, pipes, redirects, suffixes, shell blocks, content consumers, safe tails, and conservative fallbacks.
• The full branch suite passes with 2,570 tests passed and 8 ignored.
• cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --all -- --check, and git diff --check pass.

[CLAassistant]

All committers have signed the CLA.

[reneleonhardt]

Thank you so much for your important fix! ❤️

In case you want to update, git rebase master resolved automatically, that warning seems strange to me in case "Changes can be cleanly merged." is shown too 🤷

[aeppling]

Hey @ahundt, thanks for this contribution.

I'm currently working on the rewrite, since models now often use multi-line statements: #2781
It would be nice to ship both in the same release.

If you're available to align with my PR, I can merge mine and you adjust this one afterwards; otherwise I can try to retrieve your fixes into mine. Your call

If you want to align:

1. Keep rewrite_command_with_policy and requires_ask, and wire it into feat(rewrite): rewrite commands bundles #2781's clamp: Allow requires attestable && !requires_ask && verdict == Allow, everything else that rewrites is Ask. Either signal can force ask; both must agree to auto-allow.
2. Apply your logic per statement: rewrite_command now splits multi-line bundles and routes each statement through rewrite_logical_line (registry.rs)
3. Existing tests assert git log -10 | grep feat → rewritten; your consumer-aware rule changes that to raw. Update them as intended behavior.

Before merge

1. rerun test and usage with changes
2. solve any conflicts

[ahundt]

Addressed the multiline-rewrite feedback in the current head (968cf99). The branch now uses src/discover/registry.rs::rewrite_multiline_block with the shared lexer’s quote-aware newline tokens, rewrites independent clean lines, preserves raw pipeline consumers and substitution-bearing statements, and defers unsafe shell blocks. src/hooks/rewrite_cmd.rs keeps the policy path: unattestable rewrites remain Ask-only, and file-target redirects retain requires_ask. I validated the current head with 2634 passed, 8 ignored, cargo fmt --all -- --check, cargo clippy --locked --all-targets --all-features -- -D warnings, git diff --check, and a clean merge-tree check against current upstream/develop. I did not merge the older overlapping #2781 implementation because it conflicts with this newer lexer/policy implementation; the requested behavior is already present in this head and its follow-up commits.

[KuSh]

Round 2 of 3 — DUPLICATE, recommend closing

Claim: Lex shell structure before rewriting so pipeline/redirect content consumers receive the producer's original output (fixes #1560).

Scope: duplicate — superseded by #3128 (with #3343 PipelineSafety and #3704 lexer consolidation). Issue #1560, this PR's entire target, was closed 2026-09-05 as fixed on develop.

Ran: both binaries built from upstream/develop (d0c2985) and this head (9e8d128); rtk rewrite over 28 command shapes — content consumers (grep, rg, tee, xargs, wc, awk, sed, less), safe display tails (head, tail, cat), redirects (>, >>, <, 2>&1), substitutions, heredocs, if/for/case/(...)/{...} blocks, quoting and comment edge cases; plus a live correctness check of develop's rewrite against real git log output.

CI @9e8d1285: 10 green, benchmark failing on find --max 10 (-20%) and golangci-lint (empty output) — both pre-existing and untouched by this diff. Head is 3 weeks stale and conflicts with develop in every file it touches.

Why develop already covers this

Develop fixes #1560 by a different and strictly better mechanism: it leaves the producer raw and rewrites the consumer, instead of leaving the whole pipeline raw.

git log | grep feat
  develop: git log | rtk grep feat      <- git log raw, grep reads the real stream
  this PR: <raw>                        <- correct, but no savings

Verified the survivor is genuinely correct, not just smaller: git log -50 | grep feat and git log -50 | rtk grep feat both yield 17 matching lines, so grep is reading the unmodified Git stream and rtk grep only compacts the display.

Every consumer this PR names is already handled on develop — tee, xargs, wc, awk, sed, less stay fully raw on both; head, tail, cat are rewritten as safe display tails on both.

The residual delta is a regression

Because this PR drops the whole statement to raw where develop rewrites the consumer, it loses savings that develop keeps:

command	develop	this PR
git diff HEAD~30 | grep '^+'	1,485,061 → 8,483 B (99%)	0%
git log 2>&1 | grep feat	git log 2>&1 | rtk grep feat	raw
git log|grep feat	git log| rtk grep feat	raw
git status && git log | grep feat	both stages rewritten	tail left raw
if git status; then git log | grep feat; fi	rewritten, producer raw	raw

Against the 20% floor (CONTRIBUTING.md), these go from 99% to 0% on a very common shape.

The one novel piece is unsafe

The file-redirect rewrite is the only behavior here develop lacks, and it reintroduces the content-sink hazard this PR exists to close:

git status > /tmp/status.log   ->   rtk git status > /tmp/status.log

The user's file then holds RTK's compact rendering (57 B) instead of the raw output (123 B) — a content consumer being fed display output, the same class as #1560, just a file sink instead of a pipe. requires_ask narrows the blast radius but does not make it correct; develop's refusal to rewrite is the safe behavior. >> behaves the same.

Relatedly, the PR body states "Input redirects … remain conservative", but git log < /tmp/in is rewritten to rtk git log < /tmp/in on this head, while develop leaves it raw.

Your questions

• aeppling: "If you're available to align with my PR, I can merge mine and you adjust this one afterwards; otherwise I can try to retrieve your fixes into mine. Your call" → Overtaken by events. feat(rewrite): rewrite commands bundles #2781 is still open, but the pipeline-safety half of this PR landed separately via fix(rewrite): only rewrite safe final pipeline stages #3128/feat(rewrite): rewrite in pipe when consummer is safe  #3343/refactor(lexer): consolidate raw-command lexing behind shared primitives #3704 while this branch sat. Nothing here needs retrieving into feat(rewrite): rewrite commands bundles #2781.
• ahundt: "I did not merge the older overlapping feat(rewrite): rewrite commands bundles #2781 implementation because it conflicts with this newer lexer/policy implementation; the requested behavior is already present in this head" → Accepted as accurate for the head as of August. Since then develop grew its own lexer consolidation and PipelineSafety model, so the comparison that matters is now against develop rather than against feat(rewrite): rewrite commands bundles #2781.

Next

Recommend closing as superseded by #3128. The analysis in this PR was right about the bug and about the consumer/display-tail distinction — develop reached the same conclusions and kept the savings. Nothing in the remaining diff is worth rebasing through conflicts in all six files.

Rounds: 2/3. Threads: 0 open, 0 resolved.