From 5eab2597e482f27b3a65a285a2ef8ce05c3cb66f Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Tue, 20 Oct 2026 22:05:33 -0700 Subject: [PATCH] Audit of 57324fe --- docs/operations.md | 2 ++ internal/agents/clients.go | 54 ++++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index ca99dac..3735d32 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -432,6 +432,8 @@ For commands that don't need the human to see them, agents have the `bash` MCP t ### File tools +Client shell requests from `client_bash`, `client_open` and the installed byre's file commands share `Manager.runClientBash` (`internal/agents/clients.go`): it applies the command timeout plus the reply grace, sends the request and unwraps its exit code and output. The callers decide how to report a nonzero exit. + Beside `bash`, agents get `read`, `write`, `edit`, `grep` and `list` MCP tools (`internal/agents/files.go`), which byre carries out itself, as the service user and outside the agent's sandbox, the same set the client Macs get as `client_read`, `client_write`, `client_edit`, `client_grep` and `client_list`. A path is relative to the task's worktree, absolute, or starts with `~/` for the service user's home; nothing confines it to the worktree, as nothing confines `bash`. `read` and `edit` share the client tools' code: images come back as images, text as numbered lines, and `edit` refuses unless `old_string` appears exactly once. All three refuse a path that isn't a regular file (a FIFO would block), and `read` and `edit` refuse a file over 64 MiB, as `client_read` does. `write` creates parent directories and writes in place, so an existing file keeps its permissions. `grep` and `client_grep` run the same search, `internal/grep`, in-process on the server and as `byre grep [--glob GLOB] [--ignore-case] [--files-only] PATTERN PATH` on a client, from the `byre` binary every Byre.app carries. The pattern is Go RE2, matched line by line as each file streams through, so memory stays bounded; a line over 1 MiB isn't searched and prints as `path:line: line over 1 MiB, not searched`, and the deadline is checked before every read. A directory is walked recursively, without following symlinks, skipping `.git` entries, binary files (a NUL byte in the first 8000 bytes), unreadable files and whatever gitignore rules exclude: the repository's `info/exclude` (in a linked worktree, whose `.git` is a file, the one in the shared git directory its `gitdir` and `commondir` lead to) and every `.gitignore` from the repository root (the nearest ancestor holding `.git`, or the searched directory outside a repository) down, matched with go-git's `plumbing/format/gitignore`. The global `core.excludesFile` is not read. A `path` that is itself ignored, or inside an ignored directory, is searched anyway, minus what `.gitignore` files inside it exclude. `--glob` matches file names only. Matches print as `path:line:text`, the path as given joined with the file's path below it, lines cut at 500 bytes, and output stops at 64 KiB with a closing line saying so; no match prints `No matches.`. The server `grep` gives up after 2 minutes. diff --git a/internal/agents/clients.go b/internal/agents/clients.go index 56b94ad..afb0077 100644 --- a/internal/agents/clients.go +++ b/internal/agents/clients.go @@ -277,21 +277,15 @@ func (m *Manager) clientBash(ctx context.Context, raw json.RawMessage) (string, return "", fmt.Errorf("timeout_ms must be between 1 and %d", maxBashTimeout.Milliseconds()) } - ms := int(timeout.Milliseconds()) - cctx, cancel := context.WithTimeout(ctx, timeout+replyGrace) - defer cancel() - reply, err := m.clients.Call(cctx, args.Client, api.ClientRequest{Op: api.Bash, Command: &args.Command, TimeoutMs: &ms}) + exitCode, output, err := m.runClientBash(ctx, args.Client, args.Command, timeout) if err != nil { return "", err } - if reply.ExitCode == nil || reply.Output == nil { - return "", fmt.Errorf("%s's reply has no exit_code or output", args.Client) - } - status := fmt.Sprintf("exit status %d", *reply.ExitCode) - if *reply.ExitCode == -1 { + status := fmt.Sprintf("exit status %d", exitCode) + if exitCode == -1 { status = fmt.Sprintf("killed after %s", timeout) } - return status + "\n" + *reply.Output, nil + return status + "\n" + output, nil } type clientTransferArgs struct { @@ -371,19 +365,13 @@ func (m *Manager) clientOpen(ctx context.Context, l *live, raw json.RawMessage) return "", err } - command := "open " + dir + "/'" + strings.ReplaceAll(name, "'", `'\''`) + "'" - ms := int(fileTimeout.Milliseconds()) - cctx, cancel := context.WithTimeout(ctx, fileTimeout+replyGrace) - defer cancel() - reply, err := m.clients.Call(cctx, args.Client, api.ClientRequest{Op: api.Bash, Command: &command, TimeoutMs: &ms}) + command := "open " + dir + "/" + shellQuote(name) + exitCode, output, err := m.runClientBash(ctx, args.Client, command, fileTimeout) if err != nil { return "", err } - if reply.ExitCode == nil || reply.Output == nil { - return "", fmt.Errorf("%s's reply has no exit_code or output", args.Client) - } - if *reply.ExitCode != 0 { - return "", fmt.Errorf("copied %s to %s on %s, but open exited %d: %s", clean, dest, args.Client, *reply.ExitCode, *reply.Output) + if exitCode != 0 { + return "", fmt.Errorf("copied %s to %s on %s, but open exited %d: %s", clean, dest, args.Client, exitCode, output) } return fmt.Sprintf("Opened %s on %s, copied to %s.", clean, args.Client, dest), nil } @@ -616,20 +604,28 @@ func clientShellPath(p string) (string, error) { func (m *Manager) runClientByre(ctx context.Context, client, sub, args string) (string, error) { command := clientByre + " " + sub + args - ms := int(fileTimeout.Milliseconds()) - cctx, cancel := context.WithTimeout(ctx, fileTimeout+replyGrace) - defer cancel() - reply, err := m.clients.Call(cctx, client, api.ClientRequest{Op: api.Bash, Command: &command, TimeoutMs: &ms}) + exitCode, output, err := m.runClientBash(ctx, client, command, fileTimeout) if err != nil { return "", err } + if exitCode != 0 { + return "", fmt.Errorf("byre %s on %s exited %d: %s", sub, client, exitCode, output) + } + return output, nil +} + +func (m *Manager) runClientBash(ctx context.Context, client, command string, timeout time.Duration) (int, string, error) { + ms := int(timeout.Milliseconds()) + cctx, cancel := context.WithTimeout(ctx, timeout+replyGrace) + defer cancel() + reply, err := m.clients.Call(cctx, client, api.ClientRequest{Op: api.Bash, Command: &command, TimeoutMs: &ms}) + if err != nil { + return 0, "", err + } if reply.ExitCode == nil || reply.Output == nil { - return "", fmt.Errorf("%s's reply has no exit_code or output", client) + return 0, "", fmt.Errorf("%s's reply has no exit_code or output", client) } - if *reply.ExitCode != 0 { - return "", fmt.Errorf("byre %s on %s exited %d: %s", sub, client, *reply.ExitCode, *reply.Output) - } - return *reply.Output, nil + return *reply.ExitCode, *reply.Output, nil } type clientHandsArgs struct {