fix(rewrite): preserve shell pipeline and redirection semantics (#166) - #170
Merged
Merged
Conversation
The Pi/Claude auto-rewrite was substituting `contextcrawler <cmd>` for
pipeline producers, silently changing answers when the consumer parses
or counts the producer's output. The original report:
contextcrawler proxy sh -lc 'grep -R "fn " src/analytics | wc -l' # 95
grep -R "fn " src/analytics | wc -l # 85 (wrong)
Same class affected stdout redirection: `cargo check >/tmp/log` was
rewritten, putting filtered output in the file instead of raw.
Fix applies in src/discover/registry.rs:
1. New `redirect_affects_stdout(&str) -> bool` classifier — recognises
`>`, `>>`, `1>`, `1>>`, `&>`, `&>>`, `>&2`, `>&-` as stdout-affecting;
`2>`, `2>>`, `2>&1`, `2>&-`, `<`, `<<`, `<<<` as stdout-safe. Unknown
redirect tokens default to "affects stdout" (fail-closed: never
rewrite something the classifier doesn't understand).
2. New `segment_stdout_is_redirected(&str)` walks `tokenize(cmd)` once
and returns true if any trailing-redirect token diverts stdout.
3. Pipe-handling in `rewrite_compound` rewritten: a segment whose stdout
is piped MUST NOT be rewritten. The producer + the rest of the chain
up to the next `&&`/`||`/`;`/`&` boundary stays exactly as the user
typed it. Previously only `find`/`fd` were special-cased; now every
pipeline producer is preserved.
4. Process substitution `<(...)` and `$(...)` short-circuit via
`extract_substitutions` (existing path), so the outer command never
gets rewritten when it consumes a substitution payload.
5. Heredocs and `$((` arithmetic substitution already short-circuited
in `split_command_chain`; the issue_166 tests now pin that.
What still rewrites:
- Top-level simple commands with terminal stdout — `git status`,
`cargo check`, `ls -la`.
- `&&` / `||` / `;` chains where each branch is independently a
simple terminal-stdout command.
- `2>file`, `2>&1` (alone), `2>&-` — stderr-only redirects leave
stdout flowing to the model, so the filter still applies.
Nested shells (`sh -lc 'git status'`) continue to bypass rewrite —
safer than rewriting blindly into a re-tokenised inner shell.
22 issue_166 tests added/locked covering: the original bug, every
redirect shape above, command substitution, backticks, heredocs, the
baseline still-rewrites case, `&&`-chain rewriting, nested-shell
bypass, and four security-review lockdown cases (no-space chained
redirect `2>&1>file`, bash `|&` shorthand, process subst piped into
another command, pipe with consumer-side trailing redirect).
Manual reverify against the original bug:
./target/release/contextcrawler rewrite 'grep -R "fn " src/analytics | wc -l'
-> (no output: unchanged)
./target/release/contextcrawler rewrite 'cargo check >/tmp/build.log'
-> (no output: unchanged)
./target/release/contextcrawler rewrite 'git status && cargo check'
-> contextcrawler git status && contextcrawler cargo check
./target/release/contextcrawler rewrite 'git status'
-> contextcrawler git status
Security review (codex + agy + self): APPROVE, no blocking findings.
Tests: 2716 passed, 0 failed (22 issue_166 tests all green).
noogalabs
pushed a commit
to noogalabs/contextcrawler
that referenced
this pull request
Jun 4, 2026
…#246) The shell keyword "fi" was listed as a bare prefix in IGNORED_PREFIXES, causing classify_command("find ...") to return Ignored because "find".starts_with("fi") is true. This prevented find commands from being rewritten by rtk rewrite and the hook system. Move "fi" and "done" from IGNORED_PREFIXES to IGNORED_EXACT so they only match as exact standalone keywords, not as prefixes of other commands. Fixes thehoff#170, rtk-ai#204, rtk-ai#236 (partially — find classification was the root blocker for rtk rewrite). Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Correctness fix — silent wrong answers. The Pi/Claude auto-rewrite was substituting
contextcrawler <cmd>for pipeline producers, silently changing answers when the consumer parses or counts the producer's output:Same class affected stdout redirection —
cargo check >/tmp/logwas rewritten, putting filtered output in the file instead of raw.Fix (`src/discover/registry.rs`)
What still rewrites
Nested shells (`sh -lc 'git status'`) continue to bypass rewrite — safer than rewriting blindly into a re-tokenised inner shell.
Test plan
Closes #166.
Security review
codex + agy + self: APPROVE, 0 blocking findings. 4 LOW lockdown tests applied.