From a74f87652086d1e4f5cca642604dde29994d5106 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 03:51:42 +0000 Subject: [PATCH 1/2] Stamp completed_at and last_update_at from a task lifecycle hook Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- src/hooks/index.ts | 6 +- src/hooks/task.hook.ts | 135 ++++++++++++++++++ test/task-hook.test.ts | 315 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 src/hooks/task.hook.ts create mode 100644 test/task-hook.test.ts diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 11e0f95..c78aa71 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -13,4 +13,8 @@ // makes `name` optional and fails the assignment. A named array is `never[]` // while empty and infers correctly the moment something is pushed into it. -export const dulyHooks = []; +import { TaskLifecycleHook } from './task.hook.js'; + +export { TaskLifecycleHook }; + +export const dulyHooks = [TaskLifecycleHook]; diff --git a/src/hooks/task.hook.ts b/src/hooks/task.hook.ts new file mode 100644 index 0000000..854f79f --- /dev/null +++ b/src/hooks/task.hook.ts @@ -0,0 +1,135 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { Hook, HookContext } from '@objectstack/spec/data'; + +/** + * `duly_task` lifecycle stamps — the two server-owned timestamps. + * + * Both `completed_at` and `last_update_at` are `readonly: true`, so a caller's + * value is stripped at the API boundary and this hook is their ONE writer. A + * value a `beforeUpdate` hook derives is not a caller write and survives that + * strip. + * + * ── Why this hook must stay in the barrel ──────────────────────────────── + * Hooks are read from `defineStack({ hooks })` only. A `*.hook.ts` that is not + * exported from `src/hooks/index.ts` type-checks, reads as wired, and never + * runs. `duly_task` carries a `completed_at_required_when_done` validation rule + * precisely so that an unregistered hook is a LOUD refusal — "a completed task + * must carry a completion timestamp" — instead of a done task committing with + * no timestamp. The rule is the assertion; this hook is what satisfies it. + * `test/task-hook.test.ts` pins the registration for the same reason. + * + * ── Pipeline facts this hook is built on (measured, not assumed) ───────── + * All three are asserted against a real booted engine in + * `test/task-hook.test.ts` rather than taken on trust: + * + * 1. `ctx.previous` is the pre-image, bound BEFORE the `beforeUpdate` chain + * runs. It is what makes a TRANSITION detectable rather than a state. + * 2. `beforeUpdate` runs ahead of the validation rules, so a write carrying + * only `{ status: 'done' }` has `completed_at` stamped by the time the + * completion rule is evaluated — that is why the bare write commits. + * 3. The readonly strip runs AFTER this hook and drops only a key still + * holding exactly what the caller supplied, so a value derived here + * replaces a caller-supplied one instead of being dropped with it. + * + * ── Why the handler is one self-contained function ─────────────────────── + * `objectstack build` lowers an inline handler into a metadata `body`, and a + * body ships without its module scope. A handler that referenced a + * module-level constant or helper still BUILDS — it silently falls back to the + * legacy bundled runtime module with a warning — so the pressure to keep this + * self-contained is a build warning, not an error. Keep the field list and the + * comparisons inside the function. + */ +const stampTaskLifecycle = (ctx: HookContext): void => { + // `input.` IS the record field on a declarative hook — reads resolve + // against the payload, writes land in it. The envelope spelling + // `input.data.` is deliberately NOT used: it works only off the raw + // `engine.registerHook` envelope and is a TypeError in the sandboxed body + // form this handler is lowered into — which, with `onError: 'abort'`, would + // refuse every task write rather than fail quietly. + const input = ctx.input as Record; + const now = new Date().toISOString(); + + if (ctx.event === 'beforeInsert') { + // A brand-new task has just been touched, by definition. + input.last_update_at = now; + return; + } + + const previous = ctx.previous as Record | undefined; + + // No pre-image means no transition can be READ, and the dispatch that arrives + // without one is the whole-operation context of an UNSCOPED predicate write — + // precisely the bulk write this hook must never let reset the clock. Stamping + // nothing is fail-safe in both directions: the stagnation signal is left + // alone, and a bulk write that tried to set `status = 'done'` is refused + // loudly by `completed_at_required_when_done` rather than committing a + // completed task with no completion timestamp. + if (!previous) return; + + // ── completed_at — stamped on the TRANSITION, not on the state ────────── + // Reading the next status from the payload but falling back to the stored one + // is what keeps a save that merely re-sends `status: 'done'` (every + // whole-record form submit on an already-done task) from overwriting the + // original completion instant. + const nextStatus = 'status' in input ? input.status : previous.status; + const wasDone = previous.status === 'done'; + const isDone = nextStatus === 'done'; + + if (!wasDone && isDone) { + input.completed_at = now; + } else if (wasDone && !isDone) { + // Reopened, skipped or cancelled — the completion is undone, so the + // timestamp goes with it, or the record keeps a completion that no longer + // happened. + input.completed_at = null; + } + + // ── last_update_at — only when a human moved the work ─────────────────── + // + // This list is the whole stagnation signal. The "Not moving" view is + // `status in (open, in_progress) AND last_update_at < {14_days_ago}`, so the + // clock may only advance when a person actually moved the work. Stamping on + // EVERY update would let one bulk re-owner, a business-unit backfill or an + // import silently reset it across the whole table — with no error anywhere, + // the numbers just quietly get better and the signal goes quiet exactly when + // it matters. + // + // Administrative and system-owned columns are therefore deliberately absent: + // `owner`, `business_unit`, `assignment`, `duty`, `due_date`, `visible_from`, + // `period_key`, `source`, `subject`. Re-owning, re-parenting or re-dating a + // task is not progress on it. Adding a field here widens the signal's blast + // radius; do it only for something a person changes BECAUSE they worked the + // task. + // + // Compared against the pre-image rather than merely tested for presence: a + // re-save carrying an unchanged `status` is not progress. + for (const field of ['status', 'note', 'skip_reason']) { + if (!(field in input)) continue; + const next = input[field]; + const prior = previous[field]; + const bothBlank = + (next === null || next === undefined) && (prior === null || prior === undefined); + if (!bothBlank && next !== prior) { + input.last_update_at = now; + return; + } + } +}; + +export const TaskLifecycleHook: Hook = { + name: 'duly_task_lifecycle_stamps', + label: 'Task lifecycle stamps', + object: 'duly_task', + events: ['beforeInsert', 'beforeUpdate'], + description: + 'Server-owned timestamps on duly_task: completed_at on the transition into and out of ' + + 'done, and last_update_at only when status, note or skip_reason actually changed — ' + + 'never on an administrative or bulk write, which would reset the stagnation signal.', + // Explicit because it is load-bearing rather than a default worth inheriting: + // if this handler throws, the write MUST be refused. Committing a task whose + // stamps were not applied is the exact silent corruption the + // `completed_at_required_when_done` rule exists to make impossible. + onError: 'abort', + handler: stampTaskLifecycle, +}; diff --git a/test/task-hook.test.ts b/test/task-hook.test.ts new file mode 100644 index 0000000..d1a6d92 --- /dev/null +++ b/test/task-hook.test.ts @@ -0,0 +1,315 @@ +// 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 { dulyHooks } from '../src/hooks/index.js'; + +/** + * `duly_task` lifecycle stamps. + * + * These run against a REAL booted ObjectQL engine (in-memory driver) with the + * app's own `objectstack.config.ts` as the bundle, rather than against a + * hand-made hook context. That matters for two reasons: + * + * - The hook only counts if it is reachable from `defineStack({ hooks })`. A + * `*.hook.ts` missing from the barrel type-checks and reads as wired, so a + * test that imported the handler directly would pass on dead metadata. Here + * the handler runs only because `AppPlugin` found it in the real config. + * - The interesting behaviour is not the handler in isolation, it is the + * handler's PLACE in the write pipeline: ahead of the validation rules, + * behind the pre-image read, and ahead of the readonly strip. + * + * Timestamps are ISO-8601, so lexical `>` is chronological. Each write that + * should move the clock is preceded by a short sleep, which is what makes the + * "does NOT advance" assertions mean something: without it an errant stamp + * could land in the same millisecond and compare equal. + */ + +let data: any; +let kernel: any; + +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 } }); + +beforeAll(async () => { + const { plugins } = await createStandaloneStack({ + databaseDriver: 'memory', + skipSeedData: true, + }); + 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'); +}, 120_000); + +afterAll(async () => { + await kernel?.shutdown?.(); +}); + +// ── The wiring, which is the failure mode that reads as success ──────────── +describe('registration', () => { + it('is exported from the hooks barrel', () => { + const hook = dulyHooks.find((h) => h.name === 'duly_task_lifecycle_stamps'); + expect(hook, 'the hook must be in dulyHooks or it never runs').toBeDefined(); + expect(hook?.object).toBe('duly_task'); + expect(hook?.events).toEqual(['beforeInsert', 'beforeUpdate']); + }); + + it('reaches defineStack({ hooks }) — the only place the runtime reads', () => { + const names = (stack.hooks ?? []).map((h: any) => h.name); + expect(names).toContain('duly_task_lifecycle_stamps'); + }); +}); + +// ── The pipeline facts the design depends on ─────────────────────────────── +describe('platform mechanics this hook relies on', () => { + it('hands beforeUpdate the pre-image, so a TRANSITION is detectable', async () => { + const seen: Array | undefined> = []; + data.registerHook( + 'beforeUpdate', + (ctx: any) => { seen.push(ctx.previous); }, + { object: 'duly_task', hookName: 'test_previous_probe' }, + ); + + const task = await newTask({ status: 'in_progress' }); + await data.update('duly_task', { id: task.id, note: 'probing' }); + + expect(seen.length).toBe(1); + expect(seen[0], 'ctx.previous must be bound on beforeUpdate').toBeDefined(); + expect(seen[0]?.status).toBe('in_progress'); + expect(seen[0]?.id).toBe(task.id); + }); + + it('enforces completed_at_required_when_done — the hook is not trusted, it is checked', async () => { + // The negative control for every "done" assertion below. beforeInsert + // deliberately does not stamp completed_at, so an insert that asks for + // `done` is refused. If this ever stops throwing, the validation rule has + // gone quiet and the completion assertions prove nothing. + await expect(newTask({ status: 'done' })).rejects.toThrow( + /completion timestamp/i, + ); + }); +}); + +describe('completed_at', () => { + it('is left blank on insert, and last_update_at is stamped', async () => { + const task = await newTask(); + expect(task.last_update_at, 'a new task has just been touched').toBeTruthy(); + expect(task.completed_at ?? null).toBeNull(); + }); + + it('a write carrying ONLY { status: done } commits, with completed_at set', async () => { + // The stamp lands ahead of the validation rules, which is the whole reason + // a bare completion does not have to carry a timestamp of its own. + const task = await newTask(); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + + expect(done.status).toBe('done'); + expect(done.completed_at, 'completed_at must be stamped by the hook').toBeTruthy(); + expect(new Date(done.completed_at as string).getTime()).not.toBeNaN(); + }); + + it('reopening clears it, and the reopened record passes validation', async () => { + const task = await newTask(); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + expect(done.completed_at).toBeTruthy(); + + const reopened = await data.update('duly_task', { + id: task.id, + status: 'in_progress', + }); + expect(reopened.status).toBe('in_progress'); + expect(reopened.completed_at ?? null, 'a reopened task is not completed').toBeNull(); + + // And it is genuinely persisted, not just echoed back by the write. + expect((await read(task.id)).completed_at ?? null).toBeNull(); + }); + + it('is not re-stamped when an already-done task is saved again', async () => { + // Every whole-record form submit re-sends `status: 'done'`. That is a state, + // not a transition, and overwriting the original completion instant on each + // save would corrupt the one timestamp completion reporting reads. + const task = await newTask(); + const done = await data.update('duly_task', { id: task.id, status: 'done' }); + const firstCompletion = done.completed_at; + + await tick(); + const resaved = await data.update('duly_task', { + id: task.id, + status: 'done', + note: 'adding a note after the fact', + }); + + expect(resaved.completed_at).toBe(firstCompletion); + }); + + it('strips and replaces a caller-supplied value', async () => { + const task = await newTask(); + const forged = '1999-01-01T00:00:00.000Z'; + const done = await data.update('duly_task', { + id: task.id, + status: 'done', + completed_at: forged, + }); + + expect(done.completed_at, 'the caller does not get to choose this').not.toBe(forged); + expect(new Date(done.completed_at as string).getTime()).toBeGreaterThan( + new Date(forged).getTime(), + ); + }); + + it('drops a caller-supplied value on a write the hook does not stamp', async () => { + // No transition here, so the hook writes nothing and the readonly strip is + // the only thing standing between the caller and the column. + const task = await newTask(); + const before = (await read(task.id)).completed_at ?? null; + + await data.update('duly_task', { + id: task.id, + completed_at: '1999-01-01T00:00:00.000Z', + }); + + expect((await read(task.id)).completed_at ?? null).toBe(before); + }); +}); + +describe('last_update_at — the stagnation signal', () => { + it('advances when the note is edited', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const edited = await data.update('duly_task', { id: task.id, note: 'chased the vendor' }); + + expect(edited.last_update_at as string > before).toBe(true); + }); + + it('advances on a status change', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const moved = await data.update('duly_task', { id: task.id, status: 'in_progress' }); + + expect(moved.last_update_at as string > before).toBe(true); + }); + + it('advances when a skip reason is recorded', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const skipped = await data.update('duly_task', { + id: task.id, + status: 'skipped', + skip_reason: 'the plant was down, there was nothing to return', + }); + + expect(skipped.last_update_at as string > before).toBe(true); + }); + + /** + * THE assertion this hook exists for. + * + * `last_update_at` feeds the "Not moving" view — `status in (open, + * in_progress) AND last_update_at < {14_days_ago}`. A hook that stamped on + * every update would let one bulk re-owner, a business-unit backfill or an + * import silently reset the clock across the whole table. Nothing would + * error; the stagnation numbers would simply improve and the signal would go + * quiet exactly when it matters. So an administrative write must leave the + * clock alone, and that is checked here directly rather than inferred. + */ + it('does NOT advance on an administrative write (business_unit)', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const rebadged = await data.update('duly_task', { + id: task.id, + business_unit: 'bu_north', + }); + + expect(rebadged.business_unit).toBe('bu_north'); + expect( + rebadged.last_update_at, + 'a business-unit backfill is not progress on the task', + ).toBe(before); + }); + + it('does NOT advance on a re-owner', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const reowned = await data.update('duly_task', { id: task.id, owner: 'user_bob' }); + + expect(reowned.owner).toBe('user_bob'); + expect(reowned.last_update_at).toBe(before); + }); + + it('does NOT advance on a re-date', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + + await tick(); + const redated = await data.update('duly_task', { id: task.id, due_date: '2026-12-31' }); + + expect(redated.last_update_at).toBe(before); + }); + + it('does NOT advance on a re-save with no changes', async () => { + const task = await newTask({ note: 'as found' }); + const before = task.last_update_at as string; + + await tick(); + const resaved = await data.update('duly_task', { + id: task.id, + status: 'open', + note: 'as found', + }); + + expect( + resaved.last_update_at, + 're-sending unchanged values is not a touch', + ).toBe(before); + }); + + it('does NOT accept a caller-supplied value', async () => { + const task = await newTask(); + const before = task.last_update_at as string; + const forged = '2099-01-01T00:00:00.000Z'; + + await tick(); + await data.update('duly_task', { id: task.id, last_update_at: forged }); + + expect((await read(task.id)).last_update_at).toBe(before); + }); + + it('is stamped by the hook, not copied from the caller, on a real edit', async () => { + const task = await newTask(); + const forged = '2099-01-01T00:00:00.000Z'; + + await tick(); + const edited = await data.update('duly_task', { + id: task.id, + note: 'a real edit', + last_update_at: forged, + }); + + expect(edited.last_update_at).not.toBe(forged); + expect(edited.last_update_at as string < forged).toBe(true); + }); +}); From 864ba789be4f0ad948685c652e988a4658cb6790 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 03:58:15 +0000 Subject: [PATCH 2/2] =?UTF-8?q?Make=20the=20hook=20suite=20hermetic=20?= =?UTF-8?q?=E2=80=94=20never=20read=20a=20build=20artifact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Left at its default, createStandaloneStack resolves /dist/objectstack.json and the kernel loads its metadata — objects AND hooks — from that file. With a local pnpm build present the suite then reported on the last build rather than on src/, and the registration ablation came back green with the barrel entry deleted. Point the lookup at a sentinel path instead. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- test/task-hook.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/task-hook.test.ts b/test/task-hook.test.ts index d1a6d92..6c24387 100644 --- a/test/task-hook.test.ts +++ b/test/task-hook.test.ts @@ -47,6 +47,16 @@ 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 when a local + // `pnpm build` has left one there the kernel loads its metadata — objects + // AND hooks — from that file instead of from the config imported above. + // The suite then reports on the last BUILD rather than on `src/`, passes + // with the barrel entry deleted, and behaves differently in CI (where + // `pnpm test` runs before `pnpm build` and no artifact exists) than it does + // on a developer's machine. Measured, not hypothetical: it is what made the + // registration ablation come back green. + artifactPath: 'dist/objectstack.this-suite-must-not-load-an-artifact.json', }); kernel = new ObjectKernel(); for (const plugin of plugins) await kernel.use(plugin);