[brandon-pereira]

Summary

When a maintainer adds automerge to a first-time contributor's fork PR, Kodiak merges main into the branch. GitHub then holds the new pull_request runs for approval again, even though the contributor's code was already reviewed and its runs approved. The PR sits until someone clicks "Approve workflows to run" a second time; on #3532 that took about 40 minutes.

This adds a pull_request_target workflow that approves those held runs when the push is only Kodiak's merge of main. It approves only when all of these hold:

• the push came from kodiakhq[bot], the PR is from a fork, and it has the automerge label
• the new head is a merge whose first parent is the previous head and whose other parent is on the base branch
• someone with write access approved the PR and no writer is requesting changes
• every pull_request run on the previous head, on this PR's fork branch, was started by someone with write access or by this workflow on an earlier Kodiak update

The last check is the security boundary. Reviews aren't tied to a commit, and held runs can leave action_required without approval (expired after 30 days, cancelled, startup failure). Those runs keep the contributor as triggering_actor, so they fail this check. Any commit the contributor pushes still waits for a maintainer.

The job checks out only .github/scripts from the base branch and never runs PR code. The decision logic is in .github/scripts/kodiak-run-approval.js, with unit tests run from the Main lint job. No changeset: CI only.

Not verified yet

• That GITHUB_TOKEN with actions: write can call POST /actions/runs/{id}/approve. Other repos report it works; the first Kodiak update on a first-time contributor's PR will confirm.
• What triggering_actor is recorded after this workflow approves a run. If it isn't github-actions[bot], a second Kodiak update on the same PR is refused and needs a manual click. That fails closed.
• GitHub starts blocking pull_request_target by default on December 2, 2026. The event policy that keeps the Claude and Deep review workflows running needs to cover this workflow too.

How to reproduce

On main:

1. Approve a first-time contributor's fork PR and approve its workflow runs.
2. Add automerge once main has moved ahead.
3. Kodiak merges main into the branch, and the new runs show "Approve workflows to run" again.

On this branch:

1. Same steps. The "Approve Kodiak update runs" workflow approves the held runs, and its log and step summary say why.
2. Push a commit from the fork after approval, then let Kodiak update. The workflow refuses and logs that the previous head's runs were not approved by a maintainer.

References

• Related PRs: fix(hdx-eval): reject invalid MCP evidence content #3532, docs(mcp): recommend quoted Lucene values for exact matches and bare terms for body text #3548 (both needed manual approval after a Kodiak update)

[github-actions]

Duplicate code analysis

No new duplicate code introduced by this PR.

---

jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.22%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

[github-actions]

E2E Test Results

✅ All tests passed • 462 passed • 1 skipped • 1415s

Status	Count
✅ Passed	462
❌ Failed	0
⚠️ Flaky	2
⏭️ Skipped	1

Tests ran across 4 shards in parallel.

View full report →

[github-actions]

🔵 minor — No test checks the action_required rule on its own: deleting it from decideApproval still passes every test

The 'previous head runs still held' case pairs action_required with contributor, who has no write permission, so the actor check rejects it anyway. Change it to run('action_required', 'maintainer') so the r.conclusion === 'action_required' condition in kodiak-run-approval.js:92 is actually tested, and assert on decision.reason in the rejected cases so each one proves which check fired.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

🔵 minor — A null pr.head.repo (deleted fork) crashes listRuns before decideApproval can handle it

decideApproval reads pr.head.repo?.full_name, but listRuns (called first, at line 69) reads pr.head.repo.full_name and throws a TypeError. Either check pr.head.repo before any API calls and return early with a reason, or use ?. here as well so both places handle it the same way.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

🔵 minor — The headCommit.sha !== after guard can never trigger

headCommit comes from repos.getCommit({ ref: after }) in approve-kodiak-update-runs.js:45, and after is a full SHA, so the returned sha always equals after. Remove the guard, or pass headCommit in a form where a mismatch is actually possible.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

3 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 3 minor

3 posted as inline comment(s) on the changed lines.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[github-actions]

Deep Review

✅ No critical issues found. The trust boundary is well-constructed: sender === kodiakhq[bot] is enforced in both the workflow if: and decideApproval, the merge shape is verified (two parents, first parent === before, second parent proven on the base branch via compareCommitsWithBasehead identical/ahead), and the beforeRuns vouched-check fails closed so any contributor-pushed commit still waits for a maintainer. No P0/P1 production-breakage paths were found in the diff. The findings below are reliability and test-coverage recommendations.

🟡 P2 -- recommended

• .github/scripts/approve-kodiak-update-runs.js:111 -- approveWorkflowRun is called with no per-run error handling, so one failure (e.g. a 403 when a run already left action_required, or a transient 5xx) throws out of the loop, leaving some runs approved, others held, and no retry since no new synchronize fires.
  • Fix: Wrap the per-run approveWorkflowRun call in try/catch, tolerate benign already-approved/not-approvable statuses by logging and continuing, and re-throw only genuinely unexpected errors.
  • _{reliability, kieran-typescript}
• .github/scripts/approve-kodiak-update-runs.js:41 -- The security-relevant listRuns fork-scoping filter (head_repository.full_name and head_branch) that prevents approving a different fork PR's runs sharing the same head_sha, the poll/quiet-stop loop, and the 404 → 'none' permission fallback have no test coverage.
  • Fix: Add orchestration tests with a mocked github client asserting the fork-scoping filter rejects a same-head_sha run from a different repo/branch, the approved-set dedup approves each run once, and the 404 vs non-404 permission branch behaves correctly.
  • _{testing, kieran-typescript}

🔵 P3 nitpicks (5)

• .github/scripts/__tests__/kodiak-run-approval.test.js:123 -- Every rejected case asserts only approve === false and a non-empty reason, so a refactor that reorders or collapses gates could reject for the wrong reason and still pass.
  • Fix: Store an expected reason substring per override and assert decision.reason matches it.
• .github/scripts/approve-kodiak-update-runs.js:42 -- pr.head.repo.full_name is dereferenced without optional chaining and runs before decideApproval, so a deleted/null fork head throws a raw TypeError instead of the guarded not-from-a-fork no-op.
  • Fix: Use pr.head.repo?.full_name for consistency with decideApproval, or guard the null case early.
• .github/scripts/kodiak-run-approval.js:10 -- Treating any previous-head run triggered by github-actions[bot] as maintainer-vouched relies on the unenforced assumption that no other workflow approves or re-runs fork runs; a future workflow would silently widen who is trusted.
  • Fix: Narrow the vouching check to this workflow's own run identity, or guard the invariant with a test or CODEOWNERS rule on new fork-run-approving workflows.
• .github/scripts/kodiak-run-approval.js:19 -- The PENDING skip in latestReviewStates is covered only implicitly, unlike its sibling COMMENTED branch.
  • Fix: Add a direct test asserting a PENDING review is ignored and does not overwrite a prior APPROVED.
• .github/scripts/approve-kodiak-update-runs.js:120 -- Once any run is approved, the loop stops after two empty polls (~20s), so a slow-to-schedule held run can appear after the quiet window closes and never be approved, stalling the PR.
  • Fix: Key the stop condition on an expected run count or widen the quiet window relative to typical scheduling latency.

---

Reviewers (7): testing, maintainability, project-standards, reliability, kieran-typescript, agent-native, learnings-researcher. The correctness, security, and adversarial reviewers were dispatched but did not return before finalization; the orchestrator analyzed that surface directly (sender/merge-shape/vouched-runs trust boundary, compareCommitsWithBasehead semantics, second-PR-same-head_sha leak, fork-code execution via sparse checkout) and found no additional P0/P1 issues. Project-standards found the files fully compliant (test placement, action pins, no changeset needed). No relevant docs/solutions/ learnings exist; this is a good candidate to capture post-merge.

Testing gaps: The orchestration layer in approve-kodiak-update-runs.js (poll loop, fork-scoping filter, permission fallback) is entirely untested. The decision logic's acknowledged-unverified assumption — that triggering_actor becomes github-actions[bot] after this workflow approves a run, and that manual maintainer approval records a writer as triggering_actor — cannot be verified from code; if false the workflow fails closed (PR waits) rather than approving unsafely.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}