Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
57 changes: 57 additions & 0 deletions src/components/viewer-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
Copy,
Download,
Eye,
Link2,
FileCode2,
FileDiff,
FileJson2,
Expand Down Expand Up @@ -41,6 +42,7 @@ import {
type PayloadEnvelope,
} from "@/lib/payload/schema";
import { copyTextToClipboard } from "@/lib/copy-text";
import { formatMarkdownLink } from "@/lib/markdown-link";
import { cn } from "@/lib/utils";
import { LinkCreator } from "@/components/home/link-creator";
import { ArtifactSelector } from "@/components/viewer/artifact-selector";
Expand Down Expand Up @@ -260,10 +262,12 @@ export function ViewerShell() {
const [hash, setHash] = useState("");
const [rendererReady, setRendererReady] = useState(true);
const [artifactCopyState, setArtifactCopyState] = useState<"idle" | "copied" | "failed">("idle");
const [markdownLinkCopyState, setMarkdownLinkCopyState] = useState<"idle" | "copied" | "failed">("idle");
const [viewMode, setViewMode] = useState<"rendered" | "raw">("rendered");
const activeArtifactRef = useRef<ArtifactPayload | null>(null);
/** Incremented on each copy click so stale async completions cannot overwrite state from a newer request. */
const artifactCopyTokenRef = useRef(0);
const markdownLinkCopyTokenRef = useRef(0);
/** True when the current hash originated from a server-injected payload (self-hosted UUID mode). */
const injectedPayloadRef = useRef(false);

Expand Down Expand Up @@ -354,6 +358,7 @@ export function ViewerShell() {

useEffect(() => {
setArtifactCopyState("idle");
setMarkdownLinkCopyState("idle");
setViewMode("rendered");
}, [activeArtifact?.id]);

Expand All @@ -371,6 +376,20 @@ export function ViewerShell() {
};
}, [artifactCopyState]);

useEffect(() => {
if (markdownLinkCopyState !== "copied" && markdownLinkCopyState !== "failed") {
return;
}

const timer = window.setTimeout(() => {
setMarkdownLinkCopyState("idle");
}, 2000);

return () => {
window.clearTimeout(timer);
};
}, [markdownLinkCopyState]);

const setFragmentHash = useCallback((nextHash: string) => {
if (window.location.hash === nextHash) {
return;
Expand Down Expand Up @@ -423,6 +442,32 @@ export function ViewerShell() {
}
}, []);

const handleCopyMarkdownLink = useCallback(async () => {
const artifact = activeArtifactRef.current;
if (!artifact) {
return;
}

const requestArtifactId = artifact.id;
const requestToken = ++markdownLinkCopyTokenRef.current;
const label = getArtifactHeading(artifact);
const href = window.location.href;
const markdownLink = formatMarkdownLink(label, href);

try {
await copyTextToClipboard(markdownLink);
if (activeArtifactRef.current?.id !== requestArtifactId || markdownLinkCopyTokenRef.current !== requestToken) {
return;
}
setMarkdownLinkCopyState("copied");
} catch {
if (activeArtifactRef.current?.id !== requestArtifactId || markdownLinkCopyTokenRef.current !== requestToken) {
return;
}
setMarkdownLinkCopyState("failed");
}
}, []);

const handleArtifactDownload = useCallback(() => {
if (!activeArtifact) {
return;
Expand Down Expand Up @@ -526,6 +571,18 @@ export function ViewerShell() {
{artifactCopyState === "copied" ? <Check className="h-3.5 w-3.5" /> : <Copy className="h-3.5 w-3.5" />}
{artifactCopyState === "copied" ? "Copied" : artifactCopyState === "failed" ? "Copy failed" : "Copy"}
</button>
<button
type="button"
className={cn("artifact-action", markdownLinkCopyState === "copied" && "is-primary")}
onClick={handleCopyMarkdownLink}
>
{markdownLinkCopyState === "copied" ? <Check className="h-3.5 w-3.5" /> : <Link2 className="h-3.5 w-3.5" />}
{markdownLinkCopyState === "copied"
? "Copied"
: markdownLinkCopyState === "failed"
? "Copy failed"
: "Markdown link"}
</button>
{markdownArtifact && viewMode === "rendered" ? (
<button type="button" className="artifact-action" onClick={handleMarkdownPrint}>
<Printer className="h-3.5 w-3.5" />
Expand Down
14 changes: 14 additions & 0 deletions src/lib/markdown-link.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Escape characters that would break a markdown inline link label.
*/
function escapeMarkdownLinkLabel(label: string): string {
return label.replace(/\\/g, "\\\\").replace(/\[/g, "\\[").replace(/\]/g, "\\]");
}

/**
* Format a markdown inline link from a label and destination URL.
*/
export function formatMarkdownLink(label: string, href: string): string {
const trimmedLabel = label.trim() || href;
return `[${escapeMarkdownLinkLabel(trimmedLabel)}](${href})`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: href is inserted into the markdown destination without escaping ) or backslashes, so links with those characters can be rendered incorrectly or truncated by markdown parsers.

Escaping the destination before formatting the link would make the helper safe for arbitrary URLs.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

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 Escape markdown link destinations

When users copy a link for a supported arx/arx2 fragment that uses the base76 wire form, the fragment payload can contain literal ) characters; inserting window.location.href directly between ( and ) makes Markdown close the destination at the first such character, so the pasted link is truncated and no longer decodes. Escape or wrap the destination, e.g. use an angle-bracket destination with any required escaping, before copying it.

Useful? React with 👍 / 👎.

}
27 changes: 26 additions & 1 deletion tests/e2e/viewer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,37 @@ test("copy action copies artifact body to clipboard", async ({ page }) => {
});

await page.getByRole("button", { name: "Copy" }).click();
await expect(page.getByRole("button", { name: "Copied" })).toBeVisible();
await expect(page.getByRole("button", { name: "Copied" }).first()).toBeVisible();
await expect
.poll(() => page.evaluate(() => window.localStorage.getItem("copied-artifact-body")))
.toBe('export function ViewerShell() {\n return <main>Fragment-powered artifact viewer shell</main>;\n}');
});

test("markdown link action copies the current URL as a markdown link", async ({ page }) => {
await goToHash(page, getFragmentHash("Viewer bootstrap"));
await waitForViewerState(page, "artifact");

await page.evaluate(() => {
window.localStorage.removeItem("copied-markdown-link");
Object.defineProperty(navigator, "clipboard", {
configurable: true,
value: {
writeText: (value: string) => {
window.localStorage.setItem("copied-markdown-link", value);
return Promise.resolve();
},
},
});
});

await page.getByRole("button", { name: "Markdown link" }).click();
await expect(page.getByRole("button", { name: "Copied" })).toBeVisible();

const copied = await page.evaluate(() => window.localStorage.getItem("copied-markdown-link"));
const href = await page.evaluate(() => window.location.href);
expect(copied).toBe(`[viewer-shell.tsx](${href})`);
});

test("copy action shows failure when clipboard API and execCommand fallback fail", async ({ page }) => {
await goToHash(page, getFragmentHash("Viewer bootstrap"));
await waitForViewerState(page, "artifact");
Expand Down
20 changes: 20 additions & 0 deletions tests/markdown-link.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { describe, expect, it } from "vitest";
import { formatMarkdownLink } from "@/lib/markdown-link";

describe("formatMarkdownLink", () => {
it("formats a standard inline link", () => {
expect(formatMarkdownLink("Viewer bootstrap", "https://example.com/#agent-render=v1.plain.abc")).toBe(
"[Viewer bootstrap](https://example.com/#agent-render=v1.plain.abc)",
);
});

it("escapes brackets in the label", () => {
expect(formatMarkdownLink("Sprint [draft]", "https://example.com/")).toBe(
"[Sprint \\[draft\\]](https://example.com/)",
);
});

it("falls back to the URL when the label is blank", () => {
expect(formatMarkdownLink(" ", "https://example.com/")).toBe("[https://example.com/](https://example.com/)");
});
});
Comment on lines +17 to +30

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 Missing test case for backslash escaping in the label. escapeMarkdownLinkLabel escapes \\\ before handling brackets; without a test for this path a regression (e.g. dropping the first .replace) would go undetected, and artifact filenames from Windows paths can legitimately contain backslashes.

Suggested change
it("falls back to the URL when the label is blank", () => {
expect(formatMarkdownLink(" ", "https://example.com/")).toBe("[https://example.com/](https://example.com/)");
});
});
it("falls back to the URL when the label is blank", () => {
expect(formatMarkdownLink(" ", "https://example.com/")).toBe("[https://example.com/](https://example.com/)");
});
it("escapes backslashes in the label", () => {
expect(formatMarkdownLink("path\\to\\file", "https://example.com/")).toBe(
"[path\\\\to\\\\file](https://example.com/)",
);
});
});

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex

Loading