Skip to content

fix(mobile): show the diff when tapping a changed file from a session - #34073

Merged
AmoabaKelvin merged 2 commits into
mainfrom
mobile-show-tapped-file-diff
Oct 26, 2026
Merged

AmoabaKelvin merged 2 commits into
mainfrom
mobile-show-tapped-file-diff

Conversation

@AmoabaKelvin

@AmoabaKelvin AmoabaKelvin commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 0 0 0 0
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​66 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​63

ELI5

On a phone, tapping a changed file in a session's Changes screen took you back to the chat or terminal, and the diff never showed. Now you land on the file's diff.

What Changed

  • The desktop already opened the diff tab and marked it as this phone's selected tab. But the session screen keeps its own record of the tab you picked, and it only lets a host snapshot override that when the snapshot is marked follow. The Changes screen's activation is caller-only, so the session ignored it.
  • Docked Changes (wide screens) never had the bug: it calls the session's onFileOpenStart / onOpenedFileDiff, and the session switches to the diff tab itself. On phones, Changes is a separate screen pushed over the session and had no way to reach those callbacks.
  • The session now registers those two callbacks in session-diff-open-handlers.ts, keyed by host, execution host and worktree. When the pushed Changes page was opened from a session (origin=session), it passes them to the panel, so a tap takes the same path as the docked panel.
  • If no session is registered, for example when an older shell hands Source Control to a separate WebView, the old poll-and-activate path runs as before.

Why

I didn't relax the session's rule that its own pick wins. That rule (resolveActiveSessionTab) stops ordinary host republications from moving the phone off the tab you chose. Dropping the pick whenever the session regains focus would change what you see every time you come back to it. The registry only acts on the file you tapped.

Linked Issue

None.

Visual Proof

iOS simulator, OTA page, iPhone width (Changes pushed as its own screen). Terminal tab active → Changes → tap src/greet.js:

Before After

Testing

  • On the iOS simulator against a dev desktop serving the rebuilt OTA page:

    • Tapping a file from a terminal tab lands on its diff.
    • Doing the same with the diff tab already open also lands on the diff.
    • Opening Changes and pressing Back leaves the terminal selected.
  • Mobile typecheck, focused mobile tests (session, source-control, RPC recordings) and check:changed pass.

  • Starting from a chat tab wasn't tested on the device. It goes through the same session tab switch.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

No new tests. The behaviour depends on two screens sharing one page runtime, which the simulator run covers.

@AmoabaKelvin
AmoabaKelvin marked this pull request as ready for review October 26, 2026 22:19
The session keeps its own tab pick over host snapshots, so the pushed
Changes screen's host-side activation never reached it.
@AmoabaKelvin
AmoabaKelvin force-pushed the mobile-show-tapped-file-diff branch from 96fbf8c to ecfa08e Compare October 26, 2026 22:24
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6f51ac05-db75-45e7-9cca-bd4eff102013
Commits

Reviewing files that changed from the base of the PR and between 96fbf8c and ecfa08e.

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
Walkthrough

Session file actions now register file-open callbacks by host and worktree. The source-control screen looks up handlers for the current execution host and passes them to the panel when the origin is a session.

Priority: Pending latest changes

Merge Risk: Low · previous review 96fbf

After resuming a worktree from session history, returning to the original session may leave file taps in Changes unable to open a diff. This is a narrow, localized issue to fix before merge or explicitly accept.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description explains the user impact, implementation, rationale, visual result, and testing. However, the required Linked Issue section contains “None,” and the required issue link is missing. Sev… Add a valid issue reference in the Linked Issue section, such as “Fixes #12345.” Complete or explicitly address the remaining required sections and checklist items, including AI Disclosure when applicable, Review, Agent skill upstream bound…
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 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary user-facing fix: showing a diff when a changed file is tapped from a mobile session.
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 explains the user impact, implementation, rationale, visual result, and testing. However, the required Linked Issue section contains “None,” and the required issue link is missing. Several template sections and checklist items are also not provided.

Resolution

Add a valid issue reference in the Linked Issue section, such as “Fixes #12345.” Complete or explicitly address the remaining required sections and checklist items, including AI Disclosure when applicable, Review, Agent skill upstream boundary, Notes, and the requested validation checklist.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31bf2dc2-4ece-4739-a376-8f3b43ca77ab
Commits

Reviewing files that changed from the base of the PR and between 5681310 and 96fbf8c.

Files selected for processing: 3
  • mobile/app/h/[hostId]/source-control/[worktreeId].web.tsx
  • mobile/src/session/session-diff-open-handlers.ts
  • mobile/src/session/use-mobile-session-file-actions.ts

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

Resuming from Agent History pushes a second session for the same
worktree; its unmount dropped the first session's registration.
@AmoabaKelvin
AmoabaKelvin merged commit d7a8bb9 into main Oct 26, 2026
24 checks passed
@AmoabaKelvin
AmoabaKelvin deleted the mobile-show-tapped-file-diff branch October 26, 2026 22:42
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