From 383deb3b2608e02f2344fdaa1dc825409ff68242 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 02:29:24 +0000 Subject: [PATCH 1/5] fix(spec): declare ActionEngineFacade.delete's id array, and say which convention is the contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActionEngineFacade.delete` declared `id: string` while the runtime facade has always accepted `string | string[]`, iterating one `ql.delete` per id. The declaration is now `string | string[]`, and the member's doc comment states what the runtime comment used to record as an accident of two handler suites: both spellings are contract, the array form is a convenience over the same per-row path, and it is neither bulk nor atomic. Two consumers follow from the declaration: - `packages/runtime`'s "tolerant of both conventions" comment is retired (a comment correction only; the arm's behaviour is untouched). - `examples/app-todo/src/actions/task.handlers.ts` drops the hand-rolled `ActionContext` copy of the facade — which existed because the published type could not express its array call, and which had already drifted on `find` — for the published `ActionHandlerContext`. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- .../app-todo/src/actions/task.handlers.ts | 49 +++++++++-------- packages/runtime/src/action-execution.ts | 4 +- packages/spec/src/ui/action-params.test.ts | 55 +++++++++++++++++++ packages/spec/src/ui/action-params.zod.ts | 44 +++++++++++++-- 4 files changed, 121 insertions(+), 31 deletions(-) diff --git a/examples/app-todo/src/actions/task.handlers.ts b/examples/app-todo/src/actions/task.handlers.ts index ea100b6ddf..1261086b2b 100644 --- a/examples/app-todo/src/actions/task.handlers.ts +++ b/examples/app-todo/src/actions/task.handlers.ts @@ -14,22 +14,23 @@ * ``` */ -// ─── Handler Context (simplified for example purposes) ────────────── -interface ActionContext { - /** The record being acted upon */ - record: Record; - /** Current authenticated user */ - user: { id: string; name: string }; - /** Data engine for CRUD operations */ - engine: { - update(object: string, id: string, data: Record): Promise; - insert(object: string, data: Record): Promise<{ id: string }>; - find(object: string, query: Record): Promise>>; - delete(object: string, ids: string[]): Promise; - }; - /** Action parameters (from user input / params) */ - params?: Record; -} +import type { ActionHandlerContext } from '@objectstack/spec/ui'; + +// ─── Handler Context ───────────────────────────────────── +// +// `ActionHandlerContext` (`@objectstack/spec/ui`) is the PUBLISHED contract for +// what an action body reads as `ctx` — `record`, `params`, `user`, `session` and +// the trusted `engine` facade — and annotating a handler with it is what the +// contract asks an author to do. +// +// This file used to declare a local simplified copy of that context instead, +// and the copy drifted: its `find` still took an ObjectQL-shaped `query` bag +// long after the contract had settled on a FILTER (#14175). It existed because +// `ActionEngineFacade.delete` was declared as a single id while the runtime had +// always accepted an id array too, so `deleteCompletedTasks` below could not be +// written against the published type at all. The declaration now says +// `string | string[]` (#15117), so the copy is gone and this example +// type-checks against exactly the types a real app gets. /** * Mark a single task as complete. @@ -41,7 +42,7 @@ interface ActionContext { * `beforeUpdate` leg of `src/objects/task.hook.ts`, which runs on the * transition and whose write the strip lets through. */ -export async function completeTask(ctx: ActionContext): Promise { +export async function completeTask(ctx: ActionHandlerContext): Promise { const { record, engine } = ctx; await engine.update('todo_task', record.id as string, { status: 'completed', @@ -49,7 +50,7 @@ export async function completeTask(ctx: ActionContext): Promise { } /** Mark a task as in-progress */ -export async function startTask(ctx: ActionContext): Promise { +export async function startTask(ctx: ActionHandlerContext): Promise { const { record, engine } = ctx; await engine.update('todo_task', record.id as string, { status: 'in_progress', @@ -57,7 +58,7 @@ export async function startTask(ctx: ActionContext): Promise { } /** Clone a task (duplicate with reset status) */ -export async function cloneTask(ctx: ActionContext): Promise<{ id: string }> { +export async function cloneTask(ctx: ActionHandlerContext): Promise<{ id: string }> { const { record, engine } = ctx; const { id, created_at, updated_at, completed_date, ...fields } = record as Record; return engine.insert('todo_task', { @@ -68,7 +69,7 @@ export async function cloneTask(ctx: ActionContext): Promise<{ id: string }> { } /** Mark all selected tasks as complete (bulk) — same `status`-only rule as {@link completeTask} (#7036) */ -export async function massCompleteTasks(ctx: ActionContext): Promise { +export async function massCompleteTasks(ctx: ActionHandlerContext): Promise { const { params, engine } = ctx; const ids = (params?.selectedIds ?? []) as string[]; for (const id of ids) { @@ -79,7 +80,7 @@ export async function massCompleteTasks(ctx: ActionContext): Promise { } /** Delete all completed tasks */ -export async function deleteCompletedTasks(ctx: ActionContext): Promise { +export async function deleteCompletedTasks(ctx: ActionHandlerContext): Promise { const { engine } = ctx; const completed = await engine.find('todo_task', { status: 'completed' }); const ids = completed.map((r) => r.id as string); @@ -89,7 +90,7 @@ export async function deleteCompletedTasks(ctx: ActionContext): Promise { } /** Defer a task by updating its due date (params collected by the action dialog) */ -export async function deferTask(ctx: ActionContext): Promise { +export async function deferTask(ctx: ActionHandlerContext): Promise { const { record, engine, params } = ctx; await engine.update('todo_task', record.id as string, { due_date: params?.new_due_date ? String(params.new_due_date) : null, @@ -99,7 +100,7 @@ export async function deferTask(ctx: ActionContext): Promise { } /** Set a reminder on a task (params collected by the action dialog) */ -export async function setReminder(ctx: ActionContext): Promise { +export async function setReminder(ctx: ActionHandlerContext): Promise { const { record, engine, params } = ctx; await engine.update('todo_task', record.id as string, { reminder_date: params?.reminder_date ? String(params.reminder_date) : null, @@ -108,7 +109,7 @@ export async function setReminder(ctx: ActionContext): Promise { } /** Export tasks to CSV format */ -export async function exportTasksToCSV(ctx: ActionContext): Promise { +export async function exportTasksToCSV(ctx: ActionHandlerContext): Promise { const { engine } = ctx; const tasks = await engine.find('todo_task', {}); const header = 'subject,status,priority,category,due_date'; diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 109403b7d9..f812c06158 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -1466,8 +1466,8 @@ export function buildActionEngineFacade(_deps: ActionExecutionDeps, ql: any, ec? async update(object: string, id: string, data: Record): Promise { await ql.update(object, data, { where: { id }, context }); }, - // Tolerant of both the single-id and array conventions handler suites - // use (CRM handlers pass one id; todo handlers pass an id array). + // Both spellings are DECLARED contract (#15117), not a tolerance: the + // spec's `ActionEngineFacade.delete` takes `string | string[]`. async delete(object: string, idOrIds: string | string[]): Promise { const ids = Array.isArray(idOrIds) ? idOrIds : [idOrIds]; for (const id of ids) { diff --git a/packages/spec/src/ui/action-params.test.ts b/packages/spec/src/ui/action-params.test.ts index c524b3261f..8181e4c704 100644 --- a/packages/spec/src/ui/action-params.test.ts +++ b/packages/spec/src/ui/action-params.test.ts @@ -468,3 +468,58 @@ describe('#14175 — ActionEngineFacade.find takes a FILTER (the `where` half), expect('where' in envelope && 'where' in nested).toBe(true); }); }); + +// --------------------------------------------------------------------------- +// #15117 — `ActionEngineFacade.delete` takes ONE id, or an ARRAY of ids +// --------------------------------------------------------------------------- + +// The declared slot, read off the interface — not a retyped copy of it, so a +// re-narrowing back to the single-id `string` this card retired fails HERE, +// rather than in the first handler that hands the facade a list. +type DeleteIds = Parameters[1]; + +// The type-level pin (the tsc channel, `tsc -p tsconfig.test.json`). `Eq` is +// the strict mutual-assignability test, so the pre-#15117 declaration +// (`string` alone) does not satisfy it — neither does a slot widened all the +// way to `unknown`. Exported for the same reason the sibling pins are. +export type DeleteIdsAcceptsOneOrMany = Assert< Eq< DeleteIds, string | string[] > >; + +describe('#15117 — ActionEngineFacade.delete accepts one id or an array, both as contract', () => { + it('types the second parameter as `string | string[]` (the tsc channel)', () => { + // The value-level half of `DeleteIdsAcceptsOneOrMany` above: literals + // annotated with the slot type, so the runtime run exercises the same + // declaration the type pin reads. + const one: DeleteIds = 'tsk_0001'; + const many: DeleteIds = ['tsk_0001', 'tsk_0002']; + + expect([typeof one, Array.isArray(many)]).toEqual(['string', true]); + }); + + it('positive control — both handler conventions compile, and so does the empty set', () => { + // The single-id convention (the CRM handler suites). + const singleId: DeleteIds = 'tsk_0001'; + // The array convention (`examples/app-todo`'s `deleteCompletedTasks`), + // which before this card was reachable only through a hand-rolled copy of + // `ActionEngineFacade` — the workaround the widening retired. + const idArray: DeleteIds = ['tsk_0001', 'tsk_0002', 'tsk_0003']; + // An empty list deletes nothing and resolves; the member doc says so. + const empty: DeleteIds = []; + + expect([singleId, idArray, empty].length).toBe(3); + }); + + it('refuses at compile time what neither convention admits', () => { + // Each `@ts-expect-error` is itself checked: if the slot ever ADMITS one + // of these, the directive goes unused and `tsc -p tsconfig.test.json` reds. + // @ts-expect-error — an id is a string; a number is not an id under either convention. + const numericId: DeleteIds = 42; + // @ts-expect-error — the array form is an array of ids, not of numbers. + const numericIds: DeleteIds = [1, 2]; + // @ts-expect-error — one level of array, never a nested one. + const nestedIds: DeleteIds = [['tsk_0001']]; + // @ts-expect-error — "delete nothing" is the EMPTY ARRAY, never a null id. + const nullId: DeleteIds = null; + + expect([numericId, numericIds, nestedIds, nullId]).toHaveLength(4); + }); +}); diff --git a/packages/spec/src/ui/action-params.zod.ts b/packages/spec/src/ui/action-params.zod.ts index dd75f9a948..a201efcdc9 100644 --- a/packages/spec/src/ui/action-params.zod.ts +++ b/packages/spec/src/ui/action-params.zod.ts @@ -231,15 +231,49 @@ export function validateActionParams( * context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced * at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here. * - * `find` is the one member whose argument shape the signature alone never - * settled: it takes a bare FILTER — the `where` half of a query — and never - * an ObjectQL query envelope; read its doc comment before writing a handler - * or a test double against it (#14175). + * Two members carry an argument contract the signature alone does not settle, + * and both state it on the member: `find` takes a bare FILTER — the `where` + * half of a query — and never an ObjectQL query envelope (#14175); `delete` + * accepts a single id OR an array of them, both as declared contract, served + * one row at a time (#15117). Read those doc comments before writing a handler + * or a test double against either. */ export interface ActionEngineFacade { insert(object: string, data: Record): Promise<{ id: string }>; update(object: string, id: string, data: Record): Promise; - delete(object: string, id: string): Promise; + /** + * Delete rows of `object` by id. + * + * ## Which convention is the contract: BOTH — one id, or an array of them + * + * `idOrIds` takes a SINGLE id or an ARRAY of ids, and both spellings are + * contract, not a runtime tolerance a handler author has to discover by + * reading `packages/runtime`. Deleting one row is `delete(object, id)`; + * deleting a set is `delete(object, ids)` — a handler that already holds a + * list does NOT have to unroll it into a loop to stay on the contract. + * + * What the array form is, exactly: a convenience over the SAME per-row path, + * never a bulk or atomic delete. `buildActionEngineFacade`'s `delete` arm + * (`packages/runtime/src/action-execution.ts`, `:1471` on `86c50528`) + * normalises the argument to a list and issues one `ql.delete` per id, in + * order, under the caller's execution context. There is no transaction + * around the set: a failure part-way through leaves the ids before it + * deleted and the ids after it untouched, and the rejection a caller sees is + * the one that stopped it. An empty array deletes nothing and resolves. + * + * Until #15117 this slot declared `id: string` while the runtime had been + * accepting both forms all along, described in a comment there as an + * accident of the two handler suites that happened to use them. It was + * neither: it is this declaration. The one first-party suite on the array + * form (`examples/app-todo/src/actions/task.handlers.ts`) could only reach + * it by hand-rolling a copy of this interface — the same hand-written-fake + * pattern #14175 found on `find`, one member over — and that copy drifted, + * exactly as a copy does. Widening the declaration is what retired it. + * + * Both accepted forms, and the argument types that stay refused, are pinned + * in `action-params.test.ts`. + */ + delete(object: string, idOrIds: string | string[]): Promise; /** * Read the rows of `object` that match `filter`. * From 2333a1dfb736bc353e93d89b521b734d16c4dfdb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 02:46:37 +0000 Subject: [PATCH 2/5] chore(changeset): patch for the ActionEngineFacade.delete declaration (#15117) The level is measured, not assumed: nothing is removed and nothing narrows, no runtime behaviour changes, and the sibling re-declaration of the neighbouring member (`find`, #14175) shipped as a patch from the same interface. What ships is a published type that finally describes behaviour that was already served. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- .../15117-action-engine-delete-id-array.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .changeset/15117-action-engine-delete-id-array.md diff --git a/.changeset/15117-action-engine-delete-id-array.md b/.changeset/15117-action-engine-delete-id-array.md new file mode 100644 index 0000000000..e58bea63a6 --- /dev/null +++ b/.changeset/15117-action-engine-delete-id-array.md @@ -0,0 +1,30 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): `ActionEngineFacade.delete` declares the id ARRAY the runtime has always accepted, and says which convention is the contract (#15117) + +`delete(object, id: string)` declared one id. The runtime facade +(`buildActionEngineFacade` in `packages/runtime`) has accepted `string | string[]` +all along — normalising the argument and issuing one `ql.delete` per id — and +described that in a comment as a tolerance two handler suites happened to cause. +The declaration was simply behind the behaviour, and the one first-party suite on +the array form could only reach it by hand-rolling a private copy of the +interface (a copy that had already drifted on `find`). + +The slot is now `delete(object: string, idOrIds: string | string[])`, and the +member's doc comment states the contract instead of leaving it to be inferred +from a runtime comment two packages away: + +- **Both spellings are contract.** One row is `delete(object, id)`; a set is + `delete(object, ids)` — a handler holding a list does not have to unroll it + into a loop to stay on the contract. +- **The array form is a convenience over the same per-row path** — not a bulk or + atomic delete. There is no transaction around the set: a failure part-way + leaves the ids before it deleted. An empty array deletes nothing and resolves. + +Nothing is removed and nothing narrows: every existing single-id call still +type-checks, and no runtime behaviour changes — this release makes the published +type describe what was already being served. Handler authors who copied the +facade into a local context type to reach the array form can delete the copy and +annotate with `ActionHandlerContext` / `ActionHandler` from `@objectstack/spec/ui`. From 2653b827afa3f3604a08e50c813084067c58c374 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:25:41 +0000 Subject: [PATCH 3/5] docs(spec): cite the runtime delete arm by symbol, not by line number (#15117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:spec-docblock-symbol-anchors` refuses a line number as an anchor form, and the citation added with the widening (`:1471`) was one — a NEW finding, not one of the seven day-one residuals. It is now the symbol anchor `packages/runtime/src/action-execution.ts#buildActionEngineFacade`, which is also the more honest citation: this card exists partly because the line numbers the issue quoted had already drifted by three hundred lines. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- packages/spec/src/ui/action-params.zod.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/spec/src/ui/action-params.zod.ts b/packages/spec/src/ui/action-params.zod.ts index a201efcdc9..fccd803f35 100644 --- a/packages/spec/src/ui/action-params.zod.ts +++ b/packages/spec/src/ui/action-params.zod.ts @@ -253,8 +253,8 @@ export interface ActionEngineFacade { * list does NOT have to unroll it into a loop to stay on the contract. * * What the array form is, exactly: a convenience over the SAME per-row path, - * never a bulk or atomic delete. `buildActionEngineFacade`'s `delete` arm - * (`packages/runtime/src/action-execution.ts`, `:1471` on `86c50528`) + * never a bulk or atomic delete. The `delete` arm of + * `packages/runtime/src/action-execution.ts#buildActionEngineFacade` * normalises the argument to a list and issues one `ql.delete` per id, in * order, under the caller's execution context. There is no transaction * around the set: a failure part-way through leaves the ids before it From cc5b7925cbff3f7d72318b83d6a9bb5ac574e03a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 03:49:44 +0000 Subject: [PATCH 4/5] fix(spec): grade the delete widening as minor, and re-verify the flagged docs page (#15117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract review returned FAIL with two must-fixes. 1. The changeset is `minor`, not `patch`. The written rule (`.github/workflows/pr-automation.yml`, maintainer ruling 2026-09-04 batch #35) is that a purely additive widening of a published package's public surface takes at least `minor`, and a commit type may raise a bump but never lower it below what the act requires. The PR's own `Clause-②: yes` line says this widens the accept set, in those words. The `find` precedent it leaned on does not reach: that was a NARROWING, it landed the day the rule was ruled, and the rule disclaims pre-rule `patch` precedents. How the wrong level survived local verification is the more useful half: `check-changeset-no-major` reads the clause-② declaration from the event payload and nothing else, so a local run without `--event` cannot exercise the level axis at all. Its exit 0 was recorded as a reading when the instrument could not have come back the other way. 2. `content/docs/ui/actions.mdx` — the page the repo's own Docs Drift Check flagged on this PR — is re-verified against the rewritten example. Its handler snippet still annotated `ctx: ActionContext`, a type that file no longer declares; it now imports and annotates the published `ActionHandlerContext`. The same snippet also wrote `completed_date`, which is `readonly` on `todo_task` and stamped by the object's `beforeUpdate` hook: copying it made the action refuse itself against `completed_date_required`. Both facts are verified against `task.object.ts` and `task.hook.ts`. The page now states the `delete` convention beside where it already states `find`'s. Folded in: the example's comment attributed to the contract a request the contract does not make. The contract asks for `ActionHandler`; it says so, and says why a file of function declarations annotates `ActionHandlerContext` instead. Comment-only — 11 changed lines, all comments or blank. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- .../15117-action-engine-delete-id-array.md | 6 ++- content/docs/ui/actions.mdx | 38 +++++++++++++++++-- .../app-todo/src/actions/task.handlers.ts | 11 +++++- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/.changeset/15117-action-engine-delete-id-array.md b/.changeset/15117-action-engine-delete-id-array.md index e58bea63a6..1e504d9e4e 100644 --- a/.changeset/15117-action-engine-delete-id-array.md +++ b/.changeset/15117-action-engine-delete-id-array.md @@ -1,5 +1,5 @@ --- -'@objectstack/spec': patch +'@objectstack/spec': minor --- fix(spec): `ActionEngineFacade.delete` declares the id ARRAY the runtime has always accepted, and says which convention is the contract (#15117) @@ -25,6 +25,8 @@ from a runtime comment two packages away: Nothing is removed and nothing narrows: every existing single-id call still type-checks, and no runtime behaviour changes — this release makes the published -type describe what was already being served. Handler authors who copied the +type describe what was already being served. That makes it non-breaking, not a +patch: widening a published parameter is a purely additive widening of a public +surface, which takes at least `minor` whatever the commit type says. Handler authors who copied the facade into a local context type to reach the array form can delete the copy and annotate with `ActionHandlerContext` / `ActionHandler` from `@objectstack/spec/ui`. diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 9e35e501a1..27a8ea1c35 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -137,15 +137,29 @@ For logic that belongs in real source files, point `target` at a handler name and register it in your config's `onEnable` lifecycle hook: ```typescript title="src/actions/task.handlers.ts" -export async function completeTask(ctx: ActionContext): Promise { - const { record, engine } = ctx; // ctx = { record, user, engine, params } +import type { ActionHandlerContext } from '@objectstack/spec/ui'; + +export async function completeTask(ctx: ActionHandlerContext): Promise { + const { record, engine } = ctx; // ctx = { record, params, user, session, engine } await engine.update('todo_task', record.id as string, { status: 'completed', - completed_date: new Date().toISOString(), }); } ``` +Annotate `ctx` with the **published** `ActionHandlerContext`, never a local copy +of it: a hand-rolled context interface drifts from the contract the moment either +side moves, and the drift is silent because the copy is what your handler is +checked against. (The contract's own docblock asks for `ActionHandler`; that is a +function type, so it fits an arrow-function handler assigned to a `const`, while +a function *declaration* like the one above annotates its parameter instead.) + +Write only the fields the action owns. `completed_date` is **readonly** on +`todo_task` — server-owned, stamped by the object's `beforeUpdate` hook on the +transition — so a caller's write to it is stripped from the payload before the +record is validated, and sending it here made this action refuse itself against +that object's `completed_date_required` rule. + **`ctx.engine.find(object, filter)` takes a filter, not a query.** The second argument is the `where` half only — `{ status: 'completed' }`, operators @@ -161,6 +175,24 @@ refuses a primitive or a mistyped `$and` / `$or` / `$not` but still admits double must honour it too. + +**`ctx.engine.delete(object, id)` takes one id — or an array of them.** Both +spellings are contract: `ActionEngineFacade` in `@objectstack/spec/ui` declares +the parameter `string | string[]`, so a handler that already holds a list does +not have to unroll it into a loop to stay on the contract. + +```typescript +await ctx.engine.delete('todo_task', record.id as string); // one row +await ctx.engine.delete('todo_task', ['tsk_0001', 'tsk_0002']); // a set +``` + +The array form is a **convenience over the same per-row path** — not a bulk and +not an atomic delete. The runtime issues one delete per id, in order, with no +transaction around the set: a failure part-way through leaves the ids before it +deleted and the ids after it untouched, and the rejection you see is the one that +stopped it. An empty array deletes nothing and resolves. + + ```typescript title="objectstack.config.ts" export const onEnable = async (ctx: { ql: { registerAction: (...args: unknown[]) => void } }) => { ctx.ql.registerAction('todo_task', 'completeTask', completeTask); diff --git a/examples/app-todo/src/actions/task.handlers.ts b/examples/app-todo/src/actions/task.handlers.ts index 1261086b2b..4cca90c010 100644 --- a/examples/app-todo/src/actions/task.handlers.ts +++ b/examples/app-todo/src/actions/task.handlers.ts @@ -20,8 +20,15 @@ import type { ActionHandlerContext } from '@objectstack/spec/ui'; // // `ActionHandlerContext` (`@objectstack/spec/ui`) is the PUBLISHED contract for // what an action body reads as `ctx` — `record`, `params`, `user`, `session` and -// the trusted `engine` facade — and annotating a handler with it is what the -// contract asks an author to do. +// the trusted `engine` facade. +// +// What the contract asks for in its own words is `ActionHandler`. That is a +// FUNCTION type, and the handlers below are function DECLARATIONS, which cannot +// carry one; rebinding them as `const cloneTask: ActionHandler = ...` would also +// erase their return types, because `ActionHandler` returns `unknown`. Annotating +// the ctx parameter with `ActionHandlerContext` — the type `ActionHandler` is +// defined in terms of — is the same contract, reached the way this file is +// written. // // This file used to declare a local simplified copy of that context instead, // and the copy drifted: its `find` still took an ObjectQL-shaped `query` bag From 8c90c0b0f3cfcc1c321e5cd022c6c4b565c88e9b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 04:31:39 +0000 Subject: [PATCH 5/5] docs(ui): state the true reason a handler must not name a server-owned field (#15117) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous push removed `completed_date` from the handler snippet, which was right, and then explained it with a mechanism that is false. Re-measured: - A handler's `ctx.engine` runs ELEVATED — `buildActionExecutionContext` returns `{ ...base, isSystem: true }` (`packages/runtime/src/action-execution.ts`) — and the read-only strip is gated `if (!opCtx.context?.isSystem)` (`packages/objectql/src/engine.ts`). Nothing is stripped on that path, so "stripped from the payload before the record is validated" never happens. - The refusal does not exist either, for any caller. The hook's stamp is unconditional on the transition and its own docblock says that is precisely so a caller-supplied value is overwritten and survives the strip; a live test asserts it — "a caller that still sends `completed_date` is not punished for it — the hook value wins". The REJECTED row whose mechanism the page described is labelled "Measured before the fix". The true reason is close to the inverse, and is now what the page says: the write is not stripped, it LANDS, and on a write that is not a completion transition the hook does not stamp, so the handler's "now" silently replaces the real completion timestamp. I read the #7036 history at the three sites I cited and wrote it in the present tense, without reading the four sites the conclusion depended on. A citation that exists is not a citation that entails. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH --- content/docs/ui/actions.mdx | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 27a8ea1c35..4bac025a2d 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -154,11 +154,21 @@ checked against. (The contract's own docblock asks for `ActionHandler`; that is function type, so it fits an arrow-function handler assigned to a `const`, while a function *declaration* like the one above annotates its parameter instead.) -Write only the fields the action owns. `completed_date` is **readonly** on -`todo_task` — server-owned, stamped by the object's `beforeUpdate` hook on the -transition — so a caller's write to it is stripped from the payload before the -record is validated, and sending it here made this action refuse itself against -that object's `completed_date_required` rule. +Write only the fields the action owns — and read that literally, because a +handler's `ctx.engine` runs **elevated** (`isSystem`). The read-only strip that +protects a server-owned column from an ordinary caller is gated on *not* being +a system write, so it does not run on a handler's write at all. **What a +handler names, lands.** A server-owned field is therefore more dangerous here +than in a form PUT, not less. + +`completed_date` is the example. It is `readonly` on `todo_task` — the server +owns it, and the object's `beforeUpdate` hook stamps it on the completion +**transition**. Adding `completed_date: new Date().toISOString()` to the write +above looks harmless, and on that transition it is: the hook overwrites it. But +a completion write that is *not* a transition — re-completing a task that is +already `completed` — is not stamped, so nothing overwrites the handler's value +and nothing strips it either, and the action silently replaces the real +completion timestamp with "now". That is why the snippet sends `status` alone. **`ctx.engine.find(object, filter)` takes a filter, not a query.** The second