diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 58d7618b76b2..03356ef9720a 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -5603,7 +5603,13 @@ export default function ChatView(props: ChatViewProps) { diffAction === "defer" || shouldDeferLink ? previousRunningTurnId : activeRunningTurnId, }; if (diffAction !== "open" || newlyCompletedTurnId === null) return; - if (!panels.openProactive(activeThreadRef, { id: "diff", kind: "diff" }, userActionRevision)) { + if ( + !panels.openProactive( + activeThreadRef, + { kind: "diff", turnId: newlyCompletedTurnId }, + userActionRevision, + ) + ) { return; } useDiffPanelStore.getState().selectGitScope(activeThreadRef, "branch"); diff --git a/apps/web/src/rightPanelStore.test.ts b/apps/web/src/rightPanelStore.test.ts index 2b75df19d4c4..7cae71bdcb3d 100644 --- a/apps/web/src/rightPanelStore.test.ts +++ b/apps/web/src/rightPanelStore.test.ts @@ -1,5 +1,5 @@ import { scopeThreadRef } from "@t3tools/client-runtime/environment"; -import { type EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { type EnvironmentId, RunId, ThreadId } from "@t3tools/contracts"; import { beforeEach, describe, expect, it } from "vite-plus/test"; import { @@ -108,7 +108,7 @@ describe("rightPanelStore", () => { }, ); - const completedDiff = { id: "diff", kind: "diff" } as const; + const completedDiff = { kind: "diff", turnId: RunId.make("turn-1") } as const; const linkedPullRequest = pullRequestSurface({ projectId: "project-a", repository: "pingdotgg/t3code", @@ -181,9 +181,15 @@ describe("rightPanelStore", () => { expect( store.openProactive(refA, { id: "pull-requests", kind: "pull-requests" }, revision), ).toBe(false); - expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toBe( - chosen, + const { isOpen, activeSurfaceId, surfaces } = selectThreadRightPanelState( + useRightPanelStore.getState().byThreadKey, + refA, ); + expect({ isOpen, activeSurfaceId, surfaces }).toEqual({ + isOpen: chosen.isOpen, + activeSurfaceId: chosen.activeSurfaceId, + surfaces: chosen.surfaces, + }); }); it("allows automatic panels for a later turn after a manual choice", () => { @@ -193,10 +199,73 @@ describe("rightPanelStore", () => { expect(store.openProactive(refA, completedDiff, firstTurnRevision)).toBe(false); const nextTurnRevision = store.getUserActionRevision(refA); - expect(store.openProactive(refA, completedDiff, nextTurnRevision)).toBe(true); + expect( + store.openProactive( + refA, + { ...completedDiff, turnId: RunId.make("turn-2") }, + nextTurnRevision, + ), + ).toBe(true); expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff"); }); + it.each([ + { dismissal: "hide", dismiss: () => useRightPanelStore.getState().close(refA) }, + { + dismissal: "close tab", + dismiss: () => useRightPanelStore.getState().closeSurface(refA, "diff"), + }, + ])("offers each turn's diff once after $dismissal and reload", ({ dismiss }) => { + const store = useRightPanelStore.getState(); + expect(store.openProactive(refA, completedDiff, store.getUserActionRevision(refA))).toBe(true); + dismiss(); + const persisted = JSON.parse( + JSON.stringify({ byThreadKey: useRightPanelStore.getState().byThreadKey }), + ); + useRightPanelStore.setState(migratePersistedRightPanelState(persisted)); + + expect(store.openProactive(refA, completedDiff, store.getUserActionRevision(refA))).toBe(false); + expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBeNull(); + + const nextTurn = { ...completedDiff, turnId: RunId.make("turn-2") }; + expect(store.openProactive(refA, nextTurn, store.getUserActionRevision(refA))).toBe(true); + expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("diff"); + }); + + it.each([ + { action: "open file", act: () => useRightPanelStore.getState().openFile(refA, "src/app.ts") }, + { action: "open files", act: () => useRightPanelStore.getState().open(refA, "files") }, + { + action: "open pull request", + act: () => useRightPanelStore.getState().openPullRequest(refA, linkedPullRequest), + }, + ])("keeps the offered turn after dismissing, then $action, then reload", ({ act }) => { + const store = useRightPanelStore.getState(); + expect(store.openProactive(refA, completedDiff, store.getUserActionRevision(refA))).toBe(true); + store.close(refA); + act(); + const persisted = JSON.parse( + JSON.stringify({ byThreadKey: useRightPanelStore.getState().byThreadKey }), + ); + useRightPanelStore.setState(migratePersistedRightPanelState(persisted)); + const before = selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA); + + expect(store.openProactive(refA, completedDiff, store.getUserActionRevision(refA))).toBe(false); + expect(selectThreadRightPanelState(useRightPanelStore.getState().byThreadKey, refA)).toBe( + before, + ); + }); + + it("does not retry a diff offer the user already declined", () => { + const store = useRightPanelStore.getState(); + const revision = store.getUserActionRevision(refA); + store.openFile(refA, "src/app.ts"); + expect(store.openProactive(refA, completedDiff, revision)).toBe(false); + + expect(store.openProactive(refA, completedDiff, store.getUserActionRevision(refA))).toBe(false); + expect(selectActiveRightPanel(useRightPanelStore.getState().byThreadKey, refA)).toBe("file"); + }); + it("keeps manual choices scoped to their thread and environment", () => { const otherEnvironment = scopeThreadRef("env-2" as EnvironmentId, refA.threadId); const store = useRightPanelStore.getState(); diff --git a/apps/web/src/rightPanelStore.ts b/apps/web/src/rightPanelStore.ts index ec645e62c409..1c0d96b55e19 100644 --- a/apps/web/src/rightPanelStore.ts +++ b/apps/web/src/rightPanelStore.ts @@ -8,7 +8,7 @@ * workspace paths, and diff/files remain singleton surfaces. */ import { scopedThreadKey, scopeThreadRef } from "@t3tools/client-runtime/environment"; -import { EnvironmentId, ThreadId, type ScopedThreadRef } from "@t3tools/contracts"; +import { EnvironmentId, RunId, ThreadId, type ScopedThreadRef } from "@t3tools/contracts"; import { create } from "zustand"; import { createJSONStorage, persist } from "zustand/middleware"; @@ -107,6 +107,8 @@ export interface ThreadRightPanelState { activeSurfaceId: string | null; surfaces: RightPanelSurface[]; dismissedDeviceSurfaceIds?: string[]; + /** The last turn whose diff was offered automatically. Each turn is offered once. */ + proactiveDiffTurnId?: RunId; } export interface ThreadPanelVisibility { @@ -114,6 +116,10 @@ export interface ThreadPanelVisibility { popoverOpen: boolean; } +export type ProactivePanelRequest = + | Extract + | { kind: "diff"; turnId: RunId }; + interface RightPanelStoreState { byThreadKey: Record; threadPanelVisibilityByThreadKey: Record; @@ -126,7 +132,7 @@ interface RightPanelStoreState { */ openProactive: ( ref: ScopedThreadRef, - surface: Extract, + request: ProactivePanelRequest, expectedUserActionRevision: number, ) => boolean; open: ( @@ -297,12 +303,18 @@ const updateThreadStateMap = ( updater: (current: ThreadRightPanelState) => ThreadRightPanelState, ): Record => { const current = byThreadKey[threadKey] ?? EMPTY_THREAD_STATE; - const next = updater(current); + const updated = updater(current); + // Many actions rebuild the thread state from scratch. Only `openProactive` replaces the offer. + const next = + updated.proactiveDiffTurnId === undefined && current.proactiveDiffTurnId !== undefined + ? { ...updated, proactiveDiffTurnId: current.proactiveDiffTurnId } + : updated; if ( !next.isOpen && next.activeSurfaceId === null && next.surfaces.length === 0 && - !next.dismissedDeviceSurfaceIds?.length + !next.dismissedDeviceSurfaceIds?.length && + next.proactiveDiffTurnId === undefined ) { if (!(threadKey in byThreadKey)) return byThreadKey; const { [threadKey]: _removed, ...rest } = byThreadKey; @@ -530,6 +542,9 @@ export function migratePersistedRightPanelState(persistedState: unknown): { isOpen, surfaces, activeSurfaceId, + ...(typeof validThreadState?.proactiveDiffTurnId === "string" + ? { proactiveDiffTurnId: RunId.make(validThreadState.proactiveDiffTurnId) } + : {}), ...(Array.isArray(validThreadState?.dismissedDeviceSurfaceIds) ? { dismissedDeviceSurfaceIds: @@ -569,26 +584,33 @@ export const useRightPanelStore = create()( userActionRevisionByThreadKey: {}, getUserActionRevision: (ref) => get().userActionRevisionByThreadKey[scopedThreadKey(ref)] ?? 0, - openProactive: (ref, surface, expectedUserActionRevision) => { + openProactive: (ref, request, expectedUserActionRevision) => { let opened = false; set((state) => { const threadKey = scopedThreadKey(ref); - if ( - (state.userActionRevisionByThreadKey[threadKey] ?? 0) !== expectedUserActionRevision - ) { - return state; + if (request.kind === "diff") { + const current = selectThreadRightPanelState(state.byThreadKey, ref); + if (current.proactiveDiffTurnId === request.turnId) return state; + // A linked PR takes priority over a completed-turn diff. + const pullRequestActive = + selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" || + selectActiveRightPanel(state.byThreadKey, ref) === "pull-requests"; + opened = + (state.userActionRevisionByThreadKey[threadKey] ?? 0) === + expectedUserActionRevision && !pullRequestActive; + // Record the offer even when refused, so a revisit does not retry this turn. + return automaticUpdate(state, threadKey, (thread) => ({ + ...(opened ? upsertSurface(thread, singletonSurface("diff")) : thread), + proactiveDiffTurnId: request.turnId, + })); } - // A linked PR takes priority over a completed-turn diff. Manual actions - // always apply, and later user choices reject both proactive requests. if ( - surface.kind === "diff" && - (selectActiveRightPanel(state.byThreadKey, ref) === "pull-request" || - selectActiveRightPanel(state.byThreadKey, ref) === "pull-requests") + (state.userActionRevisionByThreadKey[threadKey] ?? 0) !== expectedUserActionRevision ) { return state; } opened = true; - return automaticUpdate(state, threadKey, (current) => upsertSurface(current, surface)); + return automaticUpdate(state, threadKey, (current) => upsertSurface(current, request)); }); return opened; },