ci(devops): downgrade actions/upload-artifact v4 → v3 (re-regression — needs ADR to prevent future re-upgrade) #557

Closed
opened 2026-05-13 12:57:25 +02:00 by marcel · 10 comments
Owner

Context

actions/upload-artifact@v4 was originally downgraded to v3 on 2026-03-19 in commits 9f3f022e and 4142c7cd. The downgrade was deliberate, documented in the commit body, and motivated by a real CI failure:

"Downgrade upload-artifact@v4 → v3 in both unit-tests and e2e-tests jobs; v4 is not supported on Gitea (GHES) and was causing jobs to report failure even when all tests passed, and prevented the Playwright browser cache from ever being saved (closes #14)" — 4142c7cd

On 2026-05-05 the action was upgraded back to v4 in commit 410b91e2 ("chore: upgrade upload-artifact action from v3 to v4"). The commit body does not reference the prior downgrade or its rationale. This is a regression — Gitea Actions (our self-hosted runner) still does not implement the v4 artifact protocol.

There are three call sites in the current main workflow tree:

.gitea/workflows/ci.yml:82                   uses: actions/upload-artifact@v4
.gitea/workflows/ci.yml:118                  uses: actions/upload-artifact@v4
.gitea/workflows/coverage-flake-probe.yml:61 uses: actions/upload-artifact@v4

All three need to come back to @v3.

Decision

  1. Pin all three references back to actions/upload-artifact@v3.
  2. Write an ADR (docs/adr/0XX-upload-artifact-v3-pin.md) that records:
    • The Gitea Actions limitation (no v4 protocol support on self-hosted runner)
    • The two prior commits where this was set up (9f3f022e, 4142c7cd) and the re-regression (410b91e2)
    • The rule: do not upgrade past v3 until Gitea publishes v4 protocol support (track upstream at https://gitea.com/gitea/act_runner)
    • How to spot the symptom: green tests, red job status, no artifact uploaded
  3. Add a CI grep guard in .gitea/workflows/ci.yml (or as a pre-test step) that fails the build if upload-artifact@v[4-9] appears in any workflow file. This is the same defence-in-depth pattern used by ADR-012 for the birpc grep guard. Without this guard, the next contributor or AI agent will silently re-introduce the regression.

Acceptance

  • All three call sites pinned to actions/upload-artifact@v3
  • New ADR exists, referenced by all three workflow files in a brief inline comment near the uses: line (# pinned per ADR-XXX — do not upgrade)
  • CI grep guard added — fails if a future commit re-introduces v4+
  • A workflow_dispatch run of coverage-flake-probe.yml succeeds end-to-end, with the artifact upload step actually producing an artifact (positive verification, not just "job didn't fail")

Non-goals

  • Not migrating the runner to a v4-compatible Gitea release — that's a separate decision and out of scope.
  • Not adding any other Gitea-specific shims — only this one regression.

References

  • 9f3f022e — original v3 pin (2026-03-19)
  • 4142c7cd — rationale committed (2026-03-19, closes #14)
  • 410b91e2 — regression upgrade back to v4 (2026-05-05)
  • ADR-012 — pattern for grep-guard defence-in-depth
## Context `actions/upload-artifact@v4` was originally **downgraded** to `v3` on 2026-03-19 in commits `9f3f022e` and `4142c7cd`. The downgrade was deliberate, documented in the commit body, and motivated by a real CI failure: > "Downgrade upload-artifact@v4 → v3 in both unit-tests and e2e-tests jobs; v4 is not supported on Gitea (GHES) and was causing jobs to report failure even when all tests passed, and prevented the Playwright browser cache from ever being saved (closes #14)" — `4142c7cd` On 2026-05-05 the action was **upgraded back to v4** in commit `410b91e2` ("chore: upgrade upload-artifact action from v3 to v4"). The commit body does not reference the prior downgrade or its rationale. This is a regression — Gitea Actions (our self-hosted runner) still does not implement the v4 artifact protocol. There are **three** call sites in the current `main` workflow tree: ``` .gitea/workflows/ci.yml:82 uses: actions/upload-artifact@v4 .gitea/workflows/ci.yml:118 uses: actions/upload-artifact@v4 .gitea/workflows/coverage-flake-probe.yml:61 uses: actions/upload-artifact@v4 ``` All three need to come back to `@v3`. ## Decision 1. **Pin all three references back to `actions/upload-artifact@v3`.** 2. **Write an ADR** (`docs/adr/0XX-upload-artifact-v3-pin.md`) that records: - The Gitea Actions limitation (no v4 protocol support on self-hosted runner) - The two prior commits where this was set up (`9f3f022e`, `4142c7cd`) and the re-regression (`410b91e2`) - The rule: **do not upgrade past v3 until Gitea publishes v4 protocol support** (track upstream at https://gitea.com/gitea/act_runner) - How to spot the symptom: green tests, red job status, no artifact uploaded 3. **Add a CI grep guard** in `.gitea/workflows/ci.yml` (or as a pre-test step) that fails the build if `upload-artifact@v[4-9]` appears in any workflow file. This is the same defence-in-depth pattern used by ADR-012 for the birpc grep guard. Without this guard, the next contributor or AI agent will silently re-introduce the regression. ## Acceptance - [ ] All three call sites pinned to `actions/upload-artifact@v3` - [ ] New ADR exists, referenced by all three workflow files in a brief inline comment near the `uses:` line (`# pinned per ADR-XXX — do not upgrade`) - [ ] CI grep guard added — fails if a future commit re-introduces v4+ - [ ] A workflow_dispatch run of `coverage-flake-probe.yml` succeeds end-to-end, with the artifact upload step actually producing an artifact (positive verification, not just "job didn't fail") ## Non-goals - Not migrating the runner to a v4-compatible Gitea release — that's a separate decision and out of scope. - Not adding any other Gitea-specific shims — only this one regression. ## References - `9f3f022e` — original v3 pin (2026-03-19) - `4142c7cd` — rationale committed (2026-03-19, closes #14) - `410b91e2` — regression upgrade back to v4 (2026-05-05) - ADR-012 — pattern for grep-guard defence-in-depth
marcel added the P0-criticalbugdevopsdocumentation labels 2026-05-13 12:57:31 +02:00
Author
Owner

Markus Keller — Senior Application Architect

Observations

  • This is a textbook re-regression: a deliberate, documented downgrade (9f3f022e + 4142c7cd) was reversed in 410b91e2 without any reference to the prior rationale. The downgrade was never captured as an ADR, so a contributor reading only the workflow file sees a "stale-looking v3" with no breadcrumb back to the reason. That is an institutional-memory failure, not a contributor failure.
  • The pattern matches ADR-012's defence-in-depth approach (synchronous-factory grep guard) almost line-for-line. The same shape will work here. The grep guard is more important than the ADR because the ADR convinces a careful reader; the guard stops a careless one.
  • docs/infrastructure/ci-gitea.md lines 203, 230, 332 carry the comment # ← upgraded from v3. That doc actively misled the May 5th upgrade — it reads as if v4 is the canonical target. Pinning the workflows back to v3 while leaving this doc unchanged guarantees re-regression #3. The doc fix is in scope.
  • I count two ADRs already numbered 012 in docs/adr/ (012-browser-test-mocking-strategy.md and 012-nsenter-for-host-service-management-in-ci.md). The duplicate-ID hazard is real — the new ADR must be 013-upload-artifact-v3-pin.md, not "0XX" as the issue body says.

Recommendations

  • ADR number is 013, not "0XX". Title: 013-upload-artifact-v3-pin.md. Follow the existing ADR template (Context / Decision / Consequences / Alternatives). State the upstream tracking issue explicitly so the reader knows what condition would license a re-upgrade.
  • Inline comment near each uses: line should be load-bearing, not decorative. Use the exact phrasing: # Gitea Actions does not implement upload-artifact v4 protocol — pinned per ADR-013. Do NOT upgrade. See #557. The issue number in the comment closes the loop; the next agent searches Gitea for context and finds the full story.
  • Fix docs/infrastructure/ci-gitea.md in the same PR. Replace # ← upgraded from v3 with # pinned at v3 per ADR-013 — Gitea Actions limitation. The doc and the workflow must agree, or one of them will mislead the next change.
  • Treat the grep guard as a workflow-level architectural invariant. It belongs in the same ci.yml step that already enforces the ADR-012 birpc invariant. That section is becoming a "repo invariants" block — name it that way in a comment so the pattern is visible.

Open Decisions

  • None. The ADR number, the inline comment phrasing, the doc fix scope, and the guard placement are all determined by existing patterns in the repo. This is a straightforward execution.
## Markus Keller — Senior Application Architect ### Observations - This is a textbook re-regression: a deliberate, documented downgrade (`9f3f022e` + `4142c7cd`) was reversed in `410b91e2` without any reference to the prior rationale. The downgrade was never captured as an ADR, so a contributor reading only the workflow file sees a "stale-looking v3" with no breadcrumb back to the reason. That is an institutional-memory failure, not a contributor failure. - The pattern matches ADR-012's defence-in-depth approach (synchronous-factory grep guard) almost line-for-line. The same shape will work here. The grep guard is *more* important than the ADR because the ADR convinces a careful reader; the guard stops a careless one. - `docs/infrastructure/ci-gitea.md` lines 203, 230, 332 carry the comment `# ← upgraded from v3`. That doc actively *misled* the May 5th upgrade — it reads as if v4 is the canonical target. Pinning the workflows back to v3 while leaving this doc unchanged guarantees re-regression #3. The doc fix is in scope. - I count two ADRs already numbered `012` in `docs/adr/` (`012-browser-test-mocking-strategy.md` and `012-nsenter-for-host-service-management-in-ci.md`). The duplicate-ID hazard is real — the new ADR must be `013-upload-artifact-v3-pin.md`, not "0XX" as the issue body says. ### Recommendations - **ADR number is 013, not "0XX".** Title: `013-upload-artifact-v3-pin.md`. Follow the existing ADR template (Context / Decision / Consequences / Alternatives). State the upstream tracking issue explicitly so the reader knows what condition would license a re-upgrade. - **Inline comment near each `uses:` line should be load-bearing, not decorative.** Use the exact phrasing: `# Gitea Actions does not implement upload-artifact v4 protocol — pinned per ADR-013. Do NOT upgrade. See #557.` The issue number in the comment closes the loop; the next agent searches Gitea for context and finds the full story. - **Fix `docs/infrastructure/ci-gitea.md` in the same PR.** Replace `# ← upgraded from v3` with `# pinned at v3 per ADR-013 — Gitea Actions limitation`. The doc and the workflow must agree, or one of them will mislead the next change. - **Treat the grep guard as a workflow-level architectural invariant.** It belongs in the same `ci.yml` step that already enforces the ADR-012 birpc invariant. That section is becoming a "repo invariants" block — name it that way in a comment so the pattern is visible. ### Open Decisions - None. The ADR number, the inline comment phrasing, the doc fix scope, and the guard placement are all determined by existing patterns in the repo. This is a straightforward execution.
Author
Owner

Felix Brandt — Senior Fullstack Developer

Observations

  • Three call sites, two files: .gitea/workflows/ci.yml:82, :118, and .gitea/workflows/coverage-flake-probe.yml:61. I confirmed via grep -rn "upload-artifact" .gitea/ — no other usages exist. The issue's scope is accurate.
  • The grep guard belongs alongside the existing "Assert no banned vi.mock patterns" step in ci.yml (lines 39–57). That block already enforces an ADR-012 invariant the same way — the pattern is established, and putting a second invariant check 200 lines away would split the convention.
  • coverage-flake-probe.yml:60-64 only uploads the coverage log if: failure(). The acceptance criterion "the artifact upload step actually producing an artifact (positive verification, not just 'job didn't fail')" cannot be satisfied by that step as written — a successful run uploads nothing. We need either a second if: success() upload, or a manual workflow_dispatch on a run that we expect to fail (e.g. by injecting a known birpc-trigger). Cleaner: change the existing condition to if: always() so every run produces an artifact and the v3 path is exercised on every probe invocation.
  • ADRs in this repo follow a ## Context / ## Decision / ## Consequences / ## Alternatives structure (see 012-browser-test-mocking-strategy.md). The new ADR should follow the same template, not invent a new shape.

Recommendations

  • Write the grep guard with a failing test first. Concretely: stash a copy of ci.yml, run the guard locally against the upgraded version, watch it FAIL with the expected message, then commit the guard. This is the red/green discipline for shell-script-as-test. Without the red step, we have no evidence the regex actually catches @v4, @v5, @v4.1, @v4.0.0, etc. Suggested pattern:
- name: Assert no upload-artifact past v3
  shell: bash
  run: |
    if grep -rnE 'actions/upload-artifact@v[4-9]([.0-9]*)?' .gitea/workflows/; then
      echo "FAIL: actions/upload-artifact pinned past v3 — Gitea Actions does not implement the v4 protocol. See ADR-013 / #557."
      exit 1
    fi

The [4-9]([.0-9]*)? form catches @v4, @v4.1.0, @v9, etc., without false-positiving on @v3.0.0.

  • Change the coverage-flake-probe.yml upload condition from if: failure() to if: always(). Otherwise the acceptance criterion "workflow_dispatch run succeeds end-to-end with the artifact upload step actually producing an artifact" cannot be verified — successful runs never upload. With if: always(), the v3 upload step is exercised on every matrix cell on every run, which is the positive verification the issue asks for.
  • Pair the workflow change with a tiny unit test of the grep regex itself. A 5-line bash script in infra/ that runs the same regex against three positive samples (@v4, @v4.0.1, @v5) and three negative samples (@v3, @v3.0.1, @v3-rc1) and asserts the expected outcomes. This is the regression test for the guard itself — without it, someone "fixes" the regex and silently widens the gap.
  • Commit ordering: (1) failing test of the grep (red), (2) add the guard (still red — workflows are at v4), (3) downgrade the three call sites (green), (4) ADR + inline comments (no test impact), (5) doc fix.

Open Decisions

  • None — the technical shape is determined by ADR-012's precedent and the existing test-discipline rules.
## Felix Brandt — Senior Fullstack Developer ### Observations - Three call sites, two files: `.gitea/workflows/ci.yml:82`, `:118`, and `.gitea/workflows/coverage-flake-probe.yml:61`. I confirmed via `grep -rn "upload-artifact" .gitea/` — no other usages exist. The issue's scope is accurate. - The grep guard belongs alongside the existing "Assert no banned vi.mock patterns" step in `ci.yml` (lines 39–57). That block already enforces an ADR-012 invariant the same way — the pattern is established, and putting a second invariant check 200 lines away would split the convention. - `coverage-flake-probe.yml:60-64` only uploads the coverage log `if: failure()`. The acceptance criterion "the artifact upload step actually producing an artifact (positive verification, not just 'job didn't fail')" cannot be satisfied by that step as written — a successful run uploads nothing. We need either a second `if: success()` upload, or a manual workflow_dispatch on a run that we expect to fail (e.g. by injecting a known birpc-trigger). Cleaner: change the existing condition to `if: always()` so every run produces an artifact and the v3 path is exercised on every probe invocation. - ADRs in this repo follow a `## Context / ## Decision / ## Consequences / ## Alternatives` structure (see `012-browser-test-mocking-strategy.md`). The new ADR should follow the same template, not invent a new shape. ### Recommendations - **Write the grep guard with a failing test first.** Concretely: stash a copy of `ci.yml`, run the guard locally against the upgraded version, watch it FAIL with the expected message, then commit the guard. This is the red/green discipline for shell-script-as-test. Without the red step, we have no evidence the regex actually catches `@v4`, `@v5`, `@v4.1`, `@v4.0.0`, etc. Suggested pattern: ```bash - name: Assert no upload-artifact past v3 shell: bash run: | if grep -rnE 'actions/upload-artifact@v[4-9]([.0-9]*)?' .gitea/workflows/; then echo "FAIL: actions/upload-artifact pinned past v3 — Gitea Actions does not implement the v4 protocol. See ADR-013 / #557." exit 1 fi ``` The `[4-9]([.0-9]*)?` form catches `@v4`, `@v4.1.0`, `@v9`, etc., without false-positiving on `@v3.0.0`. - **Change the `coverage-flake-probe.yml` upload condition from `if: failure()` to `if: always()`.** Otherwise the acceptance criterion "workflow_dispatch run succeeds end-to-end with the artifact upload step actually producing an artifact" cannot be verified — successful runs never upload. With `if: always()`, the v3 upload step is exercised on every matrix cell on every run, which is the positive verification the issue asks for. - **Pair the workflow change with a tiny unit test of the grep regex itself.** A 5-line bash script in `infra/` that runs the same regex against three positive samples (`@v4`, `@v4.0.1`, `@v5`) and three negative samples (`@v3`, `@v3.0.1`, `@v3-rc1`) and asserts the expected outcomes. This is the regression test for the guard itself — without it, someone "fixes" the regex and silently widens the gap. - **Commit ordering:** (1) failing test of the grep (red), (2) add the guard (still red — workflows are at v4), (3) downgrade the three call sites (green), (4) ADR + inline comments (no test impact), (5) doc fix. ### Open Decisions - None — the technical shape is determined by ADR-012's precedent and the existing test-discipline rules.
Author
Owner

Tobias Wendt — DevOps & Platform Engineer

Observations

  • This is the third bite at the apple. The downgrade happened on 2026-03-19 (9f3f022e + 4142c7cd, "closes #14"), the upgrade on 2026-05-05 (410b91e2), and we are now reversing the upgrade on 2026-05-13. Eight weeks per round trip. Without a guard, the next round trip lands by mid-July.
  • The persona advice I usually give ("use @v4, deprecated actions accumulate vulnerabilities") is wrong for this runner. Gitea Actions runs on act_runner, which does not implement the v4 artifact protocol — the v4 client uploads via a GitHub-specific API that act_runner has not shipped. Pinning to @v3 here is the correct, documented decision, not technical debt. The ADR needs to say this loudly so the next contributor (and the next AI) does not "fix" it.
  • Gitea Actions is the project's CI substrate per ADR-011 (single-tenant gitea-runner). That ADR establishes the constraint; ADR-013 inherits it. Cross-reference between them in the new ADR — readers should be able to navigate the constraint chain.
  • The symptom described in the issue body ("green tests, red job status, no artifact uploaded") is the exact failure shape I would expect — act_runner returns a non-zero exit code from the v4 client when the upload API responds with an unsupported-route error, even though the test suite already completed. Hard to diagnose because the build log shows green test output right above a job-level failure. Worth calling this out in the ADR's "How to spot the symptom" section verbatim.
  • Tracking link: the issue references https://gitea.com/gitea/act_runner for upstream v4 support. There is currently no merged PR for v4 artifact protocol in act_runner — pinning at v3 is a multi-quarter decision, not a multi-week one.

Recommendations

  • Pin to actions/upload-artifact@v3 exactly — not @v3.x.y, not @main. The v3 major track is in maintenance mode; minor pinning would just add Renovate noise. Major-only pin matches the rest of the repo's convention.
  • Bypass Renovate for this action. Add to renovate.json (or whatever config file exists):
"packageRules": [
  {
    "matchManagers": ["github-actions"],
    "matchPackageNames": ["actions/upload-artifact"],
    "enabled": false,
    "description": "Pinned at v3 per ADR-013 — Gitea Actions does not implement v4 protocol"
  }
]

Without this, Renovate will open a PR to upgrade to v4 every quarter and one of them will eventually merge. The bypass + grep guard is belt-and-braces.

  • Add a one-line check to the ADR's "How we'll know we can upgrade" section: when gitea/act_runner ships v4 protocol support, re-evaluate. Link to the upstream tracker (https://gitea.com/gitea/act_runner/issues filtered for "artifact v4" if such an issue exists; if not, this is a candidate to file). Without an explicit upgrade trigger, the pin stays forever even after it becomes obsolete.
  • Verify the grep guard runs against coverage-flake-probe.yml, not just ci.yml. A guard that only scans .gitea/workflows/ci.yml will miss the third call site. The guard's grep target must be .gitea/workflows/ (the directory), not a single file.
  • Cost-aware note: all four uploads (coverage-reports, unit-test-screenshots, e2e-results, the probe log) are <100MB each and live in Gitea's local storage — no infrastructure cost impact from the downgrade.

Open Decisions

  • None — @v3 major pin, Renovate bypass, ADR-013, and upstream tracker are the standard playbook for this exact situation.
## Tobias Wendt — DevOps & Platform Engineer ### Observations - This is the third bite at the apple. The downgrade happened on 2026-03-19 (`9f3f022e` + `4142c7cd`, "closes #14"), the upgrade on 2026-05-05 (`410b91e2`), and we are now reversing the upgrade on 2026-05-13. Eight weeks per round trip. Without a guard, the next round trip lands by mid-July. - The persona advice I usually give ("use `@v4`, deprecated actions accumulate vulnerabilities") is wrong for *this* runner. Gitea Actions runs on `act_runner`, which does not implement the v4 artifact protocol — the v4 client uploads via a GitHub-specific API that act_runner has not shipped. Pinning to `@v3` here is the correct, documented decision, not technical debt. The ADR needs to say this loudly so the next contributor (and the next AI) does not "fix" it. - Gitea Actions is the project's CI substrate per ADR-011 (single-tenant gitea-runner). That ADR establishes the constraint; ADR-013 inherits it. Cross-reference between them in the new ADR — readers should be able to navigate the constraint chain. - The symptom described in the issue body ("green tests, red job status, no artifact uploaded") is the exact failure shape I would expect — `act_runner` returns a non-zero exit code from the v4 client when the upload API responds with an unsupported-route error, even though the test suite already completed. Hard to diagnose because the build log shows green test output right above a job-level failure. Worth calling this out in the ADR's "How to spot the symptom" section verbatim. - Tracking link: the issue references `https://gitea.com/gitea/act_runner` for upstream v4 support. There is currently no merged PR for v4 artifact protocol in act_runner — pinning at v3 is a multi-quarter decision, not a multi-week one. ### Recommendations - **Pin to `actions/upload-artifact@v3` exactly — not `@v3.x.y`, not `@main`.** The v3 major track is in maintenance mode; minor pinning would just add Renovate noise. Major-only pin matches the rest of the repo's convention. - **Bypass Renovate for this action.** Add to `renovate.json` (or whatever config file exists): ```json "packageRules": [ { "matchManagers": ["github-actions"], "matchPackageNames": ["actions/upload-artifact"], "enabled": false, "description": "Pinned at v3 per ADR-013 — Gitea Actions does not implement v4 protocol" } ] ``` Without this, Renovate will open a PR to upgrade to v4 every quarter and one of them will eventually merge. The bypass + grep guard is belt-and-braces. - **Add a one-line check to the ADR's "How we'll know we can upgrade" section:** when `gitea/act_runner` ships v4 protocol support, re-evaluate. Link to the upstream tracker (`https://gitea.com/gitea/act_runner/issues` filtered for "artifact v4" if such an issue exists; if not, this is a candidate to file). Without an explicit upgrade trigger, the pin stays forever even after it becomes obsolete. - **Verify the grep guard runs against `coverage-flake-probe.yml`, not just `ci.yml`.** A guard that only scans `.gitea/workflows/ci.yml` will miss the third call site. The guard's grep target must be `.gitea/workflows/` (the directory), not a single file. - **Cost-aware note:** all four uploads (`coverage-reports`, `unit-test-screenshots`, `e2e-results`, the probe log) are <100MB each and live in Gitea's local storage — no infrastructure cost impact from the downgrade. ### Open Decisions - None — `@v3` major pin, Renovate bypass, ADR-013, and upstream tracker are the standard playbook for this exact situation.
Author
Owner

Sara Holt — QA Engineer & Test Strategist

Observations

  • The issue's acceptance list ends with: "A workflow_dispatch run of coverage-flake-probe.yml succeeds end-to-end, with the artifact upload step actually producing an artifact (positive verification, not just 'job didn't fail')". This is the right instinct — but the workflow as written (.gitea/workflows/coverage-flake-probe.yml:60) uploads only on if: failure(). A successful run produces zero artifacts. As written, the acceptance criterion is unsatisfiable.
  • The grep guard is itself a test asset. Without a test for the guard, we have no evidence it catches the regression it claims to catch. The ADR-012 birpc-guard precedent in ci.yml:39-57 does not have its own test either — that is a debt I would flag separately, but this issue is the right time to set the new pattern.
  • The "green tests, red job status, no artifact uploaded" failure mode is invisible at the test-suite layer — vitest and surefire both reported green; the failure was in the upload step. This is a CI-infrastructure regression, not an application regression. QA coverage gap: we have no automated check that the upload step succeeded on a previous CI run, only that the build was green.

Recommendations

  • Change coverage-flake-probe.yml:60 from if: failure() to if: always(). This is the only way to satisfy the acceptance criterion as written. With if: always(), the v3 upload runs on every matrix cell on every probe invocation, producing 20 artifacts per run. The probe's purpose is to detect the birpc race — but it's also our positive-verification surface for the upload action itself. Both invariants now run together.
  • Add a positive-verification acceptance step to the issue, separate from the workflow_dispatch run. Concretely: after the workflow_dispatch run, manually navigate to the run's Actions page on Gitea and confirm that the "Artifacts" section lists at least one item. Without that human step, "job didn't fail" is the only signal — and we know that signal is unreliable for this exact class of bug. Document this verification step in the PR description.
  • Test the grep regex independently. Six-test table in a bash file:
# infra/test/test-upload-artifact-grep.sh
set -e
REGEX='actions/upload-artifact@v[4-9]([.0-9]*)?'
echo 'uses: actions/upload-artifact@v4'      | grep -qE "$REGEX"
echo 'uses: actions/upload-artifact@v4.0.1'  | grep -qE "$REGEX"
echo 'uses: actions/upload-artifact@v9'      | grep -qE "$REGEX"
echo 'uses: actions/upload-artifact@v3'      | grep -qvE "$REGEX"
echo 'uses: actions/upload-artifact@v3.0.1'  | grep -qvE "$REGEX"
echo 'uses: actions/download-artifact@v4'    | grep -qvE "$REGEX"  # download, not upload
echo "PASS"

Wire this into the same CI step (or as a pre-commit hook) so the guard's regex is itself under test. Closes the recursion.

  • Add a download-artifact clarification to the guard. The current scope is upload-artifact. actions/download-artifact@v4 has the same protocol incompatibility. Confirm via grep whether any workflow uses it (I see none today), and either extend the guard to cover both, or note explicitly in the ADR that the guard is upload-only because download is not currently used. Silence on this point is a known-known we should not leave unaddressed.
  • Reuse the test pyramid mindset: the grep guard is a static check (cheap, fast, runs first). The workflow_dispatch verification is an integration check (slow, runs on demand). Both are needed; neither replaces the other. The issue body asks for both but doesn't name them as separate layers — naming helps.

Open Decisions

  • Extend the guard to cover download-artifact, or scope strictly to upload-artifact? Cost of extending: 5 chars in the regex ((upload|download)-artifact). Cost of not extending: an actions/download-artifact@v4 lands in a future PR and silently fails for the same reason, and we file issue #689. (I'd extend it — the regex change is trivial and the failure mode is identical, but flagging because the issue scoped it strictly to upload.)
## Sara Holt — QA Engineer & Test Strategist ### Observations - The issue's acceptance list ends with: "A workflow_dispatch run of `coverage-flake-probe.yml` succeeds end-to-end, with the artifact upload step actually producing an artifact (positive verification, not just 'job didn't fail')". This is the right instinct — but the workflow as written (`.gitea/workflows/coverage-flake-probe.yml:60`) uploads only on `if: failure()`. A successful run produces *zero* artifacts. As written, the acceptance criterion is unsatisfiable. - The grep guard is itself a test asset. Without a test for the guard, we have no evidence it catches the regression it claims to catch. The ADR-012 birpc-guard precedent in `ci.yml:39-57` does not have its own test either — that is a debt I would flag separately, but this issue is the right time to set the new pattern. - The "green tests, red job status, no artifact uploaded" failure mode is invisible at the test-suite layer — vitest and surefire both reported green; the failure was in the upload step. This is a CI-infrastructure regression, not an application regression. QA coverage gap: we have no automated check that the upload step *succeeded* on a previous CI run, only that the build was green. ### Recommendations - **Change `coverage-flake-probe.yml:60` from `if: failure()` to `if: always()`.** This is the only way to satisfy the acceptance criterion as written. With `if: always()`, the v3 upload runs on every matrix cell on every probe invocation, producing 20 artifacts per run. The probe's *purpose* is to detect the birpc race — but it's also our positive-verification surface for the upload action itself. Both invariants now run together. - **Add a positive-verification acceptance step to the issue, separate from the workflow_dispatch run.** Concretely: after the workflow_dispatch run, manually navigate to the run's Actions page on Gitea and confirm that the "Artifacts" section lists at least one item. Without that human step, "job didn't fail" is the only signal — and we know that signal is unreliable for this exact class of bug. Document this verification step in the PR description. - **Test the grep regex independently.** Six-test table in a bash file: ```bash # infra/test/test-upload-artifact-grep.sh set -e REGEX='actions/upload-artifact@v[4-9]([.0-9]*)?' echo 'uses: actions/upload-artifact@v4' | grep -qE "$REGEX" echo 'uses: actions/upload-artifact@v4.0.1' | grep -qE "$REGEX" echo 'uses: actions/upload-artifact@v9' | grep -qE "$REGEX" echo 'uses: actions/upload-artifact@v3' | grep -qvE "$REGEX" echo 'uses: actions/upload-artifact@v3.0.1' | grep -qvE "$REGEX" echo 'uses: actions/download-artifact@v4' | grep -qvE "$REGEX" # download, not upload echo "PASS" ``` Wire this into the same CI step (or as a `pre-commit` hook) so the guard's regex is itself under test. Closes the recursion. - **Add a `download-artifact` clarification to the guard.** The current scope is `upload-artifact`. `actions/download-artifact@v4` has the *same* protocol incompatibility. Confirm via grep whether any workflow uses it (I see none today), and either extend the guard to cover both, or note explicitly in the ADR that the guard is upload-only because download is not currently used. Silence on this point is a known-known we should not leave unaddressed. - **Reuse the test pyramid mindset:** the grep guard is a static check (cheap, fast, runs first). The workflow_dispatch verification is an integration check (slow, runs on demand). Both are needed; neither replaces the other. The issue body asks for both but doesn't name them as separate layers — naming helps. ### Open Decisions - **Extend the guard to cover `download-artifact`, or scope strictly to `upload-artifact`?** Cost of extending: 5 chars in the regex (`(upload|download)-artifact`). Cost of not extending: an `actions/download-artifact@v4` lands in a future PR and silently fails for the same reason, and we file issue #689. _(I'd extend it — the regex change is trivial and the failure mode is identical, but flagging because the issue scoped it strictly to upload.)_
Author
Owner

Nora "NullX" Steiner — Application Security Engineer

Observations

  • actions/upload-artifact@v3 is in maintenance mode as of GitHub's 2024 retirement notice, but it has not been removed and is still resolvable from the marketplace. It does not introduce a known CVE on its own — the 2024 npm-side advisories on v3 were resolved within the action's own deps. Downgrading is not a security regression. I would block this if it were, but the threat surface is unchanged.
  • The real security smell here is invisibility: a workflow change that was reverted with no record. If a malicious contributor wanted to introduce a CI step that silently exfiltrated artifacts, the same review-blind-spot would allow it. The ADR + grep guard pattern improves the audit posture for any future CI change, not just this one.
  • The proposed grep guard runs against .gitea/workflows/ — that path is itself a privileged surface. A PR that modifies ci.yml to disable the guard step would defeat the guard in the same PR that re-introduces v4. Defence-in-depth says: the guard alone is not enough; the guard plus a CODEOWNERS or branch-protection rule on .gitea/workflows/ is.
  • Pinning @v3 (major-only) keeps us on the latest patches of v3 — anything else would freeze us on a literal commit SHA. SHA pinning would be more secure (resistant to action repo compromise), but it costs Renovate noise. For a self-hosted, low-attack-surface Gitea instance with one trusted contributor, major-pin is the right balance. Worth noting in the ADR so the next reviewer doesn't "harden" it into a SHA pin without thinking about the operational cost.

Recommendations

  • Pin to @v3 (major), not a SHA. Document the trade-off explicitly in the ADR's Alternatives section: "SHA-pinning rejected — adds Renovate update friction; threat model (self-hosted single-tenant Gitea) does not justify it." A reader who later wants to harden this knows the decision was deliberate.
  • Add a CODEOWNERS entry for .gitea/workflows/ if one does not already exist. A required-review rule on workflow files is the second layer of defence — the grep guard catches the regression, CODEOWNERS catches the guard's own removal. This is the same pattern as protecting SecurityConfig.java. (I checked: marcel is the sole maintainer, so this is more of a forcing function for AI-assisted PRs than a multi-reviewer gate, but it still creates a checkpoint.)
  • No CVE check required for the downgrade itself. actions/upload-artifact@v3 is unmaintained but not vulnerable. If GitHub publishes a v3-specific advisory in the future, the ADR's "How we'll know we can upgrade" section becomes "or until v3 acquires an unpatched CVE" — add that escape hatch now so future security incidents have a clear procedure.
  • Log review: confirm no artifact contains secrets. A misconfigured upload could include .env, target/classes/application.properties, or a Maven settings file. I checked the three current uploads — frontend/coverage/, /tmp/coverage-test-*.log, frontend/test-results/screenshots/, backend/target/surefire-reports/ — none look risky, but reconfirming during this PR is cheap.

Open Decisions

  • Add CODEOWNERS on .gitea/workflows/ now, or scope it to a separate issue? Cost of adding now: 1 line in CODEOWNERS, zero behaviour change for a solo maintainer, valuable forcing-function for AI agents. Cost of deferring: a future PR that disables the guard has no second checkpoint. (I'd add the line in this PR, but it is genuinely out of the issue's scope and would warrant its own commit.)
## Nora "NullX" Steiner — Application Security Engineer ### Observations - `actions/upload-artifact@v3` is in maintenance mode as of GitHub's 2024 retirement notice, but it has not been removed and is still resolvable from the marketplace. It does not introduce a known CVE on its own — the 2024 npm-side advisories on v3 were resolved within the action's own deps. Downgrading is not a security regression. I would block this if it were, but the threat surface is unchanged. - The real security smell here is *invisibility*: a workflow change that was reverted with no record. If a malicious contributor wanted to introduce a CI step that silently exfiltrated artifacts, the same review-blind-spot would allow it. The ADR + grep guard pattern improves the audit posture for *any* future CI change, not just this one. - The proposed grep guard runs against `.gitea/workflows/` — that path is itself a privileged surface. A PR that modifies `ci.yml` to disable the guard step would defeat the guard in the same PR that re-introduces v4. Defence-in-depth says: the guard alone is not enough; the guard plus a `CODEOWNERS` or branch-protection rule on `.gitea/workflows/` is. - Pinning `@v3` (major-only) keeps us on the latest patches of v3 — anything else would freeze us on a literal commit SHA. SHA pinning would be more secure (resistant to action repo compromise), but it costs Renovate noise. For a self-hosted, low-attack-surface Gitea instance with one trusted contributor, major-pin is the right balance. Worth noting in the ADR so the next reviewer doesn't "harden" it into a SHA pin without thinking about the operational cost. ### Recommendations - **Pin to `@v3` (major), not a SHA.** Document the trade-off explicitly in the ADR's Alternatives section: "SHA-pinning rejected — adds Renovate update friction; threat model (self-hosted single-tenant Gitea) does not justify it." A reader who later wants to harden this knows the decision was deliberate. - **Add a `CODEOWNERS` entry for `.gitea/workflows/` if one does not already exist.** A required-review rule on workflow files is the second layer of defence — the grep guard catches the regression, CODEOWNERS catches the guard's own removal. This is the same pattern as protecting `SecurityConfig.java`. (I checked: `marcel` is the sole maintainer, so this is more of a forcing function for AI-assisted PRs than a multi-reviewer gate, but it still creates a checkpoint.) - **No CVE check required for the downgrade itself.** `actions/upload-artifact@v3` is unmaintained but not vulnerable. If GitHub publishes a v3-specific advisory in the future, the ADR's "How we'll know we can upgrade" section becomes "or until v3 acquires an unpatched CVE" — add that escape hatch now so future security incidents have a clear procedure. - **Log review:** confirm no artifact contains secrets. A misconfigured upload could include `.env`, `target/classes/application.properties`, or a Maven settings file. I checked the three current uploads — `frontend/coverage/`, `/tmp/coverage-test-*.log`, `frontend/test-results/screenshots/`, `backend/target/surefire-reports/` — none look risky, but reconfirming during this PR is cheap. ### Open Decisions - **Add `CODEOWNERS` on `.gitea/workflows/` now, or scope it to a separate issue?** Cost of adding now: 1 line in `CODEOWNERS`, zero behaviour change for a solo maintainer, valuable forcing-function for AI agents. Cost of deferring: a future PR that disables the guard has no second checkpoint. _(I'd add the line in this PR, but it is genuinely out of the issue's scope and would warrant its own commit.)_
Author
Owner

Elicit — Senior Requirements Engineer

Observations

  • The issue is well-framed: clear context, decision, three concrete acceptance criteria, and explicit non-goals. This is the spec-dense format the project's working memory calls for. Two of the three acceptance criteria are precise and testable. One is not.
  • AC-1 ("All three call sites pinned to actions/upload-artifact@v3") — testable, binary, automatable. Solid.
  • AC-2 ("New ADR exists, referenced by all three workflow files in a brief inline comment") — testable. The "near the uses: line" phrasing is slightly loose; I'd tighten to "on the line immediately preceding uses: actions/upload-artifact@v3."
  • AC-3 ("CI grep guard added — fails if a future commit re-introduces v4+") — partially testable. "Fails on v4+" is ambiguous: does the guard catch @v4.1, @v4-rc1, @v5, @main? Without a test fixture, this is qualitative. Felix and Sara have already proposed concrete fixtures — fold them into the AC.
  • AC-4 ("A workflow_dispatch run of coverage-flake-probe.yml succeeds end-to-end, with the artifact upload step actually producing an artifact") — currently unsatisfiable given the workflow uploads only on if: failure(). Either the workflow must change, or the AC must change. Both are valid, but right now the issue contradicts itself.
  • Non-goal clarity is good. "Not migrating the runner to a v4-compatible Gitea release" explicitly parks the bigger conversation. That keeps this issue small.
  • Missing acceptance criterion: the issue says "do not upgrade past v3 until Gitea publishes v4 protocol support" but no acceptance criterion verifies that this rule is findable. The grep guard makes it enforceable, but a contributor reading the workflow file in passing still needs the inline comment to be informative.

Recommendations

  • Rewrite AC-3 with explicit fixtures:

The grep guard MUST fail when any workflow file contains actions/upload-artifact@v4, actions/upload-artifact@v4.0.1, or actions/upload-artifact@v9. The guard MUST NOT fail on actions/upload-artifact@v3 or actions/upload-artifact@v3.0.1. A test fixture in infra/test/ MUST verify both directions.

  • Resolve AC-4's contradiction. Either:

    • Option A (preferred): change coverage-flake-probe.yml:60 from if: failure() to if: always(), then AC-4 is satisfiable on any successful run.
    • Option B: rewrite AC-4 as "After the workflow_dispatch run completes (whether the suite passes or fails), the run's Actions page lists at least one artifact." This works without changing the workflow but requires manual verification on every probe.

    Option A is preferable because it makes the verification automatic and continuous.

  • Add a new AC-5 covering the misleading documentation:

docs/infrastructure/ci-gitea.md lines 203, 230, and 332 MUST NOT carry the comment # ← upgraded from v3 (which currently misrepresents the pinned state) and MUST be updated to reference ADR-013.

Without this, the doc continues to drive re-regressions even after the workflow is fixed. Markus already raised this; promoting it to a formal AC closes the gap.

  • Replace "0XX" with the actual ADR number in the issue body. Markus identified that there are already two ADR-012 files in docs/adr/. The new ADR is 013-upload-artifact-v3-pin.md. Update the issue body so implementers don't have to re-derive this.

  • Add a Definition of Done note: every workflow change MUST be followed by a manual workflow_dispatch run before the PR merges. This is the only way to catch CI-config bugs that the suite itself cannot detect. Should become a project-wide DoD rule, but flagging here as relevant to this issue.

Open Decisions

  • AC-4 resolution (Option A vs Option B above): functionally equivalent, but Option A bundles a small if: change into this issue while Option B keeps the workflow file unchanged. (I'd take Option A for the continuous-verification benefit.)
## Elicit — Senior Requirements Engineer ### Observations - The issue is well-framed: clear context, decision, three concrete acceptance criteria, and explicit non-goals. This is the spec-dense format the project's working memory calls for. Two of the three acceptance criteria are precise and testable. One is not. - **AC-1 ("All three call sites pinned to `actions/upload-artifact@v3`")** — testable, binary, automatable. Solid. - **AC-2 ("New ADR exists, referenced by all three workflow files in a brief inline comment")** — testable. The "near the `uses:` line" phrasing is slightly loose; I'd tighten to "on the line immediately preceding `uses: actions/upload-artifact@v3`." - **AC-3 ("CI grep guard added — fails if a future commit re-introduces v4+")** — partially testable. "Fails on v4+" is ambiguous: does the guard catch `@v4.1`, `@v4-rc1`, `@v5`, `@main`? Without a test fixture, this is qualitative. Felix and Sara have already proposed concrete fixtures — fold them into the AC. - **AC-4 ("A workflow_dispatch run of `coverage-flake-probe.yml` succeeds end-to-end, with the artifact upload step actually producing an artifact")** — currently *unsatisfiable* given the workflow uploads only on `if: failure()`. Either the workflow must change, or the AC must change. Both are valid, but right now the issue contradicts itself. - **Non-goal clarity is good.** "Not migrating the runner to a v4-compatible Gitea release" explicitly parks the bigger conversation. That keeps this issue small. - **Missing acceptance criterion:** the issue says "do not upgrade past v3 until Gitea publishes v4 protocol support" but no acceptance criterion verifies that this rule is *findable*. The grep guard makes it enforceable, but a contributor reading the workflow file in passing still needs the inline comment to be informative. ### Recommendations - **Rewrite AC-3 with explicit fixtures:** > The grep guard MUST fail when any workflow file contains `actions/upload-artifact@v4`, `actions/upload-artifact@v4.0.1`, or `actions/upload-artifact@v9`. The guard MUST NOT fail on `actions/upload-artifact@v3` or `actions/upload-artifact@v3.0.1`. A test fixture in `infra/test/` MUST verify both directions. - **Resolve AC-4's contradiction.** Either: - **Option A** (preferred): change `coverage-flake-probe.yml:60` from `if: failure()` to `if: always()`, then AC-4 is satisfiable on any successful run. - **Option B**: rewrite AC-4 as "After the workflow_dispatch run completes (whether the suite passes or fails), the run's Actions page lists at least one artifact." This works without changing the workflow but requires manual verification on every probe. Option A is preferable because it makes the verification automatic and continuous. - **Add a new AC-5 covering the misleading documentation:** > `docs/infrastructure/ci-gitea.md` lines 203, 230, and 332 MUST NOT carry the comment `# ← upgraded from v3` (which currently misrepresents the pinned state) and MUST be updated to reference ADR-013. Without this, the doc continues to drive re-regressions even after the workflow is fixed. Markus already raised this; promoting it to a formal AC closes the gap. - **Replace "0XX" with the actual ADR number in the issue body.** Markus identified that there are already two ADR-012 files in `docs/adr/`. The new ADR is `013-upload-artifact-v3-pin.md`. Update the issue body so implementers don't have to re-derive this. - **Add a Definition of Done note:** every workflow change MUST be followed by a manual workflow_dispatch run before the PR merges. This is the only way to catch CI-config bugs that the suite itself cannot detect. Should become a project-wide DoD rule, but flagging here as relevant to this issue. ### Open Decisions - **AC-4 resolution (Option A vs Option B above):** functionally equivalent, but Option A bundles a small `if:` change into this issue while Option B keeps the workflow file unchanged. _(I'd take Option A for the continuous-verification benefit.)_
Author
Owner

Leonie Voss — UX Designer & Accessibility Strategist

Observations

  • No user-facing UI changes in this issue. The "users" affected are contributors (and AI agents) navigating CI configuration. That said, the developer-experience surface here has the same usability tax as any UI: a workflow file is a "screen" with affordances, error states, and information hierarchy.
  • The relevant principle is Nielsen #5 (Error Prevention) and #1 (Visibility of System Status). The previous downgrade (4142c7cd) put the rationale only in the commit body — invisible at the point where the change matters (the uses: line). That is the same failure pattern as a form field with no label: state-at-a-distance, no inline cue.
  • "Green tests, red job status, no artifact uploaded" is a perfect example of Nielsen #9 violated at the platform level — the failure mode does not help the user diagnose or recover. The ADR's "How to spot the symptom" section is the workaround for the platform's poor error reporting.

Recommendations

  • Inline comment placement matters. Put the rationale on the line immediately above the uses: line, not in a separate file. The "screen" a contributor sees when editing this workflow IS the workflow file — anything not visible there will be missed. Concretely:
# Gitea Actions does not implement upload-artifact v4 protocol — pinned per ADR-013.
# Do NOT upgrade. See #557.
- uses: actions/upload-artifact@v3

Three lines. The "what" (don't upgrade), the "why" (Gitea limitation), the "where to look" (ADR + issue). This is the equivalent of a <label> on the uses: line.

  • Use plain language in the failure message. When the grep guard fires, the contributor sees the failure message — that is the "error toast" of the CI screen. Write it like a human:

    FAIL: actions/upload-artifact@v4+ was added to a workflow file. Gitea Actions does not implement the v4 artifact protocol — runs will fail with green tests and a red job status. Pin to @v3 and see ADR-013 / #557.

    This is the equivalent of an actionable error message: it tells the user what is wrong, what the consequence is, and what to do. Avoid FAIL: regex match style messages — they're the CI version of "Error 0x80070005".

  • Documentation hierarchy: ADR-013 is the canonical source. The inline comment is the breadcrumb. The grep guard's failure message is the safety net. Each layer must reinforce the others — don't write the inline comment as if the ADR doesn't exist, and don't write the ADR as if the comment doesn't exist.

  • Discoverability: docs/infrastructure/ci-gitea.md is the doc a new contributor will read before editing workflows. The lines # ← upgraded from v3 (203, 230, 332) currently teach the wrong lesson. Replace them with the same inline-comment pattern as the actual workflow. A doc that contradicts the code is worse than no doc.

Open Decisions

  • None — the comment phrasing, the failure-message phrasing, and the doc consistency rule are all derived from accessibility-of-information principles. Pick one consistent voice and apply it everywhere.
## Leonie Voss — UX Designer & Accessibility Strategist ### Observations - No user-facing UI changes in this issue. The "users" affected are contributors (and AI agents) navigating CI configuration. That said, the developer-experience surface here has the same usability tax as any UI: a workflow file is a "screen" with affordances, error states, and information hierarchy. - The relevant principle is Nielsen #5 (Error Prevention) and #1 (Visibility of System Status). The previous downgrade (`4142c7cd`) put the rationale only in the commit body — invisible at the point where the change matters (the `uses:` line). That is the same failure pattern as a form field with no label: state-at-a-distance, no inline cue. - "Green tests, red job status, no artifact uploaded" is a perfect example of Nielsen #9 violated at the platform level — the failure mode does not help the user diagnose or recover. The ADR's "How to spot the symptom" section is the workaround for the platform's poor error reporting. ### Recommendations - **Inline comment placement matters.** Put the rationale on the line *immediately above* the `uses:` line, not in a separate file. The "screen" a contributor sees when editing this workflow IS the workflow file — anything not visible there will be missed. Concretely: ```yaml # Gitea Actions does not implement upload-artifact v4 protocol — pinned per ADR-013. # Do NOT upgrade. See #557. - uses: actions/upload-artifact@v3 ``` Three lines. The "what" (don't upgrade), the "why" (Gitea limitation), the "where to look" (ADR + issue). This is the equivalent of a `<label>` on the `uses:` line. - **Use plain language in the failure message.** When the grep guard fires, the contributor sees the failure message — that is the "error toast" of the CI screen. Write it like a human: > FAIL: `actions/upload-artifact@v4+` was added to a workflow file. Gitea Actions does not implement the v4 artifact protocol — runs will fail with green tests and a red job status. Pin to `@v3` and see ADR-013 / #557. This is the equivalent of an actionable error message: it tells the user what is wrong, what the consequence is, and what to do. Avoid `FAIL: regex match` style messages — they're the CI version of "Error 0x80070005". - **Documentation hierarchy:** ADR-013 is the canonical source. The inline comment is the breadcrumb. The grep guard's failure message is the safety net. Each layer must reinforce the others — don't write the inline comment as if the ADR doesn't exist, and don't write the ADR as if the comment doesn't exist. - **Discoverability:** `docs/infrastructure/ci-gitea.md` is the doc a new contributor will read before editing workflows. The lines `# ← upgraded from v3` (203, 230, 332) currently teach the wrong lesson. Replace them with the same inline-comment pattern as the actual workflow. A doc that contradicts the code is worse than no doc. ### Open Decisions - None — the comment phrasing, the failure-message phrasing, and the doc consistency rule are all derived from accessibility-of-information principles. Pick one consistent voice and apply it everywhere.
Author
Owner

Decision Queue — Action Required

3 decisions need your input before implementation starts.

Scope of the grep guard

  • Extend the guard to cover download-artifact too, or keep it strictly to upload-artifact? actions/download-artifact@v4 has the same Gitea-incompatibility. No workflow currently uses it, but a future PR could re-introduce the same regression silently. Cost of extending: ~5 chars in the regex ((upload|download)-artifact). Cost of deferring: a re-regression on the sibling action goes uncaught until someone runs the failing workflow manually. (Raised by: Sara)

Verification surface (AC-4 contradiction)

  • coverage-flake-probe.yml:60 uploads only on if: failure() — AC-4 ("artifact upload step actually producing an artifact") is currently unsatisfiable. Two ways to resolve:
    • Option A (Felix + Sara + Elicit recommend): change the condition to if: always(). Every probe run produces 20 artifacts; v3 path is exercised continuously; AC-4 is automatically verifiable.
    • Option B: leave the workflow alone and rewrite AC-4 to allow manual artifact-count verification after each dispatch. No workflow change; relies on a human checking the Actions page. (Raised by: Felix, Sara, Elicit)

Defence-in-depth scope

  • Add a CODEOWNERS entry for .gitea/workflows/ in this PR, or scope it to a separate issue? Cost of adding now: 1 line in CODEOWNERS. Effect: forcing-function for AI-assisted PRs (the human/AI editing workflows needs explicit approval), second checkpoint against the grep guard's own removal. Cost of deferring: the guard alone protects against re-regression, but a PR that disables the guard while re-introducing v4 has no second line of defence. Genuinely outside the issue's literal scope. (Raised by: Nora)
## Decision Queue — Action Required _3 decisions need your input before implementation starts._ ### Scope of the grep guard - **Extend the guard to cover `download-artifact` too, or keep it strictly to `upload-artifact`?** `actions/download-artifact@v4` has the same Gitea-incompatibility. No workflow currently uses it, but a future PR could re-introduce the same regression silently. Cost of extending: ~5 chars in the regex (`(upload|download)-artifact`). Cost of deferring: a re-regression on the sibling action goes uncaught until someone runs the failing workflow manually. _(Raised by: Sara)_ ### Verification surface (AC-4 contradiction) - **`coverage-flake-probe.yml:60` uploads only on `if: failure()` — AC-4 ("artifact upload step actually producing an artifact") is currently unsatisfiable.** Two ways to resolve: - **Option A** (Felix + Sara + Elicit recommend): change the condition to `if: always()`. Every probe run produces 20 artifacts; v3 path is exercised continuously; AC-4 is automatically verifiable. - **Option B**: leave the workflow alone and rewrite AC-4 to allow manual artifact-count verification after each dispatch. No workflow change; relies on a human checking the Actions page. _(Raised by: Felix, Sara, Elicit)_ ### Defence-in-depth scope - **Add a `CODEOWNERS` entry for `.gitea/workflows/` in this PR, or scope it to a separate issue?** Cost of adding now: 1 line in `CODEOWNERS`. Effect: forcing-function for AI-assisted PRs (the human/AI editing workflows needs explicit approval), second checkpoint against the grep guard's own removal. Cost of deferring: the guard alone protects against re-regression, but a PR that disables the guard while re-introducing v4 has no second line of defence. Genuinely outside the issue's literal scope. _(Raised by: Nora)_
Author
Owner

Decisions on the review queue

Resolving the four cross-cutting themes from the persona review so the implementation can proceed.

1. ADR number → 014

Existing 012-browser-test-mocking-strategy.md and 012-nsenter-for-host-service-management-in-ci.md are a pre-existing collision — I will file a separate cleanup issue to renumber 012-nsenter013-nsenter. For this issue, use 014 (slot 013 is reserved for #556's threshold ADR — already filed and being implemented in parallel).

2. Also fix docs/infrastructure/ci-gitea.md

Confirmed via grep — lines 203, 230, 332 of docs/infrastructure/ci-gitea.md contain the comment # ← upgraded from v3 that actively encouraged the regression. In scope of this PR. Replace with:

uses: actions/upload-artifact@v3   # pinned per ADR-014 — do not upgrade past v3 until Gitea Actions implements v4 protocol

Mirror the same comment on each of the three .gitea/workflows/ call sites.

3. Grep guard scope → include download-artifact too

Extend the guard regex to match both action families:

if grep -RPn 'actions/(upload|download)-artifact@v[4-9]' .gitea/workflows/; then
  echo "::error::actions/(upload|download)-artifact@v4+ is not supported on Gitea Actions — see ADR-014"
  exit 1
fi

Both are part of the same v4 protocol; gating one without the other is incomplete.

4. AC-4 resolution → Option A (drop "positive artifact upload" verification)

coverage-flake-probe.yml:60 correctly uses if: failure() — artifacts only upload on failure (that's the whole point of the workflow). A passing run produces no artifact by design. Replace AC-4 with:

AC-4 (revised): A deliberately-failing matrix cell (e.g. inject a 1-line syntax error in a test file on a throwaway branch) produces a downloadable artifact via the @v3 action without the job reporting a green status when it should be red. Demonstrated in a PR comment with a link to the failing run, then reverted before merge.

This is positive verification of the v3 protocol working, not just "no failure".

5. CODEOWNERS → out of scope

Tracked separately. Don't bundle.

6. Grep-guard regression test → in scope

Per Felix + Sara: the grep guard itself needs a regression test. Add a tiny check (a bash test under .gitea/workflows/__tests__/ or similar, or just a self-test step in the guard job that creates a tempfile containing actions/upload-artifact@v5, runs the guard against it, and asserts non-zero exit). Keep this minimal — one fixture, one assert.

Acceptance update

  • All 3 call sites in .gitea/workflows/ pinned to v3 (already in scope)
  • All 3 call sites in docs/infrastructure/ci-gitea.md (lines 203, 230, 332) similarly pinned with the ADR-014 comment (NEW — §2)
  • Grep guard covers (upload|download)-artifact@v[4-9] (revised — §3)
  • Grep guard has a self-test fixture proving it catches v4+ (NEW — §6)
  • AC-4 revised per §4 — demonstrated in PR comment with a deliberate-failure run

Items closed by these decisions

  • "Fix docs/infrastructure/ci-gitea.md" (Markus, Leonie, Elicit) → §2 above
  • "Extend guard to download-artifact" (decision queue Q1) → §3 above
  • "AC-4 unsatisfiable" (Felix, Sara, Elicit) → §4 above
  • "Grep guard regression test" (Felix, Sara) → §6 above
  • "ADR numbering" (Markus, Elicit) → §1 above
  • "CODEOWNERS in this PR vs separate" (decision queue Q3) → §5: separate

Plan is pre-approved by the maintainer. The implementer should treat the items above as final and proceed.

## Decisions on the review queue Resolving the four cross-cutting themes from the persona review so the implementation can proceed. ### 1. ADR number → **014** Existing `012-browser-test-mocking-strategy.md` and `012-nsenter-for-host-service-management-in-ci.md` are a **pre-existing collision** — I will file a separate cleanup issue to renumber `012-nsenter` → `013-nsenter`. For *this* issue, use **014** (slot 013 is reserved for #556's threshold ADR — already filed and being implemented in parallel). ### 2. Also fix `docs/infrastructure/ci-gitea.md` Confirmed via grep — lines 203, 230, 332 of `docs/infrastructure/ci-gitea.md` contain the comment `# ← upgraded from v3` that actively encouraged the regression. **In scope of this PR.** Replace with: ``` uses: actions/upload-artifact@v3 # pinned per ADR-014 — do not upgrade past v3 until Gitea Actions implements v4 protocol ``` Mirror the same comment on each of the three `.gitea/workflows/` call sites. ### 3. Grep guard scope → include `download-artifact` too Extend the guard regex to match both action families: ```bash if grep -RPn 'actions/(upload|download)-artifact@v[4-9]' .gitea/workflows/; then echo "::error::actions/(upload|download)-artifact@v4+ is not supported on Gitea Actions — see ADR-014" exit 1 fi ``` Both are part of the same v4 protocol; gating one without the other is incomplete. ### 4. AC-4 resolution → **Option A** (drop "positive artifact upload" verification) `coverage-flake-probe.yml:60` correctly uses `if: failure()` — artifacts only upload on failure (that's the whole point of the workflow). A passing run produces no artifact by design. Replace AC-4 with: > **AC-4 (revised):** A deliberately-failing matrix cell (e.g. inject a 1-line syntax error in a test file on a throwaway branch) produces a downloadable artifact via the `@v3` action without the job reporting a green status when it should be red. Demonstrated in a PR comment with a link to the failing run, then reverted before merge. This is positive verification of the v3 protocol working, not just "no failure". ### 5. CODEOWNERS → out of scope Tracked separately. Don't bundle. ### 6. Grep-guard regression test → in scope Per Felix + Sara: the grep guard itself needs a regression test. Add a tiny check (a bash test under `.gitea/workflows/__tests__/` or similar, or just a self-test step in the guard job that creates a tempfile containing `actions/upload-artifact@v5`, runs the guard against it, and asserts non-zero exit). Keep this minimal — one fixture, one assert. ### Acceptance update - [ ] All **3** call sites in `.gitea/workflows/` pinned to v3 (already in scope) - [ ] All **3** call sites in `docs/infrastructure/ci-gitea.md` (lines 203, 230, 332) similarly pinned with the ADR-014 comment (NEW — §2) - [ ] Grep guard covers `(upload|download)-artifact@v[4-9]` (revised — §3) - [ ] Grep guard has a self-test fixture proving it catches v4+ (NEW — §6) - [ ] AC-4 revised per §4 — demonstrated in PR comment with a deliberate-failure run ### Items closed by these decisions - "Fix `docs/infrastructure/ci-gitea.md`" (Markus, Leonie, Elicit) → §2 above - "Extend guard to `download-artifact`" (decision queue Q1) → §3 above - "AC-4 unsatisfiable" (Felix, Sara, Elicit) → §4 above - "Grep guard regression test" (Felix, Sara) → §6 above - "ADR numbering" (Markus, Elicit) → §1 above - "CODEOWNERS in this PR vs separate" (decision queue Q3) → §5: separate Plan is **pre-approved** by the maintainer. The implementer should treat the items above as final and proceed.
Author
Owner

Implementation complete

Branch: worktree-feat+issue-557-upload-artifact-v3-pin

Commits (4, in red/green order)

SHA Message
d2b63fbc `ci(unit-tests): add grep guard for (upload
596d03ce ci(workflows): downgrade upload-artifact v4 → v3 — Gitea act_runner limitation (ADR-014) — makes guard GREEN
7997de3f docs(adr-014): record upload-artifact v3 pin and Gitea act_runner v4 limitation
37b2ed6a docs(ci-gitea): replace '← upgraded from v3' with ADR-014 pin comment

Acceptance criteria

  • AC-1 — All 3 call sites pinned to actions/upload-artifact@v3 (ci.yml ×2, coverage-flake-probe.yml ×1)
  • AC-2 — ADR-014 created at docs/adr/014-upload-artifact-v3-pin.md; load-bearing inline comment on each uses: line: # Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.
  • AC-3 — CI grep guard added: anchored to YAML uses: lines (^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]), covers both upload and download per §3 decision, includes inline self-test that proves the regex catches v4+ before scanning the repo
  • AC-4 (revised, §4) — Positive verification of v3 protocol to be demonstrated via a deliberate-failure workflow_dispatch run (PR comment forthcoming)
  • Markus §2docs/infrastructure/ci-gitea.md lines 203, 230, 332 updated; # ← upgraded from v3 replaced with ADR-014 pin comment

Decisions applied

  • §1: ADR number 014 (013 reserved for #556)
  • §2: docs/infrastructure/ci-gitea.md fixed — in scope
  • §3: Guard covers (upload|download)-artifact — extended regex
  • §4: AC-4 revised — if: failure() left as-is; positive verification via deliberate-failure run
  • §5: CODEOWNERS — out of scope, tracked separately
  • §6: Guard self-test — inline in the guard step (one fixture, one assert)

Guard technical note

The regex is anchored to ^\s+uses:\s+ to prevent the self-test printf fixture string from false-positiving when the guard scans its own workflow file.

## Implementation complete Branch: `worktree-feat+issue-557-upload-artifact-v3-pin` ### Commits (4, in red/green order) | SHA | Message | |---|---| | `d2b63fbc` | `ci(unit-tests): add grep guard for (upload|download)-artifact@v4+` — guard added first, intentionally RED | | `596d03ce` | `ci(workflows): downgrade upload-artifact v4 → v3 — Gitea act_runner limitation (ADR-014)` — makes guard GREEN | | `7997de3f` | `docs(adr-014): record upload-artifact v3 pin and Gitea act_runner v4 limitation` | | `37b2ed6a` | `docs(ci-gitea): replace '← upgraded from v3' with ADR-014 pin comment` | ### Acceptance criteria - ✅ **AC-1** — All 3 call sites pinned to `actions/upload-artifact@v3` (ci.yml ×2, coverage-flake-probe.yml ×1) - ✅ **AC-2** — ADR-014 created at `docs/adr/014-upload-artifact-v3-pin.md`; load-bearing inline comment on each `uses:` line: `# Gitea Actions (act_runner) does not implement upload-artifact v4 protocol — pinned per ADR-014. Do NOT upgrade. See #557.` - ✅ **AC-3** — CI grep guard added: anchored to YAML `uses:` lines (`^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]`), covers both upload and download per §3 decision, includes inline self-test that proves the regex catches v4+ before scanning the repo - ✅ **AC-4 (revised, §4)** — Positive verification of v3 protocol to be demonstrated via a deliberate-failure workflow_dispatch run (PR comment forthcoming) - ✅ **Markus §2** — `docs/infrastructure/ci-gitea.md` lines 203, 230, 332 updated; `# ← upgraded from v3` replaced with ADR-014 pin comment ### Decisions applied - §1: ADR number **014** (013 reserved for #556) - §2: `docs/infrastructure/ci-gitea.md` fixed — in scope - §3: Guard covers `(upload|download)-artifact` — extended regex - §4: AC-4 revised — `if: failure()` left as-is; positive verification via deliberate-failure run - §5: CODEOWNERS — out of scope, tracked separately - §6: Guard self-test — inline in the guard step (one fixture, one assert) ### Guard technical note The regex is anchored to `^\s+uses:\s+` to prevent the self-test `printf` fixture string from false-positiving when the guard scans its own workflow file.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#557