diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c56c76be7e0e7..84acc7c74f865 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1482,7 +1482,18 @@ jobs: git add apps/server/package.json apps/desktop/package.json apps/web/package.json packages/contracts/package.json pnpm-lock.yaml git commit --no-verify -m "chore(release): prepare $RELEASE_TAG" - git push origin HEAD:main + + # main moves while this job runs, so a rejected push rebases and retries. + for attempt in 1 2 3 4 5; do + if git push origin HEAD:main; then + exit 0 + fi + echo "Push rejected (attempt $attempt). Rebasing onto latest main." + git pull --rebase origin main + done + + echo "Could not push the version bump after 5 attempts." >&2 + exit 1 announce_discord: name: Announce release on Discord diff --git a/apps/desktop/package.json b/apps/desktop/package.json index ab4e1ed382dab..c6114f1bb48b8 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,6 +1,6 @@ { "name": "@t3tools/desktop", - "version": "0.1.8", + "version": "0.1.9", "private": true, "type": "module", "main": "dist-electron/boot.cjs", diff --git a/apps/desktop/src/app/ClerkTokenStore.test.ts b/apps/desktop/src/app/ClerkTokenStore.test.ts index b7ac0e38d5142..13f6bbf758a4f 100644 --- a/apps/desktop/src/app/ClerkTokenStore.test.ts +++ b/apps/desktop/src/app/ClerkTokenStore.test.ts @@ -3,9 +3,6 @@ import * as NodeFS from "node:fs"; import * as NodeOS from "node:os"; import * as NodePath from "node:path"; import { assert, describe, it } from "@effect/vitest"; -import { vi } from "vite-plus/test"; - -vi.mock("node:fs", { spy: true }); import { openClerkTokenStorage, type ClerkTokenStorage } from "./ClerkTokenStore.ts"; @@ -91,24 +88,6 @@ describePosix("ClerkTokenStore", () => { }); }); - it("removes the legacy token file when the private copy already exists", async () => { - await withStateDir((stateDir) => { - const directory = NodePath.join(stateDir, "clerk"); - const legacyPath = NodePath.join(stateDir, "clerk-tokens.json"); - const tokenPath = NodePath.join(directory, "clerk-tokens.json"); - NodeFS.mkdirSync(directory, { mode: 0o755 }); - writeLoose(legacyPath, "stale-credential"); - writeLoose(tokenPath, "kept-credential"); - - openClerkTokenStorage(stateDir, () => idleStorage()); - - assert.equal(NodeFS.existsSync(legacyPath), false); - assert.equal(modeOf(directory), 0o700); - assert.equal(modeOf(tokenPath), 0o600); - assert.equal(NodeFS.readFileSync(tokenPath, "utf8"), "kept-credential"); - }); - }); - it("resolves a relative state directory before opening storage", async () => { await withStateDir(async (stateDir) => { const relativeStateDir = NodePath.relative(process.cwd(), stateDir); @@ -150,23 +129,6 @@ describePosix("ClerkTokenStore", () => { }); }); - it("migrates tokens when the legacy directory is on another filesystem", async () => { - await withStateDir((stateDir) => { - const legacyPath = NodePath.join(stateDir, "clerk-tokens.json"); - writeLoose(legacyPath, "existing-session"); - using _rename = vi.spyOn(NodeFS, "renameSync").mockImplementation(() => { - throw Object.assign(new Error("cross-device link"), { code: "EXDEV" }); - }); - - openClerkTokenStorage(stateDir, idleStorage); - - const tokenPath = NodePath.join(stateDir, "clerk", "clerk-tokens.json"); - assert.equal(NodeFS.readFileSync(tokenPath, "utf8"), "existing-session"); - assert.equal(modeOf(tokenPath), 0o600); - assert.equal(NodeFS.existsSync(legacyPath), false); - }); - }); - it("does not follow a symlinked clerk directory or token file", async () => { await withStateDir(async (stateDir) => { const outsideDirectory = NodePath.join(stateDir, "outside-dir"); diff --git a/apps/desktop/src/ipc/channels.ts b/apps/desktop/src/ipc/channels.ts index aebbc1c24f8c2..33dd9e272df7d 100644 --- a/apps/desktop/src/ipc/channels.ts +++ b/apps/desktop/src/ipc/channels.ts @@ -70,6 +70,7 @@ export const BOOTSTRAP_SSH_BEARER_SESSION_CHANNEL = "desktop:bootstrap-ssh-beare export const FETCH_SSH_SESSION_STATE_CHANNEL = "desktop:fetch-ssh-session-state"; export const SSH_PASSWORD_PROMPT_CHANNEL = "desktop:ssh-password-prompt"; export const RESOLVE_SSH_PASSWORD_PROMPT_CHANNEL = "desktop:resolve-ssh-password-prompt"; +export const SSH_PASSWORD_PROMPT_SETTLED_CHANNEL = "desktop:ssh-password-prompt-settled"; export const GET_SERVER_EXPOSURE_STATE_CHANNEL = "desktop:get-server-exposure-state"; export const SET_SERVER_EXPOSURE_MODE_CHANNEL = "desktop:set-server-exposure-mode"; export const SET_TAILSCALE_SERVE_ENABLED_CHANNEL = "desktop:set-tailscale-serve-enabled"; @@ -94,6 +95,8 @@ export const PREVIEW_SET_ZOOM_FACTOR_CHANNEL = "desktop:preview-set-zoom-factor" export const PREVIEW_HARD_RELOAD_CHANNEL = "desktop:preview-hard-reload"; export const PREVIEW_SET_COLOR_SCHEME_CHANNEL = "desktop:preview-set-color-scheme"; export const PREVIEW_SET_AUDIO_MUTED_CHANNEL = "desktop:preview-set-audio-muted"; +export const PREVIEW_FIND_IN_PAGE_CHANNEL = "desktop:preview-find-in-page"; +export const PREVIEW_STOP_FIND_IN_PAGE_CHANNEL = "desktop:preview-stop-find-in-page"; export const PREVIEW_SET_SHOWN_CHANNEL = "desktop:preview-set-shown"; export const PREVIEW_OPEN_DEVTOOLS_CHANNEL = "desktop:preview-open-devtools"; export const PREVIEW_CLEAR_COOKIES_CHANNEL = "desktop:preview-clear-cookies"; @@ -118,6 +121,7 @@ export const PREVIEW_RECORDING_SAVE_CHANNEL = "desktop:preview-recording-save"; export const PREVIEW_STATE_CHANGE_CHANNEL = "desktop:preview-state-change"; export const PREVIEW_POINTER_EVENT_CHANNEL = "desktop:preview-pointer-event"; export const PREVIEW_OPEN_LINK_CHANNEL = "desktop:preview-open-link"; +export const PREVIEW_FIND_RESULT_CHANNEL = "desktop:preview-find-result"; export const MAC_PERMISSION_HELPER_CHANNEL = "desktop:mac-permission-helper"; diff --git a/apps/desktop/src/ipc/methods/preview.ts b/apps/desktop/src/ipc/methods/preview.ts index 98a0aab831645..ea291b66b3149 100644 --- a/apps/desktop/src/ipc/methods/preview.ts +++ b/apps/desktop/src/ipc/methods/preview.ts @@ -5,6 +5,7 @@ import { DesktopPreviewPickElementInputSchema, DesktopPreviewArtifactInputSchema, DesktopPreviewConfigInputSchema, + DesktopPreviewFindInPageInputSchema, DesktopPreviewNavigateInputSchema, DesktopPreviewRecordingArtifactSchema, DesktopPreviewRecordingSaveInputSchema, @@ -59,6 +60,9 @@ export const installPreviewEventForwarding = Effect.fn( yield* manager.subscribeOpenLinks((event) => electronWindow.sendAll(IpcChannels.PREVIEW_OPEN_LINK_CHANNEL, event), ); + yield* manager.subscribeFindResults((event) => + electronWindow.sendAll(IpcChannels.PREVIEW_FIND_RESULT_CHANNEL, event), + ); }); const setForwardedShortcuts = DesktopIpc.makeIpcMethod({ @@ -116,6 +120,21 @@ const navigate = DesktopIpc.makeIpcMethod({ }), }); +const findInPage = DesktopIpc.makeIpcMethod({ + channel: IpcChannels.PREVIEW_FIND_IN_PAGE_CHANNEL, + payload: DesktopPreviewFindInPageInputSchema, + result: Schema.Void, + handler: Effect.fn("desktop.ipc.preview.findInPage")(function* ({ + tabId, + text, + forward, + newSession, + }) { + const manager = yield* PreviewManager.PreviewManager; + yield* manager.findInPage(tabId, text, { forward, newSession }); + }), +}); + const tabMethod = ( channel: string, name: string, @@ -169,6 +188,11 @@ const hardReload = tabMethod( "desktop.ipc.preview.hardReload", (manager, tabId) => manager.hardReload(tabId), ); +const stopFindInPage = tabMethod( + IpcChannels.PREVIEW_STOP_FIND_IN_PAGE_CHANNEL, + "desktop.ipc.preview.stopFindInPage", + (manager, tabId) => manager.stopFindInPage(tabId), +); const setZoomFactor = DesktopIpc.makeIpcMethod({ channel: IpcChannels.PREVIEW_SET_ZOOM_FACTOR_CHANNEL, payload: DesktopPreviewSetZoomFactorInputSchema, @@ -452,6 +476,8 @@ export const methods = [ zoomOut, resetZoom, hardReload, + findInPage, + stopFindInPage, setColorScheme, setZoomFactor, setAudioMuted, diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 2dbc4eeda3ae5..5bf05668b5a98 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -1,6 +1,7 @@ import type { DesktopBridge, DesktopPreviewAnnotationVoiceEvent, + DesktopPreviewFindResultEvent, DesktopPreviewOpenLinkEvent, DesktopPreviewPointerEvent, DesktopPreviewRecordingInputEvent, @@ -194,6 +195,17 @@ contextBridge.exposeInMainWorld("desktopBridge", { ipcRenderer.removeListener(IpcChannels.SSH_PASSWORD_PROMPT_CHANNEL, wrappedListener); }; }, + onSshPasswordPromptSettled: (listener) => { + const wrappedListener = (_event: Electron.IpcRendererEvent, requestId: unknown) => { + if (typeof requestId !== "string") return; + listener(requestId); + }; + + ipcRenderer.on(IpcChannels.SSH_PASSWORD_PROMPT_SETTLED_CHANNEL, wrappedListener); + return () => { + ipcRenderer.removeListener(IpcChannels.SSH_PASSWORD_PROMPT_SETTLED_CHANNEL, wrappedListener); + }; + }, resolveSshPasswordPrompt: (requestId, password) => ipcRenderer.invoke(IpcChannels.RESOLVE_SSH_PASSWORD_PROMPT_CHANNEL, { requestId, password }), getServerExposureState: () => ipcRenderer.invoke(IpcChannels.GET_SERVER_EXPOSURE_STATE_CHANNEL), @@ -369,6 +381,15 @@ contextBridge.exposeInMainWorld("desktopBridge", { ipcRenderer.invoke(IpcChannels.PREVIEW_SET_COLOR_SCHEME_CHANNEL, { tabId, colorScheme }), setAudioMuted: (tabId, audioMuted) => ipcRenderer.invoke(IpcChannels.PREVIEW_SET_AUDIO_MUTED_CHANNEL, { tabId, audioMuted }), + findInPage: (tabId, text, { forward, newSession }) => + ipcRenderer.invoke(IpcChannels.PREVIEW_FIND_IN_PAGE_CHANNEL, { + tabId, + text, + forward, + newSession, + }), + stopFindInPage: (tabId) => + ipcRenderer.invoke(IpcChannels.PREVIEW_STOP_FIND_IN_PAGE_CHANNEL, { tabId }), setShown: (tabId, shown) => ipcRenderer.invoke(IpcChannels.PREVIEW_SET_SHOWN_CHANNEL, { tabId, shown }), openDevTools: (tabId) => @@ -469,5 +490,14 @@ contextBridge.exposeInMainWorld("desktopBridge", { return () => ipcRenderer.removeListener(IpcChannels.PREVIEW_OPEN_LINK_CHANNEL, wrappedListener); }, + onFindResult: (listener) => { + const wrappedListener = (_event: Electron.IpcRendererEvent, findEvent: unknown) => { + if (typeof findEvent !== "object" || findEvent === null) return; + listener(findEvent as DesktopPreviewFindResultEvent); + }; + ipcRenderer.on(IpcChannels.PREVIEW_FIND_RESULT_CHANNEL, wrappedListener); + return () => + ipcRenderer.removeListener(IpcChannels.PREVIEW_FIND_RESULT_CHANNEL, wrappedListener); + }, }, } satisfies DesktopBridge); diff --git a/apps/desktop/src/preview/GuestProtocol.ts b/apps/desktop/src/preview/GuestProtocol.ts index a4fd88c475bca..fbf90a38c4eb9 100644 --- a/apps/desktop/src/preview/GuestProtocol.ts +++ b/apps/desktop/src/preview/GuestProtocol.ts @@ -6,6 +6,7 @@ export const ANNOTATION_THEME_CHANNEL = "preview:annotation-theme"; export const ANNOTATION_SEND_ENABLED_CHANNEL = "preview:annotation-send-enabled"; export const HUMAN_INPUT_CHANNEL = "preview:human-input"; export const MOUSE_NAVIGATE_CHANNEL = "preview:mouse-navigate"; +export const PAGE_UNHANDLED_SHORTCUT_CHANNEL = "preview:page-unhandled-shortcut"; export const RECORDING_CURSOR_CHANNEL = "preview:recording-cursor"; export const RECORDING_POINTER_CHANNEL = "preview:recording-pointer"; export const RECORDING_KEY_CHANNEL = "preview:recording-key"; diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index b4e4b2a8b75ce..79a059d828943 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -2,6 +2,7 @@ import { it as effectIt } from "@effect/vitest"; import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import type { + DesktopPreviewFindResultEvent, DesktopPreviewRecordingInputEvent, PreviewAnnotationPayload, PreviewAnnotationRect, @@ -812,6 +813,135 @@ describe("PreviewManager", () => { ), ); + effectIt.effect.each([ + ["darwin", true], + ["linux", false], + ["win32", false], + ] as const)( + "lets the page and its iframes claim the find shortcut before the host opens find on %s", + ([platform, usesMeta]) => + withManager( + (manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + const send = vi.fn(); + const mainFrame = { processId: 1, routingId: 1 }; + Object.assign(preview.webContents, { + hostWebContents: { isDestroyed: () => false, send }, + mainFrame, + focusedFrame: mainFrame, + }); + fromId.mockReturnValue(preview.webContents); + yield* manager.createTab("tab_page_find"); + yield* manager.registerWebview("tab_page_find", 42); + yield* manager.setForwardedShortcuts([ + { command: "preview.find", shortcut: parseKeybindingShortcut("mod+f")! }, + ]); + const beforeInput = preview.listeners.get("before-input-event")!; + const findKey = { + type: "keyDown", + key: "f", + code: "KeyF", + meta: usesMeta, + control: !usesMeta, + }; + const preventDefault = vi.fn(); + + beforeInput( + { preventDefault } as never, + { ...findKey, shift: false, alt: false } as never, + ); + expect(preventDefault).not.toHaveBeenCalled(); + expect(send).not.toHaveBeenCalled(); + + const ipcOn = (preview.webContents as Electron.WebContents).ipc.on as ReturnType< + typeof vi.fn + >; + const pageUnhandledShortcut = ipcOn.mock.calls.find( + ([channel]) => channel === "preview:page-unhandled-shortcut", + )![1]; + pageUnhandledShortcut( + {}, + { + key: "f", + code: "KeyF", + metaKey: usesMeta, + ctrlKey: !usesMeta, + shiftKey: false, + altKey: false, + }, + ); + expect(send).toHaveBeenCalledExactlyOnceWith("desktop:menu-action", "preview.find"); + + Object.assign(preview.webContents, { focusedFrame: { processId: 1, routingId: 2 } }); + beforeInput( + { preventDefault } as never, + { ...findKey, shift: false, alt: false } as never, + ); + expect(preventDefault).not.toHaveBeenCalled(); + expect(send).toHaveBeenCalledOnce(); + + Object.assign(preview.webContents, { focusedFrame: mainFrame }); + yield* manager.setForwardedShortcuts([ + { command: "preview.find", shortcut: parseKeybindingShortcut("ctrl+alt+8")! }, + ]); + pageUnhandledShortcut( + {}, + { + key: "[", + code: "Digit8", + metaKey: false, + ctrlKey: true, + shiftKey: false, + altKey: true, + }, + ); + expect(send).toHaveBeenCalledTimes(usesMeta ? 2 : 1); + }), + platform, + ), + ); + + effectIt.effect("searches the guest page and reports its match counts", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + const findInPage = vi.fn(); + const stopFindInPage = vi.fn(); + Object.assign(preview.webContents, { findInPage, stopFindInPage }); + fromId.mockReturnValue(preview.webContents); + yield* manager.createTab("tab_find"); + yield* manager.registerWebview("tab_find", 42); + const reported = yield* Deferred.make(); + yield* manager.subscribeFindResults((event) => Deferred.succeed(reported, event)); + + yield* manager.findInPage("tab_find", "needle", { forward: true, newSession: true }); + expect(findInPage).toHaveBeenCalledExactlyOnceWith("needle", { + forward: true, + findNext: true, + }); + + preview.listeners.get("found-in-page")!( + {} as never, + { + requestId: 1, + activeMatchOrdinal: 2, + matches: 5, + finalUpdate: true, + } as never, + ); + expect(yield* Deferred.await(reported)).toEqual({ + tabId: "tab_find", + activeMatchOrdinal: 2, + matches: 5, + }); + + yield* manager.stopFindInPage("tab_find"); + expect(stopFindInPage).toHaveBeenCalledExactlyOnceWith("clearSelection"); + }), + ), + ); + effectIt.effect("isolates failed state listeners and continues delivery", () => { const loggedErrors: Array = []; const logger = Logger.make(({ message }) => { diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index c668c39061e00..14bdd0e1d6ba9 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -12,6 +12,7 @@ import type { DesktopPreviewAnnotationVoiceState, DesktopPreviewColorScheme, DesktopPreviewFavicon, + DesktopPreviewFindResultEvent, DesktopPreviewOpenLinkEvent, DesktopPreviewPointerEvent, PreviewAnnotationPayload, @@ -80,6 +81,7 @@ import { ELEMENT_PICKED_CHANNEL, HUMAN_INPUT_CHANNEL, MOUSE_NAVIGATE_CHANNEL, + PAGE_UNHANDLED_SHORTCUT_CHANNEL, RECORDING_CURSOR_CHANNEL, RECORDING_INPUT_CHANNEL, RECORDING_CONTROLLER_CHANNEL, @@ -437,6 +439,23 @@ type PointerEventListener = (event: DesktopPreviewPointerEvent) => Effect.Effect type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect; +type FindResultListener = (event: DesktopPreviewFindResultEvent) => Effect.Effect; + +const PAGE_FIRST_FORWARDED_COMMANDS: ReadonlySet = new Set([ + "preview.find", +]); + +const decodePageShortcutKeys = Schema.decodeUnknownOption( + Schema.Struct({ + key: Schema.String, + code: Schema.String, + metaKey: Schema.Boolean, + ctrlKey: Schema.Boolean, + shiftKey: Schema.Boolean, + altKey: Schema.Boolean, + }), +); + // about:blank inherits relaxed guest preferences; external editor schemes must reach the OS handler. const POPUP_PROTOCOLS = new Set(["http:", "https:"]); @@ -557,6 +576,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const listenersRef = yield* Ref.make>(new Set()); const pointerEventListenersRef = yield* Ref.make>(new Set()); const openLinkListenersRef = yield* Ref.make>(new Set()); + const findResultListenersRef = yield* Ref.make>(new Set()); const recordingInputListenersRef = yield* Ref.make>( new Set(), ); @@ -822,7 +842,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }); const deliverEvent = ( - eventKind: "state-change" | "recording-input" | "pointer-event" | "open-link", + eventKind: "state-change" | "recording-input" | "pointer-event" | "open-link" | "find-result", tabId: string, delivery: () => Effect.Effect, ) => @@ -1425,6 +1445,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }), ); }; + // Chromium splits one search across several updates (the ordinal and the + // final count can land separately), so every update is forwarded. + const foundInPage = (_event: unknown, result: Electron.Result): void => { + runFork( + emitFindResult({ + tabId, + activeMatchOrdinal: result.activeMatchOrdinal, + matches: result.matches, + }), + ); + }; const humanInput = (_event: unknown, rawSignal?: unknown): void => { runFork(handleHumanInput(rawSignal)); }; @@ -1461,6 +1492,20 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function syncMenuShortcuts(window.webContents, input); }); }; + const findForwardedShortcut = (keys: Parameters[0]) => + !( + hostPlatform !== "darwin" && + keys.ctrlKey && + keys.altKey && + /^(?:[^a-zA-Z0-9]|Dead)$/u.test(keys.key) + ) && + forwardedShortcuts.find(({ shortcut }) => + matchesKeybindingShortcut( + keys, + shortcut, + hostPlatform === "darwin" ? "MacIntel" : hostPlatform, + ), + ); const beforeInput = (event: Electron.Event, input: Electron.Input): void => { syncMenuShortcuts(wc, input); const host = wc.hostWebContents; @@ -1470,27 +1515,18 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function host && !host.isDestroyed() && holdsKeyboardFocus(wc) && - !( - hostPlatform !== "darwin" && - input.control && - input.alt && - /^(?:[^a-zA-Z0-9]|Dead)$/u.test(input.key) - ) && - forwardedShortcuts.find(({ shortcut }) => - matchesKeybindingShortcut( - { - key: input.key, - code: input.code, - metaKey: input.meta, - ctrlKey: input.control, - shiftKey: input.shift, - altKey: input.alt, - }, - shortcut, - hostPlatform === "darwin" ? "MacIntel" : hostPlatform, - ), - ); + findForwardedShortcut({ + key: input.key, + code: input.code, + metaKey: input.meta, + ctrlKey: input.control, + shiftKey: input.shift, + altKey: input.alt, + }); if (forwarded && host) { + // The page keeps these keys. Only the main-frame preload can report one it + // left unhandled, so a focused iframe (e.g. Google Docs' input) keeps its own find. + if (PAGE_FIRST_FORWARDED_COMMANDS.has(forwarded.command)) return; event.preventDefault(); if (!input.isAutoRepeat) { if ( @@ -1515,6 +1551,15 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function return; } }; + const pageUnhandledShortcut = (_event: unknown, rawKeys?: unknown): void => { + const host = wc.hostWebContents; + if (!host || host.isDestroyed() || !holdsKeyboardFocus(wc)) return; + const keys = decodePageShortcutKeys(rawKeys); + if (Option.isNone(keys)) return; + const forwarded = findForwardedShortcut(keys.value); + if (!forwarded || !PAGE_FIRST_FORWARDED_COMMANDS.has(forwarded.command)) return; + host.send(MENU_ACTION_CHANNEL, forwarded.command); + }; yield* Scope.addFinalizer( scope, attempt({ operation: "detachListeners", tabId, webContentsId: wc.id }, () => { @@ -1529,11 +1574,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.off("dom-ready", restoreRecordingCursor); wc.off("did-fail-load", failed as never); wc.off("audio-state-changed", audioStateChanged); + wc.off("found-in-page", foundInPage as never); wc.off("did-create-window", windowCreated); wc.off("before-input-event", beforeInput); wc.ipc.off(HUMAN_INPUT_CHANNEL, humanInput); wc.ipc.off(RECORDING_INPUT_CHANNEL, recordingInput); wc.ipc.off(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); + wc.ipc.off(PAGE_UNHANDLED_SHORTCUT_CHANNEL, pageUnhandledShortcut); detachPasskeys(); }).pipe(Effect.ignore), ); @@ -1550,9 +1597,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.on("dom-ready", restoreRecordingCursor); wc.on("did-fail-load", failed as never); wc.on("audio-state-changed", audioStateChanged); + wc.on("found-in-page", foundInPage as never); wc.ipc.on(HUMAN_INPUT_CHANNEL, humanInput); wc.ipc.on(RECORDING_INPUT_CHANNEL, recordingInput); wc.ipc.on(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); + wc.ipc.on(PAGE_UNHANDLED_SHORTCUT_CHANNEL, pageUnhandledShortcut); detachPasskeys = passkeys.attachGuest(wc); wc.setWindowOpenHandler((details) => { const action = previewWindowOpenAction(details); @@ -2052,6 +2101,22 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const refresh = (tabId: string) => withWebContents("refresh", tabId, (wc) => wc.reload()); const hardReload = (tabId: string) => withWebContents("hardReload", tabId, (wc) => wc.reloadIgnoringCache()); + const findInPage = ( + tabId: string, + text: string, + options: { readonly forward: boolean; readonly newSession: boolean }, + ) => + withWebContents("findInPage", tabId, (wc) => { + // Electron's `findNext` means "begin a new session", not "go to the next match". + wc.findInPage(text, { forward: options.forward, findNext: options.newSession }); + }); + const stopFindInPage = (tabId: string) => + withWebContents("stopFindInPage", tabId, (wc) => wc.stopFindInPage("clearSelection")).pipe( + Effect.catchTags({ + PreviewTabNotFoundError: () => Effect.void, + PreviewWebContentsNotFoundError: () => Effect.void, + }), + ); const openDevTools = Effect.fn("PreviewManager.openDevTools")(function* (tabId: string) { const wc = yield* requireWebContents(tabId); @@ -3345,6 +3410,16 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function { discard: true }, ); }); + const emitFindResult = Effect.fn("PreviewManager.emitFindResult")(function* ( + event: DesktopPreviewFindResultEvent, + ) { + const listeners = yield* Ref.get(findResultListenersRef); + yield* Effect.forEach( + listeners, + (listener) => deliverEvent("find-result", event.tabId, () => listener(event)), + { discard: true }, + ); + }); const subscribe = ( ref: Ref.Ref>, listener: A, @@ -3367,6 +3442,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function Ref.set(listenersRef, new Set()), Ref.set(pointerEventListenersRef, new Set()), Ref.set(openLinkListenersRef, new Set()), + Ref.set(findResultListenersRef, new Set()), Ref.set(recordingInputListenersRef, new Set()), ], { discard: true }, @@ -3381,6 +3457,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function closeTab, copyArtifactToClipboard, createTab, + findInPage, goBack, goForward, hardReload, @@ -3410,10 +3487,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }), startRecording, closePictureInPicture, + stopFindInPage, stopRecording, subscribePointerEvents: (listener: PointerEventListener) => subscribe(pointerEventListenersRef, listener), subscribeOpenLinks: (listener: OpenLinkListener) => subscribe(openLinkListenersRef, listener), + subscribeFindResults: (listener: FindResultListener) => + subscribe(findResultListenersRef, listener), subscribeRecordingInputs: (listener: RecordingInputListener) => subscribe(recordingInputListenersRef, listener), subscribeStateChanges: (listener: Listener) => subscribe(listenersRef, listener), @@ -3595,6 +3675,12 @@ export class PreviewManager extends Context.Service< readonly resetZoom: (tabId: string) => Effect.Effect; readonly reapplyZoom: () => Effect.Effect; readonly hardReload: (tabId: string) => Effect.Effect; + readonly findInPage: ( + tabId: string, + text: string, + options: { readonly forward: boolean; readonly newSession: boolean }, + ) => Effect.Effect; + readonly stopFindInPage: (tabId: string) => Effect.Effect; readonly setColorScheme: ( tabId: string, colorScheme: DesktopPreviewColorScheme, @@ -3659,6 +3745,9 @@ export class PreviewManager extends Context.Service< readonly subscribeOpenLinks: ( listener: OpenLinkListener, ) => Effect.Effect; + readonly subscribeFindResults: ( + listener: FindResultListener, + ) => Effect.Effect; readonly subscribeRecordingInputs: ( listener: RecordingInputListener, ) => Effect.Effect; @@ -3760,6 +3849,8 @@ export const make = Effect.gen(function* PreviewManagerMake() { resetZoom: operations.resetZoom, reapplyZoom: operations.reapplyZoom, hardReload: operations.hardReload, + findInPage: operations.findInPage, + stopFindInPage: operations.stopFindInPage, setColorScheme: operations.setColorScheme, setZoomFactor: operations.setZoomFactor, setAudioMuted: operations.setAudioMuted, @@ -3808,6 +3899,7 @@ export const make = Effect.gen(function* PreviewManagerMake() { subscribeStateChanges: operations.subscribeStateChanges, subscribePointerEvents: operations.subscribePointerEvents, subscribeOpenLinks: operations.subscribeOpenLinks, + subscribeFindResults: operations.subscribeFindResults, subscribeRecordingInputs: operations.subscribeRecordingInputs, }); }).pipe(Effect.withSpan("PreviewManager.make")); diff --git a/apps/desktop/src/preview/PickPreload.ts b/apps/desktop/src/preview/PickPreload.ts index dcb16b851dc23..1d8c503d7edf8 100644 --- a/apps/desktop/src/preview/PickPreload.ts +++ b/apps/desktop/src/preview/PickPreload.ts @@ -33,6 +33,7 @@ import { ELEMENT_PICKED_CHANNEL, HUMAN_INPUT_CHANNEL, MOUSE_NAVIGATE_CHANNEL, + PAGE_UNHANDLED_SHORTCUT_CHANNEL, RECORDING_CURSOR_CHANNEL, RECORDING_POINTER_CHANNEL, RECORDING_KEY_CHANNEL, @@ -204,6 +205,26 @@ const reportHumanKeyInput = (event: KeyboardEvent): void => { window.addEventListener("pointerdown", reportHumanPointerInput, true); window.addEventListener("keydown", reportHumanKeyInput, true); +// Capture runs before a page can stop the event; the task delay lets every page +// listener call preventDefault before the host is told the key went unhandled. +const reportUnhandledShortcut = (event: KeyboardEvent): void => { + if (!event.isTrusted || event.repeat || event.isComposing) return; + if (!event.metaKey && !event.ctrlKey && !event.altKey && !/^F\d{1,2}$/u.test(event.key)) return; + setTimeout(() => { + if (event.defaultPrevented) return; + ipcRenderer.send(PAGE_UNHANDLED_SHORTCUT_CHANNEL, { + key: event.key, + code: event.code, + metaKey: event.metaKey, + ctrlKey: event.ctrlKey, + shiftKey: event.shiftKey, + altKey: event.altKey, + }); + }, 0); +}; + +window.addEventListener("keydown", reportUnhandledShortcut, true); + const MOUSE_BUTTON_BACK = 3; const MOUSE_BUTTON_FORWARD = 4; diff --git a/apps/desktop/src/snapShot/DesktopSnapShot.test.ts b/apps/desktop/src/snapShot/DesktopSnapShot.test.ts index 57a38d2ff314d..bd736f46b40fe 100644 --- a/apps/desktop/src/snapShot/DesktopSnapShot.test.ts +++ b/apps/desktop/src/snapShot/DesktopSnapShot.test.ts @@ -1048,7 +1048,6 @@ it.effect.each([ { backend: "gnome", screenshot: "PNG" }, { backend: "niri", screenshot: "PNG" }, { backend: "kde", screenshot: "PNG" }, - { backend: "hyprland", screenshot: "PNG" }, { backend: "gnome", screenshot: "invalid" }, ] as const)( "persists $backend accessibility text with a $screenshot screenshot", diff --git a/apps/desktop/src/ssh/DesktopSshPasswordPrompts.test.ts b/apps/desktop/src/ssh/DesktopSshPasswordPrompts.test.ts index c0f8210f18146..1e5b6dafc6cae 100644 --- a/apps/desktop/src/ssh/DesktopSshPasswordPrompts.test.ts +++ b/apps/desktop/src/ssh/DesktopSshPasswordPrompts.test.ts @@ -8,7 +8,10 @@ import * as TestClock from "effect/testing/TestClock"; import type * as Electron from "electron"; import * as ElectronWindow from "../electron/ElectronWindow.ts"; -import { SSH_PASSWORD_PROMPT_CHANNEL } from "../ipc/channels.ts"; +import { + SSH_PASSWORD_PROMPT_CHANNEL, + SSH_PASSWORD_PROMPT_SETTLED_CHANNEL, +} from "../ipc/channels.ts"; import * as DesktopSshPasswordPrompts from "./DesktopSshPasswordPrompts.ts"; interface SentMessage { @@ -144,6 +147,52 @@ describe("DesktopSshPasswordPrompts", () => { }).pipe(Effect.provide(layer(testWindow.window)), Effect.scoped); }); + it.effect.each(["answered", "cancelled", "timed out", "interrupted"] as const)( + "tells the window a prompt ended when it is %s", + (outcome) => { + const testWindow = makeTestWindow(); + + return Effect.gen(function* () { + const prompts = yield* DesktopSshPasswordPrompts.DesktopSshPasswordPrompts; + const fiber = yield* prompts + .request({ + destination: "devbox", + username: "julius", + prompt: "Enter the SSH password.", + attempt: 1, + }) + .pipe(Effect.exit, Effect.forkScoped); + + yield* Effect.yieldNow; + yield* Effect.yieldNow; + const request = testWindow.sentMessages[0]?.args[0] as { readonly requestId: string }; + assert.ok(request); + switch (outcome) { + case "answered": + yield* prompts.resolve({ requestId: request.requestId, password: "secret" }); + break; + case "cancelled": + yield* prompts.resolve({ requestId: request.requestId, password: null }); + break; + case "timed out": + yield* TestClock.adjust("1 second"); + break; + case "interrupted": + yield* Fiber.interrupt(fiber); + break; + } + yield* Fiber.await(fiber); + + assert.deepEqual( + testWindow.sentMessages + .filter((message) => message.channel === SSH_PASSWORD_PROMPT_SETTLED_CHANNEL) + .map((message) => message.args), + [[request.requestId]], + ); + }).pipe(Effect.provide(layer(testWindow.window)), Effect.scoped); + }, + ); + it.effect("keeps a submitted password when a later presentation step fails", () => { const testWindow = makeTestWindow({ isMinimizedError: new Error("failed to read minimized state"), diff --git a/apps/desktop/src/ssh/DesktopSshPasswordPrompts.ts b/apps/desktop/src/ssh/DesktopSshPasswordPrompts.ts index 3c16b938331ca..5fccc78d9a918 100644 --- a/apps/desktop/src/ssh/DesktopSshPasswordPrompts.ts +++ b/apps/desktop/src/ssh/DesktopSshPasswordPrompts.ts @@ -13,7 +13,10 @@ import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import * as ElectronWindow from "../electron/ElectronWindow.ts"; -import { SSH_PASSWORD_PROMPT_CHANNEL } from "../ipc/channels.ts"; +import { + SSH_PASSWORD_PROMPT_CHANNEL, + SSH_PASSWORD_PROMPT_SETTLED_CHANNEL, +} from "../ipc/channels.ts"; const DEFAULT_SSH_PASSWORD_PROMPT_TIMEOUT_MS = 3 * 60 * 1000; @@ -38,6 +41,7 @@ const DesktopSshPromptPresentationOperation = Schema.Literals([ "check-window-after-restore", "focus-window", "remove-window-close-listener", + "send-prompt-settled", ]); type DesktopSshPromptPresentationOperation = typeof DesktopSshPromptPresentationOperation.Type; @@ -371,11 +375,22 @@ export const make = Effect.fn("desktop.sshPasswordPrompts.make")(function* ( cause, }), }); + // However the request ends, the window drops its prompt, so no ended prompt stays on screen. + const notifySettled = runPresentationOperation("send-prompt-settled", () => { + if (!window.value.isDestroyed()) { + window.value.webContents.send(SSH_PASSWORD_PROMPT_SETTLED_CHANNEL, requestId); + } + }).pipe(Effect.ignore); const cleanup = runPresentationOperation("remove-window-close-listener", () => { if (!window.value.isDestroyed()) { window.value.removeListener("closed", cancelOnWindowClosed); } - }).pipe(Effect.orDie, Effect.ensuring(removePending(pendingRef, requestId)), Effect.asVoid); + }).pipe( + Effect.orDie, + Effect.ensuring(removePending(pendingRef, requestId)), + Effect.ensuring(notifySettled), + Effect.asVoid, + ); const waitForPassword = Deferred.await(deferred).pipe( Effect.timeoutOption(Duration.millis(passwordPromptTimeoutMs)), Effect.flatMap( diff --git a/apps/mobile/modules/t3-terminal/android/src/main/cpp/t3_terminal_jni.cpp b/apps/mobile/modules/t3-terminal/android/src/main/cpp/t3_terminal_jni.cpp index 95760e8ead926..23a1e0c07c995 100644 --- a/apps/mobile/modules/t3-terminal/android/src/main/cpp/t3_terminal_jni.cpp +++ b/apps/mobile/modules/t3-terminal/android/src/main/cpp/t3_terminal_jni.cpp @@ -12,7 +12,9 @@ namespace { constexpr uint32_t kSnapshotMagic = 0x54563354; // "T3VT" in little endian. constexpr uint16_t kSnapshotVersion = 1; -constexpr size_t kMaxScrollbackRows = 10000; +// The pinned ABI measures max_scrollback in bytes despite its header saying lines. +// Match Ghostty's default scrollback-limit. +constexpr size_t kMaxScrollbackBytes = 10'000'000; enum CellFlag : uint16_t { kBold = 1 << 0, @@ -205,7 +207,7 @@ Java_expo_modules_t3terminal_GhosttyBridge_nativeCreate( GhosttyTerminalOptions options = { .cols = static_cast(std::clamp(cols, 1, 65535)), .rows = static_cast(std::clamp(rows, 1, 65535)), - .max_scrollback = kMaxScrollbackRows, + .max_scrollback = kMaxScrollbackBytes, }; if (ghostty_terminal_new(nullptr, &session->terminal, options) != GHOSTTY_SUCCESS || ghostty_render_state_new(nullptr, &session->render_state) != GHOSTTY_SUCCESS || diff --git a/apps/mobile/src/features/diffs/nativeReviewDiffSource.test.ts b/apps/mobile/src/features/diffs/nativeReviewDiffSource.test.ts index 6c2cb8f0d3ad3..4950bb5ee4264 100644 --- a/apps/mobile/src/features/diffs/nativeReviewDiffSource.test.ts +++ b/apps/mobile/src/features/diffs/nativeReviewDiffSource.test.ts @@ -39,13 +39,6 @@ it("evicts source text by aggregate size and returns an over-budget response unc expect(fetch).toHaveBeenCalledTimes(5); }); -it("lets later viewport requests retry a failed source fetch", async () => { - const fetch = vi.fn().mockRejectedValueOnce(new Error("Offline")).mockResolvedValueOnce(SOURCE); - const load = createNativeReviewDiffSourceLoader(fetch); - await expect(load("retry.ts")).rejects.toThrow("Offline"); - expect(await load("retry.ts")).toBe(SOURCE); -}); - const failureKeys = [ ["string", () => "retry.ts"], ["number", () => 1], diff --git a/apps/mobile/src/features/home/useThreadListActions.test.ts b/apps/mobile/src/features/home/useThreadListActions.test.ts index 9313aef299881..64886697a5a4d 100644 --- a/apps/mobile/src/features/home/useThreadListActions.test.ts +++ b/apps/mobile/src/features/home/useThreadListActions.test.ts @@ -532,13 +532,6 @@ describe("thread action undo", () => { await undo(); expect(state.requests.map(({ action }) => action)).toEqual(["unpin"]); }); - - // regression: #21401 restore archive and thread action undo - it("refreshes the archive after archiving on a settlement-capable server", async () => { - await useThreadListActions().archiveThread(makeThread()); - expect(state.requests.map(({ action }) => action)).toEqual(["archive"]); - expect(state.archivedRefreshes).toEqual([primaryEnvironmentId]); - }); }); describe("thread archive guard", () => { diff --git a/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx b/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx index 40a7d0b675af4..e92debeea0f1d 100644 --- a/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx +++ b/apps/mobile/src/features/pull-requests/PullRequestDiscussionScreen.tsx @@ -11,7 +11,7 @@ import { squashAtomCommandFailure, } from "@t3tools/client-runtime/state/runtime"; import { AsyncResult } from "effect/reactivity"; -import type { PullRequestThreadComment } from "@t3tools/contracts"; +import { pullRequestConversationThreads, type PullRequestThreadComment } from "@t3tools/contracts"; import { useRef, useState } from "react"; import { ScrollView, View, type ScrollViewInstance } from "react-native"; import { KeyboardAvoidingView, useKeyboardState } from "react-native-keyboard-controller"; @@ -60,7 +60,12 @@ function Discussion({ params }: { params: PullRequestParams & { readonly discuss reportFailure: false, }); const loadMore = useAtomCommand(composerPullRequests.threadComments, { reportFailure: false }); - const thread = activity.data?.reviewThreads.find((thread) => thread.id === params.discussionId); + const thread = + activity.data && + pullRequestConversationThreads(activity.data).find( + (thread) => thread.id === params.discussionId, + ); + const filePath = thread?.path ?? null; const [body, setBody] = useState(""); const completions = usePullRequestCompletions({ environmentId: params.environmentId, @@ -200,20 +205,24 @@ function Discussion({ params }: { params: PullRequestParams & { readonly discuss {activity.isPending && !thread ? : null} {thread ? ( - - {thread.path} - {thread.line ? `:${thread.line}` : ""} - + {thread.path === null ? null : ( + + {thread.path} + {thread.line ? `:${thread.line}` : ""} + + )} {thread.isResolved ? "Resolved" : "Unresolved"} {thread.isOutdated ? " · Outdated" : ""} - + {focusConfirm && confirmVariant === "default" ? ( + Confirm + ) : ( + + )} diff --git a/apps/web/src/components/DiffPanel.browser.test.tsx b/apps/web/src/components/DiffPanel.browser.test.tsx index 874118daf306f..5f789469af763 100644 --- a/apps/web/src/components/DiffPanel.browser.test.tsx +++ b/apps/web/src/components/DiffPanel.browser.test.tsx @@ -121,7 +121,6 @@ function setup(failedBaseRef: string | null) { useDiffPanelStore.setState({ byThreadKey: {}, branchBaseRefByThreadKey: {}, - keepSelectionOnOpenThreadKey: null, }); useDiffPanelSessionStore.setState({ collapsedFileKeysByScopeKey: {} }); }; diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index 2e9af946d59bd..9767972d73d05 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -232,7 +232,8 @@ export default function DiffPanel({ })); const diffLayout = settings.diffLayout; const [wordWrap, setWordWrap] = useState(settings.wordWrap); - const [diffIgnoreWhitespace, setDiffIgnoreWhitespace] = useState(settings.diffIgnoreWhitespace); + const diffIgnoreWhitespace = + useDiffPanelSessionStore((state) => state.ignoreWhitespace) ?? settings.diffIgnoreWhitespace; const [fileTreeOpen, setFileTreeOpen] = useLocalStorage( DIFF_FILE_TREE_STORAGE_KEY, false, @@ -1222,7 +1223,7 @@ export default function DiffPanel({ size="sm" pressed={diffIgnoreWhitespace} onPressedChange={(pressed) => { - setDiffIgnoreWhitespace(Boolean(pressed)); + useDiffPanelSessionStore.getState().setIgnoreWhitespace(Boolean(pressed)); }} /> } diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index 543aaadb20b57..d0e243cda25c4 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -307,6 +307,37 @@ describe("resolveChangesTotals", () => { ); }); + it("uses the panel's branch comparison when it has file stats", () => { + const dirty = status({ + branchChanges: { baseRef: "main", insertions: 20, deletions: 3 }, + }); + const source = { + id: "branch-range", + kind: "branch-range", + title: "Changes vs release", + baseRef: "release", + headRef: "feature/test", + diff: "", + diffHash: "hash", + truncated: false, + } as const; + assert.deepEqual( + resolveChangesTotals(dirty, { + ...source, + files: [ + { path: "a.ts", previousPath: null, additions: 4, deletions: 1 }, + { path: "b.ts", previousPath: null, additions: 2, deletions: 0 }, + ], + }), + { insertions: 6, deletions: 1, baseRef: "release" }, + ); + assert.deepEqual(resolveChangesTotals(dirty, source), { + insertions: 20, + deletions: 3, + baseRef: "main", + }); + }); + it("falls back to the working tree when branchChanges is omitted", () => { assert.deepInclude( resolveChangesTotals( diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index c5814842eb045..399e013068364 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -1,6 +1,7 @@ import type { GitRunStackedActionResult, GitStackedAction, + ReviewDiffPreviewSource, VcsStatusResult, } from "@t3tools/contracts"; import { isTemporaryWorktreeBranch } from "@t3tools/shared/git"; @@ -448,9 +449,21 @@ export type ChangesLineTotals = { // `branchChanges ?? workingTree` treats `{0,0}` as present, so a zero branch total hid a dirty // working tree and the Changes row read +0 while Diff still showed the work. +// The Diff panel's branch comparison wins when it has file stats, so a remembered base or +// hidden whitespace reads the same in the row as in the panel. export function resolveChangesTotals( status: VcsStatusResult | null | undefined, + branchPreview?: ReviewDiffPreviewSource | null, ): ChangesLineTotals | null { + if (branchPreview?.files) { + let insertions = 0; + let deletions = 0; + for (const file of branchPreview.files) { + insertions += file.additions; + deletions += file.deletions; + } + return { insertions, deletions, baseRef: branchPreview.baseRef }; + } if (!status) return null; const branch = status.branchChanges; const tree = { ...status.workingTree, baseRef: null }; diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index 4e236f0b58c18..a9193f8fd35d9 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -94,6 +94,15 @@ import { import { useQuietThreadProjection, useQuietThreadShell } from "~/state/entities"; import { readEnvironmentScope, useEnvironmentScope } from "~/state/session"; import { useEnvironmentQuery } from "~/state/query"; +import { useFilesystemReadAccess } from "~/state/filesystem"; +import { reviewEnvironment } from "~/state/review"; +import { revisionedQuery } from "~/state/revisionedQuery"; +import { + selectThreadBranchBaseRef, + useDiffPanelSessionStore, + useDiffPanelStore, +} from "~/diffPanelStore"; +import { useClientSettings } from "~/hooks/useSettings"; import { serverEnvironment } from "~/state/server"; import { sourceControlEnvironment } from "~/state/sourceControl"; import { threadEnvironment } from "~/state/threads"; @@ -122,6 +131,7 @@ interface GitActionsControlProps { showActions?: boolean; showChanges?: boolean; onOpenChanges?: () => void; + workspaceMutationId?: string | null; } interface PendingDefaultBranchAction { @@ -933,6 +943,7 @@ export default function GitActionsControl({ showActions = true, showChanges = true, onOpenChanges, + workspaceMutationId = null, }: GitActionsControlProps) { const panelAnchorRef = useRef(null); const updateThreadMetadata = useAtomCommand( @@ -1050,7 +1061,53 @@ export default function GitActionsControl({ const isRepo = gitStatus?.isRepo ?? true; const hasPrimaryRemote = gitStatus?.hasPrimaryRemote ?? false; const gitStatusForActions = gitStatus; - const resolvedChangesTotals = resolveChangesTotals(gitStatusForActions); + const changesBaseRef = useDiffPanelStore((state) => + selectThreadBranchBaseRef(state, activeThreadRef), + ); + const ignoreWhitespaceSetting = useClientSettings((settings) => settings.diffIgnoreWhitespace); + const ignoreWhitespace = + useDiffPanelSessionStore((state) => state.ignoreWhitespace) ?? ignoreWhitespaceSetting; + const { canReadFiles } = useFilesystemReadAccess(activeEnvironmentId); + // Same input as the Diff panel's branch query, so both read one cached result. Status + // totals join the revision because edits, pulls and resets outside a turn leave the mutation + // id as is. + const changesRevision = gitStatusForActions + ? JSON.stringify([ + workspaceMutationId, + gitStatusForActions.refName, + gitStatusForActions.workingTree.insertions, + gitStatusForActions.workingTree.deletions, + gitStatusForActions.branchChanges?.baseRef, + gitStatusForActions.branchChanges?.insertions, + gitStatusForActions.branchChanges?.deletions, + ]) + : null; + const changesPreview = useEnvironmentQuery( + showChanges && + isRepo && + canReadFiles && + changesRevision !== null && + activeEnvironmentId !== null && + gitCwd !== null + ? revisionedQuery( + reviewEnvironment.diffPreview({ + environmentId: activeEnvironmentId, + input: { + cwd: gitCwd, + ...(changesBaseRef ? { baseRef: changesBaseRef } : {}), + ignoreWhitespace, + metadataOnly: true, + sourceKind: "branch-range", + }, + }), + changesRevision, + ) + : null, + ); + const resolvedChangesTotals = resolveChangesTotals( + gitStatusForActions, + changesPreview.data?.sources.find((source) => source.kind === "branch-range"), + ); const changesTotals = resolvedChangesTotals ?? { insertions: 0, deletions: 0 }; const changesBaseLabel = resolvedChangesTotals ? resolvedChangesTotals.baseRef === null diff --git a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts index d6e283fec8dbb..764a76ba74f12 100644 --- a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts +++ b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts @@ -318,6 +318,41 @@ describe("getProviderUpdateRunToastView", () => { result, }); + it("preserves the local server restart instruction after a successful update", () => { + const message = + "OpenCode CLI updated. Restart the configured local OpenCode server to use the new version."; + const updated = { + ...provider({ + driver: driver("opencode"), + version: "2.1.2", + advisoryStatus: "current", + updateState: updateState("succeeded", message), + }), + installedVersion: "2.2.5", + }; + expect( + getProviderUpdateRunToastView([ + run("Linux", "opencode", AsyncResult.success({ providers: [updated] })), + run("Laptop", "opencode", AsyncResult.success({ providers: [updated] })), + ]), + ).toMatchObject({ + type: "success", + description: `Linux · OpenCode: ${message}\nLaptop · OpenCode: ${message}`, + }); + expect( + getProviderUpdateSidebarPillView([updated], { + visibleAfterIso: sessionStartedAt, + }), + ).toMatchObject({ title: "OpenCode updated: v2.2.5", description: message }); + + const restarted = { ...updated, version: "2.2.5" }; + expect( + getProviderUpdateRunToastView([ + run("Linux", "opencode", AsyncResult.success({ providers: [restarted] })), + ]), + ).toMatchObject({ description: "New sessions will use the updated provider." }); + }); + it("lists every failed update and ignores interrupted ones", () => { const view = getProviderUpdateRunToastView([ run( diff --git a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts index 1c8a97a998112..9c9720d7e7e30 100644 --- a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts +++ b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts @@ -78,11 +78,24 @@ function dedupeProvidersByDriver(providers: ReadonlyAr return [...latestProviderByDriver.values()]; } -function getProviderUpdatedTitle(provider: Pick): string { +function getProviderUpdatedTitle( + provider: Pick, +): string { const providerName = PROVIDER_DISPLAY_NAMES[provider.driver] ?? provider.driver; - return provider.version - ? `${providerName} updated: ${formatVersion(provider.version)}` - : `${providerName} updated`; + const version = provider.installedVersion ?? provider.version; + return version ? `${providerName} updated: ${formatVersion(version)}` : `${providerName} updated`; +} + +// Only OpenCode reports an installed CLI apart from a running external server. +export function getProviderRestartNotice( + provider: Pick, +): string | null { + return provider.updateState?.status === "succeeded" && + provider.installedVersion != null && + provider.version != null && + provider.installedVersion !== provider.version + ? provider.updateState.message + : null; } function getProviderUpdatedDescription(providerCount: number): string { @@ -200,7 +213,8 @@ export interface ProviderUpdateRun { export function getProviderUpdateRunOutcome( run: ProviderUpdateRun, ): - | { readonly status: "interrupted" | "updated" } + | { readonly status: "interrupted" } + | { readonly status: "updated"; readonly message: string | null } | { readonly status: "failed"; readonly line: string } { if (isAtomCommandInterrupted(run.result)) return { status: "interrupted" }; const label = `${run.machineLabel} · ${PROVIDER_DISPLAY_NAMES[run.driver] ?? run.driver}`; @@ -211,11 +225,13 @@ export function getProviderUpdateRunOutcome( line: `${label}: ${error instanceof Error ? error.message : "Provider update failed."}`, }; } - const updateState = run.result.value.providers.find( - (provider) => provider.instanceId === run.instanceId, - )?.updateState; + const provider = run.result.value.providers.find( + (candidate) => candidate.instanceId === run.instanceId, + ); + const updateState = provider?.updateState; + const restartNotice = provider ? getProviderRestartNotice(provider) : null; return updateState?.status === "succeeded" - ? { status: "updated" } + ? { status: "updated", message: restartNotice ? `${label}: ${restartNotice}` : null } : { status: "failed", line: `${label}: ${updateState?.message ?? "Provider update did not finish."}`, @@ -236,11 +252,18 @@ export function getProviderUpdateRunToastView(runs: ReadonlyArray outcome.status === "failed" ? [outcome.line] : [], ); + const successMessages = [ + ...new Set( + settled.flatMap((outcome) => + outcome.status === "updated" && outcome.message ? [outcome.message] : [], + ), + ), + ]; if (failureLines.length === 0) { return { type: "success", title: settled.length === 1 ? "Provider updated" : `${settled.length} providers updated`, - description: getProviderUpdatedDescription(settled.length), + description: successMessages.join("\n") || getProviderUpdatedDescription(settled.length), }; } return { @@ -251,7 +274,7 @@ export function getProviderUpdateRunToastView(runs: ReadonlyArray diff --git a/apps/web/src/components/ServerUpdateAction.tsx b/apps/web/src/components/ServerUpdateAction.tsx index 32a2811aca189..14e0dd3c192cb 100644 --- a/apps/web/src/components/ServerUpdateAction.tsx +++ b/apps/web/src/components/ServerUpdateAction.tsx @@ -202,6 +202,7 @@ export function ServerUpdatesAction({ const confirmed = (await requestConfirmDialog( `Update the T3 Code desktop apps on ${desktopTargets.map((target) => target.serverLabel).join(", ")}? They will close and relaunch on those machines.`, + { initialFocus: "confirm" }, )) ?? true; if (!confirmed) return; } @@ -308,6 +309,7 @@ export function ServerUpdateAction({ const confirmed = (await requestConfirmDialog( `Update the T3 Code desktop app that runs the ${serverLabel}? It will close and relaunch on that machine.`, + { initialFocus: "confirm" }, )) ?? true; if (!confirmed) { return; diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index cba7c434b1bab..04385a5a6b873 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -223,6 +223,7 @@ import { cn, isMacPlatform, randomUUID } from "~/lib/utils"; import { getComposerPromptLengthValidationMessage, getComposerSubmissionValidationMessage, + isComposerSubmissionBlocked, submitComposerDraft, } from "./composerSubmission"; import { ComposerPromptLengthValidation } from "./ComposerPromptLengthValidation"; @@ -369,7 +370,7 @@ export interface ChatComposerProps { isRevertingCheckpoint?: boolean; sendDisabledReason: string | null; isPreparingWorktree: boolean; - isCompacting: boolean; + canSteer: boolean; bannerItems: readonly ComposerBannerStackItem[]; resumeCompactionTokens: number | null; keepFullHistory: boolean; @@ -515,7 +516,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) isRevertingCheckpoint = false, sendDisabledReason: externalSendDisabledReason, isPreparingWorktree, - isCompacting, + canSteer, environmentUnavailable, activePendingApproval: incomingPendingApproval, pendingApprovals, @@ -1823,8 +1824,21 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) }, }); const { blocksSubmission: voiceInputActive, submitAfterDictation } = speechInput; - const voiceSendDisabledReason = - sendDisabledReason ?? (speechInput.preparing ? "Preparing voice input" : null); + const submissionTarget = activePendingProgress ? "pending-user-input" : "provider-turn"; + const submissionBlocked = isComposerSubmissionBlocked({ + submissionTarget, + noProviderAvailable, + sendDisabledReason, + attachmentBlockReason, + environmentUnavailable: environmentUnavailable !== null, + }); + // The pending Submit button follows the answer's own blockers, matching submitComposer. + const primaryActionSendDisabledReason = + (activePendingProgress ? attachmentBlockReason : sendDisabledReason) ?? + (speechInput.preparing ? "Preparing voice input" : null); + const primaryActionEnvironmentUnavailable = + environmentUnavailable !== null || + (!activePendingProgress && (noProviderAvailable || projectSelectionRequired)); const dictationShortcutLabel = useDictationShortcut({ keybindings, speech: speechInput, @@ -2070,13 +2084,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) const shouldBlurMobileComposerOnSubmit = () => { if (!isSoftwareKeyboardOpen) return false; - if ( - isSendBusy || - isSendDisabled || - noProviderAvailable || - environmentUnavailable !== null || - phase === "running" - ) { + if (isSendBusy || submissionBlocked || environmentUnavailable !== null || phase === "running") { return false; } if (activePendingProgress) { @@ -2098,11 +2106,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) submitAfterDictation(() => submitComposerRef.current(dispatchMode, submissionIntent)); return false; } - if ( - noProviderAvailable || - isSendDisabled || - !readEnvironmentScope(environmentId, AuthOrchestrationOperateScope) - ) { + if (submissionBlocked || !readEnvironmentScope(environmentId, AuthOrchestrationOperateScope)) { event?.preventDefault(); return false; } @@ -2129,7 +2133,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) } const submission = submitComposerDraft({ prompt: promptRef.current, - submissionTarget: activePendingProgress ? "pending-user-input" : "provider-turn", + submissionTarget, event, onSend: (sendEvent) => { providerInputRejectedRef.current = false; @@ -2140,7 +2144,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) running: phase === "running", alternateModifier: false, activeTurnDefault: followUpBehavior, - canSteer: !isCompacting, + canSteer, }), submissionIntent, ); @@ -2181,7 +2185,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) resolveComposerDispatchMode({ running: phase === "running", alternateModifier: intent === "alternate", - canSteer: !isCompacting, + canSteer, activeTurnDefault: followUpBehavior, }), intent, @@ -2196,7 +2200,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) running: phase === "running", alternateModifier: event.metaKey || event.ctrlKey, activeTurnDefault: followUpBehavior, - canSteer: !isCompacting, + canSteer, }), ); }; @@ -2208,7 +2212,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) running: phase === "running", alternateModifier: false, activeTurnDefault: followUpBehavior, - canSteer: !isCompacting, + canSteer, }), ); }; @@ -2289,7 +2293,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) sendShortcut: settings.sendShortcut, routeKind, phase, - isCompacting, + canSteer, followUpBehavior, isSoftwareKeyboardOpen, isLiteralPendingAnswer, @@ -3518,12 +3522,8 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) isRunning={false} canInterrupt={false} isSendBusy={isSendBusy} - sendDisabledReason={voiceSendDisabledReason} - isEnvironmentUnavailable={ - environmentUnavailable !== null || - noProviderAvailable || - projectSelectionRequired - } + sendDisabledReason={primaryActionSendDisabledReason} + isEnvironmentUnavailable={primaryActionEnvironmentUnavailable} isPreparingWorktree={false} hasSendableContent={false} preserveComposerFocusOnPointerDown @@ -3578,13 +3578,16 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) {composerControls} void) | undefined; readonly environmentId: EnvironmentId; readonly threadId: ThreadId; readonly itemId: TurnItemId; @@ -35,6 +36,7 @@ export function CommandOutputPanel(props: { ? props.source(input) : orchestrationEnvironment.v2.commandOutput({ environmentId: props.environmentId, input }), ); + useInitialOutputReady(data === null && !error, props.onInitialOutputReady); const scrollRef = useRef(null); const followRef = useRef(true); const text = data?.output.text ?? ""; diff --git a/apps/web/src/components/chat/ComposerFooterControls.tsx b/apps/web/src/components/chat/ComposerFooterControls.tsx index 2e2e9323b3eda..3f753daed599c 100644 --- a/apps/web/src/components/chat/ComposerFooterControls.tsx +++ b/apps/web/src/components/chat/ComposerFooterControls.tsx @@ -199,6 +199,7 @@ export const ComposerFooterPrimaryActions = memo(function ComposerFooterPrimaryA } | null; isRunning: boolean; canInterrupt: boolean; + runId: string | null; followUpBehavior: "queue" | "steer"; canSteer: boolean; alternateShortcutLabel: string | null; @@ -226,6 +227,7 @@ export const ComposerFooterPrimaryActions = memo(function ComposerFooterPrimaryA pendingAction={props.pendingAction} isRunning={props.isRunning} canInterrupt={props.canInterrupt} + runId={props.runId} followUpBehavior={props.followUpBehavior} canSteer={props.canSteer} alternateShortcutLabel={props.alternateShortcutLabel} diff --git a/apps/web/src/components/chat/ComposerPrimaryActions.browser.test.tsx b/apps/web/src/components/chat/ComposerPrimaryActions.browser.test.tsx new file mode 100644 index 0000000000000..8c99608c3f805 --- /dev/null +++ b/apps/web/src/components/chat/ComposerPrimaryActions.browser.test.tsx @@ -0,0 +1,144 @@ +import "~/index.css"; + +import type { ComponentProps } from "react"; +import { expect, it, vi } from "vite-plus/test"; +import { act, fakeTimers, render } from "../../../test/render"; + +vi.mock("~/hooks/useSettings", () => ({ + useEnvironmentIdentificationMode: () => "none", + useClientSettings: () => false, +})); +vi.mock("../SidebarStageBackdrop", () => ({ + StageBackdropButtonArt: () => null, + useSidebarStageBackdropVariant: () => null, +})); + +import { ComposerPrimaryActions } from "./ComposerPrimaryActions"; + +function setup() { + return { + compact: true, + canOperateThread: true, + pendingAction: null, + isRunning: true, + canInterrupt: true, + canResume: false, + isSendBusy: false, + sendDisabledReason: null, + isEnvironmentUnavailable: false, + isPreparingWorktree: false, + hasSendableContent: false, + onPreviousPendingQuestion: vi.fn(), + onInterrupt: vi.fn(), + onResume: vi.fn(), + onSubmitMessage: vi.fn(), + } satisfies ComponentProps; +} + +it.each([false, true])( + "keeps repeat clicks away from Resume and Send (draft: %s)", + async (draft) => { + const props = setup(); + const container = document.createElement("div"); + Object.assign(container.style, { display: "flex", justifyContent: "flex-end", width: "400px" }); + await using screen = await render(, { container }); + const stop = screen + .getByRole("button", { name: "Stop generation" }) + .element() as HTMLButtonElement; + const { x, y, width, height } = stop.getBoundingClientRect(); + const target = () => + document.elementFromPoint(x + width / 2, y + height / 2)?.closest("button"); + await act(() => stop.click()); + await act(() => + ( + screen.getByRole("button", { name: "Stopping", exact: true }).element() as HTMLButtonElement + ).click(), + ); + await expect + .element(screen.getByRole("button", { name: "Stopping", exact: true })) + .toBeDisabled(); + expect(props.onInterrupt).toHaveBeenCalledTimes(1); + await screen.rerender( + , + ); + await expect + .element(screen.getByRole("button", { name: "Stopped", exact: true })) + .toBeDisabled(); + expect(target()).toHaveAttribute("aria-label", "Stopped"); + await act(() => { + for (let i = 0; i < 20; i++) target()?.click(); + }); + expect(props.onResume).not.toHaveBeenCalled(); + expect(props.onSubmitMessage).not.toHaveBeenCalled(); + const action = screen + .getByRole("button", { name: draft ? "Submit message" : "Resume thread" }) + .element() as HTMLButtonElement; + await act(() => action.click()); + expect(draft ? props.onSubmitMessage : props.onResume).toHaveBeenCalledTimes(1); + await screen.rerender(); + await expect.element(screen.getByRole("button", { name: "Stop generation" })).toBeEnabled(); + }, +); + +it("allows retry after ten seconds and protects a late stop", async () => { + const props = setup(); + await using screen = await render(); + using _timers = fakeTimers(); + await act(() => + ( + screen.getByRole("button", { name: "Stop generation" }).element() as HTMLButtonElement + ).click(), + ); + await act(() => vi.advanceTimersByTime(9_999)); + expect(screen.getByRole("button", { name: "Stopping", exact: true }).element()).toBeDisabled(); + await act(() => vi.advanceTimersByTime(1)); + await act(() => + ( + screen.getByRole("button", { name: "Stop generation" }).element() as HTMLButtonElement + ).click(), + ); + await screen.rerender( + , + ); + expect(screen.getByRole("button", { name: "Stopped", exact: true }).element()).toBeDisabled(); + expect(props.onInterrupt).toHaveBeenCalledTimes(2); + expect(props.onResume).not.toHaveBeenCalled(); +}); + +it("keeps Stop pending across a permission failure", async () => { + const props = setup(); + await using screen = await render(); + await act(() => + ( + screen.getByRole("button", { name: "Stop generation" }).element() as HTMLButtonElement + ).click(), + ); + await screen.rerender( + , + ); + await screen.rerender(); + await expect + .element(screen.getByRole("button", { name: "Stopping", exact: true })) + .toBeDisabled(); + expect(props.onInterrupt).toHaveBeenCalledTimes(1); +}); + +it("keeps a pending stop when the run id clears and resets it for a different run", async () => { + const props = setup(); + await using screen = await render(); + await act(() => + ( + screen.getByRole("button", { name: "Stop generation" }).element() as HTMLButtonElement + ).click(), + ); + await screen.rerender(); + expect(screen.getByRole("button", { name: "Stopping", exact: true }).element()).toBeDisabled(); + await screen.rerender(); + expect(screen.getByRole("button", { name: "Stop generation" }).element()).toBeEnabled(); +}); diff --git a/apps/web/src/components/chat/ComposerPrimaryActions.tsx b/apps/web/src/components/chat/ComposerPrimaryActions.tsx index ec9ec10332b7a..8e28cc1dd6af6 100644 --- a/apps/web/src/components/chat/ComposerPrimaryActions.tsx +++ b/apps/web/src/components/chat/ComposerPrimaryActions.tsx @@ -1,4 +1,11 @@ -import { memo, type MouseEventHandler, type PointerEventHandler, type ReactNode } from "react"; +import { + memo, + useEffect, + useState, + type MouseEventHandler, + type PointerEventHandler, + type ReactNode, +} from "react"; import { CheckIcon, ChevronLeftIcon, @@ -36,6 +43,7 @@ interface ComposerPrimaryActionsProps { pendingAction: PendingActionState | null; isRunning: boolean; canInterrupt: boolean; + runId?: string | null; followUpBehavior?: "queue" | "steer"; canSteer?: boolean; alternateShortcutLabel?: string | null; @@ -76,6 +84,8 @@ const formatPendingPrimaryActionLabel = (input: { return input.questionIndex > 0 ? "Submit answers" : "Submit answer"; }; +const STOP_PENDING_MAX_MS = 10_000; + const messageActionPillClassName = "inline-flex shrink-0 cursor-pointer items-center justify-center gap-1.5 whitespace-nowrap rounded-full bg-message-action font-medium text-base text-message-action-foreground shadow-xs shadow-message-action/24 outline-none hover:bg-message-action-hover focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background disabled:pointer-events-none disabled:opacity-64 disabled:shadow-none sm:text-sm"; @@ -90,6 +100,7 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({ pendingAction, isRunning, canInterrupt, + runId = null, followUpBehavior = "steer", canSteer = true, alternateShortcutLabel = null, @@ -110,6 +121,26 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({ keepFullHistory = false, onToggleKeepFullHistory, }: ComposerPrimaryActionsProps) { + const [stopPhase, setStopPhase] = useState<"pending" | "timedOut" | "landed" | null>(null); + const [stopRunId, setStopRunId] = useState(runId); + if (runId !== null && stopRunId !== runId) { + setStopRunId(runId); + setStopPhase(null); + } else if (canOperateThread) { + // Losing permission also clears canInterrupt; it does not acknowledge the stop. + if ((stopPhase === "pending" || stopPhase === "timedOut") && !canInterrupt) + setStopPhase("landed"); + else if (stopPhase === "landed" && canInterrupt) setStopPhase(null); + } + const isStopDisabled = stopPhase === "pending" || stopPhase === "landed"; + const stopLabel = + stopPhase === "landed" ? "Stopped" : stopPhase === "pending" ? "Stopping" : "Stop generation"; + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- timer that re-enables Stop if the interrupt never settles + useEffect(() => { + if (stopPhase !== "pending") return; + const timeout = setTimeout(() => setStopPhase("timedOut"), STOP_PENDING_MAX_MS); + return () => clearTimeout(timeout); + }, [stopPhase]); const pointerFocusProps = preserveComposerFocusOnPointerDown ? { onPointerDown: preventPointerFocus } : undefined; @@ -134,15 +165,17 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({ ) : null} {disconnectAction} diff --git a/apps/web/src/components/cloud/RelayClientInstallDialog.tsx b/apps/web/src/components/cloud/RelayClientInstallDialog.tsx index 78282f659151f..cff5a659c95a4 100644 --- a/apps/web/src/components/cloud/RelayClientInstallDialog.tsx +++ b/apps/web/src/components/cloud/RelayClientInstallDialog.tsx @@ -12,6 +12,7 @@ import { Button } from "../ui/button"; import { Dialog, DialogDescription, + DialogAction, DialogFooter, DialogHeader, DialogPanel, @@ -112,9 +113,9 @@ export function RelayClientInstallDialog() { > Cancel - + ) : null} diff --git a/apps/web/src/components/desktop/SshPasswordPromptDialog.browser.test.tsx b/apps/web/src/components/desktop/SshPasswordPromptDialog.browser.test.tsx index 53eaec1822253..c4f66b1918a65 100644 --- a/apps/web/src/components/desktop/SshPasswordPromptDialog.browser.test.tsx +++ b/apps/web/src/components/desktop/SshPasswordPromptDialog.browser.test.tsx @@ -4,28 +4,67 @@ import { expect, it } from "vite-plus/test"; import { render, stubGlobal } from "../../../test/render"; import { SshPasswordPromptDialog, startSshPasswordPromptQueue } from "./SshPasswordPromptDialog"; -it("shows a prompt that arrived before the dialog mounted", async () => { +function stubPromptBridge() { const listeners: Array<(request: DesktopSshPasswordPromptRequest) => void> = []; - using _bridge = stubGlobal("desktopBridge", { + const settledListeners: Array<(requestId: string) => void> = []; + const bridge = stubGlobal("desktopBridge", { onSshPasswordPrompt: (listener: (request: DesktopSshPasswordPromptRequest) => void) => { listeners.push(listener); return () => listeners.splice(listeners.indexOf(listener), 1); }, + onSshPasswordPromptSettled: (listener: (requestId: string) => void) => { + settledListeners.push(listener); + return () => settledListeners.splice(settledListeners.indexOf(listener), 1); + }, resolveSshPasswordPrompt: async () => undefined, }); + return { + prompt: (requestId: string, destination: string) => { + for (const listener of listeners) { + listener({ + requestId, + destination, + username: "julius", + prompt: `julius@${destination}'s password:`, + expiresAt: "2999-01-01T00:00:00.000Z", + }); + } + }, + settle: (requestId: string) => { + for (const listener of settledListeners) listener(requestId); + }, + [Symbol.dispose]: () => bridge[Symbol.dispose](), + }; +} + +it("drops a prompt the desktop ended, whether it is on screen or still waiting", async () => { + using bridge = stubPromptBridge(); + const stopQueue = startSshPasswordPromptQueue(); + using _queue = { [Symbol.dispose]: stopQueue }; + await using screen = await render(); + + bridge.prompt("ssh-1", "first-host"); + bridge.prompt("ssh-1", "first-host"); + bridge.prompt("ssh-2", "second-host"); + bridge.prompt("ssh-3", "third-host"); + await expect.element(screen.getByText("julius@first-host")).toBeVisible(); + + // ssh-2 ended while it waited behind ssh-1, and ssh-1 ended on screen. + bridge.settle("ssh-2"); + bridge.settle("ssh-1"); + await expect.element(screen.getByText("julius@third-host")).toBeVisible(); + + bridge.settle("ssh-3"); + await expect.element(screen.getByText("SSH verification required")).not.toBeInTheDocument(); +}); + +it("shows a prompt that arrived before the dialog mounted", async () => { + using bridge = stubPromptBridge(); const stopQueue = startSshPasswordPromptQueue(); using _queue = { [Symbol.dispose]: stopQueue }; // The desktop asks while the app is still on a page without the dialog (pairing, sign-in). - for (const listener of listeners) { - listener({ - requestId: "ssh-1", - destination: "devbox", - username: "julius", - prompt: "julius@devbox's password:", - expiresAt: "2999-01-01T00:00:00.000Z", - }); - } + bridge.prompt("ssh-1", "devbox"); await using screen = await render(); await expect.element(screen.getByText("SSH verification required")).toBeVisible(); diff --git a/apps/web/src/components/desktop/SshPasswordPromptDialog.tsx b/apps/web/src/components/desktop/SshPasswordPromptDialog.tsx index e09836a378c60..bc97e65c7f995 100644 --- a/apps/web/src/components/desktop/SshPasswordPromptDialog.tsx +++ b/apps/web/src/components/desktop/SshPasswordPromptDialog.tsx @@ -47,11 +47,17 @@ function setPromptQueue(next: readonly DesktopSshPasswordPromptRequest[]) { * that arrives while no dialog is mounted still waits for one. */ export function startSshPasswordPromptQueue(): () => void { - return ( - window.desktopBridge?.onSshPasswordPrompt((request) => { - setPromptQueue([...promptQueue, request]); - }) ?? (() => undefined) - ); + const bridge = window.desktopBridge; + if (!bridge) return () => undefined; + const stopPrompts = bridge.onSshPasswordPrompt((request) => { + if (promptQueue.some((queued) => queued.requestId === request.requestId)) return; + setPromptQueue([...promptQueue, request]); + }); + const stopSettled = bridge.onSshPasswordPromptSettled(removeSshPasswordPrompt); + return () => { + stopPrompts(); + stopSettled(); + }; } function subscribePromptQueue(listener: () => void): () => void { @@ -64,7 +70,8 @@ function subscribePromptQueue(listener: () => void): () => void { const readPromptQueue = () => promptQueue; function removeSshPasswordPrompt(requestId: string) { - if (promptQueue[0]?.requestId === requestId) setPromptQueue(promptQueue.slice(1)); + if (!promptQueue.some((request) => request.requestId === requestId)) return; + setPromptQueue(promptQueue.filter((request) => request.requestId !== requestId)); } export function SshPasswordPromptDialog() { diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 1455fc849a7f1..0311ce254ff4f 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -71,6 +71,7 @@ import { buildFileReviewComment } from "~/reviewCommentContext"; import { assetEnvironment } from "~/state/assets"; import { usePreviewAvailable } from "~/browser/previewRuntime"; import { useEnvironmentHttpBaseUrl, usePrimaryEnvironmentId } from "~/state/environments"; +import { hostEnvironmentIdAtom } from "~/state/primaryEnvironment"; import { previewEnvironment } from "~/state/preview"; import { serverEnvironment } from "~/state/server"; import { useEnvironmentScope } from "~/state/session"; @@ -1025,6 +1026,7 @@ export default function FilePreviewPanel({ const wordWrap = wordWrapOverride ?? defaultWordWrap; const canOperatePreview = useEnvironmentScope(environmentId, AuthPreviewOperateScope); const primaryEnvironmentId = usePrimaryEnvironmentId(); + const hostEnvironmentId = useAtomValue(hostEnvironmentIdAtom); const remoteOpenState = useRemoteOpenState(environmentId); const serverConfig = useAtomValue(serverEnvironment.configValueAtom(environmentId)); const environmentHttpBaseUrl = useEnvironmentHttpBaseUrl(environmentId); @@ -1281,7 +1283,9 @@ export default function FilePreviewPanel({ {absolutePath && - (environmentId === primaryEnvironmentId || remoteOpenState.mode !== "local-exec") ? ( + (environmentId === primaryEnvironmentId || + environmentId === hostEnvironmentId || + remoteOpenState.mode !== "local-exec") ? ( void; +} + +export function PreviewFindBar({ tabId, focusRequestId, onClose }: PreviewFindBarProps) { + const inputRef = useRef(null); + const [query, setQuery] = useState(""); + const [result, setResult] = useState<{ active: number; matches: number } | null>(null); + + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- focuses and selects the input DOM node when the bar opens or a find shortcut fires + useEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + // oxlint-disable-next-line react/exhaustive-effect-dependencies -- Each find shortcut bumps the request id, so an already-open bar refocuses and reselects its query. + }, [focusRequestId]); + + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- subscribes to Chromium find results for this tab and stops the search on unmount + useEffect(() => { + if (!previewBridge) return; + const unsubscribe = previewBridge.onFindResult((event) => { + if (event.tabId !== tabId) return; + // Chromium reports -1 for a field an update leaves unchanged. + setResult((previous) => ({ + active: event.activeMatchOrdinal >= 0 ? event.activeMatchOrdinal : (previous?.active ?? 0), + matches: event.matches >= 0 ? event.matches : (previous?.matches ?? 0), + })); + }); + return () => { + unsubscribe(); + void previewBridge?.stopFindInPage(tabId).catch(() => undefined); + }; + }, [tabId]); + + const search = (text: string, options: { forward: boolean; newSession: boolean }) => { + if (!previewBridge) return; + void previewBridge.findInPage(tabId, text, options).catch(() => undefined); + }; + + const handleQueryChange = (next: string) => { + setQuery(next); + if (next.length === 0) { + setResult(null); + void previewBridge?.stopFindInPage(tabId).catch(() => undefined); + return; + } + search(next, { forward: true, newSession: true }); + }; + + const step = (forward: boolean) => { + if (query.length === 0) return; + search(query, { forward, newSession: false }); + }; + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.nativeEvent.isComposing || event.nativeEvent.keyCode === 229) return; + if (event.key === "Enter") { + event.preventDefault(); + event.stopPropagation(); + step(!event.shiftKey); + } else if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); + onClose(); + } + }; + + const hasQuery = query.length > 0; + const noResults = hasQuery && result !== null && result.matches === 0; + const label = + hasQuery && result ? `${result.matches === 0 ? 0 : result.active}/${result.matches}` : ""; + const navigationDisabled = !result || result.matches === 0; + + return ( +
+ + handleQueryChange(event.target.value)} + onKeyDown={handleKeyDown} + /> + + + {label} + + + + + + +
+ ); +} diff --git a/apps/web/src/components/preview/PreviewMoreMenu.tsx b/apps/web/src/components/preview/PreviewMoreMenu.tsx index 3e0f6298ce8d2..963c74a6743df 100644 --- a/apps/web/src/components/preview/PreviewMoreMenu.tsx +++ b/apps/web/src/components/preview/PreviewMoreMenu.tsx @@ -6,6 +6,7 @@ import { Menu, MenuItem, MenuPopup, + MenuShortcut, MenuRadioGroup, MenuRadioItem, MenuGroup, @@ -37,6 +38,7 @@ export interface PreviewMoreMenuActions { readonly clearCache: () => void; readonly openDevTools?: () => void; readonly toggleNativePictureInPicture?: () => void; + readonly find?: () => void; } interface Props { @@ -48,6 +50,7 @@ interface Props { onToggleDeviceToolbar: () => void; nativePictureInPicture: boolean; profileName: string | undefined; + findShortcutLabel?: string | null; move?: { readonly label: string; readonly onMove: () => void }; } @@ -75,6 +78,7 @@ export function PreviewMoreMenu({ onToggleDeviceToolbar, nativePictureInPicture, profileName, + findShortcutLabel, move, }: Props) { const zoomLabel = `${Math.round(zoomFactor * 100)}%`; @@ -83,6 +87,12 @@ export function PreviewMoreMenu({ + {actions.find ? ( + + Find on page + {findShortcutLabel ? {findShortcutLabel} : null} + + ) : null} Hard reload diff --git a/apps/web/src/components/preview/PreviewView.browser.test.tsx b/apps/web/src/components/preview/PreviewView.browser.test.tsx index 1f1494fa2736d..72b4d04f5b516 100644 --- a/apps/web/src/components/preview/PreviewView.browser.test.tsx +++ b/apps/web/src/components/preview/PreviewView.browser.test.tsx @@ -73,6 +73,9 @@ vi.mock(import("./previewBridge"), async (original) => ({ open: mocks.openPictureInPicture, close: mocks.closePictureInPicture, }, + findInPage: vi.fn(async () => undefined), + stopFindInPage: vi.fn(async () => undefined), + onFindResult: vi.fn(() => () => undefined), } as never, })); vi.mock(import("~/browserHistoryStore"), async (original) => ({ @@ -388,4 +391,21 @@ describe("PreviewView", () => { act(() => dispatchPreviewAction("focus-url")); await expect.element(address).toHaveFocus(); }); + + it("does not reopen find when the user returns to a tab they left with find open", async () => { + setup(); + const tabView = (tabId: string) => ( + + ); + await using screen = await render(tabView("tab-1")); + const findBar = page.getByRole("search", { name: "Find in page" }); + + act(() => dispatchPreviewAction("find")); + await expect.element(findBar).toBeInTheDocument(); + + await screen.rerender(tabView("tab-2")); + await expect.element(findBar).not.toBeInTheDocument(); + await screen.rerender(tabView("tab-1")); + await expect.element(findBar).not.toBeInTheDocument(); + }); }); diff --git a/apps/web/src/components/preview/PreviewView.tsx b/apps/web/src/components/preview/PreviewView.tsx index 9225e61afeaf9..33fd2fa8511b4 100644 --- a/apps/web/src/components/preview/PreviewView.tsx +++ b/apps/web/src/components/preview/PreviewView.tsx @@ -59,7 +59,7 @@ import { import { readEnvironmentScope, useEnvironmentScope } from "~/state/session"; import { useRightPanelStore } from "~/rightPanelStore"; -import { effectiveShortcutsForCommand } from "~/keybindings"; +import { effectiveShortcutsForCommand, shortcutLabelForCommand } from "~/keybindings"; import { randomUUID } from "~/lib/utils"; import { primaryServerKeybindingsAtom } from "~/state/server"; import { PreviewAnnotationSpeech } from "./PreviewAnnotationSpeech"; @@ -90,6 +90,7 @@ import { useBrowserSurfaceStore } from "~/browser/browserSurfaceStore"; import { usePreviewSession } from "./usePreviewSession"; import { ZoomIndicator } from "./ZoomIndicator"; import { AgentBrowserCursor } from "./AgentBrowserCursor"; +import { PreviewFindBar } from "./PreviewFindBar"; import { findActiveBrowserRecordingRuntimeTabId, isBrowserRecordingStartCancelledError, @@ -145,6 +146,14 @@ function ServerTabFileNotShown(props: { ); } +function focusPreviewPage(runtimeTabId: string) { + const active = document.activeElement; + if (active && active !== document.body && !active.closest("[data-preview-panel-mode]")) return; + document + .querySelector(`[data-preview-viewport="${CSS.escape(runtimeTabId)}"] webview`) + ?.focus(); +} + export function PreviewView({ threadRef, tabId: requestedTabId, @@ -153,6 +162,7 @@ export function PreviewView({ onSendAnnotation, }: Props) { const urlInputRef = useRef(null); + const [find, setFind] = useState<{ tabId: string; requestId: number } | null>(null); const [pickActive, setPickActive] = useState(false); const canSendAnnotation = useEnvironmentScope(threadRef.environmentId, AuthOrchestrationOperateScope) && @@ -186,6 +196,7 @@ export function PreviewView({ const runtimeTabId = tabId ? previewRuntimeTabId(threadRef, previewState.serverEpoch, tabId) : null; + if (find !== null && find.tabId !== runtimeTabId) setFind(null); const recordingRuntimeTabId = tabId && runtimeTabId ? activeRecordingTabIds.has(runtimeTabId) @@ -247,6 +258,7 @@ export function PreviewView({ if (runtimeTabId && previewBridge) { await previewBridge.navigate(runtimeTabId, resolvedUrl); rememberPreviewUrl(threadRef, resolvedUrl); + focusPreviewPage(runtimeTabId); return true; } const result = await openPreviewSession({ openPreview: open, threadRef, url: resolvedUrl }); @@ -342,6 +354,27 @@ export function PreviewView({ if (next !== serverZoomFactor) void adjustServerTab({ zoomFactor: next }); }; + const canFindInPage = + previewBridge !== null && + runtimeTabId !== null && + desktopOverlay !== null && + !isServerTab && + !showEmptyState && + !isUnreachable; + const handleFind = () => { + if (!canFindInPage) { + toastManager.add({ type: "info", title: "Find on page isn't available for this page" }); + return; + } + setFind((previous) => ({ tabId: runtimeTabId, requestId: (previous?.requestId ?? 0) + 1 })); + }; + + // Otherwise focus falls to the body and the next find shortcut misses the preview. + const closeFind = () => { + setFind(null); + if (runtimeTabId) focusPreviewPage(runtimeTabId); + }; + const handleZoomIn = () => { if (serverOwnsRendering) stepServerZoom(1); else if (previewBridge && runtimeTabId) void previewBridge.zoomIn(runtimeTabId); @@ -455,6 +488,7 @@ export function PreviewView({ ? { openDevTools: desktopCall(previewBridge.openDevTools), toggleNativePictureInPicture: () => handleNativePictureInPicture(), + ...(canFindInPage ? { find: handleFind } : {}), } : {}), } @@ -478,6 +512,7 @@ export function PreviewView({ .catch(() => undefined), openDevTools: desktopCall(previewBridge.openDevTools), toggleNativePictureInPicture: () => handleNativePictureInPicture(), + ...(canFindInPage ? { find: handleFind } : {}), } : null; @@ -872,17 +907,23 @@ export function PreviewView({ case "reset-zoom": handleResetZoom(); return; + case "find": + handleFind(); + return; case "toggle-panel": return; } }, ); - // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- subscribes to preview actions while the view is visible + // oxlint-disable-next-line t3code/no-unreasoned-state-sync -- subscribes to preview and desktop menu actions while the view is visible useEffect(() => { if (!visible) return; const unsubscribeAction = subscribePreviewAction((action) => onPreviewAction(action)); + // A focused page swallows its own keydowns, so the desktop forwards these + // shortcuts from the guest as menu actions instead. const unsubscribeMenu = window.desktopBridge?.onMenuAction((action) => { if (action === "preview.focusUrl") onPreviewAction("focus-url"); + if (action === "preview.find") onPreviewAction("find"); }); return () => { unsubscribeAction(); @@ -955,6 +996,9 @@ export function PreviewView({ ? { move: { label: moveLabel, onMove: () => void handleMoveTab() } } : {})} profileName={activeProfileName} + findShortcutLabel={shortcutLabelForCommand(keybindings, "preview.find", { + context: { previewFocus: true }, + })} zoomFactor={ serverOwnsRendering ? serverZoomFactor : (desktopOverlay?.zoomFactor ?? 1) } @@ -1059,6 +1103,14 @@ export function PreviewView({ controller={controller} /> ) : null} + {find && canFindInPage && find.tabId === runtimeTabId ? ( + + ) : null} {navStatus._tag === "LoadFailed" && navStatus.download ? (
; + onRefresh: () => void; +}) { + const [pending, setPending] = useState(false); + const setThreadResolution = useAtomCommand(pullRequestEnvironment.setThreadResolution, { + reportFailure: false, + }); + if (!detail.capabilities.review.resolve || !canResolvePullRequestThread(detail, thread)) { + return null; + } + + const toggle = async () => { + setPending(true); + const result = await setThreadResolution({ + environmentId, + input: { ...reference, threadId: thread.id, resolved: !thread.isResolved }, + }); + setPending(false); + if (result._tag === "Failure") { + toastManager.add({ type: "error", title: "The conversation could not be updated" }); + return; + } + onRefresh(); + }; + + return ( + + ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx index 98db15d1429c7..19c6cb75c5ac5 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.browser.test.tsx @@ -185,3 +185,49 @@ it("leaves description checkboxes read-only without the right to edit it", async true, ); }); + +it("fixes an overview thread as its opening remark, since it has no line to attach", async () => { + setup(); + const author = { login: "reviewer", name: null, avatarUrl: null }; + const note = { author, createdAt: "2026-09-01T00:00:00Z", url: null }; + const remark = { + ...note, + id: "10", + kind: "review-comment" as const, + body: "Mention the flag in the README", + path: null, + reviewState: null, + }; + const onFixFinding = vi.fn(); + const value: PullRequestDetailView = { + ...detail, + comments: [remark], + overviewThreads: [ + { + id: "overview", + path: null, + line: null, + side: "right", + isResolved: false, + isOutdated: false, + comments: [{ ...note, id: "10", body: remark.body }], + }, + ], + }; + await using screen = await renderUi( + {}} + environmentId={EnvironmentId.make("environment")} + threadRef={null} + reference={value} + detail={value} + activityPending={false} + activityError={null} + onRefresh={() => {}} + onFixFinding={onFixFinding} + />, + ); + + await userEvent.click(screen.getByRole("button", { name: "Fix in a thread" })); + expect(onFixFinding).toHaveBeenCalledWith({ kind: "comment", comment: remark }); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index 58be7cfb29485..a78cdfbd6f06f 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -6,13 +6,14 @@ import { sourceControlClients, } from "@t3tools/client-runtime/source-control-clients"; import { useAtomCommand } from "~/state/use-atom-command"; -import type { - EnvironmentId, - PullRequestComment, - PullRequestDetailView, - PullRequestRef, - PullRequestReviewThread, - ScopedThreadRef, +import { + type EnvironmentId, + type PullRequestComment, + type PullRequestDetailView, + type PullRequestRef, + type PullRequestConversationThread, + type ScopedThreadRef, + pullRequestConversationThreads, } from "@t3tools/contracts"; import { ArrowDownUpIcon, @@ -93,6 +94,7 @@ import { pullRequestCompletionScope, } from "./PullRequestMarkdownTextarea"; import { PullRequestReactionBar } from "./PullRequestReactions"; +import { PullRequestResolveThreadButton } from "./PullRequestResolveThreadButton"; import { PullRequestConversationGhost } from "./PullRequestGhosts"; import { sectionCollapseAnchorScrollTop } from "./pullRequestSummaryScroll.logic"; @@ -147,7 +149,7 @@ function CommentLocation({ thread, }: { comment: PullRequestComment; - thread: PullRequestReviewThread | undefined; + thread: PullRequestConversationThread | undefined; }) { const path = thread?.path ?? comment.path; if (!path) return null; @@ -228,9 +230,10 @@ function CommentBody({ */ function chatSubject( comment: PullRequestComment, - thread: PullRequestReviewThread | undefined, + thread: PullRequestConversationThread | undefined, ): PullRequestChatSubject | null { - if (thread !== undefined) { + // An overview thread has no line to attach, so it goes as its opening remark, as fixing it does. + if (thread !== undefined && thread.path !== null) { return thread.comments.some((entry) => visibleBody(entry.body) !== null) ? { kind: "thread", thread } : null; @@ -256,6 +259,7 @@ function CollapsedComment({ label, body, reactionBar, + resolveButton, detail, thread, replyCount = 0, @@ -268,8 +272,9 @@ function CollapsedComment({ label: string; body: string | null; reactionBar: ReactNode; + resolveButton: ReactNode; detail: PullRequestDetailView; - thread: PullRequestReviewThread | undefined; + thread: PullRequestConversationThread | undefined; replyCount?: number; replies?: ReactNode; moreReplies?: ReactNode; @@ -298,6 +303,8 @@ function CollapsedComment({ className={cn("size-3.5 transition-transform", open && "rotate-180")} /> + {/* Collapsed, the status label already says "Resolved". */} + {open ? resolveButton : null} {onAddToChat ? : null} {reactionBar}
@@ -481,7 +488,10 @@ export function PullRequestSummaryTab({ DEFAULT_COMMENT_FILTER, CommentFilterSchema, ); - const items = pullRequestConversationItems(detail.comments, detail.reviewThreads); + const items = pullRequestConversationItems( + detail.comments, + pullRequestConversationThreads(detail), + ); const filteredItems = filterPullRequestConversationItems(items, commentFilter); const commentCount = items.reduce( (count, item) => count + pullRequestConversationItemComments(item).length, @@ -696,9 +706,20 @@ export function PullRequestSummaryTab({ /> ); + const resolveButtonFor = (thread: PullRequestConversationThread | undefined) => + thread === undefined ? null : ( + + ); + const renderCommentCard = ( comment: PullRequestComment, - thread: PullRequestReviewThread | undefined, + thread: PullRequestConversationThread | undefined, { reply = false }: { reply?: boolean } = {}, ) => { const body = visibleBody(comment.body); @@ -708,7 +729,7 @@ export function PullRequestSummaryTab({ (comment.kind !== "review" && comment.kind !== "review-comment") || outcome === "approved" ? null - : thread === undefined + : thread === undefined || thread.path === null ? body === null ? null : { kind: "comment", comment } @@ -747,6 +768,7 @@ export function PullRequestSummaryTab({ ) : null} + {resolveButtonFor(thread)} {onAddToChat && chat !== null ? ( onAddToChat(chat)} /> ) : null} @@ -791,6 +813,7 @@ export function PullRequestSummaryTab({ moreReplies={moreReplies} onAddToChat={onAddToChat && chat !== null ? () => onAddToChat(chat) : undefined} reactionBar={reactionBarFor(comment)} + resolveButton={resolveButtonFor(thread)} /> ); } diff --git a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx index df0e9c260d88b..abefb7e1d7e87 100644 --- a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx @@ -56,6 +56,7 @@ import { PullRequestMarkdown } from "./PullRequestMarkdown"; import { PullRequestMoreReplies } from "./PullRequestMoreReplies"; import { PullRequestMarkdownEditor } from "./PullRequestMarkdownEditor"; import { PullRequestReactionBar } from "./PullRequestReactions"; +import { PullRequestResolveThreadButton } from "./PullRequestResolveThreadButton"; import { PullRequestActorAvatar, PullRequestDiffStat, @@ -443,10 +444,12 @@ function ReviewThreadEvent({ cwd, onOpen, reactions, + detail, }: { revealAnchor?: string | null; event: PullRequestTimelineEvent; thread: PullRequestTimelineThread; + detail: PullRequestDetailView; editable: ReadonlyMap; cwd: string; onOpen: (url: string) => void; @@ -471,40 +474,60 @@ function ReviewThreadEvent({ fallback={} muted={!expanded} /> - + {expanded ? ( + ) : null} - {dismissed ? Review dismissed : null} - {thread.isOutdated ? Outdated : null} - {expanded ? null : ( - - · {event.actor?.login ?? "ghost"} - {replyCount > 0 ? ( - <> - {" "} - · {replyCount.toLocaleString()} {replyCount === 1 ? "reply" : "replies"} - - ) : null} - - )} - - + {expanded ? ( <> >( + () => new Map(), + ); const { capableEnvironments, scopedEnvironmentId, @@ -163,11 +171,12 @@ export function PullRequestsPage() { selectedProject, queryEnvironmentIds, environmentQueries, + preferredEnvironmentQueries, assignmentKey, linkedSelection, expectedHosts, projectsKnown, - } = usePullRequestScope(search); + } = usePullRequestScope(search, hostReads); const rightPanelRef = capableEnvironments.length === 0 ? null : PULL_REQUESTS_PANEL_REF; const openPanelPullRequestUrl = useOpenPanelPullRequestUrl(rightPanelRef); const rightPanelState = useRightPanelStore((state) => @@ -272,9 +281,9 @@ export function PullRequestsPage() { localFilters, hasLocalFilters, } = usePullRequestQueryFilters(search, typedQuery); - const turnRefreshes = usePullRequestTurnRefreshes( - environmentQueries.map(({ environmentId }) => environmentId), - ); + const turnRefreshes = usePullRequestTurnRefreshes([ + ...new Set(environmentQueries.map(({ environmentId }) => environmentId)), + ]); const turnRefreshToken = turnRefreshes .map(([environmentId, revision]) => `${environmentId}:${revision}`) .join("|"); @@ -308,28 +317,30 @@ export function PullRequestsPage() { }); } - const listTargets = environmentQueries.flatMap(({ environmentId, projectIds }) => { - const cursors = sentCursors?.[environmentId]; - if (sentCursors !== null && cursors === undefined && !sentRegrown.includes(environmentId)) { - return []; - } - return [ - { - environmentId, - input: { - state: search.state, - involvement: search.involvement, - limit: pageSize, - ...(scopedProjectId ? { projectId: scopedProjectId } : {}), - ...(projectIds ? { projectIds } : {}), - ...(search.host ? { host: search.host } : {}), - ...(hasFilters ? { filters } : {}), - ...(sentParsed.text ? { query: sentParsed.text } : {}), - ...(cursors === undefined ? {} : { cursors }), - } satisfies PullRequestListInput, - }, - ]; - }); + const listTargetsFor = (queries: typeof environmentQueries) => + queries.flatMap(({ environmentId, projectIds }) => { + const cursors = sentCursors?.[environmentId]; + if (sentCursors !== null && cursors === undefined && !sentRegrown.includes(environmentId)) { + return []; + } + return [ + { + environmentId, + input: { + state: search.state, + involvement: search.involvement, + limit: pageSize, + ...(scopedProjectId ? { projectId: scopedProjectId } : {}), + ...(projectIds ? { projectIds } : {}), + ...(search.host ? { host: search.host } : {}), + ...(hasFilters ? { filters } : {}), + ...(sentParsed.text ? { query: sentParsed.text } : {}), + ...(cursors === undefined ? {} : { cursors }), + } satisfies PullRequestListInput, + }, + ]; + }); + const listTargets = listTargetsFor(environmentQueries); const listQuery = usePullRequestList(listTargets); /** @@ -337,19 +348,26 @@ export function PullRequestsPage() { * say which question it belongs to: mid-switch the text has already changed and the data has * not, and a search's answer would file itself under the workspace. */ - const baselineTargets = environmentQueries.map(({ environmentId, projectIds }) => ({ - environmentId, - input: { - state: search.state, - involvement: search.involvement, - limit: PAGE_SIZE, - ...(scopedProjectId ? { projectId: scopedProjectId } : {}), - ...(projectIds ? { projectIds } : {}), - ...(search.host ? { host: search.host } : {}), - ...(menuFiltered ? { filters: menuFilters } : {}), - } satisfies PullRequestListInput, - })); + const baselineTargetsFor = (queries: typeof environmentQueries) => + queries.map(({ environmentId, projectIds }) => ({ + environmentId, + input: { + state: search.state, + involvement: search.involvement, + limit: PAGE_SIZE, + ...(scopedProjectId ? { projectId: scopedProjectId } : {}), + ...(projectIds ? { projectIds } : {}), + ...(search.host ? { host: search.host } : {}), + ...(menuFiltered ? { filters: menuFilters } : {}), + } satisfies PullRequestListInput, + })); + const baselineTargets = baselineTargetsFor(environmentQueries); const baselineQuery = usePullRequestList(baselineTargets); + const observedHosts = rememberPullRequestHosts( + rememberPullRequestHosts(hostReads, baselineQuery.observations), + listQuery.observations, + ); + if (observedHosts !== hostReads) setHostReads(observedHosts); const baselineEmpty = baselineQuery.data?.entries.length === 0 && baselineQuery.data.errors.length === 0 && @@ -694,6 +712,8 @@ export function PullRequestsPage() { heldPullRequestsBySurface, } = pullRequestGroups({ known: ordered?.key === filterKey ? ordered.entries : (listData?.entries ?? []), + // The baseline was read without typed qualifiers, and rows cannot be checked for `checks:`. + baseline: typedParsed.filters.checks === undefined ? baselineQuery.data?.entries : undefined, providers: baselineQuery.data?.providers ?? listData?.providers ?? [], viewers, involvement: search.involvement, @@ -895,6 +915,11 @@ export function PullRequestsPage() { await Promise.all( queryEnvironmentIds.map((environmentId) => invalidate({ environmentId, input: {} })), ); + listQuery.refresh([ + ...listTargetsFor(preferredEnvironmentQueries), + ...baselineTargetsFor(preferredEnvironmentQueries), + ]); + setHostReads(resetPullRequestHosts); setInvalidating(false); refreshListAndStats(requestedStatsScope); setDetailRefreshToken((token) => token + 1); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index cceadfbee3c60..97c5919f3ef2e 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -32,6 +32,7 @@ import { type PullRequestMergeMethod, type PullRequestRef, type RepositoryIdentity, + type PullRequestConversationThread, type PullRequestReviewThread, type PullRequestState, type PullRequestUpdateMethod, @@ -487,7 +488,10 @@ export interface PullRequestReplyAvailability { export interface PullRequestTimelineThread extends - Pick, + Pick< + PullRequestConversationThread, + "id" | "path" | "line" | "isResolved" | "isOutdated" | "canResolve" + >, PullRequestReplyAvailability { readonly replies: ReadonlyArray; } @@ -557,12 +561,12 @@ export type PullRequestConversationItem = readonly createdAt: string; readonly root: PullRequestComment; readonly replies: ReadonlyArray; - readonly thread: PullRequestReviewThread; + readonly thread: PullRequestConversationThread; } & PullRequestReplyAvailability); export function pullRequestConversationItems( comments: ReadonlyArray, - reviewThreads: ReadonlyArray, + reviewThreads: ReadonlyArray, ): ReadonlyArray { const byId = new Map(comments.map((comment) => [comment.id, comment])); const claimed = new Set(); @@ -623,7 +627,7 @@ export function pullRequestConversationItemComments( export function isPullRequestCommentFinished( comment: PullRequestComment, - thread?: PullRequestReviewThread, + thread?: PullRequestConversationThread, ): boolean { return ( thread?.isResolved === true || pullRequestReviewOutcome(comment.reviewState) === "dismissed" @@ -780,7 +784,7 @@ export function buildPullRequestTimeline( PullRequestDetailView, "provider" | "createdAt" | "author" | "commits" | "comments" | "mergedAt" | "closedAt" > & - Partial>, + Partial>, ): ReadonlyArray { const term = sourceControlClients.get(detail.provider).changeRequest.singular; return [ @@ -816,11 +820,16 @@ export function buildPullRequestTimeline( reviewState: null, reactions: [], })), - ...pullRequestConversationItems(detail.comments, detail.reviewThreads ?? []).map((item) => { + ...pullRequestConversationItems(detail.comments, [ + ...(detail.reviewThreads ?? []), + ...(detail.overviewThreads ?? []), + ]).map((item) => { if (item.kind === "comment") return commentTimelineEvent(item.comment); return { ...commentTimelineEvent(item.root), thread: { + id: item.thread.id, + ...(item.thread.canResolve === undefined ? {} : { canResolve: item.thread.canResolve }), path: item.thread.path, line: item.thread.line, isResolved: item.thread.isResolved, diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts index 123111ff6a352..6a38bddfc8d68 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts @@ -38,6 +38,7 @@ import { reusePullRequestEntries, settlePullRequestOverrides, } from "./pullRequestList.logic"; +import { pullRequestGroups } from "./pullRequestListGroups.logic"; import { readPullRequestListPreferences, writePullRequestListPreferences, @@ -177,6 +178,43 @@ describe("ranking what a search found", () => { rankPullRequestMatches([mentioned, titled], "welcome wizard").map((entry) => entry.number), ).toEqual([1, 2]); }); + + it("keeps loaded partial-word matches the host's whole-word search missed, after its answer", () => { + const bilal = { login: "bilal", name: null, avatarUrl: null }; + const hosted = row({ number: 1, title: "Notifications match the body text", author: bilal }); + const toast = row({ number: 2, title: "Threads show a completed toast", author: bilal }); + const toaster = row({ number: 3, title: "Toaster fixtures cover retries", author: bilal }); + const theirs = row({ number: 4, title: "Toasted sidebar" }); + const unrelated = row({ number: 5, title: "Cursor no longer adds attribution", author: bilal }); + + const { entries } = pullRequestGroups({ + known: [hosted, toast], + baseline: [theirs, unrelated, toaster, toast], + providers: [ + { + host: "github.com", + kind: SourceControlProviderKind.make("github"), + searchesOnHost: true, + projectCount: 1, + configured: true, + detail: null, + }, + ], + viewers: VIEWERS, + involvement: "all", + localFilters: { author: "bilal" }, + hasLocalFilters: true, + text: "toa", + answeredLocally: true, + baselineEmpty: false, + held: undefined, + authored: undefined, + reviewing: undefined, + partitionsWanted: false, + }); + + expect(entries.map((entry) => entry.number)).toEqual([1, 2, 3]); + }); }); describe("default merge-readiness ranking", () => { @@ -433,6 +471,51 @@ describe("merging the environments' own listings", () => { ]); }); + it.each([false, true])( + "merges worktree batches without duplicate rows (reversed: %s)", + (reverse) => { + const older = entry({ + projectId: "worktree-1" as ProjectId, + number: 4, + updatedAt: "2026-07-01T00:00:00Z", + }); + const newer = entry({ + projectId: "worktree-101" as ProjectId, + number: 4, + updatedAt: "2026-07-02T00:00:00Z", + }); + const rows = reverse ? [newer, older] : [older, newer]; + const merged = mergePullRequestLists([ + ...rows.map((row) => [ENV_1, answer({ entries: [row] })] as const), + [ENV_2, answer({ entries: [older] })], + ]); + expect(merged?.entries).toEqual([ + { ...newer, environmentId: ENV_1 }, + { ...older, environmentId: ENV_2 }, + ]); + }, + ); + + it("keeps Azure pull requests from distinct projects with the same repository name and number", () => { + const first = entry({ + provider: SourceControlProviderKind.make("azure-devops"), + host: "dev.azure.com", + repository: "web", + number: 4, + projectId: "azure-first" as ProjectId, + url: "https://dev.azure.com/acme/first/_git/web/pullrequest/4", + }); + const second = { + ...first, + projectId: "azure-second" as ProjectId, + url: "https://dev.azure.com/acme/second/_git/web/pullrequest/4", + }; + const merged = mergePullRequestLists([[ENV_1, answer({ entries: [first, second] })]]); + expect(merged?.entries).toEqual( + [first, second].map((entry) => ({ ...entry, environmentId: ENV_1 })), + ); + }); + // regression: #6490 one environment's error showed on every environment it("keeps project errors scoped to the environment that reported them", () => { const error = { @@ -478,6 +561,17 @@ describe("merging the environments' own listings", () => { }, ]); }); + + it("keeps continuations from every project batch on the same environment", () => { + const merged = mergePullRequestLists([ + [ENV_1, answer({ truncated: true, nextCursors: { "github.com acme/first": "cursor-1" } })], + [ENV_1, answer({ truncated: true, nextCursors: { "github.com acme/second": "cursor-2" } })], + ]); + expect(merged?.nextCursors).toEqual({ + [ENV_1]: { "github.com acme/first": "cursor-1", "github.com acme/second": "cursor-2" }, + }); + expect(merged?.truncatedEnvironments).toEqual([ENV_1]); + }); }); describe('who "I" am, per server', () => { diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.ts index fb95b44b81e2e..52d5cc12a06a4 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.ts @@ -536,9 +536,9 @@ export function mergePullRequestLists( ): MergedPullRequestList | null { if (answers.length === 0) return null; const viewers: Record = {}; - const truncatedEnvironments: string[] = []; + const truncatedEnvironments = new Set(); const providers = new Map(); - const entries: EnvironmentPullRequestEntry[] = []; + const entries = new Map(); const errors: EnvironmentPullRequestError[] = []; const nextCursors: Record = {}; let truncated = false; @@ -560,22 +560,28 @@ export function mergePullRequestLists( }, ); } - entries.push(...answer.entries.map((entry) => ({ ...entry, environmentId }))); + for (const entry of answer.entries) { + const key = JSON.stringify([environmentId, entry.url]); + const held = entries.get(key); + if (held === undefined || entry.updatedAt > held.updatedAt) { + entries.set(key, { ...entry, environmentId }); + } + } errors.push(...answer.errors.map((error) => ({ ...error, environmentId }))); truncated ||= answer.truncated; - if (answer.truncated) truncatedEnvironments.push(environmentId); + if (answer.truncated) truncatedEnvironments.add(environmentId); if (Object.keys(answer.nextCursors).length > 0) { - nextCursors[environmentId] = answer.nextCursors; + nextCursors[environmentId] = { ...nextCursors[environmentId], ...answer.nextCursors }; } } return { viewers, providers: [...providers.values()], - entries: entries.toSorted(comparePullRequestsByRecency), + entries: [...entries.values()].toSorted(comparePullRequestsByRecency), errors, truncated, nextCursors, - truncatedEnvironments, + truncatedEnvironments: [...truncatedEnvironments], }; } diff --git a/apps/web/src/components/pullRequest/pullRequestListGroups.logic.ts b/apps/web/src/components/pullRequest/pullRequestListGroups.logic.ts index 9a3bda738a287..31f02978c918d 100644 --- a/apps/web/src/components/pullRequest/pullRequestListGroups.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestListGroups.logic.ts @@ -15,6 +15,7 @@ import { partitionPullRequestsWithPriority, pullRequestEntryKey, pullRequestEntryViewer, + rankPullRequestMatches, type EnvironmentPullRequestEntry, type PullRequestPartitionsSnapshot, } from "./pullRequestList.logic"; @@ -25,6 +26,7 @@ export const pullRequestListEntryId = (target: Parameters; + /** The text-free rows already loaded, or undefined when the typed filters cannot vet them. */ + baseline: ReadonlyArray | undefined; providers: PullRequestListResult["providers"]; viewers: PullRequestListResult["viewers"]; involvement: PullRequestInvolvement; @@ -56,22 +60,33 @@ export function pullRequestGroups({ providers.flatMap((provider) => (provider.searchesOnHost ? [provider.host] : [])), ); - const entries = (() => { - const involvementEntries = filterPullRequestsByInvolvement(known, viewers, involvement); - // Host text searches cover fields absent from rows, so do not narrow their matches locally. - // Filter text locally while results are pending or when the host has no text search. - const narrowedEntries = hasLocalFilters + const narrowRows = (rows: ReadonlyArray) => { + const involvementEntries = filterPullRequestsByInvolvement(rows, viewers, involvement); + return hasLocalFilters ? involvementEntries.filter((entry) => matchesPullRequestFilters(entry, localFilters, pullRequestEntryViewer(entry, viewers)), ) : involvementEntries; + }; + + const entries = (() => { + const narrowedEntries = narrowRows(known); if (text.length === 0) { return narrowedEntries.toSorted(comparePullRequestsByRecency); } - return narrowedEntries.filter( + // Host text searches cover fields absent from rows, so do not narrow their matches locally. + // Filter text locally while results are pending or when the host has no text search. + const matched = narrowedEntries.filter( (entry) => (answeredLocally && searchingHosts.has(entry.host)) || matchesPullRequestQuery(entry, text), ); + // Host search matches whole words, so it can miss loaded rows a partial word matches here. + // Those follow the host's ranked answer rather than reshuffling it. + const listed = new Set(matched.map(pullRequestEntryKey)); + const missed = narrowRows(baseline ?? []).filter( + (entry) => !listed.has(pullRequestEntryKey(entry)) && matchesPullRequestQuery(entry, text), + ); + return missed.length === 0 ? matched : [...matched, ...rankPullRequestMatches(missed, text)]; })(); const listedPullRequestTabStatuses: Record = Object.fromEntries( diff --git a/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.test.ts index 5af97fea67c28..508647f3c046c 100644 --- a/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.test.ts @@ -1,8 +1,18 @@ -import type { EnvironmentId, ProjectId } from "@t3tools/contracts"; +import { + PullRequestListInput, + SourceControlProviderKind, + type EnvironmentId, + type ProjectId, + type PullRequestProviderSummary, +} from "@t3tools/contracts"; +import * as Schema from "effect/Schema"; import { describe, expect, it } from "vite-plus/test"; import { assignProjectsToEnvironments, + rememberPullRequestHosts, + resetPullRequestHosts, + pullRequestProjectQueries, resolvePickableEnvironments, type AssignableProject, } from "./pullRequestProjectAssignment.logic"; @@ -22,6 +32,134 @@ const envs = (...ids: ReadonlyArray) => ids as ReadonlyArray) => Object.fromEntries([...assignment].map(([id, projectIds]) => [id, projectIds])); +const decodeListInput = Schema.decodeSync(PullRequestListInput); + +const hostAvailability = (hosts: ReadonlyMap) => ({ + hosts: new Map([...hosts].map(([host, configured]) => [host, { configured, observedAt: 0 }])), + projectHosts: new Map(), +}); + +const provider = (host: string, configured: boolean): PullRequestProviderSummary => ({ + host, + kind: "bitbucket" as PullRequestProviderSummary["kind"], + searchesOnHost: false, + projectCount: 1, + configured, + detail: configured ? null : "Credentials rejected", +}); + +describe("host read results", () => { + const environmentId = "env-1" as EnvironmentId; + + it("remembers a failed host after the assignment stops requesting it", () => { + const failed = rememberPullRequestHosts(new Map(), [ + [environmentId, { providers: [provider("BITBUCKET.org", false)] }, 100], + ]); + const next = rememberPullRequestHosts(failed, [ + [environmentId, { providers: [provider("github.com", true)] }, 200], + ]); + expect( + [...next.get(environmentId)!.hosts].map(([host, read]) => [host, read.configured]), + ).toEqual([ + ["bitbucket.org", false], + ["github.com", true], + ]); + expect( + [...failed.get(environmentId)!.hosts].map(([host, read]) => [host, read.configured]), + ).toEqual([["bitbucket.org", false]]); + }); + + it("does not replay cached failures after refresh clears availability", () => { + const cleared = new Map([ + [ + environmentId, + { observedAt: 100, resetAt: 100, ...hostAvailability(new Map()) }, + ], + ]); + expect( + rememberPullRequestHosts(cleared, [ + [environmentId, { providers: [provider("bitbucket.org", false)] }, 100], + ]), + ).toBe(cleared); + const refreshed = rememberPullRequestHosts(cleared, [ + [environmentId, { providers: [provider("bitbucket.org", true)] }, 200], + ]); + expect(refreshed.get(environmentId)!.hosts.get("bitbucket.org")?.configured).toBe(true); + }); + + it("ignores an older answer when a newer read has already succeeded", () => { + const readable = rememberPullRequestHosts(new Map(), [ + [environmentId, { providers: [provider("bitbucket.org", true)] }, 200], + ]); + expect( + rememberPullRequestHosts(readable, [ + [environmentId, { providers: [provider("bitbucket.org", false)] }, 100], + ]), + ).toBe(readable); + }); + + it.each([false, true])( + "treats a host as readable when any active batch succeeds (reversed: %s)", + (reverse) => { + const observations = [ + [environmentId, { providers: [provider("gitlab.example", true)] }, 100], + [environmentId, { providers: [provider("gitlab.example", false)] }, 200], + ] as const; + const next = rememberPullRequestHosts( + new Map(), + reverse ? observations.toReversed() : observations, + ); + expect(next.get(environmentId)!.hosts.get("gitlab.example")).toEqual({ + configured: true, + observedAt: 200, + }); + expect(next.get(environmentId)!.observedAt).toBe(200); + const cleared = resetPullRequestHosts(next); + expect(rememberPullRequestHosts(cleared, observations)).toBe(cleared); + const failed = rememberPullRequestHosts(next, [ + [environmentId, { providers: [provider("gitlab.example", false)] }, 300], + ]); + expect(failed.get(environmentId)!.hosts.get("gitlab.example")?.configured).toBe(false); + }, + ); +}); + +describe("assignment query limits", () => { + it("splits the healthy remainder after fallback into valid project filters", () => { + const healthy = Array.from({ length: 101 }, (_, index) => + project(`healthy-${index}`, "env-1", `github.com/acme/repo-${index}`), + ); + const projects = [ + project("shared", "env-1", "bitbucket.org/acme/app"), + ...healthy, + project("copy", "env-2", "bitbucket.org/acme/app"), + ]; + const queries = pullRequestProjectQueries( + projects, + envs("env-1", "env-2"), + new Map([["env-1" as EnvironmentId, hostAvailability(new Map([["bitbucket.org", false]]))]]), + ); + expect( + queries.map(({ environmentId, projectIds }) => [environmentId, projectIds?.length]), + ).toEqual([ + ["env-1", 100], + ["env-1", 1], + ["env-2", undefined], + ]); + expect( + queries + .filter(({ environmentId }) => environmentId === "env-1") + .flatMap(({ projectIds }) => projectIds ?? []), + ).toEqual(healthy.map(({ id }) => id)); + for (const { projectIds } of queries) + expect(() => + decodeListInput({ + state: "open", + ...(projectIds === undefined ? {} : { projectIds }), + }), + ).not.toThrow(); + }); +}); describe("one server per repository", () => { it("lets the first server list a repository both hold", () => { @@ -50,6 +188,138 @@ describe("one server per repository", () => { expect(plain(assignment)).toEqual({ "env-2": ["a2"] }); }); + it("falls back from an unreadable host without moving other repositories", () => { + const assignment = assignProjectsToEnvironments( + [ + project("a1", "env-1", "bitbucket.org/acme/app"), + project("b1", "env-1", "github.com/acme/tools"), + project("a2", "env-2", "bitbucket.org/acme/app"), + project("b2", "env-2", "github.com/acme/tools"), + ], + envs("env-1", "env-2"), + "env-1" as EnvironmentId, + new Map([["env-1" as EnvironmentId, hostAvailability(new Map([["bitbucket.org", false]]))]]), + ); + expect(plain(assignment)).toEqual({ "env-1": ["b1"], "env-2": ["a2"] }); + }); + + it("keeps one stable owner and its error when every copy is unreadable", () => { + const projects = [ + project("a1", "env-1", "bitbucket.org/acme/app"), + project("a2", "env-2", "bitbucket.org/acme/app"), + project("only1", "env-1", "bitbucket.org/acme/only"), + ]; + const availability = new Map( + envs("env-1", "env-2").map((id) => [ + id, + hostAvailability(new Map([["bitbucket.org", false]])), + ]), + ); + expect( + plain( + assignProjectsToEnvironments( + projects, + envs("env-1", "env-2"), + "env-1" as EnvironmentId, + availability, + ), + ), + ).toEqual({ "env-1": ["a1", "only1"] }); + expect( + plain( + assignProjectsToEnvironments( + projects, + envs("env-2"), + "env-2" as EnvironmentId, + availability, + ), + ), + ).toEqual({ "env-2": ["a2"] }); + }); + + it("checks the owner's HTTP authority using its own identity in either project order", () => { + const projects = [ + { + ...project("a1", "env-1", "forge.example/team/repo"), + repositoryIdentity: { + canonicalKey: "forge.example/team/repo", + provider: SourceControlProviderKind.make("forgejo"), + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "https://forge.example:3000/team/repo.git", + }, + }, + }, + { + ...project("a2", "env-2", "forge.example/team/repo"), + repositoryIdentity: { + canonicalKey: "forge.example/team/repo", + provider: SourceControlProviderKind.make("forgejo"), + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "git@forge.example:team/repo.git", + }, + }, + }, + ] as const; + const availability = new Map([ + ["env-1" as EnvironmentId, hostAvailability(new Map([["forge.example:3000", false]]))], + ]); + for (const copies of [projects, projects.toReversed()]) { + expect( + plain( + assignProjectsToEnvironments( + copies, + envs("env-1", "env-2"), + "env-1" as EnvironmentId, + availability, + ), + ), + ).toEqual({ "env-2": ["a2"] }); + } + }); + + it("uses a server-resolved host for SSH without conflating ports on that host", () => { + const projects = envs("env-1", "env-2").map((environmentId) => ({ + ...project(`a-${environmentId}`, environmentId, "ssh.forge.example/team/repo"), + repositoryIdentity: { + canonicalKey: "ssh.forge.example/team/repo", + provider: SourceControlProviderKind.make("forgejo"), + locator: { + source: "git-remote" as const, + remoteName: "origin", + remoteUrl: "ssh://git@ssh.forge.example:2222/team/repo.git", + }, + }, + })); + const availability = rememberPullRequestHosts(new Map(), [ + [ + "env-1" as EnvironmentId, + { + providers: [ + { ...provider("forge.example:3000", false), projectIds: ["a-env-1" as ProjectId] }, + { ...provider("forge.example:4000", true), projectIds: ["unrelated" as ProjectId] }, + ], + }, + 100, + ], + ]); + for (const copies of [projects, projects.toReversed()]) { + expect( + plain( + assignProjectsToEnvironments( + copies, + envs("env-1", "env-2"), + "env-1" as EnvironmentId, + availability, + ), + ), + ).toEqual({ "env-2": ["a-env-2"] }); + } + }); + it("keeps every copy of a project that has no identity to compare", () => { const assignment = assignProjectsToEnvironments( [project("p1", "env-1"), project("p2", "env-2")], diff --git a/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.ts b/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.ts index c231554ee6dca..06fa71cf32c6d 100644 --- a/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestProjectAssignment.logic.ts @@ -1,9 +1,118 @@ -import type { EnvironmentId, EnvironmentMachineKind, ProjectId } from "@t3tools/contracts"; +import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients"; +import { + PULL_REQUEST_LIST_PROJECT_LIMIT, + type EnvironmentId, + type EnvironmentMachineKind, + type ProjectId, + type PullRequestProviderSummary, + type RepositoryIdentity, + type SourceControlProviderKind, +} from "@t3tools/contracts"; export interface AssignableProject { readonly id: ProjectId; readonly environmentId: EnvironmentId; - readonly repositoryIdentity?: { readonly canonicalKey?: string | undefined } | null | undefined; + readonly repositoryIdentity?: + | Partial> + | null + | undefined; +} + +export type PullRequestHostAvailability = ReadonlyMap< + EnvironmentId, + Pick +>; + +export type PullRequestHostRead = { + readonly observedAt: number; + readonly resetAt?: number; + readonly hosts: ReadonlyMap< + string, + { readonly configured: boolean; readonly observedAt: number } + >; + readonly projectHosts: ReadonlyMap< + ProjectId, + { readonly host: string; readonly observedAt: number } + >; +}; + +export function resetPullRequestHosts( + previous: ReadonlyMap, +): ReadonlyMap { + return new Map( + [...previous].map(([id, read]) => [ + id, + { ...read, resetAt: read.observedAt, hosts: new Map(), projectHosts: new Map() }, + ]), + ); +} + +export function rememberPullRequestHosts( + previous: ReadonlyMap, + observations: ReadonlyArray< + readonly [ + EnvironmentId, + { readonly providers: ReadonlyArray }, + number, + ] + >, +): ReadonlyMap { + const activeHosts = new Map< + EnvironmentId, + Map + >(); + for (const [environmentId, { providers }, observedAt] of observations) { + if (observedAt <= (previous.get(environmentId)?.resetAt ?? -Infinity)) continue; + let hosts = activeHosts.get(environmentId); + if (hosts === undefined) { + hosts = new Map(); + activeHosts.set(environmentId, hosts); + } + for (const provider of providers) { + const host = provider.host.toLowerCase(); + const held = hosts.get(host); + hosts.set(host, { + configured: provider.configured || held?.configured === true, + observedAt: Math.max(observedAt, held?.observedAt ?? -Infinity), + }); + } + } + let next = previous; + for (const [environmentId, { providers }, observedAt] of observations) { + const before = next.get(environmentId); + if (observedAt <= (before?.resetAt ?? -Infinity)) continue; + const hosts = new Map(before?.hosts); + const projectHosts = new Map(before?.projectHosts); + let changed = false; + let latestObservedAt = observedAt; + for (const provider of providers) { + const host = provider.host.toLowerCase(); + const active = activeHosts.get(environmentId)!.get(host)!; + const held = hosts.get(host); + if ( + active.observedAt > (held?.observedAt ?? -Infinity) || + (active.observedAt === held?.observedAt && active.configured && !held.configured) + ) { + hosts.set(host, active); + latestObservedAt = Math.max(latestObservedAt, active.observedAt); + changed = true; + } + for (const projectId of provider.projectIds ?? []) { + if (observedAt > (projectHosts.get(projectId)?.observedAt ?? -Infinity)) { + projectHosts.set(projectId, { host, observedAt }); + changed = true; + } + } + } + if (!changed) continue; + next = new Map(next).set(environmentId, { + ...before, + observedAt: Math.max(before?.observedAt ?? -Infinity, latestObservedAt), + hosts, + projectHosts, + }); + } + return next; } function repositoryKey(project: AssignableProject): string | undefined { @@ -14,9 +123,20 @@ export function assignProjectsToEnvironments( projects: ReadonlyArray, environmentIds: ReadonlyArray, preferredEnvironmentId?: EnvironmentId | null, + hostAvailability?: PullRequestHostAvailability, ): Map { const rank = new Map(environmentIds.map((id, index) => [id, index] as const)); - const owner = new Map(); + const owner = new Map(); + const unreadable = (project: AssignableProject) => { + const availability = hostAvailability?.get(project.environmentId); + const host = + availability?.projectHosts.get(project.id)?.host ?? + sourceControlClients.changeRequestHost( + project.repositoryIdentity, + (project.repositoryIdentity?.provider ?? "unknown") as SourceControlProviderKind, + ); + return availability?.hosts.get(host)?.configured === false; + }; for (const project of projects) { const key = repositoryKey(project); if (!key) continue; @@ -24,22 +144,28 @@ export function assignProjectsToEnvironments( if (environmentRank === undefined) continue; const current = owner.get(key); if (current === undefined) { - owner.set(key, project.environmentId); + owner.set(key, project); + continue; + } + const currentUnreadable = unreadable(current); + const candidateUnreadable = unreadable(project); + if (currentUnreadable !== candidateUnreadable) { + if (currentUnreadable) owner.set(key, project); continue; } - if (current === preferredEnvironmentId) continue; + if (current.environmentId === preferredEnvironmentId) continue; if ( project.environmentId === preferredEnvironmentId || - environmentRank < (rank.get(current) ?? Number.MAX_SAFE_INTEGER) + environmentRank < (rank.get(current.environmentId) ?? Number.MAX_SAFE_INTEGER) ) { - owner.set(key, project.environmentId); + owner.set(key, project); } } const assignment = new Map(); for (const project of projects) { if (!rank.has(project.environmentId)) continue; const key = repositoryKey(project); - if (key && owner.get(key) !== project.environmentId) continue; + if (key && owner.get(key)?.environmentId !== project.environmentId) continue; const listed = assignment.get(project.environmentId); if (listed === undefined) assignment.set(project.environmentId, [project.id]); else listed.push(project.id); @@ -47,6 +173,39 @@ export function assignProjectsToEnvironments( return assignment; } +export function pullRequestProjectQueries( + projects: ReadonlyArray, + environmentIds: ReadonlyArray, + hostAvailability?: PullRequestHostAvailability, +): ReadonlyArray<{ + readonly environmentId: EnvironmentId; + readonly projectIds?: ReadonlyArray; +}> { + const assignment = assignProjectsToEnvironments( + projects, + environmentIds, + environmentIds[0], + hostAvailability, + ); + const totals = new Map(); + for (const project of projects) + totals.set(project.environmentId, (totals.get(project.environmentId) ?? 0) + 1); + return environmentIds.flatMap((environmentId) => { + const projectIds = assignment.get(environmentId); + if (projectIds === undefined) return []; + if (projectIds.length === totals.get(environmentId)) return [{ environmentId }]; + const queries: Array<{ environmentId: EnvironmentId; projectIds: ReadonlyArray }> = + []; + for (let offset = 0; offset < projectIds.length; offset += PULL_REQUEST_LIST_PROJECT_LIMIT) { + queries.push({ + environmentId, + projectIds: projectIds.slice(offset, offset + PULL_REQUEST_LIST_PROJECT_LIMIT), + }); + } + return queries; + }); +} + export interface PickableEnvironment { readonly environmentId: EnvironmentId; readonly projectId: ProjectId; diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.ts b/apps/web/src/components/pullRequest/usePullRequestDetail.ts index 7d6ab0c7a6154..fb7515c646009 100644 --- a/apps/web/src/components/pullRequest/usePullRequestDetail.ts +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.ts @@ -182,6 +182,9 @@ export function usePullRequestDetail( ), commentsTruncated: activity?.commentsTruncated ?? false, reviewThreads: activity?.reviewThreads ?? [], + ...(activity?.overviewThreads === undefined + ? {} + : { overviewThreads: activity.overviewThreads }), commits: activity?.commits ?? [], reactions: activity?.reactions ?? [], }; diff --git a/apps/web/src/components/pullRequest/usePullRequestListView.ts b/apps/web/src/components/pullRequest/usePullRequestListView.ts index 2fad034b2362e..36973019e2063 100644 --- a/apps/web/src/components/pullRequest/usePullRequestListView.ts +++ b/apps/web/src/components/pullRequest/usePullRequestListView.ts @@ -35,7 +35,10 @@ import { } from "./pullRequestList.logic"; import type { PullRequestExpectedHost } from "./PullRequestListFilters"; import type { PullRequestListPreferences, PullRequestListSort } from "./pullRequestListPreferences"; -import { assignProjectsToEnvironments } from "./pullRequestProjectAssignment.logic"; +import { + pullRequestProjectQueries, + type PullRequestHostAvailability, +} from "./pullRequestProjectAssignment.logic"; import { pullRequestFilterProjects } from "@t3tools/client-runtime/pull-request-project-filter"; import { pullRequestListEntryId } from "./pullRequestListGroups.logic"; @@ -54,7 +57,10 @@ export interface PullRequestsSearch extends PullRequestListPreferences { const SEARCH_DEBOUNCE_MS = 250; /** Which servers and projects the page reads, from the URL and what this client knows. */ -export function usePullRequestScope(search: PullRequestsSearch) { +export function usePullRequestScope( + search: PullRequestsSearch, + hostAvailability: PullRequestHostAvailability, +) { const { environments } = useEnvironments(); const allProjects = useProjects(); const projectsKnown = useAllEnvironmentShellsBootstrapped(); @@ -115,28 +121,19 @@ export function usePullRequestScope(search: PullRequestsSearch) { scopedProjectId, projectsKnown, ); - const environmentQueries = ((): ReadonlyArray<{ + const queriesFor = ( + availability?: PullRequestHostAvailability, + ): ReadonlyArray<{ readonly environmentId: EnvironmentId; readonly projectIds?: ReadonlyArray; }> => { const plain = queryEnvironmentIds.map((environmentId) => ({ environmentId })); if (!projectsKnown || scopedProjectId !== undefined) return plain; - const assignment = assignProjectsToEnvironments( - projects, - queryEnvironmentIds, - queryEnvironmentIds[0], - ); - const totals = new Map(); - for (const project of projects) { - totals.set(project.environmentId, (totals.get(project.environmentId) ?? 0) + 1); - } - return queryEnvironmentIds.flatMap((environmentId) => { - const projectIds = assignment.get(environmentId); - if (projectIds === undefined) return []; - if (projectIds.length === (totals.get(environmentId) ?? 0)) return [{ environmentId }]; - return [{ environmentId, projectIds }]; - }); - })(); + return pullRequestProjectQueries(projects, queryEnvironmentIds, availability); + }; + const environmentQueries = queriesFor(hostAvailability); + // What the page reads once a refresh forgets host reads; refresh must refetch these too. + const preferredEnvironmentQueries = queriesFor(); const assignmentKey = environmentQueries .map(({ environmentId, projectIds }) => `${environmentId}#${projectIds?.join("+") ?? "*"}`) .join("|"); @@ -186,6 +183,7 @@ export function usePullRequestScope(search: PullRequestsSearch) { selectedProject, queryEnvironmentIds, environmentQueries, + preferredEnvironmentQueries, assignmentKey, linkedSelection, expectedHosts, diff --git a/apps/web/src/components/settings/AcpSessionManagementSection.tsx b/apps/web/src/components/settings/AcpSessionManagementSection.tsx index d187633aa3172..c7085f5b4b44a 100644 --- a/apps/web/src/components/settings/AcpSessionManagementSection.tsx +++ b/apps/web/src/components/settings/AcpSessionManagementSection.tsx @@ -34,6 +34,10 @@ interface AcpSessionProject { readonly workspaceRoot: string; } +function needsImport(session: AcpRegistrySession) { + return session.importedThreadId === null || session.nativeHistoryPending === true; +} + function reportFailure(title: string, result: AtomCommandResult) { if (result._tag !== "Failure" || isAtomCommandInterrupted(result)) return; const error = squashAtomCommandFailure(result); @@ -223,7 +227,7 @@ function InstanceSessionManagement( setSessions((current) => current.map((candidate) => candidate.sessionId === session.sessionId - ? { ...candidate, importedThreadId: result.value.threadId } + ? { ...candidate, importedThreadId: result.value.threadId, nativeHistoryPending: false } : candidate, ), ); @@ -472,12 +476,12 @@ function InstanceSessionManagement( disabled={ !canOperate || !canImport || - session.importedThreadId !== null || + !needsImport(session) || importingSessionId !== null } onClick={() => void importNativeSession(session)} > - {session.importedThreadId !== null + {!needsImport(session) ? "Imported" : importingSessionId === session.sessionId ? "Importing" diff --git a/apps/web/src/components/settings/BrowserImportWizard.tsx b/apps/web/src/components/settings/BrowserImportWizard.tsx index b12696ce89e43..7fc81b7449253 100644 --- a/apps/web/src/components/settings/BrowserImportWizard.tsx +++ b/apps/web/src/components/settings/BrowserImportWizard.tsx @@ -12,6 +12,7 @@ import { Dialog, DialogClose, DialogDescription, + DialogAction, DialogFooter, DialogHeader, DialogPanel, @@ -201,7 +202,7 @@ function QuitStep({ - + I’ve quit it ); @@ -575,7 +576,7 @@ function BlockedStep({ - {onRetry ? : null} + {onRetry ? Try again : null} ); diff --git a/apps/web/src/components/settings/KeybindingsSettings.logic.ts b/apps/web/src/components/settings/KeybindingsSettings.logic.ts index d185901e845aa..1946922f59dc4 100644 --- a/apps/web/src/components/settings/KeybindingsSettings.logic.ts +++ b/apps/web/src/components/settings/KeybindingsSettings.logic.ts @@ -384,6 +384,7 @@ export function commandLabel(command: KeybindingCommand): string { if (usagePeriod) return `Usage: Period: ${usagePeriod.label}`; if (command === "view.reopenClosed") return "Reopen Closed Tab"; if (command === "preview.focusUrl") return "Preview: Focus URL"; + if (command === "preview.find") return "Preview: Find on Page"; const raw = String(command); if (raw.startsWith("script.") && raw.endsWith(".run")) { return `Run Script: ${titleCaseCommandSegment(raw.slice("script.".length, -".run".length))}`; diff --git a/apps/web/src/components/settings/ProjectDefaultsSettings.logic.test.ts b/apps/web/src/components/settings/ProjectDefaultsSettings.logic.test.ts new file mode 100644 index 0000000000000..a14288ce90b14 --- /dev/null +++ b/apps/web/src/components/settings/ProjectDefaultsSettings.logic.test.ts @@ -0,0 +1,112 @@ +import { ProviderDriverKind, ProviderInstanceId, type ServerProvider } from "@t3tools/contracts"; +import { DEFAULT_UNIFIED_SETTINGS, type UnifiedSettings } from "@t3tools/contracts/settings"; +import { createModelSelection } from "@t3tools/shared/model"; +import { describe, expect, it } from "vite-plus/test"; + +import { providerInstanceEntriesFor } from "../../providerInstances"; +import { resolveDefaultModelSettingsPresentation } from "./ProjectDefaultsSettings.logic"; + +const claudeId = ProviderInstanceId.make("claudeAgent"); +const high = [{ id: "effort", value: "high" }]; +const savedClaude = createModelSelection(claudeId, "claude-opus-4-8", high); +function provider(input: { + instanceId: string; + driver: string; + enabled?: boolean; + availability?: ServerProvider["availability"]; + models: ReadonlyArray; +}): ServerProvider { + return { + instanceId: ProviderInstanceId.make(input.instanceId), + driver: ProviderDriverKind.make(input.driver), + enabled: input.enabled ?? true, + installed: true, + version: null, + status: "ready", + ...(input.availability ? { availability: input.availability } : {}), + auth: { status: "authenticated" }, + checkedAt: "2026-01-01T00:00:00.000Z", + models: input.models.map((slug, index) => ({ + slug, + name: slug, + isCustom: false, + ...(index === 0 ? { isDefault: true } : {}), + capabilities: {}, + })), + slashCommands: [], + skills: [], + }; +} + +const claudeAndCodex = [ + provider({ instanceId: "codex", driver: "codex", models: ["gpt-5.6"] }), + provider({ instanceId: "claudeAgent", driver: "claudeAgent", models: ["claude-opus-4-8"] }), +]; + +function present( + providers: ReadonlyArray, + overrides: Partial = {}, +) { + const settings = { + ...DEFAULT_UNIFIED_SETTINGS, + defaultModelSelection: savedClaude, + ...overrides, + }; + return resolveDefaultModelSettingsPresentation({ + entries: providerInstanceEntriesFor(providers, settings, null), + providers, + settings, + projectId: null, + }); +} + +const claudeOff: Partial = { + providerInstances: { + [claudeId]: { driver: ProviderDriverKind.make("claudeAgent"), enabled: false }, + }, +}; + +describe("resolveDefaultModelSettingsPresentation", () => { + it("keeps the saved model and explains why when its provider cannot run", () => { + const withoutClaude = claudeAndCodex.filter((entry) => entry.instanceId !== claudeId); + const claudeUnavailable = claudeAndCodex.map((entry) => + entry.instanceId === claudeId ? { ...entry, availability: "unavailable" as const } : entry, + ); + // Settings turn Claude off before the server snapshot reflects it. + expect(present(claudeAndCodex, claudeOff)).toEqual({ + selection: savedClaude, + providerNote: "Claude is turned off.", + }); + expect(present(claudeUnavailable)).toEqual({ + selection: savedClaude, + providerNote: "Claude is unavailable.", + }); + expect(present(withoutClaude)).toEqual({ + selection: savedClaude, + providerNote: "This provider is not available on this environment.", + }); + }); + + it("leaves effort editable when the saved provider can run", () => { + expect(present(claudeAndCodex)).toEqual({ selection: savedClaude, providerNote: null }); + }); + + it("still follows a hidden model on the same provider", () => { + const row = present( + [ + provider({ + instanceId: "claudeAgent", + driver: "claudeAgent", + models: ["claude-sonnet-5", "claude-opus-4-8"], + }), + ], + { + providerModelPreferences: { + [claudeId]: { hiddenModels: ["claude-opus-4-8"], modelOrder: [] }, + }, + }, + ); + expect(row.providerNote).toBeNull(); + expect(row.selection).toMatchObject({ instanceId: claudeId, model: "claude-sonnet-5" }); + }); +}); diff --git a/apps/web/src/components/settings/ProjectDefaultsSettings.logic.ts b/apps/web/src/components/settings/ProjectDefaultsSettings.logic.ts new file mode 100644 index 0000000000000..68bd7bd81fc6a --- /dev/null +++ b/apps/web/src/components/settings/ProjectDefaultsSettings.logic.ts @@ -0,0 +1,58 @@ +import type { ModelSelection, ProjectId, ServerProvider } from "@t3tools/contracts"; +import type { UnifiedSettings } from "@t3tools/contracts/settings"; + +import { resolveEffectiveDefaultModelSelection } from "../../modelSelection"; +import { + resolveDefaultProviderModelSelection, + resolveProjectDefaultProviderModelSelection, + type ProviderInstanceEntry, +} from "../../providerInstances"; + +export interface DefaultModelSettingsPresentation { + readonly selection: ModelSelection | null; + /** Set when the saved model's provider cannot run; effort stays hidden until it can. */ + readonly providerNote: string | null; +} + +export function resolveDefaultModelSettingsPresentation(input: { + readonly entries: ReadonlyArray; + readonly providers: ReadonlyArray; + readonly settings: UnifiedSettings; + readonly projectId: ProjectId | null; +}): DefaultModelSettingsPresentation { + const saved = input.settings.defaultModelSelection; + if (saved === null) { + const automatic = resolveProjectDefaultProviderModelSelection( + input.providers, + input.settings, + input.projectId, + ); + return { + selection: automatic + ? resolveEffectiveDefaultModelSelection(input.settings, input.providers, automatic) + : null, + providerNote: null, + }; + } + + const savedEntry = input.entries.find((entry) => entry.instanceId === saved.instanceId); + const resolved = resolveDefaultProviderModelSelection(input.providers, saved); + // Settings can turn a provider off before the server snapshot catches up, so check both. + if ( + !savedEntry?.enabled || + !savedEntry.isAvailable || + resolved?.instanceId !== saved.instanceId + ) { + return { selection: saved, providerNote: providerNote(savedEntry) }; + } + return { + selection: resolveEffectiveDefaultModelSelection(input.settings, input.providers, resolved), + providerNote: null, + }; +} + +function providerNote(entry: ProviderInstanceEntry | undefined): string { + if (!entry) return "This provider is not available on this environment."; + if (!entry.enabled) return `${entry.displayName} is turned off.`; + return `${entry.displayName} is unavailable.`; +} diff --git a/apps/web/src/components/settings/ProjectDefaultsSettings.tsx b/apps/web/src/components/settings/ProjectDefaultsSettings.tsx index b091d0d58f3fa..182b8bc5b1fdf 100644 --- a/apps/web/src/components/settings/ProjectDefaultsSettings.tsx +++ b/apps/web/src/components/settings/ProjectDefaultsSettings.tsx @@ -12,13 +12,8 @@ import { useClientSettings } from "../../hooks/useSettings"; import { defaultModelSelectionsAreMixed, getCustomModelOptionsByInstance, - resolveEffectiveDefaultModelSelection, } from "../../modelSelection"; -import { - providerInstanceEntriesFor, - resolveDefaultProviderModelSelection, - resolveProjectDefaultProviderModelSelection, -} from "../../providerInstances"; +import { providerInstanceEntriesFor } from "../../providerInstances"; import { useEnvironmentProviders, useEnvironments } from "../../state/environments"; import { firstManagedDenial } from "../../state/managedPolicy"; import { EMPTY_SERVER_PROVIDERS } from "../../state/server"; @@ -30,6 +25,7 @@ import { TraitsPicker } from "../chat/TraitsPicker"; import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; import { toastManager } from "../ui/toast"; import { Switch } from "../ui/switch"; +import { resolveDefaultModelSettingsPresentation } from "./ProjectDefaultsSettings.logic"; import type { ProjectSettingsCategory } from "./ProjectSettingsPanel"; import { searchableSetting } from "./settingsSearch"; import { useSettingsScope } from "./SettingsScopeContext"; @@ -67,14 +63,14 @@ export function ProjectDefaultsSettings({ category }: { category: ProjectSetting (representative?.serverConfig ? providersByEnvironment.get(representative.environmentId) : undefined) ?? EMPTY_SERVER_PROVIDERS; - const storedSelection = - settings.defaultModelSelection === null - ? resolveProjectDefaultProviderModelSelection(providers, settings, target?.projectId ?? null) - : resolveDefaultProviderModelSelection(providers, settings.defaultModelSelection); - const selection = storedSelection - ? resolveEffectiveDefaultModelSelection(settings, providers, storedSelection) - : null; const entries = providerInstanceEntriesFor(providers, settings, target?.projectId ?? null); + const presentation = resolveDefaultModelSettingsPresentation({ + entries, + providers, + settings, + projectId: target?.projectId ?? null, + }); + const selection = presentation.selection; const modelOptions = getCustomModelOptionsByInstance( settings, providers, @@ -146,7 +142,7 @@ export function ProjectDefaultsSettings({ category }: { category: ProjectSetting status={ mixedModel ? `${automaticModel ? "Automatic defaults differ." : "Defaults differ."} Select one environment or choose a shared model to edit effort.` - : effortDisabledReason + : (presentation.providerNote ?? effortDisabledReason) } resetAction={ settings.defaultModelSelection !== null ? ( @@ -154,7 +150,7 @@ export function ProjectDefaultsSettings({ category }: { category: ProjectSetting ) : null } control={ - selection && activeEntry ? ( + selection ? (
- {!mixedModel && !effortDisabledReason ? ( + {!mixedModel && !presentation.providerNote && !effortDisabledReason && activeEntry ? ( + {restartNotice ? ( +

+ {restartNotice} +

+ ) : null} {urlAuthAction && onAcceptUrlAuth ? (

{urlAuthAction.message}

diff --git a/apps/web/src/components/settings/ScheduledTasksSettings.tsx b/apps/web/src/components/settings/ScheduledTasksSettings.tsx index ff024e5ba2ed2..848b88130f48a 100644 --- a/apps/web/src/components/settings/ScheduledTasksSettings.tsx +++ b/apps/web/src/components/settings/ScheduledTasksSettings.tsx @@ -21,7 +21,6 @@ import type { ProjectId, ScheduledTask, ScheduledTaskId, - ScheduledTaskSchedule, ScheduledTaskThreadMode, ScheduledTaskUpsertInput, ScheduledTaskWebhookDeliverySummary, @@ -35,6 +34,7 @@ import { WEBHOOK_FILTER_OPS, type WebhookFilterDraft, } from "@t3tools/client-runtime/scheduled-task-webhook"; +import { scheduleTimeZoneLabel } from "@t3tools/client-runtime/scheduled-task-time-zone"; import { MAX_WEBHOOK_DELIVERY_AGE_MINUTES, MAX_WEBHOOK_FILTERS, @@ -104,7 +104,9 @@ import { defaultScheduledTaskProjectId, matchesScheduledTaskScope, scheduleFromDraft, + scheduleLabel, scheduledTaskDefaultModel, + WEEKDAY_LABELS, taskToDraft, unbindDraftFromThread, type DraftState, @@ -151,7 +153,6 @@ import { } from "./settingsLayout"; const WEEKDAY_ORDER = [1, 2, 3, 4, 5, 6, 0] as const; -const WEEKDAY_LABELS = ["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"] as const; const WEEKDAY_SHORT = ["Su", "Mo", "Tu", "We", "Th", "Fr", "Sa"] as const; // Thread ids are trimmed, so the leading space keeps this from matching one. @@ -322,30 +323,6 @@ function splitModelKey(value: string): ModelSelection | null { }; } -const ONCE_FORMATTER = new Intl.DateTimeFormat(undefined, { - dateStyle: "medium", - timeStyle: "short", -}); - -export function scheduleLabel(schedule: ScheduledTaskSchedule): string { - if (schedule.type === "webhook") return "On webhook"; - if (schedule.type === "once") return `Once, ${ONCE_FORMATTER.format(new Date(schedule.runAt))}`; - if (schedule.type === "interval") { - const minutes = schedule.everyMs / 60_000; - return Number.isInteger(minutes) - ? `Every ${minutes} min` - : `Every ${Math.round(schedule.everyMs / 1000)} sec`; - } - const weekdays = schedule.weekdays ?? []; - const days = - weekdays.length === 0 - ? "Daily" - : weekdays.length === 5 && weekdays.every((day) => day >= 1 && day <= 5) - ? "Weekdays" - : weekdays.map((day) => WEEKDAY_LABELS[day]).join(", "); - return `${days} at ${schedule.timeOfDay}`; -} - /** formatRelativeTime handles only the past; upcoming runs need future labels. */ function relativeLabel(value: string | null, nowMs: number): string { if (!value) return "Not scheduled"; @@ -552,6 +529,7 @@ function ScheduledTaskEnvironmentList({ return { environment, connected: index >= 0, + timeZone: data?.timeZone, tasks: data?.tasks.filter((task) => matchesScheduledTaskScope(scope, environment.environmentId, task.projectId), ), @@ -586,7 +564,7 @@ function ScheduledTaskEnvironmentList({ return ( - {populated.map(({ environment, tasks }) => { + {populated.map(({ environment, tasks, timeZone }) => { const { primary, secondary } = settingsScopeEnvironmentLabels(environment, environments); return (
@@ -610,6 +588,7 @@ function ScheduledTaskEnvironmentList({ key={task.id} environmentId={environment.environmentId} task={task} + timeZone={timeZone} onEdit={() => onEdit(environment.environmentId, task)} /> ))} @@ -734,6 +713,7 @@ function ScheduledTaskEnvironmentSection({ key={task.id} environmentId={environment.environmentId} task={task} + timeZone={tasksQuery.data?.timeZone} onEdit={() => onEdit(environment.environmentId, task)} /> )) @@ -747,10 +727,12 @@ function ScheduledTaskEnvironmentSection({ function ScheduledTaskRow({ environmentId, task, + timeZone, onEdit, }: { readonly environmentId: EnvironmentId; readonly task: ScheduledTask; + readonly timeZone: string | undefined; readonly onEdit: () => void; }) { const canOperate = useAtomValue( @@ -795,7 +777,7 @@ function ScheduledTaskRow({ status={
- {scheduleLabel(task.schedule)} ·{" "} + {scheduleLabel(task.schedule, timeZone, task.nextRunAt)} ·{" "} {task.blockedByPolicy !== undefined ? ( task.blockedByPolicy ) : !task.enabled ? ( @@ -1416,6 +1398,7 @@ function ScheduledTaskEditorForm({ draft.editingId !== null && tasksQuery.data !== null && !tasksQuery.data.tasks.some((entry) => entry.id === draft.editingId); + const runAtZone = scheduleTimeZoneLabel(tasksQuery.data?.timeZone); const liveTask = tasksQuery.data ? (tasksQuery.data.tasks.find((entry) => entry.id === draft.editingId) ?? null) : task; @@ -2063,6 +2046,9 @@ function ScheduledTaskEditorForm({ ({ ...current, timeOfDay: event.target.value })) } /> + {runAtZone === undefined ? null : ( + + {runAtZone} + + )} on
({ settings: {} as typeof DEFAULT_SERVER_SETTINGS, targets: [] as Array<{ environmentId: string; settings: typeof DEFAULT_SERVER_SETTINGS }>, run: vi.fn(), + update: vi.fn(), report: null as StorageCleanupReport | null, })); @@ -62,7 +66,7 @@ vi.mock(import("./useScopedSettings"), async (original) => ({ ...(await original()), useScopedSettings: (() => state.settings) as never, useScopedSettingsMixed: () => false, - useUpdateScopedSettings: () => vi.fn(), + useUpdateScopedSettings: () => state.update, useClearScopedSettings: () => vi.fn(), })); vi.mock(import("./settingsLayout"), async (original) => ({ @@ -113,6 +117,7 @@ function setup() { })); state.report = null; state.run.mockReset(); + state.update.mockReset(); } function setRetentionValues( @@ -154,6 +159,125 @@ it("keeps cleanup disabled when settled retention and all other rules are off", expect(state.run).not.toHaveBeenCalled(); }); +const rule = "Delete old browser artifacts"; +const ruleSwitch = (screen: Screen) => screen.getByRole("switch", { name: rule }); +const daysInput = (screen: Screen) => screen.getByLabelText(`${rule} in days`); +const saves = () => state.update.mock.calls.map(([patch]) => patch.storageCleanup); +const failSave = (index: number) => act(() => state.update.mock.calls[index]?.[1]?.onFailed?.()); +const withSavedAge = (days: number) => { + state.settings = { + ...state.settings, + storageCleanup: { ...state.settings.storageCleanup, browserArtifactsAfterDays: days }, + }; +}; + +it("saves a newly enabled retention rule only once its age is committed", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + expect(state.update).not.toHaveBeenCalled(); + await expect.element(ruleSwitch(screen)).toBeChecked(); + await expect.element(daysInput(screen)).toHaveFocus(); + + await userEvent.fill(daysInput(screen), "30"); + await userEvent.keyboard("{Enter}"); + expect(saves()).toEqual([{ browserArtifactsAfterDays: 30 }]); + await expect.element(ruleSwitch(screen)).toBeChecked(); +}); + +it("discards a retention draft on Escape, on leaving it untouched, or when switched back off", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + await expect.element(daysInput(screen)).toHaveFocus(); + await userEvent.keyboard("{Escape}"); + await expect.element(ruleSwitch(screen)).not.toBeChecked(); + + await userEvent.click(ruleSwitch(screen)); + await expect.element(daysInput(screen)).toHaveFocus(); + await userEvent.click(screen.getByRole("heading", { name: rule })); + await expect.element(ruleSwitch(screen)).not.toBeChecked(); + + await userEvent.click(ruleSwitch(screen)); + await userEvent.fill(daysInput(screen), "30"); + await userEvent.click(ruleSwitch(screen)); + await expect.element(daysInput(screen)).not.toBeInTheDocument(); + await expect.element(ruleSwitch(screen)).not.toBeChecked(); + expect(state.update).not.toHaveBeenCalled(); +}); + +it("saves an edited draft when focus leaves its row, not when it reaches the switch", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + await userEvent.fill(daysInput(screen), "30"); + await userEvent.tab(); + await expect.element(ruleSwitch(screen)).toHaveFocus(); + expect(state.update).not.toHaveBeenCalled(); + await userEvent.tab(); + expect(saves()).toEqual([{ browserArtifactsAfterDays: 30 }]); +}); + +it("saves a stepped first age only once it is committed", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + await userEvent.click(screen.getByRole("button", { name: `Increase ${rule}` })); + await userEvent.keyboard("{ArrowUp}"); + await expect.element(daysInput(screen)).toHaveValue("10"); + expect(state.update).not.toHaveBeenCalled(); + await userEvent.keyboard("{Enter}"); + expect(saves()).toEqual([{ browserArtifactsAfterDays: 10 }]); +}); + +it("does not save the draft age when Enter is pressed on invalid text", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + await userEvent.fill(daysInput(screen), "+"); + await userEvent.keyboard("{Enter}"); + expect(state.update).not.toHaveBeenCalled(); +}); + +it("switching an edited saved rule off never saves its age afterwards", async () => { + setup(); + withSavedAge(30); + await using screen = await render(panel()); + await userEvent.click(daysInput(screen)); + await userEvent.keyboard("{Control>}a{/Control}40"); + await userEvent.click(ruleSwitch(screen)); + await expect.element(ruleSwitch(screen)).not.toBeChecked(); + await userEvent.click(screen.getByRole("heading", { name: rule })); + expect(saves()).toEqual([{ browserArtifactsAfterDays: null }]); +}); + +it("returns a newly enabled retention rule to off when its first save fails", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + await expect.element(daysInput(screen)).toHaveFocus(); + await userEvent.keyboard("{Enter}"); + expect(saves()).toEqual([{ browserArtifactsAfterDays: 8 }]); + + await failSave(0); + await expect.element(ruleSwitch(screen)).not.toBeChecked(); +}); + +it("keeps a newer pending age open when an older first save fails", async () => { + setup(); + await using screen = await render(panel()); + await userEvent.click(ruleSwitch(screen)); + for (const age of ["30", "60"]) { + await userEvent.fill(daysInput(screen), age); + await userEvent.keyboard("{Enter}"); + } + expect(saves()).toHaveLength(2); + + await failSave(0); + await expect.element(daysInput(screen)).toHaveValue("60"); + await expect.element(ruleSwitch(screen)).toBeChecked(); +}); + it("settles mixed retention statuses independently across selected machines", async () => { setup(); using _timers = fakeTimers(); diff --git a/apps/web/src/components/settings/StorageSettings.tsx b/apps/web/src/components/settings/StorageSettings.tsx index 28f031b5e670e..312fdde385467 100644 --- a/apps/web/src/components/settings/StorageSettings.tsx +++ b/apps/web/src/components/settings/StorageSettings.tsx @@ -109,6 +109,8 @@ export function WorktreesDirectoryRow() { ); } +type SaveOptions = Parameters>[1]; + function RetentionControl({ label, value, @@ -117,19 +119,65 @@ function RetentionControl({ }: { label: string; value: number | null; - onChange: (value: number | null) => void; + onChange: (value: number | null, onFailed: () => void) => void; minimum?: number; }) { const [draft, setDraft] = useState(value); const [savedValue, setSavedValue] = useState(value); + // Saving an age can start a cleanup sweep, so turning a rule on only opens a draft. + // While its first save is in flight this holds that submission. + const [enabling, setEnabling] = useState<"draft" | "edited" | { days: number } | null>(null); + // Holds an age until Enter or focus leaves the row: one edited before focus moved to the + // switch, so switching off never saves it first, or one stepped toward a rule's first age, + // so raising it from 8 never saves a shorter age on the way. + const [held, setHeld] = useState(false); + // Hides a saved rule's age once it is switched off, so no blur can save it after null. + const [switchedOff, setSwitchedOff] = useState(false); if (savedValue !== value) { setSavedValue(value); setDraft(value); + setEnabling(null); + setHeld(false); + setSwitchedOff(false); } + const pending = typeof enabling === "object" ? enabling : null; + const on = (value !== null && !switchedOff) || enabling !== null; + const row = useRef(null); + + const commit = (next: number) => { + const days = Math.min(3650, Math.max(minimum, Math.round(next))); + setDraft(days); + setHeld(false); + if (days === pending?.days) return; + if (value !== null) { + onChange(days, () => {}); + return; + } + const submission = { days }; + setEnabling(submission); + onChange(days, () => setEnabling((current) => (current === submission ? null : current))); + }; + + const switchOff = () => { + setDraft(value); + setHeld(false); + setEnabling(null); + if (value === null && pending === null) return; + setSwitchedOff(true); + onChange(null, () => setSwitchedOff(false)); + }; return ( -
- {value !== null ? ( +
{ + if (event.currentTarget.contains(event.relatedTarget)) return; + if (held && draft !== null) commit(draft); + else setEnabling((current) => (typeof current === "string" ? null : current)); + }} + > + {on ? ( { + onValueChange={(next) => { + setDraft(next); + setEnabling((current) => (current === "draft" ? "edited" : current)); + }} + onValueCommitted={(next, { reason, event }) => { if (next === null) setDraft(value); - else { - const days = Math.min(3650, Math.max(minimum, Math.round(next))); - setDraft(days); - onChange(days); - } + else if ( + reason === "input-blur" + ? event instanceof FocusEvent && + event.relatedTarget instanceof Node && + row.current?.contains(event.relatedTarget) + : value === null + ) { + setDraft(next); + setHeld(true); + } else commit(next); }} > setHeld(false)} + onPaste={() => setHeld(false)} + onKeyDown={(event) => { + if (event.nativeEvent.isComposing || event.keyCode === 229) return; + if (event.key === "Enter") { + // Blurring lets NumberField parse the visible text and commit it once. It + // skips an unedited value, so an untouched draft is committed here. + const input = event.currentTarget; + const shown = + draft !== null && input.value === new Intl.NumberFormat().format(draft); + if (enabling === "draft" && shown) commit(draft); + input.blur(); + } else if (event.key === "Escape" && (held || typeof enabling === "string")) { + // An unhandled Escape closes the settings page. + event.preventDefault(); + event.stopPropagation(); + setDraft(pending?.days ?? value); + setHeld(false); + setEnabling((current) => (typeof current === "string" ? null : current)); + } + }} />
); @@ -486,12 +573,12 @@ export function WorktreeCleanupSection() { // A policy locks these through worktreeCleanup, which also reaches each project's own rules. const lockablePath = (key: keyof WorktreeCleanupRules) => isProjectScope ? ["worktreeCleanup"] : ["worktreeCleanup", `storageCleanup.${key}`]; - const update = (patch: Partial) => - updateSettings({ storageCleanup: patch }); - const updateWorktree = (patch: Partial) => + const update = (patch: Partial, options?: SaveOptions) => + updateSettings({ storageCleanup: patch }, options); + const updateWorktree = (patch: Partial, options?: SaveOptions) => isProjectScope - ? updateSettings({ worktreeCleanup: { mode: "custom", rules: patch } }) - : update(patch); + ? updateSettings({ worktreeCleanup: { mode: "custom", rules: patch } }, options) + : update(patch, options); if ( isProjectScope && @@ -602,7 +689,9 @@ export function WorktreeCleanupSection() { updateWorktree({ worktreeAfterDays })} + onChange={(worktreeAfterDays, onFailed) => + updateWorktree({ worktreeAfterDays }, { onFailed }) + } /> } /> @@ -634,8 +723,8 @@ export function WorktreeCleanupSection() { label="Delete settled worktrees" value={settings.worktreeSettledAfterDays} minimum={0} - onChange={(worktreeSettledAfterDays) => - updateWorktree({ worktreeSettledAfterDays }) + onChange={(worktreeSettledAfterDays, onFailed) => + updateWorktree({ worktreeSettledAfterDays }, { onFailed }) } /> } @@ -737,8 +826,8 @@ export function StorageCleanupSections() { ? "Mixed across selected machines" : undefined; if (scope.kind === "project" || scope.kind === "checkout") return null; - const update = (patch: Partial) => - updateSettings({ storageCleanup: patch }); + const update = (patch: Partial, options: SaveOptions) => + updateSettings({ storageCleanup: patch }, options); const databaseStorageNotice = ( update({ browserArtifactsAfterDays })} + onChange={(browserArtifactsAfterDays, onFailed) => + update({ browserArtifactsAfterDays }, { onFailed }) + } /> } /> @@ -789,7 +880,7 @@ export function StorageCleanupSections() { update({ logsAfterDays })} + onChange={(logsAfterDays, onFailed) => update({ logsAfterDays }, { onFailed })} /> } /> diff --git a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts index 2c96157e831f6..a4918c3d2e123 100644 --- a/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts +++ b/apps/web/src/components/settings/scheduledTasksSettings.logic.test.ts @@ -22,6 +22,7 @@ import { defaultScheduledTaskProjectId, matchesScheduledTaskScope, scheduleFromDraft, + scheduleLabel, taskToDraft, unbindDraftFromThread, } from "./scheduledTasksSettings.logic"; @@ -407,3 +408,12 @@ it("rejects dedupe templates without a request placeholder", () => { expect(scheduleFromDraft({ ...draft, replayKey: "literal" })).toBeNull(); expect(scheduleFromDraft({ ...draft, coalesceKey: "{{body.bad" })).toBeNull(); }); + +it("appends the server zone only to fixed-time schedule labels", () => { + expect(scheduleLabel({ type: "fixed_time", timeOfDay: "16:31" }, "Etc/UTC")).toBe( + "Daily at 16:31 UTC", + ); + expect(scheduleLabel({ type: "fixed_time", timeOfDay: "09:00" })).toBe("Daily at 09:00"); + expect(scheduleLabel({ type: "interval", everyMs: 60_000 }, "Etc/UTC")).toBe("Every 1 min"); + expect(scheduleLabel({ type: "webhook", signature: null }, "Etc/UTC")).toBe("On webhook"); +}); diff --git a/apps/web/src/components/settings/scheduledTasksSettings.logic.ts b/apps/web/src/components/settings/scheduledTasksSettings.logic.ts index 26d83da69f428..1095db83469ba 100644 --- a/apps/web/src/components/settings/scheduledTasksSettings.logic.ts +++ b/apps/web/src/components/settings/scheduledTasksSettings.logic.ts @@ -5,6 +5,7 @@ import { type ProjectId, ScheduledTaskId, type ScheduledTask, + type ScheduledTaskSchedule, type ScheduledTaskThreadMode, type ScheduledTaskUpsertSchedule, scheduledTaskThreadMode, @@ -22,6 +23,7 @@ import { webhookFilterDraftsChanged, type WebhookFilterDraft, } from "@t3tools/client-runtime/scheduled-task-webhook"; +import { scheduleTimeZoneLabel } from "@t3tools/client-runtime/scheduled-task-time-zone"; import { resolveProjectSettings, @@ -31,6 +33,37 @@ import type { ProviderInstanceEntry } from "../../providerInstances"; import type { ResolvedSettingsScope } from "./settingsScope"; +export const WEEKDAY_LABELS = ["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"] as const; + +const ONCE_FORMATTER = new Intl.DateTimeFormat(undefined, { + dateStyle: "medium", + timeStyle: "short", +}); + +export function scheduleLabel( + schedule: ScheduledTaskSchedule, + timeZone?: string, + runAt?: string | null, +): string { + if (schedule.type === "webhook") return "On webhook"; + if (schedule.type === "once") return `Once, ${ONCE_FORMATTER.format(new Date(schedule.runAt))}`; + if (schedule.type === "interval") { + const minutes = schedule.everyMs / 60_000; + return Number.isInteger(minutes) + ? `Every ${minutes} min` + : `Every ${Math.round(schedule.everyMs / 1000)} sec`; + } + const weekdays = schedule.weekdays ?? []; + const days = + weekdays.length === 0 + ? "Daily" + : weekdays.length === 5 && weekdays.every((day) => day >= 1 && day <= 5) + ? "Weekdays" + : weekdays.map((day) => WEEKDAY_LABELS[day]).join(", "); + const zone = scheduleTimeZoneLabel(timeZone, runAt); + return `${days} at ${schedule.timeOfDay}${zone === undefined ? "" : ` ${zone}`}`; +} + export function matchesScheduledTaskScope( scope: ResolvedSettingsScope, environmentId: EnvironmentId, diff --git a/apps/web/src/components/sidebar/SidebarUpdatePill.tsx b/apps/web/src/components/sidebar/SidebarUpdatePill.tsx index 3c18b2c7a927b..5df8007ef4677 100644 --- a/apps/web/src/components/sidebar/SidebarUpdatePill.tsx +++ b/apps/web/src/components/sidebar/SidebarUpdatePill.tsx @@ -308,6 +308,7 @@ function SidebarUpdateControl() { try { confirmed = await ensureLocalApi().dialogs.confirm( getDesktopUpdateInstallConfirmationMessage(state), + { initialFocus: "confirm" }, ); } catch (error) { setIsActionPending(false); diff --git a/apps/web/src/components/ui/alert-dialog.tsx b/apps/web/src/components/ui/alert-dialog.tsx index 6319dcfa5a052..909fe8905fc3d 100644 --- a/apps/web/src/components/ui/alert-dialog.tsx +++ b/apps/web/src/components/ui/alert-dialog.tsx @@ -1,4 +1,5 @@ import { AlertDialog as AlertDialogPrimitive } from "@base-ui/react/alert-dialog"; +import { useRender } from "@base-ui/react/use-render"; import { cn } from "~/lib/utils"; import { @@ -6,7 +7,12 @@ import { DIALOG_MOBILE_SHEET_CLASS, DIALOG_POPUP_CLASS, } from "~/components/ui/dialog-styles"; -import { ControlledOpenContext, renderRetainedPopup } from "~/components/ui/dialog"; +import { + ControlledOpenContext, + DialogAction, + renderRetainedPopup, + useDialogActionInitialFocus, +} from "~/components/ui/dialog"; function AlertDialog(props: AlertDialogPrimitive.Root.Props) { return ( @@ -51,23 +57,31 @@ function AlertDialogPopup({ bottomStickOnMobile?: boolean; portalContainer?: AlertDialogPrimitive.Portal.Props["container"]; }) { + const { popupRef, initialFocus } = useDialogActionInitialFocus(); + const popup = useRender({ + ref: popupRef, + render: ( + + ), + }); return ( - + {popup} ); @@ -146,5 +160,6 @@ export { AlertDialogTitle, AlertDialogDescription, AlertDialogClose, + DialogAction as AlertDialogAction, AlertDialogViewport, }; diff --git a/apps/web/src/components/ui/dialog.browser.test.tsx b/apps/web/src/components/ui/dialog.browser.test.tsx index b82b66abcae70..33c1cbc2383b6 100644 --- a/apps/web/src/components/ui/dialog.browser.test.tsx +++ b/apps/web/src/components/ui/dialog.browser.test.tsx @@ -1,10 +1,21 @@ -import { expect, it, vi } from "vite-plus/test"; -import { page } from "vite-plus/test/browser"; +import { createRef } from "react"; +import { describe, expect, it, vi } from "vite-plus/test"; +import { page, userEvent } from "vite-plus/test/browser"; +import { requestConfirmDialog, resetConfirmDialogForTests } from "~/confirmDialog"; import "~/index.css"; -import { render } from "../../../test/render"; +import { act, render } from "../../../test/render"; import { applyGlassAppearance } from "../../appearanceGlass"; -import { Dialog, DialogPopup } from "./dialog"; +import { ConfirmDialogHost } from "../ConfirmDialogHost"; +import { Button } from "./button"; +import { + Dialog, + DialogAction, + DialogClose, + DialogPopup, + DialogTitle, + DialogTrigger, +} from "./dialog"; function setup(opacity: number) { const root = document.documentElement; @@ -30,10 +41,15 @@ it.each([80, 100])( using _appearance = setup(initialOpacity); await using _screen = await render( - Dialog content + + Dialog content + Continue + , ); await expect.element(page.getByRole("dialog", { name: "Example dialog" })).toBeVisible(); + const action = page.getByRole("button", { name: "Continue", exact: true }); + await expect.element(action).toHaveFocus(); const backdrop = document.querySelector('[data-slot="dialog-backdrop"]'); if (!(backdrop instanceof HTMLElement)) throw new Error("Missing dialog backdrop"); const root = document.documentElement; @@ -54,7 +70,100 @@ it.each([80, 100])( expect(root.style.getPropertyValue("--glass-blur")).toBe(opaque ? "0px" : ""); expect(root.style.getPropertyValue("--dialog-backdrop-filter")).toBe(opaque ? "none" : ""); expect(getComputedStyle(backdrop).backgroundColor).toBe(tint); + await expect.element(action).toHaveFocus(); } } }, ); + +describe("dialog initial focus", () => { + it.each([ + { label: "generic", options: undefined, focused: "Cancel", confirmed: false }, + { + label: "destructive opt-in", + options: { variant: "destructive", initialFocus: "confirm" }, + focused: "Cancel", + confirmed: false, + }, + { label: "opt-in", options: { initialFocus: "confirm" }, focused: "Confirm", confirmed: true }, + ] as const)( + "focuses $focused for a $label confirmation", + async ({ options, focused, confirmed }) => { + resetConfirmDialogForTests(); + await using _screen = await render(); + let confirmation: Promise | undefined; + await act(() => { + confirmation = requestConfirmDialog("Continue?", options); + }); + + await expect.element(page.getByRole("button", { name: focused, exact: true })).toHaveFocus(); + await userEvent.keyboard("{Enter}"); + await expect(confirmation).resolves.toBe(confirmed); + }, + ); + + it("focuses the dialog action and keeps the caller's popup ref", async () => { + const popup = createRef(); + await using _screen = await render( + + Open retry + + Import failed + Cancel + Try again + + , + ); + await act(() => page.getByRole("button", { name: "Open retry" }).element().focus()); + await userEvent.keyboard("{Enter}"); + + await expect.element(page.getByRole("button", { name: "Try again" })).toHaveFocus(); + expect(popup.current).toBe(page.getByRole("dialog").element()); + }); + + it.each(["disabled action", "caller initialFocus"] as const)( + "falls back to Cancel for a %s", + async (reason) => { + const cancel = createRef(); + await using _screen = await render( + + + Install update? + }> + Cancel + + Install + + , + ); + + await expect.element(page.getByRole("button", { name: "Cancel", exact: true })).toHaveFocus(); + }, + ); + + it("focuses the popup instead of the action when opened by touch", async () => { + await using _screen = await render( + + Open installer + + Install relay client + Cancel + Install + + , + ); + + const trigger = page.getByRole("button", { name: "Open installer" }).element(); + await act(() => { + trigger.dispatchEvent( + new PointerEvent("pointerdown", { bubbles: true, pointerType: "touch" }), + ); + trigger.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, pointerType: "touch" })); + trigger.dispatchEvent( + new PointerEvent("click", { bubbles: true, pointerType: "touch", detail: 1 }), + ); + }); + + await expect.element(page.getByRole("dialog")).toHaveFocus(); + }); +}); diff --git a/apps/web/src/components/ui/dialog.tsx b/apps/web/src/components/ui/dialog.tsx index 2c5bc14905bbc..de48db6325ffe 100644 --- a/apps/web/src/components/ui/dialog.tsx +++ b/apps/web/src/components/ui/dialog.tsx @@ -1,8 +1,9 @@ import { Dialog as DialogPrimitive } from "@base-ui/react/dialog"; +import { useRender } from "@base-ui/react/use-render"; import { XIcon } from "lucide-react"; -import { type ComponentProps, createContext, type ReactNode, use, useState } from "react"; +import { type ComponentProps, createContext, type ReactNode, use, useRef, useState } from "react"; import { cn } from "~/lib/utils"; -import { Button } from "~/components/ui/button"; +import { Button, type ButtonVariant } from "~/components/ui/button"; import { DIALOG_BACKDROP_CLASS, DIALOG_MEDIA_BACKDROP_CLASS, @@ -40,6 +41,20 @@ function Dialog(props: DialogPrimitive.Root.Props) { ); } +function useDialogActionInitialFocus() { + const popupRef = useRef(null); + const initialFocus = (openType: string) => { + // Match Base UI: touch opens focus the popup to avoid opening the virtual keyboard. + if (openType === "touch") return popupRef.current; + return ( + popupRef.current?.querySelector( + '[data-slot="dialog-action"]:not(:disabled):not([aria-disabled="true"])', + ) ?? true + ); + }; + return { popupRef, initialFocus }; +} + const DialogPortal = DialogPrimitive.Portal; function DialogTrigger(props: DialogPrimitive.Trigger.Props) { @@ -93,6 +108,36 @@ function DialogPopup({ bottomStickOnMobile?: boolean; variant?: "default" | "media"; }) { + const { popupRef, initialFocus } = useDialogActionInitialFocus(); + const popup = useRender({ + ref: popupRef, + render: ( + + {children} + {showCloseButton && ( + } + > + + + )} + + ), + }); return ( {/* Media opens from inside other overlays (a composer chip, a popover), so it sits above them. */} @@ -104,34 +149,20 @@ function DialogPopup({ "z-[60] grid-rows-1 place-items-center px-4 pb-6 pt-[calc(var(--native-titlebar-height,0px)+1.5rem)] [-webkit-app-region:no-drag]", )} > - - {children} - {showCloseButton && ( - } - > - - - )} - + {popup} ); } +type DialogActionProps = Omit, "variant"> & { + variant?: Exclude; +}; + +function DialogAction(props: DialogActionProps) { + return