ci: fix the CI jobs that fail on every master run by kangmaup · Pull Request #3 · rustasea/framework

kangmaup · 2026-09-24T07:19:47Z

Summary

Every CI run on master has been red since at least 2026-09-17. Test (workspace) and cargo-deny fail on every run, and Assign vheins as reviewer fails on every PR opened from a fork. None of these failures come from the code under test.

This PR fixes all three, with five small commits, one per root cause. No production code changes.

Root causes and fixes

artisan_alias_generates_livewire_app (crates/rustasea/tests/cli.rs)
- Cause: the test expected the generated livewire Cargo.toml to contain features = ["view", "broadcast"]. The scaffold has emitted ["view", "action"] since ab02ac6 (its own manifest.rs test asserts exactly that), and the umbrella has no broadcast feature.
- Fix: align the assertion.
two_factor::enable_confirm_challenge_and_recovery_lifecycle (429 on every full run)
- Cause: the login limiter allows 5 attempts per minute per username|ip, and its registry is a process-wide static. Test requests carry no ConnectInfo, so every login resolves to 0.0.0.0. The lifecycle test logs in as ada@example.com five times, and auth_log logs in as the same user within the same minute.
- Fix: start_challenge attaches a distinct loopback ConnectInfo to each login. Test-only.
workers_renders_heartbeats (left: 2, right: 1)
- Cause: this test and workers_flags_stale_heartbeats_inactive both clear, stamp, and read the process-wide heartbeat registry in parallel.
- Fix: serialize them with a test-only lock, the same pattern as ENV_LOCK.
cargo-deny (cargo deny check exits 1)
- Cause: the workflow set RUSTC_WRAPPER: sccache for every job, but only quality, test, and msrv install sccache. cargo deny runs cargo metadata, which invokes rustc -vV through the wrapper:
```
`cargo metadata` exited with an error: error: could not execute process `sccache .../rustc -vV` (never executed)
```
  cargo audit passes because it only reads Cargo.lock.
- Fix: RUSTC_WRAPPER and SCCACHE_GHA_ENABLED move to the three jobs that install sccache.
- Also: .cargo/config.toml drops rustc-wrapper = "/home/vheins/.cargo/bin/sccache". That path only exists on one machine, so every other checkout fails its first cargo command with the same error, and the deny job would have fallen back to it. Local sccache is now opt-in, documented in CONTRIBUTING.
Assign vheins as reviewer ("Resource not accessible by integration")
- Cause: a pull_request run from a fork only gets a read-only token.
- Fix: the workflow now triggers on pull_request_target. That is safe here because the job never checks out or runs PR code; it only calls the REST API. GitHub reads pull_request_target workflows from the base branch, so this fix takes effect after merge.

Testing

cargo test --workspace --no-fail-fast: 1938 passed, 0 failed. On c501fdd, 3 tests failed.
The two formerly failing suites, repeated (both failed on every run before this PR):
- cargo test -p rustasea-app --bin rustasea-app: 146/146, twice.
- cargo test -p rustasea-queue-dashboard --lib: 25/25 runs.
cargo deny check with no RUSTC_WRAPPER set: passes. On c501fdd it fails with the error above. I also reproduced the CI failure itself with RUSTC_WRAPPER=sccache cargo deny check on a machine without sccache.
cargo xtask ci, cargo audit, and cargo +1.88.0 check --workspace pass, all without any RUSTC_WRAPPER override.
Workflow YAML parsed: RUSTC_WRAPPER is set on exactly the jobs that run sccache-action.

Notes

If you use sccache locally, export RUSTC_WRAPPER=sccache in your shell, or set build.rustc-wrapper in ~/.cargo/config.toml, since the repo config no longer sets it.
No CHANGELOG entry, following the existing practice for CI and build changes (TASK-113 to TASK-116 have none).
fix(storage): make s3 disks work with S3-compatible endpoints (RustFS, MinIO) and AWS_* env #2 (S3 storage) is red only because of these failures, so it should go green once this lands and it is rebased.

`artisan_alias_generates_livewire_app` still expected the generated `Cargo.toml` to enable `features = ["view", "broadcast"]`. The scaffold has emitted `["view", "action"]` since ab02ac6 (its own `manifest.rs` unit test asserts exactly that), and the umbrella crate has no `broadcast` feature, so the old expectation could never pass and kept the CI test job red.

`enable_confirm_challenge_and_recovery_lifecycle` failed with `429` on every full run. The `login` limiter allows five attempts per minute per `username|ip`, and its registry is a process-wide static. Test requests carry no `ConnectInfo`, so every login resolves to the `0.0.0.0` peer: the lifecycle test logs in as `ada@example.com` five times, and `auth_log` logs in as the same user through the same limiter within the same minute. `start_challenge` now attaches a distinct loopback `ConnectInfo` to each login, so the throttle no longer couples otherwise independent tests. Production code is unchanged.

…egistry `workers_renders_heartbeats` and `workers_flags_stale_heartbeats_inactive` both clear, stamp, and read `rustasea_queue::heartbeat`'s process-wide registry. Run in parallel, one test's `clear`/`stamp_at` lands in the middle of the other's snapshot (`left: 2, right: 1`), which failed the CI test job on every run. A test-only lock now serializes the two.

The `cargo-deny` job failed on every run: the workflow set `RUSTC_WRAPPER: sccache` for all jobs, but only `quality`, `test`, and `msrv` install sccache. `cargo deny` runs `cargo metadata`, which invokes `rustc -vV` through the wrapper and fails with "could not execute process `sccache ...`". `cargo audit` passed because it only reads `Cargo.lock`. `RUSTC_WRAPPER` and `SCCACHE_GHA_ENABLED` now live on the three jobs that install sccache. `.cargo/config.toml` also drops its `rustc-wrapper = "/home/vheins/.cargo/bin/sccache"`: that path exists on a single machine, so every other checkout failed its first cargo command, and the `deny` job would have fallen back to it once the env var was gone. Local sccache is now opt-in, as documented in CONTRIBUTING.

`Assign vheins as reviewer` failed on every PR opened from a fork with "Resource not accessible by integration": a `pull_request` run from a fork only gets a read-only `GITHUB_TOKEN`, whatever the workflow's `permissions:` block asks for, so `requestReviewers` is refused. `pull_request_target` runs in the base repository's context with the declared `pull-requests: write` permission. It is safe for this workflow because the job never checks out or executes the PR's code; it only calls the REST API. GitHub reads `pull_request_target` workflows from the base branch, so the fix takes effect once it is merged.

`config_renders_json` loaded the process-relative `config/app`, but tests run with the crate root as their working directory, so the file was never found and `app_env` could only come from `APP_ENV`. The test passed only when a `bootstrap::providers` test happened to have `APP_ENV` set at that moment: usually true with many test threads, rarely true on the 4-core CI runners, where it failed the test job. It also fails when run on its own. The helper now locates the workspace `config/app.toml` from `CARGO_MANIFEST_DIR`, the same approach as `routes::resources_root`.

`generated_app_compiles_for_every_variant` generates each starter kit and runs `cargo check --offline` on it. The react, vue, and svelte kits depend on WASM-side crates (`async-tungstenite`, `any_spawner`, `gloo-net`, ...) that the host `cargo test` build never downloads, so on a fresh runner the offline check failed with "attempting to make an HTTP request, but --offline was specified". Developer machines rarely notice because their cargo cache is already warm. A `cargo fetch` step now downloads every crate in `Cargo.lock`, for every target, before the tests run. Reproduced locally by moving those three crates out of the cargo cache (the gate fails exactly as on CI) and fixed by `cargo fetch`, which re-downloads them from the lockfile.

Without `--no-fail-fast`, `cargo test` stops at the first failing test binary, so each broken test hid the ones behind it: fixing the scaffold assertion revealed the two-factor test, fixing that revealed the tinker and scaffold compile-gate failures, one CI round trip at a time. Running the whole suite on every push reports all failures at once.

`track_records_success_with_original_sql` failed intermittently on CI (`left: 0, right: 1`; about one run in 40 locally with 4 test threads). The three `profile` tests share the process-wide `RECORDER` slot, and `track_without_recorder_passes_through` or `recorder_registry_round_trips` could clear or replace the capturing recorder in the middle of its `track` call. A test-only tokio mutex now serializes the three (tokio, so the async tests can hold it across `.await`), and the capturing test only counts events for its own unique SQL, so a query tracked by a concurrent DB test cannot be mistaken for its event. 200 consecutive runs pass.

- Domain: CI workflows (sccache/cargo-deny, cargo fetch, --no-fail-fast), auth throttle ConnectInfo, test serialization (ORM recorder lock, queue heartbeat lock), scaffold manifest features - Findings: 1 HIGH, 1 LOW - Blocker: production serve path never attaches ConnectInfo; login throttle key is username|0.0.0.0

vheins

@kangmaup One blocker and one doc fix. Details in the inline comments.

HIGH - The two-factor test injects a ConnectInfo peer that the production server never attaches, so the test is green while the production root cause stays live: axum::serve(listener, router) in crates/rustasea-app/src/main.rs:56 inserts no connection info, the login limiter keys every client as <username>|0.0.0.0, and any client can lock any username out with five failed attempts per minute. Fix: serve with router.into_make_service_with_connect_info::<SocketAddr>() in main.rs and in the scaffold template, then assert a real peer in a regression test.
LOW - CONTRIBUTING.md:129 still documents the test job as cargo test --workspace; the workflow now runs cargo fetch + cargo test --workspace --no-fail-fast (.github/workflows/ci.yml:74-79).

vheins · 2026-09-24T09:49:09Z

-    let (status, headers, response) =
-        call(app_with_guard(guard.clone()), post("/login", &body)).await;
+    let mut request = post("/login", &body);
+    request.extensions_mut().insert(unique_peer());


[HIGH] Login throttle collapses to one global bucket in production

Problem
This test injects a ConnectInfo peer that the production server never attaches, so the test goes green while the same root cause stays live for every real request. crates/rustasea-app/src/main.rs:56 serves with axum::serve(listener, router), which inserts no ConnectInfo extension. peer_ip therefore falls back to UNKNOWN_PEER = "0.0.0.0", and the login limiter bucket key is <normalized-username>|0.0.0.0 for every client: five failed attempts from anywhere lock a username out for the window, trusted_proxies never engages, and authentication_log.ip_address is always 0.0.0.0.

Evidence

crates/rustasea-app/src/main.rs:56 - axum::serve(listener, router), and no into_make_service_with_connect_info anywhere in the workspace

crates/rustasea-app/src/routes/auth.rs:144,179 - Option<ConnectInfo<SocketAddr>> is always None on the production path, so peer_ip hits the fallback

crates/rustasea-app/src/routes/auth.rs:80,471-483 - UNKNOWN_PEER = "0.0.0.0" becomes the throttle peer and the audit IP

crates/rustasea-app/src/routes/tests/auth_log.rs:163 - asserts Some("0.0.0.0"), encoding the broken behaviour as expected

crates/rustasea-scaffold/src/templates/core.rs:462 - generated apps ship the same axum::serve(listener, router) call

Suggestion
Serve with axum::serve(listener, router.into_make_service_with_connect_info::<SocketAddr>()) in crates/rustasea-app/src/main.rs and in the scaffold MAIN_RS template, then add a regression test that asserts a non-loopback peer reaches the throttle key and the audit row.

Agreed: the test change was hiding a live production bug. Fixed in 7d0eb24 (rustasea-app) and 13e8605 (scaffold template):

main now serves through serve(), which uses router.into_make_service_with_connect_info::<SocketAddr>(). The generated main.rs does the same.

served_app_logs_the_connection_peer starts the app on a real TCP listener through that same serve(), logs in over the socket, and asserts the audit row records 127.0.0.1. Without the fix it fails with 0.0.0.0.

lockout_is_scoped_to_the_client_peer: five failed attempts from 203.0.113.21 get that client a 429, while 203.0.113.22 still logs in as the same user.

successful_login_records_row now asserts a non-loopback peer (203.0.113.10) reaches the audit row instead of 0.0.0.0.

The two-factor test keeps attaching a peer, since every production request now carries one. The remaining trusted_proxies problem (the CIDRs are never matched and the leftmost X-Forwarded-For hop is used) is a separate fix; I'll send it as its own PR.

vheins · 2026-09-24T09:49:09Z

   Formatting violations fail the build: run `cargo fmt --all` before pushing.

-   CI uses [sccache](https://github.com/mozilla/sccache) (`mozilla-actions/sccache-action`) to cache Rust compilation across the `quality`, `test`, and `msrv` jobs.
+   CI uses [sccache](https://github.com/mozilla/sccache) (`mozilla-actions/sccache-action`) to cache Rust compilation across the `quality`, `test`, and `msrv` jobs. Locally it is opt-in: install sccache and export `RUSTC_WRAPPER=sccache` (or set `build.rustc-wrapper` in your own `~/.cargo/config.toml`).


[LOW] CI job table still documents the old test command

Problem
This line documents the new opt-in sccache behaviour, but the job table a few lines above still lists the test job as cargo test --workspace, while the job now runs cargo fetch followed by cargo test --workspace --no-fail-fast.

Evidence

CONTRIBUTING.md:129 - the test row still reads cargo test --workspace

.github/workflows/ci.yml:74-79 - cargo fetch + cargo test --workspace --no-fail-fast

Suggestion
Update the test row to list both commands so the documented CI contract matches the workflow.

Fixed in 80d9bee: the test row now reads cargo fetch, then cargo test --workspace --no-fail-fast. I updated the same description in the README's CI paragraph.

…t IP `main` served the router with `axum::serve(listener, router)`, which attaches no `ConnectInfo`, so every request's peer resolved to the `0.0.0.0` fallback. The `login` limiter keyed every client as `<username>|0.0.0.0` (five failed attempts from anywhere locked a username out), `trusted_proxies` could never engage, and the authentication log recorded `0.0.0.0` for everyone. Serving moves into `serve()`, which uses `into_make_service_with_connect_info::<SocketAddr>()`. Regression tests: - `served_app_logs_the_connection_peer` logs in over a real TCP connection through `serve()` and asserts the audit row records `127.0.0.1`; it failed with `0.0.0.0` before this change; - `lockout_is_scoped_to_the_client_peer` locks one client out after five failed attempts while another client still logs in as the same user; - `successful_login_records_row` now asserts a non-loopback peer reaches the audit row instead of encoding the `0.0.0.0` fallback.

The generated `main.rs` had the same `axum::serve(listener, router)` call, so scaffolded apps also lost the client IP on every request. It now serves `router.into_make_service_with_connect_info::<SocketAddr>()`, matching `rustasea-app`; the generated-app compile gate checks every variant.

The CONTRIBUTING job table and the README still described the `test` job as `cargo test --workspace`; it now runs `cargo fetch` and then `cargo test --workspace --no-fail-fast`.

vheins

Konflik terdeteksi di PR ini. @kangmaup tolong selesaikan konflik dengan base branch terlebih dahulu sebelum review dapat dilanjutkan.

vheins

Konflik terdeteksi di PR ini. @kangmaup tolong selesaikan konflik dengan base branch terlebih dahulu sebelum review dapat dilanjutkan.

# Conflicts: # .github/workflows/ci.yml # CONTRIBUTING.md # README.md

kangmaup added 9 commits September 24, 2026 13:45

vheins self-requested a review September 24, 2026 08:26

vheins requested changes Sep 24, 2026

View reviewed changes

kangmaup added 3 commits September 24, 2026 17:09

docs: document the CI test job's fetch and no-fail-fast steps

80d9bee

The CONTRIBUTING job table and the README still described the `test` job as `cargo test --workspace`; it now runs `cargo fetch` and then `cargo test --workspace --no-fail-fast`.

kangmaup requested a review from vheins September 24, 2026 10:48

vheins requested changes Sep 24, 2026

View reviewed changes

vheins self-requested a review September 24, 2026 11:17

vheins requested changes Sep 24, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/master' into fix/ci-green

7dae988

# Conflicts: # .github/workflows/ci.yml # CONTRIBUTING.md # README.md

kangmaup requested a review from vheins September 24, 2026 11:25

vheins merged commit 194a8fc into rustasea:master Sep 24, 2026
5 checks passed

Navigation Menu

ci: fix the CI jobs that fail on every master run - #3

ci: fix the CI jobs that fail on every master run#3
vheins merged 14 commits into
rustasea:masterfrom
kangmaup:fix/ci-green

kangmaup commented Sep 24, 2026

Uh oh!

vheins left a comment

Uh oh!

vheins Sep 24, 2026

Uh oh!

kangmaup Sep 24, 2026

Uh oh!

vheins Sep 24, 2026

Uh oh!

kangmaup Sep 24, 2026

Uh oh!

vheins left a comment

Uh oh!

vheins left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kangmaup commented Sep 24, 2026

Summary

Root causes and fixes

Testing

Notes

Uh oh!

vheins left a comment

Choose a reason for hiding this comment

Uh oh!

vheins Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

kangmaup Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vheins Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

kangmaup Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vheins left a comment

Choose a reason for hiding this comment

Uh oh!

vheins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants