From 5b1e0105f4b6441d0e6e2d9e4b846c4583228a73 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sat, 17 Jan 2026 23:58:19 +0100 Subject: [PATCH] refactor: standardize PR state representation across the application Updated the PR state handling to use a consistent uppercase format ('OPEN', 'MERGED', 'CLOSED') throughout the codebase. This includes changes to the worktree metadata interface, PR creation logic, and related tests to ensure uniformity and prevent potential mismatches in state representation. Additionally, modified the GitHub PR fetching logic to retrieve all PR states, allowing for better detection of state changes. This refactor enhances clarity and consistency in how PR states are managed and displayed. --- apps/server/src/lib/worktree-metadata.ts | 6 ++- .../src/routes/worktree/routes/create-pr.ts | 9 ++-- .../server/src/routes/worktree/routes/list.ts | 54 +++++++++++++------ .../tests/unit/lib/worktree-metadata.test.ts | 14 ++--- .../views/board-view/worktree-panel/types.ts | 9 +++- 5 files changed, 63 insertions(+), 29 deletions(-) diff --git a/apps/server/src/lib/worktree-metadata.ts b/apps/server/src/lib/worktree-metadata.ts index 3f7ea60d..ab0ba67c 100644 --- a/apps/server/src/lib/worktree-metadata.ts +++ b/apps/server/src/lib/worktree-metadata.ts @@ -9,11 +9,15 @@ import * as path from 'path'; /** Maximum length for sanitized branch names in filesystem paths */ const MAX_SANITIZED_BRANCH_PATH_LENGTH = 200; +/** GitHub PR states as returned by the GitHub API */ +export type PRState = 'OPEN' | 'MERGED' | 'CLOSED'; + export interface WorktreePRInfo { number: number; url: string; title: string; - state: string; + /** PR state: OPEN, MERGED, or CLOSED */ + state: PRState; createdAt: string; } diff --git a/apps/server/src/routes/worktree/routes/create-pr.ts b/apps/server/src/routes/worktree/routes/create-pr.ts index 1bde9448..25211854 100644 --- a/apps/server/src/routes/worktree/routes/create-pr.ts +++ b/apps/server/src/routes/worktree/routes/create-pr.ts @@ -268,11 +268,12 @@ export function createCreatePRHandler() { prAlreadyExisted = true; // Store the existing PR info in metadata + // GitHub CLI returns uppercase states: OPEN, MERGED, CLOSED await updateWorktreePRInfo(effectiveProjectPath, branchName, { number: existingPr.number, url: existingPr.url, title: existingPr.title || title, - state: existingPr.state || 'open', + state: existingPr.state || 'OPEN', createdAt: new Date().toISOString(), }); logger.debug( @@ -319,11 +320,12 @@ export function createCreatePRHandler() { if (prNumber) { try { + // Note: GitHub doesn't have a 'DRAFT' state - drafts still show as 'OPEN' await updateWorktreePRInfo(effectiveProjectPath, branchName, { number: prNumber, url: prUrl, title, - state: draft ? 'draft' : 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }); logger.debug(`Stored PR info for branch ${branchName}: PR #${prNumber}`); @@ -352,11 +354,12 @@ export function createCreatePRHandler() { prNumber = existingPr.number; prAlreadyExisted = true; + // GitHub CLI returns uppercase states: OPEN, MERGED, CLOSED await updateWorktreePRInfo(effectiveProjectPath, branchName, { number: existingPr.number, url: existingPr.url, title: existingPr.title || title, - state: existingPr.state || 'open', + state: existingPr.state || 'OPEN', createdAt: new Date().toISOString(), }); logger.debug(`Fetched and stored existing PR: #${existingPr.number}`); diff --git a/apps/server/src/routes/worktree/routes/list.ts b/apps/server/src/routes/worktree/routes/list.ts index 96782d64..a911fce7 100644 --- a/apps/server/src/routes/worktree/routes/list.ts +++ b/apps/server/src/routes/worktree/routes/list.ts @@ -14,7 +14,12 @@ import path from 'path'; import * as secureFs from '../../../lib/secure-fs.js'; import { isGitRepo } from '@automaker/git-utils'; import { getErrorMessage, logError, normalizePath, execEnv, isGhCliAvailable } from '../common.js'; -import { readAllWorktreeMetadata, type WorktreePRInfo } from '../../../lib/worktree-metadata.js'; +import { + readAllWorktreeMetadata, + updateWorktreePRInfo, + type WorktreePRInfo, + type PRState, +} from '../../../lib/worktree-metadata.js'; import { createLogger } from '@automaker/utils'; import { checkGitHubRemote, @@ -168,8 +173,11 @@ async function getGitHubRemoteStatus(projectPath: string): Promise(); for (const worktree of worktrees) { const metadata = allMetadata.get(worktree.branch); - if (metadata?.pr) { - // Use stored metadata (more complete info) - worktree.pr = metadata.pr; - } else if (includeDetails) { - // Fall back to GitHub PR detection only when includeDetails is requested - const githubPR = githubPRs.get(worktree.branch); - if (githubPR) { - worktree.pr = githubPR; + const githubPR = githubPRs.get(worktree.branch); + + if (githubPR) { + // Prefer fresh GitHub data (it has the current state) + worktree.pr = githubPR; + + // Sync metadata with GitHub state when: + // 1. No metadata exists for this PR (PR created externally) + // 2. State has changed (e.g., merged/closed on GitHub) + const needsSync = !metadata?.pr || metadata.pr.state !== githubPR.state; + if (needsSync) { + // Fire and forget - don't block the response + updateWorktreePRInfo(projectPath, worktree.branch, githubPR).catch((err) => { + logger.warn( + `Failed to update PR info for ${worktree.branch}: ${getErrorMessage(err)}` + ); + }); } + } else if (metadata?.pr) { + // Fall back to stored metadata (for PRs not in recent GitHub response) + worktree.pr = metadata.pr; } } diff --git a/apps/server/tests/unit/lib/worktree-metadata.test.ts b/apps/server/tests/unit/lib/worktree-metadata.test.ts index ab7967f3..2f84af88 100644 --- a/apps/server/tests/unit/lib/worktree-metadata.test.ts +++ b/apps/server/tests/unit/lib/worktree-metadata.test.ts @@ -121,7 +121,7 @@ describe('worktree-metadata.ts', () => { number: 123, url: 'https://github.com/owner/repo/pull/123', title: 'Test PR', - state: 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }, }; @@ -158,7 +158,7 @@ describe('worktree-metadata.ts', () => { number: 456, url: 'https://github.com/owner/repo/pull/456', title: 'Updated PR', - state: 'closed', + state: 'CLOSED', createdAt: new Date().toISOString(), }, }; @@ -177,7 +177,7 @@ describe('worktree-metadata.ts', () => { number: 789, url: 'https://github.com/owner/repo/pull/789', title: 'New PR', - state: 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }; @@ -201,7 +201,7 @@ describe('worktree-metadata.ts', () => { number: 999, url: 'https://github.com/owner/repo/pull/999', title: 'Updated PR', - state: 'merged', + state: 'MERGED', createdAt: new Date().toISOString(), }; @@ -224,7 +224,7 @@ describe('worktree-metadata.ts', () => { number: 111, url: 'https://github.com/owner/repo/pull/111', title: 'PR', - state: 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }; @@ -259,7 +259,7 @@ describe('worktree-metadata.ts', () => { number: 222, url: 'https://github.com/owner/repo/pull/222', title: 'Has PR', - state: 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }; @@ -297,7 +297,7 @@ describe('worktree-metadata.ts', () => { number: 333, url: 'https://github.com/owner/repo/pull/333', title: 'PR 3', - state: 'open', + state: 'OPEN', createdAt: new Date().toISOString(), }, }; diff --git a/apps/ui/src/components/views/board-view/worktree-panel/types.ts b/apps/ui/src/components/views/board-view/worktree-panel/types.ts index d2040048..36aa2da7 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/types.ts +++ b/apps/ui/src/components/views/board-view/worktree-panel/types.ts @@ -1,8 +1,12 @@ +/** GitHub PR states as returned by the GitHub API */ +export type PRState = 'OPEN' | 'MERGED' | 'CLOSED'; + export interface WorktreePRInfo { number: number; url: string; title: string; - state: string; + /** PR state: OPEN, MERGED, or CLOSED */ + state: PRState; createdAt: string; } @@ -43,7 +47,8 @@ export interface PRInfo { number: number; title: string; url: string; - state: string; + /** PR state: OPEN, MERGED, or CLOSED */ + state: PRState; author: string; body: string; comments: Array<{