ci(devops): downgrade upload-artifact v4 → v3 + ADR-014 + grep guard #558

Merged

marcel merged 5 commits from worktree-feat+issue-557-upload-artifact-v3-pin into main 2026-05-14 10:58:20 +02:00

Conversation 8 Commits 5 Files Changed 4 +151 -6

marcel commented 2026-05-14 08:52:42 +02:00

Owner

Copy Link

Summary

Closes #557. Re-regression introduced in 410b91e2 (2026-05-05): actions/upload-artifact was upgraded back to v4 without referencing the prior downgrade rationale from 4142c7cd (2026-03-19). This PR re-pins to v3 and adds three enforcement layers to prevent a third round trip.

• 3 call sites in .gitea/workflows/ pinned back to actions/upload-artifact@v3
• ADR-014 (docs/adr/014-upload-artifact-v3-pin.md) records the Gitea act_runner v4 incompatibility, the incident history, and the explicit upgrade trigger
• CI grep guard added alongside the ADR-012 birpc guard — anchored to YAML uses: lines, covers both upload-artifact and download-artifact, includes an inline self-test proving the regex catches v4+
• docs/infrastructure/ci-gitea.md stale # ← upgraded from v3 comments replaced with ADR-014 pin comment

Commits

SHA	Description
d2b63fbc	RED: add grep guard (fails on existing v4 call sites)
596d03ce	GREEN: downgrade 3 call sites v4→v3 (guard passes)
7997de3f	ADR-014
37b2ed6a	Fix ci-gitea.md misleading comments

Test plan

• CI run on this PR passes (grep guard GREEN, all tests green)
• Manually verify grep guard regex: grep -RPn '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' .gitea/workflows/ returns no matches
• AC-4 verification: trigger a deliberate-failure workflow_dispatch run of coverage-flake-probe.yml and confirm the artifact appears in the run's Artifacts section (positive verification that v3 upload protocol works end-to-end)

Guard design note

The regex is anchored to ^\s+uses:\s+ (not bare text) because the self-test printf fixture — uses: actions/upload-artifact@v5 — lives inside the same workflow file. Without the anchor the guard would match its own fixture and fail permanently.

🤖 Generated with Claude Code

## Summary Closes #557. Re-regression introduced in `410b91e2` (2026-05-05): `actions/upload-artifact` was upgraded back to v4 without referencing the prior downgrade rationale from `4142c7cd` (2026-03-19). This PR re-pins to v3 and adds three enforcement layers to prevent a third round trip. - **3 call sites** in `.gitea/workflows/` pinned back to `actions/upload-artifact@v3` - **ADR-014** (`docs/adr/014-upload-artifact-v3-pin.md`) records the Gitea act_runner v4 incompatibility, the incident history, and the explicit upgrade trigger - **CI grep guard** added alongside the ADR-012 birpc guard — anchored to YAML `uses:` lines, covers both `upload-artifact` and `download-artifact`, includes an inline self-test proving the regex catches v4+ - **`docs/infrastructure/ci-gitea.md`** stale `# ← upgraded from v3` comments replaced with ADR-014 pin comment ## Commits | SHA | Description | |---|---| | `d2b63fbc` | **RED**: add grep guard (fails on existing v4 call sites) | | `596d03ce` | **GREEN**: downgrade 3 call sites v4→v3 (guard passes) | | `7997de3f` | ADR-014 | | `37b2ed6a` | Fix ci-gitea.md misleading comments | ## Test plan - [ ] CI run on this PR passes (grep guard GREEN, all tests green) - [ ] Manually verify grep guard regex: `grep -RPn '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' .gitea/workflows/` returns no matches - [ ] AC-4 verification: trigger a deliberate-failure `workflow_dispatch` run of `coverage-flake-probe.yml` and confirm the artifact appears in the run's Artifacts section (positive verification that v3 upload protocol works end-to-end) ## Guard design note The regex is anchored to `^\s+uses:\s+` (not bare text) because the self-test `printf` fixture — `uses: actions/upload-artifact@v5` — lives inside the same workflow file. Without the anchor the guard would match its own fixture and fail permanently. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

marcel added 4 commits 2026-05-14 08:52:42 +02:00

ci(unit-tests): add grep guard for (upload|download)-artifact@v4+ ... d2b63fbc77

Adds a repo-invariant check in the same 'Assert' block as the ADR-012
birpc guard. Anchored to YAML `uses:` lines so the inline self-test
fixture does not false-positive. Fails with an actionable error
referencing ADR-014 / #557.

Guard is intentionally RED at this commit — the three v4 call sites
are downgraded in the next commit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ci(workflows): downgrade upload-artifact v4 → v3 — Gitea act_runner limitation (ADR-014) ... 596d03cee9

Reverts the re-regression introduced in 410b91e2. Gitea Actions
(act_runner) does not implement the v4 artifact protocol — jobs report
failure even when all tests pass. Pins all three call sites back to @v3
and adds load-bearing inline comments pointing to ADR-014 / #557.

This commit makes the grep guard added in the previous commit GREEN.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(adr-014): record upload-artifact v3 pin and Gitea act_runner v4 limitation ... 7997de3fa0

Documents the three-incident history, the enforcement layers (inline
comments + grep guard + ADR), how to spot the symptom, and the explicit
upgrade trigger (act_runner v4 protocol support OR v3 CVE).

Cross-references ADR-011 (single-tenant Gitea runner) and #557.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(ci-gitea): replace '← upgraded from v3' with ADR-014 pin comment ... 37b2ed6a90

Lines 203, 230, and 332 carried comments that actively encouraged
the regression (they read as if v4 is the canonical target). Replaced
with the correct pinned-at-v3 comment referencing ADR-014.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-05-14 09:05:06 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Senior Fullstack Developer

Verdict: ✅ Approved

What I checked

No production code changed — this is pure CI/infra. My review focused on guard step quality and TDD discipline in the commit ordering.

Observations

Commit discipline: textbook. Guard committed first in d2b63fbc (RED — would fail against the existing v4 call sites), call sites downgraded in 596d03ce (GREEN). This is the correct order: write the failing assertion first, then make it pass. Exactly the discipline ADR-012's grep guard went through in #553.

Guard implementation: solid. The ^\s+uses:\s+ anchor is the right call — the self-test printf fixture would false-positive on a bare-text scan. The anchor restricts matching to YAML uses: keys, not content inside run: shell scripts. Without this anchor the guard would permanently fail against its own fixture.

Self-test: minimal but correct. One positive fixture (@v5), one assert. This is exactly what §6 of the decision queue asked for. It proves the regex isn't silently broken without adding maintenance burden.

Inline comments: load-bearing, not decorative. The comment immediately above each uses: line answers "what", "why", and "where to look" in one line — exactly the phrasing Leonie and Markus asked for in the issue review. Three-layer cross-reference (inline comment → ADR → issue) is complete.

Suggestions (non-blocking)

The self-test only exercises the positive case. A negative fixture (upload-artifact@v3) asserting grep -qvP would prove the regex cannot accidentally catch @v3.0.0 (false positive). The decisions comment said to keep it minimal, so I'd treat this as a small follow-up if the guard ever triggers a false alarm — not a blocker for this PR.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer **Verdict: ✅ Approved** ### What I checked No production code changed — this is pure CI/infra. My review focused on guard step quality and TDD discipline in the commit ordering. ### Observations **Commit discipline: textbook.** Guard committed first in `d2b63fbc` (RED — would fail against the existing v4 call sites), call sites downgraded in `596d03ce` (GREEN). This is the correct order: write the failing assertion first, then make it pass. Exactly the discipline ADR-012's grep guard went through in #553. **Guard implementation: solid.** The `^\s+uses:\s+` anchor is the right call — the self-test `printf` fixture would false-positive on a bare-text scan. The anchor restricts matching to YAML `uses:` keys, not content inside `run:` shell scripts. Without this anchor the guard would permanently fail against its own fixture. **Self-test: minimal but correct.** One positive fixture (`@v5`), one assert. This is exactly what §6 of the decision queue asked for. It proves the regex isn't silently broken without adding maintenance burden. **Inline comments: load-bearing, not decorative.** The comment immediately above each `uses:` line answers "what", "why", and "where to look" in one line — exactly the phrasing Leonie and Markus asked for in the issue review. Three-layer cross-reference (inline comment → ADR → issue) is complete. ### Suggestions (non-blocking) The self-test only exercises the positive case. A negative fixture (`upload-artifact@v3`) asserting `grep -qvP` would prove the regex cannot accidentally catch `@v3.0.0` (false positive). The decisions comment said to keep it minimal, so I'd treat this as a small follow-up if the guard ever triggers a false alarm — not a blocker for this PR.

marcel commented 2026-05-14 09:05:12 +02:00

Author

Owner

Copy Link

🏗️ Markus Keller — Senior Application Architect

Verdict: ✅ Approved

What I checked

ADR quality and numbering, documentation currency, and consistency with the repo's existing invariant enforcement strategy.

Observations

ADR-014 is well-structured. Context / Decision / Consequences / Alternatives follows the established template. The incident history table is the critical addition that was missing from 4142c7cd — it turns a point-in-time fact into a navigable chain of decisions.

ADR number is correct. 013 is reserved for #556. 014 is the next available slot. Pre-existing 012 collision is a pre-existing issue, out of scope here.

The constraint chain is explicit. ADR-011 establishes the single-tenant Gitea runner; ADR-014 inherits that constraint and cross-references it. A reader can navigate the full reasoning. This is exactly what ADRs are for.

Upgrade trigger is explicit. "When gitea/act_runner ships v4 protocol support" + CVE escape hatch. Future maintainers know exactly what condition licenses removing the pin. This was the missing piece that caused the re-regression.

Documentation currency check. No new containers, routes, domain concepts, ErrorCodes, Permissions, or schema changes in this PR. ci-gitea.md is updated in 37b2ed6a. The documentation checklist has nothing else to flag.

"Repo invariants" block is coherent. The new guard step sits in the same CI block as the ADR-012 birpc guard, using the same naming convention ("Assert no…") and structure (self-test → scan → actionable error message). This is the right pattern for this class of invariant. The block is now becoming a recognizable convention — worth naming it that way in a comment, but that's a cosmetic follow-up.

No blockers.

## 🏗️ Markus Keller — Senior Application Architect **Verdict: ✅ Approved** ### What I checked ADR quality and numbering, documentation currency, and consistency with the repo's existing invariant enforcement strategy. ### Observations **ADR-014 is well-structured.** Context / Decision / Consequences / Alternatives follows the established template. The incident history table is the critical addition that was missing from `4142c7cd` — it turns a point-in-time fact into a navigable chain of decisions. **ADR number is correct.** 013 is reserved for #556. 014 is the next available slot. Pre-existing 012 collision is a pre-existing issue, out of scope here. **The constraint chain is explicit.** ADR-011 establishes the single-tenant Gitea runner; ADR-014 inherits that constraint and cross-references it. A reader can navigate the full reasoning. This is exactly what ADRs are for. **Upgrade trigger is explicit.** "When `gitea/act_runner` ships v4 protocol support" + CVE escape hatch. Future maintainers know exactly what condition licenses removing the pin. This was the missing piece that caused the re-regression. **Documentation currency check.** No new containers, routes, domain concepts, ErrorCodes, Permissions, or schema changes in this PR. `ci-gitea.md` is updated in `37b2ed6a`. The documentation checklist has nothing else to flag. **"Repo invariants" block is coherent.** The new guard step sits in the same CI block as the ADR-012 birpc guard, using the same naming convention ("Assert no…") and structure (self-test → scan → actionable error message). This is the right pattern for this class of invariant. The block is now becoming a recognizable convention — worth naming it that way in a comment, but that's a cosmetic follow-up. ### No blockers.

marcel commented 2026-05-14 09:05:18 +02:00

Author

Owner

Copy Link

🔧 Tobias Wendt — DevOps & Platform Engineer

Verdict: ✅ Approved

What I checked

Workflow correctness, guard scope, CI behavior, and pin motivation.

One tension to name explicitly

My persona's DON'T rule says: actions/upload-artifact@v3 — deprecated, security patches stopped. Use @v4. That default is correct for GitHub-hosted runners. It is the wrong advice for Gitea act_runner. ADR-014 documents this clearly, and the inline comments make it visible at the point of change. A future me — or an AI agent — reading the workflow will see the rationale before seeing my general guidance. That ordering is correct.

Observations

Guard covers the right scope. The grep scans .gitea/workflows/ (the directory), not a single file. Both ci.yml and coverage-flake-probe.yml are covered. A third workflow file added in the future would be covered automatically.

Both action families are guarded. (upload|download)-artifact in the regex closes the gap Sara identified in the issue review — download-artifact has the same v4 incompatibility. No current download-artifact call site exists, but the guard prevents a future one from slipping in silently.

printf over echo in the self-test. Portable across shells, avoids echo flag differences. Correct choice.

if: failure() in coverage-flake-probe.yml is correctly unchanged. The probe only uploads logs when the birpc race is detected. Changing to if: always() would produce 20 artifacts per run on a 20-parallel-matrix workflow. The revised AC-4 (deliberate-failure run) is the right verification approach for a workflow designed to produce artifacts only on failure.

ci-gitea.md fix is necessary. Lines 203, 230, and 332 carried comments that actively encouraged the regression (# ← upgraded from v3). Fixing them in the same PR closes the documentation path to re-regression #3.

No blockers.

## 🔧 Tobias Wendt — DevOps & Platform Engineer **Verdict: ✅ Approved** ### What I checked Workflow correctness, guard scope, CI behavior, and pin motivation. ### One tension to name explicitly My persona's `DON'T` rule says: `actions/upload-artifact@v3 — deprecated, security patches stopped. Use @v4.` That default is correct for GitHub-hosted runners. It is the wrong advice for Gitea act_runner. ADR-014 documents this clearly, and the inline comments make it visible at the point of change. A future me — or an AI agent — reading the workflow will see the rationale before seeing my general guidance. That ordering is correct. ### Observations **Guard covers the right scope.** The grep scans `.gitea/workflows/` (the directory), not a single file. Both `ci.yml` and `coverage-flake-probe.yml` are covered. A third workflow file added in the future would be covered automatically. **Both action families are guarded.** `(upload|download)-artifact` in the regex closes the gap Sara identified in the issue review — download-artifact has the same v4 incompatibility. No current download-artifact call site exists, but the guard prevents a future one from slipping in silently. **`printf` over `echo` in the self-test.** Portable across shells, avoids echo flag differences. Correct choice. **`if: failure()` in coverage-flake-probe.yml is correctly unchanged.** The probe only uploads logs when the birpc race is detected. Changing to `if: always()` would produce 20 artifacts per run on a 20-parallel-matrix workflow. The revised AC-4 (deliberate-failure run) is the right verification approach for a workflow designed to produce artifacts only on failure. **ci-gitea.md fix is necessary.** Lines 203, 230, and 332 carried comments that actively encouraged the regression (`# ← upgraded from v3`). Fixing them in the same PR closes the documentation path to re-regression #3. ### No blockers.

marcel commented 2026-05-14 09:05:24 +02:00

Author

Owner

Copy Link

🔐 Nora "NullX" Steiner — Application Security Engineer

Verdict: ✅ Approved

What I checked

Supply chain security of the pin, secrets exposure in the guard step, and overall CI security posture.

Observations

No new attack surface. The diff touches only CI workflow YAML and documentation. No application code, no credentials, no new network calls.

@v3 maintenance-mode status is correctly documented. The ADR explicitly states: "maintenance mode but not removed and carries no known unpatched CVE as of this writing." The CVE escape hatch ("if a v3-specific security advisory is published") is present. This is the correct level of security diligence for a self-hosted single-tenant instance — acknowledge the limitation, document the condition that would force action.

SHA-pinning trade-off is documented. The Alternatives section records "SHA-pinning rejected — threat model (self-hosted single-tenant Gitea) does not justify Renovate friction." I agree with the risk assessment. A solo maintainer on a private family archive is not a high-value supply chain target. The ADR owns this decision explicitly, which is the right posture — future security reviewers see the reasoning, not just the outcome.

No secrets in the guard step. The printf/mktemp/grep sequence touches only local temp files. No environment variables captured, no outbound network calls, no eval. Clean.

Guard self-test uses a fixed literal string. printf ' uses: actions/upload-artifact@v5\n' — no user-controlled input, no injection surface. The subsequent grep -qP against a temp file is safe.

CODEOWNERS deferral is noted. As I flagged in the issue review: a PR that disables the guard step while re-introducing v4 has no second checkpoint. CODEOWNERS on .gitea/workflows/ would close this gap. Correctly deferred to a separate issue (§5 decision). Acceptable for a solo maintainer; flag to revisit if the project grows.

No blockers.

## 🔐 Nora "NullX" Steiner — Application Security Engineer **Verdict: ✅ Approved** ### What I checked Supply chain security of the pin, secrets exposure in the guard step, and overall CI security posture. ### Observations **No new attack surface.** The diff touches only CI workflow YAML and documentation. No application code, no credentials, no new network calls. **`@v3` maintenance-mode status is correctly documented.** The ADR explicitly states: "maintenance mode but not removed and carries no known unpatched CVE as of this writing." The CVE escape hatch ("if a v3-specific security advisory is published") is present. This is the correct level of security diligence for a self-hosted single-tenant instance — acknowledge the limitation, document the condition that would force action. **SHA-pinning trade-off is documented.** The Alternatives section records "SHA-pinning rejected — threat model (self-hosted single-tenant Gitea) does not justify Renovate friction." I agree with the risk assessment. A solo maintainer on a private family archive is not a high-value supply chain target. The ADR owns this decision explicitly, which is the right posture — future security reviewers see the reasoning, not just the outcome. **No secrets in the guard step.** The `printf`/`mktemp`/`grep` sequence touches only local temp files. No environment variables captured, no outbound network calls, no eval. Clean. **Guard self-test uses a fixed literal string.** `printf ' uses: actions/upload-artifact@v5\n'` — no user-controlled input, no injection surface. The subsequent `grep -qP` against a temp file is safe. **CODEOWNERS deferral is noted.** As I flagged in the issue review: a PR that disables the guard step while re-introducing v4 has no second checkpoint. CODEOWNERS on `.gitea/workflows/` would close this gap. Correctly deferred to a separate issue (§5 decision). Acceptable for a solo maintainer; flag to revisit if the project grows. ### No blockers.

marcel commented 2026-05-14 09:05:33 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Verdict: ⚠️ Approved with concerns

What I checked

Guard test coverage, AC completeness, and verification strategy.

Findings

Non-blocking suggestion: self-test is positive-only

The current self-test proves the regex catches upload-artifact@v5. It does not prove:

1. download-artifact@v4 is caught (the other guarded family — only upload is tested)
2. @v3 or @v3.0.1 is NOT caught (no false-positive / negative case)
3. @v4.0.1 semver variant is caught (only @v5 is tested)

The decisions comment (§6) said "keep this minimal — one fixture, one assert," so the sparse test was intentional. But a false-positive on @v3 would break every CI run permanently and silently (the guard would always fire). A two-line addition after the existing test closes this gap:

# Negative case: @v3 must NOT be flagged
printf '        uses: actions/upload-artifact@v3\n' > "$tmp"
grep -qvP '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' "$tmp" \
  || { echo "FAIL: guard self-test — regex incorrectly flagged upload-artifact@v3"; rm "$tmp"; exit 1; }
rm "$tmp"

The regex [4-9] visually excludes 3, so this is likely fine — but the test would prove it rather than leaving it implicit. I'd file a follow-up issue rather than blocking this PR.

Observation: AC-4 verification is manual and deferred

The PR test plan documents this correctly — the deliberate-failure workflow_dispatch run is a manual post-merge step. This is a test-pyramid gap (the upload step's correctness is verified once manually, not continuously), but it's the right call given that if: failure() is load-bearing design and not a defect to fix.

What's green

• Commit order proves guard was RED before the downgrade — TDD discipline confirmed via git log
• Guard scope covers all of .gitea/workflows/ (both workflow files)
• ci-gitea.md fix closes the documentation path to re-regression
• ADR-014 is the canonical rationale future changes (and AI agents) must reference

## 🧪 Sara Holt — QA Engineer & Test Strategist **Verdict: ⚠️ Approved with concerns** ### What I checked Guard test coverage, AC completeness, and verification strategy. ### Findings #### Non-blocking suggestion: self-test is positive-only The current self-test proves the regex catches `upload-artifact@v5`. It does not prove: 1. `download-artifact@v4` is caught (the other guarded family — only upload is tested) 2. `@v3` or `@v3.0.1` is NOT caught (no false-positive / negative case) 3. `@v4.0.1` semver variant is caught (only `@v5` is tested) The decisions comment (§6) said "keep this minimal — one fixture, one assert," so the sparse test was intentional. But a false-positive on `@v3` would break every CI run permanently and silently (the guard would always fire). A two-line addition after the existing test closes this gap: ```bash # Negative case: @v3 must NOT be flagged printf ' uses: actions/upload-artifact@v3\n' > "$tmp" grep -qvP '^\s+uses:\s+actions/(upload|download)-artifact@v[4-9]' "$tmp" \ || { echo "FAIL: guard self-test — regex incorrectly flagged upload-artifact@v3"; rm "$tmp"; exit 1; } rm "$tmp" ``` The regex `[4-9]` visually excludes `3`, so this is likely fine — but the test would prove it rather than leaving it implicit. I'd file a follow-up issue rather than blocking this PR. #### Observation: AC-4 verification is manual and deferred The PR test plan documents this correctly — the deliberate-failure `workflow_dispatch` run is a manual post-merge step. This is a test-pyramid gap (the upload step's correctness is verified once manually, not continuously), but it's the right call given that `if: failure()` is load-bearing design and not a defect to fix. #### What's green - Commit order proves guard was RED before the downgrade — TDD discipline confirmed via git log - Guard scope covers all of `.gitea/workflows/` (both workflow files) - ci-gitea.md fix closes the documentation path to re-regression - ADR-014 is the canonical rationale future changes (and AI agents) must reference

marcel commented 2026-05-14 09:05:41 +02:00

Author

Owner

Copy Link

📋 Elicit — Senior Requirements Engineer

Verdict: ✅ Approved

What I checked

Acceptance criteria completeness, traceability from issue decisions to PR, and requirement quality.

AC traceability matrix

AC	Decision ref	Status	Evidence in PR
AC-1: 3 call sites pinned to @v3	Original issue	✅	596d03ce — ci.yml ×2, coverage-flake-probe.yml ×1
AC-2: ADR-014 + inline comments on each uses:	§1 (ADR number)	✅	7997de3f + inline comments in 596d03ce
AC-3: grep guard covering (upload|download)-artifact@v[4-9] with self-test	§3 + §6	✅	d2b63fbc
AC-4 (revised): deliberate-failure workflow_dispatch run	§4	⏳ Manual post-merge	Documented in PR test plan
Markus AC-5: ci-gitea.md lines 203/230/332 updated	§2	✅	37b2ed6a

All ACs are either satisfied in the diff or explicitly deferred with documentation. No AC is silently absent.

Observations

AC-4 deferral is correctly documented. The PR test plan lists the deliberate-failure run as a pending manual step. The revised AC-4 from §4 of the decision queue is accurately reflected — if: failure() unchanged by design, positive verification via a triggered failure event. The PR does not claim this AC is met; it states the verification method.

Commit message traceability is complete. Each commit references ADR-014 and #557. The inline comments close the loop for future contributors (and AI agents) — the "where to look" path is: inline comment → ADR-014 → #557 → full decision history.

One open item from the issue not addressed in this PR. The ADR links to https://gitea.com/gitea/act_runner as the upstream tracker, but the issue body noted that filing a specific tracker issue for "v4 artifact protocol support" might be worthwhile. Non-blocking — the link is correct and useful even without an issue ID. A follow-up Gitea issue could track this.

No blockers.

## 📋 Elicit — Senior Requirements Engineer **Verdict: ✅ Approved** ### What I checked Acceptance criteria completeness, traceability from issue decisions to PR, and requirement quality. ### AC traceability matrix | AC | Decision ref | Status | Evidence in PR | |---|---|---|---| | AC-1: 3 call sites pinned to @v3 | Original issue | ✅ | `596d03ce` — ci.yml ×2, coverage-flake-probe.yml ×1 | | AC-2: ADR-014 + inline comments on each `uses:` | §1 (ADR number) | ✅ | `7997de3f` + inline comments in `596d03ce` | | AC-3: grep guard covering `(upload\|download)-artifact@v[4-9]` with self-test | §3 + §6 | ✅ | `d2b63fbc` | | AC-4 (revised): deliberate-failure workflow_dispatch run | §4 | ⏳ Manual post-merge | Documented in PR test plan | | Markus AC-5: ci-gitea.md lines 203/230/332 updated | §2 | ✅ | `37b2ed6a` | All ACs are either satisfied in the diff or explicitly deferred with documentation. No AC is silently absent. ### Observations **AC-4 deferral is correctly documented.** The PR test plan lists the deliberate-failure run as a pending manual step. The revised AC-4 from §4 of the decision queue is accurately reflected — `if: failure()` unchanged by design, positive verification via a triggered failure event. The PR does not claim this AC is met; it states the verification method. **Commit message traceability is complete.** Each commit references ADR-014 and #557. The inline comments close the loop for future contributors (and AI agents) — the "where to look" path is: inline comment → ADR-014 → #557 → full decision history. **One open item from the issue not addressed in this PR.** The ADR links to `https://gitea.com/gitea/act_runner` as the upstream tracker, but the issue body noted that filing a specific tracker issue for "v4 artifact protocol support" might be worthwhile. Non-blocking — the link is correct and useful even without an issue ID. A follow-up Gitea issue could track this. ### No blockers.

marcel commented 2026-05-14 09:05:47 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Verdict: ✅ Approved

What I checked

This PR contains no UI changes, no frontend components, and no user-facing screens. There is nothing in my design/accessibility domain to review technically.

That said, CI workflow files are a developer-experience surface — and developer experience is UX. Applying my lens:

Inline comment placement follows the "label above the field" principle. The rationale comment sits on the line immediately above uses:, not in a sidebar, a commit body, or a separate ADR. This is the equivalent of putting the form label next to the input field — visible at the exact point of decision. A future contributor editing the uses: line cannot miss the comment without actively scrolling past it.

The guard failure message is actionable. "actions/(upload|download)-artifact@v4+ is unsupported on Gitea Actions (act_runner). Pin to @v3. See ADR-014 / #557." follows Nielsen heuristic #9 (Help users recognize, diagnose, and recover from errors): it names what is wrong, explains why it matters, and says what to do. Contrast with a terse exit 1 — same terminal state, zero usable information.

ci-gitea.md fix is a DX win. The old # ← upgraded from v3 comment was actively misleading: it read as if v4 is the canonical target and v3 was a past state to move on from. The new comment is neutral, accurate, and points to the ADR. Any developer following the documentation will now reach the right answer before reaching the wrong one.

No blockers.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist **Verdict: ✅ Approved** ### What I checked This PR contains no UI changes, no frontend components, and no user-facing screens. There is nothing in my design/accessibility domain to review technically. That said, CI workflow files are a developer-experience surface — and developer experience is UX. Applying my lens: **Inline comment placement follows the "label above the field" principle.** The rationale comment sits on the line immediately above `uses:`, not in a sidebar, a commit body, or a separate ADR. This is the equivalent of putting the form label next to the input field — visible at the exact point of decision. A future contributor editing the `uses:` line cannot miss the comment without actively scrolling past it. **The guard failure message is actionable.** `"actions/(upload|download)-artifact@v4+ is unsupported on Gitea Actions (act_runner). Pin to @v3. See ADR-014 / #557."` follows Nielsen heuristic #9 (Help users recognize, diagnose, and recover from errors): it names what is wrong, explains why it matters, and says what to do. Contrast with a terse `exit 1` — same terminal state, zero usable information. **ci-gitea.md fix is a DX win.** The old `# ← upgraded from v3` comment was actively misleading: it read as if v4 is the canonical target and v3 was a past state to move on from. The new comment is neutral, accurate, and points to the ADR. Any developer following the documentation will now reach the right answer before reaching the wrong one. ### No blockers.

marcel referenced this issue from a commit 2026-05-14 10:18:37 +02:00

ci(unit-tests): add negative self-test case to upload-artifact guard

marcel added 1 commit 2026-05-14 10:18:37 +02:00

ci(unit-tests): add negative self-test case to upload-artifact guard ... 22538e32a7

The previous self-test proved the regex catches @v5 (positive case).
This adds a negative case proving @v3 is NOT flagged — guards against
a false-positive that would break every CI run permanently.

Suggested by Sara Holt in review of PR #558.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented 2026-05-14 10:18:41 +02:00

Author

Owner

Copy Link

Fix applied — 22538e32 adds the negative self-test case from Sara's review.

The guard self-test now asserts both directions:

• @v5 is caught (positive case — was already present)
• @v3 is not caught (negative case — new)

Both verified locally before commit.

**Fix applied** — `22538e32` adds the negative self-test case from Sara's review. The guard self-test now asserts both directions: - `@v5` **is** caught (positive case — was already present) - `@v3` is **not** caught (negative case — new) Both verified locally before commit.

marcel merged commit 0abbc147e2 into main 2026-05-14 10:58:20 +02:00

marcel deleted branch worktree-feat+issue-557-upload-artifact-v3-pin 2026-05-14 10:58:20 +02:00

marcel referenced this issue from a commit 2026-05-14 10:58:21 +02:00

ci(unit-tests): add negative self-test case to upload-artifact guard

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#558