Skip to content

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

Merged
juliusmarminge merged 5 commits into
mainfrom
batch-07-pr-media/context
Nov 5, 2026
Merged

juliusmarminge merged 5 commits into
mainfrom
batch-07-pr-media/context

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

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.

Takeover

Taken over from @juliusmarminge's original commit d104aff; that commit is kept and the takeover commits sit on top.

  • The server now signs every decoded field of the resource into the capability instead of copying fields one by one. ResolvedAsset.repository is unchanged for https://git.cafe/pingdotgg/t3code/pulls/21632.
  • Web reads the repository from PullRequestDetail.repository (the repository the server fetched the PR from) instead of re-parsing the PR URL. Mobile reads it from the route's PR ref.
  • Dropped: the JSON.stringify(source) mobile image/video identity keys (back to the URL, since the bytes at a URL are the same in any repository context), and repository in the web video key and in ChatMarkdown's media scope key.
  • Tests: one server round-trip (RPC decode → signed capability → resolve), the mobile gallery test, and one new web test that checks both PR-body image and video asset requests carry the PR's repository. Each fails with the fix reverted.

Evidence

Head 597f1ae (after) vs merge-base 15707a8 (before). Environment: VM fs-11, Linux x86_64, Chromium 156, no device emulation, DPR 2, font scaling 100%, CSS viewports 1440×1000 / 900×1000 / 390×1000, UI clock fixed at 2026-11-03T12:00Z. Each still matched a second capture taken 500 ms later. Data is synthetic: fresh T3CODE_DEV_SEED=0 servers, a synthetic PR-detail and media-byte proxy, and the live assets.createUrl RPC. Mobile ran in a labelled React-DOM harness with native views stubbed.

Row What Before → After Diff0 / DiffT (>8/255)
Wide 1440 dark PR body: GitHub images and video sheet 0 px / 0 px
Wide 1440 light same sheet 0 px / 0 px
Narrow 900 light same sheet 0 px / 0 px
Narrow 390 light / dark same light · dark 0 px / 0 px
W2 fullscreen video the player at full screen W2 0 px / 0 px
W1/W2/W8 wire assets.createUrl payloads before no repository → after with the PR's repository —
W9 control branches relative, workspace and host-file media, audio, file/context chips W9: unchanged, no repository 0 px / 0 px
W10 environment switch E1→E2→E1 across two real paired dev servers, real asset hooks and RPC W10: each assets.createUrl goes to the selected environment; head carries the PR's repository and caches per environment and repository diagnostic text only
M1–M6 mobile wrappers, gallery and paging, video, thumbnail keys, legacy M2, others in the full evidence harness diagnostic text only
S1–S4 server signed round-trip, base-minted capabilities on head, tamper rejection, unchanged fetch head runtime · legacy upgrade —

Wide 1440 dark: Before, After, Diff0 and DiffT

What else changed:

  • In settled stills: nothing at zero threshold.
  • Rows W10 and M2: only the on-page repository diagnostic text the capture adds, (absent) → the PR's repository.
  • Recordings: short media-loading intervals, video clock frames, cursor and tooltips, and capture-start offsets differ between runs.

Full evidence with every row, recordings and narrated steps: PR-Evidence-round3.md · fixture matrix: matrix.md · seed and driver scripts: REBUILD.md, Scripts-round2.md, artifact manifest.

Verification log
  • Tests: AssetAccess.test.ts 37/37, pullRequestMediaSource.test.ts 1/1, PullRequestMarkdown.browser.test.tsx 9/9. All three new or rewritten tests fail with the fix reverted. vp run check:changed passed every step (format, lint, knip, typecheck of 36 packages, tests next to the changes, React compiler).
  • Review round 1: Sol (Codex) MERGE and Opus (Claude) MERGE, no findings. A runtime check confirmed that a capability whose repository was altered without re-signing is rejected. A new client against an older server still works because the unknown key is dropped.
  • Proof audit: pass 1 derived the 25-row matrix. Pass 2 found W2, W9, W10 and M2 missing; they were recaptured. W10 was first captured with stubbed asset hooks, which the auditor rejected, and was then recaptured against two real environments. Four stale W9 stills are excluded; W9 stands on its raw transcript. Final verdict: SUFFICIENT. An independent rebuild of the wide-dark capture at the head differed by 0 of 1,137,920 pixels.
  • CI on 597f1ae: green (Build and Typecheck, Check +14, Native fingerprint diff, Agent context budget).
  • Unverified: live GitHub EMU/SAML fetch (this layer doesn't fetch; https://git.cafe/pingdotgg/t3code/pulls/21632 does), native iOS and Android rendering and playback, Electron (optional row W11).
  • Takeover by Claude Opus 5.5 (lead, tests, implementation, review B) and GPT-6.1 Sol (review A, proof audit, captures) on T3 Code.

🤖 Generated with Claude Code

@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Oct 19, 2026
@juliusmarminge juliusmarminge changed the title batch 07 pr media/context fix(assets): preserve the PR repository for GitHub media Oct 19, 2026
@github-actions github-actions Bot added the size:M 30-99 changed lines (additions + deletions). label Oct 19, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 19, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 597f1ae

Macroscope's review found this PR approvable — Preserves PR repository context through web and mobile media resources and signed asset tokens, including thumbnail cache identity. Existing media validation and access safeguards remain unchanged, with no material change to permissions or product defaults.

No code changes detected at 7c251d7. Prior analysis still applies.

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

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 37481f0 · PR result: 7c251d7 · 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.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Pull request media resources now accept optional repository context. Web and mobile pull request rendering pass that context into media resources. The server preserves it in signed asset claims and resolved assets. Mobile image and video identities 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/*, apps/web/src/components/ChatMarkdown.tsx
Pull request detail supplies a repository name to Markdown context. Markdown image and video rendering pass it into media resources. A browser test checks the repository on image and video resource requests.
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 identities use the serialized media source, and the thumbnail key includes the repository.

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

Change: Bug fix

Possibly related issues

  • pingdotgg/t3code issue 26663: This issue concerns mobile pull-request description images that bypass authenticated media loading when inline. This change passes the pull request repository into mobile media resources used by that loading path.
  • pingdotgg/t3code issue 26662: This change preserves the pull request repository in mobile gallery asset resources. The reported gallery-wide preparation delay is not addressed here.

Suggested reviewers: shivamhwp, t3dotgg

Merge Risk: Minimal · up to 7c251

No merge-blocking issue was identified. Native rendering and remote connection modes remain unverified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: preserving the pull request repository for GitHub media.
Description check Passed The description is detailed and covers the problem, implementation, scope, approval requirement, verification results, evidence, and known limitations. It is more complete than the required template, …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and covers the problem, implementation, scope, approval requirement, verification results, evidence, and known limitations. It is more complete than the required template, despite not using every template heading explicitly.

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
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 597f1ae and 7c251d7.

Files selected for processing: 5

  • apps/mobile/src/features/pull-requests/PullRequestMarkdown.tsx
  • 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

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

@gitcafe-production
gitcafe-production Bot force-pushed the batch-07-pr-media/context branch from 2503bd5 to d104aff Compare November 3, 2026 19:08
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Nov 3, 2026
@juliusmarminge juliusmarminge added the takeover t3 automation: takeover label Nov 3, 2026 — with GitCafe Production
@juliusmarminge juliusmarminge added the pr:in-progress A lead is working on this PR label Nov 3, 2026

Copy link
Copy Markdown
Member Author

Taking over this one

juliusmarminge and others added 3 commits November 3, 2026 19:29
…21631)

These are the takeover's tests for #21631. They prove that a pull request's
repository survives RPC decoding, the signed capability and resolveAsset on the
server, and that the web PR body and mobile PR gallery ask for host media with
that repository. A request without one still resolves without it.

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

Read the repository from the pull request the server already resolved
(PullRequestDetail.repository on web, the route's PR ref on mobile) instead of
reparsing the PR URL, sign every resource field into the capability with a rest
spread, and keep URL-based identity keys on mobile media.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@macroscopeapp
macroscopeapp Bot dismissed their stale review November 3, 2026 19:54

Dismissing prior approval to re-evaluate 597f1ae

@juliusmarminge juliusmarminge removed the pr:in-progress A lead is working on this PR label Nov 3, 2026
…text

# Conflicts:
#	apps/web/src/components/pullRequest/PullRequestMarkdown.tsx
@juliusmarminge
juliusmarminge merged commit e4b98f9 into main Nov 5, 2026
33 checks passed
@juliusmarminge
juliusmarminge deleted the batch-07-pr-media/context branch November 5, 2026 19:58
Pipyakas pushed a commit to Pipyakas/t3code that referenced this pull request Nov 5, 2026
…1631)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit e4b98f9)
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Nov 6, 2026
**v0.1.8-nightly.20261106.4247** · 67 changes from 19 contributors

- **Environment dictation.** Choose an environment for dictation in web and desktop, including desktop browser annotations.
- **Fresh sessions.** Use /clear to start a fresh session without leaving your thread.
- **Scheduled task sessions.** Run scheduled tasks in one thread, either continuing the conversation or starting fresh each time.
- **Secret requests.** Answer secret requests using a secret provider instead of copying and pasting credentials.
- **Snooze questions.** Snooze a thread with a pending question and answer it later.
- **Claude Ultracode.** Use Ultracode at any effort level, including with Claude Sonnet 5.5 and Haiku 5.5.
- **Faster pull requests.** Pull requests open faster using data already available in the app.

### Plus 31 more bug fixes and performance improvements.

<details>
<summary>Full changelog</summary>

## What's Changed
* fix(threads): side chats inherit parent permissions by default by @extoci in pingdotgg/t3code#31114
* fix(server): threads follow their worktree's branch changes by @Yash-Singh1 in pingdotgg/t3code#31599
* fix(pi): usage reports include sessions stored in a --session-dir launch argument by @saphid in pingdotgg/t3code#30690
* perf(mobile): skip unchanged terminal toolbar fade updates by @saphid in pingdotgg/t3code#25312
* perf(mobile): assemble HTML replacements in one pass by @saphid in pingdotgg/t3code#25278
* perf(mobile): skip canceled SVG parsing by @saphid in pingdotgg/t3code#25314
* test(server): strengthen v1 cutover proof with event-stream waits and content snapshots by @saphid in pingdotgg/t3code#11869
* fix(contracts): a stored secret reference is always sensitive by @yordis in pingdotgg/t3code#31403
* fix(server): a settings save no longer reads an unreadable secret again by @yordis in pingdotgg/t3code#31226
* fix(threads): allow snoozing pending input requests by @saphid in pingdotgg/t3code#20058
* fix(server): repair cutover test crypto import breaking main CI by @Yash-Singh1 in pingdotgg/t3code#31630
* feat(server): sends can start a fresh provider session by @juliusmarminge in pingdotgg/t3code#30323
* feat: /clear starts a fresh session in the same thread by @juliusmarminge in pingdotgg/t3code#30325
* feat: scheduled tasks can run in one thread with a fresh session each run by @juliusmarminge in pingdotgg/t3code#30326
* fix(web): dragging a sidebar thread no longer scrolls the list out from under the pointer by @juliusmarminge in pingdotgg/t3code#30346
* fix(assets): preserve the PR repository for GitHub media by @juliusmarminge in pingdotgg/t3code#21631
* fix(server): load PR uploads from SSO organizations by @juliusmarminge in pingdotgg/t3code#21632
* fix(mobile): the Duo inspector stops at its widest instead of vanishing by @juliusmarminge in pingdotgg/t3code#31665
* test(server): launcher PATH lookup test no longer drops the Windows drive letter by @tris203 in pingdotgg/t3code#22534
* test(server): replay tests delete their temp directories by @only21mil in pingdotgg/t3code#23445
* fix(mobile): transcribe Android audio during recording by @PixPMusic in pingdotgg/t3code#31300
* test(web): linked-environment subagents keep opening in their own thread by @RealDoc06 in pingdotgg/t3code#26492
* test(server): pin post-commit outbox settle boundary by @Narayan201120 in pingdotgg/t3code#27560
* test: remove unreachable github cli decoding test by @t3-code-bot[bot] in pingdotgg/t3code#28115
* test(server): retain delegated child events when detaching a parent by @benjamin-di in pingdotgg/t3code#29622
* test(lint): pin which ResizeObserver constructions no-raw-resize-observer reports by @only21mil in pingdotgg/t3code#28781
* test(mobile): drop two deleted tests from the React test list by @only21mil in pingdotgg/t3code#28800
* test(web): worked-for expand test waits for the timeline to finish loading by @saphid in pingdotgg/t3code#29661
* test(web): avoid PDF worker startup timeouts in attachment tests by @voltcrash in pingdotgg/t3code#29933
* test(web): source control writing settings test waits for the reset button by @juliusmarminge in pingdotgg/t3code#30372
* test(web): keep the work heading test inside the viewport by @voltcrash in pingdotgg/t3code#30613
* test: use an empty config file instead of the null device for git fixtures on Windows by @Pyseph in pingdotgg/t3code#30772
* test: remove stale tests for unused thread and onboarding helpers by @t3-code-bot[bot] in pingdotgg/t3code#30817
* test(server): correct Windows path and settings assertions by @ElliotDrel in pingdotgg/t3code#30983
* feat(claude): Ultracode works with any effort, like Claude Desktop by @Bil0000 in pingdotgg/t3code#27034
* perf(pr): pull requests paint from data already held, on web, desktop and mobile by @Bil0000 in pingdotgg/t3code#27798
* fix(mobile): approval card buttons line up by @shivamhwp in pingdotgg/t3code#31415
* fix(gitlab): author names link to GitLab profiles by @TonybynMp4 in pingdotgg/t3code#30418
* fix(gitlab): merge messages drop agent credits by @TonybynMp4 in pingdotgg/t3code#30441
* fix(gitlab): file and image comments appear as review threads by @TonybynMp4 in pingdotgg/t3code#30425
* fix(web): screen readers no longer read out a secret typed into the request card by @juliusmarminge in pingdotgg/t3code#31653
* perf(pr): actions update every client from the host's reply instead of re-reads by @Bil0000 in pingdotgg/t3code#25761
* feat(mobile): start side chats with /side by @juliusmarminge in pingdotgg/t3code#31709
* feat(claude): expose Ultracode on Claude Sonnet 5.5 and Haiku 5.5 by @DrSeedon in pingdotgg/t3code#30781
* feat(settings): Simlock, Antigravity and OpenCode credentials can come from a secret provider by @juliusmarminge in pingdotgg/t3code#31652
* refactor(mobile): screens declare what they are; layout and AppBar own every platform bar decision by @juliusmarminge in pingdotgg/t3code#31310
* fix(server): a saved source control token that can't be read no longer falls back to another account by @juliusmarminge in pingdotgg/t3code#31646
* feat(secrets): answer a secret request with a secret provider reference by @juliusmarminge in pingdotgg/t3code#31670
* fix(settings): removing a secret provider clears the fields that used it by @juliusmarminge in pingdotgg/t3code#31651
* docs(user): explain secret providers and secret requests by @juliusmarminge in pingdotgg/t3code#31654
* feat(server): transcribe audio on a chosen environment by @juliusmarminge in pingdotgg/t3code#30625
* feat(web): opt into environment dictation on this device by @juliusmarminge in pingdotgg/t3code#30623
* feat(desktop): dictate into browser annotations with the same consent by @juliusmarminge in pingdotgg/t3code#30624
* feat(mobile): opt in to dictation on a paired environment by @juliusmarminge in pingdotgg/t3code#31059
* fix(web): keep the folder path caret in place while editing by @juliusmarminge in pingdotgg/t3code#31642
* perf(mobile): call the highlights lexer through Nitro, up to 10x faster calls on Android by @camchis in pingdotgg/t3code#31188
* fix(claude): turns use the downloaded model catalog by @juliusmarminge in pingdotgg/t3code#31717
* fix(gitlab): bot authors are flagged as bots by @TonybynMp4 in pingdotgg/t3code#30383
* fix(gitlab): merge requests can edit their labels by @TonybynMp4 in pingdotgg/t3code#30483
* perf(mobile): faster thread sync, connects, and reconnects on bad wifi by @Bil0000 in pingdotgg/t3code#28487
* fix(mobile): iPad composer drops when the keyboard floats or minimizes by @juliusmarminge in pingdotgg/t3code#31251
* perf(mobile): stop iOS syntax highlighting from looking up a thread-local on every lexer call by @camchis in pingdotgg/t3code#31187
* fix(gitlab): edited comments wake PR watchers by @TonybynMp4 in pingdotgg/t3code#30403

## New Contributors
* @benjamin-di made their first contribution in pingdotgg/t3code#29622
* @ElliotDrel made their first contribution in pingdotgg/t3code#30983
* @DrSeedon made their first contribution in pingdotgg/t3code#30781

**Full Changelog**: pingdotgg/t3code@v0.1.8-nightly.20261105.4234...v0.1.8-nightly.20261106.4247

</details>


Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.1.8-nightly.20261106.4247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). takeover t3 automation: takeover vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant