diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index ff4bb76bf12a..101c11531c37 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -10,7 +10,6 @@ import { resolveEffectiveEnvMode, resolveEnvModeLabel, resolveBranchTriggerLabel, - resolveBranchToolbarPrBranch, resolveBranchToolbarValue, resolveLockedWorkspaceLabel, resolveLocalCheckoutBranchMismatch, @@ -272,35 +271,6 @@ describe("resolveBranchTriggerLabel", () => { }); }); -describe("resolveBranchToolbarPrBranch", () => { - it("uses the explicit thread branch when it matches the displayed branch", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: "feature/current", - resolvedActiveBranch: "feature/current", - }), - ).toBe("feature/current"); - }); - - it("hides PR state while an optimistic branch switch is in flight", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: "feature/current", - resolvedActiveBranch: "feature/next", - }), - ).toBeNull(); - }); - - it("does not infer PR state without an explicit thread branch", () => { - expect( - resolveBranchToolbarPrBranch({ - activeThreadBranch: null, - resolvedActiveBranch: "feature/current", - }), - ).toBeNull(); - }); -}); - describe("resolveLocalCheckoutBranchMismatch", () => { it("detects when a local thread is associated with a different branch than the checkout", () => { expect( diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index 0577f5e8dd1f..f73123325fba 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -211,13 +211,6 @@ export function resolveBranchTriggerLabel(input: { return resolvedActiveBranch; } -export function resolveBranchToolbarPrBranch(input: { - activeThreadBranch: string | null; - resolvedActiveBranch: string | null; -}): string | null { - return input.activeThreadBranch === input.resolvedActiveBranch ? input.activeThreadBranch : null; -} - export function resolveLocalCheckoutBranchMismatch(input: { effectiveEnvMode: EnvMode; activeWorktreePath: string | null; diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index ba5251538685..b699c8857190 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -1,6 +1,3 @@ -import { useSupportsMultiplePullRequests } from "~/hooks/useSupportsMultiplePullRequests"; -import { resolveThreadCurrentPullRequestLink } from "@t3tools/shared/threadPullRequests"; -import { useRightPanelStore } from "../rightPanelStore"; import { RefreshIcon } from "~/components/ui/refresh-icon"; import { scopeProjectRef, scopeThreadRef } from "@t3tools/client-runtime/environment"; import { @@ -29,7 +26,6 @@ import { import { useComposerDraftStore, type DraftId } from "../composerDraftStore"; import { writeTextToClipboard } from "../hooks/useCopyToClipboard"; import { readLocalApi } from "../localApi"; -import { useOpenPrLink } from "../lib/openPullRequestLink"; import { shouldLoadNextBranchPageAfterScroll } from "../state/paginatedBranches"; import { usePaginatedBranches } from "../state/queries"; import { useProject, useThreadShell } from "../state/entities"; @@ -44,7 +40,6 @@ import { useComposerMenuProps } from "./chat/composerEventScope"; import { deriveLocalBranchNameFromRemoteRef, resolveBranchTriggerLabel, - resolveBranchToolbarPrBranch, resolveBranchSelectionTarget, resolveBranchToolbarValue, resolveDraftEnvModeAfterBranchChange, @@ -52,12 +47,6 @@ import { sanitizeNewRefName, shouldIncludeBranchPickerItem, } from "./BranchToolbar.logic"; -import { - ThreadPullRequestBadgeControl, - prStatusIndicator, - resolveThreadPullRequestBadge, - useLinkedThreadPullRequest, -} from "./ThreadStatusIndicators"; import { Button } from "./ui/button"; import { Switch } from "./ui/switch"; import { getVirtualizedScrollFadeClassName } from "./ui/scroll-area"; @@ -640,38 +629,6 @@ export function BranchToolbarBranchSelector({ startFromOrigin, }); - // Branch status is the fallback when this thread has no linked pull requests. - const branchPrBranch = resolveBranchToolbarPrBranch({ - activeThreadBranch, - resolvedActiveBranch, - }); - const branchPr = - branchPrBranch !== null && branchStatusQuery.data?.refName === branchPrBranch - ? (branchStatusQuery.data.pr ?? null) - : null; - const supportsMultiplePullRequests = useSupportsMultiplePullRequests(environmentId); - const linkedStatus = useLinkedThreadPullRequest( - environmentId, - serverThread?.linkedPullRequest, - true, - serverThread?.pullRequests, - serverThread?.branchPullRequest, - ); - const currentLinkedPr = supportsMultiplePullRequests - ? resolveThreadCurrentPullRequestLink(serverThread?.pullRequests ?? []) - : null; - const prBadge = supportsMultiplePullRequests - ? resolveThreadPullRequestBadge(serverThread?.pullRequests) - : null; - const displayedPr = linkedStatus?.pr ?? (currentLinkedPr === null ? branchPr : null); - const displayedPrStatus = prStatusIndicator( - displayedPr, - linkedStatus?.sourceControlProvider ?? branchStatusQuery.data?.sourceControlProvider, - ); - const prNumber = currentLinkedPr?.number ?? displayedPr?.number; - const prUrl = currentLinkedPr?.url ?? displayedPr?.url; - const openPrLink = useOpenPrLink(threadRef); - function renderPickerItem(itemValue: string, index: number) { if (checkoutPullRequestItemValue && itemValue === checkoutPullRequestItemValue) { return ( @@ -773,17 +730,6 @@ export function BranchToolbarBranchSelector({ className={cn("flex min-w-0 items-center gap-1", className)} data-composer-context-control > - useRightPanelStore.getState().open(threadRef, "pull-requests")} - onOpenPullRequest={(event) => { - if (prUrl) openPrLink(event, prUrl); - }} - /> {/* Context menu lives on the wrapper: the disabled Button has pointer-events-none, so the trigger itself never sees right-clicks while refs are loading or a branch action is pending. */} diff --git a/apps/web/src/components/GitActionsControl.logic.test.ts b/apps/web/src/components/GitActionsControl.logic.test.ts index f302e976ca70..53718975e8f7 100644 --- a/apps/web/src/components/GitActionsControl.logic.test.ts +++ b/apps/web/src/components/GitActionsControl.logic.test.ts @@ -33,7 +33,7 @@ function status(overrides: Partial = {}): VcsStatusResult { } describe("when: ref is clean and has an open PR", () => { - it("resolveQuickAction opens the existing PR", () => { + it("has nothing left to do, and says where the pull request is", () => { const quick = resolveQuickAction( status({ pr: { @@ -47,7 +47,12 @@ describe("when: ref is clean and has an open PR", () => { }), false, ); - assert.deepInclude(quick, { kind: "open_pr", label: "View PR", disabled: false }); + assert.deepInclude(quick, { + kind: "show_hint", + label: "Push", + disabled: true, + hint: "Everything is pushed. This ref already has an open pull request.", + }); }); it("buildMenuItems disables commit/push and enables open PR", () => { @@ -81,13 +86,6 @@ describe("when: ref is clean and has an open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "View PR", - disabled: false, - icon: "pr", - kind: "open_pr", - }, ]); }); }); @@ -122,14 +120,6 @@ describe("when: actions are busy", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -202,24 +192,17 @@ describe("when: ref is clean, ahead, and has an open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "View PR", - disabled: false, - icon: "pr", - kind: "open_pr", - }, ]); }); }); describe("when: ref is clean, ahead, and has no open PR", () => { - it("resolveQuickAction pushes and creates a PR", () => { + it("pushes, and leaves opening the pull request to the header's pill", () => { const quick = resolveQuickAction(status({ aheadCount: 2, pr: null }), false); assert.deepInclude(quick, { kind: "run_action", - action: "create_pr", - label: "Push & create PR", + action: "push", + label: "Push", }); }); @@ -242,46 +225,65 @@ describe("when: ref is clean, ahead, and has no open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: false, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); describe("when: source control provider uses merge requests", () => { - it("uses GitLab MR terminology in quick actions and menu items", () => { - const gitlabStatus = status({ - aheadCount: 2, - sourceControlProvider: { - kind: "gitlab", - name: "GitLab", - baseUrl: "https://gitlab.com", - }, - }); + const gitlab = { + kind: "gitlab", + name: "GitLab", + baseUrl: "https://gitlab.com", + } as const; - const quick = resolveQuickAction(gitlabStatus, false); - const items = buildMenuItems(gitlabStatus, false); + it("moves a ref without naming the change request at all", () => { + const quick = resolveQuickAction( + status({ aheadCount: 2, sourceControlProvider: gitlab }), + false, + ); + + assert.deepInclude(quick, { kind: "run_action", action: "push", label: "Push" }); + }); + + it("names the host's own word where it points at the header's pill", () => { + const quick = resolveQuickAction( + status({ aheadOfDefaultCount: 2, sourceControlProvider: gitlab }), + false, + ); assert.deepInclude(quick, { - kind: "run_action", - action: "create_pr", - label: "Push & create MR", + kind: "show_hint", + disabled: true, + hint: "Everything is pushed. Open a merge request from the header's own button.", }); - assert.deepInclude(items[2], { - id: "pr", - label: "Create MR", + }); + + it("names the host's own word when reporting one the ref already has", () => { + const quick = resolveQuickAction( + status({ + sourceControlProvider: gitlab, + pr: { + number: 30, + title: "Open MR", + url: "https://gitlab.com/g/p/-/merge_requests/30", + baseRef: "main", + headRef: "feature/test", + state: "open", + }, + }), + false, + ); + + assert.deepInclude(quick, { + kind: "show_hint", + disabled: true, + hint: "Everything is pushed. This ref already has an open merge request.", }); }); }); describe("when: ref is clean, up to date, and has no open PR", () => { - it("enables create PR when synced with upstream but ahead of default", () => { + it("points at the header's pill when synced with upstream but ahead of default", () => { const syncedFeature = status({ aheadCount: 0, behindCount: 0, @@ -291,14 +293,17 @@ describe("when: ref is clean, up to date, and has no open PR", () => { const quick = resolveQuickAction(syncedFeature, false); assert.deepInclude(quick, { - label: "Create PR", - disabled: false, - kind: "run_action", - action: "create_pr", + label: "Push", + disabled: true, + kind: "show_hint", + hint: "Everything is pushed. Open a pull request from the header's own button.", }); const items = buildMenuItems(syncedFeature, false); - assert.equal(items.find((item) => item.id === "pr")?.disabled, false); + assert.deepEqual( + items.map((item) => item.id), + ["commit", "push"], + ); }); it("resolveQuickAction returns disabled no-action state", () => { @@ -328,14 +333,6 @@ describe("when: ref is clean, up to date, and has no open PR", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -365,14 +362,6 @@ describe("when: ref is behind upstream", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -390,12 +379,12 @@ describe("when: ref has diverged from upstream", () => { }); describe("when: working tree has local changes", () => { - it("resolveQuickAction returns commit, push, and create PR", () => { + it("resolveQuickAction returns commit and push", () => { const quick = resolveQuickAction(status({ hasWorkingTreeChanges: true }), false); assert.deepInclude(quick, { kind: "run_action", - action: "commit_push_pr", - label: "Commit, push & PR", + action: "commit_push", + label: "Commit & push", }); }); @@ -455,14 +444,6 @@ describe("when: working tree has local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); @@ -497,14 +478,6 @@ describe("when: working tree has local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -540,15 +513,15 @@ describe("when: on default ref without open PR", () => { }); describe("when: working tree has local changes and ref is behind upstream", () => { - it("resolveQuickAction still prefers commit, push, and create PR", () => { + it("resolveQuickAction still prefers commit and push", () => { const quick = resolveQuickAction( status({ hasWorkingTreeChanges: true, behindCount: 1 }), false, ); assert.deepInclude(quick, { kind: "run_action", - action: "commit_push_pr", - label: "Commit, push & PR", + action: "commit_push", + label: "Commit & push", }); }); @@ -571,14 +544,6 @@ describe("when: working tree has local changes and ref is behind upstream", () = kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -611,14 +576,6 @@ describe("when: HEAD is detached and there are no local changes", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); @@ -637,7 +594,7 @@ describe("when: ref has no upstream configured", () => { }); }); - it("resolveQuickAction opens PR when clean, no upstream, no local commits are ahead, and PR exists", () => { + it("reports the existing pull request when clean, unpublished, and one already exists", () => { const quick = resolveQuickAction( status({ hasUpstream: false, @@ -652,11 +609,14 @@ describe("when: ref has no upstream configured", () => { }, }), false, + false, + false, ); assert.deepInclude(quick, { - kind: "open_pr", - label: "View PR", - disabled: false, + kind: "show_hint", + label: "Push", + disabled: true, + hint: "Nothing to push. This ref already has an open pull request.", }); }); @@ -703,18 +663,10 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); - it("resolveQuickAction runs push and create PR when no upstream and commits are ahead", () => { + it("resolveQuickAction runs push when no upstream and commits are ahead", () => { const quick = resolveQuickAction( status({ hasUpstream: false, @@ -725,8 +677,8 @@ describe("when: ref has no upstream configured", () => { ); assert.deepInclude(quick, { kind: "run_action", - action: "create_pr", - label: "Push & create PR", + action: "push", + label: "Push", disabled: false, }); }); @@ -768,14 +720,6 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: false, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); @@ -862,14 +806,6 @@ describe("when: ref has no upstream configured", () => { kind: "open_dialog", dialogAction: "push", }, - { - id: "pr", - label: "Create PR", - disabled: true, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]); }); }); diff --git a/apps/web/src/components/GitActionsControl.logic.ts b/apps/web/src/components/GitActionsControl.logic.ts index 96f7af794ace..d4092d910fda 100644 --- a/apps/web/src/components/GitActionsControl.logic.ts +++ b/apps/web/src/components/GitActionsControl.logic.ts @@ -10,23 +10,30 @@ import { type ChangeRequestTerminology, } from "../sourceControlPresentation"; -export type GitActionIconName = "commit" | "push" | "pr"; +export type GitActionIconName = "commit" | "push"; -export type GitDialogAction = "commit" | "push" | "create_pr"; +export type GitDialogAction = "commit" | "push"; export interface GitActionMenuItem { - id: "commit" | "push" | "pr"; + id: "commit" | "push"; label: string; disabled: boolean; icon: GitActionIconName; - kind: "open_dialog" | "open_pr"; + kind: "open_dialog"; dialogAction?: GitDialogAction; } +/** + * Only ever moves the ref along: commit, push, pull, publish. + * + * Viewing and creating a pull request live on the header's own pill. Those two used to surface + * here, which meant one button read "Commit & push" while work was in flight and "View PR" once it + * landed, so the control under the cursor changed meaning as the branch did. + */ export interface GitQuickAction { label: string; disabled: boolean; - kind: "run_action" | "run_pull" | "open_pr" | "open_publish" | "show_hint"; + kind: "run_action" | "run_pull" | "open_publish" | "show_hint"; action?: GitStackedAction; hint?: string; } @@ -97,13 +104,10 @@ export function buildMenuItems( hasPrimaryRemote = true, ): GitActionMenuItem[] { if (!gitStatus) return []; - const terminology = resolveChangeRequestTerminology(gitStatus); const hasBranch = gitStatus.refName !== null; const hasChanges = gitStatus.hasWorkingTreeChanges; - const hasOpenPr = gitStatus.pr?.state === "open"; const isBehind = gitStatus.behindCount > 0; - const hasDefaultBranchDelta = (gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) > 0; const canPushWithoutUpstream = hasPrimaryRemote && !gitStatus.hasUpstream; const canCommit = !isBusy && hasChanges; const canPush = @@ -112,15 +116,6 @@ export function buildMenuItems( !isBehind && gitStatus.aheadCount > 0 && (gitStatus.hasUpstream || canPushWithoutUpstream); - const canCreatePr = - !isBusy && - hasBranch && - !hasChanges && - !hasOpenPr && - hasDefaultBranchDelta && - !isBehind && - (gitStatus.hasUpstream || canPushWithoutUpstream); - const canOpenPr = !isBusy && hasOpenPr; const commitItem: GitActionMenuItem = { id: "commit", @@ -145,22 +140,6 @@ export function buildMenuItems( kind: "open_dialog", dialogAction: "push", }, - hasOpenPr - ? { - id: "pr", - label: `View ${terminology.shortLabel}`, - disabled: !canOpenPr, - icon: "pr", - kind: "open_pr", - } - : { - id: "pr", - label: `Create ${terminology.shortLabel}`, - disabled: !canCreatePr, - icon: "pr", - kind: "open_dialog", - dialogAction: "create_pr", - }, ]; } @@ -205,21 +184,18 @@ export function resolveQuickAction( if (!gitStatus.hasUpstream && !hasPrimaryRemote) { return { label: "Commit", disabled: false, kind: "run_action", action: "commit" }; } - if (hasOpenPr || isDefaultRef) { - return { label: "Commit & push", disabled: false, kind: "run_action", action: "commit_push" }; - } - return { - label: `Commit, push & ${terminology.shortLabel}`, - disabled: false, - kind: "run_action", - action: "commit_push_pr", - }; + return { label: "Commit & push", disabled: false, kind: "run_action", action: "commit_push" }; } if (!gitStatus.hasUpstream) { if (!hasPrimaryRemote) { if (hasOpenPr && !isAhead) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; + return { + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Nothing to push. This ref already has an open ${terminology.singular}.`, + }; } return { label: "Publish repository", @@ -228,9 +204,6 @@ export function resolveQuickAction( }; } if (!isAhead) { - if (hasOpenPr) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; - } return { label: "Push", disabled: true, @@ -238,19 +211,11 @@ export function resolveQuickAction( hint: "No local commits to push.", }; } - if (hasOpenPr || isDefaultRef) { - return { - label: "Push", - disabled: false, - kind: "run_action", - action: isDefaultRef ? "commit_push" : "push", - }; - } return { - label: `Push & create ${terminology.shortLabel}`, + label: "Push", disabled: false, kind: "run_action", - action: "create_pr", + action: isDefaultRef ? "commit_push" : "push", }; } @@ -272,32 +237,35 @@ export function resolveQuickAction( } if (isAhead) { - if (hasOpenPr || isDefaultRef) { - return { - label: "Push", - disabled: false, - kind: "run_action", - action: isDefaultRef ? "commit_push" : "push", - }; - } return { - label: `Push & create ${terminology.shortLabel}`, + label: "Push", disabled: false, kind: "run_action", - action: "create_pr", + action: isDefaultRef ? "commit_push" : "push", }; } - if (hasOpenPr && gitStatus.hasUpstream) { - return { label: `View ${terminology.shortLabel}`, disabled: false, kind: "open_pr" }; + // Nothing left to move. Where a change request could still be opened or read, the hint says so + // rather than leaving "no action needed" over work that is not finished. + // + // An existing one is reported as a fact about the ref, not as a place to click. The header's pill + // shows the thread's own pull request, and a ref can carry one the thread never linked, so + // pointing at the pill here would sometimes name a button that is not on screen. + if (hasOpenPr) { + return { + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Everything is pushed. This ref already has an open ${terminology.singular}.`, + }; } if (hasDefaultBranchDelta && !isDefaultRef) { return { - label: `Create ${terminology.shortLabel}`, - disabled: false, - kind: "run_action", - action: "create_pr", + label: "Push", + disabled: true, + kind: "show_hint", + hint: `Everything is pushed. Open a ${terminology.singular} from the header's own button.`, }; } diff --git a/apps/web/src/components/GitActionsControl.tsx b/apps/web/src/components/GitActionsControl.tsx index 99db055b667b..fa84228f7e1c 100644 --- a/apps/web/src/components/GitActionsControl.tsx +++ b/apps/web/src/components/GitActionsControl.tsx @@ -46,6 +46,7 @@ import { GitLabIcon, ForgejoIcon, } from "~/components/Icons"; +import { onRequestCreatePullRequest } from "~/gitActionsBus"; import { RadioGroup } from "~/components/ui/radio-group"; import { Spinner } from "~/components/ui/spinner"; import { toggleVariants } from "~/components/ui/toggle"; @@ -111,11 +112,6 @@ interface GitActionsControlProps { gitCwd: string | null; activeThreadRef: ScopedThreadRef | null; draftId?: DraftId; - /** - * Opens the thread's own change request beside it. Absent when the thread has no project to - * place it against, in which case it still opens in the browser. - */ - onOpenPullRequest?: ((number: number) => void) | undefined; } interface PendingDefaultBranchAction { @@ -379,7 +375,6 @@ function GitQuickActionIcon({ SourceControlIcon: ReturnType["Icon"]; }) { const iconClassName = "size-3.5"; - if (quickAction.kind === "open_pr") return ; if (quickAction.kind === "open_publish") return ; if (quickAction.kind === "run_pull") return ; if (quickAction.kind === "run_action") { @@ -947,7 +942,6 @@ export default function GitActionsControl({ gitCwd, activeThreadRef, draftId, - onOpenPullRequest, }: GitActionsControlProps) { const updateThreadMetadata = useAtomCommand( threadEnvironment.updateMetadata, @@ -964,7 +958,6 @@ export default function GitActionsControl({ [activeThreadRef], ); const openPrLink = useOpenPrLink(activeThreadRef ?? undefined); - const openLink = useOpenLink(activeThreadRef); const activeDraftThread = useComposerDraftStore((store) => draftId ? store.getDraftSession(draftId) @@ -1193,36 +1186,6 @@ export default function GitActionsControl({ }; }, [activeEnvironmentId, gitCwd, refreshVcsStatus]); - const openExistingPr = useCallback(async () => { - const openPr = gitStatusForActions?.pr?.state === "open" ? gitStatusForActions.pr : null; - // Beside the thread where it was made, the way the browser opens beside it. Checked before - // the shell, which opening in the app does not need. - if (openPr && onOpenPullRequest) { - onOpenPullRequest(openPr.number); - return; - } - const prUrl = openPr?.url ?? null; - if (!prUrl) { - toastManager.add({ - type: "error", - title: "No open pull request found.", - data: threadToastData, - }); - return; - } - void openLink(prUrl).catch((err: unknown) => { - console.error(err); - toastManager.add( - stackedThreadToast({ - type: "error", - title: "Unable to open pull request link", - description: err instanceof Error ? err.message : "An error occurred.", - ...(threadToastData !== undefined ? { data: threadToastData } : {}), - }), - ); - }); - }, [gitStatusForActions, onOpenPullRequest, openLink, threadToastData]); - runGitActionWithToast = useEffectEvent( async ({ action, @@ -1504,11 +1467,14 @@ export default function GitActionsControl({ }); }; + // Creating a change request is offered by the header's pull request pill, but the flow stays + // here with the progress stages, the default-ref confirmation and the toast that reports it. + const createPullRequestFromHeader = useEffectEvent(() => { + void runGitActionWithToast({ action: "create_pr" }); + }); + useEffect(() => onRequestCreatePullRequest(createPullRequestFromHeader), []); + const runQuickAction = () => { - if (quickAction.kind === "open_pr") { - void openExistingPr(); - return; - } if (quickAction.kind === "open_publish") { setIsPublishDialogOpen(true); return; @@ -1569,18 +1535,10 @@ export default function GitActionsControl({ const openDialogForMenuItem = (item: GitActionMenuItem) => { if (item.disabled) return; - if (item.kind === "open_pr") { - void openExistingPr(); - return; - } if (item.dialogAction === "push") { void runGitActionWithToast({ action: "push" }); return; } - if (item.dialogAction === "create_pr") { - void runGitActionWithToast({ action: "create_pr" }); - return; - } setExcludedFiles(new Set()); setIsEditingFiles(false); setIsCommitDialogOpen(true); diff --git a/apps/web/src/components/LegacySidebar.tsx b/apps/web/src/components/LegacySidebar.tsx index 81fd6047f13e..dd165609f7d9 100644 --- a/apps/web/src/components/LegacySidebar.tsx +++ b/apps/web/src/components/LegacySidebar.tsx @@ -22,6 +22,7 @@ import { ThreadWorktreeIndicator, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { EnvironmentMachineIcon } from "./EnvironmentMachineIcon"; import { ProjectFavicon } from "./ProjectFavicon"; import { useAtomValue } from "@effect/atom-react"; @@ -609,6 +610,17 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP threadRef, ], ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + // Without this the row's own menu answers, because the number sits inside the row. + const handlePrContextMenu = useCallback( + (event: React.MouseEvent) => { + openPrContextMenu(event, { + url: prStatus?.url ?? currentLinkedPr?.url, + providerKind: linkedPullRequestStatus?.sourceControlProvider.kind, + }); + }, + [currentLinkedPr, linkedPullRequestStatus, openPrContextMenu, prStatus], + ); const handleRenameInputRef = useCallback( (element: HTMLInputElement | null) => { if (element && renamingInputRef.current !== element) { @@ -733,6 +745,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: SidebarThreadRowP className={`inline-flex items-center justify-center ${prStatus.colorClass} cursor-pointer rounded-sm outline-hidden focus-visible:ring-1 focus-visible:ring-ring`} onPointerDown={(event) => event.stopPropagation()} onClick={handlePrClick} + onContextMenu={handlePrContextMenu} > event.stopPropagation()} onClick={handlePrClick} + onContextMenu={handlePrContextMenu} className="text-muted-foreground" aria-label={`PR #${currentLinkedPr.number}, status pending`} > diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index e5a86ff98a3c..262c6bda22bd 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -203,6 +203,7 @@ import { type TerminalStatusIndicator, useLinkedThreadPullRequest, } from "./ThreadStatusIndicators"; +import { useThreadPullRequestLinkContextMenu } from "./pullRequest/useThreadPullRequestLinkContextMenu"; import { resolveSnoozePresets, snoozeWakeDescription, @@ -1393,6 +1394,16 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { threadRef, ], ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + const handlePrContextMenu = useCallback( + (event: ReactMouseEvent) => { + openPrContextMenu(event, { + url: pr?.url ?? currentLinkedPr?.url, + providerKind: linkedPullRequestStatus?.sourceControlProvider.kind, + }); + }, + [currentLinkedPr, linkedPullRequestStatus, openPrContextMenu, pr], + ); // All sidebar rows share one surface model. Live threads used to look // like elevated cards while settled threads were plain rows, leaving neither @@ -1507,13 +1518,13 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { const prBadge = prBadgeShape?.kind === "stack" || pr || currentLinkedPr ? ( ) : null; const terminalStatusIcon = terminalStatus ? ( diff --git a/apps/web/src/components/ThreadStatusIndicators.tsx b/apps/web/src/components/ThreadStatusIndicators.tsx index 7201779f13f1..6fdf4d4a8da4 100644 --- a/apps/web/src/components/ThreadStatusIndicators.tsx +++ b/apps/web/src/components/ThreadStatusIndicators.tsx @@ -16,7 +16,7 @@ import { } from "@t3tools/shared/threadPullRequests"; import { FolderGit2Icon, GitPullRequestArrowIcon, LayersIcon, TerminalIcon } from "lucide-react"; import { useMemo, type MouseEvent } from "react"; -import { buttonVariants, InlineButton } from "./ui/button"; +import { InlineButton } from "./ui/button"; import { cn } from "../lib/utils"; import { useEnvironment, usePrimaryEnvironmentId } from "../state/environments"; import { EnvironmentMachineIcon } from "./EnvironmentMachineIcon"; @@ -137,23 +137,24 @@ function ThreadPullRequestBadgeIcon({ return ; } -/** The complete linked-PR control shared by the sidebar and composer footer. */ +/** The complete linked-PR control worn by a thread row. */ export function ThreadPullRequestBadgeControl({ - variant, badge, number, url, status, onOpenStack, onOpenPullRequest, + onContextMenuPullRequest, }: { - variant: "underline" | "ghost"; badge: ThreadPullRequestBadge | null; number?: number | undefined; url?: string | undefined; status: PrStatusIndicator | null; onOpenStack: () => void; onOpenPullRequest: (event: MouseEvent) => void; + /** Only the single-pull-request shape carries one; a stack has no one number to act on. */ + onContextMenuPullRequest?: ((event: MouseEvent) => void) | undefined; }) { const isStack = badge?.kind === "stack"; const linkedCount = badge?.kind === "pull-request" && badge.others > 0 ? badge.others + 1 : null; @@ -166,12 +167,8 @@ export function ThreadPullRequestBadgeControl({ : "" }`; const className = cn( - variant === "ghost" - ? buttonVariants({ variant: "ghost", size: "xs" }) - : "inline-flex shrink-0 cursor-pointer items-center gap-0.5 whitespace-nowrap border-b border-transparent hover:border-current focus-visible:outline-2 focus-visible:outline-ring", + "inline-flex shrink-0 cursor-pointer items-center gap-0.5 whitespace-nowrap border-b border-transparent hover:border-current focus-visible:outline-2 focus-visible:outline-ring", "text-xs tabular-nums", - variant === "ghost" && - "font-normal text-xs! active:scale-100 [--control-icon-color:currentColor]", badge !== null && (isStack || linkedCount !== null) ? PR_STATE_COLOR_CLASS[badge.state] : (status?.colorClass ?? "text-muted-foreground"), @@ -206,6 +203,7 @@ export function ThreadPullRequestBadgeControl({ aria-label={label} onPointerDown={(event) => event.stopPropagation()} onClick={onOpenPullRequest} + {...(onContextMenuPullRequest ? { onContextMenu: onContextMenuPullRequest } : {})} /> ) } diff --git a/apps/web/src/components/chat/ChatHeader.tsx b/apps/web/src/components/chat/ChatHeader.tsx index fbebc323a950..24aa2ea725af 100644 --- a/apps/web/src/components/chat/ChatHeader.tsx +++ b/apps/web/src/components/chat/ChatHeader.tsx @@ -23,6 +23,7 @@ import { type MouseEvent as ReactMouseEvent, } from "react"; import GitActionsControl from "../GitActionsControl"; +import { ThreadPullRequestHeaderPill } from "../pullRequest/ThreadPullRequestHeaderPill"; import { isTrailingDoubleClick } from "../Sidebar.logic"; import { type DraftId } from "~/composerDraftStore"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -409,6 +410,13 @@ export const ChatHeader = memo(function ChatHeader({ "[[data-panel-animations=true]_&]:motion-safe:transition-[padding-right] [[data-panel-animations=true]_&]:motion-safe:[transition-duration:var(--panel-animation-duration)] [[data-panel-animations=true]_&]:motion-safe:ease-out", )} > + {activeProjectName && ( + + )} {activeProjectScripts && ( )} diff --git a/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx b/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx new file mode 100644 index 000000000000..d740ab40d893 --- /dev/null +++ b/apps/web/src/components/pullRequest/ThreadPullRequestHeaderPill.tsx @@ -0,0 +1,131 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { resolveThreadCurrentPullRequestLink } from "@t3tools/shared/threadPullRequests"; +import { GitPullRequestArrowIcon } from "lucide-react"; +import { useCallback, useMemo, type MouseEvent as ReactMouseEvent } from "react"; + +import { useSupportsMultiplePullRequests } from "~/hooks/useSupportsMultiplePullRequests"; +import { cn } from "~/lib/utils"; +import { requestCreatePullRequest } from "~/gitActionsBus"; +import { useRightPanelStore } from "~/rightPanelStore"; +import { useThreadShell } from "~/state/entities"; +import { useEnvironmentQuery } from "~/state/query"; +import { vcsEnvironment } from "~/state/vcs"; + +import { + PrStatusTooltipContent, + prStatusIndicator, + useLinkedThreadPullRequest, +} from "../ThreadStatusIndicators"; +import { Button } from "../ui/button"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { resolveThreadPullRequestHeaderPill } from "./threadPullRequestHeaderPill.logic"; +import { useThreadPullRequestLinkContextMenu } from "./useThreadPullRequestLinkContextMenu"; + +/** + * The thread's pull request, beside the git actions in the header. + * + * It opens the linked pull requests panel rather than the pull request itself, because the number + * shown here is also the handle for changing which pull requests the thread carries. The host is + * still one right-click away, along with unlinking. + */ +export function ThreadPullRequestHeaderPill({ + threadRef, + gitCwd, + onOpenPullRequest, +}: { + readonly threadRef: ScopedThreadRef; + readonly gitCwd: string | null; + /** + * Opens one change request beside the thread. Used where the environment has no linked pull + * requests panel to manage, so the click still lands on something rather than an empty surface. + */ + readonly onOpenPullRequest?: ((number: number) => void) | undefined; +}) { + const thread = useThreadShell(threadRef); + const linkedStatus = useLinkedThreadPullRequest( + threadRef.environmentId, + thread?.linkedPullRequest, + true, + thread?.pullRequests, + thread?.branchPullRequest, + ); + const supportsMultiplePullRequests = useSupportsMultiplePullRequests(threadRef.environmentId); + const currentLinkedPr = supportsMultiplePullRequests + ? resolveThreadCurrentPullRequestLink(thread?.pullRequests ?? []) + : null; + // Shares the atom the git actions control already reads, so the header asks for one status. + const gitStatus = useEnvironmentQuery( + gitCwd === null + ? null + : vcsEnvironment.status({ environmentId: threadRef.environmentId, input: { cwd: gitCwd } }), + ); + const openPrContextMenu = useThreadPullRequestLinkContextMenu(threadRef); + + const pr = linkedStatus?.pr ?? null; + const pullRequest = useMemo(() => { + const number = pr?.number ?? currentLinkedPr?.number; + const url = pr?.url ?? currentLinkedPr?.url; + return number === undefined || url === undefined ? null : { number, url }; + }, [currentLinkedPr, pr]); + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest, + gitStatus: gitStatus.data ?? null, + }); + + const pillNumber = pill.kind === "linked" ? pill.number : null; + const handleClick = useCallback(() => { + if (pillNumber === null) { + requestCreatePullRequest(); + return; + } + if (!supportsMultiplePullRequests && onOpenPullRequest) { + onOpenPullRequest(pillNumber); + return; + } + useRightPanelStore.getState().open(threadRef, "pull-requests"); + }, [onOpenPullRequest, pillNumber, supportsMultiplePullRequests, threadRef]); + const handleContextMenu = useCallback( + (event: ReactMouseEvent) => { + openPrContextMenu(event, { + url: pullRequest?.url, + providerKind: linkedStatus?.sourceControlProvider.kind, + }); + }, + [linkedStatus, openPrContextMenu, pullRequest], + ); + + if (pill.kind === "hidden") return null; + + const status = + pill.kind === "linked" ? prStatusIndicator(pr, linkedStatus?.sourceControlProvider) : null; + const label = pill.kind === "create" ? "Create PR" : `#${pill.number}`; + const tooltip = + pill.kind === "create" + ? "Create a pull request for this ref" + : (status?.tooltip ?? `Pull request ${label}`); + + return ( + + + } + > + + + {label} + + + + {status ? : tooltip} + + + ); +} diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts index eb6f47b4c803..0384e84fd370 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.test.ts @@ -1,6 +1,53 @@ +import type { ContextMenuItem } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; -import { openOnHostLabel } from "./pullRequestLinkContextMenu"; +import { + openOnHostLabel, + showPullRequestLinkContextMenu, + type PullRequestLinkContextMenuAction, +} from "./pullRequestLinkContextMenu"; + +type Items = readonly ContextMenuItem[]; + +const URL = "https://github.com/pingdotgg/t3code/pull/23"; + +/** + * Opens the menu against a stubbed desktop bridge and hands back what it offered. + * + * These suites run on node, so the bridge `readLocalApi` reaches for is stood up here rather than + * in a DOM. Going through the exported entry point rather than the items helper keeps the test on + * the surface callers actually use. + */ +async function openMenu( + options: { + readonly unlinkFromThread?: ((url: string) => Promise) | undefined; + readonly url?: string; + }, + choose: PullRequestLinkContextMenuAction | null = null, +): Promise { + let items: Items = []; + const globals = globalThis as { window?: unknown }; + const previousWindow = globals.window; + globals.window = { + desktopBridge: { + showContextMenu: async (shown: Items) => { + items = shown; + return choose; + }, + }, + }; + try { + await showPullRequestLinkContextMenu({ + url: options.url ?? URL, + openLabel: "Open on GitHub", + position: { x: 0, y: 0 }, + ...(options.unlinkFromThread ? { unlinkFromThread: options.unlinkFromThread } : {}), + }); + } finally { + globals.window = previousWindow; + } + return items; +} describe("pull request link context menu", () => { it("names every host it knows, and says nothing false about one it does not", () => { @@ -10,4 +57,44 @@ describe("pull request link context menu", () => { expect(openOnHostLabel("azure-devops")).toBe("Open on Azure DevOps"); expect(openOnHostLabel("something-else")).toBe("Open on host"); }); + + it("leaves unlinking out until the caller says this number is the thread's own", async () => { + expect(await openMenu({})).toEqual([ + { id: "copy-link", label: "Copy link", icon: "copy" }, + { id: "open-external", label: "Open on GitHub" }, + ]); + }); + + it("puts unlinking last, behind a divider, so a misclick lands on copy instead", async () => { + expect(await openMenu({ unlinkFromThread: async () => {} })).toEqual([ + { id: "copy-link", label: "Copy link", icon: "copy" }, + { id: "open-external", label: "Open on GitHub" }, + { id: "unlink-from-thread", label: "Unlink #23 from thread", separatorBefore: true }, + ]); + }); + + it("falls back to the bare label for a url it cannot read a number out of", async () => { + const items = await openMenu({ + url: "https://example.com/some/page", + unlinkFromThread: async () => {}, + }); + expect(items.at(-1)).toEqual({ + id: "unlink-from-thread", + label: "Unlink from thread", + separatorBefore: true, + }); + }); + + it("tells the unlink callback which url was acted on, so a stale menu can decline", async () => { + const acted: string[] = []; + await openMenu( + { + unlinkFromThread: async (url) => { + acted.push(url); + }, + }, + "unlink-from-thread", + ); + expect(acted).toEqual([URL]); + }); }); diff --git a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts index 16c4fa5d3559..0b2d1e9d3ac8 100644 --- a/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts +++ b/apps/web/src/components/pullRequest/pullRequestLinkContextMenu.ts @@ -1,11 +1,12 @@ import type { ContextMenuItem } from "@t3tools/contracts"; +import { parseChangeRequestUrl } from "@t3tools/shared/changeRequestUrl"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; import { readLocalApi } from "~/localApi"; import { toastManager } from "../ui/toast"; -export type PullRequestLinkContextMenuAction = "copy-link" | "open-external"; +export type PullRequestLinkContextMenuAction = "copy-link" | "open-external" | "unlink-from-thread"; /** Named for the host rather than "externally": the point is where you will land. */ const OPEN_ON_HOST_LABELS: Partial> = { @@ -19,14 +20,29 @@ const OPEN_ON_HOST_LABELS: Partial> = { export const openOnHostLabel = (provider: string): string => OPEN_ON_HOST_LABELS[provider] ?? "Open on host"; -/** Copy first: it is the reason to right-click a number rather than click it. */ +/** + * Copy first: it is the reason to right-click a number rather than click it. + * + * Unlinking comes last, behind a divider, because it is the one item here that changes the thread + * rather than the clipboard or the browser, and a misclick on it is the only one that costs + * anything. + * + * It also names its number, which the other two do not need to: the badge a reader right-clicks is + * sometimes an aggregate that reads `+3` instead of a number, and the link on that badge is only + * one of the three. The menu says which one is about to go. + */ function pullRequestLinkContextMenuItems( openLabel: string, + unlinkLabel: string | null, ): readonly ContextMenuItem[] { - return [ + const items: ContextMenuItem[] = [ { id: "copy-link", label: "Copy link", icon: "copy" }, { id: "open-external", label: openLabel }, ]; + if (unlinkLabel !== null) { + items.push({ id: "unlink-from-thread", label: unlinkLabel, separatorBefore: true }); + } + return items; } /** @@ -42,16 +58,36 @@ export async function showPullRequestLinkContextMenu({ url, openLabel, position, + unlinkFromThread, }: { readonly url: string; readonly openLabel: string; readonly position: { readonly x: number; readonly y: number }; + /** + * Absent wherever this number is not one the thread is linked to: a pull request read off a + * branch, a row on the list page, a server too old to record links at all. + * + * Handed the URL the menu was opened on, so it can decline once that is no longer a link the + * thread holds. A menu stays open for as long as it takes to read, and in that time the agent + * can link or unlink from the same thread. + */ + readonly unlinkFromThread?: ((url: string) => Promise) | undefined; }): Promise { const api = readLocalApi(); if (!api) return; + const number = parseChangeRequestUrl(url)?.number; + const unlinkLabel = + unlinkFromThread === undefined + ? null + : number === undefined + ? "Unlink from thread" + : `Unlink #${number} from thread`; let action: PullRequestLinkContextMenuAction | null = null; try { - action = await api.contextMenu.show(pullRequestLinkContextMenuItems(openLabel), position); + action = await api.contextMenu.show( + pullRequestLinkContextMenuItems(openLabel, unlinkLabel), + position, + ); } catch { // A menu that could not be shown has already cost the reader their right-click; there is // nothing to say about it that a second popup would not make worse. @@ -60,10 +96,16 @@ export async function showPullRequestLinkContextMenu({ try { if (action === "copy-link") await writeTextToClipboard(url, "link"); else if (action === "open-external") await api.shell.openExternal(url); + else if (action === "unlink-from-thread") await unlinkFromThread?.(url); } catch { toastManager.add({ type: "error", - title: action === "copy-link" ? "Could not copy the link" : "Could not open the link", + title: + action === "copy-link" + ? "Could not copy the link" + : action === "unlink-from-thread" + ? "Could not unlink the pull request" + : "Could not open the link", }); } } diff --git a/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts new file mode 100644 index 000000000000..408c42fb3021 --- /dev/null +++ b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.test.ts @@ -0,0 +1,157 @@ +import type { VcsStatusResult } from "@t3tools/contracts"; +import { assert, describe, it } from "vite-plus/test"; + +import { resolveThreadPullRequestHeaderPill } from "./threadPullRequestHeaderPill.logic"; + +function status(overrides: Partial = {}): VcsStatusResult { + return { + isRepo: true, + hasPrimaryRemote: true, + isDefaultRef: false, + refName: "feature/test", + hasWorkingTreeChanges: false, + workingTree: { files: [], insertions: 0, deletions: 0 }, + hasUpstream: true, + aheadCount: 0, + behindCount: 0, + aheadOfDefaultCount: 1, + pr: null, + ...overrides, + }; +} + +describe("when: the thread carries a pull request", () => { + it("wears its number, whatever the ref is doing", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: { number: 42, url: "https://example.com/pr/42" }, + gitStatus: status({ hasWorkingTreeChanges: true, behindCount: 3 }), + }); + + assert.deepEqual(pill, { kind: "linked", number: 42, url: "https://example.com/pr/42" }); + }); + + it("wears its number even before git status arrives", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: { number: 7, url: "https://example.com/pr/7" }, + gitStatus: null, + }); + + assert.deepEqual(pill, { kind: "linked", number: 7, url: "https://example.com/pr/7" }); + }); +}); + +describe("when: the thread has no pull request", () => { + it("offers to create one from a clean ref that is ahead of default", () => { + assert.deepEqual( + resolveThreadPullRequestHeaderPill({ pullRequest: null, gitStatus: status() }), + { + kind: "create", + }, + ); + }); + + it("offers to create one from an unpublished ref that has a remote to publish to", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasUpstream: false, aheadCount: 2, aheadOfDefaultCount: undefined }), + }); + + assert.deepEqual(pill, { kind: "create" }); + }); + + it("falls back to the ahead count when the default branch delta is unknown", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ aheadCount: 0, aheadOfDefaultCount: undefined }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides while git status is still loading", () => { + assert.deepEqual(resolveThreadPullRequestHeaderPill({ pullRequest: null, gitStatus: null }), { + kind: "hidden", + }); + }); + + it("hides on a detached head", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ refName: null }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides with uncommitted work, because creating one would leave it behind", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasWorkingTreeChanges: true }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides on a ref with nothing on top of the default branch", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ aheadOfDefaultCount: 0 }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides while the ref is behind its upstream", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ behindCount: 1 }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides when there is nowhere to push the ref", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ hasUpstream: false, hasPrimaryRemote: false }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("hides when the ref already has an open pull request the thread has not linked", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ + pr: { + number: 21, + title: "Open PR", + url: "https://example.com/pr/21", + baseRef: "main", + headRef: "feature/test", + state: "open", + }, + }), + }); + + assert.deepEqual(pill, { kind: "hidden" }); + }); + + it("offers to create one when the ref's last pull request is closed", () => { + const pill = resolveThreadPullRequestHeaderPill({ + pullRequest: null, + gitStatus: status({ + pr: { + number: 20, + title: "Closed PR", + url: "https://example.com/pr/20", + baseRef: "main", + headRef: "feature/test", + state: "closed", + }, + }), + }); + + assert.deepEqual(pill, { kind: "create" }); + }); +}); diff --git a/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts new file mode 100644 index 000000000000..fcab0d394e9f --- /dev/null +++ b/apps/web/src/components/pullRequest/threadPullRequestHeaderPill.logic.ts @@ -0,0 +1,41 @@ +import type { VcsStatusResult } from "@t3tools/contracts"; + +export type ThreadPullRequestHeaderPill = + | { readonly kind: "linked"; readonly number: number; readonly url: string } + | { readonly kind: "create" } + | { readonly kind: "hidden" }; + +/** + * What the header's pull request control should be right now. + * + * The git actions button beside it answers "what moves my work forward", which is why it changes + * label with every commit and push. This answers the question that outlives those: where the pull + * request is, or that there is none yet. Splitting them is what stops one button meaning + * "Commit & push" in the morning and "View PR" in the afternoon. + * + * Hidden is the answer when creating one would fail anyway. Offering it on a thread with nothing + * committed, or behind its upstream, would put a button in the header whose only outcome is an + * error toast. + */ +export function resolveThreadPullRequestHeaderPill(input: { + /** The number the thread wears: an explicit link first, otherwise the one read off its branch. */ + readonly pullRequest: { readonly number: number; readonly url: string } | null; + readonly gitStatus: VcsStatusResult | null; +}): ThreadPullRequestHeaderPill { + if (input.pullRequest !== null) { + return { kind: "linked", number: input.pullRequest.number, url: input.pullRequest.url }; + } + const gitStatus = input.gitStatus; + if (gitStatus === null || gitStatus.refName === null) return { kind: "hidden" }; + // Mirrors the old menu's own create gate, so the pill appears exactly where "Create PR" used to + // be offered and nowhere it was not. + const canPushWithoutUpstream = gitStatus.hasPrimaryRemote && !gitStatus.hasUpstream; + const hasDefaultBranchDelta = (gitStatus.aheadOfDefaultCount ?? gitStatus.aheadCount) > 0; + const canCreate = + !gitStatus.hasWorkingTreeChanges && + gitStatus.pr?.state !== "open" && + hasDefaultBranchDelta && + gitStatus.behindCount === 0 && + (gitStatus.hasUpstream || canPushWithoutUpstream); + return canCreate ? { kind: "create" } : { kind: "hidden" }; +} diff --git a/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts new file mode 100644 index 000000000000..eea6a62350ae --- /dev/null +++ b/apps/web/src/components/pullRequest/useThreadPullRequestLinkContextMenu.ts @@ -0,0 +1,59 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { useCallback, type MouseEvent as ReactMouseEvent } from "react"; + +import { useLazyPullRequestLinking } from "~/hooks/usePullRequestLinking"; +import { readThreadShell } from "~/state/entities"; + +import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; + +/** + * The right-click behind every pull request number a thread wears: the sidebar row, the composer's + * branch toolbar, the legacy sidebar. + * + * Those numbers are the only place a link is visible from the thread you are reading, so they are + * where a reader goes to undo one. Unlinking was reachable before this only from the right panel or + * from the original URL in the transcript, which may be hundreds of turns up or may never have been + * a message at all when the agent linked the pull request itself. + * + * Whether the thread is linked is read when the menu opens, and again when the item is chosen: a + * menu sits open for as long as it takes to read, and the agent can link or unlink from underneath + * it in that time. That same lateness is why the linking state is resolved per click here rather + * than subscribed to: this hook is mounted once per row of the thread list. + */ +export function useThreadPullRequestLinkContextMenu(threadRef: ScopedThreadRef | null | undefined) { + const resolvePullRequestLinking = useLazyPullRequestLinking(threadRef?.environmentId); + return useCallback( + ( + event: ReactMouseEvent, + pullRequest: { + readonly url: string | undefined; + readonly providerKind: string | undefined; + }, + ) => { + const url = pullRequest.url; + if (url === undefined) return; + event.preventDefault(); + event.stopPropagation(); + const linking = resolvePullRequestLinking(); + const linked = threadRef != null && linking.isLinked(readThreadShell(threadRef), url); + void showPullRequestLinkContextMenu({ + url, + openLabel: openOnHostLabel(pullRequest.providerKind ?? ""), + position: { x: event.clientX, y: event.clientY }, + // Offered only for a number the thread is actually linked to. The same badge also shows a + // pull request read off the thread's branch, and that one is a fact about git rather than + // a choice anyone made to undo. + ...(linked && threadRef != null + ? { + unlinkFromThread: async (target: string) => { + const current = resolvePullRequestLinking(); + if (!current.isLinked(readThreadShell(threadRef), target)) return; + await current.changeLink(threadRef, target, false); + }, + } + : {}), + }); + }, + [resolvePullRequestLinking, threadRef], + ); +} diff --git a/apps/web/src/gitActionsBus.ts b/apps/web/src/gitActionsBus.ts new file mode 100644 index 000000000000..d99fad91b135 --- /dev/null +++ b/apps/web/src/gitActionsBus.ts @@ -0,0 +1,14 @@ +// Lets the header's pull request pill reach the git actions control, which owns the create flow: +// its progress stages, its default-ref confirmation, and the toast that reports the result. The +// two sit in different corners of the header, so a shared parent would have to thread that state +// through everything between them. +const CREATE_PULL_REQUEST_EVENT = "t3code:create-pull-request"; + +export function requestCreatePullRequest(): void { + window.dispatchEvent(new CustomEvent(CREATE_PULL_REQUEST_EVENT)); +} + +export function onRequestCreatePullRequest(listener: () => void): () => void { + window.addEventListener(CREATE_PULL_REQUEST_EVENT, listener); + return () => window.removeEventListener(CREATE_PULL_REQUEST_EVENT, listener); +} diff --git a/apps/web/src/hooks/usePullRequestLinking.ts b/apps/web/src/hooks/usePullRequestLinking.ts index f6c076f3009d..c6c3394cdfce 100644 --- a/apps/web/src/hooks/usePullRequestLinking.ts +++ b/apps/web/src/hooks/usePullRequestLinking.ts @@ -1,4 +1,4 @@ -import { useMemo } from "react"; +import { useCallback, useMemo } from "react"; import { sourceControlRepositorySelector } from "@t3tools/shared/sourceControl"; import type { EnvironmentId, @@ -24,81 +24,126 @@ import { matchesLinkedPullRequestUrl, parseChangeRequestUrl, } from "~/lib/openPullRequestLink"; -import { useProjects, useServerConfigs } from "~/state/entities"; +import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; +import { readProjects, readServerConfigs, useProjects, useServerConfigs } from "~/state/entities"; import { threadEnvironment } from "~/state/threads"; import { useAtomCommand } from "~/state/use-atom-command"; -/** Routes link actions through the command advertised by this environment. */ -export function usePullRequestLinking(environmentId: EnvironmentId | null | undefined) { - const configs = useServerConfigs(); - const projects = useProjects(); - const capabilities = - environmentId == null ? undefined : configs.get(environmentId)?.environment.capabilities; - const mode = threadPullRequestLinkMode(capabilities); +/** The three writes, which depend on nothing a render can change. */ +function usePullRequestLinkCommands() { const link = useAtomCommand(threadEnvironment.linkPullRequest, { reportFailure: false }); const unlink = useAtomCommand(threadEnvironment.unlinkPullRequest, { reportFailure: false }); const updateMetadata = useAtomCommand(threadEnvironment.updateMetadata, { reportFailure: false }); - return useMemo(() => { - const environmentProjects = projects.filter( - (project) => project.environmentId === environmentId, + return useMemo(() => ({ link, unlink, updateMetadata }), [link, unlink, updateMetadata]); +} + +function buildPullRequestLinking({ + environmentId, + capabilities, + projects, + commands, +}: { + readonly environmentId: EnvironmentId | null | undefined; + readonly capabilities: Parameters[0]; + readonly projects: ReadonlyArray; + readonly commands: ReturnType; +}) { + const { link, unlink, updateMetadata } = commands; + const mode = threadPullRequestLinkMode(capabilities); + const environmentProjects = projects.filter((project) => project.environmentId === environmentId); + const canLink = (url: string) => { + const parsed = parseChangeRequestUrl(url); + if (parsed === null || mode === "unsupported") return false; + return ( + (mode === "multiple" ? findProjectOnChangeRequestHost : findProjectForChangeRequest)( + environmentProjects, + parsed, + ) !== undefined ); - const canLink = (url: string) => { - const parsed = parseChangeRequestUrl(url); - if (parsed === null || mode === "unsupported") return false; - return ( - (mode === "multiple" ? findProjectOnChangeRequestHost : findProjectForChangeRequest)( - environmentProjects, - parsed, - ) !== undefined - ); - }; - const isLinked = ( - thread: { - readonly pullRequests?: readonly ThreadPullRequestLink[]; - readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; - } | null, - url: string, - ) => { - if (thread === null || mode === "unsupported") return false; - if (mode !== "multiple") - return ( - thread.linkedPullRequest != null && - matchesLinkedPullRequestUrl(thread.linkedPullRequest, url) - ); - const parsed = parseChangeRequestUrl(url); + }; + const isLinked = ( + thread: { + readonly pullRequests?: readonly ThreadPullRequestLink[]; + readonly linkedPullRequest?: ThreadLinkedPullRequest | null | undefined; + } | null, + url: string, + ) => { + if (thread === null || mode === "unsupported") return false; + if (mode !== "multiple") return ( - parsed !== null && - visibleThreadPullRequests(thread.pullRequests ?? []).some((entry) => - threadPullRequestKeysEqual(entry, parsed), - ) + thread.linkedPullRequest != null && + matchesLinkedPullRequestUrl(thread.linkedPullRequest, url) ); - }; - const changeLink = async (threadRef: ScopedThreadRef, url: string, linked: boolean) => { - const parsed = parseChangeRequestUrl(url); - if (parsed === null || threadRef.environmentId !== environmentId || (linked && !canLink(url))) - throw new Error("The pull request is not available in this environment."); - const legacyProject = findProjectForChangeRequest(environmentProjects, parsed); - const mutation = planThreadPullRequestMutation({ - capabilities, - threadId: threadRef.threadId, - reference: { ...parsed, url }, - legacyProjectId: legacyProject?.id ?? null, - legacyRepository: - sourceControlRepositorySelector(legacyProject?.repositoryIdentity) ?? undefined, - linked, - }); - if (mutation === null) - throw new Error("This environment does not support linking this pull request."); - const result = await (mutation.type === "thread.meta.update" - ? updateMetadata({ environmentId: threadRef.environmentId, input: mutation.input }) - : mutation.type === "thread.pull-request.link" - ? link({ environmentId: threadRef.environmentId, input: mutation.input }) - : unlink({ environmentId: threadRef.environmentId, input: mutation.input })); - if (result._tag === "Failure") { - if (isAtomCommandInterrupted(result)) throw new Error("Link update interrupted."); - throw squashAtomCommandFailure(result); - } - }; - return { mode, canLink, isLinked, changeLink }; - }, [capabilities, environmentId, link, mode, projects, unlink, updateMetadata]); + const parsed = parseChangeRequestUrl(url); + return ( + parsed !== null && + visibleThreadPullRequests(thread.pullRequests ?? []).some((entry) => + threadPullRequestKeysEqual(entry, parsed), + ) + ); + }; + const changeLink = async (threadRef: ScopedThreadRef, url: string, linked: boolean) => { + const parsed = parseChangeRequestUrl(url); + if (parsed === null || threadRef.environmentId !== environmentId || (linked && !canLink(url))) + throw new Error("The pull request is not available in this environment."); + const legacyProject = findProjectForChangeRequest(environmentProjects, parsed); + const mutation = planThreadPullRequestMutation({ + capabilities, + threadId: threadRef.threadId, + reference: { ...parsed, url }, + legacyProjectId: legacyProject?.id ?? null, + legacyRepository: + sourceControlRepositorySelector(legacyProject?.repositoryIdentity) ?? undefined, + linked, + }); + if (mutation === null) + throw new Error("This environment does not support linking this pull request."); + const result = await (mutation.type === "thread.meta.update" + ? updateMetadata({ environmentId: threadRef.environmentId, input: mutation.input }) + : mutation.type === "thread.pull-request.link" + ? link({ environmentId: threadRef.environmentId, input: mutation.input }) + : unlink({ environmentId: threadRef.environmentId, input: mutation.input })); + if (result._tag === "Failure") { + if (isAtomCommandInterrupted(result)) throw new Error("Link update interrupted."); + throw squashAtomCommandFailure(result); + } + }; + return { mode, canLink, isLinked, changeLink }; +} + +/** Routes link actions through the command advertised by this environment. */ +export function usePullRequestLinking(environmentId: EnvironmentId | null | undefined) { + const configs = useServerConfigs(); + const projects = useProjects(); + const commands = usePullRequestLinkCommands(); + const capabilities = + environmentId == null ? undefined : configs.get(environmentId)?.environment.capabilities; + return useMemo( + () => buildPullRequestLinking({ environmentId, capabilities, projects, commands }), + [capabilities, commands, environmentId, projects], + ); +} + +/** + * The same actions, resolved on the click rather than on every render. + * + * A caller that only acts once a menu is open has no use for the projects or the server config + * while it waits. This one is mounted per thread row, so subscribing there would put a project + * or config change through every row in the list and filter the whole projects array once per row. + */ +export function useLazyPullRequestLinking(environmentId: EnvironmentId | null | undefined) { + const commands = usePullRequestLinkCommands(); + return useCallback( + () => + buildPullRequestLinking({ + environmentId, + capabilities: + environmentId == null + ? undefined + : readServerConfigs().get(environmentId)?.environment.capabilities, + projects: readProjects(), + commands, + }), + [commands, environmentId], + ); } diff --git a/apps/web/src/state/entities.ts b/apps/web/src/state/entities.ts index d9610e20717f..72b26b113e85 100644 --- a/apps/web/src/state/entities.ts +++ b/apps/web/src/state/entities.ts @@ -154,6 +154,10 @@ export function readProjects(): ReadonlyArray { return appAtomRegistry.get(environmentProjects.projectsAtom); } +export function readServerConfigs(): ReadonlyMap { + return appAtomRegistry.get(environmentServerConfigsAtom); +} + /** Resolves when the project event reaches the live client store. */ export function waitForProject( ref: ScopedProjectRef, diff --git a/docs/user/source-control.md b/docs/user/source-control.md index d8a06fc34b64..db18dd06b1ea 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -87,8 +87,9 @@ make your first commit before pushing. ## Create a pull request -Use a thread's Git actions to commit, push, and create a pull request. T3 Code can generate commit -messages, review titles, and descriptions from your changes. +Use a thread's Git actions to commit and push, then the pull request button beside them to open one. +Once the thread has a pull request, that button shows its number and opens **Linked pull requests**. +T3 Code can generate commit messages, review titles, and descriptions from your changes. Choose the writing style and model in **Settings → Source Control**. **Repository conventions** uses the project's instructions and recent commit subjects. @@ -147,7 +148,8 @@ links. On mobile, the Git overview lists linked reviews and their stacks; tap a Linking and unlinking are available in the web and desktop clients. The **Linked pull requests** panel lists every review and groups stacks. Unlink a review from its -row menu. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; +row menu, or right-click its number anywhere it appears: the thread list, the thread header, or a +link in the conversation. An unlinked stack layer stays out of later syncs. Open linked reviews refresh on the server; closed reviews refresh periodically so reopening one on the host is detected. Merged reviews refresh when requested. With **Auto-settle merged threads** enabled, a thread can settle after every linked review is terminal. An open or unsynced link keeps it active. diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index 67574f0b0286..cbc246e16c92 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -107,6 +107,12 @@ On web and desktop, right-click a pull request link in a thread and choose same link to return to the branch PR, if one exists. The linked pull request participates in automatic settlement. +A linked pull request also shows its number on the thread's sidebar row and on +the thread header's own button. Right-click either number for the same +**Unlink from thread**, alongside **Copy link** and opening the PR on its host. +The number a thread picked up from its branch can be copied and opened the same +way, but not unlinked, because nothing linked it. + ## Find and reference work On web and desktop, open the command palette with `Cmd/Ctrl+K` to search threads