Cover the settings pane's routing and its admin gate - #995
Merged
Conversation
`settings-pane.tsx` and `settings-state.ts` had no tests. Between them they own which settings panel the operator sees, the `isAdmin` gate that keeps the Releases admin panel away from non-admins, the docs sub-nav, and the update stream that has to outlive a trip away from the Updates tab. `settings-state.test.ts` drives `useSettingsState` through route resolution (missing, unknown and valid sections), the admin check, the nav list it composes from the verdict, and the bounce that parks a non-admin who deep links to `/settings/releases` on General until the check confirms access. `settings-pane.test.tsx` mounts both exported components with every panel replaced by a marker that records its props, so the assertions cover the routing table, the wiring that `tsc` cannot check (two same-typed callbacks handed to the wrong child), the keyed scroll container, and the release stream's identity across a section switch. 46 mutants, 46 killed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hwpaeh6Kc7u3EFaRvywtUb
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 freeto 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.
Recurring test-enforcer run. The local suite was green on the first pass (35th consecutive), the flakes list was empty for the 13th run running, and the CI scan turned up no new failures — so the whole run went to the coverage target the last run queued up:
settings-pane.tsx, plus thesettings-state.tshook that feeds it.Between them these two modules own which settings panel the operator sees, the
isAdmingate that keeps the Releases admin panel away from non-admins, the docs sub-nav, and the update stream that has to outlive a trip away from the Updates tab. Neither had a test.settings-state.test.ts(13 tests)Drives
useSettingsStatethrough route resolution (missing, unknown, and valid sections), the admin check, the nav list it composes from the verdict, and the bounce that parks a non-admin who deep-links to/settings/releaseson General — then lands them back on Releases once the check confirms access.settings-pane.test.tsx(27 tests)Mounts both exported components with every settings panel replaced by a marker that records the props it was handed, so the assertions reach the routing table, the keyed scroll container, the docs sub-nav guard, the service dots, and the wiring
tsccannot check — two same-typed callbacks handed to the wrong child.Mutation battery
50 mutants, 50 killed.
One mutant survived the first pass: dropping
key={activeSection}from the scroll container. Rather than record it as unobservable, I checked the assumption — jsdom does no layout, but it does store a writtenscrollTop, so the behavior was reachable and the test was sharpened into a real scroll-reset assertion instead.The review pass then proved two assertions could not fail, and both are fixed:
useReleaseStreamdeleted from the component outright. The stub props map was never cleared between tests, so both sides of the identity check readundefined. The map is now cleared per test, the assertion guards on the panel actually being mounted, and a sharper sibling asserts the subscription happens while a different section is showing, which is the whole point of owning it here.IdeSettings'onChangewas never round-tripped, so rewiring it to a stray closure was type-correct and undetected. Both enablement panels now round-trip.Both regressions are in the battery above and both are killed.
Validation
pnpm run check,pnpm run format,pnpm run lint:web(0 errors),pnpm run test(server 164 files/2808, web 74/1130, ext 9/60),pnpm run test:e2e(181 passed/12 skipped),pnpm run test:e2e:live(11 passed, 20.6s, zero leakede2e-*tmux sessions),pnpm run finalize:web.No product code changed in this PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Hwpaeh6Kc7u3EFaRvywtUb