FAB-380: container lifecycle ops UI (instance + cluster), safe mode, resend invite#1429
Conversation
…te handling Studio UI for the FAB-380 container operations: - Per-instance container ops in the instance ⋯ menu (Start / Start in safe mode / Restart / Restart in safe mode / Stop) under a "Container" section, gated by manage permission + instance status. New CM apiClient mutation + toast/invalidate hook. - Safe-mode badge on running instances (hidden when stopped — a UI start clears it). - Stopped/transitioning instances: stop polling get_status (ops API unreachable), show a muted availability dot and a "—" placeholder instead of a perpetual spinner. - STOPPED status badge -> red. - Cluster card stays reachable when not running: fully-stopped opens the instances page, partial opens the cluster overview. - Cluster overview StatusPill: STOPPED -> red, PARTIAL -> yellow. - Add safeMode to the Instance type (returned by the CM, not yet in the generated schema). The container calls are hand-typed past the generated SDK (the /HDBInstance/{id}/container/ {action} paths and the safeMode field aren't in the CM OpenAPI yet). Follow-up: regenerate the SDK once the CM spec exposes them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add cluster-level Start/Stop/Restart to the ClusterCard ⋯ menu, status-gated (RUNNING → Restart/Restart-safe/Stop, STOPPED → Start/Start-safe, PARTIAL → Start/Stop/Restart). Safe-mode ops force parallel; Stop shows a confirm dialog; non-safe Restart opens a rolling/parallel picker. Wired via useClusterContainerOps + clusterContainerOperation (hand-typed past the generated SDK, same as terminate). Also show a "Safe mode" pill next to the status pill on the cluster overview when every instance is in safe mode, and override Cluster.instances to the patched Instance type so safeMode is available on cluster.instances. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a "Resend invite" action to the org users table, shown only for PENDING
users. Reuses the existing invite mutation (POST /UserInvite with { email,
roleId }, targeting the user's existing role); the backend treats a repeat
invite for a pending user as a resend. Renamed tableDefinition.ts -> .tsx to
host the JSX action cell.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces container lifecycle operations (start, stop, restart, and safe mode) for both individual instances and entire clusters, alongside a new "Resend invite" button for pending organization users. The review feedback focuses on improving code robustness and UI consistency, recommending the removal of local error handlers to prevent double-toasting, guarding query invalidations against undefined parameters, adding size classes to icons, utilizing appropriate button variants, and enhancing accessibility with focus-visible ring styles on custom buttons.
|
@DavidCockerill Can you get it to rebase, and then take a look at the new nav paradigm for clusters? Surfacing things in the ... menu for clusters is great, but you can also surface them on the cluster dashboard, before you navigate into the cluster. That can aid discoverability of these features. Take a pass at the instance level features too to see if any of those can be brought out of the ... on instances, and perhaps surfaced around that instances table for better discoverability. |
|
FWIW, I tend to rebase instead of merging changes in. That will better represent what will happen when we move to bring this PR into the main branch. Plus, it gives you a clearer view of your changes being additive to the changes that have landed already. |
…afe-mode UX - Cluster overview: a "Cluster actions" dropdown (AWS "Instance state" style) with all container ops plus Terminate; ops invalid for the current status are shown but disabled so the feature set is discoverable. - Safe mode: an explain-and-confirm dialog (what it does + when to use it) that both the overview dropdown and the clusters-list card menu route through, so safe mode always explains itself instead of firing silently. - Clusters-list card: a temporary status badge for container-op states — Stopping/Starting/Restarting (amber + spinner) and Stopped/Partial — that clears on its own as the list polls; RUNNING stays clean. - Fix: plain Start/Restart now send safeMode:false so they return a cluster to normal mode. Previously they omitted safeMode and the CM preserves the current mode, so restarting an in-safe-mode cluster stayed in safe mode. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rId, a11y polish - Remove local error toasts in ResendInviteButton, useClusterContainerOps, and useInstanceContainerOps; the global MutationCache.onError already surfaces failures with the server's message (avoids double-toasting). - Guard the [clusterId] query invalidation (useParams strict:false can be undefined) so we don't invalidate [undefined]. - Cancel buttons -> defaultOutline (house style) across the container-op and safe-mode dialogs so they don't compete with the primary/destructive action. - Add focus-visible ring to the raw restart-strategy <button>s for keyboard a11y. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces cluster-wide and instance-level container lifecycle operations (stop, start, restart, and safe mode) along with confirmation dialogs, status badges, and polling suppression for stopped or transitioning instances. It also adds a "Resend invite" button for pending organization users. The review feedback suggests removing a local onError handler in ClusterStateMenu to prevent duplicate error notifications, and adding size and margin classes to several icons inside dropdown items and badges to ensure proper layout and alignment.
Gemini re-review: the terminate mutation's local onError toast duplicates the global MutationCache.onError toast. Keep only the modal close. (The 3 icon-size comments are no-ops: DropdownMenuItem already applies [&_svg]:size-4 + gap-2, and Badge applies [&>svg]:size-3.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dawsontoth
left a comment
dawsontoth
left a comment
There was a problem hiding this comment.
I like it! Let's leave it in draft until the other PRs we depend are land, though, so no one's bot gets feisty and merges it.
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
Nice feature — the container lifecycle ops (start/stop/restart across instance and cluster scope) are cleanly organized and reuse the existing polling/invalidation/toast conventions correctly, and the polling-suppression for stopped/transitioning instances (badgeStatus.tsx:22) is a genuinely nice perf cleanup on top of the feature work, not just incidental. Good shape overall.
Three things worth fixing before this ships, though — flagging as a plain comment rather than a blocking review since it's already approved and held in draft pending dependent PRs, but I'd want these addressed before/shortly after merge:
1. Safe-mode confirm dialog is cluster-only — per-instance ops skip it entirely. useInstanceMenuItems.tsx:335 fires runContainerOp('start'/'restart', { safeMode: true }) directly with no dialog, while the identical action in ClusterCard.tsx:462 routes through SafeModeConfirmDialog. This is the one place the asymmetry really matters: the dialog exists specifically because "safe mode" is jargon for a recovery action that drops all user apps/components, and that's exactly as true for a single instance as for a cluster. Clicking "Start/Restart in safe mode" from the Instances page ⋯ menu today does it with zero warning.
2. Resend invite has no client-side permission gate. tableDefinition.tsx:44 renders ResendInviteButton unconditionally for any PENDING row, but every other write affordance on this same page (OrgConfigUsersIndex) is gated on useOrganizationRolePermissions(organizationId).update. ResendInviteButton (ResendInviteButton.tsx:1) fires a real POST /UserInvite and is clickable by view-only users today — the backend presumably rejects it, but that's a defense-in-depth gap and an inconsistent affordance compared to its siblings on the same page. One-line fix: thread update into the column def or check it inside the button via the same hook.
Calling these out specifically because "safe mode" and permission gating are exactly the invariants this PR is introducing to prevent accidental/unauthorized destructive actions — so the gaps are a bit ironic relative to the feature's own goal, not a knock on the overall design.
3. Smaller one, lower urgency: the container-op hooks (useClusterContainerOps.ts:32) flip isPending back to false as soon as the "accepted" response lands, before the follow-up invalidateQueries (fired with void, unawaited) resolves — a narrow window where a fast double-click could dispatch a second conflicting op before the UI reflects the first. Same shape as the pre-existing useRestartInstanceClick pattern, so not new, but now applied to two more destructive actions (Stop, safe-mode Restart). Worth a quick confirm that central-manager/host-manager reject or queue overlapping ops server-side.
None of this is blocking given the approval already in place and the draft hold for dependent PRs — just want it in front of the team before merge. Happy to help wire up #1/#2 if useful.
— Claude (Sonnet 5), reviewed via review-queue
|
@DavidCockerill will we need version gates for these features? |
|
@dawsontoth Good instinct, but I don't think we need version gates here.
The only thing a gate would guard against is an instance older than 4.7.14 silently ignoring the env var and booting normally while the UI says "safe mode" — and we don't run anything that old. — DAIvid (Opus 4.8) |
… gate resend invite
- Per-instance Start/Restart in safe mode now route through SafeModeConfirmDialog
(generalized with a `scope` prop for cluster vs instance) instead of firing
safeMode:true with no warning — the explanation matters as much for one instance
as for a cluster.
- ResendInviteButton is hidden unless the user has org-role `update` permission,
matching the page's other write affordances (it was clickable by view-only users).
useInstanceMenuItems now returns { items, dialog } and both menu consumers render the
dialog. Kris's 3rd item (isPending flips before the unawaited invalidateQueries — a
narrow double-click window) is covered server-side: the CM admission gate rejects
overlapping ops with 409 CONFLICT.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks Kris — all three addressed in
— DAIvid (Opus 4.8) |
3 participants
Summary
Studio UI for the FAB-380 container lifecycle operations (stop / start / restart, with safe mode), plus a small resend-invite addition for pending org users. This is the front-end for the central-manager / host-manager work in HarperFast/central-manager#404 and HarperFast/host-manager#126.
Container ops are a distinct class from the existing proxied Harper "restart" — they're handled host-side via the CM, are async (the endpoint returns a transitional status and the resting state lands as the poll catches up), and support
safeMode(boot Harper core without loading user apps/components).What's included
Per-instance container ops (
useInstanceMenuItems)Containersection in the instance ⋯ menu — Start / Start in safe mode (when STOPPED), Restart / Restart in safe mode / Stop (when RUNNING). Hidden mid-transition.useInstanceContainerOpshook +instanceContainerOperationclient (POST /HDBInstance/{id}/container/{action}).Cluster-wide container ops (
ClusterCard)Containersection in the cluster ⋯ menu, gated by cluster state (RUNNING → Restart/Restart-safe/Stop, STOPPED → Start/Start-safe, PARTIAL → Start/Stop/Restart).strategy: parallel(the CM rejects safeMode + rolling). Stop shows a confirm dialog; non-safe Restart opens a rolling/parallel picker.useClusterContainerOpshook +clusterContainerOperationclient (POST /Cluster/{id}/container/{action}), andClusterContainerOpModals.Cluster overview — discoverability (
ClusterStateMenu)SafeModeConfirmDialog): "safe mode" is jargon and a recovery action, so rather than a hover-only tooltip we explain what it does at the moment of use. Shared by the start-in-safe-mode and restart-in-safe-mode entries on both the overview and the card.Stopping/Starting/Restartingbadge (with spinner), and aStoppedlabel when the cluster is stopped — so users can see what's happening without opening the cluster. Clears on its own as the 10s poll catches the resting state.safeMode: falseso a cluster that's in safe mode returns to normal mode. (Previously the UI omittedsafeMode, and the CM preserves the current mode when it's omitted — so a plain restart of a safe-mode cluster stayed in safe mode.)Safe mode indicators
Stopped-state handling
isStoppedOrTransitioninghelper; stopped/transitioning instances stopget_statuspolling (the ops API is unreachable), show a muted dot instead of a stale "Available"/endless spinner, and a "—" in the sign-in cell.STOPPEDbadge is nowdestructive(red); cluster status pill colors STOPPED red / PARTIAL yellow. Stopped cards open the instances page; partial cards open the overview.Resend invite
PENDINGusers. Reuses the existing invite mutation (POST /UserInvitewith{ email, roleId }); the CM treats a repeat invite for a pending user as a resend.Screenshots
Cluster overview — Running + Safe mode pill
Instances — Safe mode badges
Per-instance ⋯ menu — Container section
Cluster ⋯ menu — Container section
Restart strategy picker (non-safe cluster restart)
Resend invite (pending users only)
Backend dependencies / notes
/container/{action}paths andsafeMode/strategybody aren't in the CM OpenAPI yet (same pattern asterminateCluster). Follow-up: regenerate the SDK once the CM OpenAPI exposes them and drop the casts.Instance.safeModeandCluster.instances(now the patchedInstance[]) are added inapi.patch.d.tsfor the same reason.409 "User already has a pending invite"; the UI is correct and will work once that build is out.Out of scope / follow-ups
MutationCache.onError(which shows the server's message) with no local error toast, so there's no double-toasting. The same pre-existing local-onErrordouble-toast pattern lives elsewhere in the app (e.g.ClusterCardterminate, org settings/roles/users,ClusterForm) — intentionally left for a separate repo-wide cleanup rather than widened here.Testing
tsc -p tsconfig.app.json,oxlint,dprintall pass; full unit suite green (pre-commit, 1239 tests).gemini-code-assistreview passes addressed — genuine findings fixed (double-toasts,[clusterId]invalidation guard, Cancel-button variants, focus-visible a11y ring); icon-size comments verified as no-ops (DropdownMenuItem/Badgealready size their svg children).🤖 Generated with Claude Code