Repository navigation
Commit 6b3ba82
fix(showcase): escalation flow routes a project with no budget or no owner (#29018)
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XUDegLcaGYWsX6oKvpcNQWK)_
Fixes #28951
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:showcase
的项目升级流程(`showcase_project_escalation`)在项目没有预算或没有负责人时也能跑完。预算判断先确认有值再比较,"普通"分支改成决策的默认出口;没有负责人时,普通通知和收尾通知改发给组织所有者(`role:owner`),紧急分支里的负责人提醒放在自己的判断后面,没负责人就跳过(旁边的高管分支已经提醒了所有者)。
- 为什么改:项目一变红,没预算的项目在决策节点就报 CEL `null > int`
失败,没负责人的项目在通知节点失败,谁都收不到提醒。这是新用户照抄的参考应用,教的恰是错误写法。
- 风险与回滚:只动示例应用(私有包,不发布),不碰引擎和 CEL。回滚即 revert。
- 先看:`examples/app-showcase/src/automation/flows/index.ts` 的 e2/e3 两条边和
Owner 分支;新测试
`examples/app-showcase/test/project-escalation-optional-fields.test.ts`。
- 两分钟验证:`pnpm --filter @objectstack/example-showcase exec vitest run
test/project-escalation-optional-fields.test.ts`,预期 `Tests 7 passed
(7)`。
- 需要你做的:无,席位落地。
## What changes
- **`examples/app-showcase/src/automation/flows/index.ts`,
`ProjectEscalationFlow`**:
- e2 (`critical`) is `record.budget != null && record.budget > 200000`.
e3 (`normal`) is the `triage` decision's `isDefault: true` edge, the
"otherwise", so a project with no budget routes as the normal path. The
flow's own wording ("Critical budget?") reads a project with no budget
as not critical.
- The critical branch's `Owner` region starts with a `has_owner`
decision whose one edge (`!isBlank(record.owner)`) leads to
`alert_owner`. A project with no owner skips that alert; the `Exec`
branch beside it already alerts `role:owner`, so a fallback there would
alert the exec twice.
- `notify_normal` and `converge` send to `isBlank(record.owner) ?
'role:owner' : record.owner`, so the escalation reaches the
organization's owners when the project has none. The notify node's
required-recipient refusal is unchanged.
- The docblock says why `has(record.budget)` guards nothing here.
- **New
`examples/app-showcase/test/project-escalation-optional-fields.test.ts`**
(7 cases).
## Why
The run's `record` is total over the object's declared fields, so a
project with no budget carries `budget: null`. CEL has no `null > int`,
so the run failed at `triage`. `has(record.owner) ? record.owner : null`
answered `null` for a project with no owner, because `has()` is true on
a null column, and the notify refused a node with no recipient. Either
way nobody was alerted, and the reference flow taught the unguarded
comparison.
Reading on the order's spelling: the order named `has(record.budget) &&
record.budget > 200000`. Measured against the formula engine on `{
budget: null }`, that still faults (`no such overload: dyn‹null› >
int`), because `has()` is true on a null key; the card's own log
(`dyn‹null›`) shows the trigger record carries the key. So the guard is
`!= null`, which is the spelling `skills/objectstack-formula` teaches
(`has()` is NOT a null check), the spelling `showcase_project`'s own
validation rules use (`record.budget != null && …`), and the one the
`validate-null-guards` gate accepts as dominating. The owner guard is
`isBlank()`, so a cleared owner (`''`) routes the same as `null`.
## Tests
- New file, on the production harness for everything the defect touches:
a real `ObjectKernel` with ObjectQL over sqlite and the automation
plugin, the app's real `showcase_project` object and real flow. Each
case writes a project, turns it red through the data engine, and runs
the flow on the row as stored (`record`, plus `previous` before the
update), the way the record-change trigger hands it. Each case first
asserts the stored row carries every optional field it left out as
`null`. Messaging is a recorder of the audience each notify hands over;
`fetch` is stubbed for the incident push.
- no budget, with owner → completes, `notify_normal` success, `alert`
skipped, both notifies to the owner;
- budget 1000, no owner → completes, both notifies to `role:owner`;
- budget 1000, blank owner (`''`) → completes, both to `role:owner`;
- neither → completes, both to `role:owner`;
- control: budget 1000 with owner → e3, `notify_normal` success;
- budget 500000, no owner → completes, `alert_owner` skipped,
`alert_exec` success, one critical alert, to `role:owner`;
- control: budget 500000 with owner → owner and exec alerted.
- **Red on base** (flows file at `origin/main` `135054eb`, test kept): 4
failed / 2 passed (blank-owner case added after this run). Failures:
`condition failed to evaluate as CEL: no such overload: dyn‹null› > int`
(no budget; neither), `Node 'notify_normal' failed: notify: at least one
recipient is required …` (no owner), `Node 'alert' failed: parallel
'alert': branch failed — Node 'alert_owner' failed: notify: …`
(critical, no owner). Both controls green.
- **Ablation 1** (`scripts/ablation-replace.mjs`, anchor hit once, blob
`9272a372` to `9aa0d008`): e2 back to `record.budget > 200000`. 2 failed
/ 5 passed (no budget; neither). Restored: blob equal to HEAD
(`9272a372`), `git diff HEAD` empty.
- **Ablation 2** (blob `9272a372` to `900018be`): the `has_owner` edge
condition replaced by `true`. 1 failed / 6 passed (critical, no owner).
Restored the same way.
- No `dist/` sits on the path: the test imports the flow from `src`.
## Acceptance notes
- **Live REST, base vs head** (`pnpm dev -- --fresh` in
`examples/app-showcase`, the card's steps: create a project as admin,
`PATCH /api/v1/data/showcase_project/ID {"health":"red"}` → 200, three
projects: no budget, no owner, control):
- base (flows file at `135054eb`): two `ERROR Trigger-fired run of flow
'showcase_project_escalation' failed (trigger 'record_change')` lines,
`{"error":"condition failed to evaluate as CEL: no such overload:
dyn‹null› > int … source: budget > 200000 …"}` and `{"error":"Node
'notify_normal' failed: notify: at least one recipient is required …
has(record.owner) ? record.owner : null …"}`. `GET
/api/v1/automation/showcase_project_escalation/runs`: `completed`,
`failed`, `failed`.
- head: no such line. Runs: three `completed`, each `start, triage
success → alert skipped → notify_normal success → converge success`.
- **Same-flow sweep (order ruling 3).** Ordering comparisons in
`src/automation/**` conditions: this flow's e2 (fixed), and three
outside it, not changed here:
- `showcase_budget_approval` start condition `budget > 100000 && budget
!= previous.budget`: a project with no budget faults it. In the live
base run above it logged nothing at the default level and the flow did
not fire, which is the right outcome for a project with no budget. Its
e4/e5 (`budget > 500000` / `<= 500000`) are reached only after that
start condition held, so `budget` is set there.
- `showcase_committee_quorum` start condition `… && total_amount >=
5000`: `total_amount` is a sum summary; whether it reads `null` on a
report with no lines is not measured.
- The same `has(FIELD) ? FIELD : null` guard is the recipient of ten
other notify slots in this file, on other flows (`assignee`, `owner`,
`task.owner`, `task.watcher`). It answers `null` on a null column the
same way, which leaves that notify with no recipient; whether each is
reachable with the field empty is not measured. (Four more slots write
the guarded value into a field, where `null` is a valid answer.)
- **Platform first (triage's question).** No lint rule warns about a
decision with no otherwise edge (the `flow-decision-*` rules are
`inclusive-overlap`, `mode-invalid`, `unconditional-branch`). The
publish-time null-guard gate
(`packages/lint/src/validate-null-guards.ts`) would have refused this
comparison, but its surface ledger still lists flow / edge `condition`
as excluded (the totality blocker is gone since #4953; the widening was
tracked on #4811, now closed). Reported to the seat as a finding; no
engine or lint change here, per the order.
## Verification
Run on `39cd0102`, which merges `origin/main` at `00d0668d` (branch base
`135054eb`); every run below is on that tree. `origin/main` has since
moved by 7 commits, none under `examples/app-showcase/`. Each test
command under `os-verify-lock` (`VERDICT command-exit 0`).
- `pnpm --filter @objectstack/example-showcase test`: Test Files 43
passed (43); Tests 518 passed (518).
- `pnpm --filter @objectstack/example-showcase typecheck`: exit 0.
- `pnpm --filter @objectstack/example-showcase validate`: exit 0; no row
names `showcase_project_escalation`.
- `eslint --no-inline-config` on the two touched files: exit 0.
- Derived gates (`dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, 40 families, each exit captured before any
pipe), reconciled: `✓ dispatch-gates --ran: 40 derived famil(ies)
accounted for — 39 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit
3)`. NOT MEASURED: `check:dual-build-cjs-loads`, reason: it requires
every published package's `dist/`. It first read 3 (seven packages
unbuilt in this worktree); after building them it read 1 with 82 `Cannot
find module '../chunk-….js'` findings across packages this diff does not
touch (overlapping local rebuilds left stale chunk references). This
diff changes no published package, so CI's fresh build is the reading.
- Changeset: none. `@objectstack/example-showcase` is `"private": true`,
so nothing here ships; `skip-changeset` rides the create.
## Human Minimum
None; the seat lands it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5kZjdnPFmYVCV7PUQU)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 474b40e commit 6b3ba82
2 files changed
Lines changed: 257 additions & 6 deletions
File tree
- examples/app-showcase
- src/automation/flows
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1419 | 1419 | | |
1420 | 1420 | | |
1421 | 1421 | | |
| 1422 | + | |
| 1423 | + | |
| 1424 | + | |
| 1425 | + | |
| 1426 | + | |
| 1427 | + | |
| 1428 | + | |
| 1429 | + | |
| 1430 | + | |
| 1431 | + | |
| 1432 | + | |
| 1433 | + | |
| 1434 | + | |
| 1435 | + | |
| 1436 | + | |
| 1437 | + | |
1422 | 1438 | | |
1423 | 1439 | | |
1424 | 1440 | | |
| |||
1446 | 1462 | | |
1447 | 1463 | | |
1448 | 1464 | | |
1449 | | - | |
1450 | | - | |
| 1465 | + | |
| 1466 | + | |
| 1467 | + | |
| 1468 | + | |
| 1469 | + | |
| 1470 | + | |
| 1471 | + | |
1451 | 1472 | | |
1452 | 1473 | | |
1453 | 1474 | | |
| |||
1484 | 1505 | | |
1485 | 1506 | | |
1486 | 1507 | | |
1487 | | - | |
| 1508 | + | |
1488 | 1509 | | |
1489 | 1510 | | |
1490 | 1511 | | |
1491 | 1512 | | |
1492 | 1513 | | |
1493 | | - | |
| 1514 | + | |
1494 | 1515 | | |
1495 | 1516 | | |
1496 | 1517 | | |
1497 | 1518 | | |
1498 | 1519 | | |
1499 | | - | |
1500 | | - | |
| 1520 | + | |
| 1521 | + | |
1501 | 1522 | | |
1502 | 1523 | | |
1503 | 1524 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
0 commit comments