neiam /gitgud

Git Gud

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

Plans: where CI differs from GitHub and Forgejo Actions

95a46eb · Gabriel Morell · 2026-10-17 05:06

13 files +1967 -0

Message

{commit_body(@commit)}

Files changed

added docs/plans/ci-gaps.md

+94 −0

@@ -0,0 +1,94 @@

	1	+# CI gaps against GitHub / Forgejo Actions
	2	+
	3	+Status: audit of 2026-10-17, on master at bfce1e7 (after the path-filter,
	4	+`event_name` and `pull_request` work). Each gap was checked in the code;
	5	+the evidence column names the file and function. What forgejo-runner
	6	+expects was checked against its source
	7	+(code.forgejo.org/forgejo/runner: `internal/app/run/runner.go`,
	8	+`workflow.go`, `internal/pkg/report/reporter.go`) and Forgejo's server
	9	+(`services/actions/context.go`).
	10	+
	11	+Severity, highest first:
	12	+
	13	+- security — another repository's data, or credentials, are exposed.
	14	+- silently wrong — the workflow runs and does the wrong thing, or
	15	+ reads empty values, with no error.
	16	+- silent no-op — a documented feature that never happens, with no
	17	+ error.
	18	+- loud — fails visibly; the author finds out.
	19	+
	20	+## Ranked
	21	+
	22	+\| # \| Gap \| Severity \| Evidence \| Plan \|
	23	+\|---\|---\|---\|---\|---\|
	24	+\| 1 \| Artifact endpoints trust the URL's run id, not the token: any job can list, download and create artifacts in any repository's runs \| security \| `lib/git_gud_web/controllers/actions_artifact_controller.ex` `create/2`, `list/2`, `download/2` (ignores `run_id`); `RunnerJwtAuth` assigns `:runner_run_id`, unused \| [workflow-artifacts-and-cache.md](workflow-artifacts-and-cache.md) \|
	25	+\| 2 \| Job token stays valid 24 h after the job ends (publish packages, read private code) \| security \| `lib/git_gud/workflows/runner_jwt.ex` `verify/1` checks only signature and `exp` (`ttl_seconds` 86 400) \| [workflow-job-token.md](workflow-job-token.md) \|
	26	+\| 3 \| Cache uploads: any job can append to/finalize another repo's entry; chunks stored in arrival order (`actions/cache` uploads 4 at a time) → caches > 32 MB corrupt; O(n²) append \| security + silently wrong \| `lib/git_gud_web/controllers/actions_cache_controller.ex` `upload_chunk/2`, `finalize/2`; `lib/git_gud/workflows/actions_cache.ex` `append/2` (read-concat-write, `Content-Range` ignored) \| [workflow-artifacts-and-cache.md](workflow-artifacts-and-cache.md) \|
	27	+\| 4 \| Cancel doesn't stop a running job: the runner's next `UpdateTask` (result unspecified) flips it back to `running` and the reply never says cancelled \| silently wrong \| `lib/git_gud_web/controllers/runner_controller.ex` `update_task/2`, `map_result/1`; `WorkflowJob.status_changeset/3` has no terminal guard; `Workflows.cancel/1` \| [workflow-cancellation-and-concurrency.md](workflow-cancellation-and-concurrency.md) \|
	28	+\| 5 \| `secrets.GITHUB_TOKEN` is empty (only user secrets are sent) \| silently wrong \| `RunnerController.build_task/1` (`GitGud.Secrets.resolve_for/1` only) \| [workflow-runner-context.md](workflow-runner-context.md) \|
	29	+\| 6 \| Workflow-level `env:` and `defaults:` never reach the runner; `on:` replaced by `"push"` (loses dispatch input defaults) \| silently wrong \| `lib/git_gud/workflows/runner_payload.ex` `encode/1` \| [workflow-runner-context.md](workflow-runner-context.md) \|
	30	+\| 7 \| `needs.<id>.outputs` / `.result` always empty \| silently wrong \| `RunnerController.build_needs/1` returns `%{}`; snapshot kept in `Workflows.resolve_dependents/1` but never sent (emptied in ed9a7cd while `TaskNeed` field numbers were still swapped — fixed in c4d7080) \| [workflow-job-graph.md](workflow-job-graph.md) \|
	31	+\| 8 \| `if: always()` / `failure()` jobs are cancelled after a failure; a job skipped by its `if:` is recorded as success and its dependents run; job `continue-on-error` ignored \| silently wrong \| `Workflows.resolve_dependents/1` (cancels on failed need), `RunnerController.map_result(:RESULT_SKIPPED)` → `"success"`, `WorkflowJob.satisfies_dependency?/1`, `Workflows.maybe_complete_run/1` \| [workflow-job-graph.md](workflow-job-graph.md) \|
	32	+\| 9 \| `github.event` is an empty object (sent as a string); `run_id`, `run_attempt`, `workflow_ref`, `server_url`, `api_url`, ids missing — so `github.event.pull_request.number`, `head_commit.message`, `inputs.*` are blank \| silently wrong \| `RunnerController.build_context_struct/4` (`"event" => run.trigger`, all values `string_value` in `wrap_as_struct/1`); runner reads `taskContext["event"].GetStructValue()` \| [workflow-runner-context.md](workflow-runner-context.md) \|
	33	+\| 10 \| `concurrency:` (groups, `cancel-in-progress`) ignored — concurrent deploys \| silently wrong \| nothing in `lib/` reads `concurrency` (grep) \| [workflow-cancellation-and-concurrency.md](workflow-cancellation-and-concurrency.md) \|
	34	+\| 11 \| One commit status per workflow name: a push run and a `pull_request` run on the same head (or two workflows sharing a `name:`) overwrite each other; no per-job statuses; PR page doesn't refresh when CI finishes \| silently wrong \| `Workflows.update_run_commit_status/1` (context `gitgud-actions/<name>`, upsert on `(repository_id, sha, context)`); `lib/git_gud_web/live/pr_live/show.ex` no `ci_topic` subscription \| [workflow-checks.md](workflow-checks.md) \|
	35	+\| 12 \| `branches:`/`tags:` `*` crosses `/`; no `!` negation there; `?`/`+` mean "any char"/literal instead of GitHub's quantifiers (also in `paths:`) \| silently wrong \| `Yaml.glob_match?/2`, `Yaml.kind_match?/5`; `lib/git_gud/workflows/glob.ex` `translate/2` \| [workflow-filter-patterns.md](workflow-filter-patterns.md) \|
	36	+\| 13 \| `permissions:` ignored: tokens are never narrowed (silent over-privilege), and `contents: write` can't push (loud) \| silently wrong / loud \| `RunnerJwt.mint/2` (no scope claim); `GitGudWeb.Plugs.GitSmartHttp` (job token: upload-pack only); `GitGud.Packages.Access.allowed?/3` \| [workflow-job-token.md](workflow-job-token.md) \|
	37	+\| 14 \| `matrix: ${{ fromJSON(...) }}` collapses to one leg with an empty matrix; `max-parallel` ignored; a fully-excluded matrix runs one empty leg \| silently wrong \| `lib/git_gud/workflows/matrix.ex` `expand/1` (`expand(_) -> [%{}]`), `pin_definition/2` (drops strategy / `max-parallel`); `Workflows.do_claim_next_job/2` \| [workflow-matrix.md](workflow-matrix.md) \|
	38	+\| 15 \| `workflow_dispatch` inputs dropped (API accepts, discards); no "Run workflow" button \| silently wrong \| `lib/git_gud_web/controllers/api/v1/workflow_controller.ex` (module doc), `Workflows.dispatch_manual/4` \| [workflow-triggers.md](workflow-triggers.md) (B) \|
	39	+\| 16 \| `[skip ci]` / `skip-checks: true` ignored \| silently wrong (extra runs) \| `Workflows.dispatch/2`, `dispatch_pull_request/3` \| [workflow-triggers.md](workflow-triggers.md) (A) \|
	40	+\| 17 \| `on: schedule` never fires \| silent no-op \| no scheduler reads workflow crons (`config/config.exs` Oban cron has only internal workers) \| [workflow-triggers.md](workflow-triggers.md) (C) \|
	41	+\| 18 \| `pull_request` `closed` / `edited` / `labeled` / `unlabeled` never fire \| silent no-op \| `Workflows.dispatch_pull_request/3` (refuses non-open), only opened/synchronize/reopened enqueued (`PullRequests`) \| [workflow-triggers.md](workflow-triggers.md) (E) \|
	42	+\| 19 \| `on: workflow_run` never fires; the `workflow_run` webhook fires only for successful (or cancelled) runs, never failed ones \| silent no-op / silently wrong \| `Workflows.maybe_complete_run/1` (calls `notify_run_webhook/1` in the success branch only) \| [workflow-triggers.md](workflow-triggers.md) (F) \|
	43	+\| 20 \| `create` / `delete` never fire \| silent no-op \| `Repositories.Workers.CommitBackfill.perform/1` (returns before dispatch on delete; new refs only as `push`) \| [workflow-triggers.md](workflow-triggers.md) (D) \|
	44	+\| 21 \| Fork and federated PRs never run (deliberate, debug log only); no `pull_request_target` \| silent no-op \| `Workflows.dispatch_pull_request/3` \| [workflow-fork-pull-requests.md](workflow-fork-pull-requests.md) \|
	45	+\| 22 \| Branch protection can't require status checks: `require_status_checks` / `required_checks` columns exist, are cast, and are read by nothing; not in the UI \| silent no-op \| `lib/git_gud/branch_protections/branch_protection.ex`; `BranchProtections.evaluate/5`; `branch_protection_live/index.ex` \| [workflow-checks.md](workflow-checks.md) \|
	46	+\| 23 \| `actions/upload-artifact@v4` / `download-artifact@v4` unsupported (only the v1 pipelines API exists); README claims "artifacts API v4" \| loud \| `lib/git_gud_web/router.ex` (`/pipelines/workflows/:run_id/artifacts` only) \| [workflow-artifacts-and-cache.md](workflow-artifacts-and-cache.md) fixes the README; v4 itself is listed there under "Out" as the next plan \|
	47	+
	48	+## No plan
	49	+
	50	+\| Gap / feature \| Why no plan \|
	51	+\|---\|---\|
	52	+\| `on: release` \| gitgud has no releases (`lib/git_gud/release.ex` is the Mix release module). Plan it with a releases feature. \|
	53	+\| `environment:` (environment secrets, protection rules, deployments) \| gitgud has no environments; the key passes through to act, which ignores it, and the job runs with repository secrets. Worth a plan only together with an environments feature. \|
	54	+\| OIDC (`permissions: id-token: write`, `ACTIONS_ID_TOKEN_REQUEST_*`) \| Loud (the action errors when the variables are missing); needs an OIDC issuer in gitgud — a feature, not a gap fix. \|
	55	+\| Reusable workflows (`jobs.<id>.uses: ./.github/workflows/x.yml`) and `on: workflow_call` \| Unverified. `Workflows.create_job_leg/3` creates such a job with `runs-on` defaulted to `ubuntu-latest` and no steps, and hands its definition to the runner; whether forgejo-runner/act expands a local `uses:` there was not tested. Test on a real runner first; plan only if it fails. \|
	56	+\| Branch-protection patterns, whose `` also crosses `/` (`BranchProtections.matches_pattern?/2`) \| Deliberately left: tightening it would silently un*protect nested branches. A maintainer decision, noted in the filter-patterns plan. \|
	57	+\| `mix precommit` can't pass on master \| Not a workflow gap: ~25 pre-existing compile warnings fail `--warnings-as-errors`, and a repo-wide `mix format` rewrites ~40 files (non-idempotent HEEx output). Every plan here works around it (compare warning lists, format only touched files). Worth a maintenance pass of its own. \|
	58	+
	59	+## Works (checked, nothing to plan)
	60	+
	61	+- Step-level `if:`, `continue-on-error`, `timeout-minutes`, step `outputs`,
	62	+ job/step `env`, job `defaults`, `services:`, `container:`, job
	63	+ `timeout-minutes` — all live in the job's definition, which is
	64	+ passed to act intact (`RunnerPayload.encode/1`, `job.definition`), and
	65	+ act implements them.
	66	+- Job `outputs:` are collected (`update_task/2` → `Workflows.set_outputs/2`);
	67	+ only their delivery to dependents is broken (#7).
	68	+- `runs-on` label routing incl. `self-hosted` and matrix-substituted
	69	+ labels (`Workflows.do_claim_next_job/2`, `Matrix.substitute/2`).
	70	+- Static matrices with `include`/`exclude`, `fail-fast` for queued legs
	71	+ (`Matrix.expand/1`, `Workflows.maybe_fail_fast/1`; running legs: #4's
	72	+ plan).
	73	+- `push` with `branches`/`tags`/`paths`, `pull_request`
	74	+ opened/synchronize/reopened, `workflow_dispatch` via the API, annotated
	75	+ tags (`Workflows.dispatch/2`, `dispatch_pull_request/3`).
	76	+- Repository and organization secrets and variables
	77	+ (`GitGud.Secrets.resolve_for/1`), reserved `GITHUB_`/`GITEA_`/
	78	+ `FORGEJO_*` names (`Secrets.ReservedNames`).
	79	+- Logs per job and step (`update_log/2` keyed by task id since 56749c0),
	80	+ live log streaming, re-run of a whole run or a single job
	81	+ (`Workflows.rerun/1`, `rerun_job/2`; the attempt counter is in the
	82	+ context plan), stale-job recovery (`Workers.StaleJobReaper`).
	83	+- Cache lookup order and scoping on read (`ActionsCache.lookup/5` filters
	84	+ by repository; PR fallback to the base branch is in the artifacts/cache
	85	+ plan).
	86	+- Commit statuses on the commit, log and PR pages
	87	+ (`core_components.ex` `commit_status_badges/1`).
	88	+
	89	+## Suggested order
	90	+
	91	+Artifacts-and-cache and job-token first (security). Then runner-context,
	92	+job-graph, cancellation-and-concurrency (the silently-wrong core). Then
	93	+checks, filter-patterns, matrix, triggers. Fork pull requests last (it
	94	+needs job-token).

added docs/plans/workflow-artifacts-and-cache.md

+194 −0

@@ -0,0 +1,194 @@

	1	+# Plan: artifacts and cache that stay in their own run and repository
	2	+
	3	+Status: ready to execute. Written to be carried out start to finish by
	4	+one agent without further questions. Independent of the other CI plans,
	5	+but read step 3 against [workflow-runner-context.md](workflow-runner-context.md):
	6	+that plan adds `run_id` to the job context, which is what the artifact
	7	+URLs are built from.
	8	+
	9	+## Why
	10	+
	11	+Two security holes and one data-corruption bug, all in the services a
	12	+running job calls back into.
	13	+
	14	+1. Artifacts trust the URL, not the token.
	15	+ `GitGudWeb.ActionsArtifactController` takes `run_id` from the path
	16	+ (`/api/actions/_apis/pipelines/workflows/:run_id/artifacts…`) and never
	17	+ compares it with the JWT's `rid`, which `GitGudWeb.Plugs.RunnerJwtAuth`
	18	+ has already put in `conn.assigns.runner_run_id`. `create/2` writes into
	19	+ whatever run the path names; `list/2` lists any run; `download/2` ignores
	20	+ `run_id` completely and serves any artifact id. So any job, in any
	21	+ repository, holding its own valid job token, can read every artifact on
	22	+ the instance (ids are sequential) and plant artifacts in other
	23	+ repositories' runs.
	24	+2. Cache uploads aren't checked against the repository either.
	25	+ `GitGudWeb.ActionsCacheController.upload_chunk/2` and `finalize/2` load
	26	+ the entry by id (`ActionsCache.get!/1`) with no check that
	27	+ `entry.repository_id` is the token's `pid`. A job can append to another
	28	+ repository's reserved-but-unfinished cache entry (cache poisoning).
	29	+3. Cache chunks are concatenated in arrival order.
	30	+ `ActionsCache.append/2` reads the whole stored object and writes it back
	31	+ with the new body appended; the `Content-Range` header the client sends
	32	+ is ignored. `actions/cache` uploads in 32 MB chunks, four at a time
	33	+ (`uploadConcurrency`), so any cache over 32 MB is stored with its chunks
	34	+ in whatever order the requests finished — and two concurrent appends can
	35	+ also lose a chunk (read-modify-write race). The cache "saves", the next
	36	+ job restores a corrupt tarball. It is also O(n²) in memory for a large
	37	+ cache.
	38	+
	39	+And two smaller ones the same work should fix:
	40	+
	41	+4. The README says "GitHub Actions artifacts API v4". The code implements a
	42	+ simplified v1 "pipelines" API only. `actions/upload-artifact@v4` and
	43	+ `download-artifact@v4` (what GitHub's docs and every new workflow use)
	44	+ speak a different, Twirp-based protocol
	45	+ (`twirp/github.actions.results.api.v1.ArtifactService/*`, located via
	46	+ `ACTIONS_RESULTS_URL`) and fail. That failure is loud, so this plan only
	47	+ corrects the README and records v4 as a follow-up (see "Out").
	48	+5. A PR run's cache falls back only to the default branch
	49	+ (`ActionsCacheController.refs_for/1`); GitHub falls back to the PR's
	50	+ base branch.
	51	+
	52	+## Semantics to match
	53	+
	54	+- An artifact belongs to one workflow run. A job may create, list and
	55	+ download artifacts of its own run only (that is how
	56	+ `upload-artifact@v3`/`download-artifact@v3` share files between jobs of a
	57	+ run). Anything else is 404 (not 403: don't confirm the id exists).
	58	+- A cache entry belongs to one repository. Only a job of that repository
	59	+ may upload to, finalize, or read it. (Read was already scoped:
	60	+ `ActionsCache.lookup/5` filters on `repository_id`.)
	61	+- Cache upload chunks carry `Content-Range: bytes <start>-<end>/*`; the
	62	+ stored object is the chunks placed at their offsets. Finalize gives
	63	+ the total `size`.
	64	+- Cache restore order for a PR run: current ref, then the PR's base
	65	+ branch, then the default branch.
	66	+
	67	+## Where things are
	68	+
	69	+\| \| \|
	70	+\|---\|---\|
	71	+\| Routes \| `lib/git_gud_web/router.ex`, `scope "/api/actions/_apis"` with `pipe_through :runner_jwt` \|
	72	+\| JWT plug \| `lib/git_gud_web/plugs/runner_jwt_auth.ex` — assigns `:runner_jwt_claims`, `:runner_job_id`, `:runner_run_id` \|
	73	+\| Artifacts \| `lib/git_gud_web/controllers/actions_artifact_controller.ex` (`create/2`, `upload/2`, `finalize/2`, `list/2`, `download/2`); schema `lib/git_gud/workflows/workflow_artifact.ex` \|
	74	+\| Cache \| `lib/git_gud_web/controllers/actions_cache_controller.ex` (`get_cache/2`, `reserve_cache/2`, `upload_chunk/2`, `finalize/2`, `refs_for/1`); `lib/git_gud/workflows/actions_cache.ex` (`reserve/4`, `append/2`, `finalize/2`); schema `lib/git_gud/workflows/cache_entry.ex` \|
	75	+\| Storage \| `GitGud.Storage` (`put/4`, `get/2`, `presign_url/4`) — check what else the behaviour offers (multipart? append?) before choosing step 4's design \|
	76	+\| Tests \| `test/git_gud_web/controllers/actions_endpoints_test.exs`, `test/git_gud/workflows/actions_cache_test.exs`, `test/git_gud/workflows/runner_jwt_test.exs` (minting a token for a job) \|
	77	+
	78	+## Steps
	79	+
	80	+### 1. Artifacts: the token's run, not the path's
	81	+
	82	+In `ActionsArtifactController`, add a private `own_run(conn, run_id_param)`
	83	+that returns `{:ok, rid}` when `String.to_integer(run_id_param) ==
	84	+conn.assigns.runner_run_id`, else `:error`; an unparsable id is `:error`.
	85	+Every action starts with it and answers 404 on `:error`.
	86	+
	87	+- `create/2`, `list/2`, `finalize/2`: as above.
	88	+- `upload/2`: already compares the artifact's run with the path; also
	89	+ require the path's run to be the token's.
	90	+- `download/2`: load the artifact and require
	91	+ `artifact.workflow_run_id == conn.assigns.runner_run_id`, else 404. Use
	92	+ `Repo.get/2`, not `get!/2` (a missing id is 404, not a 500).
	93	+- `upload/2` and `upload_chunk/2` read bodies with fixed caps
	94	+ (`1_000_000_000`, `200_000_000`). Leave the values; just make the oversize
	95	+ case answer 413 instead of crashing (`Plug.Conn.read_body/2` returns
	96	+ `{:more, …}` past `:length`).
	97	+
	98	+Also: if `run_id` in the path is `0`/empty because the job had no
	99	+`github.run_id` (see the context plan), the old behaviour wrote into run 1
	100	+of some other repository. After this step it is a 404 — loud, and fixed for
	101	+real by the context plan's `run_id`.
	102	+
	103	+### 2. Cache: the token's repository
	104	+
	105	+In `ActionsCacheController.upload_chunk/2` and `finalize/2`, load with
	106	+`ActionsCache.get/1` (add it: `Repo.get(CacheEntry, id)`) and require
	107	+`entry.repository_id == jwt_repo_id(conn)`; otherwise 404. Also refuse
	108	+(409) an upload to an entry whose `archived_at` is set — a finalized entry
	109	+is immutable.
	110	+
	111	+### 3. Cache: chunks at their offsets
	112	+
	113	+Replace read-append-write with offset-placed chunks:
	114	+
	115	+- Parse `Content-Range: bytes S-E/*` in `upload_chunk/2` (missing or
	116	+ malformed → 400). Check `E - S + 1 == byte_size(body)`.
	117	+- Store each chunk as its own object, `"#{entry.storage_key}.part.#{S}"`,
	118	+ with `Storage.put/4`. Concurrent chunks then never touch the same object.
	119	+- `finalize/2`: list the parts for the entry (keep the offsets in a new
	120	+ table `cache_entry_parts (cache_entry_id, offset, size, storage_key)`
	121	+ written by `upload_chunk/2` — simpler and more portable than listing the
	122	+ bucket; `mix ecto.gen.migration add_cache_entry_parts`), check they tile
	123	+ `0..size-1` with no gaps or overlaps (else 400 and leave the entry
	124	+ unarchived), then assemble them in offset order into
	125	+ `entry.storage_key` and delete the parts. Assemble by streaming if
	126	+ `GitGud.Storage` can (look at the behaviour; Garage is S3, so a multipart
	127	+ upload or server-side compose may exist); otherwise concatenate in
	128	+ memory — still correct, and the 200 MB-per-request cap bounds each read.
	129	+- `ActionsCache.append/2` is then unused: remove it.
	130	+- The `CacheEviction` worker (`lib/git_gud/workflows/workers/cache_eviction.ex`)
	131	+ must also delete orphaned parts of entries that were never finalized:
	132	+ check what it does with unarchived entries today and extend it.
	133	+
	134	+### 4. Cache: a PR falls back to its base branch
	135	+
	136	+In `refs_for/1`, when the run's `trigger == "pull_request"` and it has
	137	+`pr_base_ref` (column added by the pull_request plan), the fallback list is
	138	+`[run.head_ref, "refs/heads/" <> run.pr_base_ref, default_ref]`.
	139	+`ActionsCache.lookup/5` takes a single `default_ref`; change it to take a
	140	+list of fallback refs (`[ref \| fallbacks]`) and keep its exact-then-prefix
	141	+order across them. Saving still goes to the run's own ref.
	142	+
	143	+### 5. README
	144	+
	145	+In README's CI list, replace "GitHub Actions artifacts API v4" with what is
	146	+true: the v1 pipelines artifact API (`actions/upload-artifact@v3` /
	147	+`download-artifact@v3`), scoped to the run; `@v4` not yet supported. State
	148	+the cache's restore order.
	149	+
	150	+## Tests
	151	+
	152	+`actions_endpoints_test.exs` (mint tokens with `RunnerJwt.mint/2` for jobs
	153	+in two different repositories' runs):
	154	+
	155	+- job A creates an artifact in its run; job B (other repo) gets 404 on
	156	+ create/list/upload/download against A's run and on download of A's
	157	+ artifact id through B's own run path;
	158	+- job A lists and downloads its own artifact (still works);
	159	+- a second job of the same run can list and download it (cross-job
	160	+ sharing still works).
	161	+
	162	+`actions_cache_test.exs` / controller test:
	163	+
	164	+- B's token gets 404 on `upload_chunk`/`finalize` of A's entry;
	165	+- three chunks uploaded out of order (offsets 2N, 0, N) then finalize →
	166	+ the restored bytes equal the original;
	167	+- a gap (chunks 0 and 2N only) → finalize 400, entry not archived;
	168	+- upload after finalize → 409;
	169	+- PR run: an entry saved on the base branch (not the default branch) is
	170	+ found by a PR run's lookup; an entry on an unrelated branch is not.
	171	+
	172	+## Out
	173	+
	174	+- Artifacts v4 (`ArtifactService` Twirp: `CreateArtifact`,
	175	+ `FinalizeArtifact`, `ListArtifacts`, `GetSignedArtifactURL`, blob upload
	176	+ to a signed URL, `ACTIONS_RESULTS_URL` in the job env). Worth its own
	177	+ plan once this one lands; Forgejo implements it in
	178	+ `routers/api/actions/artifactsv4.go`, which is the reference.
	179	+- Cache service v2 (`actions/cache@v4` with `ACTIONS_CACHE_SERVICE_V2`).
	180	+ First check whether jobs reach this cache at all: forgejo-runner ships
	181	+ its own cache server and may point `ACTIONS_CACHE_URL` at it regardless
	182	+ of the `ACTIONS_CACHE_URL` key gitgud puts in the task context
	183	+ (`runner_controller.ex`, `build_context_struct/4`). Record what you find
	184	+ in the README.
	185	+
	186	+## Done when
	187	+
	188	+- `mix test` passes; no new compile warnings (the repo has pre-existing
	189	+ ones: compare the warning list before and after, `mix compile --force`).
	190	+ `mix format` only the files you touched — a repo-wide `mix format`
	191	+ rewrites ~40 unrelated files.
	192	+- One commit, in the repository's message style, naming the cross-repo
	193	+ artifact access as the reason.
	194	+- Not pushed. Pushing gitgud deploys the forge.

added docs/plans/workflow-cancellation-and-concurrency.md

+204 −0

@@ -0,0 +1,204 @@

	1	+# Plan: cancelling a running job, and `concurrency:`
	2	+
	3	+Status: ready to execute. Written to be carried out start to finish by
	4	+one agent without further questions. Steps 1–2 (cancellation reaching the
	5	+runner) stand alone and are worth committing on their own first; steps 3–7
	6	+(`concurrency:`) build on them. Two commits.
	7	+
	8	+## Why
	9	+
	10	+1. Cancel doesn't cancel a running job. `Workflows.cancel/1` marks the
	11	+ run's queued and running jobs `cancelled` in the database and stops
	12	+ there. forgejo-runner learns of a cancellation only from the reply to
	13	+ its periodic `UpdateTask` (`internal/pkg/report/reporter.go`,
	14	+ `ReportState`: if the reply's `state.result` is `RESULT_CANCELLED` or
	15	+ `RESULT_FAILURE`, it cancels the job's context). But while a job runs the
	16	+ runner reports `RESULT_UNSPECIFIED`, and
	17	+ `RunnerController.update_task/2` maps that to `"running"`
	18	+ (`map_result/1`) and writes it back with `Workflows.update_job_status/3`
	19	+ — so the job flips from `cancelled` back to `running`, the reply says
	20	+ `RESULT_UNSPECIFIED`, and the runner carries on to the end. The run page
	21	+ says "cancelled" while the job keeps deploying. Nothing in
	22	+ `WorkflowJob.status_changeset/3` stops a terminal job from becoming
	23	+ non-terminal again.
	24	+2. Fail-fast leaves running legs running. `Workflows.maybe_fail_fast/1`
	25	+ cancels only `queued`/`blocked` siblings ("killing a mid-flight job loses
	26	+ its logs for no gain"). GitHub cancels in-progress legs too; once step 1
	27	+ exists, cancellation keeps the logs (the runner still uploads them).
	28	+3. `concurrency:` is ignored. Nothing in `lib/` reads it (grep
	29	+ `concurrency`). A deploy workflow with
	30	+ `concurrency: {group: deploy-prod, cancel-in-progress: false}` runs two
	31	+ deploys at once when two pushes land close together; a PR workflow with
	32	+ `cancel-in-progress: true` burns runners on superseded commits. Both are
	33	+ silent.
	34	+
	35	+## Semantics to match (GitHub, "Control the concurrency of workflows and jobs")
	36	+
	37	+- `concurrency` at the workflow top level or on a job: either a string
	38	+ (the group) or `{group: <string>, cancel-in-progress: <bool>}`. Both
	39	+ values may be expressions (`${{ … }}`).
	40	+- Within a group (scoped to the repository): at most one running and
	41	+ one pending run (or job). A new member arriving while one is running:
	42	+ - `cancel-in-progress: true` → the running member and any pending member
	43	+ are cancelled; the new one starts;
	44	+ - otherwise → the new one waits as pending; an already pending member
	45	+ is cancelled (replaced by the newer one).
	46	+- When the running member finishes, the pending one starts.
	47	+- Workflow-level groups are decided when the run is created; job-level
	48	+ groups when the job would otherwise be queued (it can use `matrix` and
	49	+ `needs`).
	50	+
	51	+## Where things are
	52	+
	53	+\| \| \|
	54	+\|---\|---\|
	55	+\| Runner's view \| `lib/git_gud_web/controllers/runner_controller.ex` `update_task/2`, `map_result/1`, `result_atom/1` \|
	56	+\| Job lifecycle \| `lib/git_gud/workflows.ex` `update_job_status/3`, `cancel/1`, `maybe_fail_fast/1`, `maybe_complete_run/1`, `create_run/2`, `create_job_leg/3`, `resolve_dependents/1`, `do_claim_next_job/2` (claims `status == "queued"` only) \|
	57	+\| Schemas \| `lib/git_gud/workflows/workflow_job.ex` (`@valid_statuses`, `status_changeset/3`, `terminal?/1`), `workflow_run.ex` \|
	58	+\| Stale job reaper \| `lib/git_gud/workflows/workers/stale_job_reaper.ex` — make sure it doesn't resurrect cancelled jobs \|
	59	+\| UI \| `lib/git_gud_web/live/workflow_live/show.ex`, `index.ex`, `lib/git_gud_web/live/ci_components.ex` (status badges) \|
	60	+\| Tests \| `test/git_gud/workflows/rerun_cancel_test.exs`, `matrix_dispatch_test.exs`, `test/git_gud_web/controllers/runner_controller_test.exs` \|
	61	+
	62	+## Steps
	63	+
	64	+### 1. A terminal job stays terminal; the runner is told
	65	+
	66	+- In `update_task/2`: if the job is already terminal
	67	+ (`WorkflowJob.terminal?/1`) and the runner reports `RESULT_UNSPECIFIED`
	68	+ (still running), don't update its status; reply with
	69	+ `result_atom(job.status)` — `RESULT_CANCELLED` for a cancelled job, which
	70	+ makes the runner stop. Still apply step states and log rows (they are
	71	+ the job's real history).
	72	+- When the runner then sends its final result for a job the server
	73	+ already cancelled, keep `cancelled` (the server's decision wins) but
	74	+ record the step states; reply `RESULT_CANCELLED`.
	75	+- Guard at the schema too: `WorkflowJob.status_changeset/3` refuses a
	76	+ transition from a terminal status to `running`/`queued`/`blocked` unless
	77	+ given `force: true`; `rerun_job/2` (the one legitimate path back) passes
	78	+ it. Grep for every `status_changeset(` caller and check each.
	79	+- `StaleJobReaper` must only touch `running` jobs (check it).
	80	+
	81	+### 2. Fail-fast cancels running legs
	82	+
	83	+`maybe_fail_fast/1`: include `running` siblings. They become `cancelled`
	84	+in the database; step 1 makes their runners stop. Update the comment that
	85	+explains why it didn't. Commit steps 1–2.
	86	+
	87	+### 3. A small expression evaluator for groups
	88	+
	89	+Group names are almost always built from a few context values:
	90	+`${{ github.workflow }}-${{ github.ref }}`,
	91	+`${{ github.event.pull_request.number \|\| github.ref }}`,
	92	+`${{ github.head_ref \|\| github.run_id }}`, `${{ inputs.env }}`,
	93	+`${{ matrix.target }}`. Add `GitGud.Workflows.Expr` with
	94	+`interpolate(string, contexts) :: {:ok, String.t()} \| :error` that
	95	+replaces each `${{ … }}` with the evaluated value, supporting: property
	96	+paths (`a.b.c`, `a['b']`) into `contexts` (string-keyed maps: `github`,
	97	+`inputs`, `vars`, `matrix`, `needs`), string/number/boolean/null literals,
	98	+`\|\|`, `&&`, `==`, `!=`, `!`, parentheses, and `format('{0}-{1}', …)`.
	99	+Values stringify as GitHub does (null → `""`, booleans `true`/`false`).
	100	+Anything else (other functions, unknown syntax) → `:error`. Keep it a
	101	+recursive-descent parser in one module; no new dependency.
	102	+
	103	+Contexts are built from what the run already stores: `github` = the same
	104	+keys the runner context gets (`RunnerController.build_context_struct/4`
	105	+— extract the map-building half into a public function in
	106	+`GitGud.Workflows` or a new `GitGud.Workflows.Context` module, so the
	107	+runner and the evaluator can't disagree), `inputs` from the event payload
	108	+if [workflow-runner-context.md](workflow-runner-context.md) has been
	109	+executed (else `%{}`), `vars` from `GitGud.Secrets.resolve_for/1`'s vars,
	110	+`matrix` = the job's combination, `needs` = the job's `needs_snapshot`.
	111	+
	112	+If evaluation returns `:error`, the run/job proceeds without
	113	+concurrency control, and the reason is logged at warning level and shown
	114	+on the run (see step 6). Never block a run on an expression we couldn't
	115	+read.
	116	+
	117	+### 4. Store the group
	118	+
	119	+Migration (`mix ecto.gen.migration add_concurrency_to_workflow_runs_and_jobs`):
	120	+`concurrency_group :string` and `cancel_in_progress :boolean, default:
	121	+false` on both `workflow_runs` and `workflow_jobs`, plus an index on
	122	+`(repository_id, concurrency_group)` for runs; for jobs index
	123	+`(concurrency_group)` and join through the run for the repository. Run it
	124	+on dev and test.
	125	+
	126	+New job status `waiting` (held by concurrency, not claimable): add to
	127	+`WorkflowJob.@valid_statuses`, not terminal, and to the UI badge helpers.
	128	+`maybe_complete_run/1` treats it as in flight.
	129	+
	130	+### 5. Workflow-level concurrency
	131	+
	132	+In `create_run/2` (inside its transaction), read
	133	+`workflow_doc["concurrency"]`; evaluate the group with the run's
	134	+contexts. If a group results:
	135	+
	136	+- lock the group's members (`SELECT … FOR UPDATE` on runs of this repo in
	137	+ this group with status `queued`/`running`, plus runs with any job
	138	+ `waiting`), then:
	139	+ - `cancel-in-progress` → `Workflows.cancel/1` each of them (after
	140	+ commit: do the cancels after the transaction returns, collecting the
	141	+ ids inside it, to avoid nesting `cancel/1`'s own transaction);
	142	+ - else → cancel any member that has not started (all its jobs
	143	+ `waiting`), and if one is running, create this run's initially-queued
	144	+ jobs as `waiting` instead of `queued`.
	145	+- When a run reaches a terminal state (`maybe_complete_run/1` and
	146	+ `cancel/1`), release its group: the oldest run in the group whose jobs
	147	+ are `waiting` gets them moved to `queued` (or `blocked` when they have
	148	+ `needs`, matching `create_job_leg/3`'s rule).
	149	+
	150	+### 6. Job-level concurrency
	151	+
	152	+Same rules per job, evaluated when the job would become `queued` (in
	153	+`create_job_leg/3` for jobs without `needs`, and in
	154	+`resolve_dependents/1` when promoting), with `matrix` and `needs` in the
	155	+contexts. The group's members are jobs (any repository-scoped run) in
	156	+`queued`/`running`/`waiting`. Release in `update_job_status/3` when the
	157	+job becomes terminal.
	158	+
	159	+On the run page, show a waiting job as "Waiting for <group>" and a
	160	+cancelled-by-concurrency job with that reason: add a nullable
	161	+`status_reason :string` to `workflow_jobs` in the same migration and set
	162	+it in steps 5–6.
	163	+
	164	+### 7. Payload and docs
	165	+
	166	+Make sure `RunnerPayload.encode/1` does not pass `concurrency` to the
	167	+runner (act ignores it today; it should never decide). README CI list:
	168	+`concurrency` (workflow and job level, `cancel-in-progress`), the supported
	169	+expression subset, and that an unreadable group means no concurrency
	170	+control.
	171	+
	172	+## Tests
	173	+
	174	+- Runner protocol (`runner_controller_test.exs`): claim a job, cancel its
	175	+ run, then `UpdateTask` with `RESULT_UNSPECIFIED` → reply
	176	+ `RESULT_CANCELLED`, job still `cancelled`; a final `RESULT_SUCCESS` after
	177	+ cancel → job stays `cancelled`.
	178	+- `rerun_job/2` still revives a cancelled job.
	179	+- Fail-fast: a running sibling leg is cancelled.
	180	+- `Expr` unit tests: each pattern listed in step 3; `\|\|` fallbacks with
	181	+ null and empty string (GitHub: empty string is falsy); unknown function
	182	+ → `:error`.
	183	+- Workflow concurrency (on-disk dispatch, `path_dispatch_test.exs` style,
	184	+ with `concurrency: {group: deploy, cancel-in-progress: false}`): first
	185	+ push → jobs queued; second → jobs waiting; third → second run cancelled,
	186	+ third waiting; completing the first (drive `update_job_status/3`) →
	187	+ third queued.
	188	+- `cancel-in-progress: true`: second push cancels the first run (its
	189	+ running job is `cancelled`, and its runner would be told — assert via
	190	+ `update_task` as above).
	191	+- Group `${{ github.workflow }}-${{ github.ref }}`: pushes to two different
	192	+ branches don't interact.
	193	+- Job-level group with `${{ matrix.target }}`: legs with the same target
	194	+ serialize, different targets don't.
	195	+- An unparseable group (`${{ toJSON(github) }}`) → runs normally, warning
	196	+ logged.
	197	+
	198	+## Done when
	199	+
	200	+- `mix test` passes; no new compile warnings (compare `mix compile --force`
	201	+ warning lists before/after); `mix format` only touched files.
	202	+- Two commits in the repository's message style: cancellation, then
	203	+ concurrency.
	204	+- Not pushed.

added docs/plans/workflow-checks.md

+174 −0

@@ -0,0 +1,174 @@

	1	+# Plan: one commit status per job, and branch protection that can require them
	2	+
	3	+Status: ready to execute. Written to be carried out start to finish by
	4	+one agent without further questions. Independent of the other CI plans;
	5	+if [workflow-job-graph.md](workflow-job-graph.md) has been executed, jobs
	6	+can also be `skipped` — map that as step 1 says.
	7	+
	8	+## Why
	9	+
	10	+1. Two runs on one commit overwrite each other's status.
	11	+ `Workflows.update_run_commit_status/1` writes one row per run with
	12	+ context `"gitgud-actions/" <> (run.workflow_name \|\| run.workflow_path)`,
	13	+ upserting on `(repository_id, sha, context)`. A same-repository pull
	14	+ request's head commit gets a `push` run (the push to the branch) and a
	15	+ `pull_request` run (synchronize) of the same workflow — one context,
	16	+ so whichever finished last is what the PR page shows: a failing PR run
	17	+ can be hidden by a passing push run. Two workflow files that share a
	18	+ `name:` collide the same way.
	19	+2. Statuses are per workflow, not per job. GitHub and Forgejo report
	20	+ each job: `<workflow> / <job> (<event>)` (Forgejo:
	21	+ `services/actions/commit_status.go`). A workflow with `test` and
	22	+ `lint` shows one pill here, so you can't see which job failed, and
	23	+ branch protection (below) couldn't require `lint` alone.
	24	+3. Branch protection can't require checks. `BranchProtection` has
	25	+ `require_status_checks` and `required_checks` columns
	26	+ (`lib/git_gud/branch_protections/branch_protection.ex`), cast by its
	27	+ changeset, but nothing reads them: `BranchProtections.evaluate/5` checks
	28	+ force-push and approvals only, and the settings UI
	29	+ (`lib/git_gud_web/live/branch_protection_live/index.ex`) doesn't offer
	30	+ them. Anything that sets them through the changeset gets a rule that
	31	+ silently enforces nothing.
	32	+4. The PR page doesn't update when CI finishes.
	33	+ `lib/git_gud_web/live/pr_live/show.ex` renders
	34	+ `<.commit_status_badges repo={@repo} sha={@pr.head_sha} />` (line ~533)
	35	+ but doesn't subscribe to `Workflows.ci_topic/0`, so pills stay "pending"
	36	+ until a reload.
	37	+
	38	+## Semantics to match
	39	+
	40	+- Context per job: `"#{workflow_name} / #{job_name} (#{event})"`, where
	41	+ `workflow_name` = `run.workflow_name \|\| Path.basename(run.workflow_path)`,
	42	+ `job_name` = the job row's `name` (already `build (arm64, 28)` for a
	43	+ matrix leg), `event` = `run.trigger`. That is Forgejo's format, so
	44	+ required-check names people copy from Forgejo match.
	45	+- State: queued/blocked/waiting/running → `pending`; success → `success`;
	46	+ failure and cancelled → `failure` (today's mapping, and Forgejo's in
	47	+ `services/actions/commit_status.go`, `toCommitStatus`); skipped →
	48	+ `success` with description "Skipped" (Forgejo maps skipped to success,
	49	+ and GitHub counts a skipped required check as passing).
	50	+- Required checks (GitHub/Forgejo): a PR may be merged into a branch whose
	51	+ rule has `require_status_checks` only when, for the PR head commit,
	52	+ every pattern in `required_checks` matches at least one status and
	53	+ every matching status is `success`. Patterns are globs (`*` matches
	54	+ any run of characters, including `/` and spaces — Forgejo uses glob
	55	+ patterns for this field). An empty `required_checks` with
	56	+ `require_status_checks` on means "every status on the head commit must
	57	+ be success, and there must be at least one".
	58	+- Direct pushes to a protected branch are not blocked by status checks
	59	+ (no PR, no head to have checks on — Forgejo's behaviour); say so in the
	60	+ UI hint.
	61	+- Admin override: `merge!/3`'s `override: true` already bypasses review
	62	+ rules; let it bypass status checks too (same audit trail).
	63	+
	64	+## Where things are
	65	+
	66	+\| \| \|
	67	+\|---\|---\|
	68	+\| 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` \|
	69	+\| Status schema \| `lib/git_gud/workflows/commit_status.ex`; reader `Workflows.list_commit_statuses/2` \|
	70	+\| Pills \| `lib/git_gud_web/components/core_components.ex` `commit_status_badges/1`, `display_context/1` (strips `gitgud-actions/`) \|
	71	+\| 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) \|
	72	+\| 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` \|
	73	+\| Settings UI \| `lib/git_gud_web/live/branch_protection_live/index.ex` \|
	74	+\| Tests \| `test/git_gud/branch_protections_test.exs`, `test/git_gud/workflows/rerun_cancel_test.exs`, `test/git_gud_web/live/pr_live/` \|
	75	+
	76	+## Steps
	77	+
	78	+### 1. Per-job statuses
	79	+
	80	+Add `Workflows.update_job_commit_status(job, run)` writing the context and
	81	+state above, `target_url` = the run page anchored to the job if the run
	82	+page has job anchors (check `workflow_live/show.ex`; else the run page),
	83	+`description` = e.g. "Successful in 2m", "Failing after 40s", "Waiting to
	84	+run", "Skipped". Call it:
	85	+- for every job row created in `create_job_leg/3` (initial pending),
	86	+- in `update_job_status/3` after the update,
	87	+- in `cancel/1` for each job it cancels (it uses `update_all`; select the
	88	+ affected rows first, or re-read them after).
	89	+
	90	+Stop writing the per-run `gitgud-actions/<workflow>` status (delete the
	91	+writer, keep `broadcast_ci_change/1` — move that call into the new
	92	+function). Old rows stay on old commits; `display_context/1` keeps
	93	+stripping their prefix.
	94	+
	95	+### 2. PR page refreshes
	96	+
	97	+In `pr_live/show.ex` `mount/3` (when `connected?`), subscribe to
	98	+`Workflows.ci_topic/0`; on `{:ci_run_changed, run_id}` re-render the
	99	+status pills if the run is in this repository (load the run's
	100	+`repository_id` — cheap) — e.g. bump an assign the badges component reads
	101	+so it reloads.
	102	+
	103	+### 3. Required checks: evaluation
	104	+
	105	+In `BranchProtections`, add `check_statuses(rule, repo, head_sha)`
	106	+returning `:ok` or `{:error, {:status_checks, missing :: [pattern],
	107	+failing :: [context]}}`, using `Workflows.list_commit_statuses/2` and a
	108	+glob matcher where `*` matches anything (a private regex helper; don't use
	109	+`GitGud.Workflows.Glob`, whose `*` stops at `/`).
	110	+
	111	+Restructure `evaluate/5` from a `cond` into a `with` chain so all rules
	112	+apply in turn (today the approvals clause returns early and nothing after
	113	+it could run): force-push → approvals → status checks. Status checks apply
	114	+only when `opts[:pull_request_id]` is given (a merge); pass the PR head
	115	+sha in opts (`head_sha:`) from `merge!/3` and `Merges` (check what
	116	+`Merges` has in hand: `analysis.theirs_sha` is the head being merged).
	117	+
	118	+`merge!/3`: `override: true` skips the status-check error as it skips
	119	+review blocks. Find how override is threaded into `check_ref_update` today
	120	+(it isn't — it's only used by `check_review_block/3`) and pass it.
	121	+
	122	+### 4. Required checks: on the PR page
	123	+
	124	+Extend what the merge box shows (next to `PullRequests.merge_blockers/2`'s
	125	+review blockers): "Required checks: <pattern> missing", "<context>
	126	+failing", and disable the merge button the same way review blocks do. Put
	127	+the lookup in `PullRequests` (e.g. `status_check_blockers(repo, pr)`
	128	+calling `BranchProtections.rule_for_branch/2` + `check_statuses/3`) so the
	129	+LiveView and `merge!/3` agree.
	130	+
	131	+### 5. Required checks: settings UI
	132	+
	133	+In `branch_protection_live/index.ex`: a checkbox "Require status checks to
	134	+pass before merging" (`require_status_checks`) and a text input "Required
	135	+checks" (comma- or newline-separated → `required_checks` list; parse in
	136	+the LiveView before casting), with a hint listing the contexts seen on the
	137	+default branch's latest commit (`Workflows.list_commit_statuses/2`) so
	138	+people can copy exact names. Show them in the rule list.
	139	+
	140	+### 6. Docs
	141	+
	142	+README CI list: per-job statuses and their names; required checks and
	143	+their glob rule; that direct pushes aren't checked.
	144	+
	145	+## Tests
	146	+
	147	+- A run with jobs `test` and `lint` creates two pending statuses with the
	148	+ Forgejo-format contexts; failing `lint` → its status `failure`, `test`
	149	+ unaffected.
	150	+- A `push` run and a `pull_request` run on the same sha → four distinct
	151	+ statuses (`… (push)` and `… (pull_request)`), neither overwrites the
	152	+ other.
	153	+- Cancel → each cancelled job's status becomes `failure` with description
	154	+ "Cancelled".
	155	+- `BranchProtections` with `require_status_checks: true,
	156	+ required_checks: ["ci / test *"]`:
	157	+ - head has `ci / test (push)` success → merge allowed;
	158	+ - head has it `failure` → `{:status_checks, [], ["ci / test (push)"]}`;
	159	+ - no matching status → `{:status_checks, ["ci / test *"], []}`;
	160	+ - `required_approvals: 1` plus checks: both enforced (the old `cond`
	161	+ would have stopped at approvals);
	162	+ - a direct push (no `pull_request_id`) is not blocked by checks;
	163	+ - `merge!(…, override: true)` merges despite a failing check.
	164	+- LiveView: the PR page shows the missing/failing checks and the merge
	165	+ button is disabled; a broadcast on `ci_topic` re-renders the pills.
	166	+- Branch protection settings: saving the checkbox and a two-line
	167	+ `required_checks` field persists a two-element list.
	168	+
	169	+## Done when
	170	+
	171	+- `mix test` passes; no new compile warnings (compare `mix compile --force`
	172	+ warning lists before/after); `mix format` only touched files.
	173	+- One commit in the repository's message style.
	174	+- Not pushed.

added docs/plans/workflow-filter-patterns.md

+144 −0

@@ -0,0 +1,144 @@

	1	+# Plan: GitHub's filter patterns for branches, tags and paths
	2	+
	3	+Status: ready to execute. Written to be carried out start to finish by
	4	+one agent without further questions. Independent of the other CI plans.
	5	+
	6	+## Why
	7	+
	8	+Workflows are written against GitHub's "Filter pattern cheat sheet"
	9	+(docs.github.com, Workflow syntax → Filter pattern cheat sheet). Two
	10	+matchers here disagree with it:
	11	+
	12	+1. `branches:` / `tags:` use a looser matcher.
	13	+ `GitGud.Workflows.Yaml.glob_match?/2` turns `` into `.`, so `*`
	14	+ crosses `/`, and `?` into `.`. On GitHub `branches: ['*']` matches
	15	+ `main` but not `feature/x`, and `releases/*` does not match
	16	+ `releases/1.0/hotfix`. Here they do, so workflows meant for top-level
	17	+ branches also run for every nested branch. Silent. Noted as a follow-up
	18	+ when `paths:` landed (commit 34b2c06): "changing it changes which
	19	+ existing branch filters fire" — which is exactly the point: they fire
	20	+ when they shouldn't.
	21	+2. `?` and `+` mean something else in `GitGud.Workflows.Glob` (used
	22	+ for `paths:`). The cheat sheet: `?` "matches zero or one of the
	23	+ preceding character", `+` "matches one or more of the preceding
	24	+ character" — regex quantifiers, not wildcards. `Glob` treats `?` as any
	25	+ single character and `+` as a literal. (The path-filter plan specified
	26	+ the wrong meaning; this corrects it.) `[]` matches "one alphanumeric
	27	+ character listed in the brackets or included in ranges" — ranges only
	28	+ `a-z`, `A-Z`, `0-9` — which `Glob` approximates more loosely.
	29	+3. No negative patterns in `branches:` / `tags:`. The cheat sheet
	30	+ allows `!pattern` there with the same ordered semantics as `paths:`
	31	+ (a later `!` excludes, a later positive re-includes; at least one
	32	+ positive pattern required). Here `!release/old` is a literal that
	33	+ matches nothing, so the list behaves as if the line weren't there —
	34	+ the excluded branch runs.
	35	+4. A backslash escapes a special character (`\*`, `\[`, `\!`); neither
	36	+ matcher supports it.
	37	+
	38	+## Semantics to match (the cheat sheet)
	39	+
	40	+\| pattern \| meaning \|
	41	+\|---\|---\|
	42	+\| `*` \| zero or more characters, not `/` \|
	43	+\| `**` \| zero or more of any character \|
	44	+\| `?` \| zero or one of the preceding character \|
	45	+\| `+` \| one or more of the preceding character \|
	46	+\| `[...]` \| one character from the list/ranges (`a-z`, `A-Z`, `0-9`) \|
	47	+\| `!` at the start \| negates; only in lists, evaluated in order \|
	48	+\| `\x` \| literal `x` \|
	49	+
	50	+Plus, for paths only (already implemented, keep): `**/` also matches zero
	51	+directories (`**/README.md` matches `README.md`).
	52	+
	53	+Lists (branches, branches-ignore, tags, tags-ignore, paths, paths-ignore):
	54	+a ref/file is selected if the last pattern that matches it is
	55	+positive. A list whose patterns are all negative selects nothing (GitHub
	56	+rejects it; log a warning and treat it so). `-ignore` lists don't use `!`.
	57	+
	58	+## Where things are
	59	+
	60	+\| \| \|
	61	+\|---\|---\|
	62	+\| Path patterns \| `lib/git_gud/workflows/glob.ex` (`compile/1`, `match?/2`, `translate/2`, `class/2`) \|
	63	+\| Ref patterns \| `lib/git_gud/workflows/yaml.ex` `kind_match?/5`, `glob_match?/2`; path lists `paths_match?/3`, `path_selected?/2` \|
	64	+\| Tests \| `test/git_gud/workflows/glob_test.exs`, `trigger_filters_test.exs` \|
	65	+\| Not to touch \| `GitGud.BranchProtections.matches_pattern?/2` has the same loose `` — but protection rules are security policy and narrowing them silently would unprotect* branches. Leave it; mention it in the commit message as a separate decision for the maintainer. \|
	66	+
	67	+## Steps
	68	+
	69	+### 1. One pattern engine
	70	+
	71	+In `GitGud.Workflows.Glob.translate/2`:
	72	+- `?` and `+` become regex quantifiers on the preceding atom (the
	73	+ previous literal character or class). A `?`/`+` at the start of a
	74	+ pattern, or after ``/`*`/another quantifier, is a literal (escape it).
	75	+ Track "previous atom" in the accumulator: switch `translate/2` to build
	76	+ a list of atoms (`{:lit, char}`, `{:class, body}`, `:star`, `:globstar`,
	77	+ `:globstar_slash`) and render at the end, applying quantifiers to the
	78	+ last atom.
	79	+- `[...]`: allow only alphanumerics and ranges within `a-z`/`A-Z`/`0-9`.
	80	+ The cheat sheet has no negated classes, so `[!…]`/`[^…]` — like any
	81	+ bracket containing something else — makes the `[` a literal.
	82	+- `\x` → literal `x`.
	83	+- Keep `/` (zero or more directories), `/` and bare `**`.
	84	+- Add an option `compile(pattern, mode)` with `mode` `:path` (as now) or
	85	+ `:ref`. Today the only difference is `**/` — keep the zero-directory
	86	+ rule for both (GitHub's ref examples don't contradict it); the option
	87	+ exists so the call sites say what they mean. If you find no difference
	88	+ is needed, skip the option.
	89	+
	90	+Update the moduledoc (it says `?` matches one character) and the README
	91	+line from the path-filter commit.
	92	+
	93	+### 2. Lists with negation everywhere
	94	+
	95	+Extract the ordered-list rule from `paths_match?/3` (`path_selected?/2`)
	96	+into `Glob.select?(patterns, string)` → boolean: last matching pattern
	97	+wins, `!` negates, no match → false, all-negative list → false + warning.
	98	+Use it for `paths:` (unchanged behaviour), and in `Yaml.kind_match?/5`
	99	+for `branches:` and `tags:`. For `branches-ignore:` / `tags-ignore:` /
	100	+`paths-ignore:`: selected = any pattern matches (no negation).
	101	+
	102	+### 3. Ref matching uses the engine
	103	+
	104	+Delete `Yaml.glob_match?/2`; `kind_match?/5` calls `Glob.select?/2` /
	105	+`Glob.match?/2`.
	106	+
	107	+### 4. Docs
	108	+
	109	+README CI list: one line with the pattern table above, saying `*` no
	110	+longer crosses `/` in `branches:`/`tags:` and that `!` works there.
	111	+PROGRESS: note the behaviour change (a workflow with `branches: ['*']`
	112	+stops running for `feature/x`).
	113	+
	114	+## Tests
	115	+
	116	+`glob_test.exs`:
	117	+- `?`: `colou?r` matches `color` and `colour`, not `colouur`;
	118	+- `+`: `v1+` matches `v1`, `v11`, not `v`; `a+b` vs `b`;
	119	+- leading `?`/`+` literal;
	120	+- `[0-9]`, `[CB]at`; `[!a]` literal;
	121	+- escapes: `a\b` matches `ab` only;
	122	+- existing cases still pass, except those asserting the old `?`
	123	+ meaning (`a?c` matched `abc`) — rewrite them to the new rule and say so
	124	+ in the commit message.
	125	+
	126	+`trigger_filters_test.exs`:
	127	+- `branches: ['']` matches `main`, not `feature/x`; `branches: ['*']`
	128	+ matches both;
	129	+- `branches: ['releases/', '!releases/-alpha']` excludes
	130	+ `releases/1.0-alpha`, includes `releases/1.0`;
	131	+- re-include: `['', '!b', 'beta']` → `beta` yes, `bravo` no;
	132	+- all-negative list → nothing matches;
	133	+- `tags: ['v[0-9]+.[0-9]+.[0-9]+']` matches `v10.2.33`, not `v1.x.3`;
	134	+- `branches-ignore: ['feature/*']` ignores `feature/a` but not
	135	+ `feature/a/b` (because `*` no longer crosses `/`).
	136	+
	137	+## Done when
	138	+
	139	+- `mix test` passes; no new compile warnings (compare `mix compile --force`
	140	+ warning lists before/after); `mix format` only touched files.
	141	+- One commit in the repository's message style, stating the behaviour
	142	+ change for `branches:`/`tags:` and that branch protection patterns were
	143	+ deliberately left alone.
	144	+- Not pushed.

added docs/plans/workflow-fork-pull-requests.md

+184 −0

Click to load diff…

added docs/plans/workflow-job-graph.md

+210 −0

Click to load diff…

added docs/plans/workflow-job-token.md

+160 −0

Click to load diff…

added docs/plans/workflow-matrix.md

+148 −0

Click to load diff…

added docs/plans/workflow-runner-context.md

+213 −0

Click to load diff…

added docs/plans/workflow-triggers.md

+242 −0

Click to load diff…

Parents: bfce1e7