Repository navigation
fix(builder): stop recording fake files from shell commands - #72
Conversation
…aths in shell file ops
EntelligenceAI PR SummaryReplaced the shell-command heuristics in flowchart TD
classDef newBehavior fill:#dcfce7,stroke:#16a34a,color:#14532d;
A["Shell command or argv"] --> B["Normalize command and unwrap shell scripts"]
B --> C["Tokenize shell syntax; strip heredoc bodies"]
C --> D["Resolve known variables and working directory"]
D --> E["Record reliable file operations"]
E --> F["File list and session statistics"]
class A,B,C,D,E,F newBehavior;
Review Scorecard
Safe to merge with a low blast radius; code quality rated Excellent (5/5): the review found nothing to flag. Key Findings:
Evaluated against
|
WalkthroughReworks ChangesFiles
Replaces the shell file-operation heuristic with a conservative parser that handles quoting, escapes, heredocs, redirects, subshells, and shell wrappers. Tracks known variable assignments and directory changes to identify file effects from redirects and commands such as Files
Adds parameterized regression tests for shell operators, quoting, heredocs, local and remote commands, wrappers, variables, directory changes, copy/move semantics, subshells, and malformed input. Also covers argv-form commands, Windows paths, performance, token-leak safeguards, and unchanged shell verb detection. |
|
thanks @21lakshh for the contribution! LGTM |
Summary
The problem
For every shell command in a session,
file_ops()inbuilder.pydecides which files the command created, changed, deleted or renamed. Those files go into the map's file list, the session totals (filesTouched,filesCreated,filesDeleted) and the file-based stats such as edited files.Today it cuts the command text at
&&,;,||and line breaks. In each piece it treats every word afterrm,mv,cportouch, and every word after>, as a file. The shell doesn't read commands that way, so files that were never touched get recorded:rm -f old.log 2>&1 | tail -5old.log,2>&1,|,tailold.logdeletedgit commit -m "fix login; rm old banner"old,bannercat > t.py <<'EOF', then a linedef f() -> None:, thenEOFt.pyandNonet.pycreated (the middle line is file content)ssh host 'cd /app && rm -rf cache'cacheon this machinehost)The fix
Only the shell part of
file_ops()changes. It now splits a command the way the shell does, usingshlexfrom the Python standard library. Concretely:<<'EOF'andEOFis file content, sormor>inside it is ignored.;or>inside"…"or'…'is plain text, not a command separator or a redirect.|,&&,2>&1and>/dev/nullare never file names. The word after>or>>is the file being written.cd, and variables set in the same command such asS=/tmp/x, are applied in order. Socd app && cat > a.tsrecordsapp/a.ts, andS=/tmp/x; cat > $S/run.pyrecords/tmp/x/run.py.bash -c "…",sh -c "…",pwsh -Command "…"orcmd /c "…"is read too.When the command text doesn't show for sure which file changed, nothing is recorded instead of a guess. That covers a
$VARwhose value isn't in the command, a pattern likerm *.log, acdinto such a folder, and a command that can't be parsed (for example, an unclosed quote).Edit and Write tool calls,
apply_patch, the map format and all callers are unchanged.Results
I ran the old and new code on my own local Claude Code sessions: 36 sessions with 13,390 shell commands.
2>&1,|,tail,None,console.log)Example: one session has 9 Edit/Write calls. It listed 223 changed files before this change and 47 after. I checked all 47 against the commands, and each is a real change.
Accuracy. The two versions record 3,856 file changes (old) and 2,109 (new) across all commands. I sorted them into 3 groups and checked a random 40 from each group by hand against the command:
Scaling each group's result to its full count:
S=/tmp/x; cat > $S/run.py) and files after acdin the middle of a command.Why it can't be 100% accurate
agentplay only sees the command text, not what happened when the command ran. Some changes can't be known from the text:
python cleanup.pyormake cleandeletes files, but the command doesn't name them.DIR=$(mktemp -d); rm $DIR/x, the folder name is created at run time. Inrm *.tmp, the files depend on what was on disk.cp -r src dest/createsdest/srcifdestalready exists, anddestif it doesn't.ssh host '…'ordocker exec box sh -c '…'changes files there, not here.a && b || c, it isn't recorded whetherborcran.So the rule is: record a change only when the command text proves it. A missing entry makes a count a little low. A wrong entry shows a file change that never happened.
Known limits
make, orfind -deleteare still not recorded, same as before.cdinside anifis assumed to run. So[ -d sub ] && cd sub; touch frecordssub/f.~is the home folder of the machine runningagentplay build. That's right when you build on the machine that recorded the session.classify()/_shell_paths()). This PR covers created, changed, deleted and renamed files only; reads can be a follow-up.main:cp -r src dest/whendestdoesn't exist yet recordsdest/src.rminside a shell function is recorded where the function is defined, even if it's never called."$X"_bak, if a variableX_bakalso exists, its value is used instead ofX+_bak.Test plan
make checkpasses: lint, 1,560 Python tests, 354 viewer tests.console.logorNone) from being stored as file names.shlexis in the standard library).What I ran:
tests/test_builder.py, one for each problem above. 74 of them fail onmain; the other 44 check that commandsmainalready handled correctly still give the same result.mainand on this branch, and compared the maps,index.json,search.jsonandrepos.json.grepcommands were counted as different only because the old code gave the second one a fake file.agentplay listgives the same output, andexport/importstill work.Not tested:
["bash", "-lc", "…"]and PowerShell commands) are covered by unit tests instead.