Proposal: CI baseline (correctness + security) gate for the repo

[nopoz]

Prerequisites

• I searched open issues and this has not already been proposed.
• I searched discussions and this is not already being debated there.
• This is a concrete, actionable proposal — not a vague "it would be nice if..." request.

Area

Testing / CI

Problem or Motivation

There is no correctness or security CI on main today. The only workflows present are the description linters (issue-description-check.yml, pr-description-check.yml), so nothing automatically catches broken or low-quality submissions before review.

The open-PR backlog is large and skews big, with clear duplicates and a lot of AI-generated noise. Reviewing all of it by hand with no gate is a losing battle, and there is nothing today that lets the backlog be filtered retroactively or that blocks the next regression from landing.

This issue is a single consolidated proposal to agree on CI direction before any PRs, in line with CONTRIBUTING.md's guidance to open an issue first for larger changes. The aim is a minimal, maintainer-owned gate; nothing here would land without maintainer sign-off.

Proposed Solution

A minimal gate that runs on every PR, in two parts plus optional scaffolding:

1. Correctness checks

• Python syntax: python -m compileall over the app sources (app.py core routes src services scripts tests) — catches syntax errors without installing dependencies.
• JS syntax: node --check over static/app.js and static/js/** (frontend is plain JS, no build step; vendored static/lib is skipped).
• pytest on the existing suite, informational at first. The suite has known flaky / environment-dependent failures, so it should run non-blocking and be promoted to a required check only once it is reliably green.

This is exactly what #1015 (vdmkenny) already implemented: a single ci.yml with those three jobs, pytest already marked continue-on-error. It reads clean and well-scoped; it is now closed, but it is the natural correctness base if reopened.

2. Security scanning

• Secret scanning, dependency scanning, container/image scanning, and static code scanning.
• ci: security scanning suite and governance (consolidates #305-310) #1314 consolidates the earlier per-scanner PRs (ci: add secret scanning with gitleaks #305-310) into one bundle and is still open. It is meant to layer on top of the correctness base so the workflow files compose instead of clobbering.

3. Triage scaffolding (optional, later wave)

• A label set plus issue/PR templates that force a description and a "what did you test" note, to cut future noise at the source.
• 👉 [START HERE] Add GitHub issue & PR templates + labels + CONTRIBUTING.md #112 and ci: add basic GitHub Actions foundation #141 (both closed) cover this and overlap heavily (same templates, labels.yml, CONTRIBUTING.md). ci: add basic GitHub Actions foundation #141 also bundles its own ci.yml, which would compete with the correctness workflow above, so the cleaner scaffolding-only one would be championed and the duplicate dropped. Lower priority than the gate itself and easy to defer.

Suggested sequencing

1. Correctness baseline first (reopen or resubmit feat(ci): Add CI workflow (syntax + tests) #1015).
2. Security bundle (ci: security scanning suite and governance (consolidates #305-310) #1314) rebased on top, confirmed green.
3. Branch protection enabled with the green checks required (initially the syntax jobs plus the security scans; pytest stays informational until the suite is reliably green, then gets promoted). That admin step is what makes the gate actually hold; without it the checks run but nothing blocks a failing merge.
4. Scaffolding (labels/templates) after, if wanted.

Open questions for the maintainer

• A yes/no (or edits) on the scope and ordering above.
• Whether to reopen the closed PRs or resubmit fresh.
• Preferred merge path for workflow files (web UI merges use the maintainer account's permissions and sidestep the missing workflow token scope, so they work today with nothing to set up).
• Confirmation that Actions is enabled under Settings > Actions > General, otherwise the workflows merge but never run.

Alternatives Considered

Writing a single all-in-one CI workflow from scratch instead of building on existing PRs — ruled out to avoid throwing away review already done on #1015 and the scanner work, and to keep each piece small and revertible. Doing nothing and reviewing by hand — ruled out as unscalable given the backlog size.

Prior Art / Related Issues

• feat(ci): Add CI workflow (syntax + tests) #1015 — correctness CI (closed), proposed base.
• ci: security scanning suite and governance (consolidates #305-310) #1314 — security scanning bundle (open), consolidates ci: add secret scanning with gitleaks #305-310.
• 👉 [START HERE] Add GitHub issue & PR templates + labels + CONTRIBUTING.md #112 / ci: add basic GitHub Actions foundation #141 — issue/PR templates, labels, CONTRIBUTING scaffolding (both closed).
• Discussion Who can help? #551 — maintainer call for help, which lists PR triage and security review as priority areas.

Are you willing to implement this?

Yes — I can open a PR

[nickorlabs]

CI Baseline Proposal

Objective

Introduce a mandatory CI gate for all PRs targeting main that enforces correctness, security, and style consistency. This will catch regressions early and prevent future issues like #2267.

Proposed Checks

Check	Tool	Purpose
Lint / style	ruff or flake8	Enforce PEP 8, detect unused imports, basic errors
Type checking	mypy (with strict flags gradually)	Catch type mismatches, missing return values, etc.
Security scan	bandit + pip-audit	Static analysis for common Python security pitfalls and known vulnerabilities in dependencies
Unit tests	pytest (already exists)	Run the existing test suite, ensure nothing breaks
Coverage	pytest-cov + threshold (e.g., 80%)	Prevent decrease in test coverage over time
PR template	existing pr-template-check	Already enforced, keep as is

Workflow Integration

• Trigger: on pull_request to main (and optionally on push to feature branches if needed).
• Jobs: Each check runs as a separate job (fast parallel execution), with a final required status check that gates merging.
• Docker: Optionally build and smoke-test the Docker image in CI.

Implementation Plan

1. Extend or create a new workflow file (.github/workflows/ci.yml) that supersedes the existing test.yml and adds the new checks.
2. Keep pr-template-check.yml as a separate required check.
3. Enable the branch protection rule to require the new CI status checks before merging.

Dependencies & Tooling

• All tools are already in requirements-dev.txt or can be added.
• Use ruff over flake8 for speed and comprehensive rules.
• pip-audit can be installed as an action (pypa/gh-action-pip-audit).

This will ensure that every contribution meets a minimum quality bar and drastically reduces the chance of security regressions like path-traversal vulnerabilities sneaking in.

[nopoz]

Thanks @nickorlabs, this lines up with the spirit of the proposal. A few notes to reconcile it with what's on main today and what's already in flight:

Current state. The only workflows on main right now are the two description linters (issue-description-check.yml, pr-description-check.yml). There's no test.yml or pr-template-check, and no requirements-dev.txt (just requirements.txt and requirements-optional.txt), so a few of the "already exists" assumptions don't hold yet. The pytest + syntax base lives in #1015 (closed in the recent sweep), which this issue proposes as the correctness base.

Overlap with the security bundle (#1314). It already covers secret scanning (gitleaks), dependency scanning (Dependabot + dependency-review, plus pip-audit run against requirements.txt and requirements-optional.txt), container scanning (hadolint + Trivy), and static code scanning (CodeQL for Python and JS). So pip-audit is already in the bundle, and bandit would largely overlap CodeQL's Python security queries. I'd lean on the existing bundle rather than running parallel tools, unless there's a specific gap bandit catches that CodeQL doesn't.

Heavier additions to defer. A ruff lint gate sounds cheap but isn't on this tree: ruff check with only the default E/F rules already flags more than 600 issues in the existing code (import-not-at-top, unused imports, multiple-statements, and so on). As a required check it would fail on main itself and on nearly every open PR until a large cleanup lands first, which is the same churn problem as mypy (even gradual), a coverage threshold, and a Docker smoke build, and the kind of broad formatting change CONTRIBUTING discourages. I'd keep all of these out of the initial gate. If lint is wanted later, the path is a one-time autofix baseline plus a pinned config, then adding it as a required check (or running it advisory / changed-lines-only in the meantime).

Gating. Agreed the value is in required status checks. One caveat already in the proposal: the pytest suite has known flaky failures, so it should run non-blocking until it's reliably green, with the syntax and security jobs gating from day one.

Scope and sequencing are the maintainer's call, so I'd keep the initial gate minimal rather than expanding it unilaterally. Thanks again, and #2267 is a good example of why the security scanning earns its place.

[vdmkenny]

Using this as the umbrella for adding CI to the repo — several overlapping proposals exist and should converge here rather than landing in parallel:

• CI: enforce the syntax/test checks CONTRIBUTING already asks contributors to run #1965 — enforce the py_compile / node --check / pytest checks CONTRIBUTING already asks for (minimal; PR feat(ci): add CI workflow (syntax + tests) #1966). [disclosure: filed by me]
• Enhancement: Improve CI/CD workflows for Python, Node dependencies, and Docker build checks #1922 — improve CI for Python/Node deps + Docker build checks
• feat(ci): add non-blocking smoke-test workflow #2335 — non-blocking smoke-test workflow (has a PR)
• Exclude running CI when its a documentation change #2646 — skip CI on docs-only changes (an optimization on top, once CI exists)

Closed #261 (vague + mobile-native-build scope that doesn't apply) in favour of this. Suggest we agree the baseline scope here, then fold the above in.

[vdmkenny]

Correction to my note above: a baseline CI already landed — #1965 is closed/completed via merged PR #1966, so .github/workflows/ci.yml (py_compile + node --check + pytest) is on dev today, alongside the existing description-check workflows.

So this issue and the siblings are enhancements on top of the existing ci.yml, not adding CI from scratch:

• Proposal: CI baseline (correctness + security) gate for the repo #2288 (this) — add a correctness + security gate
• feat(ci): add non-blocking smoke-test workflow #2335 — non-blocking smoke tests
• Enhancement: Improve CI/CD workflows for Python, Node dependencies, and Docker build checks #1922 — Python/Node dep + Docker build checks
• Exclude running CI when its a documentation change #2646 — skip CI on docs-only changes

Worth scoping each against the current ci.yml to avoid overlap/competing workflow files.

[alteixeira20]

Thanks for the updates here. I think this is close to a useful first slice for PR queue hygiene.

The part I especially like is the direction of keeping stale/conflict state updated automatically after dev moves. A push trigger on dev, plus workflow_dispatch, is the right shape for keeping labels current after merges without relying on maintainers to manually re-check every open PR.

I would still narrow this before merge so v1 stays boring, predictable, and low-risk:

• Please use the existing stale pr label instead of introducing needs-rebase. The repo already has a stale/conflict status label, so adding another one would split the review workflow.
• Please do not create labels automatically from the workflow. Existing repo automation treats labels as maintainer-managed, and I think this should follow the same pattern: check whether the expected label exists, warn/skip if it does not.
• Please make v1 label-only. I would avoid posting/deleting bot comments for now because the repo has a large PR backlog and comment automation can become noisy fast. A contributor nudge/comment can be discussed as a follow-up if maintainers want it.
• Please keep the conservative behavior around UNKNOWN mergeability. Skipping ambiguous states is the right call.
• Please make the wording target-branch aware if any user-facing text remains. There is still wording around main moving, but the active PR target is dev.
• Please do not touch ready for review in this PR. That label has broader workflow implications and should be handled separately if maintainers decide to reconcile review-state labels later.

The implementation I would be comfortable with as a first pass is:

On push to dev:
  scan open PRs targeting dev
  if mergeable == CONFLICTING:
    add `stale pr`
  if mergeable == MERGEABLE:
    remove `stale pr`
  if mergeable == UNKNOWN/null:
    do nothing

On pull_request_target opened/reopened/synchronize/ready_for_review:
  check only that PR with the same rules

On workflow_dispatch:
  allow manual cleanup after merge bursts

That gives maintainers the high-impact part — keeping stale pr accurate after merges — without adding new label taxonomy, comment noise, or broader review-policy changes.

With those changes, I think this would be a strong and safe first CI/review automation slice.

[vdmkenny]

Opened #3336 as a small concrete slice of this, on the existing pr-description-check workflow rather than a new one:

• Quality gate: a check-title job that fails a PR whose title is not Conventional Commits format (inline github-script regex, no new external action). Pairs with the Conventional Commits guidance added to CONTRIBUTING in docs(contributing): require constants/helpers over hardcoded paths and URLs #3335.
• Security: pins the workflow's actions to their latest release commit SHAs (actions/checkout v6.0.3, actions/github-script v9.0.0) instead of mutable tags, the supply-chain hardening this proposal calls for.

Kept it minimal and scoped to one workflow so it is easy to accept or reject independently of the larger gate. The broader correctness checks (compileall, node --check, pytest) from your proposal are still worth a separate PR.