forked from molecule-ai/molecule-core
fix(canvas): plugin install POSTed to /workspaces/undefined/plugins
SkillsTab read \`data.id\` from its props and used the value to build
two API URLs:
POST /workspaces/\${data.id}/plugins
DELETE /workspaces/\${data.id}/plugins/\${pluginName}
But \`data\` is the React Flow node.data blob (WorkspaceNodeData) —
the workspace id lives on \`node.id\`, NOT on \`node.data\`. WorkspaceNodeData
extends \`Record<string, unknown>\`, which makes \`data.id\` type-check
silently as \`unknown\` instead of erroring. So every install/uninstall
hit \`/workspaces/undefined/plugins\`, the server's not-found path
returned 503 "workspace container not running" (misleading — the real
issue was the bogus URL), and the user got a confusing toast.
Every other tab in SidePanel takes \`workspaceId={selectedNodeId}\` as
an explicit prop. SkillsTab was the lone outlier, presumably because
"data has all the fields I need" is the obvious-looking shortcut that
TypeScript can't catch through the index-signature interface.
Fix: make \`workspaceId\` an explicit prop on SkillsTab, drop the
\`data.id\` reads, thread the prop from SidePanel like the other tabs.
Test fixture updated to pass it.
Verified: 993 canvas tests pass; tsc clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ee429cfee7
commit
176b703dbc
@ -280,7 +280,7 @@ export function SidePanel() {
|
|||||||
className="flex-1 overflow-y-auto focus:outline-none"
|
className="flex-1 overflow-y-auto focus:outline-none"
|
||||||
>
|
>
|
||||||
{panelTab === "details" && <DetailsTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
|
{panelTab === "details" && <DetailsTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
|
||||||
{panelTab === "skills" && <SkillsTab key={selectedNodeId} data={node.data} />}
|
{panelTab === "skills" && <SkillsTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
|
||||||
{panelTab === "activity" && <ActivityTab key={selectedNodeId} workspaceId={selectedNodeId} />}
|
{panelTab === "activity" && <ActivityTab key={selectedNodeId} workspaceId={selectedNodeId} />}
|
||||||
{panelTab === "chat" && <ChatTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
|
{panelTab === "chat" && <ChatTab key={selectedNodeId} workspaceId={selectedNodeId} data={node.data} />}
|
||||||
{panelTab === "terminal" && <TerminalTab key={selectedNodeId} workspaceId={selectedNodeId} />}
|
{panelTab === "terminal" && <TerminalTab key={selectedNodeId} workspaceId={selectedNodeId} />}
|
||||||
|
|||||||
@ -123,7 +123,7 @@ describe("SkillsTab — aria-label on bare source input (WCAG 1.3.1)", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('install source input has aria-label="Install from source URL"', async () => {
|
it('install source input has aria-label="Install from source URL"', async () => {
|
||||||
render(<SkillsTab data={makeSkillsData() as never} />);
|
render(<SkillsTab workspaceId="ws-test-id" data={makeSkillsData() as never} />);
|
||||||
|
|
||||||
// The source input is inside the registry section (showRegistry=false initially).
|
// The source input is inside the registry section (showRegistry=false initially).
|
||||||
// Click the "+ Install Plugin" button to reveal it.
|
// Click the "+ Install Plugin" button to reveal it.
|
||||||
@ -138,7 +138,7 @@ describe("SkillsTab — aria-label on bare source input (WCAG 1.3.1)", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("install source input is a text input (not hidden)", async () => {
|
it("install source input is a text input (not hidden)", async () => {
|
||||||
render(<SkillsTab data={makeSkillsData() as never} />);
|
render(<SkillsTab workspaceId="ws-test-id" data={makeSkillsData() as never} />);
|
||||||
|
|
||||||
const installBtn = screen.getByRole("button", { name: /install plugin/i });
|
const installBtn = screen.getByRole("button", { name: /install plugin/i });
|
||||||
fireEvent.click(installBtn);
|
fireEvent.click(installBtn);
|
||||||
|
|||||||
@ -6,6 +6,14 @@ import { useCanvasStore, summarizeWorkspaceCapabilities, type WorkspaceNodeData
|
|||||||
import { showToast } from "../Toaster";
|
import { showToast } from "../Toaster";
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
|
// The workspace's id is NOT a field on WorkspaceNodeData — that
|
||||||
|
// interface is the React Flow `node.data` blob, while the id lives
|
||||||
|
// on `node.id`. Pass it explicitly (matches every other tab in
|
||||||
|
// SidePanel) so the install/uninstall API calls don't end up
|
||||||
|
// POSTing to /workspaces/undefined/plugins. The interface extending
|
||||||
|
// Record<string, unknown> meant TypeScript silently typed
|
||||||
|
// `data.id` as `unknown` instead of erroring — easy to miss.
|
||||||
|
workspaceId: string;
|
||||||
data: WorkspaceNodeData;
|
data: WorkspaceNodeData;
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -40,7 +48,7 @@ interface SourceSchemesResponse {
|
|||||||
// Delay before reloading installed plugins after install/uninstall (workspace restarts)
|
// Delay before reloading installed plugins after install/uninstall (workspace restarts)
|
||||||
const PLUGIN_RELOAD_DELAY_MS = 15_000;
|
const PLUGIN_RELOAD_DELAY_MS = 15_000;
|
||||||
|
|
||||||
export function SkillsTab({ data }: Props) {
|
export function SkillsTab({ workspaceId, data }: Props) {
|
||||||
const capability = summarizeWorkspaceCapabilities(data);
|
const capability = summarizeWorkspaceCapabilities(data);
|
||||||
const skills = useMemo(() => extractSkills(data.agentCard), [data.agentCard]);
|
const skills = useMemo(() => extractSkills(data.agentCard), [data.agentCard]);
|
||||||
const setPanelTab = useCanvasStore((s) => s.setPanelTab);
|
const setPanelTab = useCanvasStore((s) => s.setPanelTab);
|
||||||
@ -74,8 +82,6 @@ export function SkillsTab({ data }: Props) {
|
|||||||
};
|
};
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
const workspaceId = data.id;
|
|
||||||
|
|
||||||
// Tracks whether loadInstalled has completed at least once (success
|
// Tracks whether loadInstalled has completed at least once (success
|
||||||
// or empty-array success — NOT failure). Without this the auto-
|
// or empty-array success — NOT failure). Without this the auto-
|
||||||
// expand effect below would fire on the initial render where
|
// expand effect below would fire on the initial render where
|
||||||
@ -233,7 +239,7 @@ export function SkillsTab({ data }: Props) {
|
|||||||
const handleUninstall = async (pluginName: string) => {
|
const handleUninstall = async (pluginName: string) => {
|
||||||
setUninstalling(pluginName);
|
setUninstalling(pluginName);
|
||||||
try {
|
try {
|
||||||
await api.del(`/workspaces/${data.id}/plugins/${pluginName}`);
|
await api.del(`/workspaces/${workspaceId}/plugins/${pluginName}`);
|
||||||
showToast(`Removed ${pluginName} — restarting workspace`, "success");
|
showToast(`Removed ${pluginName} — restarting workspace`, "success");
|
||||||
setInstalled((prev) => prev.filter((p) => p.name !== pluginName));
|
setInstalled((prev) => prev.filter((p) => p.name !== pluginName));
|
||||||
reloadTimerRef.current = setTimeout(() => loadInstalled(), PLUGIN_RELOAD_DELAY_MS);
|
reloadTimerRef.current = setTimeout(() => loadInstalled(), PLUGIN_RELOAD_DELAY_MS);
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user