From 06aa9a1d96fcb369b0e9b49da90796d56d363c9d Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Wed, 5 Aug 2026 20:54:54 +0000 Subject: [PATCH] =?UTF-8?q?docs(pm-dispatch,os-dev):=20=E5=81=87=E5=BC=95?= =?UTF-8?q?=E6=93=8E=E7=9A=84=20delete()=20=E4=B8=80=E5=BE=8B=E8=B7=AF?= =?UTF-8?q?=E7=94=B1=20assertEngineDeleteDispatch,=E5=B9=B6=E6=94=B6?= =?UTF-8?q?=E7=BC=96=20run-summary=20=E7=9A=84=E7=9B=B2=E5=8C=BA=E5=AE=9E?= =?UTF-8?q?=E4=BE=8B=20(#5197)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 同一天三个互不相同任务的 dev agent(3/3)新写假引擎全部踩 `check:engine-double-contract` 判红,错误一模一样 —— 手抄守卫、未路由 `assertEngineDeleteDispatch`(#5173、#5191、 #5192),各花一轮 CI 往返 ≈15 分钟;#5584 的新测试是第四次同款命中(#5604)。这不是门禁 漏了,防线工作正常,代价纯粹是「新增测试 + 需要假引擎 + delete 路径」这个高频组合缺一行 提交前的提示。 os-dev 定义与 pm-dispatch 派发词模板各加一行同措辞纪律,把这轮往返省在提交前。两处都点名 手抄守卫**真有洞**这一实测事实,而不只是「风格不推荐」:#5173 的手抄副本放行了 `where: { id: { $in: [...] } }` —— 它看着像 id,是多行谓词,真引擎无 `multi` 时拒收。 引用的样板是「门禁绿跑时自己列出的 pinned 假引擎」而不是一个会过期的计数。 第三处是 `service-automation/src/run-summary.test.ts` 的盲区实例(#5197 评论定位):它的 内联假引擎 `async delete() { return false; }` 对谓词删除照单全收,而 `delete_record` 自 #5393 起转发 `multi: cfg.multi === true`,所以 `{ objectName: 'deal', filter: { stale: true } }` 这一形状真引擎是 reject。该文件既不在 pinned 也不在 DEBT 台账 —— 门禁的形参个数 判据够不到零形参的 delete(另立 #5629 记录该扫描面缺口及实测口径),所以是检测器盲区,不是 已登记的债。后果不是假设:#5225 里 showcase 的清扫流从上线起每次 `acted: 0`,单测全绿。 收编后按「补声明」处置而非重写断言:该 fixture 从来就不是契约内合法的,补 `multi: true` 声明其批量意图,于是 sweep 真的到达驱动,驱动报告匹配 0 行,用例原本的主题(计数器读 0) 完整保留。执行器侧的 reject 传播已由 `builtin/crud-bulk-intent.test.ts:153` 钉住,不在此 重复。 断言同时加强,这一步是实测逼出来的而非顺手:反向验证(假引擎已收编、`multi` 撤掉)预期红, 实际**仍然绿** —— 因为 `acted: 0` 既是「删了 0 行」也是「删除被拒」留下的痕迹,原用例唯一 的断言两种情形都满足,是为空而绿。补 `res.success` 与该节点 `runs: 1 / failures: 0` 之后 同一撤销才真的判红(`expected false to be true`),用例才在读它声称在读的那件事。 Claude-Session: https://claude.ai/code/session_01GX3sL71LFq8m2usg6VqTSE Co-authored-by: Claude --- .claude/agents/os-dev.md | 8 ++++++ .claude/skills/pm-dispatch/SKILL.md | 5 ++++ .../src/run-summary.test.ts | 27 +++++++++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/.claude/agents/os-dev.md b/.claude/agents/os-dev.md index 193a16a5c2..d9b2ae73c1 100644 --- a/.claude/agents/os-dev.md +++ b/.claude/agents/os-dev.md @@ -113,6 +113,14 @@ build/test runs OOM it.** Binding rules: exactly when 7.28.0's own advisories landed (#5032, the live specimen of #4961's warning; brace-expansion did the same at 5.0.8). Put the upper bound at the major boundary and move only the replacement target. +5. **A new fake engine's `delete()` opens with + `assertEngineDeleteDispatch(options)`** from `@objectstack/objectql` — never a + hand-mirrored `if (!where?.id && !multi)`, which has exactly the hole + `check:engine-double-contract` names (#5173's copy passed + `where: { id: { $in: […] } }`). That gate went red on four dev agents' new + tests in two days (#5173 / #5191 / #5192 / #5584), one CI lap each — copy one + of the pinned fakes the gate lists on a green run instead + (`service-automation/src/builtin/crud-bulk-intent.test.ts` is the fullest). Definition of done, in order: diff --git a/.claude/skills/pm-dispatch/SKILL.md b/.claude/skills/pm-dispatch/SKILL.md index e802958c40..374a85db8e 100644 --- a/.claude/skills/pm-dispatch/SKILL.md +++ b/.claude/skills/pm-dispatch/SKILL.md @@ -837,6 +837,11 @@ Follow your operating procedure (you are the os-dev agent). Non-negotiables: - If the issue underspecifies a decision that changes the public contract (spec schema, API shape, naming), STOP and return status "needs_decision" with your open questions — do not guess. +- Any NEW fake engine your tests introduce must open its `delete()` with + `assertEngineDeleteDispatch(options)` from `@objectstack/objectql`, never a + hand-mirrored `if (!where?.id && !multi)` — `check:engine-double-contract` + went red on four dev agents' new tests in two days (#5173 / #5191 / #5192 / + #5584), one CI lap each. Copy a pinned fake, don't write the guard. Return ONLY the JSON report defined in your agent definition. ``` diff --git a/packages/services/service-automation/src/run-summary.test.ts b/packages/services/service-automation/src/run-summary.test.ts index 7da266420b..e699ea2aaa 100644 --- a/packages/services/service-automation/src/run-summary.test.ts +++ b/packages/services/service-automation/src/run-summary.test.ts @@ -8,6 +8,7 @@ // all read. import { describe, it, expect } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { AutomationEngine } from './engine.js'; import type { StepLogEntry, NodeExecutor, RunRecord } from './engine.js'; import { summarizeRun, formatRunSummaryLine } from './run-summary.js'; @@ -407,21 +408,43 @@ describe('node executors report what they touched', () => { const data: any = { async find() { return []; }, async findOne() { return null; }, - async delete() { return false; }, + // #5197 — the double's delete opens with the PRODUCER's own dispatch + // decision, so a sweep this fixture accepts is one `ObjectQL.delete` + // accepts. It used to answer `false` to anything, which is how the + // shape below stayed green here while #5225's real purge flow died + // on it every run. + async delete(_object: string, options: any) { + assertEngineDeleteDispatch(options); + return 0; // driver.deleteMany contract: Promise — nothing matched + }, }; const engine = new AutomationEngine(logger); registerCrudNodes(engine, { logger, getService: (n: string) => (n === 'data' ? data : undefined) } as never); engine.registerFlow('f', flowOf( [ { id: 'start', type: 'start', label: 'S' }, - { id: 'd', type: 'delete_record', label: 'D', config: { objectName: 'deal', filter: { stale: true } } }, + // `multi: true` is what makes a PREDICATE delete reach the driver + // at all (#5393); without it the engine refuses the call, which + // `builtin/crud-bulk-intent.test.ts` pins on the executor side. + // Here the subject is the counter, so the sweep declares intent + // and the driver reports that it matched nothing. + { id: 'd', type: 'delete_record', label: 'D', config: { objectName: 'deal', filter: { stale: true }, multi: true } }, { id: 'end', type: 'end', label: 'E' }, ], [{ id: 'e1', source: 'start', target: 'd' }, { id: 'e2', source: 'd', target: 'end' }], ) as never); const res = await engine.execute('f', { event: 'schedule' } as AutomationContext); + // `acted: 0` alone does NOT say the sweep deleted nothing — it is equally + // what a REFUSED delete leaves behind, so the counter has to be read + // together with the outcome or the case passes for the empty reason + // (measured: with the fake pinned and `multi` dropped, the run fails and + // this assertion alone still went green). + expect(res.success).toBe(true); expect(res.summary!.acted).toBe(0); + expect(res.summary!.nodes.find((n) => n.nodeId === 'd')).toMatchObject({ + nodeType: 'delete_record', runs: 1, failures: 0, acted: 0, + }); }); it('notify counts DELIVERED notifications as acted — a nudge sweep acts by notifying', async () => {