diff --git a/apps/desktop/scripts/ensure-electron-runtime.mjs b/apps/desktop/scripts/ensure-electron-runtime.mjs index 933a763dfa35..741a2d9ee85d 100644 --- a/apps/desktop/scripts/ensure-electron-runtime.mjs +++ b/apps/desktop/scripts/ensure-electron-runtime.mjs @@ -209,6 +209,10 @@ export function ensureElectronRuntime() { return electronPath; } +export function setup() { + ensureElectronRuntime(); +} + // `file://${argv[1]}` never matches on Windows (drive letters need `file:///C:/`). if (process.argv[1] && NodeURL.pathToFileURL(process.argv[1]).href === import.meta.url) { const electronPath = ensureElectronRuntime(); diff --git a/apps/desktop/src/electron/ElectronWindow.test.ts b/apps/desktop/src/electron/ElectronWindow.test.ts index 9eb41ed4e240..eac944722e73 100644 --- a/apps/desktop/src/electron/ElectronWindow.test.ts +++ b/apps/desktop/src/electron/ElectronWindow.test.ts @@ -148,6 +148,34 @@ describe("ElectronWindow", () => { }).pipe(Effect.provide(layerTestFor("win32"))), ); + it.effect("reports which windows were created with a title bar overlay", () => + Effect.gen(function* () { + const framed = { isDestroyed: () => false }; + const overlay = { isDestroyed: () => false }; + browserWindowMock + .mockImplementationOnce(function FramedWindowMock() { + return framed; + }) + .mockImplementationOnce(function OverlayWindowMock() { + return overlay; + }); + const electronWindow = yield* ElectronWindow.ElectronWindow; + yield* electronWindow.create({}); + yield* electronWindow.create({ + titleBarStyle: "hidden", + titleBarOverlay: { color: "#01000000", height: 40 }, + }); + getAllWindowsMock.mockReturnValue([framed, overlay, { isDestroyed: () => false }]); + + const seen: Array = []; + yield* electronWindow.syncAllAppearance((_window, { titleBarOverlay }) => + Effect.sync(() => seen.push(titleBarOverlay)), + ); + + assert.deepEqual(seen, [false, true, false]); + }).pipe(Effect.provide(layerTestFor("win32"))), + ); + it.effect("preserves destroy failures and continues with later windows", () => Effect.gen(function* () { const cause = new Error("window destroy failed"); diff --git a/apps/desktop/src/electron/ElectronWindow.ts b/apps/desktop/src/electron/ElectronWindow.ts index 15a117a52b05..b5f89969e66c 100644 --- a/apps/desktop/src/electron/ElectronWindow.ts +++ b/apps/desktop/src/electron/ElectronWindow.ts @@ -125,7 +125,10 @@ export class ElectronWindow extends Context.Service< readonly sendAll: (channel: string, ...args: readonly unknown[]) => Effect.Effect; readonly destroyAll: Effect.Effect; readonly syncAllAppearance: ( - sync: (window: Electron.BrowserWindow) => Effect.Effect, + sync: ( + window: Electron.BrowserWindow, + appearance: { readonly titleBarOverlay: boolean }, + ) => Effect.Effect, ) => Effect.Effect; } >()("@t3tools/desktop/electron/ElectronWindow") {} @@ -145,6 +148,7 @@ export const make = Effect.gen(function* () { yield* Effect.addFinalizer(() => Effect.sync(() => windowsForegroundFocus?.close())); const mainWindowRef = yield* Ref.make>(Option.none()); const auxiliaryWindows = new WeakSet(); + const titleBarOverlayWindows = new WeakSet(); const listWindows = Effect.try({ try: () => Electron.BrowserWindow.getAllWindows(), @@ -241,6 +245,9 @@ export const make = Effect.gen(function* () { try: () => { const window = new Electron.BrowserWindow(options); if (behavior?.auxiliary === true) auxiliaryWindows.add(window); + if (options.titleBarOverlay !== undefined && options.titleBarOverlay !== false) { + titleBarOverlayWindows.add(window); + } return window; }, catch: (cause) => new ElectronWindowCreateError({ options: diagnosticOptions, cause }), @@ -383,14 +390,17 @@ export const make = Effect.gen(function* () { } }), syncAllAppearance: Effect.fn("desktop.electron.window.syncAllAppearance")(function* ( - sync: (window: Electron.BrowserWindow) => Effect.Effect, + sync: ( + window: Electron.BrowserWindow, + appearance: { readonly titleBarOverlay: boolean }, + ) => Effect.Effect, ) { const windows = yield* listWindows; for (const window of windows) { if (yield* isWindowDestroyed(window)) { continue; } - yield* sync(window); + yield* sync(window, { titleBarOverlay: titleBarOverlayWindows.has(window) }); } }), }); diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 802cd315b8e6..b6387a06c22d 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -349,6 +349,47 @@ const makeTestPreviewWebContents = ( } as unknown as TestPreviewWebContents; }; +const makeStillImage = (pixels = "fresh-png") => ({ + toPNG: () => Buffer.from(pixels), + toJPEG: () => Buffer.from(pixels), + getSize: () => ({ width: 100, height: 80 }), +}); + +const makeTestStillImageWebContents = (...args: Parameters) => { + const wc = makeTestPreviewWebContents(...args); + Object.assign(wc, { isDevToolsOpened: () => false }); + holdStillImagePaint(wc, Promise.resolve()); + return wc; +}; + +const holdStillImagePaint = ( + wc: TestPreviewWebContents, + paint: Promise | Array>, +) => { + vi.mocked(wc.debugger.sendCommand).mockImplementation(async (method) => { + if (method === "Page.getFrameTree") return { frameTree: { frame: { id: "main" } } }; + if (method === "Page.createIsolatedWorld") return { executionContextId: 7 }; + if (method === "Runtime.evaluate") { + await (Array.isArray(paint) ? paint.shift() : paint); + return { result: { value: true } }; + } + return undefined; + }); +}; + +const setupStill = (manager: PreviewManager.PreviewManager["Service"], paint = Promise.resolve()) => + Effect.gen(function* () { + const image = makeStillImage(); + const capturePage = vi.fn<() => Promise>().mockResolvedValue(image); + const wc = makeTestStillImageWebContents(capturePage); + holdStillImagePaint(wc, paint); + fromId.mockReturnValue(wc); + yield* manager.createTab("tab_still"); + yield* manager.registerWebview("tab_still", wc.id); + yield* manager.setColorScheme("tab_still", "system"); + return { image, capturePage, wc, capture: manager.captureScreenshot("tab_still") }; + }); + const setupRecordingRaceTabs = (manager: PreviewManager.PreviewManager["Service"]) => Effect.gen(function* () { const capturePage = vi.fn(async () => ({ @@ -1824,6 +1865,30 @@ describe("PreviewManager", () => { ), ); + effectIt.effect("captures final pixels after warmup fails and paint readiness settles", () => + withManager((manager) => + Effect.gen(function* () { + const paint = Promise.withResolvers(); + const { capturePage, wc, capture } = yield* setupStill(manager, paint.promise); + capturePage + .mockRejectedValueOnce(new Error("UnknownVizError")) + .mockRejectedValueOnce(new Error("UnknownVizError")) + .mockRejectedValueOnce(new Error("UnknownVizError")) + .mockResolvedValue(makeStillImage("final-png")); + const fiber = yield* capture.pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(300); + expect(capturePage).toHaveBeenCalledTimes(3); + expect(writeFile).not.toHaveBeenCalled(); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false]]); + paint.resolve(); + const artifact = yield* Fiber.join(fiber); + expect(capturePage).toHaveBeenCalledTimes(4); + expect(writeFile.mock.calls).toEqual([[artifact.path, Buffer.from("final-png")]]); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + effectIt.effect("preserves both screenshots of the same site in the same millisecond", () => withManager((manager) => Effect.gen(function* () { @@ -1837,6 +1902,8 @@ describe("PreviewManager", () => { const capturePage = vi .fn<() => Promise>>() .mockResolvedValueOnce(pngImage(firstPng)) + .mockResolvedValueOnce(pngImage(firstPng)) + .mockResolvedValueOnce(pngImage(secondPng)) .mockResolvedValueOnce(pngImage(secondPng)); fromId.mockReturnValue(makeTestPreviewWebContents(capturePage)); yield* manager.createTab("tab_1"); @@ -1857,6 +1924,115 @@ describe("PreviewManager", () => { ), ); + effectIt.effect( + "waits for paint before capturing and restores rendering after overlapping requests", + () => + withManager((manager) => + Effect.gen(function* () { + const paints = [Promise.withResolvers(), Promise.withResolvers()]; + const pendingPaints = paints.map((paint) => paint.promise); + const { image, capturePage, wc, capture } = yield* setupStill(manager); + holdStillImagePaint(wc, pendingPaints); + const first = Promise.withResolvers(); + const second = Promise.withResolvers(); + capturePage + .mockResolvedValueOnce(makeStillImage("discarded-warmup")) + .mockResolvedValueOnce(makeStillImage("discarded-warmup")) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise); + const hostThrottling = vi.fn(); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: { setBackgroundThrottling: hostThrottling }, + } as never); + const one = yield* capture.pipe(Effect.forkChild({ startImmediately: true })); + const two = yield* capture.pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(0); + expect(capturePage).toHaveBeenCalledTimes(2); + expect(pendingPaints).toHaveLength(0); + expect(writeFile).not.toHaveBeenCalled(); + expect(hostThrottling.mock.calls).toEqual([[false]]); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false]]); + paints.forEach((paint) => paint.resolve()); + yield* TestClock.adjust(0); + expect(capturePage).toHaveBeenCalledTimes(4); + first.resolve(image); + yield* Fiber.join(one); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false]]); + second.resolve(image); + yield* Fiber.join(two); + const saved = writeFile.mock.calls.map(([, bytes]) => bytes); + expect(saved).toEqual([image.toPNG(), image.toPNG()]); + expect(hostThrottling.mock.calls).toEqual([[false], [true]]); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + + effectIt.effect("keeps picture-in-picture rendering after a still image completes", () => + withManager((manager) => + Effect.gen(function* () { + const { image, capturePage, wc, capture } = yield* setupStill(manager); + const pending = Promise.withResolvers(); + capturePage + .mockResolvedValueOnce(image) + .mockResolvedValueOnce(makeStillImage("discarded-warmup")) + .mockReturnValueOnce(pending.promise); + const { pictureInPictureWindow } = makeTestPictureInPictureWindow(); + browserWindowConstructor.mockImplementation(function () { + return pictureInPictureWindow; + }); + yield* manager.openPictureInPicture("tab_still"); + const still = yield* capture.pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(0); + expect(capturePage).toHaveBeenCalledTimes(3); + expect(writeFile).not.toHaveBeenCalled(); + pending.resolve(image); + yield* Fiber.join(still); + expect(writeFile.mock.calls.map((call) => call[1])).toEqual([Buffer.from("fresh-png")]); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false]]); + yield* manager.closePictureInPicture("tab_still"); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + + effectIt.effect("keeps a pending still image rendering when recording ends", () => + withManager((manager) => + Effect.gen(function* () { + const { image, capturePage, wc, capture } = yield* setupStill(manager); + const pending = Promise.withResolvers(); + capturePage.mockResolvedValueOnce(image).mockReturnValue(pending.promise); + yield* manager.startRecording("tab_still"); + const fiber = yield* capture.pipe(Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust(0); + yield* manager.stopRecording("tab_still"); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false]]); + pending.resolve(image); + yield* Fiber.join(fiber); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + + effectIt.effect("saves available pixels when paint readiness never settles", () => + withManager((manager) => + Effect.gen(function* () { + const paint = Promise.withResolvers(); + const { capturePage, wc, capture } = yield* setupStill(manager, paint.promise); + const first = yield* Effect.exit(capture).pipe( + Effect.forkChild({ startImmediately: true }), + ); + yield* TestClock.adjust(1_001); + expect(Exit.isSuccess(yield* Fiber.join(first))).toBe(true); + expect(capturePage).toHaveBeenCalledTimes(2); + expect(writeFile.mock.calls.map((call) => call[1])).toEqual([Buffer.from("fresh-png")]); + expect(wc.setBackgroundThrottling.mock.calls).toEqual([[false], [true]]); + }), + ), + ); + effectIt.effect("keeps every recorded guest unthrottled until its frame capture stops", () => withManager((manager) => Effect.gen(function* () { diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index eba18ab631dd..2c199e358fa5 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -387,7 +387,8 @@ interface ManagedListeners { readonly webContents: Electron.WebContents; } -type FrameCaptureConsumer = "picture-in-picture" | "recording"; +// Each still-image request owns a separate, short-lived rendering lease. +type FrameCaptureConsumer = "picture-in-picture" | "recording" | symbol; interface FrameCaptureSession { readonly recordingInputOptions?: RecordingInputOptions; @@ -2651,7 +2652,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const [createdAt, millis, image] = yield* Effect.all([ currentIso, currentMillis, - capturePageWithRetry( + captureStillImage( { operation: "captureScreenshot.capturePage", tabId, @@ -2699,6 +2700,100 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }; }); + const captureStillImage = Effect.fn("PreviewManager.captureStillImage")(function* ( + errorContext: PreviewOperationContext, + tabId: string, + wc: Electron.WebContents, + ) { + const consumer = Symbol("still-image"); + return yield* Effect.acquireUseRelease( + startFrameCapture(tabId, consumer), + () => + Effect.gen(function* () { + // A DOM mutation can precede its compositor frame even in a shown + // window. Keep the original guest rendering through the next paint; + // a hidden guest otherwise returns old pixels or never completes. + const probe = Effect.fn("PreviewManager.probeStillImagePaint")(function* ( + send: ( + method: string, + params?: Record, + ) => Effect.Effect, + ) { + // CDP executes against the current document without Electron's + // did-stop-loading gate. Isolate scheduling globals from page code. + const tree = (yield* send("Page.getFrameTree")) as + | { frameTree?: { frame?: { id?: string } } } + | undefined; + const frameId = tree?.frameTree?.frame?.id; + if (typeof frameId !== "string") + return yield* new PreviewOperationError({ + ...errorContext, + cause: new Error("The preview paint frame is unavailable."), + }); + const world = (yield* send("Page.createIsolatedWorld", { + frameId, + worldName: "t3-preview-still-image", + })) as { executionContextId?: number } | undefined; + if (typeof world?.executionContextId !== "number") + return yield* new PreviewOperationError({ + ...errorContext, + cause: new Error("The preview paint context is unavailable."), + }); + const response = (yield* send("Runtime.evaluate", { + // Settle and release scheduled work when only timers keep running. + // The outer deadline still bounds a frozen renderer. + expression: `new Promise(resolve => { let frame; const timer = setTimeout(() => { cancelAnimationFrame(frame); resolve(false); }, ${CAPTURE_PAGE_ATTEMPT_TIMEOUT_MS}); frame = requestAnimationFrame(() => { frame = requestAnimationFrame(() => { clearTimeout(timer); resolve(true); }); }); })`, + contextId: world.executionContextId, + awaitPromise: true, + returnByValue: true, + })) as { result?: { value?: unknown }; exceptionDetails?: unknown } | undefined; + if (response?.exceptionDetails || response?.result?.value !== true) + return yield* new PreviewOperationError({ + ...errorContext, + cause: new Error("The preview paint boundary was not confirmed."), + }); + }); + const paintReady = Effect.gen(function* () { + // Only use an existing owned session; never attach or displace + // another debugger to obtain a screenshot. + const control = (yield* SynchronizedRef.get(controlSessionsRef)).get(wc.id); + if (!control) + return yield* new PreviewOperationError({ + ...errorContext, + cause: new Error("The preview paint session is unavailable."), + }); + return yield* probe((method, params) => + attemptPromise(errorContext, () => control.debugger.sendCommand(method, params)), + ); + }).pipe( + Effect.timeout(CAPTURE_PAGE_ATTEMPT_TIMEOUT_MS), + Effect.catch((error) => + Effect.logWarning( + "Preview paint readiness was not confirmed; capturing available pixels with mutation freshness unconfirmed.", + { tabId, webContentsId: wc.id, error }, + ), + ), + ); + // Minimized guests can suspend frames even with throttling disabled. + // A native capture can resume them; discard its pixels and read again + // after the bounded paint probe. + const warmup = capturePageWithRetry(errorContext, tabId, wc).pipe( + Effect.catchTags({ + PreviewOperationError: (error) => + Effect.logWarning("Preview warmup capture failed; continuing with final capture.", { + tabId, + webContentsId: wc.id, + error, + }), + }), + ); + yield* Effect.all([paintReady, warmup], { concurrency: 2, discard: true }); + return yield* capturePageWithRetry(errorContext, tabId, wc); + }), + () => stopFrameCapture(tabId, consumer), + ); + }); + const capturePreviewFrame = Effect.fn("PreviewManager.capturePreviewFrame")(function* ( tabId: string, ) { diff --git a/apps/desktop/src/window/DesktopWindow.test.ts b/apps/desktop/src/window/DesktopWindow.test.ts index 497b315ef522..0ddcc7946d32 100644 --- a/apps/desktop/src/window/DesktopWindow.test.ts +++ b/apps/desktop/src/window/DesktopWindow.test.ts @@ -207,17 +207,20 @@ const layerElectronTheme = Layer.succeed(ElectronTheme.ElectronTheme, { onUpdated: () => Effect.void, } satisfies ElectronTheme.ElectronTheme["Service"]); -const layerDesktopEnvironment = DesktopEnvironment.layer(environmentInput).pipe( - Layer.provide( - Layer.mergeAll( - NodeServices.layer, - DesktopConfig.layerTest({ - T3CODE_PORT: "3773", - VITE_DEV_SERVER_URL: "http://127.0.0.1:5733", - }), +const layerDesktopEnvironmentFor = (platform: NodeJS.Platform) => + DesktopEnvironment.layer({ ...environmentInput, platform }).pipe( + Layer.provide( + Layer.mergeAll( + NodeServices.layer, + DesktopConfig.layerTest({ + T3CODE_PORT: "3773", + VITE_DEV_SERVER_URL: "http://127.0.0.1:5733", + }), + ), ), - ), -); + ); + +const layerDesktopEnvironment = layerDesktopEnvironmentFor(environmentInput.platform); const desktopWindowBoundsEquivalence = Schema.toEquivalence( DesktopAppSettings.DesktopWindowBoundsSchema, @@ -243,6 +246,11 @@ function layerTest(input: { readonly previewZoomReapplies?: number[]; readonly onReveal?: (window: Electron.BrowserWindow) => void; readonly focusedWindow?: Electron.BrowserWindow; + readonly platform?: NodeJS.Platform; + readonly appearanceWindows?: ReadonlyArray<{ + readonly window: Electron.BrowserWindow; + readonly titleBarOverlay: boolean; + }>; readonly primaryPasteEnabled?: boolean; readonly previewPasskeyBridgeEnabled?: boolean; }) { @@ -302,7 +310,14 @@ function layerTest(input: { reveal: (window) => Effect.sync(() => input.onReveal?.(window)), sendAll: () => Effect.void, destroyAll: input.destroyAll ?? Effect.void, - syncAllAppearance: (sync) => sync(input.window), + syncAllAppearance: (sync) => + input.appearanceWindows === undefined + ? sync(input.window, { titleBarOverlay: true }) + : Effect.forEach( + input.appearanceWindows, + ({ window, titleBarOverlay }) => sync(window, { titleBarOverlay }), + { discard: true }, + ), } satisfies ElectronWindow.ElectronWindow["Service"]); return DesktopWindow.layer.pipe( @@ -314,7 +329,9 @@ function layerTest(input: { recordMetrics: () => Effect.void, shutdown: Effect.void, }), - layerDesktopEnvironment, + input.platform === undefined + ? layerDesktopEnvironment + : layerDesktopEnvironmentFor(input.platform), layerDesktopAppSettings, layerDesktopClientSettings, layerDesktopServerExposure, @@ -423,7 +440,8 @@ const makeSplashScenario = (createOutcomes: readonly (Electron.BrowserWindow | n reveal: (window) => Ref.update(revealedWindows, (windows) => [...windows, window]), sendAll: () => Effect.void, destroyAll: Effect.void, - syncAllAppearance: (sync) => (fallbackWindow ? sync(fallbackWindow) : Effect.void), + syncAllAppearance: (sync) => + fallbackWindow ? sync(fallbackWindow, { titleBarOverlay: true }) : Effect.void, } satisfies ElectronWindow.ElectronWindow["Service"]; const layer = DesktopWindow.layer.pipe( @@ -1170,6 +1188,36 @@ describe("DesktopWindow", () => { }), ); + it.effect("updates title bar overlays only on windows created with one", () => + Effect.gen(function* () { + const framed = makeFakeBrowserWindow(); + vi.mocked(framed.window.setTitleBarOverlay).mockImplementation(() => { + throw new TypeError("Titlebar overlay is not enabled"); + }); + const overlay = makeFakeBrowserWindow(); + const layer = layerTest({ + window: overlay.window, + createCount: yield* Ref.make(0), + mainWindow: yield* Ref.make>(Option.none()), + platform: "win32", + appearanceWindows: [ + { window: framed.window, titleBarOverlay: false }, + { window: overlay.window, titleBarOverlay: true }, + ], + }); + + yield* Effect.gen(function* () { + const desktopWindow = yield* DesktopWindow.DesktopWindow; + yield* desktopWindow.syncAppearance; + }).pipe(Effect.provide(layer)); + + assert.deepEqual(vi.mocked(framed.window.setBackgroundColor).mock.calls, [["#ffffff"]]); + assert.lengthOf(vi.mocked(framed.window.setTitleBarOverlay).mock.calls, 0); + assert.deepEqual(vi.mocked(overlay.window.setBackgroundColor).mock.calls, [["#ffffff"]]); + assert.lengthOf(vi.mocked(overlay.window.setTitleBarOverlay).mock.calls, 1); + }), + ); + it.effect.each([ { window: "normal", write: "flush" }, { window: "normal", write: "debounce" }, diff --git a/apps/desktop/src/window/DesktopWindow.ts b/apps/desktop/src/window/DesktopWindow.ts index 43404e2d6cfb..93b35eb9b1a3 100644 --- a/apps/desktop/src/window/DesktopWindow.ts +++ b/apps/desktop/src/window/DesktopWindow.ts @@ -343,6 +343,7 @@ function getWindowTitleBarOptions( function syncWindowAppearance( window: Electron.BrowserWindow, + hasTitleBarOverlay: boolean, shouldUseDarkColors: boolean, platform: NodeJS.Platform, ): Effect.Effect { @@ -353,7 +354,7 @@ function syncWindowAppearance( window.setBackgroundColor(getInitialWindowBackgroundColor(shouldUseDarkColors)); const { titleBarOverlay } = getWindowTitleBarOptions(shouldUseDarkColors, platform); - if (typeof titleBarOverlay === "object") { + if (hasTitleBarOverlay && typeof titleBarOverlay === "object") { window.setTitleBarOverlay(titleBarOverlay); } }); @@ -1202,8 +1203,8 @@ export const make = Effect.gen(function* () { }), syncAppearance: Effect.gen(function* () { const shouldUseDarkColors = yield* electronTheme.shouldUseDarkColors; - yield* electronWindow.syncAllAppearance((window) => - syncWindowAppearance(window, shouldUseDarkColors, environment.platform), + yield* electronWindow.syncAllAppearance((window, { titleBarOverlay }) => + syncWindowAppearance(window, titleBarOverlay, shouldUseDarkColors, environment.platform), ); }).pipe(Effect.withSpan("desktop.window.syncAppearance")), }); diff --git a/apps/desktop/vite.config.ts b/apps/desktop/vite.config.ts index c0528c461a38..968a7a8825fb 100644 --- a/apps/desktop/vite.config.ts +++ b/apps/desktop/vite.config.ts @@ -166,6 +166,9 @@ export default defineConfig({ // The Windows lane runs workspace suites concurrently; filesystem-heavy // desktop integration tests can exceed Vitest's 5 second default there. testTimeout: 15_000, + // `require("electron")` downloads a missing binary, so concurrent workers race to + // extract it into the same dist/. Install it once before any worker starts. + globalSetup: ["./scripts/ensure-electron-runtime.mjs"], projects: [ { extends: true, diff --git a/apps/mobile/src/features/layout/createWorkspaceStackNavigator.ios.tsx b/apps/mobile/src/features/layout/createWorkspaceStackNavigator.ios.tsx index adef48e424e9..cad7dbcdd572 100644 --- a/apps/mobile/src/features/layout/createWorkspaceStackNavigator.ios.tsx +++ b/apps/mobile/src/features/layout/createWorkspaceStackNavigator.ios.tsx @@ -354,6 +354,7 @@ function WorkspaceColumns( const primary = props.descriptors[props.primary.key]; if (!primary) return null; + const primaryColumnWidth = (layout.listPaneWidth ?? 320) + (layout.listPaneGap ?? 0) / 2; return ( + + + ); } diff --git a/apps/mobile/src/lib/reserved-regions.test.ts b/apps/mobile/src/lib/reserved-regions.test.ts index c5faebc31653..60dd501ae37a 100644 --- a/apps/mobile/src/lib/reserved-regions.test.ts +++ b/apps/mobile/src/lib/reserved-regions.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vite-plus/test"; -import { deriveBottomControlInsets, type NativeLayoutMetrics } from "./reserved-regions"; +import { + deriveBottomControlInsets, + deriveContentSideInsets, + type NativeLayoutMetrics, +} from "./reserved-regions"; import { deriveLayout, deriveWorkspacePaneLayout } from "./layout"; const innerDisplay: NativeLayoutMetrics = { @@ -21,26 +25,27 @@ describe("bottom controls around reserved regions", () => { expect(innerDisplay.width - insets.left - insets.right).toBeGreaterThan(320); }); - it("does not reserve a hinge twice when a native column starts at the division", () => { + it("starts the editor after a fold that covers a native column's leading edge", () => { expect( deriveBottomControlInsets({ ...innerDisplay, - width: 412, + width: 411.33, + height: 669, safeArea: { top: 82, bottom: 34, left: 0, right: 0 }, - reservedRegions: [{ kind: "division", x: 0, y: 0, width: 40, height: 560 }], + reservedRegions: [{ kind: "division", x: 0, y: 0, width: 39.83, height: 669 }], }), - ).toEqual({ left: 0, right: 0, bottom: 0 }); + ).toEqual({ left: 39.83, right: 0, bottom: 0 }); }); - it("keeps a native column's trailing division at its existing edge", () => { + it("does not reserve a hinge twice when the safe area already covers it", () => { expect( deriveBottomControlInsets({ ...innerDisplay, - width: 456, - safeArea: { top: 82, bottom: 34, left: 0, right: 0 }, - reservedRegions: [{ kind: "division", x: 455.75, y: 0, width: 0.25, height: 560 }], + width: 412, + safeArea: { top: 82, bottom: 34, left: 40, right: 0 }, + reservedRegions: [{ kind: "division", x: 0, y: 0, width: 40, height: 560 }], }), - ).toEqual({ left: 0, right: 0, bottom: 0 }); + ).toEqual({ left: 40, right: 0, bottom: 0 }); }); it("keeps controls below a horizontal fold in a hands-free pose", () => { @@ -65,6 +70,59 @@ describe("bottom controls around reserved regions", () => { }); }); +describe("content side insets around reserved regions", () => { + it("keeps transcript text on the wider side of a book fold", () => { + expect( + deriveContentSideInsets({ + ...innerDisplay, + reservedRegions: [{ kind: "division", x: 456, y: 0, width: 48, height: 560 }], + }), + ).toEqual({ left: 8, right: 504 }); + expect( + deriveContentSideInsets({ + ...innerDisplay, + width: 800, + reservedRegions: [{ kind: "division", x: 240, y: 0, width: 40, height: 560 }], + }), + ).toEqual({ left: 280, right: 72 }); + }); + + it("starts transcript text after a fold that covers a native column's leading edge", () => { + expect( + deriveContentSideInsets({ + ...innerDisplay, + width: 411.33, + height: 669, + safeArea: { top: 82, bottom: 34, left: 0, right: 0 }, + reservedRegions: [{ kind: "division", x: 0, y: 0, width: 39.83, height: 669 }], + }), + ).toEqual({ left: 39.83, right: 0 }); + }); + + it("ends transcript text before a fold at a native column's trailing edge", () => { + expect( + deriveContentSideInsets({ + ...innerDisplay, + width: 456, + safeArea: { top: 82, bottom: 34, left: 0, right: 0 }, + reservedRegions: [{ kind: "division", x: 416, y: 0, width: 40, height: 560 }], + }), + ).toEqual({ left: 0, right: 40 }); + }); + + it("ignores a horizontal fold and bottom cameras", () => { + expect( + deriveContentSideInsets({ + ...innerDisplay, + reservedRegions: [ + { kind: "division", x: 0, y: 240, width: 960, height: 40 }, + { kind: "occlusion", x: 8, y: 510, width: 60, height: 50 }, + ], + }), + ).toEqual({ left: 8, right: 72 }); + }); +}); + describe("Duo workspace sizing", () => { it("shows both columns on the unfolded inner display in portrait", () => { // Reported by the iOS 27.1 Duo simulator: no side bar or fold, only the hinge. diff --git a/apps/mobile/src/lib/reserved-regions.ts b/apps/mobile/src/lib/reserved-regions.ts index bb8ff981a6de..a3d8b6c14065 100644 --- a/apps/mobile/src/lib/reserved-regions.ts +++ b/apps/mobile/src/lib/reserved-regions.ts @@ -23,6 +23,30 @@ export interface NativeLayoutMetrics { readonly bottomBarInset?: number; } +function isFullHeightDivision(region: ReservedRegion, metrics: NativeLayoutMetrics) { + return region.kind === "division" && region.height >= metrics.height / 2; +} + +function sideOfDivision(left: number, right: number, x: number, endX: number) { + if (x <= left) return { left: Math.min(right, endX), right }; + if (endX >= right) return { left, right: Math.max(left, x) }; + return right - endX >= x - left ? { left: endX, right } : { left, right: x }; +} + +export function deriveContentSideInsets(metrics: NativeLayoutMetrics | null) { + if (!metrics) return { left: 0, right: 0 }; + let left = metrics.safeArea.left; + let right = metrics.width - metrics.safeArea.right; + for (const region of metrics.reservedRegions) { + if (!isFullHeightDivision(region, metrics)) continue; + const x = Math.max(left, region.x); + const endX = Math.min(right, region.x + region.width); + if (endX <= x) continue; + ({ left, right } = sideOfDivision(left, right, x, endX)); + } + return { left, right: metrics.width - right }; +} + export function deriveBottomControlInsets(metrics: NativeLayoutMetrics | null) { if (!metrics) return { left: 0, right: 0, bottom: 0 }; let left = metrics.safeArea.left; @@ -34,12 +58,8 @@ export function deriveBottomControlInsets(metrics: NativeLayoutMetrics | null) { const y = Math.max(0, region.y); const endY = Math.min(bottom, region.y + region.height); if (endX <= x || endY <= y) continue; - if (region.kind === "division" && region.height >= metrics.height / 2) { - // A native column ending at a division is already split there. Only an - // interior division needs to move these controls into one side of the view. - if (x <= left || endX >= right) continue; - if (right - endX >= x - left) left = endX; - else right = x; + if (isFullHeightDivision(region, metrics)) { + ({ left, right } = sideOfDivision(left, right, x, endX)); } else if (endY >= bottom - metrics.safeArea.bottom) { if (region.width >= right - left) bottom = y; else if (right - endX >= x - left) left = endX; diff --git a/apps/server/src/claudeModelOptions.test.ts b/apps/server/src/claudeModelOptions.test.ts index b69b20db7ec4..a7513a593183 100644 --- a/apps/server/src/claudeModelOptions.test.ts +++ b/apps/server/src/claudeModelOptions.test.ts @@ -105,14 +105,33 @@ describe("compileClaudeModelSelection", () => { ).toMatchObject({ effort: "high", promptEffort: "ultrathink" }); }); - it("keeps explicit effort for models missing from the local catalog", () => { + it("ignores thinking for catalog models without a thinking option", () => { expect( - compileClaudeModelSelection( - selection("claude-pool-model-not-in-catalog", [{ id: "effort", value: "max" }]), - ).effort, - ).toBe("max"); - expect( - compileClaudeModelSelection(selection("claude-pool-model-not-in-catalog", [])).effort, - ).toBeUndefined(); + compileClaudeModelSelection(selection("claude-opus-5-5", [{ id: "thinking", value: false }])) + .settings, + ).not.toHaveProperty("alwaysThinkingEnabled"); }); + + it.each(["gateway-route", "gateway-route[1m]"])( + "keeps explicit effort and thinking for %s outside the local catalog", + (model) => { + for (const thinking of [true, false]) { + expect( + compileClaudeModelSelection( + selection(model, [ + { id: "effort", value: "max" }, + { id: "thinking", value: thinking }, + ]), + ), + ).toMatchObject({ + apiModelId: model, + effort: "max", + settings: { alwaysThinkingEnabled: thinking }, + }); + } + const unconfigured = compileClaudeModelSelection(selection(model, [])); + expect(unconfigured.effort).toBeUndefined(); + expect(unconfigured.settings).toEqual({}); + }, + ); }); diff --git a/apps/server/src/claudeModelOptions.ts b/apps/server/src/claudeModelOptions.ts index 12530db50ec5..c3376ef7ed8d 100644 --- a/apps/server/src/claudeModelOptions.ts +++ b/apps/server/src/claudeModelOptions.ts @@ -12,6 +12,7 @@ import { normalizeClaudeCatalogEffort, resolveClaudeCatalogApiModelId, resolveClaudeCatalogEffort, + resolveClaudeCatalogModel, resolveClaudeModelSlug, type ClaudeModelCatalog, } from "./provider/ClaudeModelCatalog.ts"; @@ -45,9 +46,11 @@ export function compileClaudeModelSelection( const fastMode = supportsBoolean("fastMode") ? (getModelSelectionBooleanOptionValue(selection, "fastMode") ?? false) : undefined; - const thinking = supportsBoolean("thinking") - ? getModelSelectionBooleanOptionValue(selection, "thinking") - : undefined; + const inCatalog = resolveClaudeCatalogModel(catalog, selection.model) !== undefined; + const thinking = + !inCatalog || supportsBoolean("thinking") + ? getModelSelectionBooleanOptionValue(selection, "thinking") + : undefined; const ultracode = descriptors.find((descriptor) => descriptor.id === "ultracode")?.currentValue; const settings = { ...(typeof thinking === "boolean" ? { alwaysThinkingEnabled: thinking } : {}), diff --git a/apps/server/src/cli/sshHelper.ts b/apps/server/src/cli/sshHelper.ts index 09a729ef4489..e98e5e0537af 100644 --- a/apps/server/src/cli/sshHelper.ts +++ b/apps/server/src/cli/sshHelper.ts @@ -10,6 +10,7 @@ import * as Option from "effect/Option"; import { Argument, Command } from "effect/cli"; import * as DesktopUpdateMarker from "../desktopUpdate/DesktopUpdateMarker.ts"; +import { serviceLauncherIsServing, stateDirOwned } from "../serviceLauncher.ts"; const canBind = (port: number) => new Promise((resolve) => { @@ -142,7 +143,20 @@ const updateRemaining = Command.make("update-remaining", { Command.provide(DesktopUpdateMarker.layer), ); +/** Exits 0 while a server holds this home or a boot service is about to start one. */ +const homeOwned = Command.make("home-owned", { + baseDir: Argument.String("base-dir"), +}).pipe( + Command.withHandler(({ baseDir }) => + Effect.promise(async () => { + if (!((await serviceLauncherIsServing(baseDir)) || (await stateDirOwned(baseDir)))) { + process.exitCode = 1; + } + }), + ), +); + export const sshHelperCommand = Command.make("__ssh-helper").pipe( Command.unlisted, - Command.withSubcommands([pickPort, waitReady, runtimePort, updateRemaining]), + Command.withSubcommands([pickPort, waitReady, runtimePort, updateRemaining, homeOwned]), ); diff --git a/apps/server/src/cloud/selfUpdate.test.ts b/apps/server/src/cloud/selfUpdate.test.ts index 4cae117b2bcc..73a7fea4aea7 100644 --- a/apps/server/src/cloud/selfUpdate.test.ts +++ b/apps/server/src/cloud/selfUpdate.test.ts @@ -223,9 +223,20 @@ it.layer(NodeServices.layer)("server self update", (it) => { // regression: #34502 a remote update never replaces the launcher, so it can be older than the server it.effect("names its launcher's protocol to the staged preflight, not its own", () => Effect.gen(function* () { - const { selfUpdate, preflightProtocols } = yield* makeHarness({ launcherProtocol: 3 }); + const { selfUpdate, preflightProtocols, baseDir } = yield* makeHarness({ + launcherProtocol: 3, + }); + yield* TestClock.setTime(5_000); expect((yield* selfUpdate.update({ targetVersion: "1.1.0" })).updateId).toBe("launcher-id"); expect(preflightProtocols).toEqual(["3"]); + // That launcher holds no lock between its servers; SSH launches wait on this marker instead. + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + expect( + JSON.parse( + yield* fs.readFileString(path.join(baseDir, "runtime", "desktop-update-in-progress")), + ), + ).toEqual({ startedAt: 5_000, expiresAt: 65_000 }); }), ); @@ -346,6 +357,19 @@ it.layer(NodeServices.layer)("server self update", (it) => { }), ); + it.effect("stages the runtime without asking the launcher to restart", () => + Effect.gen(function* () { + const { selfUpdate, order } = yield* makeHarness(); + expect(yield* selfUpdate.update({ targetVersion: "1.1.0", stageOnly: true })).toEqual({ + targetVersion: "1.1.0", + method: "boot-service", + }); + expect(order).toEqual(["download", "extract", "preflight"]); + expect((yield* selfUpdate.update({ targetVersion: "1.1.0" })).updateId).toBe("launcher-id"); + expect(order).toEqual(["download", "extract", "preflight", "preflight", "accept"]); + }), + ); + // regression: #28198 a slow staged preflight on a loaded host no longer fails the update it.effect("waits for a staged preflight that takes a minute on a loaded host", () => Effect.gen(function* () { diff --git a/apps/server/src/cloud/selfUpdate.ts b/apps/server/src/cloud/selfUpdate.ts index 88c9d2d46bd3..2c6d04f8259f 100644 --- a/apps/server/src/cloud/selfUpdate.ts +++ b/apps/server/src/cloud/selfUpdate.ts @@ -1,4 +1,5 @@ import { + DESKTOP_UPDATE_IN_PROGRESS_MARKER_FILE, ServerSelfUpdateError, type ServerSelfUpdateCapability, type ServerSelfUpdateInput, @@ -29,6 +30,7 @@ import { import packageJson from "../../package.json" with { type: "json" }; import * as ServerConfig from "../config.ts"; import * as DesktopAppUpdate from "../desktopUpdate/DesktopAppUpdate.ts"; +import * as DesktopUpdateMarker from "../desktopUpdate/DesktopUpdateMarker.ts"; import { databaseMigrationHead } from "../persistence/databaseMigrationHead.ts"; import * as ManagedPolicy from "../policy/ManagedPolicy.ts"; import { managedUpdateMirror, managedUpdateRefusal } from "../policy/managedUpdates.ts"; @@ -323,6 +325,10 @@ export const make = Effect.fn("cloud.server_self_update.make")(function* () { }), ); + if (input.stageOnly === true) { + yield* Ref.set(inFlight, false); + return { targetVersion, method: "boot-service" as const }; + } yield* reportProgress("installing"); const updateId = yield* Effect.uninterruptible( launcher.requestUpdate({ targetVersion, dbPath: serverConfig.dbPath }).pipe( @@ -334,6 +340,13 @@ export const make = Effect.fn("cloud.server_self_update.make")(function* () { error, ), ), + // The launcher stops this server next; one from before its serving lock holds nothing + // until the new server starts, so this marker is what an SSH launch waits on. + Effect.tap(() => + DesktopUpdateMarker.publish( + path.join(serverConfig.baseDir, "runtime", DESKTOP_UPDATE_IN_PROGRESS_MARKER_FILE), + ).pipe(Effect.provideService(FileSystem.FileSystem, fs), Effect.ignore({ log: true })), + ), Effect.tap(() => onHandoffAccepted()), ), ); diff --git a/apps/server/src/cloud/serviceProtocol.ts b/apps/server/src/cloud/serviceProtocol.ts index c0a43bfe23e1..3800d58bdb1a 100644 --- a/apps/server/src/cloud/serviceProtocol.ts +++ b/apps/server/src/cloud/serviceProtocol.ts @@ -23,6 +23,9 @@ export const SERVICE_STOP_MARKER_FILE = ".service-stopping"; export const SERVICE_RESTART_PENDING_FILE = ".restart-pending"; /** Held by a Windows launcher while it runs: Task Scheduler tracks only the task's console host. */ export const SERVICE_LAUNCHER_LOCK_FILE = "service-launcher.lock"; +/** Held by a launcher while it has, or is about to start, a server: an SSH launch waits for that + server instead of starting its own. Released while the launcher idles. */ +export const SERVICE_SERVING_LOCK_FILE = "service-serving.lock"; const unquoteSystemdValue = (raw: string) => (raw.startsWith('"') && raw.endsWith('"') diff --git a/apps/server/src/desktopUpdate/DesktopUpdateMarker.ts b/apps/server/src/desktopUpdate/DesktopUpdateMarker.ts index 1f175bbe7db1..9befa63b4bec 100644 --- a/apps/server/src/desktopUpdate/DesktopUpdateMarker.ts +++ b/apps/server/src/desktopUpdate/DesktopUpdateMarker.ts @@ -10,6 +10,24 @@ import * as Schema from "effect/Schema"; const UpdateMarker = Schema.fromJsonString( Schema.Struct({ startedAt: Schema.Number, expiresAt: Schema.Number }), ); +const encodeUpdateMarker = Schema.encodeEffect(UpdateMarker); + +/** + * Makes SSH launches wait for the server that replaces this one instead of starting their own. + * The server writes it, so it also covers a boot service whose launcher predates its own lock. + */ +export const publish = Effect.fn("DesktopUpdateMarker.publish")(function* (markerFile: string) { + const fileSystem = yield* FileSystem.FileSystem; + const startedAt = yield* Clock.currentTimeMillis; + const contents = yield* encodeUpdateMarker({ + startedAt, + expiresAt: startedAt + DESKTOP_UPDATE_IN_PROGRESS_TTL_MS, + }); + // Renamed into place: a launch must never read a half-written marker. + const temporary = `${markerFile}.tmp`; + yield* fileSystem.writeFileString(temporary, contents); + yield* fileSystem.rename(temporary, markerFile); +}); export class DesktopUpdateMarker extends Context.Service< DesktopUpdateMarker, diff --git a/apps/server/src/device/DeviceService.test.ts b/apps/server/src/device/DeviceService.test.ts index 604b92c34d09..71c7b6994ca8 100644 --- a/apps/server/src/device/DeviceService.test.ts +++ b/apps/server/src/device/DeviceService.test.ts @@ -1782,67 +1782,84 @@ it.effect("reconciles SSH host settings without publishing no-op changes", () => ), ); -it.effect("a failed tool update reports npm's exit code but not its output", () => - Effect.gen(function* () { - const host: DeviceHost.DeviceHost["Service"] = { - id: LOCAL_DEVICE_HOST_ID, - label: "This machine", - summary: Effect.succeed({ +it.effect.each(["exit", "missing"] as const)( + "a failed tool update reports a safe %s failure", + (failure) => + Effect.gen(function* () { + const host: DeviceHost.DeviceHost["Service"] = { id: LOCAL_DEVICE_HOST_ID, - kind: "local", - label: "Test server", - platforms: [], - hubInstalled: false, - agentDeviceInstalled: false, - }), - platformAvailability: (platform) => Effect.succeed({ platform, available: false }), - ensureReady: () => Effect.die("Updating a tool does not start the hub"), - ensureAgentReady: () => Effect.die("Updating a tool does not start the agent"), - current: Effect.succeed(null), - stopAgent: Effect.void, - stop: Effect.void, - }; - const npmFailure = { - code: ChildProcessSpawner.ExitCode(243), - stdout: "", - stderr: "npm error EACCES https://private:credential@registry.example.test/expo-device-hub", - timedOut: false, - stdoutTruncated: false, - stderrTruncated: false, - stdoutInvalidUtf8: false, - stderrInvalidUtf8: false, - }; - const service = yield* DeviceService.make.pipe( - Effect.provideService(DeviceHost.DeviceHost, host), - Effect.provideService(ProcessRunner.ProcessRunner, { - run: () => Effect.succeed(npmFailure), - }), - Effect.provideService( - HttpClient.HttpClient, - HttpClient.make(() => Effect.die("Updating a tool makes no hub requests")), - ), - ); + label: "This machine", + summary: Effect.succeed({ + id: LOCAL_DEVICE_HOST_ID, + kind: "local", + label: "Test server", + platforms: [], + hubInstalled: false, + agentDeviceInstalled: false, + }), + platformAvailability: (platform) => Effect.succeed({ platform, available: false }), + ensureReady: () => Effect.die("Updating a tool does not start the hub"), + ensureAgentReady: () => Effect.die("Updating a tool does not start the agent"), + current: Effect.succeed(null), + stopAgent: Effect.void, + stop: Effect.void, + }; + const npmFailure = { + code: ChildProcessSpawner.ExitCode(243), + stdout: "", + stderr: "npm error EACCES https://private:credential@registry.example.test/expo-device-hub", + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + stdoutInvalidUtf8: false, + stderrInvalidUtf8: false, + }; + const service = yield* DeviceService.make.pipe( + Effect.provideService(DeviceHost.DeviceHost, host), + Effect.provideService(ProcessRunner.ProcessRunner, { + run: ({ command, args }) => + failure === "exit" + ? Effect.succeed(npmFailure) + : Effect.fail( + new ProcessRunner.ProcessSpawnError({ + command, + argumentCount: args.length, + cause: PlatformError.systemError({ + _tag: "NotFound", + module: "ChildProcess", + method: "spawn", + description: "private diagnostic text", + }), + }), + ), + }), + Effect.provideService( + HttpClient.HttpClient, + HttpClient.make(() => Effect.die("Updating a tool makes no hub requests")), + ), + ); - const error = yield* service.updateTool("hub").pipe(Effect.flip); - expect(error).toMatchObject({ - _tag: "DeviceOperationError", - reason: "command_failed", - exitCode: 243, - }); - expect(error.message).toBe( - "Device update device tool failed: The device command failed (exit code 243).", - ); - }).pipe( - Effect.provide( - Layer.mergeAll( - ServerSettings.layerTest({ enableDeviceSupport: true }), - layerSecretResolver, - ServerConfig.layerTest(process.cwd(), { prefix: "t3-device-update-" }), - NodeCrypto.layer, - NetService.layer, - ).pipe(Layer.provideMerge(NodeServices.layer)), + const error = yield* service.updateTool("hub").pipe(Effect.flip); + expect(error).toMatchObject({ + _tag: "DeviceOperationError", + reason: failure === "exit" ? "command_failed" : "package_manager_missing", + }); + expect(error.message).toBe( + failure === "exit" + ? "Device update device tool failed: The device command failed (exit code 243)." + : "Device update device tool failed: Device support needs npm, which is not on the T3 server's PATH. Install npm (it comes with Node.js from nodejs.org), restart T3 Code, then retry.", + ); + }).pipe( + Effect.provide( + Layer.mergeAll( + ServerSettings.layerTest({ enableDeviceSupport: true }), + layerSecretResolver, + ServerConfig.layerTest(process.cwd(), { prefix: "t3-device-update-" }), + NodeCrypto.layer, + NetService.layer, + ).pipe(Layer.provideMerge(NodeServices.layer)), + ), ), - ), ); describe("agent session release on close", () => { diff --git a/apps/server/src/device/DeviceService.ts b/apps/server/src/device/DeviceService.ts index 15e159062a32..78b31b87aefd 100644 --- a/apps/server/src/device/DeviceService.ts +++ b/apps/server/src/device/DeviceService.ts @@ -1430,8 +1430,9 @@ export const make = Effect.gen(function* () { (cause) => new DeviceOperationError({ operation: "update device tool", - reason: "command_failed", - // Only the exit code goes into the shown message; npm's stderr can carry registry credentials. + reason: + cause.reason === "npm_not_found" ? "package_manager_missing" : "command_failed", + // npm's stderr can carry registry credentials; expose only the reason and exit code. ...(cause.exitCode !== undefined ? { exitCode: cause.exitCode } : {}), cause, }), diff --git a/apps/server/src/environment/ServerEnvironment.ts b/apps/server/src/environment/ServerEnvironment.ts index 745e43ed0edd..6b5e6cb9e7e6 100644 --- a/apps/server/src/environment/ServerEnvironment.ts +++ b/apps/server/src/environment/ServerEnvironment.ts @@ -288,6 +288,7 @@ export const make = Effect.gen(function* () { ...(serverSelfUpdate === "boot-service" || desktopAppUpdate ? { serverSelfUpdateProgress: true } : {}), + ...(serverSelfUpdate === "boot-service" ? { serverUpdateStaging: true } : {}), ...(desktopAppUpdate ? { desktopAppUpdate: true, desktopAppChannelSwitch: true } : {}), serverBrowser: true, peerLinks: true, diff --git a/apps/server/src/mcp/McpHttpServer.test.ts b/apps/server/src/mcp/McpHttpServer.test.ts index bd32b3b81c82..3802a5c2e402 100644 --- a/apps/server/src/mcp/McpHttpServer.test.ts +++ b/apps/server/src/mcp/McpHttpServer.test.ts @@ -276,6 +276,7 @@ it.effect.each([ const layerRecordingTools = (calls: Array) => { const RecordingToolkit = Toolkit.make( OrchestratorToolkit.tools.delegate_task, + OrchestratorToolkit.tools.t3_thread_list, OrchestratorToolkit.tools.t3_thread_read, OrchestratorToolkit.tools.t3_thread_wait, ProjectToolkit.tools.t3_project_update, @@ -292,6 +293,7 @@ const layerRecordingTools = (calls: Array) => { RecordingToolkit, McpToolAccess.toLayer(RecordingToolkit, { delegate_task: record, + t3_thread_list: record, t3_thread_read: record, t3_thread_wait: record, t3_project_update: record, @@ -727,6 +729,19 @@ it.effect.each(["limit", "runLimit", "maxCharsPerItem"])( }, ); +it.effect("clamps a t3_thread_list limit above its maximum instead of rejecting the call", () => { + const calls: Array = []; + return Effect.gen(function* () { + yield* callTool("t3_thread_list", { allProjects: true, limit: 200 }); + yield* callTool("t3_thread_list", { allProjects: true, limit: 7 }); + + expect(calls).toEqual([ + { allProjects: true, limit: 100 }, + { allProjects: true, limit: 7 }, + ]); + }).pipe(Effect.provide(layerRecordingTools(calls))); +}); + it("keeps cross-tool routing out of the delegate_task description", () => { const description = OrchestratorToolkit.tools.delegate_task.description ?? ""; expect(description).toContain("Start one child agent with only `task`"); diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts index f0c4d00c7bdf..92f0e4205b63 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts @@ -8918,6 +8918,25 @@ describe("ClaudeAdapterV2 background wake turns", () => { { status: 503, cooldown: "999999999999999999999999h", seconds: 0 }, { status: 503, cooldown: "4m 6s", seconds: 0, errorType: "overloaded_error" }, { status: 401, cooldown: "4m 6s", seconds: 0 }, + { status: 429, cooldown: "1h2m3s", seconds: 3723, proxy: true, sdkResetSeconds: 60 }, + { status: 429, cooldown: "1h2m3s", seconds: 7200, proxy: true, sdkResetSeconds: 7200 }, + { status: 429, cooldown: "1h2m3s", seconds: 0, proxy: true, sdkResetSeconds: null }, + { + status: 429, + cooldown: "nested 1h", + seconds: 0, + proxy: true, + result: + "API Error: Request rejected (429) · All credentials for model claude-opus-5-5 are cooling down (last error: rate_limit_error (reset after 1h))", + }, + { + status: 429, + cooldown: "outer cooldown", + seconds: 60, + proxy: true, + result: + "API Error: Request rejected (429) · All credentials for model claude-opus-5-5 are cooling down (reset after 1m) (last error: All credentials for model upstream are cooling down (reset after 1h))", + }, ])("recovers wrapped gateway limits with status $status and cooldown $cooldown", (input) => Effect.gen(function* () { const harness = yield* makeWakeHarness; @@ -8933,13 +8952,35 @@ describe("ClaudeAdapterV2 background wake turns", () => { }), ); yield* Queue.offerAll(harness.sdkMessages, [ + ...(input.sdkResetSeconds !== undefined + ? [ + claudeSdkFrame({ + type: "rate_limit_event", + rate_limit_info: { + status: "rejected", + rateLimitType: "five_hour", + ...(input.sdkResetSeconds === null + ? {} + : { + resetsAt: DateTime.toEpochMillis(now) / 1000 + input.sdkResetSeconds, + }), + }, + uuid: "00000000-0000-4000-8000-000000000651", + session_id: WAKE_NATIVE_SESSION, + }), + ] + : []), makeAssistantErrorFrame({ uuid: "00000000-0000-4000-8000-000000000652", - error: "server_error", + error: input.proxy ? "rate_limit" : "server_error", }), makeResultFrame({ uuid: "00000000-0000-4000-8000-000000000653", - result: `API Error: ${input.status} [claude/claude-opus-5-5] [429]: {"type":"error","error":{"type":"${input.errorType ?? "rate_limit_error"}","message":"Please try again later."}} (reset after ${input.cooldown}).`, + result: + input.result ?? + (input.proxy + ? `API Error: Request rejected (429) · All credentials for model claude-opus-5-5 are cooling down (reset after ${input.cooldown}) (last error: rate_limit_error: Please try again later.)` + : `API Error: ${input.status} [claude/claude-opus-5-5] [429]: {"type":"error","error":{"type":"${input.errorType ?? "rate_limit_error"}","message":"Please try again later."}} (reset after ${input.cooldown}).`), terminalReason: "api_error", isError: true, apiErrorStatus: input.status, @@ -8948,7 +8989,10 @@ describe("ClaudeAdapterV2 background wake turns", () => { const terminal = yield* Queue.take(harness.terminalReceipts); assert.equal(terminal.status, "failed"); if (terminal.status !== "failed") return; - assert.equal(terminal.failure.class, input.seconds > 0 ? "usage_limit" : "provider_error"); + assert.equal( + terminal.failure.class, + input.seconds > 0 || input.proxy ? "usage_limit" : "provider_error", + ); if (input.seconds > 0) { assert.equal(terminal.failure.retryable, true); assert.equal( @@ -8958,7 +9002,7 @@ describe("ClaudeAdapterV2 background wake turns", () => { ), ); } else { - assert.isUndefined(terminal.failure.resetAt); + assert.equal(terminal.failure.resetAt, input.proxy ? null : undefined); } }).pipe( Effect.provide( @@ -17889,6 +17933,14 @@ describe("ClaudeAdapterV2 background wake turns", () => { yield* harness.awaitContinuation(1); yield* Queue.shutdown(harness.sdkMessages); + yield* harness.until(() => + harness.events.some( + (event) => + event.type === "subagent.updated" && + event.subagent.nativeTaskRef?.nativeId === OUTER_TASK_ID && + event.subagent.status === "completed", + ), + ); yield* yieldTicks(50); yield* harness.runtime.startTurn( makeClaudeTestTurnInput({ @@ -19043,12 +19095,36 @@ describe("ClaudeAdapterV2 background wake turns", () => { ), ); - it.effect("records only explicit effort for nested subagents", () => - Effect.gen(function* () { + it.effect("records explicit effort, else the effort a subagent's transcript reports", () => { + const UNSPECIFIED_TOOL_USE_ID = "toolu-effort-nested-unspecified"; + const firstRead = Deferred.makeUnsafe(); + return Effect.gen(function* () { const OUTER_TOOL_USE_ID = "toolu-effort-outer"; - const UNSPECIFIED_TOOL_USE_ID = "toolu-effort-nested-unspecified"; const OVERRIDING_TOOL_USE_ID = "toolu-effort-nested-overriding"; - const harness = yield* makeWakeHarness; + const fileSystem = yield* FileSystem.FileSystem; + const configDir = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3-claude-effort-", + }); + // Claude names the folder after the resolved cwd, here as if /workspace were a symlink. + const subagentsDir = NodePath.join( + configDir, + "projects", + "-private-workspace", + WAKE_NATIVE_SESSION, + "subagents", + ); + yield* fileSystem.makeDirectory(subagentsDir, { recursive: true }); + const transcriptPath = NodePath.join( + subagentsDir, + `agent-task-${UNSPECIFIED_TOOL_USE_ID}.jsonl`, + ); + yield* fileSystem.writeFileString( + transcriptPath, + `{"type":"user","uuid":"effort-line-0","message":{"role":"user","content":"Check one thing."}}\n`, + ); + const harness = yield* makeWakeHarnessWithOptions({ + environment: { CLAUDE_CONFIG_DIR: configDir }, + }); const now = yield* DateTime.now; const parentSelection = { instanceId: CLAUDE_TEST_MODEL_SELECTION.instanceId, @@ -19118,6 +19194,22 @@ describe("ClaudeAdapterV2 background wake turns", () => { }), taskStarted(UNSPECIFIED_TOOL_USE_ID, "00000000-0000-4000-8000-000000000224"), taskStarted(OVERRIDING_TOOL_USE_ID, "00000000-0000-4000-8000-000000000225"), + claudeSdkFrame({ + type: "system", + subtype: "task_notification", + task_id: `task-${UNSPECIFIED_TOOL_USE_ID}`, + tool_use_id: UNSPECIFIED_TOOL_USE_ID, + status: "completed", + output_file: "/tmp/claude-replay/tasks/unspecified.output", + summary: "OK", + uuid: "00000000-0000-4000-8000-000000000227", + session_id: WAKE_NATIVE_SESSION, + }), + makeSubagentToolResultFrame({ + parentToolUseId: OUTER_TOOL_USE_ID, + uuid: "00000000-0000-4000-8000-000000000228", + toolUseId: UNSPECIFIED_TOOL_USE_ID, + }), makeResultFrame({ uuid: "00000000-0000-4000-8000-000000000226", result: "Spawned the subagents.", @@ -19125,7 +19217,27 @@ describe("ClaudeAdapterV2 background wake turns", () => { ]) { yield* Queue.offer(harness.sdkMessages, frame); } + // The settled subagent's reply lands after the first lookup, beyond one read's cap. + yield* Deferred.await(firstRead); + NodeFS.appendFileSync( + transcriptPath, + `${"x".repeat(512 * 1024 + 1)}\n{"type":"assistant","effort":"low","uuid":"effort-line-1","message":{"role":"assistant","id":"msg_effort_child","content":[{"type":"text","text":"OK"}]}}\n`, + ); + yield* TestClock.adjust(500); yield* Queue.take(harness.terminalReceipts); + // A run can stop reading events once its subagents settle, so the settle carries the effort. + const settle = harness.events.find( + (event) => + event.type === "subagent.updated" && + event.subagent.status === "completed" && + event.subagent.nativeTaskRef?.nativeId === `task-${UNSPECIFIED_TOOL_USE_ID}`, + ); + assert.equal( + settle?.type === "subagent.updated" + ? getModelSelectionStringOptionValue(settle.subagent.modelSelection, "effort") + : null, + "low", + ); const efforts = new Map( harness.events.flatMap((event) => @@ -19141,7 +19253,7 @@ describe("ClaudeAdapterV2 background wake turns", () => { ); assert.deepEqual(Object.fromEntries(efforts), { [`task-${OUTER_TOOL_USE_ID}`]: "low", - [`task-${UNSPECIFIED_TOOL_USE_ID}`]: undefined, + [`task-${UNSPECIFIED_TOOL_USE_ID}`]: "low", [`task-${OVERRIDING_TOOL_USE_ID}`]: "max", }); const childEfforts = harness.events.flatMap((event) => @@ -19151,6 +19263,11 @@ describe("ClaudeAdapterV2 background wake turns", () => { ); assert.deepEqual(childEfforts, ["low", undefined, "max"]); }).pipe( + Effect.provideService(WorkflowAgentTranscriptReadObserver, (read) => + read.agentId === `task-${UNSPECIFIED_TOOL_USE_ID}` + ? Deferred.succeed(firstRead, undefined).pipe(Effect.asVoid) + : Effect.void, + ), Effect.provide( Layer.mergeAll( IdAllocator.layer, @@ -19159,8 +19276,8 @@ describe("ClaudeAdapterV2 background wake turns", () => { NodeCrypto.layer, ), ), - ), - ); + ); + }); it.effect.each([ { diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts index 510782872c81..f7f4d0c221d7 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts @@ -4639,7 +4639,13 @@ function providerFailureFromResult( const resetAt = apiErrorStatus === 429 && message.result.startsWith("API Error:") ? gatewayCooldownResetAt( - /\(reset after (?:(\d+)h ?)?(?:(\d+)m ?)?(?:(\d+)s)?\)/.exec(message.result), + // CLIProxyAPI nests upstream errors in its message; only its own cooldown counts. + (message.result.startsWith( + "API Error: Request rejected (429) · All credentials for model ", + ) + ? /^API Error: Request rejected \(429\) · All credentials for model [^\s()]+ are cooling down \(reset after (?:(\d+)h ?)?(?:(\d+)m ?)?(?:(\d+)s)?\)/ + : /\(reset after (?:(\d+)h ?)?(?:(\d+)m ?)?(?:(\d+)s)?\)/ + ).exec(message.result), receivedAt, ) : undefined; @@ -7510,6 +7516,63 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( : new Map(current).set(nativeThreadId, { ...entry, messages }); }); + // Claude Code records the effort a subagent ran at only in its transcript. + const readSubagentTranscriptEffort = Effect.fnUntraced(function* ( + context: ActiveClaudeTurnContext, + taskId: string, + completed: boolean, + ) { + const cwd = context.input.runtimePolicy.cwd; + if (cwd === null) return undefined; + const configured = adapterOptions.environment.CLAUDE_CONFIG_DIR?.trim(); + const configDir = configured ? path.resolve(cwd, configured) : undefined; + const projectsDir = path.join( + configDir ?? path.join(yield* HostProcess.HomeDirectory, ".claude"), + "projects", + ); + // Claude names the project folder after the resolved, possibly hashed, cwd; find the session instead. + let found: string | undefined; + for (const project of yield* fileSystem + .readDirectory(projectsDir) + .pipe(Effect.orElseSucceed((): ReadonlyArray => []))) { + const candidate = path.join(projectsDir, project, context.nativeThreadId, "subagents"); + if ( + yield* fileSystem + .exists(path.join(candidate, `agent-${taskId}.jsonl`)) + .pipe(Effect.orElseSucceed(() => false)) + ) { + found = candidate; + break; + } + } + if (found === undefined) return undefined; + const transcriptDir = found; + let metadata: WorkflowAgentMetadata = {}; + let cursor = initialWorkflowAgentTranscriptCursor; + const read = Effect.fnUntraced(function* () { + while (metadata.messageId === undefined) { + const next = yield* readWorkflowAgentTranscript({ + transcriptDir, + agentId: taskId, + cursor, + ...(configDir === undefined ? {} : { configDir }), + }).pipe(Effect.option); + if (Option.isNone(next)) return false; + metadata = mergeWorkflowAgentMetadata(next.value.lines, metadata); + const advanced = next.value.cursor.offset !== cursor.offset; + cursor = next.value.cursor; + if (!next.value.truncated || !advanced) break; + } + return true; + }); + // A completed subagent can settle before its first reply is flushed; wait out the flush once. + if ((yield* read()) && completed && metadata.messageId === undefined) { + yield* Effect.sleep(WORKFLOW_MEMBER_TRANSCRIPT_FLUSH_MS); + yield* read(); + } + return metadata.effort; + }); + const updateClaudeSubagentNode = Effect.fnUntraced(function* (input: { readonly context: ActiveClaudeTurnContext; readonly taskId: string; @@ -7613,8 +7676,23 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( workflow === undefined ? undefined : withWorkflowMemberThreadIds(workflow, childThreadId); - // Unstated effort stays unknown: a custom agent definition may override the owner's. - const effort = existingSubagent === undefined ? input.effort : existingSubagent.effort; + // Unstated effort stays unknown until the transcript reports it: a custom agent definition may override the owner's. + // Read it before the settle: a run stops consuming events once its subagents settle. + const effort = + existingSubagent === undefined + ? input.effort + : (existingSubagent.effort ?? + (lifecycleChanged && + input.status !== "running" && + existingSubagent.task.modelSelection !== undefined && + (input.nativeTaskType ?? existingSubagent.nativeTaskType) !== + CLAUDE_WORKFLOW_TASK_TYPE + ? yield* readSubagentTranscriptEffort( + input.context, + input.taskId, + input.status === "completed", + ) + : undefined)); const model = input.model ?? priorTask?.model ?? null; const task = { ...(priorTask ?? { @@ -11404,18 +11482,6 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( usageLimited && message.subtype === "success" && message.local_command === "compact" ? context.compactionFailure : null; - const resetTimes = Array.from(context.rateLimitResetTimes.values()); - const assistantLimit = context.latestAssistantRateLimit; - if ( - assistantLimit?.type != null && - !context.rateLimitResetTimes.has(assistantLimit.type) - ) { - resetTimes.push(assistantLimit.resetAt); - } - const resetAt = - resetTimes.length > 0 && resetTimes.every((time) => time !== null) - ? resetTimes.reduce((latest, time) => (time! > latest ? time! : latest), "") - : null; const resultFailure = interrupted ? null : limitedCompaction !== null @@ -11430,9 +11496,24 @@ export const makeClaudeAdapterV2 = Effect.fn("makeClaudeAdapterV2")(function* ( failureHint, usageLimited, ); + const resetTimes = Array.from(context.rateLimitResetTimes.values()); + const assistantLimit = context.latestAssistantRateLimit; + if ( + assistantLimit?.type != null && + !context.rateLimitResetTimes.has(assistantLimit.type) + ) { + resetTimes.push(assistantLimit.resetAt); + } + if (resultFailure?.class === "usage_limit" && resultFailure.resetAt != null) { + resetTimes.push(resultFailure.resetAt); + } + const resetAt = + resetTimes.length > 0 && resetTimes.every((time) => time !== null) + ? resetTimes.reduce((latest, time) => (time! > latest ? time! : latest), "") + : null; const terminalFailure = resultFailure?.class === "usage_limit" - ? { ...resultFailure, resetAt: resetAt ?? resultFailure.resetAt ?? null } + ? { ...resultFailure, resetAt } : resultFailure; yield* finalizeActiveTurn({ context, diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts index 884e953a495d..76c10382b3cb 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts @@ -1146,6 +1146,7 @@ describe("CodexAdapterV2 session initialize", () => { runtimePolicy: CODEX_TEST_RUNTIME_POLICY, }), initializeRequests: () => initializeRequests, + events: runtime.events, }; }); @@ -1293,6 +1294,89 @@ describe("CodexAdapterV2 session initialize", () => { ), ); + it.effect("fails the open when initialize never answers during open", () => + Effect.gen(function* () { + const initializeAwaitingResponse = yield* Deferred.make(); + const opened = yield* openReplaySession( + makeCodexReplayTranscript({ + scenario: "initialize-hung-open", + entries: replayPreamble("initialize-hung-open").slice(0, 2), + }), + (entry) => + entry.label === "initialize" + ? Deferred.succeed(initializeAwaitingResponse, undefined).pipe( + Effect.andThen(Effect.never), + ) + : Effect.void, + () => Effect.die("initialize never answers"), + ).pipe(Effect.flip, Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(initializeAwaitingResponse); + yield* TestClock.adjust("30 seconds"); + + // The session manager discards a session whose open fails. + assert.instanceOf(yield* Fiber.join(opened), ProviderAdapter.ProviderAdapterOpenSessionError); + }).pipe( + Effect.scoped, + Effect.provide( + Layer.mergeAll( + IdAllocator.layer, + McpProviderSessions.layer, + NodeServices.layer, + NodeCrypto.layer, + ), + ), + ), + ); + + it.effect("retires the shared session when initialize never answers", () => + Effect.gen(function* () { + const initializeAwaitingResponse = yield* Deferred.make(); + const session = yield* openReplaySession( + makeCodexReplayTranscript({ + scenario: "initialize-hung", + entries: replayPreamble("initialize-hung").slice(0, 2), + }), + (entry) => + entry.label === "initialize" + ? Deferred.succeed(initializeAwaitingResponse, undefined).pipe( + Effect.andThen(Effect.never), + ) + : Effect.void, + ); + const eventsExit = yield* Stream.runDrain(session.events).pipe( + Effect.exit, + Effect.forkChild({ startImmediately: true }), + ); + + const ensured = yield* session + .ensureThread("thread-initialize-hung") + .pipe(Effect.flip, Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(initializeAwaitingResponse); + const waiting = yield* session + .ensureThread("thread-initialize-hung-waiting") + .pipe(Effect.flip, Effect.forkChild({ startImmediately: true })); + yield* TestClock.adjust("30 seconds"); + + assert.equal((yield* Fiber.join(ensured))._tag, "ProviderAdapterEnsureThreadError"); + // A waiter fails at once instead of re-sending initialize to the hung app-server. + assert.equal((yield* Fiber.join(waiting))._tag, "ProviderAdapterEnsureThreadError"); + assert.equal(session.initializeRequests(), 1); + // The session manager retires a session whose event stream fails. + const exit = yield* Fiber.join(eventsExit); + assert.isTrue(Exit.isFailure(exit)); + }).pipe( + Effect.scoped, + Effect.provide( + Layer.mergeAll( + IdAllocator.layer, + McpProviderSessions.layer, + NodeServices.layer, + NodeCrypto.layer, + ), + ), + ), + ); + it.effect("completes the handshake after a caller is interrupted mid-initialize", () => Effect.gen(function* () { const initializeAwaitingResponse = yield* Deferred.make(); diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts index 25843a8bf824..06cb3079dad8 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts @@ -247,6 +247,8 @@ const decodeCodexBackgroundTerminalTerminateResponse = Schema.decodeUnknownEffec const decodeCodexBackgroundTerminalsListResponse = Schema.decodeUnknownEffect( CodexBackgroundTerminalsListResponse, ); +// Initialize normally answers in under a second; leave room for a slow first launch. +const CODEX_INITIALIZE_TIMEOUT = "30 seconds"; const CodexThreadSampleResponse = Schema.Struct({ text: Schema.NullOr(Schema.String) }); const decodeCodexThreadSampleResponse = Schema.decodeUnknownEffect(CodexThreadSampleResponse); export const CODEX_PROMPT_SUGGESTION_PROMPT = @@ -1938,7 +1940,15 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( }), ), ); + const events = yield* Queue.unbounded< + ProviderAdapter.ProviderAdapterV2Event, + ProviderAdapter.ProviderAdapterEventStreamError + >(); const initialized = yield* Ref.make(false); + // After a timeout the session is retiring; waiters must not re-send to the hung app-server. + const initializeTimeout = yield* Ref.make< + ProviderAdapter.ProviderAdapterEventStreamError | undefined + >(undefined); // Codex rejects a second initialize on the shared app-server. const initializePermit = yield* Semaphore.make(1); const ensureInitialized = initializePermit.withPermit( @@ -1947,6 +1957,8 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( if (alreadyInitialized) { return; } + const timedOut = yield* Ref.get(initializeTimeout); + if (timedOut !== undefined) return yield* timedOut; const userAgent = yield* client .request("initialize", { @@ -1962,6 +1974,23 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( ? Effect.succeed(null) : Effect.fail(error), }), + // A hung app-server never answers, and every thread on this shared + // session waits behind the permit. Failing the event stream retires + // the session, so the next turn spawns a fresh app-server. + Effect.timeoutOrElse({ + duration: CODEX_INITIALIZE_TIMEOUT, + orElse: () => { + const error = new ProviderAdapter.ProviderAdapterEventStreamError({ + driver: CODEX_PROVIDER, + providerSessionId: input.providerSessionId, + cause: `Codex app-server did not answer initialize within ${CODEX_INITIALIZE_TIMEOUT}.`, + }); + return Ref.set(initializeTimeout, error).pipe( + Effect.andThen(Queue.fail(events, error)), + Effect.andThen(Effect.fail(error)), + ); + }, + }), ); const version = userAgent && /\/(\S+)/u.exec(userAgent)?.[1]; const advisory = @@ -1991,10 +2020,6 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( model: input.modelSelection.model, now, }); - const events = yield* Queue.unbounded< - ProviderAdapter.ProviderAdapterV2Event, - ProviderAdapter.ProviderAdapterEventStreamError - >(); // Drain final notifications before the session manager retires the dead process. // Queue.fail lets readers take items already buffered before they see the failure. yield* client.awaitTermination.pipe( diff --git a/apps/server/src/orchestration-v2/EffectWorker.test.ts b/apps/server/src/orchestration-v2/EffectWorker.test.ts index cc43601543ee..c753e07d4689 100644 --- a/apps/server/src/orchestration-v2/EffectWorker.test.ts +++ b/apps/server/src/orchestration-v2/EffectWorker.test.ts @@ -16,12 +16,16 @@ import { import * as Cause from "effect/Cause"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Ref from "effect/Ref"; +import * as SqlError from "effect/sql/SqlError"; import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; import * as CheckpointRollbackService from "./CheckpointRollbackService.ts"; import * as CheckpointService from "./CheckpointService.ts"; @@ -396,6 +400,138 @@ it.effect("requeues a claim when a pre-execution worker check fails", () => }), ); +it.effect("keeps requeueing a failed claim while the database is locked", () => + Effect.gen(function* () { + const now = DateTime.formatIso(yield* DateTime.now); + const effectId = "effect:worker-requeue-lock-timeout"; + const workerId = "worker-requeue-lock-timeout"; + const claimedEffect: EffectOutbox.OrchestrationEffectV2 = { + id: effectId, + commandId: CommandId.make("command:worker-requeue-lock-timeout"), + threadId, + request: { type: "provider-runtime.continue", sourceRunId: runId }, + status: "running", + attemptCount: 1, + availableAt: now, + leaseOwner: workerId, + createdAt: now, + updatedAt: now, + completedAt: null, + lastError: null, + }; + const lockedWrites = yield* Ref.make(3); + const requeued = yield* Ref.make(false); + const firstLockTimeout = yield* Deferred.make(); + const layerOutbox = Layer.mock(EffectOutbox.EffectOutboxV2)({ + claimNext: () => Effect.succeedSome(claimedEffect), + get: () => Effect.succeedSome(claimedEffect), + awaitCancellation: () => Effect.never, + clearCancellation: () => Effect.void, + retry: () => + Effect.gen(function* () { + if ((yield* Ref.getAndUpdate(lockedWrites, (count) => count - 1)) > 0) { + yield* Deferred.succeed(firstLockTimeout, undefined); + return yield* new EffectOutbox.EffectOutboxError({ + operation: "retry", + effectId, + cause: new SqlError.SqlError({ + reason: new SqlError.LockTimeoutError({ cause: null }), + }), + }); + } + yield* Ref.set(requeued, true); + return true; + }), + }); + const layerExecutor = Layer.succeed( + EffectWorker.OrchestrationEffectExecutorV2, + EffectWorker.OrchestrationEffectExecutorV2.of({ + execute: () => + Effect.fail( + new EffectWorker.OrchestrationEffectExecutionError({ + effectId, + effectType: "provider-runtime.continue", + cause: "simulated database is locked", + }), + ), + }), + ); + const layerWorker = EffectWorker.layerWithOptions({ workerId }).pipe( + Layer.provide(Layer.merge(layerOutbox, layerExecutor)), + ); + + const workerFiber = yield* EffectWorker.OrchestrationEffectWorkerV2.pipe( + Effect.flatMap((worker) => worker.runOnce()), + Effect.provide(layerWorker), + Effect.forkChild, + ); + yield* Deferred.await(firstLockTimeout); + yield* TestClock.adjust(Duration.millis(99)); + assert.isFalse(yield* Ref.get(requeued)); + yield* TestClock.adjust(Duration.seconds(1)); + yield* Effect.exit(Fiber.join(workerFiber)); + + assert.isTrue(yield* Ref.get(requeued)); + }), +); + +it.effect.each([ + { name: "requeue", request: { type: "provider-runtime.continue", sourceRunId: runId } }, + { name: "terminalize", request: { type: "provider-turn.start", runId } }, +] as const)("interrupts $name settlement retries when the worker shuts down", ({ request }) => + Effect.gen(function* () { + const claimedEffect: EffectOutbox.OrchestrationEffectV2 = { + ...restartEffect(yield* DateTime.now, { type: "detach" }), + request, + }; + const settlementStarted = yield* Deferred.make(); + const releaseLock = yield* Ref.make(false); + const lockedWrite = Effect.gen(function* () { + yield* Deferred.succeed(settlementStarted, undefined); + // An in-flight database write can finish its own wait before noticing shutdown. + yield* Effect.sleep(Duration.millis(1)); + if (yield* Ref.get(releaseLock)) return true; + return yield* new EffectOutbox.EffectOutboxError({ + operation: "settlement", + effectId: claimedEffect.id, + cause: new SqlError.SqlError({ reason: new SqlError.LockTimeoutError({ cause: null }) }), + }); + }); + const layerOutbox = Layer.mock(EffectOutbox.EffectOutboxV2)({ + claimNext: () => Effect.succeedSome(claimedEffect), + get: () => Effect.succeedSome(claimedEffect), + awaitCancellation: () => Effect.never, + clearCancellation: () => Effect.void, + succeed: () => Effect.fail(new EffectOutbox.EffectOutboxError({ operation: "succeed" })), + retry: () => lockedWrite, + fail: () => lockedWrite, + }); + const layerExecutor = Layer.succeed( + EffectWorker.OrchestrationEffectExecutorV2, + EffectWorker.OrchestrationEffectExecutorV2.of({ execute: () => Effect.void }), + ); + const layerWorker = EffectWorker.layerWithOptions({ workerId: "test-worker" }).pipe( + Layer.provide(Layer.merge(layerOutbox, layerExecutor)), + ); + + const daemon = yield* EffectWorker.runDaemonWithOptions({ concurrency: 1 }).pipe( + Effect.provide(layerWorker), + Effect.forkChild, + ); + yield* Deferred.await(settlementStarted); + const shutdown = yield* Fiber.interrupt(daemon).pipe( + Effect.timeoutOption(Duration.millis(100)), + Effect.forkChild, + ); + yield* TestClock.adjust(Duration.millis(100)); + const stopped = yield* Fiber.join(shutdown); + yield* Ref.set(releaseLock, true); + yield* TestClock.adjust(Duration.seconds(1)); + yield* Fiber.await(daemon); + assert.isTrue(Option.isSome(stopped)); + }), +); + it.effect("arms cancellation before the durable pre-execution check", () => Effect.gen(function* () { const now = DateTime.formatIso(yield* DateTime.now); diff --git a/apps/server/src/orchestration-v2/EffectWorker.ts b/apps/server/src/orchestration-v2/EffectWorker.ts index 9dd507980574..bbb214542540 100644 --- a/apps/server/src/orchestration-v2/EffectWorker.ts +++ b/apps/server/src/orchestration-v2/EffectWorker.ts @@ -9,7 +9,9 @@ import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Metric from "effect/Metric"; import * as Option from "effect/Option"; +import * as Schedule from "effect/Schedule"; import * as Schema from "effect/Schema"; +import * as SqlError from "effect/sql/SqlError"; import { increment, @@ -667,6 +669,16 @@ export class OrchestrationEffectWorkerError extends Schema.TaggedError + SqlError.isSqlError(error.cause) && error.cause.reason._tag === "LockTimeoutError"; +const lockRetrySchedule = Schedule.spaced(Duration.millis(100)); +const retryWhileLocked = (settlement: Effect.Effect) => + Effect.interruptible( + Effect.retry(settlement, { schedule: lockRetrySchedule, while: isLockTimeout }), + ); + type ClaimFilter = Omit[0], "workerId">; export interface OrchestrationEffectWorkerV2Shape { @@ -728,6 +740,7 @@ export const layerWithOptions = ( delayMs: 0, }) .pipe( + retryWhileLocked, Effect.flatMap((requeued) => requeued ? Effect.logWarning("Requeued effect after unexpected worker failure", { @@ -759,6 +772,7 @@ export const layerWithOptions = ( error: `Worker failed to settle a process-bound effect after execution started: ${Cause.pretty(cause)}`, }) .pipe( + retryWhileLocked, Effect.flatMap((failed) => failed ? Effect.logError("Terminalized process-bound effect after settlement failure", { diff --git a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts index f36ce5bdc564..2845860efac2 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.control-reads.test.ts @@ -1254,6 +1254,7 @@ it.effect("keeps delegated child pull-request links independent of the parent", creationSource: "mcp", }); const updatedParent = yield* projections.getThreadProjection(parentThreadId); + assert.deepEqual(updatedParent.subagents[0]!.modelSelection, modelSelection); const childThreadId = updatedParent.subagents[0]!.childThreadId!; const child = yield* projections.getThreadProjection(childThreadId); assert.isNull(child.thread.linkedPullRequest); diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 2870e22ea500..c6d1ab465411 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -763,6 +763,18 @@ function cancelledAppOwnedTask( }; } +function askedForLinkedMachine( + card: Extract, + linked: Extract, +): boolean { + if (card.requestedAccess !== linked.requestedAccess) return false; + if (card.environmentId !== undefined) return card.environmentId === linked.linkedEnvironmentId; + if (card.url !== undefined) return linked.url !== undefined && card.url === linked.url; + const normalize = (text: string | undefined) => text?.trim().toLowerCase() ?? ""; + const hint = normalize(card.hint); + return hint !== "" && hint === normalize(linked.hint) && hint === normalize(linked.linkedLabel); +} + // A workspace change detaches the session, which can settle the calling run at once. Queuing its // follow-up in a later command would let a delegated task report its result before that exists. function metadataUpdateContinuation( @@ -9313,6 +9325,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio prompt: command.task, title: command.title ?? null, model: command.modelSelection.model, + modelSelection: command.modelSelection, ...(command.completionWake === undefined ? {} : { completionWake: command.completionWake }), status: "running", result: null, @@ -9775,6 +9788,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio prompt: command.task, title: command.title ?? null, model: command.modelSelection.model, + modelSelection: command.modelSelection, ...(command.completionWake === undefined ? {} : { completionWake: command.completionWake }), status: "running", result: null, @@ -10446,6 +10460,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio type: _type, commandId: _commandId, turnItemId: _turnItemId, + answeringTurnItemIds, wake, ...fields } = command; @@ -10505,6 +10520,39 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio occurredAt: now, payload: turnItem, }); + // The link answers the older cards that asked for the same machine and + // access. They end without waking anyone: the agent hears this link. + // A card still being answered stays open for its own answer to end. + if (!unchanged && command.linkStatus === "linked") { + for (const item of projection.turnItems) { + if ( + item.type !== "link_request" || + item.linkStatus !== "pending" || + item.ordinal >= turnItem.ordinal || + answeringTurnItemIds?.includes(item.id) === true || + !askedForLinkedMachine(item, command) + ) { + continue; + } + yield* emit( + events, + command, + )({ + type: "turn-item.updated", + threadId: command.threadId, + ...(item.runId === null ? {} : { runId: item.runId }), + ...(item.nodeId === null ? {} : { nodeId: item.nodeId }), + occurredAt: now, + payload: { + ...item, + status: "cancelled", + linkStatus: "cancelled", + completedAt: now, + updatedAt: now, + }, + }); + } + } // The agent asked without waiting, so the answer reaches it as a // notification: it starts a turn, or follows the one running. Only the // answer that closes the card wakes it, and never a closed thread. diff --git a/apps/server/src/orchestration-v2/ProjectionStore.test.ts b/apps/server/src/orchestration-v2/ProjectionStore.test.ts index c065fdd990d1..c7f770946586 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.test.ts @@ -10606,6 +10606,26 @@ it.layer(layerTest)("ProjectionStoreV2", (it) => { ["local", targetThreadId, "command_execution"], ], ); + + const sourceThread = (yield* projectionStore.getThreadProjection(sourceThreadId)).thread; + yield* projectionStore.apply({ + id: EventId.make("event:projection-fork-source-rollback:source-deleted"), + type: "thread.deleted", + threadId: sourceThreadId, + occurredAt: now, + payload: { ...sourceThread, deletedAt: now }, + }); + const afterSourceDeleted = yield* projectionStore.getShellSnapshot(); + assert.isUndefined(afterSourceDeleted.threads.find((thread) => thread.id === sourceThreadId)); + for (const forkId of [targetThreadId, nestedThreadId]) { + const expected = (yield* projectionStore.getThreadProjection(forkId)).visibleTurnItems + .length; + assert.equal((yield* projectionStore.getThreadShell(forkId))?.visibleItemCount, expected); + assert.equal( + afterSourceDeleted.threads.find((thread) => thread.id === forkId)?.visibleItemCount, + expected, + ); + } }), ); diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 93ccfd2298c3..0faa75ca31b5 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -1535,7 +1535,7 @@ function isRolledBackBeforeFork( return rolledBackAt !== null && DateTime.isLessThan(rolledBackAt, forkCreatedAt); } -function forkInheritedRunOrdinals( +export function forkInheritedRunOrdinals( runs: ReadonlyArray, forkCreatedAt: DateTime.Utc, ): Map { @@ -7669,7 +7669,12 @@ export const layer: Layer.Layer = worktreePath, threadIds, listedOnly = false, - }: ShellSnapshotOptions & { readonly listedOnly?: boolean } = {}, + includeDeleted = false, + }: ShellSnapshotOptions & { + readonly listedOnly?: boolean; + // A retained fork still shows the history of a deleted source run. + readonly includeDeleted?: boolean; + } = {}, ) => sql` SELECT @@ -7848,7 +7853,7 @@ export const layer: Layer.Layer = candidate.ordinal DESC, candidate.run_id DESC LIMIT 1 ) AND blocked.status = 'failed' - WHERE t.deleted_at IS NULL${ + WHERE ${includeDeleted ? sql`1 = 1` : sql`t.deleted_at IS NULL`}${ listedOnly ? sql` AND json_extract(t.payload_json, '$.lineage.relationshipToParent') IS NOT 'subagent' AND json_extract(t.payload_json, '$.lineage.sideChat') IS NOT 1` @@ -8505,7 +8510,7 @@ export const layer: Layer.Layer = while (pendingSourceIds.length > 0) { const sourceId = pendingSourceIds.pop(); if (sourceId === undefined || rowsByThreadId.has(sourceId)) continue; - const source = (yield* selectShellThreadRows(sourceId))[0]; + const source = (yield* selectShellThreadRows(sourceId, { includeDeleted: true }))[0]; if (source === undefined) continue; rowsByThreadId.set(sourceId, source); if (source.forked_from_run_source_thread_id !== null) { @@ -8985,7 +8990,9 @@ export const layer: Layer.Layer = if (nextId === undefined || rowsByThreadId.has(nextId)) { continue; } - const rows = yield* selectShellThreadRows(nextId); + const rows = yield* selectShellThreadRows(nextId, { + includeDeleted: nextId !== threadId, + }); const row = rows[0]; if (row === undefined) { continue; diff --git a/apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts b/apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts index 5720dd95686d..8fe7821b8cde 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts @@ -415,6 +415,7 @@ function makeLocalCommandHarness(input: { readonly unsentResume?: boolean; readonly repeatedResume?: boolean; readonly acceptedResume?: boolean; + readonly restartedResume?: boolean; readonly nativeForkFrom?: "idle" | "busy"; readonly switchedProvider?: boolean; readonly threadRuntimeMode?: OrchestrationV2ThreadProjection["thread"]["runtimeMode"]; @@ -816,6 +817,18 @@ function makeLocalCommandHarness(input: { providerThreadId: oldProviderThreadId, status: "interrupted", }, + ...(input.sideChat.restartedResume + ? [ + { + ...projection.attempts[0]!, + id: RunAttemptId.make("unaccepted-replacement"), + runId: RunId.make("stopped-run"), + attemptOrdinal: 2, + reason: "steering_restart" as const, + status: "interrupted" as const, + }, + ] + : []), ], providerTurns: [ ...projection.providerTurns, @@ -1602,6 +1615,14 @@ effectIt.effect.each([ unsentResume: true, acceptedResume: true, }, + { + contextRun: "delivered", + runtimeMode: "full-access", + context: "", + unsentResume: true, + acceptedResume: true, + restartedResume: true, + }, { contextRun: "current", threadRuntimeMode: "approval-required", @@ -1623,14 +1644,14 @@ effectIt.effect.each([ expect(harness.open).toHaveBeenCalledWith( expect.objectContaining({ runtimePolicy: expect.objectContaining({ runtimeMode }) }), ); + const resendsOriginal = + "unsentResume" in sideChat && + (!("acceptedResume" in sideChat) || "restartedResume" in sideChat); expect(harness.startTurn.mock.calls.map(([turn]) => turn.message.text)).toEqual([ - withInlineContext( - context, - "unsentResume" in sideChat && !("acceptedResume" in sideChat) ? "Original request" : "", - ), + withInlineContext(context, resendsOriginal ? "Original request" : ""), ]); expect(harness.startTurn.mock.calls[0]?.[0].resume === true).toBe( - context === "" && (!("unsentResume" in sideChat) || "acceptedResume" in sideChat), + context === "" && !resendsOriginal, ); expect(harness.projection().contextHandoffs).toMatchObject([ { diff --git a/apps/server/src/orchestration-v2/RemoteDelegatedTask.test.ts b/apps/server/src/orchestration-v2/RemoteDelegatedTask.test.ts index f075b43d9f88..292882d0f894 100644 --- a/apps/server/src/orchestration-v2/RemoteDelegatedTask.test.ts +++ b/apps/server/src/orchestration-v2/RemoteDelegatedTask.test.ts @@ -217,6 +217,7 @@ it.layer(layerTest)("a delegated task in a linked environment", (it) => { const [task] = projection.subagents; assert.equal(task?.childThreadId, null); assert.deepEqual(task?.remoteChild, remoteChild); + assert.deepEqual(task?.modelSelection, modelSelection); assert.equal(task?.status, "running"); const item = projection.turnItems.find((candidate) => candidate.type === "subagent"); assert.deepEqual(item?.type === "subagent" ? item.remoteChild : undefined, remoteChild); diff --git a/apps/server/src/orchestration-v2/ThreadSearch.test.ts b/apps/server/src/orchestration-v2/ThreadSearch.test.ts index 421e79f5a090..1dee6c9cfc0f 100644 --- a/apps/server/src/orchestration-v2/ThreadSearch.test.ts +++ b/apps/server/src/orchestration-v2/ThreadSearch.test.ts @@ -840,7 +840,7 @@ it.layer(layerTest)("ThreadSearch filters before it ranks", (it) => { }); it.effect.each([true, false])( - "ThreadSearch narrows short words outside ASCII before the hit limit (indexed: %s)", + "ThreadSearch narrows Cyrillic and accent-folded Latin short words before the hit limit (indexed: %s)", (indexed) => Effect.gen(function* () { const projections = yield* ProjectionStore.ProjectionStoreV2; @@ -848,12 +848,15 @@ it.effect.each([true, false])( const sql = yield* SqlClient.SqlClient; const project = ProjectId.make("project:short-limit"); const older = ThreadId.make("thread:short-older"); + const olderFrench = ThreadId.make("thread:short-older-french"); const newer = ThreadId.make("thread:short-newer"); yield* createProject(project); yield* Effect.forEach( [ thread(older, project), message(older, "older", "user", "В логах ошибка"), + thread(olderFrench, project), + message(olderFrench, "older-french", "user", "École fermée à Lyon"), thread(newer, project), ], projections.apply, @@ -870,15 +873,14 @@ it.effect.each([true, false])( WITH RECURSIVE n(i) AS (SELECT 1 UNION ALL SELECT i + 1 FROM n WHERE i < 2000) INSERT INTO orchestration_v2_projection_messages (message_id, thread_id, role, streaming, created_at, updated_at, payload_json) - SELECT 'short-newer-' || i, ${newer}, 'assistant', 0, ${DateTime.formatIso(at(5))}, - ${DateTime.formatIso(at(5))}, json_object('text', 'ошибка здесь') - FROM n + SELECT 'short-newer-' || i || '-' || word, ${newer}, 'assistant', 0, + ${DateTime.formatIso(at(5))}, ${DateTime.formatIso(at(5))}, json_object('text', text) + FROM n, (SELECT 'ru' AS word, 'ошибка здесь' AS text UNION ALL SELECT 'fr', 'fermée ici') `; - const { matches } = yield* search.search({ query: "ошибка в" }); - assert.deepEqual( - matches.map((match) => match.threadId), - [older], - ); + const threadIds = (query: string) => + search.search({ query }).pipe(Effect.map(({ matches }) => matches.map((m) => m.threadId))); + assert.deepEqual(yield* threadIds("ошибка в"), [older]); + assert.deepEqual(yield* threadIds("fermée ec"), [olderFrench]); }).pipe(Effect.provide(layerTest)), ); @@ -898,6 +900,9 @@ it.layer(layerTest)("ThreadSearch short words outside ASCII", (it) => { message(russian, "russian", "user", "В логах ошибка"), thread(french, project), message(french, "french", "user", "École fermée à Lyon"), + message(french, "french-shout", "user", "Météo école"), + message(french, "romanian", "user", "Școala închisă"), + message(french, "vietnamese", "user", "Hồ Chí Minh"), ], projections.apply, { discard: true }, @@ -908,8 +913,13 @@ it.layer(layerTest)("ThreadSearch short words outside ASCII", (it) => { const assertMatches = Effect.gen(function* () { assert.deepEqual(yield* threadIds("ошибка в"), [russian]); assert.deepEqual(yield* threadIds("fermée éc"), [french]); + assert.deepEqual(yield* threadIds("ecole fermée"), [french]); + assert.deepEqual(yield* threadIds("école me"), [french]); + assert.deepEqual(yield* threadIds("închisă sc"), [french]); + assert.deepEqual(yield* threadIds("chí ho"), [french]); // They still narrow: a word the message lacks rules it out. assert.deepEqual(yield* threadIds("ошибка я"), []); + assert.deepEqual(yield* threadIds("fermée ce"), []); }); yield* assertMatches; yield* sql`DROP TABLE orchestration_v2_message_search`; diff --git a/apps/server/src/orchestration-v2/ThreadSearch.ts b/apps/server/src/orchestration-v2/ThreadSearch.ts index d3c96a40ce2b..3bcdf9e84d4a 100644 --- a/apps/server/src/orchestration-v2/ThreadSearch.ts +++ b/apps/server/src/orchestration-v2/ThreadSearch.ts @@ -69,8 +69,10 @@ const TokenList = Schema.fromJsonString(Schema.Array(Schema.String)); const SearchRequest = Schema.Struct({ /** The FTS5 query, or null to scan messages instead. */ match: Schema.NullOr(Schema.String), - /** Tokens every hit must contain, checked on the text. */ - substrings: TokenList, + /** Tokens every hit must contain, as typed or as their GLOB folding case and Latin accents. */ + substrings: Schema.fromJsonString( + Schema.Array(Schema.Struct({ token: Schema.String, accentInsensitive: Schema.String })), + ), /** For each short word outside ASCII, the spellings a hit must contain one of. */ shortSpellings: Schema.fromJsonString(Schema.Array(Schema.Array(Schema.String))), projectId: Schema.NullOr(ProjectId), @@ -120,6 +122,37 @@ function shortTokenSpellings(token: string): ReadonlyArray { return [...new Set(spellings)]; } +// Latin-1 Supplement and Extended-A/B/Additional letters that fold to ASCII, +// including Romanian and Vietnamese letters SQLite's lower() leaves alone. +const accentedForms = Map.groupBy( + [ + ...Array.from({ length: 0x250 - 0xc0 }, (_, offset) => String.fromCharCode(0xc0 + offset)), + ...Array.from({ length: 0x1f00 - 0x1e00 }, (_, offset) => String.fromCharCode(0x1e00 + offset)), + ].filter((char) => /^[a-z]$/.test(foldSearchQuery(char))), + (char) => foldSearchQuery(char), +); +// Only text that may hold one of those letters (by UTF-8 lead byte) needs the GLOB: +// lower() already folds ASCII case. +const MESSAGE_HOLDS_ACCENTED_LATIN = [ + ...new Set( + [...accentedForms.values()].flat().map((char) => Buffer.from(char).toString("hex", 0, 1)), + ), +] + .map((lead) => `instr(CAST(message.text AS BLOB), x'${lead}') > 0`) + .join(" OR "); + +/** A GLOB pattern for `token` anywhere, in any case and with or without Latin accents. */ +function accentInsensitiveGlob(token: string): string { + const pattern = Array.from(token, (char) => { + const letter = foldSearchQuery(char); + if (/^[a-z]$/.test(letter)) { + return `[${letter}${letter.toUpperCase()}${accentedForms.get(letter)?.join("") ?? ""}]`; + } + return /[*?[]/.test(char) ? `[${char}]` : char; + }); + return `*${pattern.join("")}*`; +} + /** * The SQL inputs for a message search, or null when the query has no token * long enough to search. Tokens of three or more characters go to the @@ -137,7 +170,10 @@ function buildMessageSearch( const narrowing = short.filter((token) => !NON_ASCII.test(token)); return { match: indexed ? long.map((token) => `"${token.replaceAll('"', '""')}"`).join(" ") : null, - substrings: indexed ? narrowing : [...long, ...narrowing], + substrings: (indexed ? narrowing : [...long, ...narrowing]).map((token) => ({ + token, + accentInsensitive: accentInsensitiveGlob(token), + })), shortSpellings: short.filter((token) => NON_ASCII.test(token)).map(shortTokenSpellings), }; } @@ -176,10 +212,10 @@ export function buildSearchSnippet(text: string, matchStart: number): string { return `${start > 0 ? "…" : ""}${shown.slice(0, end)}${cut ? "…" : ""}`; } -// Filter hidden subagents and side chats before the hit limit so visible matches aren't lost. -// Word starts and ranking follow the title rules in JS. -// Without FTS5 trigrams, or for two-character CJK words, scan the same eligible messages. -// The scan folds ASCII case only, so accent variants and non-ASCII case variants may not match. +// Filter hidden subagents and side chats before the hit limit; JS checks word starts and ranking. +// Both paths narrow ASCII tokens with Latin-1 and Extended-A/B/Additional accent variants. +// Without FTS5 trigrams, or for two-character CJK words, scan the same eligible messages; +// accented queries and non-Latin case can still miss: "fermée"/"fermee", "ОШИБКА"/"ошибка". export const messageHitsQuery = ( sql: SqlClient.SqlClient, { match, substrings, shortSpellings, projectId, excludedThreadIds }: typeof SearchRequest.Encoded, @@ -244,8 +280,14 @@ export const messageHitsQuery = ( -- neither find nor a jump can reach them. AND messages.message_id NOT LIKE 'async-answer:%' AND NOT EXISTS ( - SELECT 1 FROM json_each(${substrings}) AS token - WHERE instr(lower(${messageSearchText(sql)}), token.value) = 0 + -- The text read once, not once per check. + SELECT 1 FROM (SELECT ${messageSearchText(sql)} AS text) AS message, + json_each(${substrings}) AS substring + WHERE instr(lower(message.text), json_extract(substring.value, '$.token')) = 0 + AND NOT ( + (${sql.literal(MESSAGE_HOLDS_ACCENTED_LATIN)}) + AND message.text GLOB json_extract(substring.value, '$.accentInsensitive') + ) ) AND NOT EXISTS ( SELECT 1 FROM json_each(${shortSpellings}) AS token diff --git a/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.test.ts b/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.test.ts index d56256372f73..325d4e4b7aab 100644 --- a/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.test.ts @@ -19,6 +19,7 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as TestClock from "effect/testing/TestClock"; import { ATTACHMENT_ONLY_BOOTSTRAP_PROMPT } from "@t3tools/shared/composerContextReferences"; import * as SqlitePersistence from "../persistence/Sqlite.ts"; @@ -271,6 +272,53 @@ function armRegeneration(input: { readonly command: string; readonly threadId: T }); } +/** Forks a source thread at its middle run, between a rolled-back run and later work. */ +function forkSeededSource(prefix: string, sideChat = false, rollbackPriorRun = true) { + return Effect.gen(function* () { + const threads = yield* ThreadManagement.ThreadManagementService; + const sourceThreadId = yield* createThread({ + command: `${prefix}:create`, + thread: `${prefix}:source`, + }); + yield* recordRun({ + threadId: sourceThreadId, + runId: RunId.make(`${prefix}:run:0`), + ordinal: 1, + user: "Rolled back attempt", + assistant: "Discarded.", + status: rollbackPriorRun ? "rolled_back" : "completed", + }); + yield* TestClock.adjust(1); + const forkRunId = RunId.make(`${prefix}:run:1`); + yield* recordRun({ + threadId: sourceThreadId, + runId: forkRunId, + ordinal: 2, + user: "Move session storage from Redis to Postgres", + assistant: "Sessions now persist in the Postgres sessions table.", + }); + yield* recordRun({ + threadId: sourceThreadId, + runId: RunId.make(`${prefix}:run:2`), + ordinal: 3, + user: "Later work the fork never saw", + assistant: "Done.", + }); + const forkThreadId = ThreadId.make(`${prefix}:target`); + yield* threads.dispatch({ + type: "thread.fork", + commandId: CommandId.make(`${prefix}:fork`), + sourceThreadId, + targetThreadId: forkThreadId, + sourcePoint: { type: "run", runId: forkRunId }, + sideChat, + createdBy: "user", + creationSource: "web", + }); + return { sourceThreadId, forkRunId, forkThreadId }; + }); +} + describe("formatThreadTitleContext", () => { it("pins the first user message ahead of the retained tail once content stops fitting", () => { const context = ThreadTitleRegeneration.formatThreadTitleContext([ @@ -443,80 +491,251 @@ describe("ThreadTitleRegenerationService", () => { }).pipe(Effect.provide(NodeServices.layer)), ); - it.effect("titles a fork's first message with the source history up to the fork point", () => + it.effect.each([ + { name: "its first message", followUp: true }, + { name: "no message of its own yet", followUp: false }, + ])("titles a fork with $name from the source history up to the fork point", ({ followUp }) => Effect.gen(function* () { const harness = makeHarness(); yield* Effect.gen(function* () { const threads = yield* ThreadManagement.ThreadManagementService; const titleRegeneration = yield* ThreadTitleRegeneration.ThreadTitleRegenerationService; - const sourceThreadId = yield* createThread({ - command: "command:title:fork:create", - thread: "thread:title:fork:source", + const { forkThreadId } = yield* forkSeededSource("thread:title:fork"); + const requestId = CommandId.make("command:title:fork:request"); + const messageId = MessageId.make("command:title:fork:request:message"); + if (followUp) { + yield* threads.dispatch({ + type: "message.dispatch", + commandId: requestId, + threadId: forkThreadId, + messageId, + text: "Try SQLite instead", + titleSeed: "Try SQLite instead", + attachments: [], + modelSelection, + dispatchMode: { type: "defer_start" }, + createdBy: "user", + creationSource: "web", + }); + } else { + yield* armRegeneration({ command: requestId, threadId: forkThreadId }); + } + + yield* titleRegeneration.execute({ + threadId: forkThreadId, + requestId, + kind: followUp ? { type: "initial", messageId } : { type: "regenerate" }, }); - yield* recordRun({ - threadId: sourceThreadId, - runId: RunId.make("run:title:fork:0"), - ordinal: 1, - user: "Rolled back attempt", - assistant: "Discarded.", - status: "rolled_back", + + const call = harness.generateThreadTitle.mock.calls[0]?.[0]; + assert.equal(call?.message, followUp ? "Try SQLite instead" : ""); + assert.equal(call?.forkSource?.title, "Seed title"); + assert.include(call?.forkSource?.history, "Move session storage from Redis to Postgres"); + assert.include(call?.forkSource?.history, "Sessions now persist in the Postgres"); + assert.notInclude(call?.forkSource?.history, "Later work the fork never saw"); + assert.notInclude(call?.forkSource?.history, "Rolled back attempt"); + const fork = yield* threads.getThreadRecords(forkThreadId, []); + assert.equal(fork.thread.title, "Generated title"); + }).pipe(Effect.provide(harness.layer)); + }), + ); + + it.effect.each(["plain", "deleted source"] as const)( + "refuses to title an empty thread with %s history", + (history) => + Effect.gen(function* () { + const harness = makeHarness(); + yield* Effect.gen(function* () { + const threads = yield* ThreadManagement.ThreadManagementService; + const titleRegeneration = yield* ThreadTitleRegeneration.ThreadTitleRegenerationService; + const { forkThreadId } = yield* forkSeededSource("thread:title:empty"); + const { thread } = yield* threads.getThreadProjection(forkThreadId); + const eventSink = yield* EventSink.EventSinkV2; + yield* eventSink.write({ + events: [ + { + id: EventId.make("empty:lineage"), + type: "thread.metadata-updated", + threadId: forkThreadId, + occurredAt: yield* DateTime.now, + payload: { + ...thread, + forkedFrom: + history === "plain" + ? null + : { + type: "run", + threadId: ThreadId.make("absent"), + runId: RunId.make("absent"), + }, + }, + }, + ], + }); + const requestId = yield* armRegeneration({ + command: "empty:request", + threadId: forkThreadId, + }); + yield* titleRegeneration.execute({ + threadId: forkThreadId, + requestId, + kind: { type: "regenerate" }, + }); + assert.equal(harness.generateThreadTitle.mock.calls.length, 0); + const fork = yield* threads.getThreadRecords(forkThreadId, []); + assert.deepInclude(fork.thread.lastTitleRegeneration, { + requestId, + status: "failed", + message: "This thread has no messages to title yet.", + }); + }).pipe(Effect.provide(harness.layer)); + }), + ); + + it.effect("keeps the fork point when the source rolls it back after forking", () => + Effect.gen(function* () { + const harness = makeHarness(); + yield* Effect.gen(function* () { + const threads = yield* ThreadManagement.ThreadManagementService; + const titleRegeneration = yield* ThreadTitleRegeneration.ThreadTitleRegenerationService; + const { sourceThreadId, forkRunId, forkThreadId } = yield* forkSeededSource( + "thread:title:rollback", + false, + false, + ); + yield* TestClock.adjust(1); + const source = yield* threads.getThreadRecords(sourceThreadId, ["runs"]); + const run = source.runs.find((run) => run.id === forkRunId); + assert.isDefined(run); + const now = yield* DateTime.now; + const eventSink = yield* EventSink.EventSinkV2; + yield* eventSink.write({ + events: [ + { + id: EventId.make("source:rollback"), + type: "run.updated", + threadId: sourceThreadId, + runId: run.id, + occurredAt: now, + payload: { ...run, status: "rolled_back", completedAt: now }, + }, + ], }); - const forkRunId = RunId.make("run:title:fork:1"); - yield* recordRun({ - threadId: sourceThreadId, - runId: forkRunId, - ordinal: 2, - user: "Move session storage from Redis to Postgres", - assistant: "Sessions now persist in the Postgres sessions table.", + const requestId = yield* armRegeneration({ + command: "rollback:request", + threadId: forkThreadId, }); - yield* recordRun({ + yield* titleRegeneration.execute({ + threadId: forkThreadId, + requestId, + kind: { type: "regenerate" }, + }); + const call = harness.generateThreadTitle.mock.calls[0]?.[0]; + assert.include(call?.forkSource?.history, "Move session storage from Redis to Postgres"); + assert.include(call?.forkSource?.history, "Rolled back attempt"); + assert.notInclude(call?.forkSource?.history, "Later work the fork never saw"); + const fork = yield* threads.getThreadRecords(forkThreadId, []); + assert.equal(fork.thread.title, "Generated title"); + }).pipe(Effect.provide(harness.layer)); + }), + ); + + it.effect("does not inherit imported history when the fork point is missing", () => + Effect.gen(function* () { + const harness = makeHarness(); + yield* Effect.gen(function* () { + const threads = yield* ThreadManagement.ThreadManagementService; + const titleRegeneration = yield* ThreadTitleRegeneration.ThreadTitleRegenerationService; + const { sourceThreadId, forkThreadId } = yield* forkSeededSource("thread:title:invalid"); + yield* dispatchUserMessage({ + command: "imported:message", threadId: sourceThreadId, - runId: RunId.make("run:title:fork:2"), - ordinal: 3, - user: "Later work the fork never saw", - assistant: "Done.", + text: "Imported history", }); - const forkThreadId = ThreadId.make("thread:title:fork:target"); - yield* threads.dispatch({ - type: "thread.fork", - commandId: CommandId.make("command:title:fork:fork"), - sourceThreadId, - targetThreadId: forkThreadId, - sourcePoint: { type: "run", runId: forkRunId }, - createdBy: "user", - creationSource: "web", + const source = yield* threads.getThreadRecords(sourceThreadId, []); + const fork = yield* threads.getThreadRecords(forkThreadId, []); + const eventSink = yield* EventSink.EventSinkV2; + yield* eventSink.write({ + events: [ + { + id: EventId.make("source:imported"), + type: "thread.metadata-updated", + threadId: sourceThreadId, + occurredAt: yield* DateTime.now, + payload: { ...source.thread, historyOrigin: "v1_import" }, + }, + { + id: EventId.make("fork:missing-run"), + type: "thread.metadata-updated", + threadId: forkThreadId, + occurredAt: yield* DateTime.now, + payload: { + ...fork.thread, + forkedFrom: { + type: "run", + threadId: sourceThreadId, + runId: RunId.make("absent:run"), + }, + }, + }, + ], }); - const requestId = CommandId.make("command:title:fork:message"); - const messageId = MessageId.make("command:title:fork:message:message"); - yield* threads.dispatch({ - type: "message.dispatch", - commandId: requestId, + const requestId = yield* armRegeneration({ + command: "invalid:request", threadId: forkThreadId, - messageId, - text: "Try SQLite instead", - titleSeed: "Try SQLite instead", - attachments: [], - modelSelection, - dispatchMode: { type: "defer_start" }, - createdBy: "user", - creationSource: "web", }); - yield* titleRegeneration.execute({ threadId: forkThreadId, requestId, - kind: { type: "initial", messageId }, + kind: { type: "regenerate" }, + }); + assert.equal(harness.generateThreadTitle.mock.calls.length, 0); + const updated = yield* threads.getThreadRecords(forkThreadId, []); + assert.deepInclude(updated.thread.lastTitleRegeneration, { + requestId, + status: "failed", + message: "This thread has no messages to title yet.", }); + }).pipe(Effect.provide(harness.layer)); + }), + ); + it.effect("inherits a side chat's own exchange without its parent's history", () => + Effect.gen(function* () { + const harness = makeHarness(); + yield* Effect.gen(function* () { + const threads = yield* ThreadManagement.ThreadManagementService; + const titleRegeneration = yield* ThreadTitleRegeneration.ThreadTitleRegenerationService; + const { forkThreadId: sideChatId } = yield* forkSeededSource( + "thread:title:side-ancestor", + true, + ); + const runId = RunId.make("side:run"); + yield* recordRun({ + threadId: sideChatId, + runId, + ordinal: 1, + user: "Side topic", + assistant: "Side answer", + }); + const threadId = ThreadId.make("side:fork"); + yield* threads.dispatch({ + type: "thread.fork", + commandId: CommandId.make("side:fork"), + sourceThreadId: sideChatId, + targetThreadId: threadId, + sourcePoint: { type: "run", runId }, + title: "Side fork", + createdBy: "user", + creationSource: "web", + }); + const requestId = yield* armRegeneration({ command: "side:request", threadId }); + yield* titleRegeneration.execute({ threadId, requestId, kind: { type: "regenerate" } }); const call = harness.generateThreadTitle.mock.calls[0]?.[0]; - assert.equal(call?.message, "Try SQLite instead"); - assert.equal(call?.forkSource?.title, "Seed title"); - assert.include(call?.forkSource?.history, "Move session storage from Redis to Postgres"); - assert.include(call?.forkSource?.history, "Sessions now persist in the Postgres"); - assert.notInclude(call?.forkSource?.history, "Later work the fork never saw"); - assert.notInclude(call?.forkSource?.history, "Rolled back attempt"); - const fork = yield* threads.getThreadProjection(forkThreadId); - assert.equal(fork.thread.title, "Generated title"); + assert.equal(call?.forkSource?.title, "Seed title fork"); + assert.include(call?.forkSource?.history, "Side topic"); + assert.include(call?.forkSource?.history, "Side answer"); + assert.notInclude(call?.forkSource?.history, "Move session storage"); }).pipe(Effect.provide(harness.layer)); }), ); diff --git a/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.ts b/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.ts index d3216b1cbd93..725a15aa136e 100644 --- a/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.ts +++ b/apps/server/src/orchestration-v2/ThreadTitleRegenerationService.ts @@ -14,6 +14,7 @@ import { import * as Cause from "effect/Cause"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import type * as DateTime from "effect/DateTime"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -24,6 +25,7 @@ import * as ServerSettings from "../serverSettings.ts"; import * as TextGeneration from "../textGeneration/TextGeneration.ts"; import type { OrchestratorV2Error } from "./Orchestrator.ts"; import * as ProjectStore from "./ProjectStore.ts"; +import { forkInheritedRunOrdinals, ProjectionStoreThreadNotFoundError } from "./ProjectionStore.ts"; import * as ThreadManagementService from "./ThreadManagementService.ts"; import { formatThreadTitleContext } from "@t3tools/provider-core/server/threadTitleContext"; @@ -55,6 +57,7 @@ type Outcome = const MAX_FAILURE_MESSAGE_LENGTH = 300; const isTextGenerationError = Schema.is(TextGenerationError); +const isProjectionStoreThreadNotFoundError = Schema.is(ProjectionStoreThreadNotFoundError); function failureMessage(cause: Cause.Cause): string { const error = Cause.squash(cause); @@ -103,16 +106,13 @@ const make = Effect.gen(function* () { .pipe(Effect.asVoid); const messagesThroughRun = Effect.fn("ThreadTitleRegenerationService.messagesThroughRun")( - function* (threadId: ThreadId, runId: RunId) { + function* (threadId: ThreadId, runId: RunId, forkCreatedAt: DateTime.Utc) { const source = yield* threads.getThreadRecords(threadId, ["messages", "runs"], { messageRoles: ["user", "assistant"], }); - const ordinals = new Map( - source.runs - .filter((run) => run.status !== "rolled_back") - .map((run) => [run.id, run.ordinal]), - ); - const forkOrdinal = ordinals.get(runId) ?? -1; + const ordinals = forkInheritedRunOrdinals(source.runs, forkCreatedAt); + const forkOrdinal = ordinals.get(runId); + if (forkOrdinal === undefined) return undefined; const messages = source.messages.filter((message) => { if (message.streaming) return false; if (message.runId === null) return source.thread.historyOrigin === "v1_import"; @@ -128,12 +128,20 @@ const make = Effect.gen(function* () { ) { let title: string | undefined; const history: Array> = []; - let forkedFrom = thread.forkedFrom; - while (forkedFrom?.type === "run") { - const source = yield* messagesThroughRun(forkedFrom.threadId, forkedFrom.runId); + const seen = new Set([thread.id]); + let current = thread; + while ( + current.forkedFrom?.type === "run" && + current.lineage.sideChat !== true && + !seen.has(current.forkedFrom.threadId) + ) { + const { threadId, runId } = current.forkedFrom; + seen.add(threadId); + const source = yield* messagesThroughRun(threadId, runId, current.createdAt); + if (source === undefined) break; history.push(source.messages); title ??= source.thread.title; - forkedFrom = source.thread.forkedFrom; + current = source.thread; } const context = formatThreadTitleContext(history.reverse().flat()); return title === undefined || context.message.length === 0 @@ -182,17 +190,25 @@ const make = Effect.gen(function* () { projection.messages.filter((message) => !message.streaming), ); } - if (context.message.length === 0 && context.attachments.length === 0) { + // A deleted source should not cost the fork its title. + const inherited = yield* forkSource(projection.thread).pipe( + Effect.catchTags({ + OrchestratorProjectionError: (error) => + isProjectionStoreThreadNotFoundError(error.cause) + ? Effect.undefined + : Effect.fail(error), + }), + ); + if ( + context.message.length === 0 && + context.attachments.length === 0 && + inherited === undefined + ) { return { status: "failed" as const, message: "This thread has no messages to title yet.", }; } - - // A deleted source should not cost the fork its title. - const inherited = yield* forkSource(projection.thread).pipe( - Effect.orElseSucceed(() => undefined), - ); const settings = resolveProjectSettings( yield* serverSettings.getSettings, projection.thread.projectId, diff --git a/apps/server/src/orchestration-v2/manualContinuation.ts b/apps/server/src/orchestration-v2/manualContinuation.ts index ee255cb154ea..9119e2aa7dad 100644 --- a/apps/server/src/orchestration-v2/manualContinuation.ts +++ b/apps/server/src/orchestration-v2/manualContinuation.ts @@ -8,16 +8,26 @@ export function unsentManualContinuationSource( if (sourceId === undefined) return undefined; const runs = new Map(projection.runs.map((source) => [source.id, source])); const acceptedAttempts = new Set(projection.providerTurns.map((turn) => turn.runAttemptId)); - const acceptedRuns = new Set( - projection.attempts - .filter((attempt) => acceptedAttempts.has(attempt.id)) - .map((attempt) => attempt.runId), - ); + const latestAttempt = ( + runId: OrchestrationV2Run["id"], + matches: (attempt: (typeof projection.attempts)[number]) => boolean, + ) => + Math.max( + 0, + ...projection.attempts + .filter((attempt) => attempt.runId === runId && matches(attempt)) + .map((attempt) => attempt.attemptOrdinal), + ); const visited = new Set(); while (sourceId !== undefined && !visited.has(sourceId)) { visited.add(sourceId); const source = runs.get(sourceId); - if (source === undefined || acceptedRuns.has(sourceId)) return undefined; + if (source === undefined) return undefined; + const accepted = latestAttempt(sourceId, (attempt) => acceptedAttempts.has(attempt.id)); + // An accepted attempt delivered everything before it; only a later steering restart's message is unsent. + const restarted = latestAttempt(sourceId, (attempt) => attempt.reason === "steering_restart"); + if (restarted > accepted) return source; + if (accepted > 0) return undefined; if (source.manualContinuationOfRunId === undefined) return source; sourceId = source.manualContinuationOfRunId; } diff --git a/apps/server/src/peer/PeerLinkRequests.test.ts b/apps/server/src/peer/PeerLinkRequests.test.ts index ab16197ba05b..2190e9a50e6a 100644 --- a/apps/server/src/peer/PeerLinkRequests.test.ts +++ b/apps/server/src/peer/PeerLinkRequests.test.ts @@ -7,6 +7,7 @@ import { MessageId, NodeId, OrchestratorMcpFailure, + type PeerLinkCreateInput, ProjectId, ProviderDriverKind, ProviderInstanceId, @@ -125,74 +126,86 @@ const scope: McpInvocationContext.McpInvocationScope = { issuedAt: 1, }; -/** The laptop: a real orchestrator, its orchestrator toolkit, and its links. */ -const makeLaptop = Effect.gen(function* () { - const linking = yield* layerLinkingEnvironment(laptop).pipe(Layer.build); - // The orchestrator's own database, apart from the one links are kept in. - const database = yield* SqlitePersistence.layerMemory.pipe(Layer.build); - const layerDatabase = Layer.succeedContext(database); - const layerOrchestrator = Layer.mergeAll( - layerDatabase, - ProjectionStore.layer.pipe(Layer.provide(layerDatabase)), - ProviderReplayHarness.layerWithRegistry( - { name: "peer-link-requests" }, - ProviderAdapterRegistry.layerFromAdapters([adapter]), - { databaseLayer: layerDatabase, runEffectWorker: false }, - ), - ).pipe( - Layer.provide( - Layer.succeed(ProviderContinuationRequests.ProviderContinuationRequests, { - offer: () => Effect.void, - take: Effect.never, - }), - ), - ); - const layerThreads = ThreadManagement.layer.pipe( - Layer.provide(CommandReceiptStore.layer.pipe(Layer.provide(layerDatabase))), - Layer.provideMerge(Layer.merge(layerOrchestrator, ServerSettings.layerTest())), - ); - const layerHere = McpHttpServer.layerOrchestratorToolkit.pipe( - Layer.provideMerge(McpHttpServer.layerInProcessMcpServer), - Layer.provideMerge(PeerLinkRequests.layer), - Layer.provide(Layer.mock(PeerForwarding.PeerForwarding)({})), - Layer.provide(Layer.mock(RemoteDelegation.RemoteDelegation)({})), - Layer.provide(Layer.succeedContext(linking)), - Layer.provide(NodeCrypto.layer), - Layer.provideMerge(layerThreads), - Layer.provide(Layer.mock(ProviderRegistry.ProviderRegistry)({})), - Layer.provide(Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({})), - Layer.provide(Layer.mock(ScheduledTaskService.ScheduledTaskService)({})), - Layer.provide(Layer.mock(ProjectService.ProjectService)({})), - Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), - Layer.provide(Layer.mock(ThreadLaunch.ThreadLaunchService)({})), - Layer.provide(Layer.mock(GitVcsDriver.GitVcsDriver)({})), - Layer.provide( - ServerConfig.layerTest(process.cwd(), { prefix: "t3-peer-link-requests-" }).pipe( - Layer.provide(NodeServices.layer), +/** A real orchestrator and links; the wrapper can hold a link in progress. */ +const makeLaptopWith = ( + wrapLinks?: (links: PeerLinks.PeerLinks["Service"]) => PeerLinks.PeerLinks["Service"], +) => + Effect.gen(function* () { + const linking = yield* layerLinkingEnvironment(laptop).pipe(Layer.build); + const answering = + wrapLinks === undefined + ? linking + : Context.add( + linking, + PeerLinks.PeerLinks, + wrapLinks(Context.get(linking, PeerLinks.PeerLinks)), + ); + // The orchestrator's own database, apart from the one links are kept in. + const database = yield* SqlitePersistence.layerMemory.pipe(Layer.build); + const layerDatabase = Layer.succeedContext(database); + const layerOrchestrator = Layer.mergeAll( + layerDatabase, + ProjectionStore.layer.pipe(Layer.provide(layerDatabase)), + ProviderReplayHarness.layerWithRegistry( + { name: "peer-link-requests" }, + ProviderAdapterRegistry.layerFromAdapters([adapter]), + { databaseLayer: layerDatabase, runEffectWorker: false }, ), - ), - Layer.provide(NodeServices.layer), - Layer.fresh, - ); - const here = yield* Layer.build(layerHere); - const server = Context.get(here, McpServer.McpServer); - return { - links: Context.get(linking, PeerLinks.PeerLinks), - linksSql: Context.get(linking, SqlClient.SqlClient), - requests: Context.get(here, PeerLinkRequests.PeerLinkRequests), - orchestrator: Context.get(here, Orchestrator.OrchestratorV2), - sink: Context.get(here, EventSink.EventSinkV2), - projections: Context.get(here, ProjectionStore.ProjectionStoreV2), - sql: Context.get(database, SqlClient.SqlClient), - call: (args: Record, callScope = scope) => - server - .callTool({ name: "t3_environment_link", arguments: args }) - .pipe( - Effect.provideService(McpInvocationContext.McpInvocationContext, callScope), - Effect.provideService(McpSchema.McpServerClient, mcpClient), + ).pipe( + Layer.provide( + Layer.succeed(ProviderContinuationRequests.ProviderContinuationRequests, { + offer: () => Effect.void, + take: Effect.never, + }), + ), + ); + const layerThreads = ThreadManagement.layer.pipe( + Layer.provide(CommandReceiptStore.layer.pipe(Layer.provide(layerDatabase))), + Layer.provideMerge(Layer.merge(layerOrchestrator, ServerSettings.layerTest())), + ); + const layerHere = McpHttpServer.layerOrchestratorToolkit.pipe( + Layer.provideMerge(McpHttpServer.layerInProcessMcpServer), + Layer.provideMerge(PeerLinkRequests.layer), + Layer.provide(Layer.mock(PeerForwarding.PeerForwarding)({})), + Layer.provide(Layer.mock(RemoteDelegation.RemoteDelegation)({})), + Layer.provide(Layer.succeedContext(answering)), + Layer.provide(NodeCrypto.layer), + Layer.provideMerge(layerThreads), + Layer.provide(Layer.mock(ProviderRegistry.ProviderRegistry)({})), + Layer.provide(Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({})), + Layer.provide(Layer.mock(ScheduledTaskService.ScheduledTaskService)({})), + Layer.provide(Layer.mock(ProjectService.ProjectService)({})), + Layer.provide(Layer.mock(SecretRequests.SecretRequests)({})), + Layer.provide(Layer.mock(ThreadLaunch.ThreadLaunchService)({})), + Layer.provide(Layer.mock(GitVcsDriver.GitVcsDriver)({})), + Layer.provide( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-peer-link-requests-" }).pipe( + Layer.provide(NodeServices.layer), ), - }; -}); + ), + Layer.provide(NodeServices.layer), + Layer.fresh, + ); + const here = yield* Layer.build(layerHere); + const server = Context.get(here, McpServer.McpServer); + return { + links: Context.get(linking, PeerLinks.PeerLinks), + linksSql: Context.get(linking, SqlClient.SqlClient), + requests: Context.get(here, PeerLinkRequests.PeerLinkRequests), + orchestrator: Context.get(here, Orchestrator.OrchestratorV2), + sink: Context.get(here, EventSink.EventSinkV2), + projections: Context.get(here, ProjectionStore.ProjectionStoreV2), + sql: Context.get(database, SqlClient.SqlClient), + call: (args: Record, callScope = scope) => + server + .callTool({ name: "t3_environment_link", arguments: args }) + .pipe( + Effect.provideService(McpInvocationContext.McpInvocationContext, callScope), + Effect.provideService(McpSchema.McpServerClient, mcpClient), + ), + }; + }); +const makeLaptop = makeLaptopWith(); type Laptop = Effect.Success; /** The laptop's thread, mid-turn when its agent asks, in `runtimeMode`. */ @@ -1118,3 +1131,344 @@ it.live( }), ), ); + +const linkCards = (a: Laptop) => + a.orchestrator + .getThreadProjection(threadId) + .pipe( + Effect.map((projection) => + projection.turnItems.flatMap((item) => (item.type === "link_request" ? [item] : [])), + ), + ); + +const ask = (a: Laptop, args: Record) => + Effect.gen(function* () { + const asked = yield* a.call(args); + const { turnItemId } = asked.structuredContent as { readonly turnItemId: TurnItemId }; + return (yield* linkCards(a)).find((card) => card.id === turnItemId)!; + }); + +const linkBox = (a: Laptop, b: ServedPeer, cardId: TurnItemId) => + Effect.gen(function* () { + const pairing = yield* b.auth.issuePairingCredential(); + yield* a.requests.answer({ + threadId, + turnItemId: cardId, + answer: { + type: "link", + urls: [b.url], + access: "full-access", + pairingCode: pairing.credential, + }, + }); + return pairing.credential; + }); + +const recordCard = ( + a: Laptop, + key: string, + fields: { + readonly url?: string; + readonly environmentId?: typeof box.environmentId; + readonly hint?: string; + readonly requestedAccess?: "read-only" | "full-access"; + }, +) => + Effect.gen(function* () { + const id = TurnItemId.make(`turn-item:link-request:${key}`); + yield* a.orchestrator.dispatch({ + type: "link_request.record", + commandId: CommandId.make(`command:pending:${key}`), + threadId, + runId, + nodeId: rootNode, + turnItemId: id, + reason: "", + linkStatus: "pending", + ...fields, + }); + return id; + }); + +it.live("a link ends older cards that asked for the same machine by hint, id or address", () => + withBoth((a, b) => + Effect.gen(function* () { + yield* seedThread(a); + // The agent first asks by the user's word, then again by the box's address. + const older = yield* ask(a, { + reason: "Delegate to the box.", + hint: "Box", + requestedAccess: "full-access", + clientRequestId: "by-hint", + }); + const byId = yield* ask(a, { + environmentId: box.environmentId, + requestedAccess: "full-access", + clientRequestId: "by-id", + }); + const byAddress = yield* recordCard(a, "by-address", { + url: b.url, + requestedAccess: "full-access", + }); + const newer = yield* ask(a, { + url: b.url, + reason: "Delegate to the box.", + hint: "Box", + requestedAccess: "full-access", + clientRequestId: "by-url", + }); + const credential = yield* linkBox(a, b, newer.id); + + const cards = yield* linkCards(a); + expect(cards.find((card) => card.id === newer.id)).toMatchObject({ + status: "completed", + linkStatus: "linked", + linkedEnvironmentId: box.environmentId, + }); + // The older card keeps what it asked and ends; it no longer asks the user. + expect(cards.find((card) => card.id === older.id)).toMatchObject({ + status: "cancelled", + linkStatus: "cancelled", + hint: "Box", + reason: "Delegate to the box.", + requestedAccess: "full-access", + }); + for (const id of [byId.id, byAddress]) { + expect(cards.find((card) => card.id === id)).toMatchObject({ linkStatus: "cancelled" }); + } + expect((yield* a.orchestrator.getThreadShell(threadId))?.pendingRuntimeRequest).toBeNull(); + expect( + ProjectionStore.threadShellFromProjection( + yield* a.orchestrator.getThreadProjection(threadId), + ).pendingRuntimeRequest, + ).toBeNull(); + // The agent hears of the one link it made, and nothing about the card that link ended. + expect((yield* outcomeMessages(a)).messages).toHaveLength(1); + expect(yield* storedText(a)).not.toContain(credential); + }), + ), +); + +it.live("a link leaves open the cards that asked for something else, or asked later", () => + withBoth((a, b) => + Effect.gen(function* () { + yield* seedThread(a); + const other = descriptorOf("environment-other", "Other"); + const left = [ + // Another machine, and a word that names neither. + yield* recordCard(a, "other-hint", { hint: "vps", requestedAccess: "full-access" }), + yield* recordCard(a, "no-hint", { requestedAccess: "full-access" }), + // The same word, but for less access than the link was asked with. + yield* recordCard(a, "less-access", { hint: "Box", requestedAccess: "read-only" }), + // The same word, on a card that named another machine. + yield* recordCard(a, "other-id", { + environmentId: other.environmentId, + hint: "Box", + requestedAccess: "full-access", + }), + yield* recordCard(a, "other-address", { + url: "http://127.0.0.1:9", + hint: "Box", + requestedAccess: "full-access", + }), + // A word the label only contains. + yield* recordCard(a, "partial-hint", { hint: "Bo", requestedAccess: "full-access" }), + ]; + const linked = yield* ask(a, { url: b.url, hint: "Box", requestedAccess: "full-access" }); + const later = yield* recordCard(a, "later", { hint: "Box", requestedAccess: "full-access" }); + yield* linkBox(a, b, linked.id); + + const cards = yield* linkCards(a); + expect(cards.find((card) => card.id === linked.id)).toMatchObject({ linkStatus: "linked" }); + for (const id of [...left, later]) { + expect(cards.find((card) => card.id === id)).toMatchObject({ + status: "waiting", + linkStatus: "pending", + }); + } + }), + ), +); + +it.live("a later card's failed link leaves the older card open", () => + withBoth((a, b) => + Effect.gen(function* () { + yield* seedThread(a); + const older = yield* ask(a, { hint: "Box", requestedAccess: "full-access" }); + const byAddress = yield* recordCard(a, "by-address", { + url: b.url, + requestedAccess: "full-access", + }); + const newer = yield* ask(a, { url: b.url, hint: "Box", requestedAccess: "full-access" }); + yield* a.requests.answer({ + threadId, + turnItemId: newer.id, + answer: { type: "link", urls: [b.url], access: "full-access", pairingCode: "not-a-code" }, + }); + + const cards = yield* linkCards(a); + expect(cards.find((card) => card.id === newer.id)).toMatchObject({ linkStatus: "failed" }); + for (const id of [older.id, byAddress]) { + expect(cards.find((card) => card.id === id)).toMatchObject({ + status: "waiting", + linkStatus: "pending", + }); + } + expect((yield* a.orchestrator.getThreadShell(threadId))?.pendingRuntimeRequest).toMatchObject( + { kind: "user_input" }, + ); + + // The older card still takes its answer, and the failed one stays as it ended. + yield* linkBox(a, b, older.id); + const after = yield* linkCards(a); + expect(after.find((card) => card.id === older.id)).toMatchObject({ linkStatus: "linked" }); + expect(after.find((card) => card.id === newer.id)).toMatchObject({ linkStatus: "failed" }); + }), + ), +); + +const withHeldLinks = ( + hold: (access: PeerLinkCreateInput["access"]) => Effect.Effect | undefined, + body: (a: Laptop, b: ServedPeer) => Effect.Effect, +) => + Effect.scoped( + Effect.gen(function* () { + const b = yield* serveBox; + const a = yield* makeLaptopWith((links) => ({ + ...links, + link: (input) => + (hold(input.access) ?? Effect.void).pipe(Effect.andThen(links.link(input))), + })); + return yield* body(a, b); + }), + ).pipe(Effect.provide(NodeHttpServer.layerTest)); + +it.live("an older card answered while a newer one links the same machine keeps its own link", () => + Effect.gen(function* () { + const reached = yield* Deferred.make(); + const release = yield* Deferred.make(); + const fullAccessHeld = Deferred.succeed(reached, undefined).pipe( + Effect.andThen(Deferred.await(release)), + ); + yield* withHeldLinks( + (access) => (access === "full-access" ? fullAccessHeld : undefined), + (a, b) => + Effect.gen(function* () { + yield* seedThread(a); + // Both cards suggest the same machine and access. + const older = yield* ask(a, { + url: b.url, + requestedAccess: "full-access", + clientRequestId: "older", + }); + const newer = yield* ask(a, { + url: b.url, + requestedAccess: "full-access", + clientRequestId: "newer", + }); + const olderCode = yield* b.auth.issuePairingCredential(); + const newerCode = yield* b.auth.issuePairingCredential(); + + // The user grants full access on the older card, which stalls while linking. + const olderAnswer = yield* a.requests + .answer({ + threadId, + turnItemId: older.id, + answer: { + type: "link", + urls: [b.url], + access: "full-access", + pairingCode: olderCode.credential, + }, + }) + .pipe(Effect.forkChild); + yield* Deferred.await(reached); + + // Meanwhile the newer card links with read-only access and finishes. + yield* a.requests.answer({ + threadId, + turnItemId: newer.id, + answer: { + type: "link", + urls: [b.url], + access: "read-only", + pairingCode: newerCode.credential, + }, + }); + // The older card is still being answered, so the newer link leaves it open. + expect((yield* linkCards(a)).find((card) => card.id === older.id)).toMatchObject({ + status: "waiting", + linkStatus: "pending", + }); + + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(olderAnswer); + + const cards = yield* linkCards(a); + const stored = Option.getOrThrow(yield* a.links.get(box.environmentId)); + expect(cards.find((card) => card.id === newer.id)).toMatchObject({ + status: "completed", + linkStatus: "linked", + linkedAccess: "read-only", + }); + // The older card says what the link the box now has grants, and the agent hears it. + expect(stored.access).toBe("full-access"); + expect(cards.find((card) => card.id === older.id)).toMatchObject({ + status: "completed", + linkStatus: "linked", + linkedEnvironmentId: stored.environmentId, + linkedAccess: stored.access, + }); + const { messages } = yield* outcomeMessages(a); + expect(messages.map((message) => message.notification?.outcome)).toEqual([ + "completed", + "completed", + ]); + }), + ); + }), +); + +it.live("a card a newer link ended links nothing when answered after", () => + Effect.gen(function* () { + let links = 0; + yield* withHeldLinks( + () => Effect.sync(() => void links++), + (a, b) => + Effect.gen(function* () { + yield* seedThread(a); + const older = yield* ask(a, { + url: b.url, + requestedAccess: "full-access", + clientRequestId: "older", + }); + const newer = yield* ask(a, { + url: b.url, + requestedAccess: "full-access", + clientRequestId: "newer", + }); + yield* linkBox(a, b, newer.id); + expect(links).toBe(1); + + const pairing = yield* b.auth.issuePairingCredential(); + const late = yield* a.requests + .answer({ + threadId, + turnItemId: older.id, + answer: { + type: "link", + urls: [b.url], + access: "read-only", + pairingCode: pairing.credential, + }, + }) + .pipe(Effect.flip); + expect(late.reason).toBe("already_answered"); + expect(links).toBe(1); + expect(Option.getOrThrow(yield* a.links.get(box.environmentId)).access).toBe( + "full-access", + ); + }), + ); + }), +); diff --git a/apps/server/src/peer/PeerLinkRequests.ts b/apps/server/src/peer/PeerLinkRequests.ts index 3d1129a599f8..a33bb47d7ec1 100644 --- a/apps/server/src/peer/PeerLinkRequests.ts +++ b/apps/server/src/peer/PeerLinkRequests.ts @@ -118,6 +118,7 @@ const make = Effect.gen(function* () { outcome: LinkRequestOutcome, commandId: CommandId, wake?: LinkRequestWake, + answeringTurnItemIds?: ReadonlyArray, ) => threadManagement.dispatch({ type: "link_request.record", @@ -133,6 +134,7 @@ const make = Effect.gen(function* () { ...(card.requestedAccess === undefined ? {} : { requestedAccess: card.requestedAccess }), ...outcome, ...(wake === undefined ? {} : { wake }), + ...(answeringTurnItemIds === undefined ? {} : { answeringTurnItemIds }), }); const linkedOutcome = (linked: PeerLink): LinkRequestOutcome => ({ @@ -145,6 +147,11 @@ const make = Effect.gen(function* () { // One answer per card at a time: two answers racing on one card must not // both link. Other cards go on while a slow peer holds one. const answerLocks = yield* KeyedLock.make(); + // Per thread, held only to read a card before answering and to record the + // answer, never while linking. A link ends older cards for the same + // machine, but not one already being answered (its key is in answerLocks); + // an answer that starts after that reads the ended card and links nothing. + const lifecycleLocks = yield* KeyedLock.make(); /** * Closes a card still waiting when its agent retries naming a machine linked @@ -153,22 +160,27 @@ const make = Effect.gen(function* () { const closeAsLinked = (threadId: ThreadId, turnItemId: TurnItemId, linked: PeerLink) => answerLocks.withLock( turnItemId, - Effect.gen(function* () { - const card = yield* readCard(threadId, turnItemId); - if ( - card === undefined || - card.linkStatus !== "pending" || - card.runId === null || - card.nodeId === null - ) { - return; - } - yield* record( - { ...card, runId: card.runId, nodeId: card.nodeId }, - linkedOutcome(linked), - CommandId.make(`link-request:${card.id}:linked-on-retry`), - ); - }), + lifecycleLocks.withLock( + threadId, + Effect.gen(function* () { + const card = yield* readCard(threadId, turnItemId); + if ( + card === undefined || + card.linkStatus !== "pending" || + card.runId === null || + card.nodeId === null + ) { + return; + } + yield* record( + { ...card, runId: card.runId, nodeId: card.nodeId }, + linkedOutcome(linked), + CommandId.make(`link-request:${card.id}:linked-on-retry`), + undefined, + yield* answerLocks.activeKeys, + ); + }), + ), ); const request: PeerLinkRequests["Service"]["request"] = Effect.fn("PeerLinkRequests.request")( @@ -340,17 +352,23 @@ const make = Effect.gen(function* () { "orchestration_v2.thread_id": input.threadId, "link_request.answer": input.answer.type, }); - const card = yield* readCard(input.threadId, input.turnItemId).pipe( - Effect.mapError((cause) => new PeerLinkRequestError({ reason: "load_failed", cause })), + const card = yield* lifecycleLocks.withLock( + input.threadId, + Effect.gen(function* () { + const card = yield* readCard(input.threadId, input.turnItemId).pipe( + Effect.mapError((cause) => new PeerLinkRequestError({ reason: "load_failed", cause })), + ); + if (card === undefined || card.runId === null || card.nodeId === null) { + return yield* new PeerLinkRequestError({ reason: "not_found" }); + } + // Any time while the card is open: the agent hears the answer as a + // notification, whether or not its run is still going. + if (card.linkStatus !== "pending") { + return yield* new PeerLinkRequestError({ reason: "already_answered" }); + } + return { ...card, runId: card.runId, nodeId: card.nodeId }; + }), ); - if (card === undefined || card.runId === null || card.nodeId === null) { - return yield* new PeerLinkRequestError({ reason: "not_found" }); - } - // Any time while the card is open: the agent hears the answer as a - // notification, whether or not its run is still going. - if (card.linkStatus !== "pending") { - return yield* new PeerLinkRequestError({ reason: "already_answered" }); - } // An id beside an address is what answered there, recorded by an older // server. Linking that machine could send its pairing code to the address. const outcome: LinkRequestOutcome = @@ -365,16 +383,22 @@ const make = Effect.gen(function* () { : yield* answerOutcome(input.answer); // If Stop closed the card while linking, the card keeps that; the link // itself stands and Settings lists it. - yield* record( - { ...card, runId: card.runId, nodeId: card.nodeId }, - outcome, - CommandId.make(`link-request:${card.id}:${outcome.linkStatus}`), - { - messageId: MessageId.make(`message:link-request:${card.id}`), - ...outcomeNotice(card, outcome), - }, - ).pipe( - Effect.mapError((cause) => new PeerLinkRequestError({ reason: "record_failed", cause })), + yield* lifecycleLocks.withLock( + input.threadId, + Effect.flatMap(answerLocks.activeKeys, (answering) => + record( + card, + outcome, + CommandId.make(`link-request:${card.id}:${outcome.linkStatus}`), + { + messageId: MessageId.make(`message:link-request:${card.id}`), + ...outcomeNotice(card, outcome), + }, + answering, + ), + ).pipe( + Effect.mapError((cause) => new PeerLinkRequestError({ reason: "record_failed", cause })), + ), ); }, (effect, input) => diff --git a/apps/server/src/project/ProjectService.deletion.test.ts b/apps/server/src/project/ProjectService.deletion.test.ts index 28b1c40541dd..25e5ee54a273 100644 --- a/apps/server/src/project/ProjectService.deletion.test.ts +++ b/apps/server/src/project/ProjectService.deletion.test.ts @@ -28,6 +28,7 @@ import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import * as ThreadCommandExecutor from "../orchestration-v2/ThreadCommandExecutor.ts"; import { planThreadDeletion } from "../orchestration-v2/ThreadDeletion.ts"; import * as SqlitePersistence from "../persistence/Sqlite.ts"; +import * as ProjectSettingsOverrideSweep from "../provider/ProjectSettingsOverrideSweep.ts"; import * as ServerSettings from "../serverSettings.ts"; import * as WorkspacePaths from "../workspace/WorkspacePaths.ts"; import * as ProjectEnrichmentService from "./ProjectEnrichmentService.ts"; @@ -501,9 +502,13 @@ it.effect("deletes a project without force once its imported threads were delete }).pipe(Effect.provide(layerDatabase)), ); -it.effect( - "clears a deleted project's settings overrides so a machine-disabled instance cannot stay alive", - () => +it.effect.each([ + { policy: "no", worktreeCleanup: undefined }, + { policy: "its off", worktreeCleanup: { mode: "off" } as const }, + { policy: "its legacy", worktreeCleanup: null }, +])( + "clears a deleted project's settings overrides, keeping $policy worktree cleanup policy, so a machine-disabled instance cannot stay alive", + ({ worktreeCleanup }) => Effect.gen(function* () { const projectId = ProjectId.make("project:overrides-cleanup"); const instanceId = ProviderInstanceId.make("codex_work"); @@ -514,7 +519,11 @@ it.effect( const serverSettings = yield* ServerSettings.ServerSettingsService; yield* serverSettings.updateSettings({ projectSettingsOverrides: { - [projectId]: { providerInstances: { [instanceId]: { enabled: true } } }, + [projectId]: { + providerInstances: { [instanceId]: { enabled: true } }, + defaultAutoPull: true, + ...(worktreeCleanup === undefined ? {} : { worktreeCleanup }), + }, }, }); @@ -523,8 +532,35 @@ it.effect( projectId, }); - const settingsAfterDelete = yield* serverSettings.getSettings; - assert.isUndefined(settingsAfterDelete.projectSettingsOverrides[projectId]); + const entry = (yield* serverSettings.getSettings).projectSettingsOverrides[projectId]; + if (worktreeCleanup === undefined) { + assert.isUndefined(entry); + } else { + assert.deepStrictEqual(entry, { worktreeCleanup, providerInstances: {} }); + } + + const liveProjectId = ProjectId.make("project:still-live"); + const missingProjectId = ProjectId.make("project:missing"); + yield* seedProject(liveProjectId); + // Older deletions or interrupted best-effort cleanup can leave the original overrides on restart. + yield* serverSettings.updateSettings({ + projectSettingsOverrides: { + [projectId]: { + providerInstances: { [instanceId]: { enabled: true } }, + defaultAutoPull: true, + ...(worktreeCleanup === undefined ? {} : { worktreeCleanup }), + }, + [liveProjectId]: { defaultAutoPull: true }, + [missingProjectId]: { worktreeCleanup: { mode: "off" } }, + }, + }); + yield* ProjectSettingsOverrideSweep.sweep; + assert.deepStrictEqual((yield* serverSettings.getSettings).projectSettingsOverrides, { + [liveProjectId]: { defaultAutoPull: true }, + ...(worktreeCleanup === undefined + ? {} + : { [projectId]: { worktreeCleanup, providerInstances: {} } }), + }); }).pipe(Effect.provide(layerServices)); }).pipe(Effect.provide(layerDatabase)), ); diff --git a/apps/server/src/project/ProjectService.ts b/apps/server/src/project/ProjectService.ts index d6bc3e8eedf4..4bdc8386438c 100644 --- a/apps/server/src/project/ProjectService.ts +++ b/apps/server/src/project/ProjectService.ts @@ -33,6 +33,7 @@ import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import * as ThreadCommandExecutor from "../orchestration-v2/ThreadCommandExecutor.ts"; import { planThreadDeletion } from "../orchestration-v2/ThreadDeletion.ts"; import * as ServerSettings from "../serverSettings.ts"; +import { deletedProjectOverridePatch } from "../provider/ProjectSettingsOverrideSweep.ts"; import { makeIsManagedWorktreePath } from "../worktreesDirectory.ts"; import * as ProcessRunner from "../processRunner.ts"; import * as ProjectEnrichmentService from "./ProjectEnrichmentService.ts"; @@ -550,7 +551,14 @@ export const make = Effect.gen(function* () { // Best-effort cleanup releases disabled instances kept alive by overrides; crashes have no replay to revisit deletion. const clearProjectSettingsOverrides = (projectId: ProjectId) => - serverSettings.updateSettings({ projectSettingsOverrides: { [projectId]: null } }).pipe( + serverSettings.getSettings.pipe( + Effect.flatMap(({ projectSettingsOverrides }) => + serverSettings.updateSettings({ + projectSettingsOverrides: { + [projectId]: deletedProjectOverridePatch(projectSettingsOverrides[projectId]), + }, + }), + ), Effect.asVoid, Effect.catchCause((cause) => Effect.logWarning( diff --git a/apps/server/src/provider/ClaudeModelCatalog.ts b/apps/server/src/provider/ClaudeModelCatalog.ts index 1afb1e1cc614..e3d4ea4397c0 100644 --- a/apps/server/src/provider/ClaudeModelCatalog.ts +++ b/apps/server/src/provider/ClaudeModelCatalog.ts @@ -223,7 +223,7 @@ export function claudeReportedModels( return models; } -function resolveClaudeCatalogModel( +export function resolveClaudeCatalogModel( catalog: ClaudeModelCatalog, slugOrAlias: string | null | undefined, ): ClaudeCatalogModel | undefined { diff --git a/apps/server/src/provider/Drivers/ClaudeSkills.test.ts b/apps/server/src/provider/Drivers/ClaudeSkills.test.ts index 8b9661a779ae..947d5f1a67ea 100644 --- a/apps/server/src/provider/Drivers/ClaudeSkills.test.ts +++ b/apps/server/src/provider/Drivers/ClaudeSkills.test.ts @@ -3,10 +3,19 @@ import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; +import * as HostProcess from "@t3tools/shared/HostProcess"; -import { discoverClaudeSkills } from "./ClaudeSkills.ts"; +import { discoverClaudeSkills as discoverSkills } from "./ClaudeSkills.ts"; import { planClaudeSkillDispatch } from "./ClaudeSkillDispatch.ts"; +// Windows makes the managed directory fixture-controlled without reading host policy or skills. +const discoverClaudeSkills = ( + ...[config, cwd, environment = {}]: Parameters +) => + discoverSkills(config, cwd, environment).pipe( + Effect.provideService(HostProcess.Platform, "win32"), + ); + const writeSkill = Effect.fn(function* ( skillsDir: string, directoryName: string, @@ -688,7 +697,7 @@ it.layer(NodeServices.layer)("discoverClaudeSkills", (it) => { }), ); - it.effect("prefers user skills on name collisions even with a stray .agents copy", () => + it.effect("prefers managed then user skills on collisions, even with a stray .agents copy", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -723,6 +732,62 @@ it.layer(NodeServices.layer)("discoverClaudeSkills", (it) => { description: "User deploy.", }, ]); + + const programFiles = path.join(tempDir, "Program Files"); + const programData = path.join(tempDir, "ProgramData"); + yield* writeSkill( + path.join(programData, "ClaudeCode", ".claude", "skills"), + "legacy-only", + "# Legacy skills must not be discovered", + ); + yield* fs.writeFileString( + path.join(programData, "ClaudeCode", "managed-settings.json"), + '{"skillOverrides":{"deploy":"off"}}', + ); + for (const ProgramFiles of [undefined, "", programFiles]) { + const legacySkills = yield* discoverClaudeSkills({ homePath: configDir }, workspace, { + PROGRAMDATA: programData, + ...(ProgramFiles !== undefined ? { ProgramFiles } : {}), + }); + assert.deepEqual( + legacySkills, + skills.map((skill) => ({ ...skill, enabled: false })), + ); + } + yield* writeSkill( + path.join(programFiles, "ClaudeCode", ".claude", "skills"), + "deploy", + ["---", "name: deploy", "description: Managed deploy.", "---"].join("\n"), + ); + yield* fs.writeFileString( + path.join(configDir, "settings.json"), + '{"skillOverrides":{"deploy":"off"}}', + ); + yield* fs.writeFileString( + path.join(programFiles, "ClaudeCode", "managed-settings.json"), + '{"skillOverrides":{"deploy":"on"}}', + ); + const managedSkills = yield* discoverClaudeSkills({ homePath: configDir }, workspace, { + ProgramFiles: programFiles, + PROGRAMDATA: programData, + }); + + assert.deepEqual(managedSkills, [ + { + name: "deploy", + path: path.join(programFiles, "ClaudeCode", ".claude", "skills", "deploy", "SKILL.md"), + enabled: true, + scope: "user", + description: "Managed deploy.", + }, + ]); + assert.deepEqual( + yield* discoverClaudeSkills({ homePath: configDir }, workspace, { + PROGRAMFILES: ` ${programFiles} `, + PROGRAMDATA: programData, + }), + managedSkills, + ); }), ); diff --git a/apps/server/src/provider/Drivers/ClaudeSkills.ts b/apps/server/src/provider/Drivers/ClaudeSkills.ts index c24c4afca9e7..a5df2671540f 100644 --- a/apps/server/src/provider/Drivers/ClaudeSkills.ts +++ b/apps/server/src/provider/Drivers/ClaudeSkills.ts @@ -106,19 +106,27 @@ function parseSkillFrontmatter(contents: string): SkillFrontmatter { }; } -function claudeManagedSettingsPath( +// A caller-supplied environment is a plain object, without Windows' case-insensitive keys. +function windowsEnvironmentValue(environment: NodeJS.ProcessEnv, name: string) { + const key = name.toLowerCase(); + return Object.entries(environment) + .find(([candidate]) => candidate.toLowerCase() === key)?.[1] + ?.trim(); +} + +function claudeManagedDirectory( path: Path.Path, platform: NodeJS.Platform, environment: NodeJS.ProcessEnv, ): string | undefined { if (platform === "darwin") { - return "/Library/Application Support/ClaudeCode/managed-settings.json"; + return "/Library/Application Support/ClaudeCode"; } if (platform === "win32") { - const programData = environment.PROGRAMDATA?.trim(); - return programData ? path.join(programData, "ClaudeCode", "managed-settings.json") : undefined; + const programFiles = windowsEnvironmentValue(environment, "ProgramFiles"); + return programFiles ? path.join(programFiles, "ClaudeCode") : undefined; } - return "/etc/claude-code/managed-settings.json"; + return "/etc/claude-code"; } /** Claude Code settings files, lowest precedence first. */ @@ -130,7 +138,9 @@ function claudeSettingsPaths( environment: NodeJS.ProcessEnv, repositoryRoot?: string, ): ReadonlyArray { - const managedPath = claudeManagedSettingsPath(path, platform, environment); + const managedDirectory = claudeManagedDirectory(path, platform, environment); + const programData = + platform === "win32" ? windowsEnvironmentValue(environment, "ProgramData") : undefined; const root = repositoryRoot !== undefined && repositoryRoot !== cwd ? repositoryRoot : undefined; return [ path.join(configDirPath, "settings.json"), @@ -141,7 +151,8 @@ function claudeSettingsPaths( ] : []), ...(root ? [path.join(root, ".claude", "settings.local.json")] : []), - ...(managedPath ? [managedPath] : []), + ...(programData ? [path.join(programData, "ClaudeCode", "managed-settings.json")] : []), + ...(managedDirectory ? [path.join(managedDirectory, "managed-settings.json")] : []), ]; } @@ -627,7 +638,7 @@ export function claudeLaunchArgDirectories(launchArgs?: string): ReadonlyArray & Partial>, cwd?: string, @@ -642,6 +653,7 @@ export const discoverClaudeSkills = Effect.fn("discoverClaudeSkills")(function* cwd, env, ); + const managedDirectory = claudeManagedDirectory(path, yield* HostProcess.Platform, env); // Most installs have no synced folder; skip reading `.claude.json` for them. const hasSyncedPlugins = !syncedPluginsHidden && @@ -653,6 +665,9 @@ export const discoverClaudeSkills = Effect.fn("discoverClaudeSkills")(function* : undefined; const roots: ReadonlyArray = [ + ...(managedDirectory + ? [{ directory: path.join(managedDirectory, ".claude", "skills"), scope: "user" as const }] + : []), { directory: path.join(configDirPath, "skills"), scope: "user" }, ...(cwd ? [{ directory: path.join(cwd, ".claude", "skills"), scope: "project" as const }] : []), ...claudeLaunchArgDirectories(config.launchArgs) diff --git a/apps/server/src/provider/ProjectSettingsOverrideSweep.ts b/apps/server/src/provider/ProjectSettingsOverrideSweep.ts index aa7d20527149..06f61f9ad8bf 100644 --- a/apps/server/src/provider/ProjectSettingsOverrideSweep.ts +++ b/apps/server/src/provider/ProjectSettingsOverrideSweep.ts @@ -1,32 +1,41 @@ -import { type ProjectId, type ServerSettings } from "@t3tools/contracts"; +import * as NodeUtil from "node:util"; + +import { type ProjectSettingsOverrides } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import * as Settings from "../serverSettings.ts"; -const findOrphanedProjectIds = ( - settings: Pick, - liveProjectIds: ReadonlySet, -): ReadonlyArray => - Object.keys(settings.projectSettingsOverrides) - .map((projectId) => projectId as ProjectId) - .filter((projectId) => !liveProjectIds.has(projectId)); +export function deletedProjectOverridePatch(entry: ProjectSettingsOverrides | undefined) { + // Keep cleanup policy for deleted threads; omitting providerInstances would preserve obsolete references in the merge. + return entry?.worktreeCleanup !== undefined + ? { worktreeCleanup: entry.worktreeCleanup, providerInstances: {} } + : null; +} export const sweep = Effect.gen(function* () { const serverSettings = yield* Settings.ServerSettingsService; const projectStore = yield* ProjectStore.ProjectStoreV2; const settings = yield* serverSettings.getSettings; - const liveShells = yield* projectStore.listShells(); - const liveProjectIds = new Set(liveShells.map((shell) => shell.id)); + const projects = yield* projectStore.list({ includeDeleted: true }); + const projectsById = new Map( + projects.map((project) => [project.projectId, project]), + ); - const orphanedProjectIds = findOrphanedProjectIds(settings, liveProjectIds); - if (orphanedProjectIds.length === 0) return; + const overrides = Object.entries(settings.projectSettingsOverrides).flatMap( + ([projectId, entry]) => { + const project = projectsById.get(projectId); + if (project?.deletedAt === null) return []; + const patch = project ? deletedProjectOverridePatch(entry) : null; + if (NodeUtil.isDeepStrictEqual(entry, patch)) return []; + return [[projectId, patch] as const]; + }, + ); + if (overrides.length === 0) return; yield* serverSettings.updateSettings({ - projectSettingsOverrides: Object.fromEntries( - orphanedProjectIds.map((projectId) => [projectId, null]), - ), + projectSettingsOverrides: Object.fromEntries(overrides), }); }).pipe( Effect.asVoid, diff --git a/apps/server/src/provider/agentContextBudget.json b/apps/server/src/provider/agentContextBudget.json index 695bc524e81b..c83ec4d169aa 100644 --- a/apps/server/src/provider/agentContextBudget.json +++ b/apps/server/src/provider/agentContextBudget.json @@ -1,5 +1,5 @@ { "claudeSystemAppendBytes": 5930, "codexAdditionalContextBytes": 7324, - "mcpToolListJsonBytes": 50583 + "mcpToolListJsonBytes": 50569 } diff --git a/apps/server/src/serviceLauncher.test.ts b/apps/server/src/serviceLauncher.test.ts index ac699b9c7d6c..4bfe776a3aba 100644 --- a/apps/server/src/serviceLauncher.test.ts +++ b/apps/server/src/serviceLauncher.test.ts @@ -29,6 +29,7 @@ import { readServiceState, registerServiceTask, serviceLauncherIsRunning, + serviceLauncherIsServing, serviceLauncherLockPath, stateDirOwned, writeServiceState, @@ -201,6 +202,212 @@ const fakeServiceUnit = ( }); it.layer(NodeServices.layer)("service state persistence", (it) => { + it.effect.each([ + "startup", + "log write fails", + "home probe error", + "database probe error", + "pending update", + "update handoff", + "trial exit", + "failed trial start", + "stop", + ] as const)("waits for disk recovery during %s without losing the update lifecycle", (scenario) => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "t3-service-disk-" }); + const statePath = path.join(root, "runtime", "service-state.json"); + const databasePath = path.join(root, "userdata", "statev2.sqlite"); + let backupPath = path.join(root, "runtime", "db-backup", "disk-update"); + const pending = { + id: "disk-update", + fromVersion: "1.0.0", + targetVersion: "1.1.0", + dbPath: databasePath, + status: "pending" as const, + }; + yield* fs.makeDirectory(path.dirname(databasePath), { recursive: true }); + yield* fs.writeFileString(databasePath, "before trial"); + const childSource = ` +const fs = require("node:fs"); +const context = JSON.parse(process.env.T3_SERVICE_LAUNCHER_CONTEXT); +if (context.update?.status === "pending") { + if (context.childVersion !== context.update.targetVersion) throw new Error("wrong trial version"); + if (fs.readFileSync(${JSON.stringify(path.dirname(backupPath))} + "/" + context.update.id + "/database", "utf8") !== "before trial") { + throw new Error("missing original backup"); + } + if (${JSON.stringify(scenario)} === "trial exit") { + fs.writeFileSync(context.update.dbPath, "migrated"); + process.on("message", () => process.exit(1)); + process.send("migrated"); + } else { + process.send({ type: "prepared", updateId: context.update.id }); + process.on("message", (message) => { + if (message.type === "committed") process.send("ready"); + }); + } +} else if (${JSON.stringify(scenario)} === "update handoff" && context.update === undefined) { + process.on("message", (message) => { + if (message.type === "update-accepted") process.send("exiting"); + if (message === "exit") process.exit(0); + }); + process.send({ type: "request-update", targetVersion: "1.1.0", dbPath: ${JSON.stringify(databasePath)} }); +} else { + process.send("ready"); +} +setInterval(() => {}, 1_000); +`; + for (const version of ["1.0.0", "1.1.0"]) { + yield* writeFakeRuntime( + fs, + path, + path.join(root, "runtime", "versions", version), + childSource, + ); + } + yield* Effect.promise(() => + writeServiceState(statePath, { + protocol: SERVICE_LAUNCHER_PROTOCOL, + activeVersion: "1.0.0", + ...([ + "startup", + "log write fails", + "home probe error", + "database probe error", + "update handoff", + "stop", + ].includes(scenario) + ? {} + : { update: pending }), + }), + ); + let low = [ + "startup", + "log write fails", + "home probe error", + "database probe error", + "pending update", + "stop", + ].includes(scenario); + let recovered = false; + const blocked = Promise.withResolvers(); + const ready = Promise.withResolvers(); + const probing = Promise.withResolvers(); + const capacityResult = Promise.withResolvers(); + const actualStatfs = NodeFSP.statfs; + const stats = yield* Effect.promise(() => actualStatfs(root)); + yield* Effect.acquireRelease( + Effect.sync(() => { + vi.useFakeTimers({ + toFake: ["setInterval", "clearInterval", "setTimeout", "clearTimeout"], + }); + const capacity = vi.spyOn(NodeFSP, "statfs").mockImplementation(async (directory) => { + if (scenario === "stop" && directory === root) { + probing.resolve(); + await capacityResult.promise; + } + if ( + (scenario === "home probe error" && directory === root) || + (scenario === "database probe error" && directory === path.dirname(databasePath)) + ) { + throw new Error("statfs unavailable"); + } + if (scenario === "failed trial start" && !recovered && NodeFS.existsSync(backupPath)) + low = true; + return { ...stats, bavail: low ? 0 : 1024 ** 4, bsize: 1 }; + }); + const log = vi.spyOn(process.stderr, "write").mockImplementation((chunk) => { + if (String(chunk).includes("Waiting for free space")) { + blocked.resolve(); + if (scenario === "log write fails") { + throw Object.assign(new Error("no space left"), { code: "ENOSPC" }); + } + } + return true; + }); + spawnHook.current = (child) => + child.on("message", (message) => { + if (message === "ready") ready.resolve(); + if (message === "migrated" || message === "exiting") { + low = true; + child.send("exit"); + } + }); + return { capacity, log }; + }), + ({ capacity, log }) => + Effect.sync(() => { + capacity.mockRestore(); + log.mockRestore(); + spawnHook.current = undefined; + vi.useRealTimers(); + }), + ); + const beforeSpawns = spawnedChildren.length; + const launcher = new Launcher(root, yield* Effect.promise(() => readServiceState(statePath))); + const running = launcher.run(); + yield* Effect.addFinalizer(() => + Effect.promise(async () => { + await launcher.stop("SIGTERM"); + await running; + }), + ); + if (scenario === "stop") { + yield* Effect.promise(() => probing.promise); + const stopping = launcher.stop("SIGTERM"); + capacityResult.resolve(); + yield* Effect.promise(() => stopping); + yield* Effect.promise(() => running); + assert.equal(spawnedChildren.length, beforeSpawns); + assert.equal(vi.getTimerCount(), 0); + return; + } + yield* Effect.promise(() => Promise.race([blocked.promise, ready.promise, running])); + assert.equal( + spawnedChildren.length - beforeSpawns, + ["trial exit", "update handoff"].includes(scenario) ? 1 : 0, + ); + if (scenario === "update handoff") { + const state = yield* Effect.promise(() => readServiceState(statePath)); + backupPath = path.join(root, "runtime", "db-backup", state.update!.id); + } + assert.equal( + yield* fs.exists(backupPath), + scenario === "trial exit" || scenario === "failed trial start", + ); + if (scenario === "trial exit") + assert.equal(yield* fs.readFileString(databasePath), "migrated"); + if (scenario === "failed trial start") { + yield* fs.remove(path.join(root, "runtime", "versions", "1.1.0"), { recursive: true }); + } + low = false; + recovered = true; + yield* Effect.promise(() => vi.advanceTimersByTimeAsync(30_000)); + yield* Effect.promise(() => Promise.race([ready.promise, running])); + const state = yield* Effect.promise(() => readServiceState(statePath)); + assert.equal( + state.activeVersion, + ["pending update", "update handoff"].includes(scenario) ? "1.1.0" : "1.0.0", + ); + assert.equal( + state.update?.status, + { + startup: undefined, + "log write fails": undefined, + "home probe error": undefined, + "database probe error": undefined, + "pending update": "committed", + "update handoff": "committed", + "trial exit": "rolled-back", + "failed trial start": "failed", + }[scenario], + ); + assert.equal(yield* fs.readFileString(databasePath), "before trial"); + assert.isFalse(yield* fs.exists(backupPath)); + }), + ); + it.effect( "loads optional service.env literally without changing inherited or managed variables", () => @@ -748,8 +955,11 @@ process.exit(0); }); yield* Effect.promise(() => refused.promise); assert.isFalse(completed); + // An idle launcher must not make an SSH launch wait for a server it is not starting. + assert.isFalse(yield* Effect.promise(() => serviceLauncherIsServing(root))); otherOwner.close(); yield* Effect.promise(() => owned.promise); + assert.isTrue(yield* Effect.promise(() => serviceLauncherIsServing(root))); yield* Effect.promise(() => launcher.stop("SIGTERM")); yield* Effect.promise(() => running); assert.strictEqual(spawnedChildren.length - spawnsBefore, 2); diff --git a/apps/server/src/serviceLauncher.ts b/apps/server/src/serviceLauncher.ts index d57a3abaccc9..fad391896d30 100644 --- a/apps/server/src/serviceLauncher.ts +++ b/apps/server/src/serviceLauncher.ts @@ -30,6 +30,7 @@ import { serviceUnitLogPathOf, SERVICE_ENV_FILE, SERVICE_LAUNCHER_LOCK_FILE, + SERVICE_SERVING_LOCK_FILE, SERVICE_LAUNCHER_CONTEXT_ENV, SERVICE_LAUNCHER_PROTOCOL, SERVICE_STATE_FILE, @@ -50,6 +51,9 @@ const HANDOFF_DELAY_MS = 2_000; */ const PREPARED_TIMEOUT_MS = 30 * 60_000; const TERMINATE_GRACE_MS = 5_000; +// Keep SQLite startup and journaling from turning a full filesystem into a restart loop. +const MIN_SERVICE_START_FREE_BYTES = 64 * 1024 ** 2; +const DISK_SPACE_RETRY_MS = 30_000; // A replacement started by a handoff waits up to two minutes for the stopping launcher. const LOCK_RETRY_MS = 500; const LOCK_ATTEMPTS = 240; @@ -755,11 +759,12 @@ export const serviceLauncherLockPath = (baseDir: string) => // An exclusive SQLite transaction, like the server's owner lock: the OS drops it when the holder dies. async function tryAcquireSqliteLock( lockPath: string, + busyTimeoutMs = 0, ): Promise<{ readonly close: () => void } | undefined> { const { DatabaseSync } = await import("node:sqlite"); const db = new DatabaseSync(lockPath); try { - db.exec("PRAGMA busy_timeout = 0; BEGIN EXCLUSIVE;"); + db.exec(`PRAGMA busy_timeout = ${busyTimeoutMs}; BEGIN EXCLUSIVE;`); return db; } catch (cause) { db.close(); @@ -775,6 +780,13 @@ export async function serviceLauncherIsRunning(lockPath: string): Promise + NodePath.join(baseDir, "runtime", SERVICE_SERVING_LOCK_FILE); + +/** Whether a launcher has, or is about to start, a server for this home. An idle one does not. */ +export const serviceLauncherIsServing = (baseDir: string): Promise => + serviceLauncherIsRunning(servingLockPath(baseDir)).catch(() => false); + /** Resolves once no launcher holds the lock; the caller bounds the wait through `signal`. */ export async function waitForServiceLauncherExit( lockPath: string, @@ -854,6 +866,7 @@ export class Launcher { #stateDirFreeProbes = 0; readonly #output: number | undefined; readonly #lockPath: string | undefined; + #servingLock: { readonly close: () => void } | undefined; readonly #stopChildOverIpc: boolean; constructor( @@ -907,6 +920,7 @@ export class Launcher { } } } finally { + this.#releaseServing(); process.off("SIGTERM", onSigterm); process.off("SIGINT", onSigint); } @@ -924,13 +938,29 @@ export class Launcher { return undefined; } + // Advisory: without it an SSH launch may start its own server, as it does beside no service. + async #claimServing(): Promise { + // An SSH launch probes this lock for a moment; wait out that probe. + this.#servingLock ??= await tryAcquireSqliteLock(servingLockPath(this.#baseDir), 250).catch( + () => undefined, + ); + } + + #releaseServing(): void { + this.#servingLock?.close(); + this.#servingLock = undefined; + } + #terminate(child: NodeChildProcess.ChildProcess, signal: NodeJS.Signals = "SIGTERM") { return terminateChild(child, signal, this.#stopChildOverIpc); } + // Diagnostics are best-effort: a full log filesystem must not stop the launcher waiting for space. #log(line: string): void { - if (this.#output === undefined) process.stderr.write(line); - else NodeFS.writeSync(this.#output, line); + try { + if (this.#output === undefined) process.stderr.write(line); + else NodeFS.writeSync(this.#output, line); + } catch {} } #enqueue(transition: () => Promise): void { @@ -983,6 +1013,55 @@ export class Launcher { this.#timer = undefined; } + async #serviceStartDiskSpaceBlocker(): Promise { + const databasePath = + this.#state.update?.status === "pending" + ? this.#state.update.dbPath + : NodePath.join(this.#baseDir, "userdata", "statev2.sqlite"); + const resolvedDatabasePath = await restoreTarget(databasePath).catch(() => databasePath); + const freeBytes = async (directory: string): Promise => { + try { + return await filesystemBackupSpaceProbe.readFreeBytes(directory); + } catch (cause) { + const parent = NodePath.dirname(directory); + if ((cause as NodeJS.ErrnoException).code === "ENOENT" && parent !== directory) { + return freeBytes(parent); + } + return Number.POSITIVE_INFINITY; + } + }; + const available = await Promise.all([ + freeBytes(this.#baseDir), + freeBytes(NodePath.dirname(resolvedDatabasePath)), + ]); + const free = Math.min(...available); + return free < MIN_SERVICE_START_FREE_BYTES + ? `T3 Code needs at least ${MIN_SERVICE_START_FREE_BYTES} bytes free before starting the server; ${free} bytes available.` + : undefined; + } + + async #deferForDiskSpace(retry: () => Promise): Promise { + const reason = await this.#serviceStartDiskSpaceBlocker(); + if (reason === undefined) return false; + // A stop during the probe must not arm a retry that outlives it. + if (this.#stopRequested) return true; + this.#clearTimer(); + this.#releaseServing(); + this.#log(`[service-launcher] ${reason} Waiting for free space before retrying.\n`); + this.#timer = setInterval( + () => + this.#enqueue(async () => { + if (this.#child !== null || this.#stopping) return; + if ((await this.#serviceStartDiskSpaceBlocker()) !== undefined) return; + this.#clearTimer(); + await this.#claimServing(); + await retry(); + }), + DISK_SPACE_RETRY_MS, + ); + return true; + } + async #selectState(next: ServiceState): Promise { const release = await acquireServiceSelectionLock(this.#statePath); try { @@ -1004,6 +1083,7 @@ export class Launcher { } async #recover(): Promise { + await this.#claimServing(); // Clear old stop markers to preserve future tunnels; clear restart-pending only for the awaited version to avoid CLI races. await NodeFSP.rm(stopMarkerPath(this.#baseDir), { force: true }).catch(() => undefined); const restartPending = restartPendingPath(this.#baseDir); @@ -1057,6 +1137,7 @@ export class Launcher { } async #startTrial(pending: PendingServiceUpdate): Promise { + if (await this.#deferForDiskSpace(() => this.#startTrial(pending))) return; try { await backupDatabaseOnce(this.#baseDir, pending, this.#stopSignal.signal); } catch { @@ -1075,6 +1156,14 @@ export class Launcher { async #startChild(version: string, role: ChildRole, update?: ServiceUpdateRecord): Promise { if (this.#stopping) return; + if ( + await this.#deferForDiskSpace(() => + role === "trial" && update?.status === "pending" + ? this.#startTrial(update) + : this.#startChild(version, role, update), + ) + ) + return; if (!(await runtimeExists(this.#baseDir, version))) { throw new Error(`Selected t3@${version} runtime is missing or incomplete.`); } @@ -1333,6 +1422,12 @@ export class Launcher { await this.#startTrial(pending); return; } + if ( + await this.#deferForDiskSpace(() => + this.#startChild(child.version, "active", this.#state.update), + ) + ) + return; throw new Error(`Active child exited unexpectedly (${String(code ?? signal ?? "unknown")}).`); } @@ -1358,6 +1453,7 @@ export class Launcher { await this.#repointServiceUnit(next.activeVersion); } } + this.#releaseServing(); this.#log( "[service-launcher] Another T3 Code server owns this T3 home. This service starts again once it stops.\n", ); @@ -1382,6 +1478,7 @@ export class Launcher { this.#stateDirFreeProbes = owned ? 0 : this.#stateDirFreeProbes + 1; if (this.#stateDirFreeProbes < 2) return; this.#clearTimer(); + await this.#claimServing(); this.#log("[service-launcher] The T3 home is free again. Starting the server.\n"); const update = this.#state.update; if (update?.status === "pending") await this.#startTrial(update); @@ -1398,6 +1495,8 @@ export class Launcher { this.#child = null; await this.#terminate(child.process); } + if (await this.#deferForDiskSpace(() => this.#returnToPrevious(pending, status, reason))) + return; await restoreDatabaseBackup(this.#baseDir, pending); const outcome = terminalUpdate({ pending, status, reason }); const next: ServiceState = { @@ -1432,6 +1531,8 @@ export async function main(options: LauncherOptions = {}): Promise { // oxlint-disable-next-line t3code/no-global-process-runtime -- Standalone launcher has no Effect runtime. const windows = process.platform === "win32"; let logTimer: NodeJS.Timeout | undefined; + // A failed stderr write (ENOSPC on a redirected log) is emitted asynchronously; without a listener it is fatal. + process.stderr.on("error", () => undefined); try { // What the service manager started the launcher with. It locates the unit; service.env // changes only what the server gets. diff --git a/apps/server/src/telemetry/ProviderDimensions.test.ts b/apps/server/src/telemetry/ProviderDimensions.test.ts index d88010f600e3..00e59c11228c 100644 --- a/apps/server/src/telemetry/ProviderDimensions.test.ts +++ b/apps/server/src/telemetry/ProviderDimensions.test.ts @@ -76,3 +76,16 @@ it("drops input strings outside the schema's literals, since rejected calls are handoffSettings("t3_thread_launch", { workspaceStrategy: { type: "/home/me/secret" } }, {}), ).toEqual({ targetChosen: false, workspace: "root" }); }); + +it("records the binding the saved scheduled task resolved to, and none for failures", () => { + expect(handoffSettings("schedule_task", {}, { boundThreadId: null })).toEqual({ + bindToCurrentThread: false, + }); + expect( + handoffSettings( + "schedule_task", + { bindToCurrentThread: true }, + { _tag: "OrchestratorMcpFailure", code: "invalid_request", message: "Different project." }, + ), + ).toEqual({}); +}); diff --git a/apps/server/src/telemetry/ProviderDimensions.ts b/apps/server/src/telemetry/ProviderDimensions.ts index f81a22e844ae..8352ed0d15cc 100644 --- a/apps/server/src/telemetry/ProviderDimensions.ts +++ b/apps/server/src/telemetry/ProviderDimensions.ts @@ -128,9 +128,9 @@ function callerOrigin(input: { } /** - * Settings the agent chose for a handoff, read only from closed enums and - * booleans in the tool's arguments and result. Prompts, titles, and ids are - * never read. + * Settings for a handoff, recorded only as closed enums and booleans derived + * from the tool's arguments and result. Prompts, titles, and ids are never + * recorded. */ export function handoffSettings(tool: string, args: unknown, result: unknown): Fields { const input = asFields(args); @@ -166,8 +166,13 @@ export function handoffSettings(tool: string, args: unknown, result: unknown): F return defined({ delivery: stringField(output, "delivery") }); case "t3_thread_send_attachments": return { delivery: "auto" }; - case "schedule_task": - return { bindToCurrentThread: input.bindToCurrentThread !== false }; + case "schedule_task": { + // Omitting the input lets the server decide, so read the saved task. + const bound = output.boundThreadId; + return bound === null || typeof bound === "string" + ? { bindToCurrentThread: bound !== null } + : {}; + } default: return {}; } diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index e7b1454e8d5c..07c3d8a21e47 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -6386,6 +6386,112 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }); describe("remote operations", () => { + it.effect.each([ + { + configuration: "ordinary", + fetchRefspecs: ["+refs/heads/*:refs/remotes/origin/*"], + addsMapping: false, + }, + { + configuration: "restricted", + fetchRefspecs: [ + "+refs/heads/main:refs/remotes/origin/main", + "+refs/heads/team/*:refs/remotes/origin/team/*", + "+refs/heads/release/release-*:refs/remotes/origin/release/release-*", + ], + addsMapping: true, + }, + ])( + "sets a fetched branch's upstream with $configuration fetch configuration", + ({ fetchRefspecs, addsMapping }) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + yield* initRepoWithCommit(cwd); + const remote = yield* makeTmpDir("git-remote-"); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["config", "--unset-all", "remote.origin.fetch"]); + for (const refspec of fetchRefspecs) { + yield* git(cwd, ["config", "--add", "remote.origin.fetch", refspec]); + } + const remoteBranch = "feature/example-change"; + const branch = "review/example-change"; + const mapping = `+refs/heads/${remoteBranch}*:refs/remotes/origin/${remoteBranch}*`; + yield* git(cwd, [ + "push", + remote, + "HEAD:refs/heads/main", + `HEAD:refs/heads/${remoteBranch}`, + ]); + const driver = yield* GitVcsDriver.GitVcsDriver; + yield* driver.fetchRemoteTrackingBranch({ cwd, remoteName: "origin", remoteBranch }); + yield* git(cwd, ["checkout", "--no-track", "-b", branch, `origin/${remoteBranch}`]); + + const input = { cwd, branch, remoteName: "origin", remoteBranch }; + yield* driver.setBranchUpstream(input); + yield* driver.setBranchUpstream(input); + + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", `${branch}@{upstream}`]), + `origin/${remoteBranch}`, + ); + assert.equal(yield* git(cwd, ["config", `branch.${branch}.remote`]), "origin"); + assert.equal( + yield* git(cwd, ["config", `branch.${branch}.merge`]), + `refs/heads/${remoteBranch}`, + ); + assert.deepEqual( + (yield* git(cwd, ["config", "--get-all", "remote.origin.fetch"])).split("\n"), + addsMapping ? [...fetchRefspecs, mapping] : fetchRefspecs, + ); + // A merged PR's head is deleted; later fetches of the remote must keep working. + yield* git(remote, ["--git-dir", remote, "branch", "-D", remoteBranch]); + yield* git(cwd, ["fetch", "origin"]); + }), + ); + + it.effect.each(["ordinary", "restricted"] as const)( + "retries upstream configuration with a config lock and %s fetch mappings", + (configuration) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* git(cwd, ["remote", "add", "origin", "https://example.invalid/repo.git"]); + if (configuration === "restricted") { + yield* git(cwd, [ + "config", + "remote.origin.fetch", + `+refs/heads/${initialBranch}:refs/remotes/origin/${initialBranch}`, + ]); + } + yield* git(cwd, ["branch", "review/pr-head"]); + yield* git(cwd, ["update-ref", "refs/remotes/origin/pr-head", "HEAD"]); + const lockPath = (yield* Path.Path).join(cwd, ".git", "config.lock"); + yield* (yield* FileSystem.FileSystem).writeFileString(lockPath, ""); + const { driver, failures } = yield* makeDriverReleasingLock( + configuration === "restricted" + ? (args) => args[0] === "config" && args[1] === "--add" + : (args) => args.includes("--set-upstream-to"), + lockPath, + ); + + yield* TestClock.withLive( + driver.setBranchUpstream({ + cwd, + branch: "review/pr-head", + remoteName: "origin", + remoteBranch: "pr-head", + }), + ); + + assert.equal(yield* Ref.get(failures), 1); + assert.equal( + yield* git(cwd, ["rev-parse", "--abbrev-ref", "review/pr-head@{upstream}"]), + "origin/pr-head", + ); + }), + ); + it.effect("ensureRemote reuses a partial clone remote and avoids its name", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 77b9444dc664..3df984a107ae 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -5210,19 +5210,51 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ]); }); - const setBranchUpstream: GitVcsDriver.GitVcsDriver["Service"]["setBranchUpstream"] = (input) => - executeGitWithStableDiagnostics( - "GitVcsDriver.setBranchUpstream", - input.cwd, - [ + const setBranchUpstream: GitVcsDriver.GitVcsDriver["Service"]["setBranchUpstream"] = Effect.fn( + "setBranchUpstream", + )( + function* (input) { + const args = [ "branch", "--set-upstream-to", - `${input.remoteName}/${input.remoteBranch}`, + `refs/remotes/${input.remoteName}/${input.remoteBranch}`, "--", input.branch, - ], - { writesSharedConfig: true }, - ).pipe(retryWhileConfigLocked, Effect.asVoid); + ]; + const result = yield* executeGitWithStableDiagnostics( + "GitVcsDriver.setBranchUpstream", + input.cwd, + args, + { allowNonZeroExit: true }, + ); + if (result.exitCode === 0) return; + // Git requires a configured fetch mapping even when the tracking ref already exists. The + // mapping is a glob because an exact source fails every later fetch once the branch is deleted. + if ( + /^fatal: cannot set up tracking information; starting point '.+' is not a branch$/m.test( + result.stderr, + ) + ) { + yield* executeGitWithStableDiagnostics( + "GitVcsDriver.setBranchUpstream.addFetchMapping", + input.cwd, + [ + "config", + "--add", + `remote.${input.remoteName}.fetch`, + `+refs/heads/${input.remoteBranch}*:refs/remotes/${input.remoteName}/${input.remoteBranch}*`, + ], + ).pipe(retryWhileConfigLocked); + } + yield* executeGitWithStableDiagnostics( + "GitVcsDriver.setBranchUpstream", + input.cwd, + args, + ).pipe(retryWhileConfigLocked); + }, + // Keep mapping detection and both writes under the same lock across sibling worktrees. + (effect, input) => withSharedConfigLock(input.cwd, effect), + ); /** * Unlinks links under `target` without following them and removes the diff --git a/apps/web/src/components/GitActionsControl.composition.browser.test.tsx b/apps/web/src/components/GitActionsControl.composition.browser.test.tsx new file mode 100644 index 000000000000..82e522045add --- /dev/null +++ b/apps/web/src/components/GitActionsControl.composition.browser.test.tsx @@ -0,0 +1,82 @@ +import { SourceControlProviderKind } from "@t3tools/contracts"; +import * as Option from "effect/Option"; +import { expect, it, vi } from "vite-plus/test"; +import { page, userEvent } from "vite-plus/test/browser"; + +import { act, render } from "../../test/render"; +import { PublishRepositoryDialog } from "./GitActionsControl"; + +const publish = vi.hoisted(() => ({ run: vi.fn() })); + +vi.mock("~/state/query", () => ({ + useEnvironmentQuery: () => ({ + data: { + versionControlSystems: [], + sourceControlProviders: [ + { + kind: SourceControlProviderKind.make("github"), + label: "GitHub", + status: "available", + version: Option.none(), + installHint: "Install gh", + detail: Option.none(), + auth: { + status: "authenticated", + account: Option.none(), + host: Option.none(), + detail: Option.none(), + }, + }, + ], + }, + }), +})); + +vi.mock("~/state/sourceControlActions", () => ({ + useGitStackedAction: vi.fn(), + useSourceControlActionRunning: vi.fn(), + useVcsInitAction: vi.fn(), + useVcsPullAction: vi.fn(), + useSourceControlPublishRepositoryAction: () => ({ + isAllowed: true, + isPending: false, + run: publish.run, + }), +})); + +function setup() { + publish.run.mockReset(); + publish.run.mockReturnValue(new Promise(() => {})); +} + +it("keeps composition Enter in the repository path from publishing", async () => { + setup(); + await using _screen = await render( + {}} + environmentId={null} + threadRef={null} + gitCwd="/repo" + offeredProviders={undefined} + />, + ); + await page.getByRole("button", { name: "Next" }).click(); + const path = page.getByRole("textbox", { name: "Repository" }); + await path.fill("octo/repo"); + + for (const nativeEvent of [{ isComposing: true }, { keyCode: 229 }]) { + const event = new KeyboardEvent("keydown", { + key: "Enter", + ...nativeEvent, + bubbles: true, + cancelable: true, + }); + await act(() => path.element().dispatchEvent(event)); + expect(publish.run).not.toHaveBeenCalled(); + expect(event.defaultPrevented).toBe(false); + } + + await userEvent.keyboard("{Enter}"); + expect(publish.run).toHaveBeenCalledOnce(); +}); diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index d050ea9c10b7..18bd6e6b0cf3 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -468,7 +468,7 @@ interface PublishRepositoryDialogProps { readonly offeredProviders: OfferedSourceControlProviders | undefined; } -function PublishRepositoryDialog(props: PublishRepositoryDialogProps) { +export function PublishRepositoryDialog(props: PublishRepositoryDialogProps) { const openLink = useOpenLink(props.threadRef); const navigate = useNavigate(); const sourceControlDiscovery = useEnvironmentQuery( @@ -740,10 +740,14 @@ function PublishRepositoryDialog(props: PublishRepositoryDialogProps) { setPublishRepositoryOverride(event.target.value); }} onKeyDown={(event) => { - if (event.key === "Enter") { - event.preventDefault(); - submitPublishRepository(); + if (event.key !== "Enter") { + return; } + if (event.nativeEvent.isComposing || event.keyCode === 229) { + return; + } + event.preventDefault(); + submitPublishRepository(); }} placeholder={publishPathPlaceholder} disabled={publishRepositoryAction.isPending} diff --git a/apps/web/src/components/Sidebar.drag.browser.test.tsx b/apps/web/src/components/Sidebar.drag.browser.test.tsx index c5811df12b76..8151de08a711 100644 --- a/apps/web/src/components/Sidebar.drag.browser.test.tsx +++ b/apps/web/src/components/Sidebar.drag.browser.test.tsx @@ -16,6 +16,8 @@ import { type FakeDriver, } from "../../test/fakeEnvironment"; import { act, render } from "../../test/render"; +import { makeThreadFixture } from "../test-fixtures"; +import { SidebarSearchResultRow } from "./sidebar/SidebarThreadRow"; // The socket is the only fake layer; see test/appScenarios.renders.tsx. vi.mock("../../../../packages/client-runtime/src/connection/driver.ts", async (importOriginal) => { @@ -86,3 +88,37 @@ it("keeps the title when dragging a search result from a collapsed folder", asyn ).toBe(second.title); }); }); + +it("keeps focus in the search box when a click picks a search result", async () => { + const onSelect = vi.fn(); + await using screen = await render( + <> + +
    + {}} + onSelect={onSelect} + onFileDropThreads={() => {}} + /> +
+ , + ); + const search = screen.getByRole("textbox", { name: "Search threads" }); + await userEvent.click(search); + await userEvent.click(screen.getByRole("option")); + expect(onSelect).toHaveBeenCalledOnce(); + // Picking clears the results; a focused result would drop focus, and its recovery scrolls the list. + await expect.element(search).toHaveFocus(); +}); diff --git a/apps/web/src/components/chat/QueuedRunsControl.browser.test.tsx b/apps/web/src/components/chat/QueuedRunsControl.browser.test.tsx index c226a2616077..af20840837d4 100644 --- a/apps/web/src/components/chat/QueuedRunsControl.browser.test.tsx +++ b/apps/web/src/components/chat/QueuedRunsControl.browser.test.tsx @@ -1,3 +1,5 @@ +import { RegistryContext } from "@effect/atom-react"; +import type { EnvironmentThreadState } from "@t3tools/client-runtime/state/threads"; import { EnvironmentId, MessageId, ThreadId } from "@t3tools/contracts"; import { serializeAssistantCitation } from "@t3tools/shared/assistantCitations"; import { @@ -5,12 +7,14 @@ import { toComposerContextId, } from "@t3tools/shared/composerContextReferences"; import * as DateTime from "effect/DateTime"; -import type { ComponentProps } from "react"; +import * as Option from "effect/Option"; +import { AsyncResult, type Atom, AtomRegistry } from "effect/reactivity"; +import { Profiler, type ComponentProps } from "react"; import { expect, it, vi } from "vite-plus/test"; import { page } from "vite-plus/test/browser"; const state = vi.hoisted(() => ({ - projection: null as unknown, + threadState: null as unknown as Atom.Writable>, workflow: null as unknown, })); @@ -21,7 +25,6 @@ vi.mock(import("@t3tools/client-runtime/state/thread-workflows"), async (importO vi.mock(import("../../state/entities"), async (importOriginal) => ({ ...(await importOriginal()), - useThreadProjection: (() => state.projection) as never, // The linked environment is connected, but its thread is not in the loaded shell. useQuietThreadShell: ((target: { readonly environmentId: string } | null) => target === null || target.environmentId === "environment:linked" @@ -42,6 +45,25 @@ vi.mock(import("../../state/query"), async (importOriginal) => ({ })) as never, })); +// The thread detail atoms are real, fed by one state atom the test writes. +vi.mock(import("../../state/threads"), async (importOriginal) => { + const { createEnvironmentThreadDetailAtoms, EMPTY_ENVIRONMENT_THREAD_STATE } = + await import("@t3tools/client-runtime/state/threads"); + const { AsyncResult, Atom } = await import("effect/reactivity"); + const Option = await import("effect/Option"); + const { makeThreadProjectionFixture } = await import("../../test-fixtures"); + state.threadState = Atom.make>( + AsyncResult.success({ + ...EMPTY_ENVIRONMENT_THREAD_STATE, + data: Option.some(makeThreadProjectionFixture()), + }), + ).pipe(Atom.keepAlive); + return { + ...(await importOriginal()), + environmentThreadDetails: createEnvironmentThreadDetailAtoms(() => state.threadState), + }; +}); + vi.mock(import("../../state/use-atom-command"), async (importOriginal) => ({ ...(await importOriginal()), useAtomCommand: (() => async () => undefined) as never, @@ -54,11 +76,13 @@ vi.mock(import("../../assets/assetUrls"), async (importOriginal) => ({ })); import "~/index.css"; -import { render as renderUi } from "../../../test/render"; +import { EMPTY_ENVIRONMENT_THREAD_STATE } from "@t3tools/client-runtime/state/threads"; + +import { act, render as renderUi } from "../../../test/render"; +import { makeStreamingTimelineFixture, makeThreadProjectionFixture } from "../../test-fixtures"; import { QueuedRunsControl } from "./QueuedRunsControl"; function setup() { - state.projection = null; state.workflow = null; } @@ -98,7 +122,6 @@ it("previews assistant quotes as readable text and preserves the citation when e contextId: toComposerContextId("header"), label: "header.png", }); - state.projection = { projection: { messages: [] } }; state.workflow = { activeRun: { id: "run:active" }, canPromoteToSteer: true, @@ -147,7 +170,6 @@ it("shows who sent a queued message and folds automation and agent prompts", asy createdAt: DateTime.makeUnsafe("2026-10-27T12:00:00.000Z"), ...fields, }); - state.projection = { projection: { messages: [] } }; state.workflow = { activeRun: { id: "run:active" }, canPromoteToSteer: true, @@ -223,7 +245,6 @@ it("shows who sent a queued message and folds automation and agent prompts", asy it("stays inside the timeline scroller instead of growing the chat column", async () => { setup(); - state.projection = { projection: { messages: [] } }; state.workflow = { activeRun: { id: "run:active" }, canPromoteToSteer: true, @@ -250,3 +271,63 @@ it("stays inside the timeline scroller instead of growing the chat column", asyn const column = screen.getByTestId("column").element(); expect(column.scrollHeight).toBe(column.clientHeight); }); + +it("does not re-render while assistant text streams into the thread", async () => { + setup(); + state.workflow = { + activeRun: { id: "run:active" }, + canPromoteToSteer: true, + canReorder: true, + queuedRuns: [ + { + run: { id: "run:queued", userMessageId: "message:queued" }, + text: "Run the tests next", + attachments: [], + }, + ], + }; + const base = makeThreadProjectionFixture(); + // Each delta replaces only the streaming reply; every other item keeps its identity. + const timeline = makeStreamingTimelineFixture().visibleTurnItems; + const thread = (text: string, done = false) => { + const items = timeline.map((entry) => + entry.item.type === "assistant_message" && entry.item.streaming + ? { + ...entry, + item: done + ? { ...entry.item, text, streaming: false, status: "completed" as const } + : { ...entry.item, text }, + } + : entry, + ); + return AsyncResult.success({ + ...EMPTY_ENVIRONMENT_THREAD_STATE, + data: Option.some({ + ...base, + turnItems: items.map(({ item }) => item), + visibleTurnItems: items, + }), + }); + }; + const registry = AtomRegistry.make(); + await using stack = new AsyncDisposableStack(); + stack.defer(() => registry.dispose()); + registry.set(state.threadState, thread("Hel")); + let renders = 0; + await using _screen = await renderUi( + + (renders += 1)}> + + + , + ); + await expect.element(page.getByText("Run the tests next", { exact: true })).toBeVisible(); + const settled = renders; + + await act(() => registry.set(state.threadState, thread("Hello"))); + await act(() => registry.set(state.threadState, thread("Hello there"))); + expect(renders).toBe(settled); + + await act(() => registry.set(state.threadState, thread("Hello there", true))); + expect(renders).toBeGreaterThan(settled); +}); diff --git a/apps/web/src/components/chat/QueuedRunsControl.tsx b/apps/web/src/components/chat/QueuedRunsControl.tsx index 29a99d385573..68fcd504159c 100644 --- a/apps/web/src/components/chat/QueuedRunsControl.tsx +++ b/apps/web/src/components/chat/QueuedRunsControl.tsx @@ -28,7 +28,7 @@ import { useId, useImperativeHandle, useRef, useState, type ReactNode, type Ref import { useAssetUrls } from "../../assets/assetUrls"; import { threadEnvironment } from "../../state/threads"; -import { useQuietThreadShell, useThreadProjection } from "../../state/entities"; +import { useQuietThreadProjection, useQuietThreadShell } from "../../state/entities"; import { useEnvironment } from "../../state/environments"; import { useAtomCommand } from "../../state/use-atom-command"; import { isImageAttachment, type ChatMessage } from "../../types"; @@ -94,7 +94,7 @@ export function QueuedRunsControl({ readonly timestampFormat: TimestampFormat; readonly onOpenThread: (threadId: ThreadId, environmentId?: EnvironmentId) => void; }) { - const projection = useThreadProjection( + const projection = useQuietThreadProjection( scopeThreadRef(props.environmentId, props.threadId), )?.projection; const promote = useAtomCommand(threadEnvironment.promoteQueuedRun); diff --git a/apps/web/src/components/chat/ThreadRelationshipsControl.browser.test.tsx b/apps/web/src/components/chat/ThreadRelationshipsControl.browser.test.tsx index 9679f42569c6..cafc2ac171ce 100644 --- a/apps/web/src/components/chat/ThreadRelationshipsControl.browser.test.tsx +++ b/apps/web/src/components/chat/ThreadRelationshipsControl.browser.test.tsx @@ -30,7 +30,7 @@ vi.mock(import("@effect/atom-react"), async (original) => ({ vi.mock(import("../../state/entities"), () => ({ useQuietThreadShell: (() => null) as never, - useThreadProjection: ((ref: unknown) => + useQuietThreadProjection: ((ref: unknown) => ref === null ? null : { projection: state.projection }) as never, useThreadRelationshipShells: (() => state.projection === null diff --git a/apps/web/src/components/chat/ThreadRelationshipsControl.tsx b/apps/web/src/components/chat/ThreadRelationshipsControl.tsx index 49c69ff4fade..48213f9618c6 100644 --- a/apps/web/src/components/chat/ThreadRelationshipsControl.tsx +++ b/apps/web/src/components/chat/ThreadRelationshipsControl.tsx @@ -62,8 +62,8 @@ import { useArchivedThreadSnapshots } from "../../lib/archivedThreadsState"; import { buildThreadRouteParams } from "../../threadRoutes"; import { useProjects, + useQuietThreadProjection, useQuietThreadShell, - useThreadProjection, useThreadRelationshipShells, useHiddenThreadKeys, useAnyEnvironmentHiding, @@ -428,7 +428,7 @@ export function ThreadRelationshipsPanel(props: { readonly showSideChats: boolean; }) { const ref = scopeThreadRef(props.environmentId, props.threadId); - const projection = useThreadProjection(ref)?.projection ?? null; + const projection = useQuietThreadProjection(ref)?.projection ?? null; const providersByEnvironment = useEnvironmentProviders(); const settings = useAtomValue(serverEnvironment.settingsValueAtom(props.environmentId)); const relationshipShells = useThreadRelationshipShells(ref); @@ -455,7 +455,7 @@ export function ThreadRelationshipsPanel(props: { parentShell && isProviderNativeSubagentThread(parentShell) ? scopeThreadRef(props.environmentId, parentShell.id) : null; - const parentProjection = useThreadProjection(parentRef)?.projection; + const parentProjection = useQuietThreadProjection(parentRef)?.projection; const navigate = useNavigate(); const mergeBack = useAtomCommand(threadEnvironment.mergeBack); const stopSession = useAtomCommand(threadEnvironment.stopSession); diff --git a/apps/web/src/components/chat/TimelineAssistantRows.tsx b/apps/web/src/components/chat/TimelineAssistantRows.tsx index 6af0004ba24d..d508e0071dad 100644 --- a/apps/web/src/components/chat/TimelineAssistantRows.tsx +++ b/apps/web/src/components/chat/TimelineAssistantRows.tsx @@ -19,7 +19,7 @@ import { readResponseSubagents, useAssistantMessageText, useProject, - useThreadShell, + useQuietThreadShell, } from "../../state/entities"; import { MessageCopyButton } from "./MessageCopyButton"; import { AssistantCitationSource } from "./AssistantCitationSource"; @@ -444,7 +444,7 @@ function AssistantChangedFilesSectionInner({ const setExpanded = useUiStateStore((store) => store.setThreadChangedFilesExpanded); const allDirectoriesExpanded = persistedExpanded ?? false; - const thread = useThreadShell(ctx.threadRef); + const thread = useQuietThreadShell(ctx.threadRef); const activeProject = useProject( thread && thread.projectId ? { environmentId: thread.environmentId, projectId: thread.projectId } diff --git a/apps/web/src/components/chat/useComposerStash.browser.test.tsx b/apps/web/src/components/chat/useComposerStash.browser.test.tsx new file mode 100644 index 000000000000..63b5d4615196 --- /dev/null +++ b/apps/web/src/components/chat/useComposerStash.browser.test.tsx @@ -0,0 +1,89 @@ +import { EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { expect, it, vi } from "vite-plus/test"; + +import { act, render } from "../../../test/render"; +import { DraftId, useComposerDraftStore } from "../../composerDraftStore"; +import { removeLocalStorageItem } from "../../hooks/useLocalStorage"; +import { isMacPlatform } from "../../lib/utils"; +import { + PROMPT_STASH_STORAGE_KEY, + usePromptStashStore, + writePromptStashStorageForTest, +} from "../../promptStashStore"; +import { + addPreparingComposerImages, + countPreparingComposerImages, + removePreparingComposerImages, +} from "./preparingComposerImages"; +import { useComposerStash } from "./useComposerStash"; + +type StashInput = Parameters[0]; + +function Harness({ input }: { input: StashInput }) { + useComposerStash(input); + return null; +} + +it("keeps the draft when stashing while a pasted image is still compressing", async () => { + const target = DraftId.make("stash-preparing-image"); + using _draft = { + [Symbol.dispose]: () => useComposerDraftStore.getState().clearComposerContent(target), + }; + using _stash = { + [Symbol.dispose]: () => { + writePromptStashStorageForTest(""); + removeLocalStorageItem(PROMPT_STASH_STORAGE_KEY); + }, + }; + addPreparingComposerImages(target, [ + { + id: "pasted", + name: "shot.png", + file: new File(["png"], "shot.png", { type: "image/png" }), + hidden: false, + }, + ]); + using _preparing = { + [Symbol.dispose]: () => removePreparingComposerImages(target, new Set(["pasted"])), + }; + const environmentId = EnvironmentId.make("stash-env"); + const promptRef = { current: "Describe this screenshot" }; + const input = { + environmentId, + composerDraftTarget: target, + promptRef, + readAttachmentDraft: () => ({ images: [], files: [] }), + readTerminalContexts: () => [], + routeThreadRef: { environmentId, threadId: ThreadId.make("stash-thread") }, + projectSelectionRequired: false, + pendingUserInputs: [], + activePendingProgress: null, + prompt: promptRef.current, + composerMenuOpen: false, + isComposerCollapsedMobile: false, + composerPreviewAnnotations: [], + composerReviewComments: [], + composerThreadContexts: [], + attachmentTargetKey: target, + isComposerDraftTargetOpen: () => true, + composerEditorRef: { current: null }, + isRevertingCheckpoint: () => false, + pendingThreadImportsRef: { current: new Map() }, + setComposerCursor: vi.fn(), + setComposerTrigger: vi.fn(), + importContextRecords: () => new Map(), + expandMobileComposer: vi.fn(), + } satisfies StashInput; + await using _screen = await render(); + + const mod = isMacPlatform(navigator.platform) ? { metaKey: true } : { ctrlKey: true }; + await act(() => { + window.dispatchEvent( + new KeyboardEvent("keydown", { key: "s", ...mod, bubbles: true, cancelable: true }), + ); + }); + + expect(usePromptStashStore.getState().entries).toEqual([]); + expect(promptRef.current).toBe("Describe this screenshot"); + expect(countPreparingComposerImages(target)).toBe(1); +}); diff --git a/apps/web/src/components/chat/useComposerStash.ts b/apps/web/src/components/chat/useComposerStash.ts index 7d389d0ad9e6..1a765515775d 100644 --- a/apps/web/src/components/chat/useComposerStash.ts +++ b/apps/web/src/components/chat/useComposerStash.ts @@ -33,6 +33,7 @@ import { compressImageForStash } from "../../lib/imageCompression"; import { formatInlineContextReference } from "~/lib/composerContextReferences"; import { isCommandPaletteOpen } from "../../commandPaletteBus"; import { pendingDraftWork } from "./pendingDraftWork"; +import { countPreparingComposerImages } from "./preparingComposerImages"; import { isResponseContextRecord, previewAnnotationContextRecord, @@ -502,6 +503,16 @@ export function useComposerStash(input: { } return; } + // The snapshot only holds compressed images; a still-compressing one would land in the + // emptied composer instead of travelling with this entry. + if (countPreparingComposerImages(attachmentTargetKey) > 0) { + toastManager.add({ + type: "info", + title: "Still compressing a pasted image.", + description: "Stash again once it finishes compressing.", + }); + return; + } const stashedFiles: PersistedComposerFileAttachment[] = []; for (const file of files) { if (composerFileNeedsReattach(file)) { diff --git a/apps/web/src/components/files/FilePreviewPanel.test.ts b/apps/web/src/components/files/FilePreviewPanel.test.ts index d9361ba21244..cd14df70f3c4 100644 --- a/apps/web/src/components/files/FilePreviewPanel.test.ts +++ b/apps/web/src/components/files/FilePreviewPanel.test.ts @@ -407,6 +407,29 @@ Third}`; }, ); + const approved = "Approved now"; + const approvedRow = `${"

Approved now

".repeat(2)}`; + it.each([ + [ + "expands", + `${`${approved}`.repeat(2)}${approved}`, + approvedRow.repeat(3), + ], + [ + "bounds", + 'x', + `${"

x

".repeat(50)}`.repeat(500), + ], + ])("%s repeated ODT table rows and cells", async (_, rows, html) => { + const archive = new JSZip().file( + "content.xml", + `${rows}`, + ); + const result = await openDocument(archive, "odt"); + if (result.type !== "document") throw new Error("Expected document"); + expect(result.pages[0]!.html).toBe(`${html}
`); + }); + it.each(["odt", "odp"] as const)("reads %s text and rejects XML entities", async (kind) => { const archive = new JSZip().file( "content.xml", diff --git a/apps/web/src/components/files/extendedDocumentPreview.ts b/apps/web/src/components/files/extendedDocumentPreview.ts index 6b840a00543a..16bc937b0cf3 100644 --- a/apps/web/src/components/files/extendedDocumentPreview.ts +++ b/apps/web/src/components/files/extendedDocumentPreview.ts @@ -9,8 +9,8 @@ import type JSZip from "jszip"; import { type OfficeWorkerResult, type NotebookCell } from "./officePreviewProtocol"; -const ODS_PREVIEW_MAX_ROWS = 500; -const ODS_PREVIEW_MAX_COLUMNS = 50; +const TABLE_PREVIEW_MAX_ROWS = 500; +const TABLE_PREVIEW_MAX_COLUMNS = 50; // `text` and `attrs` hold decoded values: escaped text is decoded once, CDATA stays literal. type XmlNode = { name: string; attrs: Record; children: XmlNode[]; text: string }; @@ -295,13 +295,20 @@ async function odfHtml(node: XmlNode, archive: JSZip): Promise { }; const tag = tags[node.name]; if (!tag) return children; - reserveMarkup(tag.length * 2 + 5); - return `<${tag}>${children}`; + const count = + tag === "tr" + ? repeat(node.attrs["table:number-rows-repeated"], TABLE_PREVIEW_MAX_ROWS) + : tag === "td" + ? repeat(node.attrs["table:number-columns-repeated"], TABLE_PREVIEW_MAX_COLUMNS) + : 1; + const markup = `<${tag}>${children}`; + reserveMarkup(count * markup.length - children.length); + return markup.repeat(count); } function repeat(value: string | undefined, max: number) { const count = Number(value ?? 1); if (!Number.isSafeInteger(count) || count < 1) - throw new Error("This spreadsheet contains invalid repeated cells."); + throw new Error("This document contains invalid repeated content."); return Math.min(count, max); } export async function openDocument( @@ -337,29 +344,29 @@ export async function openDocument( ""; const count = repeat( cell.attrs["table:number-columns-repeated"], - ODS_PREVIEW_MAX_COLUMNS + 1, + TABLE_PREVIEW_MAX_COLUMNS + 1, ); - if (value && cells.length + count > ODS_PREVIEW_MAX_COLUMNS) truncated = true; - for (let i = 0; i < count && cells.length < ODS_PREVIEW_MAX_COLUMNS; i++) { + if (value && cells.length + count > TABLE_PREVIEW_MAX_COLUMNS) truncated = true; + for (let i = 0; i < count && cells.length < TABLE_PREVIEW_MAX_COLUMNS; i++) { if (value.length > 10_000) throw new Error("A spreadsheet cell is too large to preview."); cells.push(value); } } while (cells.at(-1) === "") cells.pop(); - const count = repeat(row.attrs["table:number-rows-repeated"], ODS_PREVIEW_MAX_ROWS + 1); + const count = repeat(row.attrs["table:number-rows-repeated"], TABLE_PREVIEW_MAX_ROWS + 1); if (!cells.length) { - pendingEmptyRows = Math.min(ODS_PREVIEW_MAX_ROWS + 1, pendingEmptyRows + count); + pendingEmptyRows = Math.min(TABLE_PREVIEW_MAX_ROWS + 1, pendingEmptyRows + count); continue; } - while (pendingEmptyRows > 0 && rows.length < ODS_PREVIEW_MAX_ROWS) { + while (pendingEmptyRows > 0 && rows.length < TABLE_PREVIEW_MAX_ROWS) { rows.push([]); pendingEmptyRows--; } pendingEmptyRows = 0; - if (rows.length + count > ODS_PREVIEW_MAX_ROWS) truncated = true; - if (rows.length >= ODS_PREVIEW_MAX_ROWS) break; - for (let i = 0; i < count && rows.length < ODS_PREVIEW_MAX_ROWS; i++) { + if (rows.length + count > TABLE_PREVIEW_MAX_ROWS) truncated = true; + if (rows.length >= TABLE_PREVIEW_MAX_ROWS) break; + for (let i = 0; i < count && rows.length < TABLE_PREVIEW_MAX_ROWS; i++) { cellBytes += cells.reduce((size, cell) => size + cell.length, 0); if (cellBytes > 8 * 1024 * 1024) throw new Error("This spreadsheet has too much text to preview."); diff --git a/apps/web/src/components/files/projectFilesQueryState.ts b/apps/web/src/components/files/projectFilesQueryState.ts index 4d7e9ae90d6b..8b991d069d22 100644 --- a/apps/web/src/components/files/projectFilesQueryState.ts +++ b/apps/web/src/components/files/projectFilesQueryState.ts @@ -15,7 +15,6 @@ import { appAtomRegistry } from "~/rpc/atomRegistry"; import { useFilesystemReadAccess } from "~/state/filesystem"; import { projectEnvironment } from "~/state/projects"; import { useProjectPathSearch } from "~/state/queries"; -import { executeAtomQuery } from "@t3tools/client-runtime/state/runtime"; import { revisionedQuery } from "~/state/revisionedQuery"; const EMPTY_PROJECT_FILE_PATH = ""; @@ -92,7 +91,7 @@ export function setProjectFileQueryData( unsavedFileMounts.set(atom, appAtomRegistry.mount(atom)); } appAtomRegistry.set(atom, { - confirmedAgainst: undefined, + saved: false, data: { relativePath, contents, @@ -128,7 +127,7 @@ export function getUnsavedProjectFileQueryData( relativePath: string, ): ProjectReadFileResult | null { const optimistic = appAtomRegistry.get(optimisticFileAtom(environmentId, cwd, relativePath)); - return optimistic?.confirmedAgainst === undefined ? (optimistic?.data ?? null) : null; + return !optimistic?.saved ? (optimistic?.data ?? null) : null; } export function confirmProjectFileQueryData( @@ -142,21 +141,13 @@ export function confirmProjectFileQueryData( if (optimisticFile?.data.contents !== contents) return false; const queryAtom = getProjectFileQueryAtom(environmentId, cwd, relativePath); - const confirmed = { + appAtomRegistry.set(atom, { ...optimisticFile, - confirmedAgainst: appAtomRegistry.get(queryAtom), - }; - appAtomRegistry.set(atom, confirmed); + saved: true, + confirmingQuery: queryAtom, + }); releaseUnsavedFile(atom); appAtomRegistry.refresh(queryAtom); - void executeAtomQuery(appAtomRegistry, queryAtom, { - reportDefect: false, - reportFailure: false, - }).then((result) => { - if (result._tag === "Success" && appAtomRegistry.get(atom) === confirmed) { - appAtomRegistry.set(atom, null); - } - }); return true; } diff --git a/apps/web/src/components/settings/ConnectionsSettings.logic.test.ts b/apps/web/src/components/settings/ConnectionsSettings.logic.test.ts index 4cd58ef38af6..a664dc99a8f4 100644 --- a/apps/web/src/components/settings/ConnectionsSettings.logic.test.ts +++ b/apps/web/src/components/settings/ConnectionsSettings.logic.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it, vi } from "vite-plus/test"; import { applyWslEnableSelection, isServerNetworkAccessible, + parseManualDesktopSshTarget, resolveAccessEnvironmentId, resolveTailscaleServeBaseUrl, selectQrEndpointOption, @@ -44,6 +45,14 @@ describe("togglePairingScopeSelection", () => { }); }); +describe("parseManualDesktopSshTarget", () => { + it.each(["22.5", "1e3", "2222garbage"])("rejects explicit port %j", (port) => { + expect(() => parseManualDesktopSshTarget({ host: "devbox", username: "", port })).toThrow( + "SSH port must be between 1 and 65535.", + ); + }); +}); + describe("resolveTailscaleServeBaseUrl", () => { it.each([ ["https://host.tail.ts.net:441/", 443, "https://host.tail.ts.net"], diff --git a/apps/web/src/components/settings/ConnectionsSettings.logic.ts b/apps/web/src/components/settings/ConnectionsSettings.logic.ts index 28d3b8f2df12..f78ef2934719 100644 --- a/apps/web/src/components/settings/ConnectionsSettings.logic.ts +++ b/apps/web/src/components/settings/ConnectionsSettings.logic.ts @@ -214,7 +214,7 @@ export function parseManualDesktopSshTarget(input: { const rawPort = input.port.trim(); if (rawPort.length > 0) { - port = Number.parseInt(rawPort, 10); + port = /^\d+$/u.test(rawPort) ? Number.parseInt(rawPort, 10) : Number.NaN; } if (hostname.length === 0) { diff --git a/apps/web/src/components/sidebar/SidebarDraftBlock.tsx b/apps/web/src/components/sidebar/SidebarDraftBlock.tsx index c4868c4776ea..7ac90c0d4821 100644 --- a/apps/web/src/components/sidebar/SidebarDraftBlock.tsx +++ b/apps/web/src/components/sidebar/SidebarDraftBlock.tsx @@ -99,7 +99,6 @@ const SidebarDraftRow = memo(function SidebarDraftRow(props: { onContextMenu={handleContextMenu} onKeyDown={handleKeyDown} > - {preview}
@@ -127,9 +126,7 @@ const SidebarDraftRow = memo(function SidebarDraftRow(props: {
-
- {preview} -
+
{preview}
diff --git a/apps/web/src/components/sidebar/SidebarThreadRow.tsx b/apps/web/src/components/sidebar/SidebarThreadRow.tsx index 216f6635f708..db896481cf85 100644 --- a/apps/web/src/components/sidebar/SidebarThreadRow.tsx +++ b/apps/web/src/components/sidebar/SidebarThreadRow.tsx @@ -1042,7 +1042,6 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP /> ) : ( ); - const accessibleTitle = isRenaming ? null : {thread.title}; const prBadgeShape = supportsMultiplePullRequests ? resolveThreadPullRequestBadge(thread.pullRequests) @@ -1262,7 +1260,6 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP > {/* `contents` keeps one-line rows a single flex row. */}
- {accessibleTitle} {inFolder ? null : ( @@ -1449,7 +1448,6 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP /> } > - {accessibleTitle}
{ + dragListeners?.onPointerDown?.(event); + event.preventDefault(); + }} onClick={props.onSelect} className={cn( "group/sidebar-row group/v2-row flex min-h-9 w-full cursor-pointer items-center gap-2.5 rounded-md px-2.5 py-1 text-left text-sm outline-none", diff --git a/apps/web/src/components/sidebar/updateEverything.logic.test.ts b/apps/web/src/components/sidebar/updateEverything.logic.test.ts index 58328dbc1eee..12ee88b03bdc 100644 --- a/apps/web/src/components/sidebar/updateEverything.logic.test.ts +++ b/apps/web/src/components/sidebar/updateEverything.logic.test.ts @@ -331,6 +331,50 @@ describe("runUpdateEverything", () => { expect(calls).toEqual(["providers", "servers"]); expect(result.localReady).toBe(false); }); + + it("stages servers while providers install and updates each once its own stage settles", async () => { + const calls: string[] = []; + const [slow, fast] = [EnvironmentId.make("slow"), EnvironmentId.make("fast")]; + let finishProviders = () => {}; + let failSlowStage = () => {}; + const providers = new Promise((resolve) => (finishProviders = resolve)); + const run = runUpdateEverything({ + updateProviders: () => { + calls.push("providers"); + return providers.then(() => { + calls.push("providers done"); + }); + }, + stageServers: () => { + calls.push("stage"); + return new Map([ + [slow, new Promise((_, reject) => (failSlowStage = () => reject(new Error("failed"))))], + [fast, Promise.resolve()], + ]); + }, + updateServers: (staged) => { + calls.push("servers"); + return Promise.all( + [slow, fast].map(async (id) => { + await staged.get(id); + calls.push(`server ${id}`); + if (id === fast) failSlowStage(); + }), + ); + }, + }); + finishProviders(); + await run; + + expect(calls).toEqual([ + "stage", + "providers", + "providers done", + "servers", + "server fast", + "server slow", + ]); + }); }); describe("describeUpdateEverything", () => { diff --git a/apps/web/src/components/sidebar/updateEverything.logic.ts b/apps/web/src/components/sidebar/updateEverything.logic.ts index cbbd3baa7c77..bc26ca9b4608 100644 --- a/apps/web/src/components/sidebar/updateEverything.logic.ts +++ b/apps/web/src/components/sidebar/updateEverything.logic.ts @@ -348,11 +348,15 @@ function trackSwitchMove( export interface UpdateEverythingSteps { readonly downloadLocal?: () => Promise; readonly updateProviders: () => Promise; - readonly updateServers: () => Promise; + readonly stageServers?: () => ReadonlyMap>; + readonly updateServers: ( + staged: ReadonlyMap>, + ) => Promise; } -/** Finish provider installs before server restarts. Install this app last when localReady; failed - * downloads skip only that install. */ +/** Finish provider installs before server restarts; servers stage meanwhile and each server's + * update waits only on its own stage. Install this app last when localReady; failed downloads skip + * only that install. */ export async function runUpdateEverything( steps: UpdateEverythingSteps, ): Promise<{ @@ -360,11 +364,14 @@ export async function runUpdateEverything( readonly providers: Providers; readonly servers: Servers; }> { + const staged = new Map( + Array.from(steps.stageServers?.() ?? [], ([id, stage]) => [id, stage.catch(() => undefined)]), + ); const [localReady, remote] = await Promise.all([ steps.downloadLocal?.() ?? Promise.resolve(true), steps .updateProviders() - .then(async (providers) => ({ providers, servers: await steps.updateServers() })), + .then(async (providers) => ({ providers, servers: await steps.updateServers(staged) })), ]); return { localReady, ...remote }; } diff --git a/apps/web/src/components/sidebar/useUpdateEverything.ts b/apps/web/src/components/sidebar/useUpdateEverything.ts index 05127d988576..ed02493197ce 100644 --- a/apps/web/src/components/sidebar/useUpdateEverything.ts +++ b/apps/web/src/components/sidebar/useUpdateEverything.ts @@ -6,8 +6,9 @@ import { isDesktopLocalConnectionTarget } from "~/connection/desktopLocal"; import { isElectron } from "~/env"; import { useEnvironments } from "~/state/environments"; import { environmentIdsKey } from "~/state/environmentAtomList"; -import { environmentUpdateStatesAtom } from "~/state/server"; +import { environmentUpdateStatesAtom, serverEnvironment } from "~/state/server"; import { environmentSessionStatesAtom } from "~/state/session"; +import { useAtomCommand } from "~/state/use-atom-command"; import { getProviderUpdateRunOutcome } from "../ProviderUpdateLaunchNotification.logic"; import { useProviderUpdates } from "../ProviderUpdatesAction"; import { canUpdateServer, type ServerUpdateOutcome, useServerUpdate } from "../ServerUpdateAction"; @@ -76,6 +77,9 @@ export function useUpdateEverything( // A track switch moves app and servers only; provider updates stay a separate choice. const providers = channelSwitch ? { machines: [], updateAll: async () => [] } : providerUpdates; const updateServer = useServerUpdate(); + const stageServerUpdate = useAtomCommand(serverEnvironment.stageServerUpdate, { + reportFailure: false, + }); const view = describeUpdateEverything({ localVersion: localUpdateVersion, @@ -181,8 +185,29 @@ export function useUpdateEverything( ); return outcomes; }, - updateServers: async () => { + ...(providers.machines.length > 0 + ? { + stageServers: () => + new Map( + servers + .filter( + (server) => + machines.find((machine) => machine.environmentId === server.environmentId) + ?.serverConfig?.environment.capabilities.serverUpdateStaging === true, + ) + .map((server) => [ + server.environmentId, + stageServerUpdate({ + environmentId: server.environmentId, + input: { targetVersion: server.targetVersion, stageOnly: true }, + }), + ]), + ), + } + : {}), + updateServers: async (staged) => { const runs = servers.map(async (server) => { + await staged.get(server.environmentId); const serverOutcome = await updateServer(server, { quiet: true }); settleUpdateRunMachine( server.environmentId, diff --git a/apps/web/src/keybindings.ts b/apps/web/src/keybindings.ts index 849183b7a1a6..75f617d3cca8 100644 --- a/apps/web/src/keybindings.ts +++ b/apps/web/src/keybindings.ts @@ -222,6 +222,8 @@ export function formatShortcutKeyLabel(key: string): string { if (key === "arrowdown") return "Down"; if (key === "arrowleft") return "Left"; if (key === "arrowright") return "Right"; + if (key === "pageup") return "PageUp"; + if (key === "pagedown") return "PageDown"; return key.slice(0, 1).toUpperCase() + key.slice(1); } diff --git a/apps/web/src/lib/snapShotShortcut.test.ts b/apps/web/src/lib/snapShotShortcut.test.ts index f8bd382fec40..c59fdf515b67 100644 --- a/apps/web/src/lib/snapShotShortcut.test.ts +++ b/apps/web/src/lib/snapShotShortcut.test.ts @@ -12,6 +12,8 @@ describe("desktop-approved window capture shortcut labels", () => { ["space", ["Ctrl", "Space"]], ["plus", ["Ctrl", "+"]], ["Ctrl++", ["Ctrl", "+"]], + ["Page_Up", ["Ctrl", "PageUp"]], + ["Page_Down", ["⇧", "PageDown"]], ])("formats %s using the shared keycap labels", (label, expected) => { const shortcut = parseDesktopSnapShotShortcut(label); expect(shortcut).not.toBeNull(); diff --git a/apps/web/src/state/entities.ts b/apps/web/src/state/entities.ts index f0f7e33be50c..0cb7bbc60093 100644 --- a/apps/web/src/state/entities.ts +++ b/apps/web/src/state/entities.ts @@ -5,6 +5,7 @@ import type { EnvironmentThread, EnvironmentThreadShell, EnvironmentThreadStructure, + ThreadStructure, ThreadStructureVisibleTurnItem, } from "@t3tools/client-runtime/state/shell"; import { @@ -22,7 +23,6 @@ import type { EnvironmentId, OrchestrationV2ProjectedTurnItem, OrchestrationV2RemoteTaskChild, - OrchestrationV2ThreadProjection, OrchestrationV2ThreadShell, NodeId, ThreadId, @@ -396,10 +396,7 @@ function shellParentThreadId( /** Retains unchanged reachable shells so unrelated streams do not re-render the panel. */ export function selectThreadRelationshipShells( threadId: ThreadId, - projection: Pick< - OrchestrationV2ThreadProjection, - "thread" | "subagents" | "contextTransfers" - > | null, + projection: Pick | null, threads: ReadonlyArray, ): ReadonlyArray { const named = new Set([threadId]); @@ -431,8 +428,16 @@ const relationshipShellsAtomFamily = Atom.family((key: string) => { return Atom.make((get) => { const next = selectThreadRelationshipShells( ref.threadId, - get(environmentThreadDetails.threadAtom(ref))?.projection ?? null, + get(environmentThreadDetails.quietThreadAtom(ref))?.projection ?? null, get(environmentThreadShells.environmentThreadsAtom(ref.environmentId)), + ).map( + (thread) => + get( + environmentThreadShells.quietThreadShellAtom({ + environmentId: ref.environmentId, + threadId: thread.id, + }), + )?.source ?? thread, ); if (arrayElementsEqual(previous, next)) return previous; previous = next; diff --git a/docs/internals/server-updates.md b/docs/internals/server-updates.md index 10b196a72adf..10df2ef464f8 100644 --- a/docs/internals/server-updates.md +++ b/docs/internals/server-updates.md @@ -27,6 +27,11 @@ Protocol 4 changed the launcher alone, so its runtime still runs under protocol The server names its launcher's protocol to preflight, not its own, since the two differ after the first such update. +A client can stage the target early with `stageOnly`, which installs and preflights the +runtime and restarts nothing. "Update everything" does this while provider installs run, +because a restart would kill them. A server without the `serverUpdateStaging` capability +ignores the flag and runs the whole update, so send it only to a server that advertises it. + After a commit or rollback the launcher points the unit at the selected runtime and reloads the manager, so the next service start runs that version's launcher; the protocol check is what makes that safe. Each start after a finished update retries this, because diff --git a/packages/client-runtime/package.json b/packages/client-runtime/package.json index b2077de10b29..c81b9f351b87 100644 --- a/packages/client-runtime/package.json +++ b/packages/client-runtime/package.json @@ -594,11 +594,15 @@ "@t3tools/source-control-definitions": "workspace:*", "effect": "catalog:", "jszip": "3.10.1", + "remark-gfm": "catalog:", + "remark-parse": "catalog:", "three": "0.180.0", - "unbash": "^5.0.0" + "unbash": "^5.0.0", + "unified": "catalog:" }, "devDependencies": { "@effect/vitest": "catalog:", + "@types/mdast": "^4.0.4", "@types/three": "0.180.0", "vite-plus": "catalog:" } diff --git a/packages/client-runtime/src/pullRequestMarkdown.test.ts b/packages/client-runtime/src/pullRequestMarkdown.test.ts index 13327d8480a5..f616f35eef5f 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.test.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.test.ts @@ -1,3 +1,7 @@ +import type { Root, RootContent } from "mdast"; +import remarkGfm from "remark-gfm"; +import remarkParse from "remark-parse"; +import { unified } from "unified"; import { describe, expect, it } from "vite-plus/test"; import { @@ -156,6 +160,85 @@ describe("pull request body segmentation", () => { }); }); +describe("reference links around lifted media", () => { + const recording = + "https://github.com/user-attachments/assets/2f8c1a90-1b2c-4d5e-8f90-abcdef123456"; + const parser = unified().use(remarkParse).use(remarkGfm); + const prose = (segments: ReadonlyArray): string[] => + segments.flatMap((segment) => + segment.kind === "markdown" + ? [segment.text] + : segment.kind === "table" + ? segment.rows.flat().flatMap((cell) => prose(cell.segments)) + : [], + ); + // Each segment renders as its own document; the first definition of a label wins, as in mdast-util-to-hast. + const referenceLinks = (body: string) => + prose(splitPullRequestBody(body)).flatMap((text) => { + const definitions = new Map(); + const references: Array<{ identifier: string; text: string }> = []; + const visit = (node: Root | RootContent) => { + if (node.type === "definition" && !definitions.has(node.identifier)) { + definitions.set(node.identifier, node.url); + } + if (node.type === "linkReference") { + references.push({ + identifier: node.identifier, + text: node.children.map((child) => ("value" in child ? child.value : "")).join(""), + }); + } + if ("children" in node) node.children.forEach(visit); + }; + visit(parser.parse(text)); + return references.map((reference) => [reference.text, definitions.get(reference.identifier)]); + }); + + it.each([ + [ + "use, then media, then definition", + `See [the docs][guide].\n\n${recording}\n\n[guide]: https://example.com/docs`, + ], + [ + "definition, then media, then use", + `[guide]: https://example.com/docs\n\n${recording}\n\nSee [the docs][guide].`, + ], + ...["\n", "\r\n", "\r"].map((lineEnding) => [ + `a definition wrapped over quoted lines with ${JSON.stringify(lineEnding)}`, + `> [guide]:${lineEnding}> https://example.com/docs${lineEnding}> "The guide"\n\n${recording}\n\nSee [the docs][guide].`, + ]), + [ + "a use inside a media table cell", + `
\n\nSee [the docs][guide].\n\n\n\n${recording}\n\n
\n\n[guide]: https://example.com/docs`, + ], + ])("resolves a reference defined across a video: %s", (_case, body) => { + expect(referenceLinks(body)).toEqual([["the docs", "https://example.com/docs"]]); + }); + + it("keeps the body's first definition of a label, wherever the media falls", () => { + expect( + referenceLinks( + `[guide]: https://example.com/first\n\n${recording}\n\nSee [the docs][guide].\n\n[GUIDE]: https://example.com/second`, + ), + ).toEqual([["the docs", "https://example.com/first"]]); + }); + + it("leaves a definition shown in code as code", () => { + expect( + referenceLinks( + `\`\`\`md\n[guide]: https://example.com/docs\n\`\`\`\n\n${recording}\n\nSee [the docs][guide].`, + ), + ).toEqual([]); + }); + + it("still writes a task tick back to its place in the body", () => { + const body = `[guide]: https://example.com/docs\n\n${recording}\n\n- [ ] Read [the docs][guide]`; + const segment = splitPullRequestBody(body).at(-1)!; + if (segment.kind !== "markdown") throw new Error("expected prose last"); + const marker = segment.text.indexOf("[ ]"); + expect(body.slice(segment.sourceOffset + marker)).toBe("[ ] Read [the docs][guide]"); + }); +}); + describe("media on custom hosts", () => { it.each([ ["https://drop.example.com/capture.PNG?token=abc#preview", "image"], diff --git a/packages/client-runtime/src/pullRequestMarkdown.ts b/packages/client-runtime/src/pullRequestMarkdown.ts index 2ca59143ad29..a508b789045b 100644 --- a/packages/client-runtime/src/pullRequestMarkdown.ts +++ b/packages/client-runtime/src/pullRequestMarkdown.ts @@ -1,3 +1,8 @@ +import type { Root, RootContent } from "mdast"; +import remarkGfm from "remark-gfm"; +import remarkParse from "remark-parse"; +import { unified } from "unified"; + import { sourceControlClients, type ReferenceAutolink } from "./sourceControlClients.ts"; /** `id` is positional on purpose: the same attachment can be embedded twice in one body. */ @@ -6,7 +11,10 @@ export type PullRequestBodySegment = readonly id: string; readonly kind: "markdown"; readonly text: string; - /** Where `text` starts in the body, so an edit made to it can be written back. */ + /** + * `sourceOffset + i` is where `text[i]` sits in the body, so an edit can be written back. + * Holds past the body's reference definitions, which `text` may lead with. + */ readonly sourceOffset: number; } | { @@ -162,8 +170,64 @@ function mediaTableAt( return { end, rows: hasMedia ? rows : null }; } +const definitionParser = unified().use(remarkParse).use(remarkGfm); + +/** + * The first definition of each label in the body, as authored, each joined onto one line so it + * stands outside the quote or list it was written in. + */ +function referenceDefinitions(body: string): string[] { + const definitions = new Map(); + const visit = (node: Root | RootContent, quotes: number) => { + if (node.type === "definition" && !definitions.has(node.identifier)) { + const wrap = new RegExp(`(?:\\r\\n|\\r|\\n)(?:[ \\t]*>){0,${quotes}}[ \\t]*`, "gu"); + const source = body.slice(node.position?.start.offset, node.position?.end.offset); + definitions.set(node.identifier, source.replace(wrap, " ")); + } + if (!("children" in node)) return; + for (const child of node.children) visit(child, quotes + (node.type === "blockquote" ? 1 : 0)); + }; + visit(definitionParser.parse(body), 0); + return [...definitions.values()]; +} + +/** + * Each markdown segment renders as its own document, so it leads with the body's definitions. + * Leading, they win over a later redefinition in the segment, as the first one does in the body. + */ +function withDefinitions( + segment: PullRequestBodySegment, + definitions: string, +): PullRequestBodySegment { + if (segment.kind === "attachment") return segment; + if (segment.kind === "table") { + return { + ...segment, + rows: segment.rows.map((cells) => + cells.map((cell) => ({ + ...cell, + segments: cell.segments.map((inner) => withDefinitions(inner, definitions)), + })), + ), + }; + } + if (!segment.text.includes("[")) return segment; + return { + ...segment, + text: definitions + segment.text, + sourceOffset: segment.sourceOffset - definitions.length, + }; +} + export function splitPullRequestBody(body: string): ReadonlyArray { - return splitBodyAt(body, 0); + const segments = splitBodyAt(body, 0); + // One markdown segment is the whole body, which resolves its own references. + const whole = segments.length === 1 && segments[0]!.kind === "markdown"; + if (whole || !body.includes("]:")) return segments; + const definitions = referenceDefinitions(body); + if (definitions.length === 0) return segments; + const leading = `${definitions.join("\n")}\n\n`; + return segments.map((segment) => withDefinitions(segment, leading)); } /** `baseOffset` is where `body` starts in the whole body, so a table cell's edits write back. */ diff --git a/packages/client-runtime/src/pullRequestQuery.test.ts b/packages/client-runtime/src/pullRequestQuery.test.ts index 7d73813f3733..d9a9fcee6504 100644 --- a/packages/client-runtime/src/pullRequestQuery.test.ts +++ b/packages/client-runtime/src/pullRequestQuery.test.ts @@ -40,6 +40,10 @@ describe("reading qualifiers out of a typed query", () => { const parsed = parsePullRequestQuery("milestone:v2 draft:maybe status: parser"); expect(parsed.filters).toEqual({ labels: [["milestone:v2"]] }); expect(parsed.text).toBe("draft:maybe status: parser"); + expect(parsePullRequestQuery("review:constructor checks:__proto__ Status:toString")).toEqual({ + text: "review:constructor checks:__proto__ Status:toString", + filters: {}, + }); }); it("reads a quoted name holding a comma as the one label it is", () => { diff --git a/packages/client-runtime/src/pullRequestQuery.ts b/packages/client-runtime/src/pullRequestQuery.ts index b80202c94b06..f89abff3a5bd 100644 --- a/packages/client-runtime/src/pullRequestQuery.ts +++ b/packages/client-runtime/src/pullRequestQuery.ts @@ -77,14 +77,18 @@ export function parsePullRequestQuery(raw: string): { draft = (value.toLowerCase() === "true") !== negated ? "only" : "hide"; continue; case "review": { - const decision = negated ? undefined : REVIEW_VALUES[value.toLowerCase()]; + const key = value.toLowerCase(); + const decision = + !negated && Object.hasOwn(REVIEW_VALUES, key) ? REVIEW_VALUES[key] : undefined; if (decision === undefined) break; review = decision; continue; } case "status": case "checks": { - const state = negated ? undefined : CHECKS_VALUES[value.toLowerCase()]; + const key = value.toLowerCase(); + const state = + !negated && Object.hasOwn(CHECKS_VALUES, key) ? CHECKS_VALUES[key] : undefined; if (state === undefined) break; checks = state; continue; diff --git a/packages/client-runtime/src/state/projectCommands.test.ts b/packages/client-runtime/src/state/projectCommands.test.ts index aa67ccd58026..c49e86d08bba 100644 --- a/packages/client-runtime/src/state/projectCommands.test.ts +++ b/packages/client-runtime/src/state/projectCommands.test.ts @@ -3,6 +3,7 @@ import { AuthFilesystemWriteScope, EnvironmentId, ProjectId, + ProjectReadFileError, ProjectReadFileInput, ProjectReadFileResult, ThreadId, @@ -49,7 +50,7 @@ const makeHarness = Effect.fn("TestProjectCommands.makeHarness")(function* () { const metadataRequests: string[][] = []; const file = { byteLength: 5 }; const readResponse = { - wait: Effect.void as Effect.Effect, + wait: Effect.void as Effect.Effect, missing: false, }; const supervisor = EnvironmentSupervisor.EnvironmentSupervisor.of({ @@ -146,6 +147,110 @@ const makeHarness = Effect.fn("TestProjectCommands.makeHarness")(function* () { }); describe("writeFile metadata", () => { + it.effect("retires a saved overlay after an equal read succeeds, preserving newer drafts", () => + Effect.gen(function* () { + const h = yield* makeHarness(); + const target = { environmentId: ENVIRONMENT_ID, cwd: "/repo", relativePath: "foo" }; + const query = h.commands.readFile({ + environmentId: ENVIRONMENT_ID, + input: { cwd: target.cwd, relativePath: target.relativePath }, + }); + h.registry.mount(query); + const read = () => AtomRegistry.getResult(h.registry, query, { suspendOnWaiting: true }); + const original = yield* read(); + const overlay = h.commands.optimisticFile(target); + h.registry.mount(overlay); + const saved = { ...original, contents: "saved" }; + h.registry.set(overlay, { data: saved, saved: true, confirmingQuery: query }); + h.readResponse.wait = Effect.fail( + new ProjectReadFileError({ + cwd: "/repo", + relativePath: "foo", + failure: "operation_failed", + }), + ); + h.registry.refresh(query); + yield* Effect.exit(read()); + expect(h.registry.get(overlay)?.data.contents).toBe("saved"); + + h.readResponse.wait = Effect.void; + h.registry.refresh(query); + const refreshed = yield* read(); + // The production query shares equal file values across this failed confirmation. + expect(refreshed).toBe(original); + expect(h.registry.get(overlay)).toBeNull(); + + h.registry.set(overlay, { data: { ...saved, contents: "newer draft" }, saved: false }); + h.registry.refresh(query); + yield* read(); + expect(h.registry.get(overlay)?.data.contents).toBe("newer draft"); + }), + ); + + it.effect.each([true, false])( + "keeps a saved overlay when an older read uses different allowMissing options (confirmation: %s)", + (allowMissing) => + Effect.gen(function* () { + const h = yield* makeHarness(); + const target = { environmentId: ENVIRONMENT_ID, cwd: "/repo", relativePath: "foo" }; + const query = h.commands.readFile({ + environmentId: ENVIRONMENT_ID, + input: { + cwd: target.cwd, + relativePath: target.relativePath, + ...(allowMissing ? { allowMissing: true } : {}), + }, + }); + const otherQuery = h.commands.readFile({ + environmentId: ENVIRONMENT_ID, + input: { + cwd: target.cwd, + relativePath: target.relativePath, + ...(!allowMissing ? { allowMissing: true } : {}), + }, + }); + const read = (atom: typeof query) => + AtomRegistry.getResult(h.registry, atom, { suspendOnWaiting: true }); + h.registry.mount(query); + const original = yield* read(query); + const overlay = h.commands.optimisticFile(target); + h.registry.mount(overlay); + const started = yield* Deferred.make(); + const release = yield* Deferred.make(); + h.readResponse.wait = Deferred.succeed(started, undefined).pipe( + Effect.andThen(Deferred.await(release)), + ); + h.registry.mount(otherQuery); + const olderRead = yield* read(otherQuery).pipe(Effect.forkChild); + yield* Deferred.await(started); + h.registry.set(overlay, { + data: { ...original, contents: "saved" }, + saved: true, + confirmingQuery: query, + }); + h.readResponse.wait = Effect.fail( + new ProjectReadFileError({ + cwd: target.cwd, + relativePath: target.relativePath, + failure: "operation_failed", + }), + ); + h.registry.refresh(query); + yield* Effect.exit(read(query)); + expect(h.registry.get(overlay)?.data.contents).toBe("saved"); + + yield* Deferred.succeed(release, undefined); + yield* Fiber.join(olderRead); + expect(h.registry.get(overlay)?.data.contents).toBe("saved"); + expect(Option.getOrNull(AsyncResult.value(h.registry.get(query)))).toBe(original); + + h.readResponse.wait = Effect.void; + h.registry.refresh(query); + expect(yield* read(query)).toBe(original); + expect(h.registry.get(overlay)).toBeNull(); + }), + ); + it.effect("preserves optional read results without seeding metadata for an absent file", () => Effect.gen(function* () { const h = yield* makeHarness(); diff --git a/packages/client-runtime/src/state/projectCommands.ts b/packages/client-runtime/src/state/projectCommands.ts index caa706d86d46..119b1de81fcb 100644 --- a/packages/client-runtime/src/state/projectCommands.ts +++ b/packages/client-runtime/src/state/projectCommands.ts @@ -37,10 +37,16 @@ export type { UpdateProjectInput, } from "../operations/commands.ts"; -export interface OptimisticProjectFile { +export type OptimisticProjectFile = { readonly data: ProjectReadFileResult; - readonly confirmedAgainst: object | null | undefined; -} +} & ( + | { readonly saved: false } + | { + readonly saved: true; + /** Only this query's next successful answer can replace the saved contents. */ + readonly confirmingQuery: Atom.Atom; + } +); export interface OptimisticProjectFileTarget { readonly environmentId: EnvironmentId; @@ -86,6 +92,29 @@ export function createProjectEnvironmentAtoms( Atom.withLabel(`environment-data:projects:optimistic-file:${key}`), ), ); + const readFile = createEnvironmentRpcQueryAtomFamily(runtime, { + label: "environment-data:projects:read-file", + tag: WS_METHODS.projectsReadFile, + staleTimeMs: 30_000, + idleTtlMs: 5 * 60_000, + onAnswer: (target, _value, registry): Effect.Effect => + Effect.sync(() => { + const atom = optimisticFileFamily( + optimisticProjectFileKey({ environmentId: target.environmentId, ...target.input }), + ); + const overlay = registry.get(atom); + if (overlay?.saved && overlay.confirmingQuery === readFile(target)) + registry.set(atom, null); + }), + execute: (input) => + fileMetadata.seedAfterRead( + request(WS_METHODS.projectsReadFile, input), + (result, isCurrent) => + result.exists === false + ? Effect.void + : fileMetadata.rememberFile(input.cwd, result, isCurrent), + ), + }); const projectConcurrency = { mode: "serial" as const, key: ({ environmentId, input }: { environmentId: string; input: { projectId: string } }) => @@ -133,20 +162,7 @@ export function createProjectEnvironmentAtoms( tag: WS_METHODS.projectsListIgnoredFiles, staleTimeMs: 15_000, }), - readFile: createEnvironmentRpcQueryAtomFamily(runtime, { - label: "environment-data:projects:read-file", - tag: WS_METHODS.projectsReadFile, - staleTimeMs: 30_000, - idleTtlMs: 5 * 60_000, - execute: (input) => - fileMetadata.seedAfterRead( - request(WS_METHODS.projectsReadFile, input), - (result, isCurrent) => - result.exists === false - ? Effect.void - : fileMetadata.rememberFile(input.cwd, result, isCurrent), - ), - }), + readFile, optimisticFile: (target: OptimisticProjectFileTarget) => optimisticFileFamily(optimisticProjectFileKey(target)), create: createEnvironmentCommand(runtime, { diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index 4db6c8bca70c..23995c4d9933 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1688,6 +1688,10 @@ export function createServerEnvironmentAtoms( tag: WS_METHODS.serverUpdateProvider, }), updateServer, + stageServerUpdate: createEnvironmentRpcCommand(runtime, { + label: "environment-data:server:stage-update", + tag: WS_METHODS.serverUpdateServer, + }), upsertKeybinding: createEnvironmentRpcCommand(runtime, { label: "environment-data:server:upsert-keybinding", tag: WS_METHODS.serverUpsertKeybinding, diff --git a/packages/client-runtime/src/state/threadDetail.test.ts b/packages/client-runtime/src/state/threadDetail.test.ts index 31fb095043ee..e32d70215e20 100644 --- a/packages/client-runtime/src/state/threadDetail.test.ts +++ b/packages/client-runtime/src/state/threadDetail.test.ts @@ -53,9 +53,29 @@ const assistant: OrchestrationV2TurnItem = { streaming: true, }; -function withItem(item: OrchestrationV2TurnItem, updatedAt = v2Now) { +const assistantMessage: OrchestrationV2ConversationMessage = { + createdBy: "agent", + creationSource: "provider", + id: MessageId.make("message-assistant"), + threadId: v2ThreadId, + runId: RunId.make("run-1"), + nodeId: null, + role: "assistant", + text: "Hel", + attachments: [], + streaming: true, + createdAt: v2Now, + updatedAt: v2Now, +}; + +function withItem( + item: OrchestrationV2TurnItem, + updatedAt = v2Now, + message: OrchestrationV2ConversationMessage = assistantMessage, +) { return { ...v2Projection, + messages: [message], turnItems: [item], visibleTurnItems: [ { @@ -74,13 +94,19 @@ describe("sameThreadStructure", () => { const before = withItem(assistant); it("holds through streamed text", () => { - const next = withItem({ ...assistant, text: "Hello", updatedAt: later }, later); + const next = withItem({ ...assistant, text: "Hello", updatedAt: later }, later, { + ...assistantMessage, + text: "Hello", + updatedAt: later, + }); expect(sameThreadStructure(before, next)).toBe(true); }); it("sees the item finish streaming", () => { const next = withItem({ ...assistant, text: "Hello", streaming: false, status: "completed" }); expect(sameThreadStructure(before, next)).toBe(false); + const message = { ...assistantMessage, text: "Hello", streaming: false }; + expect(sameThreadStructure(before, withItem(assistant, v2Now, message))).toBe(false); }); it("sees a new item", () => { diff --git a/packages/client-runtime/src/state/threadDetail.ts b/packages/client-runtime/src/state/threadDetail.ts index 74012f436318..10ba574e0bb0 100644 --- a/packages/client-runtime/src/state/threadDetail.ts +++ b/packages/client-runtime/src/state/threadDetail.ts @@ -1,5 +1,6 @@ import { OrchestrationV2AppThread, + OrchestrationV2ConversationMessage, OrchestrationV2TurnItem, type OrchestrationV2ThreadProjection, type ScopedThreadRef, @@ -159,6 +160,7 @@ function toThreadStructure(projection: OrchestrationV2ThreadProjection): ThreadS const sameTurnItem = Schema.toEquivalence(OrchestrationV2TurnItem); const sameAppThread = Schema.toEquivalence(OrchestrationV2AppThread); +const sameMessage = Schema.toEquivalence(OrchestrationV2ConversationMessage); // Overlays previous's structure on next, so the fields the strip functions // remove are exactly the ones ignored here. @@ -183,6 +185,17 @@ function sameTurnItemStructure(previous: TurnItem, next: TurnItem): boolean { return sameApartFromStripped(sameTurnItem, previous, structure, next); } +// A streaming reply's text also lands on its message; readers of that text read the live thread. +function sameMessageStructure( + previous: OrchestrationV2ConversationMessage, + next: OrchestrationV2ConversationMessage, +): boolean { + if (previous === next) return true; + if (!previous.streaming || !next.streaming || previous.id !== next.id) return false; + const { text: _text, updatedAt: _updatedAt, ...structure } = previous; + return sameApartFromStripped(sameMessage, previous, structure, next); +} + function sameProjectionArray( previous: ReadonlyArray, next: ReadonlyArray, @@ -240,6 +253,7 @@ export function sameThreadStructure( for (const key of Object.keys(next) as Array) { if ( key === "thread" || + key === "messages" || key === "turnItems" || key === "visibleTurnItems" || key === "updatedAt" @@ -260,6 +274,7 @@ export function sameThreadStructure( return false; } return ( + sameProjectionArray(previous.messages, next.messages, sameMessageStructure) && sameProjectionArray(previous.turnItems, next.turnItems, sameTurnItemStructure) && sameProjectionArray( previous.visibleTurnItems, diff --git a/packages/client-runtime/src/state/threadRelationships.ts b/packages/client-runtime/src/state/threadRelationships.ts index 2aecc93470c2..26ff658a5191 100644 --- a/packages/client-runtime/src/state/threadRelationships.ts +++ b/packages/client-runtime/src/state/threadRelationships.ts @@ -1,11 +1,12 @@ import { isOrchestrationV2WorkActive, - type OrchestrationV2ThreadProjection, type OrchestrationV2ThreadShell, type ThreadId, } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; +import type { ThreadStructure } from "./models.ts"; + export type ThreadRelationshipKind = "parent" | "fork" | "subagent" | "transfer"; export interface ThreadRelationshipNode { @@ -34,7 +35,7 @@ export interface ThreadRelationshipWalkRow { } export function resolveMergeBackTargetThreadId( - projection: Pick | null, + projection: Pick | null, ): ThreadId | null { if (projection?.thread.lineage.relationshipToParent !== "fork") return null; return projection.thread.forkedFrom?.type === "run" @@ -48,7 +49,7 @@ function edgeKey(edge: ThreadRelationshipEdge): string { export function deriveThreadRelationshipGraph(input: { readonly threads: ReadonlyArray; - readonly projection: OrchestrationV2ThreadProjection | null; + readonly projection: ThreadStructure | null; }): ThreadRelationshipGraph { const threadsById = new Map(); for (const thread of input.threads) { diff --git a/packages/contracts/src/device.ts b/packages/contracts/src/device.ts index ad52b213f10f..dee7daa44524 100644 --- a/packages/contracts/src/device.ts +++ b/packages/contracts/src/device.ts @@ -588,6 +588,7 @@ export class DeviceOperationError extends Schema.TaggedError + PositiveInt.pipe( + Schema.decodeTo( + PositiveInt.check(Schema.isLessThanOrEqualTo(maximum)), + SchemaTransformation.transform({ + decode: (value) => Math.min(value, maximum), + encode: (value) => value, + }), + ), + ); + export const OrchestratorMcpThreadListInput = Schema.Struct({ environmentId: OrchestratorMcpEnvironmentTarget, projectId: OrchestratorMcpProjectTarget, @@ -351,7 +365,7 @@ export const OrchestratorMcpThreadListInput = Schema.Struct({ snoozed: Schema.optional(Schema.Boolean), includeSubagents: Schema.optional(Schema.Boolean), cursor: Schema.optional(NonNegativeInt), - limit: Schema.optional(PositiveInt.check(Schema.isLessThanOrEqualTo(100))), + limit: Schema.optional(clampedPositiveInt(100)), }); export type OrchestratorMcpThreadListInput = typeof OrchestratorMcpThreadListInput.Type; @@ -421,20 +435,6 @@ export const OrchestratorMcpThreadListResult = Schema.Struct({ }); export type OrchestratorMcpThreadListResult = typeof OrchestratorMcpThreadListResult.Type; -// Codex hides `minimum` and `maximum` from the model, so a value above the maximum clamps on -// decode instead of failing the call; pagination returns the rest. Values below 1 still fail: -// 0 asks for nothing, and answering it with one item would be a different request. -const clampedPositiveInt = (maximum: number) => - PositiveInt.pipe( - Schema.decodeTo( - PositiveInt.check(Schema.isLessThanOrEqualTo(maximum)), - SchemaTransformation.transform({ - decode: (value) => Math.min(value, maximum), - encode: (value) => value, - }), - ), - ); - export const OrchestratorMcpThreadReadInput = Schema.Struct({ environmentId: OrchestratorMcpEnvironmentTarget, threadId: ThreadId, diff --git a/packages/contracts/src/server.ts b/packages/contracts/src/server.ts index 88fa0220fe92..d859ecb91ffa 100644 --- a/packages/contracts/src/server.ts +++ b/packages/contracts/src/server.ts @@ -1013,6 +1013,7 @@ export const ServerSelfUpdateInput = Schema.Struct({ channel: Schema.optionalKey(DesktopUpdateChannelSchema), /** The target release's highest migration; the host refuses to leave Nightly without it. */ targetDatabaseMigration: Schema.optionalKey(Schema.Int), + stageOnly: Schema.optionalKey(Schema.Boolean), }); export type ServerSelfUpdateInput = typeof ServerSelfUpdateInput.Type; diff --git a/packages/provider-cursor/src/server/adapter.test.ts b/packages/provider-cursor/src/server/adapter.test.ts index 9a535eb9b52b..d627328e7e42 100644 --- a/packages/provider-cursor/src/server/adapter.test.ts +++ b/packages/provider-cursor/src/server/adapter.test.ts @@ -1726,6 +1726,157 @@ describe("CursorAdapterV2", () => { ), ); + it.effect("keeps the last 256 KiB of streamed shell output when the result carries none", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const workspace = yield* fileSystem.makeTempDirectoryScoped({ prefix: "cursor-v2-shell-" }); + const instanceId = ProviderInstanceId.make("cursor"); + const threadId = ThreadId.make("cursor-shell-thread"); + const modelSelection = { instanceId, model: "composer-2.5" }; + const runtimePolicy = ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ + runtimeMode: "full-access", + interactionMode: "default", + cwd: workspace, + }); + const chunks = Array.from({ length: 3000 }, (_, index) => `${index}`.padEnd(255, ".") + "\n"); + const shell = { + type: "shell" as const, + args: { command: "yes", workingDirectory: workspace }, + }; + const adapter = yield* makeCursorAdapterV2({ + instanceId, + environment: { HOME: workspace }, + }).pipe( + Effect.provideService(CursorAgentSdk.CursorAgentSdkRunner, { + assertComplete: Effect.void, + open: () => + Effect.succeed({ + agentId: "native-cursor-shell", + listMessages: Effect.succeed([]), + close: Effect.void, + send: (input) => + Effect.gen(function* () { + yield* input.onDelta!({ + type: "tool-call-started", + modelCallId: "model-call", + callId: "shell-call", + toolCall: shell, + }); + for (const chunk of chunks) { + yield* input.onDelta!({ + type: "shell-output-delta", + event: { case: "stdout", value: { data: chunk }, toolCallId: "shell-call" }, + }); + } + yield* input.onDelta!({ + type: "tool-call-completed", + modelCallId: "model-call", + callId: "shell-call", + toolCall: { + ...shell, + result: { + status: "success", + value: { + exitCode: 0, + signal: "", + stdout: "", + stderr: "", + executionTime: 0, + }, + }, + }, + }); + return { + agentId: "native-cursor-shell", + runId: "native-cursor-run", + wait: Effect.succeed({ + id: "native-cursor-run", + requestId: "native-request", + status: "finished" as const, + durationMs: 1, + }), + cancel: Effect.void, + }; + }).pipe(Effect.orDie), + }), + }), + Effect.provide(TestProviderHost.layer({ cwd: workspace })), + ); + const runtime = yield* adapter.openSession({ + threadId, + providerSessionId: ProviderSessionId.make("cursor-shell-session"), + modelSelection, + runtimePolicy, + }); + const providerThread = yield* runtime.ensureThread({ + threadId, + modelSelection, + runtimePolicy, + }); + const now = yield* DateTime.now; + yield* runtime.startTurn({ + threadId, + providerThread, + modelSelection, + runtimePolicy, + runId: RunId.make("cursor-shell-run"), + runOrdinal: 1, + providerTurnOrdinal: 1, + attemptId: RunAttemptId.make("cursor-shell-attempt"), + rootNodeId: NodeId.make("cursor-shell-root"), + appThread: { + id: threadId, + projectId: ProjectId.make("cursor-shell-project"), + createdBy: "user", + creationSource: "web", + title: "Cursor shell", + providerInstanceId: instanceId, + modelSelection, + runtimeMode: "full-access", + interactionMode: "default", + branch: null, + worktreePath: null, + activeProviderThreadId: providerThread.id, + lineage: { parentThreadId: null, relationshipToParent: null, rootThreadId: threadId }, + forkedFrom: null, + createdAt: now, + updatedAt: now, + archivedAt: null, + settledOverride: null, + settledAt: null, + lastVisitedAt: null, + deletedAt: null, + }, + message: { + messageId: MessageId.make("cursor-shell-message"), + createdBy: "user", + creationSource: "web", + text: "hi", + attachments: [], + }, + }); + const events = yield* runtime.events.pipe( + Stream.takeUntil((event) => event.type === "turn.terminal"), + Stream.runCollect, + ); + const output = events + .flatMap((event) => + event.type === "turn_item.updated" && + event.turnItem.type === "command_execution" && + event.turnItem.status === "completed" + ? [event.turnItem.output] + : [], + ) + .at(-1); + assert.equal(output, chunks.join("").slice(-256 * 1024)); + }).pipe( + Effect.scoped, + Effect.provide( + Layer.mergeAll(NodeServices.layer, IdAllocator.layer, McpProviderSessions.layer), + ), + ), + ); + it.effect("names a custom model Cursor's account catalog rejects", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; diff --git a/packages/provider-cursor/src/server/adapter.ts b/packages/provider-cursor/src/server/adapter.ts index 02ec1ce09d8f..ad0525a5f19e 100644 --- a/packages/provider-cursor/src/server/adapter.ts +++ b/packages/provider-cursor/src/server/adapter.ts @@ -1364,7 +1364,9 @@ export const makeCursorAdapterV2 = Effect.fn("makeCursorAdapterV2")(function* ( | "completedAt" | "updatedAt" >; - const outputText = cursorToolOutputText(toolCall) || active.streamedOutput; + const outputText = + cursorToolOutputText(toolCall) || + active.streamedOutput.slice(-MAX_CURSOR_STREAMED_OUTPUT_CHARS); let turnItem: OrchestrationV2TurnItem; switch (toolCall.type) { case "shell": @@ -2112,9 +2114,13 @@ export const makeCursorAdapterV2 = Effect.fn("makeCursorAdapterV2")(function* ( const chunk = shellOutputText(update.event); if (chunk.length === 0) return; // Persist output once; live hub deltas avoid quadratic event-store growth. - shell.streamedOutput = (shell.streamedOutput + chunk).slice( - -MAX_CURSOR_STREAMED_OUTPUT_CHARS, - ); + shell.streamedOutput += chunk; + // Trimming on every chunk would flatten and copy the whole tail each time. + if (shell.streamedOutput.length > 2 * MAX_CURSOR_STREAMED_OUTPUT_CHARS) { + shell.streamedOutput = shell.streamedOutput.slice( + -MAX_CURSOR_STREAMED_OUTPUT_CHARS, + ); + } yield* emitProviderEvent({ type: "command_output.delta", driver: CursorAgentSdk.CURSOR_PROVIDER, diff --git a/packages/shared/src/codexFileCitations.test.ts b/packages/shared/src/codexFileCitations.test.ts index bddbc8337315..df52912d037c 100644 --- a/packages/shared/src/codexFileCitations.test.ts +++ b/packages/shared/src/codexFileCitations.test.ts @@ -64,7 +64,7 @@ describe("codexFileCitationMarkdown", () => { path: "reports/*draft*_[copy]`<&.txt", }); expect(citation && codexFileCitationMarkdown(citation)).toBe( - "[\\*draft\\*\\_\\[copy\\]\\`\\<\\&.txt]()", + "[\\*draft\\*\\_\\[copy\\]\\`\\<\\&.txt]()", ); }); }); diff --git a/packages/shared/src/codexFileCitations.ts b/packages/shared/src/codexFileCitations.ts index d29996501ea6..15cf5ec13ece 100644 --- a/packages/shared/src/codexFileCitations.ts +++ b/packages/shared/src/codexFileCitations.ts @@ -45,6 +45,7 @@ function markdownLabel(value: string): string { function markdownDestination(value: string): string { return value .replaceAll("\\", "%5C") + .replaceAll("&", "%26") .replaceAll("<", "%3C") .replaceAll(">", "%3E") .replaceAll("\r", "%0D") diff --git a/packages/shared/src/codexMarkdownDirectives.test.ts b/packages/shared/src/codexMarkdownDirectives.test.ts index 4db9e7c4b86d..d4f4c5014a8a 100644 --- a/packages/shared/src/codexMarkdownDirectives.test.ts +++ b/packages/shared/src/codexMarkdownDirectives.test.ts @@ -106,6 +106,16 @@ describe.each([ expect(link?.type).toBe("link"); expect(parseMarkdownFileLink(link?.url ?? "")).toEqual({ path, line: 7 }); }); + + it("keeps an entity-like ampersand in the path literal", () => { + const markdown = render( + ':codex-file-citation{path="/tmp/a&copy;.txt" line_range_start="7"}', + ); + const link = parseOrdinaryMarkdown(markdown).children?.[0]?.children?.[0]; + + expect(link?.type).toBe("link"); + expect(parseMarkdownFileLink(link?.url ?? "")).toEqual({ path: "/tmp/a©.txt", line: 7 }); + }); }); describe("native Markdown adapters", () => { diff --git a/packages/shared/src/nodeSqliteClient.test.ts b/packages/shared/src/nodeSqliteClient.test.ts index ac65e7f12daa..b7b9b7a0866a 100644 --- a/packages/shared/src/nodeSqliteClient.test.ts +++ b/packages/shared/src/nodeSqliteClient.test.ts @@ -240,6 +240,25 @@ it.effect("reuses a worker query after an iterator constraint failure", () => }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); +it.effect("commits on the worker writer after a returning write timed out on a lock", () => + Effect.gen(function* () { + const { filename, other } = yield* makeTempDatabase; + yield* Effect.gen(function* () { + const sql = yield* SqlClient.SqlClient; + yield* sql`PRAGMA busy_timeout = 0`; + yield* Effect.sync(() => other.exec("BEGIN IMMEDIATE")); + const error = yield* sql`UPDATE counters SET value = 1 WHERE id = 1 RETURNING value`.pipe( + Effect.flip, + ); + assert.equal(error.reason._tag, "LockTimeoutError"); + yield* Effect.sync(() => other.exec("COMMIT")); + + yield* sql.withTransaction(sql`UPDATE counters SET value = 2 WHERE id = 1`); + assert.deepEqual(yield* sql`SELECT value FROM counters WHERE id = 1`.values, [[2]]); + }).pipe(Effect.provide(SqliteClient.layer({ filename, writerWorker: true }))); + }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), +); + it.effect("keeps another connection from committing between a transaction's read and write", () => Effect.gen(function* () { const { filename, other } = yield* makeTempDatabase; diff --git a/packages/shared/src/nodeSqliteClient.ts b/packages/shared/src/nodeSqliteClient.ts index fda878a114d9..ab14675c7bfc 100644 --- a/packages/shared/src/nodeSqliteClient.ts +++ b/packages/shared/src/nodeSqliteClient.ts @@ -244,6 +244,7 @@ parentPort.on("message", (request) => { } const { id, sql, params, cached, values, raw, safeIntegers, received } = request; let operation = "open"; + let iterator; try { if (db === undefined) throw new Error("database connection is unavailable"); operation = "prepare"; @@ -255,7 +256,8 @@ parentPort.on("message", (request) => { let rows = []; let bytes = 0; let pendingBatch = false; - for (const row of statement.iterate(...params)) { + iterator = statement.iterate(...params); + for (const row of iterator) { let rowBytes = 0; for (const key in row) { const value = row[key]; @@ -281,6 +283,10 @@ parentPort.on("message", (request) => { parentPort.postMessage({ id, rows: raw ? result : [] }); } } catch (error) { + // A step that throws leaves its statement active; an active write blocks every later COMMIT. + try { + iterator?.return(); + } catch {} // A failed iterator can report its previous SQLite error when the statement is reused. statements.delete(sql); parentPort.postMessage({ id, error: { operation, ...describe(error) } }); diff --git a/packages/ssh/src/launcherTestHarness.ts b/packages/ssh/src/launcherTestHarness.ts index fb307fffd2f8..b9b7f30d86e5 100644 --- a/packages/ssh/src/launcherTestHarness.ts +++ b/packages/ssh/src/launcherTestHarness.ts @@ -3,6 +3,7 @@ import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; import * as NodeURL from "node:url"; import * as NodeChildProcess from "node:child_process"; +import * as NodeSqlite from "node:sqlite"; import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as Effect from "effect/Effect"; @@ -58,6 +59,12 @@ export const captureLauncherFor = Effect.fn("test.captureLauncher")(function* ( export const captureLauncher = captureLauncherFor({ nodeScriptPath: "/fake/server.ts" }); +function holdLock(path: string) { + const db = new NodeSqlite.DatabaseSync(path); + db.exec("BEGIN EXCLUSIVE;"); + return { [Symbol.dispose]: () => db.close() }; +} + export function runLauncher( script: string, options: { @@ -68,6 +75,9 @@ export function runLauncher( readonly clockJump?: number; readonly readyAt?: number; readonly initialPort?: number; + // A boot service between two servers holds the home with no runtime record yet. + readonly serviceServing?: boolean; + readonly allowFailure?: boolean; // The desktop publishes runtime and removes the marker right after the first runtime read. readonly publishAfterFirstRuntimeRead?: boolean; }, @@ -192,6 +202,7 @@ if [ "$1" = --version ]; then printf 1.2.3; exit; fi [ "$1" = __ssh-helper ] case "$2" in update-remaining) printf 'marker %s\\n' "$(cat "$HOME/clock")" >> "$HOME/events"; exec "$TEST_REAL_NODE" --require "$HOME/clock.cjs" "$HOME/helper.ts" "$2" "$3" ;; + home-owned) exec "$TEST_REAL_NODE" "$HOME/helper.ts" "$2" "$3" ;; runtime-port | pick-port | wait-ready) exec "$HOME/bin/test-node" - "$3" ;; *) exit 1 ;; esac @@ -199,6 +210,9 @@ esac { mode: 0o755 }, ); } + using _serving = options.serviceServing + ? holdLock(NodePath.join(runtime, "service-serving.lock")) + : undefined; const result = NodeChildProcess.spawnSync("/bin/sh", ["-s", "--", "test-target"], { input: `kill() { if [ "$1" = -0 ]; then command kill "$@"; else @@ -222,12 +236,14 @@ ${script}`, TEST_PUBLISH_AFTER_FIRST_READ: options.publishAfterFirstRuntimeRead ? "1" : "0", }, }); - if (result.error || result.status !== 0) + if (result.error || (result.status !== 0 && !options.allowFailure)) throw new Error( `launcher failed: ${result.error ?? result.status}\n${result.stderr}\n${result.stdout}\n${NodeFS.readFileSync(NodePath.join(home, "events"), "utf8")}`, ); return { - output: JSON.parse(result.stdout.trim()), + status: result.status, + stderr: result.stderr, + output: result.status === 0 ? JSON.parse(result.stdout.trim()) : undefined, events: NodeFS.readFileSync(NodePath.join(home, "events"), "utf8").trim().split("\n"), now: Number(NodeFS.readFileSync(NodePath.join(home, "clock"), "utf8")), markerExists: NodeFS.existsSync(markerPath), diff --git a/packages/ssh/src/tunnel.test.ts b/packages/ssh/src/tunnel.test.ts index 976f9d32c4b7..7c107140e02b 100644 --- a/packages/ssh/src/tunnel.test.ts +++ b/packages/ssh/src/tunnel.test.ts @@ -1520,6 +1520,33 @@ describe("SSH launch during desktop update", () => { }), ); + it.effect.each([false, true])( + "waits for a boot service between two servers (archive: %s)", + (archive) => + Effect.gen(function* () { + const script = yield* archive + ? captureLauncherFor({ archiveVersion: "1.2.3" }) + : captureLauncher; + const result = runLauncher(script, { archive, serviceServing: true, readyAt: 5_000 }); + assert.deepEqual(result.output, { remotePort: 4888, serverKind: "external" }); + assert.equal(result.now, 5_000); + assert.isFalse(result.events.some((event) => event.startsWith("start "))); + }), + ); + + it.effect("fails without starting a second server when the home's owner never answers", () => + Effect.gen(function* () { + const result = runLauncher(yield* captureLauncher, { + serviceServing: true, + allowFailure: true, + }); + assert.equal(result.status, 1); + assert.include(result.stderr, "no second server was started"); + assert.equal(result.now, 60_000); + assert.isFalse(result.events.some((event) => event.startsWith("start "))); + }), + ); + it.effect("starts immediately without a marker", () => Effect.gen(function* () { const result = runLauncher(yield* captureLauncher, {}); diff --git a/packages/ssh/src/tunnel.ts b/packages/ssh/src/tunnel.ts index 183eb9b9eac1..c3ff20293aa1 100644 --- a/packages/ssh/src/tunnel.ts +++ b/packages/ssh/src/tunnel.ts @@ -82,20 +82,28 @@ const REMOTE_UPDATE_WAIT_MS = DESKTOP_UPDATE_IN_PROGRESS_TTL_MS + 2 * (REMOTE_REUSE_READY_TIMEOUT_MS + SSH_READY_PROBE_TIMEOUT_MS) + 1_000; +// A launch waits this long for a server that holds the home to answer; its +// last poll adds a reuse probe. +const REMOTE_OWNER_WAIT_MS = + REMOTE_READY_TIMEOUT_MS + REMOTE_REUSE_READY_TIMEOUT_MS + SSH_READY_PROBE_TIMEOUT_MS; // A launch holds the home lock through up to two reuse probes, readiness, two -// server exit waits, the runtime record wait, and the update wait; each probe -// may overrun its deadline by one request. +// server exit waits, the runtime record wait, the update wait, and the owner +// wait; each probe may overrun its deadline by one request. const REMOTE_LAUNCH_LOCK_WAIT_SECONDS = Math.ceil( (REMOTE_READY_TIMEOUT_MS + 2 * REMOTE_REUSE_READY_TIMEOUT_MS + 2 * REMOTE_PID_EXIT_WAIT_MS + REMOTE_RUNTIME_RECORD_WAIT_SECONDS * 1000 + 4 * SSH_READY_PROBE_TIMEOUT_MS + - REMOTE_UPDATE_WAIT_MS) / + REMOTE_UPDATE_WAIT_MS + + REMOTE_OWNER_WAIT_MS) / 1000, ); const REMOTE_LAUNCH_TIMEOUT_MS = - REMOTE_LAUNCH_LOCK_WAIT_SECONDS * 1000 + REMOTE_UPDATE_WAIT_MS + REMOTE_LAUNCH_BASE_TIMEOUT_MS; + REMOTE_LAUNCH_LOCK_WAIT_SECONDS * 1000 + + REMOTE_UPDATE_WAIT_MS + + REMOTE_OWNER_WAIT_MS + + REMOTE_LAUNCH_BASE_TIMEOUT_MS; export interface RemoteT3RunnerOptions { readonly nodeScriptPath?: string | null; @@ -708,6 +716,39 @@ server_running() { esac } @@T3_MANAGED_SERVER_RUNNING_SCRIPT@@ +# Both are SQLite locks the OS drops with their holder: a server holds +# server-owner.sqlite, and a boot service holds service-serving.lock from +# before it starts a server, between two of them during its own update, until +# it stops or idles behind another server. +home_owned() { + if [ "$T3_ARCHIVE_MODE" = "1" ]; then + "$RUNNER_FILE" __ssh-helper home-owned "$DEFAULT_SERVER_HOME" 2>/dev/null + return + fi + node - "$DEFAULT_SERVER_HOME" 2>/dev/null <<'NODE' +const fs = require("node:fs"); +const path = require("node:path"); +const { DatabaseSync } = require("node:sqlite"); +const held = (file) => { + if (!fs.existsSync(file)) return false; + const db = new DatabaseSync(file); + try { + db.exec("PRAGMA busy_timeout = 0; BEGIN EXCLUSIVE;"); + return false; + } catch (error) { + return error?.errcode === 5; + } finally { + db.close(); + } +}; +const home = process.argv[2]; +process.exitCode = + held(path.join(home, "runtime", "service-serving.lock")) || + held(path.join(home, "userdata", "server-owner.sqlite")) + ? 0 + : 1; +NODE +} resolve_default_runtime_port() { if [ "$T3_ARCHIVE_MODE" = "1" ]; then "$RUNNER_FILE" __ssh-helper runtime-port "$DEFAULT_RUNTIME_FILE" @@ -896,6 +937,25 @@ if [ -z "$REMOTE_PORT" ]; then REMOTE_MANAGED="" fi fi +# A server that holds the home without answering yet is still starting, and a +# boot service holds it between two servers while it updates. A second server +# would lose the home to it or take the home from it, so wait while the home is +# held and start one only once it is free. The managed server an update just +# stopped may still be exiting. The sleep count bounds a host clock that stops. +OWNER_WAITED=0 +OWNER_DEADLINE=$(($(date +%s) + @@T3_READY_TIMEOUT_SECONDS@@)) +while [ -z "$REMOTE_PORT" ] && ! managed_server_running "$(cat "$PID_FILE" 2>/dev/null || true)" && home_owned; do + if adopt_default_server; then break; fi + REMOTE_PID="" + REMOTE_PORT="" + REMOTE_MANAGED="" + if [ "$OWNER_WAITED" -ge @@T3_READY_TIMEOUT_SECONDS@@ ] || [ "$(date +%s)" -ge "$OWNER_DEADLINE" ]; then + printf 'A T3 server or boot service holds %s but did not become ready, so no second server was started. Retry once it has started.\\n' "$DEFAULT_SERVER_HOME" >&2 + exit 1 + fi + sleep 1 + OWNER_WAITED=$((OWNER_WAITED + 1)) +done if [ -z "$REMOTE_PORT" ]; then REMOTE_PORT="$(pick_port)" || true if [ -z "$REMOTE_PORT" ]; then @@ -1060,6 +1120,7 @@ function buildRemoteLaunchScript(input?: RemoteT3RunnerOptions): string { T3_DEFAULT_REMOTE_PORT: String(DEFAULT_REMOTE_PORT), T3_REMOTE_PORT_SCAN_WINDOW: String(REMOTE_PORT_SCAN_WINDOW), T3_READY_TIMEOUT_MS: String(REMOTE_READY_TIMEOUT_MS), + T3_READY_TIMEOUT_SECONDS: String(REMOTE_READY_TIMEOUT_MS / 1000), T3_REUSE_READY_TIMEOUT_MS: String(REMOTE_REUSE_READY_TIMEOUT_MS), T3_READY_PROBE_TIMEOUT_MS: String(SSH_READY_PROBE_TIMEOUT_MS), T3_PID_EXIT_WAIT_TICKS: String(REMOTE_PID_EXIT_WAIT_MS / 100), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 146750ed4f84..2a34ae2e2150 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1557,16 +1557,28 @@ importers: jszip: specifier: 3.10.1 version: 3.10.1 + remark-gfm: + specifier: 'catalog:' + version: 4.0.1(supports-color@8.1.1) + remark-parse: + specifier: 'catalog:' + version: 11.0.0(supports-color@8.1.1) three: specifier: 0.180.0 version: 0.180.0 unbash: specifier: ^5.0.0 version: 5.0.0 + unified: + specifier: 'catalog:' + version: 11.0.5 devDependencies: '@effect/vitest': specifier: 4.1.2 version: 4.1.2(patch_hash=299510b71c36537a832a67e9d160a5f112a5f326a29e95c41cb45fc8234e47c7)(@astrojs/compiler-rs@0.5.1)(@types/node@24.12.4)(@vitest/browser-playwright@5.0.5)(@vitest/ui@5.0.5)(@volar/typescript@2.4.28(typescript@6.0.3))(bufferutil@4.1.0)(effect@4.1.3)(esbuild@0.28.2)(jiti@2.7.0)(jsdom@30.1.0(@noble/hashes@1.8.0))(playwright@1.64.1)(terser@5.48.0)(typescript@6.0.3)(unrun@0.2.39)(utf-8-validate@6.0.6)(yaml@2.9.0) + '@types/mdast': + specifier: ^4.0.4 + version: 4.0.4 '@types/three': specifier: 0.180.0 version: 0.180.0 @@ -2559,6 +2571,9 @@ importers: esbuild: specifier: 0.28.2 version: 0.28.2 + jszip: + specifier: 3.10.1 + version: 3.10.1 postject: specifier: 1.0.0-alpha.6 version: 1.0.0-alpha.6 diff --git a/scripts/lib/cached-tests.test.ts b/scripts/lib/cached-tests.test.ts index 7079006ddd05..80d45d9a1a86 100644 --- a/scripts/lib/cached-tests.test.ts +++ b/scripts/lib/cached-tests.test.ts @@ -6,7 +6,7 @@ import * as NodeOS from "node:os"; import * as NodePath from "node:path"; import * as NodeURL from "node:url"; -import { cachedTestTasks, matchesTestPath } from "./cached-tests.ts"; +import { cachedTestTasks, matchesTestPath, readRunSummary } from "./cached-tests.ts"; describe("cachedTestTasks", () => { it("selects every test for a checkout-root path", () => { @@ -22,9 +22,11 @@ describe("cachedTestTasks", () => { expect(matchesTestPath("apps/server-other/a.test.ts", ["apps/server"])).toBe(false); }); it("keeps a file's cache task stable when unrelated tests are added", () => { - const first = cachedTestTasks(JSON.stringify({ files: ["src/b.test.ts"], groupSize: 1 })); + const first = cachedTestTasks( + JSON.stringify({ run: "r", files: ["src/b.test.ts"], groupSize: 1 }), + ); const second = cachedTestTasks( - JSON.stringify({ files: ["src/a.test.ts", "src/b.test.ts"], groupSize: 1 }), + JSON.stringify({ run: "r", files: ["src/a.test.ts", "src/b.test.ts"], groupSize: 1 }), ); const key = Object.keys(first).find((key) => key !== "test-cache")!; expect(second[key]).toEqual(first[key]); @@ -32,24 +34,28 @@ describe("cachedTestTasks", () => { it("keeps shell metacharacters in filenames out of the command", () => { const tasks = cachedTestTasks( - JSON.stringify({ files: ["src/a'$(touch injected).test.ts"], groupSize: 1 }), + JSON.stringify({ run: "r", files: ["src/a'$(touch injected).test.ts"], groupSize: 1 }), ); const task = Object.entries(tasks).find(([key]) => key !== "test-cache")![1]; expect(task.command).toBe("vp test run 'src/a'\"'\"'$(touch injected).test.ts'"); }); - it("rejects paths outside the checkout and empty groups", () => { + it("rejects paths outside the checkout, empty groups, and unsafe run ids", () => { for (const request of [ - { files: ["../elsewhere.test.ts"], groupSize: 1 }, - { files: ["/absolute.test.ts"], groupSize: 1 }, - { files: ["src/a.test.ts"], groupSize: 0 }, + { run: "r", files: ["../elsewhere.test.ts"], groupSize: 1 }, + { run: "r", files: ["/absolute.test.ts"], groupSize: 1 }, + { run: "r", files: ["src/a.test.ts"], groupSize: 0 }, + { files: ["src/a.test.ts"], groupSize: 1 }, + { run: "r; touch injected", files: ["src/a.test.ts"], groupSize: 1 }, ]) { expect(() => cachedTestTasks(JSON.stringify(request))).toThrow(); } }); it("accepts spec files discovered by Vitest", () => { - const tasks = cachedTestTasks(JSON.stringify({ files: ["src/example.spec.ts"], groupSize: 1 })); + const tasks = cachedTestTasks( + JSON.stringify({ run: "r", files: ["src/example.spec.ts"], groupSize: 1 }), + ); expect( Object.values(tasks).some((task) => task.command === "vp test run 'src/example.spec.ts'"), ).toBe(true); @@ -59,11 +65,45 @@ describe("cachedTestTasks", () => { expect(cachedTestTasks(undefined)).toEqual({}); }); + it("reads only the task summary of the given run", async () => { + const directory = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "cached-tests-")); + const write = async (version: string, run: string) => { + const file = NodePath.join(directory, version, "last-summary.json"); + await NodeFSP.mkdir(NodePath.dirname(file)); + const tasks = cachedTestTasks(JSON.stringify({ run, files: ["a.test.ts"], groupSize: 1 })); + await NodeFSP.writeFile( + file, + JSON.stringify({ + tasks: Object.entries(tasks).map(([task_name, task]) => ({ + task_name, + command: task.command, + result: {}, + })), + }), + ); + }; + try { + expect(readRunSummary(NodePath.join(directory, "missing"), "own")).toBeUndefined(); + await NodeFSP.writeFile(NodePath.join(directory, "file"), ""); + expect(readRunSummary(NodePath.join(directory, "file"), "own")).toBeUndefined(); + await write("v18", "other"); + expect(readRunSummary(directory, "own")).toBeUndefined(); + await write("v19", "own"); + await NodeFSP.mkdir(NodePath.join(directory, "corrupt")); + await NodeFSP.writeFile(NodePath.join(directory, "corrupt", "last-summary.json"), "{"); + expect(readRunSummary(directory, "own")?.tasks).toHaveLength(3); + } finally { + await NodeFSP.rm(directory, { recursive: true, force: true }); + } + }); + it("reuses individual results, invalidates filesystem inputs, and never caches failures", async () => { const directory = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "cached-tests-")); const root = NodePath.resolve(import.meta.dirname, "../.."); + let runId = 0; const run = (files = ["a.test.ts", "b.test.ts"], uncached: Array = []) => new Promise((resolve, reject) => { + runId += 1; const child = NodeChildProcess.spawn( process.execPath, [NodePath.join(root, "node_modules/vite-plus/dist/bin.js"), "run", "test-cache"], @@ -73,7 +113,12 @@ describe("cachedTestTasks", () => { ...process.env, NODE_OPTIONS: "", VP_REMOTE_CACHE: "off", - T3_CACHED_TESTS: JSON.stringify({ files, groupSize: 1, uncached }), + T3_CACHED_TESTS: JSON.stringify({ + run: String(runId), + files, + groupSize: 1, + uncached, + }), VITE_CACHE_PATH: NodePath.join(directory, "cache"), PATH: `${NodePath.join(root, "node_modules/.bin")}${NodePath.delimiter}${process.env.PATH}`, }, @@ -83,12 +128,10 @@ describe("cachedTestTasks", () => { child.once("error", reject); child.once("close", resolve); }); - const summary = async () => { - const value: { tasks: Array<{ result: Record }> } = JSON.parse( - await NodeFSP.readFile(NodePath.join(directory, "cache/v18/last-summary.json"), "utf8"), - ); - return value.tasks.filter((task) => "CacheHit" in task.result).length; - }; + const summary = async () => + readRunSummary(NodePath.join(directory, "cache"), String(runId))!.tasks.filter( + (task) => task.result.CacheHit, + ).length; try { await NodeFSP.mkdir(NodePath.join(directory, "node_modules")); await NodeFSP.symlink( diff --git a/scripts/lib/cached-tests.ts b/scripts/lib/cached-tests.ts index a21d13bd83d8..de99cf1ca62d 100644 --- a/scripts/lib/cached-tests.ts +++ b/scripts/lib/cached-tests.ts @@ -15,6 +15,36 @@ interface CachedTestTask { }; } +interface TaskRunSummary { + tasks: Array<{ + task_name: string; + command: string; + result: { CacheHit?: { source: string; saved_duration_ms: number } }; + }>; +} + +// vp keeps one last-summary.json per cache, rewritten by every `vp run`, so a run is +// recognized by an uncached task that every test depends on and that carries its id. +const runTask = "test-cache:run"; +const runCommand = (run: string) => `node -e "" ${run}`; + +export function readRunSummary(cacheRoot: string, run: string): TaskRunSummary | undefined { + try { + for (const entry of NodeFS.readdirSync(cacheRoot)) { + try { + const summary: TaskRunSummary = JSON.parse( + NodeFS.readFileSync(NodePath.join(cacheRoot, entry, "last-summary.json"), "utf8"), + ); + if (summary.tasks.some((task) => task.command === runCommand(run))) return summary; + } catch { + continue; + } + } + } catch { + return undefined; + } +} + export function matchesTestPath(file: string, filters: Array) { return ( filters.length === 0 || @@ -36,7 +66,10 @@ export function cachedTestTasks( !("groupSize" in request) || typeof request.groupSize !== "number" || !Number.isSafeInteger(request.groupSize) || - request.groupSize < 1 + request.groupSize < 1 || + !("run" in request) || + typeof request.run !== "string" || + !/^[\w-]+$/.test(request.run) ) { throw new Error("Invalid cached test request"); } @@ -95,6 +128,7 @@ export function cachedTestTasks( for (const group of groups) { const key = NodeCrypto.createHash("sha256").update(group.join("\0")).digest("hex"); tasks[`test-cache:${key}`] = { + dependsOn: [runTask], command: `${command} ${group.map((file) => `'${NodePath.relative(directory, NodePath.join(root, file)).replaceAll("'", "'\"'\"'")}'`).join(" ")}`, ...(directory === root ? {} : { cwd: NodePath.relative(root, directory) }), cache: uncached.includes(group[0]!) @@ -113,6 +147,7 @@ export function cachedTestTasks( } return { ...tasks, + [runTask]: { command: runCommand(request.run), cache: false }, "test-cache": { command: 'node -e ""', dependsOn: Object.keys(tasks), cache: false }, }; } diff --git a/scripts/package.json b/scripts/package.json index 716a2edb3762..9eb10db4c227 100644 --- a/scripts/package.json +++ b/scripts/package.json @@ -29,6 +29,7 @@ "@types/plist": "catalog:", "@types/pngjs": "6.0.5", "esbuild": "0.28.2", + "jszip": "3.10.1", "postject": "1.0.0-alpha.6", "typescript-legacy": "npm:typescript@~6.0.3", "vite-plus": "catalog:" diff --git a/scripts/scenarios/fork-title-regenerates-from-inherited-history.e2e.ts b/scripts/scenarios/fork-title-regenerates-from-inherited-history.e2e.ts new file mode 100644 index 000000000000..535ffde20719 --- /dev/null +++ b/scripts/scenarios/fork-title-regenerates-from-inherited-history.e2e.ts @@ -0,0 +1,26 @@ +import { expect } from "e2e"; + +import { test, WEB } from "./t3.ts"; + +// Scenarios title with a Codex command that always fails, so reaching it is the fixed behavior; +// the bug refused first with "This thread has no messages to title yet." +test( + "Regenerate title on a fresh fork reaches the title model with the inherited history", + WEB, + async ({ t3, app, screen }) => { + const project = await t3.project("demo"); + const thread = await t3.thread({ project, title: "Earlier work", messages: ["first"] }); + await t3.open(thread); + + await expect(screen.getByText("first fixture turn complete", { exact: false })).toBeVisible(); + await screen.getByRole("button", "Fork from this response").first().tap(); + const forkActions = screen.getByRole("button", "Thread actions for Earlier work fork"); + await expect(forkActions).toBeVisible(); + + await forkActions.tap(); + await screen.getByRole("menuitem", "Regenerate title").tap(); + await expect(screen.getByText("Failed to regenerate thread title")).toBeVisible(); + await app.screenshot("regenerate-result"); + await expect(screen.getByText("Codex CLI command", { exact: false })).toBeVisible(); + }, +); diff --git a/scripts/scenarios/odt-preview-expands-repeated-table-cells.e2e.ts b/scripts/scenarios/odt-preview-expands-repeated-table-cells.e2e.ts new file mode 100644 index 000000000000..c8efdb146e67 --- /dev/null +++ b/scripts/scenarios/odt-preview-expands-repeated-table-cells.e2e.ts @@ -0,0 +1,39 @@ +import { expect } from "e2e"; +import JSZip from "jszip"; + +import { test, WEB } from "./t3.ts"; + +// A 2 by 2 table stored the compact way ODF allows: one row repeated twice, one cell repeated twice. +async function repeatedTableOdt() { + const cell = + 'Approved'; + return new JSZip() + .file("mimetype", "application/vnd.oasis.opendocument.text", { compression: "STORE" }) + .file( + "content.xml", + `${cell}`, + ) + .generateAsync({ type: "uint8array" }); +} + +test( + "An ODT preview shows every cell of a table stored with repeated rows and cells", + WEB, + async ({ t3, app, screen, browser }) => { + const project = await t3.project("demo", { "decisions.odt": await repeatedTableOdt() }); + const thread = await t3.thread({ project, title: "Review decisions", messages: ["Start"] }); + await t3.open(thread); + await expect(screen.getByText("fixture turn complete", { exact: false })).toBeVisible(); + + await browser.keyboard.press("Control+p"); + await screen.getByPlaceholder("Search files…").fill("decisions.odt"); + await screen.getByRole("option", "decisions.odt", { exact: false }).tap(); + + // The first document preview loads its worker, which a cold Vite cache optimizes on demand. + await expect(screen.getByText("1 page", { exact: false })).toBeVisible({ timeout: 120_000 }); + const document = browser.frameLocator('iframe[title^="decisions.odt"]'); + await expect(document.getByRole("cell", "Approved").first()).toBeVisible(); + await app.screenshot("table"); + await expect(document.getByRole("cell", "Approved")).toHaveCount(4); + }, +); diff --git a/scripts/scenarios/saved-file-shows-external-edit.e2e.ts b/scripts/scenarios/saved-file-shows-external-edit.e2e.ts new file mode 100644 index 000000000000..fea75e9743b0 --- /dev/null +++ b/scripts/scenarios/saved-file-shows-external-edit.e2e.ts @@ -0,0 +1,101 @@ +/// +// The DOM lib types the functions this scenario runs in the page. +import { expect } from "e2e"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; + +import { runPromise, test, WEB } from "./t3.ts"; + +const chmod = (path: string, mode: number) => + runPromise(Effect.flatMap(FileSystem.FileSystem, (fs) => fs.chmod(path, mode))); + +test( + "A saved file shows a later external edit after its post-save read failed", + WEB, + async ({ t3, app, browser, screen }) => { + // Logs each file RPC's outcome in order, so the test can wait for the read after the save. + await browser.addInitScript(() => { + const log: Array = []; + const tags = new Map(); + const watched = new WeakSet(); + Object.assign(window, { fileRpcLog: log }); + const send = WebSocket.prototype.send; + WebSocket.prototype.send = function (data) { + if (typeof data === "string") { + for (const message of [JSON.parse(data)].flat()) { + if (message?._tag === "Request") tags.set(String(message.id), message.tag); + } + } + if (!watched.has(this)) { + watched.add(this); + this.addEventListener("message", (event) => { + if (typeof event.data !== "string") return; + for (const message of [JSON.parse(event.data)].flat()) { + const tag = message?._tag === "Exit" ? tags.get(String(message.requestId)) : null; + if (tag?.startsWith("projects.")) log.push(`${tag}:${message.exit._tag}`); + } + }); + } + return send.call(this, data); + }; + }); + const loggedAfter = (after: string, entry: string) => + browser.evaluate( + ({ after, entry }) => { + const log = (window as unknown as { fileRpcLog: Array }).fileRpcLog; + const start = log.lastIndexOf(after); + return start >= 0 && log.indexOf(entry, start + 1) > start; + }, + { after, entry }, + ); + // The file tree and editor render in shadow roots that locators do not reach. + const openTreeRow = (path: string) => + browser.evaluate((path) => { + for (const host of document.querySelectorAll("*")) { + const row = host.shadowRoot?.querySelector(`[data-item-path="${path}"]`); + if (row) { + row.click(); + return true; + } + } + return false; + }, path); + const editorText = () => + browser.evaluate(() => { + for (const host of document.querySelectorAll("*")) { + const content = host.shadowRoot?.querySelector("[data-content]"); + if (content) return content.textContent ?? ""; + } + return null; + }); + + const project = await t3.project("demo", { "notes.txt": "before save" }); + const notes = `${project}/notes.txt`; + const thread = await t3.thread({ project, title: "Notes", messages: ["first"] }); + await t3.open(thread); + await expect(screen.getByText("first fixture turn complete", { exact: false })).toBeVisible(); + await screen.getByRole("button", "Toggle right panel").tap(); + await screen.getByText("Files").tap(); + await expect.poll(() => openTreeRow("notes.txt")).toBe(true); + await expect.poll(editorText).toContain("before save"); + + // Requires a non-root server and in-place writes: autosave succeeds, confirmation read fails. + await chmod(notes, 0o200); + await browser.keyboard.press("ControlOrMeta+a"); + await browser.keyboard.type("saved draft"); + await expect + .poll(() => loggedAfter("projects.writeFile:Success", "projects.readFile:Failure")) + .toBe(true); + + await chmod(notes, 0o644); + await runPromise( + Effect.flatMap(FileSystem.FileSystem, (fs) => fs.writeFileString(notes, "external update")), + ); + await screen.getByRole("button", "Refresh workspace files").tap(); + await expect + .poll(() => loggedAfter("projects.readFile:Failure", "projects.readFile:Success")) + .toBe(true); + await app.screenshot("after-external-edit"); + await expect.poll(editorText).toContain("external update"); + }, +); diff --git a/scripts/scenarios/t3.ts b/scripts/scenarios/t3.ts index 24bb6e73d38b..6a0a95ce6632 100644 --- a/scripts/scenarios/t3.ts +++ b/scripts/scenarios/t3.ts @@ -15,7 +15,10 @@ import { run } from "./shell.ts"; export interface T3State { /** Creates a git repository holding `files` and adds it as a project. Returns its path. */ - readonly project: (title: string, files?: Readonly>) => Promise; + readonly project: ( + title: string, + files?: Readonly>, + ) => Promise; /** * Starts a thread in a project and plays one replayed turn per message before returning. * `model` is a dev replay recording: showcase, todo-list, approval, question, multi-turn. @@ -69,7 +72,7 @@ export function makeState(home: string, env: NodeJS.ProcessEnv = {}) { const project = Effect.fn("scenario.project")(function* ( title: string, - files: Readonly>, + files: Readonly>, ) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; @@ -82,7 +85,9 @@ export function makeState(home: string, env: NodeJS.ProcessEnv = {}) { const workspace = path.join(parent, title); for (const [file, content] of Object.entries({ "README.md": "", ...files })) { yield* fs.makeDirectory(path.dirname(path.join(workspace, file)), { recursive: true }); - yield* fs.writeFileString(path.join(workspace, file), content); + yield* typeof content === "string" + ? fs.writeFileString(path.join(workspace, file), content) + : fs.writeFile(path.join(workspace, file), content); } const git = (args: ReadonlyArray) => run("git", args, { cwd: workspace }); const author = ["-c", "user.name=Scenario", "-c", "user.email=scenario@example.invalid"]; diff --git a/scripts/scenarios/thread-search-finds-accented-message.e2e.ts b/scripts/scenarios/thread-search-finds-accented-message.e2e.ts new file mode 100644 index 000000000000..1e4e2a8f4ac9 --- /dev/null +++ b/scripts/scenarios/thread-search-finds-accented-message.e2e.ts @@ -0,0 +1,25 @@ +import { expect } from "e2e"; + +import { test, WEB } from "./t3.ts"; + +test( + "Thread search finds an accented message by a short unaccented word", + WEB, + async ({ t3, app, screen }) => { + const project = await t3.project("demo"); + const thread = await t3.thread({ project, title: "Weekly notes", messages: ["Start"] }); + await t3.open(thread); + + // Search reads what the user wrote; messages the CLI sends count as an agent's. + const composer = screen.getByRole("textbox"); + await composer.pressSequentially("École fermée à Lyon"); + await composer.press("Enter"); + await expect(screen.getByText("second fixture turn complete", { exact: false })).toBeVisible(); + + await screen.getByRole("combobox", "Search threads").fill("fermée ec"); + await expect(screen.getByRole("listbox", "Thread search results")).toContainText( + "École fermée", + ); + await app.screenshot("search"); + }, +); diff --git a/scripts/test-cached.ts b/scripts/test-cached.ts index 5c52833961d5..2ca43954f8f6 100644 --- a/scripts/test-cached.ts +++ b/scripts/test-cached.ts @@ -5,7 +5,7 @@ import * as NodeFS from "node:fs"; import * as NodePath from "node:path"; import * as NodeUtil from "node:util"; -import { matchesTestPath } from "./lib/cached-tests.ts"; +import { matchesTestPath, readRunSummary } from "./lib/cached-tests.ts"; import { shardedTestFiles } from "./lib/sharded-tests.ts"; const root = NodePath.resolve(import.meta.dirname, ".."); @@ -66,9 +66,11 @@ if (files.length === 0) { console.log( `Cached tests: ${files.length} files, ${groupSize} per task, concurrency ${concurrency}`, ); +const run = NodeCrypto.randomBytes(16).toString("hex"); const env = { ...process.env, T3_CACHED_TESTS: JSON.stringify({ + run, files, groupSize, uncached: values.uncached.map((file) => NodePath.relative(root, NodePath.resolve(file))), @@ -90,24 +92,18 @@ process.exitCode = result.status ?? 1; if (verifyHits > 0) { const cacheRoot = process.env.VITE_CACHE_PATH || NodePath.join(root, "node_modules/.vite/task-cache"); - const summaryFiles = NodeFS.readdirSync(cacheRoot, { withFileTypes: true }) - .filter((entry) => entry.isDirectory()) - .map((entry) => NodePath.join(cacheRoot, entry.name, "last-summary.json")) - .filter((file) => NodeFS.existsSync(file)) - .sort((a, b) => NodeFS.statSync(b).mtimeMs - NodeFS.statSync(a).mtimeMs); - const summary: { - tasks: Array<{ - task_name: string; - result: { CacheHit?: { source: string; saved_duration_ms: number } }; - }>; - } = JSON.parse(NodeFS.readFileSync(summaryFiles[0]!, "utf8")); + const summary = readRunSummary(cacheRoot, run); + if (!summary) { + console.error(`No task summary from this run in ${cacheRoot}`); + process.exitCode ||= 1; + } const rank = (task: string) => NodeCrypto.createHash("sha256") .update( `${process.env.GITHUB_RUN_ID || "local"}:${process.env.GITHUB_RUN_ATTEMPT || "1"}:${task}`, ) .digest("hex"); - const hits = summary.tasks.filter((task) => task.result.CacheHit); + const hits = (summary?.tasks ?? []).filter((task) => task.result.CacheHit); const verify = hits .toSorted((a, b) => rank(a.task_name).localeCompare(rank(b.task_name))) .slice(0, verifyHits);