From 02ff1513a388ee1835f295f75d7a7e6dea4b71d6 Mon Sep 17 00:00:00 2001 From: zy95-12 <135135297+zy95-12@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:59:14 +0000 Subject: [PATCH] fix(workflow): read both ends of an edge by their parsed spec validateWorkflow compared an edge's artifact against the node lists as written, so a definition that spells one fact with a space on the node and without one on its edge was refused with "does not line up" even though producers, consumers, dependencyEdges and enterNode all key it by canonicalSpec. Both sides now go through canonicalSpec, like every other rule in the file, so `text: draft` and `text:draft` are one fact on an edge too. A genuinely crossed edge is still refused. Test: core/test/workflow-edge-spec.test.js (red on the base: 1 fail / 2). Docs-not-needed: the canonical-spec rule is already stated in docs/subsystems/core-host.md; this makes the edge check obey it, with no new module, contract or owned-path change. --- core/src/shared/workflow.js | 13 ++++-- core/test/workflow-edge-spec.test.js | 41 +++++++++++++++++++ patches/ds-bot/files/src/shared/workflow.js | 13 ++++-- .../files/test/workflow-edge-spec.test.js | 41 +++++++++++++++++++ 4 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 core/test/workflow-edge-spec.test.js create mode 100644 patches/ds-bot/files/test/workflow-edge-spec.test.js diff --git a/core/src/shared/workflow.js b/core/src/shared/workflow.js index 6bc1d6a8d..2d081debc 100644 --- a/core/src/shared/workflow.js +++ b/core/src/shared/workflow.js @@ -617,6 +617,13 @@ export function validateWorkflow(raw, options = {}) { } const nodeIds = def.nodes.map((node) => node.id) + // An edge names one fact at each end, and a node may spell that fact with a space (#1298): the + // alignment check reads both sides the way every other rule in this file does, or a definition + // that validation otherwise accepts is refused as misaligned over its spelling. + const specsOf = new Map(def.nodes.map((node) => [node.id, { + outputs: new Set(node.outputs.map(canonicalSpec).filter(Boolean)), + inputs: new Set(node.inputs.map(canonicalSpec).filter(Boolean)), + }])) for (const edge of def.edges) { if (!edge.from || !edge.to) { problems.push(message('edge/without-both-ends', 'an edge names only one end', { edge })) @@ -632,9 +639,9 @@ export function validateWorkflow(raw, options = {}) { ) continue } - const fromNode = nodeById(def, from.node) - const toNode = nodeById(def, to.node) - if (!fromNode.outputs.includes(from.artifact) || !toNode.inputs.includes(to.artifact)) { + const fromSpecs = specsOf.get(from.node) + const toSpecs = specsOf.get(to.node) + if (!fromSpecs?.outputs.has(from.artifact) || !toSpecs?.inputs.has(to.artifact)) { problems.push( message( 'edge/misaligned', diff --git a/core/test/workflow-edge-spec.test.js b/core/test/workflow-edge-spec.test.js new file mode 100644 index 000000000..cc6fca52c --- /dev/null +++ b/core/test/workflow-edge-spec.test.js @@ -0,0 +1,41 @@ +// An edge names one fact at each end, and a definition may spell that fact with a space (#1272, +// #1298). Everything that decides whether a definition can run keys on the parsed spec +// (`canonicalSpec`), so `text: draft` and `text:draft` are one fact on an edge too: a definition +// that mixes the two spellings between a node and its edge is runnable and must not be refused as +// misaligned. A genuinely crossed edge is still refused. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { validateWorkflow } from '../src/shared/workflow.js' + +const definition = (edge) => ({ + id: 'draft-publish', + name: 'Draft and publish', + version: 1, + inputs: [], + deliverables: ['link:published'], + nodes: [ + // The producer spells the fact with a space; the consumer spells it without one. + { id: 'draft', name: 'Draft', role: 'Writer', outputs: ['text: draft'] }, + { id: 'publish', name: 'Publish', role: 'Publisher', inputs: ['text:draft'], outputs: ['link:published'] }, + ], + edges: [edge], + loops: [], +}) + +test('an edge lines up when its two ends spell one fact differently', () => { + // The edge uses the consumer's spelling against the producer's spaced one. + const spacedNode = validateWorkflow(definition({ from: 'draft.text:draft', to: 'publish.text:draft' })) + assert.equal(spacedNode.ok, true, JSON.stringify(spacedNode.problems)) + // The edge uses the producer's spaced spelling against the consumer's canonical one. + const spacedEdge = validateWorkflow(definition({ from: 'draft.text: draft', to: 'publish.text:draft' })) + assert.equal(spacedEdge.ok, true, JSON.stringify(spacedEdge.problems)) +}) + +test('an edge that really does not line up is still refused', () => { + const crossed = validateWorkflow(definition({ from: 'draft.text:draft', to: 'publish.link:published' })) + assert.equal(crossed.ok, false) + assert.ok( + crossed.problems.some((problem) => problem.code === 'edge/misaligned' || problem.code === 'edge/unknown-end'), + JSON.stringify(crossed.problems), + ) +}) diff --git a/patches/ds-bot/files/src/shared/workflow.js b/patches/ds-bot/files/src/shared/workflow.js index 6bc1d6a8d..2d081debc 100644 --- a/patches/ds-bot/files/src/shared/workflow.js +++ b/patches/ds-bot/files/src/shared/workflow.js @@ -617,6 +617,13 @@ export function validateWorkflow(raw, options = {}) { } const nodeIds = def.nodes.map((node) => node.id) + // An edge names one fact at each end, and a node may spell that fact with a space (#1298): the + // alignment check reads both sides the way every other rule in this file does, or a definition + // that validation otherwise accepts is refused as misaligned over its spelling. + const specsOf = new Map(def.nodes.map((node) => [node.id, { + outputs: new Set(node.outputs.map(canonicalSpec).filter(Boolean)), + inputs: new Set(node.inputs.map(canonicalSpec).filter(Boolean)), + }])) for (const edge of def.edges) { if (!edge.from || !edge.to) { problems.push(message('edge/without-both-ends', 'an edge names only one end', { edge })) @@ -632,9 +639,9 @@ export function validateWorkflow(raw, options = {}) { ) continue } - const fromNode = nodeById(def, from.node) - const toNode = nodeById(def, to.node) - if (!fromNode.outputs.includes(from.artifact) || !toNode.inputs.includes(to.artifact)) { + const fromSpecs = specsOf.get(from.node) + const toSpecs = specsOf.get(to.node) + if (!fromSpecs?.outputs.has(from.artifact) || !toSpecs?.inputs.has(to.artifact)) { problems.push( message( 'edge/misaligned', diff --git a/patches/ds-bot/files/test/workflow-edge-spec.test.js b/patches/ds-bot/files/test/workflow-edge-spec.test.js new file mode 100644 index 000000000..cc6fca52c --- /dev/null +++ b/patches/ds-bot/files/test/workflow-edge-spec.test.js @@ -0,0 +1,41 @@ +// An edge names one fact at each end, and a definition may spell that fact with a space (#1272, +// #1298). Everything that decides whether a definition can run keys on the parsed spec +// (`canonicalSpec`), so `text: draft` and `text:draft` are one fact on an edge too: a definition +// that mixes the two spellings between a node and its edge is runnable and must not be refused as +// misaligned. A genuinely crossed edge is still refused. +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { validateWorkflow } from '../src/shared/workflow.js' + +const definition = (edge) => ({ + id: 'draft-publish', + name: 'Draft and publish', + version: 1, + inputs: [], + deliverables: ['link:published'], + nodes: [ + // The producer spells the fact with a space; the consumer spells it without one. + { id: 'draft', name: 'Draft', role: 'Writer', outputs: ['text: draft'] }, + { id: 'publish', name: 'Publish', role: 'Publisher', inputs: ['text:draft'], outputs: ['link:published'] }, + ], + edges: [edge], + loops: [], +}) + +test('an edge lines up when its two ends spell one fact differently', () => { + // The edge uses the consumer's spelling against the producer's spaced one. + const spacedNode = validateWorkflow(definition({ from: 'draft.text:draft', to: 'publish.text:draft' })) + assert.equal(spacedNode.ok, true, JSON.stringify(spacedNode.problems)) + // The edge uses the producer's spaced spelling against the consumer's canonical one. + const spacedEdge = validateWorkflow(definition({ from: 'draft.text: draft', to: 'publish.text:draft' })) + assert.equal(spacedEdge.ok, true, JSON.stringify(spacedEdge.problems)) +}) + +test('an edge that really does not line up is still refused', () => { + const crossed = validateWorkflow(definition({ from: 'draft.text:draft', to: 'publish.link:published' })) + assert.equal(crossed.ok, false) + assert.ok( + crossed.problems.some((problem) => problem.code === 'edge/misaligned' || problem.code === 'edge/unknown-end'), + JSON.stringify(crossed.problems), + ) +})