ci: add typecheck, ESLint, ruff, and repo-guard CI gates (closes #208, closes #209, closes #210, closes #241, closes #270) - #272
Merged
Conversation
added 4 commits
August 24, 2026 21:23
…ross-platform ruff wrapper (closes #241, closes #270) - scripts/check-python-twins.sh (NEW): five-pair drift gate, two modes (strict byte-identical, docstring-tolerant via awk), wired into pre-push + Makefile + scripts/README.md. Five pairs: log_ring, prod_guard, module_id, discord, test_prod_guard. Catches the already-drifted discord.py copy (missing PEP 8 blank line between stdlib and third-party imports — fixed here). - scripts/check-duckdb-bind-claims.sh: replace command -v probe with functional probe (try actually running the interpreter, not just checking whether its name resolves). On Windows command -v python3 matches the zero-byte Store alias stub that exits immediately and opens the Store instead of running Python. - scripts/ruff.sh (NEW): cross-platform ruff wrapper that probes functionally for a real ruff binary, then python3/python/py -m ruff, accepting a candidate only after actually running it. Forward all args. Rewires .lintstagedrc.json so .py pre-commit hooks survive a machine where ruff is not on PATH. - Twin docstrings in prod_guard.py and module_id.py (both copies) now name the gate that enforces the sync.
…mat all .py files (closes #209) - ruff.toml (NEW, repo root): single source of truth for both services. target-version = "py310" with UP rules enabled, pinned to the floor in /.python-version (ADR-029) — stops autofixes rewriting datetime.now(timezone.utc) into the 3.11-only datetime.UTC that crashed prod (#180). E/F/I/B/UP rules. verified with --show-settings that both services resolve to the same config. - image-service/pyproject.toml: removed the now-duplicate [tool.ruff] block so the root config is the sole source of truth (ruff resolves from the nearest ancestor directory, so a leftover service-local block would silently override the root config). - scripts/check-python-version.sh: follows the config to ruff.toml instead of the now-removed image-service/pyproject.toml block. - 41 .py files linted and formatted: 38 I001 (unsorted imports), 8 B905 (zip() without strict= — 7 strict=True, 1 strict=False in weather_worker where Open-Meteo's third-party arrays can legitimately be shorter than times), 2 UP006, 2 UP035, 1 F401, 1 UP045. The B905 strict=True in routes/progress.py was reverted to strict=False during review: both lists are padded *up* to TARGET_NESTS_PER_TYPE but never truncated, so their lengths can differ — strict=True raises ValueError inside write_transaction and 500s the classification write path. - All five twin pairs re-diffed — still identical (docstring-tolerant on prod_guard and module_id).
Typecheck:
- contracts/tsconfig.json (NEW): strict, noEmit for the shared types.
- backend/tsconfig.typecheck.json (NEW): extends base tsconfig with
noEmit and a widened include covering src/, tests/, and vitest
config. Tests were previously outside the typecheck scope entirely
(base tsconfig only includes src/), so a type error in a test file
was invisible to both tsc (excluded) and vitest (esbuild strips
types). lib raised from ES2020 to ES2022 for AggregateError and
Error({ cause }) — target stays ES2020 so emitted syntax is
unchanged.
- backend/vitest.config.ts renamed to .mts: Node16 module resolution
requires an explicit ESM extension for a file that uses top-level
import outside a "type":"module" directory.
- backend tests: RequestInfo|URL → string|URL|Request (deprecated
alias), import paths with .js extension (Node16 ESM resolution).
- homepage/package.json: typecheck script added.
- Root typecheck: npm run typecheck --workspaces --if-present.
- TypeScript aligned: backend ^5.3.3 → ^5.9 (resolves to 5.9.3).
ESLint:
- Single root eslint.config.js (flat config): @eslint/js recommended,
typescript-eslint recommended, react-hooks + react-refresh for
homepage/, eslint-config-prettier last. Scoped globals per area
(node for backend, browser for homepage, vitest for test files).
- no-explicit-any: error (the two pre-existing uses in homepage tests
carry justified inline disables).
- no-console: warn (~60 intentional logging sites, not debug litter).
- react-hooks/rules-of-hooks: error; exhaustive-deps: warn.
- Installed: eslint@10.9.0, typescript-eslint@8.67.0,
eslint-plugin-react-hooks@7.1.1, eslint-plugin-react-refresh@0.5.4,
eslint-config-prettier@10.1.8, globals@17.11.0.
- Wired into lint-staged: eslint --fix runs before prettier --write
on staged TS files.
- 0 errors, 63 warnings (all no-console / exhaustive-deps /
react-refresh — downgraded by design).
…y hooks; document everything (closes #210) Three new CI jobs in tests.yml (15 jobs total, up from 12): - ts-quality: tsc --noEmit across all three workspaces, then ESLint. Neither existing unit job type-checks: vitest and tsx both strip types through esbuild. - python-lint: ruff check + ruff format --check over both Python services under the root ruff.toml. Scoped to the two services (not .) because ESP32-CAM/, scripts/, tools/ carry Python that has never been linted (38 findings — widening belongs in #257). - repo-guards: the four scripts that had no CI backstop before (check-no-hardcoded-api-keys, check-stale-display-name-rule, check-stale-reset-prose, check-python-twins). production added to push triggers: scripts/deploy.sh installs with HUSKY=0 and pushes the firmware auto-bump to production from the live host, so the one commit that reaches the fleet ran no pre-push hook at all. This is an alert, not a gate (the push has already landed) — enforced promotion is #237. The esp-firmware GEO_API_KEY guard now covers production too (a missing secret there would publish Null-Island firmware to the fleet, strictly worse than the same mistake on main). Docs: - ci-gates.md: three new rows, header corrected (12→15, production trigger), new "What CI does not check" section. - auth.md: trigger matrix updated (production row, production guard). - ADR-029 addendum: retracts the now-false "ruff floor is image-service-only" and "neither service runs ruff in CI" bullets. - Three chapter-11 lessons: twin drift (#241), hooks-are-not-gates (#210), tsc-ran-first-on-prod-host (#208). - troubleshooting.md: ruff entry now points at scripts/ruff.sh. - CLAUDE.md: static gates section, ruff.sh wrapper note, test counts updated (302/198).
This was referenced Aug 25, 2026
dchaudhari7177
pushed a commit
to dchaudhari7177/highfive
that referenced
this pull request
Aug 27, 2026
schutera#272 fixed the probe in scripts/check-duckdb-bind-claims.sh by running each candidate instead of calling `command -v` on it, so this is no longer a bug report -- it is the symptom entry that was missing. The failure is worth writing down because the error text names the wrong culprit. The pre-push hook dies with Microsoft's "Python was not found; run without arguments to install from the Microsoft Store" on a machine where `python --version` prints a real version, which reads as a broken Python install rather than as a PATH shim being preferred over a working one. CI never reproduces it: the duckdb-bind-claims job runs on Linux. Records the shell transcript that distinguishes the two -- `command -v python3` resolving to the WindowsApps stub, the stub exiting 49, and `python` exiting 0 -- so a contributor hitting it on some *other* tool can recognise the shape, plus the Settings path to disable the alias. The last line points at `main` for the fix rather than describing one, since the script side landed in schutera#272.
cofade
pushed a commit
that referenced
this pull request
Aug 28, 2026
…ipts Three shell scripts hand-rolled three probes with three different candidate lists and two validation techniques, and a fourth opted out of Python entirely because of that fragility. A Windows contributor's toolchain could satisfy two of them and fail the third, with nothing shared to fix. The concrete gap was ESP32-CAM/build.sh, which tried only python3 and python. On a stock Windows 11 + Git Bash box neither resolves and only the `py` launcher does, so its fallback path would abort on a machine with a working Python 3.12. It has never fired because build.sh prefers esptool.exe and skips the interpreter whenever it finds one -- but that mask lives in a different concern from the defect and could be removed by someone with no reason to look at the probe. scripts/lib/python.sh now holds one candidate list and one functional check. resolve_python() returns rather than exiting, because the callers genuinely differ on whether a missing interpreter is fatal (build.sh) or a skip (check-duckdb-bind-claims.sh), and that stays their decision. PYTHON is an array. `py -3` is two words, and a string would need `# shellcheck disable=SC2086` at every call site to stay splittable -- which the -S info job would otherwise require, so the array is what keeps the suppressions out. It also removes the one such directive that already existed. `py -3` goes first, for all three at once, which is the single-place-to-decide property this is for. The launcher is the one interpreter a python.org install on Windows always provides and it is never an alias stub. The shellcheck CI job gains -x and scripts/lib/*.sh. Without -x every new `source` line reports SC1091 at info level and the job goes red; without the glob the helper is unlinted, since scripts/*.sh does not match a subdirectory. Verified against shellcheck 0.11.0 with the exact new command line: clean at -S info, and the old command line goes red first, so the CI edit is load-bearing rather than tidying. .gitignore needed a third carve-out. `lib/` is ignored as the Python build-output convention, and it swallowed scripts/lib/python.sh exactly as it once swallowed homepage/src/lib/location.ts -- an ignored helper that three tracked scripts source would have failed on every machine but mine. Also removes the self-contradicting comment block in check-duckdb-bind-claims.sh: its pre-existing line claimed Windows Git Bash has "only the py launcher, never python3" and asserted parity with build.sh, while the paragraph #272 added beneath it explains that a stock install does put a python3 stub on PATH. Both could not be true, and the parity claim was false when written -- build.sh has validated by execution since #99 while that loop trusted `command -v` until #270. Docs: a troubleshooting entry for the symptom, with the snippet to source for a fourth script; a scripts/README row; and a chapter 11 entry on the two lessons -- count near-duplicates of a platform workaround rather than reviewing them one at a time, and when a defect is unreachable, check what makes it unreachable.
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
1 participant
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.
What
A single PR that ships four (really five) CI-gate issues from the 2026-08-18 audit epic #262, plus the #270 Windows-interpreter-probe bug that was blocking
.pycommits on Windows.CI gates added (
.github/workflows/tests.yml, 15 jobs total)ts-qualitytsc --noEmitacross all three npm workspaces — neither existing unit job type-checks anything (vitest + tsx both strip types through esbuild without checking them). Backend tests are now in scope too (previously excluded bytsconfig.json'sinclude). Then ESLint (flat config, 0 errors, 63 warnings — allno-console/exhaustive-deps/react-refresh, downgraded by design).python-lintruff check+ruff format --checkover both Python services under one rootruff.tomlwithtarget-version = "py310". Scoped to the two services (not.) becauseESP32-CAM//scripts//tools//tests/e2e/carry 38 pre-existing findings — widening belongs in #257.repo-guardscheck-no-hardcoded-api-keys.sh(security control),check-stale-display-name-rule.sh,check-stale-reset-prose.sh,check-python-twins.sh.What changed in the tree
contracts/tsconfig.json(new),backend/tsconfig.typecheck.json(new, widens scope to includetests/),eslint.config.js(new, root flat config),backend/vitest.config.ts→.mts(Node16 ESM),libraised from ES2020→ES2022 forAggregateError/Error({ cause })(target stays ES2020), TS aligned to^5.9across workspaces,RequestInfo|URL→string|URL|Request(deprecated alias), 5 test files updated for Node16.jsimport extensions.ruff.toml(new, repo root —E/F/I/B/UP,target-version = "py310"),image-service/pyproject.toml(removed[tool.ruff]block so the root config is the sole source of truth),check-python-version.sh(follows the config toruff.toml), 41.pyfiles linted and formatted.scripts/check-python-twins.sh(new, 5 pairs, strict + awk-based docstring-tolerant modes), wired into pre-push + Makefile + CI. Fixed the already-driftedduckdb-service/services/discord.py(missing PEP 8 blank line). Twin docstrings updated.command -v python3matches the Windows Store alias stub, so the check fails on machines with a workingpython#270 (interpreter probe):scripts/check-duckdb-bind-claims.shnow probes functionally (actually runs the interpreter, not justcommand -v— the Windows Storepython3stub matches an existence check but is not a real Python).scripts/ruff.sh(new, same probe discipline, used by.lintstagedrc.json).productionadded to push triggers so the firmware auto-bump commit fromscripts/deploy.sh(which runsHUSKY=0) is finally seen by the same guards that check every human-authored commit.esp-firmware'sGEO_API_KEYguard now coversproductiontoo.ci-gates.md(3 new rows + "What CI does not check" section),auth.mdtrigger matrix updated,troubleshooting.mdruff entry,CLAUDE.mdstatic gates section.Local verification (all 12 gates green)
Issues
Closes #208, closes #209, closes #210, closes #241, closes #270.
🤖 Generated with Claude Code