diff --git a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.spec.ts b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.spec.ts index 39dd906467..2b6f49f6ee 100644 --- a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.spec.ts +++ b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.spec.ts @@ -44,4 +44,16 @@ describe('getProjectVisibilityIconColor', () => { expect(getProjectVisibilityIconColor(project)).toBe('#abcdef'); }); + + it('does not throw for a project persisted without a theme (#9139)', () => { + // A project entity can reach the store with no `theme` at all. This helper + // renders once per project in the side nav on every launch, and because + // DEFAULT_PROJECT_ICON is not an emoji the theme branch is the DEFAULT + // path — so an unguarded deref crashed the app at startup. + const project = createProject({ icon: 'work' }); + delete (project as unknown as Record).theme; + + expect(() => getProjectVisibilityIconColor(project)).not.toThrow(); + expect(getProjectVisibilityIconColor(project)).toBeNull(); + }); }); diff --git a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts index 0f888a2bca..b080746fd2 100644 --- a/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts +++ b/src/app/core-ui/magic-side-nav/nav-list/nav-list-tree.component.ts @@ -44,7 +44,7 @@ const EXPAND_ANIMATION_RESET_DELAY_MS = 250; export const getProjectVisibilityIconColor = (project: Project): string | null => isSingleEmoji(project.icon || DEFAULT_PROJECT_ICON) ? null - : (project.theme.primary ?? null); + : (project.theme?.primary ?? null); @Component({ selector: 'nav-list-tree', diff --git a/src/app/features/note/note/note.component.html b/src/app/features/note/note/note.component.html index 77fcf385b0..1c3c8f2dd7 100644 --- a/src/app/features/note/note/note.component.html +++ b/src/app/features/note/note/note.component.html @@ -119,7 +119,7 @@ mat-menu-item > {{ project.icon || DEFAULT_PROJECT_ICON }} diff --git a/src/app/features/work-context/work-context.service.spec.ts b/src/app/features/work-context/work-context.service.spec.ts index 36421a2f37..11b2db3572 100644 --- a/src/app/features/work-context/work-context.service.spec.ts +++ b/src/app/features/work-context/work-context.service.spec.ts @@ -2,6 +2,7 @@ import { TestBed } from '@angular/core/testing'; import { of } from 'rxjs'; import { resolveContextTheme, WorkContextService } from './work-context.service'; import { DEFAULT_TAG_COLOR, WORK_CONTEXT_DEFAULT_THEME } from './work-context.const'; +import { DEFAULT_PROJECT } from '../project/project.const'; import { TaskWithSubTasks } from '../tasks/task.model'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { provideMockActions } from '@ngrx/effects/testing'; @@ -12,7 +13,7 @@ import { GlobalTrackingIntervalService } from '../../core/global-tracking-interv import { DateService } from '../../core/date/date.service'; import { TimeTrackingService } from '../time-tracking/time-tracking.service'; import { TaskArchiveService } from '../archive/task-archive.service'; -import { TODAY_TAG } from '../tag/tag.const'; +import { DEFAULT_TAG, TODAY_TAG } from '../tag/tag.const'; import { WorkContext, WorkContextType } from './work-context.model'; import { selectActiveContextId, @@ -763,25 +764,31 @@ describe('resolveContextTheme()', () => { describe('regression #9139: work context persisted with no theme', () => { // A tag/project entity stored without `theme` propagated `undefined` into // resolveBackground() and _setColorTheme(), crashing on every launch. - it('returns the default theme instead of undefined', () => { + // The crashing consumers were resolveBackground() (reads backgroundImage*) + // and _setColorTheme() (reads isAutoContrast); both need a real object. + it('returns the default tag theme instead of undefined', () => { const ctx = buildCtx(); delete (ctx as unknown as Record).theme; - expect(resolveContextTheme(ctx)).toEqual(WORK_CONTEXT_DEFAULT_THEME); + // Returns the shared constant by reference — deliberate: both consumers + // only read, and a stable identity helps the downstream + // distinctUntilChanged. `toBe` pins that decision. + expect(resolveContextTheme(ctx)).toBe(DEFAULT_TAG.theme); }); - it('returns a theme the crashing consumers can dereference', () => { - const ctx = buildCtx(); + it('returns the PROJECT default for a theme-less project', () => { + // Must match what auto-fix-typia-errors would later persist, else a + // theme-less project renders tag-purple then flips to project-teal. + const ctx = buildCtx({ type: WorkContextType.PROJECT }); delete (ctx as unknown as Record).theme; - const res = resolveContextTheme(ctx); + expect(resolveContextTheme(ctx)).toBe(DEFAULT_PROJECT.theme); + }); - // The exact fields whose unguarded deref produced the reported crashes: - // resolveBackground() reads backgroundImage*, _setColorTheme() reads - // isAutoContrast. - expect(res.backgroundImageDark).toBeDefined(); - expect(res.backgroundImageLight).toBeDefined(); - expect(res.isAutoContrast).toBeDefined(); + it('handles an explicit null theme, not just a missing one', () => { + const ctx = buildCtx({ theme: null }); + + expect(resolveContextTheme(ctx)).toBe(DEFAULT_TAG.theme); }); it('yields a COMPLETE theme when the tag-color fallback applies', () => { diff --git a/src/app/features/work-context/work-context.service.ts b/src/app/features/work-context/work-context.service.ts index 647dd404fa..9d1b52142c 100644 --- a/src/app/features/work-context/work-context.service.ts +++ b/src/app/features/work-context/work-context.service.ts @@ -26,9 +26,9 @@ import { take, withLatestFrom, } from 'rxjs/operators'; -import { TODAY_TAG } from '../tag/tag.const'; +import { DEFAULT_TAG, TODAY_TAG } from '../tag/tag.const'; import { Tag } from '../tag/tag.model'; -import { DEFAULT_TAG_COLOR, WORK_CONTEXT_DEFAULT_THEME } from './work-context.const'; +import { DEFAULT_TAG_COLOR } from './work-context.const'; import { TagService } from '../tag/tag.service'; import { ArchiveTask, Task, TaskWithSubTasks } from '../tasks/task.model'; import { @@ -70,7 +70,7 @@ import { getTimeSpentForDay } from './get-time-spent-for-day.util'; import { TimeTrackingService } from '../time-tracking/time-tracking.service'; import { updateWorkContextData } from '../time-tracking/store/time-tracking.actions'; import { TaskArchiveService } from '../archive/task-archive.service'; -import { INBOX_PROJECT } from '../project/project.const'; +import { DEFAULT_PROJECT, INBOX_PROJECT } from '../project/project.const'; import { selectProjectById } from '../project/store/project.selectors'; import { Project } from '../project/project.model'; import { Log } from '../../core/log'; @@ -83,10 +83,19 @@ import { LOCAL_ACTIONS } from '../../util/local-actions.token'; * it: validation at hydration is non-fatal, so a snapshot holding a theme-less * tag loads anyway and every consumer then dereferences `undefined` (#9139 — * this crashed both `resolveBackground` and `_setColorTheme` on every launch). - * Defaulting here gives the theme pipeline a single choke point. + * + * Scope: this covers every consumer of `currentTheme$`, i.e. the *active* + * work context. It is NOT an app-wide guarantee — code that iterates over all + * projects/tags reads the raw entity and must still guard `theme?.` itself. + * + * The default is type-aware so it matches what `auto-fix-typia-errors` would + * later persist for the same entity; otherwise a theme-less project rendered + * tag-purple until a repair ran and then flipped to project-teal. */ export const resolveContextTheme = (awc: WorkContext): WorkContextThemeCfg => { - const theme = awc.theme ?? WORK_CONTEXT_DEFAULT_THEME; + const theme = + awc.theme ?? + (awc.type === WorkContextType.TAG ? DEFAULT_TAG.theme : DEFAULT_PROJECT.theme); // For tags: theme.primary is the explicit override. If it's still at // the auto-default (or unset) and tag.color is set, fall back to // tag.color so newly created tags drive Material theming with their diff --git a/src/app/op-log/core/recreate-fallback.const.ts b/src/app/op-log/core/recreate-fallback.const.ts index 95d7643c66..949384afdb 100644 --- a/src/app/op-log/core/recreate-fallback.const.ts +++ b/src/app/op-log/core/recreate-fallback.const.ts @@ -19,9 +19,11 @@ import { EMPTY_SIMPLE_COUNTER } from '../../features/simple-counter/simple-count * `requiredKeys`. * - `requiredKeys` drives only (a) the meta-reducer's diagnostic warn (which * missing schema-required fields to name in the log) and (b) the per-type - * on-disk heal branch in `auto-fix-typia-errors.ts`. Only TASK and - * SIMPLE_COUNTER have such a branch today; PROJECT/TAG rely on the generic - * recreate backfill alone, so their `requiredKeys` feed only the warn. + * on-disk heal branch in `auto-fix-typia-errors.ts`. TASK and SIMPLE_COUNTER + * have a `requiredKeys`-driven branch. TAG/PROJECT have a narrower one + * covering `theme` only (#9139), which reads its defaults directly rather + * than through this table, so for them `requiredKeys` still feeds only the + * warn — `theme` is listed below so the warn names it. * List the schema-required fields that are NOT already coerced by an * earlier generic branch in `autoFixTypiaErrors` (booleans → false, * nullable → null) — mirroring TASK's curated list. @@ -73,8 +75,8 @@ export const RECREATE_FALLBACK: Partial> = 'projectId', ], }, - PROJECT: { defaults: DEFAULT_PROJECT, requiredKeys: ['title', 'taskIds'] }, - TAG: { defaults: DEFAULT_TAG, requiredKeys: ['title', 'taskIds'] }, + PROJECT: { defaults: DEFAULT_PROJECT, requiredKeys: ['title', 'taskIds', 'theme'] }, + TAG: { defaults: DEFAULT_TAG, requiredKeys: ['title', 'taskIds', 'theme'] }, SIMPLE_COUNTER: { defaults: EMPTY_SIMPLE_COUNTER, // Curated like TASK: omit `icon` (nullable, healed by the undefined→null diff --git a/src/app/op-log/validation/auto-fix-typia-errors.spec.ts b/src/app/op-log/validation/auto-fix-typia-errors.spec.ts index bbf3db1caf..aa4636e0c1 100644 --- a/src/app/op-log/validation/auto-fix-typia-errors.spec.ts +++ b/src/app/op-log/validation/auto-fix-typia-errors.spec.ts @@ -4,8 +4,8 @@ import type { IValidation } from 'typia'; import { initialTaskState } from '../../features/tasks/store/task.reducer'; import { DEFAULT_TASK } from '../../features/tasks/task.model'; import { OP_LOG_SYNC_LOGGER } from '../core/sync-logger.adapter'; -import { DEFAULT_TAG } from '../../features/tag/tag.const'; -import { DEFAULT_PROJECT } from '../../features/project/project.const'; +import { DEFAULT_TAG, TODAY_TAG } from '../../features/tag/tag.const'; +import { DEFAULT_PROJECT, INBOX_PROJECT } from '../../features/project/project.const'; const createTypiaError = ( path: string, @@ -493,16 +493,26 @@ describe('autoFixTypiaErrors', () => { it('should backfill a missing tag.theme with the default tag theme', () => { const mockData = createAppDataCompleteMock(); (mockData as any).tag = { - ids: ['TODAY'], - entities: { TODAY: { id: 'TODAY', title: 'Today', taskIds: [] } }, + ids: ['t1', 't2'], + entities: { + t1: { id: 't1', title: 'User tag', taskIds: [] }, + t2: { id: 't2', title: 'Other', taskIds: [] }, + }, }; const errors = [ - createTypiaError('$input.tag.entities.TODAY.theme', REAL_EXPECTED, undefined), + createTypiaError('$input.tag.entities.t1.theme', REAL_EXPECTED, undefined), + createTypiaError('$input.tag.entities.t2.theme', REAL_EXPECTED, undefined), ]; const result = autoFixTypiaErrors(mockData, errors); - expect((result as any).tag.entities.TODAY.theme).toEqual(DEFAULT_TAG.theme); + const a = (result as any).tag.entities.t1.theme; + const b = (result as any).tag.entities.t2.theme; + expect(a).toEqual(DEFAULT_TAG.theme); + // Backfill a COPY: aliasing the constant across entities would let any + // later mutation write through into DEFAULT_TAG for the whole app. + expect(a).not.toBe(DEFAULT_TAG.theme); + expect(a).not.toBe(b); expect(errSpy).toHaveBeenCalledWith( '[auto-fix-typia-errors] Applied validation auto-fix', undefined, @@ -535,12 +545,12 @@ describe('autoFixTypiaErrors', () => { // not change the outcome. const mockData = createAppDataCompleteMock(); (mockData as any).tag = { - ids: ['TODAY'], - entities: { TODAY: { id: 'TODAY', title: 'Today', taskIds: [] } }, + ids: ['t1'], + entities: { t1: { id: 't1', title: 'User tag', taskIds: [] } }, }; const errors = [ createTypiaError( - '$input.tag.entities.TODAY.theme', + '$input.tag.entities.t1.theme', 'Readonly<__type>.o99999', undefined, ), @@ -548,54 +558,94 @@ describe('autoFixTypiaErrors', () => { const result = autoFixTypiaErrors(mockData, errors); - expect((result as any).tag.entities.TODAY.theme).toEqual(DEFAULT_TAG.theme); + expect((result as any).tag.entities.t1.theme).toEqual(DEFAULT_TAG.theme); }); - it('should backfill a COPY, never the shared default constant', () => { - // Aliasing DEFAULT_TAG.theme across entities would let any later - // mutation write through into the app-wide constant. + it('should repair an explicit null theme, not just a missing one', () => { + // The `setOne` 'replace' branch applies a remote entity verbatim, so + // `theme: null` is reachable and typia reports it at the same path with + // `value: null`. Gating on `undefined` alone left this dead-ending the + // repair pipeline ("state still invalid after repair"). const mockData = createAppDataCompleteMock(); (mockData as any).tag = { - ids: ['TODAY', 't2'], - entities: { - TODAY: { id: 'TODAY', title: 'Today', taskIds: [] }, - t2: { id: 't2', title: 'Other', taskIds: [] }, - }, + ids: ['t1'], + entities: { t1: { id: 't1', title: 'Other', taskIds: [], theme: null } }, + }; + const errors = [ + createTypiaError('$input.tag.entities.t1.theme', REAL_EXPECTED, null), + ]; + + const result = autoFixTypiaErrors(mockData, errors); + + expect((result as any).tag.entities.t1.theme).toEqual(DEFAULT_TAG.theme); + }); + + it('should restore the TODAY tag its own theme, not the generic tag default', () => { + // TODAY is the entity from the #9139 report and ships a distinct theme. + // The repair is written to disk, so a generic default would permanently + // restyle it (cornflower + tint-disabled -> purple + tint-enabled). + const mockData = createAppDataCompleteMock(); + (mockData as any).tag = { + ids: ['TODAY'], + entities: { TODAY: { id: 'TODAY', title: 'Today', taskIds: [] } }, }; const errors = [ createTypiaError('$input.tag.entities.TODAY.theme', REAL_EXPECTED, undefined), - createTypiaError('$input.tag.entities.t2.theme', REAL_EXPECTED, undefined), ]; const result = autoFixTypiaErrors(mockData, errors); - const a = (result as any).tag.entities.TODAY.theme; - const b = (result as any).tag.entities.t2.theme; - expect(a).not.toBe(DEFAULT_TAG.theme); - expect(b).not.toBe(DEFAULT_TAG.theme); - expect(a).not.toBe(b); - expect(a).toEqual(DEFAULT_TAG.theme); + const healed = (result as any).tag.entities.TODAY.theme; + expect(healed).toEqual(TODAY_TAG.theme); + expect(healed.primary).toBe(TODAY_TAG.theme.primary); + expect(healed.isDisableBackgroundTint).toBe(true); + expect(healed.huePrimary).toBe('400'); + // Guards the regression this test was written for. + expect(healed.primary).not.toBe(DEFAULT_TAG.theme.primary); }); - it('should NOT overwrite a theme that is present but merely invalid', () => { - // `value === undefined` scopes this to a genuinely absent field; a - // present-but-wrong theme is a different problem and must not be - // silently replaced (that would discard user styling). + it('should restore the INBOX project its own theme', () => { const mockData = createAppDataCompleteMock(); - const existingTheme = { primary: '#user-picked' }; - (mockData as any).tag = { - ids: ['TODAY'], + (mockData as any).project = { + ids: [INBOX_PROJECT.id], entities: { - TODAY: { id: 'TODAY', title: 'Today', taskIds: [], theme: existingTheme }, + [INBOX_PROJECT.id]: { id: INBOX_PROJECT.id, title: 'Inbox', taskIds: [] }, }, }; const errors = [ - createTypiaError('$input.tag.entities.TODAY.theme', REAL_EXPECTED, existingTheme), + createTypiaError( + `$input.project.entities.${INBOX_PROJECT.id}.theme`, + REAL_EXPECTED, + undefined, + ), ]; const result = autoFixTypiaErrors(mockData, errors); - expect((result as any).tag.entities.TODAY.theme).toEqual(existingTheme); + const healed = (result as any).project.entities[INBOX_PROJECT.id].theme; + expect(healed).toEqual(INBOX_PROJECT.theme); + expect(healed.primary).not.toBe(DEFAULT_PROJECT.theme.primary); + }); + + it('should not treat a hostile "__proto__" entity id as a system theme', () => { + // The system-theme lookup is a Map precisely so this returns undefined + // rather than Object.prototype. + const mockData = createAppDataCompleteMock(); + (mockData as any).tag = { + ids: ['t1'], + entities: { t1: { id: 't1', title: 'x', taskIds: [] } }, + }; + const errors = [ + createTypiaError('$input.tag.entities.t1.theme', REAL_EXPECTED, undefined), + ]; + + const result = autoFixTypiaErrors(mockData, errors); + + expect((result as any).tag.entities.t1.theme).toEqual(DEFAULT_TAG.theme); + expect( + (result as any).tag.entities.t1.theme instanceof Object && + Object.getPrototypeOf((result as any).tag.entities.t1.theme), + ).toBe(Object.prototype); }); }); }); diff --git a/src/app/op-log/validation/auto-fix-typia-errors.ts b/src/app/op-log/validation/auto-fix-typia-errors.ts index 6f17386b54..1c3e290d12 100644 --- a/src/app/op-log/validation/auto-fix-typia-errors.ts +++ b/src/app/op-log/validation/auto-fix-typia-errors.ts @@ -3,13 +3,45 @@ import { IValidation } from 'typia'; import type { SyncLogMeta } from '@sp/sync-core'; import { DEFAULT_GLOBAL_CONFIG } from '../../features/config/default-global-config.const'; import { DEFAULT_PROJECT, INBOX_PROJECT } from '../../features/project/project.const'; -import { DEFAULT_TAG } from '../../features/tag/tag.const'; +import { + DEFAULT_TAG, + IMPORTANT_TAG, + IN_PROGRESS_TAG, + TODAY_TAG, + URGENT_TAG, +} from '../../features/tag/tag.const'; +import { WorkContextThemeCfg } from '../../features/work-context/work-context.model'; import { RECREATE_FALLBACK } from '../core/recreate-fallback.const'; import { OP_LOG_SYNC_LOGGER } from '../core/sync-logger.adapter'; import { devError } from '../../util/dev-error'; const LOG_PREFIX = '[auto-fix-typia-errors]'; +/** + * System tags/projects ship a *distinct* theme, so healing them from the + * generic default would silently restyle them — and because the repair is + * written to disk, permanently. TODAY in particular is the entity from the + * #9139 report: it is cornflower with `huePrimary: '400'` and background tint + * disabled, where DEFAULT_TAG is purple with tint on. + * + * A Map, not an object literal: a hostile entity id of `__proto__` would hit + * `Object.prototype` on a plain-object lookup and be treated as a theme. + */ +const SYSTEM_ENTITY_THEMES: ReadonlyMap = new Map([ + [TODAY_TAG.id, TODAY_TAG.theme], + [URGENT_TAG.id, URGENT_TAG.theme], + [IMPORTANT_TAG.id, IMPORTANT_TAG.theme], + [IN_PROGRESS_TAG.id, IN_PROGRESS_TAG.theme], + [INBOX_PROJECT.id, INBOX_PROJECT.theme], +]); + +const getWorkContextDefaultTheme = ( + pathRoot: string | number, + entityId: string | number, +): WorkContextThemeCfg => + SYSTEM_ENTITY_THEMES.get(String(entityId)) ?? + (pathRoot === 'tag' ? DEFAULT_TAG.theme : DEFAULT_PROJECT.theme); + const getValueType = (value: unknown): string => { if (value === null) return 'null'; if (Array.isArray(value)) return 'array'; @@ -277,23 +309,23 @@ export const autoFixTypiaErrors = ( keys[1] === 'entities' && keys.length === 4 && keys[3] === 'theme' && - value === undefined + value == null ) { // A tag/project entity can be persisted with no `theme` at all (#9139). // Left unrepaired it either dead-ends legacy migration ("Migration // failed") or, on the hydration paths where validation is non-fatal, // loads and crashes the theme pipeline on every launch. // + // `== null` covers both undefined and an explicit null: the `setOne` + // 'replace' branch (lww-update.meta-reducer.ts) applies a remote entity + // verbatim, so a null theme is reachable, and typia reports it at this + // same path. Gating on `undefined` alone left it dead-ending migration. + // // Deliberately NOT matched on `error.expected`: typia reports this as // the generated name `Readonly<__type>.oNN`, whose ordinal shifts // whenever the type graph changes. Keying on it would make this branch - // silently stop firing. Path + `value === undefined` is stable. - // Copy, never share the constant by reference: two theme-less entities - // would otherwise alias one object, and any later mutation would write - // straight through into DEFAULT_TAG/DEFAULT_PROJECT for the whole app. - const theme = { - ...(keys[0] === 'tag' ? DEFAULT_TAG.theme : DEFAULT_PROJECT.theme), - }; + // silently stop firing. Path + nullish value is stable. + const theme = { ...getWorkContextDefaultTheme(keys[0], keys[2]) }; setValueByPath(data, keys, theme); logAutoFixApplied( path,