Skip to content

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

Merged
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
claude/708-local-gh-adoption-parity
Aug 18, 2026
Merged

feat(uploads): local-gh fallback parity for link adoption (#708)#710
Zach Dunn (zachdunn) merged 1 commit into
mainfrom
claude/708-local-gh-adoption-parity

Conversation

@zachdunn

Copy link
Copy Markdown
Member

Closes #708.

What

Local-gh fallback parity for issue #701's link adoption (PR #707, bot path only). syncAttachmentsComment — shared by uploads comment, attach, put --comment, and screenshot's comment sync, both CLI and MCP — now scans the PR/issue body and every comment for pasted uploads.sh URLs via local gh api once 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/attach binding (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 from apps/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

  • Bound-workspace files only — enforced by the attach endpoint; a URL that doesn't resolve there throws and is caught per-URL, matching the bot's silent-drop.
  • Copy, never moveattachExisting's own default (move: false).
  • Additive metadata — the endpoint's own preserve contract (PR feat: per-file metadata — PR/issue context, custom tags, and search #157).
  • Idempotent re-adoption — same destination key, same overwrite-in-place contract.
  • No migration into private prefixes — the source URL keeps resolving untouched; only the fresh copy lands wherever the endpoint puts it.
  • Noise guard — a lone adopted link with nothing else already attached doesn't trigger a new comment. Implementation differs slightly from the bot's shouldSyncAfterAdopt: upsertAttachmentsComment's own if (existing) branch already patches an existing managed comment unconditionally (regardless of createIfMissing), 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-new createIfMissing.
  • .uploads.yml's adoptLinkedFiles knob — 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)

  • No per-event ref. The webhook re-scans one specific body/comment text per delivery. The CLI has no such event — 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 later uploads comment run, not just once.
  • Noise-guard baseline is plain-prefix-only. The pre-adoption attachment count used for the guard is a single 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.
  • No structured skip/adopt summary returned to the caller — the webhook's 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 — new describe("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: false in .uploads.yml disables scanning entirely, and the bot-success path never touches gh for adoption.

Changeset (minor, @buildinternet/uploads). pnpm test (305 files / 4536 tests) and pnpm typecheck pass locally.

@changeset-bot

changeset-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ac93d64

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@buildinternet/uploads Minor

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

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (2)
  • coderabbit:review
  • review
🚫 Excluded labels (none allowed) (1)
  • wip

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 298dc7fd-da20-4aa7-86e5-6e9a553a7a75

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zachdunn
Zach Dunn (zachdunn) merged commit b4c96d1 into main Aug 18, 2026
4 checks passed
@zachdunn
Zach Dunn (zachdunn) deleted the claude/708-local-gh-adoption-parity branch August 18, 2026 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Link adoption: local-gh fallback parity

1 participant