Audit of 57324fe

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.

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 {