Skip to content

fix(workflow): Stop Settings Add link from submitting a pair the graph already has - #2174

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/4-settings-link-dedupe
Nov 2, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/4-settings-link-dedupe

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2173

Written-by: Dev-4
Owner: Dev-4
Reviewer: Reviewer
Claim-Id: 5c3608254a1248f88de2047c2212a66f
Branch: dev/4-settings-link-dedupe
Base: b282329
Tier: T2 — the live .github/scripts/risk-gate.mjs judge over this diff prints risk-tier: T2; basis: plugins/workflow/client.js=T2, plugins/workflow/test/client.test.js=T2, changelog.d/2173-settings-link-duplicate.fixed.md=T1, docs/subsystems/workflow.md=T0.

What changed

Settings → Workflows → Add link built the edge and submitted it without reading the edges the graph already lists. The host keys a link by its pair alone (plugins/workflow/src/graph.js:150 edgeKey takes from/to, not the kind), so a second edge over the same two Bots makes normalizeGraph throw Link A → B is listed twice (plugins/workflow/src/graph.js:257) and refuses the whole graph: workflow.edges keeps what it had, and the user is left with one wf-err line and no new link. The same refusal met a different kind over a pair that already had one.

plugins/workflow/client.js now reads the pair before it submits, with the rule the drawer's withEdge (:656) already applies to its own wires:

  • a pair the graph already lists, given the same kind, is a no-op: the form clears and nothing is submitted;
  • a pair the graph already lists, given another kind, is not submitted either — the banner names the kind that is there (This pair already has a {kind} link — replace or delete it first.), so the user reads what blocks the save instead of the host's listed twice;
  • the To list marks the targets the chosen From already reaches — Name (linked), 名称(已连接) — so the pair is visible before the form is filled in.

Three helpers sit beside withEdge: EDGE_KIND_LABEL, blankEdgeForm() (the one empty form the initial state and both clear paths use), and edgeKindOf(edge), which maps a stored edge to the form's kind the way the save writes it (loop === true → loop, when → condition, otherwise forward).

Scope is this one form. The host plugins/workflow/src/graph.js is unchanged: normalizeGraph still refuses a graph that lists one pair twice, so a caller that writes a duplicate through the API is refused exactly as before. This change only stops the Settings form from producing one. P-411 (when/until typed output names) is a separate issue and is untouched.

Red first

On live main b282329d6e79c85c9ebb3f037eeadf36bb9c1396, source untouched, only this branch's plugins/workflow/test/client.test.js dropped into a clean git archive tree (plugins/workflow/client.js there hashes f9c68c59f9de0b2343ae9bb77dd5a2c13858a5da46af79a6a96dcfc420b47085, the main blob):

not ok 109 - #2173: adding a link the graph already has is a no-op, not a refused duplicate save
  error: |-
    the pair the graph already has is not submitted again

    1 !== 0
not ok 110 - #2173: a linked pair given another kind names the link already there instead of submitting
  error: |-
    the conflicting pair is not submitted blind

    1 !== 0
not ok 111 - #2173: the To list marks a target the From node already reaches
  error: 'a target coder already reaches is marked'
  expected: true
  actual: false
# tests 112
# pass 109
# fail 3

The fourth case, #2173: a pair the graph does not have still saves and shows, passes on that base as well — it is the control that the guard does not eat a new pair. With the guard in place the same file on the tip is # tests 112 / # pass 112 / # fail 0.

The test host refuses a repeated pair the way normalizeGraph does, so the assertions are on the committed workflow/graph payload (nothing submitted / one edge) and on the banner text, not on a mock call count.

Verification at the tip

Tip: 7a586a43bb1548acdd2506fd47140183a23e5632 (tree 7d0dcc2d4f1f399b318520018b1f96d0f411352c, single parent b282329d6e79c85c9ebb3f037eeadf36bb9c1396 — live main at push time), 4 files, +135/−3: changelog.d/2173-settings-link-duplicate.fixed.md 1/0, docs/subsystems/workflow.md 2/0, plugins/workflow/client.js 29/3, plugins/workflow/test/client.test.js 103/0. git status --porcelain is empty.

Ran TZ=UTC ARCH_DOCS_BASE=b282329d6e79c85c9ebb3f037eeadf36bb9c1396 npm run verify → exit 0 — core 2648, social 555, tree 127, timeline 140, workflow 354, .github/scripts 598, interop 37, live-e2e 24, every suite # fail 0; lint 74/36/9/2/16 warnings and interop 1, live-e2e 0, 0 errors; arch-docs: OK (4 changed file(s)); ds-bot-patches check — ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5), 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/, patches/ds-bot/ is what generate writes (generate is a no-op). patches/ds-bot/ has no hit on plugins/workflow/client.js, so nothing is mirrored there and generate is not involved.

The branch was rebased onto b282329d at push time (it was built on ff160976, which is b282329d's parent): same four files, same deltas, same blobs.

Not run (and why): the CI package job (pack check, typecheck, actionlint) — it is not part of npm run verify; and the smoke bench, which this change does not reach.

Acceptance

  1. Same pair, same kind — submitting a link the graph already has submits nothing: the form clears, no error banner appears, and workflow.edges is unchanged.
  2. Same pair, another kind — the form is not submitted blind; the banner names the kind already on that pair and says to replace or delete it first, instead of the host's Link A → B is listed twice.
  3. Visible before submitting — the To list marks every target the chosen From already reaches, so the repeat is on screen before Save; a pair the graph does not have still saves and appears.

The host still refuses a duplicated pair written through the API; plugins/workflow/src/graph.js is untouched.

Checkpoint

{
  "pr": 2174,
  "claim": "Dev-4",
  "issue": 2173,
  "summary": "Settings Add link submitted a pair the graph already listed; the host keys a link by its pair, so normalizeGraph refused the whole graph and saved nothing, leaving one wf-err line. The form now reads the pair first: the same kind is a no-op, another kind names the kind already there instead of submitting, and the To list marks the targets the chosen From already reaches. The host is unchanged and still refuses a duplicate written through the API.",
  "files": [
    "plugins/workflow/client.js",
    "plugins/workflow/test/client.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/2173-settings-link-duplicate.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=b282329d6e79c85c9ebb3f037eeadf36bb9c1396 npm run verify (exit 0); red-first on clean b282329d; workflow suite on the tip", "result": "green", "at": "7a586a43bb1548acdd2506fd47140183a23e5632" },
  "next": "none — ready for review"
}

…wLab#2173)

Settings → Workflow graphs → Add link submitted the pair blindly, and the
host's normalizeGraph refuses the whole save on a pair it already lists
("Link A → B is listed twice"), so every other edit in that submission was
lost. The drawer already avoided this through withEdge; the form did not.

The form now reads the pair before it saves: repeating it is a no-op like
the drawer, another kind over the same pair names the kind already there
instead of submitting, and the To list marks the targets the chosen node
already reaches. The host still fails closed — this only stops the form
from producing a pair the host refuses.
@HugoXK
HugoXK marked this pull request as ready for review November 2, 2026 14:05
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy please review #2174 at 7a586a4

@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 7a586a4
Written-by: Reviewer
Attestation: 7a586a4 role=Reviewer

Ran at this tip:

$ cd b2174/tip && node --test plugins/workflow/test/client.test.js
# tests 112  # pass 112  # fail 0
$ cd b2174 && rm -rf red && cp -a tip red && cp base/plugins/workflow/client.js red/plugins/workflow/client.js && cd red && node --test plugins/workflow/test/client.test.js
# tests 112  # pass 109  # fail 3

Review points:

  • Scope. base b282329d6e79 → head 7a586a43 is 1 commit, 4 files: plugins/workflow/client.js +29/−3, plugins/workflow/test/client.test.js +103/−0, docs/subsystems/workflow.md +2, changelog.d/2173-settings-link-duplicate.fixed.md +1. Nothing outside the stated fix.
  • Red first. With only plugins/workflow/client.js swapped back to its base blob (tests left at the tip), the three new workflow: Settings → Add link submits an already-listed pair, so the host rejects the whole save #2173 tests fail: 109/112 pass, 3 fail (adding a link the graph already has is a no-op…, …given another kind names the link already there…, the To list marks a target…). The fix is what turns them green.
  • Matches the host rule. plugins/workflow/src/graph.js:150 keys an edge by from/to alone and normalizeGraph throws listed twice at :257, so a second edge over a pair refuses the whole save whatever its kind. client.js:1616-1628 now reads the pair before it saves: same kind is a no-op that clears the form; another kind is not submitted and the banner names the kind already there; edgeKindOf maps loop/when/forward the way the save writes them. The To list marks reachable targets (linked) (:1771), and the zh strings are added beside the existing Add-link ones. blankEdgeForm passed to useState as a lazy initializer is correct (it is called with no args and returns the object).
  • Checks at this tip. 17 check-runs: all success or cancelled duplicates, the only failure is enqueue-guard at 14:04, and its own reason is no Verifier report names the tip 7a586a43bb15 — this report is that report. tests-ok re-ran green at 14:06.
  • Not run: the package-wide npm test for plugins/workflow needs the workspace install (@deepseek-ai/dsh-llm is missing from a bare tarball), so I ran the affected test file; CI's plugins (workflow) check covers the rest and is green.

@github-actions github-actions Bot added the pass label Nov 2, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 2, 2026
Merged via the queue into ThinkFlowLab:main with commit c21e6da Nov 2, 2026
16 of 18 checks passed
@Birfy
Birfy deleted the dev/4-settings-link-dedupe branch November 2, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workflow: Settings → Add link submits an already-listed pair, so the host rejects the whole save

2 participants