GITGUD

neiam neiam /gitgud

Git Gud

public · Issues · Pulls · Labels · Forks · Compare · Actions success · Badges · Packages

⭐ 1

Log in to mark this repository.

9.5 KiB · text History b868907

Plan: run workflows for pull requests (on: pull_request)

Status: executed (2026-10-17), on top of workflow-path-filters.md (this reuses its changed-file listing and paths matching). Written to be carried out by one agent without further questions.

Why

on: pull_request is accepted by the parser (Yaml.matches?/3 would match it) but nothing ever dispatches that trigger: the only caller of Workflows.dispatch/2 is the push worker (Repositories.Workers.CommitBackfill.dispatch_workflows/4), which always sends "push". Workflows written for GitHub/Forgejo that test pull requests never run here, and the PR page’s checks (it already reads commit statuses, lib/git_gud_web/live/pr_live/show.ex) only show whatever a push to the same SHA happened to produce.

There is also a context bug to fix first: the runner context carries the trigger as event but not event_name, so github.event_name is empty in every job. A bigtrippr job with if: github.event_name == 'push' && … was skipped on every push because of it.

Scope

In:

  • github.event_name in the runner context.
  • Same-repository pull requests (source and target branches in one repo): runs on opened, synchronize (head moved), reopened.
  • types: filter, branches: / branches-ignore: (against the target branch, as on GitHub), paths: / paths-ignore: (against the PR’s diff from its merge-base).
  • The runner context a PR job expects: ref, ref_name, head_ref, base_ref, sha, event_name.

Out (state it in the docs, don’t build it):

  • Cross-repository (fork) and federated pull requests. Their head is code the target’s maintainers haven’t reviewed, and a job is handed the target repository’s secrets (runner_controller.ex: GitGud.Secrets.resolve_for(repo)) and a token that can publish its packages. GitHub runs fork PRs without secrets and with a read-only token; doing that safely here is its own piece of work. Until then, do not dispatch for them at all, and log at debug level why.
  • pull_request_target, the closed / edited / labeled / assigned activity types, merge refs (refs/pull/N/merge; GitHub tests the merge result, this tests the head — say so in the docs).

Where things are
Runner context lib/git_gud_web/controllers/runner_controller.ex, the map with "event" => run.trigger (~line 288)
Ref → filter ctx lib/git_gud/workflows/ref.ex (Ref.context/1, Ref.name/1, Ref.type/1)
Dispatch lib/git_gud/workflows.ex — dispatch/2, create_run/2; runs store trigger, head_sha, head_ref
Trigger matching lib/git_gud/workflows/yaml.ex
PRs lib/git_gud/pull_requests.ex: do_create_pull_request/3 (opened), set_state/2 (reopen: "open"), refresh_cross_repo_heads/3 (called on every push via CommitBackfill.refresh_cross_repo_prs/3; despite the name it also covers same-repo PRs, whose source_repository_id is the repo itself — that is the synchronize moment); PullRequest.cross_repo?/1, PullRequest.federated?/1
Commit statuses lib/git_gud/workflows/commit_status.ex, Workflows.update_run_commit_status/1 (keyed on the run’s head SHA, which the PR page reads)
Tests to copy from test/git_gud/workflows/tag_dispatch_test.exs (on-disk repo + dispatch); find the existing PR tests under test/git_gud/pull_requests* for creating PRs in tests

Steps

1. github.event_name

In the runner context map add "event_name" => run.trigger || "push" next to "event". Test: the existing runner controller test that asserts on the context (find it: grep ref_name under test/) gets an assertion for event_name. This step is independent: commit it on its own first.

2. Trigger matching for pull requests

In Yaml:

  • pull_request config’s types: — default ["opened", "synchronize", "reopened"] when absent; the event’s action must be in the list.
  • branches: / branches-ignore: match the target branch (ctx :branch is the target’s short name). branches-ignore may not be supported for push either — add it for both if missing, with GitHub’s rule that branches and branches-ignore can’t both be set (if both: use branches, log a warning; same choice as paths).
  • paths / paths-ignore: unchanged code from the path-filters plan; ctx :changed_paths is the PR’s diff (step 3).
  • tags: never applies to pull requests.

matches?/3 gains the action through ctx (:action), not a new arity. Pure tests in trigger_filters_test.exs: types default and explicit; target-branch filter; branches-ignore; paths on a PR.

3. Workflows.dispatch_pull_request(pr, action, opts)

New public function in GitGud.Workflows:

  • Return {:ok, []} without doing anything when pr.state != "open", PullRequest.cross_repo?(pr) or PullRequest.federated?(pr).
  • Workflows are read from the PR head commit (pr.head_sha) — that is GitHub’s behaviour for pull_request and what lets a PR change its own CI. (Fork PRs are excluded above, so this is code from the repo’s own writers.)
  • Pin refs/pull/<N>/head to pr.head_sha in the repo (Git.update_ref/4, as pin_pull_head/refresh_* already do for cross-repo PRs), so the ref the job is told about exists and actions/checkout can fetch it.
  • Changed paths: Git.changed_paths(repo_path, merge_base(target head, pr.head_sha), pr.head_sha) — only if some workflow is path-filtered; fail open as in the path plan.
  • ctx: %{branch: target short name, action: action, changed_paths: …}.
  • Create runs with trigger: "pull_request", head_sha: pr.head_sha, head_ref: "refs/pull/<N>/head", and store the PR’s number, source and target refs on the run. Add a migration (mix ecto.gen.migration add_pull_request_to_workflow_runs) with nullable pull_request_number :integer, pr_head_ref :string, pr_base_ref :string on workflow_runs, and cast them in WorkflowRun’s changeset. Refactor dispatch/2’s inner loop into a shared private function rather than duplicating it.
  • Return {:ok, runs}.

4. The runner context for a PR job

In runner_controller.ex, when run.trigger == "pull_request":

  • "ref" => "refs/pull/<N>/head" (already the run’s head_ref), "ref_name" => "<N>/head", "ref_type" => "branch"? — GitHub sets ref_type empty for PRs; use "".
  • "head_ref" => run.pr_head_ref, "base_ref" => run.pr_base_ref (short branch names, as GitHub gives them).
  • "event_name" => "pull_request" (from step 1).
  • "event" stays the trigger string — check what forgejo-runner/act does with event (it may expect the event payload object there; if the existing push jobs work with a string, keep it, and do not change push behaviour in this plan).

Make sure Ref.name/1 / Ref.type/1 are not fed refs/pull/... paths that make them crash or misreport; add clauses or bypass them for PR runs.

5. Call sites

  • opened: at the end of do_create_pull_request/3, after the transaction succeeds, call Workflows.dispatch_pull_request(pr, "opened", triggered_by_id: author.id). Do it outside the transaction; a dispatch failure must never fail PR creation (rescue + log).
  • synchronize: in refresh_cross_repo_heads/3, for each PR whose head_sha actually changed (it has old_sha), call it with "synchronize" and the pusher if available (thread pusher_id from CommitBackfill.refresh_cross_repo_prs/3 if it’s cheap; else nil). This function already handles same-repo PRs; cross-repo ones are dropped inside dispatch_pull_request/3.
  • reopened: in set_state/2 when moving "closed" → "open" successfully. Check whether the head moved while closed (re-resolve the source ref) — if refresh_* already keeps head_sha current for closed PRs, no extra work; otherwise refresh head_sha before dispatching.
  • Dispatch is DB + git work; if PR creation runs inside a LiveView process, enqueue it (Oban, like CommitBackfill) instead of running it inline. Look at how CommitBackfill is enqueued and copy that shape (Workers.PullRequestDispatch, args: pr id, action, actor id).

6. Tests

On disk, modelled on tag_dispatch_test.exs:

  • same-repo PR opened → a pull_request run on the PR head SHA, with head_ref refs/pull/N/head, and that ref exists in the repo.
  • push to the source branch → a synchronize run (and the usual push run).
  • types: [opened] → no run on synchronize.
  • branches: [master] → runs for a PR targeting master, not one targeting release.
  • paths: ["lib/**"] → PR that only changes docs/ → no run.
  • workflow present only on the PR branch (added by the PR) → it runs (workflows come from the head).
  • cross-repo PR (fork fixture, if the test helpers have one; else build the struct with a different source_repository_id) → no run, and the target’s secrets are never resolved for it.
  • closed then reopened → a reopened run.
  • runner context for a PR job (controller test): event_name, head_ref, base_ref, ref, ref_name.

7. Docs

Where workflow support is documented (see the path plan’s step 7): on: pull_request with the supported types, that it tests the PR head (not a merge result), that branches means the target branch, and that fork and federated PRs don’t run yet and why (secrets).

Done when

  • mix precommit passes.
  • Commits, in the repository’s message style: (1) event_name, (2) the pull request trigger (migration included). Separate from the path-filter commit.
  • Not pushed. Pushing gitgud deploys the forge.