Skip to content

fix(web): host videos explain missing file access instead of offering a retry #21088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BGD-1DAY00 wants to merge 3 commits into
base: main
Choose a base branch
from
+60 −12
Open

fix(web): host videos explain missing file access instead of offering a retry #21088

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe0a9fb
fix(web): host videos explain missing file access instead of offering…
BGD-1DAY00 Oct 18, 2026
08412b8
fix: mobile host videos explain missing file access, and thread find …
BGD-1DAY00 Oct 18, 2026
b2a0acb
fix(mobile): announce why an unavailable video tile cannot play
BGD-1DAY00 Oct 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/mobile/src/components/MediaVideoPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ interface MediaVideoPlayerProps {
readonly thumbnailKey: string;
readonly thumbnailVisible?: boolean;
readonly unavailable?: boolean;
readonly unavailableDetail?: string;
readonly expanded?: boolean;
readonly autoPlay?: boolean;
readonly paused?: boolean;
Expand Down Expand Up @@ -163,7 +164,11 @@ function MediaVideoPlayerContent(props: MediaVideoPlayerProps) {
/>
) : null}
{props.unavailable ? (
<AppText className="text-sm text-white/80">Video unavailable</AppText>
<AppText className="text-center text-sm text-white/80">
{props.unavailableDetail
? `Video unavailable. ${props.unavailableDetail}`
: "Video unavailable"}

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 18, 2026

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 Include failure detail in the mobile accessibility label

When a host video is blocked on iOS or Android with VoiceOver/TalkBack enabled, this new text is nested inside a Pressable whose explicit accessibilityLabel remains Play ${props.name} (line 148). The pressable is the accessible element and that label replaces its descendant text, so users hear only “Play Demo, disabled” and never the pairing explanation; set a failure-specific label including unavailableDetail when unavailable is true.

Useful? React with 👍 / 👎.

@BGD-1DAY00 BGD-1DAY00 Oct 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed. Fixed in b2a0acb: when the tile is unavailable its accessibilityLabel is now ". Video unavailable. ", built from the same text the tile shows, so VoiceOver and TalkBack read the explanation. A playable tile still announces "Play ". Checked with tsc-rs --noEmit and lint in apps/mobile; not run on a device.

</AppText>
) : props.uri === null ? (
<AppActivityIndicator color="#ffffff" accessibilityLabel="Loading video" />
) : (
Expand Down
6 changes: 6 additions & 0 deletions apps/mobile/src/features/threads/ThreadMarkdownVideo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MISSING_FILE_ACCESS_MESSAGE } from "@t3tools/client-runtime/state/assets";
import { createContext, useContext } from "react";
import { MediaVideoPlayer } from "../../components/MediaVideoPlayer";
import { useAssetUrlState, useRefreshAssetUrl } from "../../state/assets";
Expand Down Expand Up @@ -39,6 +40,11 @@ export function ThreadMarkdownVideo(props: {
thumbnailKey={thumbnailKey}
thumbnailVisible={visible}
unavailable={"resource" in source && asset._tag === "Failure"}
unavailableDetail={
"resource" in source && asset._tag === "Failure" && asset.reason === "missing-file-access"
? MISSING_FILE_ACCESS_MESSAGE
: undefined
Comment on lines +43 to +46

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 18, 2026

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 Pass missing-access detail through video preview modals

The embedded-tile follow-up now passes this detail, but fresh evidence is the normal host-video link path: a link such as [Demo](/tmp/demo.mp4) is routed by ThreadFeed.tsx:1793-1803 directly to VideoPreviewModal, bypassing this component. In that path VideoPreviewModal.tsx:36-54 collapses asset.reason to a boolean and VideoPreviewModal.ios.tsx:46-52 replaces it with a generic connection error, so Android still displays only “Video unavailable” and iOS tells the user to check the connection even though re-pairing is required; propagate missing-file-access through both modal implementations as well.

AGENTS.md reference: AGENTS.md:L67-L70

Useful? React with 👍 / 👎.

}
actionsSource={source.actionsSource}
/>
);
Expand Down
32 changes: 32 additions & 0 deletions apps/web/src/components/ChatMarkdown.assets.browser.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { MISSING_FILE_ACCESS_MESSAGE } from "@t3tools/client-runtime/state/assets";
import { EnvironmentId, ThreadId } from "@t3tools/contracts";
import { AsyncResult } from "effect/reactivity";
import { act } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test";

const mint = vi.hoisted(() => vi.fn());
const deniedEnvironmentId = vi.hoisted(() => "denied-environment");
// Atoms derived from the server config run in the real registry over a missing config.
vi.mock(import("../state/server"), async (original) => {
const actual = await original();
Expand All @@ -25,6 +27,16 @@ vi.mock(import("../state/session"), async (original) => ({
useEnvironmentScope: () => false,
readPreparedConnection: (() => ({ httpBaseUrl: "https://host.test" })) as never,
}));
vi.mock(import("../state/filesystem"), async (original) => {
const actual = await original();
return {
...actual,
useFilesystemReadAccess: ((environmentId: EnvironmentId | null) =>
environmentId === deniedEnvironmentId
? { canReadFiles: false, isPending: false, error: null }
: actual.useFilesystemReadAccess(environmentId)) as never,
};
});

import ChatMarkdown from "./ChatMarkdown";

Expand Down Expand Up @@ -68,3 +80,23 @@ it("opens host media through server authorization before the client grant loads"
),
);
});

it("explains a host video the connection cannot read instead of offering a retry", async () => {
const container = document.body.appendChild(document.createElement("div"));
const root = createRoot(container);
roots.push(root);
await act(async () =>
root.render(
<ChatMarkdown
cwd="/repo"
threadRef={{
environmentId: EnvironmentId.make(deniedEnvironmentId),
threadId: ThreadId.make("media-thread"),
}}
text="![Demo](/tmp/demo.mp4)"
/>,
),
);
await vi.waitFor(() => expect(container.textContent).toContain(MISSING_FILE_ACCESS_MESSAGE));
expect(container.textContent).not.toContain("Retry video");
});
18 changes: 10 additions & 8 deletions apps/web/src/components/ChatMarkdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,7 @@ function ChatMarkdownVideo(props: {
readonly copyMarkdown: string | undefined;
readonly originalUrl?: string | undefined;
readonly sourceFailed?: boolean | undefined;
readonly unavailableDetail?: string | undefined;
readonly style?: CSSProperties | undefined;
readonly mediaIdentity?: string | undefined;
readonly actionsSource?: MediaActionSource | undefined;
Expand All @@ -1527,6 +1528,7 @@ function ChatMarkdownVideo(props: {
key={props.mediaIdentity ?? props.copyMarkdown ?? props.src}
src={props.src}
sourceFailed={props.sourceFailed}
unavailableDetail={props.unavailableDetail}
label={props.alt}
originalUrl={props.originalUrl}
style={props.style}
Expand Down Expand Up @@ -1592,6 +1594,10 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props
: undefined;
const relativePath = reference?.kind === "file" ? reference.relativePath : undefined;
const fallbackSrc = assetUrl._tag === "Failure" ? props.fallbackSrc : undefined;
const missingFileAccess =
assetUrl._tag === "Failure" &&
assetUrl.reason === "missing-file-access" &&
fallbackSrc === undefined;
const src =
assetUrl._tag === "Success"
? assetUrl.url + (props.srcFragment ?? "")
Expand Down Expand Up @@ -1633,12 +1639,14 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props
<ChatMarkdownVideo
src={src}
sourceFailed={assetUrl._tag === "Failure" && fallbackSrc === undefined}
unavailableDetail={missingFileAccess ? MISSING_FILE_ACCESS_MESSAGE : undefined}

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Propagate the access explanation to mobile videos

On the mobile client, when a host video is viewed over a connection without filesystem:read, ThreadMarkdownVideo still turns the access failure into only unavailable (apps/mobile/src/features/threads/ThreadMarkdownVideo.tsx:20-42), and the mobile player hard-codes only “Video unavailable” (apps/mobile/src/components/MediaVideoPlayer.tsx:165-166). The access explanation added here is therefore still absent on a first-class client; pass the missing-access reason/detail through the mobile video path too.

AGENTS.md reference: AGENTS.md:L67-L70

Useful? React with 👍 / 👎.

@BGD-1DAY00 BGD-1DAY00 Oct 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, it is the same defect on mobile. Fixed in 08412b8: ThreadMarkdownVideo passes MISSING_FILE_ACCESS_MESSAGE when the asset failed with missing-file-access, and the mobile tile shows "Video unavailable. ", the same wording ThreadMarkdownImage uses. The tile has no retry, so nothing else changes. Checked with tsc-rs --noEmit and lint in apps/mobile; not run on a device.

alt={props.alt}
copyMarkdown={props.copyMarkdown}
originalUrl={props.originalUrl}
style={props.style}
mediaIdentity={JSON.stringify([props.environmentId, props.resource, props.srcFragment])}
onRetry={refreshAssetUrl}
// A retry signs a new URL; it cannot grant the file access this connection lacks.
onRetry={missingFileAccess ? undefined : refreshAssetUrl}
actionsSource={actionsSource}
/>
);
Expand All @@ -1649,13 +1657,7 @@ export const ChatMarkdownAssetImage = memo(function ChatMarkdownAssetImage(props
key={JSON.stringify([props.environmentId, props.resource, props.srcFragment])}
src={src}
sourceFailed={assetUrl._tag === "Failure" && fallbackSrc === undefined}
unavailableDetail={
assetUrl._tag === "Failure" &&
assetUrl.reason === "missing-file-access" &&
fallbackSrc === undefined
? MISSING_FILE_ACCESS_MESSAGE
: undefined
}
unavailableDetail={missingFileAccess ? MISSING_FILE_ACCESS_MESSAGE : undefined}
alt={props.alt}
copyMarkdown={props.copyMarkdown}
standalone={props.standalone ?? true}
Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/components/media/MediaVideoPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ interface MediaVideoPlayerProps {
readonly src: string | null;
readonly label: string;
readonly sourceFailed?: boolean | undefined;
readonly unavailableDetail?: string | undefined;
readonly originalUrl?: string | undefined;
readonly revision?: string | null | undefined;
readonly preload?: "visible" | "metadata" | undefined;
Expand All @@ -31,6 +32,7 @@ export function MediaVideoPlayer({
src: latestSrc,
label,
sourceFailed = false,
unavailableDetail,
originalUrl,
revision = null,
preload = "visible",
Expand Down Expand Up @@ -137,9 +139,9 @@ export function MediaVideoPlayer({
stateClassName,
)}
>
<span className="inline-flex items-center gap-1.5">
<span data-thread-find-ignore className="inline-flex items-center gap-1.5">
<TriangleAlertIcon aria-hidden className="size-3.5 shrink-0" />
Video unavailable{label ? ` · ${label}` : ""}
{["Video unavailable", label, unavailableDetail].filter(Boolean).join(" · ")}

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 18, 2026

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 Exclude the generated detail from thread search

When an access-denied video appears before real message text containing a word from this new boilerplate (for example connection), thread find now assigns occurrence 0 to the generated status instead of the persisted message match. collectThreadFindRanges skips generated DOM only under [data-thread-find-ignore] (apps/web/src/components/chat/threadFindHighlights.ts:50-56), and the image fallback already sets that marker (ChatMarkdown.tsx:1352); mark this video label as ignored as well so local highlighting remains aligned with the server's persisted-text occurrence indexes.

Useful? React with 👍 / 👎.

@BGD-1DAY00 BGD-1DAY00 Oct 18, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 08412b8: the video label now carries data-thread-find-ignore, matching ChatMarkdownMediaUnavailableLabel for images, so thread find skips the generated text.

</span>
<span className="flex flex-wrap items-center justify-center gap-2">
{latestSrc !== null || onRetry ? (
Expand Down
Loading