diff --git a/.claude/skills/sync-upstream/references/translation.md b/.claude/skills/sync-upstream/references/translation.md index 5f62249..21c34f5 100644 --- a/.claude/skills/sync-upstream/references/translation.md +++ b/.claude/skills/sync-upstream/references/translation.md @@ -71,7 +71,7 @@ A sync must keep these even when upstream's line differs. - orchestrate and swarm isolation: remote by default for orchestrate workers, and a worktree for any swarm worker that checks out, builds, or runs. - `.claude-plugin/plugin.json` carries no `version`, where upstream's `.cursor-plugin/plugin.json` does. Claude Code then versions a GitHub-hosted install by commit SHA, so every merge to `main` reaches `claude plugin update`. A version string that does not change blocks updates. The `plugin-version` rule in `scripts/check-port.ts` keeps it out. `claude plugin validate` warns about the missing version. That warning is expected. - interrogate adds the blind-spot lenses (`references/blind-spots.md`, Step 3 item 5), a PR number or URL as scope, the lead checking each Act on and Consider finding against source, and the someone-else's-PR flow (`references/review-comments.md`), all folded in from Lucas's retired pr-review-swarm skill. When a playbook runs it as the pre-PR gate, it hands the verdict back instead of ending the turn. The gate's rules live in Opening a PR step 2. It runs no tool-based analyzers: a CodeScene change-set step was tried and removed after an eval showed the reviewers already covered everything it found (PR #2). -- `skills/poteto-mode/scripts/merge-gate/` is port-only. It reuses watch-pr's readiness verdict without editing watch-pr, and adds the policy: allowed repos, human-only paths needing a human approval on the head, and head pinning with `--match-head-commit`. Every playbook merges only through it, and a refusal is final: human-only changes are the user's to merge. The policy lives outside git, and editing it is on poteto-mode's always-pause list. +- `skills/poteto-mode/scripts/merge-gate/` is port-only. It reuses watch-pr's readiness verdict without editing watch-pr, and adds the policy: allowed repos, human-only paths needing a human approval on the head, and head pinning with `--match-head-commit`. A PR in a GitHub native stack refuses `gh pr merge`, so the gate falls back to the merge-async REST endpoint with the same head SHA. Every playbook merges only through it, and a refusal is final: human-only changes are the user's to merge. The policy lives outside git, and editing it is on poteto-mode's always-pause list. - `principle-never-block-on-the-human` narrows its force-push example to "force-push to a shared branch", matching poteto-mode's Autonomy. Autopilot owners force-push their own branches with a lease on every rebase. - models.md uses only `opus` and `sonnet`. `fable` is dropped for cost: every role that upstream maps to it, and every panel slot, goes to `opus` or `sonnet`. interrogate runs two reviewers (`opus`, `sonnet`), accepting that both share one family and give less independent review than a cross-family pair. - `skills/poteto-help/` is rewritten, not ported line for line. Setup, Custom Modes, and the guide pages are replaced by `claude plugin` install and update, `models.md`, and the README's merge-policy section. Its skill map uses relative links, and the skill tells the model to give users the public GitHub URL. diff --git a/skills/poteto-mode/playbooks/opening-a-pr.md b/skills/poteto-mode/playbooks/opening-a-pr.md index 3e83f26..0912dd1 100644 --- a/skills/poteto-mode/playbooks/opening-a-pr.md +++ b/skills/poteto-mode/playbooks/opening-a-pr.md @@ -38,7 +38,7 @@ After these sections, attach videos or screenshots when they prove a claim. Do n - Final (report the reason and any listed files, and stop): `needs-human-approval`, `repo-not-allowed`, `not-author`, `changes-requested`, `base-not-trunk`, `policy-invalid`, `threads-unverified`, `reviews-unverified`, `files-incomplete`, `draft` (someone drafted it after you readied it, which means hold). A verdict with any final reason is final. - Back to Babysit, then re-gate: `checks-failed`, `threads-unresolved`, and `not-mergeable` when the detail says `DIRTY` or `BEHIND` (rebase first). - Transient (wait for the watcher, rerun once, then report): `checks-pending`, `head-moved`, `github-error`, and any other `not-mergeable`. - - `merge-unverified`: run `gh pr view --json state,mergeCommit`. If it merged, continue with the post-merge watch on that commit. If not, treat it as transient. + - `merge-unverified`: run `gh pr view --json state,mergeCommit`. If it merged, continue with the post-merge watch on that commit. If not, a stacked PR's async merge may still be queued, so poll `gh pr view` for a few minutes before you rerun with `--merge`. If it is still open after that, treat it as transient. - Any other code: report the verdict and stop. - After a merge, read the merge commit from the verdict, or from `gh pr view` when the verdict's is null. Wait for trunk's checks and every deploy workflow the merge commit triggers. If a run on the merge commit itself is red, compare the same workflow's latest completed run on the merge commit's first parent. If that was red too, report the failure as pre-existing and open no revert. Otherwise tell the user at once, and open a revert PR through steps 1 to 3 (never step 4) for the user to merge. Report its URL, any blocker, and any bot thread that names a deploy or data-compatibility risk. Never merge a revert yourself, because a red run can come from outside the change. A run that ends `cancelled` or skipped after the merge is pending: it takes the result of the same workflow's first completed run on a later trunk commit that contains the merge commit and does not revert it. If that run is red, report it as possibly either commit's and open no revert. If no such run completes, report the merge commit's status as unknown. A deploy waiting on a human approval is the user's: report it. 5. Reply with the PR URL, what the gate found, fixed, and dismissed, the Consider findings, any blocker, the merge result or refusal, and any revert PR with its blockers. diff --git a/skills/poteto-mode/scripts/merge-gate/cli.test.ts b/skills/poteto-mode/scripts/merge-gate/cli.test.ts index 5ca0cff..05c6847 100644 --- a/skills/poteto-mode/scripts/merge-gate/cli.test.ts +++ b/skills/poteto-mode/scripts/merge-gate/cli.test.ts @@ -3,12 +3,18 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { type Verdict, main } from "./cli.ts"; +import type { ReasonCode } from "./decide.ts"; import { type FakeGh, type FakeResponse, type FakeRule, fakeGh } from "./fake-gh.test-helper.ts"; const LAUNCHER = join(import.meta.dir, "merge-gate"); const HEAD = "1111111111111111111111111111111111111111"; const MOVED = "2222222222222222222222222222222222222222"; const MERGE_COMMIT = "3333333333333333333333333333333333333333"; +const JOB = "c0ffee00-0000-4000-8000-000000000000"; +const STACKED = { + stderr: "GraphQL: This pull request is part of a stack and must be merged using the asynchronous merge REST API (mergePullRequest)\n", + exit: 1, +}; const POLICY = { defaults: { humanOnly: [".github/**"] }, repos: { "acme/app": { mergeCommitBranches: ["sync/*"] } }, @@ -27,6 +33,8 @@ interface Scenario { readonly user?: FakeRule[1]; readonly merge?: FakeRule[1]; readonly outcome?: FakeRule[1]; + readonly asyncMerge?: FakeRule[1]; + readonly asyncPolls?: FakeRule[1]; readonly defaultBranch?: string | null; /** Contents of $HOME/.claude/pstack/merge-policy.json; null leaves it absent. */ readonly policy?: string | null; @@ -126,6 +134,8 @@ function rules(scenario: Scenario = {}): readonly FakeRule[] { ], ], ["pr merge 7", scenario.merge ?? [{ stdout: "" }]], + [`pulls/7/merge-async/${JOB}`, scenario.asyncPolls ?? [{ stdout: { status: "merged", details: { uuid: JOB } } }]], + ["pulls/7/merge-async", scenario.asyncMerge ?? [{ stdout: { status: "pending", details: { uuid: JOB } } }]], ["query MergeGateOutcome", scenario.outcome ?? [outcome("MERGED", MERGE_COMMIT)]], ]; } @@ -163,7 +173,23 @@ const withCodes = (verdict: Verdict) => ({ const merges = async (gh: FakeGh) => (await gh.calls()).filter((argv) => argv[0] === "pr" && argv[1] === "merge"); +const asyncMerges = async (gh: FakeGh) => + (await gh.calls()).filter((argv) => argv[0] === "api" && argv[1]?.includes("/merge-async")); + const squash = ["pr", "merge", "7", "--repo", "acme/app", "--match-head-commit", HEAD, "--squash"]; +const asyncPut = [ + "api", + "repos/acme/app/pulls/7/merge-async", + "-X", + "PUT", + "-f", + `sha=${HEAD}`, + "-f", + "merge_method=squash", + "-f", + "merge_action=direct_merge", +]; +const asyncPoll = ["api", `repos/acme/app/pulls/7/merge-async/${JOB}`]; describe("merge-gate CLI against a fake gh", () => { it.each<[string, Scenario, readonly string[], number, object, readonly (readonly string[])[]]>([ @@ -291,7 +317,7 @@ describe("merge-gate CLI against a fake gh", () => { reasons: [ { code: "merge-unverified", - detail: "gh pr merge exited 0, then reading the PR back failed, so it may have merged. Rerun merge-gate without --merge.", + detail: "gh pr merge exited 0: no stderr, then reading the PR back failed, so it may have merged. Rerun merge-gate without --merge.", }, ], }, @@ -301,6 +327,131 @@ describe("merge-gate CLI against a fake gh", () => { const result = runCli(gh, ["7", "--repo", "acme/app", "--merge"]); expect(result).toEqual({ exit, verdict }); expect(await merges(gh)).toEqual([squash]); + expect(await asyncMerges(gh)).toEqual([]); + }); + + const refused = (code: ReasonCode, detail: string): Verdict => ({ + ...subject, + decision: "refuse", + merged: false, + mergeCommit: null, + reasons: [{ code, detail }], + }); + const open = { outcome: [outcome("OPEN", null)] }; + const UNSETTLED = "; PR is OPEN, so it may still merge. Wait for gh pr view to show it settled before rerunning merge-gate --merge."; + it.each<[string, Scenario, number, Verdict, readonly (readonly string[])[]]>([ + [ + "merges through merge-async pinned to the head it read", + { merge: [STACKED] }, + 0, + { ...subject, decision: "merge", merged: true, mergeCommit: MERGE_COMMIT, reasons: [] }, + [squash, asyncPut, asyncPoll], + ], + [ + "polls merge-async until it settles", + { + merge: [STACKED], + asyncPolls: [{ stdout: { status: "pending", details: { uuid: JOB } } }, { stdout: { status: "merged" } }], + }, + 0, + { ...subject, decision: "merge", merged: true, mergeCommit: MERGE_COMMIT, reasons: [] }, + [squash, asyncPut, asyncPoll, asyncPoll], + ], + [ + "merges a mergeCommitBranches head through merge-async with a merge commit", + { merge: [STACKED], headRefName: "sync/upstream" }, + 0, + { ...subject, decision: "merge", merged: true, mergeCommit: MERGE_COMMIT, reasons: [] }, + [ + squash.map((arg) => (arg === "--squash" ? "--merge" : arg)), + asyncPut.map((arg) => (arg === "merge_method=squash" ? "merge_method=merge" : arg)), + asyncPoll, + ], + ], + [ + "refuses when the merge-async job fails", + { merge: [STACKED], asyncPolls: [{ stdout: { status: "failed", details: { message: "Merge conflict" } } }], ...open }, + 2, + refused("merge-failed", `merge-async job ${JOB} failed: Merge conflict; PR is OPEN`), + [squash, asyncPut, asyncPoll], + ], + [ + "refuses when the merge-async request itself reports failure", + { merge: [STACKED], asyncMerge: [{ stdout: { status: "failed", details: { message: "Not mergeable" } } }], ...open }, + 2, + refused("merge-failed", "merge-async failed: Not mergeable; PR is OPEN"), + [squash, asyncPut], + ], + [ + "refuses when GitHub rejects the pinned head", + { + merge: [STACKED], + asyncMerge: [{ stderr: "gh: Head branch was modified. Review and try the merge again. (HTTP 409)\n", exit: 1 }], + ...open, + }, + 2, + refused( + "merge-failed", + "merge-async request failed: gh api repos/acme/app/pulls/7/merge-async exited 1: gh: Head branch was modified. Review and try the merge again. (HTTP 409); PR is OPEN" + ), + [squash, asyncPut], + ], + [ + "reports an unverified merge when the merge-async request errors server-side", + { merge: [STACKED], asyncMerge: [{ stderr: "gh: Bad Gateway (HTTP 502)\n", exit: 1 }], ...open }, + 1, + refused( + "merge-unverified", + `merge-async request failed: gh api repos/acme/app/pulls/7/merge-async exited 1: gh: Bad Gateway (HTTP 502)${UNSETTLED}` + ), + [squash, asyncPut], + ], + [ + "reports an unverified merge when polling merge-async fails", + { merge: [STACKED], asyncPolls: [{ stderr: "HTTP 502\n", exit: 1 }], ...open }, + 1, + refused( + "merge-unverified", + `polling merge-async job ${JOB} failed: gh api repos/acme/app/pulls/7/merge-async/${JOB} exited 1: HTTP 502${UNSETTLED}` + ), + [squash, asyncPut, asyncPoll], + ], + [ + "reports an unverified merge when the job reports merged but the PR reads open", + { merge: [STACKED], ...open }, + 1, + refused("merge-unverified", `merge-async job ${JOB} reported merged${UNSETTLED}`), + [squash, asyncPut, asyncPoll], + ], + [ + "reports an unverified merge on a status it does not know", + { merge: [STACKED], asyncPolls: [{ stdout: { status: "queued" } }], ...open }, + 1, + refused("merge-unverified", `merge-async job ${JOB} reported unknown status queued${UNSETTLED}`), + [squash, asyncPut, asyncPoll], + ], + [ + "reports an unverified merge when merge-async queues a job without a uuid", + { merge: [STACKED], asyncMerge: [{ stdout: { status: "pending" } }], ...open }, + 1, + refused("merge-unverified", `merge-async queued a job without a uuid${UNSETTLED}`), + [squash, asyncPut], + ], + [ + "reports an unverified merge when the read-back fails after merge-async", + { merge: [STACKED], outcome: [{ stderr: "HTTP 502\n", exit: 1 }] }, + 1, + refused( + "merge-unverified", + `merge-async job ${JOB} reported merged, then reading the PR back failed, so it may have merged. Rerun merge-gate without --merge.` + ), + [squash, asyncPut, asyncPoll], + ], + ])("merges a stacked PR: %s", async (_name, scenario, exit, verdict, expectedMerges) => { + const gh = await setup(scenario); + const result = runCli(gh, ["7", "--repo", "acme/app", "--merge"]); + expect(result).toEqual({ exit, verdict }); + expect([...(await merges(gh)), ...(await asyncMerges(gh))]).toEqual([...expectedMerges]); }); it.each<[string, string | null]>([ diff --git a/skills/poteto-mode/scripts/merge-gate/github.ts b/skills/poteto-mode/scripts/merge-gate/github.ts index 62972b5..1ced366 100644 --- a/skills/poteto-mode/scripts/merge-gate/github.ts +++ b/skills/poteto-mode/scripts/merge-gate/github.ts @@ -11,7 +11,7 @@ import type { ReviewState, } from "./decide.ts"; -/** What GitHub reports after `gh pr merge`, whatever the command's exit code said. */ +/** What GitHub reports after the merge request, whatever the request itself said. */ export type MergeResult = | { readonly kind: "merged"; readonly commit: string | null } | { readonly kind: "failed"; readonly detail: string } @@ -68,6 +68,13 @@ query MergeGateOutcome($owner: String!, $repo: String!, $pr: Int!) { } } `; +/** GitHub refuses `gh pr merge` for a PR in a native stack and wants its merge-async endpoint. */ +const STACKED = /part of a stack/; +/** Keeps a stuck job well inside an agent's two-minute command timeout. */ +const MERGE_POLL_MS = 2_000; +const MAX_MERGE_POLLS = 30; +/** Only a 4xx proves GitHub queued nothing; a 5xx, a timeout or a garbled body may hide a queued job. */ +const REJECTED = /\(HTTP 4\d\d\)/; const FILES_PER_PAGE = 100; /** GitHub's pull request files endpoint stops at 3000 files. */ const MAX_FILE_PAGES = 30; @@ -79,6 +86,12 @@ const REVIEW_STATES = [ "DISMISSED", ] as const satisfies readonly ReviewState[]; +/** How a merge request ended before the read-back; `pending` means GitHub may still merge it. */ +interface Attempt { + readonly kind: "failed" | "pending"; + readonly detail: string; +} + interface CommandResult { readonly code: number; readonly stdout: string; @@ -223,6 +236,22 @@ function parseFile(value: unknown, index: number): ChangedFile { }; } +interface MergeJob { + readonly status: string; + readonly uuid: string | null; + readonly message: string; +} + +function parseMergeJob(value: unknown): MergeJob { + const job = record(value, "merge-async"); + const details = job.details === undefined ? {} : record(job.details, "merge-async.details"); + return { + status: text(job.status, "merge-async.status"), + uuid: details.uuid === undefined ? null : text(details.uuid, "merge-async.details.uuid"), + message: typeof details.message === "string" ? details.message : JSON.stringify(details), + }; +} + export class GhGateGitHub implements GateGitHub { private readonly reader = new GhGitHubReader(); @@ -290,23 +319,72 @@ export class GhGateGitHub implements GateGitHub { head, method === "merge" ? "--merge" : "--squash", ]); + const attempt: Attempt = + result.code !== 0 && STACKED.test(result.stderr) + ? await this.mergeAsync(pr, head, method) + : { kind: "failed", detail: `gh pr merge exited ${result.code}: ${firstLine(result.stderr) || "no stderr"}` }; let state: string; - let merged: MergeResult | null = null; try { const after = record(repository(await graphql(OUTCOME_QUERY, pr)).pullRequest, "pullRequest"); state = text(after.state, "state"); if (state === "MERGED" && optionalText(after.mergedAt, "mergedAt") !== null) - merged = { kind: "merged", commit: oid(after.mergeCommit, "mergeCommit") }; + return { kind: "merged", commit: oid(after.mergeCommit, "mergeCommit") }; } catch { return { kind: "unverified", - detail: `gh pr merge exited ${result.code}, then reading the PR back failed, so it may have merged. Rerun merge-gate without --merge.`, + detail: `${attempt.detail}, then reading the PR back failed, so it may have merged. Rerun merge-gate without --merge.`, }; } - if (merged !== null) return merged; - return { - kind: "failed", - detail: `gh pr merge exited ${result.code}: ${firstLine(result.stderr) || "no stderr"}; PR is ${state}`, - }; + return attempt.kind === "pending" + ? { + kind: "unverified", + detail: `${attempt.detail}; PR is ${state}, so it may still merge. Wait for gh pr view to show it settled before rerunning merge-gate --merge.`, + } + : { kind: "failed", detail: `${attempt.detail}; PR is ${state}` }; + } + + private async mergeAsync(pr: W.PrContext, head: string, method: MergeMethod): Promise { + const endpoint = `repos/${pr.owner}/${pr.repo}/pulls/${pr.number}/merge-async`; + let job: MergeJob; + try { + job = parseMergeJob( + await ghJson([ + "api", + endpoint, + "-X", + "PUT", + "-f", + `sha=${head}`, + "-f", + `merge_method=${method}`, + "-f", + "merge_action=direct_merge", + ]) + ); + } catch (error) { + if (!(error instanceof GitHubError)) throw error; + return { + kind: REJECTED.test(error.message) ? "failed" : "pending", + detail: `merge-async request failed: ${error.message}`, + }; + } + const { uuid } = job; + const name = uuid === null ? "merge-async" : `merge-async job ${uuid}`; + for (let poll = 0; ; poll += 1) { + if (job.status === "merged") return { kind: "pending", detail: `${name} reported merged` }; + if (job.status === "failed") return { kind: "failed", detail: `${name} failed: ${firstLine(job.message)}` }; + if (job.status !== "pending") + return { kind: "pending", detail: `${name} reported unknown status ${job.status}` }; + if (uuid === null) return { kind: "pending", detail: "merge-async queued a job without a uuid" }; + if (poll === MAX_MERGE_POLLS) + return { kind: "pending", detail: `${name} still pending after ${MAX_MERGE_POLLS} polls` }; + if (poll > 0) await Bun.sleep(MERGE_POLL_MS); + try { + job = parseMergeJob(await ghJson(["api", `${endpoint}/${uuid}`])); + } catch (error) { + if (!(error instanceof GitHubError)) throw error; + return { kind: "pending", detail: `polling ${name} failed: ${error.message}` }; + } + } } }