Skip to content

fix(desktop): resolve local media linked from remote threads - #10619

Merged
maria-rcks merged 3 commits into
pingdotgg:mainfrom
maria-rcks:t3code/render-local-media-links
Sep 8, 2026
Merged

fix(desktop): resolve local media linked from remote threads#10619
maria-rcks merged 3 commits into
pingdotgg:mainfrom
maria-rcks:t3code/render-local-media-links

Conversation

@maria-rcks

@maria-rcks maria-rcks commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

remote threads try to load desktop images and recordings from the remote server, where the files may not exist. desktop now retries eligible missing absolute media paths against its local server. remote files retain priority; relative paths and authorization failures do not fall back. previews refresh when the local server reconnects.

validation: 74 focused signing/rpc/asset/query tests passed before simplification; 46 focused asset/query tests, client-runtime/web typechecks, and scoped lint passed on this head. github checks pass.

e2e: real linux electron client, two independently authenticated servers, and a seeded remote conversation. filesystem permissions made the media inaccessible to the remote server and readable by the desktop server. image rendering and full-size preview, eight-second video playback, seeking to four seconds, and retry after restoring a missing video passed. checked light/dark appearance and 1100×780 / 1200×1100 viewports. the mac-specific setup remains untested.

before: base client asset wiring, same thread and files.

before: remote thread cannot load local image or video

after: current head, image loaded and video seeked to 0:04.

after: local image and video render in the remote thread

eight seconds of desktop video playback through local fallback

model: gpt-6. harness: codex.

Note

Fix local media resolution for remote threads via local environment fallback

  • Adds direct asset URL issuance for absolute media-file paths in the assetsCreateUrl websocket RPC handler in ws.ts, so absolute media can be served without resolving a workspace for the referenced thread.
  • Updates createAssetEnvironmentAtoms in assets.ts to retry eligible absolute media-file requests against a distinct configured local environment after asset-not-found, asset-inspection, or workspace-context errors, rewriting the result against the local HTTP base URL.
  • Supplies the connected primary Electron environment as the local media fallback in assets.ts, and refreshes media queries when that fallback context changes.
  • Adds integration tests in server.test.ts and parameterized fallback tests in assets.test.ts.
  • Behavioral Change: media URL queries now perform an additional local-environment retry for absolute paths with recognized media extensions when the target environment returns specific workspace-resolution failures; non-media and relative paths are unaffected.

Macroscope summarized 20bd171.

Summary by CodeRabbit

  • Bug Fixes
    • Media assets using absolute paths now generate valid URLs even when no local thread workspace is available.
    • Improved media URL resolution across local, remote, reconnecting, and foreign-thread environments.
    • PNG and MP4 assets now return the expected content types and file data.
    • Relative media paths continue to report clear workspace-context errors when required context is unavailable.
    • Added support for Windows-style absolute media paths and filenames containing literal special characters.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 7, 2026
Comment thread apps/server/src/ws.ts
Comment thread packages/client-runtime/src/state/assets.ts Outdated
@macroscopeapp

This comment has been minimized.

@maria-rcks
maria-rcks marked this pull request as ready for review September 8, 2026 01:40
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 20bd171

Macroscope's review found this PR approvable — This is a narrowly scoped asset-resolution bug fix: remote-thread media failures can retry against the connected local Electron server, while relative paths, authorization failures, and existing successful paths remain unchanged. The production changes reuse the existing signed asset endpoint and are backed by focused fallback, reconnect, and server integration tests.

You can add or adjust custom eligibility rules. Learn more.

@macroscopeapp
macroscopeapp Bot dismissed their stale review September 8, 2026 01:45

Dismissing prior approval to re-evaluate 20bd171

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 7120bb80-5c06-446c-aa67-d7b90cf2ef32

📥 Commits

Reviewing files that changed from the base of the PR and between 8616cef and 20bd171.

📒 Files selected for processing (4)
  • apps/web/src/state/assets.ts
  • packages/client-runtime/src/state/assets.test.ts
  • packages/client-runtime/src/state/assets.ts
  • packages/client-runtime/src/state/runtime.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The change adds direct server handling for absolute media paths and client fallback through a local media environment. It wires the web state to the primary Electron connection and adds integration and parameterized coverage.

Changes

Media asset URL resolution

Layer / File(s) Summary
Server absolute-path handling
apps/server/src/ws.ts, apps/server/src/server.test.ts
assetsCreateUrl issues URLs directly for absolute media-file paths. Tests cover PNG and MP4 responses, file contents, and relative-path errors.
Client media fallback
packages/client-runtime/src/state/runtime.ts, packages/client-runtime/src/state/assets.ts, packages/client-runtime/src/state/assets.test.ts
Asset URL queries inspect request results, retry eligible media requests through a local environment, and re-evaluate after reconnects. Tests cover environment selection, fallback, and error cases.
Web local environment wiring
apps/web/src/state/assets.ts
The web state derives a local media environment from the primary Electron connection and passes it to asset environment atoms.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 20bd1

Desktop media previews now retry eligible missing absolute paths through the local server while preserving remote success, relative-path behavior, and authorization failures. No concrete merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RemoteEnvironment
  participant EnvironmentRegistry
  participant LocalEnvironment
  Client->>RemoteEnvironment: Request assetsCreateUrl
  RemoteEnvironment-->>Client: Return URL or workspace error
  Client->>EnvironmentRegistry: Select fallback environment
  EnvironmentRegistry->>LocalEnvironment: Request assetsCreateUrl
  LocalEnvironment-->>Client: Return local-origin URL
Loading

Suggested reviewers: juliusmarminge, stienswout, t3dotgg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: resolving local media for remote threads in the desktop client.
Description check ✅ Passed The description clearly explains what changed, why it changed, fallback behavior, validation results, UI evidence, and known test limitations. It does not use the template headings or include the chec…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@maria-rcks
maria-rcks merged commit a01b227 into pingdotgg:main Sep 8, 2026
26 checks passed
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 8, 2026
Merges `pingdotgg/t3code` `8b2838e0e..a37c664` — 43 commits.

`343` files landed against `343` changed in the upstream range; fork
delta `723` files. Exact match, so nothing upstream changed was dropped.

Details in
[`docs/fork/upstream-merge-log.md`](../blob/merge/upstream-2026-09-08/docs/fork/upstream-merge-log.md).

## Two fork deltas this merge had to re-apply

**Upstream split the server-update banner into two routes.** pingdotgg#10596
added `useAutoBalanceUpdateBanner` beside the single-machine condition
the fork already gates. The conflict was on the first line only, so
resolving it correctly still left the auto-balance route ungated — an
auto-balanced project would have been offered `npx t3` against a backend
that does not implement `server.updateServer`.
`FEATURES.serverUpdateBanner` now carries two gates in `ChatView.tsx`.

**A new settings page needs a gate even though it degrades politely.**
pingdotgg#8103 added `/settings/snap-shot` for desktop window capture. Every
control drives `window.desktopBridge`, and upstream renders an
"unavailable" notice rather than hiding the page, so a hosted build
listed a sidebar section and six searchable rows for a feature it can
never run. Gated with `FEATURES.snapShots`.

Two smaller fixes: `packages/moatless-api` still ran `tsgo --noEmit`
after upstream replaced `@typescript/native-preview` with TypeScript
7.0.2, and `duplicate-adds.mjs` now skips `pnpm-lock.yaml` (it read
`iconv-lite: 0.6.3` as taken twice; `d3-dsv` and `encoding` each declare
it).

## Usable as-is

- Stop-thread keybinding command (pingdotgg#4308).
- Project import tolerates servers that predate the git-identity scan
(pingdotgg#10547).
- Proactive panels open when entering a thread (pingdotgg#10610); pull-request
markdown links open in the panel (pingdotgg#10623); markdown images navigate as
galleries (pingdotgg#10625); pull-request videos play inline (pingdotgg#10617).
- Settings project scopes are searchable and scrollable (pingdotgg#10570); ref
picker stays steady when opening (pingdotgg#9472); sidebar timer uses
`tabular-nums` (pingdotgg#10592); popup triggers stay steady when pressed
(pingdotgg#9468); settled PR colors restore on hover (pingdotgg#10023).
- Composer Fast mode persists across new chats (pingdotgg#2981); inserted
citations are removed on cancel (pingdotgg#10518).
- TypeScript 7.0.2 (pingdotgg#10663) and the knip desktop-export rules (pingdotgg#10269).

## Unsupported in Moatless / needs implementation

- **Cross-platform window capture** (pingdotgg#8103) —
`apps/desktop/src/snapShot/**`,
`apps/web/src/components/settings/SnapShotSettings.tsx`,
`apps/web/src/lib/desktopSnapShot.ts`. Needs an Electron
`window.desktopBridge`; a browser tab has none. Gated behind
`FEATURES.snapShots` in this PR.
- **Auto-balance server update** (pingdotgg#10596) —
`apps/web/src/components/chat/useAutoBalanceUpdateBanner.tsx`. Needs
`server.updateServer`, which Moatless does not dispatch. Gated behind
`FEATURES.serverUpdateBanner` in this PR.
- **Preview recording transfer** (pingdotgg#10572) —
`apps/server/src/mcp/toolkits/preview/handlers.ts`,
`apps/web/src/browser/browserRecordingUpload.ts`. Moves a finished
preview recording into the agent environment over the desktop bridge.
Adds four error types to `packages/contracts/src/previewAutomation.ts`
and no new RPC method, so no union changed. Sits behind the
`previewAutomation.connect` / `focusHost` / `respond` gap already in the
register.
- **Local media linked from remote threads** (pingdotgg#10619) and **browser
editing shortcuts** (pingdotgg#10621) — Electron shell only.
- **iOS Keychain access group** (pingdotgg#3665) and the mobile provider account
badge (pingdotgg#9899) — the fork ships no mobile build against Moatless.

## Backend behavior to consider reproducing in Moatless

- **Name the usage limit and its reset instead of relaying "out of
credits"** (pingdotgg#10473, `apps/server/src/provider/**` Codex adapter).
Moatless owns its provider runtime, so the clearer limit message has to
be produced there.
- **Report usage limits on retried turns** (pingdotgg#10549, Claude adapter). A
retry currently loses the limit signal; same ownership.
- **Disable executable capabilities in Claude metadata generation**
(pingdotgg#4169, `apps/server/src/textGeneration/ClaudeTextGeneration.ts`). Title
and metadata generation should not be able to run tools. Worth mirroring
wherever Moatless generates thread titles.

## Verification

`verify.mjs`: duplicate-adds, tripwires, resolution-check,
unsupported-methods (0 ADD, 0 DROP, 2 KEEP), fmt, lint and typecheck all
pass.

Tests pass except `@t3tools/desktop`, which cannot compile
`scripts/browser-secret-native.test.mjs` because the sandbox has no
`libsecret-1` — 1283 tests pass, 0 fail, and the file is byte-identical
to upstream. New entry in `docs/fork/gaps.md`. `t3` failed
`GrokAdapter.test.ts` once under parallel load and passes 42/42 alone.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/6d8ea486-2fcb-4c25-bd34-dcd15cc4a7ac
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 8, 2026
## What's Changed
* fix(web): open proactive panels when entering threads by @maria-rcks in pingdotgg/t3code#10610
* fix(native): wait for the KDE feedback test listener by @juliusmarminge in pingdotgg/t3code#10645
* fix(desktop): resolve local media linked from remote threads by @maria-rcks in pingdotgg/t3code#10619
* fix(web): add bottom padding to project actions header by @flamboh in pingdotgg/t3code#10634
* fix(web): update machines together in auto balance by @maria-rcks in pingdotgg/t3code#10596
* fix(preview): transfer recordings to the agent environment by @maria-rcks in pingdotgg/t3code#10572
* fix(web): navigate markdown images as galleries by @maria-rcks in pingdotgg/t3code#10625
* chore: upgrade to TypeScript 7.0.2 by @juliusmarminge in pingdotgg/t3code#10663
* fix: hide email-bearing account labels in usage limits by @juliusmarminge in pingdotgg/t3code#10668
* fix(web): keep scroll-to-end button close to composer by @Bil0000 in pingdotgg/t3code#10543
* chore(deps): upgrade Effect to rc.112 and Alchemy to beta.76 by @juliusmarminge in pingdotgg/t3code#10652
* chore(refs): sync Effect reference to rc.112 by @juliusmarminge in pingdotgg/t3code#10653
* chore(refs): sync Alchemy reference to beta.76 by @juliusmarminge in pingdotgg/t3code#10654
* fix: generate thread titles with the selected model across connections by @Bil0000 in pingdotgg/t3code#10526
* fix(desktop): enable context menus in the browser by @juliusmarminge in pingdotgg/t3code#10670
* fix(desktop): stop generating declarations during bundling by @juliusmarminge in pingdotgg/t3code#10679
* fix(desktop): restore layout control hit targets by @juliusmarminge in pingdotgg/t3code#10673


**Full Changelog**: pingdotgg/t3code@v0.0.41-nightly.20260908.1377...v0.0.41-nightly.20260908.1387

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.41-nightly.20260908.1387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant