Skip to content

fix(web): a closed proactive diff stays closed when you return to a thread - #14705

Open
ilyaliao wants to merge 4 commits into
pingdotgg:mainfrom
ilyaliao:fix/proactive-diff-offer-once
Open

ilyaliao wants to merge 4 commits into
pingdotgg:mainfrom
ilyaliao:fix/proactive-diff-offer-once

Conversation

@ilyaliao

@ilyaliao ilyaliao commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Completed-turn diffs reopen when you return to a thread, even after you dismiss them. Persist the completed run whose diff was offered in the existing thread-scoped panel state, so dismissal survives navigation and reload while the next qualifying run can still open Diff.

A qualifying offer is also consumed when a manual choice or an active Diff/PR panel prevents opening. Eligibility remains in the existing resolver; panel priority stays in the store. A refused offer returns before resetting Diff scope or invoking the open callback. Manual opening, environment/thread isolation, checkpoint readiness, size thresholds, and the Proactive panels setting retain their existing behavior. Removed the original copy-forward helper because current panel actions already preserve thread state.

Maintainer takeover of @ilyaliao's #14705, preserving their commits and merging main without rewriting the fork branch. Web and Electron share this renderer; mobile has separate navigation and is unchanged.

Verification at 71ee529:

  • Replaced six added cases in one existing test file with two persisted-state lifecycle cases. Focused store/resolver suites: 231 → 227 cases. Four related suites: 254 passing. Both new lifecycle cases fail against main; the review correction first produced three failures, then all 254 passed.
  • Scoped web typecheck, lint, formatting, and whitespace checks pass. Lint retains four pre-existing unused-import warnings in the existing resolver test file.
  • Two fresh parallel Claude Opus 5.5 / GPT-6 Sol review rounds. Round 1 found a completion while Diff/PR was active bypassed consumption; fresh test and implementation agents corrected that. Round 2 found no blockers.
  • Real Linux Chromium client with isolated synthetic completed-run/checkpoint projections: first offer opens; hide → another thread → return and reload stays hidden; next completed run opens; a later run observed while Diff is active is consumed, and closing/returning/reloading stays closed. Explicit manual reopening works. These are client fixtures, not executed provider turns. The server was stopped before fixture writes; the thread snapshot cache was refreshed after fixture changes while panel persistence was retained.

After: dismissed Diff stays closed after returning and reloading (current head).

After: Diff remains closed on the completed thread

Dismiss, switch threads, return, and reload recording · Active-Diff completion dismissal recording · Fixture observations and limits.

Limits: no matching base screenshot/recording was captured; before/after regression tests provide the baseline proof under the requested test-level fallback. Initial browser captures lost text and one tab disappeared; the replacement tab produced the usable current-head captures above. Clearing the fixture snapshot cache reset Diff scope during hydration, so exact scope preservation during a real provider completion remains test-level proof. Single-PR priority, migration/pruning, and cross-environment cases were verified by focused tests. macOS, Windows, Electron desktop, iOS, Android, relay/tunnel connections, and a real provider-completion flow were not run here.

Current-head CI: lint, build/typecheck, all eight server shards, package tests, web tests, aggregate Check, fingerprint checks, and configured bot reviews passed. The initial web run hit an unchanged ACP Registry debounce/Enter timing race; the rerun passed. No unresolved review threads.

Implementation: GPT-6.1 Sol through Codex; tests: GPT-6 Astra through Codex; independent reviews: Claude Opus 5.5 through Claude Code and GPT-6 Sol through Codex, delegated by T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 2, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 2, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 71ee529

Macroscope's review found this PR approvable — This is a focused web UI bug fix that records which completed run was offered and prevents dismissed or declined diffs from reopening after navigation or reload. The additive local persistence and panel-priority changes are contained within the existing right-panel store and covered by lifecycle tests.

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

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f0cebd95-aa79-4b03-a59d-d1c37f5edb98
📥 Commits

Reviewing files that changed from the base of the PR and between ed76c75 and 654298b.

📒 Files selected for processing: 3
  • apps/web/src/components/ChatView.tsx
  • apps/web/src/rightPanelStore.test.ts
  • apps/web/src/rightPanelStore.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.

📝 Walkthrough

ChatView now includes the completed turn ID in proactive diff requests. The right-panel store tracks offered turns, checks whether a diff can open, and restores the offer marker from persisted thread state. Tests cover dismissal, reload, repeat offers, later turns, and user-selected panels.

Changes: Proactive diff offers

Layer / File(s) Summary
Turn-scoped request state and persistence
apps/web/src/rightPanelStore.ts
The right-panel state and request type track diff offers by turn ID. Thread updates preserve the marker, and persisted state restores it.
Proactive diff offer flow
apps/web/src/components/ChatView.tsx, apps/web/src/rightPanelStore.ts, apps/web/src/rightPanelStore.test.ts
ChatView sends the completed turn ID with a diff request. The store prevents repeat offers for that turn, checks whether the diff can open, and records the turn even when it does not open. Tests cover dismissal, persistence, later turns, and user-selected panels.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ChatView
  participant RightPanelStore
  participant PersistedThreadState
  ChatView->>RightPanelStore: Request diff offer with completed turnId
  RightPanelStore->>RightPanelStore: Check prior offer and opening conditions
  RightPanelStore->>PersistedThreadState: Retain proactiveDiffTurnId
Loading

Suggested reviewers: saphid, t3dotgg

Merge Risk: ⚪ Minimal · up to 65429

No actionable issue is established in this change. The reported desktop behavior has not been confirmed against this version.

Security architecture risk: ⚪ Minimal · up to 65429

The change is confined to remembering automatic panel offers. The inspected flow preserves thread isolation and user-choice checks, without introducing additional access or privileges.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected change affects renderer-local panel state and its persisted per-environment, per-thread record. Manipulating the new marker can influence automatic panel presentation; the traced use does not grant additional service or data-access authority.

Trust Boundaries and Controls

  • observed — The store derives ownership from both environment and thread identity. Within the synchronous state update, it checks the current user-action revision and pull-request priority before opening a diff; recording a refused offer does not open a surface.
Pre-merge checks: Passed 4
Check name Status Explanation
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.
Title check Passed The title clearly and concisely describes the primary fix: preventing a dismissed proactive diff from reopening when the user returns to a thread.
Description check Passed The description provides the problem, implementation approach, scope, verification results, limitations, review status, and UI evidence. It does not use the template headings and does not include a di…
Full details: Description check

Explanation

The description provides the problem, implementation approach, scope, verification results, limitations, review status, and UI evidence. It does not use the template headings and does not include a direct link to the triaged issue or an explicit maintainer approval comment, but the required information is otherwise substantially covered.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 2, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 2, 2026 05:33

Dismissing prior approval to re-evaluate 43179b6

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 2, 2026
@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from 43179b6 to 410c41d Compare October 3, 2026 03:48
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 3, 2026 03:48

Dismissing prior approval to re-evaluate 410c41d

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 3, 2026
@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from 410c41d to 5625739 Compare October 6, 2026 17:48
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 6, 2026 17:49

Dismissing prior approval to re-evaluate 5625739

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 6, 2026
@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from 5625739 to 68db8d9 Compare October 7, 2026 14:31
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 7, 2026 14:31

Dismissing prior approval to re-evaluate 68db8d9

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 7, 2026
@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from 68db8d9 to ed76c75 Compare October 12, 2026 16:29
@vwgp8wkgjz-beep

Copy link
Copy Markdown

Still reproduces on the macOS desktop app, nightly 0.0.46-nightly.20261013.3053 (server running on Linux), with Proactive panels on. In multiple threads, the diff panel reopens every time I enter the thread, even after I close it. Turning Proactive panels off works around it. Would be glad to see this merged.

@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from ed76c75 to 654298b Compare October 16, 2026 07:49
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 16, 2026 07:50

Dismissing prior approval to re-evaluate 654298b

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 16, 2026
Proactive panels reopened a completed turn's diff on every thread visit
because the user-choice revision is re-read on entry and never persisted.
Remember the last offered turn in the persisted panel state, keep it when
other panel actions rebuild the state, and refuse a turn already offered.
@ilyaliao
ilyaliao force-pushed the fix/proactive-diff-offer-once branch from 654298b to a3abf64 Compare October 18, 2026 04:16
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 18, 2026 04:17

Dismissing prior approval to re-evaluate a3abf64

macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 18, 2026
@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch worktree-git-07. Keeping contributor commits, merging current main, then focused tests and independent reviews. No merge planned.

@macroscopeapp
macroscopeapp Bot dismissed their stale review October 19, 2026 07:40

Dismissing prior approval to re-evaluate 71ee529

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

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 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.

3 participants