Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
149 changes: 143 additions & 6 deletions apps/mobile/src/features/threads/ThreadFeed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@ import {
type CodexArtifactTemplate,
} from "@t3tools/shared/codexArtifactTemplates";
import { resolveAssetUrl } from "@t3tools/client-runtime/state/assets";
import { isMarkdownFileLinkLabel } from "@t3tools/shared/markdownLinks";
import {
isMarkdownFileLinkLabel,
resolveMarkdownFileLinkTarget,
resolveMarkdownFileLinkThreadTarget,
} from "@t3tools/shared/markdownLinks";
import { formatFilePathPosition, splitFilePathPosition } from "@t3tools/shared/fileLinks";
import { buildThreadFilePathIndex, threadFilePaths } from "@t3tools/shared/threadFilePaths";
import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments";
import { settlePromise, squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime";
import {
Expand Down Expand Up @@ -214,6 +220,8 @@ import { useAtomQueryRunner } from "../../state/use-atom-query-runner";
import { usePreparedConnection } from "../../state/session";
import { useLiveThreadLinkLabels } from "../../state/entities";
import { useThreadSelection } from "../../state/use-thread-selection";
import { useThreadVisibleTurnItems } from "../../state/use-thread-detail";
import { projectEnvironment } from "../../state/projects";
import { composerDocumentAttachmentRecord } from "../../lib/composerContext";
import * as Option from "effect/Option";
import { appAtomRegistry } from "../../state/atom-registry";
Expand Down Expand Up @@ -1669,6 +1677,25 @@ function ThreadFeedPlaceholder(props: {
);
}

/** A host path as a link destination, encoded so parsing it decodes back to the same path. */
function fileLinkHref(pathWithPosition: string): string {
const position = splitFilePathPosition(pathWithPosition);
return formatFilePathPosition({
...position,
path: position.path
.split(/([\\/])/)
.map((part) => (part === "/" || part === "\\" ? part : encodeURIComponent(part)))
.join(""),
});
}

interface ThreadFileBindings {
readonly scope: string;
readonly bound: Map<string, string>;
readonly pending: Map<string, Promise<string>>;
readonly menu: Map<string, string>;
}

export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) {
const { listRef, onEndFollowEnabledChange, onHeaderMaterialVisibilityChange } = props;
const workflowOpening = useWorkflowOpening(props.environmentId, props.threadId);
Expand Down Expand Up @@ -1785,6 +1812,107 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) {
const iconSubtleColor = theme["--color-icon-subtle"];
const screenColor = theme["--color-screen"];
const userBubbleColor = theme["--color-user-bubble"];
// A relative file link may name a file the agent touched by its absolute
// path outside the workspace root. Keyed by content so streaming updates that
// name no new path keep the same index.
const visibleTurnItems = useThreadVisibleTurnItems({
environmentId: props.environmentId,
threadId: props.threadId,
});
const threadFilePathsKey = threadFilePaths(
visibleTurnItems.map((row) => row.item),
props.workspaceRoot ?? null,
).join("\n");
const threadFilePathIndex = buildThreadFilePathIndex(
threadFilePathsKey === "" ? [] : threadFilePathsKey.split("\n"),
);
// Verified hrefs hold only for the thread, root and index they were checked
// against; the root can change while the index stays the same. A chip's menu
// remembers the href it showed, so its actions use the file it named. Held in
// a ref because these caches must outlive renders, and only handlers touch it.
const fileBindingScope = JSON.stringify([
props.environmentId,
props.threadId,
props.workspaceRoot ?? null,
threadFilePathsKey,
]);
const fileBindingsRef = useRef<ThreadFileBindings | null>(null);
const fileBindings = () => {
if (fileBindingsRef.current?.scope !== fileBindingScope) {
fileBindingsRef.current = {
scope: fileBindingScope,
bound: new Map(),
pending: new Map(),
menu: new Map(),
};
}
return fileBindingsRef.current;
};
const readFileMetadata = useAtomQueryRunner(projectEnvironment.fileMetadata, {
reportFailure: false,
});
const fileBindRequest = useRef(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium threads/ThreadFeed.tsx:1832

A slow metadata lookup can navigate to ThreadFile for the previous thread, or reopen its preview, after the user switches threads or leaves ThreadFeed. fileBindRequest is only invalidated by a later link action, so invalidate it when the environment or thread changes and on unmount.

   const fileBindRequest = useRef(0);
+  useEffect(
+    () => () => {
+      fileBindRequest.current += 1;
+    },
+    [props.environmentId, props.threadId],
+  );
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadFeed.tsx around line 1832:

A slow metadata lookup can navigate to `ThreadFile` for the previous thread, or reopen its preview, after the user switches threads or leaves `ThreadFeed`. `fileBindRequest` is only invalidated by a later link action, so invalidate it when the environment or thread changes and on unmount.

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.

Fixed in cfb4dd5. I took a slightly different route than the suggested cleanup: each check now records the environment and thread it started in, and runs its action only if the feed is still on that thread and mounted. A cleanup that only listed the IDs as dependencies added an "extra effect dependencies" lint warning.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

// A check that settles after the feed left its thread must not navigate.
const fileBindThread = useRef<string | null>(null);
useEffect(() => {
fileBindThread.current = `${props.environmentId}/${props.threadId}`;
return () => {
fileBindThread.current = null;
};
}, [props.environmentId, props.threadId]);
// The joined path stands unless it is missing and the named file exists.
// Without a workspace root nothing is joined, so the named file is the only
// reading, as on web.
const checkThreadFileHref = async (href: string, bound: Map<string, string>) => {
const candidate = resolveMarkdownFileLinkThreadTarget(
href,
props.workspaceRoot,
threadFilePathIndex,
);
if (candidate === null) return href;
const joined = resolveMarkdownFileLinkTarget(
href,
props.workspaceRoot ?? undefined,
props.workspaceRoot ?? undefined,
);
if (joined === null) {
const candidateHref = fileLinkHref(candidate);
bound.set(href, candidateHref);
return candidateHref;
}
const exists = async (path: string) => {
const metadata = await readFileMetadata({
environmentId: props.environmentId,
input: { path: splitFilePathPosition(path).path, threadId: props.threadId },
});
return metadata._tag === "Success" ? metadata.value !== null : null;
};
const [joinedExists, candidateExists] = await Promise.all([exists(joined), exists(candidate)]);
if (joinedExists === null || candidateExists === null) return href;
const result = !joinedExists && candidateExists ? fileLinkHref(candidate) : href;
bound.set(href, result);
return result;
};
// iOS asks for every link's menu while it builds them, so concurrent asks
// share one check. A failed check is dropped, so the next ask retries it.
const bindThreadFileHref = (href: string) => {
const { bound, pending } = fileBindings();
const known = bound.get(href);
if (known !== undefined) return Promise.resolve(known);
const inFlight = pending.get(href);
if (inFlight !== undefined) return inFlight;
const check = checkThreadFileHref(href, bound).finally(() => pending.delete(href));
pending.set(href, check);
return check;
};
// A slow check must not reopen a file after a later press opened another.
const withBoundFileHref = (href: string, use: (bound: string) => void) => {
const request = ++fileBindRequest.current;
const thread = `${props.environmentId}/${props.threadId}`;
void bindThreadFileHref(href).then((bound) => {
if (request === fileBindRequest.current && thread === fileBindThread.current) use(bound);
});
};
const onMarkdownLinkPress = (href: string, imageEmbed = false) => {
const linkedThreadId = parseThreadLinkHref(href);
if (linkedThreadId) {
Expand Down Expand Up @@ -1852,15 +1980,24 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) {
workspaceRoot: props.workspaceRoot ?? null,
});
const markdownLinkHandlers: MarkdownLinkHandlers = {
onLinkPress: onMarkdownLinkPress,
onImagePress: (href) => onMarkdownLinkPress(href, true),
onLinkPress: (href) => withBoundFileHref(href, (bound) => onMarkdownLinkPress(bound)),
onImagePress: (href) => withBoundFileHref(href, (bound) => onMarkdownLinkPress(bound, true)),
resolveImageSource,
// The menu opens synchronously, so it uses a verified binding when one is
// known and starts the check for its actions otherwise.
fileContextMenu: (href) => {
const target = resolveFileChipTarget(href, props.workspaceRoot);
const { bound, menu } = fileBindings();
const known = bound.get(href);
if (known === undefined) void bindThreadFileHref(href);
const shown = known ?? href;
menu.set(href, shown);
const target = resolveFileChipTarget(shown, props.workspaceRoot);
return target ? fileChipMenu(target) : undefined;
},
onFileContextMenuAction: (href, actionId) => {
const target = resolveFileChipTarget(href, props.workspaceRoot);
fileBindRequest.current += 1;
const shown = fileBindings().menu.get(href) ?? href;
const target = resolveFileChipTarget(shown, props.workspaceRoot);
if (!target) return;
switch (actionId as FileChipAction) {
case "copy-full-path":
Expand All @@ -1870,7 +2007,7 @@ export const ThreadFeed = memo(function ThreadFeed(props: ThreadFeedProps) {
if (target.relativePath) copyTextWithHaptic(target.relativePath);
return;
case "open-file":
onMarkdownLinkPress(href);
onMarkdownLinkPress(shown);
return;
case "save":
shareFileChip(target);
Expand Down
54 changes: 48 additions & 6 deletions apps/web/src/components/ChatMarkdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ import { MediaVideoPlayer } from "./media/MediaVideoPlayer";
import { MediaActions, type MediaActionSource } from "./media/MediaActions";
import { resolveProtocolRelativeMediaUrl } from "./media/mediaContent";
import { WorkspaceEntryTooltip } from "./chat/WorkspaceEntryIcon";
import { useFileMetadataQuery } from "../hooks/useFileMetadata";
import { FileTagChipContent } from "./chat/FileTagChip";
import { PierreEntryIcon } from "./chat/PierreEntryIcon";
import {
Expand Down Expand Up @@ -219,6 +220,7 @@ import {
BrowserSettingsReadError,
} from "../browser/openFileInPreview";
import { resolveLinkTarget } from "../browser/browserLinkTarget";
import { ThreadFilePathsContext } from "../threadFilePathsContext";
import { PullRequestLinkPreview } from "./pullRequest/PullRequestLinkPreview";

interface ChatMarkdownProps {
Expand Down Expand Up @@ -1935,6 +1937,23 @@ function MarkdownExternalLinkContent({
);
}

/**
* The joined path keeps the chip while it exists or its metadata is loading, so
* a link that works today never moves to the thread's candidate.
*/
function ThreadFileLinkChip(props: {
environmentId: EnvironmentId | null;
meta: MarkdownFileLinkMeta;
threadCandidate: MarkdownFileLinkMeta;
render: (meta: MarkdownFileLinkMeta) => ReactNode;
}) {
const joined = useFileMetadataQuery(props.environmentId, props.meta.filePath);
const candidate = useFileMetadataQuery(props.environmentId, props.threadCandidate.filePath);
const useCandidate =
joined.isSuccess && joined.data === null && candidate.isSuccess && candidate.data !== null;
return props.render(useCandidate ? props.threadCandidate : props.meta);
}

const MarkdownFileLink = memo(function MarkdownFileLink({
environmentId,
href,
Expand Down Expand Up @@ -2473,6 +2492,7 @@ function useChatMarkdownState({
});
};
const diffThemeName = resolveDiffThemeName(resolvedTheme);
const threadFilePaths = use(ThreadFilePathsContext);
const markdownFileLinkMetaByHref = (() => {
const metaByHref = new Map<
string,
Expand All @@ -2482,7 +2502,12 @@ function useChatMarkdownState({
if (parseComposerContextHref(href)) continue;
const normalizedHref = normalizeMarkdownLinkHrefKey(href);
if (metaByHref.has(normalizedHref)) continue;
const meta = resolveMarkdownFileLinkMeta(normalizedHref, cwd, imageBaseDir ?? cwd);
const meta = resolveMarkdownFileLinkMeta(
normalizedHref,
cwd,
imageBaseDir ?? cwd,
threadFilePaths,
);
if (meta) {
metaByHref.set(normalizedHref, meta);
}
Expand All @@ -2493,7 +2518,7 @@ function useChatMarkdownState({
const metaByText = new Map<string, MarkdownFileLinkMeta>();
for (const span of extractInlineCodeSpans(text)) {
if (metaByText.has(span)) continue;
const meta = resolveInlineCodeFileLinkMeta(span, cwd, imageBaseDir ?? cwd);
const meta = resolveInlineCodeFileLinkMeta(span, cwd, imageBaseDir ?? cwd, threadFilePaths);
if (meta) {
metaByText.set(span, meta);
}
Expand Down Expand Up @@ -2655,7 +2680,7 @@ function useChatMarkdownState({
const filePath = match && cwd ? resolvePathLinkTarget(match, cwd) : fileLinkMeta.filePath;
return revealFileInFileManager(filePath);
};
const fileLinkChip = (
const renderFileLinkChip = (
fileLinkMeta: MarkdownFileLinkMeta,
copyMarkdown: string,
mediaSource?: string,
Expand Down Expand Up @@ -2714,6 +2739,21 @@ function useChatMarkdownState({
/>
);
};
const fileLinkChip = (
fileLinkMeta: MarkdownFileLinkMeta,
copyMarkdown: string,
mediaSource?: string,
) =>
fileLinkMeta.threadCandidate ? (
<ThreadFileLinkChip
environmentId={environmentId}
meta={fileLinkMeta}
threadCandidate={fileLinkMeta.threadCandidate}
render={(meta) => renderFileLinkChip(meta, copyMarkdown, mediaSource)}
/>
) : (
renderFileLinkChip(fileLinkMeta, copyMarkdown, mediaSource)
);

const componentState = {
canOperateHost,
Expand Down Expand Up @@ -2745,6 +2785,7 @@ function useChatMarkdownState({
resolvedTheme,
pullRequestsEnabled,
skills,
threadFilePaths,
threadRef,
updateThreadPullRequestLink,
};
Expand Down Expand Up @@ -2883,6 +2924,7 @@ const CHAT_MARKDOWN_COMPONENTS = {
environmentId,
imageBaseDir,
markdownFileLinkMetaByHref,
threadFilePaths,
threadRef,
openMarkdownMedia,
openChangeRequestLink,
Expand Down Expand Up @@ -2921,7 +2963,7 @@ const CHAT_MARKDOWN_COMPONENTS = {
const normalizedHref = href ? normalizeMarkdownLinkHrefKey(href) : "";
const fileLinkMeta = normalizedHref
? (markdownFileLinkMetaByHref.get(normalizedHref) ??
resolveMarkdownFileLinkMeta(normalizedHref, cwd, imageBaseDir ?? cwd))
resolveMarkdownFileLinkMeta(normalizedHref, cwd, imageBaseDir ?? cwd, threadFilePaths))
: null;
if (!fileLinkMeta) {
const faviconHost = resolveExternalWebLinkHost(href);
Expand Down Expand Up @@ -3146,14 +3188,14 @@ const CHAT_MARKDOWN_COMPONENTS = {
);
},
code: function MarkdownCode({ node, children, className, ...props }) {
const { cwd, imageBaseDir, inlineCodeFileLinkMetaByText, fileLinkChip } = use(
const { cwd, imageBaseDir, inlineCodeFileLinkMetaByText, fileLinkChip, threadFilePaths } = use(
ChatMarkdownRendererContext,
);
if (node?.properties?.dataInlineCode != null) {
const codeText = nodeToPlainText(children);
const fileLinkMeta =
inlineCodeFileLinkMetaByText.get(codeText.trim()) ??
resolveInlineCodeFileLinkMeta(codeText, cwd, imageBaseDir ?? cwd);
resolveInlineCodeFileLinkMeta(codeText, cwd, imageBaseDir ?? cwd, threadFilePaths);
if (fileLinkMeta) {
return fileLinkChip(
fileLinkMeta,
Expand Down
10 changes: 10 additions & 0 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
TYPE_TO_FOCUS_FLOATING_LAYER_SELECTOR,
} from "./chat/composerFocusRedirect";
import { getTerminalLabel } from "@t3tools/shared/terminalLabels";
import { buildThreadFilePathIndex, threadFilePaths } from "@t3tools/shared/threadFilePaths";
import * as Schema from "effect/Schema";
import {
AuthOrchestrationOperateScope,
Expand Down Expand Up @@ -2912,6 +2913,14 @@ export default function ChatView(props: ChatViewProps) {
})
: null;
const gitStatusCwd = activeThread?.worktreePath ?? gitCwd;
// Keyed by content so file links re-resolve only when the thread names a new path.
const threadFilePathsKey = threadFilePaths(
serverVisibleTurnItems.map((row) => row.item),
gitCwd,
).join("\n");
const threadFilePathIndex = buildThreadFilePathIndex(
threadFilePathsKey === "" ? [] : threadFilePathsKey.split("\n"),
);
const gitStatusQuery = useEnvironmentQuery(
gitStatusCwd === null
? null
Expand Down Expand Up @@ -7948,6 +7957,7 @@ export default function ChatView(props: ChatViewProps) {
</div>
<div className="relative flex min-h-0 flex-1 flex-col bg-background">
<MessagesTimeline
threadFilePaths={threadFilePathIndex}
citationRequest={paintOnlyDisplayedTimeline ? null : citationRequest}
citationHistoryLoading={threadDetailLoading}
{...(!paintOnlyDisplayedTimeline
Expand Down
Loading
Loading