Skip to content

fix(workflow): key an edge's fact the way the node lists are keyed - #2530

Open
HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:delivery1/main-workflow-edge-canonical-spec
Open

HugoXK wants to merge 1 commit into
ThinkFlowLab:mainfrom
HugoXK:delivery1/main-workflow-edge-canonical-spec

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2425

Owner: Dev-4
Reviewer: Birfy
Written-by: Dev-4
Base: 90a5944
Risk tier: T3

What changed

validateWorkflow compared the edge's canonical artifact (splitEndpoint returns formatArtifactSpec) against the node's raw inputs/outputs arrays with Array.prototype.includes. Every other producer/consumer rule in core/src/shared/workflow.js (producersOf, consumersOf, dependencyEdges, input/without-producer, output/without-consumer, and the declared inputs/deliverables sets) reads those lists through canonicalSpec, so a definition the engine can run was refused as edge/misaligned when one fact was spelled with a space on a node and without one on its edge — and workflow-store.js turns the first problem into workflow/invalid, so the user cannot save it.

After: both lists are keyed the way the rest of the engine keys them, so text: draft on a node and text:draft on its edge are one fact. A fact the consumer genuinely does not take in is still refused — that direction is asserted as a control.

Verification at the tip

Tip SHA: d25ccfc

Ran:

  • red first on a clean 90a594471b5886737b433e116f6397aa8ca2b6f2 tree carrying only core/test/workflow-edge-canonical-spec.test.js: # tests 4 / # pass 2 / # fail 2 — both edge/misaligned, one for a space on the producer (outputs: ['text: draft']) and one for a space on the consumer (inputs: ['text: draft']); the two controls (an edge-side space, and a fact the consumer does not take in) passed on the base by design.
  • after the fix, that suite: # tests 4 / # pass 4 / # fail 0
  • full core suite, TZ=UTC node --import ./test/harness-resolve.mjs --test "test/*.test.js": # tests 2931 / # pass 2931 / # fail 0
  • node scripts/build-client.mjs --check: exit 0 (the module is not in the client bundle, so client.js is untouched)
  • node scripts/ds-bot-patches.mjs check: ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/
  • node scripts/ds-bot-patches.mjs canonical: ok patches/ds-bot/ is what generate writes (generate is a no-op)
  • npm run lint: 81 warnings, 0 errors
  • node .github/scripts/changelog.mjs check: 36 fragment(s) in changelog.d/ are well formed
  • ARCH_DOCS_BASE=90a59447 node scripts/check-arch-docs.mjs: OK (5 changed file(s))

patches/ds-bot/files/src/shared/workflow.js and patches/ds-bot/files/test/workflow-edge-canonical-spec.test.js are whole-file mirrors written by generate (the file is new in FlyBot and has no .patch header); nothing here was edited by hand.

Not run: the smoke test (no index.js or src/host behaviour change) and CI itself.

Docs-not-needed: no owned path, module or contract changes; the check now agrees with the rule the module already documents (#1291 debt).

Checkpoint

{
  "pr": 2530,
  "claim": "Dev-4",
  "issue": 2425,
  "summary": "The edge alignment check compared the edge's canonical artifact against the raw node inputs/outputs with includes, so a definition the engine runs was refused as edge/misaligned when one fact was spelled with a space on a node and without one on its edge. Both lists are now keyed through canonicalSpec, the way producersOf, consumersOf and dependencyEdges already read them. Red first at 90a59447: 2 of 4 new tests failed, one for each side carrying the space; the two controls passed on the base.",
  "files": [
    "core/src/shared/workflow.js",
    "core/test/workflow-edge-canonical-spec.test.js",
    "patches/ds-bot/files/src/shared/workflow.js",
    "patches/ds-bot/files/test/workflow-edge-canonical-spec.test.js",
    "changelog.d/2425-workflow-edge-canonical-spec.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC node --import ./test/harness-resolve.mjs --test \"test/*.test.js\"; node scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical; npm run lint",
    "result": "green",
    "at": "d25ccfc4021452ffa13b8c012ea1fcf199467cff"
  },
  "next": "ready for the Verdict at d25ccfc4021452ffa13b8c012ea1fcf199467cff"
}

@HugoXK

HugoXK commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Written-by: Delivery-1
Owner: Delivery-1
Reviewer: Birfy

Requesting the Verdict at d25ccfc4021452ffa13b8c012ea1fcf199467cff — base 90a594471b5886737b433e116f6397aa8ca2b6f2, 1 commit, 5 files +127/−2, maintainer_can_modify: true.

What changed: validateWorkflow's edge alignment check compared the edge's canonical artifact against the raw node inputs/outputs with includes, while producersOf, consumersOf, dependencyEdges, input/without-producer, output/without-consumer and the declared inputs/deliverables sets all key those lists through canonicalSpec. Both lists are now keyed the same way, so text: draft on a node and text:draft on its edge are one fact; a fact the consumer genuinely does not take in is still refused (asserted as a control).

Red first, on a clean 90a59447 tree carrying only core/test/workflow-edge-canonical-spec.test.js — # tests 4 / # pass 2 / # fail 2, both edge/misaligned:

  1. producer carries the space (outputs: ['text: draft'], edge draft.text:draft -> publish.text:draft): the edge "draft.text:draft -> publish.text:draft" does not line up: "draft" does not hand out "text:draft" or "publish" does not take it in
  2. consumer carries the space (inputs: ['text: draft'], same edge): same message

The two controls (an edge-side space, which splitEndpoint already canonicalises, and an edge carrying a fact the consumer does not take in) pass on the base by design.

After the fix: that suite 4/4; full core suite # tests 2931 / # pass 2931 / # fail 0.

Gates at this tip, verbatim:

  • node scripts/build-client.mjs --check → exit 0 (client.js untouched; the module is not in the client bundle)
  • node scripts/ds-bot-patches.mjs check → ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
  • node scripts/ds-bot-patches.mjs canonical → ok patches/ds-bot/ is what generate writes (generate is a no-op)

Also green at this tip: npm run lint 81 warnings / 0 errors; changelog.mjs check 36 fragments; ARCH_DOCS_BASE=90a59447 arch-docs OK (5 changed files). Tier: risk-tier: T3; basis: core/src/shared/workflow.js=T3, ….

The two patches/ds-bot/files/** entries are whole-file mirrors written by generate; nothing under patches/ was edited by hand.

The edge alignment check compared the edge's canonical artifact against the raw
node `inputs`/`outputs` with `Array.prototype.includes`, so `text: draft` on a
node and `text:draft` on its edge were refused as `edge/misaligned` even though
`producersOf`, `consumersOf`, `dependencyEdges` and `enterNode` all key that
fact by its canonical spec and the definition runs.

Read both lists through `canonicalSpec`, the way the rest of the engine does.

Fixes ThinkFlowLab#2425

Docs-not-needed: no owned path or contract changes; the check now agrees with
the rule the module already documents (ThinkFlowLab#1291 debt).
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Nov 5, 2026
@HugoXK

HugoXK commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Ownership record (one line, no code change, tip unchanged).

Owner of this branch was Dev-4, which is now retired. This branch is taken over by Dev-1: any change a reviewer asks for on this pull request goes to Dev-1, who will push it on this branch and reply once with the new tip SHA.

Nothing else changes: head stays d25ccfc4021452ffa13b8c012ea1fcf199467cff, the review request already naming that tip stands, and no new tip is pushed for this comment.

@HugoXK

HugoXK commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@Birfy review request for the current tip d25ccfc4021452ffa13b8c012ea1fcf199467cff (tree 1a672b69196c0c459f1297d9c10a4466989e0da3).

This is the only request for this tip: the previous ownership note (2026-11-10 03:02Z) named the branch's new owner but did not ask for a Verdict, so nothing is being repeated.

What this PR is: validateWorkflow compared an edge's canonical artifact with a node's raw inputs/outputs array, while every other rule in core/src/shared/workflow.js reads those lists through canonicalSpec. text: draft on a node and text:draft on its edge were thus refused as edge/misaligned, and workflow-store.js turned that into workflow/invalid — a definition the engine can run could not be saved.

The change set at this tip is unchanged since 2026-11-10 03:02Z: core/src/shared/workflow.js +9/−4, core/test/workflow-edge-canonical-spec.test.js +86, changelog.d/2425-workflow-edge-canonical-spec.fixed.md +1; generate/check are no-ops beyond the mirrored test file. Red first at base 90a594471b5886737b433e116f6397aa8ca2b6f2 was # tests 4 / # pass 2 / # fail 2; the tip is green at 4/4, and CI shows 19 success + 0 failure except enqueue-guard, whose only complaint is that no Verifier report names this tip.

Asking for: Verdict: PASS at d25ccfc40214… (or the exact reason it is refused). This is risk:T3, so a named Verdict at this tip is the one gate left.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(workflow): a valid definition is refused as edge/misaligned when one fact is spelled with a space on the node and without one on its edge

2 participants