Skip to content

fix(web): fall back when pull request avatars fail - #11728

Merged
maria-rcks merged 1 commit into
pingdotgg:mainfrom
tastelessjolt:fix/pr-avatar-load-fallback
Sep 17, 2026
Merged

maria-rcks merged 1 commit into
pingdotgg:mainfrom
tastelessjolt:fix/pr-avatar-load-fallback

Conversation

@tastelessjolt

@tastelessjolt tastelessjolt commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Closes #11727.

What Changed

PullRequestActorAvatar now switches to the existing actor-initial fallback when a remote image emits an error. A focused regression test covers the failed-image path.

This is the first-stage fallback only. It does not proxy authenticated GHES avatar bytes or change GHES cookie behavior.

Why

Private GitHub Enterprise Server avatar URLs can be cross-site relative to the T3 client. When the browser withholds SameSite=Lax or SameSite=Strict authentication cookies, GHES refuses the image request. The URL is still non-null, so the previous component left a broken image in every pull request activity row.

Before After
Broken images remain in the timeline. Failed images fall back to actor initials.
Broken GHES avatar images Actor initials after failed GHES avatar images

Verification

  • vp test run apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx
  • vp lint apps/web/src/components/pullRequest/pullRequestPresentation.tsx apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx
  • vp fmt --check apps/web/src/components/pullRequest/pullRequestPresentation.tsx apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx
  • vpr --filter @t3tools/web typecheck
  • Captured matched before/after images from the real web component with deliberately unavailable GHES-style avatar URLs.

The shared web component covers hosted web, local web, remote connections, and desktop. Mobile has no corresponding pull request activity view. No provider or wire-contract changes are needed.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the UI change
  • Video is not applicable because this change has no motion or interaction

Created with GPT-5.6 Sol in the Codex harness.

Summary by CodeRabbit

  • Bug Fixes

    • Pull request actor avatars now display an initial-based placeholder when the avatar image cannot be loaded.
  • Tests

    • Added coverage for the avatar fallback behavior when remote images fail.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 14, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 847a43f

Macroscope's review found this PR approvable — This is a localized web bug fix that replaces failed pull-request avatar images with the component's existing actor-initial fallback. Existing avatar and null-avatar paths are preserved, and the new behavior is covered by a focused test.

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

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a723c39b-801a-45a0-b271-425c97404db0

📥 Commits

Reviewing files that changed from the base of the PR and between 8ef478e and 847a43f.

📒 Files selected for processing (2)
  • apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx
  • apps/web/src/components/pullRequest/pullRequestPresentation.tsx

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

PullRequestActorAvatar now replaces failed avatar images with actor initials. A test covers the image error path and verifies the fallback output.

Changes

Avatar fallback

Layer / File(s) Summary
Failed avatar fallback
apps/web/src/components/pullRequest/pullRequestPresentation.tsx
The component tracks avatar load failures and renders the actor-initial placeholder when the avatar URL is missing or fails.
Fallback regression test
apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx
The test triggers the image error handler and verifies that the broken image is removed and the actor initial is rendered.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to 847a4

The avatar fallback is narrowly scoped and includes regression coverage for failed image loads.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding a fallback when pull request avatar images fail to load.
Description check ✅ Passed The description is complete and focused. It explains what changed, why the change is needed, the UI impact, verification steps, screenshots, scope limits, and checklist status.
Linked Issues check ✅ Passed Issue #11727 requires the existing actor-initial fallback when a non-null avatar URL fails. PullRequestActorAvatar now tracks failedAvatarUrl and renders the fallback when the failed URL matches t…
Out of Scope Changes check ✅ Passed The changes are limited to PullRequestActorAvatar and its focused regression test. They implement the linked issue without provider, wire-contract, cookie, or mobile changes.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@maria-rcks
maria-rcks merged commit df466c7 into pingdotgg:main Sep 17, 2026
21 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 17, 2026
## What's Changed
* fix(server): settle cancelled worktree setup before rollback by @juliusmarminge in pingdotgg/t3code#12176
* feat(mobile): port worktree setup progress and agent handoff by @juliusmarminge in pingdotgg/t3code#12177
* fix(server): flush checkpoint objects and refs before publishing them by @Mnigos in pingdotgg/t3code#10944
* fix(server): keep ready checkpoints when a later placeholder arrives by @Adolanium in pingdotgg/t3code#8432
* fix(server): keep VCS waits from blocking turn completion by @Vrtak-CZ in pingdotgg/t3code#11970
* fix(web): keep header spacing stable when sidebar drawer opens by @flamboh in pingdotgg/t3code#12162
* fix(web): fall back when pull request avatars fail by @tastelessjolt in pingdotgg/t3code#11728
* feat(web): enable rich text composer by default by @juliusmarminge in pingdotgg/t3code#12160
* feat(web): make keybindings searchable from settings search by @maria-rcks in pingdotgg/t3code#12175
* fix(web): preserve thread reading positions by @maria-rcks in pingdotgg/t3code#12144
* fix(diff): collapse files by default by @maria-rcks in pingdotgg/t3code#12190
* fix(web): folder links from chat open the file tree instead of a broken preview by @pc-style in pingdotgg/t3code#10909
* feat(web): command palette search matches thread IDs by @saphid in pingdotgg/t3code#11185
* fix(web): align notification icons with titles by @maria-rcks in pingdotgg/t3code#12202
* fix(skills): support unicode currency symbols as skill aliases by @WilgotM in pingdotgg/t3code#12098
* feat(settings): add automatic storage cleanup per machine and project by @maria-rcks in pingdotgg/t3code#11598
* feat(web): command palette finds the pull requests and usage pages by @flamboh in pingdotgg/t3code#12211
* feat(web): start new threads with multiple models in separate worktrees by @maria-rcks in pingdotgg/t3code#12179

## New Contributors
* @Adolanium made their first contribution in pingdotgg/t3code#8432
* @Vrtak-CZ made their first contribution in pingdotgg/t3code#11970
* @pc-style made their first contribution in pingdotgg/t3code#10909

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260917.1837...v0.0.43-nightly.20260917.1851

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260917.1851
AIdoesmyjob pushed a commit to AIdoesmyjob/t3code that referenced this pull request Sep 18, 2026
aorwall added a commit to aorwall/t3code that referenced this pull request Sep 18, 2026
Merges `pingdotgg/t3code` `6d1d549441` into the fork, from base
`0bf2d6b010` — 50 commits.

- **Landed:** 410 files against 407 in the upstream range; the gap of 3
is `docs/fork/gaps.md`, `inventory.json` and `upstream-merge-log.md`.
Everything in the range landed.
- **Fork delta:** 777 files.
- **Verification:** all 9 `verify.mjs` checks pass, tests green in all
15 packages.
- **Unsupported methods:** ADD 0, DROP 0 —
`packages/contracts/src/rpc.ts` and `auth.ts` are untouched. Upstream
added no WebSocket method in this range.

## The one that mattered

Upstream's pingdotgg#12015 moved the **entire body of the thread route** out of
`apps/web/src/routes/_chat.$environmentId.$threadId.tsx` and into a new
upstream file, `apps/web/src/components/ThreadRouteView.tsx`, rendered
by the `_chat` layout so a draft's promotion keeps the same `ChatView`
mounted. The route file is now a seven-line stub.

Three fork deltas lived in that file. They moved with it:
`useAdoptedThread`, `useAutoFollowThread` and the
`serverThreadAwaitingFirstAnswer` argument to
`resolveThreadRouteRenderState`, all reading `target.kind === "server" ?
target.threadRef : null` — a draft's reserved ref is the viewer's own
work and the listing carries it without being asked. The
`unlisted-thread-adoption` and `thread-follow` inventory entries were
re-pointed at the new file.

The fork's own delta guard is what caught this. The merge was clean and
typecheck was green; `features.test.ts` failed because
`useAutoFollowThread` was no longer in a file the inventory said it had
to be in.

## Conflicts

8 files, each resolved with the verdict `preflight.mjs` printed. Details
in the tracker entry; the short form:

| file | verdict | resolution |
| --- | --- | --- |
| `routes/_chat.$environmentId.$threadId.tsx` | unlisted | took
upstream's stub, deltas relocated (above) |
| `chat/MessagesTimeline.tsx` | `message-origin-upstream-files` | both
sides of `TimelineRowActivityState`, its memo and its deps merged;
dropped upstream's now-unused `GitPullRequestIcon` |
| `ThreadStatusIndicators.tsx` | `thread-status-indicators` | fork's
memo above upstream's early return — hooks before any conditional
`return null` |
| `settings/ProviderInstanceCard.tsx` | unlisted, in
`moatless-provider-auth` | kept the `FEATURES.providerConfiguration`
ternary, took upstream's container-query classNames inside it |
| `settings/SettingsPanels.tsx` | `settings-surface-gates` | re-stated
the fork's browser clause onto upstream's rewritten `proactive-panels`
text |
| `BranchToolbar.tsx` | `branch-toolbar-gates` | import block, both
sides kept |
| `RightPanelTabs.tsx` | `right-panel-surfaces` | import block, both
sides kept |
| `pnpm-lock.yaml` | `theirs — lockfile` | `--theirs` then `vp i`,
re-derived lockfile committed |

## Path policy closed a hole

`resolution-check` listed eight unlisted paths both sides changed;
**seven carried a real fork delta**, so next merge's `theirs` fallback
would have dropped them silently. All seven are now listed — five new
entries (`command-palette-gates`, `diff-panel-gates`,
`provider-settings-gates`, `chat-layout-route`,
`client-runtime-exports`) plus `rightPanelStore.test.ts` added to
`right-panel-surfaces`. The eighth is the thread route stub, which
resolved to upstream byte for byte.

## Usable as-is

Client work that runs against the Moatless backend today:

- **pingdotgg#12015** worktree setup card no longer flashes or shifts (the
relocation above) · **pingdotgg#12144** thread reading positions are preserved ·
**pingdotgg#12162** header spacing stays stable when the sidebar drawer opens
- **pingdotgg#8641** timestamps on tool rows and turn folds · **pingdotgg#12152** those
timestamps sit before the disclosure chevron · **pingdotgg#12147** thoughts group
into the changing tool activity line
- **pingdotgg#12075** send-shortcut and follow-up controls · **pingdotgg#12160** rich text
composer on by default · **pingdotgg#12165** composer task rows aligned ·
**pingdotgg#11787** tooltips on the composer's environment and workspace controls
· **pingdotgg#12082** simpler agent approval prompts
- **pingdotgg#12139** diff panel defaults to the working tree · **pingdotgg#12190** diff
files collapse by default · **pingdotgg#12142** a linked pull request wins over
an automatic diff
- **pingdotgg#12143** themes picked from chat with colour previews · **pingdotgg#12138**
provider settings adapt to content width · **pingdotgg#12167** follow-up and
license controls aligned
- **pingdotgg#12026** unsupported environments render as neutral rows with their
machine icon · **pingdotgg#12030** a discovered machine's icon survives a relay
refresh · **pingdotgg#12001** dropped folders become path chips locally and are
refused on remote environments
- **pingdotgg#11144** pull-request icon state centralised — a refactor the fork's
own badge filtering now rides

Not fork surfaces, landed for completeness: the mobile work (pingdotgg#11841,
pingdotgg#12169, pingdotgg#12177, version bump), the CLI installer progress bar (pingdotgg#12044),
docs (pingdotgg#11696), release chores and the Fable 5.1 badge (pingdotgg#12173).

## Unsupported in Moatless / needs implementation

- **Pull request surface** — `FEATURES.pullRequestSurface` is off, so
none of this merge's pull-request work is reachable: **pingdotgg#11994** (submit
PR comments with Cmd/Ctrl+Enter), **pingdotgg#12150** (comments easier to scan,
`apps/web/src/components/pullRequest/**` plus a `pullRequest.ts`
contract field), **pingdotgg#12168** (cached GitHub PR details reused across
entry points), **pingdotgg#12125** and **pingdotgg#11728** (author avatars and their
fallback). **pingdotgg#11706** needs backend work on top: private-repository
media in PR tabs goes through a new `packages/contracts/src/assets.ts`
proxy that Moatless would have to serve. Opening the surface means
deleting the `pullRequestSurface` entry and its gates, and dispatching
`pullRequests.list` / `.detail` / `.activity` — only
`pullRequests.summary` is served today.
- **Keybindings settings page** — **pingdotgg#12175** turns every keybinding
command into a searchable settings row pointing at
`/settings/keybindings`, which `FEATURES.serverAdministration` keeps out
of the sidebar and redirects on a typed URL. The rows still match in
settings search and land on that redirect. Left as-is this merge — it is
the same shape as the six `snap-shot-*` rows that have always done this,
and the one-line fix (a `settingsPathEnabled(item.to)` filter in
`filterAvailableSettingsSearchItems`) is a behaviour change that belongs
outside a merge. Recorded in `gaps.md`. Closes properly when
`server.upsertKeybinding` / `removeKeybinding` are dispatched.
- **Device hub** — **pingdotgg#12017** (detect unsupported legacy Android
command-line tools) and **pingdotgg#12033** (resolve Node for standalone helper
scripts) are both `apps/server/src/device/**`. `FEATURES.deviceHub` is
off and Moatless runs no device host at all, so there is nothing to do
and nothing to reproduce.

## Backend behavior to consider reproducing in Moatless

All recorded in `docs/fork/gaps.md`; nothing in this repository holds
them open.

Checkpoint and turn path, under _Runtime fixes upstream made to its own
server_:

- **pingdotgg#12154** keep large sparse checkouts on the fast checkpoint path —
streams `git ls-files --full-name --sparse -z -v` under a 4 KiB cap and
pins `sparse.expectFilesOutsideOfPatterns=false`. Without it a sparse
checkout large enough to blow the output limit drops to the slow path on
every checkpoint.
- **pingdotgg#10944** flush checkpoint objects and refs before publishing them —
otherwise a reader that acts on the announcement can find a ref pointing
at an object that is not there yet. Rare, unreproducible, permanent when
it lands.
- **pingdotgg#8432** keep a ready checkpoint when a later placeholder arrives
(`ProjectionPipeline.ts`) — the symptom is a checkpoint reverting to
pending and never coming back.
- **pingdotgg#11970** keep VCS waits from blocking turn completion
(`ProviderRuntimeIngestion.ts`, `decider.ts`) — a slow git call between
the provider's last event and the turn being marked done. Slower in a
sandbox than upstream.

Settlement, under _Settlement rules Moatless owns_:

- **pingdotgg#12161** settle on the `thread.pull-request-linked` / `-synced`
event with a per-thread sweep rather than waiting for the next periodic
one.
- **pingdotgg#12176** make the cancellation path uninterruptible around
record-and-rollback, so a cancelled worktree setup records its
settlement instead of being left mid-setup.

Client features that are inert until the backend emits or honours
something:

- **pingdotgg#11784** provider thinking traces — `orchestration` gained a
`reasoning` message role and `thread.message.reasoning.delta` /
`.complete` commands behind a `reasoningMessages: true` opt-in on
subscribe. The client renders them when they arrive; Moatless emits
none, so there are no traces.
- **pingdotgg#10822** complete counts and progressive large diffs —
`review.getDiffPreview` gained an optional `file` input (one file's
patch) and an optional `files` stat array ("absent on older servers").
Moatless dispatches the method and honours neither, so large diffs stay
truncated with incomplete counts.
- **pingdotgg#11519** native provider slash commands, exposed server-side and
consumed by the mobile client.
- **pingdotgg#12115** OpenCode Go, Cursor and Grok subscription limits in the
usage scan.

## Verification

`tripwires`, `duplicate-adds`, `resolution-check`, `inventory-check`,
`unsupported-methods`, `lockfile`, `fmt:check`, `lint` and `typecheck`
all pass; tests pass in all 15 packages. Two failures were found and
fixed on the way:

- `TS2552: Cannot find name 'label'` in `ThreadStatusIndicators.tsx` —
pingdotgg#11104/pingdotgg#11180 hoisted `label` onto the presentation object and the
fork's multi-link popover branch still read the removed local.
- The delta-guard test failure described above.

Two operational notes for the next run are in the tracker entry: `vp i`
needs `NODE_OPTIONS=--max-old-space-size=6144` in this sandbox, and
`--force-with-lease` needs the explicit `<ref>:<sha>` form with the SHA
read from `git ls-remote`, because this clone only fetches `main` and
the branch has no lease-eligible tracking ref.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
Moatless task:
https://moatless.soaplabstest.com/tasks/e3e17736-1c3d-4873-b9af-c434fd31b003
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Pull request avatars stay broken when a GHES image cannot load

2 participants