mirror of
https://github.com/stablyai/orca.git
synced 2026-10-25 00:03:59 +00:00
refactor(catalog): drop the as casts the normalizer introduced
A change whose thesis is "stop the declared types from lying" should not use `as` to paper over types. The four source casts all came from the row normalizers returning `readonly` arrays into mutably-owned fields. Take and return mutable arrays instead, and copy at the one caller that holds a readonly projection — where identity is not load-bearing, unlike the persistence path, whose dirty check compares it. Tests built deliberately malformed rows by casting a literal. Build a valid row and `Reflect.set` the bad value onto it, which says outright that the fixture violates its type; parse the non-array case from JSON, which is how it actually arrives. Fixtures that were merely incomplete needed no cast at all — `Repo` requires only the five fields they already had. Also narrow normalizeLoadedProjectCatalog to the two fields it reads.
This commit is contained in:
@@ -44,11 +44,11 @@ export function readProfileState(profileId: string, userDataPath: string): Trans
|
||||
...parsed,
|
||||
repos: arrayOrEmpty<Repo>(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<Project>(parsed.projects)) as Project[],
|
||||
// carried straight into the importing app's state.
|
||||
projects: normalizeProjectRows(arrayOrEmpty<Project>(parsed.projects)),
|
||||
projectHostSetups: normalizeProjectHostSetupRows(
|
||||
arrayOrEmpty<ProjectHostSetup>(parsed.projectHostSetups)
|
||||
) as ProjectHostSetup[],
|
||||
),
|
||||
projectGroups: arrayOrEmpty(parsed.projectGroups),
|
||||
folderWorkspaces: arrayOrEmpty(parsed.folderWorkspaces),
|
||||
sparsePresetsByRepo: recordOrEmpty<SparsePreset[]>(parsed.sparsePresetsByRepo),
|
||||
|
||||
@@ -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<T extends object>(row: T, overrides: Record<string, unknown>): 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<Pick<PersistedState, 'projects' | 'projectHostSetups'>> {
|
||||
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)
|
||||
|
||||
@@ -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<Pick<PersistedState, 'projects' | 'projectHostSetups'>>,
|
||||
markNeedsSave: () => void
|
||||
): Pick<PersistedState, 'projects' | 'projectHostSetups'> {
|
||||
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 }
|
||||
}
|
||||
|
||||
@@ -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'
|
||||
})
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<T extends object>(row: T, overrides: Record<string, unknown>): T {
|
||||
for (const [key, value] of Object.entries(overrides)) {
|
||||
Reflect.set(row, key, value)
|
||||
}
|
||||
return row
|
||||
}
|
||||
|
||||
function makeSetup(overrides: Record<string, unknown> = {}): ProjectHostSetup {
|
||||
return {
|
||||
const setup: ProjectHostSetup = {
|
||||
id: 'setup-1',
|
||||
projectId: 'project-1',
|
||||
hostId: 'local',
|
||||
@@ -18,21 +27,21 @@ function makeSetup(overrides: Record<string, unknown> = {}): 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<string, unknown> = {}): 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([])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -79,7 +79,7 @@ export function normalizeProjectRow(project: Project): Project {
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeRows<T>(rows: readonly T[], normalize: (row: T) => T): readonly T[] {
|
||||
function normalizeRows<T>(rows: T[], normalize: (row: T) => T): T[] {
|
||||
if (!Array.isArray(rows)) {
|
||||
return []
|
||||
}
|
||||
@@ -95,12 +95,10 @@ function normalizeRows<T>(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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user