fix(engine): address workflow extension PR feedback
This commit is contained in:
@@ -512,6 +512,38 @@ describe("aiMergeTask pre-merge fetch + fast-forward (smart strategies)", () =>
|
||||
expect(store.upsertMergeRequestRecord).toHaveBeenCalledWith("FN-5741-MANUAL", { state: "manual-required" });
|
||||
expect(store.transitionMergeRequestState).not.toHaveBeenCalledWith("FN-5741-MANUAL", "running");
|
||||
});
|
||||
|
||||
it("keeps globally disabled auto-merge records in manual-required unless the task opts in", async () => {
|
||||
const store = createMockStore({ id: "FN-5741-GLOBAL-MANUAL", worktree: "/tmp/root", autoMerge: undefined });
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
mergeIntegrationWorktree: "cwd-main" as const,
|
||||
mergeRequestContractShadowEnabled: true,
|
||||
autoMerge: false,
|
||||
});
|
||||
setupSyncMock({ behind: 0, ahead: 0 });
|
||||
|
||||
await aiMergeTask(store, "/tmp/root", "FN-5741-GLOBAL-MANUAL");
|
||||
|
||||
expect(store.upsertMergeRequestRecord).toHaveBeenCalledWith("FN-5741-GLOBAL-MANUAL", { state: "manual-required" });
|
||||
expect(store.transitionMergeRequestState).not.toHaveBeenCalledWith("FN-5741-GLOBAL-MANUAL", "running");
|
||||
});
|
||||
|
||||
it("allows task-level autoMerge true to opt into shadow merge when global auto-merge is disabled", async () => {
|
||||
const store = createMockStore({ id: "FN-5741-TASK-OPT-IN", worktree: "/tmp/root", autoMerge: true });
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
mergeIntegrationWorktree: "cwd-main" as const,
|
||||
mergeRequestContractShadowEnabled: true,
|
||||
autoMerge: false,
|
||||
});
|
||||
setupSyncMock({ behind: 0, ahead: 0 });
|
||||
|
||||
await aiMergeTask(store, "/tmp/root", "FN-5741-TASK-OPT-IN");
|
||||
|
||||
expect(store.upsertMergeRequestRecord).toHaveBeenCalledWith("FN-5741-TASK-OPT-IN", { state: "queued" });
|
||||
expect(store.transitionMergeRequestState).toHaveBeenCalledWith("FN-5741-TASK-OPT-IN", "running");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -3219,4 +3251,3 @@ describe("aiMergeTask post-squash audit gate", () => {
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
|
||||
@@ -7,7 +7,14 @@
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { PluginRunner, type PluginRunnerOptions } from "../plugin-runner.js";
|
||||
import type { PluginLoader, PluginStore, PluginInstallation } from "@fusion/core";
|
||||
import {
|
||||
__resetWorkflowExtensionRegistryForTests,
|
||||
getWorkflowExtensionRegistry,
|
||||
workflowExtensionRegistryId,
|
||||
type PluginLoader,
|
||||
type PluginStore,
|
||||
type PluginInstallation,
|
||||
} from "@fusion/core";
|
||||
import type { FusionPlugin, PluginToolDefinition } from "@fusion/core";
|
||||
import { createLogger } from "../logger.js";
|
||||
|
||||
@@ -154,6 +161,7 @@ describe("PluginRunner", () => {
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
__resetWorkflowExtensionRegistryForTests();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
@@ -901,6 +909,31 @@ describe("PluginRunner", () => {
|
||||
expect(pluginRunner.getPluginSetupInfo()).toEqual(setups);
|
||||
});
|
||||
|
||||
it("syncPluginWorkflowExtensions preserves force-degraded definitions across cache invalidation", async () => {
|
||||
const extensions = [{
|
||||
pluginId: "test-plugin",
|
||||
extension: {
|
||||
extensionId: "move-policy",
|
||||
name: "Move Policy",
|
||||
kind: "move-policy",
|
||||
schemaVersion: 1,
|
||||
fallback: "degradeToDefault",
|
||||
},
|
||||
}];
|
||||
mockPluginLoader.getPluginWorkflowExtensions.mockReturnValue(extensions);
|
||||
const id = workflowExtensionRegistryId("test-plugin", "move-policy");
|
||||
|
||||
pluginRunner.syncPluginWorkflowExtensions();
|
||||
const degraded = pluginRunner.disablePluginWorkflowExtensions("test-plugin", { force: true });
|
||||
(pluginRunner as unknown as { invalidateWorkflowExtensionsCache: () => void }).invalidateWorkflowExtensionsCache();
|
||||
pluginRunner.syncPluginWorkflowExtensions();
|
||||
|
||||
expect(degraded.degraded).toEqual([id]);
|
||||
expect(getWorkflowExtensionRegistry().get(id)?.degraded).toMatchObject({
|
||||
reason: "force-disabled",
|
||||
});
|
||||
});
|
||||
|
||||
it("getPromptContributionsForSurface filters by surface", async () => {
|
||||
mockPluginLoader.getPluginPromptContributions.mockReturnValue([
|
||||
{ pluginId: "test-plugin", contribution: { surface: "executor-system", content: "ok" }, config: { enabledByDefault: true, contributions: [] } },
|
||||
|
||||
@@ -65,4 +65,81 @@ describe("workflow node-handler extensions", () => {
|
||||
}));
|
||||
expect(prompt).toHaveBeenCalledWith(expect.objectContaining({ id: "human" }), expect.any(Object));
|
||||
});
|
||||
|
||||
it("preserves plugin-provided values for custom outcomes", async () => {
|
||||
const extensionKey = workflowExtensionRegistryId("node-plugin", "decision");
|
||||
getWorkflowExtensionRegistry().register("node-plugin", {
|
||||
extensionId: "decision",
|
||||
name: "Decision",
|
||||
kind: "node-handler",
|
||||
nodeKind: "prompt",
|
||||
schemaVersion: WORKFLOW_EXTENSION_SCHEMA_VERSION,
|
||||
fallback: "failClosed",
|
||||
handle: vi.fn().mockResolvedValue({
|
||||
outcome: "outcome:custom-route",
|
||||
value: "needs-human",
|
||||
}),
|
||||
});
|
||||
const workflow: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "node-extension",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "decide", kind: "prompt", column: "work", extensions: { [extensionKey]: {} } },
|
||||
{ id: "human", kind: "prompt", column: "work", config: { prompt: "human" } },
|
||||
{ id: "default", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "decide" },
|
||||
{ from: "decide", to: "human", condition: "outcome:needs-human" },
|
||||
{ from: "decide", to: "default", condition: "outcome:custom-route" },
|
||||
],
|
||||
};
|
||||
const prompt = vi.fn(async () => ({ outcome: "success" as const }));
|
||||
const executor = new WorkflowGraphExecutor({ handlers: { prompt } });
|
||||
|
||||
const result = await executor.run({ id: "FN-NODE" } as TaskDetail, settingsOn, workflow);
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(result.visitedNodeIds).toEqual(["start", "decide", "human"]);
|
||||
});
|
||||
|
||||
it("degrades faulty node handlers before falling through to default handler", async () => {
|
||||
const extensionKey = workflowExtensionRegistryId("node-plugin", "decision");
|
||||
getWorkflowExtensionRegistry().register("node-plugin", {
|
||||
extensionId: "decision",
|
||||
name: "Decision",
|
||||
kind: "node-handler",
|
||||
nodeKind: "prompt",
|
||||
schemaVersion: WORKFLOW_EXTENSION_SCHEMA_VERSION,
|
||||
fallback: "degradeToDefault",
|
||||
handle: vi.fn().mockRejectedValue(new Error("handler failed")),
|
||||
});
|
||||
const workflow: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "node-extension",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "decide", kind: "prompt", column: "work", extensions: { [extensionKey]: {} } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "decide" },
|
||||
{ from: "decide", to: "end" },
|
||||
],
|
||||
};
|
||||
const prompt = vi.fn(async () => ({ outcome: "success" as const }));
|
||||
const executor = new WorkflowGraphExecutor({ handlers: { prompt } });
|
||||
|
||||
const result = await executor.run({ id: "FN-NODE" } as TaskDetail, settingsOn, workflow);
|
||||
|
||||
expect(result.outcome).toBe("success");
|
||||
expect(prompt).toHaveBeenCalledWith(expect.objectContaining({ id: "decide" }), expect.any(Object));
|
||||
expect(getWorkflowExtensionRegistry().get(extensionKey)?.degraded).toMatchObject({
|
||||
reason: "runtime-fault",
|
||||
message: "handler failed",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8159,14 +8159,18 @@ export async function aiMergeTask(
|
||||
const providerManualRoute =
|
||||
autoMergeFacts.route === "manual-required" ||
|
||||
autoMergeFacts.route === "blocked";
|
||||
const initialState = task.autoMerge === false || providerManualRoute ? "manual-required" : "queued";
|
||||
const autoMergeManuallyGated =
|
||||
task.autoMerge === false ||
|
||||
(settings.autoMerge === false && task.autoMerge !== true) ||
|
||||
providerManualRoute;
|
||||
const initialState = autoMergeManuallyGated ? "manual-required" : "queued";
|
||||
const existingRecord = store.getMergeRequestRecord(task.id);
|
||||
const currentState = existingRecord?.state ?? initialState;
|
||||
if (!existingRecord) {
|
||||
store.upsertMergeRequestRecord(task.id, { state: initialState });
|
||||
}
|
||||
|
||||
if (task.autoMerge !== false) {
|
||||
if (!autoMergeManuallyGated) {
|
||||
if (currentState === "retrying") {
|
||||
store.transitionMergeRequestState(task.id, "queued");
|
||||
store.transitionMergeRequestState(task.id, "running");
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
getTraitRegistry,
|
||||
getWorkflowExtensionRegistry,
|
||||
resolveWorkflowIrForTask,
|
||||
workflowExtensionRegistryId,
|
||||
} from "@fusion/core";
|
||||
import { createLogger, executorLog } from "./logger.js";
|
||||
import type { WorkflowCustomNodeRunner } from "./workflow-node-handlers.js";
|
||||
@@ -415,13 +416,8 @@ export class PluginRunner {
|
||||
|
||||
getPluginWorkflowExtensions(): Array<{ pluginId: string; extension: WorkflowExtensionContribution }> {
|
||||
if (!this.cachedWorkflowExtensions || this.cachedWorkflowExtensions.version !== this.workflowExtensionsCacheVersion) {
|
||||
const loader = this.options.pluginLoader as unknown as {
|
||||
getPluginWorkflowExtensions?: () => Array<{ pluginId: string; extension: WorkflowExtensionContribution }>;
|
||||
};
|
||||
this.cachedWorkflowExtensions = {
|
||||
extensions: typeof loader.getPluginWorkflowExtensions === "function"
|
||||
? loader.getPluginWorkflowExtensions()
|
||||
: [],
|
||||
extensions: this.options.pluginLoader.getPluginWorkflowExtensions(),
|
||||
version: this.workflowExtensionsCacheVersion,
|
||||
};
|
||||
}
|
||||
@@ -531,8 +527,6 @@ export class PluginRunner {
|
||||
|
||||
for (const [pluginId, contributions] of byPlugin) {
|
||||
try {
|
||||
const previous = this.registeredPluginWorkflowExtensionIds.get(pluginId);
|
||||
if (previous) unregisterPluginWorkflowExtensions(registry, previous);
|
||||
const ids = registerPluginWorkflowExtensions({ registry, pluginId, contributions });
|
||||
this.registeredPluginWorkflowExtensionIds.set(pluginId, ids);
|
||||
} catch (err) {
|
||||
@@ -580,7 +574,7 @@ export class PluginRunner {
|
||||
if (tracked && tracked.length > 0) return tracked;
|
||||
return this.getPluginWorkflowExtensions()
|
||||
.filter((entry) => entry.pluginId === pluginId)
|
||||
.map((entry) => `plugin:${pluginId}:${entry.extension.extensionId}`);
|
||||
.map((entry) => workflowExtensionRegistryId(pluginId, entry.extension.extensionId));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -12,9 +12,7 @@ export function registerPluginWorkflowExtensions(params: {
|
||||
const registered: string[] = [];
|
||||
for (const contribution of params.contributions) {
|
||||
const id = workflowExtensionRegistryId(params.pluginId, contribution.extensionId);
|
||||
if (!params.registry.get(id)) {
|
||||
params.registry.register(params.pluginId, contribution);
|
||||
}
|
||||
params.registry.upsert(params.pluginId, contribution);
|
||||
registered.push(id);
|
||||
}
|
||||
return registered;
|
||||
|
||||
@@ -494,7 +494,7 @@ export class WorkflowGraphExecutor {
|
||||
}
|
||||
return {
|
||||
outcome: "success",
|
||||
value: result.outcome.slice("outcome:".length),
|
||||
value: result.value ?? result.outcome.slice("outcome:".length),
|
||||
contextPatch: result.contextPatch,
|
||||
};
|
||||
}
|
||||
@@ -525,7 +525,19 @@ export class WorkflowGraphExecutor {
|
||||
});
|
||||
return this.normalizePluginNodeResult(result);
|
||||
} catch (error) {
|
||||
if (extension.fallback === "degradeToDefault") continue;
|
||||
if (extension.fallback === "degradeToDefault") {
|
||||
try {
|
||||
registry.degrade(
|
||||
[definition.id],
|
||||
"runtime-fault",
|
||||
error instanceof Error ? error.message : String(error),
|
||||
);
|
||||
} catch {
|
||||
// Degradation is best-effort; falling through to the default node
|
||||
// handler is still the correct fallback for this invocation.
|
||||
}
|
||||
continue;
|
||||
}
|
||||
return {
|
||||
outcome: "failure",
|
||||
value: "plugin-node-handler-error",
|
||||
|
||||
Reference in New Issue
Block a user