From f9b3c1d0f565c7cd129165950865a9ed4b41ed23 Mon Sep 17 00:00:00 2001 From: J0UH <217747670+J0UH@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:51:43 +0200 Subject: [PATCH] watch-pr: end stack discovery at a fork PR from main and at a cycle orderStack keyed every open PR by its head branch name. An open PR from a fork's main into main has the head name main, so it became its own parent, and the ancestor walk pushed it forever for that PR and for every PR based on main. watch-pr --stack on cursor/plugins#744 never printed and was killed by a 30-second alarm. Two PRs whose branches point at each other looped the same way. A PR whose head is in another repository is no longer a parent, and stack discovery does not look for children above it. The ancestor walk stops when it reaches a PR it has already taken. The gh reader now asks for isCrossRepository. --- .../poteto-mode/scripts/watch-pr/github.ts | 18 ++- .../scripts/watch-pr/stack.test.ts | 133 ++++++++++++++++++ .../poteto-mode/scripts/watch-pr/types.ts | 2 + tools/forks.json | 4 +- 4 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 plugins/pstack/skills/poteto-mode/scripts/watch-pr/stack.test.ts diff --git a/plugins/pstack/skills/poteto-mode/scripts/watch-pr/github.ts b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/github.ts index d1de3fb3..53e237ef 100644 --- a/plugins/pstack/skills/poteto-mode/scripts/watch-pr/github.ts +++ b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/github.ts @@ -513,10 +513,15 @@ export class GhGitHubReader implements T.GitHubReader { "--limit", "300", "--json", - "number,headRefName,baseRefName", + "number,headRefName,baseRefName,isCrossRepository", ]); return list(value, "open PRs").map((item, index) => { const object = record(item, `open PRs[${index}]`); + if (typeof object.isCrossRepository !== "boolean") + missing( + `open PRs[${index}].isCrossRepository`, + object.isCrossRepository + ); return { number: parsePrNumber(object.number, `open PRs[${index}].number`), headRefName: string( @@ -527,6 +532,7 @@ export class GhGitHubReader implements T.GitHubReader { object.baseRefName, `open PRs[${index}].baseRefName` ), + isCrossRepository: object.isCrossRepository, }; }); } @@ -705,7 +711,11 @@ export function orderStack( open: readonly T.OpenPullRequest[] ): T.NonEmpty { const byNumber = new Map(open.map((pr) => [pr.number, pr])); - const byHead = new Map(open.map((pr) => [pr.headRefName, pr])); + const byHead = new Map( + open + .filter((pr) => pr.isCrossRepository !== true) + .map((pr) => [pr.headRefName, pr]) + ); const children = new Map(); for (const pr of open) children.set(pr.baseRefName, [...(children.get(pr.baseRefName) ?? []), pr]); @@ -717,7 +727,8 @@ export function orderStack( let current = start; while (byHead.has(current.baseRefName)) { const parent = byHead.get(current.baseRefName); - if (parent === undefined) break; + if (parent === undefined || parent === start || down.includes(parent)) + break; down.push(parent); current = parent; } @@ -727,6 +738,7 @@ export function orderStack( ]); const up: T.OpenPullRequest[] = []; const visit = (parent: T.OpenPullRequest): void => { + if (parent.isCrossRepository === true) return; for (const child of children.get(parent.headRefName) ?? []) { if (seen.has(child.number)) continue; seen.add(child.number); diff --git a/plugins/pstack/skills/poteto-mode/scripts/watch-pr/stack.test.ts b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/stack.test.ts new file mode 100644 index 00000000..3b5bc94c --- /dev/null +++ b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/stack.test.ts @@ -0,0 +1,133 @@ +// Stack discovery must end. A PR from a fork's main into main has the head +// branch name main, which is also the base of every PR stacked on main, and +// two PRs whose branches point at each other form a cycle. +import { afterEach, describe, expect, it } from "bun:test"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { orderStack } from "./github.ts"; +import { parsePrNumber } from "./types.ts"; + +const context = (number: number) => ({ + owner: "owner", + repo: "repo", + number: parsePrNumber(number), +}); +const pr = ( + number: number, + headRefName: string, + baseRefName: string, + isCrossRepository = false +) => ({ + number: parsePrNumber(number), + headRefName, + baseRefName, + isCrossRepository, +}); +const numbers = (stack: readonly { readonly number: number }[]) => + stack.map((item) => Number(item.number)); + +describe("stack discovery", () => { + const forkMain = pr(7, "main", "main", true); + + it("does not take a fork PR from main as the parent of a PR on main", () => { + expect( + numbers( + orderStack(context(42), [ + forkMain, + pr(42, "feature", "main"), + pr(43, "upstack", "feature"), + ]) + ) + ).toEqual([42, 43]); + }); + + it("does not stack the PRs on main above a fork PR from main", () => { + expect( + numbers(orderStack(context(7), [forkMain, pr(42, "feature", "main")])) + ).toEqual([7]); + }); + + it("stops at a cycle of branches", () => { + expect( + numbers(orderStack(context(42), [pr(41, "a", "b"), pr(42, "b", "a")])) + ).toEqual([41, 42]); + }); +}); + +describe("the gh reader on a repository with an open PR from a fork's main", () => { + const directories: string[] = []; + afterEach(() => { + for (const dir of directories.splice(0)) + rmSync(dir, { recursive: true, force: true }); + }); + + it("watches the stack without the fork PR", () => { + const dir = mkdtempSync(join(tmpdir(), "watch-fork-main-")); + directories.push(dir); + const bin = join(dir, "bin"); + mkdirSync(bin); + const gh = join(bin, "gh"); + writeFileSync( + gh, + `#!${process.execPath} +const args = process.argv.slice(2); +const fail = (stderr) => { console.error(stderr); process.exit(1); }; +const graphql = (name) => args.some((a) => a.includes('query ' + name)); +const commits = (commit) => ({ data: { repository: { pullRequest: { commits: { nodes: [{ commit }] } } } } }); +let value; +if (args[0] === 'pr' && args[1] === 'list') { + value = [ + { number: 7, headRefName: 'main', baseRefName: 'main', isCrossRepository: true }, + { number: 1, headRefName: 'feature', baseRefName: 'main', isCrossRepository: false }, + ]; +} else if (args[0] === 'pr' && args[1] === 'view') { + value = { mergeable: 'MERGEABLE', mergeStateStatus: 'CLEAN', reviewDecision: '', headRefOid: 'head', headRefName: 'feature', baseRefName: 'main', state: 'OPEN', mergedAt: null, isDraft: false }; +} else if (args[0] === 'pr' && args[1] === 'checks') { + fail("no checks reported on the 'feature' branch"); +} else if (graphql('PrCheckRollup')) { + value = commits({ statusCheckRollup: null }); +} else if (graphql('PrCommitStatuses')) { + value = commits({ oid: 'head', statusCheckRollup: null }); +} else if (graphql('ReviewThreads')) { + value = { data: { repository: { pullRequest: { reviewThreads: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } } } } } }; +} else { + fail('unexpected fixture command: ' + JSON.stringify(args)); +} +console.log(JSON.stringify(value)); +` + ); + chmodSync(gh, 0o755); + const entry = join(dir, "entry.ts"); + writeFileSync( + entry, + `import { main } from ${JSON.stringify(join(import.meta.dir, "cli.ts"))}; process.exitCode = await main(process.argv.slice(2));\n` + ); + const result = spawnSync( + process.execPath, + [ + entry, + ...["--owner", "owner", "--repo", "repo", "--pr", "1"], + ...["--stack", "--status-only", "--settle", "0"], + ], + { + encoding: "utf8", + timeout: 10_000, + env: { PATH: `${bin}:${process.env.PATH}` }, + } + ); + expect(result.signal).toBeNull(); + expect(result.status).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + kind: "STATUS", + rows: [{ kind: "open", context: context(1) }], + }); + }); +}); diff --git a/plugins/pstack/skills/poteto-mode/scripts/watch-pr/types.ts b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/types.ts index 1cc2a895..ff5754ab 100644 --- a/plugins/pstack/skills/poteto-mode/scripts/watch-pr/types.ts +++ b/plugins/pstack/skills/poteto-mode/scripts/watch-pr/types.ts @@ -55,6 +55,8 @@ export interface OpenPullRequest { readonly number: PrNumber; readonly headRefName: string; readonly baseRefName: string; + // A head branch in another repository is never the base of a PR here. + readonly isCrossRepository?: boolean; } export interface ReviewComment { readonly authorLogin: string | null; diff --git a/tools/forks.json b/tools/forks.json index 086fa801..5a8a2841 100644 --- a/tools/forks.json +++ b/tools/forks.json @@ -50,7 +50,7 @@ }, "plugins/pstack/skills/poteto-mode/scripts/watch-pr/github.ts": { "kind": "policy", - "why": "Gives the gh reader the reviewActivity method that settle.ts uses, and carries two fixes from Michael Denyer's pstack-claude: a completed check that gh buckets as pending, such as STALE, reads as failed instead of pending forever (pstack-claude #97), and a head with no checks configured reads as such when gh reports none and the head has no rollup (pstack-claude #153).", + "why": "Gives the gh reader the reviewActivity method that settle.ts uses, and carries two fixes from Michael Denyer's pstack-claude: a completed check that gh buckets as pending, such as STALE, reads as failed instead of pending forever (pstack-claude #97), and a head with no checks configured reads as such when gh reports none and the head has no rollup (pstack-claude #153); its stack discovery never takes a PR whose head is in another repository as a parent and stops at a branch cycle, so an open PR from a fork's main cannot loop it forever.", "since": "2.0.0-rc.1", "upstream": "not-proposed" }, @@ -74,7 +74,7 @@ }, "plugins/pstack/skills/poteto-mode/scripts/watch-pr/types.ts": { "kind": "policy", - "why": "Adds the settle state, decision, and reader method that settle.ts defines, and the review-required and merge-blocked gates, the no-checks read and CI state, and the confirming-block wait and timeout reason that the fixes from Michael Denyer's pstack-claude add.", + "why": "Adds the settle state, decision, and reader method that settle.ts defines, and the review-required and merge-blocked gates, the no-checks read and CI state, and the confirming-block wait and timeout reason that the fixes from Michael Denyer's pstack-claude add; it also adds the isCrossRepository flag on an open PR that stack discovery reads.", "since": "2.0.0-rc.1", "upstream": "not-proposed" },