ci: add GitHub Actions workflow for Python, Node backend, and frontend (#821)#834
Open
Rudra-clrscr wants to merge 4 commits into
Open
ci: add GitHub Actions workflow for Python, Node backend, and frontend (#821)#834Rudra-clrscr wants to merge 4 commits into
Rudra-clrscr wants to merge 4 commits into
Conversation
Userunknown84#821) - Add .github/workflows/ci.yml with three concurrent jobs (Python pytest+flake8, Node backend npm test, frontend npm test), triggered on push and pull_request. The existing backend-ci.yml's 3-way Node version matrix (18/20/22) is left as-is; this workflow adds a single-version run alongside it rather than duplicating that matrix. - Add backend/setup.cfg scoping flake8 to the correctness-relevant subset (E9, F63, F7, F82 - syntax errors and undefined names) since the full PEP8/F ruleset surfaces ~700+ pre-existing style violations and ~75 pre-existing unused-import findings unrelated to CI setup. - Add backend/eslint.config.js (none existed before) and an npm run lint script; wired into CI as continue-on-error since it surfaces 58 pre-existing problems (including real parsing errors in server.js, authRoutes.js, and adversarialGuard.js) that are out of scope here. - Add a CI status badge to README.md. Fixed while wiring this up, since flake8's error-level checks caught them: - Missing imports (filelock.Timeout, hashlib/threading/OrderedDict/time in spam_insights.py, requests in retrain.py, a duplicate `import os`). - api.py referenced a bare `_db_connection()` that never existed; aliased to the same imap_store.get_db_connection() used elsewhere in the file for the identical purpose. - api.py used `internal_endpoint_required` as a route decorator without ever defining it, aliased to the existing, working validate_internal_request decorator used for the same purpose elsewhere. - api.py's sys.path.insert for the email_connectors directory ran after the imports that needed it, so gmail_connector/outlook_connector/ email_scanner could never actually be found; moved it before those imports. - app.py defined the /health route and health_check() function twice, which crashes Flask at startup with "View function mapping is overwriting an existing endpoint function." pytest now passes 113/116 (up from erroring on collection). The 3 remaining failures are pre-existing and out of scope: two assert on an /imap/status route that doesn't exist, and one expects the non-object JSON body validation from issue Userunknown84#819 (lives on a separate branch/PR). Frontend's 28 pre-existing eslint errors and backend's 58 are left for follow-up cleanup PRs per discussion.
|
Someone is attempting to deploy a commit to the Aditya Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR adds a baseline GitHub Actions CI workflow to run backend Python lint/tests, backend Node tests, and frontend Node tests on every push and pull request, while also making a handful of small backend fixes unblocked by newly-enabled error-level linting.
Changes:
- Added
.github/workflows/ci.ymlwith parallel Python, backend Node, and frontend jobs, plus a README CI badge. - Introduced scoped flake8 configuration (
backend/setup.cfg) and a new backend ESLint config (backend/eslint.config.js) with annpm run lintscript. - Fixed several pre-existing backend issues surfaced by linting (missing imports, undefined DB connection helper, duplicate
/healthroute).
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Adds CI jobs for Python, backend Node, and frontend; currently configures lint as non-blocking. |
README.md |
Adds CI status badge linking to the new workflow. |
backend/setup.cfg |
Adds a flake8 config scoped to syntax/undefined-name errors. |
backend/eslint.config.js |
Introduces ESLint flat config for the backend with basic recommended rules and ignores. |
backend/package.json |
Adds lint script and ESLint-related devDependencies. |
backend/package-lock.json |
Locks ESLint and its dependency chain (including Node engine requirements). |
backend/api.py |
Fixes import path ordering, adds missing Timeout import, aliases decorators/helpers, and uses imap_store.get_db_connection(). |
backend/app.py |
Removes duplicate /health route definition that would crash Flask at startup. |
backend/spam_insights.py |
Adds missing imports referenced by code paths. |
backend/retrain.py |
Removes duplicate import os and adds missing requests import used for reload triggering. |
Files not reviewed (1)
- backend/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+56
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "20.x" | ||
| cache: "npm" | ||
| cache-dependency-path: backend/package-lock.json |
Comment on lines
+81
to
+86
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: "20.x" | ||
| cache: "npm" | ||
| cache-dependency-path: frontend/package-lock.json |
Comment on lines
+61
to
+66
| - name: Lint (eslint) | ||
| # The backend has never been linted before and has pre-existing | ||
| # parsing/reference errors unrelated to any given PR; report without | ||
| # blocking merges until a dedicated cleanup PR lands. | ||
| run: npm run lint | ||
| continue-on-error: true |
Comment on lines
+91
to
+96
| - name: Lint (eslint) | ||
| # Pre-existing errors unrelated to this workflow; see backend note | ||
| # above. Report without blocking until a dedicated cleanup PR lands. | ||
| run: npm run lint | ||
| continue-on-error: true | ||
|
|
Comment on lines
52
to
58
| "devDependencies": { | ||
| "@eslint/js": "^10.0.1", | ||
| "eslint": "^10.7.0", | ||
| "globals": "^17.7.0", | ||
| "jest": "^30.4.2", | ||
| "supertest": "^7.2.2" | ||
| } |
added 3 commits
July 14, 2026 03:14
fix/backend-eslint-errors (from the same batch of issues) independently creates this same file and adds this exact global (Sentry is set on `global` by server.js before route modules are required - real at runtime, just invisible to static analysis). Since both branches create backend/eslint.config.js from scratch, merging them in either order produces an add/add conflict for no reason. Pre-adding the identical line here so both branches produce byte-identical content for this file; whichever merges second is then a no-op instead of a conflict. Inert change for this branch's own purposes (nothing in this branch references a bare Sentry global) - verified `npx eslint .` still reports the same pre-existing findings as before, none introduced or removed by this addition.
The container's Flask process defaults to FLASK_HOST=127.0.0.1 (a deliberate secure default for bare-metal runs), so inside the CI container it never listens on an interface reachable through the -p 5000:5000 port mapping, and the smoke test's curl always times out. docker-compose.yml already sets FLASK_HOST=0.0.0.0 for the same reason; the CI workflow's docker run step needs the same override.
main's copies of these files have a duplicate 'function App()' declaration and a stray '#Manipulation' line before an import respectively - both invalid JS that break 'vite build' in the frontend Docker image stage. Only fixed inside the open fix/frontend-eslint-errors PR branch, never merged to main, so any branch cut from main (including this one) inherits them. Pulling in the already-verified fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #821.
.github/workflows/ci.ymlwith three jobs that run concurrently onpushandpull_request:backend/requirements.txt, runsflake8thenpytest tests/npm ci,npm run lint(non-blocking),npm testnpm ci,npm run lint(non-blocking),npm testbackend-ci.yml(3-way Node 18/20/22 matrix) is left as-is;this workflow adds a single-version run alongside it rather than
duplicating that matrix 3x.
backend/setup.cfgscoping flake8 to the correctness-relevant subset(
E9,F63,F7,F82— syntax errors and undefined names). Full PEP8 style has~700 pre-existing violations and the broader F-series has ~75 pre-existing
unused-import findings, both unrelated to CI setup — left for a dedicated
cleanup pass rather than blocking every PR.
backend/eslint.config.js(none existed before) and annpm run lintscript. Wired as
continue-on-error: truein CI since it surfaces 58pre-existing problems, including real parsing errors in
server.js,authRoutes.js, andadversarialGuard.js— out of scope here.README.md.Bugs fixed along the way
Wiring up flake8's error-level checks caught several real, pre-existing bugs
— all one-line, obvious fixes:
filelock.Timeout,hashlib/threading/OrderedDict/timeinspam_insights.py,requestsinretrain.py(also removed aduplicate
import os)api.pycalled a_db_connection()that was never defined anywhere —aliased to
imap_store.get_db_connection(), which the same file alreadyuses for the identical purpose three other times
api.pyusedinternal_endpoint_requiredas a route decorator withoutever defining it — aliased to the existing, working
validate_internal_requestdecorator used for the same purpose elsewherein the file
api.py'ssys.path.insertfor theemail_connectorsdirectory ranafter the imports that needed it, so
gmail_connector/outlook_connector/email_scannercould never actually be found — movedit before those imports
app.pydefined the/healthroute andhealth_check()twice, whichcrashes Flask at startup ("View function mapping is overwriting an
existing endpoint function") — removed the earlier, less complete
duplicate
Test plan
cd backend && flake8— cleancd backend && pytest tests/ -q— went from erroring on collection to113/116 passing. The 3 remaining failures are pre-existing and out of
scope: two assert against an
/imap/statusroute that doesn't exist, andone expects the non-object-JSON-body validation from Add input validation on the
/predictendpoint #819 (lives on aseparate branch/PR)
cd backend && npm test— passes (aside from the already-known,unrelated
fileValidation.jsissue)cd frontend && npm test— passesFrontend's 28 pre-existing eslint errors and backend's 58 are intentionally
left for separate follow-up cleanup PRs, per discussion.