diff --git a/.changeset/bot-link-adoption.md b/.changeset/bot-link-adoption.md new file mode 100644 index 00000000..0bd2cafd --- /dev/null +++ b/.changeset/bot-link-adoption.md @@ -0,0 +1,5 @@ +--- +"@buildinternet/uploads": minor +--- + +`.uploads.yml` gained an `adoptLinkedFiles` key (on by default for bound repos): when a PR body or comment references an uploads.sh file URL that was pasted in directly — rather than uploaded via `--pr`/`attach --branch` — the webhook now adopts it into that PR/issue's attachment context, so it gets pairing, dedupe, and screenshots-page grouping automatically. Only files already in the repo's own bound workspace are adopted; links to any other workspace's files are silently ignored. A lone adopted image with nothing else to consolidate doesn't trigger a managed comment on its own. diff --git a/apps/api/src/github-link-adopt.test.ts b/apps/api/src/github-link-adopt.test.ts new file mode 100644 index 00000000..85677d38 --- /dev/null +++ b/apps/api/src/github-link-adopt.test.ts @@ -0,0 +1,294 @@ +/** + * Webhook link adoption (issue #701) — pure URL extraction, workspace-scoped + * resolution, the copy+additive-metadata adoption itself, and the noise-guard + * gate on comment sync. `adoptLinkedFilesForWebhook`'s repo-link/knob no-ops + * mirror `ingestForWebhook`'s own tests (github-ingest.test.ts). + */ +import { describe, expect, it } from "vitest"; +import { + adoptLinkedFiles, + adoptLinkedFilesForWebhook, + extractCandidateUrls, + hasLinkCandidate, + resolveAdoptableKeys, +} from "./github-link-adopt"; +import { getFileMetadata } from "./file-metadata"; +import { recordRepoLink } from "./github-repo-links"; +import { sha256Hex, type WorkspaceRecord } from "./workspace"; +import { FakeKv } from "../test/fake-kv"; +import { FakeR2Bucket } from "../test/fake-r2"; +import { GITHUB_APP_CFG_ENV } from "../test/github-app-env"; +import { UsageFakeD1 } from "../test/usage-fake-d1"; + +const WS = "acme"; +const OTHER_WS = "other"; +const REPO = "acme/web"; +const NUM = 12; +const PREFIX = "acme/"; +const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 1, 2, 3, 4]); + +interface Seeded { + env: Env; + db: UsageFakeD1; + bucket: FakeR2Bucket; + kv: FakeKv; +} + +async function seededEnv(): Promise { + const record: WorkspaceRecord = { + provider: "r2", + bucket: "b", + binding: "UPLOADS_DEFAULT", + prefix: PREFIX, + publicBaseUrl: "https://storage.uploads.sh", + tokens: [{ hash: await sha256Hex("up_acme_testtoken"), createdAt: new Date().toISOString() }], + }; + const otherRecord: WorkspaceRecord = { + provider: "r2", + bucket: "b2", + binding: "UPLOADS_DEFAULT", + prefix: "other/", + publicBaseUrl: "https://storage.uploads.sh", + tokens: [], + }; + const registry = { + get: (async (key: string) => { + if (key === `ws:${WS}`) return record; + if (key === `ws:${OTHER_WS}`) return otherRecord; + return null; + }) as unknown as KVNamespace["get"], + }; + const bucket = new FakeR2Bucket(); + const db = new UsageFakeD1(); + const kv = new FakeKv(); + // Public by default (cache hit — no network call) for resolveGhKeyContextSafe. + kv.store.set("ghpriv:acme/web", { value: "0" }); + const env = { + REGISTRY: registry, + DB: db, + UPLOADS_DEFAULT: bucket, + GITHUB_CACHE: kv, + ...GITHUB_APP_CFG_ENV, + } as unknown as Env; + return { env, db, bucket, kv }; +} + +async function seedSource(seeded: Seeded, key: string, prefix = PREFIX) { + await seeded.bucket.put(`${prefix}${key}`, PNG, { + httpMetadata: { contentType: "image/png" }, + customMetadata: {}, + }); +} + +describe("extractCandidateUrls / hasLinkCandidate", () => { + it("finds distinct http(s) urls, dedups, strips trailing punctuation", () => { + const text = + "See https://storage.uploads.sh/acme/f/x.png, also " + + "https://storage.uploads.sh/acme/f/x.png. And http://example.com/y."; + expect(extractCandidateUrls(text)).toEqual([ + "https://storage.uploads.sh/acme/f/x.png", + "http://example.com/y", + ]); + }); + + it("hasLinkCandidate is a cheap presence check", () => { + expect(hasLinkCandidate("no links here")).toBe(false); + expect(hasLinkCandidate("check http://x")).toBe(true); + expect(hasLinkCandidate("check https://x")).toBe(true); + }); +}); + +describe("resolveAdoptableKeys", () => { + it("resolves a storage-host URL to a key in this workspace", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/shot.png"); + const { keys, skipped } = await resolveAdoptableKeys( + seeded.env, + WS, + "screenshot: https://storage.uploads.sh/acme/f/shot.png", + ); + expect(keys).toEqual(["f/shot.png"]); + expect(skipped).toEqual([]); + }); + + it("resolves the /f/ page URL spelling too, deduped against the storage host", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/shot.png"); + const text = + "https://storage.uploads.sh/acme/f/shot.png and https://uploads.sh/f/acme/f/shot.png"; + const { keys } = await resolveAdoptableKeys(seeded.env, WS, text); + expect(keys).toEqual(["f/shot.png"]); + }); + + it("silently drops URLs belonging to a different workspace", async () => { + const seeded = await seededEnv(); + const text = "https://storage.uploads.sh/other/f/shot.png"; + const { keys, skipped } = await resolveAdoptableKeys(seeded.env, WS, text); + expect(keys).toEqual([]); + expect(skipped).toEqual([text]); + }); + + it("silently drops non-uploads.sh URLs", async () => { + const seeded = await seededEnv(); + const { keys, skipped } = await resolveAdoptableKeys( + seeded.env, + WS, + "see https://example.com/whatever.png", + ); + expect(keys).toEqual([]); + expect(skipped).toEqual(["https://example.com/whatever.png"]); + }); +}); + +describe("adoptLinkedFiles", () => { + it("copies a resolved link additively (gh.* on top of the source's own metadata)", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/shot.png"); + await import("./file-metadata").then(({ setFileMetadata }) => + setFileMetadata(seeded.env.DB, WS, "f/shot.png", { path: "src/App.tsx" }), + ); + + const summary = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/shot.png", + ); + + expect(summary.adopted).toEqual([`gh/acme/web/pull/${NUM}/shot.png`]); + const meta = await getFileMetadata(seeded.env.DB, WS, `gh/acme/web/pull/${NUM}/shot.png`); + expect(meta.path).toBe("src/App.tsx"); // preserved, PR #157 contract + expect(meta["gh.repo"]).toBe(REPO.toLowerCase()); + expect(meta["gh.kind"]).toBe("pull"); + expect(meta["gh.number"]).toBe(String(NUM)); + + // Source key itself is untouched — the pasted URL still resolves. + expect(seeded.bucket.store.has(`${PREFIX}f/shot.png`)).toBe(true); + }); + + it("noise guard: a single lone adoption with nothing else does NOT sync", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/shot.png"); + // No GITHUB_APP config beyond the fixture's dummy id/key, but + // githubAppConfig(env) IS configured here — postManagedComment would + // proceed to installationForRepo, which needs a ghinst KV entry. Leaving + // it unseeded means a sync attempt would try a real network call; assert + // instead that synced stays false so that call never happens. + const summary = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/shot.png", + ); + expect(summary.adopted.length).toBe(1); + expect(summary.synced).toBe(false); + }); + + it("noise guard: two adopted links in one pass DOES sync", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/a.png"); + await seedSource(seeded, "f/b.png"); + seeded.kv.store.set("ghinst:acme/web", { value: "1" }); + const summary = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/a.png and https://storage.uploads.sh/acme/f/b.png", + ); + expect(summary.adopted.length).toBe(2); + // App-unconfigured or install-lookup-failure degrades postManagedComment + // to a no-op result, but the CALL still happens — synced reflects that an + // attempt was made, matching postAttachExisting's own contract that + // comment sync never throws. + expect(summary.synced).toBe(true); + }); + + it("noise guard: one adopted link mixed with an existing attachment DOES sync", async () => { + const seeded = await seededEnv(); + // Pre-existing attachment already under the PR's gh key prefix. + await seeded.bucket.put(`${PREFIX}gh/acme/web/pull/${NUM}/existing.png`, PNG, { + httpMetadata: { contentType: "image/png" }, + customMetadata: {}, + }); + await seedSource(seeded, "f/shot.png"); + const summary = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/shot.png", + ); + expect(summary.adopted.length).toBe(1); + expect(summary.synced).toBe(true); + }); + + it("resolving to no keys is a pure no-op (no workspace load, no sync)", async () => { + const seeded = await seededEnv(); + const summary = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "nothing to see here", + ); + expect(summary).toEqual({ adopted: [], skipped: [], synced: false }); + }); + + it("is idempotent: re-adopting the same source overwrites the same destination key", async () => { + const seeded = await seededEnv(); + await seedSource(seeded, "f/shot.png"); + const first = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/shot.png", + ); + const second = await adoptLinkedFiles( + seeded.env, + WS, + null, + { repo: REPO, kind: "pull", num: NUM }, + "https://storage.uploads.sh/acme/f/shot.png", + ); + expect(first.adopted).toEqual(second.adopted); + expect(seeded.bucket.store.size).toBe(2); // source + the one dest key, not two dest copies + }); +}); + +describe("adoptLinkedFilesForWebhook", () => { + it("no-ops when the repo isn't linked to any workspace", async () => { + const seeded = await seededEnv(); + await expect( + adoptLinkedFilesForWebhook(seeded.env, { + repo: REPO, + kind: "pull", + num: NUM, + source: "body", + }), + ).resolves.toBeUndefined(); + }); + + it("no-ops when adoptLinkedFiles resolves off via repo config", async () => { + const seeded = await seededEnv(); + await recordRepoLink(seeded.env.DB, REPO, WS, "test"); + const fetchImpl = (async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/contents/.uploads.yml")) { + return new Response("comment:\n adoptLinkedFiles: false\n", { status: 200 }); + } + if (url.includes("/contents/")) return new Response("not found", { status: 404 }); + throw new Error(`unexpected fetch: ${url}`); + }) as typeof fetch; + await expect( + adoptLinkedFilesForWebhook( + seeded.env, + { repo: REPO, kind: "pull", num: NUM, source: "body" }, + { fetchImpl }, + ), + ).resolves.toBeUndefined(); + }); +}); diff --git a/apps/api/src/github-link-adopt.ts b/apps/api/src/github-link-adopt.ts new file mode 100644 index 00000000..3174d2a5 --- /dev/null +++ b/apps/api/src/github-link-adopt.ts @@ -0,0 +1,285 @@ +/** + * Webhook "link adoption" (issue #701): scans a PR/issue body or comment for + * plain uploads.sh file URLs pasted by a human (or an agent uploading via raw + * curl, outside the paved `--pr`/`attach --branch` path) and adopts every one + * that resolves to an object in the repo's OWN bound workspace into that + * PR/issue's attachment context — so it gets pairing, dedupe, the activity + * feed, and screenshots-page grouping "for free", the same as anything + * uploaded through `attach`. + * + * Reuses #702's machinery verbatim rather than reimplementing it: + * `resolveAttachSourceKey` (github-attach.ts) for the URL→key resolution + * (storage host, embed host, and the `/f/` page all handled there, and a URL + * that doesn't resolve to an object in THIS workspace throws — the same + * structural cross-workspace rejection every other `files:write` path gets, + * satisfying "URLs from other workspaces are silently ignored" here since + * every throw from a candidate URL is caught and the URL is just dropped), + * and `attachExistingObject` for the copy + additive `gh.*` metadata merge + * (PR #157 preserve contract). + * + * COPY, never move (`attachExistingObject`'s default `move: false`): the + * originally-pasted URL keeps resolving untouched after adoption, which is + * also why adopted files are never migrated into a private repo's + * `gh/private/` prefix — only the fresh COPY lands there, the source `f/` key + * some human already pasted a link to is left exactly where it was. Copying + * (rather than a metadata-only in-place tag) is the deliberate choice here: + * the managed comment's `gatherCommentBody` lists objects by R2 key prefix, + * not by `gh.*` metadata query, so only a copy under the PR's attachment + * prefix is visible to the existing renderer without also changing it. + * + * Idempotent: re-adopting the same source into the same target overwrites + * the same destination key in place (`attachExistingObject`'s own contract), + * so repeated webhook redeliveries and comment-edit rescans are safe no-ops + * on the object/metadata side. The `postManagedComment` call this module + * gates is separately safe to repeat (its own upsert is idempotent). + * + * Noise guard: a lone adopted image that's already visible inline in the + * PR/comment doesn't warrant a managed comment repeating it — see + * `shouldSyncAfterAdopt`. + */ +import { attachExistingObject, resolveAttachSourceKey } from "./github-attach"; +import { commentCacheKey, gatherCommentBody } from "./github-comment"; +import type { GhTarget } from "./github-comment-render"; +import { postManagedComment } from "./github-comment-service"; +import { findRepoLinkStrict } from "./github-repo-links"; +import { resolveRepoCommentOptions } from "./repo-comment-config"; +import { storageConfig } from "./storage"; +import { + githubAppConfig, + githubFetch, + githubHeaders, + installationForRepo, + installationToken, +} from "./github-app"; +import { loadWorkspaceRecord } from "./workspace"; + +export interface AdoptSourceRef { + repo: string; + kind: GhTarget["kind"]; + num: number; + /** "body" or "comment:" — same vocabulary as `IngestSourceRef`, unused + * beyond logging (adoption re-scans the CURRENT text, it has no ledger). */ + source: string; +} + +export interface AdoptSummary { + /** Destination keys copied/refreshed this pass (may already have existed — + * adoption always re-copies, an idempotent overwrite). */ + adopted: string[]; + /** Candidate URLs that looked like uploads.sh links but didn't resolve to + * an object in this workspace (wrong workspace, unknown key, deleted). */ + skipped: string[]; + /** Whether `postManagedComment` was actually invoked this pass. */ + synced: boolean; +} + +/** Cheap, I/O-free reject: no http(s) URL at all means nothing to scan for — + * safe to call from a pure payload-extraction context (`extractWebhookEvent`) + * before any workspace/storage lookup exists. */ +export function hasLinkCandidate(text: string): boolean { + return /https?:\/\//i.test(text); +} + +const URL_RE = /https?:\/\/[^\s)"'<>\]]+/gi; + +/** Distinct http(s) URLs found in `text`, trailing prose punctuation + * stripped, order preserved, first occurrence wins on duplicates. */ +export function extractCandidateUrls(text: string): string[] { + const seen = new Set(); + const out: string[] = []; + for (const m of text.matchAll(URL_RE)) { + const url = m[0].replace(/[.,;:]+$/, ""); + if (seen.has(url)) continue; + seen.add(url); + out.push(url); + } + return out; +} + +/** + * Resolves every uploads.sh-shaped URL in `text` to a key in `workspaceName`'s + * own bucket, silently dropping anything that isn't one of the three URL + * shapes, doesn't belong to this workspace, or isn't a URL at all. Order + * preserved, deduplicated by resolved key (two different URL spellings for + * the same object collapse to one adoption). + */ +export async function resolveAdoptableKeys( + env: Env, + workspaceName: string, + text: string, +): Promise<{ keys: string[]; skipped: string[] }> { + const ws = await loadWorkspaceRecord(env, workspaceName); + if (!ws) return { keys: [], skipped: [] }; + const cfg = await storageConfig(env, ws); + + const seen = new Set(); + const keys: string[] = []; + const skipped: string[] = []; + for (const url of extractCandidateUrls(text)) { + let key: string; + try { + key = await resolveAttachSourceKey(env, cfg, workspaceName, url); + } catch { + skipped.push(url); + continue; + } + if (seen.has(key)) continue; + seen.add(key); + keys.push(key); + } + return { keys, skipped }; +} + +/** + * Adoption is worth a comment sync only when there's something to + * consolidate (issue #701's noise guard): a lone newly-scanned link with no + * other attachments already staged/attached for this target, and no managed + * comment yet, is already fully visible inline — posting a comment that just + * repeats it is noise. Sync fires when: + * - two or more links resolved this pass, or + * - at least one resolved AND the target already has other attachments + * (staged/attached/previously-adopted) under its comment prefix, or + * - a managed comment already exists for this target (heal/refresh it + * rather than leave it stale). + */ +async function shouldSyncAfterAdopt( + env: Env, + workspaceName: string, + target: GhTarget, + adoptedCount: number, + preexistingCount: number, +): Promise { + if (adoptedCount === 0) return false; + if (adoptedCount >= 2) return true; + if (preexistingCount > 0) return true; + + const cachedCommentId = await env.GITHUB_CACHE.get(commentCacheKey(workspaceName, target)); + return cachedCommentId !== null; +} + +/** + * Copy every resolved-key adoption into `target`'s attachment prefix (via + * #702's `attachExistingObject`), then sync the managed comment only when + * `shouldSyncAfterAdopt` says there's something worth consolidating. The + * "does this target already have other attachments" check that feeds the + * noise guard is a `gatherCommentBody` call BEFORE any copy this pass makes, + * so a single lone adoption is judged against the PRE-adoption state, not + * inflated by its own copy. + */ +export async function adoptLinkedFiles( + env: Env, + workspaceName: string, + mintingUserId: string | null, + target: GhTarget, + text: string, +): Promise { + const { keys, skipped } = await resolveAdoptableKeys(env, workspaceName, text); + const summary: AdoptSummary = { adopted: [], skipped, synced: false }; + if (keys.length === 0) return summary; + + const ws = await loadWorkspaceRecord(env, workspaceName); + if (!ws) return summary; + + // Baseline BEFORE this pass's copies land, so a lone adoption isn't judged + // against a prefix count its own copy just inflated. + const before = await gatherCommentBody(env, ws, workspaceName, target); + + for (const key of keys) { + try { + const result = await attachExistingObject(env, ws, workspaceName, { + source: key, + target: { repo: target.repo, kind: target.kind, num: target.num }, + }); + summary.adopted.push(result.key); + } catch (err) { + // Source vanished between resolve and copy (deleted concurrently), or a + // transient storage failure — either way this one URL is skipped, the + // rest of the pass continues. Never fails the whole webhook delivery. + console.error( + JSON.stringify({ + message: "link adoption: attach failed for resolved key", + repo: target.repo, + num: target.num, + key, + error: err instanceof Error ? err.message : String(err), + }), + ); + summary.skipped.push(key); + } + } + if (summary.adopted.length === 0) return summary; + + const sync = await shouldSyncAfterAdopt( + env, + workspaceName, + target, + summary.adopted.length, + before.count, + ); + if (sync) { + await postManagedComment(env, ws, workspaceName, mintingUserId, target, {}); + summary.synced = true; + } + return summary; +} + +/** GET the current body/comment text from GitHub, or `null` on 404 — same + * shape as github-ingest.ts's `fetchSourceText`, duplicated locally rather + * than shared since it's a four-line GET with no other coupling. */ +async function fetchSourceText( + fetchImpl: typeof fetch, + token: string, + ref: AdoptSourceRef, +): Promise { + const url = + ref.source === "body" + ? `https://api.github.com/repos/${ref.repo}/issues/${ref.num}` + : `https://api.github.com/repos/${ref.repo}/issues/comments/${ref.source.slice("comment:".length)}`; + const res = await githubFetch(fetchImpl, url, { headers: githubHeaders(token) }); + if (res.status === 404) return null; + if (!res.ok) throw new Error(`github source fetch failed: ${res.status}`); + const body = (await res.json()) as { body?: string | null }; + return body.body ?? null; +} + +/** + * Webhook entry point: resolves the repo→workspace link, the per-repo/per- + * workspace `adoptLinkedFiles` knob, fetches the current text for `ref`, and + * adopts any resolvable uploads.sh links in it. No-ops (resolves, no GitHub + * calls) when the repo isn't linked, the workspace can't be loaded, or the + * knob is off — mirrors `ingestForWebhook`'s contract exactly. Throws on + * transient failure (D1 outage, token mint failure, non-404 GitHub error) so + * the caller's queue retries the delivery. + */ +export async function adoptLinkedFilesForWebhook( + env: Env, + ref: AdoptSourceRef, + deps: { fetchImpl?: typeof fetch } = {}, +): Promise { + const link = await findRepoLinkStrict(env.DB, ref.repo); + if (!link) return; + const ws = await loadWorkspaceRecord(env, link.workspaceName); + if (!ws) return; + + const { options } = await resolveRepoCommentOptions(env, ws, ref.repo); + if (!options.adoptLinkedFiles) return; + + const fetchImpl = deps.fetchImpl ?? fetch; + const cfg = githubAppConfig(env); + if (!cfg) return; + const installationId = await installationForRepo(env, cfg, ref.repo, fetchImpl); + if (installationId === null) return; + const token = await installationToken(env, cfg, installationId, fetchImpl); + if (!token) throw new Error("github installation token mint failed"); + + const text = await fetchSourceText(fetchImpl, token, ref); + if (text === null || !hasLinkCandidate(text)) return; + + await adoptLinkedFiles( + env, + link.workspaceName, + null, + { repo: ref.repo, kind: ref.kind, num: ref.num }, + text, + ); +} diff --git a/apps/api/src/github-webhook-queue.test.ts b/apps/api/src/github-webhook-queue.test.ts index ea8f9c64..f0071179 100644 --- a/apps/api/src/github-webhook-queue.test.ts +++ b/apps/api/src/github-webhook-queue.test.ts @@ -14,9 +14,14 @@ import { handleGithubWebhookBatch, } from "./github-webhook-queue"; import { ingestForWebhook } from "./github-ingest"; +import { adoptLinkedFilesForWebhook } from "./github-link-adopt"; import { FakeKv } from "../test/fake-kv"; vi.mock("./github-ingest", () => ({ ingestForWebhook: vi.fn() })); +vi.mock("./github-link-adopt", () => ({ + adoptLinkedFilesForWebhook: vi.fn(), + hasLinkCandidate: (text: string) => /https?:\/\//i.test(text), +})); class FakeQueue { sent: WebhookEvent[] = []; @@ -206,6 +211,67 @@ describe("extractWebhookEvent", () => { }); expect(deletedPlainText?.ingest).toBeUndefined(); }); + + const UPLOADS_URL = "https://storage.uploads.sh/acme/f/shot.png"; + + it("pull_request opened with a link sets adopt", () => { + const ev = extractWebhookEvent("pull_request", { + action: "opened", + repository: { full_name: "acme/app" }, + pull_request: { number: 7, body: `screenshot: ${UPLOADS_URL}` }, + }); + expect(ev?.adopt).toEqual({ repo: "acme/app", kind: "pull", num: 7, source: "body" }); + }); + + it("pull_request opened without any link sets no adopt", () => { + const ev = extractWebhookEvent("pull_request", { + action: "opened", + repository: { full_name: "acme/app" }, + pull_request: { number: 7, body: "hi" }, + }); + expect(ev?.adopt).toBeUndefined(); + }); + + it("issues edited with a body change always sets adopt (removal case)", () => { + const ev = extractWebhookEvent("issues", { + action: "edited", + changes: { body: { from: "old" } }, + repository: { full_name: "acme/app" }, + issue: { number: 3, body: "no links anymore" }, + }); + expect(ev?.adopt).toEqual({ repo: "acme/app", kind: "issues", num: 3, source: "body" }); + }); + + it("issue_comment created with a link sets adopt with comment source", () => { + const ev = extractWebhookEvent("issue_comment", { + action: "created", + repository: { full_name: "acme/app" }, + issue: { number: 7, pull_request: {} }, + comment: { id: 44, body: UPLOADS_URL, user: { login: "octocat", type: "User" } }, + }); + expect(ev?.adopt).toEqual({ repo: "acme/app", kind: "pull", num: 7, source: "comment:44" }); + }); + + it("issue_comment deleted never sets adopt (nothing left to rescan)", () => { + const ev = extractWebhookEvent("issue_comment", { + action: "deleted", + repository: { full_name: "acme/app" }, + issue: { number: 7, pull_request: {} }, + comment: { id: 44, body: UPLOADS_URL, user: { login: "octocat", type: "User" } }, + }); + expect(ev?.adopt).toBeUndefined(); + }); + + it("issue_comment edited by our own bot sets no adopt (loop guard)", () => { + const ev = extractWebhookEvent("issue_comment", { + action: "edited", + repository: { full_name: "acme/app" }, + issue: { number: 7, pull_request: {} }, + comment: { id: 44, body: "our own write", user: { login: "our-bot", type: "Bot" } }, + sender: { login: "our-bot", type: "Bot" }, + }); + expect(ev?.adopt).toBeUndefined(); + }); }); describe("handleWebhook producer path", () => { @@ -297,6 +363,24 @@ describe("handleGithubWebhookBatch", () => { const m = msg({ keys: [], ingest: ref }); await handleGithubWebhookBatch(batch(GITHUB_WEBHOOK_QUEUE, [m]), envWith(new FakeKv())); expect(m.retried).toBe(true); + }); + + it("dispatches adopt events to adoptLinkedFilesForWebhook and acks", async () => { + vi.mocked(adoptLinkedFilesForWebhook).mockResolvedValueOnce(undefined); + const ref = { repo: "acme/app", kind: "pull" as const, num: 7, source: "body" }; + const m = msg({ keys: [], adopt: ref }); + await handleGithubWebhookBatch(batch(GITHUB_WEBHOOK_QUEUE, [m]), envWith(new FakeKv())); + expect(adoptLinkedFilesForWebhook).toHaveBeenCalledWith(expect.anything(), ref); + expect(m.acked).toBe(true); + expect(m.retried).toBe(false); + }); + + it("retries a message whose adoptLinkedFilesForWebhook throws", async () => { + vi.mocked(adoptLinkedFilesForWebhook).mockRejectedValueOnce(new Error("transient")); + const ref = { repo: "acme/app", kind: "pull" as const, num: 7, source: "body" }; + const m = msg({ keys: [], adopt: ref }); + await handleGithubWebhookBatch(batch(GITHUB_WEBHOOK_QUEUE, [m]), envWith(new FakeKv())); + expect(m.retried).toBe(true); expect(m.acked).toBe(false); }); }); diff --git a/apps/api/src/github-webhook.ts b/apps/api/src/github-webhook.ts index 7fe1034d..33014e03 100644 --- a/apps/api/src/github-webhook.ts +++ b/apps/api/src/github-webhook.ts @@ -43,6 +43,11 @@ import { commentCacheKey, gatherCommentBody, upsertBotComment } from "./github-c import { ATTACHMENTS_MARKER } from "./github-comment-render"; import type { GhTarget } from "./github-comment-render"; import { ingestForWebhook, type IngestSourceRef } from "./github-ingest"; +import { + adoptLinkedFilesForWebhook, + hasLinkCandidate, + type AdoptSourceRef, +} from "./github-link-adopt"; import { promoteBranchAttachments } from "./github-promote"; // Strict lookup on purpose (#287): a D1 outage must THROW so the queue // consumer retries the event, not read as "repo not linked" and ack-drop it. @@ -304,6 +309,10 @@ export interface WebhookEvent { /** Opt-in GitHub-native attachment ingest (spec 2026-08-11). Source ref only — * the consumer re-fetches current text, so this stays queue-compact. */ ingest?: IngestSourceRef; + /** Opt-in uploads.sh link adoption (issue #701). Same "source ref only, + * consumer re-fetches current text" shape as `ingest` above, so this stays + * queue-compact. */ + adopt?: AdoptSourceRef; /** Write-through for `repoIsPrivate`'s KV cache (issue #631) — present * whenever the delivery's `repository.private` field is a boolean, so a * webhook primes the private-prefix decision flow without an extra GitHub @@ -352,6 +361,19 @@ export function extractWebhookEvent(eventType: string, payload: unknown): Webhoo ev.ingest = { repo: fullName, kind, num: item.number, source: "body" }; } } + + // Adopt gating (issue #701): same opened/edited shape as ingest above, + // just gated on ANY http(s) link rather than specifically a + // github.com/user-attachments one — the consumer re-fetches current + // text and does the real (workspace-scoped) URL resolution. + if (action === "opened" && typeof item.body === "string" && hasLinkCandidate(item.body)) { + ev.adopt = { repo: fullName, kind, num: item.number, source: "body" }; + } else if (action === "edited") { + const changes = p.changes as { body?: unknown } | undefined; + if (changes && "body" in changes) { + ev.adopt = { repo: fullName, kind, num: item.number, source: "body" }; + } + } } } @@ -406,11 +428,25 @@ export function extractWebhookEvent(eventType: string, payload: unknown): Webhoo if (gated) { ev.ingest = { repo, kind, num, source }; } + + // Adopt gating (issue #701): mirrors the ingest gate above, but a + // "deleted" comment has nothing left to (re-)scan for uploads.sh links + // — adoption has no ledger to detach from, so unlike ingest there's no + // work to do on delete. + const hasLinkUrl = typeof body === "string" && hasLinkCandidate(body); + const adoptGated = + (ip.action === "created" && hasLinkUrl) || + (ip.action === "edited" && ip.sender?.type !== "Bot"); + if (adoptGated) { + ev.adopt = { repo, kind, num, source }; + } } } // ping and unknown events fall through with no work. - return ev.keys.length || ev.promote || ev.reconcile || ev.ingest || ev.privacy ? ev : null; + return ev.keys.length || ev.promote || ev.reconcile || ev.ingest || ev.adopt || ev.privacy + ? ev + : null; } /** @@ -450,6 +486,9 @@ export async function processWebhookEvent(env: Env, ev: WebhookEvent): Promise { promote: ev.promote ?? null, reconcile: ev.reconcile ?? null, ingest: ev.ingest ?? null, + adopt: ev.adopt ?? null, error: err instanceof Error ? err.message : String(err), }), ); diff --git a/apps/api/src/repo-comment-config.ts b/apps/api/src/repo-comment-config.ts index 454aaa60..6b5d7651 100644 --- a/apps/api/src/repo-comment-config.ts +++ b/apps/api/src/repo-comment-config.ts @@ -133,6 +133,8 @@ export function workspaceCommentDefaults(ws: WorkspaceRecord): WorkspaceCommentD if (ws.githubCommentNote !== undefined) defaults.note = ws.githubCommentNote; if (ws.githubIngestAttachments !== undefined) defaults.ingestGithubAttachments = ws.githubIngestAttachments; + if (ws.githubAdoptLinkedFiles !== undefined) + defaults.adoptLinkedFiles = ws.githubAdoptLinkedFiles; return defaults; } diff --git a/apps/api/src/routes/me.test.ts b/apps/api/src/routes/me.test.ts index 5e18321a..61ed639b 100644 --- a/apps/api/src/routes/me.test.ts +++ b/apps/api/src/routes/me.test.ts @@ -2315,6 +2315,7 @@ describe("GET /me/workspaces/:name/comment-preview", () => { note: null, ingestGithubAttachments: true, ingestBotAttachments: false, + adoptLinkedFiles: true, }); expect(body.source).toMatchObject({ imageWidth: "auto" }); expect(typeof body.body).toBe("string"); diff --git a/apps/api/src/workspace.ts b/apps/api/src/workspace.ts index dfd6046d..71ad414c 100644 --- a/apps/api/src/workspace.ts +++ b/apps/api/src/workspace.ts @@ -152,6 +152,8 @@ export interface WorkspaceRecord { githubCommentNote?: string; /** Workspace default for the repo `ingestGithubAttachments` knob (issue-spec 2026-08-11). */ githubIngestAttachments?: boolean; + /** Workspace default for the repo `adoptLinkedFiles` knob (issue #701). */ + githubAdoptLinkedFiles?: boolean; /** * Per-workspace opt-out for video poster generation (issue #299). Default * (undefined/true) generates. The surgical kill switch between "all diff --git a/apps/web/src/pages/docs/comment-config.astro b/apps/web/src/pages/docs/comment-config.astro index 58889f4d..4a3dddbe 100644 --- a/apps/web/src/pages/docs/comment-config.astro +++ b/apps/web/src/pages/docs/comment-config.astro @@ -47,6 +47,10 @@ const TOC = [ Whether GitHub-posted attachments are imported, and whether bot-authored ones count. +
  • + Whether uploads.sh links pasted into a PR or comment are adopted into that PR/issue's + attachment context. +

    • Nothing in the file can change where files are hosted or which repo a workspace can post to — @@ -89,6 +93,13 @@ const TOC = [ # Tiny images (under 200px on either side) are always skipped. ingestBotAttachments: false + # Whether uploads.sh file links pasted directly into a PR body or comment + # (bypassing --pr/attach context — a bare paste, or a raw curl upload) get + # adopted into that PR/issue's attachment context. Only links to files + # already in this repo's own bound workspace are adopted; links to any + # other workspace's files are ignored. On by default for bound repos. + adoptLinkedFiles: true + # Optional short markdown at the top of the comment, before the media — # repo-specific context, a link to a contributing guide, and so on. # Trimmed, capped at 500 characters, rendered as-is: it's diff --git a/packages/comment-config/src/index.ts b/packages/comment-config/src/index.ts index 5341fbe0..78ebdf57 100644 --- a/packages/comment-config/src/index.ts +++ b/packages/comment-config/src/index.ts @@ -18,6 +18,7 @@ export interface RepoCommentConfig { note?: string; ingestGithubAttachments?: boolean; ingestBotAttachments?: boolean; + adoptLinkedFiles?: boolean; } export interface WorkspaceCommentDefaults { imageWidth?: "full" | number; @@ -27,6 +28,7 @@ export interface WorkspaceCommentDefaults { note?: string; ingestGithubAttachments?: boolean; ingestBotAttachments?: boolean; + adoptLinkedFiles?: boolean; } export interface ResolvedCommentOptions { imageWidth: "auto" | "full" | number; @@ -37,6 +39,7 @@ export interface ResolvedCommentOptions { note: string | null; ingestGithubAttachments: boolean; ingestBotAttachments: boolean; + adoptLinkedFiles: boolean; } export type OptionSource = "repo" | "workspace" | "auto"; @@ -49,6 +52,9 @@ export const AUTO_COMMENT_OPTIONS: ResolvedCommentOptions = { note: null, ingestGithubAttachments: true, ingestBotAttachments: false, + // Default ON for bound repos (issue #701) — binding is already an + // explicit opt-in relationship, same posture as ingestGithubAttachments. + adoptLinkedFiles: true, }; export const NOTE_MAX_CHARS = 500; @@ -118,6 +124,13 @@ export function parseRepoCommentConfig( else warnings.push(`ingestBotAttachments: expected a boolean; dropped`); } + // adoptLinkedFiles: boolean + if ("adoptLinkedFiles" in c) { + const v = c.adoptLinkedFiles; + if (typeof v === "boolean") config.adoptLinkedFiles = v; + else warnings.push(`adoptLinkedFiles: expected a boolean; dropped`); + } + // meta.path / meta.state: booleans nested under `meta` if ("meta" in c) { const v = c.meta; @@ -174,6 +187,7 @@ export function resolveCommentOptions( ...(ws?.ingestBotAttachments !== undefined ? { ingestBotAttachments: ws.ingestBotAttachments } : {}), + ...(ws?.adoptLinkedFiles !== undefined ? { adoptLinkedFiles: ws.adoptLinkedFiles } : {}), }; const options = { ...AUTO_COMMENT_OPTIONS }; const source = Object.fromEntries( @@ -188,6 +202,7 @@ export function resolveCommentOptions( "linkToFilePage", "ingestGithubAttachments", "ingestBotAttachments", + "adoptLinkedFiles", ] as const) { if (cfg[key] !== undefined && source[key] === "auto") { (options as Record)[key] = cfg[key]; diff --git a/packages/uploads/src/comment-config.generated.ts b/packages/uploads/src/comment-config.generated.ts index 9b575ae8..b5024a00 100644 --- a/packages/uploads/src/comment-config.generated.ts +++ b/packages/uploads/src/comment-config.generated.ts @@ -16,6 +16,7 @@ export interface RepoCommentConfig { note?: string; ingestGithubAttachments?: boolean; ingestBotAttachments?: boolean; + adoptLinkedFiles?: boolean; } export interface WorkspaceCommentDefaults { imageWidth?: "full" | number; @@ -25,6 +26,7 @@ export interface WorkspaceCommentDefaults { note?: string; ingestGithubAttachments?: boolean; ingestBotAttachments?: boolean; + adoptLinkedFiles?: boolean; } export interface ResolvedCommentOptions { imageWidth: "auto" | "full" | number; @@ -35,6 +37,7 @@ export interface ResolvedCommentOptions { note: string | null; ingestGithubAttachments: boolean; ingestBotAttachments: boolean; + adoptLinkedFiles: boolean; } export type OptionSource = "repo" | "workspace" | "auto"; @@ -47,6 +50,9 @@ export const AUTO_COMMENT_OPTIONS: ResolvedCommentOptions = { note: null, ingestGithubAttachments: true, ingestBotAttachments: false, + // Default ON for bound repos (issue #701) — binding is already an + // explicit opt-in relationship, same posture as ingestGithubAttachments. + adoptLinkedFiles: true, }; export const NOTE_MAX_CHARS = 500; @@ -116,6 +122,13 @@ export function parseRepoCommentConfig( else warnings.push(`ingestBotAttachments: expected a boolean; dropped`); } + // adoptLinkedFiles: boolean + if ("adoptLinkedFiles" in c) { + const v = c.adoptLinkedFiles; + if (typeof v === "boolean") config.adoptLinkedFiles = v; + else warnings.push(`adoptLinkedFiles: expected a boolean; dropped`); + } + // meta.path / meta.state: booleans nested under `meta` if ("meta" in c) { const v = c.meta; @@ -172,6 +185,7 @@ export function resolveCommentOptions( ...(ws?.ingestBotAttachments !== undefined ? { ingestBotAttachments: ws.ingestBotAttachments } : {}), + ...(ws?.adoptLinkedFiles !== undefined ? { adoptLinkedFiles: ws.adoptLinkedFiles } : {}), }; const options = { ...AUTO_COMMENT_OPTIONS }; const source = Object.fromEntries( @@ -186,6 +200,7 @@ export function resolveCommentOptions( "linkToFilePage", "ingestGithubAttachments", "ingestBotAttachments", + "adoptLinkedFiles", ] as const) { if (cfg[key] !== undefined && source[key] === "auto") { (options as Record)[key] = cfg[key]; diff --git a/test/fixtures/comment-config-golden.json b/test/fixtures/comment-config-golden.json index 8df456b5..7aaf155a 100644 --- a/test/fixtures/comment-config-golden.json +++ b/test/fixtures/comment-config-golden.json @@ -94,7 +94,7 @@ } }, { - "name": "drops an over-500-char note whole, with a warning — never truncates", + "name": "drops an over-500-char note whole, with a warning \u2014 never truncates", "text": "comment:\n note: \"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\"\n", "format": "yaml", "expected": { @@ -213,6 +213,17 @@ }, "warnings": [] } + }, + { + "name": "adoptLinkedFiles boolean parses; non-boolean dropped with warning", + "text": "comment:\n adoptLinkedFiles: false\n", + "format": "yaml", + "expected": { + "config": { + "adoptLinkedFiles": false + }, + "warnings": [] + } } ], "resolveCases": [ @@ -229,7 +240,8 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": false + "ingestBotAttachments": false, + "adoptLinkedFiles": true }, "source": { "imageWidth": "auto", @@ -239,7 +251,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "auto", - "ingestBotAttachments": "auto" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "auto" } } }, @@ -261,7 +274,8 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": false + "ingestBotAttachments": false, + "adoptLinkedFiles": true }, "source": { "imageWidth": "repo", @@ -271,7 +285,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "auto", - "ingestBotAttachments": "auto" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "auto" } } }, @@ -290,7 +305,8 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": false + "ingestBotAttachments": false, + "adoptLinkedFiles": true }, "source": { "imageWidth": "auto", @@ -300,7 +316,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "auto", - "ingestBotAttachments": "auto" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "auto" } } }, @@ -321,7 +338,8 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": false + "ingestBotAttachments": false, + "adoptLinkedFiles": true }, "source": { "imageWidth": "auto", @@ -331,7 +349,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "auto", - "ingestBotAttachments": "auto" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "auto" } } }, @@ -352,7 +371,8 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": false + "ingestBotAttachments": false, + "adoptLinkedFiles": true }, "source": { "imageWidth": "auto", @@ -362,7 +382,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "repo", - "ingestBotAttachments": "auto" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "auto" } } }, @@ -383,7 +404,41 @@ "linkToFilePage": true, "note": null, "ingestGithubAttachments": true, - "ingestBotAttachments": true + "ingestBotAttachments": true, + "adoptLinkedFiles": true + }, + "source": { + "imageWidth": "auto", + "maxInlineImages": "auto", + "metaPath": "auto", + "metaState": "auto", + "linkToFilePage": "auto", + "note": "auto", + "ingestGithubAttachments": "auto", + "ingestBotAttachments": "repo", + "adoptLinkedFiles": "auto" + } + } + }, + { + "name": "adoptLinkedFiles repo over workspace", + "repo": { + "adoptLinkedFiles": false + }, + "workspace": { + "adoptLinkedFiles": true + }, + "expected": { + "options": { + "imageWidth": "auto", + "maxInlineImages": 16, + "metaPath": true, + "metaState": true, + "linkToFilePage": true, + "note": null, + "ingestGithubAttachments": true, + "ingestBotAttachments": false, + "adoptLinkedFiles": false }, "source": { "imageWidth": "auto", @@ -393,7 +448,8 @@ "linkToFilePage": "auto", "note": "auto", "ingestGithubAttachments": "auto", - "ingestBotAttachments": "repo" + "ingestBotAttachments": "auto", + "adoptLinkedFiles": "repo" } } }