CI: enforce typecheck and run the full test suite#196
Conversation
Add real quality gates discovered missing in the repo audit: the repo had TypeScript strict mode but no typecheck ran anywhere, and CI ran only a single vitest file (broker.test.ts) out of the full suite. Scripts (package.json): - typecheck / typecheck:node / typecheck:web — tsc --noEmit over tsconfig.node.json (main+preload+shared) and tsconfig.web.json (renderer). - test:all — node:test suite + full `npx vitest run`. Config fixes: - tsconfig.node.json: add target/lib ES2022 (it previously defaulted to ES3, producing ~60 false-positive downlevelIteration / regex-flag / .at() errors that don't reflect the Node 22 / Electron runtime). Build is unaffected (electron-vite transpiles with esbuild, not tsc). - vitest.config.mjs: testTimeout/hookTimeout 20s. A few timing-sensitive tests (cloud-agent git-overlay, slack writeback) exceeded the 5s default only under full-suite / back-to-back load; this stabilizes the suite as a gate without skipping any test. CI (.github/workflows/ci.yml): - checks: add `npm run typecheck` (continue-on-error — see PR body for the ~95 pre-existing node-side errors; renderer is clean) and replace the single-file vitest step with full `npx vitest run` (19 files / 263 tests). - new playwright job: fidelity (continue-on-error — pre-existing failure, web-mock now lands on the Attention Inbox tab so terminal-instance never mounts) + redraw (passing, hard gate). Nightly (.github/workflows/nightly-stress.yml): - cron daily + workflow_dispatch running `npm run test:stress` on ubuntu-latest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cd8aa9d69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "release:mac": "npm run icons:macos && npm run build && electron-builder --mac --publish always", | ||
| "typecheck:node": "tsc --noEmit -p tsconfig.node.json", | ||
| "typecheck:web": "tsc --noEmit -p tsconfig.web.json", | ||
| "typecheck": "npm run typecheck:node && npm run typecheck:web", |
There was a problem hiding this comment.
Run both typecheck targets before returning
In the CI workflow I checked, .github/workflows/ci.yml runs npm run typecheck with continue-on-error while the node config is documented as currently failing; because this script uses &&, typecheck:web is short-circuited whenever typecheck:node returns non-zero. That means renderer type regressions are not exercised by the new gate until all node errors are fixed, despite the workflow comment saying both configs are checked. Split the CI steps or make the aggregate script collect both exits before failing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 649a95e, and went one further: instead of collecting both exits under one non-blocking step, CI now runs them as separate steps with typecheck:web blocking (it's clean today, so renderer regressions can't land) and typecheck:node non-blocking until the 95 listed errors are fixed. The local typecheck script was reordered web-first so the short-circuit can't hide renderer errors either. Verified typecheck:web passes.
Codex review on #196: 'npm run typecheck' short-circuits on the failing node config, so the clean renderer config was never exercised under continue-on-error. typecheck:web now gates PRs as a hard requirement; typecheck:node stays non-blocking until its 95 pre-existing errors are fixed. Local 'typecheck' script reordered web-first for the same reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Your free trial PR review limit of 300 PRs has been reached. Please upgrade your plan to continue using CodeAnt AI. |
What this does
The repo had TypeScript strict mode configured but no typecheck ran anywhere (electron-vite builds with esbuild;
npm teststrips types), and CI ran only a single vitest file (src/main/broker.test.ts) out of the full suite. This PR wires real quality gates.New npm scripts
typecheck(typecheck:node+typecheck:web) —tsc --noEmitovertsconfig.node.json(main+preload+shared) andtsconfig.web.json(renderer).test:all— the existingnode:testsuite plus the fullnpx vitest run.What now gates PRs (
checksjob)npm run typechecknpm test(node:test)npx vitest run(full suite, 19 files / 263 tests)broker.test.tsnpm run buildNew
playwrightjobplaywright.fidelity.config.ts— non-blocking (pre-existing failure, see below).playwright.redraw.config.ts— blocking (passes today).build:webonce up front; installs chromium viaplaywright install --with-deps chromium; uploads traces on failure.New
nightly-stress.ymlcron: '0 7 * * *'(daily 07:00 UTC) +workflow_dispatch, runningnpm run test:stresson ubuntu-latest with the same setup.Config fixes (needed to make the gates meaningful)
tsconfig.node.json: addedtarget: ES2022+lib: ["ES2022"]. It previously set notarget, defaulting to ES3, which produced ~60 false-positive errors (--downlevelIteration, "regex flag only available when targeting es6+",Array.prototype.at) that do not reflect the actual Node 22 / Electron runtime. The renderer config already targets ES2022. This is typecheck-only — the build is unaffected because electron-vite transpiles with esbuild, not tsc (verified:npm run buildpasses).vitest.config.mjs:testTimeout/hookTimeout→ 20s. A few timing-sensitive tests (cloud-agent.test.tsgit-overlay cases, slack writeback) exceeded the 5s default only when the suite runs as a full gate or back-to-back with node:test under load (e.g. 5008ms). They pass deterministically in isolation. Raising the timeout stabilizes the suite as a gate without skipping any test (preferred overit.skip, which would drop coverage). After the bump,npm run test:allpasses green repeatedly (node:test 115/115, vitest 263/263).Pre-existing failures and how each is handled
1. Typecheck —
tsconfig.node.jsonhas ~95 genuine pre-existing type errors → step iscontinue-on-errortsconfig.web.json(renderer) is clean (0 errors). The node side has real errors (not config noise — captured after the ES2022 target fix). Flip the CI step to blocking once these are fixed. By file:Full list (95 errors)
2. Vitest full suite — passes (263/263) after the timeout bump
No
it.skipneeded. The only failures observed were the timing flakes described above, resolved bytestTimeout.3. Playwright
fidelitysuite — pre-existing failure → step iscontinue-on-errorAll 6 fidelity specs time out waiting for
[data-testid="terminal-instance"]. Root cause: the web-mock build now lands on the Attention Inbox tab by default —src/renderer/src/stores/ui-store.tssetsinitialTabto'issues'whenVITE_PEAR_MOCK_IPC === 'true'.bootWithAgentsspawns agents but never opens the Agents/terminal view, soterminal-instancenever mounts. Fix: add a step to open the Agents view before spawning (theredrawspec already navigates via thegeneralchannel and passes). Remove the step'scontinue-on-erroronce the spec is updated.4. Playwright
redrawsuite — passes, gates as a hard requirement.Verified locally
npm run typecheck→ renderer clean; node surfaces the 95 errors above (documented, non-blocking).npm run test:all→ node:test 115/115, vitest 263/263 (green, repeated).npm run test:redraw→ 1 passed.npm run test:fidelity→ 6 failed (root cause documented; non-blocking).npm run build→ passes (tsconfig target change does not affect the esbuild build).🤖 Generated with Claude Code