Skip to content
This repository was archived by the owner on May 26, 2026. It is now read-only.

feat(kora): KR-FE-CONFIRMDIALOG-PROP-AND-COCKPIT-A11Y-SWEEP — confirm-dialog tightening + non-multi-tenant a11y sweep - #214

Merged
rafe-walker merged 1 commit into
feature/phase2-upgradesfrom
feat/kora-KR-FE-CONFIRMDIALOG-PROP-AND-COCKPIT-A11Y-SWEEP-MEGABUCKET
May 24, 2026
Merged

feat(kora): KR-FE-CONFIRMDIALOG-PROP-AND-COCKPIT-A11Y-SWEEP — confirm-dialog tightening + non-multi-tenant a11y sweep#214
rafe-walker merged 1 commit into
feature/phase2-upgradesfrom
feat/kora-KR-FE-CONFIRMDIALOG-PROP-AND-COCKPIT-A11Y-SWEEP-MEGABUCKET

Conversation

@rafe-walker

Copy link
Copy Markdown
Owner

Summary

Closes the last two a11y deferrals from #211 + extends a11y coverage past the multi-tenant chrome to the rest of the cockpit. CC#1 NousResearch#447 still hasn't opened a PR — audit-filter verification continues to wait.

A — ConfirmDialog description-prop enforcement. Type signature on both ConfirmDialog and DeleteConfirmDialog tightens from description?: string to description: string. Dev-mode console.warn catches the runtime empty-string case the type system can't see. Drift-guard regex pins every JSX call site under web/src/ against silent prop-omission.

B — Non-multi-tenant a11y sweep. 5 pages audited, top 2-3 fixes applied each (~12 hooks added). Deferred items called out in this PR body for a future bucket.

A.1 — ConfirmDialog call-site audit

Call site Before After
OAuthProvidersCard.tsx:274 ✓ has description unchanged
DeleteConfirmDialog.tsx:17 ✓ passes through type tightened (required)
PluginsPage.tsx:569 ✓ has description (grep window earlier missed it) unchanged
WizardPage.tsx:1132 ✓ has description (from #211) unchanged
ModelsPage.tsx:621 ✓ has description unchanged
ConfigPage.tsx:646 ✓ has description unchanged
4 DeleteConfirmDialog sites (CronPage / SessionsPage / EnvPage / ProfilesPage) ✓ all pass description unchanged

The deferral concern from #211 was structural (optional prop made silent-omission possible). No call sites were actually missing — the fix is purely type-level + a runtime backstop for interpolated-string cases that resolve empty.

B — A11y findings + fixes per page

ChatPage

  • MAJOR · fixed — xterm host promoted to role=\"region\" with aria-label=\"Hermes chat terminal\". SR users previously heard only the child canvas with no announcement of the large unfocused area.
  • Deferred: aria-controls / panel-visibility sync (audit BLOCKER) — needs panel-mounting refactor; bigger than this bucket. MessageList aria-live announcer — needs structural changes to xterm output stream.

SessionsPage

  • BLOCKER · fixed — Session row click target was a plain <div onClick>. Promoted to role=\"button\" tabIndex={0} aria-expanded={isExpanded} + Enter/Space keyboard handler.
  • MAJOR · fixedaria-label includes session title + message count + relative time so SR users disambiguate rows in a long list.
  • Note on the tradeoff: couldn't make outer element a true <button> because nested action buttons (Resume / Delete) would create invalid button-in-button HTML. role+tabIndex+keyboard-handler is the standard workaround.

ModelsPage

  • MAJOR · fixed — "Use as" menu trigger gains aria-haspopup=\"menu\" + aria-expanded={open}; the menu div itself gains role=\"menu\" + aria-label. Previously SR users heard "button" with no signal that clicking opens a menu.
  • Deferred: heading-hierarchy break (modal h2 with no parent h1) — needs broader modal-structure decisions; deferred.

PluginsPage

  • MAJOR · fixed — Enable/Disable buttons get state-aware aria-label including plugin name (" is already enabled" vs "Enable runtime for "). Previously SR users heard generic "Enable runtime, dimmed" with no plugin context.
  • MAJOR · fixed — Show/Hide sidebar button: aria-label includes plugin name; decorative Eye/EyeOff icons marked aria-hidden so SRs don't double-read.
  • Deferred: plugin-card-in-li semantics — the list wrapper is <ul> but children render as <Card>. Restructuring needs design coordination on card semantics.

OAuthProvidersCard

  • MAJOR · fixed — Login button: aria-label includes provider name ("Login GitHub" rather than generic "Login"). SR users now disambiguate when multiple providers list in the same panel.
  • MAJOR · fixed — Disconnect button: same provider-aware label pattern.
  • Deferred: removing the inner Button-inside-link decorative wrapper — purely structural cleanup, no a11y impact (the link already carries the accessible name, the inner button is aria-hidden).

Build

  • tsc -b && vite build ✓ clean (existing chunk-size warning)
  • python3 -m py_compile tests/test_tenants_endpoint.py

Drift-guards added

tests/test_tenants_endpoint.py:

  • test_fe_confirm_dialog_required_description_pin — pins both type signatures + the dev-mode warning string + a regex over every .tsx under web/src/ asserting each <ConfirmDialog> / <DeleteConfirmDialog> opening tag contains a description= prop. A future call site dropping the prop fails CI.
  • test_fe_non_multi_tenant_a11y_sweep_pins — pins each per-page fix:
    • ChatPage xterm role=\"region\" + label
    • SessionsPage role=\"button\" + aria-expanded={isExpanded} + aria-label format
    • ModelsPage aria-haspopup=\"menu\" + aria-expanded={open} + role=\"menu\"
    • PluginsPage state-aware labels ("is already enabled/disabled", "Show/Hide ... sidebar") + <Eye aria-hidden> / <EyeOff aria-hidden>
    • OAuthProvidersCard \"${t.oauth.login} ${p.name}\" + \"${t.oauth.disconnect} ${p.name}\"

Test plan

Recommendation for next CC#2 dispatch

Per-tenant audit BE filter verification (still gated on CC#1 NousResearch#447). Per #208/#210/#211/#212, this is the #1 outstanding item. Check gh pr view 447 --json state,merged before dispatch.

Lighter alternatives if NousResearch#447 is still in flight:

  1. Round 2 a11y sweep on the same 5 pages — pick up the deferred items above (ChatPage aria-controls sync + MessageList aria-live, ModelsPage heading-hierarchy, PluginsPage plugin-card-in-li semantics). ~3-4 hours.
  2. Playwright-based axe CI runner (the feat(kora): KR-FE-A11Y-COMPLETION-FORCED-COLORS-AND-AXE-CORE-CI — forced-colors styling + axe-core dev integration #212 deferral) — chromium download + SPA mock harness + baseline-vs-regression diff. ~3-4 hours.
  3. Cockpit a11y sweep round 3 — pages not yet touched (HealthRollupPage, AnalyticsPage, EnvPage, CronPage, ProfilesPage). ~3-4 hours.

🤖 Generated with Claude Code

…-dialog tightening + non-multi-tenant a11y sweep

Deliverable A — ConfirmDialog description-prop enforcement:
  * ConfirmDialog.description: string (was description?: string) —
    aria-describedby binding becomes mandatory at the type level
  * DeleteConfirmDialog wrapper mirrors the requirement
  * Dev-mode console.warn when ConfirmDialog opens with an
    empty-string description (catches interpolated-string
    runtime cases the type system can't see); tree-shaken in
    prod via import.meta.env.DEV gate
  * Audit confirmed every existing call site already passes
    description (OAuthProvidersCard / DeleteConfirmDialog /
    PluginsPage / WizardPage / ModelsPage / ConfigPage / 4
    DeleteConfirmDialog sites in CronPage + SessionsPage +
    EnvPage + ProfilesPage — all good)

Deliverable B — non-multi-tenant a11y sweep (top fixes/page):

ChatPage:
  * xterm host promoted to role="region" with
    aria-label="Hermes chat terminal" so SR users hear what the
    large unfocused element is

SessionsPage:
  * SessionRow click target gets role="button" + tabIndex={0} +
    aria-expanded + keyboard handler (Enter/Space) — was a div
    with onClick that was Tab-unreachable; can't promote to
    <button> because nested action buttons would create invalid
    button-in-button
  * aria-label includes session title + message count + relative
    time so SR users disambiguate rows in a long list

ModelsPage:
  * "Use as" menu trigger gains aria-haspopup="menu" +
    aria-expanded={open}; the menu div gains role="menu" +
    aria-label so SR users hear "menu opened: Use this model as"

PluginsPage:
  * Enable/Disable buttons: state-aware aria-label including
    plugin name ("<name> is already enabled" / "Enable runtime
    for <name>" / mirror for Disable) — was generic "Enable
    runtime, dimmed" with no plugin context
  * Show/Hide sidebar button: aria-label includes plugin name;
    decorative Eye/EyeOff icons marked aria-hidden so SRs
    don't double-read

OAuthProvidersCard:
  * Login button: aria-label includes provider name
    ("Login GitHub" vs generic "Login") so SR users disambiguate
    when multiple providers list together
  * Disconnect button: same pattern

Deferred (documented in PR body): ChatPage aria-controls sync,
MessageList aria-live announcer, heading-hierarchy in ModelsPage
modal, plugin-card-in-li semantics. Recommendable for a
follow-on bucket if appetite remains.

Drift-guard extension (tests/test_tenants_endpoint.py):
  * test_fe_confirm_dialog_required_description_pin:
    - description: string type-pin on both ConfirmDialog +
      DeleteConfirmDialog
    - Dev-mode empty-desc warning string-pin
    - Regex pin: every <ConfirmDialog/<DeleteConfirmDialog JSX
      tag in web/src/ must contain description= in its opening
      block. Catches future call sites that drop the prop.
  * test_fe_non_multi_tenant_a11y_sweep_pins:
    - ChatPage xterm role+label, SessionsPage role+aria-expanded+
      aria-label format, ModelsPage menu trigger ARIA + menu role,
      PluginsPage state-aware labels + Eye/EyeOff aria-hidden,
      OAuthProvidersCard provider-aware Login/Disconnect labels

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

⚠️ npm lockfile hash out of date

Checked against commit 67dc986 (PR head at check time).

The hash = "sha256-..." line in these nix files no longer matches the committed package-lock.json:

Apply the fix

  • Apply lockfile fix — tick to push a commit with the correct hashes to this PR branch
  • Or run the Nix Lockfile Fix workflow manually (pass PR #214)
  • Or locally: nix run .#fix-lockfiles and commit the diff

@rafe-walker
rafe-walker merged commit f8bca95 into feature/phase2-upgrades May 24, 2026
2 of 4 checks passed
@rafe-walker
rafe-walker deleted the feat/kora-KR-FE-CONFIRMDIALOG-PROP-AND-COCKPIT-A11Y-SWEEP-MEGABUCKET branch May 24, 2026 20:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant