Fix CI: make the pipeline green (closes #41) #66

Merged
claude-bot merged 7 commits from fix/41-green-ci into main 2026-07-16 04:55:27 +00:00
Contributor

Fixes #41 — the keystone blocker for the v1.0.0 milestone. CI had failed on every run; every cause was a real, locally-reproduced code issue.

What changed

  • ruff check: auto-fixed 69, hand-fixed the rest (unused vars, F821 dead param, B904 exception chaining, SIM105/117). Added B008 to ignore (FastAPI Depends(...) default pattern — not a bug).
  • ruff format: reformatted 55 files; --check clean.
  • mypy app/: fixed all 73 errors across 17 files (callableCallable[...], missing generics, untyped defs, Literal variance via documented cast()). Root of app/main.py's attr-defined cascade was import app.models shadowing app = FastAPI() — fixed with from app import models as _models. Zero blanket ignores.
  • respx added to the dev dependency group (tests import it; collection failed without it).
  • cwd-dependent test: test_builtin_yaml_covers_bundled_events now anchors config/events.yaml / schemes/builtin.yaml on __file__, so it passes from any cwd.
  • Alembic drift: new migration 0008_drop_unused_indexes drops ix_sessions_expires_at, ix_sessions_subject, ix_webhook_configs_enabled. alembic upgrade head + alembic check clean.
  • ESLint: input.tsx empty-interface → type alias.
  • uv.lock committed for reproducible CI.
  • Deprecated datetime.utcnow(): replaced across app/ with a new app/timeutil.py::utcnow() returning naive-UTC — bit-identical value, no deprecation warning, and preserves the naive/naive comparisons used against the DB DateTime columns (avoids introducing tz-aware/naive comparison bugs).

Local verification (backend)

ruff check �· ruff format --check �· mypy app/ �· pytest -q → 204 passed �· alembic upgrade head + alembic check �all clean.

Not verified locally

Frontend (npm ci / lint / tsc / vitest / build) and the Docker E2E job — this dev box has no Node/Docker-runner toolchain. This PR's CI run is the authoritative check for those; the input.tsx change is correct by inspection (idiomatic shadcn/ui pattern).

Unblocks #5, #10, #56, #58, #64 and the Renovate PRs (#35–#39).

🤖 Generated with Claude Code

Fixes #41 — the keystone blocker for the v1.0.0 milestone. CI had failed on every run; every cause was a real, locally-reproduced code issue. ## What changed - **ruff check**: auto-fixed 69, hand-fixed the rest (unused vars, `F821` dead param, `B904` exception chaining, `SIM105/117`). Added `B008` to ignore (FastAPI `Depends(...)` default pattern — not a bug). - **ruff format**: reformatted 55 files; `--check` clean. - **mypy app/**: fixed all 73 errors across 17 files (`callable`→`Callable[...]`, missing generics, untyped defs, `Literal` variance via documented `cast()`). Root of `app/main.py`'s attr-defined cascade was `import app.models` shadowing `app = FastAPI()` — fixed with `from app import models as _models`. Zero blanket ignores. - **respx** added to the `dev` dependency group (tests import it; collection failed without it). - **cwd-dependent test**: `test_builtin_yaml_covers_bundled_events` now anchors `config/events.yaml` / `schemes/builtin.yaml` on `__file__`, so it passes from any cwd. - **Alembic drift**: new migration `0008_drop_unused_indexes` drops `ix_sessions_expires_at`, `ix_sessions_subject`, `ix_webhook_configs_enabled`. `alembic upgrade head` + `alembic check` clean. - **ESLint**: `input.tsx` empty-interface → type alias. - **uv.lock** committed for reproducible CI. - **Deprecated `datetime.utcnow()`**: replaced across `app/` with a new `app/timeutil.py::utcnow()` returning naive-UTC — bit-identical value, no deprecation warning, and preserves the naive/naive comparisons used against the DB `DateTime` columns (avoids introducing tz-aware/naive comparison bugs). ## Local verification (backend) `ruff check` �· `ruff format --check` �· `mypy app/` �· `pytest -q` → 204 passed �· `alembic upgrade head` + `alembic check` �all clean. ## Not verified locally Frontend (`npm ci` / lint / `tsc` / vitest / build) and the Docker E2E job — this dev box has no Node/Docker-runner toolchain. **This PR's CI run is the authoritative check for those**; the `input.tsx` change is correct by inspection (idiomatic shadcn/ui pattern). Unblocks #5, #10, #56, #58, #64 and the Renovate PRs (#35–#39). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Fix CI: ruff, mypy, tests, alembic drift, deprecated utcnow, frontend lint
Some checks failed
CI / Alembic migration check (pull_request) Successful in 1m4s
CI / Python lint & type-check (pull_request) Successful in 1m50s
CI / Python tests (pull_request) Successful in 3m18s
CI / Frontend lint, test & build (pull_request) Failing after 1m36s
CI / Docker build, health smoke & E2E (pull_request) Has been skipped
CI / Dependency audit (pull_request) Failing after 2m37s
e79e915e72
Backend:
- ruff check: auto-fix + hand-fixes (unused vars/imports, B904 exception
  chaining, SIM105/SIM117, dead JSONResponse param in auth callback);
  ignore B008 project-wide since FastAPI's `Depends(...)` default is the
  documented DI pattern, not a real function-call-in-default bug.
- ruff format: reformatted all files ruff flagged.
- mypy app/: fixed `callable` used as a type in recurrence.py (now
  Callable[[int], date]), missing dict/Task generic args, untyped defs in
  scheduler.py/schedule_service.py/auth.py, and app/main.py's attr-defined
  errors (caused by `import app.models` shadowing the `app = FastAPI()`
  module-level name — switched to `from app import models as _models`).
- Added `respx` to the dev dependency group (tests import it directly) and
  ran `uv sync --group dev`.
- Anchored config/events.yaml + schemes/builtin.yaml paths in
  test_import_export.py on `__file__` so the test passes regardless of cwd.
- Added migration 0008 dropping ix_sessions_expires_at, ix_sessions_subject,
  and ix_webhook_configs_enabled — the models no longer declare these
  indexes, so `alembic check` was flagging drift.
- Replaced `datetime.utcnow()` across app/ with a new `app.timeutil.utcnow()`
  helper that returns the same naive-UTC value via the non-deprecated
  `datetime.now(UTC)` API — avoids the deprecation warning without risking
  naive/aware comparison bugs against the naive DateTime columns used
  throughout the models.
- Committed backend/uv.lock for reproducible CI installs.

Frontend:
- input.tsx: converted the empty `InputProps` interface (which only
  re-declared its base type) to a type alias, fixing
  @typescript-eslint/no-empty-object-type.

Note: frontend lint/tsc/vitest/build could not be executed in this
environment (Node.js/npm not installed, Docker daemon not running); the
input.tsx fix is verified by inspection only — orchestrator should confirm
with `npm run lint` before merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix frontend lint: silence react-refresh co-export warnings
Some checks failed
CI / Dependency audit (pull_request) Failing after 1m28s
CI / Frontend lint, test & build (pull_request) Successful in 1m33s
CI / Alembic migration check (pull_request) Successful in 5m34s
CI / Python lint & type-check (pull_request) Successful in 5m43s
CI / Python tests (pull_request) Successful in 5m48s
CI / Docker build, health smoke & E2E (pull_request) Failing after 17m9s
b78744876c
The `--max-warnings 0` lint gate was failing on 4 pre-existing
react-refresh/only-export-components warnings (unrelated to the input.tsx
fix): the shadcn variant helpers (badge/button) and the context hooks
(UserContext/YearContext) are intentionally co-located with their
components. Added targeted eslint-disable-next-line directives.

Verified in a node:22 container: lint, tsc -b, vitest (21), and build all pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix E2E: label icon-only controls, correct brittle selectors
Some checks failed
CI / Alembic migration check (pull_request) Successful in 1m11s
CI / Python lint & type-check (pull_request) Successful in 1m37s
CI / Frontend lint, test & build (pull_request) Successful in 1m48s
CI / Python tests (pull_request) Successful in 2m17s
CI / Dependency audit (pull_request) Failing after 2m41s
CI / Docker build, health smoke & E2E (pull_request) Failing after 14m0s
21011b6679
The docker-e2e Playwright job surfaced failing happy-path specs (the app
was fine — build + health smoke pass). All were selector/a11y gaps:
- Year prev/next, settings, and user-menu buttons were icon-only with no
  accessible name. Added aria-labels ("Previous year", "Next year",
  "Settings", "User menu") — a real a11y improvement.
- The year-selector test targeted the lucide icon name (/chevronright/i),
  which is never an accessible name. Point it at the new "Next year" label.
- getByText("Authentication") was ambiguous: with DISABLE_AUTH=true a
  "Dev mode — authentication" banner also matches. Target the section
  heading via getByRole("heading", { name: "Authentication" }).

Verified by building the image and running the full suite against it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix npm audit: patch 6 advisories via lockfile (0 vulnerabilities)
Some checks failed
CI / Python lint & type-check (pull_request) Successful in 1m21s
CI / Python tests (pull_request) Successful in 1m59s
CI / Frontend lint, test & build (pull_request) Successful in 5m1s
CI / Alembic migration check (pull_request) Successful in 5m19s
CI / Dependency audit (pull_request) Successful in 10m47s
CI / Docker build, health smoke & E2E (pull_request) Failing after 26m1s
f753271538
`npm audit` (the Dependency audit CI job) failed on 6 advisories
(2 high: form-data, vite/launch-editor; moderate: js-yaml, react-router;
plus @babel/core). `npm audit fix` resolved all of them within existing
semver ranges — package-lock.json only, no package.json changes — leaving
0 vulnerabilities. Re-verified lint, tsc, vitest, build, and the full
Playwright suite against a freshly built image.

Note: the security-audit job is marked continue-on-error, but Forgejo
appears not to honor job-level continue-on-error, so this was gating.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI: reach E2E app by container name, not host-published port
Some checks failed
CI / Dependency audit (pull_request) Successful in 1m27s
CI / Frontend lint, test & build (pull_request) Successful in 5m6s
CI / Alembic migration check (pull_request) Successful in 5m48s
CI / Python lint & type-check (pull_request) Successful in 5m57s
CI / Python tests (pull_request) Successful in 6m10s
CI / Docker build, health smoke & E2E (pull_request) Failing after 15m28s
49e544293c
The docker-e2e "Wait for health" step failed on every run: the Forgejo
runner executes job steps inside a container, so `docker run -p 7842:7842`
publishes to the host, not the step's localhost. The app booted fine and
served health 200s to its internal HEALTHCHECK, but the runner's
`curl localhost:7842` could never connect.

Fix (mirrors the pattern verified locally): run the app on a user-defined
docker network and reach it by name from sibling containers — a curl
container for the health gate and the official Playwright image for the
suite (targeting the app via E2E_BASE_URL). Executor-agnostic; no reliance
on host port publishing. playwright.config.ts now reads E2E_BASE_URL,
defaulting to localhost for local runs. Satisfies #5's E2E-in-CI item.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI: copy frontend into Playwright container via docker cp
Some checks failed
CI / Dependency audit (pull_request) Successful in 1m23s
CI / Frontend lint, test & build (pull_request) Successful in 1m26s
CI / Python lint & type-check (pull_request) Successful in 5m45s
CI / Python tests (pull_request) Successful in 6m3s
CI / Alembic migration check (pull_request) Successful in 6m29s
CI / Docker build, health smoke & E2E (pull_request) Failing after 13m25s
c72aefc46a
Follow-up to the container-name networking fix: the sibling Playwright
container used `-v $PWD/frontend:/work`, but the runner executes steps
inside a container, so $PWD is the job container's fs — not a host path
the daemon can bind-mount, leaving /work empty. Stream the code in with
`docker cp` (daemon-side, executor-agnostic) instead. Also quote the
iris:ci / iris:e2e build tags to satisfy the YAML schema linter.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI: make docker-e2e setup idempotent on the reused runner host
All checks were successful
CI / Dependency audit (pull_request) Successful in 1m30s
CI / Frontend lint, test & build (pull_request) Successful in 4m14s
CI / Python lint & type-check (pull_request) Successful in 4m18s
CI / Python tests (pull_request) Successful in 4m37s
CI / Alembic migration check (pull_request) Successful in 4m42s
CI / Docker build, health smoke & E2E (pull_request) Successful in 36m39s
06fcf7e077
run #58 died at "Start container" with "network iris-net already exists":
the Forgejo runner reuses its docker host across runs, so the network (and
a named container) left by a previous failed run collided — meaning the
docker cp E2E flow never even got exercised. Remove any stale iris/pw
containers and the iris-net network before (re)creating them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
claude-bot deleted branch fix/41-green-ci 2026-07-16 04:55:27 +00:00
Sign in to join this conversation.
No description provided.