From dc594303e3c0febcd2ca2b07bb546741761293c5 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 17 Aug 2026 11:17:22 +0000 Subject: [PATCH] fix(tcfeed): surface requests that were asked and never sent a diff The issue promises "a pull request is open alongside this with the diff". When that second half does not happen, nothing notices: `check` discovers its work by searching for open pull requests, so a repository holding a standing issue and no request is invisible to it permanently. james-6-23/codex2api sat in that state until the maintainer asked where the diff was, which is the wrong person to be running the check. `check` now also lists the repositories that were asked and never shown a diff, before the "no open requests" early return, since that is exactly the case where there is no pull request to report. Named, not repaired. `tcfeed pr ` already finishes the pair on purpose, carrying the standing issue through so the request links back to it, and opening a request against somebody's repository should stay something typed rather than a side effect of asking for status. Also: the issue said "Two files under `.github/`" while openPr had been cut back to the workflow alone, so codex2api was asked on that sentence and then shown a one-file diff. Both now read the count off one INSTALLS list, so the question and the answer cannot disagree about the size of the change. Co-Authored-By: Claude Opus 5 (1M context) --- bin/tcfeed.ts | 116 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 13 deletions(-) diff --git a/bin/tcfeed.ts b/bin/tcfeed.ts index 3af3e08..bdf1557 100755 --- a/bin/tcfeed.ts +++ b/bin/tcfeed.ts @@ -635,6 +635,20 @@ async function scan( /** The branch, and therefore the identity of the request. One per repository, ever. */ const PR_BRANCH = 'threatcrush-scan'; +/** + * What a request installs, named once. + * + * The issue and the pull request describe the same diff to the same person, + * and they drifted. openPr was cut back to the workflow alone when the + * converter went away; issueBody went on promising "Two files under + * `.github/`" to every repository asked afterwards. james-6-23/codex2api was + * asked on that sentence and then shown a one-file diff, and asked about it. + * + * Both read the count off this list now, so the question and the answer cannot + * disagree about how big the change is. + */ +const INSTALLS = [{ source: 'workflow.yml', destination: '.github/workflows/threatcrush-scan.yml' }]; + /** * The pack, read from sh1pt rather than copied into this file. * @@ -1334,7 +1348,8 @@ const issueBody = (spec: string): string => 'hardcoded credentials, injection, SSRF and unsafe deserialisation, and writes', 'findings to the Security tab. Report-only — findings never fail the build.', '', - `Two files under \`.github/\`, a pinned \`${spec}\` whose tarball is hashed before`, + `${INSTALLS.length === 1 ? 'One file' : `${INSTALLS.length} files`} under \`.github/\`, a pinned` + + ` \`${spec}\` whose tarball is hashed before`, 'install, and `pull_request` rather than `pull_request_target`.', '', 'A pull request is open alongside this with the diff, if reading it is easier', @@ -1385,17 +1400,15 @@ async function openPr( const pack = packDir(); const spec = await resolveSpec(); const inputs = packInputs(spec, await resolveIntegrity(spec)); - // One file. `refresh` already installed only the workflow and actively - // removed the converter from branches that still carried it, but this path — - // the one that opens a *new* request — went on writing both, so every fresh - // offer re-added the file refresh existed to take away. A repository was - // told the diff adds one file and then shown two. - const files = [ - { - destination: '.github/workflows/threatcrush-scan.yml', - content: render(fs.readFileSync(path.join(pack, 'workflow.yml'), 'utf8'), inputs), - }, - ]; + // `refresh` already installed only the workflow and actively removed the + // converter from branches that still carried it, but this path — the one + // that opens a *new* request — went on writing both, so every fresh offer + // re-added the file refresh existed to take away. Read from INSTALLS, so the + // issue cannot describe a different diff than the one pushed here. + const files = INSTALLS.map((file) => ({ + destination: file.destination, + content: render(fs.readFileSync(path.join(pack, file.source), 'utf8'), inputs), + })); const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'tcfeed-pr-')); const src = path.join(tmp, 'src'); @@ -2353,9 +2366,71 @@ async function refreshOne(repo: string, me: string, spec: string): Promise` already finishes the pair + * deliberately — prTarget carries the standing issue through so the request + * links back to it — and opening a request against somebody's repository + * should stay something typed, not a side effect of asking for status. + */ +async function unpaired(me: string): Promise { + // One repository per line, not a JSON array. `--paginate` concatenates a + // separate document per page, so an array-shaped --jq gives back two arrays + // and a single JSON.parse of the pair throws — a failure that only appears + // once the account passes a hundred open issues, which is the worst moment + // for it to appear. + const raised = await gh([ + 'api', + '-XGET', + 'search/issues', + '-f', + `q=author:${me} type:issue state:open in:title "${ISSUE_TITLE}"`, + '-f', + 'per_page=100', + '--paginate', + '--jq', + '.items[] | select(.state == "open") | .repository_url', + ]).catch(() => ''); + + const repos = [ + ...new Set( + raised + .split('\n') + .filter(Boolean) + .map((url) => url.trim().replace(/^.*\/repos\//, '')) + ), + ].sort(); + + const missing: string[] = []; + for (const repo of repos) { + // state=all: a request that was opened and then closed still counts as + // sent, and re-sending one a maintainer closed is the exact thing the + // ask-once rule exists to prevent. + const said = await gh([ + 'api', + `repos/${repo}/pulls?state=all&head=${me}:${PR_BRANCH}&per_page=1`, + ]).catch(() => ''); + // Unreadable is not absent. A 404 here is a repository that takes no pull + // requests at all, and a promise that cannot be kept is not one to chase. + if (!said) continue; + if ((JSON.parse(said) as unknown[]).length === 0) missing.push(repo); + } + return missing; +} + /** * The subcommand. With no arguments it reads every repository this has an open - * request on, which is the list it is entitled to act on and no wider. + * request on, which is the list it is entitled to act on and no wider, plus the + * repositories it asked and never sent a diff to — named only, never acted on. */ async function checkCommand(argv: string[]): Promise { const fix = argv.includes('--fix'); @@ -2376,6 +2451,7 @@ async function checkCommand(argv: string[]): Promise { } let repos = named; + let owed: string[] = []; if (repos.length === 0) { // Found by searching for the requests themselves rather than by keeping a // list on disk. A file would drift the first time one was opened by hand. @@ -2388,6 +2464,20 @@ async function checkCommand(argv: string[]): Promise { ]) ) as string[]; repos = [...new Set(found.map((url) => url.replace(/^.*\/repos\//, '')))].sort(); + + // Only when nothing was named. The named form means "check these", not + // "audit the account", and it is also the form scripts call. + owed = await unpaired(me); + } + + // Printed before the early return below, because a repository that was asked + // and never sent the diff is precisely the case where there is no open pull + // request to report — the run would otherwise say "no open requests" and + // nothing at all about the promise still outstanding. + if (owed.length > 0) { + console.log(`${owed.length} asked, no diff sent — the issue promised one:`); + for (const repo of owed) console.log(` ${repo} — tcfeed pr ${repo}`); + console.log(''); } if (repos.length === 0) {