diff --git a/src/main/orca-profiles/profile-project-state-file.ts b/src/main/orca-profiles/profile-project-state-file.ts index 8eeed6f2b97..de956e33018 100644 --- a/src/main/orca-profiles/profile-project-state-file.ts +++ b/src/main/orca-profiles/profile-project-state-file.ts @@ -44,11 +44,11 @@ export function readProfileState(profileId: string, userDataPath: string): Trans ...parsed, repos: arrayOrEmpty(parsed.repos), // Why normalize: another profile's file is untrusted JSON, and a null repoId/path here would be - // carried straight into the importing app's state. Cast — these arrays stay mutably owned. - projects: normalizeProjectRows(arrayOrEmpty(parsed.projects)) as Project[], + // carried straight into the importing app's state. + projects: normalizeProjectRows(arrayOrEmpty(parsed.projects)), projectHostSetups: normalizeProjectHostSetupRows( arrayOrEmpty(parsed.projectHostSetups) - ) as ProjectHostSetup[], + ), projectGroups: arrayOrEmpty(parsed.projectGroups), folderWorkspaces: arrayOrEmpty(parsed.folderWorkspaces), sparsePresetsByRepo: recordOrEmpty(parsed.sparsePresetsByRepo), diff --git a/src/main/persistence/loading-store/normalize-loaded-project-catalog.test.ts b/src/main/persistence/loading-store/normalize-loaded-project-catalog.test.ts index 22d83ad4e3c..6371c9aa933 100644 --- a/src/main/persistence/loading-store/normalize-loaded-project-catalog.test.ts +++ b/src/main/persistence/loading-store/normalize-loaded-project-catalog.test.ts @@ -1,27 +1,43 @@ import { describe, expect, it, vi } from 'vitest' import type { PersistedState } from '../../../shared/persisted-state-types' +import type { Project, ProjectHostSetup } from '../../../shared/project-types' import { normalizeLoadedProjectCatalog } from './normalize-loaded-state-collections' -function makeParsed(setups: unknown[], projects: unknown[] = []): PersistedState { - return { projects, projectHostSetups: setups } as unknown as PersistedState +// Why Reflect.set and not a cast: these rows deliberately violate their declared types, which is +// exactly what a stale on-disk catalog does. +function corrupt(row: T, overrides: Record): T { + for (const [key, value] of Object.entries(overrides)) { + Reflect.set(row, key, value) + } + return row } -const badSetup = { - id: 'setup-1', - projectId: 'project-1', - hostId: 'local', - repoId: null, - path: null, - displayName: 'orca', - setupState: 'ready', - setupMethod: 'legacy-repo', - createdAt: 1, - updatedAt: 1 +function makeParsed( + setups: ProjectHostSetup[], + projects: Project[] = [] +): Partial> { + return { projects, projectHostSetups: setups } +} + +function makeBadSetup(): ProjectHostSetup { + const setup: ProjectHostSetup = { + id: 'setup-1', + projectId: 'project-1', + hostId: 'local', + repoId: 'repo-1', + path: '/Users/alice/orca', + displayName: 'orca', + setupState: 'ready', + setupMethod: 'legacy-repo', + createdAt: 1, + updatedAt: 1 + } + return corrupt(setup, { repoId: null, path: null }) } describe('normalizeLoadedProjectCatalog', () => { it('repairs stored rows whose field types do not match the declared ones', () => { - const result = normalizeLoadedProjectCatalog(makeParsed([{ ...badSetup }]), vi.fn()) + const result = normalizeLoadedProjectCatalog(makeParsed([makeBadSetup()]), vi.fn()) expect(result.projectHostSetups[0]?.repoId).toBe('') expect(result.projectHostSetups[0]?.path).toBe('') }) @@ -30,13 +46,13 @@ describe('normalizeLoadedProjectCatalog', () => { // host keeps publishing them to paired clients. Marking dirty is the migration. it('marks the profile dirty so the repair is persisted', () => { const markNeedsSave = vi.fn() - normalizeLoadedProjectCatalog(makeParsed([{ ...badSetup }]), markNeedsSave) + normalizeLoadedProjectCatalog(makeParsed([makeBadSetup()]), markNeedsSave) expect(markNeedsSave).toHaveBeenCalled() }) it('leaves a conforming catalog untouched and does not schedule a save', () => { const markNeedsSave = vi.fn() - const setups = [{ ...badSetup, repoId: 'repo-1', path: '/repo' }] + const setups = [{ ...makeBadSetup(), repoId: 'repo-1', path: '/repo' }] const parsed = makeParsed(setups) const result = normalizeLoadedProjectCatalog(parsed, markNeedsSave) expect(result.projectHostSetups).toBe(setups) diff --git a/src/main/persistence/loading-store/normalize-loaded-state-collections.ts b/src/main/persistence/loading-store/normalize-loaded-state-collections.ts index cf7dc883189..2b133d486b7 100644 --- a/src/main/persistence/loading-store/normalize-loaded-state-collections.ts +++ b/src/main/persistence/loading-store/normalize-loaded-state-collections.ts @@ -76,7 +76,7 @@ export function normalizeLoadedAutomationRuns( * every launch, and this host keeps publishing them to paired clients over the wire. */ export function normalizeLoadedProjectCatalog( - parsed: PersistedState, + parsed: Partial>, markNeedsSave: () => void ): Pick { const projects = normalizeProjectRows(parsed.projects ?? []) @@ -84,10 +84,5 @@ export function normalizeLoadedProjectCatalog( if (projects !== parsed.projects || projectHostSetups !== parsed.projectHostSetups) { markNeedsSave() } - // Cast: persisted catalog rows are owned mutably by the store; the normalizer is identity-preserving - // and hands back either the input array or a fresh one, never a frozen view. - return { - projects: projects as PersistedState['projects'], - projectHostSetups: projectHostSetups as PersistedState['projectHostSetups'] - } + return { projects, projectHostSetups } } diff --git a/src/renderer/src/store/projects/project-catalog-null-field-ingest.test.ts b/src/renderer/src/store/projects/project-catalog-null-field-ingest.test.ts index b810e49529e..1d57bde1b87 100644 --- a/src/renderer/src/store/projects/project-catalog-null-field-ingest.test.ts +++ b/src/renderer/src/store/projects/project-catalog-null-field-ingest.test.ts @@ -10,7 +10,7 @@ import { fetchProjectHostSetupCompatibility, setupWithFetchedOwner } from './pro // row is in the store, its declared field types hold. const repos = [ { id: 'repo-1', path: '/Users/alice/orca', displayName: 'orca', badgeColor: '#000', addedAt: 1 } -] as unknown as Repo[] +] satisfies Repo[] const projects = [ { @@ -21,7 +21,7 @@ const projects = [ createdAt: 1, updatedAt: 1 } -] as unknown as Project[] +] satisfies Project[] // Mirrors the Settings.tsx projectByRepoId useMemo that crashed. function buildProjectByRepoIdLikeSettings( @@ -44,36 +44,28 @@ function checkoutIdentityLikeSidebar(setup: ProjectHostSetup): string { return setup.path.trim() || setup.repoId || setup.id } -function badSetups(): unknown[] { - return [ - { - id: 'repo:repo-1::local', - projectId: 'repo:repo-1', - hostId: 'local', - repoId: 'repo-1', - path: '/Users/alice/orca', - displayName: 'orca', - setupState: 'ready', - setupMethod: 'legacy-repo', - createdAt: 1, - updatedAt: 1 - }, - { - id: 'repo:repo-1::local::2', - projectId: 'repo:repo-1', - hostId: 'local', - repoId: null, - path: null, - displayName: 'orca-2', - setupState: 'ready', - setupMethod: 'legacy-repo', - createdAt: 1, - updatedAt: 1 - } - ] +// Why Reflect.set and not a cast: the second row deliberately violates its declared types, which +// is exactly what an older host publishes over the wire. +function badSetups(): ProjectHostSetup[] { + const base = (id: string, displayName: string): ProjectHostSetup => ({ + id, + projectId: 'repo:repo-1', + hostId: 'local', + repoId: 'repo-1', + path: '/Users/alice/orca', + displayName, + setupState: 'ready', + setupMethod: 'legacy-repo', + createdAt: 1, + updatedAt: 1 + }) + const corrupted = base('repo:repo-1::local::2', 'orca-2') + Reflect.set(corrupted, 'repoId', null) + Reflect.set(corrupted, 'path', null) + return [base('repo:repo-1::local', 'orca'), corrupted] } -function stubProjectsApi(setups: unknown[], projectRows: unknown[] = projects): void { +function stubProjectsApi(setups: ProjectHostSetup[], projectRows: Project[] = projects): void { vi.stubGlobal('window', { api: { projects: { @@ -96,7 +88,7 @@ describe('project catalog ingest with non-string row fields', () => { // Why: a remote host on a different Orca version is a first-class source of these rows, and // decoders hand them over verbatim — the client cannot assume the host already repaired them. it('coerces on the remote adoption boundary too', () => { - const adopted = setupWithFetchedOwner(badSetups()[1] as ProjectHostSetup, { + const adopted = setupWithFetchedOwner(badSetups()[1]!, { kind: 'environment', environmentId: 'env-1' }) diff --git a/src/renderer/src/store/projects/project-host-routing.ts b/src/renderer/src/store/projects/project-host-routing.ts index f1d63b0aa94..1a4a45ea03a 100644 --- a/src/renderer/src/store/projects/project-host-routing.ts +++ b/src/renderer/src/store/projects/project-host-routing.ts @@ -78,8 +78,8 @@ function normalizeProjectCatalogProjection( projection: ProjectHostSetupProjection ): ProjectHostSetupProjection { return { - projects: normalizeProjectRows(projection.projects), - setups: normalizeProjectHostSetupRows(projection.setups) + projects: normalizeProjectRows([...projection.projects]), + setups: normalizeProjectHostSetupRows([...projection.setups]) } } @@ -129,7 +129,7 @@ export async function fetchProjectHostSetupCompatibility( return { // Why projects too: the same wire response carries them, and a remote host on another // Orca version can publish a row whose declared field types do not hold. - projects: normalizeProjectRows(projectResponse.projects), + projects: normalizeProjectRows([...projectResponse.projects]), setups: setupResponse.setups.map((setup) => setupWithFetchedOwner(setup, target)) } } catch { diff --git a/src/shared/project-catalog-row-normalization.test.ts b/src/shared/project-catalog-row-normalization.test.ts index 25481d703be..e22715f7fda 100644 --- a/src/shared/project-catalog-row-normalization.test.ts +++ b/src/shared/project-catalog-row-normalization.test.ts @@ -7,8 +7,17 @@ import { normalizeProjectRows } from './project-catalog-row-normalization' +// Why Reflect.set and not a cast: these fixtures deliberately violate the declared types, which is +// the whole point — writing the bad value onto a valid row says that outright. +function corrupt(row: T, overrides: Record): T { + for (const [key, value] of Object.entries(overrides)) { + Reflect.set(row, key, value) + } + return row +} + function makeSetup(overrides: Record = {}): ProjectHostSetup { - return { + const setup: ProjectHostSetup = { id: 'setup-1', projectId: 'project-1', hostId: 'local', @@ -18,21 +27,21 @@ function makeSetup(overrides: Record = {}): ProjectHostSetup { setupState: 'ready', setupMethod: 'legacy-repo', createdAt: 1, - updatedAt: 2, - ...overrides - } as unknown as ProjectHostSetup + updatedAt: 2 + } + return corrupt(setup, overrides) } function makeProject(overrides: Record = {}): Project { - return { + const project: Project = { id: 'project-1', displayName: 'Project', badgeColor: '#737373', sourceRepoIds: ['repo-1'], createdAt: 1, - updatedAt: 2, - ...overrides - } as unknown as Project + updatedAt: 2 + } + return corrupt(project, overrides) } describe('normalizeProjectHostSetupRow', () => { @@ -175,7 +184,10 @@ describe('row array normalization', () => { }) it('treats a non-array as empty', () => { - expect(normalizeProjectHostSetupRows(null as unknown as ProjectHostSetup[])).toEqual([]) - expect(normalizeProjectRows(null as unknown as Project[])).toEqual([]) + // Why JSON.parse: a persisted catalog really can hold a non-array here, and parsing is how it + // arrives — the parameter type cannot express it. + const notAnArray: never[] = JSON.parse('null') + expect(normalizeProjectHostSetupRows(notAnArray)).toEqual([]) + expect(normalizeProjectRows(notAnArray)).toEqual([]) }) }) diff --git a/src/shared/project-catalog-row-normalization.ts b/src/shared/project-catalog-row-normalization.ts index 04396b48c34..d14c1681279 100644 --- a/src/shared/project-catalog-row-normalization.ts +++ b/src/shared/project-catalog-row-normalization.ts @@ -79,7 +79,7 @@ export function normalizeProjectRow(project: Project): Project { } } -function normalizeRows(rows: readonly T[], normalize: (row: T) => T): readonly T[] { +function normalizeRows(rows: T[], normalize: (row: T) => T): T[] { if (!Array.isArray(rows)) { return [] } @@ -95,12 +95,10 @@ function normalizeRows(rows: readonly T[], normalize: (row: T) => T): readonl return repaired ?? rows } -export function normalizeProjectHostSetupRows( - setups: readonly ProjectHostSetup[] -): readonly ProjectHostSetup[] { +export function normalizeProjectHostSetupRows(setups: ProjectHostSetup[]): ProjectHostSetup[] { return normalizeRows(setups, normalizeProjectHostSetupRow) } -export function normalizeProjectRows(projects: readonly Project[]): readonly Project[] { +export function normalizeProjectRows(projects: Project[]): Project[] { return normalizeRows(projects, normalizeProjectRow) }