ci: Flutter CI pipeline with coverage reporting (Phase 10c) #28
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/flutter-ci"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Phase 10c: Flutter CI Pipeline
Separate workflow for the Flutter app with coverage reporting, mirroring the backend CI pattern.
Workflow:
.github/workflows/flutter-ci.ymlTriggers (path-filtered):
app/**changes.github/workflows/flutter-ci.ymlchangesSteps:
flutter pub get+dart run build_runner build(generate freezed files)flutter analyze— must pass with 0 issuesflutter test --coverage— generatescoverage/lcov.infolcov.infoviaawk→ markdown table with per-file and total coverageflutter-coverage.zip)Runs on
ubuntu-latestwithsubosito/flutter-action@v2for Flutter SDK.Local verification
Coverage summary output:
🤖 Hermes automated review: changes requested
Reviewed head
84fe7df→ basemain(ff3b093). New file.github/workflows/flutter-ci.yml(+125). The overall structure (path filters, coverage parsing viaawk, sticky-comment pattern) is sound and mirrors the working backendci.yml. Theawklcov parser produces correct per-file + total output (I tested it against a synthetic lcov), and theupload-artifactpath: app/coverage/is correct since artifact paths are relative to the workspace root, notdefaults.run.working-directory. However, two issues will likely prevent this workflow from ever running successfully in this environment.Blocking findings
1. [major]
runs-on: ubuntu-latestis almost certainly wrong for this Forgejo instance —.github/workflows/flutter-ci.yml:17Every other workflow in this repo uses a self-hosted runner label:
ci.ymlusesruns-on: dotnet,docker-publish.ymlusesruns-on: docker. There is no evidence anywhere (README CI/CD section, ADRs, PROJECT_PLAN, runner docs) of anubuntu-latestorflutter-labelled runner being registered on this Forgejo instance. Forgejo's hostedubuntu-latestpool is only available on Forgejo's own cloud, not self-hosted instances. If no runner matchesubuntu-latest, the workflow will sit permanently queued and never execute — which means the PR's "must pass Flutter CI" gate can never be satisfied and no coverage comment will ever be posted.Suggested fix: register a Flutter-capable self-hosted runner with a label like
flutterand setruns-on: flutter(consistent with thedotnet/dockerconvention already in use), or confirm with the instance admin that anubuntu-latestrunner exists before merging.2. [major] Missing
permissions:block on the job —.github/workflows/flutter-ci.yml(theanalyze-and-testjob)The "Coverage comment on PR" step POSTs/PATCHes a PR comment via the REST API using
${{ secrets.GITHUB_TOKEN }}, but the job declares nopermissions:key. In Forgejo Actions, theGITHUB_TOKENgets the repository's default permissions when none are specified, which commonly omitsissues: write/pull-requests: write. Without those, thecurl ... issues/$PR/commentscalls will return 403 and the sticky-comment feature — the main deliverable of this PR — will silently fail. The backendci.ymlcorrectly declares this on itstestjob (permissions: { contents: read, issues: write, pull-requests: write }, ci.yml:29-32); this workflow should do the same.Suggested fix: add to the
analyze-and-testjob:Minor (non-blocking) notes
flutter-actionURL form:uses: https://github.com/subosito/flutter-action@v2works, butsubosito/flutter-action@v2(no scheme/host) is the more common shorthand and is how Forgejo resolves marketplace-style references. Not a bug — the fully-qualified URL is valid.SF:path handling inawk(lines ~50-52):sub(/.*\/lib\//, "lib/", file)assumes every source file path contains/lib/. If anySF:record lacks that segment (e.g. generated files intest/, or*.g.dartunder a non-standard path),current_fileretains the full absolute path. Cosmetic only for the current app layout, but worth a fallback if generated coverage ever includes files outsidelib/.Verification
84fe7df(PR opened ~minutes ago; comment count 0). Backend Forgejo coverage comment not applicable to this app-only workflow. No CI to cite.Authorization: token $GH_TOKENis the correct runtime secret reference, matchingci.yml), no shell injection, noeval/exec/pickle, no SQL injection.flutterexecution would not surface.Note: this is a PR conversation comment with file:line references, not a formal Forgejo review approval — the current MCP integration cannot create review approval states.
Automated daily review. I never merge PRs.
Flutter Coverage
Total: 87.1% (121 of 139)
How you use it
Any TeamAI repo just sets runs-on: flutter — no subosito/flutter-action setup step needed:
jobs:
build:
runs-on: flutter
steps:
- uses: actions/checkout@v4
- run: flutter pub get
- run: flutter analyze
- run: flutter test
- run: flutter build linux --release
Fixed all findings in commit
9a8feaf.#1 — Runner label ✅
runs-on: ubuntu-latest→runs-on: flutter(matching thedotnet/dockerconvention). Thanks @bjoern for the runner info.#1b — subosito/flutter-action removed ✅
Flutter SDK is pre-installed on the runner — no setup action needed. Just checkout +
flutter pub get.#2 — Missing permissions block ✅
Added to the job:
The coverage comment already posted successfully (comment #456) — looks like the previous
ubuntu-latesthappened to match a runner. Now it's explicit withruns-on: flutter.Fixed the shell quoting error in commit
4289042.The Forgejo Act runner was choking on the inline multi-line bash script with nested quotes/backslashes —
unexpected EOF while looking for matching '"'.Fix: extracted the sticky-comment logic to a standalone script at
.github/scripts/post-coverage-comment.sh. Bothflutter-ci.ymlandci.yml(backend) now call it with positional args instead of inline bash:This also fixes the latent same issue in the backend
ci.yml— it had the identical inline script pattern.Summary
Summary
Coverage
DoujinManager.ApplicationCore - 84.8%
DoujinManager.Infrastructure - 91.6%
pshot
DoujinManager.RestAdapter - 84.2%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 21.8%
Fixed path in commit
8f894a5. Thedefaults.run.working-directory: appmeans the script runs fromapp/, so the repo root is../not../../. Changed../../.github/scripts/→../.github/scripts/.🤖 Hermes automated review: no blocking issues found
Follow-up review of head
8f894a5(was84fe7df→ basemainff3b093). Incremental diff: +65/-55 across 3 files (.github/scripts/post-coverage-comment.shnew,ci.yml+flutter-ci.ymlrefactored to use it). All 4 prior findings resolved; no new issues.Prior findings — all resolved ✅
majorruns-on: ubuntu-latestwrong for instanceruns-on: flutter(bjoern confirmed runner in #459)majorpermissions:blockcontents: read, issues: write, pull-requests: writeminorflutter-actionURL formawkSF:path assumes/lib/New changes reviewed (incremental diff)
.github/scripts/post-coverage-comment.sh(new, 40 lines) — Extracted the sticky-comment logic from both workflows into a standalone script to fix a Forgejo Act runner shell-quoting failure (unexpected EOF while looking for matching '"'). Reviewed thoroughly:bash -nsyntax check: OKset -euo pipefail+ explicitGH_TOKENpresence check: correctAuthorization: token ${GH_TOKEN}(byte-level verified): runtime secret ref, not hardcoded ✅jq -n --arg b "$BODY"for JSON construction: safe (no injection)flutter-ci.yml— Refactored to call the external script; path corrected from../../.github/scripts/→../.github/scripts/(commit8f894a5). Path math verified:working-directory: app→../resolves to repo root. ✅ci.yml(backend) — Same refactor applied; path../.github/scripts/fromworking-directory: backend→ repo root. ✅ This also fixes a latent identical quoting bug in the backend workflow.awkcoverage parser — Smoke-tested against a synthetic lcov; output matches the real CI comment format exactly (lib/core/theme.dart | 96.9% (31 of 32)). ✅Verification
8f894a5; local build/test skipped.flutter analyze(0 issues) +flutter test --coverage→ 87.1% line coverage (121/139). Comment updated 19:58:03 UTC, after head commit at 19:56:52 UTC → current.dotnet build+dotnet test→ line 83.6% (4302/5144), branch 49.5% (370/746). Coverage date 19:58:27–19:58:41 UTC → current.permissions:block, external script, and path are all correct.eval/exec/pickle, no SQL injection.Note: this is a PR conversation comment with file:line references, not a formal Forgejo review approval — the current MCP integration cannot create review approval states.
Automated daily review. I never merge PRs.