From e26af33b228792bb72349ca4e849a96381578a57 Mon Sep 17 00:00:00 2001 From: Bilal Bakr <62337003+Bil0000@users.noreply.github.com> Date: Fri, 18 Sep 2026 01:38:11 +0300 Subject: [PATCH 01/70] fix(web): keep PR panel actions in the current thread (#12320) Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- apps/web/src/components/ChatView.tsx | 27 +- .../PullRequestDetailPanel.test.tsx | 335 ++++++++++++++++++ .../pullRequest/PullRequestDetailPanel.tsx | 11 +- .../PullRequestListEmptyState.test.tsx | 54 --- .../PullRequestsUnavailableState.test.tsx | 73 ---- .../pullRequestDetail.logic.test.ts | 166 ++++++--- .../pullRequest/pullRequestDetail.logic.ts | 62 +++- .../pullRequestLinkContextMenu.test.ts | 13 - .../pullRequestPresentation.test.tsx | 28 -- 9 files changed, 502 insertions(+), 267 deletions(-) create mode 100644 apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx delete mode 100644 apps/web/src/components/pullRequest/PullRequestListEmptyState.test.tsx delete mode 100644 apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx delete mode 100644 apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts delete mode 100644 apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 64f6b0319a1e..267d420c9d44 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -210,7 +210,7 @@ import { selectThreadPreviewMiniPlayer, usePreviewMiniPlayerStore, } from "../previewMiniPlayerStore"; -import { isThreadOwnPullRequest } from "./pullRequest/pullRequestDetail.logic"; +import { pullRequestPanelContext } from "./pullRequest/pullRequestDetail.logic"; import { PullRequestDetailPanel } from "./pullRequest/PullRequestDetailPanel"; import { PullRequestDetailGhost } from "./pullRequest/PullRequestGhosts"; import { PullRequestsUnavailableState } from "./pullRequest/PullRequestsUnavailableState"; @@ -9627,22 +9627,15 @@ export default function ChatView(props: ChatViewProps) { repository: renderedRightPanelSurface.repository, number: renderedRightPanelSurface.number, }} - context={ - isThreadOwnPullRequest( - { - projectId: linkedThreadPullRequest?.projectId ?? null, - repository: linkedThreadPullRequest?.repository ?? null, - number: linkedThreadPullRequest?.number ?? null, - }, - { - projectId: renderedRightPanelSurface.projectId, - repository: renderedRightPanelSurface.repository, - number: renderedRightPanelSurface.number, - }, - ) - ? "thread" - : "page" - } + context={pullRequestPanelContext( + { + projectId: activeThreadMetadata?.projectId ?? null, + pullRequests: activeThreadMetadata?.pullRequests, + linkedPullRequest: activeThreadMetadata?.linkedPullRequest, + branchPullRequest: activeThreadMetadata?.branchPullRequest, + }, + renderedRightPanelSurface, + )} composerDraftTarget={composerDraftTarget} onBack={ activeThreadRef !== null && pullRequestsSurfaceAvailable && visiblePullRequestCount > 1 diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx new file mode 100644 index 000000000000..4bdf6ee7d535 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx @@ -0,0 +1,335 @@ +import { + EnvironmentId, + ProjectId, + ThreadId, + type ScopedThreadRef, + type PullRequestDetailView, + type ThreadPullRequestLink, +} from "@t3tools/contracts"; +import { DEFAULT_CLIENT_SETTINGS } from "@t3tools/contracts/settings"; +import { act, type ReactNode, type ReactElement, type ComponentProps } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; +import { DraftId, useComposerDraftStore } from "~/composerDraftStore"; + +const { newThread, prepareThread, refresh, Wrapper, Trigger } = vi.hoisted(() => ({ + newThread: vi.fn(), + prepareThread: vi.fn(), + refresh: vi.fn(), + Wrapper: ({ children }: { children?: ReactNode }) => children, + Trigger: ({ children, render }: { children?: ReactNode; render?: ReactElement }) => ( + <> + {render} + {children} + + ), +})); +vi.mock("@effect/atom-react", () => ({ useAtomValue: () => [] })); +vi.mock("~/state/server", () => ({ primaryServerKeybindingsAtom: {} })); +vi.mock("~/state/entities", () => ({ useProjects: () => [], useServerConfigs: () => new Map() })); +vi.mock("~/state/environments", () => ({ + useEnvironments: () => ({ environments: [] }), + usePrimaryEnvironmentId: () => EnvironmentId.make("env-1"), +})); +vi.mock("~/hooks/useSettings", () => ({ + useClientSettings: (select: (settings: typeof DEFAULT_CLIENT_SETTINGS) => unknown) => + select(DEFAULT_CLIENT_SETTINGS), +})); +vi.mock("~/hooks/useLiveRefresh", () => ({ useLiveRefresh: () => {} })); +vi.mock("~/hooks/useHandleNewThread", () => ({ useNewThreadHandler: () => newThread })); +vi.mock("~/lib/sourceControlActions", () => ({ + usePreparePullRequestThreadAction: () => ({ run: prepareThread }), +})); +vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() })); +vi.mock("~/state/pullRequests", () => ({ + pullRequestEnvironment: { detail: () => "detail", activity: () => "activity" }, + usePullRequestTurnRefresh: () => 0, + useSharedPullRequestSummary: () => null, +})); +vi.mock("~/state/vcs", () => ({ vcsEnvironment: { listRefs: () => null } })); +vi.mock("~/state/query", () => ({ + useEnvironmentQuery: (query: string) => ({ + data: query === "detail" ? detail : null, + isPending: false, + isSuccess: true, + error: null, + refresh, + }), +})); +vi.mock("~/state/usePullRequestStack", () => ({ + usePullRequestStack: () => ({ + data: null, + isSuccess: true, + isPending: false, + error: null, + refresh, + }), +})); +vi.mock("../ui/toast", () => ({ toastManager: { add: vi.fn(), update: vi.fn() } })); +vi.mock("../ui/tooltip", () => ({ + TooltipProvider: Wrapper, + Tooltip: Wrapper, + TooltipTrigger: Trigger, + TooltipPopup: () => null, +})); +vi.mock("../ui/menu", () => ({ + Menu: Wrapper, + MenuPopup: Wrapper, + MenuTrigger: Trigger, + MenuItem: "button", + MenuRadioGroup: Wrapper, + MenuRadioItem: "button", + MenuSeparator: () => null, + MenuShortcut: () => null, +})); +vi.mock("../ui/alert-dialog", () => ({ + AlertDialog: () => null, + AlertDialogPopup: Wrapper, + AlertDialogHeader: Wrapper, + AlertDialogTitle: Wrapper, + AlertDialogDescription: Wrapper, + AlertDialogFooter: Wrapper, + AlertDialogClose: Wrapper, +})); +vi.mock("./PullRequestMarkdown", () => ({ + PullRequestMarkdownContext: Wrapper, + PullRequestMarkdown: () => null, +})); +vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() })); +vi.mock("./PullRequestThreadLinks", () => ({ PullRequestThreadLinks: () => null })); +vi.mock("./PullRequestSummaryTab", () => ({ + PullRequestSummaryTab: ({ + onFixFinding, + }: ComponentProps) => ( + + ), +})); +vi.mock("./PullRequestCodeTab", () => ({ + default: ({ + onAddToAgentSelection, + }: ComponentProps) => ( + + ), +})); + +import { PullRequestDetailPanel } from "./PullRequestDetailPanel"; +import { pullRequestPanelContext } from "./pullRequestDetail.logic"; + +const detail: PullRequestDetailView = { + provider: "github", + projectId: ProjectId.make("project"), + projectTitle: "Project", + workspaceRoot: "/workspace", + repository: "owner/repo", + number: 1, + title: "Test pull request", + body: "Original description", + url: "https://github.com/owner/repo/pull/1", + author: { login: "author", name: null, avatarUrl: null }, + viewer: "author", + state: "open", + isDraft: false, + mergeability: "conflicting", + additions: 1, + deletions: 0, + changedFiles: 1, + headBranch: "feature", + baseBranch: "main", + createdAt: "2026-09-01T00:00:00Z", + updatedAt: "2026-09-01T00:00:00Z", + mergedAt: null, + closedAt: null, + reviewers: [], + labels: [], + checks: [{ name: "Unit tests", status: "success", description: null, url: null }], + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: [], + mergeCapabilities: { merge: false, squash: false, rebase: false }, + capabilities: { + diff: true, + comment: false, + search: true, + actions: [], + mergeMethods: [], + review: { inlineComment: false, reply: false, resolve: false, verdicts: [] }, + reviewers: { request: false, listCandidates: false }, + edit: { changeRequest: true, comment: false }, + }, + viewerPermissions: { + actions: [], + comment: false, + resolve: false, + verdicts: [], + requestReviewers: false, + }, +}; + +const threadRef: ScopedThreadRef = { + environmentId: EnvironmentId.make("env-1"), + threadId: ThreadId.make("thread-1"), +}; +const draftId = DraftId.make("draft-1"); +const newDraftId = DraftId.make("new-draft"); +let renderer: ReactTestRenderer; + +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("window", { addEventListener: vi.fn(), removeEventListener: vi.fn() }); + useComposerDraftStore.setState({ draftsByThreadKey: {} }); + newThread + .mockReset() + .mockResolvedValue({ draftId: newDraftId, threadId: ThreadId.make("new-thread") }); + prepareThread.mockReset().mockResolvedValue({ + _tag: "Success", + value: { branch: "feature", worktreePath: "/workspace/pr" }, + }); +}); +afterEach(() => { + act(() => renderer?.unmount()); + vi.unstubAllGlobals(); +}); + +async function click(label: string) { + const button = renderer.root + .findAllByType("button") + .find( + (node) => + node.props["aria-label"] === label || + node.findAll((child) => child.children.includes(label)).length > 0, + ); + expect(button, label).toBeDefined(); + await act(async () => + button!.props.onClick({ + nativeEvent: new Event("click"), + preventDefault() {}, + stopPropagation() {}, + }), + ); +} + +const actions = [ + "Resolve conflicts", + "Ask a question", + "Explain this PR", + "Fix findings in this thread", + "Fix check", + "Add to agent", +]; + +// The surface ChatView opens for `detail`, and the thread states it can be opened beside. The +// context prop is derived here the way ChatView derives it, so a wrong answer from the thread's +// link list fails these cases rather than only a hand-picked prop. +const surface = { projectId: detail.projectId, repository: detail.repository, number: 1 }; +const link = (number: number, source: ThreadPullRequestLink["source"]): ThreadPullRequestLink => ({ + host: "github.com", + repository: detail.repository, + number, + url: `https://github.com/${detail.repository}/pull/${number}`, + source, + linkedAt: "2026-09-01T00:00:00Z", + snapshot: null, + stack: null, +}); +const stackThread = { + projectId: detail.projectId, + pullRequests: [link(3, "manual"), link(2, "stack"), link(1, "stack")], + // The server's one-slot field names the top layer; the panel shows the bottom one. + linkedPullRequest: { ...surface, number: 3, url: link(3, "manual").url }, +}; +const unrelatedThread = { + projectId: detail.projectId, + pullRequests: [link(9, "created")], + linkedPullRequest: { ...surface, number: 9, url: link(9, "created").url }, +}; + +describe.each([ + ["own PR, a lower layer of the thread's stack", stackThread, threadRef], + ["another PR beside the current thread", unrelatedThread, threadRef], + ["PR beside an unsent draft", null, draftId], + ["standalone PR page", null, undefined], +] as const)("%s", (_name, thread, target) => { + const context = thread ? pullRequestPanelContext(thread, surface) : "page"; + + function render() { + renderer = create( + ({ + terminalFocus: false, + terminalOpen: false, + previewFocus: false, + previewOpen: false, + })} + />, + ); + } + + it(`${thread === stackThread ? "hides" : "offers"} the checkout`, async () => { + await act(async () => render()); + const checkout = renderer.root + .findAllByType("button") + .filter((node) => node.props["aria-label"] === "Check out"); + expect(checkout).toHaveLength(thread === stackThread ? 0 : 1); + }); + + it.each(actions)("%s writes to the correct composer", async (action) => { + if (target) useComposerDraftStore.getState().setPrompt(target, "Keep my draft"); + await act(async () => render()); + if (action === "Add to agent") await click("Code"); + await click(target ? action : action.replace("in this thread", "in a thread")); + const draft = useComposerDraftStore.getState().getComposerDraft(target ?? newDraftId); + if (action === "Resolve conflicts") expect(draft?.prompt).toContain("resolve every conflict"); + else if (action === "Fix check") expect(draft?.prompt).toContain("Fix the failing check"); + else if (action.startsWith("Fix findings")) + expect(draft?.prompt).toContain("Fix the actionable findings"); + else expect(draft?.reviewComments?.length).toBeGreaterThan(0); + if (action === "Add to agent") { + expect(draft?.prompt).toContain("Fix this line"); + expect(draft?.reviewComments).toEqual( + expect.arrayContaining([expect.objectContaining({ id: "note-1", diff: "+broken()" })]), + ); + } + if (target) { + expect(draft?.prompt).toContain("Keep my draft"); + expect(newThread).not.toHaveBeenCalled(); + expect(prepareThread).not.toHaveBeenCalled(); + } else { + expect(newThread).toHaveBeenCalled(); + } + }); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index b5b5fa2e4a3c..5a2afa9cdaeb 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -138,7 +138,6 @@ import { isStackedPullRequestBase, pullRequestActionMenuHasGroup, pullRequestActionNeedsHostRefresh, - pullRequestComposerTarget, pullRequestCheckoutCommand, pullRequestFindingKey, pullRequestHandoffLabels, @@ -511,10 +510,7 @@ export function PullRequestDetailPanel({ * again is at best a no-op and at worst git refusing a branch two checkouts. */ context?: "page" | "thread"; - /** - * The open thread's composer. Beside the thread whose own pull request this is, hand-offs - * land here instead of opening a new thread — the branch is already under the reader's feet. - */ + /** The open thread's composer. */ composerDraftTarget?: ScopedThreadRef | DraftId; /** * Beside a thread, the way back to that thread's list of pull requests. The tab strip can @@ -1064,10 +1060,7 @@ export function PullRequestDetailPanel({ reviewComments?: ReadonlyArray; }; - // Beside the thread whose own pull request this is, a task belongs in that thread's composer: - // the branch is already checked out under it, so opening a second thread would only scatter - // the work. - const attachTarget = pullRequestComposerTarget(context, composerDraftTarget); + const attachTarget = composerDraftTarget ?? null; const handoffLabels = pullRequestHandoffLabels(attachTarget !== null); const writeTaskToComposer = (target: ScopedThreadRef | DraftId, task: ThreadTask) => { diff --git a/apps/web/src/components/pullRequest/PullRequestListEmptyState.test.tsx b/apps/web/src/components/pullRequest/PullRequestListEmptyState.test.tsx deleted file mode 100644 index cef54e639d29..000000000000 --- a/apps/web/src/components/pullRequest/PullRequestListEmptyState.test.tsx +++ /dev/null @@ -1,54 +0,0 @@ -/** - * Which of the four states wins, and which of them offer to ask the hosts again. The component is - * called as a plain function and its tree read for text: the elements are walked rather than - * invoked, so the button's own hooks never run outside a render. - */ -import { isValidElement, type ReactElement, type ReactNode } from "react"; -import { describe, expect, it } from "vite-plus/test"; - -import { PullRequestListEmptyState } from "./PullRequestListEmptyState"; - -function textOf(node: ReactNode): string { - if (typeof node === "string" || typeof node === "number") return String(node); - if (Array.isArray(node)) return node.map(textOf).join(" "); - if (!isValidElement(node)) return ""; - return textOf((node as ReactElement<{ children?: ReactNode }>).props.children); -} - -const baseProps = { - query: "", - filtered: false, - searching: false, - hasProjects: true, - canLoadMore: false, - loadingMore: false, - refreshing: false, - onClearQuery: () => {}, - onLoadMore: () => {}, - onRefresh: () => {}, -}; - -function render(props: Partial): string { - return textOf(PullRequestListEmptyState({ ...baseProps, ...props })); -} - -describe("PullRequestListEmptyState", () => { - it("asks for a project ahead of anything a search or a filter could say", () => { - const text = render({ hasProjects: false, searching: true, query: "fix", filtered: true }); - expect(text).toContain("No projects in this workspace"); - expect(text).toContain("Add project"); - }); - - it("leaves the retry off the states where asking again could not change the answer", () => { - expect(render({ hasProjects: false })).not.toContain("Check again"); - expect(render({ searching: true, query: "fix" })).not.toContain("Check again"); - }); - - it("offers the retry once the hosts have answered", () => { - expect(render({})).toContain("Check again"); - expect(render({ filtered: true })).toContain("Check again"); - expect(render({ query: "fix" })).toContain("Check again"); - expect(render({ canLoadMore: true })).toContain("Load more pull requests"); - expect(render({ refreshing: true })).toContain("Checking..."); - }); -}); diff --git a/apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx b/apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx deleted file mode 100644 index 12136adbc632..000000000000 --- a/apps/web/src/components/pullRequest/PullRequestsUnavailableState.test.tsx +++ /dev/null @@ -1,73 +0,0 @@ -import { isValidElement, type ReactElement, type ReactNode } from "react"; -import { renderToStaticMarkup } from "react-dom/server"; -import { describe, expect, it } from "vite-plus/test"; - -import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; - -function textOf(node: ReactNode): string { - if (typeof node === "string" || typeof node === "number") return String(node); - if (Array.isArray(node)) return node.map(textOf).join(" "); - if (!isValidElement(node)) return ""; - return textOf((node as ReactElement<{ children?: ReactNode }>).props.children); -} - -describe("PullRequestsUnavailableState", () => { - it("can explain an unsupported environment without offering a futile retry", () => { - const text = textOf( - PullRequestsUnavailableState({ - title: "Pull requests unavailable", - error: "Update this environment's T3 Code server to browse pull requests.", - }), - ); - - expect(text).toContain("Pull requests unavailable"); - expect(text).toContain("Update this environment's T3 Code server"); - expect(text).not.toContain("Retry"); - }); - - it("retains the retry for transient load failures", () => { - const html = renderToStaticMarkup( - {}} - gitHubUrl="https://github.com/pingdotgg/t3code/pull/42" - />, - ); - - expect(html).toContain("Retry"); - expect(html).toContain("Open on GitHub"); - expect(html).toContain('href="https://github.com/pingdotgg/t3code/pull/42"'); - expect(html).toContain('target="_blank"'); - expect(html).toContain('rel="noopener noreferrer"'); - }); - - it("can offer the browser without offering a retry", () => { - const html = renderToStaticMarkup( - , - ); - - expect(html).toContain("Open on GitHub"); - expect(html).not.toContain("Retry"); - }); - - it("can offer a retry without offering GitHub", () => { - const html = renderToStaticMarkup( - {}} />, - ); - - expect(html).toContain("Retry"); - expect(html).not.toContain("Open on GitHub"); - }); - - it("renders no action content without a retry or browser target", () => { - const html = renderToStaticMarkup( - , - ); - - expect(html).not.toContain('data-slot="empty-content"'); - expect(html).not.toContain("href="); - }); -}); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 96133395f048..5e75851f08ad 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -8,6 +8,7 @@ import { type PullRequestDetail, type PullRequestDetailView, type PullRequestReviewThread, + type ThreadPullRequestLink, } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; import { formatInlineContextReference } from "~/lib/composerContextReferences"; @@ -26,7 +27,7 @@ import { stripPullRequestHandoffReferences, isPullRequestVerdictStale, isStackedPullRequestBase, - isThreadOwnPullRequest, + pullRequestPanelContext, latestPullRequestReviewOutcomes, newestPullRequestCommitAt, mergePullRequestThreadComments, @@ -34,9 +35,7 @@ import { pullRequestActionMenuHasGroup, pullRequestActionNeedsHostRefresh, pullRequestCheckoutCommand, - pullRequestComposerTarget, pullRequestFindingKey, - pullRequestHandoffLabels, pullRequestReviewOutcome, readableFailure, readPullRequestDetailSnapshot, @@ -250,33 +249,6 @@ describe("pull request primary control", () => { }); }); -describe("pull request handoff labels", () => { - it("names the open thread when actions write to its composer", () => { - expect(pullRequestHandoffLabels(true)).toEqual({ - fixFinding: "Fix in this thread", - fixCheck: "Fix in this thread", - fixFindings: "Fix findings in this thread", - }); - }); - - it("keeps the standalone pull request page labels", () => { - expect(pullRequestHandoffLabels(false)).toEqual({ - fixFinding: "Fix in a thread", - fixCheck: "Fix", - fixFindings: "Fix findings in a thread", - }); - }); -}); - -describe("pull request composer target", () => { - it("rejects a page composer so agent comments cannot open another thread", () => { - const target = { environmentId: "env-1", threadId: "thread-1" }; - - expect(pullRequestComposerTarget("page", target)).toBeNull(); - expect(pullRequestComposerTarget("thread", target)).toBe(target); - }); -}); - describe("stacked pull request classification", () => { it("requires a known default branch", () => { expect(isStackedPullRequestBase("main", [{ name: "main", isDefault: false }])).toBe(false); @@ -1373,40 +1345,124 @@ describe("how the branch stands against its base", () => { }); }); -describe("whether the panel is showing the thread's own pull request", () => { - const surface = { projectId: "proj-a", repository: "acme/app", number: 7 }; +describe("pull request panel context beside a thread", () => { + // Shapes copied from real threads: a thread that opened a stack holds the top layer as a + // manual link and every lower layer as a "stack" link, with the legacy field pointing at + // whichever one the server chose. Snapshots are null until the sync reactor's first pass. + const link = ( + number: number, + overrides: Partial = {}, + ): ThreadPullRequestLink => ({ + host: "github.com", + repository: "pingdotgg/t3code", + number, + url: `https://github.com/pingdotgg/t3code/pull/${number}`, + source: "manual", + linkedAt: "2026-09-09T00:00:00Z", + snapshot: null, + stack: null, + ...overrides, + }); + const surface = ( + number: number, + overrides: Partial[1]> = {}, + ) => ({ + projectId: "proj-a", + host: "github.com", + repository: "pingdotgg/t3code", + number, + ...overrides, + }); + const stackThread = { + projectId: "proj-a", + pullRequests: [ + link(10856), + link(10832, { source: "stack" }), + link(10677, { source: "stack" }), + link(10854, { source: "stack" }), + link(10855, { source: "stack" }), + ], + linkedPullRequest: { + projectId: "proj-a", + repository: "pingdotgg/t3code", + number: 10856, + url: "https://github.com/pingdotgg/t3code/pull/10856", + }, + }; - it("matches on project, repository and number together", () => { - expect( - isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/app", number: 7 }, surface), - ).toBe(true); + it("treats every layer of the thread's stack as its own, not only the one the legacy field names", () => { + for (const number of [10856, 10832, 10677, 10854, 10855]) { + expect(pullRequestPanelContext(stackThread, surface(number)), `#${number}`).toBe("thread"); + } }); - it("rejects a second checkout of the same repository under another project", () => { - expect( - isThreadOwnPullRequest({ projectId: "proj-b", repository: "acme/app", number: 7 }, surface), - ).toBe(false); + it("does not let the legacy field decide when the thread holds a link list", () => { + // Every prior regression flipped here: a server-side change to which link the legacy field + // resolves to must not turn the thread's own second link into a checkout-able stranger. + const thread = { + projectId: "proj-a", + pullRequests: [link(11101, { source: "created" }), link(11105, { source: "stack" })], + linkedPullRequest: { + projectId: "proj-a", + repository: "pingdotgg/t3code", + number: 11105, + url: "https://github.com/pingdotgg/t3code/pull/11105", + }, + }; + expect(pullRequestPanelContext(thread, surface(11101))).toBe("thread"); + expect(pullRequestPanelContext(thread, surface(11105))).toBe("thread"); + expect(pullRequestPanelContext({ ...thread, linkedPullRequest: null }, surface(11101))).toBe( + "thread", + ); }); - it("rejects another repository or another number", () => { - expect( - isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/web", number: 7 }, surface), - ).toBe(false); - expect( - isThreadOwnPullRequest({ projectId: "proj-a", repository: "acme/app", number: 8 }, surface), - ).toBe(false); + it("is the page for a pull request the thread is not linked to", () => { + expect(pullRequestPanelContext(stackThread, surface(12320))).toBe("page"); + expect(pullRequestPanelContext(stackThread, surface(10856, { repository: "acme/web" }))).toBe( + "page", + ); + }); + + it("is the page under another project's checkout of the same repository", () => { + expect(pullRequestPanelContext(stackThread, surface(10856, { projectId: "proj-b" }))).toBe( + "page", + ); + }); + + it("recognizes an unsynced manual link, and matches host and repository case-insensitively", () => { + const thread = { projectId: "proj-a", pullRequests: [link(7, { host: "GitHub.com" })] }; + expect(pullRequestPanelContext(thread, surface(7, { repository: "PingDotGG/T3Code" }))).toBe( + "thread", + ); + expect(pullRequestPanelContext(thread, surface(7, { host: undefined }))).toBe("thread"); + expect(pullRequestPanelContext(thread, surface(7, { host: "gitlab.com" }))).toBe("page"); + }); + + it("ignores a dismissed stack member the reader chose not to see", () => { + const thread = { + projectId: "proj-a", + pullRequests: [link(1), link(2, { source: "stack-dismissed" })], + }; + expect(pullRequestPanelContext(thread, surface(2))).toBe("page"); }); - it("rejects a thread with no project or no pull request of its own", () => { + it("falls back to the legacy fields only for a thread with no link list", () => { + const legacy = { + projectId: "proj-a", + repository: "pingdotgg/t3code", + number: 3, + url: "https://github.com/pingdotgg/t3code/pull/3", + }; expect( - isThreadOwnPullRequest({ projectId: null, repository: "acme/app", number: 7 }, surface), - ).toBe(false); + pullRequestPanelContext({ projectId: "proj-a", linkedPullRequest: legacy }, surface(3)), + ).toBe("thread"); expect( - isThreadOwnPullRequest( - { projectId: "proj-a", repository: "acme/app", number: null }, - surface, - ), - ).toBe(false); + pullRequestPanelContext({ projectId: "proj-a", branchPullRequest: legacy }, surface(3)), + ).toBe("thread"); + expect(pullRequestPanelContext({ projectId: "proj-a", pullRequests: [] }, surface(3))).toBe( + "page", + ); + expect(pullRequestPanelContext({ projectId: null }, surface(3))).toBe("page"); }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 56e22e643e1e..733e314b770f 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -21,8 +21,14 @@ import { type PullRequestState, type PullRequestUpdateMethod, type SourceControlProviderKind, + type ThreadLinkedPullRequest, + type ThreadPullRequestLink, type VcsRef, } from "@t3tools/contracts"; +import { + threadPullRequestKeysEqual, + visibleThreadPullRequests, +} from "@t3tools/shared/threadPullRequests"; import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext"; import { reviewCommentContextId } from "~/lib/composerContextRecords"; @@ -163,28 +169,55 @@ export function editPullRequestThreadComment< return comments.map((comment) => (comment.id === commentId ? { ...comment, body } : comment)); } +type LegacyLinkedPullRequest = Pick; + /** - * Whether the pull request on a right-panel surface is the thread's own one. Repository and - * number are not enough: one environment can hold two checkouts of the same repository under + * How the detail panel behaves beside a thread: "thread" for a pull request the thread itself + * is linked to (any layer of its stack), "page" for any other one the reader opened there. + * + * Decided from the thread's full link list, never from the single legacy `linkedPullRequest`: + * that field is one server-chosen link out of many, and a thread's own second link or lower + * stack layer would otherwise be handed a checkout button for a branch it already works on. The + * legacy fields only answer for servers that predate link lists. Repository and number are not + * enough either way: one environment can hold two checkouts of the same repository under * different projects, and the other project's checkout is somebody else's branch. */ -export function isThreadOwnPullRequest( +export function pullRequestPanelContext( thread: { readonly projectId: string | null; - readonly repository: string | null; - readonly number: number | null; + readonly pullRequests?: ReadonlyArray | undefined; + readonly linkedPullRequest?: LegacyLinkedPullRequest | null | undefined; + readonly branchPullRequest?: LegacyLinkedPullRequest | null | undefined; }, surface: { readonly projectId: string; + readonly host?: string | undefined; readonly repository: string; readonly number: number; }, -): boolean { - return ( - thread.projectId === surface.projectId && - thread.repository === surface.repository && - thread.number === surface.number - ); +): "page" | "thread" { + if (thread.projectId !== surface.projectId) return "page"; + const links = visibleThreadPullRequests(thread.pullRequests ?? []); + if (links.length > 0) { + const repository = surface.repository.toLowerCase(); + return links.some((link) => + surface.host !== undefined + ? threadPullRequestKeysEqual(link, { + host: surface.host, + repository: surface.repository, + number: surface.number, + }) + : link.number === surface.number && link.repository.toLowerCase() === repository, + ) + ? "thread" + : "page"; + } + const legacy = thread.linkedPullRequest ?? thread.branchPullRequest ?? null; + return legacy !== null && + legacy.repository === surface.repository && + legacy.number === surface.number + ? "thread" + : "page"; } /** Names where a pull-request task will land, without letting each surface guess independently. */ @@ -202,13 +235,6 @@ export function pullRequestHandoffLabels(inThisThread: boolean) { }; } -export function pullRequestComposerTarget( - context: "page" | "thread", - target: T | null | undefined, -): T | null { - return context === "thread" ? (target ?? null) : null; -} - /** Whether the open pull-request action group contains at least one action. */ export function pullRequestActionMenuHasGroup( showsDraftToggle: boolean, diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts deleted file mode 100644 index eb6f47b4c803..000000000000 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { describe, expect, it } from "vite-plus/test"; - -import { openOnHostLabel } from "./pullRequestLinkContextMenu"; - -describe("pull request link context menu", () => { - it("names every host it knows, and says nothing false about one it does not", () => { - expect(openOnHostLabel("github")).toBe("Open on GitHub"); - expect(openOnHostLabel("gitlab")).toBe("Open on GitLab"); - expect(openOnHostLabel("bitbucket")).toBe("Open on Bitbucket"); - expect(openOnHostLabel("azure-devops")).toBe("Open on Azure DevOps"); - expect(openOnHostLabel("something-else")).toBe("Open on host"); - }); -}); diff --git a/apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx b/apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx deleted file mode 100644 index df38d75152cf..000000000000 --- a/apps/web/src/components/pullRequest/pullRequestPresentation.test.tsx +++ /dev/null @@ -1,28 +0,0 @@ -import { act } from "react"; -import { create, type ReactTestRenderer } from "react-test-renderer"; -import { afterEach, expect, it, vi } from "vite-plus/test"; - -import { PullRequestActorAvatar } from "./pullRequestPresentation"; - -let renderer: ReactTestRenderer | undefined; - -afterEach(async () => { - await act(async () => renderer?.unmount()); - vi.unstubAllGlobals(); -}); - -it("falls back to the actor initial when a remote avatar fails", async () => { - vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); - await act(async () => { - renderer = create( - , - ); - }); - - await act(async () => renderer!.root.findByType("img").props.onError()); - - expect(renderer!.root.findAllByType("img")).toHaveLength(0); - expect(renderer!.root.findByType("span").children).toEqual(["O"]); -}); From 4cc984fedc33184c0a0fb67ea17fb02902ae2224 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 16:29:20 -0700 Subject: [PATCH 02/70] fix(web): keep browser pages aligned during panel animations (#12329) --- apps/web/src/browser/BrowserSurfaceSlot.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/web/src/browser/BrowserSurfaceSlot.tsx b/apps/web/src/browser/BrowserSurfaceSlot.tsx index 3de3ed586cb0..b875e4c221ef 100644 --- a/apps/web/src/browser/BrowserSurfaceSlot.tsx +++ b/apps/web/src/browser/BrowserSurfaceSlot.tsx @@ -64,6 +64,10 @@ export function BrowserSurfaceSlot(props: { update(); const observer = new ResizeObserver(update); observer.observe(element); + // Inline panels animate their outer width while keeping the content at + // full width. The slot moves without resizing, so measure on shell resizes too. + const panel = element.closest('[data-preview-panel-mode="inline"]'); + if (panel) observer.observe(panel); window.addEventListener("resize", update); window.addEventListener("scroll", update, true); return () => { From b17cc2ab5d5029f898121798ca19db3288cba616 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 16:41:05 -0700 Subject: [PATCH 03/70] fix(server): bound provider event log records before serialization (#12305) --- .../provider/Layers/EventNdjsonLogger.test.ts | 173 +++++++++++++++++- .../src/provider/Layers/EventNdjsonLogger.ts | 142 +++++++++++++- 2 files changed, 309 insertions(+), 6 deletions(-) diff --git a/apps/server/src/provider/Layers/EventNdjsonLogger.test.ts b/apps/server/src/provider/Layers/EventNdjsonLogger.test.ts index 500f2815c539..e87372e03381 100644 --- a/apps/server/src/provider/Layers/EventNdjsonLogger.test.ts +++ b/apps/server/src/provider/Layers/EventNdjsonLogger.test.ts @@ -21,6 +21,7 @@ import { } from "./EventNdjsonLogger.ts"; const encodeUnknownJson = Schema.encodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); +const decodeUnknownJson = Schema.decodeUnknownSync(Schema.fromJsonString(Schema.Unknown)); function ownedLogPath(basePath: string, segment: string): string { const basename = NodePath.basename(basePath); @@ -49,7 +50,7 @@ function parseLogLine(line: string) { } describe("EventNdjsonLogger", () => { - it.effect("logs bounded diagnostics when an event cannot be serialized", () => { + it.effect("summarizes circular events without exposing their contents in diagnostics", () => { const messages: Array = []; const logCapture = Logger.make(({ message }) => { if (Array.isArray(message)) { @@ -71,10 +72,14 @@ describe("EventNdjsonLogger", () => { assert.exists(logger); if (!logger) return; yield* logger.write(circular, ThreadId.make("thread-1")); + yield* logger.close(); const serialized = encodeUnknownJson(messages); assert.notInclude(serialized, secret); - assert.include(serialized, '"errorTag":"SchemaError"'); + const line = parseLogLine( + NodeFS.readFileSync(ownedLogPath(basePath, "thread-1"), "utf8").trim(), + ); + assert.equal(line.payload, '{"truncated":true}'); } finally { NodeFS.rmSync(tempDir, { recursive: true, force: true }); } @@ -314,6 +319,22 @@ describe("EventNdjsonLogger", () => { { method: "thread/realtime/transcript/delta", payload: circularDelta }, threadId, ); + yield* native.write({ method: "turn/diff/updated", payload: circularDelta }, threadId); + yield* native.write( + { + event: { + direction: "incoming", + stage: "decoded", + payload: { + method: "turn/diff/updated", + get params() { + throw new Error("unused diff snapshots must not be traversed"); + }, + }, + }, + }, + threadId, + ); yield* native.write( { event: { @@ -332,6 +353,41 @@ describe("EventNdjsonLogger", () => { }, threadId, ); + yield* native.write( + { + event: { + direction: "incoming", + stage: "decoded", + payload: { method: "item/commandExecution/outputDelta", params: circularDelta }, + }, + }, + threadId, + ); + yield* native.write( + { + event: { + direction: "incoming", + stage: "decoded", + payload: { + type: "stream_event", + event: { type: "content_block_delta", delta: circularDelta }, + }, + }, + }, + threadId, + ); + yield* native.write( + { + event: { + direction: "incoming", + stage: "raw", + get payload() { + throw new Error("raw frames must not be inspected"); + }, + }, + }, + threadId, + ); yield* native.write( { event: { @@ -375,6 +431,119 @@ describe("EventNdjsonLogger", () => { }), ); + it.effect("summarizes large histories without reading their items", () => + Effect.gen(function* () { + const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-provider-log-")); + const basePath = NodePath.join(tempDir, "events.log"); + const turns = Array.from({ length: 10_000 }); + Object.defineProperty(turns, 0, { + get: () => { + throw new Error("history must not be serialized"); + }, + }); + + try { + const store = yield* makeEventNdjsonLogStore(basePath); + yield* store.logger("native").write( + { + provider: "codex", + event: { + direction: "incoming", + stage: "decoded", + payload: { id: 42, result: { thread: { id: "native-thread", turns } } }, + }, + }, + ThreadId.make("large-history"), + ); + yield* store.close(); + + const contents = NodeFS.readFileSync(ownedLogPath(basePath, "large-history"), "utf8"); + assert.isBelow(Buffer.byteLength(contents), 2_048); + const record = decodeUnknownJson(parseLogLine(contents.trim()).payload); + assert.nestedPropertyVal(record, "event.payload.id", 42); + assert.nestedPropertyVal(record, "event.payload.result.thread.id", "native-thread"); + assert.nestedPropertyVal(record, "event.payload.result.thread.turns.itemCount", 10_000); + } finally { + NodeFS.rmSync(tempDir, { recursive: true, force: true }); + } + }), + ); + + it.effect("bounds oversized records while retaining failure details", () => + Effect.gen(function* () { + const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-provider-log-")); + const basePath = NodePath.join(tempDir, "events.log"); + + try { + const store = yield* makeEventNdjsonLogStore(basePath); + const logger = store.logger("native"); + const threadId = ThreadId.make("large-error"); + const failure = { + method: "error", + params: { + threadId: "native-thread", + turnId: "native-turn", + error: { message: "The provider is unavailable.", code: "overloaded" }, + output: "x".repeat(128 * 1_024), + }, + }; + yield* logger.write(failure, threadId); + yield* logger.write({ id: "escaped", output: "\u0000".repeat(20_000) }, threadId); + yield* store.close(); + + const contents = NodeFS.readFileSync(ownedLogPath(basePath, "large-error"), "utf8"); + const records = contents + .trim() + .split("\n") + .map((line) => decodeUnknownJson(parseLogLine(line).payload)); + assert.isBelow(Buffer.byteLength(contents), 64 * 1_024); + assert.equal(records.length, 2); + assert.nestedPropertyVal(records[0], "params.threadId", "native-thread"); + assert.nestedPropertyVal(records[0], "params.turnId", "native-turn"); + assert.nestedPropertyVal( + records[0], + "params.error.message", + "The provider is unavailable.", + ); + assert.nestedPropertyVal(records[0], "params.error.code", "overloaded"); + assert.propertyVal(records[1], "id", "escaped"); + } finally { + NodeFS.rmSync(tempDir, { recursive: true, force: true }); + } + }), + ); + + it.effect("bounds canonical diff snapshots before serializing their duplicate payloads", () => + Effect.gen(function* () { + const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-provider-log-")); + const basePath = NodePath.join(tempDir, "events.log"); + const threadId = ThreadId.make("large-diff"); + const diff = "diff-payload".repeat(128 * 1_024); + try { + const store = yield* makeEventNdjsonLogStore(basePath, { batchWindowMs: 0 }); + yield* store.logger("canonical").write( + { + type: "turn.diff.updated", + threadId, + turnId: "native-turn", + raw: { method: "turn/diff/updated", payload: { diff } }, + payload: { unifiedDiff: diff }, + }, + threadId, + ); + yield* store.close(); + const contents = NodeFS.readFileSync(ownedLogPath(basePath, "large-diff"), "utf8"); + assert.isBelow(Buffer.byteLength(contents), 2_048); + const record = decodeUnknownJson(parseLogLine(contents.trim()).payload); + assert.propertyVal(record, "type", "turn.diff.updated"); + assert.propertyVal(record, "threadId", threadId); + assert.propertyVal(record, "turnId", "native-turn"); + } finally { + NodeFS.rmSync(tempDir, { recursive: true, force: true }); + } + }), + ); + it.effect("keeps OpenCode tool input, final output, and errors in native logs", () => Effect.gen(function* () { const tempDir = NodeFS.mkdtempSync(NodePath.join(NodeOS.tmpdir(), "t3-provider-log-")); diff --git a/apps/server/src/provider/Layers/EventNdjsonLogger.ts b/apps/server/src/provider/Layers/EventNdjsonLogger.ts index de297ee020ea..dda83fe4a007 100644 --- a/apps/server/src/provider/Layers/EventNdjsonLogger.ts +++ b/apps/server/src/provider/Layers/EventNdjsonLogger.ts @@ -32,6 +32,9 @@ const DEFAULT_MAX_AGE_MS = 14 * DAY_MS; const DEFAULT_RETENTION_CHECK_INTERVAL_MS = 5 * 60 * 1_000; const DEFAULT_MAX_BUFFERED_BYTES = MEBIBYTE; const DEFAULT_MAX_BUFFERED_RECORDS = 512; +const MAX_RECORD_CHARACTERS = 64 * 1024; +const MAX_RECORD_FIELDS = 1_024; +const MAX_RECORD_DEPTH = 16; const GLOBAL_THREAD_SEGMENT = "_global"; const LOG_SCOPE = "provider-observability"; const encodeUnknownJsonString = Schema.encodeUnknownEffect(Schema.fromJsonString(Schema.Unknown)); @@ -54,6 +57,7 @@ const transientNativeMethods = new Set([ "item/reasoning/textDelta", "thread/realtime/outputAudio/delta", "thread/realtime/transcript/delta", + "turn/diff/updated", ]); const transientAcpUpdates = new Set(["agent_message_chunk", "agent_thought_chunk"]); @@ -188,7 +192,7 @@ function providerLogPath(directory: string, prefix: string, threadSegment: strin return NodePath.join(directory, `${prefix}${threadSegment}.log`); } -function shouldPersist(stream: EventNdjsonStream, event: unknown): boolean { +function shouldPersistProviderEvent(stream: EventNdjsonStream, event: unknown): boolean { if (stream === "orchestration" || typeof event !== "object" || event === null) { return true; } @@ -200,7 +204,17 @@ function shouldPersist(stream: EventNdjsonStream, event: unknown): boolean { if (stream !== "native") return true; const nested = Reflect.get(event, "event"); - const nativeEvent = typeof nested === "object" && nested !== null ? nested : event; + const envelope = typeof nested === "object" && nested !== null ? nested : event; + // Decoded frames carry the same information as raw frames without another + // copy of every token delta. Decode failures have their own diagnostic frame. + if (Reflect.get(envelope, "stage") === "raw") return false; + const decodedPayload = Reflect.get(envelope, "payload"); + const nativeEvent = + Reflect.get(envelope, "stage") === "decoded" && + typeof decodedPayload === "object" && + decodedPayload !== null + ? decodedPayload + : envelope; const method = Reflect.get(nativeEvent, "method"); if ( typeof method === "string" && @@ -212,6 +226,16 @@ function shouldPersist(stream: EventNdjsonStream, event: unknown): boolean { const nativeType = Reflect.get(nativeEvent, "type"); if (nativeType === "message.part.delta") return false; + if (nativeType === "stream_event") { + const streamEvent = Reflect.get(nativeEvent, "event"); + if ( + typeof streamEvent === "object" && + streamEvent !== null && + Reflect.get(streamEvent, "type") === "content_block_delta" + ) { + return false; + } + } const payload = Reflect.get(nativeEvent, "payload"); if (typeof payload !== "object" || payload === null) return true; @@ -246,6 +270,110 @@ function shouldPersist(stream: EventNdjsonStream, event: unknown): boolean { } } +const summaryFields = [ + "provider", + "protocol", + "kind", + "providerSessionId", + "direction", + "stage", + "type", + "subtype", + "method", + "id", + "threadId", + "turnId", + "requestId", + "session_id", + "status", + "is_error", + "api_error_status", + "terminal_reason", + "stop_reason", + "operation", + "code", + "willRetry", + "message", + "event", + "payload", + "params", + "result", + "thread", + "turn", + "error", + "turns", + "items", + "content", +] as const; + +function summarizeProviderEvent(event: unknown): unknown { + let remainingFields = 128; + let remainingCharacters = 8 * 1024; + const summarize = (value: unknown, depth: number): unknown => { + if (typeof value === "string") { + if (value.length > Math.min(1_024, remainingCharacters)) { + return { omittedCharacters: value.length }; + } + remainingCharacters -= value.length; + return value; + } + if (value === null || typeof value === "number" || typeof value === "boolean") return value; + if (typeof value !== "object") return undefined; + if (Array.isArray(value)) return { itemCount: value.length }; + const summary: Record = { truncated: true }; + if (depth >= 6) return summary; + for (const key of summaryFields) { + if (remainingFields <= 0) break; + const nested = Reflect.get(value, key); + if (nested === undefined) continue; + remainingFields -= 1; + summary[key] = summarize(nested, depth + 1); + } + return summary; + }; + try { + return summarize(event, 0); + } catch { + return { truncated: true }; + } +} + +/** Bounds traversal before the logger encodes payloads. */ +function boundProviderEventForLogging(event: unknown): unknown { + let remainingCharacters = MAX_RECORD_CHARACTERS; + let remainingFields = MAX_RECORD_FIELDS; + const ancestors = new WeakSet(); + const fits = (value: unknown, depth: number): boolean => { + if (typeof value === "string") { + remainingCharacters -= value.length; + return remainingCharacters >= 0; + } + if (typeof value !== "object" || value === null) return true; + if (depth > MAX_RECORD_DEPTH || ancestors.has(value)) return false; + if (Array.isArray(value) && value.length > remainingFields) return false; + ancestors.add(value); + for (const key in value) { + if (!Object.hasOwn(value, key)) continue; + remainingFields -= 1; + remainingCharacters -= key.length; + if ( + remainingFields < 0 || + remainingCharacters < 0 || + !fits(Reflect.get(value, key), depth + 1) + ) + return false; + } + ancestors.delete(value); + return true; + }; + try { + if (fits(event, 0)) return event; + } catch { + // A failing accessor must not escape into provider processing. + } + return summarizeProviderEvent(event); +} + export function writeBatchedMessages( sink: Pick, records: ReadonlyArray, @@ -612,9 +740,15 @@ export const makeEventNdjsonLogStore = Effect.fnUntraced(function* ( if (existing) return existing; const write = Effect.fnUntraced(function* (event: unknown, threadId: ThreadId | null) { - if (!shouldPersist(stream, event)) return; - const payload = yield* serializeEvent(event); + if (!shouldPersistProviderEvent(stream, event)) return; + let payload = yield* serializeEvent(boundProviderEventForLogging(event)); if (payload === undefined) return; + // Escaping can expand strings beyond their input size. Keep that bounded + // serialization out of the file too, while retaining routing/error fields. + if (Buffer.byteLength(payload) > MAX_RECORD_CHARACTERS) { + payload = yield* serializeEvent(summarizeProviderEvent(event)); + if (payload === undefined) return; + } const observedAt = yield* DateTime.now.pipe(Effect.map(DateTime.formatIso)); const line = `[${observedAt}] ${resolveStreamLabel(stream)}: ${payload}\n`; From b4620d595554654c259ec7c0afb3d3d5532959a8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 16:41:05 -0700 Subject: [PATCH 04/70] fix(server): reject file rewind in shared workspaces (#12306) Co-authored-by: Claude Fable 5 --- .../Layers/CheckpointReactor.test.ts | 112 ++++++++++++++++-- .../orchestration/Layers/CheckpointReactor.ts | 64 ++++++++++ apps/web/src/components/ChatView.tsx | 25 ++-- docs/user/composer.md | 5 +- 4 files changed, 186 insertions(+), 20 deletions(-) diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 2cc7a4399915..7091055cd2c2 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -297,6 +297,7 @@ describe("CheckpointReactor", () => { readonly threadWorktreePath?: string | null; readonly threadBranch?: string | null; readonly secondThreadSharingWorktree?: boolean; + readonly secondThreadWorktreePath?: (cwd: string) => string; readonly localStatusRefName?: string | null; readonly providerSessionCwd?: string; readonly providerName?: ProviderDriverKind; @@ -436,7 +437,8 @@ describe("CheckpointReactor", () => { interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, runtimeMode: "approval-required", branch: options?.threadBranch ?? null, - worktreePath: options?.threadWorktreePath ?? cwd, + worktreePath: + options?.threadWorktreePath !== undefined ? options.threadWorktreePath : cwd, createdAt, }) .pipe( @@ -455,7 +457,8 @@ describe("CheckpointReactor", () => { interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE, runtimeMode: "approval-required", branch: null, - worktreePath: options?.threadWorktreePath ?? cwd, + worktreePath: + options?.secondThreadWorktreePath?.(cwd) ?? options?.threadWorktreePath ?? cwd, createdAt, }), ) @@ -497,6 +500,81 @@ describe("CheckpointReactor", () => { }; } + effectIt.effect.each([ + "active", + "archived", + "alias", + "nested", + "ancestor", + "project-root", + "conversation", + ] as const)("preserves sibling files when reverting a shared workspace, owner=%s", (owner) => + Effect.gen(function* () { + const harness = yield* Effect.promise(() => + createHarness({ + secondThreadSharingWorktree: true, + ...(owner === "alias" || owner === "nested" || owner === "ancestor" + ? { + secondThreadWorktreePath: (cwd: string) => { + if (owner === "ancestor") return NodePath.dirname(cwd); + if (owner === "nested") { + const nested = NodePath.join(cwd, "nested-owner"); + NodeFS.mkdirSync(nested); + return nested; + } + const alias = `${cwd}-alias`; + NodeFS.symlinkSync(cwd, alias, "junction"); + tempDirs.push(alias); + return alias; + }, + } + : {}), + }), + ); + const createdAt = "2026-01-01T00:00:02.000Z"; + if (owner === "archived") + yield* harness.engine.dispatch({ + type: "thread.archive", + commandId: CommandId.make("cmd-archive-owner"), + threadId: ThreadId.make("thread-2"), + }); + if (owner === "project-root") + yield* harness.engine.dispatch({ + type: "thread.meta.update", + commandId: CommandId.make("cmd-root-owner"), + threadId: ThreadId.make("thread-2"), + worktreePath: null, + }); + const siblingFile = NodePath.join( + harness.cwd, + ...(owner === "nested" ? ["nested-owner"] : []), + "sibling-work.txt", + ); + NodeFS.writeFileSync(siblingFile, "sibling work\n"); + yield* harness.engine.dispatch({ + type: owner === "conversation" ? "thread.conversation.revert" : "thread.checkpoint.revert", + commandId: CommandId.make("cmd-shared-revert"), + threadId: ThreadId.make("thread-1"), + turnCount: 0, + createdAt, + }); + yield* Effect.promise(harness.drain); + expect(NodeFS.readFileSync(siblingFile, "utf8")).toBe("sibling work\n"); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v3\n"); + const model = yield* Effect.promise(harness.readModel); + const failure = model.threads + .find((t) => t.id === "thread-1") + ?.activities.find((a) => a.kind === "checkpoint.revert.failed"); + if (owner === "conversation") expect(failure).toBeUndefined(); + else { + expect(failure?.payload).toMatchObject({ + detail: expect.stringContaining("isolated worktree"), + }); + expect(harness.provider.rollbackConversation).not.toHaveBeenCalled(); + } + }), + ); + effectIt.effect("captures baseline and large turn summaries before completion receipts", () => Effect.gen(function* () { const harness = yield* Effect.promise(() => @@ -1963,7 +2041,7 @@ describe("CheckpointReactor", () => { }); it.each([false, true])( - "reverts without an active session using project cwd fallback: %s", + "restores files only in an isolated worktree without an active session, project cwd=%s", async (useProjectCwd) => { const harness = await createHarness({ hasSession: false, @@ -1995,12 +2073,28 @@ describe("CheckpointReactor", () => { }), ); - await waitForEvent(harness.engine, (event) => event.type === "thread.reverted"); - expect(harness.provider.rollbackConversation).toHaveBeenCalledWith({ - threadId: ThreadId.make("thread-1"), - numTurns: 1, - }); - expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v1\n"); + await harness.drain(); + if (useProjectCwd) { + expect(harness.provider.rollbackConversation).not.toHaveBeenCalled(); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v3\n"); + const model = await harness.readModel(); + expect(model.threads[0]?.activities).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: "checkpoint.revert.failed", + payload: expect.objectContaining({ + detail: expect.stringContaining("isolated worktree"), + }), + }), + ]), + ); + } else { + expect(harness.provider.rollbackConversation).toHaveBeenCalledWith({ + threadId: ThreadId.make("thread-1"), + numTurns: 1, + }); + expect(NodeFS.readFileSync(NodePath.join(harness.cwd, "README.md"), "utf8")).toBe("v1\n"); + } }, ); }); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.ts index d0d867fdd25e..822df29d0f63 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.ts @@ -15,6 +15,8 @@ import * as Crypto from "effect/Crypto"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; import * as Option from "effect/Option"; import type * as PlatformError from "effect/PlatformError"; import * as Stream from "effect/Stream"; @@ -87,6 +89,8 @@ const make = Effect.gen(function* () { const checkpointStore = yield* CheckpointStore.CheckpointStore; const receiptBus = yield* RuntimeReceiptBus; const workspaceEntries = yield* WorkspaceEntries.WorkspaceEntries; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; const vcsStatusBroadcaster = yield* VcsStatusBroadcaster; const pullRequests = yield* PullRequestService.PullRequestService; const startedTurns = new Map(); @@ -687,6 +691,55 @@ const make = Effect.gen(function* () { }); }); + // Checkpoints contain the whole checkout, so restoring a shared cwd can erase a sibling's work. + const isRestoreWorkspaceIsolated = Effect.fn("isRestoreWorkspaceIsolated")(function* ( + thread: { readonly id: ThreadId; readonly worktreePath: string | null }, + cwd: string, + ) { + if (thread.worktreePath === null) return false; + const canonicalCwd = yield* fileSystem.realPath(cwd); + if ((yield* fileSystem.realPath(thread.worktreePath)) !== canonicalCwd) return false; + const active = yield* projectionSnapshotQuery.getShellSnapshot(); + const archived = yield* projectionSnapshotQuery.getArchivedShellSnapshot(); + const projects = [...active.projects, ...archived.projects]; + const paths = new Set(); + for (const other of [...active.threads, ...archived.threads]) { + if (other.id === thread.id) continue; + const candidate = + other.worktreePath ?? + projects.find((project) => project.id === other.projectId)?.workspaceRoot; + if (candidate !== undefined) paths.add(candidate); + } + for (const session of yield* providerService.listSessions()) { + if ( + session.threadId !== thread.id && + session.status !== "closed" && + session.cwd !== undefined + ) + paths.add(session.cwd); + } + for (const candidate of paths) { + const otherCwd = yield* fileSystem + .realPath(candidate) + .pipe( + Effect.catch((error) => + error.reason._tag === "NotFound" ? Effect.succeed(null) : Effect.fail(error), + ), + ); + if (otherCwd === null) continue; + const isWithin = (parent: string, child: string) => { + const relative = path.relative(parent, child); + return ( + relative === "" || + (!path.isAbsolute(relative) && relative !== ".." && !relative.startsWith(`..${path.sep}`)) + ); + }; + // Parent and nested owners can both have files inside the restore target. + if (isWithin(canonicalCwd, otherCwd) || isWithin(otherCwd, canonicalCwd)) return false; + } + return true; + }); + const handleRevertRequested = Effect.fn("handleRevertRequested")(function* ( event: Extract, ) { @@ -742,6 +795,17 @@ const make = Effect.gen(function* () { return; } + if (!(yield* isRestoreWorkspaceIsolated(thread, checkpointCwd))) { + yield* appendRevertFailureActivity({ + threadId: thread.id, + turnCount: event.payload.turnCount, + detail: + "File restore requires an isolated worktree. This workspace may contain changes from another thread. Rewind the conversation without restoring files instead.", + createdAt: now, + }).pipe(Effect.catch(() => Effect.void)); + return; + } + const targetCheckpointRef = event.payload.turnCount === 0 ? checkpointRefForThreadTurn(event.payload.threadId, 0) diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 267d420c9d44..cfde95e49133 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -10393,20 +10393,25 @@ export default function ChatView(props: ChatViewProps) { Rewind chat to before this message. Your prompt and attachments return to the composer. + {activeWorktreePath === null + ? " Files stay as they are because this thread shares the project directory." + : null} }>Cancel - + {activeWorktreePath !== null ? ( + + ) : null} + /> ) : null} ) : ( diff --git a/apps/web/src/components/pullRequest/PullRequestEditButton.tsx b/apps/web/src/components/pullRequest/PullRequestEditButton.tsx new file mode 100644 index 000000000000..b04ec17b8754 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestEditButton.tsx @@ -0,0 +1,27 @@ +import { PencilIcon } from "lucide-react"; +import type { ComponentProps } from "react"; + +import { cn } from "../../lib/utils"; +import { Button } from "../ui/button"; + +/** Edit affordances stay visible on touch devices and reveal on hover or focus. */ +export function PullRequestEditButton({ + className, + ...props +}: Omit, "children" | "render" | "size" | "variant"> & { + "aria-label": string; +}) { + return ( + + ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx index f1e8955c3d54..d49cb52cc3f2 100644 --- a/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx +++ b/apps/web/src/components/pullRequest/PullRequestReviewAnnotation.tsx @@ -14,7 +14,6 @@ import { CircleIcon, HammerIcon, MessageSquareIcon, - PencilIcon, Trash2Icon, } from "lucide-react"; import { useRef, useState } from "react"; @@ -23,6 +22,7 @@ import { formatRelativeTimeLabel } from "~/timestampFormat"; import { cn } from "~/lib/utils"; import { Button } from "../ui/button"; +import { PullRequestEditButton } from "./PullRequestEditButton"; import { Textarea } from "../ui/textarea"; import { isCommentSubmitShortcut } from "../diffs/commentSubmitShortcut"; import { @@ -297,15 +297,10 @@ export function ReviewThreadCard({ environmentId={environmentId} /> {canEditComment(comment) ? ( - + /> ) : null} )} diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index 247f1a18f411..e89b38c28e1e 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -11,7 +11,6 @@ import { ChevronDownIcon, ChevronRightIcon, HammerIcon, - PencilIcon, TagIcon, UsersIcon, } from "lucide-react"; @@ -24,6 +23,7 @@ import { useOpenLink } from "~/browser/useOpenLink"; import { formatRelativeTimeLabel } from "~/timestampFormat"; import { Button } from "../ui/button"; +import { PullRequestEditButton } from "./PullRequestEditButton"; import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -187,15 +187,7 @@ function CommentBody({ threadRef={editing.threadRef} /> {editing.canEdit(comment) ? ( - + editing.onEdit(comment)} /> ) : null} ); @@ -869,15 +861,10 @@ export function PullRequestSummaryTab({ threadRef={threadRef} /> {canEditPullRequestChangeRequest(detail) ? ( - + /> ) : null} )} diff --git a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx index b114250549d2..5d0ba801edb5 100644 --- a/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx @@ -12,7 +12,6 @@ import { FileCode2Icon, GitCommitHorizontalIcon, MessageSquareIcon, - PencilIcon, } from "lucide-react"; import { useState, type ReactNode } from "react"; @@ -23,6 +22,7 @@ import { useAtomCommand } from "~/state/use-atom-command"; import { formatRelativeTimeLabel } from "~/timestampFormat"; import { Button } from "../ui/button"; +import { PullRequestEditButton } from "./PullRequestEditButton"; import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -235,15 +235,11 @@ function ConversationCard({ {editable !== null && !editing ? ( - + /> ) : null} {reactions.canReact || event.reactions.length > 0 ? ( Date: Thu, 17 Sep 2026 20:18:19 -0700 Subject: [PATCH 17/70] fix(mobile): share accessible connection trace controls (#12371) --- .../connection/CloudEnvironmentRows.tsx | 27 ++++------ .../connection/ConnectionEnvironmentRow.tsx | 24 +++------ .../features/connection/ConnectionTraceId.tsx | 51 +++++++++++++++++++ .../EnvironmentConnectionNotice.tsx | 18 +------ 4 files changed, 69 insertions(+), 51 deletions(-) create mode 100644 apps/mobile/src/features/connection/ConnectionTraceId.tsx diff --git a/apps/mobile/src/features/connection/CloudEnvironmentRows.tsx b/apps/mobile/src/features/connection/CloudEnvironmentRows.tsx index dbb1dca068d4..cccc6425ec13 100644 --- a/apps/mobile/src/features/connection/CloudEnvironmentRows.tsx +++ b/apps/mobile/src/features/connection/CloudEnvironmentRows.tsx @@ -1,3 +1,4 @@ +import { ConnectionTraceId } from "./ConnectionTraceId"; import { useAuth } from "@clerk/expo"; import { SymbolView } from "../../components/AppSymbol"; import { @@ -405,23 +406,15 @@ function CloudEnvironmentRowShell(props: { > {statusText} {errorTraceId ? ( - <> - {" Trace ID: "} - { - event.stopPropagation(); - copyTextWithHaptic(errorTraceId, { target: "connection-trace-id" }); - }} - onPress={(event) => { - event.stopPropagation(); - }} - > - {errorTraceId} - - + ) : null} {errorCanExpand ? ( diff --git a/apps/mobile/src/features/connection/ConnectionEnvironmentRow.tsx b/apps/mobile/src/features/connection/ConnectionEnvironmentRow.tsx index a42db147c17d..b4e32a06f55b 100644 --- a/apps/mobile/src/features/connection/ConnectionEnvironmentRow.tsx +++ b/apps/mobile/src/features/connection/ConnectionEnvironmentRow.tsx @@ -1,3 +1,4 @@ +import { ConnectionTraceId } from "./ConnectionTraceId"; import { SymbolView } from "../../components/AppSymbol"; import { connectionStatusText } from "@t3tools/client-runtime/connection"; import type { AtomCommandResult } from "@t3tools/client-runtime/state/runtime"; @@ -15,7 +16,6 @@ import { MaterialButton } from "../../components/MaterialButton"; import { MaterialIconButton } from "../../components/MaterialIconButton"; import { ThemedSwitch } from "../../components/ThemedSwitch"; import { cn } from "../../lib/cn"; -import { copyTextWithHaptic } from "../../lib/copyTextWithHaptic"; import type { ConnectedEnvironmentSummary } from "../../state/remote-runtime-types"; import { serverEnvironment } from "../../state/server"; import { ConnectionStatusDot } from "./ConnectionStatusDot"; @@ -114,23 +114,11 @@ export function ConnectionEnvironmentRow(props: { > {statusLabel} {statusTraceId ? ( - <> - {" Trace ID: "} - { - event.stopPropagation(); - copyTextWithHaptic(statusTraceId, { target: "connection-trace-id" }); - }} - onPress={(event) => { - event.stopPropagation(); - }} - > - {statusTraceId} - - + ) : null} ) : null} diff --git a/apps/mobile/src/features/connection/ConnectionTraceId.tsx b/apps/mobile/src/features/connection/ConnectionTraceId.tsx new file mode 100644 index 000000000000..a92bd109e4c6 --- /dev/null +++ b/apps/mobile/src/features/connection/ConnectionTraceId.tsx @@ -0,0 +1,51 @@ +import { AppText as Text } from "../../components/AppText"; +import { cn } from "../../lib/cn"; +import { copyTextWithHaptic } from "../../lib/copyTextWithHaptic"; + +/** Inline trace control; disclosure rows reserve ordinary taps for their own navigation. */ +export function ConnectionTraceId({ + traceId, + tone = "muted", + activation = "press", +}: { + readonly traceId: string; + readonly tone?: "muted" | "danger"; + readonly activation?: "press" | "longPress"; +}) { + const copy = () => copyTextWithHaptic(traceId, { target: "connection-trace-id" }); + return ( + <> + {" Trace ID: "} + { + event.stopPropagation(); + if (event.nativeEvent.actionName === "activate") copy(); + }} + className={cn( + "underline decoration-dotted", + tone === "danger" ? "text-danger-foreground" : "text-foreground-muted", + )} + onLongPress={ + activation === "longPress" + ? (event) => { + event.stopPropagation(); + copy(); + } + : undefined + } + onPress={(event) => { + event.stopPropagation(); + if (activation === "press") copy(); + }} + > + {traceId} + + + ); +} diff --git a/apps/mobile/src/features/connection/EnvironmentConnectionNotice.tsx b/apps/mobile/src/features/connection/EnvironmentConnectionNotice.tsx index ca799a0c8192..bb829673688c 100644 --- a/apps/mobile/src/features/connection/EnvironmentConnectionNotice.tsx +++ b/apps/mobile/src/features/connection/EnvironmentConnectionNotice.tsx @@ -1,3 +1,4 @@ +import { ConnectionTraceId } from "./ConnectionTraceId"; import { type EnvironmentConnectionPhase, type EnvironmentConnectionPresentation, @@ -6,7 +7,6 @@ import { SymbolView } from "../../components/AppSymbol"; import { ActivityIndicator, Pressable, View } from "react-native"; import { AppText as Text } from "../../components/AppText"; -import { copyTextWithHaptic } from "../../lib/copyTextWithHaptic"; function noticeTitle(phase: EnvironmentConnectionPhase, environmentLabel: string): string { switch (phase) { @@ -81,21 +81,7 @@ export function EnvironmentConnectionNotice(props: { {noticeDetail(props.connection.phase, props.resourceName, props.connection.error)} {props.connection.traceId ? ( - <> - {" Trace ID: "} - - copyTextWithHaptic(props.connection.traceId!, { - target: "connection-trace-id", - }) - } - > - {props.connection.traceId} - - + ) : null} From a1390ac655927cda10cfab3e4678d456db7b54b8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 20:19:07 -0700 Subject: [PATCH 18/70] fix(mobile): share settings control row layout (#12356) --- .../SettingsServerControlsRouteScreen.tsx | 17 +++--- .../components/SettingsControlRow.tsx | 39 ++++++++++++++ .../settings/components/SettingsSwitchRow.tsx | 54 +++++-------------- 3 files changed, 61 insertions(+), 49 deletions(-) create mode 100644 apps/mobile/src/features/settings/components/SettingsControlRow.tsx diff --git a/apps/mobile/src/features/settings/SettingsServerControlsRouteScreen.tsx b/apps/mobile/src/features/settings/SettingsServerControlsRouteScreen.tsx index 6e7cef0de93d..090467a8cd90 100644 --- a/apps/mobile/src/features/settings/SettingsServerControlsRouteScreen.tsx +++ b/apps/mobile/src/features/settings/SettingsServerControlsRouteScreen.tsx @@ -22,6 +22,7 @@ import { SettingsEnvironmentFilterHeader, } from "./components/SettingsEnvironmentFilterHeader"; import { SettingsSection } from "./components/SettingsSection"; +import { SettingsControlRow } from "./components/SettingsControlRow"; import { SettingsSwitchRow } from "./components/SettingsSwitchRow"; import { SettingsProjectOverridesSection } from "./components/SettingsProjectOverridesSection"; import { useSettingsEnvironmentFilter } from "./settings-environment-filter"; @@ -454,21 +455,21 @@ function FanoutSwitchRow(props: { } return ( - - - - {props.label} - {props.subtitle} - + props.onValueChange(true)} > Mixed · Set on - + ); } diff --git a/apps/mobile/src/features/settings/components/SettingsControlRow.tsx b/apps/mobile/src/features/settings/components/SettingsControlRow.tsx new file mode 100644 index 000000000000..6f442af1b2fc --- /dev/null +++ b/apps/mobile/src/features/settings/components/SettingsControlRow.tsx @@ -0,0 +1,39 @@ +import type { ComponentProps, ReactNode } from "react"; +import { Platform, View } from "react-native"; + +import { SymbolView } from "../../../components/AppSymbol"; +import { AppText as Text } from "../../../components/AppText"; +import { cn } from "../../../lib/cn"; + +export function SettingsControlRow(props: { + readonly disabled?: boolean; + readonly icon: ComponentProps["name"]; + readonly label: string; + readonly subtitle?: string; + readonly children: ReactNode; +}) { + return ( + + + + {props.label} + {props.subtitle ? ( + {props.subtitle} + ) : null} + + {props.children} + + ); +} diff --git a/apps/mobile/src/features/settings/components/SettingsSwitchRow.tsx b/apps/mobile/src/features/settings/components/SettingsSwitchRow.tsx index 99bf0114f39f..49fb98e49976 100644 --- a/apps/mobile/src/features/settings/components/SettingsSwitchRow.tsx +++ b/apps/mobile/src/features/settings/components/SettingsSwitchRow.tsx @@ -1,55 +1,27 @@ -import { cn } from "../../../lib/cn"; import type { ComponentProps } from "react"; -import { Platform, View } from "react-native"; -import { SymbolView } from "../../../components/AppSymbol"; -import { AppText as Text } from "../../../components/AppText"; import { ThemedSwitch } from "../../../components/ThemedSwitch"; +import { SettingsControlRow } from "./SettingsControlRow"; -type SymbolName = ComponentProps["name"]; - -export function SettingsSwitchRow(props: { - readonly disabled?: boolean; - readonly icon: SymbolName; - readonly label: string; - readonly subtitle?: string; - readonly value: boolean; - readonly onValueChange: (value: boolean) => void; -}) { +export function SettingsSwitchRow( + props: Omit, "children"> & { + readonly value: boolean; + readonly onValueChange: (value: boolean) => void; + }, +) { return ( - - - - - {props.label} - - {props.subtitle ? ( - {props.subtitle} - ) : null} - - + ); } From 6088c8104cfda0c11eafd1e1f636d47106fee136 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 20:19:16 -0700 Subject: [PATCH 19/70] refactor(web): share diagnostic process actions (#12358) --- .../settings/DiagnosticsSettings.tsx | 51 ++----------------- .../settings/ProcessSignalActions.tsx | 48 +++++++++++++++++ .../settings/ResourceTelemetryDiagnostics.tsx | 20 +------- 3 files changed, 53 insertions(+), 66 deletions(-) create mode 100644 apps/web/src/components/settings/ProcessSignalActions.tsx diff --git a/apps/web/src/components/settings/DiagnosticsSettings.tsx b/apps/web/src/components/settings/DiagnosticsSettings.tsx index ab22c40b5c68..0a987eca8c1d 100644 --- a/apps/web/src/components/settings/DiagnosticsSettings.tsx +++ b/apps/web/src/components/settings/DiagnosticsSettings.tsx @@ -1,3 +1,4 @@ +import { ProcessSignalActions } from "./ProcessSignalActions"; import { RefreshIcon } from "~/components/ui/refresh-icon"; import { AlertTriangleIcon, @@ -309,51 +310,6 @@ function ProcessNameCell({ ); } -function ProcessSignalActions({ - process, - isSignaling, - onSignal, -}: { - process: ServerProcessDiagnosticsEntry; - isSignaling: boolean; - onSignal: (pid: number, signal: ServerProcessSignal) => void; -}) { - return ( -
- - onSignal(process.pid, "SIGINT")} - > - INT - - } - /> - Send SIGINT - - - onSignal(process.pid, "SIGKILL")} - > - KILL - - } - /> - Send SIGKILL - -
- ); -} - function ProcessDiagnosticsTable({ processes, signalingPid, @@ -469,9 +425,8 @@ function ProcessDiagnosticsTable({ onSignal(process.pid, signal)} /> diff --git a/apps/web/src/components/settings/ProcessSignalActions.tsx b/apps/web/src/components/settings/ProcessSignalActions.tsx new file mode 100644 index 000000000000..ab96e019ef34 --- /dev/null +++ b/apps/web/src/components/settings/ProcessSignalActions.tsx @@ -0,0 +1,48 @@ +import type { ServerProcessSignal } from "@t3tools/contracts"; + +import { InlineButton } from "../ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; + +/** Process ownership and confirmation stay with the diagnostics view. */ +export function ProcessSignalActions({ + disabled, + onSignal, +}: { + disabled: boolean; + onSignal: (signal: ServerProcessSignal) => void; +}) { + return ( +
+ + onSignal("SIGINT")} + > + INT + + } + /> + Send SIGINT + + + onSignal("SIGKILL")} + > + KILL + + } + /> + Send SIGKILL + +
+ ); +} diff --git a/apps/web/src/components/settings/ResourceTelemetryDiagnostics.tsx b/apps/web/src/components/settings/ResourceTelemetryDiagnostics.tsx index b52e54bb5f7d..b10ed02c477e 100644 --- a/apps/web/src/components/settings/ResourceTelemetryDiagnostics.tsx +++ b/apps/web/src/components/settings/ResourceTelemetryDiagnostics.tsx @@ -1,3 +1,4 @@ +import { ProcessSignalActions } from "./ProcessSignalActions"; import { RefreshIcon } from "~/components/ui/refresh-icon"; import { ActivityIcon, @@ -535,24 +536,7 @@ function ProcessActions({ } const isSignaling = signalingKeys.has(processIdentityKey(process)); return ( -
- - -
+ onSignal(process, signal)} /> ); } From 14337401bc801d6239dd10e34997b4481081cf1d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 20:19:27 -0700 Subject: [PATCH 20/70] refactor(mobile): share Android toolbar search fields (#12359) --- .../src/components/MaterialSearchField.tsx | 59 +++++++++++++++++++ .../features/files/MaterialFilesHeader.tsx | 52 ++++------------ .../home/MaterialThreadListToolbar.tsx | 55 ++++------------- 3 files changed, 79 insertions(+), 87 deletions(-) create mode 100644 apps/mobile/src/components/MaterialSearchField.tsx diff --git a/apps/mobile/src/components/MaterialSearchField.tsx b/apps/mobile/src/components/MaterialSearchField.tsx new file mode 100644 index 000000000000..5ed3945039d1 --- /dev/null +++ b/apps/mobile/src/components/MaterialSearchField.tsx @@ -0,0 +1,59 @@ +import type { RefObject } from "react"; +import { Pressable, TextInput, View } from "react-native"; + +import { SymbolView } from "./AppSymbol"; + +export function MaterialSearchField({ + inputRef, + accessibilityLabel, + clearAccessibilityLabel, + placeholder, + value, + onChangeText, +}: { + readonly inputRef: RefObject; + readonly accessibilityLabel: string; + readonly clearAccessibilityLabel: string; + readonly placeholder: string; + readonly value: string; + readonly onChangeText: (value: string) => void; +}) { + return ( + + + + {value.length > 0 ? ( + { + onChangeText(""); + inputRef.current?.focus(); + }} + > + + + ) : null} + + ); +} diff --git a/apps/mobile/src/features/files/MaterialFilesHeader.tsx b/apps/mobile/src/features/files/MaterialFilesHeader.tsx index 2ce953569b68..8936ae9b37ed 100644 --- a/apps/mobile/src/features/files/MaterialFilesHeader.tsx +++ b/apps/mobile/src/features/files/MaterialFilesHeader.tsx @@ -1,9 +1,9 @@ import { useCallback, useEffect, useRef, useState, type ReactNode } from "react"; -import { BackHandler, Keyboard, Pressable, TextInput, View } from "react-native"; +import { BackHandler, Keyboard, type TextInput, View } from "react-native"; import { useSafeAreaInsets } from "react-native-safe-area-context"; import { AndroidHeaderIconButton, AndroidScreenHeader } from "../../components/AndroidScreenHeader"; -import { SymbolView } from "../../components/AppSymbol"; +import { MaterialSearchField } from "../../components/MaterialSearchField"; /** Keep Files search in the same header row on compact and expanded layouts. */ export function MaterialFilesHeader(props: { @@ -74,46 +74,14 @@ export function MaterialFilesHeader(props: { icon="arrow.left" onPress={closeSearch} /> - - - - {props.searchQuery.length > 0 ? ( - { - onSearchQueryChange(""); - searchRef.current?.focus(); - }} - > - - - ) : null} - + ) : null} diff --git a/apps/mobile/src/features/home/MaterialThreadListToolbar.tsx b/apps/mobile/src/features/home/MaterialThreadListToolbar.tsx index 397e6984f3af..72671bc046d3 100644 --- a/apps/mobile/src/features/home/MaterialThreadListToolbar.tsx +++ b/apps/mobile/src/features/home/MaterialThreadListToolbar.tsx @@ -1,12 +1,5 @@ import { useCallback, useEffect, useRef, useState, type ComponentProps } from "react"; -import { - BackHandler, - Keyboard, - Pressable, - TextInput, - View, - type LayoutChangeEvent, -} from "react-native"; +import { BackHandler, Keyboard, type TextInput, View, type LayoutChangeEvent } from "react-native"; import { useSafeAreaInsets } from "react-native-safe-area-context"; import type { MenuAction } from "@react-native-menu/menu"; @@ -15,7 +8,7 @@ import { CompactBrandTitle } from "../../components/CompactBrandTitle"; import { MaterialFloatingActionButton } from "../../components/MaterialFloatingActionButton"; import { AndroidAnchoredMenu } from "../../components/AndroidAnchoredMenu"; import { ControlPillMenu } from "../../components/ControlPill"; -import { SymbolView } from "../../components/AppSymbol"; +import { MaterialSearchField } from "../../components/MaterialSearchField"; import { useHardwareKeyboardCommand } from "../keyboard/hardwareKeyboardCommands"; import { WorkspaceConnectionTitle } from "./WorkspaceConnectionTitle"; import { useWorkspaceState } from "../../state/workspace"; @@ -68,42 +61,14 @@ export function MaterialThreadListToolbar(props: { ? "line.3.horizontal.decrease.circle.fill" : "line.3.horizontal.decrease.circle"; const searchField = ( - - - - {props.searchQuery.length > 0 ? ( - { - props.onSearchQueryChange(""); - searchRef.current?.focus(); - }} - > - - - ) : null} - + ); return ( From 4aa841a8d45ccd611a809351db0b693faba6ba13 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 20:19:36 -0700 Subject: [PATCH 21/70] refactor(web): share settings group surfaces (#12360) --- .../settings/FoldedSettingsSection.tsx | 7 ++--- .../settings/ProviderSettingsPanel.tsx | 23 +++++++------- .../src/components/settings/SettingsGroup.tsx | 30 +++++++++++++++++++ .../components/settings/SettingsPanels.tsx | 5 ++-- .../components/settings/settingsLayout.tsx | 13 ++------ 5 files changed, 48 insertions(+), 30 deletions(-) create mode 100644 apps/web/src/components/settings/SettingsGroup.tsx diff --git a/apps/web/src/components/settings/FoldedSettingsSection.tsx b/apps/web/src/components/settings/FoldedSettingsSection.tsx index 109e97b9d8e8..d12af6209235 100644 --- a/apps/web/src/components/settings/FoldedSettingsSection.tsx +++ b/apps/web/src/components/settings/FoldedSettingsSection.tsx @@ -1,3 +1,4 @@ +import { SettingsGroup } from "./SettingsGroup"; import { ChevronRightIcon } from "lucide-react"; import { type ReactNode, useState } from "react"; @@ -35,11 +36,7 @@ export function FoldedSettingsSection({ return (
- + }>
{deviceTabs}
) : null} -
{icon} @@ -214,7 +211,7 @@ function ProviderSettingsPlaceholder({ {children ? {children} : null} -
+ ); } @@ -1044,16 +1041,16 @@ export function EnvironmentProviderSettings({ {readOnly ? ( -
+ -
+ ) : null} -
)}
- + & { + variant?: "grouped" | "plain"; + divided?: boolean; +}) { + return ( +
*+*]:border-t [&>*+*]:border-border/50 [&>[data-slot=settings-row]]:rounded-none", + className, + )} + /> + ); +} diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 4fb847df684b..85e1de588f94 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -1,3 +1,4 @@ +import { SettingsGroup } from "./SettingsGroup"; import { Spinner } from "~/components/ui/spinner"; import { NotificationSettings } from "./NotificationSettings"; import { ArchiveIcon, ArchiveX, CheckIcon, ChevronRightIcon, SettingsIcon } from "lucide-react"; @@ -2047,7 +2048,7 @@ function LegacyFeaturesSection() { -
+ } /> -
+
diff --git a/apps/web/src/components/settings/settingsLayout.tsx b/apps/web/src/components/settings/settingsLayout.tsx index ed2fb21255f1..94938b637db7 100644 --- a/apps/web/src/components/settings/settingsLayout.tsx +++ b/apps/web/src/components/settings/settingsLayout.tsx @@ -1,3 +1,4 @@ +import { SettingsGroup } from "./SettingsGroup"; import { InfoIcon, Undo2Icon } from "lucide-react"; import { DEFAULT_SERVER_SETTINGS, type ServerSettings } from "@t3tools/contracts"; import * as Equal from "effect/Equal"; @@ -215,17 +216,9 @@ export function SettingsSection({
{headerAction}
)} -
*+*]:border-t [&>*+*]:border-border/50 [&>[data-slot=settings-row]]:rounded-none" - : "space-y-1", - )} - > + {children} -
+ ); } From 8eb1f6e74869130b7602e0c6b0ab56b7805c0889 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 17 Sep 2026 20:19:46 -0700 Subject: [PATCH 22/70] refactor(web): reuse inline settings actions (#12362) --- .../src/components/settings/ExpandableText.tsx | 8 ++++---- .../components/settings/SettingInheritance.tsx | 18 ++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/apps/web/src/components/settings/ExpandableText.tsx b/apps/web/src/components/settings/ExpandableText.tsx index fa5fb94aadd1..36c0ce7c9529 100644 --- a/apps/web/src/components/settings/ExpandableText.tsx +++ b/apps/web/src/components/settings/ExpandableText.tsx @@ -1,3 +1,4 @@ +import { InlineButton } from "../ui/button"; import { useId, useState } from "react"; import { cn } from "../../lib/utils"; @@ -33,15 +34,14 @@ export function ExpandableText({ {text} {canExpand ? ( - + ) : null} ); diff --git a/apps/web/src/components/settings/SettingInheritance.tsx b/apps/web/src/components/settings/SettingInheritance.tsx index 4bec97f9132e..7e0ed7edfca4 100644 --- a/apps/web/src/components/settings/SettingInheritance.tsx +++ b/apps/web/src/components/settings/SettingInheritance.tsx @@ -11,7 +11,7 @@ import type { EnvironmentPresentation } from "../../state/environments"; import { EnvironmentMachineIcon } from "../EnvironmentMachineIcon"; import { resolveEnvModeLabel } from "../BranchToolbar.logic"; import { PULL_REQUEST_MERGE_METHOD_LABELS } from "../pullRequest/pullRequestDetail.logic"; -import { Button } from "../ui/button"; +import { Button, InlineButton } from "../ui/button"; import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import type { ProjectOverrideEntry, ScopedSettingsTarget } from "./scopedSettings"; @@ -261,13 +261,12 @@ export function SettingInheritance({
Overridden by {onClearOverrides ? ( - + ) : null}