diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index dc8d738be44..50f62d5faf7 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -551,6 +551,28 @@ no remote metadata requests. Run its light/dark desktop/mobile checks with: STORYBOOK_TEST_BASE_URL=http://localhost:6006 pnpm exec playwright test inline-links.spec.ts --config=tests/storybook-visual/playwright.config.ts --project=stories ``` +**Link previews** mounts the same Markdown renderer with its production preview +provider. A delayed hover or keyboard focus requests PR metadata. Repeated PR URLs, +including diff links and fragments, share a viewer/company-scoped query. The card +stays aligned with the link, and the exact navigation URL stays intact. The stories +simulate the API response for open, merged, conflicts, draft, closed, loading, +unavailable and failed reads, with a narrow long-title example. Run: + +```sh +STORYBOOK_TEST_BASE_URL=http://localhost:6006 pnpm exec playwright test link-previews.spec.ts --config=tests/storybook-visual/playwright.config.ts --project=stories +``` + +The board-only `GET /api/companies/:companyId/link-preview?url=...` is optional +enrichment, independent of the External Objects experiment. Its GitHub adapter +uses the viewer's eligible managed connection, then the existing company/public +credential path only when no managed connection is configured. Personal results +remain in bounded, process-local viewer/company caches and are never persisted as +company external objects. Successful responses live for one minute; unavailable +responses use a short negative cache or GitHub's retry window. Reads have an +abortable deadline and a bounded concurrency budget. Nothing polls while idle. +The browser neither retries these reads nor refreshes them on window focus. +These stories do not verify live GitHub authentication. + **Task chat → Failure notices** covers compact run errors and adjacent recovery notices in both presentation modes, with expanded details, fixture retry callbacks (including the legacy run retry when recovery retry is unavailable), diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index a4ec3e9cca1..aed8a87d7be 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -2915,3 +2915,4 @@ export { TASK_IDENTITY_CHANGED_FIELD, issueActivityChangesTaskIdentity, } from "./issue-activity-identity.js"; +export { recognizeLinkPreview, type LinkPreviewTarget, type LinkPreviewState, type LinkPreview, type LinkPreviewResult } from "./link-preview.js"; diff --git a/packages/shared/src/link-preview.test.ts b/packages/shared/src/link-preview.test.ts new file mode 100644 index 00000000000..136041c9283 --- /dev/null +++ b/packages/shared/src/link-preview.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; +import { recognizeLinkPreview } from "./link-preview.js"; + +describe("link preview recognition", () => { + it("shares PR identity across casing, diff paths, queries and fragments", () => { + expect(recognizeLinkPreview("https://WWW.GitHub.com/Acme/Web/pull/42/files?diff=split#review")?.canonicalUrl) + .toBe("https://github.com/acme/web/pull/42"); + expect(recognizeLinkPreview("https://github.com/Acme/Web/pull/42")?.label).toBe("Acme/Web#42"); + }); + it.each([ + "http://github.com/acme/web/pull/42", "https://github.com.example.com/acme/web/pull/42", + "https://user:pass@github.com/acme/web/pull/42", "https://github.com:444/acme/web/pull/42", + "https://github.com/acme/web/issues/42", "https://github.com/acme/web/pull/42oops", + "https://github.com/acme/web/pull/0", "https://github.com/acme/web/pull/9007199254740992", + "https://example.com", "/acme/web/pull/42", "not a URL", + ])("does not resolve an unsupported or unsafe URL: %s", url => { + expect(recognizeLinkPreview(url)).toBeNull(); + }); +}); diff --git a/packages/shared/src/link-preview.ts b/packages/shared/src/link-preview.ts new file mode 100644 index 00000000000..ffa5eb960a7 --- /dev/null +++ b/packages/shared/src/link-preview.ts @@ -0,0 +1,41 @@ +/** Host-owned link metadata. Providers do not supply markup or fetch URLs. */ +export interface LinkPreviewTarget { + provider: string; + kind: string; + canonicalUrl: string; + label: string; +} + +export type LinkPreviewState = "unknown" | "open" | "draft" | "merged" | "closed" | "conflicts"; + +export interface LinkPreview { + target: LinkPreviewTarget; + title: string; + state: LinkPreviewState; + author: string | null; + updatedAt: string | null; +} + +export interface LinkPreviewResult { + preview: LinkPreview | null; + /** Demand-driven cache lifetime; this does not schedule a refresh. */ + maxAgeMs: number; +} + +/** Recognition only. Never contacts a remote service or changes navigation. */ +export function recognizeLinkPreview(href: string | null | undefined): LinkPreviewTarget | null { + if (!href || href.length > 2048) return null; + let url: URL; + try { url = new URL(href); } catch { return null; } + if (url.protocol !== "https:" || !["github.com", "www.github.com"].includes(url.hostname) + || url.username || url.password || url.port) return null; + const match = /^\/([a-z\d](?:[a-z\d-]*[a-z\d])?)\/([a-z\d_.-]+)\/pull\/([1-9]\d*)(?:\/|$)/i.exec(url.pathname); + if (!match || match[2] === "." || match[2] === ".." || !Number.isSafeInteger(Number(match[3]))) return null; + const [, owner, repo, number] = match; + return { + provider: "github", + kind: "pull_request", + canonicalUrl: `https://github.com/${owner!.toLowerCase()}/${repo!.toLowerCase()}/pull/${number}`, + label: `${owner}/${repo}#${number}`, + }; +} diff --git a/server/src/__tests__/github-link-preview.test.ts b/server/src/__tests__/github-link-preview.test.ts new file mode 100644 index 00000000000..3a405e85572 --- /dev/null +++ b/server/src/__tests__/github-link-preview.test.ts @@ -0,0 +1,59 @@ +import type { Db } from "@paperclipai/db"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { recognizeLinkPreview } from "@paperclipai/shared"; +import { createGitHubLinkPreviewProvider, resolveLinkPreviewGitHubCredential } from "../services/github-link-preview.js"; +import { resolveGitHubApiCredential, resolveManagedGitHubCredential } from "../services/git-credentials.js"; + +vi.mock("../services/git-credentials.js", async original => ({ + ...await original(), + resolveManagedGitHubCredential: vi.fn(), resolveGitHubApiCredential: vi.fn(), +})); +vi.mock("../services/secrets.js", () => ({ secretService: () => ({}) })); +const db = null as unknown as Db; +const scope = { companyId: "company-a", userId: "alice" }; +const target = recognizeLinkPreview("https://github.com/acme/web/pull/42")!; +beforeEach(() => vi.resetAllMocks()); + +describe("GitHub link preview boundary", () => { + it("uses only the current viewer's managed identity with no standing delegation", async () => { + vi.mocked(resolveManagedGitHubCredential).mockResolvedValue({ configured: true, credential: { token: "token", source: "managed_connection", secretName: null } }); + expect(await resolveLinkPreviewGitHubCredential(db, scope)).toEqual({ token: "token", source: "managed_connection", secretName: null }); + expect(resolveManagedGitHubCredential).toHaveBeenCalledWith(db, {}, scope.companyId, { responsibleUserId: "alice", allowStandingDelegation: false }); + expect(resolveGitHubApiCredential).not.toHaveBeenCalled(); + }); + it("does not bypass a configured connection that cannot supply credentials", async () => { + vi.mocked(resolveManagedGitHubCredential).mockResolvedValue({ configured: true }); + await expect(resolveLinkPreviewGitHubCredential(db, scope)).rejects.toThrow(); + expect(resolveGitHubApiCredential).not.toHaveBeenCalled(); + }); + it("uses the existing company/public fallback only without a managed connection", async () => { + vi.mocked(resolveManagedGitHubCredential).mockResolvedValue({ configured: false }); + vi.mocked(resolveGitHubApiCredential).mockResolvedValue(null); + expect(await resolveLinkPreviewGitHubCredential(db, scope)).toBeNull(); + expect(resolveGitHubApiCredential).toHaveBeenCalledWith(db, scope.companyId); + }); + it.each([ + [{ state: "open" }, "open"], [{ state: "open", mergeable: false }, "conflicts"], + [{ state: "open", mergeable: null }, "open"], [{ state: "open", draft: true }, "draft"], + [{ state: "open", draft: true, mergeable: false }, "draft"], + [{ state: "closed", merged: true }, "merged"], [{ state: "closed" }, "closed"], + ])("normalizes state %j as %s without fetching checks or reviews", async (fields, expected) => { + const fetcher = vi.fn(async () => Response.json({ title: "A change", updated_at: "2026-11-10T12:00:00Z", user: { login: "alice" }, ...fields })); + const provider = createGitHubLinkPreviewProvider(db, { fetch: fetcher, credentialProvider: () => null }); + const result = await provider.resolve(target, scope, new AbortController().signal); + expect(result.preview).toMatchObject({ title: "A change", state: expected, author: "alice", target }); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(fetcher).toHaveBeenCalledWith("https://api.github.com/repos/acme/web/pulls/42", expect.objectContaining({ signal: expect.any(AbortSignal) })); + }); + it.each([401, 403, 429, 500])("returns a quiet absence for GitHub HTTP %s", async status => { + const provider = createGitHubLinkPreviewProvider(db, { fetch: async () => new Response(null, { status }), credentialProvider: () => null }); + expect((await provider.resolve(target, scope, new AbortController().signal)).preview).toBeNull(); + }); + it("retains the instance-token public-repository boundary", async () => { + const fetcher = vi.fn(async () => Response.json({ private: true })); + const provider = createGitHubLinkPreviewProvider(db, { fetch: fetcher, credentialProvider: () => ({ token: "instance", source: "server_env", secretName: null }) }); + expect((await provider.resolve(target, scope, new AbortController().signal)).preview).toBeNull(); + expect(fetcher).toHaveBeenCalledTimes(1); + expect(fetcher).toHaveBeenCalledWith("https://api.github.com/repos/acme/web", expect.any(Object)); + }); +}); diff --git a/server/src/__tests__/link-preview-routes.test.ts b/server/src/__tests__/link-preview-routes.test.ts new file mode 100644 index 00000000000..a5026be77d8 --- /dev/null +++ b/server/src/__tests__/link-preview-routes.test.ts @@ -0,0 +1,48 @@ +import express from "express"; +import request from "supertest"; +import { describe, expect, it, vi } from "vitest"; +import type { Db } from "@paperclipai/db"; +import { linkPreviewRoutes } from "../routes/link-previews.js"; +import { HttpError } from "../errors.js"; + +function app(actor: express.Request["actor"]) { + const get = vi.fn(async () => ({ preview: null, maxAgeMs: 60_000 })); + const server = express(); + server.use((req, _res, next) => { req.actor = actor; next(); }); + server.use(linkPreviewRoutes(null as unknown as Db, { get })); + server.use((error: HttpError, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + res.status(error.status ?? 500).json({ error: error.message }); + }); + return { server, get }; +} +const board = { type: "board", source: "session", userId: "alice", companyIds: ["company-a"] } as const; +const path = "/companies/company-a/link-preview"; + +describe("link preview routes", () => { + it("passes the authenticated viewer and disables shared HTTP caching", async () => { + const { server, get } = app({ ...board, companyIds: [...board.companyIds] }); + const response = await request(server).get(path).query({ url: "https://github.com/acme/web/pull/42" }); + expect(response.status).toBe(200); + expect(response.headers["cache-control"]).toBe("private, no-store"); + expect(get).toHaveBeenCalledWith({ companyId: "company-a", userId: "alice" }, "https://github.com/acme/web/pull/42"); + }); + it("checks company access before consulting any cache or provider", async () => { + const { server, get } = app({ ...board, companyIds: ["company-b"] }); + expect((await request(server).get(path).query({ url: "https://github.com/acme/web/pull/42" })).status).toBe(403); + expect(get).not.toHaveBeenCalled(); + }); + it("rejects agents and unauthenticated requests", async () => { + for (const actor of [{ type: "none" }, { type: "agent", agentId: "agent-a", companyId: "company-a" }] as express.Request["actor"][]) { + const { server, get } = app(actor); + expect([401, 403]).toContain((await request(server).get(path).query({ url: "https://github.com/acme/web/pull/42" })).status); + expect(get).not.toHaveBeenCalled(); + } + }); + it("bounds and validates the URL input", async () => { + const { server, get } = app({ ...board, companyIds: [...board.companyIds] }); + for (const query of [{}, { url: "x".repeat(2049) }, { url: ["a", "b"] }]) { + expect((await request(server).get(path).query(query)).status).toBe(400); + } + expect(get).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/link-previews.test.ts b/server/src/__tests__/link-previews.test.ts new file mode 100644 index 00000000000..5ef63cd6aef --- /dev/null +++ b/server/src/__tests__/link-previews.test.ts @@ -0,0 +1,113 @@ +import { createServer } from "node:http"; +import { once } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createLinkPreviewService, type LinkPreviewProvider } from "../services/link-previews.js"; + +const scope = { companyId: "company-a", userId: "alice" }; +const url = "https://github.com/acme/web/pull/42"; +const unavailable = { preview: null, maxAgeMs: 60_000 }; +const provider = (resolve = vi.fn(async () => unavailable)): LinkPreviewProvider => ({ key: "github", resolve }); +afterEach(() => vi.useRealTimers()); + +describe("demand-driven link preview cache", () => { + it("does no work until requested and coalesces equivalent URLs", async () => { + const resolve = vi.fn(async () => unavailable); + const service = createLinkPreviewService([provider(resolve)]); + expect(resolve).not.toHaveBeenCalled(); + await Promise.all([ + service.get(scope, url), service.get(scope, "https://www.github.com/ACME/Web/pull/42/files?x=1#review"), + service.get(scope, url), + ]); + expect(resolve).toHaveBeenCalledTimes(1); + await service.get(scope, url); + expect(resolve).toHaveBeenCalledTimes(1); + }); + it("separates viewers, companies, and resources", async () => { + const resolve = vi.fn(async () => unavailable); + const service = createLinkPreviewService([provider(resolve)]); + await service.get(scope, url); + await service.get({ ...scope, userId: "bob" }, url); + await service.get({ ...scope, companyId: "company-b" }, url); + await service.get(scope, `${url}1`); + expect(resolve).toHaveBeenCalledTimes(4); + }); + it("caches failures, then refreshes only on a new request after expiry", async () => { + let now = 0; + const resolve = vi.fn(async () => { throw new Error("no access"); }); + const service = createLinkPreviewService([provider(resolve)], { now: () => now }); + expect(await service.get(scope, url)).toEqual(unavailable); + now = 30_000; + expect((await service.get(scope, url)).maxAgeMs).toBe(30_000); + now = 61_000; + expect(resolve).toHaveBeenCalledTimes(1); + await service.get(scope, url); + expect(resolve).toHaveBeenCalledTimes(2); + }); + it("bounds cached resources and does not resolve unsupported hosts", async () => { + const resolve = vi.fn(async () => unavailable); + const service = createLinkPreviewService([provider(resolve)], { maxEntries: 2 }); + await service.get(scope, "https://localhost/private"); + expect(resolve).not.toHaveBeenCalled(); + for (const suffix of ["", "1", "2", ""]) await service.get(scope, url + suffix); + expect(resolve).toHaveBeenCalledTimes(4); + }); + it("holds timed-out underlying work inside the concurrency limit", async () => { + vi.useFakeTimers(); + let finish!: (value: typeof unavailable) => void; + const resolve = vi.fn(() => new Promise(done => { finish = done; })); + const service = createLinkPreviewService([provider(resolve)], { timeoutMs: 20, maxConcurrent: 1 }); + const first = service.get(scope, url); + await vi.advanceTimersByTimeAsync(21); + expect(await first).toEqual(unavailable); + await service.get(scope, url + "1"); + expect(resolve).toHaveBeenCalledTimes(1); + finish(unavailable); + await vi.advanceTimersByTimeAsync(1); + const next = service.get(scope, url + "1"); + await vi.advanceTimersByTimeAsync(21); + await next; + expect(resolve).toHaveBeenCalledTimes(2); + finish(unavailable); + }); + it("refreshes an expired timeout while the earlier operation ignores cancellation", async () => { + vi.useFakeTimers(); + let finish!: (value: typeof unavailable) => void; + const resolve = vi.fn(async () => unavailable).mockImplementationOnce( + () => new Promise(done => { finish = done; }), + ); + const service = createLinkPreviewService([provider(resolve)], { timeoutMs: 20, maxConcurrent: 2 }); + const first = service.get(scope, url); + await vi.advanceTimersByTimeAsync(21); + expect(await first).toEqual(unavailable); + await service.get(scope, url); + expect(resolve).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(60_001); + await service.get(scope, url); + expect(resolve).toHaveBeenCalledTimes(2); + finish(unavailable); + }); + it("aborts a real stalled HTTP read while an unrelated lookup remains responsive", async () => { + const server = createServer((_req, res) => { res.writeHead(200, { "content-type": "application/json" }); res.write("{"); }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address() as { port: number }; + let aborted = false; + const service = createLinkPreviewService([{ key: "github", resolve: async (target, _scope, signal) => { + if (target.canonicalUrl.endsWith("/421")) return unavailable; + try { + const response = await fetch(`http://127.0.0.1:${address.port}`, { signal }); + await response.json(); + } catch { aborted = signal.aborted; } + return unavailable; + } }], { timeoutMs: 100 }); + try { + const slow = service.get(scope, url); + expect(await service.get(scope, url + "1")).toEqual(unavailable); + await slow; + await vi.waitFor(() => expect(aborted).toBe(true)); + } finally { + server.closeAllConnections(); + await new Promise(done => server.close(() => done())); + } + }); +}); diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index 7a419c064dd..1678e78207e 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -78,6 +78,7 @@ const apiPrefixes: Record = { "issues.ts": "/api", "issue-tree-control.ts": "/api", "llms.ts": "/api", + "link-previews.ts": "/api", "managed-agent-profiles.ts": "/api", "onboarding-seed.ts": "/api", "openapi.ts": "/api", diff --git a/server/src/app.ts b/server/src/app.ts index e72b0c04b97..bfafc81a905 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -121,6 +121,7 @@ import { environmentService } from "./services/environments.js"; import { environmentRuntimeService } from "./services/environment-runtime.js"; import { projectRoutes } from "./routes/projects.js"; import { issueRoutes } from "./routes/issues.js"; +import { linkPreviewRoutes } from "./routes/link-previews.js"; import { issueTreeControlRoutes } from "./routes/issue-tree-control.js"; import { caseRoutes } from "./routes/cases.js"; import { caseHandoffRoutes } from "./routes/case-handoffs.js"; @@ -920,6 +921,7 @@ export async function createApp( api.use(decisionModelRoutes(db)); api.use(agentObservabilityRoutes(db, { deploymentMode: opts.deploymentMode, deploymentExposure: opts.deploymentExposure })); api.use(fastResponseRoutes(db)); + api.use(linkPreviewRoutes(db)); api.use(promptInjectionPreventionRoutes(db)); api.use(aiConnectionRoutes(db, { deploymentMode: opts.deploymentMode, deploymentExposure: opts.deploymentExposure, trustedLocalStdioRuntimeHost, pluginWorkerManager: workerManager })); api.use( diff --git a/server/src/routes/link-previews.ts b/server/src/routes/link-previews.ts new file mode 100644 index 00000000000..bd631855b2f --- /dev/null +++ b/server/src/routes/link-previews.ts @@ -0,0 +1,21 @@ +import { Router } from "express"; +import type { Db } from "@paperclipai/db"; +import { badRequest } from "../errors.js"; +import { assertBoard, assertCompanyAccess } from "./authz.js"; +import { createLinkPreviewService } from "../services/link-previews.js"; +import { createGitHubLinkPreviewProvider } from "../services/github-link-preview.js"; + +export function linkPreviewRoutes(db: Db, service = createLinkPreviewService([createGitHubLinkPreviewProvider(db)])) { + const router = Router(); + router.get("/companies/:companyId/link-preview", async (req, res) => { + const companyId = req.params.companyId as string; + assertCompanyAccess(req, companyId); + assertBoard(req); + const href = req.query.url; + if (typeof href !== "string" || href.length > 2048) throw badRequest("A link URL is required"); + // Personal repository metadata must never be stored by a proxy or shared browser cache. + res.set("Cache-Control", "private, no-store"); + res.json(await service.get({ companyId, userId: req.actor.userId ?? null }, href)); + }); + return router; +} diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index ca8657c54eb..a0acf2e28fa 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -10151,6 +10151,26 @@ registry.registerPath({ responses: { 200: r.ok(), 401: r.unauthorized, 404: r.notFound }, }); +registry.registerPath({ + method: "get", + path: "/api/companies/{companyId}/link-preview", + tags: ["companies"], + summary: "Read optional link metadata for the current board user", + description: "Company access and board authentication are required. This read uses a viewer-scoped cache and returns a quiet null preview when provider access is unavailable. It does not change tasks or schedule background work.", + request: { params: z.object({ companyId: z.string() }), query: z.object({ url: z.string().max(2048) }) }, + responses: { + 200: r.ok(z.object({ + preview: z.object({ + target: z.object({ provider: z.string(), kind: z.string(), canonicalUrl: z.string(), label: z.string() }), + title: z.string(), state: z.enum(["unknown", "open", "draft", "merged", "closed", "conflicts"]), + author: z.string().nullable(), updatedAt: z.string().nullable(), + }).nullable(), + maxAgeMs: z.number(), + })), + 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden, + }, +}); + registry.registerPath({ method: "get", path: "/api/issues/{id}/external-objects", diff --git a/server/src/services/git-credentials.ts b/server/src/services/git-credentials.ts index 7efb61cb903..fed5e00d6fa 100644 --- a/server/src/services/git-credentials.ts +++ b/server/src/services/git-credentials.ts @@ -200,7 +200,7 @@ type GitCredentialSecretsDeps = { resolveUserSecretValue?: SecretServiceLike["resolveUserSecretValue"]; }; -export type GitHubApiCredentialSource = "company_secret" | "server_env"; +export type GitHubApiCredentialSource = "managed_connection" | "company_secret" | "server_env"; export type GitHubApiCredential = { token: string; diff --git a/server/src/services/github-external-object-provider.ts b/server/src/services/github-external-object-provider.ts index 83b47354035..e2bb5fdf77d 100644 --- a/server/src/services/github-external-object-provider.ts +++ b/server/src/services/github-external-object-provider.ts @@ -13,6 +13,14 @@ import type { type FetchLike = (url: string, init?: RequestInit) => Promise; +type GitHubObjectResolver = Omit & { + resolve(input: { + companyId: string; + object: { externalId: string; sanitizedCanonicalUrl: string | null }; + signal?: AbortSignal; + }): Promise; +}; + export interface GitHubExternalObjectProviderOptions { fetch?: FetchLike; requestTimeoutMs?: number; @@ -337,6 +345,8 @@ function pullRequestSnapshot(identity: GitHubObjectIdentity, body: Record resolveLinkPreviewGitHubCredential(db, scope)), + }).resolvers.find(candidate => candidate.objectType === "pull_request")!; + const result = await resolver.resolve({ companyId: scope.companyId, signal, object: { + externalId: `${owner}/${repo}#pull/${number}`, + sanitizedCanonicalUrl: target.canonicalUrl, + } }); + if (!result.ok) return { preview: null, maxAgeMs: Math.max(60_000, (result.retryAfterSeconds ?? 60) * 1_000) }; + const snapshot = result.snapshot; + const data = snapshot.data; + if (!data || typeof data.title !== "string" || !data.title.trim()) return { preview: null, maxAgeMs: 60_000 }; + const state: LinkPreviewState = data.merged === true ? "merged" + : data.state === "closed" ? "closed" + : data.state !== "open" ? "unknown" + : data.draft === true ? "draft" + : data.mergeable === false ? "conflicts" : "open"; + return { + preview: { + target, title: data.title, state, + author: typeof data.authorLogin === "string" ? data.authorLogin : null, + updatedAt: snapshot.remoteVersion ?? null, + }, + maxAgeMs: 60_000, + }; + }, + }; +} diff --git a/server/src/services/link-previews.ts b/server/src/services/link-previews.ts new file mode 100644 index 00000000000..29041b51ce9 --- /dev/null +++ b/server/src/services/link-previews.ts @@ -0,0 +1,60 @@ +import type { LinkPreviewResult, LinkPreviewTarget } from "@paperclipai/shared"; +import { recognizeLinkPreview } from "@paperclipai/shared"; + +export interface LinkPreviewScope { companyId: string; userId: string | null } +export interface LinkPreviewProvider { + key: string; + resolve(target: LinkPreviewTarget, scope: LinkPreviewScope, signal: AbortSignal): Promise; +} + +const UNAVAILABLE: LinkPreviewResult = { preview: null, maxAgeMs: 60_000 }; + +/** Request-driven and process-local. No DB writes, sweep, timers while idle, or cross-viewer reuse. */ +export function createLinkPreviewService(providers: readonly LinkPreviewProvider[], options: { + now?: () => number; timeoutMs?: number; maxEntries?: number; maxConcurrent?: number; +} = {}) { + const now = options.now ?? Date.now; + const cache = new Map(); + const inFlight = new Map>(); + let activeOperations = 0; + const byKey = new Map(providers.map(provider => [provider.key, provider])); + + return { + async get(scope: LinkPreviewScope, href: string): Promise { + const target = recognizeLinkPreview(href); + const provider = target && byKey.get(target.provider); + if (!target || !provider) return UNAVAILABLE; + const key = JSON.stringify([scope.companyId, scope.userId, target.canonicalUrl]); + const cached = cache.get(key); + if (cached && cached.expiresAt > now()) { + return { ...cached.result, maxAgeMs: cached.expiresAt - now() }; + } + const pending = inFlight.get(key); + if (pending) return pending; + if (activeOperations >= (options.maxConcurrent ?? 32)) return UNAVAILABLE; + + const controller = new AbortController(); + let timer: ReturnType; + const deadline = new Promise(resolve => { + timer = setTimeout(() => { controller.abort(); resolve(UNAVAILABLE); }, options.timeoutMs ?? 5_000); + }); + // Keep an underlying operation in the concurrency budget until it settles, + // even if a credential backend ignores cancellation after our deadline. + activeOperations++; + const operation = Promise.resolve().then(() => provider.resolve(target, scope, controller.signal)) + .catch(() => UNAVAILABLE).finally(() => { activeOperations--; }); + const request = Promise.race([operation, deadline]).then(result => { + const bounded = { ...result, maxAgeMs: Math.max(1_000, Math.min(result.maxAgeMs, 60 * 60_000)) }; + cache.delete(key); + while (cache.size >= (options.maxEntries ?? 1_000)) cache.delete(cache.keys().next().value!); + cache.set(key, { result: bounded, expiresAt: now() + bounded.maxAgeMs }); + return bounded; + }).finally(() => { + clearTimeout(timer!); + if (inFlight.get(key) === request) inFlight.delete(key); + }); + inFlight.set(key, request); + return request; + }, + }; +} diff --git a/tests/runner-e2e/source-sdk-public-inputs.json b/tests/runner-e2e/source-sdk-public-inputs.json index 5211d62ac65..1934700a771 100644 --- a/tests/runner-e2e/source-sdk-public-inputs.json +++ b/tests/runner-e2e/source-sdk-public-inputs.json @@ -264,7 +264,7 @@ "packages/shared/src/home-paths.ts": "fa4f1c440653b76b3dbb7a51026d557287deb45d", "packages/shared/src/humanize-connection.test.ts": "ba9e90a44c63452b0bf9f733adb90241921e8eca", "packages/shared/src/humanize-connection.ts": "fbd414476a0ef7106a7c80d8572208781b091699", - "packages/shared/src/index.ts": "a4ec3e9cca1eb3954d247eabd184b103ccefc2a2", + "packages/shared/src/index.ts": "aed8a87d7bed61a58d3a16f705c025ebca7e88bf", "packages/shared/src/issue-activity-identity.test.ts": "3dc294640f54c2be67943e3e0149cf4eff2c95ca", "packages/shared/src/issue-activity-identity.ts": "1b0f07ec3fe0506aaaf80a4fa9893f3baf8f0075", "packages/shared/src/issue-attribution.test.ts": "84563685e7f3c48029243cf1cc24a3a09ee3cde9", @@ -277,6 +277,8 @@ "packages/shared/src/issue-thread-interactions.test.ts": "6f516055744fcedd2e0c06ce82c6a1a019120762", "packages/shared/src/issue-write-denial.test.ts": "e21b93aa79e6635c019ac338eff954ec618a98ed", "packages/shared/src/issue-write-denial.ts": "e6b4d72a83f6e4e76e85e34a80f88d1d69b7dfe4", + "packages/shared/src/link-preview.test.ts": "136041c928343ad6272f43701b39cf95be255f3e", + "packages/shared/src/link-preview.ts": "ffa5eb960a7e4bcfa914b4d6029c9fc50f17e027", "packages/shared/src/markdown-work-products.test.ts": "7293d14ad45ec2b11b5deaab4ce070c36f7c9010", "packages/shared/src/markdown-work-products.ts": "16a88cfb717f325c247a516e78b47d53871b52ed", "packages/shared/src/mcp-config-help-prompt.ts": "fa58effb7cd995198909315d9ba2a93dedde0552", diff --git a/tests/storybook-visual/link-previews.spec.ts b/tests/storybook-visual/link-previews.spec.ts new file mode 100644 index 00000000000..b5d32145df2 --- /dev/null +++ b/tests/storybook-visual/link-previews.spec.ts @@ -0,0 +1,109 @@ +import { expect, test } from "@playwright/test"; + +async function finished(page: import("@playwright/test").Page) { + await expect.poll(() => page.evaluate(() => { + const channel = (window as unknown as { __STORYBOOK_ADDONS_CHANNEL__?: { data: Record> } }).__STORYBOOK_ADDONS_CHANNEL__; + return channel?.data.storyFinished?.[0]?.status; + })).toBe("success"); + expect(await page.evaluate(() => (window as unknown as { + __STORYBOOK_ADDONS_CHANNEL__?: { data: Record }; + }).__STORYBOOK_ADDONS_CHANNEL__?.data.playFunctionThrewException ?? [])).toEqual([]); +} + +for (const theme of ["light", "dark"]) { + for (const state of ["open", "merged", "conflicts", "draft", "closed", "loading", "unavailable", "failed-read", "narrow"]) { + test(`PR preview ${state} in ${theme}`, async ({ page }, testInfo) => { + if (state === "narrow") await page.setViewportSize({ width: 390, height: 844 }); + const errors: string[] = []; + const requests: string[] = []; + const remote: string[] = []; + page.on("pageerror", error => errors.push(error.message)); + page.on("request", request => { + if (request.url().includes("/link-preview?")) requests.push(request.url()); + if (["github.com", "api.github.com"].includes(new URL(request.url()).hostname)) remote.push(request.url()); + }); + await page.goto(`/iframe.html?id=tasks-link-previews--${state}&viewMode=story&globals=theme:${theme}`); + await finished(page); + const first = page.getByTestId("first-mention").getByRole("link"); + const second = page.getByTestId("second-mention").getByRole("link"); + const card = page.getByRole("tooltip"); + await expect(first).toHaveAttribute("data-link-state", "unknown"); + const mutedColor = await page.getByText("Hover or focus a link for context. Repeated links share what we learn.") + .evaluate(element => getComputedStyle(element).color); + await expect(first).toHaveCSS("color", mutedColor); + await first.hover({ position: { x: 5, y: 8 } }); + await expect(card).toBeVisible(); + if (["loading", "unavailable", "failed-read"].includes(state)) { + if (state === "loading") await expect(card.getByRole("status")).toHaveText("Loading pull request…"); + else await expect(card.getByRole("status")).toHaveCount(0); + await expect(first).toHaveAttribute("data-link-state", "unknown"); + } else { + await expect(first).toHaveAttribute("data-link-state", state === "narrow" ? "merged" : state); + await expect(second).toHaveAttribute("data-link-state", state === "narrow" ? "merged" : state); + if (["draft", "closed"].includes(state)) await expect(first).toHaveCSS("color", mutedColor); + } + // Fixture fetches are handled inside the browser and do not become network requests. + expect(remote).toEqual([]); + expect(requests).toEqual([]); + expect(errors).toEqual([]); + expect(await page.evaluate(() => document.documentElement.scrollWidth <= innerWidth)).toBe(true); + if (state !== "narrow") { + const trigger = await first.evaluate(element => element.getClientRects()[0]!.toJSON()); + const before = await card.boundingBox(); + expect(Math.abs(before!.x - trigger!.x)).toBeLessThanOrEqual(2); + await page.mouse.move(trigger.x + trigger.width - 2, trigger.y + trigger.height / 2); + const after = await card.boundingBox(); + expect(after!.x).toBeCloseTo(before!.x, 0); + expect(after!.y).toBeCloseTo(before!.y, 0); + } + await expect(second).toHaveAttribute("href", "https://www.github.com/Acme/Web/pull/241/files?diff=split#discussion_r123"); + await page.screenshot({ path: testInfo.outputPath(`pr-preview-${state}-${theme}.png`), fullPage: true }); + await first.press("Escape"); + await expect(card).toBeHidden(); + await second.focus(); + await expect(card).toBeVisible(); + await expect(second).toHaveAttribute("aria-describedby"); + }); + } +} + +test("one deferred HTTP lookup updates repeated links and is reused on later hover", async ({ page }) => { + let count = 0; + await page.route("**/api/companies/company-storybook/link-preview?*", async route => { + count++; + const canonicalUrl = new URL(route.request().url()).searchParams.get("url"); + await route.fulfill({ json: { + preview: { target: { provider: "github", kind: "pull_request", canonicalUrl, label: "acme/web#241" }, title: "Shared result", state: "merged", author: "octocat", updatedAt: null }, + maxAgeMs: 60_000, + } }); + }); + // Neutral has no opening play step. Replace fixture fetch only for this read + // so it crosses the browser's real HTTP boundary and can be counted. + await page.addInitScript(() => { + const realFetch = window.fetch.bind(window); + let activeFetch = realFetch; + Object.defineProperty(window, "fetch", { + configurable: true, + get: () => activeFetch, + set: (fixtureFetch: typeof fetch) => { + activeFetch = (input, init) => { + const url = input instanceof Request ? input.url : String(input); + return url.includes("/link-preview?") ? realFetch(input, init) : fixtureFetch(input, init); + }; + }, + }); + }); + await page.goto("/iframe.html?id=tasks-link-previews--neutral&viewMode=story"); + await finished(page); + expect(count).toBe(0); + const links = page.locator("a[data-link-state]"); + await links.first().hover({ position: { x: 5, y: 8 } }); + await expect(links.first()).toHaveAttribute("data-link-state", "merged"); + await expect(links.nth(1)).toHaveAttribute("data-link-state", "merged"); + expect(count).toBe(1); + await page.mouse.move(0, 0); + await expect(page.getByRole("tooltip")).toBeHidden(); + await links.nth(1).hover({ position: { x: 5, y: 8 } }); + await expect(page.getByRole("tooltip")).toBeVisible(); + expect(count).toBe(1); +}); diff --git a/ui/src/components/LinkPreview.tsx b/ui/src/components/LinkPreview.tsx new file mode 100644 index 00000000000..9680ad31de3 --- /dev/null +++ b/ui/src/components/LinkPreview.tsx @@ -0,0 +1,84 @@ +import { useId, useMemo, useRef, useState, type ReactElement } from "react"; +import { useQuery } from "@tanstack/react-query"; +import { PreviewCard } from "@base-ui/react/preview-card"; +import { GitMerge, GitPullRequest, GitPullRequestClosed, GitPullRequestDraft, TriangleAlert } from "lucide-react"; +import type { LinkPreview as Preview, LinkPreviewResult, LinkPreviewState, LinkPreviewTarget } from "@paperclipai/shared"; +import { useLinkPreviewContext, type LinkPreviewContextValue } from "@/context/LinkPreviewContext"; +import { timeAgo } from "@/lib/timeAgo"; +import { retryTransientOnly } from "@/lib/query-client"; + +export const linkPreviewStateLabel: Record = { + unknown: "Status unknown", open: "Open", merged: "Merged", conflicts: "Merge conflicts", draft: "Draft", closed: "Closed", +}; +export const linkPreviewStateIcon = { + unknown: GitPullRequest, open: GitPullRequest, merged: GitMerge, + conflicts: TriangleAlert, draft: GitPullRequestDraft, closed: GitPullRequestClosed, +}; + +function PreviewCardContents({ target, preview, loading }: { target: LinkPreviewTarget; preview?: Preview | null; loading: boolean }) { + const state = preview?.state ?? "unknown"; + const Icon = linkPreviewStateIcon[state]; + const updated = preview?.updatedAt && Number.isFinite(Date.parse(preview.updatedAt)) ? preview.updatedAt : null; + return
+
+
+ {preview ? <> +

{preview.title}

+ {(preview.author || updated) ?

+ {preview.author ? {preview.author} : null} + {updated ? Updated : null} +

: null} + : loading ?

Loading pull request…

: null} + {target.canonicalUrl} +
; +} + +function ConnectedLinkPreview({ context, target, children }: { + context: LinkPreviewContextValue; target: LinkPreviewTarget; + children: (preview: Preview | null, describedBy?: string) => ReactElement; +}) { + const [open, setOpen] = useState(false); + const id = useId(); + const triggerRef = useRef(null); + // Inline links can wrap: their union rectangle starts on a later line. + // Anchor to the first line (the icon), never to the pointer or union box. + const anchor = useMemo(() => ({ + getBoundingClientRect: () => triggerRef.current?.getClientRects()[0] ?? new DOMRect(), + get contextElement() { return triggerRef.current ?? undefined; }, + }), []); + const query = useQuery({ + queryKey: ["link-preview", context.companyId, context.userId, target.canonicalUrl], + // Optional enrichment: a failed read is a cached absence, not a global error. + queryFn: async (): Promise => { + try { return await context.load(target); } catch { return { preview: null, maxAgeMs: 60_000 }; } + }, + enabled: open, + staleTime: query => query.state.data?.maxAgeMs ?? 60_000, + gcTime: 5 * 60_000, + retry: retryTransientOnly(0), refetchOnMount: false, refetchOnWindowFocus: false, refetchOnReconnect: false, + }); + return + setOpen(true)} onBlur={() => setOpen(false)} + render={children(query.data?.preview ?? null, open ? id : undefined)} /> + + + + + + + + ; +} + +/** Shared by link renderers; without the app provider links stay entirely static. */ +export function LinkPreview({ target, children }: { + target: LinkPreviewTarget; + children: (preview: Preview | null, describedBy?: string) => ReactElement; +}) { + const context = useLinkPreviewContext(); + return context ? {children} : children(null); +} diff --git a/ui/src/components/MarkdownBody.tsx b/ui/src/components/MarkdownBody.tsx index 455d0ec6d6b..cbbf96641c2 100644 --- a/ui/src/components/MarkdownBody.tsx +++ b/ui/src/components/MarkdownBody.tsx @@ -11,6 +11,7 @@ import Markdown, { defaultUrlTransform, type Components, type Options } from "re import remarkGfm from "remark-gfm"; import { cn } from "../lib/utils"; import { MarkdownUrlLink } from "./MarkdownUrlLink"; +import { recognizeLinkPreview, type LinkPreviewState } from "@paperclipai/shared"; import { Link, useCaseHref } from "@/lib/router"; import { useTheme } from "../context/ThemeContext"; import { useOptionalCompany } from "../context/CompanyContext"; @@ -279,6 +280,18 @@ function MarkdownExternalLink({ `${displayKey} — ${statusLabel}${livenessSuffix}`, ]; const title = titleParts.filter(Boolean).join(" · "); + if (recognizeLinkPreview(href)) { + const knownState: LinkPreviewState = reference.statusIconKey === "git-merge" || reference.statusLabel === "Merged" ? "merged" + : reference.statusLabel === "Merge conflicts" ? "conflicts" + : reference.statusLabel === "Draft" ? "draft" + : reference.statusCategory === "open" ? "open" + : reference.statusCategory === "closed" ? "closed" : "unknown"; + return + {children} + ; + } return ( { it("decorates a known URL with the external status icon and metadata attributes", () => { const html = render("Take a look: https://github.com/acme/web/pull/241", successReference); - expect(html).toContain('class="paperclip-markdown-external-ref"'); + expect(html).toContain('data-link-state="merged"'); + expect(html).toContain("lucide-git-merge"); expect(html).toContain('data-external-link="resolved"'); expect(html).toContain('data-external-status="succeeded"'); expect(html).toContain('data-external-liveness="fresh"'); diff --git a/ui/src/components/MarkdownUrlLink.tsx b/ui/src/components/MarkdownUrlLink.tsx index 79995ecff2f..43316bb1d80 100644 --- a/ui/src/components/MarkdownUrlLink.tsx +++ b/ui/src/components/MarkdownUrlLink.tsx @@ -2,6 +2,8 @@ import { cloneElement, isValidElement, type CSSProperties, type ReactNode } from import { CircleDot, ExternalLink, GitPullRequest } from "lucide-react"; import { GithubIcon } from "./icons/github-icon"; import { parseGitHubLink } from "@/lib/github-link"; +import { recognizeLinkPreview, type LinkPreviewState, type LinkPreview as Preview } from "@paperclipai/shared"; +import { LinkPreview, linkPreviewStateIcon, linkPreviewStateLabel } from "./LinkPreview"; function isExternalHttpUrl(href: string | undefined): boolean { if (!href) return false; @@ -41,26 +43,41 @@ function compactUrlLabel(children: ReactNode, href: string | undefined, label: s return children; } -export function MarkdownUrlLink({ href, children, title, style }: { +export function MarkdownUrlLink({ href, children, title, style, knownState, externalStatus }: { href?: string; children: ReactNode; title?: string; style?: CSSProperties; + knownState?: LinkPreviewState; + externalStatus?: { category: string; liveness: string; ariaLabel: string }; }) { const github = parseGitHubLink(href); const external = isExternalHttpUrl(href); const Icon = github?.kind === "pull_request" ? GitPullRequest : github?.kind === "issue" ? CircleDot : GithubIcon; const label = github?.label ? compactUrlLabel(children, href, github.label) : children; - const leadingIcon = github ? - {github ? {github.kind === "pull_request" ? "GitHub pull request: " : github.kind === "issue" ? "GitHub issue: " : "GitHub: "} : null} - {renderLinkBody(label, leadingIcon, trailingIcon)} - ; + const target = recognizeLinkPreview(href); + const render = (state: LinkPreviewState = knownState ?? "unknown", describedBy?: string, preview?: Preview | null) => { + const StateIcon = target ? linkPreviewStateIcon[state] : Icon; + const leadingIcon = github ?