fix: type-check src/gui instead of excluding it - #159
Merged
Conversation
src/gui was listed in tsconfig.json's `exclude` and has no tests, yet it imports from src/core and src/version. Nothing checked it, so a signature change in core would only surface when release-gui.yml runs — which was three weeks before this commit. It cannot simply join the main tsconfig: it runs under Bun, using `Bun`, `import.meta.path`, and webview-bun's FFI types, and Bun resolves modules like a bundler rather than like Node. It gets its own tsconfig.gui.json, wired into `npm run check`. Two things the check found once it was actually running: - `exclude` is inherited from the base config and beats the `include` in the derived one. The first version of tsconfig.gui.json therefore checked nothing at all — it reported success with a deliberate type error sitting in src/gui/index.ts. Restated `exclude` fixes it; the comment says why so it is not "simplified" back. - `resolveUrl` tripped TS2454. A Promise executor runs synchronously, so it is assigned before the reader can reach it; a definite assignment assertion states what TypeScript cannot infer. Verified twice by planting a type error in src/gui and confirming the check fails, then confirming it passes once removed — a check that cannot fail is worse than no check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
Summary
src/guiwas listed intsconfig.json'sexcludeand has no tests, yet it imports fromsrc/core/guiUpdate.js,src/core/selfUpdate.js, andsrc/version.js. Nothing checked it, so a signature change incorewould only surface whenrelease-gui.ymlruns — which was three weeks ago.It cannot just join the main tsconfig: it runs under Bun, using
Bun,import.meta.path, and webview-bun's FFI types, and Bun resolves modules like a bundler rather than like Node. So it getstsconfig.gui.json, wired intonpm run checkastypecheck:gui.Two things the check found once it was actually running
The first version of this config checked nothing.
excludeis inherited from the base config and beats theincludein the derived one, sosrc/guistayed excluded — the check reported success with a deliberate type error sitting insrc/gui/index.ts. Restatingexcludefixes it, and the comment says why so it does not get "simplified" back later. A check that cannot fail is worse than no check: it manufactures confidence.resolveUrltripped TS2454 (used before assigned). A Promise executor runs synchronously, so it is assigned before the reader can reach it — this is a false positive, and a definite assignment assertion states what TypeScript cannot infer. Not a behaviour change.Testing
Verified the check can actually fail, twice: planted
const __p: number = "x"insrc/gui/index.ts, confirmedtypecheck:guireports the error, removed it, confirmed the check passes. Without that step this PR would have looked identical while doing nothing.npm run checkgreen: typecheck, typecheck:gui, and 323 tests across 49 files.Security and AI Governance
docs/.docs/governance/AI-CHANGE-IMPACT-ASSESSMENT.md.Second and third boxes: this adds a type check and one
!assertion. No runtime behaviour, no new model, provider, tool, or data flow.@types/bunis added as a devDependency.Notes
Found while auditing which workflow steps have never executed under real conditions. The other finding from that audit is not fixed here and needs your decision: the macOS binaries ship ad-hoc signed and Gatekeeper-rejected — verified on the actual v0.15.0 download with
codesign(Signature=adhoc,TeamIdentifier=not set) andspctl(rejected). Four signing and notarization steps acrossrelease.ymlandrelease-gui.ymlhave never run because none of the six Apple secrets are configured. That needs an Apple Developer account, so it is a product decision, not a patch.