From 13aaf5a19af2e4dd5849e7e4d4874c04eb13f683 Mon Sep 17 00:00:00 2001 From: Jonathan Burdge Date: Fri, 9 Oct 2026 12:31:10 +0300 Subject: [PATCH 1/2] Put preview links directly in PR conversation comments --- .github/workflows/publish-pages.yml | 3 +- README.md | 2 +- docs/development.md | 4 +- scripts/pages_preview.py | 59 +++++++++++++++++++++++++- tests/test_pages_preview.py | 66 ++++++++++++++++++++++++++++- 5 files changed, 128 insertions(+), 6 deletions(-) diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index 4d9406d..a008abf 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -25,6 +25,7 @@ jobs: pages: write id-token: write checks: write + pull-requests: write environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} @@ -53,7 +54,7 @@ jobs: - name: Publish dashboard and previews id: deployment uses: actions/deploy-pages@v5 - - name: Link deployed previews from PR checks + - name: Link deployed previews on PRs env: GH_TOKEN: ${{ github.token }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} diff --git a/README.md b/README.md index 55e919a..a45b80f 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Files opened here stay in your browser; they are not uploaded or included in lin - Edit or add a self-contained eval file in [configs/evals/](configs/evals/), such as [polymath.yaml](configs/evals/polymath.yaml). Each file holds its scoring rules and language assignments together; the catalogue combines them automatically. - Add weighting profiles to [configs/weights/](configs/weights/), or optional named eval sets to [configs/sets/](configs/sets/). Each directory has a `default.txt` choosing its startup selection. See [contributing configs](configs/README.md). -Use a pull request or GitHub’s **Add file → Upload files**. Changes on `main` trigger tests and a GitHub Pages rebuild. Successful PR builds get a separate review page: open **Details** on the PR’s **Pages preview** check, or use the [preview index](https://openeurollm.github.io/quickdash/pr-preview/). Previews update after successful builds and are removed when the PR closes. Invalid inputs leave the last successful page online. The repository, dashboard, and PR previews are public, so use browser imports for private comparisons. +Use a pull request or GitHub’s **Add file → Upload files**. Changes on `main` trigger tests and a GitHub Pages rebuild. Successful PR builds get a separate review page: click **Open preview** in the bot comment on the PR conversation. CI updates that same comment with the built commit and labels previews from an older successful build. The **Pages preview** check and [preview index](https://openeurollm.github.io/quickdash/pr-preview/) also link to it. Previews update after successful builds and are removed when the PR closes. Invalid inputs leave the last successful page online. The repository, dashboard, and PR previews are public, so use browser imports for private comparisons. The [build workflow](.github/workflows/pages.yml) checks and packages the dashboard, fictional demo, and licenses. The [Pages publisher](.github/workflows/publish-pages.yml) combines the main dashboard with previews at `pr-preview/pr-/`. Repository maintainers configure **Settings → Pages → Source → GitHub Actions**. Check the repository’s **Actions** tab if an update fails to appear; see [publishing and previews](docs/development.md#publish-through-github-pages). diff --git a/docs/development.md b/docs/development.md index bb88181..dad9e4d 100644 --- a/docs/development.md +++ b/docs/development.md @@ -119,7 +119,9 @@ The publisher deploys one combined site: - `/quickdash/pr-preview/pr-/` serves that PR’s latest published successful build. - `/quickdash/pr-preview/` lists open PRs, preview links, and the exact built commits. It labels a retained preview as **Previous successful build** when the current PR head has no successful build yet. -After deployment succeeds, a **Pages preview** check on each current built PR commit links directly to its preview. No PR comments are posted. Closing or merging a PR removes its directory on the next publisher run. A failed main build preserves production; a failed PR build preserves its previous preview. Review both the ordinary `build` check and the built commit before assessing a preview. Previews are public and share the Pages origin with the main dashboard. +After deployment succeeds, the publisher creates or updates one bot comment in each PR conversation with a prominent **Open preview** link and the built commit. It reuses the comment without writing when the text is unchanged, and only edits comments owned by `github-actions[bot]` with the publisher’s hidden marker. A preview from an older successful build is labelled explicitly. Closing or merging a PR removes its directory and changes its existing comment to **Preview closed** after deployment succeeds; pending comment cleanup persists across failed deployments and retries. Reopening the PR lets the next successful deployment reuse the comment. + +A **Pages preview** check on each current built PR commit also includes an **Open preview** link in its summary. A failed main build preserves production; a failed PR build preserves its previous preview. Review both the ordinary `build` check and the built commit before assessing a preview. Previews are public and share the Pages origin with the main dashboard. The trusted publisher has `pull-requests: write` for comments; PR build jobs retain read-only permissions. Generated files and their source run/commit identifiers persist on the `pages-content` branch, created automatically by the first publisher run. This is storage for the Actions publisher; **do not change Pages to deploy from this branch**. The stored files keep published previews available after their source artifacts expire. Every publisher run reconciles all open PRs and successful main builds, and publishing is serialized so concurrent builds cannot overwrite each other’s previews. Run **Publish dashboard and PR previews** manually to retry a failed deployment or reconcile missed updates. The workflow must be on `main` before automatic previews can run; existing successful artifacts can be picked up during that first deployment. diff --git a/scripts/pages_preview.py b/scripts/pages_preview.py index 6c8a1af..233032e 100644 --- a/scripts/pages_preview.py +++ b/scripts/pages_preview.py @@ -18,6 +18,7 @@ # The current dashboard is about 14 MiB. Bound both archive layers before reading. MAX_BYTES = 100 * 1024 * 1024 BUILD_FIELDS = ("run_id", "run_attempt", "sha", "artifact_id", "run_url") +COMMENT_MARKER = "" def read_bundle(data): @@ -75,6 +76,16 @@ def pulls(self): return results page += 1 + def comments(self, number): + results = [] + page = 1 + while True: + batch = self.get(f"issues/{number}/comments?per_page=100&page={page}") + results.extend(batch) + if len(batch) < 100: + return results + page += 1 + def latest_build(self, pr=None): filters = dict(status="success", per_page=100) if pr is None: @@ -147,12 +158,17 @@ def reconcile(site, source): pulls = source.pulls() open_numbers = {str(pr["number"]) for pr in pulls} + retired = set(state.get("retired_previews", [])) - open_numbers preview_dir = site / "pr-preview" preview_dir.mkdir(exist_ok=True) for number in list(state["previews"]): if number not in open_numbers: shutil.rmtree(preview_dir / ("pr-" + number)) del state["previews"][number] + retired.add(number) + # Persist cleanup until post-deployment reporting succeeds, including retries + # and coalesced close events. A reopened PR no longer needs a closed notice. + state["retired_previews"] = sorted(retired) warnings = [] for pr in pulls: number = str(pr["number"]) @@ -192,7 +208,7 @@ def reconcile(site, source): def report_checks(state, api, base_url): - """Attach the deployed URL to the PR's commit as a check, without PR comments.""" + """Attach the deployed URL to the PR's commit as a check.""" for pr in api.pulls(): number = str(pr["number"]) preview = state["previews"].get(number) @@ -212,6 +228,40 @@ def report_checks(state, api, base_url): api.write("POST", "check-runs", dict(payload, head_sha=preview["sha"])) +def update_comment(api, number, body, create=True): + existing = next((comment for comment in api.comments(number) + if comment.get("user", {}).get("login") == "github-actions[bot]" + and comment["user"].get("type") == "Bot" + and (comment.get("body") or "").startswith(COMMENT_MARKER)), None) + if existing: + if existing["body"] != body: + api.write("PATCH", f"issues/comments/{existing['id']}", {"body": body}) + elif create: + api.write("POST", f"issues/{number}/comments", {"body": body}) + + +def report_comments(state, api, base_url): + """Keep one discoverable preview link in each PR conversation.""" + open_prs = {str(pr["number"]): pr for pr in api.pulls()} + for number, pr in open_prs.items(): + preview = state["previews"].get(number) + if not preview: + continue + url = f"{base_url.rstrip('/')}/pr-preview/pr-{number}/" + body = (f"{COMMENT_MARKER}\n" + f"**🚀 [Open preview]({url})**\n\n" + f"Built from `{preview['sha'][:12]}` · [Build details]({preview['run_url']})\n\n") + if preview["sha"] != pr["head"]["sha"]: + body += "This preview is from a previous successful build; it does not include the latest PR commit.\n\n" + body += "This link updates after successful builds. The preview is removed when this PR closes." + update_comment(api, number, body) + for number in state.get("retired_previews", []): + if number not in open_prs: + update_comment(api, number, f"{COMMENT_MARKER}\n**Preview closed**\n\n" + "This PR is closed and its preview has been removed.", create=False) + state["retired_previews"] = [] + + def git(*args, **kwargs): return subprocess.run(["git", *args], check=True, **kwargs) @@ -248,7 +298,12 @@ def main(): args = parser.parse_args() api = GitHub(args.repository, args.default_branch) if args.command == "report": - report_checks(json.loads((args.site / STATE_FILE).read_text()), api, args.base_url) + state_path = args.site / STATE_FILE + state = json.loads(state_path.read_text()) + report_checks(state, api, args.base_url) + report_comments(state, api, args.base_url) + state_path.write_text(json.dumps(state, indent=2, sort_keys=True) + "\n") + save_worktree(args.site) else: if args.command == "assemble": prepare_worktree(args.site) diff --git a/tests/test_pages_preview.py b/tests/test_pages_preview.py index a454c66..9928bd3 100644 --- a/tests/test_pages_preview.py +++ b/tests/test_pages_preview.py @@ -176,6 +176,69 @@ def test_report_only_links_current_deployed_heads_and_updates_existing_check(sel pages.report_checks(state, api, "https://org.github.io/repo/") self.assertEqual(write.call_args.args[:2], ("PATCH", "check-runs/123")) + def test_comment_is_created_once_then_updated_and_unchanged_reports_do_not_write(self): + api = pages.GitHub("org/repo", "main") + state = {"previews": {"11": build(10)}, "retired_previews": []} + comments = [{"id": 1, "user": {"login": "reviewer", "type": "User"}, + "body": "\nHuman comment"}] + with patch.object(api, "pulls", return_value=[pull(11)]), patch.object(api, "comments", return_value=comments), patch.object(api, "write") as write: + pages.report_comments(state, api, "https://org.github.io/repo/") + method, endpoint, payload = write.call_args.args + self.assertEqual((method, endpoint), ("POST", "issues/11/comments")) + self.assertIn("[Open preview](https://org.github.io/repo/pr-preview/pr-11/)", payload["body"]) + self.assertIn("abc", payload["body"]) + comments.append({"id": 2, "user": {"login": "github-actions[bot]", "type": "Bot"}, "body": payload["body"]}) + write.reset_mock() + pages.report_comments(state, api, "https://org.github.io/repo") + write.assert_not_called() + state["previews"]["11"] = build(20, "new") + pages.report_comments(state, api, "https://org.github.io/repo") + self.assertEqual(write.call_args.args[:2], ("PATCH", "issues/comments/2")) + self.assertIn("previous successful build", write.call_args.args[2]["body"]) + self.assertIn("new", write.call_args.args[2]["body"]) + + def test_closed_comments_are_updated_after_removal_and_retried_until_report_succeeds(self): + source = Source() + with tempfile.TemporaryDirectory() as tmp: + site = Path(tmp) + pages.reconcile(site, source) + source.open = [] + state = pages.reconcile(site, source) + self.assertEqual(set(state["retired_previews"]), {"11", "12"}) + self.assertFalse((site / "pr-preview/pr-11").exists()) + # A deployment or reporting failure leaves cleanup pending in stored state. + self.assertEqual(pages.reconcile(site, source)["retired_previews"], state["retired_previews"]) + api = pages.GitHub("org/repo", "main") + comment = {"id": 5, "user": {"login": "github-actions[bot]", "type": "Bot"}, + "body": "\nOld link"} + with patch.object(api, "pulls", return_value=[]), patch.object(api, "comments", return_value=[comment]), patch.object(api, "write", side_effect=RuntimeError("API unavailable")): + with self.assertRaisesRegex(RuntimeError, "API unavailable"): + pages.report_comments(state, api, "https://org.github.io/repo") + self.assertEqual(set(state["retired_previews"]), {"11", "12"}) + with patch.object(api, "pulls", return_value=[]), patch.object(api, "comments", side_effect=lambda n: [comment] if n == "11" else []), patch.object(api, "write") as write: + pages.report_comments(state, api, "https://org.github.io/repo") + self.assertEqual(write.call_count, 1) + self.assertEqual(write.call_args.args[:2], ("PATCH", "issues/comments/5")) + self.assertIn("Preview closed", write.call_args.args[2]["body"]) + self.assertNotIn("[Open preview]", write.call_args.args[2]["body"]) + self.assertEqual(state["retired_previews"], []) + + def test_reopened_pr_is_not_marked_closed_and_unbuilt_pr_gets_no_comment(self): + api = pages.GitHub("org/repo", "main") + state = {"previews": {}, "retired_previews": ["11"]} + with patch.object(api, "pulls", return_value=[pull(11)]), patch.object(api, "comments") as comments, patch.object(api, "write") as write: + pages.report_comments(state, api, "https://org.github.io/repo") + comments.assert_not_called() + write.assert_not_called() + self.assertEqual(state["retired_previews"], []) + + def test_comment_lookup_paginates_long_pr_discussions(self): + api = pages.GitHub("org/repo", "main") + first = [{"id": n} for n in range(100)] + with patch.object(api, "get", side_effect=[first, [{"id": 100}]]) as get: + self.assertEqual(len(api.comments("11")), 101) + self.assertEqual(get.call_args.args[0], "issues/11/comments?per_page=100&page=2") + def test_worktree_state_round_trip_keeps_source_branch_intact(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -217,7 +280,8 @@ def test_workflow_keeps_pr_builds_unprivileged_and_publisher_on_default_branch(s self.assertEqual(steps[0]["with"]["ref"], "${{ github.event.repository.default_branch }}") self.assertEqual(publish["concurrency"]["group"], "quickdash-pages-publish") self.assertLess(next(i for i,s in enumerate(steps) if s.get("id") == "deployment"), - next(i for i,s in enumerate(steps) if s.get("name") == "Link deployed previews from PR checks")) + next(i for i,s in enumerate(steps) if s.get("name") == "Link deployed previews on PRs")) + self.assertEqual(publish["jobs"]["publish"]["permissions"]["pull-requests"], "write") if __name__ == "__main__": From 85978adb03686a256756af67e996eecc352c7fa4 Mon Sep 17 00:00:00 2001 From: Jonathan Burdge Date: Fri, 9 Oct 2026 12:42:37 +0300 Subject: [PATCH 2/2] Gate previews by OWNERS and commit-specific deploy commands --- .github/workflows/publish-pages.yml | 15 ++-- OWNERS | 3 + README.md | 2 +- docs/development.md | 12 ++- scripts/pages_preview.py | 86 +++++++++++++++++--- tests/test_pages_preview.py | 122 +++++++++++++++++++++++++--- 6 files changed, 208 insertions(+), 32 deletions(-) create mode 100644 OWNERS diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index a008abf..77ead1f 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -6,18 +6,23 @@ on: types: [completed] pull_request_target: types: [closed] + issue_comment: + types: [created] workflow_dispatch: permissions: contents: read -# Reconcile all open PRs on every run: Actions may coalesce pending runs. -concurrency: - group: quickdash-pages-publish - cancel-in-progress: false - jobs: publish: + if: >- + github.event_name != 'issue_comment' || + (github.event.issue.pull_request && startsWith(github.event.comment.body, '/deploy ')) + # Only deployment requests enter the queue; ordinary/bot comments cannot + # replace pending deployments. Every publish reconciles all open PRs. + concurrency: + group: quickdash-pages-publish + cancel-in-progress: false runs-on: ubuntu-24.04 permissions: actions: read diff --git a/OWNERS b/OWNERS new file mode 100644 index 0000000..61d7add --- /dev/null +++ b/OWNERS @@ -0,0 +1,3 @@ +# GitHub users trusted to auto-deploy previews and approve /deploy . +jonabur +spyysalo diff --git a/README.md b/README.md index a45b80f..700e8a0 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ Files opened here stay in your browser; they are not uploaded or included in lin - Edit or add a self-contained eval file in [configs/evals/](configs/evals/), such as [polymath.yaml](configs/evals/polymath.yaml). Each file holds its scoring rules and language assignments together; the catalogue combines them automatically. - Add weighting profiles to [configs/weights/](configs/weights/), or optional named eval sets to [configs/sets/](configs/sets/). Each directory has a `default.txt` choosing its startup selection. See [contributing configs](configs/README.md). -Use a pull request or GitHub’s **Add file → Upload files**. Changes on `main` trigger tests and a GitHub Pages rebuild. Successful PR builds get a separate review page: click **Open preview** in the bot comment on the PR conversation. CI updates that same comment with the built commit and labels previews from an older successful build. The **Pages preview** check and [preview index](https://openeurollm.github.io/quickdash/pr-preview/) also link to it. Previews update after successful builds and are removed when the PR closes. Invalid inputs leave the last successful page online. The repository, dashboard, and PR previews are public, so use browser imports for private comparisons. +Use a pull request or GitHub’s **Add file → Upload files**. Changes on `main` trigger tests and a GitHub Pages rebuild. PRs authored by a user in [OWNERS](OWNERS) get previews automatically after checks pass. For other authors, the bot supplies a copyable `/deploy ` command that an OWNER can post to approve that commit. Later commits require new approval. Click **Open preview** in the bot comment on the PR conversation; CI updates that same comment with the built commit. The **Pages preview** check and [preview index](https://openeurollm.github.io/quickdash/pr-preview/) also link to it. Previews are removed when the PR closes. The repository, dashboard, and PR previews are public, so use browser imports for private comparisons. The [build workflow](.github/workflows/pages.yml) checks and packages the dashboard, fictional demo, and licenses. The [Pages publisher](.github/workflows/publish-pages.yml) combines the main dashboard with previews at `pr-preview/pr-/`. Repository maintainers configure **Settings → Pages → Source → GitHub Actions**. Check the repository’s **Actions** tab if an update fails to appear; see [publishing and previews](docs/development.md#publish-through-github-pages). diff --git a/docs/development.md b/docs/development.md index dad9e4d..20c85aa 100644 --- a/docs/development.md +++ b/docs/development.md @@ -111,7 +111,13 @@ The [workflow](../.github/workflows/pages.yml) runs public tests on pull request After tests pass, it builds the shared dashboard and a separate fictional demo. When `results/` has no CSVs, the shared page embeds `examples/sample-evals.csv`; real shared CSVs take precedence. The sample also runs through both engines in CI for all shipped weighting profiles, eval sets, and aggregation modes, with complete and mismatched coverage. See [contributor requirements](../AGENTS.md). Only `output/site/` is uploaded as the build artifact: `index.html`, `demo.html`, and license files. Build artifacts are retained for 30 days. The repository root and private local output are not published as the site. -In repository **Settings → Pages**, select **GitHub Actions** as the source. Keep the `github-pages` environment restricted to `main`. The [publisher workflow](../.github/workflows/publish-pages.yml) runs after build completions, when a PR closes, or through **Run workflow**. It executes the publisher script from the default branch and uses successful build artifacts as static data. It never checks out or executes PR code with publishing permissions, including for fork PRs. The build workflow keeps read-only repository access. +In repository **Settings → Pages**, select **GitHub Actions** as the source. Keep the `github-pages` environment restricted to `main`. The [publisher workflow](../.github/workflows/publish-pages.yml) runs after build completions, when a PR closes, on `/deploy ` PR comments, or through **Run workflow**. It executes the publisher script from the default branch and uses successful build artifacts as static data. It never checks out or executes PR code with publishing permissions, including for fork PRs. The build workflow keeps read-only repository access. + +[OWNERS](../OWNERS) lists GitHub logins trusted to auto-deploy their PRs and approve other authors' previews. The initial owners are `jonabur` and `spyysalo`. The publisher reads OWNERS from its trusted default-branch checkout; changes to OWNERS within a PR cannot grant that PR access. Logins are case-insensitive, one per line, with optional `#` comments. An empty or malformed list fails publishing. + +For another author, the bot explains why it did not auto-deploy and includes a complete `/deploy <40-character-commit-sha>` command in a code block. An OWNER can copy it into a new comment; no manual hash entry is needed. Only a standalone command written by a current OWNER and matching the exact commit grants approval. Bare `/deploy`, partial hashes, quoted commands, and commands from non-owners do not grant access. Successful checks are still required. When that commit is deployed, the bot adds a rocket reaction to the owner's command. Reactions alone do not approve deployment: GitHub Actions has no reaction-created trigger. + +Approval is read from PR comments on every reconciliation, so coalesced workflow events cannot lose a command. A new push needs a new approval. A previously approved preview can stay available while the new head awaits approval; the comment labels the older build. Removing an owner or deleting their approval revokes access on the next reconciliation, including previously deployed content that no longer has valid authorization. Use **Run workflow** to apply a revocation immediately. Ordinary comments do not enter the deployment concurrency queue. The publisher deploys one combined site: @@ -119,9 +125,9 @@ The publisher deploys one combined site: - `/quickdash/pr-preview/pr-/` serves that PR’s latest published successful build. - `/quickdash/pr-preview/` lists open PRs, preview links, and the exact built commits. It labels a retained preview as **Previous successful build** when the current PR head has no successful build yet. -After deployment succeeds, the publisher creates or updates one bot comment in each PR conversation with a prominent **Open preview** link and the built commit. It reuses the comment without writing when the text is unchanged, and only edits comments owned by `github-actions[bot]` with the publisher’s hidden marker. A preview from an older successful build is labelled explicitly. Closing or merging a PR removes its directory and changes its existing comment to **Preview closed** after deployment succeeds; pending comment cleanup persists across failed deployments and retries. Reopening the PR lets the next successful deployment reuse the comment. +After deployment succeeds, the publisher creates or updates one bot comment in each PR conversation with a prominent **Open preview** link and the built commit, or the owner-approval instructions when deployment is blocked. It reuses the comment without writing when the text is unchanged, and only edits comments owned by `github-actions[bot]` with the publisher’s hidden marker. A preview from an older successful build is labelled explicitly. Closing or merging a PR removes its directory and changes its existing comment to **Preview closed** after deployment succeeds; pending comment cleanup persists across failed deployments and retries. Reopening the PR lets the next successful deployment reuse the comment. -A **Pages preview** check on each current built PR commit also includes an **Open preview** link in its summary. A failed main build preserves production; a failed PR build preserves its previous preview. Review both the ordinary `build` check and the built commit before assessing a preview. Previews are public and share the Pages origin with the main dashboard. The trusted publisher has `pull-requests: write` for comments; PR build jobs retain read-only permissions. +A **Pages preview** check on each current authorized, built PR commit also includes an **Open preview** link in its summary. A failed main build preserves production; a failed PR build preserves its previous authorized preview. Review both the ordinary `build` check and the built commit before assessing a preview. Previews are public and share the Pages origin with the main dashboard. Authorizing a preview means trusting its browser code: read-only data does not stop JavaScript from misleading users or reading same-origin browser storage and files a user imports into that page. The authorization gate is not a browser sandbox. The trusted publisher has `pull-requests: write` for comments and acknowledgment reactions; PR build jobs retain read-only permissions. Generated files and their source run/commit identifiers persist on the `pages-content` branch, created automatically by the first publisher run. This is storage for the Actions publisher; **do not change Pages to deploy from this branch**. The stored files keep published previews available after their source artifacts expire. Every publisher run reconciles all open PRs and successful main builds, and publishing is serialized so concurrent builds cannot overwrite each other’s previews. Run **Publish dashboard and PR previews** manually to retry a failed deployment or reconcile missed updates. The workflow must be on `main` before automatic previews can run; existing successful artifacts can be picked up during that first deployment. diff --git a/scripts/pages_preview.py b/scripts/pages_preview.py index 233032e..06607a2 100644 --- a/scripts/pages_preview.py +++ b/scripts/pages_preview.py @@ -6,6 +6,7 @@ import json import os from pathlib import Path +import re import shutil import subprocess import tarfile @@ -21,6 +22,32 @@ COMMENT_MARKER = "" +def read_owners(path=None): + path = path or Path(__file__).resolve().parents[1] / "OWNERS" + owners = set() + for line in Path(path).read_text().splitlines(): + login = line.split("#", 1)[0].strip().lower() + if login: + if not re.fullmatch(r"[a-z0-9](?:[a-z0-9-]{0,37}[a-z0-9])?", login): + raise ValueError("OWNERS must contain one GitHub login per line") + owners.add(login) + if not owners: + raise ValueError("OWNERS must not be empty") + return owners + + +def preview_authorization(pr, owners, comments, sha=None): + sha = sha or pr["head"]["sha"] + if pr["user"]["login"].lower() in owners: + return dict(allowed=True, sha=sha, source="owner") + for comment in comments: + author = comment.get("user", {}).get("login", "").lower() + command = re.fullmatch(r"/deploy ([0-9a-f]{40})", (comment.get("body") or "").strip()) + if author in owners and command and command[1] == sha: + return dict(allowed=True, sha=sha, source="approval", approver=author, comment_id=comment["id"]) + return dict(allowed=False, sha=sha, source="approval_required") + + def read_bundle(data): """Read the upload-pages-artifact ZIP/TAR without extracting archive paths.""" try: @@ -54,9 +81,10 @@ def read_bundle(data): class GitHub: - def __init__(self, repository, default_branch): + def __init__(self, repository, default_branch, owners=None): self.repository = repository self.default_branch = default_branch + self.owners = read_owners() if owners is None else owners def get(self, path): endpoint = f"repos/{self.repository}" + ("/" + path if path else "") @@ -86,6 +114,12 @@ def comments(self, number): return results page += 1 + def authorization(self, pr, sha=None): + # GitHub comments are the approval record. Reconciliation reads them all + # so coalesced Actions events cannot drop an owner's command. + comments = [] if pr["user"]["login"].lower() in self.owners else self.comments(pr["number"]) + return preview_authorization(pr, self.owners, comments, sha) + def latest_build(self, pr=None): filters = dict(status="success", per_page=100) if pr is None: @@ -169,9 +203,18 @@ def reconcile(site, source): # Persist cleanup until post-deployment reporting succeeds, including retries # and coalesced close events. A reopened PR no longer needs a closed notice. state["retired_previews"] = sorted(retired) + state["preview_access"] = {} warnings = [] for pr in pulls: number = str(pr["number"]) + access = source.authorization(pr) + state["preview_access"][number] = access + previous = state["previews"].get(number) + if previous and not source.authorization(pr, previous["sha"])["allowed"]: + shutil.rmtree(preview_dir / ("pr-" + number)) + del state["previews"][number] + if not access["allowed"]: + continue try: candidate = source.latest_build(pr) if candidate and not same_build(state["previews"].get(number), candidate): @@ -191,13 +234,15 @@ def reconcile(site, source): link = f'Open preview' provenance = f'{preview["sha"][:12]}' else: - status, link, provenance = "Awaiting successful build", "—", "—" + status = "Awaiting successful build" if state["preview_access"][number]["allowed"] else "Awaiting owner approval" + link, provenance = "—", "—" rows.append(f"{label}{link}{status}{provenance}") page = ''' Quickdash PR previews

Quickdash PR previews

Main dashboard

-

Previews update after successful builds. A previous successful build may be shown while a newer commit is pending or failing. +

Previews require an author listed in OWNERS or an owner's approval of the exact commit, and a successful build. +A previous approved build may be shown while a newer commit is awaiting approval, pending, or failing. The commit link identifies the build being served. Closed PRs are removed.

''' (preview_dir / "index.html").write_text(page + "".join(rows) + "
Pull requestPreviewStatusBuilt commit
\n") @@ -212,7 +257,7 @@ def report_checks(state, api, base_url): for pr in api.pulls(): number = str(pr["number"]) preview = state["previews"].get(number) - if not preview or preview["sha"] != pr["head"]["sha"]: + if not preview or preview["sha"] != pr["head"]["sha"] or not api.authorization(pr)["allowed"]: continue url = f"{base_url.rstrip('/')}/pr-preview/pr-{number}/" external_id = "quickdash-preview-" + number @@ -245,16 +290,31 @@ def report_comments(state, api, base_url): open_prs = {str(pr["number"]): pr for pr in api.pulls()} for number, pr in open_prs.items(): preview = state["previews"].get(number) - if not preview: - continue - url = f"{base_url.rstrip('/')}/pr-preview/pr-{number}/" - body = (f"{COMMENT_MARKER}\n" - f"**🚀 [Open preview]({url})**\n\n" - f"Built from `{preview['sha'][:12]}` · [Build details]({preview['run_url']})\n\n") - if preview["sha"] != pr["head"]["sha"]: - body += "This preview is from a previous successful build; it does not include the latest PR commit.\n\n" - body += "This link updates after successful builds. The preview is removed when this PR closes." + access = api.authorization(pr) + body = f"{COMMENT_MARKER}\n" + if preview: + url = f"{base_url.rstrip('/')}/pr-preview/pr-{number}/" + body += (f"**🚀 [Open preview]({url})**\n\n" + f"Built from `{preview['sha'][:12]}` · [Build details]({preview['run_url']})\n\n") + if preview["sha"] != pr["head"]["sha"]: + body += "This preview is from a previous successful build; it does not include the latest PR commit.\n\n" + if not access["allowed"]: + owners_url = f"https://github.com/{api.repository}/blob/{api.default_branch}/OWNERS" + body += (f"**Owner approval required for this commit.**\n\n" + f"Did not auto-deploy because the PR author is not listed in [OWNERS]({owners_url}). " + "An OWNER can approve this commit by copying this command into a new comment:\n\n" + f"```text\n/deploy {pr['head']['sha']}\n```\n\n" + "Checks must pass before deployment. Later commits need a new approval. " + "To enable automatic previews for this author, add them to OWNERS through a reviewed change on the default branch.") + elif not preview: + if access["source"] != "approval": + continue + body += "**Preview approved; waiting for a successful build.**" + else: + body += "This link updates after successful, authorized builds. The preview is removed when this PR closes." update_comment(api, number, body) + if preview and preview["sha"] == access["sha"] and access.get("source") == "approval": + api.write("POST", f"issues/comments/{access['comment_id']}/reactions", {"content": "rocket"}) for number in state.get("retired_previews", []): if number not in open_prs: update_comment(api, number, f"{COMMENT_MARKER}\n**Preview closed**\n\n" diff --git a/tests/test_pages_preview.py b/tests/test_pages_preview.py index 9928bd3..1bf10e1 100644 --- a/tests/test_pages_preview.py +++ b/tests/test_pages_preview.py @@ -39,9 +39,9 @@ def build(run_id, sha="abc", label="dashboard"): bundle=archive(label)) -def pull(number, sha="abc", title="Example", repo_id=1): +def pull(number, sha="abc", title="Example", repo_id=1, author="trusted"): return dict(number=number, title=title, html_url=f"https://github.com/org/repo/pull/{number}", - head=dict(sha=sha, ref="feature", repo=dict(id=repo_id))) + user=dict(login=author), head=dict(sha=sha, ref="feature", repo=dict(id=repo_id))) class Source: @@ -59,11 +59,76 @@ def latest_build(self, pr=None): def download(self, candidate): return candidate["bundle"] + def authorization(self, pr, sha=None): + return pages.preview_authorization(pr, {"trusted"}, [], sha) + class PagesPreview(unittest.TestCase): + def test_owners_are_explicit_nonempty_logins(self): + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "OWNERS" + path.write_text("# Preview owners\nJonabur\nspyysalo\n") + self.assertEqual(pages.read_owners(path), {"jonabur", "spyysalo"}) + for invalid in ("# empty\n", "@someone\n", "person with spaces\n"): + path.write_text(invalid) + with self.assertRaises(ValueError): + pages.read_owners(path) + + def test_only_owner_authors_or_explicit_owner_approval_for_exact_sha_are_allowed(self): + sha, later = "a" * 40, "b" * 40 + pr = pull(11, sha, author="outsider") + owners = {"trusted"} + self.assertFalse(pages.preview_authorization(pr, owners, [])["allowed"]) + self.assertTrue(pages.preview_authorization(pull(11, sha, author="TRUSTED"), owners, [])["allowed"]) + approval = {"id": 25, "body": "/deploy " + sha, "user": {"login": "trusted"}} + result = pages.preview_authorization(pr, owners, [approval]) + self.assertTrue(result["allowed"]) + self.assertEqual(result["comment_id"], 25) + self.assertFalse(pages.preview_authorization(pull(11, later, author="outsider"), owners, [approval])["allowed"]) + self.assertFalse(pages.preview_authorization(pr, {"another-owner"}, [approval])["allowed"]) + for body, author in [("/deploy", "trusted"), ("deploy it", "trusted"), + ("Please /deploy " + sha, "trusted"), ("/deploy " + sha, "outsider"), + ("/deploy " + sha[:8], "trusted"), ("/deploy " + later, "trusted")]: + with self.subTest(body=body, author=author): + forged = {**approval, "body": body, "user": {"login": author}} + self.assertFalse(pages.preview_authorization(pr, owners, [forged])["allowed"]) + + def test_unapproved_artifacts_are_never_downloaded_and_revoked_previews_are_removed(self): + source = Source() + with tempfile.TemporaryDirectory() as tmp: + site = Path(tmp) + pages.reconcile(site, source) + source.open[0]["user"]["login"] = "outsider" + with patch.object(source, "download", wraps=source.download) as download: + state = pages.reconcile(site, source) + download.assert_not_called() + self.assertNotIn("11", state["previews"]) + self.assertFalse((site / "pr-preview/pr-11").exists()) + self.assertFalse(state["preview_access"]["11"]["allowed"]) + self.assertIn("Awaiting owner approval", (site / "pr-preview/index.html").read_text()) + + def test_new_push_keeps_only_previously_approved_snapshot_until_new_approval(self): + sha, later = "a" * 40, "b" * 40 + source = Source() + source.open = [pull(11, sha, author="outsider")] + source.previews = {11: build(11, sha, "approved")} + approvals = [{"id": 20, "body": "/deploy " + sha, "user": {"login": "trusted"}}] + with patch.object(source, "authorization", side_effect=lambda pr, sha=None: pages.preview_authorization(pr, {"trusted"}, approvals, sha)): + with tempfile.TemporaryDirectory() as tmp: + site = Path(tmp) + pages.reconcile(site, source) + source.open[0]["head"]["sha"] = later + source.previews[11] = build(12, later, "new") + state = pages.reconcile(site, source) + self.assertEqual(state["previews"]["11"]["sha"], sha) + self.assertEqual((site / "pr-preview/pr-11/index.html").read_text(), "approved:index.html") + self.assertFalse(state["preview_access"]["11"]["allowed"]) + approvals.append({"id": 21, "body": "/deploy " + later, "user": {"login": "trusted"}}) + state = pages.reconcile(site, source) + self.assertEqual(state["previews"]["11"]["sha"], later) def test_repository_endpoint_has_no_trailing_slash(self): with patch.object(pages.subprocess, "check_output", return_value=b'{"id":1}') as command: - self.assertEqual(pages.GitHub("org/repo", "main").get("")["id"], 1) + self.assertEqual(pages.GitHub("org/repo", "main", owners={"trusted"}).get("")["id"], 1) self.assertEqual(command.call_args.args[0], ["gh", "api", "repos/org/repo"]) def test_only_expected_regular_files_can_be_published(self): @@ -143,7 +208,7 @@ def test_first_publish_requires_a_successful_main_artifact(self): pages.reconcile(Path(tmp), source) def test_fork_build_selection_uses_repository_and_sha_not_mutable_pr_metadata(self): - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) pr = pull(12, "expected", repo_id=2) runs = [dict(id=i, run_attempt=1, head_sha=sha, head_branch="feature", head_repository=dict(id=repo), event="pull_request", conclusion="success", @@ -161,7 +226,7 @@ def test_report_only_links_current_deployed_heads_and_updates_existing_check(sel source = Source() with tempfile.TemporaryDirectory() as tmp: state = pages.reconcile(Path(tmp), source) - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) source.open[1]["head"]["sha"] = "new head" with patch.object(api, "pulls", return_value=source.open), patch.object(api, "get", return_value={"check_runs": []}), patch.object(api, "write") as write: pages.report_checks(state, api, "https://org.github.io/repo") @@ -177,7 +242,7 @@ def test_report_only_links_current_deployed_heads_and_updates_existing_check(sel self.assertEqual(write.call_args.args[:2], ("PATCH", "check-runs/123")) def test_comment_is_created_once_then_updated_and_unchanged_reports_do_not_write(self): - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) state = {"previews": {"11": build(10)}, "retired_previews": []} comments = [{"id": 1, "user": {"login": "reviewer", "type": "User"}, "body": "\nHuman comment"}] @@ -208,7 +273,7 @@ def test_closed_comments_are_updated_after_removal_and_retried_until_report_succ self.assertFalse((site / "pr-preview/pr-11").exists()) # A deployment or reporting failure leaves cleanup pending in stored state. self.assertEqual(pages.reconcile(site, source)["retired_previews"], state["retired_previews"]) - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) comment = {"id": 5, "user": {"login": "github-actions[bot]", "type": "Bot"}, "body": "\nOld link"} with patch.object(api, "pulls", return_value=[]), patch.object(api, "comments", return_value=[comment]), patch.object(api, "write", side_effect=RuntimeError("API unavailable")): @@ -224,7 +289,7 @@ def test_closed_comments_are_updated_after_removal_and_retried_until_report_succ self.assertEqual(state["retired_previews"], []) def test_reopened_pr_is_not_marked_closed_and_unbuilt_pr_gets_no_comment(self): - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) state = {"previews": {}, "retired_previews": ["11"]} with patch.object(api, "pulls", return_value=[pull(11)]), patch.object(api, "comments") as comments, patch.object(api, "write") as write: pages.report_comments(state, api, "https://org.github.io/repo") @@ -233,12 +298,45 @@ def test_reopened_pr_is_not_marked_closed_and_unbuilt_pr_gets_no_comment(self): self.assertEqual(state["retired_previews"], []) def test_comment_lookup_paginates_long_pr_discussions(self): - api = pages.GitHub("org/repo", "main") + api = pages.GitHub("org/repo", "main", owners={"trusted"}) first = [{"id": n} for n in range(100)] with patch.object(api, "get", side_effect=[first, [{"id": 100}]]) as get: self.assertEqual(len(api.comments("11")), 101) self.assertEqual(get.call_args.args[0], "issues/11/comments?per_page=100&page=2") + def test_unrecognized_author_gets_copyable_command_then_approved_build_gets_reaction(self): + sha = "a" * 40 + api = pages.GitHub("org/repo", "main", owners={"trusted"}) + pr = pull(11, sha, author="outsider") + comments = [] + state = {"previews": {}, "retired_previews": []} + with patch.object(api, "pulls", return_value=[pr]), patch.object(api, "comments", return_value=comments), patch.object(api, "write") as write: + pages.report_comments(state, api, "https://org.github.io/repo") + body = write.call_args.args[2]["body"] + self.assertIn("not listed in [OWNERS]", body) + self.assertIn(f"```text\n/deploy {sha}\n```", body) + self.assertNotIn("[Open preview]", body) + comments.append({"id": 1, "body": body, "user": {"login": "github-actions[bot]", "type": "Bot"}}) + comments.append({"id": 2, "body": "/deploy " + sha, "user": {"login": "trusted", "type": "User"}}) + write.reset_mock() + pages.report_comments(state, api, "https://org.github.io/repo") + self.assertEqual(write.call_count, 1) + self.assertIn("waiting for a successful build", write.call_args.args[2]["body"]) + state["previews"]["11"] = build(10, sha) + write.reset_mock() + pages.report_comments(state, api, "https://org.github.io/repo") + self.assertEqual(write.call_count, 2) + self.assertIn("[Open preview]", write.call_args_list[0].args[2]["body"]) + self.assertEqual(write.call_args_list[1].args, ("POST", "issues/comments/2/reactions", {"content": "rocket"})) + + def test_check_cannot_advertise_a_revoked_preview(self): + sha = "a" * 40 + api = pages.GitHub("org/repo", "main", owners={"trusted"}) + state = {"previews": {"11": build(10, sha)}} + with patch.object(api, "pulls", return_value=[pull(11, sha, author="outsider")]), patch.object(api, "comments", return_value=[]), patch.object(api, "write") as write: + pages.report_checks(state, api, "https://org.github.io/repo") + write.assert_not_called() + def test_worktree_state_round_trip_keeps_source_branch_intact(self): with tempfile.TemporaryDirectory() as tmp: root = Path(tmp) @@ -275,10 +373,14 @@ def test_workflow_keeps_pr_builds_unprivileged_and_publisher_on_default_branch(s self.assertEqual(build["permissions"], {"contents": "read"}) self.assertNotIn("deploy", build["jobs"]) self.assertEqual(publish["on"]["pull_request_target"]["types"], ["closed"]) + self.assertEqual(publish["on"]["issue_comment"]["types"], ["created"]) + self.assertNotIn("concurrency", publish) + self.assertIn("github.event.issue.pull_request", publish["jobs"]["publish"]["if"]) + self.assertIn("startsWith(github.event.comment.body, '/deploy ')", publish["jobs"]["publish"]["if"]) self.assertEqual(publish["on"]["workflow_run"]["workflows"], [build["name"]]) steps = publish["jobs"]["publish"]["steps"] self.assertEqual(steps[0]["with"]["ref"], "${{ github.event.repository.default_branch }}") - self.assertEqual(publish["concurrency"]["group"], "quickdash-pages-publish") + self.assertEqual(publish["jobs"]["publish"]["concurrency"]["group"], "quickdash-pages-publish") self.assertLess(next(i for i,s in enumerate(steps) if s.get("id") == "deployment"), next(i for i,s in enumerate(steps) if s.get("name") == "Link deployed previews on PRs")) self.assertEqual(publish["jobs"]["publish"]["permissions"]["pull-requests"], "write")