Skip to content

Commit 0d5c416

Browse files
os-zhuangos-zhuangclaude
authored
docs(pm-dispatch,os-dev): 假引擎的 delete() 一律路由 assertEngineDeleteDispatch,并收编 run-summary 的盲区实例 (#5197) (#5630)
同一天三个互不相同任务的 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: os-zhuang <hr@objectstack.ai> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c57315 commit 0d5c416

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

‎.claude/agents/os-dev.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,14 @@ build/test runs OOM it.** Binding rules:
113113
exactly when 7.28.0's own advisories landed (#5032, the live specimen of
114114
#4961's warning; brace-expansion did the same at 5.0.8). Put the upper
115115
bound at the major boundary and move only the replacement target.
116+
5. **A new fake engine's `delete()` opens with
117+
`assertEngineDeleteDispatch(options)`** from `@objectstack/objectql` — never a
118+
hand-mirrored `if (!where?.id && !multi)`, which has exactly the hole
119+
`check:engine-double-contract` names (#5173's copy passed
120+
`where: { id: { $in: […] } }`). That gate went red on four dev agents' new
121+
tests in two days (#5173 / #5191 / #5192 / #5584), one CI lap each — copy one
122+
of the pinned fakes the gate lists on a green run instead
123+
(`service-automation/src/builtin/crud-bulk-intent.test.ts` is the fullest).
116124

117125
Definition of done, in order:
118126

‎.claude/skills/pm-dispatch/SKILL.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,11 @@ Follow your operating procedure (you are the os-dev agent). Non-negotiables:
837837
- If the issue underspecifies a decision that changes the public contract
838838
(spec schema, API shape, naming), STOP and return status "needs_decision"
839839
with your open questions — do not guess.
840+
- Any NEW fake engine your tests introduce must open its `delete()` with
841+
`assertEngineDeleteDispatch(options)` from `@objectstack/objectql`, never a
842+
hand-mirrored `if (!where?.id && !multi)` — `check:engine-double-contract`
843+
went red on four dev agents' new tests in two days (#5173 / #5191 / #5192 /
844+
#5584), one CI lap each. Copy a pinned fake, don't write the guard.
840845
Return ONLY the JSON report defined in your agent definition.
841846
```
842847

‎packages/services/service-automation/src/run-summary.test.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// all read.
99

1010
import { describe, it, expect } from 'vitest';
11+
import { assertEngineDeleteDispatch } from '@objectstack/objectql';
1112
import { AutomationEngine } from './engine.js';
1213
import type { StepLogEntry, NodeExecutor, RunRecord } from './engine.js';
1314
import { summarizeRun, formatRunSummaryLine } from './run-summary.js';
@@ -407,21 +408,43 @@ describe('node executors report what they touched', () => {
407408
const data: any = {
408409
async find() { return []; },
409410
async findOne() { return null; },
410-
async delete() { return false; },
411+
// #5197 — the double's delete opens with the PRODUCER's own dispatch
412+
// decision, so a sweep this fixture accepts is one `ObjectQL.delete`
413+
// accepts. It used to answer `false` to anything, which is how the
414+
// shape below stayed green here while #5225's real purge flow died
415+
// on it every run.
416+
async delete(_object: string, options: any) {
417+
assertEngineDeleteDispatch(options);
418+
return 0; // driver.deleteMany contract: Promise<number> — nothing matched
419+
},
411420
};
412421
const engine = new AutomationEngine(logger);
413422
registerCrudNodes(engine, { logger, getService: (n: string) => (n === 'data' ? data : undefined) } as never);
414423
engine.registerFlow('f', flowOf(
415424
[
416425
{ id: 'start', type: 'start', label: 'S' },
417-
{ id: 'd', type: 'delete_record', label: 'D', config: { objectName: 'deal', filter: { stale: true } } },
426+
// `multi: true` is what makes a PREDICATE delete reach the driver
427+
// at all (#5393); without it the engine refuses the call, which
428+
// `builtin/crud-bulk-intent.test.ts` pins on the executor side.
429+
// Here the subject is the counter, so the sweep declares intent
430+
// and the driver reports that it matched nothing.
431+
{ id: 'd', type: 'delete_record', label: 'D', config: { objectName: 'deal', filter: { stale: true }, multi: true } },
418432
{ id: 'end', type: 'end', label: 'E' },
419433
],
420434
[{ id: 'e1', source: 'start', target: 'd' }, { id: 'e2', source: 'd', target: 'end' }],
421435
) as never);
422436

423437
const res = await engine.execute('f', { event: 'schedule' } as AutomationContext);
438+
// `acted: 0` alone does NOT say the sweep deleted nothing — it is equally
439+
// what a REFUSED delete leaves behind, so the counter has to be read
440+
// together with the outcome or the case passes for the empty reason
441+
// (measured: with the fake pinned and `multi` dropped, the run fails and
442+
// this assertion alone still went green).
443+
expect(res.success).toBe(true);
424444
expect(res.summary!.acted).toBe(0);
445+
expect(res.summary!.nodes.find((n) => n.nodeId === 'd')).toMatchObject({
446+
nodeType: 'delete_record', runs: 1, failures: 0, acted: 0,
447+
});
425448
});
426449

427450
it('notify counts DELIVERED notifications as acted — a nudge sweep acts by notifying', async () => {

0 commit comments

Comments
 (0)