mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-04 21:23:07 +00:00
Merge pull request #562 from AutoMaker-Org/feature/v0.12.0rc-1768688900786-5ea1
refactor: standardize PR state representation across the application
This commit is contained in:
@@ -5,18 +5,14 @@
|
|||||||
|
|
||||||
import * as secureFs from './secure-fs.js';
|
import * as secureFs from './secure-fs.js';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
import type { PRState, WorktreePRInfo } from '@automaker/types';
|
||||||
|
|
||||||
|
// Re-export types for backwards compatibility
|
||||||
|
export type { PRState, WorktreePRInfo };
|
||||||
|
|
||||||
/** Maximum length for sanitized branch names in filesystem paths */
|
/** Maximum length for sanitized branch names in filesystem paths */
|
||||||
const MAX_SANITIZED_BRANCH_PATH_LENGTH = 200;
|
const MAX_SANITIZED_BRANCH_PATH_LENGTH = 200;
|
||||||
|
|
||||||
export interface WorktreePRInfo {
|
|
||||||
number: number;
|
|
||||||
url: string;
|
|
||||||
title: string;
|
|
||||||
state: string;
|
|
||||||
createdAt: string;
|
|
||||||
}
|
|
||||||
|
|
||||||
export interface WorktreeMetadata {
|
export interface WorktreeMetadata {
|
||||||
branch: string;
|
branch: string;
|
||||||
createdAt: string;
|
createdAt: string;
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import {
|
|||||||
} from '../common.js';
|
} from '../common.js';
|
||||||
import { updateWorktreePRInfo } from '../../../lib/worktree-metadata.js';
|
import { updateWorktreePRInfo } from '../../../lib/worktree-metadata.js';
|
||||||
import { createLogger } from '@automaker/utils';
|
import { createLogger } from '@automaker/utils';
|
||||||
|
import { validatePRState } from '@automaker/types';
|
||||||
|
|
||||||
const logger = createLogger('CreatePR');
|
const logger = createLogger('CreatePR');
|
||||||
|
|
||||||
@@ -268,11 +269,12 @@ export function createCreatePRHandler() {
|
|||||||
prAlreadyExisted = true;
|
prAlreadyExisted = true;
|
||||||
|
|
||||||
// Store the existing PR info in metadata
|
// Store the existing PR info in metadata
|
||||||
|
// GitHub CLI returns uppercase states: OPEN, MERGED, CLOSED
|
||||||
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
||||||
number: existingPr.number,
|
number: existingPr.number,
|
||||||
url: existingPr.url,
|
url: existingPr.url,
|
||||||
title: existingPr.title || title,
|
title: existingPr.title || title,
|
||||||
state: existingPr.state || 'open',
|
state: validatePRState(existingPr.state),
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
});
|
});
|
||||||
logger.debug(
|
logger.debug(
|
||||||
@@ -319,11 +321,12 @@ export function createCreatePRHandler() {
|
|||||||
|
|
||||||
if (prNumber) {
|
if (prNumber) {
|
||||||
try {
|
try {
|
||||||
|
// Note: GitHub doesn't have a 'DRAFT' state - drafts still show as 'OPEN'
|
||||||
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
||||||
number: prNumber,
|
number: prNumber,
|
||||||
url: prUrl,
|
url: prUrl,
|
||||||
title,
|
title,
|
||||||
state: draft ? 'draft' : 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
});
|
});
|
||||||
logger.debug(`Stored PR info for branch ${branchName}: PR #${prNumber}`);
|
logger.debug(`Stored PR info for branch ${branchName}: PR #${prNumber}`);
|
||||||
@@ -352,11 +355,12 @@ export function createCreatePRHandler() {
|
|||||||
prNumber = existingPr.number;
|
prNumber = existingPr.number;
|
||||||
prAlreadyExisted = true;
|
prAlreadyExisted = true;
|
||||||
|
|
||||||
|
// GitHub CLI returns uppercase states: OPEN, MERGED, CLOSED
|
||||||
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
await updateWorktreePRInfo(effectiveProjectPath, branchName, {
|
||||||
number: existingPr.number,
|
number: existingPr.number,
|
||||||
url: existingPr.url,
|
url: existingPr.url,
|
||||||
title: existingPr.title || title,
|
title: existingPr.title || title,
|
||||||
state: existingPr.state || 'open',
|
state: validatePRState(existingPr.state),
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
});
|
});
|
||||||
logger.debug(`Fetched and stored existing PR: #${existingPr.number}`);
|
logger.debug(`Fetched and stored existing PR: #${existingPr.number}`);
|
||||||
|
|||||||
@@ -14,8 +14,13 @@ import path from 'path';
|
|||||||
import * as secureFs from '../../../lib/secure-fs.js';
|
import * as secureFs from '../../../lib/secure-fs.js';
|
||||||
import { isGitRepo } from '@automaker/git-utils';
|
import { isGitRepo } from '@automaker/git-utils';
|
||||||
import { getErrorMessage, logError, normalizePath, execEnv, isGhCliAvailable } from '../common.js';
|
import { getErrorMessage, logError, normalizePath, execEnv, isGhCliAvailable } from '../common.js';
|
||||||
import { readAllWorktreeMetadata, type WorktreePRInfo } from '../../../lib/worktree-metadata.js';
|
import {
|
||||||
|
readAllWorktreeMetadata,
|
||||||
|
updateWorktreePRInfo,
|
||||||
|
type WorktreePRInfo,
|
||||||
|
} from '../../../lib/worktree-metadata.js';
|
||||||
import { createLogger } from '@automaker/utils';
|
import { createLogger } from '@automaker/utils';
|
||||||
|
import { validatePRState } from '@automaker/types';
|
||||||
import {
|
import {
|
||||||
checkGitHubRemote,
|
checkGitHubRemote,
|
||||||
type GitHubRemoteStatus,
|
type GitHubRemoteStatus,
|
||||||
@@ -168,8 +173,11 @@ async function getGitHubRemoteStatus(projectPath: string): Promise<GitHubRemoteS
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fetch open PRs from GitHub and create a map of branch name to PR info.
|
* Fetch all PRs from GitHub and create a map of branch name to PR info.
|
||||||
* This allows detecting PRs that were created outside the app.
|
* Uses --state all to include merged/closed PRs, allowing detection of
|
||||||
|
* state changes (e.g., when a PR is merged on GitHub).
|
||||||
|
*
|
||||||
|
* This also allows detecting PRs that were created outside the app.
|
||||||
*
|
*
|
||||||
* Uses cached GitHub remote status to avoid repeated warnings when the
|
* Uses cached GitHub remote status to avoid repeated warnings when the
|
||||||
* project doesn't have a GitHub remote configured.
|
* project doesn't have a GitHub remote configured.
|
||||||
@@ -192,9 +200,9 @@ async function fetchGitHubPRs(projectPath: string): Promise<Map<string, Worktree
|
|||||||
? `-R ${remoteStatus.owner}/${remoteStatus.repo}`
|
? `-R ${remoteStatus.owner}/${remoteStatus.repo}`
|
||||||
: '';
|
: '';
|
||||||
|
|
||||||
// Fetch open PRs from GitHub
|
// Fetch all PRs from GitHub (including merged/closed to detect state changes)
|
||||||
const { stdout } = await execAsync(
|
const { stdout } = await execAsync(
|
||||||
`gh pr list ${repoFlag} --state open --json number,title,url,state,headRefName,createdAt --limit 1000`,
|
`gh pr list ${repoFlag} --state all --json number,title,url,state,headRefName,createdAt --limit 1000`,
|
||||||
{ cwd: projectPath, env: execEnv, timeout: 15000 }
|
{ cwd: projectPath, env: execEnv, timeout: 15000 }
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -212,7 +220,8 @@ async function fetchGitHubPRs(projectPath: string): Promise<Map<string, Worktree
|
|||||||
number: pr.number,
|
number: pr.number,
|
||||||
url: pr.url,
|
url: pr.url,
|
||||||
title: pr.title,
|
title: pr.title,
|
||||||
state: pr.state,
|
// GitHub CLI returns state as uppercase: OPEN, MERGED, CLOSED
|
||||||
|
state: validatePRState(pr.state),
|
||||||
createdAt: pr.createdAt,
|
createdAt: pr.createdAt,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -351,23 +360,36 @@ export function createListHandler() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Add PR info from metadata or GitHub for each worktree
|
// Assign PR info to each worktree, preferring fresh GitHub data over cached metadata.
|
||||||
// Only fetch GitHub PRs if includeDetails is requested (performance optimization)
|
// Only fetch GitHub PRs if includeDetails is requested (performance optimization).
|
||||||
|
// Uses --state all to detect merged/closed PRs, limited to 1000 recent PRs.
|
||||||
const githubPRs = includeDetails
|
const githubPRs = includeDetails
|
||||||
? await fetchGitHubPRs(projectPath)
|
? await fetchGitHubPRs(projectPath)
|
||||||
: new Map<string, WorktreePRInfo>();
|
: new Map<string, WorktreePRInfo>();
|
||||||
|
|
||||||
for (const worktree of worktrees) {
|
for (const worktree of worktrees) {
|
||||||
const metadata = allMetadata.get(worktree.branch);
|
const metadata = allMetadata.get(worktree.branch);
|
||||||
if (metadata?.pr) {
|
const githubPR = githubPRs.get(worktree.branch);
|
||||||
// Use stored metadata (more complete info)
|
|
||||||
worktree.pr = metadata.pr;
|
if (githubPR) {
|
||||||
} else if (includeDetails) {
|
// Prefer fresh GitHub data (it has the current state)
|
||||||
// Fall back to GitHub PR detection only when includeDetails is requested
|
worktree.pr = githubPR;
|
||||||
const githubPR = githubPRs.get(worktree.branch);
|
|
||||||
if (githubPR) {
|
// Sync metadata with GitHub state when:
|
||||||
worktree.pr = githubPR;
|
// 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;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 123,
|
number: 123,
|
||||||
url: 'https://github.com/owner/repo/pull/123',
|
url: 'https://github.com/owner/repo/pull/123',
|
||||||
title: 'Test PR',
|
title: 'Test PR',
|
||||||
state: 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
@@ -158,7 +158,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 456,
|
number: 456,
|
||||||
url: 'https://github.com/owner/repo/pull/456',
|
url: 'https://github.com/owner/repo/pull/456',
|
||||||
title: 'Updated PR',
|
title: 'Updated PR',
|
||||||
state: 'closed',
|
state: 'CLOSED',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
@@ -177,7 +177,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 789,
|
number: 789,
|
||||||
url: 'https://github.com/owner/repo/pull/789',
|
url: 'https://github.com/owner/repo/pull/789',
|
||||||
title: 'New PR',
|
title: 'New PR',
|
||||||
state: 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -201,7 +201,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 999,
|
number: 999,
|
||||||
url: 'https://github.com/owner/repo/pull/999',
|
url: 'https://github.com/owner/repo/pull/999',
|
||||||
title: 'Updated PR',
|
title: 'Updated PR',
|
||||||
state: 'merged',
|
state: 'MERGED',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -224,7 +224,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 111,
|
number: 111,
|
||||||
url: 'https://github.com/owner/repo/pull/111',
|
url: 'https://github.com/owner/repo/pull/111',
|
||||||
title: 'PR',
|
title: 'PR',
|
||||||
state: 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -259,7 +259,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 222,
|
number: 222,
|
||||||
url: 'https://github.com/owner/repo/pull/222',
|
url: 'https://github.com/owner/repo/pull/222',
|
||||||
title: 'Has PR',
|
title: 'Has PR',
|
||||||
state: 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -297,7 +297,7 @@ describe('worktree-metadata.ts', () => {
|
|||||||
number: 333,
|
number: 333,
|
||||||
url: 'https://github.com/owner/repo/pull/333',
|
url: 'https://github.com/owner/repo/pull/333',
|
||||||
title: 'PR 3',
|
title: 'PR 3',
|
||||||
state: 'open',
|
state: 'OPEN',
|
||||||
createdAt: new Date().toISOString(),
|
createdAt: new Date().toISOString(),
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1,10 +1,6 @@
|
|||||||
export interface WorktreePRInfo {
|
// Re-export shared types from @automaker/types
|
||||||
number: number;
|
export type { PRState, WorktreePRInfo } from '@automaker/types';
|
||||||
url: string;
|
import type { PRState, WorktreePRInfo } from '@automaker/types';
|
||||||
title: string;
|
|
||||||
state: string;
|
|
||||||
createdAt: string;
|
|
||||||
}
|
|
||||||
|
|
||||||
export interface WorktreeInfo {
|
export interface WorktreeInfo {
|
||||||
path: string;
|
path: string;
|
||||||
@@ -43,7 +39,8 @@ export interface PRInfo {
|
|||||||
number: number;
|
number: number;
|
||||||
title: string;
|
title: string;
|
||||||
url: string;
|
url: string;
|
||||||
state: string;
|
/** PR state: OPEN, MERGED, or CLOSED */
|
||||||
|
state: PRState;
|
||||||
author: string;
|
author: string;
|
||||||
body: string;
|
body: string;
|
||||||
comments: Array<{
|
comments: Array<{
|
||||||
|
|||||||
@@ -292,3 +292,7 @@ export type {
|
|||||||
EventReplayHookResult,
|
EventReplayHookResult,
|
||||||
} from './event-history.js';
|
} from './event-history.js';
|
||||||
export { EVENT_HISTORY_VERSION, DEFAULT_EVENT_HISTORY_INDEX } from './event-history.js';
|
export { EVENT_HISTORY_VERSION, DEFAULT_EVENT_HISTORY_INDEX } from './event-history.js';
|
||||||
|
|
||||||
|
// Worktree and PR types
|
||||||
|
export type { PRState, WorktreePRInfo } from './worktree.js';
|
||||||
|
export { PR_STATES, validatePRState } from './worktree.js';
|
||||||
|
|||||||
32
libs/types/src/worktree.ts
Normal file
32
libs/types/src/worktree.ts
Normal file
@@ -0,0 +1,32 @@
|
|||||||
|
/**
|
||||||
|
* Worktree and PR-related types
|
||||||
|
* Shared across server and UI components
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** GitHub PR states as returned by the GitHub API (uppercase) */
|
||||||
|
export type PRState = 'OPEN' | 'MERGED' | 'CLOSED';
|
||||||
|
|
||||||
|
/** Valid PR states for validation */
|
||||||
|
export const PR_STATES: readonly PRState[] = ['OPEN', 'MERGED', 'CLOSED'] as const;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates a PR state value from external APIs (e.g., GitHub CLI).
|
||||||
|
* Returns the validated state if it matches a known PRState, otherwise returns 'OPEN' as default.
|
||||||
|
* This is safer than type assertions as it handles unexpected values from external APIs.
|
||||||
|
*
|
||||||
|
* @param state - The state string to validate (can be any string)
|
||||||
|
* @returns A valid PRState value
|
||||||
|
*/
|
||||||
|
export function validatePRState(state: string | undefined | null): PRState {
|
||||||
|
return PR_STATES.find((s) => s === state) ?? 'OPEN';
|
||||||
|
}
|
||||||
|
|
||||||
|
/** PR information stored in worktree metadata */
|
||||||
|
export interface WorktreePRInfo {
|
||||||
|
number: number;
|
||||||
|
url: string;
|
||||||
|
title: string;
|
||||||
|
/** PR state: OPEN, MERGED, or CLOSED */
|
||||||
|
state: PRState;
|
||||||
|
createdAt: string;
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user