diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 00c916a7d638e..b7828478eca35 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -1601,7 +1601,7 @@ describe("PreviewManager", () => { ), ); - effectIt.effect("emulates prefers-color-scheme and re-applies it across webview swaps", () => + effectIt.effect("re-applies opaque base and color scheme across webview swaps", () => withManager((manager) => Effect.gen(function* () { const makeWebContents = (id: number) => { @@ -1650,6 +1650,16 @@ describe("PreviewManager", () => { yield* manager.registerWebview("tab_scheme", 42); yield* Effect.yieldNow; + // Guests start with a transparent base; dark-scheme pages need an opaque + // one or Chromium skips their dark canvas. + const opaqueBase = { + color: { r: 255, g: 255, b: 255, a: 1 }, + }; + expect(first.sendCommand).toHaveBeenCalledWith( + "Emulation.setDefaultBackgroundColorOverride", + opaqueBase, + ); + yield* manager.setColorScheme("tab_scheme", "dark"); expect(first.sendCommand).toHaveBeenCalledWith("Emulation.setEmulatedMedia", { @@ -1662,6 +1672,10 @@ describe("PreviewManager", () => { yield* manager.registerWebview("tab_scheme", 43); yield* Effect.yieldNow; + expect(replacement.sendCommand).toHaveBeenCalledWith( + "Emulation.setDefaultBackgroundColorOverride", + opaqueBase, + ); expect(replacement.sendCommand).toHaveBeenCalledWith("Emulation.setEmulatedMedia", { features: [{ name: "prefers-color-scheme", value: "dark" }], }); diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index 1343e43a6c5b0..47a2236c2ac6f 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -1425,6 +1425,19 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ), { concurrency: "unbounded", discard: true }, ); + // Electron gives `` guests a transparent base background, and + // Chromium only paints a dark canvas for dark color-scheme pages over an + // opaque base. Without this, dark-scheme pages with no background of + // their own (text/plain, e.g. .md files) render white text on white. + // White matches the webview's existing white backing, so light pages look + // the same; Chromium still swaps in its dark canvas for dark-scheme pages. + yield* attemptPromise( + { operation: "initializeDebugger.defaultBackground", webContentsId: wc.id }, + () => + wcDebugger.sendCommand("Emulation.setDefaultBackgroundColorOverride", { + color: { r: 255, g: 255, b: 255, a: 1 }, + }), + ); return [ control, replaceMap(sessions, (copy) => { diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index d45a1015e0299..e77d083446444 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -66,6 +66,7 @@ const clientSettings: ClientSettings = { sidebarThreadSortOrder: "created_at", sidebarThreadPreviewCount: 6, legacySidebarEnabled: false, + sidebarWorkingShelfEnabled: false, loadBalancingEnabled: false, loadBalancingWeights: { "environment-1": 75, "environment-2": 0 }, pullRequestMergeMethodOverrides: {}, diff --git a/apps/mobile/src/features/cloud/cloudDebugLog.ts b/apps/mobile/src/features/cloud/cloudDebugLog.ts index 7e57b39b7bb88..e2b904520bf2b 100644 --- a/apps/mobile/src/features/cloud/cloudDebugLog.ts +++ b/apps/mobile/src/features/cloud/cloudDebugLog.ts @@ -5,10 +5,6 @@ const logger = createDebugLogger("cloud", { legacyGlobalFlag: "__T3_CLOUD_DEBUG__", }); -export function isCloudDebugEnabled(): boolean { - return logger.isEnabled(); -} - export function cloudDebugLog(event: string, data?: Record): void { logger.log(event, data); } diff --git a/apps/mobile/src/features/terminal/terminalDebugLog.ts b/apps/mobile/src/features/terminal/terminalDebugLog.ts index 31c8497915c39..2d7e5138cac1d 100644 --- a/apps/mobile/src/features/terminal/terminalDebugLog.ts +++ b/apps/mobile/src/features/terminal/terminalDebugLog.ts @@ -12,10 +12,6 @@ const logger = createDebugLogger("terminal", { legacyGlobalFlag: "__T3_TERMINAL_DEBUG__", }); -export function isTerminalDebugEnabled(): boolean { - return logger.isEnabled(); -} - export function terminalDebugLog(message: string, data?: Record): void { logger.log(message, data); } diff --git a/apps/mobile/src/features/threads/NewTaskContextPickerScreens.tsx b/apps/mobile/src/features/threads/NewTaskContextPickerScreens.tsx index 17f9a42bd55ef..458d242978bf5 100644 --- a/apps/mobile/src/features/threads/NewTaskContextPickerScreens.tsx +++ b/apps/mobile/src/features/threads/NewTaskContextPickerScreens.tsx @@ -1,7 +1,9 @@ import { MaterialListRow } from "../../components/MaterialListRow"; import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; import type { VcsRef } from "@t3tools/client-runtime/state/vcs"; -import { resolveEnvironmentMachineKind } from "@t3tools/contracts"; +import { type EnvironmentId, resolveEnvironmentMachineKind } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import { AsyncResult } from "effect/unstable/reactivity"; import { LegendList } from "@legendapp/list/react-native"; import { isAtomCommandInterrupted, @@ -29,7 +31,8 @@ import { EnvironmentMachineSymbol } from "../../components/EnvironmentMachineSym import { ThemedSwitch } from "../../components/ThemedSwitch"; import { cn } from "../../lib/cn"; import { NativeHeaderToolbar, NativeStackScreenOptions } from "../../native/StackHeader"; -import { useServerConfigs } from "../../state/entities"; +import { useServerConfigs, waitForProject } from "../../state/entities"; +import { projectEnvironment } from "../../state/projects"; import { useAtomCommand } from "../../state/use-atom-command"; import { vcsEnvironment } from "../../state/vcs"; import { @@ -204,6 +207,38 @@ export function NewTaskEnvironmentPickerRouteScreen() { const navigation = useNavigation(); const insets = useSafeAreaInsets(); const serverConfigs = useServerConfigs(); + const ensureScratch = useAtomCommand(projectEnvironment.ensureScratch, { + reportFailure: false, + }); + const [movingToEnvironmentId, setMovingToEnvironmentId] = useState(null); + + // A thread without a project moves to the other machine's own Scratch + // project, which is created there first if it does not exist yet. + async function moveScratchDraft(environmentId: EnvironmentId): Promise { + setMovingToEnvironmentId(environmentId); + try { + const result = await ensureScratch({ environmentId, input: {} }); + if (AsyncResult.isFailure(result)) { + const error = Cause.squash(result.cause); + Alert.alert( + "Could not switch machine", + error instanceof Error + ? error.message + : "The folder for threads without a project could not be created.", + ); + return; + } + const project = await waitForProject({ environmentId, projectId: result.value.projectId }); + if (project === null) { + Alert.alert("Could not switch machine", "It has not reached this device yet. Try again."); + return; + } + flow.setProject(project); + navigation.goBack(); + } finally { + setMovingToEnvironmentId(null); + } + } return ( @@ -244,9 +279,17 @@ export function NewTaskEnvironmentPickerRouteScreen() { /> } isLast={index === flow.environments.length - 1} + disabled={movingToEnvironmentId !== null} onPress={() => { void Haptics.selectionAsync(); - flow.selectEnvironment(environment.environmentId); + if (flow.isScratchDraft) { + if (environment.environmentId !== flow.selectedEnvironmentId) { + void moveScratchDraft(environment.environmentId); + return; + } + } else { + flow.selectEnvironment(environment.environmentId); + } navigation.goBack(); }} selected={flow.selectedEnvironmentId === environment.environmentId} diff --git a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx index 0f4310ce57d93..1ee4e9bc4e852 100644 --- a/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskDraftScreen.tsx @@ -1459,7 +1459,49 @@ export function NewTaskDraftScreen(props: { navigation.dispatch(StackActions.push(routeName)); }; - const hero = ( + const environmentControl = ( + ( + + )} + label={`on ${selectedEnvironmentLabel}`} + maxWidth={flow.isScratchDraft ? 170 : 260} + onPress={ + flow.environments.length > 1 ? () => openContextPicker("NewTaskEnvironment") : undefined + } + showChevron={flow.environments.length > 1} + static={flow.environments.length <= 1} + /> + ); + // A thread without a project has no project to name, so it asks plainly, + // like web, and puts the project picker beside the machine as a control. + const hero = flow.isScratchDraft ? ( + + + What should we work on? + + {/* Wraps onto two lines only when a long machine name leaves no room. */} + + + {environmentControl} + + + ) : ( @@ -1486,25 +1528,7 @@ export function NewTaskDraftScreen(props: { - ( - - )} - label={`on ${selectedEnvironmentLabel}`} - maxWidth={260} - onPress={ - flow.environments.length > 1 ? () => openContextPicker("NewTaskEnvironment") : undefined - } - showChevron={flow.environments.length > 1} - static={flow.environments.length <= 1} - /> + {environmentControl} ); const heroViewport = ( diff --git a/apps/mobile/src/features/threads/NewTaskRouteScreen.tsx b/apps/mobile/src/features/threads/NewTaskRouteScreen.tsx index 212846e26395c..80a35eabd1c42 100644 --- a/apps/mobile/src/features/threads/NewTaskRouteScreen.tsx +++ b/apps/mobile/src/features/threads/NewTaskRouteScreen.tsx @@ -154,7 +154,16 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps + !scope.projects.every((project) => + isScratchProject(project, serverConfigs.get(project.environmentId)?.scratchWorkspaceRoot), + ), + ); + const visibleScopes = filterProjectScopes(listScopes, searchText); const resumedDestinationKeyRef = useRef(null); const reservedDestinationProject = incomingShare?.destination ? (projects.find( @@ -163,14 +172,13 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps canCreateProjectInEnvironment(environment.connectionState) && @@ -182,24 +190,7 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps - project.environmentId === scratchEnvironment?.environmentId && - isScratchProject(project, scratchWorkspaceRoot), - ); - const canStartScratch = scratchWorkspaceRoot !== null && reservedDestinationProject === null; - const scratchMachineLabel = - connectedEnvironments.length > 1 ? (scratchEnvironment?.environmentLabel ?? null) : null; - const startScratchLabel = scratchMachineLabel - ? `Start without a project on ${scratchMachineLabel}` - : "Start without a project"; - const scratchRowSubtitle = scratchMachineLabel - ? `On ${scratchMachineLabel}` - : "Start a task without a project"; + const canStartScratch = scratchEnvironment !== null && reservedDestinationProject === null; const scratchStartInFlightRef = useRef(false); async function selectProject(project: EnvironmentProject): Promise { @@ -321,7 +312,56 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps - {projectScopes.length === 0 ? ( + {canStartScratch && listScopes.length > 0 ? ( + Platform.OS === "android" ? ( + + void startScratch()} + leading={ + + } + /> + + ) : ( + + void startScratch()} + className="flex-row items-center gap-3 bg-card px-4 py-3.5" + > + + + + + No project + + Start a task without a project + + + + + + ) + ) : null} + {listScopes.length === 0 ? ( {canStartScratch ? ( void startScratch()} /> @@ -382,7 +422,7 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps void startScratch()} > - {startScratchLabel} + Start without a project ) : null} @@ -486,55 +526,6 @@ export function NewTaskRouteScreen({ route }: StaticScreenProps )} - {canStartScratch && !scratchProjectExists && projectScopes.length > 0 ? ( - Platform.OS === "android" ? ( - - void startScratch()} - leading={ - - } - /> - - ) : ( - - void startScratch()} - className="flex-row items-center gap-3 bg-card px-4 py-3.5" - > - - - - - No project - - {scratchRowSubtitle} - - - - - - ) - ) : null} diff --git a/apps/mobile/src/features/threads/new-task-flow-provider.tsx b/apps/mobile/src/features/threads/new-task-flow-provider.tsx index 5b2e6888e1d28..18c023b45ee11 100644 --- a/apps/mobile/src/features/threads/new-task-flow-provider.tsx +++ b/apps/mobile/src/features/threads/new-task-flow-provider.tsx @@ -24,7 +24,12 @@ import { parseT3ProjectFile } from "@t3tools/shared/t3ProjectFile"; import * as Arr from "effect/Array"; import { pipe } from "effect/Function"; -import { useEnvironmentServerConfig, useProjects, useThreadShells } from "../../state/entities"; +import { + useEnvironmentServerConfig, + useProjects, + useServerConfigs, + useThreadShells, +} from "../../state/entities"; import type { TurnCommandMetadata } from "../../lib/commandMetadata"; import type { DraftComposerAttachment } from "../../lib/composerImages"; import type { ModelOption, ProviderGroup } from "../../lib/modelOptions"; @@ -81,8 +86,10 @@ import { } from "../../state/use-thread-outbox"; import { setPendingConnectionError, + useRemoteConnectionStatus, useSavedRemoteConnections, } from "../../state/use-remote-environment-registry"; +import { canCreateProjectInEnvironment } from "@t3tools/client-runtime/operations/projects"; import { isScratchProject } from "@t3tools/client-runtime/state/projects"; import { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; import { type VcsRef } from "@t3tools/client-runtime/state/vcs"; @@ -177,6 +184,8 @@ type NewTaskFlowContextValue = { readonly environmentLabel: string; }>; readonly selectedProject: EnvironmentProject | null; + /** True when the draft is a thread without a project (its machine's Scratch project). */ + readonly isScratchDraft: boolean; readonly modelOptions: ReadonlyArray; readonly selectedModel: ModelSelection | null; readonly selectedModelOption: ModelOption | null; @@ -344,6 +353,26 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { ? editingPendingProject : (projectsForEnvironment[0] ?? null)); + const selectedEnvironmentServerConfig = useEnvironmentServerConfig( + selectedProject?.environmentId ?? null, + ); + const isScratchDraft = + selectedProject !== null && + isScratchProject(selectedProject, selectedEnvironmentServerConfig?.scratchWorkspaceRoot); + const serverConfigs = useServerConfigs(); + const { connectedEnvironments } = useRemoteConnectionStatus(); + // A thread without a project can move to any connected machine that offers + // one; its Scratch project there is created on the switch if it is missing. + const scratchEnvironments = useMemo( + () => + connectedEnvironments.filter( + (environment) => + canCreateProjectInEnvironment(environment.connectionState) && + serverConfigs.get(environment.environmentId)?.scratchWorkspaceRoot !== undefined, + ), + [connectedEnvironments, serverConfigs], + ); + // Only offer machines that actually host the currently selected repository, so // switching computers moves the same repo across machines instead of jumping to // whatever unrelated project happens to be first on the other machine. Repository @@ -355,6 +384,12 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { const selectedWorkspaceBasename = selectedProject?.workspaceRoot.split("/").at(-1) || null; const selectedProjectTitle = selectedProject?.title ?? null; const environments = useMemo(() => { + if (isScratchDraft) { + return scratchEnvironments.map((environment) => ({ + environmentId: environment.environmentId, + environmentLabel: environment.environmentLabel, + })); + } const seen = new Set(); const result: Array<{ readonly environmentId: EnvironmentId; @@ -392,16 +427,15 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { } return result; }, [ + isScratchDraft, projects, savedConnectionsById, + scratchEnvironments, selectedRepositoryKey, selectedWorkspaceBasename, selectedProjectTitle, ]); - const selectedEnvironmentServerConfig = useEnvironmentServerConfig( - selectedProject?.environmentId ?? null, - ); // While a queued pending task is being edited its draft lives under a key // scoped to the queued message, so new-task drafts stay intact. const selectedProjectDraftKey = editingPendingTask @@ -462,10 +496,7 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { ); // A thread without a project runs in a plain folder, so worktree mode // would leave it unsendable: it is always local and offers no choice. - const canChooseWorkspace = !( - selectedProject !== null && - isScratchProject(selectedProject, selectedEnvironmentServerConfig?.scratchWorkspaceRoot) - ); + const canChooseWorkspace = !isScratchDraft; const defaultWorkspaceMode: WorkspaceMode = canChooseWorkspace ? projectSettings.settings.defaultThreadEnvMode : "local"; @@ -1200,6 +1231,7 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { expandedProvider, environments, selectedProject, + isScratchDraft, modelOptions, selectedModel, selectedModelOption, @@ -1249,6 +1281,7 @@ export function NewTaskFlowProvider(props: React.PropsWithChildren) { filteredBranches, finishEditingPendingTask, interactionMode, + isScratchDraft, planModeEnabled, loadBranches, loadMoreBranches, diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index fa453d54dc6c5..1b6b440084673 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -13,6 +13,7 @@ import * as Logger from "effect/Logger"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as References from "effect/References"; +import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; import { TestClock } from "effect/testing"; @@ -36,6 +37,7 @@ import { } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import { decodeGitHubPullRequestListJson } from "../sourceControl/gitHubPullRequests.ts"; import * as GitLabCli from "../sourceControl/GitLabCli.ts"; import * as TextGeneration from "../textGeneration/TextGeneration.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; @@ -517,6 +519,30 @@ function createGitHubCliWithFakeGh(scenario: FakeGhScenario = {}): { return { service: { execute, + // The fake answers the CLI shape, so batched lookups read it the way the fallback does. + listPullRequestsByHead: (input) => + execute({ + cwd: input.cwd, + args: [ + "pr", + "list", + "--head", + input.headSelector, + "--state", + input.state, + "--limit", + String(input.limit), + "--json", + "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + ], + }).pipe( + Effect.map((result) => { + const raw = result.stdout.trim(); + if (raw.length === 0) return []; + const decoded = decodeGitHubPullRequestListJson(raw); + return Result.isSuccess(decoded) ? decoded.success : []; + }), + ), listOpenPullRequests: (input) => execute({ cwd: input.cwd, diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts index 524afd9d8646f..7871b3ed6ee39 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.test.ts @@ -637,6 +637,88 @@ describe("PullRequestSyncReactor", () => { ), ); + it.effect("reads a burst of requested links in one sweep", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const reading = yield* Deferred.make(); + const release = yield* Deferred.make(); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([]), + summary: (input) => + Effect.gen(function* () { + if (input.number === 1) { + yield* Deferred.succeed(reading, undefined); + yield* Deferred.await(release); + } + return makeSummary(input); + }), + }); + yield* Effect.gen(function* () { + const reactor = yield* startAndSweep(fixture); + yield* Ref.set( + fixture.snapshots, + makeSnapshot( + [1, 2, 3, 4].map((number) => + makeThread(`thread-${number}`, { pullRequests: [makeLink(number)] }), + ), + ), + ); + const request = (number: number) => + reactor.requestSync({ host: "github.com", repository: "owner/repository", number }); + yield* request(1); + yield* Deferred.await(reading); + yield* Effect.forEach([2, 3, 4], request, { discard: true }); + yield* Deferred.succeed(release, undefined); + yield* reactor.drain; + // One sweep for the first link and one for the three that arrived while it read. + assert.strictEqual(yield* Queue.size(fixture.snapshotReads), 2); + assert.deepStrictEqual( + (yield* Ref.get(fixture.summaryCalls)).map((ref) => ref.number).toSorted(), + [1, 2, 3, 4], + ); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); + + it.effect("skips the stack read for a pull request the summary places in no stack", () => + Effect.scoped( + Effect.gen(function* () { + yield* TestClock.setTime(Date.parse(NOW)); + const fixture = yield* makeHarness({ + snapshot: makeSnapshot([ + makeThread("unstacked", { pullRequests: [makeLink(1)] }), + makeThread("stacked", { pullRequests: [makeLink(2)] }), + makeThread("unknown", { pullRequests: [makeLink(3)] }), + ]), + summary: (input) => + Effect.succeed( + makeSummary( + input, + input.number === 1 + ? { stack: null } + : input.number === 2 + ? { stack: { number: 9, position: 1, size: 2, base: "main" } } + : {}, + ), + ), + }); + yield* Effect.gen(function* () { + yield* startAndSweep(fixture); + assert.deepStrictEqual( + (yield* Ref.get(fixture.stackCalls)).map((ref) => ref.number).toSorted(), + [2, 3], + ); + assert.deepStrictEqual( + (yield* Ref.get(fixture.syncCommands)).map((command) => command.number).toSorted(), + [1, 2, 3], + ); + }).pipe(Effect.provide(fixture.layer)); + }), + ), + ); + it.effect("polls open pull requests on settled threads every fifteen minutes", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts index af6b5c2972ba0..670d401004b6c 100644 --- a/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestSyncReactor.ts @@ -133,6 +133,9 @@ export const make = Effect.gen(function* () { const lastSyncedAt = new Map(); const requested = new Map(); let requestGeneration = 0; + // Requested keys wait in `requested` for one queued sweep, so a burst of links (an agent + // linking dozens of pull requests) is read together and shares the summary batches. + let requestedSweepQueued = false; const retryStacks = new Set(); const isDue = (key: string, entries: ReadonlyArray, nowMs: number): boolean => { @@ -151,7 +154,8 @@ export const make = Effect.gen(function* () { (cause: Cause.Cause): Effect.Effect => Cause.hasInterruptsOnly(cause) ? Effect.failCause(cause) : Effect.logWarning(message, fields); - const sweep = Effect.fn("PullRequestSyncReactor.sweep")(function* (requestedKey?: string) { + /** `requested` reads only keys asked for through `requestSync`; `all` is the periodic pass. */ + const sweep = Effect.fn("PullRequestSyncReactor.sweep")(function* (scope: "all" | "requested") { const threads = yield* projections.getThreadsWithPullRequests(); const now = yield* DateTime.now; const nowMs = DateTime.toEpochMillis(now); @@ -252,22 +256,31 @@ export const make = Effect.gen(function* () { retryStacks.has(key) || entries.some( (entry) => - entry.link.snapshot === null || !snapshotFieldsEqual(entry.link.snapshot, fields), + entry.link.snapshot === null || + !snapshotFieldsEqual(entry.link.snapshot, fields) || + (summary.stack !== undefined && + (entry.link.stack?.number ?? null) !== (summary.stack?.number ?? null)), ); - const fetchedStack = needsStack - ? yield* pullRequests.stack(ref, { includeDetails: false }).pipe( - Effect.map((stack) => ({ - stack: stack === null ? null : ({ kind: "native", ...stack } as const), - })), - Effect.catchCauseIf( - (cause) => !Cause.hasInterruptsOnly(cause), - () => - Effect.logWarning("pull request stack lookup failed", { - key, - }).pipe(Effect.as(null)), - ), - ) - : null; + // A summary that says the pull request is in no stack, for links that hold none, already + // answers what the stack read would. + const knownUnstacked = + summary.stack === null && entries.every((entry) => entry.link.stack === null); + const fetchedStack = !needsStack + ? null + : knownUnstacked + ? { stack: null } + : yield* pullRequests.stack(ref, { includeDetails: false }).pipe( + Effect.map((stack) => ({ + stack: stack === null ? null : ({ kind: "native", ...stack } as const), + })), + Effect.catchCauseIf( + (cause) => !Cause.hasInterruptsOnly(cause), + () => + Effect.logWarning("pull request stack lookup failed", { + key, + }).pipe(Effect.as(null)), + ), + ); if (needsStack) { if (fetchedStack === null) { retryStacks.add(key); @@ -298,7 +311,7 @@ export const make = Effect.gen(function* () { yield* Effect.forEach( groups, ([key, entries]) => - (requestedKey === undefined || requestedKey === key) && isDue(key, entries, nowMs) + (scope === "all" || requested.has(key)) && isDue(key, entries, nowMs) ? syncGroup(key, entries).pipe( Effect.catchCause(logSkipped("pull request sync skipped", { key })), ) @@ -309,8 +322,12 @@ export const make = Effect.gen(function* () { ); }); - const worker = yield* makeDrainableWorker((key: string | undefined) => - sweep(key).pipe(Effect.catchCause(logSkipped("pull request sync sweep failed", {}))), + const worker = yield* makeDrainableWorker((scope: "all" | "requested") => + Effect.suspend(() => { + // Requests from here on queue another sweep; the ones already recorded are read by this. + if (scope === "requested") requestedSweepQueued = false; + return sweep(scope); + }).pipe(Effect.catchCause(logSkipped("pull request sync sweep failed", {}))), ); const start: PullRequestSyncReactor["Service"]["start"] = Effect.fn( @@ -332,7 +349,7 @@ export const make = Effect.gen(function* () { ); yield* forkParked( Effect.gen(function* () { - yield* worker.enqueue(undefined); + yield* worker.enqueue("all"); yield* worker.drain; }).pipe(Effect.repeat(Schedule.spaced("1 minute")), Effect.asVoid), ); @@ -340,9 +357,10 @@ export const make = Effect.gen(function* () { const requestSync: PullRequestSyncReactor["Service"]["requestSync"] = (key) => Effect.suspend(() => { - const syncKey = threadPullRequestKeyOf(key); - requested.set(syncKey, ++requestGeneration); - return worker.enqueue(syncKey); + requested.set(threadPullRequestKeyOf(key), ++requestGeneration); + if (requestedSweepQueued) return Effect.void; + requestedSweepQueued = true; + return worker.enqueue("requested"); }); return { start, drain: worker.drain, requestSync } satisfies PullRequestSyncReactor["Service"]; diff --git a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts index fff3688fddc74..072bf930c038e 100644 --- a/apps/server/src/orchestration-v2/ThreadPullRequestService.ts +++ b/apps/server/src/orchestration-v2/ThreadPullRequestService.ts @@ -345,8 +345,9 @@ export const make = Effect.gen(function* () { }).pipe(Effect.tap(() => Effect.sync(() => failBackfill(group)))), ), ), - // Match a batched summary read so host lookups arrive together. - { concurrency: 25, discard: true }, + // Wide enough that a sweep's GitHub branch lookups reach GitHubCli together and share one + // GraphQL document, instead of one `gh pr list` per branch. + { concurrency: 32, discard: true }, ); }); diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 9cf130123fe83..020b571fe248c 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -1825,7 +1825,12 @@ export const make = Effect.gen(function* () { const batchable = entries.filter( (entry) => buildPullRequestSummariesGraphQlQuery([entry.request]) !== null, ); - const query = buildPullRequestSummariesGraphQlQuery(batchable.map((entry) => entry.request)); + // Stack membership rides along where GitHub serves stacks, so the background sync can skip + // the REST stack read for pull requests that are in none. + const query = buildPullRequestSummariesGraphQlQuery( + batchable.map((entry) => entry.request), + first.request.host === "github.com", + ); const batched = query === null ? Effect.succeed(new Map()) diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index 1d511b61fbfcc..ed4312c125c7b 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -117,6 +117,11 @@ export interface ProviderChangeRequestSummary { readonly reviewDecision?: PullRequestReviewDecision | null | undefined; readonly checksState?: PullRequestChecksState | null | undefined; readonly mergeability?: PullRequestMergeability | undefined; + /** + * The host-native stack the pull request sits in, from the same read. Null when the host says + * it is in none; absent when the read did not ask. + */ + readonly stack?: PullRequestStackMembership | null | undefined; } /** One layer of a host-native stack, bottom to top order is the array's. */ diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index 917821c21353b..44c175ab652f7 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1619,6 +1619,7 @@ export const make = Effect.gen(function* () { ...(changeRequest.mergeability === undefined ? {} : { mergeability: changeRequest.mergeability }), + ...(changeRequest.stack === undefined ? {} : { stack: changeRequest.stack }), })), ); }), diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 7008ca7391500..12239a2af0ee5 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -1934,5 +1934,43 @@ describe("batched pull request summaries", () => { mergeability: "unknown", additions: 0, }); + // The document did not ask about stacks, so the summary does not claim an answer. + expect(decoded.success.get(0)).not.toHaveProperty("stack"); + }); + + it("reads stack membership only where the document asked for it", () => { + expect( + buildPullRequestSummariesGraphQlQuery([{ repository: "acme/web", number: 7 }], true), + ).toContain("stack { number size baseRefName } stackEntry { position }"); + expect( + buildPullRequestSummariesGraphQlQuery([{ repository: "acme/web", number: 7 }]), + ).not.toContain("stack {"); + const pullRequest = (number: number, stack: Record) => ({ + pullRequest: { + number, + title: "Stacked", + url: `https://github.com/acme/web/pull/${number}`, + headRefName: `feat/${number}`, + baseRefName: "main", + state: "OPEN", + updatedAt: "2026-08-24T00:00:00Z", + ...stack, + }, + }); + const decoded = expectSuccess( + decodePullRequestSummariesJson( + JSON.stringify({ + data: { + s0: pullRequest(7, { stack: null, stackEntry: null }), + s1: pullRequest(8, { + stack: { number: 3, size: 2, baseRefName: "main" }, + stackEntry: { position: 2 }, + }), + }, + }), + ), + ); + expect(decoded.get(0)?.stack).toBeNull(); + expect(decoded.get(1)?.stack).toEqual({ number: 3, size: 2, base: "main", position: 2 }); }); }); diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 9be1b2a5844bb..8e3a9ee330b87 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -1852,6 +1852,8 @@ export function decodePullRequestStatsJson( * shape as the search row where the two overlap: the checks arrive as GitHub's one-word rollup * rather than the whole check list `gh pr view` hands back, which is what keeps a batch cheap. */ +const STACK_MEMBERSHIP_SELECTION = "stack { number size baseRefName } stackEntry { position }"; + const PULL_REQUEST_SUMMARY_SELECTION = "number title url state isDraft mergeable reviewDecision additions deletions changedFiles " + "updatedAt mergedAt closedAt headRefName baseRefName " + @@ -1865,6 +1867,7 @@ const PULL_REQUEST_SUMMARY_SELECTION = */ export function buildPullRequestSummariesGraphQlQuery( changeRequests: ReadonlyArray<{ readonly repository: string; readonly number: number }>, + includeStacks = false, ): string | null { if (changeRequests.length === 0) return null; const selections: string[] = []; @@ -1874,7 +1877,7 @@ export function buildPullRequestSummariesGraphQlQuery( if (!REPOSITORY_PART.test(owner) || !REPOSITORY_PART.test(name)) return null; if (!Number.isSafeInteger(changeRequest.number) || changeRequest.number <= 0) return null; selections.push( - ` s${index}: repository(owner: "${owner}", name: "${name}") { pullRequest(number: ${changeRequest.number}) { ${PULL_REQUEST_SUMMARY_SELECTION} } }`, + ` s${index}: repository(owner: "${owner}", name: "${name}") { pullRequest(number: ${changeRequest.number}) { ${PULL_REQUEST_SUMMARY_SELECTION}${includeStacks ? ` ${STACK_MEMBERSHIP_SELECTION}` : ""} } }`, ); } return `query PullRequestSummaries {\n${selections.join("\n")}\n}`; @@ -1920,6 +1923,8 @@ export interface GitHubPullRequestSummary { readonly reviewDecision: PullRequestReviewDecision | null; readonly checksState: PullRequestChecksState | null; readonly mergeability: PullRequestMergeability; + /** Null when GitHub says the pull request is in no stack; absent when the read did not ask. */ + readonly stack?: PullRequestStackMembership | null; } /** @@ -1965,6 +1970,7 @@ export function decodePullRequestSummariesJson( }), ), mergeability: toMergeability(pr.mergeable), + ...(pr.stack === undefined ? {} : { stack: toStackMembership(pr) ?? null }), }); } return Result.succeed(summaries); diff --git a/apps/server/src/sourceControl/GitHubCli.test.ts b/apps/server/src/sourceControl/GitHubCli.test.ts index f28d2f8767d10..b604ec568aebe 100644 --- a/apps/server/src/sourceControl/GitHubCli.test.ts +++ b/apps/server/src/sourceControl/GitHubCli.test.ts @@ -4,6 +4,7 @@ import * as TestClock from "effect/testing/TestClock"; import * as Clock from "effect/Clock"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; import * as Redacted from "effect/Redacted"; @@ -31,13 +32,18 @@ const quotaOutput = (remaining = 5000, resetAt = "2099-01-01T00:00:00Z") => JSON.stringify({ data: { rateLimit: { cost: 1, limit: 5000, remaining, resetAt } } }), ); +const isBudgetReading = (input: VcsProcess.VcsProcessInput) => + input.args[0] === "api" && + input.args[1] === "graphql" && + input.args.at(-1)?.includes("rateLimit"); + const mockRun = vi.fn(); +// Budget readings are answered here, so `mockRun` sees only the commands under test. const layer = GitHubCli.layer.pipe( Layer.provide( Layer.mock(VcsProcess.VcsProcess)({ - run: (input) => - input.args[1] === "rate_limit" ? Effect.succeed(quotaOutput()) : mockRun(input), + run: (input) => (isBudgetReading(input) ? Effect.succeed(quotaOutput()) : mockRun(input)), }), ), ); @@ -46,9 +52,9 @@ afterEach(() => { mockRun.mockReset(); }); -it.effect("shares quota checks, preserves the reserve, and resumes after reset", () => +it.effect("reads the GraphQL budget once per window, preserves the reserve, and resumes", () => Effect.gen(function* () { - let probes = 0; + let readings = 0; const commands: string[] = []; let remaining = 501; let resetAt = DateTime.formatIso( @@ -58,8 +64,8 @@ it.effect("shares quota checks, preserves the reserve, and resumes after reset", Effect.provideService(VcsProcess.VcsProcess, { run: (input) => Effect.sync(() => { - if (input.args[1] === "rate_limit") { - probes++; + if (isBudgetReading(input)) { + readings++; assert.strictEqual(input.args[3], "enterprise.test"); return quotaOutput(remaining, resetAt); } @@ -79,24 +85,320 @@ it.effect("shares quota checks, preserves the reserve, and resumes after reset", yield* read("list"); const failure = yield* read("view").pipe(Effect.flip); assert.strictEqual(failure._tag, "GitHubCliRateLimitError"); - assert.strictEqual(probes, 1); assert.deepStrictEqual(commands, ["pr list"]); yield* read("view").pipe(Effect.provideService(GitHubCli.AllowGitHubReserve, true)); yield* gh.execute({ cwd: "/repo", args: ["pr", "merge", "1"] }); assert.deepStrictEqual(commands, ["pr list", "pr view", "pr merge"]); - remaining = 0; - yield* TestClock.adjust("30 seconds"); - yield* read("repo").pipe(Effect.flip); - assert.strictEqual(probes, 2); - yield* TestClock.adjust("30 seconds"); + // One reading covers the whole window. + assert.strictEqual(readings, 1); + yield* TestClock.adjust("1 minute"); remaining = 5000; resetAt = DateTime.formatIso(DateTime.makeUnsafe((yield* Clock.currentTimeMillis) + 60_000)); yield* Effect.all([read("list"), read("repo")], { concurrency: 2 }); - assert.strictEqual(probes, 3); + assert.strictEqual(readings, 2); assert.deepStrictEqual(commands.slice(3).toSorted(), ["pr list", "repo view"]); }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), ); +it.effect("reads the budget again at a near reset, and every ten minutes in a long window", () => + Effect.gen(function* () { + let readings = 0; + const startedAt = yield* Clock.currentTimeMillis; + let resetAt = DateTime.formatIso(DateTime.makeUnsafe(startedAt + 10_000)); + const gh = yield* GitHubCli.make.pipe( + Effect.provideService(VcsProcess.VcsProcess, { + run: (input) => + Effect.sync(() => { + if (!isBudgetReading(input)) return processOutput("[]"); + readings++; + return quotaOutput(5000, resetAt); + }), + }), + ); + const read = gh.execute({ cwd: "/repo", args: ["pr", "list"] }); + yield* read; + // The window resets ten seconds in, so the reading expires with it. + resetAt = DateTime.formatIso(DateTime.makeUnsafe(startedAt + 10_000 + 3_600_000)); + yield* TestClock.adjust("10 seconds"); + yield* read; + assert.strictEqual(readings, 2); + yield* TestClock.adjust("9 minutes"); + yield* read; + assert.strictEqual(readings, 2); + yield* TestClock.adjust("1 minute"); + yield* read; + assert.strictEqual(readings, 3); + }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), +); + +it.effect("reads anyway when the budget reading fails", () => + Effect.gen(function* () { + const gh = yield* GitHubCli.make.pipe( + Effect.provideService(VcsProcess.VcsProcess, { + run: (input) => + isBudgetReading(input) + ? Effect.fail( + new VcsProcessSpawnError({ + operation: "GitHubCli.execute", + command: "gh", + cwd: "/gone", + cause: new Error("ENOENT"), + }), + ) + : Effect.succeed(processOutput("[]")), + }), + ); + const result = yield* gh.execute({ cwd: "/repo", args: ["pr", "list"] }); + assert.strictEqual(result.stdout, "[]"); + }).pipe(Effect.provide(Layer.merge(GitHubGraphQlBudget.layer, SourceControlRateLimit.layer))), +); + +describe("selectGitHubBaseRepository", () => { + const remotes = (...entries: ReadonlyArray) => + entries + .flatMap(([name, url]) => [`${name}\t${url} (fetch)`, `${name}\t${url} (push)`]) + .join("\n"); + const select = (input: { remotes: string; resolved?: string }) => + GitHubCli.selectGitHubBaseRepository({ resolved: "", host: "github.com", ...input }); + + it("picks the repository gh reads without a prompt", () => { + assert.deepStrictEqual(select({ remotes: remotes(["fork", "git@github.com:me/web.git"]) }), { + owner: "me", + name: "web", + }); + const fork = remotes( + ["origin", "git@github.com:me/web.git"], + ["upstream", "https://github.com/Acme/Web.git"], + ); + assert.deepStrictEqual(select({ remotes: fork }), { owner: "acme", name: "web" }); + assert.deepStrictEqual(select({ remotes: fork, resolved: "remote.origin.gh-resolved base" }), { + owner: "me", + name: "web", + }); + assert.deepStrictEqual( + select({ remotes: fork, resolved: "remote.origin.gh-resolved acme/other" }), + { owner: "acme", name: "other" }, + ); + // gh ranks remote names in any case. + assert.deepStrictEqual( + select({ + remotes: remotes( + ["origin", "git@github.com:me/web.git"], + ["Upstream", "git@github.com:acme/web.git"], + ), + }), + { owner: "acme", name: "web" }, + ); + }); + + it("leaves gh to choose when it might weigh the remotes differently", () => { + for (const input of [ + { remotes: "" }, + { + remotes: remotes(["a", "git@github.com:me/web.git"], ["b", "git@github.com:acme/web.git"]), + }, + { + remotes: remotes( + ["origin", "git@github.com:me/web.git"], + ["mirror", "git@gitlab.com:me/web.git"], + ), + }, + { remotes: remotes(["origin", "git@github-work:me/web.git"]) }, + { + remotes: remotes( + ["origin", "git@github.com:me/web.git"], + ["Origin", "git@github.com:acme/web.git"], + ), + }, + { + remotes: remotes( + ["origin", "git@github.com:me/web.git"], + ["upstream", "git@github.com:acme/web.git"], + ), + resolved: "remote.origin.gh-resolved base\nremote.upstream.gh-resolved base", + }, + ]) { + assert.strictEqual(select(input), null); + } + }); +}); + +describe("GitHubCli.listPullRequestsByHead", () => { + const remoteOutput = + "origin\tgit@github.com:acme/web.git (fetch)\norigin\tgit@github.com:acme/web.git (push)\n"; + const node = (number: number, headRefName: string) => ({ + number, + title: `PR ${number}`, + url: `https://github.com/acme/web/pull/${number}`, + baseRefName: "main", + headRefName, + state: "MERGED", + mergedAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-02T00:00:00Z", + headRepository: { name: "web", nameWithOwner: "acme/web" }, + headRepositoryOwner: { login: "acme" }, + }); + const decodeRequest = Schema.decodeSync( + Schema.fromJsonString( + Schema.Struct({ + query: Schema.String, + variables: Schema.Record(Schema.String, Schema.Unknown), + }), + ), + ); + const jsonOutput = (value: unknown) => processOutput(JSON.stringify(value)); + const git = (input: VcsProcess.VcsProcessInput) => + input.args[0] === "remote" + ? processOutput(remoteOutput) + : { ...processOutput(""), exitCode: ChildProcessSpawner.ExitCode(1) }; + + it.effect("reads heads on one repository in one GraphQL document", () => + Effect.gen(function* () { + const documents: Array<{ query: string; variables: Record }> = []; + mockRun.mockImplementation((input) => + Effect.sync(() => { + if (input.command === "git") return git(input); + documents.push(decodeRequest(input.stdin ?? "")); + return jsonOutput({ + data: { + repository: { h0: { nodes: [node(7, "feature/a")] }, h1: { nodes: [] } }, + rateLimit: { cost: 1, limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, + }, + }); + }), + ); + const gh = yield* GitHubCli.GitHubCli; + const lookups = yield* Effect.all( + ["feature/a", "feature/b"].map((headSelector) => + gh.listPullRequestsByHead({ + cwd: "/repo", + headSelector, + state: "all", + limit: 100, + rateLimitHost: "github.com", + }), + ), + { concurrency: "unbounded" }, + ).pipe(Effect.forkChild); + yield* TestClock.adjust("50 millis"); + const [first, second] = yield* Fiber.join(lookups); + assert.deepStrictEqual( + first?.map((pr) => [pr.number, pr.state, pr.headRepositoryNameWithOwner]), + [[7, "merged", "acme/web"]], + ); + assert.deepStrictEqual(second, []); + assert.strictEqual(documents.length, 1); + assert.include(documents[0]!.query, "rateLimit"); + assert.deepStrictEqual(documents[0]!.variables, { + owner: "acme", + name: "web", + h0: "feature/a", + s0: ["OPEN", "CLOSED", "MERGED"], + h1: "feature/b", + s1: ["OPEN", "CLOSED", "MERGED"], + }); + }).pipe(Effect.provide(layer)), + ); + + it.effect("asks gh pr list when gh could read another repository", () => + Effect.gen(function* () { + const commands: Array> = []; + mockRun.mockImplementation((input) => + Effect.sync(() => { + commands.push([input.command, ...input.args]); + if (input.command === "git") { + return processOutput( + input.args[0] === "remote" + ? "a\tgit@github.com:me/web.git (fetch)\nb\tgit@github.com:acme/web.git (fetch)\n" + : "", + ); + } + return input.args[3] === "feature/empty" + ? processOutput("") + : jsonOutput([node(8, "feature/a")]); + }), + ); + const gh = yield* GitHubCli.GitHubCli; + const pullRequests = yield* gh.listPullRequestsByHead({ + cwd: "/repo", + headSelector: "feature/a", + state: "all", + limit: 100, + rateLimitHost: "github.com", + }); + assert.deepStrictEqual( + pullRequests.map((pr) => pr.number), + [8], + ); + assert.deepStrictEqual(commands.at(-1), [ + "gh", + "pr", + "list", + "--head", + "feature/a", + "--state", + "all", + "--limit", + "100", + "--json", + "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", + ]); + const empty = yield* gh.listPullRequestsByHead({ + cwd: "/repo", + headSelector: "feature/empty", + state: "all", + limit: 100, + rateLimitHost: "github.com", + }); + assert.deepStrictEqual(empty, []); + }).pipe(Effect.provide(layer)), + ); + + it.effect("fails a rate-limited document whole instead of asking head by head", () => + Effect.gen(function* () { + let ghCalls = 0; + mockRun.mockImplementation((input) => { + if (input.command === "git") return Effect.succeed(git(input)); + ghCalls++; + return Effect.fail( + new VcsProcessExitError({ + operation: "GitHubCli.execute", + command: "gh", + cwd: "/repo", + exitCode: 1, + failureKind: "rate-limited", + detail: "API rate limit exceeded.", + stderrLength: 24, + stderrTruncated: false, + }), + ); + }); + const gh = yield* GitHubCli.GitHubCli; + const lookups = yield* Effect.all( + ["feature/a", "feature/b"].map((headSelector) => + gh + .listPullRequestsByHead({ + cwd: "/repo", + headSelector, + state: "all", + limit: 100, + rateLimitHost: "github.com", + }) + .pipe(Effect.flip), + ), + { concurrency: "unbounded" }, + ).pipe(Effect.forkChild); + yield* TestClock.adjust("50 millis"); + const errors = yield* Fiber.join(lookups); + assert.deepStrictEqual( + errors.map((error) => error._tag), + ["GitHubCliRateLimitError", "GitHubCliRateLimitError"], + ); + assert.strictEqual(ghCalls, 1); + }).pipe(Effect.provide(layer)), + ); +}); + describe("GitHubCli.layer", () => { it.effect("shares the registry budget with CLI reads through nested layer providers", () => Effect.gen(function* () { @@ -116,8 +418,9 @@ describe("GitHubCli.layer", () => { Effect.provideService(VcsProcess.VcsProcess, { run: (input) => Effect.sync(() => { - if (input.args[1] === "rate_limit") + if (isBudgetReading(input)) { return quotaOutput(input.env?.GH_TOKEN === "empty" ? 0 : 5000); + } reads++; return processOutput("[]"); }), diff --git a/apps/server/src/sourceControl/GitHubCli.ts b/apps/server/src/sourceControl/GitHubCli.ts index e1d1627a93183..82e2da846881f 100644 --- a/apps/server/src/sourceControl/GitHubCli.ts +++ b/apps/server/src/sourceControl/GitHubCli.ts @@ -1,4 +1,5 @@ import * as Cache from "effect/Cache"; +import * as Clock from "effect/Clock"; import * as Duration from "effect/Duration"; import * as Exit from "effect/Exit"; import * as Context from "effect/Context"; @@ -8,6 +9,8 @@ import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as Redacted from "effect/Redacted"; +import * as Request from "effect/Request"; +import * as RequestResolver from "effect/RequestResolver"; import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; @@ -16,11 +19,14 @@ import { type SourceControlRepositoryVisibility, type VcsError, } from "@t3tools/contracts"; +import { normalizeGitRemoteUrl } from "@t3tools/shared/git"; +import { decodeJsonResult } from "@t3tools/shared/schemaJson"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; import * as SourceControlRateLimit from "./SourceControlRateLimit.ts"; import { + decodeGitHubPullRequestEntries, decodeGitHubPullRequestJson, decodeGitHubPullRequestListJson, type NormalizedGitHubPullRequestRecord, @@ -301,6 +307,20 @@ export class GitHubCli extends Context.Service< readonly rateLimitHost?: string; }) => Effect.Effect, GitHubCliError>; + /** + * Pull requests whose head is `headSelector`, in the repository `gh pr list` would read in + * `cwd`. Lookups on one repository that arrive together share one GraphQL document; a + * checkout whose repository gh could pick another way is asked through `gh pr list`. + */ + readonly listPullRequestsByHead: (input: { + readonly cwd: string; + readonly headSelector: string; + readonly state: "open" | "closed" | "merged" | "all"; + readonly limit: number; + /** The checkout's GitHub host. Without it the lookup is not batched. */ + readonly rateLimitHost?: string; + }) => Effect.Effect, GitHubCliError>; + readonly getPullRequest: (input: { readonly cwd: string; readonly reference: string; @@ -395,6 +415,149 @@ function deriveRepositoryCloneUrlsFromCreateOutput( }; } +type PullRequestListState = "open" | "closed" | "merged" | "all"; + +const PULL_REQUEST_LIST_JSON_FIELDS = + "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner"; +/** The `gh pr list --json` fields above, as GraphQL selects them. */ +const PULL_REQUEST_NODE_SELECTION = + "number title url baseRefName headRefName state isDraft mergedAt closedAt updatedAt isCrossRepository headRepository { name nameWithOwner } headRepositoryOwner { login }"; +const GRAPHQL_STATES: Record> = { + open: ["OPEN"], + closed: ["CLOSED"], + merged: ["MERGED"], + all: ["OPEN", "CLOSED", "MERGED"], +}; +/** + * Head lookups per GraphQL document. A document of a hundred costs one point, the same as one + * `gh pr list`, but half that keeps each answer near half a second. + */ +const HEAD_LOOKUPS_PER_DOCUMENT = 50; +/** + * How long a head lookup waits for company. Branch discovery reaches GitHub only after each + * branch's own git reads, so lookups started together arrive tens of milliseconds apart. + */ +const HEAD_LOOKUP_BATCH_WINDOW = "50 millis"; +/** A full document is 5,000 rows of well under 2 KB each. */ +const HEAD_LOOKUP_MAX_OUTPUT_BYTES = 16_000_000; +const RATE_LIMIT_READING = "query { rateLimit { cost limit remaining resetAt } }"; + +class PullRequestsByHeadRead extends Request.Class< + { + readonly cwd: string; + readonly host: string; + readonly owner: string; + readonly name: string; + readonly headRefName: string; + readonly state: PullRequestListState; + readonly limit: number; + }, + ReadonlyArray, + GitHubCliError +> {} + +const GraphQlVariables = Schema.Record( + Schema.String, + Schema.Union([Schema.String, Schema.Array(Schema.String)]), +); +/** A GraphQL request body for `gh api graphql --input -`. */ +const encodeGraphQlRequest = Schema.encodeSync( + Schema.fromJsonString(Schema.Struct({ query: Schema.String, variables: GraphQlVariables })), +); + +/** One aliased `pullRequests` connection per lookup, each head and state passed as a variable. */ +function buildPullRequestsByHeadQuery( + lookups: ReadonlyArray>, +): { readonly document: string; readonly variables: typeof GraphQlVariables.Type } { + const variables: Record> = {}; + const declarations: string[] = ["$owner: String!", "$name: String!"]; + const selections: string[] = []; + for (const [index, lookup] of lookups.entries()) { + variables[`h${index}`] = lookup.headRefName; + variables[`s${index}`] = GRAPHQL_STATES[lookup.state]; + declarations.push(`$h${index}: String!`, `$s${index}: [PullRequestState!]`); + // `gh pr list` orders the same way, so a head with more matches than the limit keeps the + // same rows. + selections.push( + ` h${index}: pullRequests(headRefName: $h${index}, states: $s${index}, first: ${lookup.limit}, orderBy: { field: CREATED_AT, direction: DESC }) { nodes { ${PULL_REQUEST_NODE_SELECTION} } }`, + ); + } + return { + document: `query PullRequestsByHead(${declarations.join(", ")}) {\n repository(owner: $owner, name: $name) {\n${selections.join("\n")}\n }\n}`, + variables, + }; +} + +/** The reset time of a `rateLimit` reading, which sets when the next one is due. */ +const decodeRateLimitReading = (raw: string) => + Result.map( + decodeJsonResult( + Schema.Struct({ + data: Schema.Struct({ rateLimit: Schema.Struct({ resetAt: Schema.String }) }), + }), + )(raw), + (reading) => reading.data.rateLimit.resetAt, + ); + +const decodePullRequestsByHead = decodeJsonResult( + Schema.Struct({ + data: Schema.Struct({ + repository: Schema.Record( + Schema.String, + Schema.NullOr(Schema.Struct({ nodes: Schema.Array(Schema.Unknown) })), + ), + }), + }), +); + +/** + * The repository `gh pr list` reads in a checkout, picked the way gh picks one without a + * prompt: the remote `gh repo set-default` marked, else the first of upstream, github, origin + * (in any case), else the only remote. `remotes` is `git remote -v` output and `resolved` is the output of + * `git config --get-regexp '^remote\..*\.gh-resolved$'`. + * + * Null whenever gh might weigh the remotes differently: a remote on another host or under an + * SSH alias, more than one mark, or several remotes with none of those names. Callers then + * ask gh itself. + */ +export function selectGitHubBaseRepository(input: { + readonly remotes: string; + readonly resolved: string; + readonly host: string; +}): { readonly owner: string; readonly name: string } | null { + const host = input.host.toLowerCase(); + const repositories = new Map(); + for (const line of input.remotes.split("\n")) { + const match = /^(\S+)\s+(\S+)\s+\(fetch\)$/u.exec(line.trim()); + if (!match) continue; + const [remoteHost, owner, name, ...rest] = normalizeGitRemoteUrl(match[2]!).split("/"); + if (remoteHost !== host || !owner || !name || rest.length > 0) return null; + repositories.set(match[1]!, { owner, name }); + } + const marks = input.resolved + .split("\n") + .map((line) => /^remote\.(.+)\.gh-resolved\s+(\S+)$/u.exec(line.trim())) + .filter((match): match is RegExpExecArray => match !== null && repositories.has(match[1]!)); + if (marks.length > 1) return null; + const [mark] = marks; + if (mark) { + if (mark[2] === "base") return repositories.get(mark[1]!) ?? null; + const [owner, name, ...rest] = mark[2]!.toLowerCase().split("/"); + return owner && name && rest.length === 0 ? { owner, name } : null; + } + // gh sorts remotes by these names, case-insensitively, and takes the first. A tie for the + // top place has no defined winner. + const score = (remoteName: string) => + ["origin", "github", "upstream"].indexOf(remoteName.toLowerCase()) + 1; + const ranked = [...repositories.entries()].toSorted( + ([left], [right]) => score(right) - score(left), + ); + const [top, next] = ranked; + return top !== undefined && (next === undefined || score(top[0]) > score(next[0])) + ? top[1] + : null; +} + /** @public Service construction is part of the canonical Effect module API. */ export const make = Effect.gen(function* () { const process = yield* VcsProcess.VcsProcess; @@ -472,66 +635,86 @@ export const make = Effect.gen(function* () { }, ); - const quota = yield* Cache.makeWith( + /** + * A GraphQL `rateLimit` reading per host and credential, so the budget knows the balance + * before reads that cannot report their own cost (`gh pr list`). GraphQL documents keep it + * current in between. Each reading costs one point and is redone at the reset, or after ten + * minutes so a `gh auth switch` is not priced against the old account for a whole hour. + */ + const budgetReading = yield* Cache.makeWith( (key: string) => { const host = key.split("\0")[0]!; return executeRaw({ cwd: globalThis.process.cwd(), - args: [ - "api", - "rate_limit", - "--hostname", - host, - "--jq", - ".resources.graphql | {data:{rateLimit:{cost:1,limit:.limit,remaining:.remaining,resetAt:(.reset|todateiso8601)}}}", - ], + args: ["api", "graphql", "--hostname", host, "-f", `query=${RATE_LIMIT_READING}`], }).pipe( Effect.tap((result) => budget.observe(host, result.stdout)), - Effect.asVoid, + Effect.flatMap((result) => + Clock.currentTimeMillis.pipe( + Effect.map((now) => { + const resetAtMs = Date.parse( + decodeRateLimitReading(result.stdout).pipe( + Result.match({ onFailure: () => "", onSuccess: (reading) => reading }), + ), + ); + // A reset that is missing or already past (a skewed clock) waits a minute, so a + // bad reading cannot ask again on every read. + return Duration.millis( + Number.isFinite(resetAtMs) && resetAtMs > now + ? Math.min(resetAtMs - now, 600_000) + : 60_000, + ); + }), + ), + ), ); }, { capacity: 32, - timeToLive: (exit) => (Exit.isSuccess(exit) ? Duration.seconds(30) : Duration.zero), + timeToLive: (exit) => (Exit.isSuccess(exit) ? exit.value : Duration.minutes(1)), }, ); - const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( - function* (input) { - const [command, action] = input.args; - if ( - !( - (command === "pr" && (action === "list" || action === "view")) || - (command === "repo" && action === "view") - ) - ) - return yield* executeRaw(input); + + /** + * Runs a GraphQL-priced read under its host's pause and the GraphQL budget. `run` receives + * the document with `rateLimit` added, so a GraphQL read can report what it spent; a CLI read + * ignores it and is priced at one point. + */ + const guardedRead = (input: { + readonly cwd: string; + readonly host: string; + readonly document: string; + readonly allowReserve: boolean; + readonly run: (document: string) => Effect.Effect; + }) => + Effect.gen(function* () { const credential = yield* PinnedGitHubCredential; - if (credential !== null && !targetsVerifiedHost(input.args, credential.host)) - return yield* executeRaw(input); - const allowReserve = input.allowReserve ?? (yield* AllowGitHubReserve); - const host = ( - credential?.host ?? - commandHosts(input.args).find((host) => host !== null) ?? - input.rateLimitHost ?? - input.env?.GH_HOST ?? - globalThis.process.env.GH_HOST ?? - "github.com" - ).toLowerCase(); - const key = { provider: "github" as const, host }; + const key = { provider: "github" as const, host: input.host }; const guarded = Effect.gen(function* () { - const lease = yield* limits.check(key, allowReserve ? { allowPaused: true } : undefined); - return yield* Effect.gen(function* () { - yield* Cache.get(quota, `${host}\0${credential?.credentialFingerprint ?? ""}`); - yield* budget.query(host, "query {}", allowReserve ? { allowReserve: true } : undefined); - return yield* executeRaw(input); - }).pipe( - Effect.tap(() => limits.recordSuccess({ ...key, lease })), - Effect.tapError((error) => - error._tag === "GitHubCliRateLimitError" - ? limits.recordRateLimit({ ...key, lease }) - : Effect.void, - ), + const lease = yield* limits.check( + key, + input.allowReserve ? { allowPaused: true } : undefined, ); + // A failed reading leaves the budget unknown; it never blocks the read itself. + yield* Cache.get( + budgetReading, + `${input.host}\0${yield* SourceControlRateLimit.CredentialScope}`, + ).pipe(Effect.ignore); + return yield* budget + .query( + input.host, + input.document, + input.allowReserve ? { allowReserve: true } : undefined, + ) + .pipe( + Effect.flatMap(input.run), + Effect.tap(() => limits.recordSuccess({ ...key, lease })), + Effect.tapError((error) => + error._tag === "GitHubCliRateLimitError" + ? limits.recordRateLimit({ ...key, lease }) + : Effect.void, + ), + ); }); return yield* guarded.pipe( Effect.provideService( @@ -550,11 +733,218 @@ export const make = Effect.gen(function* () { ), }), ); + }); + + const execute: GitHubCli["Service"]["execute"] = Effect.fn("GitHubCli.execute")( + function* (input) { + const [command, action] = input.args; + if ( + !( + (command === "pr" && (action === "list" || action === "view")) || + (command === "repo" && action === "view") + ) + ) + return yield* executeRaw(input); + const credential = yield* PinnedGitHubCredential; + if (credential !== null && !targetsVerifiedHost(input.args, credential.host)) + return yield* executeRaw(input); + return yield* guardedRead({ + cwd: input.cwd, + host: ( + credential?.host ?? + commandHosts(input.args).find((host) => host !== null) ?? + input.rateLimitHost ?? + input.env?.GH_HOST ?? + globalThis.process.env.GH_HOST ?? + "github.com" + ).toLowerCase(), + document: "query {}", + allowReserve: input.allowReserve ?? (yield* AllowGitHubReserve), + run: () => executeRaw(input), + }); }, ); + const listPullRequestsWithCli = (input: { + readonly cwd: string; + readonly headSelector: string; + readonly state: PullRequestListState; + readonly limit: number; + readonly rateLimitHost?: string | undefined; + }) => + execute({ + cwd: input.cwd, + ...(input.rateLimitHost === undefined ? {} : { rateLimitHost: input.rateLimitHost }), + args: [ + "pr", + "list", + "--head", + input.headSelector, + "--state", + input.state, + "--limit", + String(input.limit), + "--json", + PULL_REQUEST_LIST_JSON_FIELDS, + ], + }).pipe( + Effect.flatMap((result) => { + const raw = result.stdout.trim(); + if (raw.length === 0) return Effect.succeed([]); + const decoded = decodeGitHubPullRequestListJson(raw); + return Result.isSuccess(decoded) + ? Effect.succeed(decoded.success) + : Effect.fail( + new GitHubChangeRequestListDecodeError({ + command: "gh", + cwd: input.cwd, + cause: decoded.failure, + }), + ); + }), + ); + + const git = (cwd: string, args: ReadonlyArray) => + process.run({ + operation: "GitHubCli.baseRepository", + command: "git", + args, + cwd, + allowNonZeroExit: true, + timeoutMs: 5_000, + }); + + /** The repository gh reads in `cwd`, or null when gh could pick it another way. */ + const resolveBaseRepository = (cwd: string, host: string) => + globalThis.process.env.GH_REPO + ? Effect.succeed(null) + : Effect.all([ + git(cwd, ["remote", "-v"]), + git(cwd, ["config", "--get-regexp", "^remote\\..*\\.gh-resolved$"]), + ]).pipe( + Effect.map(([remotes, resolved]) => + remotes.exitCode === 0 + ? selectGitHubBaseRepository({ + remotes: remotes.stdout, + resolved: resolved.exitCode === 0 ? resolved.stdout : "", + host, + }) + : null, + ), + Effect.orElseSucceed(() => null), + ); + + const headResolver = RequestResolver.makeGrouped({ + key: ({ request, context }) => + JSON.stringify([ + request.host, + request.owner, + request.name, + Context.getOrElse(context, PinnedGitHubCredential, () => null)?.credentialFingerprint ?? + null, + Context.getOrElse(context, SourceControlRateLimit.CredentialScope, () => ""), + ]), + resolver: (entries) => { + const [first] = entries; + const { cwd, host, owner, name } = first.request; + const query = buildPullRequestsByHeadQuery(entries.map((entry) => entry.request)); + const readCli = (entry: (typeof entries)[number]) => + listPullRequestsWithCli({ + cwd: entry.request.cwd, + headSelector: entry.request.headRefName, + state: entry.request.state, + limit: entry.request.limit, + rateLimitHost: entry.request.host, + }).pipe( + Effect.exit, + Effect.map((exit) => entry.completeUnsafe(exit)), + ); + return guardedRead({ + cwd, + host, + document: query.document, + allowReserve: false, + run: (document) => + executeRaw({ + cwd, + args: ["api", "graphql", "--hostname", host, "--input", "-"], + // Up to 50 heads of 100 rows each. A default branch such as `main` can match a + // hundred fork pull requests, so the 1 MB default would cut the answer short. + maxOutputBytes: HEAD_LOOKUP_MAX_OUTPUT_BYTES, + stdin: encodeGraphQlRequest({ + query: document, + variables: { owner, name, ...query.variables }, + }), + }).pipe(Effect.tap((result) => budget.observe(host, result.stdout))), + }).pipe( + Effect.flatMap((result) => { + const decoded = decodePullRequestsByHead(result.stdout); + if (!Result.isSuccess(decoded)) { + return Effect.forEach(entries, readCli, { discard: true }); + } + const aliases = decoded.success.data.repository; + return Effect.forEach( + entries, + (entry, index) => { + const alias = aliases[`h${index}`]; + if (alias == null) return readCli(entry); + entry.completeUnsafe(Exit.succeed(decodeGitHubPullRequestEntries(alias.nodes))); + return Effect.void; + }, + { discard: true }, + ); + }), + // A document GitHub refused as a whole (a renamed repository, a field an older + // Enterprise host lacks) leaves each lookup to gh. A rate limit fails them all: + // asking one at a time would only spend what the pause is saving. + Effect.catchIf( + (error) => error._tag !== "GitHubCliRateLimitError", + () => Effect.forEach(entries, readCli, { discard: true }), + ), + Effect.catchCause((cause) => + Effect.sync(() => { + for (const entry of entries) entry.completeUnsafe(Exit.failCause(cause)); + }), + ), + ); + }, + }).pipe( + RequestResolver.setDelay(HEAD_LOOKUP_BATCH_WINDOW), + RequestResolver.batchN(HEAD_LOOKUPS_PER_DOCUMENT), + ); + + const listPullRequestsByHead: GitHubCli["Service"]["listPullRequestsByHead"] = Effect.fn( + "GitHubCli.listPullRequestsByHead", + )(function* (input) { + const host = input.rateLimitHost?.toLowerCase(); + const credential = yield* PinnedGitHubCredential; + // `owner:branch` selectors and other hosts keep gh's own handling. + const repository = + host === undefined || + input.headSelector.includes(":") || + (credential !== null && credential.host !== host) + ? null + : yield* resolveBaseRepository(input.cwd, host); + if (host === undefined || repository === null) { + return yield* listPullRequestsWithCli(input); + } + return yield* Effect.request( + new PullRequestsByHeadRead({ + cwd: input.cwd, + host, + owner: repository.owner, + name: repository.name, + headRefName: input.headSelector, + state: input.state, + limit: Math.min(Math.max(Math.trunc(input.limit), 1), 100), + }), + headResolver, + ); + }); + return GitHubCli.of({ execute, + listPullRequestsByHead, listOpenPullRequests: (input) => execute({ cwd: input.cwd, diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts index 0d46b6eab9ea8..2a214fc800ffe 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.test.ts @@ -7,6 +7,7 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import * as VcsProcess from "../vcs/VcsProcess.ts"; import * as GitHubCli from "./GitHubCli.ts"; +import * as GitHubGraphQlBudget from "./githubGraphQlBudget.ts"; import { parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; import * as GitHubSourceControlProvider from "./GitHubSourceControlProvider.ts"; @@ -32,19 +33,13 @@ function makeProvider(github: Partial) { it.effect("uses the enterprise quota for a current-repository default branch read", () => Effect.gen(function* () { - const provider = yield* GitHubSourceControlProvider.make.pipe( - Effect.provide(GitHubCli.layer), - Effect.provideService(VcsProcess.VcsProcess, { - run: (input) => - Effect.sync(() => { - if (input.args[1] !== "rate_limit") return processResult("main"); - assert.strictEqual(input.args[3], "enterprise.test"); - return processResult( - '{"data":{"rateLimit":{"cost":1,"limit":5000,"remaining":5000,"resetAt":"2099-01-01T00:00:00Z"}}}', - ); - }), - }), + // github.com is out of quota; the enterprise read must not be priced against it. + const budget = yield* GitHubGraphQlBudget.GitHubGraphQlBudget; + yield* budget.observe( + "github.com", + '{"data":{"rateLimit":{"cost":1,"limit":5000,"remaining":0,"resetAt":"2099-01-01T00:00:00Z"}}}', ); + const provider = yield* GitHubSourceControlProvider.make; const branch = yield* provider.getDefaultBranch({ cwd: "/enterprise-repo", context: { @@ -54,7 +49,12 @@ it.effect("uses the enterprise quota for a current-repository default branch rea }, }); assert.strictEqual(branch, "main"); - }), + }).pipe( + Effect.provide(GitHubCli.layer), + Effect.provideService(VcsProcess.VcsProcess, { + run: () => Effect.succeed(processResult("main")), + }), + ), ); it.effect("maps GitHub PR summaries into provider-neutral change requests", () => @@ -138,50 +138,47 @@ it.effect("adds safe request context while retaining GitHub CLI causes", () => }), ); -it.effect("uses gh json listing for non-open change request state queries", () => +it.effect("lists change request history through the batched head lookup", () => Effect.gen(function* () { - let executeArgs: ReadonlyArray = []; + let lookup: Parameters[0] | null = + null; const provider = yield* makeProvider({ - execute: (input) => { - executeArgs = input.args; - return Effect.succeed( - processResult( - JSON.stringify([ - { - number: 7, - title: "Merged work", - url: "https://github.com/pingdotgg/t3code/pull/7", - baseRefName: "main", - headRefName: "feature/merged", - state: "merged", - mergedAt: "2026-01-01T00:00:00Z", - updatedAt: "2026-01-02T00:00:00.000Z", - }, - ]), - ), - ); + listPullRequestsByHead: (input) => { + lookup = input; + return Effect.succeed([ + { + number: 7, + title: "Merged work", + url: "https://enterprise.test/acme/web/pull/7", + baseRefName: "main", + headRefName: "feature/merged", + state: "merged", + mergedAt: "2026-01-01T00:00:00Z", + updatedAt: Option.some(DateTime.makeUnsafe("2026-01-02T00:00:00.000Z")), + }, + ]); }, }); const changeRequests = yield* provider.listChangeRequests({ cwd: "/repo", + context: { + provider: { kind: "github", name: "GitHub Enterprise", baseUrl: "https://enterprise.test" }, + remoteName: "origin", + remoteUrl: "https://enterprise.test/acme/web.git", + }, headSelector: "feature/merged", state: "all", limit: 10, }); - assert.deepStrictEqual(executeArgs, [ - "pr", - "list", - "--head", - "feature/merged", - "--state", - "all", - "--limit", - "10", - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ]); + assert.deepStrictEqual(lookup, { + cwd: "/repo", + headSelector: "feature/merged", + state: "all", + limit: 10, + rateLimitHost: "enterprise.test", + }); assert.strictEqual(changeRequests[0]?.provider, "github"); assert.strictEqual(changeRequests[0]?.state, "merged"); assert.strictEqual(changeRequests[0]?.mergedAt, "2026-01-01T00:00:00Z"); @@ -192,23 +189,6 @@ it.effect("uses gh json listing for non-open change request state queries", () = }), ); -it.effect("treats empty non-open change request listing output as no results", () => - Effect.gen(function* () { - const provider = yield* makeProvider({ - execute: () => Effect.succeed(processResult("")), - }); - - const changeRequests = yield* provider.listChangeRequests({ - cwd: "/repo", - headSelector: "feature/empty", - state: "all", - limit: 10, - }); - - assert.deepStrictEqual(changeRequests, []); - }), -); - it.effect("creates GitHub PRs through provider-neutral input names", () => Effect.gen(function* () { let createInput: Parameters[0] | null = diff --git a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts index 372d2a032d799..905146d5a733d 100644 --- a/apps/server/src/sourceControl/GitHubSourceControlProvider.ts +++ b/apps/server/src/sourceControl/GitHubSourceControlProvider.ts @@ -2,16 +2,10 @@ import * as Schema from "effect/Schema"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; -import * as Result from "effect/Result"; -import { - SourceControlProviderError, - type ChangeRequest, - type ChangeRequestState, -} from "@t3tools/contracts"; +import { SourceControlProviderError, type ChangeRequest } from "@t3tools/contracts"; import * as GitHubCli from "./GitHubCli.ts"; import { findAuthenticatedGitHubAccount, parseGitHubAuthStatus } from "./gitHubAuthStatus.ts"; -import { decodeGitHubPullRequestListJson } from "./gitHubPullRequests.ts"; import * as SourceControlProvider from "./SourceControlProvider.ts"; import { combinedAuthOutput, @@ -151,59 +145,28 @@ export const make = Effect.gen(function* () { ); } - const stateArg: ChangeRequestState | "all" = input.state; return github - .execute({ + .listPullRequestsByHead({ cwd: input.cwd, + headSelector: input.headSelector, + state: input.state, + limit: input.limit ?? 20, ...(input.context === undefined ? {} : { rateLimitHost: new URL(input.context.provider.baseUrl).host }), - args: [ - "pr", - "list", - "--head", - input.headSelector, - "--state", - stateArg, - "--limit", - String(input.limit ?? 20), - "--json", - "number,title,url,baseRefName,headRefName,state,isDraft,mergedAt,closedAt,updatedAt,isCrossRepository,headRepository,headRepositoryOwner", - ], }) .pipe( - Effect.flatMap((result) => { - const raw = result.stdout.trim(); - if (raw.length === 0) { - return Effect.succeed([]); - } - return Effect.sync(() => decodeGitHubPullRequestListJson(raw)).pipe( - Effect.flatMap((decoded) => - Result.isSuccess(decoded) - ? Effect.succeed( - decoded.success.map((item) => { - const { updatedAt, ...summary } = item; - return { - ...toChangeRequest({ - ...summary, - ...(Option.isSome(updatedAt) - ? { updatedAt: DateTime.formatIso(updatedAt.value) } - : {}), - }), - updatedAt, - }; - }), - ) - : Effect.fail( - new GitHubCli.GitHubChangeRequestListDecodeError({ - command: "gh", - cwd: input.cwd, - cause: decoded.failure, - }), - ), - ), - ); - }), + Effect.map((items) => + items.map(({ updatedAt, ...summary }) => ({ + ...toChangeRequest({ + ...summary, + ...(Option.isSome(updatedAt) + ? { updatedAt: DateTime.formatIso(updatedAt.value) } + : {}), + }), + updatedAt, + })), + ), Effect.mapError( (error) => new SourceControlProviderError({ diff --git a/apps/server/src/sourceControl/gitHubPullRequests.ts b/apps/server/src/sourceControl/gitHubPullRequests.ts index b233d900adf11..c29a3806e13d6 100644 --- a/apps/server/src/sourceControl/gitHubPullRequests.ts +++ b/apps/server/src/sourceControl/gitHubPullRequests.ts @@ -114,25 +114,30 @@ const decodeGitHubPullRequestList = decodeJsonResult(Schema.Array(Schema.Unknown const decodeGitHubPullRequest = decodeJsonResult(GitHubPullRequestSchema); const decodeGitHubPullRequestEntry = Schema.decodeUnknownExit(GitHubPullRequestSchema); +/** + * Pull request rows in `gh --json` or GraphQL node shape. A row that does not decode is + * skipped, so one malformed pull request cannot hide the rest. + */ +export function decodeGitHubPullRequestEntries( + entries: ReadonlyArray, +): ReadonlyArray { + const pullRequests: NormalizedGitHubPullRequestRecord[] = []; + for (const entry of entries) { + const decodedEntry = decodeGitHubPullRequestEntry(entry); + if (Exit.isSuccess(decodedEntry)) { + pullRequests.push(normalizeGitHubPullRequestRecord(decodedEntry.value)); + } + } + return pullRequests; +} + export function decodeGitHubPullRequestListJson( raw: string, ): Result.Result< ReadonlyArray, Cause.Cause > { - const result = decodeGitHubPullRequestList(raw); - if (Result.isSuccess(result)) { - const pullRequests: NormalizedGitHubPullRequestRecord[] = []; - for (const entry of result.success) { - const decodedEntry = decodeGitHubPullRequestEntry(entry); - if (Exit.isFailure(decodedEntry)) { - continue; - } - pullRequests.push(normalizeGitHubPullRequestRecord(decodedEntry.value)); - } - return Result.succeed(pullRequests); - } - return Result.fail(result.failure); + return Result.map(decodeGitHubPullRequestList(raw), decodeGitHubPullRequestEntries); } export function decodeGitHubPullRequestJson( diff --git a/apps/web/src/components/ComposerPromptEditorTiptap.tsx b/apps/web/src/components/ComposerPromptEditorTiptap.tsx index 4a9b64494ff97..1926e4dd279f2 100644 --- a/apps/web/src/components/ComposerPromptEditorTiptap.tsx +++ b/apps/web/src/components/ComposerPromptEditorTiptap.tsx @@ -56,6 +56,12 @@ import { serializeEditorDoc, type SkillMeta, } from "~/composer-rich-text-doc"; +import { + COMPOSER_UNDO_GROUP_DELAY, + type ComposerChangeKind, + groupUndoByChangeKind, + markAsClipboardEdit, +} from "~/composer-undo-grouping"; import { collectInlineContextIds } from "~/lib/composerContextReferences"; import { cn, isMacPlatform } from "~/lib/utils"; import { basenameOfPath } from "~/pierre-icons"; @@ -582,6 +588,26 @@ export function ComposerPromptEditorTiptap(props: ComposerPromptEditorProps) { ); } +/** + * Starts a new undo step when the kind of change switches (typing, deleting, + * a paste or a store rewrite), the way the Lexical composer grouped undo. + * Runs as dispatch middleware because the grouping has to be decided before + * the history plugin applies the transaction. + */ +const ComposerUndoGroupingExtension = Extension.create< + Record, + { previous: ComposerChangeKind | null } +>({ + name: "composer-undo-grouping", + addStorage() { + return { previous: null }; + }, + dispatchTransaction({ transaction, next }) { + this.storage.previous = groupUndoByChangeKind(transaction, this.storage.previous); + next(transaction); + }, +}); + function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { const { value, @@ -794,9 +820,11 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { gapcursor: false, trailingNode: false, code: false, + undoRedo: { newGroupDelay: COMPOSER_UNDO_GROUP_DELAY }, // Plain mode has no marks: typed markers stay literal characters. ...(richText ? {} : { bold: false, italic: false, strike: false }), }), + ComposerUndoGroupingExtension, ComposerMentionExtension, ComposerSkillExtension, ComposerCitationExtension, @@ -1033,7 +1061,16 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { const editorInstance = editorHolder.current; if (editorInstance) { insertMarkdownParagraphs(text, skillLabelFor, { styling: richText }, (content) => { - editorInstance.commands.insertContent(content); + // Tagged on the same transaction insertContent builds, so the + // paste is one undo step of its own. + editorInstance + .chain() + .command(({ tr }) => { + markAsClipboardEdit(tr, "paste"); + return true; + }) + .insertContent(content) + .run(); }); scrollTiptapCaretIntoView(editorInstance); } @@ -1294,7 +1331,15 @@ function ComposerPromptEditorTiptapInner(props: ComposerPromptEditorProps) { clipboardData.setData("text/html", encodeComposerContextClipboardHtml(text, fragment)); } if (cut) { - editor.chain().focus().deleteSelection().run(); + editor + .chain() + .focus() + .command(({ tr }) => { + markAsClipboardEdit(tr, "cut"); + return true; + }) + .deleteSelection() + .run(); } }, [editor], diff --git a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts index c55338cd5c469..5e27fa901f0e0 100644 --- a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts +++ b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.test.ts @@ -21,6 +21,7 @@ import { getProviderUpdateInitialToastView, getProviderUpdateProgressToastView, getProviderUpdateRejectedToastView, + getProviderUpdateRunToastView, getProviderUpdateSidebarPillView, hasOneClickUpdateProviderCandidate, isProviderUpdateCandidate, @@ -1058,3 +1059,90 @@ it("does not offer incompatible latest versions and restores suggestions after p expect(isProviderSettingsUpdateCandidate(snapshot)).toBe(expected); } }); + +describe("getProviderUpdateRunToastView", () => { + const updateState = ( + status: "succeeded" | "failed", + message: string, + ): ServerProvider["updateState"] => ({ + status, + startedAt: checkedAt, + finishedAt: laterCheckedAt, + message, + output: null, + }); + const run = ( + machineLabel: string, + providerDriver: string, + result: Parameters[0][number]["result"], + ) => ({ + machineLabel, + driver: driver(providerDriver), + instanceId: instanceId(providerDriver), + result, + }); + + it("lists every failed update and ignores interrupted ones", () => { + const view = getProviderUpdateRunToastView([ + run( + "Mac Studio", + "codex", + AsyncResult.success({ + providers: [ + provider({ + driver: driver("codex"), + updateState: updateState("succeeded", "Provider updated."), + }), + ], + }), + ), + run( + "Mac Studio", + "claudeAgent", + AsyncResult.success({ + providers: [ + provider({ + driver: driver("claudeAgent"), + updateState: updateState("failed", "npm exited with code 1."), + }), + ], + }), + ), + run("Laptop", "codex", AsyncResult.failure(Cause.die(new Error("WebSocket closed")))), + run("Server", "codex", AsyncResult.failure(Cause.interrupt())), + ]); + + expect(view).toEqual({ + type: "error", + title: "2 of 3 provider updates failed", + description: "Mac Studio · Claude: npm exited with code 1.\nLaptop · Codex: WebSocket closed", + }); + }); + + it("reports success when every update succeeded", () => { + const succeeded = AsyncResult.success({ + providers: [ + provider({ + driver: driver("codex"), + updateState: updateState("succeeded", "Provider updated."), + }), + ], + }); + + expect( + getProviderUpdateRunToastView([ + run("Mac Studio", "codex", succeeded), + run("Laptop", "codex", succeeded), + ]), + ).toEqual({ + type: "success", + title: "2 providers updated", + description: "New sessions will use the updated providers.", + }); + expect( + getProviderUpdateRunToastView([ + run("Server", "codex", AsyncResult.failure(Cause.interrupt())), + ]), + ).toBeNull(); + }); +}); diff --git a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts index dd7b25a62cad2..e245fcb7cf30a 100644 --- a/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts +++ b/apps/web/src/components/ProviderUpdateLaunchNotification.logic.ts @@ -8,6 +8,7 @@ import { type ServerProvider, } from "@t3tools/contracts"; import { + isAtomCommandInterrupted, squashAtomCommandFailure, type AtomCommandResult, } from "@t3tools/client-runtime/state/runtime"; @@ -330,6 +331,61 @@ export function getProviderUpdateProgressToastView(input: { return getProviderUpdateRunningToastView(input.providerCount); } +/** One provider update sent by the cross-machine "Update all", with its result. */ +export interface ProviderUpdateRun { + readonly machineLabel: string; + readonly driver: ProviderDriverKind; + readonly instanceId: ProviderInstanceId; + readonly result: AtomCommandResult< + { readonly providers: ReadonlyArray }, + unknown + >; +} + +/** + * Summarize a cross-machine "Update all" as one toast, or null when every + * request was interrupted. Each update that did not succeed gets its own line, + * so a failure on one machine is not hidden by successes on the others. + */ +export function getProviderUpdateRunToastView( + runs: ReadonlyArray, +): Pick | null { + const settled = runs.filter((run) => !isAtomCommandInterrupted(run.result)); + if (settled.length === 0) { + return null; + } + const failureLines = settled.flatMap((run) => { + const label = `${run.machineLabel} · ${PROVIDER_DISPLAY_NAMES[run.driver] ?? run.driver}`; + if (run.result._tag === "Failure") { + const error = squashAtomCommandFailure(run.result); + return [`${label}: ${error instanceof Error ? error.message : "Provider update failed."}`]; + } + const updateState = run.result.value.providers.find( + (provider) => provider.instanceId === run.instanceId, + )?.updateState; + return updateState?.status === "succeeded" + ? [] + : [`${label}: ${updateState?.message ?? "Provider update did not finish."}`]; + }); + if (failureLines.length === 0) { + return { + type: "success", + title: settled.length === 1 ? "Provider updated" : `${settled.length} providers updated`, + description: getProviderUpdatedDescription(settled.length), + }; + } + return { + type: "error", + title: + failureLines.length < settled.length + ? `${failureLines.length} of ${settled.length} provider updates failed` + : settled.length === 1 + ? "Provider update failed" + : "Provider updates failed", + description: failureLines.join("\n"), + }; +} + export function collectUpdatedProviderSnapshots(input: { readonly results: ReadonlyArray< AtomCommandResult<{ readonly providers: ReadonlyArray }, unknown> diff --git a/apps/web/src/components/ProviderUpdatesAction.tsx b/apps/web/src/components/ProviderUpdatesAction.tsx new file mode 100644 index 0000000000000..742d6f6133048 --- /dev/null +++ b/apps/web/src/components/ProviderUpdatesAction.tsx @@ -0,0 +1,115 @@ +import { PROVIDER_DISPLAY_NAMES } from "@t3tools/contracts"; +import { useMemo, useRef, useState } from "react"; + +import { useEnvironments } from "~/state/environments"; +import { serverEnvironment } from "~/state/server"; +import { useAtomCommand } from "~/state/use-atom-command"; +import { + canOneClickUpdateProviderCandidate, + collectProviderUpdateCandidates, + getProviderUpdateRunToastView, + type ProviderUpdateRun, +} from "./ProviderUpdateLaunchNotification.logic"; +import { Button } from "./ui/button"; +import { stackedThreadToast, toastManager } from "./ui/toast"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "./ui/tooltip"; + +/** + * Updates every outdated provider on every connected machine at once, then + * reports the results in one toast. Each server queues updates that share an + * installer, so sending them all together is safe. The server still checks + * permissions: a session that cannot operate a machine gets a failure line for + * it. Renders nothing when no machine has a one-click update. + */ +export function ProviderUpdatesAction() { + const { environments } = useEnvironments(); + const updateProvider = useAtomCommand(serverEnvironment.updateProvider, { + reportFailure: false, + }); + const pending = useRef(false); + const [isPending, setIsPending] = useState(false); + const machines = useMemo( + () => + environments.flatMap((environment) => { + const providers = environment.serverConfig?.providers ?? []; + if (environment.connection.phase !== "connected") { + return []; + } + const candidates = collectProviderUpdateCandidates(providers).filter((candidate) => + canOneClickUpdateProviderCandidate(candidate, providers), + ); + return candidates.length > 0 + ? [{ environmentId: environment.environmentId, label: environment.label, candidates }] + : []; + }), + [environments], + ); + // Candidates leave the list as soon as their servers report them queued, so + // keep the button while the run is in flight. + if (machines.length === 0 && !isPending) { + return null; + } + + const handleUpdate = async () => { + if (pending.current) return; + pending.current = true; + setIsPending(true); + try { + const runs = await Promise.all( + machines.flatMap(({ environmentId, label, candidates }) => + candidates.map(async (candidate): Promise => ({ + machineLabel: label, + driver: candidate.driver, + instanceId: candidate.instanceId, + result: await updateProvider({ + environmentId, + input: { provider: candidate.driver, instanceId: candidate.instanceId }, + }), + })), + ), + ); + const view = getProviderUpdateRunToastView(runs); + if (view) { + toastManager.add( + stackedThreadToast({ + ...view, + description: {view.description}, + }), + ); + } + } finally { + pending.current = false; + setIsPending(false); + } + }; + + return ( + + void handleUpdate()} + > + {isPending ? "Updating…" : "Update all machines"} + + } + /> + +
+ Updates providers across connected machines. Each machine checks your permissions. +
+ {machines.map((machine) => ( +
+ {machine.label}:{" "} + {machine.candidates + .map((candidate) => PROVIDER_DISPLAY_NAMES[candidate.driver] ?? candidate.driver) + .join(", ")} +
+ ))} +
+
+ ); +} diff --git a/apps/web/src/components/Sidebar.drag.test.ts b/apps/web/src/components/Sidebar.drag.test.ts index 6e0a5ce7cb467..10ff985da3bab 100644 --- a/apps/web/src/components/Sidebar.drag.test.ts +++ b/apps/web/src/components/Sidebar.drag.test.ts @@ -657,6 +657,28 @@ describe("sidebar drag projection", () => { expect(result.get(sidebarMarkerId("snoozed-header"))).toEqual({ ...stationary, y: -46 }); }); + it("previews a time-ordered inbox drop at its time slot, above the Working shelf", () => { + const items = [ + pinnedHeader, + thread("p", "pinned"), + divider, + thread("a1", "active"), + thread("a2", "active"), + marker("working-header"), + thread("w", "working"), + settledHeader, + marker("settled-placeholder"), + ]; + const input = { items, settledOrder: [], settledExpanded: false }; + // By pointer, the unpinned row lands between a1 and a2. + expect(preview(input, "p", "a1").get("a2")?.y).toBe(0); + // By time, it lands below a2, and the shelf does not move. + const byTime = preview({ ...input, activeOrder: ["a1", "a2", "p"] }, "p", "a1"); + expect(byTime.get("a2")?.y).toBe(-83); + expect(byTime.get(sidebarMarkerId("working-header"))).toEqual(stationary); + expect(byTime.get("w")).toEqual(stationary); + }); + it("derives missing card geometry from the measured root scale", () => { const items = [ pinnedHeader, diff --git a/apps/web/src/components/Sidebar.drag.ts b/apps/web/src/components/Sidebar.drag.ts index e4d4f0386a06d..6664476e8c69b 100644 --- a/apps/web/src/components/Sidebar.drag.ts +++ b/apps/web/src/components/Sidebar.drag.ts @@ -13,6 +13,11 @@ const stationary = { x: 0, y: 0, scaleX: 1, scaleY: 1 }; const hidden = { ...stationary, scaleY: 0 }; type ThreadItem = Extract; type Layout = Parameters[0]; +const isShelfHeader = (item: SidebarListItem | undefined) => + item?.kind === "marker" && + (item.marker === "working-header" || + item.marker === "snoozed-header" || + item.marker === "settled-header"); /** Keep the lifted card below the Pins label, including when Pins is empty. * The container rect follows scrolling; the offset is measured once at pickup. */ @@ -57,13 +62,11 @@ export function createSidebarCollisionDetection( if (pointer.x >= boundary.left && pointer.x <= boundary.right) { if (pointer.y < previousY && pointer.y <= boundary.bottom) boundarySection = "pinned"; else if (pointer.y > previousY && pointer.y >= boundary.top) boundarySection = "active"; - const nextHeader = - args.droppableContainers.find( - (container) => container.id === sidebarMarkerId("snoozed-header"), - ) ?? - args.droppableContainers.find( - (container) => container.id === sidebarMarkerId("settled-header"), - ); + const nextHeader = (["working-header", "snoozed-header", "settled-header"] as const) + .map((marker) => + args.droppableContainers.find((container) => container.id === sidebarMarkerId(marker)), + ) + .find((container) => container !== undefined); const activeBottom = nextHeader?.node.current?.getBoundingClientRect().top; if (boundarySection === "pinned" || (activeBottom != null && pointer.y < activeBottom)) { const target = collisions.find((collision) => { @@ -99,6 +102,8 @@ export function createSidebarSortingStrategy(input: { /** Suspend the reorder preview while the thread is dragged out as context. */ enabled?: boolean; settledOrder: readonly string[]; + /** Time-ordered inbox (Working beta): where the lifted row would land. */ + activeOrder?: readonly string[]; settledExpanded: boolean; settledVisibleCount?: number; routeThreadKey?: string | null; @@ -124,6 +129,7 @@ export function createSidebarSortingStrategy(input: { const groups: Record = { pinned: [], active: [], + working: [], snoozed: [], settled: [], }; @@ -132,13 +138,13 @@ export function createSidebarSortingStrategy(input: { let headerScale: number | undefined; for (const [index, item] of items.entries()) { if (item.kind === "marker") { - if (item.marker === "settled-header" || item.marker === "snoozed-header") { + if (isShelfHeader(item)) { const height = rects[index]?.height; if (height) headerScale ??= height / 32; } continue; } - if (item.section === "pinned" || item.section === "active") + if (item.section === "pinned" || item.section === "active" || item.section === "working") cardHeight ??= rects[index]?.height; else slimHeight ??= rects[index]?.height; if (item.key !== active.key) groups[item.section].push(item); @@ -155,7 +161,7 @@ export function createSidebarSortingStrategy(input: { ? target.pinnedOrder : target.section === "settled" ? input.settledOrder - : target.activeOrder; + : (input.activeOrder ?? target.activeOrder); const ranks = new Map(order.map((key, index) => [key, index])); const rank = ranks.get(active.key) ?? Number.POSITIVE_INFINITY; const index = group.findIndex( @@ -183,6 +189,10 @@ export function createSidebarSortingStrategy(input: { projected.push(...groups.pinned); marker("pinned-divider"); section("active"); + if (items.some((item) => item.kind === "marker" && item.marker === "working-header")) { + marker("working-header"); + projected.push(...groups.working); + } if ( groups.snoozed.length > 0 || ((active.section !== "snoozed" || (input.snoozedThreadCount ?? 0) > 1) && @@ -197,7 +207,8 @@ export function createSidebarSortingStrategy(input: { const index = indices.get(sidebarListItemId(item)); const rect = index === undefined ? undefined : rects[index]; const fallback = - item.kind === "thread" && (item.section === "pinned" || item.section === "active") + item.kind === "thread" && + (item.section === "pinned" || item.section === "active" || item.section === "working") ? cardHeight : slimHeight; const moved = item.kind === "thread" && item.key === active.key; @@ -210,11 +221,7 @@ export function createSidebarSortingStrategy(input: { ? fallback : (rect?.height ?? fallback); }); - const firstShelf = items.findIndex( - (item) => - item.kind === "marker" && - (item.marker === "snoozed-header" || item.marker === "settled-header"), - ); + const firstShelf = items.findIndex(isShelfHeader); const shelfRect = rects[firstShelf]; const beforeShelf = rects[firstShelf - 1]; const lastRect = rects.at(-1); @@ -230,10 +237,7 @@ export function createSidebarSortingStrategy(input: { const result = items.map(() => hidden); let top = rects[0].top; for (const [projectedIndex, item] of projected.entries()) { - if ( - item.kind === "marker" && - (item.marker === "snoozed-header" || item.marker === "settled-header") - ) { + if (isShelfHeader(item)) { top += shelfSpace; shelfSpace = 0; } diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts index fd352515b2553..9e2595423a85e 100644 --- a/apps/web/src/components/Sidebar.logic.test.ts +++ b/apps/web/src/components/Sidebar.logic.test.ts @@ -21,6 +21,7 @@ import { hasUnseenCompletion, isContextMenuPointerDown, isSidebarSubagentThread, + isSidebarThreadWorking, isTrailingDoubleClick, orderItemsByPreferredIds, pinOrderKeyBetween, @@ -41,6 +42,9 @@ import { shouldShowSidebarV2Duration, shouldRecedeSidebarThread, sortLogicalProjectsForSidebar, + sortInboxThreadsByReturn, + resolveSidebarDropTarget, + planSidebarThreadDrop, sortPinnedThreadsForSidebar, sortProjectsForSidebar, sortScopedProjectsForSidebar, @@ -2048,3 +2052,177 @@ describe("navigation after parking a thread", () => { }, ); }); + +describe("Working shelf (beta)", () => { + const runtime = { + status: "running" as const, + providerName: "Codex", + providerInstanceId: ProviderInstanceId.make("codex"), + activeRunId: "turn-1" as never, + lastError: null, + updatedAt: "2026-03-09T10:00:00.000Z", + }; + // v2 parks a run at idle while only background tasks are live. + const background = { + pendingBackgroundTasks: [{ taskId: "bg-1", description: "sleep 20", kind: "command" as const }], + runtime: { ...runtime, status: "idle" as const, activeRunId: null }, + }; + const idle = { + hasActionableProposedPlan: false, + hasPendingApprovals: false, + hasPendingUserInput: false, + interactionMode: "default" as const, + latestRun: makeLatestRun(), + runtime: null, + }; + + it("folds away running and background-only threads only", () => { + expect(isSidebarThreadWorking({ ...idle, runtime })).toBe(true); + expect(isSidebarThreadWorking({ ...idle, ...background })).toBe(true); + expect(isSidebarThreadWorking(idle)).toBe(false); + expect( + isSidebarThreadWorking({ + ...idle, + runtime: { ...runtime, status: "idle", activeRunId: null }, + pendingBackgroundTasks: [], + }), + ).toBe(false); + expect(isSidebarThreadWorking({ ...idle, runtime, hasPendingApprovals: true })).toBe(false); + expect(isSidebarThreadWorking({ ...idle, runtime, hasPendingUserInput: true })).toBe(false); + expect( + isSidebarThreadWorking({ + ...idle, + runtime: { ...runtime, status: "failed" as const, lastError: "boom" }, + }), + ).toBe(false); + }); + + it("keeps a ready plan in the inbox while background work runs", () => { + expect( + isSidebarThreadWorking({ + ...idle, + ...background, + interactionMode: "plan", + hasActionableProposedPlan: true, + }), + ).toBe(false); + }); + + describe("sortInboxThreadsByReturn", () => { + const thread = ( + id: string, + input: { createdAt: string; completedAt?: string | null; unsettledAt?: string }, + ) => ({ + id: ThreadId.make(id), + environmentId: localEnvironmentId, + createdAt: input.createdAt, + unsettledAt: input.unsettledAt ?? null, + latestRun: + input.completedAt === undefined + ? null + : { ...makeLatestRun({ completedAt: input.completedAt }), requestedAt: input.createdAt }, + }); + + it("puts the thread that finished last on top, whatever its age", () => { + const sorted = sortInboxThreadsByReturn([ + thread("new", { createdAt: "2026-03-09T11:00:00.000Z" }), + thread("old-finished-now", { + createdAt: "2026-03-01T09:00:00.000Z", + completedAt: "2026-03-09T12:00:00.000Z", + }), + thread("reopened", { + createdAt: "2026-03-02T09:00:00.000Z", + unsettledAt: "2026-03-09T11:30:00.000Z", + }), + ]); + expect(sorted.map((entry) => entry.id)).toEqual(["old-finished-now", "reopened", "new"]); + }); + + it("counts a return the server does not stamp, like an approval request", () => { + const waiting = thread("asks-approval", { + createdAt: "2026-03-09T09:00:00.000Z", + completedAt: null, + }); + const finished = thread("finished", { + createdAt: "2026-03-09T09:30:00.000Z", + completedAt: "2026-03-09T11:00:00.000Z", + }); + expect(sortInboxThreadsByReturn([finished, waiting]).map((entry) => entry.id)).toEqual([ + "finished", + "asks-approval", + ]); + expect( + sortInboxThreadsByReturn([finished, waiting], (entry) => + entry === waiting ? Date.parse("2026-03-09T11:05:00.000Z") : undefined, + ).map((entry) => entry.id), + ).toEqual(["asks-approval", "finished"]); + }); + }); + + describe("dragging", () => { + const marker = (name: SidebarListMarker): SidebarListItem => ({ kind: "marker", marker: name }); + const row = (key: string, section: SidebarSection): SidebarListItem => ({ + kind: "thread", + key, + section, + }); + // Pinned p1 | Active a1 a2 | Working w1 | Settled s1 + const items: readonly SidebarListItem[] = [ + marker("pinned-header"), + row("p1", "pinned"), + marker("pinned-divider"), + row("a1", "active"), + row("a2", "active"), + marker("working-header"), + row("w1", "working"), + marker("settled-header"), + row("s1", "settled"), + ]; + + it("never drops into the Working shelf, and keeps it out of the inbox order", () => { + expect(resolveSidebarDropTarget(items, "a1", "w1")).toBeNull(); + expect(resolveSidebarDropTarget(items, "p1", "a2")).toEqual({ + section: "active", + pinnedOrder: [], + activeOrder: ["a1", "a2", "p1"], + }); + expect(resolveSidebarDropVerb("active", "working")).toBeNull(); + }); + + it("only changes lifecycle when the inbox is time-ordered", () => { + const base = { + pinnedOrder: ["p1"], + pinnedKeysById: new Map([["p1", "m"]]), + activeOrder: ["a1", "a2"], + activeKeysById: new Map([ + ["a1", "f"], + ["a2", "t"], + ]), + activeTimeOrdered: true, + }; + expect( + planSidebarThreadDrop({ + ...base, + activeKey: "a1", + activeSection: "active", + target: { section: "active", pinnedOrder: ["p1"], activeOrder: ["a2", "a1"] }, + }), + ).toEqual({ kind: "none" }); + expect( + planSidebarThreadDrop({ + ...base, + activeKey: "p1", + activeSection: "pinned", + target: { section: "active", pinnedOrder: [], activeOrder: ["a1", "p1", "a2"] }, + }), + ).toEqual({ + kind: "move-active", + order: null, + assignments: [], + unpin: true, + unsettle: false, + unsnooze: false, + }); + }); + }); +}); diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts index 2aae375d03769..a7614962891d1 100644 --- a/apps/web/src/components/Sidebar.logic.ts +++ b/apps/web/src/components/Sidebar.logic.ts @@ -127,9 +127,10 @@ export const animateSidebarLayoutChanges: AnimateLayoutChanges = (args) => // the lifecycle action; Sidebar.drag previews the resulting layout. Pinned // and active threads keep the dragged position; settled threads use time // order. Snoozed rows can leave the shelf, but dropping into it is not -// supported because snoozing requires a wake time. +// supported because snoozing requires a wake time. The Working shelf (beta) +// follows live status, so it is neither a drag source nor a destination. -export type SidebarSection = "pinned" | "active" | "snoozed" | "settled"; +export type SidebarSection = "pinned" | "active" | "working" | "snoozed" | "settled"; /** Resolve the shelf a visible thread belongs to. Snooze is temporary and * wins until its wake boundary; settlement then wins over a stale pin. */ @@ -156,6 +157,7 @@ export type SidebarListMarker = | "settled-placeholder" /** The boundary between pinned and active rows. */ | "pinned-divider" + | "working-header" | "snoozed-header" | "settled-header"; @@ -173,7 +175,7 @@ export function sidebarListItemId(item: SidebarListItem): string { /** The section a slot belongs to, read off the markers around it: from the top down, everything before the pinned divider is pinned, then the - inbox until the snoozed header, the shelf until the settled header, + inbox until the first shelf header, each shelf until the next header, then settled. */ function sectionAtSidebarSlot(items: readonly SidebarListItem[], index: number): SidebarSection { let section: SidebarSection = "pinned"; @@ -181,6 +183,7 @@ function sectionAtSidebarSlot(items: readonly SidebarListItem[], index: number): const item = items[i]!; if (item.kind !== "marker") continue; if (item.marker === "pinned-divider") section = "active"; + else if (item.marker === "working-header") section = "working"; else if (item.marker === "snoozed-header") section = "snoozed"; else if (item.marker === "settled-header") section = "settled"; } @@ -188,7 +191,7 @@ function sectionAtSidebarSlot(items: readonly SidebarListItem[], index: number): } /** Resolve the destination section and manual order from an arrayMove across - * the separators. The snoozed shelf is never a destination. */ + * the separators. The working and snoozed shelves are never destinations. */ export type SidebarDropTarget = { readonly section: "pinned" | "active" | "settled"; readonly pinnedOrder: readonly string[]; @@ -206,14 +209,19 @@ export function resolveSidebarDropTarget( const moved = items.filter((_, index) => index !== activeIndex); moved.splice(overIndex, 0, items[activeIndex]!); const section = sectionAtSidebarSlot(moved, overIndex); - if (section === "snoozed") return null; + if (section === "working" || section === "snoozed") return null; const pinnedOrder: string[] = []; const activeOrder: string[] = []; let currentSection: SidebarSection = "pinned"; for (const item of moved) { if (item.kind === "marker") { if (item.marker === "pinned-divider") currentSection = "active"; - else if (item.marker === "snoozed-header" || item.marker === "settled-header") break; + else if ( + item.marker === "working-header" || + item.marker === "snoozed-header" || + item.marker === "settled-header" + ) + break; } else if (currentSection === "pinned") pinnedOrder.push(item.key); else activeOrder.push(item.key); } @@ -239,7 +247,8 @@ export type SidebarThreadDropPlan = } | { readonly kind: "move-active"; - readonly order: readonly string[]; + /** Null when the inbox is time-ordered: the drop has no placement. */ + readonly order: readonly string[] | null; readonly assignments: ReadonlyArray<{ readonly id: string; readonly orderKey: string }>; readonly unpin: boolean; readonly unsettle: boolean; @@ -249,14 +258,14 @@ export type SidebarThreadDropPlan = /** What dropping in `to` does to a thread lifted from `from`, for the badge on the lifted row. Null while reordering inside one section and for the - snoozed shelf, which cannot be a drop target. */ + working and snoozed shelves, which cannot be drop targets. */ export type SidebarDropVerb = "pin" | "unpin" | "settle" | "unsettle" | "wake"; export function resolveSidebarDropVerb( from: SidebarSection, to: SidebarSection | null, ): SidebarDropVerb | null { - if (to === null || to === from || to === "snoozed") return null; + if (to === null || to === from || to === "working" || to === "snoozed") return null; if (to === "pinned") return "pin"; if (to === "settled") return "settle"; if (from === "pinned") return "unpin"; @@ -279,6 +288,8 @@ export function planSidebarThreadDrop(input: { readonly activeOrder: readonly string[]; readonly activeKeysById: ReadonlyMap; readonly activeReorderableKeys?: ReadonlySet; + /** Working beta: the inbox sorts by time, so drops only change lifecycle. */ + readonly activeTimeOrdered?: boolean; }): SidebarThreadDropPlan { const { activeKey, @@ -298,6 +309,20 @@ export function planSidebarThreadDrop(input: { } switch (target.section) { case "active": { + // Like the settled tail: threads can enter a time-ordered inbox, but + // not be arranged inside it. + if (input.activeTimeOrdered) { + return activeSection === "active" + ? { kind: "none" } + : { + kind: "move-active", + order: null, + assignments: [], + unpin: activePinned, + unsettle: activeSettled, + unsnooze: activeSection === "snoozed", + }; + } const order = target.activeOrder; if ( activeSection === "active" && @@ -981,6 +1006,17 @@ export function shouldShowSidebarV2Duration(status: SidebarThreadStatus): boolea return status === "working"; } +/** Working beta: threads busy with work that does not need the user fold into + the Working shelf. Approvals, questions, plan prompts, and failures stay + in the inbox. */ +export function isSidebarThreadWorking(thread: ThreadStatusInput): boolean { + const status = resolveSidebarThreadStatus(thread); + const hasWork = + status === "working" || + (status === "waiting" && (thread.pendingBackgroundTasks?.length ?? 0) > 0); + return hasWork && !hasPlanReadyPrompt(thread); +} + /** First VALID timestamp wins: `a ?? b` falls through on null, but a present- yet-malformed string must also fall through to the next candidate rather than sink the row to the epoch. */ @@ -1077,6 +1113,36 @@ export function reduceSidebarProjectScopeMenuState( } } +/** Working beta: the inbox lists threads newest first by when each last came + back to the user, so a thread that leaves the Working shelf lands on top. + `observedReturnAt` adds returns the server does not stamp, such as an + approval request mid-turn or background work ending. */ +export function sortInboxThreadsByReturn< + T extends Pick< + SidebarThreadSummary, + "id" | "environmentId" | "createdAt" | "unsettledAt" | "latestRun" + >, +>(threads: readonly T[], observedReturnAt?: (thread: T) => number | undefined): T[] { + const timestamps = new Map( + threads.map((thread) => [ + thread, + Math.max( + toSortableTimestamp(thread.createdAt) ?? 0, + toSortableTimestamp(thread.unsettledAt ?? undefined) ?? 0, + toSortableTimestamp(thread.latestRun?.requestedAt ?? undefined) ?? 0, + toSortableTimestamp(thread.latestRun?.completedAt ?? undefined) ?? 0, + observedReturnAt?.(thread) ?? 0, + ), + ]), + ); + return [...threads].sort( + (left, right) => + timestamps.get(right)! - timestamps.get(left)! || + left.id.localeCompare(right.id) || + left.environmentId.localeCompare(right.environmentId), + ); +} + /** The timestamp a working thread's elapsed label counts from: the running turn's start (request time until adoption), falling back to the session's last transition when the turn projection lags behind. Malformed @@ -1095,6 +1161,16 @@ export function formatWorkingDurationLabel(elapsedMs: number): string { return `${Math.floor(minutes / 60)}h ${minutes % 60}m`; } +/** A settled plan-mode run left a proposed plan waiting for the user. */ +function hasPlanReadyPrompt(thread: ThreadStatusInput): boolean { + return ( + !thread.hasPendingUserInput && + thread.interactionMode === "plan" && + isLatestRunSettled(thread.latestRun, thread.runtime) && + thread.hasActionableProposedPlan + ); +} + export function resolveThreadStatusPill(input: { thread: ThreadStatusInput; }): ThreadStatusPill | null { @@ -1149,12 +1225,7 @@ export function resolveThreadStatusPill(input: { }; } - const hasPlanReadyPrompt = - !thread.hasPendingUserInput && - thread.interactionMode === "plan" && - isLatestRunSettled(thread.latestRun, thread.runtime) && - thread.hasActionableProposedPlan; - if (hasPlanReadyPrompt) { + if (hasPlanReadyPrompt(thread)) { return { label: "Plan Ready", colorClass: "text-violet-600 dark:text-violet-300/90", diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 48953e81b4354..f5f655514c83b 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -181,6 +181,7 @@ import { firstValidTimestampMs, hasUnseenCompletion, isSidebarNestedLinkClick, + isSidebarThreadWorking, isTrailingDoubleClick, orderItemsByPreferredIds, planSidebarThreadDrop, @@ -200,6 +201,7 @@ import { resolveWorkingStartedAt, sidebarListItemId, sidebarMarkerId, + sortInboxThreadsByReturn, sortPinnedThreadsForSidebar, sortSidebarV2ProjectGroups, sortThreadsForSidebar, @@ -283,6 +285,39 @@ const SETTLED_TAIL_PAGE_COUNT = 25; // Fresh keys deliberately reset both shelves to collapsed for existing users. const SETTLED_SHELF_EXPANDED_KEY = "t3code:sidebar:settled-expanded"; const SNOOZED_SHELF_EXPANDED_KEY = "t3code:sidebar:snoozed-expanded"; +const WORKING_SHELF_EXPANDED_KEY = "t3code:sidebar:working-expanded"; + +// Working beta: when this client saw each thread leave the Working shelf. +// Module scope keeps the inbox order across routes that unmount the sidebar. +let lastWorkingThreadKeys: ReadonlySet | null = null; +const observedInboxReturns = new Map(); + +/** Stamps threads that stopped working since the last call. The first call + only takes a baseline, so mounting never reshuffles the inbox. Pass null + to reset when the beta is off. */ +function observeInboxReturns(threads: readonly EnvironmentThreadShell[] | null): void { + if (threads === null) { + lastWorkingThreadKeys = null; + observedInboxReturns.clear(); + return; + } + const working = new Set(); + const present = new Set(); + for (const thread of threads) { + const key = scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)); + present.add(key); + if (isSidebarThreadWorking(thread)) working.add(key); + } + // Drop deleted threads so the map stays bounded by the live thread list. + for (const key of observedInboxReturns.keys()) { + if (!present.has(key)) observedInboxReturns.delete(key); + } + const now = Date.now(); + for (const key of lastWorkingThreadKeys ?? []) { + if (present.has(key) && !working.has(key)) observedInboxReturns.set(key, now); + } + lastWorkingThreadKeys = working; +} function compactSidebarTimeLabel(label: string): string { if (label === "just now") return "now"; @@ -739,7 +774,7 @@ function SidebarDragBoundary(props: { // Shelf headers stay visible and keep their measured height while dragging. function SidebarSectionHeader(props: { - marker: "snoozed-header" | "settled-header"; + marker: "working-header" | "snoozed-header" | "settled-header"; label: string; className?: string; // While dragging, the settled header reads at full strength and takes the @@ -748,7 +783,13 @@ function SidebarSectionHeader(props: { isDropTarget?: boolean; toggle: { expanded: boolean; onToggle: () => void }; }) { - const snoozed = props.marker === "snoozed-header"; + const shelf = + props.marker === "working-header" + ? "working" + : props.marker === "snoozed-header" + ? "snoozed" + : "settled"; + const snoozed = shelf === "snoozed"; return ( {props.label} @@ -2253,6 +2294,7 @@ export default function Sidebar() { const confirmThreadArchive = useClientSettings((s) => s.confirmThreadArchive); const sidebarProjectSortOrder = useClientSettings((s) => s.sidebarProjectSortOrder); const timestampFormat = useClientSettings((s) => s.timestampFormat); + const workingShelfEnabled = useClientSettings((s) => s.sidebarWorkingShelfEnabled); const projectGroupingSettings = useClientSettings(selectProjectGroupingSettings); const { settleThread, @@ -2619,6 +2661,7 @@ export default function Sidebar() { draggableThreadKeys, activeReorderableThreadKeys, activeThreads, + workingThreads, snoozedThreads, settledThreads, snoozeNow, @@ -2632,8 +2675,14 @@ export default function Sidebar() { // Subagent child threads live in the parent's Agents surface, not the // sidebar roster (v2 models them as real threads with lineage). const visible = filterSidebarV2VisibleThreads(threads, scopedProjectKeys); + observeInboxReturns(workingShelfEnabled ? threads : null); const pinned: EnvironmentThreadShell[] = []; const active: EnvironmentThreadShell[] = []; + const working: EnvironmentThreadShell[] = []; + // Working beta: only inbox threads fold away. Pins stay where the user + // put them, and snoozed or settled threads keep their shelves. + const inbox = (thread: EnvironmentThreadShell) => + workingShelfEnabled && isSidebarThreadWorking(thread) ? working : active; const snoozed: EnvironmentThreadShell[] = []; const settled: EnvironmentThreadShell[] = []; const draggable = new Set(); @@ -2664,7 +2713,7 @@ export default function Sidebar() { ? pinned : optimisticDrop.section === "settled" ? settled - : active + : inbox(projected) ).push( optimisticDrop.clearsSnooze ? projected @@ -2682,7 +2731,7 @@ export default function Sidebar() { ? settled : section === "pinned" ? pinned - : active + : inbox(thread) ).push(thread); } } @@ -2692,7 +2741,13 @@ export default function Sidebar() { // sort, or mixed-version fleets would render different pinned orders on // web and mobile from the same data. const sortedPinned = sortPinnedThreadsForSidebar(pinned); - const sortedActive = sortThreadsForSidebar(active); + const sortedActive = workingShelfEnabled + ? sortInboxThreadsByReturn(active, (thread) => + observedInboxReturns.get( + scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)), + ), + ) + : sortThreadsForSidebar(active); return { pinnedThreads: optimisticDrop?.section !== "pinned" || optimisticDrop.order === null @@ -2712,6 +2767,8 @@ export default function Sidebar() { preferredIds: optimisticDrop.order, getId: (thread) => scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)), }), + // Newest work first, by the same clock as the inbox. + workingThreads: sortInboxThreadsByReturn(working), // Soonest wake first: "what comes back next" is the shelf's question. snoozedThreads: snoozed.toSorted( (left, right) => @@ -2721,15 +2778,29 @@ export default function Sidebar() { settledThreads: sortSettledThreads(settled), snoozeNow: preciseNow, }; - }, [nowMinute, optimisticDrop, scopedProjectKeys, serverConfigs, snoozeWakeTick, threads]); + }, [ + nowMinute, + optimisticDrop, + scopedProjectKeys, + serverConfigs, + snoozeWakeTick, + threads, + workingShelfEnabled, + ]); const threadSearchInputRef = useRef(null); const [threadSearchQuery, setThreadSearchQuery] = useState(""); const [activeSearchResultIndex, setActiveSearchResultIndex] = useState(0); const isSearchingThreads = threadSearchQuery.trim().length > 0; const searchableThreads = useMemo( - () => [...pinnedThreads, ...activeThreads, ...snoozedThreads, ...settledThreads], - [activeThreads, pinnedThreads, settledThreads, snoozedThreads], + () => [ + ...pinnedThreads, + ...activeThreads, + ...workingThreads, + ...snoozedThreads, + ...settledThreads, + ], + [activeThreads, pinnedThreads, settledThreads, snoozedThreads, workingThreads], ); const searchEnvironmentIds = useConnectedEnvironmentIds(); // useThreadSearch owns the debounce and the two-character floor. @@ -2859,9 +2930,43 @@ export default function Sidebar() { return routeThread === undefined ? EMPTY_THREADS : [routeThread]; }, [routeThreadKey, snoozedShelfExpanded, snoozedThreads]); + // The Working shelf (beta) collapses the same way, with the same route + // exception: sending a message folds the open thread into the shelf, and + // its row must stay visible there. + const [workingShelfExpanded, setWorkingShelfExpanded] = useLocalStorage( + WORKING_SHELF_EXPANDED_KEY, + false, + Schema.Boolean, + ); + const toggleWorkingShelf = useCallback( + () => setWorkingShelfExpanded((value) => !value), + [setWorkingShelfExpanded], + ); + const visibleWorkingThreads = useMemo(() => { + if (workingShelfExpanded) return workingThreads; + if (routeThreadKey === null) return EMPTY_THREADS; + const routeThread = workingThreads.find( + (thread) => + scopedThreadKey(scopeThreadRef(thread.environmentId, thread.id)) === routeThreadKey, + ); + return routeThread === undefined ? EMPTY_THREADS : [routeThread]; + }, [routeThreadKey, workingShelfExpanded, workingThreads]); + const orderedThreads = useMemo( - () => [...pinnedThreads, ...activeThreads, ...visibleSnoozedThreads, ...renderedSettledThreads], - [pinnedThreads, activeThreads, visibleSnoozedThreads, renderedSettledThreads], + () => [ + ...pinnedThreads, + ...activeThreads, + ...visibleWorkingThreads, + ...visibleSnoozedThreads, + ...renderedSettledThreads, + ], + [ + pinnedThreads, + activeThreads, + visibleWorkingThreads, + visibleSnoozedThreads, + renderedSettledThreads, + ], ); const orderedThreadKeys = useMemo( () => @@ -3321,10 +3426,11 @@ export default function Sidebar() { }; add(pinnedThreads, "pinned"); add(activeThreads, "active"); + add(workingThreads, "working"); add(snoozedThreads, "snoozed"); add(settledThreads, "settled"); return map; - }, [activeThreads, pinnedThreads, settledThreads, snoozedThreads]); + }, [activeThreads, pinnedThreads, settledThreads, snoozedThreads, workingThreads]); const pinnedKeys = useMemo( () => pinnedThreads.map((thread) => @@ -3493,6 +3599,7 @@ export default function Sidebar() { if ( pinnedThreads.length + activeThreads.length + + workingThreads.length + snoozedThreads.length + settledThreads.length === 0 @@ -3506,6 +3613,10 @@ export default function Sidebar() { const activeRows = rowsOf(activeThreads, "active"); items.push({ kind: "marker", marker: "active-placeholder" }); items.push(...activeRows); + if (workingThreads.length > 0) { + items.push({ kind: "marker", marker: "working-header" }); + items.push(...rowsOf(visibleWorkingThreads, "working")); + } if (snoozedThreads.length > 0) { items.push({ kind: "marker", marker: "snoozed-header" }); items.push(...rowsOf(visibleSnoozedThreads, "snoozed")); @@ -3522,6 +3633,8 @@ export default function Sidebar() { settledThreads.length, snoozedThreads.length, visibleSnoozedThreads, + visibleWorkingThreads, + workingThreads.length, ]); useEffect(() => { if ( @@ -3582,6 +3695,21 @@ export default function Sidebar() { applySidebarThreadDrop(thread, "settled", dragState.occurredAt), ]).map(key); }, [dragState, settledThreads, threadByKey]); + // Working beta: the inbox is time-ordered too, so the preview shows the + // slot a drop will land in, not the slot under the pointer. + const draggedActiveOrder = useMemo(() => { + const thread = dragState === null ? undefined : threadByKey.get(dragState.activeKey); + if (!workingShelfEnabled || dragState === null || thread === undefined) return undefined; + const key = (candidate: EnvironmentThreadShell) => + scopedThreadKey(scopeThreadRef(candidate.environmentId, candidate.id)); + return sortInboxThreadsByReturn( + [ + ...activeThreads.filter((candidate) => key(candidate) !== dragState.activeKey), + applySidebarThreadDrop(thread, "active", dragState.occurredAt), + ], + (candidate) => observedInboxReturns.get(key(candidate)), + ).map(key); + }, [activeThreads, dragState, threadByKey, workingShelfEnabled]); const sidebarSortingStrategy = useMemo( () => createSidebarSortingStrategy({ @@ -3589,12 +3717,14 @@ export default function Sidebar() { enabled: !isContextDrag, boundaryLabelHeight: SIDEBAR_DRAG_LABEL_HEIGHT, settledOrder: draggedSettledOrder, + ...(draggedActiveOrder === undefined ? {} : { activeOrder: draggedActiveOrder }), settledExpanded: settledShelfExpanded, settledVisibleCount, routeThreadKey, snoozedThreadCount: snoozedThreads.length, }), [ + draggedActiveOrder, draggedSettledOrder, isContextDrag, routeThreadKey, @@ -3651,6 +3781,7 @@ export default function Sidebar() { activeOrder: activeKeys, activeKeysById, activeReorderableKeys: activeReorderableThreadKeys, + activeTimeOrdered: workingShelfEnabled, }).kind !== "none" ); }, @@ -3672,6 +3803,7 @@ export default function Sidebar() { pinnedKeys, sidebarListItems, threadByKey, + workingShelfEnabled, ]); const handleThreadDragEnd = useCallback( (event: DragEndEvent) => { @@ -3699,6 +3831,7 @@ export default function Sidebar() { activeOrder: activeKeys, activeKeysById, activeReorderableKeys: activeReorderableThreadKeys, + activeTimeOrdered: workingShelfEnabled, }); if (plan.kind === "none") return; if (plan.kind === "settle" && settlingThreadKeysRef.current.has(activeKey)) return; @@ -3832,6 +3965,7 @@ export default function Sidebar() { unpinThread, unsettleThread, unsnoozeThread, + workingShelfEnabled, ], ); // One snooze per thread at a time — same double-dispatch guard as settle. @@ -4817,7 +4951,9 @@ export default function Sidebar() { // row: every other thread is a full card. Density comes // from users (or the auto rules) actually parking work, // not from the sidebar second-guessing what still matters. - const isCard = section === "active" || section === "pinned"; + // Working rows stay cards so their live status shows. + const isCard = + section === "active" || section === "pinned" || section === "working"; const rowVariant = isCard ? "card" : "slim"; return ( , ); break; + case "working-header": + items.push( + , + ); + break; case "snoozed-header": items.push( 0 && !checksStale && can("approve-workflows"); const checksSummary = checksStale ? checksState === null ? "No checks reported" @@ -2551,8 +2553,13 @@ export function PullRequestDetailPanel({ ))} {tab === "summary" ? ( - - {workflowApprovalsRequired > 0 && !checksStale && can("approve-workflows") ? ( + + {showsApproveWorkflows ? ( ) : ( {checksState !== null ? ( @@ -2601,7 +2608,7 @@ export function PullRequestDetailPanel({ ) : ( )} - {checksSummary} + {checksSummary} )} diff --git a/apps/web/src/components/settings/ProviderSettingsPanel.tsx b/apps/web/src/components/settings/ProviderSettingsPanel.tsx index cfe189c49d86d..0f0f8a3372903 100644 --- a/apps/web/src/components/settings/ProviderSettingsPanel.tsx +++ b/apps/web/src/components/settings/ProviderSettingsPanel.tsx @@ -62,6 +62,7 @@ import { isProviderUpdateActive, type ProviderSettingsUpdateCandidate, } from "../ProviderUpdateLaunchNotification.logic"; +import { ProviderUpdatesAction } from "../ProviderUpdatesAction"; import { Button } from "../ui/button"; import { Empty, @@ -1194,6 +1195,7 @@ export function EnvironmentProviderSettings({ variant="plain" headerAction={
+ {readOnly ? ( diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 942a1f9e58a7b..a11bcf730b86c 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -552,6 +552,10 @@ export function useSettingsRestore(onRestored?: () => void) { DEFAULT_UNIFIED_SETTINGS.sidebarProjectGroupingMode ? ["Project Grouping"] : []), + ...(settings.sidebarWorkingShelfEnabled !== + DEFAULT_UNIFIED_SETTINGS.sidebarWorkingShelfEnabled + ? ["Working section"] + : []), ...(settings.sidebarAutoSettleAfterDays !== DEFAULT_UNIFIED_SETTINGS.sidebarAutoSettleAfterDays ? ["Auto-settle inactive threads"] @@ -686,6 +690,7 @@ export function useSettingsRestore(onRestored?: () => void) { settings.autoResumeLimitedThreads, settings.snoozeLimitedThreads, settings.sidebarProjectGroupingMode, + settings.sidebarWorkingShelfEnabled, settings.sidebarThreadPreviewCount, settings.showSkillsInSlashMenu, settings.timestampFormat, @@ -784,6 +789,7 @@ export function useSettingsRestore(onRestored?: () => void) { panelAnimationDurationMs: DEFAULT_UNIFIED_SETTINGS.panelAnimationDurationMs, sidebarThreadPreviewCount: DEFAULT_UNIFIED_SETTINGS.sidebarThreadPreviewCount, sidebarProjectGroupingMode: DEFAULT_UNIFIED_SETTINGS.sidebarProjectGroupingMode, + sidebarWorkingShelfEnabled: DEFAULT_UNIFIED_SETTINGS.sidebarWorkingShelfEnabled, sidebarAutoSettleAfterDays: DEFAULT_UNIFIED_SETTINGS.sidebarAutoSettleAfterDays, sidebarAutoSettleOnMerge: DEFAULT_UNIFIED_SETTINGS.sidebarAutoSettleOnMerge, autoResumeLimitedThreads: DEFAULT_UNIFIED_SETTINGS.autoResumeLimitedThreads, @@ -2297,6 +2303,34 @@ export function GeneralSettingsPanel() { /> } /> + + + updateSettings({ + sidebarWorkingShelfEnabled: DEFAULT_UNIFIED_SETTINGS.sidebarWorkingShelfEnabled, + }) + } + /> + ) : null + } + control={ + + updateSettings({ sidebarWorkingShelfEnabled: Boolean(checked) }) + } + aria-label="Working section (beta)" + /> + } + /> + {supportsAutoSettlement ? ( <> { + tr.setTime(clock); + previous = groupUndoByChangeKind(tr, previous); + state = state.apply(tr); + }; + dispatch(state.tr.setSelection(TextSelection.atEnd(state.doc))); + const api = { + text: () => state.doc.textContent, + wait: (ms: number) => ((clock += ms), api), + type: (s: string, gap = 80) => { + for (const ch of s) { + clock += gap; + dispatch(state.tr.insertText(ch)); + } + return api; + }, + backspace: (n: number) => { + for (let i = 0; i < n; i += 1) { + clock += 80; + const at = state.selection.from; + dispatch(state.tr.delete(at - 1, at)); + } + return api; + }, + /** + * The transaction the composer's `handlePaste` dispatches: a plain insert, + * tagged by `markAsClipboardEdit`. ProseMirror's own paste path would tag + * it, but the composer takes every text paste before that path runs. + */ + paste: (s: string) => { + clock += 80; + dispatch(markAsClipboardEdit(state.tr.insertText(s), "paste")); + return api; + }, + /** The composer's cut handler: a plain delete of the selection, tagged. */ + cut: (n: number) => { + clock += 80; + const at = state.selection.from; + dispatch(markAsClipboardEdit(state.tr.delete(at - n, at), "cut")); + return api; + }, + /** A store-driven rewrite: autocomplete inserting a chip, list continuation. */ + rewrite: (next: string) => { + clock += 80; + dispatch( + state.tr.replaceWith( + 0, + state.doc.content.size, + schema.node("paragraph", null, [schema.text(next)]), + ), + ); + dispatch(state.tr.setSelection(TextSelection.atEnd(state.doc))); + return api; + }, + /** IME input replaces its own range on every keystroke. */ + compose: (s: string) => { + const start = state.selection.from; + for (let i = 1; i <= s.length; i += 1) { + clock += 80; + dispatch( + state.tr.insertText(s.slice(0, i), start, start + i - 1).setMeta("composition", 1), + ); + } + return api; + }, + undo: () => (undo(state, dispatch), api), + }; + return api; +} + +describe("composer undo grouping", () => { + it("keeps a continuous run of typing as one step", () => { + expect(composer().type("hello world").undo().text()).toBe(""); + }); + + it("starts a new step when typing turns into deleting, and back", () => { + const editor = composer().type("hello world").backspace(5).type("there"); + expect(editor.text()).toBe("hello there"); + expect(editor.undo().text()).toBe("hello "); + expect(editor.undo().text()).toBe("hello world"); + expect(editor.undo().text()).toBe(""); + }); + + it("gives a store rewrite its own step, and typing after it another", () => { + const editor = composer("draft ").type("one").rewrite("draft one @chip ").type("then"); + expect(editor.undo().text()).toBe("draft one @chip "); + expect(editor.undo().text()).toBe("draft one"); + expect(editor.undo().text()).toBe("draft "); + }); + + it("gives a paste its own step", () => { + const editor = composer().type("see ").paste("pasted text").type(" ok"); + expect(editor.undo().text()).toBe("see pasted text"); + expect(editor.undo().text()).toBe("see "); + }); + + it("gives a cut its own step between deletions", () => { + const editor = composer().type("hello world").backspace(1).cut(3).backspace(1); + expect(editor.text()).toBe("hello "); + expect(editor.undo().text()).toBe("hello w"); + expect(editor.undo().text()).toBe("hello worl"); + expect(editor.undo().text()).toBe("hello world"); + }); + + it("keeps IME composition as one run of typing", () => { + expect(composer().type("go ").compose("にほん").undo().text()).toBe(""); + }); + + it("merges typing across a short pause and splits it across a long one", () => { + expect(composer().type("hello").wait(700).type(" world").undo().text()).toBe(""); + expect(composer().type("hello").wait(1100).type(" world").undo().text()).toBe("hello"); + }); +}); diff --git a/apps/web/src/composer-undo-grouping.ts b/apps/web/src/composer-undo-grouping.ts new file mode 100644 index 0000000000000..f895b8d7afc60 --- /dev/null +++ b/apps/web/src/composer-undo-grouping.ts @@ -0,0 +1,64 @@ +import { closeHistory, isHistoryTransaction } from "@tiptap/pm/history"; +import type { Transaction } from "@tiptap/pm/state"; +import { ReplaceStep } from "@tiptap/pm/transform"; + +/** + * Undo grouping matching the Lexical composer it replaced. + * + * ProseMirror's history only starts a new undo step after a pause or when an + * edit is not next to the last one, so typing, backspacing and typing again + * without pausing is one step, and a store rewrite of the whole document + * (autocomplete inserting a chip, list continuation) makes every later edit + * "adjacent" and joins it. Lexical instead started a new step whenever the + * kind of change switched, and after one second without changes. + */ +export const COMPOSER_UNDO_GROUP_DELAY = 1000; + +export type ComposerChangeKind = "insert" | "delete" | "other"; + +/** + * Tags a transaction as a paste or cut, so it becomes its own undo step. + * ProseMirror tags these itself only on its built-in clipboard paths; the + * composer's `handlePaste` and cut handler replace those paths, so each of + * their dispatches must call this, or the change merges with the typing or + * deleting around it. + */ +export function markAsClipboardEdit(tr: Transaction, event: "paste" | "cut"): Transaction { + return tr.setMeta("uiEvent", event); +} + +function changeKindOf(tr: Transaction): ComposerChangeKind | null { + if (!tr.docChanged) return null; + const uiEvent = tr.getMeta("uiEvent"); + if (uiEvent === "paste" || uiEvent === "drop" || uiEvent === "cut") return "other"; + // IME input replaces its own range on each keystroke; it is still typing. + if (tr.getMeta("composition") != null) return "insert"; + let kind: ComposerChangeKind | null = null; + for (const step of tr.steps) { + if (!(step instanceof ReplaceStep)) return "other"; + const inserts = step.slice.size > 0; + const deletes = step.to > step.from; + const stepKind = inserts && !deletes ? "insert" : deletes && !inserts ? "delete" : "other"; + if (kind !== null && kind !== stepKind) return "other"; + kind = stepKind; + } + return kind; +} + +/** + * Closes the current undo step before `tr` when its kind of change differs + * from the last one, or when it is a one-off change such as a paste or a + * store rewrite, and returns the kind to remember for the next transaction. + * Undo and redo themselves count as one-off, so typing after them starts a + * fresh step. Must run before the transaction is applied. + */ +export function groupUndoByChangeKind( + tr: Transaction, + previous: ComposerChangeKind | null, +): ComposerChangeKind | null { + if (isHistoryTransaction(tr)) return "other"; + const kind = changeKindOf(tr); + if (kind === null) return previous; + if (previous !== null && (kind !== previous || kind === "other")) closeHistory(tr); + return kind; +} diff --git a/apps/web/src/pierre-icons.test.ts b/apps/web/src/pierre-icons.test.ts index 5f5aca691e99f..6090d7ed9e72b 100644 --- a/apps/web/src/pierre-icons.test.ts +++ b/apps/web/src/pierre-icons.test.ts @@ -56,7 +56,12 @@ describe("Pierre file icons", () => { it("normalizes common markdown fence language aliases", () => { assert.equal(syntheticFileNameForLanguageId("typescript"), "file.ts"); + assert.equal(syntheticFileNameForLanguageId("dockerfile"), "Dockerfile"); assert.equal(syntheticFileNameForLanguageId("shellscript"), "file.sh"); assert.equal(syntheticFileNameForLanguageId("python"), "file.py"); }); + + it("gives a dockerfile fence the Docker icon", () => { + assert.isTrue(hasSpecificPierreIconForFileName(syntheticFileNameForLanguageId("dockerfile"))); + }); }); diff --git a/apps/web/src/pierre-icons.ts b/apps/web/src/pierre-icons.ts index 49aec9cc4cef3..46f0099fedee3 100644 --- a/apps/web/src/pierre-icons.ts +++ b/apps/web/src/pierre-icons.ts @@ -51,7 +51,6 @@ const completeIconResolver = createFileTreeIconResolver(T3_PIERRE_ICONS); const LANGUAGE_EXTENSION_ALIASES: Record = { bash: "sh", csharp: "cs", - dockerfile: "dockerfile", javascript: "js", jsx: "jsx", markdown: "md", @@ -79,9 +78,17 @@ export function inferEntryKindFromPath(pathValue: string): "file" | "directory" return base.includes(".") ? "file" : "directory"; } +/** Languages whose files are recognised by name rather than by extension. */ +const LANGUAGE_FILE_NAMES: Record = { + dockerfile: "Dockerfile", +}; + export function syntheticFileNameForLanguageId(languageId: string): string { const normalized = languageId.toLowerCase(); - return `file.${LANGUAGE_EXTENSION_ALIASES[normalized] ?? normalized}`; + return ( + LANGUAGE_FILE_NAMES[normalized] ?? + `file.${LANGUAGE_EXTENSION_ALIASES[normalized] ?? normalized}` + ); } export function resolvePierreIconForEntry( diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index b3bc6c61fdc73..1172678440c7f 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -109,6 +109,16 @@ rename a thread, regenerate its title, or link and unlink a pull request. These appear on web, desktop, and mobile without requiring the originating browser to remain open. +### Fold working threads (beta) + +On web and desktop, turn on **Settings → General → Working section (beta)** to move threads that +are working or monitoring into a collapsed **Working** section at the bottom of the sidebar. A +thread returns to the top of the active list when it finishes, fails, or needs an approval or +answer. Pinned threads stay in the pinned section. + +While this is on, the active list is ordered by when each thread last came back to you, so you +cannot drag to reorder it. Your saved order returns when you turn it off. + ## Settle finished work Choose **Settle thread** from its menu to move finished work out of the active list diff --git a/docs/user/updating.md b/docs/user/updating.md index 191c0486bb3d2..c0f05fc89351f 100644 --- a/docs/user/updating.md +++ b/docs/user/updating.md @@ -71,6 +71,13 @@ update can roll back to the previous version. If the update still fails: 2. Check that you updated the server's machine, not only the device you are using. 3. For a command-line server, stop it and relaunch the exact version shown in the notice. +## Update providers + +**Settings → Providers** shows provider updates for the selected environment. +**Update all** updates every outdated provider on every connected environment +at once. Hover it to see which providers it will update. Providers that only +offer a manual update command are not included. + ## Mobile updates To update an environment from your phone, open **Settings → Environments** and diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index c06c54de89182..cf8c00db0a884 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -747,6 +747,8 @@ export const PullRequestSummary = Schema.Struct({ reviewDecision: Schema.optional(Schema.NullOr(PullRequestReviewDecision)), checksState: Schema.optional(Schema.NullOr(PullRequestChecksState)), mergeability: Schema.optional(PullRequestMergeability), + /** Null when the host says the pull request is in no stack; absent when the read did not ask. */ + stack: Schema.optional(Schema.NullOr(PullRequestStackMembership)), }); export type PullRequestSummary = typeof PullRequestSummary.Type; diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index d3557a571579a..b9e5feb621a14 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -460,6 +460,12 @@ export const ClientSettingsSchema = Schema.Struct({ // old keys, so everyone, including prior beta opt-outs, resets to the new // default sidebar. legacySidebarEnabled: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), + // Beta: working and monitoring threads fold into a Working shelf and return + // to the top of the inbox once they need the user. The inbox then orders by + // time, so manual placement there is ignored (and kept) while it is on. + sidebarWorkingShelfEnabled: Schema.Boolean.pipe( + Schema.withDecodingDefault(Effect.succeed(false)), + ), sidebarProjectGroupingMode: SidebarProjectGroupingMode.pipe( Schema.withDecodingDefault(Effect.succeed(DEFAULT_SIDEBAR_PROJECT_GROUPING_MODE)), ), @@ -1775,6 +1781,7 @@ export const ClientSettingsPatch = Schema.Struct({ proactivePanelsEnabled: Schema.optionalKey(Schema.Boolean), showSkillsInSlashMenu: Schema.optionalKey(Schema.Boolean), legacySidebarEnabled: Schema.optionalKey(Schema.Boolean), + sidebarWorkingShelfEnabled: Schema.optionalKey(Schema.Boolean), sidebarProjectGroupingMode: Schema.optionalKey(SidebarProjectGroupingMode), sidebarProjectGroupingOverrides: Schema.optionalKey( Schema.Record(TrimmedNonEmptyString, SidebarProjectGroupingMode),