diff --git a/bin/gstack-issue-links b/bin/gstack-issue-links index 8ae60f10d..1cb8da4d5 100755 --- a/bin/gstack-issue-links +++ b/bin/gstack-issue-links @@ -37,7 +37,8 @@ const HELP = `gstack-issue-links — Fixes only on explicit closure intent; ever --supersedes owner confirmed this PR supersedes a Related PR: Supersedes #N (repeatable) --credit owner confirmed crediting a Related PR's author: Contributed by @author (repeatable) --no-auto-links no Related lookup (open PRs by other authors closing a fixed issue) - --author this PR's author for that lookup (default: the gh viewer; pass it when gh runs as an app) + --author this PR's author for that lookup (default: the gh viewer; for an app viewer, the HEAD commit's + GitHub user, else a user-owned repository's owner) --repo repository for gh (default: the current remote) --out write the issues object (fixes, refs, flakes, related) for gstack-ship-receipt write --issues @file --json JSON only on stdout @@ -101,6 +102,28 @@ function closingPrs(gh: ReturnType, repo: string, fixes: numbe return { closing, viewer }; } +const AUTHOR_QUERY = 'query($owner:String!,$name:String!,$head:String!){repository(owner:$owner,name:$name){owner{__typename login} object(expression:$head){... on Commit{author{user{login}}}}}}'; +type AuthorReply = { data?: { repository?: { owner?: { __typename?: string; login?: string }; object?: { author?: { user?: { login?: string } | null } } | null } } }; + +/** + * Whose PRs are not "other authors": --author, else the gh viewer unless it is + * an app (`…[bot]`), in which case the HEAD commit's GitHub user, else the + * owner of a user-owned repository. With none, the app viewer stays and the + * gap is reported, since the owner's own PRs would then be listed as Related. + */ +function prAuthor(gh: ReturnType, repo: string, viewer: string, explicit: string | undefined, head: string | undefined): { login: string; source: string } { + if (explicit) return { login: explicit, source: '--author' }; + if (!/\[bot\]$/.test(viewer)) return { login: viewer, source: 'gh viewer' }; + const [owner, name] = repo.split('/'); + const res = head ? gh.json('this PR\'s author', ['api', 'graphql', '-f', `query=${AUTHOR_QUERY}`, '-f', `owner=${owner}`, '-f', `name=${name}`, '-f', `head=${head}`]) : undefined; + const commitUser = res?.data?.repository?.object?.author?.user?.login; + if (commitUser) return { login: commitUser, source: 'HEAD commit author' }; + const repoOwner = res?.data?.repository?.owner; + if (repoOwner?.__typename === 'User' && repoOwner.login) return { login: repoOwner.login, source: 'repository owner' }; + if (res) gh.failed('this PR\'s author', `gh runs as ${viewer} and neither the HEAD commit nor the repository owner names a user; pass --author `); + return { login: viewer, source: 'gh viewer' }; +} + function main(argv: string[]): number { if (argv.includes('--help') || argv.includes('-h')) { process.stdout.write(HELP + '\n'); return EXIT.ok; } const json = argv.includes('--json'); @@ -145,8 +168,9 @@ function main(argv: string[]): number { } const links = resolveLinks(mentions, explicit, readable, flakes); const found = autoLinks && repo && links.fixes.length ? closingPrs(gh, repo, links.fixes) : undefined; - if (found) { - const related = relatedPrs(found.closing, { repo: repo!, viewer: flags(argv, '--author')[0] ?? found.viewer, branch: git(['branch', '--show-current'], repoRoot)?.trim() || undefined }); + const author = found ? prAuthor(gh, repo!, found.viewer, flags(argv, '--author')[0], git(['rev-parse', 'HEAD'], repoRoot)?.trim()) : undefined; + if (found && author) { + const related = relatedPrs(found.closing, { repo: repo!, viewer: author.login, branch: git(['branch', '--show-current'], repoRoot)?.trim() || undefined }); if (related.length) links.related = related; } for (const [kind, list] of Object.entries(confirmed)) { @@ -161,13 +185,14 @@ function main(argv: string[]): number { const lines = renderIssueLines(links); const out = flags(argv, '--out')[0]; if (out) { fs.mkdirSync(path.dirname(out), { recursive: true }); fs.writeFileSync(out, JSON.stringify(receiptIssues(links), null, 2) + '\n'); } - if (json) process.stdout.write(JSON.stringify({ ...receiptIssues(links), cross: links.cross, unreadable: links.unreadable, candidates, flake_tests: links.flakes, rejected_flakes: rejectedFlakes, unavailable: gh.unavailable, lines }) + '\n'); + if (json) process.stdout.write(JSON.stringify({ ...receiptIssues(links), cross: links.cross, unreadable: links.unreadable, candidates, flake_tests: links.flakes, rejected_flakes: rejectedFlakes, ...(author ? { author } : {}), unavailable: gh.unavailable, lines }) + '\n'); else { process.stdout.write(lines.length ? lines.join('\n') + '\n' : 'ISSUE_LINKS: none\n'); for (const m of mentions) { const promote = candidates.some(c => c.issue === m.issue) && m.intent === 'refs' && !m.repo ? ` (promote: fix #${m.issue})` : ''; process.stderr.write(` ${m.intent === 'fixes' ? 'closure' : 'reference'} ${m.repo ? `${m.repo}#` : '#'}${m.issue} ← ${m.source}${promote}\n`); } + if (author && author.source !== 'gh viewer') process.stderr.write(` Related excludes @${author.login} (${author.source})\n`); for (const line of lineSources(links)) process.stderr.write(` ${line}\n`); for (const r of rejectedFlakes) process.stderr.write(` not a flake link: #${r.issue} (run ${r.run}) ← ${r.reason}\n`); } diff --git a/docs/pregate.md b/docs/pregate.md index 804a84309..5532c51a4 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -443,8 +443,10 @@ that names that run (its `flake:json` `first_seen.run_id`, or this repository's `Flake: ` title). Two issues for the same test keep the lower number; every refused match is printed with its reason. -Open PRs by other authors (not `--author`, default the `gh` viewer, which is -an app identity on hosts like Capy, so pass your login there; not this branch) whose closing +Open PRs by other authors (not this branch, and not this PR's author: `--author`, +else the `gh` viewer; when the viewer is an app such as `capy-ai[bot]`, the HEAD +commit's GitHub user, else the owner of a user-owned repository, else the gap is +`ISSUE_LOOKUP_FAILED`) whose closing set overlaps this PR's `Fixes` are `Related: #N (@author)`, read page by page from each issue's closing-PR references, in unattended runs too. `Supersedes #N` replaces that line and `Contributed by @author` is added (the credit lines of diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b348a8529..40d8c6ee4 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2492,13 +2492,16 @@ rendered as `Refs #N`, never `Fixes`. ### `ISSUE_LOOKUP_FAILED` **Meaning.** `gstack-issue-links` could not resolve the repository, list the open -PRs that close an issue this PR fixes, or search the flake issues naming a -`--ci-run` id. The closure and reference lines are still printed; the -`Related:` or `Flake:` lines that lookup would have added may be missing, and -`--json` lists each failure under `unavailable`. +PRs that close an issue this PR fixes, search the flake issues naming a +`--ci-run` id, or tell this PR's author: `gh` runs as an app (`…[bot]`) and +neither the HEAD commit (unpushed, or not linked to a GitHub user) nor a +user-owned repository names one, so the owner's own open PRs stay in the +`Related:` lines. The closure and reference lines are still printed; `--json` +lists each failure under `unavailable`. -**Fix.** Check `gh auth status` and pass `--repo ` when the remote -is not a GitHub repository, then rerun. If it still fails, say in the handoff +**Fix.** Check `gh auth status`, pass `--repo ` when the remote +is not a GitHub repository and `--author ` when `gh` runs as an app, +then rerun. If it still fails, say in the handoff which lookup is missing; never present the body as complete. diff --git a/lib/result-codes.ts b/lib/result-codes.ts index 499dd78a3..7dc72c756 100644 --- a/lib/result-codes.ts +++ b/lib/result-codes.ts @@ -416,8 +416,8 @@ export const RESULT_CODES = { }, ISSUE_LOOKUP_FAILED: { anchor: 'issue-lookup-failed', - summary: 'gstack-issue-links could not read the repository, the open PRs closing a fixed issue, or the flake issues naming a CI run, so those Related or Flake lines may be missing', - fix: 'check `gh auth status` and the repository (`--repo `), then rerun gstack-issue-links; report the gap in the handoff if it still fails', + summary: 'gstack-issue-links could not read the repository, this PR\'s author, the open PRs closing a fixed issue, or the flake issues naming a CI run, so Related or Flake lines may be missing or list the owner\'s own PRs', + fix: 'check `gh auth status` and the repository (`--repo `), pass `--author ` when gh runs as an app, then rerun gstack-issue-links; report the gap in the handoff if it still fails', }, ISSUE_LINKS_MISSING: { anchor: 'issue-links-missing', diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index fb1d4cbb0..3c8a95c3d 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -59,8 +59,7 @@ then return here for a new lookup and a fresh body before publishing. ### Issue links Run `~/.claude/skills/gstack/bin/gstack-issue-links --base --out .gstack/tmp/issues.json`, adding -`--issue ` per issue the user named as fixed, `--ci-run ` per failing CI run and -`--author ` when `gh` runs as an app (Capy). Interactive: +`--issue ` per issue the user named as fixed and `--ci-run ` per failing CI run. Interactive: ask once about its `(promote: fix #N)` references (each `fix #` reply adds `--issue `), and add `--supersedes `/`--credit ` for a `Related:` PR only when the owner confirms; rerun after any change. The completion message lists unpromoted candidates, `Related:` lines and any `ISSUE_LOOKUP_FAILED`. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 4caf15000..0b37b036b 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -50,8 +50,7 @@ then return here for a new lookup and a fresh body before publishing. ### Issue links Run `~/.claude/skills/gstack/bin/gstack-issue-links --base --out .gstack/tmp/issues.json`, adding -`--issue ` per issue the user named as fixed, `--ci-run ` per failing CI run and -`--author ` when `gh` runs as an app (Capy). Interactive: +`--issue ` per issue the user named as fixed and `--ci-run ` per failing CI run. Interactive: ask once about its `(promote: fix #N)` references (each `fix #` reply adds `--issue `), and add `--supersedes `/`--credit ` for a `Related:` PR only when the owner confirms; rerun after any change. The completion message lists unpromoted candidates, `Related:` lines and any `ISSUE_LOOKUP_FAILED`. diff --git a/test/helpers/pregate-fixture.ts b/test/helpers/pregate-fixture.ts index ebc08ed91..3d1e819e5 100644 --- a/test/helpers/pregate-fixture.ts +++ b/test/helpers/pregate-fixture.ts @@ -162,17 +162,19 @@ export function cleanup(repo: FixtureRepo): void { /** * A `gh` on PATH answering `gh issue view --json ...` from a map, `gh issue list` / `gh run view` from fixed data, - * `gh repo view` from `repo` (null fails) and the closing-PR `gh api graphql` query from `closing` (issue → pages of - * nodes; `after=c` reads page i; a missing issue fails like an API error). + * `gh repo view` from `repo` (null fails), the closing-PR `gh api graphql` query from `closing` (issue → pages of + * nodes; `after=c` reads page i; a missing issue fails like an API error) and the PR-author query (`head=`) + * from `headAuthor` (the HEAD commit's GitHub user, null when GitHub has no such commit) and `ownerType`. */ -export function ghShim(repo: FixtureRepo, data: { issues?: Record; issueList?: unknown[]; runs?: Record; repo?: string | null; viewer?: string; closing?: Record }): { PATH: string } { +export function ghShim(repo: FixtureRepo, data: { issues?: Record; issueList?: unknown[]; runs?: Record; repo?: string | null; viewer?: string; closing?: Record; headAuthor?: string | null; ownerType?: 'User' | 'Organization' }): { PATH: string } { const dir = path.join(repo.root, 'gh-shim'); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, 'data.json'), JSON.stringify({ issues: {}, issueList: [], runs: {}, repo: 'owner/repo', viewer: 'me', closing: {}, ...data })); + fs.writeFileSync(path.join(dir, 'data.json'), JSON.stringify({ issues: {}, issueList: [], runs: {}, repo: 'owner/repo', viewer: 'me', closing: {}, headAuthor: null, ownerType: 'User', ...data })); fs.writeFileSync(path.join(dir, 'api.js'), [ 'const d = require(process.argv[2]); const a = process.argv.slice(3);', 'const arg = k => { const i = a.findIndex(x => x.startsWith(k + "=")); return i < 0 ? undefined : a[i].slice(k.length + 1); };', 'if (a[0] === "repo") { if (d.repo === null) { console.error("no git remotes found"); process.exit(1); } console.log(JSON.stringify({ nameWithOwner: d.repo })); process.exit(0); }', + 'if (arg("head") !== undefined) { console.log(JSON.stringify({ data: { repository: { owner: { __typename: d.ownerType, login: d.repo.split("/")[0] }, object: d.headAuthor === null ? null : { author: { user: { login: d.headAuthor } } } } } })); process.exit(0); }', 'const pages = d.closing[arg("number")]; if (!pages) { console.error("HTTP 502: Bad Gateway (https://api.github.com/graphql)"); process.exit(1); }', 'const i = Number((arg("after") || "c0").slice(1));', 'console.log(JSON.stringify({ data: { viewer: { login: d.viewer }, repository: { issue: { closedByPullRequestsReferences: { nodes: pages[i] || [], pageInfo: { hasNextPage: i + 1 < pages.length, endCursor: "c" + (i + 1) } } } } } }));', diff --git a/test/ship-issue-intent.test.ts b/test/ship-issue-intent.test.ts index 7c1d40a7e..d59c1deb9 100644 --- a/test/ship-issue-intent.test.ts +++ b/test/ship-issue-intent.test.ts @@ -223,3 +223,30 @@ describe('gstack-issue-links (bin): related PRs, flake validation, lookup failur expect(fs.readFileSync(body, 'utf8').startsWith('```gstack-ship-receipt')).toBe(true); }); }); + +describe('gstack-issue-links (bin): this PR\'s author when gh runs as an app', () => { + const ownPr = { number: 8, state: 'OPEN', headRefName: 'other-wave', author: { login: 'garrytan' } }; + const contrib = { number: 7, state: 'OPEN', headRefName: 'contrib', author: { login: 'alice' } }; + const setup = (data: Parameters[1]) => { + const repo = fixture(); + commit(repo, { 'lib/util.js': "exports.add = (a, b) => a + b + 0;\n" }, 'fix: tidy\n\nFixes #14\n'); + return { repo, env: ghShim(repo, { repo: 'garrytan/gbrain', viewer: 'capy-ai[bot]', issues: { '14': { title: 't', body: 'b', comments: [] } }, closing: { '14': [[contrib, ownPr]] }, ...data }) }; + }; + + test('an app viewer is replaced by the HEAD commit author, else a user-owned repository\'s owner, so the owner\'s own PRs are not Related', () => { + const byCommit = setup({ headAuthor: 'GarryTan', ownerType: 'Organization' }); + const r = runBin(byCommit.repo, 'gstack-issue-links', ['--base', 'main', '--json'], byCommit.env); + expect(JSON.parse(r.out)).toMatchObject({ lines: ['Fixes #14', 'Related: #7 (@alice)'], author: { login: 'GarryTan', source: 'HEAD commit author' }, unavailable: [] }); + const byOwner = setup({}); + expect(JSON.parse(runBin(byOwner.repo, 'gstack-issue-links', ['--base', 'main', '--json'], byOwner.env).out)).toMatchObject({ lines: ['Fixes #14', 'Related: #7 (@alice)'], author: { login: 'garrytan', source: 'repository owner' } }); + expect(JSON.parse(runBin(byOwner.repo, 'gstack-issue-links', ['--base', 'main', '--author', 'alice', '--json'], byOwner.env).out)).toMatchObject({ lines: ['Fixes #14', 'Related: #8 (@garrytan)'], author: { login: 'alice', source: '--author' } }); + }); + + test('an app viewer with no user to stand in keeps every PR and reports the gap as ISSUE_LOOKUP_FAILED', () => { + const { repo, env } = setup({ ownerType: 'Organization' }); + const r = runBin(repo, 'gstack-issue-links', ['--base', 'main', '--json'], env); + expect(r.status).toBe(EXIT.ok); + expect(JSON.parse(r.out)).toMatchObject({ lines: ['Fixes #14', 'Related: #7 (@alice)', 'Related: #8 (@garrytan)'], author: { login: 'capy-ai[bot]', source: 'gh viewer' } }); + expect(JSON.parse(r.out).unavailable).toEqual(['this PR\'s author: gh runs as capy-ai[bot] and neither the HEAD commit nor the repository owner names a user; pass --author ']); + }); +});