Skip to content

fix(builder): stop recording fake files from shell commands - #72

Merged
adityajha2005 merged 7 commits into
Entelligence-AI:mainfrom
21lakshh:fix/shell-file-ops
Nov 4, 2026
Merged

adityajha2005 merged 7 commits into
Entelligence-AI:mainfrom
21lakshh:fix/shell-file-ops

Conversation

@21lakshh

@21lakshh 21lakshh commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Summary

The problem

For every shell command in a session, file_ops() in builder.py decides 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 after rm, mv, cp or touch, and every word after >, as a file. The shell doesn't read commands that way, so files that were never touched get recorded:

Command Recorded today What really changed
rm -f old.log 2>&1 | tail -5 deleted old.log, 2>&1, |, tail old.log deleted
git commit -m "fix login; rm old banner" deleted old, banner nothing (it's the commit message)
cat > t.py <<'EOF', then a line def f() -> None:, then EOF created t.py and None t.py created (the middle line is file content)
ssh host 'cd /app && rm -rf cache' deleted cache on this machine nothing here (it ran on host)

The fix

Only the shell part of file_ops() changes. It now splits a command the way the shell does, using shlex from the Python standard library. Concretely:

  • Heredocs: text between <<'EOF' and EOF is file content, so rm or > inside it is ignored.
  • Quotes: quoted text is one value. A ; or > inside "…" or '…' is plain text, not a command separator or a redirect.
  • Shell symbols: |, &&, 2>&1 and >/dev/null are never file names. The word after > or >> is the file being written.
  • Folders and variables: cd, and variables set in the same command such as S=/tmp/x, are applied in order. So cd app && cat > a.ts records app/a.ts, and S=/tmp/x; cat > $S/run.py records /tmp/x/run.py.
  • Inner scripts: a script passed to bash -c "…", sh -c "…", pwsh -Command "…" or cmd /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 $VAR whose value isn't in the command, a pattern like rm *.log, a cd into 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.

Measure (all 36 sessions) Before After
Files listed as created, changed, deleted or renamed in the maps 1,835 597
Of those, files named like shell symbols or code (2>&1, |, tail, None, console.log) 174 0
Calls, errors, tokens, cost, commits, PRs, delays unchanged

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:

Group Count Real, out of the 40 checked
Recorded by both versions 1,392 40
Recorded only by the old version 2,464 0
Recorded only by the new version 717 40 (35 confirmed, 5 likely but not fully confirmed)

Scaling each group's result to its full count:

Before After
Recorded changes that are real about 1,392 of 3,856 (36%) 2,019 to 2,109 of 2,109 (96% to 100%)
Recorded changes that are wrong about 2,464 0 found in the sample
  • The range comes from the 5 unconfirmed rows: 96% counts only the confirmed ones, 100% counts all of them.
  • The extra real changes the new version finds are mostly writes through a variable (S=/tmp/x; cat > $S/run.py) and files after a cd in 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:

  1. Another program makes the change. python cleanup.py or make clean deletes files, but the command doesn't name them.
  2. The value only exists while the command runs. In DIR=$(mktemp -d); rm $DIR/x, the folder name is created at run time. In rm *.tmp, the files depend on what was on disk.
  3. The result depends on what's on disk. cp -r src dest/ creates dest/src if dest already exists, and dest if it doesn't.
  4. The command runs on another machine. Text inside ssh host '…' or docker exec box sh -c '…' changes files there, not here.
  5. The transcript doesn't say which parts ran. In a && b || c, it isn't recorded whether b or c ran.

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

  • Files changed by scripts, make, or find -delete are still not recorded, same as before.
  • A cd inside an if is assumed to run. So [ -d sub ] && cd sub; touch f records sub/f.
  • An unquoted ~ is the home folder of the machine running agentplay build. That's right when you build on the machine that recorded the session.
  • Commands longer than 100,000 characters record nothing. The longest command in my sessions is 3,342 characters.
  • Files a command only reads or searches still go through the old logic (classify() / _shell_paths()). This PR covers created, changed, deleted and renamed files only; reads can be a follow-up.
  • Three rare cases are still wrong, as they are on main:
    • cp -r src dest/ when dest doesn't exist yet records dest/src.
    • rm inside a shell function is recorded where the function is defined, even if it's never called.
    • In "$X"_bak, if a variable X_bak also exists, its value is used instead of X + _bak.

Test plan

  • make check passes: lint, 1,560 Python tests, 354 viewer tests.
  • No prompt text, model output, tool input/output, file contents or command arguments can reach a map, a log line or the viewer (redaction contract). This change also stops pieces of commands and code (like console.log or None) from being stored as file names.
  • No new network calls or Python runtime dependencies (shlex is in the standard library).

What I ran:

  • New tests: 118 cases in tests/test_builder.py, one for each problem above. 74 of them fail on main; the other 44 check that commands main already handled correctly still give the same result.
  • Full builds: I built all 36 local sessions on main and on this branch, and compared the maps, index.json, search.json and repos.json.
    • Only file-related values differ.
    • One session gains one detected retry: two identical grep commands were counted as different only because the old code gave the second one a fake file.
    • agentplay list gives the same output, and export / import still work.
  • Fuzzing: about 200,000 random shell-like commands, on Linux and with Windows path handling simulated. No crashes. Every result is a file path with one of the 5 existing operations (create, modify, delete, rename-from, rename-to).
  • Speed: reading all 14,000 commands takes about 3.7 s instead of 1.2 s. The slowest single command takes 3 ms.

Not tested:

  • Codex, Cursor, Gemini, Copilot, Kiro or OpenCode sessions, because I don't have any locally. Their command formats (such as ["bash", "-lc", "…"] and PowerShell commands) are covered by unit tests instead.
  • A real Windows machine. Windows paths were only tested with simulated Windows path handling.

@entelligence-ai-pr-reviews

entelligence-ai-pr-reviews Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

EntelligenceAI PR Summary

Replaced the shell-command heuristics in builder.py with shell-aware parsing so quoted text, heredoc bodies, pipes, and redirects aren’t mistaken for file paths. The parser recognizes file-changing commands and tracks known directories and literal variables, while skipping paths it can’t reliably resolve; this improves file lists and session statistics by avoiding fake file changes. Added extensive tests covering shell syntax, wrappers, subshells, Windows paths, and uncertain input.

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;
Loading

🟢 Green = new or changed in this PR


Review Scorecard

Dimension Rating Basis
Code Quality ●●●●● 5/5 — Excellent no critical/significant findings — rated Excellent
Blast Radius Low changed symbols are referenced only within their own file(s); no high-impact surface touched, 1 file(s) / ~543 line(s) changed (size only — not a blast signal)
Merge Confidence ●●●●● 5/5 — Safe to Merge equals code quality; blast radius is shown for context only
Test Coverage — unavailable dependencies failed to install
Security Scan No findings on changed lines SonarQube · Semgrep · TruffleHog on 2 changed files

Safe to merge with a low blast radius; code quality rated Excellent (5/5): the review found nothing to flag.

Key Findings:

  • No critical or significant issues found
Evaluated against
  • 2/2 changed files reviewed
  • criteria: correctness, security & access control, robustness & error handling, concurrency & data integrity, repo conventions / steering docs
  • steering docs: none found in repo
  • PR checks: unit tests (unavailable) · security scan (SonarQube, Semgrep, TruffleHog on 2 changed files)

@entelligence-ai-pr-reviews

entelligence-ai-pr-reviews Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Walkthrough

Reworks sm.file_ops to use conservative shell-aware parsing, improving file-effect detection across redirects, wrappers, variables, and working-directory changes while reducing unsafe path guesses. Adds regression coverage for shell syntax, command forms, platform paths, performance, and token-leak safeguards.

Changes

Files

  • python/src/agentplay/builder.py

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 rm, mv, cp, and touch, while avoiding records when paths cannot be determined safely.

Files

  • python/tests/test_builder.py

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.

Comment thread python/src/agentplay/builder.py Resolved
Comment thread python/tests/test_builder.py Resolved
@entelligence-ai-pr-reviews

Copy link
Copy Markdown

Findings outside the diff (1)

GitHub only accepts inline review comments on lines the PR changed. These findings are about code this PR did not change, so they are reported here instead.


python/src/agentplay/builder.py · lines 2163-2169 · MAJOR

MAJOR CORRECTNESS Filter heuristic shell paths before emitting targets

MapBuilder still adds classify()'s raw _shell_paths results even when the new parser rejects them. For echo /repo/src/not-a-real-file.py, the absolute-path heuristic can therefore emit a phantom hit despite file_ops returning no operation.

Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

Prevent unvalidated shell paths from `classify()` from becoming targets when the new parser rejects them, while preserving valid read/search path behavior. Add an integration test that builds a tool event for an absolute path appearing only in quoted or ordinary text and confirms it does not create a target.

@entelligence-ai-pr-reviews entelligence-ai-pr-reviews Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entelligence: code quality 5/5 — Excellent.

@adityajha2005

Copy link
Copy Markdown
Contributor

thanks @21lakshh for the contribution! LGTM

@adityajha2005
adityajha2005 merged commit acbff7a into Entelligence-AI:main Nov 4, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants