refactor(ui): consolidate sidebar navigation contract#100612
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 1:10 AM ET / 05:10 UTC. Summary PR surface: Source +19, Tests +17. Total +36 across 4 files. Reproducibility: not applicable. as a refactor; there is no user-facing bug path to reproduce in this PR. Source inspection does confirm current main carries both the section model and the pinned-route model. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Keep one canonical customizable-sidebar route contract, and resolve the separate Workboard typed-hook allowlist failure in the same landing batch or a narrow prerequisite before merge. Do we have a high-confidence way to reproduce the issue? Not applicable as a refactor; there is no user-facing bug path to reproduce in this PR. Source inspection does confirm current main carries both the section model and the pinned-route model. Is this the best way to solve the issue? Yes for the sidebar contract: removing the test-only section API is the narrow maintainable cleanup for the UI surface. It is not the complete solution for the broader regression tracked in #100613 because the Workboard hook allowlist still needs a separate fix. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d5371395218c. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +19, Tests +17. Total +36 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
76aa830 to
97a90a4
Compare
* fix(ui): restore sidebar navigation contract * refactor(ui): remove duplicate sidebar navigation model
* fix(ui): restore sidebar navigation contract * refactor(ui): remove duplicate sidebar navigation model
What Problem This Solves
Current
mainbuilds again after the emergency export restoration in55a0012c44b, but it now carries two sidebar navigation models: the section API introduced by #100535 and the customizable route contract consumed by the sidebar and settings flows. The section API has no production consumer and duplicates route policy.Why This Change Was Made
Keep one canonical customizable-sidebar contract instead of retaining aliases around an unused model. The route list, persisted pin normalization, default pins, and derived More menu share one owner. Worktrees remains a normal pinnable route and is exercised through the existing browser persistence flow.
User Impact
No new product contract beyond the already-landed sidebar and Worktrees features. Users can pin Worktrees from Customize sidebar and retain that choice across reloads; maintainers have one typed route policy instead of two competing models.
Evidence
55a0012c44brestored the missing exports and build, leaving the unused section API alongside them.tbx_01kwtv7wj41ka3fybn46qhbrv7: 57/57 navigation and settings tests passed.9c8c48868bc40e63c8712d8262a14e3e5b72fcc1: Blacksmith Testboxtbx_01kwtxvd6n9vkdjxa2has3g3h8passed 57/57 navigation/settings tests and the production UI build (998 modules, Worktrees chunk emitted); run 28769594466.