fix: make gh pr merge one-command guarded merge (#654)
Co-authored-by: Codex <codex@noreply.local>
This commit is contained in:
Lyon
@@ -271,7 +271,9 @@ bun scripts/cli.ts gh pr merge <number> \
|
||||
[--delete-branch] [--dry-run]
|
||||
```
|
||||
|
||||
guarded merge：先做 closeout 预检，拒绝非 ready PR。已 merged 返回 `alreadyMerged=true`。
|
||||
一键 guarded merge：`gh pr merge` 自己读取 closeout metadata、mergeability、mergeStateStatus 和 status checks，不要求用户先跑 `gh pr preflight`。`preflight`/`closeout` 只作为只读诊断入口保留。
|
||||
|
||||
当 GitHub GraphQL 返回 `mergeable=UNKNOWN/null`、`mergeStateStatus=UNKNOWN/null` 或 status rollup unknown 且没有明确 blockers 时，`gh pr merge` 会按 `config/unidesk-cli.yaml` 的 `github.prMerge.unknownRetry` 做指数退避重试，并在默认表格 `RETRY` 列显示 `1/5`、`2/5` 等尝试次数。重试耗尽仍 unknown 时，命令自动停止且不合并；冲突、draft、失败检查、pending check 等不可合并状态不会无意义重试。已 merged 返回 `alreadyMerged=true`。
|
||||
|
||||
### 编辑 / 评论 / 关闭 / 重开
|
||||
|
||||
|
||||
@@ -5,3 +5,10 @@ output:
|
||||
dumpDir: /tmp/unidesk-cli-output
|
||||
includePreview: false
|
||||
warning: "CLI stdout exceeded YAML-configured limit; full output was dumped to /tmp for one-off drill-down only. This is a CLI usability defect: improve the command itself to print concise tables/summaries and id-specific progressive disclosure instead of repeatedly depending on dump extraction."
|
||||
github:
|
||||
prMerge:
|
||||
unknownRetry:
|
||||
maxAttempts: 5
|
||||
initialDelayMs: 1000
|
||||
maxDelayMs: 16000
|
||||
factor: 2
|
||||
|
||||
+156
-11
@@ -87,6 +87,7 @@ const COMMANDER_BRIEF_UPDATE_HEADING_PATTERNS = [
|
||||
/^##\s+\d{4}-\d{2}-\d{2}\s+\d{1,2}:\d{2}\s+北京时间指挥更新\s*$/u,
|
||||
/^###\s+\d{4}-\d{2}-\d{2}\s+\d{1,2}:\d{2}\s+CST[:：].*$/u,
|
||||
] as const;
|
||||
const UNIDESK_CLI_CONFIG_PATH = "config/unidesk-cli.yaml";
|
||||
|
||||
type IssueViewJsonField = typeof ISSUE_VIEW_JSON_FIELDS[number];
|
||||
type IssueListJsonField = typeof ISSUE_LIST_JSON_FIELDS[number];
|
||||
@@ -134,6 +135,13 @@ type RunnerDisposition = "infra-blocked" | "business-failed";
|
||||
type ClaudeQqTargetType = "private" | "group";
|
||||
type CommanderBriefDiffMode = "identical" | "append-only" | "heading-diff" | "unreliable";
|
||||
|
||||
interface PrMergeUnknownRetryConfig {
|
||||
maxAttempts: number;
|
||||
initialDelayMs: number;
|
||||
maxDelayMs: number;
|
||||
factor: number;
|
||||
}
|
||||
|
||||
interface CommanderBriefDiff {
|
||||
ok: boolean;
|
||||
mode: CommanderBriefDiffMode;
|
||||
@@ -5094,6 +5102,87 @@ function prCloseoutSummary(
|
||||
};
|
||||
}
|
||||
|
||||
function stripYamlComment(value: string): string {
|
||||
const index = value.indexOf("#");
|
||||
return (index >= 0 ? value.slice(0, index) : value).trim();
|
||||
}
|
||||
|
||||
function yamlScalarByPath(text: string, keyPath: string[]): string | null {
|
||||
const stack: Array<{ indent: number; key: string }> = [];
|
||||
for (const line of text.split(/\r?\n/u)) {
|
||||
if (line.trim().length === 0 || line.trimStart().startsWith("#")) continue;
|
||||
const match = /^(\s*)([A-Za-z0-9_-]+):(?:\s*(.*))?$/u.exec(line);
|
||||
if (match === null) continue;
|
||||
const indent = match[1].length;
|
||||
while (stack.length > 0 && stack[stack.length - 1].indent >= indent) stack.pop();
|
||||
const key = match[2];
|
||||
const rawValue = stripYamlComment(match[3] ?? "");
|
||||
const currentPath = [...stack.map((item) => item.key), key];
|
||||
if (currentPath.length === keyPath.length && currentPath.every((item, index) => item === keyPath[index])) {
|
||||
return rawValue.length === 0 ? null : rawValue;
|
||||
}
|
||||
stack.push({ indent, key });
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
function yamlNumberByPath(text: string, keyPath: string[], configPath: string): number {
|
||||
const value = yamlScalarByPath(text, keyPath);
|
||||
if (value === null) throw new Error(`${configPath} missing numeric config ${keyPath.join(".")}`);
|
||||
const parsed = Number(value);
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) throw new Error(`${configPath} config ${keyPath.join(".")} must be a positive number`);
|
||||
return parsed;
|
||||
}
|
||||
|
||||
function loadPrMergeUnknownRetryConfig(): PrMergeUnknownRetryConfig {
|
||||
const configPath = path.resolve(process.cwd(), UNIDESK_CLI_CONFIG_PATH);
|
||||
if (!existsSync(configPath)) throw new Error(`${UNIDESK_CLI_CONFIG_PATH} is required for gh pr merge retry policy`);
|
||||
const text = readFileSync(configPath, "utf8");
|
||||
const prefix = ["github", "prMerge", "unknownRetry"];
|
||||
const config = {
|
||||
maxAttempts: Math.trunc(yamlNumberByPath(text, [...prefix, "maxAttempts"], UNIDESK_CLI_CONFIG_PATH)),
|
||||
initialDelayMs: Math.trunc(yamlNumberByPath(text, [...prefix, "initialDelayMs"], UNIDESK_CLI_CONFIG_PATH)),
|
||||
maxDelayMs: Math.trunc(yamlNumberByPath(text, [...prefix, "maxDelayMs"], UNIDESK_CLI_CONFIG_PATH)),
|
||||
factor: yamlNumberByPath(text, [...prefix, "factor"], UNIDESK_CLI_CONFIG_PATH),
|
||||
};
|
||||
if (config.maxAttempts < 1) throw new Error(`${UNIDESK_CLI_CONFIG_PATH} github.prMerge.unknownRetry.maxAttempts must be >= 1`);
|
||||
if (config.maxDelayMs < config.initialDelayMs) throw new Error(`${UNIDESK_CLI_CONFIG_PATH} github.prMerge.unknownRetry.maxDelayMs must be >= initialDelayMs`);
|
||||
if (config.factor < 1) throw new Error(`${UNIDESK_CLI_CONFIG_PATH} github.prMerge.unknownRetry.factor must be >= 1`);
|
||||
return config;
|
||||
}
|
||||
|
||||
function sleepMs(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
function mergeabilityHasUnknownPending(mergeability: Record<string, unknown>): boolean {
|
||||
const blockers = Array.isArray(mergeability.blockers) ? mergeability.blockers.map(String) : [];
|
||||
if (blockers.length > 0) return false;
|
||||
const pending = Array.isArray(mergeability.pending) ? mergeability.pending.map((item) => item.toLowerCase()) : [];
|
||||
return pending.some((item) => item.includes("unknown"));
|
||||
}
|
||||
|
||||
function nextPrMergeRetryDelayMs(config: PrMergeUnknownRetryConfig, previousDelayMs: number): number {
|
||||
return Math.min(config.maxDelayMs, Math.max(1, Math.ceil(previousDelayMs * config.factor)));
|
||||
}
|
||||
|
||||
function prMergeMethodFlag(method: unknown): string {
|
||||
if (method === "squash") return "--squash";
|
||||
if (method === "rebase") return "--rebase";
|
||||
return "--merge";
|
||||
}
|
||||
|
||||
function prMergeRetryCommand(repo: string, number: unknown, method: unknown, deleteBranch: unknown): string {
|
||||
return [
|
||||
"bun scripts/cli.ts gh pr merge",
|
||||
ghText(number),
|
||||
"--repo",
|
||||
repo,
|
||||
prMergeMethodFlag(method),
|
||||
deleteBranch === true ? "--delete-branch" : "",
|
||||
].filter((item) => item.length > 0).join(" ");
|
||||
}
|
||||
|
||||
async function deleteHeadBranchAfterMerge(repo: string, token: string, pr: GitHubPullRequest): Promise<Record<string, unknown>> {
|
||||
const { owner, name } = repoParts(repo);
|
||||
const headRepo = pr.head?.repo?.full_name ?? null;
|
||||
@@ -5127,6 +5216,12 @@ async function deleteHeadBranchAfterMerge(repo: string, token: string, pr: GitHu
|
||||
|
||||
async function prMerge(repo: string, token: string, number: number, options: GitHubOptions): Promise<GitHubCommandResult> {
|
||||
const { owner, name } = repoParts(repo);
|
||||
let retryConfig: PrMergeUnknownRetryConfig;
|
||||
try {
|
||||
retryConfig = loadPrMergeUnknownRetryConfig();
|
||||
} catch (error) {
|
||||
return validationError("pr merge", repo, error instanceof Error ? error.message : String(error), { number, configPath: UNIDESK_CLI_CONFIG_PATH });
|
||||
}
|
||||
const pr = await githubRequest<GitHubPullRequest>(token, "GET", `/repos/${owner}/${name}/pulls/${number}`);
|
||||
if (isGitHubError(pr)) return commandError("pr merge", repo, pr, { number, phase: "fetch-pr" });
|
||||
const summary = prSummary(pr);
|
||||
@@ -5143,19 +5238,54 @@ async function prMerge(repo: string, token: string, number: number, options: Git
|
||||
rest: true,
|
||||
});
|
||||
}
|
||||
const metadata = await prGraphqlMetadata(repo, token, number);
|
||||
if (isGitHubError(metadata)) return commandError("pr merge", repo, metadata, { number, phase: "fetch-pr-closeout-metadata", pullRequest: summary });
|
||||
const statusChecks = statusRollupSummary(repo, number, metadata.statusCheckRollup, false);
|
||||
const mergeability = prCloseoutSummary(summary, prMetadataSummary(metadata), statusChecks);
|
||||
let metadata: GitHubPullRequestGraphqlMetadata | GitHubErrorPayload | null = null;
|
||||
let statusChecks: Record<string, unknown> = {};
|
||||
let mergeability: Record<string, unknown> = {};
|
||||
let delayMs = retryConfig.initialDelayMs;
|
||||
const retryAttempts: Array<Record<string, unknown>> = [];
|
||||
for (let attempt = 1; attempt <= retryConfig.maxAttempts; attempt += 1) {
|
||||
metadata = await prGraphqlMetadata(repo, token, number);
|
||||
if (isGitHubError(metadata)) return commandError("pr merge", repo, metadata, { number, phase: "fetch-pr-closeout-metadata", pullRequest: summary, retry: { attempt, maxAttempts: retryConfig.maxAttempts } });
|
||||
statusChecks = statusRollupSummary(repo, number, metadata.statusCheckRollup, false);
|
||||
mergeability = prCloseoutSummary(summary, prMetadataSummary(metadata), statusChecks);
|
||||
const shouldRetryUnknown = mergeability.readyForCommanderMerge !== true && mergeabilityHasUnknownPending(mergeability);
|
||||
const waitMs = shouldRetryUnknown && attempt < retryConfig.maxAttempts ? delayMs : 0;
|
||||
retryAttempts.push({
|
||||
attempt,
|
||||
maxAttempts: retryConfig.maxAttempts,
|
||||
mergeable: metadata.mergeable ?? null,
|
||||
mergeStateStatus: metadata.mergeStateStatus ?? null,
|
||||
statusCheckRollup: metadata.statusCheckRollup?.state ?? null,
|
||||
conclusion: mergeability.conclusion ?? null,
|
||||
blockers: Array.isArray(mergeability.blockers) ? mergeability.blockers : [],
|
||||
pending: Array.isArray(mergeability.pending) ? mergeability.pending : [],
|
||||
waitMs,
|
||||
});
|
||||
if (mergeability.readyForCommanderMerge === true || !shouldRetryUnknown || attempt >= retryConfig.maxAttempts) break;
|
||||
await sleepMs(delayMs);
|
||||
delayMs = nextPrMergeRetryDelayMs(retryConfig, delayMs);
|
||||
}
|
||||
if (metadata === null || isGitHubError(metadata)) return commandError("pr merge", repo, metadata ?? errorPayload("invalid-response", "PR merge metadata was not collected"), { number, phase: "fetch-pr-closeout-metadata", pullRequest: summary });
|
||||
const retry = {
|
||||
reason: "github-mergeability-unknown",
|
||||
maxAttempts: retryConfig.maxAttempts,
|
||||
attempts: retryAttempts.length,
|
||||
exhausted: mergeability.readyForCommanderMerge !== true && mergeabilityHasUnknownPending(mergeability) && retryAttempts.length >= retryConfig.maxAttempts,
|
||||
config: retryConfig,
|
||||
attemptsLog: retryAttempts,
|
||||
};
|
||||
if (mergeability.readyForCommanderMerge !== true) {
|
||||
const details = {
|
||||
number,
|
||||
method: options.mergeMethod,
|
||||
deleteBranch: options.deleteBranch,
|
||||
pullRequest: preflightPullRequestSummary(summary),
|
||||
mergeability,
|
||||
statusChecks,
|
||||
closeoutMetadata: prCloseoutMetadata(metadata),
|
||||
retry,
|
||||
};
|
||||
return withPrMergeRendered(validationError("pr merge", repo, "PR is not ready for merge; preflight blockers or pending states remain", details), details);
|
||||
return withPrMergeRendered(validationError("pr merge", repo, retry.exhausted ? "PR mergeability is still UNKNOWN after automatic retry; no merge performed" : "PR is not ready for merge; blockers or pending states remain", details), details);
|
||||
}
|
||||
if (options.dryRun) {
|
||||
return withPrMergeRendered({
|
||||
@@ -5170,6 +5300,7 @@ async function prMerge(repo: string, token: string, number: number, options: Git
|
||||
pullRequest: preflightPullRequestSummary(summary),
|
||||
mergeability,
|
||||
statusChecks,
|
||||
retry,
|
||||
});
|
||||
}
|
||||
const merged = await githubRequest<Record<string, unknown>>(token, "PUT", `/repos/${owner}/${name}/pulls/${number}/merge`, {
|
||||
@@ -5187,6 +5318,9 @@ async function prMerge(repo: string, token: string, number: number, options: Git
|
||||
method: options.mergeMethod,
|
||||
mergeResult: merged,
|
||||
pullRequest: prSummary(after),
|
||||
mergeability,
|
||||
statusChecks,
|
||||
retry,
|
||||
branchDeletion,
|
||||
rest: true,
|
||||
});
|
||||
@@ -5219,6 +5353,13 @@ function renderPrMergeTable(result: GitHubCommandResult, fallbackDetails?: Recor
|
||||
: {};
|
||||
const counts = isRecord(statusChecks.counts) ? statusChecks.counts : {};
|
||||
const branchDeletion = isRecord(result.branchDeletion) ? result.branchDeletion : {};
|
||||
const retry = isRecord(result.retry)
|
||||
? result.retry
|
||||
: isRecord(details.retry)
|
||||
? details.retry
|
||||
: {};
|
||||
const mergeMethod = result.method ?? result.mergeMethod ?? details.method;
|
||||
const deleteBranch = result.deleteBranch ?? details.deleteBranch;
|
||||
const status = result.ok === true
|
||||
? result.alreadyMerged === true
|
||||
? "already-merged"
|
||||
@@ -5233,19 +5374,21 @@ function renderPrMergeTable(result: GitHubCommandResult, fallbackDetails?: Recor
|
||||
const rows = [[
|
||||
`#${ghText(result.number ?? details.number)}`,
|
||||
status,
|
||||
ghText(result.method ?? result.mergeMethod),
|
||||
ghText(mergeMethod),
|
||||
ghText(mergeability.mergeable),
|
||||
ghText(mergeability.mergeStateStatus),
|
||||
retry.attempts !== undefined && retry.maxAttempts !== undefined ? `${ghText(retry.attempts)}/${ghText(retry.maxAttempts)}${retry.exhausted === true ? " exhausted" : ""}` : "-",
|
||||
`${ghText(statusChecks.totalContexts)} total ${ghText(counts.success)} ok ${ghText(counts.failure)} fail ${ghText(counts.pending)} pending`,
|
||||
]];
|
||||
const lines = [
|
||||
`gh pr merge (${status})`,
|
||||
"",
|
||||
ghTable(["PR", "STATUS", "METHOD", "MERGEABLE", "MERGE_STATE", "CHECKS"], rows),
|
||||
ghTable(["PR", "STATUS", "METHOD", "MERGEABLE", "MERGE_STATE", "RETRY", "CHECKS"], rows),
|
||||
"",
|
||||
"Summary:",
|
||||
` repo=${ghText(result.repo)} title=${ghShort(ghText(pullRequest.title), 96)}`,
|
||||
` blockers=${blockers.length === 0 ? "-" : blockers.join(",")} pending=${pending.length === 0 ? "-" : pending.join(",")}`,
|
||||
` retry=${retry.attempts !== undefined && retry.maxAttempts !== undefined ? `${ghText(retry.attempts)}/${ghText(retry.maxAttempts)} exhausted=${ghText(retry.exhausted)}` : "-"}`,
|
||||
` branchDeletion=${ghText(branchDeletion.ok ?? branchDeletion.skippedReason ?? branchDeletion.attempted)}`,
|
||||
"",
|
||||
"Next:",
|
||||
@@ -5253,7 +5396,8 @@ function renderPrMergeTable(result: GitHubCommandResult, fallbackDetails?: Recor
|
||||
if (result.ok === true && result.alreadyMerged !== true && result.dryRun !== true) {
|
||||
lines.push(` bun scripts/cli.ts gh pr view ${ghText(result.number ?? details.number)} --repo ${ghText(result.repo)}`);
|
||||
} else {
|
||||
lines.push(` bun scripts/cli.ts gh pr preflight ${ghText(result.number ?? details.number)} --repo ${ghText(result.repo)}`);
|
||||
lines.push(` ${prMergeRetryCommand(ghText(result.repo), result.number ?? details.number, mergeMethod, deleteBranch)}`);
|
||||
lines.push(` bun scripts/cli.ts gh pr preflight ${ghText(result.number ?? details.number)} --repo ${ghText(result.repo)} --full`);
|
||||
}
|
||||
lines.push("", "Disclosure:");
|
||||
lines.push(" default view is a bounded table; use --full or --raw on read/preflight commands for structured metadata.");
|
||||
@@ -7987,8 +8131,8 @@ export function ghHelp(): unknown {
|
||||
"PR view is the canonical GitHub CLI-compatible read path; read remains a UniDesk compatibility alias. PR view/read accept positional numbers, GitHub PR URLs, and owner/repo#number shorthand, deriving --repo unless an explicit conflicting --repo is supplied. --number is accepted on single PR/comment numeric target commands for low-friction compatibility and returns a standard syntax hint; list/create do not accept it. PR comment update/edit/delete treat --number as commentId, not a PR number. PR view/read supports REST closeout fields stateDetail, closed, closedAt, merged, mergedAt, mergeCommit, headRefName, and baseRefName; mergeable, mergeStateStatus, and statusCheckRollup are fetched through GitHub GraphQL only when requested or when --raw/--full requests full disclosure, and closeoutMetadata makes GraphQL errors plus UNKNOWN/null metadata explicit.",
|
||||
"PR preflight/closeout accept the same owner/repo#number shorthand as PR view/read so merge readiness checks do not require repeating --repo after a PR URL has already been normalized.",
|
||||
"PR list does not fetch mergeability or statusCheckRollup; request those closeout fields with gh pr view <number> --json headRefName,baseRefName,mergeable,mergeStateStatus,statusCheckRollup.",
|
||||
"PR preflight is a low-noise read-only closeout helper. It combines redacted auth capability, PR branch/state metadata, mergeability, mergeStateStatus, compact status check counts, and an explicit read-only policy. Use --full or --raw to include all fetched status contexts; gh pr merge is the separate guarded write path.",
|
||||
"PR merge is a guarded write operation: it first reads closeout metadata, refuses non-open/draft/conflicting/non-clean/failed/pending PRs, then uses GitHub REST merge. Use --dry-run to see the exact merge plan without writing.",
|
||||
"PR preflight is a low-noise read-only closeout helper for explicit diagnosis only. It combines redacted auth capability, PR branch/state metadata, mergeability, mergeStateStatus, compact status check counts, and an explicit read-only policy. It is not a required step before gh pr merge. Use --full or --raw to include all fetched status contexts.",
|
||||
"PR merge is the one-command guarded write path: it reads closeout metadata itself, retries GitHub UNKNOWN/null mergeability with YAML-configured exponential backoff, refuses non-open/draft/conflicting/non-clean/failed/pending PRs, then uses GitHub REST merge. Use --dry-run to see the exact merge plan without writing.",
|
||||
],
|
||||
};
|
||||
}
|
||||
@@ -8047,7 +8191,8 @@ function ghScopedHelpNotes(tokens: string[]): string[] {
|
||||
} else if (key === "pr comment" || key.startsWith("pr comment ")) {
|
||||
notes.push("PR comments are GitHub issue comments under the hood; use comment id targets for update/edit/delete.");
|
||||
} else if (key === "pr merge") {
|
||||
notes.push("PR merge is guarded: run `gh pr preflight <number>` first when you need an explicit readiness summary.");
|
||||
notes.push("PR merge is one-command guarded: it performs the readiness check itself; `gh pr preflight` is optional read-only diagnosis, not a required first step.");
|
||||
notes.push("When GitHub reports mergeability as UNKNOWN/null, merge automatically retries with YAML-configured exponential backoff and shows retry attempts as N/M.");
|
||||
}
|
||||
return notes;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user