fix(desktop): crash in sidebar FileRow when sidebarFileLinks pref is missing#4045
Merged
Conversation
…ed fields Existing localStorage rows predate sidebarFileLinks, so reading them back returned undefined for that key and crashed buildHint with "Cannot read properties of undefined (reading 'shift')".
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRead-heal wrappers were added for two local collections: ChangesLocal Read-Healing for Workspace & Preferences
Sequence DiagramsequenceDiagram
participant LS as Local Storage
participant Parser as Parser (withReadHeal)
participant Heal as Healer
participant Consumer as Collection Consumer
Consumer->>LS: request stored collection JSON
LS-->>Parser: raw JSON string
Parser->>Parser: baseParser.parse(raw)
Parser->>Parser: detect object entries and envelopes
Parser->>Heal: heal(entry.data)
Heal->>Heal: merge with defaults / synthesize missing fields
Heal-->>Parser: healed data
Parser-->>Consumer: envelope with healed data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryThis PR fixes a crash in the sidebar Confidence Score: 4/5Safe to merge; fix is correct and well-scoped for the reported crash. Single targeted change with a clear fix for a real crash. One P2 observation about shallow merge not covering nested schema evolution, but that is a future concern and does not affect the current bug fix. No files require special attention beyond the single changed file.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts | Adds shallow default-merge for old persisted rows missing top-level fields (e.g. sidebarFileLinks); fixes the crash. Shallow merge is sufficient for the current schema but won't guard new keys added inside nested LinkTierMap objects. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[useLiveQuery rows] --> B{rows zero exists?}
B -- No --> C[Use DEFAULT_V2_USER_PREFERENCES]
B -- Yes --> D[Shallow merge defaults then rows zero]
D --> E{Top-level field missing?}
E -- Missing in rows zero --> F[Default value retained]
E -- Present in rows zero --> G[Persisted value used]
C --> H[preferences returned to consumers]
F --> H
G --> H
H --> I[usePolicy and buildHint and FileRow render]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts:38-40
**Shallow merge doesn't guard nested schema additions**
The comment says this "guards future schema additions," but that only holds for top-level fields. If a new key is later added inside `linkTierMapSchema` (e.g. a new modifier tier), old rows will have `sidebarFileLinks: { plain, shift, meta, metaShift }` already present, so `...rows[0]` keeps that entire stale object and the new key will be `undefined` inside it — the same crash pattern would recur one level deeper.
Consider a recursive merge for `LinkTierMap` objects, or apply the Zod schema with its `.default()` values at deserialization time so both levels are covered automatically.
Reviews (1): Last reviewed commit: "fix(desktop): merge persisted v2 prefs o..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+38
to
+40
| const preferences: V2UserPreferencesRow = rows[0] | ||
| ? { ...DEFAULT_V2_USER_PREFERENCES, ...rows[0] } | ||
| : DEFAULT_V2_USER_PREFERENCES; |
Contributor
There was a problem hiding this comment.
The comment says this "guards future schema additions," but that only holds for top-level fields. If a new key is later added inside linkTierMapSchema (e.g. a new modifier tier), old rows will have sidebarFileLinks: { plain, shift, meta, metaShift } already present, so ...rows[0] keeps that entire stale object and the new key will be undefined inside it — the same crash pattern would recur one level deeper.
Consider a recursive merge for LinkTierMap objects, or apply the Zod schema with its .default() values at deserialization time so both levels are covered automatically.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts
Line: 38-40
Comment:
**Shallow merge doesn't guard nested schema additions**
The comment says this "guards future schema additions," but that only holds for top-level fields. If a new key is later added inside `linkTierMapSchema` (e.g. a new modifier tier), old rows will have `sidebarFileLinks: { plain, shift, meta, metaShift }` already present, so `...rows[0]` keeps that entire stale object and the new key will be `undefined` inside it — the same crash pattern would recur one level deeper.
Consider a recursive merge for `LinkTierMap` objects, or apply the Zod schema with its `.default()` values at deserialization time so both levels are covered automatically.
How can I resolve this? If you propose a fix, please make it concise.Top-level shallow merge only healed missing fileLinks / urlLinks / sidebarFileLinks entirely. A tier added inside an existing map (e.g. a new modifier later) would still come back undefined and crash buildHint the same way. Merge each tier map over its default too.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts (1)
42-59: ⚡ Quick win
preferencesis not memoized — new object reference on every renderThe merged
preferencesobject is reconstructed on every call touseV2UserPreferences, not just whenrowschanges. Any consumer that passespreferences(or a sub-property) into auseCallback/useEffect/useMemodependency array will re-subscribe on every render, even when the data hasn't changed. The old code (rows[0] ?? DEFAULT_V2_USER_PREFERENCES) returned a stable reference from TanStack DB or the static constant; the new code always produces a fresh object.♻️ Proposed fix: wrap in `useMemo`
- const stored = rows[0]; - const preferences: V2UserPreferencesRow = stored - ? { - ...DEFAULT_V2_USER_PREFERENCES, - ...stored, - fileLinks: { - ...DEFAULT_V2_USER_PREFERENCES.fileLinks, - ...stored.fileLinks, - }, - urlLinks: { - ...DEFAULT_V2_USER_PREFERENCES.urlLinks, - ...stored.urlLinks, - }, - sidebarFileLinks: { - ...DEFAULT_V2_USER_PREFERENCES.sidebarFileLinks, - ...stored.sidebarFileLinks, - }, - } - : DEFAULT_V2_USER_PREFERENCES; + const preferences: V2UserPreferencesRow = useMemo(() => { + const stored = rows[0]; + if (!stored) return DEFAULT_V2_USER_PREFERENCES; + return { + ...DEFAULT_V2_USER_PREFERENCES, + ...stored, + fileLinks: { + ...DEFAULT_V2_USER_PREFERENCES.fileLinks, + ...stored.fileLinks, + }, + urlLinks: { + ...DEFAULT_V2_USER_PREFERENCES.urlLinks, + ...stored.urlLinks, + }, + sidebarFileLinks: { + ...DEFAULT_V2_USER_PREFERENCES.sidebarFileLinks, + ...stored.sidebarFileLinks, + }, + }; + }, [rows]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts` around lines 42 - 59, The merged preferences object in useV2UserPreferences (variable preferences) is recreated every render; wrap the merge in a useMemo so the object identity only changes when the source data changes (e.g. stored or rows). Import React's useMemo if not present and replace the direct assignment to preferences with: const preferences = useMemo(() => stored ? { ...DEFAULT_V2_USER_PREFERENCES, ...stored, fileLinks: { ...DEFAULT_V2_USER_PREFERENCES.fileLinks, ...stored.fileLinks }, urlLinks: { ...DEFAULT_V2_USER_PREFERENCES.urlLinks, ...stored.urlLinks }, sidebarFileLinks: { ...DEFAULT_V2_USER_PREFERENCES.sidebarFileLinks, ...stored.sidebarFileLinks } } : DEFAULT_V2_USER_PREFERENCES, [stored]); ensuring the dependency correctly tracks the source (stored or rows[0]) so consumers relying on preferences identity won't re-run unnecessarily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts`:
- Around line 42-59: The merged preferences object in useV2UserPreferences
(variable preferences) is recreated every render; wrap the merge in a useMemo so
the object identity only changes when the source data changes (e.g. stored or
rows). Import React's useMemo if not present and replace the direct assignment
to preferences with: const preferences = useMemo(() => stored ? {
...DEFAULT_V2_USER_PREFERENCES, ...stored, fileLinks: {
...DEFAULT_V2_USER_PREFERENCES.fileLinks, ...stored.fileLinks }, urlLinks: {
...DEFAULT_V2_USER_PREFERENCES.urlLinks, ...stored.urlLinks }, sidebarFileLinks:
{ ...DEFAULT_V2_USER_PREFERENCES.sidebarFileLinks, ...stored.sidebarFileLinks }
} : DEFAULT_V2_USER_PREFERENCES, [stored]); ensuring the dependency correctly
tracks the source (stored or rows[0]) so consumers relying on preferences
identity won't re-run unnecessarily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a29168c-9ee9-46d1-92cd-1ddd7c9472da
📒 Files selected for processing (1)
apps/desktop/src/renderer/hooks/useV2UserPreferences/useV2UserPreferences.ts
…el merge
The previous fix patched useV2UserPreferences directly. The underlying
issue is that @tanstack/db's localStorageCollectionOptions only validates
the schema on writes — reads return whatever was last persisted, so any
collection here is one schema addition away from the same crash class.
Move the fix to the collection layer:
- withReadHeal wraps localStorageCollectionOptions and installs a
custom parser whose .parse runs each entry's data through a heal fn,
preserving the {versionKey, data} envelope the library expects.
- healV2UserPreferences lives next to DEFAULT_V2_USER_PREFERENCES in
schema.ts and deep-merges stored rows over per-tier-map defaults
(Zod alone can't cover this since per-tier defaults vary by map).
- useV2UserPreferences goes back to a plain rows[0] ?? defaults read.
The wrapper is reusable for the other localStorage-backed collections
when they hit the same evolution problem.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.ts (1)
20-54: 💤 Low valueLGTM — approach is sound and the key library assumption is confirmed.
The
parseroption is a real, exported config field in@tanstack/react-db'slocalStorageCollectionOptions(confirmed in the library source atpackages/db/src/local-storage.ts). The heal is cleanly read-only: writes go throughbaseParser.stringifyunchanged, so the stored format is never altered until a normal mutation rewrites the healed shape.One optional nit: the
Parserinterface is publicly documented by TanStack DB and exported from the library. You could import it instead of re-declaring locally to avoid potential drift, though the current definition is structurally identical.♻️ Optional: import Parser from the library
-interface Parser { - parse: (raw: string) => unknown; - stringify: (value: unknown) => string; -} +import type { Parser } from "@tanstack/react-db";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.ts` around lines 20 - 54, Replace the locally redeclared Parser type with the library's exported Parser: import the Parser interface from `@tanstack/react-db` and use it in withReadHeal (affecting the baseParser and healingParser declarations) so the function references the canonical Parser type (remove the local Parser declaration to avoid drift).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.ts`:
- Around line 20-54: Replace the locally redeclared Parser type with the
library's exported Parser: import the Parser interface from `@tanstack/react-db`
and use it in withReadHeal (affecting the baseParser and healingParser
declarations) so the function references the canonical Parser type (remove the
local Parser declaration to avoid drift).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dec3bd22-caf4-48ab-ba78-d9f0d1c1d75a
📒 Files selected for processing (3)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.ts
…Options
- healV2UserPreferences: empty/partial inputs, the original missing-
sidebarFileLinks crash shape, and a missing-tier-inside-present-map case.
- withReadHeal parser: heals each entry's data while preserving the
{versionKey, data} envelope; passes non-envelope values through.
- End-to-end through real createCollection + localStorageCollectionOptions
with a Map-backed storage. Pre-populates storage with the buggy shape,
asserts the collection exposes the healed row. Includes a regression
guard that pins library behavior without the wrapper (sidebarFileLinks
comes back undefined) so we know when the workaround is no longer needed.
Each test was verified to fail under a targeted impl mutation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts`:
- Around line 38-49: withReadHeal({}) infers T = {} so opts is typed {} and
accessing opts.parser causes a TS2339; fix by casting the returned opts to the
correct extended interface that includes parser (i.e. the same cast used in the
first test) or by supplying an explicit generic to withReadHeal to preserve the
expected return type; apply that same cast/fix to the opts variable in the
second unit test so opts.parser is correctly typed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a7d8afa-ef02-436b-8b86-1a9b286a1545
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts
Highest-risk localStorage collection: deeply nested shape (sidebarState
with discriminated unions, paneLayout, arrays of viewed files) and the
most consumers chaining into nested fields. One added field is one
undefined-property-chain away from the buildHint crash class.
healWorkspaceLocalState:
- Preserves identity fields (workspaceId, projectId, paneLayout,
createdAt) from the stored row — heal can't synthesize them.
- Fills optional top-level defaults (viewedFiles, recentlyViewedFiles)
via ?? so explicit null/undefined also fall back.
- Deep-merges sidebarState against per-field defaults pulled next to
the schema (SIDEBAR_STATE_DEFAULTS).
- Never throws on null/non-object input — a parser throw silently
zeros the entire collection.
Tests: 4 unit tests + an end-to-end through the real library with a
pre-evolution shape. Verified the nested-fill test fails when
SIDEBAR_STATE_DEFAULTS is dropped from the merge.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts (1)
37-62:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
opts.parsercauses TS2339 in both unit tests —typecheckCI will fail.
withReadHeal({}, heal)infersT = {}, sooptsis typed as{}. TypeScript's optional chaining?.does not suppress the TS2339 that fires at the property access step — TypeScript errors before the chain is evaluated. The same issue exists at line 58.The fix is to cast the return value to a type that includes
parser, or to use an explicit type argument:🛠️ Proposed fix (lines 40 and 58)
- const opts = withReadHeal({}, heal); + const opts = withReadHeal({}, heal) as { + parser?: { parse: (s: string) => unknown }; + };Apply the same change at line 58.
Alternatively, if
withReadHeal's return type were widened toT & { parser: { parse: ... } }instead of justT, both the tests and the call sites incollections.tswould benefit from more accurate types without any casts here.Run the following to confirm the current
withReadHeal.tssignature hasn't changed:#!/bin/bash # Verify the return type of withReadHeal to confirm the TS2339 concern. fd -t f "withReadHeal.ts" apps/desktop/src --exclude "*.test.ts" | xargs cat -n🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts` around lines 37 - 62, The tests fail TS2339 because withReadHeal({}, heal) infers a bare {} so accessing opts.parser triggers a type error; fix by giving withReadHeal an explicit return type or casting its result to include parser (e.g. cast the call to a type with parser: { parse: (s: string) => unknown }) or call withReadHeal with an explicit generic type that includes parser before using opts.parser in both test cases (the calls that produce opts and the subsequent opts.parser?.parse usages around the two parser checks).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts`:
- Around line 37-62: The tests fail TS2339 because withReadHeal({}, heal) infers
a bare {} so accessing opts.parser triggers a type error; fix by giving
withReadHeal an explicit return type or casting its result to include parser
(e.g. cast the call to a type with parser: { parse: (s: string) => unknown }) or
call withReadHeal with an explicit generic type that includes parser before
using opts.parser in both test cases (the calls that produce opts and the
subsequent opts.parser?.parse usages around the two parser checks).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6675f0a-9882-4b58-adbf-042db52ab072
📒 Files selected for processing (4)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.test.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/withReadHeal.test.ts
Address PR review feedback:
- Use the library-exported `Parser` type in `withReadHeal` instead of
redeclaring locally (avoids future drift).
- Annotate `getKey` parameter types at the four `withReadHeal` call sites
(collections.ts and the e2e tests) so the wrapper's passthrough generic
preserves the schema/row-type linkage that downstream library overloads
need to typecheck.
- Loosen the parser unit-test casts to a permissive `data` shape so
bun:test's strict toEqual accepts the {a, healed} literal.
- Cast `paneLayout` reference compare via `Object.is` to satisfy bun:test's
strict toBe types (the test fixture is a narrow stub of `WorkspaceState`).
CodeRabbit caught these — earlier filtered tsc grep had missed them.
Tests still pass; mutations still catch each regression.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
May 21, 2026
…missing (superset-sh#4045) * fix(desktop): merge persisted v2 prefs over defaults to avoid undefined fields Existing localStorage rows predate sidebarFileLinks, so reading them back returned undefined for that key and crashed buildHint with "Cannot read properties of undefined (reading 'shift')". * fix(desktop): also merge nested LinkTierMap defaults Top-level shallow merge only healed missing fileLinks / urlLinks / sidebarFileLinks entirely. A tier added inside an existing map (e.g. a new modifier later) would still come back undefined and crash buildHint the same way. Merge each tier map over its default too. * refactor(desktop): heal localStorage rows at read time, drop hook-level merge The previous fix patched useV2UserPreferences directly. The underlying issue is that @tanstack/db's localStorageCollectionOptions only validates the schema on writes — reads return whatever was last persisted, so any collection here is one schema addition away from the same crash class. Move the fix to the collection layer: - withReadHeal wraps localStorageCollectionOptions and installs a custom parser whose .parse runs each entry's data through a heal fn, preserving the {versionKey, data} envelope the library expects. - healV2UserPreferences lives next to DEFAULT_V2_USER_PREFERENCES in schema.ts and deep-merges stored rows over per-tier-map defaults (Zod alone can't cover this since per-tier defaults vary by map). - useV2UserPreferences goes back to a plain rows[0] ?? defaults read. The wrapper is reusable for the other localStorage-backed collections when they hit the same evolution problem. * test(desktop): cover heal-on-read against real localStorageCollectionOptions - healV2UserPreferences: empty/partial inputs, the original missing- sidebarFileLinks crash shape, and a missing-tier-inside-present-map case. - withReadHeal parser: heals each entry's data while preserving the {versionKey, data} envelope; passes non-envelope values through. - End-to-end through real createCollection + localStorageCollectionOptions with a Map-backed storage. Pre-populates storage with the buggy shape, asserts the collection exposes the healed row. Includes a regression guard that pins library behavior without the wrapper (sidebarFileLinks comes back undefined) so we know when the workaround is no longer needed. Each test was verified to fail under a targeted impl mutation. * fix(desktop): apply withReadHeal to v2WorkspaceLocalState preemptively Highest-risk localStorage collection: deeply nested shape (sidebarState with discriminated unions, paneLayout, arrays of viewed files) and the most consumers chaining into nested fields. One added field is one undefined-property-chain away from the buildHint crash class. healWorkspaceLocalState: - Preserves identity fields (workspaceId, projectId, paneLayout, createdAt) from the stored row — heal can't synthesize them. - Fills optional top-level defaults (viewedFiles, recentlyViewedFiles) via ?? so explicit null/undefined also fall back. - Deep-merges sidebarState against per-field defaults pulled next to the schema (SIDEBAR_STATE_DEFAULTS). - Never throws on null/non-object input — a parser throw silently zeros the entire collection. Tests: 4 unit tests + an end-to-end through the real library with a pre-evolution shape. Verified the nested-fill test fails when SIDEBAR_STATE_DEFAULTS is dropped from the merge. * fix(desktop): typecheck cleanup for heal wrapper Address PR review feedback: - Use the library-exported `Parser` type in `withReadHeal` instead of redeclaring locally (avoids future drift). - Annotate `getKey` parameter types at the four `withReadHeal` call sites (collections.ts and the e2e tests) so the wrapper's passthrough generic preserves the schema/row-type linkage that downstream library overloads need to typecheck. - Loosen the parser unit-test casts to a permissive `data` shape so bun:test's strict toEqual accepts the {a, healed} literal. - Cast `paneLayout` reference compare via `Object.is` to satisfy bun:test's strict toBe types (the test fixture is a narrow stub of `WorkspaceState`). CodeRabbit caught these — earlier filtered tsc grep had missed them. Tests still pass; mutations still catch each regression.
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
May 21, 2026
superset-sh#4037 NewProjectModal: drop fork-only OPTIONS multi-mode selector and adopt upstream v2-only simplification (toast errors + Label + spinner). The 'empty'/'template' modes were 'coming soon' disabled entries with no implementation behind them; no user-facing fork feature lost. superset-sh#4191 DashboardSidebarDeleteDialog: keep fork's 'Checking…' confirm-button label UX while incorporating upstream's fixed-height warning banner slot. Restore isCheckingStatus destructure + prop pass; add to DestroyConfirmPane interface so the deleting-button disabled state still gates on it. superset-sh#4045 schema.ts: backfill sidebarFileLinks field into v2UserPreferencesSchema and DEFAULT_V2_USER_PREFERENCES (upstream introduced these in superset-sh#3988 which fork will adopt in Phase 5). Also add 'shift' tier to linkTierMapSchema (matches superset-sh#3988 LinkTier expansion) so superset-sh#4045's added schema tests pass. superset-sh#4212 tray Quit Completely: replace upstream quitAppCompletely() with fork's existing requestQuit('stop') which already performs full host- service + pty-daemon teardown via prepareQuit('stop') + getTodoScheduler stop + cleanupMainWindowResources. Tray menu entry retained. superset-sh#4225 automations/page.tsx: drop iconUrl prop from ProjectThumbnail call (fork ProjectThumbnail uses githubOwner, iconUrl arrives with superset-sh#3823 v2 project icon settings in cycle 41). Pass githubOwner={null} as transient fallback. superset-sh#4226 useBranchPickerController: fork's useWorkspaceCreates.submit returns Promise<void> (in-flight tracker carries error state); upstream changed it to Promise<SubmitResult>. Wrap submit in try/catch and navigate using snapshot.id so the picker compiles. Note: foreign-worktree adopt path with canonical workspaceId waiting is deferred to cycle 44b when WorkspaceCreatesManager / submit API are upgraded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sidebarFileLinks, so reading them back returnedundefinedfor that keyuseSidebarFilePolicy->usePolicy->buildHintthen crashed withCannot read properties of undefined (reading 'shift')when rendering any sidebarFileRowDEFAULT_V2_USER_PREFERENCESover the persisted row so missing top-level fields fall back to defaults (also guards future schema additions)Test plan
v2-user-preferences-*entry that lackssidebarFileLinks, open a v2 workspace and confirm the sidebar no longer crashesSummary by cubic
Fixes a desktop sidebar crash in FileRow when older v2 preferences lack
sidebarFileLinks. Adds read-time healing for localStorage rows so preferences and workspace local state expose safe defaults and avoid undefined chains.withReadHealto wraplocalStorageCollectionOptionsfrom@tanstack/react-db, healing each entry’sdataduring parse while preserving the{versionKey, data}envelope.v2_user_preferencesandv2_workspace_local_stateviahealV2UserPreferences(deep‑merge tier maps) andhealWorkspaceLocalState(fill optional arrays, mergesidebarStatedefaults, preserve identity fields).Parsertype; annotategetKeyparam types at call sites; added unit tests for both heal functions and the wrapper, plus an end‑to‑end test with reallocalStorageCollectionOptionsand a regression guard.Written for commit d3b4a57. Summary will update on new commits.
Summary by CodeRabbit