From 4510bf7c9e2fcbeb043445850928b5dcb79908ca Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:43:24 -0500 Subject: [PATCH] ci: use code-owner-reviewed master for trusted PR workflow (#13470) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Pull request CI uses a trusted workflow on the AWS runner fleet. > - The caller used a fixed SHA that also needed runner-group admission. > - A mainline pin update left CI queued because the group still allowed older SHAs. > - This pull request calls the trusted workflow on master, which requires code-owner review. > - New merged workflow versions can use the existing master runner-group entry. ## Linked Issues or Issue Description Related: #12968. That Dependabot PR proposes another SHA rotation. This change keeps this first-party workflow on master instead. **What happened?** CI run 34975562974 stayed queued because its trusted workflow SHA was absent from the runner-group allowlist. The fleet itself was healthy. **Expected behavior** New reviewed versions of the trusted workflow on master should receive runner access without a separate SHA allowlist update. **Steps to reproduce** Change the caller to a new trusted workflow SHA without adding that SHA to the restricted runner group. Its jobs remain queued. The master reference removes that recurring synchronization step. ## What Changed - Call `paperclipai/paperclip/.github/workflows/pr-trusted.yml@master`. - Exclude this exact first-party workflow from Dependabot updates. - Update the existing E2E shard workflow tests for the master caller contract. - Document the runner-group entry, required code-owner review, and old-reference retention. ## Verification - `actionlint .github/workflows/pr.yml` passed. - `node --test scripts/__tests__/e2e-shard.test.mjs .github/scripts/tests/cloud-runner-routing.test.mjs .github/scripts/tests/pr-runner-rust-cache.test.mjs .github/scripts/tests/pr-dependency-cache.test.mjs` passed: 35 tests. - Parsed Dependabot YAML and checked the exact workflow exclusion. - `git diff --check` passed. - Live GitHub checks confirmed `.github/**` has code owners, CODEOWNERS has no errors, and the active master ruleset requires code-owner review. This covers the trusted workflow, caller, and CODEOWNERS itself. - The approved organization setting now allows `paperclipai/paperclip/.github/workflows/pr-trusted.yml@refs/heads/master`. All previous references and other runner-group settings remain intact. - Full application typecheck, tests, and build were not repeated locally for this workflow-only change. This PR's CI and review are pending. ## Risks New versions of the trusted workflow take effect for new callers after merge to master. Keep code-owner review and master protection enabled. Existing administrator pull-request bypasses remain unchanged. Third-party action pins, runner routing, and infrastructure are unchanged. Older callers still use their SHA pins; their allowed refs remain in place. ## Model Used OpenAI GPT-6 through Codex performed implementation and orchestration; its exact runtime variant and context-window size were not exposed. OpenAI `gpt-5.6-luna` with high reasoning inspected workflow assumptions and applied the approved runner-group setting. Both used code and tool access. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .github/dependabot.yml | 3 +++ .github/workflows/pr.yml | 5 +++-- doc/DEVELOPING.md | 18 ++++++++++++++++ scripts/__tests__/e2e-shard.test.mjs | 32 +++++++++++----------------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 9aae3c6d06..0f234d2553 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -37,3 +37,6 @@ updates: day: monday time: "06:00" open-pull-requests-limit: 5 + ignore: + # This first-party workflow follows CODEOWNERS-protected master. + - dependency-name: "paperclipai/paperclip/.github/workflows/pr-trusted.yml" diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 41125c3331..388c13838c 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -10,5 +10,6 @@ permissions: jobs: ci: - # Pin: #13457 merge — restore-only Rust dependency cache on the PR runner lane. - uses: paperclipai/paperclip/.github/workflows/pr-trusted.yml@f97a3f886eb4baf19ea8eefaa53a44cb1c2d4636 + # Master requires CODEOWNERS review for .github/**, including this workflow. + # The AWS runner group permits pr-trusted.yml@refs/heads/master. + uses: paperclipai/paperclip/.github/workflows/pr-trusted.yml@master diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index dc1eaa5fbd..0e4de36e84 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -23,6 +23,24 @@ GitHub Actions owns `pnpm-lock.yaml`. - Pull request CI validates dependency resolution when manifests change. - Pushes to `master` regenerate `pnpm-lock.yaml` with `pnpm install --lockfile-only --no-frozen-lockfile`, commit it back if needed, and then run verification with `--frozen-lockfile`. +## Trusted PR Workflow + +The PR caller uses `paperclipai/paperclip/.github/workflows/pr-trusted.yml@master`. +The AWS runner group `paperclip-public-pr` must allow +`paperclipai/paperclip/.github/workflows/pr-trusted.yml@refs/heads/master`. +New workflow versions merged into master then receive runner access without a +separate SHA allowlist update. Dependabot leaves this first-party reference on +master. + +Keep the `.github/**` rule in `.github/CODEOWNERS` and the active master ruleset's +code-owner review requirement enabled. This covers the caller, the trusted +workflow, and CODEOWNERS itself. Existing administrator pull-request bypasses +remain governed by the repository ruleset. + +When changing the workflow path or branch, authorize the new reference before +updating the caller. Retain older authorized SHA references while queued runs or +supported reruns still use them. + ## Start Dev From repo root: diff --git a/scripts/__tests__/e2e-shard.test.mjs b/scripts/__tests__/e2e-shard.test.mjs index 2fa156fc2f..d90021e114 100644 --- a/scripts/__tests__/e2e-shard.test.mjs +++ b/scripts/__tests__/e2e-shard.test.mjs @@ -25,19 +25,15 @@ function runShard(args) { return result.stdout.trim().split(/\s+/).filter(Boolean); } -function readPinnedTrustedPrWorkflow() { +function readTrustedPrWorkflow() { const caller = readFileSync(prCallerWorkflow, "utf8"); - const pin = caller.match( - /uses: paperclipai\/paperclip\/\.github\/workflows\/pr-trusted\.yml@([0-9a-f]{40})/, + assert.match( + caller, + /^\s+uses: paperclipai\/paperclip\/\.github\/workflows\/pr-trusted\.yml@master\s*$/m, + "pr.yml must call the trusted workflow from CODEOWNERS-protected master", ); - assert.ok(pin, "pr.yml must call the trusted workflow at a full commit SHA"); - - const result = spawnSync("git", ["show", `${pin[1]}:${trustedPrWorkflowPath}`], { - cwd: repoRoot, - encoding: "utf8", - }); - assert.equal(result.status, 0, `cannot read the pinned trusted workflow: ${result.stderr}`); - return result.stdout; + // Validate proposed workflow changes locally; CI executes the merged master version. + return readFileSync(trustedPrWorkflow, "utf8"); } function readWorkflowJobs(workflow) { @@ -161,15 +157,15 @@ test("shard arguments are validated", () => { } }); -test("pr.yml calls the trusted PR workflow at an immutable SHA", () => { - assert.ok(readPinnedTrustedPrWorkflow().length > 0); +test("pr.yml calls the trusted PR workflow from master", () => { + assert.ok(readTrustedPrWorkflow().length > 0); }); test("the trusted PR workflow keeps a stable aggregate check named e2e over the shard matrix", () => { // Branch protection requires a check literally named `e2e`. The shards run // as `e2e shard (n/3)`, so the aggregate job below is what keeps the // required-check contract intact — same pattern as the `verify` aggregate. - const workflow = readPinnedTrustedPrWorkflow(); + const workflow = readTrustedPrWorkflow(); const jobs = readWorkflowJobs(workflow); const aggregate = jobs.get("e2e"); @@ -293,7 +289,7 @@ test("the stacked PR scope selector runs full CI only where intended", () => { test("the trusted PR workflow passes the shard's spec filter to Playwright without a literal --", () => { // `pnpm run test:e2e -- $specs` forwards the literal separator to Playwright, // so the specs after it are not applied as file filters. - const workflow = readPinnedTrustedPrWorkflow(); + const workflow = readTrustedPrWorkflow(); assert.ok( !/pnpm run test:e2e --\s/.test(workflow), "pr-trusted.yml must not insert a literal `--` between `pnpm run test:e2e` and the spec filter", @@ -306,10 +302,8 @@ test("the trusted PR workflow passes the shard's spec filter to Playwright witho }); test("the trusted PR workflow regenerates stale stacked lockfiles", () => { - // Implementation PRs validate the workflow under development here. The - // caller remains pinned to the last merged trusted SHA until a separate - // activation PR advances it, so unmerged PR code never runs on trusted - // infrastructure. + // Validate the proposed workflow here. The caller executes the merged master + // workflow; edits to this workflow take effect after code-owner review and merge. const workflow = readFileSync(trustedPrWorkflow, "utf8"); assert.match( workflow,