Repository navigation
ci: build the overlay so type errors cannot ship - #67
Conversation
The overlay is a separate npm project with its own tsconfig, so neither the root typecheck nor the test job covered it. A type error that broke its build outright reached main and shipped as v0.19.0 with CI reporting green throughout. Its build is tsc && vite build, so building it in CI is what catches this. Verified against the offending commit: the step exits 2 on d535653 and 0 once fixed. Electron's binary download is skipped - CI only typechecks and bundles the renderer, and never launches Electron. Note: the overlay has no lockfile, so this uses npm install rather than npm ci and is not fully reproducible. Committing a lockfile is a separate decision.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe test workflow now installs dependencies for the ChangesOverlay CI Validation
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af3a1a33b3
ℹ️ 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".
|
|
||
| - name: Build overlay | ||
| working-directory: overlay | ||
| run: npm run build |
There was a problem hiding this comment.
In the current tree this new step will make every PR/main test run fail: overlay/package.json defines build as tsc && vite build, and overlay/src/ipc-client.ts imports ./shared/ipc-types, but there is no overlay/src/shared file and overlay/tsconfig.json only includes src/**/* with no path mapping. I checked the repo-wide references to ipc-types; the only implementation is under the root src/shared, so the newly enforced tsc pass cannot resolve this module until the shared types are copied or the overlay project is wired to the root source correctly.
Useful? React with 👍 / 👎.
1 participant
The overlay is a separate npm project with its own tsconfig, so neither the root typecheck nor the test job covered it. A type error that broke its build outright reached
mainand shipped as v0.19.0 with CI reporting green throughout.Its build is
tsc && vite build, so building it in CI is what catches this.Verified in a clean worktree (no pre-existing node_modules)
npm installwith Electron binary skippednpm run buildon fixed codenpm run buildon the offending commitd535653The step demonstrably fails on the regression that shipped, rather than merely passing on good code.
Electron's binary download is skipped — CI only typechecks and bundles the renderer, and never launches Electron.
Known limitation
The overlay has no lockfile, so this uses
npm installrather thannpm ciand is not fully reproducible. Committing a lockfile is a separate decision.Summary by CodeRabbit