GITGUD

neiam neiam /gitgud

Git Gud

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

⭐ 1

Log in to mark this repository.

9.1 KiB · text History 95a46eb

Plan: one commit status per job, and branch protection that can require them

Status: ready to execute. Written to be carried out start to finish by one agent without further questions. Independent of the other CI plans; if workflow-job-graph.md has been executed, jobs can also be skipped — map that as step 1 says.

Why

  1. Two runs on one commit overwrite each other’s status. Workflows.update_run_commit_status/1 writes one row per run with context "gitgud-actions/" <> (run.workflow_name || run.workflow_path), upserting on (repository_id, sha, context). A same-repository pull request’s head commit gets a push run (the push to the branch) and a pull_request run (synchronize) of the same workflow — one context, so whichever finished last is what the PR page shows: a failing PR run can be hidden by a passing push run. Two workflow files that share a name: collide the same way.
  2. Statuses are per workflow, not per job. GitHub and Forgejo report each job: <workflow> / <job> (<event>) (Forgejo: services/actions/commit_status.go). A workflow with test and lint shows one pill here, so you can’t see which job failed, and branch protection (below) couldn’t require lint alone.
  3. Branch protection can’t require checks. BranchProtection has require_status_checks and required_checks columns (lib/git_gud/branch_protections/branch_protection.ex), cast by its changeset, but nothing reads them: BranchProtections.evaluate/5 checks force-push and approvals only, and the settings UI (lib/git_gud_web/live/branch_protection_live/index.ex) doesn’t offer them. Anything that sets them through the changeset gets a rule that silently enforces nothing.
  4. The PR page doesn’t update when CI finishes. lib/git_gud_web/live/pr_live/show.ex renders <.commit_status_badges repo={@repo} sha={@pr.head_sha} /> (line ~533) but doesn’t subscribe to Workflows.ci_topic/0, so pills stay “pending” until a reload.

Semantics to match

  • Context per job: "#{workflow_name} / #{job_name} (#{event})", where workflow_name = run.workflow_name || Path.basename(run.workflow_path), job_name = the job row’s name (already build (arm64, 28) for a matrix leg), event = run.trigger. That is Forgejo’s format, so required-check names people copy from Forgejo match.
  • State: queued/blocked/waiting/running → pending; success → success; failure and cancelled → failure (today’s mapping, and Forgejo’s in services/actions/commit_status.go, toCommitStatus); skipped → success with description “Skipped” (Forgejo maps skipped to success, and GitHub counts a skipped required check as passing).
  • Required checks (GitHub/Forgejo): a PR may be merged into a branch whose rule has require_status_checks only when, for the PR head commit, every pattern in required_checks matches at least one status and every matching status is success. Patterns are globs (* matches any run of characters, including / and spaces — Forgejo uses glob patterns for this field). An empty required_checks with require_status_checks on means “every status on the head commit must be success, and there must be at least one”.
  • Direct pushes to a protected branch are not blocked by status checks (no PR, no head to have checks on — Forgejo’s behaviour); say so in the UI hint.
  • Admin override: merge!/3’s override: true already bypasses review rules; let it bypass status checks too (same audit trail).

Where things are
Status writer lib/git_gud/workflows.ex: update_run_commit_status/1 (called from create_run/2, maybe_complete_run/1, cancel/1), update_job_status/3, create_job_leg/3
Status schema lib/git_gud/workflows/commit_status.ex; reader Workflows.list_commit_statuses/2
Pills lib/git_gud_web/components/core_components.ex commit_status_badges/1, display_context/1 (strips gitgud-actions/)
PR page lib/git_gud_web/live/pr_live/show.ex (mount, handle_info/2, merge box), PullRequests.merge_blockers/2 (review rules, shown on the page)
Protection lib/git_gud/branch_protections.ex check_ref_update/5, evaluate/5; schema above; callers PullRequests.merge!/3, GitGud.Merges (check_protection/5), GitGudWeb.PreReceiveController
Settings UI lib/git_gud_web/live/branch_protection_live/index.ex
Tests test/git_gud/branch_protections_test.exs, test/git_gud/workflows/rerun_cancel_test.exs, test/git_gud_web/live/pr_live/

Steps

1. Per-job statuses

Add Workflows.update_job_commit_status(job, run) writing the context and state above, target_url = the run page anchored to the job if the run page has job anchors (check workflow_live/show.ex; else the run page), description = e.g. “Successful in 2m”, “Failing after 40s”, “Waiting to run”, “Skipped”. Call it:

  • for every job row created in create_job_leg/3 (initial pending),
  • in update_job_status/3 after the update,
  • in cancel/1 for each job it cancels (it uses update_all; select the affected rows first, or re-read them after).

Stop writing the per-run gitgud-actions/<workflow> status (delete the writer, keep broadcast_ci_change/1 — move that call into the new function). Old rows stay on old commits; display_context/1 keeps stripping their prefix.

2. PR page refreshes

In pr_live/show.ex mount/3 (when connected?), subscribe to Workflows.ci_topic/0; on {:ci_run_changed, run_id} re-render the status pills if the run is in this repository (load the run’s repository_id — cheap) — e.g. bump an assign the badges component reads so it reloads.

3. Required checks: evaluation

In BranchProtections, add check_statuses(rule, repo, head_sha) returning :ok or {:error, {:status_checks, missing :: [pattern], failing :: [context]}}, using Workflows.list_commit_statuses/2 and a glob matcher where * matches anything (a private regex helper; don’t use GitGud.Workflows.Glob, whose * stops at /).

Restructure evaluate/5 from a cond into a with chain so all rules apply in turn (today the approvals clause returns early and nothing after it could run): force-push → approvals → status checks. Status checks apply only when opts[:pull_request_id] is given (a merge); pass the PR head sha in opts (head_sha:) from merge!/3 and Merges (check what Merges has in hand: analysis.theirs_sha is the head being merged).

merge!/3: override: true skips the status-check error as it skips review blocks. Find how override is threaded into check_ref_update today (it isn’t — it’s only used by check_review_block/3) and pass it.

4. Required checks: on the PR page

Extend what the merge box shows (next to PullRequests.merge_blockers/2’s review blockers): “Required checks: missing”, “ failing”, and disable the merge button the same way review blocks do. Put the lookup in PullRequests (e.g. status_check_blockers(repo, pr) calling BranchProtections.rule_for_branch/2 + check_statuses/3) so the LiveView and merge!/3 agree.

5. Required checks: settings UI

In branch_protection_live/index.ex: a checkbox “Require status checks to pass before merging” (require_status_checks) and a text input “Required checks” (comma- or newline-separated → required_checks list; parse in the LiveView before casting), with a hint listing the contexts seen on the default branch’s latest commit (Workflows.list_commit_statuses/2) so people can copy exact names. Show them in the rule list.

6. Docs

README CI list: per-job statuses and their names; required checks and their glob rule; that direct pushes aren’t checked.

Tests

  • A run with jobs test and lint creates two pending statuses with the Forgejo-format contexts; failing lint → its status failure, test unaffected.
  • A push run and a pull_request run on the same sha → four distinct statuses (… (push) and … (pull_request)), neither overwrites the other.
  • Cancel → each cancelled job’s status becomes failure with description “Cancelled”.
  • BranchProtections with require_status_checks: true, required_checks: ["ci / test *"]:
    • head has ci / test (push) success → merge allowed;
    • head has it failure → {:status_checks, [], ["ci / test (push)"]};
    • no matching status → {:status_checks, ["ci / test *"], []};
    • required_approvals: 1 plus checks: both enforced (the old cond would have stopped at approvals);
    • a direct push (no pull_request_id) is not blocked by checks;
    • merge!(…, override: true) merges despite a failing check.
  • LiveView: the PR page shows the missing/failing checks and the merge button is disabled; a broadcast on ci_topic re-renders the pills.
  • Branch protection settings: saving the checkbox and a two-line required_checks field persists a two-element list.

Done when

  • mix test passes; no new compile warnings (compare mix compile --force warning lists before/after); mix format only touched files.
  • One commit in the repository’s message style.
  • Not pushed.