Skip to content

ci: run CodeQL on every PR, guard the released schemas, and fix two Windows flakes - #284

Merged
InstaZDLL merged 5 commits into
mainfrom
chore/ci-and-flakes
Oct 17, 2026
Merged

InstaZDLL merged 5 commits into
mainfrom
chore/ci-and-flakes

Conversation

@InstaZDLL

@InstaZDLL InstaZDLL commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Summary

Five small changes that each stood alone, grouped into one PR on purpose. One commit per subject.

1. CodeQL analyses every pull request

The ruleset on main requires a CodeQL result, and codeql.yml's pull_request trigger was filtered to the Rust paths. A branch touching only webapp/ or docs/ never started an analysis, so it finished green and stayed BLOCKED waiting for one: #250, #251, #254, #262, #278 and #279. The filter is gone from pull_request (kept on push, where nothing waits on it). On a client-only or docs-only PR the analysis finds nothing new, which is exactly the answer the requirement waits for.

2. A release bump can no longer forget its schema

RELEASED_SCHEMAS in tests/migrations.rs had to be extended by hand at every tag, and nothing noticed when it was not. The crate version is the tag a release ships as, so a new test, the_version_being_shipped_has_its_schema_recorded, requires a line for v{CARGO_PKG_VERSION}. It holds between releases and fails exactly on the PR that bumps version, before the tag exists, while the baseline to add is still simply the newest migration. The failure message says what to add.

3. The upload sweep test waits for the row too (closes #280)

The sweep removes the staging file first and the session row second, by design. The test waited on the file and then counted the row once, and twice on the Windows runner it read the gap between the two (left: 1, right: 0). It now waits until both are gone.

4. The start-up tool check gets thirty seconds (closes #281)

ffmpeg -version overran its five-second budget on the Windows runner under a parallel test run (#279's first run). The check catches a missing or hung binary, which thirty seconds still does; a healthy boot answers long before. Same shape as #240.

5. Rust 1.98 for the MSRV and the release image (closes #283)

CI and development compile with stable (1.98.1) while the release image was built by rust:1.94-bookworm. rust-version, the Dockerfile builder and the README badge now say 1.98, so the compiler that tests a PR is the one that builds what ships.

How I tested

  • cargo fmt --check, cargo clippy --all-targets --all-features -D warnings on 1.98.1, and the whole cargo test --all-features: every target green.
  • The new migrations test, against a Cargo.toml bumped to 2.0.0-beta.2: it fails with Cargo.toml is at v2.0.0-beta.2, and RELEASED_SCHEMAS has no line for it. Restored afterwards.
  • The sweep race cannot be provoked on demand, so that change rests on reading the order of the two deletions in sweep_expired_sessions; the test still passes, and still fails if the sweep never runs (the file assertion is unchanged).
  • The CodeQL change is proved by this PR's own checks, and better by the next docs-only PR not being BLOCKED.
  • The local CodeRabbit review: no findings.

Not in this PR

Summary by CodeRabbit

  • Compatibilité
    • La compilation et l’exécution du projet nécessitent désormais Rust 1.98 ou une version ultérieure.
  • Améliorations
    • La vérification de la disponibilité de FFmpeg et FFprobe au démarrage tolère désormais un délai plus long, jusqu’à 30 secondes, avant de signaler un dépassement de délai.
  • Vérifications
    • Les contrôles de sécurité CodeQL s’exécutent désormais aussi pour les demandes de fusion qui ne modifient que l’application Web ou la documentation.

The ruleset on main requires a CodeQL result, and the pull_request trigger was filtered to the Rust paths. A branch touching only webapp/ or docs/ never started an analysis, so it finished green and stayed BLOCKED waiting for one: #250, #251, #254, #262, #278 and #279.

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
RELEASED_SCHEMAS had to be extended by hand at every tag, and nothing noticed when it was not. The crate version is the tag a release ships as, so a test now requires a line for it: the PR that bumps the version fails until it records the schema, while the baseline is still simply the newest migration.

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
The sweep removes the staging file first and the session row second, and the test waited on the file only before counting the row once. On the Windows runner it landed between the two, twice.

Closes #280

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
ffmpeg -version overran five seconds on the Windows runner under a parallel test run. The check exists to catch a missing or hung binary, which thirty seconds still does; a healthy boot answers long before.

Closes #281

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
CI and development compile with stable (1.98.1) while the release image was built by rust:1.94. Both now say 1.98, so the compiler that tests a PR is the one that builds what ships.

Closes #283

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
@github-actions github-actions Bot added type: ci CI/CD changes scope: server Server core (Rust) scope: deps Dependencies scope: tooling Build / dev tooling scope: docs Docs, README, assets scope: streaming Streaming, transcoding, FFmpeg size: s 10-50 lines labels Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: InstaZDLL/waveflow-server/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 07e742de-ea16-4ee5-8151-7f40af04f82b
📥 Commits

Reviewing files that changed from the base of the PR and between 4341d69 and 672fad4.

📒 Files selected for processing: 7
  • .github/workflows/codeql.yml
  • Cargo.toml
  • Dockerfile
  • README.md
  • src/media.rs
  • tests/migrations.rs
  • tests/uploads.rs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

La PR modifie le déclenchement de CodeQL, met à jour la version minimale de Rust à 1.98, augmente le délai de vérification de FFmpeg et FFprobe, et ajuste deux tests.

Changes: Déclenchement de CodeQL

Layer / File(s) Summary
Déclencheur des pull requests
.github/workflows/codeql.yml
Le déclencheur pull_request s’applique à toutes les modifications des branches ciblant main. Le filtre de chemins du déclencheur push reste en place.

Version de Rust

Layer / File(s) Summary
Mise à jour de la version de Rust
Cargo.toml, Dockerfile, README.md
La version minimale déclarée et les prérequis passent à Rust 1.98. L’image de compilation utilise rust:1.98-bookworm.

Délai de vérification des outils média

Layer / File(s) Summary
Contrôle de démarrage
src/media.rs
Le contrôle de FFmpeg et FFprobe utilise un délai maximal de 30 secondes. Un dépassement produit toujours une erreur de timeout.

Vérification du schéma publié

Layer / File(s) Summary
Test de version publiée
tests/migrations.rs
Un test vérifie que la version courante du crate figure dans RELEASED_SCHEMAS et signale l’absence du schéma correspondant.

Nettoyage des transferts abandonnés

Layer / File(s) Summary
Attente du nettoyage complet
tests/uploads.rs
Le test attend, sous le même délai, la disparition du fichier de staging et de la ligne de session. Il vérifie ensuite que la ligne a été supprimée.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 672fa

The reviewed changes have no substantiated merge-blocking risk; the version check is database-free and its synchronous test form is appropriate.

Security architecture risk: 🔵 Low · up to 672fa

The change expands pull-request security scanning without changing declared permissions or application access controls. No introduced security weakness was established, but repository enforcement settings and effective permissions for fork runs remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The security-relevant exposure change is additional eligible pull-request runs under the existing job authority, not a new application or deployment role. The declared authority is visible in source, but effective permissions and secret exposure for fork runs cannot be established from the supplied repository evidence.

Trust Boundaries and Controls

  • observed — The existing workflow separates concurrency by workflow and ref, cancels superseded pull-request runs, and sequences checkout before initialization and analysis. The jobs retain timeouts, distinct language categories and normal failure propagation. These controls counter a workflow-level cross-PR result-mixing concern, but do not prove remote required-check binding or upload behavior during cancellation.
Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Out of Scope Changes check Warning Le PR ajoute aussi une modification du déclencheur CodeQL et le test the_version_being_shipped_has_its_schema_recorded dans tests/migrations.rs. Ces changements ne sont pas liés aux objectifs des … Retirer ces deux changements de ce PR, ou fournir des issues directement liées qui définissent leurs objectifs de codage.
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (4 skipped: 4… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed Le titre décrit clairement les principaux changements : l’exécution de CodeQL sur les PR, la protection des schémas publiés et la correction de deux problèmes intermittents.
Description check Passed La description couvre le contexte, les changements, les tests réalisés et les éléments exclus du PR. Elle n’utilise pas exactement les rubriques « Changes » et « Test plan » et ne reprend pas l’attest…
Linked Issues check Passed Les trois objectifs de codage sont couverts. Pour [#280], tests/uploads.rs attend, sous la même échéance, la disparition du fichier et de la ligne upload_session, puis vérifie les deux conditions.…
Full details: Description check

Explanation

La description couvre le contexte, les changements, les tests réalisés et les éléments exclus du PR. Elle n’utilise pas exactement les rubriques « Changes » et « Test plan » et ne reprend pas l’attestation finale du modèle, mais elle reste complète et pertinente.

Full details: Linked Issues check

Explanation

Les trois objectifs de codage sont couverts. Pour [#280], tests/uploads.rs attend, sous la même échéance, la disparition du fichier et de la ligne upload_session, puis vérifie les deux conditions. Pour [#281], check_tool utilise TOOL_CHECK_TIMEOUT de 30 secondes pour ffmpeg et ffprobe. Pour [#283], Cargo.toml déclare rust-version = "1.98" et le builder du Dockerfile utilise rust:1.98-bookworm. Les vérifications rapportées incluent Clippy avec -D warnings et les tests sur Rust 1.98.1.

Full details: Out of Scope Changes check

Explanation

Le PR ajoute aussi une modification du déclencheur CodeQL et le test the_version_being_shipped_has_its_schema_recorded dans tests/migrations.rs. Ces changements ne sont pas liés aux objectifs des issues directement liées [#280], [#281] et [#283]. Le fait que le PR les regroupe volontairement ne crée pas un lien de codage avec ces issues.

Resolution

Retirer ces deux changements de ce PR, ou fournir des issues directement liées qui définissent leurs objectifs de codage.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (4 skipped: 4 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added type: ci CI/CD changes and removed type: ci CI/CD changes labels Oct 17, 2026
@InstaZDLL
InstaZDLL merged commit 391bf6c into main Oct 17, 2026
15 checks passed
@InstaZDLL
InstaZDLL deleted the chore/ci-and-flakes branch October 17, 2026 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: deps Dependencies scope: docs Docs, README, assets scope: server Server core (Rust) scope: streaming Streaming, transcoding, FFmpeg scope: tooling Build / dev tooling size: s 10-50 lines type: ci CI/CD changes

Projects

None yet

1 participant