PMM-15384 Group the SEP apps under Management - #5840
Merged
Conversation
`refreshAccessToken()` hardcoded `POST /oauth/refresh` as the only way to obtain a token. An embedded host that owns the session — PMM — has no refresh cookie, so every recovery attempt would 401 there. `setTokenMinter()` replaces just that call; the default is unchanged, so the standalone SPA behaves exactly as before. Everything downstream is minter-agnostic already: the single-flight coalescer, the axios 401 retry, and the `setOnRefreshed` notification. Two supporting changes: The 401 retry now skips `/oauth/session*` as well as `/oauth/refresh`. Minting is single-flighted, so routing a mint's own 401 back through the retry interceptor would hand it the very promise it is running inside — an await on itself that never settles. The unauthorized handler still fires for those endpoints: a rejected exchange means "not signed in" and the auth layer needs to hear it. The openapi-fetch transport gained the 401 retry the axios one already had; it previously only reported unauthorized, so typed hooks could not recover at all. `fetch` consumes a Request's body, so the middleware stashes a clone taken before dispatch and replays that. The replay goes through raw `fetch` so it cannot re-enter the middleware and loop. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The embedded SEP UI authenticated as SEP's internal service principal: the token provider returned null and the proxy injected PMM_DEV_SEP_INTERNAL_TOKEN server-side. That principal hardcodes `is_admin = False`, so every admin-gated SEP surface answered 403. It now authenticates as the actual PMM user. `sepTokenStore` exchanges the ambient `pmm_session` cookie for a short-lived SEP bearer via `POST /api/oauth/session/exchange` (SEP-1692) and holds it in memory only — no localStorage, no sessionStorage, no query cache. It renews 30s ahead of the 5-minute expiry, and the transports' 401 retry covers the case where a throttled background tab misses that window. Concurrency is delegated to `refreshAccessToken()`, so a burst of parallel SEP requests triggers one exchange. A 401 from the exchange itself is sticky: minting is refused until the user retries, so a rejected session cannot drive an exchange loop. `SepAuthGate` triggers the first exchange when a SEP route mounts rather than at app startup — the UI has no PMM_ENABLE_SEP flag, so an eager exchange would hit SEP on every page load for every PMM user. It also closes a race the provider cannot: `setTokenProvider` is synchronous, so a plugin's first queries would otherwise fire before the exchange resolved. The dev proxy no longer injects the internal token on `/api/oauth/*`. Overwriting Authorization there would authenticate the exchange as the service principal and mask whether the cookie path works at all. Retiring the injection entirely is a follow-up. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
`onRequest` cloned every outbound Request so a 401 could be replayed, including the minting and login endpoints that `onResponse` explicitly excludes from the retry. Cloning buffers the body, and those clones were never going to be used. Both call sites now share one `isReplayEligible` predicate, so the clone and the retry cannot drift apart. Raised by Copilot on #5739. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Reworks how the store reports failure, against the updated ACs. Two
rules now shape it, and they pull in opposite directions.
Fail closed. Every exchange failure drops the bearer, so no request can
proceed on a stale, expired, or unverified credential, and there is no
cached value to fall back on. A session SEP has rejected stays sticky:
minting is refused outright until the user retries, so a rejection can
never drive an exchange loop.
Never destroy user work. The failure now lands at one of two altitudes.
Before a bearer has ever been held the page does not exist yet, so a
bootstrap failure takes the page over — there is nothing to preserve.
Once mounted the page stays mounted and the failure becomes an inline
notice beside it. Previously a background renewal being rejected moved
the phase to `signedOut`, which unmounted the plugin and threw away
whatever was half-typed into it.
The two are reconciled by keeping the bearer and the reporting separate:
`failClosed` always drops the credential, then chooses between a phase
change and a notice based on whether the page is up.
A renewal that fails for a reason that may not repeat is now retried
quietly with backoff — 2s, 4s, 8s, 16s — and only surfaces if all four
attempts fail. A 401 skips the backoff: the session is genuinely gone
and retrying would only repeat the rejection, so the user is told at
once, non-destructively, that submissions from this page will fail.
`getSepAuthStatus()` is replaced by `getSepAuthState()`, returning a
cached `{ phase, notice }` snapshot so `useSyncExternalStore` does not
re-render subscribers on a no-op. The old `error` phase is renamed
`unreachable`, matching the notice of the same name.
Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The base moved the SEP side-car behind a single `/sep` mount point (SEP_BASE_PATH, PMM-15279), so the session exchange and everything the 401 retry matches on had to move with it. Conflicts were textual, not behavioural — `client.ts` and `index.ts` auto-merged with both the minter seam and SEP_BASE_PATH intact: - `typed-client.ts` / both test files: import-block collisions, kept both sides. - `bootstrap.ts`: the base edited the old doc comment that described the token exchange as future work. This branch is that work, so its text wins, with the base's `/sep/api/...` path correction applied. - `vite.config.ts`: the base collapsed five proxied prefixes into `/sep`, forwarded unstripped. Both comment blocks kept, and `isSepAuthPath` now matches `/sep/api/oauth/` — the prefix is still on the URL when the proxy sees it, so the old `/api/oauth/` test silently stopped matching and would have let the internal token cover the exchange again. - Tests: reworked my handlers onto the base's new `API` constant, which now carries the prefix. Neither transport's mint guard needed changing: axios matches on a URL relative to `baseURL`, and the typed client's `includes('/oauth/session')` is unaffected by a prefix. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The proxy forwarded `/sep` unstripped on the grounds that SEP serves the prefix itself via `root_path`. It does not: SEP carries no root_path support at all - no flag, no setting, no `FastAPI(root_path=...)`, and none on the shipped side-car's `python -m app.sep.main`. So both ways of running it locally answer 404 to everything the proxy forwards. `python -m app.main` serves at `/api/...`, and `uvicorn --root-path /sep` prepends root_path to the path, so it sees `/sep/sep/...` instead. PMM_DEV_SEP_STRIP_PREFIX=1 strips the prefix on the way out, which makes the uvicorn form work while keeping `url_for()` links prefixed. It stays off by default: the right default belongs to the server-side nginx location, which does not exist in this repo yet. The internal-token guard has to match both the prefixed and stripped forms. Vite applies `rewrite` by mutating `req.url` before the proxyReq handler runs, so with the strip enabled the old prefix-only test stopped matching and would have injected the service-principal token onto the OAuth routes it must never cover - masking whether the session exchange works at all. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Add a "ServiceNow connection" tab to PMM Settings so an admin can enter the receiver endpoint and the delivery plan's named secrets, and have PMM write them to SEP's settings API. The operator obtains the token out of band; PMM-15218 replaces this entry surface with a guided round trip and leaves the write path below untouched. The write is one whole-object PATCH of DIAGNOSTICS_DELIVERY_INPUTS. SEP seals the key's leaves, so a per-leaf write is not a shape the UI may improvise, and the submitted secret map must match the declared names exactly. Those names are read at runtime from the baked plan (SEPSettings -> DIAGNOSTICS_DELIVERY -> value.secrets) rather than hardcoded, so an image that renames one is followed rather than 422'd. Secrets are addressed by position, not by name: react-hook-form reads a field name as a path, and a declared name carrying a "." would register as a nested field, read back undefined, and silently overwrite a stored secret with an empty string. Stored secrets come back masked and are resubmitted verbatim so SEP restores them, except where no override exists to restore from - that case is sent empty, since a mask with nothing behind it is a 422. An empty secret is a valid save and reads as "not configured", never as an error. A rejected save leaves the previous configuration standing and reports the per-field 422 verbatim; 401, 403 and an unreachable SEP each get their own message, and a raw HTTP status is never shown. The tab sits behind SepAuthGate, so the settings calls carry the bearer minted from the PMM session (PMM-15293) rather than a cookie, which the admin-gated settings router refuses. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
`connectionStatus` collapsed "no declared secrets" into `not-configured` unconditionally, so a deployment whose plan declares no credentials could save an endpoint and still be told its connection was not configured - with no way for the banner to ever say otherwise. The form offers the endpoint field in that case and accepts the save, so the status contradicted what the surface had just done. With no declared secrets there is no credential left for the deployment to supply, so a stored override is as configured as this form can make it. Absent an override it still reads as not configured. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The previous commit's comment claimed SEP carries no root_path support at all. That was true when it was written and stopped being true a day later: SEP-1794 (percona/SEP#1325) added a `SEP.ROOT_PATH` setting, passed to the `FastAPI(root_path=...)` constructor, so a SEP started with `SEP__ROOT_PATH=/sep` serves the prefix and the proxy forwards it untouched. Verified against a local SEP carrying the change: with ROOT_PATH set and nothing stripped, `/sep/api/oauth/session/exchange`, `/sep/api/sep/admin/settings/`, `/sep/api/apps/atw/config/` and `/sep/api/users/me` all resolve. Every one of them was a 404 before. Keep the flag: it still covers a SEP that predates the change or runs with ROOT_PATH unset. Reframe it as the fallback it now is, and warn against pairing it with uvicorn's `--root-path`, which prepends the prefix rather than declaring the mount - the two cancel out by accident rather than by design. Comment only; no behaviour change. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
…ssion-exchange # Conflicts: # ui/packages/sep/api/src/typed-client.ts
Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The merge of the base branch resolved typed-client.ts in favour of this branch, which reinstated isHtmlLoginResponse and the 303 clause that PMM-15216 had deleted with the SEP-1687 port. The Jinja login route that could answer an API call with a 200 HTML body is gone, so content-type sniffing can no longer mean "session expired" — under PMM it would only fire on a proxy misconfiguration and report that as a lost session. The axios transport in client.ts already took the deletion, and the tests covering the removed behaviour are gone, so this restores parity between the two transports. The token mint-and-replay path this branch adds is untouched. Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech>
…into PMM-15294-sep-diagnostics-settings Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech>
Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The settings form and the Support diagnostics setup gate ask the same question of the same settings LIST response, so the derivation moves out of the form into useServiceNowConnection. TanStack Query dedupes the request, so both surfaces share one fetch. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Everything the app can do ends in an upload to a ServiceNow case, so on an unconfigured instance a user could browse, create an incident and run a script only to find at the last step that nothing can be delivered. A setup screen now replaces the app until delivery is configured: what the tool does, a link to the settings tab that configures it, and the promise that nothing is collected without an explicit confirmation. The gate sits inside SepAuthGate, since reading the SEP settings needs the exchanged bearer. A failed settings read says nothing about the connection, so it fails open and lets the app report its own errors. SepPage wrapped its children in a plain div, which broke the flex chain from Page and left nothing below it able to centre vertically; it is now a growing flex column. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
"Collect Diagnostic Data" described the mechanism; "Support diagnostics" describes what it is for. The icon follows. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Heading follows the rename. The create action is withheld once the list request has failed — creating would hit the backend that just failed and only produce a second error the user cannot act on — and disabled while the list is still loading. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
A SEP build whose settings carry no DIAGNOSTICS_DELIVERY_INPUTS key read as "not configured", so the gate sent the operator to a settings tab that can only answer that it is unavailable. Treat a missing key like a failed read and let the app render. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The SEP auth gate named a "Smart Expert Platform" that does not exist. Rephrase the blocked and notice copy around what the user can act on - the page cannot load, their work is kept - and refer to the backend as the support platform. Also cancel the negative right margin MUI puts on an Alert's action slot, which left Try again hanging past the alert's padding. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Port SEP-1844 to PMM's embedded SEP packages. The SEP auth context moves into @sep/api, where the framework and the plugin packages can read it without depending on the host application, and exports `canMutate` — a semantic mutation capability derived from the session rather than the administrator flag read directly at each call site. A consumer rendered outside a provider resolves to a non-admin, non-mutating session, so a stray mount hides controls rather than throwing. PMM's session is the source: SepAuthProvider fills the context from `isPMMAdmin`, which is the same mapping SEP's Grafana auth provider applies to the exchanged bearer, and PMM has it loaded before a SEP route renders. Framework create, execute, stop, retry and delete controls are hidden rather than disabled, as are the equivalents in the ATW plugin. The `actions` list column is dropped when no delete handler is supplied so a read-only list has no dead column, and the snippet execution schema query is disabled for a session that cannot execute. Reads are untouched. This is a UI-only change and never a security boundary: SEP's API is unchanged and remains the only gate. PMM already restricts SEP routes to PMM admins in SepPage, so no PMM user reaches these surfaces read-only today; the gate keeps the shared packages in step with SEP and holds if that route restriction is ever relaxed. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Port SEP-1845 to PMM's embedded SEP packages. A failure that is only enqueued as a toast is invisible wherever the host mounts no snackbar provider, so `@sep/framework` gains a shared failure-reporting primitive — `ActionErrorAlert`, `useActionError` and `actionErrorMessage` — that renders the server's own reason from the failing component's own tree. Schema-driven create and edit forms get their persistent banner back for every non-422 failure, carrying the server's reason instead of returning the empty state; the 422 per-field path is unchanged. Task execute, delete, entity delete and stop-task now report through the primitive rather than a toast, and each emits exactly one failure signal. The execute confirmation closes on confirm like the adjacent delete, since a dialog left open hides the message rendered behind it; reopening the same action keeps a composed chain so a refused execute can be retried. `normalizeBlobError` recovers the reason from a `responseType: 'blob'` request, whose 403 body arrives as a Blob rather than parsed JSON — `useTaskFileDownload` now reports the refusal instead of `HTTP 403`. A mechanical guard test scans `ui/packages` for `.mutate` / `.mutateAsync` call sites and fails on any file that renders no failure and is not allowlisted with the mechanism it uses instead. PMM's own app code under `ui/apps` keeps its toast conventions and is not scanned. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
Picks up the base branch's merge of main (pnpm toolchain, the @vitejs/plugin-react alignment and the @percona/peak-ui rename) plus PMM-15294's ServiceNow submit path. Every conflict was this branch's refactor meeting the base branch's pre-refactor version, so all three resolve in favour of HEAD: - ServiceNowConnectionForm.tsx: this branch extracted the settings read into useServiceNowConnection() so sep/ServiceNowSetupGate.tsx can ask the same question of the same LIST response. Kept the hook and dropped the inline useSettingsList + declaredSecretNames/storedDeliveryInputs/ connectionStatus derivation, which the hook now owns. Diffing the result against PMM-15216 shows the extraction as the only delta, so none of PMM-15294's work is lost. - SepPage.tsx: kept the growing flex Box over the plain <div>; it passes Page's height down so the setup prompt can centre in the page. - Settings.messages.ts: both branches introduced PERCONA_SUPPORT_URL independently -- this one in lib/constants.ts (next to PMM_SERVICENOW_SETTINGS_PATH and SUPPORT_DIAGNOSTICS_DOCS_URL, since the gate under sep/ also needs it), the base in Settings.constants.ts. Neither had it at the merge base, so the union merge left two definitions of the same URL. Kept the lib/constants.ts one and removed the duplicate, leaving Settings.constants.ts with MAX_LABEL_WIDTH alone. Verified from ui/: pnpm install, make lint, make build, make test and make format-check all pass; tsc --noEmit clean for apps/pmm, whose suite is 461 passed / 13 skipped. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The gating added in the previous commit was unreachable: SepPage held every SEP route to PMM admins, and NavigationProvider only offered the entries to them, so no session ever rendered a control-free view. That guard predated per-control gating. SEP's API admits any authenticated session to its reads and holds every unsafe method to administrators (DEFAULT_MINIMUM_ROLE is ADMIN), so a read-only view was always something the server was willing to serve. The route now carries no role restriction and the sidebar entries are offered to every signed-in user; what a session may do is decided per control by `canMutate`. The ServiceNow setup prompt stays administrator-only. SEP holds `GET /sep/admin/settings` to administrators including its reads, so for a non-admin the settings query is skipped rather than fired to be refused, and the app renders. The prompt would be a dead end for them in any case: its only call to action is a settings tab they cannot open. The non-admin branch sits ahead of the loading branch, so a disabled query cannot leave a spinner that never resolves. Grouping the SEP entries under a "Management" section is a follow-up; this keeps the administrator's ordering unchanged. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
…rite-controls-non-admin # Conflicts: # ui/apps/pmm/src/pages/settings/components/servicenow/ServiceNowConnection.hooks.ts # ui/apps/pmm/src/sep/ServiceNowSetupGate.test.tsx # ui/apps/pmm/src/sep/ServiceNowSetupGate.tsx # ui/packages/plugins/atw/src/IncidentListPage.tsx
- Skip the merged execution-schema fetch in ATW's collect pane for a read-only session. The form it feeds is already withheld, so the request bought nothing; selecting snippets still works. - Drop "Create one to get started" from the incident empty state for a session that is offered no create control. Both reported by CodeRabbit on #5819. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
- Pass the failure state to a custom create-form slot. The slot bypasses SchemaFormRenderer, and this ticket removed the error toast beside it, so a caller supplying `renderCreateForm` was left with no failure signal at all. The two edit pages already threaded it. Documented the slot's obligation to render it, and corrected the type's now-stale "error snackbar" wording. - Replace the guard's file-count sanity check with a sentinel from each scanned package. A count drifts with the repo and can be satisfied by the wrong tree. Reported by CodeRabbit and Copilot on #5820. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
My earlier reply claimed the mutation guard already enforced this. It
does not: the guard is file-level, so a file that contains any accepted
marker passes even if a `<TaskHistoryTable onStopTask=...>` inside it
drops `actionError`. PluginDetailPage is exactly that shape — it holds
three ActionErrorAlert usages, so deleting the LogsTab wiring would go
unnoticed. CodeRabbit was right to push back.
`TaskHistoryTableProps` now carries a discriminated stop contract:
supplying `onStopTask` requires `actionError`, and omitting it forbids
both, since the connected variant reports from its own mutation and
would ignore them.
The internal split omits from the base interface rather than the props
union — `Omit` is not distributive and would have collapsed the two
branches, which was the other half of my objection and is avoidable.
No production call site changed: both already passed the error. Six
test call sites now say `actionError={null}` explicitly, and a
`@ts-expect-error` case pins the contract so it cannot silently relax.
Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
The two SEP apps rendered as loose top-level entries wedged between the inventory divider and the admin-only block. They now sit under a single collapsible "Management" section placed right below Inventory, so no pre-existing entry moves and the divider still opens with Inventory. The section has no page of its own: a collapsible takes its link from its first child. addSection() keeps a section from outliving its last child, since a childless collapsible renders as a shell that opens on nothing. Adds the NavigationProvider coverage the assembled tree never had, for admin, editor and viewer, including deep links into either SEP app. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
nachodd
requested review from
fabio-silva and
mattiasimonato
and
a lite review from Copilot
and removed request for
a team
August 26, 2026 23:38
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the PMM UI navigation tree to group the SEP routes (Support diagnostics and MySQL Backups) under a single collapsible Management section placed directly below the Inventory divider, preserving existing admin navigation ordering while improving sidebar structure for all signed-in roles.
Changes:
- Introduces a Management section (
NAV_MANAGEMENT) with SEP app children (NAV_SEP_ATW,NAV_SEP_MYSQL_BACKUPS) and updatesaddSepApps()to return the grouped section. - Adds a reusable
addSection()helper to avoid rendering empty collapsible shells when a section would have no children. - Adds/extends unit tests to cover the assembled navigation tree shape, ordering, and deep-link active-item identity behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ui/apps/pmm/src/contexts/navigation/navigation.utils.tsx | Replaces flat SEP entries with addSection() + grouped Management section via nav constants. |
| ui/apps/pmm/src/contexts/navigation/navigation.utils.test.ts | Adds coverage for addSepApps() grouping behavior and addSection() empty-children behavior. |
| ui/apps/pmm/src/contexts/navigation/navigation.provider.tsx | Updates inline comment to reflect grouped placement under Management. |
| ui/apps/pmm/src/contexts/navigation/navigation.provider.test.tsx | Adds new tests validating nav tree structure, ordering, and deep-link identity expansion for Management. |
| ui/apps/pmm/src/contexts/navigation/navigation.constants.ts | Adds NAV_MANAGEMENT, NAV_SEP_ATW, and NAV_SEP_MYSQL_BACKUPS definitions (icons, urls, matches). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fabio-silva
approved these changes
Aug 27, 2026
Carry the anonymous guard around addSepApps() on this branch too. PR #5819 adds it on the same line, and rewriting only the comment above a bare push would make the sync conflict on prose with the guard easy to drop while reconciling. With an identical `if` on both sides the conflict is comment-only. Covers the guard with a NavigationProvider test, so the Management section stays withheld from anonymous. Signed-off-by: Ignacio Durand <nachodurand@gmail.com>
mattiasimonato
self-requested a review
August 27, 2026 12:35
mattiasimonato
approved these changes
Aug 27, 2026
…p-sep-apps-management Signed-off-by: Ignacio Durand <nachodurand@gmail.com> # Conflicts: # ui/apps/pmm/src/contexts/navigation/navigation.provider.tsx
Base automatically changed from
PMM-15359-report-failed-ui-actions
to
PMM-15216
August 27, 2026 13:50
fabio-silva
added a commit
that referenced
this pull request
Aug 31, 2026
* PMM-15359 Report failed SEP UI actions in-tree (#5820) * PMM-15293 Add a token-minter seam to the SEP API client `refreshAccessToken()` hardcoded `POST /oauth/refresh` as the only way to obtain a token. An embedded host that owns the session — PMM — has no refresh cookie, so every recovery attempt would 401 there. `setTokenMinter()` replaces just that call; the default is unchanged, so the standalone SPA behaves exactly as before. Everything downstream is minter-agnostic already: the single-flight coalescer, the axios 401 retry, and the `setOnRefreshed` notification. Two supporting changes: The 401 retry now skips `/oauth/session*` as well as `/oauth/refresh`. Minting is single-flighted, so routing a mint's own 401 back through the retry interceptor would hand it the very promise it is running inside — an await on itself that never settles. The unauthorized handler still fires for those endpoints: a rejected exchange means "not signed in" and the auth layer needs to hear it. The openapi-fetch transport gained the 401 retry the axios one already had; it previously only reported unauthorized, so typed hooks could not recover at all. `fetch` consumes a Request's body, so the middleware stashes a clone taken before dispatch and replays that. The replay goes through raw `fetch` so it cannot re-enter the middleware and loop. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Mint the SEP bearer from the PMM session The embedded SEP UI authenticated as SEP's internal service principal: the token provider returned null and the proxy injected PMM_DEV_SEP_INTERNAL_TOKEN server-side. That principal hardcodes `is_admin = False`, so every admin-gated SEP surface answered 403. It now authenticates as the actual PMM user. `sepTokenStore` exchanges the ambient `pmm_session` cookie for a short-lived SEP bearer via `POST /api/oauth/session/exchange` (SEP-1692) and holds it in memory only — no localStorage, no sessionStorage, no query cache. It renews 30s ahead of the 5-minute expiry, and the transports' 401 retry covers the case where a throttled background tab misses that window. Concurrency is delegated to `refreshAccessToken()`, so a burst of parallel SEP requests triggers one exchange. A 401 from the exchange itself is sticky: minting is refused until the user retries, so a rejected session cannot drive an exchange loop. `SepAuthGate` triggers the first exchange when a SEP route mounts rather than at app startup — the UI has no PMM_ENABLE_SEP flag, so an eager exchange would hit SEP on every page load for every PMM user. It also closes a race the provider cannot: `setTokenProvider` is synchronous, so a plugin's first queries would otherwise fire before the exchange resolved. The dev proxy no longer injects the internal token on `/api/oauth/*`. Overwriting Authorization there would authenticate the exchange as the service principal and mask whether the cookie path works at all. Retiring the injection entirely is a follow-up. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Clone only replay-eligible requests `onRequest` cloned every outbound Request so a 401 could be replayed, including the minting and login endpoints that `onResponse` explicitly excludes from the retry. Cloning buffers the body, and those clones were never going to be used. Both call sites now share one `isReplayEligible` predicate, so the clone and the retry cannot drift apart. Raised by Copilot on #5739. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Fail closed without discarding user work Reworks how the store reports failure, against the updated ACs. Two rules now shape it, and they pull in opposite directions. Fail closed. Every exchange failure drops the bearer, so no request can proceed on a stale, expired, or unverified credential, and there is no cached value to fall back on. A session SEP has rejected stays sticky: minting is refused outright until the user retries, so a rejection can never drive an exchange loop. Never destroy user work. The failure now lands at one of two altitudes. Before a bearer has ever been held the page does not exist yet, so a bootstrap failure takes the page over — there is nothing to preserve. Once mounted the page stays mounted and the failure becomes an inline notice beside it. Previously a background renewal being rejected moved the phase to `signedOut`, which unmounted the plugin and threw away whatever was half-typed into it. The two are reconciled by keeping the bearer and the reporting separate: `failClosed` always drops the credential, then chooses between a phase change and a notice based on whether the page is up. A renewal that fails for a reason that may not repeat is now retried quietly with backoff — 2s, 4s, 8s, 16s — and only surfaces if all four attempts fail. A 401 skips the backoff: the session is genuinely gone and retrying would only repeat the rejection, so the user is told at once, non-destructively, that submissions from this page will fail. `getSepAuthStatus()` is replaced by `getSepAuthState()`, returning a cached `{ phase, notice }` snapshot so `useSyncExternalStore` does not re-render subscribers on a no-op. The old `error` phase is renamed `unreachable`, matching the notice of the same name. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Let the dev proxy strip the SEP prefix The proxy forwarded `/sep` unstripped on the grounds that SEP serves the prefix itself via `root_path`. It does not: SEP carries no root_path support at all - no flag, no setting, no `FastAPI(root_path=...)`, and none on the shipped side-car's `python -m app.sep.main`. So both ways of running it locally answer 404 to everything the proxy forwards. `python -m app.main` serves at `/api/...`, and `uvicorn --root-path /sep` prepends root_path to the path, so it sees `/sep/sep/...` instead. PMM_DEV_SEP_STRIP_PREFIX=1 strips the prefix on the way out, which makes the uvicorn form work while keeping `url_for()` links prefixed. It stays off by default: the right default belongs to the server-side nginx location, which does not exist in this repo yet. The internal-token guard has to match both the prefixed and stripped forms. Vite applies `rewrite` by mutating `req.url` before the proxyReq handler runs, so with the strip enabled the old prefix-only test stopped matching and would have injected the service-principal token onto the OAuth routes it must never cover - masking whether the session exchange works at all. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Submit ServiceNow inputs to SEP settings Add a "ServiceNow connection" tab to PMM Settings so an admin can enter the receiver endpoint and the delivery plan's named secrets, and have PMM write them to SEP's settings API. The operator obtains the token out of band; PMM-15218 replaces this entry surface with a guided round trip and leaves the write path below untouched. The write is one whole-object PATCH of DIAGNOSTICS_DELIVERY_INPUTS. SEP seals the key's leaves, so a per-leaf write is not a shape the UI may improvise, and the submitted secret map must match the declared names exactly. Those names are read at runtime from the baked plan (SEPSettings -> DIAGNOSTICS_DELIVERY -> value.secrets) rather than hardcoded, so an image that renames one is followed rather than 422'd. Secrets are addressed by position, not by name: react-hook-form reads a field name as a path, and a declared name carrying a "." would register as a nested field, read back undefined, and silently overwrite a stored secret with an empty string. Stored secrets come back masked and are resubmitted verbatim so SEP restores them, except where no override exists to restore from - that case is sent empty, since a mask with nothing behind it is a 422. An empty secret is a valid save and reads as "not configured", never as an error. A rejected save leaves the previous configuration standing and reports the per-field 422 verbatim; 401, 403 and an unreachable SEP each get their own message, and a raw HTTP status is never shown. The tab sits behind SepAuthGate, so the settings calls carry the bearer minted from the PMM session (PMM-15293) rather than a cookie, which the admin-gated settings router refuses. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Judge a secretless plan on the override `connectionStatus` collapsed "no declared secrets" into `not-configured` unconditionally, so a deployment whose plan declares no credentials could save an endpoint and still be told its connection was not configured - with no way for the banner to ever say otherwise. The form offers the endpoint field in that case and accepts the save, so the status contradicted what the surface had just done. With no declared secrets there is no credential left for the deployment to supply, so a stored override is as configured as this form can make it. Absent an override it still reads as not configured. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Point the strip flag at SEP__ROOT_PATH The previous commit's comment claimed SEP carries no root_path support at all. That was true when it was written and stopped being true a day later: SEP-1794 (percona/SEP#1325) added a `SEP.ROOT_PATH` setting, passed to the `FastAPI(root_path=...)` constructor, so a SEP started with `SEP__ROOT_PATH=/sep` serves the prefix and the proxy forwards it untouched. Verified against a local SEP carrying the change: with ROOT_PATH set and nothing stripped, `/sep/api/oauth/session/exchange`, `/sep/api/sep/admin/settings/`, `/sep/api/apps/atw/config/` and `/sep/api/users/me` all resolve. Every one of them was a 404 before. Keep the flag: it still covers a SEP that predates the change or runs with ROOT_PATH unset. Reframe it as the fallback it now is, and warn against pairing it with uvicorn's `--root-path`, which prepends the prefix rather than declaring the mount - the two cancel out by accident rather than by design. Comment only; no behaviour change. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Point ServiceNow form at the renamed peak-ui package Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Drop the SSR-era HTML and 303 handling again The merge of the base branch resolved typed-client.ts in favour of this branch, which reinstated isHtmlLoginResponse and the 303 clause that PMM-15216 had deleted with the SEP-1687 port. The Jinja login route that could answer an API call with a 200 HTML body is gone, so content-type sniffing can no longer mean "session expired" — under PMM it would only fire on a proxy misconfiguration and report that as a lost session. The axios transport in client.ts already took the deletion, and the tests covering the removed behaviour are gone, so this restores parity between the two transports. The token mint-and-replay path this branch adds is untouched. Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech> * PMM-15294 Extract Percona Support URL to a constant Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Extract the ServiceNow connection hook The settings form and the Support diagnostics setup gate ask the same question of the same settings LIST response, so the derivation moves out of the form into useServiceNowConnection. TanStack Query dedupes the request, so both surfaces share one fetch. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Gate diagnostics on ServiceNow setup Everything the app can do ends in an upload to a ServiceNow case, so on an unconfigured instance a user could browse, create an incident and run a script only to find at the last step that nothing can be delivered. A setup screen now replaces the app until delivery is configured: what the tool does, a link to the settings tab that configures it, and the promise that nothing is collected without an explicit confirmation. The gate sits inside SepAuthGate, since reading the SEP settings needs the exchanged bearer. A failed settings read says nothing about the connection, so it fails open and lets the app report its own errors. SepPage wrapped its children in a plain div, which broke the flex chain from Page and left nothing below it able to centre vertically; it is now a growing flex column. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Rename nav entry and swap its icon "Collect Diagnostic Data" described the mechanism; "Support diagnostics" describes what it is for. The icon follows. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Guard the New incident button Heading follows the rename. The create action is withheld once the list request has failed — creating would hit the backend that just failed and only produce a second error the user cannot act on — and disabled while the list is still loading. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Fail open when SEP lacks the delivery key A SEP build whose settings carry no DIAGNOSTICS_DELIVERY_INPUTS key read as "not configured", so the gate sent the operator to a settings tab that can only answer that it is unavailable. Treat a missing key like a failed read and let the app render. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Drop the invented platform name from SEP errors The SEP auth gate named a "Smart Expert Platform" that does not exist. Rephrase the blocked and notice copy around what the user can act on - the page cannot load, their work is kept - and refer to the backend as the support platform. Also cancel the negative right margin MUI puts on an Alert's action slot, which left Try again hanging past the alert's padding. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Hide SEP write controls from non-admins Port SEP-1844 to PMM's embedded SEP packages. The SEP auth context moves into @sep/api, where the framework and the plugin packages can read it without depending on the host application, and exports `canMutate` — a semantic mutation capability derived from the session rather than the administrator flag read directly at each call site. A consumer rendered outside a provider resolves to a non-admin, non-mutating session, so a stray mount hides controls rather than throwing. PMM's session is the source: SepAuthProvider fills the context from `isPMMAdmin`, which is the same mapping SEP's Grafana auth provider applies to the exchanged bearer, and PMM has it loaded before a SEP route renders. Framework create, execute, stop, retry and delete controls are hidden rather than disabled, as are the equivalents in the ATW plugin. The `actions` list column is dropped when no delete handler is supplied so a read-only list has no dead column, and the snippet execution schema query is disabled for a session that cannot execute. Reads are untouched. This is a UI-only change and never a security boundary: SEP's API is unchanged and remains the only gate. PMM already restricts SEP routes to PMM admins in SepPage, so no PMM user reaches these surfaces read-only today; the gate keeps the shared packages in step with SEP and holds if that route restriction is ever relaxed. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Report failed SEP UI actions in-tree Port SEP-1845 to PMM's embedded SEP packages. A failure that is only enqueued as a toast is invisible wherever the host mounts no snackbar provider, so `@sep/framework` gains a shared failure-reporting primitive — `ActionErrorAlert`, `useActionError` and `actionErrorMessage` — that renders the server's own reason from the failing component's own tree. Schema-driven create and edit forms get their persistent banner back for every non-422 failure, carrying the server's reason instead of returning the empty state; the 422 per-field path is unchanged. Task execute, delete, entity delete and stop-task now report through the primitive rather than a toast, and each emits exactly one failure signal. The execute confirmation closes on confirm like the adjacent delete, since a dialog left open hides the message rendered behind it; reopening the same action keeps a composed chain so a refused execute can be retried. `normalizeBlobError` recovers the reason from a `responseType: 'blob'` request, whose 403 body arrives as a Blob rather than parsed JSON — `useTaskFileDownload` now reports the refusal instead of `HTTP 403`. A mechanical guard test scans `ui/packages` for `.mutate` / `.mutateAsync` call sites and fails on any file that renders no failure and is not allowlisted with the mechanism it uses instead. PMM's own app code under `ui/apps` keeps its toast conventions and is not scanned. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Open SEP routes to non-admin sessions The gating added in the previous commit was unreachable: SepPage held every SEP route to PMM admins, and NavigationProvider only offered the entries to them, so no session ever rendered a control-free view. That guard predated per-control gating. SEP's API admits any authenticated session to its reads and holds every unsafe method to administrators (DEFAULT_MINIMUM_ROLE is ADMIN), so a read-only view was always something the server was willing to serve. The route now carries no role restriction and the sidebar entries are offered to every signed-in user; what a session may do is decided per control by `canMutate`. The ServiceNow setup prompt stays administrator-only. SEP holds `GET /sep/admin/settings` to administrators including its reads, so for a non-admin the settings query is skipped rather than fired to be refused, and the app renders. The prompt would be a dead end for them in any case: its only call to action is a settings tab they cannot open. The non-admin branch sits ahead of the loading branch, so a disabled query cannot leave a spinner that never resolves. Grouping the SEP entries under a "Management" section is a follow-up; this keeps the administrator's ordering unchanged. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Address PR review comments - Skip the merged execution-schema fetch in ATW's collect pane for a read-only session. The form it feeds is already withheld, so the request bought nothing; selecting snippets still works. - Drop "Create one to get started" from the incident empty state for a session that is offered no create control. Both reported by CodeRabbit on #5819. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Address PR review comments - Pass the failure state to a custom create-form slot. The slot bypasses SchemaFormRenderer, and this ticket removed the error toast beside it, so a caller supplying `renderCreateForm` was left with no failure signal at all. The two edit pages already threaded it. Documented the slot's obligation to render it, and corrected the type's now-stale "error snackbar" wording. - Replace the guard's file-count sanity check with a sentinel from each scanned package. A count drifts with the repo and can be satisfied by the wrong tree. Reported by CodeRabbit and Copilot on #5820. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Make the stop-failure contract a type error My earlier reply claimed the mutation guard already enforced this. It does not: the guard is file-level, so a file that contains any accepted marker passes even if a `<TaskHistoryTable onStopTask=...>` inside it drops `actionError`. PluginDetailPage is exactly that shape — it holds three ActionErrorAlert usages, so deleting the LogsTab wiring would go unnoticed. CodeRabbit was right to push back. `TaskHistoryTableProps` now carries a discriminated stop contract: supplying `onStopTask` requires `actionError`, and omitting it forbids both, since the connected variant reports from its own mutation and would ignore them. The internal split omits from the base interface rather than the props union — `Omit` is not distributive and would have collapsed the two branches, which was the other half of my objection and is avoidable. No production call site changed: both already passed the error. Six test call sites now say `actionError={null}` explicitly, and a `@ts-expect-error` case pins the contract so it cannot silently relax. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> --------- Signed-off-by: Ignacio Durand <nachodurand@gmail.com> Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: Fábio Silva <ffjs1993@gmail.com> * PMM-15384 Group the SEP apps under Management (#5840) * PMM-15293 Add a token-minter seam to the SEP API client `refreshAccessToken()` hardcoded `POST /oauth/refresh` as the only way to obtain a token. An embedded host that owns the session — PMM — has no refresh cookie, so every recovery attempt would 401 there. `setTokenMinter()` replaces just that call; the default is unchanged, so the standalone SPA behaves exactly as before. Everything downstream is minter-agnostic already: the single-flight coalescer, the axios 401 retry, and the `setOnRefreshed` notification. Two supporting changes: The 401 retry now skips `/oauth/session*` as well as `/oauth/refresh`. Minting is single-flighted, so routing a mint's own 401 back through the retry interceptor would hand it the very promise it is running inside — an await on itself that never settles. The unauthorized handler still fires for those endpoints: a rejected exchange means "not signed in" and the auth layer needs to hear it. The openapi-fetch transport gained the 401 retry the axios one already had; it previously only reported unauthorized, so typed hooks could not recover at all. `fetch` consumes a Request's body, so the middleware stashes a clone taken before dispatch and replays that. The replay goes through raw `fetch` so it cannot re-enter the middleware and loop. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Mint the SEP bearer from the PMM session The embedded SEP UI authenticated as SEP's internal service principal: the token provider returned null and the proxy injected PMM_DEV_SEP_INTERNAL_TOKEN server-side. That principal hardcodes `is_admin = False`, so every admin-gated SEP surface answered 403. It now authenticates as the actual PMM user. `sepTokenStore` exchanges the ambient `pmm_session` cookie for a short-lived SEP bearer via `POST /api/oauth/session/exchange` (SEP-1692) and holds it in memory only — no localStorage, no sessionStorage, no query cache. It renews 30s ahead of the 5-minute expiry, and the transports' 401 retry covers the case where a throttled background tab misses that window. Concurrency is delegated to `refreshAccessToken()`, so a burst of parallel SEP requests triggers one exchange. A 401 from the exchange itself is sticky: minting is refused until the user retries, so a rejected session cannot drive an exchange loop. `SepAuthGate` triggers the first exchange when a SEP route mounts rather than at app startup — the UI has no PMM_ENABLE_SEP flag, so an eager exchange would hit SEP on every page load for every PMM user. It also closes a race the provider cannot: `setTokenProvider` is synchronous, so a plugin's first queries would otherwise fire before the exchange resolved. The dev proxy no longer injects the internal token on `/api/oauth/*`. Overwriting Authorization there would authenticate the exchange as the service principal and mask whether the cookie path works at all. Retiring the injection entirely is a follow-up. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Clone only replay-eligible requests `onRequest` cloned every outbound Request so a 401 could be replayed, including the minting and login endpoints that `onResponse` explicitly excludes from the retry. Cloning buffers the body, and those clones were never going to be used. Both call sites now share one `isReplayEligible` predicate, so the clone and the retry cannot drift apart. Raised by Copilot on #5739. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Fail closed without discarding user work Reworks how the store reports failure, against the updated ACs. Two rules now shape it, and they pull in opposite directions. Fail closed. Every exchange failure drops the bearer, so no request can proceed on a stale, expired, or unverified credential, and there is no cached value to fall back on. A session SEP has rejected stays sticky: minting is refused outright until the user retries, so a rejection can never drive an exchange loop. Never destroy user work. The failure now lands at one of two altitudes. Before a bearer has ever been held the page does not exist yet, so a bootstrap failure takes the page over — there is nothing to preserve. Once mounted the page stays mounted and the failure becomes an inline notice beside it. Previously a background renewal being rejected moved the phase to `signedOut`, which unmounted the plugin and threw away whatever was half-typed into it. The two are reconciled by keeping the bearer and the reporting separate: `failClosed` always drops the credential, then chooses between a phase change and a notice based on whether the page is up. A renewal that fails for a reason that may not repeat is now retried quietly with backoff — 2s, 4s, 8s, 16s — and only surfaces if all four attempts fail. A 401 skips the backoff: the session is genuinely gone and retrying would only repeat the rejection, so the user is told at once, non-destructively, that submissions from this page will fail. `getSepAuthStatus()` is replaced by `getSepAuthState()`, returning a cached `{ phase, notice }` snapshot so `useSyncExternalStore` does not re-render subscribers on a no-op. The old `error` phase is renamed `unreachable`, matching the notice of the same name. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Let the dev proxy strip the SEP prefix The proxy forwarded `/sep` unstripped on the grounds that SEP serves the prefix itself via `root_path`. It does not: SEP carries no root_path support at all - no flag, no setting, no `FastAPI(root_path=...)`, and none on the shipped side-car's `python -m app.sep.main`. So both ways of running it locally answer 404 to everything the proxy forwards. `python -m app.main` serves at `/api/...`, and `uvicorn --root-path /sep` prepends root_path to the path, so it sees `/sep/sep/...` instead. PMM_DEV_SEP_STRIP_PREFIX=1 strips the prefix on the way out, which makes the uvicorn form work while keeping `url_for()` links prefixed. It stays off by default: the right default belongs to the server-side nginx location, which does not exist in this repo yet. The internal-token guard has to match both the prefixed and stripped forms. Vite applies `rewrite` by mutating `req.url` before the proxyReq handler runs, so with the strip enabled the old prefix-only test stopped matching and would have injected the service-principal token onto the OAuth routes it must never cover - masking whether the session exchange works at all. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Submit ServiceNow inputs to SEP settings Add a "ServiceNow connection" tab to PMM Settings so an admin can enter the receiver endpoint and the delivery plan's named secrets, and have PMM write them to SEP's settings API. The operator obtains the token out of band; PMM-15218 replaces this entry surface with a guided round trip and leaves the write path below untouched. The write is one whole-object PATCH of DIAGNOSTICS_DELIVERY_INPUTS. SEP seals the key's leaves, so a per-leaf write is not a shape the UI may improvise, and the submitted secret map must match the declared names exactly. Those names are read at runtime from the baked plan (SEPSettings -> DIAGNOSTICS_DELIVERY -> value.secrets) rather than hardcoded, so an image that renames one is followed rather than 422'd. Secrets are addressed by position, not by name: react-hook-form reads a field name as a path, and a declared name carrying a "." would register as a nested field, read back undefined, and silently overwrite a stored secret with an empty string. Stored secrets come back masked and are resubmitted verbatim so SEP restores them, except where no override exists to restore from - that case is sent empty, since a mask with nothing behind it is a 422. An empty secret is a valid save and reads as "not configured", never as an error. A rejected save leaves the previous configuration standing and reports the per-field 422 verbatim; 401, 403 and an unreachable SEP each get their own message, and a raw HTTP status is never shown. The tab sits behind SepAuthGate, so the settings calls carry the bearer minted from the PMM session (PMM-15293) rather than a cookie, which the admin-gated settings router refuses. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Judge a secretless plan on the override `connectionStatus` collapsed "no declared secrets" into `not-configured` unconditionally, so a deployment whose plan declares no credentials could save an endpoint and still be told its connection was not configured - with no way for the banner to ever say otherwise. The form offers the endpoint field in that case and accepts the save, so the status contradicted what the surface had just done. With no declared secrets there is no credential left for the deployment to supply, so a stored override is as configured as this form can make it. Absent an override it still reads as not configured. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Point the strip flag at SEP__ROOT_PATH The previous commit's comment claimed SEP carries no root_path support at all. That was true when it was written and stopped being true a day later: SEP-1794 (percona/SEP#1325) added a `SEP.ROOT_PATH` setting, passed to the `FastAPI(root_path=...)` constructor, so a SEP started with `SEP__ROOT_PATH=/sep` serves the prefix and the proxy forwards it untouched. Verified against a local SEP carrying the change: with ROOT_PATH set and nothing stripped, `/sep/api/oauth/session/exchange`, `/sep/api/sep/admin/settings/`, `/sep/api/apps/atw/config/` and `/sep/api/users/me` all resolve. Every one of them was a 404 before. Keep the flag: it still covers a SEP that predates the change or runs with ROOT_PATH unset. Reframe it as the fallback it now is, and warn against pairing it with uvicorn's `--root-path`, which prepends the prefix rather than declaring the mount - the two cancel out by accident rather than by design. Comment only; no behaviour change. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15294 Point ServiceNow form at the renamed peak-ui package Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Drop the SSR-era HTML and 303 handling again The merge of the base branch resolved typed-client.ts in favour of this branch, which reinstated isHtmlLoginResponse and the 303 clause that PMM-15216 had deleted with the SEP-1687 port. The Jinja login route that could answer an API call with a 200 HTML body is gone, so content-type sniffing can no longer mean "session expired" — under PMM it would only fire on a proxy misconfiguration and report that as a lost session. The axios transport in client.ts already took the deletion, and the tests covering the removed behaviour are gone, so this restores parity between the two transports. The token mint-and-replay path this branch adds is untouched. Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech> * PMM-15294 Extract Percona Support URL to a constant Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Extract the ServiceNow connection hook The settings form and the Support diagnostics setup gate ask the same question of the same settings LIST response, so the derivation moves out of the form into useServiceNowConnection. TanStack Query dedupes the request, so both surfaces share one fetch. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Gate diagnostics on ServiceNow setup Everything the app can do ends in an upload to a ServiceNow case, so on an unconfigured instance a user could browse, create an incident and run a script only to find at the last step that nothing can be delivered. A setup screen now replaces the app until delivery is configured: what the tool does, a link to the settings tab that configures it, and the promise that nothing is collected without an explicit confirmation. The gate sits inside SepAuthGate, since reading the SEP settings needs the exchanged bearer. A failed settings read says nothing about the connection, so it fails open and lets the app report its own errors. SepPage wrapped its children in a plain div, which broke the flex chain from Page and left nothing below it able to centre vertically; it is now a growing flex column. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Rename nav entry and swap its icon "Collect Diagnostic Data" described the mechanism; "Support diagnostics" describes what it is for. The icon follows. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Guard the New incident button Heading follows the rename. The create action is withheld once the list request has failed — creating would hit the backend that just failed and only produce a second error the user cannot act on — and disabled while the list is still loading. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15337 Fail open when SEP lacks the delivery key A SEP build whose settings carry no DIAGNOSTICS_DELIVERY_INPUTS key read as "not configured", so the gate sent the operator to a settings tab that can only answer that it is unavailable. Treat a missing key like a failed read and let the app render. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15293 Drop the invented platform name from SEP errors The SEP auth gate named a "Smart Expert Platform" that does not exist. Rephrase the blocked and notice copy around what the user can act on - the page cannot load, their work is kept - and refer to the backend as the support platform. Also cancel the negative right margin MUI puts on an Alert's action slot, which left Try again hanging past the alert's padding. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Hide SEP write controls from non-admins Port SEP-1844 to PMM's embedded SEP packages. The SEP auth context moves into @sep/api, where the framework and the plugin packages can read it without depending on the host application, and exports `canMutate` — a semantic mutation capability derived from the session rather than the administrator flag read directly at each call site. A consumer rendered outside a provider resolves to a non-admin, non-mutating session, so a stray mount hides controls rather than throwing. PMM's session is the source: SepAuthProvider fills the context from `isPMMAdmin`, which is the same mapping SEP's Grafana auth provider applies to the exchanged bearer, and PMM has it loaded before a SEP route renders. Framework create, execute, stop, retry and delete controls are hidden rather than disabled, as are the equivalents in the ATW plugin. The `actions` list column is dropped when no delete handler is supplied so a read-only list has no dead column, and the snippet execution schema query is disabled for a session that cannot execute. Reads are untouched. This is a UI-only change and never a security boundary: SEP's API is unchanged and remains the only gate. PMM already restricts SEP routes to PMM admins in SepPage, so no PMM user reaches these surfaces read-only today; the gate keeps the shared packages in step with SEP and holds if that route restriction is ever relaxed. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Report failed SEP UI actions in-tree Port SEP-1845 to PMM's embedded SEP packages. A failure that is only enqueued as a toast is invisible wherever the host mounts no snackbar provider, so `@sep/framework` gains a shared failure-reporting primitive — `ActionErrorAlert`, `useActionError` and `actionErrorMessage` — that renders the server's own reason from the failing component's own tree. Schema-driven create and edit forms get their persistent banner back for every non-422 failure, carrying the server's reason instead of returning the empty state; the 422 per-field path is unchanged. Task execute, delete, entity delete and stop-task now report through the primitive rather than a toast, and each emits exactly one failure signal. The execute confirmation closes on confirm like the adjacent delete, since a dialog left open hides the message rendered behind it; reopening the same action keeps a composed chain so a refused execute can be retried. `normalizeBlobError` recovers the reason from a `responseType: 'blob'` request, whose 403 body arrives as a Blob rather than parsed JSON — `useTaskFileDownload` now reports the refusal instead of `HTTP 403`. A mechanical guard test scans `ui/packages` for `.mutate` / `.mutateAsync` call sites and fails on any file that renders no failure and is not allowlisted with the mechanism it uses instead. PMM's own app code under `ui/apps` keeps its toast conventions and is not scanned. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Open SEP routes to non-admin sessions The gating added in the previous commit was unreachable: SepPage held every SEP route to PMM admins, and NavigationProvider only offered the entries to them, so no session ever rendered a control-free view. That guard predated per-control gating. SEP's API admits any authenticated session to its reads and holds every unsafe method to administrators (DEFAULT_MINIMUM_ROLE is ADMIN), so a read-only view was always something the server was willing to serve. The route now carries no role restriction and the sidebar entries are offered to every signed-in user; what a session may do is decided per control by `canMutate`. The ServiceNow setup prompt stays administrator-only. SEP holds `GET /sep/admin/settings` to administrators including its reads, so for a non-admin the settings query is skipped rather than fired to be refused, and the app renders. The prompt would be a dead end for them in any case: its only call to action is a settings tab they cannot open. The non-admin branch sits ahead of the loading branch, so a disabled query cannot leave a spinner that never resolves. Grouping the SEP entries under a "Management" section is a follow-up; this keeps the administrator's ordering unchanged. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15358 Address PR review comments - Skip the merged execution-schema fetch in ATW's collect pane for a read-only session. The form it feeds is already withheld, so the request bought nothing; selecting snippets still works. - Drop "Create one to get started" from the incident empty state for a session that is offered no create control. Both reported by CodeRabbit on #5819. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Address PR review comments - Pass the failure state to a custom create-form slot. The slot bypasses SchemaFormRenderer, and this ticket removed the error toast beside it, so a caller supplying `renderCreateForm` was left with no failure signal at all. The two edit pages already threaded it. Documented the slot's obligation to render it, and corrected the type's now-stale "error snackbar" wording. - Replace the guard's file-count sanity check with a sentinel from each scanned package. A count drifts with the repo and can be satisfied by the wrong tree. Reported by CodeRabbit and Copilot on #5820. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15359 Make the stop-failure contract a type error My earlier reply claimed the mutation guard already enforced this. It does not: the guard is file-level, so a file that contains any accepted marker passes even if a `<TaskHistoryTable onStopTask=...>` inside it drops `actionError`. PluginDetailPage is exactly that shape — it holds three ActionErrorAlert usages, so deleting the LogsTab wiring would go unnoticed. CodeRabbit was right to push back. `TaskHistoryTableProps` now carries a discriminated stop contract: supplying `onStopTask` requires `actionError`, and omitting it forbids both, since the connected variant reports from its own mutation and would ignore them. The internal split omits from the base interface rather than the props union — `Omit` is not distributive and would have collapsed the two branches, which was the other half of my objection and is avoidable. No production call site changed: both already passed the error. Six test call sites now say `actionError={null}` explicitly, and a `@ts-expect-error` case pins the contract so it cannot silently relax. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15384 Group the SEP apps under Management The two SEP apps rendered as loose top-level entries wedged between the inventory divider and the admin-only block. They now sit under a single collapsible "Management" section placed right below Inventory, so no pre-existing entry moves and the divider still opens with Inventory. The section has no page of its own: a collapsible takes its link from its first child. addSection() keeps a section from outliving its last child, since a childless collapsible renders as a shell that opens on nothing. Adds the NavigationProvider coverage the assembled tree never had, for admin, editor and viewer, including deep links into either SEP app. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> * PMM-15384 Address PR review comments Carry the anonymous guard around addSepApps() on this branch too. PR #5819 adds it on the same line, and rewriting only the comment above a bare push would make the sync conflict on prose with the guard easy to drop while reconciling. With an identical `if` on both sides the conflict is comment-only. Covers the guard with a NavigationProvider test, so the Management section stays withheld from anonymous. Signed-off-by: Ignacio Durand <nachodurand@gmail.com> --------- Signed-off-by: Ignacio Durand <nachodurand@gmail.com> Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: Fábio Silva <ffjs1993@gmail.com> * chore: fix vite config --------- Signed-off-by: Ignacio Durand <nachodurand@gmail.com> Signed-off-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: Ignacio Durand <nachodurand@gmail.com> Co-authored-by: yyyyyyy <contact@yyyyyyyan.tech> Co-authored-by: pmm-prbot[bot] <298421014+pmm-prbot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #5820 — base branch is
PMM-15359-report-failed-ui-actions, so review only the last commit.What
PMM-15358 opened the SEP routes to every signed-in user, which left Support diagnostics and MySQL Backups rendering as two loose top-level entries wedged between the inventory divider and the admin-only Inventory/Backups/Configuration block. That placement was deliberate — it kept the administrator's ordering untouched and deferred the grouping to this ticket.
They now sit under a single collapsible Management section (icon:
HandymanOutlined), placed right below Inventory.Why these choices
NAV_DIVIDERS.inventorystill opens the block with Inventory itself, so its name still describes what it opens. NoNAV_DIVIDERSchange.SidebarNavItemderives a collapsible's link fromchildren[0].urland ignores the parent's, soNAV_MANAGEMENTdeliberately carries nourl; the header opens whichever app leads the group.addSection()returns nothing at all when the child list is empty, rather than a collapsible that expands onto nothing. Callers spread the result, so the section can contribute zero entries if per-app filtering is ever added.Acceptance criteria
/pmm/sep/atwor/pmm/sep/mysql-backupsexpands the section and marks the correct child active. The sidebar auto-expands by object identity between the resolved active item and the section'schildren, so the test asserts identity, not just the id.cd ui && make lint && make format-check && make testandpnpm --filter ui exec tsc --noEmitare clean.Tests
navigation.provider.test.tsxis new — the assembled nav tree had no coverage at all. It covers the group's shape for admin/editor/viewer, that the SEP apps no longer appear at top level, that each child keeps itsurl/matches/icon, the ordering of the block the section joins, and deep links into both apps.navigation.utils.test.tsgainsaddSepAppsandaddSectioncoverage.Test plan
/pmm/sep/atwand to/pmm/sep/mysql-backups— the section is expanded and the right child highlighted in both cases.Notes
PMM-15359predates 15358's!user.isAnonymousguard aroundaddSepApps(), and merging 15358 here would drag ~60 unrelated files into this diff. Rather than wait for the sync, this branch now carries an identical guard (PMM-15384 Group the SEP apps under Management #5840 review), so the eventual conflict is comment-only and a test pins the guard in place.addSection()helper, and narrowed a whole-tree ordering assertion to the block this change actually touches.