Sign up
View stargazers13Sign up

fix(assets): preserve the PR repository for GitHub media#21631

open1
main
batch-07-pr-media/context
opened5d agobyJJulius Marminge
10 files changed
View on GitHubTwo-way sync enabledPR last imported 36m ago

Private GitHub uploads in a PR need the PR's repository context to obtain a signed media URL. The checkout can be a personal fork or an unrelated repository, which fails for Enterprise Managed Users.

Carry optional repository through the asset RPC contract and signed capability, and populate it from the actual PR in web/desktop and mobile. Image/video previews, galleries, refreshes, and media identities retain that context. Older clients and capabilities without the field remain compatible. The next stack layer consumes it for SSO media fetching; this layer changes no media fetch workflow or layout.

Rebuilt from @cbangera2's useful signed-upload work in #19191, incorporating the human EMU context finding.

Bottom layer; followed by #21632.

Independent Opus and Sol reviews completed two full-stack rounds; the final round found no code defects.

Verification: the capability round-trip and existing mobile preview cases went from 44 passing / 3 expected failures to 47 passing; 152 related tests passed. Scoped server/web/mobile/contracts types and targeted lint/format passed, with one existing test warning. No markup or callback-wiring tests were added.

Before/after protocol and runtime proof. The upper layer's full production dev server and actual route composition were exercised over loopback HTTP with mocked GitHub responses; this is not live SAML/EMU or full client UI proof. macOS, Windows, Electron desktop, iOS, Android, native playback, and remote/relay/tunnel execution remain unverified. There is no layout change to capture. Human approval of the authentication-sensitive behavior remains required before merging.

CI/review status: all executed current-head CI jobs passed, including Web, lint, build/types, package tests, all eight server shards, release smoke and the latest fingerprint job. The first Web attempt failed in unchanged AttachmentFilePreview.browser.test.tsx; one failed-job retry passed. An earlier redundant fingerprint run was cancelled and superseded by success. CI run. No unresolved review threads. Macroscope still requires human approval of the private-media context change; this is not a merge-ready claim.

Implemented with GPT-6 Astra and GPT-6.1 Sol on Codex through T3 Code; independently reviewed by Claude Opus 5.5 and GPT-6 Sol.

Checks

All checks passed

Added 1 commit

  • fix(assets): preserve the PR repository for GitHub media2503bd5
MmacroscopeappBot5d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — This bug fix adds repository context to GitHub media requests and signed asset capabilities across web, mobile, contracts, and the server. Because the context affects access to private repository media and authentication credentials, the change warrants human review despite being backward-compatible and well-covered by targeted tests.

You can add or adjust custom eligibility rules. Learn more.

Ggithub-actionsBot5d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire5.0 KiB5.0 KiB+7 B (+0.1%)6.8 KiB✅
CodexThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
CodexLive turn WebSocket wire1.2 KiB1.2 KiB+7 B (+0.6%)2.0 KiB✅
CodexLive turn WebSocket decoded20.9 KiB20.9 KiB0 B (0.0%)29.3 KiB✅
CodexLive turn messages220 (0.0%)8✅
ClaudeTotal thread wire5.0 KiB5.0 KiB0 B (0.0%)6.8 KiB✅
ClaudeThread snapshot wire3.8 KiB3.8 KiB0 B (0.0%)4.9 KiB✅
ClaudeLive turn WebSocket wire1.2 KiB1.2 KiB0 B (0.0%)2.0 KiB✅
ClaudeLive turn WebSocket decoded21.2 KiB21.2 KiB0 B (0.0%)29.3 KiB✅
ClaudeLive turn messages220 (0.0%)8✅

Baseline: 1451e7c · PR result: 2503bd5 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

CcoderabbitaiBot5d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 1eb693bb-eef9-45af-9f4d-bb3b5c717d46
📥 Commits

Reviewing files that changed from the base of the PR and between 1451e7cb23034d29774338608368a15d8845df36 and 2503bd508df4fa5f827f6c23cef252fc65596134.

📒 Files selected for processing: 10
  • apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx
  • apps/mobile/src/features/pull-requests/PullRequestMedia.tsx
  • apps/mobile/src/features/pull-requests/pullRequestMediaSource.test.ts
  • apps/mobile/src/features/pull-requests/pullRequestMediaSource.ts
  • apps/mobile/src/lib/videoPreviewSource.ts
  • apps/server/src/assets/AssetAccess.test.ts
  • apps/server/src/assets/AssetAccess.ts
  • apps/web/src/components/ChatMarkdown.tsx
  • apps/web/src/components/pullRequest/PullRequestMarkdown.tsx
  • packages/contracts/src/assets.ts

Limit details: You’ve used all 10 included reviews currently available.

📝 Walkthrough

GitHub media resources now carry optional pull request repository context through asset capabilities. Web and mobile media rendering pass that context into resource creation. Mobile image and video cache identifiers use the serialized media source.

Changes: Pull request media context

Layer / File(s)Summary
Repository-aware asset capabilities
packages/contracts/src/assets.ts, apps/server/src/assets/AssetAccess.ts, apps/server/src/assets/AssetAccess.test.ts		The GitHub media resource accepts an optional repository value. Asset capability issuance and resolution preserve it when present. A test checks that it survives decoding, issuance, and resolution.
Web pull request media context
apps/web/src/components/pullRequest/PullRequestMarkdown.tsx, apps/web/src/components/ChatMarkdown.tsx		Pull request markdown derives a repository name from the context URL and passes it to markdown image and video rendering. GitHub media resources include the repository when available.
Mobile media context and source identity
apps/mobile/src/features/pull-requests/*, apps/mobile/src/lib/videoPreviewSource.ts		Mobile pull request media passes repository context to the shared source builder. Tests check repository values in generated resources. Image and video cache identifiers use the serialized media source.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to 2503b

The repository context is carried through the reviewed media paths, with no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely states the main change: preserving the pull request repository for GitHub media.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Description checkPassedThe description clearly explains the problem, implementation, compatibility behavior, verification results, limitations, CI status, and approval requirement. It does not use the template headings, and…
Full details: Description check

Explanation

The description clearly explains the problem, implementation, compatibility behavior, verification results, limitations, CI status, and approval requirement. It does not use the template headings, and it does not provide a direct maintainer approval link or approval comment for the authentication-sensitive scope, but the required information is mostly present.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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