feat(DATAGO-133319): Support attaching existing artifacts to chat messages#1429
Merged
feat(DATAGO-133319): Support attaching existing artifacts to chat messages#1429
Conversation
amir-ghasemi
changed the title
amir-ghasemi
force-pushed
the
amir/feat-attach
branch
from
17dea48 to
80f69f5
Compare
✅ FOSSA Guard: Licensing (
|
✅ FOSSA Guard: Vulnerability (
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new chat attachment flow that lets users attach existing artifacts (by reference) in addition to uploading new files, and unifies attachment previews/removal across files, artifact refs, and pasted text.
Changes:
- Introduces an “Attach existing artifact” dialog with search, dedupe, and multi-select.
- Updates
ChatInputAreasubmission to send artifact references as special file envelopes (no re-upload) and adds preview dialogs for local files and artifact refs. - Refactors artifact preview logic into a shared
StandaloneArtifactPreviewcomponent (used by both Chat and Artifacts page) and centralizes attachment-type helpers.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/solace_agent_mesh/common/features/features.yaml | Adds artifact_attachment feature flag. |
| client/webui/frontend/src/lib/components/chat/ChatInputArea.tsx | Adds artifact-ref attachment state, unified attachment row, preview dialogs, and reference-envelope submission. |
| client/webui/frontend/src/lib/components/chat/file/AttachArtifactDialog.tsx | New dialog for selecting existing artifacts with search + infinite scroll. |
| client/webui/frontend/src/lib/components/chat/file/ArtifactAttachmentCard.tsx | New attachment card UI for artifact references with preview thumbnail/text snippet behavior. |
| client/webui/frontend/src/lib/components/chat/file/FileUploadCard.tsx | New attachment card UI for local (not-yet-uploaded) files. |
| client/webui/frontend/src/lib/components/chat/file/LocalFilePreview.tsx | New modal preview for local files. |
| client/webui/frontend/src/lib/components/chat/file/StandaloneArtifactPreview.tsx | Shared artifact preview panel (and helpers) used across pages. |
| client/webui/frontend/src/lib/components/pages/ArtifactsPage.tsx | Switches to shared StandaloneArtifactPreview and shared helpers. |
| client/webui/frontend/src/lib/components/chat/file/attachmentUtils.ts | Adds shared attachment-type helpers/constants. |
| client/webui/frontend/src/lib/components/chat/file/AttachmentCardShell.tsx | Shared card shell for attachment cards. |
| client/webui/frontend/src/lib/components/chat/file/FileBadge.tsx | Enhances badge to support click behavior + leading icon. |
| client/webui/frontend/src/lib/components/chat/file/index.ts | Exports new attachment/preview components. |
| client/webui/frontend/src/lib/components/chat/paste/PastedTextBadge.tsx | Aligns pasted-text card height with new attachment cards. |
| client/webui/frontend/src/lib/components/chat/file/DocumentThumbnail.tsx | Updates comment to reflect new global CSS location. |
| client/webui/frontend/src/lib/index.css | Adds global thumbnail-* rules for DocumentThumbnail. |
| client/webui/frontend/src/App.css | Removes obsolete thumbnail-* rules (App.css not imported). |
| client/webui/frontend/src/stories/chat/.test. | Adds/updates unit tests for new helpers and attachment flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+16
to
+20
| // understood by parseArtifactUri. | ||
| const resolveArtifactUri = (artifact: ArtifactWithSession): string | null => { | ||
| if (artifact.uri) return artifact.uri; | ||
| if (!artifact.sessionId || !artifact.filename) return null; | ||
| return `artifact://${artifact.sessionId}/${artifact.filename}`; |
There was a problem hiding this comment.
resolveArtifactUri interpolates artifact.filename directly into an artifact://… URI. If filenames contain reserved URL characters (e.g. #, ?, %, or even /), the synthesized URI can become invalid or parse to the wrong filename when parseArtifactUri uses new URL(). Encode the filename segment (and/or use new URL() to construct) so the synthesized URI is always well-formed and round-trips through parseArtifactUri.
Suggested change
| // understood by parseArtifactUri. | |
| const resolveArtifactUri = (artifact: ArtifactWithSession): string | null => { | |
| if (artifact.uri) return artifact.uri; | |
| if (!artifact.sessionId || !artifact.filename) return null; | |
| return `artifact://${artifact.sessionId}/${artifact.filename}`; | |
| // understood by parseArtifactUri. Encode the filename as a single path segment so | |
| // reserved URL characters in filenames round-trip correctly through URL parsing. | |
| const resolveArtifactUri = (artifact: ArtifactWithSession): string | null => { | |
| if (artifact.uri) return artifact.uri; | |
| if (!artifact.sessionId || !artifact.filename) return null; | |
| const url = new URL(`artifact://${artifact.sessionId}/`); | |
| url.pathname = `/${encodeURIComponent(artifact.filename)}`; | |
| return url.toString(); |
Comment on lines
+80
to
+91
| const handleAttach = () => { | ||
| const selected = visibleArtifacts | ||
| .filter(({ artifact }) => selectedKeys.has(keyFor(artifact))) | ||
| // Ensure the outgoing artifact carries a resolved `uri` so downstream | ||
| // consumers (the chat submit pipeline) don't need to re-resolve it. | ||
| .map(({ artifact, resolvedUri }) => ({ ...artifact, uri: resolvedUri ?? artifact.uri })); | ||
| if (selected.length === 0) return; | ||
| onAttach(selected); | ||
| onClose(); | ||
| }; | ||
|
|
||
| const selectedCount = selectedKeys.size; |
There was a problem hiding this comment.
Selection state can get out of sync with filtering: selectedCount is selectedKeys.size, but handleAttach() only attaches items present in visibleArtifacts. If a user selects an artifact and then changes the search so the selected item is filtered out, the Attach button stays enabled (and may show “Attach N”) but clicking it is a no-op because selected.length === 0. Compute selectedCount from the currently attachable artifacts (or clear/prune selectedKeys when the search/artifact list changes) so the button label/disabled state matches what will be attached.
Suggested change
| const handleAttach = () => { | |
| const selected = visibleArtifacts | |
| .filter(({ artifact }) => selectedKeys.has(keyFor(artifact))) | |
| // Ensure the outgoing artifact carries a resolved `uri` so downstream | |
| // consumers (the chat submit pipeline) don't need to re-resolve it. | |
| .map(({ artifact, resolvedUri }) => ({ ...artifact, uri: resolvedUri ?? artifact.uri })); | |
| if (selected.length === 0) return; | |
| onAttach(selected); | |
| onClose(); | |
| }; | |
| const selectedCount = selectedKeys.size; | |
| const selectedArtifacts = useMemo( | |
| () => | |
| visibleArtifacts | |
| .filter(({ artifact }) => selectedKeys.has(keyFor(artifact))) | |
| // Ensure the outgoing artifact carries a resolved `uri` so downstream | |
| // consumers (the chat submit pipeline) don't need to re-resolve it. | |
| .map(({ artifact, resolvedUri }) => ({ ...artifact, uri: resolvedUri ?? artifact.uri })), | |
| [visibleArtifacts, selectedKeys], | |
| ); | |
| const handleAttach = () => { | |
| if (selectedArtifacts.length === 0) return; | |
| onAttach(selectedArtifacts); | |
| onClose(); | |
| }; | |
| const selectedCount = selectedArtifacts.length; |
Comment on lines
+451
to
+465
| const handlePreviewArtifactDownload = useCallback( | ||
| async (artifact: ArtifactWithSession) => { | ||
| try { | ||
| const response = await api.webui.get(getArtifactApiUrl(artifact), { fullResponse: true }); | ||
| const blob = await response.blob(); | ||
| const downloadUrl = window.URL.createObjectURL(blob); | ||
| const link = document.createElement("a"); | ||
| link.href = downloadUrl; | ||
| link.download = artifact.filename; | ||
| document.body.appendChild(link); | ||
| link.click(); | ||
| document.body.removeChild(link); | ||
| window.URL.revokeObjectURL(downloadUrl); | ||
| addNotification?.(`Downloaded ${artifact.filename}`, "success"); | ||
| } catch (error) { |
There was a problem hiding this comment.
handlePreviewArtifactDownload doesn’t check response.ok before reading response.blob() and showing a success notification. If the API returns an error status (e.g. 403/404), this will still download the error payload (often HTML/JSON) and report success. Mirror the existing pattern used elsewhere (e.g. getArtifactContent) by checking response.ok and throwing a descriptive error when it’s false.
Comment on lines
+52
to
+53
| const sessionId = isProjectArtifact(artifact) ? undefined : artifact.sessionId; | ||
| const projectId = isProjectArtifact(artifact) ? artifact.projectId : undefined; |
There was a problem hiding this comment.
Project/session context selection here differs from getArtifactApiUrl’s contract: StandaloneArtifactPreview.getArtifactApiUrl only takes the project branch when the artifact is project-scoped and projectId is present (tests cover the fallback). In this card, isProjectArtifact(artifact) forces sessionId to undefined even when projectId is missing, which makes getArtifactContent throw (no valid context) and prevents previews for legacy/partial project artifacts. Align the logic with getArtifactApiUrl (treat as project only when artifact.projectId is set; otherwise fall back to sessionId).
Suggested change
| const sessionId = isProjectArtifact(artifact) ? undefined : artifact.sessionId; | |
| const projectId = isProjectArtifact(artifact) ? artifact.projectId : undefined; | |
| const hasProjectContext = isProjectArtifact(artifact) && !!artifact.projectId; | |
| const sessionId = hasProjectContext ? undefined : artifact.sessionId; | |
| const projectId = hasProjectContext ? artifact.projectId : undefined; |
Collaborator
|
Could you review the co-pilot suggestions above along with: Potential-blockers
Review
Nits
|
amir-ghasemi
force-pushed
the
amir/feat-attach
branch
from
1396bad to
930e674
Compare
Collaborator
|
Testing manually, just a few observations? Q1. I think this dialog could be improved? Suggestions:
Q2. Unrelated, but I noticed this odd thumbnail for an xlsx file? 
Q3. Suggestion: Add some standard dialog padding here? 
Q4: If selecting a file from a project where that file has a description, the description is lost. Expected? |
Collaborator
Author
Project badge:
|
Collaborator
Author
|
Each row's metadata line is now · · 
|
Collaborator
Author
|
More padding |
- resolveArtifactUri: encode filename so reserved URL chars (#, ?, %, /) round-trip through new URL() parsing - AttachArtifactDialog: derive selectedCount from currently attachable artifacts so the button label/disabled state stay in sync when search filters out a selected item - handlePreviewArtifactDownload: throw on !response.ok before reading the blob (was reporting "Downloaded" on 4xx/5xx HTML) - decodeBase64Snippet: trim to a 4-byte boundary before atob, which previously threw and silently returned "" for many text artifacts (1400 isn't divisible by 4) - getArtifactApiUrl: encode sessionId, not just filename - addNotification: drop optional chaining (it's required on the ChatContext) - Replace template-literal className with cn() per repo convention - Extract magic 60_000 ms blob-revoke timeout to a named constant
- Move isProjectArtifact + getArtifactApiUrl from StandaloneArtifactPreview.tsx to @/lib/api/artifacts (the presentational component is no longer the home of pure helpers). Three callers updated: ChatInputArea, ArtifactsPage, and the helpers test. - ArtifactAttachmentCard project/session derivation now mirrors getArtifactApiUrl: only treat as project-scoped when projectId is actually set, so project artifacts missing a projectId fall back to a session-scoped fetch instead of producing an unfetchable request. - StandaloneArtifactPreview hides Go-to-Project when projectId is missing (silent no-op replaced with no button). The card-level handlers drop their now-redundant if-checks. - FileBadge: replace native <button> with the design-system Button (ghost variant) per repo conventions, padding stripped to keep the badge tight.
The file's only consumer was the Storybook preview import, but the two rules it carried (#root container styling and a .logo animation) target selectors no story or component renders. The DocumentThumbnail rules previously living here were already moved to src/lib/index.css.
…phore - Extract `useLocalFileSource` and `useArtifactSource` hooks plus a shared `<FilePreviewBody>` component, eliminating ~80% duplication between LocalFilePreview and StandaloneArtifactPreview. Each component now owns only its header chrome. - Replace StandaloneArtifactPreview's useRef<Map> versionCache with React Query: `useQuery` keyed by version handles caching, and `queryClient.prefetchQuery` warms adjacent versions for instant swaps. - Promote the 4-permit fetch semaphore from ArtifactsPage to a module-level `@/lib/api/artifacts/fetchSemaphore` so the chat attachment row and the artifacts page share one connection budget (4 caps the concurrent fetches across both views). - ArtifactAttachmentCard's preview queryFn now goes through that semaphore, so a chat input with many attached cards no longer saturates the browser's connection budget.
- Move attachment-domain state and handlers out of ChatInputArea into a dedicated useChatAttachments hook: selectedFiles, selectedArtifactRefs, pendingPastedTextItems, the three modal flags, and ten handlers that mutate them. ChatInputArea drops ~120 lines. - Capture/restore-on-failure now lives in the hook as captureAndClearForSubmit / restoreFromCapture, with a per-slice "user explicitly cleared during in-flight submit" flag. The restoreFromCapture path skips slices the user just dropped — the prior length>0 guard only protected against added items, not intentional removals. Adds a regression test covering the new path.
- Replace the application/x-artifact-reference fake-File envelope
with a typed artifactReferences: AttachedArtifactRef[] parameter
on ChatContext.handleSubmit. The provider iterates the new arg
directly and emits FilePart{uri} objects, dropping the
read-text/JSON.parse step that was conflating "bytes to upload"
with "pointer to existing artifact".
- Both freshly-uploaded pasted-text artifacts and the user's
existing-artifact picks now flow through the typed path.
- User messages gain `attachedArtifacts` so the chat bubble still
displays attachment badges for ref-only artifacts; ChatMessage
renders them next to uploadedFiles.
- Update the submitRestore envelope test to assert the new arg shape.
The four skipped tests across ArtifactAttachmentCard.test.tsx and AttachArtifactDialog.test.tsx were blocked on a known workspace limitation: vi.mock can't reliably intercept transitive imports (DocumentThumbnail's pdfjs worker, IntersectionObserver under useAllArtifacts's refetchOnMount path). The Storybook project runs in real Playwright Chromium where both work natively. - Delete ArtifactAttachmentCard.test.tsx entirely (all three tests were skipped); replace with ArtifactAttachmentCard.stories.tsx covering PPTX/DOCX precedence (binary office files must NOT render via the text-snippet path) and the legitimate text path. - Delete the skipped infinite-scroll test in AttachArtifactDialog.test.tsx; replace with AttachArtifactDialog.stories.tsx driving the sentinel-to-loadMore flow through a real IntersectionObserver and MSW-backed paginated responses. All four Storybook play tests pass; no .skip remains on the branch.
…ersion
Two bugs surfaced when attaching existing artifacts:
1. The bulk /artifacts/all endpoint omitted `uri` on every record, so
AttachArtifactDialog synthesized a 2-segment legacy form
(artifact://{sessionId}/{filename}). The agent translator and
resolver both require the canonical 4-segment form
(artifact://{app_name}/{user_id}/{session_id}/{filename}); they
raised "Invalid artifact URI format" and rejected the attached
file part.
2. Once the canonical URI was generated server-side with a baked-in
?version=N, attaching a GIF whose metadata-file version had drifted
from its content version raised "Artifact 'X.metadata.json' version
N not found." The metadata-version-at-list-time isn't a stable
identifier — using "latest" at fetch time avoids the drift.
Fix:
- Bulk list helpers (`get_artifact_info_list_fast` and
`get_artifact_info_list`) now emit the canonical artifact:// URI
with no `?version=` query.
- The translator and resolve-uri helper default missing `?version=`
to "latest", which `load_artifact_content_or_metadata` already
resolves against the live version list.
- Frontend `resolveArtifactUri` no longer fabricates legacy URIs —
records without a backend-provided `uri` are hidden as unattachable.
`_prepare_a2a_filepart_for_adk` was looking up artifact metadata under the receiving agent's own app_name, ignoring the netloc the URI carried. Artifacts attached via the WebUI's existing-artifact dialog live under the gateway's app_name (e.g. "A2A_WebUI_App"), not the agent's — so list_versions returned empty and surfaced as "Could not determine latest version because the artifact has no versions available." Mirrors the contract already followed by `_resolve_artifact_uri_in_part` in common/a2a/artifact.py: trust the netloc baked into the URI.
The previous fix taught `_prepare_a2a_filepart_for_adk` to use the
URI's netloc as app_name, but the metadata-load call still used the
agent's own `user_id` and `session_id` — so a WebUI artifact at
artifact://A2A_WebUI_App/{user}/{web-session}/file resolved against
the **agent's** session, not the user's. list_versions returned
empty under that wrong namespace and the user saw "Could not
determine latest version: no versions available."
Trust the URI's path coordinates end-to-end: extract user_id and
session_id from path_parts and use them for both inline-vision and
metadata loads. The agent's own user/session remain the fallback
when the URI doesn't carry them.
The previous patches taught `_prepare_a2a_filepart_for_adk` to honor the URI's app/user/session for the metadata read, but the LLM's downstream `load_artifact` tool only ever looks under the agent's own context — there's no way for the LLM to express "fetch from that other namespace." So the metadata text summary surfaced correctly, but the LLM's first attempt to read content failed with "version 0 not found or has no data." Fix: in the URI branch, load the bytes from the URI's source namespace and immediately re-save them under the agent's namespace (same mechanism the bytes branch already uses for fresh uploads). After that, the metadata-load and every subsequent tool call hit a local copy and resolve cleanly.
The attachedArtifacts field on user messages — added so the chat bubble can show badges for artifacts attached by reference (no uploaded File blob to render) — was only set in-memory by ChatProvider. Without it in the serialize/deserialize path, the badges vanished after a navigation away and back.
Each row in the existing-artifact picker now exposes a version selector that mirrors the look of the side-panel \`ArtifactDetails\` selector (h-[16px] py-0 text-xs trigger). Default is "Latest" — the URI omits ?version= and the agent-side translator resolves the latest version at fetch time, which avoids the metadata- vs-content version drift we hit earlier. Picking a specific version encodes it as ?version=N on the URI so the agent fetches that exact snapshot. Versions are lazy-fetched per-row (Radix Select onOpenChange) via a new useArtifactVersions hook, so a list with 30+ artifacts doesn't fan out N list_versions calls upfront — only the rows the user actually clicks pay for the lookup. Adds a Storybook play test that drives the picker end-to-end and asserts the URI gets `?version=1` after a non-latest pick.
The "Latest" label in the attach-artifact dialog implied live-sync semantics that don't match reality — when the agent processes the message, the selected version is snapshot into its namespace and frozen. Replace it with the actual resolved-latest version number so the snapshot is visible up front. Backend: - The bulk artifacts endpoint now returns the resolved-latest `version` per record. Free to compute: load_artifact_content_or_metadata already calls list_versions internally to resolve "latest"; we just stop discarding the result. Frontend: - AttachArtifactDialog's per-row picker drops the "Latest" sentinel. Default value = the artifact's backend-reported version. The full version list still lazy-loads on first open; until then the picker shows just the latest as its only option. - The emitted URI always carries `?version=N` (concrete, never implicit). The agent translator's "latest"-fallback path remains for legacy URIs but is no longer exercised by the dialog. Tests + the Storybook play test updated to match the new contract.
…oint The bulk /artifacts/all endpoint already drops .converted.txt derivatives and the BM25 index, but artifacts tagged __working (RAG intermediates, workflow scratch files, structured-invocation working state, etc.) flowed through. They surfaced in the new attach-artifact dialog as user-pickable rows even though they're not meant for direct consumption.
- Fixed-width extension pill (w-12 + text-center) so filenames line up vertically across rows regardless of label length (WEBP vs TXT vs WASM vs DOCX). - Hide the version picker for project-scoped artifacts. Project knowledge files only expose their single latest version even when multiple versions exist inside the project, so a picker would be misleading. - Hide the version picker for single-version artifacts (artifact.version === 0). Versions are 0-indexed and sequential, so a latest of 0 implies exactly one version exists — nothing to choose between.
…apping
- Project artifacts now render the shared ProjectBadge component
(same one used in RecentChatsPage, ChatPage header, and
ArtifactsPage) instead of an inline Folder icon + plain text.
Session artifacts keep the chat-bubble icon + chat title.
- The mime/size/scope metadata line was wrapping when the mime type
was long: long mime types like
application/vnd.openxmlformats-officedocument.presentationml.presentation
pushed the size text ("2.74 KB") onto a second line and broke
inside the literal space, giving rows of inconsistent height.
Add whitespace-nowrap on the line + flex-shrink-0 on the size span
so only the truncatable mime type yields space.
ArtifactsPage's local supportsTextPreview matches any mime containing "xml" — which catches xlsx (application/vnd.openxmlformats-officedocument.spreadsheetml.sheet), docx (.wordprocessingml.document), and pptx (.presentationml.presentation). When binaryArtifactPreviewEnabled is off OR LibreOffice isn't installed, canAttemptDocumentThumbnail is false for those Office docs and the fetch chain fell through to the text branch, downloaded the zip bytes, and rendered them as text — producing visible "PK\x03\x04…[Content_Types].xml" gibberish in the card. Same precedence rule the attachment utilities already enforce: documents take priority over text. Gate the text branch on !isDocumentThumbnailSupported so Office files always fall back to the file-icon placeholder when their thumbnail can't be produced, never to a text dump.
The mime-type column dominated each row with strings like 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' even with truncation. The colored extension pill on the left already identifies the file type, so the mime was debug-level noise. Replace with formatRelativeTime(last_modified) — matches what the side-panel ArtifactDetails selector and ArtifactsPage cards already show. The metadata line is now: <size> · <relative time> · <scope badge>.
Drop the `p-0` override on the preview-modal DialogContent so the artifact preview gets the shadcn DialogContent default padding (p-6) — matches every other dialog in the app and gives the preview header/body room to breathe. The `border-l` on the StandaloneArtifactPreview / LocalFilePreview outer wrappers was a leftover from the side-panel layout — it painted a stray vertical line at the dialog's left edge. Move it to the side-panel call site (ArtifactsPage) where it actually serves as the separator between the artifact grid and the preview panel.
The blanket rehydrate path I introduced for cross-namespace URI references (WebUI artifacts attached by reference into agent tasks) was running for *every* URI FilePart, including ones that already pointed at the agent's own (app_name, user_id, session_id). For those, save_artifact_with_metadata bumped the version (e.g. v0 → v1 because metadata already existed at v0), and the LLM saw a metadata header for v1 even though the URI explicitly referenced v0. Compare the URI's coordinates to the agent's own. If they match, skip the load+save round trip and let the metadata-only load below work directly against the URI's path-extracted version. The cross-namespace case (different app/user/session) still rehydrates into the agent's namespace so load_artifact remains reachable. Restores the integration scenario `test_filepart_by_reference` and preserves the fix for the WebUI attach-existing-artifact flow.
amir-ghasemi
force-pushed
the
amir/feat-attach
branch
from
387484e to
54a9940
Compare
lgh-solace
approved these changes
Apr 29, 2026
The card's inline text preview was rendering blank lines as filler rows: "Hello\n\nworld" produced Hello [blank row] ... because slice(0, 2) took the first two source lines (the second being empty) and the ellipsis fired on `lines.length > 2`. Filter out blank/whitespace-only lines before slicing so the card shows up to two content lines, with the ellipsis only appearing when there's genuinely more meaningful content to see.
The previous layout used the 3rd row for a standalone "…" marker when more content was available — wasting a row that could carry real preview text. Render up to 3 non-blank content lines instead; when there's still more beyond the 3rd line, append "…" to that 3rd line itself. Denser preview, same overall card height.
The card snippet was mixing two ellipsis styles in the same card — the first two lines were CSS-truncated and rendered the single character "…" via text-overflow: ellipsis, while the third line (when short enough to fit the card width) showed our code's literal three dots "...". Switch to the Unicode ellipsis "…" everywhere so both paths produce the same glyph regardless of which truncation kicks in.
The preview modal said "Preview not yet supported" for xlsx even
though the small attachment-card thumbnail rendered fine. Same
LibreOffice-backed conversion path that DocumentThumbnail uses
already works for xlsx in the backend; the modal just lacked the
wiring.
- Add isXlsxFile + a "xlsx" branch in getRenderType.
- Add "xlsx" to RENDER_TYPES so canPreviewArtifact accepts it.
- Widen OfficeDocumentRenderer's documentType union to include
"xlsx" (its ?ext={documentType} call already routes to the right
conversion).
- Add a "xlsx" case to ContentRenderer that delegates to
OfficeDocumentRenderer.
OfficeDocumentRenderer sends content straight to the backend's /api/v1/document-conversion/to-pdf endpoint, which calls base64.b64decode() on it. The endpoint expects base64-encoded bytes. `getFileContent` only passes raw (still-base64-encoded) content through for renderer types listed in RENDER_TYPES_WITH_RAW_CONTENT — image, audio, docx, pptx. Adding xlsx to the renderer list without also adding it to RENDER_TYPES_WITH_RAW_CONTENT meant the content was decoded to bytes before the renderer got it; the renderer then sent decoded binary text to the backend, which 400'd on b64decode. Add "xlsx" to RENDER_TYPES_WITH_RAW_CONTENT alongside docx/pptx.
Filenames containing spaces or reserved chars (e.g. 'Financial Sample.xlsx') were percent-encoded in the URI by the WebUI's WHATWG-URL re-serialization (`new URL(...).toString()`), but the agent translator and the gateway resolver split the path and used the raw segments as artifact-store keys. The store key is the raw filename, so the lookup failed with Artifact 'Financial%20Sample.xlsx' version 1 not found. Decode every path segment (and netloc) with urllib's `unquote` in all four URI parsers — translation, common/a2a/artifact, gateway resolver, and parse_artifact_uri — so the keys passed to the artifact service match what was actually stored.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds support for attaching existing artifacts (files previously uploaded in any chat or project) to new chat messages, in addition to uploading new files. The UI for file and artifact attachments is unified, and users can now preview both uploaded files and referenced artifacts before sending. The implementation ensures that artifact references are sent securely and efficiently, without re-uploading data.
Artifact Attachment and Preview Enhancements:
Submission and State Management Improvements:
Code and Style Cleanups:
DocumentThumbnailstyles fromApp.cssand clarified their new location.These changes make file and artifact management in chat more flexible and intuitive, while maintaining secure and efficient backend integration.