Skip to content

Commit a6f3a13

Browse files
fix(verify)!: a hooks.run delete resolves with what the engine's delete answers, not a row (#22830)
Fixes #22599 Clause-②: no (narrowing) ## What `stack.hooks.run(object, 'delete', { id }, opts)` in `@objectstack/verify` was declared to resolve with a row (a Promise of `EngineRow`, i.e. `Record` of string to any), for a person (`{ as }`) and for the system (`{ system: true }`). The engine's `delete` answers `true` for the by-id row it removed. So a test that read a field of a delete's answer type-checked, then read `undefined`. This PR types the `delete` answer as what the engine answers, by direction A of the triage grade (comment 6093534577). It does not change any behaviour (direction B, resolving with the deleted row's pre-image, is not taken). ## Premise, re-measured first on `main` `b7cd1af9df` A temporary probe (deleted, never committed) booted the handle fixture through `bootStackOnce` and ran one by-id delete through `hooks.run` as each caller kind. Output line: `PROBE22599 {"person":true,"personType":"boolean","system":true,"systemType":"boolean"}` (1 test passed; `VERDICT command-exit 0`). The premise holds for both callers. ## The change (`packages/verify/src/handle.ts`) - `EngineDeleteAnswer`: the engine's own declared return of `ObjectQL['delete']`, awaited, read off the method with `ReturnType` and `Awaited` rather than re-declared. This follows the precedent `AutomationCondition` sets in the same file (read off `AutomationEngine['evaluateCondition']`). The engine declares `boolean | number`: `boolean` for a by-id delete, `number` for a predicate delete. No separate by-id type is exported, and an `afterDelete` hook may substitute a count on a by-id delete (the engine's `isDeleteResultShape` accepts either). So the door promises the engine's declaration, not a hand-written `boolean`. - `HookWriteAnswer`, keyed on the operation: the row for `insert` / `update` (unchanged, `EngineRow`), `EngineDeleteAnswer` for `delete`. It is a distributive conditional type. - Both `hooks.run` overloads (the person one and the #22301 system one) are now generic over the operation (`Op` constrained to `'insert' | 'update' | 'delete'`) and resolve with `HookWriteAnswer` of `Op`. The implementation is untouched: each `case` still returns the engine call unchanged. - Docblocks say what each write resolves with, and that a test needing the deleted row reads it before the delete. Neither new alias is exported, the same as `AutomationCondition`. The rebuilt `dist/index.d.ts` and `dist/index.d.cts` carry both as local declarations, and the one new import there is `ObjectQL` from `@objectstack/objectql`, a declared runtime dependency of this package. ## PM mechanism assumptions, measured 1. **Split overloads vs a non-literal operation.** Measured standalone with `tsc --strict`: splitting the `delete` literal into its own overloads makes a call whose `operation` is the union type a compile error (`TS2769: No overload matches this call … '"delete"' is not assignable to '"update" | "insert"'`). The generic form keeps that call compiling, and it resolves with `EngineRow | boolean | number`, the union of the answers. That is why the overloads are generic rather than split. The type test pins the union resolution. 2. **Callers of a delete's answer.** All 66 `hooks.run(` call sites in this repo pass a literal operation. The `'delete'` ones all live in `packages/verify` tests and await the answer without reading it (or pass it to `refusalOf`, typed as a Promise of unknown). Downstream sweep after building the closure: `pnpm turbo run typecheck --filter='...@objectstack/verify' --concurrency=2 --continue` (the `...` prefix = verify and every downstream consumer), 74 of 74 tasks successful. All 9 typecheck tasks were cache misses and executed: cli, dogfood, downstream-contract, example-crm, example-multi-package, example-showcase, example-todo, organizations, verify. `@objectstack/verify:build` was a cache miss too, so they read the rebuilt `.d.ts`. No caller needed a change. 3. **The type test can fail.** See the reverse checks below. ## Tests - New `packages/verify/src/handle.delete-answer.test.ts`, a type test. It is compiled by the package's test-layer program (`tsconfig.test.json`, run by `check:test-typecheck` inside `pnpm --filter @objectstack/verify typecheck`). `tsc --listFiles -p tsconfig.test.json` names it once (control: `handle.test.ts` named once), and the build program `tsconfig.json` names it 0 times, as designed. It pins: - a delete answer equal to the engine's declared delete answer, for a person and for the system; - no `id` property on it; - insert and update still `EngineRow`, both callers; - the union operation resolving to the union of the answers. - `pnpm --filter @objectstack/verify typecheck`: exit 0. `check:test-typecheck: OK … 0 file(s) / 0 error(s)`. - `pnpm --filter @objectstack/verify exec vitest run --maxWorkers=2`: `Test Files 31 passed (31)`, `Tests 246 passed (246)`. 31 test files are on disk (30 at the base plus the new one). ### Reverse check 1: the type test against the old declaration (committed first, at `b9399f614`) `node scripts/ablation-replace.mjs --file packages/verify/src/handle.ts --expect 2` put the old return type back on both overloads. The anchor was the new return annotation (a Promise of `HookWriteAnswer` of `Op`) and the replacement was the old one (a Promise of `EngineRow`), described in words here because this body's sanitizer deletes generic types. The tool reported the anchor count going from 2 to 0, and the blob going from `9f3fd264f2be` to `68cc306a9eb7`. Run against it, `tsc --noEmit -p packages/verify/tsconfig.test.json` went red, in the expected direction: - lines 37 and 38, the delete answer for a person and for the system: TS2344; - lines 42 and 43, `not.toHaveProperty('id')`: TS2554; - line 54, the union operation: TS2344; - `handle.ts` line 128: TS6196, `HookWriteAnswer` unused (a by-product of the mutation). The insert and update assertions stayed green. Through `pnpm --filter @objectstack/verify check:test-typecheck` the same mutation read `2 problem(s)`, exit 1. On restore, the tool reported `blob == HEAD (9f3fd26) and git diff HEAD is empty`. ### Reverse check 2: a consumer reads the rebuilt `.d.ts` A temporary file in `packages/qa/dogfood/test/` read `.id` off a system delete's answer. dogfood's program resolves `@objectstack/verify` through its `exports`, so through `dist`. `tsc --noEmit -p packages/qa/dogfood/tsconfig.json` answered `TS2339: Property 'id' does not exist on type 'number | boolean'`, its only error. The file was removed by a trap, and `git status --porcelain` came back empty. ## Gates - Union of the dispatch-derived list and the re-derivation from this diff (`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, 63 commands: all 46 named at dispatch, plus 17 this diff adds, from the changeset and the new test). I ran it once, after the final commit, on `27331626c`, each exit code recorded before any pipe, and every one exited 0. `node scripts/pm/dispatch-gates.mjs --ran` reported `63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN`. Two of them first answered `PREREQUISITE NOT MET` (exit 3), and both went green once the prerequisite was met: - `check-plugin-teardown-shape.mjs --self-test` could not read its pinned positive-control commit on this shallow clone. Fixed by fetching that one commit. - `check:dual-build-cjs-loads` found six packages outside verify's closure with no `dist/`. Fixed by building exactly those six. - `pnpm lint` (the lane blind spot): run as a proven narrowing, not as the full scan. - Population, from eslint's own config: of the 3 changed paths, eslint lints the 2 `.ts` files; the `.changeset` file answers `File ignored because no matching configuration was supplied`. - Count, from `--format json`: 3 results; the 2 linted files have 0 errors and 0 warnings. - Invariance: `eslint.config.mjs` never enables type-aware linting. Every `parserOptions` is `{ ecmaVersion, sourceType }`, with no `project` and no `projectService`. Its four custom plugins (`slot-lookup`, `query-options`, `verify-stand-in`, `comment-swallow`) judge one file's own AST and scope, and the only files it reads are two baselines this diff does not touch. So this diff cannot move the verdict of any untouched file. The full `pnpm lint` is CI's. ## Changeset `.changeset/22599-verify-hooks-run-delete-answer.md`, `@objectstack/verify` `minor`. It is marked **BREAKING** under the launch-window convention and carries `Clause-②: no (narrowing)`. Its ADR-0087 disposition is `not-required (runtime-interface-only packages/verify/src/handle.ts#VerifyHandle)`, which `check:adr-0087-registration` verified (`[BREAKING+bang+clause-②-narrowing] … verified: packages/verify/src/handle.ts#VerifyHandle (interface)`). **Note for the contract review: the migration text is prose, not a FROM → TO label.** The dispatch asked for FROM → TO and the one-line fix, and the changeset states both: the old declared type, the new one, and the assertion to write instead. It does not use the `FROM … → … TO` label convention, because: - every ADR-0087 category this change can honestly claim refuses a body the prescription detector reads as a migration; - `runtime-interface-only` inherits that refusal; - `type-surface-only` is exempt from it, but its predicate 4 reads the base type `EngineRow` as a concrete named type, not erased; - `registered` would need a `packages/spec` ledger entry for a TypeScript-only surface `objectstack migrate meta` cannot reach. This is the shape of the landed `22345-run-provenance-context-retired.md`. The marker itself says the body tells a TypeScript author what to assert instead. ## Acceptance notes - **`update` has the same gap, out of scope here.** The ruling keeps `insert` and `update` on `EngineRow`. The engine's `update` declares `Record` of string to any `| number | null`: `null` when a by-id write's readback leaves the caller's row scope, and a count on the predicate path, which `hooks.run` never takes. Not measured through the door here; recorded for whoever next touches these overloads. - `packages/verify/README.md` shows a system delete without reading its answer. It says nothing false, so it is unchanged. --- _Generated by [Claude Code](https://claude.ai/code/session_01B5CHJNXuuqzChM4w6hkTN4)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8c7d38 commit a6f3a13

3 files changed

Lines changed: 114 additions & 5 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/verify': minor
3+
---
4+
5+
fix(verify)!: `hooks.run(object, 'delete', …)` resolves with what the engine's `delete` answers, not with a row
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (runtime-interface-only packages/verify/src/handle.ts#VerifyHandle) One TypeScript-only declaration moves, and it is not metadata: the `hooks.run` member of the published `VerifyHandle` interface, whose `delete` answer narrows from a row to the engine's own declared delete answer. No spec key, authorable spelling, Zod schema, export name or stored shape moves, and no runtime behaviour changes, so `objectstack migrate meta` has nothing to rewrite. The party affected is a TypeScript author who read a field of a delete's answer: the compiler names that read at their own call site, and the body says what to assert instead. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a test handle's return type, and this diff adds none (not `registered` / `already-registered`); and the member was typed with the named `EngineRow` at the merge base, which predicate 4 reads as concrete, not erased (not `type-surface-only`). -->
10+
11+
**BREAKING** for TypeScript code that reads the answer of a `hooks.run` delete. It ships as `minor` under the launch-window convention for breaking changes. No runtime behaviour changes.
12+
13+
`hooks.run(object, 'delete', { id }, opts)`, for a person (`{ as }`) and for the system (`{ system: true }`), was declared to resolve with a row (`Promise<EngineRow>`). The engine's `delete` answers `true` for the row it removed. So a test that read a field of the answer type-checked, and then read `undefined`.
14+
15+
**What `hooks.run` declares now.** Both overloads are generic over the operation.
16+
17+
- An `insert` or an `update` resolves with the row, as before.
18+
- A `delete` resolves with the engine's own declared delete answer, `boolean | number`. It is read off `ObjectQL['delete']` rather than re-declared, and a `hooks.run` delete, which is always by id, answers `true`.
19+
- A call whose operation is the union `'insert' | 'update' | 'delete'` still type-checks, and resolves with the union of the answers.
20+
21+
**If your code read a field of a delete's answer.** That read was always `undefined` at run time, and it is now a compile error. Assert the answer itself, `expect(await stack.hooks.run(object, 'delete', { id }, opts)).toBe(true)`. If the test needs the deleted row, read it with `stack.rows(object, { id })` before the delete.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#22599] What `hooks.run` declares each write resolves with. A delete
4+
// resolves with what the engine's `delete` answers (`true` for the row it
5+
// removed), never with a row: typed as a row, a test that read a field of a
6+
// delete's answer type-checked and then read `undefined`.
7+
//
8+
// A TYPE test. Its assertions are checked by the package's test-layer tsc
9+
// program (`pnpm --filter @objectstack/verify typecheck`, which runs
10+
// `check:test-typecheck` over `tsconfig.test.json`); at run time they are
11+
// no-ops, and nothing here boots a stack.
12+
13+
import { describe, it, expectTypeOf } from 'vitest';
14+
import type { ObjectQL } from '@objectstack/objectql';
15+
16+
import type { EngineRow, VerifyHandle } from './handle.js';
17+
18+
/** The engine's own declaration, the one the handle reads its delete answer off. */
19+
type EngineDeleteAnswer = Awaited<ReturnType<ObjectQL['delete']>>;
20+
21+
/** Never called: each call is here to be resolved against the overloads, not run. */
22+
function answersOf(hooks: VerifyHandle['hooks'], anyOperation: 'insert' | 'update' | 'delete') {
23+
return {
24+
personDelete: hooks.run('obj', 'delete', { id: 'x' }, { as: 'token' }),
25+
systemDelete: hooks.run('obj', 'delete', { id: 'x' }, { system: true }),
26+
personInsert: hooks.run('obj', 'insert', { name: 'x' }, { as: 'token' }),
27+
systemInsert: hooks.run('obj', 'insert', { name: 'x' }, { system: true }),
28+
personUpdate: hooks.run('obj', 'update', { id: 'x' }, { as: 'token' }),
29+
systemUpdate: hooks.run('obj', 'update', { id: 'x' }, { system: true }),
30+
unionOperation: hooks.run('obj', anyOperation, { id: 'x' }, { as: 'token' }),
31+
};
32+
}
33+
type Answers = { [K in keyof ReturnType<typeof answersOf>]: Awaited<ReturnType<typeof answersOf>[K]> };
34+
35+
describe("hooks.run(object, 'delete', …) resolves with what the engine answers", () => {
36+
it('a delete, by a person or by the system, resolves with the engine delete answer', () => {
37+
expectTypeOf<Answers['personDelete']>().toEqualTypeOf<EngineDeleteAnswer>();
38+
expectTypeOf<Answers['systemDelete']>().toEqualTypeOf<EngineDeleteAnswer>();
39+
});
40+
41+
it("a delete's answer is not a row: reading a field of it does not type-check", () => {
42+
expectTypeOf<Answers['personDelete']>().not.toHaveProperty('id');
43+
expectTypeOf<Answers['systemDelete']>().not.toHaveProperty('id');
44+
});
45+
46+
it('an insert or an update still resolves with the row', () => {
47+
expectTypeOf<Answers['personInsert']>().toEqualTypeOf<EngineRow>();
48+
expectTypeOf<Answers['systemInsert']>().toEqualTypeOf<EngineRow>();
49+
expectTypeOf<Answers['personUpdate']>().toEqualTypeOf<EngineRow>();
50+
expectTypeOf<Answers['systemUpdate']>().toEqualTypeOf<EngineRow>();
51+
});
52+
53+
it('an operation of the union type resolves with the union of the answers', () => {
54+
expectTypeOf<Answers['unionOperation']>().toEqualTypeOf<EngineRow | EngineDeleteAnswer>();
55+
});
56+
});

‎packages/verify/src/handle.ts‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,26 @@ export type EngineRow = Record<string, any>;
107107
*/
108108
type AutomationCondition = Parameters<AutomationEngine['evaluateCondition']>[0];
109109

110+
/**
111+
* [#22599] What the engine's `delete` answers: the method's own declared
112+
* return type, read off it rather than re-declared here, so the door promises
113+
* exactly what the engine promises. The engine declares `boolean | number`:
114+
* whether a by-id delete removed its row, or how many rows a predicate delete
115+
* removed. Every `hooks.run` delete is by id, so it answers `true` for the
116+
* row it removed (a row that is not there is refused before the driver
117+
* runs). It is never the row: a test that needs the deleted row reads it
118+
* before the delete.
119+
*/
120+
type EngineDeleteAnswer = Awaited<ReturnType<ObjectQL['delete']>>;
121+
122+
/**
123+
* [#22599] What one `hooks.run` write resolves with, keyed on its operation:
124+
* the row the engine answers for an `insert` or an `update`, and
125+
* {@link EngineDeleteAnswer} for a `delete`. Distributive, so a call whose
126+
* `operation` is the union type resolves with the union of the answers.
127+
*/
128+
type HookWriteAnswer<Op extends 'insert' | 'update' | 'delete'> = Op extends 'delete' ? EngineDeleteAnswer : EngineRow;
129+
110130
/**
111131
* The refusal a dispatcher-door method (`flows.*`, `actions.run`) throws when
112132
* the route answered a 4xx/5xx — the route's own ADR-0112 envelope, carried
@@ -254,6 +274,10 @@ export interface VerifyHandle {
254274
* engine's own error: assert on its `code` (and `statusCode`), never on
255275
* the message alone.
256276
*
277+
* [#22599] An `insert` or an `update` resolves with the row the engine
278+
* answers. A `delete` resolves with what the engine's `delete` answers,
279+
* `true` for the row it removed, and never with the row.
280+
*
257281
* `update` and `delete` address the row by `input.id`. The caller is a
258282
* person (`{ as }`, this overload) or the system (`{ system: true }`,
259283
* the next one), never both.
@@ -264,12 +288,12 @@ export interface VerifyHandle {
264288
* body })` with no token. That route hands the engine its own guest
265289
* context, keeps only the fields the form collects, and answers `{ id }`.
266290
*/
267-
run(
291+
run<Op extends 'insert' | 'update' | 'delete'>(
268292
object: string,
269-
operation: 'insert' | 'update' | 'delete',
293+
operation: Op,
270294
input: EngineRow,
271295
opts: AsUser,
272-
): Promise<EngineRow>;
296+
): Promise<HookWriteAnswer<Op>>;
273297
/**
274298
* [#22301] The same three writes as the SYSTEM principal ({@link AsSystem}):
275299
* the engine calls above with `context: { isSystem: true }`, the write an
@@ -289,7 +313,12 @@ export interface VerifyHandle {
289313
* so it fires no record-change flow. Insert through `seed` for end-state
290314
* fixture data, and through this door for a write the app should react to.
291315
*/
292-
run(object: string, operation: 'insert' | 'update' | 'delete', input: EngineRow, opts: AsSystem): Promise<EngineRow>;
316+
run<Op extends 'insert' | 'update' | 'delete'>(
317+
object: string,
318+
operation: Op,
319+
input: EngineRow,
320+
opts: AsSystem,
321+
): Promise<HookWriteAnswer<Op>>;
293322
/**
294323
* [#22301] A PREDICATE update: one payload written to every row `where`
295324
* selects. This is the engine's own predicate path, `update(object, data,
@@ -694,7 +723,10 @@ export async function createHandle(kernel: ObjectKernel, origin: string): Promis
694723
hooks: {
695724
// Parameters annotated because the member is overloaded (an overload
696725
// set gives no contextual parameter types); the return type is left to
697-
// inference, as it was before the overload.
726+
// inference, as it was before the overload. [#22599] Inference gives
727+
// `Promise<any>` (the engine's `insert` declares `any`), so tsc does not
728+
// relate each `case` below to its overload's answer: each returns the
729+
// engine call unchanged, the call `HookWriteAnswer` reads its types off.
698730
async run(object: string, operation: 'insert' | 'update' | 'delete', input: EngineRow, opts: AsUser | AsSystem) {
699731
// The call's own shape is judged before anyone is resolved: a
700732
// malformed call is refused for its own reason, not for whatever the

0 commit comments

Comments
 (0)