diff --git a/README.md b/README.md index aba4245dd65b..463c80c31982 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ T3 Code Fold extends [T3 Code](https://github.com/pingdotgg/t3code) with a folda ## Extra features -Compared with upstream `main` at [08463e2c40](https://github.com/pingdotgg/t3code/commit/08463e2c40), reviewed **9 September 2026**. These tables describe the code on Fold's `main`; an older published installer may not include every change. +Compared with upstream `main` at [adcd90858c](https://github.com/pingdotgg/t3code/commit/adcd90858c), synced **21 September 2026**. These tables describe the code on Fold's `main`; an older published installer may not include every change. ### Agents, voice, and compatibility diff --git a/apps/desktop/src/ipc/channels.ts b/apps/desktop/src/ipc/channels.ts index 226793657848..f49b6dbd968e 100644 --- a/apps/desktop/src/ipc/channels.ts +++ b/apps/desktop/src/ipc/channels.ts @@ -113,3 +113,5 @@ export const PREVIEW_POINTER_EVENT_CHANNEL = "desktop:preview-pointer-event"; export const MAC_PERMISSION_HELPER_CHANNEL = "desktop:mac-permission-helper"; export const CHECK_SYSTEM_PERMISSION_CHANNEL = "desktop:check-system-permission"; + +export const PREVIEW_RECORDING_INPUT_CHANNEL = "desktop:preview-recording-input"; diff --git a/apps/desktop/src/ipc/methods/preview.ts b/apps/desktop/src/ipc/methods/preview.ts index 5fb7eff99fc6..36cf7c9abe15 100644 --- a/apps/desktop/src/ipc/methods/preview.ts +++ b/apps/desktop/src/ipc/methods/preview.ts @@ -30,11 +30,13 @@ import { } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; +import * as Option from "effect/Option"; import * as NodeURL from "node:url"; import * as ElectronWindow from "../../electron/ElectronWindow.ts"; import * as BrowserImport from "../../preview/BrowserImport/BrowserImport.ts"; import * as PreviewManager from "../../preview/Manager.ts"; +import * as DesktopClientSettings from "../../settings/DesktopClientSettings.ts"; import { PREVIEW_WEBVIEW_PREFERENCES } from "../../preview/WebviewPreferences.ts"; import * as IpcChannels from "../channels.ts"; import * as DesktopIpc from "../DesktopIpc.ts"; @@ -50,6 +52,9 @@ export const installPreviewEventForwarding = Effect.fn( yield* manager.subscribeRecordingFrames((frame) => electronWindow.sendAll(IpcChannels.PREVIEW_RECORDING_FRAME_CHANNEL, frame), ); + yield* manager.subscribeRecordingInputs((event) => + electronWindow.sendAll(IpcChannels.PREVIEW_RECORDING_INPUT_CHANNEL, event), + ); yield* manager.subscribePointerEvents((event) => electronWindow.sendAll(IpcChannels.PREVIEW_POINTER_EVENT_CHANNEL, event), ); @@ -180,11 +185,21 @@ export const cancelPickElement = tabMethod( "desktop.ipc.preview.cancelPickElement", (manager, tabId) => manager.cancelPickElement(tabId), ); -export const startRecording = tabMethod( - IpcChannels.PREVIEW_RECORDING_START_CHANNEL, - "desktop.ipc.preview.startRecording", - (manager, tabId) => manager.startRecording(tabId), -); +export const startRecording = DesktopIpc.makeIpcMethod({ + channel: IpcChannels.PREVIEW_RECORDING_START_CHANNEL, + payload: DesktopPreviewTabInputSchema, + result: Schema.Void, + handler: Effect.fn("desktop.ipc.preview.startRecording")(function* ({ tabId }) { + const manager = yield* PreviewManager.PreviewManager; + const store = yield* DesktopClientSettings.DesktopClientSettings; + const settings = yield* store.get; + const options = Option.map(settings, (value) => ({ + showKeyPresses: value.browserRecordingShowKeyPresses, + showMousePresses: value.browserRecordingShowMousePresses, + })); + yield* manager.startRecording(tabId, Option.getOrUndefined(options)); + }), +}); export const stopRecording = tabMethod( IpcChannels.PREVIEW_RECORDING_STOP_CHANNEL, "desktop.ipc.preview.stopRecording", diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 885b31a64d6b..7256f610fbeb 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -1,6 +1,7 @@ import type { DesktopBridge, DesktopPreviewPointerEvent, + DesktopPreviewRecordingInputEvent, DesktopPreviewRecordingFrame, DesktopPreviewTabState, DesktopSnapShotEvent, @@ -327,6 +328,15 @@ contextBridge.exposeInMainWorld("desktopBridge", { ipcRenderer.invoke(IpcChannels.PREVIEW_PICTURE_IN_PICTURE_CLOSE_CHANNEL, { tabId }), }, recording: { + onInput: (listener) => { + const wrappedListener = (_event: Electron.IpcRendererEvent, event: unknown) => { + if (typeof event !== "object" || event === null) return; + listener(event as DesktopPreviewRecordingInputEvent); + }; + ipcRenderer.on(IpcChannels.PREVIEW_RECORDING_INPUT_CHANNEL, wrappedListener); + return () => + ipcRenderer.removeListener(IpcChannels.PREVIEW_RECORDING_INPUT_CHANNEL, wrappedListener); + }, startScreencast: (tabId) => ipcRenderer.invoke(IpcChannels.PREVIEW_RECORDING_START_CHANNEL, { tabId }), stopScreencast: (tabId) => diff --git a/apps/desktop/src/preview/GuestProtocol.ts b/apps/desktop/src/preview/GuestProtocol.ts index e63597b71efc..1a73bb30f29e 100644 --- a/apps/desktop/src/preview/GuestProtocol.ts +++ b/apps/desktop/src/preview/GuestProtocol.ts @@ -5,3 +5,8 @@ export const ANNOTATION_CAPTURED_CHANNEL = "preview:annotation-captured"; export const ANNOTATION_THEME_CHANNEL = "preview:annotation-theme"; export const HUMAN_INPUT_CHANNEL = "preview:human-input"; export const MOUSE_NAVIGATE_CHANNEL = "preview:mouse-navigate"; +export const RECORDING_CURSOR_CHANNEL = "preview:recording-cursor"; +export const RECORDING_POINTER_CHANNEL = "preview:recording-pointer"; +export const RECORDING_KEY_CHANNEL = "preview:recording-key"; +export const RECORDING_INPUT_CHANNEL = "preview:recording-input"; +export const RECORDING_CONTROLLER_CHANNEL = "preview:recording-controller"; diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 61d3e4383363..02a92f0931cd 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -1,7 +1,10 @@ import * as NodeVM from "node:vm"; import { it as effectIt } from "@effect/vitest"; import { DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER } from "@t3tools/contracts"; -import type { DesktopPreviewRecordingFrame } from "@t3tools/contracts"; +import type { + DesktopPreviewRecordingFrame, + DesktopPreviewRecordingInputEvent, +} from "@t3tools/contracts"; import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; import * as Cause from "effect/Cause"; import * as Deferred from "effect/Deferred"; @@ -2885,6 +2888,165 @@ describe("PreviewManager", () => { ), ); + effectIt.effect( + "restores the native cursor when recording startup fails, then allows a retry", + () => + withManager((manager) => + Effect.gen(function* () { + const host = makeTestHostWebContents(); + host.executeJavaScript.mockResolvedValueOnce(false); + let cursorActive = false; + const cursorAtCapture: boolean[] = []; + const contents = Object.assign( + makeTestPreviewWebContents( + async () => { + cursorAtCapture.push(cursorActive); + return { + toJPEG: () => Buffer.from("frame"), + getSize: () => ({ width: 800, height: 600 }), + }; + }, + 42, + host, + ), + { + send: (channel: string, active: unknown) => { + if (channel === "preview:recording-cursor") cursorActive = active === true; + }, + }, + ); + fromId.mockReturnValue(contents as never); + yield* manager.createTab("tab_cursor"); + yield* manager.registerWebview("tab_cursor", 42); + const failed = yield* Effect.exit(manager.startRecording("tab_cursor")); + expect(Exit.isFailure(failed)).toBe(true); + expect(cursorAtCapture).toEqual([true]); + expect(cursorActive).toBe(false); + + yield* manager.startRecording("tab_cursor"); + expect(cursorAtCapture).toEqual([true, true]); + expect(cursorActive).toBe(true); + yield* manager.stopRecording("tab_cursor"); + expect(cursorActive).toBe(false); + }), + ), + ); + + effectIt.effect("restores the recording cursor after navigation only while recording", () => + withManager((manager) => + Effect.gen(function* () { + const listeners = new Map void>(); + let cursorActive = false; + let cursorUpdated: (() => void) | undefined; + let inputOptions: unknown; + const options = { showKeyPresses: true, showMousePresses: false }; + const contents = Object.assign( + makeTestPreviewWebContents(async () => ({ + toJPEG: () => Buffer.from("frame"), + getSize: () => ({ width: 800, height: 600 }), + })), + { + on: (event: string, listener: () => void) => listeners.set(event, listener), + send: (channel: string, active: unknown, recordingOptions: unknown) => { + if (channel !== "preview:recording-cursor") return; + cursorActive = active === true; + inputOptions = recordingOptions; + cursorUpdated?.(); + }, + }, + ); + fromId.mockReturnValue(contents as never); + yield* manager.createTab("tab_cursor_reload"); + yield* manager.registerWebview("tab_cursor_reload", 42); + yield* manager.startRecording("tab_cursor_reload", options); + for (const recording of [true, false]) { + if (!recording) yield* manager.stopRecording("tab_cursor_reload"); + // A new document has lost the previous preload's cursor overlay. + cursorActive = false; + const restored = new Promise((resolve) => { + cursorUpdated = resolve; + }); + listeners.get("dom-ready")?.(); + yield* Effect.promise(() => restored); + cursorUpdated = undefined; + expect(cursorActive).toBe(recording); + expect(inputOptions).toEqual(recording ? options : undefined); + } + }), + ), + ); + + effectIt.effect("gates recording decorations and isolates failed subscribers", () => + withManager((manager) => + Effect.gen(function* () { + const callbacks = new Map< + string, + (event: unknown, input: unknown) => Fiber.Fiber | undefined + >(); + const contents = Object.assign( + makeTestPreviewWebContents(async () => ({ + toJPEG: () => Buffer.from("frame"), + getSize: () => ({ width: 800, height: 600 }), + })), + { + ipc: { + on: ( + channel: string, + callback: (event: unknown, input: unknown) => Fiber.Fiber | undefined, + ) => callbacks.set(channel, callback), + off: vi.fn(), + }, + }, + ); + fromId.mockReturnValue(contents as never); + yield* manager.createTab("tab_recording_input"); + yield* manager.registerWebview("tab_recording_input", 42); + const received: DesktopPreviewRecordingInputEvent[] = []; + yield* manager.subscribeRecordingInputs(() => Effect.die("renderer unavailable")); + yield* manager.subscribeRecordingInputs((event) => + Effect.sync(() => { + received.push(event); + }), + ); + const send = (input: unknown) => + Effect.gen(function* () { + const fiber = callbacks.get("preview:recording-input")?.(null, input); + if (fiber) yield* Fiber.join(fiber); + }); + const key = { type: "key", label: "⌘C", held: true, width: 800 }; + const pointer = { + type: "pointer", + phase: "down", + x: 120, + y: 80, + width: 800, + height: 600, + }; + yield* send(key); + expect(received).toEqual([]); + yield* manager.startRecording("tab_recording_input", { + showKeyPresses: true, + showMousePresses: false, + }); + yield* send(key); + yield* send(pointer); + yield* send({ ...key, width: 0 }); + expect(received).toEqual([{ tabId: "tab_recording_input", input: key }]); + yield* manager.stopRecording("tab_recording_input"); + yield* send(key); + expect(received).toHaveLength(1); + yield* manager.startRecording("tab_recording_input", { + showKeyPresses: false, + showMousePresses: true, + }); + yield* send(key); + yield* send(pointer); + expect(received.at(-1)).toEqual({ tabId: "tab_recording_input", input: pointer }); + expect(received).toHaveLength(2); + }), + ), + ); + effectIt.effect("continues native recording when the source warmup fails", () => withManager((manager) => Effect.gen(function* () { @@ -3853,88 +4015,108 @@ describe("PreviewManager", () => { ), ); - effectIt.effect("emits the resolved pointer target before dispatching an automation click", () => - withManager((manager) => - Effect.gen(function* () { - let humanInput: ((_event: unknown, signal: unknown) => void) | undefined; - const activity: string[] = []; - const sendCommand = vi.fn(async (method: string, params?: Record) => { - if (method === "Runtime.evaluate") { - return { - result: { - value: { width: 800, height: 600 }, - }, - }; - } - if (method === "Input.dispatchMouseEvent" && params?.type === "mousePressed") { - activity.push("mousePressed"); - humanInput?.({}, { kind: "pointer", x: params.x, y: params.y, button: 0 }); - } - return undefined; - }); - fromId.mockReturnValue({ - id: 42, - isDestroyed: () => false, - getType: () => "webview", - getURL: () => "https://example.com", - getTitle: () => "Example", - isLoading: () => false, - isDevToolsOpened: () => false, - getZoomFactor: () => 1, - setZoomFactor: vi.fn(), - setAudioMuted: vi.fn(), - isCurrentlyAudible: () => false, - on: vi.fn(), - off: vi.fn(), - ipc: { - on: vi.fn((channel: string, listener: typeof humanInput) => { - if (channel === "preview:human-input") humanInput = listener; - }), - off: vi.fn(), - }, - send: webviewSend, - navigationHistory: { canGoBack: () => false, canGoForward: () => false }, - setIgnoreMenuShortcuts: vi.fn(), - setWindowOpenHandler: vi.fn(), - debugger: { - isAttached: () => false, - attach: vi.fn(), - sendCommand, + effectIt.effect( + "records the resolved pointer target before dispatching an automation click", + () => + withManager((manager) => + Effect.gen(function* () { + let humanInput: ((_event: unknown, signal: unknown) => void) | undefined; + const activity: string[] = []; + const sendCommand = vi.fn(async (method: string, params?: Record) => { + if (method === "Runtime.evaluate") { + return { + result: { + value: { width: 800, height: 600 }, + }, + }; + } + if (method === "Input.dispatchMouseEvent" && params?.type === "mousePressed") { + activity.push("mousePressed"); + humanInput?.({}, { kind: "pointer", x: params.x, y: params.y, button: 0 }); + } + return undefined; + }); + fromId.mockReturnValue({ + id: 42, + hostWebContents: makeTestHostWebContents(), + capturePage: vi.fn(async () => ({ toPNG: () => Buffer.from("frame") })), + setBackgroundThrottling: vi.fn(), + isDestroyed: () => false, + getType: () => "webview", + getURL: () => "https://example.com", + getTitle: () => "Example", + isLoading: () => false, + isDevToolsOpened: () => false, + getZoomFactor: () => 1, + setZoomFactor: vi.fn(), + setAudioMuted: vi.fn(), + isCurrentlyAudible: () => false, on: vi.fn(), off: vi.fn(), - }, - } as never); + ipc: { + on: vi.fn((channel: string, listener: typeof humanInput) => { + if (channel === "preview:human-input") humanInput = listener; + }), + off: vi.fn(), + }, + send: webviewSend, + navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), + setWindowOpenHandler: vi.fn(), + debugger: { + isAttached: () => false, + attach: vi.fn(), + sendCommand, + on: vi.fn(), + off: vi.fn(), + }, + } as never); - yield* manager.subscribePointerEvents((event) => - Effect.sync(() => { - activity.push(event.phase); - }), - ); - yield* manager.createTab("tab_1"); - yield* manager.registerWebview("tab_1", 42); - const click = yield* manager - .automationClick("tab_1", { x: 120, y: 80 }) - .pipe(Effect.forkChild({ startImmediately: true })); - yield* TestClock.adjust(200); - yield* Fiber.join(click); + yield* manager.subscribePointerEvents((event) => + Effect.sync(() => { + activity.push(event.phase); + }), + ); + yield* manager.createTab("tab_1"); + yield* manager.registerWebview("tab_1", 42); + yield* manager.startRecording("tab_1"); + const click = yield* manager + .automationClick("tab_1", { x: 120, y: 80 }) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(200); + yield* Fiber.join(click); - expect(activity).toEqual(["move", "click", "mousePressed"]); - expect(sendCommand).toHaveBeenCalledWith("Input.dispatchMouseEvent", { - type: "mousePressed", - x: 120, - y: 80, - button: "left", - clickCount: 1, - }); - expect(sendCommand).toHaveBeenCalledWith("Input.dispatchMouseEvent", { - type: "mouseReleased", - x: 120, - y: 80, - button: "left", - clickCount: 1, - }); - }), - ), + expect(activity).toEqual(["move", "click", "mousePressed"]); + expect( + webviewSend.mock.calls + .filter(([channel]) => channel === "preview:recording-controller") + .map(([, controller]) => controller), + ).toEqual(["agent", "none"]); + + const recordedPointer = webviewSend.mock.calls + .filter(([channel]) => channel === "preview:recording-pointer") + .map(([, event]) => event); + expect(recordedPointer).toEqual([ + expect.objectContaining({ phase: "move", x: 120, y: 80 }), + expect.objectContaining({ phase: "click", x: 120, y: 80 }), + ]); + expect(sendCommand).toHaveBeenCalledWith("Input.dispatchMouseEvent", { + type: "mousePressed", + x: 120, + y: 80, + button: "left", + clickCount: 1, + }); + yield* manager.stopRecording("tab_1"); + expect(sendCommand).toHaveBeenCalledWith("Input.dispatchMouseEvent", { + type: "mouseReleased", + x: 120, + y: 80, + button: "left", + clickCount: 1, + }); + }), + ), ); effectIt.effect( @@ -4318,6 +4500,9 @@ describe("PreviewManager", () => { }); fromId.mockReturnValue({ id: 42, + hostWebContents: makeTestHostWebContents(), + capturePage: vi.fn(async () => ({ toPNG: () => Buffer.from("frame") })), + setBackgroundThrottling: vi.fn(), isDestroyed: () => false, getType: () => "webview", getURL: () => "https://example.com", @@ -4351,6 +4536,7 @@ describe("PreviewManager", () => { yield* manager.createTab("tab_1"); yield* manager.registerWebview("tab_1", 42); + yield* manager.startRecording("tab_1"); const click = yield* manager .automationClick("tab_1", { x: 120, y: 80 }) @@ -4358,6 +4544,12 @@ describe("PreviewManager", () => { yield* TestClock.adjust(200); const exit = yield* Fiber.await(click); expect(Exit.isFailure(exit)).toBe(true); + expect( + webviewSend.mock.calls + .filter(([channel]) => channel === "preview:recording-controller") + .map(([, controller]) => controller), + ).toEqual(["agent", "human", "none"]); + yield* manager.stopRecording("tab_1"); if (Exit.isSuccess(exit)) return; const error = Option.getOrThrow(Cause.findErrorOption(exit.cause)); expect(error).toMatchObject({ diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index b9a709bd0e2b..16191e0cfccc 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -6,7 +6,10 @@ * here). Single layer-scoped browser session partition. */ import * as NodeCrypto from "node:crypto"; -import { DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER } from "@t3tools/contracts"; +import { + DesktopPreviewRecordingInputSchema, + DESKTOP_PREVIEW_RECORDING_CAPTURE_TRIGGER, +} from "@t3tools/contracts"; import type { DesktopPreviewAnnotationTheme, DesktopPreviewAutomationStatus, @@ -18,6 +21,7 @@ import type { PreviewAnnotationSubmissionResult, DesktopPreviewRecordingArtifact, DesktopPreviewRecordingFrame, + DesktopPreviewRecordingInputEvent, DesktopPreviewScreenshotArtifact, DesktopPreviewTabDefaults, PreviewAutomationClickInput, @@ -71,6 +75,11 @@ import { ELEMENT_PICKED_CHANNEL, HUMAN_INPUT_CHANNEL, MOUSE_NAVIGATE_CHANNEL, + RECORDING_CURSOR_CHANNEL, + RECORDING_POINTER_CHANNEL, + RECORDING_KEY_CHANNEL, + RECORDING_INPUT_CHANNEL, + RECORDING_CONTROLLER_CHANNEL, START_PICK_CHANNEL, } from "./GuestProtocol.ts"; import { isPreviewAnnotationPayload } from "./PickedElementPayload.ts"; @@ -81,6 +90,7 @@ import { previewAutomationEditingCommandExpression, } from "./PreviewKeyboard.ts"; import { captureFavicon, safeHttpOrigin, selectFaviconCandidates } from "./FaviconCapture.ts"; +import { DEFAULT_RECORDING_INPUT_OPTIONS, type RecordingInputOptions } from "./RecordingInput.ts"; export type PreviewNavStatus = | { kind: "Idle" } @@ -437,6 +447,7 @@ interface ManagedListeners { type FrameCaptureConsumer = "picture-in-picture" | "recording"; interface FrameCaptureSession { + readonly recordingInputOptions?: RecordingInputOptions; readonly scope: Scope.Closeable | null; readonly consumers: ReadonlySet; readonly unthrottledWebContentsIds: ReadonlySet; @@ -485,6 +496,10 @@ interface BrowserDiagnostics { readonly requests: ReadonlyMap; } +const isRecordingInput = Schema.is(DesktopPreviewRecordingInputSchema); + +type RecordingInputListener = (event: DesktopPreviewRecordingInputEvent) => Effect.Effect; + type PointerEventListener = (event: DesktopPreviewPointerEvent) => Effect.Effect; interface ExpectedAgentInput { @@ -636,6 +651,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const attachedRef = yield* Ref.make>(new Map()); const listenersRef = yield* Ref.make>(new Set()); const pointerEventListenersRef = yield* Ref.make>(new Set()); + const recordingInputListenersRef = yield* Ref.make>( + new Set(), + ); const recordingFrameListenersRef = yield* Ref.make>( new Set(), ); @@ -821,6 +839,20 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function return Effect.succeed([undefined, sessions] as const); } return setFrameCaptureWebContentsBackgroundThrottling(wc, false).pipe( + Effect.tap(() => + Effect.gen(function* () { + if (!current.consumers.has("recording")) return; + const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId); + yield* attempt({ operation: "recording.cursor", tabId, webContentsId: wc.id }, () => + wc.send( + RECORDING_CURSOR_CHANNEL, + true, + current.recordingInputOptions, + tab?.controller, + ), + ); + }), + ), Effect.map( () => [ @@ -846,6 +878,15 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function if (!current || !current.consumers.has(consumer)) { return [undefined, sessions] as const; } + if (consumer === "recording") { + yield* Effect.forEach(current.unthrottledWebContentsIds, (id) => + attempt({ operation: "recording.cursor", tabId, webContentsId: id }, () => { + const contents = webContents.fromId(id); + if (contents && !contents.isDestroyed()) + contents.send(RECORDING_CURSOR_CHANNEL, false); + }).pipe(Effect.ignore), + ); + } const consumers = new Set(current.consumers); consumers.delete(consumer); if (consumers.size > 0) { @@ -896,7 +937,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }); const deliverEvent = ( - eventKind: "state-change" | "recording-frame" | "pointer-event", + eventKind: "state-change" | "recording-frame" | "recording-input" | "pointer-event", tabId: string, delivery: () => Effect.Effect, ) => @@ -933,6 +974,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const update = Effect.fn("PreviewManager.update")(function* ( tabId: string, patch: Partial, + humanPoint?: { readonly x: number; readonly y: number }, ) { const updatedAt = yield* currentIso; const next = yield* SynchronizedRef.modify(tabsRef, (tabs) => { @@ -950,7 +992,20 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function // can commit between the modify above and here, and republishing this // snapshot would roll the UI back to a value that writer will not send // again because it suppresses unchanged audibility. - if (Option.isSome(next)) yield* emitIfCurrent(tabId, next.value); + if (Option.isSome(next)) { + if (patch.controller !== undefined && next.value.webContentsId != null) { + const capture = (yield* SynchronizedRef.get(frameCaptureSessionsRef)).get(tabId); + const webContentsId = next.value.webContentsId; + if (capture?.consumers.has("recording")) { + yield* attempt({ operation: "recording.controller", tabId }, () => { + const contents = webContents.fromId(webContentsId); + if (contents && !contents.isDestroyed()) + contents.send(RECORDING_CONTROLLER_CHANNEL, patch.controller, humanPoint); + }).pipe(Effect.ignore); + } + } + yield* emitIfCurrent(tabId, next.value); + } }); /** @@ -1728,6 +1783,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const sync = () => runFork(syncState(true)); const syncNavigation = () => runFork(syncState(false, true)); const syncInPageNavigation = () => runFork(syncState(false)); + const restoreRecordingCursor = () => + runFork( + Effect.gen(function* () { + const session = (yield* SynchronizedRef.get(frameCaptureSessionsRef)).get(tabId); + if (!wc.isDestroyed()) { + const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId); + wc.send( + RECORDING_CURSOR_CHANNEL, + session?.consumers.has("recording") ?? false, + session?.recordingInputOptions, + tab?.controller, + ); + } + }), + ); const navigationStarted = ( event: Electron.Event, ) => { @@ -1860,13 +1930,37 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function copy.set(tabId, (epochs.get(tabId) ?? 0) + 1); }), ); - yield* update(tabId, { controller: "human" }); + yield* update( + tabId, + { controller: "human" }, + isPreviewInputSignal(rawSignal) && rawSignal.kind === "pointer" + ? { x: rawSignal.x, y: rawSignal.y } + : undefined, + ); yield* Effect.sleep(750); const tabs = yield* SynchronizedRef.get(tabsRef); if (tabs.get(tabId)?.controller === "human") { yield* update(tabId, { controller: "none" }); } }); + const recordingInput = (_event: unknown, input: unknown) => { + if (!isRecordingInput(input)) return; + return runFork( + Effect.gen(function* () { + const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId); + const capture = (yield* SynchronizedRef.get(frameCaptureSessionsRef)).get(tabId); + if (tab?.webContentsId !== wc.id || !capture?.consumers.has("recording")) return; + if (input.type === "key" && !capture.recordingInputOptions?.showKeyPresses) return; + if (input.type === "pointer" && !capture.recordingInputOptions?.showMousePresses) return; + const listeners = yield* Ref.get(recordingInputListenersRef); + yield* Effect.forEach( + listeners, + (listener) => deliverEvent("recording-input", tabId, () => listener({ tabId, input })), + { discard: true }, + ); + }), + ); + }; const humanInput = (_event: unknown, rawSignal?: unknown): void => { runFork(handleHumanInput(rawSignal)); }; @@ -1928,11 +2022,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.off("page-favicon-updated", faviconUpdated as never); wc.off("did-start-loading", sync); wc.off("did-stop-loading", sync); + wc.off("dom-ready", restoreRecordingCursor); wc.off("did-fail-load", failed as never); wc.off("audio-state-changed", audioStateChanged); 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); }).pipe(Effect.ignore), ); @@ -1948,9 +2044,11 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.on("page-favicon-updated", faviconUpdated as never); wc.on("did-start-loading", sync); wc.on("did-stop-loading", sync); + wc.on("dom-ready", restoreRecordingCursor); wc.on("did-fail-load", failed as never); wc.on("audio-state-changed", audioStateChanged); wc.ipc.on(HUMAN_INPUT_CHANNEL, humanInput); + wc.ipc.on(RECORDING_INPUT_CHANNEL, recordingInput); wc.ipc.on(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); wc.setWindowOpenHandler((details) => { if (previewWindowOpenAction(details) === "popup") { @@ -3405,7 +3503,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }); }; - const startRecording = Effect.fn("PreviewManager.startRecording")(function* (tabId: string) { + const startRecording = Effect.fn("PreviewManager.startRecording")(function* ( + tabId: string, + options: RecordingInputOptions = DEFAULT_RECORDING_INPUT_OPTIONS, + ) { if ((yield* Ref.get(closingTabIdsRef)).has(tabId)) { return yield* new PreviewTabNotFoundError({ tabId }); } @@ -3413,11 +3514,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function tabId, Effect.gen(function* () { yield* startFrameCapture(tabId, "recording"); + yield* SynchronizedRef.update(frameCaptureSessionsRef, (sessions) => + replaceMap(sessions, (copy) => { + const current = copy.get(tabId); + if (current) copy.set(tabId, { ...current, recordingInputOptions: options }); + }), + ); const wc = yield* requireWebContents(tabId); const requestWebContents = wc.hostWebContents; if (requestWebContents === null) { return yield* new PreviewMainWindowClosedError({ tabId }); } + const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId); + yield* attempt({ operation: "recording.cursor", tabId, webContentsId: wc.id }, () => + wc.send(RECORDING_CURSOR_CHANNEL, true, options, tab?.controller), + ); yield* attemptPromise( { operation: "recording.warmSource", @@ -3720,6 +3831,15 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const emitPointerEvent = Effect.fn("PreviewManager.emitPointerEvent")(function* ( event: DesktopPreviewPointerEvent, ) { + const recording = (yield* SynchronizedRef.get(frameCaptureSessionsRef)).get(event.tabId); + const tab = (yield* SynchronizedRef.get(tabsRef)).get(event.tabId); + const webContentsId = tab?.webContentsId; + if (recording?.consumers.has("recording") && webContentsId != null) { + yield* attempt({ operation: "recording.pointer", tabId: event.tabId }, () => { + const contents = webContents.fromId(webContentsId); + if (contents && !contents.isDestroyed()) contents.send(RECORDING_POINTER_CHANNEL, event); + }); + } const listeners = yield* Ref.get(pointerEventListenersRef); yield* Effect.forEach( listeners, @@ -4158,6 +4278,18 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const keySequence = makePreviewAutomationNativeKeySequence(input, { isMac: hostPlatform === "darwin", }); + const recording = (yield* SynchronizedRef.get(frameCaptureSessionsRef)).get(tabId); + if (recording?.consumers.has("recording") && recording.recordingInputOptions?.showKeyPresses) { + yield* attempt({ operation: "recording.key", tabId, webContentsId: wc.id }, () => + wc.send(RECORDING_KEY_CHANNEL, { + key: keySequence.signal.key || input.key, + metaKey: input.modifiers?.includes("Meta") ?? false, + ctrlKey: input.modifiers?.includes("Control") ?? false, + altKey: input.modifiers?.includes("Alt") ?? false, + shiftKey: input.modifiers?.includes("Shift") ?? false, + }), + ); + } // CDP keyboard dispatch follows the embedder's focused renderer, and // WebContents.focus() is a no-op for webview guests. Native input targets // this guest's widget directly, so Enter cannot submit the host composer. @@ -4518,6 +4650,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function Ref.set(expectedAgentInputsRef, new Map()), Ref.set(pointerEventListenersRef, new Set()), Ref.set(recordingFrameListenersRef, new Set()), + Ref.set(recordingInputListenersRef, new Set()), ], { discard: true }, ); @@ -4561,6 +4694,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function stopRecording, subscribePointerEvents: (listener: PointerEventListener) => subscribe(pointerEventListenersRef, listener), + subscribeRecordingInputs: (listener: RecordingInputListener) => + subscribe(recordingInputListenersRef, listener), subscribeRecordingFrames: (listener: RecordingFrameListener) => subscribe(recordingFrameListenersRef, listener), subscribeStateChanges: (listener: Listener) => subscribe(listenersRef, listener), @@ -4925,7 +5060,10 @@ export class PreviewManager extends Context.Service< readonly copyArtifactToClipboard: (path: string) => Effect.Effect; readonly openPictureInPicture: (tabId: string) => Effect.Effect; readonly closePictureInPicture: (tabId: string) => Effect.Effect; - readonly startRecording: (tabId: string) => Effect.Effect; + readonly startRecording: ( + tabId: string, + options?: RecordingInputOptions, + ) => Effect.Effect; readonly stopRecording: (tabId: string) => Effect.Effect; readonly saveRecording: ( tabId: string, @@ -4966,6 +5104,9 @@ export class PreviewManager extends Context.Service< readonly subscribePointerEvents: ( listener: PointerEventListener, ) => Effect.Effect; + readonly subscribeRecordingInputs: ( + listener: RecordingInputListener, + ) => Effect.Effect; readonly subscribeRecordingFrames: ( listener: RecordingFrameListener, ) => Effect.Effect; @@ -5059,6 +5200,7 @@ export const make = Effect.gen(function* PreviewManagerMake() { subscribeStateChanges: operations.subscribeStateChanges, subscribePointerEvents: operations.subscribePointerEvents, subscribeRecordingFrames: operations.subscribeRecordingFrames, + 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 6155c4119ec8..78351e9d79c3 100644 --- a/apps/desktop/src/preview/PickPreload.ts +++ b/apps/desktop/src/preview/PickPreload.ts @@ -16,6 +16,8 @@ import type { import { resolveAnnotationSubmission } from "./AnnotationKeyboard.ts"; import { previewAnnotationStyles } from "./AnnotationStyles.generated.ts"; +import { installRecordingCursor } from "./RecordingCursor.ts"; +import { DEFAULT_RECORDING_INPUT_OPTIONS } from "./RecordingInput.ts"; import { ANNOTATION_CAPTURED_CHANNEL, ANNOTATION_THEME_CHANNEL, @@ -23,6 +25,11 @@ import { ELEMENT_PICKED_CHANNEL, HUMAN_INPUT_CHANNEL, MOUSE_NAVIGATE_CHANNEL, + RECORDING_CURSOR_CHANNEL, + RECORDING_POINTER_CHANNEL, + RECORDING_KEY_CHANNEL, + RECORDING_INPUT_CHANNEL, + RECORDING_CONTROLLER_CHANNEL, START_PICK_CHANNEL, } from "./GuestProtocol.ts"; const OVERLAY_ATTRIBUTE = "data-t3code-annotation-ui"; @@ -35,6 +42,80 @@ const ELEMENT_CONTEXT_TIMEOUT_MS = 5_000; const CONTENT_LAYER_Z_INDEX = 1; const CHROME_LAYER_Z_INDEX = 10; +let recordingCursor: ReturnType | null = null; +ipcRenderer.on( + RECORDING_CURSOR_CHANNEL, + (_event, active: unknown, inputOptions: unknown, controller: unknown) => { + if (active === true) { + const options = + typeof inputOptions === "object" && inputOptions !== null + ? { + showKeyPresses: + "showKeyPresses" in inputOptions && inputOptions.showKeyPresses === true, + showMousePresses: + "showMousePresses" in inputOptions && inputOptions.showMousePresses === true, + } + : DEFAULT_RECORDING_INPUT_OPTIONS; + recordingCursor ??= installRecordingCursor(document, window, options, (input) => + ipcRenderer.send(RECORDING_INPUT_CHANNEL, input), + ); + recordingCursor.setTheme(annotationTheme); + if (controller === "agent" || controller === "human" || controller === "none") + recordingCursor.setController(controller); + } else { + recordingCursor?.dispose(); + recordingCursor = null; + } + }, +); +ipcRenderer.on(RECORDING_CONTROLLER_CHANNEL, (_event, controller: unknown, point: unknown) => { + const humanPoint = + typeof point === "object" && + point !== null && + "x" in point && + typeof point.x === "number" && + Number.isFinite(point.x) && + "y" in point && + typeof point.y === "number" && + Number.isFinite(point.y) + ? { x: point.x, y: point.y } + : undefined; + if (controller === "agent" || controller === "human" || controller === "none") + recordingCursor?.setController(controller, humanPoint); +}); +ipcRenderer.on(RECORDING_KEY_CHANNEL, (_event, input: unknown) => { + if ( + typeof input !== "object" || + input === null || + !("key" in input) || + typeof input.key !== "string" + ) + return; + recordingCursor?.keyPress({ + key: input.key, + metaKey: "metaKey" in input && input.metaKey === true, + ctrlKey: "ctrlKey" in input && input.ctrlKey === true, + altKey: "altKey" in input && input.altKey === true, + shiftKey: "shiftKey" in input && input.shiftKey === true, + }); +}); +ipcRenderer.on(RECORDING_POINTER_CHANNEL, (_event, point: unknown) => { + if ( + typeof point === "object" && + point !== null && + "x" in point && + typeof point.x === "number" && + Number.isFinite(point.x) && + "y" in point && + typeof point.y === "number" && + Number.isFinite(point.y) + ) + recordingCursor?.move( + { x: point.x, y: point.y }, + "phase" in point && point.phase === "click" ? "click" : "move", + ); +}); + type AnnotationTool = "select" | "marquee" | "draw" | "erase"; interface SelectedElement { @@ -1361,6 +1442,7 @@ ipcRenderer.on(START_PICK_CHANNEL, (_event, theme: DesktopPreviewAnnotationTheme }); ipcRenderer.on(ANNOTATION_THEME_CHANNEL, (_event, theme: DesktopPreviewAnnotationTheme) => { annotationTheme = theme; + recordingCursor?.setTheme(theme); activeSession?.applyTheme(theme); }); ipcRenderer.on(CANCEL_PICK_CHANNEL, () => activeSession?.teardown(false)); diff --git a/apps/desktop/src/preview/RecordingCursor.ts b/apps/desktop/src/preview/RecordingCursor.ts new file mode 100644 index 000000000000..70b0a43be43a --- /dev/null +++ b/apps/desktop/src/preview/RecordingCursor.ts @@ -0,0 +1,197 @@ +import type { + DesktopPreviewAnnotationTheme, + DesktopPreviewRecordingInput, +} from "@t3tools/contracts"; + +import { + DEFAULT_RECORDING_INPUT_OPTIONS, + recordingKeyLabel, + recordingKeysAreSensitive, + type RecordingInputOptions, + type RecordingKeyPress, +} from "./RecordingInput.ts"; + +/** + * Chromium's capture cursor uses native window bounds, which do not follow a + * webview's CSS placement or scale. Draw it in the guest's coordinate space + * while recording, and make the native cursor transparent to avoid two cursors. + */ +export function installRecordingCursor( + document: Document, + window: Window, + options: RecordingInputOptions = DEFAULT_RECORDING_INPUT_OPTIONS, + emit: (input: DesktopPreviewRecordingInput) => void = () => {}, +) { + const style = document.createElement("style"); + style.textContent = + "html, html * { cursor: none !important; } @media (prefers-reduced-motion: reduce) { [data-t3code-recording-agent-cursor] { transition: none !important; } }"; + const cursor = document.createElement("div"); + cursor.setAttribute("aria-hidden", "true"); + cursor.setAttribute("data-t3code-recording-cursor", ""); + cursor.style.cssText = + "position:fixed;left:0;top:0;width:16px;height:24px;pointer-events:none;z-index:2147483647;display:none;"; + cursor.innerHTML = + ''; + const agentCursor = document.createElement("div"); + agentCursor.setAttribute("aria-hidden", "true"); + agentCursor.setAttribute("data-t3code-recording-agent-cursor", ""); + agentCursor.style.cssText = + "position:fixed;left:0;top:0;width:20px;height:20px;pointer-events:none;z-index:2147483647;display:none;filter:drop-shadow(0 1px 2px #0003);transition:transform 150ms ease-out,opacity 150ms ease-out;"; + // Match the MousePointer2 icon used by the live AgentBrowserCursor. + agentCursor.innerHTML = + ''; + document.documentElement.append(style, cursor, agentCursor); + let controller: "human" | "agent" | "none" = "none"; + let humanPoint: { readonly x: number; readonly y: number } | null = null; + const drawHuman = () => { + if (!humanPoint) return; + cursor.style.transform = `translate(${humanPoint.x}px, ${humanPoint.y}px)`; + cursor.style.display = "block"; + }; + let agentActive = false; + let agentTimer: number | undefined; + const setController = ( + next: typeof controller, + point?: { readonly x: number; readonly y: number }, + ) => { + if (point) humanPoint = point; + controller = next; + if (next === "agent") cursor.style.display = "none"; + if (next === "human") drawHuman(); + if (!agentActive) agentCursor.style.opacity = next === "human" ? "0.18" : "0.35"; + }; + const setTheme = ( + theme: Pick | null, + ) => { + agentCursor.style.setProperty("--recording-cursor-primary", theme?.primary ?? "#2563eb"); + agentCursor.style.setProperty("--recording-cursor-background", theme?.background ?? "white"); + }; + let lastKeyLabel: string | null = null; + let pointerHeld = false; + let pointerFrame: number | undefined; + let pendingPointer: DesktopPreviewRecordingInput | undefined; + const cancelPendingPointer = () => { + if (pointerFrame !== undefined) window.cancelAnimationFrame(pointerFrame); + pointerFrame = undefined; + pendingPointer = undefined; + }; + const keyPress = (input: RecordingKeyPress, held = false) => { + if (!options.showKeyPresses) return; + const label = recordingKeysAreSensitive(document) + ? null + : recordingKeyLabel(input, /Mac/.test(window.navigator.platform)); + lastKeyLabel = label; + emit({ type: "key", label, held, width: window.innerWidth }); + }; + const pointer = ( + point: { readonly x: number; readonly y: number }, + phase: "move" | "down" | "up" | "click", + ) => { + if (!options.showMousePresses || (phase === "move" && !pointerHeld)) return; + if (phase === "down") pointerHeld = true; + if (phase === "up") pointerHeld = false; + const input: DesktopPreviewRecordingInput = { + type: "pointer", + phase, + ...point, + width: window.innerWidth, + height: window.innerHeight, + }; + if (phase === "move") { + pendingPointer = input; + pointerFrame ??= window.requestAnimationFrame(() => { + pointerFrame = undefined; + if (pendingPointer) emit(pendingPointer); + pendingPointer = undefined; + }); + } else { + cancelPendingPointer(); + emit(input); + } + }; + const move = ( + point: { readonly x: number; readonly y: number }, + phase: "move" | "click" = "move", + ) => { + agentCursor.style.transform = `translate(${point.x}px, ${point.y}px)`; + agentCursor.style.display = "block"; + agentCursor.style.opacity = "1"; + agentActive = true; + window.clearTimeout(agentTimer); + agentTimer = window.setTimeout(() => { + agentActive = false; + agentCursor.style.opacity = controller === "human" ? "0.18" : "0.35"; + }, 700); + pointer(point, phase); + }; + const moveHuman = (point: { readonly x: number; readonly y: number }) => { + if (controller === "agent") return; + humanPoint = point; + drawHuman(); + }; + const pointerMove = (event: PointerEvent) => { + if (event.pointerType === "touch") return; + const point = { x: event.clientX, y: event.clientY }; + moveHuman(point); + pointer(point, "move"); + }; + const pointerDown = (event: PointerEvent) => { + if (event.pointerType === "touch") return; + moveHuman({ x: event.clientX, y: event.clientY }); + pointer({ x: event.clientX, y: event.clientY }, "down"); + }; + const pointerUp = (event: PointerEvent) => { + if (event.pointerType !== "touch") pointer({ x: event.clientX, y: event.clientY }, "up"); + }; + const keyDown = (event: KeyboardEvent) => { + if (event.isComposing || event.repeat) return; + keyPress(event, true); + }; + const keyUp = () => { + if (!options.showKeyPresses) return; + emit({ + type: "key", + label: recordingKeysAreSensitive(document) ? null : lastKeyLabel, + held: false, + width: window.innerWidth, + }); + }; + const hide = () => { + cursor.style.display = "none"; + pointerHeld = false; + cancelPendingPointer(); + if (options.showKeyPresses || options.showMousePresses) emit({ type: "clear" }); + }; + const leave = (event: PointerEvent) => { + if (event.relatedTarget === null) hide(); + }; + window.addEventListener("pointermove", pointerMove, true); + window.addEventListener("pointerdown", pointerDown, true); + window.addEventListener("pointerup", pointerUp, true); + window.addEventListener("pointercancel", pointerUp, true); + window.addEventListener("keydown", keyDown, true); + window.addEventListener("keyup", keyUp, true); + window.addEventListener("pointerout", leave, true); + window.addEventListener("blur", hide); + return { + move, + keyPress, + setController, + setTheme, + dispose: () => { + window.removeEventListener("pointermove", pointerMove, true); + window.removeEventListener("pointerdown", pointerDown, true); + window.removeEventListener("pointerup", pointerUp, true); + window.removeEventListener("pointercancel", pointerUp, true); + window.removeEventListener("keydown", keyDown, true); + window.removeEventListener("keyup", keyUp, true); + window.removeEventListener("pointerout", leave, true); + window.removeEventListener("blur", hide); + cancelPendingPointer(); + window.clearTimeout(agentTimer); + cursor.remove(); + agentCursor.remove(); + style.remove(); + }, + }; +} diff --git a/apps/desktop/src/preview/RecordingInput.test.ts b/apps/desktop/src/preview/RecordingInput.test.ts new file mode 100644 index 000000000000..062685ec2934 --- /dev/null +++ b/apps/desktop/src/preview/RecordingInput.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vite-plus/test"; +import { recordingKeyLabel, recordingKeysAreSensitive } from "./RecordingInput.ts"; + +const key = ( + value: string, + modifiers: Partial<{ + metaKey: boolean; + ctrlKey: boolean; + altKey: boolean; + shiftKey: boolean; + }> = {}, +) => ({ + key: value, + metaKey: false, + ctrlKey: false, + altKey: false, + shiftKey: false, + ...modifiers, +}); + +describe("recording key labels", () => { + it("formats macOS and other-platform shortcuts", () => { + expect(recordingKeyLabel(key("c", { metaKey: true }), true)).toBe("⌘C"); + expect(recordingKeyLabel(key("c", { ctrlKey: true }), false)).toBe("Ctrl + C"); + expect(recordingKeyLabel(key("Tab", { altKey: true, shiftKey: true }), true)).toBe("⌥⇧⇥"); + }); + it("shows held modifiers once and labels navigation keys", () => { + expect(recordingKeyLabel(key("Meta", { metaKey: true }), true)).toBe("⌘"); + expect(recordingKeyLabel(key("Shift", { shiftKey: true }), false)).toBe("Shift"); + expect(recordingKeyLabel(key("ArrowLeft"), true)).toBe("←"); + expect(recordingKeyLabel(key(" "), false)).toBe("Space"); + }); + it.each(["Dead", "Unidentified", "Process", ""])("excludes composition key %s", (value) => { + expect(recordingKeyLabel(key(value), true)).toBeNull(); + }); +}); + +describe("recording key privacy", () => { + const field = (type: string) => ({ tagName: "INPUT", getAttribute: () => type }); + const sensitive = (activeElement: unknown) => + recordingKeysAreSensitive({ activeElement } as Document); + it("excludes password fields and their shadow-root focus", () => { + expect(sensitive(field("password"))).toBe(true); + expect(sensitive({ shadowRoot: { activeElement: field("password") } })).toBe(true); + expect(sensitive(field("text"))).toBe(false); + }); + it("excludes iframe focus whose field cannot be inspected", () => { + expect(sensitive({ tagName: "IFRAME" })).toBe(true); + expect(sensitive({ tagName: "SECRET-FIELD" })).toBe(true); + }); +}); diff --git a/apps/desktop/src/preview/RecordingInput.ts b/apps/desktop/src/preview/RecordingInput.ts new file mode 100644 index 000000000000..a69102121d4e --- /dev/null +++ b/apps/desktop/src/preview/RecordingInput.ts @@ -0,0 +1,58 @@ +export interface RecordingInputOptions { + readonly showKeyPresses: boolean; + readonly showMousePresses: boolean; +} + +export const DEFAULT_RECORDING_INPUT_OPTIONS: RecordingInputOptions = { + showKeyPresses: false, + showMousePresses: false, +}; + +export interface RecordingKeyPress { + readonly key: string; + readonly metaKey: boolean; + readonly ctrlKey: boolean; + readonly altKey: boolean; + readonly shiftKey: boolean; +} + +/** Formats a single chord without duplicating a modifier pressed on its own. */ +export function recordingKeyLabel(input: RecordingKeyPress, isMac: boolean): string | null { + if (["Dead", "Process", "Unidentified", ""].includes(input.key)) return null; + const modifiers = [ + input.ctrlKey || input.key === "Control" ? (isMac ? "⌃" : "Ctrl") : null, + input.altKey || input.key === "Alt" ? (isMac ? "⌥" : "Alt") : null, + input.shiftKey || input.key === "Shift" ? (isMac ? "⇧" : "Shift") : null, + input.metaKey || input.key === "Meta" ? (isMac ? "⌘" : "Win") : null, + ].filter((value) => value !== null); + const labels: Record = { + Enter: "↵", + Tab: "⇥", + Backspace: "⌫", + Delete: "⌦", + Escape: "Esc", + ArrowUp: "↑", + ArrowDown: "↓", + ArrowLeft: "←", + ArrowRight: "→", + " ": "Space", + Space: "Space", + }; + if (!["Control", "Alt", "Shift", "Meta"].includes(input.key)) { + modifiers.push( + labels[input.key] ?? (input.key.length === 1 ? input.key.toUpperCase() : input.key), + ); + } + return modifiers.join(isMac ? "" : " + "); +} + +/** Unknown iframe or closed-shadow focus is excluded because its field type cannot be checked. */ +export function recordingKeysAreSensitive(document: Document): boolean { + let element = document.activeElement; + while (element?.shadowRoot?.activeElement) element = element.shadowRoot.activeElement; + return ( + element?.tagName === "IFRAME" || + element?.tagName.includes("-") === true || + element?.getAttribute("type")?.toLowerCase() === "password" + ); +} diff --git a/apps/mobile/modules/t3-review-diff/android/build.gradle b/apps/mobile/modules/t3-review-diff/android/build.gradle index 22bb070b3b81..d360d1580f1d 100644 --- a/apps/mobile/modules/t3-review-diff/android/build.gradle +++ b/apps/mobile/modules/t3-review-diff/android/build.gradle @@ -8,6 +8,10 @@ android { namespace 'expo.modules.t3reviewdiff' compileSdk rootProject.ext.compileSdkVersion + testOptions { + unitTests.includeAndroidResources = true + } + defaultConfig { minSdkVersion rootProject.ext.minSdkVersion targetSdkVersion rootProject.ext.targetSdkVersion @@ -16,4 +20,12 @@ android { dependencies { implementation project(':expo-modules-core') + testImplementation 'junit:junit:4.13.2' + testImplementation 'org.robolectric:robolectric:4.16.1' +} + +tasks.withType(Test).configureEach { + javaLauncher = javaToolchains.launcherFor { + languageVersion = JavaLanguageVersion.of(21) + } } diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt index 6782e6894d99..80d0410643ff 100644 --- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt +++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCanvasDrawing.kt @@ -9,6 +9,7 @@ import android.graphics.Path import android.graphics.RectF import android.graphics.Shader import android.graphics.Typeface +import android.text.TextPaint import kotlin.math.max import kotlin.math.min @@ -177,6 +178,39 @@ internal class ReviewDiffCanvasDrawing(context: Context) { textPaint.isUnderlineText = fontStyle and 4 != 0 } + var codeLayouts = CodeLayoutCache() + + /** Capture paint on the UI thread; the decode worker owns the new cache until publication. */ + fun prepareRows( + tokens: Map>, + style: DiffStyle, + width: Int + ): (List) -> CodeLayoutCache { + configureCodePaint(theme.text, 0, style) + val paint = TextPaint(textPaint) + val colors = theme + val cache = codeLayouts.copyForPreparation() + val availableWidth = ( + width - style.changeBarWidthPx - style.gutterWidthPx - + style.codePaddingPx * 2f + ).toInt() + return { rows -> + cache.apply { layout(rows, tokens, paint, style, colors, availableWidth) } + } + } + + fun codeWrapLayout( + rows: List, + tokens: Map>, + style: DiffStyle, + width: Int + ): CodeWrapLayout { + configureCodePaint(theme.text, 0, style) + val availableWidth = width - style.changeBarWidthPx - style.gutterWidthPx - + style.codePaddingPx * 2f + return codeLayouts.layout(rows, tokens, textPaint, style, theme, availableWidth.toInt()) + } + fun lineNumberColor(change: String): Int = when (change) { "add" -> theme.addText "delete" -> theme.deleteText @@ -198,13 +232,17 @@ internal class ReviewDiffCanvasDrawing(context: Context) { } } + /** Highlights word diffs; [top]..[bottom] is the row's first visual line. */ + @Suppress("LongParameterList") fun drawWordDiffRanges( canvas: Canvas, row: DiffRow, codeX: Float, top: Int, - bottom: Int + bottom: Int, + lines: CodeLines ) { + if (lines.nativeLayout != null) return if (row.wordDiffRanges.isEmpty() || (row.change != "add" && row.change != "delete")) return val color = if (row.change == "add") theme.addBar else theme.deleteBar backgroundPaint.color = withAlpha(color, 71) @@ -213,14 +251,66 @@ internal class ReviewDiffCanvasDrawing(context: Context) { val highlightHeight = max(4f * density, min(bottom - top - 4f * density, fontHeight)) val highlightTop = (top + bottom - highlightHeight) / 2f row.wordDiffRanges.forEach { range -> - val left = codeX + range.start * characterWidth - val right = max(left + 2f * density, codeX + range.end * characterWidth) - canvas.drawRoundRect( - RectF(left, highlightTop, right, highlightTop + highlightHeight), - 3f * density, - 3f * density, - backgroundPaint, - ) + // A wrapped row splits the highlight at each visual line boundary. + lines.starts.forEachIndexed { line, lineStart -> + val start = max(range.start, lineStart) + val end = min(range.end, lines.end(line, Int.MAX_VALUE)) + if (end <= start) return@forEachIndexed + val left = codeX + (start - lineStart) * characterWidth + val right = max(left + 2f * density, left + (end - start) * characterWidth) + val lineTop = highlightTop + line * lines.height + canvas.drawRoundRect( + RectF(left, lineTop, right, lineTop + highlightHeight), + 3f * density, + 3f * density, + backgroundPaint, + ) + } + } + } + + /** Draws a code row's text, or its syntax [tokens] when present, one visual line per start. */ + @Suppress("LongParameterList") + fun drawCode( + canvas: Canvas, + content: String, + tokens: List?, + codeX: Float, + baseline: Float, + style: DiffStyle, + lines: CodeLines + ) { + val nativeLayout = lines.nativeLayout + if (nativeLayout != null) { + canvas.save() + canvas.translate(codeX, baseline - nativeLayout.getLineBaseline(0)) + nativeLayout.draw(canvas) + canvas.restore() + return + } + val runs = if (tokens.isNullOrEmpty()) listOf(DiffToken(content, null, 0)) else tokens + var line = 0 + var x = codeX + var column = 0 + runs.forEach { run -> + configureCodePaint(run.color ?: theme.text, run.fontStyle, style) + var start = 0 + while (start < run.content.length) { + while (line + 1 < lines.starts.size && lines.starts[line + 1] <= column + start) { + line += 1 + x = codeX + } + val end = min(run.content.length, lines.end(line, Int.MAX_VALUE) - column) + val lineBaseline = baseline + line * lines.height + if (lineBaseline + textPaint.fontMetrics.descent >= canvas.clipBounds.top && + lineBaseline + textPaint.fontMetrics.ascent <= canvas.clipBounds.bottom + ) { + canvas.drawText(run.content, start, end, x, lineBaseline, textPaint) + x += textPaint.measureText(run.content, start, end) + } + start = end + } + column += run.content.length } } diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayout.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayout.kt new file mode 100644 index 000000000000..dc77a8467033 --- /dev/null +++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayout.kt @@ -0,0 +1,169 @@ +package expo.modules.t3reviewdiff + +import android.graphics.Color +import android.graphics.Paint +import android.graphics.Typeface +import android.text.Layout +import android.text.SpannableString +import android.text.Spanned +import android.text.StaticLayout +import android.text.TextPaint +import android.text.style.BackgroundColorSpan +import android.text.style.ForegroundColorSpan +import android.text.style.StyleSpan +import android.text.style.UnderlineSpan +import kotlin.math.ceil +import kotlin.math.max + +/** Text layout is independent of comment heights and vertical row offsets. */ +internal class CodeLines( + val starts: IntArray, + val height: Int, + val nativeLayout: StaticLayout? = null +) { + fun end(line: Int, length: Int): Int = if (line + 1 < starts.size) starts[line + 1] else length + + fun firstHeight(base: Int): Int = max(base, nativeLayout?.getLineBottom(0) ?: 0) + + fun baseline(top: Int, bottom: Int, paint: Paint): Float = nativeLayout?.let { + top + (bottom - top - it.getLineBottom(0)) / 2f + it.getLineBaseline(0) + } ?: ((top + bottom - paint.fontMetrics.ascent - paint.fontMetrics.descent) / 2f) + + val extraHeight: Int + get() = nativeLayout?.let { it.height - it.getLineBottom(0) } ?: ((starts.size - 1) * height) +} + +internal class CodeWrapLayout( + val enabled: Boolean, + private val linesByRowId: Map +) { + fun lines(rowId: String): CodeLines = linesByRowId[rowId] ?: SINGLE_LINE + fun extraHeight(rowId: String): Int = lines(rowId).extraHeight + fun rowHeight(rowId: String, base: Int): Int = lines(rowId).let { + it.firstHeight(base) + + it.extraHeight + } + + companion object { + private val SINGLE_LINE = CodeLines(intArrayOf(0), 0) + val NONE = CodeWrapLayout(false, emptyMap()) + } +} + +/** ASCII is fixed-pitch; other text needs the same shaping for measurement and drawing. */ +internal fun createCodeLines(text: CharSequence, paint: TextPaint, width: Int): CodeLines { + val characterWidth = paint.measureText("M") + val lineHeight = ceil(paint.fontMetrics.run { descent - ascent }).toInt() + if (text.all { it in ' '..'~' }) { + val columns = max(1, (width / characterWidth).toInt()) + return CodeLines( + IntArray(max(1, (text.length + columns - 1) / columns)) { + it * columns + }, + lineHeight + ) + } + val layout = StaticLayout.Builder.obtain(text, 0, text.length, paint, max(1, width)) + .setAlignment(Layout.Alignment.ALIGN_NORMAL) + .setIncludePad(false) + .setBreakStrategy(Layout.BREAK_STRATEGY_SIMPLE) + .setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NONE) + .build() + return CodeLines(IntArray(layout.lineCount) { layout.getLineStart(it) }, lineHeight, layout) +} + +internal class CodeLayoutCache { + private data class Entry(val row: DiffRow, val tokens: List?, val lines: CodeLines) + private var entries = emptyMap() + private var previousStyle: DiffStyle? = null + private var previousTheme: DiffTheme? = null + private var previousWidth = 0 + + /** Entries are immutable; a worker can reuse them without changing the displayed cache. */ + fun copyForPreparation(): CodeLayoutCache = CodeLayoutCache().also { + it.entries = entries + it.previousStyle = previousStyle + it.previousTheme = previousTheme + it.previousWidth = previousWidth + } + + @Suppress("LongParameterList") + fun layout( + rows: List, + tokens: Map>, + paint: Paint, + style: DiffStyle, + theme: DiffTheme, + width: Int + ): CodeWrapLayout { + if (!style.wordWrap || width < paint.measureText("M")) { + entries = emptyMap() + return CodeWrapLayout.NONE + } + if (previousStyle != style || previousTheme != theme || previousWidth != width) { + entries = emptyMap() + previousStyle = style + previousTheme = theme + previousWidth = width + } + val next = HashMap() + val layouts = HashMap() + for (row in rows) { + if (row.kind != "line") continue + val rowTokens = tokens[row.id] + val cached = entries[row.id] + val entry = if (cached?.row == row && cached.tokens == rowTokens) { + cached + } else { + val text = styledCode(row, rowTokens, theme) + Entry(row, rowTokens, createCodeLines(text, TextPaint(paint), width)) + } + next[row.id] = entry + layouts[row.id] = entry.lines + } + entries = next + return CodeWrapLayout(true, layouts) + } + + private fun styledCode(row: DiffRow, tokens: List?, theme: DiffTheme): CharSequence { + // The ASCII path uses the existing token drawing and rounded highlight rectangles. + if (row.content.all { it in ' '..'~' }) return row.content + val text = SpannableString(row.content) + var offset = 0 + for (token in tokens.orEmpty()) { + val end = (offset + token.content.length).coerceAtMost(text.length) + if (end > offset) { + token.color?.let { + text.setSpan(ForegroundColorSpan(it), offset, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) + } + val fontStyle = (if (token.fontStyle and 2 != 0) Typeface.BOLD else 0) or + (if (token.fontStyle and 1 != 0) Typeface.ITALIC else 0) + if (fontStyle != + 0 + ) { + text.setSpan(StyleSpan(fontStyle), offset, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) + } + if (token.fontStyle and 4 != + 0 + ) { + text.setSpan(UnderlineSpan(), offset, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) + } + } + offset = end + } + if (row.change == "add" || row.change == "delete") { + val bar = if (row.change == "add") theme.addBar else theme.deleteBar + val color = Color.argb(71, Color.red(bar), Color.green(bar), Color.blue(bar)) + for (range in row.wordDiffRanges) { + val start = range.start.coerceIn(0, text.length) + val end = range.end.coerceIn(start, text.length) + if (end > + start + ) { + text.setSpan(BackgroundColorSpan(color), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE) + } + } + } + return text + } +} diff --git a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt index 97e9f696db90..37fee1cb3613 100644 --- a/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt +++ b/apps/mobile/modules/t3-review-diff/android/src/main/java/expo/modules/t3reviewdiff/T3ReviewDiffView.kt @@ -143,10 +143,13 @@ class T3ReviewDiffView(context: Context, appContext: AppContext) : ExpoView(cont fun setRowsJson(value: String) { rowsDecodeGeneration += 1 val generation = rowsDecodeGeneration + val prepareLayout = canvasView.prepareRows() payloadDecodeExecutor.execute { val decodedRows = parseRows(value) + val codeLayouts = prepareLayout(decodedRows) post { if (generation != rowsDecodeGeneration) return@post + canvasView.useCodeLayouts(codeLayouts) rows = decodedRows lastVisibleFileId = null rebuildVisibleRows() @@ -460,7 +463,7 @@ internal data class DiffWordDiffRange( val end: Int ) -private data class DiffToken( +internal data class DiffToken( val content: String, val color: Int?, val fontStyle: Int @@ -543,6 +546,7 @@ internal data class DiffTheme( } internal data class DiffStyle( + val wordWrap: Boolean, val rowHeightPx: Float, val gutterWidthPx: Float, val codePaddingPx: Float, @@ -564,6 +568,7 @@ internal data class DiffStyle( ) { companion object { fun defaults(density: Float): DiffStyle = DiffStyle( + wordWrap = false, rowHeightPx = 20f * density, gutterWidthPx = 72f * density, codePaddingPx = 10f * density, @@ -587,6 +592,7 @@ internal data class DiffStyle( fun fromJson(value: String, fallback: DiffStyle, density: Float): DiffStyle = try { val json = JSONObject(value) DiffStyle( + wordWrap = json.optBoolean("wordWrap", fallback.wordWrap), rowHeightPx = json.floatDp("rowHeight", fallback.rowHeightPx, density), gutterWidthPx = json.floatDp("gutterWidth", fallback.gutterWidthPx, density), codePaddingPx = json.floatDp("codePadding", fallback.codePaddingPx, density), @@ -684,6 +690,8 @@ private class DiffCanvasView(context: Context) : View(context) { }, ) private var rowOffsets = intArrayOf(0) + + private var codeWrap = CodeWrapLayout.NONE private var verticalOffset = 0 private var horizontalOffset = 0 private val headerPathOffsetsByFileId = mutableMapOf() @@ -699,6 +707,7 @@ private class DiffCanvasView(context: Context) : View(context) { var tokensByRowId: Map> = emptyMap() set(value) { field = value + if (style.wordWrap) rebuildOffsets() invalidate() } var viewedFileIds: Set = emptySet() @@ -725,6 +734,7 @@ private class DiffCanvasView(context: Context) : View(context) { set(value) { field = value drawing.theme = value + if (style.wordWrap) rebuildOffsets() invalidate() } var style: DiffStyle = DiffStyle.defaults(density) @@ -742,6 +752,11 @@ private class DiffCanvasView(context: Context) : View(context) { var onRowTap: ((DiffRow, String, RowTapTarget) -> Unit)? = null var onVisibleRowsChanged: ((Int, Int) -> Unit)? = null + fun prepareRows() = drawing.prepareRows(tokensByRowId, style, width) + fun useCodeLayouts(layouts: CodeLayoutCache) { + drawing.codeLayouts = layouts + } + override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { setMeasuredDimension( MeasureSpec.getSize(widthMeasureSpec), @@ -751,6 +766,8 @@ private class DiffCanvasView(context: Context) : View(context) { override fun onSizeChanged(width: Int, height: Int, oldWidth: Int, oldHeight: Int) { super.onSizeChanged(width, height, oldWidth, oldHeight) + // Wrapped rows take their height from the width, so a new width is a new layout. + if (style.wordWrap && width != oldWidth) layoutRows() setVerticalOffset(verticalOffset) setHorizontalOffset(horizontalOffset) clampHeaderPathOffsets() @@ -818,7 +835,8 @@ private class DiffCanvasView(context: Context) : View(context) { fun horizontalOffset(): Int = horizontalOffset - fun maxHorizontalOffset(): Int = max(0, contentWidthPx - width) + fun maxHorizontalOffset(): Int = + if (codeWrap.enabled) 0 else max(0, contentWidthPx - width) fun maxHorizontalOffset(target: HorizontalPanTarget): Int = if (target.kind == HorizontalPanKind.FILE_HEADER_PATH) { @@ -844,14 +862,20 @@ private class DiffCanvasView(context: Context) : View(context) { } private fun rebuildOffsets() { + layoutRows() + requestLayout() + invalidate() + } + + private fun layoutRows() { + codeWrap = drawing.codeWrapLayout(rows, tokensByRowId, style, width) rowOffsets = IntArray(rows.size + 1) rows.forEachIndexed { index, row -> rowOffsets[index + 1] = rowOffsets[index] + rowHeight(row) } setVerticalOffset(verticalOffset) + setHorizontalOffset(horizontalOffset) clampHeaderPathOffsets() - requestLayout() - invalidate() } private fun rowHeight(row: DiffRow): Int = when (row.kind) { @@ -862,6 +886,7 @@ private class DiffCanvasView(context: Context) : View(context) { } else { (124 * density).toInt() } + "line" -> codeWrap.rowHeight(row.id, style.rowHeightPx.toInt()) else -> style.rowHeightPx.toInt() }.coerceAtLeast(1) @@ -1191,23 +1216,17 @@ private class DiffCanvasView(context: Context) : View(context) { ) } - val tokens = tokensByRowId[row.id] + // Wrapped rows keep the line number and first code line in the first row-height band. + val lines = codeWrap.lines(row.id) + val firstLineBottom = top + lines.firstHeight(style.rowHeightPx.toInt()) drawScrollableCode(canvas, top, bottom) { codeX -> drawing.configureCodePaint(theme.text, 0, style) - drawing.drawWordDiffRanges(canvas, row, codeX, top, bottom) - if (tokens.isNullOrEmpty()) { - canvas.drawText(row.content, codeX, centeredBaseline(top, bottom, textPaint), textPaint) - } else { - var x = codeX - tokens.forEach { token -> - drawing.configureCodePaint(token.color ?: theme.text, token.fontStyle, style) - canvas.drawText(token.content, x, centeredBaseline(top, bottom, textPaint), textPaint) - x += textPaint.measureText(token.content) - } - } + drawing.drawWordDiffRanges(canvas, row, codeX, top, firstLineBottom, lines) + val baseline = lines.baseline(top, firstLineBottom, textPaint) + drawing.drawCode(canvas, row.content, tokensByRowId[row.id], codeX, baseline, style, lines) } - drawLineNumber(canvas, row, top, bottom) + drawLineNumber(canvas, row, top, firstLineBottom) } private fun drawLineNumber(canvas: Canvas, row: DiffRow, top: Int, bottom: Int) { diff --git a/apps/mobile/modules/t3-review-diff/android/src/test/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayoutTest.kt b/apps/mobile/modules/t3-review-diff/android/src/test/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayoutTest.kt new file mode 100644 index 000000000000..1f4234b7175f --- /dev/null +++ b/apps/mobile/modules/t3-review-diff/android/src/test/java/expo/modules/t3reviewdiff/ReviewDiffCodeLayoutTest.kt @@ -0,0 +1,150 @@ +package expo.modules.t3reviewdiff + +import android.graphics.Bitmap +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.Typeface +import android.text.Spanned +import android.text.TextPaint +import android.text.style.BackgroundColorSpan +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotSame +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [35], manifest = Config.NONE) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class ReviewDiffCodeLayoutTest { + private val paint = TextPaint().apply { + color = Color.WHITE + textSize = 24f + typeface = Typeface.MONOSPACE + } + + @Test + fun unicodeAndTabsFitWithoutSplittingClusters() { + val fixtures = listOf("漢字表示", "e\u0301", "👨‍👩‍👧‍👦", "مرحبا بالعالم ", "\tvalue ") + for (fixture in fixtures) { + val text = fixture.repeat(40) + for (width in listOf(180, 280, 420)) { + val layout = requireNotNull(createCodeLines(text, paint, width).nativeLayout) + assertInkFits(layout, width, fixture) + assertLinesFit(layout, fixture, width) + } + } + } + + private fun assertLinesFit(layout: android.text.StaticLayout, fixture: String, width: Int) { + val text = layout.text + for (line in 0 until layout.lineCount) { + if (!fixture.contains('\t')) { + assertTrue("$fixture line $line at $width", layout.getLineMax(line) <= width + 1) + } + val start = layout.getLineStart(line) + assertTrue(start == 0 || !Character.isLowSurrogate(text[start])) + if (fixture == "👨‍👩‍👧‍👦" || fixture == "e\u0301") { + assertEquals(0, start % fixture.length) + } + } + } + + private fun assertInkFits(layout: android.text.StaticLayout, width: Int, fixture: String) { + val bitmap = Bitmap.createBitmap(width + 40, layout.height, Bitmap.Config.ARGB_8888) + layout.draw(Canvas(bitmap)) + for (x in width + 1 until bitmap.width) { + for (y in 0 until bitmap.height) { + assertEquals("$fixture ink outside width $width", 0, Color.alpha(bitmap.getPixel(x, y))) + } + } + bitmap.recycle() + } + + @Test + fun asciiSegmentsCoverTheWholeLineAndFit() { + val text = "const value = 123; ".repeat(100) + val lines = createCodeLines(text, paint, 280) + val pieces = lines.starts.indices.map { + text.substring(lines.starts[it], lines.end(it, text.length)) + } + assertEquals(text, pieces.joinToString("")) + assertTrue(pieces.all { paint.measureText(it) <= 280 }) + } + + @Test + fun changingCommentHeightReusesCodeButWidthAndContentInvalidateIt() { + val cache = CodeLayoutCache() + val row = row("漢字".repeat(100)) + val comment = row.copy(kind = "comment", id = "comment", content = "", commentText = "Before") + val style = DiffStyle.defaults(1f).copy(wordWrap = true) + val theme = DiffTheme.fallback("light") + val first = cache.layout( + listOf(row, comment), + emptyMap(), + paint, + style, + theme, + 280 + ).lines(row.id) + val second = cache.layout( + listOf(row, comment.copy(commentText = "After")), + emptyMap(), + paint, + style, + theme, + 280, + ).lines(row.id) + assertSame(first, second) + val narrow = cache.layout(listOf(row), emptyMap(), paint, style, theme, 180).lines(row.id) + assertNotSame(first, narrow) + assertTrue(narrow.extraHeight > first.extraHeight) + val edited = cache.layout( + listOf(row.copy(content = "短い")), + emptyMap(), + paint, + style, + theme, + 180 + ).lines(row.id) + assertTrue(edited.extraHeight < narrow.extraHeight) + assertEquals( + 0, + cache.layout( + listOf(row), + emptyMap(), + paint, + style.copy(wordWrap = false), + theme, + 180 + ).extraHeight(row.id) + ) + } + + @Test + fun highlightsUseNativeTextRangesAndSurviveSyntaxArrival() { + val cache = CodeLayoutCache() + val row = row("漢字".repeat(30)).copy(wordDiffRanges = listOf(DiffWordDiffRange(3, 21))) + val style = DiffStyle.defaults(1f).copy(wordWrap = true) + val theme = DiffTheme.fallback("light") + val initial = cache.layout(listOf(row), emptyMap(), paint, style, theme, 180).lines(row.id) + val tokens = mapOf(row.id to listOf(DiffToken(row.content, 0xff008800.toInt(), 2))) + val highlighted = cache.layout(listOf(row), tokens, paint, style, theme, 180).lines(row.id) + assertNotSame(initial, highlighted) + val text = requireNotNull(highlighted.nativeLayout).text as Spanned + val span = text.getSpans(0, text.length, BackgroundColorSpan::class.java).single() + assertEquals(3, text.getSpanStart(span)) + assertEquals(21, text.getSpanEnd(span)) + } + + private fun row(content: String) = DiffRow( + kind = "line", id = "line", fileId = "file", filePath = "test.ts", previousPath = null, + changeType = "modified", additions = 1, deletions = 0, text = "", content = content, + change = "add", oldLineNumber = null, newLineNumber = 1, wordDiffRanges = emptyList(), + commentText = "", commentRangeLabel = "", commentSectionTitle = "", + ) +} diff --git a/apps/mobile/modules/t3-review-diff/ios/ReviewDiffCodeLayout.swift b/apps/mobile/modules/t3-review-diff/ios/ReviewDiffCodeLayout.swift new file mode 100644 index 000000000000..1d8b3e8ea8d9 --- /dev/null +++ b/apps/mobile/modules/t3-review-diff/ios/ReviewDiffCodeLayout.swift @@ -0,0 +1,123 @@ +import UIKit + +/// ASCII uses fixed-pitch columns. TextKit handles shaping, tabs, and Unicode highlights. +final class ReviewDiffCodeLayout: NSObject { + // Measurement reuses one engine; only recently drawn rows retain a full TextKit layout. + private static var measurer: ReviewDiffTextLayout { + let key = "T3ReviewDiff.textMeasurer" + if let layout = Thread.current.threadDictionary[key] as? ReviewDiffTextLayout { return layout } + let layout = ReviewDiffTextLayout() + Thread.current.threadDictionary[key] = layout + return layout + } + private static let drawnLayouts: NSCache = { + let cache = NSCache() + cache.countLimit = 128 + return cache + }() + let text: String + let starts: [Int] + let lineHeight: CGFloat + let firstLineHeight: CGFloat + let extraHeight: CGFloat + private let font: UIFont + private let width: CGFloat + private let characterWidth: CGFloat + let usesNativeLayout: Bool + + init(text: String, font: UIFont, width: CGFloat, characterWidth: CGFloat) { + self.text = text + self.font = font + self.width = width + self.characterWidth = characterWidth + lineHeight = ceil(font.lineHeight) + if text.utf8.allSatisfy({ $0 >= 32 && $0 <= 126 }) { + let columns = max(1, Int(width / characterWidth)) + starts = Array(stride(from: 0, to: max(1, text.utf8.count), by: columns)) + firstLineHeight = font.lineHeight + extraHeight = CGFloat(starts.count - 1) * lineHeight + usesNativeLayout = false + } else { + let layout = Self.measurer + layout.configure(text: text, font: font, width: width, characterWidth: characterWidth) + let manager = layout.manager + let container = layout.container + usesNativeLayout = true + starts = [0] + firstLineHeight = manager.numberOfGlyphs > 0 + ? manager.lineFragmentRect(forGlyphAt: 0, effectiveRange: nil).height : font.lineHeight + extraHeight = max(0, manager.usedRect(for: container).height - firstLineHeight) + } + } + + private func nativeLayout() -> ReviewDiffTextLayout { + if let cached = Self.drawnLayouts.object(forKey: self) { return cached } + let layout = ReviewDiffTextLayout() + layout.configure(text: text, font: font, width: width, characterWidth: characterWidth) + Self.drawnLayouts.setObject(layout, forKey: self) + return layout + } + + /// Only colors change when syntax tokens arrive; the measured text and font stay intact. + func decorate(text: NSAttributedString, highlights: [NSRange], color: UIColor, version: Int) { + guard usesNativeLayout else { return } + let layout = nativeLayout() + guard layout.decorationVersion != version else { return } + let storage = layout.storage + let fullRange = NSRange(location: 0, length: storage.length) + storage.beginEditing() + storage.removeAttribute(.foregroundColor, range: fullRange) + storage.removeAttribute(.backgroundColor, range: fullRange) + text.enumerateAttribute(.foregroundColor, in: NSRange(location: 0, length: text.length)) { value, range, _ in + let intersection = NSIntersectionRange(range, fullRange) + if let value, intersection.length > 0 { + storage.addAttribute(.foregroundColor, value: value, range: intersection) + } + } + for range in highlights { + let intersection = NSIntersectionRange(range, fullRange) + if intersection.length > 0 { + storage.addAttribute(.backgroundColor, value: color, range: intersection) + } + } + storage.endEditing() + layout.decorationVersion = version + } + + func draw(at origin: CGPoint, clip: CGRect) { + guard usesNativeLayout else { return } + let layout = nativeLayout() + let manager = layout.manager + let container = layout.container + let visible = clip.offsetBy(dx: -origin.x, dy: -origin.y) + let range = manager.glyphRange(forBoundingRect: visible, in: container) + manager.drawBackground(forGlyphRange: range, at: origin) + manager.drawGlyphs(forGlyphRange: range, at: origin) + } +} + +private final class ReviewDiffTextLayout { + let storage = NSTextStorage() + let manager = NSLayoutManager() + let container = NSTextContainer(size: .zero) + var decorationVersion = -1 + + init() { + container.lineFragmentPadding = 0 + container.lineBreakMode = .byCharWrapping + manager.addTextContainer(container) + storage.addLayoutManager(manager) + } + + func configure(text: String, font: UIFont, width: CGFloat, characterWidth: CGFloat) { + let paragraph = NSMutableParagraphStyle() + paragraph.lineBreakMode = .byCharWrapping + paragraph.tabStops = [] + paragraph.defaultTabInterval = characterWidth * 4 + container.size = CGSize(width: max(1, width), height: .greatestFiniteMagnitude) + storage.setAttributedString(NSAttributedString(string: text, attributes: [ + .font: font, .ligature: 0, .paragraphStyle: paragraph, + ])) + manager.ensureLayout(for: container) + } +} diff --git a/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift b/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift index 74111988f150..e2400e0a9393 100644 --- a/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift +++ b/apps/mobile/modules/t3-review-diff/ios/T3ReviewDiffView.swift @@ -137,6 +137,7 @@ private struct ReviewDiffNativeTheme { } private struct ReviewDiffNativeStylePayload: Decodable { + let wordWrap: Bool? let rowHeight: Double? let contentWidth: Double? let changeBarWidth: Double? @@ -170,6 +171,7 @@ private struct ReviewDiffNativeStylePayload: Decodable { } private struct ReviewDiffNativeStyle { + let wordWrap: Bool let rowHeight: CGFloat let contentWidth: CGFloat let changeBarWidth: CGFloat @@ -203,6 +205,7 @@ private struct ReviewDiffNativeStyle { static func resolve(_ payload: ReviewDiffNativeStylePayload?) -> ReviewDiffNativeStyle { ReviewDiffNativeStyle( + wordWrap: payload?.wordWrap ?? false, rowHeight: metric(payload?.rowHeight, fallback: 24), contentWidth: metric(payload?.contentWidth, fallback: 2800), changeBarWidth: nonNegativeMetric(payload?.changeBarWidth, fallback: 4), @@ -277,6 +280,7 @@ private struct ReviewDiffNativeStyle { func applyingOverrides(rowHeight: CGFloat?, contentWidth: CGFloat?) -> ReviewDiffNativeStyle { ReviewDiffNativeStyle( + wordWrap: wordWrap, rowHeight: rowHeight ?? self.rowHeight, contentWidth: contentWidth ?? self.contentWidth, changeBarWidth: changeBarWidth, @@ -434,16 +438,21 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate { guard let self, generation == self.rowsDecodeGeneration else { return } - self.rows = decodedRows - self.contentView.rows = decodedRows - self.hasAppliedInitialRowIndex = false - self.lastVisibleFileId = nil - self.emitDebug("rows-decoded", [ - "rows": decodedRows.count, - "firstKind": decodedRows.first?.kind ?? "none", - ]) - self.updateContentMetrics() - self.applyPendingScrollIfNeeded() + self.contentView.prepareRows(decodedRows, on: self.payloadDecodeQueue, isCurrent: { [weak self] in + generation == self?.rowsDecodeGeneration + }, completion: { [weak self] in + guard let self, generation == self.rowsDecodeGeneration else { return } + self.rows = decodedRows + self.contentView.rows = decodedRows + self.hasAppliedInitialRowIndex = false + self.lastVisibleFileId = nil + self.emitDebug("rows-decoded", [ + "rows": decodedRows.count, + "firstKind": decodedRows.first?.kind ?? "none", + ]) + self.updateContentMetrics() + self.applyPendingScrollIfNeeded() + }) } } catch { let message = error.localizedDescription @@ -652,6 +661,7 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate { private func updateContentMetrics() { let style = contentView.style + contentView.viewportWidth = bounds.width let height = max(bounds.height, contentView.contentHeight) let width = bounds.width scrollView.contentSize = CGSize(width: bounds.width, height: height) @@ -661,7 +671,6 @@ public final class T3ReviewDiffView: ExpoView, UIScrollViewDelegate { width: max(width, 1), height: max(bounds.height, 1) ) - contentView.viewportWidth = bounds.width contentView.verticalOffset = scrollView.contentOffset.y contentView.invalidateVisibleViewport() contentView.setNeedsDisplay() @@ -929,6 +938,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { headerPathOffsetsByFileId.removeAll() activePanFileId = nil activePanKind = nil + codeDecorationVersion += 1 tokenAttributedStringsByRowId.removeAll() rebuildRowLayout() setNeedsDisplayForVisibleBounds() @@ -936,6 +946,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { } var tokensByRowId: [String: [ReviewDiffNativeToken]] = [:] { didSet { + codeDecorationVersion += 1 tokenAttributedStringsByRowId.removeAll() clampHorizontalOffsets() setNeedsDisplayForVisibleBounds() @@ -976,6 +987,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { } var style = ReviewDiffNativeStyle.resolve(nil) { didSet { + codeDecorationVersion += 1 tokenAttributedStringsByRowId.removeAll() rebuildRowLayout() clampHorizontalOffsets() @@ -984,6 +996,10 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { } var viewportWidth: CGFloat = 0 { didSet { + // Wrapped rows take their height from the width, so a new width is a new layout. + if style.wordWrap, viewportWidth != oldValue { + rebuildRowLayout() + } clampHorizontalOffsets() setNeedsDisplayForVisibleBounds() } @@ -992,6 +1008,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { var theme = ReviewDiffNativeTheme.resolve("light") { didSet { tokenColorsByHex.removeAll() + codeDecorationVersion += 1 tokenAttributedStringsByRowId.removeAll() setNeedsDisplayForVisibleBounds() } @@ -1004,6 +1021,13 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { private var tokenColorsByHex: [String: UIColor] = [:] private var tokenAttributedStringsByRowId: [String: NSAttributedString] = [:] private var codeCharacterWidth: CGFloat = 8 + /// Columns per visual line while word wrap is on; nil while code rows pan horizontally. + private var codeWrapColumns: Int? + /// Text geometry survives comment height changes; width, font, and content invalidate it. + private var codeLayoutsByRowId: [String: ReviewDiffCodeLayout] = [:] + private var codeLayoutWidth: CGFloat = 0 + private var codeLayoutFont: UIFont? + private var codeDecorationVersion = 0 private var panStartHorizontalOffset: CGFloat = 0 private var activePanFileId: String? private var activePanKind: ReviewDiffHorizontalPanKind? @@ -1029,6 +1053,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { stickyWidth + style.codePadding } + /// Height before word wrap. Laid-out rows use height(at:), which includes wrapped lines. private func height(for row: ReviewDiffNativeRow) -> CGFloat { if row.kind == "file" { return style.fileHeaderHeight @@ -1045,6 +1070,16 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { return style.rowHeight } + /// Laid-out height, including wrapped lines. Requires a layout built from the current rows. + private func height(at index: Int) -> CGFloat { + let nextOffset = index + 1 < rowOffsets.count ? rowOffsets[index + 1] : contentHeight + return nextOffset - rowOffsets[index] + } + + private var codeWrapLineHeight: CGFloat { + ceil(codeFont.lineHeight) + } + func frameForRow(at index: Int) -> CGRect? { guard rows.indices.contains(index), rowOffsets.indices.contains(index) else { return nil @@ -1054,43 +1089,110 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { x: 0, y: rowOffsets[index], width: max(viewportWidth, 1), - height: height(for: rows[index]) + height: height(at: index) ) } + /// Shape new content on the existing decode worker before publishing rows to the UI. + func prepareRows( + _ rows: [ReviewDiffNativeRow], + on queue: DispatchQueue, + isCurrent: @escaping () -> Bool, + completion: @escaping () -> Void + ) { + let font = codeFont + let width = viewportWidth - codeStartX - style.codePadding + let characterWidth = monospaceCharacterWidth(font: font) + guard style.wordWrap, width >= characterWidth, characterWidth > 0 else { + completion() + return + } + let cached = codeLayoutWidth == width && codeLayoutFont == font ? codeLayoutsByRowId : [:] + queue.async { [weak self] in + var layouts: [String: ReviewDiffCodeLayout] = [:] + for row in rows where row.kind == "line" { + guard let text = row.content else { continue } + if let previous = cached[row.id], previous.text == text { + layouts[row.id] = previous + } else { + layouts[row.id] = ReviewDiffCodeLayout(text: text, font: font, width: width, characterWidth: characterWidth) + } + } + DispatchQueue.main.async { [weak self] in + guard let self, isCurrent() else { return } + if self.codeFont != font || self.viewportWidth - self.codeStartX - self.style.codePadding != width { + self.prepareRows(rows, on: queue, isCurrent: isCurrent, completion: completion) + return + } + self.codeLayoutWidth = width + self.codeLayoutFont = font + self.codeLayoutsByRowId = layouts + completion() + } + } + } + private func rebuildRowLayout() { var nextOffsets: [CGFloat] = [] var nextFileHeaderRowIndices: [Int] = [] nextOffsets.reserveCapacity(rows.count) var maxColumnCountsByFileId: [String: Int] = [:] + var nextCodeLayouts: [String: ReviewDiffCodeLayout] = [:] var offset: CGFloat = 0 + let font = codeFont + let characterWidth = monospaceCharacterWidth(font: font) + let wrapAvailableWidth = viewportWidth - codeStartX - style.codePadding + let wrapColumns = style.wordWrap && characterWidth > 0 && wrapAvailableWidth >= characterWidth + ? Int(wrapAvailableWidth / characterWidth) + : nil + if codeLayoutWidth != wrapAvailableWidth || codeLayoutFont != font { + codeLayoutsByRowId.removeAll() + codeLayoutWidth = wrapAvailableWidth + codeLayoutFont = font + } for (index, row) in rows.enumerated() { nextOffsets.append(offset) if row.kind == "file" { nextFileHeaderRowIndices.append(index) } - offset += height(for: row) + var rowHeight = height(for: row) let fileId = resolvedFileId(for: row) switch row.kind { case "line": - maxColumnCountsByFileId[fileId] = max( - maxColumnCountsByFileId[fileId] ?? 0, - row.content?.count ?? 0 - ) + // UTF-16 columns match the word diff ranges and the segments drawCodeLines draws. + let columnCount = row.content?.utf16.count ?? 0 + maxColumnCountsByFileId[fileId] = max(maxColumnCountsByFileId[fileId] ?? 0, columnCount) + if wrapColumns != nil, let content = row.content { + let cached = codeLayoutsByRowId[row.id] + let layout: ReviewDiffCodeLayout + if let cached, cached.text == content { + layout = cached + } else { + layout = ReviewDiffCodeLayout( + text: content, font: font, width: wrapAvailableWidth, characterWidth: characterWidth + ) + } + nextCodeLayouts[row.id] = layout + if rowHeight > 0 { + rowHeight = max(rowHeight, layout.firstLineHeight) + layout.extraHeight + } + } case "hunk": maxColumnCountsByFileId[fileId] = max( maxColumnCountsByFileId[fileId] ?? 0, - row.text?.count ?? 0 + row.text?.utf16.count ?? 0 ) default: - continue + break } + offset += rowHeight } - let characterWidth = monospaceCharacterWidth(font: codeFont) codeCharacterWidth = characterWidth + codeWrapColumns = wrapColumns + codeLayoutsByRowId = nextCodeLayouts contentWidthsByFileId = maxColumnCountsByFileId.mapValues { maxColumnCount in let measuredWidth = ceil(CGFloat(maxColumnCount) * characterWidth) + style.codePadding * 2 return max(0, min(style.contentWidth, measuredWidth)) @@ -1498,7 +1600,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { while lowerBound <= upperBound { let midpoint = (lowerBound + upperBound) / 2 let rowStart = rowOffsets[midpoint] - let rowEnd = rowStart + height(for: rows[midpoint]) + let rowEnd = rowStart + height(at: midpoint) if absoluteY < rowStart { upperBound = midpoint - 1 @@ -1521,7 +1623,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { var upperBound = rows.count while lowerBound < upperBound { let midpoint = (lowerBound + upperBound) / 2 - let rowEnd = rowOffsets[midpoint] + height(for: rows[midpoint]) + let rowEnd = rowOffsets[midpoint] + height(at: midpoint) if rowEnd < absoluteY { lowerBound = midpoint + 1 @@ -1609,6 +1711,9 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { let row = rows.first(where: { resolvedFileId(for: $0) == target.fileId && $0.kind == "file" }) { return maxHeaderPathOffset(for: row) } + if codeWrapColumns != nil { + return 0 + } return max(0, contentWidth(for: target.fileId) - max(0, viewportWidth - codeStartX)) } @@ -1728,7 +1833,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { var drawnRowCount = 0 for rowIndex in firstRowIndex...lastRowIndex { let rowStart = rowOffsets[rowIndex] - let rowHeight = height(for: rows[rowIndex]) + let rowHeight = height(at: rowIndex) if rowHeight <= 0 { continue } @@ -1786,7 +1891,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { private func drawRow(_ row: ReviewDiffNativeRow, rowIndex: Int, context: CGContext) { let rowY = rowOffsets[rowIndex] - verticalOffset - let fullRect = CGRect(x: 0, y: rowY, width: max(bounds.width, viewportWidth), height: height(for: row)) + let fullRect = CGRect(x: 0, y: rowY, width: max(bounds.width, viewportWidth), height: height(at: rowIndex)) switch row.kind { case "file": @@ -2172,15 +2277,21 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { let horizontalOffset = horizontalOffset(for: fileId) let contentWidth = contentWidth(for: fileId) let change = row.change ?? "context" + // Wrapped rows keep the line number and first code line in the first row-height band. + let layout = codeLayoutsByRowId[row.id] + let firstLineRect = CGRect( + x: rect.minX, y: rect.minY, width: rect.width, + height: max(style.rowHeight, layout?.firstLineHeight ?? 0) + ) rowBackground(for: change).setFill() context.fill(rect) if change == "add" { theme.addBar.setFill() - context.fill(CGRect(x: 0, y: rect.minY, width: style.changeBarWidth, height: style.rowHeight)) + context.fill(CGRect(x: 0, y: rect.minY, width: style.changeBarWidth, height: rect.height)) } else if change == "delete" { drawDeleteStripes( - rect: CGRect(x: 0, y: rect.minY, width: style.changeBarWidth, height: style.rowHeight), + rect: CGRect(x: 0, y: rect.minY, width: style.changeBarWidth, height: rect.height), context: context ) } @@ -2193,7 +2304,7 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { "\(lineNumber)", rect: CGRect( x: style.changeBarWidth, - y: centeredTextY(in: rect, font: lineNumberFont), + y: centeredTextY(in: firstLineRect, font: lineNumberFont), width: style.gutterWidth - style.codePadding, height: lineNumberFont.lineHeight ), @@ -2203,28 +2314,85 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { } context.saveGState() - context.clip(to: CGRect(x: stickyWidth, y: rect.minY, width: max(0, viewportWidth - stickyWidth), height: style.rowHeight)) + context.clip(to: CGRect(x: stickyWidth, y: rect.minY, width: max(0, viewportWidth - stickyWidth), height: rect.height)) let codeTextRect = CGRect( x: codeStartX - horizontalOffset, - y: centeredTextY(in: rect, font: codeFont), + y: centeredTextY(in: firstLineRect, font: codeFont), width: contentWidth, height: codeFont.lineHeight ) - drawWordDiffRanges(row, rowRect: rect, context: context, horizontalOffset: horizontalOffset) + if let layout, layout.usesNativeLayout { + let text = tokensByRowId[row.id].map { + tokenAttributedString(rowId: row.id, tokens: $0, fallbackColor: theme.text, font: codeFont) + } ?? NSAttributedString(string: row.content ?? "", attributes: [.foregroundColor: theme.text]) + let highlights = (change == "add" || change == "delete") ? (row.wordDiffRanges ?? []) : [] + layout.decorate( + text: text, + highlights: highlights.filter { $0.start >= 0 && $0.end > $0.start }.map { + NSRange(location: $0.start, length: $0.end - $0.start) + }, + color: (change == "add" ? theme.addBar : theme.deleteBar).withAlphaComponent(0.28), + version: codeDecorationVersion + ) + layout.draw( + at: CGPoint(x: codeStartX, y: rect.minY + max(0, (firstLineRect.height - layout.firstLineHeight) / 2)), + clip: context.boundingBoxOfClipPath + ) + context.restoreGState() + return + } + let lineStarts = layout?.starts ?? [0] + drawWordDiffRanges( + row, + lineStarts: lineStarts, + firstLineRect: firstLineRect, + context: context, + horizontalOffset: horizontalOffset + ) if let tokens = tokensByRowId[row.id], !tokens.isEmpty { - drawTokenText( + let attributedText = tokenAttributedString( rowId: row.id, - tokens, - rect: codeTextRect, + tokens: tokens, fallbackColor: theme.text, font: codeFont ) + drawCodeLines(length: attributedText.length, lineStarts: lineStarts, firstLineRect: codeTextRect) { range, lineRect in + let segment = range.length == attributedText.length + ? attributedText + : attributedText.attributedSubstring(from: range) + segment.draw(in: lineRect) + } } else { - drawText(row.content ?? "", rect: codeTextRect, color: theme.text, font: codeFont) + let content = (row.content ?? "") as NSString + drawCodeLines(length: content.length, lineStarts: lineStarts, firstLineRect: codeTextRect) { range, lineRect in + let segment = range.length == content.length ? content as String : content.substring(with: range) + drawText(segment, rect: lineRect, color: theme.text, font: codeFont) + } } context.restoreGState() } + /// Draws the segment starting at each of the row's line starts on its own visual line. + private func drawCodeLines( + length: Int, + lineStarts: [Int], + firstLineRect: CGRect, + draw: (NSRange, CGRect) -> Void + ) { + var lineRect = firstLineRect + let clip = UIGraphicsGetCurrentContext()?.boundingBoxOfClipPath ?? bounds + let first = max(0, Int(floor((clip.minY - firstLineRect.minY) / codeWrapLineHeight))) + let last = min(lineStarts.count, Int(ceil((clip.maxY - firstLineRect.minY) / codeWrapLineHeight))) + guard first < last else { return } + lineRect.origin.y += CGFloat(first) * codeWrapLineHeight + for line in first.. start else { + continue + } + let highlightRect = CGRect( + x: codeStartX - horizontalOffset + CGFloat(start - lineStart) * codeCharacterWidth, + y: highlightY + CGFloat(line) * codeWrapLineHeight, + width: max(2, CGFloat(end - start) * codeCharacterWidth), + height: highlightHeight + ) + UIBezierPath(roundedRect: highlightRect, cornerRadius: 3).fill() + } } } @@ -2448,22 +2624,6 @@ private final class ReviewDiffContentView: UIView, UIGestureRecognizerDelegate { return (sample as NSString).size(withAttributes: attributes).width / CGFloat(sampleLength) } - private func drawTokenText( - rowId: String, - _ tokens: [ReviewDiffNativeToken], - rect: CGRect, - fallbackColor: UIColor, - font: UIFont - ) { - let attributedText = tokenAttributedString( - rowId: rowId, - tokens: tokens, - fallbackColor: fallbackColor, - font: font - ) - attributedText.draw(in: rect) - } - private func tokenAttributedString( rowId: String, tokens: [ReviewDiffNativeToken], diff --git a/apps/mobile/modules/t3-review-diff/tests/ios/main.swift b/apps/mobile/modules/t3-review-diff/tests/ios/main.swift new file mode 100644 index 000000000000..baeeaae77d9a --- /dev/null +++ b/apps/mobile/modules/t3-review-diff/tests/ios/main.swift @@ -0,0 +1,60 @@ +import UIKit + +func check(_ passed: Bool, _ message: String = "Failed layout check") { + if !passed { + FileHandle.standardError.write(Data((message + "\n").utf8)) + exit(1) + } +} + +// Runs the production layout against UIKit through Mac Catalyst, without launching an app. +let font = UIFont.monospacedSystemFont(ofSize: 14, weight: .regular) +let characterWidth = ("M" as NSString).size(withAttributes: [.font: font]).width +let fixtures = ["漢字表示", "e\u{301}", "👨‍👩‍👧‍👦", "مرحبا بالعالم ", "\tvalue "] +var cases = 0 +for fixture in fixtures { + let text = String(repeating: fixture, count: 40) + var previousHeight = CGFloat.greatestFiniteMagnitude + for width: CGFloat in [180, 280, 420] { + let layout = ReviewDiffCodeLayout(text: text, font: font, width: width, characterWidth: characterWidth) + let height = layout.firstLineHeight + layout.extraHeight + check(height <= previousHeight, "Wider text must not require more height") + previousHeight = height + let fullRange = NSRange(location: 0, length: text.utf16.count) + let attributed = NSAttributedString(string: text, attributes: [.foregroundColor: UIColor.black]) + layout.decorate(text: attributed, highlights: [], color: .clear, version: 0) + let format = UIGraphicsImageRendererFormat() + format.scale = 1 + format.opaque = false + format.preferredRange = .standard + let size = CGSize(width: width + 40, height: ceil(height)) + let image = UIGraphicsImageRenderer(size: size, format: format).image { _ in + layout.draw(at: .zero, clip: CGRect(origin: .zero, size: size)) + } + let bitmap = image.cgImage! + let data = bitmap.dataProvider!.data! + let bytes = CFDataGetBytePtr(data)! + // Render without a viewport clip so an overflowing glyph cannot hide behind clipping. + for y in 0.. JSON.stringify(nativeReviewDiffTheme), [nativeReviewDiffTheme], ); + // The card's height is sized from its row count, so its snippet stays unwrapped. const nativeStyleJson = useMemo( - () => JSON.stringify(nativeReviewDiffStyle), + () => JSON.stringify({ ...nativeReviewDiffStyle, wordWrap: false }), [nativeReviewDiffStyle], ); const nativeDiffHeight = useMemo( diff --git a/apps/mobile/src/features/review/nativeReviewDiffAdapter.ts b/apps/mobile/src/features/review/nativeReviewDiffAdapter.ts index 8bea04c524dd..3056cc2136ca 100644 --- a/apps/mobile/src/features/review/nativeReviewDiffAdapter.ts +++ b/apps/mobile/src/features/review/nativeReviewDiffAdapter.ts @@ -63,8 +63,13 @@ function opaqueNativeHexColor(color: string, background: string): string { return `#${channels.map((channel) => channel.toString(16).padStart(2, "0")).join("")}`; } -export function createNativeReviewDiffStyle(codeSurface: ResolvedMobileCodeSurface) { +/** `wordWrap` wraps line rows at the view width instead of panning them horizontally. */ +export function createNativeReviewDiffStyle( + codeSurface: ResolvedMobileCodeSurface, + wordWrap: boolean, +) { return { + wordWrap, rowHeight: codeSurface.rowHeight, contentWidth: NATIVE_REVIEW_DIFF_CONTENT_WIDTH, changeBarWidth: 4, diff --git a/apps/mobile/src/features/review/reviewDiffHighlightScheduler.test.ts b/apps/mobile/src/features/review/reviewDiffHighlightScheduler.test.ts new file mode 100644 index 000000000000..3792e49f54c7 --- /dev/null +++ b/apps/mobile/src/features/review/reviewDiffHighlightScheduler.test.ts @@ -0,0 +1,74 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { createReviewDiffHighlightScheduler } from "./reviewDiffHighlightScheduler"; + +describe("review diff highlighting while scrolling", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("keeps requesting new rows during gradual scrolling through a large diff", () => { + const request = vi.fn(); + const scheduler = createReviewDiffHighlightScheduler(request); + for (let firstRowIndex = 1; firstRowIndex <= 1_674; firstRowIndex++) { + scheduler.update({ firstRowIndex, lastRowIndex: firstRowIndex + 80 }); + vi.advanceTimersByTime(16); + } + expect(request.mock.calls.length).toBeGreaterThan(100); + vi.advanceTimersByTime(150); + expect(request).toHaveBeenLastCalledWith({ firstRowIndex: 1_674, lastRowIndex: 1_754 }); + }); + + it("highlights the settled viewport even below the movement threshold", () => { + const request = vi.fn(); + const scheduler = createReviewDiffHighlightScheduler(request); + scheduler.update({ firstRowIndex: 2, lastRowIndex: 82 }); + vi.advanceTimersByTime(100); + scheduler.update({ firstRowIndex: 3, lastRowIndex: 83 }); + vi.advanceTimersByTime(100); + expect(request).not.toHaveBeenCalled(); + vi.advanceTimersByTime(50); + expect(request).toHaveBeenCalledExactlyOnceWith({ firstRowIndex: 3, lastRowIndex: 83 }); + }); + + it("does not let repeated draw events starve the settled refresh", () => { + const request = vi.fn(); + const scheduler = createReviewDiffHighlightScheduler(request); + for (let i = 0; i < 10; i++) { + scheduler.update({ firstRowIndex: 1, lastRowIndex: 81 }); + vi.advanceTimersByTime(30); + } + expect(request).toHaveBeenCalledExactlyOnceWith({ firstRowIndex: 1, lastRowIndex: 81 }); + }); + + it("requests large jumps and reverse scrolling immediately without stale timers", () => { + const request = vi.fn(); + const scheduler = createReviewDiffHighlightScheduler(request); + scheduler.update({ firstRowIndex: 1, lastRowIndex: 81 }); + scheduler.update({ firstRowIndex: 1_000, lastRowIndex: 1_080 }); + scheduler.update({ firstRowIndex: 0, lastRowIndex: 80 }); + vi.runAllTimers(); + expect(request.mock.calls).toEqual([ + [{ firstRowIndex: 1_000, lastRowIndex: 1_080 }], + [{ firstRowIndex: 0, lastRowIndex: 80 }], + ]); + }); + + it("cancels pending work on disposal and resets the range for a new diff", () => { + const request = vi.fn(); + const scheduler = createReviewDiffHighlightScheduler(request); + scheduler.update({ firstRowIndex: 1, lastRowIndex: 81 }); + scheduler.cancel(); + vi.runAllTimers(); + expect(request).not.toHaveBeenCalled(); + scheduler.update({ firstRowIndex: 1_000, lastRowIndex: 1_080 }); + request.mockClear(); + scheduler.update({ firstRowIndex: 1_001, lastRowIndex: 1_081 }); + scheduler.reset(); + vi.runAllTimers(); + expect(request).not.toHaveBeenCalled(); + scheduler.update({ firstRowIndex: 1, lastRowIndex: 81 }); + expect(request).not.toHaveBeenCalled(); + vi.advanceTimersByTime(150); + expect(request).toHaveBeenCalledExactlyOnceWith({ firstRowIndex: 1, lastRowIndex: 81 }); + }); +}); diff --git a/apps/mobile/src/features/review/reviewDiffHighlightScheduler.ts b/apps/mobile/src/features/review/reviewDiffHighlightScheduler.ts new file mode 100644 index 000000000000..4cededa52954 --- /dev/null +++ b/apps/mobile/src/features/review/reviewDiffHighlightScheduler.ts @@ -0,0 +1,51 @@ +export interface NativeReviewVisibleRange { + readonly firstRowIndex: number; + readonly lastRowIndex: number; +} + +export function createReviewDiffHighlightScheduler( + request: (range: NativeReviewVisibleRange) => void, +) { + let requestedRange: NativeReviewVisibleRange = { firstRowIndex: 0, lastRowIndex: 80 }; + let visibleRange = requestedRange; + let timer: ReturnType | undefined; + + const cancel = () => { + clearTimeout(timer); + timer = undefined; + }; + const flush = () => { + cancel(); + requestedRange = visibleRange; + request(visibleRange); + }; + + return { + update(nextRange: NativeReviewVisibleRange) { + if ( + nextRange.firstRowIndex === visibleRange.firstRowIndex && + nextRange.lastRowIndex === visibleRange.lastRowIndex + ) { + return; + } + visibleRange = nextRange; + cancel(); + // Accumulate small scroll events relative to the last request, not each other. + const movedRows = + Math.abs(nextRange.firstRowIndex - requestedRange.firstRowIndex) + + Math.abs(nextRange.lastRowIndex - requestedRange.lastRowIndex); + if (movedRows >= 20) { + flush(); + } else if (movedRows > 0) { + // Cover the final viewport even when scrolling stops below the threshold. + timer = setTimeout(flush, 150); + } + }, + reset() { + cancel(); + requestedRange = { firstRowIndex: 0, lastRowIndex: 80 }; + visibleRange = requestedRange; + }, + cancel, + }; +} diff --git a/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts b/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts index 35f06c263666..61a205f9d917 100644 --- a/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts +++ b/apps/mobile/src/features/review/useNativeReviewDiffHighlighting.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { highlightNativeReviewDiffVisibleRows, @@ -8,10 +8,10 @@ import { import type { NativeReviewDiffRow } from "../diffs/nativeReviewDiffSurface"; import type { NativeReviewDiffFile } from "../diffs/nativeReviewDiffTypes"; -interface NativeReviewVisibleRange { - readonly firstRowIndex: number; - readonly lastRowIndex: number; -} +import { + createReviewDiffHighlightScheduler, + type NativeReviewVisibleRange, +} from "./reviewDiffHighlightScheduler"; function createEmptyTokenPatch(resetKey: string): string { return JSON.stringify({ resetKey, tokensByRowId: {} }); @@ -43,23 +43,22 @@ export function useNativeReviewDiffHighlighting(input: { }) { const { enabled, files, resetKey, rows, scheme } = input; const highlightedRowIdsRef = useRef>(new Set()); - const visibleRangeRef = useRef({ + const [visibleRange, setVisibleRange] = useState({ firstRowIndex: 0, lastRowIndex: 80, }); const visibleChunkIndexRef = useRef(0); const [tokensPatchJson, setTokensPatchJson] = useState(() => createEmptyTokenPatch(resetKey)); - const [visibleHighlightRequest, setVisibleHighlightRequest] = useState(0); + const [scheduler] = useState(() => createReviewDiffHighlightScheduler(setVisibleRange)); useEffect(() => { + scheduler.reset(); highlightedRowIdsRef.current = new Set(); visibleChunkIndexRef.current = 0; - visibleRangeRef.current = { firstRowIndex: 0, lastRowIndex: 80 }; + setVisibleRange({ firstRowIndex: 0, lastRowIndex: 80 }); setTokensPatchJson(createEmptyTokenPatch(resetKey)); - if (enabled && rows.length > 0) { - setVisibleHighlightRequest((request) => request + 1); - } - }, [enabled, resetKey, rows.length]); + return () => scheduler.cancel(); + }, [enabled, resetKey, rows.length, scheduler]); useEffect(() => { if (!enabled || rows.length === 0) { @@ -67,7 +66,7 @@ export function useNativeReviewDiffHighlighting(input: { } const abortController = new AbortController(); - const requestRange = visibleRangeRef.current; + const requestRange = visibleRange; const engine: NativeReviewDiffHighlightEngine = "native"; void (async () => { @@ -119,22 +118,10 @@ export function useNativeReviewDiffHighlighting(input: { })(); return () => abortController.abort(); - }, [enabled, files, resetKey, rows, scheme, visibleHighlightRequest]); - - const updateVisibleRange = useCallback((nextRange: NativeReviewVisibleRange) => { - const previousRange = visibleRangeRef.current; - const movedRows = - Math.abs(nextRange.firstRowIndex - previousRange.firstRowIndex) + - Math.abs(nextRange.lastRowIndex - previousRange.lastRowIndex); - - visibleRangeRef.current = nextRange; - if (movedRows >= 20) { - setVisibleHighlightRequest((request) => request + 1); - } - }, []); + }, [enabled, files, resetKey, rows, scheme, visibleRange]); return { tokensPatchJson, - updateVisibleRange, + updateVisibleRange: scheduler.update, }; } diff --git a/apps/mobile/src/features/settings/appearance/useAppearanceCodeSurface.ts b/apps/mobile/src/features/settings/appearance/useAppearanceCodeSurface.ts index 62760f1e43fa..5bd7bb469827 100644 --- a/apps/mobile/src/features/settings/appearance/useAppearanceCodeSurface.ts +++ b/apps/mobile/src/features/settings/appearance/useAppearanceCodeSurface.ts @@ -21,8 +21,8 @@ export function useAppearanceCodeSurface(): { ); const nativeSourceStyle = useMemo(() => createNativeSourceStyle(codeSurface), [codeSurface]); const nativeReviewDiffStyle = useMemo( - () => createNativeReviewDiffStyle(codeSurface), - [codeSurface], + () => createNativeReviewDiffStyle(codeSurface, appearance.codeWordBreak), + [appearance.codeWordBreak, codeSurface], ); return { diff --git a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx index c4865e2a8404..0a7c57bb2dbc 100644 --- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx @@ -1459,7 +1459,7 @@ export function NewTaskDraftScreen(props: { in { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpLogsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", - otlpHeaders: undefined, - otlpProtocol: "http/json", devAllowedOrigins: [], } as const; @@ -764,7 +765,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { ), ); - expect(resolved.otlpHeaders).toEqual({ + expect(resolved.otlpTracesExport.headers).toEqual({ authorization: "Basic abc==", "x-tenant": "t3", }); @@ -808,7 +809,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { ), ); - expect(resolved.otlpHeaders).toEqual({ + expect(resolved.otlpTracesExport.headers).toEqual({ authorization: "Bearer abc==", "x-tenant": "t3", }); @@ -816,7 +817,7 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { }), ); - it.effect("reads the OTLP protocol from env", () => + it.effect("gives every signal the protocol named without one", () => Effect.gen(function* () { const { join } = yield* Path.Path; const baseDir = join(NodeOS.tmpdir(), "t3-cli-config-otlp-protocol-base"); @@ -848,7 +849,11 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { ), ); - expect(resolved.otlpProtocol).toBe("http/protobuf"); + expect([ + resolved.otlpTracesExport.protocol, + resolved.otlpMetricsExport.protocol, + resolved.otlpLogsExport.protocol, + ]).toEqual(["http/protobuf", "http/protobuf", "http/protobuf"]); }), ); diff --git a/apps/server/src/cli/config.ts b/apps/server/src/cli/config.ts index 32f465b6fb0a..09a30aeb19e7 100644 --- a/apps/server/src/cli/config.ts +++ b/apps/server/src/cli/config.ts @@ -1,5 +1,9 @@ import * as NetService from "@t3tools/shared/Net"; -import { OtlpHeadersFromString, OtlpProtocol } from "@t3tools/shared/observability"; +import { + OtlpHeadersFromString, + OtlpProtocol, + type SignalExport, +} from "@t3tools/shared/observability"; import { parsePersistedServerObservabilitySettings } from "@t3tools/shared/serverSettings"; import { DesktopBackendBootstrap, PortSchema } from "@t3tools/contracts"; import * as Config from "effect/Config"; @@ -382,6 +386,14 @@ export const resolveServerConfig = ( ); const logLevel = Option.getOrElse(cliLogLevel, () => env.logLevel); + // T3 Code's own OTLP variables name no signal, so the one answer they give + // is the answer for all three. + const signalExport: SignalExport = { + protocol: env.otlpProtocol, + headers: env.otlpHeaders, + exportIntervalMs: env.otlpExportIntervalMs, + }; + const config: ServerConfig.ServerConfig["Service"] = { logLevel, traceMinLevel: env.traceMinLevel, @@ -399,10 +411,10 @@ export const resolveServerConfig = ( persistedObservabilitySettings.otlpMetricsUrl, otlpLogsUrl: env.otlpLogsUrl ?? bootstrap?.otlpLogsUrl ?? persistedObservabilitySettings.otlpLogsUrl, - otlpExportIntervalMs: env.otlpExportIntervalMs, + otlpTracesExport: signalExport, + otlpMetricsExport: signalExport, + otlpLogsExport: signalExport, otlpServiceName: env.otlpServiceName, - otlpHeaders: env.otlpHeaders, - otlpProtocol: env.otlpProtocol, mode, port, cwd, diff --git a/apps/server/src/cli/pair.ts b/apps/server/src/cli/pair.ts index f0023c8aaef6..fbc50c211594 100644 --- a/apps/server/src/cli/pair.ts +++ b/apps/server/src/cli/pair.ts @@ -16,6 +16,7 @@ import { PortSchema, } from "@t3tools/contracts"; import { resolveWorktreeT3Home } from "@t3tools/shared/devHome"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; import { buildTailscaleHttpsBaseUrl, DEFAULT_TAILSCALE_SERVE_PORT, @@ -322,10 +323,10 @@ const makePairServerConfig = Effect.fn(function* (input: { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpLogsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", - otlpHeaders: undefined, - otlpProtocol: "http/json", mode: "web", port: state.port, host: state.host, diff --git a/apps/server/src/cli/project.test.ts b/apps/server/src/cli/project.test.ts index 4c9c950023f0..424fd8121d44 100644 --- a/apps/server/src/cli/project.test.ts +++ b/apps/server/src/cli/project.test.ts @@ -39,6 +39,7 @@ import * as ProjectService from "../project/ProjectService.ts"; import * as RepositoryIdentityResolver from "../project/RepositoryIdentityResolver.ts"; import * as T3ProjectFileLoader from "../project/T3ProjectFileLoader.ts"; import * as WorkspacePaths from "../workspace/WorkspacePaths.ts"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; import { ProjectLiveServerDeclaredResponseError, ProjectLiveServerRequestError, @@ -59,12 +60,12 @@ const makeConfig = (baseDir: string) => traceBatchWindowMs: 200, traceMaxBytes: 10 * 1024 * 1024, traceMaxFiles: 10, - otlpHeaders: undefined, - otlpProtocol: "http/json" as const, otlpTracesUrl: undefined, otlpLogsUrl: undefined, otlpMetricsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", mode: "web", port: 0, diff --git a/apps/server/src/config.ts b/apps/server/src/config.ts index 482951ca0118..701aca58e59f 100644 --- a/apps/server/src/config.ts +++ b/apps/server/src/config.ts @@ -17,7 +17,7 @@ import type * as Redacted from "effect/Redacted"; import * as Schema from "effect/Schema"; import { sweepStalePendingAttachments } from "./attachmentStore.ts"; -import { OtlpProtocol } from "@t3tools/shared/observability"; +import { DEFAULT_SIGNAL_EXPORT, type SignalExport } from "@t3tools/shared/observability"; export const DEFAULT_PORT = 3773; @@ -73,10 +73,15 @@ export class ServerConfig extends Context.Service< readonly otlpTracesUrl: string | undefined; readonly otlpMetricsUrl: string | undefined; readonly otlpLogsUrl: string | undefined; - readonly otlpExportIntervalMs: number; + /** + * How each signal is exported. Read instead of a process-wide setting so + * the wire format, credential, and schedule travel with the endpoint they + * were configured beside. + */ + readonly otlpTracesExport: SignalExport; + readonly otlpMetricsExport: SignalExport; + readonly otlpLogsExport: SignalExport; readonly otlpServiceName: string; - readonly otlpHeaders: Readonly> | undefined; - readonly otlpProtocol: OtlpProtocol; readonly mode: RuntimeMode; readonly port: number; readonly host: string | undefined; @@ -212,10 +217,10 @@ const makeTest = Effect.fn("ServerConfig.makeTest")(function* ( otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpLogsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", - otlpHeaders: undefined, - otlpProtocol: "http/json", cwd, baseDir, ...derivedPaths, diff --git a/apps/server/src/environment/ServerEnvironment.test.ts b/apps/server/src/environment/ServerEnvironment.test.ts index 27653cd516bc..17bca0b1b392 100644 --- a/apps/server/src/environment/ServerEnvironment.test.ts +++ b/apps/server/src/environment/ServerEnvironment.test.ts @@ -10,6 +10,8 @@ import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; + import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; import { PUBLISH_AGENT_ACTIVITY_SECRET, @@ -54,10 +56,10 @@ const makeServerConfig = Effect.fn(function* (baseDir: string) { otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpLogsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", - otlpHeaders: undefined, - otlpProtocol: "http/json", cwd: process.cwd(), baseDir, mode: "web", diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 879ca66ca60a..7533b8c1db19 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -322,7 +322,7 @@ export const otlpTracesProxyRouteLayer = HttpRouter.add( const request = yield* HttpServerRequest.HttpServerRequest; const config = yield* ServerConfig.ServerConfig; const otlpTracesUrl = config.otlpTracesUrl; - const otlpHeaders = config.otlpHeaders; + const otlpHeaders = config.otlpTracesExport.headers; const browserTraceCollector = yield* BrowserTraceCollector.BrowserTraceCollector; const httpClient = yield* HttpClient.HttpClient; const serialization = yield* OtlpSerialization.OtlpSerialization; diff --git a/apps/server/src/observability/Layers/Observability.ts b/apps/server/src/observability/Layers/Observability.ts index 0c9acdfb1ee3..8f7b607745f4 100644 --- a/apps/server/src/observability/Layers/Observability.ts +++ b/apps/server/src/observability/Layers/Observability.ts @@ -20,7 +20,11 @@ import * as BrowserTraceCollector from "../BrowserTraceCollector.ts"; export const ObservabilityLive = Layer.unwrap( Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; - const serializationLayer = otlpSerializationLayer(config.otlpProtocol); + const traces = config.otlpTracesExport; + const metrics = config.otlpMetricsExport; + // The trace serializer stays in the returned context because the browser + // trace forwarder exports on the same signal. + const serializationLayer = otlpSerializationLayer(traces.protocol); const resource = ServerConfig.otlpResource(config); const attribution = yield* ResourceAttribution.ResourceAttribution; @@ -51,8 +55,8 @@ export const ObservabilityLive = Layer.unwrap( ? undefined : yield* OtlpTracer.make({ url: config.otlpTracesUrl, - exportInterval: `${config.otlpExportIntervalMs} millis`, - headers: config.otlpHeaders, + exportInterval: `${traces.exportIntervalMs} millis`, + headers: traces.headers, resource, }); @@ -77,10 +81,10 @@ export const ObservabilityLive = Layer.unwrap( ? Layer.empty : OtlpMetrics.layer({ url: config.otlpMetricsUrl, - exportInterval: `${config.otlpExportIntervalMs} millis`, - headers: config.otlpHeaders, + exportInterval: `${metrics.exportIntervalMs} millis`, + headers: metrics.headers, resource, - }).pipe(Layer.provideMerge(serializationLayer)); + }).pipe(Layer.provide(otlpSerializationLayer(metrics.protocol))); return Layer.mergeAll(ServerLoggerLive, traceReferencesLayer, tracerLayer, metricsLayer); }), diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.testkit.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.testkit.ts index d66e96290f15..321632531406 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.testkit.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.testkit.ts @@ -16,6 +16,7 @@ import { ProviderAdapterDriverCreateError } from "../ProviderAdapterDriver.ts"; import { makeDriverLayer as makeProviderAdapterRegistryDriverLayer } from "../ProviderAdapterRegistry.ts"; import type { OrchestratorV2ProviderReplayHarness } from "../testkit/ProviderReplayHarness.ts"; import type { ProviderReplayGate } from "../testkit/ProviderReplayGate.testkit.ts"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; import { CODEX_DEFAULT_INSTANCE_ID, CODEX_DRIVER_KIND, @@ -98,12 +99,12 @@ export function makeReplayServerConfig( traceBatchWindowMs: 200, traceMaxBytes: 10 * 1024 * 1024, traceMaxFiles: 10, - otlpHeaders: undefined, - otlpProtocol: "http/json", otlpTracesUrl: undefined, otlpLogsUrl: undefined, otlpMetricsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", mode: "web", port: 0, diff --git a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts index 1ffa25f8f8a1..09510b283d05 100644 --- a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts +++ b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.testkit.ts @@ -41,6 +41,7 @@ import { makeCursorAgentOptions, } from "./CursorAdapterV2.ts"; import type { ProviderAdapterV2RuntimePolicy } from "../ProviderAdapter.ts"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; const CursorAgentSdkReplayTranscript = Schema.Struct({ provider: Schema.Literal(CURSOR_PROVIDER), @@ -560,12 +561,12 @@ function makeReplayServerConfig( traceBatchWindowMs: 200, traceMaxBytes: 10 * 1024 * 1024, traceMaxFiles: 10, - otlpHeaders: undefined, - otlpProtocol: "http/json", otlpTracesUrl: undefined, otlpLogsUrl: undefined, otlpMetricsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", mode: "web", port: 0, diff --git a/apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts b/apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts index b75cf3603d26..98b07246ca66 100644 --- a/apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts +++ b/apps/server/src/orchestration-v2/testkit/ProviderReplayHarness.ts @@ -64,6 +64,7 @@ import { type OrchestratorV2ScenarioResult, } from "./OrchestratorScenario.ts"; import { makeProviderReplayGate, type ProviderReplayGate } from "./ProviderReplayGate.testkit.ts"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; export function makeReplayServerConfig( scenario: string, @@ -110,10 +111,10 @@ export function makeReplayServerConfig( traceMaxFiles: 10, otlpTracesUrl: undefined, otlpLogsUrl: undefined, - otlpProtocol: "http/json", - otlpHeaders: undefined, otlpMetricsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", mode: "web", port: 0, diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 19f302aff105..23a1f8c0a7c0 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -3487,6 +3487,67 @@ it.effect("a listing narrowed to some projects is its own cache entry", () => }), ); +it.effect( + "keeps listing freshness tied to read start when filtered reads finish out of order", + () => + Effect.gen(function* () { + const olderStarted = yield* Deferred.make(); + const releaseOlder = yield* Deferred.make(); + let reads = 0; + const updatedAt = "2026-07-02T00:00:00Z"; + const service = yield* makeService({ + projects: [ + project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" }), + ], + providers: [ + fakeProvider("github", { + listChangeRequests: ({ filters }) => + Effect.gen(function* () { + reads += 1; + const older = filters?.checks === "failing"; + if (older) { + yield* Deferred.succeed(olderStarted, undefined); + yield* Deferred.await(releaseOlder); + } + return { + items: [ + { + ...changeRequest(1, updatedAt), + checksState: older ? ("failing" as const) : ("passing" as const), + mergeability: older ? ("mergeable" as const) : ("conflicting" as const), + }, + ], + truncated: false, + continues: false, + }; + }), + }), + ], + }); + const olderInput = { state: "open" as const, filters: { checks: "failing" as const } }; + const newerInput = { state: "open" as const, filters: { checks: "passing" as const } }; + + const olderRead = yield* service.list(olderInput).pipe(Effect.forkChild()); + yield* Deferred.await(olderStarted); + yield* TestClock.adjust("1 second"); + const newer = yield* service.list(newerInput); + yield* Deferred.succeed(releaseOlder, undefined); + const older = yield* Fiber.join(olderRead); + + assert.strictEqual(older.entries[0]?.checksState, "failing"); + assert.strictEqual(older.entries[0]?.mergeability, "mergeable"); + assert.strictEqual(newer.entries[0]?.checksState, "passing"); + assert.strictEqual(newer.entries[0]?.mergeability, "conflicting"); + assert.strictEqual(typeof older.entries[0]?.observedAt, "number"); + assert.strictEqual(typeof newer.entries[0]?.observedAt, "number"); + assert.isBelow(older.entries[0]!.observedAt!, newer.entries[0]!.observedAt!); + + const cachedOlder = yield* service.list(olderInput); + assert.strictEqual(cachedOlder.entries[0]?.observedAt, older.entries[0]?.observedAt); + assert.strictEqual(reads, 2); + }), +); + it.effect("keeps unrelated PRs warm after a mutation, explicit refresh, and project turn", () => Effect.gen(function* () { const calls: string[] = []; diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index a87cc9a35494..99bc6c22c8e0 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -618,6 +618,12 @@ function withRateLimitBackoff( Record, never>; } +// Capture before the provider read so a slow response keeps its original freshness through caches. +const observeRead = Effect.fnUntraced(function* (read: Effect.Effect) { + const observedAt = yield* Clock.currentTimeMillis; + return { value: yield* read, observedAt }; +}); + export const make = Effect.gen(function* () { const mergedPullRequests = yield* PubSub.sliding(64); const pullRequestRefreshes = yield* SubscriptionRef.make(0); @@ -1083,6 +1089,7 @@ export const make = Effect.gen(function* () { readonly project: SupportedProject; readonly item: ProviderChangeRequest; readonly viewer: string; + readonly observedAt: number; }): PullRequestListEntry => { const viewer = input.viewer.toLowerCase(); return { @@ -1105,6 +1112,7 @@ export const make = Effect.gen(function* () { deletions: input.item.deletions, createdAt: input.item.createdAt, updatedAt: input.item.updatedAt, + observedAt: input.observedAt, ...(input.item.checksState === undefined || input.item.checksState === null ? {} : { checksState: input.item.checksState }), @@ -1253,7 +1261,8 @@ export const make = Effect.gen(function* () { }), }) .pipe( - Effect.map((page): RepositoryBatch => { + observeRead, + Effect.map(({ value: page, observedAt }): RepositoryBatch => { // The boundary instant was asked for inclusively, so the rows already sent at it // come back with the slice. Dropping them here rather than asking for strictly // older is what keeps their neighbours at the same instant from being skipped. @@ -1269,7 +1278,7 @@ export const make = Effect.gen(function* () { key, entries: items .filter((item) => matchesRowFilters(item, input.filters, viewer)) - .map((item) => toEntry({ project, item, viewer })), + .map((item) => toEntry({ project, item, viewer, observedAt })), errors: [], truncated: page.truncated, nextCursor: @@ -1330,7 +1339,8 @@ export const make = Effect.gen(function* () { ? {} : { cursor: { updatedBefore: cursor.updatedBefore, delivered: cursor.delivered } }), }).pipe( - Effect.flatMap((page) => + observeRead, + Effect.flatMap(({ value: page, observedAt }) => Effect.flatMap(Clock.currentTimeMillis, (now) => { const rows = new Map>(); for (const [key, visibleAt] of searchVisibleAt) { @@ -1391,7 +1401,7 @@ export const make = Effect.gen(function* () { key: project.cursorKey, entries: items .filter((item) => matchesRowFilters(item, input.filters, viewer)) - .map((item) => toEntry({ project, item, viewer })), + .map((item) => toEntry({ project, item, viewer, observedAt })), errors: [], truncated: page.truncated, nextCursor: @@ -1558,7 +1568,8 @@ export const make = Effect.gen(function* () { : project.api.getChangeRequestSummary(providerInput); return read.pipe( Effect.mapError(toPullRequestError("summary")), - Effect.map((changeRequest): PullRequestSummary => ({ + observeRead, + Effect.map(({ value: changeRequest, observedAt }): PullRequestSummary => ({ provider: project.api.kind, projectId: project.project.id, repository: project.repository, @@ -1571,6 +1582,7 @@ export const make = Effect.gen(function* () { closedAt: changeRequest.closedAt ?? null, mergedAt: changeRequest.mergedAt ?? null, updatedAt: changeRequest.updatedAt, + observedAt, ...(changeRequest.isDraft === undefined ? {} : { isDraft: changeRequest.isDraft }), ...(changeRequest.author === undefined ? {} : { author: changeRequest.author }), ...(changeRequest.additions === undefined @@ -1641,12 +1653,12 @@ export const make = Effect.gen(function* () { host: project.host, number: input.number, }) - .pipe(Effect.mapError(toPullRequestError("detail"))), + .pipe(Effect.mapError(toPullRequestError("detail")), observeRead), viewerOf(project), ], { concurrency: 2 }, ).pipe( - Effect.map(([changeRequest, viewer]): PullRequestDetail => ({ + Effect.map(([{ value: changeRequest, observedAt }, viewer]): PullRequestDetail => ({ provider: project.api.kind, capabilities: project.api.capabilities, projectId: project.project.id, @@ -1671,6 +1683,7 @@ export const make = Effect.gen(function* () { baseBranch: changeRequest.baseBranch, createdAt: changeRequest.createdAt, updatedAt: changeRequest.updatedAt, + observedAt, mergedAt: changeRequest.mergedAt, closedAt: changeRequest.closedAt, reviewers: changeRequest.reviewers, @@ -2925,12 +2938,14 @@ export const make = Effect.gen(function* () { closedAt: detail.closedAt, mergedAt: detail.mergedAt, updatedAt: detail.updatedAt, + observedAt: detail.observedAt, }); const shouldReplaceHeldSummary = (key: string, next: PullRequestSummary) => { const current = lastGoodSummary.peek(key); if (current === undefined) return true; if (current.state === "merged" && next.state !== "merged") return false; - return next.updatedAt >= current.updatedAt; + if (next.updatedAt !== current.updatedAt) return next.updatedAt > current.updatedAt; + return (next.observedAt ?? -Infinity) >= (current.observedAt ?? -Infinity); }; const detail: PullRequestService["Service"]["detail"] = (input) => { const key = refCacheKey(input); diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 4591ac0437bf..05796283b9b5 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -121,6 +121,13 @@ describe("pull request list decoding", () => { { statusCheckRollup: [{ context: "ci/legacy", state: "ERROR" }] }, // Neither a pass, a failure nor a wait is no verdict rather than a green tick. { statusCheckRollup: [{ name: "lint", status: "COMPLETED", conclusion: "SKIPPED" }] }, + // Cancelled reads as failing here and in the detail header, so the two never flap. + { + statusCheckRollup: [ + { name: "lint", status: "COMPLETED", conclusion: "SUCCESS" }, + { name: "test", status: "COMPLETED", conclusion: "CANCELLED" }, + ], + }, { statusCheckRollup: [] }, {}, ]), @@ -132,6 +139,7 @@ describe("pull request list decoding", () => { "passing", "failing", null, + "failing", null, null, ]); diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index a011702d685e..8a1434224ad2 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -1447,8 +1447,9 @@ function toCheckEntries( * GitHub's own indicator reads: a run that has already gone red will not go green by finishing. * * Null rather than "passing" for a head commit with no checks at all, so a repository that runs - * none shows nothing instead of a green tick it never earned. Checks whose verdict is neither a - * pass, a failure nor a wait — skipped, cancelled, neutral — count towards neither. + * none shows nothing instead of a green tick it never earned. A cancelled run is a failure, as + * GitHub's own rollup and the client's detail rollup both read it; skipped and neutral count + * towards neither, so the row and the detail header never disagree about one head commit. * * Counted off the deduped checks rather than the raw rollup, so the word and the list under it * cannot disagree: the run a re-run replaced is not a verdict twice. A row with no name at all is @@ -1463,7 +1464,7 @@ function rollupChecksState( ...(raw ?? []).filter(isNamelessCheck).map((check) => toCheckStatus(check)), ]; if (statuses.length === 0) return null; - if (statuses.includes("failure")) return "failing"; + if (statuses.includes("failure") || statuses.includes("cancelled")) return "failing"; if (statuses.includes("pending") || statuses.includes("action-required")) return "pending"; return statuses.includes("success") ? "passing" : null; } diff --git a/apps/server/src/serverLogger.test.ts b/apps/server/src/serverLogger.test.ts index 59ca908cb4ad..cbb5056ed314 100644 --- a/apps/server/src/serverLogger.test.ts +++ b/apps/server/src/serverLogger.test.ts @@ -8,6 +8,8 @@ import * as Tracer from "effect/Tracer"; import * as HttpClient from "effect/unstable/http/HttpClient"; import * as HttpClientResponse from "effect/unstable/http/HttpClientResponse"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; + import * as ServerConfig from "./config.ts"; import { ServerLoggerLive } from "./serverLogger.ts"; @@ -51,10 +53,10 @@ const configLayer = (overrides: Partial) = otlpTracesUrl: undefined, otlpMetricsUrl: undefined, otlpLogsUrl: undefined, - otlpExportIntervalMs: 10_000, + otlpTracesExport: DEFAULT_SIGNAL_EXPORT, + otlpMetricsExport: DEFAULT_SIGNAL_EXPORT, + otlpLogsExport: DEFAULT_SIGNAL_EXPORT, otlpServiceName: "t3-server", - otlpHeaders: undefined, - otlpProtocol: "http/json", cwd: baseDir, baseDir, ...derivedPaths, @@ -155,12 +157,15 @@ describe("ServerLoggerLive", () => { }), ); - it.effect("sends the headers and wire format the rest of OTLP export already uses", () => + it.effect("sends the headers and wire format the log signal asked for", () => Effect.gen(function* () { const requests = yield* logThrough({ otlpLogsUrl: "https://collector.example.com/v1/logs", - otlpProtocol: "http/protobuf", - otlpHeaders: { "x-scope": "logs" }, + otlpLogsExport: { + ...DEFAULT_SIGNAL_EXPORT, + protocol: "http/protobuf", + headers: { "x-scope": "logs" }, + }, }); assert.lengthOf(requests, 1); diff --git a/apps/server/src/serverLogger.ts b/apps/server/src/serverLogger.ts index 389a9535efab..6d19907c20ca 100644 --- a/apps/server/src/serverLogger.ts +++ b/apps/server/src/serverLogger.ts @@ -12,13 +12,14 @@ export const ServerLoggerLive = Effect.gen(function* () { const config = yield* ServerConfig; const minimumLogLevelLayer = Layer.succeed(References.MinimumLogLevel, config.logLevel); + const logs = config.otlpLogsExport; const otlpLogger = config.otlpLogsUrl === undefined ? undefined : OtlpLogger.make({ url: config.otlpLogsUrl, - exportInterval: `${config.otlpExportIntervalMs} millis`, - headers: config.otlpHeaders, + exportInterval: `${logs.exportIntervalMs} millis`, + headers: logs.headers, resource: otlpResource(config), }); @@ -41,7 +42,7 @@ export const ServerLoggerLive = Effect.gen(function* () { { mergeWithExisting: false }, ).pipe( Layer.provide(OtlpExporter.layerFlusher), - Layer.provide(otlpSerializationLayer(config.otlpProtocol)), + Layer.provide(otlpSerializationLayer(logs.protocol)), ); return Layer.mergeAll(loggerLayer, minimumLogLevelLayer); diff --git a/apps/web/src/browser/browserRecording.ts b/apps/web/src/browser/browserRecording.ts index 5ea10c2b05d5..0bbe4bd7bf18 100644 --- a/apps/web/src/browser/browserRecording.ts +++ b/apps/web/src/browser/browserRecording.ts @@ -8,6 +8,8 @@ import { previewBridge } from "~/components/preview/previewBridge"; import { ensureClientSettingsHydrated, getClientSettings } from "~/hooks/useSettings"; import { appAtomRegistry } from "~/rpc/atomRegistry"; +import { createRecordingCompositor } from "./recordingCompositor"; + import { acquireBrowserSurfaceActivity } from "./browserSurfaceStore"; export class BrowserRecordingUnavailableError extends Schema.TaggedError()( @@ -123,6 +125,7 @@ interface ActiveRecording { releaseSurfaceActivity: (() => void) | null; stream: MediaStream | null; recorder: MediaRecorder | null; + compositor: Awaited>; savedBlob?: Blob; uploadPromise?: Promise; lifecycle: BrowserRecordingLifecycle; @@ -381,6 +384,8 @@ const captureTabMediaStreamWithTimeout = async ( }; const clearActiveRecording = (recording: ActiveRecording): void => { + recording.compositor?.dispose(); + recording.compositor = null; recording.releaseSurfaceActivity?.(); recording.releaseSurfaceActivity = null; if (activeRecordings.get(recording.tabId) !== recording) return; @@ -525,6 +530,7 @@ export async function startBrowserRecording( releaseSurfaceActivity, stream: null, recorder: null, + compositor: null, lifecycle: startingLifecycle, }; activeRecordings.set(tabId, recording); @@ -534,7 +540,8 @@ export async function startBrowserRecording( clearActiveRecording(recording); throw cause; }); - const frameRate = getClientSettings().browserRecordingFrameRate; + const settings = getClientSettings(); + const frameRate = settings.browserRecordingFrameRate; await waitForBrowserRecordingPaint(); const throwIfStartupCancelled = async (): Promise => { // Once a grant starts, a stop lets startup finish so the caller receives an artifact. @@ -614,7 +621,19 @@ export async function startBrowserRecording( let recorder: MediaRecorder; try { - recorder = createMediaRecorder(stream); + recording.compositor = await createRecordingCompositor( + stream, + { + showKeyPresses: settings.browserRecordingShowKeyPresses, + showMousePresses: settings.browserRecordingShowMousePresses, + frameRate, + }, + (listener) => + bridge.recording.onInput((event) => { + if (event.tabId === tabId) listener(event.input); + }), + ); + recorder = createMediaRecorder(recording.compositor?.stream ?? stream); recording.recorder = recorder; recorder.addEventListener("dataavailable", (event) => { if (event.data.size > 0) chunks.push(event.data); @@ -694,6 +713,8 @@ const finalizeBrowserRecording = async ( cause, }); } + recording.compositor?.dispose(); + recording.compositor = null; // Encoding has flushed; release native capture before materializing and saving the file. stopMediaStream(recording.stream); recording.stream = null; diff --git a/apps/web/src/browser/recordingCompositor.test.ts b/apps/web/src/browser/recordingCompositor.test.ts new file mode 100644 index 000000000000..d10e69d24663 --- /dev/null +++ b/apps/web/src/browser/recordingCompositor.test.ts @@ -0,0 +1,179 @@ +import type { DesktopPreviewRecordingInput } from "@t3tools/contracts"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import { createRecordingCompositor, RecordingDecorations } from "./recordingCompositor"; + +const primaryColor = "oklch(0.65 0.2 310)"; +vi.mock("./annotationTheme", () => ({ + readPreviewAnnotationTheme: () => ({ primary: "oklch(0.65 0.2 310)" }), +})); + +const options = { showKeyPresses: true, showMousePresses: true, frameRate: 30 }; +const pointer = ( + phase: "move" | "down" | "up" | "click", + x = 100, +): DesktopPreviewRecordingInput => ({ + type: "pointer", + phase, + x, + y: 80, + width: 800, + height: 600, +}); +const context = () => ({ + save: vi.fn(), + restore: vi.fn(), + beginPath: vi.fn(), + fill: vi.fn(), + stroke: vi.fn(), + ellipse: vi.fn(), + roundRect: vi.fn(), + fillText: vi.fn(), + drawImage: vi.fn(), + measureText: () => ({ width: 40 }), + globalAlpha: 1, + strokeStyle: "", + fillStyle: "", +}); + +describe("recording decorations", () => { + it("keeps rings aligned through dragging and stops following the cursor after release", () => { + const decorations = new RecordingDecorations(options, primaryColor); + const ctx = context(); + decorations.apply(pointer("down"), 0); + decorations.apply(pointer("move", 120), 10); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 1600, 1200, 10); + expect(ctx.ellipse.mock.calls[0]?.slice(0, 4)).toEqual([240, 160, 40, 40]); + expect(ctx.strokeStyle).toBe(primaryColor); + expect(ctx.fillStyle).toBe(primaryColor); + expect(decorations.nextRedraw(10)).toBeNull(); + decorations.apply(pointer("up", 130), 20); + decorations.apply(pointer("move", 300), 30); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 1600, 1200, 320); + expect(ctx.ellipse.mock.calls[1]?.slice(0, 4)).toEqual([260, 160, 50, 50]); + ctx.ellipse.mockClear(); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 1600, 1200, 620); + expect(ctx.ellipse).not.toHaveBeenCalled(); + expect(decorations.nextRedraw(620)).toBeNull(); + }); + + it("pulses agent clicks and clears decorations on blur or navigation", () => { + const decorations = new RecordingDecorations(options, primaryColor); + const ctx = context(); + decorations.apply(pointer("click"), 0); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 800, 600, 300); + expect(ctx.ellipse).toHaveBeenCalledOnce(); + decorations.apply({ type: "clear" }, 301); + ctx.ellipse.mockClear(); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 800, 600, 302); + expect(ctx.ellipse).not.toHaveBeenCalled(); + expect(decorations.nextRedraw(302)).toBeNull(); + }); + + it("holds shortcut badges until release, then expires them even on a static page", () => { + const decorations = new RecordingDecorations(options, primaryColor); + const ctx = context(); + const key = { type: "key" as const, label: "⌘C", held: true, width: 800 }; + decorations.apply(key, 0); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 1600, 1200, 5000); + expect(ctx.fillText.mock.calls[0]?.slice(0, 3)).toEqual(["⌘C", 800, 1098]); + decorations.apply({ ...key, held: false }, 5000); + expect(decorations.nextRedraw(5500)).toBe(400); + ctx.fillText.mockClear(); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 1600, 1200, 5900); + expect(ctx.fillText).not.toHaveBeenCalled(); + }); + + it("removes the previous key badge on password focus", () => { + const decorations = new RecordingDecorations(options, primaryColor); + const ctx = context(); + decorations.apply({ type: "key", label: "A", held: true, width: 800 }, 0); + decorations.apply({ type: "key", label: null, held: true, width: 800 }, 1); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 800, 600, 2); + expect(ctx.fillText).not.toHaveBeenCalled(); + }); + + it("honors independent opt-in flags", () => { + const decorations = new RecordingDecorations( + { ...options, showMousePresses: false }, + primaryColor, + ); + const ctx = context(); + decorations.apply(pointer("down"), 0); + decorations.apply({ type: "key", label: "⌘C", held: true, width: 800 }, 0); + decorations.draw(ctx as unknown as CanvasRenderingContext2D, 800, 600, 1); + expect(ctx.ellipse).not.toHaveBeenCalled(); + expect(ctx.fillText).toHaveBeenCalledOnce(); + }); +}); + +describe("detached recording compositor", () => { + afterEach(() => vi.unstubAllGlobals()); + + it("keeps native capture when both decorations are off", async () => { + vi.stubGlobal("document", { + createElement: () => { + throw new Error("must not allocate"); + }, + }); + expect( + await createRecordingCompositor( + {} as MediaStream, + { + ...options, + showKeyPresses: false, + showMousePresses: false, + }, + () => { + throw new Error("must not subscribe"); + }, + ), + ).toBeNull(); + }); + + it.each([false, true])( + "releases the detached output on disposal or playback failure (%s)", + async (failPlayback) => { + const ctx = context(); + const stop = vi.fn(); + const unsubscribe = vi.fn(); + const cancelFrame = vi.fn(); + const source = { + getVideoTracks: () => [{ getSettings: () => ({ width: 800, height: 600 }) }], + } as unknown as MediaStream; + const stream = { getTracks: () => [{ stop }] } as unknown as MediaStream; + const canvas = { width: 0, height: 0, getContext: () => ctx, captureStream: () => stream }; + const video = { + muted: false, + playsInline: false, + srcObject: null as MediaStream | null, + readyState: 2, + videoWidth: 800, + videoHeight: 600, + pause: vi.fn(), + play: async () => { + if (failPlayback) throw new Error("play failed"); + }, + requestVideoFrameCallback: () => 1, + cancelVideoFrameCallback: cancelFrame, + }; + vi.stubGlobal("document", { + createElement: (tag: string) => (tag === "canvas" ? canvas : video), + }); + vi.stubGlobal("window", { clearTimeout: vi.fn(), setTimeout: vi.fn() }); + const compositor = createRecordingCompositor(source, options, () => unsubscribe); + if (failPlayback) await expect(compositor).rejects.toThrow("play failed"); + else { + const result = await compositor; + expect(result?.stream).toBe(stream); + expect(ctx.drawImage).toHaveBeenCalledOnce(); + result?.dispose(); + result?.dispose(); + } + expect(stop).toHaveBeenCalledOnce(); + expect(unsubscribe).toHaveBeenCalledOnce(); + expect(cancelFrame).toHaveBeenCalledWith(1); + expect(video.srcObject).toBeNull(); + }, + ); +}); diff --git a/apps/web/src/browser/recordingCompositor.ts b/apps/web/src/browser/recordingCompositor.ts new file mode 100644 index 000000000000..352154bd9aef --- /dev/null +++ b/apps/web/src/browser/recordingCompositor.ts @@ -0,0 +1,190 @@ +import type { DesktopPreviewRecordingInput } from "@t3tools/contracts"; + +import { readPreviewAnnotationTheme } from "./annotationTheme"; + +interface RecordingDecorationOptions { + readonly showKeyPresses: boolean; + readonly showMousePresses: boolean; + readonly frameRate: number; +} + +/** Decorates a detached canvas; no recording UI is inserted into the preview page. */ +export async function createRecordingCompositor( + source: MediaStream, + options: RecordingDecorationOptions, + subscribe: (listener: (input: DesktopPreviewRecordingInput) => void) => () => void, +) { + if (!options.showKeyPresses && !options.showMousePresses) return null; + const canvas = document.createElement("canvas"); + const context = canvas.getContext("2d", { alpha: false }); + if (!context) throw new Error("Recording canvas is unavailable."); + const video = document.createElement("video"); + video.muted = true; + video.playsInline = true; + video.srcObject = source; + const settings = source.getVideoTracks()[0]?.getSettings(); + canvas.width = settings?.width ?? 1920; + canvas.height = settings?.height ?? 1080; + const decorations = new RecordingDecorations(options, readPreviewAnnotationTheme().primary); + let disposed = false; + let frameId: number | undefined; + let timer: number | undefined; + const draw = () => { + if (disposed || video.readyState < 2) return; + const width = video.videoWidth || canvas.width; + const height = video.videoHeight || canvas.height; + if (canvas.width !== width) canvas.width = width; + if (canvas.height !== height) canvas.height = height; + context.drawImage(video, 0, 0, width, height); + const now = performance.now(); + decorations.draw(context, width, height, now); + window.clearTimeout(timer); + const next = decorations.nextRedraw(now); + if (next !== null) timer = window.setTimeout(draw, next); + }; + const frame = () => { + if (disposed) return; + draw(); + frameId = video.requestVideoFrameCallback(frame); + }; + const output = canvas.captureStream(options.frameRate); + let unsubscribe: (() => void) | undefined; + const dispose = () => { + if (disposed) return; + disposed = true; + unsubscribe?.(); + window.clearTimeout(timer); + if (frameId !== undefined) video.cancelVideoFrameCallback(frameId); + video.pause(); + video.srcObject = null; + for (const track of output.getTracks()) track.stop(); + }; + try { + unsubscribe = subscribe((input) => { + decorations.apply(input, performance.now()); + draw(); + }); + frameId = video.requestVideoFrameCallback(frame); + await video.play(); + draw(); + return { stream: output, dispose }; + } catch (error) { + dispose(); + throw error; + } +} + +/** Keeps input timing and coordinates independent of native video frame delivery. */ +export class RecordingDecorations { + private ring: { + x: number; + y: number; + width: number; + height: number; + held: boolean; + releasedAt: number | null; + } | null = null; + private key: { label: string; width: number; expiresAt: number | null } | null = null; + + constructor( + private readonly options: RecordingDecorationOptions, + private readonly primaryColor: string, + ) {} + + apply(input: DesktopPreviewRecordingInput, now: number) { + if (input.type === "clear") { + this.ring = null; + this.key = null; + } else if (input.type === "key" && this.options.showKeyPresses) { + this.key = input.label + ? { label: input.label, width: input.width, expiresAt: input.held ? null : now + 900 } + : null; + } else if (input.type === "pointer" && this.options.showMousePresses) { + if (input.phase === "down" || input.phase === "click") { + this.ring = { + x: input.x, + y: input.y, + width: input.width, + height: input.height, + held: input.phase === "down", + releasedAt: input.phase === "click" ? now : null, + }; + } else if (this.ring?.held) { + this.ring = { + ...this.ring, + x: input.x, + y: input.y, + width: input.width, + height: input.height, + held: input.phase !== "up", + releasedAt: input.phase === "up" ? now : null, + }; + } + } + } + + nextRedraw(now: number): number | null { + if ( + this.ring?.releasedAt !== null && + this.ring?.releasedAt !== undefined && + now < this.ring.releasedAt + 600 + ) { + return 1000 / this.options.frameRate; + } + return this.key?.expiresAt !== null && + this.key?.expiresAt !== undefined && + now < this.key.expiresAt + ? this.key.expiresAt - now + : null; + } + + draw(context: CanvasRenderingContext2D, width: number, height: number, now: number) { + // Guest coordinates are CSS pixels; native frames include zoom and display scale. + const scale = width / (this.key?.width ?? this.ring?.width ?? 1280); + const ring = this.ring; + if (ring && (ring.held || (ring.releasedAt !== null && now < ring.releasedAt + 600))) { + const progress = ring.releasedAt === null ? 0 : Math.min(1, (now - ring.releasedAt) / 600); + context.save(); + const opacity = 0.9 * (1 - progress); + context.strokeStyle = this.primaryColor; + context.fillStyle = this.primaryColor; + context.lineWidth = 2 * scale; + context.beginPath(); + context.ellipse( + (ring.x * width) / ring.width, + (ring.y * height) / ring.height, + ((20 * width) / ring.width) * (1 + progress * 0.5), + ((20 * height) / ring.height) * (1 + progress * 0.5), + 0, + 0, + Math.PI * 2, + ); + context.globalAlpha = opacity * 0.15; + context.fill(); + context.globalAlpha = opacity; + context.stroke(); + context.restore(); + } + const key = this.key; + if (key && (key.expiresAt === null || now < key.expiresAt)) { + context.save(); + context.font = `500 ${26 * scale}px system-ui, sans-serif`; + const badgeWidth = Math.min( + width - 32 * scale, + context.measureText(key.label).width + 36 * scale, + ); + const badgeHeight = 54 * scale; + const left = (width - badgeWidth) / 2; + const top = height - 24 * scale - badgeHeight; + context.fillStyle = "rgba(32,32,34,.86)"; + context.beginPath(); + context.roundRect(left, top, badgeWidth, badgeHeight, 14 * scale); + context.fill(); + context.fillStyle = "white"; + context.textAlign = "center"; + context.textBaseline = "middle"; + context.fillText(key.label, width / 2, top + badgeHeight / 2, badgeWidth - 24 * scale); + context.restore(); + } + } +} diff --git a/apps/web/src/components/BranchToolbar.tsx b/apps/web/src/components/BranchToolbar.tsx index eae4d87f0ed7..169e2fe46788 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -229,6 +229,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ { if (autoEnvironmentLabel) onAutoEnvironment?.(); }} @@ -246,6 +247,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ key={env.environmentId} disabled={envLocked} value={env.environmentId} + closeOnClick > @@ -270,7 +272,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ onEnvModeChange(value as EnvMode); }} > - + {activeWorktreePath ? ( @@ -282,14 +284,14 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ - + {resolveEnvModeLabel("worktree")} {previousWorktreeLabel ? ( - + {previousWorktreeLabel} diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index abebe4841cbd..8da35ee35cee 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -863,7 +863,10 @@ function MarkdownDetails({ {summary} -
+
{content}
@@ -3332,7 +3335,7 @@ function ChatMarkdown({
({ + class: cn( + "composer-tiptap block max-h-50 min-h-17.5 w-full overflow-y-auto whitespace-pre-wrap wrap-break-word bg-transparent leading-relaxed text-foreground focus:outline-none", + className, + ), + "data-testid": "composer-editor", + "data-composer-rich-text": richText ? "true" : "false", + "aria-placeholder": placeholder, + }), + [className, placeholder, richText], + ); + const editor = useEditor( { extensions: [ @@ -783,15 +796,7 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { ), editable: !disabled, editorProps: { - attributes: { - class: cn( - "composer-tiptap block max-h-50 min-h-17.5 w-full overflow-y-auto whitespace-pre-wrap wrap-break-word bg-transparent leading-relaxed text-foreground focus:outline-none", - className, - ), - "data-testid": "composer-editor", - "data-composer-rich-text": richText ? "true" : "false", - "aria-placeholder": placeholder, - }, + attributes: editorAttributes, handleKeyDown: (view, event) => { if ( isMacPlatform(navigator.platform) && @@ -982,6 +987,17 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { editorHolder.current = editor; }, [editor]); + // Tiptap forwards option changes to the view from a passive effect, so a + // class change here would reach the ProseMirror element one tick after + // React commits. The chat composer measures its resting and expanded + // geometry in layout effects that run first, and it clamps the prompt + // through `className`, so the attributes are pushed to the view here for + // those measurements to see the layout they are about to reserve for. + useLayoutEffect(() => { + if (!editor?.isInitialized) return; + editor.view.setProps({ attributes: editorAttributes }); + }, [editor, editorAttributes]); + const readSnapshot = useCallback(() => { const snapshot = snapshotRef.current; if (!editor) return snapshot; diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index 0f1bf3d352e4..35d3f6a717b0 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -84,7 +84,16 @@ import { } from "~/components/ui/dialog"; import { Group, GroupSeparator } from "~/components/ui/group"; import { Input } from "~/components/ui/input"; -import { Menu, MenuItem, MenuPopup, MenuTrigger } from "~/components/ui/menu"; +import { + Menu, + MenuItem, + MenuItemLabel, + MenuPopup, + MenuSub, + MenuSubTrigger, + MenuSubPopup, + MenuTrigger, +} from "~/components/ui/menu"; import { Popover, PopoverPopup, PopoverTrigger } from "~/components/ui/popover"; import { ScrollArea } from "~/components/ui/scroll-area"; import { Textarea } from "~/components/ui/textarea"; @@ -122,6 +131,11 @@ import { getSourceControlPresentation } from "~/sourceControlPresentation"; import { useOpenLink } from "~/browser/useOpenLink"; interface GitActionsControlProps { + /** + * "toolbar" is the standalone split button, "menu" renders as items inside a parent menu for + * narrow headers, and "panel" is the thread details panel's full-width row. + */ + presentation?: "toolbar" | "menu" | "panel"; gitCwd: string | null; activeThreadRef: ScopedThreadRef | null; draftId?: DraftId; @@ -130,7 +144,6 @@ interface GitActionsControlProps { * place it against, in which case it still opens in the browser. */ onOpenPullRequest?: ((number: number) => void) | undefined; - displayMode?: "toolbar" | "panel"; onOpenChanges?: () => void; } @@ -365,19 +378,18 @@ function GitQuickActionIcon({ SourceControlIcon: ReturnType["Icon"]; className?: string; }) { - const iconClassName = className; - if (quickAction.kind === "open_publish") return ; - if (quickAction.kind === "run_pull") return ; + if (quickAction.kind === "open_publish") return ; + if (quickAction.kind === "run_pull") return ; if (quickAction.kind === "run_action") { - if (quickAction.action === "commit") return ; + if (quickAction.action === "commit") return ; if (quickAction.action === "push" || quickAction.action === "commit_push") { - return ; + return ; } - return ; + return ; } - if (quickAction.label === "Commit") return ; - if (quickAction.label === "Push") return ; - return ; + if (quickAction.label === "Commit") return ; + if (quickAction.label === "Push") return ; + return ; } function GitActionElapsedTime({ @@ -1061,13 +1073,13 @@ function PublishRepositoryDialog(props: PublishRepositoryDialogProps) { } export default function GitActionsControl({ + presentation = "toolbar", gitCwd, activeThreadRef, draftId, - displayMode = "toolbar", onOpenChanges, }: GitActionsControlProps) { - const isPanel = displayMode === "panel"; + const isPanel = presentation === "panel"; const ActionGroup = isPanel ? "div" : Group; const panelAnchorRef = useRef(null); const updateThreadMetadata = useAtomCommand( @@ -1622,33 +1634,166 @@ export default function GitActionsControl({ const canPublishRepository = isRepo && gitStatusForActions !== null && !hasPrimaryRemote; + const initializeGit = () => { + void (async () => { + const result = await initAction.run(); + if (result._tag === "Success" || isAtomCommandInterrupted(result)) { + return; + } + const error = squashAtomCommandFailure(result); + toastManager.add( + stackedThreadToast({ + type: "error", + title: "Git initialization failed", + description: error instanceof Error ? error.message : "An error occurred.", + ...(threadToastData !== undefined ? { data: threadToastData } : {}), + }), + ); + })(); + }; + const gitItems = ( + <> + {gitActionMenuItems.map((item) => { + const disabledReason = getMenuActionDisabledReason({ + item, + gitStatus: gitStatusForActions, + isBusy: isGitActionRunning, + hasPrimaryRemote, + }); + if (item.disabled && disabledReason && presentation === "menu") { + return ( +
+ + + {item.label} + +

{disabledReason}

+
+ ); + } + if (item.disabled && disabledReason) { + return ( + + } + > + + + {item.label} + + + + {disabledReason} + + + ); + } + + return ( + { + openDialogForMenuItem(item); + }} + > + + {item.label} + + ); + })} + {canPublishRepository ? ( + { + setIsPublishDialogOpen(true); + }} + > + + Publish repository... + + ) : null} + {gitStatusForActions?.refName === null && ( +

+ Detached HEAD: create and check out a branch to enable push and pull request actions. +

+ )} + {gitStatusForActions && + gitStatusForActions.refName !== null && + !gitStatusForActions.hasWorkingTreeChanges && + gitStatusForActions.behindCount > 0 && + gitStatusForActions.aheadCount === 0 && ( +

Behind upstream. Pull/rebase first.

+ )} + {gitStatusError &&

{gitStatusError}

} + + ); + if (!gitCwd) return null; return ( <> - {!isRepo ? ( + {presentation === "menu" ? ( + !isRepo ? ( + + + + {initAction.isPending ? "Initializing..." : "Initialize Git"} + + + ) : ( + <> + + + {quickAction.label} + + {quickActionDisabledReason && ( +

+ {quickActionDisabledReason} +

+ )} + { + if (open) requestVcsStatusRefresh(refreshVcsStatus, activeEnvironmentId, gitCwd); + }} + > + + + Git actions + + {gitItems} + + + ) + ) : !isRepo ? ( + + + ); + })} + {importMenuItems} + + + {isPanel ? "Add project script" : "Add action"} + + + ); + return ( <> - {primaryScript ? ( + {presentation === "menu" ? ( + <> + {primaryScript && ( + onRunScript(primaryScript)} + > + + Run {primaryScript.name} + + {shortcutLabelForCommand(keybindings, commandForProjectScript(primaryScript.id))} + + + )} + {primaryScript || importableScripts.length > 0 ? ( + + setActionsMenuOpen({ presentation, scripts: open, imports: false }) + } + > + + + Project actions + + + {scriptItems} + + + ) : ( + + + Add project action… + + )} + + ) : primaryScript ? ( setActionsMenuOpen({ scripts: open, imports: false })} + onOpenChange={(open) => + setActionsMenuOpen({ presentation, scripts: open, imports: false }) + } > - {scripts.map((script) => { - const shortcutLabel = shortcutLabelForCommand( - keybindings, - commandForProjectScript(script.id), - ); - return ( - onRunScript(script)} - > - - - {script.runOnWorktreeCreate ? `${script.name} (setup)` : script.name} - - - {shortcutLabel && ( - - {shortcutLabel} - - )} - - - - ); - })} - {importMenuItems} - - - {isPanel ? "Add project script" : "Add action"} - + {scriptItems} @@ -308,7 +382,7 @@ export default function ProjectScriptsControl({ variant="ghost" className={THREAD_DETAILS_PANEL_SPLIT_PRIMARY_CLASS} aria-label="Project actions" - onClick={() => setActionsMenuOpen({ scripts: false, imports: true })} + onClick={() => setActionsMenuOpen({ presentation, scripts: false, imports: true })} > Actions @@ -317,7 +391,9 @@ export default function ProjectScriptsControl({ setActionsMenuOpen({ scripts: false, imports: open })} + onOpenChange={(open) => + setActionsMenuOpen({ presentation, scripts: false, imports: open }) + } > setActionsMenuOpen({ scripts: false, imports: open })} + onOpenChange={(open) => + setActionsMenuOpen({ presentation, scripts: false, imports: open }) + } > } diff --git a/apps/web/src/components/RightPanelTabs.tsx b/apps/web/src/components/RightPanelTabs.tsx index e657cf7b04e4..e3ce573e0026 100644 --- a/apps/web/src/components/RightPanelTabs.tsx +++ b/apps/web/src/components/RightPanelTabs.tsx @@ -34,6 +34,7 @@ import { type ReactNode, useCallback, useEffect, + useMemo, useRef, useState, } from "react"; @@ -63,7 +64,11 @@ import { PanelTabCloseButton } from "~/components/ui/panel-tab-close-button"; import { faviconUrlForOrigin } from "~/lib/favicon"; import { useTheme } from "~/hooks/useTheme"; import type { PreviewPanelInlineSize } from "~/hooks/usePreviewPanelInlineSize"; -import { pullRequestEnvironment } from "~/state/pullRequests"; +import { + newestPullRequestSummary, + pullRequestEnvironment, + useSharedPullRequestSummary, +} from "~/state/pullRequests"; import { useEnvironmentQuery } from "~/state/query"; import { COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS } from "~/workspaceTitlebar"; @@ -802,19 +807,26 @@ function PullRequestSurfaceIcon({ }, }), ).data; + const reference = useMemo( + () => ({ + projectId: surface.projectId as ProjectId, + repository: surface.repository, + number: surface.number, + }), + [surface.projectId, surface.repository, surface.number], + ); + const sharedSummary = useSharedPullRequestSummary(resolvedEnvironmentId, reference, null); // The compact tab intentionally shows lifecycle and draft state only. Conflict warnings have // their own presentation on surfaces that have mergeability, while this tab stays stable as // detail data arrives. - const status = - linkedSnapshot !== null - ? linkedSnapshot - : detail === null - ? (seed ?? null) - : { state: detail.state, isDraft: detail.isDraft }; + const status = linkedSnapshot ?? newestPullRequestSummary(detail, sharedSummary) ?? seed ?? null; if (status === null) { return ; } - const presentation = resolvePullRequestState({ state: status.state, isDraft: status.isDraft }); + const presentation = resolvePullRequestState({ + state: status.state, + isDraft: status.isDraft ?? detail?.isDraft ?? seed?.isDraft ?? false, + }); return ; } diff --git a/apps/web/src/components/ThreadStatusIndicators.tsx b/apps/web/src/components/ThreadStatusIndicators.tsx index c3f7a5241d7a..211f75d92532 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -91,15 +91,24 @@ export function useLinkedThreadPullRequest( ); const fallback = current === null ? ((!supportsLinks ? linkedPullRequest : null) ?? branchPullRequest) : null; - const host = fallback == null ? undefined : parseChangeRequestUrl(fallback.url)?.host; - const reference = - fallback == null ? null : { ...fallback, ...(host === undefined ? {} : { host }) }; + // Stable per link: the shared summary effect keys on this object, and a sidebar row must not + // touch the cache on every render. + const reference = useMemo(() => { + if (fallback == null) return null; + const host = parseChangeRequestUrl(fallback.url)?.host; + return { ...fallback, ...(host === undefined ? {} : { host }) }; + }, [fallback]); const queried = useEnvironmentQuery( !enabled || environmentId === null || reference === null ? null : linkedPullRequestDetailAtom({ environmentId, input: reference }), - ).data; - const detail = useSharedPullRequestSummary(environmentId, reference, queried); + ); + const detail = useSharedPullRequestSummary( + environmentId, + reference, + queried.data, + queried.dataUpdatedAt, + ); return useMemo(() => { if (current !== null) return linkedPullRequestSnapshotStatus(current); diff --git a/apps/web/src/components/WorkspaceBreadcrumb.tsx b/apps/web/src/components/WorkspaceBreadcrumb.tsx index f1c3d8b15d04..015e381aa251 100644 --- a/apps/web/src/components/WorkspaceBreadcrumb.tsx +++ b/apps/web/src/components/WorkspaceBreadcrumb.tsx @@ -1,4 +1,4 @@ -import type { ReactNode } from "react"; +import type { ComponentProps, ReactNode } from "react"; import { cn } from "../lib/utils"; @@ -26,6 +26,23 @@ interface WorkspaceBreadcrumbItemProps { readonly current?: boolean; } +export function WorkspaceBreadcrumbText({ children, className, ...props }: ComponentProps<"span">) { + return ( + + {children} + + ); +} + export function WorkspaceBreadcrumbItem({ children, className, @@ -45,10 +62,16 @@ export function WorkspaceBreadcrumbItem({ ); } -export function WorkspaceBreadcrumbSeparator({ className }: { readonly className?: string }) { +export function WorkspaceBreadcrumbSeparator({ + className, + children = "/", +}: { + readonly className?: string; + readonly children?: ReactNode; +}) { return ( ); } diff --git a/apps/web/src/components/chat/AssistantCitationChip.test.tsx b/apps/web/src/components/chat/AssistantCitationChip.test.tsx new file mode 100644 index 000000000000..7d664f237983 --- /dev/null +++ b/apps/web/src/components/chat/AssistantCitationChip.test.tsx @@ -0,0 +1,156 @@ +import { + ASSISTANT_CITATION_MAX_COMMENT_LENGTH, + EnvironmentId, + MessageId, + ThreadId, +} from "@t3tools/contracts"; +import { act, useState, type ReactNode } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import type { AssistantCitationSourceAnchor } from "~/lib/assistantTextSelection"; + +const mocks = vi.hoisted(() => ({ observeSource: vi.fn(), dispose: vi.fn() })); +vi.mock("./AssistantCitationSource", () => ({ + observeAssistantCitationCommentSource: mocks.observeSource, +})); +vi.mock("@tanstack/react-router", () => ({ + useNavigate: () => vi.fn(), + Link: ({ children }: { children: ReactNode }) => {children}, +})); +// Keep the real chip/editor lifecycle while replacing DOM positioning and floating layers. +vi.mock("../ui/tooltip", () => ({ + Tooltip: ({ children }: { children: ReactNode }) => <>{children}, + TooltipTrigger: ({ render }: { render: ReactNode }) => render, + TooltipPopup: ({ children }: { children: ReactNode }) => <>{children}, +})); +vi.mock("../ui/popover", () => ({ + Popover: ({ children }: { children: ReactNode }) => <>{children}, + PopoverTrigger: ({ children }: { children: ReactNode }) => , + PopoverPopup: ({ children }: { children: ReactNode }) => <>{children}, +})); +vi.mock("../ui/button", () => ({ + Button: (props: React.ComponentProps<"button">) => ); + // The composer hero is a sentence, so the heading's accessible name must be + // a complete sentence too. The project picker is a control rendered inline + // in the h1; without an explicit label its widget state bleeds into the + // announced phrase. + const headingLabel = hasResolvedProject + ? `What should we build in ${activeProjectDisplayName}?` + : canChooseProject + ? `${activeProjectDisplayName ?? "Choose a project"} to start` + : "Add a project to start"; + return ( -

+

{hasResolvedProject ? ( <>What should we build in {projectSelector}? ) : canChooseProject ? ( diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 704798c1ae61..dd189cb93090 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -4108,7 +4108,7 @@ const CollapsibleUserMessageBody = memo(function CollapsibleUserMessageBody(prop const UserMessageBody = memo(function UserMessageBody(props: { text: string; - renderContextReference: (reference: ChatMarkdownContextReference) => ReactNode; + renderContextReference?: (reference: ChatMarkdownContextReference) => ReactNode; skills: ReadonlyArray>; markdownCwd: string | undefined; }) { diff --git a/apps/web/src/components/chat/OpenInPicker.tsx b/apps/web/src/components/chat/OpenInPicker.tsx index c9cf5f8cdf9f..c7193753e0ee 100644 --- a/apps/web/src/components/chat/OpenInPicker.tsx +++ b/apps/web/src/components/chat/OpenInPicker.tsx @@ -15,10 +15,20 @@ import { useRemoteOpenState, } from "../../remoteOpen"; import { useEnvironment } from "../../state/environments"; -import { ChevronDownIcon, FolderClosedIcon } from "lucide-react"; +import { ChevronDownIcon, FolderClosedIcon, SquareArrowOutUpRightIcon } from "lucide-react"; import { Button } from "../ui/button"; import { Group, GroupSeparator } from "../ui/group"; -import { Menu, MenuItem, MenuPopup, MenuShortcut, MenuTrigger } from "../ui/menu"; +import { + Menu, + MenuItem, + MenuItemLabel, + MenuPopup, + MenuShortcut, + MenuSub, + MenuSubTrigger, + MenuSubPopup, + MenuTrigger, +} from "../ui/menu"; import { AntigravityIcon, CursorIcon, @@ -196,19 +206,23 @@ export const OpenInPicker = memo(function OpenInPicker({ keybindings, availableEditors, openInCwd, + presentation = "toolbar", compact = false, enableShortcut = true, - displayMode = "toolbar", }: { environmentId: EnvironmentId; keybindings: ResolvedKeybindingsConfig; availableEditors: ReadonlyArray; openInCwd: string | null; + /** + * "toolbar" is the standalone split button, "menu" renders as items inside a parent menu for + * narrow headers, and "panel" is the thread details panel's full-width row. + */ + presentation?: "toolbar" | "menu" | "panel"; compact?: boolean; enableShortcut?: boolean; - displayMode?: "toolbar" | "panel"; }) { - const isPanel = displayMode === "panel"; + const isPanel = presentation === "panel"; const ActionGroup = isPanel ? "div" : Group; const panelAnchorRef = useRef(null); const openInEditorMutation = useAtomCommand(shellEnvironment.openInEditor, "open in editor"); @@ -289,6 +303,68 @@ export const OpenInPicker = memo(function OpenInPicker({ }, [enableShortcut, keybindings, openInCwd, openInEditor, preferredEditor]); const primaryLabel = isPanel ? `Open in ${primaryOption?.label ?? "editor"}` : "Open"; + const editorItems = ( + <> + {remote.mode === "remote-unavailable" ? ( + + No SSH route to {environmentLabel} + + ) : ( + <> + {options.length === 0 && ( + + No installed editors found + + )} + {options.map(({ label, Icon, value, kind }) => ( + openInEditor(value)} + > + + ))} + {remote.mode === "remote-links" && !remoteHintSeen && ( + + Opens over SSH. Needs your key on {environmentLabel} + + )} + + )} + + ); + if (presentation === "menu") { + return ( + <> + {primaryOption && ( + openInEditor(preferredEditor)} + > + + Open in {primaryOption.label} + {openFavoriteEditorShortcutLabel && ( + {openFavoriteEditorShortcutLabel} + )} + + )} + + + + Open in… + + {editorItems} + + + ); + } + return ( - {remote.mode === "remote-unavailable" ? ( - No SSH route to {environmentLabel} - ) : ( - <> - {options.length === 0 && No installed editors found} - {options.map(({ label, Icon, value, kind }) => ( - openInEditor(value)}> - - ))} - {remote.mode === "remote-links" && !remoteHintSeen && ( - Opens over SSH. Needs your key on {environmentLabel} - )} - - )} + {editorItems}

diff --git a/apps/web/src/components/chat/ThreadDetailsPanel.test.tsx b/apps/web/src/components/chat/ThreadDetailsPanel.test.tsx index fc1ff4e3571e..ff9bfefc8f46 100644 --- a/apps/web/src/components/chat/ThreadDetailsPanel.test.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPanel.test.tsx @@ -82,7 +82,7 @@ describe("ThreadDetailsPanel", () => { expect(testState.useT3ProjectFileScripts).toHaveBeenCalledWith(environmentId, gitCwd); expect(testState.projectScriptsControl).toHaveBeenCalledWith( expect.objectContaining({ - displayMode: "panel", + presentation: "panel", scripts: [], fileScripts, }), diff --git a/apps/web/src/components/chat/ThreadDetailsPanel.tsx b/apps/web/src/components/chat/ThreadDetailsPanel.tsx index 7486e818a62d..fec11b1636c4 100644 --- a/apps/web/src/components/chat/ThreadDetailsPanel.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPanel.tsx @@ -200,13 +200,13 @@ export function ThreadDetailsPanel(props: ThreadDetailsPanelProps) { keybindings={props.keybindings} availableEditors={props.availableEditors} openInCwd={props.gitCwd} - displayMode="panel" + presentation="panel" /> ) : null} {props.activeProjectScripts ? ( = detailQuery.dataUpdatedAt + : checksQuery.data !== null && + (checksQuery.dataUpdatedAt ?? 0) >= (detailQuery.dataUpdatedAt ?? 0) ? { ...detailQuery.data, ...checksQuery.data } : detailQuery.data; const open = reference !== null && (detail?.state ?? pr?.state) === "open"; diff --git a/apps/web/src/components/chat/assistantCitationCommentDismissal.test.ts b/apps/web/src/components/chat/assistantCitationCommentDismissal.test.ts new file mode 100644 index 000000000000..1f21677a5a82 --- /dev/null +++ b/apps/web/src/components/chat/assistantCitationCommentDismissal.test.ts @@ -0,0 +1,80 @@ +import { ASSISTANT_CITATION_MAX_COMMENT_LENGTH } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { resolveAssistantCitationCommentDismissal } from "./assistantCitationCommentDismissal"; + +describe("resolveAssistantCitationCommentDismissal", () => { + it("commits typed text when the popover is dismissed by clicking away", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "outside-press", + draft: "needs a retry", + savedComment: undefined, + }), + ).toEqual({ kind: "commit", comment: "needs a retry" }); + }); + + it("commits an edited comment when focus leaves the popover", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "focus-out", + draft: "second thought", + savedComment: "first thought", + }), + ).toEqual({ kind: "commit", comment: "second thought" }); + }); + + it("closes without saving when nothing changed", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "outside-press", + draft: null, + savedComment: "kept", + }), + ).toEqual({ kind: "close" }); + expect( + resolveAssistantCitationCommentDismissal({ + reason: "outside-press", + draft: " kept ", + savedComment: "kept", + }), + ).toEqual({ kind: "close" }); + expect( + resolveAssistantCitationCommentDismissal({ + reason: "outside-press", + draft: "kept", + savedComment: " kept ", + }), + ).toEqual({ kind: "close" }); + }); + + it("clears a comment when the draft was emptied", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "trigger-press", + draft: "", + savedComment: "old", + }), + ).toEqual({ kind: "commit", comment: "" }); + }); + + it("keeps Escape as an explicit discard", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "escape-key", + draft: "unsaved", + savedComment: undefined, + }), + ).toEqual({ kind: "close" }); + }); + + it("keeps the popover open instead of dropping an over-length draft", () => { + expect( + resolveAssistantCitationCommentDismissal({ + reason: "outside-press", + draft: "x".repeat(ASSISTANT_CITATION_MAX_COMMENT_LENGTH + 1), + savedComment: undefined, + }), + ).toEqual({ kind: "keep-open" }); + }); +}); diff --git a/apps/web/src/components/chat/assistantCitationCommentDismissal.ts b/apps/web/src/components/chat/assistantCitationCommentDismissal.ts new file mode 100644 index 000000000000..a6fe2ca3bcb6 --- /dev/null +++ b/apps/web/src/components/chat/assistantCitationCommentDismissal.ts @@ -0,0 +1,21 @@ +import { ASSISTANT_CITATION_MAX_COMMENT_LENGTH } from "@t3tools/contracts"; + +export type AssistantCitationCommentDismissal = + | { kind: "commit"; comment: string } + | { kind: "close" } + | { kind: "keep-open" }; + +export function resolveAssistantCitationCommentDismissal({ + reason, + draft, + savedComment, +}: { + reason: string; + draft: string | null; + savedComment: string | undefined; +}): AssistantCitationCommentDismissal { + if (reason === "escape-key" || draft === null) return { kind: "close" }; + if (draft.trim() === (savedComment ?? "").trim()) return { kind: "close" }; + if (draft.length > ASSISTANT_CITATION_MAX_COMMENT_LENGTH) return { kind: "keep-open" }; + return { kind: "commit", comment: draft }; +} diff --git a/apps/web/src/components/preview/AgentBrowserCursor.tsx b/apps/web/src/components/preview/AgentBrowserCursor.tsx index bc89daee4595..408f268fa0c4 100644 --- a/apps/web/src/components/preview/AgentBrowserCursor.tsx +++ b/apps/web/src/components/preview/AgentBrowserCursor.tsx @@ -24,7 +24,6 @@ export function AgentBrowserCursor(props: { return ( (null); + const active = inactiveSequence !== event.sequence; useEffect(() => { - const timeout = window.setTimeout(() => setActive(false), CURSOR_ACTIVE_MS); + const timeout = window.setTimeout(() => setInactiveSequence(event.sequence), CURSOR_ACTIVE_MS); return () => window.clearTimeout(timeout); - }, []); + }, [event.sequence]); return (
({ pictureInPicture: false, showEmptyState: false, loading: false, + serverEpoch: null as string | null, + recordingTabIds: new Set(), + recordingRuntimeTabId: null as string | null, recordVisitForThread: vi.fn(), })); @@ -98,6 +101,7 @@ vi.mock("~/previewStateStore", () => ({ updatePreviewServerSnapshot: vi.fn(), useThreadPreviewState: () => ({ activeTabId: "tab-1", + serverEpoch: mocks.serverEpoch, desktopByTabId: { "tab-1": { hasWebContents: true, @@ -146,11 +150,11 @@ vi.mock("~/state/use-atom-command", () => ({ })); vi.mock("~/browser/browserRecording", () => ({ - findActiveBrowserRecordingRuntimeTabId: vi.fn(() => null), + findActiveBrowserRecordingRuntimeTabId: () => mocks.recordingRuntimeTabId, isBrowserRecordingStartCancelledError: vi.fn(() => false), startBrowserRecording: vi.fn(), stopBrowserRecording: vi.fn(), - useActiveBrowserRecordingTabIds: () => new Set(), + useActiveBrowserRecordingTabIds: () => mocks.recordingTabIds, })); vi.mock("~/browser/browserSurfaceStore", () => ({ @@ -244,7 +248,9 @@ vi.mock("./PreviewMoreMenu", () => ({ })); vi.mock("./PreviewUnreachable", () => ({ PreviewUnreachable: () => null })); vi.mock("./ZoomIndicator", () => ({ ZoomIndicator: () => null })); -vi.mock("./AgentBrowserCursor", () => ({ AgentBrowserCursor: () => null })); +vi.mock("./AgentBrowserCursor", () => ({ + AgentBrowserCursor: () => createElement("agent-cursor"), +})); vi.mock("~/browser/BrowserSurfaceSlot", () => ({ BrowserSurfaceSlot: () => null })); vi.mock("./usePreviewSession", () => ({ usePreviewSession: vi.fn() })); @@ -344,9 +350,38 @@ describe("PreviewView navigation", () => { mocks.pictureInPicture = false; mocks.showEmptyState = false; mocks.loading = false; + mocks.serverEpoch = null; + mocks.recordingTabIds = new Set(); + mocks.recordingRuntimeTabId = null; mocks.recordVisitForThread.mockClear(); }); + it("shows the cursor in a replacement browser while the old instance still records", async () => { + const document = installTestDom(); + const { createRoot } = await import("react-dom/client"); + const container = document.createElement("div"); + const root = createRoot(container as unknown as Element); + const hasCursor = (node: TestNode): boolean => + node.nodeName === "AGENT-CURSOR" || node.childNodes.some(hasCursor); + mocks.recordingTabIds.add(TEST_RUNTIME_TAB_ID); + mocks.recordingRuntimeTabId = TEST_RUNTIME_TAB_ID; + try { + await act(() => { + root.render(); + }); + expect(hasCursor(container)).toBe(false); + mocks.serverEpoch = "replacement-server"; + await act(() => { + root.render(); + }); + expect(hasCursor(container)).toBe(true); + expect(mocks.recordingTabIds.has(TEST_RUNTIME_TAB_ID)).toBe(true); + } finally { + await act(() => root.unmount()); + vi.unstubAllGlobals(); + } + }); + it("does not rerender while loading time passes", async () => { vi.useFakeTimers(); mocks.loading = true; diff --git a/apps/web/src/components/preview/PreviewView.tsx b/apps/web/src/components/preview/PreviewView.tsx index d190a78d4825..810e6d502804 100644 --- a/apps/web/src/components/preview/PreviewView.tsx +++ b/apps/web/src/components/preview/PreviewView.tsx @@ -798,18 +798,17 @@ export function PreviewView({ {snapshot && desktopOverlay ? ( ) : null} - {runtimeTabId && desktopOverlay && !showEmptyState && !isUnreachable ? ( + {runtimeTabId && + desktopOverlay && + !showEmptyState && + !isUnreachable && + !activeRecordingTabIds.has(runtimeTabId) ? ( ) : null} - {controller !== "none" ? ( -
- {controller === "agent" ? "Agent controlling browser" : "Human control"} -
- ) : null} {navStatus._tag === "LoadFailed" ? (
; + stale?: boolean; environmentId?: EnvironmentId; reference?: PullRequestRef; /** Thread the popover sits beside; a listing row has none. */ @@ -150,7 +152,7 @@ export function PullRequestChecksPopover({ }) { const presentation = pullRequestChecksStatePresentation(checksState); // Counts beat the rollup's own wording where they are known, the way GitHub's own header reads. - const summary = checks === undefined ? null : summarizePullRequestChecks(checks); + const summary = checks === undefined || stale ? null : summarizePullRequestChecks(checks); const runningCount = checks?.filter((check) => check.status === "pending").length ?? 0; return ( @@ -192,7 +194,11 @@ export function PullRequestChecksPopover({

{presentation.label}

{summary === null ? null :

{summary}

} - {checks !== undefined ? ( + {stale ? ( +

+ Check details are out of date. Refresh the pull request to update them. +

+ ) : checks !== undefined ? ( ) : environmentId !== undefined && reference !== undefined ? ( void; +}) { + const { copyToClipboard, isCopied } = useCopyToClipboard({ + target, + timeout: 1600, + ...(onError ? { onError } : {}), + }); + return ( + + copyToClipboard(value)} + /> + } + > + + {value} + + + + + {`${isCopied ? "Copied" : copyLabel}: ${value}`} + + + ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx index 18320ae33566..884bef0bf0b8 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx @@ -41,7 +41,8 @@ vi.mock("~/lib/sourceControlActions", () => ({ usePreparePullRequestThreadAction: () => ({ run: prepareThread }), })); vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() })); -vi.mock("~/state/pullRequests", () => ({ +vi.mock("~/state/pullRequests", async (importOriginal) => ({ + ...(await importOriginal()), pullRequestEnvironment: { detail: () => "detail", activity: () => "activity" }, usePullRequestTurnRefresh: () => 0, useSharedPullRequestSummary: () => null, diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 31d81315c2e1..98d172f53eb5 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -78,8 +78,13 @@ import { useProjects, useServerConfigs } from "~/state/entities"; import { useEnvironments, usePrimaryEnvironmentId } from "~/state/environments"; import { useEnvironmentQuery } from "~/state/query"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; -import { pullRequestEnvironment } from "~/state/pullRequests"; -import { usePullRequestTurnRefresh, useSharedPullRequestSummary } from "~/state/pullRequests"; +import { + pullRequestEnvironment, + pullRequestListEntryToSummary, + newestPullRequestSummary, + usePullRequestTurnRefresh, + useSharedPullRequestSummary, +} from "~/state/pullRequests"; import { useAtomCommand } from "~/state/use-atom-command"; import { PullRequestStackMenu } from "./PullRequestStackMenu"; import { PullRequestThreadLinks } from "./PullRequestThreadLinks"; @@ -116,6 +121,7 @@ import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipProvider, TooltipTrigger } from "../ui/tooltip"; import { PullRequestDetailGhost, PullRequestTimelineGhost } from "./PullRequestGhosts"; +import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; import { PullRequestActivityUnavailableState } from "./PullRequestActivityUnavailableState"; import { DiffPanelLoadingState } from "../DiffPanelShell"; import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; @@ -135,6 +141,7 @@ import { handoffPrompt, handoffReviewComments, latestPullRequestReviewOutcomes, + loadingPullRequestCheckoutCommand, isStackedPullRequestBase, pullRequestActionMenuHasGroup, pullRequestActionNeedsHostRefresh, @@ -167,6 +174,7 @@ import { PullRequestMetaLine, PullRequestReviewOutcomeIcon, pullRequestChecksState, + pullRequestChecksStatePresentation, pullRequestReviewOutcomeToneClassName, resolvePullRequestState, summarizePullRequestChecks, @@ -320,68 +328,6 @@ const openNumberContextMenu = ( }); }; -function PullRequestCopyableCode({ - value, - target, - copyLabel, - copiedLabel, - className, - tooltipSide = "top", - onError, -}: { - readonly value: string; - readonly target: string; - readonly copyLabel: string; - readonly copiedLabel: string; - readonly className?: string; - readonly tooltipSide?: "top" | "bottom"; - readonly onError?: (error: Error) => void; -}) { - const { copyToClipboard, isCopied } = useCopyToClipboard({ - target, - timeout: 1600, - ...(onError ? { onError } : {}), - }); - return ( - - copyToClipboard(value)} - /> - } - > - - {value} - - - - - {`${isCopied ? "Copied" : copyLabel}: ${value}`} - - - ); -} - /** * The stale-branch warning, said beside the branch it is about rather than as a bar of its own. * The banner this replaces held a row of chrome open across the top of every pull request that @@ -521,10 +467,11 @@ export function PullRequestDetailPanel({ }) { const environmentConfigs = useServerConfigs(); const projects = useProjects(); - const repositoryIdentity = projects.find( + const project = projects.find( (project) => project.id === requestedReference.projectId && project.environmentId === environmentId, - )?.repositoryIdentity; + ); + const repositoryIdentity = project?.repositoryIdentity; const supportsThreadPullRequests = environmentConfigs.get(environmentId)?.environment.capabilities.threadPullRequests === true; const reference = useMemo( @@ -542,6 +489,8 @@ export function PullRequestDetailPanel({ const matchingListEntry = listEntry?.projectId === reference.projectId && listEntry.repository.toLowerCase() === reference.repository.toLowerCase() && + (reference.host === undefined || + listEntry.host.toLowerCase() === reference.host.toLowerCase()) && listEntry.number === reference.number ? listEntry : null; @@ -675,14 +624,47 @@ export function PullRequestDetailPanel({ cached: cachedDetail, reference, }); - const sharedSummary = useSharedPullRequestSummary(environmentId, reference, resolvedCoreDetail); + const listSummary = useMemo( + () => (matchingListEntry === null ? null : pullRequestListEntryToSummary(matchingListEntry)), + [matchingListEntry], + ); + const detailSummary = useMemo( + () => + detailQuery.data === null + ? null + : { + ...detailQuery.data, + checksState: pullRequestChecksState(detailQuery.data.checks), + }, + [detailQuery.data], + ); + const observedSummary = useSharedPullRequestSummary( + environmentId, + reference, + detailSummary, + detailQuery.dataUpdatedAt, + ); + // The list row is also published to the shared cache, but only after this commit's layout + // effects run, so it is compared directly rather than trusted to be there already. + const sharedSummary = useMemo( + () => + newestPullRequestSummary( + resolvedCoreDetail, + newestPullRequestSummary(observedSummary, listSummary), + ), + [resolvedCoreDetail, observedSummary, listSummary], + ); const coreDetail = useMemo( () => resolvedCoreDetail === null || sharedSummary === null || sharedSummary === resolvedCoreDetail ? resolvedCoreDetail : { ...resolvedCoreDetail, - ...sharedSummary, + title: sharedSummary.title, + state: sharedSummary.state, + headBranch: sharedSummary.headBranch, + baseBranch: sharedSummary.baseBranch, + updatedAt: sharedSummary.updatedAt, author: sharedSummary.author ?? resolvedCoreDetail.author, additions: sharedSummary.additions ?? resolvedCoreDetail.additions, deletions: sharedSummary.deletions ?? resolvedCoreDetail.deletions, @@ -720,6 +702,7 @@ export function PullRequestDetailPanel({ }, [activity, coreDetail], ); + const handoffSummary = detail ?? sharedSummary; const keybindings = useAtomValue(primaryServerKeybindingsAtom); const { copyToClipboard: copyReference } = useCopyToClipboard({ target: "pull request reference", @@ -761,15 +744,22 @@ export function PullRequestDetailPanel({ repositoryUrl !== null ? new URL(`/${encodeURIComponent(detail.author.login)}`, repositoryUrl).toString() : null; - const checkoutCommand = detail + const checkoutCommand = handoffSummary ? pullRequestCheckoutCommand( - detail.provider, - detail.number, - detail.headBranch, - detail.headRepositoryNameWithOwner, - repositoryUrl, + handoffSummary.provider, + handoffSummary.number, + handoffSummary.headBranch, + detail?.headRepositoryNameWithOwner, + changeRequestRepositoryUrl(handoffSummary.url), ) - : null; + : loadingPullRequestCheckoutCommand(reference, repositoryIdentity); + const onCheckoutCommandError = useCallback((error: Error) => { + toastManager.add({ + type: "error", + title: "Could not copy checkout command", + description: error.message, + }); + }, []); const branchRefsQuery = useEnvironmentQuery( detail === null ? null @@ -945,9 +935,11 @@ export function PullRequestDetailPanel({ const acting = pickableEnvironments.find((entry) => entry.environmentId === chosenEnvironmentId) ?? null; const actingEnvironmentId = acting?.environmentId ?? environmentId; + const checkoutRoot = + acting?.workspaceRoot ?? detail?.workspaceRoot ?? project?.workspaceRoot ?? null; const prepareThread = usePreparePullRequestThreadAction({ environmentId: actingEnvironmentId, - cwd: acting?.workspaceRoot ?? detail?.workspaceRoot ?? null, + cwd: checkoutRoot, }); const finishAction = async ( @@ -1178,7 +1170,7 @@ export function PullRequestDetailPanel({ // already work — and it moves the branch under everything else that is open there. mode: "worktree" | "local" = "worktree", ) => { - if (!detail || handoff !== null) return; + if (!handoffSummary || handoff !== null) return; if (attachTarget !== null && task !== null) { writeTaskToComposer(attachTarget, task); toastManager.add({ @@ -1188,6 +1180,7 @@ export function PullRequestDetailPanel({ }); return; } + if (checkoutRoot === null) return; setHandoff(kind); // The menu closes on the press and takes its "Preparing..." label with it, so this is the // only thing answering for the checkout. It carries no timeout of its own: a loading toast @@ -1198,7 +1191,10 @@ export function PullRequestDetailPanel({ }); // Wherever the reader chose to act: the thread, the checkout it is pointed at and the composer // the task lands in are all one server's, and picking another one moves all three. - const projectRef = scopeProjectRef(actingEnvironmentId, acting?.projectId ?? detail.projectId); + const projectRef = scopeProjectRef( + actingEnvironmentId, + acting?.projectId ?? handoffSummary.projectId, + ); // The thread is opened before the checkout rather than after it, because the project's setup // script only runs for a checkout that knows which thread it is for — and a worktree with no // dependencies installed is not something anyone can test. @@ -1219,7 +1215,7 @@ export function PullRequestDetailPanel({ return; } const prepared = await prepareThread.run({ - reference: detail.url, + reference: handoffSummary.url, mode, threadId: opened.threadId, }); @@ -1348,7 +1344,7 @@ export function PullRequestDetailPanel({ }; const startCheckout = (mode: "worktree" | "local") => { - if (!detail) return; + if (!handoffSummary) return; void startHandoff(`checkout:${mode}`, null, mode); }; @@ -1380,20 +1376,20 @@ export function PullRequestDetailPanel({ baseBranch: detail.baseBranch, reviewThreads: detail.reviewThreads, comments: detail.comments, - checks: detail.checks, + checks: checksStale ? [] : detail.checks, commentsTruncated: detail.commentsTruncated, }), ); }; const startResolveConflicts = () => { - if (!detail) return; + if (!handoffSummary) return; void startHandoff("conflicts", { prompt: buildResolveConflictsPrompt({ - number: detail.number, - url: detail.url, - headBranch: detail.headBranch, - baseBranch: detail.baseBranch, + number: handoffSummary.number, + url: handoffSummary.url, + headBranch: handoffSummary.headBranch, + baseBranch: handoffSummary.baseBranch, }), }); }; @@ -1426,8 +1422,9 @@ export function PullRequestDetailPanel({ // Out of date with the base, and still cleanly mergeable — the one pairing an update button // exists for. Null everywhere else, including hosts that cannot compare at all. const freshness = detail === null ? null : resolveBaseFreshness(detail); - // A host that cannot produce a patch has no Code tab to open. The tabs themselves stay hidden - // until the detail arrives, so the loading ghost is the panel's only unfinished UI. + // A host that cannot produce a patch has no Code tab to open. While detail is loading the ghost + // uses this optimistic tab set to reserve the same chrome; a host without a patch removes Code + // when its capabilities arrive. const visibleTabs = TABS.filter( (item) => item.value !== "code" || detail === null || detail.capabilities.diff, ); @@ -1443,7 +1440,17 @@ export function PullRequestDetailPanel({ const can = (action: PullRequestAction) => detail?.capabilities.actions.includes(action) === true && detail.viewerPermissions.actions.includes(action); - const checksState = detail ? pullRequestChecksState(detail.checks) : null; + const detailChecksState = detail ? pullRequestChecksState(detail.checks) : null; + const latestChecksState = + sharedSummary?.checksState === undefined ? detailChecksState : sharedSummary.checksState; + // List rollups can omit workflows awaiting approval. Only refreshed detail can clear those. + const checksState = + latestChecksState !== "failing" && + detail?.checks.some((check) => check.status === "action-required") + ? "pending" + : latestChecksState; + // A newer rollup cannot tell us which runs changed or how many passed. + const checksStale = checksState !== detailChecksState; // The merge state remains in one stable slot from waiting through completion. Conflicts take // the slot while they need a person; the armed badge remains beside them so that state is not lost. const primaryAction = detail @@ -1494,7 +1501,13 @@ export function PullRequestDetailPanel({ const statePresentation = detail ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) : null; - const checksSummary = detail ? summarizePullRequestChecks(detail.checks) : null; + const checksSummary = checksStale + ? checksState === null + ? "No checks reported" + : pullRequestChecksStatePresentation(checksState).label + : detail + ? summarizePullRequestChecks(detail.checks) + : null; // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer // who gave it, so counting it here would have the header assert in a number what the row next // to it has just qualified. @@ -1509,10 +1522,115 @@ export function PullRequestDetailPanel({ ).length : 0; + const checkoutControl = + context === "page" ? ( + + + + + + {handoff?.startsWith("checkout") ? "Checking out..." : "Check out"} + + + + } + /> + } + /> + Check out this pull request + + + startCheckout("worktree")}> + + + In a separate worktree + + Its own folder and thread. Nothing you have open moves. + + + + startCheckout("local")}> + + + In this repository + + Switches the branch you are working in, like `gh pr checkout`. + + + + {pickableEnvironments.length > 0 ? ( + setActingScope({ pullRequestKey, environmentId: next })} + disabled={handoff !== null} + /> + ) : null} + + + ) : null; + + const resolveConflictsControl = ( + + + + + } + /> + + {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"} + + + ); + // The list already has the pull request's identity and summary. Keep them on screen // and let the richer detail read replace the remaining placeholders in place. if (detailQuery.isPending && !detail) { - return ; + return ( + + {checkoutControl} + {handoffSummary.state === "open" && handoffSummary.mergeability === "conflicting" + ? resolveConflictsControl + : null} + + ) : undefined + } + /> + ); } return ( @@ -1720,71 +1838,7 @@ export function PullRequestDetailPanel({ threadRef={null} /> ) : null} - {/* Checking a pull request out is the reason to open one here at all, so it is a - button of its own rather than a side effect of asking an agent for something. - It asks where, because the two answers are not interchangeable: one leaves your - work where it is, the other moves the repository you are standing in. Only on - the page: beside a thread the branch is already checked out right there. */} - {context === "page" ? ( - - - - - - {handoff?.startsWith("checkout") ? "Checking out..." : "Check out"} - - - - } - /> - } - /> - Check out this pull request - - - startCheckout("worktree")}> - - - In a separate worktree - - Its own folder and thread. Nothing you have open moves. - - - - startCheckout("local")}> - - - In this repository - - Switches the branch you are working in, like `gh pr checkout`. - - - - {pickableEnvironments.length > 0 ? ( - setActingScope({ pullRequestKey, environmentId: next })} - disabled={handoff !== null} - /> - ) : null} - - - ) : null} + {checkoutControl} {/* Said where the Merge button is, because it is the answer to why nobody has pressed it: the merge is already asked for, and the host is holding it. */} {autoMergeArmed && primaryAction !== "auto-merge-armed" ? ( @@ -1809,31 +1863,7 @@ export function PullRequestDetailPanel({ ) : null} {primaryAction === "resolve" ? ( - - - - - } - /> - - {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts"} - - + resolveConflictsControl ) : primaryAction === "ready" ? ( {titleDraft === null ? ( -
+
)} -
+
- toastManager.add({ - type: "error", - title: "Could not copy checkout command", - description: error.message, - }) - } + onError={onCheckoutCommandError} /> ) : null}
-
+
- + {detail.changedFiles.toLocaleString()}{" "} {detail.changedFiles === 1 ? "file" : "files"} @@ -2474,7 +2498,7 @@ export function PullRequestDetailPanel({ {tab === "summary" ? ( - {workflowApprovalsRequired > 0 && can("approve-workflows") ? ( + {workflowApprovalsRequired > 0 && !checksStale && can("approve-workflows") ? ( @@ -2649,12 +2674,14 @@ export function PullRequestDetailPanel({ reference={reference} detail={detail} activityPending={activityPending} + checksStale={checksStale} activityError={activityError} pendingFinding={handoff} fixFindingLabel={handoffLabels.fixFinding} fixCheckLabel={handoffLabels.fixCheck} onFixFinding={startFixFinding} onRefresh={refreshDetail} + onRefreshChecks={refreshFromHost} />
) : null} diff --git a/apps/web/src/components/pullRequest/PullRequestGhosts.tsx b/apps/web/src/components/pullRequest/PullRequestGhosts.tsx index f8c922356548..90f88da93668 100644 --- a/apps/web/src/components/pullRequest/PullRequestGhosts.tsx +++ b/apps/web/src/components/pullRequest/PullRequestGhosts.tsx @@ -7,16 +7,32 @@ * both themes) and the single `animate-skeleton` pulse, applied once on the container so any * number of bars costs one opacity animation. */ -import type { PullRequestListEntry } from "@t3tools/contracts"; -import { ArrowLeftIcon } from "lucide-react"; +import type { PullRequestListEntry, PullRequestSummary } from "@t3tools/contracts"; +import { + ArrowLeftIcon, + ChevronRightIcon, + EllipsisIcon, + ExternalLinkIcon, + FileDiffIcon, + PanelRightIcon, + TagIcon, + UserPlusIcon, + UsersIcon, +} from "lucide-react"; +import type { ReactNode } from "react"; +import { readLocalApi } from "~/localApi"; import { cn } from "~/lib/utils"; import { formatRelativeTimeLabel } from "~/timestampFormat"; +import { Button, InlineButton } from "../ui/button"; +import { Toggle, ToggleGroup } from "../ui/toggle-group"; +import { PullRequestCopyableCode } from "./PullRequestCopyableCode"; import { pullRequestLabelColor } from "./pullRequestList.logic"; import { PullRequestActorLabel, PullRequestDiffStat, + PullRequestMetaLine, pullRequestChecksStatePresentation, resolvePullRequestState, } from "./pullRequestPresentation"; @@ -28,6 +44,11 @@ function GhostBar({ className }: { className?: string | undefined }) { /** Widths cycle rather than randomize, so the ghost renders the same on every pass. */ const TITLE_WIDTHS = ["w-3/5", "w-2/5", "w-1/2", "w-2/3", "w-2/5", "w-3/5", "w-1/2"]; const META_WIDTHS = ["w-2/5", "w-1/3", "w-2/5", "w-1/4", "w-1/3", "w-2/5", "w-1/3"]; +const DEFAULT_DETAIL_TABS = [ + { value: "summary", label: "Summary" }, + { value: "timeline", label: "Timeline" }, + { value: "code", label: "Code" }, +] as const; /** Rows in the list's own grid — glyph, title over meta, time over diffstat. */ export function PullRequestListGhost({ @@ -72,16 +93,52 @@ export function PullRequestListGhost({ * boundaries in the ghost prevents the loaded pull request from replacing one layout with * another a moment later. */ -export function PullRequestDetailGhost({ seed }: { seed?: PullRequestListEntry | null }) { +export function PullRequestDetailGhost({ + seed: entry, + summary, + actions, + checkoutCommand, + tabs = DEFAULT_DETAIL_TABS, + activeTab, + number, + onBack, + onClose, + onCheckoutError, +}: { + seed?: PullRequestListEntry | null; + summary?: PullRequestSummary | null; + actions?: ReactNode; + checkoutCommand?: string | null; + tabs?: ReadonlyArray<{ value: string; label: string }>; + /** The panel's current tab, so the highlight does not jump when the detail arrives. */ + activeTab?: string; + number?: number; + onBack?: (() => void) | undefined; + onClose?: (() => void) | undefined; + onCheckoutError?: ((error: Error) => void) | undefined; +}) { + const seed = summary + ? { + ...entry, + ...summary, + isDraft: summary.isDraft ?? entry?.isDraft, + } + : entry; const statePresentation = seed ? resolvePullRequestState({ state: seed.state, - isDraft: seed.isDraft, + isDraft: seed.isDraft ?? false, }) : null; - const checksPresentation = seed?.checksState - ? pullRequestChecksStatePresentation(seed.checksState) - : null; + // Passing list rollups can omit workflows awaiting approval; wait for detail to claim success. + const checksPresentation = + seed?.checksState === "failing" || seed?.checksState === "pending" + ? pullRequestChecksStatePresentation(seed.checksState) + : null; + const checkout = checkoutCommand ?? null; + const changedFiles = summary?.changedFiles ?? null; + const selectedTab = + tabs.find((item) => item.value === activeTab)?.value ?? tabs[0]?.value ?? "summary"; return (
-
-
-
- {seed && statePresentation ? ( +
+
+
+ {onBack ? ( + + ) : null} + {seed ? ( <> - - {seed.repository} - - {seed.repository} + void readLocalApi()?.shell.openExternal(seed.url)} + className={cn( + "font-medium underline-offset-2 hover:underline", + statePresentation?.toneClassName, + )} + aria-label={`Open pull request #${seed.number} on host`} > #{seed.number} - + + ) : ( <> - + #{number ?? "…"} )}
-
- - -
+
+
+ {actions ?? } + + {onClose ? ( + + ) : null}
-
- {seed ? ( -

{seed.title}

- ) : ( - - )} -
- {seed ? ( - <> - - - updated {formatRelativeTimeLabel(seed.updatedAt)} - - - ) : ( - <> - - - - )} -
-
- {seed ? ( - - {seed.baseBranch} - - {seed.headBranch} - - ) : ( - <> - - - - - )} -
- +
+
+
{seed ? ( - +
+

{seed.title}

+
) : ( - +
+ +
)} +
+ {seed ? ( + + + updated {formatRelativeTimeLabel(seed.updatedAt)} + + ) : ( + + + + + + + + )} + {checkout ? ( + + ) : null} +
+ +
+ + {seed ? ( + + {seed.baseBranch} + + ) : ( + + + + )} + + {seed ? ( + + ) : ( + + )} + + + + + {changedFiles === null ? ( + + ) : ( + `${changedFiles.toLocaleString()} ${changedFiles === 1 ? "file" : "files"}` + )} + + {seed ? ( + + ) : ( + + )} + +
-
-
- - - -
+
+
-
-
-
- - -
-
- - - -
-
-
-
- - -
-
- {seed ? ( - seed.labels.slice(0, 3).map((label) => { - const color = pullRequestLabelColor(label.color); - return ( - - - {label.name} - - ); - }) - ) : ( - <> - - - - )} +
+
+
+ + + Reviewers + + + + +
-
-
-
- - +
+ + + Labels + + + {entry?.labels ? ( + entry.labels.length > 0 ? ( + entry.labels.map((label) => { + const color = pullRequestLabelColor(label.color); + return ( + + + {label.name} + + ); + }) + ) : ( + None + ) + ) : ( + <> + + + + )} + +
-
-
-
- - +
+
+
+ Description + +
-
- - - - +
+ +
+ + + + +
diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index e89b38c28e1e..4470e6c307c0 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -275,7 +275,7 @@ function MetaRow({ children: ReactNode; }) { return ( -
+
{icon} {label} @@ -461,18 +461,21 @@ export function PullRequestSummaryTab({ reference, detail, activityPending, + checksStale = false, activityError, pendingFinding, fixFindingLabel = "Fix in a thread", fixCheckLabel = "Fix", onFixFinding, onRefresh, + onRefreshChecks = onRefresh, }: { environmentId: EnvironmentId; threadRef: ScopedThreadRef | null; reference: PullRequestRef; detail: PullRequestDetailView; activityPending: boolean; + checksStale?: boolean; activityError: string | null; /** The hand-off currently preparing, if any, so only the finding it belongs to says so. */ pendingFinding?: string | null; @@ -480,6 +483,7 @@ export function PullRequestSummaryTab({ fixCheckLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; onRefresh: () => void; + onRefreshChecks?: () => void; }) { // Keyed by the pull request, so opening another one starts at the end of its conversation // rather than wherever the last one had been read back to. @@ -872,7 +876,14 @@ export function PullRequestSummaryTab({
- {detail.checks.length === 0 ? ( + {checksStale ? ( +
+ Check details are out of date. + +
+ ) : detail.checks.length === 0 ? (

No checks reported.

) : ( detail.checks.map((check, index) => { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index f46705b7560e..978ae69583df 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -7,7 +7,9 @@ import { type PullRequestComment, type PullRequestDetail, type PullRequestDetailView, + type PullRequestRef, type PullRequestReviewThread, + type RepositoryIdentity, type ThreadPullRequestLink, } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; @@ -31,6 +33,7 @@ import { stripPullRequestHandoffReferences, isPullRequestVerdictStale, isStackedPullRequestBase, + loadingPullRequestCheckoutCommand, pullRequestPanelContext, latestPullRequestReviewOutcomes, newestPullRequestCommitAt, @@ -119,6 +122,49 @@ describe("pull request checkout commands", () => { "git fetch 'https://forgejo.local/maria/repo'\\''$(echo nope)' refs/pull/42/head && git checkout -B pulls/42 FETCH_HEAD", ); }); + + const reference = (host?: string): PullRequestRef => ({ + projectId: ProjectId.make("project-1"), + ...(host === undefined ? {} : { host }), + repository: "acme/web", + number: 42, + }); + const identity = (provider: string, canonicalKey: string): RepositoryIdentity => ({ + canonicalKey, + locator: { + source: "git-remote", + remoteName: "origin", + remoteUrl: "git@github.com:acme/web.git", + }, + provider, + }); + + it("uses a public host when no repository identity is available", () => { + expect(loadingPullRequestCheckoutCommand(reference("github.com"), undefined)).toBe( + "gh pr checkout 42", + ); + expect(loadingPullRequestCheckoutCommand(reference("gitlab.com"), null)).toBe( + "glab mr checkout 42", + ); + }); + + it("uses a matching enterprise identity and rejects an explicit host mismatch", () => { + const enterprise = identity("github", "github.example.test/acme/web"); + expect(loadingPullRequestCheckoutCommand(reference("github.example.test"), enterprise)).toBe( + "gh pr checkout 42", + ); + expect(loadingPullRequestCheckoutCommand(reference("github.com"), enterprise)).toBeNull(); + }); + + it("does not infer a number-only command without a trusted provider", () => { + expect(loadingPullRequestCheckoutCommand(reference(), undefined)).toBeNull(); + expect( + loadingPullRequestCheckoutCommand( + reference("github.com"), + identity("gitlab", "gitlab.com/acme/web"), + ), + ).toBeNull(); + }); }); const TIMELINE_SOURCE: Pick< diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 6e971c371326..b8d2d897a459 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -140,6 +140,22 @@ export function pullRequestCheckoutCommand( } } +/** Build a checkout command from identity metadata while the detail request is still pending. */ +export function loadingPullRequestCheckoutCommand( + reference: PullRequestRef, + identity: RepositoryIdentity | null | undefined, +): string | null { + const host = reference.host?.trim().toLowerCase(); + const provider = + identity?.provider ?? + (host === "github.com" ? "github" : host === "gitlab.com" ? "gitlab" : null); + if (provider !== "github" && provider !== "gitlab" && provider !== "azure-devops") return null; + if (identity?.provider !== undefined && host && pullRequestHostOf(identity, provider) !== host) { + return null; + } + return pullRequestCheckoutCommand(provider, reference.number, ""); +} + /** Activity changes only when the same host resource reports a newer revision. */ export function shouldRefreshPullRequestActivity( previous: { readonly key: string; readonly updatedAt: string } | null, diff --git a/apps/web/src/components/settings/ConnectionsSettings.tsx b/apps/web/src/components/settings/ConnectionsSettings.tsx index 7295e9f6c474..7d119799a0cf 100644 --- a/apps/web/src/components/settings/ConnectionsSettings.tsx +++ b/apps/web/src/components/settings/ConnectionsSettings.tsx @@ -3418,8 +3418,8 @@ export function ConnectionsSettings() { {pendingDesktopServerExposureMode === "network-accessible" - ? "T3 Code will restart to expose this environment over the network." - : "T3 Code will restart and limit this environment back to this machine."} + ? "Let your other devices connect to T3 Code over the network. Pair devices to give them access. T3 Code will restart." + : "Devices connected over your local network will disconnect. Existing tunnels, such as T3 Connect or Tailscale HTTPS, keep working. T3 Code will restart."} @@ -3427,27 +3427,23 @@ export function ConnectionsSettings() { disabled={isUpdatingDesktopServerExposure} render={ diff --git a/apps/web/src/components/settings/IntegrationsSettings.tsx b/apps/web/src/components/settings/IntegrationsSettings.tsx index db6276d94685..60757724f2e4 100644 --- a/apps/web/src/components/settings/IntegrationsSettings.tsx +++ b/apps/web/src/components/settings/IntegrationsSettings.tsx @@ -470,6 +470,44 @@ function BrowserAppearanceSetting({ disabled }: { readonly disabled: boolean }) ); } +function BrowserRecordingInputSettings({ disabled }: { readonly disabled: boolean }) { + const showKeys = useClientSettings((settings) => settings.browserRecordingShowKeyPresses); + const showMouse = useClientSettings((settings) => settings.browserRecordingShowMousePresses); + const updateSettings = useUpdatePrimarySettings(); + return ( + <> + + updateSettings({ browserRecordingShowKeyPresses: Boolean(checked) }) + } + /> + } + /> + + updateSettings({ browserRecordingShowMousePresses: Boolean(checked) }) + } + /> + } + /> + + ); +} + function BrowserRecordingFrameRateSetting({ disabled }: { readonly disabled: boolean }) { const frameRate = useClientSettings((settings) => settings.browserRecordingFrameRate); const updateSettings = useUpdatePrimarySettings(); @@ -1326,6 +1364,7 @@ export function IntegrationsSettingsPanel() { + diff --git a/apps/web/src/components/settings/SettingsPanels.logic.test.ts b/apps/web/src/components/settings/SettingsPanels.logic.test.ts index d93db8d970b1..3f9d233ed278 100644 --- a/apps/web/src/components/settings/SettingsPanels.logic.test.ts +++ b/apps/web/src/components/settings/SettingsPanels.logic.test.ts @@ -268,6 +268,8 @@ describe("getChangedBrowserSettingLabels", () => { browserDefaultZoomFactor: 1.5, browserDefaultAppearance: "dark", browserRecordingFrameRate: 60, + browserRecordingShowKeyPresses: true, + browserRecordingShowMousePresses: true, browserLinkTarget: "app", browserAutoShowFloatingPreview: !DEFAULT_UNIFIED_SETTINGS.browserAutoShowFloatingPreview, }), @@ -276,6 +278,8 @@ describe("getChangedBrowserSettingLabels", () => { "Browser zoom", "Browser appearance", "Recording frame rate", + "Recording key presses", + "Recording mouse presses", "Open links in", "Floating preview", ]); diff --git a/apps/web/src/components/settings/SettingsPanels.logic.ts b/apps/web/src/components/settings/SettingsPanels.logic.ts index 5cbcb190a97b..d5c6359c1c9f 100644 --- a/apps/web/src/components/settings/SettingsPanels.logic.ts +++ b/apps/web/src/components/settings/SettingsPanels.logic.ts @@ -115,6 +115,8 @@ export type BrowserDefaultSettings = Pick< | "browserDefaultZoomFactor" | "browserDefaultAppearance" | "browserRecordingFrameRate" + | "browserRecordingShowKeyPresses" + | "browserRecordingShowMousePresses" | "browserLinkTarget" | "browserAutoShowFloatingPreview" >; @@ -156,6 +158,8 @@ export function getChangedBrowserSettingLabels(settings: BrowserDefaultSettings) ...(settings.browserRecordingFrameRate !== DEFAULT_UNIFIED_SETTINGS.browserRecordingFrameRate ? ["Recording frame rate"] : []), + ...(settings.browserRecordingShowKeyPresses ? ["Recording key presses"] : []), + ...(settings.browserRecordingShowMousePresses ? ["Recording mouse presses"] : []), ...(settings.browserLinkTarget !== DEFAULT_UNIFIED_SETTINGS.browserLinkTarget ? ["Open links in"] : []), diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 80a4e7dd84e4..485914a01b7c 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -646,6 +646,8 @@ export function useSettingsRestore(onRestored?: () => void) { settings.browserDefaultZoomFactor, settings.browserDefaultAppearance, settings.browserRecordingFrameRate, + settings.browserRecordingShowKeyPresses, + settings.browserRecordingShowMousePresses, settings.browserLinkTarget, settings.browserAutoShowFloatingPreview, settings.appearanceContrast, @@ -817,6 +819,8 @@ export function useSettingsRestore(onRestored?: () => void) { browserDefaultZoomFactor: DEFAULT_UNIFIED_SETTINGS.browserDefaultZoomFactor, browserDefaultAppearance: DEFAULT_UNIFIED_SETTINGS.browserDefaultAppearance, browserRecordingFrameRate: DEFAULT_UNIFIED_SETTINGS.browserRecordingFrameRate, + browserRecordingShowKeyPresses: DEFAULT_UNIFIED_SETTINGS.browserRecordingShowKeyPresses, + browserRecordingShowMousePresses: DEFAULT_UNIFIED_SETTINGS.browserRecordingShowMousePresses, browserLinkTarget: DEFAULT_UNIFIED_SETTINGS.browserLinkTarget, browserAutoShowFloatingPreview: DEFAULT_UNIFIED_SETTINGS.browserAutoShowFloatingPreview, // Re-granted like any other default. The confirmation dialog lists it by diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index d17ec14ded48..3ba7250a3701 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -640,6 +640,18 @@ export const SETTINGS_SEARCH_ITEMS = [ title: "Browser recording frame rate", to: "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/settings/integrations", }, + { + id: "browser-recording-key-presses", + title: "Show key presses in recordings", + to: "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/settings/integrations", + searchTerms: ["browser preview keyboard shortcuts keystrokes overlay capture"], + }, + { + id: "browser-recording-mouse-presses", + title: "Show mouse presses in recordings", + to: "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/settings/integrations", + searchTerms: ["browser preview clicks buttons drag overlay capture"], + }, { id: "browser-link-target", title: "Open links in", diff --git a/apps/web/src/components/ui/menu.tsx b/apps/web/src/components/ui/menu.tsx index 1daa97cc17dd..a2ad28deb7b7 100644 --- a/apps/web/src/components/ui/menu.tsx +++ b/apps/web/src/components/ui/menu.tsx @@ -27,6 +27,7 @@ function MenuPopup({ side = "bottom", anchor, onKeyDown, + keepMounted = false, ...props }: MenuPrimitive.Popup.Props & { align?: MenuPrimitive.Positioner.Props["align"]; @@ -34,6 +35,7 @@ function MenuPopup({ alignOffset?: MenuPrimitive.Positioner.Props["alignOffset"]; side?: MenuPrimitive.Positioner.Props["side"]; anchor?: MenuPrimitive.Positioner.Props["anchor"]; + keepMounted?: boolean; }) { const hasExplicitWidthClass = typeof className === "string" && @@ -43,7 +45,7 @@ function MenuPopup({ }); return ( - + ) { + return ( + + ); +} + function MenuCheckboxItem({ className, children, @@ -253,10 +278,12 @@ function MenuSub(props: MenuPrimitive.SubmenuRoot.Props) { function MenuSubTrigger({ className, inset, + density = "default", children, ...props }: MenuPrimitive.SubmenuTrigger.Props & { inset?: boolean; + density?: "default" | "touch"; }) { return ( svg:not(:last-child)]:-mx-0.5 flex min-h-8 cursor-pointer items-center gap-2 rounded-sm px-2 py-1 text-base text-foreground outline-none data-disabled:cursor-not-allowed data-disabled:pointer-events-none data-highlighted:bg-accent data-popup-open:bg-accent data-inset:ps-8 data-highlighted:text-accent-foreground data-popup-open:text-accent-foreground data-disabled:opacity-64 sm:min-h-7 sm:text-sm [&_svg:not([class*='size-'])]:size-4.5 sm:[&_svg:not([class*='size-'])]:size-4 [&_svg:not([class*='text-'])]:text-muted-foreground [&>svg:not(:last-child):not([class*='opacity-'])]:opacity-80 [&_svg]:pointer-events-none [&>svg]:shrink-0", + density === "touch" && "min-h-10 sm:min-h-10", className, )} + data-density={density} data-inset={inset} data-slot="menu-sub-trigger" {...props} @@ -315,6 +344,7 @@ export { MenuGroup, MenuGroup as DropdownMenuGroup, MenuItem, + MenuItemLabel, MenuItem as DropdownMenuItem, MenuCheckboxItem, MenuCheckboxItem as DropdownMenuCheckboxItem, diff --git a/apps/web/src/components/ui/sidebar.tsx b/apps/web/src/components/ui/sidebar.tsx index 32405fe72b06..92f185e6f629 100644 --- a/apps/web/src/components/ui/sidebar.tsx +++ b/apps/web/src/components/ui/sidebar.tsx @@ -155,7 +155,10 @@ function SidebarProvider({
- {isOpen ? : } + {isOpen ? : } Toggle Sidebar ); diff --git a/apps/web/src/state/pullRequests.test.ts b/apps/web/src/state/pullRequests.test.ts new file mode 100644 index 000000000000..48ee5b51b884 --- /dev/null +++ b/apps/web/src/state/pullRequests.test.ts @@ -0,0 +1,83 @@ +import { ProjectId, type PullRequestSummary } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { newestPullRequestObservation, newestPullRequestSummary } from "./pullRequests"; + +function summary(overrides: Partial = {}): PullRequestSummary { + return { + provider: "github", + projectId: ProjectId.make("pull-request-cache-test"), + repository: "acme/widget", + number: 7, + title: "Improve widget", + url: "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/acme/widget/pull/7", + state: "open", + isDraft: false, + headBranch: "improve-widget", + baseBranch: "main", + updatedAt: "2026-09-10T00:00:00Z", + author: { login: "oliver", name: null, avatarUrl: null }, + mergeability: "mergeable", + checksState: "passing", + ...overrides, + }; +} + +const observed = (value: PullRequestSummary, observedAt: number) => ({ + summary: value, + observedAt, +}); + +describe("pull request summary cache", () => { + it("orders same-dated snapshots by the server's read time, not by arrival", () => { + const stale = observed(summary({ observedAt: 200 }), 500); + const held = observed( + summary({ mergeability: "conflicting", checksState: "failing", observedAt: 300 }), + 100, + ); + expect(newestPullRequestObservation(stale, held)?.summary).toMatchObject({ + mergeability: "conflicting", + checksState: "failing", + observedAt: 300, + }); + // An older filtered or server-cached response finishing last must not roll status back. + expect(newestPullRequestObservation(held, stale)).toBe(held); + // The same read seen again is not a new observation. + expect(newestPullRequestObservation(held, { ...held, observedAt: 900 })).toBe(held); + }); + + it("uses arrival order only when neither snapshot carries a server read time", () => { + const first = observed(summary(), 100); + const later = observed(summary({ checksState: "failing" }), 200); + expect(newestPullRequestObservation(first, later)).toMatchObject({ observedAt: 200 }); + expect(newestPullRequestObservation(later, first)).toBe(later); + // A stamped read beats an unstamped one regardless of which arrived last. + const stamped = observed(summary({ observedAt: 50 }), 1); + expect(newestPullRequestObservation(later, stamped)?.summary.observedAt).toBe(50); + expect(newestPullRequestObservation(stamped, later)).toBe(stamped); + }); + + it("keeps known status when a newer snapshot omits it, and clears it when told to", () => { + const list = observed(summary({ reviewDecision: "approved", observedAt: 100 }), 100); + const sparse = observed( + summary({ checksState: undefined, reviewDecision: undefined, observedAt: 200 }), + 200, + ); + expect(newestPullRequestObservation(list, sparse)?.summary).toMatchObject({ + checksState: "passing", + reviewDecision: "approved", + }); + const cleared = observed(summary({ checksState: null, observedAt: 300 }), 300); + expect(newestPullRequestObservation(list, cleared)?.summary.checksState).toBeNull(); + }); + + it("treats merged as final and otherwise prefers the later host update", () => { + const merged = summary({ state: "merged", updatedAt: "2026-09-01T00:00:00Z" }); + const reopened = summary({ updatedAt: "2026-09-12T00:00:00Z", observedAt: 900 }); + expect(newestPullRequestSummary(merged, reopened)).toBe(merged); + expect(newestPullRequestSummary(reopened, merged)).toBe(merged); + const older = summary({ updatedAt: "2026-09-11T00:00:00Z", observedAt: 999 }); + expect(newestPullRequestSummary(older, reopened)).toBe(reopened); + expect(newestPullRequestSummary(reopened, older)).toBe(reopened); + }); +}); diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index e2e840c89c49..365c6812d096 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -8,6 +8,7 @@ import type { EnvironmentId, PullRequestListInput, PullRequestListStatsInput, + PullRequestListEntry, PullRequestRef, PullRequestSummary, } from "@t3tools/contracts"; @@ -30,49 +31,147 @@ export const linkedPullRequestDetailAtom = createLinkedPullRequestSummaryAtomFam pullRequestEnvironment.refreshes, ); +export interface ObservedPullRequestSummary { + readonly summary: PullRequestSummary; + /** Client arrival time, the only ordering older servers leave us for same-dated snapshots. */ + readonly observedAt: number; +} + const observedPullRequestSummaryAtom = Atom.family((key: string) => - Atom.make(null).pipe( + Atom.make(null).pipe( Atom.setIdleTTL(5 * 60_000), Atom.withLabel(`web-pull-requests:observed-summary:${key}`), ), ); +/** + * Positive when `incoming` is the newer snapshot. Merged is final. Then the host's own update + * time, then the server's read-start time, which survives its caches; a snapshot without one + * never beats a stamped read. Zero when neither side carries a read time. + */ +function compareSummaries(current: PullRequestSummary, incoming: PullRequestSummary): number { + const merged = Number(incoming.state === "merged") - Number(current.state === "merged"); + if (merged !== 0) return merged; + const updated = Date.parse(incoming.updatedAt) - Date.parse(current.updatedAt); + if (updated !== 0) return updated; + if (current.observedAt === undefined && incoming.observedAt === undefined) return 0; + return (incoming.observedAt ?? -Infinity) - (current.observedAt ?? -Infinity); +} + export function newestPullRequestSummary( current: PullRequestSummary | null, observed: PullRequestSummary | null, ): PullRequestSummary | null { if (current === null) return observed; if (observed === null) return current; - if (current.state === "merged") return current; - if (observed.state === "merged") return observed; - return Date.parse(observed.updatedAt) >= Date.parse(current.updatedAt) ? observed : current; + return compareSummaries(current, observed) >= 0 ? observed : current; +} + +/** Reuse list status without treating its deferred line-count placeholders as real stats. */ +export function pullRequestListEntryToSummary(entry: PullRequestListEntry): PullRequestSummary { + return { + provider: entry.provider, + projectId: entry.projectId, + repository: entry.repository, + number: entry.number, + title: entry.title, + url: entry.url, + state: entry.state, + isDraft: entry.isDraft, + headBranch: entry.headBranch, + baseBranch: entry.baseBranch, + updatedAt: entry.updatedAt, + ...(entry.observedAt === undefined ? {} : { observedAt: entry.observedAt }), + author: entry.author, + ...(entry.reviewDecision === undefined ? {} : { reviewDecision: entry.reviewDecision }), + ...(entry.checksState === undefined ? {} : { checksState: entry.checksState }), + mergeability: entry.mergeability, + }; +} + +// A project has one remote, so its id already pins the host. Leaving the host out lets a list +// row, a hostless legacy reference and a URL-derived thread reference share one entry. +function pullRequestSummaryKey(environmentId: EnvironmentId, reference: PullRequestRef): string { + return JSON.stringify([ + environmentId, + reference.projectId, + reference.repository.toLowerCase(), + reference.number, + ]); +} + +/** The observation to hold after `incoming` arrives. Returns `current` itself on a tie. */ +export function newestPullRequestObservation( + current: ObservedPullRequestSummary | null, + incoming: ObservedPullRequestSummary | null, +): ObservedPullRequestSummary | null { + if (current === null) return incoming; + if (incoming === null) return current; + let order = compareSummaries(current.summary, incoming.summary); + // Client clocks only break ties between snapshots that both lack a server read time. + if ( + order === 0 && + current.summary.observedAt === undefined && + incoming.summary.observedAt === undefined + ) { + order = incoming.observedAt - current.observedAt; + } + if (!(order > 0)) return current; + // A sparse summary must not erase known status, or carry old detail stats into a new list read. + return { + ...incoming, + summary: { + ...incoming.summary, + isDraft: incoming.summary.isDraft ?? current.summary.isDraft, + mergeability: incoming.summary.mergeability ?? current.summary.mergeability, + reviewDecision: + incoming.summary.reviewDecision === undefined + ? current.summary.reviewDecision + : incoming.summary.reviewDecision, + checksState: + incoming.summary.checksState === undefined + ? current.summary.checksState + : incoming.summary.checksState, + }, + }; +} + +function observePullRequestSummary( + environmentId: EnvironmentId, + reference: PullRequestRef, + summary: PullRequestSummary, + observedAt: number, +): void { + const atom = observedPullRequestSummaryAtom(pullRequestSummaryKey(environmentId, reference)); + appAtomRegistry.modify(atom, (previous) => { + const next = newestPullRequestObservation(previous, { summary, observedAt }); + return next === previous ? [false, previous] : [true, next]; + }); } export function useSharedPullRequestSummary( environmentId: EnvironmentId | null, reference: PullRequestRef | null, current: PullRequestSummary | null, + observedAt: number | null = null, ): PullRequestSummary | null { const key = environmentId === null || reference === null ? "none" - : JSON.stringify([ - environmentId, - reference.projectId, - reference.host?.toLowerCase() ?? null, - reference.repository.toLowerCase(), - reference.number, - ]); + : pullRequestSummaryKey(environmentId, reference); const atom = observedPullRequestSummaryAtom(key); const observed = useAtomValue(atom); useLayoutEffect(() => { - if (environmentId === null || current === null) return; - appAtomRegistry.modify(atom, (previous) => { - const next = newestPullRequestSummary(previous, current); - return next === previous ? [false, previous] : [true, next]; - }); - }, [atom, current, environmentId]); - return newestPullRequestSummary(current, observed); + if (environmentId === null || reference === null || current === null || observedAt === null) + return; + observePullRequestSummary(environmentId, reference, current, observedAt); + }, [current, environmentId, reference, observedAt]); + return ( + newestPullRequestObservation( + observed, + current === null || observedAt === null ? null : { summary: current, observedAt }, + )?.summary ?? current + ); } export const pullRequestStackAtom = createPullRequestStackAtomFamily( connectionAtomRuntime, @@ -90,6 +189,7 @@ interface MergedEnvironmentQueryView { /** The first environment that failed. Others may still have answered — this is not fatal. */ readonly error: string | null; readonly isPending: boolean; + readonly observations: ReadonlyArray; } /** @@ -110,6 +210,7 @@ function createMergedEnvironmentQuery( Atom.make((get): MergedEnvironmentQueryView => { const targets = JSON.parse(key) as ReadonlyArray>; const values: Array = []; + const observations: Array = []; let error: string | null = null; let isPending = false; for (const target of targets) { @@ -120,12 +221,16 @@ function createMergedEnvironmentQuery( } const value = Option.getOrNull(AsyncResult.value(result)); if (value !== null) values.push([target.environmentId, value]); + if (result._tag === "Success") { + observations.push([target.environmentId, result.value, result.timestamp]); + } } - return { values, error, isPending }; + return { values, error, isPending, observations }; }).pipe(Atom.withLabel(`${label}:${key}`)), ); const empty = Atom.make>({ values: [], + observations: [], error: null, isPending: false, }).pipe(Atom.withLabel(`${label}:empty`)); @@ -187,6 +292,23 @@ export function usePullRequestList( targets: ReadonlyArray>, ): MergedPullRequestListView { const query = usePullRequestListsQuery(targets); + useLayoutEffect(() => { + for (const [environmentId, answer, observedAt] of query.observations) { + for (const entry of answer.entries) { + observePullRequestSummary( + environmentId, + { + projectId: entry.projectId, + host: entry.host, + repository: entry.repository, + number: entry.number, + }, + pullRequestListEntryToSummary(entry), + observedAt, + ); + } + } + }, [query.observations]); const data = useMemo(() => mergePullRequestLists(query.values), [query.values]); return { data, error: query.error, isPending: query.isPending, refresh: query.refresh }; } diff --git a/apps/web/src/state/query.ts b/apps/web/src/state/query.ts index ba081ffab1b7..cd390a1ca056 100644 --- a/apps/web/src/state/query.ts +++ b/apps/web/src/state/query.ts @@ -9,7 +9,7 @@ const EMPTY_ASYNC_RESULT_ATOM = Atom.make(AsyncResult.initial(fals export interface EnvironmentQueryView { readonly data: A | null; - readonly dataUpdatedAt: number; + readonly dataUpdatedAt: number | null; readonly error: string | null; readonly isPending: boolean; readonly isSuccess: boolean; @@ -31,12 +31,14 @@ export function useEnvironmentQuery( const refresh = useAtomRefresh(selectedAtom); return { data: Option.getOrNull(AsyncResult.value(result)), + // `data` falls back to the previous success on failure, so the timestamp has to follow it; + // null means nothing has ever loaded, which callers read as "no observation to publish". dataUpdatedAt: result._tag === "Success" ? result.timestamp : result._tag === "Failure" - ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? 0) - : 0, + ? (Option.getOrNull(result.previousSuccess)?.timestamp ?? null) + : null, error: result._tag === "Failure" ? formatEnvironmentQueryError(result.cause) : null, isPending: atom !== null && result.waiting, isSuccess: result._tag === "Success", diff --git a/docs/operations/upstream-sync.md b/docs/operations/upstream-sync.md new file mode 100644 index 000000000000..84402302a664 --- /dev/null +++ b/docs/operations/upstream-sync.md @@ -0,0 +1,52 @@ +# Syncing Fold with upstream T3 Code + +Fold tracks [pingdotgg/t3code](https://github.com/pingdotgg/t3code) as the `upstream` remote +(fetch only; `origin` and `fold` both point at the Fold repository). Every sync so far is a real +merge commit, and that is the whole maintenance strategy: the merge base is what lets the next +sync present only the conflicts that appeared since this one. + +## The procedure + +```bash +git fetch upstream --prune +git switch -c sync-upstream-main origin/main +git merge upstream/main # resolve, then commit +``` + +Then open a PR into Fold's `main`. The merge commit message is the place to record why each +conflict was settled the way it was — the next person to hit the same file reads that before +re-deriving the decision. + +**Never rebase or squash a sync.** Either one throws away the merge base, and the following sync +replays every upstream commit Fold has already integrated as a fresh conflict. + +## Resolving conflicts + +Fold's divergence falls into a few recurring shapes. Recognising which one a conflict is usually +decides it: + +- **Upstream reworks a region Fold moved.** Fold relocated the chat header's script, open-in, and + Git controls into `ThreadDetailsPanel`, and replaced the queued-message timeline row with its own + queue UI. Upstream changes aimed at the old locations do not apply; take Fold's side and check + whether the change has a home in Fold's replacement. +- **Both sides add a variant of the same component.** Prefer upstream's prop name and fold Fold's + variant in as an extra union member — see the `presentation` prop on `OpenInPicker`, + `ProjectScriptsControl`, and `GitActionsControl`. Renaming upstream's API to match Fold's + convention makes every later upstream hunk on that prop conflict. +- **Upstream extracts shared JSX.** Reuse the extraction (`editorItems`, `scriptItems`, `gitItems`) + from Fold's branches too, rather than keeping a second inline copy that has to be edited twice. +- **A deleted file comes back as modify/delete.** Orchestration v2 removed the v1 harnesses + (`apps/server/src/bin.test.ts`, `apps/server/src/server.test.ts`). They stay deleted. + +Upstream code that Fold does not currently reach — such as the collapsed-header `presentation: +"menu"` branches — is still worth carrying. It costs nothing at runtime and keeps those files +merging cleanly. + +Git auto-merges hunks that are textually disjoint but semantically incompatible, so a sync is not +finished when the conflict markers are gone. Typecheck the workspaces you touched before +committing; that is what catches leftover references to props Fold removed. + +## Afterwards + +Update the upstream baseline commit and date in the README's "Extra features" preamble, so the +feature tables stay honest about what is Fold's and what upstream already ships. diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index 7cbc1d7429eb..9a6027166b13 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -659,6 +659,30 @@ export interface DesktopPreviewPointerEvent { createdAt: string; } +/** Recording decorations are forwarded separately from the captured page pixels. */ +export const DesktopPreviewRecordingInputSchema = Schema.Union([ + Schema.Struct({ + type: Schema.Literal("pointer"), + phase: Schema.Literals(["move", "down", "up", "click"]), + x: Schema.Finite, + y: Schema.Finite, + width: Schema.Finite.check(Schema.isGreaterThan(0)), + height: Schema.Finite.check(Schema.isGreaterThan(0)), + }), + Schema.Struct({ + type: Schema.Literal("key"), + label: Schema.NullOr(Schema.String.check(Schema.isMaxLength(100))), + held: Schema.Boolean, + width: Schema.Finite.check(Schema.isGreaterThan(0)), + }), + Schema.Struct({ type: Schema.Literal("clear") }), +]); +export type DesktopPreviewRecordingInput = typeof DesktopPreviewRecordingInputSchema.Type; +export interface DesktopPreviewRecordingInputEvent { + readonly tabId: string; + readonly input: DesktopPreviewRecordingInput; +} + /** * Static config a renderer needs to mount a preview ``. Returned * atomically by `DesktopPreviewBridge.getPreviewConfig()` so the renderer @@ -1296,6 +1320,7 @@ export interface DesktopPreviewBridge { close: (tabId: string) => Promise; }; recording: { + onInput: (listener: (event: DesktopPreviewRecordingInputEvent) => void) => () => void; startScreencast: (tabId: string) => Promise; stopScreencast: (tabId: string) => Promise; save: ( diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index c512a716dba6..75e9312f92aa 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -525,6 +525,8 @@ export const PullRequestListEntry = Schema.Struct({ deletions: NonNegativeInt, createdAt: IsoDateTime, updatedAt: IsoDateTime, + /** Server epoch milliseconds when the provider read started; preserved on cache hits. */ + observedAt: Schema.optional(Schema.Finite), viewerReviewRequested: Schema.Boolean, labels: Schema.Array(PullRequestLabel), /** Absent where the host does not summarise its reviews, which is every host but GitHub. */ @@ -736,6 +738,8 @@ export const PullRequestSummary = Schema.Struct({ closedAt: Schema.optional(Schema.NullOr(Schema.String)), mergedAt: Schema.optional(Schema.NullOr(Schema.String)), updatedAt: IsoDateTime, + /** Server epoch milliseconds when the provider read started; preserved on cache hits. */ + observedAt: Schema.optional(Schema.Finite), author: Schema.optional(Schema.NullOr(PullRequestActor)), additions: Schema.optional(NonNegativeInt), deletions: Schema.optional(NonNegativeInt), @@ -841,6 +845,8 @@ export const PullRequestDetail = Schema.Struct({ baseBranch: TrimmedNonEmptyString, createdAt: IsoDateTime, updatedAt: IsoDateTime, + /** Server epoch milliseconds when the provider read started; preserved on cache hits. */ + observedAt: Schema.optional(Schema.Finite), mergedAt: Schema.NullOr(IsoDateTime), closedAt: Schema.NullOr(IsoDateTime), reviewers: Schema.Array(PullRequestActor), diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index bc96d2455c46..2181a00d5d0f 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -612,6 +612,23 @@ describe("ClientSettings browser recording frame rate", () => { }); }); +describe("ClientSettings recording input overlays", () => { + it("defaults both overlays off and accepts independent opt-ins", () => { + const settings = decodeClientSettings({}); + expect(settings.browserRecordingShowKeyPresses).toBe(false); + expect(settings.browserRecordingShowMousePresses).toBe(false); + expect( + decodeClientSettingsPatch({ + browserRecordingShowKeyPresses: true, + browserRecordingShowMousePresses: false, + }), + ).toMatchObject({ + browserRecordingShowKeyPresses: true, + browserRecordingShowMousePresses: false, + }); + }); +}); + describe("ClientSettings glass opacity", () => { it("defaults to a readable translucent surface", () => { expect(decodeClientSettings({}).glassOpacity).toBe(80); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index ffb8559d533d..011c985990c2 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -314,6 +314,12 @@ export const ClientSettingsSchema = Schema.Struct({ browserRecordingFrameRate: BrowserRecordingFrameRate.pipe( Schema.withDecodingDefault(Effect.succeed(DEFAULT_BROWSER_RECORDING_FRAME_RATE)), ), + browserRecordingShowKeyPresses: Schema.Boolean.pipe( + Schema.withDecodingDefault(Effect.succeed(false)), + ), + browserRecordingShowMousePresses: Schema.Boolean.pipe( + Schema.withDecodingDefault(Effect.succeed(false)), + ), /** * Where links clicked in a thread (chat markdown, terminal output) open. * Only the desktop app has an in-app browser, so other clients ignore "app". @@ -1612,6 +1618,8 @@ export const ClientSettingsPatch = Schema.Struct({ browserDefaultZoomFactor: Schema.optionalKey(PreviewZoomFactor), browserDefaultAppearance: Schema.optionalKey(PreviewAppearancePreference), browserRecordingFrameRate: Schema.optionalKey(BrowserRecordingFrameRate), + browserRecordingShowKeyPresses: Schema.optionalKey(Schema.Boolean), + browserRecordingShowMousePresses: Schema.optionalKey(Schema.Boolean), browserLinkTarget: Schema.optionalKey(BrowserLinkTarget), browserAutoShowFloatingPreview: Schema.optionalKey(Schema.Boolean), browserProfiles: Schema.optionalKey(Schema.Array(BrowserProfile)), diff --git a/packages/shared/src/observability.ts b/packages/shared/src/observability.ts index 62121399853b..6393ed0879db 100644 --- a/packages/shared/src/observability.ts +++ b/packages/shared/src/observability.ts @@ -16,6 +16,25 @@ export type OtlpProtocol = typeof OtlpProtocol.Type; export const otlpSerializationLayer = (protocol: OtlpProtocol) => protocol === "http/protobuf" ? OtlpSerialization.layerProtobuf : OtlpSerialization.layerJson; +/** + * How one signal is exported, once whichever source named that signal's + * endpoint has been resolved. Held per signal rather than per process, so a + * wire format or a credential cannot be paired by hand with an endpoint that + * came from somewhere else. + */ +export interface SignalExport { + readonly protocol: OtlpProtocol; + readonly headers: Readonly> | undefined; + readonly exportIntervalMs: number; +} + +/** What T3 Code exports with when nothing configured a signal. */ +export const DEFAULT_SIGNAL_EXPORT: SignalExport = { + protocol: "http/json", + headers: undefined, + exportIntervalMs: 10_000, +}; + const FLUSH_BUFFER_THRESHOLD = 256; const textEncoder = new TextEncoder();