From e5deb909864307e09b90f5690c40763a039ad6ff Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sat, 9 May 2026 21:28:15 +0800 Subject: [PATCH] refactor(server): extract checkout status projection (#838) --- .../server/checkout/status-projection.test.ts | 38 +++++ .../src/server/checkout/status-projection.ts | 134 +++++++++++++++ packages/server/src/server/session.test.ts | 37 +---- packages/server/src/server/session.ts | 152 ++---------------- 4 files changed, 183 insertions(+), 178 deletions(-) create mode 100644 packages/server/src/server/checkout/status-projection.test.ts create mode 100644 packages/server/src/server/checkout/status-projection.ts diff --git a/packages/server/src/server/checkout/status-projection.test.ts b/packages/server/src/server/checkout/status-projection.test.ts new file mode 100644 index 000000000..df6c424a2 --- /dev/null +++ b/packages/server/src/server/checkout/status-projection.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, test } from "vitest"; + +import { CheckoutPrStatusSchema } from "../../shared/messages.js"; +import { normalizeCheckoutPrStatusPayload } from "./status-projection.js"; + +describe("checkout status projection", () => { + test("includes repository identity fields on the PR status wire payload", () => { + const payload = normalizeCheckoutPrStatusPayload({ + number: 123, + repoOwner: "internal-owner", + repoName: "internal-repo", + url: "https://github.com/getpaseo/paseo/pull/123", + title: "Ship PR pane", + state: "open", + baseRefName: "main", + headRefName: "feature/pr-pane", + isMerged: false, + isDraft: true, + mergeable: "MERGEABLE", + checks: [ + { + name: "typecheck", + status: "success", + url: "https://github.com/getpaseo/paseo/actions/runs/1", + workflow: "CI", + duration: "1m 20s", + }, + ], + checksStatus: "success", + reviewDecision: "approved", + }); + + expect(payload).toHaveProperty("repoOwner", "internal-owner"); + expect(payload).toHaveProperty("repoName", "internal-repo"); + expect(payload).toHaveProperty("mergeable", "MERGEABLE"); + expect(CheckoutPrStatusSchema.parse(payload)).toEqual(payload); + }); +}); diff --git a/packages/server/src/server/checkout/status-projection.ts b/packages/server/src/server/checkout/status-projection.ts new file mode 100644 index 000000000..685134869 --- /dev/null +++ b/packages/server/src/server/checkout/status-projection.ts @@ -0,0 +1,134 @@ +import type { + CheckoutPrStatusResponse, + CheckoutStatusResponse, + SessionOutboundMessage, +} from "../../shared/messages.js"; +import type { WorkspaceGitRuntimeSnapshot } from "../workspace-git-service.js"; + +type CheckoutPrStatusPayload = Extract< + SessionOutboundMessage, + { type: "checkout_pr_status_response" } +>["payload"]; +type CheckoutPrStatusPayloadStatus = NonNullable; + +export function buildCheckoutStatusPayloadFromSnapshot({ + cwd, + requestId, + snapshot, +}: { + cwd: string; + requestId: string; + snapshot: WorkspaceGitRuntimeSnapshot; +}): CheckoutStatusResponse["payload"] { + if (!snapshot.git.isGit) { + return { + cwd, + isGit: false, + repoRoot: null, + currentBranch: null, + isDirty: null, + baseRef: null, + aheadBehind: null, + aheadOfOrigin: null, + behindOfOrigin: null, + hasRemote: false, + remoteUrl: null, + isPaseoOwnedWorktree: false, + error: null, + requestId, + }; + } + + if (snapshot.git.repoRoot === null || snapshot.git.isDirty === null) { + throw new Error("Workspace git snapshot is missing required checkout status fields"); + } + + if (snapshot.git.isPaseoOwnedWorktree) { + if (snapshot.git.mainRepoRoot === null || snapshot.git.baseRef === null) { + throw new Error("Workspace git snapshot is missing required worktree status fields"); + } + + return { + cwd, + isGit: true, + repoRoot: snapshot.git.repoRoot, + mainRepoRoot: snapshot.git.mainRepoRoot, + currentBranch: snapshot.git.currentBranch ?? null, + isDirty: snapshot.git.isDirty, + baseRef: snapshot.git.baseRef, + aheadBehind: snapshot.git.aheadBehind ?? null, + aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, + behindOfOrigin: snapshot.git.behindOfOrigin ?? null, + hasRemote: snapshot.git.hasRemote, + remoteUrl: snapshot.git.remoteUrl, + isPaseoOwnedWorktree: true, + error: null, + requestId, + }; + } + + return { + cwd, + isGit: true, + repoRoot: snapshot.git.repoRoot, + mainRepoRoot: snapshot.git.mainRepoRoot, + currentBranch: snapshot.git.currentBranch ?? null, + isDirty: snapshot.git.isDirty, + baseRef: snapshot.git.baseRef ?? null, + aheadBehind: snapshot.git.aheadBehind ?? null, + aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, + behindOfOrigin: snapshot.git.behindOfOrigin ?? null, + hasRemote: snapshot.git.hasRemote, + remoteUrl: snapshot.git.remoteUrl, + isPaseoOwnedWorktree: false, + error: null, + requestId, + }; +} + +export function buildCheckoutPrStatusPayloadFromSnapshot({ + cwd, + requestId, + snapshot, +}: { + cwd: string; + requestId: string; + snapshot: WorkspaceGitRuntimeSnapshot; +}): CheckoutPrStatusResponse["payload"] { + return { + cwd, + status: normalizeCheckoutPrStatusPayload(snapshot.github.pullRequest), + githubFeaturesEnabled: snapshot.github.featuresEnabled, + error: snapshot.github.error + ? { + code: "UNKNOWN", + message: snapshot.github.error.message, + } + : null, + requestId, + }; +} + +export function normalizeCheckoutPrStatusPayload( + status: WorkspaceGitRuntimeSnapshot["github"]["pullRequest"], +): CheckoutPrStatusPayloadStatus | null { + if (!status) { + return null; + } + return { + number: status.number, + url: status.url, + title: status.title, + state: status.state, + repoOwner: status.repoOwner, + repoName: status.repoName, + baseRefName: status.baseRefName, + headRefName: status.headRefName, + isMerged: status.isMerged, + isDraft: status.isDraft ?? false, + mergeable: status.mergeable ?? "UNKNOWN", + checks: status.checks ?? [], + checksStatus: status.checksStatus, + reviewDecision: status.reviewDecision, + }; +} diff --git a/packages/server/src/server/session.test.ts b/packages/server/src/server/session.test.ts index 35cec7020..f173b0d9e 100644 --- a/packages/server/src/server/session.test.ts +++ b/packages/server/src/server/session.test.ts @@ -6,10 +6,9 @@ import { join } from "path"; import pino from "pino"; import { afterEach, describe, expect, test, vi } from "vitest"; -import { CheckoutPrStatusSchema } from "../shared/messages.js"; import type { WorkspaceDescriptorPayload } from "../shared/messages.js"; import { decodeFileTransferFrame, FileTransferOpcode } from "../shared/binary-frames/index.js"; -import { normalizeCheckoutPrStatusPayload, Session } from "./session.js"; +import { Session } from "./session.js"; import { StructuredAgentFallbackError } from "./agent/agent-response-loop.js"; import type { AgentClient, @@ -1191,40 +1190,6 @@ describe("session agent import", () => { }); }); -describe("session PR status payload normalization", () => { - test("includes repository identity fields on the wire", () => { - const payload = normalizeCheckoutPrStatusPayload({ - number: 123, - repoOwner: "internal-owner", - repoName: "internal-repo", - url: "https://github.com/getpaseo/paseo/pull/123", - title: "Ship PR pane", - state: "open", - baseRefName: "main", - headRefName: "feature/pr-pane", - isMerged: false, - isDraft: true, - mergeable: "MERGEABLE", - checks: [ - { - name: "typecheck", - status: "success", - url: "https://github.com/getpaseo/paseo/actions/runs/1", - workflow: "CI", - duration: "1m 20s", - }, - ], - checksStatus: "success", - reviewDecision: "approved", - }); - - expect(payload).toHaveProperty("repoOwner", "internal-owner"); - expect(payload).toHaveProperty("repoName", "internal-repo"); - expect(payload).toHaveProperty("mergeable", "MERGEABLE"); - expect(CheckoutPrStatusSchema.parse(payload)).toEqual(payload); - }); -}); - describe("session provider refresh cwd routing", () => { test("routes no-cwd provider snapshot refreshes through settings refresh", async () => { const { diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 7532ecbcf..5a5057810 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -20,8 +20,6 @@ import { type FileExplorerRequest, type FileDownloadTokenRequest, type GitSetupOptions, - type CheckoutPrStatusResponse, - type CheckoutStatusResponse, type StartWorkspaceScriptRequest, type CloseItemsRequest, type SubscribeCheckoutDiffRequest, @@ -192,6 +190,10 @@ import { expandTilde } from "../utils/path.js"; import { searchHomeDirectories, searchWorkspaceEntries } from "../utils/directory-suggestions.js"; import { toCheckoutError } from "./checkout-git-utils.js"; import { CheckoutDiffManager } from "./checkout-diff-manager.js"; +import { + buildCheckoutPrStatusPayloadFromSnapshot, + buildCheckoutStatusPayloadFromSnapshot, +} from "./checkout/status-projection.js"; import type { LocalSpeechModelId } from "./speech/providers/local/models.js"; import { toResolver, type Resolvable } from "./speech/provider-resolver.js"; import type { SpeechReadinessSnapshot, SpeechReadinessState } from "./speech/speech-runtime.js"; @@ -636,11 +638,6 @@ export type SessionLifecycleIntent = reason?: string; }; -type CheckoutPrStatusPayload = Extract< - SessionOutboundMessage, - { type: "checkout_pr_status_response" } ->["payload"]; -type CheckoutPrStatusPayloadStatus = NonNullable; type PullRequestTimelinePayload = Extract< SessionOutboundMessage, { type: "pull_request_timeline_response" } @@ -4489,7 +4486,7 @@ export class Session { const snapshot = await this.workspaceGitService.getSnapshot(resolvedCwd); this.emit({ type: "checkout_status_response", - payload: this.buildCheckoutStatusPayloadFromSnapshot({ + payload: buildCheckoutStatusPayloadFromSnapshot({ cwd, requestId, snapshot, @@ -4838,116 +4835,18 @@ export class Session { this.checkoutDiffSubscriptions.delete(msg.subscriptionId); } - private buildCheckoutStatusPayloadFromSnapshot({ - cwd, - requestId, - snapshot, - }: { - cwd: string; - requestId: string; - snapshot: WorkspaceGitRuntimeSnapshot; - }): CheckoutStatusResponse["payload"] { - if (!snapshot.git.isGit) { - return { - cwd, - isGit: false, - repoRoot: null, - currentBranch: null, - isDirty: null, - baseRef: null, - aheadBehind: null, - aheadOfOrigin: null, - behindOfOrigin: null, - hasRemote: false, - remoteUrl: null, - isPaseoOwnedWorktree: false, - error: null, - requestId, - }; - } - - if (snapshot.git.repoRoot === null || snapshot.git.isDirty === null) { - throw new Error("Workspace git snapshot is missing required checkout status fields"); - } - - if (snapshot.git.isPaseoOwnedWorktree) { - if (snapshot.git.mainRepoRoot === null || snapshot.git.baseRef === null) { - throw new Error("Workspace git snapshot is missing required worktree status fields"); - } - - return { - cwd, - isGit: true, - repoRoot: snapshot.git.repoRoot, - mainRepoRoot: snapshot.git.mainRepoRoot, - currentBranch: snapshot.git.currentBranch ?? null, - isDirty: snapshot.git.isDirty, - baseRef: snapshot.git.baseRef, - aheadBehind: snapshot.git.aheadBehind ?? null, - aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, - behindOfOrigin: snapshot.git.behindOfOrigin ?? null, - hasRemote: snapshot.git.hasRemote, - remoteUrl: snapshot.git.remoteUrl, - isPaseoOwnedWorktree: true, - error: null, - requestId, - }; - } - - return { - cwd, - isGit: true, - repoRoot: snapshot.git.repoRoot, - mainRepoRoot: snapshot.git.mainRepoRoot, - currentBranch: snapshot.git.currentBranch ?? null, - isDirty: snapshot.git.isDirty, - baseRef: snapshot.git.baseRef ?? null, - aheadBehind: snapshot.git.aheadBehind ?? null, - aheadOfOrigin: snapshot.git.aheadOfOrigin ?? null, - behindOfOrigin: snapshot.git.behindOfOrigin ?? null, - hasRemote: snapshot.git.hasRemote, - remoteUrl: snapshot.git.remoteUrl, - isPaseoOwnedWorktree: false, - error: null, - requestId, - }; - } - - private buildCheckoutPrStatusPayloadFromSnapshot({ - cwd, - requestId, - snapshot, - }: { - cwd: string; - requestId: string; - snapshot: WorkspaceGitRuntimeSnapshot; - }): CheckoutPrStatusResponse["payload"] { - return { - cwd, - status: normalizeCheckoutPrStatusPayload(snapshot.github.pullRequest), - githubFeaturesEnabled: snapshot.github.featuresEnabled, - error: snapshot.github.error - ? { - code: "UNKNOWN", - message: snapshot.github.error.message, - } - : null, - requestId, - }; - } - private emitCheckoutStatusUpdate(cwd: string, snapshot: WorkspaceGitRuntimeSnapshot): void { try { const requestId = `subscription:${cwd}`; this.emit({ type: "checkout_status_update", payload: { - ...this.buildCheckoutStatusPayloadFromSnapshot({ + ...buildCheckoutStatusPayloadFromSnapshot({ cwd, requestId, snapshot, }), - prStatus: this.buildCheckoutPrStatusPayloadFromSnapshot({ + prStatus: buildCheckoutPrStatusPayloadFromSnapshot({ cwd, requestId, snapshot, @@ -5382,18 +5281,11 @@ export class Session { const snapshot = await this.workspaceGitService.getSnapshot(cwd); this.emit({ type: "checkout_pr_status_response", - payload: { + payload: buildCheckoutPrStatusPayloadFromSnapshot({ cwd, - status: normalizeCheckoutPrStatusPayload(snapshot.github.pullRequest), - githubFeaturesEnabled: snapshot.github.featuresEnabled, - error: snapshot.github.error - ? { - code: "UNKNOWN", - message: snapshot.github.error.message, - } - : null, requestId, - }, + snapshot, + }), }); } catch (error) { this.emit({ @@ -8895,30 +8787,6 @@ export class Session { } } -export function normalizeCheckoutPrStatusPayload( - status: WorkspaceGitRuntimeSnapshot["github"]["pullRequest"], -): CheckoutPrStatusPayloadStatus | null { - if (!status) { - return null; - } - return { - number: status.number, - url: status.url, - title: status.title, - state: status.state, - repoOwner: status.repoOwner, - repoName: status.repoName, - baseRefName: status.baseRefName, - headRefName: status.headRefName, - isMerged: status.isMerged, - isDraft: status.isDraft ?? false, - mergeable: status.mergeable ?? "UNKNOWN", - checks: status.checks ?? [], - checksStatus: status.checksStatus, - reviewDecision: status.reviewDecision, - }; -} - function isValidPullRequestTimelineIdentity(options: { prNumber: number; repoOwner: string;