#391 Strengthen ESLint, TypeScript, And CI Configuration

[b-at-neu]

Closes #391

Summary

Tooling-only hardening across Node, TypeScript, ESLint, and CI — no product surface, no schema, no server actions. Rule selection is deliberately bounded to the rules the ticket names rather than adopting recommendedTypeChecked wholesale, so the fix set stays reviewable. Existing required-check job ids (run-prettier-check, run-linting-check, run-tsc-check, run-test-check, run-commit-message-check) are unchanged.

Changes

• package.json / README.md / .github/dependabot.yml — pin Node to 24.x via engines only (no .nvmrc, so there's a single place to update); README.md points at engines instead of restating the version number; @types/node pinned to ^24 with a dependabot ignore for future majors kept as a guardrail against @types/node drifting ahead of the pinned runtime; add typescript-eslint devDep and a prisma:migrate:status script.
• tsconfig.json — target ES2017 → ES2022; noUncheckedIndexedAccess, noUnusedLocals, noUnusedParameters enabled, with every surfaced access site fixed in code (lib/dates.ts, lib/constants.ts, lib/auth/redirect.ts, lib/utils.ts, prisma/data/applications.ts, prisma/seed/helpers.ts, tests/helpers/fixtures.ts, and several components/features/* file-upload/short-answer value reads).
• eslint.config.mjs — type-aware config via typescript-eslint's projectService, adding no-floating-promises, no-misused-promises, no-unnecessary-type-assertion, no-unnecessary-condition, switch-exhaustiveness-check, plus a no-restricted-imports guard on @/lib/prisma reaching app/**/components/**/non-auth lib/**. lib/prisma.ts gets import 'server-only' as the guard's runtime twin. Every violation the rules surfaced is fixed in code (void-wrapped fire-and-forget saves, form onSubmit handlers wrapped to satisfy void-returning attribute types, dead/unreachable conditionals removed, unnecessary assertions dropped) — no eslint-disable anywhere.
• .github/workflows/{prettier,linting,tsc}-check.yml — actions/checkout@v7 + actions/setup-node@v7 with node-version-file + cache: npm, npm ci replacing npm install, a concurrency cancel-in-progress group; tsc-check swaps the forbidden npx prisma generate for npm run prisma:generate.
• .github/workflows/{test-check,commit-message-check,migrate-db}.yml — action majors bumped to @v7, concurrency added where missing.
• .github/workflows/build-check.yml (new) — run-build-check: Postgres service + migrate + npm run build against a real database. Kept as-is: an ephemeral local Postgres decouples "does the code build" from Neon branch-capacity exhaustion.
• .github/workflows/lockfile-check.yml (new) — run-lockfile-check only (frozen install + lockfile-version/pinning assertions), pull_request-triggered with no schedule:. No run-audit-check: it was permanently red on pre-existing transitive CVEs unrelated to this PR, isn't a required check, and nobody's committed to triaging it; the weekly schedule existed to give the audit job something to catch between PRs, so it drops with the audit job (a committed lockfile doesn't drift without a commit, so the lockfile check has no need for a schedule).
• .github/workflows/codeql.yml (new) — javascript-typescript analysis on PRs + weekly schedule.
• .github/pull_request_template.md (new) — mirrors the PR body shape used here. (No .github/CODEOWNERS — removed per review; not adopting mandatory-review-by-file-path in this repo yet.)
• .github/workflows/migrate-db.yml — workflow_dispatch with a target (dev/production) input so it can be exercised on dev before it ever touches production; a GitHub environment: gate; a prisma migrate status preflight that posts to the step summary and fails specifically on detected drift (not on ordinary pending migrations, and not silently passing through any other preflight failure); a rollback runbook documented in the header comment.

Testing plan

• npm ci completes on Node pinned via package.json's engines field with no EBADENGINE warning
• npm run tsc:check, npm run eslint:check, npm run prettier:check all pass locally (all green on this PR — see Automated checks)
• npm run test:unit passes locally (all 133 tests) — npm run test (db project) requires a project-local Postgres instance not available in this sandbox; run-test-check on the PR covers it
• Temporarily add an unawaited promise statement in a prisma/actions/ file → npm run eslint:check fails with no-floating-promises; revert
• Temporarily add import { prisma } from '@/lib/prisma' to a file under components/features/ → npm run eslint:check fails with the no-restricted-imports message; revert. Confirm an existing @/prisma/client enum import in a component still passes
• Smoke the touched paths after the strictness fixes: apply to a position (short/long/file/multiple-choice-with-"Other" answers render and save), profile question editing, applications table sort/filter via URL params, a file-upload answer download
• On this PR: run-prettier-check, run-linting-check, run-tsc-check, run-test-check, run-commit-message-check, run-build-check, run-lockfile-check, and the CodeQL job all report; confirm each ran on the pinned Node version (setup-node step log) and restored an npm cache on the second run
• Opening this PR via the GitHub UI prefilled the body from .github/pull_request_template.md (already true for this PR, opened via gh, so verify on the next manually-opened PR)
• Staging exercise for migrate-db.yml (AC 7) — on dev, run the workflow via workflow_dispatch with target: dev and confirm the preflight step summary lists status and the apply step succeeds; re-run with nothing pending and confirm a clean no-op. Only then let it run on production
• Human repo-settings follow-ups (not doable from a PR): add run-build-check (and run-lockfile-check) to the required status checks on dev/main; create the production and dev GitHub Environments (the latter with no reviewer required, the former with one); leave the CodeQL job and run-neon-check out of required checks

Automated checks

• npm run prettier:check — pass
• npm run eslint:check — pass (zero warnings/errors under the new type-aware rules)
• npm run tsc:check — pass (zero errors under noUncheckedIndexedAccess/noUnusedLocals/noUnusedParameters, target ES2022)
• npm run test:unit — 133/133 passing locally

Notes

• No Prisma schema or migration changes; no new server actions or UI — no new validation/auth surface, no new async states to design.
• typescript-eslint 8.x may print a TypeScript-version-support warning against TS 6.0.3 in CI logs — stderr noise, not a lint error, intentionally left as-is.
• Vercel reads engines.node — after merge, verify the Vercel project's Node setting agrees with the pinned version; a mismatch surfaces as a build-time warning, not a failure.
• The CodeQL job and run-neon-check are intentionally not required checks — see the human follow-ups above.
• migrate-db.yml's new environment: gate requires the dev/production GitHub Environments to exist before the workflow can run on either target; until then the job will fail closed at that step, which is the intended fail-safe rather than a bug.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
aplio	Ready	Preview	Aug 19, 2026 11:06pm

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[b-at-neu]

Code Review — Cycle 1 · needs revision

3 open — 3 🟠 Medium (see inline)

[b-at-neu]

R1-M3 🟠 Medium — this hunk drops the isPending-gated <Loader2> spinner from the "Remove file" confirm button, leaving only disabled={isPending} with no visual pending feedback. Loader2 is still imported/used elsewhere in this file (the "Uploading…" state), so this isn't dead-import cleanup from the new lint rules — it reads as an unrelated regression swept into a PR described as "no product surface" changes. Restore the spinner, or note why it was intentionally dropped.

[b-at-neu]

Skipping: this AlertDialogAction sits inside the else branch of the top-level isPending ? <Uploading…> : (...) split, and upload/remove share one useTransition. So while isPending is true the whole subtree — including the confirm dialog — unmounts in favor of the "Uploading…" indicator; the button can never render with isPending true. Restoring the spinner reintroduces the exact dead code no-unnecessary-condition (added by this PR) correctly flags: eslint . --max-warnings=0 fails on it. Confirmed this is preexisting behavior, not something this PR changed — verified against the pre-#391 version of this file. Happy to file a follow-up if a visible pending state on remove (e.g. not swapping to the full "Uploading…" replacement) is wanted, but that is a UX change beyond this review's ask.

[b-at-neu]

Revision — Cycle 1

fixed R1-M1, R1-M2 · skipped R1-M3 (dead code confirmed by no-unnecessary-condition, see thread) · 1f0f50a

[b-at-neu]

Code Review — Cycle 2 · approved

0 open — all 3 prior findings resolved (R1-M1, R1-M2 fixed; R1-M3 validly dismissed as dead code), no regressions in the revision commit

[b-at-neu]

Code Review

Verdict: Changes Requested (human feedback)

1. Keep build-check.yml as-is. It uses an ephemeral local Postgres, decoupling "does the code build" from Neon branch-capacity exhaustion (which makes Vercel red for reasons unrelated to code correctness). No change needed here.

2. Cut run-audit-check and the schedule: trigger from dependency-check.yml. It's currently red on pre-existing transitive CVEs unrelated to this PR, isn't a required check, and nobody's committed to triaging it — a permanently-red non-required check is just noise. The weekly schedule existed specifically so npm audit could catch newly-disclosed CVEs against packages already in dev with no PR open; since we're dropping the audit job, drop the schedule too (a committed lockfile doesn't drift without a commit, so run-lockfile-check has no need for a schedule).

3. Keep run-lockfile-check, but rename the file dependency-check.yml → lockfile-check.yml so file/job naming matches the existing convention (prettier-check.yml/run-prettier-check, build-check.yml/run-build-check, etc.) — run-lockfile-check already matches the job-naming style once the file is renamed to match.

4. Remove .github/CODEOWNERS.

5. Consolidate Node version pinning down to package.json's engines field only. Remove .nvmrc and the README's restated version number (point the README at engines instead of duplicating the number, so there's one place to update). Keep the dependabot ignore rule for @types/node major bumps — it's not a pin location, it's a guardrail: @types/node's version tracks Node's own majors independently of engines.node, so nothing else stops Dependabot from bumping @types/node ahead of the pinned runtime again (exactly how the original 25/24/22 three-way mismatch happened). Losing that guardrail risks TypeScript type-checking code against a newer Node's API surface than what's actually running, which can silently pass a type check for something that throws at runtime.

[b-at-neu]

Revision — human feedback (post Cycle 2 approval)

addressed all 5 items: kept build-check.yml unchanged, dropped run-audit-check + schedule from lockfile-check.yml (renamed from dependency-check.yml), removed .github/CODEOWNERS, consolidated Node pinning to package.json engines (dropped .nvmrc, README now points at engines, dependabot @types/node guardrail kept) · PR description/testing plan updated to match · 329b0d1

[b-at-neu]

Code Review

Verdict: Changes Requested (human feedback)

1. codeql.yml — remove the schedule: - cron: '0 6 * * 1' trigger entirely. No one reads a scheduled CodeQL run divorced from a PR; keep it on pull_request only.

2. lockfile-check.yml — the header comment is unnecessary; cut it down drastically (a one-liner at most, or none) rather than the current multi-line explanation.

3. migrate-db.yml — the comments are way too heavy: the long header block (secrets table, rollback runbook, failure model) and the inline explanations scattered through the steps (the "Both secrets MUST use..." block, the "Drift detected means..." paragraph, etc.) all need to be cut down hard. Keep only what's truly load-bearing in as few words as possible — match the terse, one-line-by-default style used elsewhere in the repo's workflows, not a documentation page embedded in the YAML.

[b-at-neu]

Revision — human feedback

removed codeql.yml cron trigger, cut lockfile-check.yml and migrate-db.yml comments to one-liners · 7e21c4b

[b-at-neu]

Code Review — Cycle 4 · approved

0 open — no findings; the only change since Cycle 3 is comment-trimming in lockfile-check.yml/migrate-db.yml and dropping codeql.yml's weekly cron (PR now rebased onto dev), no regressions