From 834031a0fd4c6a382364ac989ae0d99544b2b858 Mon Sep 17 00:00:00 2001
From: "a.tolmachev" <a.tolmachev@tbank.ru>
Date: Sat, 11 Apr 2026 23:25:22 +0300
Subject: [PATCH] docs: add frontend refactoring plan

---
 TASKS.md          |   6 +
 __REVIEW_FRONT.md | 824 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 830 insertions(+)
 create mode 100644 __REVIEW_FRONT.md

diff --git a/TASKS.md b/TASKS.md
index 15ddbf7..b91815f 100644
--- a/TASKS.md
+++ b/TASKS.md
@@ -18,6 +18,12 @@ DoD:
 
 ## Backlog
 
+- `feat/frontend-refactoring-plan`
+- `feat/frontend-xss-hardening`
+- `feat/frontend-shell-unification`
+- `feat/frontend-wizard-modularization`
+- `feat/frontend-build-pipeline`
+- `feat/frontend-performance-polish`
 - `feat/postgres-registry-modularization`
 - `feat/sqlx-compile-time-verification`
 - `feat/typed-timestamps`
diff --git a/__REVIEW_FRONT.md b/__REVIEW_FRONT.md
new file mode 100644
index 0000000..9f4bafd
--- /dev/null
+++ b/__REVIEW_FRONT.md
@@ -0,0 +1,824 @@
+# Frontend Refactoring Plan
+
+## Purpose
+
+This document defines the frontend refactoring track for the Alpine.js UI. It is based on the current implementation, the observed code smells, and the explicit review findings.
+
+The goal is not to replace Alpine.js. The stack choice is valid for the current product size. The goal is to make the UI:
+
+- safe against obvious XSS classes;
+- internally consistent;
+- easier to maintain;
+- easier to test;
+- easier to ship and profile.
+
+This document is implementation-oriented and split into concrete tracks, slices, file ownership, and acceptance criteria.
+
+## Scope
+
+The frontend refactoring scope covers:
+
+1. XSS hardening and safe DOM rendering;
+2. shell/nav/auth unification;
+3. wizard modularization;
+4. module-pattern unification;
+5. removal of inline event handler patterns;
+6. lightweight build pipeline introduction;
+7. CSS class cleanup instead of imperative inline styles;
+8. low-risk performance improvements;
+9. frontend observability and testability improvements.
+
+## Non-Goals
+
+This plan does not assume:
+
+- migration to React/Vue/Svelte;
+- a full SPA rewrite;
+- a design-system rewrite;
+- replacing Alpine.js as the core UI model.
+
+The target is a cleaner Alpine-compatible codebase, not a framework migration.
+
+## Current State Summary
+
+### Valid architectural choices
+
+- Alpine.js is an appropriate choice for this admin UI size.
+- `api.js` is already a strong shared transport layer.
+- `ui-feedback.js` is simple and correct.
+- the visual language and token system are already above average for an admin panel.
+- EN/RU i18n support is already broad and worth preserving.
+
+### Primary issues
+
+1. unsafe `innerHTML` and string-built DOM with API data;
+2. mixed UI architecture:
+   - Alpine on some pages;
+   - imperative DOM on others;
+3. `wizard.js` is a monolith;
+4. duplicated shell logic between `auth.js` and `nav.js`;
+5. inconsistent module patterns;
+6. inline `onclick="..."` string injection in some renderers;
+7. no bundling/build pipeline;
+8. direct `element.style` mutation where CSS classes should be used;
+9. avoidable repeated filtering/computation in Alpine render paths.
+
+## Guiding Rules
+
+- Keep Alpine.js.
+- Centralize common frontend primitives instead of re-implementing them page by page.
+- Prefer DOM node construction and `textContent` over string-built HTML.
+- Restrict `innerHTML` to trusted, static, translation-reviewed content only.
+- Keep shared shell state in one place.
+- Break the wizard by concern, not by arbitrary line count alone.
+- Introduce the lightest build pipeline that solves the real problems.
+
+## Track A: XSS Hardening
+
+### Problem
+
+Several files still render data from API responses through `innerHTML` and string concatenation without guaranteed escaping.
+
+Known examples from review:
+
+- `apps/ui/js/secrets.js`
+- `apps/ui/js/workspace.js`
+- error rendering paths where backend-provided strings are interpolated into HTML
+
+This is a real frontend safety issue even in an authenticated admin panel.
+
+### Goal
+
+No untrusted API data is inserted through raw `innerHTML`.
+
+### Target Design
+
+Introduce a small shared DOM/safety utility module:
+
+```text
+apps/ui/js/utils/
+  dom.js
+  escape.js
+```
+
+### `utils/escape.js`
+
+Responsibilities:
+
+- `escapeHtml(value)`
+- `escapeAttr(value)` if attribute-safe escaping is still needed during transition
+
+Rule:
+
+- this file becomes the only source of escape helpers;
+- page scripts stop defining local copies such as the current `escapeHtml()` inside `wizard.js`.
+
+### `utils/dom.js`
+
+Responsibilities:
+
+- helper builders for repetitive patterns:
+  - `textNode(value)`
+  - `el(tag, className?, text?)`
+  - `clear(node)`
+  - `appendChildren(node, children)`
+- helper renderers for empty/error banners when they currently rely on string HTML
+
+### Required Migrations
+
+#### Slice A1: shared escape helpers
+
+Files:
+
+- `apps/ui/js/wizard.js`
+- `apps/ui/js/secrets.js`
+- `apps/ui/js/workspace.js`
+- any other file using local escape helpers or raw string interpolation into HTML
+
+Actions:
+
+- move escape helpers into shared `utils/escape.js`;
+- remove duplicate local definitions.
+
+#### Slice A2: high-risk string renderers
+
+Priority files:
+
+- `apps/ui/js/secrets.js`
+- `apps/ui/js/workspace.js`
+- `apps/ui/js/api-keys.js` if any dynamic HTML remains
+- `apps/ui/js/logs.js` / `usage.js` review for dynamic interpolation patterns
+
+Actions:
+
+- replace `innerHTML` row rendering with node creation;
+- use `textContent` for names, labels, and API-provided error strings;
+- use explicit element creation for buttons and actions.
+
+#### Slice A3: error rendering contract
+
+Actions:
+
+- never assign backend error text to `innerHTML`;
+- render API errors through:
+  - `textContent`
+  - safe toast body
+  - pre-defined empty/error state templates
+
+### Acceptance Criteria
+
+- no API-provided field is rendered via raw `innerHTML`;
+- `escapeHtml()` exists once, in a shared utility file;
+- XSS-sensitive renderers use `textContent` or safe DOM builders.
+
+## Track B: Shell/Auth/Nav Unification
+
+### Problem
+
+`auth.js` and `nav.js` both:
+
+- read `localStorage.getItem('crank_user')`;
+- derive role/workspace labels;
+- update `.nav-avatar`, `.user-dropdown-name`, `.user-dropdown-role`.
+
+This creates duplicate state and drift.
+
+### Goal
+
+One module owns shell identity state and shell rendering.
+
+### Target Design
+
+Introduce:
+
+```text
+apps/ui/js/shell/
+  identity.js
+  nav.js
+  routes.js
+```
+
+### Ownership
+
+#### `shell/identity.js`
+
+Responsibilities:
+
+- session cache;
+- local mirror persistence;
+- deriving initials;
+- deriving role label;
+- deriving current workspace label;
+- rendering shell identity DOM;
+- broadcasting session change events.
+
+Functions to absorb from current files:
+
+- from `auth.js`:
+  - `roleLabel`
+  - `currentWorkspaceLabel`
+  - `clearUserMirror`
+  - `primaryMembership`
+  - `initials`
+  - `persistUserMirror`
+  - `mirroredUser`
+  - `renderShellIdentity`
+  - `replaceSession`
+- from `nav.js`:
+  - `currentUser`
+  - `fillUserInfo`
+
+#### `shell/nav.js`
+
+Responsibilities:
+
+- dropdown open/close behavior;
+- mobile nav behavior;
+- binding click handlers for:
+  - logout
+  - profile
+  - settings
+
+It must not own identity derivation anymore.
+
+#### `auth.js`
+
+Responsibilities after refactor:
+
+- login/logout/session fetch;
+- route guarding;
+- delegating shell identity updates to `shell/identity.js`.
+
+### Slice Plan
+
+##### Slice B1: move identity rendering
+
+- create `shell/identity.js`;
+- move identity ownership there;
+- update `auth.js` and `nav.js` to consume it.
+
+##### Slice B2: remove duplicate storage constants
+
+- `STORAGE_KEY` exists once;
+- `roleLabel()` exists once.
+
+##### Slice B3: event-driven shell refresh
+
+- shell reacts to `crank:sessionchange`;
+- individual page scripts stop updating shell identity directly.
+
+### Acceptance Criteria
+
+- only one module owns mirrored user state;
+- `auth.js` and `nav.js` no longer duplicate shell rendering logic;
+- shell identity remains correct after login/logout/workspace switch.
+
+## Track C: Wizard Modularization
+
+### Problem
+
+`apps/ui/js/wizard.js` is a monolith of roughly 3k lines, holding:
+
+- global state;
+- step navigation;
+- 5 protocol flows;
+- file uploads;
+- mapping;
+- test-run handling;
+- import/export;
+- UI state transitions.
+
+This is the frontend equivalent of the `postgres.rs` monolith.
+
+### Goal
+
+Split wizard code by stable responsibility while keeping the current HTML-driven workflow.
+
+### Target Layout
+
+```text
+apps/ui/js/wizard/
+  index.js
+  state.js
+  navigation.js
+  protocol-capabilities.js
+  upstream.js
+  auth-selector.js
+  mapping.js
+  test-run.js
+  protocol-rest.js
+  protocol-graphql.js
+  protocol-grpc.js
+  protocol-websocket.js
+  protocol-soap.js
+  shared.js
+```
+
+### Ownership by Module
+
+#### `wizard/state.js`
+
+Responsibilities:
+
+- encapsulate all mutable wizard state;
+- replace top-level globals with a single state object and accessors.
+
+State currently to move:
+
+- `currentStep`
+- `wizardProtocol`
+- `wizardMode`
+- `wizardEditId`
+- `wizardWorkspaceId`
+- `wizardCurrentOperation`
+- `wizardCurrentVersion`
+- upload buffers
+- protocol capability cache
+- test-run preview state
+
+Target API:
+
+- `createWizardState()`
+- `getWizardState()`
+- `resetWizardState()`
+
+#### `wizard/navigation.js`
+
+Responsibilities:
+
+- dynamic step loading;
+- `goToStep`
+- `loadStep`
+- `step3PanelId`
+- sidebar/progress rendering;
+- continue/back button states.
+
+Important cleanup:
+
+- replace inline style mutation for disabled buttons with CSS classes.
+
+#### `wizard/protocol-capabilities.js`
+
+Responsibilities:
+
+- defaults and fetched capability model;
+- `defaultProtocolCapabilities()`
+- `currentProtocolCapabilities()`
+- `loadProtocolCapabilities()`
+
+#### `wizard/upstream.js`
+
+Responsibilities:
+
+- upstream dropdown rendering;
+- upstream auth badge rendering;
+- upstream create/edit interactions;
+- upstream selection state.
+
+#### `wizard/auth-selector.js`
+
+Responsibilities:
+
+- secret/auth profile selection;
+- quick-create secret/profile flows;
+- auth form validation and serialization.
+
+#### `wizard/mapping.js`
+
+Responsibilities:
+
+- input/output mapping;
+- preview helpers;
+- field inference helpers;
+- path defaulting.
+
+#### `wizard/test-run.js`
+
+Responsibilities:
+
+- test-run request build;
+- test-run submission;
+- mode-aware result rendering for:
+  - unary
+  - window
+  - session
+  - async job
+
+#### `wizard/protocol-*.js`
+
+Responsibilities:
+
+- only protocol-specific behavior.
+
+Examples:
+
+`protocol-graphql.js`
+
+- selected GraphQL type;
+- query/mutation defaults;
+- operation name extraction;
+- top-level field extraction.
+
+`protocol-grpc.js`
+
+- descriptor upload;
+- service/method rendering;
+- reflection-related flow.
+
+`protocol-websocket.js`
+
+- subscribe/unsubscribe templates;
+- heartbeat/reconnect form state.
+
+`protocol-soap.js`
+
+- WSDL/XSD upload;
+- service catalog inspection;
+- binding apply.
+
+### Migration Order
+
+##### Slice C1: state + navigation split
+
+- extract state and navigation first;
+- preserve current behavior.
+
+##### Slice C2: protocol capability + shared helpers
+
+- move generic helper logic out of main file.
+
+##### Slice C3: GraphQL/gRPC/REST protocol extraction
+
+- highest-traffic protocol flows first.
+
+##### Slice C4: WebSocket/SOAP protocol extraction
+
+- move less common but newer protocol flows.
+
+##### Slice C5: auth selector + test-run split
+
+- finalize with secret/auth and result handling modules.
+
+##### Slice C6: thin entrypoint
+
+- `wizard/index.js` becomes the only page bootstrap file.
+
+### Acceptance Criteria
+
+- no frontend file comparable in maintenance risk to current `wizard.js`;
+- wizard state is centralized, not scattered across globals;
+- protocol-specific logic is isolated by file.
+
+## Track D: Module Pattern Unification
+
+### Problem
+
+Current JS modules use mixed patterns:
+
+- IIFE modules with encapsulation;
+- top-level globals in page scripts;
+- direct `window` leakage.
+
+### Goal
+
+Move all first-party UI code to one consistent browser-friendly module pattern.
+
+### Recommended Target
+
+Use build-generated ES module bundles, while keeping runtime API globals explicit where needed.
+
+During transition:
+
+- keep page entrypoints small;
+- expose only intentionally public page bootstrap functions on `window`;
+- no free-floating utility functions in global scope.
+
+### Rules
+
+- utility modules must never write to `window` unless they are explicitly public;
+- page entrypoints may attach one namespace if needed:
+  - `window.CrankWizard`
+  - `window.CrankShell`
+- generic helpers must stay module-local.
+
+### Acceptance Criteria
+
+- no stray utility functions like `escapeHtml()` living in global scope;
+- global exposure is explicit and minimal.
+
+## Track E: Event Handling Cleanup
+
+### Problem
+
+Some renderers still inject handlers through HTML strings such as:
+
+- `onclick="switchWorkspace('...')"`
+
+This is brittle and unsafe if values contain quotes.
+
+### Goal
+
+All dynamic UI actions use bound event listeners, not injected inline JS.
+
+### Required Migrations
+
+Priority files:
+
+- `apps/ui/js/workspace.js`
+- any renderer that currently injects action handlers via HTML strings
+
+Approach:
+
+- build nodes;
+- attach `addEventListener`;
+- carry identifiers through closures or `data-*` attributes.
+
+### Acceptance Criteria
+
+- no dynamic `onclick="..."` string generation remains in first-party code.
+
+## Track F: Build Pipeline
+
+### Problem
+
+The current UI ships many discrete JS files directly, without bundling or minification. Vendor files are committed in-repo and loaded directly.
+
+This is acceptable for early stages but no longer ideal for:
+
+- cache invalidation;
+- dependency graph hygiene;
+- payload count;
+- module splitting;
+- future lint/test integration.
+
+### Goal
+
+Introduce a lightweight build pipeline without turning the UI into a heavyweight SPA toolchain.
+
+### Recommended Tooling
+
+Use `esbuild`.
+
+Reasoning:
+
+- minimal configuration;
+- fast;
+- good enough for bundling this codebase;
+- easy Docker integration.
+
+### Target Layout
+
+```text
+apps/ui/
+  src/
+    js/
+      ...
+  public/
+    ...
+  dist/
+```
+
+This does not require a full source tree rewrite immediately. A transitional layout is acceptable:
+
+- keep current HTML layout;
+- bundle JS entrypoints into `dist/js/*.js`;
+- keep CSS mostly as-is initially.
+
+### Required Outputs
+
+- one bundle per page entrypoint where needed:
+  - `catalog`
+  - `agents`
+  - `api-keys`
+  - `logs`
+  - `usage`
+  - `settings`
+  - `workspace-setup`
+  - `wizard`
+  - `secrets`
+- shared chunk for common utilities
+
+### Docker Changes
+
+UI Docker build should:
+
+1. install npm deps;
+2. run build;
+3. serve bundled assets via nginx.
+
+### Vendor Policy
+
+Move from committed vendor runtime files toward npm-managed dependencies where possible:
+
+- `alpinejs`
+- `js-yaml`
+
+If some vendored asset remains, it must be intentional and documented.
+
+### Acceptance Criteria
+
+- first-party JS is bundled and minified;
+- cache invalidation is deterministic;
+- page HTML references built bundles, not a long list of raw scripts.
+
+## Track G: CSS State Cleanup
+
+### Problem
+
+Some UI states are expressed by directly mutating inline styles in JS.
+
+Example class of issue:
+
+- button disabled visuals set by `element.style.opacity`, `style.cursor`, etc.
+
+### Goal
+
+Move stateful visuals into CSS classes.
+
+### Target Approach
+
+Introduce semantic classes:
+
+- `.is-disabled`
+- `.is-hidden`
+- `.is-loading`
+- `.is-active`
+- `.is-selected`
+
+JS only toggles classes and attributes.
+
+### Priority File
+
+- `apps/ui/js/wizard.js`
+
+### Acceptance Criteria
+
+- no UI state requires imperative style strings when a reusable class can express it.
+
+## Track H: Performance Polish
+
+### Problem
+
+Some Alpine getters recompute filters repeatedly.
+
+Known example:
+
+- operations tab counts in `catalog.js` recompute filtered lists multiple times per render.
+
+### Goal
+
+Reduce avoidable repeated work without introducing premature complexity.
+
+### Strategy
+
+#### `catalog.js`
+
+Replace repeated tab filtering with a cached derived snapshot, for example:
+
+- one getter that computes filtered operations once;
+- one derived count map per tab;
+- `tabCount(tabId)` reads from that cached map.
+
+#### General Rule
+
+Only optimize:
+
+- repeated O(n) list filters inside render loops;
+- repeated expensive formatting inside x-for;
+- repeated DOM queries in hot paths.
+
+Do not over-engineer with memoization everywhere.
+
+### Acceptance Criteria
+
+- obvious repeated list filtering in Alpine render paths is removed;
+- rendered behavior remains identical.
+
+## Track I: Template and i18n Safety Cleanup
+
+### Problem
+
+There are small but risky patterns around naming collisions and template ergonomics.
+
+Known example:
+
+- `x-for="t in tabDefs"` where `t` also exists globally as the translation function.
+
+### Goal
+
+Avoid template variable names that collide with global utility names.
+
+### Required Rule
+
+Within Alpine templates:
+
+- do not use `t`, `tf`, or other globally meaningful helper names as loop variables.
+
+Use:
+
+- `tab`
+- `item`
+- `entry`
+- `profile`
+- `workspace`
+
+instead.
+
+### Immediate Fixes
+
+- rename `x-for="t in tabDefs"` to `x-for="tab in tabDefs"` in operations UI;
+- review similar short variable names across templates.
+
+### Acceptance Criteria
+
+- no Alpine template variable shadows translation helpers or globally meaningful names.
+
+## Track J: Frontend Observability and Testability
+
+### Problem
+
+As the UI grows, debugging and regression confidence depend on:
+
+- shared state boundaries;
+- consistent render helpers;
+- stable e2e selectors;
+- predictable logs and request tracing.
+
+### Goal
+
+Make the UI easier to verify and debug.
+
+### Work Items
+
+#### Shared test selectors
+
+Add stable `data-testid` or equivalent hooks in critical areas:
+
+- wizard protocol cards
+- secret create/rotate flows
+- auth selector
+- shell identity
+
+#### Console discipline
+
+- no stray debug logging in production code;
+- user-facing failures flow through toasts or error states only.
+
+#### Request ID surfacing
+
+When backend tracing lands, expose request IDs in developer-facing error surfaces where appropriate:
+
+- test-run result panel;
+- logs detail drawer;
+- adapter failure toasts where safe.
+
+### Acceptance Criteria
+
+- critical UI flows have stable automation hooks;
+- frontend failures are easier to trace to backend request IDs once that backend work lands.
+
+## Delivery Order
+
+Recommended order:
+
+1. `feat/frontend-refactoring-plan`
+2. `feat/frontend-xss-hardening`
+3. `feat/frontend-shell-unification`
+4. `feat/frontend-template-safety-cleanup`
+5. `feat/frontend-wizard-modularization`
+6. `feat/frontend-build-pipeline`
+7. `feat/frontend-css-state-cleanup`
+8. `feat/frontend-performance-polish`
+9. `feat/frontend-observability-and-testability`
+
+## Per-Slice Verification
+
+Each slice must run the smallest realistic verification set for the affected area.
+
+Baseline:
+
+- `node --check` for touched JS files
+- UI Docker build
+- relevant Playwright specs where available
+
+Additional rules:
+
+- XSS hardening slices must include targeted regression checks for renderer behavior;
+- shell unification slices must include login/logout/workspace switch smoke;
+- wizard modularization slices must preserve existing wizard e2e coverage;
+- build pipeline slices must validate production asset serving in Docker.
+
+## Success Definition
+
+The frontend refactoring track is complete when:
+
+- untrusted API data is no longer rendered through unsafe HTML paths;
+- shell identity logic has one owner;
+- the wizard no longer lives in a single giant file;
+- page scripts follow a consistent module pattern;
+- inline event handler injection is removed;
+- the UI ships through a lightweight build pipeline;
+- common UI states are driven by CSS classes instead of imperative style strings;
+- obvious repeated render-time computations are cleaned up.
+
+At that point, the frontend remains Alpine-based but becomes significantly safer and easier to evolve.