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 7b76a1b8003a..304efeafefe9 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); - - 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); + 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); - 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, - }); - }), - ), + 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( + 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("types in background webviews and enables native key input", () => @@ -4230,6 +4412,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", @@ -4263,6 +4448,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 }) @@ -4270,6 +4456,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 a2e34fe54736..468c47065315 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, @@ -4110,6 +4230,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. @@ -4470,6 +4602,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 }, ); @@ -4513,6 +4646,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), @@ -4877,7 +5012,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, @@ -4918,6 +5056,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; @@ -5011,6 +5152,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..(null); const [inputConnected, setInputConnected] = useState(false); const [streamAttempt, setStreamAttempt] = useState(0); @@ -87,7 +87,7 @@ function DevicePreviewScreen({ ); useEffect(() => { const subscription = AppState.addEventListener("change", (state) => - setForeground(state === "active"), + setForeground(state !== "background"), ); return () => subscription.remove(); }, []); diff --git a/apps/mobile/src/features/devices/DeviceStreamWebView.tsx b/apps/mobile/src/features/devices/DeviceStreamWebView.tsx index 9a5cef83ae3f..423fdb1c84a0 100644 --- a/apps/mobile/src/features/devices/DeviceStreamWebView.tsx +++ b/apps/mobile/src/features/devices/DeviceStreamWebView.tsx @@ -1,7 +1,19 @@ import deviceStreamScript from "@t3tools/mobile-device-stream"; -import { useImperativeHandle, useLayoutEffect, useMemo, useRef, useState, type Ref } from "react"; -import { Platform } from "react-native"; +import { + useEffect, + useEffectEvent, + useImperativeHandle, + useLayoutEffect, + useMemo, + useRef, + useState, + type Ref, +} from "react"; +import { ActivityIndicator, Platform, Pressable, View } from "react-native"; import { WebView } from "react-native-webview"; +import type { DeviceStreamStatus } from "@t3tools/client-runtime/device/stream"; + +import { AppText } from "../../components/AppText"; import { deviceStreamDocument, @@ -27,6 +39,7 @@ export function DeviceStreamWebView({ ...props }: DeviceStreamConfiguration & NativeStreamBridge) { const [attempt, setAttempt] = useState(0); + const processRetried = useRef(false); const configuration = JSON.stringify({ access: props.access, platform: props.platform, @@ -41,7 +54,21 @@ export function DeviceStreamWebView({ background={props.colors.background} onUnauthorized={props.onUnauthorized} onInputConnected={props.onInputConnected} - onRetry={() => setAttempt((attempt) => attempt + 1)} + onRetry={() => { + processRetried.current = false; + setAttempt((attempt) => attempt + 1); + void props.onUnauthorized(); + }} + onStreaming={() => { + processRetried.current = false; + }} + onRecoverProcess={() => { + if (processRetried.current) return false; + processRetried.current = true; + setAttempt((attempt) => attempt + 1); + void props.onUnauthorized(); + return true; + }} /> ); } @@ -53,12 +80,38 @@ function DeviceStreamDocumentView({ onUnauthorized, onInputConnected, onRetry, + onStreaming, + onRecoverProcess, }: NativeStreamBridge & { readonly configuration: string; readonly background: string; readonly onRetry: () => void; + readonly onStreaming: () => void; + readonly onRecoverProcess: () => boolean; }) { const webView = useRef(null); + const active = useRef(true); + const failed = useRef(false); + const [status, setStatus] = useState("connecting"); + const [error, setError] = useState(null); + const [started, setStarted] = useState(false); + const fail = (message: string) => { + if (!active.current || failed.current) return; + failed.current = true; + void onInputConnected(false); + webView.current?.injectJavaScript("window.T3DeviceStream?.stop(); true;"); + setError(message); + setStatus("error"); + }; + // The shared transport owns video timeouts once the document acknowledges startup. + const bootstrapTimedOut = useEffectEvent(() => + fail("Device viewer could not start. Reconnect to try again."), + ); + useEffect(() => { + if (started) return; + const timer = setTimeout(bootstrapTimedOut, 15_000); + return () => clearTimeout(timer); + }, [started]); const source = useMemo( () => ({ html: deviceStreamDocument(configuration, deviceStreamScript), @@ -78,32 +131,80 @@ function DeviceStreamDocumentView({ appSwitcher: () => command("appSwitcher"), rotate: () => command("rotate"), })); + const resetInput = useEffectEvent(() => void onInputConnected(false)); useLayoutEffect(() => { + active.current = true; + resetInput(); const view = webView.current; - return () => view?.injectJavaScript("window.T3DeviceStream?.stop(); true;"); + return () => { + active.current = false; + view?.injectJavaScript("window.T3DeviceStream?.stop(); true;"); + }; }, []); + const processTerminated = () => { + if (!active.current || failed.current) return; + void onInputConnected(false); + if (!onRecoverProcess()) fail("Device viewer stopped. Reconnect to try again."); + }; return ( - void onInputConnected(false)} - onShouldStartLoadWithRequest={(request) => - request.url === "about:blank" || request.url === source.baseUrl - } - onMessage={(event) => { - const message = deviceStreamMessage(event.nativeEvent.data); - if (message?.type === "unauthorized") void onUnauthorized(); - else if (message?.type === "input") void onInputConnected(message.connected); - else if (message?.type === "retry") onRetry(); - }} - /> + + fail("Device viewer could not load. Reconnect to try again.")} + onHttpError={() => fail("Device viewer could not load. Reconnect to try again.")} + onContentProcessDidTerminate={processTerminated} + onRenderProcessGone={processTerminated} + onShouldStartLoadWithRequest={(request) => + request.url === "about:blank" || request.url === source.baseUrl + } + onMessage={(event) => { + if (!active.current || failed.current) return; + const message = deviceStreamMessage(event.nativeEvent.data); + if (message?.type === "unauthorized") void onUnauthorized(); + else if (message?.type === "input") void onInputConnected(message.connected); + else if (message?.type === "retry") onRetry(); + else if (message?.type === "status") { + setStarted(true); + if (message.status === "error") fail(message.detail ?? "Device stream failed."); + else { + setStatus(message.status); + if (message.status === "streaming") onStreaming(); + } + } + }} + /> + {status !== "streaming" ? ( + + {status === "connecting" ? : null} + + {status === "error" ? error : "Connecting to device..."} + + {status === "error" ? ( + + Reconnect + + ) : null} + + ) : null} + ); } diff --git a/apps/mobile/src/features/devices/device-stream-document.test.ts b/apps/mobile/src/features/devices/device-stream-document.test.ts index ffc08667a147..95aa6a9dad23 100644 --- a/apps/mobile/src/features/devices/device-stream-document.test.ts +++ b/apps/mobile/src/features/devices/device-stream-document.test.ts @@ -1,5 +1,5 @@ import * as NodeVM from "node:vm"; -import { describe, expect, it } from "vite-plus/test"; +import { describe, expect, it, vi } from "vite-plus/test"; import { deviceStreamDocument, deviceStreamMessage } from "./device-stream-document"; @@ -13,7 +13,7 @@ describe("native device stream document", () => { ); const script = html.match(/`; + const failure = `window.ReactNativeWebView.postMessage(JSON.stringify({type:"status",status:"error",detail:"Device viewer stopped unexpectedly."}));`; + return ``; } export function deviceStreamMessage(data: string) { @@ -36,6 +37,21 @@ export function deviceStreamMessage(data: string) { ) { return { type: message.type, connected: message.connected } as const; } + if ( + message.type === "status" && + "status" in message && + (message.status === "connecting" || + message.status === "streaming" || + message.status === "error") && + (!("detail" in message) || typeof message.detail === "string") + ) { + return { + type: message.type, + status: message.status, + detail: + "detail" in message && typeof message.detail === "string" ? message.detail : undefined, + } as const; + } } catch { // Ignore messages that are not part of the stream bridge. } diff --git a/apps/mobile/src/features/devices/device-stream.browser.test.ts b/apps/mobile/src/features/devices/device-stream.browser.test.ts new file mode 100644 index 000000000000..5aeefdac0203 --- /dev/null +++ b/apps/mobile/src/features/devices/device-stream.browser.test.ts @@ -0,0 +1,117 @@ +import { afterEach, expect, it, vi } from "vite-plus/test"; +import { start, stop } from "./device-stream.browser"; + +class Element extends EventTarget { + readonly style = {}; + naturalWidth = 0; + naturalHeight = 0; + src = ""; + readonly tag: string; + constructor(tag: string) { + super(); + this.tag = tag; + } + setAttribute() {} + removeAttribute(name: string) { + if (name === "src") this.src = ""; + } + append() {} +} + +async function setup() { + vi.useFakeTimers(); + const elements: Element[] = []; + vi.stubGlobal("document", { + documentElement: { style: {} }, + body: { style: {}, replaceChildren() {} }, + createElement: (tag: string) => { + const element = new Element(tag); + elements.push(element); + return element; + }, + }); + const postMessage = vi.fn(); + vi.stubGlobal("window", { ReactNativeWebView: { postMessage }, addEventListener() {} }); + vi.stubGlobal("fetch", () => Promise.resolve(new Response("prime"))); + const sockets: Socket[] = []; + class Socket { + static OPEN = 1; + readyState = 1; + onopen: (() => void) | null = null; + close = vi.fn(); + send = vi.fn(); + constructor() { + sockets.push(this); + } + } + vi.stubGlobal("WebSocket", Socket); + const configuration = { + platform: "ios" as const, + deviceId: "fixture-device", + access: { + httpBase: "https://device.test", + wsBase: "wss://device.test", + credentials: false, + query: {}, + }, + colors: { + background: "white", + foreground: "black", + muted: "gray", + buttonBackground: "gray", + buttonForeground: "black", + buttonBorder: "gray", + }, + }; + start(configuration); + await vi.advanceTimersByTimeAsync(0); + return { + configuration, + elements, + sockets, + messages: () => + postMessage.mock.calls.map( + ([message]) => JSON.parse(message as string) as { type: string; status?: string }, + ), + }; +} + +afterEach(() => { + stop(); + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +it("bridges shared first-frame readiness, image failure, and a successful fresh attempt to native", async () => { + const { elements, messages, sockets, configuration } = await setup(); + sockets[0]!.onopen?.(); + expect(messages()).not.toContainEqual({ type: "status", status: "streaming" }); + expect(messages()).toContainEqual({ type: "input", connected: true }); + const image = elements.find((element) => element.tag === "img")!; + image.naturalWidth = 400; + image.naturalHeight = 800; + await vi.advanceTimersByTimeAsync(250); + expect(messages()).toContainEqual({ type: "status", status: "streaming" }); + image.dispatchEvent(new Event("error")); + expect(messages()).toContainEqual({ + type: "status", + status: "error", + detail: "Could not receive the device stream. Reconnect to try again.", + }); + expect(messages()).toContainEqual({ type: "input", connected: false }); + expect(messages()).not.toContainEqual({ type: "unauthorized" }); + expect(image.src).toBe(""); + expect(sockets[0]!.close).toHaveBeenCalledOnce(); + start(configuration); + await vi.advanceTimersByTimeAsync(0); + const replacement = elements.findLast((element) => element.tag === "img")!; + replacement.naturalWidth = 400; + replacement.naturalHeight = 800; + replacement.dispatchEvent(new Event("load")); + expect(messages().at(-1)).toEqual({ type: "status", status: "streaming" }); + image.dispatchEvent(new Event("error")); + expect(sockets[1]!.close).not.toHaveBeenCalled(); + stop(); + expect(replacement.src).toBe(""); + expect(vi.getTimerCount()).toBe(0); +}); diff --git a/apps/mobile/src/features/devices/device-stream.browser.ts b/apps/mobile/src/features/devices/device-stream.browser.ts index efa81d9a043a..0b9e98a51b9f 100644 --- a/apps/mobile/src/features/devices/device-stream.browser.ts +++ b/apps/mobile/src/features/devices/device-stream.browser.ts @@ -12,13 +12,10 @@ declare global { } let activeClient: ReturnType | null = null; -let activeImage: HTMLImageElement | null = null; export function stop() { activeClient?.stop(); activeClient = null; - activeImage?.removeAttribute("src"); - activeImage = null; } export function command(button: "home" | "back" | "appSwitcher" | "rotate") { @@ -70,34 +67,6 @@ export function start(configuration: DeviceStreamConfiguration) { image.alt = ""; image.draggable = false; image.style.display = "none"; - const overlay = document.createElement("div"); - overlay.setAttribute("role", "status"); - Object.assign(overlay.style, { - position: "fixed", - inset: "0", - display: "flex", - flexDirection: "column", - alignItems: "center", - justifyContent: "center", - gap: "16px", - padding: "24px", - textAlign: "center", - background: colors.background, - }); - const detail = document.createElement("span"); - const retry = document.createElement("button"); - retry.textContent = "Retry"; - Object.assign(retry.style, { - padding: "12px 24px", - borderRadius: "20px", - border: `1px solid ${colors.buttonBorder}`, - background: colors.buttonBackground, - color: colors.buttonForeground, - font: "inherit", - display: "none", - }); - retry.addEventListener("click", () => post({ type: "retry" })); - overlay.append(detail, retry); const inputStatus = document.createElement("div"); inputStatus.setAttribute("role", "status"); inputStatus.textContent = "Reconnecting device controls..."; @@ -114,11 +83,17 @@ export function start(configuration: DeviceStreamConfiguration) { }); frame.append(canvas, image); container.append(frame); - document.body.replaceChildren(container, overlay, inputStatus); + document.body.replaceChildren(container, inputStatus); let pointerId: number | null = null; let inputConnected = false; let streaming = false; + const reportStatus = (status: "connecting" | "streaming" | "error", detail?: string) => { + if (activeClient !== client) return; + streaming = status === "streaming"; + inputStatus.style.display = streaming && !inputConnected ? "block" : "none"; + post({ type: "status", status, detail }); + }; const layout = (screen: DeviceScreenSize | null) => { const landscape = screen?.orientation === "landscape_left" || screen?.orientation === "landscape_right"; @@ -160,19 +135,11 @@ export function start(configuration: DeviceStreamConfiguration) { { ...configuration, preferMjpeg: platform === "ios" }, canvas, { - onStatus: (status, message) => { - streaming = status === "streaming"; - overlay.style.display = streaming ? "none" : "flex"; - inputStatus.style.display = streaming && !inputConnected ? "block" : "none"; - detail.textContent = - status === "error" ? (message ?? "Device stream failed.") : "Connecting to device..."; - retry.style.display = status === "error" ? "block" : "none"; - }, + onStatus: reportStatus, onScreen: layout, - onMjpegFallback: (url) => { + onMjpegFallback: () => { canvas.style.display = "none"; image.style.display = "block"; - image.src = url; }, onUnauthorized: unauthorized, onInputConnected: (connected) => { @@ -183,8 +150,7 @@ export function start(configuration: DeviceStreamConfiguration) { }, ); activeClient = client; - activeImage = image; - image.addEventListener("error", unauthorized); + client.setMjpegImage(image); const touch = (event: PointerEvent, phase: "begin" | "move" | "end") => { const rect = frame.getBoundingClientRect(); client.sendTouch( diff --git a/apps/mobile/src/features/review/ReviewCommentCard.tsx b/apps/mobile/src/features/review/ReviewCommentCard.tsx index ff348e1f2a97..5be4e87cba40 100644 --- a/apps/mobile/src/features/review/ReviewCommentCard.tsx +++ b/apps/mobile/src/features/review/ReviewCommentCard.tsx @@ -110,8 +110,9 @@ export const ReviewCommentCard = memo(function ReviewCommentCard(props: { () => 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 9732ed6865f5..681f6c5ee52d 100644 --- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx @@ -1455,7 +1455,7 @@ export function NewTaskDraftScreen(props: { in { it("applies the remaining attachment slots to photos and videos together", async () => { mocks.pickMedia.mockResolvedValue({ canceled: false, assets: [image, video] }); - const result = await pickComposerMedia({ existingCount: 7, maxVideoBytes: 50 * 1024 * 1024 }); + const result = await pickComposerMedia({ + existingCount: 99, + maxVideoBytes: 50 * 1024 * 1024, + }); expect(result.attachments).toEqual([expect.objectContaining({ type: "image" })]); - expect(result.error).toBe("You can attach up to 8 attachments per message."); + expect(result.error).toBe("You can attach up to 100 attachments per message."); expect(mocks.pickMedia).toHaveBeenCalledWith(expect.objectContaining({ selectionLimit: 1 })); expect(mocks.copy).not.toHaveBeenCalled(); }); @@ -625,9 +628,9 @@ describe("composer file attachments", () => { }); it("does not open the picker when the draft has no remaining attachment slots", async () => { - await expect(pickComposerFiles({ existingCount: 8 })).resolves.toEqual({ + await expect(pickComposerFiles({ existingCount: 100 })).resolves.toEqual({ files: [], - error: "You can attach up to 8 files per message.", + error: "You can attach up to 100 files per message.", }); expect(mocks.pickFile).not.toHaveBeenCalled(); @@ -838,7 +841,7 @@ describe("composer file attachments", () => { ], }); - const result = await pickComposerFiles({ existingCount: 7, maxBytes: 1024 * 1024 }); + const result = await pickComposerFiles({ existingCount: 99, maxBytes: 1024 * 1024 }); expect(result.files.map((file) => file.name)).toEqual(["report.pdf"]); }); diff --git a/apps/mobile/src/state/use-composer-drafts.test.ts b/apps/mobile/src/state/use-composer-drafts.test.ts index 6da72860009e..9f5dad4d5600 100644 --- a/apps/mobile/src/state/use-composer-drafts.test.ts +++ b/apps/mobile/src/state/use-composer-drafts.test.ts @@ -676,7 +676,7 @@ describe("mobile composer drafts", () => { return undefined; }); const key = "environment-1:replace-file"; - const files = Array.from({ length: 8 }, (_, index) => ({ + const files = Array.from({ length: 100 }, (_, index) => ({ id: `file-${index}`, type: "file" as const, name: `notes-${index}.txt`, @@ -687,8 +687,8 @@ describe("mobile composer drafts", () => { appendComposerDraftAttachments(key, files, { appendReference: true }); const firstLink = "[notes-0.txt](t3-context://v1/file/file-0)"; const insertion = captureComposerDraftInsertion(key, { start: 0, end: firstLink.length }); - expect(countComposerDraftAttachmentsAfterSelection(key, insertion)).toBe(7); - expect(getComposerDraftAfterSelection(key, insertion).context?.records).toHaveLength(7); + expect(countComposerDraftAttachmentsAfterSelection(key, insertion)).toBe(99); + expect(getComposerDraftAfterSelection(key, insertion).context?.records).toHaveLength(99); const replacement = { ...files[0]!, id: "replacement", fileUri: "file:///replacement.txt" }; if (kind === "attachment") { expect( @@ -716,7 +716,7 @@ describe("mobile composer drafts", () => { insertion, ), ).toBe(true); - expect(getComposerDraftSnapshot(key).attachments).toHaveLength(8); + expect(getComposerDraftSnapshot(key).attachments).toHaveLength(100); expect(getComposerDraftSnapshot(key).context?.records).toContainEqual(record); expect(getComposerDraftSnapshot(key).text).toBe( `${formatComposerContextReference(record)}${insertion.text.slice(firstLink.length)}`, @@ -730,7 +730,7 @@ describe("mobile composer drafts", () => { } const draft = getComposerDraftSnapshot(key); expect(draft.attachments.map((file) => file.id)).not.toContain("file-0"); - expect(draft.attachments.slice(0, 7)).toEqual(files.slice(1)); + expect(draft.attachments.slice(0, 99)).toEqual(files.slice(1)); expect(draft.context?.records.some((record) => record.contextId === "file-0")).toBe(false); await cleanup.promise; expect(composerAttachmentCleanupMocks.remove).toHaveBeenCalledWith(files[0]!.fileUri); @@ -746,7 +746,7 @@ describe("mobile composer drafts", () => { return undefined; }); const key = "environment-1:concurrent-import"; - const files = Array.from({ length: 8 }, (_, index) => ({ + const files = Array.from({ length: 100 }, (_, index) => ({ id: `existing-${index}`, type: "file" as const, name: `notes-${index}.txt`, @@ -906,7 +906,7 @@ describe("mobile composer drafts", () => { fileUri: `file:///documents/t3-composer-attachments/${id}.mov`, }); const draftKey = "new-task:environment-1:project-cap"; - const existing = Array.from({ length: 7 }, (_, index) => makeAttachment(`held-${index}`)); + const existing = Array.from({ length: 99 }, (_, index) => makeAttachment(`held-${index}`)); appAtomRegistry.set(composerDraftsAtom, { [draftKey]: { text: "send this", attachments: existing }, }); @@ -918,7 +918,7 @@ describe("mobile composer drafts", () => { expect(rejected).toBe(1); const draft = appAtomRegistry.get(composerDraftsAtom)[draftKey]; - expect(draft?.attachments).toHaveLength(8); + expect(draft?.attachments).toHaveLength(100); expect(draft?.attachments.at(-1)?.id).toBe("incoming-1"); await cleanup.promise; expect(composerAttachmentCleanupMocks.remove).toHaveBeenCalledExactlyOnceWith( @@ -932,7 +932,7 @@ describe("mobile composer drafts", () => { { allowOverflow: true }, ); expect(overflowRejected).toBe(0); - expect(appAtomRegistry.get(composerDraftsAtom)[draftKey]?.attachments).toHaveLength(9); + expect(appAtomRegistry.get(composerDraftsAtom)[draftKey]?.attachments).toHaveLength(101); }); it("keeps shared attachment files until every draft releases them", async () => { @@ -2270,7 +2270,7 @@ describe("mobile composer drafts", () => { previewUri: "data:image/png;base64,YWJj", }); const existingImage = image("existing"); - const sharedImages = Array.from({ length: 8 }, (_, index) => image(`shared-${index}`)); + const sharedImages = Array.from({ length: 100 }, (_, index) => image(`shared-${index}`)); const merged = mergeComposerDraftContentState( { [draftKey]: { text: "", attachments: [existingImage] } }, @@ -2278,9 +2278,9 @@ describe("mobile composer drafts", () => { { text: "", attachments: sharedImages }, ); - expect(merged[draftKey]?.attachments).toHaveLength(8); + expect(merged[draftKey]?.attachments).toHaveLength(100); expect(merged[draftKey]?.attachments[0]).toEqual(existingImage); - expect(merged[draftKey]?.attachments.at(-1)?.id).toBe("shared-6"); + expect(merged[draftKey]?.attachments.at(-1)?.id).toBe("shared-98"); }); it("restores the exact draft captured before an interrupted share import", () => { diff --git a/apps/server/src/bin.test.ts b/apps/server/src/bin.test.ts index 795bb05db148..9bc20fff84e1 100644 --- a/apps/server/src/bin.test.ts +++ b/apps/server/src/bin.test.ts @@ -15,6 +15,7 @@ import { } from "@t3tools/contracts"; import * as NetService from "@t3tools/shared/Net"; import { HostProcessEnvironment } from "@t3tools/shared/hostProcess"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as DateTime from "effect/DateTime"; @@ -101,10 +102,10 @@ const makeCliTestServerConfig = (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", mode: "web", port: 0, host: "127.0.0.1", diff --git a/apps/server/src/cli/config.test.ts b/apps/server/src/cli/config.test.ts index aea753c3398f..34ea7f685364 100644 --- a/apps/server/src/cli/config.test.ts +++ b/apps/server/src/cli/config.test.ts @@ -17,6 +17,7 @@ import { type DesktopBackendBootstrap as DesktopBackendBootstrapValue, } from "@t3tools/contracts"; import * as NetService from "@t3tools/shared/Net"; +import { DEFAULT_SIGNAL_EXPORT } from "@t3tools/shared/observability"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { deriveServerPaths } from "../config.ts"; import { resolveServerConfig } from "./config.ts"; @@ -51,10 +52,10 @@ it.layer(NodeServices.layer)("cli config resolution", (it) => { 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 9ff9a8e13b39..493e6b719416 100644 --- a/apps/server/src/cli/pair.ts +++ b/apps/server/src/cli/pair.ts @@ -15,6 +15,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, @@ -321,10 +322,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/config.ts b/apps/server/src/config.ts index 4835ebb40e44..5762ccdb6ff3 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/device/DeviceService.test.ts b/apps/server/src/device/DeviceService.test.ts index f1ed9a7f0253..183f24d95c5b 100644 --- a/apps/server/src/device/DeviceService.test.ts +++ b/apps/server/src/device/DeviceService.test.ts @@ -7,10 +7,12 @@ import { type DeviceServiceState, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; import * as Deferred from "effect/Deferred"; import * as Fiber from "effect/Fiber"; import * as PubSub from "effect/PubSub"; import * as Ref from "effect/Ref"; +import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import { HttpClient, HttpClientResponse } from "effect/unstable/http"; import { ServerSettingsService } from "../serverSettings.ts"; @@ -19,6 +21,8 @@ import { NodeRuntimeUnavailableError } from "@t3tools/shared/nodeRuntime"; import { type DeviceService, makeWithHosts, stateStream } from "./DeviceService.ts"; +const decodeJson = Schema.decodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); + const baseState: DeviceServiceState = { hosts: [], hostStatus: "idle", @@ -378,3 +382,192 @@ it.effect("keeps shutdown successful when subsequent discovery fails", () => expect(state.devices.find((device) => device.id === session.deviceId)?.booted).toBe(false); }).pipe(Effect.scoped), ); + +it.effect.each(["shutdown", "close"] as const)( + "%s releases iOS capture so reopening uses a fresh session", + (operation) => + Effect.gen(function* () { + const deviceId = DeviceId.make("11111111-1111-1111-1111-111111111111"); + const threadId = ThreadId.make("capture-recovery"); + let booted = true; + let capture: number | null = null; + let generation = 0; + const ready: DeviceHost.DeviceHostReady = { + nodePath: process.execPath, + hub: { origin: "http://device.test" }, + helpers: { serveSimAxSettings: null, serveSimCli: null }, + run: () => Effect.succeed({ code: 0, stdout: "", stderr: "" }), + }; + const host: DeviceHost.DeviceHost["Service"] = { + id: LOCAL_DEVICE_HOST_ID, + summary: Effect.succeed({ + id: LOCAL_DEVICE_HOST_ID, + kind: "local", + label: "Simulator host", + platforms: [{ platform: "ios", available: true }], + hubInstalled: true, + agentDeviceInstalled: false, + }), + platformAvailability: (platform) => Effect.succeed({ platform, available: true }), + ensureReady: () => Effect.succeed(ready), + ensureAgentReady: () => Effect.die("Agent access is not used in this test"), + current: Effect.succeed(ready), + stopAgent: Effect.void, + stop: Effect.void, + }; + const http = HttpClient.make((request) => + Effect.sync(() => { + const path = new URL(request.url).pathname; + if (path === "/api/devices") { + return HttpClientResponse.fromWeb( + request, + Response.json({ + emulators: [], + simulators: [ + { + id: deviceId, + name: "iPhone", + platform: "ios", + version: "26", + physical: false, + booted, + }, + ], + }), + ); + } + if (path === "/vendor/serve-sim/grid/api/start") capture ??= ++generation; + else if (path === "/vendor/serve-sim/grid/api/shutdown") { + if (request.body._tag !== "Uint8Array") throw new Error("Missing shutdown body"); + expect(decodeJson(new TextDecoder().decode(request.body.body))).toEqual({ + udid: deviceId, + }); + capture = null; + booted = false; + } else if (path === "/api/devices/shutdown") { + // This route powers off without releasing serve-sim's cached capture. + booted = false; + } else if (path === "/api/devices/boot") booted = true; + else throw new Error(`Unexpected hub path: ${path}`); + return HttpClientResponse.fromWeb(request, Response.json({ ok: true, id: deviceId })); + }), + ); + const service = yield* makeWithHosts(new Map([[host.id, host]])).pipe( + Effect.provideService(HttpClient.HttpClient, http), + ); + const input = { threadId, deviceId, platform: "ios" as const }; + yield* service.open(input); + expect(capture).toBe(1); + if (operation === "shutdown") yield* service.shutdown(input); + else yield* service.close({ threadId, deviceId, shutdown: true }); + expect(capture).toBeNull(); + expect((yield* service.state).sessions).toEqual([]); + yield* service.open(input); + expect(capture).toBe(2); + expect((yield* service.state).sessions).toHaveLength(1); + }).pipe( + Effect.provide(ServerSettingsService.layerTest({ enableDeviceSupport: true })), + Effect.scoped, + ), +); + +it.effect.each([ + { hubReports: "off", outcome: "succeeds" }, + { hubReports: "booted", outcome: "fails" }, + { hubReports: "missing", outcome: "fails" }, +] as const)( + "iOS shutdown $outcome when serve-sim rejects it and the hub reports the simulator $hubReports", + ({ hubReports, outcome }) => + Effect.gen(function* () { + const deviceId = DeviceId.make("22222222-2222-2222-2222-222222222222"); + const paths: string[] = []; + // The device list is stale until shutdown re-reads it from the hub. + let listed: "booted" | "off" | "missing" = "booted"; + const ready: DeviceHost.DeviceHostReady = { + nodePath: process.execPath, + hub: { origin: "http://device.test" }, + helpers: { serveSimAxSettings: null, serveSimCli: null }, + run: () => Effect.succeed({ code: 0, stdout: "", stderr: "" }), + }; + const host: DeviceHost.DeviceHost["Service"] = { + id: LOCAL_DEVICE_HOST_ID, + summary: Effect.succeed({ + id: LOCAL_DEVICE_HOST_ID, + kind: "local", + label: "Simulator host", + platforms: [{ platform: "ios", available: true }], + hubInstalled: true, + agentDeviceInstalled: false, + }), + platformAvailability: (platform) => Effect.succeed({ platform, available: true }), + ensureReady: () => Effect.succeed(ready), + ensureAgentReady: () => Effect.die("Agent access is not used in this test"), + current: Effect.succeed(ready), + stopAgent: Effect.void, + stop: Effect.void, + }; + const http = HttpClient.make((request) => + Effect.sync(() => { + const path = new URL(request.url).pathname; + paths.push(path); + if (path === "/api/devices") { + return HttpClientResponse.fromWeb( + request, + Response.json({ + emulators: [], + simulators: + listed === "missing" + ? [] + : [ + { + id: deviceId, + name: "iPhone", + platform: "ios", + version: "26", + physical: false, + booted: listed === "booted", + }, + ], + // A partial listing still decodes; it must not read as "off". + errors: listed === "missing" ? [{ message: "simctl list failed" }] : [], + }), + ); + } + if (path === "/vendor/serve-sim/grid/api/shutdown") { + // serve-sim runs `simctl shutdown` bare and returns its failure as-is. + listed = hubReports; + return HttpClientResponse.fromWeb( + request, + Response.json( + { ok: false, error: "Unable to shutdown device in current state: Shutdown" }, + { status: 500 }, + ), + ); + } + throw new Error(`Unexpected hub path: ${path}`); + }), + ); + const service = yield* makeWithHosts(new Map([[host.id, host]])).pipe( + Effect.provideService(HttpClient.HttpClient, http), + ); + yield* service.list; + const exit = yield* Effect.exit(service.shutdown({ deviceId, platform: "ios" })); + expect(paths.filter((path) => path.endsWith("shutdown"))).toEqual([ + "/vendor/serve-sim/grid/api/shutdown", + ]); + if (outcome === "succeeds") { + expect(Exit.isSuccess(exit)).toBe(true); + expect( + (yield* service.state).devices.find((device) => device.id === deviceId)?.booted, + ).toBe(false); + } else { + expect(Exit.isFailure(exit)).toBe(true); + expect( + (yield* service.state).devices.find((device) => device.id === deviceId)?.booted, + ).toBe(true); + } + }).pipe( + Effect.provide(ServerSettingsService.layerTest({ enableDeviceSupport: true })), + Effect.scoped, + ), +); diff --git a/apps/server/src/device/DeviceService.ts b/apps/server/src/device/DeviceService.ts index 30e1f18c0497..45a2f928ecaa 100644 --- a/apps/server/src/device/DeviceService.ts +++ b/apps/server/src/device/DeviceService.ts @@ -670,25 +670,44 @@ export const makeWithHosts = Effect.fn("DeviceService.makeWithHosts")(function* platform: DevicePlatform, ) { const ready = yield* readiness(hostId); - yield* HttpClientRequest.post(`${ready.hub.origin}/api/devices/shutdown`).pipe( - HttpClientRequest.bodyJson({ platform, id: deviceId }), - Effect.mapError( - (cause) => - new DeviceOperationError({ operation: "shutdown", reason: "invalid_payload", cause }), - ), - Effect.flatMap((request) => hubJson(request, HubActionResult, "shutdown")), - Effect.flatMap((result) => - result.ok - ? Effect.void - : Effect.fail( - new DeviceOperationError({ - operation: "shutdown", - reason: "hub_rejected", - cause: result, - }), + const postShutdown = (path: string, body: Record) => + HttpClientRequest.post(`${ready.hub.origin}${path}`).pipe( + HttpClientRequest.bodyJson(body), + Effect.mapError( + (cause) => + new DeviceOperationError({ operation: "shutdown", reason: "invalid_payload", cause }), + ), + Effect.flatMap((request) => hubJson(request, HubActionResult, "shutdown")), + Effect.flatMap((result) => + result.ok + ? Effect.void + : Effect.fail( + new DeviceOperationError({ + operation: "shutdown", + reason: "hub_rejected", + cause: result, + }), + ), + ), + ); + // serve-sim's shutdown closes its in-process capture session before it runs + // `simctl shutdown`; the hub's generic shutdown can leave that session cached + // across a reboot. serve-sim runs simctl bare, though, so a simulator that is + // already off fails there. Accept that failure only when the hub confirms + // the simulator is off; a failure on a running one still surfaces. + yield* platform === "ios" + ? postShutdown(`${vendorPrefix("ios")}/grid/api/shutdown`, { udid: deviceId }).pipe( + Effect.catch((cause) => + fetchDevices(ready).pipe( + Effect.flatMap(({ devices }) => + devices.find((device) => device.id === deviceId)?.booted === false + ? Effect.logInfo("iOS simulator was already shut down", { deviceId }) + : Effect.fail(cause), + ), ), - ), - ); + ), + ) + : postShutdown("/api/devices/shutdown", { platform, id: deviceId }); yield* publish((state) => ({ ...state, devices: state.devices.map((device) => diff --git a/apps/server/src/device/DeviceToolchain.ts b/apps/server/src/device/DeviceToolchain.ts index 4d59cb462383..0493902302b3 100644 --- a/apps/server/src/device/DeviceToolchain.ts +++ b/apps/server/src/device/DeviceToolchain.ts @@ -25,9 +25,9 @@ import * as Semaphore from "effect/Semaphore"; import * as ProcessRunner from "../processRunner.ts"; const DEVICE_HUB_PACKAGE = "expo-device-hub"; -export const DEVICE_HUB_VERSION = "0.9.0"; +export const DEVICE_HUB_VERSION = "0.10.1"; const AGENT_DEVICE_PACKAGE = "agent-device"; -export const AGENT_DEVICE_VERSION = "0.20.10"; +export const AGENT_DEVICE_VERSION = "0.21.7"; const INSTALL_TIMEOUT = Duration.minutes(10); const installLock = Semaphore.makeUnsafe(1); diff --git a/apps/server/src/environment/ServerEnvironment.test.ts b/apps/server/src/environment/ServerEnvironment.test.ts index 0ccf8de691eb..b4758e980065 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/Normalizer.attachments.test.ts b/apps/server/src/orchestration/Normalizer.attachments.test.ts index 51c70d7401ae..d44ec58e6a95 100644 --- a/apps/server/src/orchestration/Normalizer.attachments.test.ts +++ b/apps/server/src/orchestration/Normalizer.attachments.test.ts @@ -13,6 +13,7 @@ import { ThreadId, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Schema from "effect/Schema"; @@ -59,6 +60,53 @@ function turnStartCommand(input: { } describe("normalizeDispatchCommand attachments", () => { + it.effect("accepts 100 inline images and rejects 101 before writing files", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const attachments = Array.from({ length: 100 }, () => ({ + dataUrl: "data:image/png;base64,cGl4ZWxz", + sizeBytes: 6, + })); + const rejected = yield* normalizeDispatchCommand( + turnStartCommand({ attachments: [...attachments, attachments[0]!] }), + ).pipe(Effect.flip); + expect(rejected.message).toContain("up to 100"); + expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([]); + const accepted = yield* normalizeDispatchCommand(turnStartCommand({ attachments })); + if (accepted.type !== "thread.turn.start") throw new Error("Wrong command"); + expect(accepted.message.attachments).toHaveLength(100); + expect(NodeFS.readdirSync(config.attachmentsDir)).toHaveLength(100); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects decoded image overflow before writing it and removes earlier files", () => + Effect.gen(function* () { + const config = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + let writtenBytes = 0; + const dataUrl = `data:image/png;base64,${Buffer.alloc(10 * 1024 * 1024).toString("base64")}`; + const command = turnStartCommand({ + attachments: [ + ...Array.from({ length: 8 }, () => ({ dataUrl, sizeBytes: 1 })), + { dataUrl: "data:image/png;base64,YQ==", sizeBytes: 0 }, + ], + }); + const error = yield* normalizeDispatchCommand(command).pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fileSystem, + writeFile: (path, data, options) => { + writtenBytes += data.byteLength; + return fileSystem.writeFile(path, data, options); + }, + }), + Effect.flip, + ); + expect(error.message).toContain("80 MiB"); + expect(writtenBytes).toBe(80 * 1024 * 1024); + expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([]); + }).pipe(Effect.provide(testLayer)), + ); + it.effect("rejects duplicate client ids before persisting attachments", () => Effect.gen(function* () { const error = yield* normalizeDispatchCommand( @@ -464,25 +512,25 @@ describe("question attachments", () => { answers: { first: "", second: "" }, createdAt: "2026-08-01T00:00:00.000Z", attachmentsByQuestionId: { - first: Array.from({ length: 4 }, () => attachment), - second: Array.from({ length: 5 }, () => attachment), + first: Array.from({ length: 50 }, () => attachment), + second: Array.from({ length: 51 }, () => attachment), }, }; const failure = yield* normalizeDispatchCommand(command).pipe(Effect.flip); - expect(failure.message).toContain("up to 8"); + expect(failure.message).toContain("up to 100"); expect(NodeFS.readdirSync(config.attachmentsDir)).toEqual([`${id}.txt`]); const accepted = { ...command, attachmentsByQuestionId: { ...command.attachmentsByQuestionId, - second: Array.from({ length: 4 }, () => attachment), + second: Array.from({ length: 50 }, () => attachment), }, }; const normalized = yield* normalizeDispatchCommand(accepted); if (normalized.type !== "thread.user-input.respond") throw new Error("Wrong command"); const attachments = Object.values(normalized.attachmentsByQuestionId!).flat(); - expect(attachments).toHaveLength(8); - expect(new Set(attachments.map((item) => item.id)).size).toBe(8); + expect(attachments).toHaveLength(100); + expect(new Set(attachments.map((item) => item.id)).size).toBe(100); for (const item of attachments) { expect(item.name).toBe(attachment.name); expect( diff --git a/apps/server/src/orchestration/Normalizer.ts b/apps/server/src/orchestration/Normalizer.ts index bef58adc0581..d4959f39af59 100644 --- a/apps/server/src/orchestration/Normalizer.ts +++ b/apps/server/src/orchestration/Normalizer.ts @@ -5,7 +5,7 @@ import * as Path from "effect/Path"; import { type ClientOrchestrationCommand, type UserInputAttachments, - PROVIDER_SEND_TURN_MAX_ATTACHMENTS, + getProviderAttachmentLimitError, type IsoDateTime, type OrchestrationCommand, OrchestrationDispatchCommandError, @@ -142,13 +142,9 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => canonicalCommand.type === "thread.turn.start" ? canonicalCommand.message.attachments : Object.values(canonicalCommand.attachmentsByQuestionId ?? {}).flat(); - if ( - canonicalCommand.type === "thread.user-input.respond" && - attachments.length > PROVIDER_SEND_TURN_MAX_ATTACHMENTS - ) { - return yield* new OrchestrationDispatchCommandError({ - message: `You can attach up to ${PROVIDER_SEND_TURN_MAX_ATTACHMENTS} files per question response.`, - }); + const attachmentLimitError = getProviderAttachmentLimitError(attachments); + if (attachmentLimitError) { + return yield* new OrchestrationDispatchCommandError({ message: attachmentLimitError }); } if (canonicalCommand.type === "thread.turn.start") { const clientAttachmentIds = new Set(); @@ -163,11 +159,12 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => } } const claimedAttachmentPaths: string[] = []; + const attachmentsWithDecodedSizes = [...attachments]; // Context records bind to attachments by the id the client knew; they follow the rename. const finalAttachmentIdByClientId = new Map(); const normalizedAttachments = yield* Effect.forEach( attachments, - (attachment) => + (attachment, index) => Effect.gen(function* () { if (!("dataUrl" in attachment)) { const claim = planAttachmentClaim({ @@ -259,6 +256,11 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => sizeBytes: bytes.byteLength, ...(attachment.source ? { source: attachment.source } : {}), }; + attachmentsWithDecodedSizes[index] = persistedAttachment; + const decodedLimitError = getProviderAttachmentLimitError(attachmentsWithDecodedSizes); + if (decodedLimitError) { + return yield* new OrchestrationDispatchCommandError({ message: decodedLimitError }); + } const attachmentPath = resolveAttachmentPath({ attachmentsDir: serverConfig.attachmentsDir, @@ -286,6 +288,7 @@ export const normalizeDispatchCommand = (command: ClientOrchestrationCommand) => }), ), ); + claimedAttachmentPaths.push(attachmentPath); if (attachment.id !== undefined) { finalAttachmentIdByClientId.set(attachment.id, attachmentId); } diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 9d563d3965c3..a0d111bd710f 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -3404,6 +3404,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 a653ef94c713..6b4b2bd8bfd5 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -611,6 +611,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); @@ -1076,6 +1082,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 { @@ -1098,6 +1105,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 }), @@ -1246,7 +1254,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. @@ -1262,7 +1271,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: @@ -1323,7 +1332,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) { @@ -1384,7 +1394,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: @@ -1551,7 +1561,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, @@ -1564,6 +1575,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 @@ -1634,12 +1646,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, @@ -1664,6 +1676,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, @@ -2894,12 +2907,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..81033e2e0d94 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -97,6 +97,40 @@ describe("pull request list decoding", () => { ]); }); + it("takes the verdict from the latest reviews when GitHub summarizes none, as for a bot's approval", () => { + const batch = expectSuccess( + decodePullRequestListJson( + listJson([ + { + reviewDecision: null, + latestReviews: [{ author: { login: "macroscopeapp" }, state: "APPROVED" }], + }, + { + reviewDecision: "REVIEW_REQUIRED", + latestReviews: [ + { author: { login: "octocat" }, state: "APPROVED" }, + { author: { login: "hubot" }, state: "CHANGES_REQUESTED" }, + ], + }, + { + reviewDecision: "APPROVED", + latestReviews: [{ author: { login: "hubot" }, state: "CHANGES_REQUESTED" }], + }, + { + reviewDecision: null, + latestReviews: [{ author: { login: "octocat" }, state: "COMMENTED" }], + }, + ]), + ), + ); + expect(batch.items.map((entry) => entry.reviewDecision)).toEqual([ + "approved", + "changes-requested", + "approved", + null, + ]); + }); + it("rolls the head commit's checks up to the one word a row has space for", () => { const batch = expectSuccess( decodePullRequestListJson( @@ -121,6 +155,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 +173,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..eaaa4e34da84 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -68,6 +68,12 @@ const RawReviewRequestSchema = Schema.Struct({ name: Schema.optional(Schema.NullOr(Schema.String)), }); +/** One reviewer's most recent review: the state is all the verdict needs, the author is for who. */ +const RawLatestReviewSchema = Schema.Struct({ + author: Schema.optional(Schema.NullOr(RawActorSchema)), + state: Schema.optional(Schema.NullOr(Schema.String)), +}); + const RawCheckSchema = Schema.Struct({ __typename: Schema.optional(Schema.String), name: Schema.optional(Schema.NullOr(Schema.String)), @@ -106,6 +112,7 @@ const RawListItemSchema = Schema.Struct({ updatedAt: Schema.String, mergedAt: Schema.optional(Schema.NullOr(Schema.String)), reviewRequests: Schema.optional(Schema.Array(RawReviewRequestSchema)), + latestReviews: Schema.optional(Schema.NullOr(Schema.Array(RawLatestReviewSchema))), labels: Schema.optional(Schema.Array(RawLabelSchema)), /** * Every check of the head commit, which is the only rollup `gh pr list --json` can give: there @@ -143,6 +150,9 @@ const RawSearchItemSchema = Schema.Struct({ isDraft: Schema.optional(Schema.Boolean), mergeable: Schema.optional(Schema.NullOr(Schema.String)), reviewDecision: Schema.optional(Schema.NullOr(Schema.String)), + latestReviews: Schema.optional( + Schema.NullOr(Schema.Struct({ nodes: Schema.Array(Schema.NullOr(RawLatestReviewSchema)) })), + ), createdAt: Schema.String, updatedAt: Schema.String, mergedAt: Schema.optional(Schema.NullOr(Schema.String)), @@ -514,13 +524,7 @@ const RawReviewThreadsSchema = Schema.Struct({ ), ), latestReviews: Schema.optional( - Schema.NullOr( - Schema.Struct({ - nodes: Schema.Array( - Schema.Struct({ author: Schema.optional(Schema.NullOr(RawActorSchema)) }), - ), - }), - ), + Schema.NullOr(Schema.Struct({ nodes: Schema.Array(RawLatestReviewSchema) })), ), reviewDismissals: Schema.optional( Schema.NullOr( @@ -698,7 +702,7 @@ export function decodeActorAvatarsJson( } export const PULL_REQUEST_LIST_JSON_FIELDS = - "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,labels,statusCheckRollup"; + "number,title,url,author,headRefName,baseRefName,state,isDraft,mergeable,reviewDecision,additions,deletions,createdAt,updatedAt,mergedAt,reviewRequests,latestReviews,labels,statusCheckRollup"; export const PULL_REQUEST_DETAIL_JSON_FIELDS = `${PULL_REQUEST_LIST_JSON_FIELDS},body,changedFiles,closedAt,isCrossRepository,headRepositoryOwner,headRefOid,autoMergeRequest`; @@ -817,6 +821,7 @@ export function pullRequestSearchGraphQlQuery(rows: number, includeStacks = fals isDraft mergeable reviewDecision + latestReviews(first: 20) { nodes { state author { login } } } createdAt updatedAt mergedAt @@ -891,7 +896,7 @@ export const REVIEW_THREADS_GRAPHQL_QUERY = `query($owner: String!, $name: Strin } } latestReviews(first: 50) { - nodes { author { __typename login avatarUrl } } + nodes { state author { __typename login avatarUrl } } } reviewDismissals: timelineItems(itemTypes: [REVIEW_DISMISSED_EVENT], first: ${GRAPHQL_PAGE_SIZE}) { pageInfo { hasNextPage endCursor } @@ -1325,6 +1330,35 @@ function toMergeMethod(value: string | null | undefined): PullRequestMergeMethod } } +/** + * GitHub's own `reviewDecision` counts only reviews that satisfy the branch rules, so an + * approval from an app (a review bot) or from anyone without the required permission leaves it + * empty. The reviewers still said something, and a row should show it: when GitHub reports no + * verdict, the latest review per reviewer decides, changes requested outranking approval. + */ +function toReviewDecisionWithReviews( + value: string | null | undefined, + // `gh pr list` hands the reviews as an array; the GraphQL reads hand a connection. + latestReviews: + | ReadonlyArray> + | { readonly nodes: ReadonlyArray> } + | null + | undefined, +): PullRequestReviewDecision | null { + const summarized = toReviewDecision(value); + if (summarized === "approved" || summarized === "changes-requested") return summarized; + const reviews = + latestReviews === null || latestReviews === undefined + ? [] + : "nodes" in latestReviews + ? latestReviews.nodes + : latestReviews; + const states = new Set(reviews.map((review) => review.state?.trim().toUpperCase() ?? "")); + if (states.has("CHANGES_REQUESTED")) return "changes-requested"; + if (states.has("APPROVED")) return "approved"; + return summarized; +} + function toReviewDecision(value: string | null | undefined): PullRequestReviewDecision | null { switch (value?.trim().toUpperCase()) { case "APPROVED": @@ -1447,8 +1481,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 +1498,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; } @@ -1557,7 +1592,7 @@ function toListItem(raw: Schema.Schema.Type): GitHubPu state: toState(raw), isDraft: raw.isDraft ?? false, mergeability: toMergeability(raw.mergeable), - reviewDecision: toReviewDecision(raw.reviewDecision), + reviewDecision: toReviewDecisionWithReviews(raw.reviewDecision, raw.latestReviews), additions: raw.additions ?? 0, deletions: raw.deletions ?? 0, createdAt: raw.createdAt, @@ -1680,6 +1715,9 @@ export function decodePullRequestSearchJson( items.push({ ...toListItem({ ...node, + latestReviews: (node.latestReviews?.nodes ?? []).flatMap((review) => + review === null ? [] : [review], + ), reviewRequests: (node.reviewRequests?.nodes ?? []).flatMap((request) => { const login = trimmed(request?.requestedReviewer?.login); return login === null ? [] : [{ login }]; diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 2618020d388d..5d46c866a165 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -218,7 +218,7 @@ import { transferBudgetViolations, } from "../integration/TransferBudgetReport.integration.ts"; import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; -import { otlpSerializationLayer } from "@t3tools/shared/observability"; +import { DEFAULT_SIGNAL_EXPORT, otlpSerializationLayer } from "@t3tools/shared/observability"; const defaultProjectId = ProjectId.make("project-default"); const defaultThreadId = ThreadId.make("thread-default"); @@ -578,10 +578,10 @@ const buildAppUnderTest = (options?: { 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: "desktop", port: 0, host: "127.0.0.1", @@ -1076,7 +1076,7 @@ const buildAppUnderTest = (options?: { ...options?.layers?.browserTraceCollector, }), ), - Layer.provide(otlpSerializationLayer(config.otlpProtocol)), + Layer.provide(otlpSerializationLayer(config.otlpTracesExport.protocol)), Layer.provide( Layer.mock(ServerLifecycleEvents.ServerLifecycleEvents)({ publish: (event) => Effect.succeed({ ...(event as any), sequence: 1 }), @@ -5299,7 +5299,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { yield* buildAppUnderTest({ config: { otlpTracesUrl: collector.url, - otlpProtocol: "http/protobuf", + otlpTracesExport: { ...DEFAULT_SIGNAL_EXPORT, protocol: "http/protobuf" }, }, layers: { browserTraceCollector: { 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 3330f40a090f..cdf3aeffbea1 100644 --- a/apps/web/src/components/BranchToolbar.tsx +++ b/apps/web/src/components/BranchToolbar.tsx @@ -233,6 +233,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ { if (autoEnvironmentLabel) onAutoEnvironment?.(); }} @@ -250,6 +251,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ key={env.environmentId} disabled={envLocked} value={env.environmentId} + closeOnClick > @@ -274,7 +276,7 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ onEnvModeChange(value as EnvMode); }} > - + {activeWorktreePath ? ( @@ -286,14 +288,14 @@ const MobileRunContextSelector = memo(function MobileRunContextSelector({ - + {resolveEnvModeLabel("worktree")} {previousWorktreeLabel ? ( - + {previousWorktreeLabel} diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index d958f2f76625..a7fa1ce99724 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -545,6 +545,7 @@ export function BranchToolbarBranchSelector({ setIsBranchMenuOpen(open); if (!open) { setBranchQuery(""); + highlightedBranchValueRef.current = null; } }, []); @@ -594,6 +595,9 @@ export function BranchToolbarBranchSelector({ }, [fetchNextBranchPage, hasNextPage, isBranchMenuOpen, isFetchingNextPage]); const branchListRef = useRef(null); + // Tracks the highlighted picker value so Enter can activate it even when the + // virtualized row is not mounted (Base UI Enter clicks the mounted element). + const highlightedBranchValueRef = useRef(null); const updateBranchListScrollFades = useCallback(() => { const scrollElement = branchListRef.current?.getScrollableNode?.(); if (!(scrollElement instanceof HTMLElement)) { @@ -676,6 +680,20 @@ export function BranchToolbarBranchSelector({ const prUrl = currentLinkedPr?.url ?? displayedPr?.url; const openPrLink = useOpenPrLink(threadRef); + function selectPickerItem(itemValue: string) { + highlightedBranchValueRef.current = null; + if (itemValue === checkoutPullRequestItemValue && prReference && onCheckoutPullRequestRequest) { + handleOpenChange(false); + onComposerFocusRequest?.(); + onCheckoutPullRequestRequest(prReference); + } else if (itemValue === createBranchItemValue) { + createRef(trimmedBranchQuery); + } else { + const refName = branchByName.get(itemValue); + if (refName) selectBranch(refName); + } + } + function renderPickerItem(itemValue: string, index: number) { if (checkoutPullRequestItemValue && itemValue === checkoutPullRequestItemValue) { return ( @@ -685,15 +703,7 @@ export function BranchToolbarBranchSelector({ index={index} value={itemValue} className="pe-2" - onClick={() => { - if (!prReference || !onCheckoutPullRequestRequest) { - return; - } - setIsBranchMenuOpen(false); - setBranchQuery(""); - onComposerFocusRequest?.(); - onCheckoutPullRequestRequest(prReference); - }} + onClick={() => selectPickerItem(itemValue)} >
@@ -715,7 +725,7 @@ export function BranchToolbarBranchSelector({ index={index} value={itemValue} className="pe-1.5" - onClick={() => createRef(trimmedBranchQuery)} + onClick={() => selectPickerItem(itemValue)} > Create new ref "{newRefName}" @@ -743,7 +753,7 @@ export function BranchToolbarBranchSelector({ index={index} value={itemValue} className="pe-1.5" - onClick={() => selectBranch(refName)} + onClick={() => selectPickerItem(itemValue)} onContextMenu={(event) => handleBranchContextMenu(event, itemValue)} >
@@ -760,7 +770,8 @@ export function BranchToolbarBranchSelector({ filteredItems={filteredBranchPickerItems} autoHighlight virtualized - onItemHighlighted={(_value, eventDetails) => { + onItemHighlighted={(value, eventDetails) => { + highlightedBranchValueRef.current = typeof value === "string" ? value : null; if (!isBranchMenuOpen || eventDetails.index < 0 || eventDetails.reason !== "keyboard") { return; } @@ -828,6 +839,24 @@ export function BranchToolbarBranchSelector({ placeholder="Search refs..." value={branchQuery} onChange={(event) => setBranchQuery(event.target.value)} + onKeyDown={(event) => { + if (event.key !== "Enter" || event.nativeEvent.isComposing || event.keyCode === 229) { + return; + } + const highlightedValue = highlightedBranchValueRef.current; + if ( + highlightedValue === null || + !filteredBranchPickerItems.includes(highlightedValue) + ) { + return; + } + ( + event as typeof event & { preventBaseUIHandler?: () => void } + ).preventBaseUIHandler?.(); + event.preventDefault(); + event.stopPropagation(); + selectPickerItem(highlightedValue); + }} />
No refs found. diff --git a/apps/web/src/components/ChatMarkdown.test.tsx b/apps/web/src/components/ChatMarkdown.test.tsx index 4e22e68ca902..3e1222e0ebb9 100644 --- a/apps/web/src/components/ChatMarkdown.test.tsx +++ b/apps/web/src/components/ChatMarkdown.test.tsx @@ -55,6 +55,7 @@ vi.mock("../editorPreferences", () => ({ vi.mock("~/lib/openPullRequestLink", () => ({ findProjectOnChangeRequestHost: () => undefined, parseChangeRequestUrl: () => null, + resolvePullRequestPreviewTarget: () => null, useOpenChangeRequestLink: () => vi.fn(), })); diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index abebe4841cbd..3b398dd5100e 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -53,7 +53,6 @@ import { inlineCodeFilePathCandidate } from "@t3tools/client-runtime/markdown-li import { mediaFileReference, mediaUrlReference } from "@t3tools/client-runtime/media-reference"; import { mediaKindFromPath, mediaMimeTypeFromExtension } from "@t3tools/shared/filePreview"; import * as Cause from "effect/Cause"; -import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl"; import { AsyncResult } from "effect/unstable/reactivity"; import React, { Children, @@ -178,9 +177,9 @@ import { WORKSPACE_BASENAME_LOOKUP_LIMIT, } from "../workspaceBasenameLookup"; import { - findProjectForChangeRequest, parseChangeRequestUrl, pullRequestCandidateUrlFromReferenceAutolink, + resolvePullRequestPreviewTarget, useOpenChangeRequestLink, } from "~/lib/openPullRequestLink"; import { useOpenLink } from "../browser/useOpenLink"; @@ -863,7 +862,10 @@ function MarkdownDetails({ {summary} -
+
{content}
@@ -2886,32 +2888,14 @@ const CHAT_MARKDOWN_COMPONENTS = { const confirmBeforeOpen = pullRequestAutolink === "reference"; const pullRequestCandidateUrl = confirmBeforeOpen && href ? pullRequestCandidateUrlFromReferenceAutolink(href) : href; - const pullRequestCandidate = pullRequestCandidateUrl - ? parseChangeRequestUrl(pullRequestCandidateUrl) + const pullRequestPreviewTarget = pullRequestCandidateUrl + ? resolvePullRequestPreviewTarget({ + environmentId, + projects, + pullRequestsEnabled: serverConfig?.environment.capabilities.pullRequests === true, + url: pullRequestCandidateUrl, + }) : null; - const pullRequestProject = - environmentId !== null && - serverConfig?.environment.capabilities.pullRequests === true && - pullRequestCandidate !== null - ? findProjectForChangeRequest( - projects.filter((project) => project.environmentId === environmentId), - pullRequestCandidate, - ) - : undefined; - const pullRequestPreviewTarget = - environmentId === null || pullRequestProject === undefined || pullRequestCandidate === null - ? null - : { - environmentId, - input: { - projectId: pullRequestProject.id, - host: pullRequestCandidate.authority ?? pullRequestCandidate.host, - repository: - sourceControlRepositorySelector(pullRequestProject.repositoryIdentity) ?? - pullRequestCandidate.repository, - number: pullRequestCandidate.number, - }, - }; const isSameDocumentLink = href?.startsWith("#") ?? false; const onClick = props.onClick; const canOpenInPreview = Boolean(threadRef) && isPreviewSupportedInRuntime(); @@ -3332,7 +3316,7 @@ function ChatMarkdown({
({ vi.mock("~/lib/openPullRequestLink", () => ({ findProjectOnChangeRequestHost: () => undefined, parseChangeRequestUrl: () => null, + resolvePullRequestPreviewTarget: () => null, useOpenChangeRequestLink: () => vi.fn(), })); diff --git a/apps/web/src/components/ComposerPromptEditorTiptap.tsx b/apps/web/src/components/ComposerPromptEditorTiptap.tsx index 5126246dd05c..c108207304c4 100644 --- a/apps/web/src/components/ComposerPromptEditorTiptap.tsx +++ b/apps/web/src/components/ComposerPromptEditorTiptap.tsx @@ -725,6 +725,19 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { ); }, []); + const editorAttributes = useMemo( + () => ({ + 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: [ @@ -787,15 +800,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) && @@ -995,6 +1000,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/PullRequestThreadDialog.tsx b/apps/web/src/components/PullRequestThreadDialog.tsx index 4004b4930c27..ddbd2876d4d2 100644 --- a/apps/web/src/components/PullRequestThreadDialog.tsx +++ b/apps/web/src/components/PullRequestThreadDialog.tsx @@ -222,9 +222,12 @@ export function PullRequestThreadDialog({ if (event.key !== "Enter") { return; } + if (event.nativeEvent.isComposing || event.keyCode === 229) { + return; + } event.preventDefault(); if (!isResolving && !preparePullRequestThreadAction.isPending) { - void handleConfirm("local"); + void handleConfirm("worktree"); } }} /> diff --git a/apps/web/src/components/RightPanelTabs.tsx b/apps/web/src/components/RightPanelTabs.tsx index f234efff093e..e1bc3f151fd9 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"; @@ -62,7 +63,11 @@ import { ScrollArea } from "~/components/ui/scroll-area"; import { PanelTabCloseButton } from "~/components/ui/panel-tab-close-button"; import { faviconUrlForOrigin } from "~/lib/favicon"; import { useTheme } from "~/hooks/useTheme"; -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"; @@ -800,19 +805,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 016a2c2e431d..5b2945042ac2 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -76,15 +76,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/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index 255f61a0c752..3fe6e740fb9f 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -206,6 +206,7 @@ import { import { useOpenPrLink } from "~/lib/openPullRequestLink"; import { collectInlineContextIds, + stripInlineContextReferences, type ComposerContextReference, ensureInlineContextReferences, formatInlineContextReference, @@ -1626,6 +1627,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) const previewFile = composerFiles.find((file) => file.id === previewFileId); const composerContextActions = useMemo( () => ({ + environmentId, expandImage: (imageId: string) => { const preview = buildExpandedImagePreview(composerImages, imageId); if (preview) onExpandImage(preview); @@ -5240,6 +5242,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) options?: { readonly source?: ChatFileAttachment["source"]; readonly selection?: { start: number; end: number }; + readonly skipImageInlineChip?: boolean; }, ): Promise => { if (!activeThreadId || files.length === 0 || isRevertingCheckpointRef.current) return false; @@ -5259,6 +5262,20 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) // large image is being compressed, and the attachments and errors belong // to the thread the paste happened in. const threadId = activeThreadId; + // Images landing with no prose live on the shelf with no chip. Read before + // the awaits below: compression is async and the prompt may change while it + // runs. An explicit selection replace and states where the editor refuses + // input (connecting, approval, pending questions, project selection) still + // get chips so the image is never invisible, unless paste-as-text explicitly + // requests no inline image chip. + const imageAttachmentsGetChips = + !options?.skipImageInlineChip && + (options?.selection !== undefined || + isConnecting || + isComposerApprovalState || + pendingUserInputs.length > 0 || + projectSelectionRequired || + stripInlineContextReferences(promptRef.current).trim().length > 0); // Validation happens synchronously so concurrent pastes see each other: // accepted files reserve their attachment slots (via the pending counter) @@ -5415,7 +5432,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) : [], ); const storedImages = nextImages.filter((image) => storedImageIds.has(image.id)); - if (storedImages.length > 0) { + if (storedImages.length > 0 && imageAttachmentsGetChips) { insertedAny = insertAttachmentReferences(storedImages.map(imageContextReference)) || insertedAny; } @@ -5443,6 +5460,8 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) /** * Chips for freshly attached files land at the caret; when the editor cannot take * input (approval, pending questions) they are appended so the file is never invisible. + * Images skip this when they land with no prose and the editor takes input: + * the shelf thumbnail is enough. */ const insertAttachmentReferences = ( references: ReadonlyArray, @@ -5581,7 +5600,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ) { event.preventDefault(); event.stopPropagation(); - void addComposerAttachments(files); + void addComposerAttachments(files, { skipImageInlineChip: bypassAutoAttachment }); return; } diff --git a/apps/web/src/components/chat/ComposerBannerStack.test.tsx b/apps/web/src/components/chat/ComposerBannerStack.test.tsx new file mode 100644 index 000000000000..d2e9a649df10 --- /dev/null +++ b/apps/web/src/components/chat/ComposerBannerStack.test.tsx @@ -0,0 +1,103 @@ +import { cloneElement, type ReactElement, type ReactNode } from "react"; +import { act, create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, expect, it, vi } from "vite-plus/test"; + +import { ComposerBannerStack } from "./ComposerBannerStack"; + +vi.mock("../ui/popover", () => ({ + Popover: "popover", + PopoverTrigger: ({ render, children }: { render: ReactElement; children: ReactNode }) => + cloneElement(render, {}, children), + PopoverPopup: "popup", +})); +vi.mock("../ui/button", () => ({ Button: "button" })); +vi.mock("../ui/scroll-area", () => ({ ScrollArea: "div" })); + +let renderer: ReactTestRenderer; +afterEach(async () => { + if (renderer) await act(() => renderer.unmount()); + vi.unstubAllGlobals(); +}); + +it("only offers notice details when the description cannot fit", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + let resize = () => {}; + let mutate = () => {}; + vi.stubGlobal( + "MutationObserver", + class { + constructor(callback: () => void) { + mutate = callback; + } + observe() {} + disconnect() {} + }, + ); + vi.stubGlobal( + "ResizeObserver", + class { + constructor(callback: () => void) { + resize = callback; + } + observe() {} + disconnect() {} + }, + ); + let position = "static"; + vi.stubGlobal("getComputedStyle", () => ({ position })); + let availableWidth = 200; + const nested = { clientWidth: 100, scrollWidth: 80 }; + const text = { + querySelectorAll: () => [nested], + get clientWidth() { + return ( + availableWidth - + (renderer?.root.findAllByProps({ "aria-label": "Show notice details" }).length ? 28 : 0) + ); + }, + scrollWidth: 80, + }; + await act(() => { + renderer = create( + , + { + createNodeMock: (element) => + element.type === "span" ? text : element.type === "button" ? { offsetWidth: 24 } : null, + }, + ); + }); + const details = () => renderer.root.findAllByProps({ "aria-label": "Show notice details" }); + expect(details()).toHaveLength(0); + text.scrollWidth = 300; + await act(() => resize()); + expect(details()).toHaveLength(1); + // It fits without the icon: the icon must not keep its own overflow alive. + availableWidth = 308; + await act(() => resize()); + expect(details()).toHaveLength(0); + text.scrollWidth = 80; + await act(() => resize()); + expect(details()).toHaveLength(0); + nested.scrollWidth = 500; + await act(() => mutate()); + expect(details()).toHaveLength(1); + nested.scrollWidth = 80; + await act(() => mutate()); + expect(details()).toHaveLength(0); + position = "absolute"; + await act(() => resize()); + expect(details()).toHaveLength(1); + position = "static"; + await act(() => resize()); + expect(details()).toHaveLength(0); +}); diff --git a/apps/web/src/components/chat/ComposerBannerStack.tsx b/apps/web/src/components/chat/ComposerBannerStack.tsx index b47790b96039..ec4b44259321 100644 --- a/apps/web/src/components/chat/ComposerBannerStack.tsx +++ b/apps/web/src/components/chat/ComposerBannerStack.tsx @@ -242,6 +242,82 @@ export function ComposerBannerStack({ className, items }: ComposerBannerStackPro ); } +/** Keep full descriptions reachable only when their inline copy is clipped. */ +function NoticeDescription({ children, compact }: { children: ReactNode; compact?: boolean }) { + const descriptionRef = useRef(null); + const detailsRef = useRef(null); + const [showDetails, setShowDetails] = useState(false); + + useLayoutEffect(() => { + const description = descriptionRef.current; + if (!description) return; + const measure = () => { + // Ignore the space taken by the details button itself so it cannot + // sustain its own overflow after the description would otherwise fit. + const recoveredWidth = detailsRef.current ? detailsRef.current.offsetWidth + 4 : 0; + const hidden = getComputedStyle(description).position === "absolute"; + setShowDetails( + hidden || + [description, ...description.querySelectorAll("*")].some( + (element) => element.scrollWidth > element.clientWidth + recoveredWidth, + ), + ); + }; + measure(); + const observer = new ResizeObserver(measure); + observer.observe(description); + // A child can reveal new text without resizing its clipped box. + const mutations = new MutationObserver(measure); + mutations.observe(description, { childList: true, subtree: true, characterData: true }); + return () => { + observer.disconnect(); + mutations.disconnect(); + }; + }, []); + + return ( + + + {children} + + {showDetails ? ( + + + } + > + + + + + {children} + + + + ) : null} + + ); +} + function ComposerBannerStackAlert({ item, attached, @@ -278,44 +354,9 @@ function ComposerBannerStackAlert({ {item.title} {item.description ? ( - - - {item.description} - - - - } - > - - - - - {item.description} - - - - + + {item.description} + ) : null} {item.actions || item.onDismiss ? ( diff --git a/apps/web/src/components/chat/DraftHeroHeadline.tsx b/apps/web/src/components/chat/DraftHeroHeadline.tsx index 07956c2b299e..d542cd6a8a9f 100644 --- a/apps/web/src/components/chat/DraftHeroHeadline.tsx +++ b/apps/web/src/components/chat/DraftHeroHeadline.tsx @@ -136,10 +136,11 @@ export function DraftHeroHeadline({ + // The trigger's accessible name comes from its visible text (the + // project title) so the hero sentence reads naturally: an + // aria-label here would replace the title with an action phrase + // mid-sentence and baffle screen-reader users. + } > {activeProjectDisplayName ?? "Choose a project"} @@ -233,8 +234,21 @@ export function DraftHeroHeadline({ ); + // 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 42a45b991028..161adc8be00d 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -1778,7 +1778,7 @@ function QueuedMessageTimelineRow({
{text.length > 0 ? ( -
{text}
+ ) : null} {attachmentCount > 0 || contextCount > 0 ? (
0 && "mt-1.5")}> @@ -2570,7 +2570,12 @@ function BackgroundWorktreeSetupChip({ snapshot }: { snapshot: WorktreeSetupSnap {scriptName} - + ReactNode; + renderContextReference?: (reference: ChatMarkdownContextReference) => ReactNode; skills: ReadonlyArray>; markdownCwd: string | undefined; }) { diff --git a/apps/web/src/components/composerContextPresentation.tsx b/apps/web/src/components/composerContextPresentation.tsx index 6a945be5d679..9e5c775161cb 100644 --- a/apps/web/src/components/composerContextPresentation.tsx +++ b/apps/web/src/components/composerContextPresentation.tsx @@ -5,6 +5,7 @@ import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments" import { videoMimeType } from "@t3tools/shared/video"; import { MessageCircleIcon, MousePointerClickIcon } from "lucide-react"; import { createContext, type MouseEvent, type ReactElement, type ReactNode, use } from "react"; +import type { EnvironmentId } from "@t3tools/contracts"; import type { ComposerFileAttachment, ComposerImageAttachment } from "~/composerDraftStore"; import { composerFileNeedsReattach } from "~/composerDraftStore"; @@ -66,6 +67,7 @@ export type ComposerDraftContextRecord = /** What a chip can do beyond showing itself; the composer supplies the handlers. */ export interface ComposerContextActions { + environmentId: EnvironmentId | null; expandImage: (imageId: string) => void; expandVideo: (fileId: string) => void; openFile: (fileId: string) => void; @@ -74,6 +76,7 @@ export interface ComposerContextActions { } export const ComposerContextActionsContext = createContext({ + environmentId: null, expandImage: () => {}, expandVideo: () => {}, openFile: () => {}, @@ -247,6 +250,7 @@ function PullRequestContextChip(props: { record: ReviewCommentContext; toneClass return ( , url: string) => void; }) { + const previewTarget = usePullRequestPreviewTarget(props.environmentId, props.metadata.url); + const button = ( + + ); + if (previewTarget !== null) { + return ( + } + /> + ); + } return ( - props.onOpen(event, props.metadata.url)} - > - - {props.label} - - } - /> + diff --git a/apps/web/src/components/device/DeviceLoadingView.tsx b/apps/web/src/components/device/DeviceLoadingView.tsx index 70e848c3ce7e..332d3994cca1 100644 --- a/apps/web/src/components/device/DeviceLoadingView.tsx +++ b/apps/web/src/components/device/DeviceLoadingView.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from "react"; import { Smartphone } from "lucide-react"; import { Spinner } from "~/components/ui/spinner"; @@ -8,6 +9,7 @@ export function DeviceLoadingView(props: { readonly stage: "opening" | "stream"; readonly message: string; readonly error?: boolean; + readonly children?: ReactNode; }) { return (
: null} {props.message}
+ {props.children} {!props.error ? (
void>(), + refresh() { + accessStore.value = { ...accessStore.value }; + for (const listener of accessStore.listeners) listener(); + }, + subscribe(listener: () => void) { + accessStore.listeners.add(listener); + return () => accessStore.listeners.delete(listener); + }, +}; vi.mock("~/state/device", () => ({ - useDeviceHubAccess: () => access, - refreshDeviceHubAccess: vi.fn(), -})); -const access = { httpBase: "http://test", wsBase: "ws://test", query: {}, credentials: true }; -vi.mock("@t3tools/client-runtime/device/stream", () => ({ - createDeviceStreamClient: ( - _target: unknown, - _canvas: unknown, - events: { onMjpegFallback: (url: string) => void }, - ) => ({ - start: () => events.onMjpegFallback("http://test/stream.mjpeg"), - stop: vi.fn(), - }), + useDeviceHubAccess: () => useSyncExternalStore(accessStore.subscribe, () => accessStore.value), + refreshDeviceHubAccess: () => accessStore.refresh(), })); import { DeviceStreamView } from "./DeviceStreamView"; + +class Image extends EventTarget { + src = ""; + naturalWidth = 0; + naturalHeight = 0; + removeAttribute(name: string) { + if (name === "src") this.src = ""; + } +} let renderer: ReactTestRenderer | undefined; +let primes = 0; +beforeEach(() => { + primes = 0; +}); afterEach(async () => { await act(async () => renderer?.unmount()); + renderer = undefined; + vi.useRealTimers(); vi.unstubAllGlobals(); }); -it("removes MJPEG requests while hidden and reconnects when shown", async () => { +async function setup() { + vi.useFakeTimers(); vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("fetch", () => { + primes++; + return Promise.resolve(new Response("prime")); + }); + vi.stubGlobal( + "WebSocket", + class { + static OPEN = 1; + readyState = 1; + send() {} + close() {} + }, + ); vi.stubGlobal( "ResizeObserver", class { @@ -34,6 +65,7 @@ it("removes MJPEG requests while hidden and reconnects when shown", async () => disconnect() {} }, ); + const images: Image[] = []; const view = (visible: boolean) => (