From 865e81e7f79c6afb0ef946f81baca7b1174722cf Mon Sep 17 00:00:00 2001 From: Lyon <88232613+pikasTech@users.noreply.github.com> Date: Mon, 22 Jun 2026 14:14:47 +0800 Subject: [PATCH] fix: make gh pr merge one-command guarded merge (#654) Co-authored-by: Codex --- .agents/skills/unidesk-gh/SKILL.md | 4 +- config/unidesk-cli.yaml | 7 ++ scripts/src/gh.ts | 167 +++++++++++++++++++++++++++-- 3 files changed, 166 insertions(+), 12 deletions(-) diff --git a/.agents/skills/unidesk-gh/SKILL.md b/.agents/skills/unidesk-gh/SKILL.md index 87f4a5f5..4396849b 100644 --- a/.agents/skills/unidesk-gh/SKILL.md +++ b/.agents/skills/unidesk-gh/SKILL.md @@ -271,7 +271,9 @@ bun scripts/cli.ts gh pr merge \ [--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`。 ### 编辑 / 评论 / 关闭 / 重开 diff --git a/config/unidesk-cli.yaml b/config/unidesk-cli.yaml index f16bb534..3d3d77e5 100644 --- a/config/unidesk-cli.yaml +++ b/config/unidesk-cli.yaml @@ -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 diff --git a/scripts/src/gh.ts b/scripts/src/gh.ts index c4ebf457..3dcc8dde 100644 --- a/scripts/src/gh.ts +++ b/scripts/src/gh.ts @@ -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 { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function mergeabilityHasUnknownPending(mergeability: Record): 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> { 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 { 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(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 = {}; + let mergeability: Record = {}; + let delayMs = retryConfig.initialDelayMs; + const retryAttempts: Array> = []; + 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>(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 --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 ` 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; }