diff --git a/.changeset/tidy-dashboard-forms.md b/.changeset/tidy-dashboard-forms.md new file mode 100644 index 000000000..77225b379 --- /dev/null +++ b/.changeset/tidy-dashboard-forms.md @@ -0,0 +1,7 @@ +--- +"@internal/server": patch +"@internal/node": patch +"@internal/docs": minor +--- + +Make dashboard information easier to scan with consistent labels, readable values, and wrapping for long paths and addresses. Keep conditional fields beneath their selected radio option, disable Save for unchanged drafts, and clarify warnings and validation. Apply the shared fact styling to the node dashboard as well. On the docs site, the SSH destination-confirmation guide and its screenshot now match the always-visible fingerprint. diff --git a/apps/docs/content/docs/guides/ssh.mdx b/apps/docs/content/docs/guides/ssh.mdx index 69654fe2b..2a58c6e53 100644 --- a/apps/docs/content/docs/guides/ssh.mdx +++ b/apps/docs/content/docs/guides/ssh.mdx @@ -80,11 +80,11 @@ Select **Start SSH subshell**. If you're using another node's keys and neither t A host key is the destination machine's public SSH key. It identifies that machine; your login key or password identifies you. Remembering the host key lets SSH detect a different machine answering at the same address later. You don't need to generate, paste, or manage host keys in the dashboard.
- -
A first connection using another node's keys asks before remembering an unfamiliar destination. This screenshot uses a disposable local SSH destination. Select the image to enlarge it.
+ +
A first connection using another node's keys asks before remembering an unfamiliar destination. This screenshot was captured from a sandboxed review instance with test data. Select the image to enlarge it.
-Check the address. If you need to verify the machine, expand **Verification details** and compare the fingerprint with one obtained from its administrator through a trusted channel. Choose **Trust and connect** to remember that identity and retry the connection, or **Cancel** to leave it untrusted. +Check the address. The dialog shows the destination's host fingerprint and warns that the fingerprint came from the destination itself, not an independent check. If you need to verify the machine, compare the fingerprint with one obtained from its administrator through a trusted channel. Choose **Trust and connect** to remember that identity and retry the connection, or **Cancel** to leave it untrusted. > [!warning] First-connection trust > A key retrieved from the destination isn't independent proof that you've reached the right machine. Accepting it without checking the fingerprint trusts the first response. Subshell blocks later connections if the identity changes; it doesn't silently replace the saved identity. If that happens, stop and verify the change with the destination's administrator. diff --git a/apps/docs/public/screenshots/ssh-destination-confirmation.png b/apps/docs/public/screenshots/ssh-destination-confirmation.png index 84980f9ac..3a251f02d 100644 Binary files a/apps/docs/public/screenshots/ssh-destination-confirmation.png and b/apps/docs/public/screenshots/ssh-destination-confirmation.png differ diff --git a/apps/server/web/AGENTS.md b/apps/server/web/AGENTS.md index 907aa219e..db07bd885 100644 --- a/apps/server/web/AGENTS.md +++ b/apps/server/web/AGENTS.md @@ -440,8 +440,8 @@ Changed identities never use that prompt. Settings → Presets has Agent and SSH tabs; the SSH tab manages saved connection choices, not host keys. Applying a preset clears any local key selection. Saving a local-key preset retains only the authentication choice, never credentials or approval. Recent destinations -stay in the launch suggestions; `/settings/ssh` redirects to `/presets?type=ssh`. Verification details can reveal the public fingerprint without -requiring a user to paste or manage key material. +stay in the launch suggestions; `/settings/ssh` redirects to `/presets?type=ssh`. The confirmation shows the destination's public fingerprint outright +(copyable), with no key material for the user to paste or manage. Old `/connect` links open the same subshell dialog through `/new?kind=ssh`. Failed reads must stay distinct from empty lists, with retry actions. Read `docs/launch-form-picker.md` for the diff --git a/apps/server/web/src/components/__tests__/add-node-dialog.test.tsx b/apps/server/web/src/components/__tests__/add-node-dialog.test.tsx index 6012f0564..113628f26 100644 --- a/apps/server/web/src/components/__tests__/add-node-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/add-node-dialog.test.tsx @@ -656,9 +656,9 @@ describe("AddNodeDialog", () => { // dropdown's items are in the DOM too — Base UI force-mounts them once the // trigger has focus, which is where the dialog's initial focus lands (the // old count of 2 rode the two-step never focusing the trigger — timing, not - // truth). The paste target is the CODE row, and + // truth). The paste target is the fact value, and // there is exactly one. The key shows ONCE. - expect(screen.getAllByText("https://plane.example").filter((el) => el.tagName === "CODE")).toHaveLength(1); + expect(screen.getAllByText("https://plane.example").filter((el) => el.closest("dd") !== null)).toHaveLength(1); expect(screen.getAllByText(/nsk_secret/)).toHaveLength(1); expect(screen.getByRole("button", { name: "Copy setup key" })).toBeDefined(); expect(screen.getByRole("button", { name: "Copy server address" })).toBeDefined(); @@ -716,11 +716,11 @@ describe("AddNodeDialog", () => { expect(label.compareDocumentPosition(trigger) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); expect(trigger.compareDocumentPosition(switchButton) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); - // The pick is the same value the app path's paste row carries — one CODE row + // The pick is the same value the app path's paste row carries — one fact value // (the closed dropdown holds further copies in the DOM; see the count comment // in the test above). fireEvent.click(screen.getByRole("button", { name: "Desktop App" })); - expect(screen.getAllByText("https://plane.example").filter((el) => el.tagName === "CODE")).toHaveLength(1); + expect(screen.getAllByText("https://plane.example").filter((el) => el.closest("dd") !== null)).toHaveLength(1); } finally { restore(); } diff --git a/apps/server/web/src/components/__tests__/backup-settings.test.tsx b/apps/server/web/src/components/__tests__/backup-settings.test.tsx index 0d953b88a..f02443fed 100644 --- a/apps/server/web/src/components/__tests__/backup-settings.test.tsx +++ b/apps/server/web/src/components/__tests__/backup-settings.test.tsx @@ -159,7 +159,7 @@ describe("Settings backup workflows", () => { expect(activity.at(-1)).toBe(false); }); - it("gates encryption on shared validation and keeps gold errors beside blurred fields", async () => { + it("gates encryption and distinguishes invalid values from missing input", async () => { const requests = mockApi(); mount(); fireEvent.click(screen.getByRole("switch", { name: "Encrypt the archive with a password" })); @@ -167,10 +167,18 @@ describe("Settings backup workflows", () => { expect(submit.matches(":disabled, [aria-disabled=true]")).toBe(true); const password = screen.getByLabelText("Archive password"); const confirm = screen.getByLabelText("Confirm archive password"); - for (const input of [password, confirm]) fireEvent.change(input, { target: { value: "short" } }); + // The gold half of the tone rule: blur the EMPTY box and the sentence is + // "nothing typed yet" (text-warning), not "what you typed is wrong". + fireEvent.focus(password); fireEvent.blur(password); expect(screen.getByRole("alert").textContent).toContain("at least 8"); expect(screen.getByRole("alert").className).toContain("text-warning"); + expect(screen.getByRole("alert").className).not.toContain("text-destructive"); + // The red half: a value that validation refuses. + for (const input of [password, confirm]) fireEvent.change(input, { target: { value: "short" } }); + fireEvent.blur(password); + expect(screen.getByRole("alert").textContent).toContain("at least 8"); + expect(screen.getByRole("alert").className).toContain("text-destructive"); fireEvent.focus(password); fireEvent.change(password, { target: { value: "unique archive password" } }); fireEvent.change(confirm, { target: { value: "different" } }); @@ -399,6 +407,25 @@ describe("Settings backup workflows", () => { }); }); + it("keeps the restore overrides below the mode group, not inside the selected option", async () => { + // The seven override fields apply to BOTH modes identically. They once + // rendered inside whichever mode was selected, so flipping the radio + // teleported the block (and remounted every input in it). They are a + // named section below the group now, and switching modes must not move + // the person's typing. + mockApi(); + mount(); + await configureFile(); + expect(screen.getByRole("region", { name: "Restore overrides" })).toBeTruthy(); + const port = screen.getByLabelText("Port (optional)"); + fireEvent.change(port, { target: { value: "4000" } }); + fireEvent.click(screen.getByRole("radio", { name: "This machine is different from the original" })); + expect(screen.getByLabelText("Port (optional)")).toBe(port); + expect((port as HTMLInputElement).value).toBe("4000"); + fireEvent.click(screen.getByRole("radio", { name: "Same-machine recovery" })); + expect((port as HTMLInputElement).value).toBe("4000"); + }); + it("revalidates expired extraction before preparation and stops for review if the source changed", async () => { let inspected = 0; const requests = mockApi((request) => { diff --git a/apps/server/web/src/components/__tests__/device-name-card.test.tsx b/apps/server/web/src/components/__tests__/device-name-card.test.tsx index 2d6efd008..e4369476f 100644 --- a/apps/server/web/src/components/__tests__/device-name-card.test.tsx +++ b/apps/server/web/src/components/__tests__/device-name-card.test.tsx @@ -47,4 +47,20 @@ describe("DeviceNameCard", () => { render(); expect((screen.getByLabelText("Name") as HTMLInputElement).value).toBe("Laptop"); }); + + it("disables Save when the field still holds the stored name", () => { + // The unchanged-draft rule: the button is dead at rest, lights for a real + // edit, and goes dead again when the edit is put back — a keypress is not + // a change. The stored-name path also proves the compare is against what + // was saved, not against a blank. + window.localStorage.setItem(DEVICE_NAME_KEY, "Laptop"); + render(); + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No device name changes to save."); + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Kitchen display" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Laptop" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + }); }); diff --git a/apps/server/web/src/components/__tests__/instance-name-card.test.tsx b/apps/server/web/src/components/__tests__/instance-name-card.test.tsx index ca1637d19..788f43a78 100644 --- a/apps/server/web/src/components/__tests__/instance-name-card.test.tsx +++ b/apps/server/web/src/components/__tests__/instance-name-card.test.tsx @@ -82,6 +82,22 @@ describe("InstanceNameCard", () => { await waitFor(() => expect((screen.getByLabelText("Name") as HTMLInputElement).value).toBe("theo-desktop")); }); + it("disables Save at rest and on a revert, opens it on a real change", async () => { + // The unchanged-draft rule, on the card that seeds from the server: the + // field opens showing the stored name, so Save starts dead; a genuine + // edit lights it; retyping the stored name kills it again. + stubFetch((s) => s); + renderCard("Prod plane"); + await screen.findByLabelText("Name"); + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No instance name changes to save."); + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Homelab" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Prod plane" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + }); + it("reports a failed save instead of implying it worked", async () => { const original = globalThis.fetch; restore.push(() => { diff --git a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx index 92844f32a..ba0dc176a 100644 --- a/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx +++ b/apps/server/web/src/components/__tests__/network-plugin-card.test.tsx @@ -1163,29 +1163,24 @@ describe("NetworkPluginCard: the state matrix", () => { }); it("labels each address above its value, in the QUIET data-label grammar", async () => { - // Two defects, in order, both from the same operator's live card. First the - // tag-after-URL row (`http://… MagicDNS name`) read as two disjoint - // things, because a small muted word after a big bold URL names nothing - // until you scan back — so the label moved ABOVE the value. Then the - // grammar it moved into was the wrong one: the form label's `font-strong - // text-label` put a bold "MagicDNS name" over its URL while the `Fact` - // rows above it set a quiet "Client version" over theirs, and a card - // holding both grammars reads as two systems. These are read-only facts, so - // the label carries `Fact`'s `dt` classes — quiet colour, no weight token, - // and the body size from the LIST, exactly as the `
` supplies it. + // Facts use smaller muted labels and full-size values across cards and dialogs. await renderCard(row({ state: "joined", status: { state: "joined", addresses: ADDRESSES, hints: [] } })); - const items = screen.getAllByRole("listitem"); + const items = ADDRESSES.map((address) => screen.getByText(address.label).closest("div") as HTMLElement); const first = items[0] as HTMLElement; expect(first.children[0]?.textContent).toBe("MagicDNS name"); expect(first.children[0]?.className).toContain("text-muted-foreground"); expect(first.children[0]?.className).not.toContain("font-strong"); expect(first.children[0]?.className).not.toContain("text-label"); - // The size comes from the list so it cannot drift from the `
` beside it. - expect(first.parentElement?.className).toContain("text-sm"); + expect(first.children[0]?.className).toContain("text-detail"); + expect(first.children[1]?.className).toContain("text-body"); expect(first.children[1]?.textContent).toContain("https://box.tail1234.ts.net"); - // The secure-context sentence keeps its own line below the value, and it - // is still the address's own — one per address, comparative by repetition. - expect(first.children[2]?.textContent).toBe("Passkeys and secure cookies work at this address."); + // The secure-context sentence keeps its OWN element below the value (a + // merged line would pass a plain textContent check), and it is still the + // address's own — one per address, comparative by repetition. + const sentence = first.children[1]?.querySelector("p"); + expect(sentence?.textContent).toBe("Passkeys and secure cookies work at this address."); + // The value and its copy button come first; the sentence follows them. + expect(first.children[1]?.firstElementChild).not.toBe(sentence); const second = items[1] as HTMLElement; expect(second.children[0]?.textContent).toBe("Tailscale IP"); }); @@ -1196,7 +1191,8 @@ describe("NetworkPluginCard: the state matrix", () => { // here by scheme, by secure context or by label would make the page // contradict its own data for a reason no reader can see. await renderCard(row({ state: "joined", status: { state: "joined", addresses: ADDRESSES, hints: [] } })); - const shown = screen.getAllByRole("listitem").map((item) => item.textContent ?? ""); + const list = screen.getByText(ADDRESSES[0]?.label ?? "").closest("dl"); + const shown = Array.from(list?.children ?? [], (item) => item.textContent ?? ""); expect(shown[0]).toContain("https://box.tail1234.ts.net"); expect(shown[1]).toContain("http://100.64.0.1:3080"); }); diff --git a/apps/server/web/src/components/__tests__/profile-card.test.tsx b/apps/server/web/src/components/__tests__/profile-card.test.tsx index 77e937223..cd2db1b5a 100644 --- a/apps/server/web/src/components/__tests__/profile-card.test.tsx +++ b/apps/server/web/src/components/__tests__/profile-card.test.tsx @@ -71,6 +71,28 @@ describe("ProfileCard", () => { expect(await screen.findByText("saved")).toBeDefined(); }); + it("disables Save at rest, opens it on a real edit, and re-deadens it on a revert", async () => { + // The unchanged-draft rule this PR added: Save is dead while the field + // shows the saved name, and a type-then-retype-the-same-name is no change + // either — the draft is compared to the stored value, not to "was a key + // pressed". The reason rides the disabled explanation, as on every card. + let pressed = 0; + renderCard(async () => { + pressed++; + return { error: null }; + }); + const input = (await screen.findByLabelText("Name")) as HTMLInputElement; + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No profile changes to save."); + fireEvent.change(input, { target: { value: "Thea" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + fireEvent.change(input, { target: { value: "Old Name" } }); // back to stored + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No profile changes to save."); + expect(pressed).toBe(0); + }); + it("keeps the draft and reports the error when the save fails", async () => { renderCard(async () => ({ error: { message: "Server said no" } })); const input = (await screen.findByLabelText("Name")) as HTMLInputElement; diff --git a/apps/server/web/src/components/__tests__/restart-dialog.test.tsx b/apps/server/web/src/components/__tests__/restart-dialog.test.tsx index ef688e9fd..7971a85f4 100644 --- a/apps/server/web/src/components/__tests__/restart-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/restart-dialog.test.tsx @@ -46,6 +46,6 @@ describe("RestartDialog pane safety", () => { // Absence of evidence is not evidence of safety: a definition that could // not be read gets the same refusal as one known to be lethal. open({ installed: true, paneSafety: "unknown" }); - expect(screen.getByText(/close every running subshell/)).toBeTruthy(); + expect(screen.getByText(/Restarting may close them/)).toBeTruthy(); }); }); diff --git a/apps/server/web/src/components/__tests__/sharing-dialog.test.tsx b/apps/server/web/src/components/__tests__/sharing-dialog.test.tsx index 9977bf7cf..29d2adcbe 100644 --- a/apps/server/web/src/components/__tests__/sharing-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/sharing-dialog.test.tsx @@ -59,7 +59,40 @@ describe("SharingDialog", () => { // The grant row renders (Everyone, one Remove control); the add-row // picker omits Everyone since it's already shared. expect(await screen.findByText("Everyone")).toBeDefined(); - expect(screen.getByRole("button", { name: "Remove" })).toBeDefined(); + expect(screen.getByRole("button", { name: /^Remove access for/ })).toBeDefined(); + } finally { + restore(); + } + }); + + it("disables Save until access changes and disables it again when changes are undone", async () => { + const { calls, restore } = mockFetch([{ granteeUserId: null, permission: "view" }]); + try { + renderDialog(); + await screen.findByText("Everyone"); + const save = () => screen.getByRole("button", { name: "Save" }); + const disabled = () => save().matches(":disabled, [aria-disabled=true]"); + await waitFor(() => expect(disabled()).toBe(true)); + fireEvent.click(save()); + expect(calls.some((call) => call.method === "PUT")).toBe(false); + fireEvent.click(screen.getByRole("combobox", { name: "Access for Everyone" })); + const editOption = await screen.findByRole("option", { name: "View + edit" }); + fireEvent.pointerDown(editOption); + fireEvent.pointerUp(editOption); + fireEvent.click(editOption); + await waitFor(() => expect(disabled()).toBe(false)); + fireEvent.click(screen.getByRole("combobox", { name: "Access for Everyone" })); + const viewOption = await screen.findByRole("option", { name: "View" }); + fireEvent.pointerDown(viewOption); + fireEvent.pointerUp(viewOption); + fireEvent.click(viewOption); + await waitFor(() => expect(disabled()).toBe(true)); + fireEvent.click(screen.getByRole("button", { name: "Remove access for Everyone" })); + await waitFor(() => expect(disabled()).toBe(false)); + // Re-adding the original grant also restores the saved state. + fireEvent.click(screen.getByRole("button", { name: "Add" })); + await waitFor(() => expect(disabled()).toBe(true)); + expect(calls.some((call) => call.method === "PUT")).toBe(false); } finally { restore(); } @@ -100,7 +133,12 @@ describe("SharingDialog", () => { try { renderDialog(); await screen.findByText("Everyone"); - fireEvent.click(screen.getByRole("button", { name: "Remove" })); + fireEvent.click(screen.getByRole("button", { name: /^Remove access for/ })); + // No unsaved-changes sentence (design rule: Save's disabled state carries + // that fact); the empty state names the PENDING result instead. + expect(screen.queryByText(/unsaved/i)).toBeNull(); + expect(screen.getByText("No one will have shared access after you save.")).toBeTruthy(); + expect(calls.some((call) => call.method === "PUT")).toBe(false); fireEvent.click(screen.getByRole("button", { name: "Save" })); await waitFor(() => { const put = calls.find((c) => c.method === "PUT" && c.url.endsWith("/shares")); diff --git a/apps/server/web/src/components/__tests__/supervision-dialog.test.tsx b/apps/server/web/src/components/__tests__/supervision-dialog.test.tsx index ef53dac3c..69c6e26ae 100644 --- a/apps/server/web/src/components/__tests__/supervision-dialog.test.tsx +++ b/apps/server/web/src/components/__tests__/supervision-dialog.test.tsx @@ -7,6 +7,12 @@ afterEach(cleanup); const noop = () => {}; +/** The base view with only the service's pane-safety answer changed. */ +function withPaneSafety(paneSafety: "kills" | "unknown") { + const view = deploymentView(); + return { ...view, service: { ...view.service, paneSafety } }; +} + describe("consequences", () => { it("names the platform's own agent and ends on the fact people are actually asking about", () => { const mac = consequences("app", "darwin", true); @@ -116,6 +122,58 @@ describe("SupervisionDialog", () => { expect(screen.getByText(/Stopped the server this app was running/)).toBeTruthy(); }); + it("states the lethal panes fact once: the notice carries it, the list drops it", () => { + // The filter is by the line's own text, not its position in the list, so + // a reorder of `consequences` cannot silently drop a different step while + // the notice repeats the panes one. + render( + , + ); + expect(screen.getAllByText("Every running subshell will be closed.")).toHaveLength(1); + expect(screen.getByText("Quitting the app will stop the server")).toBeTruthy(); + expect(screen.getAllByRole("listitem")).toHaveLength(3); + }); + + it("says survival is undetermined once when the definition cannot be read", () => { + render( + , + ); + expect(screen.getAllByText("Whether running subshells survive could not be determined.")).toHaveLength(1); + expect(screen.getAllByRole("listitem")).toHaveLength(3); + }); + + it("keeps every step in the list when the switch is not lethal", () => { + render( + , + ); + expect(screen.getAllByRole("listitem")).toHaveLength(4); + expect(screen.getByText("Running subshells keep running")).toBeTruthy(); + }); + it("shows the refusal where the person is", () => { render( { } }); + it("edit mode starts unchanged, so Save is dead until something actually differs", async () => { + // The unchanged-draft rule on the surface whose save WRITES provider config: + // the field opens seeded to the stored row, so Save starts dead and says + // why; a real edit lights it; putting the edit back kills it again. The + // compare runs over the wire shape (trimmed name/issuer/clientId, the + // order-insensitive domain list), not over "was a key pressed". + const { restore } = mockFetch({ appBaseUrl: "https://plane.example" }); + try { + renderDialog({ provider: view({ allowedDomains: ["example.test"] }) }); + await settle(); + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No provider changes to save."); + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Workspace Google" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + // Retyping the domain it already had (the list sorts, order is noise). + fireEvent.change(screen.getByLabelText("Name"), { target: { value: "Google" } }); + fireEvent.change(screen.getByLabelText(/Allowed e-mail domains/), { target: { value: "example.test" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + } finally { + restore(); + } + }); + it("clicking out or pressing Escape keeps the form; a half-typed issuer survives", async () => { // The loss this pins: an outside press discarded the whole dialog with a // half-entered issuer in it (the 2026-09-30 form-dialog ruling, now read diff --git a/apps/server/web/src/components/auth/provider-dialog.tsx b/apps/server/web/src/components/auth/provider-dialog.tsx index 964ab758a..4a14c5839 100644 --- a/apps/server/web/src/components/auth/provider-dialog.tsx +++ b/apps/server/web/src/components/auth/provider-dialog.tsx @@ -106,9 +106,22 @@ function ProviderForm({ provider, onDone }: { provider: ProviderAdminView | null // sends this exact slug as `id`, so the preview and the truth are one // string (§5a's "live while filling"). const displayId = editing ? provider.id : slug; + const unchanged = + provider !== null && + name.trim() === provider.name && + issuer.trim() === (provider.issuer ?? "") && + clientId.trim() === (provider.clientId ?? "") && + secret === "" && + overrideOrigin === (provider.callbackBaseUrl ?? null) && + domains + .split(",") + .map((value) => value.trim()) + .filter(Boolean) + .sort() + .join(",") === [...(provider.allowedDomains ?? [])].sort().join(","); async function save(): Promise { setSaveError(null); - if (invalidOverride) return; + if (invalidOverride || unchanged || busy) return; const shared = { name: name.trim(), issuer: issuer.trim(), @@ -161,6 +174,7 @@ function ProviderForm({ provider, onDone }: { provider: ProviderAdminView | null : null; const saveDisabled = + unchanged || busy || name.trim() === "" || issuer.trim() === "" || @@ -408,11 +422,13 @@ function ProviderForm({ provider, onDone }: { provider: ProviderAdminView | null disabledReason={ busy ? "Wait for the sign-in provider to save." - : invalidOverride - ? "Fix the provider endpoint addresses before saving." - : entries.length === 0 - ? "Add an entry point first." - : "Complete the required provider fields first." + : unchanged + ? "No provider changes to save." + : invalidOverride + ? "Fix the provider endpoint addresses before saving." + : entries.length === 0 + ? "Add an entry point first." + : "Complete the required provider fields first." } > {busy ? "Saving…" : editing ? "Save" : "Add provider"} diff --git a/apps/server/web/src/components/auth/providers-table.tsx b/apps/server/web/src/components/auth/providers-table.tsx index bd5ebb8ba..7c204ad49 100644 --- a/apps/server/web/src/components/auth/providers-table.tsx +++ b/apps/server/web/src/components/auth/providers-table.tsx @@ -3,6 +3,7 @@ import { Pencil, Settings2, Trash2 } from "lucide-react"; import { useEffect, useRef, useState } from "react"; import { type ActionItem, ActionsMenu } from "@/components/actions-menu"; import { ProviderSettingsDialog } from "@/components/auth/provider-settings-dialog"; +import { FactList } from "@/components/fact-list"; import { useDeleteAuthProvider, usePatchAuthProvider } from "@/hooks/use-auth-providers"; import { EMAIL_PROVIDER_ID, @@ -127,16 +128,14 @@ function ProviderRow({ can repeat, so the two strings that actually identify the row — the slug id and the issuer — get label-over-value rows, the values in the mono foreground voice they wear everywhere else. */} -
-
-
Slug id
-
{provider.id}
-
-
-
Issuer
-
{provider.issuer ?? ""}
-
-
+
+ +
), confirmLabel: "Remove", diff --git a/apps/server/web/src/components/backups/backup-workflow.tsx b/apps/server/web/src/components/backups/backup-workflow.tsx index 4cabf64d8..840978279 100644 --- a/apps/server/web/src/components/backups/backup-workflow.tsx +++ b/apps/server/web/src/components/backups/backup-workflow.tsx @@ -3,13 +3,13 @@ import { CardFooter, CardHeader, CardTitle, - cn, Card as WorkflowCard, CardContent as WorkflowContent, } from "@internal/node-admin"; import { CheckCircle2, LoaderCircle } from "lucide-react"; -import { Fragment, type ReactNode } from "react"; -import { CopyButton } from "@/components/copy-button"; +import type { ReactNode } from "react"; +import { FactList } from "@/components/fact-list"; +import { ErrorNotice } from "@/components/ui/notice"; import { Card, CardContent } from "./backup-card"; export function BackupWorkflow({ @@ -69,34 +69,16 @@ export function BackupProgress({ export function BackupFacts({ rows }: { rows: { label: string; value: string; copy?: boolean }[] }) { return ( -
- {rows.map(({ label, value, copy }) => ( - -
{label}
-
-
- - {value} - - {copy && } -
-
- - ))} -
+ ({ + ...row, + mono: row.copy && row.label !== "Control plane URL", + copyText: row.copy ? row.value : undefined, + }))} + /> ); } export function BackupError({ message, className }: { message: string; className?: string }) { - return ( -

- {message} -

- ); + return {message}; } diff --git a/apps/server/web/src/components/backups/create-backup-card.tsx b/apps/server/web/src/components/backups/create-backup-card.tsx index 07dda50b5..6d35c7909 100644 --- a/apps/server/web/src/components/backups/create-backup-card.tsx +++ b/apps/server/web/src/components/backups/create-backup-card.tsx @@ -10,6 +10,7 @@ import { Field, FieldGroup, FieldLabel } from "@/components/ui/field"; import { useBackupDownload } from "@/hooks/use-backups"; import { type FieldProblems, fieldError, makeForm, useSubmitDisabled, useSubmitDisabledReason } from "@/lib/form"; import { BackupError, BackupFacts, BackupProgress, BackupWorkflow } from "./backup-workflow"; +import { problemTone } from "./field-problem-tone"; type BackupDraft = { encrypted: boolean; password: string; confirmation: string }; function backupProblems(draft: BackupDraft): FieldProblems { @@ -237,7 +238,7 @@ export function CreateBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void disabled={busy} /> {problem && ( -

+

{problem}

)} @@ -273,7 +274,7 @@ export function CreateBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void disabled={busy} /> {problem && ( -

+

{problem}

)} diff --git a/apps/server/web/src/components/backups/field-problem-tone.ts b/apps/server/web/src/components/backups/field-problem-tone.ts new file mode 100644 index 000000000..fe54a7020 --- /dev/null +++ b/apps/server/web/src/components/backups/field-problem-tone.ts @@ -0,0 +1,16 @@ +import { cn } from "@internal/node-admin"; + +/** + * The tone of a backup field's validation sentence, decided the same way on + * every backup card (ruling 2026-09-30): gold means "nothing typed yet", red + * means "what you typed is wrong". + * + * The two cards once inferred the tone separately, one from the raw value and + * one from its trimmed form, so a whitespace-only field was red on Create and + * gold on Restore. A whitespace-only input is nothing typed, everywhere. + * A problem supplied from outside the form (a server refusal, a failed + * decrypt) is always the hard kind: something the person wrote was wrong. + */ +export function problemTone(value: string, suppliedProblem?: string | null): string { + return cn("text-detail", suppliedProblem || value.trim() !== "" ? "text-destructive" : "text-warning"); +} diff --git a/apps/server/web/src/components/backups/restore-backup-card.tsx b/apps/server/web/src/components/backups/restore-backup-card.tsx index 1c2d347fb..f56322f42 100644 --- a/apps/server/web/src/components/backups/restore-backup-card.tsx +++ b/apps/server/web/src/components/backups/restore-backup-card.tsx @@ -4,6 +4,7 @@ import { useStore } from "@tanstack/react-form"; import { LoaderCircle } from "lucide-react"; import { useEffect, useRef, useState } from "react"; import { Checkbox } from "@/components/ui/checkbox"; +import { ChoiceFields } from "@/components/ui/choice-fields"; import { Field, FieldGroup, FieldLabel } from "@/components/ui/field"; import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; import { Select, SelectContent, SelectGroup, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; @@ -18,6 +19,7 @@ import { type FieldProblems, fieldError, makeForm, useSubmitDisabled, useSubmitD import { setRestoreProgressActive } from "@/lib/restore-progress"; import { Card, CardContent, CardHeader, CardTitle } from "./backup-card"; import { BackupError, BackupFacts, BackupProgress, BackupWorkflow } from "./backup-workflow"; +import { problemTone } from "./field-problem-tone"; type InspectionDraft = { file: File | null; path: string; password: string; encrypted: boolean }; function inspectionProblems(draft: InspectionDraft): FieldProblems { @@ -421,7 +423,7 @@ export function RestoreBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void onChange={(event) => field.handleChange(event.target.value)} /> {problem && ( - )} @@ -690,7 +692,6 @@ export function RestoreBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void @@ -698,17 +699,54 @@ export function RestoreBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void } > {BACKUP_RESTORE_MODES.map((item) => ( - - - - {item.label} - + + +
+ + {item.label} + + {item.description} + + +
))} -

- {BACKUP_RESTORE_MODES.find((mode) => mode.value === draft.mode)?.description} -

+ {/* The seven overrides apply to BOTH modes (the request sends + them identically however the mode is set), so they are a + named section below the group rather than a panel under + whichever option happens to be selected — flipping the + radio would otherwise teleport the whole block. */} + {!inspection?.legacyDatabaseOnly && ( + +
+ + Restore overrides + +

+ Leave a field empty to restore to the location or setting recorded in the backup. +

+
+ {textField("databasePath", "Database path (optional)")} + {textField("dataDir", "Data directory (optional)")} + {textField("configDir", "Configuration directory (optional)")} + {textField("baseUrl", "Control plane URL (optional)")} + {textField("host", "Bind address (optional)")} + {textField("port", "Port (optional)")} + {textField("trustedOrigins", "Trusted origins (optional)")} + + )} {inspection?.legacyDatabaseOnly && (

This backup contains only the database. It restores into this server using its current @@ -716,17 +754,6 @@ export function RestoreBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void

)} - {!inspection?.legacyDatabaseOnly && ( - <> - {textField("databasePath", "Database path (optional)")} - {textField("dataDir", "Data directory (optional)")} - {textField("configDir", "Configuration directory (optional)")} - {textField("baseUrl", "Control plane URL (optional)")} - {textField("host", "Bind address (optional)")} - {textField("port", "Port (optional)")} - {textField("trustedOrigins", "Trusted origins (optional)")} - - )} void } > - - Restore from - { - setSource(value === "saved" ? "saved" : "file"); - void resetSelection().catch((error) => setSelectionProblem(String(error))); - }} - > - - - - Open a backup file - - - - - - Use a saved backup - - - -
{ @@ -858,168 +860,210 @@ export function RestoreBackupCard({ onBusy }: { onBusy?: (busy: boolean) => void }} className="flex flex-col gap-4" > - {source === "file" ? ( - - { - const file = event.target.files?.[0] ?? null; - if (!file) return; - const values = { file, path: "", password: "", encrypted: false }; - inspectForm.reset(values, { keepDefaultValues: true }); - void validateSelection(values); - }} - /> - - {selected.file &&

{selected.file.name}

} - - ) : ( - - - Available backups - - - - - Available backups - - - {restore.saved.isPending ? ( -

Loading saved backups…

- ) : !restore.saved.data?.backups.length && !restore.saved.error ? ( -

No saved backups are available. Open a backup file instead.

- ) : null} - {restore.saved.error && } - {selected.path && ( -

- {restore.saved.data?.backups.find((file) => file.path === selected.path)?.name} -

- )} - - - - )} - {selected.encrypted && ( - - {(field) => { - const problem = - !busy && focusedField !== "password" - ? (passwordProblem ?? (field.state.meta.isTouched ? fieldError(field.state.meta.errors) : null)) - : null; - return ( - - - Archive password + {option.label} - setFocusedField("password")} - onBlur={() => { - field.handleBlur(); - setFocusedField(null); - }} - onChange={(event) => { - field.handleChange(event.target.value); - setInspection(null); - setPasswordProblem(null); - }} - /> - {problem && ( - + {source === option.value && ( + + {source === "file" ? ( + + { + const file = event.target.files?.[0] ?? null; + if (!file) return; + const values = { file, path: "", password: "", encrypted: false }; + inspectForm.reset(values, { keepDefaultValues: true }); + void validateSelection(values); + }} + /> + + {selected.file &&

{selected.file.name}

} + + ) : ( +
+ + Available backups + + {restore.saved.isPending ? ( +

Loading saved backups…

+ ) : !restore.saved.data?.backups.length && !restore.saved.error ? ( +

+ No saved backups are available. Open a backup file instead. +

+ ) : null} + {restore.saved.error && } + {selected.path && ( +

+ {restore.saved.data?.backups.find((file) => file.path === selected.path)?.name} +

+ )} + +
+ )} + {selected.encrypted && ( + + {(field) => { + const problem = + !busy && focusedField !== "password" + ? (passwordProblem ?? + (field.state.meta.isTouched ? fieldError(field.state.meta.errors) : null)) + : null; + return ( + + + Archive password + + setFocusedField("password")} + onBlur={() => { + field.handleBlur(); + setFocusedField(null); + }} + onChange={(event) => { + field.handleChange(event.target.value); + setInspection(null); + setPasswordProblem(null); + }} + /> + {problem && ( + + )} + + ); + }} + + )} + {selectionProblem && } + {restore.inspect.isPending && ( +

+

+ )} + {inspection ? ( +

+ Backup validated. Select Configure backup to continue. +

+ ) : ( + (selected.file || selected.path) && ( + + ) + )} + {source === "file" && ( +

+ Browser uploads support archives up to 128 MB. Use the CLI or desktop assistant for larger + files. +

+ )} + )} - - ); - }} - - )} - {selectionProblem && ( -

- {selectionProblem} -

- )} - {restore.inspect.isPending && ( -

-

- )} - {inspection ? ( -

- Backup validated. Select Configure backup to continue. -

- ) : ( - (selected.file || selected.path) && ( - - ) - )} - {source === "file" && ( -

- Browser uploads support archives up to 128 MB. Use the CLI or desktop assistant for larger files. -

- )} + + + ))} + +
); diff --git a/apps/server/web/src/components/clone-subshell-dialog.tsx b/apps/server/web/src/components/clone-subshell-dialog.tsx index 93df2ec68..31e4f4300 100644 --- a/apps/server/web/src/components/clone-subshell-dialog.tsx +++ b/apps/server/web/src/components/clone-subshell-dialog.tsx @@ -1,6 +1,7 @@ import { Button, Input } from "@internal/node-admin"; import { useNavigate } from "@tanstack/react-router"; import { type JSX, useState } from "react"; +import { FactList } from "@/components/fact-list"; import { Dialog, DialogContent, @@ -115,16 +116,14 @@ export function CloneSubshellDialog({ the date and time. -
-
Agent
-
{agentName}
-
Preset
-
{presetName}
-
Node
-
{nodeLabel}
-
Working directory
-
{source.workingDir}
-
+
)} diff --git a/apps/server/web/src/components/presets/__tests__/ssh-preset-editor.test.tsx b/apps/server/web/src/components/presets/__tests__/ssh-preset-editor.test.tsx index c35b51307..ecb63be29 100644 --- a/apps/server/web/src/components/presets/__tests__/ssh-preset-editor.test.tsx +++ b/apps/server/web/src/components/presets/__tests__/ssh-preset-editor.test.tsx @@ -92,7 +92,7 @@ it("saves the local-key radio choice without requiring a file or approval", asyn expect(screen.getByRole("radio", { name: "Use a key from my machine" }).getAttribute("aria-checked")).toBe("true"); const localKeyChoice = screen.getByRole("radio", { name: "Use a key from my machine" }); expect(document.getElementById(localKeyChoice.getAttribute("aria-describedby") ?? "")?.textContent).toContain( - "Choose a key from your machine when creating a new SSH subshell", + "Choose a private key on the device you’re using now.", ); expect(screen.queryByRole("button", { name: "Choose key…" })).toBeNull(); expect(screen.queryByRole("checkbox", { name: "I trust this website and its operator" })).toBeNull(); diff --git a/apps/server/web/src/components/presets/command-paste-field.tsx b/apps/server/web/src/components/presets/command-paste-field.tsx index 619aa3e0f..05bb7829a 100644 --- a/apps/server/web/src/components/presets/command-paste-field.tsx +++ b/apps/server/web/src/components/presets/command-paste-field.tsx @@ -1,4 +1,4 @@ -import { errMessage } from "@internal/node-admin"; +import { errMessage, Fact } from "@internal/node-admin"; import { useMemo, useState } from "react"; import { Textarea } from "@/components/ui/textarea"; import { parseCommandPaste, presetFormToCommand } from "@/lib/preset-command"; @@ -116,25 +116,23 @@ export function CommandPasteField({ : `${envCount} env var${envCount === 1 ? "" : "s"} · ${flagCount} flag${flagCount === 1 ? "" : "s"}`}

{command.env.length > 0 && ( -
+
{command.env.map((row) => ( -
-
{row.key}
-
{row.value || "(empty)"}
-
+ + {row.value || "(empty)"} + ))}
)} {command.flags.length > 0 && ( -
+
{command.flags.map((row, i) => ( // Flags are positional and may legitimately repeat // (`--allow a --allow b`), so the index is the only key. // biome-ignore lint/suspicious/noArrayIndexKey: see above -
-
{row.flag}
-
{row.value}
-
+ + {row.value || "(empty)"} + ))}
)} diff --git a/apps/server/web/src/components/profile-card.tsx b/apps/server/web/src/components/profile-card.tsx index 00df53583..99700acb0 100644 --- a/apps/server/web/src/components/profile-card.tsx +++ b/apps/server/web/src/components/profile-card.tsx @@ -1,6 +1,7 @@ import { Button, Card, CardContent, CardDescription, CardHeader, CardTitle, Input, Label } from "@internal/node-admin"; import { useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; +import { FactList } from "@/components/fact-list"; import { useCurrentUser } from "@/lib/auth"; import { authClient } from "@/lib/auth-client"; import { CURRENT_USER_QUERY_KEY } from "@/lib/query-keys"; @@ -55,9 +56,11 @@ export function ProfileCard({ updateUser = (input) => authClient.updateUser(inpu const shown = nameDraft ?? user?.name ?? ""; + const unchanged = nameIsUsable(shown) === user?.name; + async function save() { const name = nameIsUsable(shown); - if (name === null) return; + if (name === null || unchanged || busy) return; setBusy(true); setError(null); setSaved(false); @@ -111,19 +114,20 @@ export function ProfileCard({ updateUser = (input) => authClient.updateUser(inpu {saved && saved} -
- - {/* Read-only by design: the email IS the credential; changing it - is not offered here (muted to say so at a glance). */} - -
+ {error &&

{error}

} )} diff --git a/apps/server/web/src/components/prompts/__tests__/prompt-form-dialog.test.tsx b/apps/server/web/src/components/prompts/__tests__/prompt-form-dialog.test.tsx index 7f8450ec4..e3046e450 100644 --- a/apps/server/web/src/components/prompts/__tests__/prompt-form-dialog.test.tsx +++ b/apps/server/web/src/components/prompts/__tests__/prompt-form-dialog.test.tsx @@ -24,11 +24,24 @@ function mockFetch() { return { posts, restore: () => (globalThis.fetch = realFetch) }; } -function renderDialog(props: { onSaved?: () => void; onClose?: () => void } = {}) { +function renderDialog( + props: { + onSaved?: () => void; + onClose?: () => void; + editingId?: string; + initial?: { description: string; body: string; shared: boolean }; + } = {}, +) { const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); render( - props.onClose?.()} onSaved={props.onSaved} /> + props.onClose?.()} + onSaved={props.onSaved} + editingId={props.editingId} + initial={props.initial} + /> , ); } @@ -89,3 +102,77 @@ describe("PromptFormDialog gating", () => { await waitFor(() => expect(saved).toBe(true)); }); }); + +describe("PromptFormDialog edit mode", () => { + let restore: (() => void) | undefined; + afterEach(() => { + restore?.(); + restore = undefined; + }); + + it("starts dead, opens on a real edit, and re-deadens when the edit is reverted", () => { + // The unchanged-draft rule on the prompt editor. Create mode cannot see + // this path at all (its fields start empty), so edit mode is pinned here: + // a dialog that opens already valid used to offer Save for a draft that + // differed from the stored row by nothing. + const realFetch = globalThis.fetch; + const puts: Record[] = []; + globalThis.fetch = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + if (url === "/api/prompts/pr1" && (init?.method ?? "GET") === "PUT") { + puts.push(JSON.parse(String(init?.body)) as Record); + return Promise.resolve(new Response(JSON.stringify({ ok: true }))); + } + return realFetch(input as never, init as never); + }) as typeof fetch; + restore = () => { + globalThis.fetch = realFetch; + }; + renderDialog({ editingId: "pr1", initial: { description: "Kickoff", body: "start the task", shared: false } }); + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(save().getAttribute("aria-description")).toBe("No changes to save."); + fireEvent.change(descriptionField(), { target: { value: "Kickoff v2" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + fireEvent.change(descriptionField(), { target: { value: "Kickoff" } }); + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(true); + expect(puts).toHaveLength(0); + }); + + it("sends the edited draft as a PUT and reports done", async () => { + // The other half of the edit-mode loop: a REAL change lights Save, and the + // press rides the wire once with the full replacement draft (PUT is a + // complete replacement of the fields, so the body must carry all three). + const realFetch = globalThis.fetch; + const puts: { url: string; body: Record }[] = []; + globalThis.fetch = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + if (url === "/api/prompts/pr1" && (init?.method ?? "GET") === "PUT") { + puts.push({ url, body: JSON.parse(String(init?.body)) as Record }); + return Promise.resolve(new Response(JSON.stringify({ ok: true }))); + } + return realFetch(input as never, init as never); + }) as typeof fetch; + let saved = false; + try { + renderDialog({ + editingId: "pr1", + initial: { description: "Kickoff", body: "start the task", shared: false }, + onSaved: () => { + saved = true; + }, + }); + fireEvent.change(bodyField(), { target: { value: "start the task now" } }); + const save = () => screen.getByRole("button", { name: "Save" }) as HTMLButtonElement; + expect(save().matches(":disabled, [aria-disabled=true]")).toBe(false); + fireEvent.click(save()); + await waitFor(() => { + expect(puts).toHaveLength(1); + expect(puts[0]?.body).toEqual({ description: "Kickoff", body: "start the task now", shared: false }); + expect(saved).toBe(true); + }); + } finally { + globalThis.fetch = realFetch; + } + }); +}); diff --git a/apps/server/web/src/components/prompts/__tests__/stack-form-dialog.test.tsx b/apps/server/web/src/components/prompts/__tests__/stack-form-dialog.test.tsx index 5e63d3ef1..5f6dfcf02 100644 --- a/apps/server/web/src/components/prompts/__tests__/stack-form-dialog.test.tsx +++ b/apps/server/web/src/components/prompts/__tests__/stack-form-dialog.test.tsx @@ -132,11 +132,13 @@ describe("StackFormDialog", () => { shared: false, }, }); + expect(saveButton(true).matches(":disabled, [aria-disabled=true]")).toBe(true); + fireEvent.change(screen.getByLabelText(/Label/), { target: { value: "Changed label" } }); fireEvent.click(saveButton(true)); await waitFor(() => expect(m.calls.length).toBe(1)); // The unlabeled inline row rides UNLABELED (the placeholder is display). expect(m.calls[0]?.body).toEqual({ - label: "Mixed", + label: "Changed label", items: [{ promptId: "p1" }, { body: "mine" }], shared: false, }); @@ -149,6 +151,8 @@ describe("StackFormDialog", () => { const m = mockFetch({ failWith: "The stack's text joined is 30000 characters" }); try { renderDialog({ editingId: "st1", initial: { label: "Big", blocks: [block({ body: "x" })], shared: false } }); + expect(saveButton(true).matches(":disabled, [aria-disabled=true]")).toBe(true); + fireEvent.change(screen.getByLabelText(/Label/), { target: { value: "Changed label" } }); fireEvent.click(saveButton(true)); const alert = await screen.findByRole("alert"); expect(alert.textContent).toContain("30000 characters"); diff --git a/apps/server/web/src/components/prompts/prompt-form-dialog.tsx b/apps/server/web/src/components/prompts/prompt-form-dialog.tsx index 1ce6a083e..8d6223a23 100644 --- a/apps/server/web/src/components/prompts/prompt-form-dialog.tsx +++ b/apps/server/web/src/components/prompts/prompt-form-dialog.tsx @@ -1,4 +1,5 @@ import { Button, errMessage, Input, Label, Switch } from "@internal/node-admin"; +import { useStore } from "@tanstack/react-form"; import { useState } from "react"; import { Dialog, @@ -45,10 +46,17 @@ export function PromptFormDialog({ const update = useUpdatePrompt(); const busy = create.isPending || update.isPending; + const unchangedDraft = (values: PromptDraft) => + !!editingId && + !!initial && + values.description.trim() === initial.description.trim() && + values.body === initial.body && + values.shared === initial.shared; const form = makeForm({ defaultValues: initial ?? { description: "", body: "", shared: false }, validator: promptDraftSchema, onSubmit: async (draft) => { + if (unchangedDraft(draft)) return; // The guard behind the gate (Enter-key paths, races). if (draft.description.trim() === "" || draft.body.trim() === "") return; setServerError(null); @@ -65,8 +73,10 @@ export function PromptFormDialog({ } }, }); - const disabled = useSubmitDisabled(form, busy); - const disabledReason = useSubmitDisabledReason(form, busy); + const unchanged = useStore(form.store, (state) => unchangedDraft(state.values)); + const disabled = useSubmitDisabled(form, busy) || unchanged; + const validationReason = useSubmitDisabledReason(form, busy); + const disabledReason = validationReason ?? (unchanged ? "No changes to save." : undefined); return ( diff --git a/apps/server/web/src/components/prompts/stack-form-dialog.tsx b/apps/server/web/src/components/prompts/stack-form-dialog.tsx index 1c78b94b3..b4942e2f2 100644 --- a/apps/server/web/src/components/prompts/stack-form-dialog.tsx +++ b/apps/server/web/src/components/prompts/stack-form-dialog.tsx @@ -1,4 +1,5 @@ import { Button, errMessage, Input, Label, Switch } from "@internal/node-admin"; +import { useStore } from "@tanstack/react-form"; import { Plus } from "lucide-react"; import { useId, useState } from "react"; import { PromptPickerBody } from "@/components/prompts/prompt-picker-body"; @@ -55,6 +56,12 @@ export function StackFormDialog({ const update = useUpdatePromptStack(); const busy = create.isPending || update.isPending; + const unchangedDraft = (values: { label: string; blocks: PromptBlock[]; shared: boolean }) => + !!editingId && + !!initial && + values.label.trim() === initial.label.trim() && + values.shared === initial.shared && + JSON.stringify(stackMembersFromBlocks(values.blocks)) === JSON.stringify(stackMembersFromBlocks(initial.blocks)); const form = makeForm({ defaultValues: { label: initial?.label ?? "", @@ -63,6 +70,7 @@ export function StackFormDialog({ }, validator: makePromptStackSchema(editingId ? 0 : 1), onSubmit: async (values) => { + if (unchangedDraft(values)) return; // The guard behind the gate (Enter-key paths, races). if (values.label.trim() === "" || (!editingId && values.blocks.length === 0)) return; setServerError(null); @@ -82,8 +90,10 @@ export function StackFormDialog({ } }, }); - const disabled = useSubmitDisabled(form, busy); - const disabledReason = useSubmitDisabledReason(form, busy); + const unchanged = useStore(form.store, (state) => unchangedDraft(state.values)); + const disabled = useSubmitDisabled(form, busy) || unchanged; + const validationReason = useSubmitDisabledReason(form, busy); + const disabledReason = validationReason ?? (unchanged ? "No changes to save." : undefined); return ( diff --git a/apps/server/web/src/components/service/restart-dialog.tsx b/apps/server/web/src/components/service/restart-dialog.tsx index 56c5297ea..48173693b 100644 --- a/apps/server/web/src/components/service/restart-dialog.tsx +++ b/apps/server/web/src/components/service/restart-dialog.tsx @@ -9,6 +9,7 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog"; +import { WarningNotice } from "@/components/ui/notice"; import type { ServerDeployment } from "@/types/server-deployment"; /** @@ -96,12 +97,19 @@ export function RestartDialog({ Restart the server? {kills - ? view.service.manager === "docker" - ? "Restarting the Docker container closes every running subshell. Open terminals reconnect after the server returns." - : `This server's service definition will close every running subshell. Rewrite it by running ${REINSTALL_COMMAND} on this machine, or restart anyway.` + ? "Open terminals reconnect after the server returns." : "Running subshells keep running; open terminals reconnect in a few seconds."} + {kills && ( + + {view.service.manager === "docker" + ? "Restarting the Docker container closes every running subshell." + : view.service.paneSafety === "kills" + ? `This server's service definition will close every running subshell. Rewrite it by running ${REINSTALL_COMMAND} on this machine, or restart anyway.` + : `We couldn’t determine whether running subshells will survive. Restarting may close them. Rewrite the service definition by running ${REINSTALL_COMMAND} on this machine, or restart anyway.`} + + )} {enables ? (

{enables}

) : ( diff --git a/apps/server/web/src/components/service/supervision-dialog.tsx b/apps/server/web/src/components/service/supervision-dialog.tsx index ad4207d4b..b84ec5fe7 100644 --- a/apps/server/web/src/components/service/supervision-dialog.tsx +++ b/apps/server/web/src/components/service/supervision-dialog.tsx @@ -10,15 +10,37 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog"; +import { WarningNotice } from "@/components/ui/notice"; import type { PaneSafety, ServerDeployment } from "@/types/server-deployment"; +/** + * The panes line, shared by {@link consequences} and the dialog. + * + * The dialog drops this line from the list when the switch is lethal, because + * the `WarningNotice` states it louder — the comparison is by the line's own + * text, never by its position in the list. + * + * **Read, not assumed.** This line said "Running subshells keep running" + * unconditionally, and on a definition predating `KillMode=process` the + * opposite was true: removing it takes every live subshell's tmux server + * with it. A dialog that promises the survival of other people's work and + * then destroys it is worse than one that says nothing. + */ +function panesConsequence(paneSafety: PaneSafety): string { + return paneSafety === "keeps" + ? "Running subshells keep running" + : paneSafety === "kills" + ? "Every running subshell will be closed" + : "Whether running subshells survive could not be determined"; +} + /** * What switching to `target` will do, in the order it happens. * * Exported so the words are testable as data, the way `recovery-model.ts` - * treats the assistant's. The two lists end on the same fact — running - * subshells survive either way — because that is the question anyone - * hesitating over this dialog is actually asking. + * treats the assistant's. The two lists end on the same fact — what becomes + * of running subshells — because that is the question anyone hesitating over + * this dialog is actually asking. */ export function consequences( target: SupervisionMode, @@ -27,17 +49,7 @@ export function consequences( paneSafety: PaneSafety = "keeps", ): string[] { const agent = platform === "darwin" ? "launchd agent" : "systemd user service"; - // **Read, not assumed.** This line said "Running subshells keep running" - // unconditionally, and on a definition predating `KillMode=process` the - // opposite was true: removing it takes every live subshell's tmux server - // with it. A dialog that promises the survival of other people's work and - // then destroys it is worse than one that says nothing. - const panes = - paneSafety === "keeps" - ? "Running subshells keep running" - : paneSafety === "kills" - ? "Every running subshell will be closed" - : "Whether running subshells survive could not be determined"; + const panes = panesConsequence(paneSafety); if (target === "app") { return [ `Removes the ${agent}`, @@ -106,6 +118,7 @@ export function SupervisionDialog({ // Only the Service→App direction removes a definition, so only that // direction can be refused on pane safety. const lethal = toApp && view.service.paneSafety !== "keeps"; + const panesLine = panesConsequence(view.service.paneSafety); return ( @@ -120,14 +133,20 @@ export function SupervisionDialog({
    {target && - consequences(target, view.platform, autostart, view.service.paneSafety).map((line) => ( -
  • - - • - - {line} -
    • - ))} + consequences(target, view.platform, autostart, view.service.paneSafety) + // When the switch is lethal the panes line moves into the + // notice below, stated louder rather than dotted among the + // steps. Matched by its own text, never by position: the line + // is shared with `consequences` so the two cannot drift. + .filter((line) => !lethal || line !== panesLine) + .map((line) => ( +
  • + + • + + {line} +
    • + ))}
{!toApp && (
@@ -136,11 +155,17 @@ export function SupervisionDialog({
)} {lethal && ( -

- {view.service.paneSafety === "kills" - ? "This machine's service definition predates the setting that spares live panes. Running `subshell-server service install` on that machine rewrites it." - : "This machine's service definition could not be read, so this is the safe assumption."} -

+ + {/* The line is list-item grammar elsewhere (no terminal punctuation); + this is the one place it is read as prose, so it gets its period + here rather than in every bullet. */} +

{panesLine}.

+

+ {view.service.paneSafety === "kills" + ? "This machine's service definition predates the setting that spares live panes. Running `subshell-server service install` on that machine rewrites it." + : "This machine's service definition could not be read. Switching may close running subshells."} +

+ )}

This page will lose its connection for a few seconds while the server comes back. diff --git a/apps/server/web/src/components/sharing-dialog-core.tsx b/apps/server/web/src/components/sharing-dialog-core.tsx index 32c5f7196..21462ab59 100644 --- a/apps/server/web/src/components/sharing-dialog-core.tsx +++ b/apps/server/web/src/components/sharing-dialog-core.tsx @@ -9,6 +9,7 @@ import { DialogHeader, DialogTitle, } from "@/components/ui/dialog"; +import { ErrorNotice, WarningNotice } from "@/components/ui/notice"; import { Select, SelectContent, SelectGroup, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import type { RosterUser, ShareDraft } from "@/hooks/use-subshell-shares"; import type { SubshellAccess } from "@/types/subshell"; @@ -116,6 +117,12 @@ export function SharingDialogCore({ const pickerId = useId(); const grants = draft ?? serverGrants; + // Grant order isn't meaningful: removing and re-adding the same access is unchanged. + const savedPermissions = new Map(serverGrants.map((grant) => [grant.granteeUserId, grant.permission])); + const hasChanges = + grants.length !== serverGrants.length || + grants.some((grant) => savedPermissions.get(grant.granteeUserId) !== grant.permission); + const saveDisabled = saving || isLoading || !hasChanges; const alreadyShared = new Set(grants.map((g) => g.granteeUserId ?? EVERYONE)); const recipientOptions = [ @@ -149,6 +156,7 @@ export function SharingDialogCore({ } async function save() { + if (!canManage || saveDisabled) return; setError(null); try { await onSave(grants); @@ -168,23 +176,26 @@ export function SharingDialogCore({ {/* Before the grant rows, not after: the disclosure has to be read while deciding, not discovered under the Save button. */} - {warning != null && ( -

- {warning} -

- )} + {warning != null && {warning}}
+ {canManage && ( +

+ Save applies all additions, removals, and access changes. Cancel discards them. +

+ )} {isLoading &&

Loading…

} {!isLoading && grants.length === 0 && ( -

Not shared with anyone yet.

+

+ {hasChanges ? "No one will have shared access after you save." : "Not shared with anyone yet."} +

)} {grants.map((g, i) => (
- + {g.granteeUserId === null ? "Everyone" : (roster.find((u) => u.id === g.granteeUserId)?.email ?? g.granteeUserId)} @@ -196,7 +207,13 @@ export function SharingDialogCore({ value={g.permission} onChange={(level) => changeLevel(i, level)} /> - @@ -237,11 +254,7 @@ export function SharingDialogCore({
)} - {error && ( -

- {error} -

- )} + {error && {error}}
@@ -251,9 +264,13 @@ export function SharingDialogCore({ {canManage && (
diff --git a/docs/README.md b/docs/README.md index 77cf9a5a7..f88970d05 100644 --- a/docs/README.md +++ b/docs/README.md @@ -12,6 +12,7 @@ notes should link to them and explain only the additional engineering details. - [Security follow-ups](security-actionable.md): unresolved work, decision triggers, and regression history. - [Release and CI](release-and-ci.md): build, signing, publishing, updates, and deployment mechanics. - [Design system](design-system.md): product tokens, dashboard layouts, responsive tables, interaction patterns, and implementation constraints. +- [Dashboard UX review](ux-review-2026-10-17.md): October 2026 findings, corrections, verification scope, and remaining coverage limits. - [Documentation authoring](../apps/docs/AGENTS.md) and [style guide](../apps/docs/STYLE.md): public site structure, language, visuals, and verification. - [Proxmox submission research](proxmox-community-submission.md): external acceptance criteria and future integration work. diff --git a/docs/design-system.md b/docs/design-system.md index 5af48e115..f57f40f85 100644 --- a/docs/design-system.md +++ b/docs/design-system.md @@ -252,10 +252,10 @@ admitted here. - **Line item**: `label` over `detail`, differing by weight AND colour, never by tone alone. *(15px regular over 13px muted read as one paragraph.)* - **Quiet label vs control label**: a read-only data row labels over its value - with the label in `muted-foreground` at `body` size, the `Fact`/`dt` grammar, - and it takes that size from the list or `
` around it rather than from the - label itself. `font-strong` label weight belongs to a control's label and to a - section heading, ONLY. *(A networking card set bold "NetBird FQDN" over its URL + with the label in `muted-foreground` at `detail` size and the value at `body` + size in `foreground`, using the shared `Fact` component. Monospace values keep + the same body size. Size and colour together distinguish labels from values. `font-strong` + label weight belongs to a control's label and to a section heading, ONLY. *(A networking card set bold "NetBird FQDN" over its URL six pixels above a quiet "Client version" over its: two label grammars in one card, so one row read as a heading and the other as data, and neither was the control the bold claimed it was.)* @@ -364,10 +364,37 @@ admitted here. label. Use `aria-labelledby` for the title and `aria-describedby` for the description so assistive technology announces their separate roles. This applies to native radios and shared radio components across web and desktop surfaces. +- **Dependent forms**: keep a selected radio option's fields directly beneath + that option, inside a bordered, inset panel (`ChoiceFields` in the SPA). + Keep the panel outside the clickable label, associate it with the option's + title, and keep field labels visible. The boundary distinguishes the subform + from option descriptions without moving it away from its choice. Every option + keeps its short clickable description. Settings shared by the whole group use + a named section below it, never a panel under whichever option is selected. + Equivalent branches use the same container treatment. + The panel element stays inside the group element for options that are not + last: "directly beneath its option" and "outside the group" cannot both hold + for a middle option, and proximity carries the association the panel exists + to show (ruling recorded 2026-10-17). The panel is a named landmark, so it + remains reachable by landmark navigation, and Base UI's group arrow keys move + only between the radios; Tab reaches the panel's fields. +- **Dialog facts**: use the SPA's `FactList` for review and approval details: + muted detail-size labels over foreground body-size values, using the shared + `Fact` component also used by status and node cards. Technical values may + use monospace without becoming smaller. Wrap decision-critical values rather + than truncating paths, destinations, or fingerprints; offer copying when useful. +- **Pending list edits**: when Add or Remove changes a draft, explain that Save + applies the changes. Disable Save until the draft differs from saved data, + including after edits are undone; don't add an unsaved-changes message. + Compare list contents rather than their order. Empty-state wording must describe + the pending result rather than claiming the saved state has changed. +- **Confirmation warnings**: present disruptive consequences in `WarningNotice`, + separate from introductory text and target facts. Failed operations use + `ErrorNotice`; prerequisites and restrictions remain gold. - **Choice group**: radios inside `role="radiogroup"` with an `aria-label`; a - dependent setting sits BELOW the group after a rule, never indented under one - option; its dependency is a disabled control that says why and names what - would answer. *("needs the box above" named a widget.)* + setting specific to one option uses its inset `ChoiceFields` panel. Settings + shared by the group sit below it after a rule; unavailable settings explain + their dependency and what would make them available. *("needs the box above" named a widget.)* - **Copy affordance**: the copy ICON, at `icon-sm`, never the word "Copy" as a button label, and never a caption of its own: what is copied is named by `aria-label` (`Copy server address`), and the icon is a `Check` in `text-success` for the 1.5 s the diff --git a/docs/ux-review-2026-10-17.md b/docs/ux-review-2026-10-17.md new file mode 100644 index 000000000..575d776f6 --- /dev/null +++ b/docs/ux-review-2026-10-17.md @@ -0,0 +1,68 @@ +# Dashboard UX review, 2026-10-17 + +This review covers the server dashboard and the shared node-admin components +it renders. The first pass missed shared runtime facts and some conditional +forms. This follow-up reviews the recurring patterns across routes and their +components; it is not a claim that every possible product state was exercised. + +## Findings addressed + +| Pattern | Finding and correction | Surfaces | +| --- | --- | --- | +| Read-only facts | Labels and values had similar size and colour, and several screens implemented their own pairs. Shared `Fact` now uses smaller muted labels and body-sized foreground values, including monospace values. | Runtime, Status, node details, SSH trust, Networking, clone, enrollment, backup summaries, destination verification, provider removal, command previews | +| Long values | Some review values were truncated or could force a wide layout. Facts wrap complete values; machine facts collapse to one column on narrow screens. | Addresses, paths, fingerprints, commands, SSH summaries, node metadata | +| Radio-dependent fields | Fields below the whole group made their association unclear. The selected option now contains an inset, bordered field panel immediately below its description, outside its clickable label. | SSH authentication, SSH forwarding, Restore source and mode | +| Save availability | Touched fields and unchanged drafts could enable Save. Save now compares the effective draft with saved data, including reverting edits; the disabled Save and its reason carry the pending-change fact, so no separate unsaved-changes sentence is shown. | Sharing, profile, device name, instance name, providers, prompts, prompt stacks, network plugin settings, server addresses | +| Notices and validation | Consequence notices and error styling varied. Shared notices distinguish warnings from failures; backup validation distinguishes missing input from an invalid supplied value. | Sharing, restart, update, supervision, destination verification, backup creation and restore, generated API keys | +| Review information | Destination identity details were hidden behind a disclosure. Destination and copyable fingerprint are now visible at the trust decision. Profile email is presented as a fact instead of an editable-looking field. | SSH verification, account profile | + +The [design system](design-system.md) records the shared rules. Creation, +immediate-save switches, and actions that also apply an existing configuration +retain their respective behavior; they are not ordinary draft-save actions. + +## Coverage and evidence + +Source review covered dashboard routes, their dialogs and editors, and the +shared node-admin fact components. The main review criteria were information +hierarchy, complete values, conditional-field association, validation tone, +Save availability, and narrow-screen layout. + +Browser checks used the isolated review backend and Vite frontend, with a +seeded terminal containing a long working-directory path. At both 320px and +1600px, the following route entries had no document-level horizontal overflow +with the available data: + +- Subshells, Nodes, the server node detail, Presets, Prompts, Workspaces. +- Account and Preferences. +- Server Settings: General, Users, Auth, API keys, Plugins, Service, + Networking, Updates, Backups, Status, and Logs. +- The legacy SSH settings entry redirected to Presets as expected. + +That is 20 route entries and 19 distinct rendered surfaces. Many lists were +empty in the sandbox; a successful page-width check does not prove every +populated row or dialog fits. Additional browser checks opened the SSH +authentication field panel at both widths and the saved-backup Restore panel. +The SSH dialog had no horizontal overflow, and Restore exposed its associated +region. Runtime and Restore screenshots were visually inspected. + +Component tests cover the changed radio association, sharing change/revert +behavior, backup validation and its gold/red tone split, stack editing, address +presentation, enrollment facts, restart and supervision consequence wording, +restore override placement across modes, origins-draft comparison, and the +no-unsaved-message posture. The destination-confirmation e2e spec and its docs +screenshot were retargeted at the always-visible fingerprint. Workspace build, +type checks, lint, and tests are the automated verification for the changes; +these are distinct from browser workflow coverage. + +## Limits + +This was not a full accessibility certification or an end-to-end exercise of +every role, failure, and integration state. Remote node/key availability, +external identity providers, native key approval, actual server replacement, +live network changes, and populated saved-backup recovery were not exercised +through the review browser. Existing component tests cover some of those +states, but they do not substitute for integration testing. + +The native desktop assistant and mobile app were outside this dashboard +review. Shared node-admin changes reach their consumers, but those native +surfaces were not visually checked in this pass. diff --git a/e2e/tests/24-ssh-ux.spec.ts b/e2e/tests/24-ssh-ux.spec.ts index 21b0976e3..8f6839ac6 100644 --- a/e2e/tests/24-ssh-ux.spec.ts +++ b/e2e/tests/24-ssh-ux.spec.ts @@ -261,8 +261,10 @@ test("unfamiliar destinations ask in the connection flow and cancellation saves await page.getByRole("button", { name: "Start SSH subshell", exact: true }).click(); const confirmation = page.getByRole("dialog", { name: "Connect to this destination?", exact: true }); await expect(confirmation.getByText(destination, { exact: true })).toBeVisible(); - await confirmation.getByText("Verification details", { exact: true }).click(); + // The fingerprint and the unverified-identity warning are shown at the + // decision itself (spec 2026-10-17 UX pass); there is no disclosure to open. await expect(confirmation.getByText("SHA256:destination-identity", { exact: true })).toBeVisible(); + await expect(confirmation.getByText(/independently verified/)).toBeVisible(); await page.screenshot({ path: "test-results/ssh-destination-confirmation.png", fullPage: true }); await confirmation.getByRole("button", { name: "Cancel", exact: true }).click(); await expect(confirmation).toHaveCount(0); diff --git a/packages/node-admin/src/components/fact-list.tsx b/packages/node-admin/src/components/fact-list.tsx index d46be1739..54b07c666 100644 --- a/packages/node-admin/src/components/fact-list.tsx +++ b/packages/node-admin/src/components/fact-list.tsx @@ -1,4 +1,5 @@ import type { ReactNode } from "react"; +import { cn } from "../lib/utils"; import { Card, CardContent, CardHeader, CardTitle } from "../ui/card"; /** @@ -17,9 +18,9 @@ export function Fact({ wide?: boolean; }) { return ( -
-
{label}
-
{children}
+
+
{label}
+
{children}
); } @@ -36,7 +37,7 @@ export function FactCard({ title, children }: { title: string; children: ReactNo {title} -
{children}
+
{children}
 ); diff --git a/packages/node-admin/src/components/node-runtime-card.tsx b/packages/node-admin/src/components/node-runtime-card.tsx index e2a22815d..eef1966f2 100644 --- a/packages/node-admin/src/components/node-runtime-card.tsx +++ b/packages/node-admin/src/components/node-runtime-card.tsx @@ -117,7 +117,7 @@ export function NodeRuntimeCard({ node, local = false }: { node: NodeDetail; loc {runtime.tmuxPath ? ( - {runtime.tmuxPath} + {runtime.tmuxPath} ) : ( // Said here rather than discovered at launch time: without tmux the // node accepts nothing, and nothing else on this page would say so. diff --git a/packages/node-admin/src/components/ssh-trust-card.tsx b/packages/node-admin/src/components/ssh-trust-card.tsx index f3c2a2311..86a9e7b6d 100644 --- a/packages/node-admin/src/components/ssh-trust-card.tsx +++ b/packages/node-admin/src/components/ssh-trust-card.tsx @@ -3,6 +3,7 @@ import type { NodeDetail, NodeSshTrustPeer } from "../types/node"; import { Badge } from "../ui/badge"; import { Card, CardContent, CardHeader, CardTitle } from "../ui/card"; import { CopyableValue } from "../ui/copyable-value"; +import { Fact } from "./fact-list"; /** * The machine trust card (spec 2026-10-08 §4.6): this machine's own SSH relay @@ -77,20 +78,18 @@ export function NodeSshTrustCard({ copyLabel="This machine's encryption fingerprint" /> {trust.peers.length === 0 ? ( -
-
Saved machine identities
-
No saved machine identities yet.
-
+ + No saved machine identities yet. + ) : ( trust.peers.map((peer) => ( -
-
Peer {peer.nodeId}
-
+ +
-
+
{renderPeerAction &&
{renderPeerAction(peer)}
} -
+ )) )}
@@ -100,22 +99,19 @@ export function NodeSshTrustCard({ return embedded ?
{content}
: {content}; } -/** One labelled fingerprint: `label` over a mono `detail`, the line-item shape. */ +/** Fingerprints use the same label/value hierarchy as other machine facts. */ function FingerprintFact({ label, value, copyLabel }: { label: string; value: string; copyLabel: string }) { return ( -
-
{label}
-
- -
-
+ + + ); } /** The value plus its copy affordance, the thing a person takes to the other machine. */ function FingerprintValue({ value, copyLabel }: { value: string; copyLabel: string }) { return ( - + );