Skip to content

KeySelector: disambiguate zones served by multiple keys - #63

Merged
ttpears merged 2 commits into
mainfrom
feat/keyselector-multikey-disambiguation
Jul 22, 2026
Merged

KeySelector: disambiguate zones served by multiple keys#63
ttpears merged 2 commits into
mainfrom
feat/keyselector-multikey-disambiguation

Conversation

@ttpears

@ttpears ttpears commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Why

KeySelector's zone dropdown lists availableZones — a de-duped union across all keys. So a zone name served by more than one key (e.g. a split-horizon internal vs external view) shows up once, and selectZone silently auto-picks a key. The user can neither tell the zone is multi-key nor choose which key/view. (This only matters in the zone-first flow; picking a key first already narrows the list to that key.)

What

  • Until a key is chosen, a zone served by >1 key expands into one option per serving key — zone — keyName (server) — and selecting one sets the key and zone together. Single-key zones are unchanged.
  • Once a key is selected, nothing splits (the list is already that key's zones).
  • Same splitting inside the reverse-zone sub-dropdown; helper text hints when multi-key zones are present.
  • KeyContext.selectKeyAndZone(key, zone) added to set both atomically — calling selectKey then selectZone would read stale React state and re-run the auto-pick, clobbering the chosen key.
  • Composite option value is zone|keyId (| can't appear in a DNS name or our key_<ts>_<hex> IDs).

Known edge (documented): a persisted zone-only selection (no keyId) for a multi-key zone shows nothing selected until the user picks a per-key entry — rare, from older persisted state.

The backend already enforces the explicit keyId (3.2.0); this makes the UI express it unambiguously.

Tests

Extended the KeySelector suite: multi-key zone splits per key; single-key unchanged; per-key selection sets both key and zone; no split once a key is selected; reverse-zone grouping still works. Frontend 252/252 + tsc --noEmit + build clean.

No version/CHANGELOG bump — kept version-neutral so the pending batch (#60/#61/#62 + this) can share one consolidated release.

🤖 Generated with Claude Code

The zone dropdown lists availableZones (a de-duped union across all keys), so a
zone name served by more than one key (e.g. split-horizon internal vs external)
appeared once and selectZone auto-picked a key arbitrarily. Until a key is
chosen, expand such a zone into one option per serving key
("zone — keyName (server)") so the user picks the exact key/view; selecting one
sets key and zone together. Single-key zones are unchanged, and once a key is
selected the list is already narrowed to that key's zones (no splitting). The
same splitting applies inside the reverse-zone sub-dropdown, and the helper text
hints when multi-key zones are present.

KeyContext gains selectKeyAndZone(key, zone) to set both atomically — calling
selectKey then selectZone would read stale state and re-run the auto-pick,
clobbering the chosen key.

Composite option value is `zone|keyId` (| can't appear in a DNS name or in our
key IDs). Known edge: a persisted zone-only selection (no keyId) for a multi-key
zone shows nothing selected until the user picks a per-key entry.

Tests: KeySelector suite extended (multi-key zone splits per key; single-key
unchanged; per-key selection sets both key and zone; no split once a key is
selected; reverse grouping still works). Frontend 252/252 + typecheck + build clean.
@ttpears

ttpears commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Code review

Found 1 issue:

  1. Selecting a multi-key zone and then clearing the Key dropdown to "None" (or reloading from a persisted zone-only selection) leaves selectedZone set while selectedKey is null. In that state renderZoneMenuItems emits only the composite zone|keyId options, but the <Select>'s controlled value is the plain zone name (validSelectedZone) — so no option matches. MUI renders the field blank and logs an "out-of-range value" warning, while the helper text still reads "Managing <zone>". Before this PR the zone always rendered as a plain MenuItem, so the value matched; this is a regression of the zone-only-persistence behavior added in ee177d3. The reverse-zone sub-select (L245) has the same mismatch. It is called out as a "known edge" in the PR body, but the deselect-key path is a normal one-click action, so it's worth fixing — e.g. also emit a plain fallback option for the selected multi-key zone when no key is selected, or clear the zone when the key is deselected.

<Select
labelId="zone-select-label"
value={groupReverse && selectedIsReverse ? REVERSE_SENTINEL : validSelectedZone}
onChange={(e) => handleZoneSelect(e.target.value)}
label="Select Zone"

(root cause is the split in renderZoneMenuItems:

const renderZoneMenuItems = (zones: string[]) =>
zones.flatMap(zone => {
const serving = selectedKey ? [] : keysServingZone(zone);
if (serving.length > 1) {
)

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

… key

Code review found a regression: after selecting a multi-key zone and then
clearing the Key dropdown (or reloading a persisted zone-only selection),
selectedZone is set with selectedKey null. In that state renderZoneMenuItems
emits only the per-key composite options, so the plain zone name is not a
selectable value — the controlled Select bound to it rendered blank and logged a
MUI "out-of-range value" warning while the helper text still said "Managing X".

That state is genuinely ambiguous (no view chosen), so bind the Select to '' when
!selectedKey and the selected zone is served by >1 key (ambiguousZoneOnly), and
have the helper text prompt for a key/view instead of claiming "Managing". Covers
both triggers at the render layer (no KeyContext change). The per-key options
remain available to disambiguate.

Test: new case asserting the field shows no committed zone and prompts for a
view in that state. Frontend 253/253 + typecheck clean.
@ttpears

ttpears commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

Fixed in 69f590a: when no key is selected and the chosen zone is served by multiple keys, the Select now binds to '' (not the unselectable plain zone name) and the helper text prompts for a key/view — no more blank field / out-of-range warning / contradictory "Managing" text. Covers both the deselect-key and persisted-reload paths at the render layer. Added a regression test; frontend 253/253 + typecheck clean.

@ttpears
ttpears merged commit 5bafba2 into main Jul 22, 2026
3 checks passed
@ttpears
ttpears deleted the feat/keyselector-multikey-disambiguation branch July 22, 2026 03:47
@ttpears ttpears mentioned this pull request Jul 22, 2026
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