Skip to content

fix(desktop): remote-mode chat file links download via the fs bridge instead of dead file:// URLs - #44538

Closed
AIalliAI wants to merge 1 commit into
NousResearch:mainfrom
AIalliAI:fix/44523-remote-media-dead-links
Closed

fix(desktop): remote-mode chat file links download via the fs bridge instead of dead file:// URLs#44538
AIalliAI wants to merge 1 commit into
NousResearch:mainfrom
AIalliAI:fix/44523-remote-media-dead-links

Conversation

@AIalliAI

@AIalliAI AIalliAI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #44523

Problem

In remote-gateway mode, the chat's file-link fallback (MediaAttachment's final branch in apps/desktop/src/components/assistant-ui/markdown-text.tsx) renders an anchor whose click handler opens mediaExternalUrl(path) — a file:// URL built from the gateway's absolute path. On the client machine that path doesn't exist, so the click silently does nothing.

Two inputs funnel into this dead end:

  1. Images outside the /api/media roots — the endpoint 403s (intentionally; the fence is unchanged here), inline render fails, and the fallback anchor is the only affordance left.
  2. Non-image files (e.g. PDFs), even inside the media roots/api/media is image-extension-allowlisted (415), so there is no remote display path for them at all.

The audio/video OpenMediaButton shown when playback fails had the same dead file:// handler.

Fix

In remote mode, route the fallback through the existing authenticated GET /api/fs/read-data-url endpoint (added with the remote file browser in #44326 — the Files-panel preview already reads these same files over it), and hand the bytes to the user as a browser download (same fetch→blob→anchor pattern as zoomable-image.tsx / session-export.ts). The endpoint's existing size cap is honored: when the gateway refuses the read (413/403/404), the UI now shows an explicit "couldn't fetch from the gateway" note instead of silently no-opping.

Local mode is unchanged — file:// URLs are correct there and open with the OS default app.

The /api/media roots fence is not widened (per the issue: that behavior is deliberate, and #42778 tracks configurability). No server-side changes at all.

Changes

  • apps/desktop/src/lib/media.ts: new downloadGatewayMediaFile() — fetches via readDesktopFileDataUrl (the /api/fs/read-data-url facade) and triggers a client-side download.
  • apps/desktop/src/components/assistant-ui/markdown-text.tsx: shared useOpenMediaFile hook branches remote→download / local→file://; used by both the fallback anchor and OpenMediaButton; renders a failure note when the fetch fails.
  • apps/desktop/src/lib/media.test.ts: new unit tests (download path, file:// prefix stripping, gateway-refusal and empty-response rejection).

Testing

  • npx vitest run --environment jsdom src/lib/media.test.ts src/lib/desktop-fs.test.ts src/components/assistant-ui/markdown-text.test.ts — 23/23 pass.
  • tsc -p . --noEmit and eslint on changed files — clean.
  • Full desktop vitest suite: the only failures (pane-shell width override, model-settings, streaming reasoning card, toolset-config, gateway-boot reconnect) fail identically on unmodified main in this environment — pre-existing, unrelated.

@AIalliAI

Copy link
Copy Markdown
Contributor Author

Requesting maintainer review — this is ready to land from my side. Standalone fork CI is pending first-run approval here; the rollup branch in #44061 carrying this session's batch is fully green on upstream CI (all test shards, typecheck, e2e).

@AIalliAI
AIalliAI force-pushed the fix/44523-remote-media-dead-links branch from a8cd040 to 3b2f2b4 Compare June 14, 2026 17:44
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jun 14, 2026
@AIalliAI

Copy link
Copy Markdown
Contributor Author

@OutThisLife mind reviewing? In remote mode this routes chat file links through the fs bridge instead of the dead file:// URLs that currently fail to open.

…idge instead of dead file:// URLs

In remote-gateway mode, MediaAttachment's fallback anchor (and the
audio/video Open-file button) opened file://<gateway path> — a path that
only exists on the gateway machine, so the click silently did nothing.
This hit both images outside the /api/media roots (403) and every
non-image file such as PDFs (415, image-only allowlist), leaving no way
to get those files client-side at all.

Route the remote-mode fallback through the existing authenticated
GET /api/fs/read-data-url endpoint (the same one the remote file
browser preview uses) and hand the bytes to the user as a download,
honoring the endpoint's size cap. When the gateway refuses the read,
render an explicit failure note instead of a silent no-op. Local mode
keeps the file:// open-with-OS behavior.

Fixes NousResearch#44523
@AIalliAI
AIalliAI force-pushed the fix/44523-remote-media-dead-links branch from 3b2f2b4 to 6b243ad Compare June 20, 2026 04:49
@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) and removed comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026
@Aculnaj Aculnaj mentioned this pull request Jun 30, 2026
19 tasks
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused remote-mode fix. This is already implemented on current main, so this PR is redundant.

  • Automated hermes-sweeper review verified that apps/desktop/src/components/assistant-ui/markdown-text.tsx:132-145 sends remote fallback opens through downloadGatewayMediaFile() and shows a gateway-read failure note.
  • apps/desktop/src/lib/media.ts:127-147 reads through the authenticated filesystem bridge, converts the data URL to a Blob, and triggers a download rather than opening a gateway-local file:// path.
  • apps/desktop/src/lib/media.remote.test.ts:108-153 covers both the bridge download and gateway refusal behavior.
  • The implementation is present via 03406ae2553e802f11399129c3f376a096bbec4f and shipped in v2026.7.7 (Hermes Agent v0.18.1).

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
@AIalliAI
AIalliAI deleted the fix/44523-remote-media-dead-links branch July 14, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

3 participants