Skip to content

fix(ui-web): attach a picture dragged into the composer by its own path - #900

Open
LivXue wants to merge 14 commits into
mainfrom
fix/composer_drop_transcript_image
Open

LivXue wants to merge 14 commits into
mainfrom
fix/composer_drop_transcript_image

Conversation

@LivXue

@LivXue LivXue commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Summary

Dragging a picture from the transcript into the composer did not send the picture. Chromium and Firefox on Linux hand a drag that stays inside the document over with no File at all, so the drop did nothing (measured). Chrome on macOS hands it over as a File named download: its drop code names the file from the image's Content-Disposition with no URL to fall back on, and the format survives only in File.type. The composer uploaded that File as uploads/download, and everything that tells a picture by its extension (the sent bubble, the /file route, the reloaded history) drew a file chip named download where the picture should have been.

A drop now attaches the file a dragged picture was drawn from, where it already is, by path: nothing is read and nothing goes up. The composer takes the file from text/uri-list when the list names this origin's own /file route, and prefers that address over any File the drop also carries, so Chrome on macOS no longer uploads a second copy. A sent picture that is still drawn from the upload's cached data URL puts its /file address in the list when it is dragged; a picture already drawn from /file carries it already.

An address is taken at its word only when the drag that began in this document carried the same path from its start. Any site the reader drags from can put one of these addresses in the list, so a drop that did not begin here, or names a path its drag did not carry, is not attached by address. Paths are compared rather than the address as written, since the platform's drag plumbing may respell the address on the way through. The two document listeners that record a drag are registered in state/globalListeners with the document's other listeners.

A File that does go up is named by its type when its name spells no picture extension (download becomes download.png), which covers images dragged in from other sites. The extensions a name is a picture by live in one table, lib/pictureExt.ts, read by the composer and the workspace viewer alike, so the tray and the sent bubble cannot disagree about a name. The tray chip of a file attached by path has no size in its tooltip, since its bytes were never read.

An upload, and a picked deck template, is staged by the file the server wrote (abs_path), so the message's attachment note names it absolutely, while a dragged file keeps the path it was dragged with. uploads/<name> alone can name two files, the upload under agent home and a session root's own uploads/<name>, which the workspace pane lists and a drag stages by that spelling; only the absolute path says which one the reader meant, at the turn, in the bubble and after a reload alike. This is decided where the file is staged rather than mapped back at send time, because a queued message reaches the send as plain text, and two notes that spell the same path cannot be told apart there. The sub-agent direct chat writes the same note and stages its uploads the same way.

turn.send resolves an attachment the way the viewer does: the conversation's own working directory first, agent home for an upload, by the rule /file follows (viewer_root, now in raven/rpc/files.py, so that turn.send does not import the console and close an import cycle). With tools.restrict_to_workspace on it admits that directory beside agent home. Before, it fenced every attachment to agent home, so a picture attached by path from a session pinned to a folder outside agent home showed in the tray and was dropped from the turn. A path neither directory holds is still refused.

An upload is told by where its path lands, in either spelling: a path that lands inside agent home's uploads/ is agent home's. So /file serves the absolute spelling of an upload to a pinned session under the fence (it answered 403, which also left a reloaded bubble's upload chip dead there, since that chip names the engine's absolute path), and uploads/../... or a link left under uploads/ no longer reaches the rest of agent home. fs.upload keeps a name free under the session's own uploads/ as well as agent home's, for a caller that sends the relative path back, and a session folder it may not read leaves the name to agent home instead of failing the upload. The published fs.upload result says which of its two paths to hand turn.send. A reloaded bubble gives each engine path to one note entry, in order, so two entries naming suffixes of one path no longer draw one file twice.

Not covered, by design or by reach:

  • Chrome on Windows and on macOS were not run, and neither was the server on Windows, where an upload's note names a C:\ path; neither machine was available. The drop relies only on text/uri-list; where a platform's list matches no path the drag carried, the drop falls back to the upload path, which names the File by its type.
  • A drag from another Raven tab is not a drag that began in this document: it falls back to the upload path (Chrome on macOS) or does nothing.
  • Anything dragged out of this document whose list names a /file address is attached by path, not only a picture, for example a link to a file.
  • The sub-agent direct chat has no drop target.
  • deck.templates.pick keeps a name free under agent home only, and its params carry no session to look past. The composer stages the pick's absolute file, so nothing the web UI sends is affected; a caller that hands turn.send the pick's relative path could be given a session root's same-named file.
  • A draft's upload is name-checked against the resolver's policy default (<session root>/_ under raven gateway), not the folder the draft is pinned to. The composer sends the absolute file, so no reader it feeds is affected.
  • A note written before this change names an upload as uploads/<name>; its reloaded bubble still prefers the engine's absolute line, as before.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

  • node node_modules/vitest/vitest.mjs run --no-file-parallelism in ui-web/, at this head on current main: 214 files (168 under src/, 46 under scripts/gates/), 3248 tests passed, 0 failed, 0 unhandled errors. A parallel run on this shared box times out a varying handful of state/session/* tests; run serially every file passes.

  • node node_modules/typescript/bin/tsc --noEmit -p tsconfig.json: exit 0 in ui-web/ and in ui-tui/; both generated RPC clients are in sync with openrpc.json (gen-rpc-client.mjs --check, gen-rpc-types.mjs --check).

  • make check-commits check-large-files check-source-language lint-python lint-imports lint-ui: exit 0. Import contracts 10 kept, gen:check matches 210 methods, eslint 0 errors; its 4 warnings predate this branch: two in SubagentsPage.tsx, on lines this branch does not change, and one each in ConnectionsPage.tsx and CronPage.tsx.

  • uv run --frozen --all-extras pytest -q (the whole suite, at this head): 27869 passed, 119 skipped, 7 failed, all seven environment failures of this box: five proxy tests in tests/test_config_update_providers.py (the file passes 193/193 with the box proxy variables unset), tests/test_install_script.py::test_resolve_node_dir_answers_each_case_it_exists_for (an npm elsewhere on PATH) and tests/test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothing (the suite runs as root).

  • Each new test of the last review round failed before its change: after an upload, a dragged file of the same spelling reached the turn as the upload (mediaOf gave both messages the agent-home file); an upload, a picked template and the direct chat's upload were staged by the short path; viewer_root gave uploads/../notes/x.md, and the absolute spelling of an upload, the wrong root; /file answered 403 for a pinned session's absolute upload path and 200 for uploads/../notes/secret.md; an unreadable session folder raised PermissionError out of fs.upload.

  • 34 single-construct mutants of the branch's executable changes each turn at least one test red. The drop (13): the origin check, the /file route check, the carried-path check, path versus raw-address matching, clearing the drag on drop, clearing it on dragend, the address winning over a File, a chip picture only for a picture name, the null size, both path separators, svg in the picture table, the drag listeners' registration, and each engine line answering one note entry. Staging an upload (6): the short path for an upload, for a template and in the direct chat, the bubble's bytes filed under the short path for an upload and for a template, and no fallback to the short path. viewer_root and /file (6): no absolute branch, the absolute branch unresolved, the relative branch judged by its first segment or unresolved, agent home's uploads/ unresolved, and /file not asking about an absolute path. The upload name check (2): the wrong exception caught, and the session's folder not consulted. turn.send (7): a fence on agent home only, no fence, relative paths against agent home, agent home taken from the session's root, a busy: "inject" send ignoring the session's root, an unguarded root lookup, and the session key not passed to the lookup.

  • Real browsers, against an isolated raven web on this branch with tools.restrict_to_workspace on, model openrouter/z-ai/glm-5.3-flash, and real Playwright drags:

    • Chromium 151 and Firefox 153: a picture picked and sent, then dragged back in from its sent bubble (drawn from its data URL), then again after a reload (drawn from /file). One fs.upload in the whole run; both drags sent media with the upload's absolute path; the model answered "Red" for the red test image each time.
    • Chromium, two conversations in one tab. In the first, a red image.png uploaded and sent: media was the absolute agent-home file, and /file for that path answered 200. In the second, pinned to a folder whose own uploads/image.png is green, that picture, drawn by /file, dragged in and sent: media was ["uploads/image.png"], the stored user message carried [Image: image.png (path: <folder>/uploads/image.png) ...], the model answered "Green" with no tool call, and /file for the first conversation's upload by its absolute path answered 200 from the pinned conversation.
  • Relevant tests pass locally

  • Relevant lint / type checks pass locally

  • User-facing docs or screenshots are updated when needed

No user-facing doc describes dragging a picture into the composer, so there is nothing to update.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Dragging a transcript picture into the composer attaches the original file, and the uploads directory no longer gains a copy per drag. Only a drag that began in this document is attached by address; the tests cover a foreign drop, another origin, another route and a path the drag did not carry. A message's attachment note now names an upload by its absolute file, which is what the model and the stored history see. With tools.restrict_to_workspace on, turn.send also admits the conversation's own working directory, which /file already serves and that conversation's file tools may already read; /file serves agent home's uploads/ by either spelling, and no longer the rest of agent home through uploads/.. or a link under uploads/. A path outside both directories is still refused. Rollback is reverting the squash commit; no data or config migrates, and notes already written keep resolving.

Related Issues

N/A

@LivXue
LivXue requested review from 0xKT and gloryfromca October 10, 2026 09:03

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: preserve the session workdir when resolving path-backed attachments.

I reviewed the full github/main...HEAD diff, the composer/transcript callers, /file and turn.send path policy, relevant history, backward compatibility, tests, and the Web UI/runtime architecture and naming rules in AGENTS.md, CONTEXT-MAP.md, and ui-web/CONTEXT.md. I found one concrete blocker, marked inline. I did not find weakened tests or additional merge-blocking issues.

Verification:

  • Focused Vitest suite: 254 passed across Dock.test.tsx, ComposerPage.test.tsx, and TranscriptPage.test.tsx.
  • npm run type-check: passed.
  • npm run lint: passed with four pre-existing warnings in untouched files.
  • uv run pytest tests/test_rpc_turn_send.py -x: could not run because global test setup cannot import the optional raven_everos package (4 setup errors before test bodies).
  • Direct resolver reproduction: an absolute picture under a session project outside agent home raises PermissionError when the agent-home fence is enabled.

Comment thread ui-web/src/features/composer/store.ts
@LivXue
LivXue force-pushed the fix/composer_drop_transcript_image branch from 9d3a577 to d974e40 Compare October 10, 2026 12:46

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

I reviewed the rebased github/main...HEAD diff and the new session-aware attachment fix, including the /file and turn.send callers, shared viewer_root behavior, normal and injected sends, path-policy compatibility, relevant history, and the Web UI/runtime constraints in AGENTS.md, CONTEXT-MAP.md, and ui-web/CONTEXT.md. The previous blocker is fixed and its thread is resolved. I found no weakened tests or new issue worth raising.

Verification:

  • uv run pytest tests/test_rpc_turn_send.py -x: 62 passed.
  • uv run pytest tests/test_rpc_console.py -k viewer_root -x: 3 passed.
  • Focused Vitest suite for Dock, ComposerPage, and TranscriptPage: 254 passed.
  • npm run type-check: passed.
  • Focused Ruff check: passed.
  • git diff --check github/main...HEAD: passed.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

I reviewed the delta from the previously clean revision and rechecked the full github/main...HEAD change. The shared session/viewer path rules now live in raven.rpc.files, removing the new console/turn import cycle while preserving the console-facing names, /file behavior, restricted attachment resolution, and normal/injected send paths. I also checked relevant callers and history, backward compatibility, test integrity, and the architecture and repository constraints in AGENTS.md, CONTEXT-MAP.md, and ui-web/CONTEXT.md. I found no new issue worth raising, and my prior thread remains resolved.

Verification:

  • uv run pytest tests/test_import_cycle_budget.py -x: 4 passed.
  • uv run pytest tests/test_rpc_turn_send.py -x: 62 passed.
  • uv run pytest tests/test_rpc_console.py -k 'viewer_root or file' -x: 16 passed.
  • Focused Ruff check: passed.
  • git diff --check github/main...HEAD: passed.

Comment thread raven/rpc/methods/turn.py
Comment thread ui-web/src/features/composer/mount.tsx
Comment thread ui-web/src/features/composer/store.ts

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: a fresh upload can resolve to an older same-named session file.

The new review record invalidates my earlier clean stance. I independently confirmed the concrete behavior reported in discussion 4238145333: when the session root and agent home both contain uploads/image.png, viewer_root(session, rel, home) selects the session copy, while fs.upload writes and returns the agent-home copy. The composer can therefore preview the newly uploaded bytes while turn.send silently hands the model the older file. That ordinary, user-visible data substitution must be resolved before merge.

The two additional reviewer blockers about global-listener ownership and the duplicated picture-extension table remain open under their author; I am not re-grading or resolving them. My own earlier attachment-root thread remains correctly resolved.

Verification this turn:

  • uv run pytest tests/test_rpc_turn_send.py tests/test_rpc_console.py -k 'turn_send or viewer_root or upload' -x: 70 passed.
  • Focused Vitest suite for Dock, ComposerPage, and globalListeners: 88 passed.
  • Direct collision reproduction selected <session>/uploads/image.png containing OLD over <agent-home>/uploads/image.png containing NEW.

@0xKT

0xKT commented Oct 10, 2026

Copy link
Copy Markdown
Member

Not a blocker -- two smaller items from the acceptance pass on head d36d455 (merged onto main 289426c), beside the three blocking threads.

Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

  1. After a reload, a bubble can show one attached picture twice and lose the other. splitAttachments matches each note path to the engine's (path: ...) lines with absolute.find((abs) => slashed(abs) === want || slashed(abs).endsWith('/' + want)) and keeps the first hit (ui-web/src/lib/attachments.ts:159-162). Until this PR a note could only carry uploads/<unique name>, so the suffix match had one answer; addPaths now lets a note carry any path a same-origin /file drag names, relative to the session's root or absolute. Measured with the real splitAttachments on this head: a note listing out/chart.png then chart.png, with engine lines for /proj/out/chart.png and /proj/chart.png, comes back as /proj/out/chart.png twice, and /elsewhere/out/chart.png then out/chart.png comes back as /elsewhere/out/chart.png twice. The reverse order, and two uploads, come back right. The turn itself receives both files; only the reloaded bubble, and a drag out of it, is wrong. Letting each engine line answer one note entry, in order, would close it.
  2. No test pins the session key _media_root hands the resolver. With return workspace_root(loop, session_key) (raven/rpc/methods/turn.py:134) changed to workspace_root(loop, ""), tests/test_rpc_turn_send.py and tests/test_rpc_session.py still give 228 passed: the _PinnedLoop every new test uses answers the same root whatever key it is asked about (tests/test_rpc_turn_send.py:466-469). Control: dropping the fence (allowed = ()) fails 2 of them. With a real resolver the mutant roots a pinned session's attachments at the policy default instead of its own directory. A stub loop that answers per key would pin it.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

I reviewed the delta from the blocked revision and rechecked the full github/main...HEAD change, affected callers and history, backward compatibility, test integrity, and the repository architecture and rules in AGENTS.md, CONTEXT-MAP.md, ui-web/CONTEXT.md, and ui-web/CONTRIBUTING.md. The fresh-upload identity bug is fixed both at name allocation and at send time: upload paths avoid the current session's shadow names, and the live page hands turn.send the absolute file fs.upload wrote. The document listeners now belong to state/globalListeners, the picture-extension table is shared through lib, and the two attachment replay notes are covered. I found no new blocker or plain error.

The three open threads were raised by another reviewer and remain for that reviewer to close; my own earlier thread remains resolved.

Verification:

  • uv run pytest tests/test_rpc_turn_send.py tests/test_rpc_console.py tests/test_rpc_files.py tests/test_import_cycle_budget.py -x: 282 passed.
  • Focused Vitest suite across 9 affected files: 446 passed.
  • npm run type-check: passed.
  • Focused ESLint and Ruff checks: passed.
  • Import-direction gate through Vitest: 6 passed.
  • git diff --check github/main...HEAD: passed.
  • A direct node invocation of the Vitest gate was invalid for its runner API; rerunning with the repository test command produced the passing result above.

@LivXue
LivXue force-pushed the fix/composer_drop_transcript_image branch from 08b0c99 to 23296bc Compare October 10, 2026 19:52

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

This head rewrites commit history but has the exact same tree object as the previously reviewed clean revision (6d725a38c32db1495c59568a30a9ba6079e80ec2), with an empty 08b0c99a3a86..HEAD content diff. I rechecked the full github/main...HEAD change, affected callers and compatibility, test integrity, and the repository architecture and rules; the upload identity, global-listener ownership, shared picture-extension table, import-cycle repair, and attachment replay fixes remain intact. I found no new blocker or plain error.

The three threads opened by another reviewer now contain that reviewer's re-verification that they are fixed; resolving those threads remains theirs. My own earlier thread remains resolved.

Verification:

  • uv run pytest tests/test_rpc_turn_send.py tests/test_rpc_files.py tests/test_import_cycle_budget.py -x: 133 passed.
  • Focused Vitest suite across 6 affected files: 126 passed.
  • npm run type-check: passed.
  • git diff --check github/main...HEAD: passed.

Comment thread ui-web/src/lib/uploadPaths.ts Outdated
@0xKT

0xKT commented Oct 11, 2026

Copy link
Copy Markdown
Member

Not a blocker -- three items from the re-acceptance pass on head 23296bc (merged onto main 6126965), beside the blocking thread on lib/uploadPaths. Each was reproduced through the real handlers with an isolated RAVEN_HOME, and I checked the code paths below on that tree.

Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

  1. A file attached in a draft is name-checked against a folder the conversation will not run in. fs.upload picks the folder to check from params.session (raven/rpc/methods/console.py:2107), and the page sends session: sessionCurrent() || '' (ui-web/src/app/install.ts:259). A draft has no id yet, so every file attached to a new conversation's first message is checked against _workspace_root(loop, ''). That is the launch directory under raven serve and <session root>/_ under the gateway (raven/agent/workdir.py:192-206). The conversation itself runs in the folder picked on the draft, or <session root>/tui. A folder can be picked only on a draft, so the first message of every pinned conversation is this case. The turn is spared by abs_path, but /file for the note path serves the older same-named file, so the bubble's chip opens it. Measured: gateway draft -> uploads/image.png, served from <root>/tui/uploads/image.png (the old file); the same upload from an existing tui: conversation -> uploads/image-1.png. The comment at :2101-2105 says a session that cannot be resolved falls back to the old rule, but '' resolves without error, to the wrong folder.
  2. deck.templates.pick mints into the same uploads/ without the new rule. deck_templates.deposit checks agent home only (raven/rpc/deck_templates.py:629), and the pick carries no session (DeckTemplatesPickParams has only name, raven/rpc/models.py:4069-4070; install.ts:268). So a picked template whose name the conversation root already holds answers a relative path that /file and a relative turn.send resolve to the project's older file. Measured: the pick answered uploads/amber.pptx and /file served the project's OLD-PROJECT-COPY, while fs.upload of the same name in the same conversation got amber-1.pptx. A relative turn.send received the shipped template on main and the project copy on this head. While the page stays open it hands the turn the pick's abs_path instead (ui-web/src/features/composer/store.ts:528). One minting helper, with the session on the pick's wire, would keep the rule in one place.
  3. An unreadable conversation folder now fails every upload with a bare internal_error. Only the root lookup is guarded (:2106-2112). The new (shadow_dir / target.name).exists() in the loop (:2116) is not, and on Python 3.13 Path.exists re-raises EACCES: measured, it raised PermissionError under a mode-000 folder. A pinned folder that macOS TCC denies to the engine, or one owned by another user, then turns every upload into -32603, where main stored the file. turn.send catches the same probe on purpose (raven/rpc/methods/turn.py:107-113).

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: distinguish uploaded attachments from dragged same-spelled session files.

The new cross-session reproduction changes my stance on this unchanged revision. I independently confirmed the shared page-lifetime mapping: after remember("uploads/image.png", "/agent-home/uploads/image.png"), a later mediaPath("uploads/image.png") for a dragged file from another session still returns the agent-home absolute path. Since mediaOf applies that lookup to every attachment path, the newly dragged file can be silently replaced by the earlier upload.

Focused verification remains green but does not cover this collision: npm test -- --run src/lib/uploadPaths.test.ts src/state/session/runtime.inject.test.ts src/chrome/Dock.test.tsx (46 tests passed).

My earlier clean stance is withdrawn; the open uploadPaths thread now blocks this revision from my side.

LivXue and others added 13 commits October 11, 2026 06:38
Dragging a picture from the transcript into the composer attached either
nothing (Chrome on Windows and Linux, and Firefox, hand an in-page drag
over with no File) or a re-upload of the File that Chrome on macOS names
`download`, with no extension, so the sent bubble drew a file chip where
the picture should have been.

A drop that began in this document and names this origin's /file route
in `text/uri-list` now attaches that file where it already is, by path,
with nothing read or uploaded; the address wins over any File the drop
also carries. A transcript picture still drawn from the upload's cached
data URL puts its /file address in the list when it is dragged. An
address is taken only when the drag that began here carried the same
path from its start, so a site the reader drags from cannot make the
composer attach an arbitrary local file.

A File that does go up is now named by its type when its name spells no
picture extension (download becomes download.png), which covers images
dragged in from other sites.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
The composer now attaches a picture dragged out of the transcript by
its own absolute path instead of uploading a copy, so the paths
_resolve_media receives are not only uploads under agent home.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
…ricted

With tools.restrict_to_workspace on, turn.send resolved every media path
against agent home and fenced it there. The viewer serves a picture from
the conversation's own working directory, so a picture the composer
attaches by its own path, from a session pinned outside agent home,
showed in the tray and was dropped from the turn without an error.

turn.send now asks for that directory the way /file does
(console._workspace_root), finds a relative path by the viewer's own
rule (viewer_root, which now takes agent home from its caller), and with
the setting on admits that directory beside agent home. A path that
neither root holds is still refused, and a loop that cannot name the
directory leaves agent home to stand in.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
tests/test_import_cycle_budget.py counted 45 modules inside an import
cycle against a ceiling of 44. The session-aware resolver had turn.send
import viewer_root and the session-root lookup from rpc.methods.console,
and the console already reaches turn through rpc.methods.session, so
the new edge closed console -> session -> turn -> console; the guard
counts a function-local import exactly like a top-level one.

viewer_root's rule and the session-root lookup now live in
raven/rpc/files.py, which both modules import and which reaches
neither. The console keeps its names (viewer_root as a wrapper that
reads agent home where fs.upload does, _workspace_root and _UPLOAD_DIR
as imports), so its callers and tests are unchanged.

Co-authored-by: Claude (claude-opus-5-5) <noreply@anthropic.com>
Name an upload past the session root's own uploads/ as well, so the
relative path fs.upload answers can never mean two files: viewer_root
reads the session's own root first, and both turn.send and /file follow
it, so a same-named file there would answer for the upload and the
reader would see one picture while the model was handed another.

Register the composer's two page-lifetime drag listeners with the page's
other document listeners (ui-web/CONTEXT.md's one place), and teach the
order test the two new rows.

Move the picture-extension table to lib/, read by the workspace viewer
and the composer both, so the tray and the sent bubble cannot disagree
about whether a name is a picture.

Give each engine path one note entry when a sent message is read back:
two entries can name suffixes of one path, and both reading it drew that
file twice and lost the other.

Pin the session key _media_root hands the resolver with a loop that
answers per key, which the shared-root stub could not tell from a
dropped one.

Co-authored-by: Claude (deepseek-flash) <noreply@anthropic.com>
fs.upload answers with a short path (uploads/<name>) and with the
absolute file it wrote. The note keeps the short one -- the bubble
renders it and /file re-roots it -- while the turn now receives the
absolute file: viewer_root reads the session's own root before agent
home, so a same-named file appearing there after the upload could
answer for it.

lib/uploadPaths keeps the answer by path, page-lifetime and keyed like
lib/attachmentCache, and mediaOf maps each path it recovers from the
note through it. A path no upload minted is left to viewer_root as
before, and so is a note restored after a reload, when the answer is
gone.

Pin it thrice: the store case shows an upload is remembered, the
runtime case shows the turn.send media carries the file, and the
turn.send case shows a restricted, pinned session admits the absolute
upload path.

Co-authored-by: Claude (deepseek-flash) <noreply@anthropic.com>
…wered

fs.upload answers uploads/<name> as well as the absolute file it wrote.
The composer staged the short one, and the send mapped it back through a
table kept for as long as the tab stays open and keyed by that spelling
alone. So once an upload had answered uploads/x.png, a different file
with that spelling - a session root's own uploads/x.png, dragged in from
the workspace pane - reached the turn as the upload. The two messages
carried byte-identical notes, so nothing that reads the text could tell
them apart.

The composer now stages an upload, and a picked template, by the file
the server wrote, falling back to the short path only where a host
answers none, and files the bubble's bytes under that same path. The
note names an upload absolutely and a dragged file by the path it was
dragged with, so mediaOf hands the turn the note's paths as written and
lib/uploadPaths goes. A sub-agent direct chat writes the same note, so
it stages an upload the same way.

The comments and docstrings that described the note as carrying the
short path, or uploads/<name> as never meaning two files, now say what
holds.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The composer now names an upload by the file fs.upload wrote, so a sent
bubble's chip asks /file for that absolute path. viewer_root made agent
home the root of uploads/<name> alone, so with
tools.restrict_to_workspace on, a session pinned elsewhere was refused
the absolute spelling of its own upload: the fence admitted the
session's root only. The same 403 met every upload chip after a
reload, since a reloaded bubble names the engine's absolute path.

viewer_root now decides by where a path lands: inside agent home's
uploads/ it is agent home's, spelled relatively or absolutely, and /file
asks it about both spellings. Judging by the first segment let
uploads/../notes/x.md, and a link left under uploads/, reach the rest of
agent home past the fence; both land outside uploads/ and so stay the
session's own question.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
fs.upload keeps a name free under the session's own uploads/ as well,
and asks with Path.exists, which raises EACCES rather than answering
False. A pinned folder the engine may not read - one macOS keeps from
it, or another user's directory - so turned every upload into a bare
-32603, where main stored the file. Only the root lookup was guarded.

A probe that raises now logs and leaves the name to agent home, as a
session whose root cannot be resolved already did.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
The comment above the name check said a session that cannot be resolved
falls back to the old rule. An empty session resolves without error, to
the resolver's policy default - the launch directory under raven serve,
<session root>/_ under raven gateway - so a draft's upload is checked
against a folder the conversation need not run in. Say so, and say what
covers it: the web UI's composer hands every reader the absolute file.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
workspace_root's docstring said an empty key falls through to the
resolver's policy default, "which for the gateway is the launch
directory". Under raven gateway the policy is per channel, and an empty
key has no channel, so it resolves to <session root>/_; the launch
directory is raven serve's default. The text moved here from the console
with the rest of the rule, so this branch is where it is corrected.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
Four comments this branch added spoke of "the page" for the whole
document and of the workspace pane as "the file panel", two senses
ui-web/CONTEXT.md retires: a page is a row of state/pages.ts, and the
workspace column is a pane. They now say the document and the workspace
pane. Comments only.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
FsUploadResult described path as the "workspace-relative path to hand
the agent" and abs_path not at all. turn.send looks for a relative path
under the conversation's own working directory before agent home, so a
same-named file there answers for uploads/<name>; only abs_path names
the upload whatever that directory holds, and it is what the web UI's
composer now sends. The contract says so in openrpc.json, its Python
mirror and both generated clients. Descriptions only.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
"serves a pinned session nothing else of agent home" claims a set the
test does not enumerate: it asks /file for four spellings that start in
uploads/ and lead out of it, or name a file beside it. Named for the
rule those four exercise instead.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
@LivXue
LivXue force-pushed the fix/composer_drop_transcript_image branch from 23296bc to 18486ec Compare October 11, 2026 07:15
@LivXue

LivXue commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

Answers to both board notes; the blocking lib/uploadPaths thread is answered in its thread.

The note on the d36d455 pass. Both items were fixed in c8c3abd ("close the composer drag-in findings from review"), which your re-acceptance pass covered: a reloaded bubble gives each engine line to one note entry, in order, and _media_root's session key is pinned by a loop that answers per key. They had no answer here until now. Re-run at the new head, a mutant that drops taken.add(at) and one that asks workspace_root(loop, "") each turn a test red.

The note on the 23296bc pass.

  1. A draft's upload is checked against the policy default. Confirmed in workdir.py: an empty key gives <session root>/_ under raven gateway and the launch directory under raven serve. Since 0203d97 no reader the web UI feeds uses the relative answer: the note names the upload by the file fs.upload wrote, so the turn and the bubble's chip both reach that file however the draft is later pinned. In the e2e on the blocking thread, the first conversation's upload went up as a draft (session: "") and /file answered 200 for it by its absolute path. 3b1ed42 corrects the comment you quoted, and b6a64fe the same claim in files.workspace_root's docstring, which said an empty key gives the launch directory under the gateway. The check itself is unchanged, since under raven serve the policy default is where a session-less caller's turn runs.

  2. deck.templates.pick mints without the rule. Confirmed. The composer now stages the pick's abs_path (0203d97, with a template test), so the web UI hands both /file and turn.send the shipped template. I have not put a session on the pick's wire: the web UI is its only caller and no longer reads the relative answer, and widening DeckTemplatesPickParams means openrpc and both hand mirrors for no remaining reader. It is listed under "Not covered" in the description.

  3. An unreadable session folder fails every upload. Reproduced: the test denies the probe with a stub, since root reads through a mode-000 folder, and as a non-root user Python 3.10's Path.exists raised PermissionError under one. Fixed in 4e205b3: a probe that raises is logged and leaves the name to agent home, as a session whose root cannot be resolved already did. The test asserts that the folder was asked and refused and that the upload landed; a mutant that catches the wrong exception turns it red.

The description now covers both rounds.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

I independently verified the cross-session collision fix: uploads are staged by abs_path, dragged session files retain their own paths, mediaOf preserves what the note records, and the file route admits the absolute upload only when it resolves inside agent-home uploads/. The real Dock send/drop regression covers both same-spelled files. The earlier board notes are addressed by the same absolute staging and the guarded unreadable-session-root probe.

I covered the repository rules (AGENTS.md, CLAUDE.md, CONTEXT-MAP.md, and ui-web/CONTEXT.md), the full github/main...HEAD diff, the revision delta, affected callers and history, backward compatibility through the path fallback, test changes (the deleted alias-table unit tests are replaced by an end-to-end Dock regression rather than weakened), and the global-listener and path-fencing architecture constraints.

Verification: 319 backend tests passed; 229 focused UI tests passed; TypeScript type-check and the source-language gate passed. ESLint completed with zero errors and four existing hook warnings; focused Ruff checks passed. My earlier upload-path thread remains resolved; I did not resolve the other reviewer’s thread.

This branch has not been deployed

No deployments
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.

3 participants