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 () => {