-
Notifications
You must be signed in to change notification settings - Fork 0
fix(flare-proxy): shortcircuit extract_filepaths treats shell redirects as literal filenames #396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -251,6 +251,25 @@ fn extract_command_prefix(command: &str) -> String { | |
| first_word.to_string() | ||
| } | ||
|
|
||
| /// True for shell redirection/control-operator tokens (`>`, `>>`, `<`, | ||
| /// `2>`, `2>&1`, `&>`, `|`, `||`, `&&`, `;`) that a naive whitespace split | ||
| /// leaves as their own token (e.g. `2>/dev/null`) or attached to a target | ||
| /// (e.g. `>out.txt`) -- neither is a filepath the command reads/writes by | ||
| /// name, so both must be excluded rather than mistaken for one. | ||
| fn is_shell_operator_token(part: &str) -> bool { | ||
| let trimmed = part.trim_start_matches(|c: char| c.is_ascii_digit()); | ||
| matches!(trimmed.chars().next(), Some('>' | '<' | '&' | '|' | ';')) | ||
| } | ||
|
|
||
| /// True when `part` is a redirection operator with no target fused onto it | ||
| /// (e.g. `>`, `2>`, `>>`) -- the shell takes the *next* whitespace-separated | ||
| /// token as the redirect target, so that token must also be dropped rather | ||
| /// than kept as a filepath the command reads. | ||
| fn is_bare_redirect_operator(part: &str) -> bool { | ||
| let trimmed = part.trim_start_matches(|c: char| c.is_ascii_digit()); | ||
| matches!(trimmed, ">" | ">>" | "<" | "<<" | "&>" | "&>>") | ||
| } | ||
|
|
||
| fn extract_filepaths(command: &str, _output: &str) -> String { | ||
| let listing_commands = [ | ||
| "ls", "dir", "find", "tree", "pwd", "cd", "mkdir", "rmdir", "rm", | ||
|
|
@@ -273,11 +292,24 @@ fn extract_filepaths(command: &str, _output: &str) -> String { | |
| } | ||
|
|
||
| if reading_commands.contains(&base_cmd.as_str()) { | ||
| let filepaths: Vec<&str> = parts[1..] | ||
| .iter() | ||
| .filter(|p| !p.starts_with('-')) | ||
| .copied() | ||
| .collect(); | ||
| let mut filepaths: Vec<&str> = Vec::new(); | ||
| let mut skip_next = false; | ||
| for part in &parts[1..] { | ||
| if skip_next { | ||
| skip_next = false; | ||
| continue; | ||
| } | ||
| if part.starts_with('-') { | ||
| continue; | ||
| } | ||
| if is_shell_operator_token(part) { | ||
| if is_bare_redirect_operator(part) { | ||
| skip_next = true; | ||
| } | ||
| continue; | ||
| } | ||
|
Comment on lines
+305
to
+310
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- available context tools ---'
command -v ctx_search || true
command -v ctx_callgraph || true
command -v ctx_compose || true
command -v ctx_read || true
command -v ctx_shell || true
printf '%s\n' '--- target file outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline crates/flare-proxy/src/shortcircuit.rs
else
printf '%s\n' 'ast-grep unavailable'
fi
printf '%s\n' '--- relevant source ---'
sed -n '260,365p' crates/flare-proxy/src/shortcircuit.rs
printf '%s\n' '--- symbols and call sites ---'
rg -n -C 3 'is_shell_operator_token|is_bare_redirect_operator|positional|skip_next|shortcircuit' crates/flare-proxy/src/shortcircuit.rs crates/flare-proxy/srcRepository: getappz/agentflare Length of output: 24533 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
p = Path("crates/flare-proxy/src/shortcircuit.rs")
lines = p.read_text().splitlines()
for start, end in [(1, 180), (260, 365)]:
print(f"--- shortcircuit.rs:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4}: {lines[i-1]}")
PY
printf '%s\n' '--- tests and fixtures related to path extraction ---'
rg -n -C 4 'filepath|file path|positional|redirect|pipeline|operator|&&|\|\||cat |grep ' crates/flare-proxy tests crates 2>/dev/null | head -n 500Repository: getappz/agentflare Length of output: 46911 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- available context tools ---'
command -v ctx_search || true
command -v ctx_callgraph || true
command -v ctx_compose || true
command -v ctx_read || true
command -v ctx_shell || true
printf '%s\n' '--- relevant source ---'
sed -n '260,365p' crates/flare-proxy/src/shortcircuit.rs
printf '%s\n' '--- related symbols and tests ---'
rg -n -C 4 'is_shell_operator_token|is_bare_redirect_operator|positional|skip_next|filepath|redirect|pipeline|operator|&&|\|\||cat |grep ' crates/flare-proxy/src crates/flare-proxy/tests tests 2>/dev/null | head -n 500Repository: getappz/agentflare Length of output: 39976 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
def is_shell_operator_token(part):
trimmed = part.lstrip("0123456789")
return bool(trimmed) and trimmed[0] in "><&|;"
def is_bare_redirect_operator(part):
trimmed = part.lstrip("0123456789")
return trimmed in {">", ">>", "<", "<<", "&>", "&>>"}
def implemented_paths(command):
parts = command.split()
if not parts:
return []
base_cmd = parts[0].replace("\\", "/").split("/")[-1].lower()
reading = {"cat", "head", "tail", "less", "more", "bat", "type"}
if base_cmd in reading:
paths = []
skip_next = False
for part in parts[1:]:
if skip_next:
skip_next = False
continue
if part.startswith("-"):
continue
if is_shell_operator_token(part):
if is_bare_redirect_operator(part):
skip_next = True
continue
paths.append(part)
return paths
if base_cmd == "grep":
flags_with_args = {"-e", "-f", "-m", "-A", "-B", "-C"}
pattern_via_flag = False
positional = []
skip_next = False
for part in parts[1:]:
if skip_next:
skip_next = False
continue
if part.startswith("-"):
if part in flags_with_args:
if part in {"-e", "-f"}:
pattern_via_flag = True
skip_next = True
continue
if is_shell_operator_token(part):
if is_bare_redirect_operator(part):
skip_next = True
continue
positional.append(part)
return positional if pattern_via_flag else positional[1:] if len(positional) > 1 else []
cases = {
"cat src/main.rs && echo done": ["src/main.rs", "echo", "done"],
"grep pattern src/main.rs | sort": ["src/main.rs", "sort"],
"cat src/main.rs > out.txt": ["src/main.rs"],
"grep -e pattern src/main.rs | sort": ["src/main.rs", "sort"],
}
for command, expected in cases.items():
actual = implemented_paths(command)
print(f"{command!r} -> {actual!r}")
assert actual == expected, (command, actual, expected)
PY
printf '%s\n' '--- command extraction implementation ---'
sed -n '175,215p' crates/flare-proxy/src/shortcircuit.rs
printf '%s\n' '--- existing filepath tests ---'
sed -n '690,755p' crates/flare-proxy/src/shortcircuit.rsRepository: getappz/agentflare Length of output: 4120 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
def is_shell_operator_token(part):
trimmed = part.lstrip("0123456789")
return bool(trimmed) and trimmed[0] in "><&|;"
def is_bare_redirect_operator(part):
trimmed = part.lstrip("0123456789")
return trimmed in {">", ">>", "<", "<<", "&>", "&>>"}
def implemented_paths(command):
parts = command.split()
base_cmd = parts[0].replace("\\", "/").split("/")[-1].lower()
if base_cmd in {"cat", "head", "tail", "less", "more", "bat", "type"}:
paths, skip_next = [], False
for part in parts[1:]:
if skip_next:
skip_next = False
continue
if part.startswith("-"):
continue
if is_shell_operator_token(part):
if is_bare_redirect_operator(part):
skip_next = True
continue
paths.append(part)
return paths
if base_cmd == "grep":
positional, skip_next, pattern_via_flag = [], False, False
for part in parts[1:]:
if skip_next:
skip_next = False
continue
if part.startswith("-"):
if part in {"-e", "-f", "-m", "-A", "-B", "-C"}:
pattern_via_flag |= part in {"-e", "-f"}
skip_next = True
continue
if is_shell_operator_token(part):
if is_bare_redirect_operator(part):
skip_next = True
continue
positional.append(part)
return positional if pattern_via_flag else positional[1:] if len(positional) > 1 else []
cases = {
"cat src/main.rs && echo done": ["src/main.rs", "echo", "done"],
"grep pattern src/main.rs | sort": ["src/main.rs", "sort"],
}
for command, expected in cases.items():
actual = implemented_paths(command)
print(f"{command!r} -> {actual!r}")
assert actual == expected
PYRepository: getappz/agentflare Length of output: 283 Stop filepath extraction at command separators. In both loops, later command tokens become filepaths. For example, 🤖 Prompt for AI Agents |
||
| filepaths.push(part); | ||
| } | ||
| if filepaths.is_empty() { | ||
| return "<filepaths>\n</filepaths>".into(); | ||
| } | ||
|
|
@@ -304,6 +336,12 @@ fn extract_filepaths(command: &str, _output: &str) -> String { | |
| } | ||
| continue; | ||
| } | ||
| if is_shell_operator_token(part) { | ||
| if is_bare_redirect_operator(part) { | ||
| skip_next = true; | ||
| } | ||
| continue; | ||
| } | ||
| positional.push(part); | ||
| } | ||
|
|
||
|
|
@@ -677,6 +715,24 @@ mod tests { | |
| assert!(result.contains("tests/test.rs")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_extract_filepaths_ignores_stderr_redirect() { | ||
| let result = extract_filepaths("tail -5 src/main.rs 2>/dev/null", "some content"); | ||
| assert_eq!(result, "<filepaths>\nsrc/main.rs\n</filepaths>"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_extract_filepaths_ignores_stdout_redirect() { | ||
| let result = extract_filepaths("cat src/main.rs > out.txt", "some content"); | ||
| assert_eq!(result, "<filepaths>\nsrc/main.rs\n</filepaths>"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_extract_filepaths_grep_ignores_redirect() { | ||
| let result = extract_filepaths("grep pattern src/main.rs 2>/dev/null", "matches"); | ||
| assert_eq!(result, "<filepaths>\nsrc/main.rs\n</filepaths>"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_extract_filepaths_empty() { | ||
| let result = extract_filepaths("", ""); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: getappz/agentflare
Length of output: 17627
🏁 Script executed:
Repository: getappz/agentflare
Length of output: 5316
Handle redirects attached to preceding arguments.
command.split_whitespace()keepssrc/main.rs>out.txtas one token. Becauseis_shell_operator_tokenchecks only the first non-digit character, the parser reports the combined token as a filepath. Tokenize shell operators before filepath classification, or use a shell-aware tokenizer. Add cases forcat src/main.rs>out.txtandgrep pattern src/main.rs>out.txt.🤖 Prompt for AI Agents