From 8536c86f34fd176678ac57f072c98b3cc2473ba8 Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Tue, 14 Jul 2026 14:40:51 +0800 Subject: [PATCH 01/11] chore(dev): bump to 0.11.2-dev Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/desktop/package.json | 2 +- package-lock.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index d21fbfee..4f29d5dd 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@meebox/desktop", - "version": "0.11.1", + "version": "0.11.2-dev", "private": true, "description": "meebox Electron desktop app", "author": { diff --git a/package-lock.json b/package-lock.json index 7eed3d82..41eb1b2e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -32,7 +32,7 @@ }, "apps/desktop": { "name": "@meebox/desktop", - "version": "0.11.1", + "version": "0.11.2-dev", "dependencies": { "@iconify-json/material-icon-theme": "^1.2.66", "@iconify/react": "^5.2.1", From 036e87a06d74bc6f777a98f946422c3010ea9f3e Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Fri, 24 Jul 2026 11:05:10 +0800 Subject: [PATCH 02/11] fix(diff): keep inline editors alive across a comments/drafts refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior fix (6be1c60) gave the inline comment zones a reconcile-in-place controller, but two gaps remained that still interrupted an open inline editor on every poll: 1. useDiffComments called setComments() unconditionally, so an unchanged poll still produced a fresh comments array. That churned the mentionCandidates memo, whose identity change re-ran useDraftZones. Bail on structural equality (reuse sameCommentList, now extracted to a shared commentEquality module) so an unchanged poll keeps the old reference and downstream memos stay stable — mirroring what the activity view already does. 2. useDraftZones used the one-shot mountInlineZones, so a real drafts/ comments change tore down and rebuilt every DraftZone root, dropping an open draft editor's edit mode / focus / caret. Give it the same split structural/content effect + persistent createInlineZones controller as useCommentZones, reconciling draft zones by (side, line) key. A surviving draft re-renders in place (DraftZoneList keys by draft id); only a removed draft (deleted / published / file switch) unmounts and runs its cancel cleanup. The now-unused one-shot mountInlineZones wrapper is removed. The activity timeline already bailed via sameCommentList and needs no change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../features/pr/tabs/comments/CommentItem.tsx | 40 +---------- .../features/pr/tabs/diff/DraftZoneList.tsx | 6 +- .../pr/tabs/diff/hooks/useDiffComments.ts | 5 +- .../pr/tabs/diff/hooks/useDraftZones.tsx | 72 +++++++++++++------ .../pr/tabs/diff/zones/mountInlineZones.ts | 53 ++------------ .../pr/tabs/shared/commentEquality.ts | 39 ++++++++++ 6 files changed, 105 insertions(+), 110 deletions(-) create mode 100644 apps/desktop/src/renderer/src/components/features/pr/tabs/shared/commentEquality.ts diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/comments/CommentItem.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/comments/CommentItem.tsx index 858a436e..7251b145 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/comments/CommentItem.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/comments/CommentItem.tsx @@ -22,43 +22,9 @@ const InlineCodeContext = lazy(() => import('./InlineCodeContext').then((m) => ({ default: m.InlineCodeContext })), ); -/** - * Structural equality comparison of the comment tree (by remoteId + body + version + edit/delete permissions + recursive replies). poll mostly returns - * comments with unchanged content: on equality, skip setState and keep the old reference so React bails out, avoiding pointless re-render of the whole - * comment tree (including inline Monaco) (refresh flicker). - */ -export function sameCommentList(a: readonly PrComment[], b: readonly PrComment[]): boolean { - if (a.length !== b.length) return false; - for (let i = 0; i < a.length; i++) { - const x = a[i]!; - const y = b[i]!; - if ( - x.remoteId !== y.remoteId || - x.body !== y.body || - x.version !== y.version || - x.canEdit !== y.canEdit || - x.canDelete !== y.canDelete || - !sameReactions(x.reactions, y.reactions) || - !sameCommentList(x.replies, y.replies) - ) { - return false; - } - } - return true; -} - -/** Equality comparison of the reactions array (emoji + count + mine triple matching item by item): lets reaction changes after a toggle trigger a re-render. */ -function sameReactions(a: PrComment['reactions'], b: PrComment['reactions']): boolean { - const x = a ?? []; - const y = b ?? []; - if (x.length !== y.length) return false; - for (let i = 0; i < x.length; i++) { - if (x[i]!.emoji !== y[i]!.emoji || x[i]!.count !== y[i]!.count || x[i]!.mine !== y[i]!.mine) { - return false; - } - } - return true; -} +// Structural comment-tree equality lives in the shared module so the diff view (useDiffComments) can reuse the same poll bail; re-exported here for the +// activity/comments-page importers that historically pulled it from this file. +export { sameCommentList } from '../shared/commentEquality'; /** * Maximum indent level for nested replies: past this level recursion continues but **no further indent is added** (flattened display), avoiding diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DraftZoneList.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DraftZoneList.tsx index f60adc55..035aaead 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DraftZoneList.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DraftZoneList.tsx @@ -7,8 +7,10 @@ import { DraftZone } from '../drafts/DraftZone'; /** * Container for multiple drafts on the same line; each is an independent DraftZone (maintaining its own read/edit), separated by hr. * onSave / onDelete call IPC drafts:update / drafts:delete here; after writing to disk the main side - * broadcasts a drafts:changed event → drafts-store refetches → DiffView's top-level useEffect rebuilds the - * zones (this component unmounts/remounts along with it). + * broadcasts a drafts:changed event → drafts-store refetches → useDraftZones' content effect calls the zone + * controller's update(), which **reconciles** this zone in place (keyed by draft id) rather than unmounting it, so a + * DraftZone whose editor is open keeps its text / focus across the refresh. Only a removed draft (deleted / published) + * unmounts. */ export function DraftZoneList({ drafts, diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffComments.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffComments.ts index 46487f48..533ab094 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffComments.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffComments.ts @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'; import type { PrComment, StoredPullRequest } from '@meebox/shared'; import { invoke, subscribe } from '../../../../../../api'; import { formatBackendError, type FormattedError } from '../../../../../../errors'; +import { sameCommentList } from '../../shared/commentEquality'; export interface DiffCommentsState { comments: PrComment[]; @@ -37,7 +38,9 @@ export function useDiffComments( const fetchList = (force: boolean): void => { invoke('diff:listComments', { localId: pr.localId, force }) .then((cs) => { - if (!cancelled) setComments(cs); + // poll mostly returns unchanged comments: keep the old array reference on structural equality so downstream memos + // (mentionCandidates) stay identity-stable and the inline draft/comment zones aren't torn down mid-edit (see useDraftZones). + if (!cancelled) setComments((prev) => (sameCommentList(prev, cs) ? prev : cs)); }) .catch((e: unknown) => { if (!cancelled) { diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDraftZones.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDraftZones.tsx index f6a5d175..87174018 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDraftZones.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDraftZones.tsx @@ -1,14 +1,22 @@ -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import { type editor as MonacoEditor } from 'monaco-editor'; import type { PlatformKind, PlatformUser, ReviewDraft } from '@meebox/shared'; import type { DiffChangedFile } from '@meebox/ipc'; import { DraftZoneList } from '../DraftZoneList'; -import { mountInlineZones } from '../zones/mountInlineZones'; +import { createInlineZones, type InlineZonesController } from '../zones/mountInlineZones'; import type { LoadedContent } from '../diff-types'; /** - * Inline draft view zones (blue background, editable). Uses the same mountInlineZones mechanism as comment zones, bucketed by anchor.side, - * with endLine as the zone line number (aligned with the publish anchor, WYSIWYG). + * Inline draft view zones (blue background, editable). Uses the same zone mechanism as comment zones, bucketed by + * anchor.side, with endLine as the zone line number (aligned with the publish anchor, WYSIWYG). + * + * Split into two effects like {@link useCommentZones}: a **structural** effect owns the zone controller's lifecycle + * (recreated only when the editor / file / view orientation / scope changes); a **content** effect calls + * `controller.update(...)` whenever the drafts or passthrough props change, which **reconciles** zones by (side, line) + * key rather than tearing them down. This lets an in-progress inline draft edit survive a drafts/comments refresh + * (e.g. the poller pulling a new remote comment mid-typing, or another draft being added elsewhere): the anchored + * line's zone keeps its React root, so the open editor's text / focus / caret aren't discarded. A genuinely removed + * draft (deleted / published / anchor moved / file switch) still unmounts, firing its useDraftZone cancel cleanup. * * Does not render rejected (user decided not to send) / posted (the remote comment is already taken over by CommentZone; re-rendering would be visually duplicate). * The commit read-only view (scopeKind !== 'all') does not render drafts (anchored on the PR full-diff line numbers, not applicable to a single commit). @@ -48,10 +56,44 @@ export function useDraftZones(opts: { scopeKind, } = opts; + // Structural lifecycle: (re)create the zone controller only when the editor / file / view orientation / scope + // changes. A drafts or comments refresh does NOT touch these deps, so the controller (and its live zones) survives — + // the content effect below then reconciles into it instead of tearing everything down. + const controllerRef = useRef | null>(null); useEffect(() => { if (!diffEditor || !content || !selected) return; // commit read-only view: does not render local draft zones (drafts are anchored on the PR full-diff line numbers, not applicable to a single commit). if (scopeKind !== 'all') return; + const controller = createInlineZones({ + diffEditor, + renderSideBySide, + zoneClassName: 'monaco-draft-zone', + innerClassName: 'monaco-draft-zone-inner', + stopEvents: [ + 'mousedown', + 'mouseup', + 'click', + 'dblclick', + 'keydown', + 'keyup', + 'wheel', + 'contextmenu', + ], + }); + controllerRef.current = controller; + return () => { + controller.dispose(); + controllerRef.current = null; + }; + }, [diffEditor, content, selected, renderSideBySide, scopeKind]); + + // Content sync: reconcile draft zones whenever the drafts / passthrough props change. Declared after the structural + // effect so on mount the controller exists before this runs (React runs setup in order). Reconcile means a surviving + // draft re-renders in place (an open editor keeps its text / focus), only added/removed drafts mount/unmount. + useEffect(() => { + const controller = controllerRef.current; + if (!controller || !diffEditor || !content || !selected) return; + if (scopeKind !== 'all') return; const fileDrafts = (drafts ?? []).filter((d) => { if (d.status === 'rejected' || d.status === 'posted') return false; // Reply-drafts render nested under their parent comment (ReplyDraftList), not as standalone line zones — skip them here. @@ -59,7 +101,6 @@ export function useDraftZones(opts: { if (!d.anchor) return false; return d.anchor.path === selected.path || selected.oldPath === d.anchor.path; }); - if (fileDrafts.length === 0) return; const oldByLine = new Map(); const newByLine = new Map(); @@ -75,23 +116,12 @@ export function useDraftZones(opts: { target.set(anchor.endLine, arr); } - return mountInlineZones({ - diffEditor, - renderSideBySide, + // Reconcile: unchanged draft lines re-render in place (an open draft editor keeps its text / focus / caret), only + // genuinely added/removed lines mount/unmount. An empty set removes all remaining zones (no early-return, so + // deleting the last draft on a line tears its zone down through the controller). + controller.update({ oldByLine, newByLine, - zoneClassName: 'monaco-draft-zone', - innerClassName: 'monaco-draft-zone-inner', - stopEvents: [ - 'mousedown', - 'mouseup', - 'click', - 'dblclick', - 'keydown', - 'keyup', - 'wheel', - 'contextmenu', - ], initialHeight: (ds) => Math.max(ds.length * 60, 80), render: (ds) => ( ), }); - // Does not depend on zone rebuilds triggered by autoEditTokens / registerEditTrigger (registerEditTrigger is a stable - // useCallback) — avoids trigger-induced DraftZone unmount/mount, eliminating the race of re-entering edit mode after cancel. }, [ diffEditor, drafts, diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts index 717ee8c9..232f50de 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts @@ -10,35 +10,14 @@ import { remapOldByLineToModified } from './line-mapping'; * `removeZone` / `unmount` cleanup. The differences (which events to intercept, initial height estimation, what component * to render) are injected via options. * - * Two entry points sharing the same internals: - * - {@link createInlineZones} returns a persistent controller whose `update(content)` **reconciles** zones by - * `(side, line)` key: an unchanged key re-renders its existing React root in place (preserving the zone's React - * state, e.g. an in-progress inline reply/edit that must survive a comments refresh / poll), a new key mounts a - * fresh zone, a vanished key is removed. Use this when the zone content changes independently of the editor/file. - * - {@link mountInlineZones} is the original one-shot form (create + populate once, teardown on cleanup), kept for - * callers that rebuild their whole zone set per effect run (the draft zones). + * {@link createInlineZones} returns a persistent controller whose `update(content)` **reconciles** zones by + * `(side, line)` key: an unchanged key re-renders its existing React root in place (preserving the zone's React + * state, e.g. an in-progress inline reply/edit/draft that must survive a comments/drafts refresh / poll), a new key + * mounts a fresh zone, a vanished key is removed. Both the inline comment zones (useCommentZones) and the inline draft + * zones (useDraftZones) drive it from a split structural/content effect pair, so their editors survive a refresh. * * The comment zone's extra glyph decorations are not managed here (the caller useCommentZones creates / clears them itself). */ -export interface MountInlineZonesOptions { - diffEditor: MonacoEditor.IStandaloneDiffEditor; - renderSideBySide: boolean; - /** Old-side (deleted / base-side context lines) buckets: key = line number */ - oldByLine: Map; - /** New-side (added / head-side context lines) buckets: key = line number */ - newByLine: Map; - /** Class of the monaco wrapper dom ('monaco-comment-zone' / 'monaco-draft-zone') */ - zoneClassName: string; - /** Class of the real visual container inner ('monaco-comment-zone-inner' / 'monaco-draft-zone-inner') */ - innerClassName: string; - /** Set of events stopPropagation takes over on dom + inner (drafts include keydown/wheel etc., comments only mouse-click kinds) */ - stopEvents: readonly string[]; - /** Initial zone height (px) estimation; lineHeight is monaco's current line height */ - initialHeight: (items: T[], lineHeight: number) => number; - /** Render the zone content (React node) */ - render: (items: T[]) => ReactNode; -} - /** Structural options for {@link createInlineZones}: everything that identifies where/how zones mount, minus the content. */ export interface CreateInlineZonesOptions { diffEditor: MonacoEditor.IStandaloneDiffEditor; @@ -376,25 +355,3 @@ export function createInlineZones(opts: CreateInlineZonesOptions): InlineZone return { update, dispose }; } - -/** - * One-shot form (create + populate once, teardown on cleanup). Behaviourally identical to the pre-controller - * mechanism, kept for callers (the draft zones) that rebuild their entire zone set on each effect run. Returns a - * cleanup function to call in the effect's teardown. - */ -export function mountInlineZones(opts: MountInlineZonesOptions): () => void { - const controller = createInlineZones({ - diffEditor: opts.diffEditor, - renderSideBySide: opts.renderSideBySide, - zoneClassName: opts.zoneClassName, - innerClassName: opts.innerClassName, - stopEvents: opts.stopEvents, - }); - controller.update({ - oldByLine: opts.oldByLine, - newByLine: opts.newByLine, - initialHeight: opts.initialHeight, - render: opts.render, - }); - return () => controller.dispose(); -} diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/commentEquality.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/commentEquality.ts new file mode 100644 index 00000000..afec5377 --- /dev/null +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/shared/commentEquality.ts @@ -0,0 +1,39 @@ +import type { PrComment } from '@meebox/shared'; + +/** + * Structural equality comparison of the comment tree (by remoteId + body + version + edit/delete permissions + recursive replies). poll mostly returns + * comments with unchanged content: on equality, callers skip setState and keep the old reference so React bails out, avoiding pointless re-render of the + * whole comment tree (including inline Monaco) and — in the diff view — the teardown/rebuild of open inline editors driven by the comments-array churn. + */ +export function sameCommentList(a: readonly PrComment[], b: readonly PrComment[]): boolean { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + const x = a[i]!; + const y = b[i]!; + if ( + x.remoteId !== y.remoteId || + x.body !== y.body || + x.version !== y.version || + x.canEdit !== y.canEdit || + x.canDelete !== y.canDelete || + !sameReactions(x.reactions, y.reactions) || + !sameCommentList(x.replies, y.replies) + ) { + return false; + } + } + return true; +} + +/** Equality comparison of the reactions array (emoji + count + mine triple matching item by item): lets reaction changes after a toggle trigger a re-render. */ +function sameReactions(a: PrComment['reactions'], b: PrComment['reactions']): boolean { + const x = a ?? []; + const y = b ?? []; + if (x.length !== y.length) return false; + for (let i = 0; i < x.length; i++) { + if (x[i]!.emoji !== y[i]!.emoji || x[i]!.count !== y[i]!.count || x[i]!.mine !== y[i]!.mine) { + return false; + } + } + return true; +} From f4b5146c7fd424c13fb1cf0f57422d4e2aa4ed51 Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Fri, 24 Jul 2026 11:58:45 +0800 Subject: [PATCH 03/11] fix(diff): anchor inline comments to the actual render mode after auto-degrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `renderSideBySide` threaded through the diff view is the toolbar *intent*, not the mode Monaco actually renders. When the pane is too narrow Monaco auto-degrades side-by-side → inline (useInlineViewWhenSpaceIsLimited) while the intent stays true, so old-side (base) items get routed to the original editor Monaco has hidden. A published old-side inline comment then had no visible position, and its glyph dot / overview tick vanished too. Introduce a single source of truth for the *actual* mode (useActualRenderSideBySide, reading Monaco's `.monaco-diff-editor.side-by-side` class reactively, the pattern useDiffOverviewMarks already used) and feed it to every consumer that positions by editor side: - DiffView passes the actual mode to useCommentZones / useDraftZones / useLineCommentAdder / useSelectionCapture (was the raw intent). - useCommentZones remaps old-side glyph/overview decorations onto the modified editor when actually inline (mirroring the zone-body routing in computeDesired) — this also fixes old-side markers in explicitly-chosen unified, a latent bug. - useDiffOverviewMarks reuses the shared isActualSideBySide helper. Also fix old-side anchor reveal, which was broken independently: the nav anchor dropped `side` (App/PrPanel/PublishReviewModal/notification) and useDiffNav hardcoded side:'new' with no old→new remap. Thread `side` through the anchor (incl. the notification:activate IPC event) and, at reveal time, read the live actual mode: an old-side target reveals on the original editor only when it is genuinely visible, otherwise remaps the old line onto the modified editor. The cross-file search jump reuses the same path. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/main/services/notifications.ts | 6 +- .../src/components/features/pr/PrPanel.tsx | 8 ++- .../features/pr/tabs/diff/DiffView.tsx | 25 ++++++-- .../features/pr/tabs/diff/hooks/index.ts | 4 ++ .../diff/hooks/useActualRenderSideBySide.ts | 61 +++++++++++++++++++ .../pr/tabs/diff/hooks/useCommentZones.tsx | 24 ++++++-- .../features/pr/tabs/diff/hooks/useDiffNav.ts | 35 ++++++++--- .../tabs/diff/hooks/useDiffOverviewMarks.ts | 11 +--- .../pr/tabs/diff/zones/mountInlineZones.ts | 3 + .../src/renderer/src/hooks/usePrNavigation.ts | 8 ++- packages/ipc/src/events.ts | 2 +- 11 files changed, 151 insertions(+), 36 deletions(-) create mode 100644 apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useActualRenderSideBySide.ts diff --git a/apps/desktop/src/main/services/notifications.ts b/apps/desktop/src/main/services/notifications.ts index 29865c7e..6f947dd4 100644 --- a/apps/desktop/src/main/services/notifications.ts +++ b/apps/desktop/src/main/services/notifications.ts @@ -67,7 +67,11 @@ function activateOnClick(e: PollNotificationEvent): () => void { anchor: // File-level comments (no line) aren't line-navigable → no anchor (clicking just opens the PR). e.comment?.anchor && e.comment.anchor.line != null - ? { path: e.comment.anchor.path, line: e.comment.anchor.line } + ? { + path: e.comment.anchor.path, + line: e.comment.anchor.line, + side: e.comment.anchor.side, + } : null, }); }; diff --git a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx index 4e58b7e5..53b8d5da 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx @@ -37,13 +37,13 @@ export interface PrPanelProps { pendingDiffNav?: { runId?: string; findingId?: string; - anchor: { path: string; startLine: number; endLine: number }; + anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' }; } | null; onDiffNavConsumed?: () => void; onRequestDiffNav?: (target: { runId?: string; findingId?: string; - anchor: { path: string; startLine: number; endLine: number }; + anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' }; }) => void; /** External request to switch to a given tab (e.g. clicking a summary comment notification → 'activity'); cleared via onPendingTabConsumed after consumption. */ pendingTab?: PrTab | null; @@ -233,7 +233,7 @@ export function PrPanel({ // File-level anchors (no line) aren't line-navigable; CommentItem doesn't make them clickable, guard anyway. if (a.line == null) return; onRequestDiffNav?.({ - anchor: { path: a.path, startLine: a.line, endLine: a.line }, + anchor: { path: a.path, startLine: a.line, endLine: a.line, side: a.side }, }); }} /> @@ -252,6 +252,7 @@ export function PrPanel({ path: d.anchor.path, startLine: d.anchor.startLine, endLine: d.anchor.endLine, + side: d.anchor.side, }, }); }} @@ -281,6 +282,7 @@ export function PrPanel({ path: d.anchor.path, startLine: d.anchor.startLine, endLine: d.anchor.endLine, + side: d.anchor.side, }, }); }} diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DiffView.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DiffView.tsx index 8e11a08f..85a6eda0 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DiffView.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/DiffView.tsx @@ -21,6 +21,7 @@ import { BackendErrorBanner, BackendErrorView, SyncProgress } from './DiffStatus import { BlameColumn } from './blame/BlameColumn'; import { fileKey, type PendingCommitView } from './diff-types'; import { + useActualRenderSideBySide, useBlame, useChangedFiles, useCommentZones, @@ -60,7 +61,7 @@ interface DiffViewProps { pendingNav?: { runId?: string; findingId?: string; - anchor: { path: string; startLine: number; endLine: number }; + anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' }; } | null; onNavConsumed?: () => void; /** @@ -107,6 +108,10 @@ export function DiffView({ const drafts = useDraftsForPr(pr.localId); // Use state, not a ref: onMount fires asynchronously, so a state change is required to re-run the subsequent useEffect decoration logic. const [diffEditor, setDiffEditor] = useState(null); + // The ACTUAL render mode: renderSideBySide is only the toolbar intent, but Monaco auto-degrades to inline when the + // pane is too narrow. Positioning (which inner editor a comment/draft zone, glyph, or "+" adder targets) must use + // this, not the intent, or old-side items land on the original editor Monaco has hidden. See useActualRenderSideBySide. + const actualSideBySide = useActualRenderSideBySide(diffEditor, renderSideBySide); const progress = useSyncProgress(pr); const { fileListWidth, startFileListResize } = useFileListWidth(); @@ -168,9 +173,16 @@ export function DiffView({ pendingNav, onNavConsumed, triggerAutoEdit, + // Intent prop: useDiffNav reads the actual mode live at reveal time to place old-side targets correctly. + renderSideBySide, }); // Capture the Diff selection → selectionStore, so ChatPane can carry the selected code as implicit context into agent/ask questions. - useSelectionCapture({ diffEditor, selected, prLocalId: pr.localId, renderSideBySide }); + useSelectionCapture({ + diffEditor, + selected, + prLocalId: pr.localId, + renderSideBySide: actualSideBySide, + }); // sidebar mode: 'tree' (file tree) / 'search' (cross-file search), defaults to the file tree. Returns to 'tree' on PR switch. const [sidebarMode, setSidebarMode] = useState<'tree' | 'search'>('tree'); @@ -237,7 +249,9 @@ export function DiffView({ attachmentBase, prLocalId: pr.localId, prWebUrl: pr.url, - renderSideBySide, + // The actual render mode, so old-side comment zones + glyph/tick markers land on the visible editor after an + // auto-degrade to unified (the reported "published comment lacks position" case). + renderSideBySide: actualSideBySide, commentHardBreaks, reactionsMode, attachmentsEnabled, @@ -254,7 +268,7 @@ export function DiffView({ selected, prLocalId: pr.localId, registerEditTrigger, - renderSideBySide, + renderSideBySide: actualSideBySide, commentHardBreaks, attachmentsEnabled, mentionCandidates, @@ -271,7 +285,8 @@ export function DiffView({ prLocalId: pr.localId, platform: pr.platform, scopeKind: scope.kind, - renderSideBySide, + // Actual mode: wire the old-side "+" adder only when the original editor is actually visible (not auto-degraded). + renderSideBySide: actualSideBySide, readOnly, triggerAutoEdit, t, diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/index.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/index.ts index b5074e68..46b67b2c 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/index.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/index.ts @@ -11,6 +11,10 @@ export { useBlame, type BlameState } from './useBlame'; export { useDraftAutoEdit, type DraftAutoEdit } from './useDraftAutoEdit'; export { useDiffNav, type PendingNav, type PendingScroll } from './useDiffNav'; export { useCommentZones } from './useCommentZones'; +export { + useActualRenderSideBySide, + isActualSideBySide, +} from './useActualRenderSideBySide'; export { useDiffOverviewMarks } from './useDiffOverviewMarks'; export { useDraftZones } from './useDraftZones'; export { useLineCommentAdder } from './useLineCommentAdder'; diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useActualRenderSideBySide.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useActualRenderSideBySide.ts new file mode 100644 index 00000000..2e42295e --- /dev/null +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useActualRenderSideBySide.ts @@ -0,0 +1,61 @@ +import { useEffect, useState } from 'react'; +import { type editor as MonacoEditor } from 'monaco-editor'; + +/** + * `renderSideBySide` on the toolbar is the user's *intent* (persisted toggle). Monaco auto-degrades side-by-side → + * inline when the pane is too narrow (`useInlineViewWhenSpaceIsLimited`, on by default), leaving the intent `true` + * while the actual layout is unified. Monaco reflects the real mode in the `.monaco-diff-editor` root node's + * `side-by-side` class (removed when downgrading to inline). + * + * Anything that positions content by editor side — which inner editor a comment/draft zone, glyph dot, overview tick, + * or reveal targets — must key off the **actual** mode, not the intent: in the auto-degraded state the original editor + * is hidden, so an old-side item routed there by intent lands on an invisible editor ("missing position"). This + * returns the actual mode: the intent AND the class being present. + */ +export function isActualSideBySide( + diffEditor: MonacoEditor.IStandaloneDiffEditor, + renderSideBySide: boolean, +): boolean { + if (!renderSideBySide) return false; + const el = diffEditor.getContainerDomNode().querySelector('.monaco-diff-editor'); + return el ? el.classList.contains('side-by-side') : true; +} + +/** + * Reactive {@link isActualSideBySide}: recomputes when Monaco's layout crosses the side-by-side ↔ inline breakpoint + * (`onDidLayoutChange`) or the diff recomputes (`onDidUpdateDiff`). rAF-coalesced and deferred so the class is read + * after Monaco has switched it (the layout event may precede the class flip). Returns the intent prop directly until + * the editor is available. + */ +export function useActualRenderSideBySide( + diffEditor: MonacoEditor.IStandaloneDiffEditor | null, + renderSideBySide: boolean, +): boolean { + const [actual, setActual] = useState(renderSideBySide); + useEffect(() => { + if (!diffEditor) { + setActual(renderSideBySide); + return; + } + const read = (): void => setActual(isActualSideBySide(diffEditor, renderSideBySide)); + read(); + let raf = 0; + const schedule = (): void => { + if (raf) return; + raf = requestAnimationFrame(() => { + raf = 0; + read(); + }); + }; + // The original editor still emits layout changes as it collapses to / expands from hidden at the breakpoint; + // onDidUpdateDiff covers the first async diff settling after a file switch. + const layoutDisp = diffEditor.getOriginalEditor().onDidLayoutChange(schedule); + const diffDisp = diffEditor.onDidUpdateDiff(schedule); + return () => { + if (raf) cancelAnimationFrame(raf); + layoutDisp.dispose(); + diffDisp.dispose(); + }; + }, [diffEditor, renderSideBySide]); + return actual; +} diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useCommentZones.tsx b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useCommentZones.tsx index d191c8fa..6157d406 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useCommentZones.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useCommentZones.tsx @@ -8,6 +8,7 @@ import { renderHoverMd, } from '../inline-comments/InlineCommentZone'; import { createInlineZones, type InlineZonesController } from '../zones/mountInlineZones'; +import { remapOldByLineToModified } from '../zones/line-mapping'; import type { LoadedContent } from '../diff-types'; /** @@ -158,16 +159,29 @@ export function useCommentZones(opts: { const originalEditor = diffEditor.getOriginalEditor(); const modifiedEditor = diffEditor.getModifiedEditor(); - const originalDecorations = originalEditor.createDecorationsCollection( - buildDecorations(oldByLine), - ); + // Old-side markers: in side-by-side they sit on the (visible) original editor; in unified — including + // auto-degraded-from-side-by-side, since renderSideBySide here is the ACTUAL render mode — the original editor is + // hidden, so remap them onto the modified editor at the mapped line (mirroring how the zone bodies are routed in + // computeDesired), otherwise the glyph dot + overview tick vanish with the hidden editor. + const modifiedLines = new Map(newByLine); + let originalLines: Map | null = oldByLine; + if (!renderSideBySide && oldByLine.size > 0) { + originalLines = null; + const remappedOld = remapOldByLineToModified(diffEditor.getLineChanges() ?? [], oldByLine); + for (const [line, cs] of remappedOld) { + modifiedLines.set(line, [...(modifiedLines.get(line) ?? []), ...cs]); + } + } + const originalDecorations = originalLines + ? originalEditor.createDecorationsCollection(buildDecorations(originalLines)) + : null; const modifiedDecorations = modifiedEditor.createDecorationsCollection( - buildDecorations(newByLine), + buildDecorations(modifiedLines), ); return () => { try { - originalDecorations.clear(); + originalDecorations?.clear(); modifiedDecorations.clear(); } catch { // editor already disposed diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffNav.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffNav.ts index 11d8214d..d9c6bb6b 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffNav.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffNav.ts @@ -3,6 +3,8 @@ import { type editor as MonacoEditor } from 'monaco-editor'; import type { ReviewDraft } from '@meebox/shared'; import type { DiffChangedFile } from '@meebox/ipc'; import { fileKey, type LoadedContent } from '../diff-types'; +import { mapOriginalLineToModified } from '../zones/line-mapping'; +import { isActualSideBySide } from './useActualRenderSideBySide'; export interface PendingScroll { line: number; @@ -13,7 +15,7 @@ export interface PendingScroll { export interface PendingNav { runId?: string; findingId?: string; - anchor: { path: string; startLine: number; endLine: number }; + anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' }; } /** @@ -31,6 +33,8 @@ export function useDiffNav(opts: { pendingNav: PendingNav | null | undefined; onNavConsumed: (() => void) | undefined; triggerAutoEdit: (draftId: string) => void; + /** Toolbar intent; the reveal reads the ACTUAL mode from this at reveal time (see isActualSideBySide) to place old-side targets. */ + renderSideBySide: boolean; }): { pendingScroll: PendingScroll | null; setPendingScroll: React.Dispatch>; @@ -45,6 +49,7 @@ export function useDiffNav(opts: { pendingNav, onNavConsumed, triggerAutoEdit, + renderSideBySide, } = opts; const [pendingScroll, setPendingScroll] = useState(null); @@ -71,7 +76,9 @@ export function useDiffNav(opts: { setPendingScroll({ // Take endLine to align with the draft zone / publish anchor (see zone line number comment), so the highlight line matches the draft zone line: pendingNav.anchor.endLine, - side: 'new', + // Preserve the anchor's side so an old-side (base) comment/draft reveals on the correct side; older producers that + // don't carry side fall back to 'new' (head), the common case for code-review findings. + side: pendingNav.anchor.side ?? 'new', draftId: matchingDraft?.id, }); onNavConsumed?.(); @@ -84,10 +91,6 @@ export function useDiffNav(opts: { // pendingScroll comes from the nav effect (set alongside setSelectedKey); cleared after reveal useEffect(() => { if (!pendingScroll || !diffEditor || !content || !selected) return; - const editor = - pendingScroll.side === 'old' - ? diffEditor.getOriginalEditor() - : diffEditor.getModifiedEditor(); let highlightTimer: ReturnType | undefined; let revealed = false; @@ -95,15 +98,27 @@ export function useDiffNav(opts: { // onDidUpdateDiff may fire multiple times, only jump once if (revealed) return; revealed = true; + // Resolve the target editor + line by side AND the ACTUAL render mode (read live here, after the diff has settled + // and Monaco's `.side-by-side` class is stable). An old-side target sits on the original editor only when it is + // genuinely visible; when Monaco has auto-degraded to unified the original editor is hidden, so remap the old line + // onto the modified editor (same mapping the old-side zones use) and reveal there — otherwise the reveal lands on + // an invisible editor / the wrong line. + const revealOld = + pendingScroll.side === 'old' && isActualSideBySide(diffEditor, renderSideBySide); + const editor = revealOld ? diffEditor.getOriginalEditor() : diffEditor.getModifiedEditor(); + const line = + pendingScroll.side === 'old' && !revealOld + ? mapOriginalLineToModified(diffEditor.getLineChanges() ?? [], pendingScroll.line) + : pendingScroll.line; // Center-scroll to the target line - editor.revealLineInCenter(pendingScroll.line); + editor.revealLineInCenter(line); // Brief highlight: 300ms yellow-background pulse const collection = editor.createDecorationsCollection([ { range: { - startLineNumber: pendingScroll.line, + startLineNumber: line, startColumn: 1, - endLineNumber: pendingScroll.line, + endLineNumber: line, endColumn: 1, }, options: { @@ -143,7 +158,7 @@ export function useDiffNav(opts: { }; // triggerAutoEdit not in deps — it changes reference every render, including it would make reveal rerun every frame, repeatedly locating/highlighting // eslint-disable-next-line react-hooks/exhaustive-deps - }, [pendingScroll, diffEditor, content, selected]); + }, [pendingScroll, diffEditor, content, selected, renderSideBySide]); return { pendingScroll, setPendingScroll }; } diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffOverviewMarks.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffOverviewMarks.ts index 7c7f842f..b69086a9 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffOverviewMarks.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/hooks/useDiffOverviewMarks.ts @@ -2,6 +2,7 @@ import { useEffect } from 'react'; import { editor as MonacoEditorNs, type editor as MonacoEditor } from 'monaco-editor'; import type { DiffChangedFile } from '@meebox/ipc'; import type { LoadedContent } from '../diff-types'; +import { isActualSideBySide } from './useActualRenderSideBySide'; // Add/change green, delete red (same color family as GitHub diff). diff marks go on the overview ruler's Left lane, // separate from comment anchors (Right lane, see useCommentZones), so they don't overlap. @@ -38,14 +39,6 @@ export function useDiffOverviewMarks(opts: { const modCol = modifiedEditor.createDecorationsCollection([]); const origCol = originalEditor.createDecorationsCollection([]); - // Whether actually side-by-side: read Monaco's `.monaco-diff-editor.side-by-side` class that reflects the actual render mode - // (the class is removed when auto-downgrading to inline on insufficient width); fall back to the user intent prop when unavailable. - const isSideBySide = (): boolean => { - if (!renderSideBySide) return false; - const el = diffEditor.getContainerDomNode().querySelector('.monaco-diff-editor'); - return el ? el.classList.contains('side-by-side') : true; - }; - const Lane = MonacoEditorNs.OverviewRulerLane; const deco = ( startLine: number, @@ -59,7 +52,7 @@ export function useDiffOverviewMarks(opts: { const refresh = (): void => { const changes = diffEditor.getLineChanges() ?? []; - const sideBySide = isSideBySide(); + const sideBySide = isActualSideBySide(diffEditor, renderSideBySide); const modDecos: MonacoEditor.IModelDeltaDecoration[] = []; const origDecos: MonacoEditor.IModelDeltaDecoration[] = []; for (const c of changes) { diff --git a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts index 232f50de..088baf62 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/tabs/diff/zones/mountInlineZones.ts @@ -248,6 +248,9 @@ export function createInlineZones(opts: CreateInlineZonesOptions): InlineZone for (const [line, items] of content.newByLine) { desired.set(`new:${line}`, { editor: modifiedEditor, afterLine: line, items }); } + // renderSideBySide here is the ACTUAL render mode (callers pass useActualRenderSideBySide, not the toolbar intent): + // only mount old-side zones on the original editor when it is genuinely visible; when Monaco has auto-degraded to + // inline the original editor is hidden, so remap old lines onto the modified editor instead. if (renderSideBySide) { for (const [line, items] of content.oldByLine) { desired.set(`old:${line}`, { editor: originalEditor, afterLine: line, items }); diff --git a/apps/desktop/src/renderer/src/hooks/usePrNavigation.ts b/apps/desktop/src/renderer/src/hooks/usePrNavigation.ts index 4c5a457c..2b65d6d7 100644 --- a/apps/desktop/src/renderer/src/hooks/usePrNavigation.ts +++ b/apps/desktop/src/renderer/src/hooks/usePrNavigation.ts @@ -10,7 +10,9 @@ import { formatBackendError } from '../errors'; export interface PendingDiffNav { runId?: string; findingId?: string; - anchor: { path: string; startLine: number; endLine: number }; + // `side` decides which diff side the reveal targets (old = base/left, new = head/right); omitted defaults to 'new'. + // Required so an old-side comment/draft reveal lands correctly (incl. after an auto-degrade to unified — see useDiffNav). + anchor: { path: string; startLine: number; endLine: number; side?: 'old' | 'new' }; } export interface PrNavigation { @@ -156,7 +158,9 @@ export function usePrNavigation({ return subscribe('notification:activate', ({ localId, kind, anchor }) => { void jumpToPr(localId); if (anchor) { - setPendingDiffNav({ anchor: { path: anchor.path, startLine: anchor.line, endLine: anchor.line } }); + setPendingDiffNav({ + anchor: { path: anchor.path, startLine: anchor.line, endLine: anchor.line, side: anchor.side }, + }); } else if (kind === 'mention' || kind === 'reply') { setPendingTab('activity'); } diff --git a/packages/ipc/src/events.ts b/packages/ipc/src/events.ts index a0ff45fa..b7ad9ada 100644 --- a/packages/ipc/src/events.ts +++ b/packages/ipc/src/events.ts @@ -84,7 +84,7 @@ export interface IpcEvents { 'notification:activate': { localId: string; kind: PollNotificationKind; - anchor: { path: string; line: number } | null; + anchor: { path: string; line: number; side: 'old' | 'new' } | null; }; } From 31c963ba6c271625e1b0f4b029a60c520a24424f Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Fri, 24 Jul 2026 12:05:07 +0800 Subject: [PATCH 04/11] fix(mac): stop the unexpected Apple Music / media-library permission prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS the app popped a "would like to access Apple Music, your music and video activity, and your media library" prompt on launch. It's not from packaging — no music entitlement and no NSAppleMusicUsageDescription are declared. It's runtime: Chromium's macOS "Now Playing" / media-session integration queries the MediaPlayer framework, which triggers the media library permission. The app plays no media and exposes no now-playing controls, so the permission is unexpected. Disable the MediaSessionService and HardwareMediaKeyHandling features (mac startup, before app.whenReady()) so Chromium never touches the media library. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/desktop/src/main/bootstrap/os-startup-tweaks.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/desktop/src/main/bootstrap/os-startup-tweaks.ts b/apps/desktop/src/main/bootstrap/os-startup-tweaks.ts index f92b1007..a37fb752 100644 --- a/apps/desktop/src/main/bootstrap/os-startup-tweaks.ts +++ b/apps/desktop/src/main/bootstrap/os-startup-tweaks.ts @@ -56,10 +56,16 @@ function applyWindowsStartupTweaks(): void { * real keychain. Cost: cookie encryption degrades to a static key, but the key was already stored in * plaintext, so no real loss. Removable once there's a proper Developer ID signature. Must be before * app.whenReady(). + * - disable-features MediaSessionService/HardwareMediaKeyHandling: Chromium's macOS "Now Playing" / + * media-session integration queries the MediaPlayer framework, which makes macOS prompt for "access + * Apple Music / your media library" on launch. We play no media and expose no now-playing controls, so + * this permission is unexpected and confusing; disabling the features stops Chromium from ever touching + * the media library. Must be before app.whenReady(). * - Prepend common CLI dirs to PATH (see augmentMacPath): must be before pr-agent probing / running. */ function applyMacStartupTweaks(): void { app.commandLine.appendSwitch('use-mock-keychain'); + app.commandLine.appendSwitch('disable-features', 'MediaSessionService,HardwareMediaKeyHandling'); augmentMacPath(); } From 137434025bbea37755c8d4467c2c029e16b5d5e9 Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Mon, 27 Jul 2026 12:01:14 +0800 Subject: [PATCH 05/11] feat(chat): mark stale reviews with a commit divider when the PR head advances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the PR's head commit moves past the commit the most recent agent run reviewed, the run timeline now shows a sawtooth "commit divider" at the bottom labelled with the new head's short SHA — signalling that the reviews above are based on stale code. It appears as soon as the code changes, whether or not a new run has been started against it, and is suppressed while a run is active (that run is already processing the current head). - Stamp the PR head SHA (pr.sourceRef.sha) onto each run at start (ReviewRun. headSha, threaded through StartReviewRunInput / startReviewRun / the executor) as the baseline to compare the live head against. Runs predating this field have no headSha and are not used as a baseline. - ChatPane derives staleHeadSha (live head vs the latest completed run's headSha) and renders a single CommitDivider below the run list. - CommitDivider is a masked-SVG zigzag rule flanking a commit-id chip (reusing the scope-badge chip vocabulary + CommitIcon); theme-aware via a token color. - i18n: chatPane.commitDividerTitle in all four locales. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../main/services/pr-agent/run-executor.ts | 2 ++ .../src/components/features/chat/ChatPane.tsx | 27 ++++++++++++++++-- .../chat/components/CommitDivider.tsx | 28 +++++++++++++++++++ .../src/renderer/src/i18n/locales/de-DE.json | 1 + .../src/renderer/src/i18n/locales/en-US.json | 1 + .../src/renderer/src/i18n/locales/ja-JP.json | 1 + .../src/renderer/src/i18n/locales/zh-CN.json | 1 + .../src/styles/features/chat/pane.scss | 27 ++++++++++++++++++ packages/poller/src/runs.ts | 3 ++ packages/shared/src/poller-contract.ts | 7 +++++ 10 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx diff --git a/apps/desktop/src/main/services/pr-agent/run-executor.ts b/apps/desktop/src/main/services/pr-agent/run-executor.ts index c7e28658..398e0234 100644 --- a/apps/desktop/src/main/services/pr-agent/run-executor.ts +++ b/apps/desktop/src/main/services/pr-agent/run-executor.ts @@ -265,6 +265,8 @@ export class RunExecutor { origin: item.priority, // Single-commit review scope persisted with the run: the result card uses it to show a scope badge. scope: req.scope, + // PR head commit the run runs against: stamped so ChatPane can draw a commit divider when the code changes between runs. + headSha: pr.sourceRef.sha, }); // Upgrade the info (startedAt=null at enqueue) to active form + broadcast (via the scheduling layer). item.info = { ...item.info, startedAt: run.startedAt }; diff --git a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx index 3399984b..b3f976a1 100644 --- a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx +++ b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import type { Finding, @@ -24,6 +24,7 @@ import { useChatActions } from './hooks/useChatActions'; import { useChatTimeline } from './hooks/useChatTimeline'; import { AgentStepRow, ThinkingLive } from './components/AgentStep'; import { ChatEmpty } from './components/ChatEmpty'; +import { CommitDivider } from './components/CommitDivider'; import { ChatInputBar } from './components/ChatInputBar'; import { ConversationMessage } from './components/ConversationMessage'; import { PlanPanel } from './components/PlanPanel'; @@ -241,12 +242,31 @@ export function ChatPane({ prLocalId, }); + // Commit divider: the PR head SHA the most recent completed run reviewed. When the current PR head has advanced past + // it, a single sawtooth divider is shown at the bottom of the timeline marking the new head — signalling prior + // reviews are now stale, even if no run has been started against the new code yet. Suppressed while a run is active + // (it's already processing the current head). Returns the new head SHA to mark, or null when nothing is stale. + const staleHeadSha = useMemo(() => { + if (hasMyActive) return null; + const head = pr?.sourceRef.sha; + if (!head) return null; + // Timeline is ascending, so the last run entry carrying a headSha is the most recent completed review. + let lastRunHeadSha: string | undefined; + for (const entry of timeline) { + if (entry.run?.headSha) lastRunHeadSha = entry.run.headSha; + } + // No baseline run with a recorded head (e.g. only pre-feature runs) → nothing to be stale against. + if (!lastRunHeadSha) return null; + return head !== lastRunHeadSha ? head : null; + }, [timeline, hasMyActive, pr?.sourceRef.sha]); + // Pure UI state: rule preview modal / clear confirm modal / merge confirm modal const [showRulePreview, setShowRulePreview] = useState(false); const [showClearConfirm, setShowClearConfirm] = useState(false); const [showMergeConfirm, setShowMergeConfirm] = useState(false); - const { runs, error, loadingSession, matchedRules, bodyRef, hasMoreOlder, loadingOlder } = session; + const { runs, error, loadingSession, matchedRules, bodyRef, hasMoreOlder, loadingOlder } = + session; // Re-review card ↔ original finding card cross-link: scroll to and briefly highlight. The flash class differs by target: run cards use chat-run-flash // (fading background, visible on the run card's transparent base); finding cards use chat-finding-flash (an overlay highlight ring — finding cards have @@ -396,6 +416,9 @@ export function ChatPane({ ) : null, )} + {/* Commit divider: the PR head advanced past the last reviewed commit → mark the new head at the bottom of the + run list (prior reviews are stale). Shown even when no run has been started against the new code yet. */} + {staleHeadSha && } {/* This PR's queued tasks: placed after running ones, each cancellable individually. The position uses the **global** queue order (the queue is shared across PRs, otherwise every PR showing "position 1" would be misleading) — the runId's index in the global waiting array +1. */} {myWaiting.map((w) => ( diff --git a/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx b/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx new file mode 100644 index 00000000..e1385cbb --- /dev/null +++ b/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx @@ -0,0 +1,28 @@ +import { useTranslation } from 'react-i18next'; +import { CommitIcon } from '../../../common'; + +/** + * Sawtooth "commit divider" shown at the bottom of the run timeline when the PR head has advanced past the commit the + * most recent run reviewed (see ChatPane staleHeadSha). It marks the new head — signalling that the reviews above are + * now based on stale code — even if no run has been started against the new commit yet. The label is the abbreviated + * commit id (the full SHA is in the tooltip), reusing the same chip vocabulary as the single-commit scope badge in + * RunResultView. + */ +export function CommitDivider({ sha }: { sha: string }) { + const { t } = useTranslation(); + const short = sha.slice(0, 8); + const title = t('chatPane.commitDividerTitle', { sha }); + return ( +
+
+ ); +} diff --git a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json index c078be57..f961ceec 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json +++ b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json @@ -71,6 +71,7 @@ "codeExistingAria": "Originalcode", "codeImprovedAria": "Verbesserter Code", "commandNoArgs": "{{cmd}} akzeptiert keine Argumente", + "commitDividerTitle": "Code seit der letzten Review geändert — jetzt bei Commit {{sha}}", "deleteRunAria": "Löschen", "deleteRunTitle": "Diesen Lauf löschen", "draftJumpEditTitle": "Im Code bearbeiten", diff --git a/apps/desktop/src/renderer/src/i18n/locales/en-US.json b/apps/desktop/src/renderer/src/i18n/locales/en-US.json index b1174899..35cab87f 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/en-US.json +++ b/apps/desktop/src/renderer/src/i18n/locales/en-US.json @@ -71,6 +71,7 @@ "codeExistingAria": "Original code", "codeImprovedAria": "Improved code", "commandNoArgs": "{{cmd}} does not accept arguments", + "commitDividerTitle": "Code changed since the last review — now at commit {{sha}}", "deleteRunAria": "Delete", "deleteRunTitle": "Delete this run", "draftJumpEditTitle": "Edit in code", diff --git a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json index 3f6a3749..9b6452f7 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json +++ b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json @@ -71,6 +71,7 @@ "codeExistingAria": "元のコード", "codeImprovedAria": "改善後のコード", "commandNoArgs": "{{cmd}} は引数を受け付けません", + "commitDividerTitle": "前回のレビュー以降にコードが変更されました — 現在はコミット {{sha}}", "deleteRunAria": "削除", "deleteRunTitle": "この記録を削除", "draftJumpEditTitle": "コード内で編集", diff --git a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json index 9d3b61b5..05c53c5d 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json +++ b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json @@ -71,6 +71,7 @@ "codeExistingAria": "原代码", "codeImprovedAria": "改进代码", "commandNoArgs": "{{cmd}} 不接受参数", + "commitDividerTitle": "自上次评审后代码已变更 —— 当前为提交 {{sha}}", "deleteRunAria": "删除", "deleteRunTitle": "删除此记录", "draftJumpEditTitle": "在代码中编辑", diff --git a/apps/desktop/src/renderer/src/styles/features/chat/pane.scss b/apps/desktop/src/renderer/src/styles/features/chat/pane.scss index 10b5f670..f2a4aaa8 100644 --- a/apps/desktop/src/renderer/src/styles/features/chat/pane.scss +++ b/apps/desktop/src/renderer/src/styles/features/chat/pane.scss @@ -102,6 +102,33 @@ border-bottom: 1px dashed $border-muted; } +// "Commit divider": a sawtooth rule inserted into the run timeline where the PR code changed between two runs (see +// ChatPane commitDividers). A zigzag line flanks a centered commit-id chip, marking the newer commit the runs below it +// ran against, so history runs on an older revision read as a separate section. The zigzag is a masked SVG tile, so its +// color follows the theme token ($border-muted) via background-color and it renders correctly in both light and dark. +$commit-zigzag: url("data:image/svg+xml,%3Csvg%20xmlns='http://www.w3.org/2000/svg'%20width='12'%20height='8'%20viewBox='0%200%2012%208'%3E%3Cpath%20d='M0%206%20L3%202%20L6%206%20L9%202%20L12%206'%20fill='none'%20stroke='black'%20stroke-width='1.4'%20stroke-linejoin='round'/%3E%3C/svg%3E"); +.chat-commit-divider { + display: flex; + align-items: center; + gap: $space-3; + margin: $space-6 $space-2 $space-3; + user-select: none; + + &__line { + flex: 1 1 auto; + height: 8px; + background-color: $border-muted; + opacity: 0.75; + -webkit-mask: $commit-zigzag repeat-x center / auto 100%; + mask: $commit-zigzag repeat-x center / auto 100%; + } + + // Keep the commit-id chip at its intrinsic width; only the flanking zigzag lines flex. + &__chip { + flex: 0 0 auto; + } +} + // Rule chip matched by the current PR: a row below the actions bar, clicking pops up a body preview .chat-rule-chip { display: flex; diff --git a/packages/poller/src/runs.ts b/packages/poller/src/runs.ts index 6faf6311..910cff0a 100644 --- a/packages/poller/src/runs.ts +++ b/packages/poller/src/runs.ts @@ -61,6 +61,8 @@ export interface StartReviewRunInput { origin?: ReviewRun['origin']; /** Single-commit review scope (parent..sha); omitted = full PR scope. Persisted for the result card's scope badge. */ scope?: ReviewRun['scope']; + /** PR head commit SHA the run runs against (pr.sourceRef.sha); persisted so ChatPane can draw a commit divider between runs across a code change. */ + headSha?: string; } /** Write the initial running state; callers must start before invoking pr-agent. */ @@ -81,6 +83,7 @@ export async function startReviewRun( referencedFinding: input.referencedFinding, origin: input.origin, scope: input.scope, + headSha: input.headSha, status: 'running', startedAt: at.toISOString(), }; diff --git a/packages/shared/src/poller-contract.ts b/packages/shared/src/poller-contract.ts index 66ea2824..5b6ecd5b 100644 --- a/packages/shared/src/poller-contract.ts +++ b/packages/shared/src/poller-contract.ts @@ -314,6 +314,13 @@ export interface ReviewRun { * Default = whole-PR scope. The result card shows a scope badge accordingly. */ scope?: ReviewRunCommitScope; + /** + * PR head commit SHA (`pr.sourceRef.sha`) the run was executed against, stamped at start. Used only for the ChatPane + * "commit divider": when two consecutive runs have differing headSha, the code changed between them, and a sawtooth + * divider marking the newer commit is inserted between the older (history) runs and the newer ones. Historical runs + * predating this field are undefined → no divider is drawn for them (compared only between two defined, differing SHAs). + */ + headSha?: string; /** The pr-agent version obtained at probe time (CLI first line / the pr-agent version found by the embedded runtime) */ prAgentVersion: string; strategy: PrAgentStrategy; From 21a9bf2ce40c3f80de423dd0d8eca5cb30701d10 Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Mon, 27 Jul 2026 13:35:30 +0800 Subject: [PATCH 06/11] feat(agent): make the planning context aware of PR commit changes The planning/orchestration agent replays the whole prior conversation into each turn, which can reference code from before a new commit landed. Give it soft awareness of that boundary without dropping any history: stamp the PR head SHA (pr.sourceRef.sha) onto each recorded conversation message, and in buildConversationContext interleave a "PR code updated to commit X" marker wherever the head differs between consecutive kept messages, plus a trailing marker when the current turn's head has advanced past the newest message. Mirrors the head SHA already stamped on runs (same pr.sourceRef.sha basis as the UI commit divider), so seeing the divider in the UI corresponds to the marker in the agent's context. Marker is an internal prompt artifact (English, no i18n) and, like the rest of the conversation history, is never passed through to pr-agent tools. Messages predating the field have no headSha and produce no marker. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/main/services/agent/planning.ts | 17 +++++++- packages/agent/src/planner.ts | 30 ++++++++++++-- packages/agent/src/steps/planning/shared.ts | 41 ++++++++++++++++--- packages/shared/src/agent-contract.ts | 7 ++++ 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/main/services/agent/planning.ts b/apps/desktop/src/main/services/agent/planning.ts index ef420bc8..5e00a117 100644 --- a/apps/desktop/src/main/services/agent/planning.ts +++ b/apps/desktop/src/main/services/agent/planning.ts @@ -120,7 +120,13 @@ export async function runPlanning( deps.stateStore, pr.localId, // When a Diff selection reference is present, persist it alongside, for the UI to show the "referenced code" collapsed below the bubble. - { role: 'user', content: userRequest, referencedContext: deps.referencedContext }, + // headSha stamps the PR head this turn was made against, so a later turn on a newer commit gets a "code changed" marker in the planning context. + { + role: 'user', + content: userRequest, + referencedContext: deps.referencedContext, + headSha: pr.sourceRef.sha, + }, now, ); @@ -161,6 +167,8 @@ export async function runPlanning( summarySections: buildSummarySections(), userRequest, history, + // Current PR head, so the planning context can flag a code change since the newest historical turn. + currentHeadSha: pr.sourceRef.sha, referencedContext: deps.referencedContext, maxSteps: deps.maxSteps, maxFollowupAsks: deps.maxFollowupAsks, @@ -172,7 +180,12 @@ export async function runPlanning( await appendAgentMessage( deps.stateStore, pr.localId, - { role: 'assistant', content: result.finalText, recommendation: result.recommendation }, + { + role: 'assistant', + content: result.finalText, + recommendation: result.recommendation, + headSha: pr.sourceRef.sha, + }, now, ); } diff --git a/packages/agent/src/planner.ts b/packages/agent/src/planner.ts index 49dd7ff8..5514d5e0 100644 --- a/packages/agent/src/planner.ts +++ b/packages/agent/src/planner.ts @@ -8,7 +8,11 @@ import type { } from '@meebox/shared'; import { assembleSystemContext, type AssemblePrMeta } from './prompts.js'; import type { MemoryNote } from './memory.js'; -import { DEFAULT_STEP_LABELS, DEFAULT_SUMMARY_SECTIONS, type AgentStepLabels } from './orchestrator.js'; +import { + DEFAULT_STEP_LABELS, + DEFAULT_SUMMARY_SECTIONS, + type AgentStepLabels, +} from './orchestrator.js'; import { createStepRecorder } from './steps/context.js'; import { buildConversationContext, @@ -69,6 +73,12 @@ export interface PlanningInput { * agent remembers earlier exchanges across rounds; **never** passed through to pr-agent tools (tools only see PR + this round's question). */ history?: AgentMessage[]; + /** + * Current PR head commit SHA. When it differs from the newest historical message's headSha (or between historical + * turns), buildConversationContext injects a "code changed to commit X" marker so the agent knows earlier discussion + * may reference outdated code. Context awareness only — never passed through to pr-agent tools. + */ + currentHeadSha?: string; /** * Code reference selected by the user in the diff (self-describing block). Injected into this round's planning context so the agent knows which code the user is looking at; * **never** passed through to pr-agent tools (same constraint as history). Omitted = no selection reference this round. @@ -130,7 +140,7 @@ export async function runPlanningAgent( // Inject prior multi-turn conversation into the planning context (trimmed by budget) so the agent remembers exchanges across rounds; only for the planning LLM's reference, // never passed through to pr-agent tools. - const convo = buildConversationContext(input.history ?? []); + const convo = buildConversationContext(input.history ?? [], input.currentHeadSha); const ctx: PlanStepCtx = { deps, input, @@ -149,7 +159,13 @@ export async function runPlanningAgent( for (let i = 0; i < maxSteps; i++) { const outcome = await planCycleStep.run(ctx); if (outcome.kind === 'aborted') { - return { steps: rec.steps, finalText: '', tokenUsage: rec.usage, memories, terminationReason: 'aborted' }; + return { + steps: rec.steps, + finalText: '', + tokenUsage: rec.usage, + memories, + terminationReason: 'aborted', + }; } if (outcome.kind === 'final') { return { @@ -162,5 +178,11 @@ export async function runPlanningAgent( } } - return { steps: rec.steps, finalText: '', tokenUsage: rec.usage, memories, terminationReason: 'max_steps' }; + return { + steps: rec.steps, + finalText: '', + tokenUsage: rec.usage, + memories, + terminationReason: 'max_steps', + }; } diff --git a/packages/agent/src/steps/planning/shared.ts b/packages/agent/src/steps/planning/shared.ts index b3f16858..7b61587c 100644 --- a/packages/agent/src/steps/planning/shared.ts +++ b/packages/agent/src/steps/planning/shared.ts @@ -105,18 +105,49 @@ export function normalizePlan(raw: PlannerAction['plan']): AgentTodoItem[] { return out; } -/** Take the most recent rounds, each length-capped, and trim newest-to-oldest by total budget (drop earlier over-budget messages), returning text in ascending time order. */ -export function buildConversationContext(history: readonly AgentMessage[]): string { - const lines: string[] = []; +/** Boundary marker injected into the conversation context where the PR head commit changed, so the planning agent knows + * messages above it were made against older code (soft awareness — no history is dropped). Internal prompt artifact → English. */ +function commitChangeMarker(sha: string): string { + return `[--- PR code updated to commit ${sha.slice(0, 8)} at this point; messages below are based on the new code, earlier ones may reference outdated code ---]`; +} + +/** + * Take the most recent rounds, each length-capped, and trim newest-to-oldest by total budget (drop earlier over-budget + * messages), returning text in ascending time order. Where a message's `headSha` differs from the previous kept + * message's, a {@link commitChangeMarker} is interleaved so the agent perceives the code changed between those turns; + * a trailing marker is added when `currentHeadSha` (this turn's head) has advanced past the newest kept message. + * Messages without a recorded headSha (predating the field) never produce a marker. + */ +export function buildConversationContext( + history: readonly AgentMessage[], + currentHeadSha?: string, +): string { + // First pass, newest→oldest: keep the messages that fit the budget. + const kept: AgentMessage[] = []; let budget = HISTORY_BUDGET_CHARS; for (let i = history.length - 1; i >= 0; i--) { const m = history[i]!; const line = `${m.role === 'user' ? 'User' : 'Assistant'}: ${clamp(m.content, HISTORY_MESSAGE_MAX)}`; if (line.length + 1 > budget) break; // budget exhausted: trim the earlier conversation entirely budget -= line.length + 1; - lines.push(line); + kept.push(m); + } + kept.reverse(); // ascending time order + // Second pass, ascending: render each message, inserting a commit-change marker at head-commit boundaries. + const lines: string[] = []; + let prevSha: string | undefined; + for (const m of kept) { + if (m.headSha && prevSha && m.headSha !== prevSha) lines.push(commitChangeMarker(m.headSha)); + if (m.headSha) prevSha = m.headSha; + lines.push( + `${m.role === 'user' ? 'User' : 'Assistant'}: ${clamp(m.content, HISTORY_MESSAGE_MAX)}`, + ); + } + // Trailing marker: the current turn's code has advanced past the newest historical message. + if (currentHeadSha && prevSha && currentHeadSha !== prevSha) { + lines.push(commitChangeMarker(currentHeadSha)); } - return lines.reverse().join('\n'); + return lines.join('\n'); } /** Planning ReAct protocol: body is externalized in resources/prompts/protocol.md, the three section titles (localized per language) are injected via placeholders. */ diff --git a/packages/shared/src/agent-contract.ts b/packages/shared/src/agent-contract.ts index 1a6465f8..e6cc6bf4 100644 --- a/packages/shared/src/agent-contract.ts +++ b/packages/shared/src/agent-contract.ts @@ -82,6 +82,13 @@ export interface AgentMessage { * not set when there is no selection / for assistant messages. (Finding references go through the /ask run card, not this field.) */ referencedContext?: string; + /** + * PR head commit SHA (`pr.sourceRef.sha`) at the time this turn was recorded. Used by the planning agent's context + * assembly (buildConversationContext) to inject a "code changed to commit X" marker between turns whose head differs, + * so the agent perceives that earlier discussion may reference outdated code — without dropping any history. Messages + * predating this field are undefined and simply produce no marker. + */ + headSha?: string; /** Creation time (ISO), used for timeline ordering. */ at: string; } From 016fad1a94498ce373d5c935700e3d1b1f505a11 Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Mon, 27 Jul 2026 17:02:17 +0800 Subject: [PATCH 07/11] feat(pr): F5 refreshes the current PR; move auto review to Ctrl+F5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bare F5 used to kick off an auto review, which was easy to trigger by accident. Rework the PR refresh + shortcuts around a single-PR refresh: - F5 now refreshes the currently selected PR; Ctrl+F5 runs auto review (literal Ctrl on every platform — ⌘F5 collides with macOS VoiceOver; formatChord gains a `ctrl` flag so the palette hint shows ⌃ on mac). - Add a neutral (non-accent) icon refresh button to the right of the PR header's "open in browser" button; align-self:stretch matches its height. - Add a "Refresh PR" command palette entry (shown only when a PR is selected), bound to bare F5. Refresh targets a SINGLE PR, not a whole-poller tick: new prs:refreshOne IPC + refreshOnePr controller re-fetch just this PR via getSinglePullRequest, recompute localStatus from the current user's reviewer status (same rule as the poll), persist its meta, and invalidateCommentsCache (comments:changed → comments/inline-diff refetch); if the head sha advanced, best-effort sync the mirror for the diff. The one network call is this PR's fetch — usePullRequests .refreshPr then reloads the list locally (no poll of other PRs). The header button, F5, and the command all wire to it. i18n in all four locales. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/desktop/src/main/controllers/pr.ts | 59 +++++++++++++++++++ apps/desktop/src/main/ipc.ts | 1 + apps/desktop/src/renderer/src/App.tsx | 22 +++++-- .../command-palette/CommandPalette.tsx | 5 ++ .../features/command-palette/commands/pr.ts | 15 +++++ .../command-palette/commands/review.ts | 3 +- .../command-palette/commands/shortcuts.ts | 9 ++- .../command-palette/commands/types.ts | 2 + .../src/components/features/pr/PrHeader.tsx | 20 ++++++- .../src/components/features/pr/PrPanel.tsx | 8 +++ .../features/pr/hooks/usePullRequests.ts | 23 ++++++++ .../src/components/layout/TitleBar.tsx | 3 + .../renderer/src/hooks/useGlobalShortcuts.ts | 29 ++++++--- .../src/renderer/src/i18n/locales/de-DE.json | 2 + .../src/renderer/src/i18n/locales/en-US.json | 2 + .../src/renderer/src/i18n/locales/ja-JP.json | 2 + .../src/renderer/src/i18n/locales/zh-CN.json | 2 + .../styles/features/pr/header-actions.scss | 7 +++ packages/ipc/src/pr.ts | 10 ++++ 19 files changed, 208 insertions(+), 16 deletions(-) diff --git a/apps/desktop/src/main/controllers/pr.ts b/apps/desktop/src/main/controllers/pr.ts index 5ebc6452..106c6467 100644 --- a/apps/desktop/src/main/controllers/pr.ts +++ b/apps/desktop/src/main/controllers/pr.ts @@ -314,6 +314,65 @@ export const openPrByUrl: IpcController<'prs:openByUrl'> = async (_event, req) = */ export const refreshPrs: IpcController<'prs:refresh'> = () => getContext().poller.tick(); +/** + * Refresh a SINGLE PR from remote (metadata + comments) without a whole-poller tick. Fetches just this PR via the + * adapter (bypassing the discovery list), recomputes localStatus from the current user's reviewer status (remote + * authoritative, same rule as the poll), persists the updated meta, and invalidates the comments cache (broadcasts + * comments:changed → the open comments / activity / inline-diff refetch). If the head sha advanced, best-effort ensure + * the mirror has the new commits so the diff renders the new code. Returns the updated PR; the renderer then reloads the + * list locally (no network poll of other PRs). + */ +export const refreshOnePr: IpcController<'prs:refreshOne'> = async (_event, req) => { + const ctx = getContext(); + const existing = await ctx.pr.findPrOrThrow(req.localId); + const adapter = ctx.pr.adapterForOrThrow(existing); + // Remote fetch of just this PR. 403/404 normalize to error codes (matching openPrByUrl); other errors bubble up. + let fresh; + try { + fresh = await adapter.prs.getSinglePullRequest( + { projectKey: existing.repo.projectKey, repoSlug: existing.repo.repoSlug }, + existing.remoteId, + ); + } catch (err) { + const status = (err as { status?: number } | null)?.status; + if (status === 403) throw new AppError(ERROR_CODES.PR_FORBIDDEN, undefined, 'forbidden'); + if (status === 404) throw new AppError(ERROR_CODES.PR_NOT_FOUND, undefined, 'not found'); + throw err; + } + // localStatus mirrors the remote current user's reviewer status (remote authoritative, same mapping as the poll); + // when the current user is unknown (ping incomplete) keep the recorded status rather than downgrading to pending. + const me = adapter.connection.getCurrentUser(); + const mineStatus = me ? fresh.reviewers.find((r) => r.name === me.name)?.status : undefined; + const localStatus = !me + ? existing.localStatus + : mineStatus === 'approved' + ? 'approved' + : mineStatus === 'needsWork' + ? 'needs_work' + : 'pending'; + const stored: StoredPullRequest = { + ...fresh, + localId: existing.localId, + platform: existing.platform, + connectionId: existing.connectionId, + localStatus, + // Preserve local-only bookkeeping (a single-PR refresh isn't a discovery pass). + discoveryFilters: existing.discoveryFilters, + discoveredAt: existing.discoveredAt, + lastSeenAt: new Date().toISOString(), + }; + await writePrMeta(await ctx.pr.storeForPr(req.localId), req.localId, stored); + await ctx.pr.invalidateCommentsCache(req.localId); + if (fresh.sourceRef.sha !== existing.sourceRef.sha) { + try { + await ctx.pr.ensureMirrorReadyForPr(stored); + } catch { + /* non-fatal: the diff view self-heals / surfaces a readable error if the mirror still lacks the sha */ + } + } + return stored; +}; + /** * The Poller's most recent completion time (used for startup initialization). */ diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 736ace14..c01522a5 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -76,6 +76,7 @@ export function registerIpcHandlers(deps: RegisterDeps): { ipcMain.handle('prs:listArchived', pr.listArchivedPrs); // Closed (archived) PR list (read-only browsing) ipcMain.handle('prs:openByUrl', pr.openPrByUrl); // Open a current-platform PR by URL (locate / fetch archive) ipcMain.handle('prs:refresh', pr.refreshPrs); // Poll and refresh immediately + ipcMain.handle('prs:refreshOne', pr.refreshOnePr); // Refresh a single PR from remote (no whole-poller tick) ipcMain.handle('prs:lastSync', pr.getLastSync); // Most recent sync time ipcMain.handle('prs:setLocalStatus', pr.setPrStatus); // Set review status (remote first, then local) ipcMain.handle('prs:markRead', pr.markRead); // Mark PR read (advance unread watermark) diff --git a/apps/desktop/src/renderer/src/App.tsx b/apps/desktop/src/renderer/src/App.tsx index 8bc011ba..7bd1673d 100644 --- a/apps/desktop/src/renderer/src/App.tsx +++ b/apps/desktop/src/renderer/src/App.tsx @@ -36,16 +36,25 @@ export default function App() { selectedId, setSelectedId, refreshing, + refreshingPr, merging, reloadPrs, triggerRefresh, + refreshPr, setSelectedPrStatus, mergeSelectedPr, markRead, } = usePullRequests({ notifyError }); // App startup / global lifecycle (boot load, language, poll / focus refresh, wizard completion, connection hot-apply) - const { boot, fatalError, lastSyncAt, needsOnboarding, completeOnboarding, refreshBootAndPrs, patchConfig } = - useBootstrap({ setPrs, reloadPrs }); + const { + boot, + fatalError, + lastSyncAt, + needsOnboarding, + completeOnboarding, + refreshBootAndPrs, + patchConfig, + } = useBootstrap({ setPrs, reloadPrs }); // Layout state (left/right column widths / collapse), version update notice, store wiring, external link guard — each its own app-level hook const { sidebarWidth, @@ -128,12 +137,13 @@ export default function App() { // "Can engage" determination for the closed scope: merged / still-open PRs allow adding comments + AI review; declined ones are browse-only. The active scope is always engageable. const canEngage = !archived || (selectedPr ? selectedPr.state !== 'declined' : false); - // Window-level global shortcuts (F5 auto review / DevTools / view closed / Ctrl-Cmd+B·J layout toggles) — domain logic lives in useGlobalShortcuts. + // Window-level global shortcuts (F5 refresh / Ctrl+F5 auto review / DevTools / view closed / Ctrl-Cmd+B·J layout toggles) — domain logic lives in useGlobalShortcuts. useGlobalShortcuts({ platform: boot?.info.platform, selectedId, canEngage, viewArchived, + refreshPr: (localId) => void refreshPr(localId), setSidebarCollapsed, setChatCollapsed, }); @@ -177,7 +187,8 @@ export default function App() { const showDiscoveryFilter = availableDiscoveryFilters.length > 0; // Whether the platform supports the needs_work ("needs changes") review state: GitHub / Bitbucket support it, GitLab (binary approval) does not. // Determines whether the "pending" status filter is kept under discovery categories other than "awaiting my review" (see Sidebar.visibleFilters). - const supportsNeedsWork = activeConnSummary?.capabilities.reviewStatuses.includes('needsWork') ?? false; + const supportsNeedsWork = + activeConnSummary?.capabilities.reviewStatuses.includes('needsWork') ?? false; // The selected category may be invalid for the current platform after switching connections → fall back to the first available. const effectiveDiscoveryFilter = availableDiscoveryFilters.includes(discoveryFilter) ? discoveryFilter @@ -204,6 +215,7 @@ export default function App() { prStatusFilters={visibleStatusFilters} setPrStatusFilter={setStatusFilter} viewArchived={viewArchived} + refreshPr={(localId) => void refreshPr(localId)} openPrByUrl={openPrByUrl} />
@@ -238,6 +250,8 @@ export default function App() { onSetStatus={(s) => void setSelectedPrStatus(s)} onMerge={() => void mergeSelectedPr()} merging={merging} + onRefresh={() => void refreshPr(selectedPr.localId)} + refreshing={refreshingPr} capabilities={selectedConn?.capabilities} currentUserName={selectedConn?.user?.name ?? null} // The closed scope hides PR lifecycle actions (merge / approve); declined / not-engageable further hides comment / draft writes. diff --git a/apps/desktop/src/renderer/src/components/features/command-palette/CommandPalette.tsx b/apps/desktop/src/renderer/src/components/features/command-palette/CommandPalette.tsx index f07eee57..67b7c9eb 100644 --- a/apps/desktop/src/renderer/src/components/features/command-palette/CommandPalette.tsx +++ b/apps/desktop/src/renderer/src/components/features/command-palette/CommandPalette.tsx @@ -24,6 +24,8 @@ interface CommandPaletteProps { setDiscoveryFilter: (filter: PrDiscoveryFilter) => void; /** Switch to the "closed" (archived) scope (used by the PR-domain "view closed" command). */ viewArchived: () => void; + /** Refresh a single PR by localId (used by the PR-domain "Refresh PR" command for the selected PR). */ + refreshPr: (localId: string) => void; /** Open a PR of the current platform by URL (used by the PR-domain "open URL" free-text command). */ openPrByUrl: (url: string) => void | Promise; /** Selectable PR status filters (used by the PR-domain "filter by category" second-level options). */ @@ -88,6 +90,7 @@ export function CommandPalette({ discoveryFilters, setDiscoveryFilter, viewArchived, + refreshPr, openPrByUrl, prStatusFilters, setPrStatusFilter, @@ -126,6 +129,7 @@ export function CommandPalette({ discoveryFilters, setDiscoveryFilter, viewArchived, + refreshPr, openPrByUrl, prStatusFilters, setPrStatusFilter, @@ -145,6 +149,7 @@ export function CommandPalette({ discoveryFilters, setDiscoveryFilter, viewArchived, + refreshPr, openPrByUrl, prStatusFilters, setPrStatusFilter, diff --git a/apps/desktop/src/renderer/src/components/features/command-palette/commands/pr.ts b/apps/desktop/src/renderer/src/components/features/command-palette/commands/pr.ts index 3c4b96a2..538c0215 100644 --- a/apps/desktop/src/renderer/src/components/features/command-palette/commands/pr.ts +++ b/apps/desktop/src/renderer/src/components/features/command-palette/commands/pr.ts @@ -93,5 +93,20 @@ export function buildPrCommands(ctx: CommandContext): RootCommand[] { run: () => ctx.togglePrList(), }); + // Refresh PR: re-fetch just the selected PR from remote (metadata + comments), not a whole-poller tick. Only shown + // when a PR is selected. Bound to bare F5 (real key match in App's window-level listener); shortcut here is display-only. + out.push({ + id: 'refresh-pr', + category, + categoryEn, + title: t('commandPalette.cmdRefreshPr'), + titleEn: tEn('commandPalette.cmdRefreshPr'), + when: () => Boolean(ctx.selectedPrId), + shortcut: ['F5'], + run: () => { + if (ctx.selectedPrId) ctx.refreshPr(ctx.selectedPrId); + }, + }); + return out; } diff --git a/apps/desktop/src/renderer/src/components/features/command-palette/commands/review.ts b/apps/desktop/src/renderer/src/components/features/command-palette/commands/review.ts index d515b9b5..fecf6ccb 100644 --- a/apps/desktop/src/renderer/src/components/features/command-palette/commands/review.ts +++ b/apps/desktop/src/renderer/src/components/features/command-palette/commands/review.ts @@ -34,7 +34,8 @@ export function buildReviewCommands(ctx: CommandContext): RootCommand[] { // Gating: only appears when a PR is selected (meaningless without one). Reentrancy guard at execution: ignore if the same PR is already running. Uses the same channel as ChatPane's // one-click review; run state / session reflected via events + store; LLM not configured / pr-agent not ready flow back into the session as a failure from the backend. when: () => Boolean(selectedPrId), - shortcut: ['F5'], // Run (IDE convention); single key avoids combo conflicts, see App window-level shortcuts + // Ctrl+F5 (literal Ctrl on all platforms): moved off bare F5 so an accidental F5 doesn't kick off a review; bare F5 is now "Refresh PR". See App window-level shortcuts. + shortcut: formatChord(ctx.platform, 'F5', { ctrl: true }), run: () => { if (selectedPrId && !isPrRunning(selectedPrId)) { void invoke('agent:run', { localId: selectedPrId }); diff --git a/apps/desktop/src/renderer/src/components/features/command-palette/commands/shortcuts.ts b/apps/desktop/src/renderer/src/components/features/command-palette/commands/shortcuts.ts index 6c07ed22..b104f8f7 100644 --- a/apps/desktop/src/renderer/src/components/features/command-palette/commands/shortcuts.ts +++ b/apps/desktop/src/renderer/src/components/features/command-palette/commands/shortcuts.ts @@ -8,11 +8,14 @@ import type { Platform } from '@meebox/shared'; export function formatChord( platform: Platform, key: string, - mods: { shift?: boolean; alt?: boolean } = {}, + mods: { shift?: boolean; alt?: boolean; ctrl?: boolean } = {}, ): string[] { const mac = platform === 'darwin'; + // Primary modifier: normally ⌘ (mac) / Ctrl (other). With `ctrl`, force the literal Control key on both — mac shows ⌃ + // instead of ⌘ — for bindings that match `e.ctrlKey` on every platform (e.g. Ctrl+F5, where ⌘F5 would hit macOS VoiceOver). + const primary = mac ? (mods.ctrl ? '⌃' : '⌘') : 'Ctrl'; const tokens = mac - ? [mods.alt && '⌥', mods.shift && '⇧', '⌘', key] - : ['Ctrl', mods.shift && 'Shift', mods.alt && 'Alt', key]; + ? [mods.alt && '⌥', mods.shift && '⇧', primary, key] + : [primary, mods.shift && 'Shift', mods.alt && 'Alt', key]; return tokens.filter((x): x is string => Boolean(x)); } diff --git a/apps/desktop/src/renderer/src/components/features/command-palette/commands/types.ts b/apps/desktop/src/renderer/src/components/features/command-palette/commands/types.ts index 2f7d80ad..844f3cd5 100644 --- a/apps/desktop/src/renderer/src/components/features/command-palette/commands/types.ts +++ b/apps/desktop/src/renderer/src/components/features/command-palette/commands/types.ts @@ -26,6 +26,8 @@ export interface CommandContext { setDiscoveryFilter: (filter: PrDiscoveryFilter) => void; /** Switch to the "closed" (archived) scope (used by the PR-domain "view closed" command). */ viewArchived: () => void; + /** Refresh a single PR by localId (re-fetch that PR from remote; used by the PR-domain "Refresh PR" command for the selected PR, also bound to bare F5). */ + refreshPr: (localId: string) => void; /** Open a PR of the current platform by URL (used by the PR-domain "open URL" free-text command): locate locally or fetch the archive then jump, popping a toast on failure. */ openPrByUrl: (url: string) => void | Promise; /** Optional PR status filter items (pending / all / conflict / mergeable, etc., already gated by platform). */ diff --git a/apps/desktop/src/renderer/src/components/features/pr/PrHeader.tsx b/apps/desktop/src/renderer/src/components/features/pr/PrHeader.tsx index 4d24d41c..74c80798 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/PrHeader.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/PrHeader.tsx @@ -6,7 +6,7 @@ import type { ReviewerStatus, StoredPullRequest, } from '@meebox/shared'; -import { ApproveIcon, GlobeIcon, NeedsWorkIcon, PullRequestIcon } from '../../common'; +import { ApproveIcon, GlobeIcon, NeedsWorkIcon, PullRequestIcon, RetryIcon } from '../../common'; import { ReviewerStack } from './ReviewerStack'; /** @@ -19,6 +19,8 @@ export function PrHeader({ currentUserName, merging, onMerge, + onRefresh, + refreshing = false, onSetStatus, hideLifecycle = false, readOnly = false, @@ -30,6 +32,10 @@ export function PrHeader({ currentUserName?: string | null; merging: boolean; onMerge: () => void; + /** Refresh PRs (re-poll + reload); wired to the neutral refresh button beside "open in browser". */ + onRefresh: () => void; + /** Whether a refresh is in flight (disables the refresh button). */ + refreshing?: boolean; onSetStatus: (status: LocalPrStatus) => void; /** Hide PR lifecycle actions (merge + review decision): always set for the closed scope. */ hideLifecycle?: boolean; @@ -106,6 +112,18 @@ export function PrHeader({ > {t('mainPane.openInBrowser')} + {/* Refresh (re-poll + reload): neutral icon button (no accent fill) beside "open in browser"; disabled while a refresh is in flight. Also bound to F5. */} + {/* approve / needs work: current status = highlighted; clicking an already-highlighted one falls back to pending (revokes the remote mark). "Publish comments (N)" sits to the left of the decision buttons — reviewing is two steps: post comments first (left), then make the decision (right). */}
diff --git a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx index 53b8d5da..04155f61 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx +++ b/apps/desktop/src/renderer/src/components/features/pr/PrPanel.tsx @@ -28,6 +28,10 @@ export interface PrPanelProps { onSetStatus: (status: LocalPrStatus) => void; onMerge: () => void; merging?: boolean; + /** Refresh PRs (re-poll + reload); wired to the header refresh button. */ + onRefresh: () => void; + /** Whether a refresh is in flight (disables the header refresh button). */ + refreshing?: boolean; capabilities?: PlatformCapabilities; currentUserName?: string | null; /** Hide PR lifecycle actions (merge / approval): always set for the closed scope (departed PRs no longer take review decisions / merges). */ @@ -62,6 +66,8 @@ export function PrPanel({ onSetStatus, onMerge, merging = false, + onRefresh, + refreshing = false, capabilities, currentUserName, hideLifecycle = false, @@ -176,6 +182,8 @@ export function PrPanel({ currentUserName={currentUserName} merging={merging} onMerge={onMerge} + onRefresh={onRefresh} + refreshing={refreshing} onSetStatus={onSetStatus} hideLifecycle={hideLifecycle} readOnly={readOnly} diff --git a/apps/desktop/src/renderer/src/components/features/pr/hooks/usePullRequests.ts b/apps/desktop/src/renderer/src/components/features/pr/hooks/usePullRequests.ts index 1914c086..4a5ee78e 100644 --- a/apps/desktop/src/renderer/src/components/features/pr/hooks/usePullRequests.ts +++ b/apps/desktop/src/renderer/src/components/features/pr/hooks/usePullRequests.ts @@ -15,6 +15,8 @@ export function usePullRequests({ notifyError }: { notifyError: (msg: string) => const [prs, setPrs] = useState([]); const [selectedId, setSelectedId] = useState(null); const [refreshing, setRefreshing] = useState(false); + // Single-PR refresh in flight (distinct from the whole-list `refreshing`): disables the PR header's refresh button. + const [refreshingPr, setRefreshingPr] = useState(false); // Merge in progress: GitHub merge can be slow (mergeable is computed asynchronously); set the button to a waiting state and prevent repeated clicks. const [merging, setMerging] = useState(false); @@ -36,6 +38,25 @@ export function usePullRequests({ notifyError }: { notifyError: (msg: string) => } }, [refreshing, reloadPrs]); + // Refresh a single PR from remote (metadata + comments) without a whole-poller tick — the one remote call is this PR's + // fetch; reloadPrs afterwards is local (re-derives the list from disk, so the header / unread / diff head update). + // Comments/inline-diff refresh reactively via the comments:changed the main side broadcasts. + const refreshPr = useCallback( + async (localId: string): Promise => { + if (refreshingPr) return; + setRefreshingPr(true); + try { + await invoke('prs:refreshOne', { localId }); + await reloadPrs(); + } catch (e) { + console.error('refresh PR failed', e); + } finally { + setRefreshingPr(false); + } + }, + [refreshingPr, reloadPrs], + ); + // Mark PR as read: called when the user opens a PR. First optimistically clear the local unread flags (instant feedback), then persist the read watermark — // the next poll round won't re-mark it due to stale events. Send IPC on every selection: opening a PR is not high-frequency, and advancing the read watermark is inherently correct; // don't rely on the side effect of the setState updater to decide "is it unread" — the updater only runs during the render phase, so synchronously reading its side effect gets no result. @@ -105,9 +126,11 @@ export function usePullRequests({ notifyError }: { notifyError: (msg: string) => setSelectedId, selected, refreshing, + refreshingPr, merging, reloadPrs, triggerRefresh, + refreshPr, setSelectedPrStatus, mergeSelectedPr, markRead, diff --git a/apps/desktop/src/renderer/src/components/layout/TitleBar.tsx b/apps/desktop/src/renderer/src/components/layout/TitleBar.tsx index 4ae5f620..83f169b6 100644 --- a/apps/desktop/src/renderer/src/components/layout/TitleBar.tsx +++ b/apps/desktop/src/renderer/src/components/layout/TitleBar.tsx @@ -19,6 +19,7 @@ interface TitleBarProps { discoveryFilters: readonly PrDiscoveryFilter[]; setDiscoveryFilter: (filter: PrDiscoveryFilter) => void; viewArchived: () => void; + refreshPr: (localId: string) => void; openPrByUrl: (url: string) => void | Promise; prStatusFilters: ReadonlyArray<{ value: FilterKey; labelKey: string }>; setPrStatusFilter: (filter: FilterKey) => void; @@ -46,6 +47,7 @@ export function TitleBar({ discoveryFilters, setDiscoveryFilter, viewArchived, + refreshPr, openPrByUrl, prStatusFilters, setPrStatusFilter, @@ -70,6 +72,7 @@ export function TitleBar({ discoveryFilters={discoveryFilters} setDiscoveryFilter={setDiscoveryFilter} viewArchived={viewArchived} + refreshPr={refreshPr} openPrByUrl={openPrByUrl} prStatusFilters={prStatusFilters} setPrStatusFilter={setPrStatusFilter} diff --git a/apps/desktop/src/renderer/src/hooks/useGlobalShortcuts.ts b/apps/desktop/src/renderer/src/hooks/useGlobalShortcuts.ts index 1fa9d499..9f31152c 100644 --- a/apps/desktop/src/renderer/src/hooks/useGlobalShortcuts.ts +++ b/apps/desktop/src/renderer/src/hooks/useGlobalShortcuts.ts @@ -4,8 +4,11 @@ import { chatRunStore } from '../stores/chat-run-store'; /** * Window-level global shortcuts (VS Code style), all attached to a single `keydown` listener here: - * - **F5**: run auto review on the currently selected PR (same logic as the command palette: only triggers when there - * is a selected PR, it's engageable, and it's not already running — reentrancy guard). + * - **F5**: refresh the currently selected PR (re-fetch just that PR from remote, not a whole-poller tick). Bare, + * universally-understood "refresh"; no-op when nothing is selected. + * - **Ctrl+F5** (literal Ctrl on every platform — ⌘F5 would collide with macOS VoiceOver): run auto review on the + * currently selected PR (moved off bare F5 to avoid accidental triggers; same guards as the command palette: a PR is + * selected, it's engageable, and it's not already running). * - **DevTools**: mac ⌥⌘I / otherwise Ctrl+Shift+I (with Shift/Alt, distinguished from the single-modifier B/J below). * - **View closed**: mac ⌘⇧H (avoiding the system "Hide App" ⌘H) / otherwise Ctrl+H (browser history convention). * - **Layout toggles**: Ctrl/Cmd+B toggles the PR list (left sidebar), Ctrl/Cmd+J toggles the chat panel (right); @@ -18,6 +21,7 @@ export function useGlobalShortcuts({ selectedId, canEngage, viewArchived, + refreshPr, setSidebarCollapsed, setChatCollapsed, }: { @@ -25,25 +29,36 @@ export function useGlobalShortcuts({ selectedId: string | null; canEngage: boolean; viewArchived: () => void; + /** Refresh a single PR by localId (re-fetch that PR from remote); bound to bare F5 for the selected PR. */ + refreshPr: (localId: string) => void; setSidebarCollapsed: Dispatch>; setChatCollapsed: Dispatch>; }): void { - // Refs for the selected PR / engageable state: let the stable listener read live values, avoiding a resubscribe on every PR switch. + // Refs for the selected PR / engageable state / refresh: let the stable listener read live values, avoiding a resubscribe on every PR switch / refresh-fn identity change. const selectedIdRef = useRef(selectedId); selectedIdRef.current = selectedId; const canEngageRef = useRef(canEngage); canEngageRef.current = canEngage; + const refreshPrRef = useRef(refreshPr); + refreshPrRef.current = refreshPr; useEffect(() => { const isMac = platform === 'darwin'; const onKey = (e: KeyboardEvent): void => { const k = e.key.toLowerCase(); - // F5: run auto review on the currently selected PR (only triggers when there is a selected PR, it's engageable, and it's not already running — reentrancy guard) + // F5: bare = refresh the selected PR; Ctrl+F5 = run auto review on it (guards: selected + engageable + not already + // running). Always preventDefault so the Electron renderer never hard-reloads on F5 / Ctrl+F5. if (k === 'f5') { + e.preventDefault(); const id = selectedIdRef.current; - if (id && canEngageRef.current && !chatRunStore.getSnapshot().agentPrs.includes(id)) { - e.preventDefault(); - void invoke('agent:run', { localId: id }); + const onlyCtrl = e.ctrlKey && !e.metaKey && !e.shiftKey && !e.altKey; + const noMods = !e.ctrlKey && !e.metaKey && !e.shiftKey && !e.altKey; + if (onlyCtrl) { + if (id && canEngageRef.current && !chatRunStore.getSnapshot().agentPrs.includes(id)) { + void invoke('agent:run', { localId: id }); + } + } else if (noMods && id) { + refreshPrRef.current(id); } return; } diff --git a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json index f961ceec..4526a29e 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/de-DE.json +++ b/apps/desktop/src/renderer/src/i18n/locales/de-DE.json @@ -201,6 +201,7 @@ "cmdOpenDevtools": "DevTools öffnen", "cmdOpenPrUrl": "URL öffnen (aktuelle Plattform)", "cmdOpenSettings": "Einstellungen öffnen", + "cmdRefreshPr": "PR aktualisieren", "cmdRunAutoReview": "Auto-Review ausführen", "cmdSwitchLanguage": "Anzeigesprache wechseln", "cmdSwitchModel": "Modell wechseln", @@ -569,6 +570,7 @@ "ownPrReason": "Sie können Ihren eigenen PR nicht reviewen", "publishComments": "Kommentare absenden ({{n}})", "publishCommentsTitle": "{{n}} Entwürfe gesammelt auf dem Remote veröffentlichen", + "refreshTitle": "Aktualisieren", "showBlame": "Blame aktivieren (nur head-Seite)", "showWhitespace": "Leerzeichen anzeigen (Leerzeichen / Tab)", "sideBySide": "Nebeneinander", diff --git a/apps/desktop/src/renderer/src/i18n/locales/en-US.json b/apps/desktop/src/renderer/src/i18n/locales/en-US.json index 35cab87f..849f40a9 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/en-US.json +++ b/apps/desktop/src/renderer/src/i18n/locales/en-US.json @@ -201,6 +201,7 @@ "cmdOpenDevtools": "Open DevTools", "cmdOpenPrUrl": "Open URL (current platform)", "cmdOpenSettings": "Open Settings", + "cmdRefreshPr": "Refresh PR", "cmdRunAutoReview": "Run Auto Review", "cmdSwitchLanguage": "Switch Display Language", "cmdSwitchModel": "Switch Model", @@ -569,6 +570,7 @@ "ownPrReason": "You can't review your own PR", "publishComments": "Submit comments ({{n}})", "publishCommentsTitle": "Publish {{n}} drafts to the remote in bulk", + "refreshTitle": "Refresh", "showBlame": "Enable blame (head side only)", "showWhitespace": "Show whitespace (spaces / tabs)", "sideBySide": "Side by side", diff --git a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json index 9b6452f7..de8d4f33 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json +++ b/apps/desktop/src/renderer/src/i18n/locales/ja-JP.json @@ -201,6 +201,7 @@ "cmdOpenDevtools": "DevTools を開く", "cmdOpenPrUrl": "URL で開く（現在のプラットフォーム）", "cmdOpenSettings": "設定を開く", + "cmdRefreshPr": "PR を更新", "cmdRunAutoReview": "自動レビューを実行", "cmdSwitchLanguage": "表示言語を切り替え", "cmdSwitchModel": "モデルを切り替え", @@ -555,6 +556,7 @@ "ownPrReason": "自分の PR はレビューできません", "publishComments": "コメントを投稿 ({{n}})", "publishCommentsTitle": "{{n}} 件の下書きをリモートに一括で公開します", + "refreshTitle": "更新", "showBlame": "Blame を有効化（head 側のみ）", "showWhitespace": "空白文字を表示（スペース / Tab）", "sideBySide": "並べて表示", diff --git a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json index 05c53c5d..c8d2d07d 100644 --- a/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json +++ b/apps/desktop/src/renderer/src/i18n/locales/zh-CN.json @@ -201,6 +201,7 @@ "cmdOpenDevtools": "打开 DevTools", "cmdOpenPrUrl": "打开 URL（当前平台）", "cmdOpenSettings": "打开设置", + "cmdRefreshPr": "刷新 PR", "cmdRunAutoReview": "运行自动评审", "cmdSwitchLanguage": "切换显示语言", "cmdSwitchModel": "切换模型", @@ -555,6 +556,7 @@ "ownPrReason": "不能审批自己的 PR", "publishComments": "提交评论 ({{n}})", "publishCommentsTitle": "批量发布 {{n}} 条草稿到远端", + "refreshTitle": "刷新", "showBlame": "开启追溯显示（仅 head 侧）", "showWhitespace": "显示空白字符（空格 / Tab）", "sideBySide": "并排", diff --git a/apps/desktop/src/renderer/src/styles/features/pr/header-actions.scss b/apps/desktop/src/renderer/src/styles/features/pr/header-actions.scss index e29367c0..b3bd6777 100644 --- a/apps/desktop/src/renderer/src/styles/features/pr/header-actions.scss +++ b/apps/desktop/src/renderer/src/styles/features/pr/header-actions.scss @@ -16,6 +16,13 @@ gap: $space-2; } +// Refresh: neutral icon-only button beside "open in browser". Its content is a single icon (shorter than the text +// buttons' line-height), so stretch it to the row height to match the open-in-browser / decision buttons; the icon +// stays vertically centered (.btn is align-items:center). +.pr-header-refresh { + align-self: stretch; +} + // "Merge" button: branch-merge icon + label. Base state matches approve's inactive (default border + body color, only the icon uses // the green semantic color), no longer "always-green filled" — always-green is easily misread as a "clicked / merged" state. Instead, a 1s-cycle blink highlights // clickability; after clicking (merging → disabled) it goes through .btn:disabled to gray out and stop blinking. diff --git a/packages/ipc/src/pr.ts b/packages/ipc/src/pr.ts index 4ceebad4..d157c278 100644 --- a/packages/ipc/src/pr.ts +++ b/packages/ipc/src/pr.ts @@ -125,6 +125,16 @@ export interface PrChannels { }; }; 'prs:refresh': { request: void; response: PollResult }; + /** + * Refresh a SINGLE PR from remote (metadata + comments) without a whole-poller tick: re-fetch just this PR, recompute + * localStatus from the current user's reviewer status, persist its meta, and invalidate its comments cache (broadcasts + * comments:changed). Returns the updated PR (null if not found / not fetchable). Used by the header refresh button, the + * F5 shortcut, and the "Refresh PR" command. + */ + 'prs:refreshOne': { + request: { localId: string }; + response: StoredPullRequest | null; + }; /** Poller's last completion time (ISO or null); used for initialization at startup */ 'prs:lastSync': { request: void; response: { at: string | null } }; 'prs:setLocalStatus': { From 900e8825d6c21f4f62d6245f2c30e43cf949d91f Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Tue, 28 Jul 2026 09:21:42 +0800 Subject: [PATCH 08/11] =?UTF-8?q?fix(chat):=20commit=20divider=20=E2=80=94?= =?UTF-8?q?=20gap/alignment=20+=20show=20the=20commit=20message=20on=20hov?= =?UTF-8?q?er?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two polish fixes for the run-timeline commit divider: - The chip's commit icon and short sha were flush and vertically misaligned (the chip mixin sets no gap). Add a small gap and make the svg a block so it aligns to the sha's line box. - The hover tooltip showed a generic "code changed" string. Show the new head commit's actual message instead (resolved via diff:listCommits — the head is the PR's newest introduced commit; main-cached), falling back to the short sha. This is more meaningful and removes the chatPane.commitDividerTitle i18n key from all four locales. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/components/features/chat/ChatPane.tsx | 25 ++++++++++++++++++- .../chat/components/CommitDivider.tsx | 13 +++++----- .../src/renderer/src/i18n/locales/de-DE.json | 1 - .../src/renderer/src/i18n/locales/en-US.json | 1 - .../src/renderer/src/i18n/locales/ja-JP.json | 1 - .../src/renderer/src/i18n/locales/zh-CN.json | 1 - .../src/styles/features/chat/pane.scss | 8 +++++- 7 files changed, 37 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx index b3f976a1..a3268332 100644 --- a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx +++ b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx @@ -4,10 +4,12 @@ import type { Finding, LocalPrStatus, PrAgentStatus, + PrCommit, ReviewRun, ReviewRunCommitScope, StoredPullRequest, } from '@meebox/shared'; +import { invoke } from '../../../api'; import { ChatIcon, TrashIcon, ConfirmModal, PaneLoading } from '../../common'; import { useChatRunStore } from '../../../stores/chat-run-store'; import { useDraftsForPr } from '../../../stores/drafts-store'; @@ -260,6 +262,27 @@ export function ChatPane({ return head !== lastRunHeadSha ? head : null; }, [timeline, hasMyActive, pr?.sourceRef.sha]); + // The new head commit's message, for the divider tooltip. diff:listCommits is the PR's introduced commits (the head + // is the newest); main-cached, so this is cheap. Falls back to the short sha when unavailable. + const [staleHeadMessage, setStaleHeadMessage] = useState(undefined); + useEffect(() => { + if (!staleHeadSha || !prLocalId) { + setStaleHeadMessage(undefined); + return; + } + let cancelled = false; + void invoke('diff:listCommits', { localId: prLocalId }) + .then((commits: PrCommit[]) => { + if (!cancelled) setStaleHeadMessage(commits.find((c) => c.sha === staleHeadSha)?.message); + }) + .catch(() => { + if (!cancelled) setStaleHeadMessage(undefined); + }); + return () => { + cancelled = true; + }; + }, [staleHeadSha, prLocalId]); + // Pure UI state: rule preview modal / clear confirm modal / merge confirm modal const [showRulePreview, setShowRulePreview] = useState(false); const [showClearConfirm, setShowClearConfirm] = useState(false); @@ -418,7 +441,7 @@ export function ChatPane({ )} {/* Commit divider: the PR head advanced past the last reviewed commit → mark the new head at the bottom of the run list (prior reviews are stale). Shown even when no run has been started against the new code yet. */} - {staleHeadSha && } + {staleHeadSha && } {/* This PR's queued tasks: placed after running ones, each cancellable individually. The position uses the **global** queue order (the queue is shared across PRs, otherwise every PR showing "position 1" would be misleading) — the runId's index in the global waiting array +1. */} {myWaiting.map((w) => ( diff --git a/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx b/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx index e1385cbb..93917e83 100644 --- a/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx +++ b/apps/desktop/src/renderer/src/components/features/chat/components/CommitDivider.tsx @@ -1,17 +1,16 @@ -import { useTranslation } from 'react-i18next'; import { CommitIcon } from '../../../common'; /** * Sawtooth "commit divider" shown at the bottom of the run timeline when the PR head has advanced past the commit the * most recent run reviewed (see ChatPane staleHeadSha). It marks the new head — signalling that the reviews above are - * now based on stale code — even if no run has been started against the new commit yet. The label is the abbreviated - * commit id (the full SHA is in the tooltip), reusing the same chip vocabulary as the single-commit scope badge in - * RunResultView. + * now based on stale code — even if no run has been started against the new commit yet. The chip shows the abbreviated + * commit id; the tooltip shows that commit's message (falls back to the short sha when unavailable). Reuses the same + * chip vocabulary as the single-commit scope badge in RunResultView. */ -export function CommitDivider({ sha }: { sha: string }) { - const { t } = useTranslation(); +export function CommitDivider({ sha, message }: { sha: string; message?: string }) { const short = sha.slice(0, 8); - const title = t('chatPane.commitDividerTitle', { sha }); + // Tooltip = the actual commit message (more meaningful than a generic "code changed" string, and needs no i18n). + const title = message?.trim() || short; return (