coder: give the model read, write, edit and bash as first-class tools

75eb40bb55d2 · AtlantisPleb · · parent 64984dca7c45

coder: give the model read, write, edit and bash as first-class tools

The coder's only way to touch a file was `shell`, so every read was `cat`,
every write a heredoc, and every edit a `sed` — the model paid tokens on shell
quoting and the transcript recorded an invocation where it should have
recorded an intent.

These are pi's four (`earendil-works/pi`, ffc9be886), kept to pi's size. `read`
returns a file, `write` replaces one, `edit` replaces one exact run of text
inside one, and `bash` is `shell` under the name that tool set uses — one arm
answers both names, so they cannot drift apart.

`edit`'s two refusals are the design and they are copied exactly: it refuses
when `oldText` is not found, and it refuses when it is found more than once,
naming the count and asking for more context until the match is unique. That
single rule is what makes a surgical edit safe without a diff format.

Five guards the originals did not have, each a defect this repository has
already shipped and fixed:

- The bounded cut in `read` steps back to a character boundary. A raw byte
  index into a `String` has panicked here before and took the process with it.
- A failed tool reports `is_error: true`, so a failing command cannot read
  like a passing one.
- Every refusal names what went wrong and comes back as tool output the model
  reads and retries from, never a silent empty result.
- `write` and `edit` stage beside the destination and rename, so no reader
  sees a half-written file (#114).
- No path reaches outside the session's working directory, and the refusal
  says where the path landed. `..` is resolved lexically so a file that does
  not exist yet is checked too, then the deepest existing ancestor is
  canonicalised so a symlink pointing out of the tree is caught.

The four descriptions are staged text (`surfaces/coder/tool-descriptions.v1.json`,
#122), not literals, so they stay versioned and A/B-able.

Closes #127

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SoZMfWRSGnf6FZX2Ar9rQ2
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes
#127

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

Not deployed through the forge lane

No push, promotion, build, or deploy receipt references this commit (receipts are scanned over a bounded recent window). Changes shipped by full node replacement carry their proof in the release gate receipt instead.

Changed files

  • modified crates/coder-lite/src/runtime.rs
  • modified crates/openagents-cli/src/surfaces.rs
  • modified crates/openagents-cli/src/tools.rs
  • modified crates/openagents-cli/tests/coder-surfaces-golden.json
  • modified crates/openagents-cli/tests/plugin_host_test.rs
  • modified packages/openagents-cli/src/coder-surfaces.generated.ts
  • modified surfaces/coder/index.json
  • modified surfaces/coder/tool-descriptions.v1.json

Diff

8 files changed, +638 -21

crates/coder-lite/src/runtime.rs modified +9 -6

@@ -89,7 +89,7 @@ pub fn send(sink: &Sink, message: Control) {

89 89
/// The system message this session opens with.
90 90
///
91 91
/// The staged instructions first and unchanged, then the tools — because a
92
/// model that is told it has no tools when it has five will not use them, and
92
/// model that is told it has no tools when it has some will not use them, and
93 93
/// one told it has tools it does not have will claim to have run them. The
94 94
/// list is generated from what was actually declared, so the two cannot
95 95
/// disagree.

@@ -160,10 +160,10 @@ pub struct Session {

160 160
impl Session {
161 161
    /// Open a session on `lane`, reporting everything it does to `tx`.
162 162
    ///
163
    /// The tools are the full set — `shell`, `skill`, `openagents`,
164
    /// `capability`, and `delegate` — on the same terms `oa coder` gets them:
165
    /// children run on this lane, on this credential, and cannot delegate
166
    /// again.
163
    /// The tools are the full set — `read`, `write`, `edit`, `bash`, `shell`,
164
    /// `skill`, `openagents`, `capability`, and `delegate` — on the same terms
165
    /// `oa coder` gets them: children run on this lane, on this credential, and
166
    /// cannot delegate again.
167 167
    pub fn open(
168 168
        lane: Lane,
169 169
        lane_name: &str,

@@ -394,7 +394,10 @@ pub fn tool_title(name: &str, arguments: &str) -> String {

394 394
    };
395 395
396 396
    let detail = match name {
397
        "shell" => string("command"),
397
        "shell" | "bash" => string("command"),
398
        // The path, never the content: `write` carries a whole file in its
399
        // arguments, and the fallthrough below would put it in the header.
400
        "read" | "write" | "edit" => string("path"),
398 401
        "skill" => string("name"),
399 402
        "openagents" => parsed.get("args").and_then(|v| v.as_array()).map(|args| {
400 403
            args.iter()
crates/openagents-cli/src/surfaces.rs modified +9 -1

@@ -35,6 +35,14 @@ pub mod system_prompt {

35 35
36 36
/// The tool-description surface: `surfaces/coder/tool-descriptions.v1.json`.
37 37
pub mod tool_descriptions {
38
    /// `rust.read`
39
    pub const RUST_READ: &str = "Read a file and return its whole contents as text. `path` is relative to the session's working directory, or an absolute path inside it; a path that resolves outside it is refused. Use this rather than `cat` through the shell — there is nothing to quote and nothing to escape.";
40
    /// `rust.write`
41
    pub const RUST_WRITE: &str = "Write `content` to `path`, creating parent directories and replacing any file already there. The file is staged beside its destination and renamed into place, so nothing else on this machine can read it half-written. Use this rather than a shell heredoc. To change part of a file use `edit` instead: it does not ask you to reproduce the rest.";
42
    /// `rust.edit`
43
    pub const RUST_EDIT: &str = "Replace one exact run of text in a file. `oldText` must match byte for byte, whitespace and newlines included, and must appear exactly once. If it appears more than once the edit is refused and the file is left alone: add the lines above and below the one you mean until the match is unique, then call again. `newText` may be empty to delete the run. Use this rather than `sed` for a surgical change.";
44
    /// `rust.bash`
45
    pub const RUST_BASH: &str = "Run a command through `/bin/sh -c` in the session's working directory, {cwd}. Returns combined stdout and stderr, and reports a non-zero exit as a failure rather than as ordinary output. Batch independent commands into one call with `&&` instead of one call each: every call replays the conversation so far. This is the same runner as `shell`, and either name reaches it.";
38 46
    /// `rust.shell`
39 47
    pub const RUST_SHELL: &str = "Run a shell command on this machine. The working directory is {cwd}, so paths are relative to it and you do not need to ask where you are. Returns combined stdout and stderr with the exit code. Batch independent commands into one call with && instead of one call each: every call replays the conversation so far.";
40 48
    /// `rust.skill`

@@ -52,6 +60,6 @@ pub mod tool_descriptions {

52 60
/// A run records these so a bench row names exactly which text produced it.
53 61
pub const SURFACE_DIGESTS: [(&str, &str); 3] = [
54 62
    ("system-prompt", "sha256:8f0fbbbb38f4ce609a09fe2ba4e03ca160ff2c0d0f698dd4676c2bf940ded849"),
55
    ("tool-descriptions", "sha256:513c7518c72ba8c9959e517ea9f1262d7d3ee6b08452ddb48568bdabf6383c1b"),
63
    ("tool-descriptions", "sha256:f85ed27173e41eb6752cc6232600ff17b02ea06a41b59383b1963093459b1eca"),
56 64
    ("catalog-lines", "sha256:98a96bcf6acfd5847bc1bddcc575761af71be7ea144f3584b3594b24b2f80911"),
57 65
];
crates/openagents-cli/src/tools.rs modified +597 -9

@@ -1,7 +1,8 @@

1 1
//! The tools a session declares to the model, and what running them does.
2 2
//!
3
//! Five tools: `shell`, `skill`, `openagents`, `capability`, and — only where
4
//! a delegation gate exists — `delegate`. Each is declared to the model and
3
//! Nine tools: `read`, `write`, `edit`, `bash`, `shell`, `skill`,
4
//! `openagents`, `capability`, and — only where a delegation gate exists —
5
//! `delegate`. Each is declared to the model and
5 6
//! each has an implementation in [`HarnessToolRegistry::execute_tool`]; the
6 7
//! list and the match arms carry the same names, which is the only property
7 8
//! that keeps a declared tool from being a promise nothing keeps. This

@@ -9,6 +10,18 @@

9 10
//! declare it and no arm implemented it; the rule the mistake bought is that
10 11
//! a name is written here only after something answers it.
11 12
//!
13
//! `read`, `write`, `edit` and `bash` are the four a coding agent needs to
14
//! touch a file without spelling the intent as a shell command. They are pi's
15
//! four, deliberately kept to pi's size: `read` returns a file, `write`
16
//! replaces one, `edit` replaces one exact run of text inside one, and `bash`
17
//! is `shell` under the name that set uses — the same arm answers both, so
18
//! they cannot drift apart. What the originals did not have is here because
19
//! this repository has shipped and fixed each of them: a bounded cut steps
20
//! back to a character boundary, a failure reports `is_error: true`, a refusal
21
//! is output the model can read and retry from, `write` and `edit` stage and
22
//! rename rather than truncating in place (#114), and no path leaves the
23
//! session's working directory without the refusal saying where it went.
24
//!
12 25
//! `capability` searches the local catalog of digest-pinned WebAssembly
13 26
//! plugins and loads one into the session, at which point the plugin's own
14 27
//! manifest declares a second tool under its own name. The sandbox that runs

@@ -50,12 +63,21 @@ pub const OUTPUT_LIMIT: usize = 30_000;

50 63
/// session answers it with a refusal, which shadows a plugin just as
51 64
/// completely. `every_declared_tool_has_an_arm_that_answers_it` keeps this
52 65
/// list and the arms in step.
53
pub const BUILTIN_TOOL_NAMES: [&str; 5] =
54
    ["shell", "skill", "openagents", "capability", "delegate"];
66
pub const BUILTIN_TOOL_NAMES: [&str; 9] = [
67
    "read",
68
    "write",
69
    "edit",
70
    "bash",
71
    "shell",
72
    "skill",
73
    "openagents",
74
    "capability",
75
    "delegate",
76
];
55 77
56 78
/// A tool the front-end driving the session answers itself.
57 79
///
58
/// The five tools above are every session's, and they stay here. A front-end
80
/// The nine tools above are every session's, and they stay here. A front-end
59 81
/// can have a capability no other caller has — coder-lite's ACP path, which
60 82
/// hands a task to a coding agent installed on this machine, is the one this
61 83
/// exists for — and it belongs in the same declaration the other five are in,

@@ -380,6 +402,54 @@ impl HarnessToolRegistry {

380 402
        }
381 403
382 404
        let mut tools = vec![
405
            ToolDefinition {
406
                name: "read".to_string(),
407
                description: text::RUST_READ.to_string(),
408
                parameters: serde_json::json!({
409
                    "type": "object",
410
                    "properties": {
411
                        "path": {"type": "string", "description": "The file to read, relative to the working directory or absolute inside it."}
412
                    },
413
                    "required": ["path"]
414
                }),
415
            },
416
            ToolDefinition {
417
                name: "write".to_string(),
418
                description: text::RUST_WRITE.to_string(),
419
                parameters: serde_json::json!({
420
                    "type": "object",
421
                    "properties": {
422
                        "path": {"type": "string", "description": "The file to write, relative to the working directory or absolute inside it."},
423
                        "content": {"type": "string", "description": "The complete new contents of the file."}
424
                    },
425
                    "required": ["path", "content"]
426
                }),
427
            },
428
            ToolDefinition {
429
                name: "edit".to_string(),
430
                description: text::RUST_EDIT.to_string(),
431
                parameters: serde_json::json!({
432
                    "type": "object",
433
                    "properties": {
434
                        "path": {"type": "string", "description": "The file to edit, relative to the working directory or absolute inside it."},
435
                        "oldText": {"type": "string", "description": "The exact text to replace. It must appear in the file exactly once."},
436
                        "newText": {"type": "string", "description": "What to put in its place. Empty deletes the old text."}
437
                    },
438
                    "required": ["path", "oldText", "newText"]
439
                }),
440
            },
441
            ToolDefinition {
442
                name: "bash".to_string(),
443
                description: text::RUST_BASH.replace("{cwd}", &self.cwd.display().to_string()),
444
                parameters: serde_json::json!({
445
                    "type": "object",
446
                    "properties": {
447
                        "command": {"type": "string", "description": "The command line to run through /bin/sh -c."},
448
                        "timeout_seconds": {"type": "integer", "description": "How long to wait. Defaults to 120; raise for a build or test run."}
449
                    },
450
                    "required": ["command"]
451
                }),
452
            },
383 453
            ToolDefinition {
384 454
                name: "shell".to_string(),
385 455
                description: text::RUST_SHELL.replace("{cwd}", &self.cwd.display().to_string()),

@@ -469,7 +539,35 @@ impl HarnessToolRegistry {

469 539
470 540
    pub async fn execute_tool(&self, call: &ToolCall) -> ToolOutput {
471 541
        match call.name.as_str() {
472
            "shell" => {
542
            "read" => {
543
                let (output, is_error) = answer_read(&self.cwd, &call.arguments);
544
                ToolOutput {
545
                    call_id: call.id.clone(),
546
                    output,
547
                    is_error,
548
                }
549
            }
550
            "write" => {
551
                let (output, is_error) = answer_write(&self.cwd, &call.arguments);
552
                ToolOutput {
553
                    call_id: call.id.clone(),
554
                    output,
555
                    is_error,
556
                }
557
            }
558
            "edit" => {
559
                let (output, is_error) = answer_edit(&self.cwd, &call.arguments);
560
                ToolOutput {
561
                    call_id: call.id.clone(),
562
                    output,
563
                    is_error,
564
                }
565
            }
566
            // One arm for both names: `bash` is the name pi's tool set gives
567
            // this, `shell` is the name this session has always given it, and
568
            // two implementations would be two behaviours the moment either
569
            // one was touched.
570
            "shell" | "bash" => {
473 571
                let cmd = call
474 572
                    .arguments
475 573
                    .get("command")

@@ -999,6 +1097,211 @@ async fn run_real_shell(cmd: &str, cwd: &Path, timeout_secs: u64) -> (String, bo

999 1097
    }
1000 1098
}
1001 1099
1100
// ─────────────────────────────────────────────────────────── the file tools
1101
1102
/// The absolute path `raw` names, or why this session will not touch it.
1103
///
1104
/// A relative path is taken against the session's working directory. `..` is
1105
/// resolved here rather than by the filesystem, so a path that does not exist
1106
/// yet is checked too — `write` creates files, and a check that only worked on
1107
/// what already exists would be no check at all. Then the deepest part of the
1108
/// path that does exist is canonicalised, which is what catches a symlink
1109
/// inside the tree pointing out of it.
1110
///
1111
/// The refusal is the tool's output: it names where the path landed, so the
1112
/// model can retry with one inside rather than read an empty result.
1113
fn resolve_in_cwd(cwd: &Path, raw: &str) -> Result<PathBuf, String> {
1114
    if raw.trim().is_empty() {
1115
        return Err("No `path` was given.".to_string());
1116
    }
1117
    let base = cwd.canonicalize().unwrap_or_else(|_| cwd.to_path_buf());
1118
    let joined = if Path::new(raw).is_absolute() {
1119
        PathBuf::from(raw)
1120
    } else {
1121
        base.join(raw)
1122
    };
1123
1124
    let mut lexical = PathBuf::new();
1125
    for part in joined.components() {
1126
        match part {
1127
            std::path::Component::ParentDir => {
1128
                lexical.pop();
1129
            }
1130
            std::path::Component::CurDir => {}
1131
            other => lexical.push(other.as_os_str()),
1132
        }
1133
    }
1134
1135
    if !nearest_existing(&lexical).starts_with(&base) {
1136
        return Err(format!(
1137
            "Refusing to touch `{raw}`: it resolves to {}, outside this session's working \
1138
             directory {}. Name a path inside it.",
1139
            lexical.display(),
1140
            base.display()
1141
        ));
1142
    }
1143
    Ok(lexical)
1144
}
1145
1146
/// The deepest ancestor of `path` that exists, resolved through symlinks.
1147
fn nearest_existing(path: &Path) -> PathBuf {
1148
    let mut probe = path;
1149
    loop {
1150
        if let Ok(real) = probe.canonicalize() {
1151
            return real;
1152
        }
1153
        match probe.parent() {
1154
            Some(parent) => probe = parent,
1155
            None => return path.to_path_buf(),
1156
        }
1157
    }
1158
}
1159
1160
/// Write `content` to `path` by staging it beside the destination and renaming.
1161
///
1162
/// Truncating in place opens a window in which every other reader on the
1163
/// machine — a compiler, a test run, a second agent — sees a half-written file
1164
/// (#114). `rename` within a directory is atomic, so no reader sees anything
1165
/// but the old file or the new one. The staging name carries the process id
1166
/// and a counter so two writers cannot collide on it, and a failed rename
1167
/// takes the staged file with it rather than leaving litter behind.
1168
fn write_atomically(path: &Path, content: &str) -> std::io::Result<()> {
1169
    let dir = path.parent().unwrap_or_else(|| Path::new("."));
1170
    fs::create_dir_all(dir)?;
1171
1172
    static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
1173
    let staging = dir.join(format!(
1174
        ".{}.{}.{}.staged",
1175
        path.file_name().and_then(|n| n.to_str()).unwrap_or("file"),
1176
        std::process::id(),
1177
        NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
1178
    ));
1179
1180
    fs::write(&staging, content)?;
1181
    if let Err(error) = fs::rename(&staging, path) {
1182
        let _ = fs::remove_file(&staging);
1183
        return Err(error);
1184
    }
1185
    Ok(())
1186
}
1187
1188
/// The `read` tool: a file as text, or a refusal saying why not.
1189
fn answer_read(cwd: &Path, arguments: &serde_json::Value) -> (String, bool) {
1190
    let raw = arguments.get("path").and_then(|v| v.as_str()).unwrap_or("");
1191
    let path = match resolve_in_cwd(cwd, raw) {
1192
        Ok(path) => path,
1193
        Err(refusal) => return (refusal, true),
1194
    };
1195
    match fs::read_to_string(&path) {
1196
        Ok(content) if content.len() > OUTPUT_LIMIT => {
1197
            // The cut steps back to a character boundary; slicing a `String`
1198
            // at a fixed byte count panics the first time a file carries a
1199
            // non-ASCII character across the limit.
1200
            let head = &content[..floor_char_boundary(&content, OUTPUT_LIMIT)];
1201
            (
1202
                format!(
1203
                    "{head}\n\n[Output truncated: the file is {} characters, limit is {}. Read \
1204
                     the rest with `bash`.]",
1205
                    content.len(),
1206
                    OUTPUT_LIMIT
1207
                ),
1208
                false,
1209
            )
1210
        }
1211
        Ok(content) => (content, false),
1212
        Err(error) => (format!("Could not read `{raw}`: {error}."), true),
1213
    }
1214
}
1215
1216
/// The `write` tool: replace a file's contents, or say why nothing was written.
1217
fn answer_write(cwd: &Path, arguments: &serde_json::Value) -> (String, bool) {
1218
    let raw = arguments.get("path").and_then(|v| v.as_str()).unwrap_or("");
1219
    let Some(content) = arguments.get("content").and_then(|v| v.as_str()) else {
1220
        return (
1221
            "Nothing was written: `content` is required and must be the file's complete new \
1222
             contents."
1223
                .to_string(),
1224
            true,
1225
        );
1226
    };
1227
    let path = match resolve_in_cwd(cwd, raw) {
1228
        Ok(path) => path,
1229
        Err(refusal) => return (refusal, true),
1230
    };
1231
    match write_atomically(&path, content) {
1232
        Ok(()) => (
1233
            format!("Wrote {} bytes to {}.", content.len(), path.display()),
1234
            false,
1235
        ),
1236
        Err(error) => (format!("Could not write `{raw}`: {error}."), true),
1237
    }
1238
}
1239
1240
/// The `edit` tool: replace one exact run of text, or refuse and change nothing.
1241
///
1242
/// The two refusals are the design, and they are pi's. Text that is not there
1243
/// cannot be replaced, and text that is there more than once does not say
1244
/// which one was meant — so the model is told the count and asked to add
1245
/// context until the match is unique. That single rule is what makes a
1246
/// surgical edit safe without a diff format. Both refusals return before the
1247
/// write, so a refused edit leaves the file exactly as it was.
1248
fn answer_edit(cwd: &Path, arguments: &serde_json::Value) -> (String, bool) {
1249
    let raw = arguments.get("path").and_then(|v| v.as_str()).unwrap_or("");
1250
    let old = arguments
1251
        .get("oldText")
1252
        .and_then(|v| v.as_str())
1253
        .unwrap_or("");
1254
    let new = arguments
1255
        .get("newText")
1256
        .and_then(|v| v.as_str())
1257
        .unwrap_or("");
1258
    if old.is_empty() {
1259
        return (
1260
            "Nothing was changed: `oldText` is required and must be the exact text to replace."
1261
                .to_string(),
1262
            true,
1263
        );
1264
    }
1265
    let path = match resolve_in_cwd(cwd, raw) {
1266
        Ok(path) => path,
1267
        Err(refusal) => return (refusal, true),
1268
    };
1269
    let content = match fs::read_to_string(&path) {
1270
        Ok(content) => content,
1271
        Err(error) => return (format!("Could not read `{raw}` to edit it: {error}."), true),
1272
    };
1273
1274
    match content.matches(old).count() {
1275
        0 => (
1276
            format!(
1277
                "Nothing was changed: that `oldText` does not appear in {raw}. It has to match \
1278
                 byte for byte, whitespace and newlines included."
1279
            ),
1280
            true,
1281
        ),
1282
        1 => match write_atomically(&path, &content.replacen(old, new, 1)) {
1283
            Ok(()) => (
1284
                format!(
1285
                    "Replaced {} bytes with {} in {}.",
1286
                    old.len(),
1287
                    new.len(),
1288
                    path.display()
1289
                ),
1290
                false,
1291
            ),
1292
            Err(error) => (format!("Could not write `{raw}`: {error}."), true),
1293
        },
1294
        hits => (
1295
            format!(
1296
                "Nothing was changed: that `oldText` appears {hits} times in {raw} and it has to \
1297
                 appear exactly once. Add the lines above and below the one you mean until the \
1298
                 match is unique, then call again."
1299
            ),
1300
            true,
1301
        ),
1302
    }
1303
}
1304
1002 1305
/// As [`run_real_shell`]: the text, and whether it worked. A CLI that could
1003 1306
/// not even be spawned was previously reported to the model as a success.
1004 1307
/// Where the program behind the `openagents` tool came from.

@@ -1382,9 +1685,19 @@ mod tests {

1382 1685
        let names: Vec<String> = registry.list_tools().into_iter().map(|t| t.name).collect();
1383 1686
        assert_eq!(
1384 1687
            names,
1385
            vec!["shell", "skill", "openagents", "capability", "delegate"]
1688
            vec![
1689
                "read",
1690
                "write",
1691
                "edit",
1692
                "bash",
1693
                "shell",
1694
                "skill",
1695
                "openagents",
1696
                "capability",
1697
                "delegate"
1698
            ]
1386 1699
        );
1387
        // The same five names `plugins::validate_manifest` refuses a plugin
1700
        // The same nine names `plugins::validate_manifest` refuses a plugin
1388 1701
        // for taking. An arm added here and not there would leave a name a
1389 1702
        // plugin can claim and never be called under.
1390 1703
        assert_eq!(

@@ -1433,7 +1746,20 @@ mod tests {

1433 1746
        registry.add_host_tool(host_tool("acp", "handed off")).unwrap();
1434 1747
1435 1748
        let names: Vec<String> = registry.list_tools().into_iter().map(|t| t.name).collect();
1436
        assert_eq!(names, vec!["shell", "skill", "openagents", "capability", "acp"]);
1749
        assert_eq!(
1750
            names,
1751
            vec![
1752
                "read",
1753
                "write",
1754
                "edit",
1755
                "bash",
1756
                "shell",
1757
                "skill",
1758
                "openagents",
1759
                "capability",
1760
                "acp"
1761
            ]
1762
        );
1437 1763
1438 1764
        let out = registry
1439 1765
            .execute_tool(&ToolCall {

@@ -1678,4 +2004,266 @@ mod defect_tests {

1678 2004
        assert!(failed, "a timeout must be an error, got: {text}");
1679 2005
        assert!(text.contains("timed out"), "{text}");
1680 2006
    }
2007
2008
    // ─────────────────────────────────────────── read, write, edit and bash
2009
2010
    fn file_call(name: &str, arguments: serde_json::Value) -> ToolCall {
2011
        ToolCall {
2012
            id: "1".to_string(),
2013
            name: name.to_string(),
2014
            arguments,
2015
        }
2016
    }
2017
2018
    /// The rule that makes a surgical edit safe without a diff format: text
2019
    /// appearing more than once does not say which one was meant, so the edit
2020
    /// is refused — and, the half that matters, the file is left alone. An
2021
    /// `edit` that replaced the first hit would silently change the wrong line
2022
    /// and report success.
2023
    #[tokio::test]
2024
    async fn an_edit_whose_text_appears_twice_is_refused_and_the_file_is_unchanged() {
2025
        let dir = tempfile::tempdir().unwrap();
2026
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2027
        let before = "let x = 1;\nlet y = 2;\nlet x = 1;\n";
2028
        std::fs::write(dir.path().join("a.rs"), before).unwrap();
2029
2030
        let out = registry
2031
            .execute_tool(&file_call(
2032
                "edit",
2033
                serde_json::json!({"path": "a.rs", "oldText": "let x = 1;", "newText": "let x = 9;"}),
2034
            ))
2035
            .await;
2036
2037
        assert!(out.is_error, "a refused edit is a failure: {}", out.output);
2038
        assert!(
2039
            out.output.contains("appears 2 times"),
2040
            "the refusal must say how many: {}",
2041
            out.output
2042
        );
2043
        assert!(
2044
            out.output.contains("unique"),
2045
            "the refusal must say what to do about it: {}",
2046
            out.output
2047
        );
2048
        assert_eq!(
2049
            std::fs::read_to_string(dir.path().join("a.rs")).unwrap(),
2050
            before,
2051
            "a refused edit wrote to the file"
2052
        );
2053
    }
2054
2055
    /// The other refusal. Text that is not there cannot be replaced, and the
2056
    /// model needs to be told that rather than handed an empty result.
2057
    #[tokio::test]
2058
    async fn an_edit_whose_text_is_not_there_is_refused_and_the_file_is_unchanged() {
2059
        let dir = tempfile::tempdir().unwrap();
2060
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2061
        let before = "one\ntwo\n";
2062
        std::fs::write(dir.path().join("a.txt"), before).unwrap();
2063
2064
        let out = registry
2065
            .execute_tool(&file_call(
2066
                "edit",
2067
                serde_json::json!({"path": "a.txt", "oldText": "three", "newText": "four"}),
2068
            ))
2069
            .await;
2070
2071
        assert!(out.is_error, "{}", out.output);
2072
        assert!(!out.output.trim().is_empty(), "a refusal is never silent");
2073
        assert!(out.output.contains("does not appear"), "{}", out.output);
2074
        assert_eq!(
2075
            std::fs::read_to_string(dir.path().join("a.txt")).unwrap(),
2076
            before
2077
        );
2078
    }
2079
2080
    /// The success path, and the one thing about it worth pinning: only the
2081
    /// matched run changes.
2082
    #[tokio::test]
2083
    async fn a_unique_edit_replaces_that_run_and_nothing_else() {
2084
        let dir = tempfile::tempdir().unwrap();
2085
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2086
        std::fs::write(dir.path().join("a.txt"), "keep\nchange me\nkeep\n").unwrap();
2087
2088
        let out = registry
2089
            .execute_tool(&file_call(
2090
                "edit",
2091
                serde_json::json!({"path": "a.txt", "oldText": "change me", "newText": "changed"}),
2092
            ))
2093
            .await;
2094
2095
        assert!(!out.is_error, "{}", out.output);
2096
        assert_eq!(
2097
            std::fs::read_to_string(dir.path().join("a.txt")).unwrap(),
2098
            "keep\nchanged\nkeep\n"
2099
        );
2100
    }
2101
2102
    /// A missing file is a refusal the model reads and retries from, not an
2103
    /// unwrap that takes the process with it.
2104
    #[tokio::test]
2105
    async fn reading_a_file_that_is_not_there_is_a_refusal_and_not_a_raise() {
2106
        let dir = tempfile::tempdir().unwrap();
2107
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2108
2109
        let out = registry
2110
            .execute_tool(&file_call(
2111
                "read",
2112
                serde_json::json!({"path": "missing.txt"}),
2113
            ))
2114
            .await;
2115
2116
        assert!(out.is_error, "a missing file is a failure: {}", out.output);
2117
        assert!(
2118
            out.output.contains("missing.txt"),
2119
            "the refusal must name the path: {}",
2120
            out.output
2121
        );
2122
        assert!(!out.output.trim().is_empty(), "a refusal is never silent");
2123
    }
2124
2125
    /// `shell` reported every failure as a success until recently, so a
2126
    /// failing build read like a passing one. `bash` is the same arm, and this
2127
    /// holds it to the same answer through the tool interface.
2128
    #[tokio::test]
2129
    async fn a_bash_command_that_exits_non_zero_is_reported_as_an_error() {
2130
        let dir = tempfile::tempdir().unwrap();
2131
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2132
2133
        let failed = registry
2134
            .execute_tool(&file_call("bash", serde_json::json!({"command": "exit 3"})))
2135
            .await;
2136
        assert!(
2137
            failed.is_error,
2138
            "a non-zero exit must be an error: {}",
2139
            failed.output
2140
        );
2141
        assert!(failed.output.contains('3'), "{}", failed.output);
2142
2143
        let worked = registry
2144
            .execute_tool(&file_call(
2145
                "bash",
2146
                serde_json::json!({"command": "echo ok"}),
2147
            ))
2148
            .await;
2149
        assert!(!worked.is_error, "{}", worked.output);
2150
        assert!(worked.output.contains("ok"), "{}", worked.output);
2151
    }
2152
2153
    /// Nothing reaches a path outside the session's working directory, and the
2154
    /// refusal says so rather than failing somewhere further down with an
2155
    /// error about permissions.
2156
    #[tokio::test]
2157
    async fn a_path_outside_the_working_directory_is_refused_by_every_file_tool() {
2158
        let outside = tempfile::tempdir().unwrap();
2159
        let secret = outside.path().join("secret.txt");
2160
        std::fs::write(&secret, "not yours\n").unwrap();
2161
2162
        let dir = tempfile::tempdir().unwrap();
2163
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2164
2165
        for arguments in [
2166
            serde_json::json!({"path": "../escaped.txt", "content": "x", "oldText": "a", "newText": "b"}),
2167
            serde_json::json!({"path": secret.display().to_string(), "content": "x", "oldText": "not yours", "newText": "mine"}),
2168
        ] {
2169
            for tool in ["read", "write", "edit"] {
2170
                let out = registry
2171
                    .execute_tool(&file_call(tool, arguments.clone()))
2172
                    .await;
2173
                assert!(out.is_error, "`{tool}` did not refuse: {}", out.output);
2174
                assert!(
2175
                    out.output
2176
                        .contains("outside this session's working directory"),
2177
                    "`{tool}` refused without saying why: {}",
2178
                    out.output
2179
                );
2180
            }
2181
        }
2182
2183
        assert_eq!(
2184
            std::fs::read_to_string(&secret).unwrap(),
2185
            "not yours\n",
2186
            "a refused write reached outside the working directory"
2187
        );
2188
    }
2189
2190
    /// `write` creates the parents, and it stages and renames — so the only
2191
    /// thing left in the directory afterwards is the file itself. A reader
2192
    /// arriving mid-write sees the old file or the new one, never a truncated
2193
    /// one (#114).
2194
    #[tokio::test]
2195
    async fn a_write_creates_parents_and_leaves_no_staged_file_behind() {
2196
        let dir = tempfile::tempdir().unwrap();
2197
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2198
2199
        let out = registry
2200
            .execute_tool(&file_call(
2201
                "write",
2202
                serde_json::json!({"path": "deep/er/a.txt", "content": "hello\n"}),
2203
            ))
2204
            .await;
2205
        assert!(!out.is_error, "{}", out.output);
2206
2207
        let home = dir.path().join("deep").join("er");
2208
        assert_eq!(
2209
            std::fs::read_to_string(home.join("a.txt")).unwrap(),
2210
            "hello\n"
2211
        );
2212
        let left: Vec<String> = std::fs::read_dir(&home)
2213
            .unwrap()
2214
            .map(|entry| entry.unwrap().file_name().to_string_lossy().to_string())
2215
            .collect();
2216
        assert_eq!(left, vec!["a.txt"], "a staged file was left behind");
2217
    }
2218
2219
    /// The rename itself, which is the part that closes the window: the
2220
    /// destination is a different file afterwards, so no reader ever held a
2221
    /// descriptor on a truncated one. Writing in place keeps the same inode
2222
    /// and passes every other assertion here (#114).
2223
    #[cfg(unix)]
2224
    #[test]
2225
    fn a_write_replaces_the_file_by_rename_rather_than_truncating_it() {
2226
        use std::os::unix::fs::MetadataExt;
2227
2228
        let dir = tempfile::tempdir().unwrap();
2229
        let path = dir.path().join("a.txt");
2230
        std::fs::write(&path, "old\n").unwrap();
2231
        let before = std::fs::metadata(&path).unwrap().ino();
2232
2233
        write_atomically(&path, "new\n").unwrap();
2234
2235
        assert_eq!(std::fs::read_to_string(&path).unwrap(), "new\n");
2236
        assert_ne!(
2237
            before,
2238
            std::fs::metadata(&path).unwrap().ino(),
2239
            "the file was truncated in place instead of staged and renamed"
2240
        );
2241
    }
2242
2243
    /// The bounded cut in `read`, on the same defect the shell one was fixed
2244
    /// for: `&content[..OUTPUT_LIMIT]` panics when a multi-byte character
2245
    /// straddles the limit, and it takes the whole agent process with it.
2246
    #[tokio::test]
2247
    async fn reading_a_file_with_a_multibyte_character_on_the_limit_does_not_panic() {
2248
        let dir = tempfile::tempdir().unwrap();
2249
        let registry = HarnessToolRegistry::new(Some(dir.path().to_path_buf()));
2250
        let mut body = "a".repeat(OUTPUT_LIMIT - 1);
2251
        body.push_str("€€€");
2252
        assert!(
2253
            !body.is_char_boundary(OUTPUT_LIMIT),
2254
            "the probe must straddle"
2255
        );
2256
        std::fs::write(dir.path().join("big.txt"), &body).unwrap();
2257
2258
        let out = registry
2259
            .execute_tool(&file_call("read", serde_json::json!({"path": "big.txt"})))
2260
            .await;
2261
2262
        assert!(!out.is_error, "{}", out.output);
2263
        assert!(out.output.contains("[Output truncated"), "it was not cut");
2264
        assert!(
2265
            out.output.starts_with(&body[..OUTPUT_LIMIT - 1]),
2266
            "the cut did not step back to the boundary"
2267
        );
2268
    }
1681 2269
}
crates/openagents-cli/tests/coder-surfaces-golden.json modified +5 -1

@@ -2,9 +2,13 @@

2 2
  "system_prompt.local.no_tools": "You are `openagents coder`, a coding assistant in a terminal. You answer from a model running on this machine through Ollama. Nothing in this conversation leaves the machine and nothing is metered, but the context window is a fraction of a hosted model's, so keep large dumps out of the transcript.\n\nAnswer very concisely unless the reader asks for a longer response.\n\nYou have no tools in this session: you cannot read or write files, run commands, or reach anything outside this conversation. Answer from what the reader tells you, and say plainly when something would need a tool you do not have.",
3 3
  "system_prompt.thread.no_tools": "You are `openagents coder`, a coding assistant in a terminal. You answer through the OpenAgents inference proxy, on a thread opened for this session. Every round of tool calls re-sends the whole conversation to a metered model, so batch independent commands into one call and keep large dumps out of the transcript.\n\nAnswer very concisely unless the reader asks for a longer response.\n\nYou have no tools in this session: you cannot read or write files, run commands, or reach anything outside this conversation. Answer from what the reader tells you, and say plainly when something would need a tool you do not have.",
4 4
  "system_prompt.thread.two_tools": "You are `openagents coder`, a coding assistant in a terminal. You answer through the OpenAgents inference proxy, on a thread opened for this session. Every round of tool calls re-sends the whole conversation to a metered model, so batch independent commands into one call and keep large dumps out of the transcript.\n\nAnswer very concisely unless the reader asks for a longer response.\n\nYou have 2 tools, and no others:\n- `alpha`\n- `beta`\n\nThat list is complete: a capability not on it is one you do not have, whatever a model like you usually has. Read a tool's description before assuming what it covers. Where a description says what a child agent can do, that is the child's capability and not yours. Never say you ran something you did not run.",
5
  "tool.bash": "Run a command through `/bin/sh -c` in the session's working directory, /fixed/cwd. Returns combined stdout and stderr, and reports a non-zero exit as a failure rather than as ordinary output. Batch independent commands into one call with `&&` instead of one call each: every call replays the conversation so far. This is the same runner as `shell`, and either name reaches it.",
5 6
  "tool.capability": "Discover and load installed plugin capabilities: sandboxed, digest-pinned WebAssembly programs this machine already holds for common agent work. Before writing a script for a task, search here first — a capability that covers it is bounded, reviewable, and returns structured output. Call with `query` describing what you need to get the best matches; then call again with `name` set to the exact returned name to load it and make its dedicated tool available. Every later call to the loaded capability uses that exact name as the tool name.",
6 7
  "tool.delegate": "Run one prompt on independent child coding agents in parallel and return what each one found or did. Use it when work splits into parts that do not depend on each other: several files to change the same way, several hypotheses to check, several tests to run down. Each child is a full coding agent with its own shell tool, working in a git worktree of its own so children cannot overwrite each other, and it starts with no context from this conversation and cannot ask questions — so the prompt has to be self-contained. Every child runs the same prompt and each is told separately which number it is, so write the prompt for whichever child reads it: say \"read the file at your own number\" rather than naming one child. Children run on the parent lane and on this session's budget. Prefer one call with a count over several calls, and prefer `shell` over this for a single command — a child agent is for work worth a whole agent, not one line of output. At most 8 children.",
8
  "tool.edit": "Replace one exact run of text in a file. `oldText` must match byte for byte, whitespace and newlines included, and must appear exactly once. If it appears more than once the edit is refused and the file is left alone: add the lines above and below the one you mean until the match is unique, then call again. `newText` may be empty to delete the run. Use this rather than `sed` for a surgical change.",
7 9
  "tool.openagents": "Run the OpenAgents CLI commands (issue, project, repo, auth, etc.). Pass the arguments as a list without openagents itself.",
10
  "tool.read": "Read a file and return its whole contents as text. `path` is relative to the session's working directory, or an absolute path inside it; a path that resolves outside it is refused. Use this rather than `cat` through the shell — there is nothing to quote and nothing to escape.",
8 11
  "tool.shell": "Run a shell command on this machine. The working directory is /fixed/cwd, so paths are relative to it and you do not need to ask where you are. Returns combined stdout and stderr with the exit code. Batch independent commands into one call with && instead of one call each: every call replays the conversation so far.",
9
  "tool.skill": "Read one of this repository skill procedures: a written procedure with conventions, commands, and rules. Call it before doing work a skill covers. Skills available:"
12
  "tool.skill": "Read one of this repository skill procedures: a written procedure with conventions, commands, and rules. Call it before doing work a skill covers. Skills available:",
13
  "tool.write": "Write `content` to `path`, creating parent directories and replacing any file already there. The file is staged beside its destination and renamed into place, so nothing else on this machine can read it half-written. Use this rather than a shell heredoc. To change part of a file use `edit` instead: it does not ask you to reproduce the rest."
10 14
}
crates/openagents-cli/tests/plugin_host_test.rs modified +11 -1

@@ -527,7 +527,17 @@ async fn a_plugin_named_after_a_builtin_is_never_declared_and_every_declared_too

527 527
    let names: Vec<String> = registry.list_tools().into_iter().map(|t| t.name).collect();
528 528
    assert_eq!(
529 529
        names,
530
        vec!["shell", "skill", "openagents", "capability", "word_count"],
530
        vec![
531
            "read",
532
            "write",
533
            "edit",
534
            "bash",
535
            "shell",
536
            "skill",
537
            "openagents",
538
            "capability",
539
            "word_count"
540
        ],
531 541
        "the declared list is not the set of tools that can be called"
532 542
    );
533 543
packages/openagents-cli/src/coder-surfaces.generated.ts modified +1 -1

@@ -40,7 +40,7 @@ export const TOOL_DESCRIPTION_SURFACE = {

40 40
 */
41 41
export const CODER_SURFACE_DIGESTS = {
42 42
  "system-prompt": "sha256:8f0fbbbb38f4ce609a09fe2ba4e03ca160ff2c0d0f698dd4676c2bf940ded849",
43
  "tool-descriptions": "sha256:513c7518c72ba8c9959e517ea9f1262d7d3ee6b08452ddb48568bdabf6383c1b",
43
  "tool-descriptions": "sha256:f85ed27173e41eb6752cc6232600ff17b02ea06a41b59383b1963093459b1eca",
44 44
  "catalog-lines": "sha256:98a96bcf6acfd5847bc1bddcc575761af71be7ea144f3584b3594b24b2f80911",
45 45
} as const;
46 46
surfaces/coder/index.json modified +2 -2

@@ -10,8 +10,8 @@

10 10
    "tool-descriptions": {
11 11
      "file": "tool-descriptions.v1.json",
12 12
      "schema": "openagents.coder_surface.tool_descriptions.v1",
13
      "keys": 18,
14
      "digest": "sha256:513c7518c72ba8c9959e517ea9f1262d7d3ee6b08452ddb48568bdabf6383c1b"
13
      "keys": 22,
14
      "digest": "sha256:f85ed27173e41eb6752cc6232600ff17b02ea06a41b59383b1963093459b1eca"
15 15
    },
16 16
    "catalog-lines": {
17 17
      "file": "catalog-lines.v1.json",
surfaces/coder/tool-descriptions.v1.json modified +4

@@ -2,6 +2,10 @@

2 2
  "schema": "openagents.coder_surface.tool_descriptions.v1",
3 3
  "surface": "tool-descriptions",
4 4
  "text": {
5
    "rust.read": "Read a file and return its whole contents as text. `path` is relative to the session's working directory, or an absolute path inside it; a path that resolves outside it is refused. Use this rather than `cat` through the shell — there is nothing to quote and nothing to escape.",
6
    "rust.write": "Write `content` to `path`, creating parent directories and replacing any file already there. The file is staged beside its destination and renamed into place, so nothing else on this machine can read it half-written. Use this rather than a shell heredoc. To change part of a file use `edit` instead: it does not ask you to reproduce the rest.",
7
    "rust.edit": "Replace one exact run of text in a file. `oldText` must match byte for byte, whitespace and newlines included, and must appear exactly once. If it appears more than once the edit is refused and the file is left alone: add the lines above and below the one you mean until the match is unique, then call again. `newText` may be empty to delete the run. Use this rather than `sed` for a surgical change.",
8
    "rust.bash": "Run a command through `/bin/sh -c` in the session's working directory, {cwd}. Returns combined stdout and stderr, and reports a non-zero exit as a failure rather than as ordinary output. Batch independent commands into one call with `&&` instead of one call each: every call replays the conversation so far. This is the same runner as `shell`, and either name reaches it.",
5 9
    "rust.shell": "Run a shell command on this machine. The working directory is {cwd}, so paths are relative to it and you do not need to ask where you are. Returns combined stdout and stderr with the exit code. Batch independent commands into one call with && instead of one call each: every call replays the conversation so far.",
6 10
    "rust.skill": "Read one of this repository skill procedures: a written procedure with conventions, commands, and rules. Call it before doing work a skill covers. Skills available:{skills}",
7 11
    "rust.openagents": "Run the OpenAgents CLI commands (issue, project, repo, auth, etc.). Pass the arguments as a list without openagents itself.",

This page updates live while a promote is in flight · changelog