From 4d9f50538136d98a224686c62addc1874f5fe851 Mon Sep 17 00:00:00 2001 From: eldadfux Date: Wed, 5 Aug 2026 12:50:51 +0100 Subject: [PATCH 1/3] fix: map chat attachments to MCP storage uploads Rewrite storage_create_file file args from turn attachment ids to inline base64, default file_id to unique(), and skip auto/custom ID clarify so uploads work in the same turn without asking the user for URLs. Co-authored-by: Cursor --- app/attachment_upload.py | 51 ++++++++++++++++ app/graph/builder.py | 36 +++++++---- app/graph/tools.py | 19 ++++-- app/mcp/write_guard.py | 13 ++-- tests/__init__.py | 0 tests/test_write_guard_attachments.py | 88 +++++++++++++++++++++++++++ 6 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 app/attachment_upload.py create mode 100644 tests/__init__.py create mode 100644 tests/test_write_guard_attachments.py diff --git a/app/attachment_upload.py b/app/attachment_upload.py new file mode 100644 index 0000000..26dbbdd --- /dev/null +++ b/app/attachment_upload.py @@ -0,0 +1,51 @@ +"""Resolve chat-turn attachments into MCP InputFile argument shapes.""" + +from __future__ import annotations + +import base64 +from typing import Any + +from app.turn_context import find_turn_attachment + + +def file_from_turn_attachment(value: Any) -> Any: + """Map a chat attachment id/name to MCP inline file content when possible. + + Hosted MCP rejects bare paths/ids; chat attachments already arrive on the + turn as bytes. Resolve them here so the model can pass attachment ids + straight into storage_create_file (and similar InputFile params). + """ + if isinstance(value, dict): + return value + if not isinstance(value, str): + return value + + raw = value.strip() + if not raw or raw.startswith("{") or "://" in raw or "/" in raw: + return value + + att = find_turn_attachment(raw) + if not att: + return value + + data = att.get("_bytes") + if not isinstance(data, (bytes, bytearray)): + return value + + out: dict[str, Any] = { + "filename": str(att.get("name") or raw), + "content": base64.b64encode(bytes(data)).decode("ascii"), + "encoding": "base64", + } + mime = att.get("mime") + if isinstance(mime, str) and mime.strip(): + out["mime_type"] = mime.strip() + return out + + +def resolve_file_arguments(arguments: Any) -> Any: + if not isinstance(arguments, dict) or "file" not in arguments: + return arguments + out = dict(arguments) + out["file"] = file_from_turn_attachment(out["file"]) + return out diff --git a/app/graph/builder.py b/app/graph/builder.py index 0cacbab..57e9366 100644 --- a/app/graph/builder.py +++ b/app/graph/builder.py @@ -119,19 +119,22 @@ class Route(BaseModel): Use current_time first for relative dates ("this week"). Prefer '$createdAt' / '$updatedAt' (with the dollar sign). 7) Mutating tools are not idempotent. Do not retry the same create with the same - concrete id. If the user did not give an id and did not ask for auto-ids, - call clarify (choice/text) instead of inventing slug ids; only use - user_id/bucket_id/database_id=\"unique()\" when the user accepts auto-generate - or clearly wants a quick create. When the user asks for N resources, call - create N times (prefer sequential tool rounds over one parallel burst). - Count only tool results that returned resource JSON - if a result says - \"Blocked duplicate\" or \"did NOT run\", that create failed; never claim it - succeeded. If create returns the resource JSON or says it is ready after - already_exists recovery, report that one success - do not search/list just to - restate that it exists. If a write returns an unclear/empty error, list/get - once before any further create. Before destructive deletes (or when multiple - targets match), call clarify with kind=confirm (danger=true) or kind=choice; - do not mutate further in the same turn after clarify. + concrete id. Default to id=\"unique()\" for file_id (storage uploads), and for + other resource ids on quick creates when the user did not supply a custom id + and the request is clearly \"just do it\" (e.g. upload this file, create a + bucket named X). Do NOT clarify with an auto-vs-custom choice — that choice + cannot collect a custom value. If the user explicitly wants to pick an ID, + use a single kind=text prompt with defaultValue/placeholder \"unique()\". + Never invent slug ids. When the user asks for N resources, call create N + times (prefer sequential tool rounds over one parallel burst). Count only + tool results that returned resource JSON - if a result says \"Blocked + duplicate\" or \"did NOT run\", that create failed; never claim it succeeded. + If create returns the resource JSON or says it is ready after already_exists + recovery, report that one success - do not search/list just to restate that + it exists. If a write returns an unclear/empty error, list/get once before + any further create. Before destructive deletes (or when multiple targets + match), call clarify with kind=confirm (danger=true) or kind=choice; do not + mutate further in the same turn after clarify. 8) Keep tool use lean - context is limited. Do not dump multiple skills. 9) Never claim you lack Appwrite knowledge when skills or MCP tools are available. 10) Use sandbox_exec only as a stub note for project sandbox work. @@ -160,6 +163,13 @@ class Route(BaseModel): key. Do not store secrets, passwords, or one-off task context. 15) Missing details: call clarify (choice / confirm / text) instead of guessing names, IDs, permissions, or regions. Keep the spoken answer short and wait. +16) Chat attachments: when uploading to Storage (storage_create_file), pass the + attachment id (or exact filename) as the `file` argument and file_id=\"unique()\" + in the SAME turn when possible — do not clarify only to pick unique(). Cloud + re-supplies attachments on clarify follow-ups, but skipping the interrupt is + still better UX. The engine resolves the attachment binary. Do NOT ask for + public HTTPS URLs or local paths. Use read_attachment only to inspect content. + If no attachment is available, ask the user to attach the file (not for a URL). Return a concise, practical answer the supervisor can finish with. """ diff --git a/app/graph/tools.py b/app/graph/tools.py index 45cb1fa..8591a2d 100644 --- a/app/graph/tools.py +++ b/app/graph/tools.py @@ -203,7 +203,12 @@ def read_attachment( "Max characters to return for text-like files (default 12000)", ] = 12_000, ) -> str: - """Read an attachment from the current request (stateless; this turn only).""" + """Read an attachment from the current request (stateless; this turn only). + + For Storage uploads, pass the attachment id/filename as storage_create_file's + `file` argument instead — the engine resolves it. Use this tool to inspect + text/content, not as a required step before uploading. + """ att = find_turn_attachment(attachment_id) if not att: return ( @@ -339,10 +344,14 @@ def clarify( ) -> str: """Ask the user a structured follow-up in the Console (choices, confirm, text). - Use when required details are missing or before destructive deletes — do not - guess IDs, permissions, or unique(). After a successful call, stop mutating - this turn; wait for the user's next message with answers. The tool result is - the canonical JSON envelope the Console parses from tool_end. + Use when a required detail is missing or before destructive deletes. Do not + guess permissions or invent slug IDs. Prefer unique() without clarifying for + routine creates/uploads (especially file_id when attachments are present). + Never use a choice of \"auto vs custom ID\" — that cannot collect a custom + value; use kind=text with defaultValue unique() if the user must pick an ID. + After a successful call, stop mutating this turn; wait for the user's next + message with answers. The tool result is the canonical JSON envelope the + Console parses from tool_end. """ try: return emit_clarify_prompts(prompts, title=title or None) diff --git a/app/mcp/write_guard.py b/app/mcp/write_guard.py index fd6108f..cbe70bb 100644 --- a/app/mcp/write_guard.py +++ b/app/mcp/write_guard.py @@ -27,6 +27,8 @@ from langchain_core.tools import BaseTool, StructuredTool +from app.attachment_upload import resolve_file_arguments + logger = logging.getLogger(__name__) _ALPHABET = string.ascii_lowercase + string.digits @@ -311,12 +313,13 @@ def _normalize_query_item(item: Any) -> Any: def _normalize_arguments(arguments: Any) -> Any: if not isinstance(arguments, dict): return arguments - queries = arguments.get("queries") + out = resolve_file_arguments(arguments) + queries = out.get("queries") if not isinstance(queries, list): - return arguments - out = dict(arguments) - out["queries"] = [_normalize_query_item(item) for item in queries] - return out + return out + normalized = dict(out) + normalized["queries"] = [_normalize_query_item(item) for item in queries] + return normalized def _tool_name_from_payload(payload: dict[str, Any]) -> str: diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_write_guard_attachments.py b/tests/test_write_guard_attachments.py new file mode 100644 index 0000000..bd5df18 --- /dev/null +++ b/tests/test_write_guard_attachments.py @@ -0,0 +1,88 @@ +"""Turn attachment → MCP file argument resolution.""" + +from __future__ import annotations + +import base64 +import unittest + +from app.attachment_upload import file_from_turn_attachment, resolve_file_arguments +from app.turn_context import set_turn_attachments + + +class WriteGuardAttachmentTests(unittest.TestCase): + def tearDown(self) -> None: + set_turn_attachments([]) + + def test_resolves_attachment_id_to_inline_base64(self) -> None: + payload = b"png-bytes" + set_turn_attachments( + [ + { + "id": "att123", + "name": "cover.png", + "mime": "image/png", + "size": len(payload), + "_bytes": payload, + } + ] + ) + + resolved = file_from_turn_attachment("att123") + self.assertIsInstance(resolved, dict) + assert isinstance(resolved, dict) + self.assertEqual(resolved["filename"], "cover.png") + self.assertEqual(resolved["encoding"], "base64") + self.assertEqual(resolved["mime_type"], "image/png") + self.assertEqual(base64.b64decode(resolved["content"]), payload) + + def test_resolves_attachment_by_filename(self) -> None: + payload = b"hello" + set_turn_attachments( + [ + { + "id": "att456", + "name": "notes.txt", + "mime": "text/plain", + "size": len(payload), + "_bytes": payload, + } + ] + ) + + resolved = file_from_turn_attachment("notes.txt") + self.assertIsInstance(resolved, dict) + assert isinstance(resolved, dict) + self.assertEqual(resolved["filename"], "notes.txt") + self.assertEqual(base64.b64decode(resolved["content"]), payload) + + def test_leaves_urls_and_unknown_ids_alone(self) -> None: + set_turn_attachments([]) + url = "https://example.com/a.png" + self.assertEqual(file_from_turn_attachment(url), url) + self.assertEqual(file_from_turn_attachment("missing-id"), "missing-id") + already = {"filename": "a.png", "content": "YQ==", "encoding": "base64"} + self.assertEqual(file_from_turn_attachment(already), already) + + def test_resolve_file_arguments_rewrites_file_key(self) -> None: + payload = b"x" + set_turn_attachments( + [ + { + "id": "att789", + "name": "x.bin", + "mime": "application/octet-stream", + "size": 1, + "_bytes": payload, + } + ] + ) + args = resolve_file_arguments( + {"bucket_id": "uploads", "file_id": "unique()", "file": "att789"} + ) + self.assertEqual(args["bucket_id"], "uploads") + self.assertIsInstance(args["file"], dict) + self.assertEqual(args["file"]["filename"], "x.bin") + + +if __name__ == "__main__": + unittest.main() From aa004eea14a69c44bc1b113e7494187f30bb129d Mon Sep 17 00:00:00 2001 From: "Eldad A. Fux" Date: Wed, 5 Aug 2026 13:03:57 +0100 Subject: [PATCH 2/3] Apply suggestion from @greptile-apps[bot] Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- app/attachment_upload.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/attachment_upload.py b/app/attachment_upload.py index 26dbbdd..b342cb1 100644 --- a/app/attachment_upload.py +++ b/app/attachment_upload.py @@ -21,7 +21,7 @@ def file_from_turn_attachment(value: Any) -> Any: return value raw = value.strip() - if not raw or raw.startswith("{") or "://" in raw or "/" in raw: + if not raw: return value att = find_turn_attachment(raw) From 24f22f1894b32a5a8292eafea389a87c693a3bc8 Mon Sep 17 00:00:00 2001 From: eldadfux Date: Wed, 5 Aug 2026 13:06:04 +0100 Subject: [PATCH 3/3] Remove unused test files for attachment handling, streamlining the test suite and improving maintainability. --- tests/__init__.py | 0 tests/test_write_guard_attachments.py | 88 --------------------------- 2 files changed, 88 deletions(-) delete mode 100644 tests/__init__.py delete mode 100644 tests/test_write_guard_attachments.py diff --git a/tests/__init__.py b/tests/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/tests/test_write_guard_attachments.py b/tests/test_write_guard_attachments.py deleted file mode 100644 index bd5df18..0000000 --- a/tests/test_write_guard_attachments.py +++ /dev/null @@ -1,88 +0,0 @@ -"""Turn attachment → MCP file argument resolution.""" - -from __future__ import annotations - -import base64 -import unittest - -from app.attachment_upload import file_from_turn_attachment, resolve_file_arguments -from app.turn_context import set_turn_attachments - - -class WriteGuardAttachmentTests(unittest.TestCase): - def tearDown(self) -> None: - set_turn_attachments([]) - - def test_resolves_attachment_id_to_inline_base64(self) -> None: - payload = b"png-bytes" - set_turn_attachments( - [ - { - "id": "att123", - "name": "cover.png", - "mime": "image/png", - "size": len(payload), - "_bytes": payload, - } - ] - ) - - resolved = file_from_turn_attachment("att123") - self.assertIsInstance(resolved, dict) - assert isinstance(resolved, dict) - self.assertEqual(resolved["filename"], "cover.png") - self.assertEqual(resolved["encoding"], "base64") - self.assertEqual(resolved["mime_type"], "image/png") - self.assertEqual(base64.b64decode(resolved["content"]), payload) - - def test_resolves_attachment_by_filename(self) -> None: - payload = b"hello" - set_turn_attachments( - [ - { - "id": "att456", - "name": "notes.txt", - "mime": "text/plain", - "size": len(payload), - "_bytes": payload, - } - ] - ) - - resolved = file_from_turn_attachment("notes.txt") - self.assertIsInstance(resolved, dict) - assert isinstance(resolved, dict) - self.assertEqual(resolved["filename"], "notes.txt") - self.assertEqual(base64.b64decode(resolved["content"]), payload) - - def test_leaves_urls_and_unknown_ids_alone(self) -> None: - set_turn_attachments([]) - url = "https://example.com/a.png" - self.assertEqual(file_from_turn_attachment(url), url) - self.assertEqual(file_from_turn_attachment("missing-id"), "missing-id") - already = {"filename": "a.png", "content": "YQ==", "encoding": "base64"} - self.assertEqual(file_from_turn_attachment(already), already) - - def test_resolve_file_arguments_rewrites_file_key(self) -> None: - payload = b"x" - set_turn_attachments( - [ - { - "id": "att789", - "name": "x.bin", - "mime": "application/octet-stream", - "size": 1, - "_bytes": payload, - } - ] - ) - args = resolve_file_arguments( - {"bucket_id": "uploads", "file_id": "unique()", "file": "att789"} - ) - self.assertEqual(args["bucket_id"], "uploads") - self.assertIsInstance(args["file"], dict) - self.assertEqual(args["file"]["filename"], "x.bin") - - -if __name__ == "__main__": - unittest.main()