diff --git a/src/actions/index.ts b/src/actions/index.ts index f36c0af..445184c 100644 --- a/src/actions/index.ts +++ b/src/actions/index.ts @@ -14,7 +14,19 @@ // while empty and infers correctly the moment something is pushed into it. import { CatalogApplyAction, CatalogSyncAction } from './catalog.actions.js'; +import { TaskCompleteAction, TaskSkipAction, TaskUndoAction } from './task.actions.js'; export { CatalogApplyAction, CatalogSyncAction }; +export { TaskCompleteAction, TaskSkipAction, TaskUndoAction }; -export const dulyActions = [CatalogApplyAction, CatalogSyncAction]; +export const dulyActions = [ + CatalogApplyAction, + CatalogSyncAction, + // Object-bound (`objectName: 'duly_task'`), so defineStack() merges them + // into duly_task.actions and the dispatcher can find their declaration. + // An action reachable from a row still needs its handler registered in + // register-handlers.ts — see task.handlers.ts. + TaskCompleteAction, + TaskUndoAction, + TaskSkipAction, +]; diff --git a/src/actions/register-handlers.ts b/src/actions/register-handlers.ts index 21ace8f..ca967ea 100644 --- a/src/actions/register-handlers.ts +++ b/src/actions/register-handlers.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { registerCatalogActionHandlers } from './catalog.handlers.js'; +import { registerTaskActionHandlers } from './task.handlers.js'; /** * Action handler registration. @@ -19,7 +20,7 @@ export interface HandlerRegistrationContext { } export function registerDulyActionHandlers(ql: HandlerRegistrationContext): void { - // Register handlers here, one call per action: - // registerTaskActionHandlers(ql); + // Register handlers here, one call per feature: registerCatalogActionHandlers(ql); + registerTaskActionHandlers(ql); } diff --git a/src/actions/task.actions.ts b/src/actions/task.actions.ts new file mode 100644 index 0000000..e8241c0 --- /dev/null +++ b/src/actions/task.actions.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { P, defineAction } from '@objectstack/spec'; + +import { + TASK_COMPLETE_ACTION, + TASK_OBJECT, + TASK_SKIP_ACTION, + TASK_UNDO_ACTION, +} from './task.handlers.js'; + +/** + * The interaction the whole product rests on. + * + * If ticking a task costs more than a second the list stops being maintained, + * and every metric downstream becomes a report on a dataset nobody keeps. So: + * one click to complete, one click to reverse, and a modal in exactly one + * place — skip, where the object's own validation will refuse the write + * without a reason. + * + * ── What is deliberately absent ─────────────────────────────────────────── + * No `confirmText` on complete or undo. No completion percentage, no required + * note, no evidence gate. `duly_task.enable.files` is on so people CAN attach + * something, never so they must. Undo replaces confirmation: that trade is + * what buys the tick itself having no ceremony, and it only holds while undo + * stays one click away. + * + * ── Predicates are CEL and `record.`-qualified ──────────────────────────── + * A bare `status` evaluates to `null` and hides the action on EVERY record — + * a button that silently never appears, with nothing red anywhere. Action + * predicates are checked by `pnpm validate`; flow predicates are not + * (objectstack#14089). Written correctly here because it is correct, not + * because a gate is watching. + * + * ⚠️ `visible` is a UI hide, not authorization — the button is gone, the + * route is not. Each handler re-checks the same condition server-side; see + * `task.handlers.ts`. + * + * ── Why `type: 'script'` and not a declarative field write ──────────────── + * Because no declarative row-action field write exists. The BULK forms of + * complete and skip ARE declarative — `bulkActionDefs` with + * `operation: 'update'` and a static `patch`, in `src/views/task.view.ts` — + * and this asymmetry is the platform gap filed upstream this round. The full + * argument, including why `type: 'api'` with a hand-written data-API path is + * not the declarative form, is in the `task.handlers.ts` header. + */ + +/** + * `duly_task_complete` — the tick. + * + * Sends `{ status: 'done' }` and nothing else. `completed_at` is stamped by + * the lifecycle hook on the transition; sending it from here would fight the + * readonly strip, whose outcome then depends on value equality. + */ +export const TaskCompleteAction = defineAction({ + name: TASK_COMPLETE_ACTION, + objectName: TASK_OBJECT, + label: 'Complete', + description: 'Mark this task done. One click, no questions — and one click to undo.', + icon: 'check', + type: 'script', + target: TASK_COMPLETE_ACTION, + locations: ['list_item', 'record_header'], + variant: 'primary', + // Lowest order in the group, so the tick is the primary button in the + // record header rather than whatever registered first. + order: 10, + visible: P`record.status == "open" || record.status == "in_progress"`, + // The platform's own one-click reversal: the runtime snapshots the record's + // prior field values and offers Undo on the success toast. It covers the + // mistake noticed IMMEDIATELY; `duly_task_undo` below covers the one noticed + // after the toast is gone. Both exist because the toast is transient and the + // promise this action makes ("an accidental tick costs one click") is not. + undoable: true, + refreshAfter: true, + successMessage: 'Done.', +}); + +/** + * `duly_task_undo` — the reversal, on a just-completed row. + * + * This is the action that makes ticking cheap. It is not a nicety: without it + * the correct design would be a confirm dialog on every completion, which is + * the ceremony this product cannot afford. + */ +export const TaskUndoAction = defineAction({ + name: TASK_UNDO_ACTION, + objectName: TASK_OBJECT, + label: 'Undo', + description: 'Reopen this task. The completion timestamp is cleared with it.', + icon: 'undo-2', + type: 'script', + target: TASK_UNDO_ACTION, + locations: ['list_item', 'record_header'], + variant: 'secondary', + order: 20, + visible: P`record.status == "done"`, + refreshAfter: true, + successMessage: 'Reopened.', +}); + +/** + * `duly_task_skip` — a legitimate outcome, recorded as one. + * + * The reason is collected by the param dialog, which is correct HERE and only + * here: `skip_needs_reason` refuses the write without it, so the alternative + * to the dialog is a button that always fails. + * + * The question rides on `description`, not `confirmText`: an action that + * declares `confirmText` beside non-empty `params` shows two dialogs for one + * decision, and the schema refuses the pair. + */ +export const TaskSkipAction = defineAction({ + name: TASK_SKIP_ACTION, + objectName: TASK_OBJECT, + label: 'Skip', + description: + 'Skipping is a legitimate outcome — the plant was down, there was nothing to return. Recording why is what keeps skip from becoming a synonym for done.', + icon: 'skip-forward', + type: 'script', + target: TASK_SKIP_ACTION, + locations: ['list_item', 'record_more'], + variant: 'secondary', + order: 30, + visible: P`record.status == "open" || record.status == "in_progress"`, + params: [ + { + name: 'skip_reason', + // `field: 'skip_reason'` is deliberately NOT used in place of this pair: + // the param must be required even though the FIELD is optional (a task + // may be imported with no reason; one skipped by hand may not). + label: 'Why skipped', + type: 'text', + required: true, + placeholder: 'The plant was down — there was nothing to return', + helpText: 'Stored on the task. Short is fine; blank is not.', + }, + ], + undoable: true, + refreshAfter: true, + successMessage: 'Skipped.', +}); diff --git a/src/actions/task.handlers.ts b/src/actions/task.handlers.ts new file mode 100644 index 0000000..1fdcca7 --- /dev/null +++ b/src/actions/task.handlers.ts @@ -0,0 +1,294 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { ActionHandler, ActionHandlerContext } from '@objectstack/spec/ui'; + +import type { HandlerRegistrationContext } from './register-handlers.js'; + +/** + * Runtime handlers for the three task-lifecycle row actions. + * + * ── Why these are hand-written and the BULK forms are not ───────────────── + * The bulk forms of complete and skip are pure metadata: the list views + * declare them as `bulkActionDefs` with `operation: 'update'` and a static + * `patch`, which is the platform's DECLARATIVE "set these fields on every + * selected record" — no handler, no code, and the write runs under the + * caller's own permissions. See `src/views/task.view.ts`. + * + * There is no row-level equivalent. `ActionType` is + * `url | form | flow | script | api | modal` — no `update_record`, no action + * `effect`. The two near-misses and why neither is the declarative form: + * + * - `type: 'api'` + `method: 'PATCH'` + `bodyExtra: { status: 'done' }` is a + * declarative HTTP CALL, not a declarative field write. The author + * hand-writes the platform's own data-API path into application metadata + * (`/api/v1/data//` — `basePath` `/api` + version `v1` + + * `crud.dataPrefix` `/data`, measured in @objectstack/rest 17.2.0). Nothing + * binds that string to `objectName`, nothing checks it at author time, and + * it is wrong in the spec's own worked example, which omits the `/data` + * segment the shipped router requires. A metadata app pinning the + * platform's transport route is precisely the contract-first violation the + * repo forbids, and for AI-authored metadata it is the worst available + * shape: it parses green and 404s at the click. + * - `type: 'flow'` + a flow carrying an `update_record` node is declarative, + * but it is three flows to assign one string, on the one surface this repo + * already knows `pnpm validate` does not fully check (objectstack#14089). + * + * Filed upstream as the fourth platform gap of this round; named in the PR. + * + * ── The trap with no author-time gate ───────────────────────────────────── + * An action whose handler is not registered RENDERS, IS CLICKABLE, and fails + * at call time with `Action '' on object 'duly_task' not found` (404 + * from the dispatcher). `pnpm validate` cannot see it. `test/task-actions.test.ts` + * asserts the wiring against a real booted engine, because that assertion is + * the only gate that exists. + * + * ── Why each handler re-checks what the button already gated ────────────── + * `ctx.engine` is TRUSTED — system-elevated and RLS/FLS-bypassing by design + * (`buildActionExecutionContext` stamps `isSystem: true` onto the caller's + * context, and both dispatch surfaces log the write as such). An action's + * `visible` predicate is a UI hide, not authorization: "the button is gone, + * the route is not". So a handler that took `ctx.params.recordId` on trust + * would let any caller who can reach the action route complete anybody's + * task, on an object whose `sharingModel` is `private`. + * + * The subject is therefore read from {@link readSubject}, which refuses + * unless `ctx.record` came back carrying a real field. That is sound because + * of a measured dispatcher detail: the record is loaded under the CALLER's + * execution context, a failed load is swallowed to `{}`, and only then is + * `record.id = recordId` stamped on unconditionally. So `ctx.record.id` is + * present even when the caller cannot read the row — `ctx.record.status` + * (declared `required: true`, so every stored row has one) is what actually + * distinguishes "loaded" from "refused or missing". + * + * This authorization re-check, not the one-line write, is the real cost of + * having no declarative row-action field set — and it is the part of the + * upstream report that matters. + */ + +/** The object these actions are registered and dispatched under. */ +export const TASK_OBJECT = 'duly_task'; + +/** Action names. Exported so the metadata, the wiring and the tests agree by construction. */ +export const TASK_COMPLETE_ACTION = 'duly_task_complete'; +export const TASK_UNDO_ACTION = 'duly_task_undo'; +export const TASK_SKIP_ACTION = 'duly_task_skip'; + +/** + * The complete payload of a completion. ONE field. + * + * `completed_at` and `last_update_at` are stamped by `src/hooks/task.hook.ts` + * on the transition, and both are `readonly: true` so a caller's value is + * stripped at the API boundary. Sending either from here does not merely + * duplicate the hook — the strip drops a key still holding exactly what the + * caller supplied, so the outcome would depend on whether the hook's value + * happened to equal ours. Exported so the row action, the bulk `patch` and + * the tests are one declaration rather than three that agree today. + */ +export const COMPLETE_PATCH = { status: 'done' } as const; + +/** Undo returns the task to the board, not to untouched. The hook clears `completed_at`. */ +export const UNDO_PATCH = { status: 'in_progress' } as const; + +/** Skip's fixed half. `skip_reason` is the caller's and is required — see the action metadata. */ +export const SKIP_PATCH = { status: 'skipped' } as const; + +/** + * The statuses a task can be completed or skipped FROM. + * + * `cancelled` is absent deliberately: a cancelled task is one the org stopped + * asking for, and re-completing it would put it back into on-time rates that + * had correctly forgotten it. + */ +export const ACTIONABLE_STATUSES: readonly string[] = ['open', 'in_progress']; + +/** Undo applies to exactly one prior state. */ +export const UNDOABLE_STATUSES: readonly string[] = ['done']; + +// ── Shapes ────────────────────────────────────────────────────────────────── + +export interface TaskActionParams extends Record { + recordId?: unknown; + skip_reason?: unknown; +} + +export interface TaskActionResult { + action: string; + task: string; + /** The status this write set. */ + status: string; + /** The status the record held before it. Makes a run legible in the audit log. */ + from: string; +} + +/** The subject of a row action: its id, and the status the caller was actually allowed to read. */ +interface TaskSubject { + id: string; + status: string; +} + +// ── Refusals ──────────────────────────────────────────────────────────────── +// +// Thrown with `code` and `status` (ADR-0112 envelope) rather than as bare +// `Error`s. The dispatcher reads `err.status` / `err.code` and maps them onto +// the response, so a refusal reaches the caller as its own answer instead of a +// 500. A bare `throw new Error(...)` would also make the rejection tests +// untrustworthy: `expect(...).toThrow()` alone passes for any throw, including +// the wrong one. + +function refuse(message: string, code: string, status: number): Error { + return Object.assign(new Error(message), { code, status }); +} + +function text(value: unknown): string { + return typeof value === 'string' ? value : ''; +} + +/** + * Resolve the record this action is acting on, or refuse. + * + * Both refusals are fail-closed and deliberately indistinguishable to the + * caller: "you may not read it" and "it is not there" are the same answer on a + * `private` object, and telling them apart would confirm the existence of rows + * the caller cannot see. + */ +function readSubject(ctx: ActionHandlerContext, action: string): TaskSubject { + const record = (ctx.record ?? {}) as Record; + const id = text(ctx.params?.recordId) || text(record.id); + if (!id) { + throw refuse( + `${action} acts on one task and was dispatched without one. Invoke it from a task row, or pass a recordId.`, + 'DULY_TASK_NO_SUBJECT', + 400, + ); + } + + // `status` is `required: true` on duly_task, so every stored row carries + // one. Its absence means the subject read returned nothing under the + // caller's own scope — see the module header. + const status = text(record.status); + if (!status) { + throw refuse( + `Task ${id} is not available to you.`, + 'DULY_TASK_NOT_AVAILABLE', + 404, + ); + } + + return { id, status }; +} + +function requireStatusIn( + subject: TaskSubject, + allowed: readonly string[], + action: string, +): void { + if (allowed.includes(subject.status)) return; + throw refuse( + `${action} applies to a task that is ${allowed.join(' or ')}; this one is ${subject.status}.`, + 'DULY_TASK_WRONG_STATUS', + 409, + ); +} + +// ── duly_task_complete ────────────────────────────────────────────────────── + +/** + * Tick the task. No modal, no confirmation, no note, no percentage. + * + * The ceremony budget for this interaction is one click, and undo is what buys + * it: an accidental tick costs one click to reverse. A five-second tick that + * becomes a five-minute chore is how the list stops being maintained, and + * every metric downstream then reports on a dataset nobody keeps. + */ +export const completeTaskHandler: ActionHandler = async (ctx) => { + const subject = readSubject(ctx, TASK_COMPLETE_ACTION); + requireStatusIn(subject, ACTIONABLE_STATUSES, TASK_COMPLETE_ACTION); + + await ctx.engine.update(TASK_OBJECT, subject.id, { ...COMPLETE_PATCH }); + + const result: TaskActionResult = { + action: TASK_COMPLETE_ACTION, + task: subject.id, + status: COMPLETE_PATCH.status, + from: subject.status, + }; + return result; +}; + +// ── duly_task_undo ────────────────────────────────────────────────────────── + +/** + * Reverse a completion. + * + * Returns the task to `in_progress` rather than to whatever it was: the work + * was demonstrably touched, and sending it back to `open` would erase that. + * The hook clears `completed_at` on the transition out of `done`. + */ +export const undoTaskHandler: ActionHandler = async (ctx) => { + const subject = readSubject(ctx, TASK_UNDO_ACTION); + requireStatusIn(subject, UNDOABLE_STATUSES, TASK_UNDO_ACTION); + + await ctx.engine.update(TASK_OBJECT, subject.id, { ...UNDO_PATCH }); + + const result: TaskActionResult = { + action: TASK_UNDO_ACTION, + task: subject.id, + status: UNDO_PATCH.status, + from: subject.status, + }; + return result; +}; + +// ── duly_task_skip ────────────────────────────────────────────────────────── + +/** + * Record that the task legitimately did not happen — "the plant was down, + * there was nothing to return". + * + * This is the ONE place a modal is correct, because `skip_needs_reason` will + * reject the write without a reason. Forcing that answer to be recorded as + * `done` or left `open` corrupts the data either way. + * + * The reason is NOT re-validated here beyond emptiness. The object's rule is + * the authority and it runs on the write; a second, subtly different check in + * this handler would be a rule that can drift from the one that actually + * decides. What this does is refuse a BLANK reason early, with the same + * outcome the rule would produce, so a programmatic caller that bypassed the + * param dialog gets the same answer as the button. + */ +export const skipTaskHandler: ActionHandler = async (ctx) => { + const subject = readSubject(ctx, TASK_SKIP_ACTION); + requireStatusIn(subject, ACTIONABLE_STATUSES, TASK_SKIP_ACTION); + + const reason = text(ctx.params?.skip_reason).trim(); + if (!reason) { + throw refuse('Say why the task was skipped.', 'DULY_TASK_SKIP_NEEDS_REASON', 400); + } + + await ctx.engine.update(TASK_OBJECT, subject.id, { ...SKIP_PATCH, skip_reason: reason }); + + const result: TaskActionResult = { + action: TASK_SKIP_ACTION, + task: subject.id, + status: SKIP_PATCH.status, + from: subject.status, + }; + return result; +}; + +// ── Wiring ────────────────────────────────────────────────────────────────── + +/** + * Register the three task handlers on the engine. + * + * Called from `registerDulyActionHandlers` in `register-handlers.ts`, which + * `objectstack.config.ts` invokes from `onEnable`. All three register under + * {@link TASK_OBJECT} — the actions declare `objectName: 'duly_task'`, and + * `executeAction` is an exact-string map lookup on `:`, so a + * handler filed under `global` would be unreachable from a task row. + */ +export function registerTaskActionHandlers(ql: HandlerRegistrationContext): void { + ql.registerAction(TASK_OBJECT, TASK_COMPLETE_ACTION, completeTaskHandler); + ql.registerAction(TASK_OBJECT, TASK_UNDO_ACTION, undoTaskHandler); + ql.registerAction(TASK_OBJECT, TASK_SKIP_ACTION, skipTaskHandler); +} diff --git a/src/views/task.view.ts b/src/views/task.view.ts index e888857..159d806 100644 --- a/src/views/task.view.ts +++ b/src/views/task.view.ts @@ -1,6 +1,6 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -import { defineView } from '@objectstack/spec'; +import { P, defineView } from '@objectstack/spec'; const data = { provider: 'object' as const, object: 'duly_task' }; @@ -13,6 +13,76 @@ const columns = [ { field: 'source' }, ]; +/** + * Bulk complete and bulk skip — declarative, and the ONLY declarative field + * write the platform offers. + * + * `operation: 'update'` with a static `patch` IS the platform's "set these + * fields on every selected record": no action, no handler, no code. The write + * runs on the data plane under the CALLER's own permissions — strictly safer + * than N dispatches through an action handler's `ctx.engine`, which is + * system-elevated and RLS-bypassing by design. + * + * A week's worth of ticks in one gesture is the difference between a Monday + * habit and a chore, so this is not a convenience: it is the same interaction + * budget as the row tick, applied to the week. + * + * ── `visible` here is load-bearing, not decoration ──────────────────────── + * It is evaluated once PER SELECTED RECORD, and the run covers only the rows + * that pass. That is what keeps an already-`done` row out of the batch — and + * it has to, for a reason that is MEASURED rather than theoretical: a + * predicate update carries ONE payload for all N rows (`driver.updateMany` + * takes one SET clause), so `task.hook.ts` stamping `completed_at` for a row + * that is genuinely transitioning writes that timestamp to the whole batch. + * Verified against a booted engine: bulk-completing a selection that already + * contains a done row moves that row's `completed_at` to now. The predicate + * is what makes such a selection unreachable from the UI; + * `test/task-actions.test.ts` pins both halves. + * + * Labels are plain strings: an authored def is not i18n-resolved. That is a + * real cost, accepted here because the repo carries no translation bundle yet + * (`dulyTranslations` is empty) and the alternative — promoting the row + * actions via `bulkActions: ['duly_task_complete']` — is N action dispatches + * through the elevated facade instead of one data-plane write. + */ +const bulkActions = [ + { + name: 'duly_task_bulk_complete', + label: 'Complete', + icon: 'check', + variant: 'primary' as const, + operation: 'update' as const, + // The complete payload, same one field as the row action. No + // `completed_at`: the hook owns it, and it is readonly to callers. + patch: { status: 'done' }, + confirmText: 'Mark the selected tasks done.', + confirmLabel: 'Complete', + visible: P`record.status == "open" || record.status == "in_progress"`, + }, + { + name: 'duly_task_bulk_skip', + label: 'Skip', + icon: 'skip-forward', + variant: 'secondary' as const, + operation: 'update' as const, + patch: { status: 'skipped' }, + // One reason for the whole selection. That is honest for the case this + // exists to serve — a plant shutdown skips the week together — and the + // per-task wording stays available on the row action. + params: [ + { + name: 'skip_reason', + label: 'Why skipped', + type: 'text' as const, + required: true, + placeholder: 'The plant was down — there was nothing to return', + help: 'Recorded on every task in the selection.', + }, + ], + visible: P`record.status == "open" || record.status == "in_progress"`, + }, +]; + /** * Task views. * @@ -26,6 +96,7 @@ export const TaskViews = defineView({ type: 'grid', data, columns, + bulkActionDefs: bulkActions, sort: [{ field: 'due_date', order: 'asc' }], }, @@ -40,6 +111,7 @@ export const TaskViews = defineView({ { field: 'status', operator: 'in', value: ['open', 'in_progress'] }, { field: 'visible_from', operator: 'less_than_or_equal', value: '{today}' }, ], + bulkActionDefs: bulkActions, sort: [{ field: 'due_date', order: 'asc' }], }, @@ -55,6 +127,7 @@ export const TaskViews = defineView({ { field: 'due_date', operator: 'less_than', value: '{today}' }, { field: 'status', operator: 'in', value: ['open', 'in_progress'] }, ], + bulkActionDefs: bulkActions, sort: [{ field: 'due_date', order: 'asc' }], }, @@ -70,6 +143,7 @@ export const TaskViews = defineView({ { field: 'status', operator: 'in', value: ['open', 'in_progress'] }, { field: 'last_update_at', operator: 'less_than', value: '{14_days_ago}' }, ], + bulkActionDefs: bulkActions, sort: [{ field: 'last_update_at', order: 'asc' }], }, diff --git a/test/catalog-instantiate.test.ts b/test/catalog-instantiate.test.ts index 950d721..1a6e5c6 100644 --- a/test/catalog-instantiate.test.ts +++ b/test/catalog-instantiate.test.ts @@ -534,13 +534,26 @@ describe('handler wiring', () => { return calls; } - it('every declared action has a registered handler', () => { - const calls = registered(); - const declared = dulyActions.map((a) => a.name).sort(); - const wired = calls.map((c) => c.action).sort(); + /** The catalog actions, which is what the claims in this file are about. */ + const CATALOG_ACTIONS = [CATALOG_APPLY_ACTION, CATALOG_SYNC_ACTION]; + + it('every declared script action has a handler under a key that can reach it', () => { + // Widened from "the wired names equal the declared names" when the first + // OBJECT-BOUND actions landed (duly#4): that spelling asserted the app had + // no actions but these two, so it failed on the next feature rather than on + // a real defect. The bijection is the invariant worth holding, and this is + // it — `executeAction` is an exact-string Map lookup on `:`, + // and the dispatcher tries the action's own object before `global`. + const wired = new Set(registered().map((c) => `${c.object}:${c.action}`)); - expect(declared).toEqual([CATALOG_APPLY_ACTION, CATALOG_SYNC_ACTION].sort()); - expect(wired).toEqual(declared); + for (const action of dulyActions) { + if (action.type !== 'script') continue; + const keys = [`${action.objectName ?? GLOBAL_ACTION_OBJECT}:${action.name}`, `${GLOBAL_ACTION_OBJECT}:${action.name}`]; + expect( + keys.some((k) => wired.has(k)), + `${action.name} renders, is clickable and 404s without one of ${keys.join(' or ')}`, + ).toBe(true); + } }); it('object-less actions register under the canonical "global" key', () => { @@ -548,13 +561,15 @@ describe('handler wiring', () => { // handler filed under any other key is unreachable, however the action is // declared. for (const call of registered()) { + if (!CATALOG_ACTIONS.includes(call.action)) continue; expect(call.object).toBe(GLOBAL_ACTION_OBJECT); expect(typeof call.handler).toBe('function'); } }); - it('the declared actions are object-less and headless, matching that key', () => { + it('the catalog actions are object-less and headless, matching that key', () => { for (const action of dulyActions) { + if (!CATALOG_ACTIONS.includes(action.name)) continue; expect(action.objectName).toBeUndefined(); // `global_nav` was retired in protocol 17 and every surviving location is // object-bound, so `locations: []` is the only honest declaration here. @@ -564,7 +579,7 @@ describe('handler wiring', () => { it('each script action names a target, so it cannot 404 for want of a binding', () => { for (const action of dulyActions) { - expect(action.type).toBe('script'); + if (action.type !== 'script') continue; expect(action.target).toBe(action.name); } }); diff --git a/test/task-actions.test.ts b/test/task-actions.test.ts new file mode 100644 index 0000000..b62d382 --- /dev/null +++ b/test/task-actions.test.ts @@ -0,0 +1,588 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { AppPlugin, ObjectKernel, createStandaloneStack } from '@objectstack/runtime'; + +import stack from '../objectstack.config.js'; +import { Task } from '../src/objects/index.js'; +import { dulyActions } from '../src/actions/index.js'; +import { TaskViews } from '../src/views/task.view.js'; +import { registerDulyActionHandlers } from '../src/actions/register-handlers.js'; +import { + ACTIONABLE_STATUSES, + COMPLETE_PATCH, + SKIP_PATCH, + TASK_COMPLETE_ACTION, + TASK_OBJECT, + TASK_SKIP_ACTION, + TASK_UNDO_ACTION, + UNDO_PATCH, +} from '../src/actions/task.handlers.js'; + +/** + * One-click completion, undo, and skip-with-reason. + * + * Everything here runs against a REAL booted ObjectQL engine (in-memory + * driver) with the app's own `objectstack.config.ts` as the bundle. That is + * not ceremony — it is the only way two of these assertions mean anything: + * + * - **The registration.** An action whose handler is not registered RENDERS, + * IS CLICKABLE, and fails at call time; `pnpm validate` passes green. The + * suite dispatches through the engine's own `executeAction`, so a missing + * or mis-keyed registration surfaces as the same 404 the console would get. + * - **The payload.** "Completing sends `{ status: 'done' }` and nothing else, + * and the record comes back with `completed_at` set" is a claim about the + * hook, the readonly strip and the validation rules acting on the write in + * that order. Only the real pipeline can answer it. + */ + +let data: any; +let kernel: any; + +beforeAll(async () => { + const { plugins } = await createStandaloneStack({ + databaseDriver: 'memory', + skipSeedData: true, + // Point the artifact lookup at a path that cannot exist. Left to its + // default it resolves `/dist/objectstack.json`, and a local + // `pnpm build` leaving one there would make this suite report on the last + // BUILD instead of on `src/` — passing with the barrel entry deleted, and + // behaving differently in CI (where `pnpm test` runs before `pnpm build`). + // Measured on the sibling hook suite, not hypothetical. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', + }); + kernel = new ObjectKernel(); + for (const plugin of plugins) await kernel.use(plugin); + await kernel.use(new AppPlugin(stack, undefined, { skipSeedData: true })); + await kernel.bootstrap(); + data = kernel.getService('data'); + + // `onEnable` is invoked by the CLI boot path, not by `new AppPlugin(...)`, + // so the handlers are wired here through the REAL registration function. + // Registering them by hand instead would let this suite pass with + // `registerTaskActionHandlers` missing from `registerDulyActionHandlers` — + // which is the exact defect it exists to catch. + registerDulyActionHandlers(data); +}, 180_000); + +afterAll(async () => { + await kernel?.shutdown?.(); +}); + +const tick = () => new Promise((resolve) => setTimeout(resolve, 5)); + +const newTask = async (over: Record = {}) => + data.insert('duly_task', { + subject: 'Return the safety inspection', + owner: 'user_alice', + source: 'catalog', + status: 'open', + ...over, + }); + +const read = async (id: string) => data.findOne('duly_task', { where: { id } }); + +/** + * Dispatch an action the way the platform dispatcher does. + * + * Mirrors `handleActionsRequest` in @objectstack/runtime 17.2.0 — the same + * shape the MCP path builds — deliberately including the two details this + * suite depends on: + * + * 1. `record` is loaded under the CALLER's scope and a failed load is + * swallowed to `{}`; + * 2. `record.id = recordId` is then stamped on UNCONDITIONALLY, so an id is + * present even when the row was never read. That is why the handlers key + * their availability check on a real field and not on `record.id`. + * + * `subject: 'unreadable'` reproduces (1)+(2) — the shape a caller who cannot + * read the row actually produces — without needing a second identity. + */ +async function dispatch( + action: string, + opts: { recordId?: string; params?: Record; subject?: 'load' | 'unreadable' } = {}, +): Promise { + const { recordId, params = {}, subject = 'load' } = opts; + + let record: Record = {}; + if (recordId && subject === 'load') { + const got = await read(recordId); + if (got) record = { ...got }; + } + if (record.id == null && recordId) record.id = recordId; + + return data.executeAction(TASK_OBJECT, action, { + record, + user: { id: 'user_alice' }, + session: { userId: 'user_alice' }, + // The slim facade the dispatcher hands a handler. TRUSTED — system + // elevated, RLS/FLS-bypassing by design; reproduced exactly so the suite + // exercises the same write the console does. + engine: { + insert: async (object: string, values: Record) => data.insert(object, values), + update: async (object: string, id: string, values: Record) => + data.update(object, values, { where: { id }, context: { isSystem: true } }), + delete: async (object: string, id: string) => data.delete(object, { where: { id } }), + find: async (object: string, query: Record) => data.find(object, { where: query }), + }, + params: { ...params, recordId, objectName: TASK_OBJECT }, + }); +} + +/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */ +async function refusal(promise: Promise): Promise<{ code: unknown; status: unknown; message: string }> { + try { + await promise; + } catch (error: any) { + return { code: error?.code, status: error?.status, message: String(error?.message ?? '') }; + } + throw new Error('expected the dispatch to be refused, but it resolved'); +} + +const declared = (name: string) => dulyActions.find((a: any) => a?.name === name) as any; + +// ── The failure mode that reads as success ───────────────────────────────── +// +// There is no author-time gate for an unregistered handler. This block IS the +// gate. +describe('registration', () => { + it('every task action is declared in the actions barrel, bound to duly_task', () => { + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + const action = declared(name); + expect(action, `${name} must be in dulyActions or it is dead metadata`).toBeDefined(); + expect(action.objectName).toBe(TASK_OBJECT); + expect(action.type).toBe('script'); + // A `script` action with neither `body` nor `target` is refused at + // author time. `target` is what the dispatcher resolves the handler by. + expect(action.target).toBe(name); + } + }); + + it('reaches duly_task.actions — defineStack merged it onto the object', () => { + const merged = (data.getSchema(TASK_OBJECT)?.actions ?? []).map((a: any) => a?.name); + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + expect(merged, `${name} must reach the object the row renders`).toContain(name); + } + }); + + it('every declared handler-backed action has a handler registered under duly_task', () => { + const registered = data + .listRegisteredActions() + .map((r: { objectName: string; actionName: string }) => `${r.objectName}:${r.actionName}`); + + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + // `executeAction` is an exact-string map lookup on `:`, so + // a handler filed under `global` is unreachable from a task row even + // though it registered without error. + expect(registered, `${name} renders and 404s without this`).toContain(`${TASK_OBJECT}:${name}`); + } + }); + + it('dispatches for real — the wiring end to end, not just the registry', async () => { + const task = await newTask(); + const result: any = await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + expect(result?.action).toBe(TASK_COMPLETE_ACTION); + expect(result?.task).toBe(task.id); + }); + + it('an unregistered action name is refused the way a missing handler would be', async () => { + // Pins the failure shape the registration assertions above protect + // against, so "not found" stays recognisable if the engine reworks it. + await expect(dispatch('duly_task_not_registered', { recordId: 'x' })).rejects.toThrow(/not found/i); + }); +}); + +// ── Predicates ───────────────────────────────────────────────────────────── +describe('predicates', () => { + const taskFields = Object.keys((Task as any).fields ?? {}); + + const scan = (source: string, where: string) => { + expect(source.length, `${where} must carry a predicate`).toBeGreaterThan(0); + for (const field of taskFields) { + // Every mention of a duly_task field must be `record.`-qualified. A bare + // `status` evaluates to null and hides the action on EVERY record — a + // button that silently never appears, with nothing red anywhere. + const bare = new RegExp(`(^|[^.\\w])${field}\\b`); + expect(bare.test(source), `${where}: "${field}" is not record.-qualified — ${source}`).toBe(false); + } + }; + + it('every action predicate is CEL and record.-qualified', () => { + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + const visible = declared(name)?.visible; + expect(visible?.dialect, `${name}.visible must be CEL`).toBe('cel'); + scan(String(visible?.source ?? ''), `${name}.visible`); + } + }); + + it('every bulk predicate is CEL and record.-qualified', () => { + for (const def of allBulkDefs()) { + expect(def.visible?.dialect, `${def.name}.visible must be CEL`).toBe('cel'); + scan(String(def.visible?.source ?? ''), `${def.name}.visible`); + } + }); + + it('complete and skip are offered on exactly the actionable statuses', () => { + // Not a re-spelling of the source: this drives the same vocabulary the + // handlers enforce, so a status added to one and not the other is caught. + for (const name of [TASK_COMPLETE_ACTION, TASK_SKIP_ACTION]) { + const source = String(declared(name)?.visible?.source ?? ''); + for (const status of ACTIONABLE_STATUSES) expect(source).toContain(`"${status}"`); + expect(source).not.toContain('"done"'); + expect(source).not.toContain('"cancelled"'); + } + expect(String(declared(TASK_UNDO_ACTION)?.visible?.source ?? '')).toContain('"done"'); + }); +}); + +// ── The payload, and what must never be in it ────────────────────────────── +describe('payload', () => { + const SERVER_OWNED = ['completed_at', 'last_update_at']; + + it('completing sends { status: done } and nothing else', () => { + expect(COMPLETE_PATCH).toEqual({ status: 'done' }); + expect(Object.keys(COMPLETE_PATCH)).toHaveLength(1); + }); + + it('undo returns the task to in_progress', () => { + expect(UNDO_PATCH).toEqual({ status: 'in_progress' }); + }); + + it('no payload anywhere writes a server-owned timestamp', () => { + // The hook is their one writer, and both are `readonly: true`: a caller's + // value is stripped only while the key still holds exactly what the caller + // supplied, so sending one makes the outcome depend on value equality. + const payloads: Array<[string, Record]> = [ + ['COMPLETE_PATCH', COMPLETE_PATCH], + ['UNDO_PATCH', UNDO_PATCH], + ['SKIP_PATCH', SKIP_PATCH], + ...allBulkDefs().map((d): [string, Record] => [`${d.name}.patch`, d.patch ?? {}]), + ]; + for (const [where, payload] of payloads) { + for (const key of SERVER_OWNED) { + expect(Object.keys(payload), `${where} must not write ${key}`).not.toContain(key); + } + } + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + const params = (declared(name)?.params ?? []) as Array<{ name?: string; field?: string }>; + for (const p of params) { + expect(SERVER_OWNED, `${name} must not collect a server-owned field`).not.toContain(p.name ?? p.field ?? ''); + } + } + }); + + it('no action asks for a percentage, an evidence upload or a confirmation', () => { + // The product invariant, asserted rather than remembered: completion never + // requires evidence, a note, or a percentage, and undo replaces the + // "are you sure?" step on the two actions that write without a dialog. + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION]) { + const action = declared(name); + expect(action.confirmText, `${name} must not confirm — undo is the confirmation`).toBeUndefined(); + expect(action.params ?? [], `${name} must not collect anything`).toHaveLength(0); + } + const everyParam = [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION] + .flatMap((n) => (declared(n)?.params ?? []) as Array<{ name?: string; type?: string }>); + for (const p of everyParam) { + expect(p.type, 'no action may demand an attachment to close a task').not.toBe('file'); + expect(p.name).not.toMatch(/percent|progress|evidence|attachment/i); + } + }); + + it('complete offers the platform Undo affordance', () => { + // `undoable` is the runtime's own one-click reversal: it snapshots the + // record's prior field values and offers Undo on the success toast. It is + // what makes a no-confirmation tick defensible at the moment of the + // mistake; `duly_task_undo` covers the mistake noticed after the toast. + expect(declared(TASK_COMPLETE_ACTION)?.undoable).toBe(true); + }); +}); + +// ── Behaviour against the real write pipeline ────────────────────────────── +describe('complete', () => { + it('sets status done and the record comes back with completed_at', async () => { + const task = await newTask(); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + + const row = await read(task.id); + expect(row.status).toBe('done'); + expect(row.completed_at, 'the hook stamps it; the validation rule refuses the write without it').toBeTruthy(); + expect(row.last_update_at).toBeTruthy(); + }); + + it('works from in_progress as well as open', async () => { + const task = await newTask({ status: 'in_progress' }); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + expect((await read(task.id)).status).toBe('done'); + }); + + it('is refused on a task that is already done', async () => { + const task = await newTask(); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + const before = await read(task.id); + await tick(); + + const { code, status } = await refusal(dispatch(TASK_COMPLETE_ACTION, { recordId: task.id })); + expect(code).toBe('DULY_TASK_WRONG_STATUS'); + expect(status).toBe(409); + + // And the refusal is not merely a message: the original completion instant + // survives it. Re-stamping would silently move the record's history. + expect((await read(task.id)).completed_at).toBe(before.completed_at); + }); + + it('is refused on a cancelled task', async () => { + const task = await newTask({ status: 'cancelled' }); + const { code, status } = await refusal(dispatch(TASK_COMPLETE_ACTION, { recordId: task.id })); + expect(code).toBe('DULY_TASK_WRONG_STATUS'); + expect(status).toBe(409); + }); +}); + +describe('undo', () => { + it('returns the record to in_progress with completed_at null', async () => { + const task = await newTask(); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + expect((await read(task.id)).completed_at).toBeTruthy(); + + await dispatch(TASK_UNDO_ACTION, { recordId: task.id }); + + const row = await read(task.id); + expect(row.status).toBe('in_progress'); + expect(row.completed_at ?? null).toBeNull(); + }); + + it('is refused on a task that was never completed', async () => { + const task = await newTask(); + const { code, status } = await refusal(dispatch(TASK_UNDO_ACTION, { recordId: task.id })); + expect(code).toBe('DULY_TASK_WRONG_STATUS'); + expect(status).toBe(409); + }); + + it('a completion can be taken back and retaken', async () => { + const task = await newTask(); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + const first = (await read(task.id)).completed_at; + await dispatch(TASK_UNDO_ACTION, { recordId: task.id }); + await tick(); + await dispatch(TASK_COMPLETE_ACTION, { recordId: task.id }); + + const row = await read(task.id); + expect(row.status).toBe('done'); + expect(row.completed_at > first, 'the second completion is its own instant').toBe(true); + }); +}); + +describe('skip', () => { + it('records the reason and leaves completed_at unset', async () => { + const task = await newTask(); + await dispatch(TASK_SKIP_ACTION, { + recordId: task.id, + params: { skip_reason: 'The plant was down — there was nothing to return' }, + }); + + const row = await read(task.id); + expect(row.status).toBe('skipped'); + expect(row.skip_reason).toBe('The plant was down — there was nothing to return'); + expect(row.completed_at ?? null, 'a skip is not a completion').toBeNull(); + expect(row.last_update_at, 'recording a skip is progress on the task').toBeTruthy(); + }); + + it('without a reason is refused, and nothing is written', async () => { + const task = await newTask(); + const { code, status } = await refusal(dispatch(TASK_SKIP_ACTION, { recordId: task.id })); + expect(code).toBe('DULY_TASK_SKIP_NEEDS_REASON'); + expect(status).toBe(400); + expect((await read(task.id)).status, 'the refusal must not be a partial write').toBe('open'); + }); + + it('with a blank reason is refused too', async () => { + const task = await newTask(); + const { code } = await refusal(dispatch(TASK_SKIP_ACTION, { recordId: task.id, params: { skip_reason: ' ' } })); + expect(code).toBe('DULY_TASK_SKIP_NEEDS_REASON'); + }); + + it("the OBJECT's rule is the authority — a direct write with no reason is refused by it", async () => { + // The handler's early refusal is a courtesy for the programmatic caller. + // This asserts the rule that actually decides, with its own message, so + // the two cannot drift into one guard doing all the work. + const task = await newTask(); + let caught: any; + try { + await data.update('duly_task', { id: task.id, status: 'skipped' }); + } catch (error) { + caught = error; + } + expect(caught, 'skip_needs_reason must refuse this').toBeDefined(); + expect(caught.code).toBe('VALIDATION_FAILED'); + expect(caught.message).toBe('Say why the task was skipped.'); + }); + + it('declares the reason as a required param — the one modal that is correct', () => { + const params = (declared(TASK_SKIP_ACTION)?.params ?? []) as Array; + expect(params).toHaveLength(1); + expect(params[0].name).toBe('skip_reason'); + expect(params[0].required).toBe(true); + // Pairing `confirmText` with params shows two dialogs for one decision, + // and the schema refuses the pair; the question rides on `description`. + expect(declared(TASK_SKIP_ACTION)?.confirmText).toBeUndefined(); + expect(String(declared(TASK_SKIP_ACTION)?.description ?? '').length).toBeGreaterThan(0); + }); +}); + +// ── Availability: the button is a hide, the handler is the gate ──────────── +describe('authorization', () => { + it('refuses when the subject did not load under the caller scope', async () => { + const task = await newTask(); + // The dispatcher stamps `record.id` on even when the read returned + // nothing, so an id alone proves nothing about read access. + const { code, status } = await refusal( + dispatch(TASK_COMPLETE_ACTION, { recordId: task.id, subject: 'unreadable' }), + ); + expect(code).toBe('DULY_TASK_NOT_AVAILABLE'); + expect(status).toBe(404); + expect((await read(task.id)).status, 'nothing may be written on a refusal').toBe('open'); + }); + + it('refuses a dispatch with no subject at all', async () => { + const { code, status } = await refusal(dispatch(TASK_COMPLETE_ACTION, {})); + expect(code).toBe('DULY_TASK_NO_SUBJECT'); + expect(status).toBe(400); + }); + + it('refuses an unknown record id', async () => { + const { code, status } = await refusal(dispatch(TASK_COMPLETE_ACTION, { recordId: 'duly_task-nope' })); + expect(code).toBe('DULY_TASK_NOT_AVAILABLE'); + expect(status).toBe(404); + }); + + it('all three actions are gated, not just complete', async () => { + for (const name of [TASK_COMPLETE_ACTION, TASK_UNDO_ACTION, TASK_SKIP_ACTION]) { + const { code } = await refusal(dispatch(name, { recordId: 'duly_task-nope' })); + expect(code, `${name} must refuse an unreadable subject`).toBe('DULY_TASK_NOT_AVAILABLE'); + } + }); +}); + +// ── Bulk ─────────────────────────────────────────────────────────────────── + +/** Every bulk def declared on any task list view. */ +function allBulkDefs(): Array { + const views = TaskViews as any; + const entries = [views.list, ...Object.values(views.listViews ?? {})]; + return entries.flatMap((v: any) => (v?.bulkActionDefs ?? []) as Array); +} + +describe('bulk', () => { + const BULK_COMPLETE = 'duly_task_bulk_complete'; + const BULK_SKIP = 'duly_task_bulk_skip'; + + it('is offered on every multi-select grid, and not on the calendar', () => { + const views = TaskViews as any; + const grids: Array<[string, any]> = [ + ['list', views.list], + ['my_week', views.listViews.my_week], + ['late', views.listViews.late], + ['stalled', views.listViews.stalled], + ]; + for (const [label, view] of grids) { + const names = (view?.bulkActionDefs ?? []).map((d: any) => d?.name); + expect(names, `${label} must offer bulk complete`).toContain(BULK_COMPLETE); + expect(names, `${label} must offer bulk skip`).toContain(BULK_SKIP); + } + // A calendar has no multi-select gesture to hang a selection bar on. + expect(views.listViews.calendar?.bulkActionDefs).toBeUndefined(); + }); + + it('is declarative — a data-plane update carrying the same patch as the row action', () => { + const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE); + expect(complete.operation).toBe('update'); + expect(complete.patch).toEqual(COMPLETE_PATCH); + // `execution` only applies to `operation: 'custom'`; the schema refuses it + // here, and its absence is what keeps this a data-plane write rather than + // N dispatches through the elevated action facade. + expect(complete.execution).toBeUndefined(); + + const skip = allBulkDefs().find((d) => d.name === BULK_SKIP); + expect(skip.operation).toBe('update'); + expect(skip.patch).toEqual(SKIP_PATCH); + // `patch` merges UNDER the collected params, so the reason lands beside + // the fixed status without being exposed as an editable status field. + expect(skip.params).toHaveLength(1); + expect(skip.params[0].name).toBe('skip_reason'); + expect(skip.params[0].required).toBe(true); + }); + + it('completes a week of tasks in one write path, all stamped', async () => { + const ids: string[] = []; + for (let i = 0; i < 20; i += 1) ids.push((await newTask({ subject: `bulk ${i}` })).id); + + // The data-plane form a `bulkActionDefs` update lowers to: ONE predicate + // write scoped to the selection, not 20 dispatches. + const affected = await data.update('duly_task', { ...COMPLETE_PATCH }, { + multi: true, + where: { id: { $in: ids } }, + }); + expect(affected).toBe(20); + + for (const id of ids) { + const row = await read(id); + expect(row.status).toBe('done'); + expect(row.completed_at, `${id} must be stamped by the hook like any other write`).toBeTruthy(); + } + }); + + it('the visible predicate is what keeps an already-done row out of the batch', async () => { + // MEASURED, and the reason the predicate is load-bearing rather than + // decoration: a predicate update carries ONE payload for all N rows + // (`driver.updateMany` takes one SET clause), so the `completed_at` the + // hook stamps for a row that IS transitioning is written to every row in + // the batch — including one that was completed days ago. + const open = (await newTask({ subject: 'still open' })).id; + const alreadyDone = (await newTask({ subject: 'done last week' })).id; + await dispatch(TASK_COMPLETE_ACTION, { recordId: alreadyDone }); + const original = (await read(alreadyDone)).completed_at; + await tick(); + + await data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }); + + expect( + (await read(alreadyDone)).completed_at, + 'a done row inside the batch has its completion instant overwritten — which is why the def excludes it', + ).not.toBe(original); + + // So the declaration has to exclude it, and does. + const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE); + const source = String(complete.visible?.source ?? ''); + expect(source).not.toContain('"done"'); + for (const status of ACTIONABLE_STATUSES) expect(source).toContain(`"${status}"`); + }); + + it('bulk skip writes the reason alongside the status', async () => { + const ids: string[] = []; + for (let i = 0; i < 3; i += 1) ids.push((await newTask({ subject: `skip ${i}` })).id); + + await data.update('duly_task', { ...SKIP_PATCH, skip_reason: 'Plant shutdown, week 34' }, { + multi: true, + where: { id: { $in: ids } }, + }); + + for (const id of ids) { + const row = await read(id); + expect(row.status).toBe('skipped'); + expect(row.skip_reason).toBe('Plant shutdown, week 34'); + } + }); + + it('a bulk skip with no reason is refused by the object rule, not silently dropped', async () => { + const id = (await newTask()).id; + let caught: any; + try { + await data.update('duly_task', { ...SKIP_PATCH }, { multi: true, where: { id: { $in: [id] } } }); + } catch (error) { + caught = error; + } + expect(caught, 'skip_needs_reason applies to the bulk path too').toBeDefined(); + expect(caught.code).toBe('VALIDATION_FAILED'); + expect((await read(id)).status).toBe('open'); + }); +});