Give the coder read, write, edit and bash as first-class tools #127

Closed AtlantisPleb opened this 1d ago 2 comments

The coder's only way to touch a file is shell. Every read is cat, every
write is a heredoc, every edit is sed or a rewrite — so the model spends
tokens on shell quoting, and the transcript records a shell invocation where it
should record an intent.

Pi solved this in one commit at the very start of its coding agent
(earendil-works/pi, ffc9be886, "Agent package + coding agent WIP"), with
four files totalling 158 lines: read.ts 29, write.ts 31, edit.ts 61,
bash.ts 37. That is the whole of it, and it is worth copying because of how
little it is.

What the four originals do

  • read — one parameter, path. Resolve, check it exists, return the whole
    file as text. Errors come back as a string the model can read, not an
    exception.
  • writepath and content. mkdir -p the parent, write, report
    Successfully wrote N bytes. Creates or overwrites.
  • editpath, oldText, newText. Exact-match replacement, and the
    two refusals are the design: it refuses when oldText is not found, and
    it refuses when it is found more than once, telling the model to add
    context until the match is unique. That single rule is what makes a
    surgical edit safe without a diff format.
  • bashcommand, 30s timeout, 10 MB buffer, stdout with stderr appended
    under a STDERR: marker.

Pi's default active set is exactly read, bash, edit, write, with
grep, find and ls available read-only.
(docs/teardowns/2026-07-21-pi-agent-teardown.md §3.4.)

What to build

Rust equivalents in crates/openagents-cli/src/tools.rs, declared beside
shell, skill, openagents, capability, delegate and acp. Keep them
this small. The point of the reference is its size.

What ours must do that the originals did not

These are not embellishments — each one is a defect this repository has already
shipped and fixed once:

  • A bounded cut steps back to a character boundary. A byte index into a
    String has panicked here ten times, including on the path whose job is
    reporting a refusal. tracker::floor_char_boundary exists; use it.
  • A failed tool reports is_error: true. shell reported every failure as
    a success until recently, so a failing build read like a passing one.
  • A refusal names what went wrong and returns it as output, exactly as pi
    does — the model reads the refusal and retries. It must never be a silent
    empty result.
  • write and edit stage and rename rather than writing in place. A
    reader in the truncate window otherwise sees a half-written file; see #114.
  • No path escapes the session's working directory without saying so.

Tests

Assert the behaviour that makes edit safe: a oldText appearing twice is
refused and the file is unchanged. Assert read on a missing file returns a
refusal rather than raising. Assert a non-zero bash exit is reported as an
error. Verify adversarially — revert each guard and watch the test fail.

  1. AtlantisPleb opened this issue 1d ago
  2. A AtlantisPleb Author 1d ago

    Compared pi's originals against ****, a fork that took the same four tools a long way. Synced to (already in the manifest at line 169; the local clone is a zip-import with unrelated history so it will not fast-forward — read origin/main directly).

    The size difference is the story:

    tool pi (ffc9be886) oh-my-pi (origin/main)
    read 29 2,385
    write 31 1,769
    bash 37 1,779
    edit 61 gone — replaced by ast-edit

    Build pi's four. Do not build oh-my-pi's four. 158 lines that work beats 6,000 lines of someone else's product decisions. But three ideas in there are worth knowing about now, because they change what you would design if you knew them.

    1. edit did not survive contact. oh-my-pi deleted exact-string edit entirely and replaced it with ast_edit — structural rewrites via ast-grep, $NAME metavariables matching whole AST nodes, backed by a Rust crate (crates/pi-natives/src/ast.rs). That is a real signal: the exact-match-with-uniqueness rule is a good first tool and it is the one that gets outgrown first. Worth knowing before we grow ours in the same direction by accident.

    2. Hashline — the idea I would steal. packages/hashline is "a compact, line-anchored patch language" whose stated purpose is: it binds every hunk to a file-content hash so stale anchors are rejected before they corrupt code. That is the same failure our exact-match rule guards against — a model editing against a file that has since moved — solved once, generally, instead of per-tool. Our edit should at minimum refuse when the file changed under it, and hashline is the shape of the general answer.

    3. A repeat-read hint. read.ts tracks reads per session and, after three identical ones, appends a hint to the result — catching the model in a loop and telling it so. Cheap, and it addresses something we have watched happen.

    Also there, less transferable but worth naming: read takes internal URIs (memory://, skill://) and URLs through the same path parameter; it returns a separate displayContent so the TUI renders without re-parsing the model-facing text; it counts unresolved git conflicts for an inline badge; bash has grown into five files including a PTY selector and an interceptor.

    None of that changes this issue's scope. Build the 158 lines, with the five guards already listed. The reason to record this is so the second version is not designed in ignorance of where this road goes.

  3. A AtlantisPleb Author 1d ago

    Correction to the comment above: two names were eaten by shell expansion before
    it posted. The fork is can1357/oh-my-pi, and the local clone is at
    ~/work/projects/repos/oh-my-pi. Nothing else in that comment changed.

  4. AtlantisPleb closed this as completed in 75eb40b 1d ago
Sign in with GitHub to comment on this issue.