From 4affd647bf7b30fee7383ef31be68a19d48805ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:52:25 +0000 Subject: [PATCH 1/3] Initial plan From 298f9d39726c513881151b2b0b721b7f88f189a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 8 Sep 2026 23:13:34 +0000 Subject: [PATCH 2/3] fix: tighten repository reconciliation tracking Co-authored-by: groupthinking <154503486+groupthinking@users.noreply.github.com> --- .../workflows/repository-reconciliation.yml | 20 +- ...test_repository_reconciliation_workflow.py | 202 ++++++++++++++++++ 2 files changed, 220 insertions(+), 2 deletions(-) diff --git a/.github/workflows/repository-reconciliation.yml b/.github/workflows/repository-reconciliation.yml index ba9297f11..735d4188a 100644 --- a/.github/workflows/repository-reconciliation.yml +++ b/.github/workflows/repository-reconciliation.yml @@ -4,6 +4,22 @@ on: schedule: - cron: "17 13 * * *" workflow_dispatch: + pull_request_target: + types: + - opened + - reopened + - edited + - synchronize + - ready_for_review + - converted_to_draft + - closed + issues: + types: + - opened + - reopened + - closed + create: + delete: permissions: contents: read @@ -27,7 +43,7 @@ jobs: const repoFullName = `${owner}/${repo}`; const now = Date.now(); const staleAfterMs = 14 * 24 * 60 * 60 * 1000; - const closingPattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)/gi; + const closingPattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)?#(\d+)/gi; const pulls = await github.paginate(github.rest.pulls.list, { owner, repo, state: "open", per_page: 100 @@ -56,7 +72,7 @@ jobs: for (const issueNum of allIssueNumbers) { try { const resp = await github.rest.issues.get({ owner, repo, issue_number: issueNum }); - if (!resp.data.pull_request && resp.data.state === "open") { + if (!resp.data.pull_request) { validIssues.add(issueNum); } } catch (err) { diff --git a/tests/unit/test_repository_reconciliation_workflow.py b/tests/unit/test_repository_reconciliation_workflow.py index d7142f691..c82e188e5 100644 --- a/tests/unit/test_repository_reconciliation_workflow.py +++ b/tests/unit/test_repository_reconciliation_workflow.py @@ -1,7 +1,11 @@ from __future__ import annotations +import json +import shutil +import subprocess from pathlib import Path +import pytest import yaml WORKFLOW_PATH = ( @@ -22,6 +26,127 @@ def _get_script(workflow: dict) -> str: return script_step["with"]["script"] +def _run_reconciliation(tmp_path: Path, scenario: dict) -> dict: + """Execute the workflow's real reconciliation script against stubbed GitHub APIs.""" + node = shutil.which("node") + if node is None: # pragma: no cover - depends on the runner image + pytest.skip("node is required to execute the workflow's inline script") + + script_path = tmp_path / "repository_reconciliation.js" + script_path.write_text(_get_script(_load_workflow()), encoding="utf-8") + scenario_path = tmp_path / "scenario.json" + scenario_path.write_text(json.dumps(scenario), encoding="utf-8") + driver_path = tmp_path / "driver.js" + driver_path.write_text( + """ +const fs = require("fs"); +const vm = require("vm"); + +const [scriptPath, scenarioPath] = process.argv.slice(2); +const script = fs.readFileSync(scriptPath, "utf8"); +const scenario = JSON.parse(fs.readFileSync(scenarioPath, "utf8")); +const actions = { issueUpdates: [], issueCreates: [], comments: [], pullUpdates: [], infos: [] }; + +const pullsList = async () => ({ data: scenario.pulls || [] }); +pullsList.__tag = "pulls.list"; +const listBranches = async () => ({ data: scenario.branches || [] }); +listBranches.__tag = "repos.listBranches"; +const listComments = async ({ issue_number }) => ({ data: (scenario.commentsByIssue || {})[issue_number] || [] }); +listComments.__tag = "issues.listComments"; + +const github = { + paginate: async (fn, params) => { + switch (fn.__tag) { + case "pulls.list": + return scenario.pulls || []; + case "repos.listBranches": + return scenario.branches || []; + case "issues.listComments": + return ((scenario.commentsByIssue || {})[params.issue_number]) || []; + default: + throw new Error(`Unsupported paginate call: ${fn.__tag}`); + } + }, + rest: { + pulls: { + list: pullsList, + update: async (payload) => { + actions.pullUpdates.push(payload); + return { data: payload }; + }, + }, + repos: { + listBranches, + getCommit: async ({ ref }) => ({ + data: { + commit: { + committer: { date: ((scenario.commitsBySha || {})[ref]) || "2026-07-01T00:00:00Z" }, + }, + }, + }), + }, + issues: { + get: async ({ issue_number }) => { + const issue = (scenario.issuesByNumber || {})[issue_number]; + if (!issue) { + const err = new Error(`Missing issue ${issue_number}`); + err.status = 404; + throw err; + } + return { data: issue }; + }, + update: async (payload) => { + actions.issueUpdates.push(payload); + return { data: payload }; + }, + create: async (payload) => { + actions.issueCreates.push(payload); + return { data: { number: scenario.createdReportNumber || 999 } }; + }, + listComments, + createComment: async (payload) => { + actions.comments.push(payload); + return { data: payload }; + }, + }, + search: { + issuesAndPullRequests: async () => ({ + data: { + items: scenario.existingReport ? [scenario.existingReport] : [], + }, + }), + }, + }, +}; + +const context = { repo: { owner: "groupthinking", repo: "EventRelay" } }; +const core = { info: (message) => actions.infos.push(message) }; + +(async () => { + await vm.runInNewContext( + `(async () => {${script}\\n})()`, + { context, github, core, console, Date, Set, Map, Number, Error }, + ); + process.stdout.write(JSON.stringify(actions)); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +""", + encoding="utf-8", + ) + + result = subprocess.run( + [node, str(driver_path), str(script_path), str(scenario_path)], + capture_output=True, + text=True, + timeout=60, + check=False, + ) + assert result.returncode == 0, f"driver failed: {result.stderr}" + return json.loads(result.stdout) + + def test_reconciliation_workflow_file_is_valid_yaml() -> None: workflow = _load_workflow() assert workflow["name"] == "Repository Reconciliation" @@ -37,6 +162,23 @@ def test_reconciliation_workflow_triggers_on_schedule_and_dispatch() -> None: assert len(crons) >= 1 +def test_reconciliation_workflow_reacts_to_repo_state_changes() -> None: + """The report must refresh when PR, issue, or branch state changes.""" + triggers = _load_workflow()[True] + assert triggers["pull_request_target"]["types"] == [ + "opened", + "reopened", + "edited", + "synchronize", + "ready_for_review", + "converted_to_draft", + "closed", + ] + assert triggers["issues"]["types"] == ["opened", "reopened", "closed"] + assert "create" in triggers + assert "delete" in triggers + + def test_reconciliation_workflow_minimum_permissions() -> None: workflow = _load_workflow() perms = workflow["permissions"] @@ -102,3 +244,63 @@ def test_reconciliation_workflow_report_is_idempotent() -> None: # Should update the existing issue if found, otherwise create a new one. assert "issues.update" in script assert "issues.create" in script + + +def test_reconciliation_accepts_repo_qualified_issue_references(tmp_path: Path) -> None: + """Canonical references like owner/repo#123 must not be reported missing.""" + outcome = _run_reconciliation( + tmp_path, + { + "pulls": [ + { + "number": 903, + "title": "fix(auth): restore Google OAuth configuration in Vercel production", + "body": "## Canonical issue\\n\\nCloses groupthinking/EventRelay#900", + "draft": False, + "head": { + "ref": "jules-15243187445261469621-ffdb089e", + "repo": {"full_name": "groupthinking/EventRelay"}, + }, + } + ], + "branches": [], + "issuesByNumber": {"900": {"state": "open"}}, + "existingReport": {"number": 1584, "title": "[automation] Repository drift report"}, + }, + ) + + assert outcome["comments"] == [] + assert ( + "- Ready PRs without exactly one canonical issue: **0**" + in outcome["issueUpdates"][0]["body"] + ) + + +def test_reconciliation_keeps_closed_canonical_issues_tracked(tmp_path: Path) -> None: + """A PR linked to one real issue stays canonical even after that issue closes.""" + outcome = _run_reconciliation( + tmp_path, + { + "pulls": [ + { + "number": 1673, + "title": "Canonicalize retired routes into the Studio workbench", + "body": "## Canonical issue\\n\\nCloses #1669", + "draft": False, + "head": { + "ref": "copilot/make-home-sell-page-and-route-studio", + "repo": {"full_name": "groupthinking/EventRelay"}, + }, + } + ], + "branches": [], + "issuesByNumber": {"1669": {"state": "closed"}}, + "existingReport": {"number": 1584, "title": "[automation] Repository drift report"}, + }, + ) + + assert outcome["comments"] == [] + assert ( + "- Ready PRs without exactly one canonical issue: **0**" + in outcome["issueUpdates"][0]["body"] + ) From 41c8ec52d49446e021494b4195e867d01b18551c Mon Sep 17 00:00:00 2001 From: "vercel[bot]" <35613825+vercel[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 09:05:21 +0000 Subject: [PATCH 3/3] Fix: Broadened closing-reference regex accepts foreign `owner/repo#N` references and silently maps them onto local issue numbers, mis-binding PRs and fabricating duplicate groups. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes the issue reported at .github/workflows/repository-reconciliation.yml:40 ## Bug The closing-reference regex was broadened from `...#(\d+)` to: ```js /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#(\d+)/gi ``` The `(?:[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?` prefix group is **non-capturing** and matches **any** `owner/repo`, but it is never compared against `repoFullName` (`${owner}/${repo}`). Only the trailing number is captured (`m[1]`), and downstream it is validated/classified against the **local** repo: ```js const resp = await github.rest.issues.get({ owner, repo, issue_number: issueNum }); ``` ### Concrete trigger A PR body containing `Closes otherorg/otherrepo#42` is parsed as issue `42`. It is then: 1. Validated via `issues.get({ owner, repo, issue_number: 42 })` against the **local** repo — if a local issue #42 exists, the PR is treated as canonically tracked by the wrong issue. 2. Added to `issueToPulls.get(42)`, so if another local PR legitimately references local #42, they are reported as a **false "competing PRs" duplicate group**, and the superseded-draft auto-close remediation could then act on them. This silently binds a PR to the wrong (local) issue and can trigger destructive remediation (auto-closing draft PRs). ## Fix - Made the repo prefix a **capturing** group so it can be inspected; the issue number is now `m[2]`. - Added an `extractIssueNumbers(body)` helper that accepts a reference only when the prefix is **absent** (unqualified `#N`) or **equal to the current repo's full name** (`owner/repo#N`), discarding cross-repo references. - Replaced both matchAll sites (issue-number collection and per-PR classification) with the helper. Verified via a standalone Node script: ``` unqualified: [ 900 ] local qualified: [ 900 ] foreign: [] mixed: [ 5, 7 ] ``` Foreign references are dropped while the intended canonical repo-qualified form (`groupthinking/EventRelay#900`) is still accepted. Co-authored-by: Vercel Co-authored-by: groupthinking --- .github/workflows/repository-reconciliation.yml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/repository-reconciliation.yml b/.github/workflows/repository-reconciliation.yml index 735d4188a..5c00cb33c 100644 --- a/.github/workflows/repository-reconciliation.yml +++ b/.github/workflows/repository-reconciliation.yml @@ -43,7 +43,16 @@ jobs: const repoFullName = `${owner}/${repo}`; const now = Date.now(); const staleAfterMs = 14 * 24 * 60 * 60 * 1000; - const closingPattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+(?:[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)?#(\d+)/gi; + const closingPattern = /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+([A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)?#(\d+)/gi; + + // Extract closing-issue references from a PR body, accepting only references that are + // unqualified (`#N`) or explicitly qualified with THIS repository + // (`owner/repo#N`). Cross-repo references like `Closes otherorg/otherrepo#42` are + // ignored so they are never mapped onto local issue numbers. + const extractIssueNumbers = (body) => + [...(body || "").matchAll(closingPattern)] + .filter(m => !m[1] || m[1] === repoFullName) + .map(m => Number(m[2])); const pulls = await github.paginate(github.rest.pulls.list, { owner, repo, state: "open", per_page: 100 @@ -65,7 +74,7 @@ jobs: // references like "Closes #999999" from creating fictitious duplicate groups. const allIssueNumbers = new Set(); for (const pr of pulls) { - const nums = [...(pr.body || "").matchAll(closingPattern)].map(m => Number(m[1])); + const nums = extractIssueNumbers(pr.body); nums.forEach(n => allIssueNumbers.add(n)); } const validIssues = new Set(); @@ -84,8 +93,7 @@ jobs: const untracked = []; const issueToPulls = new Map(); for (const pr of pulls) { - const issues = [...(pr.body || "").matchAll(closingPattern)] - .map(match => Number(match[1])); + const issues = extractIssueNumbers(pr.body); // Restrict to validated issue references only. const validUnique = [...new Set(issues)].filter(n => validIssues.has(n)); // Drafts mirror the governance workflow's deferred-enforcement rule and are excluded.