From 018be987bbe4ee9f4da6ad686914a417faf7c8c4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 06:11:30 +0200 Subject: [PATCH 1/2] sync: upstream e16b8b059c..0f602b3372 (13 commits) --- PATCH.md | 38 +++ .../src/electron/ElectronShell.test.ts | 30 ++- apps/desktop/src/electron/ElectronShell.ts | 15 +- apps/web/src/components/ChatView.tsx | 1 + .../components/CommandPalette.logic.test.ts | 26 ++ .../src/components/CommandPalette.logic.ts | 3 + apps/web/src/components/Sidebar.logic.test.ts | 29 ++- apps/web/src/components/Sidebar.logic.ts | 16 +- apps/web/src/components/Sidebar.tsx | 8 +- apps/web/src/components/chat/ChatComposer.tsx | 2 +- .../components/chat/ExpandedImageDialog.tsx | 18 +- .../src/components/chat/MessagesTimeline.tsx | 2 +- .../components/chat/ProviderModelPicker.tsx | 6 +- .../components/chat/ZoomableImage.test.tsx | 85 +++++++ .../web/src/components/chat/ZoomableImage.tsx | 239 ++++++++++++++++++ .../components/pullRequest/PullRequestRow.tsx | 4 +- apps/web/src/remoteOpen.test.ts | 12 +- .../ghostty/surface.middle-click.test.ts | 204 +++++++++++++++ apps/web/src/terminal/ghostty/surface.ts | 28 +- docs/user/attachments.md | 7 + docs/user/source-control.md | 9 + docs/user/terminal.md | 4 + packages/contracts/src/editor.ts | 25 +- packages/shared/package.json | 4 + packages/shared/src/threadPullRequests.ts | 9 + 25 files changed, 782 insertions(+), 42 deletions(-) create mode 100644 apps/web/src/components/chat/ZoomableImage.test.tsx create mode 100644 apps/web/src/components/chat/ZoomableImage.tsx create mode 100644 apps/web/src/terminal/ghostty/surface.middle-click.test.ts create mode 100644 packages/shared/src/threadPullRequests.ts diff --git a/PATCH.md b/PATCH.md index bc82fb09f..6f7522099 100644 --- a/PATCH.md +++ b/PATCH.md @@ -357,6 +357,44 @@ This fork stays close to `pingdotgg/t3code` and carries only the following opera ownership, ordinary failures do not abort the batch, and failed/unprocessed threads stay selected. Navigation and worktree-cleanup failures are reported separately from a completed deletion, including the fork's archived-thread deletion path. +- The 2026-09-10 sync (`e16b8b059c..0f602b3372`, 13 upstream commits) carries seven + independent changes and retains the following boundaries: + - Linked-PR search (`f0401c6290`) runs on the existing V2 `linkedPullRequest` in web's + sidebar and command palette. The shared `threadPullRequestSearchTerms` exposes the PR + number, repository and URL without host reads. It does not introduce the upstream + multi-link projection, title snapshots or frozen Expo changes. + - Image zoom/pan (`8d8189e67d`) uses the fork's `ExpandedImageDialog` gallery, keeping its + original download handling. Zoom resets on navigation, arrow keys pan while zoomed, + and modal presence blocks type-to-focus. The standalone image component tolerates SSR. + - Composer model labels use available width (`b7b3ef1e6f`, web half); touch devices expose + user-message copy controls (`385cc0a4c6`, assistant controls were already visible). + PR list diff counts move to the title's trailing edge (`addfb1390e`) within the existing + row layout; review/check metadata already lives on the second line. + - Linux/BSD middle-click pastes the terminal's own selection (`d1eeb16247`) through the + existing paste race/bracketed-paste path. VT mouse reporting keeps priority; the fork's + modifier-click links, native copy, selection and split-pane activation remain intact. + Zed remote SSH links (`0f602b3372`) use the shared editor catalog and the fork's Electron + external-link validator. No new runtime capability or migration is needed. + - Duplicate-command expansion (`50f918c57a`) is already covered by V2's + `buildToolCallExpandedBody` / projected-item disclosure; the fork has no + `commandMatchesVisibleLabel` expansion guard. Android feed positioning (`75e4ceb964`) + and glass backing (`383cc40f4d`) remain excluded under the Expo freeze. + - Multiple linked PRs (`afb84898be`) remain deferred for a coordinated V2/Swift port: + upstream introduces host-level link identity, stack-dismissal tombstones, cached snapshots, + multi-PR settlement, automatic linking after creation, and credential-scoped MCP tools. + These must land together on V2's JSON projection and existing MCP capability model. + `050_ProjectionThreadPullRequests` targets V1 tables and collides with a fork-owned number; + it is dropped. The `threadPullRequests` capability, V1 link/unlink commands, RPCs, + provider instructions, client-runtime commands and dependent UI are not advertised/carried. + - GitHub stack navigation/merge/rebase (`de37964db2`) remains deferred with the multi-PR + stack service. A dedicated port must retain reviewed-head checks, branch permissions, + partial-rebase reporting and remote-only operations, then adapt the fork's panel stores + and native client. The `pullRequestStackActions` capability is not carried. + - Restart-persistent PR reads (`33242d0164`) cache the upstream `summary` / `stack` service + methods that the fork does not have (its earlier PR-discovery port is also deferred). + Do not add an unused cache layer or replace the fork's detail-cache semantics by inference. + Carry this with the missing service, including expiry and mutation/in-flight invalidation. + Advancing this sync marker records review of these deferred commits, not feature support. - The 2026-09-09 sync (`223ff4490f..e16b8b059c`, 185 upstream commits) manually carries independent correctness fixes while retaining the boundaries above: - `thread.stop` (`09e8de9c65`) uses web/desktop's existing V2 `interruptThreadTurn` path. diff --git a/apps/desktop/src/electron/ElectronShell.test.ts b/apps/desktop/src/electron/ElectronShell.test.ts index 9ae6f502b..a4ad375bc 100644 --- a/apps/desktop/src/electron/ElectronShell.test.ts +++ b/apps/desktop/src/electron/ElectronShell.test.ts @@ -52,6 +52,33 @@ describe("ElectronShell", () => { }).pipe(Effect.provide(ElectronShell.layer)), ); + it.effect("opens Zed's ssh deep link", () => + Effect.gen(function* () { + openExternalMock.mockResolvedValue(undefined); + + const electronShell = yield* ElectronShell.ElectronShell; + const result = yield* electronShell.openExternal("zed://ssh/example.com/home/user/project"); + + assert.equal(result, true); + assert.deepEqual(openExternalMock.mock.calls, [["zed://ssh/example.com/home/user/project"]]); + }).pipe(Effect.provide(ElectronShell.layer)), + ); + + it.effect("does not open editor URLs that mix up link shapes", () => + Effect.gen(function* () { + openExternalMock.mockResolvedValue(undefined); + + const electronShell = yield* ElectronShell.ElectronShell; + const results = yield* Effect.all([ + electronShell.openExternal("zed://extension/attacker"), + electronShell.openExternal("vscode://ssh/example.com/home/user/project"), + ]); + + assert.deepEqual(results, [false, false]); + assert.equal(openExternalMock.mock.calls.length, 0); + }).pipe(Effect.provide(ElectronShell.layer)), + ); + it.effect("does not open remote editor URLs with userinfo", () => Effect.gen(function* () { openExternalMock.mockResolvedValue(undefined); @@ -64,9 +91,10 @@ describe("ElectronShell", () => { electronShell.openExternal( "vscode://:secret@vscode-remote/ssh-remote+example.com/home/user/project", ), + electronShell.openExternal("zed://ssh/user@example.com/home/user/project"), ]); - assert.deepEqual(results, [false, false]); + assert.deepEqual(results, [false, false, false]); assert.equal(openExternalMock.mock.calls.length, 0); }).pipe(Effect.provide(ElectronShell.layer)), ); diff --git a/apps/desktop/src/electron/ElectronShell.ts b/apps/desktop/src/electron/ElectronShell.ts index 2ed13bfeb..756dfce2e 100644 --- a/apps/desktop/src/electron/ElectronShell.ts +++ b/apps/desktop/src/electron/ElectronShell.ts @@ -6,8 +6,8 @@ import * as Option from "effect/Option"; import * as Electron from "electron"; -// Remote open-in-editor deep links (`vscode://vscode-remote/ssh-remote+…`) -// must reach the OS handler; every other non-web scheme stays blocked. +// Remote editor links use VS Code’s vscode-remote shape or Zed’s ssh shape. +// Other non-web schemes stay blocked. const SAFE_WEB_PROTOCOLS = new Set(["http:", "https:"]); const REMOTE_EDITOR_PROTOCOLS = new Set( REMOTE_CAPABLE_EDITOR_IDS.flatMap((id) => { @@ -16,13 +16,18 @@ const REMOTE_EDITOR_PROTOCOLS = new Set( }), ); +// Zed's host sits in the first path segment, so it needs its own userinfo ban. +const ZED_SSH_PATHNAME = /^\/[^/@:]+\/.+$/; + const isRemoteEditorUrl = (url: URL) => REMOTE_EDITOR_PROTOCOLS.has(url.protocol) && url.username.length === 0 && url.password.length === 0 && - url.host === "vscode-remote" && - url.pathname.startsWith("/ssh-remote+") && - url.pathname.length > "/ssh-remote+".length; + (url.protocol === "zed:" + ? url.host === "ssh" && ZED_SSH_PATHNAME.test(url.pathname) + : url.host === "vscode-remote" && + url.pathname.startsWith("/ssh-remote+") && + url.pathname.length > "/ssh-remote+".length); export function parseSafeExternalUrl(rawUrl: unknown): Option.Option { if (typeof rawUrl !== "string") { diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 80a224bb6..4e36fd1f7 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -540,6 +540,7 @@ const TYPE_TO_FOCUS_INTERACTIVE_SELECTOR = [ '[role="tab"]', ].join(","); const TYPE_TO_FOCUS_FLOATING_LAYER_SELECTOR = [ + '[role="dialog"][aria-modal="true"]', '[data-slot="dialog"]', '[data-slot="menu-popup"]', '[data-slot="select-popup"]', diff --git a/apps/web/src/components/CommandPalette.logic.test.ts b/apps/web/src/components/CommandPalette.logic.test.ts index 4f6644ca8..9ebc44d7b 100644 --- a/apps/web/src/components/CommandPalette.logic.test.ts +++ b/apps/web/src/components/CommandPalette.logic.test.ts @@ -169,6 +169,32 @@ function makeThread(overrides: Partial = {}): Thread { } describe("buildThreadActionItems", () => { + it("includes the V2 linked PR in thread search", () => { + const [item] = buildThreadActionItems({ + threads: [ + makeThread({ + linkedPullRequest: { + projectId: PROJECT_ID, + repository: "Bl4ckBl1zZ/t3code", + number: 287, + url: "https://github.com/Bl4ckBl1zZ/t3code/pull/287", + }, + }), + ], + projectTitleById: new Map([[PROJECT_ID, "T3 Code"]]), + sortOrder: "updated_at", + icon: null, + runThread: async () => undefined, + }); + expect(item?.searchTerms).toEqual( + expect.arrayContaining([ + "#287", + "Bl4ckBl1zZ/t3code#287", + "https://github.com/Bl4ckBl1zZ/t3code/pull/287", + ]), + ); + }); + it("orders threads by most recent activity and formats timestamps from updatedAt", () => { vi.useFakeTimers(); vi.setSystemTime(new Date("2026-03-25T12:00:00.000Z")); diff --git a/apps/web/src/components/CommandPalette.logic.ts b/apps/web/src/components/CommandPalette.logic.ts index db1709fac..49a6c322e 100644 --- a/apps/web/src/components/CommandPalette.logic.ts +++ b/apps/web/src/components/CommandPalette.logic.ts @@ -1,3 +1,4 @@ +import { threadPullRequestSearchTerms } from "@t3tools/shared/threadPullRequests"; import { type FilesystemBrowseEntry, type KeybindingCommand, @@ -180,6 +181,7 @@ export type BuildThreadActionItemsThread = Pick< > & { updatedAt: string; latestUserMessageAt?: string | null; + linkedPullRequest?: SidebarThreadSummary["linkedPullRequest"]; }; export function buildThreadActionItems(input: { @@ -232,6 +234,7 @@ export function buildThreadActionItems { }); }); -describe("searchSidebarThreadsByTitle", () => { +describe("searchSidebarThreads", () => { + it("finds the V2 linked PR by number, repository or URL without changing order", () => { + const linkedPullRequest = { + projectId: ProjectId.make("project"), + repository: "Bl4ckBl1zZ/t3code", + number: 287, + url: "https://github.com/Bl4ckBl1zZ/t3code/pull/287", + }; + const threads = [ + { title: "First", linkedPullRequest }, + { title: "Unlinked", linkedPullRequest: null }, + { title: "Last", linkedPullRequest }, + ]; + for (const query of [" #287 ", "bl4ckbl1zz/t3code#287", linkedPullRequest.url]) { + expect(searchSidebarThreads(threads, query)).toEqual([threads[0], threads[2]]); + } + expect(searchSidebarThreads(threads, "#999")).toEqual([]); + expect(searchSidebarThreads([{ title: "Older server" }], "#287")).toEqual([]); + }); + const threads = [ { id: "thread-1", title: "Fix workspace search", project: "Alpha" }, { id: "thread-2", title: "Review providers", project: "Workspace" }, @@ -1372,15 +1391,15 @@ describe("searchSidebarThreadsByTitle", () => { ]; it("matches thread titles case-insensitively and preserves their order", () => { - expect(searchSidebarThreadsByTitle(threads, "work")).toEqual([threads[0], threads[2]]); + expect(searchSidebarThreads(threads, "work")).toEqual([threads[0], threads[2]]); }); it("does not match project metadata", () => { - expect(searchSidebarThreadsByTitle(threads, "workspace")).toEqual([threads[0]]); + expect(searchSidebarThreads(threads, "workspace")).toEqual([threads[0]]); }); it("returns no results for an empty query", () => { - expect(searchSidebarThreadsByTitle(threads, " ")).toEqual([]); + expect(searchSidebarThreads(threads, " ")).toEqual([]); }); }); diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts index 108737392..9c9ab59c6 100644 --- a/apps/web/src/components/Sidebar.logic.ts +++ b/apps/web/src/components/Sidebar.logic.ts @@ -1,3 +1,4 @@ +import { threadPullRequestSearchTerms } from "@t3tools/shared/threadPullRequests"; import { isAtomCommandInterrupted, type AtomCommandResult, @@ -1021,17 +1022,20 @@ export { export { sortPinnedThreadsByOrderKey as sortPinnedThreadsForSidebar } from "@t3tools/client-runtime/state/thread-sort"; /** - * Search the already-ordered sidebar thread collection by title only. + * Search the already-ordered sidebar thread collection by title or linked PR. * Keeping the input order means lifecycle ordering (active, snoozed, settled) * remains stable while the user narrows the list. */ -export function searchSidebarThreadsByTitle( - threads: readonly T[], - query: string, -): T[] { +export function searchSidebarThreads< + T extends { readonly title: string } & Parameters[0], +>(threads: readonly T[], query: string): T[] { const normalizedQuery = query.trim().toLowerCase(); if (normalizedQuery.length === 0) return []; - return threads.filter((thread) => thread.title.toLowerCase().includes(normalizedQuery)); + return threads.filter((thread) => + [thread.title, ...threadPullRequestSearchTerms(thread)].some((term) => + term.toLowerCase().includes(normalizedQuery), + ), + ); } type SettledTimestampInput = Pick< diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 01543ec6a..86de43274 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -165,7 +165,7 @@ import { resolveSettledTimestamp, resolveSidebarThreadStatus, resolveThreadLastVisitedAt, - searchSidebarThreadsByTitle, + searchSidebarThreads, shouldCreateNewThreadInCurrentProject, resolveWorkingStartedAt, resolveWorkInboxBadge, @@ -2765,7 +2765,7 @@ export default function Sidebar() { [activeThreads, settledThreads, snoozedThreads], ); const threadSearchResults = useMemo( - () => searchSidebarThreadsByTitle(searchableThreads, threadSearchQuery), + () => searchSidebarThreads(searchableThreads, threadSearchQuery), [searchableThreads, threadSearchQuery], ); const threadSearchResultOrderKey = threadSearchResults @@ -4053,8 +4053,8 @@ export default function Sidebar() { setActiveSearchResultIndex(0); }} onKeyDown={handleThreadSearchKeyDown} - placeholder="Search" - aria-label="Search threads" + placeholder="Search threads or PRs" + aria-label="Search threads or PRs" role="combobox" aria-autocomplete="list" aria-expanded={isSearchingThreads && threadSearchResults.length > 0} diff --git a/apps/web/src/components/chat/ChatComposer.tsx b/apps/web/src/components/chat/ChatComposer.tsx index de7e13576..6fe9501d2 100644 --- a/apps/web/src/components/chat/ChatComposer.tsx +++ b/apps/web/src/components/chat/ChatComposer.tsx @@ -3596,7 +3596,7 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps) ) : ( void; @@ -48,6 +50,7 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ onClose, }: ExpandedImageDialogProps) { const [imageOffset, setImageOffset] = useState(0); + const zoomableImageRef = useRef(null); const index = (preview.index + imageOffset + preview.images.length) % preview.images.length; const navigateImage = useCallback((direction: -1 | 1) => { @@ -62,6 +65,11 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ onClose(); return; } + if (zoomableImageRef.current?.pan(event.key)) { + event.preventDefault(); + event.stopPropagation(); + return; + } if (preview.images.length <= 1) return; if (event.key === "ArrowLeft") { event.preventDefault(); @@ -137,11 +145,11 @@ export const ExpandedImageDialog = memo(function ExpandedImageDialog({ - {item.name}

{item.name} diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index c2c9df567..29cf0759a 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -1261,7 +1261,7 @@ function UserTimelineRow({ row }: { row: Extract ) : null} -

+
}> diff --git a/apps/web/src/components/chat/ProviderModelPicker.tsx b/apps/web/src/components/chat/ProviderModelPicker.tsx index db55edcf8..93bdf93c4 100644 --- a/apps/web/src/components/chat/ProviderModelPicker.tsx +++ b/apps/web/src/components/chat/ProviderModelPicker.tsx @@ -34,7 +34,7 @@ export const ProviderModelPicker = memo(function ProviderModelPicker(props: { keybindings?: ResolvedKeybindingsConfig; modelOptionsByInstance: ReadonlyMap>; activeProviderIconClassName?: string; - compact?: boolean; + isComposerOwned?: boolean; disabled?: boolean; terminalOpen?: boolean; open?: boolean; @@ -150,8 +150,8 @@ export const ProviderModelPicker = memo(function ProviderModelPicker(props: { variant={props.triggerVariant ?? "ghost"} data-chat-provider-model-picker="true" className={cn( - "min-w-0 justify-between whitespace-nowrap", - props.compact ? "max-w-42 shrink-0" : "max-w-48 shrink sm:max-w-56", + "min-w-0 shrink justify-between whitespace-nowrap", + !props.isComposerOwned && "max-w-48 sm:max-w-56", props.triggerClassName, )} disabled={props.disabled} diff --git a/apps/web/src/components/chat/ZoomableImage.test.tsx b/apps/web/src/components/chat/ZoomableImage.test.tsx new file mode 100644 index 000000000..093bf033b --- /dev/null +++ b/apps/web/src/components/chat/ZoomableImage.test.tsx @@ -0,0 +1,85 @@ +import { act, createRef } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, expect, it, vi } from "vite-plus/test"; + +import { ExpandedImageDialog } from "./ExpandedImageDialog"; +import { ZoomableImage, type ZoomableImageHandle } from "./ZoomableImage"; + +let renderer: ReactTestRenderer | undefined; +afterEach(async () => { + await act(async () => renderer?.unmount()); + renderer = undefined; + vi.unstubAllGlobals(); +}); + +it("zooms, pans with arrow keys, returns to fit and resets on gallery navigation", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const windowEvents = Object.assign(new EventTarget(), { innerWidth: 800, innerHeight: 600 }); + vi.stubGlobal("window", windowEvents); + const viewport = Object.assign(new EventTarget(), { + scrollLeft: 0, + scrollTop: 0, + clientWidth: 400, + clientHeight: 300, + getBoundingClientRect: () => ({ left: 0, top: 0 }), + }); + const preview = { + images: [ + { src: "/one.png", name: "One" }, + { src: "/two.png", name: "Two" }, + ], + index: 0, + }; + await act(async () => { + renderer = create( {}} />, { + createNodeMock: (element) => (element.type === "div" ? viewport : null), + }); + }); + const region = () => renderer!.root.findByProps({ role: "region" }); + const key = async (value: string) => { + const event = Object.assign(new Event("keydown", { cancelable: true }), { key: value }); + await act(async () => { + windowEvents.dispatchEvent(event); + }); + return event; + }; + await act(async () => region().props.onKeyDown({ key: "+", preventDefault() {} })); + expect(renderer!.root.findByProps({ "aria-live": "polite" }).children.join("")).toBe("150% zoom"); + const previousLeft = viewport.scrollLeft; + expect((await key("ArrowRight")).defaultPrevented).toBe(true); + expect(viewport.scrollLeft).toBe(previousLeft + 40); + expect(renderer!.root.findByType("img").props.src).toBe("/one.png"); + await act(async () => region().props.onKeyDown({ key: "0", preventDefault() {} })); + await key("ArrowRight"); + expect(renderer!.root.findByType("img").props.src).toBe("/two.png"); + expect(renderer!.root.findByProps({ "aria-live": "polite" }).children.join("")).toBe("100% zoom"); +}); + +it("clamps zoom and only captures arrow navigation while zoomed", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("window", Object.assign(new EventTarget(), { innerWidth: 800, innerHeight: 600 })); + const ref = createRef(); + const viewport = Object.assign(new EventTarget(), { + scrollLeft: 0, + scrollTop: 0, + clientWidth: 400, + clientHeight: 300, + getBoundingClientRect: () => ({ left: 0, top: 0 }), + }); + await act(async () => { + renderer = create(, { + createNodeMock: (element) => (element.type === "div" ? viewport : null), + }); + }); + expect(ref.current?.pan("ArrowRight")).toBe(false); + const region = renderer!.root.findByProps({ role: "region" }); + await act(async () => { + for (let i = 0; i < 20; i++) region.props.onKeyDown({ key: "+", preventDefault() {} }); + }); + expect(renderer!.root.findByProps({ "aria-live": "polite" }).children.join("")).toBe("800% zoom"); + expect(ref.current?.pan("Escape")).toBe(false); + await act(async () => { + for (let i = 0; i < 20; i++) region.props.onKeyDown({ key: "-", preventDefault() {} }); + }); + expect(ref.current?.pan("ArrowRight")).toBe(false); +}); diff --git a/apps/web/src/components/chat/ZoomableImage.tsx b/apps/web/src/components/chat/ZoomableImage.tsx new file mode 100644 index 000000000..1ea5ed4f0 --- /dev/null +++ b/apps/web/src/components/chat/ZoomableImage.tsx @@ -0,0 +1,239 @@ +import { + useCallback, + useEffect, + useImperativeHandle, + useLayoutEffect, + useRef, + useState, + type Ref, +} from "react"; + +const MAX_ZOOM = 8; + +export interface ZoomableImageHandle { + pan: (key: string) => boolean; +} + +/** Zooms around the pointer and keeps the whole image accessible by dragging or scrolling. */ +export function ZoomableImage({ + src, + name, + onError, + ref, +}: { + src: string; + name: string; + onError?: () => void; + ref?: Ref; +}) { + const viewportRef = useRef(null); + const [naturalSize, setNaturalSize] = useState({ width: 0, height: 0 }); + const [windowSize, setWindowSize] = useState(() => ({ + width: typeof window === "undefined" ? 800 : window.innerWidth, + height: typeof window === "undefined" ? 600 : window.innerHeight, + })); + const [zoom, setZoom] = useState(1); + const zoomRef = useRef(1); + const anchorRef = useRef<{ x: number; y: number; clientX: number; clientY: number } | null>(null); + const dragRef = useRef<{ + pointerId: number; + x: number; + y: number; + left: number; + top: number; + } | null>(null); + const suppressClickRef = useRef(false); + const [dragging, setDragging] = useState(false); + const maxHeight = Math.max(1, Math.min(windowSize.height * 0.86, windowSize.height - 80)); + const fit = Math.min( + 1, + (windowSize.width * 0.92) / (naturalSize.width || 1), + maxHeight / (naturalSize.height || 1), + ); + const width = naturalSize.width * fit * zoom; + const height = naturalSize.height * fit * zoom; + + useImperativeHandle( + ref, + () => ({ + pan(key) { + const viewport = viewportRef.current; + if (!viewport || zoomRef.current <= 1) return false; + switch (key) { + case "ArrowLeft": + viewport.scrollLeft -= 40; + break; + case "ArrowRight": + viewport.scrollLeft += 40; + break; + case "ArrowUp": + viewport.scrollTop -= 40; + break; + case "ArrowDown": + viewport.scrollTop += 40; + break; + default: + return false; + } + return true; + }, + }), + [], + ); + + const changeZoom = useCallback((next: number, point?: { x: number; y: number }) => { + const viewport = viewportRef.current; + const previous = zoomRef.current; + const clamped = Math.min(MAX_ZOOM, Math.max(1, next)); + if (!viewport || previous === clamped) return; + const bounds = viewport.getBoundingClientRect(); + const x = point ? point.x - bounds.left : viewport.clientWidth / 2; + const y = point ? point.y - bounds.top : viewport.clientHeight / 2; + anchorRef.current = { + x: (viewport.scrollLeft + x) / previous, + y: (viewport.scrollTop + y) / previous, + clientX: bounds.left + x, + clientY: bounds.top + y, + }; + zoomRef.current = clamped; + setZoom(clamped); + }, []); + + useLayoutEffect(() => { + const viewport = viewportRef.current; + const anchor = anchorRef.current; + if (!viewport || !anchor) return; + const bounds = viewport.getBoundingClientRect(); + viewport.scrollLeft = anchor.x * zoom - (anchor.clientX - bounds.left); + viewport.scrollTop = anchor.y * zoom - (anchor.clientY - bounds.top); + anchorRef.current = null; + }, [zoom]); + + useEffect(() => { + const resize = () => { + setWindowSize({ width: window.innerWidth, height: window.innerHeight }); + changeZoom(1); + }; + window.addEventListener("resize", resize); + return () => window.removeEventListener("resize", resize); + }, [changeZoom]); + + useEffect(() => { + const viewport = viewportRef.current; + if (!viewport) return; + const wheel = (event: WheelEvent) => { + if (event.deltaY === 0) return; + event.preventDefault(); + const delta = + event.deltaY * + (event.deltaMode === 1 ? 16 : event.deltaMode === 2 ? viewport.clientHeight : 1); + changeZoom(zoomRef.current * Math.exp(-delta * (event.ctrlKey ? 0.01 : 0.002)), { + x: event.clientX, + y: event.clientY, + }); + }; + viewport.addEventListener("wheel", wheel, { passive: false }); + return () => viewport.removeEventListener("wheel", wheel); + }, [changeZoom]); + + return ( +
+
1 ? (dragging ? "grabbing" : "grab") : "zoom-in", + }} + onClick={(event) => { + // Pointer capture also produces a click after dragging; leave the image zoomed. + if (suppressClickRef.current || event.detail > 1) return; + changeZoom(zoomRef.current > 1 ? 1 : 2, { x: event.clientX, y: event.clientY }); + }} + onKeyDown={(event) => { + if (event.ctrlKey || event.metaKey || event.altKey) return; + if (event.key === "Enter" || event.key === " ") { + event.preventDefault(); + if (!event.repeat) changeZoom(zoomRef.current > 1 ? 1 : 2); + } else if (event.key === "+" || event.key === "=") { + event.preventDefault(); + changeZoom(zoomRef.current * 1.5); + } else if (event.key === "-") { + event.preventDefault(); + changeZoom(zoomRef.current / 1.5); + } else if (event.key === "0") { + event.preventDefault(); + changeZoom(1); + } + }} + onPointerDown={(event) => { + if (dragRef.current) return; + suppressClickRef.current = false; + if (event.pointerType !== "mouse" || event.button !== 0 || zoomRef.current <= 1) return; + const viewport = event.currentTarget; + const bounds = viewport.getBoundingClientRect(); + if ( + event.clientX - bounds.left >= viewport.clientWidth || + event.clientY - bounds.top >= viewport.clientHeight + ) + return; + dragRef.current = { + pointerId: event.pointerId, + x: event.clientX, + y: event.clientY, + left: viewport.scrollLeft, + top: viewport.scrollTop, + }; + viewport.setPointerCapture(event.pointerId); + setDragging(true); + }} + onPointerMove={(event) => { + const drag = dragRef.current; + if (!drag || drag.pointerId !== event.pointerId) return; + if (Math.hypot(event.clientX - drag.x, event.clientY - drag.y) > 4) { + suppressClickRef.current = true; + } + event.currentTarget.scrollLeft = drag.left - (event.clientX - drag.x); + event.currentTarget.scrollTop = drag.top - (event.clientY - drag.y); + }} + onPointerUp={(event) => { + if (dragRef.current?.pointerId !== event.pointerId) return; + if (event.currentTarget.hasPointerCapture(event.pointerId)) { + event.currentTarget.releasePointerCapture(event.pointerId); + } + dragRef.current = null; + setDragging(false); + }} + onLostPointerCapture={(event) => { + if (dragRef.current?.pointerId !== event.pointerId) return; + dragRef.current = null; + setDragging(false); + }} + > + {name} { + setNaturalSize({ + width: event.currentTarget.naturalWidth, + height: event.currentTarget.naturalHeight, + }); + }} + onError={onError} + /> +
+ + {Math.round(zoom * 100)}% zoom + +
+ ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index 6ee686a9d..8a17bae57 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -115,7 +115,7 @@ function PullRequestRowImpl({ {entry.title} - + {formatRelativeTimeLabel(entry.updatedAt)} @@ -199,7 +199,7 @@ function PullRequestRowImpl({ diff --git a/apps/web/src/remoteOpen.test.ts b/apps/web/src/remoteOpen.test.ts index ff78967aa..6f7c17d81 100644 --- a/apps/web/src/remoteOpen.test.ts +++ b/apps/web/src/remoteOpen.test.ts @@ -141,8 +141,18 @@ describe("buildRemoteOpenUrl", () => { ).toBe("vscode://vscode-remote/ssh-remote+sol/C%3A/Users/theo"); }); + it("builds Zed's ssh deep link", () => { + expect( + buildRemoteOpenUrl({ + editor: "zed", + host: "sol.tail1234.ts.net", + absolutePath: "/home/theo/code/my repo", + }), + ).toBe("zed://ssh/sol.tail1234.ts.net/home/theo/code/my%20repo"); + }); + it("returns undefined for editors without remote support", () => { - expect(buildRemoteOpenUrl({ editor: "zed", host: "sol", absolutePath: "/tmp/x" })).toBe( + expect(buildRemoteOpenUrl({ editor: "idea", host: "sol", absolutePath: "/tmp/x" })).toBe( undefined, ); }); diff --git a/apps/web/src/terminal/ghostty/surface.middle-click.test.ts b/apps/web/src/terminal/ghostty/surface.middle-click.test.ts new file mode 100644 index 000000000..3ee03cd5b --- /dev/null +++ b/apps/web/src/terminal/ghostty/surface.middle-click.test.ts @@ -0,0 +1,204 @@ +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; +import { GhosttyTerminalCore } from "./core"; +import { GhosttyTerminalSurface, type GhosttyTerminalSurfaceOptions } from "./surface"; + +vi.mock("./vendor/ghostty-vt.wasm?url", async () => ({ + default: (await import("./vendor/ghostty-vt.wasm?inline")).default, +})); +vi.mock("./vendor/ghostty-write-pty.wasm?url&no-inline", async () => ({ + default: (await import("./vendor/ghostty-write-pty.wasm?inline")).default, +})); + +describe("GhosttyTerminalSurface middle-click paste", () => { + const surfaces = new Set(); + + // Keep the real surface, renderer, and WASM core. Only browser layout and + // scheduling are replaced so tests can count work while the terminal is hidden. + function createHarness() { + vi.useFakeTimers(); + const frames = new Map(); + const resizeCallbacks = new Set<() => void>(); + const paint = vi.fn((_operation: string, _args: ReadonlyArray) => {}); + let frameId = 0; + const requestFrame = vi.fn((callback: FrameRequestCallback) => { + frames.set(++frameId, callback); + return frameId; + }); + + class TerminalTestElement extends EventTarget { + style: Record = {}; + parentElement: TerminalTestElement | null = null; + clientWidth = 168; + clientHeight = 104; + width = 300; + height = 150; + value = ""; + private readonly captures = new Set(); + + setAttribute() {} + append(...children: TerminalTestElement[]) { + for (const child of children) child.parentElement = this; + } + replaceChildren(...children: TerminalTestElement[]) { + this.append(...children); + } + remove() { + this.parentElement = null; + } + getContext() { + return context; + } + focus() { + this.dispatchEvent(new Event("focus")); + } + setPointerCapture(pointerId: number) { + this.captures.add(pointerId); + } + hasPointerCapture(pointerId: number) { + return this.captures.has(pointerId); + } + releasePointerCapture(pointerId: number) { + this.captures.delete(pointerId); + } + getBoundingClientRect() { + return { left: 0, top: 0, right: 168, bottom: 104, width: 168, height: 104 }; + } + } + + const canvas = new TerminalTestElement(); + const mount = new TerminalTestElement(); + const context = { + canvas, + beginPath() {}, + clip() {}, + rect() {}, + resetTransform() {}, + restore() {}, + save() {}, + setTransform() {}, + fillRect: (...args: number[]) => paint("fillRect", args), + strokeRect: (...args: number[]) => paint("strokeRect", args), + fillText: (...args: [string, number, number, number?]) => paint("fillText", args), + measureText: (text: string) => ({ + width: text.length * 8, + actualBoundingBoxAscent: 9, + actualBoundingBoxDescent: 3, + }), + }; + vi.stubGlobal("document", { + createElement: (tag: string) => (tag === "canvas" ? canvas : new TerminalTestElement()), + fonts: Object.assign(new EventTarget(), { load: async () => [], add() {} }), + }); + vi.stubGlobal( + "window", + Object.assign(new EventTarget(), { + devicePixelRatio: 1, + requestAnimationFrame: requestFrame, + cancelAnimationFrame: (id: number) => frames.delete(id), + setTimeout, + clearTimeout, + setInterval, + clearInterval, + matchMedia: () => Object.assign(new EventTarget(), { matches: false }), + }), + ); + vi.stubGlobal( + "ResizeObserver", + class { + constructor(private readonly callback: () => void) { + resizeCallbacks.add(callback); + } + observe() {} + disconnect() { + resizeCallbacks.delete(this.callback); + } + }, + ); + const snapshot = vi.spyOn(GhosttyTerminalCore.prototype, "snapshot"); + const onData = vi.fn<(data: string) => void>(); + + return { + mount, + frames, + paint, + requestFrame, + snapshot, + onData, + get renderedSnapshot() { + const result = snapshot.mock.results.at(-1); + if (result?.type !== "return") throw new Error("No terminal snapshot was rendered"); + return result.value; + }, + flushFrame() { + const queued = [...frames.values()]; + frames.clear(); + for (const callback of queued) callback(0); + }, + resize() { + for (const callback of resizeCallbacks) callback(); + }, + pointer(type: string, clientX: number, buttons: number, shiftKey = false, button = 0) { + canvas.dispatchEvent( + Object.assign(new Event(type, { cancelable: true }), { + clientX, + clientY: 5, + pointerId: 1, + button, + buttons, + shiftKey, + }), + ); + }, + async create(options: Partial = {}) { + const surface = await GhosttyTerminalSurface.create(mount as unknown as HTMLElement, { + theme: { + foreground: { r: 255, g: 255, b: 255 }, + background: { r: 0, g: 0, b: 0 }, + cursor: { r: 255, g: 255, b: 255 }, + }, + onData, + onResize() {}, + onSelectionChange() {}, + beforeKey: () => false, + onLinkActivate() {}, + ...options, + }); + surfaces.add(surface); + return surface; + }, + }; + } + + afterEach(() => { + for (const surface of surfaces) surface.dispose(); + surfaces.clear(); + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("pastes the terminal selection, and only that, on a Linux middle click", async () => { + const harness = createHarness(); + const readText = vi.fn(async () => "clipboard text"); + vi.stubGlobal("navigator", { platform: "Linux x86_64", clipboard: { readText } }); + const surface = await harness.create(); + surface.write("hello world"); + harness.flushFrame(); + harness.pointer("pointerdown", 5, 1); + harness.pointer("pointermove", 37, 1); + harness.pointer("pointerup", 37, 0); + expect(surface.getSelection()).toBe("hello"); + + harness.onData.mockClear(); + harness.pointer("pointerdown", 5, 4, false, 1); + await vi.waitFor(() => expect(harness.onData).toHaveBeenCalled()); + expect(harness.onData.mock.calls.at(-1)?.[0]).toBe("hello"); + expect(surface.getSelection()).toBe("hello"); + + // Without a selection there is no primary buffer to paste; the clipboard + // holds what the user copied and must not be substituted. + surface.clearSelection(); + harness.pointer("pointerdown", 5, 4, false, 1); + expect(readText).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/terminal/ghostty/surface.ts b/apps/web/src/terminal/ghostty/surface.ts index df5c1f1cf..6fbc420fd 100644 --- a/apps/web/src/terminal/ghostty/surface.ts +++ b/apps/web/src/terminal/ghostty/surface.ts @@ -439,6 +439,11 @@ export function isTerminalSelectAllShortcut( return isMacPlatform(platform) ? event.metaKey : event.ctrlKey && event.shiftKey; } +/** Linux/BSD middle-click uses the terminal selection, never the clipboard. */ +function isMiddleClickPastePlatform(): boolean { + return /linux|bsd/i.test(navigator.platform); +} + export function isTerminalCompositionCommitInput(event: Pick): boolean { return ( event.inputType === "" || @@ -1362,6 +1367,14 @@ export class GhosttyTerminalSurface { this.canvas.setPointerCapture(event.pointerId); return; } + if (event.button === 1 && isMiddleClickPastePlatform()) { + // Keep mousedown bubbling so the containing split pane is activated. + const selection = this.getSelection(); + if (selection.length > 0) { + void this.pasteFromClipboard(() => Promise.resolve(selection)); + } + return; + } if (event.button !== 0) return; if (isTerminalLinkPointerGesture(event)) { event.preventDefault(); @@ -1574,6 +1587,10 @@ export class GhosttyTerminalSurface { if (this.canvas.hasPointerCapture(event.pointerId)) { this.canvas.releasePointerCapture(event.pointerId); } + if (event.button === 1 && isMiddleClickPastePlatform()) { + event.preventDefault(); + return; + } if (event.button !== 0) return; if (!this.selectionMoved && this.selectionMode === "cell") { this.clearSelection(); @@ -1610,10 +1627,17 @@ export class GhosttyTerminalSurface { }; private readonly onMouseDown = (event: MouseEvent) => { - if (event.button === 0) event.preventDefault(); + if (event.button === 0 || (event.button === 1 && isMiddleClickPastePlatform())) { + event.preventDefault(); + } this.focus(); }; + // Suppress Chromium’s native PRIMARY paste into the focused hidden textarea. + private readonly onMouseUp = (event: MouseEvent) => { + if (event.button === 1 && isMiddleClickPastePlatform()) event.preventDefault(); + }; + private readonly onContextMenu = (event: MouseEvent) => { if (shouldReportTerminalMouse(this.core.isMouseTracking(), event)) { event.preventDefault(); @@ -1703,6 +1727,7 @@ export class GhosttyTerminalSurface { this.canvas.addEventListener("pointercancel", this.onPointerUp); this.canvas.addEventListener("wheel", this.onWheel, { passive: false }); this.canvas.addEventListener("mousedown", this.onMouseDown); + this.canvas.addEventListener("mouseup", this.onMouseUp); this.canvas.addEventListener("contextmenu", this.onContextMenu); this.scrollbar.addEventListener("pointerdown", this.onScrollbarPointerDown); this.scrollbar.addEventListener("pointermove", this.onScrollbarPointerMove); @@ -1728,6 +1753,7 @@ export class GhosttyTerminalSurface { this.canvas.removeEventListener("pointercancel", this.onPointerUp); this.canvas.removeEventListener("wheel", this.onWheel); this.canvas.removeEventListener("mousedown", this.onMouseDown); + this.canvas.removeEventListener("mouseup", this.onMouseUp); this.canvas.removeEventListener("contextmenu", this.onContextMenu); this.scrollbar.removeEventListener("pointerdown", this.onScrollbarPointerDown); this.scrollbar.removeEventListener("pointermove", this.onScrollbarPointerMove); diff --git a/docs/user/attachments.md b/docs/user/attachments.md index 30e48c8d4..ef89f5629 100644 --- a/docs/user/attachments.md +++ b/docs/user/attachments.md @@ -36,3 +36,10 @@ Deleting a thread deletes that thread's uploads. After you send a message, each attachment shows the path it was saved to. Click the path to open the file, or use the menu to copy it. Conversations that have no project attached show no path — there is nowhere to save the file, so its contents are sent with the message as before. If T3 Code had a project but could not write to it (a read-only checkout, for example), the attachment is marked **Not saved to the workspace** and its contents are sent with the message instead. Either way the message still goes through. + +## Zooming image previews + +In web and desktop, open an image from the conversation, then click it to zoom in or return +to fit. Scroll to zoom and drag to pan. With the image focused, **+** and **−** change zoom +and **0** returns to fit. Arrow keys pan while zoomed and move between images while fitted. +Downloading still saves the original image. diff --git a/docs/user/source-control.md b/docs/user/source-control.md index ac11e3a06..69ea41133 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -156,3 +156,12 @@ Control settings**. - [GitHub CLI](https://cli.github.com/) - [GitLab CLI](https://gitlab.com/gitlab-org/cli) - [Azure CLI](https://learn.microsoft.com/en-us/cli/azure/) + +## Finding a thread by its linked pull request + +On web and desktop, sidebar search and the command palette match the linked PR number +(such as **#287**), repository plus number, or URL. This searches links already attached to +threads; it does not query the source-control host. + +Remote **Open in editor** also supports Zed over SSH when the environment advertises an +SSH target and Zed is installed on the client machine. diff --git a/docs/user/terminal.md b/docs/user/terminal.md index 036f123b2..105607424 100644 --- a/docs/user/terminal.md +++ b/docs/user/terminal.md @@ -8,3 +8,7 @@ These limits apply when you reconnect and when T3 Code restores saved terminal history. A client can show less scrollback than the server keeps. On Windows and Linux, **Ctrl+Insert** copies the current terminal selection. + +On Linux and BSD, middle-click pastes the selection from that terminal. With no terminal +selection, it does nothing; it does not paste the system clipboard. Applications that +capture mouse input still receive the click themselves. diff --git a/packages/contracts/src/editor.ts b/packages/contracts/src/editor.ts index b7a7dd7c7..21b2b16fa 100644 --- a/packages/contracts/src/editor.ts +++ b/packages/contracts/src/editor.ts @@ -13,7 +13,8 @@ type EditorDefinition = { /** * URL scheme for editors that support VS Code's remote deep links * (`://vscode-remote/ssh-remote+`). Only set for VS Code - * and forks that ship the Remote-SSH machinery. + * and forks that ship the Remote-SSH machinery, plus Zed, which uses its own + * `zed://ssh/` shape. */ readonly remoteScheme?: string; }; @@ -49,7 +50,13 @@ export const EDITORS = [ launchStyle: "goto", remoteScheme: "vscodium", }, - { id: "zed", label: "Zed", commands: ["zed", "zeditor"], launchStyle: "direct-path" }, + { + id: "zed", + label: "Zed", + commands: ["zed", "zeditor"], + launchStyle: "direct-path", + remoteScheme: "zed", + }, { id: "antigravity", label: "Antigravity", commands: ["agy"], launchStyle: "goto" }, { id: "idea", label: "IntelliJ IDEA", commands: ["idea"], launchStyle: "line-column" }, { id: "aqua", label: "Aqua", commands: ["aqua"], launchStyle: "line-column" }, @@ -84,7 +91,7 @@ export type LaunchEditorInput = typeof LaunchEditorInput.Type; const remoteSchemeOf = (editor: EditorDefinition): string | undefined => editor.remoteScheme; -/** Editors that can open a remote workspace via `vscode-remote` deep links. */ +/** Editors that can open a remote workspace via SSH deep links. */ export const REMOTE_CAPABLE_EDITOR_IDS: ReadonlyArray = EDITORS.flatMap((editor) => remoteSchemeOf(editor) !== undefined ? [editor.id] : [], ); @@ -95,9 +102,10 @@ export const remoteSchemeForEditor = (id: EditorId): string | undefined => { }; /** - * Builds a `://vscode-remote/ssh-remote+` deep link that - * opens `absolutePath` on `host` in the local editor over SSH. Returns - * undefined for editors without remote deep-link support. + * Builds a `://vscode-remote/ssh-remote+` deep link (Zed + * takes `zed://ssh/`) that opens `absolutePath` on `host` in the + * local editor over SSH. Returns undefined for editors without remote + * deep-link support. */ export const buildRemoteOpenUrl = (input: { readonly editor: EditorId; @@ -112,7 +120,10 @@ export const buildRemoteOpenUrl = (input: { const posixPath = input.absolutePath.replaceAll("\\", "/"); const rootedPath = posixPath.startsWith("/") ? posixPath : `/${posixPath}`; const encodedPath = rootedPath.split("/").map(encodeURIComponent).join("/"); - return `${scheme}://vscode-remote/ssh-remote+${encodeURIComponent(input.host)}${encodedPath}`; + const encodedHost = encodeURIComponent(input.host); + return input.editor === "zed" + ? `${scheme}://ssh/${encodedHost}${encodedPath}` + : `${scheme}://vscode-remote/ssh-remote+${encodedHost}${encodedPath}`; }; /** diff --git a/packages/shared/package.json b/packages/shared/package.json index b5ef9935a..4f881507b 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -3,6 +3,10 @@ "private": true, "type": "module", "exports": { + "./threadPullRequests": { + "types": "./src/threadPullRequests.ts", + "import": "./src/threadPullRequests.ts" + }, "./themePalettes": { "types": "./src/themePalettes.ts", "import": "./src/themePalettes.ts" diff --git a/packages/shared/src/threadPullRequests.ts b/packages/shared/src/threadPullRequests.ts new file mode 100644 index 000000000..9f011fa15 --- /dev/null +++ b/packages/shared/src/threadPullRequests.ts @@ -0,0 +1,9 @@ +import type { ThreadLinkedPullRequest } from "@t3tools/contracts"; + +/** Search terms from the V2 single-link projection, without another host request. */ +export function threadPullRequestSearchTerms(thread: { + readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; +}): string[] { + const link = thread.linkedPullRequest; + return link ? [`#${link.number}`, `${link.repository}#${link.number}`, link.url] : []; +} From d3d0cf4aafaf7da56b3a0f2247bd172de4562557 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:26:35 +0200 Subject: [PATCH 2/2] feat(ios): add linked PR collections, stack actions and image zoom --- PATCH.md | 37 +- apps/server/src/auth/RpcAuthorization.ts | 1 + .../src/environment/ServerEnvironment.ts | 2 + .../src/orchestration-v2/Orchestrator.ts | 25 +- .../orchestration-v2/ProjectionStore.test.ts | 26 ++ .../src/orchestration-v2/ProjectionStore.ts | 6 + .../src/pullRequest/GitHubPullRequestCli.ts | 82 ++++ .../pullRequest/GitHubPullRequestProvider.ts | 8 + .../src/pullRequest/PullRequestProvider.ts | 7 + .../pullRequest/PullRequestService.test.ts | 68 +++ .../src/pullRequest/PullRequestService.ts | 48 +- .../src/pullRequest/gitHubPullRequestJson.ts | 74 ++++ .../pullRequest/githubStackActions.test.ts | 413 +++++++++++++++++ .../src/pullRequest/githubStackActions.ts | 418 ++++++++++++++++++ apps/server/src/ws.ts | 4 + apps/swift-ios/App/NativeFeatureClient.swift | 115 +++-- apps/swift-ios/Core/Models.swift | 6 + .../Core/OrchestrationV2Models.swift | 2 + apps/swift-ios/Core/PullRequestModels.swift | 42 +- apps/swift-ios/Core/T3Client.swift | 22 + .../FeatureLinkedPullRequestSettlement.swift | 19 + .../Features/Chat/MarkdownMediaView.swift | 6 +- .../Chat/PullRequestDetailSheet.swift | 66 ++- .../Chat/PullRequestStackActionSheet.swift | 83 ++++ .../Features/Chat/ThreadDetailsSheet.swift | 6 +- .../Chat/ThreadLinkedPullRequestSheet.swift | 65 +-- .../Features/Chat/ZoomableMessageImage.swift | 65 +++ .../Features/Root/FeatureRootModel.swift | 14 +- .../Features/Shared/FeatureClient.swift | 8 + .../Features/Shared/FeatureModels.swift | 13 + .../Features/Workspace/DailyUXModels.swift | 12 +- .../Features/Workspace/WorkspaceView.swift | 2 +- .../Fixtures/orchestrationV2Projection.json | 20 + .../CoreTests/Fixtures/pullRequestStack.json | 27 ++ .../OrchestrationV2ContractTests.swift | 2 + .../FeatureTests/DailyUXSidebarTests.swift | 19 + .../LinkedPullRequestSettlementTests.swift | 36 ++ .../src/components/CommandPalette.logic.ts | 1 + docs/internals/thread-pull-requests.md | 28 ++ docs/user/chat-formatting.md | 7 + docs/user/linked-pull-requests.md | 21 + packages/client-runtime/src/state/models.ts | 4 + .../src/state/threadSettled.test.ts | 15 + .../client-runtime/src/state/threadSettled.ts | 6 + packages/contracts/src/environment.ts | 3 + packages/contracts/src/orchestrationV2.ts | 9 + packages/contracts/src/pullRequest.ts | 26 ++ packages/contracts/src/rpc.ts | 9 + .../shared/src/threadPullRequests.test.ts | 61 +++ packages/shared/src/threadPullRequests.ts | 67 ++- scripts/generate-swift-contract-fixtures.ts | 45 ++ 51 files changed, 2053 insertions(+), 118 deletions(-) create mode 100644 apps/server/src/pullRequest/githubStackActions.test.ts create mode 100644 apps/server/src/pullRequest/githubStackActions.ts create mode 100644 apps/swift-ios/Features/Chat/FeatureLinkedPullRequestSettlement.swift create mode 100644 apps/swift-ios/Features/Chat/PullRequestStackActionSheet.swift create mode 100644 apps/swift-ios/Features/Chat/ZoomableMessageImage.swift create mode 100644 apps/swift-ios/Tests/CoreTests/Fixtures/pullRequestStack.json create mode 100644 apps/swift-ios/Tests/FeatureTests/LinkedPullRequestSettlementTests.swift create mode 100644 docs/internals/thread-pull-requests.md create mode 100644 docs/user/linked-pull-requests.md create mode 100644 packages/shared/src/threadPullRequests.test.ts diff --git a/PATCH.md b/PATCH.md index 6f7522099..d9f89bbd1 100644 --- a/PATCH.md +++ b/PATCH.md @@ -379,22 +379,27 @@ This fork stays close to `pingdotgg/t3code` and carries only the following opera `buildToolCallExpandedBody` / projected-item disclosure; the fork has no `commandMatchesVisibleLabel` expansion guard. Android feed positioning (`75e4ceb964`) and glass backing (`383cc40f4d`) remain excluded under the Expo freeze. - - Multiple linked PRs (`afb84898be`) remain deferred for a coordinated V2/Swift port: - upstream introduces host-level link identity, stack-dismissal tombstones, cached snapshots, - multi-PR settlement, automatic linking after creation, and credential-scoped MCP tools. - These must land together on V2's JSON projection and existing MCP capability model. - `050_ProjectionThreadPullRequests` targets V1 tables and collides with a fork-owned number; - it is dropped. The `threadPullRequests` capability, V1 link/unlink commands, RPCs, - provider instructions, client-runtime commands and dependent UI are not advertised/carried. - - GitHub stack navigation/merge/rebase (`de37964db2`) remains deferred with the multi-PR - stack service. A dedicated port must retain reviewed-head checks, branch permissions, - partial-rebase reporting and remote-only operations, then adapt the fork's panel stores - and native client. The `pullRequestStackActions` capability is not carried. - - Restart-persistent PR reads (`33242d0164`) cache the upstream `summary` / `stack` service - methods that the fork does not have (its earlier PR-discovery port is also deferred). - Do not add an unused cache layer or replace the fork's detail-cache semantics by inference. - Carry this with the missing service, including expiry and mutation/in-flight invalidation. - Advancing this sync marker records review of these deferred commits, not feature support. + - The approved native parity follow-up now supports multiple explicit PR links on V2: + `thread.metadata.update` adds/removes one link atomically, with a 50-link limit and + host/repository/number identity. The JSON projection carries `linkedPullRequests` while + `linkedPullRequest` remains the primary for older clients. Legacy edits preserve other links. + Swift gates collection editing on `threadPullRequestsV2`, searches every link, and requires + every linked PR to read as terminal before settling. Link changes restart its observations. + Web/Expo still render the primary and conservatively avoid automatic settlement for collections. + Automatic discovery/linking after creation, stack-dismissal tombstones, cached snapshots and + credential-scoped MCP link tools remain unported. Upstream's `threadPullRequests` flag and V1 + commands stay excluded; `050_ProjectionThreadPullRequests` is dropped, with no new migration. + - GitHub stack navigation/merge/rebase (`de37964db2`) is now available to Swift through + `pullRequests.stack` and `pullRequestStackActions`. The standalone GitHub action implementation + retains reviewed-head checks, per-branch permissions, partial-rebase reporting and remote-only + operations. Confirmation holds the reviewed stack immutable; mutations invalidate every + reviewed PR's cached reads even after partial failure. Web/Expo stack controls remain unported. + - Restart-persistent PR summary/stack reads (`33242d0164`) remain excluded. Stack reads are + on demand; the earlier V2 background PR-discovery/summary service is still missing. Carry a + durable read cache with that service, including expiry and mutation/in-flight invalidation. + Advancing this sync marker records review of deferred work, not full upstream feature support. + - Swift's existing image galleries now support pinch/pan, double-tap zoom and an accessible + fit action while retaining original-byte export and current/adjacent-page loading. - The 2026-09-09 sync (`223ff4490f..e16b8b059c`, 185 upstream commits) manually carries independent correctness fixes while retaining the boundaries above: - `thread.stop` (`09e8de9c65`) uses web/desktop's existing V2 `interruptThreadTurn` path. diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 429d6be6e..a50cb706e 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -77,6 +77,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.cloudInstallRelayClient]: AuthRelayWriteScope, [WS_METHODS.pullRequestsList]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsListStats]: AuthOrchestrationReadScope, + [WS_METHODS.pullRequestsStack]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsDetail]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsActivity]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsThreadComments]: AuthOrchestrationReadScope, diff --git a/apps/server/src/environment/ServerEnvironment.ts b/apps/server/src/environment/ServerEnvironment.ts index 432d8df4e..9671adcd1 100644 --- a/apps/server/src/environment/ServerEnvironment.ts +++ b/apps/server/src/environment/ServerEnvironment.ts @@ -163,6 +163,8 @@ export const make = Effect.gen(function* () { threadPinReorder: true, threadTitleRegeneration: true, threadPullRequestLinking: true, + threadPullRequestsV2: true, + pullRequestStackActions: true, ...(serverSelfUpdate === null ? {} : { serverSelfUpdate }), ...(serverSelfUpdate === "boot-service" ? { serverSelfUpdateProgress: true } : {}), }, diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 7e5264645..2915a1e58 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -1,3 +1,4 @@ +import { updateLinkedPullRequests } from "@t3tools/shared/threadPullRequests"; import { type ChatAttachment, CommandId, @@ -1449,6 +1450,24 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ), ); const thread = projection.thread; + if (command.type === "thread.metadata.update") { + const edits = [ + command.linkedPullRequest, + command.linkPullRequest, + command.unlinkPullRequest, + ].filter((value) => value !== undefined); + if ( + edits.length > 1 || + updateLinkedPullRequests(thread, command).linkedPullRequests.length > 50 + ) { + return yield* new OrchestratorDispatchError({ + commandId: command.commandId, + commandType: command.type, + cause: + "Send one pull-request edit at a time; a thread can link at most 50 pull requests.", + }); + } + } if (thread.deletedAt !== null && command.type !== "thread.delete") { return yield* new OrchestratorDispatchError({ commandId: command.commandId, @@ -1730,9 +1749,11 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ? {} : { activeOrderKey: command.activeOrderKey }), // Absent leaves the link alone; null unlinks. - ...(command.linkedPullRequest === undefined + ...(command.linkedPullRequest === undefined && + command.linkPullRequest === undefined && + command.unlinkPullRequest === undefined ? {} - : { linkedPullRequest: command.linkedPullRequest }), + : updateLinkedPullRequests(thread, command)), ...(command.workInboxRole === undefined ? {} : { diff --git a/apps/server/src/orchestration-v2/ProjectionStore.test.ts b/apps/server/src/orchestration-v2/ProjectionStore.test.ts index 806b77dc9..430be7ccc 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.test.ts @@ -149,6 +149,32 @@ it.layer(TestLayer)("ProjectionStoreV2", (it) => { payload: thread, }); + const links = [41, 42].map((number) => ({ + projectId, + repository: "owner/repo", + number, + url: `https://github.com/owner/repo/pull/${number}`, + })); + yield* projectionStore.apply({ + id: EventId.make("event:projection-read-state:links"), + type: "thread.metadata-updated", + threadId, + occurredAt: markedUnreadOccurredAt, + payload: { ...thread, linkedPullRequest: links[0]!, linkedPullRequests: links }, + }); + assert.deepEqual( + (yield* projectionStore.getThreadProjection(threadId)).thread.linkedPullRequests, + links, + ); + assert.deepEqual( + (yield* projectionStore.getThreadShell(threadId))?.linkedPullRequests, + links, + ); + assert.deepEqual( + (yield* projectionStore.getShellSnapshot()).threads.find((shell) => shell.id === threadId) + ?.linkedPullRequests, + links, + ); const markedUnread = yield* projectionStore.getThreadProjection(threadId); assert.isNull(markedUnread.thread.lastVisitedAt); assert.deepEqual(markedUnread.thread.updatedAt, createdAt); diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 2d6e3bc25..0fca043c9 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -975,6 +975,9 @@ export function threadShellFromProjection( ? {} : { worktreeStatus: projection.thread.worktreeStatus }), linkedPullRequest: projection.thread.linkedPullRequest ?? null, + ...(projection.thread.linkedPullRequests === undefined + ? {} + : { linkedPullRequests: projection.thread.linkedPullRequests }), lineage: projection.thread.lineage, forkedFrom: projection.thread.forkedFrom, activeProviderThreadId: projection.thread.activeProviderThreadId, @@ -1162,6 +1165,9 @@ function shellFromState(input: { ? {} : { worktreeStatus: input.state.thread.worktreeStatus }), linkedPullRequest: input.state.thread.linkedPullRequest ?? null, + ...(input.state.thread.linkedPullRequests === undefined + ? {} + : { linkedPullRequests: input.state.thread.linkedPullRequests }), lineage: input.state.thread.lineage, forkedFrom: input.state.thread.forkedFrom, activeProviderThreadId: input.state.thread.activeProviderThreadId, diff --git a/apps/server/src/pullRequest/GitHubPullRequestCli.ts b/apps/server/src/pullRequest/GitHubPullRequestCli.ts index 5f8ee8c4e..e28242179 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestCli.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestCli.ts @@ -1,3 +1,9 @@ +import { runGitHubStackAction, type GitHubStackActionError } from "./githubStackActions.ts"; +import { + decodePullRequestStacksJson, + type GitHubPullRequestStack, +} from "./gitHubPullRequestJson.ts"; +import type { PullRequestStackHead } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; @@ -241,6 +247,7 @@ export class GitHubSubjectScopeError extends Schema.TaggedErrorClass Effect.Effect; + readonly getPullRequestStack: (input: { + readonly cwd: string; + readonly repository: string; + readonly host: string; + readonly number: number; + readonly includeDetails?: boolean; + }) => Effect.Effect; readonly runPullRequestAction: (input: { readonly cwd: string; readonly repository: string; readonly host: string; readonly number: number; readonly action: PullRequestAction; + readonly stackNumber?: number; + readonly expectedStackHeads?: ReadonlyArray; readonly mergeMethod?: PullRequestMergeMethod; readonly updateMethod?: PullRequestUpdateMethod; }) => Effect.Effect; @@ -1696,7 +1712,73 @@ export const make = Effect.gen(function* () { .pipe(Effect.asVoid); }, + getPullRequestStack: (input) => { + const { owner, name } = parseRepositorySelector(input.repository); + return github + .execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + `repos/${owner}/${name}/stacks?pull_request=${input.number}`, + ], + }) + .pipe( + Effect.flatMap((result) => { + const decoded = decodePullRequestStacksJson(result.stdout.trim()); + return Result.isSuccess(decoded) + ? Effect.succeed(decoded.success) + : Effect.fail( + new GitHubPullRequestReadError({ + command: "gh", + cwd: input.cwd, + operation: "getPullRequestStack", + cause: decoded.failure, + }), + ); + }), + Effect.flatMap((stack) => { + if (!input.includeDetails || stack === null) return Effect.succeed(stack); + return github + .execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + `repos/${owner}/${name}/stacks/${stack.number}`, + ], + }) + .pipe( + Effect.flatMap((result) => { + const decoded = decodePullRequestStacksJson(`[${result.stdout.trim()}]`); + return Result.isSuccess(decoded) + ? Effect.succeed(decoded.success) + : Effect.fail( + new GitHubPullRequestReadError({ + command: "gh", + cwd: input.cwd, + operation: "getPullRequestStack", + cause: decoded.failure, + }), + ); + }), + ); + }), + // Hosts without the stacks preview return 404. Other failures must preserve the + // previously synced stack and let the caller retry. + Effect.catchTags({ + GitHubPullRequestNotFoundError: () => Effect.succeed(null), + }), + ); + }, + runPullRequestAction: (input) => { + if (input.stackNumber !== undefined) + return runGitHubStackAction({ ...input, stackNumber: input.stackNumber }).pipe( + Effect.provideService(GitHubCli.GitHubCli, github), + ); const [subcommand, ...flags] = actionArgs( input.action, input.mergeMethod, diff --git a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts index cc097c30c..085d60fbf 100644 --- a/apps/server/src/pullRequest/GitHubPullRequestProvider.ts +++ b/apps/server/src/pullRequest/GitHubPullRequestProvider.ts @@ -414,6 +414,10 @@ export const make = Effect.gen(function* () { }) .pipe(Effect.mapError(fail("setReviewerRequest"))), + getStack: (input) => + cli + .getPullRequestStack({ ...input, includeDetails: true }) + .pipe(Effect.mapError(fail("getStack"))), runAction: (input) => cli .runPullRequestAction({ @@ -422,6 +426,10 @@ export const make = Effect.gen(function* () { host: input.host, number: input.number, action: input.action, + ...(input.stackNumber === undefined ? {} : { stackNumber: input.stackNumber }), + ...(input.expectedStackHeads === undefined + ? {} + : { expectedStackHeads: input.expectedStackHeads }), ...(input.mergeMethod === undefined ? {} : { mergeMethod: input.mergeMethod }), ...(input.updateMethod === undefined ? {} : { updateMethod: input.updateMethod }), }) diff --git a/apps/server/src/pullRequest/PullRequestProvider.ts b/apps/server/src/pullRequest/PullRequestProvider.ts index eadc5e931..ac25c8e77 100644 --- a/apps/server/src/pullRequest/PullRequestProvider.ts +++ b/apps/server/src/pullRequest/PullRequestProvider.ts @@ -1,3 +1,4 @@ +import type { PullRequestStack, PullRequestStackHead } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import type { @@ -356,10 +357,16 @@ export interface PullRequestProviderApi { }, ) => Effect.Effect; + readonly getStack?: ( + input: ProviderRepositoryRef & { readonly number: number }, + ) => Effect.Effect; + readonly runAction: ( input: ProviderRepositoryRef & { readonly number: number; readonly action: PullRequestAction; + readonly stackNumber?: number; + readonly expectedStackHeads?: ReadonlyArray; /** Meaningful for `merge` and `enable-auto-merge`; absent takes the host's own default. */ readonly mergeMethod?: PullRequestMergeMethod; /** Only meaningful for `update-branch`; absent takes the host's own default. */ diff --git a/apps/server/src/pullRequest/PullRequestService.test.ts b/apps/server/src/pullRequest/PullRequestService.test.ts index 8f96bbca4..e4610409f 100644 --- a/apps/server/src/pullRequest/PullRequestService.test.ts +++ b/apps/server/src/pullRequest/PullRequestService.test.ts @@ -3420,3 +3420,71 @@ it.effect("names the signed-in account in the detail, and says nothing where the assert.strictEqual(unnamed.viewer, undefined); }), ); + +it.effect("routes stack reads and preserves reviewed heads through action authorization", () => + Effect.gen(function* () { + const reference = { projectId: "p1" as ProjectId, repository: "acme/web", number: 2 }; + const heads = [ + { number: 1, headSha: "abc" }, + { number: 2, headSha: "def" }, + ]; + const stack = { + id: "stack-1", + number: 1, + url: "https://github.com/acme/web/stack/1", + base: "main", + layers: [], + }; + let calls = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + getStack: (input) => { + assert.equal(input.number, 2); + return Effect.succeed(stack); + }, + runAction: (input) => { + calls++; + assert.equal(input.stackNumber, 1); + assert.deepEqual(input.expectedStackHeads, heads); + return Effect.void; + }, + }), + ], + }); + assert.deepEqual(yield* service.stack(reference), stack); + yield* service.runAction({ + ...reference, + stackNumber: 1, + expectedStackHeads: heads, + action: "merge", + mergeMethod: "merge", + }); + assert.equal(calls, 1); + }), +); + +it.effect("returns no stack and refuses stack mutations on unsupported hosts", () => + Effect.gen(function* () { + const reference = { projectId: "p1" as ProjectId, repository: "acme/web", number: 2 }; + let calls = 0; + const service = yield* makeService({ + projects: [project({ id: "p1", title: "web", workspaceRoot: "/a", repository: "acme/web" })], + providers: [ + fakeProvider("github", { + runAction: () => { + calls++; + return Effect.void; + }, + }), + ], + }); + assert.isNull(yield* service.stack(reference)); + const error = yield* Effect.flip( + service.runAction({ ...reference, stackNumber: 1, action: "merge" }), + ); + assert.equal(error._tag, "PullRequestOperationError"); + assert.equal(calls, 0); + }), +); diff --git a/apps/server/src/pullRequest/PullRequestService.ts b/apps/server/src/pullRequest/PullRequestService.ts index dbb4d7a66..7d278f582 100644 --- a/apps/server/src/pullRequest/PullRequestService.ts +++ b/apps/server/src/pullRequest/PullRequestService.ts @@ -1,3 +1,4 @@ +import type { PullRequestStack } from "@t3tools/contracts"; import * as Cache from "effect/Cache"; import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; @@ -135,6 +136,9 @@ export class PullRequestService extends Context.Service< readonly diffFileContents: ( input: PullRequestDiffFileContentsInput, ) => Effect.Effect; + readonly stack: ( + input: PullRequestRef, + ) => Effect.Effect; readonly runAction: (input: PullRequestActionInput) => Effect.Effect; readonly update: (input: PullRequestUpdateInput) => Effect.Effect; readonly comment: (input: PullRequestCommentInput) => Effect.Effect; @@ -441,6 +445,7 @@ function withRateLimitBackoff( ...(api.getDiffFileContents === undefined ? {} : { getDiffFileContents: wrap("getDiffFileContents", api.getDiffFileContents) }), + ...(api.getStack === undefined ? {} : { getStack: wrap("getStack", api.getStack) }), runAction: interactive("runAction", api.runAction), ...(api.updateChangeRequest === undefined ? {} @@ -1296,9 +1301,33 @@ export const make = Effect.gen(function* () { }), ); + const stack: PullRequestService["Service"]["stack"] = (input) => + requireProject(input).pipe( + Effect.flatMap((project) => + project.api.getStack + ? project.api + .getStack({ + cwd: project.project.workspaceRoot, + repository: project.repository, + host: project.host, + number: input.number, + }) + .pipe(Effect.mapError(toPullRequestError("stack"))) + : Effect.succeed(null), + ), + ); + const runAction: PullRequestService["Service"]["runAction"] = (input) => requireProject(input).pipe( Effect.flatMap((project): Effect.Effect => { + if (input.stackNumber !== undefined && !project.api.getStack) { + return Effect.fail( + new PullRequestOperationError({ + operation: "runAction", + detail: "This host does not support stack actions.", + }), + ); + } // The surface hides what a host cannot do, and this refuses it as well: a request that // reached here anyway must not be handed to a provider that never claimed the action. if (!project.api.capabilities.actions.includes(input.action)) { @@ -1367,6 +1396,10 @@ export const make = Effect.gen(function* () { host: project.host, number: input.number, action: input.action, + ...(input.stackNumber === undefined ? {} : { stackNumber: input.stackNumber }), + ...(input.expectedStackHeads === undefined + ? {} + : { expectedStackHeads: input.expectedStackHeads }), ...(input.mergeMethod === undefined ? {} : { mergeMethod: input.mergeMethod }), ...(input.updateMethod === undefined ? {} : { updateMethod: input.updateMethod }), }) @@ -2101,7 +2134,20 @@ export const make = Effect.gen(function* () { threadComments, diff, diffFileContents, - runAction: invalidatedByMutation(runAction), + stack, + runAction: (input) => + invalidatedByMutation(runAction)(input).pipe( + Effect.ensuring( + input.stackNumber === undefined + ? Effect.void + : Effect.sync(() => { + // A rebase can fail after updating earlier layers. Invalidate every reviewed layer. + for (const head of input.expectedStackHeads ?? []) + bumpRefEpoch({ ...input, number: head.number }); + listingsEpoch = ++epochCounter; + }), + ), + ), update: invalidatedByMutation(update), comment: invalidatedByMutation(comment), updateComment: invalidatedByMutation(updateComment), diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.ts index 6ec17ea11..e367b0a8c 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.ts @@ -2239,3 +2239,77 @@ export function decodePullRequestFilesJson( omittedFileStats, }); } + +/** One pull request as the stacks API lists it: a number, a head, and whether it is done. */ +const RawStackPullRequestSchema = Schema.Struct({ + title: Schema.optional(Schema.String), + draft: Schema.optional(Schema.Boolean), + number: Schema.Int, + head: Schema.Struct({ ref: Schema.String, sha: Schema.optional(Schema.String) }), + state: Schema.optional(Schema.NullOr(Schema.String)), + merged_at: Schema.optional(Schema.NullOr(Schema.String)), +}); + +/** + * A stack as `GET /repos/{owner}/{repo}/stacks` answers it, in a public preview whose shape may + * still move. Only what a stack is made of is required — where it lives, what it stands on, and + * its pull requests — and `base` is accepted both as the ref object the preview sends today and + * as the bare branch name it started out as. + */ +const RawStackSchema = Schema.Struct({ + id: Schema.optional(Schema.NullOr(Schema.Union([Schema.Int, Schema.String]))), + number: Schema.Int, + node_id: Schema.optional(Schema.NullOr(Schema.String)), + url: Schema.String, + html_url: Schema.optional(Schema.NullOr(Schema.String)), + base: Schema.Union([Schema.String, Schema.Struct({ ref: Schema.String })]), + pull_requests: Schema.Array(RawStackPullRequestSchema), +}); + +const decodeStacks = decodeJsonResult(Schema.Array(RawStackSchema)); + +export interface GitHubPullRequestStackLayer { + readonly title?: string; + readonly isDraft?: boolean; + readonly headSha?: string; + readonly number: number; + readonly headBranch: string; + readonly state: PullRequestState; +} + +export interface GitHubPullRequestStack { + readonly id: string; + readonly number: number; + readonly url: string; + readonly base: string; + /** Bottom to top, which is the order GitHub lists them in. */ + readonly layers: ReadonlyArray; +} + +/** + * The first stack of a `?pull_request=` listing, or null for an empty one: a pull request is in + * at most one stack, so the array is GitHub's way of saying "none" rather than a page. + */ +export function decodePullRequestStacksJson( + raw: string, +): Result.Result { + const decoded = decodeStacks(raw); + if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure); + const stack = decoded.success[0]; + if (stack === undefined) return Result.succeed(null); + return Result.succeed({ + id: stack.id == null ? (trimmed(stack.node_id) ?? String(stack.number)) : String(stack.id), + number: stack.number, + // The page a person opens where the preview reports one; the API URL is what it always has. + url: trimmed(stack.html_url) ?? stack.url, + base: typeof stack.base === "string" ? stack.base : stack.base.ref, + layers: stack.pull_requests.map((pullRequest) => ({ + ...(pullRequest.title === undefined ? {} : { title: pullRequest.title }), + ...(pullRequest.draft === undefined ? {} : { isDraft: pullRequest.draft }), + ...(pullRequest.head.sha === undefined ? {} : { headSha: pullRequest.head.sha }), + number: pullRequest.number, + headBranch: pullRequest.head.ref, + state: toState({ state: pullRequest.state, mergedAt: pullRequest.merged_at }), + })), + }); +} diff --git a/apps/server/src/pullRequest/githubStackActions.test.ts b/apps/server/src/pullRequest/githubStackActions.test.ts new file mode 100644 index 000000000..4ed254514 --- /dev/null +++ b/apps/server/src/pullRequest/githubStackActions.test.ts @@ -0,0 +1,413 @@ +import { expect, it } from "@effect/vitest"; +import * as Layer from "effect/Layer"; +import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; +import * as TestClock from "effect/testing/TestClock"; +import { ChildProcessSpawner } from "effect/unstable/process"; +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import { runGitHubStackAction as runStackAction } from "./githubStackActions.ts"; + +const runGitHubStackAction = ( + execute: GitHubCli.GitHubCli["Service"]["execute"], + input: Parameters[0], +) => runStackAction(input).pipe(Effect.provide(Layer.mock(GitHubCli.GitHubCli)({ execute }))); + +const stack = [ + { + number: 50, + url: "https://api.github.com/repos/acme/web/stacks/50", + base: { ref: "main" }, + pull_requests: [ + { + number: 1, + title: "Base", + head: { ref: "base", sha: "aaa" }, + state: "closed", + merged_at: "2026-01-01T00:00:00Z", + }, + { + number: 2, + title: "Middle", + head: { ref: "middle", sha: "bbb" }, + state: "open", + draft: false, + }, + { number: 3, title: "Top", head: { ref: "top", sha: "ccc" }, state: "open", draft: false }, + ], + }, +]; +const input = { + cwd: "/repo", + repository: "acme/web", + host: "github.com", + number: 3, + stackNumber: 50, + expectedStackHeads: [ + { number: 2, headSha: "bbb" }, + { number: 3, headSha: "ccc" }, + ], + action: "merge" as const, +}; +const access = { + data: { + repository: { + pr2: { headRepository: { viewerPermission: "WRITE" }, maintainerCanModify: false }, + pr3: { headRepository: { viewerPermission: "WRITE" }, maintainerCanModify: false }, + }, + }, +}; + +const branch = (number: number, headRefOid: string, behindBy = 1, processed: string[] = []) => ({ + data: { + processed: processed.map((headRefOid) => ({ headRefOid })), + repository: { + pullRequest: { id: `PR_${number}`, headRefOid, baseRef: { compare: { behindBy } } }, + }, + }, +}); +const rebased = { + data: { updatePullRequestBranch: { pullRequest: { headRefOid: "rebased-sha" } } }, +}; +const rebaseResponses = [branch(2, "bbb"), rebased, branch(3, "ccc", 1, ["rebased-sha"]), rebased]; + +function fake(responses: readonly unknown[]) { + const calls: ReadonlyArray[] = []; + const execute: GitHubCli.GitHubCli["Service"]["execute"] = (request) => + Effect.sync(() => { + calls.push(request.args); + const value = responses[calls.length - 1]; + if (value === undefined) throw new Error("Unexpected GitHub request"); + return { + exitCode: ChildProcessSpawner.ExitCode(0), + // @effect-diagnostics-next-line preferSchemaOverJson:off + stdout: JSON.stringify(value), + stderr: "", + stdoutTruncated: false, + stderrTruncated: false, + stdoutInvalidUtf8: false, + }; + }); + return { execute, calls }; +} + +it.effect("submits one atomic merge with the reviewed head and respects the merge queue", () => + Effect.gen(function* () { + const api = fake([stack, { status: "enqueued", details: {} }]); + yield* runGitHubStackAction(api.execute, { ...input, mergeMethod: "squash" }); + expect(api.calls).toHaveLength(2); + expect(api.calls[1]).toContain("repos/acme/web/pulls/3/merge-async"); + expect(api.calls[1]).toContain("sha=ccc"); + expect(api.calls[1]).toContain("merge_action=default"); + expect(api.calls[1]).toContain("merge_method=squash"); + }), +); + +it.effect("merges through the selected layer without including later draft layers", () => + Effect.gen(function* () { + const fiveLayers = [ + { + ...stack[0], + pull_requests: Array.from({ length: 5 }, (_, index) => ({ + number: index + 1, + head: { ref: `layer-${index + 1}`, sha: `sha-${index + 1}` }, + state: "open", + draft: index >= 3, + })), + }, + ]; + const api = fake([fiveLayers, { status: "merged", details: {} }]); + yield* runGitHubStackAction(api.execute, { + ...input, + number: 3, + expectedStackHeads: [1, 2, 3].map((number) => ({ number, headSha: `sha-${number}` })), + }); + expect(api.calls).toHaveLength(2); + expect(api.calls[1]).toContain("repos/acme/web/pulls/3/merge-async"); + expect(api.calls[1]).toContain("sha=sha-3"); + expect(api.calls[1]).toContain("merge_action=default"); + }), +); + +it.effect("rejects stale reviewed heads below a selected middle layer", () => + Effect.gen(function* () { + const api = fake([stack]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + number: 2, + expectedStackHeads: [{ number: 2, headSha: "old-head" }], + }).pipe(Effect.result); + expect(result).toMatchObject({ _tag: "Failure", failure: { _tag: "GitHubStackChangedError" } }); + expect(api.calls).toHaveLength(1); + }), +); + +it.effect("does not merge from an already merged layer", () => + Effect.gen(function* () { + const api = fake([stack]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + number: 1, + expectedStackHeads: [], + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackUnsupportedError" }, + }); + expect(api.calls).toHaveLength(1); + }), +); + +it.effect("polls an accepted merge and reports a later rule rejection", () => + Effect.gen(function* () { + const api = fake([ + stack, + { status: "pending", details: { uuid: "operation" } }, + { status: "failed", details: { message: "Required checks have not passed" } }, + ]); + const fiber = yield* runGitHubStackAction(api.execute, input).pipe( + Effect.result, + Effect.forkChild, + ); + yield* TestClock.adjust("1 second"); + const result = yield* Fiber.join(fiber); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackMergeRejectedError" }, + }); + expect(api.calls[2]).toContain("repos/acme/web/pulls/3/merge-async/operation"); + }), +); + +it.effect("retains stack identity and a rejection response without a message", () => + Effect.gen(function* () { + const rejection = { status: "failed", details: {} }; + const api = fake([stack, rejection]); + const result = yield* runGitHubStackAction(api.execute, input).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { + _tag: "GitHubStackMergeRejectedError", + repository: input.repository, + number: input.number, + stackNumber: input.stackNumber, + cause: rejection, + }, + }); + }), +); + +it.effect("refuses a changed stack before performing any mutation", () => + Effect.gen(function* () { + const api = fake([stack]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + expectedStackHeads: [ + { number: 2, headSha: "old" }, + { number: 3, headSha: "ccc" }, + ], + }).pipe(Effect.result); + expect(result).toMatchObject({ _tag: "Failure", failure: { _tag: "GitHubStackChangedError" } }); + expect(api.calls).toHaveLength(1); + }), +); + +it.effect("rebases unmerged layers bottom to top without local git commands", () => + Effect.gen(function* () { + const api = fake([stack, access, ...rebaseResponses]); + yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); + const mutations = api.calls.filter((args) => + args.some((arg) => arg.startsWith("query=mutation")), + ); + expect(mutations).toHaveLength(2); + expect(mutations[0]).toContain("id=PR_2"); + expect(mutations[0]).toContain("sha=bbb"); + expect(mutations[1]).toContain("id=PR_3"); + expect(mutations[1]).toContain("sha=ccc"); + expect(api.calls.every((args) => args[0] === "api")).toBe(true); + }), +); + +it.effect("does not update later layers after a rebase failure", () => + Effect.gen(function* () { + const api = fake([stack, access, branch(2, "bbb")]); + const execute: typeof api.execute = (request) => + !request.args.some((arg) => arg.startsWith("query=mutation")) + ? api.execute(request) + : Effect.fail( + new GitHubCli.GitHubCliAuthenticationError({ + command: "gh", + cwd: "/repo", + cause: new Error("denied"), + }), + ); + const result = yield* runGitHubStackAction(execute, { ...input, action: "update-branch" }).pipe( + Effect.result, + ); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackRebaseFailedError", number: 2, completed: 0 }, + }); + }), +); + +it.effect("refuses the entire rebase before mutation when a later fork denies write access", () => + Effect.gen(function* () { + const api = fake([ + stack, + { + data: { + repository: { + ...access.data.repository, + pr3: { headRepository: { viewerPermission: "READ" }, maintainerCanModify: false }, + }, + }, + }, + ]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + action: "update-branch", + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackPermissionError" }, + }); + expect(api.calls).toHaveLength(2); + expect(api.calls.every((args) => args[0] === "api")).toBe(true); + }), +); + +it.effect("allows a fork that explicitly permits maintainer updates", () => + Effect.gen(function* () { + const api = fake([ + stack, + { + data: { + repository: { + ...access.data.repository, + pr3: { headRepository: { viewerPermission: "READ" }, maintainerCanModify: true }, + }, + }, + }, + ...rebaseResponses, + ]); + yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); + expect(api.calls.at(-1)).toContain("id=PR_3"); + }), +); + +it.effect("bounds polling and reports a still-running merge without claiming success", () => + Effect.gen(function* () { + const api = fake([ + stack, + ...Array.from({ length: 40 }, () => ({ status: "pending", details: { uuid: "operation" } })), + ]); + const fiber = yield* runGitHubStackAction(api.execute, input).pipe( + Effect.result, + Effect.forkChild, + ); + yield* TestClock.adjust("6 minutes"); + expect(yield* Fiber.join(fiber)).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackMergePendingError" }, + }); + expect(api.calls.length).toBeLessThan(40); + }), +); + +it.effect("rejects a push after preflight without rebasing the new revision", () => + Effect.gen(function* () { + const api = fake([stack, access, branch(2, "new-head")]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + action: "update-branch", + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackChangedError", number: 2, completed: 0 }, + }); + expect(api.calls).toHaveLength(3); + }), +); + +it.effect("skips current layers without submitting a rebase mutation", () => + Effect.gen(function* () { + const api = fake([stack, access, branch(2, "bbb", 0), branch(3, "ccc", 0, ["bbb"])]); + yield* runGitHubStackAction(api.execute, { ...input, action: "update-branch" }); + expect(api.calls.some((args) => args.some((arg) => arg.startsWith("query=mutation")))).toBe( + false, + ); + }), +); + +it.effect("keeps earlier progress and stops after a later layer fails", () => + Effect.gen(function* () { + const api = fake([ + stack, + access, + branch(2, "bbb"), + rebased, + branch(3, "ccc", 1, ["rebased-sha"]), + { data: { updatePullRequestBranch: null } }, + ]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + action: "update-branch", + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackRebaseFailedError", number: 3, completed: 1 }, + }); + }), +); + +it.effect("reports partial progress when a later head changes during the rebase", () => + Effect.gen(function* () { + const api = fake([ + stack, + access, + branch(2, "bbb"), + rebased, + branch(3, "concurrent-head", 1, ["rebased-sha"]), + ]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + action: "update-branch", + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackChangedError", number: 3, completed: 1 }, + }); + if (result._tag === "Failure") { + expect(result.failure.message).toContain("Earlier updates remain on GitHub"); + } + expect( + api.calls.filter((args) => args.some((arg) => arg.startsWith("query=mutation"))), + ).toHaveLength(1); + }), +); + +it.effect.each([false, true])("rejects a push to a processed layer, rebased=%s", (rebasedParent) => + Effect.gen(function* () { + const api = fake([ + stack, + access, + branch(2, "bbb", rebasedParent ? 1 : 0), + ...(rebasedParent ? [rebased] : []), + branch(3, "ccc", 1, ["concurrent-parent-head"]), + ]); + const result = yield* runGitHubStackAction(api.execute, { + ...input, + action: "update-branch", + }).pipe(Effect.result); + expect(result).toMatchObject({ + _tag: "Failure", + failure: { _tag: "GitHubStackChangedError", number: 2, completed: 1 }, + }); + expect(api.calls.at(-1)?.some((arg) => arg.includes('processed:nodes(ids:["PR_2"])'))).toBe( + true, + ); + expect( + api.calls.filter((args) => args.some((arg) => arg.startsWith("query=mutation"))), + ).toHaveLength(rebasedParent ? 1 : 0); + }), +); diff --git a/apps/server/src/pullRequest/githubStackActions.ts b/apps/server/src/pullRequest/githubStackActions.ts new file mode 100644 index 000000000..d13de0187 --- /dev/null +++ b/apps/server/src/pullRequest/githubStackActions.ts @@ -0,0 +1,418 @@ +import type { + PullRequestAction, + PullRequestMergeMethod, + PullRequestStackHead, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Clock from "effect/Clock"; +import * as Result from "effect/Result"; +import * as Schema from "effect/Schema"; + +import * as GitHubCli from "../sourceControl/GitHubCli.ts"; +import { decodePullRequestStacksJson } from "./gitHubPullRequestJson.ts"; + +const stackErrorIdentity = { + repository: Schema.String, + number: Schema.Int, + stackNumber: Schema.Int, +}; + +export class GitHubStackChangedError extends Schema.TaggedErrorClass()( + "GitHubStackChangedError", + { ...stackErrorIdentity, completed: Schema.Int }, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return this.completed > 0 + ? `The stack changed at PR #${this.number} after ${this.completed} layers. Earlier updates remain on GitHub. Refresh it before trying again.` + : "The stack changed. Refresh it before trying again."; + } +} + +export class GitHubStackUnsupportedError extends Schema.TaggedErrorClass()( + "GitHubStackUnsupportedError", + stackErrorIdentity, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return "This operation is not supported for this stack."; + } +} + +export class GitHubStackResponseInvalidError extends Schema.TaggedErrorClass()( + "GitHubStackResponseInvalidError", + { ...stackErrorIdentity, cause: Schema.optional(Schema.Defect()) }, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return "GitHub returned an unreadable stack operation response."; + } +} + +export class GitHubStackMergeRejectedError extends Schema.TaggedErrorClass()( + "GitHubStackMergeRejectedError", + { ...stackErrorIdentity, cause: Schema.Defect() }, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return "GitHub refused the stack merge. Check the stack's branch rules and merge requirements."; + } +} + +export class GitHubStackMergePendingError extends Schema.TaggedErrorClass()( + "GitHubStackMergePendingError", + stackErrorIdentity, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return "The merge is still running on GitHub. Check its status there before submitting another request."; + } +} + +export class GitHubStackPermissionError extends Schema.TaggedErrorClass()( + "GitHubStackPermissionError", + stackErrorIdentity, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return "You cannot update every branch in this stack. Check write access and fork maintainer permissions before retrying."; + } +} + +export class GitHubStackRebaseFailedError extends Schema.TaggedErrorClass()( + "GitHubStackRebaseFailedError", + { ...stackErrorIdentity, completed: Schema.Int, cause: Schema.Defect() }, +) { + get detail(): string { + return this.message; + } + + override get message(): string { + return `Stack rebase stopped at PR #${this.number} after ${this.completed} layers. Earlier updates remain on GitHub; resolve the failing layer before retrying.`; + } +} + +export type GitHubStackActionError = + | GitHubStackChangedError + | GitHubStackUnsupportedError + | GitHubStackResponseInvalidError + | GitHubStackMergeRejectedError + | GitHubStackMergePendingError + | GitHubStackPermissionError + | GitHubStackRebaseFailedError; + +const MergeResponse = Schema.Struct({ + status: Schema.Literals(["pending", "merged", "enqueued", "failed"]), + details: Schema.Struct({ + uuid: Schema.optional(Schema.String), + message: Schema.optional(Schema.String), + }), +}); + +const decodeBranchAccess = Schema.decodeEffect( + Schema.fromJsonString( + Schema.Struct({ + data: Schema.Struct({ + repository: Schema.NullOr( + Schema.Record( + Schema.String, + Schema.NullOr( + Schema.Struct({ + headRepository: Schema.NullOr( + Schema.Struct({ viewerPermission: Schema.NullOr(Schema.String) }), + ), + maintainerCanModify: Schema.Boolean, + }), + ), + ), + ), + }), + }), + ), +); + +const decodeRebaseBranch = Schema.decodeEffect( + Schema.fromJsonString( + Schema.Struct({ + data: Schema.Struct({ + processed: Schema.optional( + Schema.Array(Schema.NullOr(Schema.Struct({ headRefOid: Schema.String }))), + ), + repository: Schema.Struct({ + pullRequest: Schema.Struct({ + id: Schema.String, + headRefOid: Schema.String, + baseRef: Schema.Struct({ compare: Schema.Struct({ behindBy: Schema.Int }) }), + }), + }), + }), + }), + ), +); +const decodeRebaseResponse = Schema.decodeEffect( + Schema.fromJsonString( + Schema.Struct({ + data: Schema.Struct({ + updatePullRequestBranch: Schema.Struct({ + pullRequest: Schema.Struct({ headRefOid: Schema.String }), + }), + }), + }), + ), +); + +const encodeNodeIds = Schema.encodeSync(Schema.fromJsonString(Schema.Array(Schema.String))); + +const decodeMergeResponse = Schema.decodeEffect(Schema.fromJsonString(MergeResponse)); + +/** Remote-only updates: a stack rebase never switches or rewrites the environment's checkout. */ +export const runGitHubStackAction = Effect.fn("runGitHubStackAction")(function* (input: { + cwd: string; + repository: string; + host: string; + number: number; + stackNumber: number; + expectedStackHeads?: ReadonlyArray; + action: PullRequestAction; + mergeMethod?: PullRequestMergeMethod; +}) { + const github = yield* GitHubCli.GitHubCli; + const identity = { + repository: input.repository, + number: input.number, + stackNumber: input.stackNumber, + }; + if (input.action !== "merge" && input.action !== "update-branch") + return yield* new GitHubStackUnsupportedError({ ...identity }); + const endpoint = `repos/${input.repository}`; + const read = yield* github.execute({ + cwd: input.cwd, + args: ["api", "--hostname", input.host, `${endpoint}/stacks?pull_request=${input.number}`], + }); + const decoded = decodePullRequestStacksJson(read.stdout); + if (Result.isFailure(decoded)) + return yield* new GitHubStackResponseInvalidError({ ...identity, cause: decoded.failure }); + const stack = decoded.success; + const targetIndex = stack?.layers.findIndex((layer) => layer.number === input.number) ?? -1; + const target = stack?.layers[targetIndex]; + if ( + stack?.number !== input.stackNumber || + target === undefined || + (input.action === "update-branch" && targetIndex !== stack.layers.length - 1) + ) { + return yield* new GitHubStackChangedError({ ...identity, number: input.number, completed: 0 }); + } + const affectedLayers = + input.action === "merge" ? stack.layers.slice(0, targetIndex + 1) : stack.layers; + const open = affectedLayers.filter((layer) => layer.state !== "merged"); + if (input.action === "merge" && target.state !== "open") + return yield* new GitHubStackUnsupportedError({ ...identity }); + if ( + !input.expectedStackHeads || + input.expectedStackHeads.length !== open.length || + new Set(input.expectedStackHeads.map((layer) => layer.number)).size !== open.length || + open.some( + (layer) => + !layer.headSha || + !input.expectedStackHeads?.some( + (expected) => expected.number === layer.number && expected.headSha === layer.headSha, + ), + ) + ) { + return yield* new GitHubStackChangedError({ ...identity, number: input.number, completed: 0 }); + } + if (open.length === 0 || open.some((layer) => layer.state !== "open")) + return yield* new GitHubStackUnsupportedError({ ...identity }); + if (input.action === "update-branch") { + const [owner, name] = input.repository.split("/"); + const permissions = yield* github.execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + "graphql", + "-f", + `owner=${owner}`, + "-f", + `name=${name}`, + "-f", + `query=query($owner:String!,$name:String!){repository(owner:$owner,name:$name){${open + .map( + (layer) => + `pr${layer.number}:pullRequest(number:${layer.number}){headRepository{viewerPermission} maintainerCanModify}`, + ) + .join(" ")}}}`, + ], + }); + const access = yield* decodeBranchAccess(permissions.stdout).pipe( + Effect.mapError((cause) => new GitHubStackResponseInvalidError({ ...identity, cause })), + ); + // viewerCanUpdateBranch is false for an already-current layer, even if rebasing its parent + // will make it stale. Check branch write access separately before touching any layer. + if ( + open.some((layer) => { + const pr = access.data.repository?.[`pr${layer.number}`]; + return ( + !pr?.headRepository || + (!pr.maintainerCanModify && + !["ADMIN", "MAINTAIN", "WRITE"].includes(pr.headRepository.viewerPermission ?? "")) + ); + }) + ) + return yield* new GitHubStackPermissionError({ ...identity }); + const processed: Array<{ id: string; number: number; headSha: string }> = []; + for (const [index, layer] of open.entries()) { + yield* Effect.gen(function* () { + const read = yield* github.execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + "graphql", + "-f", + `owner=${owner}`, + "-f", + `name=${name}`, + "-F", + `number=${layer.number}`, + "-f", + `sha=${layer.headSha}`, + "-f", + `query=query($owner:String!,$name:String!,$number:Int!,$sha:String!){${ + processed.length === 0 + ? "" + : `processed:nodes(ids:${encodeNodeIds(processed.map((head) => head.id))}){... on PullRequest{headRefOid}}` + } repository(owner:$owner,name:$name){pullRequest(number:$number){id headRefOid baseRef{compare(headRef:$sha){behindBy}}}}}`, + ], + }); + const { + data: { + processed: observed, + repository: { pullRequest: pr }, + }, + } = yield* decodeRebaseBranch(read.stdout); + // A push to an earlier layer must not silently become the next layer's new base. + const changed = processed.find( + (head, index) => observed?.[index]?.headRefOid !== head.headSha, + ); + if (changed !== undefined) + return yield* new GitHubStackChangedError({ + ...identity, + number: changed.number, + completed: index, + }); + if (pr.headRefOid !== layer.headSha) + return yield* new GitHubStackChangedError({ + ...identity, + number: layer.number, + completed: index, + }); + if (pr.baseRef.compare.behindBy === 0) { + processed.push({ id: pr.id, number: layer.number, headSha: pr.headRefOid }); + return; + } + // Pass the reviewed revision to GitHub, including when a push races this read. + const updated = yield* github.execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + "graphql", + "-f", + `id=${pr.id}`, + "-f", + `sha=${layer.headSha}`, + "-f", + "query=mutation($id:ID!,$sha:GitObjectID!){updatePullRequestBranch(input:{pullRequestId:$id,expectedHeadOid:$sha,updateMethod:REBASE}){pullRequest{headRefOid}}}", + ], + }); + const response = yield* decodeRebaseResponse(updated.stdout); + processed.push({ + id: pr.id, + number: layer.number, + headSha: response.data.updatePullRequestBranch.pullRequest.headRefOid, + }); + }).pipe( + Effect.mapError((cause) => + cause._tag === "GitHubStackChangedError" + ? cause + : new GitHubStackRebaseFailedError({ + ...identity, + number: layer.number, + completed: index, + cause, + }), + ), + ); + } + return; + } + if (open.some((layer) => layer.isDraft)) + return yield* new GitHubStackUnsupportedError({ ...identity }); + const decode = (raw: string) => + decodeMergeResponse(raw).pipe( + Effect.mapError((cause) => new GitHubStackResponseInvalidError({ ...identity, cause })), + ); + const request = yield* github.execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + "--method", + "PUT", + `${endpoint}/pulls/${input.number}/merge-async`, + "-f", + `merge_method=${input.mergeMethod ?? "merge"}`, + "-f", + "merge_action=default", + "-f", + `sha=${target.headSha}`, + ], + }); + let result = yield* decode(request.stdout); + const deadline = (yield* Clock.currentTimeMillis) + 5 * 60_000; + for ( + let attempt = 0; + result.status === "pending" && (yield* Clock.currentTimeMillis) < deadline; + attempt++ + ) { + const uuid = result.details.uuid; + if (!uuid) return yield* new GitHubStackResponseInvalidError({ ...identity }); + yield* Effect.sleep(Math.min(1_000 * 2 ** attempt, 10_000)); + const poll = yield* github.execute({ + cwd: input.cwd, + args: [ + "api", + "--hostname", + input.host, + `${endpoint}/pulls/${input.number}/merge-async/${encodeURIComponent(uuid)}`, + ], + }); + result = yield* decode(poll.stdout); + } + if (result.status === "pending") return yield* new GitHubStackMergePendingError({ ...identity }); + if (result.status === "failed") + return yield* new GitHubStackMergeRejectedError({ ...identity, cause: result }); +}); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 7d3a0aa68..6d99cc652 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -2069,6 +2069,10 @@ const makeWsRpcLayer = ( observeRpcEffect(WS_METHODS.pullRequestsListStats, pullRequests.listStats(input), { "rpc.aggregate": "pull-requests", }), + [WS_METHODS.pullRequestsStack]: (input) => + observeRpcEffect(WS_METHODS.pullRequestsStack, pullRequests.stack(input), { + "rpc.aggregate": "pull-requests", + }), [WS_METHODS.pullRequestsDetail]: (input) => observeRpcEffect(WS_METHODS.pullRequestsDetail, pullRequests.detail(input), { "rpc.aggregate": "pull-requests", diff --git a/apps/swift-ios/App/NativeFeatureClient.swift b/apps/swift-ios/App/NativeFeatureClient.swift index 5a37e6fd6..79de8b53a 100644 --- a/apps/swift-ios/App/NativeFeatureClient.swift +++ b/apps/swift-ios/App/NativeFeatureClient.swift @@ -1259,12 +1259,41 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, try? await refresh(client: route.client) } + func addThreadPullRequest(threadID: String, number: Int) async throws -> FeatureLinkedPullRequest? { + try await changeThreadLinkedPullRequest(threadID: threadID, number: number, adding: true) + } + + func removeThreadPullRequest(threadID: String, link: FeatureLinkedPullRequest) async throws { + let route = try threadRoute(for: threadID) + guard (try await runtime.environments()).first(where: { $0.id == route.environmentID })?.descriptor?.capabilities.threadPullRequestsV2 == true else { + throw FeatureCapabilityUnavailable("Multiple pull requests") + } + guard let shell = shellsByEnvironmentID[route.environmentID], + let thread = shell.threads.first(where: { $0.id == route.wireID }), + let wire = (thread.linkedPullRequests ?? thread.linkedPullRequest.map { [$0] } ?? []).first(where: { $0.number == link.number && $0.url == link.url }) else { + throw NativeFeatureClientError.workspaceNotFound + } + _ = try await route.client.dispatch(OrchestrationCommands.updateMetadata(threadID: route.wireID, fields: ["unlinkPullRequest": try JSONValue.encode(wire)])) + try? await refresh(client: route.client) + } + + @discardableResult + func setThreadLinkedPullRequest(threadID: String, number: Int?) async throws -> FeatureLinkedPullRequest? { + try await changeThreadLinkedPullRequest(threadID: threadID, number: number, adding: false) + } + @discardableResult - func setThreadLinkedPullRequest( + private func changeThreadLinkedPullRequest( threadID: String, - number: Int? + number: Int?, + adding: Bool ) async throws -> FeatureLinkedPullRequest? { let route = try threadRoute(for: threadID) + if adding { + guard (try await runtime.environments()).first(where: { $0.id == route.environmentID })?.descriptor?.capabilities.threadPullRequestsV2 == true else { + throw FeatureCapabilityUnavailable("Multiple pull requests") + } + } guard let number else { _ = try await route.client.setLinkedPullRequest( threadID: route.wireID, @@ -1290,15 +1319,9 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, repository: repository, number: number ) - _ = try await route.client.setLinkedPullRequest( - threadID: route.wireID, - pullRequest: OrchestrationV2ThreadLinkedPullRequest( - projectId: project.id, - repository: repository, - number: detail.number, - url: detail.url - ) - ) + let link = OrchestrationV2ThreadLinkedPullRequest(projectId: project.id, repository: repository, number: detail.number, url: detail.url) + _ = try await route.client.dispatch(OrchestrationCommands.updateMetadata(threadID: route.wireID, + fields: [adding ? "linkPullRequest" : "linkedPullRequest": try JSONValue.encode(link)])) try? await refresh(client: route.client) return FeatureLinkedPullRequest( projectID: FeatureScopedID.project( @@ -1983,6 +2006,29 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, return detail } + func pullRequestStack(threadID: String, number: Int) async throws -> PullRequestStack? { + let route = try threadRoute(for: threadID) + guard (try await runtime.environments()).first(where: { $0.id == route.environmentID })?.descriptor?.capabilities.pullRequestStackActions == true else { return nil } + guard let shell = shellsByEnvironmentID[route.environmentID], + let thread = shell.threads.first(where: { $0.id == route.wireID }), + let project = shell.projects.first(where: { $0.id == thread.projectId }), + let repository = project.repositoryIdentity?.displayName else { throw NativeFeatureClientError.repositoryIdentityUnavailable } + return try await route.client.pullRequestStack(projectID: project.id, repository: repository, number: number) + } + + func runPullRequestStackAction(threadID: String, number: Int, stack: PullRequestStack, action: String, mergeMethod: String?) async throws { + let route = try threadRoute(for: threadID) + guard (try await runtime.environments()).first(where: { $0.id == route.environmentID })?.descriptor?.capabilities.pullRequestStackActions == true else { + throw FeatureCapabilityUnavailable("Stack actions") + } + guard let shell = shellsByEnvironmentID[route.environmentID], + let thread = shell.threads.first(where: { $0.id == route.wireID }), + let project = shell.projects.first(where: { $0.id == thread.projectId }), + let repository = project.repositoryIdentity?.displayName else { throw NativeFeatureClientError.repositoryIdentityUnavailable } + try await route.client.runPullRequestStackAction(projectID: project.id, repository: repository, number: number, + stack: stack, action: action, mergeMethod: mergeMethod) + } + func pullRequestOverview(threadID: String, number: Int) async throws -> FeaturePullRequestOverview { @@ -2049,19 +2095,18 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, // A linked pull request is the thread's own answer and outranks whatever // its worktree's branch happens to point at: the same branch can back // several requests, and a thread whose worktree is gone still has one. - var linkedByThreadID: [String: LinkedChangeRequestSubscription] = [:] + var linkedByThreadID: [String: [LinkedChangeRequestSubscription]] = [:] for threadID in threadIDs { guard let route = try? threadRoute(for: threadID), let shell = shellsByEnvironmentID[route.environmentID], let thread = shell.threads.first(where: { $0.id == route.wireID }) else { continue } - if let linked = thread.linkedPullRequest { - linkedByThreadID[threadID] = LinkedChangeRequestSubscription( - environmentID: route.environmentID, - projectWireID: linked.projectId, - repository: linked.repository, - number: linked.number - ) + let links = thread.linkedPullRequests ?? thread.linkedPullRequest.map { [$0] } ?? [] + if !links.isEmpty { + linkedByThreadID[threadID] = links.map { linked in + LinkedChangeRequestSubscription(environmentID: route.environmentID, + projectWireID: linked.projectId, repository: linked.repository, number: linked.number) + } continue } guard let context = try? workspaceContext(route: route), @@ -2144,28 +2189,20 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, /// within half a minute instead of at the next app launch. private func pollLinkedChangeRequest( threadID: String, - subscription: LinkedChangeRequestSubscription, + subscription: [LinkedChangeRequestSubscription], accumulator: ChangeRequestAccumulator, into continuation: AsyncStream<[String: FeaturePullRequest]>.Continuation ) async { while !Task.isCancelled { - guard let client = environmentClients[subscription.environmentID] else { return } - let detail = try? await client.pullRequestDetail( - projectID: subscription.projectWireID, - repository: subscription.repository, - number: subscription.number - ) - if Task.isCancelled { return } - // A failed read leaves the previous answer in place. The host is - // reached through the `gh` CLI, so a flaky read is ordinary; blanking - // the badge on one would make a merged row bounce back to Active. - if let detail, - let merged = accumulator.applyLinked( - threadID: threadID, - pullRequest: NativeWorkspaceMapper.pullRequest(detail) - ) { - continuation.yield(merged) + guard let first = subscription.first, let client = environmentClients[first.environmentID] else { return } + var reads: [FeaturePullRequest?] = [] + for link in subscription { + let detail = try? await client.pullRequestDetail(projectID: link.projectWireID, repository: link.repository, number: link.number) + if Task.isCancelled { return } + reads.append(detail.map(NativeWorkspaceMapper.pullRequest)) } + let summary = FeatureLinkedPullRequestSettlement.aggregate(reads) ?? FeaturePullRequest(number: first.number, title: "Pull requests unavailable", state: "unknown") + if let merged = accumulator.applyLinked(threadID: threadID, pullRequest: summary) { continuation.yield(merged) } try? await Task.sleep(for: .seconds(30)) } } @@ -4595,6 +4632,9 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, thread.linkedPullRequest, environment: environment ), + linkedPullRequests: thread.linkedPullRequests.map { links in links.compactMap { mapLinkedPullRequest($0, environment: environment) } }, + supportsMultiplePullRequests: environment.descriptor?.capabilities.threadPullRequestsV2, + supportsPullRequestStackActions: environment.descriptor?.capabilities.pullRequestStackActions, supportsPullRequestLinking: environment.descriptor?.capabilities .threadPullRequestLinking, attentionAt: latestRun?.status == "failed" @@ -4724,6 +4764,9 @@ final class NativeFeatureClient: FeatureClient, FeatureDeviceManaging, thread.linkedPullRequest, environment: environment ), + linkedPullRequests: thread.linkedPullRequests.map { links in links.compactMap { mapLinkedPullRequest($0, environment: environment) } }, + supportsMultiplePullRequests: environment.descriptor?.capabilities.threadPullRequestsV2, + supportsPullRequestStackActions: environment.descriptor?.capabilities.pullRequestStackActions, supportsPullRequestLinking: environment.descriptor?.capabilities .threadPullRequestLinking, // A failed run is the only thing that earns an attention marker; a diff --git a/apps/swift-ios/Core/Models.swift b/apps/swift-ios/Core/Models.swift index bebbbfbcf..0bc2d3815 100644 --- a/apps/swift-ios/Core/Models.swift +++ b/apps/swift-ios/Core/Models.swift @@ -50,6 +50,8 @@ public struct EnvironmentDescriptor: Codable, Equatable, Sendable { /// Absent on older servers, so the link action stays hidden rather than /// sending a command the server will reject. public let threadPullRequestLinking: Bool? + public let threadPullRequestsV2: Bool? + public let pullRequestStackActions: Bool? public let pullRequests: Bool? public let serverSelfUpdate: String? public let serverSelfUpdateProgress: Bool? @@ -62,6 +64,8 @@ public struct EnvironmentDescriptor: Codable, Equatable, Sendable { case threadPinning, threadActiveOrderV2, threadQuestionActionsV2 case threadTitleRegeneration case threadPullRequestLinking + case threadPullRequestsV2 + case pullRequestStackActions case pullRequests case serverSelfUpdate case serverSelfUpdateProgress @@ -81,6 +85,8 @@ public struct EnvironmentDescriptor: Codable, Equatable, Sendable { Bool.self, forKey: .threadTitleRegeneration ) + threadPullRequestsV2 = try container.decodeIfPresent(Bool.self, forKey: .threadPullRequestsV2) + pullRequestStackActions = try container.decodeIfPresent(Bool.self, forKey: .pullRequestStackActions) threadPullRequestLinking = try container.decodeIfPresent( Bool.self, forKey: .threadPullRequestLinking diff --git a/apps/swift-ios/Core/OrchestrationV2Models.swift b/apps/swift-ios/Core/OrchestrationV2Models.swift index 4e73e22ad..5a66ce322 100644 --- a/apps/swift-ios/Core/OrchestrationV2Models.swift +++ b/apps/swift-ios/Core/OrchestrationV2Models.swift @@ -880,6 +880,7 @@ public struct OrchestrationV2AppThread: Codable, Equatable, Sendable, Identifiab public let worktreePath: String? /// See `OrchestrationV2ThreadShell.linkedPullRequest`. public let linkedPullRequest: OrchestrationV2ThreadLinkedPullRequest? + public var linkedPullRequests: [OrchestrationV2ThreadLinkedPullRequest]? = nil public let activeProviderThreadId: String? public let historyOrigin: String? public let lineage: OrchestrationV2AppThreadLineage @@ -1299,6 +1300,7 @@ public struct OrchestrationV2ThreadShell: Codable, Equatable, Sendable, Identifi /// Absent on servers that predate pull-request linking, and on threads with /// nothing linked. Nil means "resolve the pull request from the branch". public var linkedPullRequest: OrchestrationV2ThreadLinkedPullRequest? + public var linkedPullRequests: [OrchestrationV2ThreadLinkedPullRequest]? = nil public var lineage: OrchestrationV2AppThreadLineage public var forkedFrom: OrchestrationV2ForkSource? public var activeProviderThreadId: String? diff --git a/apps/swift-ios/Core/PullRequestModels.swift b/apps/swift-ios/Core/PullRequestModels.swift index f6ecf86c9..e3e7eafb9 100644 --- a/apps/swift-ios/Core/PullRequestModels.swift +++ b/apps/swift-ios/Core/PullRequestModels.swift @@ -2,9 +2,8 @@ import Foundation // Pull-request detail and activity, as `packages/contracts/src/pullRequest.ts` // reports them over the `pullRequests.detail` and `pullRequests.activity` WS -// RPCs. Only the fields the read-only sheet renders are modelled; the -// capability, permission and merge-method blocks the actions UI would need are -// left undeclared, which `JSONDecoder` simply skips. +// RPCs. The detail sheet also decodes host capabilities and viewer permissions +// to gate reviewed stack actions. Unused response fields are skipped. // // Dates stay ISO strings, matching how the other Core models carry // `IsoDateTime`. @@ -106,6 +105,8 @@ public enum PullRequestMergeability: String, Codable, Sendable { } public struct PullRequestDetail: Codable, Equatable, Sendable { + public var capabilities: NativePullRequestCapabilities? = nil + public var viewerPermissions: NativePullRequestViewerPermissions? = nil public let projectId: String public let projectTitle: String public let repository: String @@ -146,3 +147,38 @@ public struct PullRequestActivity: Codable, Equatable, Sendable { public let reviewThreads: [PullRequestReviewThread] public let commits: [PullRequestCommit] } + +public struct PullRequestStack: Codable, Equatable, Sendable { + public let id: String + public let number: Int + public let url: String + public let base: String + public let layers: [Layer] + + public struct Layer: Codable, Equatable, Sendable, Identifiable { + public var id: Int { number } + public let number: Int + public let title: String? + public let isDraft: Bool? + public let headSha: String? + public let headBranch: String + public let state: PullRequestState + } + + /// Only the reviewed open layers travel; the server revalidates each revision before writing. + public func affectedLayers(number: Int, action: String) -> [Layer] { + guard let index = layers.firstIndex(where: { $0.number == number }) else { return [] } + return (action == "merge" ? Array(layers.prefix(index + 1)) : layers).filter { $0.state != .merged } + } +} + +public struct NativePullRequestCapabilities: Codable, Equatable, Sendable { + public let actions: [String] + public let mergeMethods: [String] + public let updateMethods: [String]? +} + +public struct NativePullRequestViewerPermissions: Codable, Equatable, Sendable { + public let actions: [String] + public let updateMethods: [String]? +} diff --git a/apps/swift-ios/Core/T3Client.swift b/apps/swift-ios/Core/T3Client.swift index d964d89e5..a58be442c 100644 --- a/apps/swift-ios/Core/T3Client.swift +++ b/apps/swift-ios/Core/T3Client.swift @@ -742,6 +742,28 @@ public actor T3Client { ) } + public func pullRequestStack(projectID: String, repository: String, number: Int) async throws -> PullRequestStack? { + try await rpc.request("pullRequests.stack", payload: .object([ + "projectId": .string(projectID), "repository": .string(repository), "number": .number(Double(number)), + ]), as: Optional.self) + } + + public func runPullRequestStackAction(projectID: String, repository: String, number: Int, + stack: PullRequestStack, action: String, mergeMethod: String?) async throws { + let heads = stack.affectedLayers(number: number, action: action) + guard !heads.isEmpty, heads.allSatisfy({ $0.headSha != nil }) else { + throw RPCError.remote("Refresh the stack before performing this action.") + } + var fields: [String: JSONValue] = [ + "projectId": .string(projectID), "repository": .string(repository), "number": .number(Double(number)), + "stackNumber": .number(Double(stack.number)), "action": .string(action), + "expectedStackHeads": .array(heads.map { .object(["number": .number(Double($0.number)), "headSha": .string($0.headSha!)]) }), + ] + if let mergeMethod { fields["mergeMethod"] = .string(mergeMethod) } + if action == "update-branch" { fields["updateMethod"] = .string("rebase") } + let _: JSONValue = try await rpc.request("pullRequests.runAction", payload: .object(fields), as: JSONValue.self) + } + public func pullRequestActivity( projectID: String, repository: String, diff --git a/apps/swift-ios/Features/Chat/FeatureLinkedPullRequestSettlement.swift b/apps/swift-ios/Features/Chat/FeatureLinkedPullRequestSettlement.swift new file mode 100644 index 000000000..330eda4bf --- /dev/null +++ b/apps/swift-ios/Features/Chat/FeatureLinkedPullRequestSettlement.swift @@ -0,0 +1,19 @@ +import Foundation + +/// A primary badge may only become terminal after every linked request has answered as terminal. +enum FeatureLinkedPullRequestSettlement { + static func aggregate(_ reads: [FeaturePullRequest?]) -> FeaturePullRequest? { + let known = reads.compactMap { $0 } + guard var result = known.first(where: { $0.state == "open" }) ?? known.first else { return nil } + if result.state == "open" { return result } + guard known.count == reads.count, + known.allSatisfy({ $0.state == "closed" || $0.state == "merged" }) else { + result.state = "unknown" + return result + } + // If merge settlement is disabled, a mixed closed/merged collection must stay active. + result.state = known.contains(where: { $0.state == "merged" }) ? "merged" : "closed" + result.updatedAt = known.compactMap(\.updatedAt).max() + return result + } +} diff --git a/apps/swift-ios/Features/Chat/MarkdownMediaView.swift b/apps/swift-ios/Features/Chat/MarkdownMediaView.swift index ff0eb79ca..ab2f6536a 100644 --- a/apps/swift-ios/Features/Chat/MarkdownMediaView.swift +++ b/apps/swift-ios/Features/Chat/MarkdownMediaView.swift @@ -340,9 +340,7 @@ struct FeatureImagePreviewSheet: View { AsyncImage(url: url) { phase in switch phase { case let .success(image): - image - .resizable() - .scaledToFit() + ZoomableMessageImage(image: image) case .failure: ContentUnavailableView( "Image unavailable", @@ -403,7 +401,7 @@ struct MarkdownGallerySheet: View { } else if let url = urls[page] { AsyncImage(url: url) { phase in switch phase { - case let .success(image): image.resizable().scaledToFit() + case let .success(image): ZoomableMessageImage(image: image, isCurrentPage: page == index) case .failure: ContentUnavailableView("Image unavailable", systemImage: "photo") default: ProgressView() } diff --git a/apps/swift-ios/Features/Chat/PullRequestDetailSheet.swift b/apps/swift-ios/Features/Chat/PullRequestDetailSheet.swift index 129ffcc50..b4f6d4513 100644 --- a/apps/swift-ios/Features/Chat/PullRequestDetailSheet.swift +++ b/apps/swift-ios/Features/Chat/PullRequestDetailSheet.swift @@ -1,17 +1,18 @@ import SwiftUI -// A read-only, native view of one change request: the summary the host's page -// leads with, and the conversation-plus-commits chronology under it. Opened -// from the thread details sheet's Version Control section; anything beyond -// reading — reviews, merges, comments — stays in the browser, one tap away. -// -// Every rule lives in PullRequestDetailSections.swift; this file is the view. +// Native PR details and reviewed, remote-only GitHub stack actions. struct PullRequestDetailSheet: View { let client: any FeatureClient let threadID: String let number: Int + @State private var selectedNumber: Int? + @State private var stack: PullRequestStack? + @State private var stackError: String? + @State private var pendingStackAction: NativeStackAction? + private var displayedNumber: Int { selectedNumber ?? number } + @State private var overview: FeaturePullRequestOverview? @State private var loadError: String? @State private var tab: PullRequestDetailTab = .summary @@ -29,7 +30,7 @@ struct PullRequestDetailSheet: View { } } .background(T3Colors.background) - .navigationTitle("Pull Request #\(number)") + .navigationTitle("Pull Request #\(displayedNumber)") .navigationBarTitleDisplayMode(.inline) .toolbar { ToolbarItem(placement: .topBarTrailing) { @@ -43,15 +44,35 @@ struct PullRequestDetailSheet: View { } } } - .task { await load() } + .task(id: displayedNumber) { await load() } + .sheet(item: $pendingStackAction, onDismiss: { Task { await load() } }) { request in + PullRequestStackActionSheet(request: request, client: client, threadID: threadID) { + pendingStackAction = nil + } + } .accessibilityIdentifier("pull-request-detail-sheet") } private func load() async { + let requestedNumber = displayedNumber loadError = nil + overview = nil + stack = nil + stackError = nil do { - overview = try await client.pullRequestOverview(threadID: threadID, number: number) + let result = try await client.pullRequestOverview(threadID: threadID, number: requestedNumber) + guard !Task.isCancelled, displayedNumber == requestedNumber else { return } + overview = result + do { + let loadedStack = try await client.pullRequestStack(threadID: threadID, number: requestedNumber) + guard !Task.isCancelled, displayedNumber == requestedNumber else { return } + stack = loadedStack + } catch { + guard !Task.isCancelled, displayedNumber == requestedNumber else { return } + stackError = error.localizedDescription + } } catch { + guard !Task.isCancelled, displayedNumber == requestedNumber else { return } loadError = error.localizedDescription } } @@ -77,6 +98,11 @@ struct PullRequestDetailSheet: View { ScrollView { VStack(alignment: .leading, spacing: 16) { header(overview.detail) + if let stack { stackSection(stack, detail: overview.detail) } + if let stackError { + Text("Could not load stack: \(stackError)").font(T3Typography.supporting).foregroundStyle(T3Colors.warning) + Button("Retry stack") { Task { await load() } } + } Picker("Section", selection: $tab) { ForEach(PullRequestDetailTab.allCases, id: \.self) { tab in @@ -99,6 +125,28 @@ struct PullRequestDetailSheet: View { .scrollIndicators(.hidden) } + private func stackSection(_ stack: PullRequestStack, detail: PullRequestDetail) -> some View { + ThreadDetailsSection(title: "Stack · \(stack.layers.count) layers", footer: "Layers run from base to top. Actions update GitHub without changing your checkout.") { + ForEach(stack.layers) { layer in + ThreadDetailsRow(systemImage: layer.number == displayedNumber ? "checkmark.circle.fill" : "arrow.triangle.pull", + title: "#\(layer.number) \(layer.title ?? layer.headBranch)", subtitle: layer.state.rawValue.capitalized, + showsChevron: layer.number != displayedNumber, action: { selectedNumber = layer.number }) + } + if detail.state == .open, let capabilities = detail.capabilities, let viewer = detail.viewerPermissions { + if capabilities.actions.contains("merge"), viewer.actions.contains("merge") { + ThreadDetailsRow(systemImage: "arrow.triangle.merge", title: "Review merge through #\(displayedNumber)…", + action: { pendingStackAction = NativeStackAction(stack: stack, number: displayedNumber, action: "merge", mergeMethods: capabilities.mergeMethods) }) + } + if stack.layers.last?.number == displayedNumber, + capabilities.actions.contains("update-branch"), viewer.actions.contains("update-branch"), + capabilities.updateMethods?.contains("rebase") == true, viewer.updateMethods?.contains("rebase") == true { + ThreadDetailsRow(systemImage: "arrow.triangle.branch", title: "Review stack rebase…", + action: { pendingStackAction = NativeStackAction(stack: stack, number: displayedNumber, action: "update-branch", mergeMethods: []) }) + } + } + } + } + private func header(_ detail: PullRequestDetail) -> some View { VStack(alignment: .leading, spacing: 8) { Text("#\(detail.number) \(detail.title)") diff --git a/apps/swift-ios/Features/Chat/PullRequestStackActionSheet.swift b/apps/swift-ios/Features/Chat/PullRequestStackActionSheet.swift new file mode 100644 index 000000000..39c5762bb --- /dev/null +++ b/apps/swift-ios/Features/Chat/PullRequestStackActionSheet.swift @@ -0,0 +1,83 @@ +import SwiftUI + +struct NativeStackAction: Identifiable { + let id = UUID() + let stack: PullRequestStack + let number: Int + let action: String + let mergeMethods: [String] + var layers: [PullRequestStack.Layer] { stack.affectedLayers(number: number, action: action) } +} + +/// Holds the exact stack the reader reviewed; a refresh must never alter an armed operation. +struct PullRequestStackActionSheet: View { + let request: NativeStackAction + let client: any FeatureClient + let threadID: String + let onFinished: () -> Void + @State private var method = "" + @State private var isBusy = false + @State private var errorMessage: String? + + private var isMerge: Bool { request.action == "merge" } + private var ready: Bool { + !request.layers.isEmpty && request.layers.allSatisfy { $0.state == .open && $0.headSha != nil } + && (!isMerge || request.mergeMethods.contains(method)) + } + + var body: some View { + NavigationStack { + ScrollView { + VStack(alignment: .leading, spacing: 16) { + Text(isMerge ? "Merge through #\(request.number)" : "Rebase stack") + .font(T3Typography.threadHeading2) + Text("These layers will be updated on GitHub. Your local checkout stays unchanged.") + .font(T3Typography.supporting).foregroundStyle(T3Colors.textSecondary) + ThreadDetailsSection(title: "Affected layers") { + ForEach(request.layers) { layer in + ThreadDetailsRow(systemImage: "arrow.triangle.pull", title: "#\(layer.number) \(layer.title ?? layer.headBranch)", + subtitle: String((layer.headSha ?? "Revision unavailable").prefix(12)), showsChevron: false) + } + } + if isMerge { + Picker("Merge strategy", selection: $method) { + ForEach(request.mergeMethods, id: \.self) { Text($0.capitalized).tag($0) } + } + } + if let errorMessage { + Text(errorMessage).foregroundStyle(T3Colors.danger) + Text("Earlier completed updates remain on GitHub. Close this sheet to refresh before another attempt.") + .font(T3Typography.supporting).foregroundStyle(T3Colors.textSecondary) + } + if !ready { + Text("Refresh the stack to load every open layer’s revision before continuing.") + .font(T3Typography.supporting).foregroundStyle(T3Colors.warning) + } + SettingsActionButton(title: isMerge ? "Merge reviewed layers" : "Rebase reviewed layers", + systemImage: "arrow.triangle.merge", tone: .primary, isBusy: isBusy, + isDisabled: !ready || errorMessage != nil, action: perform) + }.padding(16) + } + .background(T3Colors.background) + .navigationTitle("Confirm stack action") + .navigationBarTitleDisplayMode(.inline) + .toolbar { ToolbarItem(placement: .cancellationAction) { Button("Close", action: onFinished).disabled(isBusy) } } + .t3NavigationChrome() + } + .interactiveDismissDisabled(isBusy) + .onAppear { method = request.mergeMethods.first ?? "" } + } + + private func perform() { + guard ready, !isBusy, errorMessage == nil else { return } + isBusy = true + Task { @MainActor in + do { + try await client.runPullRequestStackAction(threadID: threadID, number: request.number, + stack: request.stack, action: request.action, mergeMethod: isMerge ? method : nil) + onFinished() + } catch { errorMessage = error.localizedDescription } + isBusy = false + } + } +} diff --git a/apps/swift-ios/Features/Chat/ThreadDetailsSheet.swift b/apps/swift-ios/Features/Chat/ThreadDetailsSheet.swift index ba384123e..bf4003226 100644 --- a/apps/swift-ios/Features/Chat/ThreadDetailsSheet.swift +++ b/apps/swift-ios/Features/Chat/ThreadDetailsSheet.swift @@ -496,10 +496,8 @@ struct ThreadDetailsSheet: View { ThreadDetailsDivider() ThreadDetailsRow( systemImage: "link", - title: "Linked pull request", - subtitle: ThreadDetailsGit.linkedPullRequestSubtitle( - thread.linkedPullRequest - ), + title: thread.supportsMultiplePullRequests == true ? "Linked pull requests" : "Linked pull request", + subtitle: thread.allLinkedPullRequests.isEmpty ? "None" : thread.allLinkedPullRequests.map { "#\($0.number)" }.joined(separator: ", "), action: { isEditingLinkedPullRequest = true } ) } diff --git a/apps/swift-ios/Features/Chat/ThreadLinkedPullRequestSheet.swift b/apps/swift-ios/Features/Chat/ThreadLinkedPullRequestSheet.swift index 06becbb20..36341cff7 100644 --- a/apps/swift-ios/Features/Chat/ThreadLinkedPullRequestSheet.swift +++ b/apps/swift-ios/Features/Chat/ThreadLinkedPullRequestSheet.swift @@ -24,11 +24,13 @@ struct ThreadLinkedPullRequestSheet: View { @FocusState private var isFieldFocused: Bool private var linked: FeatureLinkedPullRequest? { thread.linkedPullRequest } + private var links: [FeatureLinkedPullRequest] { thread.allLinkedPullRequests } + @State private var selectedLink: FeatureLinkedPullRequest? /// Hidden when the branch's request is already the linked one: "Link #12" /// under a row that says #12 is linked reads as a bug. private var linkableBranchPullRequest: ThreadDetailsPullRequest? { - guard let branchPullRequest, branchPullRequest.number != linked?.number else { return nil } + guard let branchPullRequest, !links.contains(where: { $0.number == branchPullRequest.number }) else { return nil } return branchPullRequest } @@ -37,30 +39,16 @@ struct ThreadLinkedPullRequestSheet: View { var body: some View { ScrollView { VStack(alignment: .leading, spacing: 16) { - if let linked { - ThreadDetailsSection( - title: "Linked", - footer: """ - This thread follows pull request #\(linked.number) instead of whatever \ - its branch points at. - """ - ) { - ThreadDetailsRow( - systemImage: "arrow.triangle.pull", - title: "#\(linked.number)", - subtitle: linked.repository, - showsChevron: false - ) - ThreadDetailsDivider() - ThreadDetailsRow( - systemImage: "link.badge.plus", - iconTint: T3Colors.danger, - title: "Unlink", - subtitle: "Go back to following the branch", - isDisabled: isBusy, - showsChevron: false, - action: { commit(number: nil) } - ) + if !links.isEmpty { + ThreadDetailsSection(title: "Linked pull requests", footer: "The task stays active while any linked pull request is open.") { + ForEach(links, id: \.self) { link in + ThreadDetailsRow(systemImage: "arrow.triangle.pull", title: "#\(link.number)", subtitle: link.repository, + action: { selectedLink = link }) + ThreadDetailsRow(systemImage: "link.badge.plus", iconTint: T3Colors.danger, + title: "Unlink #\(link.number)", isDisabled: isBusy, showsChevron: false, + action: { unlink(link) }) + if link != links.last { ThreadDetailsDivider() } + } } } @@ -78,7 +66,7 @@ struct ThreadLinkedPullRequestSheet: View { } ThreadDetailsSection( - title: linked == nil ? "Link a pull request" : "Link a different one", + title: thread.supportsMultiplePullRequests == true ? "Add a pull request" : (linked == nil ? "Link a pull request" : "Link a different one"), footer: """ Enter a number or paste a pull request URL. It has to belong to this \ thread's project. @@ -119,7 +107,10 @@ struct ThreadLinkedPullRequestSheet: View { .scrollDismissesKeyboard(.interactively) .frame(maxWidth: .infinity, maxHeight: .infinity) .background(T3Colors.background) - .navigationTitle("Pull request") + .navigationTitle("Pull requests") + .navigationDestination(item: $selectedLink) { link in + PullRequestDetailSheet(client: client, threadID: thread.id, number: link.number) + } .navigationBarTitleDisplayMode(.inline) .toolbar { ToolbarItem(placement: .cancellationAction) { @@ -129,6 +120,17 @@ struct ThreadLinkedPullRequestSheet: View { } } + private func unlink(_ link: FeatureLinkedPullRequest) { + guard !isBusy else { return } + if thread.supportsMultiplePullRequests != true { commit(number: nil); return } + isBusy = true; errorMessage = nil + Task { @MainActor in + do { try await client.removeThreadPullRequest(threadID: thread.id, link: link); onFinished() } + catch { errorMessage = error.localizedDescription } + isBusy = false + } + } + /// `nil` unlinks. Either way the sheet closes on success and stays open on /// failure, because the failure is about the value still in the field. private func commit(number: Int?) { @@ -137,10 +139,11 @@ struct ThreadLinkedPullRequestSheet: View { errorMessage = nil Task { @MainActor in do { - _ = try await client.setThreadLinkedPullRequest( - threadID: thread.id, - number: number - ) + if thread.supportsMultiplePullRequests == true, let number { + _ = try await client.addThreadPullRequest(threadID: thread.id, number: number) + } else { + _ = try await client.setThreadLinkedPullRequest(threadID: thread.id, number: number) + } onFinished() } catch { errorMessage = error.localizedDescription diff --git a/apps/swift-ios/Features/Chat/ZoomableMessageImage.swift b/apps/swift-ios/Features/Chat/ZoomableMessageImage.swift new file mode 100644 index 000000000..3cacc6b6f --- /dev/null +++ b/apps/swift-ios/Features/Chat/ZoomableMessageImage.swift @@ -0,0 +1,65 @@ +import SwiftUI + +/// Keeps zoom inside the current image; fitted pages leave swipes to the gallery. +struct ZoomableMessageImage: View { + let image: Image + var isCurrentPage = true + @State private var scale: CGFloat = 1 + @State private var settledScale: CGFloat = 1 + @State private var offset: CGSize = .zero + @State private var settledOffset: CGSize = .zero + + var body: some View { + GeometryReader { geometry in + image.resizable().scaledToFit() + .frame(width: geometry.size.width, height: geometry.size.height) + .scaleEffect(scale).offset(offset) + .contentShape(Rectangle()) + .gesture(MagnifyGesture().onChanged { value in + scale = min(8, max(1, settledScale * value.magnification)) + offset = bounded(offset, in: geometry.size) + }.onEnded { _ in + settledScale = scale + settledOffset = offset + }) + .highPriorityGesture(DragGesture().onChanged { value in + offset = bounded(CGSize(width: settledOffset.width + value.translation.width, + height: settledOffset.height + value.translation.height), + in: geometry.size) + }.onEnded { _ in settledOffset = offset }, including: scale > 1 ? .all : .none) + .onTapGesture(count: 2) { + if scale > 1 { reset() } else { scale = 2; settledScale = 2 } + } + .accessibilityLabel("Image") + .accessibilityValue("\(Int(scale * 100)) percent zoom") + .accessibilityAdjustableAction { direction in + scale = min(8, max(1, scale + (direction == .increment ? 1 : -1))) + settledScale = scale + offset = bounded(offset, in: geometry.size) + settledOffset = offset + } + .accessibilityAction(named: "Fit image", reset) + } + .clipped() + .overlay(alignment: .topTrailing) { + Button("Fit image", systemImage: "arrow.down.right.and.arrow.up.left") { reset() } + .font(T3Typography.supportingStrong) + .padding(10) + .background(T3Colors.surface, in: Capsule()) + .padding(8) + .opacity(scale > 1 ? 1 : 0) + .allowsHitTesting(scale > 1) + .accessibilityHidden(scale <= 1) + } + .onChange(of: isCurrentPage) { reset() } + } + + private func bounded(_ value: CGSize, in size: CGSize) -> CGSize { + CGSize(width: min(size.width * (scale - 1) / 2, max(-size.width * (scale - 1) / 2, value.width)), + height: min(size.height * (scale - 1) / 2, max(-size.height * (scale - 1) / 2, value.height))) + } + + private func reset() { + scale = 1; settledScale = 1; offset = .zero; settledOffset = .zero + } +} diff --git a/apps/swift-ios/Features/Root/FeatureRootModel.swift b/apps/swift-ios/Features/Root/FeatureRootModel.swift index c3560828e..11b45ce42 100644 --- a/apps/swift-ios/Features/Root/FeatureRootModel.swift +++ b/apps/swift-ios/Features/Root/FeatureRootModel.swift @@ -65,6 +65,7 @@ public final class FeatureRootModel { private var outboxRetryAttempt = 0 private var outboxGeneration: UInt64 = 0 private var changeRequestThreadIDs: [String] = [] + private var changeRequestLinks: [String: [FeatureLinkedPullRequest]] = [:] private var changeRequestTask: Task? public init( @@ -96,12 +97,15 @@ public final class FeatureRootModel { /// showing. Safe to call whenever that list is rebuilt: an unchanged set of /// threads keeps the existing subscriptions rather than restarting them. public func observeChangeRequests(threadIDs: [String]) { - guard threadIDs != changeRequestThreadIDs else { return } + let observed = Set(threadIDs) + let links = Dictionary(uniqueKeysWithValues: snapshot.threads.filter { observed.contains($0.id) }.map { ($0.id, $0.allLinkedPullRequests) }) + guard threadIDs != changeRequestThreadIDs || links != changeRequestLinks else { return } + let previousLinks = changeRequestLinks + changeRequestLinks = links changeRequestThreadIDs = threadIDs changeRequestTask?.cancel() - let observed = Set(threadIDs) - changeRequestsByThreadID = changeRequestsByThreadID.filter { observed.contains($0.key) } + changeRequestsByThreadID = changeRequestsByThreadID.filter { observed.contains($0.key) && previousLinks[$0.key] == links[$0.key] } guard !threadIDs.isEmpty else { changeRequestTask = nil return @@ -119,7 +123,7 @@ public final class FeatureRootModel { ) { // A cancelled stream can still hold one last emission; applying // it would overwrite the replacement stream's fresher state. - if Task.isCancelled || self.changeRequestThreadIDs != threadIDs { return } + if Task.isCancelled || self.changeRequestThreadIDs != threadIDs || self.changeRequestLinks != links { return } self.changeRequestsByThreadID = pullRequests } } @@ -680,6 +684,7 @@ public final class FeatureRootModel { } threadCollectionRevision &+= 1 homePresentationRevision &+= 1 + observeChangeRequests(threadIDs: changeRequestThreadIDs) } private func removeThread(id: String) { @@ -722,6 +727,7 @@ public final class FeatureRootModel { threadCollectionRevision &+= 1 } snapshot = value + observeChangeRequests(threadIDs: changeRequestThreadIDs) if value.connection.state == .connected || value.environments.contains(where: { $0.connectionState == .connected }) { scheduleOutboxDrain() diff --git a/apps/swift-ios/Features/Shared/FeatureClient.swift b/apps/swift-ios/Features/Shared/FeatureClient.swift index b359957f5..56d99e929 100644 --- a/apps/swift-ios/Features/Shared/FeatureClient.swift +++ b/apps/swift-ios/Features/Shared/FeatureClient.swift @@ -72,6 +72,10 @@ public protocol FeatureClient: AnyObject { threadID: String, number: Int? ) async throws -> FeatureLinkedPullRequest? + func addThreadPullRequest(threadID: String, number: Int) async throws -> FeatureLinkedPullRequest? + func removeThreadPullRequest(threadID: String, link: FeatureLinkedPullRequest) async throws + func pullRequestStack(threadID: String, number: Int) async throws -> PullRequestStack? + func runPullRequestStackAction(threadID: String, number: Int, stack: PullRequestStack, action: String, mergeMethod: String?) async throws func setRuntimeMode(id: String, mode: FeatureRuntimeMode) async throws func setInteractionMode(id: String, mode: FeatureInteractionMode) async throws /// Persists the model and its options (effort, context window) on the @@ -256,6 +260,10 @@ public extension FeatureClient { ) async throws -> FeatureLinkedPullRequest? { throw FeatureCapabilityUnavailable("Pull request linking") } + func addThreadPullRequest(threadID: String, number: Int) async throws -> FeatureLinkedPullRequest? { throw FeatureCapabilityUnavailable("Multiple pull requests") } + func removeThreadPullRequest(threadID: String, link: FeatureLinkedPullRequest) async throws { throw FeatureCapabilityUnavailable("Multiple pull requests") } + func pullRequestStack(threadID: String, number: Int) async throws -> PullRequestStack? { nil } + func runPullRequestStackAction(threadID: String, number: Int, stack: PullRequestStack, action: String, mergeMethod: String?) async throws { throw FeatureCapabilityUnavailable("Stack actions") } func setRuntimeMode(id: String, mode: FeatureRuntimeMode) async throws {} func setInteractionMode(id: String, mode: FeatureInteractionMode) async throws {} func setModelSelection(id: String, selection: FeatureSelection) async throws {} diff --git a/apps/swift-ios/Features/Shared/FeatureModels.swift b/apps/swift-ios/Features/Shared/FeatureModels.swift index fe9a2e908..6297e5478 100644 --- a/apps/swift-ios/Features/Shared/FeatureModels.swift +++ b/apps/swift-ios/Features/Shared/FeatureModels.swift @@ -270,6 +270,13 @@ public struct FeatureThread: Identifiable, Sendable, Equatable, Hashable, Codabl /// branch-derived one: the row shows it, and it is what the merge settle /// rule watches. public var linkedPullRequest: FeatureLinkedPullRequest? + public var linkedPullRequests: [FeatureLinkedPullRequest]? = nil + public var supportsMultiplePullRequests: Bool? = nil + public var supportsPullRequestStackActions: Bool? = nil + + public var allLinkedPullRequests: [FeatureLinkedPullRequest] { + linkedPullRequests ?? linkedPullRequest.map { [$0] } ?? [] + } /// Whether this thread's server persists a pull-request link. Resolved from /// the environment's capabilities at map time, so the action is hidden /// rather than offered and refused. @@ -325,6 +332,9 @@ public struct FeatureThread: Identifiable, Sendable, Equatable, Hashable, Codabl isRegeneratingTitle: Bool = false, supportsTitleRegeneration: Bool? = nil, linkedPullRequest: FeatureLinkedPullRequest? = nil, + linkedPullRequests: [FeatureLinkedPullRequest]? = nil, + supportsMultiplePullRequests: Bool? = nil, + supportsPullRequestStackActions: Bool? = nil, supportsPullRequestLinking: Bool? = nil, attentionAt: Date? = nil, workingStartedAt: Date? = nil, @@ -372,6 +382,9 @@ public struct FeatureThread: Identifiable, Sendable, Equatable, Hashable, Codabl self.isRegeneratingTitle = isRegeneratingTitle self.supportsTitleRegeneration = supportsTitleRegeneration self.linkedPullRequest = linkedPullRequest + self.linkedPullRequests = linkedPullRequests + self.supportsMultiplePullRequests = supportsMultiplePullRequests + self.supportsPullRequestStackActions = supportsPullRequestStackActions self.supportsPullRequestLinking = supportsPullRequestLinking self.attentionAt = attentionAt self.workingStartedAt = workingStartedAt diff --git a/apps/swift-ios/Features/Workspace/DailyUXModels.swift b/apps/swift-ios/Features/Workspace/DailyUXModels.swift index 2b4388f5c..066a6cea3 100644 --- a/apps/swift-ios/Features/Workspace/DailyUXModels.swift +++ b/apps/swift-ios/Features/Workspace/DailyUXModels.swift @@ -344,12 +344,13 @@ struct DailyUXSidebarIndex { } return candidates.filter { thread in let project = projectByID[thread.projectID] - return [ + return ([ thread.title, thread.preview ?? "", project?.name ?? "", project?.path ?? "", - ].contains { $0.localizedCaseInsensitiveContains(normalizedQuery) } + ] + thread.allLinkedPullRequests.flatMap { ["#\($0.number)", "\($0.repository)#\($0.number)", $0.url] }) + .contains { $0.localizedCaseInsensitiveContains(normalizedQuery) } } } } @@ -402,6 +403,7 @@ enum DailyUXSidebarRefresh { // ordinary inactivity clock. !thread.changeRequestAutoSettles(changeRequest), changeRequest?.state != "open", + !thread.hasUnresolvedLinkedPullRequests(changeRequest), let autoSettleAfterDays = thread.autoSettleAfterDays, let lastActivityAt = thread.lastActivityAt else { return nil @@ -602,6 +604,11 @@ extension FeatureThread { return max(createdAt, latestUserActivityAt) } + /// A missing aggregate must not let the inactivity timer hide unresolved linked work. + func hasUnresolvedLinkedPullRequests(_ changeRequest: FeaturePullRequest?) -> Bool { + allLinkedPullRequests.count > 1 && !["open", "closed", "merged"].contains(changeRequest?.state ?? "unknown") + } + /// Swift port of `changeRequestAutoSettles` in /// `packages/client-runtime/src/state/threadSettled.ts`. /// @@ -641,6 +648,7 @@ extension FeatureThread { if keepsActive { return false } + if hasUnresolvedLinkedPullRequests(changeRequest) { return false } if changeRequestAutoSettles(changeRequest) { return true } diff --git a/apps/swift-ios/Features/Workspace/WorkspaceView.swift b/apps/swift-ios/Features/Workspace/WorkspaceView.swift index 01003412f..09cb8c07a 100644 --- a/apps/swift-ios/Features/Workspace/WorkspaceView.swift +++ b/apps/swift-ios/Features/Workspace/WorkspaceView.swift @@ -719,7 +719,7 @@ public struct WorkspaceView: View { Image(systemName: "magnifyingglass") .font(.system(size: 14, weight: .medium)) .foregroundStyle(T3Colors.textTertiary) - TextField("Search tasks and projects", text: $searchText) + TextField("Search tasks, projects and PRs", text: $searchText) .font(.subheadline) .foregroundStyle(T3Colors.textPrimary) .focused($isSearchFocused) diff --git a/apps/swift-ios/Tests/CoreTests/Fixtures/orchestrationV2Projection.json b/apps/swift-ios/Tests/CoreTests/Fixtures/orchestrationV2Projection.json index 8281a9662..b751807c3 100644 --- a/apps/swift-ios/Tests/CoreTests/Fixtures/orchestrationV2Projection.json +++ b/apps/swift-ios/Tests/CoreTests/Fixtures/orchestrationV2Projection.json @@ -14,6 +14,26 @@ "interactionMode": "default", "branch": null, "worktreePath": null, + "linkedPullRequest": { + "projectId": "project-v2", + "repository": "example/repo", + "number": 41, + "url": "https://github.com/example/repo/pull/41" + }, + "linkedPullRequests": [ + { + "projectId": "project-v2", + "repository": "example/repo", + "number": 41, + "url": "https://github.com/example/repo/pull/41" + }, + { + "projectId": "project-v2", + "repository": "example/repo", + "number": 42, + "url": "https://github.com/example/repo/pull/42" + } + ], "activeProviderThreadId": "provider-thread-1", "lineage": { "parentThreadId": null, diff --git a/apps/swift-ios/Tests/CoreTests/Fixtures/pullRequestStack.json b/apps/swift-ios/Tests/CoreTests/Fixtures/pullRequestStack.json new file mode 100644 index 000000000..a437ac8ac --- /dev/null +++ b/apps/swift-ios/Tests/CoreTests/Fixtures/pullRequestStack.json @@ -0,0 +1,27 @@ +{ + "id": "stack-1", + "number": 1, + "url": "https://github.com/o/r/stack/1", + "base": "main", + "layers": [ + { + "number": 1, + "headBranch": "one", + "state": "merged" + }, + { + "number": 2, + "title": "Second layer", + "isDraft": false, + "headSha": "abc", + "headBranch": "two", + "state": "open" + }, + { + "number": 3, + "headSha": "def", + "headBranch": "three", + "state": "open" + } + ] +} diff --git a/apps/swift-ios/Tests/CoreTests/OrchestrationV2ContractTests.swift b/apps/swift-ios/Tests/CoreTests/OrchestrationV2ContractTests.swift index 249ba26e6..b46dfca4b 100644 --- a/apps/swift-ios/Tests/CoreTests/OrchestrationV2ContractTests.swift +++ b/apps/swift-ios/Tests/CoreTests/OrchestrationV2ContractTests.swift @@ -44,6 +44,8 @@ final class OrchestrationV2ContractTests: XCTestCase { func testNativeParityFieldsDecodeFromServerContract() throws { let projection = try projection() XCTAssertEqual(projection.thread.activeOrderKey, "n") + XCTAssertEqual(projection.thread.linkedPullRequests?.map(\.number), [41, 42]) + XCTAssertEqual(projection.thread.linkedPullRequest?.number, 41) XCTAssertEqual(projection.runtimeRequests.first?.responseMode, "message") let dismiss = OrchestrationCommands.respondToUserInput(threadID: "t", requestID: "q", answers: [:], dismiss: true) XCTAssertEqual(dismiss["dismiss"], .bool(true)) diff --git a/apps/swift-ios/Tests/FeatureTests/DailyUXSidebarTests.swift b/apps/swift-ios/Tests/FeatureTests/DailyUXSidebarTests.swift index bd0693fc2..4d950797c 100644 --- a/apps/swift-ios/Tests/FeatureTests/DailyUXSidebarTests.swift +++ b/apps/swift-ios/Tests/FeatureTests/DailyUXSidebarTests.swift @@ -4,6 +4,25 @@ import Testing @Suite("Sidebar v2") struct DailyUXSidebarTests { + @Test func unavailableLinkedRequestsBlockInactivitySettlement() { + var item = thread(id: "linked", created: -500, updated: -500) + item.linkedPullRequests = [41, 42].map { FeatureLinkedPullRequest(projectID: item.projectID, repository: "example/repo", number: $0, url: "https://github.com/example/repo/pull/\($0)") } + let later = now.addingTimeInterval(10 * 24 * 60 * 60) + #expect(!item.isEffectivelySettled(at: later)) + #expect(!item.isEffectivelySettled(at: later, changeRequest: FeaturePullRequest(number: 41, title: "Unavailable", state: "unknown"))) + item.isSettled = true + #expect(item.isEffectivelySettled(at: later)) + } + + @Test func searchesEveryLinkedPullRequest() { + var item = thread(id: "linked", created: -100, updated: -50) + item.linkedPullRequests = [41, 42].map { FeatureLinkedPullRequest(projectID: item.projectID, repository: "example/repo", number: $0, url: "https://github.com/example/repo/pull/\($0)") } + for query in ["#41", "#42", "example/repo#42", "https://github.com/example/repo/pull/42"] { + #expect(DailyUXSidebarIndex.matchingThreads([item], snapshot: FeatureSnapshot(threads: [item]), query: query).map(\.id) == ["linked"]) + } + #expect(DailyUXSidebarIndex.matchingThreads([item], snapshot: FeatureSnapshot(threads: [item]), query: "#43").isEmpty) + } + private let now = Date(timeIntervalSince1970: 2_000_000) @Test func manualOrderKeepsNewAndReopenedThreadsFirst() { diff --git a/apps/swift-ios/Tests/FeatureTests/LinkedPullRequestSettlementTests.swift b/apps/swift-ios/Tests/FeatureTests/LinkedPullRequestSettlementTests.swift new file mode 100644 index 000000000..4bf51cc00 --- /dev/null +++ b/apps/swift-ios/Tests/FeatureTests/LinkedPullRequestSettlementTests.swift @@ -0,0 +1,36 @@ +import Foundation +import Testing +@testable import T3Code + +@Suite("Linked pull request settlement") +struct LinkedPullRequestSettlementTests { + private func pr(_ number: Int, _ state: String) -> FeaturePullRequest { + FeaturePullRequest(number: number, title: "PR", state: state, updatedAt: Date(timeIntervalSince1970: Double(number))) + } + + @Test func anOpenLinkKeepsTheCollectionOpen() { + #expect(FeatureLinkedPullRequestSettlement.aggregate([pr(1, "merged"), pr(2, "open")])?.state == "open") + } + + @Test func failedReadsCannotSettleACollection() { + #expect(FeatureLinkedPullRequestSettlement.aggregate([pr(1, "merged"), nil])?.state == "unknown") + #expect(FeatureLinkedPullRequestSettlement.aggregate([nil, nil]) == nil) + #expect(FeatureLinkedPullRequestSettlement.aggregate([pr(1, "closed"), pr(2, "unknown")])?.state == "unknown") + } + + @Test func allTerminalLinksUseTheLatestTimestampAndRespectMergePreference() { + let result = FeatureLinkedPullRequestSettlement.aggregate([pr(1, "merged"), pr(2, "closed")]) + #expect(result?.state == "merged") + #expect(result?.updatedAt == Date(timeIntervalSince1970: 2)) + #expect(FeatureLinkedPullRequestSettlement.aggregate([pr(1, "closed"), pr(2, "closed")])?.state == "closed") + } + + @Test func stackActionsUseExactlyTheReviewedAffectedLayers() throws { + let fixture = URL(fileURLWithPath: #filePath).deletingLastPathComponent().deletingLastPathComponent() + .appendingPathComponent("CoreTests/Fixtures/pullRequestStack.json") + let stack = try JSONDecoder().decode(PullRequestStack.self, from: Data(contentsOf: fixture)) + #expect(stack.affectedLayers(number: 2, action: "merge").map(\.number) == [2]) + #expect(stack.affectedLayers(number: 3, action: "update-branch").map(\.number) == [2, 3]) + #expect(stack.affectedLayers(number: 99, action: "merge").isEmpty) + } +} diff --git a/apps/web/src/components/CommandPalette.logic.ts b/apps/web/src/components/CommandPalette.logic.ts index 49a6c322e..d75785002 100644 --- a/apps/web/src/components/CommandPalette.logic.ts +++ b/apps/web/src/components/CommandPalette.logic.ts @@ -182,6 +182,7 @@ export type BuildThreadActionItemsThread = Pick< updatedAt: string; latestUserMessageAt?: string | null; linkedPullRequest?: SidebarThreadSummary["linkedPullRequest"]; + linkedPullRequests?: SidebarThreadSummary["linkedPullRequests"]; }; export function buildThreadActionItems(input: { diff --git a/docs/internals/thread-pull-requests.md b/docs/internals/thread-pull-requests.md new file mode 100644 index 000000000..b0fbfca9d --- /dev/null +++ b/docs/internals/thread-pull-requests.md @@ -0,0 +1,28 @@ +# V2 thread pull requests + +The V2 JSON thread projection optionally carries `linkedPullRequests`. An absent +collection falls back to `linkedPullRequest`; an empty collection is explicit. +The primary field mirrors the first collection entry for older clients. + +`thread.metadata.update` accepts one of `linkPullRequest`, `unlinkPullRequest`, +or the legacy `linkedPullRequest` edit. The orchestrator applies the edit to the +latest projection under its existing dispatch serialization. Links deduplicate +by URL host, repository and number, with at most 50 entries. A legacy replacement +or removal changes only its primary entry, retaining additional links. + +Swift gates collection editing on `threadPullRequestsV2` and polls every link for +visible tasks. Its aggregate remains nonterminal when any request is open or a +read fails. Changing the collection restarts subscriptions and drops the prior +aggregate seed. Web/Expo continue showing the primary; their shared settlement +helper declines primary-only automatic settlement for a collection. + +`pullRequests.stack` reads a GitHub stack on demand. Other providers return null. +`pullRequestStackActions` gates the native UI. `pullRequests.runAction` accepts +`stackNumber` and `expectedStackHeads`; the GitHub boundary re-reads membership, +checks revisions and branch permissions, and performs remote-only mutations. +All reviewed references are invalidated even on partial failure. Native action +confirmation holds an immutable reviewed stack and refreshes after dismissal. + +This does not implement upstream's automatic PR discovery/linking, stack link +tombstones, MCP linking tools, or restart-persistent summary cache. It adds no +SQLite migration or V1 thread-runtime dependency. diff --git a/docs/user/chat-formatting.md b/docs/user/chat-formatting.md index 098188a89..c9bf94703 100644 --- a/docs/user/chat-formatting.md +++ b/docs/user/chat-formatting.md @@ -30,3 +30,10 @@ open local files. A preview is separate from a saved file, and showing one does not mean the assistant has tested the saved result. These instructions guide future answers; they do not rewrite earlier messages. + +## Inspecting images on iOS + +Open an image to view it full screen. Pinch to zoom, drag to pan while zoomed, +or double-tap to toggle zoom. Fit image restores the full image. At its fitted +size, swipe between images in the message's gallery. VoiceOver offers zoom +adjustments and a Fit image action. Saving or sharing keeps the original image. diff --git a/docs/user/linked-pull-requests.md b/docs/user/linked-pull-requests.md new file mode 100644 index 000000000..a40c4117f --- /dev/null +++ b/docs/user/linked-pull-requests.md @@ -0,0 +1,21 @@ +# Linked pull requests on iOS + +Open a task's Details, then Version Control → Linked pull requests. Add a PR number +or paste its URL to link a request from the task's repository. Add more requests +in the same sheet, tap one to read it, or unlink a request without closing it on +the host. Search tasks by PR number (`#42`), repository, or PR URL. + +A task with multiple links stays active while any linked request is open or its +status is unavailable. Automatic settlement still respects your settle-on-merge +preference. Older servers offer one linked request instead. + +For a GitHub PR that belongs to a stack, its detail screen lists the layers from +base to top. Tap a layer to read it. If your account has permission, review a +merge through the selected layer or a rebase from the top layer. The confirmation +lists the affected revisions before you submit. These operations update GitHub; +they do not switch or rewrite your local checkout. + +If a stack changed since you reviewed it, refresh before retrying. A rebase can +stop after updating earlier layers; those completed updates remain on GitHub. +If GitHub reports that a merge is still running, check its status before submitting +another request. Stack controls require a server and host that support them. diff --git a/packages/client-runtime/src/state/models.ts b/packages/client-runtime/src/state/models.ts index 9caed8567..61c8819ba 100644 --- a/packages/client-runtime/src/state/models.ts +++ b/packages/client-runtime/src/state/models.ts @@ -80,6 +80,7 @@ export interface EnvironmentThreadShell { readonly worktreePath: string | null; /** Pull request a user pinned to this thread; null when nothing is linked. */ readonly linkedPullRequest: ThreadLinkedPullRequest | null; + readonly linkedPullRequests?: readonly ThreadLinkedPullRequest[] | undefined; readonly lineage: OrchestrationV2ThreadShell["lineage"]; readonly forkedFrom: OrchestrationV2ThreadShell["forkedFrom"]; readonly activeProviderThreadId: OrchestrationV2ThreadShell["activeProviderThreadId"]; @@ -231,6 +232,9 @@ export function presentThreadShell( branch: thread.branch, worktreePath: thread.worktreePath, linkedPullRequest: thread.linkedPullRequest ?? null, + ...(thread.linkedPullRequests === undefined + ? {} + : { linkedPullRequests: thread.linkedPullRequests }), lineage: thread.lineage, forkedFrom: thread.forkedFrom, activeProviderThreadId: thread.activeProviderThreadId, diff --git a/packages/client-runtime/src/state/threadSettled.test.ts b/packages/client-runtime/src/state/threadSettled.test.ts index 7ee7ba1ca..b06b82b3f 100644 --- a/packages/client-runtime/src/state/threadSettled.test.ts +++ b/packages/client-runtime/src/state/threadSettled.test.ts @@ -17,6 +17,11 @@ const FRESH = "2026-04-09T00:00:00.000Z"; const STALE = "2026-04-06T23:59:59.999Z"; describe("changeRequestAutoSettles", () => { + it("does not settle a collection from a legacy primary-only status read", () => { + expect( + changeRequestAutoSettles({ state: "merged" }, { thread: { linkedPullRequests: [{}, {}] } }), + ).toBe(false); + }); it.each([ ["open", true, false], ["merged", true, true], @@ -193,6 +198,16 @@ describe("threadLastActivityAt", () => { }); describe("effectiveSettled", () => { + it("blocks inactivity settlement for collections but honors explicit settlement", () => { + const shell = { ...makeShell({ activityAt: STALE }), linkedPullRequests: [{}, {}] }; + const options = { + now: NOW, + autoSettleAfterDays: 1, + changeRequest: { state: "merged" as const }, + }; + expect(effectiveSettled(shell, options)).toBe(false); + expect(effectiveSettled({ ...shell, settledOverride: "settled" }, options)).toBe(true); + }); const overrideCases = [null, "settled", "active"] as const; const changeRequestStates = [undefined, "open", "merged"] as const; const inactivityCases = [ diff --git a/packages/client-runtime/src/state/threadSettled.ts b/packages/client-runtime/src/state/threadSettled.ts index 06c230486..2cfccaca5 100644 --- a/packages/client-runtime/src/state/threadSettled.ts +++ b/packages/client-runtime/src/state/threadSettled.ts @@ -25,6 +25,7 @@ interface QueuedThreadShell { } interface SettlementThreadShell extends QueuedThreadShell { + readonly linkedPullRequests?: readonly unknown[] | undefined; readonly settledOverride: "settled" | "active" | null; readonly settledAt: string | null; readonly hasPendingApprovals: boolean; @@ -45,6 +46,7 @@ export interface ChangeRequestSettleSource { /** What the settle rules need to know about the thread's own timeline. */ export interface ThreadActivitySource { + readonly linkedPullRequests?: readonly unknown[] | undefined; readonly createdAt?: string | null; readonly latestUserMessageAt?: string | null; readonly latestRun?: SettlementRunLike | null; @@ -90,6 +92,8 @@ export function changeRequestAutoSettles( } = {}, ): boolean { if (changeRequest == null) return false; + // Legacy clients only read the primary PR; they cannot settle an entire collection from it. + if ((options.thread?.linkedPullRequests?.length ?? 0) > 1) return false; const terminal = changeRequest.state === "closed" || (changeRequest.state === "merged" && options.autoSettleOnMerge !== false); @@ -382,6 +386,8 @@ export function effectiveSettled( // "active" is the explicit keep-active pin: it suppresses auto-settle // until real activity clears it server-side. if (shell.settledOverride === "active") return false; + // Primary-only status cannot establish that every linked request has finished. + if ((shell.linkedPullRequests?.length ?? 0) > 1) return false; if ( changeRequestAutoSettles(options.changeRequest, { autoSettleOnMerge: options.autoSettleOnMerge, diff --git a/packages/contracts/src/environment.ts b/packages/contracts/src/environment.ts index abb8e9be8..abc153b8d 100644 --- a/packages/contracts/src/environment.ts +++ b/packages/contracts/src/environment.ts @@ -90,6 +90,9 @@ export const ExecutionEnvironmentCapabilities = Schema.Struct({ threadTitleRegeneration: Schema.optionalKey(Schema.Boolean), /** Server persists a pull request reference on thread.meta.update. */ threadPullRequestLinking: Schema.optionalKey(Schema.Boolean), + /** V2 atomic metadata link/unlink operations and a linkedPullRequests collection. */ + threadPullRequestsV2: Schema.optionalKey(Schema.Boolean), + pullRequestStackActions: Schema.optionalKey(Schema.Boolean), /** The update path clients should offer for this server. Absent on servers that must be relaunched manually (dev checkouts, Windows foreground runs, pre-update servers). */ diff --git a/packages/contracts/src/orchestrationV2.ts b/packages/contracts/src/orchestrationV2.ts index 709e40c78..a89a207f2 100644 --- a/packages/contracts/src/orchestrationV2.ts +++ b/packages/contracts/src/orchestrationV2.ts @@ -326,6 +326,9 @@ export const OrchestrationV2AppThread = Schema.Struct({ worktreePath: Schema.NullOr(TrimmedNonEmptyString), worktreeStatus: Schema.optional(OrchestrationV2ThreadWorktreeStatus), linkedPullRequest: Schema.optional(Schema.NullOr(ThreadLinkedPullRequest)), + linkedPullRequests: Schema.optional( + Schema.Array(ThreadLinkedPullRequest).check(Schema.isMaxLength(50)), + ), activeProviderThreadId: Schema.NullOr(ProviderThreadId), historyOrigin: Schema.optional(OrchestrationV2ThreadHistoryOrigin), lineage: OrchestrationV2AppThreadLineage, @@ -1638,6 +1641,9 @@ export const OrchestrationV2ThreadShell = Schema.Struct({ worktreePath: Schema.NullOr(TrimmedNonEmptyString), worktreeStatus: Schema.optional(OrchestrationV2ThreadWorktreeStatus), linkedPullRequest: Schema.optional(Schema.NullOr(ThreadLinkedPullRequest)), + linkedPullRequests: Schema.optional( + Schema.Array(ThreadLinkedPullRequest).check(Schema.isMaxLength(50)), + ), lineage: OrchestrationV2AppThreadLineage, forkedFrom: Schema.NullOr(OrchestrationV2AppThread.fields.forkedFrom), activeProviderThreadId: Schema.NullOr(ProviderThreadId), @@ -2453,6 +2459,9 @@ export const OrchestrationV2Command = Schema.Union([ expectedWorktreePath: Schema.optional(Schema.NullOr(TrimmedNonEmptyString)), /** Absent leaves the link alone; null unlinks. */ linkedPullRequest: Schema.optional(Schema.NullOr(ThreadLinkedPullRequest)), + /** Atomic collection edits; older clients keep using the single-link field. */ + linkPullRequest: Schema.optional(ThreadLinkedPullRequest), + unlinkPullRequest: Schema.optional(ThreadLinkedPullRequest), pinned: Schema.optional(Schema.Boolean), /** Fractional key placing this thread within the pinned run. Sent alone to reorder, or alongside `pinned: true` to place a fresh pin. */ diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index 1f30803aa..7c2648508 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -801,7 +801,33 @@ export const PullRequestDiffFileContentsResult = Schema.Struct({ }); export type PullRequestDiffFileContentsResult = typeof PullRequestDiffFileContentsResult.Type; +export const PullRequestStack = Schema.Struct({ + id: TrimmedNonEmptyString, + number: PositiveInt, + url: TrimmedNonEmptyString, + base: TrimmedNonEmptyString, + layers: Schema.Array( + Schema.Struct({ + number: PositiveInt, + title: Schema.optional(Schema.String), + isDraft: Schema.optional(Schema.Boolean), + headSha: Schema.optional(TrimmedNonEmptyString), + headBranch: TrimmedNonEmptyString, + state: PullRequestState, + }), + ), +}); +export type PullRequestStack = typeof PullRequestStack.Type; + +export const PullRequestStackHead = Schema.Struct({ + number: PositiveInt, + headSha: TrimmedNonEmptyString, +}); +export type PullRequestStackHead = typeof PullRequestStackHead.Type; + export const PullRequestActionInput = Schema.Struct({ + stackNumber: Schema.optional(PositiveInt), + expectedStackHeads: Schema.optional(Schema.Array(PullRequestStackHead)), ...PullRequestRef.fields, action: PullRequestAction, /** diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index e02bbacfc..b1cfe3f8e 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -1,3 +1,4 @@ +import { PullRequestStack } from "./pullRequest.ts"; import * as Schema from "effect/Schema"; import * as Rpc from "effect/unstable/rpc/Rpc"; import * as RpcGroup from "effect/unstable/rpc/RpcGroup"; @@ -385,6 +386,7 @@ export const WS_METHODS = { pullRequestsList: "pullRequests.list", pullRequestsListStats: "pullRequests.listStats", pullRequestsDetail: "pullRequests.detail", + pullRequestsStack: "pullRequests.stack", pullRequestsActivity: "pullRequests.activity", pullRequestsThreadComments: "pullRequests.threadComments", pullRequestsDiffFileContents: "pullRequests.diffFileContents", @@ -614,6 +616,12 @@ export const WsPullRequestsListStatsRpc = Rpc.make(WS_METHODS.pullRequestsListSt error: PullRequestRpcError, }); +export const WsPullRequestsStackRpc = Rpc.make(WS_METHODS.pullRequestsStack, { + payload: PullRequestRef, + success: Schema.NullOr(PullRequestStack), + error: PullRequestRpcError, +}); + export const WsPullRequestsDetailRpc = Rpc.make(WS_METHODS.pullRequestsDetail, { payload: PullRequestRef, success: PullRequestDetail, @@ -1340,6 +1348,7 @@ export const WsRpcGroup = RpcGroup.make( WsPullRequestsListRpc, WsPullRequestsListStatsRpc, WsPullRequestsDetailRpc, + WsPullRequestsStackRpc, WsPullRequestsActivityRpc, WsPullRequestsThreadCommentsRpc, WsPullRequestsDiffFileContentsRpc, diff --git a/packages/shared/src/threadPullRequests.test.ts b/packages/shared/src/threadPullRequests.test.ts new file mode 100644 index 000000000..8102106ad --- /dev/null +++ b/packages/shared/src/threadPullRequests.test.ts @@ -0,0 +1,61 @@ +import { ProjectId } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; +import { + linkedPullRequestsOf, + threadPullRequestSearchTerms, + updateLinkedPullRequests, +} from "./threadPullRequests.ts"; +const link = (number: number, host = "github.com") => ({ + projectId: ProjectId.make("p"), + repository: "owner/repo", + number, + url: `https://${host}/owner/repo/pull/${number}`, +}); +describe("V2 linked pull requests", () => { + it("upgrades legacy state and composes successive additions without losing links", () => { + const first = updateLinkedPullRequests( + { linkedPullRequest: link(1) }, + { linkPullRequest: link(2) }, + ); + const second = updateLinkedPullRequests(first, { linkPullRequest: link(3) }); + expect(second.linkedPullRequests.map((x) => x.number)).toEqual([1, 2, 3]); + expect(second.linkedPullRequest).toEqual(link(1)); + }); + it("deduplicates repository identity but keeps hosts distinct", () => { + const initial = updateLinkedPullRequests({}, { linkPullRequest: link(1) }); + const duplicate = updateLinkedPullRequests(initial, { + linkPullRequest: { + ...link(1), + projectId: ProjectId.make("another"), + repository: "OWNER/REPO", + }, + }); + expect(duplicate.linkedPullRequests).toHaveLength(1); + expect( + updateLinkedPullRequests(duplicate, { linkPullRequest: link(1, "github.enterprise") }) + .linkedPullRequests, + ).toHaveLength(2); + }); + it("legacy replacement and unlink preserve additional links", () => { + const initial = { linkedPullRequest: link(1), linkedPullRequests: [link(1), link(2)] }; + expect( + updateLinkedPullRequests(initial, { linkedPullRequest: link(3) }).linkedPullRequests.map( + (x) => x.number, + ), + ).toEqual([3, 2]); + expect( + updateLinkedPullRequests(initial, { linkedPullRequest: null }).linkedPullRequest, + ).toEqual(link(2)); + expect( + updateLinkedPullRequests(initial, { unlinkPullRequest: link(2) }).linkedPullRequests, + ).toEqual([link(1)]); + }); + it("respects an empty collection and searches all links", () => { + expect(linkedPullRequestsOf({ linkedPullRequest: link(1), linkedPullRequests: [] })).toEqual( + [], + ); + expect(threadPullRequestSearchTerms({ linkedPullRequests: [link(1), link(2)] })).toContain( + "owner/repo#2", + ); + }); +}); diff --git a/packages/shared/src/threadPullRequests.ts b/packages/shared/src/threadPullRequests.ts index 9f011fa15..4a69c6bdf 100644 --- a/packages/shared/src/threadPullRequests.ts +++ b/packages/shared/src/threadPullRequests.ts @@ -1,9 +1,66 @@ import type { ThreadLinkedPullRequest } from "@t3tools/contracts"; -/** Search terms from the V2 single-link projection, without another host request. */ -export function threadPullRequestSearchTerms(thread: { +type ThreadLinks = { readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; -}): string[] { - const link = thread.linkedPullRequest; - return link ? [`#${link.number}`, `${link.repository}#${link.number}`, link.url] : []; + readonly linkedPullRequests?: readonly ThreadLinkedPullRequest[] | undefined; +}; + +/** Missing collection denotes an older projection; an empty collection explicitly clears it. */ +export function linkedPullRequestsOf(thread: ThreadLinks): readonly ThreadLinkedPullRequest[] { + return thread.linkedPullRequests ?? (thread.linkedPullRequest ? [thread.linkedPullRequest] : []); +} + +export function linkedPullRequestKey(link: ThreadLinkedPullRequest): string { + let host: string; + try { + host = new URL(link.url).host.toLowerCase(); + } catch { + host = link.projectId; + } + return `${host}/${link.repository.toLowerCase()}#${link.number}`; +} + +/** Updates are applied to the latest durable projection, so concurrent additions compose. */ +export function updateLinkedPullRequests( + thread: ThreadLinks, + command: { + readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; + readonly linkPullRequest?: ThreadLinkedPullRequest | undefined; + readonly unlinkPullRequest?: ThreadLinkedPullRequest | undefined; + }, +) { + let links = [...linkedPullRequestsOf(thread)]; + if (command.linkedPullRequest !== undefined) { + const previous = thread.linkedPullRequest; + if (previous) + links = links.filter((link) => linkedPullRequestKey(link) !== linkedPullRequestKey(previous)); + if (command.linkedPullRequest) { + links = [ + command.linkedPullRequest, + ...links.filter( + (link) => linkedPullRequestKey(link) !== linkedPullRequestKey(command.linkedPullRequest!), + ), + ]; + } + } + if (command.unlinkPullRequest) { + const key = linkedPullRequestKey(command.unlinkPullRequest); + links = links.filter((link) => linkedPullRequestKey(link) !== key); + } + if (command.linkPullRequest) { + const key = linkedPullRequestKey(command.linkPullRequest); + const existing = links.findIndex((link) => linkedPullRequestKey(link) === key); + if (existing < 0) links.push(command.linkPullRequest); + else links[existing] = command.linkPullRequest; + } + return { linkedPullRequest: links[0] ?? null, linkedPullRequests: links }; +} + +/** Searches every explicit link without another host request. */ +export function threadPullRequestSearchTerms(thread: ThreadLinks): string[] { + return linkedPullRequestsOf(thread).flatMap((link) => [ + `#${link.number}`, + `${link.repository}#${link.number}`, + link.url, + ]); } diff --git a/scripts/generate-swift-contract-fixtures.ts b/scripts/generate-swift-contract-fixtures.ts index ba0dc40e9..c210fc7fc 100644 --- a/scripts/generate-swift-contract-fixtures.ts +++ b/scripts/generate-swift-contract-fixtures.ts @@ -16,6 +16,7 @@ */ import { ServerProviderUsageLimits, + PullRequestStack, CheckpointId, CheckpointScopeId, ContextHandoffId, @@ -286,6 +287,18 @@ const projection = { interactionMode: "default" as const, branch: null, worktreePath: null, + linkedPullRequests: [41, 42].map((number) => ({ + projectId, + repository: "example/repo", + number, + url: `https://github.com/example/repo/pull/${number}`, + })), + linkedPullRequest: { + projectId, + repository: "example/repo", + number: 41, + url: "https://github.com/example/repo/pull/41", + }, activeProviderThreadId: providerThreadId, activeOrderKey: "n", lineage: { rootThreadId: threadId, parentThreadId: null, relationshipToParent: null }, @@ -400,3 +413,35 @@ if (process.argv.includes("--check")) { } else { NodeFS.writeFileSync(limitsPath, limitsSerialized); } + +const stackPath = NodePath.join(NodePath.dirname(outputPath), "pullRequestStack.json"); +const stackSerialized = `${JSON.stringify( + Schema.encodeSync(PullRequestStack)({ + id: "stack-1", + number: 1, + url: "https://github.com/o/r/stack/1", + base: "main", + layers: [ + { number: 1, headBranch: "one", state: "merged" }, + { + number: 2, + headBranch: "two", + title: "Second layer", + isDraft: false, + state: "open", + headSha: "abc", + }, + { number: 3, headBranch: "three", state: "open", headSha: "def" }, + ], + }), + null, + 2, +)}\n`; +if (process.argv.includes("--check")) { + if (!NodeFS.existsSync(stackPath) || NodeFS.readFileSync(stackPath, "utf8") !== stackSerialized) { + console.error("[swift-fixtures] pullRequestStack.json is stale; regenerate fixtures."); + process.exit(1); + } +} else { + NodeFS.writeFileSync(stackPath, stackSerialized); +}