Skip to content

🤖 fix: keep plugin card actions in one place on every card #7754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ThomasK33 merged 3 commits into from
Oct 30, 2026
Merged

🤖 fix: keep plugin card actions in one place on every card #7754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8acf9a8
🤖 fix: keep plugin card actions in a fixed header row
ThomasK33 Oct 30, 2026
3148a8a
🤖 fix: left-align plugin card actions on narrow cards
ThomasK33 Oct 30, 2026
8856dd9
🤖 tests: cover plugin card action placement in existing stories
ThomasK33 Oct 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/browser/features/Settings/Sections/PluginsSettingsSection.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,16 @@ const MANAGED_ITEM: AgentPluginListItem = {
mcpServerCount: 1,
};

const PINNED_DESCRIPTION =
"Deploy helpers for the infra team: rollout skills for staging and production, a release-notes drafter, and MCP servers that read cluster health and page the on-call engineer when a canary regresses.";

const PINNED_ITEM: AgentPluginListItem = {
name: "deploy-tools",
managed: true,
present: true,
location: "~/.mux/plugins/deploy-tools",
version: "2.0.0",
description: PINNED_DESCRIPTION,
source: {
type: "git",
url: "git@git.corp:infra/deploy-tools.git",
Expand Down Expand Up @@ -205,6 +209,12 @@ async function openHooksReview(canvasElement: HTMLElement) {
return canvas;
}

function pluginCardOf(element: HTMLElement): HTMLElement {
const card = element.closest<HTMLElement>("div[class*='rounded-md']");
if (card === null) throw new Error("Plugin card not found");
return card;
}

const PluginsSectionStoryShell: FC<{ options: MockORPCClientOptions; children: ReactNode }> = (
props
) => {
Expand Down Expand Up @@ -277,6 +287,12 @@ export const InstalledWithUpdateStates: Story = {
await expectInterfaceAsText(canvasElement);
// Update action appears only for rows whose tracking ref moved.
await canvas.findAllByRole("button", { name: /Update/ });
// A long description must not push the actions off the name row.
const uninstall = await canvas.findByRole("button", { name: "Uninstall deploy-tools" });
const description = within(pluginCardOf(uninstall)).getByText(PINNED_DESCRIPTION);
if (uninstall.getBoundingClientRect().bottom > description.getBoundingClientRect().top) {
throw new Error("Plugin actions dropped below the description");
}
},
};

Expand Down Expand Up @@ -338,6 +354,12 @@ export const InstalledPhoneViewport: Story = {
) {
throw new Error("Max-length capability badge overflows its card at phone width");
}
// Wrapped actions start flush with the card text, not at the right edge.
const manage = await canvas.findByRole("button", { name: "Manage components for grill" });
const name = within(pluginCardOf(manage)).getByText("grill");
if (Math.abs(manage.getBoundingClientRect().left - name.getBoundingClientRect().left) > 1) {
throw new Error("Wrapped plugin actions are not flush with the card text");
}
},
};

Expand Down
91 changes: 47 additions & 44 deletions src/browser/features/Settings/Sections/PluginsSettingsSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1652,19 +1652,19 @@
);
};

export const PluginsSettingsSection: React.FC = () => {
const { api } = useAPI();
const [items, setItems] = useState<AgentPluginListItem[] | null>(null);
// List/mutation errors and update-check errors live in separate state: the
// mount-time list query and update check run concurrently, and a later
// refresh success must not clear a check failure (an unreachable remote has
// to stay visibly unknown, never silently "up to date").
const [error, setError] = useState<string | null>(null);
const [updateCheckError, setUpdateCheckError] = useState<string | null>(null);
const [updateChecks, setUpdateChecks] = useState<Map<string, AgentPluginUpdateCheck>>(
() => new Map()
);
const [checkingUpdates, setCheckingUpdates] = useState(false);

Check warning on line 1667 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: false -> true 

No unit test failed when PluginsSettingsSection had false -> true. Add a test that tells them apart, or say why they are equivalent.
// Backend-provided container path: the root is config-derived (canonically
// ~/.shux, possibly custom/legacy), so this copy must never hardcode it.
const [containerLocation, setContainerLocation] = useState<string | null>(null);
Expand Down Expand Up @@ -1695,7 +1695,7 @@
const [hooksTarget, setHooksTarget] = useState<string | null>(
initialIntent?.type === "review-hooks" ? initialIntent.name : null
);
const [installSucceeded, setInstallSucceeded] = useState(false);

Check warning on line 1698 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: false -> true 

No unit test failed when PluginsSettingsSection had false -> true. Add a test that tells them apart, or say why they are equivalent.
/** Name of the plugin with an update/uninstall in flight. */
const [busyPlugin, setBusyPlugin] = useState<string | null>(null);
/** Pending update whose capability changes await the user's confirmation. */
Expand All @@ -1703,8 +1703,8 @@
initialIntent?.type === "review-update" ? initialIntent.review : null
);
/** Monotonic ids of the latest list/update-check requests; stale responses must not commit state. */
const listGenerationRef = useRef(0);

Check warning on line 1706 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: 0 -> 1 

No unit test failed when PluginsSettingsSection had 0 -> 1. Add a test that tells them apart, or say why they are equivalent.
const checkGenerationRef = useRef(0);

Check warning on line 1707 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: 0 -> 1 

No unit test failed when PluginsSettingsSection had 0 -> 1. Add a test that tells them apart, or say why they are equivalent.

const componentReview = useComponentReview({
api,
Expand All @@ -1725,7 +1725,7 @@
};
const openAddPanel = () => {
// Feedback belongs to the completed install, not the next attempt from any entry point.
setInstallSucceeded(false);

Check warning on line 1728 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: false -> true 

No unit test failed when PluginsSettingsSection had false -> true. Add a test that tells them apart, or say why they are equivalent.
setAddOpen(true);
};

Expand Down Expand Up @@ -1764,7 +1764,7 @@
// palette update): only the latest request may commit state, or a stale
// response can resurrect an update badge the update just cleared.
const generation = ++checkGenerationRef.current;
setCheckingUpdates(true);

Check warning on line 1767 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: true -> false 

No unit test failed when PluginsSettingsSection had true -> false. Add a test that tells them apart, or say why they are equivalent.
try {
const result = await api.agentPlugins.checkUpdates();
if (generation !== checkGenerationRef.current) {
Expand All @@ -1781,8 +1781,8 @@
setUpdateCheckError(getErrorMessage(err));
}
} finally {
if (generation === checkGenerationRef.current) {

Check warning on line 1784 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: === -> !== 

No unit test failed when PluginsSettingsSection had === -> !==. Add a test that tells them apart, or say why they are equivalent.
setCheckingUpdates(false);

Check warning on line 1785 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: false -> true 

No unit test failed when PluginsSettingsSection had false -> true. Add a test that tells them apart, or say why they are equivalent.
}
}
};
Expand All @@ -1796,7 +1796,7 @@
const intent = takeInitialIntent();
void refresh().finally(() => componentReview.openFromIntent(intent));
void checkForUpdates();
void api?.agentPlugins.containerLocation().then(setContainerLocation, () => undefined);

Check warning on line 1799 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: ?. -> . 

No unit test failed when PluginsSettingsSection had ?. -> .. Add a test that tells them apart, or say why they are equivalent.
// eslint-disable-next-line react-hooks/exhaustive-deps -- fetch on mount / API reconnect only; refresh/checkForUpdates are plain handlers (compiler-memoized), not inputs
}, [api]);

Expand Down Expand Up @@ -1846,7 +1846,7 @@
* SHAs — see installService.update).
*/
const handleUpdate = async (name: string) => {
if (!api || busyPlugin !== null) return;

Check warning on line 1849 in src/browser/features/Settings/Sections/PluginsSettingsSection.tsx

View workflow job for this annotation

GitHub Actions / Quality / Mutation

Mutation survived: || -> && 

No unit test failed when PluginsSettingsSection had || -> &&. Add a test that tells them apart, or say why they are equivalent.
setBusyPlugin(name);
setError(null);
setUpdateReview(null);
Expand Down Expand Up @@ -2037,52 +2037,28 @@
key={`${item.location}:${item.name}`}
className="border-border-medium rounded-md border p-3"
>
<div className="flex flex-wrap items-start justify-between gap-2">
<div className="min-w-0">
<div className="flex flex-wrap items-baseline gap-2">
{/* break-all: names can be 64 separator-free chars. */}
<span className="text-foreground text-sm font-medium break-all">
{item.name}
</span>
{item.version && (
<span className="text-muted text-xs">v{item.version}</span>
)}
{!item.managed && <Badge tone="muted">unmanaged</Badge>}
{item.managed && !item.present && <Badge tone="error">missing</Badge>}
{check?.status === "update-available" && (
<Badge tone="accent">update available</Badge>
)}
{check?.status === "tag-moved" && <Badge tone="warning">tag moved</Badge>}
{check?.status === "pinned" && <Badge tone="muted">pinned</Badge>}
{check?.status === "error" && <Badge tone="warning">check failed</Badge>}
<NeedsReviewBadge item={item} />
</div>
{item.description && (
<p className="text-muted mt-0.5 text-xs">{item.description}</p>
)}
{item.interface && <PluginInterfaceDetails info={item.interface} />}
{/* break-all: locations/sources can contain unbreakable
64-char tokens (max-length plugin names) that would
otherwise overflow the card at phone widths. */}
<p className="text-muted counter-nums mt-0.5 text-[11px] break-all">
{item.importedSkillCount ?? item.skillCount} of {item.skillCount} skills
imported · {item.importedMcpServerCount ?? item.mcpServerCount} of{" "}
{item.mcpServerCount} MCP servers imported · <code>{item.location}</code>
</p>
{formatSource(item) && (
<p className="text-muted mt-0.5 text-[11px] break-all">
{formatSource(item)}
{item.lockedSha ? ` · ${item.lockedSha.slice(0, 12)}` : ""}
</p>
)}
{check?.status === "error" && check.message && (
<p className="text-warning mt-0.5 flex items-start gap-1 text-[11px]">
<CircleAlert className="mt-0.5 h-3 w-3 shrink-0" />
<span className="min-w-0 break-words">{check.message}</span>
</p>
{/* The header holds only name, badges and actions: whether the actions
wrap then depends on the title alone, not on description length, so
every card puts them in the same place. The flex-1 name block pushes the
actions to the right on the name row. Once they wrap they start at the
left, flush with the card text, so they need no ml-auto/justify-end. */}
<div className="flex min-h-7 flex-wrap items-center gap-x-3 gap-y-2">
<div className="flex min-w-0 flex-1 basis-48 flex-wrap items-baseline gap-2">
{/* break-all: names can be 64 separator-free chars. */}
<span className="text-foreground text-sm font-medium break-all">
{item.name}
</span>
{item.version && <span className="text-muted text-xs">v{item.version}</span>}
{!item.managed && <Badge tone="muted">unmanaged</Badge>}
{item.managed && !item.present && <Badge tone="error">missing</Badge>}
{check?.status === "update-available" && (
<Badge tone="accent">update available</Badge>
)}
{check?.status === "tag-moved" && <Badge tone="warning">tag moved</Badge>}
{check?.status === "pinned" && <Badge tone="muted">pinned</Badge>}
{check?.status === "error" && <Badge tone="warning">check failed</Badge>}
<NeedsReviewBadge item={item} />
</div>

{item.managed && (
<div className="flex flex-wrap gap-1">
<ReviewComponentsButton
Expand Down Expand Up @@ -2150,6 +2126,33 @@
)}
</div>

{item.description && (
<p className="text-muted mt-1 text-xs">{item.description}</p>
)}
{item.interface && <PluginInterfaceDetails info={item.interface} />}
<div className="text-muted mt-2 space-y-0.5 text-[11px]">
{/* break-all: locations/sources can contain unbreakable
64-char tokens (max-length plugin names) that would
otherwise overflow the card at phone widths. */}
<p className="counter-nums break-all">
{item.importedSkillCount ?? item.skillCount} of {item.skillCount} skills
imported · {item.importedMcpServerCount ?? item.mcpServerCount} of{" "}
{item.mcpServerCount} MCP servers imported · <code>{item.location}</code>
</p>
{formatSource(item) && (
<p className="break-all">
{formatSource(item)}
{item.lockedSha ? ` · ${item.lockedSha.slice(0, 12)}` : ""}
</p>
)}
</div>
{check?.status === "error" && check.message && (
<p className="text-warning mt-1 flex items-start gap-1 text-[11px]">
<CircleAlert className="mt-0.5 h-3 w-3 shrink-0" />
<span className="min-w-0 break-words">{check.message}</span>
</p>
)}

{/* Managed rows only: an unmanaged plugin in another
container can share the manifest name, and rendering the
confirmation under its row would visually attach a
Expand Down
Loading