feat(uploads): local-gh fallback parity for link adoption (#708)

.changeset/gh-fallback-link-adoption.md

@@ -0,0 +1,5 @@
+---
+"@buildinternet/uploads": minor
+---
+
+Link adoption (`adoptLinkedFiles`, issue #701) now also runs on the local-`gh` fallback comment path, not just the bot-managed one. `uploads comment --pr`/`--issue` (and the attach-time comment sync) scans the PR/issue body and comments for pasted uploads.sh URLs via local `gh` and adopts any that resolve to a file in the current workspace, before rendering the comment. Same semantics as the bot path: copy never move, additive metadata, idempotent re-adoption, no migration into private prefixes, and a lone adopted image with nothing else to consolidate doesn't trigger a fresh comment on its own (it still heals an existing one).

packages/uploads/src/commands.ts

@@ -69,6 +69,9 @@ import {
   timedExecRunner,
   ghMetadataFromTargetWithTitle,
   upsertAttachmentsComment,
+  hasLinkCandidate,
+  extractCandidateUrls,
+  fetchAdoptionCandidateText,
   type CommandRunner,
 } from "./github-gh.js";
 import { deriveRepoFromGit, deriveRepoSlugFromGit } from "./keys.js";
@@ -833,6 +836,58 @@ export async function syncAttachmentsComment(
     }
   }
 
+  // Link adoption (issue #708): local-gh fallback parity with the bot's own
+  // adoption (issue #701, apps/api/src/github-link-adopt.ts). When the bot
+  // already handled this target the server already adopted for us, so this
+  // only runs once we've fallen through to gh. Scans the PR/issue body and
+  // every comment for pasted uploads.sh URLs and adopts each one that
+  // resolves (server-side, inside `POST .../github/attach`) to a file in
+  // THIS workspace's own bound-repo attachment prefix — copy, never move,
+  // same as the bot path. Best-effort end to end: any failure (not a git
+  // repo, `gh` unavailable/unauthenticated, config unreadable) degrades to
+  // "adopt nothing" rather than blocking the comment sync it rides along
+  // with.
+  let adoptedCount = 0;
+  let preAdoptionAttachmentCount: number | undefined;
+  try {
+    const root = run("git", ["rev-parse", "--show-toplevel"]).trim();
+    const { config: adoptConfig } = readLocalRepoCommentConfig(root);
+    const { options: adoptOptions } = resolveCommentOptions(adoptConfig, null);
+    if (adoptOptions.adoptLinkedFiles) {
+      const text = fetchAdoptionCandidateText(target, run);
+      if (hasLinkCandidate(text)) {
+        const urls = extractCandidateUrls(text);
+        if (urls.length > 0) {
+          // Baseline BEFORE this pass's adoptions land (mirrors the bot
+          // path's `gatherCommentBody` call before its own copies) — feeds
+          // the noise guard below without a lone adoption inflating its own
+          // count. Plain prefix only (not the private-prefix listing done
+          // for the final render below) — good enough for a guard decision.
+          preAdoptionAttachmentCount = (await client.listAll({ prefix: ghKeyPrefix(target) }))
+            .length;
+          for (const url of urls) {
+            try {
+              await client.attachExisting({
+                source: url,
+                repo: target.repo,
+                ...(target.kind === "pull" ? { pr: target.num } : { issue: target.num }),
+              });
+              adoptedCount++;
+            } catch {
+              // Not a resolvable uploads.sh URL, belongs to a different
+              // workspace, or the source was deleted — silently dropped,
+              // matching the bot path's contract (a throw from
+              // `resolveAttachSourceKey` is caught per-URL there too).
+            }
+          }
+        }
+      }
+    }
+  } catch {
+    // Not a git repo, `.uploads.yml` unreadable, or `gh` unavailable for the
+    // PR/comments fetch — degrade to no adoption this pass.
+  }
+
   // gh fallback: gather from this workspace's own data and post via local `gh`.
   // Note (issues #304, #365): this CLI process has no server-side
   // WorkspaceRecord in scope, so it cannot honor a workspace's
@@ -939,11 +994,21 @@ export async function syncAttachmentsComment(
   // bot identity, so this note would be wrong there.
   const body = `${attachmentsCommentBody(items, previewGalleries, marker, renderOptions, target)}\n${GH_FALLBACK_AUTHOR_NOTE}`;
   const count = items.length + previewGalleries.length;
+  // Noise guard (issue #708, mirrors the bot's `shouldSyncAfterAdopt`): a
+  // lone adopted link with nothing else already attached is already fully
+  // visible inline in the PR/comment — don't create a brand-new comment just
+  // to repeat it. `upsertAttachmentsComment` still PATCHes an existing
+  // managed comment unconditionally (its own `if (existing)` branch runs
+  // regardless of `createIfMissing`), so "a managed comment already exists"
+  // and "other attachments are already present" both heal/sync for free
+  // here without any extra condition — this only ever suppresses a fresh
+  // create.
+  const skipLoneAdoptionCreate = adoptedCount === 1 && (preAdoptionAttachmentCount ?? 0) === 0;
   // Empty (count 0) renders the neutral empty-state body but must not create a
   // comment — it only rewrites one that already exists (`action: "skipped"`
   // when none does).
   const { action } = upsertAttachmentsComment(target, body, run, marker, {
-    createIfMissing: count > 0,
+    createIfMissing: count > 0 && !skipLoneAdoptionCreate,
   });
   return { action, count, via: "gh" };
 }

packages/uploads/src/github-gh.ts

@@ -402,6 +402,74 @@ function reconcileAfterCreate(
   }
 }
 
+// --- link adoption (issue #708, local-gh fallback parity with the bot's
+// #701/apps/api/src/github-link-adopt.ts) ---
+
+/** Cheap, regex-only reject: no http(s) URL at all means nothing to scan for.
+ * Mirrors `hasLinkCandidate` in apps/api/src/github-link-adopt.ts. */
+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. Ported
+ * verbatim from apps/api/src/github-link-adopt.ts's `extractCandidateUrls`
+ * so the two paths recognize the same URL spellings — resolution itself
+ * (storage host / embed host / `/f/` page → key, and the bound-workspace
+ * check) happens server-side inside `POST .../github/attach`, so this file
+ * doesn't need its own copy of that logic.
+ */
+export function extractCandidateUrls(text: string): string[] {
+  const seen = new Set<string>();
+  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;
+}
+
+/**
+ * Best-effort concatenation of a PR/issue's current body plus every comment's
+ * current body, for link-adoption scanning. Returns "" on any `gh` failure
+ * (not authenticated, network, repo/number not found) — adoption degrades to
+ * a no-op rather than blocking the comment sync it rides along with.
+ *
+ * Unlike the webhook path (which re-scans one specific body/comment ref per
+ * event), the CLI has no per-event ref to key off of — `uploads comment` is
+ * one ad-hoc invocation — so this scans the PR/issue body and every comment
+ * on the thread in one pass every time it runs. That's a documented
+ * divergence: harmless (adoption is idempotent) but does mean a link posted
+ * in comment #1 gets rescanned on every later `uploads comment` run too.
+ */
+export function fetchAdoptionCandidateText(target: GhTarget, run: CommandRunner): string {
+  let body = "";
+  try {
+    body = run("gh", ["api", `repos/${target.repo}/issues/${target.num}`, "--jq", '.body // ""']);
+  } catch {
+    // Not found / no access — fall through with an empty body; comments may
+    // still be readable.
+  }
+  let comments = "";
+  try {
+    comments = run("gh", [
+      "api",
+      `repos/${target.repo}/issues/${target.num}/comments?per_page=100`,
+      "--paginate",
+      "--jq",
+      '[.[].body] | join("\\n")',
+    ]);
+  } catch {
+    // Same degrade — an empty comments blob just means nothing more to scan.
+  }
+  return `${body}\n${comments}`;
+}
+
 /** PATCH one comment's body via stdin, so the body is never shell-interpolated. */
 function patchComment(target: GhTarget, run: CommandRunner, id: number, body: string): void {
   run(

packages/uploads/test/commands-comment.test.ts

@@ -468,6 +468,216 @@ describe("syncAttachmentsComment", () => {
     expect(calls.some((c) => c.args.includes("repos/acme/web/issues/12/comments"))).toBe(false);
   });
 
+  describe("link adoption (issue #708, local-gh fallback parity)", () => {
+    /**
+     * gh+git runner for adoption tests: `git rev-parse --show-toplevel`
+     * resolves to a scratch dir (no `.uploads.yml`, so the knob's default
+     * "on" applies), the issue/PR body call returns `prBody`, the comments
+     * call returns `[]` (no comments to scan), the marker hunt finds no
+     * existing comment, and any create/patch just records its body.
+     */
+    function adoptionRunner(prBody: string): { run: CommandRunner; calls: string[][] } {
+      const dir = fs.mkdtempSync(path.join(os.tmpdir(), "uploads-commands-comment-adopt-"));
+      const calls: string[][] = [];
+      const run: CommandRunner = (cmd, args) => {
+        calls.push([cmd, ...args]);
+        if (cmd === "git" && args.includes("--show-toplevel")) return `${dir}\n`;
+        if (cmd !== "gh") throw new Error(`unexpected command: ${cmd}`);
+        if (args[0] === "api" && args[1]?.startsWith("repos/") && args[1]?.includes("/comments?")) {
+          return "[]";
+        }
+        if (args[0] === "api" && args[1]?.match(/^repos\/[^/]+\/[^/]+\/issues\/\d+$/)) {
+          return prBody;
+        }
+        if (args[1]?.includes("per_page=100")) return "[]"; // marker hunt: no existing comment
+        return JSON.stringify({ id: 9 }); // create
+      };
+      return { run, calls };
+    }
+
+    it("adopts a pasted uploads.sh URL and syncs when other attachments already exist", async () => {
+      const { run } = adoptionRunner("see https://uploads.sh/f/acme/other-workspace-image.png");
+      const attachExisting = vi.fn(async () => ({
+        key: "gh/acme/web/pull/12/other-workspace-image.png",
+        url: "u",
+        embedUrl: null,
+        moved: false,
+        source: { key: "f/acme/other-workspace-image.png" },
+        comment: { posted: false, reason: "not_installed" },
+      }));
+      const client = fakeClient({
+        attachExisting,
+        listAll: async () => [{ key: "gh/acme/web/pull/12/a.png", url: "u", embedUrl: null }],
+        findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }),
+      } as never);
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      expect(attachExisting).toHaveBeenCalledWith({
+        source: "https://uploads.sh/f/acme/other-workspace-image.png",
+        repo: "acme/web",
+        pr: 12,
+      });
+      expect(res.via).toBe("gh");
+      expect(res.action).toBe("created");
+    });
+
+    it("does not create a comment for a lone adoption with nothing else attached (noise guard)", async () => {
+      const { run } = adoptionRunner("see https://uploads.sh/f/acme/solo.png");
+      const attachExisting = vi.fn(async () => ({
+        key: "gh/acme/web/pull/12/solo.png",
+        url: "u",
+        embedUrl: null,
+        moved: false,
+        source: { key: "f/acme/solo.png" },
+        comment: { posted: false, reason: "not_installed" },
+      }));
+      // First listAll() (baseline, before adoption) reports empty; the second
+      // listAll() (post-adoption gather for rendering) reports the newly
+      // adopted file — mirroring how a real workspace would look either side
+      // of the copy this pass makes.
+      let listCalls = 0;
+      const client = fakeClient({
+        attachExisting,
+        listAll: async () => {
+          listCalls++;
+          return listCalls === 1
+            ? []
+            : [{ key: "gh/acme/web/pull/12/solo.png", url: "u", embedUrl: null }];
+        },
+        findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }),
+      } as never);
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      expect(attachExisting).toHaveBeenCalledTimes(1);
+      // Copy still happened, but no NEW comment was created for it.
+      expect(res.action).toBe("skipped");
+    });
+
+    it("heals an existing managed comment even for a lone adoption", async () => {
+      const dir = fs.mkdtempSync(path.join(os.tmpdir(), "uploads-commands-comment-adopt-"));
+      const calls: string[][] = [];
+      const run: CommandRunner = (cmd, args) => {
+        calls.push([cmd, ...args]);
+        if (cmd === "git" && args.includes("--show-toplevel")) return `${dir}\n`;
+        if (cmd !== "gh") throw new Error(`unexpected command: ${cmd}`);
+        // The adoption comments-scan and the marker hunt share the same URL
+        // shape (`.../comments?per_page=100`) — the adoption call is the one
+        // carrying `--jq`, so check that first.
+        if (args[0] === "api" && args[1]?.includes("/comments?") && args.includes("--jq")) {
+          return "[]"; // no comments to scan for links
+        }
+        if (args[0] === "api" && args[1]?.match(/^repos\/[^/]+\/[^/]+\/issues\/\d+$/)) {
+          return "see https://uploads.sh/f/acme/solo.png";
+        }
+        if (args[1]?.includes("per_page=100")) {
+          // The marker hunt (no --jq) finds an existing comment.
+          return JSON.stringify([{ id: 7, body: `${ATTACHMENTS_MARKER}\nold` }]);
+        }
+        return JSON.stringify({ id: 7 });
+      };
+      const attachExisting = vi.fn(async () => ({
+        key: "gh/acme/web/pull/12/solo.png",
+        url: "u",
+        embedUrl: null,
+        moved: false,
+        source: { key: "f/acme/solo.png" },
+        comment: { posted: false, reason: "not_installed" },
+      }));
+      // Baseline (before adoption) is empty — the noise guard alone would
+      // suppress a fresh create — but a managed comment already exists, so
+      // it must still heal via PATCH regardless of `createIfMissing`.
+      let listCalls = 0;
+      const client = fakeClient({
+        attachExisting,
+        listAll: async () => {
+          listCalls++;
+          return listCalls === 1
+            ? []
+            : [{ key: "gh/acme/web/pull/12/solo.png", url: "u", embedUrl: null }];
+        },
+        findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }),
+      } as never);
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      expect(attachExisting).toHaveBeenCalledTimes(1);
+      expect(res.action).toBe("updated");
+      expect(calls.some((c) => c[0] === "gh" && c.includes("PATCH"))).toBe(true);
+    });
+
+    it("skips an unresolvable candidate URL without failing the sync", async () => {
+      const { run } = adoptionRunner("see https://example.com/not-uploads.png");
+      const attachExisting = vi.fn(async () => {
+        throw new Error("source_not_found");
+      });
+      const client = fakeClient({
+        attachExisting,
+        listAll: async () => [],
+        findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }),
+      } as never);
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      // A non-uploads.sh URL is still a "candidate" by the cheap regex, so
+      // attachExisting is called and this asserts the per-URL catch: the
+      // failure is swallowed, not thrown.
+      expect(attachExisting).toHaveBeenCalled();
+      expect(res.action).toBe("skipped");
+    });
+
+    it("honors adoptLinkedFiles: false in .uploads.yml", async () => {
+      const dir = fs.mkdtempSync(path.join(os.tmpdir(), "uploads-commands-comment-adopt-off-"));
+      fs.writeFileSync(path.join(dir, ".uploads.yml"), "comment:\n  adoptLinkedFiles: false\n");
+      const run: CommandRunner = (cmd, args) => {
+        if (cmd === "git" && args.includes("--show-toplevel")) return `${dir}\n`;
+        if (cmd !== "gh") throw new Error(`unexpected command: ${cmd}`);
+        if (args[1]?.includes("per_page=100")) return "[]";
+        return JSON.stringify({ id: 9 });
+      };
+      const attachExisting = vi.fn();
+      const client = fakeClient({
+        attachExisting,
+        listAll: async () => [],
+        findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }),
+      } as never);
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      expect(attachExisting).not.toHaveBeenCalled();
+      expect(res.action).toBe("skipped");
+    });
+
+    it("does not scan for adoption when the bot posts successfully", async () => {
+      const attachExisting = vi.fn();
+      const client = fakeClient({
+        attachExisting,
+        upsertGithubComment: async () => ({ posted: true, action: "created", count: 1 }),
+      } as never);
+      const run: CommandRunner = () => {
+        throw new Error("gh must not be called on the bot success path");
+      };
+      const res = await syncAttachmentsComment(
+        client,
+        { repo: "acme/web", num: 12, kind: "pull" },
+        run,
+      );
+      expect(res.via).toBe("bot");
+      expect(attachExisting).not.toHaveBeenCalled();
+    });
+  });
+
   describe("repo comment config (.uploads.yml)", () => {
     let dirs: string[] = [];