Skip to content

Attach images to Ask Me answers (workspace-confined writes) - #26

Merged
aaroncoville merged 4 commits into
mainfrom
feat/ask-me-image-attachments
Aug 26, 2026
Merged

aaroncoville merged 4 commits into
mainfrom
feat/ask-me-image-attachments

Conversation

@aaroncoville

Copy link
Copy Markdown
Owner

What & why

Plenty of the asks that reach a human through ASK ME are visual — "what does
this look like on your machine?", "is this the right layout?" — and the answer
box was text only. The screenshot had to be saved by hand somewhere the agent
could reach and its path typed in, which is enough friction that the answer
usually came back as prose describing the picture instead.

An answer now takes images three ways — paste, drop, or the picker — and carries
the paths the main process hands back, so the agent opens them with its own file
tool. The renderer sends BYTES and a task id and nothing else: the directory,
the file name and the extension are all decided in the main process, and the
format is decided by sniffing the leading bytes, because an extension and a MIME
label are both caller input and neither is evidence about the bytes behind it.

Generating the path is not containment on its own, and the difference matters
because the hive is a SHARED directory — other agents, a restored backup, a
cloned tree all write into it. Whatever plants a symlink at
asks/attachments/<task> turns an in-root NAME into an external DIRECTORY: the
name passes any string check, and the mkdir/write that follows is resolved by
the kernel, which follows the link and drops the image outside the hive. So both
the folder and the final file go through safeResolve, the same guard the file
IPC already uses, and the write is an exclusive create through a new
openForCreate sibling of openForRead — the guard and the open resolve the
name separately, so a link that appears in between is refused by the kernel
rather than followed, and EEXIST becomes an atomic answer to "is this name
free?" in place of an existsSync probe that can be won in the gap.

The other thing an IPC round trip costs is time the human does not wait for.
Paste a screenshot and send in the same second and the answer used to be
assembled from whatever had already come back: the image was written to the
hive, then cleared with the rest of the draft when the pending call landed after
the send — stored on disk, named in neither the card nor the message to the
agent, and no error anywhere, because the upload itself succeeded. Each card now
keeps one chained promise for its attachments in flight; the send button holds
while it runs, and sendAnswer waits on it and then reads the store rather than
its own render closure. The wait has to live in sendAnswer and not only on the
button, because Ctrl+Enter is the fast path here and no disabled prop covers a
keystroke.

Depends on the workspace path guard in src/main/fs.ts (the symlink-sandbox
containment fix): this reuses safeResolve rather than reimplementing
containment, and only adds openForCreate beside the existing openForRead.
Merge that first.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

The UI half is a paste target and a chip row; the two behaviours worth pinning
are invisible ones, so both are shown as a test going from red to green against
the same file. Attach a screen recording of paste → chip → "respond & unblock"
for the visible half before opening.

Before

Containment — a symlink planted at the task folder is followed, and the image
lands outside the hive:

$ node --test test/ask-attachments.test.cjs
not ok 10 - refuses a task folder symlinked to a directory outside the hive
  error: |-
    a task folder that leaves the hive must be refused

    true !== false
  expected: false
  actual: true
# tests 12
# pass 11
# fail 1

The send race — Ctrl+Enter while the upload is still in flight writes the answer
without it, and an in-flight upload that comes back REFUSED leaves the card (and
with it the whole board, which shares the state) stuck mid-send:

$ node --test test/ask-me-attachments.test.cjs
not ok 5 - an attachment still in flight at send time is waited for, not dropped
  error: |-
    the answer must not be written without the image

    1 !== 0
not ok 6 - an in-flight attachment that comes back REFUSED releases the card instead of wedging it
  error: 'the card must not be stuck mid-send'
  expected: 'sending…'
# tests 6
# pass 4
# fail 2

After

$ npm run test:focused
# tests 664
# pass 664
# fail 0

Mutation-checked, so the tests pin the guard rather than merely running past it:
swapping await safeResolve(hiveRoot, relDir) back for a plain
join(hiveRoot, relDir) turns test 10 red again, and swapping openForCreate
for a plain open(target, 'w') turns "two attachments in the same task never
overwrite each other" red.

How I tested it

  • OS: macOS (darwin 25.6.0), Node via the repo toolchain.
  • Steps:
    1. npm run test:focused on the base branch first — 642 pass, 0 fail — so the
      numbers below are provably this branch's delta and nothing else.
    2. Wrote the containment and race tests first and captured them failing
      (above), then implemented against them.
    3. npm run test:focused — 664 pass, 0 fail (+22: 12 in
      ask-attachments.test.cjs, 6 in ask-me-attachments.test.cjs, 4 in
      attached-images.test.cjs).
    4. Reverted the containment and the exclusive create in turn and re-ran, to
      confirm the new tests go red without them.
    5. npm run typecheck — clean. npm run build — succeeds.

Discord (optional)

Discord:

Checklist

  • Before and after evidence is attached above, under both headings.
  • npm run typecheck passes.
  • npm run test:focused passes.
  • npm run build succeeds.
  • This PR is one change. Unrelated fixes belong in their own PR.
  • I read the diff myself before opening this, and there is no debug output,
    commented-out code, or unrelated formatting churn in it.
  • Any new UI derives from DESIGN.md / tokens.ts — no ad-hoc colors,
    spacing, or fonts.
  • If I added art, it's my own or compatibly licensed, and listed in
    ATTRIBUTION.md. (No art added.)

Aaron Coville added 4 commits August 26, 2026 14:51
An answer to an ASK ME card can only be prose today, so a screenshot — the
fastest way to answer "what does it look like on your machine?" — has to be
saved by hand and its path typed in. This adds the storage half: one main-
process endpoint that takes bytes and writes them into the hive.

The renderer supplies BYTES and a task id, and nothing else. The directory, the
file name and the extension are all decided in the main process, under
<hiveRoot>/asks/attachments/<task>/<timestamp>-<n>.<ext>: a renderer that is
compromised — or merely buggy — has no filename to poison, so there is nothing
to traverse with. The task id is slugged down to one path segment for the same
reason.

Generating the path is not containment on its own, though, and the difference
matters here because the hive is a SHARED directory — other agents, a restored
backup, a cloned tree all write into it. Whatever plants a symlink at
asks/attachments/<task> turns an in-root NAME into an external DIRECTORY: the
name still passes any string check, and the mkdir/write that follows is resolved
by the kernel, which follows the link and drops the image outside the hive. A
dangling link is worse, because its target does not exist for anything to
inspect and the write CREATES the external file. So both the folder and the
final file go through the same safeResolve guard the file IPC uses, and the
write is an exclusive create through the new openForCreate: the guard and the
open resolve the name separately, so a link that appears between them is refused
by the kernel rather than followed. EEXIST is not a failure there — it is an
atomic answer to "is this name free?", replacing an existsSync probe that could
be won in the gap.

The format is decided by sniffing the leading bytes rather than by trusting an
extension or a MIME string, because both of those are caller input: a text file
renamed to .png is still a text file, and an "image/png" label proves nothing
about the bytes behind it. Anything that is not PNG, JPEG, GIF or WebP — or is
over 10MB — is refused before it reaches the disk, with a message the caller can
show the human as-is.
An answer's attachments have to be listed in two places written seconds apart
and read months apart: the answer stored on the card (humanQA[].a) and the
message mailed to the agent. Formatting each independently is how they end up
disagreeing — the card documenting an answer the agent never received.

One function, so there is only one format to drift.
The React host in test/render-hooks.cjs exists so a component test can assert
on what a component actually renders rather than on a pure function it might no
longer call. Any component wired to the zustand store was out of its reach
though, for two reasons that are both one line each:

  - `@/…` — the alias every renderer module uses for its own imports — did not
    resolve, so the component failed to load at all; and
  - zustand reads through the useSyncExternalStore shim, which calls
    useDebugValue and useLayoutEffect. Neither was seeded, so the very first
    store read threw "useDebugValue is not a function".

useSyncExternalStore is seeded too, so a store that talks to React 18 directly
works the same way: the snapshot is the value, and re-rendering stays the
test's own call via `render()` rather than something a subscription does behind
its back.
Plenty of the asks that reach a human are visual — "what does this look like on
your machine?", "is this the right layout?" — and the answer was text only. The
screenshot had to be saved by hand somewhere the agent could reach and its path
typed into the box, which is enough friction that the answer usually came back
as prose describing the picture instead.

An answer now takes images three ways: paste one from the clipboard, drop the
file anywhere on the answer area, or pick one with the button. Each shows as a
thumbnail chip that can be removed right up until send, and an image on its own
is a complete answer — a screenshot frequently is one.

The bytes go to the main process the moment they are attached, and the answer
carries only the paths it gets back, so the agent opens them with its own file
tool. Both sinks — the answer stored on the card and the message mailed to the
agent — are built from the same string, so the card can never document an
answer the agent did not receive. A file the main process refuses (not an
image, over the size cap) is reported on the card in its own words rather than
disappearing, which is the failure mode a paste target invites.

Storing an image is an IPC round trip, and the human does not wait for it: paste
a screenshot and send in the same second and the answer would be assembled from
whatever had already come back. The image was written to the hive, then cleared
with the rest of the draft when the pending call landed after the send — stored
on disk, named in neither the card nor the message to the agent, and no error
anywhere, because the upload itself had succeeded. So each card keeps the one
chained promise for its attachments in flight, the send button holds while that
is running, and sendAnswer waits on it and then reads the STORE rather than its
own render closure — the attachment that just landed is in the former and cannot
be in the latter. The wait has to live in sendAnswer and not only on the button:
Ctrl+Enter is the fast path here and no `disabled` prop covers a keystroke.

That wait also hands sendAnswer a way to leave without finishing — the awaited
upload comes back refused, an image-only answer then has nothing to send — so
`sending` is cleared in a finally rather than after the try. It is one value for
the whole board and it is what rejects the NEXT send, so a single path out that
forgets to clear it wedges every card until the view is remounted. Being told
"not an image" and retrying with a real one is precisely what the human does
next, and that has to work.

Attachments live in the store beside the drafts for the reason the drafts do:
switching tabs unmounts this view, and it must not quietly discard what someone
just attached.
@github-actions

Copy link
Copy Markdown

🚫 This PR is missing its before/after evidence

Every pull request here has to show its work. Screenshots or a short screen recording, before the change and after it.

  • Before — no image or video under that heading
  • After — no image or video under that heading

How to fix it: edit the description, keep the ### Before and ### After headings from the template, and drag an image or video under each. GitHub uploads it inline. This check re-runs the moment you save.

A bug fix with no visible surface still needs it: show the failing behaviour, then the same steps passing. A terminal recording is fine.

Genuinely nothing to show — a CI tweak, a typo, a dependency bump? A maintainer can apply the no-visual-change label. Please don't ask unless it truly has no observable effect.

📖 CONTRIBUTING.md → Evidence is mandatory

@aaroncoville
aaroncoville merged commit f6ff59a into main Aug 26, 2026
2 of 3 checks passed
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