feat(FN-3704): separate plugin lifecycle from setup probe state
Fixes a bug where plugin lifecycle operations were incorrectly gated by the setup probe state, separating the two concerns across the API, PluginManager component, routes, and tests, with updated documentation. Fusion-Task-Id: FN-3704
This commit is contained in:
@@ -7844,8 +7844,13 @@ export async function updatePluginSettings(
|
||||
|
||||
export type PluginSetupStatusResponse =
|
||||
| { hasSetup: false }
|
||||
| { hasSetup: false; status: Extract<PluginSetupCheckResult, { status: "error" }> }
|
||||
| ({ hasSetup: true } & PluginSetupCheckResult);
|
||||
| ({ hasSetup: true } & PluginSetupCheckResult)
|
||||
| {
|
||||
hasSetup: true;
|
||||
setupCheckDeferred: true;
|
||||
deferredReason: "plugin-not-started";
|
||||
pluginState: PluginInstallation["state"];
|
||||
};
|
||||
|
||||
/** Fetch plugin setup status */
|
||||
export async function fetchPluginSetupStatus(id: string, projectId?: string): Promise<PluginSetupStatusResponse> {
|
||||
|
||||
@@ -417,6 +417,11 @@
|
||||
color: var(--color-warning);
|
||||
}
|
||||
|
||||
.plugin-builtins-setup-status--deferred {
|
||||
background: var(--status-archived-bg);
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.plugin-builtins-description-text {
|
||||
flex: 1 1 100%;
|
||||
color: var(--text-muted);
|
||||
|
||||
@@ -233,7 +233,12 @@ export function PluginManager({ addToast, projectId }: PluginManagerProps) {
|
||||
}
|
||||
setBuiltinSetupStatusById((prev) => ({
|
||||
...prev,
|
||||
[builtinPlugin.id]: { hasSetup: true, status: "error", error: "Failed to check setup status" },
|
||||
[builtinPlugin.id]: {
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: "installed",
|
||||
},
|
||||
}));
|
||||
}
|
||||
}));
|
||||
@@ -772,12 +777,20 @@ export function PluginManager({ addToast, projectId }: PluginManagerProps) {
|
||||
const installedPlugin = installedPluginsById.get(builtinPlugin.id);
|
||||
const isInstalled = Boolean(installedPlugin);
|
||||
const setupStatus = builtinSetupStatusById[builtinPlugin.id];
|
||||
const setupStatusDeferred = Boolean(
|
||||
setupStatus
|
||||
&& "setupCheckDeferred" in setupStatus
|
||||
&& setupStatus.setupCheckDeferred,
|
||||
);
|
||||
const pluginSetupState = setupStatus && "status" in setupStatus ? setupStatus.status : undefined;
|
||||
const requiresSetupAction =
|
||||
isInstalled
|
||||
&& builtinPlugin.hasSetup
|
||||
&& setupStatus?.hasSetup
|
||||
&& (setupStatus.status === "not-installed" || setupStatus.status === "error");
|
||||
const setupReady = isInstalled && setupStatus?.hasSetup && setupStatus.status === "installed";
|
||||
&& !setupStatusDeferred
|
||||
&& installedPlugin?.state === "started"
|
||||
&& (pluginSetupState === "not-installed" || pluginSetupState === "error");
|
||||
const setupReady = isInstalled && setupStatus?.hasSetup && pluginSetupState === "installed";
|
||||
const setupCheckInFlight = loadingBuiltinSetupId === builtinPlugin.id;
|
||||
const metadataOnly = !builtinPlugin.path;
|
||||
|
||||
@@ -799,6 +812,9 @@ export function PluginManager({ addToast, projectId }: PluginManagerProps) {
|
||||
{setupCheckInFlight && (
|
||||
<span className="plugin-builtins-setup-status plugin-builtins-setup-status--pending">Checking setup...</span>
|
||||
)}
|
||||
{setupStatusDeferred && (
|
||||
<span className="plugin-builtins-setup-status plugin-builtins-setup-status--deferred">Start plugin to check setup</span>
|
||||
)}
|
||||
<span className="plugin-builtins-description-text">{builtinPlugin.description}</span>
|
||||
</div>
|
||||
{metadataOnly ? (
|
||||
|
||||
@@ -337,6 +337,52 @@ describe("PluginManager", () => {
|
||||
expect(screen.getByLabelText("Command Timeout (ms)")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does not show setup-required state when built-in setup check is deferred for non-started plugin", async () => {
|
||||
vi.mocked(fetchPlugins).mockResolvedValueOnce([
|
||||
{
|
||||
...mockPlugins[0],
|
||||
id: BUILTIN_AGENT_BROWSER_PLUGIN_ID,
|
||||
name: "Agent Browser",
|
||||
state: "installed",
|
||||
},
|
||||
]);
|
||||
vi.mocked(fetchPluginSetupStatus).mockResolvedValueOnce({
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: "installed",
|
||||
});
|
||||
|
||||
render(<PluginManager addToast={addToast} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Start plugin to check setup")).toBeTruthy();
|
||||
});
|
||||
expect(screen.queryByText("Setup required")).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps setup-required state for true setup errors on started built-in plugins", async () => {
|
||||
vi.mocked(fetchPlugins).mockResolvedValueOnce([
|
||||
{
|
||||
...mockPlugins[0],
|
||||
id: BUILTIN_AGENT_BROWSER_PLUGIN_ID,
|
||||
name: "Agent Browser",
|
||||
state: "started",
|
||||
},
|
||||
]);
|
||||
vi.mocked(fetchPluginSetupStatus).mockResolvedValueOnce({
|
||||
hasSetup: true,
|
||||
status: "error",
|
||||
error: "Binary missing",
|
||||
});
|
||||
|
||||
render(<PluginManager addToast={addToast} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Setup required")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("installs built-in runtime plugins from the built-in section", async () => {
|
||||
render(<PluginManager addToast={addToast} />);
|
||||
|
||||
|
||||
@@ -841,6 +841,27 @@ describe("plugin setup routes", () => {
|
||||
expect(res.body).toEqual({ hasSetup: false });
|
||||
});
|
||||
|
||||
it("GET /plugins/:id/setup-status returns deferred status when plugin is not started", async () => {
|
||||
(pluginStore.getPlugin as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ ...FAKE_PLUGIN, state: "installed" });
|
||||
pluginRunner.getPluginSetupInfo.mockReturnValueOnce([
|
||||
{
|
||||
pluginId: "test-plugin",
|
||||
manifest: { binaryName: "agent-browser", description: "Binary" },
|
||||
hooks: { checkSetup: vi.fn() },
|
||||
},
|
||||
]);
|
||||
|
||||
const res = await REQUEST(buildApp(), "GET", "/api/plugins/test-plugin/setup-status");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: "installed",
|
||||
});
|
||||
expect(pluginRunner.checkPluginSetup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("GET /plugins/:id/setup-status returns 404 for nonexistent plugin", async () => {
|
||||
(pluginStore.getPlugin as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error('Plugin "missing" not found'));
|
||||
|
||||
|
||||
@@ -1102,7 +1102,7 @@ describe("createPluginRouter plugin setup routes", () => {
|
||||
expect(res.body).toEqual({ hasSetup: false });
|
||||
});
|
||||
|
||||
it("returns plugin not loaded status when setup metadata exists but plugin is stopped", async () => {
|
||||
it("returns deferred setup status when setup metadata exists but plugin is not started", async () => {
|
||||
(pluginStore.getPlugin as ReturnType<typeof vi.fn>).mockResolvedValueOnce({ ...INSTALLED_PLUGIN, state: "installed" });
|
||||
pluginRunner.getPluginSetupInfo.mockReturnValueOnce([
|
||||
{
|
||||
@@ -1116,9 +1116,12 @@ describe("createPluginRouter plugin setup routes", () => {
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
hasSetup: false,
|
||||
status: { status: "error", error: "Plugin not loaded" },
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: "installed",
|
||||
});
|
||||
expect(pluginRunner.checkPluginSetup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns setup status when setup metadata exists and plugin is started", async () => {
|
||||
|
||||
@@ -406,8 +406,10 @@ export function createPluginRouter(
|
||||
|
||||
if (plugin.state !== "started") {
|
||||
res.json({
|
||||
hasSetup: false,
|
||||
status: { status: "error", error: "Plugin not loaded" },
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: plugin.state,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -3590,8 +3590,10 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
|
||||
if (plugin.state !== "started") {
|
||||
res.json({
|
||||
hasSetup: false,
|
||||
status: { status: "error", error: "Plugin not loaded" },
|
||||
hasSetup: true,
|
||||
setupCheckDeferred: true,
|
||||
deferredReason: "plugin-not-started",
|
||||
pluginState: plugin.state,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -212,16 +212,7 @@ export function registerMessagingScriptRoutes(ctx: ApiRoutesContext): void {
|
||||
const storeRoot = resolve(scopedStore.getRootDir());
|
||||
for (const engine of engineManager.getAllEngines().values()) {
|
||||
if (resolve(engine.getWorkingDirectory()) === storeRoot) {
|
||||
const monitor = engine.getHeartbeatMonitor();
|
||||
if (!monitor) {
|
||||
return undefined;
|
||||
}
|
||||
return {
|
||||
rootDir: engine.getWorkingDirectory(),
|
||||
startRun: monitor.startRun.bind(monitor),
|
||||
executeHeartbeat: monitor.executeHeartbeat.bind(monitor),
|
||||
stopRun: monitor.stopRun.bind(monitor),
|
||||
};
|
||||
return (engine.getHeartbeatMonitor() ?? undefined) as HeartbeatMonitorHandle | undefined;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
|
||||
Reference in New Issue
Block a user