Skip to content

Let the chat agent attach files to its reply - #176

Merged
tim-smart merged 12 commits into
mainfrom
agent/nelson/62472337dc9f
Nov 2, 2026
Merged

tim-smart merged 12 commits into
mainfrom
agent/nelson/62472337dc9f

Conversation

@tim-smart

@tim-smart tim-smart commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes EFF-3309

The chat agent can now attach files from its sandbox to its reply on Slack and Discord. Until now, the only file that ever went out was the automatic reply.md for a reply too long to post as messages.

The tool

attachFile({ path, name? }) is a loop-side tool, like PR review's reportFinding. It reads the file from the sandbox right away and adds it to the turn's list. name is the filename shown in the thread and defaults to the file's own name. The content type comes from the extension, with application/octet-stream for anything else. The system prompt tells the agent to use the tool for generated images, diffs, logs and anything too big to paste.

Limits are checked inside the tool, so the model gets a message it can act on instead of a failure at post time:

  • At most 9 files per reply. Discord takes 10 files a message, and an overflowing reply needs one of them for reply.md.
  • At most 9 MiB in all. Discord's limit is 10 MiB a message on a server without boosts, and the remaining room covers reply.md. This is a total, so a single file over 9 MiB fails too.

The list belongs to one sandbox (makeReplyFiles in Chat/ReplyFiles.ts, created per attempt in Session.ts). A turn that resumes after losing its sandbox starts with an empty list, so stale bytes are never posted. A turn that fails, is cancelled, or is stopped by the kill switch posts nothing.

Reading from the sandbox

ChatSandbox.readFile(path, maxBytes) calls a new GET /files route on the container, which reads through readWorkspaceFile (Chat/Sandbox/Files.ts). It is a path filter. The path must be absolute and under /workspace. Symlinks are resolved with realpath, and the resolved file must still be under /workspace, be a regular file and fit maxBytes. The read uses the resolved path and stops one byte past maxBytes.

Posting

postReply takes files and posts them in the same message as the reply, as decided on the issue:

  • Discord: the files go with the last chunk. Earlier chunks stay text-only, and the allowed_mentions / message_reference handling is unchanged.
  • Slack: the files are uploaded before any text is posted. The earlier chunks then go out as markdown_text as before, and the last chunk is shared with the files through files.completeUploadExternal, its text as initial_comment in mrkdwn.
  • reply.md: an overflowing reply is posted as reply.md plus the files, in one message, on both platforms.

If the platform refuses the files, whatever of the reply isn't out yet goes as text instead, ending in "(Couldn't attach plot.png.)": the whole reply when the files were refused with it, the last chunk when the earlier chunks are already out, and an overflowing reply as text chunks, however many it takes. The platform is the only layer that knows what went out, so it owns this, and nothing already posted is ever posted again. A failure whose answer was lost, where the message may be up, fails the reply, and the session doesn't retry anything. postReply succeeds with the text it posted, and the session saves that, so the stored reply keeps the note. The turn counts as replied whenever the text went out. uploadFile now takes bytes and a content type.

Open points

  • Slack formatting: the message that carries the files loses some formatting. files.completeUploadExternal fails with internal_error when it carries a markdown block (python-slack-sdk#1756, still open), so that message's text goes as initial_comment in mrkdwn. Tim accepted the loss to keep the files in the reply's message. toMrkdwn rewrites headers and bold to *bold*, ~~strike~~ to ~strike~ and links to <url|text> outside code, escapes &, < and >, and drops a code fence's language. Tables, lists and italics stay as written. Replies without files still go as markdown_text. initial_comment is the documented argument, but a live check on Slack is still worth doing.
  • What the path check guards: it refuses a path or symlink that resolves outside /workspace. It is a path filter, not isolation from the agent's own scripts: they run as root in the same container, and a symlink swapped in between the check and the read can still point outside. That gains them nothing, since they can copy any file into /workspace anyway. The container holds no credentials; the egress proxy swaps in the real tokens.

Tests

One test per behaviour, nine in all:

  • Sandbox/Files.test.ts: readWorkspaceFile refuses .. escapes, absolute paths elsewhere, symlinks to a file or directory outside the workspace, and oversized files.
  • Session.test.ts: attached files go out with the reply, named and typed by extension. The tool refuses a file over 9 MiB and a tenth file. Files from a turn that ends without a reply are never posted. A reply whose files can't be uploaded is still posted, naming them.
  • Discord/Platform.test.ts and Slack/Platform.test.ts, one each: the files go in the same message as the reply's last part, and an overflowing reply sends reply.md and the files in one message.

The Session test for a refused upload still expects the session to post the reply a second time, which it no longer does; it needs to move to the platforms, where the fallback now lives. Until then vp test passes 946 of 947 tests, and vp check reports 0 errors.

The chat agent gets an attachFile tool (EFF-3309). The tests pin:

- readWorkspaceFile, the container-side read, rejects paths that escape
  /workspace through `..`, an absolute path or a symlink, and files over
  the size limit.
- attachFile fails in the tool for a file too large to post, and for more
  files than one Discord message holds next to reply.md.
- Attached files go out with the reply, named and typed by extension, and
  are dropped when the turn fails or is cancelled.
- A reply whose files can't be uploaded is still posted, naming them.
- Discord puts the files on the reply's last chunk; Slack shares them with
  the text in one files.completeUploadExternal; an overflowing reply.md
  goes in the same message as the files on both.

The session harness's fake executor now runs `await tool(params)` scripts
against the loop-side tools, and its sandboxes serve readFile.
The chat agent gets a loop-side `attachFile({ path, name? })` tool
(EFF-3309). It reads the file from the sandbox into the turn's list, and
the files go out in the same message as the reply's last part, or with
its reply.md.

- The container serves `GET /files`, which reads through
  `readWorkspaceFile`: symlinks are resolved first, and anything that
  resolves outside /workspace, isn't a regular file or is over the limit
  is refused. `ChatSandbox.readFile` reaches it.
- A reply carries at most 9 files and 9 MiB in all: Discord takes 10
  files and 10 MiB a message unboosted, and reply.md needs room. Over
  either limit, the tool fails with a message the model can act on.
- The list belongs to one sandbox: a resume after a lost sandbox starts
  without it, and a turn that ends without a reply posts none.
- `postReply` takes `files`. Discord sends them with the last chunk.
  Slack uploads them before posting any text, then shares them with the
  last chunk as a `markdown` block through files.completeUploadExternal.
- A reply whose files can't be posted is posted again without them,
  naming them.
- `uploadFile` takes bytes and a content type; the content type comes
  from the file's extension, `application/octet-stream` otherwise.
- The system prompt tells the agent about the tool.
Review findings on PR #176:

- A reply refused with its files after some of it went out was posted
  again in full. `ChatPlatformError` now carries `partial` when a reply
  may be partly out, and the session posts it again without the files
  only when it isn't. When Discord or Slack refuse the message carrying
  the files after the earlier chunks are out, the platform posts that
  last chunk as text with the note instead. An outcome that is unknown
  (a lost answer, or a Slack write that may have landed) is never posted
  again.
- An overflowing reply whose uploads are refused now goes out as text
  chunks with the note, however many that takes: a reply.md alone would
  be refused the same way.
- `readWorkspaceFile` stops reading one byte past the limit, so a file
  that grows after its stat is never read in full.

The module doc now says what the path check guards and what it doesn't.
Re-review findings on PR #176:

- `postReply` now succeeds with the text it posted. When a platform
  finished a reply as text after its files were refused, the session
  saved the reply without the "couldn't attach" note; it now saves what
  the thread shows.
- A `chat.postMessage` answered `ok` but without its `ts` counted as
  refused, so a reply with files could be posted again in full. It is
  now uncertain: Slack said ok, so the message may be up.

The doc on `readWorkspaceFile` now calls the check a path filter: a
symlink swapped in between the check and the read can still point
outside /workspace.
The tests this PR adds go from 12 to 9:

- The two turn endings without a reply share one test, ended by a
  provider failure.
- The limits test checks the 9-file and 9 MiB caps directly instead of
  bounding them.
- The Discord and Slack refusal tests go, with the fakes only they used.
  The session's failed-upload test covers the fallback.
The session no longer posts a reply a second time without its files, and
ChatPlatformError loses its partial flag: the platform is the only layer
that knows what went out, so it posts whatever hasn't gone out as text,
naming the files, and never posts anything again. Discord and Slack each
have one fallback path instead of two, Discord's send/post split and
Slack's uncertain mapping of a malformed chat.postMessage answer go, and
reply.md and the note are built once in Platform.ts.
The platform alone delivers a reply whose files are refused, so the test
moves there: on Discord and on Slack, a two-chunk reply whose files are
refused goes out once as text, the last chunk naming them. The session
test and the fake platform's `failUploads` go.

The Slack test fails: when the uploads are refused before any text goes
out, only the last chunk is posted, with the note, and the earlier
chunks are never posted.
Slack fails files.completeUploadExternal with internal_error when it
carries a markdown block (python-slack-sdk#1756), and treats that as an
uncertain write, so a reply with files never went out. The message that
carries the files now takes its text as initial_comment, in mrkdwn: Tim
accepted the lost formatting to keep the files in the reply's message.

`toMrkdwn` rewrites the trivial cases outside code: headers and bold to
*bold*, ~~strike~~ to ~strike~, [text](url) to <url|text>. It escapes
&, < and >, keeps a quote's >, and drops a fence's language. Replies
without files still go as markdown_text.
The Slack refusal test now runs twice: with the files refused while
staging, before any text goes out, and with files.completeUploadExternal
refused after the first chunk went out. Each time the reply goes out
once, the last chunk naming the files.

The files-with-reply test asserts the text as initial_comment, in
mrkdwn.
Review findings on PR #176:

- toMrkdwn cut a link whose URL holds parentheses, such as
  [docs](https://example.com/a(b)), at the first `)`. It now reads one
  level of parentheses in a URL, and leaves a link it can't read as
  written.
- The files message's mrkdwn could pass Slack's 40,000 characters, since
  escaping grows `&` fivefold, and Slack truncates past that. Such a
  reply now goes as reply.md with its files, like a reply too long for
  messages.
- The files-with-reply test's text links a URL with parentheses, and
  asserts its mrkdwn.
- The refusal test logs Slack's posts and shares in order, so a refused
  share is shown to come after the first chunk, with nothing posted
  twice.
- The overflow test also runs a one-chunk reply of `&`, whose mrkdwn
  would pass 40,000 characters: it goes as reply.md with its files.
@tim-smart
tim-smart merged commit 5d32a71 into main Nov 2, 2026
4 checks passed
@tim-smart
tim-smart deleted the agent/nelson/62472337dc9f branch November 2, 2026 23:45
tim-smart added a commit that referenced this pull request Nov 3, 2026
Alchemy's container diff returned early for every Effect-native container:
its props carry the program as an Effect in `exports`, so `isResolved` is
always false. The engine then planned noop, and the chat and PR review
sandbox images stayed on the 2026-10-28 build. The chat sandbox never got
the /files read route from #176, so attachFile got a 405.

Patch the Cloudflare provider to strip Effects before that check, as its
Worker provider already does.
tim-smart added a commit that referenced this pull request Nov 3, 2026
Alchemy's container diff returned early for every Effect-native container:
its props carry the program as an Effect in `exports`, so `isResolved` is
always false. The engine then planned noop, and the chat and PR review
sandbox images stayed on the 2026-10-28 build. The chat sandbox never got
the /files read route from #176, so attachFile got a 405.

Patch the Cloudflare provider to strip Effects before that check, as its
Worker provider already does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant