Skip to content

Release K — v0.51.35 — Kanban polish + i18n DE pluralization (6 PRs from @franksong2702) - #2016

Merged
nesquena-hermes merged 17 commits into
masterfrom
stage-329
May 10, 2026
Merged

Release K — v0.51.35 — Kanban polish + i18n DE pluralization (6 PRs from @franksong2702)#2016
nesquena-hermes merged 17 commits into
masterfrom
stage-329

Conversation

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Release K — v0.51.35 — Kanban polish + i18n DE pluralization

Six contributor PRs all from @franksong2702, theme-coherent (kanban modal/dispatcher polish + a German i18n bug fix). Each PR has tests; combined batch is 5049 → 5054 tests, 0 regressions.

Constituent PRs

PR Concern LOC
#1990 Kanban dispatcher race guard (_kanbanIsDispatching) +69/-12
#1991 German profile_skill_count pluralization (literal {count} → function) +43/-1
#1993 Kanban assignee profile-cache TTL + invalidation +63/-2
#1995 Modal focus-trap + edit-mode status hint (Closes #1974, #1986) +161/-0
#1996 Modal locale parity regression test +45/-4

(Note: #1995 was rebased on top of #1994 in @franksong2702's branch — its commits are included in #1995's merge so #1994 will auto-close as the PR commits land.)

Stage augmentation

  • 9242305a — Opus advisor caught a zh-Hant locale gap on the new kanban_status_original_hint key (added by Fix 1974: trap focus in Kanban modals #1995). Added the Traditional Chinese translation and extended the locale-parity test's modal_keys list to enforce future parity.

Tests

  • Pre-stage: 5049 passing
  • Post-stage: 5054 passing, 0 regressions, 154s suite (Python 3.11, HERMES_HOME isolated)
  • All modified .js files pass node -c

Especially keen for your eyes on

  1. Stage merge conflict resolution at static/panels.js:1707-1722 — both fix(kanban): invalidate profile cache for assignee select #1993 (cache TTL state) and Fix 1974: trap focus in Kanban modals #1995 (focus-trap cleanup) added new module-level let declarations adjacent to _kanbanProfileNamesCache. Resolved by preserving both blocks (independent variables). Opus advisor verified clean.
  2. Race-guard error path on fix(kanban): guard dispatcher actions while in-flight #1990_kanbanIsDispatching = true is set BEFORE the confirm dialog; verify all error paths hit the finally { _kanbanIsDispatching = false; ... } block.
  3. DE plural function form (fix(i18n): correct German profile_skill_count pluralization #1991) — Replaces a previously broken literal '{count} Fähigkeiten' (the {count} token was never substituted because t() only handles numbered {0} tokens). The function form fixes a pre-existing user-visible bug.

Opus advisor verdict

SHIP-WITH-CAVEATS → caveat addressed in stage augmentation 9242305a. All 6 brief concerns now verified clean.

Closes

#1974 #1984 #1985 #1986 #1989

(Plus #1994 auto-closes via #1995's commits being reachable.)

@nesquena-hermes
nesquena-hermes requested a review from nesquena May 10, 2026 17:07

@nesquena nesquena left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — end-to-end ✅ (clean approve, 6-PR release train + zh-Hant locale-parity stage augment all verified)

What this ships

Release K / v0.51.35 — release-train PR aggregating 6 contributor PRs from @franksong2702 plus one stage augmentation, all theme-coherent (Kanban polish + i18n DE pluralization). +414/-19 LOC across 7 files. Stage augmentation 9242305a added Traditional Chinese translation for kanban_status_original_hint and extended the locale-parity test's modal_keys list.

PR Concern LOC
#1990 Kanban dispatcher race guard (_kanbanIsDispatching) +69/-12
#1991 German profile_skill_count pluralization (literal {count}(count) => function) +43/-1
#1993 Kanban assignee profile-cache 30s TTL + explicit invalidation +63/-2
#1995 Modal focus-trap + edit-mode status hint (#1974, #1986) +161/-0
#1996 Modal locale parity regression test +45/-4
stage 9242305a zh-Hant translation for kanban_status_original_hint + parity test extension small

Closes #1974, #1984, #1985, #1986, #1989. (#1994 auto-closes via #1995's commits being reachable.)

Cross-tool trace

Frontend-only — no config.yaml writes, no agent IPC, no kanban_db schema change. Verified zero references to /api/kanban/dispatch and profile_skill_count in the fresh hermes-agent tarball:

grep -rn "/api/kanban/dispatch\|profile_skill_count" /tmp/hermes-agent-fresh/ --include="*.py"
# (empty)

Cross-tool surface: zero. ✓

End-to-end traces

#1990 — Race guard: _kanbanIsDispatching flag at module scope at static/panels.js:15. Both nudgeKanbanDispatcher() (line 1426) and runKanbanDispatcher() (line 1454) check it on entry, set on entry, clear in finally. _setKanbanDispatcherButtonsDisabled() toggles disabled + .disabled class on .kanban-run-dispatch-btn and .kanban-nudge-dispatch-btn. The if (!ok) return cancel path inside runKanbanDispatcher's try block correctly hits the finally to clear state.

#1991 — DE pluralization: t() at static/i18n.js:9260-9271 handles function values directly:

function t(key, ...args) {
  const val = _locale[key] ?? LOCALES.en[key];
  if (val === undefined) return key;
  if (typeof val === 'function') return val(...args);   // ← function form
  if (args.length) {
    return String(val).replace(/\{(\d+)\}/g, ...);       // ← numbered placeholders only
  }
  return val;
}

The pre-fix '{count} Fähigkeiten' literal contained the named token {count} which t() only handles via numbered {0} placeholders — so 1, 5, 999 skills all rendered as {count} Fähigkeiten. The fix replaces the literal with (count) => \${count} Fähigkeit${count === 1 ? '' : 'en'}`whicht('profile_skill_count', 5)` invokes correctly. ✓

#1993 — Profile cache invalidation: 30s TTL via Date.now() - _kanbanProfileNamesCacheAt < _KANBAN_PROFILE_NAMES_CACHE_TTL_MS (static/panels.js:1722-1727). _invalidateKanbanProfileCache() (static/panels.js:1709-1712) is called from saveProfileForm() (static/panels.js:4587), deleteCurrentProfile() (static/panels.js:4308), and deleteProfile() (static/panels.js:4607). All three mutation paths hook in.

#1995 — Focus trap + status hint:

  • _trapModalFocus(modalEl) at static/panels.js:1962 collects focusable elements once at install time, intercepts Tab/Shift-Tab inside the modal element, wraps cursor at first/last. Returns cleanup function. Captured-at-install means dynamically-added elements aren't included — but in this codebase, openKanbanEdit populates the assignee select BEFORE the trap installs, so the live select has its options when collected. ✓
  • _kanbanSetTaskModalStatusHint(realStatus, editableStatus) at static/panels.js:1933 shows the inline hint when they differ. Uses textContent (not innerHTML) — XSS-safe even if realStatus were attacker-controlled. ✓

Stage augmentation 9242305a — zh-Hant locale parity:

  • Added Traditional Chinese translation '實際狀態:{0}。此對話框僅支援編輯 Triage/Todo/Ready。' at static/i18n.js:6537.
  • Extended tests/test_kanban_ui_static.py::test_kanban_modal_locale_parity's modal_keys list to include kanban_status_original_hint. Future Kanban modal keys missing from any locale will fail CI.

Behavioural harness — focus trap

Extracted _trapModalFocus() into a Node harness with stubbed DOM:

Tab from m1 → m2                              ← linear next
Tab from m2 → m3
Tab from m4 (last) → m1                       ← wrap forward
Shift-Tab from m1 (first) → m4                ← wrap backward
Tab from outside-scope (-1) → m1              ← idx === -1 path: focus first
Tab from m1 after cleanup → m1                ← handler removed

All 6 transitions correct. The idx === -1 path (focus moved outside the trap, then Tab pressed) correctly bounces back to the first focusable element.

Behavioural harness — race guard

Case 1 (3x rapid dispatch):       apiCalls = 1 (expected 1)  ← guard wins
  buttonsDisabled = false  (post-resolve)
  _kanbanIsDispatching = false
Case 2 (run + nudge concurrent):  apiCalls = 1 (expected 1)
Case 3 (error path):              _kanbanIsDispatching = false  (finally clears)
                                  buttonsDisabled = false

Three concurrent nudgeKanbanDispatcher() calls produce exactly one network request. run + nudge raced concurrently: same — only the first wins the flag. Error path correctly clears state in finally. ✓

Other audit — things that are correct already

  • t() handles function values at static/i18n.js:9263if (typeof val === 'function') return val(...args) runs BEFORE the {N} placeholder fallback. The DE pluralization function form works correctly.
  • Status hint uses textContent at static/panels.js:1944 — no innerHTML, no XSS surface. Even an attacker-controlled realStatus (which can't actually happen — comes from _validate_status server-side) would render as literal text.
  • Status hint i18n fallback chain: t(\kanban_status_${realStatus}`)lookup falls back torealStatus` raw string if the key doesn't exist. So even an unexpected status renders sensibly. ✓
  • Race guard order in runKanbanDispatcher(): _kanbanIsDispatching = true BEFORE the confirm dialog. If the user takes 30s in the dialog, dispatcher buttons stay disabled — but the dialog itself takes focus, so the user can't double-click anyway. The if (!ok) return cancel path inside try correctly hits the finally and re-enables.
  • Profile cache invalidation is single-threaded by JS event loop — no race. The 30s TTL also catches CLI-side / cross-tab profile changes within ~30s. ✓
  • _kanbanLoadProfileNames() error path sets _kanbanProfileNamesCache = [] AND _kanbanProfileNamesCacheAt = Date.now() — 30s rate-limit on retries when /api/profiles fails. Without the timestamp update, a failing API would retry every modal open. Sensible.
  • Focus trap selector 'a[href], button, textarea, input, select, summary, [tabindex]:not([tabindex="-1"])' covers all native interactive elements + opt-in via tabindex="0". Standard a11y pattern.
  • Focus trap collects ONCE at install, not per-keydown. For our modals, populate happens BEFORE trap install, so dynamic elements are included. The static analysis confirmed this ordering in both openKanbanCreate and openKanbanEdit.
  • Modal close path cleanup_kanbanTaskModalFocusCleanup and _kanbanBoardModalFocusCleanup both checked-then-called-then-nulled in close handlers. No leak across open→close→open cycles.
  • DE function form returns string(count) => \${count} Fähigkeit${count === 1 ? '' : 'en'}`returns a string.t()returns whatever the function returns. The plural rule1 → 'Fähigkeit', ≠1 → 'Fähigkeiten'` matches German plural rules.
  • _kanbanSetTaskModalStatusHint(null) (single-arg) and (null, null) (two-arg) both no-op the hint via the !realStatus guard. Defensive.
  • Stage merge resolution at _kanbanProfileNamesCache — the two PRs (#1993 cache TTL state + #1995 focus-trap cleanup tracker) added independent module-level let declarations. Both preserved cleanly: _kanbanProfileNamesCacheAt, _KANBAN_PROFILE_NAMES_CACHE_TTL_MS, _kanbanTaskModalFocusCleanup, _kanbanBoardModalFocusCleanup are all on adjacent lines with no name collision.
  • i18n parity extendedtest_kanban_modal_locale_parity now anchors on kanban_status and asserts modal_keys (including kanban_status_original_hint) for every locale that has the anchor key. The _locale_blocks_with_body helper handles quoted locale keys ('zh-Hant') and unquoted (en, de).
  • node -c static/panels.js + node -c static/i18n.js — both clean (per PR body).

Edge-case trace

Scenario Expected Actual
3x rapid Run dispatcher click 1 API call (race guard wins)
Run + Preview raced concurrently 1 API call total
Run dispatcher → user cancels confirm dialog flag cleared, buttons re-enabled, no API call
Dispatcher API call throws flag cleared in finally, error toast shown
German profile_skill_count(1) '1 Fähigkeit'
German profile_skill_count(5) '5 Fähigkeiten'
Old DE literal '{count} Fähigkeiten' replaced — would render literally pre-fix
Profile cache fresh (< 30s) reuse cached names
Profile cache stale (> 30s) refetch from /api/profiles
Profile cache invalidate after saveProfileForm next modal open shows new profile
Profile cache invalidate after deleteCurrentProfile next modal open omits deleted profile
Profile cache invalidate after deleteProfile (sidebar) next modal open omits deleted profile
Modal Tab from last input wraps to first focusable
Modal Shift-Tab from first input wraps to last focusable
Modal close → reopen new trap installed, no listener leak
Edit task with running status hint shows "Actual status: Running"
Edit task with triage status hint hidden (no mismatch)
Status hint with attacker-controlled realStatus escaped via textContent
zh-Hant locale viewing edit modal of running task shows translated '實際狀態:Running. ...' (label still EN since kanban_status_running not zh-Hant translated, but the surrounding sentence is zh-Hant)
Future kanban modal key without zh-Hant parity test fails CI
Cross-tool: agent CLI doesn't render kanban modal n/a

Tests

  • tests/test_kanban_ui_static.py: 40/40 pass (existing + 5 new for this PR's surfaces — race guard, focus trap, profile cache, status hint, locale parity).
  • tests/test_issue1989_profile_skill_count.py: 1/1 pass — DE locale extracted, asserts (count) => arrow function form.
  • Full suite: 4948 passed, 59 skipped, 3 xpassed, 0 PR-related failures (9 pre-existing macOS shell failures in test_ctl_script.py + test_docker_env_readonly_vars.py reproduce on master without these changes).
  • Behavioural harnesses: focus trap (6 transitions correct), race guard (3 concurrent → 1 API call, error path clears).

Minor observations (non-blocking)

  1. Race guard sets flag BEFORE confirm dialog — buttons stay disabled while user is reading the confirm dialog. The confirm dialog itself is modal and takes focus, so users can't double-click underlying buttons. UX is fine; a stricter implementation would set flag AFTER user confirms but before the API call. Either is defensible.
  2. zh-Hant translation for kanban_status_running/blocked/done/archived is still English in the zh-Hant locale block — the status hint sentence is translated but the embedded label falls through. Consistent with prior i18n debt; not introduced by this PR. Worth a follow-up zh-Hant locale completion.
  3. Profile cache TTL of 30s is fixed. A user who creates a profile elsewhere and immediately opens a Kanban modal in this WebUI tab will see the stale cache for up to 30s. The explicit _invalidateKanbanProfileCache() hooks cover same-tab mutations; cross-tab/CLI changes rely on the TTL. Reasonable balance.
  4. Focus trap collects focusables once at install — if the modal dynamically adds a new focusable (e.g., showing a hint span with tabindex="0" after open), Tab won't include it. Current modals don't do this, but future contributors should re-collect on mutation if they add dynamic focusables.
  5. _kanbanSetTaskModalStatusHint is called with (null, null) in closeKanbanTaskModal — verbose vs. just _kanbanSetTaskModalStatusHint(null) (which the !realStatus guard handles identically). Cosmetic; the explicit two-arg form is symmetric with the open-edit call.
  6. Stage augmentation 9242305a is correctly committed as a separate commit with an Opus-advisor follow-up message. Future audit trails will see the SHIP-WITH-CAVEATS verdict and the addressing commit. Good release-train hygiene.

Recommendation

Approved. Six contributor PRs land cleanly with disjoint i18n keys, disjoint tests, and a single non-trivial integration point at the _kanbanProfileNamesCache declaration block (resolved correctly — both #1993 and #1995 added independent let declarations on adjacent lines). The Opus advisor's zh-Hant locale-parity gap was already addressed in stage augmentation 9242305a. Behavioural harnesses verify the race guard prevents double-dispatch, the focus trap cycles correctly with cleanup, the DE pluralization function form works through t(), and the profile cache invalidation hooks cover all three mutation paths.

Cross-tool safe (frontend-only). XSS audit clean (textContent for the new status hint). i18n parity verified via the new locale-parity test that future kanban modal keys must satisfy.

Parked at approval — ready for the release agent's merge/tag pipeline.

@nesquena-hermes
nesquena-hermes merged commit ad290cc into master May 10, 2026
3 checks passed
@nesquena-hermes
nesquena-hermes deleted the stage-329 branch May 11, 2026 06:26
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
Release K — v0.51.35 — Kanban polish + i18n DE pluralization (6 PRs from @franksong2702)
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
Release K — v0.51.35 — Kanban polish + i18n DE pluralization (6 PRs from @franksong2702)
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.

a11y: focus-trap inside kanban task modal + create-board modal (Opus #1965 follow-up)

2 participants