From 981eecd1038d803523e555729f77c64235f25ab1 Mon Sep 17 00:00:00 2001 From: Will Washburn Date: Wed, 10 Jun 2026 07:35:09 -0400 Subject: [PATCH] Fix all node-side type errors and make typecheck:node a blocking gate 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 --- .github/workflows/ci.yml | 7 +-- src/main/auth.test.ts | 8 +++- src/main/broker.test.ts | 4 +- src/main/broker.ts | 18 +++++-- src/main/cloud-agent.test.ts | 2 +- src/main/git.test.ts | 2 +- src/main/index.ts | 2 +- src/main/integration-event-bridge.ts | 57 +++++++++++++++++------ src/main/integration-mounts.test.ts | 2 +- src/main/integrations.test.ts | 10 ++-- src/main/integrations.ts | 6 +++ src/main/relayfile-mount-launcher.test.ts | 9 ++-- src/main/store.ts | 55 ++++++++++++++++++++-- 13 files changed, 140 insertions(+), 42 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 50f1df45..c2da07c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,12 +21,9 @@ jobs: # Renderer typecheck (tsconfig.web.json) is clean today — BLOCKING so # renderer type regressions can't land. - run: npm run typecheck:web - # Main+preload typecheck (tsconfig.node.json) surfaces ~95 pre-existing - # type errors (see PR "CI: enforce typecheck and run the full test suite" - # for the full list). NON-BLOCKING until those are fixed; then delete the - # flag (and this comment) so it becomes a hard gate too. + # Main+preload typecheck (tsconfig.node.json) is clean today — BLOCKING so + # main/preload type regressions can't land. - run: npm run typecheck:node - continue-on-error: true - run: npm test # Full vitest suite (node + dom projects). Previously CI only ran the single # src/main/broker.test.ts file; this now gates the entire suite (19 files / 263 tests). diff --git a/src/main/auth.test.ts b/src/main/auth.test.ts index f626e2f2..4de061b7 100644 --- a/src/main/auth.test.ts +++ b/src/main/auth.test.ts @@ -591,8 +591,12 @@ describe('getAccessToken (refresh flow)', () => { apiUrl: 'https://cloud.example', expiresAt: new Date(Date.now() - 1_000).toISOString() }) - let resolveFetch: ((value: unknown) => void) | null = null - mock.fetchMock.mockImplementationOnce(() => new Promise((resolve) => { + // Promise is explicit: the executor never calls resolve() itself + // (the test does, below), so TS would otherwise infer the resolve param as + // never. The definite-assignment `!` keeps resolveFetch callable since the + // mock implementation assigns it synchronously when fetch is first invoked. + let resolveFetch!: (value: unknown) => void + mock.fetchMock.mockImplementationOnce(() => new Promise((resolve) => { resolveFetch = resolve })) diff --git a/src/main/broker.test.ts b/src/main/broker.test.ts index fbc6274b..ed3bcd03 100644 --- a/src/main/broker.test.ts +++ b/src/main/broker.test.ts @@ -119,7 +119,7 @@ const mock = vi.hoisted(() => { } class HarnessDriverClient { - static spawn = vi.fn(async () => { + static spawn = vi.fn(async (_options: unknown) => { const client = createMockClient(state.nextLocalAgents.splice(0)) state.spawnedClients.push(client) return client @@ -224,7 +224,7 @@ function restoreProcessResourcesPath(): void { if (originalResourcesPathDescriptor) { Object.defineProperty(process, 'resourcesPath', originalResourcesPathDescriptor) } else { - delete (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath + delete (process as { resourcesPath?: string }).resourcesPath } } diff --git a/src/main/broker.ts b/src/main/broker.ts index e7b6e2db..1d34bfc8 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -649,8 +649,10 @@ function isDeliveryEventForMessage( function deliveryFailureMessage(event: BrokerEvent): string { if (!isRecord(event)) return 'Broker delivery failed' - const reason = typeof event.reason === 'string' ? event.reason : undefined - const lastError = typeof event.lastError === 'string' ? event.lastError : undefined + // reason/lastError are not declared on the base BrokerEvent union; read them + // through the same dynamic accessor used for other optional broker fields. + const reason = brokerEventString(event, 'reason') + const lastError = brokerEventString(event, 'lastError') return reason || lastError || 'Broker delivery failed' } @@ -841,8 +843,16 @@ function directMessageTargetFromRelayMessage( .filter(Boolean) const target = message.target - if (target?.kind === 'agent' && target.agentName) return target.agentName - if (target?.kind === 'channel' && target.channelName) return normalizeChatChannelTarget(target.channelName) + // RelayMessageTarget's union includes a loose `{ kind: string; [k]: unknown }` + // catch-all member, so a `kind` check alone leaves agentName/channelName typed + // as `unknown`; the typeof guard narrows them to string (and keeps the + // existing truthy/non-empty behavior). + if (target?.kind === 'agent' && typeof target.agentName === 'string' && target.agentName) { + return target.agentName + } + if (target?.kind === 'channel' && typeof target.channelName === 'string' && target.channelName) { + return normalizeChatChannelTarget(target.channelName) + } if (normalizedParticipants.length > 0) { const otherParticipants = normalizedParticipants.filter((participant) => diff --git a/src/main/cloud-agent.test.ts b/src/main/cloud-agent.test.ts index fe52e63b..c292071a 100644 --- a/src/main/cloud-agent.test.ts +++ b/src/main/cloud-agent.test.ts @@ -66,7 +66,7 @@ const mock = vi.hoisted(() => { onBrokerEvent: vi.fn(), attachCloudSandbox: vi.fn(async () => undefined), detachCloudSandbox: vi.fn(async () => undefined), - workspaceKeyForProject: vi.fn(async () => undefined) + workspaceKeyForProject: vi.fn(async (): Promise => undefined) }, fetch: vi.fn(async (url: string | URL | Request, init?: RequestInit) => { const normalizedUrl = String(url) diff --git a/src/main/git.test.ts b/src/main/git.test.ts index 6fc7a047..08cf0f92 100644 --- a/src/main/git.test.ts +++ b/src/main/git.test.ts @@ -48,7 +48,7 @@ const gitEnvKeys = [ const originalGitEnv = new Map() function gitEnv(): NodeJS.ProcessEnv { - const env = { + const env: NodeJS.ProcessEnv = { ...process.env, GIT_CONFIG_NOSYSTEM: '1', GIT_TERMINAL_PROMPT: '0', diff --git a/src/main/index.ts b/src/main/index.ts index d62240d9..dc63eff7 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -314,7 +314,7 @@ if (!gotSingleInstanceLock) { }) if (process.platform === 'darwin' && appIcon) { - app.dock.setIcon(appIcon) + app.dock?.setIcon(appIcon) } registerAvatarCacheProtocol() diff --git a/src/main/integration-event-bridge.ts b/src/main/integration-event-bridge.ts index d25566a1..8c6424ef 100644 --- a/src/main/integration-event-bridge.ts +++ b/src/main/integration-event-bridge.ts @@ -7,12 +7,29 @@ import { RelayFileClient, RelayFileSync, RelayfileSetup, - type ChangeEvent, + type ChangeEvent as SdkChangeEvent, + type Expansion, + type ExpansionLevel, type FileReadResponse, type FilesystemEvent, + type FilesystemEventType, type RelayFileSyncOptions, type Subscription } from '@relayfile/sdk' + +// Module-internal view of a relayfile change event as it actually flows through +// this bridge. The @relayfile SDK narrows `ChangeEvent['type']` to the single +// literal 'relayfile.changed' and types `expand` as a generic method, but in +// practice the live change stream delivers the underlying filesystem event type +// (e.g. 'writeback.succeeded', 'file.deleted') and this bridge additionally +// synthesizes 'relayfile.changed.summary' rollup events (see +// dispatchSummaryEvent). Widening `type` and relaxing `expand` here keeps every +// synthesized event and runtime discriminant check honest without per-site +// casts. SDK-delivered events (the narrow type) remain assignable to this. +type ChangeEvent = Omit & { + type: SdkChangeEvent['type'] | FilesystemEventType | 'relayfile.changed.summary' + expand: (level?: ExpansionLevel) => Promise +} import type { ConnectedIntegration } from './integrations' // @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. import { isSlackWritebackCommandRoot, slackWritebackCommandMountPathFor } from './slack-writeback-command-roots.ts' @@ -409,6 +426,13 @@ function isRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value) } +// View any value as a string-keyed record for dynamic field access. Non-record +// inputs collapse to an empty record so callers can read optional keys off +// ChangeEvent/resource/summary without tripping over the SDK's narrow types. +function asRecord(value: unknown): Record { + return isRecord(value) ? value : {} +} + function stringList(value: unknown): string[] { if (!Array.isArray(value)) return [] return value.filter((entry): entry is string => typeof entry === 'string' && entry.trim().length > 0) @@ -1398,9 +1422,9 @@ function injectionDeduplicationKey(projectId: string, event: ChangeEvent, matche } function eventRecordValue(event: ChangeEvent, key: string): unknown { - const resource = isRecord(event.resource) ? event.resource : {} - const summary = isRecord(event.summary) ? event.summary : {} - return (event as Record)[key] ?? resource[key] ?? summary[key] + const resource = asRecord(event.resource) + const summary = asRecord(event.summary) + return asRecord(event)[key] ?? resource[key] ?? summary[key] } function eventOrigin(event: ChangeEvent): string | null { @@ -1733,8 +1757,8 @@ function eventSummaryValue(value: unknown): string | undefined { } function integrationEventMetadata(event: ChangeEvent): Record { - const summary = isRecord(event.summary) ? event.summary : {} - const resource = isRecord(event.resource) ? event.resource : {} + const summary = asRecord(event.summary) + const resource = asRecord(event.resource) const actor = isRecord(summary.actor) ? eventSummaryValue(summary.actor.displayName) || eventSummaryValue(summary.actor.id) : undefined @@ -2080,7 +2104,7 @@ function formatSlackIntegrationEventMessage( contextPreview?: EventContextPreview, resolvedPath?: string ): string | null { - const resource = isRecord(event.resource) ? event.resource : {} + const resource = asRecord(event.resource) const provider = eventSummaryValue(resource.provider) || eventProvider(event) const relayfilePath = eventSummaryValue(resource.path) if (provider !== 'slack' || !relayfilePath || !slackEventContextPath(relayfilePath)) return null @@ -2119,8 +2143,8 @@ function formatIntegrationEventMessage( const slackMessage = formatSlackIntegrationEventMessage(event, contextPreview, resolvedPath) if (slackMessage) return slackMessage - const summary = isRecord(event.summary) ? event.summary : {} - const resource = isRecord(event.resource) ? event.resource : {} + const summary = asRecord(event.summary) + const resource = asRecord(event.resource) const provider = eventSummaryValue(resource.provider) || 'integration' const relayfilePath = eventSummaryValue(resource.path) const displayPath = resolvedPath || relayfilePath @@ -2714,7 +2738,7 @@ export class IntegrationEventBridge { try { const expanded = await event.expand('full') - const expandedRecord = isRecord(expanded) ? expanded : {} + const expandedRecord = asRecord(expanded) return eventContextPreviewFromData( typeof expandedRecord.path === 'string' ? expandedRecord.path : path, expandedRecord.data @@ -3390,12 +3414,17 @@ export class IntegrationEventBridge { private async getWorkspaceHandle(): Promise { if (this.deps.getWorkspaceHandle) return this.deps.getWorkspaceHandle() + // @ts-expect-error Node's strip-types test runner requires the explicit .ts extension. const { accountWorkspaceReadyRetryOptions, getAccountWorkspaceId, refreshCloudAuth, resolveCloudAuth } = await import('./auth.ts') - let auth = await resolveCloudAuth() - if (!auth) { + const initialAuth = await resolveCloudAuth() + if (!initialAuth) { accountIntegrationEventHandle = null throw new Error('cloud-auth-required') } + // Non-null from here on: reassigned only to a refreshed CloudAuth below. + // Initializing from the narrowed `initialAuth` keeps the type non-null + // inside the joinWorkspace closure (a plain `let` would widen back to null). + let auth = initialAuth const accountWorkspaceId = await getAccountWorkspaceId(accountWorkspaceReadyRetryOptions()) if ( @@ -3408,9 +3437,9 @@ export class IntegrationEventBridge { } const joinWorkspace = async () => { - const tokenProvider = async (): Promise => { + const tokenProvider = async (): Promise => { const fresh = await resolveCloudAuth() - return fresh?.accessToken ?? auth?.accessToken + return fresh?.accessToken ?? auth.accessToken } const setup = new RelayfileSetup({ cloudApiUrl: auth.apiUrl, diff --git a/src/main/integration-mounts.test.ts b/src/main/integration-mounts.test.ts index 01f4b8c7..5162b208 100644 --- a/src/main/integration-mounts.test.ts +++ b/src/main/integration-mounts.test.ts @@ -64,7 +64,7 @@ const mock = vi.hoisted(() => { startMount, mkdir: vi.fn(async () => undefined), chmod: vi.fn(async () => undefined), - readFile: vi.fn(async () => '{}'), + readFile: vi.fn(async (_path: string) => '{}'), rm: vi.fn(async () => undefined), RelayfileSetup, get currentAuth() { diff --git a/src/main/integrations.test.ts b/src/main/integrations.test.ts index 6cd69e90..257d2f87 100644 --- a/src/main/integrations.test.ts +++ b/src/main/integrations.test.ts @@ -62,7 +62,7 @@ const mock = vi.hoisted(() => { const workspaceHandle = { workspaceId: 'account-workspace-id', client: vi.fn(() => relayClient), - requestJson: vi.fn(async (_request: { path: string }) => { + requestJson: vi.fn(async (_request: { path: string }): Promise => { throw new Error('unexpected workspace request') }), refreshToken: vi.fn(async () => undefined) @@ -177,8 +177,8 @@ const mock = vi.hoisted(() => { saveStore: vi.fn(() => undefined), integrationMountManager: { ensureMounted: vi.fn(() => mountReconcilePromise), - currentWorkspaceId: vi.fn(() => null), - localPathsFor: vi.fn(() => []), + currentWorkspaceId: vi.fn((): string | null => null), + localPathsFor: vi.fn((): string[] => []), setHealthObserver: vi.fn(), stop: vi.fn(async () => undefined) }, @@ -195,8 +195,8 @@ const mock = vi.hoisted(() => { relayWorkspaceManager, workspaceHandle, brokerManager: { - listAgents: vi.fn(async () => []), - sendMessage: vi.fn(async () => undefined), + listAgents: vi.fn(async (): Promise> => []), + sendMessage: vi.fn(async (_projectId: string | undefined, _input: unknown) => undefined), sendMessageAndWaitForDelivery: vi.fn(async () => undefined) }, ensureProjectIntegrationsLink: vi.fn(async () => undefined), diff --git a/src/main/integrations.ts b/src/main/integrations.ts index 5174185a..cd1b0403 100644 --- a/src/main/integrations.ts +++ b/src/main/integrations.ts @@ -822,6 +822,12 @@ export class IntegrationsManager { this.setAuthRecoveryState(alert.reason, undefined, alert.message) return } + // Only auth-stall alerts carry pendingWriteback/message and map to a + // mount-auth-stall event. reconcile-stalled alerts have neither field, so + // emitting one here previously produced a malformed event with undefined + // pendingWriteback/message; ignore them rather than mislabel a reconcile + // stall as an auth stall. + if (alert.type !== 'auth-stall') return this.emit({ type: 'mount-auth-stall', remotePath: alert.remotePath, diff --git a/src/main/relayfile-mount-launcher.test.ts b/src/main/relayfile-mount-launcher.test.ts index 3a7a515f..12862dbc 100644 --- a/src/main/relayfile-mount-launcher.test.ts +++ b/src/main/relayfile-mount-launcher.test.ts @@ -54,7 +54,8 @@ describe('createPearMountLauncher', () => { env: { RELAYFILE_TOKEN: 'relay_pa_test-token', RELAYFILE_LOCAL_DIR: localDir - } + }, + readyTimeoutMs: 5_000 }) const credsPath = join(localDir, '.relay', 'creds.json') @@ -78,12 +79,14 @@ describe('createPearMountLauncher', () => { await launcher.start({ env: { RELAYFILE_LOCAL_DIR: join(tempDir, 'missing-token') - } + }, + readyTimeoutMs: 5_000 }) await launcher.start({ env: { RELAYFILE_TOKEN: 'relay_pa_test-token' - } + }, + readyTimeoutMs: 5_000 }) expect(mock.start).toHaveBeenCalledTimes(2) diff --git a/src/main/store.ts b/src/main/store.ts index f17e8874..452dcdbb 100644 --- a/src/main/store.ts +++ b/src/main/store.ts @@ -15,10 +15,27 @@ import { const ProjectSchema = makeProjectSchema(ProjectRootSchema) const StoreSchema = StoreDataSchema(ProjectSchema) +export type CloudAgentWorkspaceMode = 'git-overlay' | 'git' | 'relayfile' + export type ProjectRoot = z.infer export type ProjectIntegration = z.infer -export type Project = z.infer -type StoreData = z.infer + +// The persisted project schema parses with `.passthrough()`, so wave-additive +// fields (cloud-agent / proactive-agent / workspace mode) survive a load/save +// round-trip on disk even though the base zod schema does not name them. Widen +// the static type to match what is actually persisted and read back, so the +// main-process consumers (cloud-agent.ts, proactive-agent.ts) see real types +// instead of property-not-found errors. No runtime behavior changes. +export type Project = z.infer & { + cloudAgent?: ProjectCloudAgent + cloudAgentWorkspaceMode?: CloudAgentWorkspaceMode + proactiveAgents?: ProactiveAgentBinding[] +} + +type StoreData = Omit, 'projects'> & { + projects: Project[] + relayWorkspace?: RelayWorkspaceRecord +} const defaultData: StoreData = { projects: [], activeProjectId: null } @@ -66,6 +83,35 @@ function defaultRootName(path: string): string { return basename(path) || path } +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +// Normalize an untrusted relay-workspace record read from disk or supplied by a +// caller into a clean RelayWorkspaceRecord, or undefined when it lacks a usable +// id. Restores behavior lost in the wave-scaffolding merge (the call sites in +// setRelayWorkspace/setRelayWorkspaceRecord survived without the definition). +function normalizeRelayWorkspace(value: unknown): RelayWorkspaceRecord | undefined { + if (!isRecord(value)) return undefined + + const id = typeof value.id === 'string' ? value.id.trim() : '' + if (!id) return undefined + + const createdAt = typeof value.createdAt === 'string' ? value.createdAt.trim() : '' + const createdAtTime = createdAt ? Date.parse(createdAt) : Number.NaN + const apiUrl = typeof value.apiUrl === 'string' ? value.apiUrl.trim().replace(/\/+$/, '') : '' + const authKey = typeof value.authKey === 'string' ? value.authKey.trim() : '' + + return { + id, + createdAt: Number.isNaN(createdAtTime) + ? new Date(0).toISOString() + : new Date(createdAtTime).toISOString(), + ...(apiUrl ? { apiUrl } : {}), + ...(authKey ? { authKey } : {}) + } +} + function loadStoreFromDisk(): StoreData { try { const raw = readFileSync(getStorePath(), 'utf-8') @@ -123,9 +169,12 @@ export function setRelayWorkspaceRecord(record: RelayWorkspaceRecord | null): vo export function addProject(name: string, rootPath: string): Project { const data = loadStore() + const id = crypto.randomUUID() const project: Project = { - id: crypto.randomUUID(), + id, name, + // Mirrors the schema default (relayWorkspaceId falls back to the project id). + relayWorkspaceId: id, rootPath, roots: [{ id: crypto.randomUUID(), name: defaultRootName(rootPath), path: rootPath }], channels: ['general'],