From 6ac297d374361d2bec7c3f29d5a487696a48e62a Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Mon, 19 Oct 2026 19:46:24 -0700 Subject: [PATCH] Show images in Files and add client_open to open files on a client Mac --- docs/operations.md | 5 ++- internal/agents/clients.go | 70 ++++++++++++++++++++++++++++++++--- mac/Sources/Byre/Files.swift | 72 ++++++++++++++++++++++++++++++++++-- 3 files changed, 137 insertions(+), 10 deletions(-) diff --git a/docs/operations.md b/docs/operations.md index b8ade38..cf14be3 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -371,11 +371,12 @@ Answers reach the agent as one message, listing each answer under the question i Task agents use the Macs the human has byre's app open on directly, with no approval, as they use the machine byre runs on. While it runs, the app (`mac/Sources/Byre/Remote.swift`, started with the global event stream and stopped with it) keeps a WebSocket open to `GET /api/clients/connect`, reconnecting 2s after it drops. The connection is keyed by its `Byre-Client` header and named by its `Byre-Host` header (both required); a newer connection with the same `Byre-Client` replaces the older one, while two apps open on one Mac each keep their own. The client tools take the host name, or the `id` from `list_clients`, which they need when more than one app is connected from that host (the call fails listing the ids). byre keeps the connections in memory (`internal/clients`), pinging each every 30s and dropping one that doesn't answer within 15s. The app pings too, every 10s, and reconnects when it has had no pong for 25s. That catches a socket byre dropped while the Mac slept: the close never reached the Mac, and without something sent on it the app would wait on it indefinitely. Over it byre sends `ClientRequest` text messages, each with a fresh `id`, and the app answers each with a `ClientReply` carrying that `id` (`error` when it couldn't carry it out), in any order, handling requests concurrently. A request whose client disconnects before replying fails with " disconnected before replying". -Implementer sessions get eight MCP tools for it (`internal/agents/clients.go`). The app sets its WebSocket's `maximumMessageSize` to 256 MiB, the same as byre's read limit, so file contents fit in one message each way. +Implementer sessions get nine MCP tools for it (`internal/agents/clients.go`). The app sets its WebSocket's `maximumMessageSize` to 256 MiB, the same as byre's read limit, so file contents fit in one message each way. - `list_clients`: the status's `clients` (see "Status"), with each one's host, login, `id`, connection time, app commit and `last_action_at`; the most recently active is listed first and marked. - `client_bash`: `op: bash`. The app runs `/bin/zsh -l -c ` in the human's home directory through `Process`, which starts it in its own process group, stdin from `/dev/null` and stdout and stderr into one temporary file, and kills the group with `SIGKILL` after `timeout_ms` (default 120000, at most 600000), replying `exit_code` -1. Otherwise `exit_code` is the exit status, or 128 plus the signal. `output` is the file's last 64 KiB. The agent's call waits the timeout plus 30s for the reply. Background children the command leaves are not waited for or killed, and a command that quits the app keeps running. - `client_upload`: `op: upload`. `path` is checked against the worktree through an `os.Root`; byre registers a transfer naming the path's directory and base name under a random id for as long as the call waits (at most 10 minutes), and the app downloads `GET /api/clients/transfers/{transferId}`, which streams `tar -czf - -C -- ` (404 once the call has ended). The app unpacks it into a new `.byre-upload-` directory beside `dest` (`~/` expanded, parent directories created), removes whatever is at `dest`, moves the archive's one entry there and removes the temporary directory. +- `client_open`: a `client_upload` of `path` to `~/Library/Caches/byre/open//`, then an `op: bash` running `open` on it, so the file opens in that Mac's default app for its type. The folder is left in place for the human to keep or for macOS to clear. - `client_download`: `op: download`, the reverse. byre makes `dest`'s parent directories through an `os.Root` on the worktree and registers a receiving transfer (`Transfer.Receive`) for a fresh temporary directory; the app runs `tar -czf -C -- ` and `PUT`s it to `/api/clients/transfers/{transferId}`, which writes it once (`O_EXCL`) and answers 204, and 404 for a sending transfer, one already received or one whose call has ended (`GET` likewise answers 404 for a receiving one). byre unpacks it into a new `.byre-download-` directory beside `dest`, removes whatever is at `dest`, renames the archive's one entry there and removes the temporary directories, all inside the `os.Root`. - `client_read`: `op: read_file`; the app replies with the file's bytes in `data`, reading on a detached task, off the main actor. It resolves symlinks, refuses anything but a regular file (a FIFO would block the open), and reads at most 64 MiB plus one byte, refusing a longer file, so the base64 reply stays well under byre's 256 MiB read limit, past which byre would drop the connection. A `.png`, `.jpg`, `.jpeg`, `.gif` or `.webp` path comes back as an image result; other files must be UTF-8 and come back as `cat -n` style numbered lines from `offset` (default 1) for `limit` lines (default 2000), with a closing line naming the next offset when lines remain. - `client_write`: `op: write_file` with `path` and `data`; on a detached task, off the main actor, the app resolves symlinks, refuses an existing path that isn't a regular file (a FIFO would block the write), creates the parent directories and writes the file in place (`Data.write` without `.atomic`), so an existing file keeps its permissions. @@ -406,7 +407,7 @@ The Changes view lists the files with their counts, opens a file's diff on click The Files tab browses the selected task's worktree, read-only (`internal/server/files.go`). `GET /api/tasks/{taskId}/dir?path=` lists one directory, directories first and then by name, without entries named `.git`; `GET /api/tasks/{taskId}/file?path=` returns one regular file. Both take a slash-separated path relative to the worktree root and work only while the task is active (409 otherwise). Every path is opened through an `os.Root` on the worktree, so a path that leaves it, through `..`, an absolute path or a symlink at any depth, is a 400; a path with a `.git` component is also refused. A listing reports each entry as `dir`, `file` (with its size) or `other`, following symlinks that stay inside the worktree (`symlink` marks them); a symlink that leaves the worktree or points nowhere is `other` and can't be opened. Gitignored entries are listed with `ignored` set: one `git check-ignore --stdin` per listing (`git.Ignored`) over the entries' paths, taken from the directory's real path so a listing reached through an in-worktree symlink is checked where it really lives; untracked paths under an ignored directory count as ignored, tracked files never do. The view shows them dimmed with `ignored` beside them, and they open like anything else. A listing stops at `maxDirEntries` entries with `truncated` set. A file read sends at most `maxFileBytes`, cut back to a whole UTF-8 character, with `truncated` set when the file is longer; a file with a NUL byte or invalid UTF-8 in that part is `binary` and comes back with no text. -The view shows a breadcrumb from `worktree` down to the current directory or file, the directory's entries (a `../` row to go up), or the file's text with line numbers and a note when only its start is shown. A file whose name ends in `.md` is rendered as Markdown with the transcript's renderer (`MarkdownView`) instead, with relative image paths taken from the file's own directory. Where you are is kept per task in memory (`Store.filePlaces`), so switching tasks or tabs returns to it, but a relaunch starts at the root. It refetches what it shows 1.5s after the selected task's last tool result or turn end, keeping the scroll position. +The view shows a breadcrumb from `worktree` down to the current directory or file, the directory's entries (a `../` row to go up), or the file's text with line numbers and a note when only its start is shown. A file whose name ends in `.png`, `.jpg`, `.jpeg`, `.gif`, `.webp` or `.svg` is shown as an image, fetched whole from `GET /api/tasks/{taskId}/image` (see "Screenshots from implementers") again on each refetch, fitted to the pane width and opening full size on click. A file whose name ends in `.md` is rendered as Markdown with the transcript's renderer (`MarkdownView`) instead, with relative image paths taken from the file's own directory. Where you are is kept per task in memory (`Store.filePlaces`), so switching tasks or tabs returns to it, but a relaunch starts at the root. It refetches what it shows 1.5s after the selected task's last tool result or turn end, keeping the scroll position. Markdown rendered with a task (its transcript, questions, reviews, Markdown files in Files) links the worktree paths it mentions to Files (`InlineRenderer` in `mac/Sources/Byre/MarkdownView.swift`): a relative Markdown link (`[x](docs/a.md)`, from the file's own directory inside Files), a code span holding a path with a `/` that ends in a file name with an extension or in `/` (`` `mac/Sources/` ``), and the same kind of path in plain text, file names only. An absolute path counts when it starts with the task's worktree. A `:line`, `:line:col` or `#L12` suffix is dropped. The link is `byre-app://file?task=&path=`, which sets the task's place in Files to that file (or directory, for a trailing `/`) and focuses the task's Files tab. Nothing checks that the path exists; one that doesn't opens Files on its 404. diff --git a/internal/agents/clients.go b/internal/agents/clients.go index 5bf6843..bd80c2b 100644 --- a/internal/agents/clients.go +++ b/internal/agents/clients.go @@ -2,6 +2,7 @@ package agents import ( "context" + "crypto/rand" "encoding/base64" "encoding/json" "errors" @@ -77,6 +78,22 @@ func (m *Manager) clientTools(l *live) []mcp.Tool { return m.clientUpload(ctx, l, raw) }, }, + { + Name: "client_open", + Description: "Show the human a file from your worktree on a client Mac: copy it into a fresh folder under ~/Library/Caches/byre/open/ there and open it with macOS's `open`, in that Mac's default app for its type (Preview for images and PDFs, the browser for HTML). " + + "Use it when the human asks to see a file on their Mac, or for a file the Files tab can't show; it brings an app forward on their screen.", + InputSchema: json.RawMessage(`{ + "type": "object", + "properties": { + "client": {"type": "string", "description": "The client's name from list_clients, or its id when several are connected from that Mac."}, + "path": {"type": "string", "description": "The file or directory, relative to your worktree."} + }, + "required": ["client", "path"] + }`), + Handler: func(ctx context.Context, raw json.RawMessage) (string, error) { + return m.clientOpen(ctx, l, raw) + }, + }, { Name: "client_download", Description: "Copy a file or directory from a client Mac into your worktree, replacing whatever is at dest there. " + @@ -264,10 +281,17 @@ func (m *Manager) clientUpload(ctx context.Context, l *live, raw json.RawMessage if err != nil { return "", err } + clean, err := m.upload(ctx, l, args.Client, args.Path, args.Dest) + if err != nil { + return "", err + } + return fmt.Sprintf("Copied %s to %s on %s.", clean, args.Dest, args.Client), nil +} - clean := filepath.Clean(args.Path) +func (m *Manager) upload(ctx context.Context, l *live, client, path, dest string) (string, error) { + clean := filepath.Clean(path) if filepath.IsAbs(clean) { - return "", fmt.Errorf("path %q must be relative to your worktree", args.Path) + return "", fmt.Errorf("path %q must be relative to your worktree", path) } root, err := os.OpenRoot(l.dir) if err != nil { @@ -276,7 +300,7 @@ func (m *Manager) clientUpload(ctx context.Context, l *live, raw json.RawMessage _, err = root.Lstat(clean) root.Close() if err != nil { - return "", fmt.Errorf("path %q: %w", args.Path, err) + return "", fmt.Errorf("path %q: %w", path, err) } full := filepath.Join(l.dir, clean) @@ -285,11 +309,47 @@ func (m *Manager) clientUpload(ctx context.Context, l *live, raw json.RawMessage cctx, cancel := context.WithTimeout(ctx, copyTimeout) defer cancel() - _, err = m.clients.Call(cctx, args.Client, api.ClientRequest{Op: api.Upload, Transfer: &id, Dest: &args.Dest}) + _, err = m.clients.Call(cctx, client, api.ClientRequest{Op: api.Upload, Transfer: &id, Dest: &dest}) + return clean, err +} + +type clientOpenArgs struct { + Client string `json:"client"` + Path string `json:"path"` +} + +func (m *Manager) clientOpen(ctx context.Context, l *live, raw json.RawMessage) (string, error) { + args := clientOpenArgs{} + err := json.Unmarshal(raw, &args) + if err != nil { + return "", fmt.Errorf("malformed client_open call: %w", err) + } + dir := "~/Library/Caches/byre/open/" + rand.Text() + name := filepath.Base(filepath.Clean(args.Path)) + if name == "." { + return "", errors.New("path names your worktree, not a file in it") + } + dest := dir + "/" + name + clean, err := m.upload(ctx, l, args.Client, args.Path, dest) if err != nil { return "", err } - return fmt.Sprintf("Copied %s to %s on %s.", clean, args.Dest, args.Client), nil + + 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}) + 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) + } + return fmt.Sprintf("Opened %s on %s, copied to %s.", clean, args.Client, dest), nil } func (m *Manager) clientDownload(ctx context.Context, l *live, raw json.RawMessage) (string, error) { diff --git a/mac/Sources/Byre/Files.swift b/mac/Sources/Byre/Files.swift index b1e5eef..e2b7839 100644 --- a/mac/Sources/Byre/Files.swift +++ b/mac/Sources/Byre/Files.swift @@ -44,6 +44,12 @@ private func bytes(_ n: Int64) -> String { return "\((Double(n) / 1024 / 1024).formatted(.number.precision(.fractionLength(1)).grouping(.never))) MiB" } +private let imageExts: Set = ["png", "jpg", "jpeg", "gif", "webp", "svg"] + +private func isImage(_ path: String) -> Bool { + imageExts.contains((path as NSString).pathExtension.lowercased()) +} + private func shown(_ c: FileContent) -> Int64 { Int64(c.text.utf8.count) } @@ -56,6 +62,7 @@ private final class FilesModel { private(set) var content: FileContent? private(set) var error = "" private(set) var generation = 0 + private(set) var loads = 0 @ObservationIgnored private var target = 0 @ObservationIgnored private var timer: Task? @@ -104,6 +111,7 @@ private final class FilesModel { if started == target { content = fresh listing = nil + loads += 1 } } else { let fresh = try await store.api.client.getTaskDir(path: .init(taskId: taskId), query: .init(path: p.dir)).ok.body.json @@ -142,6 +150,9 @@ private final class FilesModel { return "" } if let c = content { + if place.file.map(isImage) == true { + return bytes(c.size) + } if c.binary { return "\(bytes(c.size)) · binary" } @@ -229,11 +240,14 @@ private struct FilesBody: View { EmptyText(text: "No task selected.") } else if !model.error.isEmpty { EmptyText(text: model.error, error: true) - } else if let c = model.content { - if c.binary { + } else if let c = model.content, let taskId = store.selectedTaskId { + if let file = model.place.file, isImage(file) { + FileImage(taskId: taskId, path: file, load: model.loads) + .id(model.generation) + } else if c.binary { EmptyText(text: "Binary file.") } else if model.place.file?.lowercased().hasSuffix(".md") == true { - FileMarkdown(content: c, taskId: store.selectedTaskId, dir: model.place.dir) + FileMarkdown(content: c, taskId: taskId, dir: model.place.dir) .id(model.generation) } else { FileText(content: c) @@ -340,6 +354,58 @@ private struct FileMarkdown: View { } } +private struct FileImage: View { + let taskId: String + let path: String + let load: Int + @Environment(Store.self) private var store + @Environment(Overlays.self) private var overlays + @ViewState private var image: NSImage? + @ViewState private var error = "" + + var body: some View { + Group { + if let image { + ScrollView { + Image(nsImage: image) + .resizable() + .scaledToFit() + .frame(maxWidth: image.size.width, maxHeight: image.size.height) + .overlay(Rectangle().stroke(Palette.border)) + .onTapGesture { + overlays.image = image + } + .help("Open full size") + .padding(16) + .frame(maxWidth: .infinity, alignment: .leading) + } + } else if !error.isEmpty { + EmptyText(text: error, error: true) + } else { + EmptyText(text: "Loading…") + } + } + .task(id: load) { + do { + let data = try await ImageCache.fetch(.worktree(taskId: taskId, path: path), store.api) + guard let img = NSImage(data: data) else { + image = nil + error = "Not a readable image." + return + } + image = img + error = "" + } catch { + if Task.isCancelled { + return + } + image = nil + self.error = errorText(error) + } + } + } +} + private struct FileText: View { let content: FileContent