lmxopcua/archreview/plans/04-adminui-plan.md

# Architecture Review 04 — AdminUI: Design + Implementation Plan

> **Status (2026-07-08):** not started. Top item: C-1 (ungated mutating pages → `ConfigEditor` policy +
> reflection guard). No bUnit → live-verify on docker-dev. See [`STATUS.md`](STATUS.md).

- **Source report:** `archreview/04-adminui.md` (commit `9cad9ed0`)
- **Plan author date:** 2026-07-08 · verified against current tree at `9cad9ed0`
- **Scope:** AdminUI pages, ScriptAnalysis backend, tag editors, authorization
- **Repo constraint that shapes every test recipe:** there is **no bUnit**. Razor `@code`/binding logic and page-level `[Authorize]` gating are only observable by live-verify on the docker-dev rig (AdminUI `http://localhost:9200`). *And* on docker-dev **login is disabled** (`Security:Auth:DisableLogin=true` → `AutoLoginAuthenticationHandler` grants `DevAuthRoles.All` = every role incl. Administrator/Designer/Operator). **Consequence: a Viewer-denied path cannot be observed on the default rig** — negative authz must be proven by (a) a pure policy-registration unit test, (b) a reflection test over page attributes, and (c) a manual real-login pass with a Viewer LDAP bind. This is called out in every authz recipe below.

## Verification summary

| ID | Sev | Status | Evidence |
|---|---|---|---|
| C-1 | High | **Confirmed** | 30+ pages carry fully-qualified bare `@attribute [Microsoft.AspNetCore.Authorization.Authorize]`; only `Deployments`/`Scripts`/`ScriptEdit` use `Roles="Administrator,Designer"`, `RoleGrants` uses `Policy="FleetAdmin"`, `Certificates`/`Alerts`/`DriverStatusPanel` use imperative `"DriverOperator"`/`"FleetAdmin"`. `FallbackPolicy` is only `RequireAuthenticatedUser()`. |
| C-2 | Med | **Confirmed** | `ScriptAnalysisEndpoints.cs:17-18` = `Roles="Administrator,Designer"`; `CLAUDE.md:223` + `docs/plans/2026-06-11-adminui-disable-login-design.md:141` say "FleetAdmin". |
| C-5 | Med | **Confirmed** | `ScriptEdit.razor` / `Deployments.razor` / `Fleet.razor` / `Hosts.razor` inject `IDbContextFactory` and run EF + `SaveChangesAsync` in `@code`; UNS pages use `IUnsTreeService`. |
| S-1 | Med | **Confirmed** | `Fleet.razor:171` + `AlarmsHistorian.razor:90` sync `Dispose() => _timer?.Dispose()`; `DriverTestConnectButton.razor:69` `System.Timers.Timer` with `async (_,_)` `Elapsed` + sync `Dispose`. (`DriverStatusPanel`/`Alerts`/`Hosts` already correct via `System.Threading.Timer`+`DisposeAsync`.) |
| S-2 | Med | **Confirmed** | `Fleet.LoadAsync` (119-159) `try/finally` no `catch`; `AlarmsHistorian` (73-78) bare `catch {}` leaving "Loading…". |
| S-3 | Med | **Confirmed** | `grep ErrorBoundary` = none; `MainLayout.razor:53` `<ChildContent>@Body</ChildContent>` unwrapped. |
| S-4 | Med | **Confirmed** | `GlobalUns.ConfirmDeleteAsync` re-`Load…Async` then passes fresh `.RowVersion`; `EquipmentPage.DeleteTag/VirtualTag/Alarm` (370/415/460) same ("Load … fresh to capture its current RowVersion"). |
| S-5 | Med | **Confirmed** | `EquipmentPage.razor:172/217/263` single-click `@onclick="() => DeleteTag(...)"`; `ScriptEdit.DeleteAsync` single-click. `GlobalUns` has a confirm modal; these don't. |
| S-6 | Low | **Partial** | `DriverStatusPanel` *does* drain via `DisposeAsync` (287-310), but `ShowOpResult` replaces the chip timer with a sync `Dispose()` and no token guard → an already-queued callback can clear a newer message. Race is real but narrow. |
| S-7 | Low | **Confirmed** | `AdminOperationsClient.cs:58-59` `AskAsync<T>` forwards only caller `ct`; typed methods double-guard with `AskTimeout`. |
| P-1 | Med | **Confirmed** | `monaco-init.js:205-207` `onDidChangeModelContent` → `invokeMethodAsync("OnValueChanged", editor.getValue())` no debounce; diagnostics ARE debounced (500 ms). |
| P-2 | Med | **Confirmed** | `ScriptAnalysisService.Analyze` (60-67) re-`ParseText` + `CSharpCompilation.Create` per call; refs/preamble static (good), no `(text→tree)` memo. |
| P-3/P-4/P-5 | Low | **Confirmed** (spot-checked) | Per-circuit polling / `SnapshotAndFlatten` per Deployments render / no `@key` on `Alerts` rows — accepted as bounded. |
| P-6 | Low | **Confirmed** | `monaco-init.js:99-117` registers inlay-hints provider that POSTs every change; `InlayHints` endpoint always empty. |
| C-3 / C-4 | Good | **Confirmed good** | Tag-editor map covers 7 drivers + Galaxy raw path; all driver pages carry `JsonStringEnumConverter`, pinned by `*FormSerializationTests`. No action. |
| U-1..U-4 | — | **Confirmed** | No bUnit; documented stubs; unvalidated raw-JSON fallback; thin a11y. |

**Nothing in this report is stale/already-fixed.** All actionable findings reproduce at `9cad9ed0`. One nuance correction: the task brief mentioned a `DriverAdmin` policy — **no such policy exists**; only `FleetAdmin` and `DriverOperator` are registered (`Security/ServiceCollectionExtensions.cs:155-160`).

---

## Priority order

1. **C-1** (High, overall action-item #5) — gate the mutating surface; standardize on one policy idiom with constants.
2. **C-2** (Med) — folds into C-1 (converge ScriptAnalysis gate + fix the doc).
3. **S-4 + S-5** (Med) — delete concurrency + confirmation.
4. **S-3** (Med) — `ErrorBoundary`.
5. **S-1 + S-2** (Med) — timer/error-handling convergence.
6. **P-1** (Med) — debounce Monaco value push.
7. **P-2** (Med) — ScriptAnalysis compilation memo.
8. **C-5** (Med) — declare service-seam canonical; migrate `ScriptEdit`.
9. **Batch Lows** — S-6, S-7, P-3/4/5, P-6, C-6, U-2, U-3, U-4.

---

## 1. C-1 (High) — Standardize authorization; gate the mutating surface

**Restated:** Three authz idioms coexist and the largest mutating surface (UNS, equipment, cluster/node/namespace/ACL editors, all 8 driver pages, Reservations) carries only bare `[Authorize]`, so any authenticated user — incl. a read-only Viewer — can create/edit/delete config. Only the deploy/scripts pages are role-gated.

**Verification (confirmed):**
- Policies defined: `Security/ServiceCollectionExtensions.cs:143-161` — `FleetAdmin = RequireRole("Administrator")`, `DriverOperator = RequireRole("Operator","Administrator")`, `FallbackPolicy = RequireAuthenticatedUser()`. No "write" policy exists.
- Roles: `AdminRole` enum = `Viewer`, `Designer`, `Administrator` (`Configuration/Enums/AdminRole.cs`); `Operator` is an appsettings-only control-plane role (`DevAuthRoles.cs:14`).
- Idiom census (all string literals, no constants):
  - `Roles="Administrator,Designer"`: `Deployments.razor:12`, `Scripts.razor:2`, `ScriptEdit.razor:5`, `ScriptAnalysisEndpoints.cs:18`.
  - `Policy="FleetAdmin"`: `RoleGrants.razor:2` (page), `Certificates.razor:90` (`AuthorizeView`), `Certificates.razor:186` (imperative).
  - Imperative `"DriverOperator"`: `Alerts.razor:165`, `DriverStatusPanel.razor:166`, `GalaxyAddressPickerBody.razor:124`, `OpcUaClientAddressPickerBody.razor:75`.
  - **Bare `[Authorize]`** (= FallbackPolicy, any authenticated user): `GlobalUns`, `EquipmentPage`, `ClusterEdit`, `NodeEdit`, `NamespaceEdit`, `NewCluster`, `AclEdit`, `ClusterAcls/Namespaces/Drivers/Overview/Audit/Redundancy`, `ClustersList`, all 8 driver pages + `DriverEditRouter` + `DriverTypePicker`, `Reservations`, plus the read-only dashboards.

**Root cause:** Pages were authored by copying the nearest neighbor; the fused Host's `FallbackPolicy` makes bare `[Authorize]` *look* protective (you must be logged in) while providing zero role separation. No policy-name constants exist to make the correct gate discoverable, so drift is the path of least resistance. Project posture (memory: roles are global, "simplest authz") was never actually applied to the mutating pages.

### Proposed design

**Standardize on the policy idiom with named constants**, and introduce one write policy.

1. **Add a policy-constants type** in the Security project (co-located with the definitions), e.g. `Security/Auth/AdminUiPolicies.cs`:
   ```csharp
   public static class AdminUiPolicies
   {
       public const string FleetAdmin     = "FleetAdmin";      // RequireRole(Administrator)
       public const string DriverOperator = "DriverOperator";  // RequireRole(Operator, Administrator)
       public const string ConfigEditor   = "ConfigEditor";    // NEW: RequireRole(Administrator, Designer)
   }
   ```
   Keep the string *values* identical to today's literals so nothing else has to change atomically.

2. **Register the new `ConfigEditor` policy** in `AddOtOpcUaAuth` (`ServiceCollectionExtensions.cs:143`):
   ```csharp
   o.AddPolicy(AdminUiPolicies.ConfigEditor, p => p.RequireRole("Administrator", "Designer"));
   ```
   `ConfigEditor` is exactly the semantic already spelled out four times as `Roles="Administrator,Designer"` — so `Deployments`/`Scripts`/`ScriptEdit`/`ScriptAnalysisEndpoints` converge onto it too, collapsing idiom #1 into idiom #2.

3. **Apply `[Authorize(Policy = AdminUiPolicies.ConfigEditor)]`** to every *mutation-dedicated* page. Read-only dashboards keep the FallbackPolicy (bare `[Authorize]`).

   **Gate with `ConfigEditor` (mutation pages):**
   `GlobalUns`, `EquipmentPage`, `ClusterEdit`, `NodeEdit`, `NamespaceEdit`, `NewCluster`, `AclEdit`, `ClusterRedundancy`, `DriverEditRouter`, `DriverTypePicker`, all 8 driver pages (`Modbus/S7/AbCip/AbLegacy/TwinCAT/Focas/OpcUaClient/Galaxy`), `Reservations`, plus converge `Deployments`/`Scripts`/`ScriptEdit`.

   **Keep FleetAdmin:** `RoleGrants` (already), `Certificates` mutating actions (already per-action).

   **Leave read-only (FallbackPolicy):** `Home`, `Fleet`, `Hosts`, `Alerts` (view; the Ack/Shelve buttons stay imperative-`DriverOperator`), `ScriptLog`, `AlarmsHistorian`, `Account`, `ClusterOverview`, `ClustersList`, `ClusterAudit`, `Certificates` (view).

4. **Mixed list pages** (`ClustersList`, `ClusterAcls`, `ClusterNamespaces`, `ClusterDrivers`) contain both read content and destructive buttons (New/Edit/Delete). Two options:
   - **(A, recommended) Keep the page viewable, wrap destructive controls in `<AuthorizeView Policy="@AdminUiPolicies.ConfigEditor">`** so a Viewer sees the fleet but can't mutate. Preserves read access, matches least-surprise. Slightly more than one attribute per page.
   - **(B, minimal) Page-level `ConfigEditor`** — a Viewer loses read access to those lists entirely. Cheaper, but hides visibility a Viewer arguably should have.

   Recommend **(A)** for the four list pages, **page-level `ConfigEditor`** for the pure edit/create forms and driver pages. This is the correct read/write split for a global-role model.

5. **Convert the imperative/AuthorizeView literals to the constants** (`Alerts`, `DriverStatusPanel`, both pickers, `Certificates`, `RoleGrants`) — mechanical: `"DriverOperator"` → `AdminUiPolicies.DriverOperator`, etc. In razor markup use `Policy="@AdminUiPolicies.ConfigEditor"`.

**Alternatives considered / rejected:**
- *Keep role-strings everywhere* — rejected: the canonical role rename (`ConfigEditor→Designer`, `FleetAdmin→Administrator`) already shows role strings are brittle; policies decouple the gate from the role vocabulary in one place.
- *Fine-grained per-action policies (WriteTag/WriteCluster/…)* — rejected as over-engineering against the deliberate global-role posture (memory: "roles are global; simplest authz that works").
- *Move gating into the `IUnsTreeService`/EF layer* — good defense-in-depth but out of scope here and doesn't fix the surface-level page problem; note as a future hardening.

### Implementation steps

1. Add `Security/Auth/AdminUiPolicies.cs` (constants).
2. `ServiceCollectionExtensions.cs`: register `ConfigEditor`; switch the two existing `AddPolicy` names to the constants.
3. Edit page `@attribute` lines (one line each) for the mutation pages listed above → `[Authorize(Policy = AdminUiPolicies.ConfigEditor)]`. Add `@using ZB.MOM.WW.OtOpcUa.Security.Auth` to `Components/_Imports.razor` so the constant resolves in every razor file.
4. Wrap destructive controls in the four list pages with `<AuthorizeView Policy="@AdminUiPolicies.ConfigEditor">` (option A).
5. Replace imperative/AuthorizeView literals with constants (`Alerts`, `DriverStatusPanel`, pickers, `Certificates`, `RoleGrants`, `ScriptAnalysisEndpoints`).
6. Confirm `AutoLoginAuthenticationHandler` still satisfies `ConfigEditor` (it grants `DevAuthRoles.All` incl. `Administrator`+`Designer` — yes, no dev-rig regression).

### Tests + live-verify

- **Unit (Security.Tests, CI-runnable, cheap):** build a `ServiceProvider` via `AddOtOpcUaAuth`, resolve `IAuthorizationService`, and assert the `ConfigEditor` policy:
  - `ClaimsPrincipal` with role `Viewer` only → **denied**.
  - with `Designer` → allowed; with `Administrator` → allowed; with `Operator` only → denied.
  This is the only automated proof of the *policy semantics* (mirrors the existing `Configuration.Tests/AuthorizationTests.cs` style).
- **Reflection guard (AdminUI.Tests, CI-runnable) — closes the "built-but-never-wired" gap for authz:** enumerate all page component types (`typeof(App).Assembly` types with a `[Route]` attribute), and assert every type in a hard-coded `MutatingPages` set carries an `AuthorizeAttribute` whose `Policy == AdminUiPolicies.ConfigEditor` (and that no mutating page has a bare `[Authorize]`). This is the cheapest possible substitute for bUnit and catches a new page copying the wrong idiom — directly implements OVERALL cross-cutting theme #1 ("assert production wiring").
- **Live-verify (docker-dev, manual, negative path needs real login):**
  1. Positive smoke on the default rig (auto-admin): confirm the gated pages still load and mutate — deploy a config, open `/uns`, add/delete a tag, open a driver page. Auto-login grants all roles so everything must still work (regression check that the gate didn't over-block admins).
  2. **Negative path requires disabling auto-login.** Set `Security:Auth:DisableLogin=false`, bring the AdminUI up against the shared GLAuth (`10.100.0.35:3893`), bind a **Viewer** LDAP user (group→`Viewer` via `Security:Ldap:GroupToRole`), and verify: `/uns`, `/uns/equipment/*`, `/clusters/*/edit`, driver pages, `/reservations` all render the "You do not have permission" `NotAuthorized` slot (from `Routes.razor:19`); the four list pages render but hide New/Edit/Delete. Then bind a **Designer** user and confirm access is restored. Document this recipe in the PR — it is the only way to observe the deny, because the default rig can't.

**Effort:** M. **Risk/blast-radius:** Medium — touches ~25 page files but each change is one line; the real risk is *over-gating a read-only dashboard* (hiding it from Viewers) or *under-gating a mutation page*. The reflection guard + the manual Viewer pass are the mitigations. No runtime/data-plane impact (OPC UA data-plane auth is independent LDAP).

---

## 2. C-2 (Med) — ScriptAnalysis gate doc/code drift

**Restated:** `CLAUDE.md` says `/api/script-analysis/*` is "gated by the `FleetAdmin` policy"; code uses `Roles="Administrator,Designer"`.

**Verification (confirmed):** `ScriptAnalysisEndpoints.cs:17-18` `RequireAuthorization(new AuthorizeAttribute { Roles = "Administrator,Designer" })`; `CLAUDE.md:223` + design plan `…adminui-disable-login-design.md:141` claim FleetAdmin. The original monaco plan (`2026-06-09-monaco-script-editor.md:309`) *intended* `RequireAuthorization("FleetAdmin")` but the implementation diverged to match the Scripts page.

**Root cause:** implementation deliberately matched the Scripts/ScriptEdit page gate (which is `Administrator,Designer`) but the docs were written from the original plan and never reconciled.

**Design/fix:** fold into C-1. When `ScriptAnalysisEndpoints` converges onto `AdminUiPolicies.ConfigEditor` (= `Administrator,Designer`), update the doc to state the truth: **script-analysis and the Scripts/ScriptEdit/Deployments pages are gated by the `ConfigEditor` policy (Administrator or Designer)**, distinct from `FleetAdmin` (Administrator-only). This also fixes OVERALL cross-cutting theme #5 for this claim.

**Implementation:** edit `CLAUDE.md:223`; the design-plan files are historical (leave, or add a one-line "superseded" note). **Tests:** the C-1 policy unit test covers the endpoint's effective gate; add one endpoint-level assertion in `ScriptAnalysis` tests if a `WebApplicationFactory` is already in use there (check first — the suite is service-level today).

**Effort:** S. **Risk:** trivial (doc + one converged literal).

---

## 3. S-4 + S-5 (Med) — Delete concurrency + confirmation

**Restated:** Delete paths re-fetch a *fresh* RowVersion at click time (defeating optimistic concurrency → last-writer-wins), and `EquipmentPage`/`ScriptEdit` deletes fire on a single click with no confirmation.

**Verification (confirmed):**
- S-4: `GlobalUns.ConfirmDeleteAsync` (Area/Line/Equipment branches) `Load…Async` → `Delete…Async(id, loaded.RowVersion)`; `EquipmentPage.DeleteTag/DeleteVirtualTag/DeleteAlarm` (370/415/460) each comment "Load … fresh to capture its current RowVersion". *Update* paths correctly carry the modal-load RowVersion (`EquipmentPage.razor:555`), so the contract is genuinely inconsistent. Note `ScriptEdit.DeleteAsync` is the exception — it uses `_form.RowVersion` (the page-load value), so it is *not* a fresh-refetch; S-4 applies to GlobalUns + EquipmentPage only.
- S-5: `EquipmentPage.razor:172/217/263` `@onclick="() => DeleteTag(...)"` immediate; `ScriptEdit` delete button immediate. `GlobalUns` has a confirm modal (`GlobalUns.razor:71-98`); `DriverStatusPanel` has an inline Restart confirm.

**Root cause:** the list DTOs the rows render from don't carry RowVersion, so the delete handlers re-load to get *a* RowVersion — silently making delete unconditional. Confirmation was added to GlobalUns/DriverStatusPanel but not propagated to the per-equipment tables or ScriptEdit.

### Proposed design

- **S-4 — carry the rendered RowVersion:** add `RowVersion` (byte[]/base64) to the tag/vtag/alarm *list* DTOs returned by `IUnsTreeService` (they already exist for the edit path; extend the list projections). The delete handlers pass the row's RowVersion straight into `Delete…Async` — no re-load. Then a concurrent edit between render and click yields the existing `DbUpdateConcurrencyException` → surfaced as "changed by another user", matching the update contract (first-writer-wins). Same for GlobalUns Area/Line/Equipment (the tree nodes would carry RowVersion, or keep GlobalUns re-load but **document it as intentional** — the tree is a coarser surface).
  - *Alternative:* explicitly document delete as unconditional last-writer-wins. Rejected for EquipmentPage (inconsistent with its own updates); acceptable as a documented fallback for the GlobalUns tree if threading RowVersion through the tree DTO is disproportionate.
- **S-5 — shared confirm:** extract a small reusable `ConfirmButton`/`ConfirmModal` component (generalize the `GlobalUns` confirm modal) and use it for `EquipmentPage` tag/vtag/alarm deletes and `ScriptEdit` delete. Reduces duplication and gives every destructive action a consistent guard.

### Implementation steps

1. Extend list DTOs in `Uns/UnsTreeService.cs` + `IUnsTreeService.cs` (tag/vtag/alarm list projections) with `RowVersion`.
2. `EquipmentPage` delete handlers: drop the fresh-load, pass `row.RowVersion`.
3. Add `Components/Shared/ConfirmModal.razor` (extract from GlobalUns) or a `ConfirmButton`; wire into `EquipmentPage` (×3) and `ScriptEdit`.
4. Optionally thread RowVersion into the GlobalUns tree node DTO, or add a one-line "delete is unconditional here" doc-comment.

### Tests + live-verify

- **Unit (UnsTreeService.Tests — the strong seam):** list projections now include RowVersion; add a test asserting `DeleteTagAsync(id, staleRowVersion)` returns a concurrency failure (the service already has RowVersion tests to mirror).
- **Live-verify:** on docker-dev, open `/uns/equipment/{id}` Tags tab, click Delete → confirm modal appears → cancel leaves the tag → confirm deletes. For concurrency: open the equipment in two tabs, edit+save a tag in tab A, then Delete it in tab B → expect the "changed by another user" error, not a silent delete. Repeat for ScriptEdit delete-confirm.

**Effort:** M. **Risk:** Low-Medium — DTO shape change ripples to a few call sites; the confirm-modal extraction is additive.

---

## 4. S-3 (Med) — No `ErrorBoundary`

**Restated:** A single unhandled exception in any handler/render tears down the whole circuit; there's no `ErrorBoundary` anywhere in a 77-component console.

**Verification (confirmed):** `grep ErrorBoundary` = none; `MainLayout.razor:53` renders `@Body` bare inside `ThemeShell`.

**Root cause:** never added; the shared `ThemeShell` chassis doesn't provide one.

**Design:** wrap `@Body` in `MainLayout` in an `<ErrorBoundary>` with an error view + a "Reload page" / `Recover()` action; on error, log via the injected logger and show a themed panel instead of the framework's error UI. Consider a second, tighter boundary around the live-tail panels (`Alerts`, `ScriptLog`, `DriverStatusPanel`) so a bridge/render fault there doesn't blank the dashboard — but the single `MainLayout` boundary is the high-value 80%.

**Implementation:** edit `MainLayout.razor` `ChildContent` to `<ErrorBoundary Context="ex">…<ErrorContent>` + recover button; add a `_boundary.Recover()` on navigation if desired.

**Tests + live-verify:** no bUnit — live-verify by temporarily throwing in a page handler (or triggering a known-faulting path) and confirming the boundary panel + Recover works instead of a dead circuit. Leave a note that this can't be unit-tested.

**Effort:** S. **Risk:** Low (purely additive resilience).

---

## 5. S-1 + S-2 (Med) — Timer disposal + refresh error handling

**Restated:** Three components don't drain in-flight timer callbacks and `Fleet`/`AlarmsHistorian` mishandle refresh errors.

**Verification (confirmed):**
- S-1: `Fleet.razor:171` + `AlarmsHistorian.razor:90` sync `Dispose() => _timer?.Dispose()`; `DriverTestConnectButton.razor:66-81` `System.Timers.Timer` + `async (_,_) => {…}` Elapsed (effectively async-void) + sync `Dispose`. The house pattern (`System.Threading.Timer` + `IAsyncDisposable`) is used correctly in `DriverStatusPanel`/`Alerts`/`Hosts`.
- S-2: `Fleet.LoadAsync` `try/finally` no `catch` (a faulted `_ = InvokeAsync(LoadAsync)` is unobserved); `AlarmsHistorian` bare `catch {}` → permanent "Loading…".

**Root cause:** three stragglers predate the async-dispose convention; `Fleet` copied the timer-schedule but not the `Hosts` catch-log-degrade shape.

**Design:** converge all three timers onto `System.Threading.Timer` + `public async ValueTask DisposeAsync()` that awaits `_timer.DisposeAsync()` (and unsubscribes first, per the `DriverStatusPanel` template). For `DriverTestConnectButton`, replace `System.Timers.Timer`/`Elapsed` with a `System.Threading.Timer` one-shot (`Timeout.InfiniteTimeSpan` period) — kills the async-void. Give `Fleet.LoadAsync` a `catch (Exception ex) { _error = …; Log … }` (copy `Hosts.LoadConfigAsync`) and add an explicit `_error` surface. Give `AlarmsHistorian` an explicit "no historian on this node role" state instead of infinite "Loading…" (also closes U-2's admitted TODO).

**Implementation:** edit the three components; make them `@implements IAsyncDisposable`; add error fields + rendering. Reuse the exact `DriverStatusPanel.DisposeAsync` shape.

**Tests + live-verify:** live-verify on docker-dev — open `/fleet`, `/alarms-historian` on an admin-only vs driver node and confirm graceful states; navigate away rapidly during a refresh tick to confirm no disposed-component `StateHasChanged` warnings in the circuit log. Not unit-testable without bUnit.

**Effort:** S. **Risk:** Low.

---

## 6. P-1 (Med) — Debounce Monaco value push

**Restated:** `OnValueChanged(editor.getValue())` crosses the SignalR circuit on every keystroke and re-renders the parent.

**Verification (confirmed):** `monaco-init.js:205-207` fires `invokeMethodAsync("OnValueChanged", …)` inside `onDidChangeModelContent` with no debounce; diagnostics use a 500 ms `setTimeout` debounce already.

**Root cause:** the value sync was wired for immediacy; the .NET side only consumes it for Save + the inline problems panel, neither of which needs per-keystroke fidelity.

**Design:** debounce the value push ~200-300 ms (reuse the existing `diagTimer` debounce shape → add a `valueTimer`), *or* push on the diagnostic tick + on blur/save. Ensure Save reads the latest value (either via a final flush on blur/save-trigger or by having Save request `editor.getValue()` through a JS interop call synchronously). Keep the model-content event driving diagnostics as-is.

**Implementation:** edit `wwwroot/js/monaco-init.js` (`onDidChangeModelContent`), add a debounced value push + a flush path invoked before Save. Verify `ScriptEdit` + the virtual-tag modal Save both see current text.

**Tests + live-verify:** live-verify on docker-dev `/scripts/{id}` — type rapidly and confirm (network panel) `OnValueChanged` fires at debounce cadence not per keystroke; save and confirm the persisted source matches the editor. No unit coverage (JS interop).

**Effort:** S. **Risk:** Low-Medium — the one hazard is a lost final edit if the flush-before-save is missed; the flush path must be verified.

---

## 7. P-2 (Med) — Memoize ScriptAnalysis compilation

**Restated:** Each of the six endpoints re-parses + creates a fresh `CSharpCompilation` for the same text during a typing burst.

**Verification (confirmed):** `ScriptAnalysisService.Analyze` (60-67) `CSharpSyntaxTree.ParseText(full)` + `CSharpCompilation.Create(...)` per call; `Sandbox.References`/`Preamble`/`CompileOptions` are static (already cached).

**Root cause:** no `(normalized text → tree/compilation/model)` memo; a 500 ms diagnostics tick immediately followed by completions/hover/signature-help all recompile identical text.

**Design:** add a **size-1 LRU** (or a tiny `MemoryCache` keyed on the normalized source hash) inside `ScriptAnalysisService` returning the `(Tree, Compilation, Model, PreambleLength)` tuple, shared by all six endpoints. Size-1 collapses the common completions-after-diagnostics case (same text) at negligible memory. Bound it and note it's single-admin-scale; flag before multi-user. Keep it thread-safe (`lock`/`ConcurrentDictionary`) since the service is a shared registration.

**Implementation:** wrap `Analyze(userSource)` with a cache check keyed on the source (or a SHA of it) → last-value cache field. No API change; endpoints unchanged.

**Tests + live-verify:** **unit (ScriptAnalysis.Tests — a strong existing seam):** assert two successive `Analyze`/diagnose+complete calls on identical text produce identical diagnostics (correctness preserved) and that the cache returns the same compilation instance (a memoization assertion via an internal hook). Live-verify: type in the editor, confirm completions/hover latency drops and diagnostics stay correct.

**Effort:** S. **Risk:** Low — correctness risk is stale cache after an edit; keying on the full text (or hash) eliminates it.

---

## 8. C-5 (Med) — Service-seam vs EF-in-page duality

**Restated:** UNS pages go through the testable `IUnsTreeService`; `ScriptEdit`/`Scripts`/`Deployments`/`Fleet`/`Hosts` run EF + `SaveChangesAsync` directly in `@code`, untestable without a host.

**Verification (confirmed):** `ScriptEdit.razor` `DeleteAsync`/save use `DbFactory` + `db.Scripts.Remove` + `SaveChangesAsync` in-page; UNS pages use `Svc`. `IUnsTreeService.UpdateScriptSourceAsync` already exists (`IUnsTreeService.cs:502`).

**Root cause:** new pages copy the nearest file; both idioms are "blessed" by precedent.

**Design:** **declare the `IUnsTreeService` service-seam canonical for all config mutation.** Migrate `ScriptEdit`'s save+delete into the service (an `UpdateScriptSourceAsync` already exists; add `CreateScriptAsync`/`DeleteScriptAsync` if missing) so the logic gains the same RowVersion + test coverage the UNS methods have. `Fleet`/`Hosts` are *read-only* projections — leave their EF-in-page (or move to a thin read service later); the priority is the *mutating* EF-in-page (`ScriptEdit`, and the `Deployments` deploy trigger, which already has service seams). This dovetails with S-4 (list DTOs) and OVERALL theme #4 (verification).

**Implementation:** add/confirm `IUnsTreeService` script CRUD methods; rewrite `ScriptEdit` save/delete to call them; keep read pages as-is with a note. 

**Tests:** the migrated script CRUD gets unit coverage in `UnsTreeService.Tests` (delete-with-stale-RowVersion, create, update-source) — turning previously untestable `@code` into covered seam logic. Live-verify script create/edit/delete on `/scripts`.

**Effort:** M. **Risk:** Low-Medium — behavior-preserving refactor of one page; regression risk mitigated by the new seam tests.

---

## 9. Batched Lows / Underdeveloped

Group into one cleanup PR (or fold opportunistically into the above):

- **S-6 (Low):** `DriverStatusPanel.ShowOpResult` — add the `Alerts`-style stale-fire token compare so a queued chip-clear timer can't wipe a newer result. (`DisposeAsync` drain is already correct.) *Effort S.*
- **S-7 (Low):** `AdminOperationsClient.AskAsync<T>` — wrap with a linked CTS + `AskTimeout` to match the three typed methods' contract. *Effort S.*
- **P-3/P-4 (Low):** per-circuit dashboard polling + `Deployments` `SnapshotAndFlattenAsync` per render. Defer; optionally back with `IMemoryCache` keyed on the deploy event before any multi-viewer rollout. Document as accepted-at-current-scale.
- **P-5 (Low):** add `@key` to `Alerts` rows (it `Insert(0, …)` + renders without `@key`, forcing whole-table diffs). *Effort S.*
- **P-6 (Low):** stop registering the Monaco inlay-hints provider until the endpoint is non-empty (`monaco-init.js:99-117`); it round-trips for a documented no-op (`ScriptAnalysisService.InlayHints` always returns empty). *Effort S.*
- **C-6 (Low):** replace the `"GalaxyMxGateway"` literal (`TagModal.razor:311`) with the driver-type constant; dedupe the `DataTypes` array; normalize `TwinCat`/`FOCAS`/`Focas` casing. *Effort S.*
- **U-2 (stubs):** `AlarmsHistorian` role message (folded into S-2); `MonacoEditor.MarkersChanged` `object[]` — model the DTO; `GlobalUns` default-branch "not yet available" message — reword to not leak not-implemented posture. *Effort S each.*
- **U-3 (raw-JSON fallback):** in `TagModal.SaveAsync`, `JsonDocument.Parse` the fallback textarea before submit so malformed JSON is caught client-side instead of at deploy. `_form.TagConfig` is `[Required]` but never validated for well-formed JSON. *Effort S — good defensive win; add a validator unit test.*
- **U-1 / U-4:** strategic, not this-PR. **U-1:** either adopt bUnit for the modal/table state machines, or keep extracting `@code` into plain classes (`HostsDriverView`/`VirtualTagModalHelpers` precedent) — the reflection authz guard (§1) is a cheap down payment. **U-4:** targeted a11y pass on `UnsTree` expander `aria-expanded`/labels, modal focus-trap/Escape, `aria-live` on `Alerts`, table `scope`/captions — do if operator diversity matters.

**Effort:** S each; **Risk:** Low across the batch.

---

## Cross-cutting notes for the implementer

- **The docker-dev auto-login (`DisableLogin=true`) makes every negative-authz test impossible on the default rig** — it grants `DevAuthRoles.All`. Every authz claim in this plan must be proven by the policy unit test + reflection guard (CI) *and* one manual real-login Viewer pass (§1 recipe). Do not accept "it loads for me on docker-dev" as authz verification.
- **No bUnit** means S-1/S-2/S-3/S-5/P-1 are only observable by live `/run`. Prefer, wherever cheap, extracting logic into plain classes (C-5 direction) so the untested `@code` residue stays trivial — this is the repo's stated posture and OVERALL theme #4.
- **The reflection authz guard (§1) is the single highest-leverage test in this report**: it converts "which policy is on which page" from a live-only fact into a CI invariant, directly countering the "built-but-never-wired" house failure mode (OVERALL theme #1).
- Sequencing: land **C-1 + C-2 together** (shared constants/policy), then **S-4/S-5**, **S-3**, **S-1/S-2**, then the perf pair **P-1/P-2**, then **C-5**, then the Lows batch. C-1 and C-5 both touch page files — do C-1 first (attributes) to avoid churn.