mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-10 20:50:08 +02:00
Fixes #7514 — the prefix-validation piece of the #5456 auto-linker 404-storm umbrella. The remaining pieces (404 retry guard, word-boundary tightening, comment edit/soft-delete) stay open under #5456. Part of #5456. ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Agents and humans cross-reference work in markdown — issue descriptions, comments, documents — which the UI renders through a shared `MarkdownBody` > - That renderer auto-links any `IDENT-123`-shaped token to an internal `/issues/IDENT-123` link > - But foreign tracker keys share that exact shape: a Jira `TREE-604` (or any external `ORG-123`) mentioned in prose becomes a link to a Paperclip issue that does not exist — it 404s, and the renderer also fires a wasted issue-fetch for the bogus identifier > - The set of real issue prefixes is already in the browser: every company carries an `issuePrefix`, exposed via `CompanyContext` > - This PR gates bare-token auto-linking to known company prefixes, leaving explicit `issue://` / `/issues/` references and real markdown links untouched > - The benefit is no dead internal links from foreign keys, with no new query, cache, or server change — and zero regression when prefixes aren't yet known ## What Changed - **`ui/src/lib/issue-reference.ts`** — `parseIssueReferenceFromHref` takes an optional `knownPrefixes` set and rejects a bare `IDENT-123` token whose prefix isn't in it; threaded through `remarkLinkIssueReferences(options)` → tree rewrite → text and inline-code paths. An omitted/empty set keeps the legacy permissive behavior. Explicit `issue://` scheme and `/issues/` path forms are never gated. - **`ui/src/context/CompanyContext.tsx`** — adds `useOptionalCompany()`, a non-throwing variant of `useCompany()` (returns `null` outside a provider). - **`ui/src/components/MarkdownBody.tsx`** — reads company prefixes via `useOptionalCompany()` and passes them to the linkifier. The non-throwing read keeps `MarkdownBody` renderable in provider-less surfaces (e.g. standalone/exported markdown). - Tests extended in `issue-reference.test.ts` (gating + remark-plugin cases) and `MarkdownBody.test.tsx` (gating, empty-companies permissive, explicit-path bypass). ## Verification - `pnpm --filter @paperclipai/ui exec vitest run src/lib/issue-reference.test.ts src/components/MarkdownBody.test.tsx` — green (17 + 40 tests). - Full UI suite: `pnpm --filter @paperclipai/ui exec vitest run` — **1161 passed / 183 files**; pre-existing `MarkdownBody` link tests pass unmodified (they hit the permissive `null`-context path), confirming no regression. - `pnpm --filter @paperclipai/ui run typecheck` — clean. - _Screenshots pending — opening as draft; before/after images to follow before marking ready._ - Manual (before/after): in an issue description containing both a real Paperclip identifier and a Jira key — _before_ both render as `/issues/...` links (the Jira one dead); _after_ only the real identifier links and the Jira key is plain text. ## Risks - **Low risk.** No server/API/migration change; pure client rendering logic. - A referenced issue whose company isn't in the viewer's `companies` list stops auto-linking — acceptable, since that internal link wouldn't resolve for that viewer anyway; explicit `/issues/IDENT` references still render. - During initial load (companies not yet fetched) behavior is identical to today (permissive), so no new flicker. ## Model Used - **Anthropic Claude Opus 4.8** (`claude-opus-4-8`), 1M-token context, extended thinking + tool use. Plan authored and implemented with the model; all decisions reviewed by the human contributor. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes (no doc changes required — behavior gated, no public API/doc surface affected) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
150 lines
5.9 KiB
TypeScript
150 lines
5.9 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import { parseIssuePathIdFromPath, parseIssueReferenceFromHref, remarkLinkIssueReferences } from "./issue-reference";
|
|
|
|
type TreeNode = { type: string; value?: string; url?: string; children?: TreeNode[] };
|
|
|
|
function paragraph(value: string): TreeNode {
|
|
return { type: "root", children: [{ type: "paragraph", children: [{ type: "text", value }] }] };
|
|
}
|
|
|
|
function paragraphChildren(tree: TreeNode): TreeNode[] {
|
|
return tree.children?.[0]?.children ?? [];
|
|
}
|
|
|
|
describe("issue-reference", () => {
|
|
it("extracts issue ids from company-scoped issue paths", () => {
|
|
expect(parseIssuePathIdFromPath("/PAP/issues/PAP-1271")).toBe("PAP-1271");
|
|
expect(parseIssuePathIdFromPath("/PAP/issues/pap-1272")).toBe("PAP-1272");
|
|
expect(parseIssuePathIdFromPath("/issues/pc1a2-7")).toBe("PC1A2-7");
|
|
expect(parseIssuePathIdFromPath("/PC1A2/issues/pc1a2-7")).toBe("PC1A2-7");
|
|
expect(parseIssuePathIdFromPath("/issues/PAP-1179")).toBe("PAP-1179");
|
|
expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull();
|
|
});
|
|
|
|
it("does not treat full issue URLs as internal issue paths", () => {
|
|
expect(parseIssuePathIdFromPath("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull();
|
|
expect(parseIssuePathIdFromPath("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull();
|
|
});
|
|
|
|
it("does not treat GitHub issue URLs as internal Paperclip issue links", () => {
|
|
expect(parseIssuePathIdFromPath("https://github.com/paperclipai/paperclip/issues/1778")).toBeNull();
|
|
expect(parseIssueReferenceFromHref("https://github.com/paperclipai/paperclip/issues/1778")).toBeNull();
|
|
});
|
|
|
|
it("ignores placeholder issue paths", () => {
|
|
expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull();
|
|
expect(parseIssuePathIdFromPath("http://localhost:3100/issues/:id")).toBeNull();
|
|
expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull();
|
|
});
|
|
|
|
it("normalizes bare identifiers, relative issue paths, and issue scheme links into internal links", () => {
|
|
expect(parseIssueReferenceFromHref("pap-1271")).toEqual({
|
|
issuePathId: "PAP-1271",
|
|
href: "/issues/PAP-1271",
|
|
});
|
|
expect(parseIssueReferenceFromHref("pc1a2-7")).toEqual({
|
|
issuePathId: "PC1A2-7",
|
|
href: "/issues/PC1A2-7",
|
|
});
|
|
expect(parseIssueReferenceFromHref("/PAP/issues/pap-1180")).toEqual({
|
|
issuePathId: "PAP-1180",
|
|
href: "/issues/PAP-1180",
|
|
});
|
|
expect(parseIssueReferenceFromHref("issue://PAP-1310")).toEqual({
|
|
issuePathId: "PAP-1310",
|
|
href: "/issues/PAP-1310",
|
|
});
|
|
expect(parseIssueReferenceFromHref("issue://:PAP-1311")).toEqual({
|
|
issuePathId: "PAP-1311",
|
|
href: "/issues/PAP-1311",
|
|
});
|
|
});
|
|
|
|
it("normalizes exact inline-code-like issue identifiers", () => {
|
|
expect(parseIssueReferenceFromHref("PAP-1271")).toEqual({
|
|
issuePathId: "PAP-1271",
|
|
href: "/issues/PAP-1271",
|
|
});
|
|
});
|
|
|
|
it("preserves absolute Paperclip issue URLs so origin, port, and hash are not lost", () => {
|
|
expect(parseIssueReferenceFromHref("http://localhost:3100/PAP/issues/PAP-1179")).toBeNull();
|
|
expect(parseIssueReferenceFromHref("http://remote.example.test:3103/PAPA/issues/PAPA-115#comment-850083f3-24de-43e7-a8cd-bc01f7cc9f0d")).toBeNull();
|
|
});
|
|
|
|
it("ignores literal route placeholder paths", () => {
|
|
expect(parseIssueReferenceFromHref("/issues/:id")).toBeNull();
|
|
expect(parseIssueReferenceFromHref("http://localhost:3100/api/issues/:id")).toBeNull();
|
|
});
|
|
|
|
describe("known-prefix gating", () => {
|
|
it("links a bare identifier whose prefix is known", () => {
|
|
expect(parseIssueReferenceFromHref("PAP-1271", new Set(["PAP"]))).toEqual({
|
|
issuePathId: "PAP-1271",
|
|
href: "/issues/PAP-1271",
|
|
});
|
|
});
|
|
|
|
it("matches the prefix case-insensitively", () => {
|
|
expect(parseIssueReferenceFromHref("pap-12", new Set(["PAP"]))).toEqual({
|
|
issuePathId: "PAP-12",
|
|
href: "/issues/PAP-12",
|
|
});
|
|
});
|
|
|
|
it("does not link a bare identifier whose prefix is unknown (e.g. a Jira key)", () => {
|
|
expect(parseIssueReferenceFromHref("JIRA-456", new Set(["PAP"]))).toBeNull();
|
|
});
|
|
|
|
it("stays permissive when no prefix set is supplied", () => {
|
|
expect(parseIssueReferenceFromHref("FOO-1")).toEqual({
|
|
issuePathId: "FOO-1",
|
|
href: "/issues/FOO-1",
|
|
});
|
|
});
|
|
|
|
it("stays permissive when the prefix set is empty", () => {
|
|
expect(parseIssueReferenceFromHref("FOO-1", new Set())).toEqual({
|
|
issuePathId: "FOO-1",
|
|
href: "/issues/FOO-1",
|
|
});
|
|
});
|
|
|
|
it("never gates explicit issue:// scheme references", () => {
|
|
expect(parseIssueReferenceFromHref("issue://ACME-9", new Set(["PAP"]))).toEqual({
|
|
issuePathId: "ACME-9",
|
|
href: "/issues/ACME-9",
|
|
});
|
|
});
|
|
|
|
it("never gates explicit /issues/ path references", () => {
|
|
expect(parseIssueReferenceFromHref("/ACME/issues/ACME-9", new Set(["PAP"]))).toEqual({
|
|
issuePathId: "ACME-9",
|
|
href: "/issues/ACME-9",
|
|
});
|
|
});
|
|
});
|
|
|
|
describe("remarkLinkIssueReferences", () => {
|
|
it("links only known-prefix tokens and leaves foreign keys as text", () => {
|
|
const tree = paragraph("See PAP-1 and JIRA-2 today.");
|
|
remarkLinkIssueReferences({ knownPrefixes: ["PAP"] })(tree);
|
|
|
|
const children = paragraphChildren(tree);
|
|
expect(children).toEqual([
|
|
{ type: "text", value: "See " },
|
|
{ type: "link", url: "/issues/PAP-1", children: [{ type: "text", value: "PAP-1" }] },
|
|
{ type: "text", value: " and JIRA-2 today." },
|
|
]);
|
|
});
|
|
|
|
it("links every identifier when no prefixes are supplied (legacy permissive)", () => {
|
|
const tree = paragraph("See PAP-1 and JIRA-2.");
|
|
remarkLinkIssueReferences()(tree);
|
|
|
|
const links = paragraphChildren(tree).filter((node) => node.type === "link");
|
|
expect(links.map((node) => node.url)).toEqual(["/issues/PAP-1", "/issues/JIRA-2"]);
|
|
});
|
|
});
|
|
});
|