Merge pull request #122 from ProxySQL/codex/ui-restorative-topology-worktree

Modernize Orchestrator web UI and audit workflows

Author: renecannao

.gitignore

@@ -23,3 +23,4 @@ main
 /bin
 /build
 .claude/worktrees/
+.worktrees/

.superpowers/sdd/2026-08-12-failure-analysis-workspace/final-fix-report.md

@@ -0,0 +1,128 @@
+# Failure Analysis Workspace Final Fix Report
+
+Date: 2026-08-12
+
+## Scope and commit
+
+- Scope: the complete final-review fix wave for the Failure Analysis Workspace.
+- Commit: `fix(ui): complete failure analysis final review` (this report is included in that single commit; the final hash is reported in the handoff).
+- Production APIs, endpoints, refresh behavior, and visual design were unchanged.
+
+## Findings and TDD evidence
+
+### IMPORTANT: unmatched display-relevant analysis
+
+Root cause: `appendEntry` returned when `ClusterDetails.ClusterName` was absent from the `clusters-info` index, but the model did not record that discarded actionable or structural entry. The adapter consequently rendered a healthy empty or incomplete state.
+
+RED command:
+
+```text
+node --test go/http/testdata/clusters_analysis_state_test.js
+```
+
+Test-only run outcome: 10 passed, 3 failed. Relevant failures were:
+
+- `incident model tracks an unmatched structural entry`: expected `unmatchedEntryCount === 1`, received `undefined`.
+- `document adapter renders unavailable state when an actionable analysis has no matching cluster`: expected `Analysis unavailable`, received `0 active incidents across 0 clusters`.
+
+GREEN implementation:
+
+- `buildClustersAnalysisModel` increments `unmatchedEntryCount` only when an actionable or structural entry reaches `appendEntry` and lacks its cluster.
+- The adapter renders the unavailable state before topology URL adjustment or incident summary rendering whenever that count is nonzero.
+- Separate regressions cover an actionable `DeadMaster` with `clusters=[]` at the adapter boundary and a structural-only unmatched entry at the model boundary.
+
+GREEN outcome: the focused JavaScript suite passed 13/13.
+
+### MINOR 1: accurate test title and explicit actionable derivation
+
+The mixed model test was renamed from claiming actionable and downtimed derivation to the behavior it actually asserts: blocked and structural entries. A focused test now asserts the complete literal actionable entry model.
+
+Because actionable derivation was already correct, the new characterization test was mutation-checked rather than represented as a naturally failing baseline.
+
+Mutation RED command:
+
+```text
+node --test --test-name-pattern='incident model derives an actionable entry' go/http/testdata/clusters_analysis_state_test.js
+```
+
+Outcome after temporarily changing the production actionable status label: 0 passed, 1 failed, with literal `statusLabel` mismatch (`Action required` versus `Requires attention`). The mutation was reverted.
+
+Restored GREEN command: the same command passed 1/1.
+
+### MINOR 2: deterministic analysis-entry ordering
+
+Root cause: entries were appended in replication-analysis API order and only clusters were sorted.
+
+RED command:
+
+```text
+node --test go/http/testdata/clusters_analysis_state_test.js
+```
+
+Test-only run outcome: `incident model sorts entries by state, instance, and analysis` failed with the reversed API order intact.
+
+GREEN implementation: each cluster's entries are sorted by state precedence (`blocked`, `actionable`, `warning`, `downtimed`), then instance, then analysis, before cluster state derivation. The regression uses reversed mixed-state input and a hand-written literal expected order, including an analysis tie-break for the same instance.
+
+GREEN outcome: the focused JavaScript suite passed 13/13.
+
+### MINOR 3: complete workspace CSS selector scoping
+
+Root cause: the stylesheet guard rejected only newline-prefixed `.popover` and `.container` strings and did not validate arbitrary rule selectors.
+
+RED command:
+
+```text
+go test ./go/http -run 'TestClustersAnalysisWorkspaceStylesAreScoped|TestUnscopedWorkspaceCSSSelectorsRejectsArbitraryGlobalRule' -count=1
+```
+
+Test-only run outcome: build failed because the new all-selector validator did not exist.
+
+GREEN implementation:
+
+- `TestClustersAnalysisWorkspaceStylesAreScoped` now runs all selectors returned by the existing recursive `workspaceCSSSelectors` parser through the existing workspace-ID selector validator.
+- A focused real-parser regression includes `.unexpected-global` inside a media rule and asserts that it is rejected; no mock is used.
+
+GREEN outcome: the focused Go test command passed.
+
+## Files changed
+
+- `resources/public/js/clusters-analysis.js`
+- `go/http/testdata/clusters_analysis_state_test.js`
+- `go/http/static_assets_test.go`
+- `.superpowers/sdd/2026-08-12-failure-analysis-workspace/final-fix-report.md`
+
+## Full verification
+
+Command:
+
+```text
+node --test go/http/testdata/*.js && \
+node --check resources/public/js/clusters-analysis.js && \
+gofmt -w go/http/static_assets_test.go && \
+go test ./go/http -count=1 && \
+bash tests/functional/test-smoke.sh && \
+git diff --check
+```
+
+Outcome: exit 0.
+
+- Node behavior tests: 20 passed, 0 failed.
+- JavaScript syntax check: passed.
+- Go HTTP package: passed.
+- Functional smoke: 32 passed, 0 failed, 0 skipped.
+- Formatting: `gofmt` applied to the changed Go test.
+- Diff whitespace check: passed.
+- Existing healthy lab was used; no containers were recreated or restarted.
+
+## Self-review
+
+- Confirmed only display-relevant actionable and structural entries contribute to the unmatched count; non-interesting non-structural analysis remains ignored as before.
+- Confirmed any unmatched count forces unavailable rendering, preventing a partial incident list as well as a false healthy empty state.
+- Confirmed sorting is independent of API order and uses explicit state precedence followed by lexical instance and analysis keys.
+- Confirmed the CSS guard recursively checks selectors inside media rules and reports every unscoped selector.
+- Confirmed no production changes were made outside the JavaScript model/adapter and no CSS was altered.
+- Confirmed the final diff contains no unrelated workspace changes.
+
+## Concerns
+
+None. The lab remained healthy throughout verification.

.superpowers/sdd/2026-08-12-live-failover-audit-ui/final-fix-report.md

@@ -0,0 +1,94 @@
+# Final safety-fix report
+
+Date: 2026-08-12 (Asia/Bangkok)
+
+## Outcome
+
+All three Important safety findings are addressed in one focused change set.
+No live failover was run and no MySQL container was started, stopped, or
+recreated during this correction.
+
+## TDD evidence
+
+Initial RED command:
+
+```text
+go test ./go/http -run 'Test(AuditFailoverHarnessSafetyContracts|SmokeEndsOnlyMaintenanceCreatedByItsBeginCall|MaintenanceBegunResponseReturnsCreatedMaintenanceKey)$' -count=1
+```
+
+It failed at compile time with:
+
+```text
+go/http/api_test.go:60:14: undefined: maintenanceBegunResponse
+```
+
+After the minimal handler response helper was introduced, the same command
+failed on the shell regressions: missing `deadline=$((SECONDS + 90))`, curl
+max-time two, deadline loop, cleanup early-return, and keyed end-maintenance;
+it also detected `start mysql2 mysql3` and instance-based maintenance cleanup.
+
+A focused boundary RED then failed because the deadline loop did not budget its
+final curl against the remaining seconds. That test named the missing
+`remaining=$((deadline - SECONDS))` and reduced curl argument.
+
+Final focused GREEN:
+
+```text
+ok github.com/proxysql/orchestrator/go/http
+```
+
+## Implemented contracts
+
+1. Recovery polling is bounded by an actual 90-second wall-clock deadline.
+   Every curl has a two-second maximum, reduced to the remaining deadline
+   budget when necessary, and success/failure output uses actual elapsed time.
+2. `restore_lab` is a true no-op while `MYSQL1_STOPPED=false`. Once mysql1 was
+   stopped, cleanup starts mysql1 only. It never starts mysql2/mysql3; replica
+   repair uses `docker compose exec` and therefore operates only on replicas
+   that are already running.
+3. BeginMaintenance success preserves the historical `Details.Hostname` and
+   `Details.Port` fields and adds its new maintenance ID as
+   `Details.MaintenanceKey`, while preserving `Code: OK` and the existing
+   Message. Smoke validates the direct response's status, code, exact instance
+   message, instance details, and positive integer key, then calls only
+   `/api/end-maintenance/$MAINTENANCE_KEY`. Failed or unrelated responses cause
+   no cleanup call.
+
+## Additive API compatibility correction
+
+The initial safety correction represented `Details` as the maintenance-key
+number, which regressed the successful BeginMaintenance response contract for
+clients that read `Details.Hostname` and `Details.Port`. A focused TDD test
+against that implementation failed with:
+
+```text
+json: cannot unmarshal number into Go struct field .Details of type struct { Hostname string; Port int; MaintenanceKey int64 }
+```
+
+The response now embeds the original `inst.InstanceKey` fields in its details
+object and exposes `MaintenanceKey` additively. Failure responses were not
+changed.
+
+## Verification
+
+- `go test ./go/http -count=1`: pass.
+- Node UI state tests: 23/23 pass across four files.
+- `bash -n tests/functional/test-audit-ui-failover.sh tests/functional/test-smoke.sh`: pass.
+- `bash tests/functional/test-smoke.sh`: 35 passed, 0 failed, 0 skipped;
+  begin returned `Details.MaintenanceKey` 1 alongside `Hostname`/`Port`, and
+  cleanup ended that exact key.
+- `git diff --check`: pass.
+- Live failover: intentionally not run.
+
+Only Orchestrator was recreated for smoke; a before/after comparison confirmed
+all three MySQL container IDs were unchanged. An initial run failed at the
+readiness gate because the mounted binary was Darwin rather than Linux; no
+maintenance began. Rebuilding with the existing Linux/arm64 Go image resolved
+the environment mismatch, after which smoke passed.
+
+## Concerns
+
+None for the three corrected findings. Recreating Orchestrator resets the
+functional SQLite audit database by design, so historical live failover rows
+from the earlier review are no longer resident; their captured evidence remains
+in `final-report.md`. No new failover was run.

.superpowers/sdd/2026-08-12-live-failover-audit-ui/final-report.md

@@ -0,0 +1,155 @@
+# Final verification: populated audit history
+
+## Scope and commits
+
+This handoff verifies the recovered, restored functional lab and the populated
+audit-history UI evidence produced by this work.
+
+Commits created before this handoff:
+
+- `5556dbe9 test(ui): persist audit history in functional lab`
+- `52bbe45f test(ui): verify functional audit persistence`
+- `e722ba7c test(ui): exercise populated audit history`
+
+Initial report commit: `d3c81f67 docs(ui): record populated audit verification`.
+
+No production UI correction was required after browser review.
+
+## Recovery and audit evidence
+
+The controlled failure produced successful `DeadMaster` recovery records for
+`mysql1:3306`; the recorded successor was `mysql2:3306`. Two recovery records
+are present, each records `IsSuccessful: true`, `AnalysisEntry.Analysis:
+DeadMaster`, and the successor `mysql2:3306` (the most recent is ID 2).
+
+Fresh API counts from 2026-08-12 14:54 ICT:
+
+| Endpoint | Records |
+| --- | ---: |
+| `/api/audit/0` | 20 |
+| `/api/audit-failure-detection/0` | 2 |
+| `/api/audit-recovery/0` | 2 |
+
+Both detection records and both recovery records represent `DeadMaster` for
+`mysql1:3306`; the recovery records are successful with `mysql2:3306` as
+successor.
+
+## Restored topology and identity
+
+Fresh container inspection retained the IDs captured by the recovery harness:
+
+| Service | Container ID | State | Role / replication |
+| --- | --- | --- | --- |
+| mysql1 | `76e92eb4a8be` | healthy | `read_only=0` |
+| mysql2 | `ca05b9577b38` | healthy | source `mysql1`; IO `Yes`; SQL `Yes` |
+| mysql3 | `cf2ffd96825e` | healthy | source `mysql1`; IO `Yes`; SQL `Yes` |
+
+This matches the pre-restoration identity record: no MySQL container was
+recreated. `SHOW REPLICA STATUS\\G` for mysql2 and mysql3 also reported zero
+last IO and SQL errors and zero seconds behind source.
+
+## Automated verification
+
+All prescribed commands were run fresh and exited zero:
+
+| Command / suite | Result |
+| --- | --- |
+| `go test ./go/http -count=1` | 1 package passed; fresh JSON run counted 77 passing Go tests |
+| `for file in go/http/testdata/*_test.js; do node --test "$file" \|\| exit 1; done` | 4 Node test files; 23/23 tests passed |
+| `node --check resources/public/js/*.js` | 30/30 JavaScript files parsed successfully |
+| `bash tests/functional/test-smoke.sh` | 35 passed, 0 failed, 0 skipped |
+| `git diff --check` | no whitespace errors |
+
+The smoke run rediscovered all three instances and passed its audit-persistence,
+web/API, health, metrics, and ProxySQL checks.
+
+## Commit hygiene
+
+After the initial report commit `d3c81f67`, `git status --short` produced no
+output. The tracked worktree was clean; this report was the only file staged
+and committed for that handoff.
+
+## Browser evidence
+
+Task 3 inspected the populated application at the default desktop viewport and
+again at 390x844. At both sizes:
+
+- `/web/audit` displayed its populated rows and correct pager states.
+- `/web/audit-failure-detection` displayed two `DeadMaster` detections; the
+  expanded detection showed the two replicas, changelog, processing node, and
+  its recovery link.
+- `/web/audit-recovery` displayed two `DeadMaster` recoveries and working UID
+  detail links.
+- `/web/audit-recovery/id/2` displayed failed `mysql1:3306`, successor
+  `mysql2:3306`, timing and acknowledgement data, affected replicas, and all
+  26 recovery steps. Its related-detection link also rendered the corresponding
+  detail.
+
+At 390px, the table/detail shells scrolled internally without document-level
+horizontal overflow; empty and unavailable states stayed hidden while populated
+content was shown. Browser console inspection found **0 errors and 0 warnings**
+at both viewport sizes.
+
+## Safety and unresolved concerns
+
+The final state has the original mysql1 writer and two healthy replicas sourced
+from mysql1. The recovery workflow restored this topology without recreating
+containers, deleting volumes, or discarding SQLite history.
+
+Unresolved concerns: **none**. Docker Compose emitted its pre-existing
+obsolete-top-level-`version` notice and the MySQL client emitted its standard
+password-on-command-line warning during the earlier live verification.
+
+## Final safety corrections (2026-08-12)
+
+Three Important review findings were corrected without running another live
+failover:
+
+- The recovery poll now uses a `SECONDS + 90` wall-clock deadline, limits each
+  curl to at most two seconds (and to the remaining deadline budget near the
+  boundary), and reports actual elapsed seconds.
+- `restore_lab` returns immediately unless this harness stopped mysql1. During
+  restoration it starts only mysql1; mysql2/mysql3 are never started, and
+  replication repair is attempted only through `exec` against their existing
+  running containers.
+- successful `begin-maintenance` responses retain the existing Code, Message,
+  `Details.Hostname`, and `Details.Port` fields while adding the created key as
+  `Details.MaintenanceKey`. The smoke test accepts only the direct successful
+  response for mysql2, extracts its positive integer key, and ends maintenance
+  only through `/api/end-maintenance/{key}`.
+
+Strict RED evidence was captured before each correction. The handler contract
+first failed to build with `undefined: maintenanceBegunResponse`. After the
+minimal API response change exposed the shell regressions, the focused test
+reported all missing deadline/no-op/keyed-cleanup contracts and detected both
+unsafe instance cleanup branches. A second deadline-boundary RED reported the
+missing remaining-budget calculation before that behavior was added.
+
+A final scoped review found that the first key-returning response had replaced
+the historical instance details with a number. The additive compatibility test
+failed against that version because numeric Details could not decode into
+`Hostname`, `Port`, and `MaintenanceKey`. The corrected response preserves the
+two historical fields and adds the key; the smoke consumer now verifies all
+three before cleanup. Failure responses remain unchanged.
+
+Fresh GREEN verification:
+
+| Command | Result |
+| --- | --- |
+| focused three-regression `go test` | pass |
+| `go test ./go/http -count=1` | pass |
+| four `go/http/testdata/*_test.js` files | 23/23 pass |
+| `bash -n` on both changed functional scripts | pass |
+| `bash tests/functional/test-smoke.sh` | 35 passed, 0 failed, 0 skipped |
+| `git diff --check` | pass |
+
+The test binary was rebuilt for the lab's Linux/arm64 platform and only the
+Orchestrator service was recreated. The smoke test received
+`Details.MaintenanceKey` 1 alongside the historical instance fields, ended
+exactly that key, and passed 35/35 checks. MySQL container ID comparison before
+and after had no diff. The first smoke attempt failed safely at readiness
+because a host Darwin binary had been mounted into the Linux container; no
+maintenance call occurred. Rebuilding in the existing `golang:1.25.7` Linux
+image corrected that environment mismatch. No live failover was run.
+command-line-password warning during topology inspection; neither is an
+application/browser-console warning or a verification failure.