Skip to content

Hand the agent a file for anything that is not a picture - #200

Merged
Jing-yilin merged 3 commits into
mainfrom
fix/attach-non-images-by-reference
Sep 28, 2026
Merged

Jing-yilin merged 3 commits into
mainfrom
fix/attach-non-images-by-reference

Conversation

@Jing-yilin

@Jing-yilin Jing-yilin commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #199, which did this for boards.

Problem

A video, a note or a drawing the person placed on a canvas was drawn with toImage and sent to the agent as a picture. The agent can't watch a video, and a note or a drawing is a record it can read directly.

Fix

  • canvasAttach.tsx: an item the person placed (anything but an image shape) is attached with reference: true, as a board already is. Pictures, from the library or dropped in by the person, still go to the agent as images.
  • The flag is now called reference instead of page, since it no longer means only a board.
  • server/agent.ts resolves any <slug>/<path>, not only <slug>/<file>.html. That covers files/clip.mp4 and canvas.json#shape:…, looked up in the canvases of the project the item was attached from. Each part of the path is checked: none may start with a dot or contain a backslash.
  • The chat still shows the tile preview and the transcript thumbnail as before.

Test

tsc -b and vitest run pass. The release note under Unreleased is broadened to match.

🤖 Generated with Claude Code


Devin Review

A video, a note or a drawing the person placed was drawn and sent as a
picture, as a board was. Now only pictures go as pictures; the rest keep
their drawing for the tile and point the agent at the file behind them,
`files/…` or `canvas.json#<id>`, in the project they were attached from.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Devin Review

Comment thread canvas/src/agents.test.ts
data: "CCC",
path: "/tmp/sp-chat-r/2.png",
page: "/proj/canvases/shop/01-home.html",
reference: "/proj/canvases/shop/01-home.html",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 Video and shape references lack tests

The existing test only renames a board reference. Neither new video paths nor shape-record references are exercised, despite the viewer-test requirement in CONTRIBUTING.md.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2efcbee.

Comment thread canvas/server/agent.ts Outdated
Comment on lines +514 to +515
// Anything but a picture — a board, a video, a note — keeps its picture for the
// panel, and the agent is pointed at its file, `<slug>/<path>` in the canvases of

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 New comments contain em dashes

The changed comments here and in canvasAttach.tsx and agents.ts use em dashes, contrary to REVIEW.md.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 2efcbee.

Comment thread canvas/server/agent.ts Outdated
Comment on lines +527 to +529
rest.every(
(s) => s && !s.startsWith(".") && !s.includes("\\"),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟥 Attached references escape canvas folders

A reference containing x/.. passes the component check, then path.join normalizes it outside the canvas folder. The agent receives an attacker-chosen local path instead of a canvas file.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A .. part starts with a dot, so the check rejects it before path.join runs; x/.. never resolves. 2efcbee moves this into canvasFile (sp.ts) with a test covering shop/../../etc, shop/files/.. and backslash paths.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Deploying super-prototyping with  Cloudflare Pages  Cloudflare Pages

Latest commit: f78f602
Status: ✅  Deploy successful!
Preview URL: https://65ea039a.super-prototyping.pages.dev
Branch Preview URL: https://fix-attach-non-images-by-ref.super-prototyping.pages.dev

View logs

The resolution moves to sp.ts as canvasFile, with a test for a board, a
video, a shape record, an app canvas, and names that try to leave the
folder. Drop the em dashes REVIEW.md rules out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2efcbee970

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread canvas/src/canvasAttach.tsx Outdated
kind: "board",
name: personsShapeName(editor, target, slug),
src: URL.createObjectURL(blob),
reference: target.type === "image" ? undefined : true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Flush canvas.json before handing off its path

When a user creates or edits a note/drawing and immediately attaches and sends it, this switches the agent from the freshly rendered snapshot to canvas.json without flushing or awaiting the canvas-content save. installCanvasContent waits 500 ms before starting that write, so the agent can read an older record—or no record at all—while the chat thumbnail shows the current shape. Await saveNow(slug) before dispatching a reference to canvas.json.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f78f602: the + awaits saveNow(slug) before handing over a reference to one of the person's shapes.

A note drawn a moment ago is still waiting out the half second before
canvas.json is written, so the agent could read the record from before
it. The + now awaits saveNow first, as an agent write already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Jing-yilin
Jing-yilin merged commit 85a71b2 into main Sep 28, 2026
10 checks passed
@Jing-yilin
Jing-yilin deleted the fix/attach-non-images-by-reference branch September 28, 2026 05:41
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