Skip to content

fix(server): private repository attachments load in pull requests - #23

Open
tebiggs wants to merge 1 commit into
mainfrom
fix/mobile-private-pr-attachments
Open

tebiggs wants to merge 1 commit into
mainfrom
fix/mobile-private-pr-attachments

Conversation

@tebiggs

@tebiggs tebiggs commented Nov 1, 2026

Copy link
Copy Markdown
Owner

Image attachments in a private repository's pull request showed up as blank tiles on Android, and as broken images on web. The URL in the description, github.com/user-attachments/assets/<id>, answers 404 to any token, so the server's media fetch had nothing to return.

The fix

When that URL answers 404, the GitHub driver calls POST /markdown with mode: gfm and context: <owner/name>. GitHub's HTML then holds a signed private-user-images.githubusercontent.com link, which the server fetches with no credential. The server caches the link for a minute because GitHub signs it for five, so a video's range requests don't each spend quota. Clients never see it.

  • Why /markdown and not body_html: it needs no issue or comment id, so descriptions, comments and guide text all work through one path.
  • Contract change: the media asset resource gets an optional repository, because the project at cwd may not be the pull request's repository. Mobile and web both send it.
  • Public repositories: their URL still redirects to the object store, so they make no extra call. A test asserts this.
  • Bad links: a rendered link is only followed if it is on the signed host and its file name carries the attachment id. Anything else keeps the original 404.

The kind: "video" in gitHubMediaUrl was not the cause. It only applies to a URL alone on a line, and ![alt](url) stays an image.

Surfaces

  • Covered, same path: mobile description, timeline, discussion and guide; web summary, timeline, review comments and guide.
  • Both sides must update: an old client on a new server, or a new client on an old server, keeps today's blank tiles but nothing else breaks.

Not covered or not tested

  • No Android screen capture. Agent device access is off in the session that did this, so nothing has been seen on a device or emulator. The APK below is for that check.
  • Accounts that aren't the owner. Measured with the owner's fine-grained token: a repository context the account owns gets a signed link, whether private or public; pingdotgg/t3code and cli/cli do not. T3's GitHub App sign-in and a read-only collaborator are untested. If GitHub gives no signed link, the tile stays blank as it does today.
  • Legacy /<owner>/<repo>/assets/<n>/<id> URLs. They are unchanged, as no private sample turned up to test against.
  • Bare private videos. They go through the same path, but no sample turned up, and the link parser only reads <img> tags.
  • <img> tags in the mobile Attachments row. GitHub's web composer can write attachments as <img> HTML, and that row skips HTML nodes. This is from reading the code, it applies to public repositories too, and this PR leaves it alone.

Verification

The private repository's details stay out of this public PR, so the screenshot is behind Access.

Before the fix, on the box against a private repository's pull request:

curl <attachment url>                      -> 404
curl -H 'Authorization: Bearer ...' <url>  -> 404

After the fix, on a dev server at 05fa731 with the web client open on that pull request, all 17 attachments came through the asset route:

200 image/png, 193736 to 399771 bytes, first image decoded at 688x1764

Screenshot: https://artifacts.ghostsignal.nexus/k/t3code-private-pr-attachments-web

APK at 05fa731 for the phone check, which also needs a server with this change: https://artifacts.ghostsignal.nexus/k/t3code-private-pr-attachments

  • The new test failed first on the bug (expected 206, got 404). gitHubMediaFetch, definition, AssetAccess, pullRequestMediaSource, useMediaVideoPlayback, assetImageSource and the PullRequestMarkdown browser test all pass.
  • Typecheck passes for contracts, source-control-core, source-control-github, server, mobile and web. Format, lint and knip pass.
  • vp run check:changed did not complete: git diff hit SIGSEGV in the boat sandbox on both tries, before any check ran. I ran its parts separately instead, and the fork has no pull request CI to back this up.

Done with Claude Opus 5.5 in Claude Code, running in T3 Code.

🤖 Generated with Claude Code

A private repository's `github.com/user-attachments/assets/<id>` URL is a 404
to any token, so the media fetch came back empty and clients drew blank tiles.
When that URL is a 404, ask GitHub to render it in the change request's
repository, which yields a short-lived signed link, and fetch that instead.
Mobile and web now send the repository with the media resource.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant