[mark-sil]

A sidebar or list click whose handling throws no longer disables later clicks, and SidePane.CurrentItem returns null when no tab is selected, as documented. Until now ListPropertyChoice cleared its static click flag only when the click handler returned, so after an error the user chose to continue from, every later sidebar and list click was ignored for the rest of the session. SidePane.CurrentItem indexed its item areas with a null tab and threw, which SidebarAdapter would hit if a ToolForAreaNamed_<area> change arrived before the first areaChoice change; that order does not occur today.

Affected members:

• ListPropertyChoice.OnClick
• SidePane.CurrentItem
• SidebarAdapter.SetToolForCurrentArea (with no tab selected it now selects the tool instead of throwing)

Reviewer notes

Start here: ListPropertyChoice.OnClick (Src/XCore/xCoreInterfaces/Choice.cs:642-655) -- the try/finally around the static m_fHandlingClick. Then SidePane.CurrentItem (Src/XCore/SilSidePane/SidePane.cs:346-353).

Why now: both paths matter once sidebar property sets move from queued Mediator broadcasts to immediate delivery. Today the listeners of a click's property set run later, from the queue, outside OnClick; immediately delivered, they run inside HandleItemClick while the flag is set, so one listener exception would disable the sidebar. The tool-switch redesign also reorders the startup notifications, which makes "tool before area" reachable.

Where to look

• The one case that changes behaviour today: a command-list click whose handler throws an error the user continues from (the Mediator wraps the handler's exception, Mediator.cs:1409-1432; ContinuableErrorException is continuable, FieldWorks.cs:1194-1196). Later clicks now work instead of being ignored. Pinned by ListPropertyChoiceClickTests.ClickAfterAClickThatThrew_IsHandled.
• CurrentItem's null return -- its callers are SidebarAdapter.SetToolForCurrentArea (NavBarAdapter.cs:624, where null means "not current", so the tool is selected) and SidePaneTests. Pinned by SidePaneTests.CurrentItem_whenNoTabSelected (all three item-area styles) and SidebarAdapterSelectionTests.ToolForAreaChange_BeforeAnyTabIsSelected_SelectsThatTool, which also asserts no property is set.

Deliberately not here: no Pub/Sub conversion; the sidebar's property sets still broadcast through the Mediator.

Verification

• .\build.ps1 -CommentHygiene -TokenHygiene clean.
• .\test.ps1 -SkipNative -TestProject Src/XCore/xCoreTests: 27/27; -TestProject Src/XCore/SilSidePane/SilSidePaneTests: 149/149.
• Control run with main's production code: every new test fails (ArgumentNullException from CurrentItem; one handled click instead of two).
• No manual test: the null-tab order does not occur in the running app, and no UI step makes a click handler throw on demand.

Next: approve.

Preflight review details

Code Review Summary

Branch: claude/pubsub-click-path-hardening-3b5006
Base: main (merge base 1903f5d)
Date: 2026-11-04
Review model: Claude Opus 5.5
Files changed: 5 (1 commit)

Contract/API Changes

SidePane.CurrentItem returns null with no tab selected instead of throwing ArgumentNullException. Its callers are SidebarAdapter.SetToolForCurrentArea (NavBarAdapter.cs:624) and SidePaneTests.cs:456; neither relied on the exception.

Findings

Critical - Must address before merge

None.

Important - Should address before merge

None.

Minor - Consider

• If the try/finally ever regresses, ListPropertyChoiceClickTests leaves the static flag set and SidebarAdapterSelectionTests.ItemSelectedOutsideTheAdapter_StillSetsTheProperty fails in the same run (left as is: the only remedy is resetting a private static by reflection, and the flag test itself fails with a clear message)

Interview Notes

• The fix for the null tab is at the source, in SidePane.CurrentItem, rather than a guard in SetToolForCurrentArea.
• Developer-facing hardening; no Jira ticket.

🤖 Generated with Claude Code

---

This change is 

[github-actions]

NUnit Tests

    1 files  ±0      1 suites  ±0   15m 20s ⏱️ +50s
7 041 tests +5  6 956 ✅ +5  85 💤 ±0  0 ❌ ±0 
7 050 runs  +5  6 965 ✅ +5  85 💤 ±0  0 ❌ ±0 

Results for commit 3280293. ± Comparison against base commit 8afbeb5.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.51%. Comparing base (1903f5d) to head (3280293).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
Src/XCore/xCoreInterfaces/Choice.cs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1229      +/-   ##
==========================================
+ Coverage   40.45%   40.51%   +0.06%     
==========================================
  Files        1552     1559       +7     
  Lines      355828   356090     +262     
  Branches    41530    41585      +55     
==========================================
+ Hits       143949   144275     +326     
+ Misses     182579   182501      -78     
- Partials    29300    29314      +14     

Files with missing lines	Coverage Δ
Src/XCore/SilSidePane/SidePane.cs	91.87% <100.00%> (+0.04%)	⬆️
Src/XCore/xCoreInterfaces/Choice.cs	62.33% <0.00%> (+7.15%)	⬆️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thejambi]

@thejambi reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on mark-sil).