Fix all node-side type errors and make typecheck:node a blocking gate#204
Conversation
Burns down the ~95 pre-existing `tsconfig.node.json` type errors and flips the CI `typecheck:node` step from continue-on-error to blocking. Fixes are type-only except where an error exposed a genuine runtime bug (restored normalizeRelayWorkspace, reconcile-stall mislabeling). Prefer real types over casts; zero new `any` (net cast count went down). Real bugs found: - store.ts: normalizeRelayWorkspace was called by setRelayWorkspace/ setRelayWorkspaceRecord but never defined (lost in the #7 wave-scaffolding merge). At runtime those paths threw ReferenceError. Restored the function and its isRecord helper from git history. - integrations.ts: the mount health observer fell through and emitted a mount-auth-stall event with undefined pendingWriteback/message for reconcile-stalled alerts (which carry neither field). Narrowed to auth-stall. Schema/type families: - store.ts/cloud-agent.ts/proactive-agent.ts: widened the persisted Project / StoreData types to include the wave-additive fields (cloudAgent, cloudAgentWorkspaceMode, proactiveAgents, relayWorkspace) that the schema already round-trips via .passthrough(). Type-only widening, no runtime change. - integration-event-bridge.ts: the @relayfile SDK narrows ChangeEvent['type'] to one literal, but the live stream delivers filesystem event types and the bridge synthesizes summary events. Aliased the SDK import and defined a module-internal widened ChangeEvent; added an asRecord() accessor to replace the `isRecord(x) ? x : {}` patterns. The TS2367 "impossible" comparisons were investigated and are correct runtime checks defeated by the narrow SDK type. - broker.ts: read reason/lastError via the existing brokerEventString accessor; added typeof guards for RelayMessageTarget's loose catch-all union member. - index.ts: guarded app.dock?.setIcon. - test files: fixed mock parameter/return typings (never[]/undefined inference). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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 34 minutes and 4 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 (13)
✨ 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.
Code Review
This pull request focuses on improving TypeScript type safety, resolving compiler warnings, and restoring missing utility functions across several files. Key changes include narrowing and widening types where appropriate (such as introducing a module-internal ChangeEvent and widening the Project type to support persisted fields), adding safe navigation operators, implementing the asRecord helper to simplify dynamic field access, and restoring the normalizeRelayWorkspace utility in store.ts. Additionally, test mocks have been updated with explicit parameter and return types. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
What this does
Burns down the ~95 pre-existing
tsconfig.node.jsontype errors documented in #196 and flips the CItypecheck:nodestep fromcontinue-on-errorto a blocking gate (and updates the now-stale comment).typecheck:webwas already clean.All fixes are type-only except where a type error revealed a genuine runtime bug (see below). Real types are preferred over
ascasts — zero newany, and the net cast count went down (removed(event as Record<string, unknown>), removed theaddProjectliteral's implicit need for a cast).Real bugs found
normalizeRelayWorkspaceundefined at runtimestore.tssetRelayWorkspace/setRelayWorkspaceRecordbut never defined — the definition was dropped in the #7 wave-scaffolding merge while the call sites survived (TS2304: Cannot find name). At runtime those code paths threwReferenceError: normalizeRelayWorkspace is not defined. Restored the function (and itsisRecordhelper) from git history.reconcile-stalledalerts mislabeled as auth stallsintegrations.tsauth-required, then fell through and emitted amount-auth-stallevent for everything else — includingreconcile-stalledalerts, which carry nopendingWriteback/messagefields. That produced a malformed event withundefinedvalues. Added a narrowif (alert.type !== 'auth-stall') returnso reconcile stalls are no longer mislabeled.The
TS2367"impossible comparison" errors inintegration-event-bridge.ts(writeback.succeeded,file.deleted,relayfile.changed.summary) were each investigated: they are correct runtime checks defeated by an overly-narrow SDK type, not logic bugs — the live change stream delivers the underlying filesystem event type and the bridge synthesizesrelayfile.changed.summaryrollups. Fixed at the type level (below), not by deleting the checks.Error families and how each was fixed
store.tsnormalizeRelayWorkspace/isRecord(real bug); widened the persistedProject/StoreDatatypes to include the wave-additive fields (cloudAgent,cloudAgentWorkspaceMode,proactiveAgents,relayWorkspace) that the zod schema already round-trips via.passthrough(). Type-only widening — no runtime change. SetrelayWorkspaceIdon theaddProjectliteral to match the schema default.cloud-agent.ts/proactive-agent.tsProjecttype widening above — fixed once at the type definition, not per call site.integration-event-bridge.tsChangeEvent as SdkChangeEvent) and defined a module-internal widenedChangeEvent(broadtypeunion + relaxedexpand) reflecting what actually flows through the bridge; added anasRecord()accessor replacing theisRecord(x) ? x : {}patterns (fixes the{}-property and conversion errors); fixed the.tsdynamic import with the existing@ts-expect-errorconvention; fixed theauthpossibly-null + token-provider return type by initializinglet authfrom the narrowedinitialAuth.broker.tsevent.reason/event.lastErrornow read via the existingbrokerEventStringaccessor; addedtypeofguards forRelayMessageTarget's loose{ kind: string; [k]: unknown }catch-all union member (truthy/non-empty behavior preserved).index.tsapp.dock?.setIcon(...).integrations.tsvi.fn()defaults that inferrednever[]/undefined/Promise<never>, typed mock params somock.callstuples have the right arity, suppliedMountLauncherStart.readyTimeoutMs, fixed thedeleteoptionality and anew Promise<unknown>+ definite-assignment for a closure-captured resolver.Verification (run locally)
npm run typecheck→ both clean (web + node, 0 errors).npm run test:all→ green: node:test 115/115, vitest 20 files, 287 passed + 1 expected-fail (288).npm run build→ passes (esbuild build unaffected; tsconfig changes are typecheck-only).Scope
Did not touch
src/main/git.ts,tests/playwright/, or eslint configs (owned by parallel branches).src/shared/schemas/project.tsis unchanged — the wave-field widening lives instore.tsso the shared schema stays generic and renderer-agnostic.🤖 Generated with Claude Code