Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
133 changes: 132 additions & 1 deletion apps/web/src/components/ChatMarkdown.browser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,25 @@ import { filesystemEnvironment } from "../state/filesystem";
import { useRightPanelStore } from "../rightPanelStore";
import { setMarkdownTaskChecked } from "./files/filePreviewMode";

const { workspaceSearch, filesystemMetadata } = vi.hoisted(() => ({
const { workspaceSearch, filesystemMetadata, chipMetadata } = vi.hoisted(() => ({
workspaceSearch: vi.fn(),
filesystemMetadata: vi.fn(),
chipMetadata: new Map<string, AsyncResult.AsyncResult<{ kind: "file" } | null>>(),
}));
vi.mock(import("../state/use-atom-query-runner"), async (original) => ({
...(await original()),
useAtomQueryRunner: ((family: unknown) =>
family === filesystemEnvironment.getMetadata ? filesystemMetadata : workspaceSearch) as never,
}));
// What each chip already fetched about its own path; unset paths have not loaded.
vi.mock(import("../hooks/useFileMetadata"), async (original) => {
const { Atom } = await import("effect/reactivity");
return {
...(await original()),
fileMetadataAtom: ((_environmentId: unknown, path: string) =>
Atom.make(chipMetadata.get(path) ?? AsyncResult.initial())) as never,
};
});

// Atoms derived from the server config run in the real registry over a missing config.
vi.mock(import("../state/server"), async (original) => {
Expand Down Expand Up @@ -78,6 +88,15 @@ function setup() {
renderDiagram.mockReset();
workspaceSearch.mockReset().mockResolvedValue(AsyncResult.success({ entries: [] }));
filesystemMetadata.mockReset().mockResolvedValue(AsyncResult.success({ entries: [null] }));
chipMetadata.clear();
}

function fileChip(container: HTMLElement, label: string) {
const chip = [...container.querySelectorAll<HTMLElement>(".chat-markdown-file-link")].find(
(element) => element.textContent === label,
);
if (!chip) throw new Error(`No file chip labelled ${label}`);
return chip;
}

function renderText(text: string, props: Partial<Parameters<typeof ChatMarkdown>[0]> = {}) {
Expand Down Expand Up @@ -227,6 +246,118 @@ describe("ChatMarkdown workspace file opens", () => {
expect(openFile).toHaveBeenCalledExactlyOnceWith(threadRef, "/elsewhere/b.md", 9);
expect(workspaceSearch).toHaveBeenCalledOnce();
});

it("opens a path relative to a folder the reply names when the workspace root lacks it", async () => {
setup();
chipMetadata.set("/tmp/work/phase2/A-findings.md", AsyncResult.success(null));
filesystemMetadata.mockResolvedValue(
AsyncResult.success({ entries: [null, { kind: "file" }] }),
);
const message = (folders: string) => (
<ChatMarkdown
cwd="/tmp/work"
threadRef={threadRef}
text={`Everything is in ${folders}.\n\n- \`phase2/A-findings.md\`: raw audit`}
/>
);
await using screen = await render(message("`/tmp/work/backend/.notes/audit/`"));
using openFile = vi
.spyOn(useRightPanelStore.getState(), "openFile")
.mockImplementation(() => {});
await act(async () => fileChip(screen.container, "A-findings.md").click());

await vi.waitFor(() =>
expect(openFile).toHaveBeenLastCalledWith(
threadRef,
"backend/.notes/audit/phase2/A-findings.md",
undefined,
),
);
expect(filesystemMetadata).toHaveBeenCalledExactlyOnceWith({
environmentId: threadRef.environmentId,
input: {
paths: [
"/tmp/work/phase2/A-findings.md",
"/tmp/work/backend/.notes/audit/phase2/A-findings.md",
],
},
});
expect(workspaceSearch).not.toHaveBeenCalled();

// A directory by that name is not the file the chip names.
filesystemMetadata.mockResolvedValue(
AsyncResult.success({ entries: [null, { kind: "directory" }, { kind: "file" }] }),
);
await screen.rerender(
message("`/tmp/work/backend/.notes/audit/` and `/tmp/work/frontend/.notes/audit/`"),
);
await act(async () => fileChip(screen.container, "A-findings.md").click());
await vi.waitFor(() =>
expect(openFile).toHaveBeenLastCalledWith(
threadRef,
"frontend/.notes/audit/phase2/A-findings.md",
undefined,
),
);

// Both named folders hold the file, so guessing could open the wrong one.
filesystemMetadata.mockResolvedValue(
AsyncResult.success({ entries: [null, { kind: "file" }, { kind: "file" }] }),
);
await act(async () => fileChip(screen.container, "A-findings.md").click());
await vi.waitFor(() =>
expect(openFile).toHaveBeenLastCalledWith(threadRef, "phase2/A-findings.md", undefined),
);
});

it("opens the one nested repository file a repository-relative path names", async () => {
setup();
chipMetadata.set("/tmp/work/infra/main.tf", AsyncResult.success({ kind: "file" }));
chipMetadata.set("/tmp/work/tests/sandbox/test_real.py", AsyncResult.success(null));
chipMetadata.set("/tmp/work/docs/guide.md", AsyncResult.success(null));
const files = (...paths: Array<string>) =>
AsyncResult.success({
entries: paths.map((path) => ({ path, kind: "file" })),
truncated: false,
});
workspaceSearch.mockImplementation(async ({ input }: { input: { query: string } }) =>
input.query === "tests/sandbox/test_real.py"
? files("backend/tests/sandbox/test_real.py", "backend/tests/sandbox/test_real_more.py")
: files("backend/docs/guide.md", "frontend/docs/guide.md"),
);
await using screen = await renderText(
"Changed `infra/main.tf` and `notes/todo.md`, skipped `tests/sandbox/test_real.py`, kept `docs/guide.md`.",
{ cwd: "/tmp/work", threadRef },
);
using openFile = vi
.spyOn(useRightPanelStore.getState(), "openFile")
.mockImplementation(() => {});
const { container } = screen;

// A file the root has, or one whose metadata has not loaded, opens without waiting.
await act(async () => fileChip(container, "main.tf").click());
expect(openFile).toHaveBeenLastCalledWith(threadRef, "infra/main.tf", undefined);
await act(async () => fileChip(container, "todo.md").click());
expect(openFile).toHaveBeenLastCalledWith(threadRef, "notes/todo.md", undefined);
expect(filesystemMetadata).not.toHaveBeenCalled();
expect(workspaceSearch).not.toHaveBeenCalled();

await act(async () => fileChip(container, "test_real.py").click());
await vi.waitFor(() =>
expect(openFile).toHaveBeenLastCalledWith(
threadRef,
"backend/tests/sandbox/test_real.py",
undefined,
),
);

// Two repositories hold this path, so guessing could open the wrong file.
await act(async () => fileChip(container, "guide.md").click());
await vi.waitFor(() =>
expect(openFile).toHaveBeenLastCalledWith(threadRef, "docs/guide.md", undefined),
);
expect(openFile).toHaveBeenCalledTimes(4);
});
});

describe("ChatMarkdown headings", () => {
Expand Down
100 changes: 94 additions & 6 deletions apps/web/src/components/ChatMarkdown.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,35 @@
import { buildFileLinkParentSuffixByPath, fileLinkLabel } from "@t3tools/shared/fileLinks";
import {
buildFileLinkParentSuffixByPath,
fileLinkLabel,
resolvePathLinkTarget,
} from "@t3tools/shared/fileLinks";
import {
isWindowsDrivePathHref,
normalizeMarkdownLinkDestination,
extractInlineCodeSpans,
extractMarkdownLinkHrefs,
inlineCodeFilePathCandidate,
} from "@t3tools/shared/markdownLinks";
import { isHostPath, resolveWorkspaceFilePath } from "@t3tools/shared/path";
import {
isHostPath,
isNetworkPath,
normalizeProjectPathForComparison,
resolveWorkspaceFilePath,
workspaceRelativeFilePath,
} from "@t3tools/shared/path";
import { usePullRequestLinking } from "~/hooks/usePullRequestLinking";
import {
AuthFilesystemReadScope,
AuthOrchestrationOperateScope,
FILESYSTEM_METADATA_BATCH_LIMIT,
SOURCE_CONTROL_MEDIA_ASSET,
} from "@t3tools/contracts";
import {
CHAT_MARKDOWN_REMARK_PLUGINS,
CHAT_MARKDOWN_REMARK_PLUGINS_WITH_BREAKS,
CHAT_MARKDOWN_REHYPE_PLUGINS,
} from "@t3tools/shared/markdownPipeline";
import { useAtomValue } from "@effect/atom-react";
import { RegistryContext, useAtomValue } from "@effect/atom-react";
import {
COMPOSER_CONTEXT_CLIPBOARD_MIME,
encodeComposerContextClipboardHtml,
Expand Down Expand Up @@ -165,11 +176,15 @@ import { filesystemEnvironment } from "../state/filesystem";
import {
claimWorkspaceBasenameLookup,
needsWorkspaceBasenameLookup,
nestedWorkspaceLookupPath,
pickNestedWorkspaceMatch,
pickStrippedWorkspaceMatch,
pickWorkspaceBasenameMatch,
stripRepeatedWorkspacePrefix,
WORKSPACE_BASENAME_LOOKUP_LIMIT,
} from "@t3tools/client-runtime/workspace-basename-lookup";
import { FileMetadataThreadContext } from "../hooks/FileMetadataThreadProvider";
import { fileMetadataAtom } from "../hooks/useFileMetadata";
import { resolvePullRequestPreviewTarget } from "@t3tools/client-runtime/change-request-links";
import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients";
import { useOpenChangeRequestLink } from "~/lib/openPullRequestLink";
Expand Down Expand Up @@ -494,6 +509,8 @@ function useChatMarkdownState({
reportFailure: false,
refresh: true,
});
const registry = use(RegistryContext);
const fileMetadataThread = use(FileMetadataThreadContext);
const openPreview = useAtomCommand(previewEnvironment.open, {
reportFailure: false,
});
Expand Down Expand Up @@ -736,6 +753,74 @@ function useChatMarkdownState({
openPreview,
});
};
// Acts only on the chip's settled metadata, so a click never waits on the
// network unless the chip already knows its root path is missing.
const isMissingNestedPath = (fileLinkMeta: MarkdownFileLinkMeta) => {
if (
environmentId === null ||
fileLinkMeta.workspaceLookupPath === null ||
nestedWorkspaceLookupPath(fileLinkMeta.workspaceLookupPath) === null
) {
return false;
}
const atom = fileMetadataAtom(
environmentId,
fileLinkMeta.filePath,
undefined,
fileMetadataThread,
readEnvironmentScope(environmentId, AuthFilesystemReadScope),
);
if (atom === null) return false;
const known = registry.get(atom);
return known._tag === "Success" && !known.waiting && known.value === null;
};
const findNestedWorkspaceFile = async (
fileLinkMeta: MarkdownFileLinkMeta,
workspaceRoot: string,
lookupEnvironmentId: EnvironmentId,
) => {
const nestedPath = nestedWorkspaceLookupPath(fileLinkMeta.workspaceLookupPath ?? "");
if (nestedPath === null || !isMissingNestedPath(fileLinkMeta)) return null;
// Keyed like the metadata cache, so one Windows folder spelled two ways stays one candidate.
const seen = new Set([normalizeProjectPathForComparison(fileLinkMeta.filePath)]);
const candidates = [
...markdownFileLinkMetaByHref.values(),
...inlineCodeFileLinkMetaByText.values(),
]
.filter((meta) => meta.workspaceLookupPath === null)
.map((meta) => resolvePathLinkTarget(nestedPath, meta.filePath))
.filter((path) => {
const key = normalizeProjectPathForComparison(path);
if (path.length > 512 || isNetworkPath(path) || seen.has(key)) return false;
seen.add(key);
return true;
})
.slice(0, FILESYSTEM_METADATA_BATCH_LIMIT - 1);
// The chip's answer can predate the file, so the root path is checked again.
const metadata = await getFilesystemMetadata({
environmentId: lookupEnvironmentId,
input: { paths: [fileLinkMeta.filePath, ...candidates] },
});
if (metadata._tag !== "Success" || metadata.value.entries[0] !== null) return null;
const named = candidates.filter(

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.

🟠 High components/ChatMarkdown.tsx:805

findNestedWorkspaceFile treats any non-null metadata entry as a file match, so a matching directory can be selected instead of the actual file and cause panel reads to fail with WorkspacePathNotFileError. Filter candidates to kind === "file" before counting or selecting matches.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 805:

`findNestedWorkspaceFile` treats any non-null metadata entry as a file match, so a matching directory can be selected instead of the actual file and cause panel reads to fail with `WorkspacePathNotFileError`. Filter candidates to `kind === "file"` before counting or selecting matches.

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 b578e1d. A named-folder candidate now counts only when its metadata kind is "file". That matches the workspace index step, which already searches files only. The browser test covers a reply naming two folders where the first holds a directory with that name and the second holds the file: the chip opens the file.

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.

(_, index) => metadata.value.entries[index + 1]?.kind === "file",
);
if (named.length > 1) return null;
if (named[0] !== undefined)
return workspaceRelativeFilePath(named[0], workspaceRoot) ?? named[0];
const result = await searchProjectEntries({
environmentId: lookupEnvironmentId,
input: {
cwd: workspaceRoot,
query: nestedPath,
limit: WORKSPACE_BASENAME_LOOKUP_LIMIT,
kind: "file",
},
});
return result._tag === "Success"

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.

🟠 High components/ChatMarkdown.tsx:820

When the nested-path search is truncated, findNestedWorkspaceFile passes partial entries to pickNestedWorkspaceMatch, so an early docs/guide.md result is treated as unique and redirects the chip, editor, and copied paths to an arbitrary repository copy. Reject result.value.truncated before selecting a match.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatMarkdown.tsx around line 820:

When the nested-path search is truncated, `findNestedWorkspaceFile` passes partial `entries` to `pickNestedWorkspaceMatch`, so an early `docs/guide.md` result is treated as unique and redirects the chip, editor, and copied paths to an arbitrary repository copy. Reject `result.value.truncated` before selecting a match.

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.

I checked this one and am leaving the code as it is. truncated counts fuzzy matches too, so it is true for most queries. The finder ranks exact whole-suffix matches above fuzzy ones.

I replayed every nested lookup from one real multi-repo project (647 threads, real files on disk). For 357 references, the top 25 results held exactly one exact suffix match, and 206 of those searches were truncated. I then fetched every result for each one. None gained a second exact match, and the single match was always ranked first. Rejecting truncated results would drop about 58% of these fixes to guard against a case that never occurred.

When several repositories hold the path, all the exact matches rank above the fuzzy ones, so the ambiguity shows up inside the top 25 and pickNestedWorkspaceMatch returns null. The docs/guide.md browser case covers that. The existing basename lookup depends on the same ranking and limit. If you want more margin, I can widen this search page (the contract allows up to 200) rather than reject truncated results.

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.

? pickNestedWorkspaceMatch(nestedPath, workspaceRoot, result.value.entries)
: null;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
};
const findWorkspaceFileMatch = async (fileLinkMeta: MarkdownFileLinkMeta) => {
const workspaceRelativePath = fileLinkMeta.workspaceLookupPath;
if (
Expand All @@ -750,7 +835,9 @@ function useChatMarkdownState({
const strippedPath = isBasename
? null
: stripRepeatedWorkspacePrefix(workspaceRelativePath, cwd);
if (!isBasename && strippedPath === null) return null;
if (!isBasename && strippedPath === null) {
return findNestedWorkspaceFile(fileLinkMeta, cwd, environmentId);
}
const result = await searchProjectEntries({
environmentId,
input: {
Expand All @@ -765,7 +852,7 @@ function useChatMarkdownState({
return pickWorkspaceBasenameMatch(workspaceRelativePath, result.value.entries);
}
const match = pickStrippedWorkspaceMatch(strippedPath, cwd, result.value.entries);
if (match === null) return null;
if (match === null) return findNestedWorkspaceFile(fileLinkMeta, cwd, environmentId);
// Search can be truncated or still scanning, so it cannot rule out the
// literal path. Check the host before redirecting to the stripped match.
const metadata = await getFilesystemMetadata({
Expand All @@ -784,7 +871,8 @@ function useChatMarkdownState({
fileLinkMeta.workspaceLookupPath === null ||
!cwd ||
(!needsWorkspaceBasenameLookup(panelPath) &&
stripRepeatedWorkspacePrefix(panelPath, cwd) === null)
stripRepeatedWorkspacePrefix(panelPath, cwd) === null &&
!isMissingNestedPath(fileLinkMeta))
) {
openAt(panelPath);
return;
Expand Down
52 changes: 34 additions & 18 deletions apps/web/src/hooks/useFileMetadata.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { AuthFilesystemReadScope, type EnvironmentId } from "@t3tools/contracts";
import {
AuthFilesystemReadScope,
type EnvironmentId,
type ScopedThreadRef,
} from "@t3tools/contracts";
import { resolvePathLinkTarget, splitFilePathPosition } from "@t3tools/shared/fileLinks";
import {
isAbsolutePath,
Expand All @@ -12,29 +16,41 @@ import { projectEnvironment } from "../state/projects";
import { useEnvironmentQuery } from "../state/query";
import { FileMetadataThreadContext } from "./FileMetadataThreadProvider";

export function useFileMetadata(
export function fileMetadataAtom(
environmentId: EnvironmentId | null | undefined,
path: string,
cwd?: string,
cwd: string | undefined,
threadRef: ScopedThreadRef | null,
canRead: boolean,
) {
const canRead = useEnvironmentScope(environmentId ?? null, AuthFilesystemReadScope);
const threadRef = useContext(FileMetadataThreadContext);
const hostPath = splitFilePathPosition(path).path;
const targetPath = normalizeProjectPathForComparison(
hostPath.startsWith("~/") || !cwd ? hostPath : resolvePathLinkTarget(hostPath, cwd),
);
const absolute = isAbsolutePath(targetPath) || targetPath.startsWith("~/");
return useEnvironmentQuery(
// Servers stat what they are asked about, and a network path leaks the
// Windows user's NTLM credentials to its host before any chip is clicked.
environmentId && canRead && absolute && !isNetworkPath(targetPath) && targetPath.length <= 512
? projectEnvironment.fileMetadata({
environmentId,
input: {
path: targetPath,
...(threadRef?.environmentId === environmentId ? { threadId: threadRef.threadId } : {}),
},
})
: null,
).data;
// Servers stat what they are asked about, and a network path leaks the
// Windows user's NTLM credentials to its host before any chip is clicked.
return environmentId &&
canRead &&
absolute &&
!isNetworkPath(targetPath) &&
targetPath.length <= 512
? projectEnvironment.fileMetadata({
environmentId,
input: {
path: targetPath,
...(threadRef?.environmentId === environmentId ? { threadId: threadRef.threadId } : {}),
},
})
: null;
}

export function useFileMetadata(
environmentId: EnvironmentId | null | undefined,
path: string,
cwd?: string,
) {
const canRead = useEnvironmentScope(environmentId ?? null, AuthFilesystemReadScope);
const threadRef = useContext(FileMetadataThreadContext);
return useEnvironmentQuery(fileMetadataAtom(environmentId, path, cwd, threadRef, canRead)).data;
}
Loading
Loading