feat(uploads): local-gh fallback parity for link adoption (#708) - #710
Conversation
🦋 Changeset detectedLatest commit: ac93d64 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closes #708.
What
Local-
ghfallback parity for issue #701's link adoption (PR #707, bot path only).syncAttachmentsComment— shared byuploads comment,attach,put --comment, andscreenshot's comment sync, both CLI and MCP — now scans the PR/issue body and every comment for pasted uploads.sh URLs via localgh apionce it has fallen through to the gh path (the bot, when it succeeds, already did adoption server-side, so this never double-scans).Each candidate URL is adopted via the existing
POST /v1/workspaces/:ws/github/attachbinding (client.attachExisting, from #706/#702) — that endpoint does the URL→key resolution (storage host, embed host,/f/page) and the bound-workspace check server-side, so the CLI doesn't need its own copy of that logic; it just extracts candidate URLs (extractCandidateUrls, ported verbatim fromapps/api/src/github-link-adopt.ts) and lets the endpoint accept or reject each one.New in
packages/uploads/src/github-gh.ts:hasLinkCandidate,extractCandidateUrls,fetchAdoptionCandidateText.Semantics matched from the webhook path
attachExisting's own default (move: false).shouldSyncAfterAdopt:upsertAttachmentsComment's ownif (existing)branch already patches an existing managed comment unconditionally (regardless ofcreateIfMissing), so "a managed comment already exists" and "other attachments already present" both heal/sync for free without any extra condition here — the guard only ever has to suppress a brand-newcreateIfMissing..uploads.yml'sadoptLinkedFilesknob — already plumbed CLI-side (comment-config.generated.ts); now actually consulted by the fallback path.Deviations from the webhook path (unavoidable given the CLI's shape)
uploads comment --pr <n>is one ad-hoc invocation — so it scans the PR/issue body and every comment on the thread in one pass, every time it runs. Harmless (adoption is idempotent) but means a link posted in comment Add list and revoke endpoints to admin token API #1 gets rescanned on every lateruploads commentrun, not just once.listAll({ prefix: ghKeyPrefix(target) })— it doesn't also enumerate the active private-prefix listing the final render does (issue feat: randomized URL prefix for private-repo attachments (capability URLs) #631). Good enough for a guard decision (worst case: one extra comment create in an edge case involving a private-prefixed repo with a lone newly-adopted link), not worth a second network round trip just for precision here.AdoptSummary(adopted/skipped/synced) has no CLI equivalent; failures are swallowed the same way but nothing is surfaced beyond the final comment action.Tests
packages/uploads/test/commands-comment.test.ts— newdescribe("link adoption …")block: adopts + syncs when other attachments exist, noise-guard suppresses a lone adoption's fresh create, heals an existing managed comment even for a lone adoption, an unresolvable/cross-workspace URL is skipped without failing the sync,adoptLinkedFiles: falsein.uploads.ymldisables scanning entirely, and the bot-success path never touchesghfor adoption.Changeset (minor,
@buildinternet/uploads).pnpm test(305 files / 4536 tests) andpnpm typecheckpass locally.