fix(followup): validador exato — ciclo sem espera e acumulação por condensação SCC [onda 2]

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0143T3gYRomX8kFxK3KhJK1U
2026-10-02 09:34:46 +08:00 · 2026-07-21 19:30:26 -03:00
parent 9a27c2b80d
commit ea5a746b33
2 changed files with 215 additions and 65 deletions
@@ -224,22 +224,116 @@ describe('validateFlowForPublish', () => {
    expect(validateFlowForPublish(g).ok).toBe(true);
  });

-  it('flags max_steps_exceeded when the longest acyclic path from trigger exceeds 30 nodes', () => {
+  // A regression fixture for the SCC-based cycle_without_wait check: A and B
+  // form a cycle with no wait in it; C is a *separate* cycle with A that DOES
+  // have a sufficient wait. A naive "does the whole component contain a wait"
+  // check would merge A/B/C into one component (since C links back into A)
+  // and wrongly conclude the component is safe. Removing sufficient-wait
+  // nodes before computing SCCs (the actual algorithm) keeps A<->B a cycle on
+  // its own and correctly flags it.
+  it('flags cycle_without_wait when a wait-free cycle shares a node with a wait-guarded one', () => {
+    const g = graph(
+      [trigger('t1'), condition('A'), condition('B'), wait('C', { mode: 'fixed', duration_ms: 300_000 }), end('end1')],
+      [
+        edge('t1', 'A', always()),
+        edge('A', 'B', condResult(true)),
+        edge('B', 'A', condResult(true)),
+        edge('A', 'C', condResult(false)),
+        edge('C', 'A', always()),
+        edge('B', 'end1', condResult(false)),
+      ]
+    );
+    const result = validateFlowForPublish(g);
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.errors.map((e) => e.code)).toEqual(['cycle_without_wait']);
+    }
+  });
+
+  it('does not flag max_steps_exceeded for a path of exactly 30 nodes', () => {
    const waitNodes: FlowNode[] = [];
    const edges: FlowEdge[] = [];
    let prev = 't1';
-    for (let i = 1; i <= 31; i++) {
+    for (let i = 1; i <= 28; i++) {
      const id = `w${i}`;
      waitNodes.push(wait(id, { mode: 'fixed', duration_ms: 300_000 }));
      edges.push(edge(prev, id, always()));
      prev = id;
    }
    edges.push(edge(prev, 'e1', always()));
-    const g = graph([trigger('t1'), ...waitNodes, end('e1')], edges);
+    const g = graph([trigger('t1'), ...waitNodes, end('e1')], edges); // 1 + 28 + 1 = 30 nodes
+    expect(validateFlowForPublish(g).ok).toBe(true);
+  });
+
+  it('flags max_steps_exceeded for a path of exactly 31 nodes', () => {
+    const waitNodes: FlowNode[] = [];
+    const edges: FlowEdge[] = [];
+    let prev = 't1';
+    for (let i = 1; i <= 29; i++) {
+      const id = `w${i}`;
+      waitNodes.push(wait(id, { mode: 'fixed', duration_ms: 300_000 }));
+      edges.push(edge(prev, id, always()));
+      prev = id;
+    }
+    edges.push(edge(prev, 'e1', always()));
+    const g = graph([trigger('t1'), ...waitNodes, end('e1')], edges); // 1 + 29 + 1 = 31 nodes
    const result = validateFlowForPublish(g);
    expect(result.ok).toBe(false);
    if (!result.ok) {
-      expect(result.errors.map((e) => e.code)).toContain('max_steps_exceeded');
+      expect(result.errors.map((e) => e.code)).toEqual(['max_steps_exceeded']);
+    }
+  });
+
+  // Regression for the old per-path DFS: a width-2 "diamond" DAG re-converges
+  // every layer, so the number of distinct trigger->leaf paths is 2^layers —
+  // astronomically more than the old MAX_DFS_CALLS cap could enumerate for a
+  // schema-legal 60-node graph, which silently truncated and could miss a
+  // violation. The SCC-condensation sweep is O(V+E) regardless of path count,
+  // so this must both finish fast and still catch the violator correctly.
+  it('flags long_wait_needs_template in a wide reconverging DAG without blowing up', () => {
+    // 27 wait layers + trigger + action + end = 30 steps exactly, so this
+    // exercises long_wait_needs_template in isolation without also crossing
+    // the (separately regression-tested) max_steps_exceeded boundary.
+    const LAYERS = 27;
+    const LAYER_WAIT_MS = 3_200_000; // 27 * 3.2M = 86.4M ms == 24h threshold
+    const nodes: FlowNode[] = [trigger('t1')];
+    const edges: FlowEdge[] = [];
+
+    const layerId = (layer: number, branch: 'a' | 'b') => `d${layer}_${branch}`;
+
+    for (let layer = 1; layer <= LAYERS; layer++) {
+      nodes.push(wait(layerId(layer, 'a'), { mode: 'fixed', duration_ms: LAYER_WAIT_MS }));
+      nodes.push(wait(layerId(layer, 'b'), { mode: 'fixed', duration_ms: LAYER_WAIT_MS }));
+
+      const sources =
+        layer === 1 ? (['t1', 't1'] as const) : ([layerId(layer - 1, 'a'), layerId(layer - 1, 'b')] as const);
+      for (const source of sources) {
+        edges.push(edge(source, layerId(layer, 'a'), always()));
+        edges.push(edge(source, layerId(layer, 'b'), always()));
+      }
+    }
+
+    nodes.push(actionAiMessage('act_bad')); // violator: no fallback_template_id
+    nodes.push(actionTemplate('act_ok'));
+    nodes.push(end('end_diamond'));
+    for (const source of [layerId(LAYERS, 'a'), layerId(LAYERS, 'b')] as const) {
+      edges.push(edge(source, 'act_bad', always()));
+      edges.push(edge(source, 'act_ok', always()));
+    }
+    edges.push(edge('act_bad', 'end_diamond', always()));
+    edges.push(edge('act_ok', 'end_diamond', always()));
+
+    expect(nodes.length).toBe(58); // well within the schema's 60-node cap
+
+    const start = performance.now();
+    const result = validateFlowForPublish(graph(nodes, edges));
+    const elapsedMs = performance.now() - start;
+
+    expect(elapsedMs).toBeLessThan(1000); // polynomial, not exponential
+    expect(result.ok).toBe(false);
+    if (!result.ok) {
+      expect(result.errors.map((e) => e.code)).toEqual(['long_wait_needs_template']);
+      expect(result.errors[0]!.node_id).toBe('act_bad');
    }
  });

@@ -33,15 +33,19 @@ export type PublishValidationResult =
 const LONG_WAIT_THRESHOLD_MS = 86_400_000; // 24h
 const MIN_CYCLE_WAIT_MS = 300_000; // 5min
 const MAX_PATH_STEPS = 30;
-// ponytail: caps worst-case path-DFS blowup on adversarial dense graphs; the
-// schema already bounds graphs to 60 nodes / 120 edges so this cap is headroom,
-// not a real limit for any graph a user can actually build in the editor.
-const MAX_DFS_CALLS = 200_000;

 function waitMs(config: Extract<FlowNode, { type: 'wait' }>['config']): number {
  return config.mode === 'fixed' ? config.duration_ms : config.max_ms;
 }

+/** A wait node whose duration meets the 5min floor required to break a cycle. */
+function isSufficientWaitNode(node: FlowNode): boolean {
+  if (node.type !== 'wait') return false;
+  return node.config.mode === 'fixed'
+    ? node.config.duration_ms >= MIN_CYCLE_WAIT_MS
+    : node.config.min_ms >= MIN_CYCLE_WAIT_MS;
+}
+
 function buildOutEdges(edges: FlowEdge[]): Map<string, FlowEdge[]> {
  const map = new Map<string, FlowEdge[]>();
  for (const edge of edges) {
@@ -67,7 +71,13 @@ function bfsReachable(startIds: string[], outEdges: Map<string, FlowEdge[]>): Se
  return visited;
 }

-/** Tarjan strongly-connected-components, used to locate cycles. */
+/**
+ * Tarjan strongly-connected-components. Returned components are in the
+ * algorithm's natural finishing order, which is the REVERSE of a topological
+ * order of the condensation DAG (a component finishes only after every
+ * component reachable from it has already finished). Callers that need a
+ * source-to-sink sweep should iterate the result back-to-front.
+ */
 function stronglyConnectedComponents(
  nodeIds: string[],
  outEdges: Map<string, FlowEdge[]>
@@ -115,47 +125,98 @@ function stronglyConnectedComponents(
 }

 /**
- * Single per-path DFS from the trigger, tracking cumulative wait time
- * (fixed -> duration_ms, smart -> max_ms) and path length. A node already on
- * the current path is not re-entered (cycles count one iteration, per spec).
+ * Whether a component (as returned by stronglyConnectedComponents) is an
+ * actual cycle in `outEdges`: more than one node, or a single node with a
+ * self-loop.
 */
-function walkPaths(
+function isCycleComponent(component: string[], outEdges: Map<string, FlowEdge[]>): boolean {
+  if (component.length > 1) return true;
+  const onlyId = component[0]!; // Tarjan never yields an empty component
+  return (outEdges.get(onlyId) ?? []).some((e) => e.target === onlyId);
+}
+
+/**
+ * SCC condensation + topological forward sweep from the trigger, computing —
+ * per component reachable from it — the MAX accumulated wait (fixed ->
+ * duration_ms, smart -> max_ms) and MAX accumulated step count over any path
+ * from the trigger. A component's own internal wait/step total is counted
+ * once no matter how many original-graph cycles loop inside it (implements
+ * "cycles count 1 iteration"). Polynomial (O(V+E)): no per-path enumeration,
+ * so branching/reconverging DAGs can't blow it up.
+ */
+function analyzeCondensedPaths(
  startId: string,
+  nodes: FlowNode[],
  nodesById: Map<string, FlowNode>,
  outEdges: Map<string, FlowEdge[]>
 ): { longWaitNodeIds: Set<string>; maxStepsExceeded: boolean } {
  const longWaitNodeIds = new Set<string>();
  let maxStepsExceeded = false;
-  let calls = 0;

-  function dfs(nodeId: string, accumulatedWait: number, pathVisited: Set<string>) {
-    calls++;
-    if (calls > MAX_DFS_CALLS) return;
-    if (pathVisited.has(nodeId)) return;
-    const node = nodesById.get(nodeId);
-    if (!node) return; // dangling edge reference — not this validator's concern
+  const components = stronglyConnectedComponents(
+    nodes.map((n) => n.id),
+    outEdges
+  );
+  const componentIndexById = new Map<string, number>();
+  components.forEach((comp, idx) => comp.forEach((id) => componentIndexById.set(id, idx)));

-    const nextVisited = new Set(pathVisited);
-    nextVisited.add(nodeId);
-    if (nextVisited.size > MAX_PATH_STEPS) maxStepsExceeded = true;
+  const waitWeight = components.map((comp) =>
+    comp.reduce((sum, id) => {
+      const node = nodesById.get(id);
+      return node && node.type === 'wait' ? sum + waitMs(node.config) : sum;
+    }, 0)
+  );
+  const stepWeight = components.map((comp) => comp.length);

-    const nextAccumulated = node.type === 'wait' ? accumulatedWait + waitMs(node.config) : accumulatedWait;
-
-    if (
-      node.type === 'action' &&
-      node.config.mode === 'ai_message' &&
-      !node.config.fallback_template_id &&
-      nextAccumulated >= LONG_WAIT_THRESHOLD_MS
-    ) {
-      longWaitNodeIds.add(nodeId);
+  const condOut = new Map<number, Set<number>>();
+  for (const edgeList of outEdges.values()) {
+    for (const edge of edgeList) {
+      const cu = componentIndexById.get(edge.source);
+      const cv = componentIndexById.get(edge.target);
+      if (cu === undefined || cv === undefined || cu === cv) continue;
+      const succs = condOut.get(cu);
+      if (succs) succs.add(cv);
+      else condOut.set(cu, new Set([cv]));
    }
+  }

-    for (const edge of outEdges.get(nodeId) ?? []) {
-      dfs(edge.target, nextAccumulated, nextVisited);
+  const startComp = componentIndexById.get(startId);
+  if (startComp === undefined) return { longWaitNodeIds, maxStepsExceeded };
+
+  const arriveWait = new Map<number, number>([[startComp, 0]]);
+  const arriveSteps = new Map<number, number>([[startComp, 0]]);
+
+  // components[] is in reverse-topological (Tarjan finishing) order; walking
+  // it back-to-front visits every predecessor component before its successors.
+  for (let idx = components.length - 1; idx >= 0; idx--) {
+    const arrivedWait = arriveWait.get(idx);
+    if (arrivedWait === undefined) continue; // not reachable from the trigger
+    const arrivedSteps = arriveSteps.get(idx)!;
+
+    const totalWait = arrivedWait + waitWeight[idx]!;
+    const totalSteps = arrivedSteps + stepWeight[idx]!;
+
+    if (totalSteps > MAX_PATH_STEPS) maxStepsExceeded = true;
+
+    for (const id of components[idx]!) {
+      const node = nodesById.get(id);
+      if (
+        node &&
+        node.type === 'action' &&
+        node.config.mode === 'ai_message' &&
+        !node.config.fallback_template_id &&
+        totalWait >= LONG_WAIT_THRESHOLD_MS
+      ) {
+        longWaitNodeIds.add(id);
+      }
+    }
+
+    for (const succ of condOut.get(idx) ?? []) {
+      arriveWait.set(succ, Math.max(arriveWait.get(succ) ?? -Infinity, totalWait));
+      arriveSteps.set(succ, Math.max(arriveSteps.get(succ) ?? -Infinity, totalSteps));
    }
  }

-  dfs(startId, 0, new Set());
  return { longWaitNodeIds, maxStepsExceeded };
 }

@@ -213,7 +274,12 @@ export function validateFlowForPublish(graph: FlowGraph): PublishValidationResul
      }
    }

-    const { longWaitNodeIds, maxStepsExceeded } = walkPaths(startTrigger.id, nodesById, outEdges);
+    const { longWaitNodeIds, maxStepsExceeded } = analyzeCondensedPaths(
+      startTrigger.id,
+      nodes,
+      nodesById,
+      outEdges
+    );
    for (const id of [...longWaitNodeIds].sort()) {
      errors.push({
        node_id: id,
@@ -276,37 +342,27 @@ export function validateFlowForPublish(graph: FlowGraph): PublishValidationResul
    }
  }

-  // ponytail: SCC-based over-approximation. A cycle is flagged only when NO
-  // node in its whole strongly-connected component is a sufficient wait node —
-  // exact for "no wait anywhere in the loop", but a component with a safe wait
-  // node reachable only by SOME of its cycles won't distinguish between them.
-  // Upgrade to per-simple-cycle checking if that gap ever bites in practice.
-  const components = stronglyConnectedComponents(
-    nodes.map((n) => n.id),
-    outEdges
+  // cycle_without_wait — exact, not an approximation: a directed cycle with no
+  // sufficient-wait node exists IFF removing every sufficient-wait node still
+  // leaves a cycle (SCC with >1 node, or a self-loop) in the remaining
+  // subgraph. Runs independently of trigger reachability — a node can carry
+  // both unreachable_node and cycle_without_wait at once; that's intentional,
+  // each signal is independently actionable for the editor UI.
+  const sufficientWaitIds = new Set(nodes.filter(isSufficientWaitNode).map((n) => n.id));
+  const remainingIds = nodes.map((n) => n.id).filter((id) => !sufficientWaitIds.has(id));
+  const remainingEdges = edges.filter(
+    (e) => !sufficientWaitIds.has(e.source) && !sufficientWaitIds.has(e.target)
  );
-  for (const component of components) {
-    const firstId = component[0]!; // Tarjan never yields an empty component
-    const isCycle =
-      component.length > 1 || (outEdges.get(firstId) ?? []).some((e) => e.target === firstId);
-    if (!isCycle) continue;
-
-    const hasSufficientWait = component.some((id) => {
-      const node = nodesById.get(id);
-      if (!node || node.type !== 'wait') return false;
-      return node.config.mode === 'fixed'
-        ? node.config.duration_ms >= MIN_CYCLE_WAIT_MS
-        : node.config.min_ms >= MIN_CYCLE_WAIT_MS;
+  const remainingOutEdges = buildOutEdges(remainingEdges);
+  const cycleComponents = stronglyConnectedComponents(remainingIds, remainingOutEdges);
+  for (const component of cycleComponents) {
+    if (!isCycleComponent(component, remainingOutEdges)) continue;
+    const nodeId = [...component].sort()[0]!;
+    errors.push({
+      node_id: nodeId,
+      code: 'cycle_without_wait',
+      message: `Ciclo sem espera mínima de 5min detectado (contém "${nodeId}").`,
    });
-
-    if (!hasSufficientWait) {
-      const nodeId = [...component].sort()[0]!; // same non-empty guarantee as above
-      errors.push({
-        node_id: nodeId,
-        code: 'cycle_without_wait',
-        message: `Ciclo sem espera mínima de 5min detectado (contém "${nodeId}").`,
-      });
-    }
  }

  if (errors.length === 0) return { ok: true };