Skip to content

feat(bpm-core-react): let the designer configure the decision policy - #12

Open
coollugi wants to merge 10 commits into
Rytass:mainfrom
coollugi:feat/decision-policy-designer
Open

feat(bpm-core-react): let the designer configure the decision policy#12
coollugi wants to merge 10 commits into
Rytass:mainfrom
coollugi:feat/decision-policy-designer

Conversation

@coollugi

@coollugi coollugi commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

本 PR 把流程設計器的「決策方式」從唯讀說明文字改成可設定的下拉

做了什麼

  • libs/shared 新增 setUserTaskDecisionPolicy command 與 4 個測試(nx test shared 31 → 35 全綠)
  • 簽核節點屬性面板新增「決策方式」下拉,選擇「達到指定門檻」時漸進展開門檻計算方式與門檻值,比照既有 SLA 區塊的模式
  • 新增 3 支 e2e,其中一支專門驗「沒有 decisionPolicy 欄位的既有範本」

兩個設計取捨

一、下拉不提供 SEQUENTIAL

引擎對它的判定與 PARALLEL_ALL 逐字相同(workflow-engine.service.ts 兩者都走 completedCount >= totalCount),提供它等於在畫面上承諾一個尚未存在的依序移交
既有範本若已經使用 SEQUENTIAL,仍會顯示為「依序簽核(既有設定)」而不是空白,改選其他選項才會被覆寫

二、沒有把 decisionPolicy 併進 normalizeUserTaskPolicies

併進去的話所有既有範本一開啟就會被標記為有變更,出現不存在的未儲存提示
改成在讀取端 fallback 到 SINGLE,資料維持原樣、畫面顯示正確

欄位順序

「決策方式」刻意排在「簽核者」之後
因為 setUserTaskApprover 會把 decisionPolicy 重設為 { type: 'SINGLE' }workflow-command.ts:504),順序相反的話使用者先選決策方式再改簽核者,選的設定會無聲消失

第三顆 commit 與功能無關

chore: sync the lockfile with the react-dom peer declaration

63e12b2react-dom 加進 libs/bpm-core-react/package.json 的 peerDependencies,但 pnpm-lock.yaml 沒有同步
本機執行任何 pnpm 指令都會被自動補上這 3 行,而 CI 走 pnpm install --frozen-lockfile
若貴專案另有安排,直接 drop 這一顆即可,不影響前三顆

第四顆 commit:夾住 PERCENTAGE 門檻

fix(bpm-core-react): cap PERCENTAGE quorum threshold and sanitise every write

獨立 review 指出 QUORUM 門檻有一條與 COUNT 同型、但更不容易察覺的死鎖

引擎對 PERCENTAGE 算的是 Math.ceil(totalCount * threshold / 100),任何大於 100 的值恆大於 totalCount
⇒ 門檻填 500、3 位簽核者 ⇒ 需要 15 人同意 ⇒ 這一關永遠無法完成

有兩條寫入路徑會產生這個狀態,第二條特別隱蔽:

  1. 在門檻欄位直接輸入大於 100 的值
  2. 先在 COUNT 模式輸入 500(合法,COUNT 沒有可知的上界),再把「門檻計算方式」切成 PERCENTAGE
    —— 該 Select 的 handler 把既有 threshold 原封不動帶過去

因此夾取沒有放在各個呼叫端,而是收進 updateUserTaskDecisionPolicy,也就是所有 QUORUM 寫入的唯一出口
第二條路徑因此不必修改它自己的 handler 就被涵蓋,日後新增的呼叫端也會自動被守住

COUNT 維持不夾,因為設計期無法得知 resolver 會解析出幾位簽核者(ORG_UNIT_MANAGER / POSITION / ORG_UNIT_POSITION 都會用到),沒有正確的上界可設,所以它的提示文字保留「可能永遠無法通過」的警告
PERCENTAGE 夾到 100 之後該風險對新輸入已不存在,提示文字改為說明自動夾取行為,而不是複製一句夾完就不成立的警告

驗證強度(誠實邊界)

項目 結果
pnpm typecheck 全過
pnpm lint 全過,0 error
nx test shared 35 passed
nx build bpm-core-react 通過
e2e template-designer-w3 14 passed / 1 failed(已實際執行)

唯一失敗的是既有的 normalizes legacy approval policy defaults before saving,與本 PR 無關

該測試在乾淨的 upstream/main56f02ff)上是通過的,因為 main 已含 073df4a(載入時 normalize edge data)與 a7b33a3(放寬 workflowDefinitionHasLinearTask 的 node id 比對)
本分支的分歧點早於這兩顆,合併後即消失

已知限制(本 PR 未涵蓋)

三項皆為刻意取捨,非疏漏,列出供審查判斷:

  1. setUserTaskApprover 仍會清掉 decisionPolicy,AI toolset 路徑未涵蓋
    designer 端已在改簽核者後重新套用既有 policy,但 AI 助理透過 workflow-toolset.ts 產生的是同一顆 command
    ⇒ 對既有節點改簽核者時,QUORUM / PARALLEL_ALL 仍會被靜默重設為 SINGLE
    根治需要改動 shared 契約(workflow-command.ts),超出本 PR 範圍

  2. COUNT 門檻超過實際簽核者人數的死鎖,只做到提示文字層
    引擎的 completedCount >= Math.max(threshold, 1) 未變動
    若希望比照提示原本宣稱的行為(以實際人數為準),需要在引擎加上 Math.min(threshold, totalCount),那會影響既有進行中的案件判定,因此未在本 PR 進行

  3. 夾取只作用於今後由本表單寫入的值
    normalizeUserTaskPolicies 只補「缺少 decisionPolicy」的節點,不會改寫既有值
    ⇒ 資料庫中若已存在 threshold 大於 100 的 PERCENTAGE policy,載入時仍會原樣顯示,且在未觸碰該欄位的情況下存檔會被原樣寫回

coollugi and others added 9 commits August 11, 2026 21:57
`DecisionPolicy` has five variants and the engine honours all of them, but
the designer had no way to change one: `createWorkflowNode` writes
`{ type: 'SINGLE' }` and the only other writer was `setUserTaskApprover`,
which resets it back to SINGLE.

Adds `setUserTaskDecisionPolicy`, mirroring `setUserTaskSla`.

Four cases cover it, including one that pins the reset behaviour: changing
the approver after choosing a policy silently drops back to SINGLE, so the
designer form has to order those two fields accordingly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVBKX88XEgSHGrxzsL2Zjj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVBKX88XEgSHGrxzsL2Zjj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVBKX88XEgSHGrxzsL2Zjj
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVBKX88XEgSHGrxzsL2Zjj
Fixes false/misleading copy in the decision-policy panel (QUORUM count
threshold hint promised clamping the engine never does; the field hint
implied SINGLE and PARALLEL_ANY only match at one approver, when they
are always identical), guards the quorum threshold input against
committing 0 or a fractional value, backfills a missing decisionPolicy
on legacy templates during load normalisation so they can publish
again, preserves a non-default decision policy across an approver edit
(the shared setUserTaskApprover command intentionally resets it to
SINGLE), and replaces the SEQUENTIAL fallback's misleading "queued"
label plus a comment that wrongly credited the AI assistant, which has
no decision-policy tool.
This branch's approval-node panel is long enough that a select opened
near its foot renders its options below the fold at the previous
viewport size, causing intermittent click timeouts in
"creates, designs, publishes, and reviews a template version" and
"configures the return comment requirement and a business-day SLA".
…r spec

"sanitises the quorum threshold input" saves twice and waited for each
save with `expect.poll(() => savedData !== null)`. That only proves
some save landed, not the second one specifically: under full-suite
load a stale callback from the first save could still satisfy the
non-nullness check ahead of the second save actually completing, so
the final assertion intermittently read the first save's value (1)
instead of the second's (2). Poll on the specific threshold each save
is expected to produce instead.
…ry write

A QUORUM threshold has no upper bound while its type is COUNT (there is no
knowable ceiling at design time), but the engine computes
Math.ceil(totalCount * threshold / 100) for PERCENTAGE, so any value above
100 always exceeds totalCount and the step can never complete. Two write
paths could produce that state: typing a large value into the threshold
input, and switching quorumThresholdType from COUNT to PERCENTAGE, which
carried the existing (unbounded) threshold across unchanged.

Sanitisation now lives in updateUserTaskDecisionPolicy, the single function
every QUORUM write funnels through, instead of being duplicated per call
site. It clamps threshold to [1, 100] whenever thresholdType is PERCENTAGE
and floors COUNT at 1, so the thresholdType switch is covered without
touching its own handler, and any future caller is covered automatically.
The PERCENTAGE hint text now documents the auto-clamp instead of copying
COUNT's "may never complete" warning, since clamping to 100 makes that
outcome unreachable for values entered through this form going forward.
@coollugi

Copy link
Copy Markdown
Contributor Author

更正本 PR body 的一句話

body 的「第三顆 commit 與功能無關」一節寫著:

若貴專案另有安排,直接 drop 這一顆即可,不影響前三顆

這句話是錯的,該顆不可移除

chore: sync the lockfile with the react-dom peer declarationpnpm-lock.yaml +3 行)目前是唯一讓 CI 能夠通過 Install dependencies 的東西

main56f02ff)上 libs/bpm-core-react/package.jsonpeerDependenciesreact-dom,但 pnpm-lock.yaml 沒有對應條目,因此:

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile"
* 1 dependencies were added: react-dom@^18.0.0 || ^19.0.0

順帶說明本 PR 目前 CI 顯示 failure 的原因:Test 步驟失敗於 bpm-core-client 的三支時區測試,那是 main 上既有的問題(在乾淨 mainTZ=UTC 即可重現),與本 PR 無關,已於 #15 修正

The publish validator only checks `decisionPolicy?.type`, so an
API-authored template can legitimately store a bare `{ type: 'QUORUM' }`.
`readQuorumParts` trusted `threshold` and `thresholdType` as declared,
which put `String(undefined)` into the threshold number input — the field
renders blank and unusable — and wrote `thresholdType: undefined` back
into the policy on the next keystroke, corrupting it further.

Grant both fields the same tolerance this read path already grants
`type`. Only the shape is repaired: a stored out-of-range value is still
displayed as stored, so the panel never shows a number that differs from
the persisted definition.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants