Skip to content

fix(sidebar): guard kanban worktree sort against undefined displayName (crash 99657ab1) - #8683

Merged
OrcaWin merged 1 commit into
mainfrom
OrcaWin/fix-kanban-worktree-undefined-displayname
Jul 14, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
OrcaWin/fix-kanban-worktree-undefined-displayname

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

@

Description

Fixes Windows crash report 99657ab1 (react-error-boundary, boundary_id: sidebar.worktrees), version 1.4.137.

A worktree reached the sidebar with an undefined displayName. The kanban lane sort comparator then executed a.displayName.localeCompare(b.displayName) and threw:

TypeError: Cannot read properties of undefined (reading localeCompare)
  at sortManualBoardWorktrees
  at Array.sort (<anonymous>)
  at groupWorkspaceKanbanWorktrees
  at WorkspaceKanbanDrawer (useMemo)

This unmounted the entire sidebar.worktrees error boundary — the sidebar disappeared. Although the report is Windows-only, the bug is cross-platform JS: any worktree with a missing displayName triggers it on any OS.

Both kanban comparators (sortManualBoardWorktrees for Manual sort, sortBoardWorktrees for Recent/other sort) share the same unguarded call, so both paths are affected.

Fix

Route both comparators through a single null-safe helper:

function compareDisplayName(a: Worktree, b: Worktree): number {
  return (a.displayName ?? ).localeCompare(b.displayName ?? )
}

A missing name now only affects tie-break ordering instead of crashing the sidebar. When displayName is present (the overwhelmingly common case), (a.displayName ?? ) is identically a.displayName, so sort order is unchanged.

Fix evidence (clear & undeniable)

Regression tests reproduce the exact crash, then verify the fix. The tests force the comparator to actually be reached (equal primary-sort term → falls through to the name tie-break) and give both worktrees an undefined name so the crash is deterministic regardless of V8 sort argument order.

Before the fix (repro confirmed — identical error string to the crash report):

× does not crash when a worktree is missing its displayName under Manual sort
× does not crash when a worktree is missing its displayName outside Manual sort
  TypeError: Cannot read properties of undefined (reading 'localeCompare')

After the fix:

✓ workspace-kanban-worktree-groups.test.ts (4 tests) — 4 passed

Also verified: oxlint clean on both files, tsc --noEmit -p config/tsconfig.tc.web.json exit 0.

ELI5

Orca sorts the workspace cards in the sidebar by name. Sorting compares two names letter-by-letter. One workspace somehow had no name at all (blank/undefined), and asking a blank name to compare itself made the code explode — which knocked out the whole sidebar. Now we treat a missing name as an empty string, so sorting just puts it wherever and keeps calm instead of crashing.

Trade-offs / behavior changes

  • None negative. For worktrees with a normal name, sort output is byte-for-byte identical.
  • A worktree with a genuinely missing name now sorts as if its name were "" (it sorts first alphabetically within its lane) instead of taking down the sidebar. This is strictly better than the crash.
  • This is a leaf/crash-boundary guard. The deeper question of why a worktree reaches the sidebar with an undefined displayName (the Worktree type declares it non-optional) is a separate, larger investigation and is intentionally out of scope for this crash fix.

Adversarial review

Two independent adversarial review loops were run to clean:

  • Loop 1 (correctness lens): Verified the fix closes the only .localeCompare path; confirmed undefined numeric fields (lastActivityAt, sortOrder) degrade to NaN→falsy tie-break rather than throwing; confirmed the regression tests genuinely reach the comparator and fail-before/pass-after. Result: CLEAN.
  • Loop 2 (sibling-crash-path lens): Swept all sidebar render-path displayName method calls to check the boundary can't still crash via a different call. Found the list-view sort (smart-sort.ts) and card title (worktree-card-title-display.ts) already guard undefined names; confirmed no unguarded sibling call exists in the boundary's render path. Result: CLEAN.

Both loops flagged two non-render-path unguarded .displayName.localeCompare calls (use-complete-git-repo-add.ts, add-repo-existing-workspaces-telemetry.ts) as pre-existing, separate follow-ups — they run in async/telemetry paths and cannot reproduce this render crash, so they are left out to keep this PR tightly scoped.
@

@
fix(sidebar): guard kanban worktree sort against undefined displayName (crash 99657ab1)

A worktree reached the sidebar with an undefined displayName, so the
Manual-sort comparator ran `a.displayName.localeCompare(...)` and threw
`TypeError: Cannot read properties of undefined (reading localeCompare)`,
unmounting the sidebar.worktrees error boundary.

Route both kanban comparators through a null-safe compareDisplayName so a
missing name only affects tie-break order instead of crashing. Behavior is
unchanged when displayName is present.

Adds regression tests for both the manual and recent sort paths.
@
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6805a1f2-8950-4042-85e2-87b46579e796

📥 Commits

Reviewing files that changed from the base of the PR and between 9a324bf and 5ba813f.

📒 Files selected for processing (2)
  • src/renderer/src/components/sidebar/workspace-kanban-worktree-groups.test.ts
  • src/renderer/src/components/sidebar/workspace-kanban-worktree-groups.ts

📝 Walkthrough

Walkthrough

Worktree sorting now uses a safe display-name comparison helper that falls back to an empty string when displayName is undefined. Both regular and manual sorting use the helper for tie-breaking. Regression tests cover grouping with undefined display names under manual and recent sorting modes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template sections, and it omits explicit Screenshots, Security Audit, and full AI Review Report content. Restructure the PR description to match the template and add the missing Screenshots, Testing checklist, AI Review Report, Security Audit, and Notes sections.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the main fix: guarding kanban worktree sorting against undefined displayName.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OrcaWin
OrcaWin merged commit 3bb6f91 into main Jul 14, 2026
2 checks passed
nwparker added a commit that referenced this pull request Jul 18, 2026
… (crash 99657ab1)

Worktree.displayName is typed non-optional but arrives undefined at runtime for
persisted/discovered worktrees (crash 99657ab1). PR #8683 fixed the kanban board;
this extends the same guard to the five other sort sites that share those worktree
objects — including the Cmd+J switcher (order-empty-query-worktrees) — via one
shared compareWorktreeDisplayName helper in lib/. Behavior-neutral when names are
present; a missing name now sorts as '' instead of throwing.

Co-authored-by: Orca <help@stably.ai>
nwparker added a commit that referenced this pull request Jul 18, 2026
… (crash 99657ab1) (#9315)

* fix(sidebar): guard every worktree sort against undefined displayName (crash 99657ab1)

Worktree.displayName is typed non-optional but arrives undefined at runtime for
persisted/discovered worktrees (crash 99657ab1). PR #8683 fixed the kanban board;
this extends the same guard to the five other sort sites that share those worktree
objects — including the Cmd+J switcher (order-empty-query-worktrees) — via one
shared compareWorktreeDisplayName helper in lib/. Behavior-neutral when names are
present; a missing name now sorts as '' instead of throwing.

Co-authored-by: Orca <help@stably.ai>

* docs(agents): add over-commenting anti-pattern example to the comment rule

Anchors the existing 'document the why, briefly' rule with a concrete
before/after (the worktree-displayName guard) so agents stop dumping the
crash id, mechanism, file location, and every call site into a comment.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
fix(sidebar): guard kanban worktree sort against undefined displayName (crash 99657ab1)

A worktree reached the sidebar with an undefined displayName, so the
Manual-sort comparator ran `a.displayName.localeCompare(...)` and threw
`TypeError: Cannot read properties of undefined (reading localeCompare)`,
unmounting the sidebar.worktrees error boundary.

Route both kanban comparators through a null-safe compareDisplayName so a
missing name only affects tie-break order instead of crashing. Behavior is
unchanged when displayName is present.

Adds regression tests for both the manual and recent sort paths.
@
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
… (crash 99657ab1) (stablyai#9315)

* fix(sidebar): guard every worktree sort against undefined displayName (crash 99657ab1)

Worktree.displayName is typed non-optional but arrives undefined at runtime for
persisted/discovered worktrees (crash 99657ab1). PR stablyai#8683 fixed the kanban board;
this extends the same guard to the five other sort sites that share those worktree
objects — including the Cmd+J switcher (order-empty-query-worktrees) — via one
shared compareWorktreeDisplayName helper in lib/. Behavior-neutral when names are
present; a missing name now sorts as '' instead of throwing.

Co-authored-by: Orca <help@stably.ai>

* docs(agents): add over-commenting anti-pattern example to the comment rule

Anchors the existing 'document the why, briefly' rule with a concrete
before/after (the worktree-displayName guard) so agents stop dumping the
crash id, mechanism, file location, and every call site into a comment.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant