From 51a435d3cf779b0a0a8cb9a06d03fa50d8d93da2 Mon Sep 17 00:00:00 2001 From: Christoph Leiter Date: Fri, 28 Aug 2026 10:50:31 +0200 Subject: [PATCH] feat(app): find workspaces by PR/MR number in the command center (#3008) * feat(app): find workspaces by PR/MR number in the command center The command center matched workspaces on title, host, project, and branch. Typing the change-request number you actually remember found nothing, so you had to recall the branch name instead. Workspace results now carry their pull/merge request number, and a numeric query resolves it directly: 42, #42, !42, pr 42, and mr 42 all reach the same workspace. Matching compares the parsed number exactly rather than as a substring, so 42 no longer drags in PR 142 or a branch that happens to contain those digits. Exact number hits sort above ordinary text matches for the same query. Nothing changes visually. The number is a search key only; result rows render exactly as before. The number is read from the pull request the daemon already sends. selectPrHintFromStatus is the fallback only: it derives the number by parsing the PR/MR URL, which yields nothing for a noncanonical or absent URL even when the daemon sent a usable number. Co-Authored-By: Claude Opus 5 * test(app): exercise change-request parsing through the matcher parseChangeRequestQuery was exported only so the test could reach it, which made an implementation helper part of the module surface and let the parser cases pass without covering how the parser composes with workspace matching. Unexport it and drive the same spellings through matchWorkspaceQuery. The rejection cases now assert what they actually protect: a query like fix-42-retries must not take the number path, so a workspace whose PR is 42 does not match it. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .../app/src/command-center/command-center.tsx | 13 +-- .../app/src/command-center/results.test.ts | 9 +- packages/app/src/command-center/results.ts | 2 + .../command-center/workspace-search.test.ts | 82 +++++++++++++++++++ .../src/command-center/workspace-search.ts | 70 ++++++++++++++++ .../app/src/screens/projects-screen.test.tsx | 1 + packages/app/src/utils/projects.test.ts | 71 ++++++++++++++++ packages/app/src/utils/projects.ts | 18 ++++ 8 files changed, 255 insertions(+), 11 deletions(-) create mode 100644 packages/app/src/command-center/workspace-search.test.ts create mode 100644 packages/app/src/command-center/workspace-search.ts diff --git a/packages/app/src/command-center/command-center.tsx b/packages/app/src/command-center/command-center.tsx index d61ffa2e54..e909bf28e5 100644 --- a/packages/app/src/command-center/command-center.tsx +++ b/packages/app/src/command-center/command-center.tsx @@ -56,6 +56,7 @@ import { navigateToAgent } from "@/utils/navigate-to-agent"; import { formatTimeAgo } from "@/utils/time"; import { shortenPath } from "@/utils/shorten-path"; import { useCommandCenterContributions } from "./provider"; +import { filterAndRankWorkspaces } from "./workspace-search"; import { buildContributionSections, filterAndRankBuiltInResults, @@ -121,10 +122,6 @@ function agentSearchFields(result: CommandCenterAgentResult): CommandCenterSearc return { visible: [result.title, result.subtitle], hidden: [result.agent.cwd] }; } -function workspaceSearchFields(result: CommandCenterWorkspaceResult): CommandCenterSearchFields { - return { visible: [result.title, result.subtitle], hidden: [] }; -} - /** * Build every pinned row, in its default order. Deliberately not keyed on `query`: none of this * work depends on what was typed, and rebuilding it per keystroke means re-running an Intl @@ -158,6 +155,7 @@ function useBuiltInRows(open: boolean): { project.projectName, workspace.currentBranch, ]), + changeRequestNumber: workspace.changeRequestNumber, run: () => { clearCommandCenterFocusRestoreElement(); navigateToWorkspace({ serverId: host.serverId, workspaceId: workspace.id }); @@ -208,12 +206,7 @@ function useBuiltInSections(open: boolean, query: string): CommandCenterResultSe band: PINNED_SECTION_BAND, rank: 2, title: t("shell.commandCenter.workspaces"), - results: filterAndRankBuiltInResults( - rows.workspaces, - query, - workspaceSearchFields, - compareWorkspacesByTitle, - ), + results: filterAndRankWorkspaces(rows.workspaces, query, compareWorkspacesByTitle), }, { id: "agents", diff --git a/packages/app/src/command-center/results.test.ts b/packages/app/src/command-center/results.test.ts index 6e37de1fed..d62ea92f35 100644 --- a/packages/app/src/command-center/results.test.ts +++ b/packages/app/src/command-center/results.test.ts @@ -102,7 +102,14 @@ function autoModeChoice(): CommandCenterContribution { } function workspace(id: string, title = id, subtitle = "host"): CommandCenterWorkspaceResult { - return { kind: "workspace", id, title, subtitle, run: () => undefined }; + return { + kind: "workspace", + id, + title, + subtitle, + changeRequestNumber: null, + run: () => undefined, + }; } function sectionResultIds(sections: ReturnType): string[] { diff --git a/packages/app/src/command-center/results.ts b/packages/app/src/command-center/results.ts index 3d50020ccc..ff9d5c90ad 100644 --- a/packages/app/src/command-center/results.ts +++ b/packages/app/src/command-center/results.ts @@ -48,6 +48,8 @@ export interface CommandCenterWorkspaceResult { id: string; title: string; subtitle: string; + /** PR/MR number backing this workspace, so a number query can match it exactly. */ + changeRequestNumber: number | null; run(): void; } diff --git a/packages/app/src/command-center/workspace-search.test.ts b/packages/app/src/command-center/workspace-search.test.ts new file mode 100644 index 0000000000..2cdae53dd2 --- /dev/null +++ b/packages/app/src/command-center/workspace-search.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import type { CommandCenterWorkspaceResult } from "./results"; +import { filterAndRankWorkspaces } from "./workspace-search"; + +function workspace(input: { + id: string; + title?: string; + subtitle?: string; + changeRequestNumber?: number | null; +}): CommandCenterWorkspaceResult { + return { + kind: "workspace", + id: input.id, + title: input.title ?? input.id, + subtitle: input.subtitle ?? "", + changeRequestNumber: input.changeRequestNumber ?? null, + run: () => undefined, + }; +} + +function ids(rows: readonly CommandCenterWorkspaceResult[], query: string): string[] { + return filterAndRankWorkspaces(rows, query, (left, right) => left.id.localeCompare(right.id)).map( + (row) => row.id, + ); +} + +describe("filterAndRankWorkspaces", () => { + const subject = workspace({ + id: "payments", + title: "Payments refactor", + subtitle: "Primary host · feature/checkout", + changeRequestNumber: 42, + }); + + it("matches everything when the query is empty", () => { + expect(ids([subject], " ")).toEqual(["payments"]); + }); + + it("matches its own change-request number in every spelling", () => { + const spellings = ["42", " 42 ", "#42", "!42", "pr 42", "mr 42", "PR #42", "mr!42", "pr42"]; + for (const query of spellings) { + expect(ids([subject], query)).toEqual(["payments"]); + } + }); + + it("does not take the number path for text that merely contains digits", () => { + const numbered = workspace({ id: "unrelated workspace", changeRequestNumber: 42 }); + for (const query of ["fix-42-retries", "42x", "v4.2", "pr"]) { + expect(ids([numbered], query)).toEqual([]); + } + }); + + it("does not match a different change-request number that shares digits", () => { + const other = workspace({ id: "unrelated", changeRequestNumber: 142 }); + expect(ids([other], "42")).toEqual([]); + }); + + it("still matches on title and branch text", () => { + expect(ids([subject], "checkout")).toEqual(["payments"]); + expect(ids([subject], "payments")).toEqual(["payments"]); + }); + + it("leaves a workspace without a change request unaffected", () => { + const noPr = workspace({ id: "docs workspace" }); + expect(ids([noPr], "42")).toEqual([]); + expect(ids([noPr], "docs")).toEqual(["docs workspace"]); + }); + + it("falls back to text matching when the number does not match", () => { + const textual = workspace({ + id: "text", + subtitle: "branch fix-42-retries", + changeRequestNumber: 7, + }); + expect(ids([textual], "42")).toEqual(["text"]); + }); + + it("ranks an exact change request above an ordinary text match", () => { + const text = workspace({ id: "a-text", title: "Migration 42" }); + expect(ids([text, subject], "42")).toEqual(["payments", "a-text"]); + }); +}); diff --git a/packages/app/src/command-center/workspace-search.ts b/packages/app/src/command-center/workspace-search.ts new file mode 100644 index 0000000000..317d1f1561 --- /dev/null +++ b/packages/app/src/command-center/workspace-search.ts @@ -0,0 +1,70 @@ +import { + compareCommandCenterScores, + filterAndRankBuiltInResults, + scoreSearchFields, + type CommandCenterScore, + type CommandCenterWorkspaceResult, +} from "./results"; + +/** A change-request query: an optional `pr`/`mr` word, an optional `#`/`!`, a number. */ +const CHANGE_REQUEST_QUERY = /^(?:(?:pr|mr)\s*)?[#!]?(\d+)$/i; + +/** + * Parse a change-request-shaped query into its number. + * + * Accepts `42`, `#42`, `!42`, `pr 42`, `pr42`, `PR #42`, `mr!42`, and so on. + * Returns null for anything else, including text queries that merely contain + * digits (`fix-42-retries`), so those fall through to normal text matching. + */ +function parseChangeRequestQuery(query: string): number | null { + const match = CHANGE_REQUEST_QUERY.exec(query.trim()); + if (!match) return null; + const number = Number.parseInt(match[1], 10); + return Number.isSafeInteger(number) ? number : null; +} + +interface ScoredWorkspace { + workspace: CommandCenterWorkspaceResult; + score: CommandCenterScore | null; + changeRequestHit: boolean; +} + +function searchFields(workspace: CommandCenterWorkspaceResult) { + return { visible: [workspace.title, workspace.subtitle], hidden: [] }; +} + +function compareScoredWorkspaces( + left: ScoredWorkspace, + right: ScoredWorkspace, + tiebreak: (left: CommandCenterWorkspaceResult, right: CommandCenterWorkspaceResult) => number, +): number { + if (left.changeRequestHit !== right.changeRequestHit) { + return left.changeRequestHit ? -1 : 1; + } + if (left.score && right.score) { + const scoreDelta = compareCommandCenterScores(left.score, right.score); + if (scoreDelta !== 0) return scoreDelta; + } + return tiebreak(left.workspace, right.workspace); +} + +export function filterAndRankWorkspaces( + workspaces: readonly CommandCenterWorkspaceResult[], + query: string, + tiebreak: (left: CommandCenterWorkspaceResult, right: CommandCenterWorkspaceResult) => number, +): CommandCenterWorkspaceResult[] { + if (!query.trim()) { + return filterAndRankBuiltInResults(workspaces, query, searchFields, tiebreak); + } + + const changeRequestNumber = parseChangeRequestQuery(query); + const matches: ScoredWorkspace[] = []; + for (const workspace of workspaces) { + const score = scoreSearchFields(query, searchFields(workspace)); + const changeRequestHit = + changeRequestNumber !== null && workspace.changeRequestNumber === changeRequestNumber; + if (score || changeRequestHit) matches.push({ workspace, score, changeRequestHit }); + } + matches.sort((left, right) => compareScoredWorkspaces(left, right, tiebreak)); + return matches.map((match) => match.workspace); +} diff --git a/packages/app/src/screens/projects-screen.test.tsx b/packages/app/src/screens/projects-screen.test.tsx index 2dc1709587..0533d7c810 100644 --- a/packages/app/src/screens/projects-screen.test.tsx +++ b/packages/app/src/screens/projects-screen.test.tsx @@ -217,6 +217,7 @@ function workspaceSummary(overrides: Partial = {}): WorkspaceS workspaceKind: "directory", status: "done", currentBranch: "main", + changeRequestNumber: null, ...overrides, }; } diff --git a/packages/app/src/utils/projects.test.ts b/packages/app/src/utils/projects.test.ts index 5fedcd7f73..c3416d19b1 100644 --- a/packages/app/src/utils/projects.test.ts +++ b/packages/app/src/utils/projects.test.ts @@ -136,3 +136,74 @@ describe("buildProjects", () => { ); }); }); + +describe("workspace change request number", () => { + function summarize(overrides: Partial) { + const result = buildProjects({ + hosts: [ + { + serverId: "host-a", + serverName: "Host A", + isOnline: true, + projects: [descriptor("prj_a", "shared", "/a/app")], + workspaces: [{ ...workspace("ws-a", "prj_a", "/a/app"), ...overrides }], + }, + ], + }); + return result.projects[0]?.hosts[0]?.workspaces[0]?.changeRequestNumber; + } + + const pullRequest = { + url: "https://github.com/acme/app/pull/42", + title: "Refactor payments", + state: "open", + baseRefName: "main", + headRefName: "feature/checkout", + isMerged: false, + }; + + test("prefers the number the daemon sent over parsing the url", () => { + expect( + summarize({ + // The url says 999, the authoritative field says 42 — the field wins. + githubRuntime: { + pullRequest: { ...pullRequest, number: 42, url: "https://example.test/pull/999" }, + }, + forge: "github", + }), + ).toBe(42); + }); + + test("falls back to the url when the daemon omits the number", () => { + expect(summarize({ githubRuntime: { pullRequest }, forge: "github" })).toBe(42); + }); + + test("parses a gitlab merge request url", () => { + expect( + summarize({ + githubRuntime: { + pullRequest: { ...pullRequest, url: "https://gitlab.com/acme/app/-/merge_requests/7" }, + }, + forge: "gitlab", + }), + ).toBe(7); + }); + + test("resolves the number when an old daemon omits the forge", () => { + expect(summarize({ githubRuntime: { pullRequest: { ...pullRequest, number: 42 } } })).toBe(42); + }); + + test("is null when the workspace has no pull request", () => { + expect(summarize({})).toBeNull(); + expect(summarize({ githubRuntime: { pullRequest: null } })).toBeNull(); + }); + + test("is null when the number is absent and the url is unparseable", () => { + expect( + summarize({ + githubRuntime: { pullRequest: { ...pullRequest, url: "not-a-url" } }, + forge: "github", + }), + ).toBeNull(); + }); +}); diff --git a/packages/app/src/utils/projects.ts b/packages/app/src/utils/projects.ts index 83f11d5932..f5ee1142c4 100644 --- a/packages/app/src/utils/projects.ts +++ b/packages/app/src/utils/projects.ts @@ -1,6 +1,7 @@ import type { ProjectDescriptor, WorkspaceDescriptor } from "@/stores/session-store"; import type { HostProjectListItem } from "@/projects/host-project-model"; import { buildWorkspaceStructureProjects } from "@/projects/workspace-structure"; +import { selectPrHintFromStatus } from "@/git/pr-hint"; export interface WorkspaceSummary { id: string; @@ -10,6 +11,8 @@ export interface WorkspaceSummary { status: WorkspaceDescriptor["status"]; currentBranch: string | null; archivingAt?: string; + /** Pull/merge request number backing this workspace, so a query can find it. */ + changeRequestNumber: number | null; } export interface ProjectHostEntry { @@ -133,6 +136,20 @@ function resolveHostRepoRoot(input: { return input.workspaces[0]?.projectRootPath || input.projectRoot; } +/** + * Resolve the workspace's pull/merge request number. + * + * The wire carries the number directly, so prefer it. `selectPrHintFromStatus` + * is only the fallback: it derives the number by regex-parsing the PR/MR URL, + * which yields null for a noncanonical or absent URL even when the daemon sent + * a perfectly good number. + */ +function toChangeRequestNumber(workspace: WorkspaceDescriptor): number | null { + const pullRequest = workspace.githubRuntime?.pullRequest; + if (!pullRequest) return null; + return pullRequest.number ?? selectPrHintFromStatus(pullRequest, workspace.forge)?.number ?? null; +} + function toWorkspaceSummary(workspace: WorkspaceDescriptor): WorkspaceSummary { const currentBranch = workspace.gitRuntime?.currentBranch?.trim(); return { @@ -143,6 +160,7 @@ function toWorkspaceSummary(workspace: WorkspaceDescriptor): WorkspaceSummary { status: workspace.status, currentBranch: currentBranch && currentBranch !== "HEAD" ? currentBranch : null, ...(workspace.archivingAt ? { archivingAt: workspace.archivingAt } : {}), + changeRequestNumber: toChangeRequestNumber(workspace), }; }