FN-8794: add workflow review-kind markers
Classify workflow review nodes so review feedback can target their persisted results. - validate and type explicit plan/code review-kind markers in workflow IR - expose review-kind selection in the workflow editor and document the authoring contract - persist markers through task workflow routes and graph execution with regression coverage Files changed: .changeset/fn-8794-review-kind.md | 7 + docs/dashboard-guide.md | 4 + docs/workflow-steps.md | 10 + .../__tests__/workflow-ir-optional-group.test.ts | 17 ++ packages/core/src/__tests__/workflow-ir.test.ts | 83 +++++++++ packages/core/src/index.gate.ts | 2 +- packages/core/src/index.ts | 2 +- packages/core/src/types.ts | 2 + packages/core/src/types/workflow/workflow-steps.ts | 8 + .../src/workflows/builtin-code-review-group.ts | 1 + .../src/workflows/builtin-plan-review-group.ts | 1 + packages/core/src/workflows/workflow-ir-types.ts | 5 + packages/core/src/workflows/workflow-ir.ts | 24 +++ .../app/components/WorkflowNodeEditor.tsx | 17 ++ .../__tests__/WorkflowNodeEditor.test.tsx | 206 +++++++++++++++++++++ .../dashboard/src/__tests__/routes-tasks.test.ts | 166 ++++++++++++++++- .../src/routes/register-task-workflow-routes.ts | 82 ++++++-- .../workflow-graph-optional-group.test.ts | 177 +++++++++++++++++- .../src/workflows/workflow-graph-executor.ts | 20 +- 19 files changed, 801 insertions(+), 33 deletions(-) Fusion-Task-Id: FN-8794 Fusion-Task-Lineage: a7752469-59bc-406f-82e8-6ffde6c392ef Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-8794-review-kind.md
Normal file
7
.changeset/fn-8794-review-kind.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Classify custom workflow review nodes for direct Review-tab feedback.
|
||||
category: feature
|
||||
dev: Persists explicit plan/code reviewKind snapshots on supported top-level workflow results.
|
||||
@@ -2404,3 +2404,7 @@ When a completed task is successfully reverted, Fusion removes it from ordinary
|
||||
### Todo Lists plugin enablement
|
||||
|
||||
Todo Lists is an optional first-party plugin. Enable `fusion-plugin-todos` for a project in the Plugins settings to make its plugin-discovered Todos destination and `/api/plugins/fusion-plugin-todos/todos/*` API available for that project. Disabled or uninstalled projects expose neither route nor navigation entry; enablement is per project and replaces the former experimental Todo setting.
|
||||
|
||||
## Workflow direct-review items
|
||||
|
||||
The Review tab shows a custom workflow result only when its selected workflow declares the exact top-level node and result source, the result explicitly snapshots `reviewKind: "plan"` or `"code"`, it is current, has `passed` or `failed` status, is not bypassed or superseded, and contains nonblank output or notes. Pending, skipped, historical prior attempts, blank results, and records without that declared top-level identity (including template instances) are not selectable or addressable. Node-ID punctuation alone does not identify a template instance. Existing historical `plan-review` and `code-review` results retain narrow compatibility; Fusion does not infer or backfill custom review meaning from names, verdicts, prose, or gate settings.
|
||||
|
||||
@@ -948,3 +948,13 @@ When the project or task-level [`reviewArtifacts`](./settings-reference.md) poli
|
||||
The MVP requires a persisted `review-artifact-scenario` task document containing JSON such as `{ "baseUrl": "http://127.0.0.1:5173", "targetRoute": "/settings" }`. The URL must be `http` or `https` on `127.0.0.1`, `localhost`, or `::1`; missing, malformed, remote, or unreachable scenarios are skipped. Fusion does not start or manage this server. An optional `flowScript` identifier is accepted for future registered flows; unknown identifiers use the default navigate-and-settle recording.
|
||||
|
||||
Capture uses local Chromium through `playwright-core` and records WebM. Recording is capped at 15 seconds (normally three seconds); output over the size cap is rejected without artifact registration rather than trimmed or re-encoded. A successful recording is registered through the normal artifact registry as `type="video"`, `mimeType="video/webm"`, and linked to its task, so existing review-artifact galleries display it. When review artifacts are enabled, executor-generated verification videos also appear in the top-level Quality hub.
|
||||
|
||||
## Direct-review kind for custom nodes
|
||||
|
||||
A top-level `prompt`, `gate`, `script`, or `optional-group` node may set `config.reviewKind` to exactly `"plan"` or `"code"`. This is an explicit metadata declaration for the direct Review tab; it does not change routing, gate mode, retries, merge blocking, or execution behavior. For example:
|
||||
|
||||
```json
|
||||
{ "id": "architecture-review", "kind": "prompt", "config": { "name": "Architecture review", "reviewKind": "code", "prompt": "Review the proposed architecture." } }
|
||||
```
|
||||
|
||||
When a marked supported node runs, its pending and terminal workflow-step result snapshots the declared value. Omission means the node is **not** a direct review, regardless of its ID, label, verdict, output prose, phase, or gate mode. Markers are rejected on foreach and loop templates and optional-group source/template nodes: those executions do not yet have an instance-safe current-result or Review-tab address contract.
|
||||
|
||||
@@ -93,6 +93,23 @@ describe("optional-group validation", () => {
|
||||
expect(resolveOptionalStepRevisionBudget("sometimes", 3)).toEqual({ unbounded: false, max: 3 });
|
||||
});
|
||||
|
||||
it.each(["plan", "code"] as const)("rejects valid reviewKind in every optional-group template node as unsupported placement", (reviewKind) => {
|
||||
const template = groupTemplate();
|
||||
template.nodes = template.nodes.map((node, index) => ({
|
||||
...node,
|
||||
id: `nested-review-${index}`,
|
||||
kind: index === 0 ? "prompt" : "gate",
|
||||
config: { ...node.config, reviewKind },
|
||||
}));
|
||||
expect(() => parseWorkflowIr(groupIr({ template }))).toThrow(/nested-review-0.*unsupported nested template placement/);
|
||||
});
|
||||
|
||||
it.each(["", "review", true, null])("rejects malformed optional-group template reviewKind before placement", (reviewKind) => {
|
||||
const template = groupTemplate();
|
||||
template.nodes[0] = { ...template.nodes[0], id: "nested-review", config: { reviewKind } };
|
||||
expect(() => parseWorkflowIr(groupIr({ template }))).toThrow(/nested-review.*invalid reviewKind/);
|
||||
});
|
||||
|
||||
it("rejects an empty template", () => {
|
||||
expect(() => parseWorkflowIr(groupIr({ template: { nodes: [], edges: [] } }))).toThrow(/non-empty/);
|
||||
});
|
||||
|
||||
@@ -222,6 +222,89 @@ describe("parseWorkflowIr — v2 columns & placement", () => {
|
||||
// `optionalSteps` key on an old v2 row is now TOLERATED — no longer validated or
|
||||
// required — so old rows still parse as v2 (optional steps are graph-native
|
||||
// `optional-group` nodes now).
|
||||
describe("parseWorkflowIr — review kind markers", () => {
|
||||
const supportedReviewKinds = ["prompt", "gate", "script", "optional-group"] as const;
|
||||
const configFor = (kind: typeof supportedReviewKinds[number], reviewKind?: unknown) => ({
|
||||
...(kind === "optional-group" ? { template: { nodes: [{ id: "inner", kind: "prompt" }], edges: [] } } : {}),
|
||||
...(reviewKind === undefined ? {} : { reviewKind }),
|
||||
});
|
||||
|
||||
it.each(supportedReviewKinds)("accepts absence and round-trips both closed markers for top-level %s nodes", (kind) => {
|
||||
for (const reviewKind of [undefined, "plan", "code"] as const) {
|
||||
const ir = v2(
|
||||
[{ id: "work", name: "Work", traits: [] }],
|
||||
[{ id: "start", kind: "start" }, { id: "review", kind, config: configFor(kind, reviewKind) }, { id: "end", kind: "end" }],
|
||||
[{ from: "start", to: "review" }, { from: "review", to: "end" }],
|
||||
);
|
||||
const parsed = parseWorkflowIr(ir) as WorkflowIrV2;
|
||||
expect(parsed.nodes.find((node) => node.id === "review")?.config?.reviewKind).toBe(reviewKind);
|
||||
expect(parseWorkflowIr(serializeWorkflowIr(parsed))).toEqual(parsed);
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects every malformed reviewKind value for every supported top-level kind", () => {
|
||||
for (const kind of supportedReviewKinds) {
|
||||
for (const reviewKind of ["", "review", true, null]) {
|
||||
const ir = v2(
|
||||
[{ id: "work", name: "Work", traits: [] }],
|
||||
[{ id: "start", kind: "start" }, { id: "declared-review", kind, config: configFor(kind, reviewKind) }, { id: "end", kind: "end" }],
|
||||
[{ from: "start", to: "declared-review" }, { from: "declared-review", to: "end" }],
|
||||
);
|
||||
expect(() => parseWorkflowIr(ir)).toThrow(/declared-review.*invalid reviewKind/);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects a valid marker on unsupported top-level nodes", () => {
|
||||
const ir = v2(
|
||||
[{ id: "work", name: "Work", traits: [] }],
|
||||
[{ id: "start", kind: "start" }, { id: "not-a-review", kind: "hold", config: { release: "manual", reviewKind: "plan" } }, { id: "end", kind: "end" }],
|
||||
[{ from: "start", to: "not-a-review" }, { from: "not-a-review", to: "end" }],
|
||||
);
|
||||
expect(() => parseWorkflowIr(ir)).toThrow(/not-a-review.*unsupported node kind/);
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:WorkflowReviewKind 2026-08-05-03:08:
|
||||
* Foreach and loop template nodes have materialized execution identities, not a
|
||||
* top-level current-result/address contract. Validate malformed values before
|
||||
* placement so imported config cannot turn either error into silent omission.
|
||||
*/
|
||||
it.each([
|
||||
["foreach", "plan", /foreach-child.*unsupported nested template placement/],
|
||||
["foreach", "invalid", /foreach-child.*invalid reviewKind/],
|
||||
["loop", "code", /loop-child.*unsupported nested template placement/],
|
||||
["loop", "", /loop-child.*invalid reviewKind/],
|
||||
["loop", true, /loop-child.*invalid reviewKind/],
|
||||
] as const)("rejects %s template reviewKind %j with the owning node diagnostic", (container, reviewKind, diagnostic) => {
|
||||
const template = container === "foreach"
|
||||
? {
|
||||
nodes: [
|
||||
{ id: "foreach-child", kind: "prompt", config: { seam: "step-execute", reviewKind } },
|
||||
{ id: "foreach-exit", kind: "step-review", config: { type: "code" } },
|
||||
],
|
||||
edges: [{ from: "foreach-child", to: "foreach-exit", condition: "success" }],
|
||||
}
|
||||
: { nodes: [{ id: "loop-child", kind: "prompt", config: { reviewKind } }], edges: [] };
|
||||
const nodes: WorkflowIrNode[] = container === "foreach"
|
||||
? [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "parse", kind: "parse-steps", config: { artifact: "PROMPT.md", parser: "step-headings" } },
|
||||
{ id: "each", kind: "foreach", config: { source: "task-steps", template } },
|
||||
{ id: "end", kind: "end" },
|
||||
]
|
||||
: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "repeat", kind: "loop", config: { maxIterations: 2, exitWhen: { type: "output-contains", value: "DONE" }, template } },
|
||||
{ id: "end", kind: "end" },
|
||||
];
|
||||
const edges: WorkflowIrEdge[] = container === "foreach"
|
||||
? [{ from: "start", to: "parse" }, { from: "parse", to: "each" }, { from: "each", to: "end" }]
|
||||
: [{ from: "start", to: "repeat" }, { from: "repeat", to: "end" }];
|
||||
expect(() => parseWorkflowIr(v2([{ id: "work", name: "Work", traits: [] }], nodes, edges))).toThrow(diagnostic);
|
||||
});
|
||||
});
|
||||
|
||||
describe("parseWorkflowIr — legacy optionalSteps tolerated", () => {
|
||||
const columns = DEFAULT_WORKFLOW_COLUMN_IDS.map((id) => ({ id, name: id, traits: [] }));
|
||||
const base = (): WorkflowIrV2 => v2(
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -263,6 +263,7 @@ import type {
|
||||
WorkflowStepToolMode,
|
||||
WorkflowStepGateMode,
|
||||
WorkflowStepPhase,
|
||||
WorkflowReviewKind,
|
||||
WorkflowStep,
|
||||
NtfyNotificationEvent,
|
||||
NotificationEvent,
|
||||
@@ -282,6 +283,7 @@ export type {
|
||||
WorkflowStepToolMode,
|
||||
WorkflowStepGateMode,
|
||||
WorkflowStepPhase,
|
||||
WorkflowReviewKind,
|
||||
WorkflowStep,
|
||||
NtfyNotificationEvent,
|
||||
NotificationEvent,
|
||||
|
||||
@@ -34,6 +34,9 @@ export type WorkflowStepGateMode = "gate" | "advisory";
|
||||
/** Lifecycle phase for workflow step execution. */
|
||||
export type WorkflowStepPhase = "pre-merge" | "post-merge";
|
||||
|
||||
/** Closed snapshot of an author-declared direct-review category. */
|
||||
export type WorkflowReviewKind = "plan" | "code";
|
||||
|
||||
export interface WorkflowStep {
|
||||
/** Unique identifier (e.g., "WS-001") */
|
||||
id: string;
|
||||
@@ -247,6 +250,11 @@ export interface WorkflowStepResult {
|
||||
source?: "optional-group" | "node";
|
||||
/** Execution status */
|
||||
status: "passed" | "failed" | "advisory_failure" | "skipped" | "pending";
|
||||
/**
|
||||
* Author-declared direct-review category snapshotted when a supported top-level
|
||||
* graph node starts. Absent preserves historical and non-review semantics.
|
||||
*/
|
||||
reviewKind?: WorkflowReviewKind;
|
||||
/** Output from the workflow step agent (findings, errors, etc.) */
|
||||
output?: string;
|
||||
/**
|
||||
|
||||
@@ -97,6 +97,7 @@ export function codeReviewOptionalGroupNode(
|
||||
column,
|
||||
config: {
|
||||
name: CODE_REVIEW_NAME,
|
||||
reviewKind: "code",
|
||||
// Default-ON: runs for every coding task by default, but operators can toggle it
|
||||
// off per task (remove `code-review` from enabledWorkflowSteps).
|
||||
defaultOn: options.defaultOn ?? true,
|
||||
|
||||
@@ -81,6 +81,7 @@ export function planReviewOptionalGroupNode(
|
||||
...(column ? { column } : {}),
|
||||
config: {
|
||||
name: PLAN_REVIEW_NAME,
|
||||
reviewKind: "plan",
|
||||
defaultOn: options.defaultOn ?? true,
|
||||
/*
|
||||
* FNXC:WorkflowRemediation 2026-06-29-12:14:
|
||||
|
||||
@@ -47,6 +47,8 @@ export type WorkflowIrNodeKind =
|
||||
| "ask-user"
|
||||
| "exit-gate";
|
||||
|
||||
import type { WorkflowReviewKind } from "../types/workflow/workflow-steps.js";
|
||||
|
||||
export interface WorkflowIrNode {
|
||||
id: string;
|
||||
kind: WorkflowIrNodeKind;
|
||||
@@ -54,6 +56,7 @@ export interface WorkflowIrNode {
|
||||
column?: string;
|
||||
/** Plugin-namespaced extension metadata keyed as `plugin:<pluginId>:<extensionId>`. */
|
||||
extensions?: Record<string, Record<string, unknown>>;
|
||||
/** Open node config; supported top-level review producers may set `reviewKind`. */
|
||||
config?: Record<string, unknown>;
|
||||
}
|
||||
|
||||
@@ -217,6 +220,8 @@ Built-in Plan Review/spec and Code Review groups have workflow-value overrides (
|
||||
* graph attempt. Unlike `foreach`/`loop`, the template has no internal iteration;
|
||||
* an outer remediation edge may re-enter it subject to `maxRevisions`. */
|
||||
export interface WorkflowOptionalGroupConfig {
|
||||
/** Optional direct-review classification, legal only on the top-level group. */
|
||||
reviewKind?: WorkflowReviewKind;
|
||||
/** Workflow-author default for whether new tasks enable this group. */
|
||||
defaultOn?: boolean;
|
||||
/** Display name for the group (editor + per-task toggle surfaces). */
|
||||
|
||||
@@ -521,6 +521,7 @@ function validateForeach(
|
||||
// top-level column id (column-agent plan KTD-1) — otherwise a dangling reference
|
||||
// is a silent no-binding no-op at runtime instead of a typed authoring error.
|
||||
for (const inner of templateNodes) {
|
||||
validateReviewKind(inner, "nested");
|
||||
if (inner.kind === "foreach" || inner.kind === "loop") {
|
||||
throw new WorkflowIrError(
|
||||
`foreach node '${node.id}' template may not contain nested loop/foreach ('${inner.id}')`,
|
||||
@@ -664,6 +665,7 @@ function validateLoop(
|
||||
);
|
||||
}
|
||||
for (const inner of templateNodes) {
|
||||
validateReviewKind(inner, "nested");
|
||||
if (inner.kind === "loop" || inner.kind === "foreach") {
|
||||
throw new WorkflowIrError(
|
||||
`loop node '${node.id}' template may not contain nested loop/foreach ('${inner.id}')`,
|
||||
@@ -789,6 +791,7 @@ function validateOptionalGroup(
|
||||
throw new WorkflowIrError(`optional-group node '${node.id}' template has duplicate node ids`);
|
||||
}
|
||||
for (const inner of templateNodes) {
|
||||
validateReviewKind(inner, "nested");
|
||||
if (inner.kind === "loop" || inner.kind === "foreach" || inner.kind === "optional-group") {
|
||||
throw new WorkflowIrError(
|
||||
`optional-group node '${node.id}' template may not contain nested loop/foreach/optional-group ('${inner.id}')`,
|
||||
@@ -1638,6 +1641,26 @@ function validateColumnRecovery(column: WorkflowIrColumn): void {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:WorkflowReviewKind 2026-08-05-02:31:
|
||||
* Direct-review semantics are author-declared data, never inferred from a node label,
|
||||
* verdict, or gate behavior. Template executions lack a stable persisted current-result
|
||||
* identity, so valid markers there fail separately after malformed values fail first.
|
||||
*/
|
||||
function validateReviewKind(node: WorkflowIrNode, placement: "top-level" | "nested"): void {
|
||||
const value = node.config?.reviewKind;
|
||||
if (value === undefined) return;
|
||||
if (value !== "plan" && value !== "code") {
|
||||
throw new WorkflowIrError(`Workflow node '${node.id}' has invalid reviewKind '${String(value)}'; expected 'plan' or 'code'`);
|
||||
}
|
||||
if (placement === "nested") {
|
||||
throw new WorkflowIrError(`Workflow node '${node.id}' has reviewKind in an unsupported nested template placement`);
|
||||
}
|
||||
if (node.kind !== "prompt" && node.kind !== "gate" && node.kind !== "script" && node.kind !== "optional-group") {
|
||||
throw new WorkflowIrError(`Workflow node '${node.id}' has reviewKind on unsupported node kind '${node.kind}'`);
|
||||
}
|
||||
}
|
||||
|
||||
function validateV2(ir: WorkflowIrV2): void {
|
||||
validateColumns(ir);
|
||||
|
||||
@@ -1673,6 +1696,7 @@ function validateV2(ir: WorkflowIrV2): void {
|
||||
const nodesById = new Map(ir.nodes.map((n) => [n.id, n]));
|
||||
|
||||
for (const node of ir.nodes) {
|
||||
validateReviewKind(node, "top-level");
|
||||
validateExtensionMetadata(`Workflow node '${node.id}'`, node.extensions);
|
||||
if (node.column !== undefined && !columnIds.has(node.column)) {
|
||||
throw new WorkflowIrError(
|
||||
|
||||
@@ -4323,6 +4323,23 @@ function InnerEditor({
|
||||
) : null}
|
||||
|
||||
<fieldset className="wf-inspector-fields" disabled={isBuiltin}>
|
||||
{/* FNXC:WorkflowReviewKind 2026-08-05-02:31: Only top-level result-producing
|
||||
nodes have an instance-safe current-result contract. Clearing this selector
|
||||
passes undefined through config cleanup instead of serializing a sentinel. */}
|
||||
{!selectedNode.parentId && (selectedNode.data.kind === "prompt" || selectedNode.data.kind === "gate" || selectedNode.data.kind === "script" || selectedNode.data.kind === "optional-group") ? (
|
||||
<label className="wf-field">
|
||||
<span>{t("workflowNodes.reviewKind", "Review kind")}</span>
|
||||
<select
|
||||
data-testid="wf-review-kind"
|
||||
value={String(selectedNode.data.config?.reviewKind ?? "")}
|
||||
onChange={(e) => updateSelectedData({ config: { reviewKind: e.target.value || undefined } })}
|
||||
>
|
||||
<option value="">{t("workflowNodes.notAReview", "Not a review")}</option>
|
||||
<option value="plan">{t("workflowNodes.planReview", "Plan review")}</option>
|
||||
<option value="code">{t("workflowNodes.codeReview", "Code review")}</option>
|
||||
</select>
|
||||
</label>
|
||||
) : null}
|
||||
{selectedNode.data.kind === "prompt" ? (
|
||||
<>
|
||||
<label className="wf-field">
|
||||
|
||||
@@ -2148,6 +2148,41 @@ function stepwiseDef(): WorkflowDefinition {
|
||||
/** A v2 workflow with an optional-group container (defaultOn:false) holding one
|
||||
* template child, so the editor's optional-group surfaces have something to
|
||||
* render, toggle, and delete. */
|
||||
function topLevelReviewDef(kind: "prompt" | "gate" | "script", reviewKind?: "plan" | "code"): WorkflowDefinition {
|
||||
const definition = optionalGroupDef();
|
||||
definition.id = `WF-${kind}`;
|
||||
definition.name = `${kind} review`;
|
||||
definition.ir.nodes = [
|
||||
{ id: "start", kind: "start", column: "plan" },
|
||||
{ id: "review", kind, column: "in-progress", config: { ...(reviewKind ? { reviewKind } : {}) } },
|
||||
{ id: "end", kind: "end", column: "done" },
|
||||
];
|
||||
definition.ir.edges = [{ from: "start", to: "review" }, { from: "review", to: "end" }];
|
||||
return definition;
|
||||
}
|
||||
|
||||
function loopTemplateDef(): WorkflowDefinition {
|
||||
const definition = optionalGroupDef();
|
||||
definition.id = "WF-LOOP";
|
||||
definition.name = "Loop template";
|
||||
definition.ir.nodes = [
|
||||
{ id: "start", kind: "start", column: "plan" },
|
||||
{
|
||||
id: "repeat",
|
||||
kind: "loop",
|
||||
column: "in-progress",
|
||||
config: {
|
||||
maxIterations: 1,
|
||||
exitWhen: { type: "output-contains", value: "DONE" },
|
||||
template: { nodes: [{ id: "inner", kind: "prompt", config: { prompt: "loop work" } }], edges: [] },
|
||||
},
|
||||
},
|
||||
{ id: "end", kind: "end", column: "done" },
|
||||
];
|
||||
definition.ir.edges = [{ from: "start", to: "repeat" }, { from: "repeat", to: "end" }];
|
||||
return definition;
|
||||
}
|
||||
|
||||
function optionalGroupDef(): WorkflowDefinition {
|
||||
return {
|
||||
id: "WF-OPT",
|
||||
@@ -2298,6 +2333,177 @@ describe("WorkflowNodeEditor — U8 step-inversion authoring", () => {
|
||||
expect(template.nodes).toHaveLength(1);
|
||||
});
|
||||
|
||||
it.each(["prompt", "gate", "script"] as const)("renders, saves, and reopens review kind for top-level %s nodes", async (kind) => {
|
||||
const definition = topLevelReviewDef(kind, "plan");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...definition, ...(updates as object) }));
|
||||
|
||||
const renderEditor = () => render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
renderEditor();
|
||||
const node = await waitFor(() => {
|
||||
const candidate = document.querySelector(`.react-flow__node[data-id="review"]`);
|
||||
expect(candidate).toBeInTheDocument();
|
||||
return candidate as HTMLElement;
|
||||
});
|
||||
fireEvent.click(node);
|
||||
const reviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reviewKind.value).toBe("plan");
|
||||
fireEvent.change(reviewKind, { target: { value: "code" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalled());
|
||||
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
|
||||
const savedIr = (updates as { ir: WorkflowDefinition["ir"] }).ir;
|
||||
const saved = savedIr.nodes.find((candidate) => candidate.id === "review");
|
||||
expect(saved?.config?.reviewKind).toBe("code");
|
||||
|
||||
cleanup();
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([{ ...definition, ir: savedIr }]);
|
||||
renderEditor();
|
||||
fireEvent.click(await waitFor(() => {
|
||||
const candidate = document.querySelector(`.react-flow__node[data-id="review"]`);
|
||||
expect(candidate).toBeInTheDocument();
|
||||
return candidate as HTMLElement;
|
||||
}));
|
||||
expect((await screen.findByTestId("wf-review-kind") as HTMLSelectElement).value).toBe("code");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["optional-group", optionalGroupDef(), "opt", "verify"],
|
||||
["foreach", stepwiseDef(), "loop", "exec"],
|
||||
["loop", loopTemplateDef(), "repeat", "inner"],
|
||||
] as const)("does not render review kind for %s template nodes", async (_containerKind, definition, parentId, childId) => {
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
const templateNode = await waitFor(() => {
|
||||
const candidate = document.querySelector(`.react-flow__node[data-id="${foreachChildFlowId(parentId, childId)}"]`);
|
||||
expect(candidate).toBeInTheDocument();
|
||||
return candidate as HTMLElement;
|
||||
});
|
||||
fireEvent.click(templateNode);
|
||||
expect(screen.queryByTestId("wf-review-kind")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:WorkflowReviewKind 2026-08-05-03:32:
|
||||
* The optional-group inspector is a top-level authoring surface. Saving and
|
||||
* reopening must preserve an explicit marker, while Not a review deletes it
|
||||
* instead of serializing a false review sentinel.
|
||||
*/
|
||||
it("edits, reopens, and clears the top-level optional-group review kind without a sentinel", async () => {
|
||||
const definition = optionalGroupDef();
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...definition, ...(updates as object) }));
|
||||
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByTestId("wf-node-optional-group"));
|
||||
const reviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reviewKind.value).toBe("");
|
||||
fireEvent.change(reviewKind, { target: { value: "code" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalled());
|
||||
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
|
||||
const savedIr = (updates as { ir: WorkflowDefinition["ir"] }).ir;
|
||||
expect(savedIr.nodes.find((node) => node.id === "opt")?.config?.reviewKind).toBe("code");
|
||||
|
||||
cleanup();
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([{ ...definition, ir: savedIr }]);
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByTestId("wf-node-optional-group"));
|
||||
const reopenedReviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reopenedReviewKind.value).toBe("code");
|
||||
fireEvent.change(reopenedReviewKind, { target: { value: "" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(2));
|
||||
const [, clearedUpdates] = vi.mocked(updateWorkflow).mock.calls[1];
|
||||
const clearedIr = (clearedUpdates as { ir: WorkflowDefinition["ir"] }).ir;
|
||||
expect(clearedIr.nodes.find((node) => node.id === "opt")?.config).not.toHaveProperty("reviewKind");
|
||||
});
|
||||
|
||||
it("clears reviewKind instead of serializing an empty sentinel", async () => {
|
||||
const definition = optionalGroupDef();
|
||||
const group = definition.ir.nodes.find((node) => node.id === "opt");
|
||||
if (!group) throw new Error("optional group fixture missing");
|
||||
group.config = { ...group.config, reviewKind: "code" };
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...definition, ...(updates as object) }));
|
||||
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByTestId("wf-node-optional-group"));
|
||||
const reviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reviewKind.value).toBe("code");
|
||||
fireEvent.change(reviewKind, { target: { value: "" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalled());
|
||||
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
|
||||
const savedGroup = (updates as { ir: { nodes: Array<{ id: string; config?: Record<string, unknown> }> } }).ir.nodes.find((node) => node.id === "opt");
|
||||
expect(savedGroup?.config).not.toHaveProperty("reviewKind");
|
||||
});
|
||||
|
||||
it.each(["prompt", "gate", "script"] as const)("edits, reopens, and clears review kind in mobile top-level %s detail", async (kind) => {
|
||||
mockWorkflowEditorViewport("mobile");
|
||||
const definition = topLevelReviewDef(kind, "plan");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...definition, ...(updates as object) }));
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByRole("button", { name: `${kind} review` }));
|
||||
const row = await screen.findByTestId("mobile-wf-node-review");
|
||||
fireEvent.click(within(row).getAllByRole("button")[0]);
|
||||
const reviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reviewKind.value).toBe("plan");
|
||||
fireEvent.change(reviewKind, { target: { value: "code" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1));
|
||||
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
|
||||
const savedIr = (updates as { ir: WorkflowDefinition["ir"] }).ir;
|
||||
expect(savedIr.nodes.find((node) => node.id === "review")?.config?.reviewKind).toBe("code");
|
||||
|
||||
cleanup();
|
||||
mockWorkflowEditorViewport("mobile");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([{ ...definition, ir: savedIr }]);
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByRole("button", { name: `${kind} review` }));
|
||||
fireEvent.click(within(await screen.findByTestId("mobile-wf-node-review")).getAllByRole("button")[0]);
|
||||
const reopenedReviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reopenedReviewKind.value).toBe("code");
|
||||
fireEvent.change(reopenedReviewKind, { target: { value: "" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(2));
|
||||
const [, clearedUpdates] = vi.mocked(updateWorkflow).mock.calls[1];
|
||||
expect((clearedUpdates as { ir: WorkflowDefinition["ir"] }).ir.nodes.find((node) => node.id === "review")?.config ?? {}).not.toHaveProperty("reviewKind");
|
||||
});
|
||||
|
||||
it("edits, reopens, and clears review kind in the mobile optional-group detail", async () => {
|
||||
mockWorkflowEditorViewport("mobile");
|
||||
const definition = optionalGroupDef();
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([definition]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...definition, ...(updates as object) }));
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Optional" }));
|
||||
fireEvent.click(await screen.findByTestId("wf-node-optional-group"));
|
||||
const reviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reviewKind.value).toBe("");
|
||||
fireEvent.change(reviewKind, { target: { value: "code" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(1));
|
||||
const [, updates] = vi.mocked(updateWorkflow).mock.calls[0];
|
||||
const savedIr = (updates as { ir: WorkflowDefinition["ir"] }).ir;
|
||||
expect(savedIr.nodes.find((node) => node.id === "opt")?.config?.reviewKind).toBe("code");
|
||||
|
||||
cleanup();
|
||||
mockWorkflowEditorViewport("mobile");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([{ ...definition, ir: savedIr }]);
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
fireEvent.click(await screen.findByRole("button", { name: "Optional" }));
|
||||
fireEvent.click(await screen.findByTestId("wf-node-optional-group"));
|
||||
const reopenedReviewKind = await screen.findByTestId("wf-review-kind") as HTMLSelectElement;
|
||||
expect(reopenedReviewKind.value).toBe("code");
|
||||
fireEvent.change(reopenedReviewKind, { target: { value: "" } });
|
||||
fireEvent.click(screen.getByText("Save").closest("button")!);
|
||||
await waitFor(() => expect(updateWorkflow).toHaveBeenCalledTimes(2));
|
||||
const [, clearedUpdates] = vi.mocked(updateWorkflow).mock.calls[1];
|
||||
expect((clearedUpdates as { ir: WorkflowDefinition["ir"] }).ir.nodes.find((node) => node.id === "opt")?.config).not.toHaveProperty("reviewKind");
|
||||
});
|
||||
|
||||
it("edits optional-group maxRevisions and unbounded revision mode", async () => {
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([optionalGroupDef()]);
|
||||
vi.mocked(updateWorkflow).mockImplementation(async (_id, updates) => ({ ...optionalGroupDef(), ...(updates as object) }));
|
||||
|
||||
@@ -2715,6 +2715,27 @@ describe("POST /tasks/:id/review/address", () => {
|
||||
return app;
|
||||
}
|
||||
|
||||
function authorizeMarkedTopLevelReviewNodes(...nodes: Array<{ id: string; kind?: "prompt" | "gate" | "script" | "optional-group" }>) {
|
||||
const workflowId = "WF-review-kind";
|
||||
const workflowIr = {
|
||||
version: "v1" as const,
|
||||
name: "Marked review nodes",
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" as const },
|
||||
...nodes.map(({ id, kind = "prompt" }) => ({ id, kind, config: { reviewKind: "code" } })),
|
||||
{ id: "end", kind: "end" as const },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: nodes[0]?.id ?? "end" },
|
||||
...nodes.slice(0, -1).map((node, index) => ({ from: node.id, to: nodes[index + 1]!.id })),
|
||||
...(nodes.length > 0 ? [{ from: nodes[nodes.length - 1]!.id, to: "end" }] : []),
|
||||
],
|
||||
};
|
||||
store.getTaskWorkflowSelection = vi.fn().mockReturnValue({ workflowId, stepIds: [] });
|
||||
store.getTaskWorkflowSelectionAsync = vi.fn().mockResolvedValue({ workflowId, stepIds: [] });
|
||||
store.getWorkflowDefinition = vi.fn().mockResolvedValue({ ir: workflowIr });
|
||||
}
|
||||
|
||||
function mockReviewerBlockLogs() {
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
@@ -2758,33 +2779,166 @@ describe("POST /tasks/:id/review/address", () => {
|
||||
{ workflowStepId: "code-review", workflowStepName: "Code Review", phase: "pre-merge", status: "passed", verdict: "APPROVE_WITH_NOTES", output: "1. Keep the assertion focused.", completedAt: "2026-01-02T00:00:00.000Z" },
|
||||
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", phase: "pre-merge", status: "failed", verdict: "REVISE", notes: "Clarify the rollback plan.", startedAt: "2026-01-03T00:00:00.000Z" },
|
||||
{ workflowStepId: "code-review", workflowStepName: "Code Review", phase: "pre-merge", status: "advisory_failure", verdict: "APPROVE", completedAt: "2026-01-04T00:00:00.000Z" },
|
||||
{ workflowStepId: "code-review", workflowStepName: "Invalid marked builtin", source: "optional-group", status: "advisory_failure", verdict: "REVISE", reviewKind: "invalid" as unknown as "code", output: "Invalid marker must not become legacy compatibility.", completedAt: "2026-01-04T01:00:00.000Z" },
|
||||
{ workflowStepId: "custom-review", workflowStepName: "Custom Review", status: "passed", verdict: "REVISE", output: "Must not be inferred." },
|
||||
{ workflowStepId: "custom-plan", workflowStepName: "Custom Plan", source: "node", status: "failed", reviewKind: "plan", notes: "Persisted marker qualifies this feedback.", completedAt: "2026-01-05T00:00:00.000Z" },
|
||||
{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "pending", verdict: "REVISE" },
|
||||
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "skipped", verdict: "REVISE" },
|
||||
{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "passed", verdict: "REVISE", supersededAt: "2026-01-05T00:00:00.000Z" },
|
||||
{ workflowStepId: "superseded-custom", workflowStepName: "Superseded Custom", source: "node", status: "failed", reviewKind: "code", output: "Must be excluded even when only the reason remains.", supersededReason: "replaced" },
|
||||
{ workflowStepId: "plan-review", workflowStepName: "Plan Review", status: "failed", verdict: "REVISE", bypassedAt: "2026-01-05T00:00:00.000Z" },
|
||||
],
|
||||
log: [{ timestamp: reviewerBlockTimestamp, action: "code review Step 1: REVISE - legacy fallback must not duplicate structured data" }],
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithWorkflowReviews);
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([{ agent: "reviewer", type: "text", text: "## Code Review:\\n### Verdict: REVISE" }]);
|
||||
authorizeMarkedTopLevelReviewNodes({ id: "custom-plan" });
|
||||
|
||||
const first = await REQUEST(buildApp(), "GET", "/api/tasks/FN-009/review");
|
||||
const refreshed = await REQUEST(buildApp(), "POST", "/api/tasks/FN-009/review/refresh");
|
||||
|
||||
expect(first.status).toBe(200);
|
||||
expect(refreshed.status).toBe(200);
|
||||
expect(first.body.items).toHaveLength(3);
|
||||
expect(first.body.items).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ title: "Code Review APPROVE_WITH_NOTES", body: "1. Keep the assertion focused.", verdict: "APPROVE_WITH_NOTES", reviewType: "code" }),
|
||||
expect.objectContaining({ title: "Plan Review REVISE", body: "Clarify the rollback plan.", verdict: "REVISE", reviewType: "plan" }),
|
||||
expect.objectContaining({ verdict: "APPROVE", body: "No written feedback was provided by this review step." }),
|
||||
expect(first.body.items).toHaveLength(4);
|
||||
expect(first.body.items.map((item: { body: string; reviewType: string }) => [item.body, item.reviewType])).toEqual(expect.arrayContaining([
|
||||
["1. Keep the assertion focused.", "code"],
|
||||
["Clarify the rollback plan.", "plan"],
|
||||
["Persisted marker qualifies this feedback.", "plan"],
|
||||
["No written feedback was provided by this review step.", "code"],
|
||||
]));
|
||||
expect(first.body.summary).toEqual(expect.objectContaining({ verdict: "APPROVE" }));
|
||||
expect(first.body.summary).toEqual({ summary: "Custom Plan failed" });
|
||||
expect(first.body.items.map((item: { itemId: string }) => item.itemId)).toEqual(refreshed.body.items.map((item: { itemId: string }) => item.itemId));
|
||||
expect(store.getAgentLogs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not expose materialized template identities even when manually marked", async () => {
|
||||
const taskWithMaterializedResult = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-011",
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "steps#0:step-execute",
|
||||
workflowStepName: "Template review",
|
||||
source: "node",
|
||||
status: "passed",
|
||||
reviewKind: "code",
|
||||
output: "A manually persisted template result must not be addressable.",
|
||||
}],
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(taskWithMaterializedResult);
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
|
||||
const review = await REQUEST(buildApp(), "GET", "/api/tasks/FN-011/review");
|
||||
expect(review.status).toBe(200);
|
||||
expect(review.body.items).toEqual([]);
|
||||
|
||||
const address = await REQUEST(buildApp(), "POST", "/api/tasks/FN-011/review/address", JSON.stringify({
|
||||
selectedItems: [{ id: "workflow-review-forged", source: "reviewer-agent" }],
|
||||
}), { "Content-Type": "application/json" });
|
||||
expect(address.status).toBe(400);
|
||||
});
|
||||
|
||||
it("accepts marked top-level review node ids containing template-like delimiters", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-011-delimiters",
|
||||
column: "in-review",
|
||||
status: "awaiting-user-review",
|
||||
assignedAgentId: null,
|
||||
sessionFile: null,
|
||||
workflowStepResults: [
|
||||
{ workflowStepId: "architecture::review", workflowStepName: "Architecture review", source: "node", status: "passed", reviewKind: "plan", output: "Exact top-level identity is authoritative." },
|
||||
{ workflowStepId: "review#12:code", workflowStepName: "Code review", source: "optional-group", status: "failed", reviewKind: "code", notes: "Punctuation does not imply a template instance." },
|
||||
],
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
authorizeMarkedTopLevelReviewNodes(
|
||||
{ id: "architecture::review", kind: "prompt" },
|
||||
{ id: "review#12:code", kind: "optional-group" },
|
||||
);
|
||||
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-delimiter" });
|
||||
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...task, column: "in-progress", status: null });
|
||||
|
||||
const review = await REQUEST(buildApp(), "GET", "/api/tasks/FN-011-delimiters/review");
|
||||
expect(review.body.items).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ title: "Architecture review passed", reviewType: "plan" }),
|
||||
expect.objectContaining({ title: "Code review failed", reviewType: "code" }),
|
||||
]));
|
||||
|
||||
const address = await REQUEST(buildApp(), "POST", "/api/tasks/FN-011-delimiters/review/address", JSON.stringify({
|
||||
selectedItems: [{ id: review.body.items[0].itemId, source: "reviewer-agent" }],
|
||||
}), { "Content-Type": "application/json" });
|
||||
expect(address.status).toBe(200);
|
||||
});
|
||||
|
||||
it("addresses a marked custom review with a server-owned canonical snapshot", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-012",
|
||||
column: "in-review",
|
||||
status: "awaiting-user-review",
|
||||
assignedAgentId: null,
|
||||
sessionFile: null,
|
||||
reviewState: undefined,
|
||||
workflowStepResults: [{
|
||||
workflowStepId: "custom-code-check",
|
||||
workflowStepName: "Custom code check",
|
||||
source: "node",
|
||||
status: "failed",
|
||||
reviewKind: "code",
|
||||
output: "Use the authoritative persisted feedback.",
|
||||
completedAt: "2026-01-06T00:00:00.000Z",
|
||||
}],
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
authorizeMarkedTopLevelReviewNodes({ id: "custom-code-check" });
|
||||
(store.addSteeringComment as ReturnType<typeof vi.fn>).mockResolvedValue({ id: "sc-custom" });
|
||||
(store.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...task, column: "in-progress", status: null });
|
||||
|
||||
const review = await REQUEST(buildApp(), "GET", "/api/tasks/FN-012/review");
|
||||
expect(review.body.items).toEqual([expect.objectContaining({
|
||||
title: "Custom code check failed",
|
||||
body: "Use the authoritative persisted feedback.",
|
||||
reviewType: "code",
|
||||
sourceMode: "reviewer-agent",
|
||||
})]);
|
||||
const itemId = review.body.items[0].itemId;
|
||||
const address = await REQUEST(buildApp(), "POST", "/api/tasks/FN-012/review/address", JSON.stringify({
|
||||
selectedItems: [{ id: itemId, source: "reviewer-agent", body: "FORGED" }],
|
||||
}), { "Content-Type": "application/json" });
|
||||
|
||||
expect(address.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-012", expect.objectContaining({
|
||||
reviewState: expect.objectContaining({
|
||||
addressing: [expect.objectContaining({ itemId, snapshot: expect.objectContaining({ body: "Use the authoritative persisted feedback." }) })],
|
||||
}),
|
||||
}));
|
||||
});
|
||||
|
||||
it.each([
|
||||
["unmarked", { workflowStepId: "review-by-name", workflowStepName: "Review", source: "node", status: "failed", verdict: "REVISE", output: "lookalike" }],
|
||||
["pending", { workflowStepId: "pending", workflowStepName: "Pending", source: "node", status: "pending", reviewKind: "plan", output: "not terminal" }],
|
||||
["skipped", { workflowStepId: "skipped", workflowStepName: "Skipped", source: "node", status: "skipped", reviewKind: "code", output: "not current" }],
|
||||
["bypassed", { workflowStepId: "bypassed", workflowStepName: "Bypassed", source: "node", status: "failed", reviewKind: "code", output: "not current", bypassReason: "operator" }],
|
||||
["superseded", { workflowStepId: "superseded", workflowStepName: "Superseded", source: "node", status: "passed", reviewKind: "plan", output: "not current", supersededReason: "retry" }],
|
||||
["prior attempt only", { workflowStepId: "prior", workflowStepName: "Prior", source: "node", status: "skipped", priorAttempts: [{ workflowStepId: "prior", workflowStepName: "Prior", source: "node", status: "failed", reviewKind: "code", output: "history only" }] }],
|
||||
["blank", { workflowStepId: "blank", workflowStepName: "Blank", source: "optional-group", status: "passed", reviewKind: "code", output: " ", notes: "" }],
|
||||
["template instance", { workflowStepId: "group::child", workflowStepName: "Template", source: "node", status: "passed", reviewKind: "code", output: "not addressable" }],
|
||||
])("rejects GET and canonical address for excluded marked custom %s results", async (_state, result) => {
|
||||
const task = { ...FAKE_TASK_DETAIL, id: "FN-013", workflowStepResults: [result], reviewState: undefined };
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
(store.getAgentLogs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
const review = await REQUEST(buildApp(), "GET", "/api/tasks/FN-013/review");
|
||||
const refreshed = await REQUEST(buildApp(), "POST", "/api/tasks/FN-013/review/refresh");
|
||||
expect(review.body.items).toEqual([]);
|
||||
expect(refreshed.body.items).toEqual([]);
|
||||
const address = await REQUEST(buildApp(), "POST", "/api/tasks/FN-013/review/address", JSON.stringify({
|
||||
selectedItems: [{ id: "workflow-review-forged", source: "reviewer-agent" }],
|
||||
}), { "Content-Type": "application/json" });
|
||||
expect(address.status).toBe(400);
|
||||
});
|
||||
|
||||
it("uses legacy activity review feedback when workflow results are absent or unsupported", async () => {
|
||||
const taskWithUnsupportedWorkflowReview = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
|
||||
@@ -60,6 +60,7 @@ import {
|
||||
isEphemeralAgent,
|
||||
parseExplicitDuplicateMarker,
|
||||
resolveWorkflowIrForTask,
|
||||
resolveWorkflowIrForTaskWithProvenance,
|
||||
resolveReviewColumns,
|
||||
workflowHasColumn,
|
||||
workflowPlansInColumn,
|
||||
@@ -816,7 +817,8 @@ function buildReviewerAgentItemId(input: { index: number; reviewType: "plan" | "
|
||||
}
|
||||
|
||||
const CURRENT_WORKFLOW_REVIEW_STEP_IDS = new Set(["code-review", "plan-review"]);
|
||||
const CURRENT_WORKFLOW_REVIEW_STATUSES = new Set<WorkflowStepResult["status"]>(["passed", "failed", "advisory_failure"]);
|
||||
const CURRENT_WORKFLOW_REVIEW_STATUSES = new Set<WorkflowStepResult["status"]>(["passed", "failed"]);
|
||||
const LEGACY_WORKFLOW_REVIEW_STATUSES = new Set<WorkflowStepResult["status"]>(["passed", "failed", "advisory_failure"]);
|
||||
function parseTaskReviewVerdict(value: string | undefined): TaskReviewVerdict | undefined {
|
||||
switch (value) {
|
||||
case "APPROVE":
|
||||
@@ -836,14 +838,13 @@ function parseTaskReviewVerdict(value: string | undefined): TaskReviewVerdict |
|
||||
* current, terminal, verdict-bearing results win over compatibility-only reviewer prose/activity
|
||||
* parsing. Their persisted identity produces canonical ids so GET, refresh, and address reconstruct
|
||||
* the same server-owned feedback; address snapshots and steering must never trust client prose.
|
||||
* Custom step names, historical attempts, pending/skipped, superseded, and bypassed results remain
|
||||
* non-selectable until workflow results gain an authoritative persisted review-kind marker.
|
||||
* Explicit `reviewKind` results may also qualify when terminal and nonblank; custom names,
|
||||
* verdicts, and gate modes remain non-semantic. Historical attempts, pending/skipped,
|
||||
* superseded, and bypassed results are non-selectable.
|
||||
*/
|
||||
function isCurrentWorkflowReviewResult(result: WorkflowStepResult): result is WorkflowStepResult & { verdict: TaskReviewVerdict } {
|
||||
return CURRENT_WORKFLOW_REVIEW_STEP_IDS.has(result.workflowStepId)
|
||||
&& CURRENT_WORKFLOW_REVIEW_STATUSES.has(result.status)
|
||||
&& result.verdict !== undefined
|
||||
&& result.supersededAt === undefined
|
||||
function hasCurrentWorkflowReviewIdentity(result: WorkflowStepResult): boolean {
|
||||
return result.supersededAt === undefined
|
||||
&& result.supersededReason === undefined
|
||||
&& result.bypassedBy === undefined
|
||||
&& result.bypassedAt === undefined
|
||||
&& result.bypassReason === undefined
|
||||
@@ -851,6 +852,53 @@ function isCurrentWorkflowReviewResult(result: WorkflowStepResult): result is Wo
|
||||
&& result.bypassedFromVerdict === undefined;
|
||||
}
|
||||
|
||||
function isCurrentMarkedWorkflowReviewResult(result: WorkflowStepResult): result is WorkflowStepResult & { reviewKind: "plan" | "code" } {
|
||||
return CURRENT_WORKFLOW_REVIEW_STATUSES.has(result.status)
|
||||
&& hasCurrentWorkflowReviewIdentity(result)
|
||||
&& Boolean(result.output?.trim() || result.notes?.trim())
|
||||
&& (result.reviewKind === "plan" || result.reviewKind === "code")
|
||||
&& (result.source === "node" || result.source === "optional-group");
|
||||
}
|
||||
|
||||
async function getDeclaredTopLevelReviewResultSources(task: Task, store: TaskStore): Promise<Map<string, WorkflowStepResult["source"]>> {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
|
||||
if (resolved.source !== "selection") return new Map();
|
||||
|
||||
/*
|
||||
FNXC:WorkflowReviewKind 2026-08-05-06:14:
|
||||
A persisted marker is authoritative only for a node and result source that the task's selected
|
||||
workflow declares at the top level. Resolve exact node identities instead of reserving punctuation: custom node IDs
|
||||
may legitimately contain `::` or `#<number>:`, while template instances have no matching top-level
|
||||
declaration and remain outside this task's currentness/addressing contract.
|
||||
*/
|
||||
return new Map(resolved.ir.nodes.flatMap((node): Array<[string, WorkflowStepResult["source"]]> => {
|
||||
if (node.kind === "optional-group") return [[node.id, "optional-group"]];
|
||||
if (node.kind === "prompt" || node.kind === "gate" || node.kind === "script") return [[node.id, "node"]];
|
||||
return [];
|
||||
}));
|
||||
}
|
||||
|
||||
function isCurrentLegacyWorkflowReviewResult(result: WorkflowStepResult): boolean {
|
||||
// Historical built-ins predate reviewKind. Preserve FN-8793's deliberately narrow
|
||||
// id-and-verdict compatibility contract without applying marked-custom requirements.
|
||||
return CURRENT_WORKFLOW_REVIEW_STEP_IDS.has(result.workflowStepId)
|
||||
&& result.reviewKind === undefined
|
||||
&& LEGACY_WORKFLOW_REVIEW_STATUSES.has(result.status)
|
||||
&& result.verdict !== undefined
|
||||
&& hasCurrentWorkflowReviewIdentity(result);
|
||||
}
|
||||
|
||||
function getWorkflowReviewKind(
|
||||
result: WorkflowStepResult,
|
||||
declaredTopLevelReviewResultSources: ReadonlyMap<string, WorkflowStepResult["source"]>,
|
||||
): "plan" | "code" | undefined {
|
||||
if (isCurrentMarkedWorkflowReviewResult(result) && declaredTopLevelReviewResultSources.get(result.workflowStepId) === result.source) return result.reviewKind;
|
||||
if (isCurrentLegacyWorkflowReviewResult(result)) {
|
||||
return result.workflowStepId === "plan-review" ? "plan" : "code";
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function buildWorkflowReviewItemId(task: Task, result: WorkflowStepResult): string {
|
||||
const identity = JSON.stringify({
|
||||
taskId: task.id,
|
||||
@@ -859,6 +907,7 @@ function buildWorkflowReviewItemId(task: Task, result: WorkflowStepResult): stri
|
||||
phase: result.phase,
|
||||
status: result.status,
|
||||
verdict: result.verdict,
|
||||
reviewKind: result.reviewKind,
|
||||
completedAt: result.completedAt,
|
||||
startedAt: result.startedAt,
|
||||
output: result.output,
|
||||
@@ -867,17 +916,18 @@ function buildWorkflowReviewItemId(task: Task, result: WorkflowStepResult): stri
|
||||
return `workflow-review-${createHash("sha256").update(identity).digest("hex").slice(0, 24)}`;
|
||||
}
|
||||
|
||||
function buildWorkflowReviewItems(task: Task): TaskReviewItem[] {
|
||||
async function buildWorkflowReviewItems(task: Task, store: TaskStore): Promise<TaskReviewItem[]> {
|
||||
const declaredTopLevelReviewResultSources = await getDeclaredTopLevelReviewResultSources(task, store);
|
||||
return (task.workflowStepResults ?? [])
|
||||
.filter(isCurrentWorkflowReviewResult)
|
||||
.map((result): TaskReviewItem => {
|
||||
const reviewType = result.workflowStepId === "plan-review" ? "plan" as const : "code" as const;
|
||||
.flatMap((result): TaskReviewItem[] => {
|
||||
const reviewType = getWorkflowReviewKind(result, declaredTopLevelReviewResultSources);
|
||||
if (!reviewType) return [];
|
||||
const body = result.output?.trim() || result.notes?.trim() || "No written feedback was provided by this review step.";
|
||||
const timestamp = result.completedAt ?? result.startedAt ?? task.updatedAt ?? task.createdAt;
|
||||
return {
|
||||
return [{
|
||||
itemId: buildWorkflowReviewItemId(task, result),
|
||||
sourceMode: "reviewer-agent",
|
||||
title: `${result.workflowStepName || result.workflowStepId} ${result.verdict}`,
|
||||
title: `${result.workflowStepName || result.workflowStepId} ${result.verdict ?? result.status}`,
|
||||
body,
|
||||
author: "reviewer-agent",
|
||||
createdAt: timestamp,
|
||||
@@ -886,7 +936,7 @@ function buildWorkflowReviewItems(task: Task): TaskReviewItem[] {
|
||||
verdict: result.verdict,
|
||||
reviewType,
|
||||
progressStatus: null,
|
||||
};
|
||||
}];
|
||||
})
|
||||
.sort((a, b) => Date.parse(b.createdAt ?? "") - Date.parse(a.createdAt ?? "") || a.itemId.localeCompare(b.itemId));
|
||||
}
|
||||
@@ -899,7 +949,7 @@ function buildDirectReviewSummary(items: TaskReviewItem[]): TaskReviewSummary |
|
||||
}
|
||||
|
||||
async function buildDirectTaskReviewData(task: Task, store: TaskStore): Promise<TaskReviewData> {
|
||||
const structuredItems = buildWorkflowReviewItems(task);
|
||||
const structuredItems = await buildWorkflowReviewItems(task, store);
|
||||
if (structuredItems.length > 0) {
|
||||
return {
|
||||
mode: "reviewer-agent",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR, BUILTIN_STEPWISE_CODING_WORKFLOW_IR } from "@fusion/core";
|
||||
import type { TaskDetail, WorkflowIr } from "@fusion/core";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR, BUILTIN_STEPWISE_CODING_WORKFLOW_IR, upsertWorkflowStepResult } from "@fusion/core";
|
||||
import type { TaskDetail, WorkflowIr, WorkflowStepResult } from "@fusion/core";
|
||||
|
||||
import {
|
||||
PLAN_REVIEW_PROVIDER_FAILURE_HOLD_VALUE,
|
||||
@@ -299,7 +299,11 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
requestPreMergeOptionalStepFix: requestFix,
|
||||
});
|
||||
|
||||
const result = await executor.run(taskWith(["group"]), settingsOn(), reviseGroupIr());
|
||||
const ir = reviseGroupIr();
|
||||
const group = ir.nodes.find((node) => node.id === "group");
|
||||
if (!group) throw new Error("review group missing");
|
||||
group.config = { ...group.config, reviewKind: "code" };
|
||||
const result = await executor.run(taskWith(["group"]), settingsOn(), ir);
|
||||
|
||||
expect(requestFix).toHaveBeenCalledWith("FN-OG", {
|
||||
stepName: "Code Review",
|
||||
@@ -313,7 +317,8 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
expect(calls).not.toContain("after");
|
||||
expect(result.context["node:group:fixScheduled"]).toBe(true);
|
||||
expect(records).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ workflowStepId: "group", status: "advisory_failure", verdict: "REVISE", output: "Fix the review finding" }),
|
||||
expect.objectContaining({ workflowStepId: "group", status: "pending", reviewKind: "code" }),
|
||||
expect.objectContaining({ workflowStepId: "group", status: "advisory_failure", verdict: "REVISE", output: "Fix the review finding", reviewKind: "code" }),
|
||||
]));
|
||||
});
|
||||
|
||||
@@ -1037,6 +1042,170 @@ describe("WorkflowGraphExecutor optional-group", () => {
|
||||
}
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:WorkflowReviewKind 2026-08-05-03:08:
|
||||
* Marked top-level nodes are direct-review producers even without skillName.
|
||||
* Exercise each writer through normal, failure, and retry paths so the marker
|
||||
* is a declaration snapshot rather than a success-only presentation hint.
|
||||
*/
|
||||
it.each([
|
||||
["prompt", "plan"],
|
||||
["gate", "code"],
|
||||
["script", "plan"],
|
||||
] as const)("snapshots marked custom top-level %s results without skillName across terminal outcomes", async (kind, reviewKind) => {
|
||||
const records: Array<{ status: string; reviewKind?: string; source?: string }> = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "marked-top-level-review",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "review", kind, config: { reviewKind } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [{ from: "start", to: "review" }, { from: "review", to: "end", condition: "success" }, { from: "review", to: "end", condition: "failure" }],
|
||||
};
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: { [kind]: async () => ({ outcome: "failure", contextPatch: { notes: "declared review failure" } }) },
|
||||
recordWorkflowStepResult: async (_taskId, result) => { records.push(result); },
|
||||
});
|
||||
|
||||
await executor.run(taskWith([]), settingsOn(), ir);
|
||||
|
||||
expect(records).toEqual([
|
||||
expect.objectContaining({ workflowStepId: "review", source: "node", status: "pending", reviewKind }),
|
||||
expect.objectContaining({ workflowStepId: "review", source: "node", status: "failed", reviewKind, notes: "declared review failure" }),
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["prompt", "plan"],
|
||||
["gate", "code"],
|
||||
["script", "plan"],
|
||||
] as const)("snapshots marked custom top-level %s results on normal completion without skillName", async (kind, reviewKind) => {
|
||||
const records: Array<{ status: string; reviewKind?: string; source?: string }> = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "marked-top-level-review-success",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "review", kind, config: { reviewKind } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [{ from: "start", to: "review" }, { from: "review", to: "end", condition: "success" }],
|
||||
};
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: { [kind]: async () => ({ outcome: "success", contextPatch: { output: "declared review passed" } }) },
|
||||
recordWorkflowStepResult: async (_taskId, result) => { records.push(result); },
|
||||
});
|
||||
|
||||
await executor.run(taskWith([]), settingsOn(), ir);
|
||||
|
||||
expect(records).toEqual([
|
||||
expect.objectContaining({ workflowStepId: "review", source: "node", status: "pending", reviewKind }),
|
||||
expect.objectContaining({ workflowStepId: "review", source: "node", status: "passed", reviewKind, output: "declared review passed" }),
|
||||
]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["prompt", "plan"],
|
||||
["gate", "code"],
|
||||
["script", "plan"],
|
||||
] as const)("preserves %s reviewKind on current and prior failed retry records", async (kind, reviewKind) => {
|
||||
let stored: WorkflowStepResult[] = [];
|
||||
const ir: WorkflowIr = {
|
||||
version: "v2",
|
||||
name: "marked-top-level-review-retry",
|
||||
columns: [{ id: "work", name: "Work", traits: [] }],
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "review", kind, config: { reviewKind } },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [{ from: "start", to: "review" }, { from: "review", to: "end", condition: "failure" }],
|
||||
};
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: { [kind]: async () => ({ outcome: "failure", contextPatch: { notes: "retryable finding" } }) },
|
||||
recordWorkflowStepResult: async (_taskId, result) => { stored = upsertWorkflowStepResult(stored, result); },
|
||||
});
|
||||
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
vi.setSystemTime(new Date("2026-08-05T03:21:00.000Z"));
|
||||
await executor.run(taskWith([]), settingsOn(), ir);
|
||||
vi.setSystemTime(new Date("2026-08-05T03:22:00.000Z"));
|
||||
await executor.run(taskWith([]), settingsOn(), ir);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
|
||||
expect(stored).toEqual([
|
||||
expect.objectContaining({ status: "failed", reviewKind, priorAttempts: [expect.objectContaining({ status: "failed", reviewKind })] }),
|
||||
]);
|
||||
});
|
||||
|
||||
it("snapshots a marked custom optional-group review kind on pending and terminal results", async () => {
|
||||
const records: Array<{ status: string; reviewKind?: string; source?: string }> = [];
|
||||
const ir = optionalGroupIr();
|
||||
const group = ir.nodes.find((node) => node.id === "group");
|
||||
if (!group) throw new Error("test workflow group missing");
|
||||
group.config = { ...group.config, reviewKind: "code" };
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: { prompt: async () => ({ outcome: "success", contextPatch: { output: "review feedback" } }) },
|
||||
recordWorkflowStepResult: async (_taskId, result) => { records.push(result); },
|
||||
});
|
||||
|
||||
await executor.run(taskWith(["group"]), settingsOn(), ir);
|
||||
|
||||
expect(records.filter((result) => result.source === "optional-group")).toEqual([
|
||||
expect.objectContaining({ status: "pending", reviewKind: "code" }),
|
||||
expect.objectContaining({ status: "passed", reviewKind: "code" }),
|
||||
]);
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:WorkflowReviewKind 2026-08-05-03:32:
|
||||
* Optional groups replace their pending result after every execution. The
|
||||
* declared marker must survive failed retries on both the current result and
|
||||
* its bounded audit snapshot; it is not inferred from the template outcome.
|
||||
*/
|
||||
it("preserves optional-group reviewKind on current and prior failed retries", async () => {
|
||||
let stored: WorkflowStepResult[] = [];
|
||||
const ir = optionalGroupIr();
|
||||
const group = ir.nodes.find((node) => node.id === "group");
|
||||
if (!group) throw new Error("test workflow group missing");
|
||||
group.config = { ...group.config, reviewKind: "code" };
|
||||
const executor = new WorkflowGraphExecutor({
|
||||
handlers: {
|
||||
prompt: async (node) => node.id === "optstep"
|
||||
? { outcome: "failure", contextPatch: { notes: "retryable group finding" } }
|
||||
: { outcome: "success" },
|
||||
},
|
||||
recordWorkflowStepResult: async (_taskId, result) => { stored = upsertWorkflowStepResult(stored, result); },
|
||||
});
|
||||
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
vi.setSystemTime(new Date("2026-08-05T03:32:00.000Z"));
|
||||
await executor.run(taskWith(["group"]), settingsOn(), ir);
|
||||
vi.setSystemTime(new Date("2026-08-05T03:33:00.000Z"));
|
||||
await executor.run(taskWith(["group"]), settingsOn(), ir);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
|
||||
expect(stored).toEqual([
|
||||
expect.objectContaining({
|
||||
workflowStepId: "group",
|
||||
source: "optional-group",
|
||||
status: "failed",
|
||||
reviewKind: "code",
|
||||
priorAttempts: [expect.objectContaining({ status: "failed", reviewKind: "code" })],
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("blocks builtin coding review and merge when Code Review requests revision and no remediation is scheduled", async () => {
|
||||
const requestFix = vi.fn(async () => false);
|
||||
const calls: string[] = [];
|
||||
|
||||
@@ -891,6 +891,7 @@ export class WorkflowGraphExecutor {
|
||||
phase: stepPhase,
|
||||
status: "pending",
|
||||
source: "optional-group",
|
||||
...(this.workflowReviewKind(node) ? { reviewKind: this.workflowReviewKind(node) } : {}),
|
||||
startedAt: stepStartedAt,
|
||||
// U3/KTD-4: stamp the lease owner so a concurrent/crashed re-entry
|
||||
// adopts this pending gate instead of dispatching a second reviewer.
|
||||
@@ -972,6 +973,7 @@ export class WorkflowGraphExecutor {
|
||||
phase: stepPhase,
|
||||
source: "optional-group",
|
||||
status: stepStatus,
|
||||
...(this.workflowReviewKind(node) ? { reviewKind: this.workflowReviewKind(node) } : {}),
|
||||
...(verdict ? { verdict } : {}),
|
||||
...(stepOutput !== undefined ? { output: stepOutput } : {}),
|
||||
...(stepNotes !== undefined ? { notes: stepNotes } : {}),
|
||||
@@ -1673,13 +1675,19 @@ export class WorkflowGraphExecutor {
|
||||
return failureResult;
|
||||
}
|
||||
|
||||
/** FNXC:WorkflowReviewKind 2026-08-05-02:31: Persist only the declared closed
|
||||
* marker; execution never derives review semantics from a label, verdict, or gate mode. */
|
||||
private workflowReviewKind(node: WorkflowIrNode): WorkflowStepResult["reviewKind"] | undefined {
|
||||
return node.config?.reviewKind === "plan" || node.config?.reviewKind === "code"
|
||||
? node.config.reviewKind
|
||||
: undefined;
|
||||
}
|
||||
|
||||
private shouldRecordNodeProgress(node: WorkflowIrNode): boolean {
|
||||
/*
|
||||
* FNXC:WorkflowNodeProgress 2026-06-29-15:05:
|
||||
* Compound Engineering stages are top-level skill prompt/gate nodes, not parsed implementation steps or optional toggles. Record those skill nodes into `task.workflowStepResults` so cards and task details show the active CE stage while avoiding duplicate records for ordinary model prompts and optional-group template internals.
|
||||
*/
|
||||
const skillName = typeof node.config?.skillName === "string" ? node.config.skillName.trim() : "";
|
||||
return skillName.length > 0 && (node.kind === "prompt" || node.kind === "gate");
|
||||
const markedReview = this.workflowReviewKind(node) !== undefined;
|
||||
return (markedReview && (node.kind === "prompt" || node.kind === "gate" || node.kind === "script"))
|
||||
|| (skillName.length > 0 && (node.kind === "prompt" || node.kind === "gate"));
|
||||
}
|
||||
|
||||
private workflowNodeProgressName(node: WorkflowIrNode): string {
|
||||
@@ -1695,6 +1703,7 @@ export class WorkflowGraphExecutor {
|
||||
phase: node.config?.phase === "post-merge" ? "post-merge" : "pre-merge",
|
||||
source: "node",
|
||||
status: "pending",
|
||||
...(this.workflowReviewKind(node) ? { reviewKind: this.workflowReviewKind(node) } : {}),
|
||||
startedAt,
|
||||
};
|
||||
await this.recordOptionalGroupStepResult(taskId, result);
|
||||
@@ -1732,6 +1741,7 @@ export class WorkflowGraphExecutor {
|
||||
phase: started?.phase ?? (node.config?.phase === "post-merge" ? "post-merge" : "pre-merge"),
|
||||
source: "node",
|
||||
status,
|
||||
...(this.workflowReviewKind(node) ? { reviewKind: this.workflowReviewKind(node) } : {}),
|
||||
...(output !== undefined ? { output } : {}),
|
||||
...(notes !== undefined ? { notes } : {}),
|
||||
startedAt: started?.startedAt ?? new Date().toISOString(),
|
||||
|
||||
Reference in New Issue
Block a user